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@xxxxxxxxxx>
>> ---
>> 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.
--
cheers,
-roger
