Re: [U-Boot] [U-Boot PATCH 6/8] spi: xilinx_spi: Driver clean-up

On 22 April 2015 at 18:10, Michal Simek michal.simek@xilinx.com wrote:

On 04/21/2015 08:27 PM, Jagannadha Sutradharudu Teki wrote:

...

• Zap unneeded macros
• Re-arrange the code
• Removed __attribute__((weak))
• Replace __func__ macro with func names to save macro transition.
• Re-arranged comment lines.
• Arrange driver code in more readable format[1]

[1] http://lists.denx.de/pipermail/u-boot/2013-August/160473.html

Signed-off-by: Jagannadha Sutradharudu Teki jagannadh.teki@gmail.com Cc: Michal Simek michal.simek@xilinx.com

---

drivers/spi/xilinx_spi.c | 164 ++++++++++++++++------------------------------- 1 file changed, 57 insertions(+), 107 deletions(-)

diff --git a/drivers/spi/xilinx_spi.c b/drivers/spi/xilinx_spi.c index 8073edc..650e494 100644 --- a/drivers/spi/xilinx_spi.c +++ b/drivers/spi/xilinx_spi.c @@ -1,79 +1,31 @@ /*

• Xilinx SPI driver

• • supports 8 bit SPI transfers only, with or w/o FIFO

• • Supports 8 bit SPI transfers only, with or w/o FIFO

• • based on bfin_spi.c, by way of altera_spi.c
• • Copyright (c) 2005-2008 Analog Devices Inc.
• • Copyright (c) 2010 Thomas Chou thomas@wytron.com.tw
• • Copyright (c) 2010 Graeme Smecher graeme.smecher@mail.mcgill.ca

• • Based on bfin_spi.c, by way of altera_spi.c
  • Copyright (c) 2012 Stephan Linz linz@li-pro.net
• • Copyright (c) 2010 Graeme Smecher graeme.smecher@mail.mcgill.ca
• • Copyright (c) 2010 Thomas Chou thomas@wytron.com.tw
• • Copyright (c) 2005-2008 Analog Devices Inc.
  • SPDX-License-Identifier: GPL-2.0+

• • [0]/ip_documentation/axi_spi_ds742.pdf
  */

#include <config.h> #include <common.h> #include <malloc.h> #include <spi.h>

/*

• • Xilinx SPI Register Definition

• • Xilinx SPI Register Definitions
  • page 8, Register Descriptions
  • page 7, Register Overview Table
  */

-struct xilinx_spi_reg {

• u32 __space0__[7];
  

• u32 dgier;      /* Device Global Interrupt Enable Register (DGIER) */
  

• u32 ipisr;      /* IP Interrupt Status Register (IPISR) */
  

• u32 __space1__;
  

• u32 ipier;      /* IP Interrupt Enable Register (IPIER) */
  

• u32 __space2__[5];
  

• u32 srr;        /* Softare Reset Register (SRR) */
  

• u32 __space3__[7];
  

• u32 spicr;      /* SPI Control Register (SPICR) */
  

• u32 spisr;      /* SPI Status Register (SPISR) */
  

• u32 spidtr;     /* SPI Data Transmit Register (SPIDTR) */
  

• u32 spidrr;     /* SPI Data Receive Register (SPIDRR) */
  

• u32 spissr;     /* SPI Slave Select Register (SPISSR) */
  

• u32 spitfor;    /* SPI Transmit FIFO Occupancy Register (SPITFOR) */
  

• u32 spirfor;    /* SPI Receive FIFO Occupancy Register (SPIRFOR) */
  

-};

-/* Device Global Interrupt Enable Register (dgier), [1] p15, [2] p15 */ -#define DGIER_GIE (1 << 31)

-/* IP Interrupt Status Register (ipisr), [1] p15, [2] p15 */ -#define IPISR_DRR_NOT_EMPTY (1 << 8) -#define IPISR_SLAVE_SELECT (1 << 7) -#define IPISR_TXF_HALF_EMPTY (1 << 6) -#define IPISR_DRR_OVERRUN (1 << 5) -#define IPISR_DRR_FULL (1 << 4) -#define IPISR_DTR_UNDERRUN (1 << 3) -#define IPISR_DTR_EMPTY (1 << 2) -#define IPISR_SLAVE_MODF (1 << 1) -#define IPISR_MODF (1 << 0)

-/* IP Interrupt Enable Register (ipier), [1] p17, [2] p18 */ -#define IPIER_DRR_NOT_EMPTY (1 << 8) -#define IPIER_SLAVE_SELECT (1 << 7) -#define IPIER_TXF_HALF_EMPTY (1 << 6) -#define IPIER_DRR_OVERRUN (1 << 5) -#define IPIER_DRR_FULL (1 << 4) -#define IPIER_DTR_UNDERRUN (1 << 3) -#define IPIER_DTR_EMPTY (1 << 2) -#define IPIER_SLAVE_MODF (1 << 1) -#define IPIER_MODF (1 << 0)

-/* Softare Reset Register (srr), [1] p9, [2] p8 */ -#define SRR_RESET_CODE 0x0000000A

