Message ID | 1305628420-10592-1-git-send-email-peter.ujfalusi@ti.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Tue, 17 May 2011 13:33:40 +0300 Peter Ujfalusi <peter.ujfalusi@ti.com> wrote: > On OMAP4 we have one interrupt line per McBSP port. > At proble time tx, and rx irq value will be -ENXIO, > and only the tx irq will get corrected. > In omap_mcbsp_request if the rx_irq is not 0 we proceed, > and try to request the interrupt, which will fail on > OMAP4 (rx_irq == -6). > To avoid this error, clear the rx_irq at probe time > on OMAP4. > > Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com> > --- > arch/arm/plat-omap/mcbsp.c | 4 +++- > 1 files changed, 3 insertions(+), 1 deletions(-) > Acked-by: Jarkko Nikula <jhnikula@gmail.com> -- 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
* Jarkko Nikula <jhnikula@gmail.com> [110517 15:24]: > On Tue, 17 May 2011 13:33:40 +0300 > Peter Ujfalusi <peter.ujfalusi@ti.com> wrote: > > > On OMAP4 we have one interrupt line per McBSP port. > > At proble time tx, and rx irq value will be -ENXIO, > > and only the tx irq will get corrected. > > In omap_mcbsp_request if the rx_irq is not 0 we proceed, > > and try to request the interrupt, which will fail on > > OMAP4 (rx_irq == -6). > > To avoid this error, clear the rx_irq at probe time > > on OMAP4. > > > > Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com> > > --- > > arch/arm/plat-omap/mcbsp.c | 4 +++- > > 1 files changed, 3 insertions(+), 1 deletions(-) > > > Acked-by: Jarkko Nikula <jhnikula@gmail.com> This file should be under drivers/ somewhere, can you guys please take care of that? Regards, Tony -- 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
On Wed, 18 May 2011 08:52:07 +0300 Peter Ujfalusi <peter.ujfalusi@ti.com> wrote: > On Tuesday 17 May 2011 15:57:00 Tony Lindgren wrote: > > This file should be under drivers/ somewhere, can you > > guys please take care of that? > > Yeah, this has been discussed several times, and we have not reached agreement > where to move this very OMAP specific code. > One option was to move it under sound/soc/omap/ , since currently the only > user for mcbsp is audio. > But McBSP is really versatile beast, it can be used for other things (for > example it can handle SPI bus as well), so if we move it under ASoC, we are > going to limit/block other use of these pins. > We can not just cp arc/arm/plat-omap/mcbsp.c drivers/wherever... > If we do that, we need to move it under some framework, or create a new one > (bus driver?), which might be a bit tricky since we have special use of McBSP > from audio side, this does not really fit the bus mode. Other uses of McBSP > might be happy with the bus driver conversion, but we just do not have those. > > IMHO the only place we can move this is under sound/soc/omap/ , but who can > decide, that the McBSP can only be used for audio?? > I think we would need some higher level abstraction for this McBSP use model where the lowest level driver (here OMAP McBSP) is just used to configure the serial interface and a layer on top of that takes care of DMA transfer and protocol like SPI, I2S, etc. Why higher level abstraction? It's not only OMAP that has a general purpose serial interface. Also TI DaVinci has similar called McBSP/ASP and how about other SoCs, probably? What I looked once the DaVinci McBSP/ASP it wasn't compatible with OMAP McBSP but made me thinking that if these differences can be handled by a generic API.
Hi Tony, On Tuesday 17 May 2011 15:57:00 Tony Lindgren wrote: > This file should be under drivers/ somewhere, can you > guys please take care of that? Do you have a place in mind? We have several things under arch/arm/plat-omap, like i2c, dma, fb, gpio, mcbsp, usb... Should we create a directory for omap specific drivers under drivers? For example drivers/omap/ and move the mcbsp there first, without any API change? If needed we can think of changing the interface within McBSP if it is needed for other use later. IMHO invention a common framework (as Jarkko was suggesting) for similar interfaces need more work, and synchronization between other platforms.
* Jarkko Nikula <jhnikula@gmail.com> [110518 00:54]: > On Wed, 18 May 2011 08:52:07 +0300 > Peter Ujfalusi <peter.ujfalusi@ti.com> wrote: > > > On Tuesday 17 May 2011 15:57:00 Tony Lindgren wrote: > > > This file should be under drivers/ somewhere, can you > > > guys please take care of that? > > > > Yeah, this has been discussed several times, and we have not reached agreement > > where to move this very OMAP specific code. > > One option was to move it under sound/soc/omap/ , since currently the only > > user for mcbsp is audio. > > But McBSP is really versatile beast, it can be used for other things (for > > example it can handle SPI bus as well), so if we move it under ASoC, we are > > going to limit/block other use of these pins. > > We can not just cp arc/arm/plat-omap/mcbsp.c drivers/wherever... > > If we do that, we need to move it under some framework, or create a new one > > (bus driver?), which might be a bit tricky since we have special use of McBSP > > from audio side, this does not really fit the bus mode. Other uses of McBSP > > might be happy with the bus driver conversion, but we just do not have those. > > > > IMHO the only place we can move this is under sound/soc/omap/ , but who can > > decide, that the McBSP can only be used for audio?? > > > I think we would need some higher level abstraction for this McBSP use > model where the lowest level driver (here OMAP McBSP) is just used to > configure the serial interface and a layer on top of that takes care of > DMA transfer and protocol like SPI, I2S, etc. > > Why higher level abstraction? It's not only OMAP that has a general > purpose serial interface. Also TI DaVinci has similar called McBSP/ASP > and how about other SoCs, probably? What I looked once the DaVinci > McBSP/ASP it wasn't compatible with OMAP McBSP but made me thinking > that if these differences can be handled by a generic API. Yeah I think this is the way to go. Tony -- 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
* Peter Ujfalusi <peter.ujfalusi@ti.com> [110518 05:36]: > Hi Tony, > > On Tuesday 17 May 2011 15:57:00 Tony Lindgren wrote: > > This file should be under drivers/ somewhere, can you > > guys please take care of that? > > Do you have a place in mind? How about drivers/mcbsp? > We have several things under arch/arm/plat-omap, like i2c, dma, fb, gpio, > mcbsp, usb... These are all going away into drivers. Only the code to initialize platform dta should be under arch/arm/*omap*/. > Should we create a directory for omap specific drivers under drivers? No, this was discussed few weeks ago on LAKML. The drivers should be grouped by type instead. > For example drivers/omap/ and move the mcbsp there first, without any API > change? This is up to Greg, but I'd assume that a generic framework is the preferred way to go. > If needed we can think of changing the interface within McBSP if it is needed > for other use later. > IMHO invention a common framework (as Jarkko was suggesting) for similar > interfaces need more work, and synchronization between other platforms. Sure, but that's the only option we have to merge any new code. Regards, Tony -- 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
* Peter Ujfalusi <peter.ujfalusi@ti.com> [110608 00:46]: > Hi Tony, > > On Tuesday 31 May 2011 10:57:02 Tony Lindgren wrote: > > Sure, but that's the only option we have to merge any new code. > > So this patch will be not taken (even if without this patch OMAP4 McBSP is > broken), unless we move the McBSP code out from plat-omap? > > I have discussed with Jarkko, and internally about the mcbsp code move. This > will certainly take some time to figure out a sane way to do this. > To be hones I would be surprised if we can do this for 3.1... Sure we can merge fixes, but let's get the move done before adding new features. Regards, Tony -- 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
Hi Tony, On Monday 13 June 2011 15:35:43 Tony Lindgren wrote: > Sure we can merge fixes Can you take this patch forward? > but let's get the move done before adding > new features. What is considered a new feature here? Is it a new feature, if I fix the McBSP for OMAP4 (FIFO usage, and small updates)? We have discussion ongoing about the mcbsp code move, and so far the current understanding/plan is (Jarkko/Liam correct me if I'm wrong): - Fix the OMAP4 support - move the code under sound/soc/omap/ - the reason for this is that McBSP block can be only used for streaming type of modes (from omap2430 onwards McBSP does not have clock stop functionality). This makes other type of usage hard to say the least. - Clean up the code (remove the SPI mode, remove unused code paths at the same time) - Consolidate the interface for audio only use - Only OMAP3 has sidetone (on OMAP2 EAC block has the sidetone), might need some change, but I think the current way can be reused. In this way we are not going to hide the OMAP4 fixes with the code move/consolidation effort, so one can track what has been done, and why. Opinions?
On Tue, 14 Jun 2011 14:19:34 +0300 Péter Ujfalusi <peter.ujfalusi@ti.com> wrote: > We have discussion ongoing about the mcbsp code move, and so far the current > understanding/plan is (Jarkko/Liam correct me if I'm wrong): > - Fix the OMAP4 support > - move the code under sound/soc/omap/ > - the reason for this is that McBSP block can be only used for streaming type > of modes (from omap2430 onwards McBSP does not have clock stop functionality). > This makes other type of usage hard to say the least. > - Clean up the code (remove the SPI mode, remove unused code paths at the same > time) > - Consolidate the interface for audio only use Yeah, now is perfect time to tell if there is any need for something else than audio. OMAP1xxx/59xx and 2420 are already something like more than 5 years old obsolete components and we haven't seen that any of them would need SPI/clock stop functionality. Actually we haven't seen any other use for McBSP than audio. > - Only OMAP3 has sidetone (on OMAP2 EAC block has the sidetone), might need > some change, but I think the current way can be reused. > Yeah, they probably need work on kernel side (hwmod stuff etc) but I think for keeping userspace intact over the cleanup we might want to keep these special sysfs nodes for sidetone filter coefficients, dma operating mode and FIFO threshold as it over the cleanup.
* Jarkko Nikula <jhnikula@gmail.com> [110614 04:59]: > On Tue, 14 Jun 2011 14:19:34 +0300 > Péter Ujfalusi <peter.ujfalusi@ti.com> wrote: > > > We have discussion ongoing about the mcbsp code move, and so far the current > > understanding/plan is (Jarkko/Liam correct me if I'm wrong): > > - Fix the OMAP4 support > > - move the code under sound/soc/omap/ > > - the reason for this is that McBSP block can be only used for streaming type > > of modes (from omap2430 onwards McBSP does not have clock stop functionality). > > This makes other type of usage hard to say the least. > > - Clean up the code (remove the SPI mode, remove unused code paths at the same > > time) > > - Consolidate the interface for audio only use > > Yeah, now is perfect time to tell if there is any need for something > else than audio. > > OMAP1xxx/59xx and 2420 are already something like more > than 5 years old obsolete components and we haven't seen that any of > them would need SPI/clock stop functionality. Actually we haven't seen > any other use for McBSP than audio. Yes this sounds OK to me. So the order to do things should be: - Fix issues - Remove unused code - Move to drivers - Add new features as needed > > - Only OMAP3 has sidetone (on OMAP2 EAC block has the sidetone), might need > > some change, but I think the current way can be reused. > > > Yeah, they probably need work on kernel side (hwmod stuff etc) but I > think for keeping userspace intact over the cleanup we might want to > keep these special sysfs nodes for sidetone filter coefficients, dma > operating mode and FIFO threshold as it over the cleanup. Also related is the sound/soc/omap mess that should not be doing any platform level stuff at all but instead get the configuration in platform data and device tree data eventually. Just grep for platform_set_drvdata in sound/soc/omap to see it.. Regards, Tony -- 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
diff --git a/arch/arm/plat-omap/mcbsp.c b/arch/arm/plat-omap/mcbsp.c index 5587acf..1e4e32e 100644 --- a/arch/arm/plat-omap/mcbsp.c +++ b/arch/arm/plat-omap/mcbsp.c @@ -1867,8 +1867,10 @@ static int __devinit omap_mcbsp_probe(struct platform_device *pdev) mcbsp->rx_irq = platform_get_irq_byname(pdev, "rx"); /* From OMAP4 there will be a single irq line */ - if (mcbsp->tx_irq == -ENXIO) + if (mcbsp->tx_irq == -ENXIO) { mcbsp->tx_irq = platform_get_irq(pdev, 0); + mcbsp->rx_irq = 0; + } res = platform_get_resource_byname(pdev, IORESOURCE_DMA, "rx"); if (!res) {
On OMAP4 we have one interrupt line per McBSP port. At proble time tx, and rx irq value will be -ENXIO, and only the tx irq will get corrected. In omap_mcbsp_request if the rx_irq is not 0 we proceed, and try to request the interrupt, which will fail on OMAP4 (rx_irq == -6). To avoid this error, clear the rx_irq at probe time on OMAP4. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com> --- arch/arm/plat-omap/mcbsp.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-)