2.6.13-rc3-mm1: horribly drivers/scsi/qla2xxx/Makefile

2.6.13-rc3-mm1: horribly drivers/scsi/qla2xxx/Makefile

[Adrian Bunk]

On Fri, Jul 15, 2005 at 01:36:53AM -0700, Andrew Morton wrote:
>...
> Changes since 2.6.13-rc2-mm2:
>...
>  git-scsi-misc.patch
>...
>  Subsystem trees
>...

--- linux-2.6.13-rc3/drivers/scsi/qla2xxx/Makefile      2005-06-17 16:04:01.000000000 -0700
+++ devel/drivers/scsi/qla2xxx/Makefile 2005-07-15 00:46:18.000000000 -0700
@@ -1,4 +1,6 @@
 EXTRA_CFLAGS += -DUNIQUE_FW_NAME
+CONFIG_SCSI_QLA24XX=m
+EXTRA_CFLAGS += -DCONFIG_SCSI_QLA24XX -DCONFIG_SCSI_QLA24XX_MODULE
 
 qla2xxx-y := qla_os.o qla_init.o qla_mbx.o qla_iocb.o qla_isr.o qla_gs.o \
                qla_dbg.o qla_sup.o qla_rscn.o qla_attr.o
@@ -14,3 +16,4 @@ obj-$(CONFIG_SCSI_QLA22XX) += qla2xxx.o 
 obj-$(CONFIG_SCSI_QLA2300) += qla2xxx.o qla2300.o
 obj-$(CONFIG_SCSI_QLA2322) += qla2xxx.o qla2322.o
 obj-$(CONFIG_SCSI_QLA6312) += qla2xxx.o qla6312.o
+obj-$(CONFIG_SCSI_QLA24XX) += qla2xxx.o


I don't know what exactly you want to achieve, but this is so horribly 
wrong.


cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: 2.6.13-rc3-mm1: horribly drivers/scsi/qla2xxx/Makefile

[Andrew Vasquez]

On Fri, 15 Jul 2005, Adrian Bunk wrote:

> On Fri, Jul 15, 2005 at 01:36:53AM -0700, Andrew Morton wrote:
> >...
> > Changes since 2.6.13-rc2-mm2:
> >...
> >  git-scsi-misc.patch
> >...
> >  Subsystem trees
> >...
> 
...
> +obj-$(CONFIG_SCSI_QLA24XX) += qla2xxx.o
> 
> 
> I don't know what exactly you want to achieve, but this is so horribly 
> wrong.


Yes, quite.  How about the following to correct the intention.



Add correct Kconfig option for ISP24xx support.

Signed-off-by: Andrew Vasquez <andrew.vasquez@qlogic.com>
---

diff --git a/drivers/scsi/qla2xxx/Kconfig b/drivers/scsi/qla2xxx/Kconfig
--- a/drivers/scsi/qla2xxx/Kconfig
+++ b/drivers/scsi/qla2xxx/Kconfig
@@ -39,3 +39,11 @@ config SCSI_QLA6312
 	---help---
 	This driver supports the QLogic 63xx (ISP6312 and ISP6322) host
 	adapter family.
+
+config SCSI_QLA24XX
+	tristate "QLogic ISP24xx host adapter family support"
+	depends on SCSI_QLA2XXX
+        select SCSI_FC_ATTRS
+	---help---
+	This driver supports the QLogic 24xx (ISP2422 and ISP2432) host
+	adapter family.
diff --git a/drivers/scsi/qla2xxx/Makefile b/drivers/scsi/qla2xxx/Makefile
--- a/drivers/scsi/qla2xxx/Makefile
+++ b/drivers/scsi/qla2xxx/Makefile
@@ -1,6 +1,4 @@
 EXTRA_CFLAGS += -DUNIQUE_FW_NAME
