| Message ID | 20220315032411.2826-7-leilk.liu@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | spi: mediatek: add single/quad mode support | expand |
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. > + 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. > + 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. > + clk_prepare_enable(mdata->spi_hclk); There's a typo. ret = clk_prepare_enable.... > + 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. > + 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
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 > >
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)) { + 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)) + 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)) { + 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; + } + } } ret = spi_master_resume(master); - if (ret < 0) + if (ret < 0) { clk_disable_unprepare(mdata->spi_clk); + if (!IS_ERR(mdata->spi_hclk)) + 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;
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(-)