Message-ID: <58e05149-abc2-4cf4-a6e8-35380823d94a@oss.qualcomm.com>
Date: Mon, 10 Feb 2025 10:12:37 +0800
From: Jie Gan <jie.gan@....qualcomm.com>
To: Luo Jie <quic_luoj@...cinc.com>, Andrew Lunn <andrew+netdev@...n.ch>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>, Rob Herring <robh@...nel.org>,
        Krzysztof Kozlowski <krzk+dt@...nel.org>,
        Conor Dooley <conor+dt@...nel.org>, Lei Wei <quic_leiwei@...cinc.com>,
        Suruchi Agarwal <quic_suruchia@...cinc.com>,
        Pavithra R <quic_pavir@...cinc.com>, Simon Horman <horms@...nel.org>,
        Jonathan Corbet <corbet@....net>, Kees Cook <kees@...nel.org>,
        "Gustavo A. R. Silva" <gustavoars@...nel.org>,
        Philipp Zabel <p.zabel@...gutronix.de>
Cc: linux-arm-msm@...r.kernel.org, netdev@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-doc@...r.kernel.org, linux-hardening@...r.kernel.org,
        quic_kkumarcs@...cinc.com, quic_linchen@...cinc.com,
        srinivas.kandagatla@...aro.org, bartosz.golaszewski@...aro.org,
        john@...ozen.org
Subject: Re: [PATCH net-next v3 03/14] net: ethernet: qualcomm: Add PPE driver
 for IPQ9574 SoC



