[PATCH V1 0/4] mainly add a new SoC support for spi-mtk-nor

[PATCH V1 0/4] mainly add a new SoC support for spi-mtk-nor

[guochun.mao]

There is a new SoC mt8186, which spi-nor controller has some differences.
It needs one more clk, axi_s, for dma feature.
And also needs 1 extra dummy bit when reads flash registers.

Guochun Mao (4):
  dt-bindings: spi: add mt8186-nor compatible string
  spi: spi-mtk-nor: improve device table for adding more capabilities
  spi: spi-mtk-nor: add new soc mt8186 support
  spi: spi-mtk-nor: add axi_s clock for mt8186

 .../bindings/spi/mediatek,spi-mtk-nor.yaml    |  4 ++
 drivers/spi/spi-mtk-nor.c                     | 63 ++++++++++++++++---
 2 files changed, 58 insertions(+), 9 deletions(-)

-- 
2.25.1



_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[PATCH v1 1/4] dt-bindings: spi: add mt8186-nor compatible string

[guochun.mao]

From: Guochun Mao <guochun.mao@mediatek.com>

Add MT8186 spi-nor controller support.
MT8186 needs a new clock name, axi_s, for spi nor axi slave bus clock.

Signed-off-by: Guochun Mao <guochun.mao@mediatek.com>
Signed-off-by: Zhen Zhang <zhen.zhang@mediatek.com>
---
 .../devicetree/bindings/spi/mediatek,spi-mtk-nor.yaml         | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-nor.yaml b/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-nor.yaml
index 4e4694e3d539..1a678751bcca 100644
--- a/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-nor.yaml
+++ b/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-nor.yaml
@@ -30,6 +30,7 @@ properties:
               - mediatek,mt7622-nor
               - mediatek,mt7623-nor
               - mediatek,mt7629-nor
+              - mediatek,mt8186-nor
               - mediatek,mt8192-nor
               - mediatek,mt8195-nor
           - enum:
@@ -49,6 +50,8 @@ properties:
       - description: clock used for controller
       - description: clock used for nor dma bus. this depends on hardware
                      design, so this is optional.
+      - description: clock used for controller axi slave bus. this depens
+                     on hardwre design, so this is optional.
 
   clock-names:
     minItems: 2
@@ -56,6 +59,7 @@ properties:
       - const: spi
       - const: sf
       - const: axi
+      - const: axi_s
 
 required:
   - compatible
-- 
2.25.1


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH v1 1/4] dt-bindings: spi: add mt8186-nor compatible string

[Rob Herring]

On Fri, Jan 14, 2022 at 12:24 AM <guochun.mao@mediatek.com> wrote:
>
> From: Guochun Mao <guochun.mao@mediatek.com>

Use get_maintainers.pl and send to the right lists.

> Add MT8186 spi-nor controller support.
> MT8186 needs a new clock name, axi_s, for spi nor axi slave bus clock.
>
> Signed-off-by: Guochun Mao <guochun.mao@mediatek.com>
> Signed-off-by: Zhen Zhang <zhen.zhang@mediatek.com>
> ---
>  .../devicetree/bindings/spi/mediatek,spi-mtk-nor.yaml         | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-nor.yaml b/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-nor.yaml
> index 4e4694e3d539..1a678751bcca 100644
> --- a/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-nor.yaml
> +++ b/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-nor.yaml
> @@ -30,6 +30,7 @@ properties:
>                - mediatek,mt7622-nor
>                - mediatek,mt7623-nor
>                - mediatek,mt7629-nor
> +              - mediatek,mt8186-nor
>                - mediatek,mt8192-nor
>                - mediatek,mt8195-nor
>            - enum:
> @@ -49,6 +50,8 @@ properties:
>        - description: clock used for controller
>        - description: clock used for nor dma bus. this depends on hardware
>                       design, so this is optional.
> +      - description: clock used for controller axi slave bus. this depens
> +                     on hardwre design, so this is optional.

typos.

>
>    clock-names:
>      minItems: 2
> @@ -56,6 +59,7 @@ properties:
>        - const: spi
>        - const: sf
>        - const: axi
> +      - const: axi_s
>
>  required:
>    - compatible
> --
> 2.25.1
>

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH v1 1/4] dt-bindings: spi: add mt8186-nor compatible string

[Guochun Mao]

Hi Rob,

Thanks for your comments.


On Fri, 2022-01-14 at 08:54 -0600, Rob Herring wrote:
> On Fri, Jan 14, 2022 at 12:24 AM <guochun.mao@mediatek.com> wrote:
> > 
> > From: Guochun Mao <guochun.mao@mediatek.com>
> 
> Use get_maintainers.pl and send to the right lists.
> 

I have already used get_maintainers.pl to get maintainers list.
It shows like this,

./scripts/get_maintainer.pl 00*

