diff mbox series

[SRU,jammy,xilinx-zynqmp,1/3] arm64: zynqmp: Sync clock labels with kr260 revB

Message ID 20240308002515.1211465-2-portia.stephens@canonical.com
State New
Headers show
Series Update on-chip oscillator clock nodes for Kria | expand

Commit Message

Portia Stephens March 8, 2024, 12:25 a.m. UTC 
From: Michal Simek <michal.simek@amd.com>

BugLink: https://bugs.launchpad.net/bugs/2055241

Board description describes the hard part of chip (PS) but programmable
logic (PL) part is not described in this file. But clocks on the board are
not only connected to PS but also wired to PL. And because two revisions
are available where revA is using one si5332 and revB multiple clock chips
using the same clock labels helping with keeping only one device tree
overlay which targets PL. That's why synchronize clock labels and use
labels from revB which are more generic.
Unfortunately if there is driver for si5332 chip split could happen again
but it is still worth to do it now and solve this issue when occurs.

(cherry picked from commit bd1a7261325afa7526ed12fbaeb8f2e939bd02f8 linux-xlnx/xlnx_rebase_v6.6_LTS)

Reported-by: Sagar Karmarkar <sagar.karmarkar@amd.com>
Link: https://lore.kernel.org/r/abac6069e6029ed4076ec7b9d6b33604b6072aa3.1706253871.git.michal.simek@amd.com
Signed-off-by: Michal Simek <michal.simek@amd.com>
State: pending
Signed-off-by: Portia Stephens <portia.stephens@canonical.com>
---
 .../arm64/boot/dts/xilinx/zynqmp-sck-kr-g-revA.dts | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

Comments

Andrei Gherzan March 13, 2024, 10:11 a.m. UTC | #1 
On 24/03/08 10:25AM, Portia Stephens wrote:
> From: Michal Simek <michal.simek@amd.com>
> 
> BugLink: https://bugs.launchpad.net/bugs/2055241
> 
> Board description describes the hard part of chip (PS) but programmable
> logic (PL) part is not described in this file. But clocks on the board are
> not only connected to PS but also wired to PL. And because two revisions
> are available where revA is using one si5332 and revB multiple clock chips
> using the same clock labels helping with keeping only one device tree
> overlay which targets PL. That's why synchronize clock labels and use
> labels from revB which are more generic.
> Unfortunately if there is driver for si5332 chip split could happen again
> but it is still worth to do it now and solve this issue when occurs.
> 
> (cherry picked from commit bd1a7261325afa7526ed12fbaeb8f2e939bd02f8 linux-xlnx/xlnx_rebase_v6.6_LTS)

