netdev - Re: [PATCH net-next 03/10] net: dsa: mv88e6xxx: move hidden registers operations in own file

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:   Thu, 22 Aug 2019 13:21:18 -0400
From:   Vivien Didelot <vivien.didelot@...il.com>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     Marek Behún <marek.behun@....cz>,
        netdev@...r.kernel.org, Florian Fainelli <f.fainelli@...il.com>,
        Vladimir Oltean <olteanv@...il.com>
Subject: Re: [PATCH net-next 03/10] net: dsa: mv88e6xxx: move hidden registers
 operations in own file

Hi Marek, Andrew,

On Thu, 22 Aug 2019 15:10:47 +0200, Andrew Lunn <andrew@...n.ch> wrote:
> On Thu, Aug 22, 2019 at 01:27:17AM +0200, Marek Behún wrote:
> > This patch moves the functions operating on the hidden debug registers
> > into it's own file, hidden.c.
> 
> Humm, actually...
> 
> These are in the port register space. Maybe it would be better to move
> them into port.c/port.h?
> 
> What you really need is that they have global scope within the driver
> so you can call them. So add the functions definitions to port.h.
> 
> Vivien, what do you think?

Andrew's correct. Code accessing internal registers in the mv88e6xxx driver
is split per internal SMI device. An internal SMI device is a "column" of 32
registers found in the datasheet, accessed via its dedicated internal SMI
device address, like ports, Global 1 (often 0x1b), Global 2 (often 0x1c),
and so on. Each internal SMI device has its unique header file, describing
all registers it contains. Then if the corresponding .c file has a portion
specific enough, it can be moved to its own .c file, like global1_atu.c,
global1_vtu.c, etc.

So keep these port registers definitions ordered in port.h with a naming as
closed to the documentation as possible, prefixing them with MV88E6XXX_PORT_
(or the model of reference if that is specific to a few models only), and
please describe the bits with an ordered 0x1234 format as well. The port.h
header is fortunately already a good example of how it should be done.

Then you can include it in a new port_hidden.c file, which implements
mv88e6xxx_port_hidden_* internal helpers (or mv88e6789_port_ if specific to a
model again) to access these hidden port registers, and use them as convenience
in chip.c:mv88e6xxx_errata_setup() or wherever a feature is implemented.

Thanks a lot,

	Vivien