[U-Boot] [uboot-snps-arc] Re: [PATCH v5 2/2] DW SPI: Get clock value from Device Tree

[Marek Vasut]

On 12/09/2017 04:23 PM, Eugeniy Paltsev wrote:
> On Wed, 2017-11-15 at 10:24 +0100, Marek Vasut wrote:
>> On 11/14/2017 04:33 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) don'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 | 42 ++++++++++++++++++++++++++++++++++++++++--
>>>  1 file changed, 40 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/spi/designware_spi.c b/drivers/spi/designware_spi.c
>>> index 6cc4f51..470a3a7 100644
>>> --- a/drivers/spi/designware_spi.c
>>> +++ b/drivers/spi/designware_spi.c
>>> @@ -12,6 +12,7 @@
>>>  
>>> +/*
>>> + * 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 clk clk;
>>> +	int ret;
>>> +
>>> +	ret = clk_get_by_index(bus, 0, &clk);
>>> +	if (ret)
>>> +		return -EINVAL;
>>> +
>>> +	ret = clk_enable(&clk);
>>> +	if (ret && ret != -ENOSYS)
>>> +		return ret;
>>> +
>>> +	*rate = clk_get_rate(&clk);
>>> +	if (!*rate) {
>>> +		clk_disable(&clk);
>>> +		return -EINVAL;
>>> +	}
>>> +
>>> +	debug("%s: get spi controller clk via device tree: %lu Hz\n",
>>> +	      __func__, *rate);
>>> +
>>> +	clk_free(&clk);
>>
>> You probably want to retain the handle to these clock in the private
>> data, since otherwise you won't be able to turn the clock off in
>> .remove() callback of the driver (if/once it's implemented)
> 
> No, .remove() callback isn't implemented in this driver, so it isn't necessary.

Either implement it, or if you're lazy, put the clock handle into the
private data, since that's a good practice.

>> Otherwise the patch look good, thanks !
>>
>>


-- 
Best regards,
Marek Vasut