All of lore.kernel.org
 help / color / mirror / Atom feed
From: gregory.clement@free-electrons.com (Gregory CLEMENT)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 3/6] arm: kconfig: don't select TWD with local timer for Armada 370/XP
Date: Mon, 21 Jan 2013 23:37:22 +0100	[thread overview]
Message-ID: <50FDC322.2040107@free-electrons.com> (raw)
In-Reply-To: <201301211831.45947.arnd@arndb.de>

On 01/21/2013 07:31 PM, Arnd Bergmann wrote:
> On Monday 21 January 2013, Gregory CLEMENT wrote:
>> @@ -1624,7 +1624,7 @@ config LOCAL_TIMERS
>>         bool "Use local timer interrupts"
>>         depends on SMP
>>         default y
>> -       select HAVE_ARM_TWD if (!ARCH_MSM_SCORPIONMP && !EXYNOS4_MCT)
>> +       select HAVE_ARM_TWD if (!ARCH_MSM_SCORPIONMP && !EXYNOS4_MCT && !ARMADA_370_XP_TIMER)
>>         
> 
> Your change is fine, but I just noticed that this line is asking for trouble
> when we enable multipleform support for MSM and/or EXYNOS.
> 
> Also, I wonder if we should change this somehow in 3.8, because it seems
> that for a multiplatform kernel including armadaxp and e.g. versatile express,
> you have no ARM_TWD support in the kernel, which seems wrong. Is there any
> reason we can't enable the ARM_TWD code to be built-in on platforms that
> don't use it?

I don't see a strong reason to not enable it if we don't use it. My concern
was that I don't need it so I didn't want to include it and generating extra
code for nothing. Then just after having sent this patch set, I received your
patch set about build regression in 3.8 and especially the part about
CONFIG_MULTIPLATFORM made me realized that it could be a problem.

> 
> Maybe it can be written as
> 
> config LOCAL_TIMERS
> 	bool "Use local timer interrupts"
> 	depends on SMP
> 	default y
> 
> config HAVE_ARM_TWD
> 	depends on LOCAL_TIMERS
> 	default ARCH_MULTIPLATFORM || (!ARCH_MSM_SCORPIONMP && !EXYNOS4_MCT)

So in this case why not written something like this:
 	default ARCH_MULTIPLATFORM || (!ARCH_MSM_SCORPIONMP && !EXYNOS4_MCT && !ARMADA_370_XP_TIMER)

> 	default y
I am not a kconfig expert, but won't this line set HAVE_ARM_TWD to 'y' whatever
the result of the previous line?

> 
> This will still be slightly wrong (generating extra code) on a multiplatform
> kernel that has no platform other than MSM or EXYNOS, but the other alternative
> would be that each platform with TWD support has to select HAVE_ARM_TWD itself.

Gregory

WARNING: multiple messages have this Message-ID (diff)
From: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
To: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
Cc: Lior Amsalem <alior-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>,
	Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org>,
	Ike Pan <ike.pan-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>,
	John Stultz <johnstul-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>,
	David Marlin <dmarlin-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>,
	Yehuda Yitschak <yehuday-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>,
	Jani Monoses
	<jani.monoses-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>,
	Russell King <linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>,
	Tawfik Bayouk <tawfik-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>,
	Dan Frazier
	<dann.frazier-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>,
	Eran Ben-Avi <benavi-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>,
	Leif Lindholm <leif.lindholm-5wv7dgnIgG8@public.gmane.org>,
	Sebastian Hesselbarth
	<sebastian.hesselbarth-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Jason Cooper <jason-NLaQJdtUoK4Be96aLqz0jA@public.gmane.org>,
	Nadav Haklai <nadavh-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>,
	Jon Masters <jcm-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
	Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>,
	Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	Chris Van Hoof <vanhoof-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Maen Suleiman <maen-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>,
	Shadi Ammouri <shadi@marvell>
Subject: Re: [PATCH 3/6] arm: kconfig: don't select TWD with local timer for Armada 370/XP
Date: Mon, 21 Jan 2013 23:37:22 +0100	[thread overview]
Message-ID: <50FDC322.2040107@free-electrons.com> (raw)
In-Reply-To: <201301211831.45947.arnd-r2nGTMty4D4@public.gmane.org>

On 01/21/2013 07:31 PM, Arnd Bergmann wrote:
> On Monday 21 January 2013, Gregory CLEMENT wrote:
>> @@ -1624,7 +1624,7 @@ config LOCAL_TIMERS
>>         bool "Use local timer interrupts"
>>         depends on SMP
>>         default y
>> -       select HAVE_ARM_TWD if (!ARCH_MSM_SCORPIONMP && !EXYNOS4_MCT)
>> +       select HAVE_ARM_TWD if (!ARCH_MSM_SCORPIONMP && !EXYNOS4_MCT && !ARMADA_370_XP_TIMER)
>>         
> 
> Your change is fine, but I just noticed that this line is asking for trouble
> when we enable multipleform support for MSM and/or EXYNOS.
> 
> Also, I wonder if we should change this somehow in 3.8, because it seems
> that for a multiplatform kernel including armadaxp and e.g. versatile express,
> you have no ARM_TWD support in the kernel, which seems wrong. Is there any
> reason we can't enable the ARM_TWD code to be built-in on platforms that
> don't use it?

I don't see a strong reason to not enable it if we don't use it. My concern
was that I don't need it so I didn't want to include it and generating extra
code for nothing. Then just after having sent this patch set, I received your
patch set about build regression in 3.8 and especially the part about
CONFIG_MULTIPLATFORM made me realized that it could be a problem.

