[U-Boot] [PATCH] dm: clk: Remove simple version of clk_get_by_index()

Michal Simek michal.simek at xilinx.com
Fri Jul 15 08:01:14 CEST 2016


On 14.7.2016 20:17, Stephen Warren wrote:
> On 07/14/2016 05:24 AM, Michal Simek wrote:
>> Simple version of clk_get_by_index() added by:
>> "dm: clk: Add a simple version of clk_get_by_index()"
>> (sha1: a4b10c088c4f6ef2e2bba33e8cfea369bcbbce44)
>> is not sufficient if you use multiple clocks in the system
>> because clk->id is phandle id which for example fixed-clock
>> is not able to handle. Use the same implementation as is used
>> in full version.
> 
> It took me a while to work out what failure case you were describing. It
> might be worth more explicitly pointing out that the existing simple
> implementation fails in any case where #clock-cells=<0>, or for larger
> #clock-cells, where the clock ID isn't in the first cell.

TBH I didn't try to understand if this is related to clock-cells size
because I thought that fixed-clock are tested in mainline and my testing
platform is ZynqMP where we don't have own clock driver.
That's why this is not described in commit message.


> To be honest, I'd be inclined to always include the real
> clk_get_by_name() in SPL builds too. If it's never called, the function
> will be dropped by the linker. If it is called, the dummy implementation
> probably actively causes failures that we should avoid by using the real
> implementation. I'm not sure why the original SPL-specific code existed,
> unless the 773 byte code increase you mention is actually problematic
> for some specific build?

Agree.

The next part I found is that when I was playing with mmc and serial
driver that I do call the same clk sequence in both drivers and will be
good to introduce own function to ensure that the same code is not
copied in several places which increase just size.

For my testing the sequence is like this (clock should be probably
return value).

+       int ret;
+       struct clk clk;
+	unsigned long clock;
+
+       ret = clk_get_by_index(dev, 0, &clk);
+       if (ret < 0) {
+               dev_err(dev, "failed to get clock\n");
+               return ret;
+       }
+
+       clock = clk_get_rate(&clk);
+       if (IS_ERR_VALUE(clock)) {
+               dev_err(dev, "failed to get rate\n");
+               return clock;
+       }
+       debug("%s: CLK %ld\n", __func__, clock);
+
+       ret = clk_enable(&clk);
+       if (ret && ret != -ENOSYS) {
+               dev_err(dev, "failed to enable clock\n");
+               return ret;
+       }

Thanks,
Michal



More information about the U-Boot mailing list