2024-05-17 18:10:41

by Frank Li

[permalink] [raw]
Subject: [PATCH 0/5] mtd: nand: gpmi-nand: add imx8qxp gpmi nand support

Update binding doc to support imx8qxp NAND.
Add new compatible string "fsl,imx8qxp-gpmi-nand".
Update dts for imx8qxp and imx8dxl

Run dt_binding_check: fsl,mxs-dma.yaml
SCHEMA Documentation/devicetree/bindings/processed-schema.json
DTEX Documentation/devicetree/bindings/dma/fsl,mxs-dma.example.dts
DTC_CHK Documentation/devicetree/bindings/dma/fsl,mxs-dma.example.dtb
Run dt_binding_check: gpmi-nand.yaml
SCHEMA Documentation/devicetree/bindings/processed-schema.json
DTEX Documentation/devicetree/bindings/mtd/gpmi-nand.example.dts
DTC_CHK Documentation/devicetree/bindings/mtd/gpmi-nand.example.dtb

No warning:

make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- -j8 CHECK_DTBS=y freescale/imx8dxl-evk.dtb
SYNC include/config/auto.conf.cmd
UPD include/config/kernel.release
DTC_CHK arch/arm64/boot/dts/freescale/imx8dxl-evk.dtb

Signed-off-by: Frank Li <[email protected]>
---
Frank Li (4):
dt-bindings: mtd: gpmi-nand: Add 'fsl,imx8qxp-gpmi-nand' compatible string
dt-bindings: dma: fsl-mxs-dma: Add compatible string "fsl,imx8qxp-dma-apbh"
arm64: dts: imx8-ss-conn: add gpmi nand node
arm64: dts: imx8dxl-ss-conn: add gpmi nand

Han Xu (1):
mtd: rawnand: gpmi: add iMX8QXP support.

.../devicetree/bindings/dma/fsl,mxs-dma.yaml | 15 +++++
.../devicetree/bindings/mtd/gpmi-nand.yaml | 22 +++++++
arch/arm64/boot/dts/freescale/imx8-ss-conn.dtsi | 69 ++++++++++++++++++++++
arch/arm64/boot/dts/freescale/imx8dxl-ss-conn.dtsi | 11 ++++
drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c | 20 ++++++-
drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.h | 4 ++
6 files changed, 138 insertions(+), 3 deletions(-)
---
base-commit: dbd9e2e056d8577375ae4b31ada94f8aa3769e8a
change-id: 20240516-gpmi_nand-e11272cbdfae

Best regards,
---
Frank Li <[email protected]>



2024-05-17 18:10:44

by Frank Li

[permalink] [raw]
Subject: [PATCH 1/5] dt-bindings: mtd: gpmi-nand: Add 'fsl,imx8qxp-gpmi-nand' compatible string

Add 'fsl,imx8qxp-gpmi-nand' compatible string and clock-names restriction.

Signed-off-by: Frank Li <[email protected]>
---
.../devicetree/bindings/mtd/gpmi-nand.yaml | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)

diff --git a/Documentation/devicetree/bindings/mtd/gpmi-nand.yaml b/Documentation/devicetree/bindings/mtd/gpmi-nand.yaml
index 021c0da0b072f..f9eb1868ca1f4 100644
--- a/Documentation/devicetree/bindings/mtd/gpmi-nand.yaml
+++ b/Documentation/devicetree/bindings/mtd/gpmi-nand.yaml
@@ -24,6 +24,7 @@ properties:
- fsl,imx6q-gpmi-nand
- fsl,imx6sx-gpmi-nand
- fsl,imx7d-gpmi-nand
+ - fsl,imx8qxp-gpmi-nand
- items:
- enum:
- fsl,imx8mm-gpmi-nand
@@ -151,6 +152,27 @@ allOf:
- const: gpmi_io
- const: gpmi_bch_apb

