| Message ID | 1367874058-2378-2-git-send-email-eduardo.valentin@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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?
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.
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
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
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..
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 > >
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
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/
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
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.
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 --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
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(+)