linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: ambresh@ti.com (Ambresh K)
To: linux-arm-kernel@lists.infradead.org
Subject: [Patch 3/3] clk: Avoid re-parenting orphan clk's having invalid parent index.
Date: Thu, 13 Jun 2013 12:36:43 +0530	[thread overview]
Message-ID: <51B96F83.10309@ti.com> (raw)
In-Reply-To: <20130611220744.8816.8856@quantum>




>> Sorry for not being descriptive in commit message.  
>>
>> a) Avoids unnecessary re-parenting cycle for orphan clock's with invalid parent for every clock
>>
> 
> True, but this is a minor optimisation.  If this is a big optimization
> for you then you really need to fix your bootloader.  We shouldn't be
> optimizing slow error paths just because we refuse to fix the errors.

Got it! Should be fixed in boot-loader. 

> 
>> b) With patch [1/3], after a clk with invalid parent was encountered, for every clk registered thereafter seeing following logs. 
>> <Snippet>
>> [    0.000000] __clk_init: orphan clk(gpu_core_gclk_mux) invalid parent
>> [    0.000000] __clk_init: orphan clk(gpu_core_gclk_mux) invalid parent
>> [    0.000000] __clk_init: orphan clk(gpu_core_gclk_mux) invalid parent
>> [    0.000000] __clk_init: orphan clk(gpu_core_gclk_mux) invalid parent
>> [    0.000000] __clk_init: orphan clk(gpu_core_gclk_mux) invalid parent
>> [    0.000000] __clk_init: orphan clk(gpu_core_gclk_mux) invalid parent
>> [    0.000000] __clk_init: orphan clk(gpu_core_gclk_mux) invalid parent
>> [    0.000000] __clk_init: orphan clk(gpu_core_gclk_mux) invalid parent  
>>
>> Please advice, if can be handled better. 
>>
> 
> First off, I don't think we should create new structures to work around
> bugs that should be fixed.  pr_err_once() will let us know something is
> wrong and won't flood the log.  Even then I'm inclined to say that
> flooding the log is OK and will motivate you to fix up your bootloader.
> Error prints are there for a reason.
> 
> Secondly, I spent like 10 minutes looking at this code and I'm still
> confused.  For a clock with invalid parent programming, are you adding
> it to BOTH the orphan list and the has_invalid_parent list?  Why?  Is
> this just avoid the spurious prints?  For everyone clock registered we
> walk the list of orphans to see if that orphan can be reparented.  This
> patch adds another nested list walk (likely a short list) for each of
> those orphans in the first list walk, so it starts to look like O(n^2).
> I don't like it.
> 
> I think the first two patches in the series look good, but unless I am
> misunderstanding this patch I feel that it can be dropped entirely.
> 

Thanks for your time! 
Will drop this patch and send V2 for first 2 patches. 


Regards,
Ambresh

      reply	other threads:[~2013-06-13  7:06 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-05-02  6:25 [Patch 0/3] Fix to clk framework while handling orphan clks Ambresh K
2013-05-02  6:25 ` [Patch 1/3] clk: fix clk_mux_get_parent return's signed value Ambresh K
2013-05-29  7:21   ` Mike Turquette
2013-06-04  6:27     ` Ambresh K
2013-06-04  6:35       ` Mike Turquette
2013-05-02  6:25 ` [Patch 2/3] clk: skip re-parenting orphan clk Ambresh K
2013-05-02 10:09   ` skannan at codeaurora.org
2013-05-02  6:25 ` [Patch 3/3] clk: Avoid re-parenting orphan clk's having invalid parent index Ambresh K
2013-05-29  7:18   ` Mike Turquette
2013-06-04  7:16     ` Ambresh K
2013-06-11 22:07       ` Mike Turquette
2013-06-13  7:06         ` Ambresh K [this message]

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=51B96F83.10309@ti.com \
    --to=ambresh@ti.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /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;
as well as URLs for NNTP newsgroup(s).