/* SPI Control Register (spicr), [1] p9, [2] p8 */ #define SPICR_LSB_FIRST (1 << 9) @@ -110,17 +62,42 @@ struct xilinx_spi_reg { #define SPISSR_ACT(cs) ~SPISSR_MASK(cs) #define SPISSR_OFF ~0UL

-/* SPI Transmit FIFO Occupancy Register (spitfor), [1] p13, [2] p14 */ -#define SPITFOR_OCYVAL_POS 0 -#define SPITFOR_OCYVAL_MASK (0xf << SPITFOR_OCYVAL_POS)

-/* SPI Receive FIFO Occupancy Register (spirfor), [1] p14, [2] p14 */ -#define SPIRFOR_OCYVAL_POS 0 -#define SPIRFOR_OCYVAL_MASK (0xf << SPIRFOR_OCYVAL_POS)

/* SPI Software Reset Register (ssr) */ #define SPISSR_RESET_VALUE 0x0a

+#define XILSPI_MAX_XFER_BITS 8 +#define XILSPI_SPICR_DFLT_ON (SPICR_MANUAL_SS | SPICR_MASTER_MODE | \

•                         SPICR_SPE)
  

+#define XILSPI_SPICR_DFLT_OFF (SPICR_MASTER_INHIBIT | SPICR_MANUAL_SS)

+#ifndef CONFIG_XILINX_SPI_IDLE_VAL +#define CONFIG_XILINX_SPI_IDLE_VAL 0xff +#endif

+#ifndef CONFIG_SYS_XILINX_SPI_LIST +#define CONFIG_SYS_XILINX_SPI_LIST { CONFIG_SYS_SPI_BASE } +#endif

+/* xilinx spi register set */ +struct xilinx_spi_reg {

• u32 __space0__[7];
  

• u32 dgier;      /* Device Global Interrupt Enable Register (DGIER) */
  

• u32 ipisr;      /* IP Interrupt Status Register (IPISR) */
  

• u32 __space1__;
  

• u32 ipier;      /* IP Interrupt Enable Register (IPIER) */
  

• u32 __space2__[5];
  

• u32 srr;        /* Softare Reset Register (SRR) */
  

• u32 __space3__[7];
  

• u32 spicr;      /* SPI Control Register (SPICR) */
  

• u32 spisr;      /* SPI Status Register (SPISR) */
  

• u32 spidtr;     /* SPI Data Transmit Register (SPIDTR) */
  

• u32 spidrr;     /* SPI Data Receive Register (SPIDRR) */
  

• u32 spissr;     /* SPI Slave Select Register (SPISSR) */
  

• u32 spitfor;    /* SPI Transmit FIFO Occupancy Register (SPITFOR) */
  

• u32 spirfor;    /* SPI Receive FIFO Occupancy Register (SPIRFOR) */
  

+};

+/* xilinx spi slave */ struct xilinx_spi_slave { struct spi_slave slave; struct xilinx_spi_reg *regs; @@ -129,37 +106,17 @@ struct xilinx_spi_slave { };

static inline struct xilinx_spi_slave *to_xilinx_spi_slave(

•                                 struct spi_slave *slave)
  

•                 struct spi_slave *slave)
  

{ return container_of(slave, struct xilinx_spi_slave, slave); }

-#ifndef CONFIG_SYS_XILINX_SPI_LIST -#define CONFIG_SYS_XILINX_SPI_LIST { CONFIG_SYS_SPI_BASE } -#endif

-#ifndef CONFIG_XILINX_SPI_IDLE_VAL -#define CONFIG_XILINX_SPI_IDLE_VAL 0xff -#endif

-#define XILSPI_SPICR_DFLT_ON (SPICR_MANUAL_SS | \

•                                  SPICR_MASTER_MODE | \
  

•                                  SPICR_SPE)
  

-#define XILSPI_SPICR_DFLT_OFF (SPICR_MASTER_INHIBIT | \

•                                  SPICR_MANUAL_SS)
  

-#define XILSPI_MAX_XFER_BITS 8

static unsigned long xilinx_spi_base_list[] = CONFIG_SYS_XILINX_SPI_LIST;

-__attribute__((weak)) int spi_cs_is_valid(unsigned int bus, unsigned int cs) { return bus < ARRAY_SIZE(xilinx_spi_base_list) && cs < 32; }

-__attribute__((weak)) void spi_cs_activate(struct spi_slave *slave) { struct xilinx_spi_slave *xilspi = to_xilinx_spi_slave(slave); @@ -167,7 +124,6 @@ void spi_cs_activate(struct spi_slave *slave) writel(SPISSR_ACT(slave->cs), &xilspi->regs->spissr); }

-__attribute__((weak)) void spi_cs_deactivate(struct spi_slave *slave) { struct xilinx_spi_slave *xilspi = to_xilinx_spi_slave(slave); @@ -180,33 +136,26 @@ void spi_init(void) /* do nothing */ }

-void spi_set_speed(struct spi_slave *slave, uint hz) -{

• /* xilinx spi core does not support programmable speed */
  

-}

struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs, unsigned int max_hz, unsigned int mode) { struct xilinx_spi_slave *xilspi;

  if (!spi_cs_is_valid(bus, cs)) {

•         printf("XILSPI error: %s: unsupported bus %d / cs %d\n",
  

•                         __func__, bus, cs);
  

•         printf("XILSPI error: unsupported bus %d / cs %d\n", bus, cs);
          return NULL;
  }
  
  xilspi = spi_alloc_slave(struct xilinx_spi_slave, bus, cs);
  if (!xilspi) {
  

•         printf("XILSPI error: %s: malloc of SPI structure failed\n",
  

•                         __func__);
  

•         printf("XILSPI error: malloc of SPI structure failed\n");
          return NULL;
  }
  xilspi->regs = (struct xilinx_spi_reg *)xilinx_spi_base_list[bus];
  xilspi->freq = max_hz;
  xilspi->mode = mode;
  

• debug("%s: bus:%i cs:%i base:%p mode:%x max_hz:%d\n", __func__,
  

•         bus, cs, xilspi->regs, xilspi->mode, xilspi->freq);
  

• debug("spi_setup_slave: bus:%i cs:%i base:%p mode:%x max_hz:%d\n",
  

•       bus, cs, xilspi->regs, xilspi->mode, xilspi->freq);
  
  writel(SPISSR_RESET_VALUE, &xilspi->regs->srr);
  
  

@@ -225,7 +174,7 @@ int spi_claim_bus(struct spi_slave *slave) struct xilinx_spi_slave *xilspi = to_xilinx_spi_slave(slave); u32 spicr;

• debug("%s: bus:%i cs:%i\n", __func__, slave->bus, slave->cs);
  

• debug("spi_claim_bus: bus:%i cs:%i\n", slave->bus, slave->cs);
  writel(SPISSR_OFF, &xilspi->regs->spissr);
  
  spicr = XILSPI_SPICR_DFLT_ON;
  

@@ -246,7 +195,7 @@ void spi_release_bus(struct spi_slave *slave) { struct xilinx_spi_slave *xilspi = to_xilinx_spi_slave(slave);

• debug("%s: bus:%i cs:%i\n", __func__, slave->bus, slave->cs);
  

• debug("spi_release_bus: bus:%i cs:%i\n", slave->bus, slave->cs);
  writel(SPISSR_OFF, &xilspi->regs->spissr);
  writel(XILSPI_SPICR_DFLT_OFF, &xilspi->regs->spicr);
  

} @@ -262,14 +211,15 @@ int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout, unsigned rxecount = 17; /* max. 16 elements in FIFO, leftover 1 */ unsigned global_timeout;

• debug("%s: bus:%i cs:%i bitlen:%i bytes:%i flags:%lx\n", __func__,
  

•         slave->bus, slave->cs, bitlen, bytes, flags);
  

• debug("spi_xfer: bus:%i cs:%i bitlen:%i bytes:%i flags:%lx\n",
  

•       slave->bus, slave->cs, bitlen, bytes, flags);
  

• if (bitlen == 0)
          goto done;
  
  if (bitlen % XILSPI_MAX_XFER_BITS) {
  

•         printf("XILSPI warning: %s: Not a multiple of %d bits\n",
  

•                         __func__, XILSPI_MAX_XFER_BITS);
  

•         printf("XILSPI warning: Not a multiple of %d bits\n",
  

•                XILSPI_MAX_XFER_BITS);
          flags |= SPI_XFER_END;
          goto done;
  }
  

@@ -281,7 +231,7 @@ int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout, }

  if (!rxecount) {

•         printf("XILSPI error: %s: Rx buffer not empty\n", __func__);
  

•         printf("XILSPI error: Rx buffer not empty\n");
          return -1;
  }
  
  

@@ -296,7 +246,7 @@ int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout, unsigned timeout = global_timeout; /* get Tx element from data out buffer and count up */ unsigned char d = txp ? *txp++ : CONFIG_XILINX_SPI_IDLE_VAL;

•         debug("%s: tx:%x ", __func__, d);
  

•         debug("spi_xfer: tx:%x ", d);
  
          /* write out and wait for processing (receive data) */
          writel(d & SPIDTR_8BIT_MASK, &xilspi->regs->spidtr);
  

@@ -307,7 +257,7 @@ int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout, }

          if (!timeout) {

•                 printf("XILSPI error: %s: Xfer timeout\n", __func__);
  

•                 printf("XILSPI error: Xfer timeout\n");
                  return -1;
          }
  
  

@@ -315,7 +265,7 @@ int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *dout, d = readl(&xilspi->regs->spidrr) & SPIDRR_8BIT_MASK; if (rxp) *rxp++ = d;

•         debug("rx:%x\n", d);
  

•         debug("spi_xfer: rx:%x\n", d);
  }
  
  

  done:

Hopefully there is functional change there.

It's debug print update similar to tx, may be I will add note on commit message.

Acked-by: Michal Simek michal.simek@xilinx.com

thanks!