Re: [3/3] hwmon: iio_hwmon: defer probe when no channel is found

[Guenter Roeck]

On 07/03/2016 03:47 AM, Jonathan Cameron wrote:
> On 30/06/16 15:51, Guenter Roeck wrote:
> > On 06/30/2016 06:59 AM, Jonathan Cameron wrote:
> > > On 30 June 2016 04:47:25 BST, Guenter Roeck <linux@xxxxxxxxxxxx> wrote:
> > > > On Tue, Jun 28, 2016 at 10:18:17AM +0200, Quentin Schulz wrote:
> > > > > iio_channel_get_all returns -ENODEV when it cannot find either
> > > > phandles and
> > > > > properties in the Device Tree or channels whose consumer_dev_name
> > > > matches
> > > > > iio_hwmon in iio_map_list. The iio_map_list is filled in by iio
> > > > drivers
> > > > > which might be probed after iio_hwmon.
> > > > >
> > > > > It is better to defer the probe of iio_hwmon if such error is
> > > > returned by
> > > > > iio_channel_get_all in order to let a chance to iio drivers to expose
> > > > > channels in iio_map_list.
> > > > >
> > > > > Signed-off-by: Quentin Schulz <quentin.schulz@xxxxxxxxxxxxxxxxxx>
> > > > > ---
> > > > >    drivers/hwmon/iio_hwmon.c | 5 ++++-
> > > > >    1 file changed, 4 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/hwmon/iio_hwmon.c b/drivers/hwmon/iio_hwmon.c
> > > > > index b550ba5..c0da4d9 100644
> > > > > --- a/drivers/hwmon/iio_hwmon.c
> > > > > +++ b/drivers/hwmon/iio_hwmon.c
> > > > > @@ -73,8 +73,11 @@ static int iio_hwmon_probe(struct platform_device
> > > > *pdev)
> > > > >            name = dev->of_node->name;
> > > > >
> > > > >        channels = iio_channel_get_all(dev);
> > > > > -    if (IS_ERR(channels))
> > > > > +    if (IS_ERR(channels)) {
> > > > > +        if (PTR_ERR(channels) == -ENODEV)
> > > > > +            return -EPROBE_DEFER;
> > > >
> > > > The problem, as I see it, is with iio, which should return
> > > > -EPROBE_DEFER
> > > > in this situation.
> > > Agreed. New fangled stuff this deferred probing :)
> > > > We can not convert -ENODEV to -EPROBE_DEFER without risking that the
> > > > channels are _really_ not there, which would result in endless
> > > > "deferred"
> > > > messages.
> > > Hmm not entirely sure how we prevent that happening wherever it is done..
> >
> > Outch. Better at the source, though. I didn't look at the iio code recently,
> > but can you detect the defer situation at least with devicetree ?
> >
> > For non-devicetree situations, the only option I can think of would be
> > to replace the module initcall with a later initcall. That should solve
> > the problem if both iio_hwmon and and underlying drivers are built
> > into the kernel. If iio_hwmon is modular, the only real option I can
> > see is to make sure that all drivers it needs are loaded first.
> >
> > Does this make sense ?
> I think we need to look in a couple of directions.  Firstly, investigate doing
> something similar to gpio and basically move the setup of maps much earlier.
> This will replace drivers presenting their own maps
>
> The other direction is to get userspace (i.e. configfs) setup of these maps
> working so for cases where it's a bit less hardware defined (i.e. using an
> accelerometer as an input device) we can do once we know all devices relevant
> are present and instantiate new instances on the fly.
>
> Anyhow, neither is trivial unfortunately.

On a higher level, I think it would be better if the iio-hwmon bridge was tied
to chips (and thus to the chip driver), and not be an independent binding.
No idea if/how we can do that, though.

Guenter