* [PATCH 1/2] leds: ns2: Absorb platform data @ 2020-01-07 14:10 Linus Walleij 2020-01-07 14:10 ` [PATCH 2/2] leds: ns2: Convert to GPIO descriptors Linus Walleij 0 siblings, 1 reply; 7+ messages in thread From: Linus Walleij @ 2020-01-07 14:10 UTC (permalink / raw) To: Jacek Anaszewski, Pavel Machek, Dan Murphy Cc: linux-leds, Linus Walleij, Simon Guinot, Vincent Donnefort Nothing in the kernel includes the external header <linux/platform_data/leds-kirkwood-ns2.h> so just push the contents into the ns2 leds driver. If someone wants to use platform data or board files to describe this device they should be able to do so using GPIO machine descriptors but in any case device tree should be the way forward for these systems in all cases I can think of, and the driver already supports that. Cc: Simon Guinot <simon.guinot@sequanux.org> Cc: Vincent Donnefort <vdonnefort@gmail.com> Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- drivers/leds/leds-ns2.c | 30 +++++++++++++-- .../linux/platform_data/leds-kirkwood-ns2.h | 38 ------------------- 2 files changed, 27 insertions(+), 41 deletions(-) delete mode 100644 include/linux/platform_data/leds-kirkwood-ns2.h diff --git a/drivers/leds/leds-ns2.c b/drivers/leds/leds-ns2.c index 7c500dfdcfa3..6d37dda12c39 100644 --- a/drivers/leds/leds-ns2.c +++ b/drivers/leds/leds-ns2.c @@ -12,14 +12,38 @@ #include <linux/kernel.h> #include <linux/platform_device.h> #include <linux/slab.h> -#include <linux/gpio.h> +#include <linux/gpio/consumer.h> #include <linux/leds.h> #include <linux/module.h> -#include <linux/platform_data/leds-kirkwood-ns2.h> #include <linux/of.h> -#include <linux/of_gpio.h> #include "leds.h" +enum ns2_led_modes { + NS_V2_LED_OFF, + NS_V2_LED_ON, + NS_V2_LED_SATA, +}; + +struct ns2_led_modval { + enum ns2_led_modes mode; + int cmd_level; + int slow_level; +}; + +struct ns2_led { + const char *name; + const char *default_trigger; + unsigned cmd; + unsigned slow; + int num_modes; + struct ns2_led_modval *modval; +}; + +struct ns2_led_platform_data { + int num_leds; + struct ns2_led *leds; +}; + /* * The Network Space v2 dual-GPIO LED is wired to a CPLD. Three different LED * modes are available: off, on and SATA activity blinking. The LED modes are diff --git a/include/linux/platform_data/leds-kirkwood-ns2.h b/include/linux/platform_data/leds-kirkwood-ns2.h deleted file mode 100644 index eb8a6860e816..000000000000 --- a/include/linux/platform_data/leds-kirkwood-ns2.h +++ /dev/null @@ -1,38 +0,0 @@ -/* - * Platform data structure for Network Space v2 LED driver - * - * This file is licensed under the terms of the GNU General Public - * License version 2. This program is licensed "as is" without any - * warranty of any kind, whether express or implied. - */ - -#ifndef __LEDS_KIRKWOOD_NS2_H -#define __LEDS_KIRKWOOD_NS2_H - -enum ns2_led_modes { - NS_V2_LED_OFF, - NS_V2_LED_ON, - NS_V2_LED_SATA, -}; - -struct ns2_led_modval { - enum ns2_led_modes mode; - int cmd_level; - int slow_level; -}; - -struct ns2_led { - const char *name; - const char *default_trigger; - unsigned cmd; - unsigned slow; - int num_modes; - struct ns2_led_modval *modval; -}; - -struct ns2_led_platform_data { - int num_leds; - struct ns2_led *leds; -}; - -#endif /* __LEDS_KIRKWOOD_NS2_H */ -- 2.23.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] leds: ns2: Convert to GPIO descriptors 2020-01-07 14:10 [PATCH 1/2] leds: ns2: Absorb platform data Linus Walleij @ 2020-01-07 14:10 ` Linus Walleij 2020-01-07 14:17 ` Pavel Machek 2020-01-12 13:56 ` Simon Guinot 0 siblings, 2 replies; 7+ messages in thread From: Linus Walleij @ 2020-01-07 14:10 UTC (permalink / raw) To: Jacek Anaszewski, Pavel Machek, Dan Murphy Cc: linux-leds, Linus Walleij, Simon Guinot, Vincent Donnefort This converts the NS2 LED driver to use GPIO descriptors. We take care to request the GPIOs "as is" which is what the current driver goes to lengths to achieve, then we use GPIOs throughout. As the nodes for each LED does not have any corresponding device, we need to use the DT-specific accessors to get these GPIO descriptors from the device tree. Cc: Simon Guinot <simon.guinot@sequanux.org> Cc: Vincent Donnefort <vdonnefort@gmail.com> Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- drivers/leds/leds-ns2.c | 73 +++++++++++++++++------------------------ 1 file changed, 31 insertions(+), 42 deletions(-) diff --git a/drivers/leds/leds-ns2.c b/drivers/leds/leds-ns2.c index 6d37dda12c39..538ca5755602 100644 --- a/drivers/leds/leds-ns2.c +++ b/drivers/leds/leds-ns2.c @@ -33,8 +33,8 @@ struct ns2_led_modval { struct ns2_led { const char *name; const char *default_trigger; - unsigned cmd; - unsigned slow; + struct gpio_desc *cmd; + struct gpio_desc *slow; int num_modes; struct ns2_led_modval *modval; }; @@ -53,8 +53,8 @@ struct ns2_led_platform_data { struct ns2_led_data { struct led_classdev cdev; - unsigned int cmd; - unsigned int slow; + struct gpio_desc *cmd; + struct gpio_desc *slow; bool can_sleep; unsigned char sata; /* True when SATA mode active. */ rwlock_t rw_lock; /* Lock GPIOs. */ @@ -70,8 +70,8 @@ static int ns2_led_get_mode(struct ns2_led_data *led_dat, int cmd_level; int slow_level; - cmd_level = gpio_get_value_cansleep(led_dat->cmd); - slow_level = gpio_get_value_cansleep(led_dat->slow); + cmd_level = gpiod_get_value_cansleep(led_dat->cmd); + slow_level = gpiod_get_value_cansleep(led_dat->slow); for (i = 0; i < led_dat->num_modes; i++) { if (cmd_level == led_dat->modval[i].cmd_level && @@ -104,15 +104,15 @@ static void ns2_led_set_mode(struct ns2_led_data *led_dat, write_lock_irqsave(&led_dat->rw_lock, flags); if (!led_dat->can_sleep) { - gpio_set_value(led_dat->cmd, - led_dat->modval[i].cmd_level); - gpio_set_value(led_dat->slow, - led_dat->modval[i].slow_level); + gpiod_set_value(led_dat->cmd, + led_dat->modval[i].cmd_level); + gpiod_set_value(led_dat->slow, + led_dat->modval[i].slow_level); goto exit_unlock; } - gpio_set_value_cansleep(led_dat->cmd, led_dat->modval[i].cmd_level); - gpio_set_value_cansleep(led_dat->slow, led_dat->modval[i].slow_level); + gpiod_set_value_cansleep(led_dat->cmd, led_dat->modval[i].cmd_level); + gpiod_set_value_cansleep(led_dat->slow, led_dat->modval[i].slow_level); exit_unlock: write_unlock_irqrestore(&led_dat->rw_lock, flags); @@ -200,26 +200,6 @@ create_ns2_led(struct platform_device *pdev, struct ns2_led_data *led_dat, int ret; enum ns2_led_modes mode; - ret = devm_gpio_request_one(&pdev->dev, template->cmd, - gpio_get_value_cansleep(template->cmd) ? - GPIOF_OUT_INIT_HIGH : GPIOF_OUT_INIT_LOW, - template->name); - if (ret) { - dev_err(&pdev->dev, "%s: failed to setup command GPIO\n", - template->name); - return ret; - } - - ret = devm_gpio_request_one(&pdev->dev, template->slow, - gpio_get_value_cansleep(template->slow) ? - GPIOF_OUT_INIT_HIGH : GPIOF_OUT_INIT_LOW, - template->name); - if (ret) { - dev_err(&pdev->dev, "%s: failed to setup slow GPIO\n", - template->name); - return ret; - } - rwlock_init(&led_dat->rw_lock); led_dat->cdev.name = template->name; @@ -229,8 +209,8 @@ create_ns2_led(struct platform_device *pdev, struct ns2_led_data *led_dat, led_dat->cdev.groups = ns2_led_groups; led_dat->cmd = template->cmd; led_dat->slow = template->slow; - led_dat->can_sleep = gpio_cansleep(led_dat->cmd) | - gpio_cansleep(led_dat->slow); + led_dat->can_sleep = gpiod_cansleep(led_dat->cmd) | + gpiod_cansleep(led_dat->slow); if (led_dat->can_sleep) led_dat->cdev.brightness_set_blocking = ns2_led_set_blocking; else @@ -285,17 +265,26 @@ ns2_leds_get_of_pdata(struct device *dev, struct ns2_led_platform_data *pdata) const char *string; int i, num_modes; struct ns2_led_modval *modval; + struct gpio_desc *gd; - ret = of_get_named_gpio(child, "cmd-gpio", 0); - if (ret < 0) - goto err_node_put; - led->cmd = ret; - ret = of_get_named_gpio(child, "slow-gpio", 0); - if (ret < 0) - goto err_node_put; - led->slow = ret; ret = of_property_read_string(child, "label", &string); led->name = (ret == 0) ? string : child->name; + + gd = gpiod_get_from_of_node(child, "cmd-gpio", 0, + GPIOD_ASIS, led->name); + if (IS_ERR(gd)) { + ret = PTR_ERR(gd); + goto err_node_put; + } + led->cmd = gd; + gd = gpiod_get_from_of_node(child, "slow-gpio", 0, + GPIOD_ASIS, led->name); + if (IS_ERR(gd)) { + ret = PTR_ERR(gd); + goto err_node_put; + } + led->slow = gd; + ret = of_property_read_string(child, "linux,default-trigger", &string); if (ret == 0) -- 2.23.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] leds: ns2: Convert to GPIO descriptors 2020-01-07 14:10 ` [PATCH 2/2] leds: ns2: Convert to GPIO descriptors Linus Walleij @ 2020-01-07 14:17 ` Pavel Machek 2020-01-07 14:55 ` Simon Guinot 2020-01-07 21:12 ` Linus Walleij 2020-01-12 13:56 ` Simon Guinot 1 sibling, 2 replies; 7+ messages in thread From: Pavel Machek @ 2020-01-07 14:17 UTC (permalink / raw) To: Linus Walleij Cc: Jacek Anaszewski, Dan Murphy, linux-leds, Simon Guinot, Vincent Donnefort [-- Attachment #1: Type: text/plain, Size: 636 bytes --] Hi! > This converts the NS2 LED driver to use GPIO descriptors. > We take care to request the GPIOs "as is" which is what > the current driver goes to lengths to achieve, then we use > GPIOs throughout. > > As the nodes for each LED does not have any corresponding > device, we need to use the DT-specific accessors to get these > GPIO descriptors from the device tree. Ok. But what is motivation for doing this? Was this tested? Does anyone still use leds-ns2? Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 195 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] leds: ns2: Convert to GPIO descriptors 2020-01-07 14:17 ` Pavel Machek @ 2020-01-07 14:55 ` Simon Guinot 2020-01-07 21:12 ` Linus Walleij 1 sibling, 0 replies; 7+ messages in thread From: Simon Guinot @ 2020-01-07 14:55 UTC (permalink / raw) To: Pavel Machek Cc: Linus Walleij, Jacek Anaszewski, Dan Murphy, linux-leds, Vincent Donnefort [-- Attachment #1: Type: text/plain, Size: 830 bytes --] On Tue, Jan 07, 2020 at 03:17:42PM +0100, Pavel Machek wrote: > Hi! > > > This converts the NS2 LED driver to use GPIO descriptors. > > We take care to request the GPIOs "as is" which is what > > the current driver goes to lengths to achieve, then we use > > GPIOs throughout. > > > > As the nodes for each LED does not have any corresponding > > device, we need to use the DT-specific accessors to get these > > GPIO descriptors from the device tree. > > Ok. But what is motivation for doing this? Was this tested? > > Does anyone still use leds-ns2? Hi, Yes the leds-ns2 driver is used by The LaCie Network Space v2 family of NAS and by the Seagate NAS 2 and 4-Bay. The latter is a bit old but not obsolete. AFAIK all this NASes are supported by Debian. I'll do the testing this week-end. Simon [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] leds: ns2: Convert to GPIO descriptors 2020-01-07 14:17 ` Pavel Machek 2020-01-07 14:55 ` Simon Guinot @ 2020-01-07 21:12 ` Linus Walleij 1 sibling, 0 replies; 7+ messages in thread From: Linus Walleij @ 2020-01-07 21:12 UTC (permalink / raw) To: Pavel Machek Cc: Jacek Anaszewski, Dan Murphy, Linux LED Subsystem, Simon Guinot, Vincent Donnefort On Tue, Jan 7, 2020 at 3:17 PM Pavel Machek <pavel@ucw.cz> wrote: > Ok. But what is motivation for doing this? Zip from drivers/gpio/TODO: Starting with commit 79a9becda894 the GPIO subsystem embarked on a journey to move away from the global GPIO numberspace and toward a decriptor-based approach. This means that GPIO consumers, drivers and machine descriptions ideally have no use or idea of the global GPIO numberspace that has/was used in the inception of the GPIO subsystem. The motivation for that in turn is that the GPIO number space has become unmanageable and is also unpredictable due to factors such as probe ordering and the introduction of -EPROBE_DEFER. The number space issue is the same as to why irq is moving away from irq numbers to IRQ descriptors. > Was this tested? No, I change a lot of code I can't test, I rely on volunteers to test it, it seems Simon volunteered. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] leds: ns2: Convert to GPIO descriptors 2020-01-07 14:10 ` [PATCH 2/2] leds: ns2: Convert to GPIO descriptors Linus Walleij 2020-01-07 14:17 ` Pavel Machek @ 2020-01-12 13:56 ` Simon Guinot 2020-02-26 13:30 ` Pavel Machek 1 sibling, 1 reply; 7+ messages in thread From: Simon Guinot @ 2020-01-12 13:56 UTC (permalink / raw) To: Linus Walleij, Pavel Machek Cc: Jacek Anaszewski, Dan Murphy, linux-leds, Vincent Donnefort [-- Attachment #1: Type: text/plain, Size: 5583 bytes --] On Tue, Jan 07, 2020 at 03:10:29PM +0100, Linus Walleij wrote: > This converts the NS2 LED driver to use GPIO descriptors. > We take care to request the GPIOs "as is" which is what > the current driver goes to lengths to achieve, then we use > GPIOs throughout. > > As the nodes for each LED does not have any corresponding > device, we need to use the DT-specific accessors to get these > GPIO descriptors from the device tree. > > Cc: Simon Guinot <simon.guinot@sequanux.org> > Cc: Vincent Donnefort <vdonnefort@gmail.com> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> Hi Linus and Pavel, I have tested this patches on a LaCie d2 Network v2 board and the LEDs are still working as expected. Tested-by: Simon Guinot <simon.guinot@sequanux.org> > --- > drivers/leds/leds-ns2.c | 73 +++++++++++++++++------------------------ > 1 file changed, 31 insertions(+), 42 deletions(-) > > diff --git a/drivers/leds/leds-ns2.c b/drivers/leds/leds-ns2.c > index 6d37dda12c39..538ca5755602 100644 > --- a/drivers/leds/leds-ns2.c > +++ b/drivers/leds/leds-ns2.c > @@ -33,8 +33,8 @@ struct ns2_led_modval { > struct ns2_led { > const char *name; > const char *default_trigger; > - unsigned cmd; > - unsigned slow; > + struct gpio_desc *cmd; > + struct gpio_desc *slow; > int num_modes; > struct ns2_led_modval *modval; > }; > @@ -53,8 +53,8 @@ struct ns2_led_platform_data { > > struct ns2_led_data { > struct led_classdev cdev; > - unsigned int cmd; > - unsigned int slow; > + struct gpio_desc *cmd; > + struct gpio_desc *slow; > bool can_sleep; > unsigned char sata; /* True when SATA mode active. */ > rwlock_t rw_lock; /* Lock GPIOs. */ > @@ -70,8 +70,8 @@ static int ns2_led_get_mode(struct ns2_led_data *led_dat, > int cmd_level; > int slow_level; > > - cmd_level = gpio_get_value_cansleep(led_dat->cmd); > - slow_level = gpio_get_value_cansleep(led_dat->slow); > + cmd_level = gpiod_get_value_cansleep(led_dat->cmd); > + slow_level = gpiod_get_value_cansleep(led_dat->slow); > > for (i = 0; i < led_dat->num_modes; i++) { > if (cmd_level == led_dat->modval[i].cmd_level && > @@ -104,15 +104,15 @@ static void ns2_led_set_mode(struct ns2_led_data *led_dat, > write_lock_irqsave(&led_dat->rw_lock, flags); > > if (!led_dat->can_sleep) { > - gpio_set_value(led_dat->cmd, > - led_dat->modval[i].cmd_level); > - gpio_set_value(led_dat->slow, > - led_dat->modval[i].slow_level); > + gpiod_set_value(led_dat->cmd, > + led_dat->modval[i].cmd_level); > + gpiod_set_value(led_dat->slow, > + led_dat->modval[i].slow_level); > goto exit_unlock; > } > > - gpio_set_value_cansleep(led_dat->cmd, led_dat->modval[i].cmd_level); > - gpio_set_value_cansleep(led_dat->slow, led_dat->modval[i].slow_level); > + gpiod_set_value_cansleep(led_dat->cmd, led_dat->modval[i].cmd_level); > + gpiod_set_value_cansleep(led_dat->slow, led_dat->modval[i].slow_level); > > exit_unlock: > write_unlock_irqrestore(&led_dat->rw_lock, flags); > @@ -200,26 +200,6 @@ create_ns2_led(struct platform_device *pdev, struct ns2_led_data *led_dat, > int ret; > enum ns2_led_modes mode; > > - ret = devm_gpio_request_one(&pdev->dev, template->cmd, > - gpio_get_value_cansleep(template->cmd) ? > - GPIOF_OUT_INIT_HIGH : GPIOF_OUT_INIT_LOW, > - template->name); > - if (ret) { > - dev_err(&pdev->dev, "%s: failed to setup command GPIO\n", > - template->name); > - return ret; > - } > - > - ret = devm_gpio_request_one(&pdev->dev, template->slow, > - gpio_get_value_cansleep(template->slow) ? > - GPIOF_OUT_INIT_HIGH : GPIOF_OUT_INIT_LOW, > - template->name); > - if (ret) { > - dev_err(&pdev->dev, "%s: failed to setup slow GPIO\n", > - template->name); > - return ret; > - } > - > rwlock_init(&led_dat->rw_lock); > > led_dat->cdev.name = template->name; > @@ -229,8 +209,8 @@ create_ns2_led(struct platform_device *pdev, struct ns2_led_data *led_dat, > led_dat->cdev.groups = ns2_led_groups; > led_dat->cmd = template->cmd; > led_dat->slow = template->slow; > - led_dat->can_sleep = gpio_cansleep(led_dat->cmd) | > - gpio_cansleep(led_dat->slow); > + led_dat->can_sleep = gpiod_cansleep(led_dat->cmd) | > + gpiod_cansleep(led_dat->slow); > if (led_dat->can_sleep) > led_dat->cdev.brightness_set_blocking = ns2_led_set_blocking; > else > @@ -285,17 +265,26 @@ ns2_leds_get_of_pdata(struct device *dev, struct ns2_led_platform_data *pdata) > const char *string; > int i, num_modes; > struct ns2_led_modval *modval; > + struct gpio_desc *gd; > > - ret = of_get_named_gpio(child, "cmd-gpio", 0); > - if (ret < 0) > - goto err_node_put; > - led->cmd = ret; > - ret = of_get_named_gpio(child, "slow-gpio", 0); > - if (ret < 0) > - goto err_node_put; > - led->slow = ret; > ret = of_property_read_string(child, "label", &string); > led->name = (ret == 0) ? string : child->name; > + > + gd = gpiod_get_from_of_node(child, "cmd-gpio", 0, > + GPIOD_ASIS, led->name); > + if (IS_ERR(gd)) { > + ret = PTR_ERR(gd); > + goto err_node_put; > + } > + led->cmd = gd; > + gd = gpiod_get_from_of_node(child, "slow-gpio", 0, > + GPIOD_ASIS, led->name); > + if (IS_ERR(gd)) { > + ret = PTR_ERR(gd); > + goto err_node_put; > + } > + led->slow = gd; > + > ret = of_property_read_string(child, "linux,default-trigger", > &string); > if (ret == 0) > -- > 2.23.0 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] leds: ns2: Convert to GPIO descriptors 2020-01-12 13:56 ` Simon Guinot @ 2020-02-26 13:30 ` Pavel Machek 0 siblings, 0 replies; 7+ messages in thread From: Pavel Machek @ 2020-02-26 13:30 UTC (permalink / raw) To: Simon Guinot Cc: Linus Walleij, Jacek Anaszewski, Dan Murphy, linux-leds, Vincent Donnefort [-- Attachment #1: Type: text/plain, Size: 1067 bytes --] On Sun 2020-01-12 14:56:30, Simon Guinot wrote: > On Tue, Jan 07, 2020 at 03:10:29PM +0100, Linus Walleij wrote: > > This converts the NS2 LED driver to use GPIO descriptors. > > We take care to request the GPIOs "as is" which is what > > the current driver goes to lengths to achieve, then we use > > GPIOs throughout. > > > > As the nodes for each LED does not have any corresponding > > device, we need to use the DT-specific accessors to get these > > GPIO descriptors from the device tree. > > > > Cc: Simon Guinot <simon.guinot@sequanux.org> > > Cc: Vincent Donnefort <vdonnefort@gmail.com> > > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > > Hi Linus and Pavel, > > I have tested this patches on a LaCie d2 Network v2 board and the LEDs > are still working as expected. > > Tested-by: Simon Guinot <simon.guinot@sequanux.org> Thanks, applied to my for-next branch. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 195 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-02-26 13:30 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-01-07 14:10 [PATCH 1/2] leds: ns2: Absorb platform data Linus Walleij 2020-01-07 14:10 ` [PATCH 2/2] leds: ns2: Convert to GPIO descriptors Linus Walleij 2020-01-07 14:17 ` Pavel Machek 2020-01-07 14:55 ` Simon Guinot 2020-01-07 21:12 ` Linus Walleij 2020-01-12 13:56 ` Simon Guinot 2020-02-26 13:30 ` Pavel Machek
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).