diff mbox series

[v5,1/2] riscv: dts: sifive: unleashed/unmatched: Remove PWM controlled LED's active-low properties

Message ID 20231024101902.6689-2-nylon.chen@sifive.com
State New
Headers
Series Change PWM-controlled LED pin active mode and algorithm |

Commit Message

Nylon Chen Oct. 24, 2023, 10:19 a.m. UTC 
  This removes the active-low properties of the PWM-controlled LEDs in
the HiFive Unmatched device tree.

The reference is hifive-unleashed-a00.pdf[0] and hifive-unmatched-schematics-v3.pdf[1].

Link: https://sifive.cdn.prismic.io/sifive/c52a8e32-05ce-4aaf-95c8-7bf8453f8698_hifive-unleashed-a00-schematics-1.pdf [0]
Link: https://sifive.cdn.prismic.io/sifive/6a06d6c0-6e66-49b5-8e9e-e68ce76f4192_hifive-unmatched-schematics-v3.pdf [1]

Signed-off-by: Vincent Chen <vincent.chen@sifive.com>
Signed-off-by: Nylon Chen <nylon.chen@sifive.com>
---
 arch/riscv/boot/dts/sifive/hifive-unleashed-a00.dts |  8 ++++----
 arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts | 12 ++++--------
 2 files changed, 8 insertions(+), 12 deletions(-)

Comments

Conor Dooley Oct. 24, 2023, 2:55 p.m. UTC | #1 
Hey,

On Tue, Oct 24, 2023 at 06:19:01PM +0800, Nylon Chen wrote:
> This removes the active-low properties of the PWM-controlled LEDs in
> the HiFive Unmatched device tree.
> 
> The reference is hifive-unleashed-a00.pdf[0] and hifive-unmatched-schematics-v3.pdf[1].
> 
> Link: https://sifive.cdn.prismic.io/sifive/c52a8e32-05ce-4aaf-95c8-7bf8453f8698_hifive-unleashed-a00-schematics-1.pdf [0]
> Link: https://sifive.cdn.prismic.io/sifive/6a06d6c0-6e66-49b5-8e9e-e68ce76f4192_hifive-unmatched-schematics-v3.pdf [1]

> 

This blank line should be removed if there is a follow-up.

> Signed-off-by: Vincent Chen <vincent.chen@sifive.com>

What did Vincent contribute to this patch? Are you missing a
co-developed-by tag, perhaps?

> Signed-off-by: Nylon Chen <nylon.chen@sifive.com>

Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
Acked-by: Conor Dooley <conor.dooley@microchip.com>

I expect this to go via the pwm tree since this is going to "break" (in
the loosest possible sense) existing systems if merged separately.

Cheers,
Conor.

