1. Patchew
  2. linux
  3. View series

[v2] spi: spi-mtk-nor: Modify the clock architecture of nor controller

mtk22730 posted 1 patch 9 months, 2 weeks ago 
drivers/spi/spi-mtk-nor.c | 103 ++++++++++++++++++++------------------
1 file changed, 54 insertions(+), 49 deletions(-)
[v2] spi: spi-mtk-nor: Modify the clock architecture of nor controller
Posted by mtk22730 9 months, 2 weeks ago 
The clocks used by different platforms are not same. So it is
necessary to modify the clock architecture to be adaptable to more
platforms.

Signed-off-by: Cloud Zhang <cloud.zhang@mediatek.com>
---
Changes in v2:
  -Use clk_bulk_xxx related functions to enable/disable clocks.

Changes in v1:
  -Add new function mtk_nor_parse_clk() to parse nor clock parameters.
---
---
 drivers/spi/spi-mtk-nor.c | 103 ++++++++++++++++++++------------------
 1 file changed, 54 insertions(+), 49 deletions(-)

diff --git a/drivers/spi/spi-mtk-nor.c b/drivers/spi/spi-mtk-nor.c
index 85ab5ce96c4d..4863b9cb2706 100644
--- a/drivers/spi/spi-mtk-nor.c
+++ b/drivers/spi/spi-mtk-nor.c
@@ -99,6 +99,8 @@
 
 #define CLK_TO_US(sp, clkcnt)		DIV_ROUND_UP(clkcnt, sp->spi_freq / 1000000)
 
+#define MAX_CLOCK_CNT		6
+
 struct mtk_nor_caps {
 	u8 dma_bits;
 
@@ -116,10 +118,8 @@ struct mtk_nor {
 	void __iomem *base;
 	u8 *buffer;
 	dma_addr_t buffer_dma;
-	struct clk *spi_clk;
-	struct clk *ctlr_clk;
-	struct clk *axi_clk;
-	struct clk *axi_s_clk;
+	struct clk_bulk_data clocks[MAX_CLOCK_CNT];
+	int clock_cnt;
 	unsigned int spi_freq;
 	bool wbuf_en;
 	bool has_irq;
@@ -703,44 +703,68 @@ static int mtk_nor_transfer_one_message(struct spi_controller *host,
 
 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);
+	clk_bulk_disable_unprepare(sp->clock_cnt, sp->clocks);
 }
 
 static int mtk_nor_enable_clk(struct mtk_nor *sp)
 {
 	int ret;
+	int i;
 
-	ret = clk_prepare_enable(sp->spi_clk);
-	if (ret)
-		return ret;
-
-	ret = clk_prepare_enable(sp->ctlr_clk);
+	ret = clk_bulk_prepare_enable(sp->clock_cnt, sp->clocks);
 	if (ret) {
-		clk_disable_unprepare(sp->spi_clk);
+		dev_err(sp->dev, "enable clk failed\n");
 		return ret;
 	}
 
-	ret = clk_prepare_enable(sp->axi_clk);
-	if (ret) {
-		clk_disable_unprepare(sp->spi_clk);
-		clk_disable_unprepare(sp->ctlr_clk);
-		return ret;
-	}
+	for (i = 0; i < sp->clock_cnt; i++) {
+		if (IS_ERR(sp->clocks[i].clk)) {
+			dev_err(sp->dev, "get %s fail\n", sp->clocks[i].id);
+			return PTR_ERR(sp->clocks[i].clk);
+		}
 
-	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;
+		if (!strcmp(sp->clocks[i].id, "spi"))
+			sp->spi_freq = clk_get_rate(sp->clocks[i].clk);
 	}
 
 	return 0;
 }
 
+static int mtk_nor_parse_clk(struct device *dev, struct mtk_nor *sp)
+{
+	struct device_node *np = dev->of_node;
+	int ret;
+	const char *name;
+	int cnt, i;
+
+	cnt = of_property_count_strings(np, "clock-names");
+	if (!cnt || (cnt == -EINVAL)) {
+		dev_err(dev, "Unable to find clocks\n");
+		ret = -EINVAL;
+		goto out;
+	} else if (cnt < 0) {
+		dev_err(dev, "Count clock strings failed, err %d\n", cnt);
+		ret = cnt;
+		goto out;
+	}
+
+	sp->clock_cnt = cnt;
+	for (i = 0; i < cnt; i++) {
+		ret = of_property_read_string_index(np, "clock-names", i, &name);
+		if (ret) {
+			dev_err(dev, "failed to get clock string\n");
+			return ret;
+		}
+
+		sp->clocks[i].id = name;
+	}
+
+	ret = devm_clk_bulk_get(dev, sp->clock_cnt, sp->clocks);
+
+out:
+	return ret;
+}
+
 static void mtk_nor_init(struct mtk_nor *sp)
 {
 	writel(0, sp->base + MTK_NOR_REG_IRQ_EN);
@@ -813,29 +837,12 @@ 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, *axi_s_clk;
 	int ret, irq;
 
 	base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(base))
 		return PTR_ERR(base);
 
-	spi_clk = devm_clk_get(&pdev->dev, "spi");
-	if (IS_ERR(spi_clk))
-		return PTR_ERR(spi_clk);
-
-	ctlr_clk = devm_clk_get(&pdev->dev, "sf");
-	if (IS_ERR(ctlr_clk))
-		return PTR_ERR(ctlr_clk);
-
-	axi_clk = devm_clk_get_optional(&pdev->dev, "axi");
-	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);
 
 	ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(caps->dma_bits));
