linux-kernel - Re: [PATCH 0/6] usb: dwc3: Calculate REFCLKPER et. al. from reference clock

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <79d6057a70dae09bcc30a35f608f51c5a768d097.camel@calian.com>
Date:   Mon, 17 Jan 2022 23:49:35 +0000
From:   Robert Hancock <robert.hancock@...ian.com>
To:     "sean.anderson@...o.com" <sean.anderson@...o.com>,
        "lijun.kernel@...il.com" <lijun.kernel@...il.com>,
        "jun.li@....com" <jun.li@....com>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "bjagadee@...eaurora.org" <bjagadee@...eaurora.org>,
        "linux-arm-msm@...r.kernel.org" <linux-arm-msm@...r.kernel.org>,
        "michal.simek@...inx.com" <michal.simek@...inx.com>,
        "Thinh.Nguyen@...opsys.com" <Thinh.Nguyen@...opsys.com>,
        "baruch@...s.co.il" <baruch@...s.co.il>,
        "balbi@...nel.org" <balbi@...nel.org>,
        "bjorn.andersson@...aro.org" <bjorn.andersson@...aro.org>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "agross@...nel.org" <agross@...nel.org>
Subject: Re: [PATCH 0/6] usb: dwc3: Calculate REFCLKPER et. al. from reference
 clock

On Mon, 2022-01-17 at 20:30 +0800, Jun Li wrote:
> Sean Anderson <sean.anderson@...o.com> 于2022年1月15日周六 10:11写道：
> > 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.
> 
> DWC3 databook states a *condition* for program those settings:
> 
> This field must be programmed to a non-zero value only if
> GFLADJ_REFCLK_LPM_SEL is set to '1' or GCTL.SOFITPSYNC is set to '1'.
> The value is derived as follows:
> FLADJ_REF_CLK_FLADJ=((125000/ref_clk_period_integer)-(125000/ref_clk_period))
> * ref_clk_period where
> ■ the ref_clk_period_integer is the integer value of the ref_clk
> period got by truncating the decimal (fractional) value that is
> programmed in the GUCTL.REF_CLK_PERIOD field.
> ■ the ref_clk_period is the ref_clk period including the fractional value.
> 
> So you may need a condition check, with that, only required users
> are effected even with "ref" clock specified.
> 

The Xilinx register documentation for this register in the DWC3 core ( 
https://www.xilinx.com/html_docs/registers/ug1087/usb3_xhci___gfladj.html ) has
the same description, but it is rather confusingly worded. I suspect what they
really mean is that "this field only needs to be programmed if
GFLADJ_REFCLK_LPM_SEL is set to '1' or GCTL.SOFITPSYNC is set to '1'", not
"this field should only be programmed if GFLADJ_REFCLK_LPM_SEL is set to '1' or
GCTL.SOFITPSYNC is set to '1'". I'm not sure if someone can confirm that
interpretation is correct?

However, looking at that description a bit further, it looks like there are
some other fields in that register which are dependent on the reference clock:
GFLADJ_REFCLK_240MHZ_DECR (bits 30:24) and GFLADJ_REFCLK_240MHZDECR_PLS1 (bit
31). It looks like the Xilinx board I am using has those set properly, i.e. to
12 and 0 respectively (I'm guessing by hardware default, since I don't see
anything in the FSBL psu_init code setting those), but it wouldn't hurt to
ensure those fields are also set correctly.

> Li Jun
> 
> > [1] 
> > https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/20220114044230.2677283-1-robert.hancock@calian.com/__;!!IOGos0k!387DmPelOIA5Z6ZSfzSF2spWPu3lARlrkdmIRGcPwlWDZGDQzdlEdEKBw1RWG8aRC38$
> >  
> > 
> > 
> > Sean Anderson (6):
> >   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: Handle fractional reference clocks
> >   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                       | 98 ++++++++++++++++---
> >  drivers/usb/dwc3/core.h                       |  6 +-
> >  6 files changed, 98 insertions(+), 24 deletions(-)
> > 
> > --
> > 2.25.1
> > 
-- 
Robert Hancock
Senior Hardware Designer, Calian Advanced Technologies
www.calian.com