From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1759817AbcISKUS (ORCPT ); Mon, 19 Sep 2016 06:20:18 -0400 Received: from mout.kundenserver.de ([217.72.192.74]:58547 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754265AbcISKUK (ORCPT ); Mon, 19 Sep 2016 06:20:10 -0400 From: Arnd Bergmann To: linux-arm-kernel@lists.infradead.org Cc: Jisheng Zhang , broonie@kernel.org, linux-kernel@vger.kernel.org, linux-spi@vger.kernel.org Subject: Re: [PATCH] spi: dw: use relaxed IO accessor Date: Mon, 19 Sep 2016 12:19:33 +0200 Message-ID: <5512851.aA4Sb0uIYP@wuerfel> User-Agent: KMail/5.1.3 (Linux/4.4.0-34-generic; KDE/5.18.0; x86_64; ; ) In-Reply-To: <20160919090041.4024-1-jszhang@marvell.com> References: <20160919090041.4024-1-jszhang@marvell.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Provags-ID: V03:K0:7Q62nBsY555xsD4IhDNC2hUujKv1XMRDAEAdT8OiRRMRV/e0Bk0 Ionkbbvhf+7SenH5r2oNxjEInD/pYQFJGsUPUzrIsQh6D1cyq5NgrsCHLlvIPR1lZWsowsM rrhE1p2NcdQ+wXHaCDUCVKchaH6L82Wccf7PvxFyAClaImedHs6PGwnZxb6ssxG0jXpUyDs LKX8HAu1mIYFs7LrL0P2A== X-UI-Out-Filterresults: notjunk:1;V01:K0:gixR/igLmbs=:ftoizR8oOLbJhefwM+9ZVc 3D/Ya4uRqVQ8s+YxTJSKuSwd48SCvVYgsfrF8ALJXMeXzSDy6nWrsj4Teg1ql7D2n1L9r+RKk NYNQ5DEoX4T0XtIzTkkOz3mNbc4r7jAwMo6Ty5LzSSIRsE37sYST72laXvfJiX4habn3Wd0iv 4zw2LbVh8XLp+huDvv3OOywQGNsDCTCD6fhS0X5UhBcrUeeBITBb8ewFIqPtKQpLtVKsDU1ep yLmxmPsZ+Y58iQcyk8+8XKN7LXssmImFCvVLOOVpqug+0OOT5AJ8ZSfxTiHoJ90E1icj0oZPd wTZQb72sSZU2r1eRxpC61XlxYC/J+df+Jcm1JdZ0cgg0M6uZex0W3B5BMKk/icmS+kSpe5syq +sFHFUWy5ZK2neh4CwFXu+bS2mb7YImlvsZm4+Za4r0PmtCw1qtfXi1xJm4M1Zj4MShN7tQOS sl3Zdl2eVCEjqEvMP1A3BmYaqIGgCFjB4FA4IlgWUU2aXfGZcUycEsTxVb/0ArfVX/7jX0W1S 72LV0Jxkk4vCNkJjJJvP8PAnLvE/MOWydS4X3wSnMoM3w30Fnn/5MPZ0QGWFppwqCtghWMav7 wr2xnvVI4TymV1Z4o6LXxTS+HH4m1gOOafWqgi5FxWFk9PiN6nhDDpDpb7XNy8sCyG0Mpq1aR dPsKUqtoMwPDO3ukaSRcIAqlr1TD8KvGCxN8RVywW/2vivIIQ8HXpqBCQ6TOgTz2Lyr1UbnIY scxllpJA2+dCvZsG Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 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