[AMD Official Use Only - AMD Internal Distribution Only] Hi @Simon Glass, Do you have any feedback on this?
Thanks Venkatesh > -----Original Message----- > From: Abbarapu, Venkatesh <[email protected]> > Sent: Wednesday, May 28, 2025 3:47 PM > To: [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 > > [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]
