[PATCH 1/8] staging: et131x: Remove TODO entry 'alloc_etherdev initilising memory with zero'

[PATCH 1/8] staging: et131x: Remove TODO entry 'alloc_etherdev initilising memory with zero'

[Mark Einon]

Removing this as I'm pretty sure its not true, and alloc_etherdev isn't provided by this driver anyway. Alternatively, its a badly written comment and I don't understand it.

This drivers use of alloc_etherdev() is within keeping with other net devices, so I'm happy.

Signed-off-by: Mark Einon <mark.einon@gmail.com>
---
 drivers/staging/et131x/README |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/drivers/staging/et131x/README b/drivers/staging/et131x/README
index 7e846b2..d24ef84 100644
--- a/drivers/staging/et131x/README
+++ b/drivers/staging/et131x/README
@@ -9,7 +9,6 @@ Note, the powermanagement options were removed from the vendor provided
 driver as they did not build properly at the time.
 
 TODO:
-	- alloc_etherdev is initializing memory with zero?!?
 	- add_timer call in et131x_netdev.c is correct?
 	- Add power saving functionality (suspend, sleep, resume)
 	- Implement a few more kernel Parameter (set mac )
-- 
1.7.6.4

[PATCH 2/8] staging: et131x: Introduce et1310_in_phy_coma() call

[Mark Einon]

In several places in the code, the pm_csr register is read and the PHY_SW_COMA bit checked.

Move this check into its own small function to make the code more readable.

Signed-off-by: Mark Einon <mark.einon@gmail.com>
---
 drivers/staging/et131x/et1310_mac.c     |    4 ++--
 drivers/staging/et131x/et1310_pm.c      |   15 +++++++++++++++
 drivers/staging/et131x/et131x.h         |    1 +
 drivers/staging/et131x/et131x_initpci.c |   28 +++++++++-------------------
 drivers/staging/et131x/et131x_isr.c     |    2 +-
 5 files changed, 28 insertions(+), 22 deletions(-)

diff --git a/drivers/staging/et131x/et1310_mac.c b/drivers/staging/et131x/et1310_mac.c
index 08d7691..157462f 100644
--- a/drivers/staging/et131x/et1310_mac.c
+++ b/drivers/staging/et131x/et1310_mac.c
@@ -612,7 +612,7 @@ void et1310_setup_device_for_multicast(struct et131x_adapter *adapter)
 
 	/* Write out the new hash to the device */
 	pm_csr = readl(&adapter->regs->global.pm_csr);
