Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Vladimir Oltean <vladimir.oltean@nxp.com>
To:     Maxime Chevallier <maxime.chevallier@bootlin.com>
CC:     "davem@davemloft.net" <davem@davemloft.net>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
        "thomas.petazzoni@bootlin.com" <thomas.petazzoni@bootlin.com>,
        Andrew Lunn <andrew@lunn.ch>,
        Florian Fainelli <f.fainelli@gmail.com>,
        Heiner Kallweit <hkallweit1@gmail.com>,
        Russell King <linux@armlinux.org.uk>,
        "linux-arm-kernel@lists.infradead.org" 
        <linux-arm-kernel@lists.infradead.org>,
        Luka Perkov <luka.perkov@sartura.hr>,
        Robert Marko <robert.marko@sartura.hr>
Subject: Re: [PATCH net-next v5 1/5] net: dt-bindings: Introduce the Qualcomm
 IPQESS Ethernet controller
Thread-Topic: [PATCH net-next v5 1/5] net: dt-bindings: Introduce the Qualcomm
 IPQESS Ethernet controller
Thread-Index: AQHY5UsVP18kioT6hUa4h8fI414WPq4Y6OIA
Date:   Fri, 21 Oct 2022 14:30:59 +0000
Message-ID: <20221021143058.fg5vet5or4qcnet2@skbuf>
References: <20221021124556.100445-1-maxime.chevallier@bootlin.com>
 <20221021124556.100445-2-maxime.chevallier@bootlin.com>
In-Reply-To: <20221021124556.100445-2-maxime.chevallier@bootlin.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?B+Onru/PwlfHLKDHY1vJtc3TaiJSGQLsoxdrN+MNcadmWEaM82gWVDFcmX/5?=
 =?us-ascii?Q?D2OoITJ4G3HD/O+jQtlA9ZRnhzSI4W3P7rkI0Dmz/6TE12obJWrYJJ66rIA7?=
 =?us-ascii?Q?cNnu/U+9wYSVoDiGNzGvBB8y1NdTUKfnvgYfF6GHA8+m6t2Y19gPINef/E2b?=
 =?us-ascii?Q?qOQNc61awIyRsXpLx96C5aWCto+7EdCqVbdrbXdm7nWeCxWdzwt70pmp8po0?=
 =?us-ascii?Q?khyWeEIBZP4NCRy2UT0JUSyQY1Q9ZCnCKuWmfcA0QbrESGPWu0NZU/U9IG4f?=
 =?us-ascii?Q?y135abCxykoLLt0m4hmdz36G823uE0R+JLC5PVuJAkR1P2AxaNp6ObrQcK2M?=
 =?us-ascii?Q?tlmFH2nkxZguxcH5XIrxs5qeM852n4AA3MCvCIkyKIB94fjlyKPoI1+2SRLg?=
 =?us-ascii?Q?R1D/KRViwmLuTIcl4PEWp9+9lBSZL5RZ8YV2dWsyZRRye3ncGFYEM/cmysQK?=
 =?us-ascii?Q?O5YwkUXNJ+Nd3u84wukuKNSajYV7mEUYWaaZGrpTNuXfQ8sy+ox9sAB/U8/+?=
 =?us-ascii?Q?Oq5jqrX1A0NZYgVMnf7GwiYI4BlUQpMNwQ3u82bx0c6Gl25CuxGxutmsaS3M?=
 =?us-ascii?Q?pmjxuhOaFOv/WzmkhCWnIWBoHYH1gPEXRLo/9fvR9HpDCgP8sZB1VvSBvMto?=
 =?us-ascii?Q?qRPr9sA3p8Uhke2FFOmlLroz5AxcBORN71Wyci4MdrACL0yJ+eqdGhvQv4FJ?=
 =?us-ascii?Q?tLfKbpsRWNXCmIWKrIGCSLiBfxeb+kSDfewBNyV/MeQ2rgn6bECOzUDlMo9J?=
 =?us-ascii?Q?gLZz47v1fd8UgA3+1iKnYyuUXEzpQbDxW0aSnSglRgl+ysRQ2hhBMaqv+nIf?=
 =?us-ascii?Q?V2uMEG2yNe2IX62MrdrgPrP9B2r32c3bIqciGtpRoL5imtncYlkeNbgc3T0S?=
 =?us-ascii?Q?sAwZXDXeOBN79WiTRLVOwGfPTba+dqy9x8SL2eryzR689w0G320N7byq0nfi?=
 =?us-ascii?Q?S73MwuCLtUJOpgNNOIghUhpi/r1utVz62H3T3jsCil11re9z1Vs6Zovo1bTS?=
 =?us-ascii?Q?LekQ1CTUl+60iEjF31m8aN/jgwImaP0hRcLASyRdF89LWJeZg2pE/b6iJFtO?=
 =?us-ascii?Q?mdzGHNwYe0Cg5m7ONhuX/HJv9s5bTPfdYwoL4c5NIbxXpovCOzOZz3FkBNHV?=
 =?us-ascii?Q?sS2cRAUktgPXireYQcd2bnMCXDEJaQSsLzq+l0soQLWTUDcTE0knoyClSsoU?=
 =?us-ascii?Q?o0oCRGdOcxXbDJB0RmtJsTu4sfod8Q3Muh7Osp9BeCfZNayBxkcFFVOe6S8T?=
 =?us-ascii?Q?/x7zdewMLemhefcH8xsHLMLSZiGhWrTjEeE4e6V8q8BtNrohDS6EnK6q+ivP?=
 =?us-ascii?Q?38QFPz7s6wONf8oidJmF4rpuuBTnJgjcV0UvaZ/wXYxPKQhyqywf+eVkUV1v?=
 =?us-ascii?Q?DDeDTt9YEbqbV37L0qhK2LML3z0PVLrrjoLOO4+jiGFDHZGYUw6CW4/R4lNn?=
 =?us-ascii?Q?c6TvSghFjvjpzzpe30XBrcqGq22py4fsaZ24DKMCcUgiiDFs/mx/+jyt4DlN?=
 =?us-ascii?Q?8jGraEZ3krIIihyAeiphM/h0Nk7vEIAdF6BplSbDRW8t38GuvrbiUbmNRruA?=
 =?us-ascii?Q?lb824VYgNcz+TWUUoZ3BbEbNCNM+WLQuX/+wADLYVS+sHGnbW8H1wM8aerrM?=
 =?us-ascii?Q?wA=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <C127843DFBD92141A638F6390BA1EDC8@eurprd04.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: VI1PR04MB5136.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 21d747e3-91bf-4d0f-db48-08dab370df7e
