2022-12-08 06:45:16

by Xiangsheng Hou

[permalink] [raw]
Subject: [PATCH v3 0/9] Add MediaTek MT7986 SPI NAND and ECC support

Changes since V2:
- Change ECC err_mask value with GENMASK macro.
- Change snfi mediatek,rx-latch-latency to mediatek,rx-latch-latency-ns.
- Add a separate patch for DTS change.
- Move common description to top-level pattern properties.
- Drop redundant parts in dt-bindings.

Changes since V1:
- Use existing sample delay property.
- Add restricting for optional nfi_hclk.
- Improve and perfect dt-bindings documentation.
- Change existing node name to match NAND controller DT bingings.
- Fix issues reported by dt_binding_check.
- Fix issues reported by dtbs_check.

Xiangsheng Hou (9):
spi: mtk-snfi: Change default page format to setup default setting
spi: mtk-snfi: Add optional nfi_hclk which is needed for MT7986
mtd: nand: ecc-mtk: Add ECC support fot MT7986 IC
dt-bindings: spi: mtk-snfi: Add compatible for MT7986
spi: mtk-snfi: Add snfi sample delay and read latency adjustment
dt-bindings: spi: mtk-snfi: Add read latch latency property
dt-bindings: mtd: Split ECC engine with rawnand controller
arm/arm64: dts: mediatek: Fix existing NAND controller node name
dt-bindings: mtd: mediatek,nand-ecc-engine: Add compatible for MT7986

.../bindings/mtd/mediatek,mtk-nfc.yaml | 154 +++++++++++++++
.../mtd/mediatek,nand-ecc-engine.yaml | 63 +++++++
.../devicetree/bindings/mtd/mtk-nand.txt | 176 ------------------
.../bindings/spi/mediatek,spi-mtk-snfi.yaml | 54 +++++-
arch/arm/boot/dts/mt2701.dtsi | 2 +-
arch/arm64/boot/dts/mediatek/mt2712e.dtsi | 2 +-
arch/arm64/boot/dts/mediatek/mt7622.dtsi | 2 +-
drivers/mtd/nand/ecc-mtk.c | 28 ++-
drivers/spi/spi-mtk-snfi.c | 41 +++-
9 files changed, 329 insertions(+), 193 deletions(-)
create mode 100644 Documentation/devicetree/bindings/mtd/mediatek,mtk-nfc.yaml
create mode 100644 Documentation/devicetree/bindings/mtd/mediatek,nand-ecc-engine.yaml
delete mode 100644 Documentation/devicetree/bindings/mtd/mtk-nand.txt

--
2.25.1


2022-12-08 07:04:16

by Xiangsheng Hou

[permalink] [raw]
Subject: [PATCH v3 2/9] spi: mtk-snfi: Add optional nfi_hclk which is needed for MT7986

Add optional nfi_hclk which is needed for MT7986.

Signed-off-by: Xiangsheng Hou <[email protected]>
Reviewed-by: AngeloGioacchino Del Regno <[email protected]>
---
drivers/spi/spi-mtk-snfi.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)

diff --git a/drivers/spi/spi-mtk-snfi.c b/drivers/spi/spi-mtk-snfi.c
index 719fc6f53ab1..85644308df23 100644
--- a/drivers/spi/spi-mtk-snfi.c
+++ b/drivers/spi/spi-mtk-snfi.c
@@ -297,6 +297,7 @@ struct mtk_snand {
struct device *dev;
struct clk *nfi_clk;
struct clk *pad_clk;
+ struct clk *nfi_hclk;
void __iomem *nfi_base;
int irq;
struct completion op_done;
@@ -1339,7 +1340,16 @@ static int mtk_snand_enable_clk(struct mtk_snand *ms)
dev_err(ms->dev, "unable to enable pad clk\n");
goto err1;
}
+ ret = clk_prepare_enable(ms->nfi_hclk);
+ if (ret) {
+ dev_err(ms->dev, "unable to enable nfi hclk\n");
+ goto err2;
+ }
+
return 0;
+
+err2:
+ clk_disable_unprepare(ms->pad_clk);
err1:
clk_disable_unprepare(ms->nfi_clk);
return ret;
@@ -1347,6 +1357,7 @@ static int mtk_snand_enable_clk(struct mtk_snand *ms)

static void mtk_snand_disable_clk(struct mtk_snand *ms)
{
+ clk_disable_unprepare(ms->nfi_hclk);
clk_disable_unprepare(ms->pad_clk);
clk_disable_unprepare(ms->nfi_clk);
}
@@ -1401,6 +1412,13 @@ static int mtk_snand_probe(struct platform_device *pdev)
goto release_ecc;
}

