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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 14 Nov 2016 13:29:21 +1100
From:   Vivien Didelot <vivien.didelot@...oirfairelinux.com>
To:     Andrew Lunn <andrew@...n.ch>, David Miller <davem@...emloft.net>
Cc:     netdev <netdev@...r.kernel.org>, Andrew Lunn <andrew@...n.ch>
Subject: Re: [PATCH net-next 07/11] net: dsa: mv88e6xxx: Add mv88e6390 statistics unit init

Hi Andrew,

Andrew Lunn <andrew@...n.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