Re: [PATCH v2] regulator: ad5398: Fix min/max current limit boundary checking

[Lars-Peter Clausen <lars@metafoo.de>]

On 07/03/2012 09:54 AM, Zhang, Sonic wrote:
> 
> 
>> -----Original Message-----
>> From: Axel Lin [mailto:axel.lin@gmail.com]
>> Sent: Tuesday, July 03, 2012 3:43 PM
>> To: Mark Brown
>> Cc: linux-kernel@vger.kernel.org; Zhang, Sonic; Lars-Peter Clausen; Liam
>> Girdwood
>> Subject: [PATCH v2] regulator: ad5398: Fix min/max current limit boundary
>> checking
>>
>> It is ok to request current limit with min_uA < chip->min_uA and
>> max_uA > chip->max_uA.
>>
>> We need to set min_uA = chip->min_uA if (min_uA < chip->min_uA),
>> this ensures the equation to calcuate selator does not return negative number.
>>
> 
> You should not do it in driver. Set a correct min_uA value in your application.

I think the patch makes sense. If a application request a current range
which overlaps with the range support by the chip, but either the requested
min is smaller than the supported min or the requested max is larger than
the supported max the driver will fail with an error. E.g.

req-min     req-max
   |-----------|
        |------------|
     chip-min  chip-max

or even

req-min                req-max
   |----------------------|
        |------------|
     chip-min  chip-max


While it is obviously possible for the chip to fulfill this request.
Axel's patch takes care of this situation and ensures that the request is
satisfied and the output current is set to a current within the requested
range and the supported range.

- Lars

> 
> Regards,
> 
> Sonic
> 
>> Signed-off-by: Axel Lin <axel.lin@gmail.com>
>> ~
>> ---
>> drivers/regulator/ad5398.c |    7 ++++---
>> 1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/regulator/ad5398.c b/drivers/regulator/ad5398.c
>> index 46d05f3..84fdcda 100644
>> --- a/drivers/regulator/ad5398.c
>> +++ b/drivers/regulator/ad5398.c
>> @@ -89,9 +89,10 @@ static int ad5398_set_current_limit(struct regulator_dev
>> *rdev, int min_uA, int
>>       unsigned short data;
>>       int ret;
>>
>> -      if (min_uA > chip->max_uA || min_uA < chip->min_uA)
>> -              return -EINVAL;
>> -      if (max_uA > chip->max_uA || max_uA < chip->min_uA)
>> +      if (min_uA < chip->min_uA)
>> +              min_uA = chip->min_uA;
>> +
>> +      if (min_uA > chip->max_uA || max_uA < chip->min_uA)
>>               return -EINVAL;
>>
>>       selector = DIV_ROUND_UP((min_uA - chip->min_uA) * chip->current_level,
>> --
>> 1.7.9.5
>>
>>
>>
>