RE: [PATCH] net: ti: netcp: restore get/set_pad_info() functionality

-----Original Message-----
From: David Laight [mailto:David.Laight@ACULAB.COM]
Sent: Tuesday, February 09, 2016 11:38 AM
To: Karicheri, Muralidharan; Strashko, Grygorii; netdev@vger.kernel.org; David S . Miller;
Arnd Bergmann
Cc: Cooper Jr., Franklin; Nori, Sekhar; linux-kernel@vger.kernel.org; Kwok, WingMan; N,
Mugunthan V
Subject: RE: [PATCH] net: ti: netcp: restore get/set_pad_info() functionality

From: Karicheri, Muralidharan

quoted

Sent: 09 February 2016 16:19

quoted

...

quoted

quoted

In reality the 'pad' fields ought to be renamed - since they aren't pads.
Perhaps they should be a union?

quoted

No. At the end of the descriptor, host software can add scratchpad
which is not modified by the hardware, but is used by the driver.
So please don't rename.

So comment in the definition that the hardware doesn't modify them.
The driver is defining these fields and they are definitely NOT padding.

It is scratch pad, not padding. Looks like pad is a confusing name.
Can be renamed to sw_data to be in sync with spec below.

The hardware spec from
http://www.ti.com/lit/ug/sprugr9h/sprugr9h.pdf

The other SW data portion of the descriptor exists after all of the
defined words and is reserved for use by the host software to store
completely private data. This region is not used in any way by the DMA
or queue manager modules in a Multicore Navigator system and these
modules will not modify any bytes within this region.

Right, so comment that the hardware doesn't look at the fields.
But name/type the structure fields to indicate what they contain.
Maybe sw_buf_len etc - but I suspect there are much more meaningful names.

The descriptors are usable by different drivers, one driver may use it as 
buf ptr/ len, other for something else. So they should remain as generic
and it is up to individual drivers to use it in whatever way it requires.
My suggestion is to rename pad field in struct knav_dma_desc to sw_data
to avoid confusion. i.e 

+       __le32  pad[4];

to 

+       __le32  sw_data[4];

Murali

David