[PATCH] e100: do not go D3 in shutdown unless system is powering off

[PATCH] e100: do not go D3 in shutdown unless system is powering off

[Thadeu Lima de Souza Cascardo]

After experimenting with kexec with the last merges after 2.6.29, I've
had some problems when probing e100. It would not read the eeprom. After
some bisects, I realized this has been like that since forever (at least
2.6.18). The problem is that shutdown is doing the same thing that
suspend does and puts the device in D3 state. I couldn't find a way to
get the device back to a sane state in the probe function. So, based on
some similar patches from Rafael J. Wysocki for e1000, e1000e and ixgbe,
I wrote this one for e100.

Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@holoscopio.com>
---
 drivers/net/e100.c |   27 +++++++++++++++++++++------
 1 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/drivers/net/e100.c b/drivers/net/e100.c
index c0f8443..3db7b29 100644
--- a/drivers/net/e100.c
+++ b/drivers/net/e100.c
@@ -2728,7 +2728,7 @@ static void __devexit e100_remove(struct pci_dev *pdev)
 #define E100_82552_SMARTSPEED   0x14   /* SmartSpeed Ctrl register */
 #define E100_82552_REV_ANEG     0x0200 /* Reverse auto-negotiation */
 #define E100_82552_ANEG_NOW     0x0400 /* Auto-negotiate now */
-static int e100_suspend(struct pci_dev *pdev, pm_message_t state)
+static int __e100_shutdown(struct pci_dev *pdev, bool *enable_wake)
 {
 	struct net_device *netdev = pci_get_drvdata(pdev);
 	struct nic *nic = netdev_priv(netdev);
@@ -2749,19 +2749,31 @@ static int e100_suspend(struct pci_dev *pdev, pm_message_t state)
 			           E100_82552_SMARTSPEED, smartspeed |
 			           E100_82552_REV_ANEG | E100_82552_ANEG_NOW);
 		}
-		if (pci_enable_wake(pdev, PCI_D3cold, true))
-			pci_enable_wake(pdev, PCI_D3hot, true);
+		*enable_wake = true;
 	} else {
-		pci_enable_wake(pdev, PCI_D3hot, false);
+		*enable_wake = false;
 	}
 
 	pci_disable_device(pdev);
-	pci_set_power_state(pdev, PCI_D3hot);
 
 	return 0;
 }
 
+static void __e100_power_off(struct pci_dev *pdev, bool wake)
+{
+	pci_enable_wake(pdev, PCI_D3hot, wake);
+	pci_set_power_state(pdev, PCI_D3hot);
+}
+
 #ifdef CONFIG_PM
