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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux