Re: [PATCH v4 01/11] firmware: scmi: switch to dev_ofnode during bind

Hi Anshul,

On 2026-08-04T21:55:02, Bryan Brattlof <[email protected]> wrote:
> firmware: scmi: switch to dev_ofnode during bind

>
> When using ofnode_null() the node offset passed to the FDT API will be
> -1 which is invalid and breaks probe. Switch to dev_ofnode()

Please flesh this out a bit. Which transport is this seen on, which
FDT API call trips, and what does the failure look like (return code /
dev_err message)? A Fixes: tag pointing at 7eb4eb541c1 ('firmware:
scmi: install base protocol to SCMI agent') would also help - that is
where the ofnode_null() was introduced.

>
> When using ofnode_null() the node offset passed to the FDT API will be
> -1 which is invalid and breaks probe. Switch to dev_ofnode()
>
> Tested-by: Anshul Dalal <[email protected]>
> Signed-off-by: Anshul Dalal <[email protected]>
> Signed-off-by: Bryan Brattlof <[email protected]>
>
> drivers/firmware/scmi/scmi_agent-uclass.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

> diff --git a/drivers/firmware/scmi/scmi_agent-uclass.c b/drivers/firmware/scmi/scmi_agent-uclass.c
> @@ -437,7 +437,7 @@ static int scmi_bind_protocols(struct udevice *dev)
>       /* initialize the device from device tree */
>       drv = DM_DRIVER_GET(scmi_base_drv);
>       name = "scmi-base.0";
> -     ret = device_bind(dev, drv, name, NULL, ofnode_null(), &proto);
> +     ret = device_bind(dev, drv, name, NULL, dev_ofnode(dev), &proto);

I'm worried about the side effect here. Previously the base protocol
device had no ofnode, so in the transports (mailbox_agent.c,
optee_agent.c, smccc_agent.c, sandbox-scmi_agent.c) the
'!dev_has_ofnode(protocol)' or dev_read_u32(protocol, ...) check would
fail and the code would correctly fall back to the agent's base
channel. With dev_ofnode(dev), the base protocol now shares the parent
SCMI node, and any transport-specific properties on the SCMI node
itself (e.g. 'arm,smc-id', 'linaro,optee-channel-id', 'shmem') would
be picked up on the base protocol device and cause it to try to set up
a dedicated channel rather than reuse the agent's. Have you tested
this against SMCCC or OP-TEE transports? It might be safer to keep the
base device ofnode-less and teach the failing call site to tolerate
ofnode_null(), or use a specific getter that does the right thing.

Regards,
Simon