All of lore.kernel.org
 help / color / mirror / Atom feed
From: Leilk Liu <leilk.liu@mediatek.com>
To: AngeloGioacchino Del Regno 
	<angelogioacchino.delregno@collabora.com>,
	"Mark Brown" <broonie@kernel.org>
Cc: Rob Herring <robh+dt@kernel.org>,
	Matthias Brugger <matthias.bgg@gmail.com>,
	<devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>,
	<linux-spi@vger.kernel.org>, <linux-mediatek@lists.infradead.org>
Subject: Re: [PATCH V4 6/6] spi: mediatek: support hclk
Date: Thu, 17 Mar 2022 17:14:42 +0800	[thread overview]
Message-ID: <42ab6ee6bdad6157a9d24197cd9d0311da0423e7.camel@mediatek.com> (raw)
In-Reply-To: <e3facd86-9e93-bda7-b6dd-c18f1660466a@collabora.com>

On Tue, 2022-03-15 at 10:37 +0100, AngeloGioacchino Del Regno wrote:
> Il 15/03/22 04:24, Leilk Liu ha scritto:
> > this patch adds hclk support.
> > 
> > Signed-off-by: Leilk Liu <leilk.liu@mediatek.com>
> > ---
> >   drivers/spi/spi-mt65xx.c | 85 ++++++++++++++++++++++++++++++++---
> > -----
> >   1 file changed, 69 insertions(+), 16 deletions(-)
> > 
> > diff --git a/drivers/spi/spi-mt65xx.c b/drivers/spi/spi-mt65xx.c
> > index 8958c3fa4fea..d4a602e78aa7 100644
> > --- a/drivers/spi/spi-mt65xx.c
> > +++ b/drivers/spi/spi-mt65xx.c
> > @@ -130,7 +130,7 @@ struct mtk_spi {
> >   	u32 state;
> >   	int pad_num;
> >   	u32 *pad_sel;
> > -	struct clk *parent_clk, *sel_clk, *spi_clk;
> > +	struct clk *parent_clk, *sel_clk, *spi_clk, *spi_hclk;
> >   	struct spi_transfer *cur_transfer;
> >   	u32 xfer_len;
> >   	u32 num_xfered;
> > @@ -1252,25 +1252,38 @@ static int mtk_spi_probe(struct
> > platform_device *pdev)
> >   		goto err_put_master;
> >   	}
> >   
> > +	mdata->spi_hclk = devm_clk_get(&pdev->dev, "hclk");
> > +	if (!IS_ERR(mdata->spi_hclk)) {
> 
> What you're doing here can be simplified by using
> devm_clk_get_optional() instead.
> Please use that.

Hi AngeloGioacchino, thanks for your suggestion, I'll fix it.

> 
> > +		ret = clk_prepare_enable(mdata->spi_hclk);
> > +		if (ret < 0) {
> > +			dev_err(&pdev->dev, "failed to enable hclk
> > (%d)\n", ret);
> > +			goto err_put_master;
> > +		}
> > +	}
> > +
> >   	ret = clk_prepare_enable(mdata->spi_clk);
> >   	if (ret < 0) {
> >   		dev_err(&pdev->dev, "failed to enable spi_clk (%d)\n",
> > ret);
> > -		goto err_put_master;
> > +		goto err_disable_spi_hclk;
> >   	}
> >   
> >   	ret = clk_set_parent(mdata->sel_clk, mdata->parent_clk);
> >   	if (ret < 0) {
> >   		dev_err(&pdev->dev, "failed to clk_set_parent (%d)\n",
> > ret);
> > -		clk_disable_unprepare(mdata->spi_clk);
> > -		goto err_put_master;
> > +		goto err_disable_spi_clk;
> >   	}
> >   
> >   	mdata->spi_clk_hz = clk_get_rate(mdata->spi_clk);
> >   
> > -	if (mdata->dev_comp->no_need_unprepare)
> > +	if (mdata->dev_comp->no_need_unprepare) {
> >   		clk_disable(mdata->spi_clk);
> > -	else
> > +		if (!IS_ERR(mdata->spi_hclk))
> > +			clk_disable(mdata->spi_hclk);
> > +	} else {
> >   		clk_disable_unprepare(mdata->spi_clk);
> > +		if (!IS_ERR(mdata->spi_hclk))
> > +			clk_disable_unprepare(mdata->spi_hclk);
> > +	}
> >   
> >   	pm_runtime_enable(&pdev->dev);
> >   
> > @@ -1310,6 +1323,11 @@ static int mtk_spi_probe(struct
> > platform_device *pdev)
> >   
> >   err_disable_runtime_pm:
> >   	pm_runtime_disable(&pdev->dev);
> > +err_disable_spi_clk:
> > +	clk_disable_unprepare(mdata->spi_clk);
> > +err_disable_spi_hclk:
> > +	if (!IS_ERR(mdata->spi_hclk))
> 
> When using devm_clk_get_optional(), you can simply omit this check,
> since it will
> be impossible to reach this point with an error pointer stored in
> spi_hclk.
OK, I'll fix it.

> 
> > +		clk_disable_unprepare(mdata->spi_hclk);
> >   err_put_master:
> >   	spi_master_put(master);
> >   
> > @@ -1325,8 +1343,11 @@ static int mtk_spi_remove(struct
> > platform_device *pdev)
> >   
> >   	mtk_spi_reset(mdata);
> >   
> > -	if (mdata->dev_comp->no_need_unprepare)
> > +	if (mdata->dev_comp->no_need_unprepare) {
> >   		clk_unprepare(mdata->spi_clk);
> > +		if (!IS_ERR(mdata->spi_hclk))
> > +			clk_unprepare(mdata->spi_hclk);
> > +	}
> >   
> >   	return 0;
> >   }
> > @@ -1342,8 +1363,11 @@ static int mtk_spi_suspend(struct device
> > *dev)
> >   	if (ret)
> >   		return ret;
> >   
> > -	if (!pm_runtime_suspended(dev))
> > +	if (!pm_runtime_suspended(dev)) {
> >   		clk_disable_unprepare(mdata->spi_clk);
> > +		if (!IS_ERR(mdata->spi_hclk))
> > +			clk_disable_unprepare(mdata->spi_hclk);
> > +	}
> >   
> >   	return ret;
> >   }
> > @@ -1360,11 +1384,23 @@ static int mtk_spi_resume(struct device
> > *dev)
> >   			dev_err(dev, "failed to enable spi_clk (%d)\n",
> > ret);
> >   			return ret;
> >   		}
> > +
> > +		if (!IS_ERR(mdata->spi_hclk)) {
> 
> Since you will be using devm_clk_get_optional(), you can also omit
> this check.
OK, I'll fix it.

> 
> > +			clk_prepare_enable(mdata->spi_hclk);
> 
> There's a typo. ret = clk_prepare_enable....
OK, I'll fix it.

> 
> > +			if (ret < 0) {
> > +				dev_err(dev, "failed to enable spi_hclk
> > (%d)\n", ret);
> > +				clk_disable_unprepare(mdata->spi_clk);
> > +				return ret;
> > +			}
> > +		}
> >   	}
> >   
> >   	ret = spi_master_resume(master);
> > -	if (ret < 0)
> > +	if (ret < 0) {
> >   		clk_disable_unprepare(mdata->spi_clk);
> > +		if (!IS_ERR(mdata->spi_hclk))
> 
> Same here and everywhere else, no error check if you set this as
> optional clock.
OK, I'll fix it.

> 
> > +			clk_disable_unprepare(mdata->spi_hclk);
> > +	}
> >   
> >   	return ret;
> >   }
> > @@ -1376,10 +1412,15 @@ static int mtk_spi_runtime_suspend(struct
> > device *dev)
> >   	struct spi_master *master = dev_get_drvdata(dev);
> >   	struct mtk_spi *mdata = spi_master_get_devdata(master);
> >   
> > -	if (mdata->dev_comp->no_need_unprepare)
> > +	if (mdata->dev_comp->no_need_unprepare) {
> >   		clk_disable(mdata->spi_clk);
> > -	else
> > +		if (!IS_ERR(mdata->spi_hclk))
> > +			clk_disable(mdata->spi_hclk);
> > +	} else {
> >   		clk_disable_unprepare(mdata->spi_clk);
> > +		if (!IS_ERR(mdata->spi_hclk))
> > +			clk_disable_unprepare(mdata->spi_hclk);
> > +	}
> >   
> >   	return 0;
> >   }
> > @@ -1390,13 +1431,25 @@ static int mtk_spi_runtime_resume(struct
> > device *dev)
> >   	struct mtk_spi *mdata = spi_master_get_devdata(master);
> >   	int ret;
> >   
> > -	if (mdata->dev_comp->no_need_unprepare)
> > +	if (mdata->dev_comp->no_need_unprepare) {
> >   		ret = clk_enable(mdata->spi_clk);
> > -	else
> > +		if (!IS_ERR(mdata->spi_hclk))
> > +			clk_enable(mdata->spi_hclk);
> > +	} else {
> >   		ret = clk_prepare_enable(mdata->spi_clk);
> > -	if (ret < 0) {
> > -		dev_err(dev, "failed to enable spi_clk (%d)\n", ret);
> > -		return ret;
> > +		if (ret < 0) {
> > +			dev_err(dev, "failed to enable spi_clk (%d)\n",
> > ret);
> > +			return ret;
> > +		}
> > +
> > +		if (!IS_ERR(mdata->spi_hclk)) {
> > +			ret = clk_prepare_enable(mdata->spi_hclk);
> > +			if (ret < 0) {
> > +				dev_err(dev, "failed to enable spi_hclk
> > (%d)\n", ret);
> > +				clk_disable_unprepare(mdata->spi_clk);
> > +				return ret;
> > +			}
> > +		}
> >   	}
> >   
> >   	return 0;
> > 
> 
> 
> Regards,
> Angelo
> 
> 


WARNING: multiple messages have this Message-ID (diff)
From: Leilk Liu <leilk.liu@mediatek.com>
To: AngeloGioacchino Del Regno
	<angelogioacchino.delregno@collabora.com>,
	"Mark Brown" <broonie@kernel.org>
Cc: Rob Herring <robh+dt@kernel.org>,
	Matthias Brugger <matthias.bgg@gmail.com>,
	<devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>,
	<linux-spi@vger.kernel.org>, <linux-mediatek@lists.infradead.org>
Subject: Re: [PATCH V4 6/6] spi: mediatek: support hclk
Date: Thu, 17 Mar 2022 17:14:42 +0800	[thread overview]
Message-ID: <42ab6ee6bdad6157a9d24197cd9d0311da0423e7.camel@mediatek.com> (raw)
In-Reply-To: <e3facd86-9e93-bda7-b6dd-c18f1660466a@collabora.com>

On Tue, 2022-03-15 at 10:37 +0100, AngeloGioacchino Del Regno wrote:
> Il 15/03/22 04:24, Leilk Liu ha scritto:
> > this patch adds hclk support.
> > 
> > Signed-off-by: Leilk Liu <leilk.liu@mediatek.com>
> > ---
> >   drivers/spi/spi-mt65xx.c | 85 ++++++++++++++++++++++++++++++++---
> > -----
> >   1 file changed, 69 insertions(+), 16 deletions(-)
> > 
> > diff --git a/drivers/spi/spi-mt65xx.c b/drivers/spi/spi-mt65xx.c
> > index 8958c3fa4fea..d4a602e78aa7 100644
> > --- a/drivers/spi/spi-mt65xx.c
> > +++ b/drivers/spi/spi-mt65xx.c
> > @@ -130,7 +130,7 @@ struct mtk_spi {
> >   	u32 state;
> >   	int pad_num;
> >   	u32 *pad_sel;
> > -	struct clk *parent_clk, *sel_clk, *spi_clk;
> > +	struct clk *parent_clk, *sel_clk, *spi_clk, *spi_hclk;
> >   	struct spi_transfer *cur_transfer;
> >   	u32 xfer_len;
> >   	u32 num_xfered;
> > @@ -1252,25 +1252,38 @@ static int mtk_spi_probe(struct
> > platform_device *pdev)
> >   		goto err_put_master;
> >   	}
> >   
> > +	mdata->spi_hclk = devm_clk_get(&pdev->dev, "hclk");
> > +	if (!IS_ERR(mdata->spi_hclk)) {
> 
> What you're doing here can be simplified by using
> devm_clk_get_optional() instead.
> Please use that.

Hi AngeloGioacchino, thanks for your suggestion, I'll fix it.

> 
> > +		ret = clk_prepare_enable(mdata->spi_hclk);
> > +		if (ret < 0) {
> > +			dev_err(&pdev->dev, "failed to enable hclk
> > (%d)\n", ret);
> > +			goto err_put_master;
> > +		}
> > +	}
> > +
> >   	ret = clk_prepare_enable(mdata->spi_clk);
> >   	if (ret < 0) {
> >   		dev_err(&pdev->dev, "failed to enable spi_clk (%d)\n",
> > ret);
> > -		goto err_put_master;
> > +		goto err_disable_spi_hclk;
> >   	}
> >   
> >   	ret = clk_set_parent(mdata->sel_clk, mdata->parent_clk);
> >   	if (ret < 0) {
> >   		dev_err(&pdev->dev, "failed to clk_set_parent (%d)\n",
> > ret);
> > -		clk_disable_unprepare(mdata->spi_clk);
> > -		goto err_put_master;
> > +		goto err_disable_spi_clk;
> >   	}
> >   
> >   	mdata->spi_clk_hz = clk_get_rate(mdata->spi_clk);
> >   
> > -	if (mdata->dev_comp->no_need_unprepare)
> > +	if (mdata->dev_comp->no_need_unprepare) {
> >   		clk_disable(mdata->spi_clk);
> > -	else
> > +		if (!IS_ERR(mdata->spi_hclk))
> > +			clk_disable(mdata->spi_hclk);
> > +	} else {
> >   		clk_disable_unprepare(mdata->spi_clk);
> > +		if (!IS_ERR(mdata->spi_hclk))
> > +			clk_disable_unprepare(mdata->spi_hclk);
> > +	}
> >   
> >   	pm_runtime_enable(&pdev->dev);
> >   
> > @@ -1310,6 +1323,11 @@ static int mtk_spi_probe(struct
> > platform_device *pdev)
> >   
> >   err_disable_runtime_pm:
> >   	pm_runtime_disable(&pdev->dev);
> > +err_disable_spi_clk:
> > +	clk_disable_unprepare(mdata->spi_clk);
> > +err_disable_spi_hclk:
> > +	if (!IS_ERR(mdata->spi_hclk))
> 
> When using devm_clk_get_optional(), you can simply omit this check,
> since it will
> be impossible to reach this point with an error pointer stored in
> spi_hclk.
OK, I'll fix it.

> 
> > +		clk_disable_unprepare(mdata->spi_hclk);
> >   err_put_master:
> >   	spi_master_put(master);
> >   
> > @@ -1325,8 +1343,11 @@ static int mtk_spi_remove(struct
> > platform_device *pdev)
> >   
> >   	mtk_spi_reset(mdata);
> >   
> > -	if (mdata->dev_comp->no_need_unprepare)
> > +	if (mdata->dev_comp->no_need_unprepare) {
> >   		clk_unprepare(mdata->spi_clk);
> > +		if (!IS_ERR(mdata->spi_hclk))
> > +			clk_unprepare(mdata->spi_hclk);
> > +	}
> >   
> >   	return 0;
> >   }
> > @@ -1342,8 +1363,11 @@ static int mtk_spi_suspend(struct device
> > *dev)
> >   	if (ret)
> >   		return ret;
> >   
> > -	if (!pm_runtime_suspended(dev))
> > +	if (!pm_runtime_suspended(dev)) {
> >   		clk_disable_unprepare(mdata->spi_clk);
> > +		if (!IS_ERR(mdata->spi_hclk))
> > +			clk_disable_unprepare(mdata->spi_hclk);
> > +	}
> >   
> >   	return ret;
> >   }
> > @@ -1360,11 +1384,23 @@ static int mtk_spi_resume(struct device
> > *dev)
> >   			dev_err(dev, "failed to enable spi_clk (%d)\n",
> > ret);
> >   			return ret;
> >   		}
> > +
> > +		if (!IS_ERR(mdata->spi_hclk)) {
> 
> Since you will be using devm_clk_get_optional(), you can also omit
> this check.
OK, I'll fix it.

> 
> > +			clk_prepare_enable(mdata->spi_hclk);
> 
> There's a typo. ret = clk_prepare_enable....
OK, I'll fix it.

> 
> > +			if (ret < 0) {
> > +				dev_err(dev, "failed to enable spi_hclk
> > (%d)\n", ret);
> > +				clk_disable_unprepare(mdata->spi_clk);
> > +				return ret;
> > +			}
> > +		}
> >   	}
> >   
> >   	ret = spi_master_resume(master);
> > -	if (ret < 0)
> > +	if (ret < 0) {
> >   		clk_disable_unprepare(mdata->spi_clk);
> > +		if (!IS_ERR(mdata->spi_hclk))
> 
> Same here and everywhere else, no error check if you set this as
> optional clock.
OK, I'll fix it.

> 
> > +			clk_disable_unprepare(mdata->spi_hclk);
> > +	}
> >   
> >   	return ret;
> >   }
> > @@ -1376,10 +1412,15 @@ static int mtk_spi_runtime_suspend(struct
> > device *dev)
> >   	struct spi_master *master = dev_get_drvdata(dev);
> >   	struct mtk_spi *mdata = spi_master_get_devdata(master);
> >   
> > -	if (mdata->dev_comp->no_need_unprepare)
> > +	if (mdata->dev_comp->no_need_unprepare) {
> >   		clk_disable(mdata->spi_clk);
> > -	else
> > +		if (!IS_ERR(mdata->spi_hclk))
> > +			clk_disable(mdata->spi_hclk);
> > +	} else {
> >   		clk_disable_unprepare(mdata->spi_clk);
> > +		if (!IS_ERR(mdata->spi_hclk))
> > +			clk_disable_unprepare(mdata->spi_hclk);
> > +	}
> >   
> >   	return 0;
> >   }
> > @@ -1390,13 +1431,25 @@ static int mtk_spi_runtime_resume(struct
> > device *dev)
> >   	struct mtk_spi *mdata = spi_master_get_devdata(master);
> >   	int ret;
> >   
> > -	if (mdata->dev_comp->no_need_unprepare)
> > +	if (mdata->dev_comp->no_need_unprepare) {
> >   		ret = clk_enable(mdata->spi_clk);
> > -	else
> > +		if (!IS_ERR(mdata->spi_hclk))
> > +			clk_enable(mdata->spi_hclk);
> > +	} else {
> >   		ret = clk_prepare_enable(mdata->spi_clk);
> > -	if (ret < 0) {
> > -		dev_err(dev, "failed to enable spi_clk (%d)\n", ret);
> > -		return ret;
> > +		if (ret < 0) {
> > +			dev_err(dev, "failed to enable spi_clk (%d)\n",
> > ret);
> > +			return ret;
> > +		}
> > +
> > +		if (!IS_ERR(mdata->spi_hclk)) {
> > +			ret = clk_prepare_enable(mdata->spi_hclk);
> > +			if (ret < 0) {
> > +				dev_err(dev, "failed to enable spi_hclk
> > (%d)\n", ret);
> > +				clk_disable_unprepare(mdata->spi_clk);
> > +				return ret;
> > +			}
> > +		}
> >   	}
> >   
> >   	return 0;
> > 
> 
> 
> Regards,
> Angelo
> 
> 
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

WARNING: multiple messages have this Message-ID (diff)
From: Leilk Liu <leilk.liu@mediatek.com>
To: AngeloGioacchino Del Regno
	<angelogioacchino.delregno@collabora.com>,
	"Mark Brown" <broonie@kernel.org>
Cc: Rob Herring <robh+dt@kernel.org>,
	Matthias Brugger <matthias.bgg@gmail.com>,
	<devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>,
	<linux-spi@vger.kernel.org>, <linux-mediatek@lists.infradead.org>
Subject: Re: [PATCH V4 6/6] spi: mediatek: support hclk
Date: Thu, 17 Mar 2022 17:14:42 +0800	[thread overview]
Message-ID: <42ab6ee6bdad6157a9d24197cd9d0311da0423e7.camel@mediatek.com> (raw)
In-Reply-To: <e3facd86-9e93-bda7-b6dd-c18f1660466a@collabora.com>

On Tue, 2022-03-15 at 10:37 +0100, AngeloGioacchino Del Regno wrote:
> Il 15/03/22 04:24, Leilk Liu ha scritto:
> > this patch adds hclk support.
> > 
> > Signed-off-by: Leilk Liu <leilk.liu@mediatek.com>
> > ---
> >   drivers/spi/spi-mt65xx.c | 85 ++++++++++++++++++++++++++++++++---
> > -----
> >   1 file changed, 69 insertions(+), 16 deletions(-)
> > 
> > diff --git a/drivers/spi/spi-mt65xx.c b/drivers/spi/spi-mt65xx.c
> > index 8958c3fa4fea..d4a602e78aa7 100644
> > --- a/drivers/spi/spi-mt65xx.c
> > +++ b/drivers/spi/spi-mt65xx.c
> > @@ -130,7 +130,7 @@ struct mtk_spi {
> >   	u32 state;
> >   	int pad_num;
> >   	u32 *pad_sel;
> > -	struct clk *parent_clk, *sel_clk, *spi_clk;
> > +	struct clk *parent_clk, *sel_clk, *spi_clk, *spi_hclk;
> >   	struct spi_transfer *cur_transfer;
> >   	u32 xfer_len;
> >   	u32 num_xfered;
> > @@ -1252,25 +1252,38 @@ static int mtk_spi_probe(struct
> > platform_device *pdev)
> >   		goto err_put_master;
> >   	}
> >   
> > +	mdata->spi_hclk = devm_clk_get(&pdev->dev, "hclk");
> > +	if (!IS_ERR(mdata->spi_hclk)) {
> 
> What you're doing here can be simplified by using
> devm_clk_get_optional() instead.
> Please use that.

Hi AngeloGioacchino, thanks for your suggestion, I'll fix it.

> 
> > +		ret = clk_prepare_enable(mdata->spi_hclk);
> > +		if (ret < 0) {
> > +			dev_err(&pdev->dev, "failed to enable hclk
> > (%d)\n", ret);
> > +			goto err_put_master;
> > +		}
> > +	}
> > +
> >   	ret = clk_prepare_enable(mdata->spi_clk);
> >   	if (ret < 0) {
> >   		dev_err(&pdev->dev, "failed to enable spi_clk (%d)\n",
> > ret);
> > -		goto err_put_master;
> > +		goto err_disable_spi_hclk;
> >   	}
> >   
> >   	ret = clk_set_parent(mdata->sel_clk, mdata->parent_clk);
> >   	if (ret < 0) {
> >   		dev_err(&pdev->dev, "failed to clk_set_parent (%d)\n",
> > ret);
> > -		clk_disable_unprepare(mdata->spi_clk);
> > -		goto err_put_master;
> > +		goto err_disable_spi_clk;
> >   	}
> >   
> >   	mdata->spi_clk_hz = clk_get_rate(mdata->spi_clk);
> >   
> > -	if (mdata->dev_comp->no_need_unprepare)
> > +	if (mdata->dev_comp->no_need_unprepare) {
> >   		clk_disable(mdata->spi_clk);
> > -	else
> > +		if (!IS_ERR(mdata->spi_hclk))
> > +			clk_disable(mdata->spi_hclk);
> > +	} else {
> >   		clk_disable_unprepare(mdata->spi_clk);
> > +		if (!IS_ERR(mdata->spi_hclk))
> > +			clk_disable_unprepare(mdata->spi_hclk);
> > +	}
> >   
> >   	pm_runtime_enable(&pdev->dev);
> >   
> > @@ -1310,6 +1323,11 @@ static int mtk_spi_probe(struct
> > platform_device *pdev)
> >   
> >   err_disable_runtime_pm:
> >   	pm_runtime_disable(&pdev->dev);
> > +err_disable_spi_clk:
> > +	clk_disable_unprepare(mdata->spi_clk);
> > +err_disable_spi_hclk:
> > +	if (!IS_ERR(mdata->spi_hclk))
> 
> When using devm_clk_get_optional(), you can simply omit this check,
> since it will
> be impossible to reach this point with an error pointer stored in
> spi_hclk.
OK, I'll fix it.

> 
> > +		clk_disable_unprepare(mdata->spi_hclk);
> >   err_put_master:
> >   	spi_master_put(master);
> >   
> > @@ -1325,8 +1343,11 @@ static int mtk_spi_remove(struct
> > platform_device *pdev)
> >   
> >   	mtk_spi_reset(mdata);
> >   
> > -	if (mdata->dev_comp->no_need_unprepare)
> > +	if (mdata->dev_comp->no_need_unprepare) {
> >   		clk_unprepare(mdata->spi_clk);
> > +		if (!IS_ERR(mdata->spi_hclk))
> > +			clk_unprepare(mdata->spi_hclk);
> > +	}
> >   
> >   	return 0;
> >   }
> > @@ -1342,8 +1363,11 @@ static int mtk_spi_suspend(struct device
> > *dev)
> >   	if (ret)
> >   		return ret;
> >   
> > -	if (!pm_runtime_suspended(dev))
> > +	if (!pm_runtime_suspended(dev)) {
> >   		clk_disable_unprepare(mdata->spi_clk);
> > +		if (!IS_ERR(mdata->spi_hclk))
> > +			clk_disable_unprepare(mdata->spi_hclk);
> > +	}
> >   
> >   	return ret;
> >   }
> > @@ -1360,11 +1384,23 @@ static int mtk_spi_resume(struct device
> > *dev)
> >   			dev_err(dev, "failed to enable spi_clk (%d)\n",
> > ret);
> >   			return ret;
> >   		}
> > +
> > +		if (!IS_ERR(mdata->spi_hclk)) {
> 
> Since you will be using devm_clk_get_optional(), you can also omit
> this check.
OK, I'll fix it.

> 
> > +			clk_prepare_enable(mdata->spi_hclk);
> 
> There's a typo. ret = clk_prepare_enable....
OK, I'll fix it.

> 
> > +			if (ret < 0) {
> > +				dev_err(dev, "failed to enable spi_hclk
> > (%d)\n", ret);
> > +				clk_disable_unprepare(mdata->spi_clk);
> > +				return ret;
> > +			}
> > +		}
> >   	}
> >   
> >   	ret = spi_master_resume(master);
> > -	if (ret < 0)
> > +	if (ret < 0) {
> >   		clk_disable_unprepare(mdata->spi_clk);
> > +		if (!IS_ERR(mdata->spi_hclk))
> 
> Same here and everywhere else, no error check if you set this as
> optional clock.
OK, I'll fix it.

> 
> > +			clk_disable_unprepare(mdata->spi_hclk);
> > +	}
> >   
> >   	return ret;
> >   }
> > @@ -1376,10 +1412,15 @@ static int mtk_spi_runtime_suspend(struct
> > device *dev)
> >   	struct spi_master *master = dev_get_drvdata(dev);
> >   	struct mtk_spi *mdata = spi_master_get_devdata(master);
> >   
> > -	if (mdata->dev_comp->no_need_unprepare)
> > +	if (mdata->dev_comp->no_need_unprepare) {
> >   		clk_disable(mdata->spi_clk);
> > -	else
> > +		if (!IS_ERR(mdata->spi_hclk))
> > +			clk_disable(mdata->spi_hclk);
> > +	} else {
> >   		clk_disable_unprepare(mdata->spi_clk);
> > +		if (!IS_ERR(mdata->spi_hclk))
> > +			clk_disable_unprepare(mdata->spi_hclk);
> > +	}
> >   
> >   	return 0;
> >   }
> > @@ -1390,13 +1431,25 @@ static int mtk_spi_runtime_resume(struct
> > device *dev)
> >   	struct mtk_spi *mdata = spi_master_get_devdata(master);
> >   	int ret;
> >   
> > -	if (mdata->dev_comp->no_need_unprepare)
> > +	if (mdata->dev_comp->no_need_unprepare) {
> >   		ret = clk_enable(mdata->spi_clk);
> > -	else
> > +		if (!IS_ERR(mdata->spi_hclk))
> > +			clk_enable(mdata->spi_hclk);
> > +	} else {
> >   		ret = clk_prepare_enable(mdata->spi_clk);
> > -	if (ret < 0) {
> > -		dev_err(dev, "failed to enable spi_clk (%d)\n", ret);
> > -		return ret;
> > +		if (ret < 0) {
> > +			dev_err(dev, "failed to enable spi_clk (%d)\n",
> > ret);
> > +			return ret;
> > +		}
> > +
> > +		if (!IS_ERR(mdata->spi_hclk)) {
> > +			ret = clk_prepare_enable(mdata->spi_hclk);
> > +			if (ret < 0) {
> > +				dev_err(dev, "failed to enable spi_hclk
> > (%d)\n", ret);
> > +				clk_disable_unprepare(mdata->spi_clk);
> > +				return ret;
> > +			}
> > +		}
> >   	}
> >   
> >   	return 0;
> > 
> 
> 
> Regards,
> Angelo
> 
> 
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2022-03-17  9:14 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-15  3:24 [PATCH V4 0/6] spi: mediatek: add single/quad mode support Leilk Liu
2022-03-15  3:24 ` Leilk Liu
2022-03-15  3:24 ` Leilk Liu
2022-03-15  3:24 ` [PATCH V4 1/6] spi: mediatek: support tick_delay without enhance_timing Leilk Liu
2022-03-15  3:24   ` Leilk Liu
2022-03-15  3:24   ` Leilk Liu
2022-03-15  3:24 ` [PATCH V4 2/6] dt-bindings: spi: Add compatible for MT7986 Leilk Liu
2022-03-15  3:24   ` Leilk Liu
2022-03-15  3:24   ` Leilk Liu
2022-03-15  9:38   ` AngeloGioacchino Del Regno
2022-03-15  9:38     ` AngeloGioacchino Del Regno
2022-03-15  9:38     ` AngeloGioacchino Del Regno
2022-03-15  3:24 ` [PATCH V4 3/6] spi: mediatek: add ipm design support " Leilk Liu
2022-03-15  3:24   ` Leilk Liu
2022-03-15  3:24   ` Leilk Liu
2022-03-15  9:38   ` AngeloGioacchino Del Regno
2022-03-15  9:38     ` AngeloGioacchino Del Regno
2022-03-15  9:38     ` AngeloGioacchino Del Regno
2022-03-15  9:39   ` AngeloGioacchino Del Regno
2022-03-15  9:39     ` AngeloGioacchino Del Regno
2022-03-15  9:39     ` AngeloGioacchino Del Regno
2022-03-15  3:24 ` [PATCH V4 4/6] spi: mediatek: add spi memory support for ipm design Leilk Liu
2022-03-15  3:24   ` Leilk Liu
2022-03-15  3:24   ` Leilk Liu
2022-03-15  9:31   ` AngeloGioacchino Del Regno
2022-03-15  9:31     ` AngeloGioacchino Del Regno
2022-03-15  9:31     ` AngeloGioacchino Del Regno
2022-03-17  9:27     ` Leilk Liu
2022-03-17  9:27       ` Leilk Liu
2022-03-17  9:27       ` Leilk Liu
2022-03-17  9:33       ` AngeloGioacchino Del Regno
2022-03-17  9:33         ` AngeloGioacchino Del Regno
2022-03-17  9:33         ` AngeloGioacchino Del Regno
2022-03-17  9:44         ` Leilk Liu
2022-03-17  9:44           ` Leilk Liu
2022-03-17  9:44           ` Leilk Liu
2022-03-15  3:24 ` [PATCH V4 5/6] dt-bindings: spi: support hclk Leilk Liu
2022-03-15  3:24   ` Leilk Liu
2022-03-15  3:24   ` Leilk Liu
2022-03-15  3:24 ` [PATCH V4 6/6] spi: mediatek: " Leilk Liu
2022-03-15  3:24   ` Leilk Liu
2022-03-15  3:24   ` Leilk Liu
2022-03-15  9:37   ` AngeloGioacchino Del Regno
2022-03-15  9:37     ` AngeloGioacchino Del Regno
2022-03-15  9:37     ` AngeloGioacchino Del Regno
2022-03-17  9:14     ` Leilk Liu [this message]
2022-03-17  9:14       ` Leilk Liu
2022-03-17  9:14       ` Leilk Liu
2022-03-15 13:41 ` (subset) [PATCH V4 0/6] spi: mediatek: add single/quad mode support Mark Brown
2022-03-15 13:41   ` Mark Brown
2022-03-15 13:41   ` Mark Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=42ab6ee6bdad6157a9d24197cd9d0311da0423e7.camel@mediatek.com \
    --to=leilk.liu@mediatek.com \
    --cc=angelogioacchino.delregno@collabora.com \
    --cc=broonie@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=linux-spi@vger.kernel.org \
    --cc=matthias.bgg@gmail.com \
    --cc=robh+dt@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.