Hi Simon, > From: Simon Goldschmidt <[email protected]> > Sent: vendredi 6 mars 2020 12:28 > > On Fri, Mar 6, 2020 at 11:01 AM Patrick Delaunay <[email protected]> > wrote: > > > > Add stub for functions clk_...() when CONFIG_CLK is deactivated. > > > > This patch avoids compilation issues for driver using these API > > without protection (#if CONFIG_IS_ENABLED(CLK)) > > > > For example, before this patch we have undefined reference to > > `clk_disable_bulk') for code: > > clk_disable_bulk(&priv->clks); > > clk_release_bulk(&priv->clks); > > > > The bulk stub set and test bulk->count to avoid error for the sequence: > > > > clk_get_bulk(dev, &priv->bulk); > > .... > > if (clk_enable(&priv>bulk)) > > return -EIO; > > > > Signed-off-by: Patrick Delaunay <[email protected]> > > --- > > > > Changes in v5: > > - use ERR_PTR in clk_get_parent() > > - force bulk->count = 0 in clk_get_bulk to avoid issue > > for next call of clk_enable_bulk / clk_enable_bulk > > I wonder if this is correct. I think it only produces additional code. If you > always set > bulk->count to 0, the enable code will never fail. > > However, the enable function should never be executed as enable returns an > error. > I can imagine the *disable* function could be called, but the return value of > this > function is currently only handled in clk_sandbox_test.c... > > Wouldn't it work to just remove all checks for bulk->count and let *all* > redefined > functions return a constant (success or failure)? That would help to keep the > code > small.
I wasn't sure if each stub should raise error or success. When I code the stub the first time I found one example for API usage without protection which can cause issue in drivers/net/ftgmac100.c ftgmac100_ofdata_to_platdata => clk_get_bulk(dev, &priv->clks); ftgmac100_probe(struct udevice *dev) => clk_enable_bulk(&priv->clks); An other example : drivers/power/domain/meson-ee-pwrc.c meson_ee_pwrc_probe => clk_get_bulk(dev, &priv->clks); meson_ee_pwrc_on => clk_enable_bulk(&priv->clks); And same in drivers/power/domain/meson-gx-pwrc-vpu.c But finally after deeper check today, these driver can't handle the proposed clk stubs (error during probe for clk_get_bulk). So I agree that I complexify the clk stub only for over protection. > Looking at linux, clock disable functions have *no* return value that needs > to be > checked, clock enable functions return success when compiled without clock > support. I also check the stub for reset function in "./include/reset.h" - reset_free - reset_assert - reset_deassert - reset_deassert_bulk - reset_assert_bulk All these reset stub functions return 0 So to be coherent I will update the patch to return success for clk_free, clk_enable, clk_disable, clk_disable_bulk and clk_enable_bulk. And other functions return error -ENOSYS: request / get_rate / set_rate > Regards, > Simon > Regards Patrick
