Re: [PATCH] usb: dwc3: reference clock configuration

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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





[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux