From: Miquel Raynal <miquel.raynal@bootlin.com> To: Herve Codina <herve.codina@bootlin.com> Cc: Richard Weinberger <richard@nod.at>, Vignesh Raghavendra <vigneshr@ti.com>, linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org, Thomas Petazzoni <thomas.petazzoni@bootlin.com> Subject: Re: [PATCH 2/4] mtd: rawnand: fsmc: Force to use 8 bits access when expected Date: Fri, 12 Nov 2021 16:38:59 +0100 [thread overview] Message-ID: <20211112163859.23a2487a@xps13> (raw) In-Reply-To: <20211112143855.2678989-3-herve.codina@bootlin.com> Hi Hervé, herve.codina@bootlin.com 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@bootlin.com> [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:
WARNING: multiple messages have this Message-ID (diff)
From: Miquel Raynal <miquel.raynal@bootlin.com> To: Herve Codina <herve.codina@bootlin.com> Cc: Richard Weinberger <richard@nod.at>, Vignesh Raghavendra <vigneshr@ti.com>, linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org, Thomas Petazzoni <thomas.petazzoni@bootlin.com> Subject: Re: [PATCH 2/4] mtd: rawnand: fsmc: Force to use 8 bits access when expected Date: Fri, 12 Nov 2021 16:38:59 +0100 [thread overview] Message-ID: <20211112163859.23a2487a@xps13> (raw) In-Reply-To: <20211112143855.2678989-3-herve.codina@bootlin.com> Hi Hervé, herve.codina@bootlin.com 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@bootlin.com> [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: ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
next prev parent reply other threads:[~2021-11-12 15:39 UTC|newest] Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-11-12 14:38 [PATCH 0/4] mtd: rawnand: Fixes nand infra delay setting and FSMC nand controller Herve Codina 2021-11-12 14:38 ` Herve Codina 2021-11-12 14:38 ` [PATCH 1/4] mtd: rawnand: Fix nand_erase_op delay Herve Codina 2021-11-12 14:38 ` Herve Codina 2021-11-12 14:38 ` [PATCH 2/4] mtd: rawnand: fsmc: Force to use 8 bits access when expected Herve Codina 2021-11-12 14:38 ` Herve Codina 2021-11-12 15:38 ` Miquel Raynal [this message] 2021-11-12 15:38 ` Miquel Raynal 2021-11-17 13:47 ` Herve Codina 2021-11-17 13:47 ` Herve Codina 2021-11-12 14:38 ` [PATCH 3/4] mtd: rawnand: fsmc: Take instruction delay into account Herve Codina 2021-11-12 14:38 ` Herve Codina 2021-11-12 14:38 ` [PATCH 4/4] mtd: rawnand: fsmc: Fix timing computation Herve Codina 2021-11-12 14:38 ` Herve Codina 2021-11-12 15:52 ` Miquel Raynal 2021-11-12 15:52 ` Miquel Raynal
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=20211112163859.23a2487a@xps13 \ --to=miquel.raynal@bootlin.com \ --cc=herve.codina@bootlin.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mtd@lists.infradead.org \ --cc=richard@nod.at \ --cc=thomas.petazzoni@bootlin.com \ --cc=vigneshr@ti.com \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.