Message ID | 20240308002515.1211465-2-portia.stephens@canonical.com |
---|---|
State | New |
Headers | show |
Series | Update on-chip oscillator clock nodes for Kria | expand |
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
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
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.
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 --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"; };