2022-11-28 02:13:31

by Xiangsheng Hou

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

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

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 mt7986 IC snfi bindings
spi: mtk-snfi: add snfi sample delay and read latency adjustment
dt-bindings: spi: mtk-snfi: add two timing delay property
dt-bindings: mtd: Split ECC engine with rawnand controller
dt-bindings: mtd: ecc-mtk: add mt7986 IC ecc bindings

.../bindings/mtd/mtk,nand-ecc-engine.yaml | 61 ++++++
.../devicetree/bindings/mtd/mtk-nand.txt | 176 ------------------
.../devicetree/bindings/mtd/mtk-nand.yaml | 92 +++++++++
.../bindings/spi/mediatek,spi-mtk-snfi.yaml | 22 +++
drivers/mtd/nand/ecc-mtk.c | 18 ++
drivers/spi/spi-mtk-snfi.c | 63 ++++++-
6 files changed, 252 insertions(+), 180 deletions(-)
create mode 100644 Documentation/devicetree/bindings/mtd/mtk,nand-ecc-engine.yaml
delete mode 100644 Documentation/devicetree/bindings/mtd/mtk-nand.txt
create mode 100644 Documentation/devicetree/bindings/mtd/mtk-nand.yaml

--
2.25.1


2022-11-28 02:14:27

by Xiangsheng Hou

[permalink] [raw]
Subject: [PATCH 1/9] spi: mtk-snfi: add snfi support for mt7986 IC

add snfi support for mt7986 IC

Signed-off-by: Xiangsheng Hou <[email protected]>
---
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

2022-11-28 02:19:18

by Xiangsheng Hou

[permalink] [raw]
Subject: [PATCH 4/9] mtd: nand: ecc-mtk: add ecc support fot mt7986 IC

add ecc support fot mt7986 IC

Signed-off-by: Xiangsheng Hou <[email protected]>
---
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

2022-11-28 02:27:13

by Xiangsheng Hou

[permalink] [raw]
Subject: [PATCH 3/9] spi: mtk-snfi: add optional nfi_hclk which needed for mt7986

add optional nfi_hclk which needed for mt7986

Signed-off-by: Xiangsheng Hou <[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-11-28 02:34:19

by Xiangsheng Hou

[permalink] [raw]
Subject: [PATCH 2/9] spi: mtk-snfi: change default page format to setup default setting

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 <[email protected]>
---
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

2022-11-28 03:05:25

by Xiangsheng Hou

[permalink] [raw]
Subject: [PATCH 7/9] dt-bindings: spi: mtk-snfi: add two timing delay property

add rx-sample-delay and rx-latch-latency property.

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

diff --git a/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-snfi.yaml b/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-snfi.yaml
index ee20075cd0e7..367862688e92 100644
--- a/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-snfi.yaml
+++ b/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-snfi.yaml
@@ -55,6 +55,22 @@ properties:
description: device-tree node of the accompanying ECC engine.
$ref: /schemas/types.yaml#/definitions/phandle

+ rx-sample-delay:
+ description: Rx delay to sample data with this value, the valid
+ values are from 0 to 47. The delay is smaller than
+ the rx-latch-latency.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minItems: 0
+ maxItems: 47
+ default: 0
+
+ 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

2022-11-28 09:23:34

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 7/9] dt-bindings: spi: mtk-snfi: add two timing delay property

On 28/11/2022 03:06, Xiangsheng Hou wrote:
> add rx-sample-delay and rx-latch-latency property.

Start sentences with capital letter.

Here and in commit subject: property->properties

>
> Signed-off-by: Xiangsheng Hou <[email protected]>
> ---
> .../bindings/spi/mediatek,spi-mtk-snfi.yaml | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-snfi.yaml b/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-snfi.yaml
> index ee20075cd0e7..367862688e92 100644
> --- a/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-snfi.yaml
> +++ b/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-snfi.yaml
> @@ -55,6 +55,22 @@ properties:
> description: device-tree node of the accompanying ECC engine.
> $ref: /schemas/types.yaml#/definitions/phandle
>
> + rx-sample-delay:

No, use existing property, don't invent your own stuff - missing unit
suffix. See spi-peripheral-props.yaml.

> + description: Rx delay to sample data with this value, the valid
> + values are from 0 to 47. The delay is smaller than
> + the rx-latch-latency.
> + $ref: /schemas/types.yaml#/definitions/uint32

Drop $ref.

> + minItems: 0
> + maxItems: 47
> + default: 0
> +
> + rx-latch-latency:

Same problems. Did you check spi-peripheral-props.yaml or other SPI
controller schemas for such property?

> + description: Rx delay to sample data with this value, the value
> + unit is clock cycle.

I think the unit should be rather time (e.g. us).

> + $ref: /schemas/types.yaml#/definitions/uint32
> + enum: [0, 1, 2, 3]
> + default: 0
> +
> required:
> - compatible
> - reg

Best regards,
Krzysztof

2022-11-28 12:44:50

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH 7/9] dt-bindings: spi: mtk-snfi: add two timing delay property


