[U-Boot] [PATCH 7/9] net: rtl8169: Properly align buffers

[Simon Glass]

Hi Nobuhiro,

On 12 November 2014 16:38, Nobuhiro Iwamatsu <iwamatsu at nigauri.org> wrote:
> Hi!
>
> I comment to latest patch.
>
> Best regards,
>   Nobuhiro

For me this one gives a warning, but I suppose the warning is correct.

>
> 2014-11-13 1:23 GMT+09:00 Simon Glass <sjg at chromium.org>:
>> On 22 August 2014 03:15, Thierry Reding <thierry.reding at gmail.com> wrote:
>>> On Wed, Aug 20, 2014 at 01:29:57PM -0600, Stephen Warren wrote:
>>>> On 08/18/2014 02:00 AM, Thierry Reding wrote:
>>>> >From: Thierry Reding <treding at nvidia.com>
>>>> >
>>>> >RX and TX descriptor rings should be aligned to 256 byte boundaries. Use
>>>> >the DEFINE_ALIGN_BUFFER() macro to define the buffers so that they don't
>>>> >have to be manually aligned later on. Also make sure that the buffers do
>>>> >align to cache-line boundaries in case the cache-line is higher than the
>>>> >256 byte alignment requirements of the NIC.
>>>> >
>>>> >Also add a warning if the cache-line size is larger than the descriptor
>>>> >size, because the driver may discard changes to descriptors made by the
>>>> >hardware when requeuing RX buffers.
>>>>
>>>> >@@ -909,6 +899,18 @@ static int rtl_init(struct eth_device *dev, bd_t *bis)
>>>>
>>>> >+    /*
>>>> >+     * Warn if the cache-line size is larger than the descriptor size. In
>>>> >+     * such cases the driver will likely fail because the CPU needs to
>>>> >+     * flush the cache when requeuing RX buffers, therefore descriptors
>>>> >+     * written by the hardware may be discarded.
>>>> >+     */
>>>> >+    if (ARCH_DMA_MINALIGN > sizeof(struct RxDesc))
>>>> >+            printf("WARNING: cache-line size is larger than descriptor size\n");
>>>>
>>>> I'd be tempted to make that a compile-time #error (or perhaps just a
>>>> #warning) Perhaps #error would break compilation of existing boards though?
>>>
>>> There are two SH4 boards that use the rtl8169 (r7780mp and sh7785lcr)
>>> for which this condition is true, so #error would break them (well,
>>> technically not r7780mp since it comments out CONFIG_RTL8169 in the
>>> configuration). I'll make it a #warning instead.
>>
>> Adding the maintainer for comment.
>>
>> Regards,
>> Simon
>
>
>
> --
> Nobuhiro Iwamatsu
>    iwamatsu at {nigauri.org / debian.org}
>    GPG ID: 40AD1FA6

Regards,
Simon