Mark Brown <broonie@kernel.org> (maintainer:SPI SUBSYSTEM)
Rob Herring <robh+dt@kernel.org> (maintainer:OPEN FIRMWARE AND
FLATTENED DEVICE TREE BINDINGS)
Matthias Brugger <matthias.bgg@gmail.com> (maintainer:ARM/Mediatek SoC
support)
Bayi Cheng <bayi.cheng@mediatek.com> (in file)
Chuanhong Guo <gch981213@gmail.com> (in file)
linux-spi@vger.kernel.org (open list:SPI SUBSYSTEM)
devicetree@vger.kernel.org (open list:OPEN FIRMWARE AND FLATTENED
DEVICE TREE BINDINGS)
linux-arm-kernel@lists.infradead.org (moderated list:ARM/Mediatek SoC
support)
linux-mediatek@lists.infradead.org (moderated list:ARM/Mediatek SoC
support)
linux-kernel@vger.kernel.org (open list)

If you think I need add more reviewers,
could you give me the names who I need to add to these mails?
thanks.

> > Add MT8186 spi-nor controller support.
> > MT8186 needs a new clock name, axi_s, for spi nor axi slave bus
> > clock.
> > 
> > Signed-off-by: Guochun Mao <guochun.mao@mediatek.com>
> > Signed-off-by: Zhen Zhang <zhen.zhang@mediatek.com>
> > ---
> >  .../devicetree/bindings/spi/mediatek,spi-mtk-nor.yaml         | 4
> > ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/spi/mediatek,spi-
> > mtk-nor.yaml b/Documentation/devicetree/bindings/spi/mediatek,spi-
> > mtk-nor.yaml
> > index 4e4694e3d539..1a678751bcca 100644
> > --- a/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-
> > nor.yaml
> > +++ b/Documentation/devicetree/bindings/spi/mediatek,spi-mtk-
> > nor.yaml
> > @@ -30,6 +30,7 @@ properties:
> >                - mediatek,mt7622-nor
> >                - mediatek,mt7623-nor
> >                - mediatek,mt7629-nor
> > +              - mediatek,mt8186-nor
> >                - mediatek,mt8192-nor
> >                - mediatek,mt8195-nor
> >            - enum:
> > @@ -49,6 +50,8 @@ properties:
> >        - description: clock used for controller
> >        - description: clock used for nor dma bus. this depends on
> > hardware
> >                       design, so this is optional.
> > +      - description: clock used for controller axi slave bus. this
> > depens
> > +                     on hardwre design, so this is optional.
> 
> typos.
> 

I'll fix it next version.

thanks.


> > 
> >    clock-names:
> >      minItems: 2
> > @@ -56,6 +59,7 @@ properties:
> >        - const: spi
> >        - const: sf
> >        - const: axi
> > +      - const: axi_s
> > 
> >  required:
> >    - compatible
> > --
> > 2.25.1
> > 
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[PATCH v1 2/4] spi: spi-mtk-nor: improve device table for adding more capabilities

[guochun.mao]

From: Guochun Mao <guochun.mao@mediatek.com>

Define a structure for adding more capabilities.

Signed-off-by: Guochun Mao <guochun.mao@mediatek.com>
Signed-off-by: Zhen Zhang <zhen.zhang@mediatek.com>
---
 drivers/spi/spi-mtk-nor.c | 40 +++++++++++++++++++++++++++++++--------
 1 file changed, 32 insertions(+), 8 deletions(-)

diff --git a/drivers/spi/spi-mtk-nor.c b/drivers/spi/spi-mtk-nor.c
index 5c93730615f8..f3bcdc1d4d8b 100644
--- a/drivers/spi/spi-mtk-nor.c
+++ b/drivers/spi/spi-mtk-nor.c
@@ -95,6 +95,11 @@
 
 #define CLK_TO_US(sp, clkcnt)		DIV_ROUND_UP(clkcnt, sp->spi_freq / 1000000)
 
+struct mtk_nor_caps {
+	u32 dma_bits;
+	u32 extra_dummy_bit;
+};
+
 struct mtk_nor {
 	struct spi_controller *ctlr;
 	struct device *dev;
@@ -109,6 +114,7 @@ struct mtk_nor {
 	bool has_irq;
 	bool high_dma;
 	struct completion op_done;
+	const struct mtk_nor_caps *caps;
 };
 
 static inline void mtk_nor_rmw(struct mtk_nor *sp, u32 reg, u32 set, u32 clr)
@@ -554,7 +560,12 @@ static int mtk_nor_spi_mem_prg(struct mtk_nor *sp, const struct spi_mem_op *op)
 	}
 
 	// trigger op
-	writel(prg_len * BITS_PER_BYTE, sp->base + MTK_NOR_REG_PRG_CNT);
+	if (rx_len)
+		writel(prg_len * BITS_PER_BYTE + sp->caps->extra_dummy_bit,
+		       sp->base + MTK_NOR_REG_PRG_CNT);
+	else
+		writel(prg_len * BITS_PER_BYTE, sp->base + MTK_NOR_REG_PRG_CNT);
+
 	ret = mtk_nor_cmd_exec(sp, MTK_NOR_CMD_PROGRAM,
 			       prg_len * BITS_PER_BYTE);
 	if (ret)
@@ -743,9 +754,19 @@ static const struct spi_controller_mem_ops mtk_nor_mem_ops = {
 	.exec_op = mtk_nor_exec_op
 };
 
