[PATCH] i2c: mux: Fix the crash when the i2c-arbitrator node is present
Heiko Schocher
hs at denx.de
Wed May 28 11:34:58 CEST 2025
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 at 72 i.e., i2c at f1950000->i2c-arbitrator at 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.abbarapu at 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: hs at denx.de
More information about the U-Boot
mailing list