Re: [PATCH 1/3] pci: New PCI-E reset API

2007-03-09 Thread Brian King
Greg KH wrote:
 On Thu, Mar 08, 2007 at 02:44:26PM -0600, Brian King wrote:
 Greg,

 I saw you pulled this into your gregkh-2.6 tree. Does that mean
 it is queued for 2.6.22?
 
 Yes it is.  Is that a problem?

No problem at all. 

Thanks,

Brian

-- 
Brian King
eServer Storage I/O
IBM Linux Technology Center
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/3] pci: New PCI-E reset API

2007-03-08 Thread Brian King
Greg,

I saw you pulled this into your gregkh-2.6 tree. Does that mean
it is queued for 2.6.22?

Thanks,

Brian

Brian King wrote:
 Adds a new API which can be used to issue various types
 of PCI-E reset, including PCI-E warm reset and PCI-E hot reset.
 This is needed for an ipr PCI-E adapter which does not properly
 implement BIST. Running BIST on this adapter results in PCI-E
 errors. The only reliable reset mechanism that exists on this
 hardware is PCI Fundamental reset (warm reset). Since driving
 this type of reset is architecture unique, this provides the
 necessary hooks for architectures to add this support.
 
 Signed-off-by: Brian King [EMAIL PROTECTED]
 ---
 
  linux-2.6-bjking1/drivers/pci/pci.c   |   29 +
  linux-2.6-bjking1/include/linux/pci.h |   14 ++
  2 files changed, 43 insertions(+)
 
 diff -puN drivers/pci/pci.c~pci_pci_reset_api drivers/pci/pci.c
 --- linux-2.6/drivers/pci/pci.c~pci_pci_reset_api 2007-02-16 
 10:10:30.0 -0600
 +++ linux-2.6-bjking1/drivers/pci/pci.c   2007-02-16 10:10:30.0 
 -0600
 @@ -893,6 +893,34 @@ pci_disable_device(struct pci_dev *dev)
  }
 
  /**
 + * pcibios_set_pcie_reset_state - set reset state for device dev
 + * @dev: the PCI-E device reset
 + * @state: Reset state to enter into
 + *
 + *
 + * Sets the PCI-E reset state for the device. This is the default
 + * implementation. Architecture implementations can override this.
 + */
 +int __attribute__ ((weak)) pcibios_set_pcie_reset_state(struct pci_dev *dev,
 + enum pcie_reset_state 
 state)
 +{
 + return -EINVAL;
 +}
 +
 +/**
 + * pci_set_pcie_reset_state - set reset state for device dev
 + * @dev: the PCI-E device reset
 + * @state: Reset state to enter into
 + *
 + *
 + * Sets the PCI reset state for the device.
 + */
 +int pci_set_pcie_reset_state(struct pci_dev *dev, enum pcie_reset_state 
 state)
 +{
 + return pcibios_set_pcie_reset_state(dev, state);
 +}
 +
 +/**
   * pci_enable_wake - enable device to generate PME# when suspended
   * @dev: - PCI device to operate on
   * @state: - Current state of device.
 @@ -1374,4 +1402,5 @@ EXPORT_SYMBOL(pci_set_power_state);
  EXPORT_SYMBOL(pci_save_state);
  EXPORT_SYMBOL(pci_restore_state);
  EXPORT_SYMBOL(pci_enable_wake);
 +EXPORT_SYMBOL_GPL(pci_set_pcie_reset_state);
 
 diff -puN include/linux/pci.h~pci_pci_reset_api include/linux/pci.h
 --- linux-2.6/include/linux/pci.h~pci_pci_reset_api   2007-02-16 
 10:10:30.0 -0600
 +++ linux-2.6-bjking1/include/linux/pci.h 2007-02-16 10:10:30.0 
 -0600
 @@ -96,6 +96,19 @@ enum pci_channel_state {
   pci_channel_io_perm_failure = (__force pci_channel_state_t) 3,
  };
 
 +typedef unsigned int __bitwise pcie_reset_state_t;
 +
 +enum pcie_reset_state {
 + /* Reset is NOT asserted (Use to deassert reset) */
 + pci_reset_normal = (__force pcie_reset_state_t) 1,
 +
 + /* Use #PERST to reset PCI-E device */
 + pci_reset_pcie_warm_reset = (__force pcie_reset_state_t) 2,
 +
 + /* Use PCI-E Hot Reset to reset device */
 + pci_reset_pcie_hot_reset = (__force pcie_reset_state_t) 3
 +};
 +
  typedef unsigned short __bitwise pci_bus_flags_t;
  enum pci_bus_flags {
   PCI_BUS_FLAGS_NO_MSI = (__force pci_bus_flags_t) 1,
 @@ -539,6 +552,7 @@ static inline int pci_is_managed(struct 
 
  void pci_disable_device(struct pci_dev *dev);
  void pci_set_master(struct pci_dev *dev);
 +int pci_set_pcie_reset_state(struct pci_dev *dev, enum pcie_reset_state 
 state);
  #define HAVE_PCI_SET_MWI
  int __must_check pci_set_mwi(struct pci_dev *dev);
  void pci_clear_mwi(struct pci_dev *dev);
 _


-- 
Brian King
eServer Storage I/O
IBM Linux Technology Center
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/3] pci: New PCI-E reset API

2007-03-08 Thread Greg KH
On Thu, Mar 08, 2007 at 02:44:26PM -0600, Brian King wrote:
 Greg,
 
 I saw you pulled this into your gregkh-2.6 tree. Does that mean
 it is queued for 2.6.22?

Yes it is.  Is that a problem?

thanks,

greg k-h
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/3] pci: New PCI-E reset API

