Skip to contentSkip to conversation

Xilinx SystemACE device driver

RFC In review Grant Likely 3 participants 7 messages Apr 15, 2007 – Apr 27, 2007 Block layer

lore
Share & tools
lore
#0 lore raw

RFC 1/1 Xilinx SystemACE device driver

Add support for block device access to the Xilinx SystemACE Compact flash interface

---
I think this driver is in pretty good shape. I've got a few things to clean up a bit. Specifically, I'm still working on error handling and making sure that the state machine is sane at all times.

I would appreciate any review/comments. One area where I am undecided is the format of the state machine. The current code uses one big function with a large switch() statment for each state. I'm considering breaking this up into a seperate function for each state, and adding a static state table with pointers to each state function.

I feel this driver is pretty close to done, and I'd like to get it into mainline for the 2.6.22 timeframe.

Cheers,
g.

 drivers/block/Kconfig   |    6 +
 drivers/block/Makefile  |    1 +
 drivers/block/xsysace.c | 1070 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 1077 insertions(+), 0 deletions(-)
 create mode 100644 drivers/block/xsysace.c
drivers/block/Kconfig +6 -0
index 17ee97f..08ad23c 100644
@@ -453,6 +453,12 @@ config ATA_OVER_ETH
 	This driver provides Support for ATA over Ethernet block
 	devices like the Coraid EtherDrive (R) Storage Blade.
 
+config XILINX_SYSACE
+	tristate "Xilinx SystemACE support"
+	depends on XILINX_VIRTEX
+	help
+	  Include support for the Xilinx SystemACE CompactFlash interface
+
 endmenu
 
 endif
drivers/block/Makefile +1 -0
index dd88e33..31ea323 100644
@@ -28,4 +28,5 @@ obj-$(CONFIG_BLK_DEV_CRYPTOLOOP) += cryptoloop.o
 obj-$(CONFIG_VIODASD)		+= viodasd.o
 obj-$(CONFIG_BLK_DEV_SX8)	+= sx8.o
 obj-$(CONFIG_BLK_DEV_UB)	+= ub.o
+obj-$(CONFIG_XILINX_SYSACE)	+= xsysace.o
 
drivers/block/xsysace.c new file +1070 -0

1,070 lines not on this page · raw message

Signature · 1 line
1.5.1
#1 lore raw

Hi Grant,

Quoted 8 lines · Grant Likely

Add support for block device access to the Xilinx SystemACE Compact flash interface

---
I think this driver is in pretty good shape. I've got a few things to clean up a bit. Specifically, I'm still working on error handling and making sure that the state machine is sane at all times.

I tested the driver on my system (Katmai 440SPe). Both CF's work correctly regarding reading and writing. No problems so far.

Quoted 5 lines

I would appreciate any review/comments. One area where I am undecided is the format of the state machine. The current code uses one big function with a large switch() statment for each state. I'm considering breaking this up into a seperate function for each state, and adding a static state table with pointers to each state function.

I have no real preference here. I don't mind if the code stays in this large switch statement. It's not that much code.

Two small issues though:

drivers/block/Kconfig config ATA_OVER_ETH · line 460

Quoted 5 lines
drivers/block/Kconfig +5 -0
+config XILINX_SYSACE
+     tristate "Xilinx SystemACE support"
+     depends on XILINX_VIRTEX
+     help
+       Include support for the Xilinx SystemACE CompactFlash interface

Please remove the XILINX_VIRTEX dependency here. It makes it impossible to use the driver on my 440SPe system.

Quoted 22 lines
drivers/block/xsysace.c new file +6 -0
+/* register access macros */
+#if 1 /* Little endian 16-bit regs */
+#define ace_reg_read8(ace, reg) in_8(ace->baseaddr + reg)
+#define ace_reg_read16(ace, reg) in_le16(ace->baseaddr + reg)
+#define ace_reg_readdata(ace, reg) in_be16(ace->baseaddr + reg)
+#define ace_reg_read32(ace, reg) ((in_le16(ace->baseaddr + reg+2) << 16) |