+static int e100_suspend(struct pci_dev *pdev, pm_message_t state)
+{
+	bool wake;
+	int retval = __e100_shutdown(pdev, &wake);
+	__e100_power_off(pdev, wake);
+	return retval;
+}
+
 static int e100_resume(struct pci_dev *pdev)
 {
 	struct net_device *netdev = pci_get_drvdata(pdev);
@@ -2792,7 +2804,10 @@ static int e100_resume(struct pci_dev *pdev)
 
 static void e100_shutdown(struct pci_dev *pdev)
 {
-	e100_suspend(pdev, PMSG_SUSPEND);
+	bool wake;
+	__e100_shutdown(pdev, &wake);
+	if (system_state == SYSTEM_POWER_OFF)
+		__e100_power_off(pdev, wake);
 }
 
 /* ------------------ PCI Error Recovery infrastructure  -------------- */
-- 
1.6.2.2

Re: [PATCH] e100: do not go D3 in shutdown unless system is powering off

[Rafael J. Wysocki]

On Monday 20 April 2009, Thadeu Lima de Souza Cascardo wrote:
> After experimenting with kexec with the last merges after 2.6.29, I've
> had some problems when probing e100. It would not read the eeprom. After
> some bisects, I realized this has been like that since forever (at least
> 2.6.18). The problem is that shutdown is doing the same thing that
> suspend does and puts the device in D3 state. I couldn't find a way to
> get the device back to a sane state in the probe function. So, based on
> some similar patches from Rafael J. Wysocki for e1000, e1000e and ixgbe,
> I wrote this one for e100.
> 
> Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@holoscopio.com>
> ---
>  drivers/net/e100.c |   27 +++++++++++++++++++++------
>  1 files changed, 21 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/e100.c b/drivers/net/e100.c
> index c0f8443..3db7b29 100644
> --- a/drivers/net/e100.c
> +++ b/drivers/net/e100.c
> @@ -2728,7 +2728,7 @@ static void __devexit e100_remove(struct pci_dev *pdev)
>  #define E100_82552_SMARTSPEED   0x14   /* SmartSpeed Ctrl register */
>  #define E100_82552_REV_ANEG     0x0200 /* Reverse auto-negotiation */
>  #define E100_82552_ANEG_NOW     0x0400 /* Auto-negotiate now */
> -static int e100_suspend(struct pci_dev *pdev, pm_message_t state)
> +static int __e100_shutdown(struct pci_dev *pdev, bool *enable_wake)
>  {
>  	struct net_device *netdev = pci_get_drvdata(pdev);
>  	struct nic *nic = netdev_priv(netdev);
> @@ -2749,19 +2749,31 @@ static int e100_suspend(struct pci_dev *pdev, pm_message_t state)
>  			           E100_82552_SMARTSPEED, smartspeed |
>  			           E100_82552_REV_ANEG | E100_82552_ANEG_NOW);
>  		}
> -		if (pci_enable_wake(pdev, PCI_D3cold, true))
> -			pci_enable_wake(pdev, PCI_D3hot, true);
> +		*enable_wake = true;
>  	} else {
> -		pci_enable_wake(pdev, PCI_D3hot, false);
> +		*enable_wake = false;
>  	}
>  
>  	pci_disable_device(pdev);
> -	pci_set_power_state(pdev, PCI_D3hot);
>  
>  	return 0;
>  }
>  
> +static void __e100_power_off(struct pci_dev *pdev, bool wake)
> +{
> +	pci_enable_wake(pdev, PCI_D3hot, wake);
> +	pci_set_power_state(pdev, PCI_D3hot);
> +}
> +
>  #ifdef CONFIG_PM
> +static int e100_suspend(struct pci_dev *pdev, pm_message_t state)
> +{
> +	bool wake;
> +	int retval = __e100_shutdown(pdev, &wake);

I'd call pci_prepare_to_sleep() here if wake is 'true' instead of the
__e100_power_off(), because there is a chance the platform will prefer some
other power state to put the device into.

In fact, looking at the entire driver's code, I think you could just call
pci_prepare_to_sleep(pdev) here instead of __e100_power_off(pdev, wake)
and discard the value of wake.

> +	__e100_power_off(pdev, wake);

Also, retval will always be 0 as far as I can see and if it could be different
from 0, it would be a good idea to return the error code before putting the
device into a low power state (.resume() won't be called for this device if
.suspend() fails).

Apart from this, the patch looks fine to me.

> +	return retval;
> +}
> +
>  static int e100_resume(struct pci_dev *pdev)
>  {
>  	struct net_device *netdev = pci_get_drvdata(pdev);
> @@ -2792,7 +2804,10 @@ static int e100_resume(struct pci_dev *pdev)
>  
>  static void e100_shutdown(struct pci_dev *pdev)
>  {
> -	e100_suspend(pdev, PMSG_SUSPEND);
> +	bool wake;
> +	__e100_shutdown(pdev, &wake);
> +	if (system_state == SYSTEM_POWER_OFF)
> +		__e100_power_off(pdev, wake);
>  }
>  
>  /* ------------------ PCI Error Recovery infrastructure  -------------- */

Best,
Rafael

Re: [PATCH] e100: do not go D3 in shutdown unless system is powering off

[Thadeu Lima de Souza Cascardo]

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

On Mon, Apr 20, 2009 at 09:13:59PM +0200, Rafael J. Wysocki wrote:
> On Monday 20 April 2009, Thadeu Lima de Souza Cascardo wrote:
> > After experimenting with kexec with the last merges after 2.6.29, I've
> > had some problems when probing e100. It would not read the eeprom. After
> > some bisects, I realized this has been like that since forever (at least
> > 2.6.18). The problem is that shutdown is doing the same thing that
> > suspend does and puts the device in D3 state. I couldn't find a way to
> > get the device back to a sane state in the probe function. So, based on
> > some similar patches from Rafael J. Wysocki for e1000, e1000e and ixgbe,
> > I wrote this one for e100.
> > 
> > Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@holoscopio.com>
> > ---
> >  drivers/net/e100.c |   27 +++++++++++++++++++++------
> >  1 files changed, 21 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/net/e100.c b/drivers/net/e100.c
> > index c0f8443..3db7b29 100644
> > --- a/drivers/net/e100.c
> > +++ b/drivers/net/e100.c
> > @@ -2728,7 +2728,7 @@ static void __devexit e100_remove(struct pci_dev *pdev)
> >  #define E100_82552_SMARTSPEED   0x14   /* SmartSpeed Ctrl register */
> >  #define E100_82552_REV_ANEG     0x0200 /* Reverse auto-negotiation */
> >  #define E100_82552_ANEG_NOW     0x0400 /* Auto-negotiate now */
> > -static int e100_suspend(struct pci_dev *pdev, pm_message_t state)
> > +static int __e100_shutdown(struct pci_dev *pdev, bool *enable_wake)
> >  {
> >  	struct net_device *netdev = pci_get_drvdata(pdev);
> >  	struct nic *nic = netdev_priv(netdev);
> > @@ -2749,19 +2749,31 @@ static int e100_suspend(struct pci_dev *pdev, pm_message_t state)
> >  			           E100_82552_SMARTSPEED, smartspeed |
> >  			           E100_82552_REV_ANEG | E100_82552_ANEG_NOW);
> >  		}
> > -		if (pci_enable_wake(pdev, PCI_D3cold, true))
> > -			pci_enable_wake(pdev, PCI_D3hot, true);
> > +		*enable_wake = true;
> >  	} else {
> > -		pci_enable_wake(pdev, PCI_D3hot, false);
> > +		*enable_wake = false;
> >  	}
> >  
> >  	pci_disable_device(pdev);
> > -	pci_set_power_state(pdev, PCI_D3hot);
> >  
> >  	return 0;
> >  }
> >  
> > +static void __e100_power_off(struct pci_dev *pdev, bool wake)
> > +{
> > +	pci_enable_wake(pdev, PCI_D3hot, wake);
> > +	pci_set_power_state(pdev, PCI_D3hot);
> > +}
> > +
> >  #ifdef CONFIG_PM
> > +static int e100_suspend(struct pci_dev *pdev, pm_message_t state)
> > +{
> > +	bool wake;
> > +	int retval = __e100_shutdown(pdev, &wake);
> 
> I'd call pci_prepare_to_sleep() here if wake is 'true' instead of the
> __e100_power_off(), because there is a chance the platform will prefer some
> other power state to put the device into.
> 
> In fact, looking at the entire driver's code, I think you could just call
> pci_prepare_to_sleep(pdev) here instead of __e100_power_off(pdev, wake)
> and discard the value of wake.
> 

If there is no advantage in using pci_enable_wake with false in the case
the device cannot WOL or ASF, I will just use pci_prepare_to_sleep and
drop this enable_wake/wake variable in both suspend and shutdown. Any
reason we should use pci_enable_wake with false?

> > +	__e100_power_off(pdev, wake);
> 
> Also, retval will always be 0 as far as I can see and if it could be different
> from 0, it would be a good idea to return the error code before putting the
> device into a low power state (.resume() won't be called for this device if
> .suspend() fails).
> 
> Apart from this, the patch looks fine to me.
> 
> > +	return retval;
> > +}
> > +
> >  static int e100_resume(struct pci_dev *pdev)
> >  {
> >  	struct net_device *netdev = pci_get_drvdata(pdev);
> > @@ -2792,7 +2804,10 @@ static int e100_resume(struct pci_dev *pdev)
> >  
> >  static void e100_shutdown(struct pci_dev *pdev)
> >  {
> > -	e100_suspend(pdev, PMSG_SUSPEND);
> > +	bool wake;
> > +	__e100_shutdown(pdev, &wake);
> > +	if (system_state == SYSTEM_POWER_OFF)
> > +		__e100_power_off(pdev, wake);
> >  }
> >  
> >  /* ------------------ PCI Error Recovery infrastructure  -------------- */
> 
> Best,
> Rafael

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] e100: do not go D3 in shutdown unless system is powering off

[Rafael J. Wysocki]

On Tuesday 21 April 2009, Thadeu Lima de Souza Cascardo wrote:
> On Mon, Apr 20, 2009 at 09:13:59PM +0200, Rafael J. Wysocki wrote:
> > On Monday 20 April 2009, Thadeu Lima de Souza Cascardo wrote:
> > > After experimenting with kexec with the last merges after 2.6.29, I've
> > > had some problems when probing e100. It would not read the eeprom. After
> > > some bisects, I realized this has been like that since forever (at least
> > > 2.6.18). The problem is that shutdown is doing the same thing that
> > > suspend does and puts the device in D3 state. I couldn't find a way to
> > > get the device back to a sane state in the probe function. So, based on
> > > some similar patches from Rafael J. Wysocki for e1000, e1000e and ixgbe,
> > > I wrote this one for e100.
> > > 
> > > Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@holoscopio.com>
> > > ---
> > >  drivers/net/e100.c |   27 +++++++++++++++++++++------
> > >  1 files changed, 21 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/drivers/net/e100.c b/drivers/net/e100.c
> > > index c0f8443..3db7b29 100644
> > > --- a/drivers/net/e100.c
> > > +++ b/drivers/net/e100.c
> > > @@ -2728,7 +2728,7 @@ static void __devexit e100_remove(struct pci_dev *pdev)
> > >  #define E100_82552_SMARTSPEED   0x14   /* SmartSpeed Ctrl register */
> > >  #define E100_82552_REV_ANEG     0x0200 /* Reverse auto-negotiation */
> > >  #define E100_82552_ANEG_NOW     0x0400 /* Auto-negotiate now */
> > > -static int e100_suspend(struct pci_dev *pdev, pm_message_t state)
> > > +static int __e100_shutdown(struct pci_dev *pdev, bool *enable_wake)
> > >  {
> > >  	struct net_device *netdev = pci_get_drvdata(pdev);
> > >  	struct nic *nic = netdev_priv(netdev);
> > > @@ -2749,19 +2749,31 @@ static int e100_suspend(struct pci_dev *pdev, pm_message_t state)
> > >  			           E100_82552_SMARTSPEED, smartspeed |
> > >  			           E100_82552_REV_ANEG | E100_82552_ANEG_NOW);
> > >  		}
> > > -		if (pci_enable_wake(pdev, PCI_D3cold, true))
> > > -			pci_enable_wake(pdev, PCI_D3hot, true);
> > > +		*enable_wake = true;
> > >  	} else {
> > > -		pci_enable_wake(pdev, PCI_D3hot, false);
> > > +		*enable_wake = false;
> > >  	}
> > >  
> > >  	pci_disable_device(pdev);
> > > -	pci_set_power_state(pdev, PCI_D3hot);
> > >  
> > >  	return 0;
> > >  }
> > >  
> > > +static void __e100_power_off(struct pci_dev *pdev, bool wake)
> > > +{
> > > +	pci_enable_wake(pdev, PCI_D3hot, wake);
> > > +	pci_set_power_state(pdev, PCI_D3hot);
> > > +}
> > > +
> > >  #ifdef CONFIG_PM
> > > +static int e100_suspend(struct pci_dev *pdev, pm_message_t state)
> > > +{
> > > +	bool wake;
> > > +	int retval = __e100_shutdown(pdev, &wake);
> > 
> > I'd call pci_prepare_to_sleep() here if wake is 'true' instead of the
> > __e100_power_off(), because there is a chance the platform will prefer some
> > other power state to put the device into.
> > 
> > In fact, looking at the entire driver's code, I think you could just call
> > pci_prepare_to_sleep(pdev) here instead of __e100_power_off(pdev, wake)
> > and discard the value of wake.
> > 
> 
> If there is no advantage in using pci_enable_wake with false in the case
> the device cannot WOL or ASF, I will just use pci_prepare_to_sleep and
> drop this enable_wake/wake variable in both suspend and shutdown. Any
> reason we should use pci_enable_wake with false?

In principle there is one.  Namely, if you call it with 'false', it will call
the platform (eg. ACPI) to disable the wake-up functionality of the device,
which generally may be necessary.

Also, pci_prepare_to_sleep() is really designed for suspend/hibernation,
because it first finds the appropriate state to put the device into and that
depends on the target sleep state of the system.

So, I'd recommend using pci_prepare_to_sleep() in .suspend() and the
pci_enable_wake()/pci_set_power_state() combo in .shutdown() (if the system
is going for power off).

Thanks,
Rafael

[PATCH v2] e100: do not go D3 in shutdown unless system is powering off

[Thadeu Lima de Souza Cascardo]

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

After experimenting with kexec with the last merges after 2.6.29, I've
had some problems when probing e100. It would not read the eeprom. After
some bisects, I realized this has been like that since forever (at least
2.6.18). The problem is that shutdown is doing the same thing that
suspend does and puts the device in D3 state. I couldn't find a way to
get the device back to a sane state in the probe function. So, based on
some similar patches from Rafael J. Wysocki for e1000, e1000e and ixgbe,
I wrote this one for e100.

Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@holoscopio.com>
---
 drivers/net/e100.c |   27 +++++++++++++++++++++------
 1 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/drivers/net/e100.c b/drivers/net/e100.c
index c0f8443..3db7b29 100644
--- a/drivers/net/e100.c
+++ b/drivers/net/e100.c
@@ -2728,7 +2728,7 @@ static void __devexit e100_remove(struct pci_dev *pdev)
 #define E100_82552_SMARTSPEED   0x14   /* SmartSpeed Ctrl register */
 #define E100_82552_REV_ANEG     0x0200 /* Reverse auto-negotiation */
 #define E100_82552_ANEG_NOW     0x0400 /* Auto-negotiate now */
-static int e100_suspend(struct pci_dev *pdev, pm_message_t state)
+static int __e100_shutdown(struct pci_dev *pdev, bool *enable_wake)
 {
 	struct net_device *netdev = pci_get_drvdata(pdev);
 	struct nic *nic = netdev_priv(netdev);
@@ -2749,19 +2749,31 @@ static int e100_suspend(struct pci_dev *pdev, pm_message_t state)
 			           E100_82552_SMARTSPEED, smartspeed |
 			           E100_82552_REV_ANEG | E100_82552_ANEG_NOW);
 		}
