| Message ID | 5277864.qlyRKbC1Zy@ws-stein (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
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. 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?). 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. Thanks, Quentin -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. Best regards, Alexander -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 26/07/2016 12:00, 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. This would mean this needs to be done in any driver *using iio_map array* to get iio channels. The other way of getting iio channels is using properties in the Device Tree so no need for late_initcall in that case. Quentin -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. Guenter -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 26/07/16 10:33, 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. > > 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?). If that did happen I'd be tempted to introduce a new function to be used outside of probe. I definitely don't like rewriting in individual drivers. The defer question is still rather open in my mind. Will address it in the other thread. > > 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. Ultimately we probably need to rethink how the registration works (it was written before deferred probing came along and that has rather changed things for us!) For now I'm not convinced that deferring is that painful even if we do have to try reprobing every time a new module gets probed.. Jonathan > > Thanks, > Quentin > -- > To unsubscribe from this list: send the line "unsubscribe linux-iio" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. To my mind late_initcall tricks are rather hideous as well. At least deferred probing should always work (with a fair cost associated with it of course) Lots of discussions ongoing on how to do dependency resolution that will hopefully mean deferred probing only occurs in the pathological cases in the future anyway. Interesting to see where that goes. Jonathan > > Guenter > -- > To unsubscribe from this list: send the line "unsubscribe linux-iio" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 15 August 2016 18:07:30 BST, Guenter Roeck <linux@roeck-us.net> wrote: >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 ? Yes. Though whether it really adds much over doing that in drivers isn't clear. Hmm. Needs more thought... > >Thanks, >Guenter >-- >To unsubscribe from this list: send the line "unsubscribe linux-iio" in >the body of a message to majordomo@vger.kernel.org >More majordomo info at http://vger.kernel.org/majordomo-info.html
On 15/08/2016 23:35, Jonathan Cameron wrote: > > > On 15 August 2016 18:07:30 BST, Guenter Roeck <linux@roeck-us.net> wrote: >> 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 ? > Yes. Though whether it really adds much over doing that in drivers isn't clear. > > Hmm. Needs more thought... Either we do the exact same "hack" as in the v2[1] in what you call iio_channel_get_all_or_defer or we duplicate the code from iio_channel_get_all in iio_channel_get_all_or_defer. Both do not seem right to me but I really dislike the late_initcall method. With this method we can only have one level of "channel dependency". This means if we ever create a new driver which depends on channels from the driver using late_initcall, we will also have to use late_initcall and we can't be sure the new driver will always be probed after the driver he depends on. [1] https://lkml.org/lkml/2016/7/15/215 Quentin >> >> Thanks, >> Guenter >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-iio" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 01/09/2016 09:15, Quentin Schulz wrote: > On 15/08/2016 23:35, Jonathan Cameron wrote: >> >> >> On 15 August 2016 18:07:30 BST, Guenter Roeck <linux@roeck-us.net> wrote: >>> 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 ? >> Yes. Though whether it really adds much over doing that in drivers isn't clear. >> >> Hmm. Needs more thought... > > Either we do the exact same "hack" as in the v2[1] in what you call > iio_channel_get_all_or_defer or we duplicate the code from > iio_channel_get_all in iio_channel_get_all_or_defer. Both do not seem > right to me but I really dislike the late_initcall method. With this > method we can only have one level of "channel dependency". > > This means if we ever create a new driver which depends on channels from > the driver using late_initcall, we will also have to use late_initcall > and we can't be sure the new driver will always be probed after the > driver he depends on. > > [1] https://lkml.org/lkml/2016/7/15/215 > > Quentin Should I revert back to the hack introduced in v2 then? >>> >>> Thanks, >>> Guenter >>> -- >>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in >>> the body of a message to majordomo@vger.kernel.org >>> More majordomo info at http://vger.kernel.org/majordomo-info.html >> -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 01/09/16 10:03, Quentin Schulz wrote: > On 01/09/2016 09:15, Quentin Schulz wrote: >> On 15/08/2016 23:35, Jonathan Cameron wrote: >>> >>> >>> On 15 August 2016 18:07:30 BST, Guenter Roeck <linux@roeck-us.net> wrote: >>>> 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 ? >>> Yes. Though whether it really adds much over doing that in drivers isn't clear. >>> >>> Hmm. Needs more thought... >> >> Either we do the exact same "hack" as in the v2[1] in what you call >> iio_channel_get_all_or_defer or we duplicate the code from >> iio_channel_get_all in iio_channel_get_all_or_defer. Both do not seem >> right to me but I really dislike the late_initcall method. With this >> method we can only have one level of "channel dependency". >> >> This means if we ever create a new driver which depends on channels from >> the driver using late_initcall, we will also have to use late_initcall >> and we can't be sure the new driver will always be probed after the >> driver he depends on. >> >> [1] https://lkml.org/lkml/2016/7/15/215 >> >> Quentin > > Should I revert back to the hack introduced in v2 then? I think so. Sorry I didn't see this until after you'd sent v4. That hack had it's disadvantages but in many ways it was a least clean. Jonathan > >>>> >>>> Thanks, >>>> Guenter >>>> -- >>>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in >>>> the body of a message to majordomo@vger.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@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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) {