| Message ID | 20240628-am65-cpsw-multi-rx-v2-0-c399cb77db56@kernel.org |
|---|---|
| Headers | show |
| Series | net: ethernet: ti: am65-cpsw: Add multi queue RX support | expand |
On Fri, Jun 28, 2024 at 03:01:54PM +0300, Roger Quadros wrote: > The Policer registers in the ALE register space are just shadow registers > and use an index field in the policer table control register to read/write > to the actual Polier registers. > Add helper functions to Read and Write to Policer registers. > > Also add a helper function to set the thread value to classifier/policer > mapping. Any packet that first matches the classifier will be sent to the > thread (flow) that is set in the classifer to thread mapping table. nit: classifier Flagged by checkpatch.pl --codespell > If not set then it goes to the default flow. > > Signed-off-by: Roger Quadros <rogerq@kernel.org> > --- > drivers/net/ethernet/ti/cpsw_ale.c | 24 ++++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > > diff --git a/drivers/net/ethernet/ti/cpsw_ale.c b/drivers/net/ethernet/ti/cpsw_ale.c > index 7bd0dc20f894..75a17184d34c 100644 > --- a/drivers/net/ethernet/ti/cpsw_ale.c > +++ b/drivers/net/ethernet/ti/cpsw_ale.c > @@ -1626,3 +1626,27 @@ u32 cpsw_ale_get_num_entries(struct cpsw_ale *ale) > { > return ale ? ale->params.ale_entries : 0; > } > + > +/* Reads the specified policer index into ALE POLICER registers */ > +static void cpsw_ale_policer_read_idx(struct cpsw_ale *ale, u32 idx) > +{ > + idx &= ALE_POLICER_TBL_INDEX_MASK; > + writel_relaxed(idx, ale->params.ale_regs + ALE_POLICER_TBL_CTL); > +} > + > +/* Writes the ALE POLICER registers into the specified policer index */ > +static void cpsw_ale_policer_write_idx(struct cpsw_ale *ale, u32 idx) > +{ > + idx &= ALE_POLICER_TBL_INDEX_MASK; > + idx |= ALE_POLICER_TBL_WRITE_ENABLE; > + writel_relaxed(idx, ale->params.ale_regs + ALE_POLICER_TBL_CTL); > +} > + > +/* enables/disables the custom thread value for the specified policer index */ > +static void cpsw_ale_policer_thread_idx_enable(struct cpsw_ale *ale, u32 idx, > + u32 thread_id, bool enable) > +{ > + regmap_field_write(ale->fields[ALE_THREAD_CLASS_INDEX], idx); > + regmap_field_write(ale->fields[ALE_THREAD_VALUE], thread_id); > + regmap_field_write(ale->fields[ALE_THREAD_ENABLE], enable ? 1 : 0); > +} > I like that this patch-set is broken out into nice discrete patches, including this one. So I'm in two minds about the comment I'm about to make, but here goes. As these helpers are unused this raises Warnings with W=1 builds on gcc-13 and clang-18, which is generally undesirable for networking patches. I can think of a few options here; * Ignore the warnings * Squash this patch into the following one * Add some annotations, e.g. __maybe_unused or __always_unused. Likely dropped in the next patch. I think I lean towards the last option. But I won't push the point any further regardless.
On Fri, Jun 28, 2024 at 03:01:52PM +0300, Roger Quadros wrote: > Use regfields for number of ALE Entries and Policers. > > The variants that support Policers/Classifiers have the number > of policers encoded in the ALE_STATUS register. > > Use that and show the number of Policers in the ALE info message. > > Signed-off-by: Roger Quadros <rogerq@kernel.org> Reviewed-by: Simon Horman <horms@kernel.org>
On Fri, Jun 28, 2024 at 03:01:56PM +0300, Roger Quadros wrote: > Now that we support multiple RX queues, enable default priority > to flow mapping so that higher priority packets come on higher > channels (flows). > > The Classifier checks for PCP/DSCP priority in the packet and > routes them to the appropriate flow. > > Signed-off-by: Roger Quadros <rogerq@kernel.org> Reviewed-by: Simon Horman <horms@kernel.org>
On 01/07/2024 10:35, Simon Horman wrote: > On Fri, Jun 28, 2024 at 03:01:54PM +0300, Roger Quadros wrote: >> The Policer registers in the ALE register space are just shadow registers >> and use an index field in the policer table control register to read/write >> to the actual Polier registers. >> Add helper functions to Read and Write to Policer registers. >> >> Also add a helper function to set the thread value to classifier/policer >> mapping. Any packet that first matches the classifier will be sent to the >> thread (flow) that is set in the classifer to thread mapping table. > > nit: classifier > > Flagged by checkpatch.pl --codespell will fix. > >> If not set then it goes to the default flow. >> >> Signed-off-by: Roger Quadros <rogerq@kernel.org> >> --- >> drivers/net/ethernet/ti/cpsw_ale.c | 24 ++++++++++++++++++++++++ >> 1 file changed, 24 insertions(+) >> >> diff --git a/drivers/net/ethernet/ti/cpsw_ale.c b/drivers/net/ethernet/ti/cpsw_ale.c >> index 7bd0dc20f894..75a17184d34c 100644 >> --- a/drivers/net/ethernet/ti/cpsw_ale.c >> +++ b/drivers/net/ethernet/ti/cpsw_ale.c >> @@ -1626,3 +1626,27 @@ u32 cpsw_ale_get_num_entries(struct cpsw_ale *ale) >> { >> return ale ? ale->params.ale_entries : 0; >> } >> + >> +/* Reads the specified policer index into ALE POLICER registers */ >> +static void cpsw_ale_policer_read_idx(struct cpsw_ale *ale, u32 idx) >> +{ >> + idx &= ALE_POLICER_TBL_INDEX_MASK; >> + writel_relaxed(idx, ale->params.ale_regs + ALE_POLICER_TBL_CTL); >> +} >> + >> +/* Writes the ALE POLICER registers into the specified policer index */ >> +static void cpsw_ale_policer_write_idx(struct cpsw_ale *ale, u32 idx) >> +{ >> + idx &= ALE_POLICER_TBL_INDEX_MASK; >> + idx |= ALE_POLICER_TBL_WRITE_ENABLE; >> + writel_relaxed(idx, ale->params.ale_regs + ALE_POLICER_TBL_CTL); >> +} >> + >> +/* enables/disables the custom thread value for the specified policer index */ >> +static void cpsw_ale_policer_thread_idx_enable(struct cpsw_ale *ale, u32 idx, >> + u32 thread_id, bool enable) >> +{ >> + regmap_field_write(ale->fields[ALE_THREAD_CLASS_INDEX], idx); >> + regmap_field_write(ale->fields[ALE_THREAD_VALUE], thread_id); >> + regmap_field_write(ale->fields[ALE_THREAD_ENABLE], enable ? 1 : 0); >> +} >> > > I like that this patch-set is broken out into nice discrete patches, > including this one. So I'm in two minds about the comment I'm about to > make, but here goes. > > As these helpers are unused this raises Warnings with W=1 builds on > gcc-13 and clang-18, which is generally undesirable for networking patches. > > I can think of a few options here; > * Ignore the warnings > * Squash this patch into the following one > * Add some annotations, e.g. __maybe_unused or __always_unused. > Likely dropped in the next patch. > > I think I lean towards the last option. > But I won't push the point any further regardless. Thanks for the suggestions. I do not like using any of the __unused flags. So I'm leaning towards your second solution. Will fix this in next revision.
Hi, am65-cpsw can support up to 8 queues at Rx. So far we have been using only one queue (i.e. default flow) for all RX traffic. This series adds multi-queue support. The driver starts with 1 RX queue by default. User can increase the RX queues via ethtool, e.g. 'ethtool -L ethx rx <N>' The series also adds regmap and regfield support to some of the ALE registers. It adds Policer/Classifier registers and fields. Converting the existing ALE control APIs to regfields can be a separate exercise. Some helper functions are added to read/write to the Policer/Classifier registers and a default Classifier setup function is added that routes packets based on their PCP/DSCP priority to different RX queues. Signed-off-by: Roger Quadros <rogerq@kernel.org> --- Changes in v2: - rebase to net/next - fixed RX stall issue during iperf - Link to v1: https://lore.kernel.org/r/20240606-am65-cpsw-multi-rx-v1-0-0704b0cb6fdc@kernel.org --- Roger Quadros (7): net: ethernet: ti: am65-cpsw: Introduce multi queue Rx net: ethernet: ti: cpsw_ale: use regfields for ALE registers net: ethernet: ti: cpsw_ale: use regfields for number of Entries and Policers net: ethernet: ti: cpsw_ale: add Policer and Thread control register fields net: ethernet: ti: cpsw_ale: add policer/classifier helpers net: ethernet: ti: cpsw_ale: add helper to setup classifier defaults net: ethernet: ti: am65-cpsw: setup priority to flow mapping drivers/net/ethernet/ti/am65-cpsw-ethtool.c | 59 +++-- drivers/net/ethernet/ti/am65-cpsw-nuss.c | 361 ++++++++++++++++------------ drivers/net/ethernet/ti/am65-cpsw-nuss.h | 35 +-- drivers/net/ethernet/ti/cpsw_ale.c | 273 +++++++++++++++++++-- drivers/net/ethernet/ti/cpsw_ale.h | 62 ++++- 5 files changed, 583 insertions(+), 207 deletions(-) --- base-commit: 84562f9953ec5f91a4922baa2bd4f2d4f64fac31 change-id: 20240606-am65-cpsw-multi-rx-fb6cf8dea5eb Best regards,