-CONFIG_SCSI_QLA24XX=m
-EXTRA_CFLAGS += -DCONFIG_SCSI_QLA24XX -DCONFIG_SCSI_QLA24XX_MODULE
 
 qla2xxx-y := qla_os.o qla_init.o qla_mbx.o qla_iocb.o qla_isr.o qla_gs.o \
 		qla_dbg.o qla_sup.o qla_rscn.o qla_attr.o

Re: 2.6.13-rc3-mm1: horribly drivers/scsi/qla2xxx/Makefile

[Jindrich Makovicka]

Andrew Vasquez wrote:
> Yes, quite.  How about the following to correct the intention.
> 
> 
> 
> Add correct Kconfig option for ISP24xx support.
> 
> Signed-off-by: Andrew Vasquez <andrew.vasquez@qlogic.com>
> ---
> 
> diff --git a/drivers/scsi/qla2xxx/Kconfig b/drivers/scsi/qla2xxx/Kconfig
> --- a/drivers/scsi/qla2xxx/Kconfig
> +++ b/drivers/scsi/qla2xxx/Kconfig
> @@ -39,3 +39,11 @@ config SCSI_QLA6312
>  	---help---
>  	This driver supports the QLogic 63xx (ISP6312 and ISP6322) host
>  	adapter family.
> +
> +config SCSI_QLA24XX
> +	tristate "QLogic ISP24xx host adapter family support"
> +	depends on SCSI_QLA2XXX
> +        select SCSI_FC_ATTRS

there should be also "select FW_LOADER", as it uses request_firmware &
release_firmware

> +	---help---
> +	This driver supports the QLogic 24xx (ISP2422 and ISP2432) host
> +	adapter family.

-- 
Jindrich Makovicka

[2.6 patch] SCSI_QLA2ABC mustn't select SCSI_FC_ATTRS

[Adrian Bunk]

[ The subject was adapted to linux-kernel spam filters... ]

On Fri, Jul 15, 2005 at 07:40:37AM -0700, Andrew Vasquez wrote:
> On Fri, 15 Jul 2005, Adrian Bunk wrote:
> 
> > On Fri, Jul 15, 2005 at 01:36:53AM -0700, Andrew Morton wrote:
> > >...
> > > Changes since 2.6.13-rc2-mm2:
> > >...
> > >  git-scsi-misc.patch
> > >...
> > >  Subsystem trees
> > >...
> > 
> ...
> > +obj-$(CONFIG_SCSI_QLA24XX) += qla2xxx.o
> > 
> > 
> > I don't know what exactly you want to achieve, but this is so horribly 
> > wrong.
> 
> 
> Yes, quite.  How about the following to correct the intention.
>...