-		if (pci_enable_wake(pdev, PCI_D3cold, true))
-			pci_enable_wake(pdev, PCI_D3hot, true);
+		*enable_wake = true;
 	} else {
-		pci_enable_wake(pdev, PCI_D3hot, false);
+		*enable_wake = false;
 	}
 
 	pci_disable_device(pdev);
-	pci_set_power_state(pdev, PCI_D3hot);
 
 	return 0;
 }
 
+static void __e100_power_off(struct pci_dev *pdev, bool wake)
+{
+	pci_enable_wake(pdev, PCI_D3hot, wake);
+	pci_set_power_state(pdev, PCI_D3hot);
+}
+
 #ifdef CONFIG_PM
+static int e100_suspend(struct pci_dev *pdev, pm_message_t state)
+{
+	bool wake;
+	int retval = __e100_shutdown(pdev, &wake);
+	__e100_power_off(pdev, wake);
+	return retval;
+}
+
 static int e100_resume(struct pci_dev *pdev)
 {
 	struct net_device *netdev = pci_get_drvdata(pdev);
@@ -2792,7 +2804,10 @@ static int e100_resume(struct pci_dev *pdev)
 
 static void e100_shutdown(struct pci_dev *pdev)
 {
-	e100_suspend(pdev, PMSG_SUSPEND);
+	bool wake;
+	__e100_shutdown(pdev, &wake);
+	if (system_state == SYSTEM_POWER_OFF)
+		__e100_power_off(pdev, wake);
 }
 
 /* ------------------ PCI Error Recovery infrastructure  -------------- */
-- 
1.6.2.3


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

[PATCH v2 fixed] e100: do not go D3 in shutdown unless system is powering off

[Thadeu Lima de Souza Cascardo]

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

After experimenting with kexec with the last merges after 2.6.29, I've
had some problems when probing e100. It would not read the eeprom. After
some bisects, I realized this has been like that since forever (at least
2.6.18). The problem is that shutdown is doing the same thing that
suspend does and puts the device in D3 state. I couldn't find a way to
get the device back to a sane state in the probe function. So, based on
some similar patches from Rafael J. Wysocki for e1000, e1000e and ixgbe,
I wrote this one for e100.

Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@holoscopio.com>
---
 drivers/net/e100.c |   30 +++++++++++++++++++++++-------
 1 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/drivers/net/e100.c b/drivers/net/e100.c
index c0f8443..68e3b79 100644
--- a/drivers/net/e100.c
+++ b/drivers/net/e100.c
@@ -2728,7 +2728,7 @@ static void __devexit e100_remove(struct pci_dev *pdev)
 #define E100_82552_SMARTSPEED   0x14   /* SmartSpeed Ctrl register */
 #define E100_82552_REV_ANEG     0x0200 /* Reverse auto-negotiation */
 #define E100_82552_ANEG_NOW     0x0400 /* Auto-negotiate now */
-static int e100_suspend(struct pci_dev *pdev, pm_message_t state)
+static void __e100_shutdown(struct pci_dev *pdev, bool *enable_wake)
 {
 	struct net_device *netdev = pci_get_drvdata(pdev);
 	struct nic *nic = netdev_priv(netdev);
@@ -2749,19 +2749,32 @@ static int e100_suspend(struct pci_dev *pdev, pm_message_t state)
 			           E100_82552_SMARTSPEED, smartspeed |
 			           E100_82552_REV_ANEG | E100_82552_ANEG_NOW);
 		}
