Hi Kathiravan, Baruch,

On Thu, Feb 25, 2021 at 10:17:49PM +0530, Kathiravan T wrote:

On 2021-02-15 22:28, Kathiravan T wrote:

quoted

On 2021-02-10 11:40, Baruch Siach wrote:

quoted

Hi Bjorn,

Thanks for your review comments.

On Mon, Feb 08 2021, Bjorn Andersson wrote:

quoted

On Mon 08 Feb 00:00 CST 2021, Baruch Siach wrote:

quoted

From: Balaji Prakash J <redacted>

DWC_USB3_GFLADJ and DWC_USB3_GUCTL registers contain options
to control the behavior of controller with respect to SOF and ITP.
The reset values of these registers are aligned for 19.2 MHz
reference clock source. This change will add option to override
these settings for reference clock other than 19.2 MHz

Tested on IPQ6018 SoC based CP01 board with 24MHz reference clock.

Signed-off-by: Balaji Prakash J <redacted>
[ baruch: mention tested hardware ]
Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
 .../devicetree/bindings/usb/dwc3.txt          |  5 ++
 drivers/usb/dwc3/core.c                       | 52
+++++++++++++++++++
 drivers/usb/dwc3/core.h                       | 12 +++++
 3 files changed, 69 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt

b/Documentation/devicetree/bindings/usb/dwc3.txt
index 1aae2b6160c1..4ffa87b697dc 100644

--- a/Documentation/devicetree/bindings/usb/dwc3.txt
+++ b/Documentation/devicetree/bindings/usb/dwc3.txt

@@ -89,6 +89,11 @@ Optional properties:
  - snps,quirk-frame-length-adjustment: Value for

GFLADJ_30MHZ field of GFLADJ
 	register for post-silicon frame length adjustment when the
 	fladj_30mhz_sdbnd signal is invalid or incorrect.
+ - snps,quirk-ref-clock-adjustment: Value for
GFLADJ_REFCLK_* fields of GFLADJ
+	register for reference clock other than 19.2 MHz is used.

What are typical values for this property? What unit does it
have? How
does it actually relate to the frequency of the reference clock?

Downstream codeaurora kernel (fig branch) sets 0xA87F0 for IPQ6018
(24MHz reference clock), and 0x49459 for IPQ5018 (60MHz). So this
value
appears to correlates with clock rate. I have no access to DWC3
documentation. I only tested IPQ6018 hardware.

It will be written as (0xA87F0 << 7) retaining the 0-7 LSB value.
I could see, BIT(23) of GFLADJ register enables the functionality of
running SOF/ITP counters based on the reference clock. Since this bit
is set, we need to
compute the other fields as well i.e., from 8th bit to 31st bit.
Finally it is derived to
0xA87F0 for IPQ6018.

Bjorn / All,

Any comments on this? Please do suggest if this can be handled in a better
way.

quoted

quoted

quoted

quoted

+ - snps,quirk-ref-clock-period: Value for REFCLKPER filed
of GUCTL. This field
+	indicates in terms of nano seconds the period of ref_clk.
To calculate the
+	ideal value, REFCLKPER = (1/ref_clk in Hz)*10^9.

Can't we make the dwc3 reference this clock and use
clk_get_rate() and
then do this math in the driver?

This is doable, I believe. Though current code does not identify
specific clocks, as far as I can see.

I agree it should be doable. Looks like prior to 0d3a97083e0c ("usb:
dwc3: Rework clock initialization to be more flexible") the core did
support specific clocks ("ref", "bus_early", "suspend"), but was
changed to use a simpler devm_clk_bulk_get_all() call.

quoted

We can mention one more clock(ref) in the USB device node and do the
math (NSEC_PER_SEC / clk_get_rate()) in dwc3 driver.

Yea, just need to make sure "ref" clk is specified in the DT node. Then
in the driver you can just iterate through dwc->clks and try to find one
with .id=="ref". If clk_get_rate() succeeds then you can use the value
to calculate the GUCTL.REFCLKPER and GFLADJ register fields.

Or perhaps even use a lookup table, since according to the DWC3
programming guide only 6 refclk frequencies (16, 17, 19.2, 20, 24, 39.7
MHz) are supported so that might be simpler than a few integer divide
operations that would otherwise be required.

Jack
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project