linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
To: Kathiravan T <kathirav@codeaurora.org>,
	Thinh Nguyen <Thinh.Nguyen@synopsys.com>
Cc: Baruch Siach <baruch@tkos.co.il>, Felipe Balbi <balbi@kernel.org>,
	"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
	Andy Gross <agross@kernel.org>,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	"linux-arm-msm@vger.kernel.org" <linux-arm-msm@vger.kernel.org>,
	Balaji Prakash J <bjagadee@codeaurora.org>
Subject: Re: [PATCH] usb: dwc3: reference clock configuration
Date: Thu, 8 Apr 2021 01:53:21 +0000	[thread overview]
Message-ID: <4e5a3487-bae2-b21a-df90-80b5f0d170ba@synopsys.com> (raw)
In-Reply-To: <8cc9617dc62be17ac3a9420e7750b0ee@codeaurora.org>

Kathiravan T wrote:
> On 2021-03-31 06:47, Thinh Nguyen wrote:
>> Baruch Siach wrote:
>>> From: Balaji Prakash J <bjagadee@codeaurora.org>
>>>
>>> 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@codeaurora.org>
>>> [ 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.
>>> + - 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. As I mention in
the separate email thread, using clk_get_rate() doesn't make sense for
PCI devices.

The snps,quirk-ref-clock-adjustment property updates multiple fields of
the GFLADJ and not just GFLADJ_REFCLK_FLADJ. I'd suggest to split them
to different properties for different fields for clarity. If the other
fields of GFLADJ.REFCLK_* are derived based on the GUCTL.REFCLKPER
according to the example of the programming guide, then do that
calculation in the driver as default. However, I'd suggest to create a
separate property (maybe "snps,use-refclk-for-sof-itp"?) to select
GFLADJ.GFLADJ_REFCLK_LPM_SEL or GCTL.SOFITPSYNC depending whether the
controller is operating as host or device mode. Note that this feature
is only available for DWC_usb32 and DWC_usb31 v1.80a or higher. I need
to double check for DWC_usb3 IP, but I believe it's v3.30a or higher.

Btw, we don't need to mention 19.2 MHz since it's the specific default
configuration of your setup. Other setups may not be the same.

BR,
Thinh

  reply	other threads:[~2021-04-08  1:53 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-08  6:00 [PATCH] usb: dwc3: reference clock configuration Baruch Siach
2021-02-08  6:56 ` Greg KH
2021-02-08 18:26 ` Bjorn Andersson
2021-02-10  6:10   ` Baruch Siach
2021-02-15 16:58     ` Kathiravan T
2021-02-25 16:47       ` Kathiravan T
2021-03-24  8:14         ` Jack Pham
2021-03-31  0:52           ` Thinh Nguyen
2021-03-31  1:17 ` Thinh Nguyen
2021-04-07 11:56   ` Kathiravan T
2021-04-08  1:53     ` Thinh Nguyen [this message]
2021-04-08  2:23       ` Bjorn Andersson
2021-04-08  2:50         ` Thinh Nguyen
2021-04-08  2:54           ` Thinh Nguyen
2021-04-08  3:15           ` Bjorn Andersson
2021-04-08  3:39             ` Thinh Nguyen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4e5a3487-bae2-b21a-df90-80b5f0d170ba@synopsys.com \
    --to=thinh.nguyen@synopsys.com \
    --cc=agross@kernel.org \
    --cc=balbi@kernel.org \
    --cc=baruch@tkos.co.il \
    --cc=bjagadee@codeaurora.org \
    --cc=bjorn.andersson@linaro.org \
    --cc=kathirav@codeaurora.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).