lkml.org 
[lkml]   [2013]   [Feb]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH] drivers/block/xsysace - replace in(out)_8/in(out)_be16/in(out)_le16 with generic iowrite(read)8/16(be)
On Tue, Feb 5, 2013 at 11:07 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Wed, 2013-02-06 at 01:25 +0400, Alexey Brodkin wrote:
>> Sounds good but how should one tell which approach is correct? For
>> example here - is the one implemented by Xilinx is golden reference or
>> not?
>
> So I'm reading that PDF you pointed to. So far what I can see is:
>
> - In 8-bit mode you only do 8-bit accesses, so endianness should be
> totally irrelevant (at least the pdf says so)
>
> - In 16-bit mode, that's where things become interesting... the doc
> says:
>
> PLB Data Bus | System ACE Data Bus
> ----------------------+--------------------
> PLB_DBus[8 : 15] | SysACE_MPD[15 : 8]
> PLB_DBus[0 : 7] | SysACE_MPD[7 : 0]
>
> Now, I'm not 100% of the bit numbering used by Xilinx here but it smells
> like PLB used ppc numbering and SystemACE use the classic numbering, in
> which case the above would mean that the MSB of the PLB is connected to
> the LSB of the SystemACE and vice-versa.
>
> If that is the case then this is the *correct* wiring and means that the
> data port (if any) doesn't need any byteswapping. It also means that the
> registers need byteswap on BE, as expected for a LE device. IE. Just
> always use ioread32 (or _rep variants for the data port if there's such
> a thing on it).
>
> So that looks good... unless I misunderstood the Xilinx spec, this looks
> like the right way to do and the only one we should support.

Huh? That makes no sense. This device out in the wild with both big
and little endian bus attachments. You can argue all day that one of
them is wrong, but it doesn't matter. It exists, is used, and must be
supported. In fact, the driver already knows about this and figures
out at runtime how the device is wired up to the bus. This is not the
problem.

BTW, that document describes only one of the systemace bus
attachments. There is a different on used on Microblaze little-endian,
and some boards have the SystemACE directly wired to the SoC external
bus (no adapter IP).

The only problem that I see is that the ARM and Microblaze
ioread16be/iowrite16be helpers are missing barriers which smells like
a bug and should be fixed.

Michal, have you tested Alexey's patch? If it works for you then I'm
comfortable with acking it. It looks correct to me.

g.

--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.


\
 
 \ /
  Last update: 2013-02-06 11:42    [W:0.163 / U:0.148 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site