| 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
| ||
|
Message-ID: <CAL_JsqJPe1=K7VimSWz+AH2h4fu_2WEud_rUw1dV=SE7pY3C6w@mail.gmail.com>
Date: Mon, 8 Jul 2024 14:19:57 -0600
From: Rob Herring <robh@...nel.org>
To: Lorenzo Bianconi <lorenzo@...nel.org>
Cc: netdev@...r.kernel.org, nbd@....name, lorenzo.bianconi83@...il.com,
davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com,
conor@...nel.org, linux-arm-kernel@...ts.infradead.org,
krzysztof.kozlowski+dt@...aro.org, conor+dt@...nel.org,
devicetree@...r.kernel.org, catalin.marinas@....com, will@...nel.org,
upstream@...oha.com, angelogioacchino.delregno@...labora.com,
benjamin.larsson@...exis.eu, rkannoth@...vell.com, sgoutham@...vell.com,
andrew@...n.ch, arnd@...db.de, horms@...nel.org
Subject: Re: [PATCH v5 net-next 1/2] dt-bindings: net: airoha: Add EN7581
ethernet controller
On Mon, Jul 8, 2024 at 11:03 AM Lorenzo Bianconi <lorenzo@...nel.org> wrote:
>
> > On Thu, Jul 04, 2024 at 10:08:10AM +0200, Lorenzo Bianconi wrote:
> > > Introduce device-tree binding documentation for Airoha EN7581 ethernet
> > > mac controller.
> > >
> > > Signed-off-by: Lorenzo Bianconi <lorenzo@...nel.org>
> > > ---
> > > .../bindings/net/airoha,en7581-eth.yaml | 146 ++++++++++++++++++
> > > 1 file changed, 146 insertions(+)
> > > create mode 100644 Documentation/devicetree/bindings/net/airoha,en7581-eth.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/net/airoha,en7581-eth.yaml b/Documentation/devicetree/bindings/net/airoha,en7581-eth.yaml
> > > new file mode 100644
> > > index 000000000000..f4b1f8afddd0
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/net/airoha,en7581-eth.yaml
> > > @@ -0,0 +1,146 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/net/airoha,en7581-eth.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Airoha EN7581 Frame Engine Ethernet controller
> > > +
> > > +allOf:
> > > + - $ref: ethernet-controller.yaml#
> >
> > Again, to rephrase, what are you using from this binding? It does not
> > make sense for the parent and child both to use it.
>
> Below I reported the ethernet dts node I am using (I have not posted the dts
> changes yet):
What happens when you remove this $ref? Nothing, because you use 0
properties from it. If none of the properties apply, then don't
reference it. It is that simple.
>
> eth0: ethernet@...50000 {
> compatible = "airoha,en7581-eth";
> reg = <0 0x1fb50000 0 0x2600>,
> <0 0x1fb54000 0 0x2000>,
> <0 0x1fb56000 0 0x2000>;
> reg-names = "fe", "qdma0", "qdma1";
>
> resets = <&scuclk EN7581_FE_RST>,
> <&scuclk EN7581_FE_PDMA_RST>,
> <&scuclk EN7581_FE_QDMA_RST>,
> <&scuclk EN7581_XSI_MAC_RST>,
> <&scuclk EN7581_DUAL_HSI0_MAC_RST>,
> <&scuclk EN7581_DUAL_HSI1_MAC_RST>,
> <&scuclk EN7581_HSI_MAC_RST>,
> <&scuclk EN7581_XFP_MAC_RST>;
> reset-names = "fe", "pdma", "qdma", "xsi-mac",
> "hsi0-mac", "hsi1-mac", "hsi-mac",
> "xfp-mac";
>
> interrupts = <GIC_SPI 37 IRQ_TYPE_LEVEL_HIGH>,
> <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>,
> <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>,
> <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>,
> <GIC_SPI 38 IRQ_TYPE_LEVEL_HIGH>,
> <GIC_SPI 58 IRQ_TYPE_LEVEL_HIGH>,
> <GIC_SPI 59 IRQ_TYPE_LEVEL_HIGH>,
> <GIC_SPI 60 IRQ_TYPE_LEVEL_HIGH>,
> <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>,
> <GIC_SPI 64 IRQ_TYPE_LEVEL_HIGH>;
>
> status = "disabled";
>
> #address-cells = <1>;
> #size-cells = <0>;
>
> gdm1: mac@1 {
> compatible = "airoha,eth-mac";
> reg = <1>;
> phy-mode = "internal";
> status = "disabled";
>
> fixed-link {
> speed = <1000>;
> full-duplex;
> pause;
> };
> };
> };
>
> I am using phy related binding for gdm1:mac@1 node.
Right, so you should reference ethernet-controller.yaml for the mac
node because you use properties from the schema.
> gdm1 is the GMAC port used
> as cpu port by the mt7530 dsa switch
That has nothing to do with *this* binding...
>
> switch: switch@...58000 {
> compatible = "airoha,en7581-switch";
> reg = <0 0x1fb58000 0 0x8000>;
> resets = <&scuclk EN7581_GSW_RST>;
>
> interrupt-controller;
> #interrupt-cells = <1>;
> interrupt-parent = <&gic>;
> interrupts = <GIC_SPI 209 IRQ_TYPE_LEVEL_HIGH>;
>
> status = "disabled";
>
> #address-cells = <1>;
> #size-cells = <1>;
>
> ports {
> #address-cells = <1>;
> #size-cells = <0>;
>
> gsw_port1: port@1 {
> reg = <1>;
> label = "lan1";
> phy-mode = "internal";
> phy-handle = <&gsw_phy1>;
> };
>
> gsw_port2: port@2 {
> reg = <2>;
> label = "lan2";
> phy-mode = "internal";
> phy-handle = <&gsw_phy2>;
> };
>
> gsw_port3: port@3 {
> reg = <3>;
> label = "lan3";
> phy-mode = "internal";
> phy-handle = <&gsw_phy3>;
> };
>
> gsw_port4: port@4 {
> reg = <4>;
> label = "lan4";
> phy-mode = "internal";
> phy-handle = <&gsw_phy4>;
> };
>
> port@6 {
> reg = <6>;
> label = "cpu";
> ethernet = <&gdm1>;
> phy-mode = "internal";
>
> fixed-link {
> speed = <1000>;
> full-duplex;
> pause;
> };
> };
> };
>
> mdio {
> #address-cells = <1>;
> #size-cells = <0>;
>
> gsw_phy1: ethernet-phy@1 {
> compatible = "ethernet-phy-ieee802.3-c22";
> reg = <9>;
> phy-mode = "internal";
> };
>
> gsw_phy2: ethernet-phy@2 {
> compatible = "ethernet-phy-ieee802.3-c22";
> reg = <10>;
> phy-mode = "internal";
> };
>
> gsw_phy3: ethernet-phy@3 {
> compatible = "ethernet-phy-ieee802.3-c22";
> reg = <11>;
> phy-mode = "internal";
> };
>
> gsw_phy4: ethernet-phy@4 {
> compatible = "ethernet-phy-ieee802.3-c22";
> reg = <12>;
> phy-mode = "internal";
> };
> };
> };
None of this is relevant.
> > > +patternProperties:
> > > + "^mac@[1-4]$":
> >
> > 'ethernet' is the defined node name for users of
> > ethernet-controller.yaml.
>
> Looking at the dts above, ethernet is already used by the parent node.
So? Not really any reason a node named foo can't have a child named foo, too.
An 'ethernet' node should implement an ethernet interface. It is the
child nodes that implement the ethernet interface(s). Whether you use
'ethernet' on the parent or not, I don't care too much.
> This approach has been already used here [0],[1],[2]. Is it fine to reuse it?
That one appears to be wrong too with the parent referencing
ethernet-controller.yaml.
Rob
> Regards,
> Lorenzo
>
> [0] https://github.com/torvalds/linux/blob/master/arch/arm64/boot/dts/mediatek/mt7622.dtsi#L964
> [1] https://github.com/torvalds/linux/blob/master/arch/arm64/boot/dts/mediatek/mt7622-bananapi-bpi-r64.dts#L136
> [2] https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/net/mediatek%2Cnet.yaml#L370
Powered by blists - more mailing lists