[PATCH 1/1 v3] PCI: Device specific reset function

[PATCH 1/1 v3] PCI: Device specific reset function

[Tadeusz Struk]

---
 drivers/pci/pci.h    |    1 +
 drivers/pci/quirks.c |   33 +++++++++++++++++++++++++++------
 include/linux/pci.h  |    1 +
 3 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 1009a5e..4d10479 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -315,6 +315,7 @@ struct pci_dev_reset_methods {
 	u16 vendor;
 	u16 device;
 	int (*reset)(struct pci_dev *dev, int probe);
+	struct list_head list;
 };
 
 #ifdef CONFIG_PCI_QUIRKS
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 6476547..f423d2f 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -3070,26 +3070,47 @@ static int reset_intel_82599_sfp_virtfn(struct pci_dev *dev, int probe)
 }
 
 #define PCI_DEVICE_ID_INTEL_82599_SFP_VF   0x10ed
-
-static const struct pci_dev_reset_methods pci_dev_reset_methods[] = {
+static struct pci_dev_reset_methods pci_dev_reset_methods[] = {
 	{ PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82599_SFP_VF,
-		 reset_intel_82599_sfp_virtfn },
+		reset_intel_82599_sfp_virtfn },
 	{ PCI_VENDOR_ID_INTEL, PCI_ANY_ID,
 		reset_intel_generic_dev },
-	{ 0 }
 };
 
+static LIST_HEAD(reset_list);
+
+void pci_dev_specific_reset_add(struct pci_dev_reset_methods *reset_method)
+{
+	INIT_LIST_HEAD(&reset_method->list);
+	list_add(&reset_method->list, &reset_list);
+}
+
+static int __init pci_dev_specific_reset_init(void)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(pci_dev_reset_methods); i++) {
+		pci_dev_specific_reset_add(&pci_dev_reset_methods[i]);
+	}
+	return 0;
+}
+
+late_initcall(pci_dev_specific_reset_init);
+
 int pci_dev_specific_reset(struct pci_dev *dev, int probe)
 {
 	const struct pci_dev_reset_methods *i;
+	struct pci_driver *drv = dev->driver;
+
+	if (drv && drv->reset)
+		return drv->reset(dev, probe);
 
-	for (i = pci_dev_reset_methods; i->reset; i++) {
+	list_for_each_entry(i, &reset_list, list) {
 		if ((i->vendor == dev->vendor ||
 		     i->vendor == (u16)PCI_ANY_ID) &&
 		    (i->device == dev->device ||
 		     i->device == (u16)PCI_ANY_ID))
 			return i->reset(dev, probe);
 	}
-
 	return -ENOTTY;
 }
diff --git a/include/linux/pci.h b/include/linux/pci.h
index a16b1df..a3a0bc5 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -560,6 +560,7 @@ struct pci_driver {
 	int  (*resume_early) (struct pci_dev *dev);
 	int  (*resume) (struct pci_dev *dev);	                /* Device woken up */
 	void (*shutdown) (struct pci_dev *dev);
+	int  (*reset) (struct pci_dev *dev, int probe); /* Device specific reset */
 	struct pci_error_handlers *err_handler;
 	struct device_driver	driver;
 	struct pci_dynids dynids;
-- 
1.7.7.6

--------------------------------------------------------------
Intel Shannon Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263
Business address: Dromore House, East Park, Shannon, Co. Clare

This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.

Re: [PATCH 1/1 v3] PCI: Device specific reset function

[Greg KH]

On Mon, Mar 05, 2012 at 10:00:49AM +0000, Tadeusz Struk wrote:
> 
> ---
>  drivers/pci/pci.h    |    1 +
>  drivers/pci/quirks.c |   33 +++++++++++++++++++++++++++------
>  include/linux/pci.h  |    1 +
>  3 files changed, 29 insertions(+), 6 deletions(-)

Please read Documentation/SubmittingPatches for how to properly create,
and send, patches that are in a format that can be accepted.

Hint, also run your patch through scripts/checkpatch.pl to find the
obvious problems that are in it, to keep us from having to do that for
you...

> 
> This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.

I have been told that such email footers require that the patch be
deleted and never be accepted.  Please fix this if you expect your
patches to be able to be applied.

greg k-h

Re: [PATCH 1/1 v3] PCI: Device specific reset function

[Bjorn Helgaas]

On Mon, Mar 5, 2012 at 3:00 AM, Tadeusz Struk <tadeusz.struk@intel.com> wrote:
>
> ---
>  drivers/pci/pci.h    |    1 +
>  drivers/pci/quirks.c |   33 +++++++++++++++++++++++++++------
>  include/linux/pci.h  |    1 +
>  3 files changed, 29 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 1009a5e..4d10479 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -315,6 +315,7 @@ struct pci_dev_reset_methods {
>        u16 vendor;
>        u16 device;
>        int (*reset)(struct pci_dev *dev, int probe);
> +       struct list_head list;
>  };
>
>  #ifdef CONFIG_PCI_QUIRKS
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 6476547..f423d2f 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -3070,26 +3070,47 @@ static int reset_intel_82599_sfp_virtfn(struct pci_dev *dev, int probe)
>  }
>
>  #define PCI_DEVICE_ID_INTEL_82599_SFP_VF   0x10ed
> -
> -static const struct pci_dev_reset_methods pci_dev_reset_methods[] = {
> +static struct pci_dev_reset_methods pci_dev_reset_methods[] = {
>        { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82599_SFP_VF,
> -                reset_intel_82599_sfp_virtfn },
> +               reset_intel_82599_sfp_virtfn },
>        { PCI_VENDOR_ID_INTEL, PCI_ANY_ID,
>                reset_intel_generic_dev },
> -       { 0 }
>  };
>
> +static LIST_HEAD(reset_list);
> +
> +void pci_dev_specific_reset_add(struct pci_dev_reset_methods *reset_method)
> +{
> +       INIT_LIST_HEAD(&reset_method->list);
> +       list_add(&reset_method->list, &reset_list);
> +}
> +
> +static int __init pci_dev_specific_reset_init(void)
> +{
> +       int i;
> +
> +       for (i = 0; i < ARRAY_SIZE(pci_dev_reset_methods); i++) {
> +               pci_dev_specific_reset_add(&pci_dev_reset_methods[i]);
> +       }
> +       return 0;
> +}
> +
> +late_initcall(pci_dev_specific_reset_init);
> +
>  int pci_dev_specific_reset(struct pci_dev *dev, int probe)
>  {
>        const struct pci_dev_reset_methods *i;
> +       struct pci_driver *drv = dev->driver;
> +
> +       if (drv && drv->reset)
> +               return drv->reset(dev, probe);
>
> -       for (i = pci_dev_reset_methods; i->reset; i++) {
> +       list_for_each_entry(i, &reset_list, list) {
>                if ((i->vendor == dev->vendor ||
>                     i->vendor == (u16)PCI_ANY_ID) &&
>                    (i->device == dev->device ||
>                     i->device == (u16)PCI_ANY_ID))
>                        return i->reset(dev, probe);
>        }
> -
>        return -ENOTTY;
>  }
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index a16b1df..a3a0bc5 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -560,6 +560,7 @@ struct pci_driver {
>        int  (*resume_early) (struct pci_dev *dev);
>        int  (*resume) (struct pci_dev *dev);                   /* Device woken up */
>        void (*shutdown) (struct pci_dev *dev);
> +       int  (*reset) (struct pci_dev *dev, int probe); /* Device specific reset */
>        struct pci_error_handlers *err_handler;
>        struct device_driver    driver;
>        struct pci_dynids dynids;

This patch now consists of two pieces:
  1) Convert the reset method table to a list, and
  2) Add the "reset" function pointer in struct pci_driver and the "if
(drv && drv->reset)" block.

These should be split into two patches.

After you split them, I don't think you even need part 1, so you
should probably just drop it.

Common practice would be to also include your driver patch that
actually uses the pci_driver.reset pointer as an additional patch in
the same series.  That gives us more confidence that this solution
actually works and will be used.

Bjorn