[PATCH v2 0/9] Add MediaTek MT7986 SPI NAND and ECC support

[PATCH v2 0/9] Add MediaTek MT7986 SPI NAND and ECC support

[Xiangsheng Hou]

This patch series add MediaTek MT7986 SPI NAND and ECC controller
support, split ECC engine with rawnand controller in bindings and
hange to YAML schema.

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: Add snfi support for MT7986 IC
  spi: mtk-snfi: Change default page format to setup default setting
  spi: mtk-snfi: Add optional nfi_hclk which 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
  dt-bindings: mtd: ecc-mtk: Add compatible for MT7986

 .../bindings/mtd/mediatek,mtk-nfc.yaml        | 171 +++++++++++++++++
 .../mtd/mediatek,nand-ecc-engine.yaml         |  63 +++++++
 .../devicetree/bindings/mtd/mtk-nand.txt      | 176 ------------------
 .../bindings/spi/mediatek,spi-mtk-snfi.yaml   |  58 +++++-
 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                    |  18 ++
 drivers/spi/spi-mtk-snfi.c                    |  66 ++++++-
 9 files changed, 366 insertions(+), 192 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

[PATCH v2 1/9] spi: mtk-snfi: Add snfi support for MT7986 IC

[Xiangsheng Hou]

Add snfi support for MT7986 IC.

Signed-off-by: Xiangsheng Hou <xiangsheng.hou@mediatek.com>
---
 drivers/spi/spi-mtk-snfi.c | 29 +++++++++++++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-mtk-snfi.c b/drivers/spi/spi-mtk-snfi.c
index d66bf9762557..fa8412ba20e2 100644
--- a/drivers/spi/spi-mtk-snfi.c
+++ b/drivers/spi/spi-mtk-snfi.c
@@ -126,7 +126,8 @@
 #define STR_DATA BIT(0)
 
 #define NFI_STA 0x060
-#define NFI_NAND_FSM GENMASK(28, 24)
+#define NFI_NAND_FSM_7622 GENMASK(28, 24)
+#define NFI_NAND_FSM_7986 GENMASK(29, 23)
 #define NFI_FSM GENMASK(19, 16)
 #define READ_EMPTY BIT(12)
 
@@ -158,6 +159,7 @@
 #define MAS_WR GENMASK(5, 3)
 #define MAS_RDDLY GENMASK(2, 0)
 #define NFI_MASTERSTA_MASK_7622 (MAS_ADDR | MAS_RD | MAS_WR | MAS_RDDLY)
+#define NFI_MASTERSTA_MASK_7986 3
 
 // SNFI registers
 #define SNF_MAC_CTL 0x500
@@ -220,6 +222,11 @@
 
 static const u8 mt7622_spare_sizes[] = { 16, 26, 27, 28 };
 