-		if (pci_enable_wake(pdev, PCI_D3cold, true))
-			pci_enable_wake(pdev, PCI_D3hot, true);
+		*enable_wake = true;
 	} else {
-		pci_enable_wake(pdev, PCI_D3hot, false);
+		*enable_wake = false;
 	}
 
 	pci_disable_device(pdev);
-	pci_set_power_state(pdev, PCI_D3hot);
+}
 
-	return 0;
+static int __e100_power_off(struct pci_dev *pdev, bool wake)
+{
+	if (wake) {
+		return pci_prepare_to_sleep(pdev);
+	} else {
+		pci_wake_from_d3(pdev, false);
+		return pci_set_power_state(pdev, PCI_D3hot);
+	}
 }
 
 #ifdef CONFIG_PM
+static int e100_suspend(struct pci_dev *pdev, pm_message_t state)
+{
+	bool wake;
+	__e100_shutdown(pdev, &wake);
+	return __e100_power_off(pdev, wake);
+}
+
 static int e100_resume(struct pci_dev *pdev)
 {
 	struct net_device *netdev = pci_get_drvdata(pdev);
@@ -2792,7 +2805,10 @@ static int e100_resume(struct pci_dev *pdev)
 
 static void e100_shutdown(struct pci_dev *pdev)
 {
-	e100_suspend(pdev, PMSG_SUSPEND);
+	bool wake;
+	__e100_shutdown(pdev, &wake);
+	if (system_state == SYSTEM_POWER_OFF)
+		__e100_power_off(pdev, wake);
 }
 
 /* ------------------ PCI Error Recovery infrastructure  -------------- */
-- 
1.6.2.3


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH v2 fixed] e100: do not go D3 in shutdown unless system is powering off

[Jeff Kirsher]

On Tue, Apr 21, 2009 at 9:38 PM, Thadeu Lima de Souza Cascardo
<cascardo@holoscopio.com> wrote:
> After experimenting with kexec with the last merges after 2.6.29, I've
> had some problems when probing e100. It would not read the eeprom. After
> some bisects, I realized this has been like that since forever (at least
> 2.6.18). The problem is that shutdown is doing the same thing that
> suspend does and puts the device in D3 state. I couldn't find a way to
> get the device back to a sane state in the probe function. So, based on
> some similar patches from Rafael J. Wysocki for e1000, e1000e and ixgbe,
> I wrote this one for e100.
>
> Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@holoscopio.com>
> ---

Thanks, I will add it to my queue of patches for e100.

Also, just FYI, Auke is no longer a maintainer for
e100/e1000/e1000e/igb/ixgb/ixgbe devices, so I have removed him from
the thread.

-- 
Cheers,
Jeff

Re: [PATCH v2 fixed] e100: do not go D3 in shutdown unless system is powering off

[Jeff Kirsher]

On Tue, Apr 21, 2009 at 9:38 PM, Thadeu Lima de Souza Cascardo
<cascardo@holoscopio.com> wrote:
> After experimenting with kexec with the last merges after 2.6.29, I've
> had some problems when probing e100. It would not read the eeprom. After
> some bisects, I realized this has been like that since forever (at least
> 2.6.18). The problem is that shutdown is doing the same thing that
> suspend does and puts the device in D3 state. I couldn't find a way to
> get the device back to a sane state in the probe function. So, based on
> some similar patches from Rafael J. Wysocki for e1000, e1000e and ixgbe,
> I wrote this one for e100.
>
> Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@holoscopio.com>
> ---

Thanks, I will add it to my queue of patches for e100.

Also, just FYI, Auke is no longer a maintainer for
e100/e1000/e1000e/igb/ixgb/ixgbe devices, so I have removed him from
the thread.

-- 
Cheers,
Jeff

------------------------------------------------------------------------------
Stay on top of everything new and different, both inside and 
around Java (TM) technology - register by April 22, and save
$200 on the JavaOne (SM) conference, June 2-5, 2009, San Francisco.
300 plus technical and hands-on sessions. Register today. 
Use priority code J9JMT32. http://p.sf.net/sfu/p

Re: [PATCH v2 fixed] e100: do not go D3 in shutdown unless system is powering off

[Thadeu Lima de Souza Cascardo]

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

