diff mbox series

[v7,06/10] ASoC: pxa: Suppress SSPA on ARM64

Message ID 20231102152033.5511-1-duje.mihanovic@skole.hr (mailing list archive)
State Handled Elsewhere
Headers show
Series Initial Marvell PXA1908 support | expand

Commit Message

Duje Mihanović Nov. 2, 2023, 3:20 p.m. UTC
The SSPA driver currently seems to generate ARM32 assembly, which causes
build errors when building a kernel for an ARM64 ARCH_MMP platform.

Fixes: fa375d42f0e5 ("ASoC: mmp: add sspa support")
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202310230518.zs9Qpg3j-lkp@intel.com/
Signed-off-by: Duje Mihanović <duje.mihanovic@skole.hr>
---
 sound/soc/pxa/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Mark Brown Nov. 2, 2023, 3:26 p.m. UTC | #1
On Thu, Nov 02, 2023 at 04:20:29PM +0100, Duje Mihanović wrote:
> The SSPA driver currently seems to generate ARM32 assembly, which causes
> build errors when building a kernel for an ARM64 ARCH_MMP platform.
> 
> Fixes: fa375d42f0e5 ("ASoC: mmp: add sspa support")
> Reported-by: kernel test robot <lkp@intel.com>

>  	tristate "SoC Audio via MMP SSPA ports"
> -	depends on ARCH_MMP
> +	depends on ARCH_MMP && ARM

This isn't a fix for the existing code, AFAICT the issue here is that
ARCH_MMP is currently only available for arm and presumably something in
the rest of your series makes it available for arm64.  This would be a
prerequisite for that patch.

Please don't just insert random fixes tags just because you can.
Robin Murphy Nov. 3, 2023, 3:23 p.m. UTC | #2
On 2023-11-02 3:26 pm, Mark Brown wrote:
> On Thu, Nov 02, 2023 at 04:20:29PM +0100, Duje Mihanović wrote:
>> The SSPA driver currently seems to generate ARM32 assembly, which causes
>> build errors when building a kernel for an ARM64 ARCH_MMP platform.
>>
>> Fixes: fa375d42f0e5 ("ASoC: mmp: add sspa support")
>> Reported-by: kernel test robot <lkp@intel.com>
> 
>>   	tristate "SoC Audio via MMP SSPA ports"
>> -	depends on ARCH_MMP
>> +	depends on ARCH_MMP && ARM
> 
> This isn't a fix for the existing code, AFAICT the issue here is that
> ARCH_MMP is currently only available for arm and presumably something in
> the rest of your series makes it available for arm64.  This would be a
> prerequisite for that patch.
> 
> Please don't just insert random fixes tags just because you can.

FWIW it doesn't even seem to be the right reason either. AFACIT the 
issue being introduced is that SND_MMP_SOC_SSPA selects SND_ARM which 
depends on ARM, but after patch #8 ARCH_MMP itself will no longer 
necessarily imply ARM. The fact that selecting SND_ARM with unmet 
dependencies also allows SND_ARMAACI to be enabled (which appears to be 
the only thing actually containing open-coded Arm asm) is tangential.

Robin.
Duje Mihanović Nov. 3, 2023, 4:58 p.m. UTC | #3
On Friday, November 3, 2023 4:23:28 PM CET Robin Murphy wrote:
> On 2023-11-02 3:26 pm, Mark Brown wrote:
> > This isn't a fix for the existing code, AFAICT the issue here is that
> > ARCH_MMP is currently only available for arm and presumably something in
> > the rest of your series makes it available for arm64.  This would be a
> > prerequisite for that patch.
> > 
> > Please don't just insert random fixes tags just because you can.
> 
> FWIW it doesn't even seem to be the right reason either. AFACIT the
> issue being introduced is that SND_MMP_SOC_SSPA selects SND_ARM which
> depends on ARM, but after patch #8 ARCH_MMP itself will no longer
> necessarily imply ARM. The fact that selecting SND_ARM with unmet
> dependencies also allows SND_ARMAACI to be enabled (which appears to be
> the only thing actually containing open-coded Arm asm) is tangential.