+static const u8 mt7986_spare_sizes[] = {
+	16, 26, 27, 28, 32, 36, 40, 44, 48, 49, 50, 51, 52, 62, 61, 63, 64, 67,
+	74
+};
+
 struct mtk_snand_caps {
 	u16 sector_size;
 	u16 max_sectors;
@@ -230,6 +237,7 @@ struct mtk_snand_caps {
 	bool bbm_swap;
 	bool empty_page_check;
 	u32 mastersta_mask;
+	u32 nandfsm_mask;
 
 	const u8 *spare_sizes;
 	u32 num_spare_size;
@@ -244,6 +252,7 @@ static const struct mtk_snand_caps mt7622_snand_caps = {
 	.bbm_swap = false,
 	.empty_page_check = false,
 	.mastersta_mask = NFI_MASTERSTA_MASK_7622,
+	.nandfsm_mask = NFI_NAND_FSM_7622,
 	.spare_sizes = mt7622_spare_sizes,
 	.num_spare_size = ARRAY_SIZE(mt7622_spare_sizes)
 };
@@ -257,10 +266,25 @@ static const struct mtk_snand_caps mt7629_snand_caps = {
 	.bbm_swap = true,
 	.empty_page_check = false,
 	.mastersta_mask = NFI_MASTERSTA_MASK_7622,
+	.nandfsm_mask = NFI_NAND_FSM_7622,
 	.spare_sizes = mt7622_spare_sizes,
 	.num_spare_size = ARRAY_SIZE(mt7622_spare_sizes)
 };
 
+static const struct mtk_snand_caps mt7986_snand_caps = {
+	.sector_size = 1024,
+	.max_sectors = 8,
+	.fdm_size = 8,
+	.fdm_ecc_size = 1,
+	.fifo_size = 64,
+	.bbm_swap = true,
+	.empty_page_check = true,
+	.mastersta_mask = NFI_MASTERSTA_MASK_7986,
+	.nandfsm_mask = NFI_NAND_FSM_7986,
+	.spare_sizes = mt7986_spare_sizes,
+	.num_spare_size = ARRAY_SIZE(mt7986_spare_sizes)
+};
+
 struct mtk_snand_conf {
 	size_t page_size;
 	size_t oob_size;
@@ -360,7 +384,7 @@ static int mtk_nfi_reset(struct mtk_snand *snf)
 	}
 
 	ret = readl_poll_timeout(snf->nfi_base + NFI_STA, val,
-				 !(val & (NFI_FSM | NFI_NAND_FSM)), 0,
+				 !(val & (NFI_FSM | snf->caps->nandfsm_mask)), 0,
 				 SNFI_POLL_INTERVAL);
 	if (ret) {
 		dev_err(snf->dev, "Failed to reset NFI\n");
@@ -1295,6 +1319,7 @@ static irqreturn_t mtk_snand_irq(int irq, void *id)
 static const struct of_device_id mtk_snand_ids[] = {
 	{ .compatible = "mediatek,mt7622-snand", .data = &mt7622_snand_caps },
 	{ .compatible = "mediatek,mt7629-snand", .data = &mt7629_snand_caps },
+	{ .compatible = "mediatek,mt7986-snand", .data = &mt7986_snand_caps },
 	{},
 };
 
-- 
2.25.1

Re: [PATCH v2 1/9] spi: mtk-snfi: Add snfi support for MT7986 IC

[AngeloGioacchino Del Regno]

Il 05/12/22 07:57, Xiangsheng Hou ha scritto:
> Add snfi support for MT7986 IC.
> 
> Signed-off-by: Xiangsheng Hou <xiangsheng.hou@mediatek.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

[PATCH v2 2/9] spi: mtk-snfi: Change default page format to setup default setting

[Xiangsheng Hou]

Change default page format to setup default setting since the sector
size 1024 on MT7986 will lead to probe fail.

Signed-off-by: Xiangsheng Hou <xiangsheng.hou@mediatek.com>
---
 drivers/spi/spi-mtk-snfi.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/spi/spi-mtk-snfi.c b/drivers/spi/spi-mtk-snfi.c
index fa8412ba20e2..719fc6f53ab1 100644
--- a/drivers/spi/spi-mtk-snfi.c
+++ b/drivers/spi/spi-mtk-snfi.c
@@ -1430,8 +1430,7 @@ static int mtk_snand_probe(struct platform_device *pdev)
 
 	// setup an initial page format for ops matching page_cache_op template
 	// before ECC is called.
-	ret = mtk_snand_setup_pagefmt(ms, ms->caps->sector_size,
-				      ms->caps->spare_sizes[0]);
+	ret = mtk_snand_setup_pagefmt(ms, SZ_2K, SZ_64);
 	if (ret) {
 		dev_err(ms->dev, "failed to set initial page format\n");
 		goto disable_clk;
-- 
2.25.1

[PATCH v2 3/9] spi: mtk-snfi: Add optional nfi_hclk which needed for MT7986

[Xiangsheng Hou]

Add optional nfi_hclk which needed for MT7986.

Signed-off-by: Xiangsheng Hou <xiangsheng.hou@mediatek.com>
---
 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

Re: [PATCH v2 3/9] spi: mtk-snfi: Add optional nfi_hclk which needed for MT7986

[AngeloGioacchino Del Regno]

Il 05/12/22 07:57, Xiangsheng Hou ha scritto:
> Add optional nfi_hclk which needed for MT7986.
> 
> Signed-off-by: Xiangsheng Hou <xiangsheng.hou@mediatek.com>

Is there any operation for which you need NFI_HCLK enabled, but at the same time
PAD_CLK and/or NFI_CLK can be disabled?

If NFI_HCLK and NFI_CLK must always be ON at the same time, adding this clock to
spi-mtk-snfi.c is *not* an optimal way of doing things: you can, at this point,
set NFI_HCLK as parent of NFI_CLK in the MT7986 clock driver instead, without
making any addition to this driver at all.

Regards,
Angelo

Re: [PATCH v2 3/9] spi: mtk-snfi: Add optional nfi_hclk which needed for MT7986

[Xiangsheng Hou (侯祥胜)]

Hi Angelo,

On Mon, 2022-12-05 at 15:21 +0100, AngeloGioacchino Del Regno wrote:
> Il 05/12/22 07:57, Xiangsheng Hou ha scritto:
> > Add optional nfi_hclk which needed for MT7986.
> > 
> > Signed-off-by: Xiangsheng Hou <xiangsheng.hou@mediatek.com>
> 
> Is there any operation for which you need NFI_HCLK enabled, but at
> the same time
> PAD_CLK and/or NFI_CLK can be disabled?

No, for the new IP design on MT7986, will need the
PAD_CLK/NFI_CLK/NFI_HCLK enabled at the same time.

> If NFI_HCLK and NFI_CLK must always be ON at the same time, adding
> this clock to
> spi-mtk-snfi.c is *not* an optimal way of doing things: you can, at
> this point,
> set NFI_HCLK as parent of NFI_CLK in the MT7986 clock driver instead,
> without
> making any addition to this driver at all.

For some IC, there may have only NFI_CLK/PAD_CLK, and have no NFI_HCLK,
this rely on IC design.

Thanks
Xiangsheng Hou

Re: [PATCH v2 3/9] spi: mtk-snfi: Add optional nfi_hclk which needed for MT7986

[AngeloGioacchino Del Regno]

Il 07/12/22 02:42, Xiangsheng Hou (侯祥胜) ha scritto:
> Hi Angelo,
> 
> On Mon, 2022-12-05 at 15:21 +0100, AngeloGioacchino Del Regno wrote:
>> Il 05/12/22 07:57, Xiangsheng Hou ha scritto:
>>> Add optional nfi_hclk which needed for MT7986.
>>>
>>> Signed-off-by: Xiangsheng Hou <xiangsheng.hou@mediatek.com>
>>
>> Is there any operation for which you need NFI_HCLK enabled, but at
>> the same time
>> PAD_CLK and/or NFI_CLK can be disabled?
> 
> No, for the new IP design on MT7986, will need the
> PAD_CLK/NFI_CLK/NFI_HCLK enabled at the same time.
> 
>> If NFI_HCLK and NFI_CLK must always be ON at the same time, adding
>> this clock to
>> spi-mtk-snfi.c is *not* an optimal way of doing things: you can, at
>> this point,
>> set NFI_HCLK as parent of NFI_CLK in the MT7986 clock driver instead,
>> without
>> making any addition to this driver at all.
> 
> For some IC, there may have only NFI_CLK/PAD_CLK, and have no NFI_HCLK,
> this rely on IC design.
> 

I've just checked clk-mt7986-infracfg and we can't reparent NFI1_CK, nor SPINFI1_CK
as they have xxxx_sel parents already, which are not common with the HCK.

You're right, the addition of the nfi_hclk clock is needed, which means that for
this commit, you get my


Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>


P.S.: Thanks for clarifying!

Regards,
Angelo

[PATCH v2 4/9] mtd: nand: ecc-mtk: Add ECC support fot MT7986 IC

[Xiangsheng Hou]

Add ECC support fot MT7986 IC.

Signed-off-by: Xiangsheng Hou <xiangsheng.hou@mediatek.com>
---
 drivers/mtd/nand/ecc-mtk.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/mtd/nand/ecc-mtk.c b/drivers/mtd/nand/ecc-mtk.c
index 9f9b201fe706..c2f6cfa76a04 100644
--- a/drivers/mtd/nand/ecc-mtk.c
+++ b/drivers/mtd/nand/ecc-mtk.c
@@ -79,6 +79,10 @@ static const u8 ecc_strength_mt7622[] = {
 	4, 6, 8, 10, 12
 };
 
+static const u8 ecc_strength_mt7986[] = {
+	4, 6, 8, 10, 12, 14, 16, 18, 20, 22, 24
+};
+
 enum mtk_ecc_regs {
 	ECC_ENCPAR00,
 	ECC_ENCIRQ_EN,
@@ -483,6 +487,17 @@ static const struct mtk_ecc_caps mtk_ecc_caps_mt7622 = {
 	.pg_irq_sel = 0,
 };
 
+static const struct mtk_ecc_caps mtk_ecc_caps_mt7986 = {
+	.err_mask = 0x1f,
+	.err_shift = 8,
+	.ecc_strength = ecc_strength_mt7986,
+	.ecc_regs = mt2712_ecc_regs,
+	.num_ecc_strength = 11,
+	.ecc_mode_shift = 5,
+	.parity_bits = 14,
+	.pg_irq_sel = 1,
+};
+
 static const struct of_device_id mtk_ecc_dt_match[] = {
 	{
 		.compatible = "mediatek,mt2701-ecc",
@@ -493,6 +508,9 @@ static const struct of_device_id mtk_ecc_dt_match[] = {
 	}, {
 		.compatible = "mediatek,mt7622-ecc",
 		.data = &mtk_ecc_caps_mt7622,
+	}, {
+		.compatible = "mediatek,mt7986-ecc",
+		.data = &mtk_ecc_caps_mt7986,
 	},
 	{},
 };
-- 
2.25.1

Re: [PATCH v2 4/9] mtd: nand: ecc-mtk: Add ECC support fot MT7986 IC

[AngeloGioacchino Del Regno]

Il 05/12/22 07:57, Xiangsheng Hou ha scritto:
> Add ECC support fot MT7986 IC.
> 
> Signed-off-by: Xiangsheng Hou <xiangsheng.hou@mediatek.com>
> ---
>   drivers/mtd/nand/ecc-mtk.c | 18 ++++++++++++++++++
>   1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/mtd/nand/ecc-mtk.c b/drivers/mtd/nand/ecc-mtk.c
> index 9f9b201fe706..c2f6cfa76a04 100644
> --- a/drivers/mtd/nand/ecc-mtk.c
> +++ b/drivers/mtd/nand/ecc-mtk.c
> @@ -79,6 +79,10 @@ static const u8 ecc_strength_mt7622[] = {
>   	4, 6, 8, 10, 12
>   };
>   
> +static const u8 ecc_strength_mt7986[] = {
> +	4, 6, 8, 10, 12, 14, 16, 18, 20, 22, 24
> +};
> +
>   enum mtk_ecc_regs {
>   	ECC_ENCPAR00,
>   	ECC_ENCIRQ_EN,
> @@ -483,6 +487,17 @@ static const struct mtk_ecc_caps mtk_ecc_caps_mt7622 = {
>   	.pg_irq_sel = 0,
>   };
>   
> +static const struct mtk_ecc_caps mtk_ecc_caps_mt7986 = {
> +	.err_mask = 0x1f,

Can't we use GENMASK() to define err_mask instead?

#define MT7986_ERRNUM	GENMASK(4, 0)

P.S.: Did I get that right? Is that referred to the ERRNUM(x) bits?

Regards,
Angelo

Re: [PATCH v2 4/9] mtd: nand: ecc-mtk: Add ECC support fot MT7986 IC

[Xiangsheng Hou (侯祥胜)]

Hi Angelo,

On Mon, 2022-12-05 at 15:21 +0100, AngeloGioacchino Del Regno wrote:
> Il 05/12/22 07:57, Xiangsheng Hou ha scritto:
> > Add ECC support fot MT7986 IC.
> > 
> > Signed-off-by: Xiangsheng Hou <xiangsheng.hou@mediatek.com>
> > ---
> >   drivers/mtd/nand/ecc-mtk.c | 18 ++++++++++++++++++
> >   1 file changed, 18 insertions(+)
> > 
> > diff --git a/drivers/mtd/nand/ecc-mtk.c b/drivers/mtd/nand/ecc-
> > mtk.c
> > index 9f9b201fe706..c2f6cfa76a04 100644
> > --- a/drivers/mtd/nand/ecc-mtk.c
> > +++ b/drivers/mtd/nand/ecc-mtk.c
> > @@ -79,6 +79,10 @@ static const u8 ecc_strength_mt7622[] = {
> >   	4, 6, 8, 10, 12
> >   };
> >   
> > +static const u8 ecc_strength_mt7986[] = {
> > +	4, 6, 8, 10, 12, 14, 16, 18, 20, 22, 24
> > +};
> > +
> >   enum mtk_ecc_regs {
> >   	ECC_ENCPAR00,
> >   	ECC_ENCIRQ_EN,
> > @@ -483,6 +487,17 @@ static const struct mtk_ecc_caps
> > mtk_ecc_caps_mt7622 = {
> >   	.pg_irq_sel = 0,
> >   };
> >   
> > +static const struct mtk_ecc_caps mtk_ecc_caps_mt7986 = {
> > +	.err_mask = 0x1f,
> 
> Can't we use GENMASK() to define err_mask instead?
> 
> #define MT7986_ERRNUM	GENMASK(4, 0)
> 
> P.S.: Did I get that right? Is that referred to the ERRNUM(x) bits

Yes, you are right.
I will change like
#define ECC_ERRMASK(x) GENMASK(x, 0),
since other IC driver data will use 0x3f and 0x7f err_mask.


Thanks
Xiangsheng Hou

Re: [PATCH v2 4/9] mtd: nand: ecc-mtk: Add ECC support fot MT7986 IC

[AngeloGioacchino Del Regno]

Il 06/12/22 10:04, Xiangsheng Hou (侯祥胜) ha scritto:
> Hi Angelo,
> 
> On Mon, 2022-12-05 at 15:21 +0100, AngeloGioacchino Del Regno wrote:
>> Il 05/12/22 07:57, Xiangsheng Hou ha scritto:
>>> Add ECC support fot MT7986 IC.
>>>
>>> Signed-off-by: Xiangsheng Hou <xiangsheng.hou@mediatek.com>
>>> ---
>>>    drivers/mtd/nand/ecc-mtk.c | 18 ++++++++++++++++++
>>>    1 file changed, 18 insertions(+)
>>>
>>> diff --git a/drivers/mtd/nand/ecc-mtk.c b/drivers/mtd/nand/ecc-
>>> mtk.c
>>> index 9f9b201fe706..c2f6cfa76a04 100644
>>> --- a/drivers/mtd/nand/ecc-mtk.c
>>> +++ b/drivers/mtd/nand/ecc-mtk.c
>>> @@ -79,6 +79,10 @@ static const u8 ecc_strength_mt7622[] = {
>>>    	4, 6, 8, 10, 12
>>>    };
>>>    
>>> +static const u8 ecc_strength_mt7986[] = {
>>> +	4, 6, 8, 10, 12, 14, 16, 18, 20, 22, 24
>>> +};
>>> +
>>>    enum mtk_ecc_regs {
>>>    	ECC_ENCPAR00,
>>>    	ECC_ENCIRQ_EN,
>>> @@ -483,6 +487,17 @@ static const struct mtk_ecc_caps
>>> mtk_ecc_caps_mt7622 = {
>>>    	.pg_irq_sel = 0,
>>>    };
>>>    
>>> +static const struct mtk_ecc_caps mtk_ecc_caps_mt7986 = {
>>> +	.err_mask = 0x1f,
>>
>> Can't we use GENMASK() to define err_mask instead?
>>
>> #define MT7986_ERRNUM	GENMASK(4, 0)
>>
>> P.S.: Did I get that right? Is that referred to the ERRNUM(x) bits
> 
> Yes, you are right.
> I will change like
> #define ECC_ERRMASK(x) GENMASK(x, 0),
> since other IC driver data will use 0x3f and 0x7f err_mask.
> 

I would prefer, instead, something like

#define MT7986_ERRNUM	GENMASK(....)
#define MT7622_ERRNUM	GENMASK(....)
#define MT.... (etc)

instead of a macro calling another macro.

Regards,
Angelo

Re: [PATCH v2 4/9] mtd: nand: ecc-mtk: Add ECC support fot MT7986 IC

[Xiangsheng Hou (侯祥胜)]

On Tue, 2022-12-06 at 13:22 +0100, AngeloGioacchino Del Regno wrote:
> Il 06/12/22 10:04, Xiangsheng Hou (侯祥胜) ha scritto:
> > Hi Angelo,
> > 
> > On Mon, 2022-12-05 at 15:21 +0100, AngeloGioacchino Del Regno
> > wrote:
> > > Il 05/12/22 07:57, Xiangsheng Hou ha scritto:
> > > > Add ECC support fot MT7986 IC.
> > > > 
> > > > Signed-off-by: Xiangsheng Hou <xiangsheng.hou@mediatek.com>
> > > > ---
> > > >    drivers/mtd/nand/ecc-mtk.c | 18 ++++++++++++++++++
> > > >    1 file changed, 18 insertions(+)
> > > > 
> > > > diff --git a/drivers/mtd/nand/ecc-mtk.c b/drivers/mtd/nand/ecc-
> > > > mtk.c
> > > > index 9f9b201fe706..c2f6cfa76a04 100644
> > > > --- a/drivers/mtd/nand/ecc-mtk.c
> > > > +++ b/drivers/mtd/nand/ecc-mtk.c
> > > > @@ -79,6 +79,10 @@ static const u8 ecc_strength_mt7622[] = {
> > > >    	4, 6, 8, 10, 12
> > > >    };
> > > >    
> > > > +static const u8 ecc_strength_mt7986[] = {
> > > > +	4, 6, 8, 10, 12, 14, 16, 18, 20, 22, 24
> > > > +};
> > > > +
> > > >    enum mtk_ecc_regs {
> > > >    	ECC_ENCPAR00,
> > > >    	ECC_ENCIRQ_EN,
> > > > @@ -483,6 +487,17 @@ static const struct mtk_ecc_caps
> > > > mtk_ecc_caps_mt7622 = {
> > > >    	.pg_irq_sel = 0,
> > > >    };
> > > >    
> > > > +static const struct mtk_ecc_caps mtk_ecc_caps_mt7986 = {
> > > > +	.err_mask = 0x1f,
> > > 
> > > Can't we use GENMASK() to define err_mask instead?
> > > 
> > > #define MT7986_ERRNUM	GENMASK(4, 0)
> > > 
> > > P.S.: Did I get that right? Is that referred to the ERRNUM(x)
> > > bits
> > 
> > Yes, you are right.
> > I will change like
> > #define ECC_ERRMASK(x) GENMASK(x, 0),
> > since other IC driver data will use 0x3f and 0x7f err_mask.
> > 
> 
> I would prefer, instead, something like
> 
> #define MT7986_ERRNUM	GENMASK(....)
> #define MT7622_ERRNUM	GENMASK(....)
> #define MT.... (etc)
> 
> instead of a macro calling another macro.

Will do.

Thanks
Xiangsheng Hou

[PATCH v2 5/9] dt-bindings: spi: mtk-snfi: Add compatible for MT7986

[Xiangsheng Hou]

1. Add dt-bindings documentation of SPI NAND controller
for MediaTek MT7986 SoC platform.
2. Add optional nfi_hclk property which needed for MT7986.

Signed-off-by: Xiangsheng Hou <xiangsheng.hou@mediatek.com>
---
 .../bindings/spi/mediatek,spi-mtk-snfi.yaml   | 51 +++++++++++++++----
 1 file changed, 42 insertions(+), 9 deletions(-)

diff --git a/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-snfi.yaml b/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-snfi.yaml
index 6e6e02c91780..bab23f1b11fd 100644
--- a/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-snfi.yaml
+++ b/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-snfi.yaml
@@ -18,14 +18,12 @@ description: |
   using the accompanying ECC engine. There should be only one spi
   slave device following generic spi bindings.
 
-allOf:
-  - $ref: /schemas/spi/spi-controller.yaml#
-
 properties:
   compatible:
     enum:
       - mediatek,mt7622-snand
       - mediatek,mt7629-snand
+      - mediatek,mt7986-snand
 
   reg:
     items:
@@ -36,14 +34,12 @@ properties:
       - description: NFI interrupt
 
   clocks:
-    items:
-      - description: clock used for the controller
-      - description: clock used for the SPI bus
+    minItems: 2
+    maxItems: 3
 
   clock-names:
-    items:
-      - const: nfi_clk
-      - const: pad_clk
+    minItems: 2
+    maxItems: 3
 
   nand-ecc-engine:
     description: device-tree node of the accompanying ECC engine.
@@ -57,6 +53,43 @@ required:
   - clock-names
   - nand-ecc-engine
 
+allOf:
+  - $ref: /schemas/spi/spi-controller.yaml#
+  - if:
+      properties:
+        compatible:
+          enum:
+            - mediatek,mt7622-snand
+            - mediatek,mt7629-snand
+    then:
+      properties:
+        clocks:
+          items:
+            - description: clock used for the controller
+            - description: clock used for the SPI bus
+        clock-names:
+          items:
+            - const: nfi_clk
+            - const: pad_clk
+
+  - if:
+      properties:
+        compatible:
+          enum:
+            - mediatek,mt7986-snand
+    then:
+      properties:
+        clocks:
+          items:
+            - description: clock used for the controller
+            - description: clock used for the SPI bus
+            - description: clock used for the AHB bus
+        clock-names:
+          items:
+            - const: nfi_clk
+            - const: pad_clk
+            - const: nfi_hclk
+
 unevaluatedProperties: false
 
 examples:
-- 
2.25.1

Re: [PATCH v2 5/9] dt-bindings: spi: mtk-snfi: Add compatible for MT7986

[Krzysztof Kozlowski]

On 05/12/2022 07:57, Xiangsheng Hou wrote:
> 1. Add dt-bindings documentation of SPI NAND controller
> for MediaTek MT7986 SoC platform.
> 2. Add optional nfi_hclk property which needed for MT7986.
> 
> Signed-off-by: Xiangsheng Hou <xiangsheng.hou@mediatek.com>
> ---


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

[PATCH v2 6/9] spi: mtk-snfi: Add snfi sample delay and read latency adjustment

[Xiangsheng Hou]

Add snfi sample delay and read latency adjustment which can get
from dts property.

Signed-off-by: Xiangsheng Hou <xiangsheng.hou@mediatek.com>
---
 drivers/spi/spi-mtk-snfi.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/spi/spi-mtk-snfi.c b/drivers/spi/spi-mtk-snfi.c
index 85644308df23..32a9a817869c 100644
--- a/drivers/spi/spi-mtk-snfi.c
+++ b/drivers/spi/spi-mtk-snfi.c
@@ -195,6 +195,8 @@
 #define DATA_READ_MODE_X4 2
 #define DATA_READ_MODE_DUAL 5
 #define DATA_READ_MODE_QUAD 6
+#define DATA_READ_LATCH_LAT GENMASK(9, 8)
+#define DATA_READ_LATCH_LAT_S 8
 #define PG_LOAD_CUSTOM_EN BIT(7)
 #define DATARD_CUSTOM_EN BIT(6)
 #define CS_DESELECT_CYC_S 0
@@ -205,6 +207,9 @@
 
 #define SNF_DLY_CTL3 0x548
 #define SFCK_SAM_DLY_S 0
+#define SFCK_SAM_DLY GENMASK(5, 0)
+#define SFCK_SAM_DLY_TOTAL 9
+#define SFCK_SAM_DLY_RANGE 47
 
 #define SNF_STA_CTL1 0x550
 #define CUS_PG_DONE BIT(28)
@@ -1368,6 +1373,7 @@ static int mtk_snand_probe(struct platform_device *pdev)
 	const struct of_device_id *dev_id;
 	struct spi_controller *ctlr;
 	struct mtk_snand *ms;
+	u32 val = 0;
 	int ret;
 
 	dev_id = of_match_node(mtk_snand_ids, np);
@@ -1446,6 +1452,16 @@ static int mtk_snand_probe(struct platform_device *pdev)
 	// switch to SNFI mode
 	nfi_write32(ms, SNF_CFG, SPI_MODE);
 
+	ret = of_property_read_u32(np, "rx-sample-delay-ns", &val);
+	if (!ret)
+		nfi_rmw32(ms, SNF_DLY_CTL3, SFCK_SAM_DLY,
+			  val * SFCK_SAM_DLY_RANGE / SFCK_SAM_DLY_TOTAL);
+
+	ret = of_property_read_u32(np, "mediatek,rx-latch-latency", &val);
+	if (!ret)
+		nfi_rmw32(ms, SNF_MISC_CTL, DATA_READ_LATCH_LAT,
+			  val << DATA_READ_LATCH_LAT_S);
+
 	// setup an initial page format for ops matching page_cache_op template
 	// before ECC is called.
 	ret = mtk_snand_setup_pagefmt(ms, SZ_2K, SZ_64);
-- 
2.25.1

Re: [PATCH v2 6/9] spi: mtk-snfi: Add snfi sample delay and read latency adjustment

[AngeloGioacchino Del Regno]

Il 05/12/22 07:57, Xiangsheng Hou ha scritto:
> Add snfi sample delay and read latency adjustment which can get
> from dts property.
> 
> Signed-off-by: Xiangsheng Hou <xiangsheng.hou@mediatek.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

[PATCH v2 7/9] dt-bindings: spi: mtk-snfi: Add read latch latency property

[Xiangsheng Hou]

Add mediatek,rx-latch-latency property which adjust read delay in the
unit of clock cycle.

Signed-off-by: Xiangsheng Hou <xiangsheng.hou@mediatek.com>
---
 .../devicetree/bindings/spi/mediatek,spi-mtk-snfi.yaml     | 7 +++++++
 1 file changed, 7 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..6e6ff8d73fcd 100644
--- a/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-snfi.yaml
+++ b/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-snfi.yaml
@@ -45,6 +45,13 @@ properties:
     description: device-tree node of the accompanying ECC engine.
     $ref: /schemas/types.yaml#/definitions/phandle
 
+  mediatek,rx-latch-latency:
+    description: Rx delay to sample data with this value, the value
+                 unit is clock cycle.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [0, 1, 2, 3]
+    default: 0
+
 required:
   - compatible
   - reg
-- 
2.25.1

Re: [PATCH v2 7/9] dt-bindings: spi: mtk-snfi: Add read latch latency property

[Krzysztof Kozlowski]

On 05/12/2022 07:57, Xiangsheng Hou wrote:
> Add mediatek,rx-latch-latency property which adjust read delay in the
> unit of clock cycle.
> 
> Signed-off-by: Xiangsheng Hou <xiangsheng.hou@mediatek.com>


Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH v2 7/9] dt-bindings: spi: mtk-snfi: Add read latch latency property

[AngeloGioacchino Del Regno]

Il 05/12/22 07:57, Xiangsheng Hou ha scritto:
> Add mediatek,rx-latch-latency property which adjust read delay in the
> unit of clock cycle.
> 
> Signed-off-by: Xiangsheng Hou <xiangsheng.hou@mediatek.com>
> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>   .../devicetree/bindings/spi/mediatek,spi-mtk-snfi.yaml     | 7 +++++++
>   1 file changed, 7 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..6e6ff8d73fcd 100644
> --- a/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-snfi.yaml
> +++ b/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-snfi.yaml
> @@ -45,6 +45,13 @@ properties:
>       description: device-tree node of the accompanying ECC engine.
>       $ref: /schemas/types.yaml#/definitions/phandle
>   
> +  mediatek,rx-latch-latency:
> +    description: Rx delay to sample data with this value, the value
> +                 unit is clock cycle.

Can't we use nanoseconds or microseconds as a unit here, instead of clock cycles?

Regards,
Angelo

Re: [PATCH v2 7/9] dt-bindings: spi: mtk-snfi: Add read latch latency property

[Xiangsheng Hou (侯祥胜)]

Hi Angelo,

On Mon, 2022-12-05 at 15:21 +0100, AngeloGioacchino Del Regno wrote:
> Il 05/12/22 07:57, Xiangsheng Hou ha scritto:
> > Add mediatek,rx-latch-latency property which adjust read delay in
> > the
> > unit of clock cycle.
> > 
> > Signed-off-by: Xiangsheng Hou <xiangsheng.hou@mediatek.com>
> > Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > ---
> >   .../devicetree/bindings/spi/mediatek,spi-mtk-snfi.yaml     | 7
> > +++++++
> >   1 file changed, 7 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..6e6ff8d73fcd 100644
> > --- a/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-
> > snfi.yaml
> > +++ b/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-
> > snfi.yaml
> > @@ -45,6 +45,13 @@ properties:
> >       description: device-tree node of the accompanying ECC engine.
> >       $ref: /schemas/types.yaml#/definitions/phandle
> >   
> > +  mediatek,rx-latch-latency:
> > +    description: Rx delay to sample data with this value, the
> > value
> > +                 unit is clock cycle.
> 
> Can't we use nanoseconds or microseconds as a unit here, instead of
> clock cycles?

The clock cycle will be various with MediaTek SPI NAND controller which
clock frequency can support 26/52/68/81/104MHz...
It`s may be easy to configure and understand with clock cycle in unit.

Thanks
Xiangsheng Hou

Re: [PATCH v2 7/9] dt-bindings: spi: mtk-snfi: Add read latch latency property

[AngeloGioacchino Del Regno]

Il 06/12/22 10:04, Xiangsheng Hou (侯祥胜) ha scritto:
> Hi Angelo,
> 
> On Mon, 2022-12-05 at 15:21 +0100, AngeloGioacchino Del Regno wrote:
>> Il 05/12/22 07:57, Xiangsheng Hou ha scritto:
>>> Add mediatek,rx-latch-latency property which adjust read delay in
>>> the
>>> unit of clock cycle.
>>>
>>> Signed-off-by: Xiangsheng Hou <xiangsheng.hou@mediatek.com>
>>> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>> ---
>>>    .../devicetree/bindings/spi/mediatek,spi-mtk-snfi.yaml     | 7
>>> +++++++
>>>    1 file changed, 7 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..6e6ff8d73fcd 100644
>>> --- a/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-
>>> snfi.yaml
>>> +++ b/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-
>>> snfi.yaml
>>> @@ -45,6 +45,13 @@ properties:
>>>        description: device-tree node of the accompanying ECC engine.
>>>        $ref: /schemas/types.yaml#/definitions/phandle
>>>    
>>> +  mediatek,rx-latch-latency:
>>> +    description: Rx delay to sample data with this value, the
>>> value
>>> +                 unit is clock cycle.
>>
>> Can't we use nanoseconds or microseconds as a unit here, instead of
>> clock cycles?
> 
> The clock cycle will be various with MediaTek SPI NAND controller which
> clock frequency can support 26/52/68/81/104MHz...
> It`s may be easy to configure and understand with clock cycle in unit.
> 

Yes, but whatever clock frequency we use, the target is to always wait for
X nanoseconds, right?

Waiting for 5 clock cycles at 104MHz is obviously not the same as waiting
for the same 5 clock cycles at 26MHz: in that case, expressing the value
in nanoseconds or microseconds would make that independent from the
controller's clock frequency as the calculation from `time` to `cycles`
would be performed inside of the driver.

Regards,
Angelo

Re: [PATCH v2 7/9] dt-bindings: spi: mtk-snfi: Add read latch latency property

[Xiangsheng Hou (侯祥胜)]

Hi Angelo,

On Tue, 2022-12-06 at 13:19 +0100, AngeloGioacchino Del Regno wrote:
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/spi/mediatek,spi-
> > > > mtk-snfi.yaml
> > > > b/Documentation/devicetree/bindings/spi/mediatek,spi-
> > > > mtk-snfi.yaml
> > > > index bab23f1b11fd..6e6ff8d73fcd 100644
> > > > --- a/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-
> > > > snfi.yaml
> > > > +++ b/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-
> > > > snfi.yaml
> > > > @@ -45,6 +45,13 @@ properties:
> > > >        description: device-tree node of the accompanying ECC
> > > > engine.
> > > >        $ref: /schemas/types.yaml#/definitions/phandle
> > > >    
> > > > +  mediatek,rx-latch-latency:
> > > > +    description: Rx delay to sample data with this value, the
> > > > value
> > > > +                 unit is clock cycle.
> > > 
> > > Can't we use nanoseconds or microseconds as a unit here, instead
> > > of
> > > clock cycles?
> > 
> > The clock cycle will be various with MediaTek SPI NAND controller
> > which
> > clock frequency can support 26/52/68/81/104MHz...
> > It`s may be easy to configure and understand with clock cycle in
> > unit.
> > 
> 
> Yes, but whatever clock frequency we use, the target is to always
> wait for
> X nanoseconds, right?
> 
> Waiting for 5 clock cycles at 104MHz is obviously not the same as
> waiting
> for the same 5 clock cycles at 26MHz: in that case, expressing the
> value
> in nanoseconds or microseconds would make that independent from the
> controller's clock frequency as the calculation from `time` to
> `cycles`
> would be performed inside of the driver.

There have two rx related timing properties in spi-peripheral-props.
The rx-sample-delay-ns have been used in Mediatek snfi driver to adjust
controller sample delay.
However another spi-rx-delay-us is in microseconds. Take 52MHz for
example, the clock cycle will be 19.23ns which lower than 1us. This may
not easy to by one clock cycle.

Thanks
Xiangsheng Hou

Re: [PATCH v2 7/9] dt-bindings: spi: mtk-snfi: Add read latch latency property

[AngeloGioacchino Del Regno]

Il 07/12/22 03:00, Xiangsheng Hou (侯祥胜) ha scritto:
> Hi Angelo,
> 
> On Tue, 2022-12-06 at 13:19 +0100, AngeloGioacchino Del Regno wrote:
>>>>> diff --git
>>>>> a/Documentation/devicetree/bindings/spi/mediatek,spi-
>>>>> mtk-snfi.yaml
>>>>> b/Documentation/devicetree/bindings/spi/mediatek,spi-
>>>>> mtk-snfi.yaml
>>>>> index bab23f1b11fd..6e6ff8d73fcd 100644
>>>>> --- a/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-
>>>>> snfi.yaml
>>>>> +++ b/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-
>>>>> snfi.yaml
>>>>> @@ -45,6 +45,13 @@ properties:
>>>>>         description: device-tree node of the accompanying ECC
>>>>> engine.
>>>>>         $ref: /schemas/types.yaml#/definitions/phandle
>>>>>     
>>>>> +  mediatek,rx-latch-latency:
>>>>> +    description: Rx delay to sample data with this value, the
>>>>> value
>>>>> +                 unit is clock cycle.
>>>>
>>>> Can't we use nanoseconds or microseconds as a unit here, instead
>>>> of
>>>> clock cycles?
>>>
>>> The clock cycle will be various with MediaTek SPI NAND controller
>>> which
>>> clock frequency can support 26/52/68/81/104MHz...
>>> It`s may be easy to configure and understand with clock cycle in
>>> unit.
>>>
>>
>> Yes, but whatever clock frequency we use, the target is to always
>> wait for
>> X nanoseconds, right?
>>
>> Waiting for 5 clock cycles at 104MHz is obviously not the same as
>> waiting
>> for the same 5 clock cycles at 26MHz: in that case, expressing the
>> value
>> in nanoseconds or microseconds would make that independent from the
>> controller's clock frequency as the calculation from `time` to
>> `cycles`
>> would be performed inside of the driver.
> 
> There have two rx related timing properties in spi-peripheral-props.
> The rx-sample-delay-ns have been used in Mediatek snfi driver to adjust
> controller sample delay.
> However another spi-rx-delay-us is in microseconds. Take 52MHz for
> example, the clock cycle will be 19.23ns which lower than 1us. This may
> not easy to by one clock cycle.
> 

I agree, but nothing prevents you from adding your own property for that.

I propose "mediatek,rx-latch-latency-ns" or "mediatek,rx-latency-ns", so that
we can specify the delay in nanoseconds: in that case, when we specify 19ns,
the driver will safely round that resulting in 52MHz == 19.23ns => 19ns valid.

Regards,
Angelo

Re: [PATCH v2 7/9] dt-bindings: spi: mtk-snfi: Add read latch latency property

[Xiangsheng Hou (侯祥胜)]

On Wed, 2022-12-07 at 10:48 +0100, AngeloGioacchino Del Regno wrote:
> Il 07/12/22 03:00, Xiangsheng Hou (侯祥胜) ha scritto:
> > Hi Angelo,
> > 
> > On Tue, 2022-12-06 at 13:19 +0100, AngeloGioacchino Del Regno
> > wrote:
> > > > > > diff --git
> > > > > > a/Documentation/devicetree/bindings/spi/mediatek,spi-
> > > > > > mtk-snfi.yaml
> > > > > > b/Documentation/devicetree/bindings/spi/mediatek,spi-
> > > > > > mtk-snfi.yaml
> > > > > > index bab23f1b11fd..6e6ff8d73fcd 100644
> > > > > > --- a/Documentation/devicetree/bindings/spi/mediatek,spi-
> > > > > > mtk-
> > > > > > snfi.yaml
> > > > > > +++ b/Documentation/devicetree/bindings/spi/mediatek,spi-
> > > > > > mtk-
> > > > > > snfi.yaml
> > > > > > @@ -45,6 +45,13 @@ properties:
> > > > > >         description: device-tree node of the accompanying
> > > > > > ECC
> > > > > > engine.
> > > > > >         $ref: /schemas/types.yaml#/definitions/phandle
> > > > > >     
> > > > > > +  mediatek,rx-latch-latency:
> > > > > > +    description: Rx delay to sample data with this value,
> > > > > > the
> > > > > > value
> > > > > > +                 unit is clock cycle.
> > > > > 
> > > > > Can't we use nanoseconds or microseconds as a unit here,
> > > > > instead
> > > > > of
> > > > > clock cycles?
> > > > 
> > > > The clock cycle will be various with MediaTek SPI NAND
> > > > controller
> > > > which
> > > > clock frequency can support 26/52/68/81/104MHz...
> > > > It`s may be easy to configure and understand with clock cycle
> > > > in
> > > > unit.
> > > > 
> > > 
> > > Yes, but whatever clock frequency we use, the target is to always
> > > wait for
> > > X nanoseconds, right?
> > > 
> > > Waiting for 5 clock cycles at 104MHz is obviously not the same as
> > > waiting
> > > for the same 5 clock cycles at 26MHz: in that case, expressing
> > > the
> > > value
> > > in nanoseconds or microseconds would make that independent from
> > > the
> > > controller's clock frequency as the calculation from `time` to
> > > `cycles`
> > > would be performed inside of the driver.
> > 
> > There have two rx related timing properties in spi-peripheral-
> > props.
> > The rx-sample-delay-ns have been used in Mediatek snfi driver to
> > adjust
> > controller sample delay.
> > However another spi-rx-delay-us is in microseconds. Take 52MHz for
> > example, the clock cycle will be 19.23ns which lower than 1us. This
> > may
> > not easy to by one clock cycle.
> > 
> 
> I agree, but nothing prevents you from adding your own property for
> that.
> 
> I propose "mediatek,rx-latch-latency-ns" or "mediatek,rx-latency-ns", 
> so that
> we can specify the delay in nanoseconds: in that case, when we
> specify 19ns,
> the driver will safely round that resulting in 52MHz == 19.23ns =>
> 19ns valid.

Will be fixed in next series.

Thanks
Xiangsheng Hou

Re: [PATCH v2 7/9] dt-bindings: spi: mtk-snfi: Add read latch latency property

[Krzysztof Kozlowski]

On 08/12/2022 02:15, Xiangsheng Hou (侯祥胜) wrote:
>>>
>>> There have two rx related timing properties in spi-peripheral-
>>> props.
>>> The rx-sample-delay-ns have been used in Mediatek snfi driver to
>>> adjust
>>> controller sample delay.
>>> However another spi-rx-delay-us is in microseconds. Take 52MHz for
>>> example, the clock cycle will be 19.23ns which lower than 1us. This
>>> may
>>> not easy to by one clock cycle.
>>>
>>
>> I agree, but nothing prevents you from adding your own property for
>> that.
>>
>> I propose "mediatek,rx-latch-latency-ns" or "mediatek,rx-latency-ns", 
>> so that
>> we can specify the delay in nanoseconds: in that case, when we
>> specify 19ns,
>> the driver will safely round that resulting in 52MHz == 19.23ns =>
>> 19ns valid.
> 
> Will be fixed in next series.

I am fine with this approach, but after explanations I was also fine
with clock cycles as unit. It's still quite specific unit and I think
several timings on buses are clock-cycle dependent, not time dependent.

Best regards,
Krzysztof

[PATCH v2 8/9] dt-bindings: mtd: Split ECC engine with rawnand controller

[Xiangsheng Hou]

1. Split MediaTek ECC engine with rawnand controller and convert to
YAML schema.
2. Change the existing node name in order to match NAND controller DT
bindings.

Signed-off-by: Xiangsheng Hou <xiangsheng.hou@mediatek.com>
---
 .../bindings/mtd/mediatek,mtk-nfc.yaml        | 171 +++++++++++++++++
 .../mtd/mediatek,nand-ecc-engine.yaml         |  62 ++++++
 .../devicetree/bindings/mtd/mtk-nand.txt      | 176 ------------------
 arch/arm/boot/dts/mt2701.dtsi                 |   2 +-
 arch/arm64/boot/dts/mediatek/mt2712e.dtsi     |   2 +-
 arch/arm64/boot/dts/mediatek/mt7622.dtsi      |   2 +-
 6 files changed, 236 insertions(+), 179 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..2b1c92edc9d0
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/mediatek,mtk-nfc.yaml
@@ -0,0 +1,171 @@
+# 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 <xiangsheng.hou@mediatek.com>
+
+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: true
+
+  partitions:
+    $ref: mtd.yaml#
+
+allOf:
+  - $ref: nand-controller.yaml#
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: mediatek,mt2701-nfc
+    then:
+      patternProperties:
+        "^nand@[a-f0-9]$":
+          type: object
+          properties:
+            reg:
+              minimum: 0
+              maximum: 1
+            nand-ecc-mode:
+              const: hw
+            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]$":
+          type: object
+          properties:
+            reg:
+              minimum: 0
+              maximum: 1
+            nand-ecc-mode:
+              const: hw
+            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]$":
+          type: object
+          properties:
+            reg:
+              minimum: 0
+              maximum: 1
+            nand-ecc-mode:
+              const: hw
+            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>;
+                    };
+                };
+            };
+        };
+
+        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/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 <xiangsheng.hou@mediatek.com>
+
+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";
-	};
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

Re: [PATCH v2 8/9] dt-bindings: mtd: Split ECC engine with rawnand controller

[Krzysztof Kozlowski]

On 05/12/2022 07:57, Xiangsheng Hou wrote:
> 1. Split MediaTek ECC engine with rawnand controller and convert to
> YAML schema.
> 2. Change the existing node name in order to match NAND controller DT
> bindings.

One patch - one logical change. Not two. This applies to all your
patches, so whenever you want to enumerate, please think twice.

> 
> Signed-off-by: Xiangsheng Hou <xiangsheng.hou@mediatek.com>
> ---
>  .../bindings/mtd/mediatek,mtk-nfc.yaml        | 171 +++++++++++++++++
>  .../mtd/mediatek,nand-ecc-engine.yaml         |  62 ++++++
>  .../devicetree/bindings/mtd/mtk-nand.txt      | 176 ------------------
>  arch/arm/boot/dts/mt2701.dtsi                 |   2 +-
>  arch/arm64/boot/dts/mediatek/mt2712e.dtsi     |   2 +-
>  arch/arm64/boot/dts/mediatek/mt7622.dtsi      |   2 +-

Do not combine bindings and DTS.

>  6 files changed, 236 insertions(+), 179 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..2b1c92edc9d0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/mediatek,mtk-nfc.yaml
> @@ -0,0 +1,171 @@
> +# 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 <xiangsheng.hou@mediatek.com>
> +
> +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: true

I don't think this could be anything. You need to describe it, so $ref
and description.

> +
> +  partitions:
> +    $ref: mtd.yaml#

How the partitions are MTD device? Open that file and see how it should
be defined... Anyway mtd.yaml is part of nand-chip, not nand-controller.

> +
> +allOf:
> +  - $ref: nand-controller.yaml#
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: mediatek,mt2701-nfc
> +    then:
> +      patternProperties:
> +        "^nand@[a-f0-9]$":
> +          type: object

No need for type, the definition is already there through
nand-controller.yaml.

> +          properties:
> +            reg:
> +              minimum: 0
> +              maximum: 1

This is the same as other variant, so should be defined in top-level
pattern properties.

> +            nand-ecc-mode:
> +              const: hw

Ditto

> +            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]$":
> +          type: object
> +          properties:
> +            reg:
> +              minimum: 0
> +              maximum: 1
> +            nand-ecc-mode:
> +              const: hw
> +            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]$":
> +          type: object
> +          properties:
> +            reg:
> +              minimum: 0
> +              maximum: 1
> +            nand-ecc-mode:
> +              const: hw
> +            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>;
> +                    };
> +                };
> +            };
> +        };
> +
> +        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";

You already have example of ecc in other binding, so drop from this one.

> +        };
> +    };
> 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


Best regards,
Krzysztof

Re: [PATCH v2 8/9] dt-bindings: mtd: Split ECC engine with rawnand controller

[Xiangsheng Hou (侯祥胜)]

Hi Krzysztof,

On Mon, 2022-12-05 at 10:21 +0100, Krzysztof Kozlowski wrote:
> On 05/12/2022 07:57, Xiangsheng Hou wrote:
> > 1. Split MediaTek ECC engine with rawnand controller and convert to
> > YAML schema.
> > 2. Change the existing node name in order to match NAND controller
> > DT
> > bindings.
> 
> One patch - one logical change. Not two. This applies to all your
> patches, so whenever you want to enumerate, please think twice.

Will be corrected in next series.

> 
> > 
> > Signed-off-by: Xiangsheng Hou <xiangsheng.hou@mediatek.com>
> > ---
> >  .../bindings/mtd/mediatek,mtk-nfc.yaml        | 171
> > +++++++++++++++++
> >  .../mtd/mediatek,nand-ecc-engine.yaml         |  62 ++++++
> >  .../devicetree/bindings/mtd/mtk-nand.txt      | 176 --------------
> > ----
> >  arch/arm/boot/dts/mt2701.dtsi                 |   2 +-
> >  arch/arm64/boot/dts/mediatek/mt2712e.dtsi     |   2 +-
> >  arch/arm64/boot/dts/mediatek/mt7622.dtsi      |   2 +-
> 
> Do not combine bindings and DTS.

The DTS modification will be separated.

> > 
> > +
> > +  ecc-engine: true
> 
> I don't think this could be anything. You need to describe it, so
> $ref
> and description.

Will do.

> > +
> > +  partitions:
> > +    $ref: mtd.yaml#
> 
> How the partitions are MTD device? Open that file and see how it
> should
> be defined... Anyway mtd.yaml is part of nand-chip, not nand-
> controller.

This will be dropped in next series since nand-chip is part of nand-
controller.

> > +
> > +allOf:
> > +  - $ref: nand-controller.yaml#
> > +
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            const: mediatek,mt2701-nfc
> > +    then:
> > +      patternProperties:
> > +        "^nand@[a-f0-9]$":
> > +          type: object
> 
> No need for type, the definition is already there through
> nand-controller.yaml.
> 
> > +          properties:
> > +            reg:
> > +              minimum: 0
> > +              maximum: 1
> 
> This is the same as other variant, so should be defined in top-level
> pattern properties.
> 
> > +            nand-ecc-mode:
> > +              const: hw
> 
> Ditto

Will be fixed in next series.

Thanks
Xiangsheng Hou

[PATCH v2 9/9] dt-bindings: mtd: ecc-mtk: Add compatible for MT7986

[Xiangsheng Hou]

Add dt-bindings documentation of ECC for MediaTek MT7986 SoC
platform.

Signed-off-by: Xiangsheng Hou <xiangsheng.hou@mediatek.com>
---
 .../devicetree/bindings/mtd/mediatek,nand-ecc-engine.yaml        | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/mtd/mediatek,nand-ecc-engine.yaml b/Documentation/devicetree/bindings/mtd/mediatek,nand-ecc-engine.yaml
index b13d801eda76..505baf1e8830 100644
--- a/Documentation/devicetree/bindings/mtd/mediatek,nand-ecc-engine.yaml
+++ b/Documentation/devicetree/bindings/mtd/mediatek,nand-ecc-engine.yaml
@@ -18,6 +18,7 @@ properties:
       - mediatek,mt2701-ecc
       - mediatek,mt2712-ecc
       - mediatek,mt7622-ecc
+      - mediatek,mt7986-ecc
 
   reg:
     items:
-- 
2.25.1

Re: [PATCH v2 9/9] dt-bindings: mtd: ecc-mtk: Add compatible for MT7986

[Krzysztof Kozlowski]

On 05/12/2022 07:57, Xiangsheng Hou wrote:
> Add dt-bindings documentation of ECC for MediaTek MT7986 SoC
> platform.
> 

Now your subject prefix does not match file. Filename is
"mediatek,nand-ecc-engine." so use it instead of "ecc-mtk".


Best regards,
Krzysztof

Re: (subset) [PATCH v2 0/9] Add MediaTek MT7986 SPI NAND and ECC support

[Mark Brown]

On Mon, 5 Dec 2022 14:57:47 +0800, Xiangsheng Hou wrote:
> This patch series add MediaTek MT7986 SPI NAND and ECC controller
> support, split ECC engine with rawnand controller in bindings and
> hange to YAML schema.
> 
> 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.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next

Thanks!

[1/9] spi: mtk-snfi: Add snfi support for MT7986 IC
      commit: 7073888c86601389e17f3ee8ab15ab7aef148839

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark