Re: [PATCH v3 1/3] iio: adc: Cosmic Circuits 10001 ADC driver

[Ezequiel Garcia <ezequiel.garcia@imgtec.com>]

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