| Message ID | 20240513-boston-v1-9-fac96938417e@flygoat.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Daniel Schwierzeck |
| Headers | show |
| Series | MIPS: Boston: Various enhancements | expand |
Hi Jiaxun, On 2024-05-13 20:13, Jiaxun Yang wrote: > When bootph-all is enabled for a syscon driver, the device > may leave unprobed when syscon_get_regmap is called by another > driver. > > Perform device_probe in syscon_get_regmap, there is no side > affect as device_probe will return 0 quickly for an activated > device. > > Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com> > --- > drivers/core/syscon-uclass.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/core/syscon-uclass.c b/drivers/core/syscon-uclass.c > index f0e69d7216b3..b09f7013194d 100644 > --- a/drivers/core/syscon-uclass.c > +++ b/drivers/core/syscon-uclass.c > @@ -32,10 +32,14 @@ > */ > struct regmap *syscon_get_regmap(struct udevice *dev) > { > + int ret; > struct syscon_uc_info *priv; > > if (device_get_uclass_id(dev) != UCLASS_SYSCON) > return ERR_PTR(-ENOEXEC); > + ret = device_probe(dev); > + if (ret) > + return ERR_PTR(ret); Please explain in more details what the issue this is trying to solve. Typically syscon_get_regmap() is called on a udevice returned from a uclass_get_device call, and that should trigger a probe for the device and its parents. Adding device_probe() here possible just hides an issue that exists somewhere else. In what instance are you ending up with a call to this function with a udevice that has not been probed? Also, please add a new test to test/dm/regmap.c if this solves a real issue. Regards, Jonas > priv = dev_get_uclass_priv(dev); > return priv->regmap; > } >
在2024年5月14日五月 下午3:50，Jonas Karlman写道： > Hi Jiaxun, [...] >> + return ERR_PTR(ret); > > Please explain in more details what the issue this is trying to solve. > > Typically syscon_get_regmap() is called on a udevice returned from a > uclass_get_device call, and that should trigger a probe for the device > and its parents. > > Adding device_probe() here possible just hides an issue that exists > somewhere else. In what instance are you ending up with a call to > this function with a udevice that has not been probed? Hi Jonas, Thanks for the reply, my problem is in [PATCH 10/13] I'm using dev->parent directly to get parent device and then use that pointer to access syscon_get_regmap. There is no chance to invoke uclass_get_device_* as what we need is just the parent device. I can think of two solution without touching syscon code here. First would be performing uclass_get_device_by_ofnode against parent's ofnode, which seems a little bit overkilling. Second would be trigger probing in dev_get_parent. Thanks - Jiaxun > > Also, please add a new test to test/dm/regmap.c if this solves a real > issue. > > Regards, > Jonas > >> priv = dev_get_uclass_priv(dev); >> return priv->regmap; >> } >>
Hi Jiaxun, On 2024-05-15 02:12, Jiaxun Yang wrote: > > > 在2024年5月14日五月 下午3:50，Jonas Karlman写道： >> Hi Jiaxun, > [...] >>> + return ERR_PTR(ret); >> >> Please explain in more details what the issue this is trying to solve. >> >> Typically syscon_get_regmap() is called on a udevice returned from a >> uclass_get_device call, and that should trigger a probe for the device >> and its parents. >> >> Adding device_probe() here possible just hides an issue that exists >> somewhere else. In what instance are you ending up with a call to >> this function with a udevice that has not been probed? > > Hi Jonas, > > Thanks for the reply, my problem is in [PATCH 10/13] I'm using dev->parent > directly to get parent device and then use that pointer to access > syscon_get_regmap. Looking at the driver model lifecycle docs and the clk_boston driver I would suggest you to change from using the of_to_plat() ops and do same in probe() ops instead. That way your parent udevice will have been probed and you do not need to manually probe it at of_to_plat() stage. https://docs.u-boot.org/en/latest/develop/driver-model/design.html#driver-lifecycle """ If your device relies on its parent setting up a suitable address space, so that dev_read_addr() works correctly, then make sure that the parent device has its setup code in of_to_plat(). If it has it in the probe method, then you cannot call dev_read_addr() from the child device's of_to_plat() method. Move it to probe() instead. """ Moving your syscon_get_regmap() call from of_to_plat() to probe() should make this syscon patch unnecessary. Regards, Jonas > > There is no chance to invoke uclass_get_device_* as what we need is just > the parent device. > > I can think of two solution without touching syscon code here. > > First would be performing uclass_get_device_by_ofnode against parent's > ofnode, which seems a little bit overkilling. > > Second would be trigger probing in dev_get_parent. > > Thanks > - Jiaxun > >> >> Also, please add a new test to test/dm/regmap.c if this solves a real >> issue. >> >> Regards, >> Jonas >> >>> priv = dev_get_uclass_priv(dev); >>> return priv->regmap; >>> } >>> >
diff --git a/drivers/core/syscon-uclass.c b/drivers/core/syscon-uclass.c index f0e69d7216b3..b09f7013194d 100644 --- a/drivers/core/syscon-uclass.c +++ b/drivers/core/syscon-uclass.c @@ -32,10 +32,14 @@ */ struct regmap *syscon_get_regmap(struct udevice *dev) { + int ret; struct syscon_uc_info *priv; if (device_get_uclass_id(dev) != UCLASS_SYSCON) return ERR_PTR(-ENOEXEC); + ret = device_probe(dev); + if (ret) + return ERR_PTR(ret); priv = dev_get_uclass_priv(dev); return priv->regmap; }
When bootph-all is enabled for a syscon driver, the device may leave unprobed when syscon_get_regmap is called by another driver. Perform device_probe in syscon_get_regmap, there is no side affect as device_probe will return 0 quickly for an activated device. Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com> --- drivers/core/syscon-uclass.c | 4 ++++ 1 file changed, 4 insertions(+)