Skip to contentSkip to conversation

staging: et131x: Remove TODO entry 'alloc_etherdev initilising memory with zero'

PATCH In review Mark Einon 3 participants 13 messages Oct 1, 2011 – Oct 2, 2011 Staging

lore
Share & tools
lore

Mark Einon

author
#0 lore raw

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

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.

---
 drivers/staging/et131x/README |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)
drivers/staging/et131x/README +0 -1
index 7e846b2..d24ef84 100644
@@ -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 )
Signature · 1 line
1.7.6.4
author
#1 lore raw

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

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.

---
 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(-)
drivers/staging/et131x/et1310_mac.c +2 -2
index 08d7691..157462f 100644
@@ -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);
drivers/staging/et131x/et1310_pm.c +15 -0
index b20d5d6..55f2d36 100644
@@ -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
  *
drivers/staging/et131x/et131x.h +1 -0
index c047e6e..d967c5a1 100644
@@ -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);
 
drivers/staging/et131x/et131x_initpci.c +9 -19
index e9daa73..667ab80 100644
@@ -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;
drivers/staging/et131x/et131x_isr.c +1 -1
index 0747935..fb108d2 100644
@@ -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);
 			}
 		}
Signature · 1 line
1.7.6.4
author
#2 lore raw

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

In reply to 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.

---
 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(-)
drivers/staging/et131x/et1310_phy.c +0 -3
index fc37b18..a72046b 100644
@@ -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);
 
drivers/staging/et131x/et131x_adapter.h +0 -3
index fabc2c5..b5195ef 100644
@@ -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;
 
drivers/staging/et131x/et131x_initpci.c +3 -4
index 667ab80..10283a3 100644
@@ -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?
Signature · 1 line
1.7.6.4
author
#3 lore raw

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

In reply to Mark Einon

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

---
 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(-)
drivers/staging/et131x/README +0 -1
index d24ef84..42c388d 100644
@@ -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:
drivers/staging/et131x/et131x.h +2 -0
index d967c5a1..15486d1 100644
@@ -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);
drivers/staging/et131x/et131x_initpci.c +43 -2

57 lines not on this page · raw message

Signature · 1 line
1.7.6.4
#4 lore raw

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

Mark Einon <[email protected]> :

Quoted 5 lines

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

[...]

Quoted 30 lines
drivers/staging/et131x/et131x_initpci.c +22 -0
index 10283a3..e166fde 100644
@@ -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))

drivers/staging/et131x/et131x_initpci.c static void __devexit et131x_pci_remove(struct pci_dev *pdev) · line 903

Quoted 10 lines
drivers/staging/et131x/et131x_initpci.c +10 -0
+		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.

[...]

drivers/staging/et131x/et131x_initpci.c static struct pci_driver et131x_driver = { · line 929

Quoted 10 lines
drivers/staging/et131x/et131x_initpci.c +4 -2
@@ -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.

Signature · 1 line
Ueimor
author
#5 lore raw

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

Quoted 4 lines · Francois Romieu

Mark Einon <[email protected]> :

Quoted 3 lines

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

---

Quoted 10 lines
Quoted 10 lines
drivers/staging/et131x/et131x_initpci.c +10 -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);
Quoted 8 lines
Quoted 6 lines

+
+     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?

Quoted 15 lines

[...]

drivers/staging/et131x/et131x_initpci.c static struct pci_driver et131x_driver = { · line 929

Quoted 10 lines
drivers/staging/et131x/et131x_initpci.c +4 -2
@@ -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.

Quoted 4 lines

--
Ueimor

#6 lore raw

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

Mark Einon <[email protected]> :

Quoted 1 line · Francois Romieu

[...]

Quoted 10 lines
Quoted 7 lines
Quoted 5 lines

+ ? ? 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.

Signature · 1 line
Ueimor
author
#7 lore raw

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

In reply to 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.

---
 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(-)
drivers/staging/et131x/README +0 -1
index 42c388d..a0ebaa0 100644
@@ -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:
drivers/staging/et131x/et131x.h +1 -0
index 15486d1..c8f5ab1 100644
@@ -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);
drivers/staging/et131x/et131x_initpci.c +0 -7
index e166fde..8bff4a0 100644
@@ -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
drivers/staging/et131x/et131x_netdev.c +4 -0
index 6ec4a0f..2184682 100644
@@ -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 */
Signature · 1 line
1.7.6.4
author
#8 lore raw

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

In reply to 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.

---
 drivers/staging/et131x/README |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)
drivers/staging/et131x/README +0 -1
index a0ebaa0..b262ec7 100644
@@ -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 <[email protected]>
Signature · 1 line
1.7.6.4
author
#9 lore raw

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

In reply to Mark Einon

Adding copyright notices and adding myself as a module author.

---
 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(-)
drivers/staging/et131x/et1310_address_map.h +2 -0
index 38ec56c..b502ce3 100644
@@ -6,6 +6,8 @@
  * All rights reserved.
  *   http://www.agere.com
  *
+ * Copyright (c) 2011 Mark Einon <[email protected]>
+ *
  *------------------------------------------------------------------------------
  *
  * et1310_address_map.h - Contains the register mapping for the ET1310
drivers/staging/et131x/et1310_eeprom.c +2 -0
index e810254..747b1dc 100644
@@ -6,6 +6,8 @@
  * All rights reserved.
  *   http://www.agere.com
  *
+ * Copyright (c) 2011 Mark Einon <[email protected]>
+ *
  *------------------------------------------------------------------------------
  *
  * et1310_eeprom.c - Code used to access the device's EEPROM
drivers/staging/et131x/et1310_mac.c +2 -0
index 157462f..b4c9df1 100644
@@ -6,6 +6,8 @@
  * All rights reserved.
  *   http://www.agere.com
  *
+ * Copyright (c) 2011 Mark Einon <[email protected]>
+ *
  *------------------------------------------------------------------------------
  *
  * et1310_mac.c - All code and routines pertaining to the MAC
drivers/staging/et131x/et1310_phy.c +2 -0
index a72046b..145ec76 100644
@@ -6,6 +6,8 @@
  * All rights reserved.
  *   http://www.agere.com
  *
+ * Copyright (c) 2011 Mark Einon <[email protected]>
+ *
  *------------------------------------------------------------------------------
  *
  * et1310_phy.c - Routines for configuring and accessing the PHY
drivers/staging/et131x/et1310_phy.h +2 -0
index 82037ac..2803bdf 100644
@@ -6,6 +6,8 @@
  * All rights reserved.
  *   http://www.agere.com
  *
+ * Copyright (c) 2011 Mark Einon <[email protected]>
+ *
  *------------------------------------------------------------------------------
  *
  * et1310_phy.h - Defines, structs, enums, prototypes, etc. pertaining to the
drivers/staging/et131x/et1310_pm.c +2 -0
index 55f2d36..fd1eb6f 100644
@@ -6,6 +6,8 @@
  * All rights reserved.
  *   http://www.agere.com
  *
+ * Copyright (c) 2011 Mark Einon <[email protected]>
+ *
  *------------------------------------------------------------------------------
  *
  * et1310_pm.c - All power management related code (not completely implemented)
drivers/staging/et131x/et1310_rx.c +2 -0
index c402c5e..600a8f1 100644
@@ -6,6 +6,8 @@
  * All rights reserved.
  *   http://www.agere.com
  *
+ * Copyright (c) 2011 Mark Einon <[email protected]>
+ *
  *------------------------------------------------------------------------------
  *
  * et1310_rx.c - Routines used to perform data reception
drivers/staging/et131x/et1310_rx.h +2 -0
index 1448aa9..a19fab9 100644
@@ -6,6 +6,8 @@
  * All rights reserved.
  *   http://www.agere.com
  *
+ * Copyright (c) 2011 Mark Einon <[email protected]>
+ *
  *------------------------------------------------------------------------------
  *
  * et1310_rx.h - Defines, structs, enums, prototypes, etc. pertaining to data
drivers/staging/et131x/et1310_tx.c +2 -0
index 1f80651..2ecb934 100644
@@ -6,6 +6,8 @@
  * All rights reserved.
  *   http://www.agere.com
  *
+ * Copyright (c) 2011 Mark Einon <[email protected]>
+ *
  *------------------------------------------------------------------------------
  *
  * et1310_tx.c - Routines used to perform data transmission.
drivers/staging/et131x/et131x_adapter.h +2 -0
index b5195ef..b062e6d 100644
@@ -6,6 +6,8 @@
  * All rights reserved.
  *   http://www.agere.com
  *
+ * Copyright (c) 2011 Mark Einon <[email protected]>
+ *
  *------------------------------------------------------------------------------
  *
  * et131x_adapter.h - Header which includes the private adapter structure, along
drivers/staging/et131x/et131x_defs.h +2 -0
index 3d5193f..872a5af 100644
@@ -6,6 +6,8 @@
  * All rights reserved.
  *   http://www.agere.com
  *
+ * Copyright (c) 2011 Mark Einon <[email protected]>
+ *
  *------------------------------------------------------------------------------
  *
  * et131x_defs.h - Defines, structs, enums, prototypes, etc. to assist with OS
drivers/staging/et131x/et131x_ethtool.c +1 -3
index c4555b9..903b5a0 100644
@@ -1,5 +1,5 @@
 /*
- *  Copyright (C) 2011 Mark Einon
+ *  Copyright (c) 2011 Mark Einon <[email protected]>
  *
  *  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 <[email protected]>
  */
 #include "et131x_version.h"
 #include "et131x_defs.h"
drivers/staging/et131x/et131x_initpci.c +3 -0
index 8bff4a0..ed22615 100644
@@ -6,6 +6,8 @@
  * All rights reserved.
  *   http://www.agere.com
  *
+ * Copyright (c) 2011 Mark Einon <[email protected]>
+ *
  *------------------------------------------------------------------------------
  *
  * 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);
drivers/staging/et131x/et131x_isr.c +2 -0
index fb108d2..bccd0e5 100644
@@ -6,6 +6,8 @@
  * All rights reserved.
  *   http://www.agere.com
  *
+ * Copyright (c) 2011 Mark Einon <[email protected]>
+ *
  *------------------------------------------------------------------------------
  *
  * et131x_isr.c - File which contains the ISR, ISR handler, and related routines
drivers/staging/et131x/et131x_netdev.c +2 -0
index 2184682..79127de 100644
@@ -6,6 +6,8 @@
  * All rights reserved.
  *   http://www.agere.com
  *
+ * Copyright (c) 2011 Mark Einon <[email protected]>
+ *
  *------------------------------------------------------------------------------
  *
  * et131x_netdev.c - Routines and data required by all Linux network devices.
drivers/staging/et131x/et131x_version.h +1 -0
index 2aa9bda..c08b5e5 100644
@@ -59,6 +59,7 @@
 #define __ET131X_VERSION_H__
 
 #define DRIVER_AUTHOR		"Victor Soriano ([email protected])"
+#define DRIVER_AUTHOR2		"Mark Einon ([email protected])"
 #define DRIVER_LICENSE		"Dual BSD/GPL"
 #define DRIVER_DEVICE_STRING	"ET1310"
 #define DRIVER_NAME		"et131x"
Signature · 1 line
1.7.6.4
#10 lore raw

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

drivers/staging/et131x/et131x_initpci.c module_exit(et131x_cleanup_module); · line 952

Quoted 7 lines · Mark Einon
drivers/staging/et131x/et131x_initpci.c +1 -0
@@ -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 <[email protected]>"); MODULE_AUTHOR("Mark Einon <[email protected]>");

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

author
#11 lore raw

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

On 1 October 2011 20:58, Dan Carpenter <[email protected]> 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 <[email protected]>");> MODULE_AUTHOR("Mark Einon <[email protected]>");>> 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.

Quoted 1 line
Quoted 1 line

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

author
#12 lore raw

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

In reply to 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.

---
 drivers/staging/et131x/et1310_pm.c      |    5 +++--
 drivers/staging/et131x/et131x_initpci.c |   12 +++++++++---
 2 files changed, 12 insertions(+), 5 deletions(-)
drivers/staging/et131x/et1310_pm.c +3 -2
index fd1eb6f..a1b72bf 100644
@@ -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);
 }
 
drivers/staging/et131x/et131x_initpci.c +9 -3
index ed22615..9795310 100644
@@ -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
Signature · 1 line
1.7.6.4