On Mon, 28 Nov 2022 10:06:11 +0800, Xiangsheng Hou wrote:
> add rx-sample-delay and rx-latch-latency property.
>
> Signed-off-by: Xiangsheng Hou <[email protected]>
> ---
> .../bindings/spi/mediatek,spi-mtk-snfi.yaml | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-snfi.yaml: properties:rx-sample-delay:minItems: 0 is less than the minimum of 1
from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-snfi.yaml: properties:rx-sample-delay:maxItems: False schema does not allow 47
hint: Scalar properties should not have array keywords
from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-snfi.yaml: properties:rx-sample-delay:minItems: False schema does not allow 0
hint: Scalar properties should not have array keywords
from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command.

2022-11-29 04:22:35

by Xiangsheng Hou

[permalink] [raw]
Subject: Re: [PATCH 7/9] dt-bindings: spi: mtk-snfi: add two timing delay property

Hi Krzysztof,

On Mon, 2022-11-28 at 10:04 +0100, Krzysztof Kozlowski wrote:
> On 28/11/2022 03:06, Xiangsheng Hou wrote:
> > add rx-sample-delay and rx-latch-latency property.
>
> Start sentences with capital letter.
>
> Here and in commit subject: property->properties
Will be fixed in next series.

> >
> > --- a/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-
> > snfi.yaml
> > +++ b/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-
> > snfi.yaml
> > @@ -55,6 +55,22 @@ properties:
> > description: device-tree node of the accompanying ECC engine.
> > $ref: /schemas/types.yaml#/definitions/phandle
> >
> > + rx-sample-delay:
>
> No, use existing property, don't invent your own stuff - missing unit
> suffix. See spi-peripheral-props.yaml.
Will change to other private property. The read sample delay with
MediaTek SPI NAND controller can be set with values from 0 to 47.
However, it`s difficult to say the unit of each vaule, because the unit
value will be difference with different chip process or different
corner IC.

> > + description: Rx delay to sample data with this value, the
> > valid
> > + values are from 0 to 47. The delay is smaller
> > than
> > + the rx-latch-latency.
> > + $ref: /schemas/types.yaml#/definitions/uint32
>
> Drop $ref.
Will do.

>
> > + minItems: 0
> > + maxItems: 47
> > + default: 0
> > +
> > + rx-latch-latency:
>
> Same problems. Did you check spi-peripheral-props.yaml or other SPI
> controller schemas for such property?
>
> > + description: Rx delay to sample data with this value, the
> > value
> > + unit is clock cycle.
>
> I think the unit should be rather time (e.g. us).
>
Yes, I checked the spi-peripheral-props.yaml and the delay values are
described exactly unit with ns or us. However the unit of MediaTek read
latch latency is clock cycle and it`s difference with different clock
frequency.

> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + enum: [0, 1, 2, 3]
> > + default: 0
> > +
> > required:
> > - compatible
> > - reg
>
Best regards,
Xiangsheng Hou

2022-11-29 08:12:19

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 7/9] dt-bindings: spi: mtk-snfi: add two timing delay property

On 29/11/2022 03:50, Xiangsheng Hou (侯祥胜) wrote:

>>> --- a/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-
>>> snfi.yaml
>>> +++ b/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-
>>> snfi.yaml
>>> @@ -55,6 +55,22 @@ properties:
>>> description: device-tree node of the accompanying ECC engine.
>>> $ref: /schemas/types.yaml#/definitions/phandle
>>>
>>> + rx-sample-delay:
>>
>> No, use existing property, don't invent your own stuff - missing unit
>> suffix. See spi-peripheral-props.yaml.
> Will change to other private property. The read sample delay with
> MediaTek SPI NAND controller can be set with values from 0 to 47.
> However, it`s difficult to say the unit of each vaule, because the unit
> value will be difference with different chip process or different
> corner IC.

Why you cannot use same formula as other SPI drivers for sample-delay?
And divide/multiple by some factor specific to SoC, which is taken from
driver_data?

>
>>> + description: Rx delay to sample data with this value, the
>>> valid
>>> + values are from 0 to 47. The delay is smaller
>>> than
>>> + the rx-latch-latency.
>>> + $ref: /schemas/types.yaml#/definitions/uint32
>>
>> Drop $ref.
> Will do.
>
>>
>>> + minItems: 0
>>> + maxItems: 47
>>> + default: 0
>>> +
>>> + rx-latch-latency:
>>
>> Same problems. Did you check spi-peripheral-props.yaml or other SPI
>> controller schemas for such property?
>>
>>> + description: Rx delay to sample data with this value, the
>>> value
>>> + unit is clock cycle.
>>
>> I think the unit should be rather time (e.g. us).
>>
> Yes, I checked the spi-peripheral-props.yaml and the delay values are
> described exactly unit with ns or us. However the unit of MediaTek read
> latch latency is clock cycle and it`s difference with different clock
> frequency.

This is fine in such case.

>
>>> + $ref: /schemas/types.yaml#/definitions/uint32
>>> + enum: [0, 1, 2, 3]
>>> + default: 0
>>> +

