mbox series

[v6,0/7] leds: pwm: Make automatic labels work

Message ID 20200930234637.7573-1-post@lespocky.de
Headers show
Series leds: pwm: Make automatic labels work | expand

Message

Alexander Dahl Sept. 30, 2020, 11:46 p.m. UTC
Hei hei,

for leds-gpio you can use the properties 'function' and 'color' in the
devicetree node and omit 'label', the label is constructed
automatically.  This is a common feature supposed to be working for all
LED drivers.  However it did not yet work for the 'leds-pwm' driver.

This series removes platform_data support for the leds-pwm driver and
takes the opportunity to update the leds-pwm dt-bindings accordingly.

After myself being one week on vacation patch 2/3 was already picked by
Pavel and I gathered some more feedback on the remaining issues.

v6 was compile tested and dt_bindings_check and dtbs_check were run.

Note: I added some patches to fix DT schema warnings, but I did not put
every reviewer/supporter/maintainer printed by get_maintainers in Cc to
keep that list reasonable small.

Series changelog below …

Greets
Alex

v6:
- rebased series on recent pavel/for-next
- added Reviewed-by from Marek to patch 1
- patch 2 from v5 was picked by Pavel and is already in his for-next
  branch
- previous patch 3/3 (now 2/7) was reworked based on feedback by Rob
- added more dt patches fixing warnings after binding conversion to yaml

v5:
- replaced patch 1/3 by a new patch removing platform_data support for
  the leds-pwm driver
- little rewording of commit message in patch 2/3
- updated patch 3/3 based on feedback by Rob Herring
- added Marek Behún to Cc, because he also works on removing
  platform_data support
- rebased series on pavel/for-next

v4:
- added led-class patch handling fwnode passing differently (patch 1/3)
- adapted leds-pwm patch to new led-class (patch 2/3)
- contacted original author of leds-pwm dt binding on license issue
  (patch 3/3)

v3:
- series rebased on v5.9-rc4
- changed license of .yaml file to recommended one (patch 2/2)
- added Acked-by to both patches

v2:
- series rebased on v5.9-rc3
- added the dt-bindings update patch (2/2)

v1:
- based on v5.9-rc2
- backport on v5.4.59 tested and working

Alexander Dahl (7):
  leds: pwm: Remove platform_data support
  dt-bindings: leds: Convert pwm to yaml
  dt-bindings: mfd: Fix schema warnings for pwm-leds
  ARM: dts: at91: smartkiz: Reference led node directly
  ARM: dts: Fix schema warnings for pwm-leds
  arm64: dts: meson: Fix schema warnings for pwm-leds
  MIPS: DTS: img: Fix schema warnings for pwm-leds

 .../devicetree/bindings/leds/leds-pwm.txt     | 50 -------------
 .../devicetree/bindings/leds/leds-pwm.yaml    | 70 +++++++++++++++++++
 .../devicetree/bindings/mfd/iqs62x.yaml       |  5 +-
 arch/arm/boot/dts/at91-kizbox.dts             | 10 +--
 arch/arm/boot/dts/at91-kizbox2-common.dtsi    |  8 +--
 arch/arm/boot/dts/at91-kizbox3-hs.dts         | 16 ++---
 arch/arm/boot/dts/at91-kizbox3_common.dtsi    | 10 +--
 arch/arm/boot/dts/at91-kizboxmini-common.dtsi |  8 +--
 arch/arm/boot/dts/at91-smartkiz.dts           |  6 +-
 arch/arm/boot/dts/at91sam9m10g45ek.dts        | 10 +--
 arch/arm/boot/dts/at91sam9rlek.dts            | 10 +--
 .../boot/dts/berlin2cd-google-chromecast.dts  |  6 +-
 arch/arm/boot/dts/exynos5422-odroidhc1.dts    |  4 +-
 arch/arm/boot/dts/exynos5422-odroidxu4.dts    |  4 +-
 .../boot/dts/exynos54xx-odroidxu-leds.dtsi    | 11 +--
 arch/arm/boot/dts/imx53-ppd.dts               | 15 ++--
 arch/arm/boot/dts/imx6qdl-cubox-i.dtsi        |  4 +-
 .../boot/dts/imx6sx-softing-vining-2000.dts   |  8 +--
 arch/arm/boot/dts/omap3-beagle-xm.dts         | 10 +--
 arch/arm/boot/dts/omap3-overo-base.dtsi       |  4 +-
 arch/arm/boot/dts/omap4-kc1.dts               |  6 +-
 arch/arm/boot/dts/omap4-sdp.dts               | 26 +++----
 arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts     | 12 ++--
 .../amlogic/meson-gxl-s905x-khadas-vim.dts    |  4 +-
 .../dts/amlogic/meson-gxm-khadas-vim2.dts     |  4 +-
 .../boot/dts/amlogic/meson-sm1-sei610.dts     |  8 +--
 arch/mips/boot/dts/img/pistachio_marduk.dts   |  5 +-
 drivers/leds/leds-pwm.c                       | 30 ++------
 28 files changed, 184 insertions(+), 180 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/leds/leds-pwm.txt
 create mode 100644 Documentation/devicetree/bindings/leds/leds-pwm.yaml


