On Fri, Aug 23, 2024 at 05:35:59PM +0800, Chen-Yu Tsai wrote: > On Thu, Aug 22, 2024 at 10:09 PM Andy Shevchenko > <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote: > > On Thu, Aug 22, 2024 at 05:20:00PM +0800, Chen-Yu Tsai wrote: ... > > Hmm... why not > > > > static int i2c_of_probe_get_res(struct device *dev, struct device_node *node, > > struct i2c_of_probe_data *data) > > { > > struct property *prop; > > int ret; > > > > ret = i2c_of_probe_get_regulator(dev, node, data); > > if (ret < 0) { > > i2c_of_probe_free_res(data); > > return dev_err_probe(dev, ret, "Failed to get regulator supplies from %pOF\n", node); > > } > > > > return 0; > > } > > That would be more churn in the next patch, which introduces another > error condition requiring the same cleanup. OK! ... > > > + /* largest post-power-on pre-reset-deassert delay seen among drivers */ > > > + msleep(500); > > > > How would we monitor if any [new] driver wants to use bigger timeout? > > The assumption is that the person doing the integration should test for > this. This prober doesn't get called everywhere. It needs a driver to > call it, and that driver is written by someone for some specific platform. > Maybe I should explicitly spell that out in the function description? > Or even make it a parameter? > > Also, having an arbitrarily large number here doesn't help platforms that > want to minimize boot time. On that front I'm also thinking about whether > it is possible to do a handover to the actual driver so that the latter > doesn't have to go through the whole power sequence again. Yeah, I think the best effort is to have a parameter. -- With Best Regards, Andy Shevchenko