Re: [PATCH net-next 07/11] net: dsa: mv88e6xxx: Add mv88e6390 statistics unit init

[Vivien Didelot]

Hi Andrew,

Andrew Lunn <and...@lunn.ch> writes:

> The statistics unit on the mv88e6390 needs to the configured in a
> different register to the others as to what histogram statistics is
> should return.

Can you re-phrase the above please?

> +static int mv88e6390_stats_init(struct mv88e6xxx_chip *chip)
> +{
> +     u16 val;
> +     int err;
> +
> +     err = mv88e6xxx_g1_read(chip, GLOBAL_CONTROL_2, &val);
> +     if (err)
> +             return err;
> +
> +     val |= GLOBAL_CONTROL_2_HIST_RX_TX;
> +
> +     err = mv88e6xxx_g1_write(chip, GLOBAL_CONTROL_2, val);
> +
> +     return err;
> +}

Can you please move this Global 1 specific helper in global1.c under an
ordered snippet such as:

    /* Offset 0x1C: Global Control 2 */

    int mv88e6xxx_g1_set_foo(struct mv88e6xxx_chip *chip)
    {
        ...
    }

I'd like internal SMI devices to be self documented in their specific
files and easy to hack for new developers. Ordered helpers will help.

Also, the helper should reflect what it really does. It is used to set
the Histogram Counters Mode. So please name it accordingly, something
like mv88e6xxx_g1_set_hist_count_mode().

Thanks,

        Vivien