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

Jagan Teki jagannadh.teki at gmail.com
Thu Apr 23 08:46:16 CEST 2015


On 22 April 2015 at 18:10, Michal Simek <michal.simek at 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 at gmail.com>
>> Cc: Michal Simek <michal.simek at 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 at wytron.com.tw>
>> - * Copyright (c) 2010 Graeme Smecher <graeme.smecher at mail.mcgill.ca>
>> + * Based on bfin_spi.c, by way of altera_spi.c
>>   * Copyright (c) 2012 Stephan Linz <linz at li-pro.net>
>> + * Copyright (c) 2010 Graeme Smecher <graeme.smecher at mail.mcgill.ca>
>> + * Copyright (c) 2010 Thomas Chou <thomas at wytron.com.tw>
>> + * Copyright (c) 2005-2008 Analog Devices Inc.
>>   *
>>   * SPDX-License-Identifier:  GPL-2.0+
>> - *
>> - * [0]: http://www.xilinx.com/support/documentation
>> - *
>> - * [S]:      [0]/ip_documentation/xps_spi.pdf
>> - *   [0]/ip_documentation/axi_spi_ds742.pdf
>>   */
>> +
>>  #include <config.h>
>>  #include <common.h>
>>  #include <malloc.h>
>>  #include <spi.h>
>>
>>  /*
>> - * Xilinx SPI Register Definition
>> + * [0]: http://www.xilinx.com/support/documentation
>>   *
>> + * Xilinx SPI Register Definitions
>>   * [1]:      [0]/ip_documentation/xps_spi.pdf
>>   *   page 8, Register Descriptions
>>   * [2]:      [0]/ip_documentation/axi_spi_ds742.pdf
>>   *   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 at xilinx.com>

thanks!
-- 
Jagan.


More information about the U-Boot mailing list