All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tero Kristo <t-kristo@ti.com>
To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Ladislav Michl <ladis@linux-mips.org>
Cc: <linux-omap@vger.kernel.org>, <linux-clk@vger.kernel.org>,
	Paul Walmsley <paul@pwsan.com>, Richard Watts <rrw@kynesim.co.uk>,
	Tony Lindgren <tony@atomide.com>,
	Alexander Kinzer <a.kinzer@plusoptix.de>,
	Stephen Boyd <sboyd@codeaurora.org>,
	Michael Turquette <mturquette@baylibre.com>
Subject: Re: [PATCH v3] clk: ti: omap36xx: Work around sprz319 advisory 2.1
Date: Mon, 5 Dec 2016 14:24:05 +0200	[thread overview]
Message-ID: <f020ef5f-0f0b-fd4c-d4da-55daa539c3d0@ti.com> (raw)
In-Reply-To: <3176488.xoM6adFiVO@avalon>

On 05/12/16 13:08, Laurent Pinchart wrote:
> Hi Ladislav,
>
> On Monday 05 Dec 2016 10:36:49 Ladislav Michl wrote:
>> On Mon, Dec 05, 2016 at 10:46:43AM +0200, Laurent Pinchart wrote:
>>> On Monday 05 Dec 2016 09:22:10 Ladislav Michl wrote:
>>
>> [snip]
>>
>>>> Table 36 list two options with 26MHz clocks: m=443, n=11 and m=480, n=12
>>>> with a statement: "The choice between these two options with a 26 MHz
>>>> input should be based on characterization on the end system."
>>>>
>>>> Shall we care about that?
>>>
>>> I'd like to, but at the moment I don't see how. Proposals are welcome :-)
>>> I
>>
>> One of proposals raised earlier was DT property, but that idea was scratched
>> later.
>
> It might not be such a bad idea, given that the decision should be made based
> on the characterization of the whole system. One could argue that such
> platform information could have its place in DT.
>
>>> don't think addressing that issue should be a blocker to get this patch
>>> merged though.
>>
>> Of course not. I'd like to even see it in stable ;-)
>>
>> [snip]
>>
>>> I had tried that, but I find the code less readable :-S
>>
>> Oh... Please reconsider (I really do not like that extra test and extra
>> assignment to local variables (also I had 'precomputed' as mixed definition,
>> but Tero did not quite like that)) :-)
>
> I still like to favour code readability when possible (especially when the
> compiler should optimize both versions the same way). I'm not the maintainer
> of this driver though, so I'll let Tero decides what he prefers.

The compiler should ideally generate same size code for these both. 
Personally, I don't mind which version goes in; I'd say both are as 
readable.

Stephen, Mike, is one of you going to pick this up? I don't think I have 
anything else to pull due to the ongoing discussion with the other 
pending stuff.

-Tero

>
>> Also, checked if the same values are written to clk as with my patch, so
>> here's my:
>> Tested-by: Ladislav Michl <ladis@linux-mips.org>
>
> Thank you.
>

WARNING: multiple messages have this Message-ID (diff)
From: Tero Kristo <t-kristo@ti.com>
To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Ladislav Michl <ladis@linux-mips.org>
Cc: linux-omap@vger.kernel.org, linux-clk@vger.kernel.org,
	Paul Walmsley <paul@pwsan.com>, Richard Watts <rrw@kynesim.co.uk>,
	Tony Lindgren <tony@atomide.com>,
	Alexander Kinzer <a.kinzer@plusoptix.de>,
	Stephen Boyd <sboyd@codeaurora.org>,
	Michael Turquette <mturquette@baylibre.com>
Subject: Re: [PATCH v3] clk: ti: omap36xx: Work around sprz319 advisory 2.1
Date: Mon, 5 Dec 2016 14:24:05 +0200	[thread overview]
Message-ID: <f020ef5f-0f0b-fd4c-d4da-55daa539c3d0@ti.com> (raw)
In-Reply-To: <3176488.xoM6adFiVO@avalon>

