[PATCH 5/5] clk: Add clk_get_by_name_optional

[Sean Anderson]

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 at 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 at 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.

--Sean

>> + * @dev:       The client device.
>> + * @name:      The name of the clock to request, within the client's list of
>> + *             clocks.
>> + * @clk:       A pointer to a clock struct to initialize.
>> + *
>> + * Behaves the same as clk_get_by_name(), except when there is no clock
>> + * provider. In the latter case, return 0.
>> + *
>> + * Return: 0 if OK, or a negative error code.
>> + */
>> +static inline int clk_get_by_name_optional(struct udevice *dev,
>> +                                          const char *name, struct clk *clk)
>> +{
>> +       int ret;
>> +
>> +       ret = clk_get_by_name_optional(dev, name, clk);
>> +       if (ret == -ENODATA)
>> +               return 0;
>> +
>> +       return ret;
>> +}
>> +
>>   /**
>>    * clk_get_by_name_nodev_optional - Get/request an optinonal clock by name
>>    *             without a device.
>> --
>> 2.33.0
>>
> 
> Regards,
> SImon
>