@@ -868,10 +875,6 @@ static int mtk_nor_probe(struct platform_device *pdev)
 	sp->wbuf_en = false;
 	sp->ctlr = ctlr;
 	sp->dev = &pdev->dev;
-	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,
@@ -885,11 +888,13 @@ static int mtk_nor_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
-	ret = mtk_nor_enable_clk(sp);
+	ret = mtk_nor_parse_clk(sp->dev, sp);
 	if (ret < 0)
 		return ret;
 
-	sp->spi_freq = clk_get_rate(sp->spi_clk);
+	ret = mtk_nor_enable_clk(sp);
+	if (ret < 0)
+		return ret;
 
 	mtk_nor_init(sp);
 
-- 
2.46.0
Re: [v2] spi: spi-mtk-nor: Modify the clock architecture of nor controller
Posted by AngeloGioacchino Del Regno 9 months, 2 weeks ago 
Il 03/03/25 12:45, mtk22730 ha scritto:
> The clocks used by different platforms are not same. So it is
> necessary to modify the clock architecture to be adaptable to more
> platforms.
> 
> Signed-off-by: Cloud Zhang <cloud.zhang@mediatek.com>

You really shall fix your identity on outgoing emails: your name appears as
"mtk22730", that's not right.

Also, the title could be clarified a bit...

spi: spi-mtk-nor: Migrate to clk_bulk API

...and then: this should be a series, and the first commit shall be adjusting
the dt-binding for this driver accordigly, otherwise whatever you're doing here
will turn out to be unusable.