+const struct mtk_nor_caps mtk_nor_caps_mt8173 = {
+	.dma_bits = 32,
+	.extra_dummy_bit = 0,
+};
+
+const struct mtk_nor_caps mtk_nor_caps_mt8192 = {
+	.dma_bits = 36,
+	.extra_dummy_bit = 0,
+};
+
 static const struct of_device_id mtk_nor_match[] = {
-	{ .compatible = "mediatek,mt8192-nor", .data = (void *)36 },
-	{ .compatible = "mediatek,mt8173-nor", .data = (void *)32 },
+	{ .compatible = "mediatek,mt8173-nor", .data = &mtk_nor_caps_mt8173 },
+	{ .compatible = "mediatek,mt8192-nor", .data = &mtk_nor_caps_mt8192 },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, mtk_nor_match);
@@ -754,10 +775,10 @@ static int mtk_nor_probe(struct platform_device *pdev)
 {
 	struct spi_controller *ctlr;
 	struct mtk_nor *sp;
+	struct mtk_nor_caps *caps;
 	void __iomem *base;
 	struct clk *spi_clk, *ctlr_clk, *axi_clk;
 	int ret, irq;
-	unsigned long dma_bits;
 
 	base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(base))
@@ -775,9 +796,11 @@ static int mtk_nor_probe(struct platform_device *pdev)
 	if (IS_ERR(axi_clk))
 		return PTR_ERR(axi_clk);
 
-	dma_bits = (unsigned long)of_device_get_match_data(&pdev->dev);
-	if (dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(dma_bits))) {
-		dev_err(&pdev->dev, "failed to set dma mask(%lu)\n", dma_bits);
+	caps = (struct mtk_nor_caps *)of_device_get_match_data(&pdev->dev);
+	if (dma_set_mask_and_coherent(&pdev->dev,
+				      DMA_BIT_MASK(caps->dma_bits))) {
+		dev_err(&pdev->dev, "failed to set dma mask(%u)\n",
+			caps->dma_bits);
 		return -EINVAL;
 	}
 
@@ -808,7 +831,8 @@ static int mtk_nor_probe(struct platform_device *pdev)
 	sp->spi_clk = spi_clk;
 	sp->ctlr_clk = ctlr_clk;
 	sp->axi_clk = axi_clk;
-	sp->high_dma = (dma_bits > 32);
+	sp->caps = caps;
+	sp->high_dma = caps->dma_bits > 32;
 	sp->buffer = dmam_alloc_coherent(&pdev->dev,
 				MTK_NOR_BOUNCE_BUF_SIZE + MTK_NOR_DMA_ALIGN,
 				&sp->buffer_dma, GFP_KERNEL);
-- 
2.25.1


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH v1 2/4] spi: spi-mtk-nor: improve device table for adding more capabilities

[AngeloGioacchino Del Regno]

Il 14/01/22 07:24, guochun.mao@mediatek.com ha scritto:
> From: Guochun Mao <guochun.mao@mediatek.com>

Hello Guochun,

thanks for the patch! However, I have some comments...

> 
> Define a structure for adding more capabilities.

Please also mention that you're adding the extra_dummy_bit feature
in the commit description.

