On 27/09/2024 22:51, Drew Fustini wrote: > On Fri, Sep 27, 2024 at 11:34:48AM +0200, Krzysztof Kozlowski wrote: >> On Thu, Sep 26, 2024 at 11:15:50AM -0700, Drew Fustini wrote: >>> From: Jisheng Zhang <jszhang@xxxxxxxxxx> >>> >>> Add documentation to describe T-HEAD dwmac. >>> >>> Signed-off-by: Jisheng Zhang <jszhang@xxxxxxxxxx> >>> Signed-off-by: Emil Renner Berthing <emil.renner.berthing@xxxxxxxxxxxxx> >>> [drew: change apb registers from syscon to second reg of gmac node] >>> [drew: rename compatible, add thead rx/tx internal delay properties] >>> Signed-off-by: Drew Fustini <dfustini@xxxxxxxxxxxxxxx> >>> --- >>> .../devicetree/bindings/net/snps,dwmac.yaml | 1 + >>> .../devicetree/bindings/net/thead,th1520-gmac.yaml | 109 +++++++++++++++++++++ >>> MAINTAINERS | 1 + >>> 3 files changed, 111 insertions(+) >>> >>> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml >>> index 4e2ba1bf788c..474ade185033 100644 >>> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml >>> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml >>> @@ -99,6 +99,7 @@ properties: >>> - snps,dwxgmac-2.10 >>> - starfive,jh7100-dwmac >>> - starfive,jh7110-dwmac >>> + - thead,th1520-gmac >>> >>> reg: >>> minItems: 1 >>> diff --git a/Documentation/devicetree/bindings/net/thead,th1520-gmac.yaml b/Documentation/devicetree/bindings/net/thead,th1520-gmac.yaml >>> new file mode 100644 >>> index 000000000000..1070e891c025 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/net/thead,th1520-gmac.yaml >>> @@ -0,0 +1,109 @@ >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/net/thead,th1520-gmac.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: T-HEAD TH1520 GMAC Ethernet controller >>> + >>> +maintainers: >>> + - Drew Fustini <dfustini@xxxxxxxxxxxxxxx> >>> + >>> +description: | >>> + The TH1520 GMAC is described in the TH1520 Peripheral Interface User Manual >>> + https://git.beagleboard.org/beaglev-ahead/beaglev-ahead/-/tree/main/docs >>> + >>> + Features include >>> + - Compliant with IEEE802.3 Specification >>> + - IEEE 1588-2008 standard for precision networked clock synchronization >>> + - Supports 10/100/1000Mbps data transfer rate >>> + - Supports RGMII/MII interface >>> + - Preamble and start of frame data (SFD) insertion in Transmit path >>> + - Preamble and SFD deletion in the Receive path >>> + - Automatic CRC and pad generation options for receive frames >>> + - MDIO master interface for PHY device configuration and management >>> + >>> + The GMAC Registers consists of two parts >>> + - APB registers are used to configure clock frequency/clock enable/clock >>> + direction/PHY interface type. >>> + - AHB registers are use to configure GMAC core (DesignWare Core part). >>> + GMAC core register consists of DMA registers and GMAC registers. >>> + >>> +select: >>> + properties: >>> + compatible: >>> + contains: >>> + enum: >>> + - thead,th1520-gmac >>> + required: >>> + - compatible >>> + >>> +allOf: >>> + - $ref: snps,dwmac.yaml# >>> + >>> +properties: >>> + compatible: >>> + items: >>> + - enum: >>> + - thead,th1520-gmac >>> + - const: snps,dwmac-3.70a >>> + >>> + reg: >>> + items: >>> + - description: DesignWare GMAC IP core registers >>> + - description: GMAC APB registers >>> + >>> + reg-names: >>> + items: >>> + - const: dwmac >>> + - const: apb >>> + >>> + thead,rx-internal-delay: >>> + $ref: /schemas/types.yaml#/definitions/uint32 >>> + description: | >>> + RGMII receive clock delay. The value is used for the delay_ctrl >>> + field in GMAC_RXCLK_DELAY_CTRL. Units are not specified. >> >> What do you mean by "unspecified units"? They are always specified, >> hardware does not work randomly, e.g. once uses clock cycles, but next >> time you run it will use picoseconds. >> >> You also miss default (property is not required) and some sort of constraints. > > I should have stated that I don't know the units for delay_ctrl. The > 5-bit field has a max value of 31 which seems far too small for > picoseconds. Unfortunately, the documentation from the SoC vendor does > not give anymore details about what the value represents. > > Andrew Lunn replied [1] to my cover letter that it is best to hard code > the field to 0 (which is the hardware reset value) if I don't know what > the units are for delay_ctrl. The hardware that I have works okay with > delay_ctrl of 0, so it seems these new vendor properties are not needed. Then just say that this is using register values directly. Best regards, Krzysztof
