[AMD Official Use Only - AMD Internal Distribution Only] Hi Heiko,
> -----Original Message----- > From: Heiko Schocher <[email protected]> > Sent: Wednesday, May 28, 2025 3:05 PM > To: Abbarapu, Venkatesh <[email protected]>; [email protected]; > Simon Glass <[email protected]> > Cc: Simek, Michal <[email protected]>; [email protected]; git (AMD-Xilinx) > <[email protected]> > Subject: Re: [PATCH] i2c: mux: Fix the crash when the i2c-arbitrator node is > present > > 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? Observing crash like below U-Boot 2025.01 (May 22 2025 - 12:32:01 +0000) CPU: ZynqMP Silicon: v3 Chip: xck24 Model: ZynqMP SC K24 RevA Board: Xilinx ZynqMP DRAM: 2 GiB initcall failed at call 0000000008037364 (err=-22) ### ERROR ### Please RESET the board ### > > > 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-a > > rb.txt > > > > Signed-off-by: Venkatesh Yadav Abbarapu <[email protected]> > > --- > > 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! Thanks Venkatesh > > 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: [email protected]
