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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4FF2A7F9.3090307@metafoo.de>
Date:	Tue, 03 Jul 2012 10:06:17 +0200
From:	Lars-Peter Clausen <lars@...afoo.de>
To:	"Zhang, Sonic" <Sonic.Zhang@...log.com>
CC:	Axel Lin <axel.lin@...il.com>,
	Mark Brown <broonie@...nsource.wolfsonmicro.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Liam Girdwood <lrg@...com>
Subject: Re: [PATCH v2] regulator: ad5398: Fix min/max current limit boundary
 checking

On 07/03/2012 09:54 AM, Zhang, Sonic wrote:
> 
> 
>> -----Original Message-----
>> From: Axel Lin [mailto:axel.lin@...il.com]
>> Sent: Tuesday, July 03, 2012 3:43 PM
>> To: Mark Brown
>> Cc: linux-kernel@...r.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@...il.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
>>
>>
>>
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/