diff mbox

[RESEND,1/3] arm: dts: introduce config HAS_BANDGAP

Message ID 1367874058-2378-2-git-send-email-eduardo.valentin@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Eduardo Valentin May 6, 2013, 9 p.m. UTC
Introduce HAS_BANDGAP config entry. This config is a
boolean value so that arch code can flag is they
feature a bandgap device.

Cc: Russell King <linux@arm.linux.org.uk>
Cc: Tony Lindgren <tony@atomide.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-omap@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Eduardo Valentin <eduardo.valentin@ti.com>
---
 arch/arm/Kconfig            | 3 +++
 arch/arm/mach-omap2/Kconfig | 1 +
 2 files changed, 4 insertions(+)

Comments

Stephen Warren May 6, 2013, 9:31 p.m. UTC | #1
On 05/06/2013 03:00 PM, Eduardo Valentin wrote:
> Introduce HAS_BANDGAP config entry. This config is a
> boolean value so that arch code can flag is they
> feature a bandgap device.

What does this patch have to do with device tree (there's "dts" in the
subject)? What's a BANDGAP device?
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Aaro Koskinen May 6, 2013, 9:34 p.m. UTC | #2
On Mon, May 06, 2013 at 05:00:56PM -0400, Eduardo Valentin wrote:
> Introduce HAS_BANDGAP config entry. This config is a
> boolean value so that arch code can flag is they
> feature a bandgap device.

Maybe it could be mentioned that omap-thermal already depend on this?
At least for a random reviewer it was not immediately clear why this is
added, especially since there were no users for it in subsequent patches.

A.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jason Gunthorpe May 6, 2013, 10:36 p.m. UTC | #3
On Tue, May 07, 2013 at 12:34:13AM +0300, Aaro Koskinen wrote:
> On Mon, May 06, 2013 at 05:00:56PM -0400, Eduardo Valentin wrote:
> > Introduce HAS_BANDGAP config entry. This config is a
> > boolean value so that arch code can flag is they
> > feature a bandgap device.
> 
> Maybe it could be mentioned that omap-thermal already depend on this?
> At least for a random reviewer it was not immediately clear why this is
> added, especially since there were no users for it in subsequent patches.

I looked (very briefly), and it seemed like omap-thermal is self
contained and doesn't need arch support?

I get the impression it is desired to minimize driver kconfig
dependencies to the minimum required to compile to increase build
testing coverage, so maybe it would be appropriate to drop this
entirely?

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eduardo Valentin May 6, 2013, 10:40 p.m. UTC | #4
On 06-05-2013 17:34, Aaro Koskinen wrote:
> On Mon, May 06, 2013 at 05:00:56PM -0400, Eduardo Valentin wrote:
>> Introduce HAS_BANDGAP config entry. This config is a
>> boolean value so that arch code can flag is they
>> feature a bandgap device.
> 
> Maybe it could be mentioned that omap-thermal already depend on this?
> At least for a random reviewer it was not immediately clear why this is
> added, especially since there were no users for it in subsequent patches.
> 
> A.
> 
> 
The flag has been proposed here [1]. The adaptation on the driver has
been introduced here [2,3]. It has reached the staging (next) tree. That
is why you do not see any follow up patch. Although it does not have
users at v3.9, the patch on the user has been sent several weeks ago.

[1] -
http://lists.infradead.org/pipermail/linux-arm-kernel/2012-May/101827.html
[2] - http://marc.info/?l=linux-omap&m=136335347305197&w=2
[3] - http://marc.info/?l=linux-driver-devel&m=136335404205514&w=1
Eduardo Valentin May 7, 2013, 12:20 a.m. UTC | #5
On 06-05-2013 17:31, Stephen Warren wrote:
> On 05/06/2013 03:00 PM, Eduardo Valentin wrote:
>> Introduce HAS_BANDGAP config entry. This config is a
>> boolean value so that arch code can flag is they
>> feature a bandgap device.
> 
> What does this patch have to do with device tree (there's "dts" in the
> subject)? What's a BANDGAP device?
> 
> 
In fact, this one has nothing to do with DT. dts in the title is a typo..
Eduardo Valentin May 7, 2013, 12:23 a.m. UTC | #6
On 06-05-2013 18:36, Jason Gunthorpe wrote:
> On Tue, May 07, 2013 at 12:34:13AM +0300, Aaro Koskinen wrote:
>> On Mon, May 06, 2013 at 05:00:56PM -0400, Eduardo Valentin wrote:
>>> Introduce HAS_BANDGAP config entry. This config is a
>>> boolean value so that arch code can flag is they
>>> feature a bandgap device.
>>
>> Maybe it could be mentioned that omap-thermal already depend on this?
>> At least for a random reviewer it was not immediately clear why this is
>> added, especially since there were no users for it in subsequent patches.
> 
> I looked (very briefly), and it seemed like omap-thermal is self
> contained and doesn't need arch support?
> 