I just looked at it again and it looks like no code in sound/soc/pxa/* or 
sound/arm/pxa* depends on AACI in any way. Therefore, I believe that to fix 
this correctly, I would have to remove "select SND_ARM" from sound/soc/pxa/
Kconfig and optionally move the PXA2xx code out of sound/arm/ and into sound/
soc/pxa/. Is this correct? If so, I'd also split that fix into a separate 
series.

Regards,
Duje
Mark Brown Nov. 6, 2023, 10:58 a.m. UTC | #4
On Fri, Nov 03, 2023 at 05:58:05PM +0100, Duje Mihanović wrote:

> I just looked at it again and it looks like no code in sound/soc/pxa/* or 
> sound/arm/pxa* depends on AACI in any way. Therefore, I believe that to fix 
> this correctly, I would have to remove "select SND_ARM" from sound/soc/pxa/
> Kconfig and optionally move the PXA2xx code out of sound/arm/ and into sound/
> soc/pxa/. Is this correct? If so, I'd also split that fix into a separate 
> series.

There's the pxa-ac97 driver to consider...
Duje Mihanović Nov. 10, 2023, 7:28 p.m. UTC | #5
On Monday, November 6, 2023 11:58:46 AM CET Mark Brown wrote:
> On Fri, Nov 03, 2023 at 05:58:05PM +0100, Duje Mihanović wrote:
> > I just looked at it again and it looks like no code in sound/soc/pxa/* or
> > sound/arm/pxa* depends on AACI in any way. Therefore, I believe that to 
fix
> > this correctly, I would have to remove "select SND_ARM" from sound/soc/
pxa/
> > Kconfig and optionally move the PXA2xx code out of sound/arm/ and into
> > sound/
> > soc/pxa/. Is this correct? If so, I'd also split that fix into a separate
> > series.
> 
> There's the pxa-ac97 driver to consider...

Can you elaborate? As far as I can tell there are 2 drivers named pxa2xx-ac97 
in sound/{arm,soc/pxa} and neither one has any dependency on AACI. 

Regards,
Duje
Mark Brown Nov. 11, 2023, 11:39 a.m. UTC | #6
On Fri, Nov 10, 2023 at 08:28:56PM +0100, Duje Mihanović wrote:
> On Monday, November 6, 2023 11:58:46 AM CET Mark Brown wrote:
> > On Fri, Nov 03, 2023 at 05:58:05PM +0100, Duje Mihanović wrote:

> > > this correctly, I would have to remove "select SND_ARM" from sound/soc/
> pxa/
> > > Kconfig and optionally move the PXA2xx code out of sound/arm/ and into
> > > sound/
> > > soc/pxa/. Is this correct? If so, I'd also split that fix into a separate
> > > series.

> > There's the pxa-ac97 driver to consider...

> Can you elaborate? As far as I can tell there are 2 drivers named pxa2xx-ac97 
> in sound/{arm,soc/pxa} and neither one has any dependency on AACI. 

They do both share a lot of library code and the one in sound/arm has no
dependency on ASoC so I don't understand why you're suggesting moving it
to sound/soc.
Duje Mihanović Nov. 11, 2023, 9:01 p.m. UTC | #7
On Saturday, November 11, 2023 12:39:17 PM CET Mark Brown wrote:
> On Fri, Nov 10, 2023 at 08:28:56PM +0100, Duje Mihanović wrote:
> > Can you elaborate? As far as I can tell there are 2 drivers named
> > pxa2xx-ac97
> > in sound/{arm,soc/pxa} and neither one has any dependency on AACI.
> 
> They do both share a lot of library code and the one in sound/arm has no
> dependency on ASoC so I don't understand why you're suggesting moving it
> to sound/soc.

Right. Do you have any objections on removing "select SND_ARM" from 
SND_MMP_SOC_SSPA?

Regards,
Duje
diff mbox series

Patch

diff --git a/sound/soc/pxa/Kconfig b/sound/soc/pxa/Kconfig
index e6bca9070953..8ebce669e4a7 100644
--- a/sound/soc/pxa/Kconfig
+++ b/sound/soc/pxa/Kconfig
@@ -33,7 +33,7 @@  config SND_PXA_SOC_SSP
 
 config SND_MMP_SOC_SSPA
 	tristate "SoC Audio via MMP SSPA ports"
-	depends on ARCH_MMP
+	depends on ARCH_MMP && ARM
 	select SND_SOC_GENERIC_DMAENGINE_PCM
 	select SND_ARM
 	help