From: Wolfram Sang <wsa+renesas@sang-engineering.com>
To: linux-renesas-soc@vger.kernel.org
Cc: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
Mark Brown <broonie@kernel.org>,
Sergey Shtylyov <s.shtylyov@omp.ru>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] memory: renesas-rpc-if: Remove redundant division of dummy
Date: Fri, 13 Jan 2023 09:10:12 +0100 [thread overview]
Message-ID: <Y8ER5B5D1XCTOOBX@ninjato> (raw)
In-Reply-To: <20230112090655.43367-1-wsa+renesas@sang-engineering.com>
[-- Attachment #1: Type: text/plain, Size: 2979 bytes --]
Using Sergei's recent email...
On Thu, Jan 12, 2023 at 10:06:55AM +0100, Wolfram Sang wrote:
> From: Cong Dang <cong.dang.xn@renesas.com>
>
> The dummy cycles value was wrongly calculated if dummy.buswidth > 1,
> which affects QSPI, OSPI, HyperFlash on various SoCs. We're lucky in
> Single SPI case since its dummy.buswidth equals to 1, so the result of
> the division is unchanged
>
> This issue can be reproduced using something like the following commands
> A. QSPI mode: Mount device with jffs2 format
> jffs2: CLEANMARKER node found at 0x00000004, not first node in block (0x00000000)
>
> B. QSPI mode: Write data to mtd10, where mtd10 is a parition on SPI Flash
> storage, defined properly in a device tree
>
> [Correct fragment, read from SPI Flash]
>
> root@v3x:~# echo "hello" > /dev/mtd10
> root@v3x:~# hexdump -C -n100 /dev/mtd10
> 00000000 68 65 6c 6c 6f 0a ff ff ff ff ff ff ff ff ff ff |hello...........|
> 00000010 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff |................|
>
> [Incorrect read of the same fragment: see the difference at offsets 0-3]
>
> root@v3x:~# echo "hello" > /dev/mtd10
> root@v3x:~# hexdump -C -n100 /dev/mtd10
> 00000000 00 00 00 00 68 65 6c 6c 6f 0a ff ff ff ff ff ff |....hello.......|
> 00000010 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff |................|
>
> As seen from the result, 4 NULL bytes were inserted before the test data.
> Wrong calculation in rpcif_prepare() led to miss of some dummy cycle. A
> division by bus width is redundant because it had been performed already
> in spi-rpc-if.c::rpcif_spi_mem_prepare()
>
> Fix this by removing the redundant division.
>
> Fixes: ca7d8b980b67 ("memory: add Renesas RPC-IF driver")
> Signed-off-by: Cong Dang <cong.dang.xn@renesas.com>
> Signed-off-by: Hai Pham <hai.pham.ud@renesas.com>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
>
> Sadly, I cannot test this patch myself because I don't have access to
> hardware which uses a buswidth > 1 for the dummy read. However, from
> code review, this patch makes sense. The division by buswidth is done
> twice, once in the SPI driver and once in the RPC core. It should stay
> only in the SPI driver.
>
> drivers/memory/renesas-rpc-if.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/memory/renesas-rpc-if.c b/drivers/memory/renesas-rpc-if.c
> index 09cd4318a83d..c36b407851ff 100644
> --- a/drivers/memory/renesas-rpc-if.c
> +++ b/drivers/memory/renesas-rpc-if.c
> @@ -430,8 +430,7 @@ void rpcif_prepare(struct rpcif *rpc, const struct rpcif_op *op, u64 *offs,
>
> if (op->dummy.buswidth) {
> rpc->enable |= RPCIF_SMENR_DME;
> - rpc->dummy = RPCIF_SMDMCR_DMCYC(op->dummy.ncycles /
> - op->dummy.buswidth);
> + rpc->dummy = RPCIF_SMDMCR_DMCYC(op->dummy.ncycles);
> }
>
> if (op->option.buswidth) {
> --
> 2.30.2
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
next prev parent reply other threads:[~2023-01-13 8:12 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-12 9:06 [PATCH] memory: renesas-rpc-if: Remove redundant division of dummy Wolfram Sang
2023-01-13 8:10 ` Wolfram Sang [this message]
2023-01-20 17:52 ` Krzysztof Kozlowski
2023-01-26 10:15 ` Krzysztof Kozlowski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Y8ER5B5D1XCTOOBX@ninjato \
--to=wsa+renesas@sang-engineering.com \
--cc=broonie@kernel.org \
--cc=krzysztof.kozlowski@linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-renesas-soc@vger.kernel.org \
--cc=s.shtylyov@omp.ru \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).