[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <17731a13.1cce.19974dfc64d.Coremail.caohang@eswincomputing.com>
Date: Tue, 23 Sep 2025 12:40:46 +0800 (GMT+08:00)
From: "Hang Cao" <caohang@...incomputing.com>
To: "Conor Dooley" <conor@...nel.org>
Cc: gregkh@...uxfoundation.org, robh@...nel.org, krzk+dt@...nel.org,
conor+dt@...nel.org, Thinh.Nguyen@...opsys.com,
p.zabel@...gutronix.de, linux-kernel@...r.kernel.org,
linux-usb@...r.kernel.org, devicetree@...r.kernel.org,
ningyu@...incomputing.com, linmin@...incomputing.com,
pinkesh.vaghela@...fochips.com,
"Senchuan Zhang" <zhangsenchuan@...incomputing.com>
Subject: Re: Re: [PATCH v3 1/2] dt-bindings: usb: Add ESWIN EIC7700 USB
controller
Hi, Conor Dooley
First of all, thank you very much for your detailed and professional review work. Additionally, we sincerely apologize
for any confusion caused to you.I will try to explain the content of the design framework.
> > From: Hang Cao <caohang@...incomputing.com>
> >
> > Add Device Tree binding documentation for the ESWIN EIC7700
> > usb controller module.
> >
> > Signed-off-by: Senchuan Zhang <zhangsenchuan@...incomputing.com>
> > Signed-off-by: Hang Cao <caohang@...incomputing.com>
> > ---
> > .../bindings/usb/eswin,eic7700-usb.yaml | 99 +++++++++++++++++++
> > 1 file changed, 99 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/usb/eswin,eic7700-usb.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/usb/eswin,eic7700-usb.yaml b/Documentation/devicetree/bindings/usb/eswin,eic7700-usb.yaml
> > new file mode 100644
> > index 000000000000..37797b85f417
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/usb/eswin,eic7700-usb.yaml
> > @@ -0,0 +1,99 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/usb/eswin,eic7700-usb.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: ESWIN EIC7700 SoC Usb Controller
> > +
> > +maintainers:
> > + - Wei Yang <yangwei1@...incomputing.com>
> > + - Senchuan Zhang <zhangsenchuan@...incomputing.com>
> > + - Hang Cao <caohang@...incomputing.com>
> > +
> > +description:
> > + The Usb controller on EIC7700 SoC.
> > +
> > +allOf:
> > + - $ref: snps,dwc3-common.yaml#
> > +
> > +properties:
> > + compatible:
> > + const: eswin,eic7700-dwc3
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + interrupts:
> > + maxItems: 1
> > +
> > + interrupt-names:
> > + items:
> > + - const: peripheral
> > +
> > + clocks:
> > + maxItems: 2
> > +
> > + clock-names:
> > + items:
> > + - const: aclk
> > + - const: cfg
> > +
> > + resets:
> > + maxItems: 1
> > +
> > + reset-names:
> > + items:
> > + - const: vaux
> > +
> > + eswin,hsp-sp-csr:
> > + description:
> > + HSP CSR is to control and get status of different high-speed peripherals
> > + (such as Ethernet, USB, SATA, etc.) via register, which can close
> > + module's clock,reset module independently and tune board-level's
> > + parameters of PHY, etc.
> > + $ref: /schemas/types.yaml#/definitions/phandle-array
> > + items:
> > + - items:
> > + - description: phandle to HSP Register Controller hsp_sp_csr node.
> > + - description: usb bus register offset.
> > + - description: axi low power register offset.
> > + - description: vbus frequency register offset.
> > + - description: mpll register offset.
>
> As I mentioned on the shdci binding patch, I'm not happy with the
> justification for this phandle. What exactly is the clock that this
> controls and why does it not have a dedicated clock-controller driver
> and reset-controller driver?
>
In the current design framework, the clock can be divided into two parts:
1. The top-clock, which is used to manage and control the clocks of various subsystems (such as HSP, GPU, NPU, etc.);
2. The subsystem clocks managed independently by each subsystem.
The top-clock is a standard clock design(featuring gate, divider, and mux functions) that has been registered in the
common clock framework,with a dedicated clock controller driver.
The subsystem clocks managed by subsystems are controlled and configured through the CSR (Control and Status Register)
of each respective subsystem. For example, the HSP subsystem uses the eswin,hsp-sp-csr. Additionally, this CSR is
responsible for managing startup functions, performing independent reset of specific modules, and adjusting
PHY parameters to achieve board-level tuning (for USB/SATA interfaces, etc.).
The top-clock manages the global clocks of subsystems. Taking the HSP subsystem as an example, the top-clock
configures the hsp_aclk_ctrl and hsp_cfg_ctrl of HSP subsystem only.
In contrast, the subsystem clocks are managed via their own CSRs. For instance, the USB ref clock used in the USB module of
the HSP subsystem can only be configured through the hsp-csr, and cannot be set via the top-clock controller driver.
As for the reset function, it is not integrated into a dedicated controller driver either, for reasons similar to those of the
clock management mentioned above.
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - clocks
> > + - clock-names
> > + - interrupts
> > + - interrupt-names
> > + - resets
> > + - reset-names
> > + - eswin,hsp-sp-csr
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > + - |
> > + usb@...80000 {
> > + compatible = "eswin,eic7700-dwc3";
> > + reg = <0x50480000 0x10000>;
> > + clocks = <&clock 170>,
> > + <&clock 171>;
> > + clock-names = "aclk", "cfg";
> > + interrupt-parent = <&plic>;
> > + interrupts = <85>;
> > + interrupt-names = "peripheral";
> > + resets = <&reset 84>;
> > + reset-names = "vaux";
> > + dr_mode = "peripheral";
> > + maximum-speed = "high-speed";
> > + phy_type = "utmi";
> > + snps,dis_enblslpm_quirk;
> > + snps,dis-u2-freeclk-exists-quirk;
> > + snps,dis_u2_susphy_quirk;
> > + snps,dis-del-phy-power-chg-quirk;
> > + snps,parkmode-disable-ss-quirk;
> > + eswin,hsp-sp-csr = <&hsp_sp_csr 0x800 0x818 0x83c 0x840>;
> > + };
> > --
> > 2.34.1
> >
I wonder if this explanation has addressed your doubts. If you have any other questions, please feel free to further communicate
with us. Thank you a lot.
Best regards,
Hang Cao
Download attachment "signature.asc" of type "application/pgp-signature" (235 bytes)
Powered by blists - more mailing lists