The inclusion of the "cherry pick" inside the original git message was
confusing to me. After checking the original patches (e.g.
https://github.com/Xilinx/linux-xlnx/commit/bd1a7261325afa7526ed12fbaeb8f2e939bd02f8)
I can confirm that this is not inherited from it.

> 
> Reported-by: Sagar Karmarkar <sagar.karmarkar@amd.com>
> Link: https://lore.kernel.org/r/abac6069e6029ed4076ec7b9d6b33604b6072aa3.1706253871.git.michal.simek@amd.com
> Signed-off-by: Michal Simek <michal.simek@amd.com>
> State: pending
> Signed-off-by: Portia Stephens <portia.stephens@canonical.com>
> ---
>  .../arm64/boot/dts/xilinx/zynqmp-sck-kr-g-revA.dts | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/xilinx/zynqmp-sck-kr-g-revA.dts b/arch/arm64/boot/dts/xilinx/zynqmp-sck-kr-g-revA.dts
> index 702cd91c1153..3e49dcbfe672 100644
> --- a/arch/arm64/boot/dts/xilinx/zynqmp-sck-kr-g-revA.dts
> +++ b/arch/arm64/boot/dts/xilinx/zynqmp-sck-kr-g-revA.dts
> @@ -25,37 +25,37 @@ ina260-u14 {
>  		io-channels = <&u14 0>, <&u14 1>, <&u14 2>;
>  	};
>  
> -	si5332_0: si5332_0 { /* u17 - GEM0/1 */
> +	clk_125: si5332_0 { /* u17 - GEM0/1 */
>  		compatible = "fixed-clock";
>  		#clock-cells = <0>;
>  		clock-frequency = <125000000>;
>  	};
>  
> -	si5332_1: si5332_1 { /* u17 - DP */
> +	clk_27: si5332_1 { /* u17 - DP */
>  		compatible = "fixed-clock";
>  		#clock-cells = <0>;
>  		clock-frequency = <27000000>;
>  	};
>  
> -	si5332_2: si5332_2 { /* u17 - USB */
> +	clk_26: si5332_2 { /* u17 - USB */
>  		compatible = "fixed-clock";
>  		#clock-cells = <0>;
>  		clock-frequency = <26000000>;
>  	};
>  
> -	si5332_3: si5332_3 { /* u17 - SFP+ */
> +	clk_156: si5332_3 { /* u17 - SFP+ */
>  		compatible = "fixed-clock";
>  		#clock-cells = <0>;
>  		clock-frequency = <156250000>;
>  	};
>  
> -	si5332_4: si5332_4 { /* u17 - GEM2 */
> +	clk_25_0: si5332_4 { /* u17 - GEM2 */
>  		compatible = "fixed-clock";
>  		#clock-cells = <0>;
>  		clock-frequency = <25000000>;
>  	};
>  
> -	si5332_5: si5332_5 { /* u17 - GEM3 */
> +	clk_25_1: si5332_5 { /* u17 - GEM3 */
>  		compatible = "fixed-clock";
>  		#clock-cells = <0>;
>  		clock-frequency = <25000000>;
> @@ -115,7 +115,7 @@ usbhub_i2c1: i2c@1 {
>  &psgtr {
>  	status = "okay";
>  	/* gem0/1, dp, usb */
> -	clocks = <&si5332_0>, <&si5332_1>, <&si5332_2>;
> +	clocks = <&clk_125>, <&clk_27>, <&clk_26>;
>  	clock-names = "ref0", "ref1", "ref2";
>  };
>  
> -- 
> 2.34.1
> 
> 
> -- 
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
Portia Stephens March 13, 2024, 11:36 p.m. UTC | #2 
On Wed, Mar 13, 2024 at 8:12 PM Andrei Gherzan
<andrei.gherzan@canonical.com> wrote:
>
> On 24/03/08 10:25AM, Portia Stephens wrote:
> > From: Michal Simek <michal.simek@amd.com>
> >
> > BugLink: https://bugs.launchpad.net/bugs/2055241
> >
> > Board description describes the hard part of chip (PS) but programmable
> > logic (PL) part is not described in this file. But clocks on the board are
> > not only connected to PS but also wired to PL. And because two revisions
> > are available where revA is using one si5332 and revB multiple clock chips
> > using the same clock labels helping with keeping only one device tree
> > overlay which targets PL. That's why synchronize clock labels and use
> > labels from revB which are more generic.
> > Unfortunately if there is driver for si5332 chip split could happen again
> > but it is still worth to do it now and solve this issue when occurs.
> >
> > (cherry picked from commit bd1a7261325afa7526ed12fbaeb8f2e939bd02f8 linux-xlnx/xlnx_rebase_v6.6_LTS)
>
> The inclusion of the "cherry pick" inside the original git message was
> confusing to me. After checking the original patches (e.g.
> https://github.com/Xilinx/linux-xlnx/commit/bd1a7261325afa7526ed12fbaeb8f2e939bd02f8)
> I can confirm that this is not inherited from it.

Would you expect the "cherry pick" to come directly before my
signed-off-by line? I can fix it up when I apply.

>
> >
> > Reported-by: Sagar Karmarkar <sagar.karmarkar@amd.com>
> > Link: https://lore.kernel.org/r/abac6069e6029ed4076ec7b9d6b33604b6072aa3.1706253871.git.michal.simek@amd.com
> > Signed-off-by: Michal Simek <michal.simek@amd.com>
> > State: pending
> > Signed-off-by: Portia Stephens <portia.stephens@canonical.com>
> > ---
> >  .../arm64/boot/dts/xilinx/zynqmp-sck-kr-g-revA.dts | 14 +++++++-------
> >  1 file changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/xilinx/zynqmp-sck-kr-g-revA.dts b/arch/arm64/boot/dts/xilinx/zynqmp-sck-kr-g-revA.dts
> > index 702cd91c1153..3e49dcbfe672 100644
> > --- a/arch/arm64/boot/dts/xilinx/zynqmp-sck-kr-g-revA.dts
> > +++ b/arch/arm64/boot/dts/xilinx/zynqmp-sck-kr-g-revA.dts
> > @@ -25,37 +25,37 @@ ina260-u14 {
> >               io-channels = <&u14 0>, <&u14 1>, <&u14 2>;
> >       };
> >
> > -     si5332_0: si5332_0 { /* u17 - GEM0/1 */
> > +     clk_125: si5332_0 { /* u17 - GEM0/1 */
> >               compatible = "fixed-clock";
> >               #clock-cells = <0>;
> >               clock-frequency = <125000000>;
> >       };
> >
> > -     si5332_1: si5332_1 { /* u17 - DP */
> > +     clk_27: si5332_1 { /* u17 - DP */
> >               compatible = "fixed-clock";
> >               #clock-cells = <0>;
> >               clock-frequency = <27000000>;
> >       };
> >
> > -     si5332_2: si5332_2 { /* u17 - USB */
> > +     clk_26: si5332_2 { /* u17 - USB */
> >               compatible = "fixed-clock";
> >               #clock-cells = <0>;
> >               clock-frequency = <26000000>;
> >       };
> >
> > -     si5332_3: si5332_3 { /* u17 - SFP+ */
> > +     clk_156: si5332_3 { /* u17 - SFP+ */
> >               compatible = "fixed-clock";
> >               #clock-cells = <0>;
> >               clock-frequency = <156250000>;
> >       };
> >
> > -     si5332_4: si5332_4 { /* u17 - GEM2 */
> > +     clk_25_0: si5332_4 { /* u17 - GEM2 */
> >               compatible = "fixed-clock";
> >               #clock-cells = <0>;
> >               clock-frequency = <25000000>;
> >       };
> >
> > -     si5332_5: si5332_5 { /* u17 - GEM3 */
> > +     clk_25_1: si5332_5 { /* u17 - GEM3 */
> >               compatible = "fixed-clock";
> >               #clock-cells = <0>;
> >               clock-frequency = <25000000>;
> > @@ -115,7 +115,7 @@ usbhub_i2c1: i2c@1 {
> >  &psgtr {
> >       status = "okay";
> >       /* gem0/1, dp, usb */
> > -     clocks = <&si5332_0>, <&si5332_1>, <&si5332_2>;
> > +     clocks = <&clk_125>, <&clk_27>, <&clk_26>;
> >       clock-names = "ref0", "ref1", "ref2";
> >  };
> >
> > --
> > 2.34.1
> >
> >
> > --
> > kernel-team mailing list
> > kernel-team@lists.ubuntu.com
> > https://lists.ubuntu.com/mailman/listinfo/kernel-team
>
> --
> Andrei Gherzan
Andrei Gherzan March 18, 2024, 10:42 a.m. UTC | #3 
On 24/03/14 09:36AM, Portia Stephens wrote:
> On Wed, Mar 13, 2024 at 8:12 PM Andrei Gherzan
> <andrei.gherzan@canonical.com> wrote:
> >
> > On 24/03/08 10:25AM, Portia Stephens wrote:
> > > From: Michal Simek <michal.simek@amd.com>
> > >
> > > BugLink: https://bugs.launchpad.net/bugs/2055241
> > >
> > > Board description describes the hard part of chip (PS) but programmable
> > > logic (PL) part is not described in this file. But clocks on the board are
> > > not only connected to PS but also wired to PL. And because two revisions
> > > are available where revA is using one si5332 and revB multiple clock chips
> > > using the same clock labels helping with keeping only one device tree
> > > overlay which targets PL. That's why synchronize clock labels and use
> > > labels from revB which are more generic.
> > > Unfortunately if there is driver for si5332 chip split could happen again
> > > but it is still worth to do it now and solve this issue when occurs.
> > >
> > > (cherry picked from commit bd1a7261325afa7526ed12fbaeb8f2e939bd02f8 linux-xlnx/xlnx_rebase_v6.6_LTS)
> >
> > The inclusion of the "cherry pick" inside the original git message was
> > confusing to me. After checking the original patches (e.g.
> > https://github.com/Xilinx/linux-xlnx/commit/bd1a7261325afa7526ed12fbaeb8f2e939bd02f8)
> > I can confirm that this is not inherited from it.
> 
> Would you expect the "cherry pick" to come directly before my
> signed-off-by line? I can fix it up when I apply.

My standard rule (and expectation) is that we append to the git log
message - except BugLink. I would keep this as a comment unless the
maintainer considers it essential. I don't see anything about this in
the documentation; hence, it does not warrant a resubmission.
Andrei Gherzan March 18, 2024, 10:42 a.m. UTC | #4 
On 24/03/18 10:42AM, Andrei Gherzan wrote:
> On 24/03/14 09:36AM, Portia Stephens wrote:
> > On Wed, Mar 13, 2024 at 8:12 PM Andrei Gherzan
> > <andrei.gherzan@canonical.com> wrote:
> > >
> > > On 24/03/08 10:25AM, Portia Stephens wrote:
> > > > From: Michal Simek <michal.simek@amd.com>
> > > >
> > > > BugLink: https://bugs.launchpad.net/bugs/2055241
> > > >
> > > > Board description describes the hard part of chip (PS) but programmable
> > > > logic (PL) part is not described in this file. But clocks on the board are
> > > > not only connected to PS but also wired to PL. And because two revisions
> > > > are available where revA is using one si5332 and revB multiple clock chips
> > > > using the same clock labels helping with keeping only one device tree
> > > > overlay which targets PL. That's why synchronize clock labels and use
> > > > labels from revB which are more generic.
> > > > Unfortunately if there is driver for si5332 chip split could happen again
> > > > but it is still worth to do it now and solve this issue when occurs.
> > > >
> > > > (cherry picked from commit bd1a7261325afa7526ed12fbaeb8f2e939bd02f8 linux-xlnx/xlnx_rebase_v6.6_LTS)
> > >
> > > The inclusion of the "cherry pick" inside the original git message was
> > > confusing to me. After checking the original patches (e.g.
> > > https://github.com/Xilinx/linux-xlnx/commit/bd1a7261325afa7526ed12fbaeb8f2e939bd02f8)
> > > I can confirm that this is not inherited from it.
> > 
> > Would you expect the "cherry pick" to come directly before my
> > signed-off-by line? I can fix it up when I apply.
> 
> My standard rule (and expectation) is that we append to the git log
> message - except BugLink. I would keep this as a comment unless the
> maintainer considers it essential. I don't see anything about this in
> the documentation; hence, it does not warrant a resubmission.

Acked-by: Andrei Gherzan <andrei.gherzan@canonical.com>
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/xilinx/zynqmp-sck-kr-g-revA.dts b/arch/arm64/boot/dts/xilinx/zynqmp-sck-kr-g-revA.dts
index 702cd91c1153..3e49dcbfe672 100644
--- a/arch/arm64/boot/dts/xilinx/zynqmp-sck-kr-g-revA.dts
+++ b/arch/arm64/boot/dts/xilinx/zynqmp-sck-kr-g-revA.dts
@@ -25,37 +25,37 @@  ina260-u14 {
 		io-channels = <&u14 0>, <&u14 1>, <&u14 2>;
 	};
 
-	si5332_0: si5332_0 { /* u17 - GEM0/1 */
+	clk_125: si5332_0 { /* u17 - GEM0/1 */
 		compatible = "fixed-clock";
 		#clock-cells = <0>;
 		clock-frequency = <125000000>;
 	};
 
-	si5332_1: si5332_1 { /* u17 - DP */
+	clk_27: si5332_1 { /* u17 - DP */
 		compatible = "fixed-clock";
 		#clock-cells = <0>;
 		clock-frequency = <27000000>;
 	};
 
-	si5332_2: si5332_2 { /* u17 - USB */
+	clk_26: si5332_2 { /* u17 - USB */
 		compatible = "fixed-clock";
 		#clock-cells = <0>;
 		clock-frequency = <26000000>;
 	};
 
-	si5332_3: si5332_3 { /* u17 - SFP+ */
+	clk_156: si5332_3 { /* u17 - SFP+ */
 		compatible = "fixed-clock";
 		#clock-cells = <0>;
 		clock-frequency = <156250000>;
 	};
 
-	si5332_4: si5332_4 { /* u17 - GEM2 */
+	clk_25_0: si5332_4 { /* u17 - GEM2 */
 		compatible = "fixed-clock";
 		#clock-cells = <0>;
 		clock-frequency = <25000000>;
 	};
 
-	si5332_5: si5332_5 { /* u17 - GEM3 */
+	clk_25_1: si5332_5 { /* u17 - GEM3 */
 		compatible = "fixed-clock";
 		#clock-cells = <0>;
 		clock-frequency = <25000000>;
@@ -115,7 +115,7 @@  usbhub_i2c1: i2c@1 {
 &psgtr {
 	status = "okay";
 	/* gem0/1, dp, usb */
-	clocks = <&si5332_0>, <&si5332_1>, <&si5332_2>;
+	clocks = <&clk_125>, <&clk_27>, <&clk_26>;
 	clock-names = "ref0", "ref1", "ref2";
 };