Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH] Documentation: broadcom-bcmgenet: fix address and cells properties
From: Florian Fainelli @ 2014-02-18  4:37 UTC (permalink / raw)
  To: netdev; +Cc: davem, sergei.shtylyov
In-Reply-To: <1392683255-28136-1-git-send-email-f.fainelli@gmail.com>

Le lundi 17 février 2014, 16:27:35 Florian Fainelli a écrit :
> This patch fixes a typo in the Device Tree binding for the
> leading '#'.
> 
> Reported-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---

Forgot to mention that this patch is for the net-next tree. Thanks!

>  Documentation/devicetree/bindings/net/broadcom-bcmgenet.txt | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/broadcom-bcmgenet.txt
> b/Documentation/devicetree/bindings/net/broadcom-bcmgenet.txt index
> 88c3d04..25339ee 100644
> --- a/Documentation/devicetree/bindings/net/broadcom-bcmgenet.txt
> +++ b/Documentation/devicetree/bindings/net/broadcom-bcmgenet.txt
> @@ -10,8 +10,8 @@ Required properties:
>  - phy-mode: String, operation mode of the PHY interface. Supported values
> are "mii", "rgmii", "rgmii-txid", "rev-mii", "moca". Analogous to ePAPR
> "phy-connection-type" values
> -- address-cells: should be 1
> -- size-cells: should be 1
> +- #address-cells: should be 1
> +- #size-cells: should be 1
> 
>  Optional properties:
>  - clocks: When provided, must be two phandles to the functional clocks
> nodes @@ -42,8 +42,8 @@ MDIO bus node required properties:
>    parent node compatible property (e.g: brcm,genet-v4 pairs with
>    brcm,genet-mdio-v4)
>  - reg: address and length relative to the parent node base register address
> -- address-cells: address cell for MDIO bus addressing, should be 1 --
> size-cells: size of the cells for MDIO bus addressing, should be 0 +-
> #address-cells: address cell for MDIO bus addressing, should be 1 +-
> #size-cells: size of the cells for MDIO bus addressing, should be 0
> 
>  Ethernet PHY node properties:

-- 
Florian

^ permalink raw reply

* Re: [PATCH v4 net-next 01/12] bonding: remove bond->lock from bond_arp_rcv
From: Ding Tianhong @ 2014-02-18  4:02 UTC (permalink / raw)
  To: Veaceslav Falico, netdev; +Cc: Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1392648088-21336-2-git-send-email-vfalico@redhat.com>

On 2014/2/17 22:41, Veaceslav Falico wrote:
> We're always called with rcu_read_lock() held (bond_arp_rcv() is only
> called from bond_handle_frame(), which is rx_handler and always called
> under rcu from __netif_receive_skb_core() ).
> 
> The slave active/passive and/or bonding params can change in-flight, however
> we don't really care about that - we only modify the last time packet was
> received, which is harmless.
> 
> CC: Jay Vosburgh <fubar@us.ibm.com>
> CC: Andy Gospodarek <andy@greyhouse.net>
> Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
> ---
>  drivers/net/bonding/bond_main.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 3bce855..3c50bec 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -2260,8 +2260,6 @@ int bond_arp_rcv(const struct sk_buff *skb, struct bonding *bond,
>  	if (skb->protocol != __cpu_to_be16(ETH_P_ARP))
>  		return RX_HANDLER_ANOTHER;
>  
> -	read_lock(&bond->lock);
> -
>  	if (!slave_do_arp_validate(bond, slave))
>  		goto out_unlock;
>  
> @@ -2318,7 +2316,6 @@ int bond_arp_rcv(const struct sk_buff *skb, struct bonding *bond,
>  		bond_validate_arp(bond, slave, tip, sip);
>  
>  out_unlock:
> -	read_unlock(&bond->lock);
>  	if (arp != (struct arphdr *)skb->data)
>  		kfree(arp);
>  	return RX_HANDLER_ANOTHER;
> 

I think it is not enough, you should add rcu_dereference for bond->curr_active_slave, it may be changed during
the recv processing.

Regards
Ding

^ permalink raw reply

* Re: [PATCH net-next 1/3] bonding: add bond_set_slave_state/flags()
From: Ding Tianhong @ 2014-02-18  3:50 UTC (permalink / raw)
  To: Jay Vosburgh
  Cc: vfalico, andy, cwang, jiri, thomas, eric.dumazet, sfeldma, davem,
	netdev
In-Reply-To: <5329.1392689302@death.nxdomain>

On 2014/2/18 10:08, Jay Vosburgh wrote:
> Ding Tianhong <dingtianhong@huawei.com> wrote:
> 
>> The new function could change the slave state and flags, then call
>> rtmsg_ifinfo() according to the input parameters notify.
>>
>> Cc: Jay Vosburgh <fubar@us.ibm.com>
>> Cc: Veaceslav Falico <vfalico@redhat.com>
>> Cc: Andy Gospodarek <andy@greyhouse.net>
>> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
>> ---
>> drivers/net/bonding/bonding.h | 26 ++++++++++++++++++++++++++
>> 1 file changed, 26 insertions(+)
>>
>> diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
>> index 86ccfb9..d210124 100644
>> --- a/drivers/net/bonding/bonding.h
>> +++ b/drivers/net/bonding/bonding.h
>> @@ -303,6 +303,18 @@ static inline void bond_set_backup_slave(struct slave *slave)
>> 	}
>> }
>>
>> +static inline void bond_set_slave_state(struct slave *slave,
>> +					int slave_state, bool notify)
>> +{
>> +	if (slave->backup != slave_state)
>> +		slave->backup = slave_state;
>> +	else
>> +		return;
>> +
>> +	if (notify)
>> +		rtmsg_ifinfo(RTM_NEWLINK, slave->dev, 0, GFP_KERNEL);
> 
> 	I think this would be clearer if coded as:
> 
> 	if (slave->backup == slave_slave)
> 		return;
> 
> 	slave->backup = slave_state;
> 	if (notify)
> 		rtmsg_ifinfo(RTM_NEWLINK, slave->dev, 0, GFP_KERNEL);
> 
Yes, more simple.

>> +}
>> +
>> static inline void bond_slave_state_change(struct bonding *bond)
>> {
>> 	struct list_head *iter;
>> @@ -408,6 +420,20 @@ static inline void bond_set_slave_active_flags(struct slave *slave)
>> 	slave->inactive = 0;
>> }
>>
>> +static inline void bond_set_slave_flags(struct slave *slave,
>> +					int state, bool notify)
>> +
>> +{
>> +	if (state == BOND_STATE_ACTIVE) {
>> +		bond_set_slave_state(slave, state, notify);
>> +		slave->inactive = 0;
>> +	} else if (state == BOND_STATE_BACKUP && !bond_is_lb(slave->bond)) {
>> +		bond_set_slave_state(slave, state, notify);
>> +		if (!slave->bond->params.all_slaves_active)
>> +			slave->inactive = 1;
>> +	}
>> +}
> 
> 	As I said in my other reply, I don't see why this shouldn't be
> integrated into the existing state change functions instead of creating
> a new function.
> 
> 	-J

Ok, thanks.

Ding