X-MS-Exchange-CrossTenant-originalarrivaltime: 21 Oct 2022 14:30:59.3532
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: MEY6m0MrPTQo/8da2KR+9tQ0Fci1D7MCXmIIlvFrKSzkU5UDAbujLrHMPU+7nCo+9O9wrfUQGrWXZJtPH+tN5w==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AS8PR04MB8136
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,URIBL_BLOCKED autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Fri, Oct 21, 2022 at 02:45:52PM +0200, Maxime Chevallier wrote:
> +  interrupts:
> +    minItems: 2
> +    maxItems: 32
> +    description: One interrupt per tx and rx queue, with up to 16 queues=
.

What does the binding require in terms of ordering, exactly? Whose
interrupt is the 7th one (GIC_SPI 71 in your example)? RX/TX of which
queue?

> +
> +  clocks:
> +    maxItems: 1
> +
> +  resets:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - resets
> +  - phy-mode
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/qcom,gcc-ipq4019.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    gmac: ethernet@c080000 {
> +        compatible =3D "qcom,ipq4019-ess-edma";
> +        reg =3D <0xc080000 0x8000>;
> +        resets =3D <&gcc ESS_RESET>;
> +        clocks =3D <&gcc GCC_ESS_CLK>;
> +        interrupts =3D <GIC_SPI  65 IRQ_TYPE_EDGE_RISING>,
> +                     <GIC_SPI  66 IRQ_TYPE_EDGE_RISING>,
> +                     <GIC_SPI  67 IRQ_TYPE_EDGE_RISING>,
> +                     <GIC_SPI  68 IRQ_TYPE_EDGE_RISING>,
> +                     <GIC_SPI  69 IRQ_TYPE_EDGE_RISING>,
> +                     <GIC_SPI  70 IRQ_TYPE_EDGE_RISING>,
> +                     <GIC_SPI  71 IRQ_TYPE_EDGE_RISING>,
> +                     <GIC_SPI  72 IRQ_TYPE_EDGE_RISING>,
> +                     <GIC_SPI  73 IRQ_TYPE_EDGE_RISING>,
> +                     <GIC_SPI  74 IRQ_TYPE_EDGE_RISING>,
> +                     <GIC_SPI  75 IRQ_TYPE_EDGE_RISING>,
> +                     <GIC_SPI  76 IRQ_TYPE_EDGE_RISING>,
> +                     <GIC_SPI  77 IRQ_TYPE_EDGE_RISING>,
> +                     <GIC_SPI  78 IRQ_TYPE_EDGE_RISING>,
> +                     <GIC_SPI  79 IRQ_TYPE_EDGE_RISING>,
> +                     <GIC_SPI  80 IRQ_TYPE_EDGE_RISING>,
> +                     <GIC_SPI 240 IRQ_TYPE_EDGE_RISING>,
> +                     <GIC_SPI 241 IRQ_TYPE_EDGE_RISING>,
> +                     <GIC_SPI 242 IRQ_TYPE_EDGE_RISING>,
> +                     <GIC_SPI 243 IRQ_TYPE_EDGE_RISING>,
> +                     <GIC_SPI 244 IRQ_TYPE_EDGE_RISING>,
> +                     <GIC_SPI 245 IRQ_TYPE_EDGE_RISING>,
> +                     <GIC_SPI 246 IRQ_TYPE_EDGE_RISING>,
> +                     <GIC_SPI 247 IRQ_TYPE_EDGE_RISING>,
> +                     <GIC_SPI 248 IRQ_TYPE_EDGE_RISING>,
> +                     <GIC_SPI 249 IRQ_TYPE_EDGE_RISING>,
> +                     <GIC_SPI 250 IRQ_TYPE_EDGE_RISING>,
> +                     <GIC_SPI 251 IRQ_TYPE_EDGE_RISING>,
> +                     <GIC_SPI 252 IRQ_TYPE_EDGE_RISING>,
> +                     <GIC_SPI 253 IRQ_TYPE_EDGE_RISING>,
> +                     <GIC_SPI 254 IRQ_TYPE_EDGE_RISING>,
> +                     <GIC_SPI 255 IRQ_TYPE_EDGE_RISING>;
> +
> +        phy-mode =3D "internal";

I think empty lines are typically added between properties and nodes
(and between nodes on the same hierarchy), rather than between 2
properties.

> +        fixed-link {
> +            speed =3D <1000>;
> +            full-duplex;
> +            pause;
> +            asym-pause;

Any particular reason for "asym-pause"? Looking at the comment above
linkmode_resolve_pause(), I don't think it makes any difference to the
flow control resolution (link partner AsmDir is "don't care" when we
have MAC_SYM_PAUSE in mac_capabilities and the "fixed-link" node -
effectively the link partner - also has "pause").

> +        };
> +    };
> +
> +...
> --=20
> 2.37.3
>=