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