Hi Sean, On Thu, 2 Dec 2021 at 07:25, Sean Anderson <[email protected]> wrote: > > Hi Simon, > > On 12/2/21 8:43 AM, Simon Glass wrote: > > Hi Sean, > > > > On Wed, 1 Dec 2021 at 11:43, Sean Anderson <[email protected]> wrote: > >> > >> This adds a helper function for clk_get_by_name in cases where the clock is > >> optional. Hopefully this helps point driver writers in the right direction. > >> Also convert some existing users. > >> > >> Signed-off-by: Sean Anderson <[email protected]> > >> --- > >> > >> drivers/clk/clk_zynq.c | 5 +++-- > >> drivers/rng/meson-rng.c | 4 ++-- > >> include/clk.h | 24 ++++++++++++++++++++++++ > >> 3 files changed, 29 insertions(+), 4 deletions(-) > >> > >> diff --git a/drivers/clk/clk_zynq.c b/drivers/clk/clk_zynq.c > >> index 18915c3e04..e80500e382 100644 > >> --- a/drivers/clk/clk_zynq.c > >> +++ b/drivers/clk/clk_zynq.c > >> @@ -472,8 +472,9 @@ static int zynq_clk_probe(struct udevice *dev) > >> > >> for (i = 0; i < 2; i++) { > >> sprintf(name, "gem%d_emio_clk", i); > >> - ret = clk_get_by_name(dev, name, &priv->gem_emio_clk[i]); > >> - if (ret < 0 && ret != -ENODATA) { > >> + ret = clk_get_by_name_optional(dev, name, > >> + &priv->gem_emio_clk[i]); > >> + if (ret) { > >> dev_err(dev, "failed to get %s clock\n", name); > >> return ret; > >> } > >> diff --git a/drivers/rng/meson-rng.c b/drivers/rng/meson-rng.c > >> index 5a4f45ad5a..e0a1e8c7e0 100644 > >> --- a/drivers/rng/meson-rng.c > >> +++ b/drivers/rng/meson-rng.c > >> @@ -91,8 +91,8 @@ static int meson_rng_of_to_plat(struct udevice *dev) > >> return -ENODEV; > >> > >> /* Get optional "core" clock */ > >> - err = clk_get_by_name(dev, "core", &pdata->clk); > >> - if (err && err != -ENODATA) > >> + err = clk_get_by_name_optional(dev, "core", &pdata->clk); > >> + if (err) > >> return err; > >> > >> return 0; > >> diff --git a/include/clk.h b/include/clk.h > >> index 103ef16bf9..36721188d0 100644 > >> --- a/include/clk.h > >> +++ b/include/clk.h > >> @@ -292,6 +292,30 @@ static inline int clk_release_all(struct clk *clk, > >> int count) > >> } > >> #endif > >> > >> +/** > >> + * clk_get_by_name_optional() - Get/request a optional clock by name. > > > > Can I suggest we rename this to ..._opt as it is shorter? > > This follows the general trend of suffixing _optional. For example (and > several of these are borrowed from Linux): > > clk_get_optional > reset_control_bulk_get_optional_exclusive > gpiod_get_optional > platform_get_irq_byname_optional > > and particularly, the Linux clock subsystem uses _optional and not _opt > as a suffix. While some of these names can get fairly long-winded, I > think we should go for consistency here.
Yes I agree. I do wish people would consider brevity as these names are pretty hopeless for readability. Regards, Simon
