public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Michal Simek <michal.simek@xilinx.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] dm: clk: Remove simple version of clk_get_by_index()
Date: Fri, 15 Jul 2016 08:01:14 +0200	[thread overview]
Message-ID: <57887C2A.2000106@xilinx.com> (raw)
In-Reply-To: <5787D728.4080702@wwwdotorg.org>

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

  reply	other threads:[~2016-07-15  6:01 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-14 11:24 [U-Boot] [PATCH] dm: clk: Remove simple version of clk_get_by_index() Michal Simek
2016-07-14 18:17 ` Stephen Warren
2016-07-15  6:01   ` Michal Simek [this message]
2016-07-17 14:14     ` Simon Glass

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=57887C2A.2000106@xilinx.com \
    --to=michal.simek@xilinx.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox