From mboxrd@z Thu Jan  1 00:00:00 1970
From: Claudiu Manoil <claudiu.manoil@freescale.com>
Date: Fri, 4 Oct 2013 19:25:40 +0300
Subject: [U-Boot] [PATCH 7/9][v2] net: tsec: Use portable types and
 accessors for BDs
In-Reply-To: <1380901846.7979.2.camel@snotra.buserror.net>
References: <1380752101.12932.127.camel@snotra.buserror.net>
	<1380800932-22552-1-git-send-email-claudiu.manoil@freescale.com>
	<1380825428.12932.154.camel@snotra.buserror.net>
	<524E7BEA.2070103@freescale.com>
	<1380901846.7979.2.camel@snotra.buserror.net>
Message-ID: <524EEC04.1030105@freescale.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de


On 10/4/2013 6:50 PM, Scott Wood wrote:
> On Fri, 2013-10-04 at 11:27 +0300, Claudiu Manoil wrote:
>> 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 thought you'd need __force to convert a non-iomem pointer to an
> __iomem pointer.
>
>>> 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:
>
> Even if that were the case, you could put it on the pointers, which is
> how it's usually used.
>
>>
>> -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?
>
> Yes.
>

[v3] declaring the BDs as __iomem to avoid casting submitted:
http://patchwork.ozlabs.org/patch/280664/

Thanks.
Claudiu