All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tero Kristo <t-kristo@ti.com>
To: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>,
	linux-omap@vger.kernel.org, tony@atomide.com, paul@pwsan.com
Cc: linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCHv4 23/35] ARM: OMAP2+: id: cache omap_type value
Date: Thu, 19 Mar 2015 09:23:19 +0200	[thread overview]
Message-ID: <550A7967.5070409@ti.com> (raw)
In-Reply-To: <5509C2E0.8010705@cogentembedded.com>

On 03/18/2015 08:24 PM, Sergei Shtylyov wrote:
> Hello.
>
> On 03/18/2015 05:44 PM, Tero Kristo wrote:
>
>> There is no need to read the register with every invocation of the
>> function,
>> as the value is constant. Thus, cache the value in a static variable.
>
>> Signed-off-by: Tero Kristo <t-kristo@ti.com>
>> ---
>>   arch/arm/mach-omap2/id.c |    5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>
>> diff --git a/arch/arm/mach-omap2/id.c b/arch/arm/mach-omap2/id.c
>> index 2a2f4d5..1f5949f 100644
>> --- a/arch/arm/mach-omap2/id.c
>> +++ b/arch/arm/mach-omap2/id.c
>> @@ -52,7 +52,10 @@ EXPORT_SYMBOL(omap_rev);
>>
>>   int omap_type(void)
>>   {
>> -    u32 val = 0;
>> +    static u32 val = OMAP2_DEVICETYPE_MASK + 1;
>> +
>> +    if (val < OMAP2_DEVICETYPE_MASK)
>
>     <=, perhaps?

Hmm right, there is something fishy with this patch. Basically either 
comparison works as the MASK is a shifted value which is always several 
magnitudes larger than the resulting omap_type value itself. The +1 part 
in the init value is completely unnecessary though.

-Tero

>
>> +        return val;
> [...]
>
> WBR, Sergei
>


WARNING: multiple messages have this Message-ID (diff)
From: t-kristo@ti.com (Tero Kristo)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCHv4 23/35] ARM: OMAP2+: id: cache omap_type value
Date: Thu, 19 Mar 2015 09:23:19 +0200	[thread overview]
Message-ID: <550A7967.5070409@ti.com> (raw)
In-Reply-To: <5509C2E0.8010705@cogentembedded.com>