Best regards,
Krzysztof

2022-11-30 08:47:40

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 7/9] dt-bindings: spi: mtk-snfi: add two timing delay property

On 30/11/2022 09:18, Xiangsheng Hou (侯祥胜) wrote:
> Hi Krzysztof,
>
> On Tue, 2022-11-29 at 08:47 +0100, Krzysztof Kozlowski wrote:
>> On 29/11/2022 03:50, Xiangsheng Hou (侯祥胜) wrote:
>>
>>>>> --- a/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-
>>>>> snfi.yaml
>>>>> +++ b/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-
>>>>> snfi.yaml
>>>>> @@ -55,6 +55,22 @@ properties:
>>>>> description: device-tree node of the accompanying ECC
>>>>> engine.
>>>>> $ref: /schemas/types.yaml#/definitions/phandle
>>>>>
>>>>> + rx-sample-delay:
>>>>
>>>> No, use existing property, don't invent your own stuff - missing
>>>> unit
>>>> suffix. See spi-peripheral-props.yaml.
>>>
>>> Will change to other private property. The read sample delay with
>>> MediaTek SPI NAND controller can be set with values from 0 to 47.
>>> However, it`s difficult to say the unit of each vaule, because the
>>> unit
>>> value will be difference with different chip process or different
>>> corner IC.
>>
>> Why you cannot use same formula as other SPI drivers for sample-
>> delay?
>> And divide/multiple by some factor specific to SoC, which is taken
>> from
>> driver_data?
>
> Even for specific SoC, the unit of sample delay may be various with
> different corner IC.

Which is easy to achieve with driver_data as I said.

> Besides, whether it`s acceptable by change the property rx-sample-delay
> and rx-latch-latency to mediatek,rx-sample-delay and mediatek,rx-latch-
> latency?

Not for sample delay, because you should use existing properties. Your
driver implementation is not usually argument to duplicate properties in
the bindings.

Best regards,
Krzysztof

2022-11-30 09:00:56

by Xiangsheng Hou

[permalink] [raw]
Subject: Re: [PATCH 7/9] dt-bindings: spi: mtk-snfi: add two timing delay property

Hi Krzysztof,

On Tue, 2022-11-29 at 08:47 +0100, Krzysztof Kozlowski wrote:
> On 29/11/2022 03:50, Xiangsheng Hou (侯祥胜) wrote:
>
> > > > --- a/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-
> > > > snfi.yaml
> > > > +++ b/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-
> > > > snfi.yaml
> > > > @@ -55,6 +55,22 @@ properties:
> > > > description: device-tree node of the accompanying ECC
> > > > engine.
> > > > $ref: /schemas/types.yaml#/definitions/phandle
> > > >
> > > > + rx-sample-delay:
> > >
> > > No, use existing property, don't invent your own stuff - missing
> > > unit
> > > suffix. See spi-peripheral-props.yaml.
> >
> > Will change to other private property. The read sample delay with
> > MediaTek SPI NAND controller can be set with values from 0 to 47.
> > However, it`s difficult to say the unit of each vaule, because the
> > unit
> > value will be difference with different chip process or different
> > corner IC.
>
> Why you cannot use same formula as other SPI drivers for sample-
> delay?
> And divide/multiple by some factor specific to SoC, which is taken
> from
> driver_data?

Even for specific SoC, the unit of sample delay may be various with
different corner IC.
Besides, whether it`s acceptable by change the property rx-sample-delay
and rx-latch-latency to mediatek,rx-sample-delay and mediatek,rx-latch-
latency?

Thanks
Xiangsheng Hou

2022-11-30 10:21:43

by Chuanhong Guo

[permalink] [raw]
Subject: Re: [PATCH 7/9] dt-bindings: spi: mtk-snfi: add two timing delay property

Hi!

On Wed, Nov 30, 2022 at 4:35 PM Krzysztof Kozlowski
<[email protected]> wrote:
> >>> Will change to other private property. The read sample delay with
> >>> MediaTek SPI NAND controller can be set with values from 0 to 47.
> >>> However, it`s difficult to say the unit of each vaule, because the
> >>> unit
> >>> value will be difference with different chip process or different
> >>> corner IC.
> >>
> >> Why you cannot use same formula as other SPI drivers for sample-
> >> delay?
> >> And divide/multiple by some factor specific to SoC, which is taken
> >> from
> >> driver_data?
> >
> > Even for specific SoC, the unit of sample delay may be various with
> > different corner IC.
>
> Which is easy to achieve with driver_data as I said.

I think Xiangsheng means this:
This sample delay isn't achieved using a fixed clock signal. It's
probably some kind of delay circuit whose delay value varies
due to its manufacturing process. Every single chip made got
different delay units, so it's impossible to specify a single unit
for one chip model.

If that's true, shouldn't this be a value calibrated on-the-fly
on probe instead? A single device-tree is supposed to be
applied to all devices of the same model, so a value that
varies on a device-by-device basis probably shouldn't
be a device-tree property.

--
Regards,
Chuanhong Guo