| Message ID | 20250209-qcom_ipq_ppe-v3-4-453ea18d3271@quicinc.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Add PPE driver for Qualcomm IPQ9574 SoC | expand |
> +/* Assign the share buffer number 1550 to group 0 by default. */ > +static const int ipq9574_ppe_bm_group_config = 1550; To a large extent, the comment is useless. What should be in the comment is why, not what. Andrew
> + /* Configure BM flow control related threshold. */ > + PPE_BM_PORT_FC_SET_WEIGHT(bm_fc_val, port_cfg.weight); > + PPE_BM_PORT_FC_SET_RESUME_OFFSET(bm_fc_val, port_cfg.resume_offset); > + PPE_BM_PORT_FC_SET_RESUME_THRESHOLD(bm_fc_val, port_cfg.resume_ceil); > + PPE_BM_PORT_FC_SET_DYNAMIC(bm_fc_val, port_cfg.dynamic); > + PPE_BM_PORT_FC_SET_REACT_LIMIT(bm_fc_val, port_cfg.in_fly_buf); > + PPE_BM_PORT_FC_SET_PRE_ALLOC(bm_fc_val, port_cfg.pre_alloc); ... > +#define PPE_BM_PORT_FC_CFG_TBL_ADDR 0x601000 > +#define PPE_BM_PORT_FC_CFG_TBL_ENTRIES 15 > +#define PPE_BM_PORT_FC_CFG_TBL_INC 0x10 > +#define PPE_BM_PORT_FC_W0_REACT_LIMIT GENMASK(8, 0) > +#define PPE_BM_PORT_FC_W0_RESUME_THRESHOLD GENMASK(17, 9) > +#define PPE_BM_PORT_FC_W0_RESUME_OFFSET GENMASK(28, 18) > +#define PPE_BM_PORT_FC_W0_CEILING_LOW GENMASK(31, 29) > +#define PPE_BM_PORT_FC_W1_CEILING_HIGH GENMASK(7, 0) > +#define PPE_BM_PORT_FC_W1_WEIGHT GENMASK(10, 8) > +#define PPE_BM_PORT_FC_W1_DYNAMIC BIT(11) > +#define PPE_BM_PORT_FC_W1_PRE_ALLOC GENMASK(22, 12) > + > +#define PPE_BM_PORT_FC_SET_REACT_LIMIT(tbl_cfg, value) \ > + u32p_replace_bits((u32 *)tbl_cfg, value, PPE_BM_PORT_FC_W0_REACT_LIMIT) > +#define PPE_BM_PORT_FC_SET_RESUME_THRESHOLD(tbl_cfg, value) \ > + u32p_replace_bits((u32 *)tbl_cfg, value, PPE_BM_PORT_FC_W0_RESUME_THRESHOLD) Where is u32p_replace_bits()? This cast does not look good. And this does not look like anything any other driver does. I suspect you are not using FIELD_PREP() etc when you should. https://elixir.bootlin.com/linux/v6.14-rc2/source/include/linux/bitfield.h Andrew
On 2/11/2025 9:14 PM, Andrew Lunn wrote: >> +/* Assign the share buffer number 1550 to group 0 by default. */ >> +static const int ipq9574_ppe_bm_group_config = 1550; > > To a large extent, the comment is useless. What should be in the > comment is why, not what. > > Andrew > OK, I will improve the comment to describe it better. There are total 2048 buffers available in PPE, out of which some buffers are reserved for some specific purposes. The rest of the pool of 1550 buffers are assigned to the general 'group0' which is shared among all ports of the PPE.
On 2/11/2025 9:22 PM, Andrew Lunn wrote: >> + /* Configure BM flow control related threshold. */ >> + PPE_BM_PORT_FC_SET_WEIGHT(bm_fc_val, port_cfg.weight); >> + PPE_BM_PORT_FC_SET_RESUME_OFFSET(bm_fc_val, port_cfg.resume_offset); >> + PPE_BM_PORT_FC_SET_RESUME_THRESHOLD(bm_fc_val, port_cfg.resume_ceil); >> + PPE_BM_PORT_FC_SET_DYNAMIC(bm_fc_val, port_cfg.dynamic); >> + PPE_BM_PORT_FC_SET_REACT_LIMIT(bm_fc_val, port_cfg.in_fly_buf); >> + PPE_BM_PORT_FC_SET_PRE_ALLOC(bm_fc_val, port_cfg.pre_alloc); > > ... > >> +#define PPE_BM_PORT_FC_CFG_TBL_ADDR 0x601000 >> +#define PPE_BM_PORT_FC_CFG_TBL_ENTRIES 15 >> +#define PPE_BM_PORT_FC_CFG_TBL_INC 0x10 >> +#define PPE_BM_PORT_FC_W0_REACT_LIMIT GENMASK(8, 0) >> +#define PPE_BM_PORT_FC_W0_RESUME_THRESHOLD GENMASK(17, 9) >> +#define PPE_BM_PORT_FC_W0_RESUME_OFFSET GENMASK(28, 18) >> +#define PPE_BM_PORT_FC_W0_CEILING_LOW GENMASK(31, 29) >> +#define PPE_BM_PORT_FC_W1_CEILING_HIGH GENMASK(7, 0) >> +#define PPE_BM_PORT_FC_W1_WEIGHT GENMASK(10, 8) >> +#define PPE_BM_PORT_FC_W1_DYNAMIC BIT(11) >> +#define PPE_BM_PORT_FC_W1_PRE_ALLOC GENMASK(22, 12) >> + >> +#define PPE_BM_PORT_FC_SET_REACT_LIMIT(tbl_cfg, value) \ >> + u32p_replace_bits((u32 *)tbl_cfg, value, PPE_BM_PORT_FC_W0_REACT_LIMIT) >> +#define PPE_BM_PORT_FC_SET_RESUME_THRESHOLD(tbl_cfg, value) \ >> + u32p_replace_bits((u32 *)tbl_cfg, value, PPE_BM_PORT_FC_W0_RESUME_THRESHOLD) > > Where is u32p_replace_bits()? u32p_replace_bits is defined by the macro __MAKE_OP(32) in the header file "include/linux/bitfield.h". > > This cast does not look good. Yes, we can remove the cast. > And this does not look like anything any > other driver does. I suspect you are not using FIELD_PREP() etc when > you should. > > https://elixir.bootlin.com/linux/v6.14-rc2/source/include/linux/bitfield.h > > Andrew The PPE_BM_XXX macros defined here write to either of two different 32bit words in the register table, and the actual word used (0 or 1) is hidden within the macro. For example, the below macro. #define PPE_BM_PORT_FC_SET_CEILING_HIGH(tbl_cfg, value) \ u32p_replace_bits((u32 *)(tbl_cfg) + 0x1, value, PPE_BM_PORT_FC_W1_CEILING_HIGH) We could have used FIELD_PREP as well for this purpose. However using u32p_replace_bits() seemed more convenient and cleaner in this case, since with FIELD_PREP, we would have needed an assignment statement to be defined in the macro implementation. We also noticed many other drivers using u32_replace_bits(). Hope this is ok.
On Thu, Feb 20, 2025 at 10:38:03PM +0800, Jie Luo wrote: > > > On 2/11/2025 9:22 PM, Andrew Lunn wrote: > > > + /* Configure BM flow control related threshold. */ > > > + PPE_BM_PORT_FC_SET_WEIGHT(bm_fc_val, port_cfg.weight); > > > + PPE_BM_PORT_FC_SET_RESUME_OFFSET(bm_fc_val, port_cfg.resume_offset); > > > + PPE_BM_PORT_FC_SET_RESUME_THRESHOLD(bm_fc_val, port_cfg.resume_ceil); > > > + PPE_BM_PORT_FC_SET_DYNAMIC(bm_fc_val, port_cfg.dynamic); > > > + PPE_BM_PORT_FC_SET_REACT_LIMIT(bm_fc_val, port_cfg.in_fly_buf); > > > + PPE_BM_PORT_FC_SET_PRE_ALLOC(bm_fc_val, port_cfg.pre_alloc); > > > > ... > > > > > +#define PPE_BM_PORT_FC_CFG_TBL_ADDR 0x601000 > > > +#define PPE_BM_PORT_FC_CFG_TBL_ENTRIES 15 > > > +#define PPE_BM_PORT_FC_CFG_TBL_INC 0x10 > > > +#define PPE_BM_PORT_FC_W0_REACT_LIMIT GENMASK(8, 0) > > > +#define PPE_BM_PORT_FC_W0_RESUME_THRESHOLD GENMASK(17, 9) > > > +#define PPE_BM_PORT_FC_W0_RESUME_OFFSET GENMASK(28, 18) > > > +#define PPE_BM_PORT_FC_W0_CEILING_LOW GENMASK(31, 29) > > > +#define PPE_BM_PORT_FC_W1_CEILING_HIGH GENMASK(7, 0) > > > +#define PPE_BM_PORT_FC_W1_WEIGHT GENMASK(10, 8) > > > +#define PPE_BM_PORT_FC_W1_DYNAMIC BIT(11) > > > +#define PPE_BM_PORT_FC_W1_PRE_ALLOC GENMASK(22, 12) > > > + > > > +#define PPE_BM_PORT_FC_SET_REACT_LIMIT(tbl_cfg, value) \ > > > + u32p_replace_bits((u32 *)tbl_cfg, value, PPE_BM_PORT_FC_W0_REACT_LIMIT) > > > +#define PPE_BM_PORT_FC_SET_RESUME_THRESHOLD(tbl_cfg, value) \ > > > + u32p_replace_bits((u32 *)tbl_cfg, value, PPE_BM_PORT_FC_W0_RESUME_THRESHOLD) > > > > Where is u32p_replace_bits()? > > u32p_replace_bits is defined by the macro __MAKE_OP(32) in the header > file "include/linux/bitfield.h". Given it is pretty well hidden, and not documented, it makes me think you should not be using it. The macros you are expected to use from that file are all well documented. > > This cast does not look good. > > Yes, we can remove the cast. To some extent, this is a symptom. Why is the cast there in the first place? Cast suggest bad design, not thinking about types, thinking it is actual O.K. to cast between types. Please look at all the casts you have. Is it because of bad design? If so, please fix your types to eliminate the casts. > > And this does not look like anything any > > other driver does. I suspect you are not using FIELD_PREP() etc when > > you should. > > > > https://elixir.bootlin.com/linux/v6.14-rc2/source/include/linux/bitfield.h > > > > Andrew > > The PPE_BM_XXX macros defined here write to either of two different > 32bit words in the register table, and the actual word used (0 or 1) > is hidden within the macro. For example, the below macro. > > #define PPE_BM_PORT_FC_SET_CEILING_HIGH(tbl_cfg, value) \ > u32p_replace_bits((u32 *)(tbl_cfg) + 0x1, value, > PPE_BM_PORT_FC_W1_CEILING_HIGH) > > We could have used FIELD_PREP as well for this purpose. However using > u32p_replace_bits() seemed more convenient and cleaner in this case, > since with FIELD_PREP, we would have needed an assignment statement to > be defined in the macro implementation. We also noticed many other > drivers using u32_replace_bits(). Hope this is ok. Please extend the set of FIELD_{GET,PREP} macros to cover your use case. Document them to the level of the existing macros. Submit the patch to: Yury Norov <yury.norov@gmail.com> (maintainer:BITMAP API) Rasmus Villemoes <linux@rasmusvillemoes.dk> (reviewer:BITMAP API) etc and see what they say about this. Andrew
On 2/20/2025 11:09 PM, Andrew Lunn wrote: > On Thu, Feb 20, 2025 at 10:38:03PM +0800, Jie Luo wrote: >> >> >> On 2/11/2025 9:22 PM, Andrew Lunn wrote: >>>> + /* Configure BM flow control related threshold. */ >>>> + PPE_BM_PORT_FC_SET_WEIGHT(bm_fc_val, port_cfg.weight); >>>> + PPE_BM_PORT_FC_SET_RESUME_OFFSET(bm_fc_val, port_cfg.resume_offset); >>>> + PPE_BM_PORT_FC_SET_RESUME_THRESHOLD(bm_fc_val, port_cfg.resume_ceil); >>>> + PPE_BM_PORT_FC_SET_DYNAMIC(bm_fc_val, port_cfg.dynamic); >>>> + PPE_BM_PORT_FC_SET_REACT_LIMIT(bm_fc_val, port_cfg.in_fly_buf); >>>> + PPE_BM_PORT_FC_SET_PRE_ALLOC(bm_fc_val, port_cfg.pre_alloc); >>> >>> ... >>> >>>> +#define PPE_BM_PORT_FC_CFG_TBL_ADDR 0x601000 >>>> +#define PPE_BM_PORT_FC_CFG_TBL_ENTRIES 15 >>>> +#define PPE_BM_PORT_FC_CFG_TBL_INC 0x10 >>>> +#define PPE_BM_PORT_FC_W0_REACT_LIMIT GENMASK(8, 0) >>>> +#define PPE_BM_PORT_FC_W0_RESUME_THRESHOLD GENMASK(17, 9) >>>> +#define PPE_BM_PORT_FC_W0_RESUME_OFFSET GENMASK(28, 18) >>>> +#define PPE_BM_PORT_FC_W0_CEILING_LOW GENMASK(31, 29) >>>> +#define PPE_BM_PORT_FC_W1_CEILING_HIGH GENMASK(7, 0) >>>> +#define PPE_BM_PORT_FC_W1_WEIGHT GENMASK(10, 8) >>>> +#define PPE_BM_PORT_FC_W1_DYNAMIC BIT(11) >>>> +#define PPE_BM_PORT_FC_W1_PRE_ALLOC GENMASK(22, 12) >>>> + >>>> +#define PPE_BM_PORT_FC_SET_REACT_LIMIT(tbl_cfg, value) \ >>>> + u32p_replace_bits((u32 *)tbl_cfg, value, PPE_BM_PORT_FC_W0_REACT_LIMIT) >>>> +#define PPE_BM_PORT_FC_SET_RESUME_THRESHOLD(tbl_cfg, value) \ >>>> + u32p_replace_bits((u32 *)tbl_cfg, value, PPE_BM_PORT_FC_W0_RESUME_THRESHOLD) >>> >>> Where is u32p_replace_bits()? >> >> u32p_replace_bits is defined by the macro __MAKE_OP(32) in the header >> file "include/linux/bitfield.h". > > Given it is pretty well hidden, and not documented, it makes me think > you should not be using it. The macros you are expected to use from > that file are all well documented. OK, understand. > >>> This cast does not look good. >> >> Yes, we can remove the cast. > > To some extent, this is a symptom. Why is the cast there in the first > place? Cast suggest bad design, not thinking about types, thinking it > is actual O.K. to cast between types. Please look at all the casts you > have. Is it because of bad design? If so, please fix your types to > eliminate the casts. Sure, this cast is actually redundant, the type of value passed to this macro is already defined as the type u32. I will review and remove the remaining casts in the ppe_reg.h file. > >>> And this does not look like anything any >>> other driver does. I suspect you are not using FIELD_PREP() etc when >>> you should. >>> >>> https://elixir.bootlin.com/linux/v6.14-rc2/source/include/linux/bitfield.h >>> >>> Andrew >> >> The PPE_BM_XXX macros defined here write to either of two different >> 32bit words in the register table, and the actual word used (0 or 1) >> is hidden within the macro. For example, the below macro. >> >> #define PPE_BM_PORT_FC_SET_CEILING_HIGH(tbl_cfg, value) \ >> u32p_replace_bits((u32 *)(tbl_cfg) + 0x1, value, >> PPE_BM_PORT_FC_W1_CEILING_HIGH) >> >> We could have used FIELD_PREP as well for this purpose. However using >> u32p_replace_bits() seemed more convenient and cleaner in this case, >> since with FIELD_PREP, we would have needed an assignment statement to >> be defined in the macro implementation. We also noticed many other >> drivers using u32_replace_bits(). Hope this is ok. > > Please extend the set of FIELD_{GET,PREP} macros to cover your use > case. Document them to the level of the existing macros. Submit the > patch to: > > Yury Norov <yury.norov@gmail.com> (maintainer:BITMAP API) > Rasmus Villemoes <linux@rasmusvillemoes.dk> (reviewer:BITMAP API) > etc > > and see what they say about this. > > Andrew Thanks for the suggestion. Just to clarify, we preferred u32p_replace_bits() over FIELD_PREP() because the former does a clear-and-set operation against a given mask, where as with FIELD_PREP(), we need to clear the bits first before we use the macro and then set it. Due to this, we preferred using u32_replace_bits() since it made the macro definitions to modify the registers simpler. Given this, would it be acceptable to document u32p_replace_bits() better, as it is already being used by other drivers as well? If you prefer to use FIELD_PREP() over u32p_replace_bits(), we can update the driver to change the macros to use FIELD_PREP(). Please note that all our macros for register modifications operate only on 32bit values. so we do not have any necessity for casts in the code. Below is one example per my understanding, implemented for both cases - u32p_replace_bits() and FIELD_PREP: #define PPE_BM_PORT_FC_SET_WEIGHT(tbl_cfg, value) \ u32p_replace_bits(tbl_cfg + 0x1, value, PPE_BM_PORT_FC_W1_WEIGHT) #define PPE_BM_PORT_FC_SET_WEIGHT(tbl_cfg, value) \ do { \ *(tbl_cfg + 0x1) &= ~PPE_BM_PORT_FC_W1_WEIGHT; \ *(tbl_cfg + 0x1) |= FIELD_PREP(PPE_BM_PORT_FC_W1_WEIGHT, value); \ } while (0)
> Thanks for the suggestion. Just to clarify, we preferred > u32p_replace_bits() over FIELD_PREP() because the former does > a clear-and-set operation against a given mask, where as with > FIELD_PREP(), we need to clear the bits first before we use the > macro and then set it. Due to this, we preferred using > u32_replace_bits() since it made the macro definitions to modify > the registers simpler. Given this, would it be acceptable to > document u32p_replace_bits() better, as it is already being used > by other drivers as well? I suggest you submit a patch to those who maintain that file and see what they say. But maybe also look at how others are using u32p_replace_bits() and should it be wrapped up in a macro? FIELD_MOD()? These macros do a lot of build time checking that you are not overflowing the type. It would be good to have that to catch bugs at build time, rather than years later at runtime. Andrew
On 3/6/2025 11:29 PM, Andrew Lunn wrote: >> Thanks for the suggestion. Just to clarify, we preferred >> u32p_replace_bits() over FIELD_PREP() because the former does >> a clear-and-set operation against a given mask, where as with >> FIELD_PREP(), we need to clear the bits first before we use the >> macro and then set it. Due to this, we preferred using >> u32_replace_bits() since it made the macro definitions to modify >> the registers simpler. Given this, would it be acceptable to >> document u32p_replace_bits() better, as it is already being used >> by other drivers as well? > > I suggest you submit a patch to those who maintain that file and see > what they say. > > But maybe also look at how others are using u32p_replace_bits() and > should it be wrapped up in a macro? FIELD_MOD()? These macros do a lot > of build time checking that you are not overflowing the type. It would > be good to have that to catch bugs at build time, rather than years > later at runtime. > > Andrew OK, understand. I will submit the patch by adding the FIELD_MODIFY() with required build time checking included. Below is a draft of the macro, please take a look if possible before it is posted to maintainers. I will update the driver to use this macro if it can be accepted. Thanks. /** * FIELD_MODIFY() - modify a bitfield element * @_mask: shifted mask defining the field's length and position * @_reg_p: point to the memory that should be updated * @_val: value to store in the bitfield * * FIELD_MODIFY() modifies the set of bits in @_reg_p specified * by @_mask, by replacing them with the bitfield value passed * in as @_val. */ #define FIELD_MODIFY(_mask, _reg_p, _val) \ ({ \ __BF_FIELD_CHECK(_mask, *_reg_p, _val, "FIELD_MODIFY: "); \ *_reg_p &= ~(_mask); \ *_reg_p |= (_val) << __bf_shf(_mask) & (_mask); \ })
diff --git a/drivers/net/ethernet/qualcomm/ppe/Makefile b/drivers/net/ethernet/qualcomm/ppe/Makefile index 63d50d3b4f2e..410a7bb54cfe 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 +qcom-ppe-objs := ppe.o ppe_config.o diff --git a/drivers/net/ethernet/qualcomm/ppe/ppe.c b/drivers/net/ethernet/qualcomm/ppe/ppe.c index 40da7d240594..253de6a15466 100644 --- a/drivers/net/ethernet/qualcomm/ppe/ppe.c +++ b/drivers/net/ethernet/qualcomm/ppe/ppe.c @@ -15,6 +15,7 @@ #include <linux/reset.h> #include "ppe.h" +#include "ppe_config.h" #define PPE_PORT_MAX 8 #define PPE_CLK_RATE 353000000 @@ -194,6 +195,10 @@ static int qcom_ppe_probe(struct platform_device *pdev) if (ret) return dev_err_probe(dev, ret, "PPE clock config failed\n"); + ret = ppe_hw_config(ppe_dev); + if (ret) + return dev_err_probe(dev, ret, "PPE HW config failed\n"); + platform_set_drvdata(pdev, ppe_dev); return 0; diff --git a/drivers/net/ethernet/qualcomm/ppe/ppe_config.c b/drivers/net/ethernet/qualcomm/ppe/ppe_config.c new file mode 100644 index 000000000000..18e9544a4e37 --- /dev/null +++ b/drivers/net/ethernet/qualcomm/ppe/ppe_config.c @@ -0,0 +1,195 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (c) 2025 Qualcomm Innovation Center, Inc. All rights reserved. + */ + +/* PPE HW initialization configs such as BM(buffer management), + * QM(queue management) and scheduler configs. + */ + +#include <linux/bitfield.h> +#include <linux/bits.h> +#include <linux/device.h> +#include <linux/regmap.h> + +#include "ppe.h" +#include "ppe_config.h" +#include "ppe_regs.h" + +/** + * struct ppe_bm_port_config - PPE BM port configuration. + * @port_id_start: The fist BM port ID to configure. + * @port_id_end: The last BM port ID to configure. + * @pre_alloc: BM port dedicated buffer number. + * @in_fly_buf: Buffer number for receiving the packet after pause frame sent. + * @ceil: Ceil to generate the back pressure. + * @weight: Weight value. + * @resume_offset: Resume offset from the threshold value. + * @resume_ceil: Ceil to resume from the back pressure state. + * @dynamic: Dynamic threshold used or not. + * + * The is for configuring the threshold that impacts the port + * flow control. + */ +struct ppe_bm_port_config { + unsigned int port_id_start; + unsigned int port_id_end; + unsigned int pre_alloc; + unsigned int in_fly_buf; + unsigned int ceil; + unsigned int weight; + unsigned int resume_offset; + unsigned int resume_ceil; + bool dynamic; +}; + +/* Assign the share buffer number 1550 to group 0 by default. */ +static const int ipq9574_ppe_bm_group_config = 1550; + +/* 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 + * EIP port. + */ +static const struct ppe_bm_port_config ipq9574_ppe_bm_port_config[] = { + { + /* Buffer configuration for the BM port ID 0 of EDMA. */ + .port_id_start = 0, + .port_id_end = 0, + .pre_alloc = 0, + .in_fly_buf = 100, + .ceil = 1146, + .weight = 7, + .resume_offset = 8, + .resume_ceil = 0, + .dynamic = true, + }, + { + /* Buffer configuration for the BM port ID 1-7 of EDMA. */ + .port_id_start = 1, + .port_id_end = 7, + .pre_alloc = 0, + .in_fly_buf = 100, + .ceil = 250, + .weight = 4, + .resume_offset = 36, + .resume_ceil = 0, + .dynamic = true, + }, + { + /* Buffer configuration for the BM port ID 8-13 of PPE ports. */ + .port_id_start = 8, + .port_id_end = 13, + .pre_alloc = 0, + .in_fly_buf = 128, + .ceil = 250, + .weight = 4, + .resume_offset = 36, + .resume_ceil = 0, + .dynamic = true, + }, + { + /* Buffer configuration for the BM port ID 14 of EIP. */ + .port_id_start = 14, + .port_id_end = 14, + .pre_alloc = 0, + .in_fly_buf = 40, + .ceil = 250, + .weight = 4, + .resume_offset = 36, + .resume_ceil = 0, + .dynamic = true, + }, +}; + +static int ppe_config_bm_threshold(struct ppe_device *ppe_dev, int bm_port_id, + const struct ppe_bm_port_config port_cfg) +{ + u32 reg, val, bm_fc_val[2]; + int ret; + + reg = PPE_BM_PORT_FC_CFG_TBL_ADDR + PPE_BM_PORT_FC_CFG_TBL_INC * bm_port_id; + ret = regmap_bulk_read(ppe_dev->regmap, reg, + bm_fc_val, ARRAY_SIZE(bm_fc_val)); + if (ret) + return ret; + + /* Configure BM flow control related threshold. */ + PPE_BM_PORT_FC_SET_WEIGHT(bm_fc_val, port_cfg.weight); + PPE_BM_PORT_FC_SET_RESUME_OFFSET(bm_fc_val, port_cfg.resume_offset); + PPE_BM_PORT_FC_SET_RESUME_THRESHOLD(bm_fc_val, port_cfg.resume_ceil); + PPE_BM_PORT_FC_SET_DYNAMIC(bm_fc_val, port_cfg.dynamic); + PPE_BM_PORT_FC_SET_REACT_LIMIT(bm_fc_val, port_cfg.in_fly_buf); + PPE_BM_PORT_FC_SET_PRE_ALLOC(bm_fc_val, port_cfg.pre_alloc); + + /* Configure low/high bits of the ceiling for the BM port. */ + val = FIELD_GET(GENMASK(2, 0), port_cfg.ceil); + PPE_BM_PORT_FC_SET_CEILING_LOW(bm_fc_val, val); + val = FIELD_GET(GENMASK(10, 3), port_cfg.ceil); + PPE_BM_PORT_FC_SET_CEILING_HIGH(bm_fc_val, val); + + ret = regmap_bulk_write(ppe_dev->regmap, reg, + bm_fc_val, ARRAY_SIZE(bm_fc_val)); + if (ret) + return ret; + + /* Assign the default group ID 0 to the BM port. */ + val = FIELD_PREP(PPE_BM_PORT_GROUP_ID_SHARED_GROUP_ID, 0); + reg = PPE_BM_PORT_GROUP_ID_ADDR + PPE_BM_PORT_GROUP_ID_INC * bm_port_id; + ret = regmap_update_bits(ppe_dev->regmap, reg, + PPE_BM_PORT_GROUP_ID_SHARED_GROUP_ID, + val); + if (ret) + return ret; + + /* Enable BM port flow control. */ + reg = PPE_BM_PORT_FC_MODE_ADDR + PPE_BM_PORT_FC_MODE_INC * bm_port_id; + + return regmap_set_bits(ppe_dev->regmap, reg, PPE_BM_PORT_FC_MODE_EN); +} + +/* Configure the buffer threshold for the port flow control function. */ +static int ppe_config_bm(struct ppe_device *ppe_dev) +{ + const struct ppe_bm_port_config *port_cfg; + unsigned int i, bm_port_id, port_cfg_cnt; + u32 reg, val; + int ret; + + /* Configure the allocated buffer number only for group 0. + * The buffer number of group 1-3 is already cleared to 0 + * after PPE reset during the probe of PPE driver. + */ + reg = PPE_BM_SHARED_GROUP_CFG_ADDR; + val = FIELD_PREP(PPE_BM_SHARED_GROUP_CFG_SHARED_LIMIT, + ipq9574_ppe_bm_group_config); + ret = regmap_update_bits(ppe_dev->regmap, reg, + PPE_BM_SHARED_GROUP_CFG_SHARED_LIMIT, + val); + if (ret) + 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); + 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++) { + ret = ppe_config_bm_threshold(ppe_dev, bm_port_id, + port_cfg[i]); + if (ret) + goto bm_config_fail; + } + } + + return 0; + +bm_config_fail: + dev_err(ppe_dev->dev, "PPE BM config error %d\n", ret); + return ret; +} + +int ppe_hw_config(struct ppe_device *ppe_dev) +{ + return ppe_config_bm(ppe_dev); +} diff --git a/drivers/net/ethernet/qualcomm/ppe/ppe_config.h b/drivers/net/ethernet/qualcomm/ppe/ppe_config.h new file mode 100644 index 000000000000..7b2f6a71cd4c --- /dev/null +++ b/drivers/net/ethernet/qualcomm/ppe/ppe_config.h @@ -0,0 +1,12 @@ +/* SPDX-License-Identifier: GPL-2.0-only + * + * Copyright (c) 2025 Qualcomm Innovation Center, Inc. All rights reserved. + */ + +#ifndef __PPE_CONFIG_H__ +#define __PPE_CONFIG_H__ + +#include "ppe.h" + +int ppe_hw_config(struct ppe_device *ppe_dev); +#endif diff --git a/drivers/net/ethernet/qualcomm/ppe/ppe_regs.h b/drivers/net/ethernet/qualcomm/ppe/ppe_regs.h new file mode 100644 index 000000000000..b00f77ec45fe --- /dev/null +++ b/drivers/net/ethernet/qualcomm/ppe/ppe_regs.h @@ -0,0 +1,59 @@ +/* SPDX-License-Identifier: GPL-2.0-only + * + * Copyright (c) 2025 Qualcomm Innovation Center, Inc. All rights reserved. + */ + +/* PPE hardware register and table declarations. */ +#ifndef __PPE_REGS_H__ +#define __PPE_REGS_H__ + +#include <linux/bitfield.h> + +/* 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 EIP port. + */ +#define PPE_BM_PORT_FC_MODE_ADDR 0x600100 +#define PPE_BM_PORT_FC_MODE_ENTRIES 15 +#define PPE_BM_PORT_FC_MODE_INC 0x4 +#define PPE_BM_PORT_FC_MODE_EN BIT(0) + +#define PPE_BM_PORT_GROUP_ID_ADDR 0x600180 +#define PPE_BM_PORT_GROUP_ID_ENTRIES 15 +#define PPE_BM_PORT_GROUP_ID_INC 0x4 +#define PPE_BM_PORT_GROUP_ID_SHARED_GROUP_ID GENMASK(1, 0) + +#define PPE_BM_SHARED_GROUP_CFG_ADDR 0x600290 +#define PPE_BM_SHARED_GROUP_CFG_ENTRIES 4 +#define PPE_BM_SHARED_GROUP_CFG_INC 0x4 +#define PPE_BM_SHARED_GROUP_CFG_SHARED_LIMIT GENMASK(10, 0) + +#define PPE_BM_PORT_FC_CFG_TBL_ADDR 0x601000 +#define PPE_BM_PORT_FC_CFG_TBL_ENTRIES 15 +#define PPE_BM_PORT_FC_CFG_TBL_INC 0x10 +#define PPE_BM_PORT_FC_W0_REACT_LIMIT GENMASK(8, 0) +#define PPE_BM_PORT_FC_W0_RESUME_THRESHOLD GENMASK(17, 9) +#define PPE_BM_PORT_FC_W0_RESUME_OFFSET GENMASK(28, 18) +#define PPE_BM_PORT_FC_W0_CEILING_LOW GENMASK(31, 29) +#define PPE_BM_PORT_FC_W1_CEILING_HIGH GENMASK(7, 0) +#define PPE_BM_PORT_FC_W1_WEIGHT GENMASK(10, 8) +#define PPE_BM_PORT_FC_W1_DYNAMIC BIT(11) +#define PPE_BM_PORT_FC_W1_PRE_ALLOC GENMASK(22, 12) + +#define PPE_BM_PORT_FC_SET_REACT_LIMIT(tbl_cfg, value) \ + u32p_replace_bits((u32 *)tbl_cfg, value, PPE_BM_PORT_FC_W0_REACT_LIMIT) +#define PPE_BM_PORT_FC_SET_RESUME_THRESHOLD(tbl_cfg, value) \ + u32p_replace_bits((u32 *)tbl_cfg, value, PPE_BM_PORT_FC_W0_RESUME_THRESHOLD) +#define PPE_BM_PORT_FC_SET_RESUME_OFFSET(tbl_cfg, value) \ + u32p_replace_bits((u32 *)tbl_cfg, value, PPE_BM_PORT_FC_W0_RESUME_OFFSET) +#define PPE_BM_PORT_FC_SET_CEILING_LOW(tbl_cfg, value) \ + u32p_replace_bits((u32 *)tbl_cfg, value, PPE_BM_PORT_FC_W0_CEILING_LOW) +#define PPE_BM_PORT_FC_SET_CEILING_HIGH(tbl_cfg, value) \ + u32p_replace_bits((u32 *)(tbl_cfg) + 0x1, value, PPE_BM_PORT_FC_W1_CEILING_HIGH) +#define PPE_BM_PORT_FC_SET_WEIGHT(tbl_cfg, value) \ + u32p_replace_bits((u32 *)(tbl_cfg) + 0x1, value, PPE_BM_PORT_FC_W1_WEIGHT) +#define PPE_BM_PORT_FC_SET_DYNAMIC(tbl_cfg, value) \ + u32p_replace_bits((u32 *)(tbl_cfg) + 0x1, value, PPE_BM_PORT_FC_W1_DYNAMIC) +#define PPE_BM_PORT_FC_SET_PRE_ALLOC(tbl_cfg, value) \ + u32p_replace_bits((u32 *)(tbl_cfg) + 0x1, value, PPE_BM_PORT_FC_W1_PRE_ALLOC) +#endif
The BM (Buffer Management) config controls the pause frame generated on the PPE port. There are maximum 15 BM ports and 4 groups supported, all BM ports are assigned to group 0 by default. The number of hardware buffers configured for the port influence the threshold of the flow control for that port. Signed-off-by: Luo Jie <quic_luoj@quicinc.com> --- drivers/net/ethernet/qualcomm/ppe/Makefile | 2 +- drivers/net/ethernet/qualcomm/ppe/ppe.c | 5 + drivers/net/ethernet/qualcomm/ppe/ppe_config.c | 195 +++++++++++++++++++++++++ drivers/net/ethernet/qualcomm/ppe/ppe_config.h | 12 ++ drivers/net/ethernet/qualcomm/ppe/ppe_regs.h | 59 ++++++++ 5 files changed, 272 insertions(+), 1 deletion(-)