From: Huacai Chen <chenhuacai@kernel.org>
To: Binbin Zhou <zhoubb.aaron@gmail.com>
Cc: Binbin Zhou <zhoubinbin@loongson.cn>,
Huacai Chen <chenhuacai@loongson.cn>,
Rob Herring <robh+dt@kernel.org>,
Krzysztof Kozlowski <krzk+dt@kernel.org>,
Conor Dooley <conor+dt@kernel.org>,
Keguang Zhang <keguang.zhang@gmail.com>,
Miquel Raynal <miquel.raynal@bootlin.com>,
Richard Weinberger <richard@nod.at>,
Vignesh Raghavendra <vigneshr@ti.com>,
Xuerui Wang <kernel@xen0n.name>,
loongarch@lists.linux.dev, devicetree@vger.kernel.org,
linux-mtd@lists.infradead.org
Subject: Re: [PATCH v2 4/8] mtd: rawnand: loongson: Add nand chip select support
Date: Mon, 11 Aug 2025 16:59:31 +0800 [thread overview]
Message-ID: <CAAhV-H6JyGdg8hA2vsrPGFAysSk3F9KJqmS1A2Gqwx52Q2x2_A@mail.gmail.com> (raw)
In-Reply-To: <CAMpQs4+5CoXKBVqE+YPYPyMyZFbhXQLfqqqex3JCVusDS5AUTA@mail.gmail.com>
On Mon, Aug 11, 2025 at 4:49 PM Binbin Zhou <zhoubb.aaron@gmail.com> wrote:
>
> Hi:
>
> On Mon, Aug 11, 2025 at 2:40 PM Huacai Chen <chenhuacai@kernel.org> wrote:
> >
> > Hi, Binbin,
> >
> > On Mon, Aug 11, 2025 at 2:03 PM Binbin Zhou <zhoubinbin@loongson.cn> wrote:
> > >
> > > The page address register describes the page address of the starting
> > > address for NAND read/write/erase operations.
> > >
> > > According to the manual, it consists of two parts:
> > > {chip select, page number}
> > >
> > > The `chip select` is fixed at 2 bits, and the `page number` is
> > > determined based on the actual capacity of the single-chip memory.
> > > Therefore we need to determine the `chip select` bits base on the `page
> > > number`.
> > >
> > > For example, for a 1GB capacity chip (2K page size), it has 1M pages.
> > > Thus, [19:0] is used to represent the page number, and [21:20]
> > > represents the chip select.
> > >
> > > Signed-off-by: Binbin Zhou <zhoubinbin@loongson.cn>
> > > ---
> > > .../mtd/nand/raw/loongson-nand-controller.c | 133 ++++++++++++++----
> > > 1 file changed, 103 insertions(+), 30 deletions(-)
> > >
> > > diff --git a/drivers/mtd/nand/raw/loongson-nand-controller.c b/drivers/mtd/nand/raw/loongson-nand-controller.c
> > > index 97cd566420a8..5a51c7d299cc 100644
> > > --- a/drivers/mtd/nand/raw/loongson-nand-controller.c
> > > +++ b/drivers/mtd/nand/raw/loongson-nand-controller.c
> > > @@ -82,6 +82,7 @@ struct loongson_nand_data {
> > > unsigned int op_scope_field;
> > > unsigned int hold_cycle;
> > > unsigned int wait_cycle;
> > > + unsigned int nand_cs;
> > > void (*set_addr)(struct loongson_nand_host *host, struct loongson_nand_op *op);
> > > };
> > >
> > > @@ -90,6 +91,7 @@ struct loongson_nand_host {
> > > struct nand_chip chip;
> > > struct nand_controller controller;
> > > const struct loongson_nand_data *data;
> > > + unsigned int addr_cs_field;
> > > void __iomem *reg_base;
> > > struct regmap *regmap;
> > > /* DMA Engine stuff */
> > > @@ -215,6 +217,26 @@ static int loongson_nand_parse_instructions(struct nand_chip *chip, const struct
> > > return 0;
> > > }
> > >
> > > +static void loongson_nand_set_addr_cs(struct loongson_nand_host *host)
> > > +{
> > > + struct nand_chip *chip = &host->chip;
> > > + struct mtd_info *mtd = nand_to_mtd(chip);
> > > +
> > > + if (!host->data->nand_cs)
> > > + return;
> > > +
> > > + /*
> > > + * The Manufacturer/Chip ID read operation precedes attach_chip, at which point
> > > + * information such as NAND chip selection and capacity is unknown. As a
> > > + * workaround, we use 128MB cellsize (2KB pagesize) as a fallback.
> > > + */
> > > + if (!mtd->writesize)
> > > + host->addr_cs_field = GENMASK(17, 16);
> > > +
> > > + regmap_update_bits(host->regmap, LOONGSON_NAND_ADDR2, host->addr_cs_field,
> > > + host->data->nand_cs << __ffs(host->addr_cs_field));
> > > +}
> > > +
> > > static void ls1b_nand_set_addr(struct loongson_nand_host *host, struct loongson_nand_op *op)
> > > {
> > > struct nand_chip *chip = &host->chip;
> > > @@ -263,6 +285,8 @@ static void ls1c_nand_set_addr(struct loongson_nand_host *host, struct loongson_
> > > regmap_update_bits(host->regmap, LOONGSON_NAND_ADDR2, mask, val);
> > > }
> > > }
> > > +
> > > + loongson_nand_set_addr_cs(host);
> > > }
> > >
> > > static void loongson_nand_trigger_op(struct loongson_nand_host *host, struct loongson_nand_op *op)
> > > @@ -603,42 +627,89 @@ static int loongson_nand_exec_op(struct nand_chip *chip, const struct nand_opera
> > > return nand_op_parser_exec_op(chip, &loongson_nand_op_parser, op, check_only);
> > > }
> > >
> > > -static int loongson_nand_attach_chip(struct nand_chip *chip)
> > > +static int loongson_nand_get_chip_capacity(struct nand_chip *chip)
> > > {
> > > struct loongson_nand_host *host = nand_get_controller_data(chip);
> > > u64 chipsize = nanddev_target_size(&chip->base);
> > > - int cell_size = 0;
> > > + struct mtd_info *mtd = nand_to_mtd(chip);
> > >
> > > - switch (chipsize) {
> > > - case SZ_128M:
> > > - cell_size = 0x0;
> > > - break;
> > > - case SZ_256M:
> > > - cell_size = 0x1;
> > > - break;
> > > - case SZ_512M:
> > > - cell_size = 0x2;
> > > - break;
> > > - case SZ_1G:
> > > - cell_size = 0x3;
> > > - break;
> > > - case SZ_2G:
> > > - cell_size = 0x4;
> > > - break;
> > > - case SZ_4G:
> > > - cell_size = 0x5;
> > > - break;
> > > - case SZ_8G:
> > > - cell_size = 0x6;
> > > - break;
> > > - case SZ_16G:
> > > - cell_size = 0x7;
> > > - break;
> > > - default:
> > > - dev_err(host->dev, "unsupported chip size: %llu MB\n", chipsize);
> > > - return -EINVAL;
> > > + if (mtd->writesize == SZ_4K && chipsize == SZ_2G) {
> > > + host->addr_cs_field = GENMASK(20, 19);
> > > + return 0x4;
> > > }
> > I think you want to sort this part by the return value, but "return
> > 0x4" has already break it.
> >
> > So I think it is better to use a big switch-case for writesize, and
> > sort the big switch-case by writesize.
>
> How about:
>
> switch (mtd->writesize) {
> case SZ_512:
> switch (chipsize) {
> case SZ_8M:
> host->addr_cs_field = GENMASK(15, 14);
> return 0x9;
> case SZ_16M:
> host->addr_cs_field = GENMASK(16, 15);
> return 0xa;
> case SZ_32M:
> host->addr_cs_field = GENMASK(17, 16);
> return 0xb;
> case SZ_64M:
> host->addr_cs_field = GENMASK(18, 17);
> return 0xc;
> case SZ_128M:
> host->addr_cs_field = GENMASK(19, 18);
> return 0xd;
> }
> break;
> case SZ_2K:
> switch (chipsize) {
> case SZ_128M:
> host->addr_cs_field = GENMASK(17, 16);
> return 0;
Use 0x0 for consistency.
> case SZ_256M:
> host->addr_cs_field = GENMASK(18, 17);
> return 0x1;
> case SZ_512M:
> host->addr_cs_field = GENMASK(19, 18);
> return 0x2;
> case SZ_1G:
> host->addr_cs_field = GENMASK(20, 19);
> return 0x3;
> }
> break;
> case SZ_4K:
> if (chipsize == SZ_2G) {
> host->addr_cs_field = GENMASK(20, 19);
> return 0x4;
> }
> break;
> case SZ_8K:
> switch (chipsize) {
> case SZ_4G:
> host->addr_cs_field = GENMASK(20, 19);
> return 0x5;
> case SZ_8G:
> host->addr_cs_field = GENMASK(21, 20);
> return 0x6;
> case SZ_16G:
> host->addr_cs_field = GENMASK(22, 21);
> return 0x7;
> }
> break;
> default:
> break;
The default branch is unneeded. Others are OK for me.
Huacai
> }
>
> dev_err(host->dev, "Unsupported chip size: %llu MB with page
> size %u B\n",
> chipsize, mtd->writesize);
> >
> > Huacai
> >
> > >
> > > + if (mtd->writesize == SZ_2K) {
> > > + switch (chipsize) {
> > > + case SZ_128M:
> > > + host->addr_cs_field = GENMASK(17, 16);
> > > + return 0;
> > > + case SZ_256M:
> > > + host->addr_cs_field = GENMASK(18, 17);
> > > + return 0x1;
> > > + case SZ_512M:
> > > + host->addr_cs_field = GENMASK(19, 18);
> > > + return 0x2;
> > > + case SZ_1G:
> > > + host->addr_cs_field = GENMASK(20, 19);
> > > + return 0x3;
> > > + default:
> > > + goto err;
> > > + }
> > > + }
> > > +
> > > + if (mtd->writesize == SZ_8K) {
> > > + switch (chipsize) {
> > > + case SZ_4G:
> > > + host->addr_cs_field = GENMASK(20, 19);
> > > + return 0x5;
> > > + case SZ_8G:
> > > + host->addr_cs_field = GENMASK(21, 20);
> > > + return 0x6;
> > > + case SZ_16G:
> > > + host->addr_cs_field = GENMASK(22, 21);
> > > + return 0x7;
> > > + default:
> > > + goto err;
> > > + }
> > > + }
> > > +
> > > + if (mtd->writesize == SZ_512) {
> > > + switch (chipsize) {
> > > + case SZ_8M:
> > > + host->addr_cs_field = GENMASK(15, 14);
> > > + return 0x9;
> > > + case SZ_16M:
> > > + host->addr_cs_field = GENMASK(16, 15);
> > > + return 0xa;
> > > + case SZ_32M:
> > > + host->addr_cs_field = GENMASK(17, 16);
> > > + return 0xb;
> > > + case SZ_64M:
> > > + host->addr_cs_field = GENMASK(18, 17);
> > > + return 0xc;
> > > + case SZ_128M:
> > > + host->addr_cs_field = GENMASK(19, 18);
> > > + return 0xd;
> > > + default:
> > > + goto err;
> > > + }
> > > + }
> > > +
> > > +err:
> > > + dev_err(host->dev, "Unsupported chip size: %llu MB with page size %u B\n",
> > > + chipsize, mtd->writesize);
> > > + return -EINVAL;
> > > +}
> > > +
> > > +static int loongson_nand_attach_chip(struct nand_chip *chip)
> > > +{
> > > + struct loongson_nand_host *host = nand_get_controller_data(chip);
> > > + int cell_size = 0;
> > > +
> > > + cell_size = loongson_nand_get_chip_capacity(chip);
> > > + if (cell_size < 0)
> > > + return cell_size;
> > > +
> > > switch (chip->ecc.engine_type) {
> > > case NAND_ECC_ENGINE_TYPE_NONE:
> > > break;
> > > @@ -818,6 +889,7 @@ static const struct loongson_nand_data ls1b_nand_data = {
> > > .status_field = GENMASK(15, 8),
> > > .hold_cycle = 0x2,
> > > .wait_cycle = 0xc,
> > > + .nand_cs = 0x0,
> > > .set_addr = ls1b_nand_set_addr,
> > > };
> > >
> > > @@ -828,6 +900,7 @@ static const struct loongson_nand_data ls1c_nand_data = {
> > > .op_scope_field = GENMASK(29, 16),
> > > .hold_cycle = 0x2,
> > > .wait_cycle = 0xc,
> > > + .nand_cs = 0x0,
> > > .set_addr = ls1c_nand_set_addr,
> > > };
> > >
> > > --
> > > 2.47.3
> > >
> > >
>
> --
> Thanks.
> Binbin
next prev parent reply other threads:[~2025-08-11 8:59 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-08-11 6:02 [PATCH v2 0/8] mtd: rawnand: loongson: Add Loongson-2K nand controller support Binbin Zhou
2025-08-11 6:02 ` [PATCH v2 1/8] mtd: rawnand: loongson1: Rename the prefix from ls1x to loongson Binbin Zhou
2025-08-11 6:02 ` [PATCH v2 2/8] mtd: rawnand: loongson: Rename loongson1 " Binbin Zhou
2025-08-11 6:36 ` Huacai Chen
2025-08-11 9:41 ` Binbin Zhou
2025-08-18 7:04 ` Keguang Zhang
2025-08-18 9:40 ` Binbin Zhou
2025-08-11 6:02 ` [PATCH v2 3/8] mtd: rawnand: loongson: Add 6-byte NAND ID reading support Binbin Zhou
2025-08-11 6:37 ` Huacai Chen
2025-08-19 9:12 ` Keguang Zhang
2025-08-11 6:03 ` [PATCH v2 4/8] mtd: rawnand: loongson: Add nand chip select support Binbin Zhou
2025-08-11 6:40 ` Huacai Chen
2025-08-11 8:49 ` Binbin Zhou
2025-08-11 8:59 ` Huacai Chen [this message]
2025-08-19 9:27 ` Keguang Zhang
2025-08-11 6:03 ` [PATCH v2 5/8] dt-bindings: mtd: loongson,ls1b-nand-controller: Document the Loongson-2K0500 NAND controller Binbin Zhou
2025-08-11 7:32 ` Krzysztof Kozlowski
2025-08-11 7:42 ` Binbin Zhou
2025-08-11 8:13 ` Krzysztof Kozlowski
2025-08-11 8:19 ` Binbin Zhou
2025-08-11 6:03 ` [PATCH v2 6/8] mtd: rawnand: loongson: Add Loongson-2K0500 NAND controller support Binbin Zhou
2025-08-11 18:09 ` Yao Zi
2025-08-12 1:46 ` Binbin Zhou
2025-08-21 6:45 ` Keguang Zhang
2025-08-21 7:13 ` Binbin Zhou
2025-08-11 6:03 ` [PATCH v2 7/8] dt-bindings: mtd: loongson,ls1b-nand-controller: Document the Loongson-2K1000 NAND controller Binbin Zhou
2025-08-18 15:55 ` Rob Herring (Arm)
2025-08-11 6:03 ` [PATCH v2 8/8] mtd: rawnand: loongson: Add Loongson-2K1000 NAND controller support Binbin Zhou
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=CAAhV-H6JyGdg8hA2vsrPGFAysSk3F9KJqmS1A2Gqwx52Q2x2_A@mail.gmail.com \
--to=chenhuacai@kernel.org \
--cc=chenhuacai@loongson.cn \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=keguang.zhang@gmail.com \
--cc=kernel@xen0n.name \
--cc=krzk+dt@kernel.org \
--cc=linux-mtd@lists.infradead.org \
--cc=loongarch@lists.linux.dev \
--cc=miquel.raynal@bootlin.com \
--cc=richard@nod.at \
--cc=robh+dt@kernel.org \
--cc=vigneshr@ti.com \
--cc=zhoubb.aaron@gmail.com \
--cc=zhoubinbin@loongson.cn \
/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).