| Message ID | 1440920686-6892-3-git-send-email-mpa@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sun, Aug 30, 2015 at 12:44 AM, Markus Pargmann <mpa@pengutronix.de> wrote: > The gpio name is now stored in the gpio descriptor, so we can simply use > that instead of an argument to the function. > > Signed-off-by: Markus Pargmann <mpa@pengutronix.de> Several problems with this patch: - Does not apply to v4.3-rc1 - Fixed that but noted that it just alters the call to gpiod_hog() in of_gpiochip_scan_hogs(), there is a local const char *name that should be removed too. - Looking at of_get_gpio_hog() it is unclear to me that .name is set in the gpio_desc as desired. Please check this. Crucial code looks like this: if (name && of_property_read_string(np, "line-name", name)) *name = np->name; i.e. is the line-name really propagated to gpiod->name? Are you sure you have tested this with a DTS using line-name and verified that it propagates properly? Yours, Linus Walleij
Hi, On Mon, Sep 21, 2015 at 04:28:48PM -0700, Linus Walleij wrote: > On Sun, Aug 30, 2015 at 12:44 AM, Markus Pargmann <mpa@pengutronix.de> wrote: > > > The gpio name is now stored in the gpio descriptor, so we can simply use > > that instead of an argument to the function. > > > > Signed-off-by: Markus Pargmann <mpa@pengutronix.de> > > Several problems with this patch: > > - Does not apply to v4.3-rc1 Yes, a bit older already, will rebase it. > - Fixed that but noted that it just alters the call to gpiod_hog() > in of_gpiochip_scan_hogs(), there is a local const char *name that > should be removed too. The local const char *name is temporarily used in of_gpiochip_scan_hogs() to get the name from DT and assign it to the descriptor: desc = of_parse_own_gpio(np, &name, &lflags, &dflags); ... else desc->name = name; > - Looking at of_get_gpio_hog() it is unclear to me that .name > is set in the gpio_desc as desired. Please check this. > > Crucial code looks like this: > > if (name && of_property_read_string(np, "line-name", name)) > *name = np->name; of_get_gpio_hog() only parses the properties of the DT. It does not assign any properties to the descriptor itself. So I kept it this way and added the code for assignment to of_gpiochip_scan_gpios. > > i.e. is the line-name really propagated to gpiod->name? > Are you sure you have tested this with a DTS using line-name > and verified that it propagates properly? Yes this is tested and works for me. I am using the two series together without problems. Best Regards, Markus
On Wed, Sep 23, 2015 at 11:39 PM, Markus Pargmann <mpa@pengutronix.de> wrote: >> - Fixed that but noted that it just alters the call to gpiod_hog() >> in of_gpiochip_scan_hogs(), there is a local const char *name that >> should be removed too. > > The local const char *name is temporarily used in > of_gpiochip_scan_hogs() to get the name from DT and assign it to the > descriptor: > desc = of_parse_own_gpio(np, &name, &lflags, &dflags); > ... > else > desc->name = name; OK the problem is that this series is dependent on the named GPIOs series. I want this series to stand alone, because this series is not as controversial, and I want to merge these initvals first. Yours, Linus Walleij
On Thu, Sep 24, 2015 at 10:52:55AM -0700, Linus Walleij wrote: > On Wed, Sep 23, 2015 at 11:39 PM, Markus Pargmann <mpa@pengutronix.de> wrote: > > >> - Fixed that but noted that it just alters the call to gpiod_hog() > >> in of_gpiochip_scan_hogs(), there is a local const char *name that > >> should be removed too. > > > > The local const char *name is temporarily used in > > of_gpiochip_scan_hogs() to get the name from DT and assign it to the > > descriptor: > > desc = of_parse_own_gpio(np, &name, &lflags, &dflags); > > ... > > else > > desc->name = name; > > OK the problem is that this series is dependent on the > named GPIOs series. I want this series to stand alone, > because this series is not as controversial, and I want to > merge these initvals first. OK, I think I won't get this completely independent but it shouldn't be a problem to get this before the controversial patches. Will do that. Best Regards, Markus
diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c index 48a7579dd62d..f1fe5123da28 100644 --- a/drivers/gpio/gpiolib-of.c +++ b/drivers/gpio/gpiolib-of.c @@ -232,7 +232,7 @@ static void of_gpiochip_scan_gpios(struct gpio_chip *chip) continue; } - if (gpiod_hog(desc, name, lflags, dflags)) + if (gpiod_hog(desc, lflags, dflags)) continue; } } diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 872fdd3617c1..f1559fa72c36 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -2181,15 +2181,15 @@ EXPORT_SYMBOL_GPL(__gpiod_get_index_optional); /** * gpiod_hog - Hog the specified GPIO desc given the provided flags * @desc: gpio whose value will be assigned - * @name: gpio line name * @lflags: gpio_lookup_flags - returned from of_find_gpio() or * of_get_gpio_hog() * @dflags: gpiod_flags - optional GPIO initialization flags */ -int gpiod_hog(struct gpio_desc *desc, const char *name, - unsigned long lflags, enum gpiod_flags dflags) +int gpiod_hog(struct gpio_desc *desc, unsigned long lflags, + enum gpiod_flags dflags) { int status; + const char *name = desc->name; status = __gpiod_request(desc, name); if (status) { @@ -2211,7 +2211,7 @@ int gpiod_hog(struct gpio_desc *desc, const char *name, set_bit(FLAG_IS_HOGGED, &desc->flags); pr_info("GPIO line %d (%s) hogged as %s%s\n", - desc_to_gpio(desc), name, + desc_to_gpio(desc), desc->name, (dflags&GPIOD_FLAGS_BIT_DIR_OUT) ? "output" : "input", (dflags&GPIOD_FLAGS_BIT_DIR_OUT) ? (dflags&GPIOD_FLAGS_BIT_DIR_VAL) ? "/high" : "/low":""); diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h index 78e634d1c719..6c2aeff59f86 100644 --- a/drivers/gpio/gpiolib.h +++ b/drivers/gpio/gpiolib.h @@ -97,8 +97,8 @@ struct gpio_desc { int gpiod_request(struct gpio_desc *desc, const char *label); void gpiod_free(struct gpio_desc *desc); -int gpiod_hog(struct gpio_desc *desc, const char *name, - unsigned long lflags, enum gpiod_flags dflags); +int gpiod_hog(struct gpio_desc *desc, unsigned long lflags, + enum gpiod_flags dflags); /* * Return the GPIO number of the passed descriptor relative to its chip
The gpio name is now stored in the gpio descriptor, so we can simply use that instead of an argument to the function. Signed-off-by: Markus Pargmann <mpa@pengutronix.de> --- drivers/gpio/gpiolib-of.c | 2 +- drivers/gpio/gpiolib.c | 8 ++++---- drivers/gpio/gpiolib.h | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-)