| Message ID | 20191112094214.12686-2-patrick.delaunay@st.com |
|---|---|
| State | Superseded |
| Delegated to: | Marek Vasut |
| Headers | show |
| Series | usb: host: dwc2: use driver model for PHY and CLOCK | expand |
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
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 > >
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
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 >
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
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(+)