Re: [PATCH net-next v3 13/14] net: ethernet: qualcomm: Add PPE debugfs support for PPE counters
Re: [PATCH net-next v3 13/14] net: ethernet: qualcomm: Add PPE debugfs support for PPE counters
> > > +/* The number of packets dropped because of no buffer available, no PPE > > > + * buffer assigned to these packets. > > > + */ > > > +static void ppe_port_rx_drop_counter_get(struct ppe_device *ppe_dev, > > > + struct seq_file *seq) > > > +{ > > > + u32 reg, drop_cnt = 0; > > > + int ret, i, tag = 0; > > > + > > > + PRINT_COUNTER_PREFIX("PRX_DROP_CNT", "SILENT_DROP:"); > > > + for (i = 0; i < PPE_DROP_CNT_TBL_ENTRIES; i++) { > > > + reg = PPE_DROP_CNT_TBL_ADDR + i * PPE_DROP_CNT_TBL_INC; > > > + ret = ppe_pkt_cnt_get(ppe_dev, reg, PPE_PKT_CNT_SIZE_1WORD, > > > + &drop_cnt, NULL); > > > + if (ret) { > > > + seq_printf(seq, "ERROR %d\n", ret); > > > + return; > > > + } > > > > This is an error getting the value from the hardware? You should not > > put that into the debugfs itself, you want the read() call to return > > it. > > > > Yes, this error code is returned by regmap read functions in > ppe_pkt_cnt_get() when the hardware counter read fails. I will > remove it from debugfs file and instead log the error to the > console (dev_info). and return it to user space via the read() call. These functions normally return the number of bytes put into the buffer. But you can also return a negative error code which gets passed back to user space instead. Andrew
Re: [PATCH net-next v3 13/14] net: ethernet: qualcomm: Add PPE debugfs support for PPE counters
On 2/11/2025 9:55 PM, Andrew Lunn wrote: +#define PRINT_COUNTER_PREFIX(desc, cnt_type) \ + seq_printf(seq, "%-16s %16s", desc, cnt_type) + +#define PRINT_CPU_CODE_COUNTER(cnt, code) \ + seq_printf(seq, "%10u(cpucode:%d)", cnt, code) + +#define PRINT_DROP_CODE_COUNTER(cnt, port, code) \ + seq_printf(seq, "%10u(port=%d),dropcode:%d", cnt, port, code) + +#define PRINT_SINGLE_COUNTER(tag, cnt, str, index) \ +do { \ + if (!((tag) % 4)) \ + seq_printf(seq, "\n%-16s %16s", "", ""); \ + seq_printf(seq, "%10u(%s=%04d)", cnt, str, index);\ +} while (0) + +#define PRINT_TWO_COUNTERS(tag, cnt0, cnt1, str, index) \ +do { \ + if (!((tag) % 4)) \ + seq_printf(seq, "\n%-16s %16s", "", ""); \ + seq_printf(seq, "%10u/%u(%s=%04d)", cnt0, cnt1, str, index); \ +} while (0) I don't think these make the code any more readable. Just inline it. OK. +/* The number of packets dropped because of no buffer available, no PPE + * buffer assigned to these packets. + */ +static void ppe_port_rx_drop_counter_get(struct ppe_device *ppe_dev, +struct seq_file *seq) +{ + u32 reg, drop_cnt = 0; + int ret, i, tag = 0; + + PRINT_COUNTER_PREFIX("PRX_DROP_CNT", "SILENT_DROP:"); + for (i = 0; i < PPE_DROP_CNT_TBL_ENTRIES; i++) { + reg = PPE_DROP_CNT_TBL_ADDR + i * PPE_DROP_CNT_TBL_INC; + ret = ppe_pkt_cnt_get(ppe_dev, reg, PPE_PKT_CNT_SIZE_1WORD, + &drop_cnt, NULL); + if (ret) { + seq_printf(seq, "ERROR %d\n", ret); + return; + } This is an error getting the value from the hardware? You should not put that into the debugfs itself, you want the read() call to return it. Yes, this error code is returned by regmap read functions in ppe_pkt_cnt_get() when the hardware counter read fails. I will remove it from debugfs file and instead log the error to the console (dev_info). +/* Display the various packet counters of PPE. */ +static int ppe_packet_counter_show(struct seq_file *seq, void *v) +{ + struct ppe_device *ppe_dev = seq->private; + + ppe_port_rx_drop_counter_get(ppe_dev, seq); + ppe_port_rx_bm_drop_counter_get(ppe_dev, seq); + ppe_port_rx_bm_port_counter_get(ppe_dev, seq); + ppe_parse_pkt_counter_get(ppe_dev, seq); + ppe_port_rx_counter_get(ppe_dev, seq); + ppe_vp_rx_counter_get(ppe_dev, seq); + ppe_pre_l2_counter_get(ppe_dev, seq); + ppe_vlan_counter_get(ppe_dev, seq); + ppe_cpu_code_counter_get(ppe_dev, seq); + ppe_eg_vsi_counter_get(ppe_dev, seq); + ppe_vp_tx_counter_get(ppe_dev, seq); + ppe_port_tx_counter_get(ppe_dev, seq); + ppe_queue_tx_counter_get(ppe_dev, seq); It would be more normal to have one debugfs file per group of counters. Andrew Sure. We used a single file as it may be more convenient to display these counters all together in one go, since dumping single group counter has no help on tracing packet drops inside the PPE path. But perhaps, a script can be used as well on top of the segregated files to dump all the counters.
Re: [PATCH net-next v3 13/14] net: ethernet: qualcomm: Add PPE debugfs support for PPE counters
> +#define PRINT_COUNTER_PREFIX(desc, cnt_type) \ > + seq_printf(seq, "%-16s %16s", desc, cnt_type) > + > +#define PRINT_CPU_CODE_COUNTER(cnt, code)\ > + seq_printf(seq, "%10u(cpucode:%d)", cnt, code) > + > +#define PRINT_DROP_CODE_COUNTER(cnt, port, code) \ > + seq_printf(seq, "%10u(port=%d),dropcode:%d", cnt, port, code) > + > +#define PRINT_SINGLE_COUNTER(tag, cnt, str, index) \ > +do { \ > + if (!((tag) % 4)) > \ > + seq_printf(seq, "\n%-16s %16s", "", "");\ > + seq_printf(seq, "%10u(%s=%04d)", cnt, str, index); \ > +} while (0) > + > +#define PRINT_TWO_COUNTERS(tag, cnt0, cnt1, str, index) > \ > +do { \ > + if (!((tag) % 4)) > \ > + seq_printf(seq, "\n%-16s %16s", "", "");\ > + seq_printf(seq, "%10u/%u(%s=%04d)", cnt0, cnt1, str, index);\ > +} while (0) I don't think these make the code any more readable. Just inline it. > +/* The number of packets dropped because of no buffer available, no PPE > + * buffer assigned to these packets. > + */ > +static void ppe_port_rx_drop_counter_get(struct ppe_device *ppe_dev, > + struct seq_file *seq) > +{ > + u32 reg, drop_cnt = 0; > + int ret, i, tag = 0; > + > + PRINT_COUNTER_PREFIX("PRX_DROP_CNT", "SILENT_DROP:"); > + for (i = 0; i < PPE_DROP_CNT_TBL_ENTRIES; i++) { > + reg = PPE_DROP_CNT_TBL_ADDR + i * PPE_DROP_CNT_TBL_INC; > + ret = ppe_pkt_cnt_get(ppe_dev, reg, PPE_PKT_CNT_SIZE_1WORD, > + &drop_cnt, NULL); > + if (ret) { > + seq_printf(seq, "ERROR %d\n", ret); > + return; > + } This is an error getting the value from the hardware? You should not put that into the debugfs itself, you want the read() call to return it. > +/* Display the various packet counters of PPE. */ > +static int ppe_packet_counter_show(struct seq_file *seq, void *v) > +{ > + struct ppe_device *ppe_dev = seq->private; > + > + ppe_port_rx_drop_counter_get(ppe_dev, seq); > + ppe_port_rx_bm_drop_counter_get(ppe_dev, seq); > + ppe_port_rx_bm_port_counter_get(ppe_dev, seq); > + ppe_parse_pkt_counter_get(ppe_dev, seq); > + ppe_port_rx_counter_get(ppe_dev, seq); > + ppe_vp_rx_counter_get(ppe_dev, seq); > + ppe_pre_l2_counter_get(ppe_dev, seq); > + ppe_vlan_counter_get(ppe_dev, seq); > + ppe_cpu_code_counter_get(ppe_dev, seq); > + ppe_eg_vsi_counter_get(ppe_dev, seq); > + ppe_vp_tx_counter_get(ppe_dev, seq); > + ppe_port_tx_counter_get(ppe_dev, seq); > + ppe_queue_tx_counter_get(ppe_dev, seq); It would be more normal to have one debugfs file per group of counters. Andrew
[PATCH net-next v3 13/14] net: ethernet: qualcomm: Add PPE debugfs support for PPE counters
The PPE hardware counters maintain counters for packets handled by the various functional blocks of PPE. They help in tracing the packets passed through PPE and debugging any packet drops. The counters displayed by this debugfs file are ones that are common for all Ethernet ports, and they do not include the counters that are specific for a MAC port. Hence they cannot be displayed using ethtool. The per-MAC counters will be supported using "ethtool -S" along with the netdevice driver. The PPE hardware packet counters are made available through the debugfs entry "/sys/kernel/debug/ppe/packet_counters". Signed-off-by: Luo Jie --- drivers/net/ethernet/qualcomm/ppe/Makefile | 2 +- drivers/net/ethernet/qualcomm/ppe/ppe.c | 11 + drivers/net/ethernet/qualcomm/ppe/ppe.h | 3 + drivers/net/ethernet/qualcomm/ppe/ppe_debugfs.c | 692 drivers/net/ethernet/qualcomm/ppe/ppe_debugfs.h | 16 + drivers/net/ethernet/qualcomm/ppe/ppe_regs.h| 102 6 files changed, 825 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/qualcomm/ppe/Makefile b/drivers/net/ethernet/qualcomm/ppe/Makefile index 410a7bb54cfe..9e60b2400c16 100644 --- a/drivers/net/ethernet/qualcomm/ppe/Makefile +++ b/drivers/net/ethernet/qualcomm/ppe/Makefile @@ -4,4 +4,4 @@ # obj-$(CONFIG_QCOM_PPE) += qcom-ppe.o -qcom-ppe-objs := ppe.o ppe_config.o +qcom-ppe-objs := ppe.o ppe_config.o ppe_debugfs.o diff --git a/drivers/net/ethernet/qualcomm/ppe/ppe.c b/drivers/net/ethernet/qualcomm/ppe/ppe.c index 253de6a15466..17f6770c59ae 100644 --- a/drivers/net/ethernet/qualcomm/ppe/ppe.c +++ b/drivers/net/ethernet/qualcomm/ppe/ppe.c @@ -16,6 +16,7 @@ #include "ppe.h" #include "ppe_config.h" +#include "ppe_debugfs.h" #define PPE_PORT_MAX 8 #define PPE_CLK_RATE 35300 @@ -199,11 +200,20 @@ static int qcom_ppe_probe(struct platform_device *pdev) if (ret) return dev_err_probe(dev, ret, "PPE HW config failed\n"); + ppe_debugfs_setup(ppe_dev); platform_set_drvdata(pdev, ppe_dev); return 0; } +static void qcom_ppe_remove(struct platform_device *pdev) +{ + struct ppe_device *ppe_dev; + + ppe_dev = platform_get_drvdata(pdev); + ppe_debugfs_teardown(ppe_dev); +} + static const struct of_device_id qcom_ppe_of_match[] = { { .compatible = "qcom,ipq9574-ppe" }, {} @@ -216,6 +226,7 @@ static struct platform_driver qcom_ppe_driver = { .of_match_table = qcom_ppe_of_match, }, .probe = qcom_ppe_probe, + .remove = qcom_ppe_remove, }; module_platform_driver(qcom_ppe_driver); diff --git a/drivers/net/ethernet/qualcomm/ppe/ppe.h b/drivers/net/ethernet/qualcomm/ppe/ppe.h index cc6767b7c2b8..e9a208b77459 100644 --- a/drivers/net/ethernet/qualcomm/ppe/ppe.h +++ b/drivers/net/ethernet/qualcomm/ppe/ppe.h @@ -11,6 +11,7 @@ struct device; struct regmap; +struct dentry; /** * struct ppe_device - PPE device private data. @@ -18,6 +19,7 @@ struct regmap; * @regmap: PPE register map. * @clk_rate: PPE clock rate. * @num_ports: Number of PPE ports. + * @debugfs_root: Debugfs root entry. * @num_icc_paths: Number of interconnect paths. * @icc_paths: Interconnect path array. * @@ -30,6 +32,7 @@ struct ppe_device { struct regmap *regmap; unsigned long clk_rate; unsigned int num_ports; + struct dentry *debugfs_root; unsigned int num_icc_paths; struct icc_bulk_data icc_paths[] __counted_by(num_icc_paths); }; diff --git a/drivers/net/ethernet/qualcomm/ppe/ppe_debugfs.c b/drivers/net/ethernet/qualcomm/ppe/ppe_debugfs.c new file mode 100644 index ..5c05cbb7a0fe --- /dev/null +++ b/drivers/net/ethernet/qualcomm/ppe/ppe_debugfs.c @@ -0,0 +1,692 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (c) 2025 Qualcomm Innovation Center, Inc. All rights reserved. + */ + +/* PPE debugfs routines for display of PPE counters useful for debug. */ + +#include +#include +#include +#include + +#include "ppe.h" +#include "ppe_config.h" +#include "ppe_debugfs.h" +#include "ppe_regs.h" + +#define PPE_PKT_CNT_TBL_SIZE 3 +#define PPE_DROP_PKT_CNT_TBL_SIZE 5 + +#define PPE_W0_PKT_CNT GENMASK(31, 0) +#define PPE_W2_DROP_PKT_CNT_LOWGENMASK(31, 8) +#define PPE_W3_DROP_PKT_CNT_HIGH GENMASK(7, 0) + +#define PPE_GET_PKT_CNT(tbl_cnt) \ + u32_get_bits(*((u32 *)(tbl_cnt)), PPE_W0_PKT_CNT) +#define PPE_GET_DROP_PKT_CNT_LOW(tbl_cnt) \ + u32_get_bits(*((u32 *)(tbl_cnt) + 0x2), PPE_W2_DROP_PKT_CNT_LOW) +#define PPE_GET_DROP_PKT_CNT_HIGH(tbl_cnt) \ + u32_get_bits(*((u32 *)(tbl_cnt) + 0x3), PPE_W3_DROP_PKT_CNT_HIGH) + +#define PRINT_COUNTER_PREFIX(desc, cnt_type) \ + seq_printf(seq, "%-16s %16s", desc, cnt_