On Tue, Apr 21, 2009 at 10:46:19PM -0700, Jeff Kirsher wrote:
> On Tue, Apr 21, 2009 at 9:38 PM, Thadeu Lima de Souza Cascardo
> <cascardo@holoscopio.com> wrote:
> > After experimenting with kexec with the last merges after 2.6.29, I've
> > had some problems when probing e100. It would not read the eeprom. After
> > some bisects, I realized this has been like that since forever (at least
> > 2.6.18). The problem is that shutdown is doing the same thing that
> > suspend does and puts the device in D3 state. I couldn't find a way to
> > get the device back to a sane state in the probe function. So, based on
> > some similar patches from Rafael J. Wysocki for e1000, e1000e and ixgbe,
> > I wrote this one for e100.
> >
> > Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@holoscopio.com>
> > ---
> 
> Thanks, I will add it to my queue of patches for e100.
> 
> Also, just FYI, Auke is no longer a maintainer for
> e100/e1000/e1000e/igb/ixgb/ixgbe devices, so I have removed him from
> the thread.
> 
> -- 
> Cheers,
> Jeff

Perhaps, Rafael should be mentioned in a Reviewed-by? As well as you if
you did review it? I guess a Tested-by and Reported-by me is overkill,
right? :-)

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH v2 fixed] e100: do not go D3 in shutdown unless system is powering off

[Jeff Kirsher]

On Wed, Apr 22, 2009 at 12:37 PM, Thadeu Lima de Souza Cascardo
<cascardo@holoscopio.com> wrote:
> On Tue, Apr 21, 2009 at 10:46:19PM -0700, Jeff Kirsher wrote:
>> On Tue, Apr 21, 2009 at 9:38 PM, Thadeu Lima de Souza Cascardo
>> <cascardo@holoscopio.com> wrote:
>> > After experimenting with kexec with the last merges after 2.6.29, I've
>> > had some problems when probing e100. It would not read the eeprom. After
>> > some bisects, I realized this has been like that since forever (at least
>> > 2.6.18). The problem is that shutdown is doing the same thing that
>> > suspend does and puts the device in D3 state. I couldn't find a way to
>> > get the device back to a sane state in the probe function. So, based on
>> > some similar patches from Rafael J. Wysocki for e1000, e1000e and ixgbe,
>> > I wrote this one for e100.
>> >
>> > Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@holoscopio.com>
>> > ---
>>
>> Thanks, I will add it to my queue of patches for e100.
>>
>> Also, just FYI, Auke is no longer a maintainer for
>> e100/e1000/e1000e/igb/ixgb/ixgbe devices, so I have removed him from
>> the thread.
>>
>> --
>> Cheers,
>> Jeff
>
> Perhaps, Rafael should be mentioned in a Reviewed-by? As well as you if
> you did review it? I guess a Tested-by and Reported-by me is overkill,
> right? :-)
>

Correct, that would be overkill.  If Rafael wants his Ack or
Reviewed-by added, he will let us know.

Once we have done some regression testing on the patch, I will add my
signed-off-by before sending it to Dave.

-- 
Cheers,
Jeff

Re: [PATCH v2 fixed] e100: do not go D3 in shutdown unless system is powering off

[Jeff Kirsher]

On Wed, Apr 22, 2009 at 12:37 PM, Thadeu Lima de Souza Cascardo
<cascardo@holoscopio.com> wrote:
> On Tue, Apr 21, 2009 at 10:46:19PM -0700, Jeff Kirsher wrote:
>> On Tue, Apr 21, 2009 at 9:38 PM, Thadeu Lima de Souza Cascardo
>> <cascardo@holoscopio.com> wrote:
>> > After experimenting with kexec with the last merges after 2.6.29, I've
>> > had some problems when probing e100. It would not read the eeprom. After
>> > some bisects, I realized this has been like that since forever (at least
>> > 2.6.18). The problem is that shutdown is doing the same thing that
>> > suspend does and puts the device in D3 state. I couldn't find a way to
>> > get the device back to a sane state in the probe function. So, based on
>> > some similar patches from Rafael J. Wysocki for e1000, e1000e and ixgbe,
>> > I wrote this one for e100.
>> >
>> > Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@holoscopio.com>
>> > ---
>>
>> Thanks, I will add it to my queue of patches for e100.
>>
>> Also, just FYI, Auke is no longer a maintainer for
>> e100/e1000/e1000e/igb/ixgb/ixgbe devices, so I have removed him from
>> the thread.
>>
>> --
>> Cheers,
>> Jeff
>
> Perhaps, Rafael should be mentioned in a Reviewed-by? As well as you if
> you did review it? I guess a Tested-by and Reported-by me is overkill,
> right? :-)
>

Correct, that would be overkill.  If Rafael wants his Ack or
Reviewed-by added, he will let us know.

Once we have done some regression testing on the patch, I will add my
signed-off-by before sending it to Dave.

-- 
Cheers,
Jeff

Re: [PATCH v2 fixed] e100: do not go D3 in shutdown unless system is powering off

[Rafael J. Wysocki]

On Wednesday 22 April 2009, Jeff Kirsher wrote:
> On Wed, Apr 22, 2009 at 12:37 PM, Thadeu Lima de Souza Cascardo
> <cascardo@holoscopio.com> wrote:
> > On Tue, Apr 21, 2009 at 10:46:19PM -0700, Jeff Kirsher wrote:
> >> On Tue, Apr 21, 2009 at 9:38 PM, Thadeu Lima de Souza Cascardo
> >> <cascardo@holoscopio.com> wrote:
> >> > After experimenting with kexec with the last merges after 2.6.29, I've
> >> > had some problems when probing e100. It would not read the eeprom. After
> >> > some bisects, I realized this has been like that since forever (at least
> >> > 2.6.18). The problem is that shutdown is doing the same thing that
> >> > suspend does and puts the device in D3 state. I couldn't find a way to
> >> > get the device back to a sane state in the probe function. So, based on
> >> > some similar patches from Rafael J. Wysocki for e1000, e1000e and ixgbe,
> >> > I wrote this one for e100.
> >> >
> >> > Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@holoscopio.com>
> >> > ---
> >>
> >> Thanks, I will add it to my queue of patches for e100.
> >>
> >> Also, just FYI, Auke is no longer a maintainer for
> >> e100/e1000/e1000e/igb/ixgb/ixgbe devices, so I have removed him from
> >> the thread.
> >>
> >> --
> >> Cheers,
> >> Jeff
> >
> > Perhaps, Rafael should be mentioned in a Reviewed-by? As well as you if
> > you did review it? I guess a Tested-by and Reported-by me is overkill,
> > right? :-)
> >
> 
> Correct, that would be overkill.  If Rafael wants his Ack or
> Reviewed-by added, he will let us know.

Please add my ACK to the patch.

Best,
Rafael

Re: [PATCH v2 fixed] e100: do not go D3 in shutdown unless system is powering off

[Rafael J. Wysocki]

On Wednesday 22 April 2009, Jeff Kirsher wrote:
> On Wed, Apr 22, 2009 at 12:37 PM, Thadeu Lima de Souza Cascardo
> <cascardo@holoscopio.com> wrote:
> > On Tue, Apr 21, 2009 at 10:46:19PM -0700, Jeff Kirsher wrote:
> >> On Tue, Apr 21, 2009 at 9:38 PM, Thadeu Lima de Souza Cascardo
> >> <cascardo@holoscopio.com> wrote:
> >> > After experimenting with kexec with the last merges after 2.6.29, I've
> >> > had some problems when probing e100. It would not read the eeprom. After
> >> > some bisects, I realized this has been like that since forever (at least
> >> > 2.6.18). The problem is that shutdown is doing the same thing that
> >> > suspend does and puts the device in D3 state. I couldn't find a way to
> >> > get the device back to a sane state in the probe function. So, based on
> >> > some similar patches from Rafael J. Wysocki for e1000, e1000e and ixgbe,
> >> > I wrote this one for e100.
> >> >
> >> > Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@holoscopio.com>
> >> > ---
> >>
> >> Thanks, I will add it to my queue of patches for e100.
> >>
> >> Also, just FYI, Auke is no longer a maintainer for
> >> e100/e1000/e1000e/igb/ixgb/ixgbe devices, so I have removed him from
> >> the thread.
> >>
> >> --
> >> Cheers,
> >> Jeff
> >
> > Perhaps, Rafael should be mentioned in a Reviewed-by? As well as you if
> > you did review it? I guess a Tested-by and Reported-by me is overkill,
> > right? :-)
> >
> 
> Correct, that would be overkill.  If Rafael wants his Ack or
> Reviewed-by added, he will let us know.

Please add my ACK to the patch.

Best,
Rafael

------------------------------------------------------------------------------
Stay on top of everything new and different, both inside and 
around Java (TM) technology - register by April 22, and save
$200 on the JavaOne (SM) conference, June 2-5, 2009, San Francisco.
300 plus technical and hands-on sessions. Register today. 
Use priority code J9JMT32. http://p.sf.net/sfu/p