| Message ID | 1344379514-30076-2-git-send-email-matt@genesi-usa.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Aug 07, 2012 at 05:45:13PM -0500, Matt Sealey wrote: > If i.MX SSI FIQ support is enabled then it is impossible to build a Thumb2 > kernel image due to the code not being written with Thumb2 in mind (over-use > of registers). In order not to break Thumb2 kernels, compile this as ARM. All > the processors which require this support will run ARM code so there is no > problem at runtime in doing this, even if it does produce a strange mix of > code that may not be desired. > > In theory this should only be needed on configs based on imx_v4_v5_defconfig > where CONFIG_THUMB2_KERNEL cannot be selected anyway since these SoCs do not > have ARM cores capable of running Thumb2 code. This also makes rewriting the > code as Thumb-compatible kind of redundant. But since one Eukrea board audio > driver needs it, and there is an i.MX51 CPU module for this board, it gets > pulled in for imx_v6_v7_defconfig too, making this a necessity. > > Signed-off-by: Matt Sealey <matt@genesi-usa.com> > --- > arch/arm/plat-mxc/ssi-fiq.S | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/arm/plat-mxc/ssi-fiq.S b/arch/arm/plat-mxc/ssi-fiq.S > index 8397a2d..ac09af8 100644 > --- a/arch/arm/plat-mxc/ssi-fiq.S > +++ b/arch/arm/plat-mxc/ssi-fiq.S > @@ -28,6 +28,7 @@ > #define SSI_SIER_RFF0_EN (1 << 2) > > .text > + .arm > .global imx_ssi_fiq_start > .global imx_ssi_fiq_end > .global imx_ssi_fiq_base I think it would be better to add a depends on !THUMB2_KERNEL to SND_IMX_SOC_PCM_FIQ. The above may result in broken code in a thumb2 kernel, so I'd rather keep the compile error instead. Sascha
On Wed, Aug 8, 2012 at 1:55 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote: >> .text >> + .arm >> .global imx_ssi_fiq_start >> .global imx_ssi_fiq_end >> .global imx_ssi_fiq_base > > I think it would be better to add a depends on !THUMB2_KERNEL to > SND_IMX_SOC_PCM_FIQ. The above may result in broken code in a thumb2 > kernel, so I'd rather keep the compile error instead. I'm curious as to how/why would it result in broken code? It's not possible that the processors relying on the imx_ssi_fiq_* stuff cannot run ARM code (unless Freescale shipped a weird version) so it should magically enter and exit. I wonder if it needs some thumb-interworking stuff wrapped around it though. You'd know better than me.. I'm a little worried that making it !CONFIG_THUMB2_KERNEL would basically make more than one of the boards in imx_v6_v7_defconfig suddenly lose audio support for no other reason.. I don't like things just disappearing from the kernel by dependency magic. That said, a crash would be worse. I guess the eukrea tlv320 audio support should also be split, or looked at. This is the snippet that confuses me the most; config SND_SOC_EUKREA_TLV320 tristate "Eukrea TLV320" depends on MACH_EUKREA_MBIMX27_BASEBOARD \ || MACH_EUKREA_MBIMXSD25_BASEBOARD \ || MACH_EUKREA_MBIMXSD35_BASEBOARD \ || MACH_EUKREA_MBIMXSD51_BASEBOARD depends on I2C select SND_SOC_TLV320AIC23 select SND_SOC_IMX_PCM_FIQ select SND_SOC_IMX_AUDMUX select SND_SOC_IMX_SSI help Enable I2S based access to the TLV320AIC23B codec attached to the SSI interface .. Since you said I2S support doesn't require it, only AC97, this alone seems redundant, but since I wasn't the one to write it, test it, have no access to this board I have no idea. Mark's assertion that it's just not required now DMA works properly, could be true and this can be ditched, but I don't want to patch something I can't actually test.. my primary concern was fixing the build breakage. If that gets fixed, then the only dependencies are the WM1131_EV1 board which isn't in imx_v6_v7_defconfig, and the phyCORE support which needs it for AC97. That's one of your customers, right? I wouldn't want to disable a board, but I can justify to myself disabling the audio for *one* board based if it's just a caveat of a Thumb2 kernel. I'm going to do a trapse through and find where Russell nacked Dave's thumb-aware rewrite.. would you mind if you have any of these boards seeing if it really DOES cause broken code? The moment we noticed this was broken we quit designing with AC97 codecs, so there's no Genesi board of similar pedigree around that ever got to a PCB..
On Wed, Aug 08, 2012 at 12:32:39PM -0500, Matt Sealey wrote: > On Wed, Aug 8, 2012 at 1:55 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote: > >> .text > >> + .arm > >> .global imx_ssi_fiq_start > >> .global imx_ssi_fiq_end > >> .global imx_ssi_fiq_base > > > > I think it would be better to add a depends on !THUMB2_KERNEL to > > SND_IMX_SOC_PCM_FIQ. The above may result in broken code in a thumb2 > > kernel, so I'd rather keep the compile error instead. > > I'm curious as to how/why would it result in broken code? It's not > possible that the processors relying on > the imx_ssi_fiq_* stuff cannot run ARM code (unless Freescale shipped > a weird version) so it should > magically enter and exit. I wonder if it needs some thumb-interworking > stuff wrapped around it though. > You'd know better than me.. Currently I don't know if the code compiled in arm mode on an otherwise thumb2 kernel does work and I do not have a thumb2 capacle hardware with sound support to test this. > > I'm a little worried that making it !CONFIG_THUMB2_KERNEL would > basically make more than > one of the boards in imx_v6_v7_defconfig suddenly lose audio support > for no other reason.. Obviously no v6_v7 board does use this code as it is, because it does not compile. So there's no risk of breaking something if we just disable FIQ support in thumb2 mode. Better disable an unused feature than add some untested code. Sascha
diff --git a/arch/arm/plat-mxc/ssi-fiq.S b/arch/arm/plat-mxc/ssi-fiq.S index 8397a2d..ac09af8 100644 --- a/arch/arm/plat-mxc/ssi-fiq.S +++ b/arch/arm/plat-mxc/ssi-fiq.S @@ -28,6 +28,7 @@ #define SSI_SIER_RFF0_EN (1 << 2) .text + .arm .global imx_ssi_fiq_start .global imx_ssi_fiq_end .global imx_ssi_fiq_base
If i.MX SSI FIQ support is enabled then it is impossible to build a Thumb2 kernel image due to the code not being written with Thumb2 in mind (over-use of registers). In order not to break Thumb2 kernels, compile this as ARM. All the processors which require this support will run ARM code so there is no problem at runtime in doing this, even if it does produce a strange mix of code that may not be desired. In theory this should only be needed on configs based on imx_v4_v5_defconfig where CONFIG_THUMB2_KERNEL cannot be selected anyway since these SoCs do not have ARM cores capable of running Thumb2 code. This also makes rewriting the code as Thumb-compatible kind of redundant. But since one Eukrea board audio driver needs it, and there is an i.MX51 CPU module for this board, it gets pulled in for imx_v6_v7_defconfig too, making this a necessity. Signed-off-by: Matt Sealey <matt@genesi-usa.com> --- arch/arm/plat-mxc/ssi-fiq.S | 1 + 1 file changed, 1 insertion(+)