> 
>> static inline bool bond_is_slave_inactive(struct slave *slave)
>> {
>> 	return slave->inactive;
>> -- 
>> 1.8.0
> 
> ---
> 	-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> .
> 

^ permalink raw reply

* Re: [PATCH net-next 3/3] bonding: Fix the RTNL assertion failed for 802.3ad state machine
From: Ding Tianhong @ 2014-02-18  3:47 UTC (permalink / raw)
  To: Jay Vosburgh
  Cc: vfalico, andy, cwang, jiri, thomas, eric.dumazet, sfeldma, davem,
	netdev
In-Reply-To: <5300.1392689172@death.nxdomain>

On 2014/2/18 10:06, Jay Vosburgh wrote:
> Ding Tianhong <dingtianhong@huawei.com> wrote:
> 
>> The 802.3ad state machine don't run in RTNL, but when the slave's
>> state changed, the rtmsg_ifinfo will be called, it will cause
>> warning message because the RTML is not locked, acquiring RTNL
>> for the __enable_port and __disable_port cases is difficult, as
>> those calls generally already hold the state machine lock, and
>> can't unconditionally call rtnl_lock because either they already
>> hold RTNL (for calls via bond_3ad_unbind_slave) or due to the
>> potential for deadlock with bond_3ad_adapter_speed_changed,
>> bond_3ad_adapter_duplex_changed, bond_3ad_link_change, or
>> bond_3ad_update_lacp_rate. All four of those are called with RTNL
>> held, and acquire the state machine lock second, The calling contexts for
>> __enable_port and __disable_port already hold the state machine lock,
>> and may or may not need RTNL.
>>
>> So according to the Jay's opinion, the __enable_port and __disable_port
>> should not call rtmsg_ifinfo in the state machine lock, any change in
>> the state of slave could set a flag in the slave, it will indicated that
>> an rtmsg_ifinfo should be called at the end of the state machine.
> 
> 	To clarify, my opinion being referenced here was really asking
> Scott Feldman <sfeldma@cumulusnetworks.com> if: (a) the calls had to be
> synchronous, and, (b) if the intermediate calls to adjust flags within
> the ARP monitor "cycle through slaves looking for a functional slave"
> all required notifications.  My suspicion is that the answer to both of
> those is "no," but I haven't heard from Scott.
> 

Yes, this question has been in existence for a long time, and I admin your opinions, it is very
clear and reasonable, so I try to fix it by this way, in my original idea, I think not only
the 802.3ad state machine, but the ab, loadbalance monitor is still need to modify, the existing
solution for bond_ab_arp_probe which I think is not good enough, so I think it will be a big
patchset, so I send this patchset just only fix for 802.3ad for review.

>> Cc: Jay Vosburgh <fubar@us.ibm.com>
>> Cc: Veaceslav Falico <vfalico@redhat.com>
>> Cc: Andy Gospodarek <andy@greyhouse.net>
>> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
>> ---
>> drivers/net/bonding/bond_3ad.c | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
>> index cce1f1b..e80b78f 100644
>> --- a/drivers/net/bonding/bond_3ad.c
>> +++ b/drivers/net/bonding/bond_3ad.c
>> @@ -181,7 +181,7 @@ static inline int __agg_has_partner(struct aggregator *agg)
>>  */
>> static inline void __disable_port(struct port *port)
>> {
>> -	bond_set_slave_inactive_flags(port->slave);
>> +	bond_set_slave_flags(port->slave, BOND_STATE_BACKUP, false);
>> }
>>
>> /**
>> @@ -193,7 +193,7 @@ static inline void __enable_port(struct port *port)
>> 	struct slave *slave = port->slave;
>>
>> 	if ((slave->link == BOND_LINK_UP) && IS_UP(slave->dev))
>> -		bond_set_slave_active_flags(slave);
>> +		bond_set_slave_flags(slave, BOND_STATE_ACTIVE, false);
> 
> 	I don't agree that we need to have two separate systems (your
> new bond_set_slave_flags plus bond_set_slave_{active,inactive}_flags)
> that both tweak the "active" or "inactive" flags for a slave.  It would
> be much cleaner and consistent with the current code to add a "notify"
> boolean to the existing functions.
> 
> 	-J

Yep, this problem has troubled me for a long time, whether to add a new function
or modify current function, I think your answer has gave me the right direction,
thanks.

Regards
Ding 

> 
>> }
>>
>> /**
>> @@ -2123,6 +2123,7 @@ void bond_3ad_state_machine_handler(struct work_struct *work)
>> re_arm:
>> 	rcu_read_unlock();
>> 	read_unlock(&bond->lock);
>> +	bond_slave_state_notify(bond, false);
>> 	queue_delayed_work(bond->wq, &bond->ad_work, ad_delta_in_ticks);
>> }
>>
>> -- 
>> 1.8.0
> 
> ---
> 	-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> .
> 

^ permalink raw reply

* Re: [PATCH net-next 2/3] bonding: add new slave param and bond_slave_state_notify()
From: Ding Tianhong @ 2014-02-18  3:49 UTC (permalink / raw)
  To: Jay Vosburgh
  Cc: vfalico, andy, cwang, jiri, thomas, eric.dumazet, sfeldma, davem,
	netdev
In-Reply-To: <5310.1392689226@death.nxdomain>

On 2014/2/18 10:07, Jay Vosburgh wrote:
> Ding Tianhong <dingtianhong@huawei.com> wrote:
> 
>> Add a new slave parameter which called should_notify, if the slave's state
>> changed and don't notify yet, the parameter will be set to 1, and then if
>> the slave's state changed again, the param will be set to 0, it indicate that
>> the slave's state has been restored, no need to notify any one.
>>
>> The bond_slave_state_notify() will check whether the status changed and then
>> decide to notify or not.
>>
>> Cc: Jay Vosburgh <fubar@us.ibm.com>
>> Cc: Veaceslav Falico <vfalico@redhat.com>
>> Cc: Andy Gospodarek <andy@greyhouse.net>
>> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
>> ---
>> drivers/net/bonding/bonding.h | 44 +++++++++++++++++++++++++++++++++++++++++--
>> 1 file changed, 42 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
>> index d210124..4d0cd41 100644
>> --- a/drivers/net/bonding/bonding.h
>> +++ b/drivers/net/bonding/bonding.h
>> @@ -195,7 +195,8 @@ struct slave {
>> 	s8     new_link;
>> 	u8     backup:1,   /* indicates backup slave. Value corresponds with
>> 			      BOND_STATE_ACTIVE and BOND_STATE_BACKUP */
>> -	       inactive:1; /* indicates inactive slave */
>> +	       inactive:1, /* indicates inactive slave */
>> +	       should_notify:1; /* indicateds whether the state changed */
>> 	u8     duplex;
>> 	u32    original_mtu;
>> 	u32    link_failure_count;
>> @@ -311,8 +312,47 @@ static inline void bond_set_slave_state(struct slave *slave,
>> 	else
>> 		return;
>>
>> -	if (notify)
>> +	if (notify) {
>> 		rtmsg_ifinfo(RTM_NEWLINK, slave->dev, 0, GFP_KERNEL);
>> +		slave->should_notify = 0;
>> +	} else {
>> +		if (slave->should_notify)
>> +			slave->should_notify = 0;
>> +		else
>> +			slave->should_notify = 1;
>> +	}
>> +}
>> +
>> +static inline void bond_slave_state_notify(struct bonding *bond,
>> +					   bool rtnl_locked)
>> +{
>> +	struct list_head *iter;
>> +	struct slave *tmp;
>> +
>> +	rcu_read_lock();
>> +	bond_for_each_slave_rcu(bond, tmp, iter) {
>> +		if (tmp->should_notify) {
>> +			rcu_read_unlock();
>> +			goto should_notify;
>> +		}
>> +	}
>> +	rcu_read_unlock();
>> +	return;
>> +
>> +should_notify:
>> +
>> +	if (!rtnl_locked && !rtnl_trylock())
>> +		return;
>> +
>> +	bond_for_each_slave(bond, tmp, iter) {
>> +		if (tmp->should_notify) {
>> +			rtmsg_ifinfo(RTM_NEWLINK, tmp->dev, 0, GFP_KERNEL);
>> +			tmp->should_notify = 0;
>> +		}
>> +	}
>> +
>> +	if (!rtnl_locked)
>> +		rtnl_unlock();
>> }
> 
> 	This function (bond_slave_state_notify) seems overly complicated
> given that there appears to be only one caller.  In particular, why
> bother with the "rtnl_locked" flag at all, when it is never called with
> it set to true?  Really, with only one caller (in patch 3 of the
> series), I'm not convinced this even needs to be a separate function.
> 
> 	-J
> 
In my original opinion, I think it may be used in RTNL for other monitor,
so add this one, I will remove it, thanks.

Regards
Ding

>>
>> static inline void bond_slave_state_change(struct bonding *bond)
>> -- 
>> 1.8.0
> 
> ---
> 	-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
> 
> 
> .
> 

^ permalink raw reply

* Fw:[Bug 70471] xfrm policy node will double unlink.
From: Xianpeng Zhao @ 2014-02-18  2:55 UTC (permalink / raw)
  To: netdev; +Cc: alan

Hi Group,
     I found a problem about xfrm policy.

     In corner case, xfrm policy node will be double unlinked from the list.

    The scenario like this:
    In thread context, After removed the node from list, before remove the xfrm policy expire timer. At this point, a timer interrupt come, and call the run_timer_softirq to execute the xfrm_policy_timer to remove the expired policy node; because this policy node had already removed from list. this remove will cause the node double unlinked.

     I have done such patch to protect this case. I am not sure here the policy->walk.dead means. From the name, I think it mark the policy node was dead. Maybe I use this flag is not correct, please the expert correct me if I am wrong. More detail information can reference bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=70471

        my patch:
--- net/xfrm/xfrm_policy.c_old	2014-02-10 10:18:28.421504317 +0800
+++ net/xfrm/xfrm_policy.c	2014-02-10 10:19:01.661503334 +0800
@@ -330,7 +330,6 @@ static void xfrm_queue_purge(struct sk_b
 
 static void xfrm_policy_kill(struct xfrm_policy *policy)
 {
-	policy->walk.dead = 1;
 
 	atomic_inc(&policy->genid);
 
@@ -1156,6 +1155,7 @@ static struct xfrm_policy *__xfrm_policy
 	if (hlist_unhashed(&pol->bydst))
 		return NULL;
 
+	pol->walk.dead = 1;
 	hlist_del(&pol->bydst);
 	hlist_del(&pol->byidx);
 	list_del(&pol->walk.all);

------------------
Best Regards
Xianpeng

^ permalink raw reply

* Re: [PATCH net-next 1/3] bonding: add bond_set_slave_state/flags()
From: Jay Vosburgh @ 2014-02-18  2:08 UTC (permalink / raw)
  To: Ding Tianhong
  Cc: vfalico, andy, cwang, jiri, thomas, eric.dumazet, sfeldma, davem,
	netdev
In-Reply-To: <1392626151-23916-2-git-send-email-dingtianhong@huawei.com>

Ding Tianhong <dingtianhong@huawei.com> wrote:

>The new function could change the slave state and flags, then call
>rtmsg_ifinfo() according to the input parameters notify.
>
>Cc: Jay Vosburgh <fubar@us.ibm.com>
>Cc: Veaceslav Falico <vfalico@redhat.com>
>Cc: Andy Gospodarek <andy@greyhouse.net>
>Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
>---
> drivers/net/bonding/bonding.h | 26 ++++++++++++++++++++++++++
> 1 file changed, 26 insertions(+)
>
>diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
>index 86ccfb9..d210124 100644
>--- a/drivers/net/bonding/bonding.h
>+++ b/drivers/net/bonding/bonding.h
>@@ -303,6 +303,18 @@ static inline void bond_set_backup_slave(struct slave *slave)
> 	}
> }
>
>+static inline void bond_set_slave_state(struct slave *slave,
>+					int slave_state, bool notify)
>+{
>+	if (slave->backup != slave_state)
>+		slave->backup = slave_state;
>+	else
>+		return;
>+
>+	if (notify)
>+		rtmsg_ifinfo(RTM_NEWLINK, slave->dev, 0, GFP_KERNEL);

	I think this would be clearer if coded as:

	if (slave->backup == slave_slave)
		return;

	slave->backup = slave_state;
	if (notify)
		rtmsg_ifinfo(RTM_NEWLINK, slave->dev, 0, GFP_KERNEL);

>+}
>+
> static inline void bond_slave_state_change(struct bonding *bond)
> {
> 	struct list_head *iter;
>@@ -408,6 +420,20 @@ static inline void bond_set_slave_active_flags(struct slave *slave)
> 	slave->inactive = 0;
> }
>
>+static inline void bond_set_slave_flags(struct slave *slave,
>+					int state, bool notify)
>+
>+{
>+	if (state == BOND_STATE_ACTIVE) {
>+		bond_set_slave_state(slave, state, notify);
>+		slave->inactive = 0;
>+	} else if (state == BOND_STATE_BACKUP && !bond_is_lb(slave->bond)) {
>+		bond_set_slave_state(slave, state, notify);
>+		if (!slave->bond->params.all_slaves_active)
>+			slave->inactive = 1;
>+	}
>+}

	As I said in my other reply, I don't see why this shouldn't be
integrated into the existing state change functions instead of creating
a new function.

	-J

> static inline bool bond_is_slave_inactive(struct slave *slave)
> {
> 	return slave->inactive;
>-- 
>1.8.0

---
	-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com

^ permalink raw reply

* Re: [PATCH net-next 2/3] bonding: add new slave param and bond_slave_state_notify()
From: Jay Vosburgh @ 2014-02-18  2:07 UTC (permalink / raw)
  To: Ding Tianhong
  Cc: vfalico, andy, cwang, jiri, thomas, eric.dumazet, sfeldma, davem,
	netdev
In-Reply-To: <1392626151-23916-3-git-send-email-dingtianhong@huawei.com>

Ding Tianhong <dingtianhong@huawei.com> wrote:

>Add a new slave parameter which called should_notify, if the slave's state
>changed and don't notify yet, the parameter will be set to 1, and then if
>the slave's state changed again, the param will be set to 0, it indicate that
>the slave's state has been restored, no need to notify any one.
>
>The bond_slave_state_notify() will check whether the status changed and then
>decide to notify or not.
>
>Cc: Jay Vosburgh <fubar@us.ibm.com>
>Cc: Veaceslav Falico <vfalico@redhat.com>
>Cc: Andy Gospodarek <andy@greyhouse.net>
>Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
>---
> drivers/net/bonding/bonding.h | 44 +++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 42 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
>index d210124..4d0cd41 100644
>--- a/drivers/net/bonding/bonding.h
>+++ b/drivers/net/bonding/bonding.h
>@@ -195,7 +195,8 @@ struct slave {
> 	s8     new_link;
> 	u8     backup:1,   /* indicates backup slave. Value corresponds with
> 			      BOND_STATE_ACTIVE and BOND_STATE_BACKUP */
>-	       inactive:1; /* indicates inactive slave */
>+	       inactive:1, /* indicates inactive slave */
>+	       should_notify:1; /* indicateds whether the state changed */
> 	u8     duplex;
> 	u32    original_mtu;
> 	u32    link_failure_count;
>@@ -311,8 +312,47 @@ static inline void bond_set_slave_state(struct slave *slave,
> 	else
> 		return;
>
>-	if (notify)
>+	if (notify) {
> 		rtmsg_ifinfo(RTM_NEWLINK, slave->dev, 0, GFP_KERNEL);
>+		slave->should_notify = 0;
>+	} else {
>+		if (slave->should_notify)
>+			slave->should_notify = 0;
>+		else
>+			slave->should_notify = 1;
>+	}
>+}
>+
>+static inline void bond_slave_state_notify(struct bonding *bond,
>+					   bool rtnl_locked)
>+{
>+	struct list_head *iter;
>+	struct slave *tmp;
>+
>+	rcu_read_lock();
>+	bond_for_each_slave_rcu(bond, tmp, iter) {
>+		if (tmp->should_notify) {
>+			rcu_read_unlock();
>+			goto should_notify;
>+		}
>+	}
>+	rcu_read_unlock();
>+	return;
>+
>+should_notify:
>+
>+	if (!rtnl_locked && !rtnl_trylock())
>+		return;
>+
>+	bond_for_each_slave(bond, tmp, iter) {
>+		if (tmp->should_notify) {
>+			rtmsg_ifinfo(RTM_NEWLINK, tmp->dev, 0, GFP_KERNEL);
>+			tmp->should_notify = 0;
>+		}
>+	}
>+
>+	if (!rtnl_locked)
>+		rtnl_unlock();
> }

	This function (bond_slave_state_notify) seems overly complicated
given that there appears to be only one caller.  In particular, why
bother with the "rtnl_locked" flag at all, when it is never called with
it set to true?  Really, with only one caller (in patch 3 of the
series), I'm not convinced this even needs to be a separate function.

	-J

>
> static inline void bond_slave_state_change(struct bonding *bond)
>-- 
>1.8.0

---
	-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com

^ permalink raw reply

* Re: [PATCH net-next 3/3] bonding: Fix the RTNL assertion failed for 802.3ad state machine
From: Jay Vosburgh @ 2014-02-18  2:06 UTC (permalink / raw)
  To: Ding Tianhong
  Cc: vfalico, andy, cwang, jiri, thomas, eric.dumazet, sfeldma, davem,
	netdev
In-Reply-To: <1392626151-23916-4-git-send-email-dingtianhong@huawei.com>

Ding Tianhong <dingtianhong@huawei.com> wrote:

>The 802.3ad state machine don't run in RTNL, but when the slave's
>state changed, the rtmsg_ifinfo will be called, it will cause
>warning message because the RTML is not locked, acquiring RTNL
>for the __enable_port and __disable_port cases is difficult, as
>those calls generally already hold the state machine lock, and
>can't unconditionally call rtnl_lock because either they already
>hold RTNL (for calls via bond_3ad_unbind_slave) or due to the
>potential for deadlock with bond_3ad_adapter_speed_changed,
>bond_3ad_adapter_duplex_changed, bond_3ad_link_change, or
>bond_3ad_update_lacp_rate. All four of those are called with RTNL
>held, and acquire the state machine lock second, The calling contexts for
>__enable_port and __disable_port already hold the state machine lock,
>and may or may not need RTNL.
>
>So according to the Jay's opinion, the __enable_port and __disable_port
>should not call rtmsg_ifinfo in the state machine lock, any change in
>the state of slave could set a flag in the slave, it will indicated that
>an rtmsg_ifinfo should be called at the end of the state machine.

	To clarify, my opinion being referenced here was really asking
Scott Feldman <sfeldma@cumulusnetworks.com> if: (a) the calls had to be
synchronous, and, (b) if the intermediate calls to adjust flags within
the ARP monitor "cycle through slaves looking for a functional slave"
all required notifications.  My suspicion is that the answer to both of
those is "no," but I haven't heard from Scott.

>Cc: Jay Vosburgh <fubar@us.ibm.com>
>Cc: Veaceslav Falico <vfalico@redhat.com>
>Cc: Andy Gospodarek <andy@greyhouse.net>
>Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
>---
> drivers/net/bonding/bond_3ad.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
>index cce1f1b..e80b78f 100644
>--- a/drivers/net/bonding/bond_3ad.c
>+++ b/drivers/net/bonding/bond_3ad.c
>@@ -181,7 +181,7 @@ static inline int __agg_has_partner(struct aggregator *agg)
>  */
> static inline void __disable_port(struct port *port)
> {
>-	bond_set_slave_inactive_flags(port->slave);
>+	bond_set_slave_flags(port->slave, BOND_STATE_BACKUP, false);
> }
>
> /**
>@@ -193,7 +193,7 @@ static inline void __enable_port(struct port *port)
> 	struct slave *slave = port->slave;
>
> 	if ((slave->link == BOND_LINK_UP) && IS_UP(slave->dev))
>-		bond_set_slave_active_flags(slave);
>+		bond_set_slave_flags(slave, BOND_STATE_ACTIVE, false);

	I don't agree that we need to have two separate systems (your
new bond_set_slave_flags plus bond_set_slave_{active,inactive}_flags)
that both tweak the "active" or "inactive" flags for a slave.  It would
be much cleaner and consistent with the current code to add a "notify"
boolean to the existing functions.

	-J

> }
>
> /**
>@@ -2123,6 +2123,7 @@ void bond_3ad_state_machine_handler(struct work_struct *work)
> re_arm:
> 	rcu_read_unlock();
> 	read_unlock(&bond->lock);
>+	bond_slave_state_notify(bond, false);
> 	queue_delayed_work(bond->wq, &bond->ad_work, ad_delta_in_ticks);
> }
>
>-- 
>1.8.0

---
	-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com

^ permalink raw reply

* linux-next: manual merge of the net-next tree with the net tree
From: Stephen Rothwell @ 2014-02-18  1:52 UTC (permalink / raw)
  To: David Miller, netdev; +Cc: linux-next, linux-kernel, dingtianhong, Joe Perches

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

Hi all,

Today's linux-next merge of the net-next tree got a conflict in
drivers/net/bonding/bond_main.c between commit f80889a5b79c ("bonding:
Fix deadlock in bonding driver when using netpoll") from the net tree and
commit 90194264ceff ("bonding: Neaten pr_<level>") from the net-next tree.

I fixed it up (see below) and can carry the fix as necessary (no action
is required).

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

diff --cc drivers/net/bonding/bond_main.c
index 1c6104d3501d,3bce855e627b..000000000000
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@@ -1547,13 -1544,12 +1545,13 @@@ int bond_enslave(struct net_device *bon
  		write_lock_bh(&bond->curr_slave_lock);
  		bond_select_active_slave(bond);
  		write_unlock_bh(&bond->curr_slave_lock);
 +		unblock_netpoll_tx();
  	}
  
- 	pr_info("%s: enslaving %s as a%s interface with a%s link.\n",
+ 	pr_info("%s: Enslaving %s as %s interface with %s link\n",
  		bond_dev->name, slave_dev->name,
- 		bond_is_active_slave(new_slave) ? "n active" : " backup",
- 		new_slave->link != BOND_LINK_DOWN ? "n up" : " down");
+ 		bond_is_active_slave(new_slave) ? "an active" : "a backup",
+ 		new_slave->link != BOND_LINK_DOWN ? "an up" : "a down");
  
  	/* enslave is successful */
  	return 0;
@@@ -2865,11 -2858,9 +2862,11 @@@ static int bond_slave_netdev_event(unsi
  			break;
  		}
  
- 		pr_info("%s: Primary slave changed to %s, reselecting active slave.\n",
- 			bond->dev->name, bond->primary_slave ? slave_dev->name :
- 							       "none");
+ 		pr_info("%s: Primary slave changed to %s, reselecting active slave\n",
+ 			bond->dev->name,
+ 			bond->primary_slave ? slave_dev->name : "none");
 +
 +		block_netpoll_tx();
  		write_lock_bh(&bond->curr_slave_lock);
  		bond_select_active_slave(bond);
  		write_unlock_bh(&bond->curr_slave_lock);

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

^ permalink raw reply

* linux-next: manual merge of the net-next tree with the net tree
From: Stephen Rothwell @ 2014-02-18  1:48 UTC (permalink / raw)
  To: David Miller, netdev; +Cc: linux-next, linux-kernel, Jiri Bohac, Joe Perches

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

Hi all,

Today's linux-next merge of the net-next tree got a conflict in
drivers/net/bonding/bond_3ad.h between commit 163c8ff30dbe ("bonding:
802.3ad: make aggregator_identifier bond-private") from the net tree and
commit 2ea24f2ecfdc ("bonding: Convert c99 comments") from the net-next
tree.

I fixed it up (see below) and can carry the fix as necessary (no action
is required).

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

diff --cc drivers/net/bonding/bond_3ad.h
index f4dd9592ac62,3b97fe487dca..000000000000
--- a/drivers/net/bonding/bond_3ad.h
+++ b/drivers/net/bonding/bond_3ad.h
@@@ -251,9 -251,8 +251,9 @@@ struct ad_system 
  #define SLAVE_AD_INFO(slave) ((slave)->ad_info)
  
  struct ad_bond_info {
- 	struct ad_system system;	    /* 802.3ad system structure */
- 	u32 agg_select_timer;	    // Timer to select aggregator after all adapter's hand shakes
+ 	struct ad_system system;	/* 802.3ad system structure */
+ 	u32 agg_select_timer;		/* Timer to select aggregator after all adapter's hand shakes */
 +	u16 aggregator_identifier;
  };
  
  struct ad_slave_info {

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

^ permalink raw reply

* [PATCH v2] ethtool: check the ethtool_ops is NULL in dev_ethtool
From: Wang Weidong @ 2014-02-18  1:42 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Daniel Borkmann
In-Reply-To: <5301F32C.4040704@huawei.com>

some drivers maybe not implement the ethtool_ops with only
set NULL to ethtool_ops. So when call the ethtool devx will
lead to a 'NULL pointer dereference'.

So add a check in dev_ethtool

Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
---
Change note:

v2: fix a trailing whitespace/tab pointed out by Daniel

---
 net/core/ethtool.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 30071de..c8cfd8f 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -1500,6 +1500,9 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
 			return -EPERM;
 	}
 
+	if (!dev->ethtool_ops)
+		return -EOPNOTSUPP;
+
 	if (dev->ethtool_ops->begin) {
 		rc = dev->ethtool_ops->begin(dev);
 		if (rc  < 0)
-- 
1.7.12

^ permalink raw reply related

* Re: [PATCH] ethtool: check the ethtool_ops is NULL in dev_ethtool
From: Wang Weidong @ 2014-02-18  1:20 UTC (permalink / raw)
  To: Daniel Borkmann; +Cc: David Miller, netdev
In-Reply-To: <53024266.8010700@redhat.com>

On 2014/2/18 1:09, Daniel Borkmann wrote:
> On 02/17/2014 12:31 PM, Wang Weidong wrote:
>> some drivers maybe not implement the ethtool_ops with only
>> set NULL. So when call the ethtool cmds will lead to a
>> 'NULL pointer dereference'.
>>
>> So add a checking in dev_ethtool.
>>
>> Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
>> ---
>>   net/core/ethtool.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
>> index 30071de..f418dcb 100644
>> --- a/net/core/ethtool.c
>> +++ b/net/core/ethtool.c
>> @@ -1499,6 +1499,9 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
>>           if (!ns_capable(net->user_ns, CAP_NET_ADMIN))
>>               return -EPERM;
>>       }
>> +   
> 
> You have a trailing whitespace/tab in the line above. Please
> use checkpatch for detecting such things.
> 
Sorry for that. I will fix it soon.

> Can you be more specific with "some drivers"? Any driver that
> is in the mainline tree?
> 
No. My team implements a driver without considering the ethtool_ops.
So I got the problem.

>> +    if (!dev->ethtool_ops)
>> +        return -EOPNOTSUPP;
>>
>>       if (dev->ethtool_ops->begin) {
>>           rc = dev->ethtool_ops->begin(dev);
>>
> 
> 

^ permalink raw reply

* Re: [PATCH net] bonding: fix arp requests sends with isolated routes
From: Jay Vosburgh @ 2014-02-18  1:07 UTC (permalink / raw)
  To: David Miller; +Cc: f.cachereul, vfalico, andy, netdev
In-Reply-To: <20140217.145635.1123180851794758928.davem@davemloft.net>

David Miller <davem@davemloft.net> wrote:

>From: François Cachereul <f.cachereul@alphalink.fr>
>Date: Fri, 14 Feb 2014 16:59:23 +0100
>
>> Make arp_send_all() try to send arp packets through slave devices event
>> if no route to arp_ip_target is found. This is useful when the route
>> is in an isolated routing table with routing rule parameters like oif or
>> iif in which case ip_route_output() return an error.
>> Thus, the arp packet is send without vlan and with the bond ip address
>> as sender.
>> 
>> Signed-off-by: François CACHEREUL <f.cachereul@alphalink.fr>
>> ---
>> This previously worked, the problem was added in 2.6.35 with vlan 0
>> added by default when the module 8021q is loaded. Before that no route
>> lookup was done if the bond device did not have any vlan. The problem
>> now exists event if the module 8021q is not loaded.
>
>I don't like this at all, you're trying to paper over the fact that we
>can't set the flow key correctly at this point.
>
>Just assuming the route might be there and trying anyways is not really
>acceptable in my opinion.  There's a reason we do a route lookup at all.

	The reason for the route lookup is to get a VLAN ID for the
outgoing ARP (if VLANs are configured above the bond), so it can be
correctly tagged.

	As Francois says, older versions of the bond_arp_send_all
function would skip the route lookup entirely if there were no VLANs
configured above the bond.  E.g., the original logic from a 2.6.32-era
kernel looks like:

	for (i = 0; (i < BOND_MAX_ARP_TARGETS); i++) {
[...]
		if (!bond->vlgrp) {
			pr_debug("basa: empty vlan: arp_send\n");
			bond_arp_send(slave->dev, ARPOP_REQUEST, targets[i],
				      bond->master_ip, 0);
			continue;
		}

		/*
		 * If VLANs are configured, we do a route lookup to
		 * determine which VLAN interface would be used, so we
		 * can tag the ARP with the proper VLAN tag.
		 */
		memset(&fl, 0, sizeof(fl));
		fl.fl4_dst = targets[i];
		fl.fl4_tos = RTO_ONLINK;

		rv = ip_route_output_key(&init_net, &rt, &fl);
[...]

	So, in the past, this particular case (oif / iif in route
selection) would "work," in the sense that an ARP would go out with no
VLAN ID, but only when there were known to be no VLANs configured above
the bond.  If any VLANs were configured above the bond, this case would
fail as we're seeing here.

	Nowadays, there is no easy way to tell if there are VLANs above
the bond, and there's generally a VID 0 configured anyway, so the route
lookup is unconditional.  In the case at issue here (the route lookup
for the arp_ip_target IP address fails), it's not possible for bonding
to determine what interface would be used, and therefore what VLAN tag
to use.

	Francois's patch would make bonding essentially take a best
guess of "no VLAN" and send an untagged ARP for any destination not
found in the regular (no iif, oif, etc, rule) routing table, which is
what used to happen for the "known no VLAN" case.

	With the patch, these ARPs may have an all-zero source IP
address (since the bond_confirm_addr call may not find a suitable source
address for something it can't find a route to).  That is a legal ARP
(used for duplicate address detection according to RFC 2131), but when
last I tried it a couple of years ago, the replies won't pass
arp_validate (as the target IP of 0.0.0.0 in the reply doesn't match any
of the bond's IP address), and I suspect that hasn't changed.

	In the days of yore code above, bonding kept track of what it
thought the bond's IP address was (bond->master_ip), and used that as
the source IP in the ARPs.  That wasn't always correct if the bond had
multiple IP addresses.

	So, ultimately, Francois is correct that this is a regression of
a behavior that used to work.  On the other hand, this patch isn't
really a complete restoration of the prior behavior.  It's no longer
possible to know that there aren't any VLANs above the bond, and so the
"no VLAN" guess is much less reliable than it used to be, plus the ARPs
that will be generated probably won't work with arp_validate.

	As much as I loathe adding more options to bonding, a manually
selected "force VLAN ID" for the arp_ip_target(s) would resolve this for
the minority of cases where the automatic VLAN ID selection does not
function.

	-J

---
	-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com

^ permalink raw reply

* [PATCH] Documentation: broadcom-bcmgenet: fix address and cells properties
From: Florian Fainelli @ 2014-02-18  0:27 UTC (permalink / raw)
  To: netdev; +Cc: davem, sergei.shtylyov, Florian Fainelli

This patch fixes a typo in the Device Tree binding for the
leading '#'.

Reported-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 Documentation/devicetree/bindings/net/broadcom-bcmgenet.txt | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/broadcom-bcmgenet.txt b/Documentation/devicetree/bindings/net/broadcom-bcmgenet.txt
index 88c3d04..25339ee 100644
--- a/Documentation/devicetree/bindings/net/broadcom-bcmgenet.txt
+++ b/Documentation/devicetree/bindings/net/broadcom-bcmgenet.txt
@@ -10,8 +10,8 @@ Required properties:
 - phy-mode: String, operation mode of the PHY interface. Supported values are
   "mii", "rgmii", "rgmii-txid", "rev-mii", "moca". Analogous to ePAPR
   "phy-connection-type" values
-- address-cells: should be 1
-- size-cells: should be 1
+- #address-cells: should be 1
+- #size-cells: should be 1
 
 Optional properties:
 - clocks: When provided, must be two phandles to the functional clocks nodes
@@ -42,8 +42,8 @@ MDIO bus node required properties:
   parent node compatible property (e.g: brcm,genet-v4 pairs with
   brcm,genet-mdio-v4)
 - reg: address and length relative to the parent node base register address
-- address-cells: address cell for MDIO bus addressing, should be 1
-- size-cells: size of the cells for MDIO bus addressing, should be 0
+- #address-cells: address cell for MDIO bus addressing, should be 1
+- #size-cells: size of the cells for MDIO bus addressing, should be 0
 
 Ethernet PHY node properties:
 
-- 
1.8.3.2

^ permalink raw reply related

* [PATCH v4] sh_eth: add device tree support
From: Sergei Shtylyov @ 2014-02-18  0:12 UTC (permalink / raw)
  To: robh+dt, pawel.moll, mark.rutland, grant.likely, devicetree,
	linux-sh, ijc+devicetree, galak, netdev
  Cc: nobuhiro.iwamatsu.yj, rob, linux-doc

Add support of the device tree probing for the Renesas SH-Mobile SoCs
documenting the device tree binding as necessary.

This work is loosely based on the original patch by Nobuhiro Iwamatsu
<nobuhiro.iwamatsu.yj@renesas.com>.

Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

---
This patch is against DaveM's 'net-next.git' repo but should also apply to the
recent 'renesas.git' repo's 'devel' branch. It assumes the patch documenting all
Ethernet bindings in one file to be applied as well.

Changes in version 4:
- removed devm_kfree() call from error path in sh_eth_parse_dt();
- removed error handling for of_match_device() call as it's guaranteed to
  succeed after DT probing.

Changes in version 3:
- added probing for R8A7791 and R7S72100;
- added irq_of_parse_and_map() call to read PHY IRQ from device tree;
- removed '!phy' check before reading the PHY node's "reg" property;
- replaced "phy-handle" and "phy-mode" property descriptions with references to
  the common Ethernet bindings file;
- added "clocks" required property; 
- removed "local-mac-address" optional property; 
- replaced Armadiallo800EVA board with Lager board in the binding example;
- updated driver's copyrights;
- refreshed the patch.

Changes in version 2:
- added sh_eth_match_table[] entry for "renesas,ether-r8a7778", documented it;
- clarified descriptions of the "reg" and "interrupt" properties;
- moved "interrupt-parent" from required properties to optional;
- mentioned the necessary PHY subnode to the "phy-handle" property description,
  documented the requered "#address-cells" and "#size-cells" properties;
- clarified the types/descriptions of the Renesas specific properties;
- refreshed the patch.

 Documentation/devicetree/bindings/net/sh_eth.txt |   55 ++++++++++++++++++
 drivers/net/ethernet/renesas/sh_eth.c            |   69 ++++++++++++++++++++++-
 2 files changed, 121 insertions(+), 3 deletions(-)

Index: net-next/Documentation/devicetree/bindings/net/sh_eth.txt
===================================================================
--- /dev/null
+++ net-next/Documentation/devicetree/bindings/net/sh_eth.txt
@@ -0,0 +1,55 @@
+* Renesas Electronics SH EtherMAC
+
+This file provides information on what the device node for the SH EtherMAC
+interface contains.
+
+Required properties:
+- compatible: "renesas,gether-r8a7740" if the device is a part of R8A7740 SoC.
+	      "renesas,ether-r8a7778"  if the device is a part of R8A7778 SoC.
+	      "renesas,ether-r8a7779"  if the device is a part of R8A7779 SoC.
+	      "renesas,ether-r8a7790"  if the device is a part of R8A7790 SoC.
+	      "renesas,ether-r8a7791"  if the device is a part of R8A7791 SoC.
+	      "renesas,ether-r7s72100" if the device is a part of R7S72100 SoC.
+- reg: offset and length of (1) the E-DMAC/feLic register block (required),
+       (2) the TSU register block (optional).
+- interrupts: interrupt specifier for the sole interrupt.
+- phy-mode: see ethernet.txt file in the same directory.
+- phy-handle: see ethernet.txt file in the same directory.
+- #address-cells: number of address cells for the MDIO bus, must be equal to 1.
+- #size-cells: number of size cells on the MDIO bus, must be equal to 0.
+- clocks: clock phandle and specifier pair.
+- pinctrl-0: phandle, referring to a default pin configuration node.
+
+Optional properties:
+- interrupt-parent: the phandle for the interrupt controller that services
+		    interrupts for this device.
+- pinctrl-names: pin configuration state name ("default").
+- renesas,no-ether-link: boolean, specify when a board does not provide a proper
+			 Ether LINK signal.
+- renesas,ether-link-active-low: boolean, specify when the Ether LINK signal is
+				 active-low instead of normal active-high.
+
+Example (Lager board):
+
+	ethernet@ee700000 {
+		compatible = "renesas,ether-r8a7790";
+		reg = <0 0xee700000 0 0x400>;
+		interrupt-parent = <&gic>;
+		interrupts = <0 162 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&mstp8_clks R8A7790_CLK_ETHER>;
+		phy-mode = "rmii";
+		phy-handle = <&phy1>;
+		pinctrl-0 = <&ether_pins>;
+		pinctrl-names = "default";
+		renesas,ether-link-active-low;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		phy1: ethernet-phy@1 {
+			reg = <1>;
+			interrupt-parent = <&irqc0>;
+			interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
+			pinctrl-0 = <&phy1_pins>;
+			pinctrl-names = "default";
+		};
+	};
Index: net-next/drivers/net/ethernet/renesas/sh_eth.c
===================================================================
--- net-next.orig/drivers/net/ethernet/renesas/sh_eth.c
+++ net-next/drivers/net/ethernet/renesas/sh_eth.c
@@ -1,8 +1,8 @@
 /*  SuperH Ethernet device driver
  *
  *  Copyright (C) 2006-2012 Nobuhiro Iwamatsu
- *  Copyright (C) 2008-2013 Renesas Solutions Corp.
- *  Copyright (C) 2013 Cogent Embedded, Inc.
+ *  Copyright (C) 2008-2014 Renesas Solutions Corp.
+ *  Copyright (C) 2013-2014 Cogent Embedded, Inc.
  *
  *  This program is free software; you can redistribute it and/or modify it
  *  under the terms and conditions of the GNU General Public License,
@@ -27,6 +27,10 @@
 #include <linux/platform_device.h>
 #include <linux/mdio-bitbang.h>
 #include <linux/netdevice.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_irq.h>
+#include <linux/of_net.h>
 #include <linux/phy.h>
 #include <linux/cache.h>
 #include <linux/io.h>
@@ -2710,6 +2714,54 @@ static const struct net_device_ops sh_et
 	.ndo_change_mtu		= eth_change_mtu,
 };
 
+#ifdef CONFIG_OF
+static struct sh_eth_plat_data *sh_eth_parse_dt(struct device *dev)
+{
+	struct device_node *np = dev->of_node;
+	struct sh_eth_plat_data *pdata;
+	struct device_node *phy;
+	const char *mac_addr;
+
+	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return NULL;
+
+	pdata->phy_interface = of_get_phy_mode(np);
+
+	phy = of_parse_phandle(np, "phy-handle", 0);
+	if (of_property_read_u32(phy, "reg", &pdata->phy))
+		return NULL;
+	pdata->phy_irq = irq_of_parse_and_map(phy, 0);
+
+	mac_addr = of_get_mac_address(np);
+	if (mac_addr)
+		memcpy(pdata->mac_addr, mac_addr, ETH_ALEN);
+
+	pdata->no_ether_link =
+		of_property_read_bool(np, "renesas,no-ether-link");
+	pdata->ether_link_active_low =
+		of_property_read_bool(np, "renesas,ether-link-active-low");
+
+	return pdata;
+}
+
+static const struct of_device_id sh_eth_match_table[] = {
+	{ .compatible = "renesas,gether-r8a7740", .data = &r8a7740_data },
+	{ .compatible = "renesas,ether-r8a7778", .data = &r8a777x_data },
+	{ .compatible = "renesas,ether-r8a7779", .data = &r8a777x_data },
+	{ .compatible = "renesas,ether-r8a7790", .data = &r8a779x_data },
+	{ .compatible = "renesas,ether-r8a7791", .data = &r8a779x_data },
+	{ .compatible = "renesas,ether-r7s72100", .data = &r7s72100_data },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, sh_eth_match_table);
+#else
+static inline struct sh_eth_plat_data *sh_eth_parse_dt(struct device *dev)
+{
+	return NULL;
+}
+#endif
+
 static int sh_eth_drv_probe(struct platform_device *pdev)
 {
 	int ret, devno = 0;
@@ -2763,6 +2815,8 @@ static int sh_eth_drv_probe(struct platf
 	pm_runtime_enable(&pdev->dev);
 	pm_runtime_resume(&pdev->dev);
 
+	if (pdev->dev.of_node)
+		pd = sh_eth_parse_dt(&pdev->dev);
 	if (!pd) {
 		dev_err(&pdev->dev, "no platform data\n");
 		ret = -EINVAL;
@@ -2778,7 +2832,15 @@ static int sh_eth_drv_probe(struct platf
 	mdp->ether_link_active_low = pd->ether_link_active_low;
 
 	/* set cpu data */
-	mdp->cd = (struct sh_eth_cpu_data *)id->driver_data;
+	if (id) {
+		mdp->cd = (struct sh_eth_cpu_data *)id->driver_data;
+	} else	{
+		const struct of_device_id *match;
+
+		match = of_match_device(of_match_ptr(sh_eth_match_table),
+					&pdev->dev);
+		mdp->cd = (struct sh_eth_cpu_data *)match->data;
+	}
 	mdp->reg_offset = sh_eth_get_register_offset(mdp->cd->register_type);
 	sh_eth_set_default_cpu_data(mdp->cd);
 
@@ -2920,6 +2982,7 @@ static struct platform_driver sh_eth_dri
 	.driver = {
 		   .name = CARDNAME,
 		   .pm = SH_ETH_PM_OPS,
+		   .of_match_table = of_match_ptr(sh_eth_match_table),
 	},
 };
 

^ permalink raw reply

* [PATCH v5] DT: net: document Ethernet bindings in one place
From: Sergei Shtylyov @ 2014-02-17 23:41 UTC (permalink / raw)
  To: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, pawel.moll-5wv7dgnIgG8,
	mark.rutland-5wv7dgnIgG8, ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
	galak-sgV2jX0FEOL9JmXXK+q4OQ, devicetree-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA
  Cc: rob-VoJi6FS/r0vR7s880joybQ, linux-doc-u79uwXL29TY76Z2rM5mHXA,
	jcmvbkbc-Re5JQEeQqe8AvxtiuMwx3w, linux-sh-u79uwXL29TY76Z2rM5mHXA

This patch is an attempt to gather the Ethernet related bindings in one file,
like it's done in the MMC and some other subsystems. It should save some of
the trouble of documenting several properties over and over in each binding
document, instead only making reference to the main file.

I have used the Embedded Power Architecture(TM) Platform Requirements (ePAPR)
standard as a base for the properties description, also documenting some ad-hoc
properties that have been introduced over time despite having direct analogs in
ePAPR.

Signed-off-by: Sergei Shtylyov <sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org>

---
The patch is against DaveM's 'net-next.git' repo.

Changes in version 5:
- added references to common file in the Broadcom GENET binding;
- resolved reject, refreshed the patch.

Changes in version 4:
- documented "phy-mode" property as a de-facto standard;
- added "phy-device" property to the common file;
- removed "[local-]mac-address" properties being mentioned in cases where they
  were optional.

Changes in version 3:
- noted about the "max-frame-size" property's contradictory definition in ePAPR
  1.1, reformatted the description (forgot to save the file last time).

Changes in version 2:
- restored the mentions of the common properties in the individual bindings, but
  made them reference the common file instead;
- edited some property descriptions in the common file, indicating preferred and  not recommended properties;
- moved the "max-frame-size" property definition to the common file;
- resolved rejects, refreshed the patch.

 Documentation/devicetree/bindings/net/allwinner,sun4i-emac.txt    |    6 --
 Documentation/devicetree/bindings/net/arc_emac.txt                |   11 ----
 Documentation/devicetree/bindings/net/broadcom-bcmgenet.txt       |    9 +--
 Documentation/devicetree/bindings/net/cavium-mix.txt              |    7 --
 Documentation/devicetree/bindings/net/cavium-pip.txt              |    7 --
 Documentation/devicetree/bindings/net/cdns-emac.txt               |    6 --
 Documentation/devicetree/bindings/net/cpsw.txt                    |    5 --
 Documentation/devicetree/bindings/net/davicom-dm9000.txt          |    2 
 Documentation/devicetree/bindings/net/davinci_emac.txt            |    3 -
 Documentation/devicetree/bindings/net/ethernet.txt                |   25 ++++++++++
 Documentation/devicetree/bindings/net/fsl-fec.txt                 |    5 --
 Documentation/devicetree/bindings/net/fsl-tsec-phy.txt            |   13 +----
 Documentation/devicetree/bindings/net/lpc-eth.txt                 |    5 --
 Documentation/devicetree/bindings/net/macb.txt                    |    6 --
 Documentation/devicetree/bindings/net/marvell-armada-370-neta.txt |    6 --
 Documentation/devicetree/bindings/net/marvell-orion-net.txt       |    4 -
 Documentation/devicetree/bindings/net/micrel-ks8851.txt           |    3 -
 Documentation/devicetree/bindings/net/smsc-lan91c111.txt          |    3 -
 Documentation/devicetree/bindings/net/smsc911x.txt                |    5 --
 Documentation/devicetree/bindings/net/stmmac.txt                  |    7 --
 20 files changed, 53 insertions(+), 85 deletions(-)

Index: net-next/Documentation/devicetree/bindings/net/allwinner,sun4i-emac.txt
===================================================================
--- net-next.orig/Documentation/devicetree/bindings/net/allwinner,sun4i-emac.txt
+++ net-next/Documentation/devicetree/bindings/net/allwinner,sun4i-emac.txt
@@ -5,13 +5,9 @@ Required properties:
               "allwinner,sun4i-emac")
 - reg: address and length of the register set for the device.
 - interrupts: interrupt for the device
-- phy: A phandle to a phy node defining the PHY address (as the reg
-  property, a single integer).
+- phy: see ethernet.txt file in the same directory.
 - clocks: A phandle to the reference clock for this device
 
-Optional properties:
-- (local-)mac-address: mac address to be used by this driver
-
 Example:
 
 emac: ethernet@01c0b000 {
Index: net-next/Documentation/devicetree/bindings/net/arc_emac.txt
===================================================================
--- net-next.orig/Documentation/devicetree/bindings/net/arc_emac.txt
+++ net-next/Documentation/devicetree/bindings/net/arc_emac.txt
@@ -6,19 +6,12 @@ Required properties:
 - interrupts: Should contain the EMAC interrupts
 - clock-frequency: CPU frequency. It is needed to calculate and set polling
 period of EMAC.
-- max-speed: Maximum supported data-rate in Mbit/s. In some HW configurations
-bandwidth of external memory controller might be a limiting factor. That's why
-it's required to specify which data-rate is supported on current SoC or FPGA.
-For example if only 10 Mbit/s is supported (10BASE-T) set "10". If 100 Mbit/s is
-supported (100BASE-TX) set "100".
-- phy: PHY device attached to the EMAC via MDIO bus
+- max-speed: see ethernet.txt file in the same directory.
+- phy: see ethernet.txt file in the same directory.
 
 Child nodes of the driver are the individual PHY devices connected to the
 MDIO bus. They must have a "reg" property given the PHY address on the MDIO bus.
 
-Optional properties:
-- mac-address: 6 bytes, mac address
-
 Examples:
 
 	ethernet@c0fc2000 {
Index: net-next/Documentation/devicetree/bindings/net/broadcom-bcmgenet.txt
===================================================================
--- net-next.orig/Documentation/devicetree/bindings/net/broadcom-bcmgenet.txt
+++ net-next/Documentation/devicetree/bindings/net/broadcom-bcmgenet.txt
@@ -7,9 +7,7 @@ Required properties:
 - interrupts: must be two cells, the first cell is the general purpose
   interrupt line, while the second cell is the interrupt for the ring
   RX and TX queues operating in ring mode
-- phy-mode: String, operation mode of the PHY interface. Supported values are
-  "mii", "rgmii", "rgmii-txid", "rev-mii", "moca". Analogous to ePAPR
-  "phy-connection-type" values
+- phy-mode: see ethernet.txt file in the same directory
 - address-cells: should be 1
 - size-cells: should be 1
 
@@ -20,9 +18,8 @@ Optional properties:
 - clock-names: When provided, names of the functional clock phandles, first
   name should be "enet" and second should be "enet-wol".
 
-- phy-handle: A phandle to a phy node defining the PHY address (as the reg
-  property, a single integer), used to describe configurations where a PHY
-  (internal or external) is used.
+- phy-handle: See ethernet.txt file in the same directory; used to describe
+  configurations where a PHY (internal or external) is used.
 
 - fixed-link: When the GENET interface is connected to a MoCA hardware block or
   when operating in a RGMII to RGMII type of connection, or when the MDIO bus is
Index: net-next/Documentation/devicetree/bindings/net/cavium-mix.txt
===================================================================
--- net-next.orig/Documentation/devicetree/bindings/net/cavium-mix.txt
+++ net-next/Documentation/devicetree/bindings/net/cavium-mix.txt
@@ -18,12 +18,7 @@ Properties:
 - interrupts: Two interrupt specifiers.  The first is the MIX
   interrupt routing and the second the routing for the AGL interrupts.
 
-- mac-address: Optional, the MAC address to assign to the device.
-
-- local-mac-address: Optional, the MAC address to assign to the device
-  if mac-address is not specified.
-
-- phy-handle: Optional, a phandle for the PHY device connected to this device.
+- phy-handle: Optional, see ethernet.txt file in the same directory.
 
 Example:
 	ethernet@1070000100800 {
Index: net-next/Documentation/devicetree/bindings/net/cavium-pip.txt
===================================================================
--- net-next.orig/Documentation/devicetree/bindings/net/cavium-pip.txt
+++ net-next/Documentation/devicetree/bindings/net/cavium-pip.txt
@@ -35,12 +35,7 @@ Properties for PIP port which is a child
 
 - reg: The port number within the interface group.
 
-- mac-address: Optional, the MAC address to assign to the device.
-
-- local-mac-address: Optional, the MAC address to assign to the device
-  if mac-address is not specified.
-
-- phy-handle: Optional, a phandle for the PHY device connected to this device.
+- phy-handle: Optional, see ethernet.txt file in the same directory.
 
 Example:
 
Index: net-next/Documentation/devicetree/bindings/net/cdns-emac.txt
===================================================================
--- net-next.orig/Documentation/devicetree/bindings/net/cdns-emac.txt
+++ net-next/Documentation/devicetree/bindings/net/cdns-emac.txt
@@ -6,11 +6,7 @@ Required properties:
   or the generic form: "cdns,emac".
 - reg: Address and length of the register set for the device
 - interrupts: Should contain macb interrupt
-- phy-mode: String, operation mode of the PHY interface.
-  Supported values are: "mii", "rmii".
-
-Optional properties:
-- local-mac-address: 6 bytes, mac address
+- phy-mode: see ethernet.txt file in the same directory.
 
 Examples:
 
Index: net-next/Documentation/devicetree/bindings/net/cpsw.txt
===================================================================
--- net-next.orig/Documentation/devicetree/bindings/net/cpsw.txt
+++ net-next/Documentation/devicetree/bindings/net/cpsw.txt
@@ -28,9 +28,8 @@ Optional properties:
 Slave Properties:
 Required properties:
 - phy_id		: Specifies slave phy id
-- phy-mode		: The interface between the SoC and the PHY (a string
-			  that of_get_phy_mode() can understand)
-- mac-address		: Specifies slave MAC address
+- phy-mode		: See ethernet.txt file in the same directory
+- mac-address		: See ethernet.txt file in the same directory
 
 Optional properties:
 - dual_emac_res_vlan	: Specifies VID to be used to segregate the ports
Index: net-next/Documentation/devicetree/bindings/net/davicom-dm9000.txt
===================================================================
--- net-next.orig/Documentation/devicetree/bindings/net/davicom-dm9000.txt
+++ net-next/Documentation/devicetree/bindings/net/davicom-dm9000.txt
@@ -9,8 +9,6 @@ Required properties:
 - interrupts : interrupt specifier specific to interrupt controller
 
 Optional properties:
-- local-mac-address : A bytestring of 6 bytes specifying Ethernet MAC address
-    to use (from firmware or bootloader)
 - davicom,no-eeprom : Configuration EEPROM is not available
 - davicom,ext-phy : Use external PHY
 
Index: net-next/Documentation/devicetree/bindings/net/davinci_emac.txt
===================================================================
--- net-next.orig/Documentation/devicetree/bindings/net/davinci_emac.txt
+++ net-next/Documentation/devicetree/bindings/net/davinci_emac.txt
@@ -17,9 +17,8 @@ Required properties:
 			  Miscellaneous Interrupt>
 
 Optional properties:
-- phy-handle: Contains a phandle to an Ethernet PHY.
+- phy-handle: See ethernet.txt file in the same directory.
               If absent, davinci_emac driver defaults to 100/FULL.
-- local-mac-address : 6 bytes, mac address
 - ti,davinci-rmii-en: 1 byte, 1 means use RMII
 - ti,davinci-no-bd-ram: boolean, does EMAC have BD RAM?
 
Index: net-next/Documentation/devicetree/bindings/net/ethernet.txt
===================================================================
--- /dev/null
+++ net-next/Documentation/devicetree/bindings/net/ethernet.txt
@@ -0,0 +1,25 @@
+The following properties are common to the Ethernet controllers:
+
+- local-mac-address: array of 6 bytes, specifies the MAC address that was
+  assigned to the network device;
+- mac-address: array of 6 bytes, specifies the MAC address that was last used by
+  the boot program; should be used in cases where the MAC address assigned to
+  the device by the boot program is different from the "local-mac-address"
+  property;
+- max-speed: number, specifies maximum speed in Mbit/s supported by the device;
+- max-frame-size: number, maximum transfer unit (IEEE defined MTU), rather than
+  the maximum frame size (there's contradiction in ePAPR).
+- phy-mode: string, operation mode of the PHY interface; supported values are
+  "mii", "gmii", "sgmii", "tbi", "rev-mii", "rmii", "rgmii", "rgmii-id",
+  "rgmii-rxid", "rgmii-txid", "rtbi", "smii", "xgmii"; this is now a de-facto
+  standard property;
+- phy-connection-type: the same as "phy-mode" property but described in ePAPR;
+- phy-handle: phandle, specifies a reference to a node representing a PHY
+  device; this property is described in ePAPR and so preferred;
+- phy: the same as "phy-handle" property, not recommended for new bindings.
+- phy-device: the same as "phy-handle" property, not recommended for new
+  bindings.
+
+Child nodes of the Ethernet controller are typically the individual PHY devices
+connected via the MDIO bus (sometimes the MDIO bus controller is separate).
+They are described in the phy.txt file in this same directory.
Index: net-next/Documentation/devicetree/bindings/net/fsl-fec.txt
===================================================================
--- net-next.orig/Documentation/devicetree/bindings/net/fsl-fec.txt
+++ net-next/Documentation/devicetree/bindings/net/fsl-fec.txt
@@ -4,12 +4,9 @@ Required properties:
 - compatible : Should be "fsl,<soc>-fec"
 - reg : Address and length of the register set for the device
 - interrupts : Should contain fec interrupt
-- phy-mode : String, operation mode of the PHY interface.
-  Supported values are: "mii", "gmii", "sgmii", "tbi", "rmii",
-  "rgmii", "rgmii-id", "rgmii-rxid", "rgmii-txid", "rtbi", "smii".
+- phy-mode : See ethernet.txt file in the same directory
 
 Optional properties:
-- local-mac-address : 6 bytes, mac address
 - phy-reset-gpios : Should specify the gpio for phy reset
 - phy-reset-duration : Reset duration in milliseconds.  Should present
   only if property "phy-reset-gpios" is available.  Missing the property
Index: net-next/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt
===================================================================
--- net-next.orig/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt
+++ net-next/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt
@@ -38,22 +38,17 @@ Properties:
   - model : Model of the device.  Can be "TSEC", "eTSEC", or "FEC"
   - compatible : Should be "gianfar"
   - reg : Offset and length of the register set for the device
-  - local-mac-address : List of bytes representing the ethernet address of
-    this controller
   - interrupts : For FEC devices, the first interrupt is the device's
     interrupt.  For TSEC and eTSEC devices, the first interrupt is
     transmit, the second is receive, and the third is error.
-  - phy-handle : The phandle for the PHY connected to this ethernet
-    controller.
+  - phy-handle : See ethernet.txt file in the same directory.
   - fixed-link : <a b c d e> where a is emulated phy id - choose any,
     but unique to the all specified fixed-links, b is duplex - 0 half,
     1 full, c is link speed - d#10/d#100/d#1000, d is pause - 0 no
     pause, 1 pause, e is asym_pause - 0 no asym_pause, 1 asym_pause.
-  - phy-connection-type : a string naming the controller/PHY interface type,
-    i.e., "mii" (default), "rmii", "gmii", "rgmii", "rgmii-id", "sgmii",
-    "tbi", or "rtbi".  This property is only really needed if the connection
-    is of type "rgmii-id", as all other connection types are detected by
-    hardware.
+  - phy-connection-type : See ethernet.txt file in the same directory.
+    This property is only really needed if the connection is of type
+    "rgmii-id", as all other connection types are detected by hardware.
   - fsl,magic-packet : If present, indicates that the hardware supports
     waking up via magic packet.
   - bd-stash : If present, indicates that the hardware supports stashing
Index: net-next/Documentation/devicetree/bindings/net/lpc-eth.txt
===================================================================
--- net-next.orig/Documentation/devicetree/bindings/net/lpc-eth.txt
+++ net-next/Documentation/devicetree/bindings/net/lpc-eth.txt
@@ -6,10 +6,9 @@ Required properties:
 - interrupts: Should contain ethernet controller interrupt
 
 Optional properties:
-- phy-mode: String, operation mode of the PHY interface.
-  Supported values are: "mii", "rmii" (default)
+- phy-mode: See ethernet.txt file in the same directory. If the property is
+  absent, "rmii" is assumed.
 - use-iram: Use LPC32xx internal SRAM (IRAM) for DMA buffering
-- local-mac-address : 6 bytes, mac address
 
 Example:
 
Index: net-next/Documentation/devicetree/bindings/net/macb.txt
===================================================================
--- net-next.orig/Documentation/devicetree/bindings/net/macb.txt
+++ net-next/Documentation/devicetree/bindings/net/macb.txt
@@ -8,16 +8,12 @@ Required properties:
   the Cadence GEM, or the generic form: "cdns,gem".
 - reg: Address and length of the register set for the device
 - interrupts: Should contain macb interrupt
-- phy-mode: String, operation mode of the PHY interface.
-  Supported values are: "mii", "rmii", "gmii", "rgmii".
+- phy-mode: See ethernet.txt file in the same directory.
 - clock-names: Tuple listing input clock names.
 	Required elements: 'pclk', 'hclk'
 	Optional elements: 'tx_clk'
 - clocks: Phandles to input clocks.
 
-Optional properties:
-- local-mac-address: 6 bytes, mac address
-
 Examples:
 
 	macb0: ethernet@fffc4000 {
Index: net-next/Documentation/devicetree/bindings/net/marvell-armada-370-neta.txt
===================================================================
--- net-next.orig/Documentation/devicetree/bindings/net/marvell-armada-370-neta.txt
+++ net-next/Documentation/devicetree/bindings/net/marvell-armada-370-neta.txt
@@ -4,10 +4,8 @@ Required properties:
 - compatible: should be "marvell,armada-370-neta".
 - reg: address and length of the register set for the device.
 - interrupts: interrupt for the device
-- phy: A phandle to a phy node defining the PHY address (as the reg
-  property, a single integer).
-- phy-mode: The interface between the SoC and the PHY (a string that
-  of_get_phy_mode() can understand)
+- phy: See ethernet.txt file in the same directory.
+- phy-mode: See ethernet.txt file in the same directory
 - clocks: a pointer to the reference clock for this device.
 
 Example:
Index: net-next/Documentation/devicetree/bindings/net/marvell-orion-net.txt
===================================================================
--- net-next.orig/Documentation/devicetree/bindings/net/marvell-orion-net.txt
+++ net-next/Documentation/devicetree/bindings/net/marvell-orion-net.txt
@@ -36,7 +36,7 @@ Required port properties:
       "marvell,kirkwood-eth-port".
  - reg: port number relative to ethernet controller, shall be 0, 1, or 2.
  - interrupts: port interrupt.
- - local-mac-address: 6 bytes MAC address.
+ - local-mac-address: See ethernet.txt file in the same directory.
 
 Optional port properties:
  - marvell,tx-queue-size: size of the transmit ring buffer.
@@ -48,7 +48,7 @@ Optional port properties:
 
 and
 
- - phy-handle: phandle reference to ethernet PHY.
+ - phy-handle: See ethernet.txt file in the same directory.
 
 or
 
Index: net-next/Documentation/devicetree/bindings/net/micrel-ks8851.txt
===================================================================
--- net-next.orig/Documentation/devicetree/bindings/net/micrel-ks8851.txt
+++ net-next/Documentation/devicetree/bindings/net/micrel-ks8851.txt
@@ -4,6 +4,3 @@ Required properties:
 - compatible = "micrel,ks8851-ml" of parallel interface
 - reg : 2 physical address and size of registers for data and command
 - interrupts : interrupt connection
-
-Optional properties:
-- local-mac-address : Ethernet mac address to use
Index: net-next/Documentation/devicetree/bindings/net/smsc-lan91c111.txt
===================================================================
--- net-next.orig/Documentation/devicetree/bindings/net/smsc-lan91c111.txt
+++ net-next/Documentation/devicetree/bindings/net/smsc-lan91c111.txt
@@ -6,8 +6,7 @@ Required properties:
 - interrupts : interrupt connection
 
 Optional properties:
-- phy-device : phandle to Ethernet phy
-- local-mac-address : Ethernet mac address to use
+- phy-device : see ethernet.txt file in the same directory
 - reg-io-width : Mask of sizes (in bytes) of the IO accesses that
   are supported on the device.  Valid value for SMSC LAN91c111 are
   1, 2 or 4.  If it's omitted or invalid, the size would be 2 meaning
Index: net-next/Documentation/devicetree/bindings/net/smsc911x.txt
===================================================================
--- net-next.orig/Documentation/devicetree/bindings/net/smsc911x.txt
+++ net-next/Documentation/devicetree/bindings/net/smsc911x.txt
@@ -6,9 +6,7 @@ Required properties:
 - interrupts : Should contain SMSC LAN interrupt line
 - interrupt-parent : Should be the phandle for the interrupt controller
   that services interrupts for this device
-- phy-mode : String, operation mode of the PHY interface.
-  Supported values are: "mii", "gmii", "sgmii", "tbi", "rmii",
-  "rgmii", "rgmii-id", "rgmii-rxid", "rgmii-txid", "rtbi", "smii".
+- phy-mode : See ethernet.txt file in the same directory
 
 Optional properties:
 - reg-shift : Specify the quantity to shift the register offsets by
@@ -23,7 +21,6 @@ Optional properties:
   external PHY
 - smsc,save-mac-address : Indicates that mac address needs to be saved
   before resetting the controller
-- local-mac-address : 6 bytes, mac address
 
 Examples:
 
Index: net-next/Documentation/devicetree/bindings/net/stmmac.txt
===================================================================
--- net-next.orig/Documentation/devicetree/bindings/net/stmmac.txt
+++ net-next/Documentation/devicetree/bindings/net/stmmac.txt
@@ -10,8 +10,7 @@ Required properties:
 - interrupt-names: Should contain the interrupt names "macirq"
   "eth_wake_irq" if this interrupt is supported in the "interrupts"
   property
-- phy-mode: String, operation mode of the PHY interface.
-  Supported values are: "mii", "rmii", "gmii", "rgmii".
+- phy-mode: See ethernet.txt file in the same directory.
 - snps,reset-gpio 	gpio number for phy reset.
 - snps,reset-active-low boolean flag to indicate if phy reset is active low.
 - snps,reset-delays-us  is triplet of delays
@@ -28,12 +27,10 @@ Required properties:
 				ignored if force_thresh_dma_mode is set.
 
 Optional properties:
-- mac-address: 6 bytes, mac address
 - resets: Should contain a phandle to the STMMAC reset signal, if any
 - reset-names: Should contain the reset signal name "stmmaceth", if a
 	reset phandle is given
-- max-frame-size:	Maximum Transfer Unit (IEEE defined MTU), rather
-			than the maximum frame size.
+- max-frame-size: See ethernet.txt file in the same directory
 
 Examples:
 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCHv2] irtty-sir.c: Do not set_termios() on irtty_close()
From: One Thousand Gnomes @ 2014-02-17 23:13 UTC (permalink / raw)
  To: Tommie Gannert; +Cc: Samuel Ortiz, netdev, linux-kernel
In-Reply-To: <5302750C.6080407@gannert.se>

> v2: Resend due to mail format issues. More Thunderbird settings applied.
> 
> I know very little of this code, and I'm not sure this is a good solution,
> so here's some background:

I wouldn't worry too much. I'm amazed the code even works these days.
It's incredibly bitrotted and doesn't use the tty layer properly (mind
you it never did)


> The effect of this is that /dev/ttyUSB* is still in use, and thus leaking
> at least dev nodes. This is a minimal patch that solves that oops.
> --- linux-3.12/drivers/net/irda/irtty-sir.c.orig	2014-02-12 21:36:46.132496089 +0000
> +++ linux-3.12/drivers/net/irda/irtty-sir.c	2014-02-12 21:57:21.635843884 +0000
> @@ -521,7 +521,6 @@ static void irtty_close(struct tty_struc
>  	sirdev_put_instance(priv->dev);
>  
>  	/* Stop tty */
> -	irtty_stop_receiver(tty, TRUE);
>  	clear_bit(TTY_DO_WRITE_WAKEUP, &tty->flags);
>  	if (tty->ops->stop)
>  		tty->ops->stop(tty);

This change looks correct to me. The three lines below your change are
amusing all incorrect and should probably also all be deleted.

Alan

^ permalink raw reply

* [PATCH] Drivers: net: ethernet: 3com: 3c589_cs fixed coding style issues
From: Justin van Wijngaarden @ 2014-02-17 22:58 UTC (permalink / raw)
  To: paul.gortmaker
  Cc: gregkh, hsweeten, davem, justinvanwijngaarden, netdev,
	linux-kernel

checkpatch.pl clean-up, from 14 error/ 277 warnings, to 0 errors, 7 warnings

Signed-off-by: Justin van Wijngaarden <justinvanwijngaarden@gmail.com>
---
 drivers/net/ethernet/3com/3c589_cs.c | 1127 ++++++++++++++++++----------------
 1 file changed, 584 insertions(+), 543 deletions(-)

diff --git a/drivers/net/ethernet/3com/3c589_cs.c b/drivers/net/ethernet/3com/3c589_cs.c
index 5992860..063557e 100644
--- a/drivers/net/ethernet/3com/3c589_cs.c
+++ b/drivers/net/ethernet/3com/3c589_cs.c
@@ -1,23 +1,24 @@
-/*======================================================================
-
-    A PCMCIA ethernet driver for the 3com 3c589 card.
-
-    Copyright (C) 1999 David A. Hinds -- dahinds@users.sourceforge.net
-
-    3c589_cs.c 1.162 2001/10/13 00:08:50
-
-    The network driver code is based on Donald Becker's 3c589 code:
-
-    Written 1994 by Donald Becker.
-    Copyright 1993 United States Government as represented by the
-    Director, National Security Agency.  This software may be used and
-    distributed according to the terms of the GNU General Public License,
-    incorporated herein by reference.
-    Donald Becker may be reached at becker@scyld.com
-
-    Updated for 2.5.x by Alan Cox <alan@lxorguk.ukuu.org.uk>
-
-======================================================================*/
+/* ======================================================================
+ *
+ * A PCMCIA ethernet driver for the 3com 3c589 card.
+ *
+ * Copyright (C) 1999 David A. Hinds -- dahinds@users.sourceforge.net
+ *
+ * 3c589_cs.c 1.162 2001/10/13 00:08:50
+ *
+ * The network driver code is based on Donald Becker's 3c589 code:
+ *
+ * Written 1994 by Donald Becker.
+ * Copyright 1993 United States Government as represented by the
+ * Director, National Security Agency.  This software may be used and
+ * distributed according to the terms of the GNU General Public License,
+ * incorporated herein by reference.
+ * Donald Becker may be reached at becker@scyld.com
+ *
+ * Updated for 2.5.x by Alan Cox <alan@lxorguk.ukuu.org.uk>
+ *
+ * ======================================================================
+ */
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
@@ -41,18 +42,20 @@
 #include <linux/ioport.h>
 #include <linux/bitops.h>
 #include <linux/jiffies.h>
+#include <linux/uaccess.h>
+#include <linux/io.h>
 
 #include <pcmcia/cistpl.h>
 #include <pcmcia/cisreg.h>
 #include <pcmcia/ciscode.h>
 #include <pcmcia/ds.h>
 
-#include <asm/uaccess.h>
-#include <asm/io.h>
 
 /* To minimize the size of the driver source I only define operating
-   constants if they are used several times.  You'll need the manual
-   if you want to understand driver details. */
+ * constants if they are used several times. You'll need the manual
+ * if you want to understand driver details.
+ */
+
 /* Offsets from base I/O address. */
 #define EL3_DATA	0x00
 #define EL3_TIMER	0x0a
@@ -65,7 +68,9 @@
 #define EL3WINDOW(win_num) outw(SelectWindow + (win_num), ioaddr + EL3_CMD)
 
 /* The top five bits written to EL3_CMD are a command, the lower
-   11 bits are the parameter, if applicable. */
+ * 11 bits are the parameter, if applicable.
+ */
+
 enum c509cmd {
 	TotalReset	= 0<<11,
 	SelectWindow	= 1<<11,
@@ -190,138 +195,142 @@ static const struct net_device_ops el3_netdev_ops = {
 
 static int tc589_probe(struct pcmcia_device *link)
 {
-    struct el3_private *lp;
-    struct net_device *dev;
+	struct el3_private *lp;
+	struct net_device *dev;
 
-    dev_dbg(&link->dev, "3c589_attach()\n");
+	dev_dbg(&link->dev, "3c589_attach()\n");
 
-    /* Create new ethernet device */
-    dev = alloc_etherdev(sizeof(struct el3_private));
-    if (!dev)
-	 return -ENOMEM;
-    lp = netdev_priv(dev);
-    link->priv = dev;
-    lp->p_dev = link;
+	/* Create new ethernet device */
+	dev = alloc_etherdev(sizeof(struct el3_private));
+	if (!dev)
+		return -ENOMEM;
+	lp = netdev_priv(dev);
+	link->priv = dev;
+	lp->p_dev = link;
 
-    spin_lock_init(&lp->lock);
-    link->resource[0]->end = 16;
-    link->resource[0]->flags |= IO_DATA_PATH_WIDTH_16;
+	spin_lock_init(&lp->lock);
+	link->resource[0]->end = 16;
+	link->resource[0]->flags |= IO_DATA_PATH_WIDTH_16;
 
-    link->config_flags |= CONF_ENABLE_IRQ;
-    link->config_index = 1;
+	link->config_flags |= CONF_ENABLE_IRQ;
+	link->config_index = 1;
 
-    dev->netdev_ops = &el3_netdev_ops;
-    dev->watchdog_timeo = TX_TIMEOUT;
+	dev->netdev_ops = &el3_netdev_ops;
+	dev->watchdog_timeo = TX_TIMEOUT;
 
-    SET_ETHTOOL_OPS(dev, &netdev_ethtool_ops);
+	SET_ETHTOOL_OPS(dev, &netdev_ethtool_ops);
 
-    return tc589_config(link);
+	return tc589_config(link);
 }
 
 static void tc589_detach(struct pcmcia_device *link)
 {
-    struct net_device *dev = link->priv;
+	struct net_device *dev = link->priv;
 
-    dev_dbg(&link->dev, "3c589_detach\n");
+	dev_dbg(&link->dev, "3c589_detach\n");
 
-    unregister_netdev(dev);
+	unregister_netdev(dev);
 
-    tc589_release(link);
+	tc589_release(link);
 
-    free_netdev(dev);
+	free_netdev(dev);
 } /* tc589_detach */
 
 static int tc589_config(struct pcmcia_device *link)
 {
-    struct net_device *dev = link->priv;
-    __be16 *phys_addr;
-    int ret, i, j, multi = 0, fifo;
-    unsigned int ioaddr;
-    static const char * const ram_split[] = {"5:3", "3:1", "1:1", "3:5"};
-    u8 *buf;
-    size_t len;
-
-    dev_dbg(&link->dev, "3c589_config\n");
-
-    phys_addr = (__be16 *)dev->dev_addr;
-    /* Is this a 3c562? */
-    if (link->manf_id != MANFID_3COM)
-	    dev_info(&link->dev, "hmmm, is this really a 3Com card??\n");
-    multi = (link->card_id == PRODID_3COM_3C562);
-
-    link->io_lines = 16;
-
-    /* For the 3c562, the base address must be xx00-xx7f */
-    for (i = j = 0; j < 0x400; j += 0x10) {
-	if (multi && (j & 0x80)) continue;
-	link->resource[0]->start = j ^ 0x300;
-	i = pcmcia_request_io(link);
-	if (i == 0)
-		break;
-    }
-    if (i != 0)
-	goto failed;
-
-    ret = pcmcia_request_irq(link, el3_interrupt);
-    if (ret)
-	    goto failed;
-
-    ret = pcmcia_enable_device(link);
-    if (ret)
-	    goto failed;
-
-    dev->irq = link->irq;
-    dev->base_addr = link->resource[0]->start;
-    ioaddr = dev->base_addr;
-    EL3WINDOW(0);
-
-    /* The 3c589 has an extra EEPROM for configuration info, including
-       the hardware address.  The 3c562 puts the address in the CIS. */
-    len = pcmcia_get_tuple(link, 0x88, &buf);
-    if (buf && len >= 6) {
-	    for (i = 0; i < 3; i++)
-		    phys_addr[i] = htons(le16_to_cpu(buf[i*2]));
-	    kfree(buf);
-    } else {
-	kfree(buf); /* 0 < len < 6 */
-	for (i = 0; i < 3; i++)
-	    phys_addr[i] = htons(read_eeprom(ioaddr, i));
-	if (phys_addr[0] == htons(0x6060)) {
-	    dev_err(&link->dev, "IO port conflict at 0x%03lx-0x%03lx\n",
-		    dev->base_addr, dev->base_addr+15);
-	    goto failed;
+	struct net_device *dev = link->priv;
+	__be16 *phys_addr;
+	int ret, i, j, multi = 0, fifo;
+	unsigned int ioaddr;
+	static const char * const ram_split[] = {"5:3", "3:1", "1:1", "3:5"};
+	u8 *buf;
+	size_t len;
+
+	dev_dbg(&link->dev, "3c589_config\n");
+
+	phys_addr = (__be16 *)dev->dev_addr;
+	/* Is this a 3c562? */
+	if (link->manf_id != MANFID_3COM)
+		dev_info(&link->dev, "hmmm, is this really a 3Com card??\n");
+	multi = (link->card_id == PRODID_3COM_3C562);
+
+	link->io_lines = 16;
+
+	/* For the 3c562, the base address must be xx00-xx7f */
+	for (i = j = 0; j < 0x400; j += 0x10) {
+		if (multi && (j & 0x80))
+			continue;
+		link->resource[0]->start = j ^ 0x300;
+		i = pcmcia_request_io(link);
+		if (i == 0)
+			break;
 	}
-    }
-
-    /* The address and resource configuration register aren't loaded from
-       the EEPROM and *must* be set to 0 and IRQ3 for the PCMCIA version. */
-    outw(0x3f00, ioaddr + 8);
-    fifo = inl(ioaddr);
-
-    /* The if_port symbol can be set when the module is loaded */
-    if ((if_port >= 0) && (if_port <= 3))
-	dev->if_port = if_port;
-    else
-	dev_err(&link->dev, "invalid if_port requested\n");
-
-    SET_NETDEV_DEV(dev, &link->dev);
-
-    if (register_netdev(dev) != 0) {
-	    dev_err(&link->dev, "register_netdev() failed\n");
-	goto failed;
-    }
-
-    netdev_info(dev, "3Com 3c%s, io %#3lx, irq %d, hw_addr %pM\n",
-		(multi ? "562" : "589"), dev->base_addr, dev->irq,
-		dev->dev_addr);
-    netdev_info(dev, "  %dK FIFO split %s Rx:Tx, %s xcvr\n",
-		(fifo & 7) ? 32 : 8, ram_split[(fifo >> 16) & 3],
-		if_names[dev->if_port]);
-    return 0;
+	if (i != 0)
+		goto failed;
+
+	ret = pcmcia_request_irq(link, el3_interrupt);
+	if (ret)
+		goto failed;
+
+	ret = pcmcia_enable_device(link);
+	if (ret)
+		goto failed;
+
+	dev->irq = link->irq;
+	dev->base_addr = link->resource[0]->start;
+	ioaddr = dev->base_addr;
+	EL3WINDOW(0);
+
+	/* The 3c589 has an extra EEPROM for configuration info, including
+	 * the hardware address.  The 3c562 puts the address in the CIS.
+	 */
+	len = pcmcia_get_tuple(link, 0x88, &buf);
+	if (buf && len >= 6) {
+		for (i = 0; i < 3; i++)
+			phys_addr[i] = htons(le16_to_cpu(buf[i*2]));
+		kfree(buf);
+	} else {
+		kfree(buf); /* 0 < len < 6 */
+		for (i = 0; i < 3; i++)
+			phys_addr[i] = htons(read_eeprom(ioaddr, i));
+		if (phys_addr[0] == htons(0x6060)) {
+			dev_err(&link->dev, "IO port conflict at 0x%03lx-0x%03lx\n",
+					dev->base_addr, dev->base_addr+15);
+			goto failed;
+		}
+	}
+
+	/* The address and resource configuration register aren't loaded from
+	 * the EEPROM and *must* be set to 0 and IRQ3 for the PCMCIA version.
+	 */
+
+	outw(0x3f00, ioaddr + 8);
+	fifo = inl(ioaddr);
+
+	/* The if_port symbol can be set when the module is loaded */
+	if ((if_port >= 0) && (if_port <= 3))
+		dev->if_port = if_port;
+	else
+		dev_err(&link->dev, "invalid if_port requested\n");
+
+	SET_NETDEV_DEV(dev, &link->dev);
+
+	if (register_netdev(dev) != 0) {
+		dev_err(&link->dev, "register_netdev() failed\n");
+		goto failed;
+	}
+
+	netdev_info(dev, "3Com 3c%s, io %#3lx, irq %d, hw_addr %pM\n",
+			(multi ? "562" : "589"), dev->base_addr, dev->irq,
+			dev->dev_addr);
+	netdev_info(dev, "  %dK FIFO split %s Rx:Tx, %s xcvr\n",
+			(fifo & 7) ? 32 : 8, ram_split[(fifo >> 16) & 3],
+			if_names[dev->if_port]);
+	return 0;
 
 failed:
-    tc589_release(link);
-    return -ENODEV;
+	tc589_release(link);
+	return -ENODEV;
 } /* tc589_config */
 
 static void tc589_release(struct pcmcia_device *link)
@@ -353,113 +362,120 @@ static int tc589_resume(struct pcmcia_device *link)
 
 /*====================================================================*/
 
-/*
-  Use this for commands that may take time to finish
-*/
+/* Use this for commands that may take time to finish */
+
 static void tc589_wait_for_completion(struct net_device *dev, int cmd)
 {
-    int i = 100;
-    outw(cmd, dev->base_addr + EL3_CMD);
-    while (--i > 0)
-	if (!(inw(dev->base_addr + EL3_STATUS) & 0x1000)) break;
-    if (i == 0)
-	netdev_warn(dev, "command 0x%04x did not complete!\n", cmd);
+	int i = 100;
+	outw(cmd, dev->base_addr + EL3_CMD);
+	while (--i > 0)
+		if (!(inw(dev->base_addr + EL3_STATUS) & 0x1000))
+			break;
+	if (i == 0)
+		netdev_warn(dev, "command 0x%04x did not complete!\n", cmd);
 }
 
-/*
-  Read a word from the EEPROM using the regular EEPROM access register.
-  Assume that we are in register window zero.
-*/
+/* Read a word from the EEPROM using the regular EEPROM access register.
+ * Assume that we are in register window zero.
+ */
+
 static u16 read_eeprom(unsigned int ioaddr, int index)
 {
-    int i;
-    outw(EEPROM_READ + index, ioaddr + 10);
-    /* Reading the eeprom takes 162 us */
-    for (i = 1620; i >= 0; i--)
-	if ((inw(ioaddr + 10) & EEPROM_BUSY) == 0)
-	    break;
-    return inw(ioaddr + 12);
+	int i;
+	outw(EEPROM_READ + index, ioaddr + 10);
+	/* Reading the eeprom takes 162 us */
+	for (i = 1620; i >= 0; i--)
+		if ((inw(ioaddr + 10) & EEPROM_BUSY) == 0)
+			break;
+	return inw(ioaddr + 12);
 }
 
-/*
-  Set transceiver type, perhaps to something other than what the user
-  specified in dev->if_port.
-*/
+/* Set transceiver type, perhaps to something other than what the user
+ * specified in dev->if_port.
+ */
+
 static void tc589_set_xcvr(struct net_device *dev, int if_port)
 {
-    struct el3_private *lp = netdev_priv(dev);
-    unsigned int ioaddr = dev->base_addr;
-
-    EL3WINDOW(0);
-    switch (if_port) {
-    case 0: case 1: outw(0, ioaddr + 6); break;
-    case 2: outw(3<<14, ioaddr + 6); break;
-    case 3: outw(1<<14, ioaddr + 6); break;
-    }
-    /* On PCMCIA, this just turns on the LED */
-    outw((if_port == 2) ? StartCoax : StopCoax, ioaddr + EL3_CMD);
-    /* 10baseT interface, enable link beat and jabber check. */
-    EL3WINDOW(4);
-    outw(MEDIA_LED | ((if_port < 2) ? MEDIA_TP : 0), ioaddr + WN4_MEDIA);
-    EL3WINDOW(1);
-    if (if_port == 2)
-	lp->media_status = ((dev->if_port == 0) ? 0x8000 : 0x4000);
-    else
-	lp->media_status = ((dev->if_port == 0) ? 0x4010 : 0x8800);
+	struct el3_private *lp = netdev_priv(dev);
+	unsigned int ioaddr = dev->base_addr;
+
+	EL3WINDOW(0);
+	switch (if_port) {
+	case 0:
+	case 1:
+		outw(0, ioaddr + 6);
+		break;
+	case 2:
+		outw(3<<14, ioaddr + 6);
+		break;
+	case 3:
+		outw(1<<14, ioaddr + 6);
+		break;
+	}
+	/* On PCMCIA, this just turns on the LED */
+	outw((if_port == 2) ? StartCoax : StopCoax, ioaddr + EL3_CMD);
+	/* 10baseT interface, enable link beat and jabber check. */
+	EL3WINDOW(4);
+	outw(MEDIA_LED | ((if_port < 2) ? MEDIA_TP : 0), ioaddr + WN4_MEDIA);
+	EL3WINDOW(1);
+	if (if_port == 2)
+		lp->media_status = ((dev->if_port == 0) ? 0x8000 : 0x4000);
+	else
+		lp->media_status = ((dev->if_port == 0) ? 0x4010 : 0x8800);
 }
 
 static void dump_status(struct net_device *dev)
 {
-    unsigned int ioaddr = dev->base_addr;
-    EL3WINDOW(1);
-    netdev_info(dev, "  irq status %04x, rx status %04x, tx status %02x  tx free %04x\n",
-		inw(ioaddr+EL3_STATUS), inw(ioaddr+RX_STATUS),
-		inb(ioaddr+TX_STATUS), inw(ioaddr+TX_FREE));
-    EL3WINDOW(4);
-    netdev_info(dev, "  diagnostics: fifo %04x net %04x ethernet %04x media %04x\n",
-		inw(ioaddr+0x04), inw(ioaddr+0x06), inw(ioaddr+0x08),
-		inw(ioaddr+0x0a));
-    EL3WINDOW(1);
+	unsigned int ioaddr = dev->base_addr;
+	EL3WINDOW(1);
+	netdev_info(dev, "  irq status %04x, rx status %04x, tx status %02x  tx free %04x\n",
+			inw(ioaddr+EL3_STATUS), inw(ioaddr+RX_STATUS),
+			inb(ioaddr+TX_STATUS), inw(ioaddr+TX_FREE));
+	EL3WINDOW(4);
+	netdev_info(dev, "  diagnostics: fifo %04x net %04x ethernet %04x media %04x\n",
+			inw(ioaddr+0x04), inw(ioaddr+0x06), inw(ioaddr+0x08),
+			inw(ioaddr+0x0a));
+	EL3WINDOW(1);
 }
 
 /* Reset and restore all of the 3c589 registers. */
 static void tc589_reset(struct net_device *dev)
 {
-    unsigned int ioaddr = dev->base_addr;
-    int i;
-
-    EL3WINDOW(0);
-    outw(0x0001, ioaddr + 4);			/* Activate board. */
-    outw(0x3f00, ioaddr + 8);			/* Set the IRQ line. */
-
-    /* Set the station address in window 2. */
-    EL3WINDOW(2);
-    for (i = 0; i < 6; i++)
-	outb(dev->dev_addr[i], ioaddr + i);
-
-    tc589_set_xcvr(dev, dev->if_port);
-
-    /* Switch to the stats window, and clear all stats by reading. */
-    outw(StatsDisable, ioaddr + EL3_CMD);
-    EL3WINDOW(6);
-    for (i = 0; i < 9; i++)
-	inb(ioaddr+i);
-    inw(ioaddr + 10);
-    inw(ioaddr + 12);
-
-    /* Switch to register set 1 for normal use. */
-    EL3WINDOW(1);
-
-    set_rx_mode(dev);
-    outw(StatsEnable, ioaddr + EL3_CMD); /* Turn on statistics. */
-    outw(RxEnable, ioaddr + EL3_CMD); /* Enable the receiver. */
-    outw(TxEnable, ioaddr + EL3_CMD); /* Enable transmitter. */
-    /* Allow status bits to be seen. */
-    outw(SetStatusEnb | 0xff, ioaddr + EL3_CMD);
-    /* Ack all pending events, and set active indicator mask. */
-    outw(AckIntr | IntLatch | TxAvailable | RxEarly | IntReq,
+	unsigned int ioaddr = dev->base_addr;
+	int i;
+
+	EL3WINDOW(0);
+	outw(0x0001, ioaddr + 4);			/* Activate board. */
+	outw(0x3f00, ioaddr + 8);			/* Set the IRQ line. */
+
+	/* Set the station address in window 2. */
+	EL3WINDOW(2);
+	for (i = 0; i < 6; i++)
+		outb(dev->dev_addr[i], ioaddr + i);
+
+	tc589_set_xcvr(dev, dev->if_port);
+
+	/* Switch to the stats window, and clear all stats by reading. */
+	outw(StatsDisable, ioaddr + EL3_CMD);
+	EL3WINDOW(6);
+	for (i = 0; i < 9; i++)
+		inb(ioaddr+i);
+	inw(ioaddr + 10);
+	inw(ioaddr + 12);
+
+	/* Switch to register set 1 for normal use. */
+	EL3WINDOW(1);
+
+	set_rx_mode(dev);
+	outw(StatsEnable, ioaddr + EL3_CMD); /* Turn on statistics. */
+	outw(RxEnable, ioaddr + EL3_CMD); /* Enable the receiver. */
+	outw(TxEnable, ioaddr + EL3_CMD); /* Enable transmitter. */
+	/* Allow status bits to be seen. */
+	outw(SetStatusEnb | 0xff, ioaddr + EL3_CMD);
+	/* Ack all pending events, and set active indicator mask. */
+	outw(AckIntr | IntLatch | TxAvailable | RxEarly | IntReq,
 	 ioaddr + EL3_CMD);
-    outw(SetIntrEnb | IntLatch | TxAvailable | RxComplete | StatsFull
+	outw(SetIntrEnb | IntLatch | TxAvailable | RxComplete | StatsFull
 	 | AdapterFailure, ioaddr + EL3_CMD);
 }
 
@@ -478,381 +494,406 @@ static const struct ethtool_ops netdev_ethtool_ops = {
 
 static int el3_config(struct net_device *dev, struct ifmap *map)
 {
-    if ((map->port != (u_char)(-1)) && (map->port != dev->if_port)) {
-	if (map->port <= 3) {
-	    dev->if_port = map->port;
-	    netdev_info(dev, "switched to %s port\n", if_names[dev->if_port]);
-	    tc589_set_xcvr(dev, dev->if_port);
-	} else
-	    return -EINVAL;
-    }
-    return 0;
+	if ((map->port != (u_char)(-1)) && (map->port != dev->if_port)) {
+		if (map->port <= 3) {
+			dev->if_port = map->port;
+			netdev_info(dev, "switched to %s port\n", if_names[dev->if_port]);
+			tc589_set_xcvr(dev, dev->if_port);
+		} else {
+			return -EINVAL;
+		}
+	}
+	return 0;
 }
 
 static int el3_open(struct net_device *dev)
 {
-    struct el3_private *lp = netdev_priv(dev);
-    struct pcmcia_device *link = lp->p_dev;
+	struct el3_private *lp = netdev_priv(dev);
+	struct pcmcia_device *link = lp->p_dev;
 
-    if (!pcmcia_dev_present(link))
-	return -ENODEV;
+	if (!pcmcia_dev_present(link))
+		return -ENODEV;
 
-    link->open++;
-    netif_start_queue(dev);
+	link->open++;
+	netif_start_queue(dev);
 
-    tc589_reset(dev);
-    init_timer(&lp->media);
-    lp->media.function = media_check;
-    lp->media.data = (unsigned long) dev;
-    lp->media.expires = jiffies + HZ;
-    add_timer(&lp->media);
+	tc589_reset(dev);
+	init_timer(&lp->media);
+	lp->media.function = media_check;
+	lp->media.data = (unsigned long) dev;
+	lp->media.expires = jiffies + HZ;
+	add_timer(&lp->media);
 
-    dev_dbg(&link->dev, "%s: opened, status %4.4x.\n",
+	dev_dbg(&link->dev, "%s: opened, status %4.4x.\n",
 	  dev->name, inw(dev->base_addr + EL3_STATUS));
 
-    return 0;
+	return 0;
 }
 
 static void el3_tx_timeout(struct net_device *dev)
 {
-    unsigned int ioaddr = dev->base_addr;
-
-    netdev_warn(dev, "Transmit timed out!\n");
-    dump_status(dev);
-    dev->stats.tx_errors++;
-    dev->trans_start = jiffies; /* prevent tx timeout */
-    /* Issue TX_RESET and TX_START commands. */
-    tc589_wait_for_completion(dev, TxReset);
-    outw(TxEnable, ioaddr + EL3_CMD);
-    netif_wake_queue(dev);
+	unsigned int ioaddr = dev->base_addr;
+
+	netdev_warn(dev, "Transmit timed out!\n");
+	dump_status(dev);
+	dev->stats.tx_errors++;
+	dev->trans_start = jiffies; /* prevent tx timeout */
+	/* Issue TX_RESET and TX_START commands. */
+	tc589_wait_for_completion(dev, TxReset);
+	outw(TxEnable, ioaddr + EL3_CMD);
+	netif_wake_queue(dev);
 }
 
 static void pop_tx_status(struct net_device *dev)
 {
-    unsigned int ioaddr = dev->base_addr;
-    int i;
-
-    /* Clear the Tx status stack. */
-    for (i = 32; i > 0; i--) {
-	u_char tx_status = inb(ioaddr + TX_STATUS);
-	if (!(tx_status & 0x84)) break;
-	/* reset transmitter on jabber error or underrun */
-	if (tx_status & 0x30)
-		tc589_wait_for_completion(dev, TxReset);
-	if (tx_status & 0x38) {
-		netdev_dbg(dev, "transmit error: status 0x%02x\n", tx_status);
-		outw(TxEnable, ioaddr + EL3_CMD);
-		dev->stats.tx_aborted_errors++;
+	unsigned int ioaddr = dev->base_addr;
+	int i;
+
+	/* Clear the Tx status stack. */
+	for (i = 32; i > 0; i--) {
+		u_char tx_status = inb(ioaddr + TX_STATUS);
+		if (!(tx_status & 0x84))
+			break;
+		/* reset transmitter on jabber error or underrun */
+		if (tx_status & 0x30)
+			tc589_wait_for_completion(dev, TxReset);
+		if (tx_status & 0x38) {
+			netdev_dbg(dev, "transmit error: status 0x%02x\n", tx_status);
+			outw(TxEnable, ioaddr + EL3_CMD);
+			dev->stats.tx_aborted_errors++;
+		}
+		outb(0x00, ioaddr + TX_STATUS); /* Pop the status stack. */
 	}
-	outb(0x00, ioaddr + TX_STATUS); /* Pop the status stack. */
-    }
 }
 
 static netdev_tx_t el3_start_xmit(struct sk_buff *skb,
 					struct net_device *dev)
 {
-    unsigned int ioaddr = dev->base_addr;
-    struct el3_private *priv = netdev_priv(dev);
-    unsigned long flags;
+	unsigned int ioaddr = dev->base_addr;
+	struct el3_private *priv = netdev_priv(dev);
+	unsigned long flags;
 
-    netdev_dbg(dev, "el3_start_xmit(length = %ld) called, status %4.4x.\n",
+	netdev_dbg(dev, "el3_start_xmit(length = %ld) called, status %4.4x.\n",
 	       (long)skb->len, inw(ioaddr + EL3_STATUS));
 
-    spin_lock_irqsave(&priv->lock, flags);
+	spin_lock_irqsave(&priv->lock, flags);
 
-    dev->stats.tx_bytes += skb->len;
+	dev->stats.tx_bytes += skb->len;
 
-    /* Put out the doubleword header... */
-    outw(skb->len, ioaddr + TX_FIFO);
-    outw(0x00, ioaddr + TX_FIFO);
-    /* ... and the packet rounded to a doubleword. */
-    outsl(ioaddr + TX_FIFO, skb->data, (skb->len + 3) >> 2);
+	/* Put out the doubleword header... */
+	outw(skb->len, ioaddr + TX_FIFO);
+	outw(0x00, ioaddr + TX_FIFO);
+	/* ... and the packet rounded to a doubleword. */
+	outsl(ioaddr + TX_FIFO, skb->data, (skb->len + 3) >> 2);
 
-    if (inw(ioaddr + TX_FREE) <= 1536) {
-	netif_stop_queue(dev);
-	/* Interrupt us when the FIFO has room for max-sized packet. */
-	outw(SetTxThreshold + 1536, ioaddr + EL3_CMD);
-    }
+	if (inw(ioaddr + TX_FREE) <= 1536) {
+		netif_stop_queue(dev);
+		/* Interrupt us when the FIFO has room for max-sized packet. */
+		outw(SetTxThreshold + 1536, ioaddr + EL3_CMD);
+	}
 
-    pop_tx_status(dev);
-    spin_unlock_irqrestore(&priv->lock, flags);
-    dev_kfree_skb(skb);
+	pop_tx_status(dev);
+	spin_unlock_irqrestore(&priv->lock, flags);
+	dev_kfree_skb(skb);
 
-    return NETDEV_TX_OK;
+	return NETDEV_TX_OK;
 }
 
 /* The EL3 interrupt handler. */
 static irqreturn_t el3_interrupt(int irq, void *dev_id)
 {
-    struct net_device *dev = (struct net_device *) dev_id;
-    struct el3_private *lp = netdev_priv(dev);
-    unsigned int ioaddr;
-    __u16 status;
-    int i = 0, handled = 1;
+	struct net_device *dev = (struct net_device *) dev_id;
+	struct el3_private *lp = netdev_priv(dev);
+	unsigned int ioaddr;
+	__u16 status;
+	int i = 0, handled = 1;
 
-    if (!netif_device_present(dev))
-	return IRQ_NONE;
+	if (!netif_device_present(dev))
+		return IRQ_NONE;
 
-    ioaddr = dev->base_addr;
+	ioaddr = dev->base_addr;
 
-    netdev_dbg(dev, "interrupt, status %4.4x.\n", inw(ioaddr + EL3_STATUS));
+	netdev_dbg(dev, "interrupt, status %4.4x.\n", inw(ioaddr + EL3_STATUS));
 
-    spin_lock(&lp->lock);
-    while ((status = inw(ioaddr + EL3_STATUS)) &
+	spin_lock(&lp->lock);
+	while ((status = inw(ioaddr + EL3_STATUS)) &
 	(IntLatch | RxComplete | StatsFull)) {
-	if ((status & 0xe000) != 0x2000) {
-		netdev_dbg(dev, "interrupt from dead card\n");
-		handled = 0;
-		break;
-	}
-	if (status & RxComplete)
-		el3_rx(dev);
-	if (status & TxAvailable) {
-		netdev_dbg(dev, "    TX room bit was handled.\n");
-		/* There's room in the FIFO for a full-sized packet. */
-		outw(AckIntr | TxAvailable, ioaddr + EL3_CMD);
-		netif_wake_queue(dev);
-	}
-	if (status & TxComplete)
-		pop_tx_status(dev);
-	if (status & (AdapterFailure | RxEarly | StatsFull)) {
-	    /* Handle all uncommon interrupts. */
-	    if (status & StatsFull)		/* Empty statistics. */
-		update_stats(dev);
-	    if (status & RxEarly) {		/* Rx early is unused. */
-		el3_rx(dev);
-		outw(AckIntr | RxEarly, ioaddr + EL3_CMD);
-	    }
-	    if (status & AdapterFailure) {
-		u16 fifo_diag;
-		EL3WINDOW(4);
-		fifo_diag = inw(ioaddr + 4);
-		EL3WINDOW(1);
-		netdev_warn(dev, "adapter failure, FIFO diagnostic register %04x.\n",
+		if ((status & 0xe000) != 0x2000) {
+			netdev_dbg(dev, "interrupt from dead card\n");
+			handled = 0;
+			break;
+		}
+		if (status & RxComplete)
+			el3_rx(dev);
+		if (status & TxAvailable) {
+			netdev_dbg(dev, "    TX room bit was handled.\n");
+			/* There's room in the FIFO for a full-sized packet. */
+			outw(AckIntr | TxAvailable, ioaddr + EL3_CMD);
+			netif_wake_queue(dev);
+		}
+		if (status & TxComplete)
+			pop_tx_status(dev);
+		if (status & (AdapterFailure | RxEarly | StatsFull)) {
+			/* Handle all uncommon interrupts. */
+			if (status & StatsFull)		/* Empty statistics. */
+				update_stats(dev);
+			if (status & RxEarly) {
+				/* Rx early is unused. */
+				el3_rx(dev);
+				outw(AckIntr | RxEarly, ioaddr + EL3_CMD);
+			}
+			if (status & AdapterFailure) {
+				u16 fifo_diag;
+				EL3WINDOW(4);
+				fifo_diag = inw(ioaddr + 4);
+				EL3WINDOW(1);
+				netdev_warn(dev, "adapter failure, FIFO diagnostic register %04x.\n",
 			    fifo_diag);
-		if (fifo_diag & 0x0400) {
-		    /* Tx overrun */
-		    tc589_wait_for_completion(dev, TxReset);
-		    outw(TxEnable, ioaddr + EL3_CMD);
+				if (fifo_diag & 0x0400) {
+					/* Tx overrun */
+					tc589_wait_for_completion(dev, TxReset);
+					outw(TxEnable, ioaddr + EL3_CMD);
+				}
+				if (fifo_diag & 0x2000) {
+					/* Rx underrun */
+					tc589_wait_for_completion(dev, RxReset);
+					set_rx_mode(dev);
+					outw(RxEnable, ioaddr + EL3_CMD);
+				}
+				outw(AckIntr | AdapterFailure, ioaddr + EL3_CMD);
+			}
 		}
-		if (fifo_diag & 0x2000) {
-		    /* Rx underrun */
-		    tc589_wait_for_completion(dev, RxReset);
-		    set_rx_mode(dev);
-		    outw(RxEnable, ioaddr + EL3_CMD);
+		if (++i > 10) {
+			netdev_err(dev, "infinite loop in interrupt, status %4.4x.\n",
+					status);
+			/* Clear all interrupts */
+			outw(AckIntr | 0xFF, ioaddr + EL3_CMD);
+			break;
 		}
-		outw(AckIntr | AdapterFailure, ioaddr + EL3_CMD);
-	    }
+		/* Acknowledge the IRQ. */
+		outw(AckIntr | IntReq | IntLatch, ioaddr + EL3_CMD);
 	}
-	if (++i > 10) {
-		netdev_err(dev, "infinite loop in interrupt, status %4.4x.\n",
-			   status);
-		/* Clear all interrupts */
-		outw(AckIntr | 0xFF, ioaddr + EL3_CMD);
-		break;
-	}
-	/* Acknowledge the IRQ. */
-	outw(AckIntr | IntReq | IntLatch, ioaddr + EL3_CMD);
-    }
-    lp->last_irq = jiffies;
-    spin_unlock(&lp->lock);
-    netdev_dbg(dev, "exiting interrupt, status %4.4x.\n",
-	       inw(ioaddr + EL3_STATUS));
-    return IRQ_RETVAL(handled);
+	lp->last_irq = jiffies;
+	spin_unlock(&lp->lock);
+	netdev_dbg(dev, "exiting interrupt, status %4.4x.\n",
+			inw(ioaddr + EL3_STATUS));
+	return IRQ_RETVAL(handled);
 }
 
 static void media_check(unsigned long arg)
 {
-    struct net_device *dev = (struct net_device *)(arg);
-    struct el3_private *lp = netdev_priv(dev);
-    unsigned int ioaddr = dev->base_addr;
-    u16 media, errs;
-    unsigned long flags;
+	struct net_device *dev = (struct net_device *)(arg);
+	struct el3_private *lp = netdev_priv(dev);
+	unsigned int ioaddr = dev->base_addr;
+	u16 media, errs;
+	unsigned long flags;
 
-    if (!netif_device_present(dev)) goto reschedule;
+	if (!netif_device_present(dev))
+		goto reschedule;
 
-    /* Check for pending interrupt with expired latency timer: with
-       this, we can limp along even if the interrupt is blocked */
-    if ((inw(ioaddr + EL3_STATUS) & IntLatch) &&
+	/* Check for pending interrupt with expired latency timer: with
+	 * this, we can limp along even if the interrupt is blocked
+	 */
+	if ((inw(ioaddr + EL3_STATUS) & IntLatch) &&
 	(inb(ioaddr + EL3_TIMER) == 0xff)) {
-	if (!lp->fast_poll)
-		netdev_warn(dev, "interrupt(s) dropped!\n");
-
-	local_irq_save(flags);
-	el3_interrupt(dev->irq, dev);
-	local_irq_restore(flags);
-
-	lp->fast_poll = HZ;
-    }
-    if (lp->fast_poll) {
-	lp->fast_poll--;
-	lp->media.expires = jiffies + HZ/100;
-	add_timer(&lp->media);
-	return;
-    }
-
-    /* lp->lock guards the EL3 window. Window should always be 1 except
-       when the lock is held */
-    spin_lock_irqsave(&lp->lock, flags);
-    EL3WINDOW(4);
-    media = inw(ioaddr+WN4_MEDIA) & 0xc810;
-
-    /* Ignore collisions unless we've had no irq's recently */
-    if (time_before(jiffies, lp->last_irq + HZ)) {
-	media &= ~0x0010;
-    } else {
-	/* Try harder to detect carrier errors */
-	EL3WINDOW(6);
-	outw(StatsDisable, ioaddr + EL3_CMD);
-	errs = inb(ioaddr + 0);
-	outw(StatsEnable, ioaddr + EL3_CMD);
-	dev->stats.tx_carrier_errors += errs;
-	if (errs || (lp->media_status & 0x0010)) media |= 0x0010;
-    }
+		if (!lp->fast_poll)
+			netdev_warn(dev, "interrupt(s) dropped!\n");
+
+		local_irq_save(flags);
+		el3_interrupt(dev->irq, dev);
+		local_irq_restore(flags);
+
+		lp->fast_poll = HZ;
+	}
+	if (lp->fast_poll) {
+		lp->fast_poll--;
+		lp->media.expires = jiffies + HZ/100;
+		add_timer(&lp->media);
+		return;
+	}
+
+	/* lp->lock guards the EL3 window. Window should always be 1 except
+	 * when the lock is held
+	 */
+
+	spin_lock_irqsave(&lp->lock, flags);
+	EL3WINDOW(4);
+	media = inw(ioaddr+WN4_MEDIA) & 0xc810;
+
+	/* Ignore collisions unless we've had no irq's recently */
+	if (time_before(jiffies, lp->last_irq + HZ)) {
+		media &= ~0x0010;
+	} else {
+		/* Try harder to detect carrier errors */
+		EL3WINDOW(6);
+		outw(StatsDisable, ioaddr + EL3_CMD);
+		errs = inb(ioaddr + 0);
+		outw(StatsEnable, ioaddr + EL3_CMD);
+		dev->stats.tx_carrier_errors += errs;
+		if (errs || (lp->media_status & 0x0010))
+			media |= 0x0010;
+	}
 
-    if (media != lp->media_status) {
-	if ((media & lp->media_status & 0x8000) &&
-	    ((lp->media_status ^ media) & 0x0800))
+	if (media != lp->media_status) {
+		if ((media & lp->media_status & 0x8000) &&
+				((lp->media_status ^ media) & 0x0800))
 		netdev_info(dev, "%s link beat\n",
-			    (lp->media_status & 0x0800 ? "lost" : "found"));
-	else if ((media & lp->media_status & 0x4000) &&
+				(lp->media_status & 0x0800 ? "lost" : "found"));
+		else if ((media & lp->media_status & 0x4000) &&
 		 ((lp->media_status ^ media) & 0x0010))
 		netdev_info(dev, "coax cable %s\n",
-			    (lp->media_status & 0x0010 ? "ok" : "problem"));
-	if (dev->if_port == 0) {
-	    if (media & 0x8000) {
-		if (media & 0x0800)
-			netdev_info(dev, "flipped to 10baseT\n");
-		else
+				(lp->media_status & 0x0010 ? "ok" : "problem"));
+		if (dev->if_port == 0) {
+			if (media & 0x8000) {
+				if (media & 0x0800)
+					netdev_info(dev, "flipped to 10baseT\n");
+				else
 			tc589_set_xcvr(dev, 2);
-	    } else if (media & 0x4000) {
-		if (media & 0x0010)
-		    tc589_set_xcvr(dev, 1);
-		else
-		    netdev_info(dev, "flipped to 10base2\n");
-	    }
+			} else if (media & 0x4000) {
+				if (media & 0x0010)
+					tc589_set_xcvr(dev, 1);
+				else
+					netdev_info(dev, "flipped to 10base2\n");
+			}
+		}
+		lp->media_status = media;
 	}
-	lp->media_status = media;
-    }
 
-    EL3WINDOW(1);
-    spin_unlock_irqrestore(&lp->lock, flags);
+	EL3WINDOW(1);
+	spin_unlock_irqrestore(&lp->lock, flags);
 
 reschedule:
-    lp->media.expires = jiffies + HZ;
-    add_timer(&lp->media);
+	lp->media.expires = jiffies + HZ;
+	add_timer(&lp->media);
 }
 
 static struct net_device_stats *el3_get_stats(struct net_device *dev)
 {
-    struct el3_private *lp = netdev_priv(dev);
-    unsigned long flags;
-    struct pcmcia_device *link = lp->p_dev;
+	struct el3_private *lp = netdev_priv(dev);
+	unsigned long flags;
+	struct pcmcia_device *link = lp->p_dev;
 
-    if (pcmcia_dev_present(link)) {
-	spin_lock_irqsave(&lp->lock, flags);
-	update_stats(dev);
-	spin_unlock_irqrestore(&lp->lock, flags);
-    }
-    return &dev->stats;
+	if (pcmcia_dev_present(link)) {
+		spin_lock_irqsave(&lp->lock, flags);
+		update_stats(dev);
+		spin_unlock_irqrestore(&lp->lock, flags);
+	}
+	return &dev->stats;
 }
 
-/*
-  Update statistics.  We change to register window 6, so this should be run
-  single-threaded if the device is active. This is expected to be a rare
-  operation, and it's simpler for the rest of the driver to assume that
-  window 1 is always valid rather than use a special window-state variable.
-
-  Caller must hold the lock for this
+/* Update statistics.  We change to register window 6, so this should be run
+* single-threaded if the device is active. This is expected to be a rare
+* operation, and it's simpler for the rest of the driver to assume that
+* window 1 is always valid rather than use a special window-state variable.
+*
+* Caller must hold the lock for this
 */
+
 static void update_stats(struct net_device *dev)
 {
-    unsigned int ioaddr = dev->base_addr;
-
-    netdev_dbg(dev, "updating the statistics.\n");
-    /* Turn off statistics updates while reading. */
-    outw(StatsDisable, ioaddr + EL3_CMD);
-    /* Switch to the stats window, and read everything. */
-    EL3WINDOW(6);
-    dev->stats.tx_carrier_errors	+= inb(ioaddr + 0);
-    dev->stats.tx_heartbeat_errors	+= inb(ioaddr + 1);
-    /* Multiple collisions. */		inb(ioaddr + 2);
-    dev->stats.collisions		+= inb(ioaddr + 3);
-    dev->stats.tx_window_errors		+= inb(ioaddr + 4);
-    dev->stats.rx_fifo_errors		+= inb(ioaddr + 5);
-    dev->stats.tx_packets		+= inb(ioaddr + 6);
-    /* Rx packets   */			inb(ioaddr + 7);
-    /* Tx deferrals */			inb(ioaddr + 8);
-    /* Rx octets */			inw(ioaddr + 10);
-    /* Tx octets */			inw(ioaddr + 12);
-
-    /* Back to window 1, and turn statistics back on. */
-    EL3WINDOW(1);
-    outw(StatsEnable, ioaddr + EL3_CMD);
+	unsigned int ioaddr = dev->base_addr;
+
+	netdev_dbg(dev, "updating the statistics.\n");
+	/* Turn off statistics updates while reading. */
+	outw(StatsDisable, ioaddr + EL3_CMD);
+	/* Switch to the stats window, and read everything. */
+	EL3WINDOW(6);
+	dev->stats.tx_carrier_errors	+= inb(ioaddr + 0);
+	dev->stats.tx_heartbeat_errors	+= inb(ioaddr + 1);
+	/* Multiple collisions. */
+	inb(ioaddr + 2);
+	dev->stats.collisions		+= inb(ioaddr + 3);
+	dev->stats.tx_window_errors		+= inb(ioaddr + 4);
+	dev->stats.rx_fifo_errors		+= inb(ioaddr + 5);
+	dev->stats.tx_packets		+= inb(ioaddr + 6);
+	/* Rx packets   */
+	inb(ioaddr + 7);
+	/* Tx deferrals */
+	inb(ioaddr + 8);
+	/* Rx octets */
+	inw(ioaddr + 10);
+	/* Tx octets */
+	inw(ioaddr + 12);
+
+	/* Back to window 1, and turn statistics back on. */
+	EL3WINDOW(1);
+	outw(StatsEnable, ioaddr + EL3_CMD);
 }
 
 static int el3_rx(struct net_device *dev)
 {
-    unsigned int ioaddr = dev->base_addr;
-    int worklimit = 32;
-    short rx_status;
+	unsigned int ioaddr = dev->base_addr;
+	int worklimit = 32;
+	short rx_status;
 
-    netdev_dbg(dev, "in rx_packet(), status %4.4x, rx_status %4.4x.\n",
+	netdev_dbg(dev, "in rx_packet(), status %4.4x, rx_status %4.4x.\n",
 	       inw(ioaddr+EL3_STATUS), inw(ioaddr+RX_STATUS));
-    while (!((rx_status = inw(ioaddr + RX_STATUS)) & 0x8000) &&
+	while (!((rx_status = inw(ioaddr + RX_STATUS)) & 0x8000) &&
 		    worklimit > 0) {
-	worklimit--;
-	if (rx_status & 0x4000) { /* Error, update stats. */
-	    short error = rx_status & 0x3800;
-	    dev->stats.rx_errors++;
-	    switch (error) {
-	    case 0x0000:	dev->stats.rx_over_errors++; break;
-	    case 0x0800:	dev->stats.rx_length_errors++; break;
-	    case 0x1000:	dev->stats.rx_frame_errors++; break;
-	    case 0x1800:	dev->stats.rx_length_errors++; break;
-	    case 0x2000:	dev->stats.rx_frame_errors++; break;
-	    case 0x2800:	dev->stats.rx_crc_errors++; break;
-	    }
-	} else {
-	    short pkt_len = rx_status & 0x7ff;
-	    struct sk_buff *skb;
-
-	    skb = netdev_alloc_skb(dev, pkt_len + 5);
-
-	    netdev_dbg(dev, "    Receiving packet size %d status %4.4x.\n",
+		worklimit--;
+		if (rx_status & 0x4000) { /* Error, update stats. */
+			short error = rx_status & 0x3800;
+			dev->stats.rx_errors++;
+			switch (error) {
+			case 0x0000:
+				dev->stats.rx_over_errors++;
+				break;
+			case 0x0800:
+				dev->stats.rx_length_errors++;
+				break;
+			case 0x1000:
+				dev->stats.rx_frame_errors++;
+				break;
+			case 0x1800:
+				dev->stats.rx_length_errors++;
+				break;
+			case 0x2000:
+				dev->stats.rx_frame_errors++;
+				break;
+			case 0x2800:
+				dev->stats.rx_crc_errors++;
+				break;
+			}
+		} else {
+			short pkt_len = rx_status & 0x7ff;
+			struct sk_buff *skb;
+
+			skb = netdev_alloc_skb(dev, pkt_len + 5);
+
+			netdev_dbg(dev, "    Receiving packet size %d status %4.4x.\n",
 		       pkt_len, rx_status);
-	    if (skb != NULL) {
-		skb_reserve(skb, 2);
-		insl(ioaddr+RX_FIFO, skb_put(skb, pkt_len),
+			if (skb != NULL) {
+				skb_reserve(skb, 2);
+				insl(ioaddr+RX_FIFO, skb_put(skb, pkt_len),
 			(pkt_len+3)>>2);
-		skb->protocol = eth_type_trans(skb, dev);
-		netif_rx(skb);
-		dev->stats.rx_packets++;
-		dev->stats.rx_bytes += pkt_len;
-	    } else {
-		netdev_dbg(dev, "couldn't allocate a sk_buff of size %d.\n",
+				skb->protocol = eth_type_trans(skb, dev);
+				netif_rx(skb);
+				dev->stats.rx_packets++;
+				dev->stats.rx_bytes += pkt_len;
+			} else {
+				netdev_dbg(dev, "couldn't allocate a sk_buff of size %d.\n",
 			   pkt_len);
-		dev->stats.rx_dropped++;
-	    }
+				dev->stats.rx_dropped++;
+			}
+		}
+		/* Pop the top of the Rx FIFO */
+		tc589_wait_for_completion(dev, RxDiscard);
 	}
-	/* Pop the top of the Rx FIFO */
-	tc589_wait_for_completion(dev, RxDiscard);
-    }
-    if (worklimit == 0)
-	netdev_warn(dev, "too much work in el3_rx!\n");
-    return 0;
+	if (worklimit == 0)
+		netdev_warn(dev, "too much work in el3_rx!\n");
+	return 0;
 }
 
 static void set_rx_mode(struct net_device *dev)
 {
-    unsigned int ioaddr = dev->base_addr;
-    u16 opts = SetRxFilter | RxStation | RxBroadcast;
-
-    if (dev->flags & IFF_PROMISC)
-	opts |= RxMulticast | RxProm;
-    else if (!netdev_mc_empty(dev) || (dev->flags & IFF_ALLMULTI))
-	opts |= RxMulticast;
-    outw(opts, ioaddr + EL3_CMD);
+	unsigned int ioaddr = dev->base_addr;
+	u16 opts = SetRxFilter | RxStation | RxBroadcast;
+
+	if (dev->flags & IFF_PROMISC)
+		opts |= RxMulticast | RxProm;
+	else if (!netdev_mc_empty(dev) || (dev->flags & IFF_ALLMULTI))
+		opts |= RxMulticast;
+	outw(opts, ioaddr + EL3_CMD);
 }
 
 static void set_multicast_list(struct net_device *dev)
@@ -867,44 +908,44 @@ static void set_multicast_list(struct net_device *dev)
 
 static int el3_close(struct net_device *dev)
 {
-    struct el3_private *lp = netdev_priv(dev);
-    struct pcmcia_device *link = lp->p_dev;
-    unsigned int ioaddr = dev->base_addr;
-
-    dev_dbg(&link->dev, "%s: shutting down ethercard.\n", dev->name);
+	struct el3_private *lp = netdev_priv(dev);
+	struct pcmcia_device *link = lp->p_dev;
+	unsigned int ioaddr = dev->base_addr;
+
+	dev_dbg(&link->dev, "%s: shutting down ethercard.\n", dev->name);
+
+	if (pcmcia_dev_present(link)) {
+		/* Turn off statistics ASAP.  We update dev->stats below. */
+		outw(StatsDisable, ioaddr + EL3_CMD);
+
+		/* Disable the receiver and transmitter. */
+		outw(RxDisable, ioaddr + EL3_CMD);
+		outw(TxDisable, ioaddr + EL3_CMD);
+
+		if (dev->if_port == 2)
+			/* Turn off thinnet power.  Green! */
+			outw(StopCoax, ioaddr + EL3_CMD);
+		else if (dev->if_port == 1) {
+			/* Disable link beat and jabber */
+			EL3WINDOW(4);
+			outw(0, ioaddr + WN4_MEDIA);
+		}
 
-    if (pcmcia_dev_present(link)) {
-	/* Turn off statistics ASAP.  We update dev->stats below. */
-	outw(StatsDisable, ioaddr + EL3_CMD);
+		/* Switching back to window 0 disables the IRQ. */
+		EL3WINDOW(0);
+		/* But we explicitly zero the IRQ line select anyway. */
+		outw(0x0f00, ioaddr + WN0_IRQ);
 
-	/* Disable the receiver and transmitter. */
-	outw(RxDisable, ioaddr + EL3_CMD);
-	outw(TxDisable, ioaddr + EL3_CMD);
-
-	if (dev->if_port == 2)
-	    /* Turn off thinnet power.  Green! */
-	    outw(StopCoax, ioaddr + EL3_CMD);
-	else if (dev->if_port == 1) {
-	    /* Disable link beat and jabber */
-	    EL3WINDOW(4);
-	    outw(0, ioaddr + WN4_MEDIA);
+		/* Check if the card still exists */
+		if ((inw(ioaddr+EL3_STATUS) & 0xe000) == 0x2000)
+			update_stats(dev);
 	}
 
-	/* Switching back to window 0 disables the IRQ. */
-	EL3WINDOW(0);
-	/* But we explicitly zero the IRQ line select anyway. */
-	outw(0x0f00, ioaddr + WN0_IRQ);
-
-	/* Check if the card still exists */
-	if ((inw(ioaddr+EL3_STATUS) & 0xe000) == 0x2000)
-	    update_stats(dev);
-    }
-
-    link->open--;
-    netif_stop_queue(dev);
-    del_timer_sync(&lp->media);
+	link->open--;
+	netif_stop_queue(dev);
+	del_timer_sync(&lp->media);
 
-    return 0;
+	return 0;
 }
 
 static const struct pcmcia_device_id tc589_ids[] = {
-- 
1.9.0.rc3

^ permalink raw reply related

* pull request: wireless 2014-02-17
From: John W. Linville @ 2014-02-17 22:39 UTC (permalink / raw)
  To: davem; +Cc: linux-wireless, netdev, linux-kernel

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

Dave,

Please pull this batch of fixes intended for the 3.14 stream...

For the iwlwifi one, Emmanuel says:

"As explicitly written in the commit message, we prefer to disable Tx
AMPDU on NICs supported by iwldvm. This feature gives a big boost in
Tx performance, but the firmware is buggy and we can't rely on it.
Our hope is that most of the users out there want wifi to surf on
the web which means that they care more for Rx traffic than for Tx.
People who want to enable it can do so with the help of a module
parameter."

On top of that...

Dan Carpenter fixes a typo/thinko in ath5k.

Olivier Langlois fixes a couple of rtlwifi issues, one which leaves
IRQs disabled too long (causing a variety of problems elsewhere),
and one which fixes an incorrect return code when failing to enable
the NIC.

Russell King fixes a NULL pointer dereference in hostap.

Stanislaw Gruszka fixes a DMA coherence issue in the rtl8187 driver.

Please let me know if there are problems!

Thanks,

John

---

The following changes since commit eb85569fe2d06c2fbf4de7b66c263ca095b397aa:

  usbnet: remove generic hard_header_len check (2014-02-17 14:35:46 -0500)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless.git for-davem

for you to fetch changes up to ff95fe382c658d81ebe01ba68dca2e00ca814ff0:

  Merge branch 'master' of git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless into for-davem (2014-02-17 15:54:31 -0500)

----------------------------------------------------------------

Dan Carpenter (1):
      ath5k: shifting the wrong variable for AR5K_AR5210

Emmanuel Grumbach (1):
      iwlwifi: disable TX AMPDU by default for iwldvm

John W. Linville (2):
      Merge git://git.kernel.org/.../iwlwifi/iwlwifi-fixes
      Merge branch 'master' of git://git.kernel.org/.../linville/wireless into for-davem

Olivier Langlois (2):
      rtlwifi: rtl8192ce: Fix too long disable of IRQs
      rtlwifi: Fix incorrect return from rtl_ps_enable_nic()

Russell King - ARM Linux (1):
      hostap: fix "hostap: proc: Use remove_proc_subtree()"

Stanislaw Gruszka (1):
      rtl8187: fix regression on MIPS without coherent DMA

 drivers/net/wireless/ath/ath5k/phy.c           |  2 +-
 drivers/net/wireless/hostap/hostap_proc.c      |  2 +-
 drivers/net/wireless/iwlwifi/dvm/mac80211.c    | 22 ++++++++++++++++++++--
 drivers/net/wireless/iwlwifi/iwl-drv.c         |  2 +-
 drivers/net/wireless/iwlwifi/iwl-modparams.h   | 11 +++++++----
 drivers/net/wireless/iwlwifi/mvm/mac80211.c    | 22 ++++++++++++++++++++--
 drivers/net/wireless/rtl818x/rtl8187/rtl8187.h | 10 ++++++++--
 drivers/net/wireless/rtlwifi/ps.c              |  2 +-
 drivers/net/wireless/rtlwifi/rtl8192ce/hw.c    | 18 ++++++++++++++++--
 9 files changed, 75 insertions(+), 16 deletions(-)
diff --git a/drivers/net/wireless/ath/ath5k/phy.c b/drivers/net/wireless/ath/ath5k/phy.c
index d6bc7cb61bfb..1a2973b7acf2 100644
--- a/drivers/net/wireless/ath/ath5k/phy.c
+++ b/drivers/net/wireless/ath/ath5k/phy.c
@@ -110,7 +110,7 @@ ath5k_hw_radio_revision(struct ath5k_hw *ah, enum ieee80211_band band)
 		ath5k_hw_reg_write(ah, 0x00010000, AR5K_PHY(0x20));
 
 	if (ah->ah_version == AR5K_AR5210) {
-		srev = ath5k_hw_reg_read(ah, AR5K_PHY(256) >> 28) & 0xf;
+		srev = (ath5k_hw_reg_read(ah, AR5K_PHY(256)) >> 28) & 0xf;
 		ret = (u16)ath5k_hw_bitswap(srev, 4) + 1;
 	} else {
 		srev = (ath5k_hw_reg_read(ah, AR5K_PHY(0x100)) >> 24) & 0xff;
diff --git a/drivers/net/wireless/hostap/hostap_proc.c b/drivers/net/wireless/hostap/hostap_proc.c
index aa7ad3a7a69b..4e5c0f8c9496 100644
--- a/drivers/net/wireless/hostap/hostap_proc.c
+++ b/drivers/net/wireless/hostap/hostap_proc.c
@@ -496,7 +496,7 @@ void hostap_init_proc(local_info_t *local)
 
 void hostap_remove_proc(local_info_t *local)
 {
-	remove_proc_subtree(local->ddev->name, hostap_proc);
+	proc_remove(local->proc);
 }
 
 
diff --git a/drivers/net/wireless/iwlwifi/dvm/mac80211.c b/drivers/net/wireless/iwlwifi/dvm/mac80211.c
index c24d1d3d55f6..73086c1629ca 100644
--- a/drivers/net/wireless/iwlwifi/dvm/mac80211.c
+++ b/drivers/net/wireless/iwlwifi/dvm/mac80211.c
@@ -696,6 +696,24 @@ static int iwlagn_mac_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
 	return ret;
 }
 
+static inline bool iwl_enable_rx_ampdu(const struct iwl_cfg *cfg)
+{
+	if (iwlwifi_mod_params.disable_11n & IWL_DISABLE_HT_RXAGG)
+		return false;
+	return true;
+}
+
+static inline bool iwl_enable_tx_ampdu(const struct iwl_cfg *cfg)
+{
+	if (iwlwifi_mod_params.disable_11n & IWL_DISABLE_HT_TXAGG)
+		return false;
+	if (iwlwifi_mod_params.disable_11n & IWL_ENABLE_HT_TXAGG)
+		return true;
+
+	/* disabled by default */
+	return false;
+}
+
 static int iwlagn_mac_ampdu_action(struct ieee80211_hw *hw,
 				   struct ieee80211_vif *vif,
 				   enum ieee80211_ampdu_mlme_action action,
@@ -717,7 +735,7 @@ static int iwlagn_mac_ampdu_action(struct ieee80211_hw *hw,
 
 	switch (action) {
 	case IEEE80211_AMPDU_RX_START:
-		if (iwlwifi_mod_params.disable_11n & IWL_DISABLE_HT_RXAGG)
+		if (!iwl_enable_rx_ampdu(priv->cfg))
 			break;
 		IWL_DEBUG_HT(priv, "start Rx\n");
 		ret = iwl_sta_rx_agg_start(priv, sta, tid, *ssn);
@@ -729,7 +747,7 @@ static int iwlagn_mac_ampdu_action(struct ieee80211_hw *hw,
 	case IEEE80211_AMPDU_TX_START:
 		if (!priv->trans->ops->txq_enable)
 			break;
-		if (iwlwifi_mod_params.disable_11n & IWL_DISABLE_HT_TXAGG)
+		if (!iwl_enable_tx_ampdu(priv->cfg))
 			break;
 		IWL_DEBUG_HT(priv, "start Tx\n");
 		ret = iwlagn_tx_agg_start(priv, vif, sta, tid, ssn);
diff --git a/drivers/net/wireless/iwlwifi/iwl-drv.c b/drivers/net/wireless/iwlwifi/iwl-drv.c
index c3728163be46..75103554cd63 100644
--- a/drivers/net/wireless/iwlwifi/iwl-drv.c
+++ b/drivers/net/wireless/iwlwifi/iwl-drv.c
@@ -1286,7 +1286,7 @@ module_param_named(swcrypto, iwlwifi_mod_params.sw_crypto, int, S_IRUGO);
 MODULE_PARM_DESC(swcrypto, "using crypto in software (default 0 [hardware])");
 module_param_named(11n_disable, iwlwifi_mod_params.disable_11n, uint, S_IRUGO);
 MODULE_PARM_DESC(11n_disable,
-	"disable 11n functionality, bitmap: 1: full, 2: agg TX, 4: agg RX");
+	"disable 11n functionality, bitmap: 1: full, 2: disable agg TX, 4: disable agg RX, 8 enable agg TX");
 module_param_named(amsdu_size_8K, iwlwifi_mod_params.amsdu_size_8K,
 		   int, S_IRUGO);
 MODULE_PARM_DESC(amsdu_size_8K, "enable 8K amsdu size (default 0)");
diff --git a/drivers/net/wireless/iwlwifi/iwl-modparams.h b/drivers/net/wireless/iwlwifi/iwl-modparams.h
index 0a84ade7edac..b29075c3da8e 100644
--- a/drivers/net/wireless/iwlwifi/iwl-modparams.h
+++ b/drivers/net/wireless/iwlwifi/iwl-modparams.h
@@ -79,9 +79,12 @@ enum iwl_power_level {
 	IWL_POWER_NUM
 };
 
-#define IWL_DISABLE_HT_ALL	BIT(0)
-#define IWL_DISABLE_HT_TXAGG	BIT(1)
-#define IWL_DISABLE_HT_RXAGG	BIT(2)
+enum iwl_disable_11n {
+	IWL_DISABLE_HT_ALL	 = BIT(0),
+	IWL_DISABLE_HT_TXAGG	 = BIT(1),
+	IWL_DISABLE_HT_RXAGG	 = BIT(2),
+	IWL_ENABLE_HT_TXAGG	 = BIT(3),
+};
 
 /**
  * struct iwl_mod_params
@@ -90,7 +93,7 @@ enum iwl_power_level {
  *
  * @sw_crypto: using hardware encryption, default = 0
  * @disable_11n: disable 11n capabilities, default = 0,
- *	use IWL_DISABLE_HT_* constants
+ *	use IWL_[DIS,EN]ABLE_HT_* constants
  * @amsdu_size_8K: enable 8K amsdu size, default = 0
  * @restart_fw: restart firmware, default = 1
  * @wd_disable: enable stuck queue check, default = 0
diff --git a/drivers/net/wireless/iwlwifi/mvm/mac80211.c b/drivers/net/wireless/iwlwifi/mvm/mac80211.c
index 6bf9766e5982..c35b8661b395 100644
--- a/drivers/net/wireless/iwlwifi/mvm/mac80211.c
+++ b/drivers/net/wireless/iwlwifi/mvm/mac80211.c
@@ -328,6 +328,24 @@ static void iwl_mvm_mac_tx(struct ieee80211_hw *hw,
 	ieee80211_free_txskb(hw, skb);
 }
 
+static inline bool iwl_enable_rx_ampdu(const struct iwl_cfg *cfg)
+{
+	if (iwlwifi_mod_params.disable_11n & IWL_DISABLE_HT_RXAGG)
+		return false;
+	return true;
+}
+
+static inline bool iwl_enable_tx_ampdu(const struct iwl_cfg *cfg)
+{
+	if (iwlwifi_mod_params.disable_11n & IWL_DISABLE_HT_TXAGG)
+		return false;
+	if (iwlwifi_mod_params.disable_11n & IWL_ENABLE_HT_TXAGG)
+		return true;
+
+	/* enabled by default */
+	return true;
+}
+
 static int iwl_mvm_mac_ampdu_action(struct ieee80211_hw *hw,
 				    struct ieee80211_vif *vif,
 				    enum ieee80211_ampdu_mlme_action action,
@@ -347,7 +365,7 @@ static int iwl_mvm_mac_ampdu_action(struct ieee80211_hw *hw,
 
 	switch (action) {
 	case IEEE80211_AMPDU_RX_START:
-		if (iwlwifi_mod_params.disable_11n & IWL_DISABLE_HT_RXAGG) {
+		if (!iwl_enable_rx_ampdu(mvm->cfg)) {
 			ret = -EINVAL;
 			break;
 		}
@@ -357,7 +375,7 @@ static int iwl_mvm_mac_ampdu_action(struct ieee80211_hw *hw,
 		ret = iwl_mvm_sta_rx_agg(mvm, sta, tid, 0, false);
 		break;
 	case IEEE80211_AMPDU_TX_START:
-		if (iwlwifi_mod_params.disable_11n & IWL_DISABLE_HT_TXAGG) {
+		if (!iwl_enable_tx_ampdu(mvm->cfg)) {
 			ret = -EINVAL;
 			break;
 		}
diff --git a/drivers/net/wireless/rtl818x/rtl8187/rtl8187.h b/drivers/net/wireless/rtl818x/rtl8187/rtl8187.h
index 56aee067f324..a6ad79f61bf9 100644
--- a/drivers/net/wireless/rtl818x/rtl8187/rtl8187.h
+++ b/drivers/net/wireless/rtl818x/rtl8187/rtl8187.h
@@ -15,6 +15,8 @@
 #ifndef RTL8187_H
 #define RTL8187_H
 
+#include <linux/cache.h>
+
 #include "rtl818x.h"
 #include "leds.h"
 
@@ -139,7 +141,10 @@ struct rtl8187_priv {
 	u8 aifsn[4];
 	u8 rfkill_mask;
 	struct {
-		__le64 buf;
+		union {
+			__le64 buf;
+			u8 dummy1[L1_CACHE_BYTES];
+		} ____cacheline_aligned;
 		struct sk_buff_head queue;
 	} b_tx_status; /* This queue is used by both -b and non-b devices */
 	struct mutex io_mutex;
@@ -147,7 +152,8 @@ struct rtl8187_priv {
 		u8 bits8;
 		__le16 bits16;
 		__le32 bits32;
-	} *io_dmabuf;
+		u8 dummy2[L1_CACHE_BYTES];
+	} *io_dmabuf ____cacheline_aligned;
 	bool rfkill_off;
 	u16 seqno;
 };
diff --git a/drivers/net/wireless/rtlwifi/ps.c b/drivers/net/wireless/rtlwifi/ps.c
index deedae3c5449..d1c0191a195b 100644
--- a/drivers/net/wireless/rtlwifi/ps.c
+++ b/drivers/net/wireless/rtlwifi/ps.c
@@ -48,7 +48,7 @@ bool rtl_ps_enable_nic(struct ieee80211_hw *hw)
 
 	/*<2> Enable Adapter */
 	if (rtlpriv->cfg->ops->hw_init(hw))
-		return 1;
+		return false;
 	RT_CLEAR_PS_LEVEL(ppsc, RT_RF_OFF_LEVL_HALT_NIC);
 
 	/*<3> Enable Interrupt */
diff --git a/drivers/net/wireless/rtlwifi/rtl8192ce/hw.c b/drivers/net/wireless/rtlwifi/rtl8192ce/hw.c
index a82b30a1996c..2eb0b38384dd 100644
--- a/drivers/net/wireless/rtlwifi/rtl8192ce/hw.c
+++ b/drivers/net/wireless/rtlwifi/rtl8192ce/hw.c
@@ -937,14 +937,26 @@ int rtl92ce_hw_init(struct ieee80211_hw *hw)
 	bool is92c;
 	int err;
 	u8 tmp_u1b;
+	unsigned long flags;
 
 	rtlpci->being_init_adapter = true;
+
+	/* Since this function can take a very long time (up to 350 ms)
+	 * and can be called with irqs disabled, reenable the irqs
+	 * to let the other devices continue being serviced.
+	 *
+	 * It is safe doing so since our own interrupts will only be enabled
+	 * in a subsequent step.
+	 */
+	local_save_flags(flags);
+	local_irq_enable();
+
 	rtlpriv->intf_ops->disable_aspm(hw);
 	rtstatus = _rtl92ce_init_mac(hw);
 	if (!rtstatus) {
 		RT_TRACE(rtlpriv, COMP_ERR, DBG_EMERG, "Init MAC failed\n");
 		err = 1;
-		return err;
+		goto exit;
 	}
 
 	err = rtl92c_download_fw(hw);
@@ -952,7 +964,7 @@ int rtl92ce_hw_init(struct ieee80211_hw *hw)
 		RT_TRACE(rtlpriv, COMP_ERR, DBG_WARNING,
 			 "Failed to download FW. Init HW without FW now..\n");
 		err = 1;
-		return err;
+		goto exit;
 	}
 
 	rtlhal->last_hmeboxnum = 0;
@@ -1032,6 +1044,8 @@ int rtl92ce_hw_init(struct ieee80211_hw *hw)
 		RT_TRACE(rtlpriv, COMP_INIT, DBG_TRACE, "under 1.5V\n");
 	}
 	rtl92c_dm_init(hw);
+exit:
+	local_irq_restore(flags);
 	rtlpci->being_init_adapter = false;
 	return err;
 }
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

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

^ permalink raw reply related

* Re: [PATCH net-next v2 3/3] Documentation: networking: update phy.txt with recent changes
From: Sergei Shtylyov @ 2014-02-17 23:32 UTC (permalink / raw)
  To: Florian Fainelli, netdev; +Cc: davem, Shaohui.Xie
In-Reply-To: <1392672844-1767-4-git-send-email-f.fainelli@gmail.com>

Hello.

On 02/18/2014 12:34 AM, Florian Fainelli wrote:

> The PHY library was missing a bunch of newly added PHY driver callbacks
> along with a smallish description of what they do, fix that.

> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
[...]

    Again too quick merge, not all types have been fixed. :-/

> diff --git a/Documentation/networking/phy.txt b/Documentation/networking/phy.txt
> index ebf2707..e602c6f 100644
> --- a/Documentation/networking/phy.txt
> +++ b/Documentation/networking/phy.txt
> @@ -253,16 +253,25 @@ Writing a PHY driver
>
>    Each driver consists of a number of function pointers:
[...]
> +   ts_info: Queries about the HW timestamping status
> +   hwtstamp: Set the PHY HW timestamping configuration
> +   rxtstamp: Requests a receive timestamp at the PHY level for a 'skb'
> +   txtsamp: Requests a transmit timestamp at the PHY level for a 'skb'

    s/txtsamp/txtstamp/.

WBR, Sergei

^ permalink raw reply

* Re: [PATCH] csum_partial_copy_from_user: clean up inconsistencies in implementations
From: Daniel Borkmann @ 2014-02-17 22:31 UTC (permalink / raw)
  To: Mikulas Patocka
  Cc: David Miller, netdev, linux-kernel, mcree, mattst88,
	mathieu.desnoyers, jay.estabrook
In-Reply-To: <alpine.LRH.2.02.1402171714030.21254@file01.intranet.prod.int.rdu2.redhat.com>

On 02/17/2014 11:20 PM, Mikulas Patocka wrote:
...
>> The proper way to add commentary is to put it after the "---" delimiter
>> at the end of the commit message and before the actual patch.
>
> Interesting - I used "---" as a delimiter between the commentary and the
> git message in the past and some people said that their patch parser can't
> detect "---" and that I should use "From:" line as a delimiter. And now I
> see that your patch parser doesn't detect "From:" and needs "---".

It's simple: Documentation/SubmittingPatches +582  says ...

The "---" marker line serves the essential purpose of marking for patch
handling tools where the changelog message ends.

One good use for the additional comments after the "---" marker is for
a diffstat, to show what files have changed, and the number of
inserted and deleted lines per file.  A diffstat is especially useful
on bigger patches.  Other comments relevant only to the moment or the
maintainer, not suitable for the permanent changelog, should also go
here.  A good example of such comments might be "patch changelogs"
which describe what has changed between the v1 and v2 version of the
patch.

So, between "---" and the diffstat you could have put your comment.
That's what the official document says in that regard. ;)

^ permalink raw reply

* Re: [PATCH] csum_partial_copy_from_user: clean up inconsistencies in implementations
From: Mikulas Patocka @ 2014-02-17 22:20 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, linux-kernel, mcree, mattst88, mathieu.desnoyers,
	jay.estabrook
In-Reply-To: <20140217.162105.899153264722449005.davem@davemloft.net>



On Mon, 17 Feb 2014, David Miller wrote:

> From: Mikulas Patocka <mpatocka@redhat.com>
> Date: Sat, 15 Feb 2014 10:49:54 -0500 (EST)
> 
> > Here I'm sending a patch for networking to clean up inconsistent 
> > implementations of csum_partial_copy_from_user in various architectures. 
> > This patch doesn't fix any bug, but the confusion in implementations 
> > caused a bug in the past. The patch should be queued for the kernel 3.15.
> > 
> > Mikulas
> 
> Please do not add commentary to the main body text of a patch submission,
> otherwise I have to edit it out.
> 
> The proper way to add commentary is to put it after the "---" delimiter
> at the end of the commit message and before the actual patch.

Interesting - I used "---" as a delimiter between the commentary and the 
git message in the past and some people said that their patch parser can't 
detect "---" and that I should use "From:" line as a delimiter. And now I 
see that your patch parser doesn't detect "From:" and needs "---".

> > From: Mikulas Patocka <mpatocka@redhat.com>
> > 
> > csum_partial_copy_from_user is called only from csum_and_copy_from_user in
> > include/net/checksum.h. csum_and_copy_from_user verifies the userspace
> > range with access_ok, so there is no need to repeat access_ok in the
> > implementation of csum_partial_copy_from_user.

...

> > The purpose of this patch is to make all the implementations of
> > csum_partial_copy_from_user consistent, so that people will keep them
> > consistent in the future.
> > 
> > Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
> 
> That is mainly a cleanup and entirely independent of fixing the Alpha
> bug.
> 
> You should submit the Alpha bug fix through the usual arch channels
> and get that into Linus's tree.
> 
> Then, after that fix propagates, you can do the access_ok() global
> cleanup as a separate patch targetting net-next.
> 
> Thanks.

I already pushed the alpha bug fix to the upstream kernel and affected 
stable kernels.

This patch is a cleanup that removes code bloat, intended for the next 
kernel cycle.

Mikulas

^ permalink raw reply

* Re: [PATCH net-next v5 09/10] Documentation: add Device tree bindings for Broadcom GENET
From: Florian Fainelli @ 2014-02-17 22:18 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: netdev, David Miller, devicetree@vger.kernel.org, Kevin Cernekee,
	Mark Rutland, Francois Romieu
In-Reply-To: <53029702.6010803@cogentembedded.com>

2014-02-17 15:10 GMT-08:00 Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>:
> Hello.
>
>
> On 02/14/2014 03:08 AM, Florian Fainelli wrote:
>
>> This patch adds the Device Tree bindings for the Broadcom GENET Gigabit
>> Ethernet controller. A bunch of examples are provided to illustrate the
>> versatile aspect of the hardare.
>
>
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>
> [...]
>
>    Too bad I didn't read this patch before and now it has been already
> applied.
>
> [...]
>
>
>> diff --git a/Documentation/devicetree/bindings/net/broadcom-bcmgenet.txt
>> b/Documentation/devicetree/bindings/net/broadcom-bcmgenet.txt
>> new file mode 100644
>> index 0000000..afd31f9
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/broadcom-bcmgenet.txt
>> @@ -0,0 +1,121 @@
>> +* Broadcom BCM7xxx Ethernet Controller (GENET)
>> +
>> +Required properties:
>> +- compatible: should contain one of "brcm,genet-v1", "brcm,genet-v2",
>> +  "brcm,genet-v3", "brcm,genet-v4".
>> +- reg: address and length of the register set for the device
>> +- interrupts: must be two cells, the first cell is the general purpose
>> +  interrupt line, while the second cell is the interrupt for the ring
>> +  RX and TX queues operating in ring mode
>> +- phy-mode: String, operation mode of the PHY interface. Supported values
>> are
>> +  "mii", "rgmii", "rgmii-txid", "rev-mii", "moca". Analogous to ePAPR
>> +  "phy-connection-type" values
>> +- address-cells: should be 1
>> +- size-cells: should be 1
>
>
>    The above two prop names should have # in front of them. Examples seem to
> be correct. Some words about the downstream bus wouldn't hurt too.

Absolutely, let me follow up with a patch fixing this.

>
> [...]
>
>> +Required child nodes:
>> +
>> +- mdio bus node: this node should always be present regarless of the PHY
>> +  configuration of the GENET instance
>> +
>> +MDIO bus node required properties:
>> +
>> +- compatible: should contain one of "brcm,genet-mdio-v1",
>> "brcm,genet-mdio-v2"
>> +  "brcm,genet-mdio-v3", "brcm,genet-mdio-v4", the version has to match
>> the
>> +  parent node compatible property (e.g: brcm,genet-v4 pairs with
>> +  brcm,genet-mdio-v4)
>> +- reg: address and length relative to the parent node base register
>> address
>> +- address-cells: address cell for MDIO bus addressing, should be 1
>> +- size-cells: size of the cells for MDIO bus addressing, should be 0
>
>
>    The above two props should have # in front of them as well. Examples seem
> to be correct.
>
> WBR, Sergei
>



-- 
Florian

^ permalink raw reply

* Re: [PATCH net-next v5 09/10] Documentation: add Device tree bindings for Broadcom GENET
From: Sergei Shtylyov @ 2014-02-17 23:10 UTC (permalink / raw)
  To: Florian Fainelli, netdev
  Cc: davem, devicetree, cernekee, mark.rutland, romieu
In-Reply-To: <1392336531-28875-10-git-send-email-f.fainelli@gmail.com>

Hello.

On 02/14/2014 03:08 AM, Florian Fainelli wrote:

> This patch adds the Device Tree bindings for the Broadcom GENET Gigabit
> Ethernet controller. A bunch of examples are provided to illustrate the
> versatile aspect of the hardare.

> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
[...]

    Too bad I didn't read this patch before and now it has been already applied.

[...]

> diff --git a/Documentation/devicetree/bindings/net/broadcom-bcmgenet.txt b/Documentation/devicetree/bindings/net/broadcom-bcmgenet.txt
> new file mode 100644
> index 0000000..afd31f9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/broadcom-bcmgenet.txt
> @@ -0,0 +1,121 @@
> +* Broadcom BCM7xxx Ethernet Controller (GENET)
> +
> +Required properties:
> +- compatible: should contain one of "brcm,genet-v1", "brcm,genet-v2",
> +  "brcm,genet-v3", "brcm,genet-v4".
> +- reg: address and length of the register set for the device
> +- interrupts: must be two cells, the first cell is the general purpose
> +  interrupt line, while the second cell is the interrupt for the ring
> +  RX and TX queues operating in ring mode
> +- phy-mode: String, operation mode of the PHY interface. Supported values are
> +  "mii", "rgmii", "rgmii-txid", "rev-mii", "moca". Analogous to ePAPR
> +  "phy-connection-type" values
> +- address-cells: should be 1
> +- size-cells: should be 1

    The above two prop names should have # in front of them. Examples seem to 
be correct. Some words about the downstream bus wouldn't hurt too.

[...]
> +Required child nodes:
> +
> +- mdio bus node: this node should always be present regarless of the PHY
> +  configuration of the GENET instance
> +
> +MDIO bus node required properties:
> +
> +- compatible: should contain one of "brcm,genet-mdio-v1", "brcm,genet-mdio-v2"
> +  "brcm,genet-mdio-v3", "brcm,genet-mdio-v4", the version has to match the
> +  parent node compatible property (e.g: brcm,genet-v4 pairs with
> +  brcm,genet-mdio-v4)
> +- reg: address and length relative to the parent node base register address
> +- address-cells: address cell for MDIO bus addressing, should be 1
> +- size-cells: size of the cells for MDIO bus addressing, should be 0

    The above two props should have # in front of them as well. Examples seem 
to be correct.

WBR, Sergei

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox