* [PATCH 1/2] OMAP: features: export symbol omap3_features @ 2010-09-24 3:41 Omar Ramirez Luna 2010-09-24 14:58 ` Kevin Hilman 2010-09-24 15:34 ` Paul Walmsley 0 siblings, 2 replies; 7+ messages in thread From: Omar Ramirez Luna @ 2010-09-24 3:41 UTC (permalink / raw) To: linux-arm-kernel From: Kevin Hilman <khilman@deeprootsystems•com> Since omap3_features is a global variable, only code built-in the kernel can make use of it and thus use omap_has ##feat functions; exporting this as a kernel symbol makes modules able to use feature detection framework too. Thread: http://marc.info/?l=linux-omap&m=128528822902211&w=2 Signed-off-by: Kevin Hilman <khilman@deeprootsystems•com> Signed-off-by: Omar Ramirez Luna <omar.ramirez@ti•com> CC: Tony Lindgren <tony@atomide•com> CC: Russell King <linux@arm•linux.org.uk> CC: Nishanth Menon <nm@ti•com> CC: Paul Walmsley <paul@pwsan•com> CC: Sanjeev Premi <premi@ti•com> CC: linux-omap at vger.kernel.org CC: linux-arm-kernel at lists.infradead.org --- arch/arm/mach-omap2/id.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/arch/arm/mach-omap2/id.c b/arch/arm/mach-omap2/id.c index 9a879f9..a8c6d19 100644 --- a/arch/arm/mach-omap2/id.c +++ b/arch/arm/mach-omap2/id.c @@ -31,6 +31,7 @@ static struct omap_chip_id omap_chip; static unsigned int omap_revision; u32 omap3_features; +EXPORT_SYMBOL(omap3_features); unsigned int omap_rev(void) { -- 1.7.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 1/2] OMAP: features: export symbol omap3_features 2010-09-24 3:41 [PATCH 1/2] OMAP: features: export symbol omap3_features Omar Ramirez Luna @ 2010-09-24 14:58 ` Kevin Hilman 2010-09-24 15:34 ` Paul Walmsley 1 sibling, 0 replies; 7+ messages in thread From: Kevin Hilman @ 2010-09-24 14:58 UTC (permalink / raw) To: linux-arm-kernel Omar Ramirez Luna <omar.ramirez@ti•com> writes: > From: Kevin Hilman <khilman@deeprootsystems•com> Minor nit, can you make the subject something like: OMAP: features: make feature checks available to modules Thanks, Kevin > Since omap3_features is a global variable, only code > built-in the kernel can make use of it and thus use > omap_has ##feat functions; exporting this as a kernel > symbol makes modules able to use feature detection > framework too. > > Thread: http://marc.info/?l=linux-omap&m=128528822902211&w=2 > > Signed-off-by: Kevin Hilman <khilman@deeprootsystems•com> > Signed-off-by: Omar Ramirez Luna <omar.ramirez@ti•com> > CC: Tony Lindgren <tony@atomide•com> > CC: Russell King <linux@arm•linux.org.uk> > CC: Nishanth Menon <nm@ti•com> > CC: Paul Walmsley <paul@pwsan•com> > CC: Sanjeev Premi <premi@ti•com> > CC: linux-omap at vger.kernel.org > CC: linux-arm-kernel at lists.infradead.org > --- > arch/arm/mach-omap2/id.c | 1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/arch/arm/mach-omap2/id.c b/arch/arm/mach-omap2/id.c > index 9a879f9..a8c6d19 100644 > --- a/arch/arm/mach-omap2/id.c > +++ b/arch/arm/mach-omap2/id.c > @@ -31,6 +31,7 @@ static struct omap_chip_id omap_chip; > static unsigned int omap_revision; > > u32 omap3_features; > +EXPORT_SYMBOL(omap3_features); > > unsigned int omap_rev(void) > { ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] OMAP: features: export symbol omap3_features 2010-09-24 3:41 [PATCH 1/2] OMAP: features: export symbol omap3_features Omar Ramirez Luna 2010-09-24 14:58 ` Kevin Hilman @ 2010-09-24 15:34 ` Paul Walmsley 2010-09-24 16:51 ` Nishanth Menon 1 sibling, 1 reply; 7+ messages in thread From: Paul Walmsley @ 2010-09-24 15:34 UTC (permalink / raw) To: linux-arm-kernel Hi Omar, Kevin, On Thu, 23 Sep 2010, Omar Ramirez Luna wrote: > From: Kevin Hilman <khilman@deeprootsystems•com> > > Since omap3_features is a global variable, only code > built-in the kernel can make use of it and thus use > omap_has ##feat functions; exporting this as a kernel > symbol makes modules able to use feature detection > framework too. > > Thread: http://marc.info/?l=linux-omap&m=128528822902211&w=2 > > Signed-off-by: Kevin Hilman <khilman@deeprootsystems•com> > Signed-off-by: Omar Ramirez Luna <omar.ramirez@ti•com> > CC: Tony Lindgren <tony@atomide•com> > CC: Russell King <linux@arm•linux.org.uk> > CC: Nishanth Menon <nm@ti•com> > CC: Paul Walmsley <paul@pwsan•com> > CC: Sanjeev Premi <premi@ti•com> > CC: linux-omap at vger.kernel.org > CC: linux-arm-kernel at lists.infradead.org > --- > arch/arm/mach-omap2/id.c | 1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/arch/arm/mach-omap2/id.c b/arch/arm/mach-omap2/id.c > index 9a879f9..a8c6d19 100644 > --- a/arch/arm/mach-omap2/id.c > +++ b/arch/arm/mach-omap2/id.c > @@ -31,6 +31,7 @@ static struct omap_chip_id omap_chip; > static unsigned int omap_revision; > > u32 omap3_features; > +EXPORT_SYMBOL(omap3_features); > > unsigned int omap_rev(void) > { > -- > 1.7.1 > This omap3_features variable should not be used directly by any device drivers since it is an OMAP-ism. This type of feature info should be passed through struct platform_data. Looks like this would be quite easy to add by editing mach-omap2/devices.c and adding platform_data? ... In the medium-term, definitely all of those #if defined(CONFIG_ARCH_OMAP*) and if (cpu_is_omap*()) { in this driver need to be removed. The integration code (currently in mach-omap2/devices.c) is what should handle this, or better yet, struct dev_attr from hwmod. Also the entire mailboxes driver at some point should be moved into drivers/* ... - Paul ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] OMAP: features: export symbol omap3_features 2010-09-24 15:34 ` Paul Walmsley @ 2010-09-24 16:51 ` Nishanth Menon 2010-09-24 17:06 ` Paul Walmsley 0 siblings, 1 reply; 7+ messages in thread From: Nishanth Menon @ 2010-09-24 16:51 UTC (permalink / raw) To: linux-arm-kernel Paul Walmsley had written, on 09/24/2010 10:34 AM, the following: [...] >> >> u32 omap3_features; >> +EXPORT_SYMBOL(omap3_features); >> >> unsigned int omap_rev(void) >> { >> -- >> 1.7.1 >> > > This omap3_features variable should not be used directly by any device > drivers since it is an OMAP-ism. This type of feature info should be > passed through struct platform_data. Looks like this would be quite easy > to add by editing mach-omap2/devices.c and adding platform_data? > > ... > > In the medium-term, definitely all of those > > #if defined(CONFIG_ARCH_OMAP*) > > and > > if (cpu_is_omap*()) { > > in this driver need to be removed. The integration code (currently in > mach-omap2/devices.c) is what should handle this, or better yet, struct > dev_attr from hwmod. > > Also the entire mailboxes driver at some point should be moved into > drivers/* ... just my 2 cents: The intent of omap_has_featureX() is to ensure that the drivers dont do cpu_is_omap123(). Now if we had OMAP dma driver which has DMA chaining - what options do we have DMA driver? a) we introduce it based on cpu_is_omap123() -> bad bad nightmare for maintenance b) we introduce it based on module h/w block(TI internal terminology "IP block") revision -> unfortunately no luck in some of the h/w blocks. c) we use if (omap_has_dma_chaining()) If the hwmod itself was expanded to contain feature, and erratas then it is excellent solution - one stop information source for all drivers and a standardized soln as well.. (but we dont have it on omap1). by not exporting this, the drivers are restricted to being static, and exporting allows those omap specific ones to be loadable (if someone really wants so minimalist a system - i believe i2c is an example of loadable module).. I believe omap_has_feature() is a readable and maintainable way of handling features and exporting helps those drivers to become modules (yes with the risk of some other driver misusing it as well) - this definitely is something we've been doing for some time in l-o I guess.. a better alternative might be hwmod - but I am no expert to comment on it's feasibility.. -- Regards, Nishanth Menon ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] OMAP: features: export symbol omap3_features 2010-09-24 16:51 ` Nishanth Menon @ 2010-09-24 17:06 ` Paul Walmsley 2010-09-24 17:17 ` Paul Walmsley 0 siblings, 1 reply; 7+ messages in thread From: Paul Walmsley @ 2010-09-24 17:06 UTC (permalink / raw) To: linux-arm-kernel On Fri, 24 Sep 2010, Nishanth Menon wrote: > Paul Walmsley had written, on 09/24/2010 10:34 AM, the following: > [...] > > > u32 omap3_features; > > > +EXPORT_SYMBOL(omap3_features); > > > unsigned int omap_rev(void) > > > { > > > -- > > > 1.7.1 > > > > > > > This omap3_features variable should not be used directly by any device > > drivers since it is an OMAP-ism. This type of feature info should be passed > > through struct platform_data. Looks like this would be quite easy to add by > > editing mach-omap2/devices.c and adding platform_data? > > > > ... > > > > In the medium-term, definitely all of those > > #if defined(CONFIG_ARCH_OMAP*) > > > > and > > if (cpu_is_omap*()) { > > > > in this driver need to be removed. The integration code (currently in > > mach-omap2/devices.c) is what should handle this, or better yet, struct > > dev_attr from hwmod. > > > > Also the entire mailboxes driver at some point should be moved into > > drivers/* ... > just my 2 cents: > > The intent of omap_has_featureX() is to ensure that the drivers dont do > cpu_is_omap123(). Now if we had OMAP dma driver which has DMA chaining - what > options do we have DMA driver? > > a) we introduce it based on cpu_is_omap123() -> bad bad nightmare for > maintenance > b) we introduce it based on module h/w block(TI internal terminology "IP > block") revision -> unfortunately no luck in some of the h/w blocks. > c) we use if (omap_has_dma_chaining()) d) you pass in errata/feature flags via a platform_data struct, like McBSP, McSPI, MMC, MUSB, I2C, etc. already do on OMAP. On OMAP1, which doesn't have hwmod support, you set your platform_data in your OMAP1 integration code. On OMAP2+ (which has hwmod support), you define your errata/feature flags and any other integration data that you need to pass via the struct omap_hwmod.dev_attr field, then pass that data via struct platform_data in the OMAP2+ integration code. See for example http://www.mail-archive.com/linux-omap at vger.kernel.org/msg13288.html and http://marc.info/?l=linux-omap&m=124419789124570&w=2 (search for "dev_attr" in both cases) ... As an aside, I think that any errata/features that can be automatically discriminated by the IP block revision register are better handled that way than by platform_data; but of course, as you mention, the hardware people sometimes neglect to bump that when they change something. - Paul ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] OMAP: features: export symbol omap3_features 2010-09-24 17:06 ` Paul Walmsley @ 2010-09-24 17:17 ` Paul Walmsley 2010-09-24 17:26 ` Nishanth Menon 0 siblings, 1 reply; 7+ messages in thread From: Paul Walmsley @ 2010-09-24 17:17 UTC (permalink / raw) To: linux-arm-kernel On Fri, 24 Sep 2010, Paul Walmsley wrote: > On Fri, 24 Sep 2010, Nishanth Menon wrote: > > > The intent of omap_has_featureX() is to ensure that the drivers dont do > > cpu_is_omap123(). Now if we had OMAP dma driver which has DMA chaining - what > > options do we have DMA driver? > > > > a) we introduce it based on cpu_is_omap123() -> bad bad nightmare for > > maintenance > > b) we introduce it based on module h/w block(TI internal terminology "IP > > block") revision -> unfortunately no luck in some of the h/w blocks. > > c) we use if (omap_has_dma_chaining()) > > d) you pass in errata/feature flags via a platform_data struct, like > McBSP, McSPI, MMC, MUSB, I2C, etc. already do on OMAP. On OMAP1, which > doesn't have hwmod support, you set your platform_data in your OMAP1 > integration code. On OMAP2+ (which has hwmod support), you define your > errata/feature flags and any other integration data that you need to pass > via the struct omap_hwmod.dev_attr field, then pass that data via struct > platform_data in the OMAP2+ integration code. Just to clarify something that may be unclear: there's no problem with calling cpu_is_omapXXXX(), or any other OMAP core-specific function, from the OMAP<->driver integration code, living in arch/arm/*omap*. The results of those functions can then be passed through platform_data. But there is a problem with calling OMAP core-specific functions directly from the driver code itself, since driver code should be completely platform-independent -- e.g., the same DMA controller could exist on OMAP, DaVinci, etc. - Paul ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] OMAP: features: export symbol omap3_features 2010-09-24 17:17 ` Paul Walmsley @ 2010-09-24 17:26 ` Nishanth Menon 0 siblings, 0 replies; 7+ messages in thread From: Nishanth Menon @ 2010-09-24 17:26 UTC (permalink / raw) To: linux-arm-kernel Paul Walmsley had written, on 09/24/2010 12:17 PM, the following: > On Fri, 24 Sep 2010, Paul Walmsley wrote: > >> On Fri, 24 Sep 2010, Nishanth Menon wrote: >> >>> The intent of omap_has_featureX() is to ensure that the drivers dont do >>> cpu_is_omap123(). Now if we had OMAP dma driver which has DMA chaining - what >>> options do we have DMA driver? >>> >>> a) we introduce it based on cpu_is_omap123() -> bad bad nightmare for >>> maintenance >>> b) we introduce it based on module h/w block(TI internal terminology "IP >>> block") revision -> unfortunately no luck in some of the h/w blocks. >>> c) we use if (omap_has_dma_chaining()) >> d) you pass in errata/feature flags via a platform_data struct, like >> McBSP, McSPI, MMC, MUSB, I2C, etc. already do on OMAP. On OMAP1, which >> doesn't have hwmod support, you set your platform_data in your OMAP1 >> integration code. On OMAP2+ (which has hwmod support), you define your >> errata/feature flags and any other integration data that you need to pass >> via the struct omap_hwmod.dev_attr field, then pass that data via struct >> platform_data in the OMAP2+ integration code. > > Just to clarify something that may be unclear: there's no problem with > calling cpu_is_omapXXXX(), or any other OMAP core-specific function, from > the OMAP<->driver integration code, living in arch/arm/*omap*. The > results of those functions can then be passed through platform_data. But > there is a problem with calling OMAP core-specific functions directly from > the driver code itself, since driver code should be completely > platform-independent -- e.g., the same DMA controller could exist on OMAP, > DaVinci, etc. > yep. your comment makes sense now. thanks for clarifying. NAK from me as well. -- Regards, Nishanth Menon ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2010-09-24 17:26 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-09-24 3:41 [PATCH 1/2] OMAP: features: export symbol omap3_features Omar Ramirez Luna 2010-09-24 14:58 ` Kevin Hilman 2010-09-24 15:34 ` Paul Walmsley 2010-09-24 16:51 ` Nishanth Menon 2010-09-24 17:06 ` Paul Walmsley 2010-09-24 17:17 ` Paul Walmsley 2010-09-24 17:26 ` Nishanth Menon
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox