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 [off-list ref] wrote:

On Thu, Aug 22, 2019 at 01:27:17AM +0200, Marek Behún wrote:

quoted

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