diff mbox

[08/10] aacraid: Disable device ID wildcard

Message ID 1448973589-9216-9-git-send-email-RaghavaAditya.Renukunta@pmcs.com (mailing list archive)
State New, archived
Headers show

Commit Message

Raghava Aditya Renukunta Dec. 1, 2015, 12:39 p.m. UTC 
From: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>

Added module parameter that disables device ID wild card binding.

Signed-off-by: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
---
 drivers/scsi/aacraid/aachba.c  | 5 +++++
 drivers/scsi/aacraid/aacraid.h | 1 +
 drivers/scsi/aacraid/linit.c   | 5 +++++
 3 files changed, 11 insertions(+)

Comments

Johannes Thumshirn Dec. 2, 2015, 10:02 a.m. UTC | #1 
On Tue, 2015-12-01 at 04:39 -0800, Raghava Aditya Renukunta wrote:
> From: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
> 
> Added module parameter that disables device ID wild card binding.
> 
> Signed-off-by: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
> ---
>  drivers/scsi/aacraid/aachba.c  | 5 +++++
>  drivers/scsi/aacraid/aacraid.h | 1 +
>  drivers/scsi/aacraid/linit.c   | 5 +++++
>  3 files changed, 11 insertions(+)
> 
> diff --git a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c
> index 06cbab8..87f4f21 100644
> --- a/drivers/scsi/aacraid/aachba.c
> +++ b/drivers/scsi/aacraid/aachba.c
> @@ -315,6 +315,11 @@ MODULE_PARM_DESC(wwn, "Select a WWN type for the
> arrays:\n"
>  	"\t1 - Array Meta Data Signature (default)\n"
>  	"\t2 - Adapter Serial Number");
>  
> +int aac_disable_device_id_wildcards;
> +module_param_named(disable_device_id_wildcards,
> +	aac_disable_device_id_wildcards, int, S_IRUGO | S_IWUSR);
> +MODULE_PARM_DESC(disable_device_id_wildcards,
> +	"Disable device ID wildcards");
>  
>  static inline int aac_valid_context(struct scsi_cmnd *scsicmd,
>  		struct fib *fibptr) {
> diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
> index 594de5f..7708a2c 100644
> --- a/drivers/scsi/aacraid/aacraid.h
> +++ b/drivers/scsi/aacraid/aacraid.h
> @@ -2177,3 +2177,4 @@ extern int aac_commit;
>  extern int update_interval;
>  extern int check_interval;
>  extern int aac_check_reset;
> +extern int aac_disable_device_id_wildcards;
> diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
> index 3a4dbe7..2094842 100644
> --- a/drivers/scsi/aacraid/linit.c
> +++ b/drivers/scsi/aacraid/linit.c
> @@ -1135,6 +1135,11 @@ static int aac_probe_one(struct pci_dev *pdev, const
> struct pci_device_id *id)
>  	u64 dmamask;
>  	extern int aac_sync_mode;
>  
> +	if (aac_disable_device_id_wildcards &&
> +		id->subvendor == PCI_ANY_ID &&
> +		id->subdevice == PCI_ANY_ID)
> +		return -ENODEV;
> +
>  	/*
>  	 * Only series 7 needs freset.
>  	 */

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.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
Tomas Henzl Dec. 3, 2015, 3:54 p.m. UTC | #2 
On 1.12.2015 13:39, Raghava Aditya Renukunta wrote:
> From: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
>
> Added module parameter that disables device ID wild card binding.

What is the use case for this?

It looks like something which could be solved in the modprobe config,
without a module option.

Cheers,
Tomas

>
> Signed-off-by: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
> ---
>  drivers/scsi/aacraid/aachba.c  | 5 +++++
>  drivers/scsi/aacraid/aacraid.h | 1 +
>  drivers/scsi/aacraid/linit.c   | 5 +++++
>  3 files changed, 11 insertions(+)
>
> diff --git a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c
> index 06cbab8..87f4f21 100644
> --- a/drivers/scsi/aacraid/aachba.c
> +++ b/drivers/scsi/aacraid/aachba.c
> @@ -315,6 +315,11 @@ MODULE_PARM_DESC(wwn, "Select a WWN type for the arrays:\n"
>  	"\t1 - Array Meta Data Signature (default)\n"
>  	"\t2 - Adapter Serial Number");
>  
> +int aac_disable_device_id_wildcards;
> +module_param_named(disable_device_id_wildcards,
> +	aac_disable_device_id_wildcards, int, S_IRUGO | S_IWUSR);
> +MODULE_PARM_DESC(disable_device_id_wildcards,
> +	"Disable device ID wildcards");
>  
>  static inline int aac_valid_context(struct scsi_cmnd *scsicmd,
>  		struct fib *fibptr) {
> diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
> index 594de5f..7708a2c 100644
> --- a/drivers/scsi/aacraid/aacraid.h
> +++ b/drivers/scsi/aacraid/aacraid.h
> @@ -2177,3 +2177,4 @@ extern int aac_commit;
>  extern int update_interval;
>  extern int check_interval;
>  extern int aac_check_reset;
> +extern int aac_disable_device_id_wildcards;
> diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
> index 3a4dbe7..2094842 100644
> --- a/drivers/scsi/aacraid/linit.c
> +++ b/drivers/scsi/aacraid/linit.c
> @@ -1135,6 +1135,11 @@ static int aac_probe_one(struct pci_dev *pdev, const struct pci_device_id *id)
>  	u64 dmamask;
>  	extern int aac_sync_mode;
>  
> +	if (aac_disable_device_id_wildcards &&
> +		id->subvendor == PCI_ANY_ID &&
> +		id->subdevice == PCI_ANY_ID)
> +		return -ENODEV;
> +
>  	/*
>  	 * Only series 7 needs freset.
>  	 */

--
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
Raghava Aditya Renukunta Dec. 3, 2015, 9:32 p.m. UTC | #3 
Hello Tomas,


> -----Original Message-----
> From: Tomas Henzl [mailto:thenzl@redhat.com]
> Sent: Thursday, December 3, 2015 7:55 AM
> To: Raghava Aditya Renukunta; JBottomley@Parallels.com; linux-
> scsi@vger.kernel.org
> Cc: Mahesh Rajashekhara; Murthy Bhat; Santosh Akula; Gana Sridaran;
> aacraid@pmc-sierra.com; Rich Bono
> Subject: Re: [PATCH 08/10] aacraid: Disable device ID wildcard
> 
> On 1.12.2015 13:39, Raghava Aditya Renukunta wrote:
> > From: Raghava Aditya Renukunta <raghavaaditya.renukunta@pmcs.com>
> >
> > Added module parameter that disables device ID wild card binding.
> 
> What is the use case for this?
> 
> It looks like something which could be solved in the modprobe config,
> without a module option.

This will enable us to prevent aacraid from loading for PCI devices that match
device ID wildcards. Enabling us to use say a new driver for future devices.

> Cheers,
> Tomas
> 
> >
> > Signed-off-by: Raghava Aditya Renukunta
> <raghavaaditya.renukunta@pmcs.com>
> > ---
> >  drivers/scsi/aacraid/aachba.c  | 5 +++++
> >  drivers/scsi/aacraid/aacraid.h | 1 +
> >  drivers/scsi/aacraid/linit.c   | 5 +++++
> >  3 files changed, 11 insertions(+)
> >
> > diff --git a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c
> > index 06cbab8..87f4f21 100644
> > --- a/drivers/scsi/aacraid/aachba.c
> > +++ b/drivers/scsi/aacraid/aachba.c
> > @@ -315,6 +315,11 @@ MODULE_PARM_DESC(wwn, "Select a WWN type
> for the arrays:\n"
> >  	"\t1 - Array Meta Data Signature (default)\n"
> >  	"\t2 - Adapter Serial Number");
> >
> > +int aac_disable_device_id_wildcards;
> > +module_param_named(disable_device_id_wildcards,
> > +	aac_disable_device_id_wildcards, int, S_IRUGO | S_IWUSR);
> > +MODULE_PARM_DESC(disable_device_id_wildcards,
> > +	"Disable device ID wildcards");
> >
> >  static inline int aac_valid_context(struct scsi_cmnd *scsicmd,
> >  		struct fib *fibptr) {
> > diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
> > index 594de5f..7708a2c 100644
> > --- a/drivers/scsi/aacraid/aacraid.h
> > +++ b/drivers/scsi/aacraid/aacraid.h
> > @@ -2177,3 +2177,4 @@ extern int aac_commit;
> >  extern int update_interval;
> >  extern int check_interval;
> >  extern int aac_check_reset;
> > +extern int aac_disable_device_id_wildcards;
> > diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
> > index 3a4dbe7..2094842 100644
> > --- a/drivers/scsi/aacraid/linit.c
> > +++ b/drivers/scsi/aacraid/linit.c
> > @@ -1135,6 +1135,11 @@ static int aac_probe_one(struct pci_dev *pdev,
> const struct pci_device_id *id)
> >  	u64 dmamask;
> >  	extern int aac_sync_mode;
> >
> > +	if (aac_disable_device_id_wildcards &&
> > +		id->subvendor == PCI_ANY_ID &&
> > +		id->subdevice == PCI_ANY_ID)
> > +		return -ENODEV;
> > +
> >  	/*
> >  	 * Only series 7 needs freset.
> >  	 */

Regards,
Raghava Aditya
--
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
Christoph Hellwig Dec. 4, 2015, 8:33 a.m. UTC | #4 
On Thu, Dec 03, 2015 at 09:32:18PM +0000, Raghava Aditya Renukunta wrote:
> This will enable us to prevent aacraid from loading for PCI devices that match
> device ID wildcards. Enabling us to use say a new driver for future devices.

This looks like a bogus reason.  The same PCI ID should always be
compatible and mathed by the same driver.  Even if you add a new driver
to expose additional feature and break these semantics there is no point
to do a) reject them conditionally on a module option and b) do this
before said driver is merged.
--
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
Raghava Aditya Renukunta Dec. 7, 2015, 7:07 p.m. UTC | #5 
Hello Christoph,

> -----Original Message-----
> From: Christoph Hellwig [mailto:hch@infradead.org]
> Sent: Friday, December 4, 2015 12:33 AM
> To: Raghava Aditya Renukunta
> Cc: Tomas Henzl; JBottomley@Parallels.com; linux-scsi@vger.kernel.org;
> Mahesh Rajashekhara; Murthy Bhat; Santosh Akula; Gana Sridaran;
> aacraid@pmc-sierra.com; Rich Bono
> Subject: Re: [PATCH 08/10] aacraid: Disable device ID wildcard
> 
> On Thu, Dec 03, 2015 at 09:32:18PM +0000, Raghava Aditya Renukunta wrote:
> > This will enable us to prevent aacraid from loading for PCI devices that
> match
> > device ID wildcards. Enabling us to use say a new driver for future devices.
> 
> This looks like a bogus reason.  The same PCI ID should always be
> compatible and mathed by the same driver.  Even if you add a new driver
> to expose additional feature and break these semantics there is no point
> to do a) reject them conditionally on a module option and b) do this
> before said driver is merged.

