Re: [PATCH 5/5] clk: Add clk_get_by_name_optional

Hi Sean,

On Thu, 2 Dec 2021 at 07:25, Sean Anderson seanga2@gmail.com 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 seanga2@gmail.com 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 seanga2@gmail.com

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