Re: [PATCH net-next v2 8/9] net: dsa: mv88e6xxx: support Block Address setting in hidden registers

Hi Marek,

On Sat, 24 Aug 2019 22:52:16 +0200, Marek Behun [off-list ref] wrote:

quoted

There's something I'm having trouble to follow here. This series keeps
adding and modifying its own code. Wouldn't it be simpler for everyone
if you directly implement the final mv88e6xxx_port_hidden_{read,write}
functions taking this block argument, and update the code to switch to it?

I wanted the commits to be atomic, in the sense that one commit does
not do three different things at once. Renaming macros is cosmetic
change, and moving functions to another file is a not a semantic
change, while adding additional argument to functions is a semantic
change. I can of course do all in one patch, but I though it would be
better not to.

You add code, move it, rename it, then change it. It is hard to follow and
read, especially in a series of 9 patches.

I think you could do it the other way around. For example implement the
.serdes_get_lane operation, its users, the mv88e6xxx_port_hidden_* API, its
users, remove or convert old code, etc. Atomicity has nothing to do with it.

quoted

While at it, I don't really mind the "hidden" name, but is this the name
used in the documentation, if any?

Yes, the registers are indeed named Hidden Registers in documentation.

OK good to know, port_hidden_ makes sense indeed then.


Thanks,

	Vivien