| Message ID | 20220119002438.106079-1-sean.anderson@seco.com |
|---|---|
| Headers | show |
| Series | usb: dwc3: Calculate REFCLKPER et. al. from reference clock | expand |
Hi Sean, On Tue, Jan 18 2022, Sean Anderson wrote: > This is a rework of patches 3-5 of [1]. It attempts to correctly program > REFCLKPER and REFCLK_FLADJ based on the reference clock frequency. Since > we no longer need a special property duplicating this configuration, > snps,ref-clock-period-ns is deprecated. > > Please test this! Patches 3/4 in this series have the effect of > programming REFCLKPER and REFCLK_FLADJ on boards which already configure > the "ref" clock. I have build tested, but not much else. Tested here on IPQ6010 based system. USB still works. But the with "ref" clock at 24MHz, period is calculated as 0x29. Previous snps,ref-clock-period-ns value used to be 0x32. Is that expected? Thanks, baruch > > [1] https://lore.kernel.org/linux-usb/20220114044230.2677283-1-robert.hancock@calian.com/ > > Changes in v2: > - Document clock members > - Also program GFLADJ.240MHZDECR > - Don't program GFLADJ if the version is < 2.50a > - Add snps,ref-clock-frequency-hz property for ACPI > > Sean Anderson (7): > dt-bindings: usb: dwc3: Deprecate snps,ref-clock-period-ns > usb: dwc3: Get clocks individually > usb: dwc3: Calculate REFCLKPER based on reference clock > usb: dwc3: Program GFLADJ > usb: dwc3: Add snps,ref-clock-frequency-hz property for ACPI > arm64: dts: zynqmp: Move USB clocks to dwc3 node > arm64: dts: ipq6018: Use reference clock to set dwc3 period > > .../devicetree/bindings/usb/snps,dwc3.yaml | 7 +- > arch/arm64/boot/dts/qcom/ipq6018.dtsi | 3 +- > .../arm64/boot/dts/xilinx/zynqmp-clk-ccf.dtsi | 4 +- > arch/arm64/boot/dts/xilinx/zynqmp.dtsi | 4 +- > drivers/usb/dwc3/core.c | 112 +++++++++++++++--- > drivers/usb/dwc3/core.h | 17 ++- > 6 files changed, 120 insertions(+), 27 deletions(-)
Hi Baruch, On 1/19/22 1:14 PM, Baruch Siach wrote: > Hi Sean, > > On Tue, Jan 18 2022, Sean Anderson wrote: >> This is a rework of patches 3-5 of [1]. It attempts to correctly program >> REFCLKPER and REFCLK_FLADJ based on the reference clock frequency. Since >> we no longer need a special property duplicating this configuration, >> snps,ref-clock-period-ns is deprecated. >> >> Please test this! Patches 3/4 in this series have the effect of >> programming REFCLKPER and REFCLK_FLADJ on boards which already configure >> the "ref" clock. I have build tested, but not much else. > > Tested here on IPQ6010 based system. USB still works. But the with "ref" > clock at 24MHz, period is calculated as 0x29. Previous > snps,ref-clock-period-ns value used to be 0x32. > > Is that expected? Yes. From the documentation for GFLADJ_REFCLK_240MHZ_DECR: > Examples: > If the ref_clk is 24 MHz then > - GUCTL.REF_CLK_PERIOD = 41 > - GFLADJ.GFLADJ_REFCLK_240MHZ_DECR = 240/24 = 10 And 41 == 0x29. --Sean
On 2022-01-19 23:44, Baruch Siach wrote: > Hi Sean, > > On Tue, Jan 18 2022, Sean Anderson wrote: >> This is a rework of patches 3-5 of [1]. It attempts to correctly >> program >> REFCLKPER and REFCLK_FLADJ based on the reference clock frequency. >> Since >> we no longer need a special property duplicating this configuration, >> snps,ref-clock-period-ns is deprecated. >> >> Please test this! Patches 3/4 in this series have the effect of >> programming REFCLKPER and REFCLK_FLADJ on boards which already >> configure >> the "ref" clock. I have build tested, but not much else. > > Tested here on IPQ6010 based system. USB still works. But the with > "ref" > clock at 24MHz, period is calculated as 0x29. Previous > snps,ref-clock-period-ns value used to be 0x32. > > Is that expected? > > Thanks, > baruch > Hi Baruch, Yes, it is 0x29 for IPQ60xx based SoCs. In downstream it was wrongly mentioned as 0x32, which was corrected recently. Thanks, Kathiravan T. >> >> [1] >> https://lore.kernel.org/linux-usb/20220114044230.2677283-1-robert.hancock@calian.com/ >> >> Changes in v2: >> - Document clock members >> - Also program GFLADJ.240MHZDECR >> - Don't program GFLADJ if the version is < 2.50a >> - Add snps,ref-clock-frequency-hz property for ACPI >> >> Sean Anderson (7): >> dt-bindings: usb: dwc3: Deprecate snps,ref-clock-period-ns >> usb: dwc3: Get clocks individually >> usb: dwc3: Calculate REFCLKPER based on reference clock >> usb: dwc3: Program GFLADJ >> usb: dwc3: Add snps,ref-clock-frequency-hz property for ACPI >> arm64: dts: zynqmp: Move USB clocks to dwc3 node >> arm64: dts: ipq6018: Use reference clock to set dwc3 period >> >> .../devicetree/bindings/usb/snps,dwc3.yaml | 7 +- >> arch/arm64/boot/dts/qcom/ipq6018.dtsi | 3 +- >> .../arm64/boot/dts/xilinx/zynqmp-clk-ccf.dtsi | 4 +- >> arch/arm64/boot/dts/xilinx/zynqmp.dtsi | 4 +- >> drivers/usb/dwc3/core.c | 112 >> +++++++++++++++--- >> drivers/usb/dwc3/core.h | 17 ++- >> 6 files changed, 120 insertions(+), 27 deletions(-)
Hi Kathiravan, On Thu, Jan 20 2022, Kathiravan T wrote: > On 2022-01-19 23:44, Baruch Siach wrote: >> Hi Sean, >> On Tue, Jan 18 2022, Sean Anderson wrote: >>> This is a rework of patches 3-5 of [1]. It attempts to correctly program >>> REFCLKPER and REFCLK_FLADJ based on the reference clock frequency. Since >>> we no longer need a special property duplicating this configuration, >>> snps,ref-clock-period-ns is deprecated. >>> Please test this! Patches 3/4 in this series have the effect of >>> programming REFCLKPER and REFCLK_FLADJ on boards which already configure >>> the "ref" clock. I have build tested, but not much else. >> Tested here on IPQ6010 based system. USB still works. But the with >> "ref" >> clock at 24MHz, period is calculated as 0x29. Previous >> snps,ref-clock-period-ns value used to be 0x32. >> Is that expected? > > Yes, it is 0x29 for IPQ60xx based SoCs. In downstream it was wrongly mentioned > as 0x32, which was corrected recently. Thanks for the update. This needs fixing in upstream kernel. I'll send a patch. For some reason USB appears to work here with both values. Is it because I only use USB2 signals? If this is the case them I can not actually test this series on my system. Thanks, baruch >>> [1] >>> https://lore.kernel.org/linux-usb/20220114044230.2677283-1-robert.hancock@calian.com/ >>> Changes in v2: >>> - Document clock members >>> - Also program GFLADJ.240MHZDECR >>> - Don't program GFLADJ if the version is < 2.50a >>> - Add snps,ref-clock-frequency-hz property for ACPI >>> Sean Anderson (7): >>> dt-bindings: usb: dwc3: Deprecate snps,ref-clock-period-ns >>> usb: dwc3: Get clocks individually >>> usb: dwc3: Calculate REFCLKPER based on reference clock >>> usb: dwc3: Program GFLADJ >>> usb: dwc3: Add snps,ref-clock-frequency-hz property for ACPI >>> arm64: dts: zynqmp: Move USB clocks to dwc3 node >>> arm64: dts: ipq6018: Use reference clock to set dwc3 period >>> .../devicetree/bindings/usb/snps,dwc3.yaml | 7 +- >>> arch/arm64/boot/dts/qcom/ipq6018.dtsi | 3 +- >>> .../arm64/boot/dts/xilinx/zynqmp-clk-ccf.dtsi | 4 +- >>> arch/arm64/boot/dts/xilinx/zynqmp.dtsi | 4 +- >>> drivers/usb/dwc3/core.c | 112 +++++++++++++++--- >>> drivers/usb/dwc3/core.h | 17 ++- >>> 6 files changed, 120 insertions(+), 27 deletions(-)
On Tue, 2022-01-18 at 19:24 -0500, Sean Anderson wrote: > These clocks are not used by the dwc3-xilinx driver except to > enable/disable them. Move them to the dwc3 node so its driver can use > them to configure the reference clock period. > > Signed-off-by: Sean Anderson <sean.anderson@seco.com> > --- > > (no changes since v1) > > arch/arm64/boot/dts/xilinx/zynqmp-clk-ccf.dtsi | 4 ++-- > arch/arm64/boot/dts/xilinx/zynqmp.dtsi | 4 ++-- > 2 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/arch/arm64/boot/dts/xilinx/zynqmp-clk-ccf.dtsi > b/arch/arm64/boot/dts/xilinx/zynqmp-clk-ccf.dtsi > index 1e0b1bca7c94..8493dd7d5f1f 100644 > --- a/arch/arm64/boot/dts/xilinx/zynqmp-clk-ccf.dtsi > +++ b/arch/arm64/boot/dts/xilinx/zynqmp-clk-ccf.dtsi > @@ -223,11 +223,11 @@ &uart1 { > clocks = <&zynqmp_clk UART1_REF>, <&zynqmp_clk LPD_LSBUS>; > }; > > -&usb0 { > +&dwc3_0 { > clocks = <&zynqmp_clk USB0_BUS_REF>, <&zynqmp_clk USB3_DUAL_REF>; > }; > > -&usb1 { > +&dwc3_1 { > clocks = <&zynqmp_clk USB1_BUS_REF>, <&zynqmp_clk USB3_DUAL_REF>; > }; > > diff --git a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi > b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi > index 74e66443e4ce..ba68fb8529ee 100644 > --- a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi > +++ b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi > @@ -811,7 +811,6 @@ usb0: usb@ff9d0000 { > status = "disabled"; > compatible = "xlnx,zynqmp-dwc3"; > reg = <0x0 0xff9d0000 0x0 0x100>; > - clock-names = "bus_clk", "ref_clk"; > power-domains = <&zynqmp_firmware PD_USB_0>; > resets = <&zynqmp_reset ZYNQMP_RESET_USB0_CORERESET>, > <&zynqmp_reset ZYNQMP_RESET_USB0_HIBERRESET>, > @@ -825,6 +824,7 @@ dwc3_0: usb@fe200000 { > interrupt-parent = <&gic>; > interrupt-names = "dwc_usb3", "otg"; > interrupts = <0 65 4>, <0 69 4>; > + clock-names = "bus_early", "ref"; > #stream-id-cells = <1>; > iommus = <&smmu 0x860>; > snps,quirk-frame-length-adjustment = <0x20>; > @@ -838,7 +838,6 @@ usb1: usb@ff9e0000 { > status = "disabled"; > compatible = "xlnx,zynqmp-dwc3"; > reg = <0x0 0xff9e0000 0x0 0x100>; > - clock-names = "bus_clk", "ref_clk"; > power-domains = <&zynqmp_firmware PD_USB_1>; > resets = <&zynqmp_reset ZYNQMP_RESET_USB1_CORERESET>, > <&zynqmp_reset ZYNQMP_RESET_USB1_HIBERRESET>, > @@ -852,6 +851,7 @@ dwc3_1: usb@fe300000 { > interrupt-parent = <&gic>; > interrupt-names = "dwc_usb3", "otg"; > interrupts = <0 70 4>, <0 74 4>; > + clock-names = "bus_early", "ref"; > #stream-id-cells = <1>; > iommus = <&smmu 0x861>; > snps,quirk-frame-length-adjustment = <0x20>; Tested and working on ZynqMP. Reviewed-by: Robert Hancock <robert.hancock@calian.com> Tested-by: Robert Hancock <robert.hancock@calian.com>
Hi Baruch, On 1/20/2022 3:59 PM, Baruch Siach wrote: > Hi Kathiravan, > > On Thu, Jan 20 2022, Kathiravan T wrote: >> On 2022-01-19 23:44, Baruch Siach wrote: >>> Hi Sean, >>> On Tue, Jan 18 2022, Sean Anderson wrote: >>>> This is a rework of patches 3-5 of [1]. It attempts to correctly program >>>> REFCLKPER and REFCLK_FLADJ based on the reference clock frequency. Since >>>> we no longer need a special property duplicating this configuration, >>>> snps,ref-clock-period-ns is deprecated. >>>> Please test this! Patches 3/4 in this series have the effect of >>>> programming REFCLKPER and REFCLK_FLADJ on boards which already configure >>>> the "ref" clock. I have build tested, but not much else. >>> Tested here on IPQ6010 based system. USB still works. But the with >>> "ref" >>> clock at 24MHz, period is calculated as 0x29. Previous >>> snps,ref-clock-period-ns value used to be 0x32. >>> Is that expected? >> Yes, it is 0x29 for IPQ60xx based SoCs. In downstream it was wrongly mentioned >> as 0x32, which was corrected recently. > Thanks for the update. This needs fixing in upstream kernel. I'll send a > patch. > > For some reason USB appears to work here with both values. Is it because > I only use USB2 signals? If this is the case them I can not actually > test this series on my system. I could recollect we did see some issue on USB2.0 port as well, but it wasn't fatal one. Anyways it is better to test it. Thanks, Kathiravan T. > > Thanks, > baruch > >>>> [1] >>>> https://lore.kernel.org/linux-usb/20220114044230.2677283-1-robert.hancock@calian.com/ >>>> Changes in v2: >>>> - Document clock members >>>> - Also program GFLADJ.240MHZDECR >>>> - Don't program GFLADJ if the version is < 2.50a >>>> - Add snps,ref-clock-frequency-hz property for ACPI >>>> Sean Anderson (7): >>>> dt-bindings: usb: dwc3: Deprecate snps,ref-clock-period-ns >>>> usb: dwc3: Get clocks individually >>>> usb: dwc3: Calculate REFCLKPER based on reference clock >>>> usb: dwc3: Program GFLADJ >>>> usb: dwc3: Add snps,ref-clock-frequency-hz property for ACPI >>>> arm64: dts: zynqmp: Move USB clocks to dwc3 node >>>> arm64: dts: ipq6018: Use reference clock to set dwc3 period >>>> .../devicetree/bindings/usb/snps,dwc3.yaml | 7 +- >>>> arch/arm64/boot/dts/qcom/ipq6018.dtsi | 3 +- >>>> .../arm64/boot/dts/xilinx/zynqmp-clk-ccf.dtsi | 4 +- >>>> arch/arm64/boot/dts/xilinx/zynqmp.dtsi | 4 +- >>>> drivers/usb/dwc3/core.c | 112 +++++++++++++++--- >>>> drivers/usb/dwc3/core.h | 17 ++- >>>> 6 files changed, 120 insertions(+), 27 deletions(-) >
Kathiravan Thirumoorthy wrote: > Hi Baruch, > > On 1/20/2022 3:59 PM, Baruch Siach wrote: >> Hi Kathiravan, >> >> On Thu, Jan 20 2022, Kathiravan T wrote: >>> On 2022-01-19 23:44, Baruch Siach wrote: >>>> Hi Sean, >>>> On Tue, Jan 18 2022, Sean Anderson wrote: >>>>> This is a rework of patches 3-5 of [1]. It attempts to correctly >>>>> program >>>>> REFCLKPER and REFCLK_FLADJ based on the reference clock frequency. >>>>> Since >>>>> we no longer need a special property duplicating this configuration, >>>>> snps,ref-clock-period-ns is deprecated. >>>>> Please test this! Patches 3/4 in this series have the effect of >>>>> programming REFCLKPER and REFCLK_FLADJ on boards which already >>>>> configure >>>>> the "ref" clock. I have build tested, but not much else. >>>> Tested here on IPQ6010 based system. USB still works. But the with >>>> "ref" >>>> clock at 24MHz, period is calculated as 0x29. Previous >>>> snps,ref-clock-period-ns value used to be 0x32. >>>> Is that expected? >>> Yes, it is 0x29 for IPQ60xx based SoCs. In downstream it was wrongly >>> mentioned >>> as 0x32, which was corrected recently. >> Thanks for the update. This needs fixing in upstream kernel. I'll send a >> patch. >> >> For some reason USB appears to work here with both values. Is it because >> I only use USB2 signals? If this is the case them I can not actually >> test this series on my system. The controller uses the GUCTL.REFCLKPER under specific conditions and settings, and the conditions are different between host mode and device mode. For example, if you're running as device-mode and not operating in low power, and or not using periodic endpoints, you're not probably testing this. BR, Thinh > > I could recollect we did see some issue on USB2.0 port as well, but it > wasn't fatal one. Anyways it is better to test it. > > Thanks, > > Kathiravan T. > >> >> Thanks, >> baruch >> >>>>> [1] >>>>> https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/20220114044230.2677283-1-robert.hancock@calian.com/__;!!A4F2R9G_pg!MCqqTGYTR42-4_LUHhrSJjkUOkDZKb795I8lB3PY4dB-kVCBnR9YKucgzrwhlj0tZZm5$ >>>>> Changes in v2: >>>>> - Document clock members >>>>> - Also program GFLADJ.240MHZDECR >>>>> - Don't program GFLADJ if the version is < 2.50a >>>>> - Add snps,ref-clock-frequency-hz property for ACPI >>>>> Sean Anderson (7): >>>>> dt-bindings: usb: dwc3: Deprecate snps,ref-clock-period-ns >>>>> usb: dwc3: Get clocks individually >>>>> usb: dwc3: Calculate REFCLKPER based on reference clock >>>>> usb: dwc3: Program GFLADJ >>>>> usb: dwc3: Add snps,ref-clock-frequency-hz property for ACPI >>>>> arm64: dts: zynqmp: Move USB clocks to dwc3 node >>>>> arm64: dts: ipq6018: Use reference clock to set dwc3 period >>>>> .../devicetree/bindings/usb/snps,dwc3.yaml | 7 +- >>>>> arch/arm64/boot/dts/qcom/ipq6018.dtsi | 3 +- >>>>> .../arm64/boot/dts/xilinx/zynqmp-clk-ccf.dtsi | 4 +- >>>>> arch/arm64/boot/dts/xilinx/zynqmp.dtsi | 4 +- >>>>> drivers/usb/dwc3/core.c | 112 >>>>> +++++++++++++++--- >>>>> drivers/usb/dwc3/core.h | 17 ++- >>>>> 6 files changed, 120 insertions(+), 27 deletions(-) >>