[PATCH 0/4] EEH Cleanup

[PATCH 0/4] EEH Cleanup

[Wei Yang]

Four patches to cleanup the EEH, in type, comment and logic.

Wei Yang (4):
  powerpc/eeh: fix start/end/flags type in struct pci_io_addr_range{}
  powerpc/eeh: fix comment for wait_state()
  powerpc/eeh: fix powernv_eeh_wait_state delay logic
  powerpc/eeh: remove unused macro IS_BRIDGE

 arch/powerpc/kernel/eeh.c                    |    2 --
 arch/powerpc/kernel/eeh_cache.c              |   16 ++++++++--------
 arch/powerpc/kernel/eeh_driver.c             |    2 +-
 arch/powerpc/platforms/powernv/eeh-powernv.c |    4 ++--
 arch/powerpc/platforms/pseries/eeh_pseries.c |    2 +-
 5 files changed, 12 insertions(+), 14 deletions(-)

-- 
1.7.9.5

[PATCH 1/4] powerpc/eeh: fix start/end/flags type in struct pci_io_addr_range{}

[Wei Yang]

struct pci_io_addr_range{} stores the information of pci resources. It
would be better to keep these related fields have the same type as in
struct resource{}.

This patch fixes the start/end/flags type in struct pci_io_addr_range{} to
have the same type as in struct resource{}.

Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/eeh_cache.c |   16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/kernel/eeh_cache.c b/arch/powerpc/kernel/eeh_cache.c
index 07d8a24..a30ed67 100644
--- a/arch/powerpc/kernel/eeh_cache.c
+++ b/arch/powerpc/kernel/eeh_cache.c
@@ -48,11 +48,11 @@
  */
 struct pci_io_addr_range {
 	struct rb_node rb_node;
-	unsigned long addr_lo;
-	unsigned long addr_hi;
+	resource_size_t addr_lo;
+	resource_size_t addr_hi;
 	struct eeh_dev *edev;
 	struct pci_dev *pcidev;
-	unsigned int flags;
+	unsigned long flags;
 };
 
 static struct pci_io_addr_cache {
@@ -125,8 +125,8 @@ static void eeh_addr_cache_print(struct pci_io_addr_cache *cache)
 
 /* Insert address range into the rb tree. */
 static struct pci_io_addr_range *
-eeh_addr_cache_insert(struct pci_dev *dev, unsigned long alo,
-		      unsigned long ahi, unsigned int flags)
+eeh_addr_cache_insert(struct pci_dev *dev, resource_size_t alo,
+		      resource_size_t ahi, unsigned long flags)
 {
 	struct rb_node **p = &pci_io_addr_cache_root.rb_root.rb_node;
 	struct rb_node *parent = NULL;
@@ -200,9 +200,9 @@ static void __eeh_addr_cache_insert_dev(struct pci_dev *dev)
 
 	/* Walk resources on this device, poke them into the tree */
 	for (i = 0; i < DEVICE_COUNT_RESOURCE; i++) {
-		unsigned long start = pci_resource_start(dev,i);
-		unsigned long end = pci_resource_end(dev,i);
-		unsigned int flags = pci_resource_flags(dev,i);
+		resource_size_t start = pci_resource_start(dev,i);
+		resource_size_t end = pci_resource_end(dev,i);
+		unsigned long flags = pci_resource_flags(dev,i);
 
 		/* We are interested only bus addresses, not dma or other stuff */
 		if (0 == (flags & (IORESOURCE_IO | IORESOURCE_MEM)))
-- 
1.7.9.5

[PATCH 2/4] powerpc/eeh: fix comment for wait_state()

[Wei Yang]

To retrieve the PCI slot state, EEH driver would set a timeout for that.
While current comment is not aligned to what the code does.

This patch fixes those comments according to the code.

Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/eeh_driver.c             |    2 +-
 arch/powerpc/platforms/powernv/eeh-powernv.c |    2 +-
 arch/powerpc/platforms/pseries/eeh_pseries.c |    2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index d099540..cc8d1db 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -682,7 +682,7 @@ static void eeh_handle_normal_event(struct eeh_pe *pe)
 	eeh_pe_dev_traverse(pe, eeh_report_error, &result);
 
 	/* Get the current PCI slot state. This can take a long time,
-	 * sometimes over 3 seconds for certain systems.
+	 * sometimes over 300 seconds for certain systems.
 	 */
 	rc = eeh_ops->wait_state(pe, MAX_WAIT_FOR_RECOVERY*1000);
 	if (rc < 0 || rc == EEH_STATE_NOT_SUPPORT) {
diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
index 7a5021b..5abb4c2 100644
--- a/arch/powerpc/platforms/powernv/eeh-powernv.c
+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
@@ -313,7 +313,7 @@ static int powernv_eeh_reset(struct eeh_pe *pe, int option)
 /**
  * powernv_eeh_wait_state - Wait for PE state
  * @pe: EEH PE
- * @max_wait: maximal period in microsecond
+ * @max_wait: maximal period in millisecond
  *
  * Wait for the state of associated PE. It might take some time
  * to retrieve the PE's state.
diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c b/arch/powerpc/platforms/pseries/eeh_pseries.c
index a6c7e19..9de46c5 100644
--- a/arch/powerpc/platforms/pseries/eeh_pseries.c
+++ b/arch/powerpc/platforms/pseries/eeh_pseries.c
@@ -538,7 +538,7 @@ static int pseries_eeh_reset(struct eeh_pe *pe, int option)
 /**
  * pseries_eeh_wait_state - Wait for PE state
  * @pe: EEH PE
- * @max_wait: maximal period in microsecond
+ * @max_wait: maximal period in millisecond
  *
  * Wait for the state of associated PE. It might take some time
  * to retrieve the PE's state.
-- 
1.7.9.5

[PATCH 3/4] powerpc/eeh: fix powernv_eeh_wait_state delay logic

[Wei Yang]

As the comment indicates, powernv_eeh_get_state() will inform EEH core to
delay 1 second. This means the delay doesn't happen when
powernv_eeh_get_state() returns.

This patch moves the delay subtraction just before msleep(), which is the
same logic in pseries_eeh_wait_state().

Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/powernv/eeh-powernv.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
index 5abb4c2..af1be8f 100644
--- a/arch/powerpc/platforms/powernv/eeh-powernv.c
+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
@@ -334,13 +334,13 @@ static int powernv_eeh_wait_state(struct eeh_pe *pe, int max_wait)
 		if (ret != EEH_STATE_UNAVAILABLE)
 			return ret;
 
-		max_wait -= mwait;
 		if (max_wait <= 0) {
 			pr_warn("%s: Timeout getting PE#%x's state (%d)\n",
 				__func__, pe->addr, max_wait);
 			return EEH_STATE_NOT_SUPPORT;
 		}
 
+		max_wait -= mwait;
 		msleep(mwait);
 	}
 
-- 
1.7.9.5

[PATCH 4/4] powerpc/eeh: remove unused macro IS_BRIDGE

[Wei Yang]

Currently, the macro IS_BRIDGE is not used any where.
This patch just removes it.

Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/eeh.c |    2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index 3b2252e..60df70c 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -144,8 +144,6 @@ struct eeh_stats {
 
 static struct eeh_stats eeh_stats;
 
-#define IS_BRIDGE(class_code) (((class_code)<<16) == PCI_BASE_CLASS_BRIDGE)
-
 static int __init eeh_setup(char *str)
 {
 	if (!strcmp(str, "off"))
-- 
1.7.9.5

Re: [PATCH 4/4] powerpc/eeh: remove unused macro IS_BRIDGE

[Gavin Shan]

On Mon, Mar 09, 2015 at 11:17:32AM +0800, Wei Yang wrote:
>Currently, the macro IS_BRIDGE is not used any where.
>This patch just removes it.
>
>Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>

Acked-by: Gavin Shan <gwshan@linux.vnet.ibm.com>

Thanks,
Gavin

>---
> arch/powerpc/kernel/eeh.c |    2 --
> 1 file changed, 2 deletions(-)
>
>diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
>index 3b2252e..60df70c 100644
>--- a/arch/powerpc/kernel/eeh.c
>+++ b/arch/powerpc/kernel/eeh.c
>@@ -144,8 +144,6 @@ struct eeh_stats {
>
> static struct eeh_stats eeh_stats;
>
>-#define IS_BRIDGE(class_code) (((class_code)<<16) == PCI_BASE_CLASS_BRIDGE)
>-
> static int __init eeh_setup(char *str)
> {
> 	if (!strcmp(str, "off"))
>-- 
>1.7.9.5
>

Re: [PATCH 3/4] powerpc/eeh: fix powernv_eeh_wait_state delay logic

[Gavin Shan]

On Mon, Mar 09, 2015 at 11:17:31AM +0800, Wei Yang wrote:
>As the comment indicates, powernv_eeh_get_state() will inform EEH core to
>delay 1 second. This means the delay doesn't happen when
>powernv_eeh_get_state() returns.
>
>This patch moves the delay subtraction just before msleep(), which is the
>same logic in pseries_eeh_wait_state().
>
>Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>

The patch would conflict with the patches to remove EEH chip layer done
in https://patchwork.ozlabs.org/patch/439956/. Could you please repost
with corrected function names in commit log when the dependent patches
show up in Michael's next branch?

Except that, the changes look ok to me:

Acked-by: Gavin Shan <gwshan@linux.vnet.ibm.com>

Thanks,
Gavin

>---
> arch/powerpc/platforms/powernv/eeh-powernv.c |    2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
>index 5abb4c2..af1be8f 100644
>--- a/arch/powerpc/platforms/powernv/eeh-powernv.c
>+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
>@@ -334,13 +334,13 @@ static int powernv_eeh_wait_state(struct eeh_pe *pe, int max_wait)
> 		if (ret != EEH_STATE_UNAVAILABLE)
> 			return ret;
>
>-		max_wait -= mwait;
> 		if (max_wait <= 0) {
> 			pr_warn("%s: Timeout getting PE#%x's state (%d)\n",
> 				__func__, pe->addr, max_wait);
> 			return EEH_STATE_NOT_SUPPORT;
> 		}
>
>+		max_wait -= mwait;
> 		msleep(mwait);
> 	}
>
>-- 
>1.7.9.5
>

Re: [PATCH 2/4] powerpc/eeh: fix comment for wait_state()

[Gavin Shan]

On Mon, Mar 09, 2015 at 11:17:30AM +0800, Wei Yang wrote:
>To retrieve the PCI slot state, EEH driver would set a timeout for that.
>While current comment is not aligned to what the code does.
>
>This patch fixes those comments according to the code.
>
>Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>

Please repost after https://patchwork.ozlabs.org/patch/439956/ shows up
in PPC next branch. The changes look good to me:

Acked-by: Gavin Shan <gwshan@linux.vnet.ibm.com>

Thanks,
Gavin

>---
> arch/powerpc/kernel/eeh_driver.c             |    2 +-
> arch/powerpc/platforms/powernv/eeh-powernv.c |    2 +-
> arch/powerpc/platforms/pseries/eeh_pseries.c |    2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
>
>diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
>index d099540..cc8d1db 100644
>--- a/arch/powerpc/kernel/eeh_driver.c
>+++ b/arch/powerpc/kernel/eeh_driver.c
>@@ -682,7 +682,7 @@ static void eeh_handle_normal_event(struct eeh_pe *pe)
> 	eeh_pe_dev_traverse(pe, eeh_report_error, &result);
>
> 	/* Get the current PCI slot state. This can take a long time,
>-	 * sometimes over 3 seconds for certain systems.
>+	 * sometimes over 300 seconds for certain systems.
> 	 */
> 	rc = eeh_ops->wait_state(pe, MAX_WAIT_FOR_RECOVERY*1000);
> 	if (rc < 0 || rc == EEH_STATE_NOT_SUPPORT) {
>diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
>index 7a5021b..5abb4c2 100644
>--- a/arch/powerpc/platforms/powernv/eeh-powernv.c
>+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
>@@ -313,7 +313,7 @@ static int powernv_eeh_reset(struct eeh_pe *pe, int option)
> /**
>  * powernv_eeh_wait_state - Wait for PE state
>  * @pe: EEH PE
>- * @max_wait: maximal period in microsecond
>+ * @max_wait: maximal period in millisecond
>  *
>  * Wait for the state of associated PE. It might take some time
>  * to retrieve the PE's state.
>diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c b/arch/powerpc/platforms/pseries/eeh_pseries.c
>index a6c7e19..9de46c5 100644
>--- a/arch/powerpc/platforms/pseries/eeh_pseries.c
>+++ b/arch/powerpc/platforms/pseries/eeh_pseries.c
>@@ -538,7 +538,7 @@ static int pseries_eeh_reset(struct eeh_pe *pe, int option)
> /**
>  * pseries_eeh_wait_state - Wait for PE state
>  * @pe: EEH PE
>- * @max_wait: maximal period in microsecond
>+ * @max_wait: maximal period in millisecond
>  *
>  * Wait for the state of associated PE. It might take some time
>  * to retrieve the PE's state.
>-- 
>1.7.9.5
>

Re: [PATCH 1/4] powerpc/eeh: fix start/end/flags type in struct pci_io_addr_range{}

[Gavin Shan]

On Mon, Mar 09, 2015 at 11:17:29AM +0800, Wei Yang wrote:
>struct pci_io_addr_range{} stores the information of pci resources. It
>would be better to keep these related fields have the same type as in
>struct resource{}.
>
>This patch fixes the start/end/flags type in struct pci_io_addr_range{} to
>have the same type as in struct resource{}.
>
>Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>

Acked-by: Gavin Shan <gwshan@linux.vnet.ibm.com>

Thanks,
Gavin

>---
> arch/powerpc/kernel/eeh_cache.c |   16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
>diff --git a/arch/powerpc/kernel/eeh_cache.c b/arch/powerpc/kernel/eeh_cache.c
>index 07d8a24..a30ed67 100644
>--- a/arch/powerpc/kernel/eeh_cache.c
>+++ b/arch/powerpc/kernel/eeh_cache.c
>@@ -48,11 +48,11 @@
>  */
> struct pci_io_addr_range {
> 	struct rb_node rb_node;
>-	unsigned long addr_lo;
>-	unsigned long addr_hi;
>+	resource_size_t addr_lo;
>+	resource_size_t addr_hi;
> 	struct eeh_dev *edev;
> 	struct pci_dev *pcidev;
>-	unsigned int flags;
>+	unsigned long flags;
> };
>
> static struct pci_io_addr_cache {
>@@ -125,8 +125,8 @@ static void eeh_addr_cache_print(struct pci_io_addr_cache *cache)
>
> /* Insert address range into the rb tree. */
> static struct pci_io_addr_range *
>-eeh_addr_cache_insert(struct pci_dev *dev, unsigned long alo,
>-		      unsigned long ahi, unsigned int flags)
>+eeh_addr_cache_insert(struct pci_dev *dev, resource_size_t alo,
>+		      resource_size_t ahi, unsigned long flags)
> {
> 	struct rb_node **p = &pci_io_addr_cache_root.rb_root.rb_node;
> 	struct rb_node *parent = NULL;
>@@ -200,9 +200,9 @@ static void __eeh_addr_cache_insert_dev(struct pci_dev *dev)
>
> 	/* Walk resources on this device, poke them into the tree */
> 	for (i = 0; i < DEVICE_COUNT_RESOURCE; i++) {
>-		unsigned long start = pci_resource_start(dev,i);
>-		unsigned long end = pci_resource_end(dev,i);
>-		unsigned int flags = pci_resource_flags(dev,i);
>+		resource_size_t start = pci_resource_start(dev,i);
>+		resource_size_t end = pci_resource_end(dev,i);
>+		unsigned long flags = pci_resource_flags(dev,i);
>
> 		/* We are interested only bus addresses, not dma or other stuff */
> 		if (0 == (flags & (IORESOURCE_IO | IORESOURCE_MEM)))
>-- 
>1.7.9.5
>

Re: [PATCH 3/4] powerpc/eeh: fix powernv_eeh_wait_state delay logic

[Wei Yang]

On Wed, Mar 11, 2015 at 04:13:46PM +1100, Gavin Shan wrote:
>On Mon, Mar 09, 2015 at 11:17:31AM +0800, Wei Yang wrote:
>>As the comment indicates, powernv_eeh_get_state() will inform EEH core to
>>delay 1 second. This means the delay doesn't happen when
>>powernv_eeh_get_state() returns.
>>
>>This patch moves the delay subtraction just before msleep(), which is the
>>same logic in pseries_eeh_wait_state().
>>
>>Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
>
>The patch would conflict with the patches to remove EEH chip layer done
>in https://patchwork.ozlabs.org/patch/439956/. Could you please repost
>with corrected function names in commit log when the dependent patches
>show up in Michael's next branch?
>
>Except that, the changes look ok to me:
>
>Acked-by: Gavin Shan <gwshan@linux.vnet.ibm.com>

Thanks, I will repost it accordingly.

>
>Thanks,
>Gavin
>
>>---
>> arch/powerpc/platforms/powernv/eeh-powernv.c |    2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>>diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
>>index 5abb4c2..af1be8f 100644
>>--- a/arch/powerpc/platforms/powernv/eeh-powernv.c
>>+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
>>@@ -334,13 +334,13 @@ static int powernv_eeh_wait_state(struct eeh_pe *pe, int max_wait)
>> 		if (ret != EEH_STATE_UNAVAILABLE)
>> 			return ret;
>>
>>-		max_wait -= mwait;
>> 		if (max_wait <= 0) {
>> 			pr_warn("%s: Timeout getting PE#%x's state (%d)\n",
>> 				__func__, pe->addr, max_wait);
>> 			return EEH_STATE_NOT_SUPPORT;
>> 		}
>>
>>+		max_wait -= mwait;
>> 		msleep(mwait);
>> 	}
>>
>>-- 
>>1.7.9.5
>>

-- 
Richard Yang
Help you, Help me