From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mailapp01.imgtec.com ([195.59.15.196]:50841 "EHLO mailapp01.imgtec.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750781AbaK0PUV (ORCPT ); Thu, 27 Nov 2014 10:20:21 -0500 Message-ID: <547740C8.6020604@imgtec.com> Date: Thu, 27 Nov 2014 12:18:32 -0300 From: Ezequiel Garcia MIME-Version: 1.0 To: Lars-Peter Clausen , Hartmut Knaack , Andrew Bresticker , James Hartley , Jonathan Cameron CC: , , , , , , , , Phani Movva Subject: Re: [PATCH v3 1/3] iio: adc: Cosmic Circuits 10001 ADC driver References: <1415888044-16635-1-git-send-email-ezequiel.garcia@imgtec.com> <1415888044-16635-2-git-send-email-ezequiel.garcia@imgtec.com> <546FE3A7.6040308@gmx.de> <5474C062.2020604@imgtec.com> <5474F7A5.6030008@gmx.de> <5474FCC8.8080906@imgtec.com> <5475085E.8090806@gmx.de> <54773E63.4020300@imgtec.com> <5477406A.8020403@metafoo.de> In-Reply-To: <5477406A.8020403@metafoo.de> Content-Type: text/plain; charset="iso-8859-15" Sender: linux-iio-owner@vger.kernel.org List-Id: linux-iio@vger.kernel.org On 11/27/2014 12:16 PM, Lars-Peter Clausen wrote: > [...] >>>> >>>>>>>> + >>>>>>>> + adc_dev->reg = devm_regulator_get(&pdev->dev, "vref"); >>>>>>>> + if (IS_ERR_OR_NULL(adc_dev->reg)) >>>>>>>> + return -EINVAL; >>>>>>> if (IS_ERR(adc_dev->reg)) >>>>>>> return PTR_ERR(adc_dev->reg); >>>>>> >>>>>> Are you sure? What if devm_regulator_get() returns NULL? >>>>>> >>>>> I had a look at it, but couldn't figure out a condition in which it >>>>> would return NULL. But I'd be happy to be informed of anything >>>>> different. >>>> >>>> If CONFIG_REGULATOR=n you get NULL there (although I added a select >>>> REGULATOR >>>> on the v4 I just posted). >>>> >>> Indeed, thanks. Now I'm thinking if it would make sense to handle >>> this special case in a special way (as it would be caused by an >>> invalid kernel config). Well, I guess, I leave it up to you. >> >> Well, adding the select REGULATOR that shouldn't happen, so let's better >> drop the NULL check entirely. > > Doing a 'select REGULATOR' can (and probably will sooner or later) > create nasty dependency loops. User selectable config items should not > be selected by another config item. If the driver really has a hard > dependency on the regulator framework then 'depends on REGULATOR' is the > right thing to do. > Fine, then. 'depends on REGULATOR' it is. -- Ezequiel From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ezequiel Garcia Subject: Re: [PATCH v3 1/3] iio: adc: Cosmic Circuits 10001 ADC driver Date: Thu, 27 Nov 2014 12:18:32 -0300 Message-ID: <547740C8.6020604@imgtec.com> References: <1415888044-16635-1-git-send-email-ezequiel.garcia@imgtec.com> <1415888044-16635-2-git-send-email-ezequiel.garcia@imgtec.com> <546FE3A7.6040308@gmx.de> <5474C062.2020604@imgtec.com> <5474F7A5.6030008@gmx.de> <5474FCC8.8080906@imgtec.com> <5475085E.8090806@gmx.de> <54773E63.4020300@imgtec.com> <5477406A.8020403@metafoo.de> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-15" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <5477406A.8020403-Qo5EllUWu/uELgA04lAiVw@public.gmane.org> Sender: linux-iio-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Lars-Peter Clausen , Hartmut Knaack , Andrew Bresticker , James Hartley , Jonathan Cameron Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, Pawel.Moll-5wv7dgnIgG8@public.gmane.org, Mark.Rutland-5wv7dgnIgG8@public.gmane.org, ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org, galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org, Naidu.Tellapati-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org, Phani Movva List-Id: devicetree@vger.kernel.org On 11/27/2014 12:16 PM, Lars-Peter Clausen wrote: > [...] >>>> >>>>>>>> + >>>>>>>> + adc_dev->reg = devm_regulator_get(&pdev->dev, "vref"); >>>>>>>> + if (IS_ERR_OR_NULL(adc_dev->reg)) >>>>>>>> + return -EINVAL; >>>>>>> if (IS_ERR(adc_dev->reg)) >>>>>>> return PTR_ERR(adc_dev->reg); >>>>>> >>>>>> Are you sure? What if devm_regulator_get() returns NULL? >>>>>> >>>>> I had a look at it, but couldn't figure out a condition in which it >>>>> would return NULL. But I'd be happy to be informed of anything >>>>> different. >>>> >>>> If CONFIG_REGULATOR=n you get NULL there (although I added a select >>>> REGULATOR >>>> on the v4 I just posted). >>>> >>> Indeed, thanks. Now I'm thinking if it would make sense to handle >>> this special case in a special way (as it would be caused by an >>> invalid kernel config). Well, I guess, I leave it up to you. >> >> Well, adding the select REGULATOR that shouldn't happen, so let's better >> drop the NULL check entirely. > > Doing a 'select REGULATOR' can (and probably will sooner or later) > create nasty dependency loops. User selectable config items should not > be selected by another config item. If the driver really has a hard > dependency on the regulator framework then 'depends on REGULATOR' is the > right thing to do. > Fine, then. 'depends on REGULATOR' it is. -- Ezequiel