+ ms->nfi_hclk = devm_clk_get_optional(&pdev->dev, "nfi_hclk");
+ if (IS_ERR(ms->nfi_hclk)) {
+ ret = PTR_ERR(ms->nfi_hclk);
+ dev_err(&pdev->dev, "unable to get nfi_hclk, err = %d\n", ret);
+ goto release_ecc;
+ }
+
ret = mtk_snand_enable_clk(ms);
if (ret)
goto release_ecc;
--
2.25.1

2022-12-08 07:06:38

by Xiangsheng Hou

[permalink] [raw]
Subject: [PATCH v3 7/9] dt-bindings: mtd: Split ECC engine with rawnand controller

Split MediaTek ECC engine with rawnand controller and convert to
YAML schema.

Signed-off-by: Xiangsheng Hou <[email protected]>
---
.../bindings/mtd/mediatek,mtk-nfc.yaml | 154 +++++++++++++++
.../mtd/mediatek,nand-ecc-engine.yaml | 62 ++++++
.../devicetree/bindings/mtd/mtk-nand.txt | 176 ------------------
3 files changed, 216 insertions(+), 176 deletions(-)
create mode 100644 Documentation/devicetree/bindings/mtd/mediatek,mtk-nfc.yaml
create mode 100644 Documentation/devicetree/bindings/mtd/mediatek,nand-ecc-engine.yaml
delete mode 100644 Documentation/devicetree/bindings/mtd/mtk-nand.txt

diff --git a/Documentation/devicetree/bindings/mtd/mediatek,mtk-nfc.yaml b/Documentation/devicetree/bindings/mtd/mediatek,mtk-nfc.yaml
new file mode 100644
index 000000000000..eb1a44c7ae4e
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/mediatek,mtk-nfc.yaml
@@ -0,0 +1,154 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mtd/mediatek,mtk-nfc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MediaTek(MTK) SoCs raw NAND FLASH controller (NFC)
+
+maintainers:
+ - Xiangsheng Hou <[email protected]>
+
+properties:
+ compatible:
+ enum:
+ - mediatek,mt2701-nfc
+ - mediatek,mt2712-nfc
+ - mediatek,mt7622-nfc
+
+ reg:
+ items:
+ - description: Base physical address and size of NFI.
+
+ interrupts:
+ items:
+ - description: NFI interrupt
+
+ clocks:
+ items:
+ - description: clock used for the controller
+ - description: clock used for the pad
+
+ clock-names:
+ items:
+ - const: nfi_clk
+ - const: pad_clk
+
+ ecc-engine:
+ description: device-tree node of the required ECC engine.
+ $ref: /schemas/types.yaml#/definitions/phandle
+
+patternProperties:
+ "^nand@[a-f0-9]$":
+ type: object
+ properties:
+ reg:
+ minimum: 0
+ maximum: 1
+ nand-ecc-mode:
+ const: hw
+
+allOf:
+ - $ref: nand-controller.yaml#
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: mediatek,mt2701-nfc
+ then:
+ patternProperties:
+ "^nand@[a-f0-9]$":
+ properties:
+ nand-ecc-step-size:
+ enum: [ 512, 1024 ]
+ nand-ecc-strength:
+ enum: [4, 6, 8, 10, 12, 14, 16, 18, 20, 22, 24, 28, 32, 36,
+ 40, 44, 48, 52, 56, 60]
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: mediatek,mt2712-nfc
+ then:
+ patternProperties:
+ "^nand@[a-f0-9]$":
+ properties:
+ nand-ecc-step-size:
+ enum: [ 512, 1024 ]
+ nand-ecc-strength:
+ enum: [4, 6, 8, 10, 12, 14, 16, 18, 20, 22, 24, 28, 32, 36,
+ 40, 44, 48, 52, 56, 60, 68, 72, 80]
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: mediatek,mt7622-nfc
+ then:
+ patternProperties:
+ "^nand@[a-f0-9]$":
+ properties:
+ nand-ecc-step-size:
+ const: 512
+ nand-ecc-strength:
+ enum: [4, 6, 8, 10, 12]
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - clocks
+ - clock-names
+ - ecc-engine
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/mt2701-clk.h>
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/interrupt-controller/irq.h>
+
+ soc {
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ nand-controller@1100d000 {
+ compatible = "mediatek,mt2701-nfc";
+ reg = <0 0x1100d000 0 0x1000>;
+ interrupts = <GIC_SPI 56 IRQ_TYPE_LEVEL_LOW>;
+ clocks = <&pericfg CLK_PERI_NFI>,
+ <&pericfg CLK_PERI_NFI_PAD>;
+ clock-names = "nfi_clk", "pad_clk";
+ ecc-engine = <&bch>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ nand@0 {
+ reg = <0>;
+
+ nand-on-flash-bbt;
+ nand-ecc-mode = "hw";
+ nand-ecc-step-size = <1024>;
+ nand-ecc-strength = <24>;
+
+ partitions {
+ compatible = "fixed-partitions";
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ preloader@0 {
+ label = "pl";
+ read-only;
+ reg = <0x0 0x400000>;
+ };
+ android@400000 {
+ label = "android";
+ reg = <0x400000 0x12c00000>;
+ };
+ };
+ };
+ };
+ };
diff --git a/Documentation/devicetree/bindings/mtd/mediatek,nand-ecc-engine.yaml b/Documentation/devicetree/bindings/mtd/mediatek,nand-ecc-engine.yaml
new file mode 100644
index 000000000000..b13d801eda76
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/mediatek,nand-ecc-engine.yaml
@@ -0,0 +1,62 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mtd/mediatek,nand-ecc-engine.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MediaTek(MTK) SoCs NAND ECC engine
+
+maintainers:
+ - Xiangsheng Hou <[email protected]>
+
+description: |
+ MTK NAND ECC engine can cowork with MTK raw NAND and SPI NAND controller.
+
+properties:
+ compatible:
+ enum:
+ - mediatek,mt2701-ecc
+ - mediatek,mt2712-ecc
+ - mediatek,mt7622-ecc
+
+ reg:
+ items:
+ - description: Base physical address and size of ECC.
+
+ interrupts:
+ items:
+ - description: ECC interrupt
+
+ clocks:
+ maxItems: 1
+
+ clock-names:
+ const: nfiecc_clk
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - clocks
+ - clock-names
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/mt2701-clk.h>
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/interrupt-controller/irq.h>
+
+ soc {
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ bch: ecc@1100e000 {
+ compatible = "mediatek,mt2701-ecc";
+ reg = <0 0x1100e000 0 0x1000>;
+ interrupts = <GIC_SPI 55 IRQ_TYPE_LEVEL_LOW>;
+ clocks = <&pericfg CLK_PERI_NFI_ECC>;
+ clock-names = "nfiecc_clk";
+ };
+ };
diff --git a/Documentation/devicetree/bindings/mtd/mtk-nand.txt b/Documentation/devicetree/bindings/mtd/mtk-nand.txt
deleted file mode 100644
index 839ea2f93d04..000000000000
--- a/Documentation/devicetree/bindings/mtd/mtk-nand.txt
+++ /dev/null
@@ -1,176 +0,0 @@
-MTK SoCs NAND FLASH controller (NFC) DT binding
-
-This file documents the device tree bindings for MTK SoCs NAND controllers.
-The functional split of the controller requires two drivers to operate:
-the nand controller interface driver and the ECC engine driver.
-
-The hardware description for both devices must be captured as device
-tree nodes.
-
-1) NFC NAND Controller Interface (NFI):
-=======================================
-
-The first part of NFC is NAND Controller Interface (NFI) HW.
-Required NFI properties:
-- compatible: Should be one of
- "mediatek,mt2701-nfc",
- "mediatek,mt2712-nfc",
- "mediatek,mt7622-nfc".
-- reg: Base physical address and size of NFI.
-- interrupts: Interrupts of NFI.
-- clocks: NFI required clocks.
-- clock-names: NFI clocks internal name.
-- ecc-engine: Required ECC Engine node.
-- #address-cells: NAND chip index, should be 1.
-- #size-cells: Should be 0.
-
-Example:
-
- nandc: nfi@1100d000 {
- compatible = "mediatek,mt2701-nfc";
- reg = <0 0x1100d000 0 0x1000>;
- interrupts = <GIC_SPI 56 IRQ_TYPE_LEVEL_LOW>;
- clocks = <&pericfg CLK_PERI_NFI>,
- <&pericfg CLK_PERI_NFI_PAD>;
- clock-names = "nfi_clk", "pad_clk";
- ecc-engine = <&bch>;
- #address-cells = <1>;
- #size-cells = <0>;
- };
-
-Platform related properties, should be set in {platform_name}.dts:
-- children nodes: NAND chips.
-
-Children nodes properties:
-- reg: Chip Select Signal, default 0.
- Set as reg = <0>, <1> when need 2 CS.
-Optional:
-- nand-on-flash-bbt: Store BBT on NAND Flash.
-- nand-ecc-mode: the NAND ecc mode (check driver for supported modes)
-- nand-ecc-step-size: Number of data bytes covered by a single ECC step.
- valid values:
- 512 and 1024 on mt2701 and mt2712.
- 512 only on mt7622.
- 1024 is recommended for large page NANDs.
-- nand-ecc-strength: Number of bits to correct per ECC step.
- The valid values that each controller supports:
- mt2701: 4, 6, 8, 10, 12, 14, 16, 18, 20, 22, 24, 28,
- 32, 36, 40, 44, 48, 52, 56, 60.
- mt2712: 4, 6, 8, 10, 12, 14, 16, 18, 20, 22, 24, 28,
- 32, 36, 40, 44, 48, 52, 56, 60, 68, 72, 80.
- mt7622: 4, 6, 8, 10, 12, 14, 16.
- The strength should be calculated as follows:
- E = (S - F) * 8 / B
- S = O / (P / Q)
- E : nand-ecc-strength.
- S : spare size per sector.
- F : FDM size, should be in the range [1,8].
- It is used to store free oob data.
- O : oob size.
- P : page size.
- Q : nand-ecc-step-size.
- B : number of parity bits needed to correct
- 1 bitflip.
- According to MTK NAND controller design,
- this number depends on max ecc step size
- that MTK NAND controller supports.
- If max ecc step size supported is 1024,
- then it should be always 14. And if max
- ecc step size is 512, then it should be
- always 13.
- If the result does not match any one of the listed
- choices above, please select the smaller valid value from
- the list.
- (otherwise the driver will do the adjustment at runtime)
-- pinctrl-names: Default NAND pin GPIO setting name.
-- pinctrl-0: GPIO setting node.
-
-Example:
- &pio {
- nand_pins_default: nanddefault {
- pins_dat {
- pinmux = <MT2701_PIN_111_MSDC0_DAT7__FUNC_NLD7>,
- <MT2701_PIN_112_MSDC0_DAT6__FUNC_NLD6>,
- <MT2701_PIN_114_MSDC0_DAT4__FUNC_NLD4>,
- <MT2701_PIN_118_MSDC0_DAT3__FUNC_NLD3>,
- <MT2701_PIN_121_MSDC0_DAT0__FUNC_NLD0>,
- <MT2701_PIN_120_MSDC0_DAT1__FUNC_NLD1>,
- <MT2701_PIN_113_MSDC0_DAT5__FUNC_NLD5>,
- <MT2701_PIN_115_MSDC0_RSTB__FUNC_NLD8>,
- <MT2701_PIN_119_MSDC0_DAT2__FUNC_NLD2>;
- input-enable;
- drive-strength = <MTK_DRIVE_8mA>;
- bias-pull-up;
- };
-
- pins_we {
- pinmux = <MT2701_PIN_117_MSDC0_CLK__FUNC_NWEB>;
- drive-strength = <MTK_DRIVE_8mA>;
- bias-pull-up = <MTK_PUPD_SET_R1R0_10>;
- };
-
- pins_ale {
- pinmux = <MT2701_PIN_116_MSDC0_CMD__FUNC_NALE>;
- drive-strength = <MTK_DRIVE_8mA>;
- bias-pull-down = <MTK_PUPD_SET_R1R0_10>;
- };
- };
- };
-
- &nandc {
- status = "okay";
- pinctrl-names = "default";
- pinctrl-0 = <&nand_pins_default>;
- nand@0 {
- reg = <0>;
- nand-on-flash-bbt;
- nand-ecc-mode = "hw";
- nand-ecc-strength = <24>;
- nand-ecc-step-size = <1024>;
- };
- };
-
-NAND chip optional subnodes:
-- Partitions, see Documentation/devicetree/bindings/mtd/mtd.yaml
-
-Example:
- nand@0 {
- partitions {
- compatible = "fixed-partitions";
- #address-cells = <1>;
- #size-cells = <1>;
-
- preloader@0 {
- label = "pl";
- read-only;
- reg = <0x00000000 0x00400000>;
- };
- android@00400000 {
- label = "android";
- reg = <0x00400000 0x12c00000>;
- };
- };
- };
-
-2) ECC Engine:
-==============
-
-Required BCH properties:
-- compatible: Should be one of
- "mediatek,mt2701-ecc",
- "mediatek,mt2712-ecc",
- "mediatek,mt7622-ecc".
-- reg: Base physical address and size of ECC.
-- interrupts: Interrupts of ECC.
-- clocks: ECC required clocks.
-- clock-names: ECC clocks internal name.
-
-Example:
-
- bch: ecc@1100e000 {
- compatible = "mediatek,mt2701-ecc";
- reg = <0 0x1100e000 0 0x1000>;
- interrupts = <GIC_SPI 55 IRQ_TYPE_LEVEL_LOW>;
- clocks = <&pericfg CLK_PERI_NFI_ECC>;
- clock-names = "nfiecc_clk";
- };
--
2.25.1

