Subject: [PATCH v6 0/3] Add support for Loongson-1 NAND

Add the driver and dt-binding document for Loongson-1 NAND.
And modify nand_read_subpage() to allow subpage read by a single operation.

Changes in v6:
- Amend Kconfig
- Add the dt-binding document
- Modify nand_read_subpage() to allow subpage read by a single operation
- Add DT support for driver
- Use DT data instead of platform data
- Remove MAX_ID_SIZE
- Remove case NAND_OP_CMD_INSTR in ls1x_nand_set_controller()
- Move ECC configuration to ls1x_nand_attach_chip()
- Rename variable "nand" to "ls1x"
- Rename variable "nc" to "nfc"
- Some minor fixes
- Link to v5: https://lore.kernel.org/all/[email protected]

Changes in v5:
- Update the driver to fit the raw NAND framework.
- Implement exec_op() instead of legacy cmdfunc().
- Use dma_request_chan() instead of dma_request_channel().
- Some minor fixes and cleanups.

Changes in v4:
- Retrieve the controller from nand_hw_control.

Changes in v3:
- Replace __raw_readl/__raw_writel with readl/writel.
- Split ls1x_nand into two structures:
ls1x_nand_chip and ls1x_nand_controller.

Changes in v2:
- Modify the dependency in Kconfig due to the changes of DMA module.

Signed-off-by: Keguang Zhang <[email protected]>
---
Keguang Zhang (3):
dt-bindings: mtd: Add Loongson-1 NAND Controller
mtd: rawnand: Enable monolithic read when reading subpages
mtd: rawnand: Add Loongson-1 NAND Controller driver

.../devicetree/bindings/mtd/loongson,ls1x-nfc.yaml | 66 ++
drivers/mtd/nand/raw/Kconfig | 7 +
drivers/mtd/nand/raw/Makefile | 1 +
drivers/mtd/nand/raw/loongson1_nand.c | 748 +++++++++++++++++++++
drivers/mtd/nand/raw/nand_base.c | 5 +-
include/linux/mtd/rawnand.h | 5 +
6 files changed, 830 insertions(+), 2 deletions(-)
---
base-commit: 084c8e315db34b59d38d06e684b1a0dd07d30287
change-id: 20240316-loongson1-nand-98327d77e0f6

Best regards,
--
Keguang Zhang <[email protected]>




Subject: [PATCH v6 1/3] dt-bindings: mtd: Add Loongson-1 NAND Controller

From: Keguang Zhang <[email protected]>

Add devicetree binding document for Loongson-1 NAND Controller.

Signed-off-by: Keguang Zhang <[email protected]>
---
Changes in v6:
- A newly added patch
---
.../devicetree/bindings/mtd/loongson,ls1x-nfc.yaml | 66 ++++++++++++++++++++++
1 file changed, 66 insertions(+)

diff --git a/Documentation/devicetree/bindings/mtd/loongson,ls1x-nfc.yaml b/Documentation/devicetree/bindings/mtd/loongson,ls1x-nfc.yaml
new file mode 100644
index 000000000000..2494c7b3b506
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/loongson,ls1x-nfc.yaml
@@ -0,0 +1,66 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mtd/loongson,ls1x-nfc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Loongson-1 NAND Controller
+
+maintainers:
+ - Keguang Zhang <[email protected]>
+
+allOf:
+ - $ref: nand-controller.yaml
+
+properties:
+ compatible:
+ oneOf:
+ - const: loongson,ls1b-nfc
+ - items:
+ - enum:
+ - loongson,ls1a-nfc
+ - loongson,ls1c-nfc
+ - const: loongson,ls1b-nfc
+
+ reg:
+ maxItems: 1
+
+ dmas:
+ maxItems: 1
+
+ dma-names:
+ const: rxtx
+
+patternProperties:
+ "^nand@[0-3]$":
+ type: object
+ $ref: raw-nand-chip.yaml
+
+ unevaluatedProperties: false
+
+required:
+ - compatible
+ - reg
+ - dmas
+ - dma-names
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ nand-controller@1fe78000 {
+ compatible = "loongson,ls1b-nfc";
+ reg = <0x1fe78000 0x40>;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ dmas = <&dma 0>;
+ dma-names = "rxtx";
+
+ nand@0 {
+ reg = <0>;
+ nand-use-soft-ecc-engine;
+ nand-ecc-algo = "hamming";
+ };
+ };

--
2.40.1



Subject: [PATCH v6 2/3] mtd: rawnand: Enable monolithic read when reading subpages

From: Keguang Zhang <[email protected]>

nand_read_subpage() reads data and ECC data by two separate
operations.
This patch allows the NAND controllers who support
monolithic page read to do subpage read by a single operation,
which is more effective than nand_read_subpage().
---
Changes in v6:
- A newly added patch

Signed-off-by: Keguang Zhang <[email protected]>
---
drivers/mtd/nand/raw/nand_base.c | 5 +++--
include/linux/mtd/rawnand.h | 5 +++++
2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index d7dbbd469b89..eeb654c6b4fc 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -3630,7 +3630,7 @@ static int nand_do_read_ops(struct nand_chip *chip, loff_t from,
oob_required,
page);
else if (!aligned && NAND_HAS_SUBPAGE_READ(chip) &&
- !oob)
+ !NAND_HAS_MONOLITHIC_READ(chip) && !oob)
ret = chip->ecc.read_subpage(chip, col, bytes,
bufpoi, page);
else
@@ -3648,7 +3648,8 @@ static int nand_do_read_ops(struct nand_chip *chip, loff_t from,
* partial pages or when a bounce buffer is required.
*/
if (use_bounce_buf) {
- if (!NAND_HAS_SUBPAGE_READ(chip) && !oob &&
+ if ((!NAND_HAS_SUBPAGE_READ(chip) ||
+ NAND_HAS_MONOLITHIC_READ(chip)) && !oob &&
!(mtd->ecc_stats.failed - ecc_stats.failed) &&
(ops->mode != MTD_OPS_RAW)) {
chip->pagecache.page = realpage;
diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
index e84522e31301..92d3ab491c9c 100644
--- a/include/linux/mtd/rawnand.h
+++ b/include/linux/mtd/rawnand.h
@@ -150,6 +150,11 @@ struct gpio_desc;
/* Device needs 3rd row address cycle */
#define NAND_ROW_ADDR_3 BIT(14)

+/* Device supports monolithic reads */
+#define NAND_MONOLITHIC_READ BIT(15)
+/* Macros to identify the above */
+#define NAND_HAS_MONOLITHIC_READ(chip) ((chip->options & NAND_MONOLITHIC_READ))
+
/* Non chip related options */
/* This option skips the bbt scan during initialization. */
#define NAND_SKIP_BBTSCAN BIT(16)

--
2.40.1



2024-03-27 16:23:39

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v6 1/3] dt-bindings: mtd: Add Loongson-1 NAND Controller

On Wed, Mar 27, 2024 at 06:43:59PM +0800, Keguang Zhang via B4 Relay wrote:
> From: Keguang Zhang <[email protected]>
>
> Add devicetree binding document for Loongson-1 NAND Controller.
>
> Signed-off-by: Keguang Zhang <[email protected]>
> ---
> Changes in v6:
> - A newly added patch
> ---
> .../devicetree/bindings/mtd/loongson,ls1x-nfc.yaml | 66 ++++++++++++++++++++++
> 1 file changed, 66 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mtd/loongson,ls1x-nfc.yaml b/Documentation/devicetree/bindings/mtd/loongson,ls1x-nfc.yaml
> new file mode 100644
> index 000000000000..2494c7b3b506
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/loongson,ls1x-nfc.yaml
> @@ -0,0 +1,66 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mtd/loongson,ls1x-nfc.yaml#

Please make the filename match the compatible.

> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Loongson-1 NAND Controller
> +
> +maintainers:
> + - Keguang Zhang <[email protected]>
> +
> +allOf:
> + - $ref: nand-controller.yaml
> +
> +properties:
> + compatible:
> + oneOf:
> + - const: loongson,ls1b-nfc
> + - items:
> + - enum:
> + - loongson,ls1a-nfc
> + - loongson,ls1c-nfc
> + - const: loongson,ls1b-nfc
> +
> + reg:
> + maxItems: 1
> +
> + dmas:
> + maxItems: 1
> +
> + dma-names:
> + const: rxtx

If you only have one dma, why do you need a dma-names entry for it?

Looks fine to me otherwise though,
COnor.

> +
> +patternProperties:
> + "^nand@[0-3]$":
> + type: object
> + $ref: raw-nand-chip.yaml
> +
> + unevaluatedProperties: false
> +
> +required:
> + - compatible
> + - reg
> + - dmas
> + - dma-names
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + nand-controller@1fe78000 {
> + compatible = "loongson,ls1b-nfc";
> + reg = <0x1fe78000 0x40>;
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + dmas = <&dma 0>;
> + dma-names = "rxtx";
> +
> + nand@0 {
> + reg = <0>;
> + nand-use-soft-ecc-engine;
> + nand-ecc-algo = "hamming";
> + };
> + };
>
> --
> 2.40.1
>
>


Attachments:
(No filename) (2.34 kB)
signature.asc (235.00 B)
Download all attachments

2024-03-28 08:55:48

by Keguang Zhang

[permalink] [raw]
Subject: Re: [PATCH v6 1/3] dt-bindings: mtd: Add Loongson-1 NAND Controller

On Thu, Mar 28, 2024 at 12:23 AM Conor Dooley <[email protected]> wrote:
>
> On Wed, Mar 27, 2024 at 06:43:59PM +0800, Keguang Zhang via B4 Relay wrote:
> > From: Keguang Zhang <[email protected]>
> >
> > Add devicetree binding document for Loongson-1 NAND Controller.
> >
> > Signed-off-by: Keguang Zhang <[email protected]>
> > ---
> > Changes in v6:
> > - A newly added patch
> > ---
> > .../devicetree/bindings/mtd/loongson,ls1x-nfc.yaml | 66 ++++++++++++++++++++++
> > 1 file changed, 66 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/mtd/loongson,ls1x-nfc.yaml b/Documentation/devicetree/bindings/mtd/loongson,ls1x-nfc.yaml
> > new file mode 100644
> > index 000000000000..2494c7b3b506
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mtd/loongson,ls1x-nfc.yaml
> > @@ -0,0 +1,66 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/mtd/loongson,ls1x-nfc.yaml#
>
> Please make the filename match the compatible.
>
Got it. I'll rename it to loongson,ls1-nfc.yaml and change the
compatible as follows.
compatible:
items:
- enum:
- loongson,ls1a-nfc
- loongson,ls1b-nfc
- loongson,ls1c-nfc
- const: loongson,ls1-nfc

> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Loongson-1 NAND Controller
> > +
> > +maintainers:
> > + - Keguang Zhang <[email protected]>
> > +
> > +allOf:
> > + - $ref: nand-controller.yaml
> > +
> > +properties:
> > + compatible:
> > + oneOf:
> > + - const: loongson,ls1b-nfc
> > + - items:
> > + - enum:
> > + - loongson,ls1a-nfc
> > + - loongson,ls1c-nfc
> > + - const: loongson,ls1b-nfc
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + dmas:
> > + maxItems: 1
> > +
> > + dma-names:
> > + const: rxtx
>
> If you only have one dma, why do you need a dma-names entry for it?
>
Without "dma-names", the following error will come out when doing
dt_binding_check.
nand-controller@1fe78000: 'dma-names' is a required property

In addition, loongson1_nand.c calls dma_request_chan(). Then
dma_request_chan() calls of_dma_request_slave_channel(), in which the
'dma-names' is necessary.
struct dma_chan *of_dma_request_slave_channel(struct device_node *np,
const char *name)
{
...
count = of_property_count_strings(np, "dma-names");
if (count < 0) {
pr_err("%s: dma-names property of node '%pOF' missing
or empty\n",
__func__, np);
return ERR_PTR(-ENODEV);
}
...
}

Thanks for your review!

> Looks fine to me otherwise though,
> COnor.
>
> > +
> > +patternProperties:
> > + "^nand@[0-3]$":
> > + type: object
> > + $ref: raw-nand-chip.yaml
> > +
> > + unevaluatedProperties: false
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - dmas
> > + - dma-names
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > + - |
> > + nand-controller@1fe78000 {
> > + compatible = "loongson,ls1b-nfc";
> > + reg = <0x1fe78000 0x40>;
> > +
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + dmas = <&dma 0>;
> > + dma-names = "rxtx";
> > +
> > + nand@0 {
> > + reg = <0>;
> > + nand-use-soft-ecc-engine;
> > + nand-ecc-algo = "hamming";
> > + };
> > + };
> >
> > --
> > 2.40.1
> >
> >



--
Best regards,

Keguang Zhang

2024-03-28 09:38:02

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v6 1/3] dt-bindings: mtd: Add Loongson-1 NAND Controller

On Thu, Mar 28, 2024 at 04:54:59PM +0800, Keguang Zhang wrote:
> On Thu, Mar 28, 2024 at 12:23 AM Conor Dooley <[email protected]> wrote:
> >
> > On Wed, Mar 27, 2024 at 06:43:59PM +0800, Keguang Zhang via B4 Relay wrote:
> > > From: Keguang Zhang <[email protected]>
> > >
> > > Add devicetree binding document for Loongson-1 NAND Controller.
> > >
> > > Signed-off-by: Keguang Zhang <[email protected]>
> > > ---
> > > Changes in v6:
> > > - A newly added patch
> > > ---
> > > .../devicetree/bindings/mtd/loongson,ls1x-nfc.yaml | 66 ++++++++++++++++++++++
> > > 1 file changed, 66 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/mtd/loongson,ls1x-nfc.yaml b/Documentation/devicetree/bindings/mtd/loongson,ls1x-nfc.yaml
> > > new file mode 100644
> > > index 000000000000..2494c7b3b506
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/mtd/loongson,ls1x-nfc.yaml
> > > @@ -0,0 +1,66 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/mtd/loongson,ls1x-nfc.yaml#
> >
> > Please make the filename match the compatible.
> >
> Got it. I'll rename it to loongson,ls1-nfc.yaml and change the
> compatible as follows.
> compatible:
> items:
> - enum:
> - loongson,ls1a-nfc
> - loongson,ls1b-nfc
> - loongson,ls1c-nfc
> - const: loongson,ls1-nfc

No, please do not do this. Just call the file "ls1b-nfc".

Thanks,
Conor.


Attachments:
(No filename) (1.51 kB)
signature.asc (235.00 B)
Download all attachments