On 2/9/2025 10:29 PM, Luo Jie wrote:
> The PPE (Packet Process Engine) hardware block is available
> on Qualcomm IPQ SoC that support PPE architecture, such as
> IPQ9574.
> 
> The PPE in IPQ9574 includes six integrated ethernet MAC
> (for 6 PPE ports), buffer management, queue management and
> scheduler functions. The MACs can connect with the external
> PHY or switch devices using the UNIPHY PCS block available
> in the SoC.
> 
> The PPE also includes various packet processing offload
> capabilities such as L3 routing and L2 bridging, VLAN and
> tunnel processing offload. It also includes Ethernet DMA
> function for transferring packets between ARM cores and
> PPE ethernet ports.
> 
> This patch adds the base source files and Makefiles for
> the PPE driver such as platform driver registration,
> clock initialization, and PPE reset routines.
> 
> Signed-off-by: Luo Jie <quic_luoj@...cinc.com>
> ---
>   drivers/net/ethernet/qualcomm/Kconfig      |  15 ++
>   drivers/net/ethernet/qualcomm/Makefile     |   1 +
>   drivers/net/ethernet/qualcomm/ppe/Makefile |   7 +
>   drivers/net/ethernet/qualcomm/ppe/ppe.c    | 218 +++++++++++++++++++++++++++++
>   drivers/net/ethernet/qualcomm/ppe/ppe.h    |  36 +++++
>   5 files changed, 277 insertions(+)
> 
> diff --git a/drivers/net/ethernet/qualcomm/Kconfig b/drivers/net/ethernet/qualcomm/Kconfig
> index 9210ff360fdc..59931c4edbeb 100644
> --- a/drivers/net/ethernet/qualcomm/Kconfig
> +++ b/drivers/net/ethernet/qualcomm/Kconfig
> @@ -61,6 +61,21 @@ config QCOM_EMAC
>   	  low power, Receive-Side Scaling (RSS), and IEEE 1588-2008
>   	  Precision Clock Synchronization Protocol.
>   
> +config QCOM_PPE
> +	tristate "Qualcomm Technologies, Inc. PPE Ethernet support"
> +	depends on HAS_IOMEM && OF
> +	depends on COMMON_CLK
> +	select REGMAP_MMIO
> +	help
> +	  This driver supports the Qualcomm Technologies, Inc. packet
> +	  process engine (PPE) available with IPQ SoC. The PPE includes
> +	  the ethernet MACs, Ethernet DMA (EDMA) and switch core that
> +	  supports L3 flow offload, L2 switch function, RSS and tunnel
> +	  offload.
> +
> +	  To compile this driver as a module, choose M here. The module
> +	  will be called qcom-ppe.
> +
>   source "drivers/net/ethernet/qualcomm/rmnet/Kconfig"
>   
>   endif # NET_VENDOR_QUALCOMM
> diff --git a/drivers/net/ethernet/qualcomm/Makefile b/drivers/net/ethernet/qualcomm/Makefile
> index 9250976dd884..166a59aea363 100644
> --- a/drivers/net/ethernet/qualcomm/Makefile
> +++ b/drivers/net/ethernet/qualcomm/Makefile
> @@ -11,4 +11,5 @@ qcauart-objs := qca_uart.o
>   
>   obj-y += emac/
>   
> +obj-$(CONFIG_QCOM_PPE) += ppe/
>   obj-$(CONFIG_RMNET) += rmnet/
> diff --git a/drivers/net/ethernet/qualcomm/ppe/Makefile b/drivers/net/ethernet/qualcomm/ppe/Makefile
> new file mode 100644
> index 000000000000..63d50d3b4f2e
> --- /dev/null
> +++ b/drivers/net/ethernet/qualcomm/ppe/Makefile
> @@ -0,0 +1,7 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +#
> +# Makefile for the device driver of PPE (Packet Process Engine) in IPQ SoC
> +#
> +
> +obj-$(CONFIG_QCOM_PPE) += qcom-ppe.o
> +qcom-ppe-objs := ppe.o
> diff --git a/drivers/net/ethernet/qualcomm/ppe/ppe.c b/drivers/net/ethernet/qualcomm/ppe/ppe.c
> new file mode 100644
> index 000000000000..40da7d240594
> --- /dev/null
> +++ b/drivers/net/ethernet/qualcomm/ppe/ppe.c
> @@ -0,0 +1,218 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2025 Qualcomm Innovation Center, Inc. All rights reserved.
> + */
> +
> +/* PPE platform device probe, DTSI parser and PPE clock initializations. */
> +
> +#include <linux/clk.h>
> +#include <linux/interconnect.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/reset.h>
> +
> +#include "ppe.h"
> +
> +#define PPE_PORT_MAX		8
> +#define PPE_CLK_RATE		353000000
> +
> +/* 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[] = {
> +	{
> +		.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,
> +	},
> +	{
> +		.name = "nssnoc_memnoc",
> +		.avg_bw = 533333,
> +		.peak_bw = 533333,
> +	},
> +	{
> +		.name = "memnoc_nssnoc",
> +		.avg_bw = 533333,
> +		.peak_bw = 533333,
> +	},
> +	{
> +		.name = "memnoc_nssnoc_1",
> +		.avg_bw = 533333,
> +		.peak_bw = 533333,
> +	},
> +};
> +
> +static const struct regmap_range ppe_readable_ranges[] = {
> +	regmap_reg_range(0x0, 0x1ff),		/* Global */
> +	regmap_reg_range(0x400, 0x5ff),		/* LPI CSR */
> +	regmap_reg_range(0x1000, 0x11ff),	/* GMAC0 */
> +	regmap_reg_range(0x1200, 0x13ff),	/* GMAC1 */
> +	regmap_reg_range(0x1400, 0x15ff),	/* GMAC2 */
> +	regmap_reg_range(0x1600, 0x17ff),	/* GMAC3 */
> +	regmap_reg_range(0x1800, 0x19ff),	/* GMAC4 */
> +	regmap_reg_range(0x1a00, 0x1bff),	/* GMAC5 */
> +	regmap_reg_range(0xb000, 0xefff),	/* PRX CSR */
> +	regmap_reg_range(0xf000, 0x1efff),	/* IPE */
> +	regmap_reg_range(0x20000, 0x5ffff),	/* PTX CSR */
> +	regmap_reg_range(0x60000, 0x9ffff),	/* IPE L2 CSR */
> +	regmap_reg_range(0xb0000, 0xeffff),	/* IPO CSR */
> +	regmap_reg_range(0x100000, 0x17ffff),	/* IPE PC */
> +	regmap_reg_range(0x180000, 0x1bffff),	/* PRE IPO CSR */
> +	regmap_reg_range(0x1d0000, 0x1dffff),	/* Tunnel parser */
> +	regmap_reg_range(0x1e0000, 0x1effff),	/* Ingress parse */
> +	regmap_reg_range(0x200000, 0x2fffff),	/* IPE L3 */
> +	regmap_reg_range(0x300000, 0x3fffff),	/* IPE tunnel */
> +	regmap_reg_range(0x400000, 0x4fffff),	/* Scheduler */
> +	regmap_reg_range(0x500000, 0x503fff),	/* XGMAC0 */
> +	regmap_reg_range(0x504000, 0x507fff),	/* XGMAC1 */
> +	regmap_reg_range(0x508000, 0x50bfff),	/* XGMAC2 */
> +	regmap_reg_range(0x50c000, 0x50ffff),	/* XGMAC3 */
> +	regmap_reg_range(0x510000, 0x513fff),	/* XGMAC4 */
> +	regmap_reg_range(0x514000, 0x517fff),	/* XGMAC5 */
> +	regmap_reg_range(0x600000, 0x6fffff),	/* BM */
> +	regmap_reg_range(0x800000, 0x9fffff),	/* QM */
> +	regmap_reg_range(0xb00000, 0xbef800),	/* EDMA */
> +};
> +
> +static const struct regmap_access_table ppe_reg_table = {
> +	.yes_ranges = ppe_readable_ranges,
> +	.n_yes_ranges = ARRAY_SIZE(ppe_readable_ranges),
> +};
> +
> +static const struct regmap_config regmap_config_ipq9574 = {
> +	.reg_bits = 32,
> +	.reg_stride = 4,
> +	.val_bits = 32,
> +	.rd_table = &ppe_reg_table,
> +	.wr_table = &ppe_reg_table,
> +	.max_register = 0xbef800,
> +	.fast_io = true,
> +};
> +
> +static int ppe_clock_init_and_reset(struct ppe_device *ppe_dev)
> +{
> +	unsigned long ppe_rate = ppe_dev->clk_rate;
> +	struct device *dev = ppe_dev->dev;
> +	struct reset_control *rstc;
> +	struct clk_bulk_data *clks;
> +	struct clk *clk;
> +	int ret, i;
> +
> +	for (i = 0; i < ppe_dev->num_icc_paths; i++) {
> +		ppe_dev->icc_paths[i].name = ppe_icc_data[i].name;
> +		ppe_dev->icc_paths[i].avg_bw = ppe_icc_data[i].avg_bw ? :
> +					       Bps_to_icc(ppe_rate);
it's ppe_dev->icc_paths[i].avg_bw = ppe_icc_data[i].avg_bw ? 
ppe_icc_data[i].avg_bw : Bps_to_icc(ppe_rate);  ?


> +		ppe_dev->icc_paths[i].peak_bw = ppe_icc_data[i].peak_bw ? :
> +						Bps_to_icc(ppe_rate);
Same with previous one?

> +	}
> +
> +	ret = devm_of_icc_bulk_get(dev, ppe_dev->num_icc_paths,
> +				   ppe_dev->icc_paths);
> +	if (ret)
> +		return ret;
> +
> +	ret = icc_bulk_set_bw(ppe_dev->num_icc_paths, ppe_dev->icc_paths);
> +	if (ret)
> +		return ret;
> +
> +	/* The PPE clocks have a common parent clock. Setting the clock
Should be:
/*
  * 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.
  */

BTW, it's better wrapped with ~75 characters per line.

> +	 * rate of "ppe" ensures the clock rate of all PPE clocks is
> +	 * configured to the same rate.
> +	 */
> +	clk = devm_clk_get(dev, "ppe");
> +	if (IS_ERR(clk))
> +		return PTR_ERR(clk);
> +
> +	ret = clk_set_rate(clk, ppe_rate);
> +	if (ret)
> +		return ret;
> +
> +	ret = devm_clk_bulk_get_all_enabled(dev, &clks);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Reset the PPE. */
> +	rstc = devm_reset_control_get_exclusive(dev, NULL);
> +	if (IS_ERR(rstc))
> +		return PTR_ERR(rstc);
> +
> +	ret = reset_control_assert(rstc);
> +	if (ret)
> +		return ret;
> +
> +	/* The delay 10 ms of assert is necessary for resetting PPE. */
> +	usleep_range(10000, 11000);
> +
> +	return reset_control_deassert(rstc);
> +}
> +
> +static int qcom_ppe_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct ppe_device *ppe_dev;
> +	void __iomem *base;
> +	int ret, num_icc;
I think it's better with:
	int num_icc = ARRAY_SIZE(ppe_icc_data);

