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
prev parent 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).