lkml.org 
[lkml]   [2022]   [Jan]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [PATCH 0/6] usb: dwc3: Calculate REFCLKPER et. al. from reference clock
    Date
    On Mon, 2022-01-17 at 20:30 +0800, Jun Li wrote:
    > Sean Anderson <sean.anderson@seco.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
    \
     
     \ /
      Last update: 2022-01-18 00:50    [W:3.713 / U:0.012 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site