> +
> +	num_icc = ARRAY_SIZE(ppe_icc_data);
> +	ppe_dev = devm_kzalloc(dev, struct_size(ppe_dev, icc_paths, num_icc),
> +			       GFP_KERNEL);
> +	if (!ppe_dev)
> +		return -ENOMEM;
> +
> +	base = devm_platform_ioremap_resource(pdev, 0);
> +	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);
> +	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->num_icc_paths = num_icc;
> +
> +	ret = ppe_clock_init_and_reset(ppe_dev);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "PPE clock config failed\n");
> +
> +	platform_set_drvdata(pdev, ppe_dev);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id qcom_ppe_of_match[] = {
> +	{ .compatible = "qcom,ipq9574-ppe" },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, qcom_ppe_of_match);
> +
> +static struct platform_driver qcom_ppe_driver = {
> +	.driver = {
> +		.name = "qcom_ppe",
> +		.of_match_table = qcom_ppe_of_match,
> +	},
> +	.probe	= qcom_ppe_probe,
> +};
> +module_platform_driver(qcom_ppe_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("Qualcomm Technologies, Inc. IPQ PPE driver");
> diff --git a/drivers/net/ethernet/qualcomm/ppe/ppe.h b/drivers/net/ethernet/qualcomm/ppe/ppe.h
> new file mode 100644
> index 000000000000..cc6767b7c2b8
> --- /dev/null
> +++ b/drivers/net/ethernet/qualcomm/ppe/ppe.h
> @@ -0,0 +1,36 @@
> +/* SPDX-License-Identifier: GPL-2.0-only
> + *
> + * Copyright (c) 2025 Qualcomm Innovation Center, Inc. All rights reserved.
> + */
> +
> +#ifndef __PPE_H__
> +#define __PPE_H__
> +
> +#include <linux/compiler.h>
> +#include <linux/interconnect.h>
> +
> +struct device;
#include <linux/device.h> ?

> +struct regmap;
Same with previous one, include it's header file?

> +
> +/**
> + * struct ppe_device - PPE device private data.
> + * @dev: PPE device structure.
> + * @regmap: PPE register map.
> + * @clk_rate: PPE clock rate.
> + * @num_ports: Number of PPE ports.
> + * @num_icc_paths: Number of interconnect paths.
> + * @icc_paths: Interconnect path array.
> + *
> + * PPE device is the instance of PPE hardware, which is used to
> + * configure PPE packet process modules such as BM (buffer management),
> + * QM (queue management), and scheduler.
> + */
> +struct ppe_device {
> +	struct device *dev;
> +	struct regmap *regmap;
> +	unsigned long clk_rate;
> +	unsigned int num_ports;
> +	unsigned int num_icc_paths;
> +	struct icc_bulk_data icc_paths[] __counted_by(num_icc_paths);
> +};
> +#endif
> 

Thanks,
Jie

Powered by blists - more mailing lists