> 
> Signed-off-by: Guochun Mao <guochun.mao@mediatek.com>
> Signed-off-by: Zhen Zhang <zhen.zhang@mediatek.com>
> ---
>   drivers/spi/spi-mtk-nor.c | 40 +++++++++++++++++++++++++++++++--------
>   1 file changed, 32 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/spi/spi-mtk-nor.c b/drivers/spi/spi-mtk-nor.c
> index 5c93730615f8..f3bcdc1d4d8b 100644
> --- a/drivers/spi/spi-mtk-nor.c
> +++ b/drivers/spi/spi-mtk-nor.c
> @@ -95,6 +95,11 @@
>   
>   #define CLK_TO_US(sp, clkcnt)		DIV_ROUND_UP(clkcnt, sp->spi_freq / 1000000)
>   
> +struct mtk_nor_caps {
> +	u32 dma_bits;

dma_bits can be u8: I really don't think that we'll ever see more than
0xff bits...

> +	u32 extra_dummy_bit;

This one looks a bit strange, can you please add a comment explaining the
reason why we have this "extra dummy bit"?

> +};
> +
>   struct mtk_nor {
>   	struct spi_controller *ctlr;
>   	struct device *dev;
> @@ -109,6 +114,7 @@ struct mtk_nor {
>   	bool has_irq;
>   	bool high_dma;
>   	struct completion op_done;
> +	const struct mtk_nor_caps *caps;
>   };
>   
>   static inline void mtk_nor_rmw(struct mtk_nor *sp, u32 reg, u32 set, u32 clr)
> @@ -554,7 +560,12 @@ static int mtk_nor_spi_mem_prg(struct mtk_nor *sp, const struct spi_mem_op *op)
>   	}
>   
>   	// trigger op
> -	writel(prg_len * BITS_PER_BYTE, sp->base + MTK_NOR_REG_PRG_CNT);
> +	if (rx_len)
> +		writel(prg_len * BITS_PER_BYTE + sp->caps->extra_dummy_bit,
> +		       sp->base + MTK_NOR_REG_PRG_CNT);
> +	else
> +		writel(prg_len * BITS_PER_BYTE, sp->base + MTK_NOR_REG_PRG_CNT);
> +
>   	ret = mtk_nor_cmd_exec(sp, MTK_NOR_CMD_PROGRAM,
>   			       prg_len * BITS_PER_BYTE);
>   	if (ret)
> @@ -743,9 +754,19 @@ static const struct spi_controller_mem_ops mtk_nor_mem_ops = {
>   	.exec_op = mtk_nor_exec_op
>   };
>   
> +const struct mtk_nor_caps mtk_nor_caps_mt8173 = {
> +	.dma_bits = 32,
> +	.extra_dummy_bit = 0,
> +};
> +
> +const struct mtk_nor_caps mtk_nor_caps_mt8192 = {
> +	.dma_bits = 36,
> +	.extra_dummy_bit = 0,
> +};
> +
>   static const struct of_device_id mtk_nor_match[] = {
> -	{ .compatible = "mediatek,mt8192-nor", .data = (void *)36 },
> -	{ .compatible = "mediatek,mt8173-nor", .data = (void *)32 },
> +	{ .compatible = "mediatek,mt8173-nor", .data = &mtk_nor_caps_mt8173 },
> +	{ .compatible = "mediatek,mt8192-nor", .data = &mtk_nor_caps_mt8192 },
>   	{ /* sentinel */ }
>   };
>   MODULE_DEVICE_TABLE(of, mtk_nor_match);
> @@ -754,10 +775,10 @@ static int mtk_nor_probe(struct platform_device *pdev)
>   {
>   	struct spi_controller *ctlr;
>   	struct mtk_nor *sp;
> +	struct mtk_nor_caps *caps;
>   	void __iomem *base;
>   	struct clk *spi_clk, *ctlr_clk, *axi_clk;
>   	int ret, irq;
> -	unsigned long dma_bits;
>   
>   	base = devm_platform_ioremap_resource(pdev, 0);
>   	if (IS_ERR(base))
> @@ -775,9 +796,11 @@ static int mtk_nor_probe(struct platform_device *pdev)
>   	if (IS_ERR(axi_clk))
>   		return PTR_ERR(axi_clk);
>   
> -	dma_bits = (unsigned long)of_device_get_match_data(&pdev->dev);
> -	if (dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(dma_bits))) {
> -		dev_err(&pdev->dev, "failed to set dma mask(%lu)\n", dma_bits);
> +	caps = (struct mtk_nor_caps *)of_device_get_match_data(&pdev->dev);
> +	if (dma_set_mask_and_coherent(&pdev->dev,
> +				      DMA_BIT_MASK(caps->dma_bits))) {

While you're at it, can you please also make this return the right value?
The function dma_set_mask_and_coherent() may return an error code, so it's
worth it to just return that.

P.S.: 83 columns is ok

	ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(caps->dma_bits));
	if (ret) {

> +		dev_err(&pdev->dev, "failed to set dma mask(%u)\n",
> +			caps->dma_bits);
>   		return -EINVAL;
>   	}
>   
> @@ -808,7 +831,8 @@ static int mtk_nor_probe(struct platform_device *pdev)
>   	sp->spi_clk = spi_clk;
>   	sp->ctlr_clk = ctlr_clk;
>   	sp->axi_clk = axi_clk;
> -	sp->high_dma = (dma_bits > 32);
> +	sp->caps = caps;
> +	sp->high_dma = caps->dma_bits > 32;
>   	sp->buffer = dmam_alloc_coherent(&pdev->dev,
>   				MTK_NOR_BOUNCE_BUF_SIZE + MTK_NOR_DMA_ALIGN,
>   				&sp->buffer_dma, GFP_KERNEL);
> 

Regards,
- Angelo

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH v1 2/4] spi: spi-mtk-nor: improve device table for adding more capabilities

[Guochun Mao]

Hello Angelo,

Thanks for your comments.