2007-02-19 Thread Brian King

Adds a new API which can be used to issue various types
of PCI-E reset, including PCI-E warm reset and PCI-E hot reset.
This is needed for an ipr PCI-E adapter which does not properly
implement BIST. Running BIST on this adapter results in PCI-E
errors. The only reliable reset mechanism that exists on this
hardware is PCI Fundamental reset (warm reset). Since driving
this type of reset is architecture unique, this provides the
necessary hooks for architectures to add this support.

Signed-off-by: Brian King [EMAIL PROTECTED]
---

 linux-2.6-bjking1/drivers/pci/pci.c   |   29 +
 linux-2.6-bjking1/include/linux/pci.h |   14 ++
 2 files changed, 43 insertions(+)

diff -puN drivers/pci/pci.c~pci_pci_reset_api drivers/pci/pci.c
--- linux-2.6/drivers/pci/pci.c~pci_pci_reset_api   2007-02-16 
10:10:30.0 -0600
+++ linux-2.6-bjking1/drivers/pci/pci.c 2007-02-16 10:10:30.0 -0600
@@ -893,6 +893,34 @@ pci_disable_device(struct pci_dev *dev)
 }
 
 /**
+ * pcibios_set_pcie_reset_state - set reset state for device dev
+ * @dev: the PCI-E device reset
+ * @state: Reset state to enter into
+ *
+ *
+ * Sets the PCI-E reset state for the device. This is the default
+ * implementation. Architecture implementations can override this.
+ */
+int __attribute__ ((weak)) pcibios_set_pcie_reset_state(struct pci_dev *dev,
+   enum pcie_reset_state 
state)
+{
+   return -EINVAL;
+}
+
+/**
+ * pci_set_pcie_reset_state - set reset state for device dev
+ * @dev: the PCI-E device reset
+ * @state: Reset state to enter into
+ *
+ *
+ * Sets the PCI reset state for the device.
+ */
+int pci_set_pcie_reset_state(struct pci_dev *dev, enum pcie_reset_state state)
+{
+   return pcibios_set_pcie_reset_state(dev, state);
+}
+
+/**
  * pci_enable_wake - enable device to generate PME# when suspended
  * @dev: - PCI device to operate on
  * @state: - Current state of device.
@@ -1374,4 +1402,5 @@ EXPORT_SYMBOL(pci_set_power_state);
 EXPORT_SYMBOL(pci_save_state);
 EXPORT_SYMBOL(pci_restore_state);
 EXPORT_SYMBOL(pci_enable_wake);
+EXPORT_SYMBOL_GPL(pci_set_pcie_reset_state);
 
diff -puN include/linux/pci.h~pci_pci_reset_api include/linux/pci.h
--- linux-2.6/include/linux/pci.h~pci_pci_reset_api 2007-02-16 
10:10:30.0 -0600
+++ linux-2.6-bjking1/include/linux/pci.h   2007-02-16 10:10:30.0 
-0600
@@ -96,6 +96,19 @@ enum pci_channel_state {
pci_channel_io_perm_failure = (__force pci_channel_state_t) 3,
 };
 
+typedef unsigned int __bitwise pcie_reset_state_t;
+
+enum pcie_reset_state {
+   /* Reset is NOT asserted (Use to deassert reset) */
+   pci_reset_normal = (__force pcie_reset_state_t) 1,
+
+   /* Use #PERST to reset PCI-E device */
+   pci_reset_pcie_warm_reset = (__force pcie_reset_state_t) 2,
+
+   /* Use PCI-E Hot Reset to reset device */
+   pci_reset_pcie_hot_reset = (__force pcie_reset_state_t) 3
+};
+
 typedef unsigned short __bitwise pci_bus_flags_t;
 enum pci_bus_flags {
PCI_BUS_FLAGS_NO_MSI = (__force pci_bus_flags_t) 1,
@@ -539,6 +552,7 @@ static inline int pci_is_managed(struct 
 
 void pci_disable_device(struct pci_dev *dev);
 void pci_set_master(struct pci_dev *dev);
+int pci_set_pcie_reset_state(struct pci_dev *dev, enum pcie_reset_state state);
 #define HAVE_PCI_SET_MWI
 int __must_check pci_set_mwi(struct pci_dev *dev);
 void pci_clear_mwi(struct pci_dev *dev);
_
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/3] pci: New PCI-E reset API

2007-02-09 Thread Brian King
Matthew,

Any further comments on this?

Thanks,

Brian

Brian King wrote:
 Matthew Wilcox wrote:
 On Thu, Feb 01, 2007 at 11:30:21AM -0600, Brian King wrote:
 Adds a new API which can be used to issue various types
 of PCI-E reset, including PCI-E warm reset and PCI-E hot reset.
 This is needed for an ipr PCI-E adapter which does not properly
 implement BIST. Running BIST on this adapter results in PCI-E
 errors. The only reliable reset mechanism that exists on this
 hardware is PCI Fundamental reset (warm reset). Since driving
 this type of reset is architecture unique, this provides the
 necessary hooks for architectures to add this support.
 A few points ...

  - When doing a warm reset, you reset the entire device not just the
function (== pci_dev) that gets passed in.  How happy are drivers for
the other functions going to be about this?
 
 I guess I don't see how a warm reset could be issued to a single function
 of a PCI device. I would argue that for a multi-function device, you would
 have to use function level reset. 
 
 
  - You've missed the requirement:

 To allow components to perform internal initialization, system software
 must wait for at least 100 ms from the end of a Conventional Reset of one
 or more devices before it is permitted to issue Configuration Requests
 to those devices.

To fix this, we need to call pci_block_user_cfg_access() before
calling the pcibios function, then msleep(100) after calling it, then
call pci_unblock_user_cfg_access().
 
 What I've done is to provide a very low-level API that can be used to
 accomplish this. In my implementation, the ipr driver is the one doing
 all the required delays and calling pci_block_user_cfg_access, since it
 already was doing that in order to run BIST on the adapter.
 
  - There's no attempt to support either cold or function-level reset in
this patch.
 
 Correct. I had no requirement to implement this. It can always be added
 if there is a need. A function level reset can be performed by simply
 writing a bit in config space, so *technically* we wouldn't need an
 API to do that for us, but it could certainly be added here.
 
 I suspect the Right Way of handling hot/warm/cold reset is going to be
 some kind of integration with error handling.  This driver understands
 about slots being different from functions, and has the ability to
 notify drivers of other functions that a reset is happening.
 
 Perhaps. It would require a way for the adapter device driver to
 indicate what type of reset(s) will work for a particular pci device.
 It would also require a method for a device driver to invoke a reset,
 which does not currently exist today. I think it would be the first
 case of the device driver invoking pci error recovery, so I'm not sure
 how difficult that would be to do with the current code.
 
 I actually thought this API might be used by PCI error recovery
 code, since it may need to perform these sorts of functions.
 
 CC'ing Linas Vepstas since he wrote the powerpc pci recovery code.
 
 Brian
 
 


-- 
Brian King
eServer Storage I/O
IBM Linux Technology Center
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/3] pci: New PCI-E reset API

