mbox series

[leds,+,devicetree,00/13] leds: tca6507 cleanup

Message ID 20200919221548.29984-1-marek.behun@nic.cz
Headers show
Series leds: tca6507 cleanup | expand

Message

Marek Behún Sept. 19, 2020, 10:15 p.m. UTC
Hi Pavel,

this is a cleanup of tca6507 LED driver.

This series applies on your for-next, but:
- the last patch should be added only after LED core parses
  `linux,default-trigger` property
- there is DT binding change and device tree change, I am not sure
  who should apply those patches

Mainly we are getting rid of platform data, but also a potential bug is
being fixed and bindings are DT being aligned.

Marek

Cc: NeilBrown <neilb@suse.de>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: H. Nikolaus Schaller <hns@goldelico.com>
Cc: Tony Lindgren <tony@atomide.com>
Cc: Adam Ford <aford173@gmail.com>
Cc: Viresh Kumar <viresh.kumar@linaro.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: devicetree@vger.kernel.org

Marek Behún (13):
  leds: tca6507: Absorb platform data
  leds: tca6507: use fwnode API instead of OF
  dt-bindings: leds: tca6507: convert to YAML
  ARM: dts: omap3: gta04: rename LED controlled subnodes
  leds: tca6507: do not set GPIO names
  leds: tca6507: cosmetic change: use helper variable
  leds: tca6507: register LEDs and GPIOs immediately after parsing
  leds: tca6507: remove binding comment
  leds: tca6507: use devres for LED and gpiochip registration
  leds: tca6507: let gpiolib set gpiochip's of_node
  leds: tca6507: fail on reg value conflict
  leds: tca6507: set registers to zero before LEDs/GPIOs registration
  leds: tca6507: use struct led_init_data when registering

 .../devicetree/bindings/leds/tca6507.txt      |  49 ----
 .../devicetree/bindings/leds/ti,tca6507.yaml  | 134 ++++++++++
 arch/arm/boot/dts/omap3-gta04.dtsi            |  12 +-
 drivers/leds/leds-tca6507.c                   | 234 ++++++------------
 include/linux/leds-tca6507.h                  |  21 --
 5 files changed, 217 insertions(+), 233 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/leds/tca6507.txt
 create mode 100644 Documentation/devicetree/bindings/leds/ti,tca6507.yaml
 delete mode 100644 include/linux/leds-tca6507.h


base-commit: a0e550dc351ab5fabe8ea86e45b974494a0a6bf8

Comments

Marek Behún Sept. 19, 2020, 10:31 p.m. UTC | #1
This patch should also remove the use of of_match_ptr, since the
definition of variable of_tca6507_leds_match isn't guarded by CONFIG_OF
anymore. (If kernel test robot enables this driver but disables
CONFIG_OF, a warning will be issued).

Pavel, should I respin this, or send another patch afterwards?

Marek
H. Nikolaus Schaller Sept. 20, 2020, 5:09 p.m. UTC | #2
> Am 20.09.2020 um 00:15 schrieb Marek Behún <marek.behun@nic.cz>:

> 

> The only in-tree usage of this driver is via device-tree. No on else

> includes linux/leds-tca6507.h, so absorb the definition of platdata

> structure.

> 

> Signed-off-by: Marek Behún <marek.behun@nic.cz>

> Cc: NeilBrown <neilb@suse.de>

> Cc: Linus Walleij <linus.walleij@linaro.org>

> Cc: H. Nikolaus Schaller <hns@goldelico.com>


Tested-by: H. Nikolaus Schaller <hns@goldelico.com>


on GTA04 (omap3-gta04.dtsi).

> ---

> drivers/leds/leds-tca6507.c  | 11 ++++++++++-

> include/linux/leds-tca6507.h | 21 ---------------------

> 2 files changed, 10 insertions(+), 22 deletions(-)

> delete mode 100644 include/linux/leds-tca6507.h

> 

> diff --git a/drivers/leds/leds-tca6507.c b/drivers/leds/leds-tca6507.c

