Messages in this thread | | | From | "Zhang, Sonic" <> | Date | Tue, 3 Jul 2012 04:33:19 -0400 | Subject | RE: [PATCH v2] regulator: ad5398: Fix min/max current limit boundary checking |
| |
>-----Original Message----- >From: Lars-Peter Clausen [mailto:lars@metafoo.de] >Sent: Tuesday, July 03, 2012 4:24 PM >To: Zhang, Sonic >Cc: Axel Lin; Mark Brown; linux-kernel@vger.kernel.org; Liam Girdwood >Subject: Re: [PATCH v2] regulator: ad5398: Fix min/max current limit boundary >checking > >On 07/03/2012 10:13 AM, Zhang, Sonic wrote: >> >> >>> -----Original Message----- >>> From: Lars-Peter Clausen [mailto:lars@metafoo.de] >>> Sent: Tuesday, July 03, 2012 4:06 PM >>> To: Zhang, Sonic >>> Cc: Axel Lin; Mark Brown; linux-kernel@vger.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@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. >> >> If the requested minimum current is smaller than the capability of the hardware, >does a bigger min value fulfill this request? > >As long as it is smaller than the maximum requested current, yes. You >request a current range with the regulator API and any value within this >range is fine as the actual output current. >
If so, please also set the max_uA as well.
Sonic
>> >> If this logic is correct, I am fine to ACK. >> >> >> Regards, >> >> Sonic >> >> >>> >>> - 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 >>>>> >>>>> >>>>> >>>> >>> >> >
| |