2007-02-01 Thread Brian King

Adds a new API which can be used to issue various types
of PCI-E reset, including PCI-E warm reset and PCI-E hot reset.
This is needed for an ipr PCI-E adapter which does not properly
implement BIST. Running BIST on this adapter results in PCI-E
errors. The only reliable reset mechanism that exists on this
hardware is PCI Fundamental reset (warm reset). Since driving
this type of reset is architecture unique, this provides the
necessary hooks for architectures to add this support.

Signed-off-by: Brian King [EMAIL PROTECTED]
---

 linux-2.6-bjking1/drivers/pci/pci.c   |   29 +
 linux-2.6-bjking1/include/linux/pci.h |   14 ++
 2 files changed, 43 insertions(+)

diff -puN drivers/pci/pci.c~pci_pci_reset_api drivers/pci/pci.c
--- linux-2.6/drivers/pci/pci.c~pci_pci_reset_api   2007-01-30 
12:48:54.0 -0600
+++ linux-2.6-bjking1/drivers/pci/pci.c 2007-01-30 12:48:54.0 -0600
@@ -791,6 +791,34 @@ pci_disable_device(struct pci_dev *dev)
 }
 
 /**
+ * pcibios_set_pcie_reset_state - set reset state for device dev
+ * @dev: the PCI-E device reset
+ * @state: Reset state to enter into
+ *
+ *
+ * Sets the PCI-E reset state for the device. This is the default
+ * implementation. Architecture implementations can override this.
+ */
+int __attribute__ ((weak)) pcibios_set_pcie_reset_state(struct pci_dev *dev,
+   enum pcie_reset_state 
state)
+{
+   return -EINVAL;
+}
+
+/**
+ * pci_set_pcie_reset_state - set reset state for device dev
+ * @dev: the PCI-E device reset
+ * @state: Reset state to enter into
+ *
+ *
+ * Sets the PCI reset state for the device.
+ */
+int pci_set_pcie_reset_state(struct pci_dev *dev, enum pcie_reset_state state)
+{
+   return pcibios_set_pcie_reset_state(dev, state);
+}
+
+/**
  * pci_enable_wake - enable device to generate PME# when suspended
  * @dev: - PCI device to operate on
  * @state: - Current state of device.
@@ -1210,6 +1238,7 @@ EXPORT_SYMBOL(pci_set_power_state);
 EXPORT_SYMBOL(pci_save_state);
 EXPORT_SYMBOL(pci_restore_state);
 EXPORT_SYMBOL(pci_enable_wake);
+EXPORT_SYMBOL_GPL(pci_set_pcie_reset_state);
 
 /* Quirk info */
 
