| Message ID | 20221219024019.31974-1-xiangsheng.hou@mediatek.com |
|---|---|
| Headers | show |
| Series | Add MediaTek MT7986 SPI NAND and ECC support | expand |
Thanks for your patch! There is something to improve please see below. On 19/12/2022 03:40, Xiangsheng Hou wrote: > 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); Couldn't you just set sector_size in mt7986_snand_caps? Regards, Matthias > if (ret) { > dev_err(ms->dev, "failed to set initial page format\n"); > goto disable_clk;
Hi Rob, On Mon, 2022-12-19 at 09:38 -0600, Rob Herring wrote: > On Mon, Dec 19, 2022 at 10:40:15AM +0800, Xiangsheng Hou wrote: > > Add mediatek,rx-latch-latency-ns property which adjust data read > > latch latency in the unit of nanoseconds. > > > > Signed-off-by: Xiangsheng Hou <xiangsheng.hou@mediatek.com> > > Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > > Reviewed-by: AngeloGioacchino Del Regno < > > angelogioacchino.delregno@collabora.com> > > --- > > .../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. > > Doesn't the common 'rx-sample-delay-ns' work for you? The driver need two timing related parameter, one for sample delay which have been used by rx-sample-delay-ns. Another is read latency, just introduce this private timing property since the common spi-rx- delay-us is microsecond in unit. Thanks XIangsheng Hou
Hi Matthias, On Mon, 2022-12-19 at 16:30 +0100, Matthias Brugger wrote: > Thanks for your patch! There is something to improve please see > below. > > On 19/12/2022 03:40, Xiangsheng Hou wrote: > > 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); > > Couldn't you just set sector_size in mt7986_snand_caps? The function mtk_snand_setup_pagefmt need use page and OOB size of NAND device to setup pagefmt. The controller page size can support 512/1k/2k/4k..., the sector size 1k have been set in mt7986_snand_caps. However this will also lead to fail in this function since the 1k page size will not be supported. Just use page size 2k and OOB size 64 as default parameter since this can be supported by all ICs with this controller. Thanks Xiangsheng Hou
On Tue, 2022-12-20 at 10:15 +0800, xiangsheng.hou wrote: Hi Matthias, Angelo > > On Mon, 2022-12-19 at 16:30 +0100, Matthias Brugger wrote: > > Thanks for your patch! There is something to improve please see > > below. > > > > On 19/12/2022 03:40, Xiangsheng Hou wrote: > > > 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); > > > > Couldn't you just set sector_size in mt7986_snand_caps? > > The function mtk_snand_setup_pagefmt need use page and OOB size of > NAND > device to setup pagefmt. > The controller page size can support 512/1k/2k/4k..., the sector size > 1k have been set in mt7986_snand_caps. However this will also lead to > fail in this function since the 1k page size will not be supported. > Just use page size 2k and OOB size 64 as default parameter since this > can be supported by all ICs with this controller. Could you please help to review this patch. Best Regards, Xiangsheng Hou