[PATCH net-next 0/4] net: ethernet: qualcomm: ppe: Add IPQ5332 support

[PATCH net-next 0/4] net: ethernet: qualcomm: ppe: Add IPQ5332 support

[Til Kaiser]

IPQ5332 uses the same PPE architecture as IPQ9574, but has two front
panel ports, a 200 MHz PPE clock, and different interconnect, buffer
and scheduler settings. This series adds support for it.

Patch 1 moves the IPQ9574 settings into per-SoC match data and a
hardware configuration structure, renames the shared regmap configuration,
and makes the reserved scheduler resource lookup independent of the port
count. Patch 2 tightens the port node constraints in the DT schema.
Patch 3 adds IPQ5332 to the schema with per-SoC interconnect, clock, EDMA
reset and interrupt constraints, while retaining the original binding
filename. Patch 4 adds the IPQ5332 configuration to the driver and adds
its port and UNIPHY counts to the documentation.

The IPQ5332 values come from the PPE initialization in Qualcomm's
downstream QSDK 14.0.1595.1665, qca-ssdk revision
48484d531927b070b996685f1570aff3bccb9c32 (src/init/ssdk_hppe.c,
src/init/ssdk_appe.c and src/adpt/hppe/adpt_hppe_qm.c).

IPQ5332 requires the GCC IM sleep clock in addition to the four PPE
clocks, two EDMA resets instead of one, and 49 EDMA interrupts instead
of 65. These differences are constrained without changing the existing
IPQ9574 layouts.

Tested on the Ubiquiti UniFi 7 Pro XGS access point, using the EDMA
driver from OpenWrt, which is not yet upstream. The device has a
single Ethernet port, connected to PPE port 1. The second front
panel port and its configuration are untested from my side.

Thanks,
Til

Til Kaiser (4):
  net: ethernet: qualcomm: ppe: Select hardware configuration by SoC
  dt-bindings: net: qcom,ipq9574-ppe: Tighten ethernet port node
    constraints
  dt-bindings: net: qcom,ipq9574-ppe: Add IPQ5332 support
  net: ethernet: qualcomm: ppe: Add IPQ5332 support

 .../bindings/net/qcom,ipq9574-ppe.yaml        | 103 +++++++++-
 .../ethernet/qualcomm/ppe/ppe.rst             |   9 +-
 drivers/net/ethernet/qualcomm/ppe/ppe.c       |  81 ++++++--
 .../net/ethernet/qualcomm/ppe/ppe_config.c    | 185 ++++++++++++++++--
 .../net/ethernet/qualcomm/ppe/ppe_config.h    |  35 +++-
 5 files changed, 369 insertions(+), 44 deletions(-)


base-commit: 8b4e7209c842d8cb9516f1f5ef0a88aa2d8831a6
-- 
2.55.0

[PATCH net-next 1/4] net: ethernet: qualcomm: ppe: Select hardware configuration by SoC

[Til Kaiser]

The PPE driver hardcodes IPQ9574 parameters: the PPE clock rate,
interconnect paths, port count, buffer limits and scheduler arbitration
tables. Other SoCs with the same PPE architecture need different values.

Move these into per-SoC platform match data, and collect the buffer
manager (BM), queue manager (QM) and scheduler tables in a new struct
ppe_soc_config that is passed to the hardware initialization code. The
IPQ9574 values are unchanged; no functional change intended.

The IPQ9574 scheduler resource table has nine entries, with the reserved
pool at index 8. Select that final entry for port ID num_ports, preserving
the IPQ9574 lookup while allowing a smaller port count on other SoCs.

Rename regmap_config_ipq9574 to ppe_reg_config for the shared PPE
register layout.

Signed-off-by: Til Kaiser <mail@tk154.de>
---
 drivers/net/ethernet/qualcomm/ppe/ppe.c       | 48 ++++++++++-----
 .../net/ethernet/qualcomm/ppe/ppe_config.c    | 58 +++++++++++++------
 .../net/ethernet/qualcomm/ppe/ppe_config.h    | 34 ++++++++++-
 3 files changed, 106 insertions(+), 34 deletions(-)

diff --git a/drivers/net/ethernet/qualcomm/ppe/ppe.c b/drivers/net/ethernet/qualcomm/ppe/ppe.c
index 3c301e609d3e..0ef72c20364b 100644
--- a/drivers/net/ethernet/qualcomm/ppe/ppe.c
+++ b/drivers/net/ethernet/qualcomm/ppe/ppe.c
@@ -11,6 +11,7 @@
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
+#include <linux/property.h>
 #include <linux/regmap.h>
 #include <linux/reset.h>
 
@@ -18,13 +19,15 @@
 #include "ppe_config.h"
 #include "ppe_debugfs.h"
 
-#define PPE_PORT_MAX		8
-#define PPE_CLK_RATE		353000000
+struct ppe_plat_data {
+	const struct ppe_soc_config *soc_cfg;
+	const struct icc_bulk_data *icc_data;
+	unsigned long ppe_clk_rate;
+	unsigned int num_icc, num_ports;
+};
 
-/* ICC clocks for enabling PPE device. The avg_bw and peak_bw with value 0
- * will be updated by the clock rate of PPE.
- */
-static const struct icc_bulk_data ppe_icc_data[] = {
+/* IPQ9574 interconnect paths. Zero bandwidths use the PPE clock rate. */
+static const struct icc_bulk_data ipq9574_icc_data[] = {
 	{
 		.name = "ppe",
 		.avg_bw = 0,
@@ -62,6 +65,15 @@ static const struct icc_bulk_data ppe_icc_data[] = {
 	},
 };
 
+static const struct ppe_plat_data ipq9574_ppe_data = {
+	.soc_cfg = &ipq9574_soc_config,
+	.icc_data = ipq9574_icc_data,
+	.ppe_clk_rate = 353000000,
+	.num_icc = ARRAY_SIZE(ipq9574_icc_data),
+	.num_ports = 8,
+};
+
+/* PPE register access ranges. */
 static const struct regmap_range ppe_readable_ranges[] = {
 	regmap_reg_range(0x0, 0x1ff),		/* Global */
 	regmap_reg_range(0x400, 0x5ff),		/* LPI CSR */
@@ -99,7 +111,7 @@ static const struct regmap_access_table ppe_reg_table = {
 	.n_yes_ranges = ARRAY_SIZE(ppe_readable_ranges),
 };
 
-static const struct regmap_config regmap_config_ipq9574 = {
+static const struct regmap_config ppe_reg_config = {
 	.reg_bits = 32,
 	.reg_stride = 4,
 	.val_bits = 32,
@@ -108,7 +120,8 @@ static const struct regmap_config regmap_config_ipq9574 = {
 	.max_register = 0xbef800,
 };
 
-static int ppe_clock_init_and_reset(struct ppe_device *ppe_dev)
+static int ppe_clock_init_and_reset(struct ppe_device *ppe_dev,
+				    const struct icc_bulk_data *ppe_icc_data)
 {
 	unsigned long ppe_rate = ppe_dev->clk_rate;
 	struct device *dev = ppe_dev->dev;
@@ -172,12 +185,17 @@ static int ppe_clock_init_and_reset(struct ppe_device *ppe_dev)
 
 static int qcom_ppe_probe(struct platform_device *pdev)
 {
+	const struct ppe_plat_data *plat_data;
 	struct device *dev = &pdev->dev;
 	struct ppe_device *ppe_dev;
 	void __iomem *base;
 	int ret, num_icc;
 
-	num_icc = ARRAY_SIZE(ppe_icc_data);
+	plat_data = device_get_match_data(dev);
+	if (!plat_data)
+		return -EINVAL;
+
+	num_icc = plat_data->num_icc;
 	ppe_dev = devm_kzalloc(dev, struct_size(ppe_dev, icc_paths, num_icc),
 			       GFP_KERNEL);
 	if (!ppe_dev)
@@ -187,20 +205,20 @@ static int qcom_ppe_probe(struct platform_device *pdev)
 	if (IS_ERR(base))
 		return dev_err_probe(dev, PTR_ERR(base), "PPE ioremap failed\n");
 
-	ppe_dev->regmap = devm_regmap_init_mmio(dev, base, &regmap_config_ipq9574);
+	ppe_dev->regmap = devm_regmap_init_mmio(dev, base, &ppe_reg_config);
 	if (IS_ERR(ppe_dev->regmap))
 		return dev_err_probe(dev, PTR_ERR(ppe_dev->regmap),
 				     "PPE initialize regmap failed\n");
 	ppe_dev->dev = dev;
-	ppe_dev->clk_rate = PPE_CLK_RATE;
-	ppe_dev->num_ports = PPE_PORT_MAX;
+	ppe_dev->clk_rate = plat_data->ppe_clk_rate;
+	ppe_dev->num_ports = plat_data->num_ports;
 	ppe_dev->num_icc_paths = num_icc;
 
-	ret = ppe_clock_init_and_reset(ppe_dev);
+	ret = ppe_clock_init_and_reset(ppe_dev, plat_data->icc_data);
 	if (ret)
 		return dev_err_probe(dev, ret, "PPE clock config failed\n");
 
-	ret = ppe_hw_config(ppe_dev);
+	ret = ppe_hw_config(ppe_dev, plat_data->soc_cfg);
 	if (ret)
 		return dev_err_probe(dev, ret, "PPE HW config failed\n");
 
@@ -219,7 +237,7 @@ static void qcom_ppe_remove(struct platform_device *pdev)
 }
 
 static const struct of_device_id qcom_ppe_of_match[] = {
-	{ .compatible = "qcom,ipq9574-ppe" },
+	{ .compatible = "qcom,ipq9574-ppe", .data = &ipq9574_ppe_data },
 	{}
 };
 MODULE_DEVICE_TABLE(of, qcom_ppe_of_match);
diff --git a/drivers/net/ethernet/qualcomm/ppe/ppe_config.c b/drivers/net/ethernet/qualcomm/ppe/ppe_config.c
index 125b73be92b1..65ac6c0841d5 100644
--- a/drivers/net/ethernet/qualcomm/ppe/ppe_config.c
+++ b/drivers/net/ethernet/qualcomm/ppe/ppe_config.c
@@ -835,6 +835,19 @@ static const struct ppe_port_schedule_resource ppe_scheduler_res[] = {
 	},
 };
 
+const struct ppe_soc_config ipq9574_soc_config = {
+	.bm_group_config = ipq9574_ppe_bm_group_config,
+	.bm_port_config = ipq9574_ppe_bm_port_config,
+	.bm_port_config_cnt = ARRAY_SIZE(ipq9574_ppe_bm_port_config),
+	.qm_group_config = ipq9574_ppe_qm_group_config,
+	.qm_queue_config = ipq9574_ppe_qm_queue_config,
+	.qm_queue_config_cnt = ARRAY_SIZE(ipq9574_ppe_qm_queue_config),
+	.sch_bm_config = ipq9574_ppe_sch_bm_config,
+	.sch_bm_config_cnt = ARRAY_SIZE(ipq9574_ppe_sch_bm_config),
+	.sch_qm_config = ipq9574_ppe_sch_qm_config,
+	.sch_qm_config_cnt = ARRAY_SIZE(ipq9574_ppe_sch_qm_config),
+};
+
 /* Set the PPE queue level scheduler configuration. */
 static int ppe_scheduler_l0_queue_map_set(struct ppe_device *ppe_dev,
 					  int node_id, int port,
@@ -1080,7 +1093,12 @@ int ppe_port_resource_get(struct ppe_device *ppe_dev, int port,
 	if (port > ppe_dev->num_ports)
 		return -EINVAL;
 
-	res = ppe_scheduler_res[port];
+	/* Port ID num_ports selects the reserved pool in the final entry. */
+	if (port == ppe_dev->num_ports)
+		res = ppe_scheduler_res[ARRAY_SIZE(ppe_scheduler_res) - 1];
+	else
+		res = ppe_scheduler_res[port];
+
 	switch (type) {
 	case PPE_RES_UCAST:
 		*res_start = res.ucastq_start;
@@ -1427,7 +1445,8 @@ static int ppe_config_bm_threshold(struct ppe_device *ppe_dev, int bm_port_id,
 }
 
 /* Configure the buffer threshold for the port flow control function. */
-static int ppe_config_bm(struct ppe_device *ppe_dev)
+static int ppe_config_bm(struct ppe_device *ppe_dev,
+			 const struct ppe_soc_config *soc_cfg)
 {
 	const struct ppe_bm_port_config *port_cfg;
 	unsigned int i, bm_port_id, port_cfg_cnt;
@@ -1440,7 +1459,7 @@ static int ppe_config_bm(struct ppe_device *ppe_dev)
 	 */
 	reg = PPE_BM_SHARED_GROUP_CFG_ADDR;
 	val = FIELD_PREP(PPE_BM_SHARED_GROUP_CFG_SHARED_LIMIT,
-			 ipq9574_ppe_bm_group_config);
+			 soc_cfg->bm_group_config);
 	ret = regmap_update_bits(ppe_dev->regmap, reg,
 				 PPE_BM_SHARED_GROUP_CFG_SHARED_LIMIT,
 				 val);
@@ -1448,8 +1467,8 @@ static int ppe_config_bm(struct ppe_device *ppe_dev)
 		goto bm_config_fail;
 
 	/* Configure buffer thresholds for the BM ports. */
-	port_cfg = ipq9574_ppe_bm_port_config;
-	port_cfg_cnt = ARRAY_SIZE(ipq9574_ppe_bm_port_config);
+	port_cfg = soc_cfg->bm_port_config;
+	port_cfg_cnt = soc_cfg->bm_port_config_cnt;
 	for (i = 0; i < port_cfg_cnt; i++) {
 		for (bm_port_id = port_cfg[i].port_id_start;
 		     bm_port_id <= port_cfg[i].port_id_end; bm_port_id++) {
@@ -1470,7 +1489,8 @@ static int ppe_config_bm(struct ppe_device *ppe_dev)
 /* Configure PPE hardware queue depth, which is decided by the threshold
  * of queue.
  */
-static int ppe_config_qm(struct ppe_device *ppe_dev)
+static int ppe_config_qm(struct ppe_device *ppe_dev,
+			 const struct ppe_soc_config *soc_cfg)
 {
 	const struct ppe_qm_queue_config *queue_cfg;
 	int ret, i, queue_id, queue_cfg_count;
@@ -1485,15 +1505,15 @@ static int ppe_config_qm(struct ppe_device *ppe_dev)
 	if (ret)
 		goto qm_config_fail;
 
-	PPE_AC_GRP_SET_BUF_LIMIT(group_cfg, ipq9574_ppe_qm_group_config);
+	PPE_AC_GRP_SET_BUF_LIMIT(group_cfg, soc_cfg->qm_group_config);
 
 	ret = regmap_bulk_write(ppe_dev->regmap, reg,
 				group_cfg, ARRAY_SIZE(group_cfg));
 	if (ret)
 		goto qm_config_fail;
 
-	queue_cfg = ipq9574_ppe_qm_queue_config;
-	queue_cfg_count = ARRAY_SIZE(ipq9574_ppe_qm_queue_config);
+	queue_cfg = soc_cfg->qm_queue_config;
+	queue_cfg_count = soc_cfg->qm_queue_config_cnt;
 	for (i = 0; i < queue_cfg_count; i++) {
 		queue_id = queue_cfg[i].queue_start;
 
@@ -1625,7 +1645,8 @@ static int ppe_node_scheduler_config(struct ppe_device *ppe_dev,
 /* Initialize scheduler settings for PPE buffer utilization and dispatching
  * packet on PPE queue.
  */
-static int ppe_config_scheduler(struct ppe_device *ppe_dev)
+static int ppe_config_scheduler(struct ppe_device *ppe_dev,
+				const struct ppe_soc_config *soc_cfg)
 {
 	const struct ppe_scheduler_port_config *port_cfg;
 	const struct ppe_scheduler_qm_config *qm_cfg;
@@ -1633,8 +1654,8 @@ static int ppe_config_scheduler(struct ppe_device *ppe_dev)
 	int ret, i, count;
 	u32 val, reg;
 
-	count = ARRAY_SIZE(ipq9574_ppe_sch_bm_config);
-	bm_cfg = ipq9574_ppe_sch_bm_config;
+	count = soc_cfg->sch_bm_config_cnt;
+	bm_cfg = soc_cfg->sch_bm_config;
 
 	/* Configure the depth of BM scheduler entries. */
 	val = FIELD_PREP(PPE_BM_SCH_CTRL_SCH_DEPTH, count);
@@ -1664,8 +1685,8 @@ static int ppe_config_scheduler(struct ppe_device *ppe_dev)
 			goto sch_config_fail;
 	}
 
-	count = ARRAY_SIZE(ipq9574_ppe_sch_qm_config);
-	qm_cfg = ipq9574_ppe_sch_qm_config;
+	count = soc_cfg->sch_qm_config_cnt;
+	qm_cfg = soc_cfg->sch_qm_config;
 
 	/* Configure the depth of QM scheduler entries. */
 	val = FIELD_PREP(PPE_PSCH_SCH_DEPTH_CFG_SCH_DEPTH, count);
@@ -1995,19 +2016,20 @@ static int ppe_bridge_init(struct ppe_device *ppe_dev)
 	return 0;
 }
 
-int ppe_hw_config(struct ppe_device *ppe_dev)
+int ppe_hw_config(struct ppe_device *ppe_dev,
+		  const struct ppe_soc_config *soc_cfg)
 {
 	int ret;
 
-	ret = ppe_config_bm(ppe_dev);
+	ret = ppe_config_bm(ppe_dev, soc_cfg);
 	if (ret)
 		return ret;
 
-	ret = ppe_config_qm(ppe_dev);
+	ret = ppe_config_qm(ppe_dev, soc_cfg);
 	if (ret)
 		return ret;
 
-	ret = ppe_config_scheduler(ppe_dev);
+	ret = ppe_config_scheduler(ppe_dev, soc_cfg);
 	if (ret)
 		return ret;
 
diff --git a/drivers/net/ethernet/qualcomm/ppe/ppe_config.h b/drivers/net/ethernet/qualcomm/ppe/ppe_config.h
index 60493e51e0a4..0dafab264743 100644
--- a/drivers/net/ethernet/qualcomm/ppe/ppe_config.h
+++ b/drivers/net/ethernet/qualcomm/ppe/ppe_config.h
@@ -287,7 +287,39 @@ struct ppe_rss_hash_cfg {
 	u8 hash_fin_outer[PPE_RSS_HASH_TUPLES];
 };
 
-int ppe_hw_config(struct ppe_device *ppe_dev);
+/**
+ * struct ppe_soc_config - SoC-specific PPE hardware configuration.
+ * @bm_group_config: Buffer Management (BM) group configuration.
+ * @bm_port_config: Pointer to the BM port configuration array.
+ * @bm_port_config_cnt: Number of entries in the BM port configuration array.
+ * @qm_group_config: Queue Management (QM) group configuration.
+ * @qm_queue_config: Pointer to the QM queue configuration array.
+ * @qm_queue_config_cnt: Number of entries in the QM queue configuration array.
+ * @sch_bm_config: Pointer to the BM scheduler arbitration array.
+ * @sch_bm_config_cnt: Number of entries in the BM scheduler arbitration array.
+ * @sch_qm_config: Pointer to the QM scheduler arbitration array.
+ * @sch_qm_config_cnt: Number of entries in the QM scheduler arbitration array.
+ */
+struct ppe_soc_config {
+	int bm_group_config;
+	const struct ppe_bm_port_config *bm_port_config;
+	unsigned int bm_port_config_cnt;
+
+	int qm_group_config;
+	const struct ppe_qm_queue_config *qm_queue_config;
+	unsigned int qm_queue_config_cnt;
+
+	const struct ppe_scheduler_bm_config *sch_bm_config;
+	unsigned int sch_bm_config_cnt;
+
+	const struct ppe_scheduler_qm_config *sch_qm_config;
+	unsigned int sch_qm_config_cnt;
+};
+
+extern const struct ppe_soc_config ipq9574_soc_config;
+
+int ppe_hw_config(struct ppe_device *ppe_dev,
+		  const struct ppe_soc_config *soc_cfg);
 int ppe_queue_scheduler_set(struct ppe_device *ppe_dev,
 			    int node_id, bool flow_level, int port,
 			    struct ppe_scheduler_cfg scheduler_cfg);
-- 
2.56.0

[PATCH net-next 2/4] dt-bindings: net: qcom,ipq9574-ppe: Tighten ethernet port node constraints

[Til Kaiser]

The port node pattern "^ethernet-port@[1-6]+$" also matches multi-digit
unit addresses such as ethernet-port@11, which the reg property does not
allow. Drop the stray '+'.

Also restrict the ethernet-ports object to its declared properties.
Without this, nodes outside the port pattern are accepted through the
generic ethernet-switch schema.

Signed-off-by: Til Kaiser <mail@tk154.de>
---
 .../devicetree/bindings/net/qcom,ipq9574-ppe.yaml     | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/qcom,ipq9574-ppe.yaml b/Documentation/devicetree/bindings/net/qcom,ipq9574-ppe.yaml
index 6d0b21a10732..1a78e2c82b90 100644
--- a/Documentation/devicetree/bindings/net/qcom,ipq9574-ppe.yaml
+++ b/Documentation/devicetree/bindings/net/qcom,ipq9574-ppe.yaml
@@ -174,8 +174,17 @@ properties:
       - interrupt-names
 
   ethernet-ports:
+    type: object
+    additionalProperties: false
+
+    properties:
+      '#address-cells':
+        const: 1
+      '#size-cells':
+        const: 0
+
     patternProperties:
-      "^ethernet-port@[1-6]+$":
+      "^ethernet-port@[1-6]$":
         type: object
         unevaluatedProperties: false
         $ref: ethernet-switch-port.yaml#
-- 
2.56.0

Re: [PATCH net-next 2/4] dt-bindings: net: qcom,ipq9574-ppe: Tighten ethernet port node constraints

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 0 · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] This patch only closes off the `ethernet-ports` spelling of the
  port container.
- [Low] The commit fixes schema-validation defects already present in the
  binding, but it has no `Fixes:` tag.

--

dt-bindings: net: qcom,ipq9574-ppe: Tighten ethernet port node constraints

This patch drops the stray '+' from the ethernet-port node pattern in the
qcom,ipq9574-ppe binding, so only single-digit unit addresses match. It
also gives the ethernet-ports container type: object and
additionalProperties: false, plus explicit #address-cells and #size-cells
constraints.

> Also restrict the ethernet-ports object to its declared properties.
> Without this, nodes outside the port pattern are accepted through the
> generic ethernet-switch schema.
>
> Signed-off-by: Til Kaiser <mail@tk154.de>

[Severity: Low]
This fixes validation problems that have been in the binding since it was
added. Should it carry a Fixes: tag?

Both the unclosed ethernet-ports block and the "^ethernet-port@[1-6]+$"
pattern come from this commit:

Fixes: 1898fc572118 ("dt-bindings: net: Add PPE for Qualcomm IPQ9574 SoC")

> diff --git a/Documentation/devicetree/bindings/net/qcom,ipq9574-ppe.yaml b/Documentation/devicetree/bindings/net/qcom,ipq9574-ppe.yaml
> index 6d0b21a10732a..1a78e2c82b90c 100644
> --- a/Documentation/devicetree/bindings/net/qcom,ipq9574-ppe.yaml
> +++ b/Documentation/devicetree/bindings/net/qcom,ipq9574-ppe.yaml
> @@ -174,8 +174,17 @@ properties:
>        - interrupt-names
>  
>    ethernet-ports:
> +    type: object
> +    additionalProperties: false
> +
> +    properties:
> +      '#address-cells':
> +        const: 1
> +      '#size-cells':
> +        const: 0
> +
>      patternProperties:
> -      "^ethernet-port@[1-6]+$":
> +      "^ethernet-port@[1-6]$":

[Severity: High]
This closes off the ethernet-ports container. Can the same nodes still get
through if the container is named ports instead?

The binding pulls in the generic schema:

    allOf:
      - $ref: ethernet-switch.yaml

ethernet-switch.yaml accepts either container name, and accepts any
port@ or ethernet-port@ child with a hex unit address:

    '^(ethernet-)?ports$':
      type: object
      unevaluatedProperties: false
      ...
      patternProperties:
        '^(ethernet-)?port@[0-9a-f]+$':
          type: object
          description: Ethernet switch ports
    ...
    oneOf:
      - required:
          - ports
      - required:
          - ethernet-ports

qcom,ipq9574-ppe.yaml has no ports entry, and it does not list
ethernet-ports under its top-level required: list. Because ports is
evaluated through the allOf $ref, the top-level unevaluatedProperties:
false does not reject it either.

So it looks like a PPE node like this would still validate:

    ports {
        #address-cells = <1>;
        #size-cells = <0>;
        port@7 { };
        ethernet-port@11 { };
    };

None of the PPE per-port constraints apply to that node. That covers the
reg 1..6 range, the required clocks/clock-names/resets/reset-names, the
unevaluatedProperties: false on the port node, and the new single-digit
pattern.

That means the case described in the commit message, "nodes outside the
port pattern are accepted through the generic ethernet-switch schema", can
still happen.

The later patch in this series, "dt-bindings: net: qcom,ipq9574-ppe: Add
IPQ5332 support", also constrains only ethernet-ports in its IPQ5332
if/then block. That includes the "^ethernet-port@[3-6]$": false rule, so
ports { port@3 { }; } would also get past the IPQ5332 port-count limit.

Other bindings with per-port constraints match '^(ethernet-)?ports$', so
both spellings are covered. Examples are dsa/qca8k.yaml,
dsa/mediatek,mt7530.yaml, dsa/microchip,ksz.yaml and dsa/nxp,sja1105.yaml.

Could this binding do the same? Another option would be to forbid the
other spelling with ports: false, and add ethernet-ports to the top-level
required: list.

>          type: object
>          unevaluatedProperties: false
>          $ref: ethernet-switch-port.yaml#

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261005152915.101780-1-mail%40tk154.de

[PATCH net-next 3/4] dt-bindings: net: qcom,ipq9574-ppe: Add IPQ5332 support

[Til Kaiser]

IPQ5332 uses the same PPE architecture as IPQ9574, but has two front
panel ports instead of six and four interconnect paths instead of seven.

Add the qcom,ipq5332-ppe compatible and constrain the interconnects
and port nodes per SoC: IPQ5332 takes four interconnects and only
ethernet-port@1 and ethernet-port@2, while IPQ9574 keeps seven
interconnects.

The PPE core, EDMA and MAC clock functions are shared, but IPQ5332 also
needs the GCC IM sleep clock. The PPE core uses one reset and each MAC
uses three resets on both SoCs. IPQ5332 EDMA needs separate system and
configuration resets; IPQ9574 uses one combined reset. Constrain the
clock and reset counts accordingly. Keep EDMA resets positional, with
the system reset first and the configuration reset second on IPQ5332,
consistent with the existing unnamed IPQ9574 reset.

IPQ5332 EDMA has 24 TX/completion rings, 16 RX descriptor rings and eight
RX fill rings, so the full EDMA interrupt set comprises 49 interrupts.
Constrain the interrupt counts and ring names per SoC as well.

The IPQ5332 clock and reset functions are described by Qualcomm's NSSCC
driver submission. Its sleep clock is also listed in Qualcomm's
PPE DTS submission. The ring limits come from QSDK 14.0.1595.1665,
qca-nss-dp hal/soc_ops/ipq53xx/nss_ipq53xx.h.

Link: https://lore.kernel.org/r/20240829082830.56959-7-quic_varada@quicinc.com
Link: https://lore.kernel.org/r/20240110112059.2498-3-quic_luoj@quicinc.com

Signed-off-by: Til Kaiser <mail@tk154.de>
---
 .../bindings/net/qcom,ipq9574-ppe.yaml        | 92 +++++++++++++++++--
 1 file changed, 84 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/qcom,ipq9574-ppe.yaml b/Documentation/devicetree/bindings/net/qcom,ipq9574-ppe.yaml
index 1a78e2c82b90..29dd34210ac8 100644
--- a/Documentation/devicetree/bindings/net/qcom,ipq9574-ppe.yaml
+++ b/Documentation/devicetree/bindings/net/qcom,ipq9574-ppe.yaml
@@ -28,11 +28,15 @@ description: |
   supporting various modes such as SGMII/QSGMII/PSGMII/USXGMII/10G-BASER. There
   are 3 UNIPHY (PCS) instances supported on the IPQ9574.
 
-  Ethernet DMA is used to transmit and receive packets between the six Ethernet
+  IPQ5332 uses the same PPE architecture with two front panel ports and two
+  UNIPHY (PCS) instances. Its Ethernet datapath has a CPU port (port0) and
+  two GMAC/XGMAC ports (port1 and port2).
+
+  Ethernet DMA is used to transmit and receive packets between the Ethernet
   ports and ARM host CPU.
 
-  The follow diagram shows the PPE hardware block along with its connectivity
-  to the external hardware blocks such clock hardware blocks (CMNPLL, GCC,
+  The following diagram shows the IPQ9574 PPE hardware block and its
+  connectivity to external clock hardware blocks (CMNPLL, GCC,
   NSS clock controller) and Ethernet PCS/PHY blocks. For depicting the PHY
   connectivity, one 4x1 Gbps PHY (QCA8075) and two 10 GBps PHYs are used as an
   example.
@@ -77,24 +81,29 @@ description: |
 properties:
   compatible:
     enum:
+      - qcom,ipq5332-ppe
       - qcom,ipq9574-ppe
 
   reg:
     maxItems: 1
 
   clocks:
+    minItems: 4
     items:
       - description: PPE core clock
       - description: PPE APB (Advanced Peripheral Bus) clock
       - description: PPE IPE (Ingress Process Engine) clock
       - description: PPE BM, QM and scheduler clock
+      - description: PPE sleep clock from GCC
 
   clock-names:
+    minItems: 4
     items:
       - const: ppe
       - const: apb
       - const: ipe
       - const: btq
+      - const: sleep
 
   resets:
     maxItems: 1
@@ -105,6 +114,7 @@ properties:
     description: PPE switch miscellaneous interrupt
 
   interconnects:
+    minItems: 4
     items:
       - description: Bus interconnect path leading to PPE switch core function
       - description: Bus interconnect path leading to PPE register access
@@ -115,6 +125,7 @@ properties:
       - description: Bus interconnect path leading to enhanced memory NOC from NSS NOC
 
   interconnect-names:
+    minItems: 4
     items:
       - const: ppe
       - const: ppe_cfg
@@ -130,7 +141,9 @@ properties:
     description:
       EDMA (Ethernet DMA) is used to transmit packets between PPE and ARM
       host CPU. There are 32 TX descriptor rings, 32 TX completion rings,
-      24 RX descriptor rings and 8 RX fill rings supported.
+      24 RX descriptor rings and 8 RX fill rings supported on IPQ9574.
+      IPQ5332 supports 24 TX descriptor rings, 24 TX completion rings,
+      16 RX descriptor rings and 8 RX fill rings.
 
     properties:
       clocks:
@@ -144,15 +157,18 @@ properties:
           - const: apb
 
       resets:
-        maxItems: 1
-        description: EDMA reset
+        minItems: 1
+        items:
+          - description:
+              EDMA system reset. On IPQ9574 this also resets the configuration.
+          - description: EDMA configuration reset
 
       interrupts:
-        minItems: 65
+        minItems: 49
         maxItems: 65
 
       interrupt-names:
-        minItems: 65
+        minItems: 49
         maxItems: 65
         items:
           oneOf:
@@ -165,6 +181,7 @@ properties:
           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.
+          On IPQ5332, only "txcmpl_[0-23]" and "rxdesc_[0-15]" are supported.
 
     required:
       - clocks
@@ -238,6 +255,65 @@ required:
 
 allOf:
   - $ref: ethernet-switch.yaml
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: qcom,ipq5332-ppe
+    then:
+      properties:
+        clocks:
+          minItems: 5
+        clock-names:
+          minItems: 5
+        interconnects:
+          maxItems: 4
+        interconnect-names:
+          maxItems: 4
+        ethernet-dma:
+          properties:
+            resets:
+              minItems: 2
+            interrupts:
+              maxItems: 49
+            interrupt-names:
+              maxItems: 49
+              items:
+                oneOf:
+                  - pattern: '^txcmpl_(1?[0-9]|2[0-3])$'
+                  - pattern: '^rxfill_[0-7]$'
+                  - pattern: '^rxdesc_([0-9]|1[0-5])$'
+                  - const: misc
+        ethernet-ports:
+          patternProperties:
+            "^ethernet-port@[1-2]$":
+              properties:
+                reg:
+                  maximum: 2
+            "^ethernet-port@[3-6]$": false
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: qcom,ipq9574-ppe
+    then:
+      properties:
+        clocks:
+          maxItems: 4
+        clock-names:
+          maxItems: 4
+        interconnects:
+          minItems: 7
+        interconnect-names:
+          minItems: 7
+        ethernet-dma:
+          properties:
+            resets:
+              maxItems: 1
+            interrupts:
+              minItems: 65
+            interrupt-names:
+              minItems: 65
 
 unevaluatedProperties: false
 
-- 
2.56.0

Re: [PATCH net-next 3/4] dt-bindings: net: qcom,ipq9574-ppe: Add IPQ5332 support

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The IPQ5332 port limits (`"^ethernet-port@[3-6]$": false` and
  `reg: maximum: 2`) apply only under the `ethernet-ports` container…

--

dt-bindings: net: qcom,ipq9574-ppe: Add IPQ5332 support

This adds the qcom,ipq5332-ppe compatible to the IPQ9574 PPE binding. It
adds an optional GCC sleep clock and a second EDMA reset, and lowers the
minimum interconnect and EDMA interrupt counts. New per-SoC if/then blocks
constrain those counts, and limit IPQ5332 to ethernet-port@1 and
ethernet-port@2.

Link: https://lore.kernel.org/r/20240829082830.56959-7-quic_varada@quicinc.com
Link: https://lore.kernel.org/r/20240110112059.2498-3-quic_luoj@quicinc.com

> diff --git a/Documentation/devicetree/bindings/net/qcom,ipq9574-ppe.yaml b/Documentation/devicetree/bindings/net/qcom,ipq9574-ppe.yaml
> index 1a78e2c82b90c..29dd34210ac89 100644
> --- a/Documentation/devicetree/bindings/net/qcom,ipq9574-ppe.yaml
> +++ b/Documentation/devicetree/bindings/net/qcom,ipq9574-ppe.yaml

[ ... ]

> @@ -238,6 +255,65 @@ required:
>  
>  allOf:
>    - $ref: ethernet-switch.yaml
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: qcom,ipq5332-ppe
> +    then:

[ ... ]

> +        ethernet-ports:
> +          patternProperties:
> +            "^ethernet-port@[1-2]$":
> +              properties:
> +                reg:
> +                  maximum: 2
> +            "^ethernet-port@[3-6]$": false

[Severity: Low]
Is the IPQ5332 port limit enforced when the node uses "ports" as the
container name instead of "ethernet-ports"?

The new constraints only apply under ethernet-ports. The binding also
pulls in ethernet-switch.yaml through allOf, and that schema accepts
either container name:

Documentation/devicetree/bindings/net/ethernet-switch.yaml:
    patternProperties:
      '^(ethernet-)?ports$':
        ...
        patternProperties:
          '^(ethernet-)?port@[0-9a-f]+$':
            type: object
    ...
    oneOf:
      - required:
          - ports
      - required:
          - ethernet-ports

The referenced schema evaluates "ports", so the top-level
unevaluatedProperties: false in qcom,ipq9574-ppe.yaml does not reject it.
The PPE binding also does not list ethernet-ports in its required list.

So an IPQ5332 node like this would seem to pass dt_binding_check:

    ports {
        #address-cells = <1>;
        #size-cells = <0>;
        ethernet-port@3 {
            reg = <3>;
        };
    };

That would not match the commit message, which says IPQ5332 takes "only
ethernet-port@1 and ethernet-port@2".

The same gap already lets nodes skip the shared per-port clocks and resets
requirements for IPQ9574. The earlier patch in this series that tightened
the port node constraints also covered only ethernet-ports.

Would it make sense to forbid ports (for example ports: false), require
ethernet-ports, or apply the constraints to both spellings?

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261005152915.101780-1-mail%40tk154.de

[PATCH net-next 4/4] net: ethernet: qualcomm: ppe: Add IPQ5332 support

[Til Kaiser]

IPQ5332 shares the IPQ9574 PPE architecture, but has only two front
panel ports and uses a different clock rate, interconnect paths, buffer
limits and scheduler arbitration.

Add match data for qcom,ipq5332-ppe: a 200 MHz PPE clock, four
interconnect paths and three PPE ports (the CPU port and two front panel
ports), together with its BM and QM limits and thresholds and its
scheduler arbitration tables. The values come from Qualcomm's QSDK
14.0.1595.1665 qca-ssdk revision 48484d531927b070b996685f1570aff3bccb9c32:
src/init/ssdk_hppe.c and ssdk_appe.c, and src/adpt/hppe/adpt_hppe_qm.c.

Reuse the IPQ9574 scheduler resources for ports 0 (CPU), 1 and 2.
The queue, flow and DRR node ranges for ports 1 and 2 match the
IPQ5332 port_scheduler_resource definitions in QSDK ipq5332.dtsi.
The CPU flow and DRR node ranges also match; retain the IPQ9574 CPU
and reserved queue partition. QSDK assigns CPU unicast queues 0-95
and multicast queues 256-263, while this driver uses 0-63 and 256-271.
The reserved pool is selected from the final resource-table entry
using port ID 3 on IPQ5332.

QSDK uses the same block base addresses and HPPE register offsets
for the common PPE blocks on IPQ5332 and IPQ9574, so retain the
existing regmap ranges and offsets for those blocks.

Document the IPQ5332 port and UNIPHY counts and add it to the list of
supported SoCs.

Signed-off-by: Til Kaiser <mail@tk154.de>
---
 .../ethernet/qualcomm/ppe/ppe.rst             |   9 +-
 drivers/net/ethernet/qualcomm/ppe/ppe.c       |  33 +++++
 .../net/ethernet/qualcomm/ppe/ppe_config.c    | 127 ++++++++++++++++++
 .../net/ethernet/qualcomm/ppe/ppe_config.h    |   1 +
 4 files changed, 169 insertions(+), 1 deletion(-)

diff --git a/Documentation/networking/device_drivers/ethernet/qualcomm/ppe/ppe.rst b/Documentation/networking/device_drivers/ethernet/qualcomm/ppe/ppe.rst
index 4ab299a28969..caf8ab1431de 100644
--- a/Documentation/networking/device_drivers/ethernet/qualcomm/ppe/ppe.rst
+++ b/Documentation/networking/device_drivers/ethernet/qualcomm/ppe/ppe.rst
@@ -96,6 +96,10 @@ The port wrapper provides connections from the 6 GMAC/XGMACS to UNIPHY (PCS) sup
 various modes such as SGMII/QSGMII/PSGMII/USXGMII/10G-BASER. There are 3 UNIPHY (PCS)
 instances supported on the IPQ9574.
 
+IPQ5332 uses the same PPE architecture with two front panel ports and two UNIPHY
+(PCS) instances. Its Ethernet datapath has a CPU port (port0) and two GMAC/XGMAC
+ports (port1 and port2).
+
 Ethernet DMA is used to transmit and receive packets between the Ethernet subsystem
 and ARM host CPU.
 
@@ -118,6 +122,8 @@ PPE driver:
     through the UNIPHY PCS block. Each MAC block includes the GMAC and XGMAC blocks and
     the switch port can select to use GMAC or XMAC through a MUX selection according to
     the external PHY's capability.
+    IPQ5332 has two MACs (MAC0 and MAC1), connected to switch ports port1 and
+    port2 through two UNIPHY instances.
 - EDMA (Ethernet DMA)
     The Ethernet DMA is used to transmit and receive Ethernet packets between the PPE
     ports and the ARM cores.
@@ -164,8 +170,9 @@ CPU code counters and queue counters.
 PPE Driver Supported SoCs
 =========================
 
-The PPE driver supports the following IPQ SoC:
+The PPE driver supports the following IPQ SoCs:
 
+- IPQ5332
 - IPQ9574
 
 
diff --git a/drivers/net/ethernet/qualcomm/ppe/ppe.c b/drivers/net/ethernet/qualcomm/ppe/ppe.c
index 0ef72c20364b..69e970db4a78 100644
--- a/drivers/net/ethernet/qualcomm/ppe/ppe.c
+++ b/drivers/net/ethernet/qualcomm/ppe/ppe.c
@@ -26,6 +26,30 @@ struct ppe_plat_data {
 	unsigned int num_icc, num_ports;
 };
 
+/* IPQ5332 interconnect paths. Zero bandwidths use the PPE clock rate. */
+static const struct icc_bulk_data ipq5332_icc_data[] = {
+	{
+		.name = "ppe",
+		.avg_bw = 0,
+		.peak_bw = 0,
+	},
+	{
+		.name = "ppe_cfg",
+		.avg_bw = 0,
+		.peak_bw = 0,
+	},
+	{
+		.name = "qos_gen",
+		.avg_bw = 6000,
+		.peak_bw = 6000,
+	},
+	{
+		.name = "timeout_ref",
+		.avg_bw = 6000,
+		.peak_bw = 6000,
+	},
+};
+
 /* IPQ9574 interconnect paths. Zero bandwidths use the PPE clock rate. */
 static const struct icc_bulk_data ipq9574_icc_data[] = {
 	{
@@ -65,6 +89,14 @@ static const struct icc_bulk_data ipq9574_icc_data[] = {
 	},
 };
 
+static const struct ppe_plat_data ipq5332_ppe_data = {
+	.soc_cfg = &ipq5332_soc_config,
+	.icc_data = ipq5332_icc_data,
+	.ppe_clk_rate = 200000000,
+	.num_icc = ARRAY_SIZE(ipq5332_icc_data),
+	.num_ports = 3,
+};
+
 static const struct ppe_plat_data ipq9574_ppe_data = {
 	.soc_cfg = &ipq9574_soc_config,
 	.icc_data = ipq9574_icc_data,
@@ -237,6 +269,7 @@ static void qcom_ppe_remove(struct platform_device *pdev)
 }
 
 static const struct of_device_id qcom_ppe_of_match[] = {
+	{ .compatible = "qcom,ipq5332-ppe", .data = &ipq5332_ppe_data },
 	{ .compatible = "qcom,ipq9574-ppe", .data = &ipq9574_ppe_data },
 	{}
 };
diff --git a/drivers/net/ethernet/qualcomm/ppe/ppe_config.c b/drivers/net/ethernet/qualcomm/ppe/ppe_config.c
index 65ac6c0841d5..91e6ee9524a6 100644
--- a/drivers/net/ethernet/qualcomm/ppe/ppe_config.c
+++ b/drivers/net/ethernet/qualcomm/ppe/ppe_config.c
@@ -174,6 +174,9 @@ struct ppe_port_schedule_resource {
  */
 static const int ipq9574_ppe_bm_group_config = 1550;
 
+/* IPQ5332 BM group0 shared buffer limit. */
+static const int ipq5332_ppe_bm_group_config = 360;
+
 /* The buffer configurations per PPE port. There are 15 BM ports and
  * 4 BM groups supported by PPE. BM port (0-7) is for EDMA port 0,
  * BM port (8-13) is for PPE physical port 1-6 and BM port 14 is for
@@ -230,6 +233,34 @@ static const struct ppe_bm_port_config ipq9574_ppe_bm_port_config[] = {
 	},
 };
 
+/* IPQ5332: BM ports 0-7 serve EDMA, ports 8-9 serve ports 1-2. */
+static const struct ppe_bm_port_config ipq5332_ppe_bm_port_config[] = {
+	{
+		/* Buffer configuration for the BM port ID 0-7 of EDMA. */
+		.port_id_start	= 0,
+		.port_id_end	= 7,
+		.pre_alloc	= 12,
+		.in_fly_buf	= 40,
+		.ceil		= 30,
+		.weight		= 7,
+		.resume_offset	= 5,
+		.resume_ceil	= 20,
+		.dynamic	= true,
+	},
+	{
+		/* Buffer configuration for the BM port ID 8-9 of PPE ports. */
+		.port_id_start	= 8,
+		.port_id_end	= 9,
+		.pre_alloc	= 12,
+		.in_fly_buf	= 128,
+		.ceil		= 48,
+		.weight		= 7,
+		.resume_offset	= 5,
+		.resume_ceil	= 20,
+		.dynamic	= true,
+	},
+};
+
 /* QM fetches the packet from PPE buffer management for transmitting the
  * packet out. The QM group configuration limits the total number of buffers
  * enqueued by all PPE hardware queues.
@@ -240,6 +271,9 @@ static const struct ppe_bm_port_config ipq9574_ppe_bm_port_config[] = {
  */
 static const int ipq9574_ppe_qm_group_config = 2000;
 
+/* IPQ5332 QM group0 buffer limit. */
+static const int ipq5332_ppe_qm_group_config = 500;
+
 /* Default QM settings for unicast and multicast queues for IPQ9754. */
 static const struct ppe_qm_queue_config ipq9574_ppe_qm_queue_config[] = {
 	{
@@ -264,6 +298,30 @@ static const struct ppe_qm_queue_config ipq9574_ppe_qm_queue_config[] = {
 	},
 };
 
+/* IPQ5332 unicast and multicast queue thresholds. */
+static const struct ppe_qm_queue_config ipq5332_ppe_qm_queue_config[] = {
+	{
+		/* QM settings for unicast queues 0 to 255. */
+		.queue_start	= 0,
+		.queue_end	= 255,
+		.prealloc_buf	= 0,
+		.ceil		= 300,
+		.weight		= 7,
+		.resume_offset	= 18,
+		.dynamic	= true,
+	},
+	{
+		/* QM settings for multicast queues 256 to 299. */
+		.queue_start	= 256,
+		.queue_end	= 299,
+		.prealloc_buf	= 0,
+		.ceil		= 50,
+		.weight		= 0,
+		.resume_offset	= 18,
+		.dynamic	= false,
+	},
+};
+
 /* PPE scheduler configuration for BM includes multiple entries. Each entry
  * indicates the primary port to be assigned the buffers for the ingress or
  * to release the buffers for the egress. Backup port ID will be used when
@@ -372,6 +430,44 @@ static const struct ppe_scheduler_bm_config ipq9574_ppe_sch_bm_config[] = {
 	{true, PPE_SCH_EGRESS,  7, false, 0},
 };
 
+/* IPQ5332 BM arbitration. The invalid final slot is included in the
+ * 32-slot scheduler depth.
+ */
+static const struct ppe_scheduler_bm_config ipq5332_ppe_sch_bm_config[] = {
+	{true,  PPE_SCH_INGRESS, 2, false, 0},
+	{true,  PPE_SCH_EGRESS,  0, false, 0},
+	{true,  PPE_SCH_INGRESS, 1, false, 0},
+	{true,  PPE_SCH_EGRESS,  1, false, 0},
+	{true,  PPE_SCH_INGRESS, 2, false, 0},
+	{true,  PPE_SCH_EGRESS,  2, false, 0},
+	{true,  PPE_SCH_INGRESS, 0, false, 0},
+	{true,  PPE_SCH_EGRESS,  0, false, 0},
+	{true,  PPE_SCH_INGRESS, 2, false, 0},
+	{true,  PPE_SCH_EGRESS,  1, false, 0},
+	{true,  PPE_SCH_INGRESS, 1, false, 0},
+	{true,  PPE_SCH_EGRESS,  2, false, 0},
+	{true,  PPE_SCH_INGRESS, 0, false, 0},
+	{true,  PPE_SCH_EGRESS,  0, false, 0},
+	{true,  PPE_SCH_INGRESS, 1, false, 0},
+	{true,  PPE_SCH_EGRESS,  1, false, 0},
+	{true,  PPE_SCH_INGRESS, 2, false, 0},
+	{true,  PPE_SCH_EGRESS,  2, false, 0},
+	{true,  PPE_SCH_INGRESS, 1, false, 0},
+	{true,  PPE_SCH_EGRESS,  0, false, 0},
+	{true,  PPE_SCH_INGRESS, 2, false, 0},
+	{true,  PPE_SCH_EGRESS,  1, false, 0},
+	{true,  PPE_SCH_INGRESS, 0, false, 0},
+	{true,  PPE_SCH_EGRESS,  2, false, 0},
+	{true,  PPE_SCH_INGRESS, 2, false, 0},
+	{true,  PPE_SCH_EGRESS,  0, false, 0},
+	{true,  PPE_SCH_INGRESS, 1, false, 0},
+	{true,  PPE_SCH_EGRESS,  1, false, 0},
+	{true,  PPE_SCH_INGRESS, 0, false, 0},
+	{true,  PPE_SCH_EGRESS,  2, false, 0},
+	{true,  PPE_SCH_INGRESS, 1, false, 0},
+	{false, PPE_SCH_INGRESS, 0, false, 0},
+};
+
 /* PPE scheduler configuration for QM includes multiple entries. Each entry
  * contains ports to be dispatched for enqueueing and dequeueing. The backup
  * port for dequeueing is supported to be used when the primary port for
@@ -439,6 +535,22 @@ static const struct ppe_scheduler_qm_config ipq9574_ppe_sch_qm_config[] = {
 	{0x98, 6, 5, true, 2},
 };
 
+/* IPQ5332 QM arbitration. The enqueue bitmap is zero in every slot;
+ * ensch_port identifies the enqueue port. Backup dequeue ports are disabled.
+ */
+static const struct ppe_scheduler_qm_config ipq5332_ppe_sch_qm_config[] = {
+	{0x0, 0x2, 0, false, 0x0},
+	{0x0, 0x1, 2, false, 0x0},
+	{0x0, 0x0, 1, false, 0x0},
+	{0x0, 0x0, 2, false, 0x0},
+	{0x0, 0x1, 0, false, 0x0},
+	{0x0, 0x2, 1, false, 0x0},
+	{0x0, 0x0, 2, false, 0x0},
+	{0x0, 0x0, 1, false, 0x0},
+	{0x0, 0x0, 2, false, 0x0},
+	{0x0, 0x0, 1, false, 0x0},
+};
+
 static const struct ppe_scheduler_port_config ppe_port_sch_config[] = {
 	{
 		.port		= 0,
@@ -732,6 +844,8 @@ static const struct ppe_scheduler_port_config ppe_port_sch_config[] = {
 
 /* The scheduler resource is applied to each PPE port, The resource
  * includes the unicast & multicast queues, flow nodes and DRR nodes.
+ * Entries 1 and 2 serve ports 1 and 2 on both IPQ5332 and IPQ9574.
+ * The reserved pool is the last entry.
  */
 static const struct ppe_port_schedule_resource ppe_scheduler_res[] = {
 	{	.ucastq_start	= 0,
@@ -835,6 +949,19 @@ static const struct ppe_port_schedule_resource ppe_scheduler_res[] = {
 	},
 };
 
+const struct ppe_soc_config ipq5332_soc_config = {
+	.bm_group_config = ipq5332_ppe_bm_group_config,
+	.bm_port_config = ipq5332_ppe_bm_port_config,
+	.bm_port_config_cnt = ARRAY_SIZE(ipq5332_ppe_bm_port_config),
+	.qm_group_config = ipq5332_ppe_qm_group_config,
+	.qm_queue_config = ipq5332_ppe_qm_queue_config,
+	.qm_queue_config_cnt = ARRAY_SIZE(ipq5332_ppe_qm_queue_config),
+	.sch_bm_config = ipq5332_ppe_sch_bm_config,
+	.sch_bm_config_cnt = ARRAY_SIZE(ipq5332_ppe_sch_bm_config),
+	.sch_qm_config = ipq5332_ppe_sch_qm_config,
+	.sch_qm_config_cnt = ARRAY_SIZE(ipq5332_ppe_sch_qm_config),
+};
+
 const struct ppe_soc_config ipq9574_soc_config = {
 	.bm_group_config = ipq9574_ppe_bm_group_config,
 	.bm_port_config = ipq9574_ppe_bm_port_config,
diff --git a/drivers/net/ethernet/qualcomm/ppe/ppe_config.h b/drivers/net/ethernet/qualcomm/ppe/ppe_config.h
index 0dafab264743..b42731aa3e7e 100644
--- a/drivers/net/ethernet/qualcomm/ppe/ppe_config.h
+++ b/drivers/net/ethernet/qualcomm/ppe/ppe_config.h
@@ -316,6 +316,7 @@ struct ppe_soc_config {
 	unsigned int sch_qm_config_cnt;
 };
 
+extern const struct ppe_soc_config ipq5332_soc_config;
 extern const struct ppe_soc_config ipq9574_soc_config;
 
 int ppe_hw_config(struct ppe_device *ppe_dev,
-- 
2.56.0

Re: [PATCH net-next 4/4] net: ethernet: qualcomm: ppe: Add IPQ5332 support

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] On IPQ5332, the comment in ppe_clock_init_and_reset() (ppe.c
  lines 187-190) is no longer true.

Pre-existing issues:
- [Medium] This problem wasn't introduced by this patch, but the patch
  depends on it.

--

net: ethernet: qualcomm: ppe: Add IPQ5332 support

This patch adds qcom,ipq5332-ppe match data to the PPE driver: a 200 MHz
PPE clock, four interconnect paths and three ports. It also adds IPQ5332
specific BM/QM limits, thresholds and scheduler arbitration tables, and
reuses the IPQ9574 scheduler resource table for ports 0-2.

> diff --git a/drivers/net/ethernet/qualcomm/ppe/ppe.c b/drivers/net/ethernet/qualcomm/ppe/ppe.c
> index 0ef72c20364be..69e970db4a78b 100644
> --- a/drivers/net/ethernet/qualcomm/ppe/ppe.c
> +++ b/drivers/net/ethernet/qualcomm/ppe/ppe.c

[ ... ]

> @@ -65,6 +89,14 @@ static const struct icc_bulk_data ipq9574_icc_data[] = {
>  	},
>  };
>  
> +static const struct ppe_plat_data ipq5332_ppe_data = {
> +	.soc_cfg = &ipq5332_soc_config,
> +	.icc_data = ipq5332_icc_data,
> +	.ppe_clk_rate = 200000000,

[Severity: Low]
This isn't a bug, but now that IPQ5332 goes through
ppe_clock_init_and_reset(), is the comment there still accurate?

drivers/net/ethernet/qualcomm/ppe/ppe.c:ppe_clock_init_and_reset() {
    ...
	/* The PPE clocks have a common parent clock. Setting the clock
	 * rate of "ppe" ensures the clock rate of all PPE clocks is
	 * configured to the same rate.
	 */
	clk = devm_clk_get(dev, "ppe");
    ...
}

In this series, the qcom,ipq9574-ppe.yaml binding requires a fifth clock
for qcom,ipq5332-ppe. It is described as "PPE sleep clock from GCC" and
does not share the NSSCC parent of the other PPE clocks.

devm_clk_bulk_get_all_enabled() enables that clock at its own rate. Only
"ppe" gets clk_set_rate().

Could the comment be updated so it no longer claims that all PPE clocks
end up at the same rate?

> +	.num_icc = ARRAY_SIZE(ipq5332_icc_data),
> +	.num_ports = 3,
> +};

[ ... ]

> diff --git a/drivers/net/ethernet/qualcomm/ppe/ppe_config.c b/drivers/net/ethernet/qualcomm/ppe/ppe_config.c
> index 65ac6c0841d5a..91e6ee9524a60 100644
> --- a/drivers/net/ethernet/qualcomm/ppe/ppe_config.c
> +++ b/drivers/net/ethernet/qualcomm/ppe/ppe_config.c

[ ... ]

> @@ -264,6 +298,30 @@ static const struct ppe_qm_queue_config ipq9574_ppe_qm_queue_config[] = {
>  	},
>  };
>  
> +/* IPQ5332 unicast and multicast queue thresholds. */
> +static const struct ppe_qm_queue_config ipq5332_ppe_qm_queue_config[] = {

[ ... ]

> +	{
> +		/* QM settings for multicast queues 256 to 299. */
> +		.queue_start	= 256,
> +		.queue_end	= 299,
> +		.prealloc_buf	= 0,
> +		.ceil		= 50,
> +		.weight		= 0,
> +		.resume_offset	= 18,
> +		.dynamic	= false,
> +	},
> +};

[Severity: Medium]
This is a pre-existing issue, but do these multicast thresholds ever reach
the hardware? ppe_config_qm() indexes the multicast admission control
table with the global queue ID:

drivers/net/ethernet/qualcomm/ppe/ppe_config.c:ppe_config_qm() {
    ...
	} else {
		reg = PPE_AC_MULTICAST_QUEUE_CFG_TBL_ADDR +
		      PPE_AC_MULTICAST_QUEUE_CFG_TBL_INC * queue_id;
    ...
}

The multicast config table is a separate table with 44 entries:

#define PPE_AC_MULTICAST_QUEUE_CFG_TBL_ADDR	0x84a000
#define PPE_AC_MULTICAST_QUEUE_CFG_TBL_ENTRIES	44
#define PPE_AC_MULTICAST_QUEUE_CFG_TBL_INC	0x10

With that indexing, queue 256 maps to 0x84b000 and queue 299 maps to
0x84b2b0. Both addresses are past the end of the table at 0x84a2c0 and
before PPE_AC_GRP_CFG_TBL_ADDR (0x84c000).

The debugfs code for the matching multicast counter table subtracts the
unicast entry count first:

drivers/net/ethernet/qualcomm/ppe/ppe_debugfs.c:ppe_queue_counter_get() {
    ...
			int mq_offset = i - PPE_AC_UNICAST_QUEUE_CFG_TBL_ENTRIES;
    ...
}

If that is the intended indexing, neither these IPQ5332 values (ceil 50,
resume_offset 18) nor the IPQ9574 ones are ever written. The real
multicast entries keep their reset values, and the read-modify-write
cycles hit undocumented QM addresses instead.

This looks like it goes back to commit 806268dc7efd ("net: ethernet:
qualcomm: Initialize PPE queue management for IPQ9574").

Should ppe_config_qm() use queue_id - PPE_AC_UNICAST_QUEUE_CFG_TBL_ENTRIES
as the multicast table index?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261005152915.101780-1-mail%40tk154.de