\ + (in_le16(ace->baseaddr + reg)))

(quoted diff) +2 -0
+#define ace_reg_write16(ace, reg, val) out_le16(ace->baseaddr + reg, val)
+#define ace_reg_writedata(ace, reg, val) out_be16(ace->baseaddr + reg,

val) +#define ace_reg_write32(ace, reg, val) { \

drivers/block/xsysace.c new file +8 -0
+                           out_le16(ace->baseaddr + reg+2, (val) >> 16); \
+                           out_le16(ace->baseaddr + reg, val); \
+                       }
+#else /* Big endian 16-bit regs */
+#define ace_reg_read8(ace, reg) in_8(ace->baseaddr + reg)
+#define ace_reg_read16(ace, reg) in_be16(ace->baseaddr + reg)
+#define ace_reg_readdata(ace, reg) in_le16(ace->baseaddr + reg)
+#define ace_reg_read32(ace, reg) ((in_be16(ace->baseaddr + reg+2) << 16) |

\ + (in_be16(ace->baseaddr + reg)))

(quoted diff) +2 -0
+#define ace_reg_write16(ace, reg, val) out_be16(ace->baseaddr + reg, val)
+#define ace_reg_writedata(ace, reg, val) out_le16(ace->baseaddr + reg,

val) +#define ace_reg_write32(ace, reg, val) { \

drivers/block/xsysace.c line 190

Quoted 4 lines
drivers/block/xsysace.c new file +4 -0
+                           out_be16(ace->baseaddr + reg+2, (val) >> 16); \
+                           out_be16(ace->baseaddr + reg, val); \
+                       }
+#endif

We should make the endianess selectable (I need big endian). You talked about autodetecting the endianess once, but that would add extra code on every register access. So I suggest to just add a configuration option for the endianess selection.

Quoted 2 lines

I feel this driver is pretty close to done, and I'd like to get it into mainline for the 2.6.22 timeframe.

Yes, please.

Best regards,
Stefan

#2 lore raw
In reply to Grant Likely

Hi Grant,

Quoted 2 lines · Grant Likely

Add support for block device access to the Xilinx SystemACE Compact flash interface

Does the driver support 8-bit bus_width?

Just gave the driver a try on ML300, and my first attempt failed. Wonder if it's me having done something wrong, or something the driver doesn't handle yet.

Thanks,
Andrei

#3 lore raw
Quoted 11 lines · Andrei Konovalov

Hi Grant,

Quoted 2 lines · Grant Likely

Add support for block device access to the Xilinx SystemACE Compact flash interface

Does the driver support 8-bit bus_width?

Just gave the driver a try on ML300, and my first attempt failed. Wonder if it's me having done something wrong, or something the driver doesn't handle yet.

Doesn't handle it yet, you'll need to add 8-bit access macros to the top of the file.

Cheers,
g.

Signature · 4 lines
Grant Likely, B.Sc. P.Eng.
Secret Lab Technologies Ltd.
[email protected]
(403) 399-0195
#4 lore raw
Quoted 7 lines · Grant Likely
Quoted 6 lines · Andrei Konovalov

Hi Grant,

Quoted 2 lines · Grant Likely

Add support for block device access to the Xilinx SystemACE Compact flash interface

Quoted 8 lines
Quoted 5 lines

Does the driver support 8-bit bus_width?

Just gave the driver a try on ML300, and my first attempt failed. Wonder if it's me having done something wrong, or something the driver doesn't handle yet.

Doesn't handle it yet, you'll need to add 8-bit access macros to the top of the file.

OK. Thanks!
Will try that.

Thanks,
Andrei

Quoted 3 lines

Cheers,
g.

#5 lore raw

RFC 1/1 add 8-bit bus support to the Xilinx SystemACE device driver + minor clean-ups

In reply to Grant Likely

Hello,