base-commit: 8fd8f94235c2c925d80b2316e0ab2bdd00af9bae

Comments

Krzysztof Kozlowski Oct. 2, 2020, 9:12 a.m. UTC | #1
On Thu, 1 Oct 2020 at 01:53, Alexander Dahl <post@lespocky.de> wrote:
>
> The node names for devices using the pwm-leds driver follow a certain
> naming scheme (now).
>
> Signed-off-by: Alexander Dahl <post@lespocky.de>
> ---
>
> Notes:
>     v6:
>       * added this patch to series
>
>  arch/arm/boot/dts/at91-kizbox.dts             | 10 +++----
>  arch/arm/boot/dts/at91-kizbox2-common.dtsi    |  8 +++---
>  arch/arm/boot/dts/at91-kizbox3-hs.dts         | 16 ++++++------
>  arch/arm/boot/dts/at91-kizbox3_common.dtsi    | 10 +++----
>  arch/arm/boot/dts/at91-kizboxmini-common.dtsi |  8 +++---
>  arch/arm/boot/dts/at91sam9m10g45ek.dts        | 10 +++----
>  arch/arm/boot/dts/at91sam9rlek.dts            | 10 +++----
>  .../boot/dts/berlin2cd-google-chromecast.dts  |  6 ++---
>  arch/arm/boot/dts/exynos5422-odroidhc1.dts    |  4 +--
>  arch/arm/boot/dts/exynos5422-odroidxu4.dts    |  4 +--

Somehow you did not CC the maintainers... please use
scripts/get_maintainers.pl to obtain list of addresses.

All these should be separate patches per sub-architecture.