I have spoken with my team and  it does make more sense to submit
it when the new driver is merged. I will withdraw this patch.

Regards,
Raghava Aditya
--
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/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c
index 06cbab8..87f4f21 100644
--- a/drivers/scsi/aacraid/aachba.c
+++ b/drivers/scsi/aacraid/aachba.c
@@ -315,6 +315,11 @@  MODULE_PARM_DESC(wwn, "Select a WWN type for the arrays:\n"
 	"\t1 - Array Meta Data Signature (default)\n"
 	"\t2 - Adapter Serial Number");
 
+int aac_disable_device_id_wildcards;
+module_param_named(disable_device_id_wildcards,
+	aac_disable_device_id_wildcards, int, S_IRUGO | S_IWUSR);
+MODULE_PARM_DESC(disable_device_id_wildcards,
+	"Disable device ID wildcards");
 
 static inline int aac_valid_context(struct scsi_cmnd *scsicmd,
 		struct fib *fibptr) {
diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
index 594de5f..7708a2c 100644
--- a/drivers/scsi/aacraid/aacraid.h
+++ b/drivers/scsi/aacraid/aacraid.h
@@ -2177,3 +2177,4 @@  extern int aac_commit;
 extern int update_interval;
 extern int check_interval;
 extern int aac_check_reset;
+extern int aac_disable_device_id_wildcards;
diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
index 3a4dbe7..2094842 100644
--- a/drivers/scsi/aacraid/linit.c
+++ b/drivers/scsi/aacraid/linit.c
@@ -1135,6 +1135,11 @@  static int aac_probe_one(struct pci_dev *pdev, const struct pci_device_id *id)
 	u64 dmamask;
 	extern int aac_sync_mode;
 
+	if (aac_disable_device_id_wildcards &&
+		id->subvendor == PCI_ANY_ID &&
+		id->subdevice == PCI_ANY_ID)
+		return -ENODEV;
+
 	/*
 	 * Only series 7 needs freset.
 	 */