The attached patch should be applied on top of the SystemACE device driver patch by Grant.

Here is the list of changes:

1) 8-bit bus support added to the driver.
SystemACE chip could be connected to the FPGA by 8-bit or 16-bit data bus.
ML403 uses the 16-bit connection, while ML300 - the 8-bit one.

2) In the original driver the data bus width and endiannes has been hard-coded with "#if 1" etc statements.
I've tried to bind this selection to the board the kernel is built
for: the bus width is defined by the board design, am I right? Please let me know if this concept is correct.

3) Added "do {" "} while(0)" brackets to the macros that need them.

I've done some testing for the Grant's driver with this patch applied. Both ML403 and ML300 booted OK with rootfs on CF card and the microdrive respectively. ML403 survived "bonnie++ -f -u root -x 5 -r 8 -s 16" test (it took 15 minutes to complete), and I wouldn't like to repeat this test because of the risk to kill the CF card. ML300 oopsed during the 3d pass of "bonnie++ -f -u root -x 5 -r 10 -s 20" (kernel access of bad area, sig: 11). Haven't dig into this yet.

Thanks,
Andrei

Quoted 18 lines · Grant Likely

Add support for block device access to the Xilinx SystemACE Compact flash interface

---
I think this driver is in pretty good shape. I've got a few things to clean up a bit. Specifically, I'm still working on error handling and making sure that the state machine is sane at all times.

I would appreciate any review/comments. One area where I am undecided is the format of the state machine. The current code uses one big function with a large switch() statment for each state. I'm considering breaking this up into a seperate function for each state, and adding a static state table with pointers to each state function.

I feel this driver is pretty close to done, and I'd like to get it into mainline for the 2.6.22 timeframe.

Quoted 7 lines

Cheers,
g.

 drivers/block/Kconfig   |    6 +
 drivers/block/Makefile  |    1 +
 drivers/block/xsysace.c | 1070 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 1077 insertions(+), 0 deletions(-)
arch/ppc/platforms/4xx/Kconfig +2 -0
Index: linux-2.6.20/arch/ppc/platforms/4xx/Kconfig
===================================================================
@@ -55,12 +55,14 @@ config WALNUT
 config XILINX_ML300
 	bool "Xilinx-ML300"
 	select XILINX_VIRTEX_II_PRO
+	select XSYSACE_8BIT
 	help
 	  This option enables support for the Xilinx ML300 evaluation board.
 
 config XILINX_ML403
 	bool "Xilinx-ML403"
 	select XILINX_VIRTEX_4_FX
+	select XSYSACE_16BIT_LE
 	help
 	  This option enables support for the Xilinx ML403 evaluation board.
 endchoice
drivers/block/Kconfig +10 -0
Index: linux-2.6.20/drivers/block/Kconfig
===================================================================
@@ -459,6 +459,16 @@ config XILINX_SYSACE
 	help
 	  Include support for the Xilinx SystemACE CompactFlash interface
 
+config XSYSACE_8BIT
+	bool
+	depends on XILINX_SYSACE
+	default n
+
+config XSYSACE_16BIT_LE
+	bool
+	depends on XILINX_SYSACE
+	default n
+
 endmenu
 
 endif
drivers/block/xsysace.c +39 -6

67 lines not on this page · raw message

#6 lore raw

Re: [RFC] add 8-bit bus support to the Xilinx SystemACE device driver + minor clean-ups

Quoted 4 lines · Andrei Konovalov

Hello,

The attached patch should be applied on top of the SystemACE device driver patch by Grant.

I just realized I never replyed to you on this one. Sorry!

Thanks for the patch. I'm getting the driver ready for mainline submission now and I'll make sure 8 bit support is included. I'll also need your help with some more testing as I'm doing a fair bit of rework on the bus attachment and error handling.

Thanks again,
g.

Signature · 4 lines
Grant Likely, B.Sc. P.Eng.
Secret Lab Technologies Ltd.
[email protected]
(403) 399-0195