lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Tue, 20 Sep 2011 14:50:00 -0600
From:	Grant Likely <grant.likely@...retlab.ca>
To:	H Hartley Sweeten <hartleys@...ionengravers.com>
Cc:	Linux Kernel <linux-kernel@...r.kernel.org>,
	spi-devel-general@...ts.sourceforge.net,
	Feng Tang <feng.tang@...el.com>
Subject: Re: [PATCH] spi: spi-dw: fix all sparse warnings

On Tue, Sep 20, 2011 at 11:06:17AM -0700, H Hartley Sweeten wrote:
> The dw_{read,write}[lw] macros produce sparse warnings everytime they
> are used.  The "read" ones cause:
> 
> warning: cast removes address space of expression
> warning: incorrect type in argument 1 (different address spaces)
>    expected void const volatile [noderef] <asn:2>*addr
>    got unsigned int *<noident>
> 
> And the "write" ones:
> 
> warning: cast removes address space of expression
> warning: incorrect type in argument 2 (different address spaces)
>    expected void volatile [noderef] <asn:2>*addr
>    got unsigned int *<noident>
> 
> Fix this by removing struct dw_spi_reg and converting all the register
> offsets to #defines. Then convert the macros into inlined functions so
> that proper type checking can occur.
> 
> While here, also fix the three sparse warnings in spi-dw-mid.c due to
> the return value of ioremap_nocache being stored in a u32 * not a
> void __iomem *.
> 
> With these changes the spi-dw* files all build with no sparse warnings.
> 
> Signed-off-by: H Hartley Sweeten <hsweeten@...ionengravers.com>
> Cc: Grant Likely <grant.likely@...retlab.ca>

Looks good to me.  Feng, does this look okay to you?

g.

