Re: [RFC PATCH v3 2/5] i2c: of: Introduce component probe function

[Rob Herring <robh@kernel.org>]

On Fri, Dec 01, 2023 at 04:57:46PM -0800, Doug Anderson wrote:
> Hi,
> 
> On Tue, Nov 28, 2023 at 12:45 AM Chen-Yu Tsai <wenst@chromium.org> wrote:
> >
> > @@ -217,4 +217,114 @@ static int of_i2c_notify(struct notifier_block *nb, unsigned long action,
> >  struct notifier_block i2c_of_notifier = {
> >         .notifier_call = of_i2c_notify,
> >  };
> > +
> > +/*
> > + * Some devices, such as Google Hana Chromebooks, are produced by multiple
> > + * vendors each using their preferred components. Such components are all
> > + * in the device tree. Instead of having all of them enabled and having each
> > + * driver separately try and probe its device while fighting over shared
> > + * resources, they can be marked as "fail-needs-probe" and have a prober
> > + * figure out which one is actually used beforehand.
> > + *
> > + * This prober assumes such drop-in parts are on the same I2C bus, have
> > + * non-conflicting addresses, and can be directly probed by seeing which
> > + * address responds.
> > + *
> > + * TODO:
> > + * - Support handling common regulators and GPIOs.
> 
> IMO you should prototype how you're going to handle regulators and
> GPIOs before finalizing the design. I was going to write that you
> should just document that it was up to the caller to power things up
> before calling this function, but then I realized that the caller
> would have to duplicate much of this function in order to do so. In
> the very least they'd have to find the nodes of the relevant devices
> so that they could grab regulators and/or GPIOs. In order to avoid
> this duplication, would the design need to change? Perhaps this would
> be as simple as adding a callback function here that's called with all
> of the nodes before probing? If that's right, it would be nice to have
> that callback from the beginning so we don't need two variants of the
> function...
> 
> > + * - Support I2C muxes
> > + */
> > +
> > +/**
> > + * i2c_of_probe_component() - probe for devices of "type" on the same i2c bus
> > + * @dev: &struct device of the caller, only used for dev_* printk messages
> > + * @type: a string to match the device node name prefix to probe for
> > + *
> > + * Probe for possible I2C components of the same "type" on the same I2C bus
> > + * that have their status marked as "fail".
> 
> Should document these current limitations with the code:
> 
> * Assumes that across the entire device tree the only instances of
> nodes named "type" are ones we're trying to handle second sourcing
> for. In other words if we're searching for "touchscreen" then all
> nodes named "touchscreen" are ones that need to be probed.

named "type" and marked as needs probe.

> 
> * Assumes that there is exactly one group of each "type". In other
> words, if we're searching for "touchscreen" then exactly one
> touchscreen will be enabled across the whole tree.

Does that need to be a limitation? If you just keep going thru all 
devices, wouldn't that just work?

Rob