SystemACE driver - abstract register ops
RFC 1/1 SystemACE driver - abstract register ops
Grant,
Thanks for your work on the SystemACE driver - I'll be porting/merging this across to MicroBlaze very shortly.
Given that SysACE can be hooked up in any number of ways, bit widths, endians, PPC/Microblaze, can you please offer your comments on the attached patch?
It introduce a private ace_reg_ops structure, with various member functions for the different kinds of accesses to the HW.
This patch should not change the functionality of your original driver at all, it's just groundwork for what's to come.
I recognise that it adds indirection into the various access paths, and potentially a little bloat. Whether this is better than #if 1...#else...#endif is debatable.
Similar issues will arise for most (all?) of the Xilinx drivers that we will share between PPC and MicroBlaze. Hopefully we can converge on a nice consistent and clean way of handling these dual arch drivers.
Cheers,
John
drivers/block/xsysace.c +121 -23
@@ -73,6 +73,11 @@* interrupt, then the kernel timer will expire and the driver can * continue where it left off. * +* SystemACE can be wired up in different endian orders and data widths. +* It works on both PPC and MicroBlaze architectures. For this reason, +* an ace_reg_ops structure is used that abstracts away low level +* endian/width/arch access to the HW registers. +* To Do: * - Add FPGA configuration control interface. * - Request major number from lanana
@@ -161,32 +166,120 @@/* --------------------------------------------------------------------- * Low level register access */ +struct reg_ops { +u8 (*read8)(void *addr); +u16 (*read16)(void *addr); +u32 (*read32)(void *addr); +u32 (*readdata)(void *addr); -/* register access macros */ +void (*write16)(void *addr,u16 val); +void (*write32)(void *addr,u32 val); +void (*writedata)(void *addr,u32 val); +}; + +#define ace_reg_read8 (ace, reg) (ace->ops->read8(ace->baseaddr + reg)) +#define ace_reg_read16 (ace, reg) (ace->ops->read16(ace->baseaddr + reg)) +#define ace_reg_readdata (ace, reg) (ace->ops->readdata(ace->baseaddr + reg)) +#define ace_reg_read32 (ace, reg) (ace->ops->read32(ace->baseaddr+reg)) +#define ace_reg_write16 (ace, reg, val) (ace->ops->write16(ace->baseaddr+reg, val)) +#define ace_reg_writedata (ace, reg, val) (ace->ops->writedata(ace->baseaddr + reg, val)) +#define ace_reg_write32 (ace, reg, val) (ace->ops->write32(ace->baseaddr+reg, val)) + +/* register access functions */ #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))) -#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) { \ - out_le16(ace->baseaddr + reg+2, (val) >> 16); \ - out_le16(ace->baseaddr + reg, val); \ - } +static u8 ace_le16_read8 (void *addr) +{ +return in_8 (addr); +} + +static u16 ace_le16_read16 (void *addr) +{ +return in_le16 (addr); +} + +static u32 ace_le16_read32 (void *addr) +{ +return ((in_le16 (addr+2 ) <<16 ) | (in_le16 (addr))); +} + +static u32 ace_le16_readdata (void *addr) +{ +return in_be16 (addr); +} + +static void ace_le16_write16 (void *addr,u16 val) +{ +out_le16 (addr, val); +} + +static void ace_le16_write32 (void *addr,u32 val) +{ +out_le16 (addr+2 ,(val) >>16 ); \ +out_le16 (addr, val); +} + +static void ace_le16_writedata (void *addr,u32 val) +{ +out_be16 (addr, val); +} + +static struct reg_ops ace_ops = { + .read8 = ace_le16_read8, + .read16 = ace_le16_read16, + .read32 = ace_le16_read32, + .readdata = ace_le16_readdata, + .write16 = ace_le16_write16, + .write32 = ace_le16_write32, + .writedata = ace_le16_writedata +} ; +#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))) -#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) { \ - out_be16(ace->baseaddr + reg+2, (val) >> 16); \ - out_be16(ace->baseaddr + reg, val); \ - } +static u8 ace_be16_read8 (void *addr) +{ +return in_8 (addr); +} + +static u16 ace_be16_read16 (void *addr) +{ +return in_be16 (addr); +} + +static u32 ace_be16_read32 (void *addr) +{ +return ((in_be16 (addr+2 ) <<16 ) | (in_be16 (addr))); +} + +static u32 ace_be16_readdata (void *addr) +{ +return in_le16 (addr); +} + +static void ace_be16_write16 (void *addr,u16 val) +{ +out_be16 (addr, val); +} + +static void ace_be16_write32 (void *addr,u32 val) +{ +out_be16 (addr+2 ,(val) >>16 ); \ +out_be16 (addr, val); +} + +static void ace_be16_writedata (void *addr,u32 val) +{ +out_le16 (addr, val); +} + +static struct reg_ops ace_ops = { + .read8 = ace_be16_read8, + .read16 = ace_be16_read16, + .read32 = ace_be16_read32, + .readdata = ace_be16_readdata, + .write16 = ace_be16_write16, + .write32 = ace_be16_write32, + .writedata = ace_be16_writedata +} ; +#endif struct ace_device {
@@ -222,6 +315,9 @@int bus_width;/* 0 := 8 bit; 1 := 16 bit */ int lock_count; +/* Register access ops */ +struct reg_ops *ops; +/* Block device data structures */ spinlock_t lock;struct device *dev;
@@ -995,6 +1091,8 @@ ace->id = dev->id; ace->irq =NO_IRQ ; + ace->ops=&ace_ops; +for (i =0 ; i < dev->num_resources; i++) {if (dev->resource[i].flags &IORESOURCE_MEM ) ace->physaddr = dev->resource[i].start;
Quoted 8 lines · John Williams
Grant,
Thanks for your work on the SystemACE driver - I'll be porting/merging this across to MicroBlaze very shortly.
Given that SysACE can be hooked up in any number of ways, bit widths, endians, PPC/Microblaze, can you please offer your comments on the attached patch?
HAHAHAHAHAHAHA! Guess what I just finished writing before receiving your email.
I've got something very similar that I'm testing now, but thanks for the patch. I feel better that someone else has the same opinion on how to hook up the bus.
Cheers,
g.
Quoted 200 lines
It introduce a private
ace_reg_opsstructure, with various member functions for the different kinds of accesses to the HW.This patch should not change the functionality of your original driver at all, it's just groundwork for what's to come.
I recognise that it adds indirection into the various access paths, and potentially a little bloat. Whether this is better than #if 1...#else...#endif is debatable.
Similar issues will arise for most (all?) of the Xilinx drivers that we will share between PPC and MicroBlaze. Hopefully we can converge on a nice consistent and clean way of handling these dual arch drivers.
Cheers,
John
Index:
linux-2.6.x-petalogix/drivers/block/xsysace.c
===================================================================drivers/block/xsysace.c (working copy) renamed from drivers/block/xsysace.c (revision 2628) +121
-23@@ -73,6 +73,11 @@ * interrupt, then the kernel timer will expire and the driver can * continue where it left off. * + * SystemACE can be wired up in different endian orders and data widths. + * It works on both PPC and MicroBlaze architectures. For this reason, + * an ace_reg_ops structure is used that abstracts away low level + * endian/width/arch access to the HW registers. + * To Do: * - Add FPGA configuration control interface. * - Request major number from lanana@@ -161,32 +166,120 @@ /* --------------------------------------------------------------------- * Low level register access */ +struct reg_ops { + u8 (*read8)(void *addr); + u16 (*read16)(void *addr); + u32 (*read32)(void *addr); + u32 (*readdata)(void *addr); -/* register access macros */ + void (*write16)(void *addr, u16 val); + void (*write32)(void *addr, u32 val); + void (*writedata)(void *addr, u32 val); +}; + +#define ace_reg_read8(ace, reg) (ace->ops->read8(ace->baseaddr + reg)) +#define ace_reg_read16(ace, reg) (ace->ops->read16(ace->baseaddr + reg)) +#define ace_reg_readdata(ace, reg) (ace->ops->readdata(ace->baseaddr + reg)) +#define ace_reg_read32(ace, reg) (ace->ops->read32(ace->baseaddr+reg)) +#define ace_reg_write16(ace, reg, val) (ace->ops->write16(ace->baseaddr+reg, val)) +#define ace_reg_writedata(ace, reg, val) (ace->ops->writedata(ace->baseaddr + reg, val)) +#define ace_reg_write32(ace, reg, val) (ace->ops->write32(ace->baseaddr+reg, val)) + +/* register access functions */ #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))) -#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) { \ - out_le16(ace->baseaddr + reg+2, (val) >> 16); \ - out_le16(ace->baseaddr + reg, val); \ - } +static u8 ace_le16_read8(void *addr) +{ + return in_8(addr); +} + +static u16 ace_le16_read16(void *addr) +{ + return in_le16(addr); +} + +static u32 ace_le16_read32(void *addr) +{ + return ((in_le16(addr+2) << 16) | (in_le16(addr))); +} + +static u32 ace_le16_readdata(void *addr) +{ + return in_be16(addr); +} + +static void ace_le16_write16(void *addr, u16 val) +{ + out_le16(addr, val); +} + +static void ace_le16_write32(void *addr, u32 val) +{ + out_le16(addr+2,(val) >> 16); \ + out_le16(addr, val); +} + +static void ace_le16_writedata(void *addr, u32 val) +{ + out_be16(addr, val); +} + +static struct reg_ops ace_ops = { + .read8 = ace_le16_read8, + .read16 = ace_le16_read16, + .read32 = ace_le16_read32, + .readdata = ace_le16_readdata, + .write16 = ace_le16_write16, + .write32 = ace_le16_write32, + .writedata = ace_le16_writedata +} ; + #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))) -#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) { \ - out_be16(ace->baseaddr + reg+2, (val) >> 16); \ - out_be16(ace->baseaddr + reg, val); \ - } +static u8 ace_be16_read8(void *addr) +{ + return in_8(addr); +} + +static u16 ace_be16_read16(void *addr) +{ + return in_be16(addr); +} + +static u32 ace_be16_read32(void *addr) +{ + return ((in_be16(addr+2) << 16) | (in_be16(addr))); +} + +static u32 ace_be16_readdata(void *addr) +{ + return in_le16(addr); +} + +static void ace_be16_write16(void *addr, u16 val) +{ + out_be16(addr, val); +} + +static void ace_be16_write32(void *addr, u32 val) +{ + out_be16(addr+2,(val) >> 16); \ + out_be16(addr, val); +} + +static void ace_be16_writedata(void *addr, u32 val) +{ + out_le16(addr, val); +} + +static struct reg_ops ace_ops = { + .read8 = ace_be16_read8, + .read16 = ace_be16_read16, + .read32 = ace_be16_read32, + .readdata = ace_be16_readdata, + .write16 = ace_be16_write16, + .write32 = ace_be16_write32, + .writedata = ace_be16_writedata +} ; + #endif struct ace_device {@@ -222,6 +315,9 @@ int bus_width; /* 0 := 8 bit; 1 := 16 bit */ int lock_count; + /* Register access ops */ + struct reg_ops *ops; + /* Block device data structures */ spinlock_t lock; struct device *dev;@@ -995,6 +1091,8 @@ ace->id = dev->id; ace->irq = NO_IRQ; + ace->ops=&ace_ops; + for (i = 0; i < dev->num_resources; i++) { if (dev->resource[i].flags & IORESOURCE_MEM) ace->physaddr = dev->resource[i].start;
Signature · 4 lines
Grant Likely, B.Sc. P.Eng. Secret Lab Technologies Ltd. [email protected] (403) 399-0195
Quoted 4 lines · John Williams
Grant,
Thanks for your work on the SystemACE driver - I'll be porting/merging this across to MicroBlaze very shortly.
Very cool; I hope it works well.
Quoted 3 lines
Given that SysACE can be hooked up in any number of ways, bit widths, endians, PPC/Microblaze, can you please offer your comments on the attached patch?
Okay, after getting over my initial mirth about working on the *exact* same thing and finishing it at the *exact* time that you sent me your patch, I think I'm ready to make useful comments. :-)
Quoted 10 lines
It introduce a private
ace_reg_opsstructure, with various member functions for the different kinds of accesses to the HW.This patch should not change the functionality of your original driver at all, it's just groundwork for what's to come.
I recognise that it adds indirection into the various access paths, and potentially a little bloat. Whether this is better than #if 1...#else...#endif is debatable.
I'm not to concerned with the added redirection. On my 405 designs, I find that bus overhead has a far greater impact than any of the processing paths in the driver, so this shouldn't be a problem. Besides, when we finally move to arch/powerpc, it will become very feasable to have a single kernel image that will boot on multiple ppc405 FPGA configurations; just change the device tree passed in.
Quoted 3 lines
Similar issues will arise for most (all?) of the Xilinx drivers that we will share between PPC and MicroBlaze. Hopefully we can converge on a nice consistent and clean way of handling these dual arch drivers.
I agree 100%
For your reading pleasure, I've attached the bus attachment changes that I've made in my tree. I hope to get this driver accepted into mainline during the 2.6.22 merge window; so please get any comments you have back to me ASAP.
Cheers,
g.
From 10211a2cb23f9c7cc447f800a1f3828630cea819 Mon Sep 17 00:00:00 2001
From: Grant Likely <[email protected]>
Date: Fri, 27 Apr 2007 00:19:09 -0600
Subject: [PATCH] [SYSACE] Make bus binding selectable at runtime
The SystemACE can be wired up to many different bus arangements. This patch defines 8, be16 and le16 bus attachments and adds the code to select them at runtime.
--- drivers/block/xsysace.c | 256 ++++++++++++++++++++++++++++++++++------------ 1 files changed, 189 insertions(+), 67 deletions(-)
drivers/block/xsysace.c +189 -67
@@ -80,7 +80,6 @@*/ #undef DEBUG -#undef DEBUG_ENDIAN/* Uncomment to debug register endinaness */ #include <linux/module.h> #include <linux/ctype.h>
@@ -156,38 +155,11 @@ MODULE_LICENSE("GPL");#define ACE_FATSTAT (0x1c)#define ACE_NUM_MINORS 16 -#define ACE_BUF_PER_SECTOR (512 / 32)/* 512_byte_sector / 32_byte_fifo */ +#define ACE_SECTOR_SIZE (512) +#define ACE_FIFO_SIZE (32) +#define ACE_BUF_PER_SECTOR (ACE_SECTOR_SIZE / ACE_FIFO_SIZE) -/* --------------------------------------------------------------------- -* Low level register access -*/ - -/* 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))) -#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) { \ - 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))) -#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) { \ - out_be16(ace->baseaddr + reg+2, (val) >> 16); \ - out_be16(ace->baseaddr + reg, val); \ - } -#endif +struct ace_reg_ops ;struct ace_device {/* driver state data */
@@ -220,6 +192,7 @@ struct ace_device {void * baseaddr;int irq;int bus_width;/* 0 := 8 bit; 1 := 16 bit */ +struct ace_reg_ops *reg_ops;int lock_count;/* Block device data structures */
@@ -236,6 +209,168 @@ static LIST_HEAD(ace_instances);static int ace_major =0 ;/* --------------------------------------------------------------------- +* Low level register access +*/ + +struct ace_reg_ops { +uint16_t (*in)(struct ace_device *ace,ulong reg); +void (*out)(struct ace_device *ace,ulong reg,uint16_t val); +void (*identin)(struct ace_device *ace); +void (*datain)(struct ace_device *ace); +void (*dataout)(struct ace_device *ace); +}; + +/* 8 Bit bus width */ +static uint16_t ace_in_8 (struct ace_device *ace,ulong reg) +{ +void * r = ace->baseaddr + reg; +return in_8 (r) | (in_8 (r+1 ) <<8 ); +} + +static void ace_out_8 (struct ace_device *ace,ulong reg,uint16_t val) +{ +void * r = ace->baseaddr + reg; +out_8 (r, val); +out_8 (r+1 , val >>8 ); +} + +static void ace_identin_8 (struct ace_device *ace) +{ +void * r = ace->baseaddr +0x40 ; +int i =ACE_FIFO_SIZE /2 ; + while (i--) +#if defined(__BIG_ENDIAN) + *ace->data_ptr = (in_8 (r)) | (in_8 (r+1 )<<8 ); +#else + *ace->data_ptr = (in_8 (r)<<8 ) | (in_8 (r+1 )); +#endif + ace->data_count--; +} + +static void ace_datain_8 (struct ace_device *ace) +{ +void * r = ace->baseaddr +0x40 ; +int i =ACE_FIFO_SIZE /2 ; + while (i--) +#if defined(__BIG_ENDIAN) + *ace->data_ptr = (in_8 (r)<<8 ) | (in_8 (r+1 )); +#else + *ace->data_ptr = (in_8 (r)) | (in_8 (r+1 )<<8 ); +#endif +} + +static void ace_dataout_8 (struct ace_device *ace) +{ +void * r = ace->baseaddr +0x40 ; +int i =ACE_FIFO_SIZE /2 ; +while (i--) { +#if defined(__BIG_ENDIAN) +out_8 (r, *ace->data_ptr >>8 ); +out_8 (r+1 , *ace->data_ptr); +#else +out_8 (r, *ace->data_ptr); +out_8 (r+1 , *ace->data_ptr >>8 ); +#endif + ace->data_ptr++; + } +} + +static struct ace_reg_ops ace_reg_8_ops = { + .in = ace_in_8, + .out = ace_out_8, + .identin = ace_identin_8, + .datain = ace_datain_8, + .dataout = ace_dataout_8, +}; + +/* 16 bit big endian bus attachment */ +static uint16_t ace_in_be16 (struct ace_device *ace,ulong reg) +{ +return in_be16 (ace->baseaddr+reg); +} + +static void ace_out_be16 (struct ace_device *ace,ulong reg,uint16_t val) +{ +out_be16 (ace->baseaddr+reg, val); +} + +static void ace_fifoin_be16 (struct ace_device *ace) +{ +int i =ACE_FIFO_SIZE /2 ; +while (i--) + *ace->data_ptr++ =in_be16 (ace->baseaddr +0x40 ); +} + +static void ace_fifoout_be16 (struct ace_device *ace) +{ +int i =ACE_FIFO_SIZE /2 ; +while (i--) +out_be16 (ace->baseaddr +0x40 , *ace->data_ptr++); +} + +/* 16 bit little endian bus attachment */ +static uint16_t ace_in_le16 (struct ace_device *ace,ulong reg) +{ +return in_le16 (ace->baseaddr+reg); +} + +static void ace_out_le16 (struct ace_device *ace,ulong reg,uint16_t val) +{ +out_le16 (ace->baseaddr+reg, val); +} + +static void ace_fifoin_le16 (struct ace_device *ace) +{ +int i =ACE_FIFO_SIZE /2 ; +while (i--) + *ace->data_ptr++ =in_le16 (ace->baseaddr +0x40 ); +} + +static void ace_fifoout_le16 (struct ace_device *ace) +{ +int i =ACE_FIFO_SIZE /2 ; +while (i--) +out_le16 (ace->baseaddr +0x40 , *ace->data_ptr++); +} + +static struct ace_reg_ops ace_reg_be16_ops = { + .in = ace_in_be16, + .out = ace_out_be16, + .identin = ace_fifoin_be16, + .datain = ace_fifoin_le16,/* little endian! */ + .dataout = ace_fifoout_le16,/* little endian! */ +}; + +static struct ace_reg_ops ace_reg_le16_ops = { + .in = ace_in_le16, + .out = ace_out_le16, + .identin = ace_fifoin_le16, + .datain = ace_fifoin_be16,/* big endian! */ + .dataout = ace_fifoout_be16,/* big endian! */ +}; + +#define ace_in (ace, reg) ace->reg_ops->in(ace, reg) +static inline uint32_t ace_in32 (struct ace_device *ace,ulong reg) +{ +return ace_in (ace, reg) | (ace_in (ace, reg+2 ) <<16 ); +} +#define ace_out (ace, reg, val) ace->reg_ops->out(ace, reg, val) +static inline void ace_out32 (struct ace_device *ace,ulong reg,uint32_t val) +{ +ace_out (ace, reg, val); +ace_out (ace, reg+2 , val >>16 ); +} +#define ace_identin (ace) ace->reg_ops->identin(ace) +#define ace_datain (ace) ace->reg_ops->datain(ace) +#define ace_dataout (ace) ace->reg_ops->dataout(ace) + +/* legacy macros, to be removed */ +#define ace_reg_read16 (ace, reg) ace_in(ace, reg) +#define ace_reg_read32 (ace, reg) ace_in32(ace, reg) +#define ace_reg_write16 (ace, reg, val) ace_out(ace, reg, val) +#define ace_reg_write32 (ace, reg, val) ace_out32(ace, reg, val) + +/* --------------------------------------------------------------------- * Debug support functions */
@@ -270,31 +405,6 @@ static inline void ace_dump_mem(void* base, int len) {}static void ace_dump_regs (struct ace_device *ace) { - -#if defined(DEBUG_ENDIAN ) -/* some test routines to see if 8 16 and 32 bit access is working */ -ace_info (ace,"register dump:" );/* No '\n' needed! */ - { -int i; -for (i =0 ; i <0x20 ; i++) { -if (!(i %16 )) -printk ("\n" KERN_INFO " %.2x:" , i); -printk (" %.2x" ,ace_reg_read8 (ace, i)); - } -for (i =0 ; i <0x20 ; i+=2 ) { -if (!(i %16 )) -printk ("\n" KERN_INFO " %.2x:" , i); -printk (" %.4x " ,ace_reg_read16 (ace, i)); - } -for (i =0 ; i <0x20 ; i+=4 ) { -if (!(i %16 )) -printk ("\n" KERN_INFO " %.2x:" , i); -printk (" %.8x " ,ace_reg_read32 (ace, i)); - } - } -printk ("\n" ); -#endif - ace_info(ace, " ctrl: %.8x seccnt/cmd: %.4x ver:%.4x\n" " status:%.8x mpu_lba:%.8x busmode:%4x\n" " error: %.8x cfg_lba:%.8x fatstat:%.4x\n",
@@ -348,7 +458,8 @@ void ace_fix_driveid (struct hd_driveid *id)#define ACE_FSM_STATE_REQ_PREPARE 7#define ACE_FSM_STATE_REQ_TRANSFER 8#define ACE_FSM_STATE_REQ_COMPLETE 9 -#define ACE_FSM_NUM_STATES 10 +#define ACE_FSM_STATE_ERROR 10 +#define ACE_FSM_NUM_STATES 11#if defined(DEBUG ) const char* ace_statenames[ACE_FSM_NUM_STATES] = {
@@ -497,9 +608,7 @@ static void ace_fsm_dostate(struct ace_device *ace) }/* Transfer the next buffer */ - i =16 ; -while (i--) - *ace->data_ptr++ =ace_reg_read16 (ace,0x40 ); +ace_identin (ace); ace->data_count--;/* If there are still buffers to be transfers; jump out here */
@@ -608,11 +717,9 @@ static void ace_fsm_dostate(struct ace_device *ace)/* Transfer the next buffer */ i =16 ;if (ace->fsm_task ==ACE_TASK_WRITE ) -while (i--) -ace_reg_writedata (ace,0x40 , *ace->data_ptr++); +ace_dataout (ace);else -while (i--) - *ace->data_ptr++ =ace_reg_readdata (ace,0x40 ); +ace_datain (ace); ace->data_count--;/* If there are still buffers to be transfers; jump out here */
@@ -906,8 +1013,20 @@ static int ace_setup(struct ace_device *ace)snprintf (ace->gd->disk_name,32 ,"xs%c" , ace->id +'a' );device_rename (ace->dev, ace->gd->disk_name); -/* set 16-bit bus width */ -ace_reg_write16 (ace,ACE_BUSMODE ,0x0001 ); +/* set bus width */ +if (ace->bus_width ==1 ) { +/* 0x0101 should work regardless of endianess */ +ace_out_le16 (ace,ACE_BUSMODE ,0x0101 ); + +/* read it back to determine endianess */ +if (ace_in_le16 (ace,ACE_BUSMODE ) ==0x0001 ) + ace->reg_ops = &ace_reg_le16_ops; +else + ace->reg_ops = &ace_reg_be16_ops; + }else { +ace_out_8 (ace,ACE_BUSMODE ,0x00 ); + ace->reg_ops = &ace_reg_8_ops; + }/* Make sure version register is sane */ version =ace_reg_read16 (ace,ACE_VERSION );
@@ -1001,6 +1120,9 @@ static int ace_probe(struct device *device)if (dev->resource[i].flags &IORESOURCE_IRQ ) ace->irq = dev->resource[i].start; } + +/* FIXME: Should get bus_width from the platform_device struct */ + ace->bus_width =1 ;dev_set_drvdata (&dev->dev, ace);
Signature · 1 line
1.5.1
Quoted 4 lines · Grant Likely
For your reading pleasure, I've attached the bus attachment changes that I've made in my tree. I hope to get this driver accepted into mainline during the 2.6.22 merge window; so please get any comments you have back to me ASAP.
Works fine on my Katmai 440SPe system with SysACE connected in 16bit big-endian mode. Thanks.
Acked-by Stefan Roese <[email protected]>
Best regards,
Stefan
Hi All,
I wanted to say that I am VERY excited about all the great work that is happening here. As some of you know I am currently working on setting up a Linux kernel git tree that contains a multitude of Xilinx drivers such as IIC, SPI, GPIO, SYSACE, EMAC, EMACLITE, UARTLITE versions, FLASH as well support for the MicroBlaze processor. In addition I pulled Grant's virtex-dev branch (with TEMAC and SYSACE). I will integrate those as soon as I'm done with the other work. I am already eying the FrameBuffer driver from Andrei.
The end goal is to have a kernel tree that contains ALL Xilinx drivers and the multitude of drivers that are being created/updated and contributed here. Having such a kernel available will simplify the process of bringing up a new system. The hope is that contributors will adopt the idea of having a "Xilinx" kernel and will start using that tree as a "base reference".
At this point the Xilinx kernel branch is based on version v2.6.20 with the uc0 patch applied, as well as the various LynuxWorks patches that support MicroBlaze and add the mentioned drivers. The LynuxWorks code has been cleaned up to not contain the proprietary hardcoded sections to make it more acceptable to the Open Source community. I'm sure there's more work to be done, but it's a start.
Once the git server is up and running we will be adding new drivers they become available and integrate patches and updates along the way.
Unfortunately, I spend a lot of my time now working the politics of getting a git server up and running and convincing management that this actually _is_ a good idea. It's slow going and there's a lot of meetings and discussions going on. But I'm still hopeful. As a backup I'm trying to get a kernel.org/git account so I can publish the tree there.
I will post news here as they unfold. In the meantime it would be very interesting to hear ideas and suggestions from you. Concerns that people have. Pitfalls to look out for etc.
Thanks,
Wolfgang
Quoted 54 lines · Grant Likely
Quoted 4 lines · John Williams
Grant,
Thanks for your work on the SystemACE driver - I'll be porting/merging this across to MicroBlaze very shortly.
Very cool; I hope it works well.
Quoted 3 lines
Given that SysACE can be hooked up in any number of ways, bit widths, endians, PPC/Microblaze, can you please offer your comments on the attached patch?
Okay, after getting over my initial mirth about working on the *exact* same thing and finishing it at the *exact* time that you sent me your patch, I think I'm ready to make useful comments. :-)
Quoted 10 lines
It introduce a private
ace_reg_opsstructure, with various member functions for the different kinds of accesses to the HW.This patch should not change the functionality of your original driver at all, it's just groundwork for what's to come.
I recognise that it adds indirection into the various access paths, and potentially a little bloat. Whether this is better than #if 1...#else...#endif is debatable.
I'm not to concerned with the added redirection. On my 405 designs, I find that bus overhead has a far greater impact than any of the processing paths in the driver, so this shouldn't be a problem. Besides, when we finally move to
arch/powerpc, it will become very feasable to have a single kernel image that will boot on multiple ppc405 FPGA configurations; just change the device tree passed in.Quoted 3 lines
Similar issues will arise for most (all?) of the Xilinx drivers that we will share between PPC and MicroBlaze. Hopefully we can converge on a nice consistent and clean way of handling these dual arch drivers.
I agree 100%
For your reading pleasure, I've attached the bus attachment changes that I've made in my tree. I hope to get this driver accepted into mainline during the 2.6.22 merge window; so please get any comments you have back to me ASAP.
Cheers,
g.------------------------------------------------------------------------
_______________________________________________
Linuxppc-embedded mailing list
[email protected]
https://ozlabs.org/mailman/listinfo/linuxppc-embedded
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"WR" == Wolfgang Reissnegger <[email protected]> writes:
Hi,
WR> The end goal is to have a kernel tree that contains ALL Xilinx WR> drivers and the multitude of drivers that are being
WR> created/updated and contributed here. Having such a kernel WR> available will simplify the process of bringing up a new WR> system. The hope is that contributors will adopt the idea of WR> having a "Xilinx" kernel and will start using that tree as a "base WR> reference".
But there already is such a tree - It's called 'mainline'. I can see some advantage in having a Xilinx tree as a staging area, but it really HAS to be a temporary thing and stuff needs to be pushed to mainline.
WR> I will post news here as they unfold. In the meantime it would be WR> very interesting to hear ideas and suggestions from you. Concerns WR> that people have. Pitfalls to look out for etc.
Hereby my 2 cents.
Signature · 1 line
Bye, Peter Korsgaard
Hi Peter,
thanks for the feedback.
Yes, I agree. Having the code in mainline would be most preferable. However, there is always a delay between when people submit patches and when code actually does get into mainline. Also, it's not always guaranteed that the code is actually accepted into mainline. In the meantime there are a multitude of patches floating around which IMHO can be quite confusing.
The purpose of the "Xilinx git tree" is to provide a central place where those patches can be accumulated, including PPC and MicroBlaze. Also, as Xilinx is creating new IP cores, drivers for those cores will be directly added to that tree, eliminating the EDK->Linux horrors.
So, yes, I think it's partly a staging area for code that should make it into mainline, but it's also a repository for code that is not accepted into mainline (yet), or new, or otherwise useful to the community.
Wolfgang
Quoted 23 lines · Peter Korsgaard
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"WR" == Wolfgang Reissnegger <[email protected]> writes:
Hi,
WR> The end goal is to have a kernel tree that contains ALL Xilinx WR> drivers and the multitude of drivers that are being
WR> created/updated and contributed here. Having such a kernel WR> available will simplify the process of bringing up a new WR> system. The hope is that contributors will adopt the idea of WR> having a "Xilinx" kernel and will start using that tree as a "base WR> reference".But there already is such a tree - It's called 'mainline'. I can see some advantage in having a Xilinx tree as a staging area, but it really HAS to be a temporary thing and stuff needs to be pushed to mainline.
WR> I will post news here as they unfold. In the meantime it would be WR> very interesting to hear ideas and suggestions from you. Concerns WR> that people have. Pitfalls to look out for etc.
Hereby my 2 cents.
Hi Grant,
Quoted 7 lines · Grant Likely
For your reading pleasure, I've attached the bus attachment changes that I've made in my tree. I hope to get this driver accepted into mainline during the 2.6.22 merge window; so please get any comments you have back to me ASAP.
Cheers,
g.
ML300 + IBM microdrive.
8-bit, __BIG_ENDIAN
(ace->bus_width = 0;)
Have noticed a misprint (see below).
Correcting it seems to make the device geometry to be recognized OK, but then:
[ 3.798449] xsysace xsa: Xilinx SystemACE revision 1.0.12 [ 3.891228] xsysace xsa: capacity: 2104704 sectors [ 3.973193] xsa:<4>xsysace xsa: kicking stalled fsm; state=3 task=1 iter=2 dc=0 [ 6.103446] xsysace xsa: kicking stalled fsm; state=3 task=1 iter=2 dc=0 [ 7.207342] xsysace xsa: kicking stalled fsm; state=3 task=1 iter=2 dc=0 [ 8.311230] xsysace xsa: kicking stalled fsm; state=3 task=1 iter=2 dc=0 [ 9.411113] xsysace xsa: kicking stalled fsm; state=3 task=1 iter=2 dc=0 [ 10.511016] xsysace xsa: kicking stalled fsm; state=3 task=1 iter=2 dc=0
Will have a deeper look tomorrow.
And try CF card too.
At the moment not sure if this the driver issue.
Thanks,
Andrei
drivers/block/xsysace.c +4 -4
@@ -240,9 +240,9 @@ static void ace_identin_8(struct ace_devint i =ACE_FIFO_SIZE /2 ; while (i--)#if defined(__BIG_ENDIAN) - *ace->data_ptr = (in_8 (r)) | (in_8 (r+1 )<<8 ); + *ace->data_ptr++ = (in_8 (r)) | (in_8 (r+1 )<<8 );#else - *ace->data_ptr = (in_8 (r)<<8 ) | (in_8 (r+1 )); + *ace->data_ptr++ = (in_8 (r)<<8 ) | (in_8 (r+1 ));#endif ace->data_count--; }
@@ -253,9 +253,9 @@ static void ace_datain_8(struct ace_deviint i =ACE_FIFO_SIZE /2 ; while (i--)#if defined(__BIG_ENDIAN) - *ace->data_ptr = (in_8 (r)<<8 ) | (in_8 (r+1 )); + *ace->data_ptr++ = (in_8 (r)<<8 ) | (in_8 (r+1 ));#else - *ace->data_ptr = (in_8 (r)) | (in_8 (r+1 )<<8 ); + *ace->data_ptr++ = (in_8 (r)) | (in_8 (r+1 )<<8 );#endif }
Quoted 5 lines · Andrei Konovalov
[ 10.511016] xsysace xsa: kicking stalled fsm; state=3 task=1 iter=2 dc=0Will have a deeper look tomorrow.
And try CF card too.
At the moment not sure if this the driver issue.
Oops, I messed it up; try the attached change (I had left in a bogus data_count decrement). Thanks for catching the ptr increment bug.
g.
Signature · 4 lines
Grant Likely, B.Sc. P.Eng. Secret Lab Technologies Ltd. [email protected] (403) 399-0195
From eefc86e16d1794e67384bb8571129f282c9e3242 Mon Sep 17 00:00:00 2001
From: Grant Likely <[email protected]>
Date: Fri, 27 Apr 2007 12:41:34 -0600
Subject: [PATCH] [SYSACE] Bug fixes to 8 bit binding
--- drivers/block/xsysace.c | 9 ++++----- 1 files changed, 4 insertions(+), 5 deletions(-)
drivers/block/xsysace.c +4 -5
@@ -240,11 +240,10 @@ static void ace_identin_8(struct ace_device *ace)int i =ACE_FIFO_SIZE /2 ; while (i--)#if defined(__BIG_ENDIAN) - *ace->data_ptr = (in_8 (r)) | (in_8 (r+1 )<<8 ); + *ace->data_ptr++ = (in_8 (r)) | (in_8 (r+1 )<<8 );#else - *ace->data_ptr = (in_8 (r)<<8 ) | (in_8 (r+1 )); + *ace->data_ptr++ = (in_8 (r)<<8 ) | (in_8 (r+1 ));#endif - ace->data_count--; }static void ace_datain_8 (struct ace_device *ace)
@@ -253,9 +252,9 @@ static void ace_datain_8(struct ace_device *ace)int i =ACE_FIFO_SIZE /2 ; while (i--)#if defined(__BIG_ENDIAN) - *ace->data_ptr = (in_8 (r)<<8 ) | (in_8 (r+1 )); + *ace->data_ptr++ = (in_8 (r)<<8 ) | (in_8 (r+1 ));#else - *ace->data_ptr = (in_8 (r)) | (in_8 (r+1 )<<8 ); + *ace->data_ptr++ = (in_8 (r)) | (in_8 (r+1 )<<8 );#endif }
Signature · 1 line
1.5.1
John Williams
Acked-byGrant,
Quoted 6 lines · Grant Likely
Quoted 2 lines · John Williams
Thanks for your work on the SystemACE driver - I'll be porting/merging this across to MicroBlaze very shortly.
Very cool; I hope it works well.
Indeed it does - your latest patchset version of the systemACE driver "just works" on MicroBlaze 2.6.20
I have only tested the 16-bit buswidth (standard ML401 reference design).
Quoted 4 lines
For your reading pleasure, I've attached the bus attachment changes that I've made in my tree. I hope to get this driver accepted into mainline during the 2.6.22 merge window; so please get any comments you have back to me ASAP.
John
Quoted 9 lines · John Williams
Grant,
Quoted 1 line · Grant Likely
Very cool; I hope it works well.
Indeed it does - your latest patchset version of the systemACE driver "just works" on MicroBlaze 2.6.20
Acked-by John Williams <[email protected]>
Wonderful, thanks John. I've got a few last little details that I'm going to try and get sorted out in the next 24 hours or so, and then I'll post the driver to the lkml. If things go really well, it might even get picked up for 2.6.22. I'll make sure to CC you when I send it.
Cheers,
g.
Signature · 4 lines
Grant Likely, B.Sc. P.Eng. Secret Lab Technologies Ltd. [email protected] (403) 399-0195