It looks good (except that you used spaces instead of a tab in the 
"select" line, but that's only a minor nitpick).

Below is another fix for a different issue that was already present.

cu
Adrian


<--  snip  -->


SCSI_QLA2XXX is automatically enabled for (SCSI && PCI).
It therefore mustn't select SCSI_FC_ATTRS, since it otherwise 
unconditionally enables SCSI_FC_ATTRS for all users with
(SCSI && PCI) enabled, even when they don't need any support for
QLogic hardware.

This patch also does a cosmetic change for making the "default" look 
more like in other kernel code.

Signed-off-by: Adrian Bunk <bunk@stusta.de>

--- linux-2.6.13-rc3-mm1-full/drivers/scsi/qla2xxx/Kconfig.old	2005-07-15 22:05:19.000000000 +0200
+++ linux-2.6.13-rc3-mm1-full/drivers/scsi/qla2xxx/Kconfig	2005-07-15 22:07:42.000000000 +0200
@@ -1,8 +1,7 @@
 config SCSI_QLA2XXX
 	tristate
-	default (SCSI && PCI)
 	depends on SCSI && PCI
-	select SCSI_FC_ATTRS
+	default y
 
 config SCSI_QLA21XX
 	tristate "QLogic ISP2100 host adapter family support"

Re: [2.6 patch] SCSI_QLA2ABC mustn't select SCSI_FC_ATTRS

[Lee Revell]

On Sun, 2005-07-17 at 04:38 +0200, Adrian Bunk wrote:
> SCSI_QLA2XXX is automatically enabled for (SCSI && PCI).

This has bugged me for a while.  Why does this one SCSI driver default
to Y in the first place?

Lee

Re: [2.6 patch] SCSI_QLA2ABC mustn't select SCSI_FC_ATTRS

[randy_dunlap]

On Sat, 16 Jul 2005 23:11:26 -0400 Lee Revell wrote:

> On Sun, 2005-07-17 at 04:38 +0200, Adrian Bunk wrote:
> > SCSI_QLA2XXX is automatically enabled for (SCSI && PCI).
> 
> This has bugged me for a while.  Why does this one SCSI driver default
> to Y in the first place?

It's not a driver, it's a subdirectory.

---
~Randy

Re: [2.6 patch] SCSI_QLA2ABC mustn't select SCSI_FC_ATTRS

[Lee Revell]

On Sat, 2005-07-16 at 21:04 -0700, randy_dunlap wrote:
> On Sat, 16 Jul 2005 23:11:26 -0400 Lee Revell wrote:
> 
> > On Sun, 2005-07-17 at 04:38 +0200, Adrian Bunk wrote:
> > > SCSI_QLA2XXX is automatically enabled for (SCSI && PCI).
> > 
> > This has bugged me for a while.  Why does this one SCSI driver default
> > to Y in the first place?
> 
> It's not a driver, it's a subdirectory.

Ah, ok.  Thanks.

Lee

[-mm patch] SCSI_QLA2ABC options must select FW_LOADER

[Adrian Bunk]

[ The subject was adapted to linux-kernel spam filters... ]

On Sat, Jul 16, 2005 at 07:26:44PM +0200, Jindrich Makovicka wrote:
> Andrew Vasquez wrote:
> > Yes, quite.  How about the following to correct the intention.
> > 
> > 
> > 
> > Add correct Kconfig option for ISP24xx support.
> > 
> > Signed-off-by: Andrew Vasquez <andrew.vasquez@qlogic.com>
> > ---
> > 
> > diff --git a/drivers/scsi/qla2xxx/Kconfig b/drivers/scsi/qla2xxx/Kconfig
> > --- a/drivers/scsi/qla2xxx/Kconfig
> > +++ b/drivers/scsi/qla2xxx/Kconfig
> > @@ -39,3 +39,11 @@ config SCSI_QLA6312
> >  	---help---
> >  	This driver supports the QLogic 63xx (ISP6312 and ISP6322) host
> >  	adapter family.
> > +
> > +config SCSI_QLA24XX
> > +	tristate "QLogic ISP24xx host adapter family support"
> > +	depends on SCSI_QLA2XXX
> > +        select SCSI_FC_ATTRS
> 
> there should be also "select FW_LOADER", as it uses request_firmware &
> release_firmware
>...

You are right, patch below.

> Jindrich Makovicka

cu
Adrian


<--  snip  -->


qla_init.c now uses code that requires FW_LOADER.

Additionally, this patch removes spaces instead of tabs at the 
SCSI_FC_ATTRS selects.

Signed-off-by: Adrian Bunk <bunk@stusta.de>

--- linux-2.6.13-rc3-mm1-full/drivers/scsi/qla2xxx/Kconfig.old	2005-07-17 15:44:26.000000000 +0200
+++ linux-2.6.13-rc3-mm1-full/drivers/scsi/qla2xxx/Kconfig	2005-07-17 15:45:45.000000000 +0200
@@ -1,49 +1,55 @@
 config SCSI_QLA2XXX
 	tristate
 	depends on SCSI && PCI
 	default y
 
 config SCSI_QLA21XX
 	tristate "QLogic ISP2100 host adapter family support"
 	depends on SCSI_QLA2XXX
-        select SCSI_FC_ATTRS
+	select SCSI_FC_ATTRS
+	select FW_LOADER
 	---help---
 	This driver supports the QLogic 21xx (ISP2100) host adapter family.
 
 config SCSI_QLA22XX
 	tristate "QLogic ISP2200 host adapter family support"
 	depends on SCSI_QLA2XXX
-        select SCSI_FC_ATTRS
+	select SCSI_FC_ATTRS
+	select FW_LOADER
 	---help---
 	This driver supports the QLogic 22xx (ISP2200) host adapter family.
 
 config SCSI_QLA2300
 	tristate "QLogic ISP2300 host adapter family support"
 	depends on SCSI_QLA2XXX
-        select SCSI_FC_ATTRS
+	select SCSI_FC_ATTRS
+	select FW_LOADER
 	---help---
 	This driver supports the QLogic 2300 (ISP2300 and ISP2312) host
 	adapter family.
 
 config SCSI_QLA2322
 	tristate "QLogic ISP2322 host adapter family support"
 	depends on SCSI_QLA2XXX
-        select SCSI_FC_ATTRS
+	select SCSI_FC_ATTRS
+	select FW_LOADER
 	---help---
 	This driver supports the QLogic 2322 (ISP2322) host adapter family.
 
 config SCSI_QLA6312
 	tristate "QLogic ISP63xx host adapter family support"
 	depends on SCSI_QLA2XXX
-        select SCSI_FC_ATTRS
+	select SCSI_FC_ATTRS
+	select FW_LOADER
 	---help---
 	This driver supports the QLogic 63xx (ISP6312 and ISP6322) host
 	adapter family.
 
 config SCSI_QLA24XX
 	tristate "QLogic ISP24xx host adapter family support"
 	depends on SCSI_QLA2XXX
-        select SCSI_FC_ATTRS
+	select SCSI_FC_ATTRS
+	select FW_LOADER
 	---help---
 	This driver supports the QLogic 24xx (ISP2422 and ISP2432) host
 	adapter family.

Re: [-mm patch] SCSI_QLA2ABC options must select FW_LOADER

[Jesper Juhl]

On 7/19/05, Adrian Bunk <bunk@stusta.de> wrote:
> [ The subject was adapted to linux-kernel spam filters... ]
> 
> On Sat, Jul 16, 2005 at 07:26:44PM +0200, Jindrich Makovicka wrote:
> > Andrew Vasquez wrote:
> > > Yes, quite.  How about the following to correct the intention.
> > >
> > >
> > >
> > > Add correct Kconfig option for ISP24xx support.
> > >
> > > Signed-off-by: Andrew Vasquez <andrew.vasquez@qlogic.com>
> > > ---
> > >
> > > diff --git a/drivers/scsi/qla2xxx/Kconfig b/drivers/scsi/qla2xxx/Kconfig
> > > --- a/drivers/scsi/qla2xxx/Kconfig
> > > +++ b/drivers/scsi/qla2xxx/Kconfig
> > > @@ -39,3 +39,11 @@ config SCSI_QLA6312
> > >     ---help---
> > >     This driver supports the QLogic 63xx (ISP6312 and ISP6322) host
> > >     adapter family.
> > > +
> > > +config SCSI_QLA24XX
> > > +   tristate "QLogic ISP24xx host adapter family support"
> > > +   depends on SCSI_QLA2XXX
> > > +        select SCSI_FC_ATTRS
> >
> > there should be also "select FW_LOADER", as it uses request_firmware &
> > release_firmware
> >...
> 
> You are right, patch below.
> 
> > Jindrich Makovicka
> 
> cu
> Adrian
> 
> 
> <--  snip  -->
> 
> 
> qla_init.c now uses code that requires FW_LOADER.
> 
> Additionally, this patch removes spaces instead of tabs at the
> SCSI_FC_ATTRS selects.
> 
> Signed-off-by: Adrian Bunk <bunk@stusta.de>
> 
> --- linux-2.6.13-rc3-mm1-full/drivers/scsi/qla2xxx/Kconfig.old  2005-07-17 15:44:26.000000000 +0200
> +++ linux-2.6.13-rc3-mm1-full/drivers/scsi/qla2xxx/Kconfig      2005-07-17 15:45:45.000000000 +0200
> @@ -1,49 +1,55 @@
>  config SCSI_QLA2XXX
>         tristate
>         depends on SCSI && PCI
>         default y
> 
>  config SCSI_QLA21XX
>         tristate "QLogic ISP2100 host adapter family support"
>         depends on SCSI_QLA2XXX
> -        select SCSI_FC_ATTRS
> +       select SCSI_FC_ATTRS
> +       select FW_LOADER
>         ---help---
>         This driver supports the QLogic 21xx (ISP2100) host adapter family.
> 
>  config SCSI_QLA22XX
>         tristate "QLogic ISP2200 host adapter family support"
>         depends on SCSI_QLA2XXX
> -        select SCSI_FC_ATTRS
> +       select SCSI_FC_ATTRS
> +       select FW_LOADER
>         ---help---
>         This driver supports the QLogic 22xx (ISP2200) host adapter family.
> 
>  config SCSI_QLA2300
>         tristate "QLogic ISP2300 host adapter family support"
>         depends on SCSI_QLA2XXX
> -        select SCSI_FC_ATTRS
> +       select SCSI_FC_ATTRS
> +       select FW_LOADER
>         ---help---
>         This driver supports the QLogic 2300 (ISP2300 and ISP2312) host
>         adapter family.
> 
>  config SCSI_QLA2322
>         tristate "QLogic ISP2322 host adapter family support"
>         depends on SCSI_QLA2XXX
> -        select SCSI_FC_ATTRS
> +       select SCSI_FC_ATTRS
> +       select FW_LOADER
>         ---help---
>         This driver supports the QLogic 2322 (ISP2322) host adapter family.
> 
>  config SCSI_QLA6312
>         tristate "QLogic ISP63xx host adapter family support"
>         depends on SCSI_QLA2XXX
> -        select SCSI_FC_ATTRS
> +       select SCSI_FC_ATTRS
> +       select FW_LOADER
>         ---help---
>         This driver supports the QLogic 63xx (ISP6312 and ISP6322) host
>         adapter family.
> 
>  config SCSI_QLA24XX
>         tristate "QLogic ISP24xx host adapter family support"
>         depends on SCSI_QLA2XXX
> -        select SCSI_FC_ATTRS
> +       select SCSI_FC_ATTRS
> +       select FW_LOADER
>         ---help---
>         This driver supports the QLogic 24xx (ISP2422 and ISP2432) host
>         adapter family.

I send a patch for this yesterday that lets SCSI_QLA2XXX select
FW_LOADER. I believe that's a bit better since the other options
depend on SCSI_QLA2XXX anyway, there's no point in having them all set
FW_LOADER. My patch also fixes another little issue; that you cannot
disable SCSI_QLA2XXX if you don't need it.

See the patch here: http://lkml.org/lkml/2005/7/19/147
The mail contains 3 patches, but the third one is the best fix IMHO.

-- 
Jesper Juhl <jesper.juhl@gmail.com>
Don't top-post  http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please      http://www.expita.com/nomime.html

Re: [-mm patch] SCSI_QLA2ABC options must select FW_LOADER

[Adrian Bunk]

On Wed, Jul 20, 2005 at 03:38:02PM +0200, Jesper Juhl wrote:
>...
> I send a patch for this yesterday that lets SCSI_QLA2XXX select
> FW_LOADER. I believe that's a bit better since the other options
> depend on SCSI_QLA2XXX anyway, there's no point in having them all set
> FW_LOADER. My patch also fixes another little issue; that you cannot
> disable SCSI_QLA2XXX if you don't need it.
>...

That's not an issue, this seems to be intentional.

Whether SCSI_QLA2XXX should be user-visible (as your patches make it) or 
stay as it is (with the fixes from my patches) doesn't matter much - 
both are valid setups.

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed