[U-Boot] [PATCH v6 2/2] DW SPI: Get clock value from Device Tree

[Simon Glass]

On 11 December 2017 at 09:41, Marek Vasut <marex at denx.de> wrote:
> On 12/11/2017 05:37 PM, Eugeniy Paltsev wrote:
>> On Mon, 2017-12-11 at 17:21 +0100, Marek Vasut wrote:
>>> On 12/11/2017 05:18 PM, Eugeniy Paltsev wrote:
>>>> Add option to set spi controller clock frequency via device tree
>>>> using standard clock bindings.
>>>>
>>>> Define dw_spi_get_clk function as 'weak' as some targets
>>>> (like SOCFPGA_GEN5 and SOCFPGA_ARRIA10) fon't use standard clock API
>>>> and implement dw_spi_get_clk their own way in their clock manager.
>>>>
>>>> Get rid of clock_manager.h include as we don't use
>>>> cm_get_spi_controller_clk_hz function anymore. (we use redefined
>>>> dw_spi_get_clk in SOCFPGA clock managers instead)
>>>>
>>>> Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev at synopsys.com>
>>>> ---
>>>>  drivers/spi/designware_spi.c | 43 +++++++++++++++++++++++++++++++++++++++++--
>>>>  1 file changed, 41 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/spi/designware_spi.c b/drivers/spi/designware_spi.c
>>>> index 5aa507b..a037376 100644
>>>> --- a/drivers/spi/designware_spi.c
>>>> +++ b/drivers/spi/designware_spi.c
>>>> @@ -11,6 +11,7 @@
>>>>   */
>>>>
>>>>  #include <common.h>
>>>> +#include <clk.h>
>>>>  #include <dm.h>
>>>>  #include <errno.h>
>>>>  #include <malloc.h>
>>>> @@ -18,7 +19,6 @@
>>>>  #include <fdtdec.h>
>>>>  #include <linux/compat.h>
>>>>  #include <asm/io.h>
>>>> -#include <asm/arch/clock_manager.h>
>>>>
>>>>  DECLARE_GLOBAL_DATA_PTR;
>>>>
>>>> @@ -94,6 +94,8 @@ struct dw_spi_priv {
>>>>     void __iomem *regs;
>>>>     unsigned int freq;              /* Default frequency */
>>>>     unsigned int mode;
>>>> +   struct clk clk;
>>>> +   unsigned long bus_clk_rate;
>>>>
>>>>     int bits_per_word;
>>>>     u8 cs;                  /* chip select pin */
>>>> @@ -176,14 +178,51 @@ static void spi_hw_init(struct dw_spi_priv *priv)
>>>>     debug("%s: fifo_len=%d\n", __func__, priv->fifo_len);
>>>>  }
>>>>
>>>> +/*
>>>> + * We define dw_spi_get_clk function as 'weak' as some targets
>>>> + * (like SOCFPGA_GEN5 and SOCFPGA_ARRIA10) fon't use standard clock API
>>>> + * and implement dw_spi_get_clk their own way in their clock manager.
>>>> + */
>>>> +__weak int dw_spi_get_clk(struct udevice *bus, ulong *rate)
>>>> +{
>>>> +   struct dw_spi_priv *priv = dev_get_priv(bus);
>>>> +   int ret;
>>>> +
>>>> +   ret = clk_get_by_index(bus, 0, &priv->clk);
>>>> +   if (ret)
>>>> +           return -EINVAL;
>>>
>>> if (ret)
>>>  return ret;
>> Ok.
>>
>>>
>>>> +   ret = clk_enable(&priv->clk);
>>>> +   if (ret && ret != -ENOSYS && ret != -ENOSYS && ret != -ENOTSUPP)
>>>
>>> You have ENOSYS twice in there, but you can do just if (ret) return ret
>>> again.
>>
>> The idea is to skip error if error code is -ENOSYS or -ENOTSUPP
>> as it is OK situation: some clock drivers don't implement .enable/.disable
>> callbacks so clk_enable returns -ENOSYS.
>> Also some clock drivers implement .enable/.disable callbacks not for all
>> clock IDs and return -ENOSYS or -ENOTSUPP for others.
>>
>> But definitely I should remove second ENOSYS here.
> +CC Simon, that's somewhat iffy ...

I think it is OK.

Regards,
Simon