1. Patchew
  2. linux
  3. [PATCH v6 0/6] Add interconnect driver for IPQ9574 SoC
  4. View patch

[PATCH v6 3/6] interconnect: icc-clk: Add devm_icc_clk_register

Varadarajan Narayanan posted 6 patches 1 year, 10 months ago
There is a newer version of this series
[PATCH v6 3/6] interconnect: icc-clk: Add devm_icc_clk_register
Posted by Varadarajan Narayanan 1 year, 10 months ago 
Wrap icc_clk_register to create devm_icc_clk_register to be
able to release the resources properly.

Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
---
v5: Introduced devm_icc_clk_register
---
 drivers/interconnect/icc-clk.c   | 29 +++++++++++++++++++++++++++++
 include/linux/interconnect-clk.h |  4 ++++
 2 files changed, 33 insertions(+)

diff --git a/drivers/interconnect/icc-clk.c b/drivers/interconnect/icc-clk.c
index d787f2ea36d9..89f11fed8820 100644
--- a/drivers/interconnect/icc-clk.c
+++ b/drivers/interconnect/icc-clk.c
@@ -148,6 +148,35 @@ struct icc_provider *icc_clk_register(struct device *dev,
 }
 EXPORT_SYMBOL_GPL(icc_clk_register);
 
+static void devm_icc_release(struct device *dev, void *res)
+{
+	icc_clk_unregister(res);
+}
+
+struct icc_provider *devm_icc_clk_register(struct device *dev,
+				      unsigned int first_id,
+				      unsigned int num_clocks,
+				      const struct icc_clk_data *data)
+{
+	struct icc_provider *prov, **provp;
+
+	provp = devres_alloc(devm_icc_release, sizeof(*provp), GFP_KERNEL);
+	if (!provp)
+		return ERR_PTR(-ENOMEM);
+
+	prov = icc_clk_register(dev, first_id, num_clocks, data);
+
+	if (!IS_ERR(prov)) {
+		*provp = prov;
+		devres_add(dev, provp);
+	} else {
+		devres_free(provp);
+	}
+
+	return prov;
+}
+EXPORT_SYMBOL_GPL(devm_icc_clk_register);
+
 /**
  * icc_clk_unregister() - unregister a previously registered clk interconnect provider
  * @provider: provider returned by icc_clk_register()
diff --git a/include/linux/interconnect-clk.h b/include/linux/interconnect-clk.h
index 0cd80112bea5..cb7b648eb1c0 100644
--- a/include/linux/interconnect-clk.h
+++ b/include/linux/interconnect-clk.h
@@ -17,6 +17,10 @@ struct icc_provider *icc_clk_register(struct device *dev,
 				      unsigned int first_id,
 				      unsigned int num_clocks,
 				      const struct icc_clk_data *data);
+struct icc_provider *devm_icc_clk_register(struct device *dev,
+					   unsigned int first_id,
+					   unsigned int num_clocks,
+					   const struct icc_clk_data *data);
 void icc_clk_unregister(struct icc_provider *provider);
 
 #endif
-- 
2.34.1
Re: [PATCH v6 3/6] interconnect: icc-clk: Add devm_icc_clk_register
Posted by Dmitry Baryshkov 1 year, 10 months ago 
On Tue, 2 Apr 2024 at 13:34, Varadarajan Narayanan
<quic_varada@quicinc.com> wrote:
>
> Wrap icc_clk_register to create devm_icc_clk_register to be
> able to release the resources properly.
>
> Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> ---
> v5: Introduced devm_icc_clk_register
> ---
>  drivers/interconnect/icc-clk.c   | 29 +++++++++++++++++++++++++++++
>  include/linux/interconnect-clk.h |  4 ++++
>  2 files changed, 33 insertions(+)

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

-- 
With best wishes
Dmitry
Re: [PATCH v6 3/6] interconnect: icc-clk: Add devm_icc_clk_register
Posted by Dmitry Baryshkov 1 year, 10 months ago 
On Tue, 2 Apr 2024 at 13:40, Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> On Tue, 2 Apr 2024 at 13:34, Varadarajan Narayanan
> <quic_varada@quicinc.com> wrote:
> >
> > Wrap icc_clk_register to create devm_icc_clk_register to be
> > able to release the resources properly.
> >
> > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> > ---
> > v5: Introduced devm_icc_clk_register
> > ---
> >  drivers/interconnect/icc-clk.c   | 29 +++++++++++++++++++++++++++++
> >  include/linux/interconnect-clk.h |  4 ++++
> >  2 files changed, 33 insertions(+)
>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

Wait. Actually,

Unreviewed-by: me

Please return int from devm_icc_clk_register instead of returning the pointer.

-- 
With best wishes
Dmitry
Re: [PATCH v6 3/6] interconnect: icc-clk: Add devm_icc_clk_register
Posted by Varadarajan Narayanan 1 year, 10 months ago 
On Tue, Apr 02, 2024 at 01:48:08PM +0300, Dmitry Baryshkov wrote:
> On Tue, 2 Apr 2024 at 13:40, Dmitry Baryshkov
> <dmitry.baryshkov@linaro.org> wrote:
> >
> > On Tue, 2 Apr 2024 at 13:34, Varadarajan Narayanan
> > <quic_varada@quicinc.com> wrote:
> > >
> > > Wrap icc_clk_register to create devm_icc_clk_register to be
> > > able to release the resources properly.
> > >
> > > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> > > ---
> > > v5: Introduced devm_icc_clk_register
> > > ---
> > >  drivers/interconnect/icc-clk.c   | 29 +++++++++++++++++++++++++++++
> > >  include/linux/interconnect-clk.h |  4 ++++
> > >  2 files changed, 33 insertions(+)
> >
> > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>
> Wait. Actually,
>
> Unreviewed-by: me
>
> Please return int from devm_icc_clk_register instead of returning the pointer.

Wouldn't returning int break the general assumption that
devm_foo(), returns the same type as foo(). For example
devm_clk_hw_get_clk and clk_hw_get_clk return struct clk *?

Thanks
Varada
Re: [PATCH v6 3/6] interconnect: icc-clk: Add devm_icc_clk_register
Posted by Dmitry Baryshkov 1 year, 10 months ago 
On Tue, 2 Apr 2024 at 14:02, Varadarajan Narayanan
<quic_varada@quicinc.com> wrote:
>
> On Tue, Apr 02, 2024 at 01:48:08PM +0300, Dmitry Baryshkov wrote:
> > On Tue, 2 Apr 2024 at 13:40, Dmitry Baryshkov
> > <dmitry.baryshkov@linaro.org> wrote:
> > >
> > > On Tue, 2 Apr 2024 at 13:34, Varadarajan Narayanan
> > > <quic_varada@quicinc.com> wrote:
> > > >
> > > > Wrap icc_clk_register to create devm_icc_clk_register to be
> > > > able to release the resources properly.
> > > >
> > > > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> > > > ---
> > > > v5: Introduced devm_icc_clk_register
> > > > ---
> > > >  drivers/interconnect/icc-clk.c   | 29 +++++++++++++++++++++++++++++
> > > >  include/linux/interconnect-clk.h |  4 ++++
> > > >  2 files changed, 33 insertions(+)
> > >
> > > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> >
> > Wait. Actually,
> >
> > Unreviewed-by: me
> >
> > Please return int from devm_icc_clk_register instead of returning the pointer.
>
> Wouldn't returning int break the general assumption that
> devm_foo(), returns the same type as foo(). For example
> devm_clk_hw_get_clk and clk_hw_get_clk return struct clk *?

Not always. The only reason to return icc_provider was to make it
possible to destroy it. With devres-managed function you don't have to
do anything.


-- 
With best wishes
Dmitry
Re: [PATCH v6 3/6] interconnect: icc-clk: Add devm_icc_clk_register
Posted by Varadarajan Narayanan 1 year, 10 months ago 
On Tue, Apr 02, 2024 at 02:16:56PM +0300, Dmitry Baryshkov wrote:
> On Tue, 2 Apr 2024 at 14:02, Varadarajan Narayanan
> <quic_varada@quicinc.com> wrote:
> >
> > On Tue, Apr 02, 2024 at 01:48:08PM +0300, Dmitry Baryshkov wrote:
> > > On Tue, 2 Apr 2024 at 13:40, Dmitry Baryshkov
> > > <dmitry.baryshkov@linaro.org> wrote:
> > > >
> > > > On Tue, 2 Apr 2024 at 13:34, Varadarajan Narayanan
> > > > <quic_varada@quicinc.com> wrote:
> > > > >
> > > > > Wrap icc_clk_register to create devm_icc_clk_register to be
> > > > > able to release the resources properly.
> > > > >
> > > > > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> > > > > ---
> > > > > v5: Introduced devm_icc_clk_register
> > > > > ---
> > > > >  drivers/interconnect/icc-clk.c   | 29 +++++++++++++++++++++++++++++
> > > > >  include/linux/interconnect-clk.h |  4 ++++
> > > > >  2 files changed, 33 insertions(+)
> > > >
> > > > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > >
> > > Wait. Actually,
> > >
> > > Unreviewed-by: me
> > >
> > > Please return int from devm_icc_clk_register instead of returning the pointer.
> >
> > Wouldn't returning int break the general assumption that
> > devm_foo(), returns the same type as foo(). For example
> > devm_clk_hw_get_clk and clk_hw_get_clk return struct clk *?
>
> Not always. The only reason to return icc_provider was to make it
> possible to destroy it. With devres-managed function you don't have to
> do anything.

Ok. Will change as follows

	return prov; -> return PTR_ERR_OR_ZERO(prov);

Thanks
Varada
Re: [PATCH v6 3/6] interconnect: icc-clk: Add devm_icc_clk_register
Posted by Dmitry Baryshkov 1 year, 10 months ago 
On Tue, 2 Apr 2024 at 14:23, Varadarajan Narayanan
<quic_varada@quicinc.com> wrote:
>
> On Tue, Apr 02, 2024 at 02:16:56PM +0300, Dmitry Baryshkov wrote:
> > On Tue, 2 Apr 2024 at 14:02, Varadarajan Narayanan
> > <quic_varada@quicinc.com> wrote:
> > >
> > > On Tue, Apr 02, 2024 at 01:48:08PM +0300, Dmitry Baryshkov wrote:
> > > > On Tue, 2 Apr 2024 at 13:40, Dmitry Baryshkov
> > > > <dmitry.baryshkov@linaro.org> wrote:
> > > > >
> > > > > On Tue, 2 Apr 2024 at 13:34, Varadarajan Narayanan
> > > > > <quic_varada@quicinc.com> wrote:
> > > > > >
> > > > > > Wrap icc_clk_register to create devm_icc_clk_register to be
> > > > > > able to release the resources properly.
> > > > > >
> > > > > > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> > > > > > ---
> > > > > > v5: Introduced devm_icc_clk_register
> > > > > > ---
> > > > > >  drivers/interconnect/icc-clk.c   | 29 +++++++++++++++++++++++++++++
> > > > > >  include/linux/interconnect-clk.h |  4 ++++
> > > > > >  2 files changed, 33 insertions(+)
> > > > >
> > > > > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > > >
> > > > Wait. Actually,
> > > >
> > > > Unreviewed-by: me
> > > >
> > > > Please return int from devm_icc_clk_register instead of returning the pointer.
> > >
> > > Wouldn't returning int break the general assumption that
> > > devm_foo(), returns the same type as foo(). For example
> > > devm_clk_hw_get_clk and clk_hw_get_clk return struct clk *?
> >
> > Not always. The only reason to return icc_provider was to make it
> > possible to destroy it. With devres-managed function you don't have to
> > do anything.
>
> Ok. Will change as follows
>
>         return prov; -> return PTR_ERR_OR_ZERO(prov);
>

I think the code might become simpler if you first allocate the ICC
provider and then just 'return devm_add_action_or_reset(dev,
your_icc_clk_release, provider)'


-- 
With best wishes
Dmitry
Re: [PATCH v6 3/6] interconnect: icc-clk: Add devm_icc_clk_register
Posted by Varadarajan Narayanan 1 year, 10 months ago 
On Tue, Apr 02, 2024 at 03:12:04PM +0300, Dmitry Baryshkov wrote:
> On Tue, 2 Apr 2024 at 14:23, Varadarajan Narayanan
> <quic_varada@quicinc.com> wrote:
> >
> > On Tue, Apr 02, 2024 at 02:16:56PM +0300, Dmitry Baryshkov wrote:
> > > On Tue, 2 Apr 2024 at 14:02, Varadarajan Narayanan
> > > <quic_varada@quicinc.com> wrote:
> > > >
> > > > On Tue, Apr 02, 2024 at 01:48:08PM +0300, Dmitry Baryshkov wrote:
> > > > > On Tue, 2 Apr 2024 at 13:40, Dmitry Baryshkov
> > > > > <dmitry.baryshkov@linaro.org> wrote:
> > > > > >
> > > > > > On Tue, 2 Apr 2024 at 13:34, Varadarajan Narayanan
> > > > > > <quic_varada@quicinc.com> wrote:
> > > > > > >
> > > > > > > Wrap icc_clk_register to create devm_icc_clk_register to be
> > > > > > > able to release the resources properly.
> > > > > > >
> > > > > > > Signed-off-by: Varadarajan Narayanan <quic_varada@quicinc.com>
> > > > > > > ---
> > > > > > > v5: Introduced devm_icc_clk_register
> > > > > > > ---
> > > > > > >  drivers/interconnect/icc-clk.c   | 29 +++++++++++++++++++++++++++++
> > > > > > >  include/linux/interconnect-clk.h |  4 ++++
> > > > > > >  2 files changed, 33 insertions(+)
> > > > > >
> > > > > > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > > > >
> > > > > Wait. Actually,
> > > > >
> > > > > Unreviewed-by: me
> > > > >
> > > > > Please return int from devm_icc_clk_register instead of returning the pointer.
> > > >
> > > > Wouldn't returning int break the general assumption that
> > > > devm_foo(), returns the same type as foo(). For example
> > > > devm_clk_hw_get_clk and clk_hw_get_clk return struct clk *?
> > >
> > > Not always. The only reason to return icc_provider was to make it
> > > possible to destroy it. With devres-managed function you don't have to
> > > do anything.
> >
> > Ok. Will change as follows
> >
> >         return prov; -> return PTR_ERR_OR_ZERO(prov);
> >
>
> I think the code might become simpler if you first allocate the ICC
> provider and then just 'return devm_add_action_or_reset(dev,
> your_icc_clk_release, provider)'

Have posted v7 incorporating these and other feedback.
Please review.

Thanks
Varada

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.