> ---
>  arch/riscv/boot/dts/sifive/hifive-unleashed-a00.dts |  8 ++++----
>  arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts | 12 ++++--------
>  2 files changed, 8 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/riscv/boot/dts/sifive/hifive-unleashed-a00.dts b/arch/riscv/boot/dts/sifive/hifive-unleashed-a00.dts
> index 900a50526d77..11e7ac1c54bb 100644
> --- a/arch/riscv/boot/dts/sifive/hifive-unleashed-a00.dts
> +++ b/arch/riscv/boot/dts/sifive/hifive-unleashed-a00.dts
> @@ -49,7 +49,7 @@ led-controller {
>  		compatible = "pwm-leds";
>  
>  		led-d1 {
> -			pwms = <&pwm0 0 7812500 PWM_POLARITY_INVERTED>;
> +			pwms = <&pwm0 0 7812500 0>;
>  			active-low;
>  			color = <LED_COLOR_ID_GREEN>;
>  			max-brightness = <255>;
> @@ -57,7 +57,7 @@ led-d1 {
>  		};
>  
>  		led-d2 {
> -			pwms = <&pwm0 1 7812500 PWM_POLARITY_INVERTED>;
> +			pwms = <&pwm0 1 7812500 0>;
>  			active-low;
>  			color = <LED_COLOR_ID_GREEN>;
>  			max-brightness = <255>;
> @@ -65,7 +65,7 @@ led-d2 {
>  		};
>  
>  		led-d3 {
> -			pwms = <&pwm0 2 7812500 PWM_POLARITY_INVERTED>;
> +			pwms = <&pwm0 2 7812500 0>;
>  			active-low;
>  			color = <LED_COLOR_ID_GREEN>;
>  			max-brightness = <255>;
> @@ -73,7 +73,7 @@ led-d3 {
>  		};
>  
>  		led-d4 {
> -			pwms = <&pwm0 3 7812500 PWM_POLARITY_INVERTED>;
> +			pwms = <&pwm0 3 7812500 0>;
>  			active-low;
>  			color = <LED_COLOR_ID_GREEN>;
>  			max-brightness = <255>;
> diff --git a/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts b/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts
> index 07387f9c135c..b328ee80693f 100644
> --- a/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts
> +++ b/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts
> @@ -51,8 +51,7 @@ led-controller-1 {
>  		compatible = "pwm-leds";
>  
>  		led-d12 {
> -			pwms = <&pwm0 0 7812500 PWM_POLARITY_INVERTED>;
> -			active-low;
> +			pwms = <&pwm0 0 7812500 0>;
>  			color = <LED_COLOR_ID_GREEN>;
>  			max-brightness = <255>;
>  			label = "d12";
> @@ -68,20 +67,17 @@ multi-led {
>  			label = "d2";
>  
>  			led-red {
> -				pwms = <&pwm0 2 7812500 PWM_POLARITY_INVERTED>;
> -				active-low;
> +				pwms = <&pwm0 2 7812500 0>;
>  				color = <LED_COLOR_ID_RED>;
>  			};
>  
>  			led-green {
> -				pwms = <&pwm0 1 7812500 PWM_POLARITY_INVERTED>;
> -				active-low;
> +				pwms = <&pwm0 1 7812500 0>;
>  				color = <LED_COLOR_ID_GREEN>;
>  			};
>  
>  			led-blue {
> -				pwms = <&pwm0 3 7812500 PWM_POLARITY_INVERTED>;
> -				active-low;
> +				pwms = <&pwm0 3 7812500 0>;
>  				color = <LED_COLOR_ID_BLUE>;
>  			};
>  		};
> -- 
> 2.42.0
>
Nylon Chen Oct. 25, 2023, 9:32 a.m. UTC | #2 
Hi Conor,

Conor Dooley <conor@kernel.org> 於 2023年10月24日 週二 下午10:55寫道：
>
> Hey,
>
> On Tue, Oct 24, 2023 at 06:19:01PM +0800, Nylon Chen wrote:
> > This removes the active-low properties of the PWM-controlled LEDs in
> > the HiFive Unmatched device tree.
> >
> > The reference is hifive-unleashed-a00.pdf[0] and hifive-unmatched-schematics-v3.pdf[1].
> >
> > Link: https://sifive.cdn.prismic.io/sifive/c52a8e32-05ce-4aaf-95c8-7bf8453f8698_hifive-unleashed-a00-schematics-1.pdf [0]
> > Link: https://sifive.cdn.prismic.io/sifive/6a06d6c0-6e66-49b5-8e9e-e68ce76f4192_hifive-unmatched-schematics-v3.pdf [1]
>
> >
>
> This blank line should be removed if there is a follow-up.
thanks, I got it.
>
> > Signed-off-by: Vincent Chen <vincent.chen@sifive.com>
>
> What did Vincent contribute to this patch? Are you missing a
> co-developed-by tag, perhaps?
Yes, Vincent was the first person to find the PWM driver problem, and
Zong Li helped me with the relevant review internally.

so in the next version, I will add the correct tags for these two developers.

Thank you again for your time.
>
> > Signed-off-by: Nylon Chen <nylon.chen@sifive.com>
>
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> Acked-by: Conor Dooley <conor.dooley@microchip.com>
>
> I expect this to go via the pwm tree since this is going to "break" (in
> the loosest possible sense) existing systems if merged separately.
>
> Cheers,
> Conor.
>
> > ---
> >  arch/riscv/boot/dts/sifive/hifive-unleashed-a00.dts |  8 ++++----
> >  arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts | 12 ++++--------
> >  2 files changed, 8 insertions(+), 12 deletions(-)
> >
> > diff --git a/arch/riscv/boot/dts/sifive/hifive-unleashed-a00.dts b/arch/riscv/boot/dts/sifive/hifive-unleashed-a00.dts
> > index 900a50526d77..11e7ac1c54bb 100644
> > --- a/arch/riscv/boot/dts/sifive/hifive-unleashed-a00.dts
> > +++ b/arch/riscv/boot/dts/sifive/hifive-unleashed-a00.dts
> > @@ -49,7 +49,7 @@ led-controller {
> >               compatible = "pwm-leds";
> >
> >               led-d1 {
> > -                     pwms = <&pwm0 0 7812500 PWM_POLARITY_INVERTED>;
> > +                     pwms = <&pwm0 0 7812500 0>;
> >                       active-low;
> >                       color = <LED_COLOR_ID_GREEN>;
> >                       max-brightness = <255>;
> > @@ -57,7 +57,7 @@ led-d1 {
> >               };
> >
> >               led-d2 {
> > -                     pwms = <&pwm0 1 7812500 PWM_POLARITY_INVERTED>;
> > +                     pwms = <&pwm0 1 7812500 0>;
> >                       active-low;
> >                       color = <LED_COLOR_ID_GREEN>;
> >                       max-brightness = <255>;
> > @@ -65,7 +65,7 @@ led-d2 {
> >               };
> >
> >               led-d3 {
> > -                     pwms = <&pwm0 2 7812500 PWM_POLARITY_INVERTED>;
> > +                     pwms = <&pwm0 2 7812500 0>;
> >                       active-low;
> >                       color = <LED_COLOR_ID_GREEN>;
> >                       max-brightness = <255>;
> > @@ -73,7 +73,7 @@ led-d3 {
> >               };
> >
> >               led-d4 {
> > -                     pwms = <&pwm0 3 7812500 PWM_POLARITY_INVERTED>;
> > +                     pwms = <&pwm0 3 7812500 0>;
> >                       active-low;
> >                       color = <LED_COLOR_ID_GREEN>;
> >                       max-brightness = <255>;
> > diff --git a/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts b/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts
> > index 07387f9c135c..b328ee80693f 100644
> > --- a/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts
> > +++ b/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts
> > @@ -51,8 +51,7 @@ led-controller-1 {
> >               compatible = "pwm-leds";
> >
> >               led-d12 {
> > -                     pwms = <&pwm0 0 7812500 PWM_POLARITY_INVERTED>;
> > -                     active-low;
> > +                     pwms = <&pwm0 0 7812500 0>;
> >                       color = <LED_COLOR_ID_GREEN>;
> >                       max-brightness = <255>;
> >                       label = "d12";
> > @@ -68,20 +67,17 @@ multi-led {
> >                       label = "d2";
> >
> >                       led-red {
> > -                             pwms = <&pwm0 2 7812500 PWM_POLARITY_INVERTED>;
> > -                             active-low;
> > +                             pwms = <&pwm0 2 7812500 0>;
> >                               color = <LED_COLOR_ID_RED>;
> >                       };
> >
> >                       led-green {
> > -                             pwms = <&pwm0 1 7812500 PWM_POLARITY_INVERTED>;
> > -                             active-low;
> > +                             pwms = <&pwm0 1 7812500 0>;
> >                               color = <LED_COLOR_ID_GREEN>;
> >                       };
> >
> >                       led-blue {
> > -                             pwms = <&pwm0 3 7812500 PWM_POLARITY_INVERTED>;
> > -                             active-low;
> > +                             pwms = <&pwm0 3 7812500 0>;
> >                               color = <LED_COLOR_ID_BLUE>;
> >                       };
> >               };
> > --
> > 2.42.0
> >
Conor Dooley Oct. 25, 2023, 2:14 p.m. UTC | #3 
On Wed, Oct 25, 2023 at 05:32:21PM +0800, Nylon Chen wrote:
> Hi Conor,
> 
> Conor Dooley <conor@kernel.org> 於 2023年10月24日 週二 下午10:55寫道：
> >
> > Hey,
> >
> > On Tue, Oct 24, 2023 at 06:19:01PM +0800, Nylon Chen wrote:
> > > This removes the active-low properties of the PWM-controlled LEDs in
> > > the HiFive Unmatched device tree.
> > >
> > > The reference is hifive-unleashed-a00.pdf[0] and hifive-unmatched-schematics-v3.pdf[1].
> > >
> > > Link: https://sifive.cdn.prismic.io/sifive/c52a8e32-05ce-4aaf-95c8-7bf8453f8698_hifive-unleashed-a00-schematics-1.pdf [0]
> > > Link: https://sifive.cdn.prismic.io/sifive/6a06d6c0-6e66-49b5-8e9e-e68ce76f4192_hifive-unmatched-schematics-v3.pdf [1]
> >
> > >
> >
> > This blank line should be removed if there is a follow-up.
> thanks, I got it.
> >
> > > Signed-off-by: Vincent Chen <vincent.chen@sifive.com>
> >
> > What did Vincent contribute to this patch? Are you missing a
> > co-developed-by tag, perhaps?
> Yes, Vincent was the first person to find the PWM driver problem, and

That sounds like s/Signed-off-by/Reported-by/ then.

Cheers,
Conor.
Nylon Chen Oct. 27, 2023, 8:54 a.m. UTC | #4 
Conor Dooley <conor@kernel.org> 於 2023年10月25日 週三 下午10:14寫道：
>
> On Wed, Oct 25, 2023 at 05:32:21PM +0800, Nylon Chen wrote:
> > Hi Conor,
> >
> > Conor Dooley <conor@kernel.org> 於 2023年10月24日 週二 下午10:55寫道：
> > >
> > > Hey,
> > >
> > > On Tue, Oct 24, 2023 at 06:19:01PM +0800, Nylon Chen wrote:
> > > > This removes the active-low properties of the PWM-controlled LEDs in
> > > > the HiFive Unmatched device tree.
> > > >
> > > > The reference is hifive-unleashed-a00.pdf[0] and hifive-unmatched-schematics-v3.pdf[1].
> > > >
> > > > Link: https://sifive.cdn.prismic.io/sifive/c52a8e32-05ce-4aaf-95c8-7bf8453f8698_hifive-unleashed-a00-schematics-1.pdf [0]
> > > > Link: https://sifive.cdn.prismic.io/sifive/6a06d6c0-6e66-49b5-8e9e-e68ce76f4192_hifive-unmatched-schematics-v3.pdf [1]
> > >
> > > >
> > >
> > > This blank line should be removed if there is a follow-up.
> > thanks, I got it.
> > >
> > > > Signed-off-by: Vincent Chen <vincent.chen@sifive.com>
> > >
> > > What did Vincent contribute to this patch? Are you missing a
> > > co-developed-by tag, perhaps?
> > Yes, Vincent was the first person to find the PWM driver problem, and
>
> That sounds like s/Signed-off-by/Reported-by/ then.
Thanks, I got it.
>
> Cheers,
> Conor.
Uwe Kleine-König Dec. 11, 2023, 8:42 p.m. UTC | #5 
On Tue, Oct 24, 2023 at 06:19:01PM +0800, Nylon Chen wrote:
> This removes the active-low properties of the PWM-controlled LEDs in
> the HiFive Unmatched device tree.
> 
> The reference is hifive-unleashed-a00.pdf[0] and hifive-unmatched-schematics-v3.pdf[1].
> 
> Link: https://sifive.cdn.prismic.io/sifive/c52a8e32-05ce-4aaf-95c8-7bf8453f8698_hifive-unleashed-a00-schematics-1.pdf [0]
> Link: https://sifive.cdn.prismic.io/sifive/6a06d6c0-6e66-49b5-8e9e-e68ce76f4192_hifive-unmatched-schematics-v3.pdf [1]

IMHO the commit log should mention that the driver got inversion wrong
and so both dts and driver need adaption.

Otherwise looks fine to me.

Best regards
Uwe
diff mbox series

Patch

diff --git a/arch/riscv/boot/dts/sifive/hifive-unleashed-a00.dts b/arch/riscv/boot/dts/sifive/hifive-unleashed-a00.dts
index 900a50526d77..11e7ac1c54bb 100644
--- a/arch/riscv/boot/dts/sifive/hifive-unleashed-a00.dts
+++ b/arch/riscv/boot/dts/sifive/hifive-unleashed-a00.dts
@@ -49,7 +49,7 @@  led-controller {
 		compatible = "pwm-leds";
 
 		led-d1 {
-			pwms = <&pwm0 0 7812500 PWM_POLARITY_INVERTED>;
+			pwms = <&pwm0 0 7812500 0>;
 			active-low;
 			color = <LED_COLOR_ID_GREEN>;
 			max-brightness = <255>;
@@ -57,7 +57,7 @@  led-d1 {
 		};
 
 		led-d2 {
-			pwms = <&pwm0 1 7812500 PWM_POLARITY_INVERTED>;
+			pwms = <&pwm0 1 7812500 0>;
 			active-low;
 			color = <LED_COLOR_ID_GREEN>;
 			max-brightness = <255>;
@@ -65,7 +65,7 @@  led-d2 {
 		};
 
 		led-d3 {
-			pwms = <&pwm0 2 7812500 PWM_POLARITY_INVERTED>;
+			pwms = <&pwm0 2 7812500 0>;
 			active-low;
 			color = <LED_COLOR_ID_GREEN>;
 			max-brightness = <255>;
@@ -73,7 +73,7 @@  led-d3 {
 		};
 
 		led-d4 {
-			pwms = <&pwm0 3 7812500 PWM_POLARITY_INVERTED>;
+			pwms = <&pwm0 3 7812500 0>;
 			active-low;
 			color = <LED_COLOR_ID_GREEN>;
 			max-brightness = <255>;
diff --git a/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts b/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts
index 07387f9c135c..b328ee80693f 100644
--- a/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts
+++ b/arch/riscv/boot/dts/sifive/hifive-unmatched-a00.dts
@@ -51,8 +51,7 @@  led-controller-1 {
 		compatible = "pwm-leds";
 
 		led-d12 {
-			pwms = <&pwm0 0 7812500 PWM_POLARITY_INVERTED>;
-			active-low;
+			pwms = <&pwm0 0 7812500 0>;
 			color = <LED_COLOR_ID_GREEN>;
 			max-brightness = <255>;
 			label = "d12";
@@ -68,20 +67,17 @@  multi-led {
 			label = "d2";
 
 			led-red {
-				pwms = <&pwm0 2 7812500 PWM_POLARITY_INVERTED>;
-				active-low;
+				pwms = <&pwm0 2 7812500 0>;
 				color = <LED_COLOR_ID_RED>;
 			};
 
 			led-green {
-				pwms = <&pwm0 1 7812500 PWM_POLARITY_INVERTED>;
-				active-low;
+				pwms = <&pwm0 1 7812500 0>;
 				color = <LED_COLOR_ID_GREEN>;
 			};
 
 			led-blue {
-				pwms = <&pwm0 3 7812500 PWM_POLARITY_INVERTED>;
-				active-low;
+				pwms = <&pwm0 3 7812500 0>;
 				color = <LED_COLOR_ID_BLUE>;
 			};
 		};