-	if ((pm_csr & ET_PM_PHY_SW_COMA) == 0) {
+	if (!et1310_in_phy_coma(adapter)) {
 		writel(hash1, &rxmac->multi_hash1);
 		writel(hash2, &rxmac->multi_hash2);
 		writel(hash3, &rxmac->multi_hash3);
@@ -653,7 +653,7 @@ void et1310_setup_device_for_unicast(struct et131x_adapter *adapter)
 		   adapter->addr[5];
 
 	pm_csr = readl(&adapter->regs->global.pm_csr);
-	if ((pm_csr & ET_PM_PHY_SW_COMA) == 0) {
+	if (!et1310_in_phy_coma(adapter)) {
 		writel(uni_pf1, &rxmac->uni_pf_addr1);
 		writel(uni_pf2, &rxmac->uni_pf_addr2);
 		writel(uni_pf3, &rxmac->uni_pf_addr3);
diff --git a/drivers/staging/et131x/et1310_pm.c b/drivers/staging/et131x/et1310_pm.c
index b20d5d6..55f2d36 100644
--- a/drivers/staging/et131x/et1310_pm.c
+++ b/drivers/staging/et131x/et1310_pm.c
@@ -87,6 +87,21 @@
 #include "et131x.h"
 
 /**
+ * et1310_in_phy_coma - check if the device is in phy coma
+ * @adapter: pointer to our adapter structure
+ *
+ * Returns 0 if the device is not in phy coma, 1 if it is in phy coma
+ */
+int et1310_in_phy_coma(struct et131x_adapter *adapter)
+{
+	u32 pmcsr;
+
+	pmcsr = readl(&adapter->regs->global.pm_csr);
+
+	return ET_PM_PHY_SW_COMA & pmcsr ? 1 : 0;
+}
+
+/**
  * et1310_enable_phy_coma - called when network cable is unplugged
  * @adapter: pointer to our adapter structure
  *
diff --git a/drivers/staging/et131x/et131x.h b/drivers/staging/et131x/et131x.h
index c047e6e..d967c5a1 100644
--- a/drivers/staging/et131x/et131x.h
+++ b/drivers/staging/et131x/et131x.h
@@ -87,6 +87,7 @@ void et131x_enable_txrx(struct net_device *netdev);
 void et131x_disable_txrx(struct net_device *netdev);
 
 /* et1310_pm.c */
+int et1310_in_phy_coma(struct et131x_adapter *adapter);
 void et1310_enable_phy_coma(struct et131x_adapter *adapter);
 void et1310_disable_phy_coma(struct et131x_adapter *adapter);
 
diff --git a/drivers/staging/et131x/et131x_initpci.c b/drivers/staging/et131x/et131x_initpci.c
index e9daa73..667ab80 100644
--- a/drivers/staging/et131x/et131x_initpci.c
+++ b/drivers/staging/et131x/et131x_initpci.c
@@ -239,15 +239,11 @@ static int et131x_pci_init(struct et131x_adapter *adapter,
 void et131x_error_timer_handler(unsigned long data)
 {
 	struct et131x_adapter *adapter = (struct et131x_adapter *) data;
-	u32 pm_csr;
 
-	pm_csr = readl(&adapter->regs->global.pm_csr);
-
-	if ((pm_csr & ET_PM_PHY_SW_COMA) == 0)
+	if (!et1310_in_phy_coma(adapter))
 		et1310_update_macstat_host_counters(adapter);
 	else
-		dev_err(&adapter->pdev->dev,
-		    "No interrupts, in PHY coma, pm_csr = 0x%x\n", pm_csr);
+		dev_err(&adapter->pdev->dev, "No interrupts, in PHY coma\n");
 
 	if (!(adapter->bmsr & BMSR_LSTATUS) &&
 	    adapter->boot_coma < 11) {
@@ -256,7 +252,7 @@ void et131x_error_timer_handler(unsigned long data)
 
 	if (adapter->boot_coma == 10) {
 		if (!(adapter->bmsr & BMSR_LSTATUS)) {
-			if ((pm_csr & ET_PM_PHY_SW_COMA) == 0) {
+			if (!et1310_in_phy_coma(adapter)) {
 				/* NOTE - This was originally a 'sync with
 				 *  interrupt'. How to do that under Linux?
 				 */
@@ -443,9 +439,6 @@ static void et131x_adjust_link(struct net_device *netdev)
 {
 	struct et131x_adapter *adapter = netdev_priv(netdev);
 	struct  phy_device *phydev = adapter->phydev;
-	struct address_map __iomem *iomem = adapter->regs;
-
-	u32 pm_csr;
 
 	if (netif_carrier_ok(netdev)) {
 		adapter->boot_coma = 20;
@@ -488,16 +481,13 @@ static void et131x_adjust_link(struct net_device *netdev)
 	}
 
 	if (phydev->link != adapter->link) {
-		/* If we are in coma mode, we need to disable it. */
-		pm_csr = readl(&iomem->global.pm_csr);
-		if (pm_csr & ET_PM_PHY_SW_COMA) {
-			/*
-			 * Check to see if we are in coma mode and if
-			 * so, disable it because we will not be able
-			 * to read PHY values until we are out.
-			 */
+		/*
+		 * Check to see if we are in coma mode and if
+		 * so, disable it because we will not be able
+		 * to read PHY values until we are out.
+		 */
+		if (et1310_in_phy_coma(adapter))
 			et1310_disable_phy_coma(adapter);
-		}
 
 		if (phydev->link) {
 			adapter->boot_coma = 20;
diff --git a/drivers/staging/et131x/et131x_isr.c b/drivers/staging/et131x/et131x_isr.c
index 0747935..fb108d2 100644
--- a/drivers/staging/et131x/et131x_isr.c
+++ b/drivers/staging/et131x/et131x_isr.c
@@ -304,7 +304,7 @@ void et131x_isr_handler(struct work_struct *work)
 				 * bp xon/xoff)
 				 */
 				pm_csr = readl(&iomem->global.pm_csr);
-				if ((pm_csr & ET_PM_PHY_SW_COMA) == 0)
+				if (!et1310_in_phy_coma(adapter))
 					writel(3, &iomem->txmac.bp_ctrl);
 			}
 		}
-- 
1.7.6.4

[PATCH 3/8] staging: et131x: Remove adapter->bmsr, replace with phydev equivalents

[Mark Einon]

adapter->bmsr is no longer being updated, but is also used to check the link state in places.

Remove bmsr from adapter, and replace link state checks with phydev->link check.

Signed-off-by: Mark Einon <mark.einon@gmail.com>
---
 drivers/staging/et131x/et1310_phy.c     |    3 ---
 drivers/staging/et131x/et131x_adapter.h |    3 ---
 drivers/staging/et131x/et131x_initpci.c |    7 +++----
 3 files changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/et131x/et1310_phy.c b/drivers/staging/et131x/et1310_phy.c
index fc37b18..a72046b 100644
--- a/drivers/staging/et131x/et1310_phy.c
+++ b/drivers/staging/et131x/et1310_phy.c
@@ -337,9 +337,6 @@ void et131x_xcvr_init(struct et131x_adapter *adapter)
 	u16 isr;
 	u16 lcr2;
 
-	/* Zero out the adapter structure variable representing BMSR */
-	adapter->bmsr = 0;
-
 	et131x_mii_read(adapter, PHY_INTERRUPT_STATUS, &isr);
 	et131x_mii_read(adapter, PHY_INTERRUPT_MASK, &imr);
 
diff --git a/drivers/staging/et131x/et131x_adapter.h b/drivers/staging/et131x/et131x_adapter.h
index fabc2c5..b5195ef 100644
--- a/drivers/staging/et131x/et131x_adapter.h
+++ b/drivers/staging/et131x/et131x_adapter.h
@@ -201,9 +201,6 @@ struct et131x_adapter {
 	u16 pdown_speed;
 	u8 pdown_duplex;
 
-	/* Xcvr status at last poll */
-	u16 bmsr;
-
 	/* Tx Memory Variables */
 	struct tx_ring tx_ring;
 
diff --git a/drivers/staging/et131x/et131x_initpci.c b/drivers/staging/et131x/et131x_initpci.c
index 667ab80..10283a3 100644
--- a/drivers/staging/et131x/et131x_initpci.c
+++ b/drivers/staging/et131x/et131x_initpci.c
@@ -239,19 +239,18 @@ static int et131x_pci_init(struct et131x_adapter *adapter,
 void et131x_error_timer_handler(unsigned long data)
 {
 	struct et131x_adapter *adapter = (struct et131x_adapter *) data;
+	struct phy_device *phydev = adapter->phydev;
 
 	if (!et1310_in_phy_coma(adapter))
 		et1310_update_macstat_host_counters(adapter);
 	else
 		dev_err(&adapter->pdev->dev, "No interrupts, in PHY coma\n");
 
-	if (!(adapter->bmsr & BMSR_LSTATUS) &&
-	    adapter->boot_coma < 11) {
+	if (!phydev->link && adapter->boot_coma < 11)
 		adapter->boot_coma++;
-	}
 
 	if (adapter->boot_coma == 10) {
-		if (!(adapter->bmsr & BMSR_LSTATUS)) {
+		if (!phydev->link) {
 			if (!et1310_in_phy_coma(adapter)) {
 				/* NOTE - This was originally a 'sync with
 				 *  interrupt'. How to do that under Linux?
-- 
1.7.6.4

[PATCH 4/8] staging: et131x: Add pci suspend & resume functions

[Mark Einon]

Added basic suspend & resume functionality.
Tested on an et1310 device, and putting Fedora15 host in and out of
hibernation successfully.

Signed-off-by: Mark Einon <mark.einon@gmail.com>
---
 drivers/staging/et131x/README           |    1 -
 drivers/staging/et131x/et131x.h         |    2 +
 drivers/staging/et131x/et131x_initpci.c |   45 +++++++++++++++++++++++++++++-
 3 files changed, 45 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/et131x/README b/drivers/staging/et131x/README
index d24ef84..42c388d 100644
--- a/drivers/staging/et131x/README
+++ b/drivers/staging/et131x/README
@@ -10,7 +10,6 @@ driver as they did not build properly at the time.
 
 TODO:
 	- add_timer call in et131x_netdev.c is correct?
-	- Add power saving functionality (suspend, sleep, resume)
 	- Implement a few more kernel Parameter (set mac )
 
 Please send patches to:
diff --git a/drivers/staging/et131x/et131x.h b/drivers/staging/et131x/et131x.h
index d967c5a1..15486d1 100644
--- a/drivers/staging/et131x/et131x.h
+++ b/drivers/staging/et131x/et131x.h
@@ -82,6 +82,8 @@ void et1310_setup_device_for_multicast(struct et131x_adapter *adapter);
 void et1310_setup_device_for_unicast(struct et131x_adapter *adapter);
 
 /* et131x_netdev.c */
+int et131x_open(struct net_device *netdev);
+int et131x_close(struct net_device *netdev);
 struct net_device *et131x_device_alloc(void);
 void et131x_enable_txrx(struct net_device *netdev);
 void et131x_disable_txrx(struct net_device *netdev);
diff --git a/drivers/staging/et131x/et131x_initpci.c b/drivers/staging/et131x/et131x_initpci.c
index 10283a3..e166fde 100644
--- a/drivers/staging/et131x/et131x_initpci.c
+++ b/drivers/staging/et131x/et131x_initpci.c
@@ -869,6 +869,45 @@ static void __devexit et131x_pci_remove(struct pci_dev *pdev)
 	pci_disable_device(pdev);
 }
 
+static int et131x_pci_suspend(struct pci_dev *pdev, pm_message_t state)
+{
+	struct net_device *netdev = pci_get_drvdata(pdev);
+
+	if (!netif_running(netdev))
+		return 0;
+
+	et131x_close(netdev);
+	netif_device_detach(netdev);
+
+	pci_save_state(pdev);
+	pci_set_power_state(pdev, pci_choose_state(pdev, state));
+
+	return 0;
+}
+
+static int et131x_pci_resume(struct pci_dev *pdev)
+{
+	struct net_device *netdev = pci_get_drvdata(pdev);
+	int err = 0;
+
+	if (!netif_running(netdev))
+		return 0;
+
+	pci_set_power_state(pdev, PCI_D0);
+	pci_restore_state(pdev);
+
+	err = et131x_open(netdev);
+	if (err) {
+		dev_err(&pdev->dev, "Can't resume interface!\n");
+		goto out;
+	}
+
+	netif_device_attach(netdev);
+
+out:
+	return err;
+}
+
 static struct pci_device_id et131x_pci_table[] __devinitdata = {
 	{ET131X_PCI_VENDOR_ID, ET131X_PCI_DEVICE_ID_GIG, PCI_ANY_ID,
 	 PCI_ANY_ID, 0, 0, 0UL},
@@ -884,8 +923,10 @@ static struct pci_driver et131x_driver = {
 	.id_table	= et131x_pci_table,
 	.probe		= et131x_pci_setup,
 	.remove		= __devexit_p(et131x_pci_remove),
-	.suspend	= NULL,		/* et131x_pci_suspend */
-	.resume		= NULL,		/* et131x_pci_resume */
+#ifdef CONFIG_PM
+	.suspend	= et131x_pci_suspend,
+	.resume		= et131x_pci_resume,
+#endif
 };
 
 /**
-- 
1.7.6.4

Re: [PATCH 4/8] staging: et131x: Add pci suspend & resume functions

[Francois Romieu]

Mark Einon <mark.einon@gmail.com> :
> Added basic suspend & resume functionality.
> Tested on an et1310 device, and putting Fedora15 host in and out of
> hibernation successfully.
> 
> Signed-off-by: Mark Einon <mark.einon@gmail.com>
[...]
> diff --git a/drivers/staging/et131x/et131x_initpci.c b/drivers/staging/et131x/et131x_initpci.c
> index 10283a3..e166fde 100644
> --- a/drivers/staging/et131x/et131x_initpci.c
> +++ b/drivers/staging/et131x/et131x_initpci.c
> @@ -869,6 +869,45 @@ static void __devexit et131x_pci_remove(struct pci_dev *pdev)
>  	pci_disable_device(pdev);
>  }
>  
> +static int et131x_pci_suspend(struct pci_dev *pdev, pm_message_t state)
> +{
> +	struct net_device *netdev = pci_get_drvdata(pdev);
> +
> +	if (!netif_running(netdev))
> +		return 0;
> +
> +	et131x_close(netdev);
> +	netif_device_detach(netdev);
> +
> +	pci_save_state(pdev);
> +	pci_set_power_state(pdev, pci_choose_state(pdev, state));
> +
> +	return 0;
> +}
> +
> +static int et131x_pci_resume(struct pci_dev *pdev)
> +{
> +	struct net_device *netdev = pci_get_drvdata(pdev);
> +	int err = 0;
> +
> +	if (!netif_running(netdev))
> +		return 0;
> +
> +	pci_set_power_state(pdev, PCI_D0);
> +	pci_restore_state(pdev);
> +
> +	err = et131x_open(netdev);
> +	if (err) {
> +		dev_err(&pdev->dev, "Can't resume interface!\n");
> +		goto out;
> +	}

You can/should split et131x_{open/close} to remove this failure point.

[...]
> @@ -884,8 +923,10 @@ static struct pci_driver et131x_driver = {
>  	.id_table	= et131x_pci_table,
>  	.probe		= et131x_pci_setup,
>  	.remove		= __devexit_p(et131x_pci_remove),
> -	.suspend	= NULL,		/* et131x_pci_suspend */
> -	.resume		= NULL,		/* et131x_pci_resume */
> +#ifdef CONFIG_PM
> +	.suspend	= et131x_pci_suspend,
> +	.resume		= et131x_pci_resume,
> +#endif

There is not much driver specific there. It should probably go through
pci_driver.driver.pm.

-- 
Ueimor

Re: [PATCH 4/8] staging: et131x: Add pci suspend & resume functions

[Mark Einon]

On 1 October 2011 12:03, Francois Romieu <romieu@fr.zoreil.com> wrote:
> Mark Einon <mark.einon@gmail.com> :
>> Added basic suspend & resume functionality.
>> Tested on an et1310 device, and putting Fedora15 host in and out of
>> hibernation successfully.
---
>> +static int et131x_pci_resume(struct pci_dev *pdev)
>> +{
>> +     struct net_device *netdev = pci_get_drvdata(pdev);
>> +     int err = 0;
>> +
>> +     if (!netif_running(netdev))
>> +             return 0;
>> +
>> +     pci_set_power_state(pdev, PCI_D0);
>> +     pci_restore_state(pdev);
>> +
>> +     err = et131x_open(netdev);
>> +     if (err) {
>> +             dev_err(&pdev->dev, "Can't resume interface!\n");
>> +             goto out;
>> +     }
>
> You can/should split et131x_{open/close} to remove this failure point.

Hi, thanks for the comments francois,
I'm not quite sure what you mean here. Can you expand on your comment please?

>
> [...]
>> @@ -884,8 +923,10 @@ static struct pci_driver et131x_driver = {
>>       .id_table       = et131x_pci_table,
>>       .probe          = et131x_pci_setup,
>>       .remove         = __devexit_p(et131x_pci_remove),
>> -     .suspend        = NULL,         /* et131x_pci_suspend */
>> -     .resume         = NULL,         /* et131x_pci_resume */
>> +#ifdef CONFIG_PM
>> +     .suspend        = et131x_pci_suspend,
>> +     .resume         = et131x_pci_resume,
>> +#endif
>
> There is not much driver specific there. It should probably go through
> pci_driver.driver.pm.

Ok, understood. I'll do this.

>
> --
> Ueimor
>

Re: [PATCH 4/8] staging: et131x: Add pci suspend & resume functions

[Francois Romieu]

Mark Einon <mark.einon@gmail.com> :
> On 1 October 2011 12:03, Francois Romieu <romieu@fr.zoreil.com> wrote:
[...]
> >> + ? ? err = et131x_open(netdev);
> >> + ? ? if (err) {
> >> + ? ? ? ? ? ? dev_err(&pdev->dev, "Can't resume interface!\n");
> >> + ? ? ? ? ? ? goto out;
> >> + ? ? }
> >
> > You can/should split et131x_{open/close} to remove this failure point.
> 
> Hi, thanks for the comments francois,
> I'm not quite sure what you mean here. Can you expand on your comment please?

Something like:

int et131x_open(struct net_device *netdev)
{
	add_timer(...)

	err = request_irq(...)
	if (err < 0)
		goto release_timer;

	et131x_start(...)

out:
	return err;

release_timer:
	blah;
	goto out;
}

Same thing with et131x_close / et131x_stop. You only use et131x_{start/stop}
for suspend/resume : one error path is gone away.

-- 
Ueimor

[PATCH 5/8] staging: et131x: Fix add_timer() from et131x_open

[Mark Einon]

The error_timer was only getting initialised and an initial jiffies value set following a probe.
This could result in the timer needlessly expiring immediately after et131x_open is called.
Now this is all done from the open call instead.

Signed-off-by: Mark Einon <mark.einon@gmail.com>
---
 drivers/staging/et131x/README           |    1 -
 drivers/staging/et131x/et131x.h         |    1 +
 drivers/staging/et131x/et131x_initpci.c |    7 -------
 drivers/staging/et131x/et131x_netdev.c  |    4 ++++
 4 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/et131x/README b/drivers/staging/et131x/README
index 42c388d..a0ebaa0 100644
--- a/drivers/staging/et131x/README
+++ b/drivers/staging/et131x/README
@@ -9,7 +9,6 @@ Note, the powermanagement options were removed from the vendor provided
 driver as they did not build properly at the time.
 
 TODO:
-	- add_timer call in et131x_netdev.c is correct?
 	- Implement a few more kernel Parameter (set mac )
 
 Please send patches to:
diff --git a/drivers/staging/et131x/et131x.h b/drivers/staging/et131x/et131x.h
index 15486d1..c8f5ab1 100644
--- a/drivers/staging/et131x/et131x.h
+++ b/drivers/staging/et131x/et131x.h
@@ -52,6 +52,7 @@
 int et131x_init_eeprom(struct et131x_adapter *adapter);
 
 /* et131x_initpci.c */
+void et131x_error_timer_handler(unsigned long data);
 void et131x_configure_global_regs(struct et131x_adapter *adapter);
 void et131x_enable_interrupts(struct et131x_adapter *adapter);
 void et131x_disable_interrupts(struct et131x_adapter *adapter);
diff --git a/drivers/staging/et131x/et131x_initpci.c b/drivers/staging/et131x/et131x_initpci.c
index e166fde..8bff4a0 100644
--- a/drivers/staging/et131x/et131x_initpci.c
+++ b/drivers/staging/et131x/et131x_initpci.c
@@ -750,10 +750,6 @@ static int __devinit et131x_pci_setup(struct pci_dev *pdev,
 	/* Copy address into the net_device struct */
 	memcpy(netdev->dev_addr, adapter->addr, ETH_ALEN);
 
-	adapter->error_timer.expires = jiffies + TX_ERROR_PERIOD * HZ / 1000;
-	adapter->error_timer.function = et131x_error_timer_handler;
-	adapter->error_timer.data = (unsigned long)adapter;
-
 	/* Init variable for counting how long we do not have link status */
 	adapter->boot_coma = 0;
 	et1310_disable_phy_coma(adapter);
@@ -795,9 +791,6 @@ static int __devinit et131x_pci_setup(struct pci_dev *pdev,
 	/* Setup et1310 as per the documentation */
 	et131x_adapter_setup(adapter);
 
-	/* Create a timer to count errors received by the NIC */
-	init_timer(&adapter->error_timer);
-
 	/* We can enable interrupts now
 	 *
 	 *  NOTE - Because registration of interrupt handler is done in the
diff --git a/drivers/staging/et131x/et131x_netdev.c b/drivers/staging/et131x/et131x_netdev.c
index 6ec4a0f..2184682 100644
--- a/drivers/staging/et131x/et131x_netdev.c
+++ b/drivers/staging/et131x/et131x_netdev.c
@@ -187,6 +187,10 @@ int et131x_open(struct net_device *netdev)
 	struct et131x_adapter *adapter = netdev_priv(netdev);
 
 	/* Start the timer to track NIC errors */
+	init_timer(&adapter->error_timer);
+	adapter->error_timer.expires = jiffies + TX_ERROR_PERIOD * HZ / 1000;
+	adapter->error_timer.function = et131x_error_timer_handler;
+	adapter->error_timer.data = (unsigned long)adapter;
 	add_timer(&adapter->error_timer);
 
 	/* Register our IRQ */
-- 
1.7.6.4

[PATCH 6/8] staging: et131x: Update TODO file for kernel parameters

[Mark Einon]

set_mac is implemented, and there have been lots of new ethtool_ops added, so
removing the TODO to add more kernel parameters.

Signed-off-by: Mark Einon <mark.einon@gmail.com>
---
 drivers/staging/et131x/README |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/drivers/staging/et131x/README b/drivers/staging/et131x/README
index a0ebaa0..b262ec7 100644
--- a/drivers/staging/et131x/README
+++ b/drivers/staging/et131x/README
@@ -9,7 +9,6 @@ Note, the powermanagement options were removed from the vendor provided
 driver as they did not build properly at the time.
 
 TODO:
-	- Implement a few more kernel Parameter (set mac )
 
 Please send patches to:
 	Greg Kroah-Hartman <gregkh@suse.de>
-- 
1.7.6.4

[PATCH 7/8] staging: et131x: Updating copyright statements and module authors

[Mark Einon]

Adding copyright notices and adding myself as a module author.

Signed-off-by: Mark Einon <mark.einon@gmail.com>
---
 drivers/staging/et131x/et1310_address_map.h |    2 ++
 drivers/staging/et131x/et1310_eeprom.c      |    2 ++
 drivers/staging/et131x/et1310_mac.c         |    2 ++
 drivers/staging/et131x/et1310_phy.c         |    2 ++
 drivers/staging/et131x/et1310_phy.h         |    2 ++
 drivers/staging/et131x/et1310_pm.c          |    2 ++
 drivers/staging/et131x/et1310_rx.c          |    2 ++
 drivers/staging/et131x/et1310_rx.h          |    2 ++
 drivers/staging/et131x/et1310_tx.c          |    2 ++
 drivers/staging/et131x/et131x_adapter.h     |    2 ++
 drivers/staging/et131x/et131x_defs.h        |    2 ++
 drivers/staging/et131x/et131x_ethtool.c     |    4 +---
 drivers/staging/et131x/et131x_initpci.c     |    3 +++
 drivers/staging/et131x/et131x_isr.c         |    2 ++
 drivers/staging/et131x/et131x_netdev.c      |    2 ++
 drivers/staging/et131x/et131x_version.h     |    1 +
 16 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/et131x/et1310_address_map.h b/drivers/staging/et131x/et1310_address_map.h
index 38ec56c..b502ce3 100644
--- a/drivers/staging/et131x/et1310_address_map.h
+++ b/drivers/staging/et131x/et1310_address_map.h
@@ -6,6 +6,8 @@
  * All rights reserved.
  *   http://www.agere.com
  *
+ * Copyright (c) 2011 Mark Einon <mark.einon@gmail.com>
+ *
  *------------------------------------------------------------------------------
  *
  * et1310_address_map.h - Contains the register mapping for the ET1310
diff --git a/drivers/staging/et131x/et1310_eeprom.c b/drivers/staging/et131x/et1310_eeprom.c
index e810254..747b1dc 100644
--- a/drivers/staging/et131x/et1310_eeprom.c
+++ b/drivers/staging/et131x/et1310_eeprom.c
@@ -6,6 +6,8 @@
  * All rights reserved.
  *   http://www.agere.com
  *
+ * Copyright (c) 2011 Mark Einon <mark.einon@gmail.com>
+ *
  *------------------------------------------------------------------------------
  *
  * et1310_eeprom.c - Code used to access the device's EEPROM
diff --git a/drivers/staging/et131x/et1310_mac.c b/drivers/staging/et131x/et1310_mac.c
index 157462f..b4c9df1 100644
--- a/drivers/staging/et131x/et1310_mac.c
+++ b/drivers/staging/et131x/et1310_mac.c
@@ -6,6 +6,8 @@
  * All rights reserved.
  *   http://www.agere.com
  *
+ * Copyright (c) 2011 Mark Einon <mark.einon@gmail.com>
+ *
  *------------------------------------------------------------------------------
  *
  * et1310_mac.c - All code and routines pertaining to the MAC
diff --git a/drivers/staging/et131x/et1310_phy.c b/drivers/staging/et131x/et1310_phy.c
index a72046b..145ec76 100644
--- a/drivers/staging/et131x/et1310_phy.c
+++ b/drivers/staging/et131x/et1310_phy.c
@@ -6,6 +6,8 @@
  * All rights reserved.
  *   http://www.agere.com
  *
+ * Copyright (c) 2011 Mark Einon <mark.einon@gmail.com>
+ *
  *------------------------------------------------------------------------------
  *
  * et1310_phy.c - Routines for configuring and accessing the PHY
diff --git a/drivers/staging/et131x/et1310_phy.h b/drivers/staging/et131x/et1310_phy.h
index 82037ac..2803bdf 100644
--- a/drivers/staging/et131x/et1310_phy.h
+++ b/drivers/staging/et131x/et1310_phy.h
@@ -6,6 +6,8 @@
  * All rights reserved.
  *   http://www.agere.com
  *
+ * Copyright (c) 2011 Mark Einon <mark.einon@gmail.com>
+ *
  *------------------------------------------------------------------------------
  *
  * et1310_phy.h - Defines, structs, enums, prototypes, etc. pertaining to the
diff --git a/drivers/staging/et131x/et1310_pm.c b/drivers/staging/et131x/et1310_pm.c
index 55f2d36..fd1eb6f 100644
--- a/drivers/staging/et131x/et1310_pm.c
+++ b/drivers/staging/et131x/et1310_pm.c
@@ -6,6 +6,8 @@
  * All rights reserved.
  *   http://www.agere.com
  *
+ * Copyright (c) 2011 Mark Einon <mark.einon@gmail.com>
+ *
  *------------------------------------------------------------------------------
  *
  * et1310_pm.c - All power management related code (not completely implemented)
diff --git a/drivers/staging/et131x/et1310_rx.c b/drivers/staging/et131x/et1310_rx.c
index c402c5e..600a8f1 100644
--- a/drivers/staging/et131x/et1310_rx.c
+++ b/drivers/staging/et131x/et1310_rx.c
@@ -6,6 +6,8 @@
  * All rights reserved.
  *   http://www.agere.com
  *
+ * Copyright (c) 2011 Mark Einon <mark.einon@gmail.com>
+ *
  *------------------------------------------------------------------------------
  *
  * et1310_rx.c - Routines used to perform data reception
diff --git a/drivers/staging/et131x/et1310_rx.h b/drivers/staging/et131x/et1310_rx.h
index 1448aa9..a19fab9 100644
--- a/drivers/staging/et131x/et1310_rx.h
+++ b/drivers/staging/et131x/et1310_rx.h
@@ -6,6 +6,8 @@
  * All rights reserved.
  *   http://www.agere.com
  *
+ * Copyright (c) 2011 Mark Einon <mark.einon@gmail.com>
+ *
  *------------------------------------------------------------------------------
  *
  * et1310_rx.h - Defines, structs, enums, prototypes, etc. pertaining to data
diff --git a/drivers/staging/et131x/et1310_tx.c b/drivers/staging/et131x/et1310_tx.c
index 1f80651..2ecb934 100644
--- a/drivers/staging/et131x/et1310_tx.c
+++ b/drivers/staging/et131x/et1310_tx.c
@@ -6,6 +6,8 @@
  * All rights reserved.
  *   http://www.agere.com
  *
+ * Copyright (c) 2011 Mark Einon <mark.einon@gmail.com>
+ *
  *------------------------------------------------------------------------------
  *
  * et1310_tx.c - Routines used to perform data transmission.
diff --git a/drivers/staging/et131x/et131x_adapter.h b/drivers/staging/et131x/et131x_adapter.h
index b5195ef..b062e6d 100644
--- a/drivers/staging/et131x/et131x_adapter.h
+++ b/drivers/staging/et131x/et131x_adapter.h
@@ -6,6 +6,8 @@
  * All rights reserved.
  *   http://www.agere.com
  *
+ * Copyright (c) 2011 Mark Einon <mark.einon@gmail.com>
+ *
  *------------------------------------------------------------------------------
  *
  * et131x_adapter.h - Header which includes the private adapter structure, along
diff --git a/drivers/staging/et131x/et131x_defs.h b/drivers/staging/et131x/et131x_defs.h
index 3d5193f..872a5af 100644
--- a/drivers/staging/et131x/et131x_defs.h
+++ b/drivers/staging/et131x/et131x_defs.h
@@ -6,6 +6,8 @@
  * All rights reserved.
  *   http://www.agere.com
  *
+ * Copyright (c) 2011 Mark Einon <mark.einon@gmail.com>
+ *
  *------------------------------------------------------------------------------
  *
  * et131x_defs.h - Defines, structs, enums, prototypes, etc. to assist with OS
diff --git a/drivers/staging/et131x/et131x_ethtool.c b/drivers/staging/et131x/et131x_ethtool.c
index c4555b9..903b5a0 100644
--- a/drivers/staging/et131x/et131x_ethtool.c
+++ b/drivers/staging/et131x/et131x_ethtool.c
@@ -1,5 +1,5 @@
 /*
- *  Copyright (C) 2011 Mark Einon
+ *  Copyright (c) 2011 Mark Einon <mark.einon@gmail.com>
  *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -14,8 +14,6 @@
  *  You should have received a copy of the GNU General Public License
  *  along with this program; if not, write to the Free Software
  *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
- *
- *  Mark Einon <mark.einon@gmail.com>
  */
 #include "et131x_version.h"
 #include "et131x_defs.h"
diff --git a/drivers/staging/et131x/et131x_initpci.c b/drivers/staging/et131x/et131x_initpci.c
index 8bff4a0..ed22615 100644
--- a/drivers/staging/et131x/et131x_initpci.c
+++ b/drivers/staging/et131x/et131x_initpci.c
@@ -6,6 +6,8 @@
  * All rights reserved.
  *   http://www.agere.com
  *
+ * Copyright (c) 2011 Mark Einon <mark.einon@gmail.com>
+ *
  *------------------------------------------------------------------------------
  *
  * et131x_initpci.c - Routines and data used to register the driver with the
@@ -945,5 +947,6 @@ module_exit(et131x_cleanup_module);
 
 /* Modinfo parameters (filled out using defines from et131x_version.h) */
 MODULE_AUTHOR(DRIVER_AUTHOR);
+MODULE_AUTHOR(DRIVER_AUTHOR2);
 MODULE_DESCRIPTION(DRIVER_INFO);
 MODULE_LICENSE(DRIVER_LICENSE);
diff --git a/drivers/staging/et131x/et131x_isr.c b/drivers/staging/et131x/et131x_isr.c
index fb108d2..bccd0e5 100644
--- a/drivers/staging/et131x/et131x_isr.c
+++ b/drivers/staging/et131x/et131x_isr.c
@@ -6,6 +6,8 @@
  * All rights reserved.
  *   http://www.agere.com
  *
+ * Copyright (c) 2011 Mark Einon <mark.einon@gmail.com>
+ *
  *------------------------------------------------------------------------------
  *
  * et131x_isr.c - File which contains the ISR, ISR handler, and related routines
diff --git a/drivers/staging/et131x/et131x_netdev.c b/drivers/staging/et131x/et131x_netdev.c
index 2184682..79127de 100644
--- a/drivers/staging/et131x/et131x_netdev.c
+++ b/drivers/staging/et131x/et131x_netdev.c
@@ -6,6 +6,8 @@
  * All rights reserved.
  *   http://www.agere.com
  *
+ * Copyright (c) 2011 Mark Einon <mark.einon@gmail.com>
+ *
  *------------------------------------------------------------------------------
  *
  * et131x_netdev.c - Routines and data required by all Linux network devices.
diff --git a/drivers/staging/et131x/et131x_version.h b/drivers/staging/et131x/et131x_version.h
index 2aa9bda..c08b5e5 100644
--- a/drivers/staging/et131x/et131x_version.h
+++ b/drivers/staging/et131x/et131x_version.h
@@ -59,6 +59,7 @@
 #define __ET131X_VERSION_H__
 
 #define DRIVER_AUTHOR		"Victor Soriano (vjsoriano@agere.com)"
+#define DRIVER_AUTHOR2		"Mark Einon (mark.einon@gmail.com)"
 #define DRIVER_LICENSE		"Dual BSD/GPL"
 #define DRIVER_DEVICE_STRING	"ET1310"
 #define DRIVER_NAME		"et131x"
-- 
1.7.6.4

Re: [PATCH 7/8] staging: et131x: Updating copyright statements and module authors

[Dan Carpenter]

On Sat, Oct 01, 2011 at 11:14:16AM +0100, Mark Einon wrote:
> @@ -945,5 +947,6 @@ module_exit(et131x_cleanup_module);
>  
>  /* Modinfo parameters (filled out using defines from et131x_version.h) */
>  MODULE_AUTHOR(DRIVER_AUTHOR);
> +MODULE_AUTHOR(DRIVER_AUTHOR2);
>  MODULE_DESCRIPTION(DRIVER_INFO);
>  MODULE_LICENSE(DRIVER_LICENSE);

Both DRIVER_AUTHOR and DRIVER_AUTHOR2 are silly macros.  Put the name
in directly.  That way you won't need the comment explaining where to
find the definitions.  Also normally people use gt lt brackets for
emails:

MODULE_AUTHOR("Victor Soriano <vjsoriano@agere.com>");
MODULE_AUTHOR("Mark Einon <mark.einon@gmail.com>");

Go ahead and fix this in a later patch if you want.  All the macros
there should probably be replaced.

Btw, the problems with kernel.org have complicated things here, so
I can't tell if you have already fixed this, but the driver doesn't
compile any more.  The Kconfig file says it "depends on NETDEV_1000"
but NETDEV_1000 isn't around any more since f860b0522f65d3
"drivers/net: Kconfig and Makefile cleanup" was merged.

regards,
dan carpenter

Re: [PATCH 7/8] staging: et131x: Updating copyright statements and module authors

[Mark Einon]

On 1 October 2011 20:58, Dan Carpenter <dan.carpenter@oracle.com>
wrote:> On Sat, Oct 01, 2011 at 11:14:16AM +0100, Mark Einon wrote:>>
@@ -945,5 +947,6 @@ module_exit(et131x_cleanup_module);>>>>  /*
Modinfo parameters (filled out using defines from et131x_version.h)
*/>>  MODULE_AUTHOR(DRIVER_AUTHOR);>>
+MODULE_AUTHOR(DRIVER_AUTHOR2);>>  MODULE_DESCRIPTION(DRIVER_INFO);>>
MODULE_LICENSE(DRIVER_LICENSE);>> Both DRIVER_AUTHOR and
DRIVER_AUTHOR2 are silly macros.  Put the name> in directly.  That way
you won't need the comment explaining where to> find the definitions.
Also normally people use gt lt brackets for> emails:>>
MODULE_AUTHOR("Victor Soriano <vjsoriano@agere.com>");>
MODULE_AUTHOR("Mark Einon <mark.einon@gmail.com>");>> Go ahead and fix
this in a later patch if you want.  All the macros> there should
probably be replaced.
Hi Dan, thanks for the feedback.Removing et131x_version.h file and
simplifying the macros is alreadyon my todo list.
>> Btw, the problems with kernel.org have complicated things here, so> I can't tell if you have already fixed this, but the driver doesn't> compile any more.  The Kconfig file says it "depends on NETDEV_1000"> but NETDEV_1000 isn't around any more since f860b0522f65d3> "drivers/net: Kconfig and Makefile cleanup" was merged.>
No, I've not experienced this issue yet. I'm using Greg's
staging-2.6tree, last updated at 7cbf3c7cd59 which doesn't have the
commit youmention.I'll look out for it when everything gets moving
again.
Cheers,
Mark

[PATCH 8/8] staging: et131x: Fix indefinite low power sleep

[Mark Einon]

The mechanism by which the device is put into low power sleep is broken
in that the device can never come back out of low power mode afterwards.
Temorary fix to bring the device back out of sleep almost immediately,
until a suitable wake trigger can be found.

Signed-off-by: Mark Einon <mark.einon@gmail.com>
---
 drivers/staging/et131x/et1310_pm.c      |    5 +++--
 drivers/staging/et131x/et131x_initpci.c |   12 +++++++++---
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/et131x/et1310_pm.c b/drivers/staging/et131x/et1310_pm.c
index fd1eb6f..a1b72bf 100644
--- a/drivers/staging/et131x/et1310_pm.c
+++ b/drivers/staging/et131x/et1310_pm.c
@@ -147,6 +147,8 @@ void et1310_enable_phy_coma(struct et131x_adapter *adapter)
 
 	/* Wait for outstanding Receive packets */
 
+	et131x_disable_txrx(adapter->netdev);
+
 	/* Gate off JAGCore 3 clock domains */
 	pmcsr &= ~ET_PMCSR_INIT;
 	writel(pmcsr, &adapter->regs->global.pm_csr);
@@ -198,7 +200,6 @@ void et1310_disable_phy_coma(struct et131x_adapter *adapter)
 	/* Allow Tx to restart */
 	adapter->flags &= ~fMP_ADAPTER_LOWER_POWER;
 
-	/* Need to re-enable Rx. */
-	et131x_rx_dma_enable(adapter);
+	et131x_enable_txrx(adapter->netdev);
 }
 
diff --git a/drivers/staging/et131x/et131x_initpci.c b/drivers/staging/et131x/et131x_initpci.c
index ed22615..9795310 100644
--- a/drivers/staging/et131x/et131x_initpci.c
+++ b/drivers/staging/et131x/et131x_initpci.c
@@ -243,10 +243,15 @@ void et131x_error_timer_handler(unsigned long data)
 	struct et131x_adapter *adapter = (struct et131x_adapter *) data;
 	struct phy_device *phydev = adapter->phydev;
 
-	if (!et1310_in_phy_coma(adapter))
+	if (et1310_in_phy_coma(adapter)) {
+		/* Bring the device immediately out of coma, to
+		 * prevent it from sleeping indefinitely, this
+		 * mechanism could be improved! */
+		et1310_disable_phy_coma(adapter);
+		adapter->boot_coma = 20;
+	} else {
 		et1310_update_macstat_host_counters(adapter);
-	else
-		dev_err(&adapter->pdev->dev, "No interrupts, in PHY coma\n");
+	}
 
 	if (!phydev->link && adapter->boot_coma < 11)
 		adapter->boot_coma++;
@@ -495,6 +500,7 @@ static void et131x_adjust_link(struct net_device *netdev)
 		} else {
 			dev_warn(&adapter->pdev->dev,
 			    "Link down - cable problem ?\n");
+			adapter->boot_coma = 0;
 
 			if (phydev && phydev->speed == SPEED_10) {
 				/* NOTE - Is there a way to query this without
-- 
1.7.6.4