Re: [PATCH v2 2/4] ACPI / gpio: Add support for naming GPIOs
On Thu, Oct 20, 2016 at 02:06:42PM +0200, Linus Walleij wrote: > On Thu, Sep 29, 2016 at 3:39 PM, Mika Westerberg >wrote: > > > DT has property 'gpio-line-names' to name GPIO lines the controller has if > > present. Use this very same property in ACPI as well to provide nice names > > for the GPIOS. > > > > Signed-off-by: Mika Westerberg > > Oh wait: > > > +static void acpi_gpiochip_set_names(struct acpi_gpio_chip *achip) > > +{ > > + struct gpio_chip *chip = achip->chip; > > + struct gpio_device *gdev = chip->gpiodev; > > + const char **names; > > + int ret, i; > > + > > + ret = device_property_read_string_array(chip->parent, > > "gpio-line-names", > > + NULL, 0); > > + if (ret < 0) > > + return; > > + > > + if (ret != gdev->ngpio) { > > + dev_warn(chip->parent, > > +"names %d do not match number of GPIOs %d\n", ret, > > +gdev->ngpio); > > + return; > > + } > > + > > + names = kcalloc(gdev->ngpio, sizeof(*names), GFP_KERNEL); > > + if (!names) > > + return; > > + > > + ret = device_property_read_string_array(chip->parent, > > "gpio-line-names", > > + names, gdev->ngpio); > > + if (ret < 0) { > > + dev_warn(chip->parent, "Failed to read GPIO line names\n"); > > + return; > > + } > > + > > + /* > > +* It is fine to assign the name, it will be allocated as long as > > +* the ACPI device exists. > > +*/ > > + for (i = 0; i < gdev->ngpio; i++) > > + gdev->descs[i].name = names[i]; > > + > > + kfree(names); > > +} > > Wouldn't this entire function work just as fine on device tree? Now that you mentioned it, yes, I think it should work with DT as well. > So should this snippet using device_property_* be moved into > a file like gpiolib-devprop.c+gpiolib.h signature and get used from > both gpiolib-of.c and gpiolib-acpi.c, replacing the DT-specific > code in gpiolib-of.c? Works for me :)
Re: [PATCH v2 2/4] ACPI / gpio: Add support for naming GPIOs
On Thu, Oct 20, 2016 at 02:06:42PM +0200, Linus Walleij wrote: > On Thu, Sep 29, 2016 at 3:39 PM, Mika Westerberg > wrote: > > > DT has property 'gpio-line-names' to name GPIO lines the controller has if > > present. Use this very same property in ACPI as well to provide nice names > > for the GPIOS. > > > > Signed-off-by: Mika Westerberg > > Oh wait: > > > +static void acpi_gpiochip_set_names(struct acpi_gpio_chip *achip) > > +{ > > + struct gpio_chip *chip = achip->chip; > > + struct gpio_device *gdev = chip->gpiodev; > > + const char **names; > > + int ret, i; > > + > > + ret = device_property_read_string_array(chip->parent, > > "gpio-line-names", > > + NULL, 0); > > + if (ret < 0) > > + return; > > + > > + if (ret != gdev->ngpio) { > > + dev_warn(chip->parent, > > +"names %d do not match number of GPIOs %d\n", ret, > > +gdev->ngpio); > > + return; > > + } > > + > > + names = kcalloc(gdev->ngpio, sizeof(*names), GFP_KERNEL); > > + if (!names) > > + return; > > + > > + ret = device_property_read_string_array(chip->parent, > > "gpio-line-names", > > + names, gdev->ngpio); > > + if (ret < 0) { > > + dev_warn(chip->parent, "Failed to read GPIO line names\n"); > > + return; > > + } > > + > > + /* > > +* It is fine to assign the name, it will be allocated as long as > > +* the ACPI device exists. > > +*/ > > + for (i = 0; i < gdev->ngpio; i++) > > + gdev->descs[i].name = names[i]; > > + > > + kfree(names); > > +} > > Wouldn't this entire function work just as fine on device tree? Now that you mentioned it, yes, I think it should work with DT as well. > So should this snippet using device_property_* be moved into > a file like gpiolib-devprop.c+gpiolib.h signature and get used from > both gpiolib-of.c and gpiolib-acpi.c, replacing the DT-specific > code in gpiolib-of.c? Works for me :)
Re: [PATCH v2 2/4] ACPI / gpio: Add support for naming GPIOs
On Thu, Oct 20, 2016 at 02:02:34PM +0200, Linus Walleij wrote: > On Thu, Sep 29, 2016 at 3:39 PM, Mika Westerberg >wrote: > > > DT has property 'gpio-line-names' to name GPIO lines the controller has if > > present. Use this very same property in ACPI as well to provide nice names > > for the GPIOS. > > > > Signed-off-by: Mika Westerberg > > Looks OK to me, is this patch independent so I can merge it into the > GPIO tree or does it need to go in with patch 1/4 to the ACPI tree? This (2/4) and the 3/4 are independent of patch 1/4 so they can go via any tree.
Re: [PATCH v2 2/4] ACPI / gpio: Add support for naming GPIOs
On Thu, Oct 20, 2016 at 02:02:34PM +0200, Linus Walleij wrote: > On Thu, Sep 29, 2016 at 3:39 PM, Mika Westerberg > wrote: > > > DT has property 'gpio-line-names' to name GPIO lines the controller has if > > present. Use this very same property in ACPI as well to provide nice names > > for the GPIOS. > > > > Signed-off-by: Mika Westerberg > > Looks OK to me, is this patch independent so I can merge it into the > GPIO tree or does it need to go in with patch 1/4 to the ACPI tree? This (2/4) and the 3/4 are independent of patch 1/4 so they can go via any tree.
Re: [PATCH v2 2/4] ACPI / gpio: Add support for naming GPIOs
On Thu, Sep 29, 2016 at 3:39 PM, Mika Westerbergwrote: > DT has property 'gpio-line-names' to name GPIO lines the controller has if > present. Use this very same property in ACPI as well to provide nice names > for the GPIOS. > > Signed-off-by: Mika Westerberg Oh wait: > +static void acpi_gpiochip_set_names(struct acpi_gpio_chip *achip) > +{ > + struct gpio_chip *chip = achip->chip; > + struct gpio_device *gdev = chip->gpiodev; > + const char **names; > + int ret, i; > + > + ret = device_property_read_string_array(chip->parent, > "gpio-line-names", > + NULL, 0); > + if (ret < 0) > + return; > + > + if (ret != gdev->ngpio) { > + dev_warn(chip->parent, > +"names %d do not match number of GPIOs %d\n", ret, > +gdev->ngpio); > + return; > + } > + > + names = kcalloc(gdev->ngpio, sizeof(*names), GFP_KERNEL); > + if (!names) > + return; > + > + ret = device_property_read_string_array(chip->parent, > "gpio-line-names", > + names, gdev->ngpio); > + if (ret < 0) { > + dev_warn(chip->parent, "Failed to read GPIO line names\n"); > + return; > + } > + > + /* > +* It is fine to assign the name, it will be allocated as long as > +* the ACPI device exists. > +*/ > + for (i = 0; i < gdev->ngpio; i++) > + gdev->descs[i].name = names[i]; > + > + kfree(names); > +} Wouldn't this entire function work just as fine on device tree? So should this snippet using device_property_* be moved into a file like gpiolib-devprop.c+gpiolib.h signature and get used from both gpiolib-of.c and gpiolib-acpi.c, replacing the DT-specific code in gpiolib-of.c? Yours, Linus Walleij
Re: [PATCH v2 2/4] ACPI / gpio: Add support for naming GPIOs
On Thu, Sep 29, 2016 at 3:39 PM, Mika Westerberg wrote: > DT has property 'gpio-line-names' to name GPIO lines the controller has if > present. Use this very same property in ACPI as well to provide nice names > for the GPIOS. > > Signed-off-by: Mika Westerberg Oh wait: > +static void acpi_gpiochip_set_names(struct acpi_gpio_chip *achip) > +{ > + struct gpio_chip *chip = achip->chip; > + struct gpio_device *gdev = chip->gpiodev; > + const char **names; > + int ret, i; > + > + ret = device_property_read_string_array(chip->parent, > "gpio-line-names", > + NULL, 0); > + if (ret < 0) > + return; > + > + if (ret != gdev->ngpio) { > + dev_warn(chip->parent, > +"names %d do not match number of GPIOs %d\n", ret, > +gdev->ngpio); > + return; > + } > + > + names = kcalloc(gdev->ngpio, sizeof(*names), GFP_KERNEL); > + if (!names) > + return; > + > + ret = device_property_read_string_array(chip->parent, > "gpio-line-names", > + names, gdev->ngpio); > + if (ret < 0) { > + dev_warn(chip->parent, "Failed to read GPIO line names\n"); > + return; > + } > + > + /* > +* It is fine to assign the name, it will be allocated as long as > +* the ACPI device exists. > +*/ > + for (i = 0; i < gdev->ngpio; i++) > + gdev->descs[i].name = names[i]; > + > + kfree(names); > +} Wouldn't this entire function work just as fine on device tree? So should this snippet using device_property_* be moved into a file like gpiolib-devprop.c+gpiolib.h signature and get used from both gpiolib-of.c and gpiolib-acpi.c, replacing the DT-specific code in gpiolib-of.c? Yours, Linus Walleij
Re: [PATCH v2 2/4] ACPI / gpio: Add support for naming GPIOs
On Thu, Sep 29, 2016 at 3:39 PM, Mika Westerbergwrote: > DT has property 'gpio-line-names' to name GPIO lines the controller has if > present. Use this very same property in ACPI as well to provide nice names > for the GPIOS. > > Signed-off-by: Mika Westerberg Looks OK to me, is this patch independent so I can merge it into the GPIO tree or does it need to go in with patch 1/4 to the ACPI tree? Yours, Linus Walleij
Re: [PATCH v2 2/4] ACPI / gpio: Add support for naming GPIOs
On Thu, Sep 29, 2016 at 3:39 PM, Mika Westerberg wrote: > DT has property 'gpio-line-names' to name GPIO lines the controller has if > present. Use this very same property in ACPI as well to provide nice names > for the GPIOS. > > Signed-off-by: Mika Westerberg Looks OK to me, is this patch independent so I can merge it into the GPIO tree or does it need to go in with patch 1/4 to the ACPI tree? Yours, Linus Walleij
Re: [PATCH v2 2/4] ACPI / gpio: Add support for naming GPIOs
On Sun, Oct 9, 2016 at 6:01 PM, Mika Westerbergwrote: > On Fri, Oct 07, 2016 at 08:05:14PM +0300, Andy Shevchenko wrote: >> On Thu, Sep 29, 2016 at 4:39 PM, Mika Westerberg >> wrote: >> > + if (!chip->names) >> > + acpi_gpiochip_set_names(acpi_gpio); >> > + >> >> I'm okay with this, though wouldn't be better to call it >> unconditionally like it's done for below call and move check inside? > > DT does it like this. I can move the check inside the function as well. Up to you. If it even worth to change. -- With Best Regards, Andy Shevchenko
Re: [PATCH v2 2/4] ACPI / gpio: Add support for naming GPIOs
On Sun, Oct 9, 2016 at 6:01 PM, Mika Westerberg wrote: > On Fri, Oct 07, 2016 at 08:05:14PM +0300, Andy Shevchenko wrote: >> On Thu, Sep 29, 2016 at 4:39 PM, Mika Westerberg >> wrote: >> > + if (!chip->names) >> > + acpi_gpiochip_set_names(acpi_gpio); >> > + >> >> I'm okay with this, though wouldn't be better to call it >> unconditionally like it's done for below call and move check inside? > > DT does it like this. I can move the check inside the function as well. Up to you. If it even worth to change. -- With Best Regards, Andy Shevchenko
Re: [PATCH v2 2/4] ACPI / gpio: Add support for naming GPIOs
On Fri, Oct 07, 2016 at 08:05:14PM +0300, Andy Shevchenko wrote: > On Thu, Sep 29, 2016 at 4:39 PM, Mika Westerberg >wrote: > > DT has property 'gpio-line-names' to name GPIO lines the controller has if > > present. Use this very same property in ACPI as well to provide nice names > > for the GPIOS. > > One nit below. > > > @@ -835,6 +875,9 @@ void acpi_gpiochip_add(struct gpio_chip *chip) > > return; > > } > > > > + if (!chip->names) > > + acpi_gpiochip_set_names(acpi_gpio); > > + > > I'm okay with this, though wouldn't be better to call it > unconditionally like it's done for below call and move check inside? DT does it like this. I can move the check inside the function as well. > > > acpi_gpiochip_request_regions(acpi_gpio); > > acpi_walk_dep_device_list(handle); > > } > > -- > With Best Regards, > Andy Shevchenko
Re: [PATCH v2 2/4] ACPI / gpio: Add support for naming GPIOs
On Fri, Oct 07, 2016 at 08:05:14PM +0300, Andy Shevchenko wrote: > On Thu, Sep 29, 2016 at 4:39 PM, Mika Westerberg > wrote: > > DT has property 'gpio-line-names' to name GPIO lines the controller has if > > present. Use this very same property in ACPI as well to provide nice names > > for the GPIOS. > > One nit below. > > > @@ -835,6 +875,9 @@ void acpi_gpiochip_add(struct gpio_chip *chip) > > return; > > } > > > > + if (!chip->names) > > + acpi_gpiochip_set_names(acpi_gpio); > > + > > I'm okay with this, though wouldn't be better to call it > unconditionally like it's done for below call and move check inside? DT does it like this. I can move the check inside the function as well. > > > acpi_gpiochip_request_regions(acpi_gpio); > > acpi_walk_dep_device_list(handle); > > } > > -- > With Best Regards, > Andy Shevchenko
Re: [PATCH v2 2/4] ACPI / gpio: Add support for naming GPIOs
On Thu, Sep 29, 2016 at 4:39 PM, Mika Westerbergwrote: > DT has property 'gpio-line-names' to name GPIO lines the controller has if > present. Use this very same property in ACPI as well to provide nice names > for the GPIOS. One nit below. > @@ -835,6 +875,9 @@ void acpi_gpiochip_add(struct gpio_chip *chip) > return; > } > > + if (!chip->names) > + acpi_gpiochip_set_names(acpi_gpio); > + I'm okay with this, though wouldn't be better to call it unconditionally like it's done for below call and move check inside? > acpi_gpiochip_request_regions(acpi_gpio); > acpi_walk_dep_device_list(handle); > } -- With Best Regards, Andy Shevchenko
Re: [PATCH v2 2/4] ACPI / gpio: Add support for naming GPIOs
On Thu, Sep 29, 2016 at 4:39 PM, Mika Westerberg wrote: > DT has property 'gpio-line-names' to name GPIO lines the controller has if > present. Use this very same property in ACPI as well to provide nice names > for the GPIOS. One nit below. > @@ -835,6 +875,9 @@ void acpi_gpiochip_add(struct gpio_chip *chip) > return; > } > > + if (!chip->names) > + acpi_gpiochip_set_names(acpi_gpio); > + I'm okay with this, though wouldn't be better to call it unconditionally like it's done for below call and move check inside? > acpi_gpiochip_request_regions(acpi_gpio); > acpi_walk_dep_device_list(handle); > } -- With Best Regards, Andy Shevchenko