+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - fsl,imx8qxp-gpmi-nand
+ then:
+ properties:
+ clocks:
+ items:
+ - description: SoC gpmi io clock
+ - description: SoC gpmi apb clock
+ - description: SoC gpmi bch clock
+ - description: SoC gpmi bch apb clock
+ clock-names:
+ items:
+ - const: gpmi_io
+ - const: gpmi_apb
+ - const: gpmi_bch
+ - const: gpmi_bch_apb
+
examples:
- |
nand-controller@8000c000 {

--
2.34.1


2024-05-17 18:11:00

by Frank Li

[permalink] [raw]
Subject: [PATCH 2/5] dt-bindings: dma: fsl-mxs-dma: Add compatible string "fsl,imx8qxp-dma-apbh"

Add compatible string "fsl,imx8qxp-dma-apbh". It requires power-domains
compared with "fsl,imx28-dma-apbh".

Allow 'power-domains' property because i.MX8DXL i.MX8QM and i.MX8QXP need
it.

Keep the same restriction about 'power-domains' for other compatible
strings.

Signed-off-by: Frank Li <[email protected]>
---
Documentation/devicetree/bindings/dma/fsl,mxs-dma.yaml | 15 +++++++++++++++
1 file changed, 15 insertions(+)

diff --git a/Documentation/devicetree/bindings/dma/fsl,mxs-dma.yaml b/Documentation/devicetree/bindings/dma/fsl,mxs-dma.yaml
index add9c77e8b52a..a17cf2360dd4a 100644
--- a/Documentation/devicetree/bindings/dma/fsl,mxs-dma.yaml
+++ b/Documentation/devicetree/bindings/dma/fsl,mxs-dma.yaml
@@ -11,6 +11,17 @@ maintainers:

allOf:
- $ref: dma-controller.yaml#
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: fsl,imx8qxp-dma-apbh
+ then:
+ required:
+ - power-domains
+ else:
+ properties:
+ power-domains: false

properties:
compatible:
@@ -20,6 +31,7 @@ properties:
- fsl,imx6q-dma-apbh
- fsl,imx6sx-dma-apbh
- fsl,imx7d-dma-apbh
+ - fsl,imx8qxp-dma-apbh
- const: fsl,imx28-dma-apbh
- enum:
- fsl,imx23-dma-apbh
@@ -42,6 +54,9 @@ properties:
dma-channels:
enum: [4, 8, 16]

+ power-domains:
+ maxItems: 1
+
required:
- compatible
- reg

--
2.34.1


2024-05-17 18:11:13

by Frank Li

[permalink] [raw]
Subject: [PATCH 3/5] mtd: rawnand: gpmi: add iMX8QXP support.

From: Han Xu <[email protected]>

Add "fsl,imx8qxp-gpmi-nand" compatible string. iMX8QXP gpmi nand is similar
with iMX7D. But it using 4 clock: "gpmi_io", "gpmi_apb", "gpmi_bch" and
"gpmi_bch_apb".

Signed-off-by: Han Xu <[email protected]>
Signed-off-by: Frank Li <[email protected]>
---
drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c | 20 +++++++++++++++++---
drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.h | 4 ++++
2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
index e71ad2fcec232..f90c5207bacb6 100644
--- a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
@@ -983,7 +983,8 @@ static int gpmi_setup_interface(struct nand_chip *chip, int chipnr,
return PTR_ERR(sdr);

/* Only MX28/MX6 GPMI controller can reach EDO timings */
- if (sdr->tRC_min <= 25000 && !GPMI_IS_MX28(this) && !GPMI_IS_MX6(this))
+ if (sdr->tRC_min <= 25000 && !GPMI_IS_MX28(this) &&
+ !(GPMI_IS_MX6(this) || GPMI_IS_MX8(this)))
return -ENOTSUPP;

/* Stop here if this call was just a check */
@@ -1178,6 +1179,18 @@ static const struct gpmi_devdata gpmi_devdata_imx7d = {
.clks_count = ARRAY_SIZE(gpmi_clks_for_mx7d),
};

+static const char *gpmi_clks_for_mx8qxp[GPMI_CLK_MAX] = {
+ "gpmi_io", "gpmi_apb", "gpmi_bch", "gpmi_bch_apb",
+};
+
+static const struct gpmi_devdata gpmi_devdata_imx8qxp = {
+ .type = IS_MX8QXP,
+ .bch_max_ecc_strength = 62,
+ .max_chain_delay = 12000,
+ .clks = gpmi_clks_for_mx8qxp,
+ .clks_count = ARRAY_SIZE(gpmi_clks_for_mx8qxp),
+};
+
static int acquire_register_block(struct gpmi_nand_data *this,
const char *res_name)
{
@@ -2281,7 +2294,7 @@ static int gpmi_init_last(struct gpmi_nand_data *this)
* (1) the chip is imx6, and
* (2) the size of the ECC parity is byte aligned.
*/
- if (GPMI_IS_MX6(this) &&
+ if ((GPMI_IS_MX6(this) || GPMI_IS_MX8(this)) &&
((bch_geo->gf_len * bch_geo->ecc_strength) % 8) == 0) {
ecc->read_subpage = gpmi_ecc_read_subpage;
chip->options |= NAND_SUBPAGE_READ;
@@ -2692,7 +2705,7 @@ static int gpmi_nand_init(struct gpmi_nand_data *this)
this->base.ops = &gpmi_nand_controller_ops;
chip->controller = &this->base;

- ret = nand_scan(chip, GPMI_IS_MX6(this) ? 2 : 1);
+ ret = nand_scan(chip, (GPMI_IS_MX6(this) || GPMI_IS_MX8(this)) ? 2 : 1);
if (ret)
goto err_out;

@@ -2721,6 +2734,7 @@ static const struct of_device_id gpmi_nand_id_table[] = {
{ .compatible = "fsl,imx6q-gpmi-nand", .data = &gpmi_devdata_imx6q, },
{ .compatible = "fsl,imx6sx-gpmi-nand", .data = &gpmi_devdata_imx6sx, },
{ .compatible = "fsl,imx7d-gpmi-nand", .data = &gpmi_devdata_imx7d,},
+ { .compatible = "fsl,imx8qxp-gpmi-nand", .data = &gpmi_devdata_imx8qxp, },
{}
};
MODULE_DEVICE_TABLE(of, gpmi_nand_id_table);
diff --git a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.h b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.h
index c3ff56ac62a7e..502f01a8338c3 100644
--- a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.h
+++ b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.h
@@ -78,6 +78,7 @@ enum gpmi_type {
IS_MX6Q,
IS_MX6SX,
IS_MX7D,
+ IS_MX8QXP,
};

struct gpmi_devdata {
@@ -172,8 +173,11 @@ struct gpmi_nand_data {
#define GPMI_IS_MX6Q(x) ((x)->devdata->type == IS_MX6Q)
#define GPMI_IS_MX6SX(x) ((x)->devdata->type == IS_MX6SX)
#define GPMI_IS_MX7D(x) ((x)->devdata->type == IS_MX7D)
+#define GPMI_IS_MX8QXP(x) ((x)->devdata->type == IS_MX8QXP)

#define GPMI_IS_MX6(x) (GPMI_IS_MX6Q(x) || GPMI_IS_MX6SX(x) || \
GPMI_IS_MX7D(x))
+
+#define GPMI_IS_MX8(x) (GPMI_IS_MX8QXP(x))
#define GPMI_IS_MXS(x) (GPMI_IS_MX23(x) || GPMI_IS_MX28(x))
#endif

--
2.34.1


2024-05-17 18:11:25

by Frank Li

[permalink] [raw]
Subject: [PATCH 4/5] arm64: dts: imx8-ss-conn: add gpmi nand node

Add gpmi nand support.

Signed-off-by: Frank Li <[email protected]>
---
arch/arm64/boot/dts/freescale/imx8-ss-conn.dtsi | 69 +++++++++++++++++++++++++
1 file changed, 69 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8-ss-conn.dtsi b/arch/arm64/boot/dts/freescale/imx8-ss-conn.dtsi
index 4aaf5a0c1ed8a..a4a10ce03bfe0 100644
--- a/arch/arm64/boot/dts/freescale/imx8-ss-conn.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8-ss-conn.dtsi
@@ -28,6 +28,13 @@ conn_ipg_clk: clock-conn-ipg {
clock-output-names = "conn_ipg_clk";
};

+conn_bch_clk: clock-conn-bch {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <400000000>;
+ clock-output-names = "conn_bch_clk";
+};
+
conn_subsys: bus@5b000000 {
compatible = "simple-bus";
#address-cells = <1>;
@@ -302,4 +309,66 @@ usb3_lpcg: clock-controller@5b280000 {
"usb3_aclk";
power-domains = <&pd IMX_SC_R_USB_2_PHY>;
};
+
+ rawnand_0_lpcg: clock-controller@5b290000 {
+ compatible = "fsl,imx8qxp-lpcg";
+ reg = <0x5b290000 0x4>;
+ #clock-cells = <1>;
+ clocks = <&clk IMX_SC_R_NAND IMX_SC_PM_CLK_PER>,
+ <&clk IMX_SC_R_NAND IMX_SC_PM_CLK_MST_BUS>,
+ <&conn_axi_clk>,
+ <&conn_axi_clk>;
+ clock-indices = <IMX_LPCG_CLK_0>, <IMX_LPCG_CLK_1>,
+ <IMX_LPCG_CLK_4>, <IMX_LPCG_CLK_5>;
+ clock-output-names = "gpmi_bch",
+ "gpmi_io",
+ "gpmi_apb",
+ "gpmi_bch_apb";
+ power-domains = <&pd IMX_SC_R_NAND>;
+ };
+
+ rawnand_4_lpcg: clock-controller@5b290004 {
+ compatible = "fsl,imx8qxp-lpcg";
+ reg = <0x5b290004 0x10000>;
+ #clock-cells = <1>;
+ clocks = <&conn_axi_clk>;
+ clock-indices = <IMX_LPCG_CLK_4>;
+ clock-output-names = "apbhdma_hclk";
+ power-domains = <&pd IMX_SC_R_NAND>;
+ };
+
+ dma_apbh: dma-controller@5b810000 {
+ compatible = "fsl,imx8qxp-dma-apbh", "fsl,imx28-dma-apbh";
+ reg = <0x5b810000 0x2000>;
+ interrupts = <GIC_SPI 274 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 274 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 274 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 274 IRQ_TYPE_LEVEL_HIGH>;
+ #dma-cells = <1>;
+ dma-channels = <4>;
+ clocks = <&rawnand_4_lpcg IMX_LPCG_CLK_0>;
+ power-domains = <&pd IMX_SC_R_NAND>;
+ };
+
+ gpmi: nand-controller@5b812000{
+ compatible = "fsl,imx8qxp-gpmi-nand";
+ reg = <0x5b812000 0x2000>, <0x5b814000 0x2000>;
+ reg-names = "gpmi-nand", "bch";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ interrupts = <GIC_SPI 272 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "bch";
+ clocks = <&rawnand_0_lpcg IMX_LPCG_CLK_1>,
+ <&rawnand_0_lpcg IMX_LPCG_CLK_4>,
+ <&rawnand_0_lpcg IMX_LPCG_CLK_0>,
+ <&rawnand_0_lpcg IMX_LPCG_CLK_5>;
+ clock-names = "gpmi_io", "gpmi_apb",
+ "gpmi_bch", "gpmi_bch_apb";
+ dmas = <&dma_apbh 0>;
+ dma-names = "rx-tx";
+ power-domains = <&pd IMX_SC_R_NAND>;
+ assigned-clocks = <&clk IMX_SC_R_NAND IMX_SC_PM_CLK_MST_BUS>;
+ assigned-clock-rates = <50000000>;
+ status = "disabled";
+ };
};

--
2.34.1


2024-05-17 18:11:34

by Frank Li

[permalink] [raw]
Subject: [PATCH 5/5] arm64: dts: imx8dxl-ss-conn: add gpmi nand

Update gpmi nand and dma_apbh interrupt number for imx8dxl.

Signed-off-by: Frank Li <[email protected]>
---
arch/arm64/boot/dts/freescale/imx8dxl-ss-conn.dtsi | 11 +++++++++++
1 file changed, 11 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8dxl-ss-conn.dtsi b/arch/arm64/boot/dts/freescale/imx8dxl-ss-conn.dtsi
index 6d13e4fafb761..1e02b04494e94 100644
--- a/arch/arm64/boot/dts/freescale/imx8dxl-ss-conn.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8dxl-ss-conn.dtsi
@@ -108,6 +108,13 @@ usb2_2_lpcg: clock-controller@5b280000 {

};

+&dma_apbh {
+ interrupts = <GIC_SPI 176 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 176 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 176 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 176 IRQ_TYPE_LEVEL_HIGH>;
+};
+
&enet0_lpcg {
clocks = <&conn_enet0_root_clk>,
<&conn_enet0_root_clk>,
@@ -127,6 +134,10 @@ &fec1 {
assigned-clock-rates = <125000000>;
};

+&gpmi {
+ interrupts = <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
+};
+
&usdhc1 {
compatible = "fsl,imx8dxl-usdhc", "fsl,imx8qxp-usdhc";
interrupts = <GIC_SPI 138 IRQ_TYPE_LEVEL_HIGH>;

--
2.34.1


2024-05-17 18:36:40

by Miquel Raynal

[permalink] [raw]
Subject: Re: [PATCH 1/5] dt-bindings: mtd: gpmi-nand: Add 'fsl,imx8qxp-gpmi-nand' compatible string

Hi Frank,

[email protected] wrote on Fri, 17 May 2024 14:09:48 -0400:

> Add 'fsl,imx8qxp-gpmi-nand' compatible string and clock-names restriction.
>
> Signed-off-by: Frank Li <[email protected]>
> ---
> .../devicetree/bindings/mtd/gpmi-nand.yaml | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mtd/gpmi-nand.yaml b/Documentation/devicetree/bindings/mtd/gpmi-nand.yaml
> index 021c0da0b072f..f9eb1868ca1f4 100644
> --- a/Documentation/devicetree/bindings/mtd/gpmi-nand.yaml
> +++ b/Documentation/devicetree/bindings/mtd/gpmi-nand.yaml
> @@ -24,6 +24,7 @@ properties:
> - fsl,imx6q-gpmi-nand
> - fsl,imx6sx-gpmi-nand
> - fsl,imx7d-gpmi-nand
> + - fsl,imx8qxp-gpmi-nand
> - items:
> - enum:
> - fsl,imx8mm-gpmi-nand
> @@ -151,6 +152,27 @@ allOf:
> - const: gpmi_io
> - const: gpmi_bch_apb
>
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - fsl,imx8qxp-gpmi-nand
> + then:
> + properties:
> + clocks:
> + items:
> + - description: SoC gpmi io clock
> + - description: SoC gpmi apb clock

I believe these two clocks are mandatory?

> + - description: SoC gpmi bch clock
> + - description: SoC gpmi bch apb clock
> + clock-names:
> + items:
> + - const: gpmi_io
> + - const: gpmi_apb
> + - const: gpmi_bch
> + - const: gpmi_bch_apb
> +
> examples:
> - |
> nand-controller@8000c000 {
>


Thanks,
Miquèl

2024-05-17 18:39:26

by Miquel Raynal

[permalink] [raw]
Subject: Re: [PATCH 3/5] mtd: rawnand: gpmi: add iMX8QXP support.

Hi Frank,

[email protected] wrote on Fri, 17 May 2024 14:09:50 -0400:

> From: Han Xu <[email protected]>
>
> Add "fsl,imx8qxp-gpmi-nand" compatible string. iMX8QXP gpmi nand is similar
> with iMX7D. But it using 4 clock: "gpmi_io", "gpmi_apb", "gpmi_bch" and

to? is clocks

> "gpmi_bch_apb".
>
> Signed-off-by: Han Xu <[email protected]>
> Signed-off-by: Frank Li <[email protected]>
> ---
> drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c | 20 +++++++++++++++++---
> drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.h | 4 ++++
> 2 files changed, 21 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
> index e71ad2fcec232..f90c5207bacb6 100644
> --- a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
> +++ b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
> @@ -983,7 +983,8 @@ static int gpmi_setup_interface(struct nand_chip *chip, int chipnr,
> return PTR_ERR(sdr);
>
> /* Only MX28/MX6 GPMI controller can reach EDO timings */
> - if (sdr->tRC_min <= 25000 && !GPMI_IS_MX28(this) && !GPMI_IS_MX6(this))
> + if (sdr->tRC_min <= 25000 && !GPMI_IS_MX28(this) &&
> + !(GPMI_IS_MX6(this) || GPMI_IS_MX8(this)))

Feels completely redundant, no? If it's not an imx6 nor an imx28, it
already returns -ENOTSUPP.

> return -ENOTSUPP;


Fine otherwise.

Thanks,
Miquèl

2024-05-17 19:16:11

by Frank Li

[permalink] [raw]
Subject: Re: [PATCH 1/5] dt-bindings: mtd: gpmi-nand: Add 'fsl,imx8qxp-gpmi-nand' compatible string

On Fri, May 17, 2024 at 08:36:21PM +0200, Miquel Raynal wrote:
> Hi Frank,
>
> [email protected] wrote on Fri, 17 May 2024 14:09:48 -0400:
>
> > Add 'fsl,imx8qxp-gpmi-nand' compatible string and clock-names restriction.
> >
> > Signed-off-by: Frank Li <[email protected]>
> > ---
> > .../devicetree/bindings/mtd/gpmi-nand.yaml | 22 ++++++++++++++++++++++
> > 1 file changed, 22 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/mtd/gpmi-nand.yaml b/Documentation/devicetree/bindings/mtd/gpmi-nand.yaml
> > index 021c0da0b072f..f9eb1868ca1f4 100644
> > --- a/Documentation/devicetree/bindings/mtd/gpmi-nand.yaml
> > +++ b/Documentation/devicetree/bindings/mtd/gpmi-nand.yaml
> > @@ -24,6 +24,7 @@ properties:
> > - fsl,imx6q-gpmi-nand
> > - fsl,imx6sx-gpmi-nand
> > - fsl,imx7d-gpmi-nand
> > + - fsl,imx8qxp-gpmi-nand
> > - items:
> > - enum:
> > - fsl,imx8mm-gpmi-nand
> > @@ -151,6 +152,27 @@ allOf:
> > - const: gpmi_io
> > - const: gpmi_bch_apb
> >
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + enum:
> > + - fsl,imx8qxp-gpmi-nand
> > + then:
> > + properties:
> > + clocks:
> > + items:
> > + - description: SoC gpmi io clock
> > + - description: SoC gpmi apb clock
>
> I believe these two clocks are mandatory?

minItems default is equal to items numbers, here is 4. So all 4 clock are
mandatory.

Anything wrong here?

Frank

>
> > + - description: SoC gpmi bch clock
> > + - description: SoC gpmi bch apb clock
> > + clock-names:
> > + items:
> > + - const: gpmi_io
> > + - const: gpmi_apb
> > + - const: gpmi_bch
> > + - const: gpmi_bch_apb
> > +
> > examples:
> > - |
> > nand-controller@8000c000 {
> >
>
>
> Thanks,
> Miqu?l

2024-05-17 19:52:26

by Miquel Raynal

[permalink] [raw]
Subject: Re: [PATCH 1/5] dt-bindings: mtd: gpmi-nand: Add 'fsl,imx8qxp-gpmi-nand' compatible string

Hi Frank,

[email protected] wrote on Fri, 17 May 2024 15:15:42 -0400:

> On Fri, May 17, 2024 at 08:36:21PM +0200, Miquel Raynal wrote:
> > Hi Frank,
> >
> > [email protected] wrote on Fri, 17 May 2024 14:09:48 -0400:
> >
> > > Add 'fsl,imx8qxp-gpmi-nand' compatible string and clock-names restriction.
> > >
> > > Signed-off-by: Frank Li <[email protected]>
> > > ---
> > > .../devicetree/bindings/mtd/gpmi-nand.yaml | 22 ++++++++++++++++++++++
> > > 1 file changed, 22 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/mtd/gpmi-nand.yaml b/Documentation/devicetree/bindings/mtd/gpmi-nand.yaml
> > > index 021c0da0b072f..f9eb1868ca1f4 100644
> > > --- a/Documentation/devicetree/bindings/mtd/gpmi-nand.yaml
> > > +++ b/Documentation/devicetree/bindings/mtd/gpmi-nand.yaml
> > > @@ -24,6 +24,7 @@ properties:
> > > - fsl,imx6q-gpmi-nand
> > > - fsl,imx6sx-gpmi-nand
> > > - fsl,imx7d-gpmi-nand
> > > + - fsl,imx8qxp-gpmi-nand
> > > - items:
> > > - enum:
> > > - fsl,imx8mm-gpmi-nand
> > > @@ -151,6 +152,27 @@ allOf:
> > > - const: gpmi_io
> > > - const: gpmi_bch_apb
> > >
> > > + - if:
> > > + properties:
> > > + compatible:
> > > + contains:
> > > + enum:
> > > + - fsl,imx8qxp-gpmi-nand
> > > + then:
> > > + properties:
> > > + clocks:
> > > + items:
> > > + - description: SoC gpmi io clock
> > > + - description: SoC gpmi apb clock
> >
> > I believe these two clocks are mandatory?
>
> minItems default is equal to items numbers, here is 4. So all 4 clock are
> mandatory.
>
> Anything wrong here?

I'd say that the two "bch" clocks are only used if you decide to
configure the on-host hardware ECC engine and thus are not needed with
software corrections, but I'm fine keeping the fourth described in all
cases if that's simpler.

Also,here the diff just shows that "if we provide a clocks property
with this compatible, then we need to provide 4 members", I believe the
"required" property is already filled somewhere with the
clocks/clock-names properties?

Thanks,
Miquèl

2024-05-17 20:05:10

by Frank Li

[permalink] [raw]
Subject: Re: [PATCH 3/5] mtd: rawnand: gpmi: add iMX8QXP support.

On Fri, May 17, 2024 at 08:39:04PM +0200, Miquel Raynal wrote:
> Hi Frank,
>
> [email protected] wrote on Fri, 17 May 2024 14:09:50 -0400:
>
> > From: Han Xu <[email protected]>
> >
> > Add "fsl,imx8qxp-gpmi-nand" compatible string. iMX8QXP gpmi nand is similar
> > with iMX7D. But it using 4 clock: "gpmi_io", "gpmi_apb", "gpmi_bch" and
>
> to? is clocks
>
> > "gpmi_bch_apb".
> >
> > Signed-off-by: Han Xu <[email protected]>
> > Signed-off-by: Frank Li <[email protected]>
> > ---
> > drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c | 20 +++++++++++++++++---
> > drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.h | 4 ++++
> > 2 files changed, 21 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
> > index e71ad2fcec232..f90c5207bacb6 100644
> > --- a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
> > +++ b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
> > @@ -983,7 +983,8 @@ static int gpmi_setup_interface(struct nand_chip *chip, int chipnr,
> > return PTR_ERR(sdr);
> >
> > /* Only MX28/MX6 GPMI controller can reach EDO timings */
> > - if (sdr->tRC_min <= 25000 && !GPMI_IS_MX28(this) && !GPMI_IS_MX6(this))
> > + if (sdr->tRC_min <= 25000 && !GPMI_IS_MX28(this) &&
> > + !(GPMI_IS_MX6(this) || GPMI_IS_MX8(this)))
>
> Feels completely redundant, no? If it's not an imx6 nor an imx28, it
> already returns -ENOTSUPP.

if this is mx8

sdr->tRC_min <= 25000: true
!GPMI_IS_MX28(this): true
!GPMI_IS_MX6(this): true

so whole statement is true

after change
sdr->tRC_min <= 25000: true
!GPMI_IS_MX28(this): true
!(GPMI_IS_MX6(this) || GPMI_IS_MX8(this)): false
(GPMI_IS_MX6(this) || GPMI_IS_MX8(this)): true
GPMI_IS_MX6(this): false
GPMI_IS_MX8(this): true

so whole statement is false. Maybe use

sdr->tRC_min <= 25000 && !GPMI_IS_MX28(this) && !(GPMI_IS_MX6(this) &&
!(GPMI_IS_MX8(this))

looks better.

Or use a drvflag.
if (sdr->tRC_min <= 25000 && (!this->drvflag->support_tRC_min))

Frank

>
> > return -ENOTSUPP;
>
>
> Fine otherwise.
>
> Thanks,
> Miqu?l

2024-05-17 21:09:44

by Frank Li

[permalink] [raw]
Subject: Re: [PATCH 1/5] dt-bindings: mtd: gpmi-nand: Add 'fsl,imx8qxp-gpmi-nand' compatible string

On Fri, May 17, 2024 at 09:50:55PM +0200, Miquel Raynal wrote:
> Hi Frank,
>
> [email protected] wrote on Fri, 17 May 2024 15:15:42 -0400:
>
> > On Fri, May 17, 2024 at 08:36:21PM +0200, Miquel Raynal wrote:
> > > Hi Frank,
> > >
> > > [email protected] wrote on Fri, 17 May 2024 14:09:48 -0400:
> > >
> > > > Add 'fsl,imx8qxp-gpmi-nand' compatible string and clock-names restriction.
> > > >
> > > > Signed-off-by: Frank Li <[email protected]>
> > > > ---
> > > > .../devicetree/bindings/mtd/gpmi-nand.yaml | 22 ++++++++++++++++++++++
> > > > 1 file changed, 22 insertions(+)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/mtd/gpmi-nand.yaml b/Documentation/devicetree/bindings/mtd/gpmi-nand.yaml
> > > > index 021c0da0b072f..f9eb1868ca1f4 100644
> > > > --- a/Documentation/devicetree/bindings/mtd/gpmi-nand.yaml
> > > > +++ b/Documentation/devicetree/bindings/mtd/gpmi-nand.yaml
> > > > @@ -24,6 +24,7 @@ properties:
> > > > - fsl,imx6q-gpmi-nand
> > > > - fsl,imx6sx-gpmi-nand
> > > > - fsl,imx7d-gpmi-nand
> > > > + - fsl,imx8qxp-gpmi-nand
> > > > - items:
> > > > - enum:
> > > > - fsl,imx8mm-gpmi-nand
> > > > @@ -151,6 +152,27 @@ allOf:
> > > > - const: gpmi_io
> > > > - const: gpmi_bch_apb
> > > >
> > > > + - if:
> > > > + properties:
> > > > + compatible:
> > > > + contains:
> > > > + enum:
> > > > + - fsl,imx8qxp-gpmi-nand
> > > > + then:
> > > > + properties:
> > > > + clocks:
> > > > + items:
> > > > + - description: SoC gpmi io clock
> > > > + - description: SoC gpmi apb clock
> > >
> > > I believe these two clocks are mandatory?
> >
> > minItems default is equal to items numbers, here is 4. So all 4 clock are
> > mandatory.
> >
> > Anything wrong here?
>
> I'd say that the two "bch" clocks are only used if you decide to
> configure the on-host hardware ECC engine and thus are not needed with
> software corrections, but I'm fine keeping the fourth described in all
> cases if that's simpler.
>
> Also,here the diff just shows that "if we provide a clocks property
> with this compatible, then we need to provide 4 members", I believe the
> "required" property is already filled somewhere with the
> clocks/clock-names properties?

yes, before allOf

required:
..
- clocks
- clock-names
..

>
> Thanks,
> Miqu?l

2024-05-20 20:40:42

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH 2/5] dt-bindings: dma: fsl-mxs-dma: Add compatible string "fsl,imx8qxp-dma-apbh"


On Fri, 17 May 2024 14:09:49 -0400, Frank Li wrote:
> Add compatible string "fsl,imx8qxp-dma-apbh". It requires power-domains
> compared with "fsl,imx28-dma-apbh".
>
> Allow 'power-domains' property because i.MX8DXL i.MX8QM and i.MX8QXP need
> it.
>
> Keep the same restriction about 'power-domains' for other compatible
> strings.
>
> Signed-off-by: Frank Li <[email protected]>
> ---
> Documentation/devicetree/bindings/dma/fsl,mxs-dma.yaml | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>

Reviewed-by: Rob Herring (Arm) <[email protected]>