From: Doug Berger <opendmb@gmail.com> To: Gregory Fong <gregory.0xf0@gmail.com> Cc: Linus Walleij <linus.walleij@linaro.org>, Bartosz Golaszewski <bgolaszewski@baylibre.com>, Florian Fainelli <f.fainelli@gmail.com>, bcm-kernel-feedback-list <bcm-kernel-feedback-list@broadcom.com>, linux-gpio@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] gpio: brcmstb: support gpio-line-names property Date: Mon, 9 Mar 2020 10:42:08 -0700 [thread overview] Message-ID: <810d8c3d-482f-078c-70b5-c4123c2679b8@gmail.com> (raw) In-Reply-To: <CADtm3G4eq_p1YZtQrtcx4LGPiY82BpgXf55YtJCW+WJSrM7siw@mail.gmail.com> On 3/9/20 12:45 AM, Gregory Fong wrote: > Hi Doug, > > On Fri, Mar 6, 2020 at 4:14 PM Doug Berger <opendmb@gmail.com> wrote: >> >> The default handling of the gpio-line-names property by the >> gpiolib-of implementation does not work with the multiple >> gpiochip banks per device structure used by the gpio-brcmstb >> driver. >> >> This commit adds driver level support for the device tree >> property so that GPIO lines can be assigned friendly names. >> >> Signed-off-by: Doug Berger <opendmb@gmail.com> > > I've added a few comments below. With the suggested updates: > Acked-by: Gregory Fong <gregory.0xf0@gmail.com> > >> --- >> drivers/gpio/gpio-brcmstb.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 44 insertions(+) >> >> diff --git a/drivers/gpio/gpio-brcmstb.c b/drivers/gpio/gpio-brcmstb.c >> index 05e3f99ae59c..e9ab246e2d42 100644 >> --- a/drivers/gpio/gpio-brcmstb.c >> +++ b/drivers/gpio/gpio-brcmstb.c >> @@ -603,6 +603,49 @@ static const struct dev_pm_ops brcmstb_gpio_pm_ops = { >> .resume_noirq = brcmstb_gpio_resume, >> }; >> >> +static void brcmstb_gpio_set_names(struct device *dev, >> + struct brcmstb_gpio_bank *bank) >> +{ >> + struct device_node *np = dev->of_node; >> + const char **names; >> + int nstrings, base; >> + unsigned int i; >> + >> + base = bank->id * MAX_GPIO_PER_BANK; >> + >> + nstrings = of_property_count_strings(np, "gpio-line-names"); >> + if (nstrings <= base) >> + /* Line names not present */ >> + return; >> + >> + names = devm_kcalloc(dev, MAX_GPIO_PER_BANK, sizeof(char *), > > Please use sizeof(*names) instead of sizeof(char *). Good idea. Will do. > >> + GFP_KERNEL); >> + if (!names) >> + return; >> + >> + /* >> + * Make sure to not index beyond the end of the number of descriptors >> + * of the GPIO device. >> + */ >> + for (i = 0; i < bank->width; i++) { >> + const char *name; >> + int ret; >> + >> + ret = of_property_read_string_index(np, "gpio-line-names", >> + base + i, &name); >> + if (ret) { >> + if (ret != -ENODATA) >> + dev_err(dev, "unable to name line %d: %d\n", >> + i, ret); > > Recommend adding the GPIO bank ID to this error message. I'll use the absolute line number rather than the bank relative line number to reduce confusion, but since it's different from your suggestion I'll leave off your Ack in case you really want the bank number for some reason. > > Best regards, > Gregory > Thanks Gregory! V2 coming soon.
WARNING: multiple messages have this Message-ID (diff)
From: Doug Berger <opendmb@gmail.com> To: Gregory Fong <gregory.0xf0@gmail.com> Cc: Florian Fainelli <f.fainelli@gmail.com>, linux-gpio@vger.kernel.org, Linus Walleij <linus.walleij@linaro.org>, linux-kernel@vger.kernel.org, Bartosz Golaszewski <bgolaszewski@baylibre.com>, bcm-kernel-feedback-list <bcm-kernel-feedback-list@broadcom.com>, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH] gpio: brcmstb: support gpio-line-names property Date: Mon, 9 Mar 2020 10:42:08 -0700 [thread overview] Message-ID: <810d8c3d-482f-078c-70b5-c4123c2679b8@gmail.com> (raw) In-Reply-To: <CADtm3G4eq_p1YZtQrtcx4LGPiY82BpgXf55YtJCW+WJSrM7siw@mail.gmail.com> On 3/9/20 12:45 AM, Gregory Fong wrote: > Hi Doug, > > On Fri, Mar 6, 2020 at 4:14 PM Doug Berger <opendmb@gmail.com> wrote: >> >> The default handling of the gpio-line-names property by the >> gpiolib-of implementation does not work with the multiple >> gpiochip banks per device structure used by the gpio-brcmstb >> driver. >> >> This commit adds driver level support for the device tree >> property so that GPIO lines can be assigned friendly names. >> >> Signed-off-by: Doug Berger <opendmb@gmail.com> > > I've added a few comments below. With the suggested updates: > Acked-by: Gregory Fong <gregory.0xf0@gmail.com> > >> --- >> drivers/gpio/gpio-brcmstb.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 44 insertions(+) >> >> diff --git a/drivers/gpio/gpio-brcmstb.c b/drivers/gpio/gpio-brcmstb.c >> index 05e3f99ae59c..e9ab246e2d42 100644 >> --- a/drivers/gpio/gpio-brcmstb.c >> +++ b/drivers/gpio/gpio-brcmstb.c >> @@ -603,6 +603,49 @@ static const struct dev_pm_ops brcmstb_gpio_pm_ops = { >> .resume_noirq = brcmstb_gpio_resume, >> }; >> >> +static void brcmstb_gpio_set_names(struct device *dev, >> + struct brcmstb_gpio_bank *bank) >> +{ >> + struct device_node *np = dev->of_node; >> + const char **names; >> + int nstrings, base; >> + unsigned int i; >> + >> + base = bank->id * MAX_GPIO_PER_BANK; >> + >> + nstrings = of_property_count_strings(np, "gpio-line-names"); >> + if (nstrings <= base) >> + /* Line names not present */ >> + return; >> + >> + names = devm_kcalloc(dev, MAX_GPIO_PER_BANK, sizeof(char *), > > Please use sizeof(*names) instead of sizeof(char *). Good idea. Will do. > >> + GFP_KERNEL); >> + if (!names) >> + return; >> + >> + /* >> + * Make sure to not index beyond the end of the number of descriptors >> + * of the GPIO device. >> + */ >> + for (i = 0; i < bank->width; i++) { >> + const char *name; >> + int ret; >> + >> + ret = of_property_read_string_index(np, "gpio-line-names", >> + base + i, &name); >> + if (ret) { >> + if (ret != -ENODATA) >> + dev_err(dev, "unable to name line %d: %d\n", >> + i, ret); > > Recommend adding the GPIO bank ID to this error message. I'll use the absolute line number rather than the bank relative line number to reduce confusion, but since it's different from your suggestion I'll leave off your Ack in case you really want the bank number for some reason. > > Best regards, > Gregory > Thanks Gregory! V2 coming soon. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2020-03-09 17:42 UTC|newest] Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-03-07 0:13 [PATCH] gpio: brcmstb: support gpio-line-names property Doug Berger 2020-03-07 0:13 ` Doug Berger 2020-03-09 7:45 ` Gregory Fong 2020-03-09 7:45 ` Gregory Fong 2020-03-09 17:42 ` Doug Berger [this message] 2020-03-09 17:42 ` Doug Berger
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=810d8c3d-482f-078c-70b5-c4123c2679b8@gmail.com \ --to=opendmb@gmail.com \ --cc=bcm-kernel-feedback-list@broadcom.com \ --cc=bgolaszewski@baylibre.com \ --cc=f.fainelli@gmail.com \ --cc=gregory.0xf0@gmail.com \ --cc=linus.walleij@linaro.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-gpio@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.