From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755338AbcFQGaX (ORCPT ); Fri, 17 Jun 2016 02:30:23 -0400 Received: from mailout4.w1.samsung.com ([210.118.77.14]:54610 "EHLO mailout4.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752665AbcFQGaU (ORCPT ); Fri, 17 Jun 2016 02:30:20 -0400 X-AuditID: cbfec7f5-f792a6d000001302-3c-576398f9f106 Subject: Re: [PATCH v3 4/7] max8903: adds requesting of gpios. To: Chris Lapa , dwmw2@infradead.org, dbaryshkov@gmail.com, sre@kernel.org, mark.rutland@arm.com, robh+dt@kernel.org References: <1464849897-21527-3-git-send-email-chris@lapa.com.au> <1466139626-51434-1-git-send-email-chris@lapa.com.au> <1466139626-51434-5-git-send-email-chris@lapa.com.au> Cc: devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org From: Krzysztof Kozlowski X-Enigmail-Draft-Status: N1110 Message-id: <576398F7.4050704@samsung.com> Date: Fri, 17 Jun 2016 08:30:15 +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: <1466139626-51434-5-git-send-email-chris@lapa.com.au> Content-type: text/plain; charset=windows-1252 Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFvrDLMWRmVeSWpSXmKPExsVy+t/xK7o/ZySHGyw/Lmfx8IyZxaQn75kt 5h85x2oxceVkZovXLwwtLu+aw2bxufcIo8XS6xeZLFr3HmG3OL27xIHLY828NYweO2fdZffY vELLY9OqTjaPZXM3M3r0bVnF6PF5k1wAexSXTUpqTmZZapG+XQJXRu+aMywFZ5Qq1nzbz97A uEqyi5GTQ0LARGLvlBXMELaYxIV769m6GLk4hASWMkrMf72OEcJ5xihxZFkzUBUHh7CAncS2 1XUgcRGBTkaJ7y+3QxVtZJR4M+kNK8goZoEIic4Hh9hBbDYBY4nNy5ewQayQk+jtnsQCYvMK aEks7TgBVsMioCqx8XkbWFwUqHfW9h9MEDWCEj8m3wOLcwo4Sex/fIUF5AhmAT2J+xe1IFbJ S2xe85Z5AqPgLCQdsxCqZiGpWsDIvIpRNLU0uaA4KT3XSK84Mbe4NC9dLzk/dxMjJE6+7mBc eszqEKMAB6MSD+8K0eRwIdbEsuLK3EOMEhzMSiK8mtOBQrwpiZVVqUX58UWlOanFhxilOViU xHln7nofIiSQnliSmp2aWpBaBJNl4uCUamCMWMj8+uHJ1hMPHvrLTpDxLp9x2PNqwEaZcAX1 5ks2BjGLv//mk2T9GHbmmr1N8HG9U6wrZMV4t35P7rzJ1Na3U+yI6JMm65otX6N5u2fOY7t2 9ZSBJVN5Evunjoiw0/YeEotaFn1Vzgv5/uaIctS/mOn/1R+H/w5zTive33Uj9NdHjaVrUvmU WIozEg21mIuKEwE9jXCjjwIAAA== Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 06/17/2016 07:00 AM, Chris Lapa wrote: > From: Chris Lapa > > This change ensures all gpios are available for the driver to use. > > Signed-off-by: Chris Lapa > --- > drivers/power/max8903_charger.c | 79 ++++++++++++++++++++++++++++++++++++----- > 1 file changed, 70 insertions(+), 9 deletions(-) > > diff --git a/drivers/power/max8903_charger.c b/drivers/power/max8903_charger.c > index dbd911c4..c068efe 100644 > --- a/drivers/power/max8903_charger.c > +++ b/drivers/power/max8903_charger.c > @@ -212,6 +212,16 @@ static int max8903_probe(struct platform_device *pdev) > > if (pdata->dc_valid) { > if (pdata->dok && gpio_is_valid(pdata->dok)) { > + ret = devm_gpio_request(dev, > + pdata->dok, No need to split these two arguments. They fit in 80 chars. > + data->psy_desc.name); > + if (ret) { > + dev_err(dev, > + "Failed GPIO request for dok: %d err %d\n", > + pdata->dok, ret); Indentation here is wrong. These should be aligned to first argument (dev). > + return -EINVAL; Return 'ret'. All these three comments apply also to code below. Anyway the probe function is getting bigger and more difficult to read. Consider factoring out some code to a separate function. For example all the GPIO handling stuff could be factored. Best regards, Krzysztof > + } > + > gpio = pdata->dok; /* PULL_UPed Interrupt */ > ta_in = gpio_get_value(gpio) ? 0 : 1; > } else { > @@ -223,6 +233,16 @@ static int max8903_probe(struct platform_device *pdev) > > if (pdata->dcm) { > if (gpio_is_valid(pdata->dcm)) { > + ret = devm_gpio_request(dev, > + pdata->dcm, > + data->psy_desc.name); > + if (ret) { > + dev_err(dev, > + "Failed GPIO request for dcm: %d err %d\n", > + pdata->dcm, ret); > + return -EINVAL; > + } > + > gpio = pdata->dcm; /* Output */ > gpio_set_value(gpio, ta_in); > } else { > @@ -233,6 +253,16 @@ static int max8903_probe(struct platform_device *pdev) > > if (pdata->usb_valid) { > if (pdata->uok && gpio_is_valid(pdata->uok)) { > + ret = devm_gpio_request(dev, > + pdata->uok, > + data->psy_desc.name); > + if (ret) { > + dev_err(dev, > + "Failed GPIO request for uok: %d err %d\n", > + pdata->uok, ret); > + return -EINVAL; > + } > + > gpio = pdata->uok; > usb_in = gpio_get_value(gpio) ? 0 : 1; > } else { > @@ -244,6 +274,16 @@ static int max8903_probe(struct platform_device *pdev) > > if (pdata->cen) { > if (gpio_is_valid(pdata->cen)) { > + ret = devm_gpio_request(dev, > + pdata->cen, > + data->psy_desc.name); > + if (ret) { > + dev_err(dev, > + "Failed GPIO request for cen: %d err %d\n", > + pdata->cen, ret); > + return -EINVAL; > + } > + > gpio_set_value(pdata->cen, (ta_in || usb_in) ? 0 : 1); > } else { > dev_err(dev, "Invalid pin: cen.\n"); > @@ -252,23 +292,44 @@ static int max8903_probe(struct platform_device *pdev) > } > > if (pdata->chg) { > - if (!gpio_is_valid(pdata->chg)) { > - dev_err(dev, "Invalid pin: chg.\n"); > - return -EINVAL; > + if (gpio_is_valid(pdata->chg)) { > + ret = devm_gpio_request(dev, > + pdata->chg, > + data->psy_desc.name); > + if (ret) { > + dev_err(dev, > + "Failed GPIO request for chg: %d err %d\n", > + pdata->chg, ret); > + return -EINVAL; > + } > } > } > > if (pdata->flt) { > - if (!gpio_is_valid(pdata->flt)) { > - dev_err(dev, "Invalid pin: flt.\n"); > - return -EINVAL; > + if (gpio_is_valid(pdata->flt)) { > + ret = devm_gpio_request(dev, > + pdata->flt, > + data->psy_desc.name); > + if (ret) { > + dev_err(dev, > + "Failed GPIO request for flt: %d err %d\n", > + pdata->flt, ret); > + return -EINVAL; > + } > } > } > > if (pdata->usus) { > - if (!gpio_is_valid(pdata->usus)) { > - dev_err(dev, "Invalid pin: usus.\n"); > - return -EINVAL; > + if (gpio_is_valid(pdata->usus)) { > + ret = devm_gpio_request(dev, > + pdata->usus, > + data->psy_desc.name); > + if (ret) { > + dev_err(dev, > + "Failed GPIO request for usus: %d err %d\n", > + pdata->usus, ret); > + return -EINVAL; > + } > } > } > >