Please pull from 'upstream-linus' branch of
master.kernel.org:/pub/scm/linux/kernel/git/jgarzik/netdev-2.6.git upstream-linus
to receive the following updates:
drivers/net/atl1/atl1_main.c | 19 +++++++------------
drivers/net/ehea/ehea.h | 5 ++++-
drivers/net/ehea/ehea_main.c | 16 ++++++++++++++--
drivers/net/phy/phy.c | 4 ++--
drivers/net/phy/phy_device.c | 4 ++--
drivers/net/sky2.c | 9 ++++++++-
drivers/net/spider_net.c | 12 ++++--------
7 files changed, 41 insertions(+), 28 deletions(-)
Hans-J?rgen Koch (1):
Fix a lock problem in generic phy code
Ishizaki Kou (1):
spidernet: fix interrupt reason recognition
Jan-Bernd Themann (2):
ehea: propagate physical port state
ehea: fix last_rx update
Luca Tettamanti (1):
atl1: disable broken 64-bit DMA
Stephen Hemminger (1):
sky2: restore multicast list on resume and other ops
diff --git a/drivers/net/atl1/atl1_main.c b/drivers/net/atl1/atl1_main.c
index 3c1984e..f23e13c 100644
--- a/drivers/net/atl1/atl1_main.c
+++ b/drivers/net/atl1/atl1_main.c
@@ -2203,21 +2203,20 @@ static int __devinit atl1_probe(struct pci_dev *pdev,
struct net_device *netdev;
struct atl1_adapter *adapter;
static int cards_found = 0;
- bool pci_using_64 = true;
int err;
err = pci_enable_device(pdev);
if (err)
return err;
- err = pci_set_dma_mask(pdev, DMA_64BIT_MASK);
+ /*
+ * 64-bit DMA currently has data corruption problems, so let's just
+ * use 32-bit DMA for now. This is a big hack that is probably wrong.
+ */
+ err = pci_set_dma_mask(pdev, DMA_32BIT_MASK);
if (err) {
- err = pci_set_dma_mask(pdev, DMA_32BIT_MASK);
- if (err) {
- dev_err(&pdev->dev, "no usable DMA configuration\n");
- goto err_dma;
- }
- pci_using_64 = false;
+ dev_err(&pdev->dev, "no usable DMA configuration\n");
+ goto err_dma;
}
/* Mark all PCI regions associated with PCI device
* pdev as being reserved by owner atl1_driver_name
@@ -2282,7 +2281,6 @@ static int __devinit atl1_probe(struct pci_dev *pdev,
netdev->ethtool_ops = &atl1_ethtool_ops;
adapter->bd_number = cards_found;
- adapter->pci_using_64 = pci_using_64;
/* setup the private structure */
err = atl1_sw_init(adapter);
@@ -2299,9 +2297,6 @@ static int __devinit atl1_probe(struct pci_dev *pdev,
*/
/* netdev->features |= NETIF_F_TSO; */
- if (pci_using_64)
- netdev->features |= NETIF_F_HIGHDMA;
-
netdev->features |= NETIF_F_LLTX;
/*
diff --git a/drivers/net/ehea/ehea.h b/drivers/net/ehea/ehea.h
index d67f97b..8d58be5 100644
--- a/drivers/net/ehea/ehea.h
+++ b/drivers/net/ehea/ehea.h
@@ -39,7 +39,7 @@
#include <asm/io.h>
#define DRV_NAME "ehea"
-#define DRV_VERSION "EHEA_0073"
+#define DRV_VERSION "EHEA_0074"
/* eHEA capability flags */
#define DLPAR_PORT_ADD_REM 1
@@ -402,6 +402,8 @@ struct ehea_mc_list {
#define EHEA_PORT_UP 1
#define EHEA_PORT_DOWN 0
+#define EHEA_PHY_LINK_UP 1
+#define EHEA_PHY_LINK_DOWN 0
#define EHEA_MAX_PORT_RES 16
struct ehea_port {
struct ehea_adapter *adapter; /* adapter that owns this port */
@@ -427,6 +429,7 @@ struct ehea_port {
u32 msg_enable;
u32 sig_comp_iv;
u32 state;
+ u8 phy_link;
u8 full_duplex;
u8 autoneg;
u8 num_def_qps;
diff --git a/drivers/net/ehea/ehea_main.c b/drivers/net/ehea/ehea_main.c
index db57474..717b129 100644
--- a/drivers/net/ehea/ehea_main.c
+++ b/drivers/net/ehea/ehea_main.c
@@ -53,17 +53,21 @@ static int rq3_entries = EHEA_DEF_ENTRIES_RQ3;
static int sq_entries = EHEA_DEF_ENTRIES_SQ;
static int use_mcs = 0;
static int num_tx_qps = EHEA_NUM_TX_QP;
+static int prop_carrier_state = 0;
module_param(msg_level, int, 0);
module_param(rq1_entries, int, 0);
module_param(rq2_entries, int, 0);
module_param(rq3_entries, int, 0);
module_param(sq_entries, int, 0);
+module_param(prop_carrier_state, int, 0);
module_param(use_mcs, int, 0);
module_param(num_tx_qps, int, 0);
MODULE_PARM_DESC(num_tx_qps, "Number of TX-QPS");
MODULE_PARM_DESC(msg_level, "msg_level");
+MODULE_PARM_DESC(prop_carrier_state, "Propagate carrier state of physical "
+ "port to stack. 1:yes, 0:no. Default = 0 ");
MODULE_PARM_DESC(rq3_entries, "Number of entries for Receive Queue 3 "
"[2^x - 1], x = [6..14]. Default = "
__MODULE_STRING(EHEA_DEF_ENTRIES_RQ3) ")");
@@ -467,7 +471,7 @@ static struct ehea_cqe *ehea_proc_rwqes(struct net_device *dev,
else
netif_receive_skb(skb);
- dev->last_rx = jiffies;
+ port->netdev->last_rx = jiffies;
} else {
pr->p_stats.poll_receive_errors++;
port_reset = ehea_treat_poll_error(pr, rq, cqe,
@@ -814,7 +818,9 @@ int ehea_set_portspeed(struct ehea_port *port, u32 port_speed)
ehea_error("Failed setting port speed");
}
}
- netif_carrier_on(port->netdev);
+ if (!prop_carrier_state || (port->phy_link == EHEA_PHY_LINK_UP))
+ netif_carrier_on(port->netdev);
+
kfree(cb4);
out:
return ret;
@@ -869,13 +875,19 @@ static void ehea_parse_eqe(struct ehea_adapter *adapter, u64 eqe)
}
if (EHEA_BMASK_GET(NEQE_EXTSWITCH_PORT_UP, eqe)) {
+ port->phy_link = EHEA_PHY_LINK_UP;
if (netif_msg_link(port))
ehea_info("%s: Physical port up",
port->netdev->name);
+ if (prop_carrier_state)
+ netif_carrier_on(port->netdev);
} else {
+ port->phy_link = EHEA_PHY_LINK_DOWN;
if (netif_msg_link(port))
ehea_info("%s: Physical port down",
port->netdev->name);
+ if (prop_carrier_state)
+ netif_carrier_off(port->netdev);
}
if (EHEA_BMASK_GET(NEQE_EXTSWITCH_PRIMARY, eqe))
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index e323efd..0cc4369 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -755,7 +755,7 @@ out_unlock:
*/
void phy_start(struct phy_device *phydev)
{
- spin_lock(&phydev->lock);
+ spin_lock_bh(&phydev->lock);
switch (phydev->state) {
case PHY_STARTING:
@@ -769,7 +769,7 @@ void phy_start(struct phy_device *phydev)
default:
break;
}
- spin_unlock(&phydev->lock);
+ spin_unlock_bh(&phydev->lock);
}
EXPORT_SYMBOL(phy_stop);
EXPORT_SYMBOL(phy_start);
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index e275df8..49328e0 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -644,7 +644,7 @@ static int phy_probe(struct device *dev)
if (!(phydrv->flags & PHY_HAS_INTERRUPT))
phydev->irq = PHY_POLL;
- spin_lock(&phydev->lock);
+ spin_lock_bh(&phydev->lock);
/* Start out supporting everything. Eventually,
* a controller will attach, and may modify one
@@ -658,7 +658,7 @@ static int phy_probe(struct device *dev)
if (phydev->drv->probe)
err = phydev->drv->probe(phydev);
- spin_unlock(&phydev->lock);
+ spin_unlock_bh(&phydev->lock);
return err;
diff --git a/drivers/net/sky2.c b/drivers/net/sky2.c
index e6d937e..5d812de 100644
--- a/drivers/net/sky2.c
+++ b/drivers/net/sky2.c
@@ -149,6 +149,8 @@ static const char *yukon2_name[] = {
"FE", /* 0xb7 */
};
+static void sky2_set_multicast(struct net_device *dev);
+
/* Access to external PHY */
static int gm_phy_write(struct sky2_hw *hw, unsigned port, u16 reg, u16 val)
{
@@ -2900,8 +2902,10 @@ static int sky2_set_settings(struct net_device *dev, struct ethtool_cmd *ecmd)
sky2->autoneg = ecmd->autoneg;
sky2->advertising = ecmd->advertising;
- if (netif_running(dev))
+ if (netif_running(dev)) {
sky2_phy_reinit(sky2);
+ sky2_set_multicast(dev);
+ }
return 0;
}
@@ -2994,6 +2998,7 @@ static int sky2_nway_reset(struct net_device *dev)
return -EINVAL;
sky2_phy_reinit(sky2);
+ sky2_set_multicast(dev);
return 0;
}
@@ -4171,6 +4176,8 @@ static int sky2_resume(struct pci_dev *pdev)
dev_close(dev);
goto out;
}
+
+ sky2_set_multicast(dev);
}
}
diff --git a/drivers/net/spider_net.c b/drivers/net/spider_net.c
index 590b12c..82d837a 100644
--- a/drivers/net/spider_net.c
+++ b/drivers/net/spider_net.c
@@ -1441,17 +1441,14 @@ static void
spider_net_handle_error_irq(struct spider_net_card *card, u32 status_reg)
{
u32 error_reg1, error_reg2;
- u32 mask_reg1, mask_reg2;
u32 i;
int show_error = 1;
error_reg1 = spider_net_read_reg(card, SPIDER_NET_GHIINT1STS);
error_reg2 = spider_net_read_reg(card, SPIDER_NET_GHIINT2STS);
- mask_reg1 = spider_net_read_reg(card, SPIDER_NET_GHIINT1MSK);
- mask_reg2 = spider_net_read_reg(card,SPIDER_NET_GHIINT2MSK);
- error_reg1 &= mask_reg1;
- error_reg2 &= mask_reg2;
+ error_reg1 &= SPIDER_NET_INT1_MASK_VALUE;
+ error_reg2 &= SPIDER_NET_INT2_MASK_VALUE;
/* check GHIINT0STS ************************************/
if (status_reg)
@@ -1679,11 +1676,10 @@ spider_net_interrupt(int irq, void *ptr)
{
struct net_device *netdev = ptr;
struct spider_net_card *card = netdev_priv(netdev);
- u32 status_reg, mask_reg;
+ u32 status_reg;
status_reg = spider_net_read_reg(card, SPIDER_NET_GHIINT0STS);
- mask_reg = spider_net_read_reg(card, SPIDER_NET_GHIINT0MSK);
- status_reg &= mask_reg;
+ status_reg &= SPIDER_NET_INT0_MASK_VALUE;
if (!status_reg)
return IRQ_NONE;
On Thu, 2007-09-13 at 01:30 -0400, Jeff Garzik wrote:
> Please pull from 'upstream-linus' branch of
> master.kernel.org:/pub/scm/linux/kernel/git/jgarzik/netdev-2.6.git upstream-linus
>
> to receive the following updates:
>
> drivers/net/atl1/atl1_main.c | 19 +++++++------------
> drivers/net/ehea/ehea.h | 5 ++++-
> drivers/net/ehea/ehea_main.c | 16 ++++++++++++++--
> drivers/net/phy/phy.c | 4 ++--
> drivers/net/phy/phy_device.c | 4 ++--
> drivers/net/sky2.c | 9 ++++++++-
> drivers/net/spider_net.c | 12 ++++--------
> 7 files changed, 41 insertions(+), 28 deletions(-)
>
> Hans-Jürgen Koch (1):
> Fix a lock problem in generic phy code
>
> Ishizaki Kou (1):
> spidernet: fix interrupt reason recognition
>
> Jan-Bernd Themann (2):
> ehea: propagate physical port state
> ehea: fix last_rx update
>
> Luca Tettamanti (1):
> atl1: disable broken 64-bit DMA
>
> Stephen Hemminger (1):
> sky2: restore multicast list on resume and other ops
>
> diff --git a/drivers/net/atl1/atl1_main.c b/drivers/net/atl1/atl1_main.c
> index 3c1984e..f23e13c 100644
> --- a/drivers/net/atl1/atl1_main.c
> +++ b/drivers/net/atl1/atl1_main.c
> @@ -2203,21 +2203,20 @@ static int __devinit atl1_probe(struct pci_dev *pdev,
> struct net_device *netdev;
> struct atl1_adapter *adapter;
> static int cards_found = 0;
> - bool pci_using_64 = true;
> int err;
>
> err = pci_enable_device(pdev);
> if (err)
> return err;
>
> - err = pci_set_dma_mask(pdev, DMA_64BIT_MASK);
> + /*
> + * 64-bit DMA currently has data corruption problems, so let's just
> + * use 32-bit DMA for now. This is a big hack that is probably wrong.
> + */
> + err = pci_set_dma_mask(pdev, DMA_32BIT_MASK);
> if (err) {
> - err = pci_set_dma_mask(pdev, DMA_32BIT_MASK);
> - if (err) {
> - dev_err(&pdev->dev, "no usable DMA configuration\n");
> - goto err_dma;
> - }
> - pci_using_64 = false;
> + dev_err(&pdev->dev, "no usable DMA configuration\n");
> + goto err_dma;
> }
> /* Mark all PCI regions associated with PCI device
> * pdev as being reserved by owner atl1_driver_name
> @@ -2282,7 +2281,6 @@ static int __devinit atl1_probe(struct pci_dev *pdev,
>
> netdev->ethtool_ops = &atl1_ethtool_ops;
> adapter->bd_number = cards_found;
> - adapter->pci_using_64 = pci_using_64;
>
> /* setup the private structure */
> err = atl1_sw_init(adapter);
> @@ -2299,9 +2297,6 @@ static int __devinit atl1_probe(struct pci_dev *pdev,
> */
> /* netdev->features |= NETIF_F_TSO; */
>
> - if (pci_using_64)
> - netdev->features |= NETIF_F_HIGHDMA;
> -
> netdev->features |= NETIF_F_LLTX;
>
> /*
> diff --git a/drivers/net/ehea/ehea.h b/drivers/net/ehea/ehea.h
> index d67f97b..8d58be5 100644
> --- a/drivers/net/ehea/ehea.h
> +++ b/drivers/net/ehea/ehea.h
> @@ -39,7 +39,7 @@
> #include <asm/io.h>
>
> #define DRV_NAME "ehea"
> -#define DRV_VERSION "EHEA_0073"
> +#define DRV_VERSION "EHEA_0074"
>
> /* eHEA capability flags */
> #define DLPAR_PORT_ADD_REM 1
> @@ -402,6 +402,8 @@ struct ehea_mc_list {
>
> #define EHEA_PORT_UP 1
> #define EHEA_PORT_DOWN 0
> +#define EHEA_PHY_LINK_UP 1
> +#define EHEA_PHY_LINK_DOWN 0
> #define EHEA_MAX_PORT_RES 16
> struct ehea_port {
> struct ehea_adapter *adapter; /* adapter that owns this port */
> @@ -427,6 +429,7 @@ struct ehea_port {
> u32 msg_enable;
> u32 sig_comp_iv;
> u32 state;
> + u8 phy_link;
> u8 full_duplex;
> u8 autoneg;
> u8 num_def_qps;
> diff --git a/drivers/net/ehea/ehea_main.c b/drivers/net/ehea/ehea_main.c
> index db57474..717b129 100644
> --- a/drivers/net/ehea/ehea_main.c
> +++ b/drivers/net/ehea/ehea_main.c
> @@ -53,17 +53,21 @@ static int rq3_entries = EHEA_DEF_ENTRIES_RQ3;
> static int sq_entries = EHEA_DEF_ENTRIES_SQ;
> static int use_mcs = 0;
> static int num_tx_qps = EHEA_NUM_TX_QP;
> +static int prop_carrier_state = 0;
>
> module_param(msg_level, int, 0);
> module_param(rq1_entries, int, 0);
> module_param(rq2_entries, int, 0);
> module_param(rq3_entries, int, 0);
> module_param(sq_entries, int, 0);
> +module_param(prop_carrier_state, int, 0);
> module_param(use_mcs, int, 0);
> module_param(num_tx_qps, int, 0);
>
> MODULE_PARM_DESC(num_tx_qps, "Number of TX-QPS");
> MODULE_PARM_DESC(msg_level, "msg_level");
> +MODULE_PARM_DESC(prop_carrier_state, "Propagate carrier state of physical "
> + "port to stack. 1:yes, 0:no. Default = 0 ");
WTF? why would the default be to _not_ propagate carrier state? Are
there some mitigating circumstances that require this driver to not
notify the stack of carrier on/off? Userspace stuff really should know
about the carrier state, and this disables it by default.
Dan
> MODULE_PARM_DESC(rq3_entries, "Number of entries for Receive Queue 3 "
> "[2^x - 1], x = [6..14]. Default = "
> __MODULE_STRING(EHEA_DEF_ENTRIES_RQ3) ")");
> @@ -467,7 +471,7 @@ static struct ehea_cqe *ehea_proc_rwqes(struct net_device *dev,
> else
> netif_receive_skb(skb);
>
> - dev->last_rx = jiffies;
> + port->netdev->last_rx = jiffies;
> } else {
> pr->p_stats.poll_receive_errors++;
> port_reset = ehea_treat_poll_error(pr, rq, cqe,
> @@ -814,7 +818,9 @@ int ehea_set_portspeed(struct ehea_port *port, u32 port_speed)
> ehea_error("Failed setting port speed");
> }
> }
> - netif_carrier_on(port->netdev);
> + if (!prop_carrier_state || (port->phy_link == EHEA_PHY_LINK_UP))
> + netif_carrier_on(port->netdev);
> +
> kfree(cb4);
> out:
> return ret;
> @@ -869,13 +875,19 @@ static void ehea_parse_eqe(struct ehea_adapter *adapter, u64 eqe)
> }
>
> if (EHEA_BMASK_GET(NEQE_EXTSWITCH_PORT_UP, eqe)) {
> + port->phy_link = EHEA_PHY_LINK_UP;
> if (netif_msg_link(port))
> ehea_info("%s: Physical port up",
> port->netdev->name);
> + if (prop_carrier_state)
> + netif_carrier_on(port->netdev);
> } else {
> + port->phy_link = EHEA_PHY_LINK_DOWN;
> if (netif_msg_link(port))
> ehea_info("%s: Physical port down",
> port->netdev->name);
> + if (prop_carrier_state)
> + netif_carrier_off(port->netdev);
> }
>
> if (EHEA_BMASK_GET(NEQE_EXTSWITCH_PRIMARY, eqe))
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index e323efd..0cc4369 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -755,7 +755,7 @@ out_unlock:
> */
> void phy_start(struct phy_device *phydev)
> {
> - spin_lock(&phydev->lock);
> + spin_lock_bh(&phydev->lock);
>
> switch (phydev->state) {
> case PHY_STARTING:
> @@ -769,7 +769,7 @@ void phy_start(struct phy_device *phydev)
> default:
> break;
> }
> - spin_unlock(&phydev->lock);
> + spin_unlock_bh(&phydev->lock);
> }
> EXPORT_SYMBOL(phy_stop);
> EXPORT_SYMBOL(phy_start);
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index e275df8..49328e0 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -644,7 +644,7 @@ static int phy_probe(struct device *dev)
> if (!(phydrv->flags & PHY_HAS_INTERRUPT))
> phydev->irq = PHY_POLL;
>
> - spin_lock(&phydev->lock);
> + spin_lock_bh(&phydev->lock);
>
> /* Start out supporting everything. Eventually,
> * a controller will attach, and may modify one
> @@ -658,7 +658,7 @@ static int phy_probe(struct device *dev)
> if (phydev->drv->probe)
> err = phydev->drv->probe(phydev);
>
> - spin_unlock(&phydev->lock);
> + spin_unlock_bh(&phydev->lock);
>
> return err;
>
> diff --git a/drivers/net/sky2.c b/drivers/net/sky2.c
> index e6d937e..5d812de 100644
> --- a/drivers/net/sky2.c
> +++ b/drivers/net/sky2.c
> @@ -149,6 +149,8 @@ static const char *yukon2_name[] = {
> "FE", /* 0xb7 */
> };
>
> +static void sky2_set_multicast(struct net_device *dev);
> +
> /* Access to external PHY */
> static int gm_phy_write(struct sky2_hw *hw, unsigned port, u16 reg, u16 val)
> {
> @@ -2900,8 +2902,10 @@ static int sky2_set_settings(struct net_device *dev, struct ethtool_cmd *ecmd)
> sky2->autoneg = ecmd->autoneg;
> sky2->advertising = ecmd->advertising;
>
> - if (netif_running(dev))
> + if (netif_running(dev)) {
> sky2_phy_reinit(sky2);
> + sky2_set_multicast(dev);
> + }
>
> return 0;
> }
> @@ -2994,6 +2998,7 @@ static int sky2_nway_reset(struct net_device *dev)
> return -EINVAL;
>
> sky2_phy_reinit(sky2);
> + sky2_set_multicast(dev);
>
> return 0;
> }
> @@ -4171,6 +4176,8 @@ static int sky2_resume(struct pci_dev *pdev)
> dev_close(dev);
> goto out;
> }
> +
> + sky2_set_multicast(dev);
> }
> }
>
> diff --git a/drivers/net/spider_net.c b/drivers/net/spider_net.c
> index 590b12c..82d837a 100644
> --- a/drivers/net/spider_net.c
> +++ b/drivers/net/spider_net.c
> @@ -1441,17 +1441,14 @@ static void
> spider_net_handle_error_irq(struct spider_net_card *card, u32 status_reg)
> {
> u32 error_reg1, error_reg2;
> - u32 mask_reg1, mask_reg2;
> u32 i;
> int show_error = 1;
>
> error_reg1 = spider_net_read_reg(card, SPIDER_NET_GHIINT1STS);
> error_reg2 = spider_net_read_reg(card, SPIDER_NET_GHIINT2STS);
> - mask_reg1 = spider_net_read_reg(card, SPIDER_NET_GHIINT1MSK);
> - mask_reg2 = spider_net_read_reg(card,SPIDER_NET_GHIINT2MSK);
>
> - error_reg1 &= mask_reg1;
> - error_reg2 &= mask_reg2;
> + error_reg1 &= SPIDER_NET_INT1_MASK_VALUE;
> + error_reg2 &= SPIDER_NET_INT2_MASK_VALUE;
>
> /* check GHIINT0STS ************************************/
> if (status_reg)
> @@ -1679,11 +1676,10 @@ spider_net_interrupt(int irq, void *ptr)
> {
> struct net_device *netdev = ptr;
> struct spider_net_card *card = netdev_priv(netdev);
> - u32 status_reg, mask_reg;
> + u32 status_reg;
>
> status_reg = spider_net_read_reg(card, SPIDER_NET_GHIINT0STS);
> - mask_reg = spider_net_read_reg(card, SPIDER_NET_GHIINT0MSK);
> - status_reg &= mask_reg;
> + status_reg &= SPIDER_NET_INT0_MASK_VALUE;
>
> if (!status_reg)
> return IRQ_NONE;
> -
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Dan Williams wrote:
> WTF? why would the default be to _not_ propagate carrier state? Are
> there some mitigating circumstances that require this driver to not
> notify the stack of carrier on/off? Userspace stuff really should know
> about the carrier state, and this disables it by default.
The commit explains that...
Jeff
On Fri, 2007-09-14 at 14:17 -0400, Jeff Garzik wrote:
> Dan Williams wrote:
> > WTF? why would the default be to _not_ propagate carrier state? Are
> > there some mitigating circumstances that require this driver to not
> > notify the stack of carrier on/off? Userspace stuff really should know
> > about the carrier state, and this disables it by default.
>
>
> The commit explains that...
I admit that I probably don't understand the system architecture of
where ehea would be used, but would this
cause /sys/class/net/ethX/carrier to be TRUE even if the device has no
carrier? That seems quite wrong IMHO. When does ehea not have a
carrier? And in that case, does sysfs say 1 or 0 for the carrier?
Dan
Dan Williams <[email protected]> wrote:
[...]
>I admit that I probably don't understand the system architecture of
>where ehea would be used, but would this
>cause /sys/class/net/ethX/carrier to be TRUE even if the device has no
>carrier? That seems quite wrong IMHO. When does ehea not have a
>carrier? And in that case, does sysfs say 1 or 0 for the carrier?
I don't work on ehea, but I'm generally familiar with it, and
particularly with this patch.
The usual environment for ehea devices is on large systems
subdivided into multiple logical partitions. One ehea device serves
many partitions. By having ehea always report "link up" to the logical
ports (the ports seen by the partitions), the partitions can communicate
amongst themselves even if the external ports (the ports that go to the
switch or whatever) have no link.
The ehea device, more or less, acts as a switch connecting the
partitions together. This switch type of functionality is not dependent
upon the link state of the external ports (any more than the
functionality of any switch is dependent upon whether or not it is
connected to a gateway).
This, if I'm not mistaken, is the way ehea has always operated
until this particular patch was added.
This patch (to optionally pass carrier state to the logical
ports) was added largely for bonding, so that the bonding driver can
detect link failures on the external ports (when so desired). The
default behavior remains the original behavior, i.e., do not pass
external port link state to the logical ports.
Anyway, to answer your question, the carrier state reported for
the ehea interface on the partition will always be true. Think of it as
reporting the link state from the logical interface to the "switch" that
connects the partitions; that link exists only within the ehea device
itself, and really can't fail unless the ehea device itself fails.
With the new option enabled, then ehea is more or less mimicing
a trunk failover type of function, and passing the carrier state of the
"external switch port" to the internal port.
-J
---
-Jay Vosburgh, IBM Linux Technology Center, [email protected]
On Fri, 2007-09-14 at 12:19 -0700, Jay Vosburgh wrote:
> Dan Williams <[email protected]> wrote:
> [...]
> >I admit that I probably don't understand the system architecture of
> >where ehea would be used, but would this
> >cause /sys/class/net/ethX/carrier to be TRUE even if the device has no
> >carrier? That seems quite wrong IMHO. When does ehea not have a
> >carrier? And in that case, does sysfs say 1 or 0 for the carrier?
>
> I don't work on ehea, but I'm generally familiar with it, and
> particularly with this patch.
>
> The usual environment for ehea devices is on large systems
> subdivided into multiple logical partitions. One ehea device serves
> many partitions. By having ehea always report "link up" to the logical
> ports (the ports seen by the partitions), the partitions can communicate
> amongst themselves even if the external ports (the ports that go to the
> switch or whatever) have no link.
(forgive my ignorance of course)
So essentially the ehea device has a 1(+) external ports that may/may
not be connected, but all lpars share the physical hardware itself,
which is quite happy to let all the lpars talk to each other essentially
via loopback even if there is no actual carrier detected on the external
port(s)? How does addressing work here, is it just L2 addresses? Feel
free to point me to some docs and tell me to shut up :)
At least these days module parameters can be changed at runtime through
sysfs. Stuff that can only be set at module load doesn't provide
userspace the flexibility it needs to configure stuff on the fly.
Dan
> The ehea device, more or less, acts as a switch connecting the
> partitions together. This switch type of functionality is not dependent
> upon the link state of the external ports (any more than the
> functionality of any switch is dependent upon whether or not it is
> connected to a gateway).
>
> This, if I'm not mistaken, is the way ehea has always operated
> until this particular patch was added.
>
> This patch (to optionally pass carrier state to the logical
> ports) was added largely for bonding, so that the bonding driver can
> detect link failures on the external ports (when so desired). The
> default behavior remains the original behavior, i.e., do not pass
> external port link state to the logical ports.
>
> Anyway, to answer your question, the carrier state reported for
> the ehea interface on the partition will always be true. Think of it as
> reporting the link state from the logical interface to the "switch" that
> connects the partitions; that link exists only within the ehea device
> itself, and really can't fail unless the ehea device itself fails.
>
> With the new option enabled, then ehea is more or less mimicing
> a trunk failover type of function, and passing the carrier state of the
> "external switch port" to the internal port.
>
> -J
>
> ---
> -Jay Vosburgh, IBM Linux Technology Center, [email protected]
Dan Williams <[email protected]> wrote:
[...]
>So essentially the ehea device has a 1(+) external ports that may/may
>not be connected, but all lpars share the physical hardware itself,
>which is quite happy to let all the lpars talk to each other essentially
>via loopback even if there is no actual carrier detected on the external
>port(s)? [...]
Yes.
>[...] How does addressing work here, is it just L2 addresses?
Yes. The logical ports all have unique MAC addresses.
> [...] Feel
>free to point me to some docs and tell me to shut up :)
http://www.redbooks.ibm.com/redpieces/abstracts/redp4340.html
I found this via google; I haven't read it in detail, but it
seems to cover the HEA architecture at a high level. It talks about the
whole "IVE" (integrated virtual ethernet: the adapter, hypervisor, etc)
system, but HEA is part of that, so it's probably got the answers you're
looking for.
-J
---
-Jay Vosburgh, IBM Linux Technology Center, [email protected]
Dan Williams wrote:
> On Thu, 2007-09-13 at 01:30 -0400, Jeff Garzik wrote:
>> Please pull from 'upstream-linus' branch of
>> master.kernel.org:/pub/scm/linux/kernel/git/jgarzik/netdev-2.6.git upstream-linus
>>
>> to receive the following updates:
>>
>> drivers/net/atl1/atl1_main.c | 19 +++++++------------
>> drivers/net/ehea/ehea.h | 5 ++++-
>> drivers/net/ehea/ehea_main.c | 16 ++++++++++++++--
>> drivers/net/phy/phy.c | 4 ++--
>> drivers/net/phy/phy_device.c | 4 ++--
>> drivers/net/sky2.c | 9 ++++++++-
>> drivers/net/spider_net.c | 12 ++++--------
>> 7 files changed, 41 insertions(+), 28 deletions(-)
>>
>> Hans-Jürgen Koch (1):
>> Fix a lock problem in generic phy code
>>
>> Ishizaki Kou (1):
>> spidernet: fix interrupt reason recognition
>>
>> Jan-Bernd Themann (2):
>> ehea: propagate physical port state
>> ehea: fix last_rx update
>>
maybe a little bit late with this comment:
>> ehea_error("Failed setting port speed");
>> }
>> }
>> - netif_carrier_on(port->netdev);
>> + if (!prop_carrier_state || (port->phy_link == EHEA_PHY_LINK_UP))
>> + netif_carrier_on(port->netdev);
>> +
>> kfree(cb4);
>> out:
>> return ret;
>> @@ -869,13 +875,19 @@ static void ehea_parse_eqe(struct ehea_adapter *adapter, u64 eqe)
>> }
>>
>> if (EHEA_BMASK_GET(NEQE_EXTSWITCH_PORT_UP, eqe)) {
>> + port->phy_link = EHEA_PHY_LINK_UP;
>> if (netif_msg_link(port))
>> ehea_info("%s: Physical port up",
>> port->netdev->name);
>> + if (prop_carrier_state)
>> + netif_carrier_on(port->netdev);
>> } else {
>> + port->phy_link = EHEA_PHY_LINK_DOWN;
>> if (netif_msg_link(port))
>> ehea_info("%s: Physical port down",
>> port->netdev->name);
>> + if (prop_carrier_state)
>> + netif_carrier_off(port->netdev);
maybe it was better to code this as 'ehea_carrier_off/on()' which then tests
(prop_carrier_state) - this now begs for regressions where this isn't properly
done in future commits, and on top of that there are all these extra conditions now.
Cheers,
Auke