[U-Boot,v3,1/5] dm: clk: add stub for clk_disable_bulk when CONFIG_CLK is desactivated
diff mbox series

Message ID 20191112094214.12686-2-patrick.delaunay@st.com
State New
Delegated to: Marek Vasut
Headers show
Series
  • usb: host: dwc2: use driver model for PHY and CLOCK
Related show

Commit Message

Patrick Delaunay Nov. 12, 2019, 9:42 a.m. UTC
Add stub for clk_disable_bulk() when CONFIG_CLK is desactivated.

That avoid compilation issue (undefined reference to
`clk_disable_bulk') for code:

clk_disable_bulk(&priv->clks);
clk_release_bulk(&priv->clks);

Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
---

Changes in v3:
- Add stub for clk_disable_bulk

Changes in v2: None

 include/clk.h | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Jean-Jacques Hiblot Nov. 12, 2019, 10:16 a.m. UTC | #1
Hi Patrick,

On 12/11/2019 10:42, Patrick Delaunay wrote:
> Add stub for clk_disable_bulk() when CONFIG_CLK is desactivated.
>
> That avoid compilation issue (undefined reference to
> `clk_disable_bulk') for code:
>
> clk_disable_bulk(&priv->clks);
> clk_release_bulk(&priv->clks);
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
>
> Changes in v3:
> - Add stub for clk_disable_bulk
>
> Changes in v2: None
>
>   include/clk.h | 4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/include/clk.h b/include/clk.h
> index a5ee53d94a..6f0b0fe4bc 100644
> --- a/include/clk.h
> +++ b/include/clk.h
> @@ -379,7 +379,11 @@ int clk_disable(struct clk *clk);
>    *		by clk_get_bulk().
>    * @return zero on success, or -ve error code.
>    */
> + #if CONFIG_IS_ENABLED(CLK)
>   int clk_disable_bulk(struct clk_bulk *bulk);
> +#else
> +inline int clk_disable_bulk(struct clk_bulk *bulk) { return 0; }
> +#endif

Maybe this could be done for all clk operations ?

JJ

>   
>   /**
>    * clk_is_match - check if two clk's point to the same hardware clock
Simon Goldschmidt Nov. 12, 2019, 10:40 a.m. UTC | #2
Patrick Delaunay <patrick.delaunay@st.com> schrieb am Di., 12. Nov. 2019,
10:42:

> Add stub for clk_disable_bulk() when CONFIG_CLK is desactivated.
>
> That avoid compilation issue (undefined reference to
> `clk_disable_bulk') for code:
>
> clk_disable_bulk(&priv->clks);
> clk_release_bulk(&priv->clks);
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
>
> Changes in v3:
> - Add stub for clk_disable_bulk
>
> Changes in v2: None
>
>  include/clk.h | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/include/clk.h b/include/clk.h
> index a5ee53d94a..6f0b0fe4bc 100644
> --- a/include/clk.h
> +++ b/include/clk.h
> @@ -379,7 +379,11 @@ int clk_disable(struct clk *clk);
>   *             by clk_get_bulk().
>   * @return zero on success, or -ve error code.
>   */
> + #if CONFIG_IS_ENABLED(CLK)
>  int clk_disable_bulk(struct clk_bulk *bulk);
> +#else
> +inline int clk_disable_bulk(struct clk_bulk *bulk) { return 0; }
> +#endif
>

Doing this inline at this place seems quite different than what is done for
the other functions?

Regards,
Simon


>  /**
>   * clk_is_match - check if two clk's point to the same hardware clock
> --
> 2.17.1
>
>
Patrick Delaunay Nov. 12, 2019, 3:47 p.m. UTC | #3
Hi Jean-Jacques,


> From: Jean-Jacques Hiblot <jjhiblot@ti.com>
> Sent: mardi 12 novembre 2019 11:17
> 
> Hi Patrick,
> 
> On 12/11/2019 10:42, Patrick Delaunay wrote:
> > Add stub for clk_disable_bulk() when CONFIG_CLK is desactivated.
> >
> > That avoid compilation issue (undefined reference to
> > `clk_disable_bulk') for code:
> >
> > clk_disable_bulk(&priv->clks);
> > clk_release_bulk(&priv->clks);
> >
> > Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> > ---
> >
> > Changes in v3:
> > - Add stub for clk_disable_bulk
> >
> > Changes in v2: None
> >
> >   include/clk.h | 4 ++++
> >   1 file changed, 4 insertions(+)
> >
> > diff --git a/include/clk.h b/include/clk.h index
> > a5ee53d94a..6f0b0fe4bc 100644
> > --- a/include/clk.h
> > +++ b/include/clk.h
> > @@ -379,7 +379,11 @@ int clk_disable(struct clk *clk);
> >    *		by clk_get_bulk().
> >    * @return zero on success, or -ve error code.
> >    */
> > + #if CONFIG_IS_ENABLED(CLK)
> >   int clk_disable_bulk(struct clk_bulk *bulk);
> > +#else
> > +inline int clk_disable_bulk(struct clk_bulk *bulk) { return 0; }
> > +#endif
> 
> Maybe this could be done for all clk operations ?

I think about, but after reflection

1/ stub already exist for :
clk_get_by_index
clk_get_bulk
clk_get_by_name
clk_release_all

=> just inline , return -ENOSYS

2/ clk_release_bulk inline call for clk_release_all

3/ other function (clk_request, clk_free, clk_get_rate, clk_enable, clk_disable) 
    should be not used as "clk" parameter is never valid / available if CONFIG_CLK is not activited

4/ the only remaining case is 

	int clk_disable_bulk(struct clk_bulk *bulk);

	=> clk_get_bulk return -ENOSYS but normally this information is not keept by caller....

	On error bulk.count = 0, and for me clk_disable_bulk(bulk wthou count = 0) is valid even if CONFIG_CLK is disable....

So I decide to limit the patch to this function to minimize the impacts
also because the 2020.01 windows is closed.

Moreover  I have not board to test CONFIG_CLK disabled.

But I agree : it is more clear a have a stub for other function which can be used 
	including clk_valid

=> I can propose a 2nd separate patch with this proposal if it is required.

> JJ

Regards 
Patrick
Patrick Delaunay Nov. 20, 2019, 10:04 a.m. UTC | #4
Hi Simon

>De : Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
>Envoyé : mardi 12 novembre 2019 11:40
>
>
>Patrick Delaunay <patrick.delaunay@st.com> schrieb am Di., 12. Nov. 2019, 10:42:
>Add stub for clk_disable_bulk() when CONFIG_CLK is desactivated.
>
>That avoid compilation issue (undefined reference to
>`clk_disable_bulk') for code:
>
>clk_disable_bulk(&priv->clks);
>clk_release_bulk(&priv->clks);
>
>Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
>---
>
>Changes in v3:
>- Add stub for clk_disable_bulk
>
>Changes in v2: None
>
> include/clk.h | 4 ++++
> 1 file changed, 4 insertions(+)
>
>diff --git a/include/clk.h b/include/clk.h
>index a5ee53d94a..6f0b0fe4bc 100644
>--- a/include/clk.h
>+++ b/include/clk.h
>@@ -379,7 +379,11 @@ int clk_disable(struct clk *clk);
>  *             by clk_get_bulk().
>  * @return zero on success, or -ve error code.
>  */
>+ #if CONFIG_IS_ENABLED(CLK)
> int clk_disable_bulk(struct clk_bulk *bulk);
>+#else
>+inline int clk_disable_bulk(struct clk_bulk *bulk) { return 0; }
>+#endif
>
>Doing this inline at this place seems quite different than what is done for the other functions?

which functions ?

I see:

static inline int clk_get_by_index(struct udevice *dev, int index,
   struct clk *clk)
{
return -ENOSYS;
}

static inline int clk_set_defaults(struct udevice *dev)
{
return 0;
}

But I agree, that it is a simplified stub....

Disable clk bulk if OK only if bulk.count ==0 but if should be always
the case if CONFIG_CLK is disabled (as clk_get_bulk return -ENOSYS).

A more complete (better ?) stub is :

inline int clk_disable_bulk(struct clk_bulk *bulk) {
if (bulk && bulk->count == 0) return 0;
else return -ENOSYS;
}

I think about a other solution:

inline int clk_disable_bulk(struct clk_bulk *bulk) {
return -ENOSYS;
}

But that cause issue if the return value is tested by caller.

ret = clk_disable_bulk(bulk)
if (ret)
            return ret;

>Regards,
>Simon

Regards
Patrick

>
> /**
>  * clk_is_match - check if two clk's point to the same hardware clock
>--
>2.17.1
>

Patch
diff mbox series

diff --git a/include/clk.h b/include/clk.h
index a5ee53d94a..6f0b0fe4bc 100644
--- a/include/clk.h
+++ b/include/clk.h
@@ -379,7 +379,11 @@  int clk_disable(struct clk *clk);
  *		by clk_get_bulk().
  * @return zero on success, or -ve error code.
  */
+ #if CONFIG_IS_ENABLED(CLK)
 int clk_disable_bulk(struct clk_bulk *bulk);
+#else
+inline int clk_disable_bulk(struct clk_bulk *bulk) { return 0; }
+#endif
 
 /**
  * clk_is_match - check if two clk's point to the same hardware clock