On 03/18/2015 08:24 PM, Sergei Shtylyov wrote:
> Hello.
>
> On 03/18/2015 05:44 PM, Tero Kristo wrote:
>
>> There is no need to read the register with every invocation of the
>> function,
>> as the value is constant. Thus, cache the value in a static variable.
>
>> Signed-off-by: Tero Kristo <t-kristo@ti.com>
>> ---
>>   arch/arm/mach-omap2/id.c |    5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>
>> diff --git a/arch/arm/mach-omap2/id.c b/arch/arm/mach-omap2/id.c
>> index 2a2f4d5..1f5949f 100644
>> --- a/arch/arm/mach-omap2/id.c
>> +++ b/arch/arm/mach-omap2/id.c
>> @@ -52,7 +52,10 @@ EXPORT_SYMBOL(omap_rev);
>>
>>   int omap_type(void)
>>   {
>> -    u32 val = 0;
>> +    static u32 val = OMAP2_DEVICETYPE_MASK + 1;
>> +
>> +    if (val < OMAP2_DEVICETYPE_MASK)
>
>     <=, perhaps?

Hmm right, there is something fishy with this patch. Basically either 
comparison works as the MASK is a shifted value which is always several 
magnitudes larger than the resulting omap_type value itself. The +1 part 
in the init value is completely unnecessary though.

-Tero

>
>> +        return val;
> [...]
>
> WBR, Sergei
>

  reply	other threads:[~2015-03-19  7:23 UTC|newest]

Thread overview: 92+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-18 14:43 [PATCHv4 00/35] ARM: OMAP2+: PRCM / SCM cleanups against 4.0-rc1 Tero Kristo
2015-03-18 14:43 ` Tero Kristo
2015-03-18 14:43 ` [PATCHv4 01/35] ARM: OMAP2+: PRCM: rename of_prcm_init to omap_prcm_init Tero Kristo
2015-03-18 14:43   ` Tero Kristo
2015-03-18 14:43 ` [PATCHv4 02/35] ARM: OMAP3: PRM: invert the wkst_mask for the prm_clear_mod_irqs Tero Kristo
2015-03-18 14:43   ` Tero Kristo
2015-03-18 14:43 ` [PATCHv4 03/35] ARM: OMAP2+: PRM: add generic API for clear_mod_irqs Tero Kristo
2015-03-18 14:43   ` Tero Kristo
2015-03-18 14:43 ` [PATCHv4 04/35] ARM: OMAP3+: PRM: add common APIs for prm_vp_check/clear_txdone Tero Kristo
2015-03-18 14:43   ` Tero Kristo
2015-03-18 14:43 ` [PATCHv4 05/35] ARM: OMAP4+: PRM: move omap_prm_base_init under OMAP4 PRM driver Tero Kristo
2015-03-18 14:43   ` Tero Kristo
2015-03-18 14:43 ` [PATCHv4 06/35] ARM: OMAP4+: CM: move omap_cm_base_init under OMAP4 CM driver Tero Kristo
2015-03-18 14:43   ` Tero Kristo
2015-03-18 14:43 ` [PATCHv4 07/35] ARM: OMAP4: PRM: move omap4xxx_prm_init earlier in init order Tero Kristo
2015-03-18 14:43   ` Tero Kristo
2015-03-18 14:43 ` [PATCHv4 08/35] clk: ti: fix ti_clk_get_reg_addr error handling Tero Kristo
2015-03-18 14:43   ` Tero Kristo
2015-03-18 14:43 ` [PATCHv4 09/35] Documentation: DT: document PRCM compatible strings for dm81x SoCs Tero Kristo
2015-03-18 14:43   ` Tero Kristo
2015-03-18 14:43 ` [PATCHv4 10/35] ARM: OMAP2+: PRCM: add support for static clock memmap indices Tero Kristo
2015-03-18 14:43   ` Tero Kristo
2015-03-18 14:43 ` [PATCHv4 11/35] ARM: OMAP2+: clock: move clock provider infrastructure to clock driver Tero Kristo
2015-03-18 14:43   ` Tero Kristo
2015-03-18 14:43 ` [PATCHv4 12/35] ARM: OMAP2+: PRCM: split PRCM module init to their own driver files Tero Kristo
2015-03-18 14:43   ` Tero Kristo
2015-03-18 14:43 ` [PATCHv4 13/35] ARM: OMAP2+: CM: determine CM base address from device tree Tero Kristo
2015-03-18 14:43   ` Tero Kristo
2015-03-18 14:43 ` [PATCHv4 14/35] ARM: OMAP2+: PRM: determine PRM " Tero Kristo
2015-03-18 14:43   ` Tero Kristo
2015-03-18 14:44 ` [PATCHv4 15/35] ARM: OMAP2+: control: determine control module base address from DT Tero Kristo
2015-03-18 14:44   ` Tero Kristo
2015-03-18 14:44 ` [PATCHv4 16/35] ARM: OMAP2+: PRM: move SoC specific init calls within a generic API Tero Kristo
2015-03-18 14:44   ` Tero Kristo
2015-03-18 14:44 ` [PATCHv4 17/35] ARM: OMAP4+: PRM: determine prm_device_inst based on DT compatibility Tero Kristo
2015-03-18 14:44   ` Tero Kristo
2015-03-18 14:44 ` [PATCHv4 18/35] ARM: OMAP2+: CM: move SoC specific init calls within a generic API Tero Kristo
2015-03-18 14:44   ` Tero Kristo
2015-03-18 14:44 ` [PATCHv4 19/35] ARM: OMAP4+: PRM: setup prm_features from the PRM init time flags Tero Kristo
2015-03-18 14:44   ` Tero Kristo
2015-03-18 14:44 ` [PATCHv4 20/35] ARM: OMAP4+: PRM: get rid of cpu_is_omap44xx calls from interrupt init Tero Kristo
2015-03-18 14:44   ` Tero Kristo
2015-03-18 14:44 ` [PATCHv4 21/35] ARM: OMAP2+: clock: add low-level support for regmap Tero Kristo
2015-03-18 14:44   ` Tero Kristo
2015-03-18 14:44 ` [PATCHv4 22/35] ARM: OMAP2+: control: remove API for getting control module base address Tero Kristo
2015-03-18 14:44   ` Tero Kristo
2015-03-18 14:44 ` [PATCHv4 23/35] ARM: OMAP2+: id: cache omap_type value Tero Kristo
2015-03-18 14:44   ` Tero Kristo
2015-03-18 18:24   ` Sergei Shtylyov
2015-03-18 18:24     ` Sergei Shtylyov
2015-03-19  7:23     ` Tero Kristo [this message]
2015-03-19  7:23       ` Tero Kristo
2015-03-18 14:44 ` [PATCHv4 24/35] ARM: OMAP2+: control: add syscon support for register accesses Tero Kristo
2015-03-18 14:44   ` Tero Kristo
2015-03-18 14:44 ` [PATCHv4 25/35] ARM: dts: omap24xx: merge control module features under scrm node Tero Kristo
2015-03-18 14:44   ` Tero Kristo
2015-03-18 14:44 ` [PATCHv4 26/35] ARM: dts: omap3: " Tero Kristo
2015-03-18 14:44   ` Tero Kristo
2015-03-18 14:44 ` [PATCHv4 27/35] ARM: dts: am33xx: " Tero Kristo
2015-03-18 14:44   ` Tero Kristo
2015-03-18 14:44 ` [PATCHv4 28/35] ARM: dts: am43xx-epos-evm: fix pinmux node layout Tero Kristo
2015-03-18 14:44   ` Tero Kristo
2015-03-18 14:44 ` [PATCHv4 29/35] ARM: dts: am4372: merge control module features under scrm node Tero Kristo
2015-03-18 14:44   ` Tero Kristo
2015-03-18 14:44 ` [PATCHv4 30/35] ARM: dts: omap4: add system control module node Tero Kristo
2015-03-18 14:44   ` Tero Kristo
2015-03-18 21:30   ` Tony Lindgren
2015-03-18 21:30     ` Tony Lindgren
2015-03-19 20:25     ` Tony Lindgren
2015-03-19 20:25       ` Tony Lindgren
2015-03-20  6:40       ` Tero Kristo
2015-03-20  6:40         ` Tero Kristo
2015-03-18 14:44 ` [PATCHv4 31/35] ARM: OMAP4: display: convert display to use syscon for dsi muxing Tero Kristo
2015-03-18 14:44   ` Tero Kristo
2015-03-18 14:44 ` [PATCHv4 32/35] ARM: OMAP4+: control: remove support for legacy pad read/write Tero Kristo
2015-03-18 14:44   ` Tero Kristo
2015-03-18 14:44 ` [PATCHv4 33/35] ARM: dts: omap5: add system control module node Tero Kristo
2015-03-18 14:44   ` Tero Kristo
2015-03-18 14:44 ` [PATCHv4 34/35] ARM: dts: dra7: " Tero Kristo
2015-03-18 14:44   ` Tero Kristo
2015-03-18 14:44 ` [PATCHv4 35/35] ARM: OMAP4+: control: add support for initializing control module via DT Tero Kristo
2015-03-18 14:44   ` Tero Kristo
2015-03-18 22:07 ` [PATCHv4 00/35] ARM: OMAP2+: PRCM / SCM cleanups against 4.0-rc1 Tony Lindgren
2015-03-18 22:07   ` Tony Lindgren
2015-03-19  7:48   ` Tero Kristo
2015-03-19  7:48     ` Tero Kristo
2015-03-19 16:10     ` Tony Lindgren
2015-03-19 16:10       ` Tony Lindgren
2015-03-19  0:00 ` Sakari Ailus
2015-03-19  0:00   ` Sakari Ailus
2015-03-19  8:05   ` Tero Kristo
2015-03-19  8:05     ` Tero Kristo

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=550A7967.5070409@ti.com \
    --to=t-kristo@ti.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=paul@pwsan.com \
    --cc=sergei.shtylyov@cogentembedded.com \
    --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.