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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <b1ebf22d-389e-5d2e-93f2-f77728d28f28@kernel.org>
Date:	Sun, 3 Jul 2016 11:47:17 +0100
From:	Jonathan Cameron <jic23@...nel.org>
To:	Guenter Roeck <linux@...ck-us.net>,
	Jonathan Cameron <jic23@...23.retrosnub.co.uk>,
	Quentin Schulz <quentin.schulz@...e-electrons.com>
Cc:	jdelvare@...e.com, knaack.h@....de, lars@...afoo.de,
	pmeerw@...erw.net, maxime.ripard@...e-electrons.com, wens@...e.org,
	lee.jones@...aro.org, linux-kernel@...r.kernel.org,
	linux-hwmon@...r.kernel.org, linux-iio@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org,
	thomas.petazzoni@...e-electrons.com,
	antoine.tenart@...e-electrons.com
Subject: Re: [3/3] hwmon: iio_hwmon: defer probe when no channel is found

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@...ck-us.net> 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@...e-electrons.com>
>>>> ---
>>>>   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.

J
> 
> Guenter
> 
>>>
>>> Guenter
>>>
>>>>           return PTR_ERR(channels);
>>>> +    }
>>>>
>>>>       st = devm_kzalloc(dev, sizeof(*st), GFP_KERNEL);
>>>>       if (st == NULL) {
>>> -- 
>>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>>> the body of a message to majordomo@...r.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
> 
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html