> index a7e9fd85b6dd5..b5b5bafe2176e 100644

> --- a/drivers/leds/leds-tca6507.c

> +++ b/drivers/leds/leds-tca6507.c

> @@ -95,7 +95,6 @@

> #include <linux/i2c.h>

> #include <linux/gpio/driver.h>

> #include <linux/workqueue.h>

> -#include <linux/leds-tca6507.h>

> #include <linux/of.h>

> 

> /* LED select registers determine the source that drives LED outputs */

> @@ -108,6 +107,16 @@

> #define TCA6507_LS_BLINK0	0x6	/* Blink at Bank0 rate */

> #define TCA6507_LS_BLINK1	0x7	/* Blink at Bank1 rate */

> 

> +struct tca6507_platform_data {

> +	struct led_platform_data leds;

> +#ifdef CONFIG_GPIOLIB

> +	int gpio_base;

> +	void (*setup)(unsigned gpio_base, unsigned ngpio);

> +#endif

> +};

> +

> +#define	TCA6507_MAKE_GPIO 1

> +

> enum {

> 	BANK0,

> 	BANK1,

> diff --git a/include/linux/leds-tca6507.h b/include/linux/leds-tca6507.h

> deleted file mode 100644

> index 50d330ed11005..0000000000000

> --- a/include/linux/leds-tca6507.h

> +++ /dev/null

> @@ -1,21 +0,0 @@

> -/* SPDX-License-Identifier: GPL-2.0-only */

> -/*

> - * TCA6507 LED chip driver.

> - *

> - * Copyright (C) 2011 Neil Brown <neil@brown.name>

> - */

> -

> -#ifndef __LINUX_TCA6507_H

> -#define __LINUX_TCA6507_H

> -#include <linux/leds.h>

> -

> -struct tca6507_platform_data {

> -	struct led_platform_data leds;

> -#ifdef CONFIG_GPIOLIB

> -	int gpio_base;

> -	void (*setup)(unsigned gpio_base, unsigned ngpio);

> -#endif

> -};

> -

> -#define	TCA6507_MAKE_GPIO 1

> -#endif /* __LINUX_TCA6507_H*/

> -- 

> 2.26.2

>
Marek Behún Sept. 20, 2020, 5:35 p.m. UTC | #3
On Sun, 20 Sep 2020 19:10:19 +0200
H. Nikolaus Schaller <hns@goldelico.com> wrote:

> I wanted to test, but was not able to git am this patch to my working
> environment (v5.9-rc5 or linux-next). So maybe some prerequisite is missing.

Could you try applying on Pavel's for-next?

https://git.kernel.org/pub/scm/linux/kernel/git/pavel/linux-leds.git/log/?h=for-next

Marek
Marek Behún Sept. 20, 2020, 5:39 p.m. UTC | #4
On Sun, 20 Sep 2020 19:35:03 +0200
Marek Behun <marek.behun@nic.cz> wrote:

> On Sun, 20 Sep 2020 19:10:19 +0200

> H. Nikolaus Schaller <hns@goldelico.com> wrote:

> 

> > I wanted to test, but was not able to git am this patch to my working

> > environment (v5.9-rc5 or linux-next). So maybe some prerequisite is missing.

> 

> Could you try applying on Pavel's for-next?

> 

> https://git.kernel.org/pub/scm/linux/kernel/git/pavel/linux-leds.git/log/?h=for-next

> 

> Marek


Or even better, could you try my branch leds-cleanup-for-pavel? This is
applied on top of Pavel's for-next branch.

https://git.kernel.org/pub/scm/linux/kernel/git/kabel/linux.git/log/?h=leds-cleanup-for-pavel

Marek
Marek
H. Nikolaus Schaller Sept. 20, 2020, 6:09 p.m. UTC | #5
> Am 20.09.2020 um 19:39 schrieb Marek Behun <marek.behun@nic.cz>:
> 
> On Sun, 20 Sep 2020 19:35:03 +0200
> Marek Behun <marek.behun@nic.cz> wrote:
> 
>> On Sun, 20 Sep 2020 19:10:19 +0200
>> H. Nikolaus Schaller <hns@goldelico.com> wrote:
>> 
>>> I wanted to test, but was not able to git am this patch to my working
>>> environment (v5.9-rc5 or linux-next). So maybe some prerequisite is missing.
>> 
>> Could you try applying on Pavel's for-next?
>> 
>> https://git.kernel.org/pub/scm/linux/kernel/git/pavel/linux-leds.git/log/?h=for-next
>> 
>> Marek
> 
> Or even better, could you try my branch leds-cleanup-for-pavel? This is
> applied on top of Pavel's for-next branch.
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/kabel/linux.git/log/?h=leds-cleanup-for-pavel

Ok, seems to depend on:

leds: various: use only available OF children
leds: various: use dev_of_node(dev) instead of dev->of_node

Will try tomorrow.

BR and thanks,
Nikolaus
H. Nikolaus Schaller Sept. 22, 2020, 7:29 p.m. UTC | #6
> Am 20.09.2020 um 20:09 schrieb H. Nikolaus Schaller <hns@goldelico.com>:

> 

> 

>> Am 20.09.2020 um 19:39 schrieb Marek Behun <marek.behun@nic.cz>:

>> 

>> On Sun, 20 Sep 2020 19:35:03 +0200

>> Marek Behun <marek.behun@nic.cz> wrote:

>> 

>>> On Sun, 20 Sep 2020 19:10:19 +0200

>>> H. Nikolaus Schaller <hns@goldelico.com> wrote:

>>> 

>>>> I wanted to test, but was not able to git am this patch to my working

>>>> environment (v5.9-rc5 or linux-next). So maybe some prerequisite is missing.

>>> 

>>> Could you try applying on Pavel's for-next?

>>> 

>>> https://git.kernel.org/pub/scm/linux/kernel/git/pavel/linux-leds.git/log/?h=for-next

>>> 

>>> Marek

>> 

>> Or even better, could you try my branch leds-cleanup-for-pavel? This is

>> applied on top of Pavel's for-next branch.

>> 

>> https://git.kernel.org/pub/scm/linux/kernel/git/kabel/linux.git/log/?h=leds-cleanup-for-pavel

> 

> Ok, seems to depend on:

> 

> leds: various: use only available OF children

> leds: various: use dev_of_node(dev) instead of dev->of_node

> 

> Will try tomorrow.


Ok, I have not found a negative effect on GTA04...
Well, it only uses DT for ages.

BR,
Nikolaus
Pavel Machek Sept. 24, 2020, 12:14 p.m. UTC | #7
Hi!

> > leds: various: use only available OF children

> > leds: various: use dev_of_node(dev) instead of dev->of_node

> > 

> > Will try tomorrow.

> 

> Ok, I have not found a negative effect on GTA04...

> Well, it only uses DT for ages.


Thanks, I applied 1 and 2 of the series.

Device tree and documentation changes are not really for me (I can
take the documentation ones if I get and ack). The rest does not look bad,
but I would not mind someone testing them.

Best regards,

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Linus Walleij Sept. 29, 2020, 1:18 p.m. UTC | #8
On Sun, Sep 20, 2020 at 12:15 AM Marek Behún <marek.behun@nic.cz> wrote:

> The only in-tree usage of this driver is via device-tree. No on else
> includes linux/leds-tca6507.h, so absorb the definition of platdata
> structure.
>
> Signed-off-by: Marek Behún <marek.behun@nic.cz>
> Cc: NeilBrown <neilb@suse.de>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: H. Nikolaus Schaller <hns@goldelico.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij
Pavel Machek Sept. 30, 2020, 5:01 p.m. UTC | #9
Hi!

>  		if (fwnode_property_match_string(child, "compatible",
>  						 "gpio") >= 0) {
>  			gpio_bitmap |= BIT(reg);
>  			continue;
> +		} else {
> +			led_bitmap |= BIT(reg);
>  		}
>  

You have "continue" above, so you don't really need the else branch.

Thanks,
									Pavel