Bjorn Andersson wrote: > On Wed 07 Apr 21:50 CDT 2021, Thinh Nguyen wrote: > >> Bjorn Andersson wrote: >>> On Wed 07 Apr 20:53 CDT 2021, Thinh Nguyen wrote: >>> >>>> Kathiravan T wrote: >>>>> On 2021-03-31 06:47, Thinh Nguyen wrote: >>>>>> Baruch Siach wrote: >>>>>>> From: Balaji Prakash J <bjagadee@xxxxxxxxxxxxxx> >>>>>>> >>>>>>> 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 <bjagadee@xxxxxxxxxxxxxx> >>>>>>> [ baruch: mention tested hardware ] >>>>>>> Signed-off-by: Baruch Siach <baruch@xxxxxxxxxx> >>>>>>> --- >>>>>>> .../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. >>>>>>> + - 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. >>>>>> >>>>>> Why is this a quirk? It's not a quirk. The user can inform the ref_clk >>>>>> period to the controller here. >>>>>> >>>>>> The default value from GUCTL.REFCLKPER is a value from coreConsultant >>>>>> setting. The designer knows what period it should be and should properly >>>>>> set it if the default is not correctly set. >>>>> >>>>> Thanks Thinh for your inputs. Can we have the DT property for both the >>>>> GUCTL.REFCLKPER and GFLADJ.REFCLK* fields? >>>>> Since GFLADJ.REFCLK* field values derived based on the GUCTL.REFCLKPER. >>>>> In other words, are you fine with the >>>>> approach followed here? If so, we can work on the nitpicks and send the V2. >>>>> >>>>> Please let us know your thoughts on this. >>>>> >>>> >>>> Hi Kathiravan, >>>> >>>> Yes, IMO, using DT properties work just fine to inform the controller if >>>> the default settings don't match the HW configuration. >>> >>> I'm not against using a separate DT property if the information it >>> provides can't be derived from what's already there. >> >> That's the issue. That information is not available if dwc3 is using PCI >> bus. >> >>> >>>> As I mention in >>>> the separate email thread, using clk_get_rate() doesn't make sense for >>>> PCI devices. >>>> >>> >>> I'm sorry, can you help me understand why this relate to PCI? >> >> It's because the clock's attributes are not exposed if we're using the >> PCI bus. The driver cannot access this information unless the user >> provides it in some other way. >> > > So we have a DWC3 controller on a PCI bus, somehow described in DT, but > the refclock is derived from something that's not described as a clock > in the OS? > It's not described in DT. We create a platform device in the PCI glue driver and pass in specific properties as if it's a platform device. >From the PCI function, we have no clue of the refclk. It may be better if the DWC3 driver can initialize and drive the PCI function without converting it to a platform device. However, I don't see this will change any time soon. BR, Thinh
