On Mon, 2015-10-12 at 17:44 +0200, Arnd Bergmann wrote:
> On Monday 12 October 2015 08:28:01 James Bottomley wrote:
> > > 
> > > diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
> > > index d2f480b04a52..d4aa6a1a806c 100644
> > > --- a/drivers/scsi/Kconfig
> > > +++ b/drivers/scsi/Kconfig
> > > @@ -499,6 +499,7 @@ config SCSI_ADVANSYS
> > >       tristate "AdvanSys SCSI support"
> > >       depends on SCSI
> > >       depends on ISA || EISA || PCI
> > > +     depends on ISA_DMA_API || !ISA
> > >       help
> > >         This is a driver for all SCSI host adapters manufactured by
> > >         AdvanSys. It is documented in the kernel source in
> > 
> > This fix looks wrong.  the request_dma code is confined within an #ifdef
> > CONFIG_ISA section but the advansys doesn't actually require an ISA DMA
> > channel to function, so you're saying there are systems with ISA but
> > without request_dma()?
> 
> Yes. Specifically, the ARM EBSA110 and SA1100 platforms can have some
> PIO based ISA devices, but they have nothing close enough to an i8237
> to support the request_dma interface.
> 
> > If so I think we leave the depends alone and try to bring the board up
> > in NO_ISA_DMA mode.
> 
> Ok
> 
> >  That means the narrowboard check should be gated by
> > CONFIG_ISA_DMA_API ... do we also have to gate free_dma as well?
> 
> Yes. A few more as well, as we also don't want to do inb/outb
> instructions to a DMA controller that is not there.
> 
> I've compile-tested the patch below.
> 
>       Arnd
> 
> 8<-------
> From 67fed3da5dd65abf5e4d01d8731c5217eb823fdb Mon Sep 17 00:00:00 2001
> From: Arnd Bergmann <a...@arndb.de>
> Date: Sat, 10 Oct 2015 21:13:29 +0200
> Subject: [PATCH] scsi: advansys: fix build without ISA DMA API
> 
> The advansys drvier uses the request_dma function that is used on ISA
> machines for the internal DMA controller, which causes build errors
> on platforms that have ISA slots but do not provide the ISA DMA API:
> 
> drivers/scsi/advansys.c: In function 'advansys_board_found':
> drivers/scsi/advansys.c:11300:10: error: implicit declaration of function 
> 'request_dma' [-Werror=implicit-function-declaration]
> 
> The problem now showed up in ARM randconfig builds after commit
> 6571fb3f8b7f ("advansys: Update to version 3.5 and remove compilation
> warning") made it possible to build on platforms that have neither
> VIRT_TO_BUS nor ISA_DMA_API but that do have ISA.
> 
> This changes the existing #ifdefs in the driver to check for both
> CONFIG_ISA and CONFIG_ISA_DMA_API where appropriate. It should
> still be possible to use the driver in PIO mode with ISA when
> DMA is not available.
> 
> Signed-off-by: Arnd Bergmann <a...@arndb.de>
> 
> diff --git a/drivers/scsi/advansys.c b/drivers/scsi/advansys.c
> index 1c1cd657c380..9f8aae40dcc8 100644
> --- a/drivers/scsi/advansys.c
> +++ b/drivers/scsi/advansys.c
> @@ -2883,9 +2883,9 @@ static void asc_prt_asc_board_eeprom(struct seq_file 
> *m, struct Scsi_Host *shost
>       ASC_DVC_VAR *asc_dvc_varp;
>       ASCEEP_CONFIG *ep;
>       int i;
> -#ifdef CONFIG_ISA
> +#if defined(CONFIG_ISA) && defined(CONFIG_ISA_DMA_API)
>       int isa_dma_speed[] = { 10, 8, 7, 6, 5, 4, 3, 2 };
> -#endif /* CONFIG_ISA */
> +#endif /* ISA */
>       uchar serialstr[13];
>  
>       asc_dvc_varp = &boardp->dvc_var.asc_dvc_var;
> @@ -2937,13 +2937,13 @@ static void asc_prt_asc_board_eeprom(struct seq_file 
> *m, struct Scsi_Host *shost
>                          (ep->init_sdtr & ADV_TID_TO_TIDMASK(i)) ? 'Y' : 'N');
>       seq_putc(m, '\n');
>  
> -#ifdef CONFIG_ISA
> +#if defined(CONFIG_ISA) && defined(CONFIG_ISA_DMA_API)
>       if (asc_dvc_varp->bus_type & ASC_IS_ISA) {
>               seq_printf(m,
>                          " Host ISA DMA speed:   %d MB/S\n",
>                          isa_dma_speed[ASC_EEP_GET_DMA_SPD(ep)]);
>       }
> -#endif /* CONFIG_ISA */
> +#endif /* ISA */
>  }
>  
>  /*
> @@ -8664,7 +8664,7 @@ static unsigned char AscGetChipVersion(PortAddr 
> iop_base,
>       return AscGetChipVerNo(iop_base);
>  }
>  
> -#ifdef CONFIG_ISA
> +#if defined(CONFIG_ISA) && defined(CONFIG_ISA_DMA_API)
>  static void AscEnableIsaDma(uchar dma_channel)
>  {
>       if (dma_channel < 4) {
> @@ -8675,7 +8675,7 @@ static void AscEnableIsaDma(uchar dma_channel)
>               outp(0x00D4, (ushort)(dma_channel - 4));
>       }
>  }
> -#endif /* CONFIG_ISA */
> +#endif /* ISA */
>  
>  static int AscStopQueueExe(PortAddr iop_base)
>  {
> @@ -8704,7 +8704,7 @@ static unsigned int AscGetMaxDmaCount(ushort bus_type)
>       return ASC_MAX_PCI_DMA_COUNT;
>  }
>  
> -#ifdef CONFIG_ISA
> +#if defined(CONFIG_ISA) && defined(CONFIG_ISA_DMA_API)
>  static ushort AscGetIsaDmaChannel(PortAddr iop_base)
>  {
>       ushort channel;
> @@ -8754,7 +8754,7 @@ static uchar AscSetIsaDmaSpeed(PortAddr iop_base, uchar 
> speed_value)
>       AscSetBank(iop_base, 0);
>       return AscGetIsaDmaSpeed(iop_base);
>  }
> -#endif /* CONFIG_ISA */
> +#endif /* ISA */
>  
>  static void AscInitAscDvcVar(ASC_DVC_VAR *asc_dvc)
>  {
> @@ -8821,16 +8821,18 @@ static void AscInitAscDvcVar(ASC_DVC_VAR *asc_dvc)
>       }
>  
>       asc_dvc->cfg->isa_dma_speed = ASC_DEF_ISA_DMA_SPEED;
> -#ifdef CONFIG_ISA
> +#if defined(CONFIG_ISA)
>       if ((asc_dvc->bus_type & ASC_IS_ISA) != 0) {
>               if (chip_version >= ASC_CHIP_MIN_VER_ISA_PNP) {
>                       AscSetChipIFC(iop_base, IFC_INIT_DEFAULT);
>                       asc_dvc->bus_type = ASC_IS_ISAPNP;
>               }
> +#if defined(CONFIG_ISA_DMA_API)
>               asc_dvc->cfg->isa_dma_channel =
>                   (uchar)AscGetIsaDmaChannel(iop_base);
> +#endif /* ISA DMA */
>       }
> -#endif /* CONFIG_ISA */
> +#endif /* ISA */
>       for (i = 0; i <= ASC_MAX_TID; i++) {
>               asc_dvc->cur_dvc_qng[i] = 0;
>               asc_dvc->max_dvc_qng[i] = ASC_MAX_SCSI1_QNG;
> @@ -9377,7 +9379,7 @@ static int AscInitSetConfig(struct pci_dev *pdev, 
> struct Scsi_Host *shost)
>           asc_dvc->cfg->chip_scsi_id) {
>               asc_dvc->err_code |= ASC_IERR_SET_SCSI_ID;
>       }
> -#ifdef CONFIG_ISA
> +#if defined(CONFIG_ISA) && defined(CONFIG_ISA_DMA_API)
>       if (asc_dvc->bus_type & ASC_IS_ISA) {
>               AscSetIsaDmaChannel(iop_base, asc_dvc->cfg->isa_dma_channel);
>               AscSetIsaDmaSpeed(iop_base, asc_dvc->cfg->isa_dma_speed);
> @@ -10986,7 +10988,11 @@ static int advansys_board_found(struct Scsi_Host 
> *shost, unsigned int iop,
>               switch (asc_dvc_varp->bus_type) {
>  #ifdef CONFIG_ISA
>               case ASC_IS_ISA:
> +#if defined(CONFIG_ISA_DMA_API)
>                       shost->unchecked_isa_dma = true;
> +#else
> +                     shost->unchecked_isa_dma = false;
> +#endif
>                       share_irq = 0;
>                       break;
>               case ASC_IS_VL:
> @@ -11292,7 +11298,7 @@ static int advansys_board_found(struct Scsi_Host 
> *shost, unsigned int iop,
>  
>       /* Register DMA Channel for Narrow boards. */
>       shost->dma_channel = NO_ISA_DMA;        /* Default to no ISA DMA. */
> -#ifdef CONFIG_ISA
> +#if defined(CONFIG_ISA) && defined(CONFIG_ISA_DMA_API)
>       if (ASC_NARROW_BOARD(boardp)) {
>               /* Register DMA channel for ISA bus. */
>               if (asc_dvc_varp->bus_type & ASC_IS_ISA) {
> @@ -11379,7 +11385,7 @@ static int advansys_board_found(struct Scsi_Host 
> *shost, unsigned int iop,
>   err_free_irq:
>       free_irq(boardp->irq, shost);
>   err_free_dma:
> -#ifdef CONFIG_ISA
> +#if defined(CONFIG_ISA) && defined(CONFIG_ISA_DMA_API)
>       if (shost->dma_channel != NO_ISA_DMA)
>               free_dma(shost->dma_channel);
>  #endif
> @@ -11403,7 +11409,7 @@ static int advansys_release(struct Scsi_Host *shost)
>       ASC_DBG(1, "begin\n");
>       scsi_remove_host(shost);
>       free_irq(board->irq, shost);
> -#ifdef CONFIG_ISA
> +#if defined(CONFIG_ISA) && defined(CONFIG_ISA_DMA_API)
>       if (shost->dma_channel != NO_ISA_DMA) {
>               ASC_DBG(1, "free_dma()\n");
>               free_dma(shost->dma_channel);

OK, that makes much more of a mess of the driver than I'd anticipated.
Unless there's an actual use case for forcing the non-DMA channel on the
relevant hardware, let's just go with the dependency based fix.

James


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to