[PATCH] spi: xilinx_spi: Fix up I/O routine wrapping bogosity.

[PATCH] spi: xilinx_spi: Fix up I/O routine wrapping bogosity.

[Paul Mundt]

xilinx_spi presently makes some fairly questionable assumptions about I/O
routines, and attempts to assign ioread32/iowrite32 and friends directly
to its own internal function pointers. On many platforms these I/O
routines are macros or wrappers and not actual functions on their own,
resulting in things like:

ERROR: "ioread32be" [drivers/spi/xilinx_spi.ko] undefined!
ERROR: "iowrite32be" [drivers/spi/xilinx_spi.ko] undefined!
ERROR: "iowrite32" [drivers/spi/xilinx_spi.ko] undefined!
ERROR: "ioread32" [drivers/spi/xilinx_spi.ko] undefined!

If xilinx_spi wants to do this sort of casting, it needs to provide its
own wrappers for these, or change how it does accesses completely.

I've opted for the first approach, and the attached silly patch does
that. If someone with the hardware available wants to give the second
option a try that's ok too. In any event, the current code is broken for
at least: arm, avr32, blackfin, microblaze, mn10300, and sh.

Signed-off-by: Paul Mundt <lethal@linux-sh.org>

---

 drivers/spi/xilinx_spi.c |   28 ++++++++++++++++++++++++----
 1 file changed, 24 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/xilinx_spi.c b/drivers/spi/xilinx_spi.c
index 9f38637..154e908 100644
--- a/drivers/spi/xilinx_spi.c
+++ b/drivers/spi/xilinx_spi.c
@@ -93,6 +93,26 @@ struct xilinx_spi {
 	void (*rx_fn) (struct xilinx_spi *);
 };
 
+static void xspi_write32(u32 val, void __iomem *addr)
+{
+	iowrite32(val, addr);
+}
+
+static unsigned int xspi_read32(void __iomem *addr)
+{
+	return ioread32(addr);
+}
+
+static void xspi_write32_be(u32 val, void __iomem *addr)
+{
+	iowrite32be(val, addr);
+}
+
+static unsigned int xspi_read32_be(void __iomem *addr)
+{
+	return ioread32be(addr);
+}
+
 static void xspi_tx8(struct xilinx_spi *xspi)
 {
 	xspi->write_fn(*xspi->tx_ptr, xspi->regs + XSPI_TXD_OFFSET);
@@ -374,11 +394,11 @@ struct spi_master *xilinx_spi_init(struct device *dev, struct resource *mem,
 	xspi->mem = *mem;
 	xspi->irq = irq;
 	if (pdata->little_endian) {
-		xspi->read_fn = ioread32;
-		xspi->write_fn = iowrite32;
+		xspi->read_fn = xspi_read32;
+		xspi->write_fn = xspi_write32;
 	} else {
-		xspi->read_fn = ioread32be;
-		xspi->write_fn = iowrite32be;
+		xspi->read_fn = xspi_read32_be;
+		xspi->write_fn = xspi_write32_be;
 	}
 	xspi->bits_per_word = pdata->bits_per_word;
 	if (xspi->bits_per_word == 8) {

Re: [PATCH] spi: xilinx_spi: Fix up I/O routine wrapping bogosity.

[Richard Röjfors]

Paul Mundt wrote:
> xilinx_spi presently makes some fairly questionable assumptions about I/O
> routines, and attempts to assign ioread32/iowrite32 and friends directly
> to its own internal function pointers. On many platforms these I/O
> routines are macros or wrappers and not actual functions on their own,
> resulting in things like:
> 
> ERROR: "ioread32be" [drivers/spi/xilinx_spi.ko] undefined!
> ERROR: "iowrite32be" [drivers/spi/xilinx_spi.ko] undefined!
> ERROR: "iowrite32" [drivers/spi/xilinx_spi.ko] undefined!
> ERROR: "ioread32" [drivers/spi/xilinx_spi.ko] undefined!
> 
> If xilinx_spi wants to do this sort of casting, it needs to provide its
> own wrappers for these, or change how it does accesses completely.
> 
> I've opted for the first approach, and the attached silly patch does
> that. If someone with the hardware available wants to give the second
> option a try that's ok too. In any event, the current code is broken for
> at least: arm, avr32, blackfin, microblaze, mn10300, and sh.
> 
> Signed-off-by: Paul Mundt <lethal@linux-sh.org>

Looks good to me.

Acked-by: Richard Röjfors <richard.rojfors@pelagicore.com>

> 
> ---
> 
>  drivers/spi/xilinx_spi.c |   28 ++++++++++++++++++++++++----
>  1 file changed, 24 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/spi/xilinx_spi.c b/drivers/spi/xilinx_spi.c
> index 9f38637..154e908 100644
> --- a/drivers/spi/xilinx_spi.c
> +++ b/drivers/spi/xilinx_spi.c
> @@ -93,6 +93,26 @@ struct xilinx_spi {
>  	void (*rx_fn) (struct xilinx_spi *);
>  };
>  
> +static void xspi_write32(u32 val, void __iomem *addr)
> +{
> +	iowrite32(val, addr);
> +}
> +
> +static unsigned int xspi_read32(void __iomem *addr)
> +{
> +	return ioread32(addr);
> +}
> +
> +static void xspi_write32_be(u32 val, void __iomem *addr)
> +{
> +	iowrite32be(val, addr);
> +}
> +
> +static unsigned int xspi_read32_be(void __iomem *addr)
> +{
> +	return ioread32be(addr);
> +}
> +
>  static void xspi_tx8(struct xilinx_spi *xspi)
>  {
>  	xspi->write_fn(*xspi->tx_ptr, xspi->regs + XSPI_TXD_OFFSET);
> @@ -374,11 +394,11 @@ struct spi_master *xilinx_spi_init(struct device *dev, struct resource *mem,
>  	xspi->mem = *mem;
>  	xspi->irq = irq;
>  	if (pdata->little_endian) {
> -		xspi->read_fn = ioread32;
> -		xspi->write_fn = iowrite32;
> +		xspi->read_fn = xspi_read32;
> +		xspi->write_fn = xspi_write32;
>  	} else {
> -		xspi->read_fn = ioread32be;
> -		xspi->write_fn = iowrite32be;
> +		xspi->read_fn = xspi_read32_be;
> +		xspi->write_fn = xspi_write32_be;
>  	}
>  	xspi->bits_per_word = pdata->bits_per_word;
>  	if (xspi->bits_per_word == 8) {

Re: [PATCH] spi: xilinx_spi: Fix up I/O routine wrapping bogosity.

[Grant Likely]

On Tue, Dec 15, 2009 at 10:01 PM, Paul Mundt <lethal-M7jkjyW5wf5g9hUCZPvPmw@public.gmane.org> wrote:
> xilinx_spi presently makes some fairly questionable assumptions about I/O
> routines, and attempts to assign ioread32/iowrite32 and friends directly
> to its own internal function pointers. On many platforms these I/O
> routines are macros or wrappers and not actual functions on their own,
> resulting in things like:
>
> ERROR: "ioread32be" [drivers/spi/xilinx_spi.ko] undefined!
> ERROR: "iowrite32be" [drivers/spi/xilinx_spi.ko] undefined!
> ERROR: "iowrite32" [drivers/spi/xilinx_spi.ko] undefined!
> ERROR: "ioread32" [drivers/spi/xilinx_spi.ko] undefined!

Unfortunately, ARM doesn't have either ioread32be or iowrite32be
defined, so the problem is still not solved with this patch.  I've
crafted a patch to add them which I'll send to the ARM list right now.

g.

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

------------------------------------------------------------------------------
This SF.Net email is sponsored by the Verizon Developer Community
Take advantage of Verizon's best-in-class app development support
A streamlined, 14 day to market process makes app distribution fast and easy
Join now and get one step closer to millions of Verizon customers
http://p.sf.net/sfu/verizon-dev2dev