Messages in this thread Patch in this message | | | Subject | Re: [greybus-dev] [PATCH] staging: greybus: Use gpio_is_valid() | From | Arvind Yadav <> | Date | Fri, 27 Apr 2018 18:20:37 +0530 |
| |
On Friday 27 April 2018 05:47 PM, Alex Elder wrote: > On 04/27/2018 05:52 AM, Arvind Yadav wrote: >> Replace the manual validity checks for the GPIO with the >> gpio_is_valid(). > I haven't looked through the code paths very closely, but I > think that get_named_gpio() might return -EPROBE_DEFER, which > would be something we want to pass to the caller. Yes of_get_name_gpio() can return other error value apart from -EPROBE_DEFER. > So rather than returning -ENODEV and hiding the reason the > call to of_get_named_gpio() failed, you should continue > returning the errno it supplies (if not a valid gpio number). > > -Alex I have return -ENODEV because invalid gpio pin can be positive. static inline bool gpio_is_valid(int number) { return number >= 0 && number < ARCH_NR_GPIOS; } Here if number > ARCH_NR_GPIOS then it's invalid but return value will be positive.
We can return like this " return (gpio > 0) ? -ENODEV: gpio;"
But not sure this is worth to handle this.
~arvind > >> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com> >> --- >> drivers/staging/greybus/arche-platform.c | 12 ++++++------ >> 1 file changed, 6 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/staging/greybus/arche-platform.c b/drivers/staging/greybus/arche-platform.c >> index 83254a7..fc6bf60 100644 >> --- a/drivers/staging/greybus/arche-platform.c >> +++ b/drivers/staging/greybus/arche-platform.c >> @@ -448,9 +448,9 @@ static int arche_platform_probe(struct platform_device *pdev) >> arche_pdata->svc_reset_gpio = of_get_named_gpio(np, >> "svc,reset-gpio", >> 0); >> - if (arche_pdata->svc_reset_gpio < 0) { >> + if (!gpio_is_valid(arche_pdata->svc_reset_gpio)) { >> dev_err(dev, "failed to get reset-gpio\n"); >> - return arche_pdata->svc_reset_gpio; >> + return -ENODEV; >> } >> ret = devm_gpio_request(dev, arche_pdata->svc_reset_gpio, "svc-reset"); >> if (ret) { >> @@ -468,9 +468,9 @@ static int arche_platform_probe(struct platform_device *pdev) >> arche_pdata->svc_sysboot_gpio = of_get_named_gpio(np, >> "svc,sysboot-gpio", >> 0); >> - if (arche_pdata->svc_sysboot_gpio < 0) { >> + if (!gpio_is_valid(arche_pdata->svc_sysboot_gpio)) { >> dev_err(dev, "failed to get sysboot gpio\n"); >> - return arche_pdata->svc_sysboot_gpio; >> + return -ENODEV; >> } >> ret = devm_gpio_request(dev, arche_pdata->svc_sysboot_gpio, "sysboot0"); >> if (ret) { >> @@ -487,9 +487,9 @@ static int arche_platform_probe(struct platform_device *pdev) >> arche_pdata->svc_refclk_req = of_get_named_gpio(np, >> "svc,refclk-req-gpio", >> 0); >> - if (arche_pdata->svc_refclk_req < 0) { >> + if (!gpio_is_valid(arche_pdata->svc_refclk_req)) { >> dev_err(dev, "failed to get svc clock-req gpio\n"); >> - return arche_pdata->svc_refclk_req; >> + return -ENODEV; >> } >> ret = devm_gpio_request(dev, arche_pdata->svc_refclk_req, >> "svc-clk-req"); >>
| |