> 
> Maybe it can be written as
> 
> config LOCAL_TIMERS
> 	bool "Use local timer interrupts"
> 	depends on SMP
> 	default y
> 
> config HAVE_ARM_TWD
> 	depends on LOCAL_TIMERS
> 	default ARCH_MULTIPLATFORM || (!ARCH_MSM_SCORPIONMP && !EXYNOS4_MCT)

So in this case why not written something like this:
 	default ARCH_MULTIPLATFORM || (!ARCH_MSM_SCORPIONMP && !EXYNOS4_MCT && !ARMADA_370_XP_TIMER)

> 	default y
I am not a kconfig expert, but won't this line set HAVE_ARM_TWD to 'y' whatever
the result of the previous line?

> 
> This will still be slightly wrong (generating extra code) on a multiplatform
> kernel that has no platform other than MSM or EXYNOS, but the other alternative
> would be that each platform with TWD support has to select HAVE_ARM_TWD itself.

Gregory

  parent reply	other threads:[~2013-01-21 22:37 UTC|newest]

Thread overview: 64+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-01-21 17:53 [PATCH 0/6] arm: mvebu: add support for local timer for Armada 370/XP Gregory CLEMENT
2013-01-21 17:53 ` Gregory CLEMENT
2013-01-21 17:53 ` [PATCH 1/6] arm: mvebu: Add support for local interrupt Gregory CLEMENT
2013-01-21 17:53   ` Gregory CLEMENT
2013-01-21 17:53   ` Gregory CLEMENT
2013-01-21 18:17   ` Thomas Petazzoni
2013-01-21 18:17     ` Thomas Petazzoni
2013-01-21 22:07     ` Gregory CLEMENT
2013-01-21 22:07       ` Gregory CLEMENT
2013-01-21 23:26       ` Ezequiel Garcia
2013-01-21 23:26         ` Ezequiel Garcia
2013-01-22  9:09         ` Gregory CLEMENT
2013-01-22  9:09           ` Gregory CLEMENT
2013-01-22 16:55           ` Thomas Petazzoni
2013-01-22 16:55             ` Thomas Petazzoni
2013-01-21 17:53 ` [PATCH 2/6] clocksource: time-armada-370-xp: add local timer support Gregory CLEMENT
2013-01-21 17:53   ` Gregory CLEMENT
2013-01-21 17:53   ` Gregory CLEMENT
2013-01-21 17:59   ` Russell King - ARM Linux
2013-01-21 17:59     ` Russell King - ARM Linux
2013-01-21 18:04     ` Gregory CLEMENT
2013-01-21 18:04       ` Gregory CLEMENT
2013-01-23 13:11   ` Gregory CLEMENT
2013-01-23 13:11     ` Gregory CLEMENT
2013-01-21 17:53 ` [PATCH 3/6] arm: kconfig: don't select TWD with local timer for Armada 370/XP Gregory CLEMENT
2013-01-21 17:53   ` Gregory CLEMENT
2013-01-21 18:31   ` Arnd Bergmann
2013-01-21 18:31     ` Arnd Bergmann
2013-01-21 19:29     ` Matt Sealey
2013-01-21 19:29       ` Matt Sealey
2013-01-21 20:44       ` Arnd Bergmann
2013-01-21 20:44         ` Arnd Bergmann
2013-01-22 17:12         ` Russell King - ARM Linux
2013-01-22 17:12           ` Russell King - ARM Linux
2013-01-22 20:46         ` Rob Herring
2013-01-22 20:46           ` Rob Herring
2013-01-22 20:46           ` Rob Herring
2013-01-22 21:19           ` Arnd Bergmann
2013-01-22 21:19             ` Arnd Bergmann
2013-01-22 21:19             ` Arnd Bergmann
2013-01-21 22:37     ` Gregory CLEMENT [this message]
2013-01-21 22:37       ` Gregory CLEMENT
2013-01-22 15:57       ` Arnd Bergmann
2013-01-22 15:57         ` Arnd Bergmann
2013-01-22 16:34         ` Gregory CLEMENT
2013-01-22 16:34           ` Gregory CLEMENT
2013-01-22 17:42           ` [PATCH] arm: kconfig: always select TWD with local timer for multiplatform Gregory CLEMENT
2013-01-22 17:18         ` [PATCH 3/6] arm: kconfig: don't select TWD with local timer for Armada 370/XP Russell King - ARM Linux
2013-01-22 17:18           ` Russell King - ARM Linux
2013-01-22 17:42           ` Arnd Bergmann
2013-01-22 17:42             ` Arnd Bergmann
2013-01-21 17:54 ` [PATCH 4/6] arm: mvebu: update defconfig with local timer support Gregory CLEMENT
2013-01-21 17:54   ` Gregory CLEMENT
2013-01-21 17:54   ` Gregory CLEMENT
2013-01-21 17:54 ` [PATCH 5/6] arm: mvebu: update DT to support local timers Gregory CLEMENT
2013-01-21 17:54   ` Gregory CLEMENT
2013-01-21 17:54 ` [PATCH 6/6] clocksource: update and move armada-370-xp-timer documentation to timer directory Gregory CLEMENT
2013-01-21 17:54   ` Gregory CLEMENT
2013-01-21 18:22   ` Arnd Bergmann
2013-01-21 18:22     ` Arnd Bergmann
2013-01-21 22:05     ` Gregory CLEMENT
2013-01-21 22:05       ` Gregory CLEMENT
2013-01-21 22:26       ` Arnd Bergmann
2013-01-21 22:26         ` Arnd Bergmann

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=50FDC322.2040107@free-electrons.com \
    --to=gregory.clement@free-electrons.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 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.