diff mbox

scsi: advansys needs ISA dma api for ISA support

Message ID 5081546.QjQ32dGX2L@wuerfel (mailing list archive)
State New, archived
Headers show

Commit Message

Arnd Bergmann Oct. 12, 2015, 3:10 p.m. UTC 
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 adds the missing dependency.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>


--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

James Bottomley Oct. 12, 2015, 3:28 p.m. UTC | #1 
On Mon, 2015-10-12 at 17:10 +0200, Arnd Bergmann wrote:
> 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 adds the missing dependency.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> 
> 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()?

If so I think we leave the depends alone and try to bring the board up
in NO_ISA_DMA mode.  That means the narrowboard check should be gated by
CONFIG_ISA_DMA_API ... do we also have to gate free_dma as well?

James


--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hannes Reinecke Oct. 13, 2015, 5:47 a.m. UTC | #2 
On 10/12/2015 05:10 PM, Arnd Bergmann wrote:
> 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 adds the missing dependency.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> 
> 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
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
Hannes Reinecke Oct. 15, 2015, 12:06 p.m. UTC | #3 
On 10/12/2015 05:10 PM, Arnd Bergmann wrote:
> 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 adds the missing dependency.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> 
> 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
> 
Sorry to chime in again, but wouldn't this allow to build on platforms
which have neither ISA_DMA_API nor ISA, like oldish sparc systems with
proprietary S-BUS?

Why not ISA_DMA_API || EISA || PCI ?

Cheers,

Hannes
Hannes Reinecke Oct. 15, 2015, 12:09 p.m. UTC | #4 
On 10/15/2015 02:06 PM, Hannes Reinecke wrote:
> On 10/12/2015 05:10 PM, Arnd Bergmann wrote:
>> 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 adds the missing dependency.
>>
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>>
>> 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
>>
> Sorry to chime in again, but wouldn't this allow to build on platforms
> which have neither ISA_DMA_API nor ISA, like oldish sparc systems with
> proprietary S-BUS?
> 
> Why not ISA_DMA_API || EISA || PCI ?
> 
Gnaa. Ignore this.

Mailer got confused about which mails are more recent than others :-(

Cheers,

Hannes
Arnd Bergmann Oct. 15, 2015, 3:04 p.m. UTC | #5 
On Thursday 15 October 2015 14:09:31 Hannes Reinecke wrote:
> On 10/15/2015 02:06 PM, Hannes Reinecke wrote:
> > On 10/12/2015 05:10 PM, Arnd Bergmann 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
> >>
> > Sorry to chime in again, but wouldn't this allow to build on platforms
> > which have neither ISA_DMA_API nor ISA, like oldish sparc systems with
> > proprietary S-BUS?

No, the first dependency is still in place. It could also be written as

depends on (ISA && ISA_DMA_API) || EISA || PCI

which might be easier to understand. Do you want me to resubmit it like that?

> > Why not ISA_DMA_API || EISA || PCI ?

That would indeed make it possible to enable it on ancient systems that
have none of ISA, EISA or PCI but still support ISA_DMA_API, e.g. for
a floppy controller.

A theoretical downside is that a system with a PCI advansys adapter
and ISA slots but no ISA_DMA_API would not be able to build this.
As far as I know, such a system does not exist.

> Gnaa. Ignore this.
> 
> Mailer got confused about which mails are more recent than others 

Just to make sure we are on the same page: James thought my second patch
was making things too ugly and wanted to get back to this one.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hannes Reinecke Oct. 16, 2015, 11:49 a.m. UTC | #6 
On 10/15/2015 05:04 PM, Arnd Bergmann wrote:
> On Thursday 15 October 2015 14:09:31 Hannes Reinecke wrote:
>> On 10/15/2015 02:06 PM, Hannes Reinecke wrote:
>>> On 10/12/2015 05:10 PM, Arnd Bergmann 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
>>>>
>>> Sorry to chime in again, but wouldn't this allow to build on platforms
>>> which have neither ISA_DMA_API nor ISA, like oldish sparc systems with
>>> proprietary S-BUS?
> 
> No, the first dependency is still in place. It could also be written as
> 
> depends on (ISA && ISA_DMA_API) || EISA || PCI
> 
> which might be easier to understand. Do you want me to resubmit it like that?
> 
Yes, please do so.

Cheers,

Hannes
Arnd Bergmann Oct. 16, 2015, 12:04 p.m. UTC | #7 
On Friday 16 October 2015 13:49:06 Hannes Reinecke wrote:
> On 10/15/2015 05:04 PM, Arnd Bergmann wrote:
> > On Thursday 15 October 2015 14:09:31 Hannes Reinecke wrote:
> >> On 10/15/2015 02:06 PM, Hannes Reinecke wrote:
> >>> On 10/12/2015 05:10 PM, Arnd Bergmann 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
> >>>>
> >>> Sorry to chime in again, but wouldn't this allow to build on platforms
> >>> which have neither ISA_DMA_API nor ISA, like oldish sparc systems with
> >>> proprietary S-BUS?
> > 
> > No, the first dependency is still in place. It could also be written as
> > 
> > depends on (ISA && ISA_DMA_API) || EISA || PCI
> > 
> > which might be easier to understand. Do you want me to resubmit it like that?
> > 
> Yes, please do so.
> 

I now realized that they are not the same: the new version allows building
on systems with PCI and ISA but no ISA_DMA_API, which the driver should be
able to handle but doesn't (unless my second patch is applied).

However, I don't think that this combination exists, so we should still
be good, and it does express what the hardware can do. I'll send it after
my build regression tests are complete.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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