It is. At least I tried to make it, at least to the extenstion I could get.

> I get the impression it is desired to minimize driver kconfig
> dependencies to the minimum required to compile to increase build
> testing coverage, so maybe it would be appropriate to drop this
> entirely?
> 

Well, it is also desired to compile things to the correct target right?
Thats the idea behind this config option. It follows the same design as
CONFIG_ARCH_HAS_CPUFREQ, for instance.

> Jason
> 
>
Jason Gunthorpe May 7, 2013, 12:36 a.m. UTC | #7
On Mon, May 06, 2013 at 08:23:13PM -0400, Eduardo Valentin wrote:

> > I get the impression it is desired to minimize driver kconfig
> > dependencies to the minimum required to compile to increase build
> > testing coverage, so maybe it would be appropriate to drop this
> > entirely?

> Well, it is also desired to compile things to the correct target
> right?

There is some of that too..

But broadly the direction seems that drivers should have minimal
dependencies so, eg, the thermal maintainer compiling for x86 should
be able to compile test/static analyze your driver..

> Thats the idea behind this config option. It follows the same design as
> CONFIG_ARCH_HAS_CPUFREQ, for instance.

That is entirely contained inside arch/arm and doesn't involve
drivers.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eduardo Valentin May 7, 2013, 1:15 p.m. UTC | #8
Hello Jason,

On 06-05-2013 20:36, Jason Gunthorpe wrote:
> On Mon, May 06, 2013 at 08:23:13PM -0400, Eduardo Valentin wrote:
> 
>>> I get the impression it is desired to minimize driver kconfig
>>> dependencies to the minimum required to compile to increase build
>>> testing coverage, so maybe it would be appropriate to drop this
>>> entirely?
> 
>> Well, it is also desired to compile things to the correct target
>> right?
> 
> There is some of that too..
> 
> But broadly the direction seems that drivers should have minimal
> dependencies so, eg, the thermal maintainer compiling for x86 should
> be able to compile test/static analyze your driver..
> 

Well, I do not see much of this attempt actually. Do you have some link
/ evidene that shows someone who actually cares about compiling drivers
for targets that they are not used for? On this specific driver, I
actually have  had exactly the opposite advice [1]. I am not convinced
people actually want to do that.

>> Thats the idea behind this config option. It follows the same design as
>> CONFIG_ARCH_HAS_CPUFREQ, for instance.
> 
> That is entirely contained inside arch/arm and doesn't involve
> drivers.

It actually goes outside arch/arm.

> 
> Jason
> 
> 

[1] - https://patchwork.kernel.org/patch/1185431/
Jason Gunthorpe May 7, 2013, 6:27 p.m. UTC | #9
On Tue, May 07, 2013 at 09:15:00AM -0400, Eduardo Valentin wrote:
> > But broadly the direction seems that drivers should have minimal
> > dependencies so, eg, the thermal maintainer compiling for x86 should
> > be able to compile test/static analyze your driver..

> Well, I do not see much of this attempt actually. Do you have some link
> / evidene that shows someone who actually cares about compiling drivers
> for targets that they are not used for? On this specific driver, I
> actually have  had exactly the opposite advice [1]. I am not convinced
> people actually want to do that.

There was a discussion a bit ago, but I can't find a link.. The
context was subsystem maintainers are being asked to look after more
code with the DT transition moving things out of arch/arm and at least
one complained they couldn't even compile test on x86... But again, I
can't find a link and you are right, there are lots and lots of
'depends ARCH..' style things in kConfig already.

Lets just call it something to think about.

> >> Thats the idea behind this config option. It follows the same design as
> >> CONFIG_ARCH_HAS_CPUFREQ, for instance.
> > 
> > That is entirely contained inside arch/arm and doesn't involve
> > drivers.
> 
> It actually goes outside arch/arm.

Hm, must have missed that, seemed like all it did was control
including drivers/cpufreq/Kconfig within the ARM kconfigs.. And
unicore32 copied the name, but did the same thing.

Regards,
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Aaro Koskinen May 7, 2013, 7:06 p.m. UTC | #10
On Tue, May 07, 2013 at 12:27:11PM -0600, Jason Gunthorpe wrote:
> On Tue, May 07, 2013 at 09:15:00AM -0400, Eduardo Valentin wrote:
> > > But broadly the direction seems that drivers should have minimal
> > > dependencies so, eg, the thermal maintainer compiling for x86 should
> > > be able to compile test/static analyze your driver..
> 
> > Well, I do not see much of this attempt actually. Do you have some link
> > / evidene that shows someone who actually cares about compiling drivers
> > for targets that they are not used for? On this specific driver, I
> > actually have  had exactly the opposite advice [1]. I am not convinced
> > people actually want to do that.
> 
> There was a discussion a bit ago, but I can't find a link.. The
> context was subsystem maintainers are being asked to look after more
> code with the DT transition moving things out of arch/arm and at least
> one complained they couldn't even compile test on x86... But again, I
> can't find a link and you are right, there are lots and lots of
> 'depends ARCH..' style things in kConfig already.
> 
> Lets just call it something to think about.

Tomi started a thread related to this recently:

	http://marc.info/?l=linux-kernel&m=136731558332265&w=2

I think there's some good reasons listed there, but I guess up to the
subsystem maintainers to decide what they prefer.

A.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tomi Valkeinen May 8, 2013, 6:04 a.m. UTC | #11
On 07/05/13 22:06, Aaro Koskinen wrote:
> On Tue, May 07, 2013 at 12:27:11PM -0600, Jason Gunthorpe wrote:
>> On Tue, May 07, 2013 at 09:15:00AM -0400, Eduardo Valentin wrote:
>>>> But broadly the direction seems that drivers should have minimal
>>>> dependencies so, eg, the thermal maintainer compiling for x86 should
>>>> be able to compile test/static analyze your driver..
>>
>>> Well, I do not see much of this attempt actually. Do you have some link
>>> / evidene that shows someone who actually cares about compiling drivers
>>> for targets that they are not used for? On this specific driver, I
>>> actually have  had exactly the opposite advice [1]. I am not convinced
>>> people actually want to do that.
>>
>> There was a discussion a bit ago, but I can't find a link.. The
>> context was subsystem maintainers are being asked to look after more
>> code with the DT transition moving things out of arch/arm and at least
>> one complained they couldn't even compile test on x86... But again, I
>> can't find a link and you are right, there are lots and lots of
>> 'depends ARCH..' style things in kConfig already.
>>
>> Lets just call it something to think about.
> 
> Tomi started a thread related to this recently:
> 
> 	http://marc.info/?l=linux-kernel&m=136731558332265&w=2
> 
> I think there's some good reasons listed there, but I guess up to the
> subsystem maintainers to decide what they prefer.

As Sam pointed out in that thread, there's no need to change the Kconfig
language for this. I did some testing with fb drivers, by having a
CONFIG_ALL_FB_DRIVERS option which "removes" the arch dependencies for
some fb drivers.

It does uglify the Kconfig files a bit:

-       depends on FB && (SUPERH || ARCH_SHMOBILE) && HAVE_CLK
+       depends on FB && (SUPERH || ARCH_SHMOBILE || ALL_FB_DRIVERS) &&
HAVE_CLK

I'm still undecided if I want to pursue this with fb drivers, as it
seems that quite many of them do really depend on the arch code, and I'm
not interested in trying to fix them.

But if other subsystems would benefit of this also, perhaps we could
have a common CONFIG entry that would allow compiling a driver on any
arch. I just can't think of a good name for the config entry =).

 Tomi
diff mbox

Patch

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index f95674a..39fc561 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -176,6 +176,9 @@  config ARCH_HAS_CPUFREQ
 	  and that the relevant menu configurations are displayed for
 	  it.
 
+config ARCH_HAS_BANDGAP
+	bool
+
 config GENERIC_HWEIGHT
 	bool
 	default y
diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
index f49cd51..8620ab5 100644
--- a/arch/arm/mach-omap2/Kconfig
+++ b/arch/arm/mach-omap2/Kconfig
@@ -4,6 +4,7 @@  config ARCH_OMAP
 config ARCH_OMAP2PLUS
 	bool "TI OMAP2/3/4/5 SoCs with device tree support" if (ARCH_MULTI_V6 || ARCH_MULTI_V7)
 	select ARCH_HAS_CPUFREQ
+	select ARCH_HAS_BANDGAP
 	select ARCH_HAS_HOLES_MEMORYMODEL
 	select ARCH_OMAP
 	select ARCH_REQUIRE_GPIOLIB