[U-Boot] [PATCH 7/9][v2] net: tsec: Use portable types and accessors for BDs

Claudiu Manoil
2013-10-04T08:27:22Z

On 10/3/2013 9:37 PM, Scott Wood wrote:
> On Thu, 2013-10-03 at 14:48 +0300, Claudiu Manoil wrote:
>> +static inline u16 read_txbd_stat(uint idx)
>> +{
>> + return in_be16((u16 __iomem *)&txbd[idx].status);
>> +}
>> +
>> +static inline void write_txbd_stat(uint idx, u16 status)
>> +{
>> + out_be16((u16 __iomem *)&txbd[idx].status, status);
>> +}
>> +
>> +static inline u16 read_rxbd_stat(uint idx)
>> +{
>> + return in_be16((u16 __iomem *)&rxbd[idx].status);
>> +}
>> +
>> +static inline void write_rxbd_stat(uint idx, u16 status)
>> +{
>> + out_be16((u16 __iomem *)&rxbd[idx].status, status);
>> +}
>
> Do you need __force on these to make sparse happy?
>
No, we don't need __force in this case, in_be/out_be are less
restrictive and take plain unsigned pointers (not __beNN pointers).
On the other hand, they require the __iomem address space marker, to
make sparse happy.

> I'd rather see these declared as __iomem than use casts (at which point,
> you probably don't need per-field accessor functions).
>
Me too, but I wasn't sure how to do that. I thought __iomem works with
pointer declarations only. But it turns out it works this way too:

-static struct txbd8 txbd[TX_BUF_CNT] __aligned(8);
-static struct rxbd8 rxbd[PKTBUFSRX] __aligned(8);
[...]
+static struct txbd8 __iomem txbd[TX_BUF_CNT] __aligned(8);
+static struct rxbd8 __iomem rxbd[PKTBUFSRX] __aligned(8);

[...]
- for (i = 0; read_txbd_stat(tx_idx) & TXBD_READY; i++) {
+ for (i = 0; in_be16(&txbd[tx_idx].status) & TXBD_READY; i++) {
[...]

And sparse doesn't complain about it. In this case I'll drop the
read_txbd_stat() and friends. Is this acceptable?

Thanks.
Claudiu