| Message ID | 20240611171600.1105124-3-aniketmaurya@google.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | i3c: dw: Add APB clk | expand |
Hello, On 11/06/2024 17:16:00+0000, Aniket wrote: > Besides the core clock, IP also has an apb > interface clock. Add an optional hook for > the same and appropriately enable and disable. > > Signed-off-by: Aniket <aniketmaurya@google.com> > --- > drivers/i3c/master/dw-i3c-master.c | 12 ++++++++++++ > drivers/i3c/master/dw-i3c-master.h | 1 + > 2 files changed, 13 insertions(+) > > diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c > index 77a2a1c3fd1d..41cdfd6741e3 100644 > --- a/drivers/i3c/master/dw-i3c-master.c > +++ b/drivers/i3c/master/dw-i3c-master.c > @@ -1470,12 +1470,20 @@ int dw_i3c_common_probe(struct dw_i3c_master *master, > if (IS_ERR(master->core_clk)) > return PTR_ERR(master->core_clk); > > + master->pclk = devm_clk_get_optional(&pdev->dev, "pclk"); > + if (IS_ERR(master->pclk)) > + return PTR_ERR(master->pclk); > + I guess you need to update the device tree binding documentation too. > master->core_rst = devm_reset_control_get_optional_exclusive(&pdev->dev, > "core_rst"); > if (IS_ERR(master->core_rst)) > return PTR_ERR(master->core_rst); > > ret = clk_prepare_enable(master->core_clk); > + if (ret) > + return ret; > + > + ret = clk_prepare_enable(master->pclk); > if (ret) > goto err_disable_core_clk; > > @@ -1520,6 +1528,8 @@ int dw_i3c_common_probe(struct dw_i3c_master *master, > err_assert_rst: > reset_control_assert(master->core_rst); > > + clk_disable_unprepare(master->pclk); > + > err_disable_core_clk: > clk_disable_unprepare(master->core_clk); > > @@ -1533,6 +1543,8 @@ void dw_i3c_common_remove(struct dw_i3c_master *master) > > reset_control_assert(master->core_rst); > > + clk_disable_unprepare(master->pclk); > + > clk_disable_unprepare(master->core_clk); > } > EXPORT_SYMBOL_GPL(dw_i3c_common_remove); > diff --git a/drivers/i3c/master/dw-i3c-master.h b/drivers/i3c/master/dw-i3c-master.h > index 8cb617b8147e..50c38e790c0e 100644 > --- a/drivers/i3c/master/dw-i3c-master.h > +++ b/drivers/i3c/master/dw-i3c-master.h > @@ -36,6 +36,7 @@ struct dw_i3c_master { > void __iomem *regs; > struct reset_control *core_rst; > struct clk *core_clk; > + struct clk *pclk; > char version[5]; > char type[5]; > bool ibi_capable; > -- > 2.45.2.505.gda0bf45e8d-goog >
On 18/06/2024 22:06:00+0200, Alexandre Belloni wrote: > Hello, > > On 11/06/2024 17:16:00+0000, Aniket wrote: > > Besides the core clock, IP also has an apb > > interface clock. Add an optional hook for > > the same and appropriately enable and disable. > > > > Signed-off-by: Aniket <aniketmaurya@google.com> > > --- > > drivers/i3c/master/dw-i3c-master.c | 12 ++++++++++++ > > drivers/i3c/master/dw-i3c-master.h | 1 + > > 2 files changed, 13 insertions(+) > > > > diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c > > index 77a2a1c3fd1d..41cdfd6741e3 100644 > > --- a/drivers/i3c/master/dw-i3c-master.c > > +++ b/drivers/i3c/master/dw-i3c-master.c > > @@ -1470,12 +1470,20 @@ int dw_i3c_common_probe(struct dw_i3c_master *master, > > if (IS_ERR(master->core_clk)) > > return PTR_ERR(master->core_clk); > > > > + master->pclk = devm_clk_get_optional(&pdev->dev, "pclk"); > > + if (IS_ERR(master->pclk)) > > + return PTR_ERR(master->pclk); > > + > > I guess you need to update the device tree binding documentation too. Sorry, you did and you had changes requested, forget about that. > > > master->core_rst = devm_reset_control_get_optional_exclusive(&pdev->dev, > > "core_rst"); > > if (IS_ERR(master->core_rst)) > > return PTR_ERR(master->core_rst); > > > > ret = clk_prepare_enable(master->core_clk); It could be worth having a look at devm_clk_get_optional_enabled > > + if (ret) > > + return ret; > > + > > + ret = clk_prepare_enable(master->pclk); > > if (ret) > > goto err_disable_core_clk; > > > > @@ -1520,6 +1528,8 @@ int dw_i3c_common_probe(struct dw_i3c_master *master, > > err_assert_rst: > > reset_control_assert(master->core_rst); > > > > + clk_disable_unprepare(master->pclk); > > + > > err_disable_core_clk: > > clk_disable_unprepare(master->core_clk); > > > > @@ -1533,6 +1543,8 @@ void dw_i3c_common_remove(struct dw_i3c_master *master) > > > > reset_control_assert(master->core_rst); > > > > + clk_disable_unprepare(master->pclk); > > + > > clk_disable_unprepare(master->core_clk); > > } > > EXPORT_SYMBOL_GPL(dw_i3c_common_remove); > > diff --git a/drivers/i3c/master/dw-i3c-master.h b/drivers/i3c/master/dw-i3c-master.h > > index 8cb617b8147e..50c38e790c0e 100644 > > --- a/drivers/i3c/master/dw-i3c-master.h > > +++ b/drivers/i3c/master/dw-i3c-master.h > > @@ -36,6 +36,7 @@ struct dw_i3c_master { > > void __iomem *regs; > > struct reset_control *core_rst; > > struct clk *core_clk; > > + struct clk *pclk; > > char version[5]; > > char type[5]; > > bool ibi_capable; > > -- > > 2.45.2.505.gda0bf45e8d-goog > > > > -- > Alexandre Belloni, co-owner and COO, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com
Hey, > > > ret = clk_prepare_enable(master->core_clk); > > It could be worth having a look at devm_clk_get_optional_enabled Do we want to use *_enabled clock apis for both core_clk and pclk? Thanks, Aniket
On 20/06/2024 11:42:25+0530, Aniket . wrote: > Hey, > > > > ret = clk_prepare_enable(master->core_clk); > > > > It could be worth having a look at devm_clk_get_optional_enabled > Do we want to use *_enabled clock apis for both core_clk and pclk? Yes > > Thanks, > Aniket > > -- > linux-i3c mailing list > linux-i3c@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-i3c
diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c index 77a2a1c3fd1d..41cdfd6741e3 100644 --- a/drivers/i3c/master/dw-i3c-master.c +++ b/drivers/i3c/master/dw-i3c-master.c @@ -1470,12 +1470,20 @@ int dw_i3c_common_probe(struct dw_i3c_master *master, if (IS_ERR(master->core_clk)) return PTR_ERR(master->core_clk); + master->pclk = devm_clk_get_optional(&pdev->dev, "pclk"); + if (IS_ERR(master->pclk)) + return PTR_ERR(master->pclk); + master->core_rst = devm_reset_control_get_optional_exclusive(&pdev->dev, "core_rst"); if (IS_ERR(master->core_rst)) return PTR_ERR(master->core_rst); ret = clk_prepare_enable(master->core_clk); + if (ret) + return ret; + + ret = clk_prepare_enable(master->pclk); if (ret) goto err_disable_core_clk; @@ -1520,6 +1528,8 @@ int dw_i3c_common_probe(struct dw_i3c_master *master, err_assert_rst: reset_control_assert(master->core_rst); + clk_disable_unprepare(master->pclk); + err_disable_core_clk: clk_disable_unprepare(master->core_clk); @@ -1533,6 +1543,8 @@ void dw_i3c_common_remove(struct dw_i3c_master *master) reset_control_assert(master->core_rst); + clk_disable_unprepare(master->pclk); + clk_disable_unprepare(master->core_clk); } EXPORT_SYMBOL_GPL(dw_i3c_common_remove); diff --git a/drivers/i3c/master/dw-i3c-master.h b/drivers/i3c/master/dw-i3c-master.h index 8cb617b8147e..50c38e790c0e 100644 --- a/drivers/i3c/master/dw-i3c-master.h +++ b/drivers/i3c/master/dw-i3c-master.h @@ -36,6 +36,7 @@ struct dw_i3c_master { void __iomem *regs; struct reset_control *core_rst; struct clk *core_clk; + struct clk *pclk; char version[5]; char type[5]; bool ibi_capable;
Besides the core clock, IP also has an apb interface clock. Add an optional hook for the same and appropriately enable and disable. Signed-off-by: Aniket <aniketmaurya@google.com> --- drivers/i3c/master/dw-i3c-master.c | 12 ++++++++++++ drivers/i3c/master/dw-i3c-master.h | 1 + 2 files changed, 13 insertions(+)