| Message ID | 20220119002438.106079-1-sean.anderson@seco.com (mailing list archive) |
|---|---|
| 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(-)
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(-) >>