| Message ID | 20160919090041.4024-1-jszhang@marvell.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Monday, September 19, 2016 5:00:41 PM CEST Jisheng Zhang wrote: > Using the __raw functions is discouraged. Update the driver to use the > relaxed functions. > > Signed-off-by: Jisheng Zhang <jszhang@marvell.com> This should mention that it fixes the driver for big-endian kernels. However, it seems that the fix is only correct for the MMIO registers, while the polled FIFO access is now wrong AFAICT, both reader and writer: static void dw_reader(struct dw_spi *dws) { u32 max = rx_max(dws); u16 rxw; while (max--) { rxw = dw_read_io_reg(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) *(u8 *)(dws->rx) = rxw; else *(u16 *)(dws->rx) = rxw; } dws->rx += dws->n_bytes; } } As the FIFO is a byte stream, we have to use a non-swapping accessor here, such as readsl() which would also take care of the loop. The "n_bytes == 1" case probably should look like e.g. *(u8 *)(dws->rx) = *(u8 *)&rxw; so you are sure to get the first byte of the rxw variable rather than the lower 8 bits. > diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h > index 61bc3cb..2cfdc4d 100644 > --- a/drivers/spi/spi-dw.h > +++ b/drivers/spi/spi-dw.h > @@ -143,22 +143,22 @@ struct dw_spi { > > static inline u32 dw_readl(struct dw_spi *dws, u32 offset) > { > - return __raw_readl(dws->regs + offset); > + return readl_relaxed(dws->regs + offset); > } > > static inline u16 dw_readw(struct dw_spi *dws, u32 offset) > { > - return __raw_readw(dws->regs + offset); > + return readw_relaxed(dws->regs + offset); > } > What is the reason for using readl_relaxed() rather than the normal readl() here? In almost all instances that these are called, you don't care about the added latency and should just default to the normal I/O functions. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h index 61bc3cb..2cfdc4d 100644 --- a/drivers/spi/spi-dw.h +++ b/drivers/spi/spi-dw.h @@ -143,22 +143,22 @@ struct dw_spi { static inline u32 dw_readl(struct dw_spi *dws, u32 offset) { - return __raw_readl(dws->regs + offset); + return readl_relaxed(dws->regs + offset); } static inline u16 dw_readw(struct dw_spi *dws, u32 offset) { - return __raw_readw(dws->regs + offset); + return readw_relaxed(dws->regs + offset); } static inline void dw_writel(struct dw_spi *dws, u32 offset, u32 val) { - __raw_writel(val, dws->regs + offset); + writel_relaxed(val, dws->regs + offset); } static inline void dw_writew(struct dw_spi *dws, u32 offset, u16 val) { - __raw_writew(val, dws->regs + offset); + writew_relaxed(val, dws->regs + offset); } static inline u32 dw_read_io_reg(struct dw_spi *dws, u32 offset)
Using the __raw functions is discouraged. Update the driver to use the relaxed functions. Signed-off-by: Jisheng Zhang <jszhang@marvell.com> --- drivers/spi/spi-dw.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)