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: <DB904C5425BA6F4E8424B3B51A1414D17140805DAB@NWD2CMBX1.ad.analog.com>
Date:	Tue, 3 Jul 2012 04:13:44 -0400
From:	"Zhang, Sonic" <Sonic.Zhang@...log.com>
To:	Lars-Peter Clausen <lars@...afoo.de>
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



>-----Original Message-----
>From: Lars-Peter Clausen [mailto:lars@...afoo.de]
>Sent: Tuesday, July 03, 2012 4:06 PM
>To: Zhang, Sonic
>Cc: Axel Lin; Mark Brown; linux-kernel@...r.kernel.org; Liam Girdwood
>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.

If the requested minimum current is smaller than the capability of the hardware, does a bigger min value fulfill this request?

If this logic is correct, I am fine to ACK.


Regards,

Sonic


>
>- 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
>>>
>>>
>>>
>>
>