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

Michal Simek michal.simek at xilinx.com
Wed Apr 22 14:40:16 CEST 2015


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.

Acked-by: Michal Simek <michal.simek at xilinx.com>

Thanks,
Michal




More information about the U-Boot mailing list