Best regards,
Krzysztof
Neil Armstrong Oct. 2, 2020, 9:23 a.m. UTC | #2
On 01/10/2020 01:46, Alexander Dahl wrote:
> The node names for devices using the pwm-leds driver follow a certain
> naming scheme (now).
> 
> Signed-off-by: Alexander Dahl <post@lespocky.de>
> ---
> 
> Notes:
>     v6:
>       * added this patch to series
> 
>  .../arm64/boot/dts/amlogic/meson-gxl-s905x-khadas-vim.dts | 4 ++--
>  arch/arm64/boot/dts/amlogic/meson-gxm-khadas-vim2.dts     | 4 ++--
>  arch/arm64/boot/dts/amlogic/meson-sm1-sei610.dts          | 8 ++++----
>  3 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-khadas-vim.dts b/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-khadas-vim.dts
> index 8bcdffdf55d0..adfc72500e66 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-khadas-vim.dts
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxl-s905x-khadas-vim.dts
> @@ -42,10 +42,10 @@
>  		};
>  	};
>  
> -	pwmleds {
> +	led-controller {
>  		compatible = "pwm-leds";
>  
> -		power {
> +		led-1 {
>  			label = "vim:red:power";
>  			pwms = <&pwm_AO_ab 1 7812500 0>;
>  			max-brightness = <255>;
> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxm-khadas-vim2.dts b/arch/arm64/boot/dts/amlogic/meson-gxm-khadas-vim2.dts
> index bff8ec2c1c70..dcb435af4e0b 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-gxm-khadas-vim2.dts
> +++ b/arch/arm64/boot/dts/amlogic/meson-gxm-khadas-vim2.dts
> @@ -81,10 +81,10 @@
>  		};
>  	};
>  
> -	pwmleds {
> +	led-controller {
>  		compatible = "pwm-leds";
>  
> -		power {
> +		led-1 {
>  			label = "vim:red:power";
>  			pwms = <&pwm_AO_ab 1 7812500 0>;
>  			max-brightness = <255>;
> diff --git a/arch/arm64/boot/dts/amlogic/meson-sm1-sei610.dts b/arch/arm64/boot/dts/amlogic/meson-sm1-sei610.dts
> index 5ab139a34c01..039a8d0d1e9b 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-sm1-sei610.dts
> +++ b/arch/arm64/boot/dts/amlogic/meson-sm1-sei610.dts
> @@ -101,20 +101,20 @@
>  		};
>  	};
>  
> -	leds {
> +	led-controller-1 {
>  		compatible = "gpio-leds";
>  
> -		led-bluetooth {
> +		led-1 {
>  			label = "sei610:blue:bt";
>  			gpios = <&gpio GPIOC_7 (GPIO_ACTIVE_LOW | GPIO_OPEN_DRAIN)>;
>  			default-state = "off";
>  		};
>  	};
>  
> -	pwmleds {
> +	led-controller-2 {
>  		compatible = "pwm-leds";
>  
> -		power {
> +		led-2 {
>  			label = "sei610:red:power";
>  			pwms = <&pwm_AO_ab 0 30518 0>;
>  			max-brightness = <255>;
> 

Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
Alexander Dahl Oct. 2, 2020, 9:27 a.m. UTC | #3
Hello Krzysztof,

Am Freitag, 2. Oktober 2020, 11:12:50 CEST schrieb Krzysztof Kozlowski:
> On Thu, 1 Oct 2020 at 01:53, Alexander Dahl <post@lespocky.de> wrote:
> > The node names for devices using the pwm-leds driver follow a certain
> > naming scheme (now).
> > 
> > Signed-off-by: Alexander Dahl <post@lespocky.de>
> > ---
> > 
> > Notes:
> >     v6:
> >       * added this patch to series
> >  
> >  arch/arm/boot/dts/at91-kizbox.dts             | 10 +++----
> >  arch/arm/boot/dts/at91-kizbox2-common.dtsi    |  8 +++---
> >  arch/arm/boot/dts/at91-kizbox3-hs.dts         | 16 ++++++------
> >  arch/arm/boot/dts/at91-kizbox3_common.dtsi    | 10 +++----
> >  arch/arm/boot/dts/at91-kizboxmini-common.dtsi |  8 +++---
> >  arch/arm/boot/dts/at91sam9m10g45ek.dts        | 10 +++----
> >  arch/arm/boot/dts/at91sam9rlek.dts            | 10 +++----
> >  .../boot/dts/berlin2cd-google-chromecast.dts  |  6 ++---
> >  arch/arm/boot/dts/exynos5422-odroidhc1.dts    |  4 +--
> >  arch/arm/boot/dts/exynos5422-odroidxu4.dts    |  4 +--
> 
> Somehow you did not CC the maintainers... please use
> scripts/get_maintainers.pl to obtain list of addresses.

Well, that will be a huge list of Cc then.  What is the policy?  Everybody 
gets the whole series or different list of receivers per patch?

> All these should be separate patches per sub-architecture.

I already suspected that.  Will do in v7.

Thanks for your feedback.
Alex
Krzysztof Kozlowski Oct. 2, 2020, 9:31 a.m. UTC | #4
On Thu, 1 Oct 2020 at 01:52, Alexander Dahl <post@lespocky.de> wrote:
>
> The example was adapted in the following ways:
>
> - make use of the now supported 'function' and 'color' properties
> - remove pwm nodes, those are documented elsewhere
> - tweake node names to be matched by new dtschema rules

tweak? or align?

>
> License was discussed with the original author.

Since you relicense their work, you need an ack or signed off from
every author. You cannot just say "I discussed" and post it. That way
I could pretend (lie) I talked to Linus and try to relicense Linux to
BSD...

You need acks/SoB from Peter and Russel.

Best regards,
Krzysztof
Krzysztof Kozlowski Oct. 2, 2020, 10:06 a.m. UTC | #5
On Fri, 2 Oct 2020 at 11:28, Alexander Dahl <ada@thorsis.com> wrote:
>
> Hello Krzysztof,
>
> Am Freitag, 2. Oktober 2020, 11:12:50 CEST schrieb Krzysztof Kozlowski:
> > On Thu, 1 Oct 2020 at 01:53, Alexander Dahl <post@lespocky.de> wrote:
> > > The node names for devices using the pwm-leds driver follow a certain
> > > naming scheme (now).
> > >
> > > Signed-off-by: Alexander Dahl <post@lespocky.de>
> > > ---
> > >
> > > Notes:
> > >     v6:
> > >       * added this patch to series
> > >
> > >  arch/arm/boot/dts/at91-kizbox.dts             | 10 +++----
> > >  arch/arm/boot/dts/at91-kizbox2-common.dtsi    |  8 +++---
> > >  arch/arm/boot/dts/at91-kizbox3-hs.dts         | 16 ++++++------
> > >  arch/arm/boot/dts/at91-kizbox3_common.dtsi    | 10 +++----
> > >  arch/arm/boot/dts/at91-kizboxmini-common.dtsi |  8 +++---
> > >  arch/arm/boot/dts/at91sam9m10g45ek.dts        | 10 +++----
> > >  arch/arm/boot/dts/at91sam9rlek.dts            | 10 +++----
> > >  .../boot/dts/berlin2cd-google-chromecast.dts  |  6 ++---
> > >  arch/arm/boot/dts/exynos5422-odroidhc1.dts    |  4 +--
> > >  arch/arm/boot/dts/exynos5422-odroidxu4.dts    |  4 +--
> >
> > Somehow you did not CC the maintainers... please use
> > scripts/get_maintainers.pl to obtain list of addresses.
>
> Well, that will be a huge list of Cc then.  What is the policy?  Everybody
> gets the whole series or different list of receivers per patch?

With git send email and get_maintainers.pl you can simplify it:
1. Send cover letter to everyone (could be skipped and instead send to
mailing lists and then link included under --- in each patch).
2. Send automatically each patch only to maintainers, with adjusted
in-reply-to to the cover letter already sent. Something like: git
send-email --cc-cmd "scripts/get_maintainer.pl --no-git --no-roles
--no-rolestats".

Probably 1+2 could be even one command if you put manually lists as Cc
in the cover letter.

Best regards,
Krzysztof
Alexander Dahl Oct. 2, 2020, 10:46 a.m. UTC | #6
Hei hei,

Am Freitag, 2. Oktober 2020, 11:31:09 CEST schrieb Krzysztof Kozlowski:
> On Thu, 1 Oct 2020 at 01:52, Alexander Dahl <post@lespocky.de> wrote:
> > The example was adapted in the following ways:
> > 
> > - make use of the now supported 'function' and 'color' properties
> > - remove pwm nodes, those are documented elsewhere
> > - tweake node names to be matched by new dtschema rules
> 
> tweak? or align?

Depends on if schema actually checks it (child nodes) or if it's just DT 
policy (parent node).  I'll reword in v7.

> > License was discussed with the original author.
> 
> Since you relicense their work, you need an ack or signed off from
> every author. You cannot just say "I discussed" and post it. That way
> I could pretend (lie) I talked to Linus and try to relicense Linux to
> BSD...

I know.  Peter promised to give his Ack publicly on the list back when I 
worked on v2 or v3, so he is in Cc since then, but apparently he did not yet 
post it. ;-)

> You need acks/SoB from Peter and Russel.

Well, I should add Russel in v7, too, then.

Thanks
Alex
Krzysztof Kozlowski Oct. 2, 2020, 10:52 a.m. UTC | #7
On Fri, 2 Oct 2020 at 12:46, Alexander Dahl <ada@thorsis.com> wrote:
>
> Hei hei,
>
> Am Freitag, 2. Oktober 2020, 11:31:09 CEST schrieb Krzysztof Kozlowski:
> > On Thu, 1 Oct 2020 at 01:52, Alexander Dahl <post@lespocky.de> wrote:
> > > The example was adapted in the following ways:
> > >
> > > - make use of the now supported 'function' and 'color' properties
> > > - remove pwm nodes, those are documented elsewhere
> > > - tweake node names to be matched by new dtschema rules
> >
> > tweak? or align?
>
> Depends on if schema actually checks it (child nodes) or if it's just DT
> policy (parent node).  I'll reword in v7.
>
> > > License was discussed with the original author.
> >
> > Since you relicense their work, you need an ack or signed off from
> > every author. You cannot just say "I discussed" and post it. That way
> > I could pretend (lie) I talked to Linus and try to relicense Linux to
> > BSD...
>
> I know.  Peter promised to give his Ack publicly on the list back when I
> worked on v2 or v3, so he is in Cc since then, but apparently he did not yet
> post it. ;-)
>
> > You need acks/SoB from Peter and Russel.
>
> Well, I should add Russel in v7, too, then.

Yes, please.

For the patch itself:
Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof
Rob Herring Oct. 5, 2020, 1:51 p.m. UTC | #8
On Thu, 01 Oct 2020 01:46:32 +0200, Alexander Dahl wrote:
> The example was adapted in the following ways:
> 
> - make use of the now supported 'function' and 'color' properties
> - remove pwm nodes, those are documented elsewhere
> - tweake node names to be matched by new dtschema rules
> 
> License was discussed with the original author.
> 
> Suggested-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
> Signed-off-by: Alexander Dahl <post@lespocky.de>
> Cc: Peter Ujfalusi <peter.ujfalusi@ti.com>
> ---
> 
> Notes:
>     v5 -> v6:
>       * removed pwm nodes from example (Rob)
>       * renamed led-controller node in example (Rob)
> 
>     v4 -> v5:
>       * updated based on feedback by Rob Herring
>       * removed Acked-by
> 
>     v3 -> v4:
>       * added Cc to original author of the binding
> 
>     v2 -> v3:
>       * changed license identifier to recommended one
>       * added Acked-by
> 
>     v2:
>       * added this patch to series (Suggested-by: Jacek Anaszewski)
> 
>  .../devicetree/bindings/leds/leds-pwm.txt     | 50 -------------
>  .../devicetree/bindings/leds/leds-pwm.yaml    | 70 +++++++++++++++++++
>  2 files changed, 70 insertions(+), 50 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/leds/leds-pwm.txt
>  create mode 100644 Documentation/devicetree/bindings/leds/leds-pwm.yaml
> 


My bot found errors running 'make dt_binding_check' on your patch:

/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mfd/iqs62x.example.dt.yaml: pwmleds: 'panel' does not match any of the regexes: '^led(-[0-9a-f]+)?$', 'pinctrl-[0-9]+'
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/leds/leds-pwm.yaml


See https://patchwork.ozlabs.org/patch/1374765

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure dt-schema is up to date:

pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade

Please check and re-submit.
Rob Herring Oct. 5, 2020, 1:54 p.m. UTC | #9
On Thu, 01 Oct 2020 01:46:32 +0200, Alexander Dahl wrote:
> The example was adapted in the following ways:
> 
> - make use of the now supported 'function' and 'color' properties
> - remove pwm nodes, those are documented elsewhere
> - tweake node names to be matched by new dtschema rules
> 
> License was discussed with the original author.
> 
> Suggested-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
> Signed-off-by: Alexander Dahl <post@lespocky.de>
> Cc: Peter Ujfalusi <peter.ujfalusi@ti.com>
> ---
> 
> Notes:
>     v5 -> v6:
>       * removed pwm nodes from example (Rob)
>       * renamed led-controller node in example (Rob)
> 
>     v4 -> v5:
>       * updated based on feedback by Rob Herring
>       * removed Acked-by
> 
>     v3 -> v4:
>       * added Cc to original author of the binding
> 
>     v2 -> v3:
>       * changed license identifier to recommended one
>       * added Acked-by
> 
>     v2:
>       * added this patch to series (Suggested-by: Jacek Anaszewski)
> 
>  .../devicetree/bindings/leds/leds-pwm.txt     | 50 -------------
>  .../devicetree/bindings/leds/leds-pwm.yaml    | 70 +++++++++++++++++++
>  2 files changed, 70 insertions(+), 50 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/leds/leds-pwm.txt
>  create mode 100644 Documentation/devicetree/bindings/leds/leds-pwm.yaml
> 

Reviewed-by: Rob Herring <robh@kernel.org>