On 05/12/16 13:08, Laurent Pinchart wrote:
> Hi Ladislav,
>
> On Monday 05 Dec 2016 10:36:49 Ladislav Michl wrote:
>> On Mon, Dec 05, 2016 at 10:46:43AM +0200, Laurent Pinchart wrote:
>>> On Monday 05 Dec 2016 09:22:10 Ladislav Michl wrote:
>>
>> [snip]
>>
>>>> Table 36 list two options with 26MHz clocks: m=443, n=11 and m=480, n=12
>>>> with a statement: "The choice between these two options with a 26 MHz
>>>> input should be based on characterization on the end system."
>>>>
>>>> Shall we care about that?
>>>
>>> I'd like to, but at the moment I don't see how. Proposals are welcome :-)
>>> I
>>
>> One of proposals raised earlier was DT property, but that idea was scratched
>> later.
>
> It might not be such a bad idea, given that the decision should be made based
> on the characterization of the whole system. One could argue that such
> platform information could have its place in DT.
>
>>> don't think addressing that issue should be a blocker to get this patch
>>> merged though.
>>
>> Of course not. I'd like to even see it in stable ;-)
>>
>> [snip]
>>
>>> I had tried that, but I find the code less readable :-S
>>
>> Oh... Please reconsider (I really do not like that extra test and extra
>> assignment to local variables (also I had 'precomputed' as mixed definition,
>> but Tero did not quite like that)) :-)
>
> I still like to favour code readability when possible (especially when the
> compiler should optimize both versions the same way). I'm not the maintainer
> of this driver though, so I'll let Tero decides what he prefers.

The compiler should ideally generate same size code for these both. 
Personally, I don't mind which version goes in; I'd say both are as 
readable.

Stephen, Mike, is one of you going to pick this up? I don't think I have 
anything else to pull due to the ongoing discussion with the other 
pending stuff.

-Tero

>
>> Also, checked if the same values are written to clk as with my patch, so
>> here's my:
>> Tested-by: Ladislav Michl <ladis@linux-mips.org>
>
> Thank you.
>


  reply	other threads:[~2016-12-05 12:24 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-02 21:14 [PATCH v3] clk: ti: omap36xx: Work around sprz319 advisory 2.1 Laurent Pinchart
2016-12-02 22:41 ` Michael Turquette
2016-12-02 22:41   ` Michael Turquette
2016-12-05  8:22 ` Ladislav Michl
2016-12-05  8:46   ` Laurent Pinchart
2016-12-05  9:36     ` Ladislav Michl
2016-12-05 11:08       ` Laurent Pinchart
2016-12-05 12:24         ` Tero Kristo [this message]
2016-12-05 12:24           ` Tero Kristo
2016-12-08  0:16           ` Stephen Boyd
2016-12-08  7:11             ` Ladislav Michl
2016-12-08 11:40               ` Laurent Pinchart
2016-12-08 21:14                 ` Stephen Boyd
2016-12-05 23:59       ` Laurent Pinchart
2016-12-07 16:34         ` Ladislav Michl
2016-12-08 21:16 ` Stephen Boyd
2016-12-08 21:24   ` Laurent Pinchart
2017-01-03 18:00     ` Adam Ford
2017-01-03 18:49       ` Stephen Boyd
2017-01-03 22:16         ` Laurent Pinchart
2017-01-04 12:59           ` Adam Ford

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=f020ef5f-0f0b-fd4c-d4da-55daa539c3d0@ti.com \
    --to=t-kristo@ti.com \
    --cc=a.kinzer@plusoptix.de \
    --cc=ladis@linux-mips.org \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=mturquette@baylibre.com \
    --cc=paul@pwsan.com \
    --cc=rrw@kynesim.co.uk \
    --cc=sboyd@codeaurora.org \
    --cc=tony@atomide.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.