lkml.org 
[lkml]   [2016]   [Aug]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v3 1/4] hwmon: iio_hwmon: delay probing with late_initcall
    On Mon, Aug 15, 2016 at 04:40:21PM +0100, Jonathan Cameron wrote:
    > On 26/07/16 17:04, Guenter Roeck wrote:
    > > On Tue, Jul 26, 2016 at 12:00:33PM +0200, Alexander Stein wrote:
    > >> On Tuesday 26 July 2016 11:33:59, Quentin Schulz wrote:
    > >>> On 26/07/2016 11:05, Alexander Stein wrote:
    > >>>> On Tuesday 26 July 2016 10:24:48, Quentin Schulz wrote:
    > >>>>> On 26/07/2016 10:21, Alexander Stein wrote:
    > >>>>>> On Tuesday 26 July 2016 09:43:44, 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.
    > >>>>>>
    > >>>>>> Would it work if iio_channel_get_all returning ENODEV is used for
    > >>>>>> returning
    > >>>>>> EPROBE_DEFER in iio_channel_get_all? Using late initcalls for
    > >>>>>> driver/device
    > >>>>>> dependencies seems not right for me at this place.
    > >>>>>
    > >>>>> Then what if the iio_channel_get_all is called outside of the probe of a
    > >>>>> driver? We'll have to change the error code, things we are apparently
    > >>>>> trying to avoid (see v2 patches' discussions).
    > >>>>
    > >>>> Maybe I didn't express my idea enough. I don't want to change the behavior
    > >>>> of iio_channel_get_all at all. Just the result evaluation of
    > >>>> iio_channel_get_all in iio_hwmon_probe. I have something link the patch
    > >>>> below in mind.
    > >>>>
    > >>>> Best regards,
    > >>>> Alexander
    > >>>> ---
    > >>>> diff --git a/drivers/hwmon/iio_hwmon.c b/drivers/hwmon/iio_hwmon.c
    > >>>> index b550ba5..e32d150 100644
    > >>>> --- a/drivers/hwmon/iio_hwmon.c
    > >>>> +++ b/drivers/hwmon/iio_hwmon.c
    > >>>> @@ -73,8 +73,12 @@ static int iio_hwmon_probe(struct platform_device
    > >>>> *pdev)
    > >>>>
    > >>>> name = dev->of_node->name;
    > >>>>
    > >>>> channels = iio_channel_get_all(dev);
    > >>>>
    > >>>> - if (IS_ERR(channels))
    > >>>> - return PTR_ERR(channels);
    > >>>> + if (IS_ERR(channels)) {
    > >>>> + if (PTR_ERR(channels) == -ENODEV)
    > >>>> + return -EPROBE_DEFER;
    > >>>> + else
    > >>>> + return PTR_ERR(channels);
    > >>>> + }
    > >>>>
    > >>>> st = devm_kzalloc(dev, sizeof(*st), GFP_KERNEL);
    > >>>> if (st == NULL) {
    > >>>
    > >>> Indeed, I misunderstood what you told me.
    > >>>
    > >>> Actually, the patch you proposed is part of my v1
    > >>> (https://lkml.org/lkml/2016/6/28/203) and v2
    > >>> (https://lkml.org/lkml/2016/7/15/215).
    > >>> Jonathan and Guenter didn't really like the idea of changing the -ENODEV
    > >>> in -EPROBE_DEFER.
    > >>
    > >> Thanks for the links.
    > >>
    > >>> What I thought you were proposing was to change the -ENODEV return code
    > >>> inside iio_channel_get_all. This cannot be an option since the function
    > >>> might be called outside of a probe (it is not yet, but might be in the
    > >>> future?).
    > >>
    > >> AFAICS this is a helper function not knowing about device probing itself. And
    > >> it should stay at that.
    > >>
    > >>> Of what I understood, two possibilities are then possible (proposed
    > >>> either by Guenter or Jonathan): either rework the iio framework to
    > >>> register iio map array earlier or to use late_initcall instead of init
    > >>> for the driver consuming the iio channels.
    > >>
    > >> Interestingly using this problem would not arise due to module dependencies.
    > >> But using late_initcall would mean this needs to be done on any driver using
    > >> iio channels? I would rather keep those consumers simple.
    > >>
    > > Me too, but that would imply a solution in iio. The change you propose above
    > > isn't exactly simple either, and would also be needed in each consumer driver.
    > >
    > > Just for the record, I dislike the late_initcall solution as well, but I prefer
    > > it over blindly converting ENODEV to EPROBE_DEFER.
    > I'm falling on the other side on this one right now. Though I'd be tempted
    > to renaming the function to something like iio_channel_get_all_or_defer
    > to make it explicit that it can result in deferred probing.
    >
    Would this new function return -EPROBE_DEFER instead of -ENODEV ?

    Thanks,
    Guenter

    \
     
     \ /
      Last update: 2016-09-17 09:56    [W:3.339 / U:1.012 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site