Re: [PATCH] i2c: mux: Fix the crash when the i2c-arbitrator node is present

[Heiko Schocher]

Hello Venkatesh,

On 28.05.25 05:12, Venkatesh Yadav Abbarapu wrote:

Observing the crash when we add the i2c-arbitrator node in the device

Which crash?

tree as per the DT bindings. The issue is with the child node of
i2c-arbitrator@72 i.e., i2c@f1950000->i2c-arbitrator@72->i2c-arb, as the
arbitrator uses the uclass of mux(UCLASS_I2C_MUX) and the mux uclass driver
checks for the "reg" property using the i2c_mux_child_post_bind() function,
if it won't find the "reg" property it will return -EINVAL which is leading
to the crash.
So, add the logic to check whether the  child node has the "reg" property,
if the "reg" property exists then read the "reg" and update the channel.

https://www.kernel.org/doc/Documentation/devicetree/bindings/i2c/i2c-arb.txt

Signed-off-by: Venkatesh Yadav Abbarapu <venkatesh.abbar...@amd.com>
---
  drivers/i2c/muxes/i2c-mux-uclass.c | 11 +++++++----
  1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/muxes/i2c-mux-uclass.c 
b/drivers/i2c/muxes/i2c-mux-uclass.c
index d1999d21feb..b18999c1fe3 100644
--- a/drivers/i2c/muxes/i2c-mux-uclass.c
+++ b/drivers/i2c/muxes/i2c-mux-uclass.c
@@ -40,11 +40,14 @@ static int i2c_mux_child_post_bind(struct udevice *dev)
        struct i2c_mux_bus *plat = dev_get_parent_plat(dev);
        int channel;

- channel = dev_read_u32_default(dev, "reg", -1);

Shouldn;t this function return -1 (the default value), if property "reg" is
not present? (And also if dev_ofnode(dev) is not valid?)

Seems we should look into this function and may fix the problem there?

Aha, this comes from

drivers/core/read.c
int dev_read_u32_default(const struct udevice *dev, const char *propname,
                         int def)
{
        return ofnode_read_u32_default(dev_ofnode(dev), propname, def);
}

with drivers/core/ofnode.c
u32 ofnode_read_u32_default(ofnode node, const char *propname, u32 def)
{
        assert(ofnode_valid(node));
        ofnode_read_u32_index(node, propname, 0, &def);

        return def;
}

added Simon.

@Simon: may proper fix would be to check in ofnode_read_u32_default()
        if propname exists, and if not return default value?
        May also return with default value if node is not valid?

        What do you think?

        Thanks!

-       if (channel < 0)
-               return -EINVAL;
-       plat->channel = channel;
+       ofnode node = dev_ofnode(dev);

+ if (ofnode_has_property(node, "reg")) {

If we fix it in this code, you can here simply return immediately
if "reg" propertyis missing, so you can save the identation level,
and patch looks cleaner.

+               channel = dev_read_u32_default(dev, "reg", -1);
+               if (channel < 0)
+                       return -EINVAL;
+               plat->channel = channel;
+       }
        return 0;
  }

Thanks!

bye,
Heiko
--
DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: h...@denx.de