2022-12-08 07:11:28

by Xiangsheng Hou

[permalink] [raw]
Subject: [PATCH v3 6/9] dt-bindings: spi: mtk-snfi: Add read latch latency property

Add mediatek,rx-latch-latency-ns property which adjust data read
latch latency in the unit of nanoseconds.

Signed-off-by: Xiangsheng Hou <[email protected]>
Acked-by: Krzysztof Kozlowski <[email protected]>
---
.../devicetree/bindings/spi/mediatek,spi-mtk-snfi.yaml | 3 +++
1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-snfi.yaml b/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-snfi.yaml
index bab23f1b11fd..1e5e89a693c3 100644
--- a/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-snfi.yaml
+++ b/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-snfi.yaml
@@ -45,6 +45,9 @@ properties:
description: device-tree node of the accompanying ECC engine.
$ref: /schemas/types.yaml#/definitions/phandle

+ mediatek,rx-latch-latency-ns:
+ description: Data read latch latency, unit is nanoseconds.
+
required:
- compatible
- reg
--
2.25.1

2022-12-08 07:12:57

by Xiangsheng Hou

[permalink] [raw]
Subject: [PATCH v3 8/9] arm/arm64: dts: mediatek: Fix existing NAND controller node name

Change the existing node name in order to match NAND controller DT
bindings.

