Re: [PATCH 2/4] mtd: rawnand: fsmc: Force to use 8 bits access when expected

From: Miquel Raynal
Date: Fri Nov 12 2021 - 10:39:28 EST


Hi Hervé,

herve.codina@xxxxxxxxxxx wrote on Fri, 12 Nov 2021 15:38:53 +0100:

> Some data transfers are expected on 8 bits by the nand core.
> The fsmc driver did not check this constraint and these transfers
> can be done on 32 bits depending on buffer alignment and transfers
> data size.
>
> This patch ensures that these transfers will be 8bits transfers in
> all cases.

I believe there is a misunderstanding here: NAND buses -between the
NAND controller and the NAND chip- are either 8-bit or 16-bit wide and
the amount of bytes that you will retrieve per register read is not
related to it.

When the controller supports 16-bit accesses, there are certain
operations that must be performed using only the lowest 8 bits of the
NAND bus, such as reading a status [1]. In this case, the controller
must have a way to disable the 16-bit mode temporarily. See [2] and [3]
for an example. Reading with readb() or readl() will IMHO not impact the
amount of data lines used for the operation.

> Signed-off-by: Herve Codina <herve.codina@xxxxxxxxxxx>

[1] https://elixir.bootlin.com/linux/latest/source/drivers/mtd/nand/raw/nand_base.c#L673
[2] Marvell NAND controller can change the used width of the bus
https://elixir.bootlin.com/linux/latest/source/drivers/mtd/nand/raw/marvell_nand.c#L1777
[3] ... while still doing 32-bit accesses
https://elixir.bootlin.com/linux/latest/source/drivers/mtd/nand/raw/marvell_nand.c#L906

Thanks,
Miquèl

> ---
> drivers/mtd/nand/raw/fsmc_nand.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/fsmc_nand.c b/drivers/mtd/nand/raw/fsmc_nand.c
> index 658f0cbe7ce8..7f057cfee6c4 100644
> --- a/drivers/mtd/nand/raw/fsmc_nand.c
> +++ b/drivers/mtd/nand/raw/fsmc_nand.c
> @@ -540,12 +540,12 @@ static int dma_xfer(struct fsmc_nand_data *host, void *buffer, int len,
> * @len: number of bytes to write
> */
> static void fsmc_write_buf(struct fsmc_nand_data *host, const u8 *buf,
> - int len)
> + int len, bool force_8bit)
> {
> int i;
>
> if (IS_ALIGNED((uintptr_t)buf, sizeof(u32)) &&
> - IS_ALIGNED(len, sizeof(u32))) {
> + IS_ALIGNED(len, sizeof(u32)) && !force_8bit) {
> u32 *p = (u32 *)buf;
>
> len = len >> 2;
> @@ -563,12 +563,13 @@ static void fsmc_write_buf(struct fsmc_nand_data *host, const u8 *buf,
> * @buf: buffer to store date
> * @len: number of bytes to read
> */
> -static void fsmc_read_buf(struct fsmc_nand_data *host, u8 *buf, int len)
> +static void fsmc_read_buf(struct fsmc_nand_data *host, u8 *buf, int len,
> + bool force_8bit)
> {
> int i;
>
> if (IS_ALIGNED((uintptr_t)buf, sizeof(u32)) &&
> - IS_ALIGNED(len, sizeof(u32))) {
> + IS_ALIGNED(len, sizeof(u32)) && !force_8bit) {
> u32 *p = (u32 *)buf;
>
> len = len >> 2;
> @@ -646,7 +647,8 @@ static int fsmc_exec_op(struct nand_chip *chip, const struct nand_operation *op,
> instr->ctx.data.len);
> else
> fsmc_read_buf(host, instr->ctx.data.buf.in,
> - instr->ctx.data.len);
> + instr->ctx.data.len,
> + instr->ctx.data.force_8bit);
> break;
>
> case NAND_OP_DATA_OUT_INSTR:
> @@ -656,7 +658,8 @@ static int fsmc_exec_op(struct nand_chip *chip, const struct nand_operation *op,
> instr->ctx.data.len);
> else
> fsmc_write_buf(host, instr->ctx.data.buf.out,
> - instr->ctx.data.len);
> + instr->ctx.data.len,
> + instr->ctx.data.force_8bit);
> break;
>
> case NAND_OP_WAITRDY_INSTR: