[PATCH 1/13]: PCI Err: pci.h header file changes

[PATCH 1/13]: PCI Err: pci.h header file changes

[Linas Vepstas]

[-- Attachment #1: Type: text/plain, Size: 259 bytes --]



pci-err-1-pci.h.patch

This patch adds PCI error recovery callbacks, error state and 
error return codes to include/linux/pci.h.  These are closely 
described in the next patch, a documentation file.

Signed-off-by: Linas Vepstas <linas@linas.org>

[-- Attachment #2: pci-err-1-pci.h.patch --]
[-- Type: text/plain, Size: 2196 bytes --]

--- linux-2.6.12-git10/include/linux/pci.h.linas-orig	2005-06-17 14:48:29.000000000 -0500
+++ linux-2.6.12-git10/include/linux/pci.h	2005-06-24 14:44:59.000000000 -0500
@@ -660,6 +660,37 @@ struct pci_dynids {
 	unsigned int use_driver_data:1; /* pci_driver->driver_data is used */
 };
 
+/* ---------------------------------------------------------------- */
+/** PCI error recovery infrastructure.  If a PCI device driver provides
+ *  a set fof callbacks in struct pci_error_handlers, then that device driver
+ *  will be notified of PCI bus errors, and can be driven to recovery.
+ */
+
+enum pci_channel_state {
+	pci_channel_io_normal = 0, /* I/O channel is in normal state */
+	pci_channel_io_frozen = 1, /* I/O to channel is blocked */
+	pci_channel_io_perm_failure, /* pci card is dead */
+};
+
+enum pcierr_result {
+	PCIERR_RESULT_NONE=0,        /* no result/none/not supported in device driver */
+	PCIERR_RESULT_CAN_RECOVER=1, /* Device driver can recover without slot reset */
+	PCIERR_RESULT_NEED_RESET,    /* Device driver wants slot to be reset. */
+	PCIERR_RESULT_DISCONNECT,    /* Device has completely failed, is unrecoverable */
+	PCIERR_RESULT_RECOVERED,     /* Device driver is fully recovered and operational */
+};
+
+/* PCI bus error event callbacks */
+struct pci_error_handlers
+{
+	enum pci_channel_state error_state;       /* current error state */
+	int (*error_detected)(struct pci_dev *dev, enum pci_channel_state error);
+	int (*mmio_enabled)(struct pci_dev *dev); /* MMIO has been reanbled, but not DMA */
+	int (*link_reset)(struct pci_dev *dev);   /* PCI Express link has been reset */
+	int (*slot_reset)(struct pci_dev *dev);   /* PCI slot has been reset */
+	void (*resume)(struct pci_dev *dev);      /* Device driver may resume normal operations */
+};
+
 struct module;
 struct pci_driver {
 	struct list_head node;
@@ -673,6 +704,7 @@ struct pci_driver {
 	int  (*enable_wake) (struct pci_dev *dev, pci_power_t state, int enable);   /* Enable wake event */
 	void (*shutdown) (struct pci_dev *dev);
 
+	struct pci_error_handlers err_handler;
 	struct device_driver	driver;
 	struct pci_dynids dynids;
 };

Re: [PATCH 1/13]: PCI Err: pci.h header file changes

[Greg KH]

On Tue, Jun 28, 2005 at 06:58:17PM -0500, Linas Vepstas wrote:
> @@ -673,6 +704,7 @@ struct pci_driver {
>  	int  (*enable_wake) (struct pci_dev *dev, pci_power_t state, int enable);   /* Enable wake event */
>  	void (*shutdown) (struct pci_dev *dev);
>  
> +	struct pci_error_handlers err_handler;
>  	struct device_driver	driver;
>  	struct pci_dynids dynids;
>  };

Shouldn't that be a pointer and not the whole structure?  Wouldn't that
make it easier to "reuse" error handlers?

thanks,

greg k-h

Re: [PATCH 1/13]: PCI Err: pci.h header file changes

[Andi Kleen]

On Tue, Jun 28, 2005 at 05:29:51PM -0700, Greg KH wrote:
> On Tue, Jun 28, 2005 at 06:58:17PM -0500, Linas Vepstas wrote:
> > @@ -673,6 +704,7 @@ struct pci_driver {
> >  	int  (*enable_wake) (struct pci_dev *dev, pci_power_t state, int enable);   /* Enable wake event */
> >  	void (*shutdown) (struct pci_dev *dev);
> >  
> > +	struct pci_error_handlers err_handler;
> >  	struct device_driver	driver;
> >  	struct pci_dynids dynids;
> >  };
> 
> Shouldn't that be a pointer and not the whole structure?  Wouldn't that
> make it easier to "reuse" error handlers?

Yes, it's a good idea. In fact we could have a generic NIC error
handler structure then that just calls the watchdog timeout function.
I suspect that would be sufficient for most NICs.

-Andi

Re: [PATCH 1/13]: PCI Err: pci.h header file changes

[Benjamin Herrenschmidt]

On Tue, 2005-06-28 at 18:58 -0500, Linas Vepstas wrote:
> +
> +/* PCI bus error event callbacks */
> +struct pci_error_handlers
> +{
> +       enum pci_channel_state error_state;       /* current error state */
> +       int (*error_detected)(struct pci_dev *dev, enum pci_channel_state error);
> +       int (*mmio_enabled)(struct pci_dev *dev); /* MMIO has been reanbled, but not DMA */
> +       int (*link_reset)(struct pci_dev *dev);   /* PCI Express link has been reset */
> +       int (*slot_reset)(struct pci_dev *dev);   /* PCI slot has been reset */
> +       void (*resume)(struct pci_dev *dev);      /* Device driver may resume normal operations */
> +};

The state variable shouldn't be in that structure. As Greg pointed, we
should have a pointer to that structure in pci_driver, not a copy, and
the error state should be in pci_dev. (With you current code, it's per
driver which is broken anyway).

Ben.