> ---
> Changes in v2:
>    -Use clk_bulk_xxx related functions to enable/disable clocks.
> 
> Changes in v1:
>    -Add new function mtk_nor_parse_clk() to parse nor clock parameters.
> ---
> ---
>   drivers/spi/spi-mtk-nor.c | 103 ++++++++++++++++++++------------------
>   1 file changed, 54 insertions(+), 49 deletions(-)
> 
> diff --git a/drivers/spi/spi-mtk-nor.c b/drivers/spi/spi-mtk-nor.c
> index 85ab5ce96c4d..4863b9cb2706 100644
> --- a/drivers/spi/spi-mtk-nor.c
> +++ b/drivers/spi/spi-mtk-nor.c
> @@ -99,6 +99,8 @@
>   
>   #define CLK_TO_US(sp, clkcnt)		DIV_ROUND_UP(clkcnt, sp->spi_freq / 1000000)
>   
> +#define MAX_CLOCK_CNT		6
> +
>   struct mtk_nor_caps {
>   	u8 dma_bits;
>   
> @@ -116,10 +118,8 @@ struct mtk_nor {
>   	void __iomem *base;
>   	u8 *buffer;
>   	dma_addr_t buffer_dma;
> -	struct clk *spi_clk;
> -	struct clk *ctlr_clk;
> -	struct clk *axi_clk;
> -	struct clk *axi_s_clk;
> +	struct clk_bulk_data clocks[MAX_CLOCK_CNT];

What about having this as a pointer?

	struct clk_bulk_data *clks;

...Then you count the clocks, and devm allocate the number of clks that you need.

> +	int clock_cnt;

u8 clock_cnt;

>   	unsigned int spi_freq;
>   	bool wbuf_en;
>   	bool has_irq;
> @@ -703,44 +703,68 @@ static int mtk_nor_transfer_one_message(struct spi_controller *host,
>   
>   static void mtk_nor_disable_clk(struct mtk_nor *sp)

A function with one call means that you don't need a function at all.

>   {
> -	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);
> +	clk_bulk_disable_unprepare(sp->clock_cnt, sp->clocks);
>   }
>   
>   static int mtk_nor_enable_clk(struct mtk_nor *sp)
>   {

You can also remove this function and transfer the contents into mtk_nor_probe():
it's just something like 6 lines and even called only once, so... :-)

>   	int ret;
> +	int i;
>   
> -	ret = clk_prepare_enable(sp->spi_clk);
> -	if (ret)
> -		return ret;
> -
> -	ret = clk_prepare_enable(sp->ctlr_clk);
> +	ret = clk_bulk_prepare_enable(sp->clock_cnt, sp->clocks);
>   	if (ret) {
> -		clk_disable_unprepare(sp->spi_clk);
> +		dev_err(sp->dev, "enable clk failed\n");
>   		return ret;
>   	}
>   
> -	ret = clk_prepare_enable(sp->axi_clk);
> -	if (ret) {
> -		clk_disable_unprepare(sp->spi_clk);
> -		clk_disable_unprepare(sp->ctlr_clk);
> -		return ret;
> -	}
> +	for (i = 0; i < sp->clock_cnt; i++) {
> +		if (IS_ERR(sp->clocks[i].clk)) {
> +			dev_err(sp->dev, "get %s fail\n", sp->clocks[i].id);
> +			return PTR_ERR(sp->clocks[i].clk);
> +		}
>   
> -	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;
> +		if (!strcmp(sp->clocks[i].id, "spi"))
> +			sp->spi_freq = clk_get_rate(sp->clocks[i].clk);
>   	}
>   
>   	return 0;
>   }
>   
> +static int mtk_nor_parse_clk(struct device *dev, struct mtk_nor *sp)
> +{
> +	struct device_node *np = dev->of_node;
> +	int ret;
> +	const char *name;
> +	int cnt, i;

	struct device_node *np = dev->of_node;
	const char *name;
	int cnt, i, ret;


> +
> +	cnt = of_property_count_strings(np, "clock-names");
> +	if (!cnt || (cnt == -EINVAL)) {
> +		dev_err(dev, "Unable to find clocks\n");

return -EINVAL;

> +		ret = -EINVAL;
> +		goto out;
> +	} else if (cnt < 0) {
> +		dev_err(dev, "Count clock strings failed, err %d\n", cnt);

return cnt;

> +		ret = cnt;
> +		goto out;
> +	}
> +
> +	sp->clock_cnt = cnt;
> +	for (i = 0; i < cnt; i++) {
> +		ret = of_property_read_string_index(np, "clock-names", i, &name);
> +		if (ret) {
> +			dev_err(dev, "failed to get clock string\n");
> +			return ret;
> +		}
> +
> +		sp->clocks[i].id = name;
> +	}
> +
> +	ret = devm_clk_bulk_get(dev, sp->clock_cnt, sp->clocks);

	if (ret)
		return ret;

	return 0;

> +
> +out:
> +	return ret;
> +}
> +

Cheers,
Angelo

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.