Signed-off-by: Xiangsheng Hou <[email protected]>
---
arch/arm/boot/dts/mt2701.dtsi | 2 +-
arch/arm64/boot/dts/mediatek/mt2712e.dtsi | 2 +-
arch/arm64/boot/dts/mediatek/mt7622.dtsi | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/mt2701.dtsi b/arch/arm/boot/dts/mt2701.dtsi
index b8eba3ba153c..049ed797766b 100644
--- a/arch/arm/boot/dts/mt2701.dtsi
+++ b/arch/arm/boot/dts/mt2701.dtsi
@@ -360,7 +360,7 @@ thermal: thermal@1100b000 {
mediatek,apmixedsys = <&apmixedsys>;
};

- nandc: nfi@1100d000 {
+ nandc: nand-controller@1100d000 {
compatible = "mediatek,mt2701-nfc";
reg = <0 0x1100d000 0 0x1000>;
interrupts = <GIC_SPI 56 IRQ_TYPE_LEVEL_LOW>;
diff --git a/arch/arm64/boot/dts/mediatek/mt2712e.dtsi b/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
index 92212cddd37e..cfbec2a2ed9d 100644
--- a/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
@@ -560,7 +560,7 @@ spi0: spi@1100a000 {
status = "disabled";
};

- nandc: nfi@1100e000 {
+ nandc: nand-controller@1100e000 {
compatible = "mediatek,mt2712-nfc";
reg = <0 0x1100e000 0 0x1000>;
interrupts = <GIC_SPI 96 IRQ_TYPE_LEVEL_LOW>;
diff --git a/arch/arm64/boot/dts/mediatek/mt7622.dtsi b/arch/arm64/boot/dts/mediatek/mt7622.dtsi
index 146e18b5b1f4..d98aa4936092 100644
--- a/arch/arm64/boot/dts/mediatek/mt7622.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt7622.dtsi
@@ -539,7 +539,7 @@ bluetooth {
};
};

- nandc: nfi@1100d000 {
+ nandc: nand-controller@1100d000 {
compatible = "mediatek,mt7622-nfc";
reg = <0 0x1100D000 0 0x1000>;
interrupts = <GIC_SPI 96 IRQ_TYPE_LEVEL_LOW>;
--
2.25.1

2022-12-08 10:00:59

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v3 7/9] dt-bindings: mtd: Split ECC engine with rawnand controller

On 08/12/2022 07:29, Xiangsheng Hou wrote:
> Split MediaTek ECC engine with rawnand controller and convert to
> YAML schema.
>
> Signed-off-by: Xiangsheng Hou <[email protected]>
> ---
> .../bindings/mtd/mediatek,mtk-nfc.yaml | 154 +++++++++++++++
> .../mtd/mediatek,nand-ecc-engine.yaml | 62 ++++++
> .../devicetree/bindings/mtd/mtk-nand.txt | 176 ------------------
> 3 files changed, 216 insertions(+), 176 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/mtd/mediatek,mtk-nfc.yaml
> create mode 100644 Documentation/devicetree/bindings/mtd/mediatek,nand-ecc-engine.yaml
> delete mode 100644 Documentation/devicetree/bindings/mtd/mtk-nand.txt
>
> diff --git a/Documentation/devicetree/bindings/mtd/mediatek,mtk-nfc.yaml b/Documentation/devicetree/bindings/mtd/mediatek,mtk-nfc.yaml
> new file mode 100644
> index 000000000000..eb1a44c7ae4e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/mediatek,mtk-nfc.yaml
> @@ -0,0 +1,154 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mtd/mediatek,mtk-nfc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MediaTek(MTK) SoCs raw NAND FLASH controller (NFC)
> +
> +maintainers:
> + - Xiangsheng Hou <[email protected]>
> +
> +properties:
> + compatible:
> + enum:
> + - mediatek,mt2701-nfc
> + - mediatek,mt2712-nfc
> + - mediatek,mt7622-nfc
> +
> + reg:
> + items:
> + - description: Base physical address and size of NFI.
> +
> + interrupts:
> + items:
> + - description: NFI interrupt
> +
> + clocks:
> + items:
> + - description: clock used for the controller
> + - description: clock used for the pad
> +
> + clock-names:
> + items:
> + - const: nfi_clk
> + - const: pad_clk
> +
> + ecc-engine:
> + description: device-tree node of the required ECC engine.
> + $ref: /schemas/types.yaml#/definitions/phandle
> +
> +patternProperties:
> + "^nand@[a-f0-9]$":
> + type: object

This should be instead:
$ref: nand-chip.yaml#
unevaluatedProperties: false

and then properties below (due to current dtschema limitations) should
list properties from nand-controller.yaml:

nand-on-flash-bbt: true

Optionally, we could create additional schema - nand-controller-chip,
which would be referenced directly by nand-controller and itself would
ref nand-chip.

> + properties:
> + reg:
> + minimum: 0

no need, 0 is the minimum.

> + maximum: 1
> + nand-ecc-mode:
> + const: hw
> +
> +allOf:
> + - $ref: nand-controller.yaml#
> +
> + - if:

Best regards,
Krzysztof

2022-12-08 10:15:12

by Miquel Raynal

[permalink] [raw]
Subject: Re: [PATCH v3 7/9] dt-bindings: mtd: Split ECC engine with rawnand controller

Hi Krzysztof,

[email protected] wrote on Thu, 8 Dec 2022 10:44:17 +0100:

> On 08/12/2022 07:29, Xiangsheng Hou wrote:
> > Split MediaTek ECC engine with rawnand controller and convert to
> > YAML schema.
> >
> > Signed-off-by: Xiangsheng Hou <[email protected]>
> > ---
> > .../bindings/mtd/mediatek,mtk-nfc.yaml | 154 +++++++++++++++
> > .../mtd/mediatek,nand-ecc-engine.yaml | 62 ++++++
> > .../devicetree/bindings/mtd/mtk-nand.txt | 176 ------------------
> > 3 files changed, 216 insertions(+), 176 deletions(-)
> > create mode 100644 Documentation/devicetree/bindings/mtd/mediatek,mtk-nfc.yaml
> > create mode 100644 Documentation/devicetree/bindings/mtd/mediatek,nand-ecc-engine.yaml
> > delete mode 100644 Documentation/devicetree/bindings/mtd/mtk-nand.txt
> >
> > diff --git a/Documentation/devicetree/bindings/mtd/mediatek,mtk-nfc.yaml b/Documentation/devicetree/bindings/mtd/mediatek,mtk-nfc.yaml
> > new file mode 100644
> > index 000000000000..eb1a44c7ae4e
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mtd/mediatek,mtk-nfc.yaml
> > @@ -0,0 +1,154 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/mtd/mediatek,mtk-nfc.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: MediaTek(MTK) SoCs raw NAND FLASH controller (NFC)
> > +
> > +maintainers:
> > + - Xiangsheng Hou <[email protected]>
> > +
> > +properties:
> > + compatible:
> > + enum:
> > + - mediatek,mt2701-nfc
> > + - mediatek,mt2712-nfc
> > + - mediatek,mt7622-nfc
> > +
> > + reg:
> > + items:
> > + - description: Base physical address and size of NFI.
> > +
> > + interrupts:
> > + items:
> > + - description: NFI interrupt
> > +
> > + clocks:
> > + items:
> > + - description: clock used for the controller
> > + - description: clock used for the pad
> > +
> > + clock-names:
> > + items:
> > + - const: nfi_clk
> > + - const: pad_clk
> > +
> > + ecc-engine:
> > + description: device-tree node of the required ECC engine.
> > + $ref: /schemas/types.yaml#/definitions/phandle
> > +
> > +patternProperties:
> > + "^nand@[a-f0-9]$":
> > + type: object
>
> This should be instead:
> $ref: nand-chip.yaml#
> unevaluatedProperties: false
>
> and then properties below (due to current dtschema limitations) should
> list properties from nand-controller.yaml:
>
> nand-on-flash-bbt: true
>
> Optionally, we could create additional schema - nand-controller-chip,
> which would be referenced directly by nand-controller and itself would
> ref nand-chip.

Isn't this enough? (in linux-next)
https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git/tree/Documentation/devicetree/bindings/mtd/nand-controller.yaml?h=mtd/next#n54

Thanks,
Miquèl

2022-12-08 10:44:29

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v3 7/9] dt-bindings: mtd: Split ECC engine with rawnand controller

On 08/12/2022 11:00, Miquel Raynal wrote:
> Hi Krzysztof,
>
> [email protected] wrote on Thu, 8 Dec 2022 10:44:17 +0100:
>
>> On 08/12/2022 07:29, Xiangsheng Hou wrote:
>>> Split MediaTek ECC engine with rawnand controller and convert to
>>> YAML schema.
>>>
>>> Signed-off-by: Xiangsheng Hou <[email protected]>
>>> ---
>>> .../bindings/mtd/mediatek,mtk-nfc.yaml | 154 +++++++++++++++
>>> .../mtd/mediatek,nand-ecc-engine.yaml | 62 ++++++
>>> .../devicetree/bindings/mtd/mtk-nand.txt | 176 ------------------
>>> 3 files changed, 216 insertions(+), 176 deletions(-)
>>> create mode 100644 Documentation/devicetree/bindings/mtd/mediatek,mtk-nfc.yaml
>>> create mode 100644 Documentation/devicetree/bindings/mtd/mediatek,nand-ecc-engine.yaml
>>> delete mode 100644 Documentation/devicetree/bindings/mtd/mtk-nand.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/mtd/mediatek,mtk-nfc.yaml b/Documentation/devicetree/bindings/mtd/mediatek,mtk-nfc.yaml
>>> new file mode 100644
>>> index 000000000000..eb1a44c7ae4e
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/mtd/mediatek,mtk-nfc.yaml
>>> @@ -0,0 +1,154 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/mtd/mediatek,mtk-nfc.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: MediaTek(MTK) SoCs raw NAND FLASH controller (NFC)
>>> +
>>> +maintainers:
>>> + - Xiangsheng Hou <[email protected]>
>>> +
>>> +properties:
>>> + compatible:
>>> + enum:
>>> + - mediatek,mt2701-nfc
>>> + - mediatek,mt2712-nfc
>>> + - mediatek,mt7622-nfc
>>> +
>>> + reg:
>>> + items:
>>> + - description: Base physical address and size of NFI.
>>> +
>>> + interrupts:
>>> + items:
>>> + - description: NFI interrupt
>>> +
>>> + clocks:
>>> + items:
>>> + - description: clock used for the controller
>>> + - description: clock used for the pad
>>> +
>>> + clock-names:
>>> + items:
>>> + - const: nfi_clk
>>> + - const: pad_clk
>>> +
>>> + ecc-engine:
>>> + description: device-tree node of the required ECC engine.
>>> + $ref: /schemas/types.yaml#/definitions/phandle
>>> +
>>> +patternProperties:
>>> + "^nand@[a-f0-9]$":
>>> + type: object
>>
>> This should be instead:
>> $ref: nand-chip.yaml#
>> unevaluatedProperties: false
>>
>> and then properties below (due to current dtschema limitations) should
>> list properties from nand-controller.yaml:
>>
>> nand-on-flash-bbt: true
>>
>> Optionally, we could create additional schema - nand-controller-chip,
>> which would be referenced directly by nand-controller and itself would
>> ref nand-chip.
>
> Isn't this enough? (in linux-next)
> https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git/tree/Documentation/devicetree/bindings/mtd/nand-controller.yaml?h=mtd/next#n54

No, I tested it and it does not work as intended. In this particular
case. I think this is a limitation of dtschema, because binding itself
looks fine. The problem is that you have:
1. mtk-nfc having nand@ children. mtk-nfc references nand-controller
which brings these children.
2. However nand-controller while bringing these children does two things:
a. ref: nand-chip
b. add more propeties

3. The mtk-nfc must further extend the nand@ child.
4. If you add "unevaluatedProperties: false" you notice warnings of
unevaluated propertie from nand-controller children.

Best regards,
Krzysztof

2022-12-09 10:00:46

by Miquel Raynal

[permalink] [raw]
Subject: Re: [PATCH v3 7/9] dt-bindings: mtd: Split ECC engine with rawnand controller

Hi Krzysztof,

[email protected] wrote on Thu, 8 Dec 2022 11:27:27 +0100:

> On 08/12/2022 11:00, Miquel Raynal wrote:
> > Hi Krzysztof,
> >
> > [email protected] wrote on Thu, 8 Dec 2022 10:44:17 +0100:
> >
> >> On 08/12/2022 07:29, Xiangsheng Hou wrote:
> >>> Split MediaTek ECC engine with rawnand controller and convert to
> >>> YAML schema.
> >>>
> >>> Signed-off-by: Xiangsheng Hou <[email protected]>
> >>> ---
> >>> .../bindings/mtd/mediatek,mtk-nfc.yaml | 154 +++++++++++++++
> >>> .../mtd/mediatek,nand-ecc-engine.yaml | 62 ++++++
> >>> .../devicetree/bindings/mtd/mtk-nand.txt | 176 ------------------
> >>> 3 files changed, 216 insertions(+), 176 deletions(-)
> >>> create mode 100644 Documentation/devicetree/bindings/mtd/mediatek,mtk-nfc.yaml
> >>> create mode 100644 Documentation/devicetree/bindings/mtd/mediatek,nand-ecc-engine.yaml
> >>> delete mode 100644 Documentation/devicetree/bindings/mtd/mtk-nand.txt
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/mtd/mediatek,mtk-nfc.yaml b/Documentation/devicetree/bindings/mtd/mediatek,mtk-nfc.yaml
> >>> new file mode 100644
> >>> index 000000000000..eb1a44c7ae4e
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/mtd/mediatek,mtk-nfc.yaml
> >>> @@ -0,0 +1,154 @@
> >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >>> +%YAML 1.2
> >>> +---
> >>> +$id: http://devicetree.org/schemas/mtd/mediatek,mtk-nfc.yaml#
> >>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >>> +
> >>> +title: MediaTek(MTK) SoCs raw NAND FLASH controller (NFC)
> >>> +
> >>> +maintainers:
> >>> + - Xiangsheng Hou <[email protected]>
> >>> +
> >>> +properties:
> >>> + compatible:
> >>> + enum:
> >>> + - mediatek,mt2701-nfc
> >>> + - mediatek,mt2712-nfc
> >>> + - mediatek,mt7622-nfc
> >>> +
> >>> + reg:
> >>> + items:
> >>> + - description: Base physical address and size of NFI.
> >>> +
> >>> + interrupts:
> >>> + items:
> >>> + - description: NFI interrupt
> >>> +
> >>> + clocks:
> >>> + items:
> >>> + - description: clock used for the controller
> >>> + - description: clock used for the pad
> >>> +
> >>> + clock-names:
> >>> + items:
> >>> + - const: nfi_clk
> >>> + - const: pad_clk
> >>> +
> >>> + ecc-engine:
> >>> + description: device-tree node of the required ECC engine.
> >>> + $ref: /schemas/types.yaml#/definitions/phandle
> >>> +
> >>> +patternProperties:
> >>> + "^nand@[a-f0-9]$":
> >>> + type: object
> >>
> >> This should be instead:
> >> $ref: nand-chip.yaml#
> >> unevaluatedProperties: false
> >>
> >> and then properties below (due to current dtschema limitations) should
> >> list properties from nand-controller.yaml:
> >>
> >> nand-on-flash-bbt: true
> >>
> >> Optionally, we could create additional schema - nand-controller-chip,
> >> which would be referenced directly by nand-controller and itself would
> >> ref nand-chip.
> >
> > Isn't this enough? (in linux-next)
> > https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git/tree/Documentation/devicetree/bindings/mtd/nand-controller.yaml?h=mtd/next#n54
>
> No, I tested it and it does not work as intended. In this particular
> case. I think this is a limitation of dtschema, because binding itself
> looks fine. The problem is that you have:
> 1. mtk-nfc having nand@ children. mtk-nfc references nand-controller
> which brings these children.
> 2. However nand-controller while bringing these children does two things:
> a. ref: nand-chip
> b. add more propeties
>
> 3. The mtk-nfc must further extend the nand@ child.
> 4. If you add "unevaluatedProperties: false" you notice warnings of
> unevaluated propertie from nand-controller children.

Thanks for the details. Any chances this can eventually be fixed at
dt-schema level?

Thanks,
Miquèl