From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932779AbcE3Ise (ORCPT ); Mon, 30 May 2016 04:48:34 -0400 Received: from mailout2.w1.samsung.com ([210.118.77.12]:47727 "EHLO mailout2.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932210AbcE3Isd (ORCPT ); Mon, 30 May 2016 04:48:33 -0400 X-AuditID: cbfec7f4-f796c6d000001486-2d-574bfe5e3de2 Subject: Re: [PATCH RFT] regulator: max8972: Fix setting ramp delay To: Axel Lin , Mark Brown References: <1464520619.14023.4.camel@ingics.com> Cc: Laxman Dewangan , Mikko Perttunen , Liam Girdwood , linux-kernel@vger.kernel.org From: Krzysztof Kozlowski Message-id: <574BFE5C.5010006@samsung.com> Date: Mon, 30 May 2016 10:48:28 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.8.0 MIME-version: 1.0 In-reply-to: <1464520619.14023.4.camel@ingics.com> Content-type: text/plain; charset=utf-8 Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFrrHLMWRmVeSWpSXmKPExsVy+t/xK7px/7zDDTZNYrM4Mucrs8XUh0/Y LF6/MLRYum81i8W3Kx1MFpd3zWGzeHB1GpsDu8fOWXfZPdp+lnlsWtXJ5tHb/I7No2/LKkaP z5vkAtiiuGxSUnMyy1KL9O0SuDJ6L91gLJjJVzF72nbmBsaV3F2MnBwSAiYS/YvmsEHYYhIX 7q0Hsrk4hASWMkrMXHyYFcJ5xijx5e8qVpAqYQFnieXLljGD2CJAdteuVrBuIQFDia/ne5hA GpgFZjFKvN9wiwkkwSZgLLF5+RKwIl4BLYmr6/sYQWwWAVWJHx2zwWpEBSIkZm3/wQRRIyjx Y/I9FhCbU8BI4s7lN0D1HEBD1SWmTMkFCTMLyEtsXvOWeQIj0CKEjlkIVbOQVC1gZF7FKJpa mlxQnJSea6hXnJhbXJqXrpecn7uJERLqX3YwLj5mdYhRgINRiYe3QNM7XIg1say4MvcQowQH s5II79vHQCHelMTKqtSi/Pii0pzU4kOM0hwsSuK8c3e9DxESSE8sSc1OTS1ILYLJMnFwSjUw VjztLDa9f7v9S4jGqtCVi9fnLJb35/j8Y4ZclIln7Uq2WWUOr97mTP5e/vD49Qi76TphFpy2 RllOnu9vb+mTENjsLJ9feckvtiDu6jz/Pk6DXyx3b15weZftGS/s/IFZscV2u+02nYUfH2qd 2rX7kNdBJ95PRz/qnz3aU3Zo9/O1vLtuLut7ocRSnJFoqMVcVJwIAN2R6GJxAgAA Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 05/29/2016 01:16 PM, Axel Lin wrote: > Current code can set ramp delay to a wrong setting that the return value > from .set_voltage_time_sel is not enough for proper delay. I don't understand what yo wanted to say here. What wrong setting is possible? Why do you mention set_voltage_time_sel() here? Can you elaborate? The only difference I spotted is how you round up the ramp_delay values. Best regards, Krzysztof > Fix the logic in .set_ramp_delay and also remove unused ret_val variable. > > Signed-off-by: Axel Lin > --- > drivers/regulator/max8973-regulator.c | 16 ++++++---------- > 1 file changed, 6 insertions(+), 10 deletions(-) > > diff --git a/drivers/regulator/max8973-regulator.c b/drivers/regulator/max8973-regulator.c > index 08d2f13..3958f50 100644 > --- a/drivers/regulator/max8973-regulator.c > +++ b/drivers/regulator/max8973-regulator.c > @@ -271,22 +271,18 @@ static int max8973_set_ramp_delay(struct regulator_dev *rdev, > struct max8973_chip *max = rdev_get_drvdata(rdev); > unsigned int control; > int ret; > - int ret_val; > > /* Set ramp delay */ > - if (ramp_delay < 25000) { > + if (ramp_delay <= 12000) > control = MAX8973_RAMP_12mV_PER_US; > - ret_val = 12000; > - } else if (ramp_delay < 50000) { > + else if (ramp_delay <= 25000) > control = MAX8973_RAMP_25mV_PER_US; > - ret_val = 25000; > - } else if (ramp_delay < 200000) { > + else if (ramp_delay <= 50000) > control = MAX8973_RAMP_50mV_PER_US; > - ret_val = 50000; > - } else { > + else if (ramp_delay <= 200000) > control = MAX8973_RAMP_200mV_PER_US; > - ret_val = 200000; > - } > + else > + return -EINVAL; > > ret = regmap_update_bits(max->regmap, MAX8973_CONTROL1, > MAX8973_RAMP_MASK, control); >