Re: [PATCH v2 2/3] drivers/block/xsysace - use "_rep" accessors with CPU endianess for data

[Michal Simek <monstr@monstr.eu>]

[-- Attachment #1: Type: text/plain, Size: 3989 bytes --]

On 06/25/2013 08:32 AM, Alexey Brodkin wrote:
> On 06/25/2013 09:58 AM, Michal Simek wrote:
>> On 06/24/2013 10:26 AM, Alexey Brodkin wrote:
>>> Initially different data accessors were used for LE abd BE CPUs:
>>> "ioread16" in "ace_datain_be16" and "ioread16be" in "ace_datain_le16".
>>> The same with writes.
>>>
>>> While it worked in some cases (for example on BE PPC) it didn't work in
>>> others (LE ARC).
>>
>> I am not sure about this. It seems to me that what you need to do
>> is swapped wires in your hw design to use the same configuration
>> as is used on ppc and microblaze for data access.
>  >
>>> Mentioned functions access data (by 16-bit chunks) from storage (i.e.
>>> CompactFlash card) via DATABUFREG of Xilinx SystemACE CF controller.
>>> And to interpret data properly CPU needs to access data in DATABUFREG
>>> with native endianess.
>>
>> I have had a lot of discussions about using native endianess.
>> This driver supports endian detection on register side
>> but not on data side.
>> Is this soft IP? If yes then just swapped wires on bus and use
>> standard configuration.
> 
> I don't think there's a wiring problem.
> For starters "Xilinx SystemACE CF controller" (at least the one I'm 
> dealing with on "Xilinx ML-509" board) is a real hardware IC (with part 
> number XCCACE-TQ144I).
> 
> And what about your HW? Is your SystemACE controller is a soft-IP?

bus-> sysace soft IP -> connetion to the real chip(MPD below) -> chip -> CF

In xilinx mhs file there are MPD pins which are data pins
and they are wired like this for 16bit mode
PORT fpga_0_SysACE_CompactFlash_SysACE_MPD_pin = fpga_0_SysACE_CompactFlash_SysACE_MPD_pin, DIR = IO, VEC = [15:0]
and like this for 8bit mode.
PORT SysACE_MPD = SysACE_MPD, DIR = IO, VEC = [7:0]

I am not sure about configuration but if you have something similar
what xilinx has then you should be simple able to change data pins
and this will reflect linux driver.

what configuration do you use?

> 
> As described in corresponding datasheet 
> (http://www.xilinx.com/support/documentation/data_sheets/ds080.pdf) 
> CF-card is connected to this IC directly - so CPU itself doesn't have 
> any connection to CF.

I am not talking about connection between system ace chip and CF.
xilinx configuration is done as I have described above where
we can setup data lines order. I haven't done any experiment with it
but seems to me straight forward to do it.

> CPU only can access (read/write) SystemACE's registers and by these actions:
> 1. Config SystemACE or read its configuration and status (registers 
> 0x00-0x1d).
> 2. Read/write data from/to CF-card (register 0x40).
> 
> And as long as configuration/status registers access is proven to work I 
> expect access to data via just reads/writes from/to another same 
> register should work as well.
>
>> Grant is driver owner and he has to decide if this is acceptable
>> or not.
> 
> As far as I may see from latest MAINTAINERS file grant is no longer 
> maintains it. So who may take any decision now? Arnd?

I should probably assign it to my me as the part of my microblaze
responsibility. And xilinx virtex as part of my work for xilinx.

MAINTAINERS: Update Grant's email address and maintainership
sha1: 19624236cce1b33a5d43895a92e3a9d438dc36e2


>> I can test it on microblaze hw.
> 
> Would be very nice and helpful if you give it a shot on Microblaze HW.
> BTW what is an endianess of this HW? Only BE or both BE/LE?

8bit BE/LE 16bit BE. I have one 16bit LE configuration too
but before I do testing on real HW let's clear your connection.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]