> 
> ---
> 
> diff --git a/drivers/spi/spi-dw-mid.c b/drivers/spi/spi-dw-mid.c
> index 130e555..e743a45 100644
> --- a/drivers/spi/spi-dw-mid.c
> +++ b/drivers/spi/spi-dw-mid.c
> @@ -116,13 +116,13 @@ static int mid_spi_dma_transfer(struct dw_spi *dws, int cs_change)
>  	/* 1. setup DMA related registers */
>  	if (cs_change) {
>  		spi_enable_chip(dws, 0);
> -		dw_writew(dws, dmardlr, 0xf);
> -		dw_writew(dws, dmatdlr, 0x10);
> +		dw_writew(dws, DW_SPI_DMARDLR, 0xf);
> +		dw_writew(dws, DW_SPI_DMATDLR, 0x10);
>  		if (dws->tx_dma)
>  			dma_ctrl |= 0x2;
>  		if (dws->rx_dma)
>  			dma_ctrl |= 0x1;
> -		dw_writew(dws, dmacr, dma_ctrl);
> +		dw_writew(dws, DW_SPI_DMACR, dma_ctrl);
>  		spi_enable_chip(dws, 1);
>  	}
>  
> @@ -200,7 +200,8 @@ static struct dw_spi_dma_ops mid_dma_ops = {
>  
>  int dw_spi_mid_init(struct dw_spi *dws)
>  {
> -	u32 *clk_reg, clk_cdiv;
> +	void __iomem *clk_reg;
> +	u32 clk_cdiv;
>  
>  	clk_reg = ioremap_nocache(MRST_CLK_SPI0_REG, 16);
>  	if (!clk_reg)
> diff --git a/drivers/spi/spi-dw.c b/drivers/spi/spi-dw.c
> index 0b99320..082458d 100644
> --- a/drivers/spi/spi-dw.c
> +++ b/drivers/spi/spi-dw.c
> @@ -89,35 +89,35 @@ static ssize_t  spi_show_regs(struct file *file, char __user *user_buf,
>  	len += snprintf(buf + len, SPI_REGS_BUFSIZE - len,
>  			"=================================\n");
>  	len += snprintf(buf + len, SPI_REGS_BUFSIZE - len,
> -			"CTRL0: \t\t0x%08x\n", dw_readl(dws, ctrl0));
> +			"CTRL0: \t\t0x%08x\n", dw_readl(dws, DW_SPI_CTRL0));
>  	len += snprintf(buf + len, SPI_REGS_BUFSIZE - len,
> -			"CTRL1: \t\t0x%08x\n", dw_readl(dws, ctrl1));
> +			"CTRL1: \t\t0x%08x\n", dw_readl(dws, DW_SPI_CTRL1));
>  	len += snprintf(buf + len, SPI_REGS_BUFSIZE - len,
> -			"SSIENR: \t0x%08x\n", dw_readl(dws, ssienr));
> +			"SSIENR: \t0x%08x\n", dw_readl(dws, DW_SPI_SSIENR));
>  	len += snprintf(buf + len, SPI_REGS_BUFSIZE - len,
> -			"SER: \t\t0x%08x\n", dw_readl(dws, ser));
> +			"SER: \t\t0x%08x\n", dw_readl(dws, DW_SPI_SER));
>  	len += snprintf(buf + len, SPI_REGS_BUFSIZE - len,
> -			"BAUDR: \t\t0x%08x\n", dw_readl(dws, baudr));
> +			"BAUDR: \t\t0x%08x\n", dw_readl(dws, DW_SPI_BAUDR));
>  	len += snprintf(buf + len, SPI_REGS_BUFSIZE - len,
> -			"TXFTLR: \t0x%08x\n", dw_readl(dws, txfltr));
> +			"TXFTLR: \t0x%08x\n", dw_readl(dws, DW_SPI_TXFLTR));
>  	len += snprintf(buf + len, SPI_REGS_BUFSIZE - len,
> -			"RXFTLR: \t0x%08x\n", dw_readl(dws, rxfltr));
> +			"RXFTLR: \t0x%08x\n", dw_readl(dws, DW_SPI_RXFLTR));
>  	len += snprintf(buf + len, SPI_REGS_BUFSIZE - len,
> -			"TXFLR: \t\t0x%08x\n", dw_readl(dws, txflr));
> +			"TXFLR: \t\t0x%08x\n", dw_readl(dws, DW_SPI_TXFLR));
>  	len += snprintf(buf + len, SPI_REGS_BUFSIZE - len,
> -			"RXFLR: \t\t0x%08x\n", dw_readl(dws, rxflr));
> +			"RXFLR: \t\t0x%08x\n", dw_readl(dws, DW_SPI_RXFLR));
>  	len += snprintf(buf + len, SPI_REGS_BUFSIZE - len,
> -			"SR: \t\t0x%08x\n", dw_readl(dws, sr));
> +			"SR: \t\t0x%08x\n", dw_readl(dws, DW_SPI_SR));
>  	len += snprintf(buf + len, SPI_REGS_BUFSIZE - len,
> -			"IMR: \t\t0x%08x\n", dw_readl(dws, imr));
> +			"IMR: \t\t0x%08x\n", dw_readl(dws, DW_SPI_IMR));
>  	len += snprintf(buf + len, SPI_REGS_BUFSIZE - len,
> -			"ISR: \t\t0x%08x\n", dw_readl(dws, isr));
> +			"ISR: \t\t0x%08x\n", dw_readl(dws, DW_SPI_ISR));
>  	len += snprintf(buf + len, SPI_REGS_BUFSIZE - len,
> -			"DMACR: \t\t0x%08x\n", dw_readl(dws, dmacr));
> +			"DMACR: \t\t0x%08x\n", dw_readl(dws, DW_SPI_DMACR));
>  	len += snprintf(buf + len, SPI_REGS_BUFSIZE - len,
> -			"DMATDLR: \t0x%08x\n", dw_readl(dws, dmatdlr));
> +			"DMATDLR: \t0x%08x\n", dw_readl(dws, DW_SPI_DMATDLR));
>  	len += snprintf(buf + len, SPI_REGS_BUFSIZE - len,
> -			"DMARDLR: \t0x%08x\n", dw_readl(dws, dmardlr));
> +			"DMARDLR: \t0x%08x\n", dw_readl(dws, DW_SPI_DMARDLR));
>  	len += snprintf(buf + len, SPI_REGS_BUFSIZE - len,
>  			"=================================\n");
>  
> @@ -167,7 +167,7 @@ static inline u32 tx_max(struct dw_spi *dws)
>  	u32 tx_left, tx_room, rxtx_gap;
>  
>  	tx_left = (dws->tx_end - dws->tx) / dws->n_bytes;
> -	tx_room = dws->fifo_len - dw_readw(dws, txflr);
> +	tx_room = dws->fifo_len - dw_readw(dws, DW_SPI_TXFLR);
>  
>  	/*
>  	 * Another concern is about the tx/rx mismatch, we
> @@ -188,7 +188,7 @@ static inline u32 rx_max(struct dw_spi *dws)
>  {
>  	u32 rx_left = (dws->rx_end - dws->rx) / dws->n_bytes;
>  
> -	return min(rx_left, (u32)dw_readw(dws, rxflr));
> +	return min(rx_left, (u32)dw_readw(dws, DW_SPI_RXFLR));
>  }
>  
>  static void dw_writer(struct dw_spi *dws)
> @@ -204,7 +204,7 @@ static void dw_writer(struct dw_spi *dws)
>  			else
>  				txw = *(u16 *)(dws->tx);
>  		}
> -		dw_writew(dws, dr, txw);
> +		dw_writew(dws, DW_SPI_DR, txw);
>  		dws->tx += dws->n_bytes;
>  	}
>  }
> @@ -215,7 +215,7 @@ static void dw_reader(struct dw_spi *dws)
>  	u16 rxw;
>  
>  	while (max--) {
> -		rxw = dw_readw(dws, dr);
> +		rxw = dw_readw(dws, DW_SPI_DR);
>  		/* Care rx only if the transfer's original "rx" is not null */
>  		if (dws->rx_end - dws->len) {
>  			if (dws->n_bytes == 1)
> @@ -323,13 +323,13 @@ EXPORT_SYMBOL_GPL(dw_spi_xfer_done);
>  
>  static irqreturn_t interrupt_transfer(struct dw_spi *dws)
>  {
> -	u16 irq_status = dw_readw(dws, isr);
> +	u16 irq_status = dw_readw(dws, DW_SPI_ISR);
>  
>  	/* Error handling */
>  	if (irq_status & (SPI_INT_TXOI | SPI_INT_RXOI | SPI_INT_RXUI)) {
> -		dw_readw(dws, txoicr);
> -		dw_readw(dws, rxoicr);
> -		dw_readw(dws, rxuicr);
> +		dw_readw(dws, DW_SPI_TXOICR);
> +		dw_readw(dws, DW_SPI_RXOICR);
> +		dw_readw(dws, DW_SPI_RXUICR);
>  		int_error_stop(dws, "interrupt_transfer: fifo overrun/underrun");
>  		return IRQ_HANDLED;
>  	}
> @@ -353,7 +353,7 @@ static irqreturn_t interrupt_transfer(struct dw_spi *dws)
>  static irqreturn_t dw_spi_irq(int irq, void *dev_id)
>  {
>  	struct dw_spi *dws = dev_id;
> -	u16 irq_status = dw_readw(dws, isr) & 0x3f;
> +	u16 irq_status = dw_readw(dws, DW_SPI_ISR) & 0x3f;
>  
>  	if (!irq_status)
>  		return IRQ_NONE;
> @@ -521,11 +521,11 @@ static void pump_transfers(unsigned long data)
>  	 *	2. clk_div is changed
>  	 *	3. control value changes
>  	 */
> -	if (dw_readw(dws, ctrl0) != cr0 || cs_change || clk_div || imask) {
> +	if (dw_readw(dws, DW_SPI_CTRL0) != cr0 || cs_change || clk_div || imask) {
>  		spi_enable_chip(dws, 0);
>  
> -		if (dw_readw(dws, ctrl0) != cr0)
> -			dw_writew(dws, ctrl0, cr0);
> +		if (dw_readw(dws, DW_SPI_CTRL0) != cr0)
> +			dw_writew(dws, DW_SPI_CTRL0, cr0);
>  
>  		spi_set_clk(dws, clk_div ? clk_div : chip->clk_div);
>  		spi_chip_sel(dws, spi->chip_select);
> @@ -535,7 +535,7 @@ static void pump_transfers(unsigned long data)
>  		if (imask)
>  			spi_umask_intr(dws, imask);
>  		if (txint_level)
> -			dw_writew(dws, txfltr, txint_level);
> +			dw_writew(dws, DW_SPI_TXFLTR, txint_level);
>  
>  		spi_enable_chip(dws, 1);
>  		if (cs_change)
> @@ -791,13 +791,13 @@ static void spi_hw_init(struct dw_spi *dws)
>  	if (!dws->fifo_len) {
>  		u32 fifo;
>  		for (fifo = 2; fifo <= 257; fifo++) {
> -			dw_writew(dws, txfltr, fifo);
> -			if (fifo != dw_readw(dws, txfltr))
> +			dw_writew(dws, DW_SPI_TXFLTR, fifo);
> +			if (fifo != dw_readw(dws, DW_SPI_TXFLTR))
>  				break;
>  		}
>  
>  		dws->fifo_len = (fifo == 257) ? 0 : fifo;
> -		dw_writew(dws, txfltr, 0);
> +		dw_writew(dws, DW_SPI_TXFLTR, 0);
>  	}
>  }
>  
> diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h
> index 8b7b07b..9c57c07 100644
> --- a/drivers/spi/spi-dw.h
> +++ b/drivers/spi/spi-dw.h
> @@ -4,6 +4,33 @@
>  #include <linux/io.h>
>  #include <linux/scatterlist.h>
>  
> +/* Register offsets */
> +#define DW_SPI_CTRL0			0x00
> +#define DW_SPI_CTRL1			0x04
> +#define DW_SPI_SSIENR			0x08
> +#define DW_SPI_MWCR			0x0c
> +#define DW_SPI_SER			0x10
> +#define DW_SPI_BAUDR			0x14
> +#define DW_SPI_TXFLTR			0x18
> +#define DW_SPI_RXFLTR			0x1c
> +#define DW_SPI_TXFLR			0x20
> +#define DW_SPI_RXFLR			0x24
> +#define DW_SPI_SR			0x28
> +#define DW_SPI_IMR			0x2c
> +#define DW_SPI_ISR			0x30
> +#define DW_SPI_RISR			0x34
> +#define DW_SPI_TXOICR			0x38
> +#define DW_SPI_RXOICR			0x3c
> +#define DW_SPI_RXUICR			0x40
> +#define DW_SPI_MSTICR			0x44
> +#define DW_SPI_ICR			0x48
> +#define DW_SPI_DMACR			0x4c
> +#define DW_SPI_DMATDLR			0x50
> +#define DW_SPI_DMARDLR			0x54
> +#define DW_SPI_IDR			0x58
> +#define DW_SPI_VERSION			0x5c
> +#define DW_SPI_DR			0x60
> +
>  /* Bit fields in CTRLR0 */
>  #define SPI_DFS_OFFSET			0
>  
> @@ -55,35 +82,6 @@ enum dw_ssi_type {
>  	SSI_NS_MICROWIRE,
>  };
>  
> -struct dw_spi_reg {
> -	u32	ctrl0;
> -	u32	ctrl1;
> -	u32	ssienr;
> -	u32	mwcr;
> -	u32	ser;
> -	u32	baudr;
> -	u32	txfltr;
> -	u32	rxfltr;
> -	u32	txflr;
> -	u32	rxflr;
> -	u32	sr;
> -	u32	imr;
> -	u32	isr;
> -	u32	risr;
> -	u32	txoicr;
> -	u32	rxoicr;
> -	u32	rxuicr;
> -	u32	msticr;
> -	u32	icr;
> -	u32	dmacr;
> -	u32	dmatdlr;
> -	u32	dmardlr;
> -	u32	idr;
> -	u32	version;
> -	u32	dr;		/* Currently oper as 32 bits,
> -				though only low 16 bits matters */
> -} __packed;
> -
>  struct dw_spi;
>  struct dw_spi_dma_ops {
>  	int (*dma_init)(struct dw_spi *dws);
> @@ -161,23 +159,34 @@ struct dw_spi {
>  #endif
>  };
>  
> -#define dw_readl(dw, name) \
> -	__raw_readl(&(((struct dw_spi_reg *)dw->regs)->name))
> -#define dw_writel(dw, name, val) \
> -	__raw_writel((val), &(((struct dw_spi_reg *)dw->regs)->name))
> -#define dw_readw(dw, name) \
> -	__raw_readw(&(((struct dw_spi_reg *)dw->regs)->name))
> -#define dw_writew(dw, name, val) \
> -	__raw_writew((val), &(((struct dw_spi_reg *)dw->regs)->name))
> +static inline u32 dw_readl(struct dw_spi *dws, u32 offset)
> +{
> +	return __raw_readl(dws->regs + offset);
> +}
> +
> +static inline void dw_writel(struct dw_spi *dws, u32 offset, u32 val)
> +{
> +	__raw_writel(val, dws->regs + offset);
> +}
> +
> +static inline u16 dw_readw(struct dw_spi *dws, u32 offset)
> +{
> +	return __raw_readw(dws->regs + offset);
> +}
> +
> +static inline void dw_writew(struct dw_spi *dws, u32 offset, u16 val)
> +{
> +	__raw_writew(val, dws->regs + offset);
> +}
>  
>  static inline void spi_enable_chip(struct dw_spi *dws, int enable)
>  {
> -	dw_writel(dws, ssienr, (enable ? 1 : 0));
> +	dw_writel(dws, DW_SPI_SSIENR, (enable ? 1 : 0));
>  }
>  
>  static inline void spi_set_clk(struct dw_spi *dws, u16 div)
>  {
> -	dw_writel(dws, baudr, div);
> +	dw_writel(dws, DW_SPI_BAUDR, div);
>  }
>  
>  static inline void spi_chip_sel(struct dw_spi *dws, u16 cs)
> @@ -188,7 +197,7 @@ static inline void spi_chip_sel(struct dw_spi *dws, u16 cs)
>  	if (dws->cs_control)
>  		dws->cs_control(1);
>  
> -	dw_writel(dws, ser, 1 << cs);
> +	dw_writel(dws, DW_SPI_SER, 1 << cs);
>  }
>  
>  /* Disable IRQ bits */
> @@ -196,8 +205,8 @@ static inline void spi_mask_intr(struct dw_spi *dws, u32 mask)
>  {
>  	u32 new_mask;
>  
> -	new_mask = dw_readl(dws, imr) & ~mask;
> -	dw_writel(dws, imr, new_mask);
> +	new_mask = dw_readl(dws, DW_SPI_IMR) & ~mask;
> +	dw_writel(dws, DW_SPI_IMR, new_mask);
>  }
>  
>  /* Enable IRQ bits */
> @@ -205,8 +214,8 @@ static inline void spi_umask_intr(struct dw_spi *dws, u32 mask)
>  {
>  	u32 new_mask;
>  
> -	new_mask = dw_readl(dws, imr) | mask;
> -	dw_writel(dws, imr, new_mask);
> +	new_mask = dw_readl(dws, DW_SPI_IMR) | mask;
> +	dw_writel(dws, DW_SPI_IMR, new_mask);
>  }
>  
>  /*
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