Hi Roger,

On Tue, Sep 10, 2024 at 11:25 AM Roger Quadros [off-list ref] 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>
---
Changelog:
v4:
- reverse Xmas tree declaration order fixes

Thanks for your patch, which is now commit 11cbcfeaa79e5c76 ("net:
ethernet: ti: cpsw_ale: use regfields for number of Entries
and Policers").

This is causing the following warning on BeagleBone Black:

    WARNING: CPU: 0 PID: 34 at drivers/base/regmap/regmap.c:1208
devm_regmap_field_alloc+0xac/0xc8
    invalid empty mask defined
    CPU: 0 UID: 0 PID: 34 Comm: kworker/u4:3 Not tainted
6.11.0-rc7-boneblack-01443-g11cbcfeaa79e #152
    Hardware name: Generic AM33XX (Flattened Device Tree)
    Workqueue: events_unbound deferred_probe_work_func
    Call trace:
     unwind_backtrace from show_stack+0x10/0x14
     show_stack from dump_stack_lvl+0x68/0x88
     dump_stack_lvl from __warn+0x6c/0x1a8
     __warn from warn_slowpath_fmt+0x1bc/0x1d0
     warn_slowpath_fmt from devm_regmap_field_alloc+0xac/0xc8
     devm_regmap_field_alloc from cpsw_ale_create+0x10c/0x36c
     cpsw_ale_create from cpsw_init_common+0x1fc/0x310

You are adding these entries only to ale_fields_cpsw_nu[], not
to ale_fields_cpsw[], while cpsw_ale_regfield_init() loops over
ALE_FIELDS_MAX entries, whether they are valid or not:

    static int cpsw_ale_regfield_init(struct cpsw_ale *ale)
    {
            const struct reg_field *reg_fields = ale->params.reg_fields;
            struct device *dev = ale->params.dev;
            struct regmap *regmap = ale->regmap;
            int i;

            for (i = 0; i < ALE_FIELDS_MAX; i++) {
                    ale->fields[i] = devm_regmap_field_alloc(dev, regmap,
                                                             reg_fields[i]);

                    [...]
            }

            return 0;
    }

I tried fixing this by skipping entries where all of .reg, .lsb,
and .msb are zero, but that doesn't work as that runs beyond the
end of ale_fields_cpsw[], thus operating on random data.
I think you do have to store the size of the array, instead of assuming
ALE_FIELDS_MAX entries everywhere.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds