From: Rob Herring <robh@kernel.org>
To: Lucas Tanure <tanure@linux.com>
Cc: xianwei.zhao@amlogic.com, Andrew Lunn <andrew+netdev@lunn.ch>,
davem@davemloft.net, Eric Dumazet <edumazet@kernel.org>,
Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
Krzysztof Kozlowski <krzk+dt@kernel.org>,
Conor Dooley <conor+dt@kernel.org>,
Neil Armstrong <neil.armstrong@linaro.org>,
Kevin Hilman <khilman@baylibre.com>,
Jerome Brunet <jbrunet@baylibre.com>,
Martin Blumenstingl <martin.blumenstingl@googlemail.com>,
Maxime Chevallier <maxime.chevallier@bootlin.com>,
Maxime Coquelin <mcoquelin.stm32@gmail.com>,
Alexandre Torgue <alexandre.torgue@foss.st.com>,
netdev@vger.kernel.org, devicetree@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-amlogic@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v6 2/7] dt-bindings: net: amlogic,meson-dwmac: add amlogic,t7-dwmac
Date: Fri, 13 Nov 2026 08:23:29 -0600 [thread overview]
Message-ID: <20261113142329.GA593855-robh@kernel.org> (raw)
In-Reply-To: <CAJX_Q+2WoawiGT9zF2W+LCUhVn2QV3F9rUDBHiGtR6pddKAkNA@mail.gmail.com>
On Fri, Nov 13, 2026 at 12:07:55PM +0000, Lucas Tanure wrote:
> On Tue, Nov 10, 2026 at 5:54 PM Rob Herring <robh@kernel.org> wrote:
> >
> > On Thu, Nov 05, 2026 at 10:03:17AM +0000, Lucas Tanure wrote:
> > > The T7 has the same ethernet glue as the G12A but needs two clocks of
> > > its own. The controller reaches memory through a pipeline stage with
> > > its own gate; nothing else claims it, so it is switched off as unused
> > > and transfers never finish. The glue registers sit in a block clocked
> > > together with the MDIO multiplexer, which probes after the controller
> > > has programmed them: the writes are lost and the port keeps the
> > > bootloader's delays.
> > >
> > > So the T7 gets its own clock list, without the timing adjustment clock
> > > it never uses. Its delays follow ethernet-controller.yaml: the PHY adds
> > > the 2ns, and only the receive delay line offers fine tuning, so only
> > > rx-internal-delay-ps is valid here.
> > >
> > > Assisted-by: LLM
> > > Signed-off-by: Lucas Tanure <tanure@linux.com>
> > > ---
> > > .../bindings/net/amlogic,meson-dwmac.yaml | 49 +++++++++++++++++++
> > > 1 file changed, 49 insertions(+)
> >
> > I tend to agree with the sashiko comment, but am fine either way. Once
> > the if/then schemas are longer than the main schema, it's a sign to
> > split.
>
> In V7 I will split as I think it will be easier to review.
>
> >
> > >
> > > diff --git a/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml b/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
> > > index e769c9bb70d8..ff5e50f65114 100644
> > > --- a/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
> > > +++ b/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
> > > @@ -23,6 +23,7 @@ select:
> > > - amlogic,meson-gxbb-dwmac
> > > - amlogic,meson-axg-dwmac
> > > - amlogic,meson-g12a-dwmac
> > > + - amlogic,t7-dwmac
> > > required:
> > > - compatible
> > >
> > > @@ -84,6 +85,49 @@ allOf:
> > > rx-internal-delay-ps:
> > > default: 0
> > >
> > > + - if:
> > > + properties:
> > > + compatible:
> > > + contains:
> > > + const: amlogic,t7-dwmac
> > > + then:
> > > + properties:
> > > + clocks:
> > > + items:
> > > + - description: GMAC main clock
> > > + - description: First parent clock of the internal mux
> > > + - description: Second parent clock of the internal mux
> > > + - description:
> > > + clock of the pipeline stage inserted in the bus path between
> > > + the controller and the DRAM. Without it, the controller cannot
> > > + complete DMA transfers.
> > > + - description:
> > > + clock of the register block holding PRG_ETH0 and PRG_ETH1.
> > > + Without it, the block reads as zero and ignores writes.
> > > +
> > > + clock-names:
> > > + items:
> > > + - const: stmmaceth
> > > + - const: clkin0
> > > + - const: clkin1
> > > + - const: pipeline
> > > + - const: pclk
> > > +
> > > + # The PHY adds the 2ns RGMII delays, see ethernet-controller.yaml.
> > > + # The RX delay line of this glue is fine tuning in 200ps steps,
> >
> > Sounds like constraints:
> >
> > maximum: 2000
> > multipleOf: 200
>
> Yes, with maximum: 3000: the field is four bits, 0 to 15 steps of
> 200ps.
> >
> > > + # added on top of what phy-mode describes. The TX side has no fine
> > > + # tuning: its only step is a quarter of the clock period, the full
> > > + # 2ns at 1Gbit/s, so the TX property is not accepted.
> >
> > Move this to tx-internal-delay-ps. You can (kind of ackwardly) do:
> >
> > tx-internal-delay-ps:
> > description: ...
> > not: {}
>
> dt_binding_check does not accept "not" on a property under
> properties: or in an allOf entry: cell.yaml allows only description,
> deprecated, const, enum, minimum, maximum, multipleOf, default, $ref
> and oneOf there. It passes only inside if/then, and the new file has
> no if/then. So the property stays false, with the explanation as a
> comment above it.
Okay, but we should fix that.
Rob
WARNING: multiple messages have this Message-ID (diff)
From: Rob Herring <robh@kernel.org>
To: Lucas Tanure <tanure@linux.com>
Cc: xianwei.zhao@amlogic.com, Andrew Lunn <andrew+netdev@lunn.ch>,
davem@davemloft.net, Eric Dumazet <edumazet@kernel.org>,
Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
Krzysztof Kozlowski <krzk+dt@kernel.org>,
Conor Dooley <conor+dt@kernel.org>,
Neil Armstrong <neil.armstrong@linaro.org>,
Kevin Hilman <khilman@baylibre.com>,
Jerome Brunet <jbrunet@baylibre.com>,
Martin Blumenstingl <martin.blumenstingl@googlemail.com>,
Maxime Chevallier <maxime.chevallier@bootlin.com>,
Maxime Coquelin <mcoquelin.stm32@gmail.com>,
Alexandre Torgue <alexandre.torgue@foss.st.com>,
netdev@vger.kernel.org, devicetree@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-amlogic@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v6 2/7] dt-bindings: net: amlogic,meson-dwmac: add amlogic,t7-dwmac
Date: Fri, 13 Nov 2026 08:23:29 -0600 [thread overview]
Message-ID: <20261113142329.GA593855-robh@kernel.org> (raw)
In-Reply-To: <CAJX_Q+2WoawiGT9zF2W+LCUhVn2QV3F9rUDBHiGtR6pddKAkNA@mail.gmail.com>
On Fri, Nov 13, 2026 at 12:07:55PM +0000, Lucas Tanure wrote:
> On Tue, Nov 10, 2026 at 5:54 PM Rob Herring <robh@kernel.org> wrote:
> >
> > On Thu, Nov 05, 2026 at 10:03:17AM +0000, Lucas Tanure wrote:
> > > The T7 has the same ethernet glue as the G12A but needs two clocks of
> > > its own. The controller reaches memory through a pipeline stage with
> > > its own gate; nothing else claims it, so it is switched off as unused
> > > and transfers never finish. The glue registers sit in a block clocked
> > > together with the MDIO multiplexer, which probes after the controller
> > > has programmed them: the writes are lost and the port keeps the
> > > bootloader's delays.
> > >
> > > So the T7 gets its own clock list, without the timing adjustment clock
> > > it never uses. Its delays follow ethernet-controller.yaml: the PHY adds
> > > the 2ns, and only the receive delay line offers fine tuning, so only
> > > rx-internal-delay-ps is valid here.
> > >
> > > Assisted-by: LLM
> > > Signed-off-by: Lucas Tanure <tanure@linux.com>
> > > ---
> > > .../bindings/net/amlogic,meson-dwmac.yaml | 49 +++++++++++++++++++
> > > 1 file changed, 49 insertions(+)
> >
> > I tend to agree with the sashiko comment, but am fine either way. Once
> > the if/then schemas are longer than the main schema, it's a sign to
> > split.
>
> In V7 I will split as I think it will be easier to review.
>
> >
> > >
> > > diff --git a/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml b/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
> > > index e769c9bb70d8..ff5e50f65114 100644
> > > --- a/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
> > > +++ b/Documentation/devicetree/bindings/net/amlogic,meson-dwmac.yaml
> > > @@ -23,6 +23,7 @@ select:
> > > - amlogic,meson-gxbb-dwmac
> > > - amlogic,meson-axg-dwmac
> > > - amlogic,meson-g12a-dwmac
> > > + - amlogic,t7-dwmac
> > > required:
> > > - compatible
> > >
> > > @@ -84,6 +85,49 @@ allOf:
> > > rx-internal-delay-ps:
> > > default: 0
> > >
> > > + - if:
> > > + properties:
> > > + compatible:
> > > + contains:
> > > + const: amlogic,t7-dwmac
> > > + then:
> > > + properties:
> > > + clocks:
> > > + items:
> > > + - description: GMAC main clock
> > > + - description: First parent clock of the internal mux
> > > + - description: Second parent clock of the internal mux
> > > + - description:
> > > + clock of the pipeline stage inserted in the bus path between
> > > + the controller and the DRAM. Without it, the controller cannot
> > > + complete DMA transfers.
> > > + - description:
> > > + clock of the register block holding PRG_ETH0 and PRG_ETH1.
> > > + Without it, the block reads as zero and ignores writes.
> > > +
> > > + clock-names:
> > > + items:
> > > + - const: stmmaceth
> > > + - const: clkin0
> > > + - const: clkin1
> > > + - const: pipeline
> > > + - const: pclk
> > > +
> > > + # The PHY adds the 2ns RGMII delays, see ethernet-controller.yaml.
> > > + # The RX delay line of this glue is fine tuning in 200ps steps,
> >
> > Sounds like constraints:
> >
> > maximum: 2000
> > multipleOf: 200
>
> Yes, with maximum: 3000: the field is four bits, 0 to 15 steps of
> 200ps.
> >
> > > + # added on top of what phy-mode describes. The TX side has no fine
> > > + # tuning: its only step is a quarter of the clock period, the full
> > > + # 2ns at 1Gbit/s, so the TX property is not accepted.
> >
> > Move this to tx-internal-delay-ps. You can (kind of ackwardly) do:
> >
> > tx-internal-delay-ps:
> > description: ...
> > not: {}
>
> dt_binding_check does not accept "not" on a property under
> properties: or in an allOf entry: cell.yaml allows only description,
> deprecated, const, enum, minimum, maximum, multipleOf, default, $ref
> and oneOf there. It passes only inside if/then, and the new file has
> no if/then. So the property stays false, with the explanation as a
> comment above it.
Okay, but we should fix that.
Rob
_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic
next prev parent reply other threads:[~2026-11-13 14:23 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-11-05 10:03 [PATCH v6 0/7] Add ethernet support for the Amlogic T7 Lucas Tanure
2026-11-05 10:03 ` Lucas Tanure
2026-11-05 10:03 ` [PATCH v6 1/7] dt-bindings: net: amlogic,meson-dwmac: list the compatible combinations Lucas Tanure
2026-11-05 10:03 ` Lucas Tanure
2026-11-05 10:03 ` [PATCH v6 2/7] dt-bindings: net: amlogic,meson-dwmac: add amlogic,t7-dwmac Lucas Tanure
2026-11-05 10:03 ` Lucas Tanure
2026-11-06 10:04 ` sashiko-bot
2026-11-06 10:04 ` sashiko-bot
2026-11-10 17:54 ` Rob Herring
2026-11-10 17:54 ` Rob Herring
2026-11-13 12:07 ` Lucas Tanure
2026-11-13 12:07 ` Lucas Tanure
2026-11-13 14:23 ` Rob Herring [this message]
2026-11-13 14:23 ` Rob Herring
2026-11-05 10:03 ` [PATCH v6 3/7] net: stmmac: dwmac-meson8b: add support for the Amlogic T7 Lucas Tanure
2026-11-05 10:03 ` Lucas Tanure
2026-11-05 10:03 ` [PATCH v6 4/7] net: stmmac: dwmac-meson8b: follow the generic RGMII delay rules on the T7 Lucas Tanure
2026-11-05 10:03 ` Lucas Tanure
2026-11-05 10:03 ` [PATCH v6 5/7] arm64: dts: amlogic: t7: add the ethernet pinctrl nodes Lucas Tanure
2026-11-05 10:03 ` Lucas Tanure
2026-11-05 10:03 ` [PATCH v6 6/7] arm64: dts: amlogic: t7: add the ethernet controller Lucas Tanure
2026-11-05 10:03 ` Lucas Tanure
2026-11-05 10:03 ` [PATCH v6 7/7] arm64: dts: amlogic: t7: khadas-vim4: enable the ethernet port Lucas Tanure
2026-11-05 10:03 ` Lucas Tanure
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=20261113142329.GA593855-robh@kernel.org \
--to=robh@kernel.org \
--cc=alexandre.torgue@foss.st.com \
--cc=andrew+netdev@lunn.ch \
--cc=conor+dt@kernel.org \
--cc=davem@davemloft.net \
--cc=devicetree@vger.kernel.org \
--cc=edumazet@kernel.org \
--cc=jbrunet@baylibre.com \
--cc=khilman@baylibre.com \
--cc=krzk+dt@kernel.org \
--cc=kuba@kernel.org \
--cc=linux-amlogic@lists.infradead.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=martin.blumenstingl@googlemail.com \
--cc=maxime.chevallier@bootlin.com \
--cc=mcoquelin.stm32@gmail.com \
--cc=neil.armstrong@linaro.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=tanure@linux.com \
--cc=xianwei.zhao@amlogic.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.