Re: [PATCH v2] SPARC: Fix parport_pc support for 32-bit platforms

[Randy Dunlap]

On 6/18/23 18:01, Maciej W. Rozycki wrote:
> For 32-bit SPARC platforms PC-style parallel ports are only available as 
> PCI options.  Adjust <asm/parport.h> accordingly, fixing build errors:
> 
> sparc-linux-gnu-ld: drivers/parport/parport_pc.o: in function `ecpp_remove':
> parport_pc.c:(.text+0x8f0): undefined reference to `ebus_dma_irq_enable'
> sparc-linux-gnu-ld: parport_pc.c:(.text+0x8f8): undefined reference to `ebus_dma_unregister'
> sparc-linux-gnu-ld: drivers/parport/parport_pc.o: in function `parport_pc_fifo_write_block_dma':
> parport_pc.c:(.text+0x1430): undefined reference to `ebus_dma_enable'
> sparc-linux-gnu-ld: parport_pc.c:(.text+0x1444): undefined reference to `ebus_dma_prepare'
> sparc-linux-gnu-ld: parport_pc.c:(.text+0x14e4): undefined reference to `ebus_dma_enable'
> sparc-linux-gnu-ld: parport_pc.c:(.text+0x14f4): undefined reference to `ebus_dma_request'
> sparc-linux-gnu-ld: parport_pc.c:(.text+0x1584): undefined reference to `ebus_dma_enable'
> sparc-linux-gnu-ld: parport_pc.c:(.text+0x1594): undefined reference to `ebus_dma_residue'
> sparc-linux-gnu-ld: parport_pc.c:(.text+0x1608): undefined reference to `ebus_dma_enable'
> sparc-linux-gnu-ld: parport_pc.c:(.text+0x1618): undefined reference to `ebus_dma_residue'
> sparc-linux-gnu-ld: drivers/parport/parport_pc.o: in function `ecpp_probe':
> parport_pc.c:(.text+0x33a0): undefined reference to `ebus_dma_register'
> sparc-linux-gnu-ld: parport_pc.c:(.text+0x33b8): undefined reference to `ebus_dma_irq_enable'
> sparc-linux-gnu-ld: parport_pc.c:(.text+0x34b0): undefined reference to `ebus_dma_irq_enable'
> sparc-linux-gnu-ld: parport_pc.c:(.text+0x34bc): undefined reference to `ebus_dma_unregister'
> 
> Reported-by: Randy Dunlap <rdunlap@xxxxxxxxxxxxx> 
> Closes: https://lore.kernel.org/r/20230406160548.25721-1-rdunlap@xxxxxxxxxxxxx/
> Signed-off-by: Maciej W. Rozycki <macro@xxxxxxxxxxx>
> Fixes: 66bcd06099bb ("parport_pc: Also enable driver for PCI systems")
> Cc: stable@xxxxxxxxxxxxxxx # v5.18+

Acked-by: Randy Dunlap <rdunlap@xxxxxxxxxxxxx>
Tested-by: Randy Dunlap <rdunlap@xxxxxxxxxxxxx> # build-tested

Thanks for fixing this.

> ---

> Changes from v1:
> 
> - Wrap `dma_spin_lock' into HAS_DMA to fix a "'dma_spin_lock' defined but 
>   not used" warning.  I think it's slightly cleaner this way compared to 
>   wrapping the whole part into CONFIG_SPARC64, as this better reflects the 
>   structure of the dependencies here.  It also follows the structure of 
>   parport_pc.c, which likewise has `parport_pc_fifo_write_block_dma' put 
>   into CONFIG_PARPORT_PC_FIFO and then HAS_DMA conditionals.
> 
> - Add Reported-by tag.
> ---
>  arch/sparc/include/asm/parport.h |    7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> linux-sparc32-parport-pc.diff
> Index: linux-macro/arch/sparc/include/asm/parport.h
> ===================================================================
> --- linux-macro.orig/arch/sparc/include/asm/parport.h
> +++ linux-macro/arch/sparc/include/asm/parport.h
> @@ -19,9 +19,11 @@
>   * While sparc64 doesn't have an ISA DMA API, we provide something that looks
>   * close enough to make parport_pc happy
>   */
> +#ifdef CONFIG_SPARC64
>  #define HAS_DMA
> +#endif
>  
> -#ifdef CONFIG_PARPORT_PC_FIFO
> +#if defined(CONFIG_PARPORT_PC_FIFO) && defined(HAS_DMA)
>  static DEFINE_SPINLOCK(dma_spin_lock);
>  
>  #define claim_dma_lock() \
> @@ -249,7 +251,8 @@ static struct platform_driver ecpp_drive
>  
>  static int parport_pc_find_nonpci_ports(int autoirq, int autodma)
>  {
> -	return platform_driver_register(&ecpp_driver);
> +	return (IS_ENABLED(CONFIG_SPARC64) &&
> +		platform_driver_register(&ecpp_driver));
>  }
>  
>  #endif /* !(_ASM_SPARC64_PARPORT_H */

-- 
~Randy