From: Luo Jie <quic_luoj@quicinc.com>
To: Krzysztof Kozlowski <krzk@kernel.org>
Cc: Andrew Lunn <andrew+netdev@lunn.ch>,
"David S. Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@google.com>,
Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
Rob Herring <robh@kernel.org>,
Krzysztof Kozlowski <krzk+dt@kernel.org>,
Conor Dooley <conor+dt@kernel.org>,
Lei Wei <quic_leiwei@quicinc.com>,
Suruchi Agarwal <quic_suruchia@quicinc.com>,
Pavithra R <quic_pavir@quicinc.com>,
"Simon Horman" <horms@kernel.org>,
Jonathan Corbet <corbet@lwn.net>, Kees Cook <kees@kernel.org>,
"Gustavo A. R. Silva" <gustavoars@kernel.org>,
"Philipp Zabel" <p.zabel@pengutronix.de>,
<linux-arm-msm@vger.kernel.org>, <netdev@vger.kernel.org>,
<devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
<linux-doc@vger.kernel.org>, <linux-hardening@vger.kernel.org>,
<quic_kkumarcs@quicinc.com>, <quic_linchen@quicinc.com>
Subject: Re: [PATCH net-next v5 01/14] dt-bindings: net: Add PPE for Qualcomm IPQ9574 SoC
Date: Thu, 3 Jul 2025 17:36:33 +0800 [thread overview]
Message-ID: <e6185a22-8e32-417e-a2d5-a7526ff91bc6@quicinc.com> (raw)
In-Reply-To: <20250701-mottled-clever-walrus-f7dcd3@krzk-bin>
On 7/1/2025 3:11 PM, Krzysztof Kozlowski wrote:
> On Thu, Jun 26, 2025 at 10:31:00PM +0800, Luo Jie wrote:
>> + resets:
>> + maxItems: 1
>> + description: EDMA reset from NSS clock controller
>> +
>> + interrupts:
>> + minItems: 65
>> + maxItems: 65
>> +
>> + interrupt-names:
>> + minItems: 65
>> + maxItems: 65
>> + description:
>> + Interrupts "txcmpl_[0-31]" are the Ethernet DMA TX completion ring interrupts.
>> + Interrupts "rxfill_[0-7]" are the Ethernet DMA RX fill ring interrupts.
>> + Interrupts "rxdesc_[0-23]" are the Ethernet DMA RX Descriptor ring interrupts.
>> + Interrupt "misc" is the Ethernet DMA miscellaneous error interrupt.
>> +
>> + required:
>> + - clocks
>> + - clock-names
>> + - resets
>> + - interrupts
>> + - interrupt-names
>> +
>> +patternProperties:
>> + "^(ethernet-)?port@[0-9a-f]+$":
>
> Only one port? What are you switching here?
Sorry that this was missed in the update. We wanted to
add the ethernet-port' property to ensure we document
the per-port clocks/resets for completeness, but missed
adding the 'ethernet-ports' property. I will add the
'ethernet-ports' node in the updated version of the patch
to accurately reflect the schema and hardware hierarchy.
There are six physical ports in the PPE that can be part of
the switch function.
>
> Anyway, ^ethernet-port..... is preferred over port.
OK, I will update to use '^ethernet-port' instead.
>
> But other problem is that it does not match referenced schema at all and
> nothing in commit msg explains why this appered. 1.5 years of
> development of this and some significant, unexpected and not correct
> changes.
>
I understand your concern. This change was described briefly in the V5
cover letter, but I will improve this description in cover letter and
update the commit message as well, to explicitly document this change.
The motivation for adding the ethernet-port node in bindings was to
document the required per-port clocks and resets as well, as these
are essential for enabling Ethernet functionality on this hardware.
Could you please review the following proposed changes and let me know
if this approach is acceptable?
patternProperties:
"^(ethernet-)?ports$":
additionalProperties: true
patternProperties:
"^ethernet-port@[1-6]+$":
type: object
unevaluatedProperties: false
$ref: ethernet-controller.yaml#
properties:
reg:
minimum: 1
maximum: 6
description: PPE Ethernet port ID
clocks:
items:
- description: Port MAC clock from NSS clock controller
- description: Port RX clock from NSS clock controller
- description: Port TX clock from NSS clock controller
clock-names:
items:
- const: mac
- const: rx
- const: tx
resets:
items:
- description: Port MAC reset from NSS clock controller
- description: Port RX reset from NSS clock controller
- description: Port TX reset from NSS clock controller
reset-names:
items:
- const: mac
- const: rx
- const: tx
required:
- reg
- clocks
- clock-names
- resets
- reset-names
...
allOf:
- $ref: ethernet-switch.yaml
...
>> + unevaluatedProperties: false
>> + $ref: ethernet-switch-port.yaml#
>> +
>> + properties:
>> + clocks:
>> + items:
>> + - description: Port MAC clock from NSS clock controller
>> + - description: Port RX clock from NSS clock controller
>> + - description: Port TX clock from NSS clock controller
>> +
>> + clock-names:
>> + items:
>> + - const: mac
>> + - const: rx
>> + - const: tx
>> +
>> + resets:
>> + items:
>> + - description: Port MAC reset from NSS clock controller
>> + - description: Port RX reset from NSS clock controller
>> + - description: Port TX reset from NSS clock controller
>> +
>> + reset-names:
>> + items:
>> + - const: mac
>> + - const: rx
>> + - const: tx
>> +
>> + required:
>> + - reg
>> + - clocks
>> + - clock-names
>> + - resets
>> + - reset-names
>> +
>> +required:
>> + - compatible
>> + - reg
>> + - clocks
>> + - clock-names
>> + - resets
>> + - interconnects
>> + - interconnect-names
>> + - ethernet-dma
>> +
>> +allOf:
>> + - $ref: ethernet-switch.yaml
>> +
>> +unevaluatedProperties: false
>> +
>> +examples:
>> + - |
>> + #include <dt-bindings/clock/qcom,ipq9574-gcc.h>
>> + #include <dt-bindings/clock/qcom,ipq9574-nsscc.h>
>> + #include <dt-bindings/interconnect/qcom,ipq9574.h>
>> + #include <dt-bindings/interrupt-controller/arm-gic.h>
>> + #include <dt-bindings/reset/qcom,ipq9574-nsscc.h>
>> +
>> + ethernet-switch@3a000000 {
>> + compatible = "qcom,ipq9574-ppe";
>> + reg = <0x3a000000 0xbef800>;
>> + clocks = <&nsscc NSS_CC_PPE_SWITCH_CLK>,
>> + <&nsscc NSS_CC_PPE_SWITCH_CFG_CLK>,
>> + <&nsscc NSS_CC_PPE_SWITCH_IPE_CLK>,
>> + <&nsscc NSS_CC_PPE_SWITCH_BTQ_CLK>;
>> + clock-names = "ppe",
>> + "apb",
>> + "ipe",
>> + "btq";
>> + resets = <&nsscc PPE_FULL_RESET>;
>> + interrupts = <GIC_SPI 498 IRQ_TYPE_LEVEL_HIGH>;
>> + interconnects = <&nsscc MASTER_NSSNOC_PPE &nsscc SLAVE_NSSNOC_PPE>,
>> + <&nsscc MASTER_NSSNOC_PPE_CFG &nsscc SLAVE_NSSNOC_PPE_CFG>,
>> + <&gcc MASTER_NSSNOC_QOSGEN_REF &gcc SLAVE_NSSNOC_QOSGEN_REF>,
>> + <&gcc MASTER_NSSNOC_TIMEOUT_REF &gcc SLAVE_NSSNOC_TIMEOUT_REF>,
>> + <&gcc MASTER_MEM_NOC_NSSNOC &gcc SLAVE_MEM_NOC_NSSNOC>,
>> + <&gcc MASTER_NSSNOC_MEMNOC &gcc SLAVE_NSSNOC_MEMNOC>,
>> + <&gcc MASTER_NSSNOC_MEM_NOC_1 &gcc SLAVE_NSSNOC_MEM_NOC_1>;
>> + interconnect-names = "ppe",
>> + "ppe_cfg",
>> + "qos_gen",
>> + "timeout_ref",
>> + "nssnoc_memnoc",
>> + "memnoc_nssnoc",
>> + "memnoc_nssnoc_1";
>> +
>> + ethernet-dma {
>> + clocks = <&nsscc NSS_CC_PPE_EDMA_CLK>,
>> + <&nsscc NSS_CC_PPE_EDMA_CFG_CLK>;
>> + clock-names = "sys",
>> + "apb";
>> + resets = <&nsscc EDMA_HW_RESET>;
>> + interrupts = <GIC_SPI 363 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 364 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 365 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 366 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 367 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 368 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 369 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 370 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 371 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 372 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 373 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 374 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 375 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 376 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 377 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 378 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 379 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 380 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 381 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 382 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 383 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 384 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 509 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 508 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 507 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 506 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 505 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 504 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 503 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 502 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 501 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 500 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 355 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 356 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 357 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 358 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 359 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 360 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 361 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 362 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 331 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 332 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 333 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 334 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 335 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 336 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 337 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 338 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 339 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 340 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 341 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 342 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 343 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 344 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 345 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 346 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 347 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 348 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 349 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 350 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 351 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 352 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 353 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 354 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 499 IRQ_TYPE_LEVEL_HIGH>;
>> + interrupt-names = "txcmpl_0",
>> + "txcmpl_1",
>> + "txcmpl_2",
>> + "txcmpl_3",
>> + "txcmpl_4",
>> + "txcmpl_5",
>> + "txcmpl_6",
>> + "txcmpl_7",
>> + "txcmpl_8",
>> + "txcmpl_9",
>> + "txcmpl_10",
>> + "txcmpl_11",
>> + "txcmpl_12",
>> + "txcmpl_13",
>> + "txcmpl_14",
>> + "txcmpl_15",
>> + "txcmpl_16",
>> + "txcmpl_17",
>> + "txcmpl_18",
>> + "txcmpl_19",
>> + "txcmpl_20",
>> + "txcmpl_21",
>> + "txcmpl_22",
>> + "txcmpl_23",
>> + "txcmpl_24",
>> + "txcmpl_25",
>> + "txcmpl_26",
>> + "txcmpl_27",
>> + "txcmpl_28",
>> + "txcmpl_29",
>> + "txcmpl_30",
>> + "txcmpl_31",
>> + "rxfill_0",
>> + "rxfill_1",
>> + "rxfill_2",
>> + "rxfill_3",
>> + "rxfill_4",
>> + "rxfill_5",
>> + "rxfill_6",
>> + "rxfill_7",
>> + "rxdesc_0",
>> + "rxdesc_1",
>> + "rxdesc_2",
>> + "rxdesc_3",
>> + "rxdesc_4",
>> + "rxdesc_5",
>> + "rxdesc_6",
>> + "rxdesc_7",
>> + "rxdesc_8",
>> + "rxdesc_9",
>> + "rxdesc_10",
>> + "rxdesc_11",
>> + "rxdesc_12",
>> + "rxdesc_13",
>> + "rxdesc_14",
>> + "rxdesc_15",
>> + "rxdesc_16",
>> + "rxdesc_17",
>> + "rxdesc_18",
>> + "rxdesc_19",
>> + "rxdesc_20",
>> + "rxdesc_21",
>> + "rxdesc_22",
>> + "rxdesc_23",
>> + "misc";
>> + };
>> +
>> + ethernet-ports {
>
> Look at your binding, not what it said...
I will fix this to add the ethernet-ports node in the next version.
>
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + port@1 {
>
> Best regards,
> Krzysztof
>
>
next prev parent reply other threads:[~2025-07-03 9:36 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-06-26 14:30 [PATCH net-next v5 00/14] Add PPE driver for Qualcomm IPQ9574 SoC Luo Jie
2025-06-26 14:31 ` [PATCH net-next v5 01/14] dt-bindings: net: Add PPE " Luo Jie
2025-07-01 7:11 ` Krzysztof Kozlowski
2025-07-03 9:36 ` Luo Jie [this message]
2025-06-26 14:31 ` [PATCH net-next v5 02/14] docs: networking: Add PPE driver documentation " Luo Jie
2025-06-26 22:02 ` Randy Dunlap
2025-06-27 13:09 ` Lei Wei
2025-06-26 14:31 ` [PATCH net-next v5 03/14] net: ethernet: qualcomm: Add PPE driver for " Luo Jie
2025-06-27 16:21 ` Konrad Dybcio
2025-07-01 12:24 ` Luo Jie
2025-07-30 11:57 ` Konrad Dybcio
2025-08-01 14:42 ` Luo Jie
2025-06-26 14:31 ` [PATCH net-next v5 04/14] net: ethernet: qualcomm: Initialize PPE buffer management for IPQ9574 Luo Jie
2025-06-26 14:31 ` [PATCH net-next v5 05/14] net: ethernet: qualcomm: Initialize PPE queue " Luo Jie
2025-06-26 14:31 ` [PATCH net-next v5 06/14] net: ethernet: qualcomm: Initialize the PPE scheduler settings Luo Jie
2025-06-26 14:31 ` [PATCH net-next v5 07/14] net: ethernet: qualcomm: Initialize PPE queue settings Luo Jie
2025-06-26 14:31 ` [PATCH net-next v5 08/14] net: ethernet: qualcomm: Initialize PPE service code settings Luo Jie
2025-06-26 14:31 ` [PATCH net-next v5 09/14] net: ethernet: qualcomm: Initialize PPE port control settings Luo Jie
2025-06-26 14:31 ` [PATCH net-next v5 10/14] net: ethernet: qualcomm: Initialize PPE RSS hash settings Luo Jie
2025-07-17 20:48 ` Konrad Dybcio
2025-07-18 7:41 ` Luo Jie
2025-06-26 14:31 ` [PATCH net-next v5 11/14] net: ethernet: qualcomm: Initialize PPE queue to Ethernet DMA ring mapping Luo Jie
2025-06-26 14:31 ` [PATCH net-next v5 12/14] net: ethernet: qualcomm: Initialize PPE L2 bridge settings Luo Jie
2025-06-26 14:31 ` [PATCH net-next v5 13/14] net: ethernet: qualcomm: Add PPE debugfs support for PPE counters Luo Jie
2025-06-26 14:31 ` [PATCH net-next v5 14/14] MAINTAINERS: Add maintainer for Qualcomm PPE driver Luo Jie
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=e6185a22-8e32-417e-a2d5-a7526ff91bc6@quicinc.com \
--to=quic_luoj@quicinc.com \
--cc=andrew+netdev@lunn.ch \
--cc=conor+dt@kernel.org \
--cc=corbet@lwn.net \
--cc=davem@davemloft.net \
--cc=devicetree@vger.kernel.org \
--cc=edumazet@google.com \
--cc=gustavoars@kernel.org \
--cc=horms@kernel.org \
--cc=kees@kernel.org \
--cc=krzk+dt@kernel.org \
--cc=krzk@kernel.org \
--cc=kuba@kernel.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-hardening@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=p.zabel@pengutronix.de \
--cc=pabeni@redhat.com \
--cc=quic_kkumarcs@quicinc.com \
--cc=quic_leiwei@quicinc.com \
--cc=quic_linchen@quicinc.com \
--cc=quic_pavir@quicinc.com \
--cc=quic_suruchia@quicinc.com \
--cc=robh@kernel.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).