RE: [PATCH v6 2/8] thunderbolt: Updating the register definitions

On Sun, Sep 11 2016, 03:02 AM, Andreas Noever wrote:

On Mon, Aug 1, 2016 at 2:23 PM, Amir Levy [off-list ref] wrote:

quoted

Adding more Thunderbolt(TM) register definitions and some helper
macros.

Thinking about this again I would prefer it if you would put your definitions
into a separate file under icm/ (even if there is some duplication). The style
(bitfields vs. genmask) is different between the drivers and for a reader it is
difficult to find out what is actually supposed to be used by the two drivers
(ring_desc vs tbt_buf_desc or the ring RING_INT_EN/DISABLE macros in the
header file vs. ring_interrupt_active in nhi.c).

This would also completely separate the two drivers.

Andreas

I'm also in favor of completely separating the drivers, but is it the right thing to do with the register definitions
when the underlying registers layout is exactly the same?

Note that bitfields are not so recommended when you care about the format/order of bits, like in the ring descriptor.

Amir