diff -puN include/linux/pci.h~pci_pci_reset_api include/linux/pci.h
--- linux-2.6/include/linux/pci.h~pci_pci_reset_api 2007-01-30 
12:48:54.0 -0600
+++ linux-2.6-bjking1/include/linux/pci.h   2007-01-30 12:48:54.0 
-0600
@@ -96,6 +96,19 @@ enum pci_channel_state {
pci_channel_io_perm_failure = (__force pci_channel_state_t) 3,
 };
 
+typedef unsigned int __bitwise pcie_reset_state_t;
+
+enum pcie_reset_state {
+   /* Reset is NOT asserted (Use to deassert reset) */
+   pci_reset_normal = (__force pcie_reset_state_t) 1,
+
+   /* Use #PERST to reset PCI-E device */
+   pci_reset_pcie_warm_reset = (__force pcie_reset_state_t) 2,
+
+   /* Use PCI-E Hot Reset to reset device */
+   pci_reset_pcie_hot_reset = (__force pcie_reset_state_t) 3
+};
+
 typedef unsigned short __bitwise pci_bus_flags_t;
 enum pci_bus_flags {
PCI_BUS_FLAGS_NO_MSI = (__force pci_bus_flags_t) 1,
@@ -523,6 +536,7 @@ int __must_check pci_enable_device(struc
 int __must_check pci_enable_device_bars(struct pci_dev *dev, int mask);
 void pci_disable_device(struct pci_dev *dev);
 void pci_set_master(struct pci_dev *dev);
+int pci_set_pcie_reset_state(struct pci_dev *dev, enum pcie_reset_state state);
 #define HAVE_PCI_SET_MWI
 int __must_check pci_set_mwi(struct pci_dev *dev);
 void pci_clear_mwi(struct pci_dev *dev);
_
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/3] pci: New PCI-E reset API

2007-02-01 Thread Matthew Wilcox
On Thu, Feb 01, 2007 at 11:30:21AM -0600, Brian King wrote:
 Adds a new API which can be used to issue various types
 of PCI-E reset, including PCI-E warm reset and PCI-E hot reset.
 This is needed for an ipr PCI-E adapter which does not properly
 implement BIST. Running BIST on this adapter results in PCI-E
 errors. The only reliable reset mechanism that exists on this
 hardware is PCI Fundamental reset (warm reset). Since driving
 this type of reset is architecture unique, this provides the
 necessary hooks for architectures to add this support.

A few points ...

 - When doing a warm reset, you reset the entire device not just the
   function (== pci_dev) that gets passed in.  How happy are drivers for
   the other functions going to be about this?

 - You've missed the requirement:

To allow components to perform internal initialization, system software
must wait for at least 100 ms from the end of a Conventional Reset of one
or more devices before it is permitted to issue Configuration Requests
to those devices.

   To fix this, we need to call pci_block_user_cfg_access() before
   calling the pcibios function, then msleep(100) after calling it, then
   call pci_unblock_user_cfg_access().

 - There's no attempt to support either cold or function-level reset in
   this patch.

I suspect the Right Way of handling hot/warm/cold reset is going to be
some kind of integration with error handling.  This driver understands
about slots being different from functions, and has the ability to
notify drivers of other functions that a reset is happening.

To a certain extent, what's going on with IPR here *is* an error
condition, it's just that we can recover from it with a warm reset
rather than a hot reset.

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


Re: [PATCH 1/3] pci: New PCI-E reset API

2007-02-01 Thread Brian King
Matthew Wilcox wrote:
 On Thu, Feb 01, 2007 at 11:30:21AM -0600, Brian King wrote:
 Adds a new API which can be used to issue various types
 of PCI-E reset, including PCI-E warm reset and PCI-E hot reset.
 This is needed for an ipr PCI-E adapter which does not properly
 implement BIST. Running BIST on this adapter results in PCI-E
 errors. The only reliable reset mechanism that exists on this
 hardware is PCI Fundamental reset (warm reset). Since driving
 this type of reset is architecture unique, this provides the
 necessary hooks for architectures to add this support.
 
 A few points ...
 
  - When doing a warm reset, you reset the entire device not just the
function (== pci_dev) that gets passed in.  How happy are drivers for
the other functions going to be about this?

I guess I don't see how a warm reset could be issued to a single function
of a PCI device. I would argue that for a multi-function device, you would
have to use function level reset. 


  - You've missed the requirement:
 
 To allow components to perform internal initialization, system software
 must wait for at least 100 ms from the end of a Conventional Reset of one
 or more devices before it is permitted to issue Configuration Requests
 to those devices.
 
To fix this, we need to call pci_block_user_cfg_access() before
calling the pcibios function, then msleep(100) after calling it, then
call pci_unblock_user_cfg_access().

What I've done is to provide a very low-level API that can be used to
accomplish this. In my implementation, the ipr driver is the one doing
all the required delays and calling pci_block_user_cfg_access, since it
already was doing that in order to run BIST on the adapter.

  - There's no attempt to support either cold or function-level reset in
this patch.

Correct. I had no requirement to implement this. It can always be added
if there is a need. A function level reset can be performed by simply
writing a bit in config space, so *technically* we wouldn't need an
API to do that for us, but it could certainly be added here.

 I suspect the Right Way of handling hot/warm/cold reset is going to be
 some kind of integration with error handling.  This driver understands
 about slots being different from functions, and has the ability to
 notify drivers of other functions that a reset is happening.

Perhaps. It would require a way for the adapter device driver to
indicate what type of reset(s) will work for a particular pci device.
It would also require a method for a device driver to invoke a reset,
which does not currently exist today. I think it would be the first
case of the device driver invoking pci error recovery, so I'm not sure
how difficult that would be to do with the current code.

I actually thought this API might be used by PCI error recovery
code, since it may need to perform these sorts of functions.

CC'ing Linas Vepstas since he wrote the powerpc pci recovery code.

Brian


-- 
Brian King
eServer Storage I/O
IBM Linux Technology Center
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html