On Fri, 2022-01-14 at 11:40 +0100, AngeloGioacchino Del Regno wrote:
> Il 14/01/22 07:24, guochun.mao@mediatek.com ha scritto:
> > From: Guochun Mao <guochun.mao@mediatek.com>
> 
> Hello Guochun,
> 
> thanks for the patch! However, I have some comments...
> 
> > 
> > Define a structure for adding more capabilities.
> 
> Please also mention that you're adding the extra_dummy_bit feature
> in the commit description.
> 
> > 
> > +struct mtk_nor_caps {
> > +	u32 dma_bits;
> 
> dma_bits can be u8: I really don't think that we'll ever see more
> than
> 0xff bits...
> 

I'll fix it next version.

> > +	u32 extra_dummy_bit;
> 
> This one looks a bit strange, can you please add a comment explaining
> the
> reason why we have this "extra dummy bit"?
> 

This one is adding for mt8186, which extra_dummy_bit is set to be 1.
Due to the IP design, it need a extra_dummy bit when reading nor
flash's register(status/config...).

Do you think I should move the modification to another patch
"[PATCH v1 3/4]spi:spi-mtk-nor:add new soc mt8186 sopport" ?
Or I just add this comment here?

> > 
> >   	// trigger op
> > -	writel(prg_len * BITS_PER_BYTE, sp->base +
> > MTK_NOR_REG_PRG_CNT);
> > +	if (rx_len)
> > +		writel(prg_len * BITS_PER_BYTE + sp->caps-
> > >extra_dummy_bit,
> > +		       sp->base + MTK_NOR_REG_PRG_CNT);
> > +	else
> > +		writel(prg_len * BITS_PER_BYTE, sp->base +
> > MTK_NOR_REG_PRG_CNT);
> > +
> >   	ret = mtk_nor_cmd_exec(sp, MTK_NOR_CMD_PROGRAM,
> >   			       prg_len * BITS_PER_BYTE);
> >   	if (ret)
> > @@ -743,9 +754,19 @@ static const struct spi_controller_mem_ops
> > mtk_nor_mem_ops = {
> >   	.exec_op = mtk_nor_exec_op
> >   };
> >   
> > +const struct mtk_nor_caps mtk_nor_caps_mt8173 = {
> > +	.dma_bits = 32,
> > +	.extra_dummy_bit = 0,
> > +};
> > +
> > +const struct mtk_nor_caps mtk_nor_caps_mt8192 = {
> > +	.dma_bits = 36,
> > +	.extra_dummy_bit = 0,
> > +};
> > +
> >   static const struct of_device_id mtk_nor_match[] = {
> > -	{ .compatible = "mediatek,mt8192-nor", .data = (void *)36 },
> > -	{ .compatible = "mediatek,mt8173-nor", .data = (void *)32 },
> > +	{ .compatible = "mediatek,mt8173-nor", .data =
> > &mtk_nor_caps_mt8173 },
> > +	{ .compatible = "mediatek,mt8192-nor", .data =
> > &mtk_nor_caps_mt8192 },
> >   	{ /* sentinel */ }
>   };
> >   MODULE_DEVICE_TABLE(of, mtk_nor_match);
> > @@ -754,10 +775,10 @@ static int mtk_nor_probe(struct
> > platform_device *pdev)
> >   {
> >   	struct spi_controller *ctlr;
> >   	struct mtk_nor *sp;
> > +	struct mtk_nor_caps *caps;
> >   	void __iomem *base;
> >   	struct clk *spi_clk, *ctlr_clk, *axi_clk;
> >   	int ret, irq;
> > -	unsigned long dma_bits;
> >   
> >   	base = devm_platform_ioremap_resource(pdev, 0);
> >   	if (IS_ERR(base))
> > @@ -775,9 +796,11 @@ static int mtk_nor_probe(struct
> > platform_device *pdev)
> >   	if (IS_ERR(axi_clk))
> >   		return PTR_ERR(axi_clk);
>   
> > -	dma_bits = (unsigned long)of_device_get_match_data(&pdev->dev);
> > -	if (dma_set_mask_and_coherent(&pdev->dev,
> > DMA_BIT_MASK(dma_bits))) {
> > -		dev_err(&pdev->dev, "failed to set dma mask(%lu)\n",
> > dma_bits);
> > +	caps = (struct mtk_nor_caps *)of_device_get_match_data(&pdev-
> > >dev);
> > +	if (dma_set_mask_and_coherent(&pdev->dev,
> > +				      DMA_BIT_MASK(caps->dma_bits))) {
> 
> While you're at it, can you please also make this return the right
> value?
> The function dma_set_mask_and_coherent() may return an error code, so
> it's
> worth it to just return that.
> 
> P.S.: 83 columns is ok
> 

ok, I'll fix it next version.
Thanks.

BR,
Guochun
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH v1 2/4] spi: spi-mtk-nor: improve device table for adding more capabilities

[AngeloGioacchino Del Regno]

Il 17/01/22 03:30, Guochun Mao ha scritto:
> Hello Angelo,
> 
> Thanks for your comments.
> 
> On Fri, 2022-01-14 at 11:40 +0100, AngeloGioacchino Del Regno wrote:
>> Il 14/01/22 07:24, guochun.mao@mediatek.com ha scritto:
>>> From: Guochun Mao <guochun.mao@mediatek.com>
>>
>> Hello Guochun,
>>
>> thanks for the patch! However, I have some comments...
>>
>>>
>>> Define a structure for adding more capabilities.
>>
>> Please also mention that you're adding the extra_dummy_bit feature
>> in the commit description.
>>
>>>
>>> +struct mtk_nor_caps {
>>> +	u32 dma_bits;
>>
>> dma_bits can be u8: I really don't think that we'll ever see more
>> than
>> 0xff bits...
>>
> 
> I'll fix it next version.
> 
>>> +	u32 extra_dummy_bit;
>>
>> This one looks a bit strange, can you please add a comment explaining
>> the
>> reason why we have this "extra dummy bit"?
>>
> 
> This one is adding for mt8186, which extra_dummy_bit is set to be 1.
> Due to the IP design, it need a extra_dummy bit when reading nor
> flash's register(status/config...).
> 
> Do you think I should move the modification to another patch
> "[PATCH v1 3/4]spi:spi-mtk-nor:add new soc mt8186 sopport" ?
> Or I just add this comment here?
> 

It's more appropriate to keep it in this patch and mentioning it into
the commit description. Also, please add this comment in the code and,
if possible, also explain why this IP design needs an extra dummy bit.
Otherwise, if not possible, the provided explaination would also be fine.

Thanks,
- Angelo

>>>
>>>    	// trigger op
>>> -	writel(prg_len * BITS_PER_BYTE, sp->base +
>>> MTK_NOR_REG_PRG_CNT);
>>> +	if (rx_len)
>>> +		writel(prg_len * BITS_PER_BYTE + sp->caps-
>>>> extra_dummy_bit,
>>> +		       sp->base + MTK_NOR_REG_PRG_CNT);
>>> +	else
>>> +		writel(prg_len * BITS_PER_BYTE, sp->base +
>>> MTK_NOR_REG_PRG_CNT);
>>> +
>>>    	ret = mtk_nor_cmd_exec(sp, MTK_NOR_CMD_PROGRAM,
>>>    			       prg_len * BITS_PER_BYTE);
>>>    	if (ret)
>>> @@ -743,9 +754,19 @@ static const struct spi_controller_mem_ops
>>> mtk_nor_mem_ops = {
>>>    	.exec_op = mtk_nor_exec_op
>>>    };
>>>    
>>> +const struct mtk_nor_caps mtk_nor_caps_mt8173 = {
>>> +	.dma_bits = 32,
>>> +	.extra_dummy_bit = 0,
>>> +};
>>> +
>>> +const struct mtk_nor_caps mtk_nor_caps_mt8192 = {
>>> +	.dma_bits = 36,
>>> +	.extra_dummy_bit = 0,
>>> +};
>>> +
>>>    static const struct of_device_id mtk_nor_match[] = {
>>> -	{ .compatible = "mediatek,mt8192-nor", .data = (void *)36 },
>>> -	{ .compatible = "mediatek,mt8173-nor", .data = (void *)32 },
>>> +	{ .compatible = "mediatek,mt8173-nor", .data =
>>> &mtk_nor_caps_mt8173 },
>>> +	{ .compatible = "mediatek,mt8192-nor", .data =
>>> &mtk_nor_caps_mt8192 },
>>>    	{ /* sentinel */ }
>>    };
>>>    MODULE_DEVICE_TABLE(of, mtk_nor_match);
>>> @@ -754,10 +775,10 @@ static int mtk_nor_probe(struct
>>> platform_device *pdev)
>>>    {
>>>    	struct spi_controller *ctlr;
>>>    	struct mtk_nor *sp;
>>> +	struct mtk_nor_caps *caps;
>>>    	void __iomem *base;
>>>    	struct clk *spi_clk, *ctlr_clk, *axi_clk;
>>>    	int ret, irq;
>>> -	unsigned long dma_bits;
>>>    
>>>    	base = devm_platform_ioremap_resource(pdev, 0);
>>>    	if (IS_ERR(base))
>>> @@ -775,9 +796,11 @@ static int mtk_nor_probe(struct
>>> platform_device *pdev)
>>>    	if (IS_ERR(axi_clk))
>>>    		return PTR_ERR(axi_clk);
>>    
>>> -	dma_bits = (unsigned long)of_device_get_match_data(&pdev->dev);
>>> -	if (dma_set_mask_and_coherent(&pdev->dev,
>>> DMA_BIT_MASK(dma_bits))) {
>>> -		dev_err(&pdev->dev, "failed to set dma mask(%lu)\n",
>>> dma_bits);
>>> +	caps = (struct mtk_nor_caps *)of_device_get_match_data(&pdev-
>>>> dev);
>>> +	if (dma_set_mask_and_coherent(&pdev->dev,
>>> +				      DMA_BIT_MASK(caps->dma_bits))) {
>>
>> While you're at it, can you please also make this return the right
>> value?
>> The function dma_set_mask_and_coherent() may return an error code, so
>> it's
>> worth it to just return that.
>>
>> P.S.: 83 columns is ok
>>
> 
> ok, I'll fix it next version.
> Thanks.
> 
> BR,
> Guochun
> 


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH v1 2/4] spi: spi-mtk-nor: improve device table for adding more capabilities

[Guochun Mao]

On Mon, 2022-01-17 at 11:16 +0100, AngeloGioacchino Del Regno wrote:
> Il 17/01/22 03:30, Guochun Mao ha scritto:
> > Hello Angelo,
> > 
> > Thanks for your comments.
> > 
> > On Fri, 2022-01-14 at 11:40 +0100, AngeloGioacchino Del Regno
> > wrote:
> > > Il 14/01/22 07:24, guochun.mao@mediatek.com ha scritto:
> > > > From: Guochun Mao <guochun.mao@mediatek.com>
> > > 
> > > Hello Guochun,
> > > 
> > > thanks for the patch! However, I have some comments...
> > > 
> > > > 
> > > > Define a structure for adding more capabilities.
> > > 
> > > Please also mention that you're adding the extra_dummy_bit
> > > feature
> > > in the commit description.
> > > 
> > > > 
> > > > +struct mtk_nor_caps {
> > > > +	u32 dma_bits;
> > > 
> > > dma_bits can be u8: I really don't think that we'll ever see more
> > > than
> > > 0xff bits...
> > > 
> > 
> > I'll fix it next version.
> > 
> > > > +	u32 extra_dummy_bit;
> > > 
> > > This one looks a bit strange, can you please add a comment
> > > explaining
> > > the
> > > reason why we have this "extra dummy bit"?
> > > 
> > 
> > This one is adding for mt8186, which extra_dummy_bit is set to be
> > 1.
> > Due to the IP design, it need a extra_dummy bit when reading nor
> > flash's register(status/config...).
> > 
> > Do you think I should move the modification to another patch
> > "[PATCH v1 3/4]spi:spi-mtk-nor:add new soc mt8186 sopport" ?
> > Or I just add this comment here?
> > 
> 
> It's more appropriate to keep it in this patch and mentioning it into
> the commit description. Also, please add this comment in the code
> and,
> if possible, also explain why this IP design needs an extra dummy
> bit.
> Otherwise, if not possible, the provided explaination would also be
> fine.
> 
> Thanks,
> - Angelo
> 

OK, thanks.

BR,
Guochun

> > > > 
> > > >    	// trigger op
> > > > -	writel(prg_len * BITS_PER_BYTE, sp->base +
> > > > MTK_NOR_REG_PRG_CNT);
> > > > +	if (rx_len)
> > > > +		writel(prg_len * BITS_PER_BYTE + sp->caps-
> > > > > extra_dummy_bit,
> > > > 
> > > > +		       sp->base + MTK_NOR_REG_PRG_CNT);
> > > > +	else
> > > > +		writel(prg_len * BITS_PER_BYTE, sp->base +
> > > > MTK_NOR_REG_PRG_CNT);
> > > > +
> > > >    	ret = mtk_nor_cmd_exec(sp, MTK_NOR_CMD_PROGRAM,
> > > >    			       prg_len * BITS_PER_BYTE);
> > > >    	if (ret)
> > > > @@ -743,9 +754,19 @@ static const struct spi_controller_mem_ops
> > > > mtk_nor_mem_ops = {
> > > >    	.exec_op = mtk_nor_exec_op
> > > >    };
> > > >    
> > > > +const struct mtk_nor_caps mtk_nor_caps_mt8173 = {
> > > > +	.dma_bits = 32,
> > > > +	.extra_dummy_bit = 0,
> > > > +};
> > > > +
> > > > +const struct mtk_nor_caps mtk_nor_caps_mt8192 = {
> > > > +	.dma_bits = 36,
> > > > +	.extra_dummy_bit = 0,
> > > > +};
> > > > +
> > > >    static const struct of_device_id mtk_nor_match[] = {
> > > > -	{ .compatible = "mediatek,mt8192-nor", .data = (void
> > > > *)36 },
> > > > -	{ .compatible = "mediatek,mt8173-nor", .data = (void
> > > > *)32 },
> > > > +	{ .compatible = "mediatek,mt8173-nor", .data =
> > > > &mtk_nor_caps_mt8173 },
> > > > +	{ .compatible = "mediatek,mt8192-nor", .data =
> > > > &mtk_nor_caps_mt8192 },
> > > >    	{ /* sentinel */ }
> > > 
> > >    };
> > > >    MODULE_DEVICE_TABLE(of, mtk_nor_match);
> > > > @@ -754,10 +775,10 @@ static int mtk_nor_probe(struct
> > > > platform_device *pdev)
> > > >    {
> > > >    	struct spi_controller *ctlr;
> > > >    	struct mtk_nor *sp;
> > > > +	struct mtk_nor_caps *caps;
> > > >    	void __iomem *base;
> > > >    	struct clk *spi_clk, *ctlr_clk, *axi_clk;
> > > >    	int ret, irq;
> > > > -	unsigned long dma_bits;
> > > >    
> > > >    	base = devm_platform_ioremap_resource(pdev, 0);
> > > >    	if (IS_ERR(base))
> > > > @@ -775,9 +796,11 @@ static int mtk_nor_probe(struct
> > > > platform_device *pdev)
> > > >    	if (IS_ERR(axi_clk))
> > > >    		return PTR_ERR(axi_clk);
> > > 
> > >    
> > > > -	dma_bits = (unsigned
> > > > long)of_device_get_match_data(&pdev->dev);
> > > > -	if (dma_set_mask_and_coherent(&pdev->dev,
> > > > DMA_BIT_MASK(dma_bits))) {
> > > > -		dev_err(&pdev->dev, "failed to set dma
> > > > mask(%lu)\n",
> > > > dma_bits);
> > > > +	caps = (struct mtk_nor_caps
> > > > *)of_device_get_match_data(&pdev-
> > > > > dev);
> > > > 
> > > > +	if (dma_set_mask_and_coherent(&pdev->dev,
> > > > +				      DMA_BIT_MASK(caps-
> > > > >dma_bits))) {
> > > 
> > > While you're at it, can you please also make this return the
> > > right
> > > value?
> > > The function dma_set_mask_and_coherent() may return an error
> > > code, so
> > > it's
> > > worth it to just return that.
> > > 
> > > P.S.: 83 columns is ok
> > > 
> > 
> > ok, I'll fix it next version.
> > Thanks.
> > 
> > BR,
> > Guochun
> > 
> 
> 
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[PATCH v1 3/4] spi: spi-mtk-nor: add new soc mt8186 support

[guochun.mao]

From: Guochun Mao <guochun.mao@mediatek.com>

Add compatible mediatek,mt8186-nor implementation.

Signed-off-by: Guochun Mao <guochun.mao@mediatek.com>
Signed-off-by: Zhen Zhang <zhen.zhang@mediatek.com>
---
 drivers/spi/spi-mtk-nor.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/spi/spi-mtk-nor.c b/drivers/spi/spi-mtk-nor.c
index f3bcdc1d4d8b..f8f6e6c1b551 100644
--- a/drivers/spi/spi-mtk-nor.c
+++ b/drivers/spi/spi-mtk-nor.c
@@ -759,6 +759,11 @@ const struct mtk_nor_caps mtk_nor_caps_mt8173 = {
 	.extra_dummy_bit = 0,
 };
 
+const struct mtk_nor_caps mtk_nor_caps_mt8186 = {
+	.dma_bits = 32,
+	.extra_dummy_bit = 1,
+};
+
 const struct mtk_nor_caps mtk_nor_caps_mt8192 = {
 	.dma_bits = 36,
 	.extra_dummy_bit = 0,
@@ -766,6 +771,7 @@ const struct mtk_nor_caps mtk_nor_caps_mt8192 = {
 
 static const struct of_device_id mtk_nor_match[] = {
 	{ .compatible = "mediatek,mt8173-nor", .data = &mtk_nor_caps_mt8173 },
+	{ .compatible = "mediatek,mt8186-nor", .data = &mtk_nor_caps_mt8186 },
 	{ .compatible = "mediatek,mt8192-nor", .data = &mtk_nor_caps_mt8192 },
 	{ /* sentinel */ }
 };
-- 
2.25.1


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

[PATCH v1 4/4] spi: spi-mtk-nor: add axi_s clock for mt8186

[guochun.mao]

From: Guochun Mao <guochun.mao@mediatek.com>

MT8186 needs axi_s clock for DMA feature.

Signed-off-by: Guochun Mao <guochun.mao@mediatek.com>
Signed-off-by: Zhen Zhang <zhen.zhang@mediatek.com>
---
 drivers/spi/spi-mtk-nor.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-mtk-nor.c b/drivers/spi/spi-mtk-nor.c
index f8f6e6c1b551..4ccbac674ff7 100644
--- a/drivers/spi/spi-mtk-nor.c
+++ b/drivers/spi/spi-mtk-nor.c
@@ -109,6 +109,7 @@ struct mtk_nor {
 	struct clk *spi_clk;
 	struct clk *ctlr_clk;
 	struct clk *axi_clk;
+	struct clk *axi_s_clk;
 	unsigned int spi_freq;
 	bool wbuf_en;
 	bool has_irq;
@@ -685,6 +686,7 @@ static void mtk_nor_disable_clk(struct mtk_nor *sp)
 	clk_disable_unprepare(sp->spi_clk);
 	clk_disable_unprepare(sp->ctlr_clk);
 	clk_disable_unprepare(sp->axi_clk);
+	clk_disable_unprepare(sp->axi_s_clk);
 }
 
 static int mtk_nor_enable_clk(struct mtk_nor *sp)
@@ -708,6 +710,14 @@ static int mtk_nor_enable_clk(struct mtk_nor *sp)
 		return ret;
 	}
 
+	ret = clk_prepare_enable(sp->axi_s_clk);
+	if (ret) {
+		clk_disable_unprepare(sp->spi_clk);
+		clk_disable_unprepare(sp->ctlr_clk);
+		clk_disable_unprepare(sp->axi_clk);
+		return ret;
+	}
+
 	return 0;
 }
 
@@ -783,7 +793,7 @@ static int mtk_nor_probe(struct platform_device *pdev)
 	struct mtk_nor *sp;
 	struct mtk_nor_caps *caps;
 	void __iomem *base;
-	struct clk *spi_clk, *ctlr_clk, *axi_clk;
+	struct clk *spi_clk, *ctlr_clk, *axi_clk, *axi_s_clk;
 	int ret, irq;
 
 	base = devm_platform_ioremap_resource(pdev, 0);
@@ -802,6 +812,10 @@ static int mtk_nor_probe(struct platform_device *pdev)
 	if (IS_ERR(axi_clk))
 		return PTR_ERR(axi_clk);
 
+	axi_s_clk = devm_clk_get_optional(&pdev->dev, "axi_s");
+	if (IS_ERR(axi_s_clk))
+		return PTR_ERR(axi_s_clk);
+
 	caps = (struct mtk_nor_caps *)of_device_get_match_data(&pdev->dev);
 	if (dma_set_mask_and_coherent(&pdev->dev,
 				      DMA_BIT_MASK(caps->dma_bits))) {
@@ -837,6 +851,7 @@ static int mtk_nor_probe(struct platform_device *pdev)
 	sp->spi_clk = spi_clk;
 	sp->ctlr_clk = ctlr_clk;
 	sp->axi_clk = axi_clk;
+	sp->axi_s_clk = axi_s_clk;
 	sp->caps = caps;
 	sp->high_dma = caps->dma_bits > 32;
 	sp->buffer = dmam_alloc_coherent(&pdev->dev,
-- 
2.25.1


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH v1 4/4] spi: spi-mtk-nor: add axi_s clock for mt8186

[AngeloGioacchino Del Regno]

Il 14/01/22 07:24, guochun.mao@mediatek.com ha scritto:
> From: Guochun Mao <guochun.mao@mediatek.com>
> 
> MT8186 needs axi_s clock for DMA feature.
> 
> Signed-off-by: Guochun Mao <guochun.mao@mediatek.com>
> Signed-off-by: Zhen Zhang <zhen.zhang@mediatek.com>

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


_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek