* [PATCH 1/2] usb: gadget: g_ether: fix frame size check
From: Ian Coolidge @ 2012-11-06 21:00 UTC (permalink / raw)
To: Oliver Neukum, Greg Kroah-Hartman, Felipe Balbi, linux-usb,
netdev
Cc: Ian Coolidge
In-Reply-To: <1352235611-13066-1-git-send-email-iancoolidge@gmail.com>
Checking skb->len against ETH_FRAME_LEN assumes a 1514
ethernet frame size. With an 802.1Q VLAN header, ethernet
frame length can now be 1518. Validate frame length against that.
Signed-off-by: Ian Coolidge <iancoolidge@gmail.com>
---
drivers/usb/gadget/u_ether.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/usb/gadget/u_ether.c b/drivers/usb/gadget/u_ether.c
index 29c854b..ff2c70b 100644
--- a/drivers/usb/gadget/u_ether.c
+++ b/drivers/usb/gadget/u_ether.c
@@ -19,6 +19,7 @@
#include <linux/ctype.h>
#include <linux/etherdevice.h>
#include <linux/ethtool.h>
+#include <linux/if_vlan.h>
#include "u_ether.h"
@@ -301,7 +302,7 @@ static void rx_complete(struct usb_ep *ep, struct usb_request *req)
while (skb2) {
if (status < 0
|| ETH_HLEN > skb2->len
- || skb2->len > ETH_FRAME_LEN) {
+ || skb2->len > VLAN_ETH_FRAME_LEN) {
dev->net->stats.rx_errors++;
dev->net->stats.rx_length_errors++;
DBG(dev, "rx length %d\n", skb2->len);
--
1.7.6.5
^ permalink raw reply related
* [PATCH 0/2] Fix CDC_EEM with 802.1Q VLAN and 1500 MTU
From: Ian Coolidge @ 2012-11-06 21:00 UTC (permalink / raw)
To: Oliver Neukum, Greg Kroah-Hartman, Felipe Balbi, linux-usb,
netdev
Cc: Ian Coolidge
cdc_eem USB host driver and gadget driver both are broken in 1500 MTU
case when using 802.1Q VLANs. In both cases, this is due to incomplete
assumptions of Ethernet frame size.
Ian Coolidge (2):
usb: gadget: g_ether: fix frame size check
net: usb: cdc_eem: Fix rx skb allocation for 802.1Q VLANs
drivers/net/usb/cdc_eem.c | 4 ++++
drivers/usb/gadget/u_ether.c | 3 ++-
2 files changed, 6 insertions(+), 1 deletions(-)
--
1.7.6.5
^ permalink raw reply
* Re: [PATCH net-next] bonding: convert delay parameters to unsigned integers
From: Nikolay Aleksandrov @ 2012-11-06 19:43 UTC (permalink / raw)
To: netdev; +Cc: fubar, andy, davem
In-Reply-To: <50996776.7030205@redhat.com>
On 06/11/12 20:39, Nikolay Aleksandrov wrote:
> On 06/11/12 20:03, Jay Vosburgh wrote:
>> Nikolay Aleksandrov<nikolay@redhat.com> wrote:
>>
>>> This patch converts the following parameters from int to unsigned
>>> int in order
>>> to remove the unnecessary< 0 checks in the code and make it less
>>> error prone.
>>> The functionality remains unchanged as before it wasn't allowed to
>>> have these
>>> be less than zero too. Their maximum value was INT_MAX, now it will
>>> be UINT_MAX.
>>> It also fixes how min_links parameter is treated since it is
>>> unsigned int
>>> already but was treated as signed integer.
>> Is this patch fixing any actual bugs, and if so, what?
> I don't claim that it fixes any bugs, it is a cosmetic change that
> allows to remove
> < 0 checks in the code without affecting functionality. The only "fix"
> there is, is
> how min_links is treated when changed (everywhere it was used as int
> but it
> is in fact unsigned int), although this didn't present any problems it
> was more
> of an inconsistency.
>>> struct bond_params members:
>>> updelay downdelay miimon arp_interval
>>>
>>> The delay variable is initialized from updelay and downdelay
>>> parameters and
>>> counted down to zero so it should never be< 0.
>>>
>>> struct slave members:
>>> delay
>>>
>>> The change to struct ifbond's miimon parameter (s32 -> u32) will
>>> probably
>>> affect some user-space software but it should be all right in 99 %
>>> of the
>>> cases as I don't think anyone uses> INT_MAX for that.
>> The struct ifbond is used to pass information from the kernel to
>> user space in response to an SIOCBONDINFOQUERY ioctl. It is not used to
>> set values within the kernel. In any event, I don't believe it is
>> acceptable to modify a user-visible structure like this, even to change
>> types from signed to unsigned of the same size.
> I should've explained more about the "limited" part. This is exactly
> what I meant,
> if this variable type can/should be changed. But I don't know who
> would use this
> at such high values (it loses any meaning IMO). If it is a problem I
> will re-post this
> patch without this change, I did it only to be consistent with the
> miimon type, but
> then upon returning the ifbond miimon variable might have wrong value.
>>> Limited testing of this patch was done in VMs.
>> If this is not actually fixing any bugs (i.e., is a cosmetic
>> change only), then I think it's reasonable to expect the changes to be
>> fully tested by you to insure there are no regressions introduced.
>>
>> In particular, if a negative number is passed in after your
>> patch is applied, is it accepted or rejected, and if it is accepted,
>> what is the value? For example, if the user specifies "miimon=-1" does
>> that fail, or is miimon set to 4294967295?
>>
>> -J
> The unsigned int changes have been tested and to answer your question,
> when you define a module parameter to be of a certain type, that type is
> enforced by the way kernel_param_ops are defined for that type. In this
> case if you specify a negative value the parameter won't be set and if
> it is
> on the command line (and not through sysfs) the module won't get loaded
> as before.
Sorry, not as before, before it was set to a default value if < 0, now
it won't get loaded
due to the wrong parameter. So this actually changes the behaviour, if such
change is wrong, then ignore this patch.
Nik
> (Information based on tests, kernel/params.c & kernel/module.c)
>> --- -Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
> Best regards,
> Nikolay Aleksandrov
>
> --
> 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] bonding: convert delay parameters to unsigned integers
From: Nikolay Aleksandrov @ 2012-11-06 19:39 UTC (permalink / raw)
To: netdev; +Cc: fubar, andy, davem
In-Reply-To: <5112.1352228613@death.nxdomain>
On 06/11/12 20:03, Jay Vosburgh wrote:
> Nikolay Aleksandrov<nikolay@redhat.com> wrote:
>
>> This patch converts the following parameters from int to unsigned int in order
>> to remove the unnecessary< 0 checks in the code and make it less error prone.
>> The functionality remains unchanged as before it wasn't allowed to have these
>> be less than zero too. Their maximum value was INT_MAX, now it will be UINT_MAX.
>> It also fixes how min_links parameter is treated since it is unsigned int
>> already but was treated as signed integer.
> Is this patch fixing any actual bugs, and if so, what?
I don't claim that it fixes any bugs, it is a cosmetic change that
allows to remove
< 0 checks in the code without affecting functionality. The only "fix"
there is, is
how min_links is treated when changed (everywhere it was used as int but it
is in fact unsigned int), although this didn't present any problems it
was more
of an inconsistency.
>> struct bond_params members:
>> updelay downdelay miimon arp_interval
>>
>> The delay variable is initialized from updelay and downdelay parameters and
>> counted down to zero so it should never be< 0.
>>
>> struct slave members:
>> delay
>>
>> The change to struct ifbond's miimon parameter (s32 -> u32) will probably
>> affect some user-space software but it should be all right in 99 % of the
>> cases as I don't think anyone uses> INT_MAX for that.
> The struct ifbond is used to pass information from the kernel to
> user space in response to an SIOCBONDINFOQUERY ioctl. It is not used to
> set values within the kernel. In any event, I don't believe it is
> acceptable to modify a user-visible structure like this, even to change
> types from signed to unsigned of the same size.
I should've explained more about the "limited" part. This is exactly
what I meant,
if this variable type can/should be changed. But I don't know who would
use this
at such high values (it loses any meaning IMO). If it is a problem I
will re-post this
patch without this change, I did it only to be consistent with the
miimon type, but
then upon returning the ifbond miimon variable might have wrong value.
>> Limited testing of this patch was done in VMs.
> If this is not actually fixing any bugs (i.e., is a cosmetic
> change only), then I think it's reasonable to expect the changes to be
> fully tested by you to insure there are no regressions introduced.
>
> In particular, if a negative number is passed in after your
> patch is applied, is it accepted or rejected, and if it is accepted,
> what is the value? For example, if the user specifies "miimon=-1" does
> that fail, or is miimon set to 4294967295?
>
> -J
The unsigned int changes have been tested and to answer your question,
when you define a module parameter to be of a certain type, that type is
enforced by the way kernel_param_ops are defined for that type. In this
case if you specify a negative value the parameter won't be set and if it is
on the command line (and not through sysfs) the module won't get loaded
as before.
(Information based on tests, kernel/params.c & kernel/module.c)
> --- -Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
Best regards,
Nikolay Aleksandrov
^ permalink raw reply
* Re: [PATCH net-next] bonding: convert delay parameters to unsigned integers
From: Jay Vosburgh @ 2012-11-06 19:03 UTC (permalink / raw)
To: Nikolay Aleksandrov; +Cc: netdev, andy, davem
In-Reply-To: <1352202733-27906-1-git-send-email-nikolay@redhat.com>
Nikolay Aleksandrov <nikolay@redhat.com> wrote:
>This patch converts the following parameters from int to unsigned int in order
>to remove the unnecessary < 0 checks in the code and make it less error prone.
>The functionality remains unchanged as before it wasn't allowed to have these
>be less than zero too. Their maximum value was INT_MAX, now it will be UINT_MAX.
>It also fixes how min_links parameter is treated since it is unsigned int
>already but was treated as signed integer.
Is this patch fixing any actual bugs, and if so, what?
>struct bond_params members:
>updelay downdelay miimon arp_interval
>
>The delay variable is initialized from updelay and downdelay parameters and
>counted down to zero so it should never be < 0.
>
>struct slave members:
>delay
>
>The change to struct ifbond's miimon parameter (s32 -> u32) will probably
>affect some user-space software but it should be all right in 99 % of the
>cases as I don't think anyone uses > INT_MAX for that.
The struct ifbond is used to pass information from the kernel to
user space in response to an SIOCBONDINFOQUERY ioctl. It is not used to
set values within the kernel. In any event, I don't believe it is
acceptable to modify a user-visible structure like this, even to change
types from signed to unsigned of the same size.
>Limited testing of this patch was done in VMs.
If this is not actually fixing any bugs (i.e., is a cosmetic
change only), then I think it's reasonable to expect the changes to be
fully tested by you to insure there are no regressions introduced.
In particular, if a negative number is passed in after your
patch is applied, is it accepted or rejected, and if it is accepted,
what is the value? For example, if the user specifies "miimon=-1" does
that fail, or is miimon set to 4294967295?
-J
>Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
>---
> drivers/net/bonding/bond_main.c | 92 +++++++--------------
> drivers/net/bonding/bond_procfs.c | 10 +--
> drivers/net/bonding/bond_sysfs.c | 169 ++++++++++++++++----------------------
> drivers/net/bonding/bonding.h | 10 +--
> include/uapi/linux/if_bonding.h | 2 +-
> 5 files changed, 114 insertions(+), 169 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index b2530b0..a6983e2 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -87,21 +87,21 @@
> #define BOND_LINK_MON_INTERV 0
> #define BOND_LINK_ARP_INTERV 0
>
>-static int max_bonds = BOND_DEFAULT_MAX_BONDS;
>+static unsigned int max_bonds = BOND_DEFAULT_MAX_BONDS;
> static int tx_queues = BOND_DEFAULT_TX_QUEUES;
> static int num_peer_notif = 1;
>-static int miimon = BOND_LINK_MON_INTERV;
>-static int updelay;
>-static int downdelay;
>+static unsigned int miimon = BOND_LINK_MON_INTERV;
>+static unsigned int updelay;
>+static unsigned int downdelay;
> static int use_carrier = 1;
> static char *mode;
> static char *primary;
> static char *primary_reselect;
> static char *lacp_rate;
>-static int min_links;
>+static unsigned int min_links;
> static char *ad_select;
> static char *xmit_hash_policy;
>-static int arp_interval = BOND_LINK_ARP_INTERV;
>+static unsigned int arp_interval = BOND_LINK_ARP_INTERV;
> static char *arp_ip_target[BOND_MAX_ARP_TARGETS];
> static char *arp_validate;
> static char *fail_over_mac;
>@@ -109,7 +109,7 @@ static int all_slaves_active = 0;
> static struct bond_params bonding_defaults;
> static int resend_igmp = BOND_DEFAULT_RESEND_IGMP;
>
>-module_param(max_bonds, int, 0);
>+module_param(max_bonds, uint, 0);
> MODULE_PARM_DESC(max_bonds, "Max number of bonded devices");
> module_param(tx_queues, int, 0);
> MODULE_PARM_DESC(tx_queues, "Max number of transmit queues (default = 16)");
>@@ -119,11 +119,11 @@ MODULE_PARM_DESC(num_grat_arp, "Number of peer notifications to send on "
> module_param_named(num_unsol_na, num_peer_notif, int, 0644);
> MODULE_PARM_DESC(num_unsol_na, "Number of peer notifications to send on "
> "failover event (alias of num_grat_arp)");
>-module_param(miimon, int, 0);
>+module_param(miimon, uint, 0);
> MODULE_PARM_DESC(miimon, "Link check interval in milliseconds");
>-module_param(updelay, int, 0);
>+module_param(updelay, uint, 0);
> MODULE_PARM_DESC(updelay, "Delay before considering link up, in milliseconds");
>-module_param(downdelay, int, 0);
>+module_param(downdelay, uint, 0);
> MODULE_PARM_DESC(downdelay, "Delay before considering link down, "
> "in milliseconds");
> module_param(use_carrier, int, 0);
>@@ -151,14 +151,14 @@ module_param(ad_select, charp, 0);
> MODULE_PARM_DESC(ad_select, "803.ad aggregation selection logic; "
> "0 for stable (default), 1 for bandwidth, "
> "2 for count");
>-module_param(min_links, int, 0);
>+module_param(min_links, uint, 0);
> MODULE_PARM_DESC(min_links, "Minimum number of available links before turning on carrier");
>
> module_param(xmit_hash_policy, charp, 0);
> MODULE_PARM_DESC(xmit_hash_policy, "balance-xor and 802.3ad hashing method; "
> "0 for layer 2 (default), 1 for layer 3+4, "
> "2 for layer 2+3");
>-module_param(arp_interval, int, 0);
>+module_param(arp_interval, uint, 0);
> MODULE_PARM_DESC(arp_interval, "arp interval in milliseconds");
> module_param_array(arp_ip_target, charp, NULL, 0);
> MODULE_PARM_DESC(arp_ip_target, "arp targets in n.n.n.n form");
>@@ -983,7 +983,7 @@ static struct slave *bond_find_best_slave(struct bonding *bond)
> {
> struct slave *new_active, *old_active;
> struct slave *bestslave = NULL;
>- int mintime = bond->params.updelay;
>+ unsigned int mintime = bond->params.updelay;
> int i;
>
> new_active = bond->curr_active_slave;
>@@ -1063,7 +1063,7 @@ void bond_change_active_slave(struct bonding *bond, struct slave *new_active)
>
> if (new_active->link == BOND_LINK_BACK) {
> if (USES_PRIMARY(bond->params.mode)) {
>- pr_info("%s: making interface %s the new active one %d ms earlier.\n",
>+ pr_info("%s: making interface %s the new active one %u ms earlier.\n",
> bond->dev->name, new_active->dev->name,
> (bond->params.updelay - new_active->delay) * bond->params.miimon);
> }
>@@ -2356,7 +2356,7 @@ static int bond_miimon_inspect(struct bonding *bond)
> slave->link = BOND_LINK_FAIL;
> slave->delay = bond->params.downdelay;
> if (slave->delay) {
>- pr_info("%s: link status down for %sinterface %s, disabling it in %d ms.\n",
>+ pr_info("%s: link status down for %sinterface %s, disabling it in %u ms.\n",
> bond->dev->name,
> (bond->params.mode ==
> BOND_MODE_ACTIVEBACKUP) ?
>@@ -2373,7 +2373,7 @@ static int bond_miimon_inspect(struct bonding *bond)
> */
> slave->link = BOND_LINK_UP;
> slave->jiffies = jiffies;
>- pr_info("%s: link status up again after %d ms for interface %s.\n",
>+ pr_info("%s: link status up again after %u ms for interface %s.\n",
> bond->dev->name,
> (bond->params.downdelay - slave->delay) *
> bond->params.miimon,
>@@ -2381,7 +2381,7 @@ static int bond_miimon_inspect(struct bonding *bond)
> continue;
> }
>
>- if (slave->delay <= 0) {
>+ if (slave->delay == 0) {
> slave->new_link = BOND_LINK_DOWN;
> commit++;
> continue;
>@@ -2398,7 +2398,7 @@ static int bond_miimon_inspect(struct bonding *bond)
> slave->delay = bond->params.updelay;
>
> if (slave->delay) {
>- pr_info("%s: link status up for interface %s, enabling it in %d ms.\n",
>+ pr_info("%s: link status up for interface %s, enabling it in %u ms.\n",
> bond->dev->name, slave->dev->name,
> ignore_updelay ? 0 :
> bond->params.updelay *
>@@ -2408,7 +2408,7 @@ static int bond_miimon_inspect(struct bonding *bond)
> case BOND_LINK_BACK:
> if (!link_state) {
> slave->link = BOND_LINK_DOWN;
>- pr_info("%s: link status down again after %d ms for interface %s.\n",
>+ pr_info("%s: link status down again after %u ms for interface %s.\n",
> bond->dev->name,
> (bond->params.updelay - slave->delay) *
> bond->params.miimon,
>@@ -2420,7 +2420,7 @@ static int bond_miimon_inspect(struct bonding *bond)
> if (ignore_updelay)
> slave->delay = 0;
>
>- if (slave->delay <= 0) {
>+ if (slave->delay == 0) {
> slave->new_link = BOND_LINK_UP;
> commit++;
> ignore_updelay = false;
>@@ -2811,7 +2811,7 @@ void bond_loadbalance_arp_mon(struct work_struct *work)
> arp_work.work);
> struct slave *slave, *oldcurrent;
> int do_failover = 0;
>- int delta_in_ticks, extra_ticks;
>+ unsigned int delta_in_ticks, extra_ticks;
> int i;
>
> read_lock(&bond->lock);
>@@ -3020,7 +3020,7 @@ static int bond_ab_arp_inspect(struct bonding *bond, int delta_in_ticks)
> *
> * Called with RTNL and bond->lock for read.
> */
>-static void bond_ab_arp_commit(struct bonding *bond, int delta_in_ticks)
>+static void bond_ab_arp_commit(struct bonding *bond, unsigned int delta_in_ticks)
> {
> struct slave *slave;
> int i;
>@@ -3166,7 +3166,7 @@ void bond_activebackup_arp_mon(struct work_struct *work)
> struct bonding *bond = container_of(work, struct bonding,
> arp_work.work);
> bool should_notify_peers = false;
>- int delta_in_ticks;
>+ unsigned int delta_in_ticks;
>
> read_lock(&bond->lock);
>
>@@ -4574,30 +4574,6 @@ static int bond_check_params(struct bond_params *params)
> params->ad_select = BOND_AD_STABLE;
> }
>
>- if (max_bonds < 0) {
>- pr_warning("Warning: max_bonds (%d) not in range %d-%d, so it was reset to BOND_DEFAULT_MAX_BONDS (%d)\n",
>- max_bonds, 0, INT_MAX, BOND_DEFAULT_MAX_BONDS);
>- max_bonds = BOND_DEFAULT_MAX_BONDS;
>- }
>-
>- if (miimon < 0) {
>- pr_warning("Warning: miimon module parameter (%d), not in range 0-%d, so it was reset to %d\n",
>- miimon, INT_MAX, BOND_LINK_MON_INTERV);
>- miimon = BOND_LINK_MON_INTERV;
>- }
>-
>- if (updelay < 0) {
>- pr_warning("Warning: updelay module parameter (%d), not in range 0-%d, so it was reset to 0\n",
>- updelay, INT_MAX);
>- updelay = 0;
>- }
>-
>- if (downdelay < 0) {
>- pr_warning("Warning: downdelay module parameter (%d), not in range 0-%d, so it was reset to 0\n",
>- downdelay, INT_MAX);
>- downdelay = 0;
>- }
>-
> if ((use_carrier != 0) && (use_carrier != 1)) {
> pr_warning("Warning: use_carrier module parameter (%d), not of valid value (0/1), so it was set to 1\n",
> use_carrier);
>@@ -4651,7 +4627,7 @@ static int bond_check_params(struct bond_params *params)
> }
>
> if (bond_mode == BOND_MODE_ALB) {
>- pr_notice("In ALB mode you might experience client disconnections upon reconnection of a link if the bonding module updelay parameter (%d msec) is incompatible with the forwarding delay time of the switch\n",
>+ pr_notice("In ALB mode you might experience client disconnections upon reconnection of a link if the bonding module updelay parameter (%u msec) is incompatible with the forwarding delay time of the switch\n",
> updelay);
> }
>
>@@ -4660,19 +4636,19 @@ static int bond_check_params(struct bond_params *params)
> /* just warn the user the up/down delay will have
> * no effect since miimon is zero...
> */
>- pr_warning("Warning: miimon module parameter not set and updelay (%d) or downdelay (%d) module parameter is set; updelay and downdelay have no effect unless miimon is set\n",
>+ pr_warning("Warning: miimon module parameter not set and updelay (%u) or downdelay (%u) module parameter is set; updelay and downdelay have no effect unless miimon is set\n",
> updelay, downdelay);
> }
> } else {
> /* don't allow arp monitoring */
> if (arp_interval) {
>- pr_warning("Warning: miimon (%d) and arp_interval (%d) can't be used simultaneously, disabling ARP monitoring\n",
>+ pr_warning("Warning: miimon (%u) and arp_interval (%u) can't be used simultaneously, disabling ARP monitoring\n",
> miimon, arp_interval);
> arp_interval = 0;
> }
>
> if ((updelay % miimon) != 0) {
>- pr_warning("Warning: updelay (%d) is not a multiple of miimon (%d), updelay rounded to %d ms\n",
>+ pr_warning("Warning: updelay (%u) is not a multiple of miimon (%u), updelay rounded to %u ms\n",
> updelay, miimon,
> (updelay / miimon) * miimon);
> }
>@@ -4680,7 +4656,7 @@ static int bond_check_params(struct bond_params *params)
> updelay /= miimon;
>
> if ((downdelay % miimon) != 0) {
>- pr_warning("Warning: downdelay (%d) is not a multiple of miimon (%d), downdelay rounded to %d ms\n",
>+ pr_warning("Warning: downdelay (%u) is not a multiple of miimon (%u), downdelay rounded to %u ms\n",
> downdelay, miimon,
> (downdelay / miimon) * miimon);
> }
>@@ -4688,12 +4664,6 @@ static int bond_check_params(struct bond_params *params)
> downdelay /= miimon;
> }
>
>- if (arp_interval < 0) {
>- pr_warning("Warning: arp_interval module parameter (%d) , not in range 0-%d, so it was reset to %d\n",
>- arp_interval, INT_MAX, BOND_LINK_ARP_INTERV);
>- arp_interval = BOND_LINK_ARP_INTERV;
>- }
>-
> for (arp_ip_count = 0;
> (arp_ip_count < BOND_MAX_ARP_TARGETS) && arp_ip_target[arp_ip_count];
> arp_ip_count++) {
>@@ -4711,7 +4681,7 @@ static int bond_check_params(struct bond_params *params)
>
> if (arp_interval && !arp_ip_count) {
> /* don't allow arping if no arp_ip_target given... */
>- pr_warning("Warning: arp_interval module parameter (%d) specified without providing an arp_ip_target parameter, arp_interval was reset to 0\n",
>+ pr_warning("Warning: arp_interval module parameter (%u) specified without providing an arp_ip_target parameter, arp_interval was reset to 0\n",
> arp_interval);
> arp_interval = 0;
> }
>@@ -4737,11 +4707,11 @@ static int bond_check_params(struct bond_params *params)
> arp_validate_value = 0;
>
> if (miimon) {
>- pr_info("MII link monitoring set to %d ms\n", miimon);
>+ pr_info("MII link monitoring set to %u ms\n", miimon);
> } else if (arp_interval) {
> int i;
>
>- pr_info("ARP monitoring set to %d ms, validate %s, with %d target(s):",
>+ pr_info("ARP monitoring set to %u ms, validate %s, with %d target(s):",
> arp_interval,
> arp_validate_tbl[arp_validate_value].modename,
> arp_ip_count);
>diff --git a/drivers/net/bonding/bond_procfs.c b/drivers/net/bonding/bond_procfs.c
>index 3cea38d..0b4c278 100644
>--- a/drivers/net/bonding/bond_procfs.c
>+++ b/drivers/net/bonding/bond_procfs.c
>@@ -94,17 +94,17 @@ static void bond_info_show_master(struct seq_file *seq)
>
> seq_printf(seq, "MII Status: %s\n", netif_carrier_ok(bond->dev) ?
> "up" : "down");
>- seq_printf(seq, "MII Polling Interval (ms): %d\n", bond->params.miimon);
>- seq_printf(seq, "Up Delay (ms): %d\n",
>+ seq_printf(seq, "MII Polling Interval (ms): %u\n", bond->params.miimon);
>+ seq_printf(seq, "Up Delay (ms): %u\n",
> bond->params.updelay * bond->params.miimon);
>- seq_printf(seq, "Down Delay (ms): %d\n",
>+ seq_printf(seq, "Down Delay (ms): %u\n",
> bond->params.downdelay * bond->params.miimon);
>
>
> /* ARP information */
> if (bond->params.arp_interval > 0) {
> int printed = 0;
>- seq_printf(seq, "ARP Polling Interval (ms): %d\n",
>+ seq_printf(seq, "ARP Polling Interval (ms): %u\n",
> bond->params.arp_interval);
>
> seq_printf(seq, "ARP IP target/s (n.n.n.n form):");
>@@ -126,7 +126,7 @@ static void bond_info_show_master(struct seq_file *seq)
> seq_puts(seq, "\n802.3ad info\n");
> seq_printf(seq, "LACP rate: %s\n",
> (bond->params.lacp_fast) ? "fast" : "slow");
>- seq_printf(seq, "Min links: %d\n", bond->params.min_links);
>+ seq_printf(seq, "Min links: %u\n", bond->params.min_links);
> seq_printf(seq, "Aggregator selection policy (ad_select): %s\n",
> ad_select_tbl[bond->params.ad_select].modename);
>
>diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
>index ef8d2a0..b9a2131 100644
>--- a/drivers/net/bonding/bond_sysfs.c
>+++ b/drivers/net/bonding/bond_sysfs.c
>@@ -503,28 +503,23 @@ static ssize_t bonding_show_arp_interval(struct device *d,
> {
> struct bonding *bond = to_bond(d);
>
>- return sprintf(buf, "%d\n", bond->params.arp_interval);
>+ return sprintf(buf, "%u\n", bond->params.arp_interval);
> }
>
> static ssize_t bonding_store_arp_interval(struct device *d,
> struct device_attribute *attr,
> const char *buf, size_t count)
> {
>- int new_value, ret = count;
>+ int ret = count;
>+ unsigned int new_value;
> struct bonding *bond = to_bond(d);
>
>- if (sscanf(buf, "%d", &new_value) != 1) {
>+ if (sscanf(buf, "%u", &new_value) != 1) {
> pr_err("%s: no arp_interval value specified.\n",
> bond->dev->name);
> ret = -EINVAL;
> goto out;
> }
>- if (new_value < 0) {
>- pr_err("%s: Invalid arp_interval value %d not in range 1-%d; rejected.\n",
>- bond->dev->name, new_value, INT_MAX);
>- ret = -EINVAL;
>- goto out;
>- }
> if (bond->params.mode == BOND_MODE_ALB ||
> bond->params.mode == BOND_MODE_TLB) {
> pr_info("%s: ARP monitoring cannot be used with ALB/TLB. Only MII monitoring is supported on %s.\n",
>@@ -532,7 +527,7 @@ static ssize_t bonding_store_arp_interval(struct device *d,
> ret = -EINVAL;
> goto out;
> }
>- pr_info("%s: Setting ARP monitoring interval to %d.\n",
>+ pr_info("%s: Setting ARP monitoring interval to %u.\n",
> bond->dev->name, new_value);
> bond->params.arp_interval = new_value;
> if (bond->params.miimon) {
>@@ -682,14 +677,15 @@ static ssize_t bonding_show_downdelay(struct device *d,
> {
> struct bonding *bond = to_bond(d);
>
>- return sprintf(buf, "%d\n", bond->params.downdelay * bond->params.miimon);
>+ return sprintf(buf, "%u\n", bond->params.downdelay * bond->params.miimon);
> }
>
> static ssize_t bonding_store_downdelay(struct device *d,
> struct device_attribute *attr,
> const char *buf, size_t count)
> {
>- int new_value, ret = count;
>+ int ret = count;
>+ unsigned int new_value;
> struct bonding *bond = to_bond(d);
>
> if (!(bond->params.miimon)) {
>@@ -699,31 +695,22 @@ static ssize_t bonding_store_downdelay(struct device *d,
> goto out;
> }
>
>- if (sscanf(buf, "%d", &new_value) != 1) {
>+ if (sscanf(buf, "%u", &new_value) != 1) {
> pr_err("%s: no down delay value specified.\n", bond->dev->name);
> ret = -EINVAL;
> goto out;
> }
>- if (new_value < 0) {
>- pr_err("%s: Invalid down delay value %d not in range %d-%d; rejected.\n",
>- bond->dev->name, new_value, 1, INT_MAX);
>- ret = -EINVAL;
>- goto out;
>- } else {
>- if ((new_value % bond->params.miimon) != 0) {
>- pr_warning("%s: Warning: down delay (%d) is not a multiple of miimon (%d), delay rounded to %d ms\n",
>- bond->dev->name, new_value,
>- bond->params.miimon,
>- (new_value / bond->params.miimon) *
>- bond->params.miimon);
>- }
>- bond->params.downdelay = new_value / bond->params.miimon;
>- pr_info("%s: Setting down delay to %d.\n",
>- bond->dev->name,
>- bond->params.downdelay * bond->params.miimon);
>-
>+ if ((new_value % bond->params.miimon) != 0) {
>+ pr_warning("%s: Warning: down delay (%u) is not a multiple of miimon (%u), delay rounded to %u ms\n",
>+ bond->dev->name, new_value,
>+ bond->params.miimon,
>+ (new_value / bond->params.miimon) *
>+ bond->params.miimon);
> }
>-
>+ bond->params.downdelay = new_value / bond->params.miimon;
>+ pr_info("%s: Setting down delay to %u.\n",
>+ bond->dev->name,
>+ bond->params.downdelay * bond->params.miimon);
> out:
> return ret;
> }
>@@ -736,7 +723,7 @@ static ssize_t bonding_show_updelay(struct device *d,
> {
> struct bonding *bond = to_bond(d);
>
>- return sprintf(buf, "%d\n", bond->params.updelay * bond->params.miimon);
>+ return sprintf(buf, "%u\n", bond->params.updelay * bond->params.miimon);
>
> }
>
>@@ -744,7 +731,8 @@ static ssize_t bonding_store_updelay(struct device *d,
> struct device_attribute *attr,
> const char *buf, size_t count)
> {
>- int new_value, ret = count;
>+ int ret = count;
>+ unsigned int new_value;
> struct bonding *bond = to_bond(d);
>
> if (!(bond->params.miimon)) {
>@@ -754,30 +742,23 @@ static ssize_t bonding_store_updelay(struct device *d,
> goto out;
> }
>
>- if (sscanf(buf, "%d", &new_value) != 1) {
>+ if (sscanf(buf, "%u", &new_value) != 1) {
> pr_err("%s: no up delay value specified.\n",
> bond->dev->name);
> ret = -EINVAL;
> goto out;
> }
>- if (new_value < 0) {
>- pr_err("%s: Invalid down delay value %d not in range %d-%d; rejected.\n",
>- bond->dev->name, new_value, 1, INT_MAX);
>- ret = -EINVAL;
>- goto out;
>- } else {
>- if ((new_value % bond->params.miimon) != 0) {
>- pr_warning("%s: Warning: up delay (%d) is not a multiple of miimon (%d), updelay rounded to %d ms\n",
>- bond->dev->name, new_value,
>- bond->params.miimon,
>- (new_value / bond->params.miimon) *
>- bond->params.miimon);
>- }
>- bond->params.updelay = new_value / bond->params.miimon;
>- pr_info("%s: Setting up delay to %d.\n",
>- bond->dev->name,
>- bond->params.updelay * bond->params.miimon);
>+ if ((new_value % bond->params.miimon) != 0) {
>+ pr_warning("%s: Warning: up delay (%u) is not a multiple of miimon (%u), updelay rounded to %u ms\n",
>+ bond->dev->name, new_value,
>+ bond->params.miimon,
>+ (new_value / bond->params.miimon) *
>+ bond->params.miimon);
> }
>+ bond->params.updelay = new_value / bond->params.miimon;
>+ pr_info("%s: Setting up delay to %u.\n",
>+ bond->dev->name,
>+ bond->params.updelay * bond->params.miimon);
>
> out:
> return ret;
>@@ -846,7 +827,7 @@ static ssize_t bonding_show_min_links(struct device *d,
> {
> struct bonding *bond = to_bond(d);
>
>- return sprintf(buf, "%d\n", bond->params.min_links);
>+ return sprintf(buf, "%u\n", bond->params.min_links);
> }
>
> static ssize_t bonding_store_min_links(struct device *d,
>@@ -952,65 +933,59 @@ static ssize_t bonding_show_miimon(struct device *d,
> {
> struct bonding *bond = to_bond(d);
>
>- return sprintf(buf, "%d\n", bond->params.miimon);
>+ return sprintf(buf, "%u\n", bond->params.miimon);
> }
>
> static ssize_t bonding_store_miimon(struct device *d,
> struct device_attribute *attr,
> const char *buf, size_t count)
> {
>- int new_value, ret = count;
>+ int ret = count;
>+ unsigned int new_value;
> struct bonding *bond = to_bond(d);
>
>- if (sscanf(buf, "%d", &new_value) != 1) {
>+ if (sscanf(buf, "%u", &new_value) != 1) {
> pr_err("%s: no miimon value specified.\n",
> bond->dev->name);
> ret = -EINVAL;
> goto out;
> }
>- if (new_value < 0) {
>- pr_err("%s: Invalid miimon value %d not in range %d-%d; rejected.\n",
>- bond->dev->name, new_value, 1, INT_MAX);
>- ret = -EINVAL;
>- goto out;
>- } else {
>- pr_info("%s: Setting MII monitoring interval to %d.\n",
>- bond->dev->name, new_value);
>- bond->params.miimon = new_value;
>- if (bond->params.updelay)
>- pr_info("%s: Note: Updating updelay (to %d) since it is a multiple of the miimon value.\n",
>- bond->dev->name,
>- bond->params.updelay * bond->params.miimon);
>- if (bond->params.downdelay)
>- pr_info("%s: Note: Updating downdelay (to %d) since it is a multiple of the miimon value.\n",
>- bond->dev->name,
>- bond->params.downdelay * bond->params.miimon);
>- if (bond->params.arp_interval) {
>- pr_info("%s: MII monitoring cannot be used with ARP monitoring. Disabling ARP monitoring...\n",
>- bond->dev->name);
>- bond->params.arp_interval = 0;
>- if (bond->params.arp_validate) {
>- bond->params.arp_validate =
>- BOND_ARP_VALIDATE_NONE;
>- }
>- if (delayed_work_pending(&bond->arp_work)) {
>- cancel_delayed_work(&bond->arp_work);
>- flush_workqueue(bond->wq);
>- }
>+ pr_info("%s: Setting MII monitoring interval to %u.\n",
>+ bond->dev->name, new_value);
>+ bond->params.miimon = new_value;
>+ if (bond->params.updelay)
>+ pr_info("%s: Note: Updating updelay (to %u) since it is a multiple of the miimon value.\n",
>+ bond->dev->name,
>+ bond->params.updelay * bond->params.miimon);
>+ if (bond->params.downdelay)
>+ pr_info("%s: Note: Updating downdelay (to %u) since it is a multiple of the miimon value.\n",
>+ bond->dev->name,
>+ bond->params.downdelay * bond->params.miimon);
>+ if (bond->params.arp_interval) {
>+ pr_info("%s: MII monitoring cannot be used with ARP monitoring. Disabling ARP monitoring...\n",
>+ bond->dev->name);
>+ bond->params.arp_interval = 0;
>+ if (bond->params.arp_validate) {
>+ bond->params.arp_validate =
>+ BOND_ARP_VALIDATE_NONE;
> }
>+ if (delayed_work_pending(&bond->arp_work)) {
>+ cancel_delayed_work(&bond->arp_work);
>+ flush_workqueue(bond->wq);
>+ }
>+ }
>
>- if (bond->dev->flags & IFF_UP) {
>- /* If the interface is up, we may need to fire off
>- * the MII timer. If the interface is down, the
>- * timer will get fired off when the open function
>- * is called.
>- */
>- if (!delayed_work_pending(&bond->mii_work)) {
>- INIT_DELAYED_WORK(&bond->mii_work,
>- bond_mii_monitor);
>- queue_delayed_work(bond->wq,
>- &bond->mii_work, 0);
>- }
>+ if (bond->dev->flags & IFF_UP) {
>+ /* If the interface is up, we may need to fire off
>+ * the MII timer. If the interface is down, the
>+ * timer will get fired off when the open function
>+ * is called.
>+ */
>+ if (!delayed_work_pending(&bond->mii_work)) {
>+ INIT_DELAYED_WORK(&bond->mii_work,
>+ bond_mii_monitor);
>+ queue_delayed_work(bond->wq,
>+ &bond->mii_work, 0);
> }
> }
> out:
>diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
>index f8af2fc..604124c 100644
>--- a/drivers/net/bonding/bonding.h
>+++ b/drivers/net/bonding/bonding.h
>@@ -139,14 +139,14 @@ static inline int is_netpoll_tx_blocked(struct net_device *dev)
> struct bond_params {
> int mode;
> int xmit_policy;
>- int miimon;
>+ unsigned int miimon;
> u8 num_peer_notif;
>- int arp_interval;
>+ unsigned int arp_interval;
> int arp_validate;
> int use_carrier;
> int fail_over_mac;
>- int updelay;
>- int downdelay;
>+ unsigned int updelay;
>+ unsigned int downdelay;
> int lacp_fast;
> unsigned int min_links;
> int ad_select;
>@@ -175,7 +175,7 @@ struct slave {
> struct slave *next;
> struct slave *prev;
> struct bonding *bond; /* our master */
>- int delay;
>+ unsigned int delay;
> unsigned long jiffies;
> unsigned long last_arp_rx;
> s8 link; /* one of BOND_LINK_XXXX */
>diff --git a/include/uapi/linux/if_bonding.h b/include/uapi/linux/if_bonding.h
>index a17edda..9796b4a 100644
>--- a/include/uapi/linux/if_bonding.h
>+++ b/include/uapi/linux/if_bonding.h
>@@ -95,7 +95,7 @@
> typedef struct ifbond {
> __s32 bond_mode;
> __s32 num_slaves;
>- __s32 miimon;
>+ __u32 miimon;
> } ifbond;
>
> typedef struct ifslave {
>--
>1.7.11.4
>
---
-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
^ permalink raw reply
* Re: net/smsc911x: problems after commit 3ac3546e [Always wait for the chip to be ready]
From: David Miller @ 2012-11-06 17:36 UTC (permalink / raw)
To: kamlakant.patel; +Cc: steve, linus.walleij, robert.marklund, netdev
In-Reply-To: <20121106054643.GB8634@patelk@broadcom.com>
From: "Kamlakant Patel" <kamlakant.patel@broadcom.com>
Date: Tue, 6 Nov 2012 11:16:44 +0530
> The issue is that the logic of the commit 3ac3546e does not work if
> WORD_SWAP is needed. So we have to either revert that commit or add
> logic to check for swapped register if we read PMT_CTRL before
> programming WORD_SWAP.
I understand the problem.
You program the device into a state the driver is not expecting,
you have to therefore undo that before transferring control to
the kernel.
I accept no other solution, testing random reserved bits is
bogus beyond belief.
^ permalink raw reply
* [PATCH V4 4/7] cpsw: simplify the setup of the register pointers
From: Mugunthan V N @ 2012-11-06 17:32 UTC (permalink / raw)
To: netdev
Cc: davem, devicetree-discuss, linux-arm-kernel, linux-omap,
Richard Cochran, Mugunthan V N
In-Reply-To: <1352223159-27504-1-git-send-email-mugunthanvnm@ti.com>
From: Richard Cochran <richardcochran@gmail.com>
Instead of having a host of different register offsets in the device tree,
this patch simplifies the CPSW code by letting the driver set the proper
register offsets automatically, based on the CPSW version.
Signed-off-by: Richard Cochran <richardcochran@gmail.com>
Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
---
Documentation/devicetree/bindings/net/cpsw.txt | 34 ----
drivers/net/ethernet/ti/cpsw.c | 209 +++++++++--------------
include/linux/platform_data/cpsw.h | 19 --
3 files changed, 82 insertions(+), 180 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt
index 2214607..6cf5d92 100644
--- a/Documentation/devicetree/bindings/net/cpsw.txt
+++ b/Documentation/devicetree/bindings/net/cpsw.txt
@@ -9,15 +9,7 @@ Required properties:
number
- interrupt-parent : The parent interrupt controller
- cpdma_channels : Specifies number of channels in CPDMA
-- host_port_no : Specifies host port shift
-- cpdma_reg_ofs : Specifies CPDMA submodule register offset
-- cpdma_sram_ofs : Specifies CPDMA SRAM offset
-- ale_reg_ofs : Specifies ALE submodule register offset
- ale_entries : Specifies No of entries ALE can hold
-- host_port_reg_ofs : Specifies host port register offset
-- hw_stats_reg_ofs : Specifies hardware statistics register offset
-- cpts_reg_ofs : Specifies the offset of the CPTS registers
-- bd_ram_ofs : Specifies internal desciptor RAM offset
- bd_ram_size : Specifies internal descriptor RAM size
- rx_descs : Specifies number of Rx descriptors
- mac_control : Specifies Default MAC control register content
@@ -26,8 +18,6 @@ Required properties:
- cpts_active_slave : Specifies the slave to use for time stamping
- cpts_clock_mult : Numerator to convert input clock ticks into nanoseconds
- cpts_clock_shift : Denominator to convert input clock ticks into nanoseconds
-- slave_reg_ofs : Specifies slave register offset
-- sliver_reg_ofs : Specifies slave sliver register offset
- phy_id : Specifies slave phy id
- mac-address : Specifies slave MAC address
@@ -49,15 +39,7 @@ Examples:
interrupts = <55 0x4>;
interrupt-parent = <&intc>;
cpdma_channels = <8>;
- host_port_no = <0>;
- cpdma_reg_ofs = <0x800>;
- cpdma_sram_ofs = <0xa00>;
- ale_reg_ofs = <0xd00>;
ale_entries = <1024>;
- host_port_reg_ofs = <0x108>;
- hw_stats_reg_ofs = <0x900>;
- cpts_reg_ofs = <0xc00>;
- bd_ram_ofs = <0x2000>;
bd_ram_size = <0x2000>;
no_bd_ram = <0>;
rx_descs = <64>;
@@ -67,15 +49,11 @@ Examples:
cpts_clock_mult = <0x80000000>;
cpts_clock_shift = <29>;
cpsw_emac0: slave@0 {
- slave_reg_ofs = <0x200>;
- sliver_reg_ofs = <0xd80>;
phy_id = "davinci_mdio.16:00";
/* Filled in by U-Boot */
mac-address = [ 00 00 00 00 00 00 ];
};
cpsw_emac1: slave@1 {
- slave_reg_ofs = <0x300>;
- sliver_reg_ofs = <0xdc0>;
phy_id = "davinci_mdio.16:01";
/* Filled in by U-Boot */
mac-address = [ 00 00 00 00 00 00 ];
@@ -87,15 +65,7 @@ Examples:
compatible = "ti,cpsw";
ti,hwmods = "cpgmac0";
cpdma_channels = <8>;
- host_port_no = <0>;
- cpdma_reg_ofs = <0x800>;
- cpdma_sram_ofs = <0xa00>;
- ale_reg_ofs = <0xd00>;
ale_entries = <1024>;
- host_port_reg_ofs = <0x108>;
- hw_stats_reg_ofs = <0x900>;
- cpts_reg_ofs = <0xc00>;
- bd_ram_ofs = <0x2000>;
bd_ram_size = <0x2000>;
no_bd_ram = <0>;
rx_descs = <64>;
@@ -105,15 +75,11 @@ Examples:
cpts_clock_mult = <0x80000000>;
cpts_clock_shift = <29>;
cpsw_emac0: slave@0 {
- slave_reg_ofs = <0x200>;
- sliver_reg_ofs = <0xd80>;
phy_id = "davinci_mdio.16:00";
/* Filled in by U-Boot */
mac-address = [ 00 00 00 00 00 00 ];
};
cpsw_emac1: slave@1 {
- slave_reg_ofs = <0x300>;
- sliver_reg_ofs = <0xdc0>;
phy_id = "davinci_mdio.16:01";
/* Filled in by U-Boot */
mac-address = [ 00 00 00 00 00 00 ];
diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 7007aba..f94aa8f 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -80,6 +80,29 @@ do { \
#define CPSW_VERSION_1 0x19010a
#define CPSW_VERSION_2 0x19010c
+
+#define HOST_PORT_NUM 0
+#define SLIVER_SIZE 0x40
+
+#define CPSW1_HOST_PORT_OFFSET 0x028
+#define CPSW1_SLAVE_OFFSET 0x050
+#define CPSW1_SLAVE_SIZE 0x040
+#define CPSW1_CPDMA_OFFSET 0x100
+#define CPSW1_STATERAM_OFFSET 0x200
+#define CPSW1_CPTS_OFFSET 0x500
+#define CPSW1_ALE_OFFSET 0x600
+#define CPSW1_SLIVER_OFFSET 0x700
+
+#define CPSW2_HOST_PORT_OFFSET 0x108
+#define CPSW2_SLAVE_OFFSET 0x200
+#define CPSW2_SLAVE_SIZE 0x100
+#define CPSW2_CPDMA_OFFSET 0x800
+#define CPSW2_STATERAM_OFFSET 0xa00
+#define CPSW2_CPTS_OFFSET 0xc00
+#define CPSW2_ALE_OFFSET 0xd00
+#define CPSW2_SLIVER_OFFSET 0xd80
+#define CPSW2_BD_OFFSET 0x2000
+
#define CPDMA_RXTHRESH 0x0c0
#define CPDMA_RXFREE 0x0e0
#define CPDMA_TXHDP 0x00
@@ -87,21 +110,6 @@ do { \
#define CPDMA_TXCP 0x40
#define CPDMA_RXCP 0x60
-#define cpsw_dma_regs(base, offset) \
- (void __iomem *)((base) + (offset))
-#define cpsw_dma_rxthresh(base, offset) \
- (void __iomem *)((base) + (offset) + CPDMA_RXTHRESH)
-#define cpsw_dma_rxfree(base, offset) \
- (void __iomem *)((base) + (offset) + CPDMA_RXFREE)
-#define cpsw_dma_txhdp(base, offset) \
- (void __iomem *)((base) + (offset) + CPDMA_TXHDP)
-#define cpsw_dma_rxhdp(base, offset) \
- (void __iomem *)((base) + (offset) + CPDMA_RXHDP)
-#define cpsw_dma_txcp(base, offset) \
- (void __iomem *)((base) + (offset) + CPDMA_TXCP)
-#define cpsw_dma_rxcp(base, offset) \
- (void __iomem *)((base) + (offset) + CPDMA_RXCP)
-
#define CPSW_POLL_WEIGHT 64
#define CPSW_MIN_PACKET_SIZE 60
#define CPSW_MAX_PACKET_SIZE (1500 + 14 + 4 + 4)
@@ -629,8 +637,7 @@ static int cpsw_ndo_open(struct net_device *ndev)
pm_runtime_get_sync(&priv->pdev->dev);
- reg = __raw_readl(&priv->regs->id_ver);
- priv->version = reg;
+ reg = priv->version;
dev_info(priv->dev, "initializing cpsw version %d.%d (%d)\n",
CPSW_MAJOR_VERSION(reg), CPSW_MINOR_VERSION(reg),
@@ -995,15 +1002,16 @@ static const struct ethtool_ops cpsw_ethtool_ops = {
.get_ts_info = cpsw_get_ts_info,
};
-static void cpsw_slave_init(struct cpsw_slave *slave, struct cpsw_priv *priv)
+static void cpsw_slave_init(struct cpsw_slave *slave, struct cpsw_priv *priv,
+ u32 slave_reg_ofs, u32 sliver_reg_ofs)
{
void __iomem *regs = priv->regs;
int slave_num = slave->slave_num;
struct cpsw_slave_data *data = priv->data.slave_data + slave_num;
slave->data = data;
- slave->regs = regs + data->slave_reg_ofs;
- slave->sliver = regs + data->sliver_reg_ofs;
+ slave->regs = regs + slave_reg_ofs;
+ slave->sliver = regs + sliver_reg_ofs;
}
static int cpsw_probe_dt(struct cpsw_platform_data *data,
@@ -1051,8 +1059,6 @@ static int cpsw_probe_dt(struct cpsw_platform_data *data,
return -EINVAL;
}
- data->no_bd_ram = of_property_read_bool(node, "no_bd_ram");
-
if (of_property_read_u32(node, "cpdma_channels", &prop)) {
pr_err("Missing cpdma_channels property in the DT.\n");
ret = -EINVAL;
@@ -1060,34 +1066,6 @@ static int cpsw_probe_dt(struct cpsw_platform_data *data,
}
data->channels = prop;
- if (of_property_read_u32(node, "host_port_no", &prop)) {
- pr_err("Missing host_port_no property in the DT.\n");
- ret = -EINVAL;
- goto error_ret;
- }
- data->host_port_num = prop;
-
- if (of_property_read_u32(node, "cpdma_reg_ofs", &prop)) {
- pr_err("Missing cpdma_reg_ofs property in the DT.\n");
- ret = -EINVAL;
- goto error_ret;
- }
- data->cpdma_reg_ofs = prop;
-
- if (of_property_read_u32(node, "cpdma_sram_ofs", &prop)) {
- pr_err("Missing cpdma_sram_ofs property in the DT.\n");
- ret = -EINVAL;
- goto error_ret;
- }
- data->cpdma_sram_ofs = prop;
-
- if (of_property_read_u32(node, "ale_reg_ofs", &prop)) {
- pr_err("Missing ale_reg_ofs property in the DT.\n");
- ret = -EINVAL;
- goto error_ret;
- }
- data->ale_reg_ofs = prop;
-
if (of_property_read_u32(node, "ale_entries", &prop)) {
pr_err("Missing ale_entries property in the DT.\n");
ret = -EINVAL;
@@ -1095,34 +1073,6 @@ static int cpsw_probe_dt(struct cpsw_platform_data *data,
}
data->ale_entries = prop;
- if (of_property_read_u32(node, "host_port_reg_ofs", &prop)) {
- pr_err("Missing host_port_reg_ofs property in the DT.\n");
- ret = -EINVAL;
- goto error_ret;
- }
- data->host_port_reg_ofs = prop;
-
- if (of_property_read_u32(node, "hw_stats_reg_ofs", &prop)) {
- pr_err("Missing hw_stats_reg_ofs property in the DT.\n");
- ret = -EINVAL;
- goto error_ret;
- }
- data->hw_stats_reg_ofs = prop;
-
- if (of_property_read_u32(node, "cpts_reg_ofs", &prop)) {
- pr_err("Missing cpts_reg_ofs property in the DT.\n");
- ret = -EINVAL;
- goto error_ret;
- }
- data->cpts_reg_ofs = prop;
-
- if (of_property_read_u32(node, "bd_ram_ofs", &prop)) {
- pr_err("Missing bd_ram_ofs property in the DT.\n");
- ret = -EINVAL;
- goto error_ret;
- }
- data->bd_ram_ofs = prop;
-
if (of_property_read_u32(node, "bd_ram_size", &prop)) {
pr_err("Missing bd_ram_size property in the DT.\n");
ret = -EINVAL;
@@ -1156,22 +1106,6 @@ static int cpsw_probe_dt(struct cpsw_platform_data *data,
}
slave_data->phy_id = phy_id;
- if (of_property_read_u32(slave_node, "slave_reg_ofs", &prop)) {
- pr_err("Missing slave[%d] slave_reg_ofs property\n", i);
- ret = -EINVAL;
- goto error_ret;
- }
- slave_data->slave_reg_ofs = prop;
-
- if (of_property_read_u32(slave_node, "sliver_reg_ofs",
- &prop)) {
- pr_err("Missing slave[%d] sliver_reg_ofs property\n",
- i);
- ret = -EINVAL;
- goto error_ret;
- }
- slave_data->sliver_reg_ofs = prop;
-
mac_addr = of_get_mac_address(slave_node);
if (mac_addr)
memcpy(slave_data->mac_addr, mac_addr, ETH_ALEN);
@@ -1201,8 +1135,9 @@ static int __devinit cpsw_probe(struct platform_device *pdev)
struct cpsw_priv *priv;
struct cpdma_params dma_params;
struct cpsw_ale_params ale_params;
- void __iomem *regs;
+ void __iomem *ss_regs, *wr_regs;
struct resource *res;
+ u32 slave_offset, sliver_offset, slave_size;
int ret = 0, i, k = 0;
ndev = alloc_etherdev(sizeof(struct cpsw_priv));
@@ -1270,15 +1205,14 @@ static int __devinit cpsw_probe(struct platform_device *pdev)
ret = -ENXIO;
goto clean_clk_ret;
}
- regs = ioremap(priv->cpsw_res->start, resource_size(priv->cpsw_res));
- if (!regs) {
+ ss_regs = ioremap(priv->cpsw_res->start, resource_size(priv->cpsw_res));
+ if (!ss_regs) {
dev_err(priv->dev, "unable to map i/o region\n");
goto clean_cpsw_iores_ret;
}
- priv->regs = regs;
- priv->host_port = data->host_port_num;
- priv->host_port_regs = regs + data->host_port_reg_ofs;
- priv->cpts.reg = regs + data->cpts_reg_ofs;
+ priv->regs = ss_regs;
+ priv->version = __raw_readl(&priv->regs->id_ver);
+ priv->host_port = HOST_PORT_NUM;
priv->cpsw_wr_res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
if (!priv->cpsw_wr_res) {
@@ -1292,32 +1226,59 @@ static int __devinit cpsw_probe(struct platform_device *pdev)
ret = -ENXIO;
goto clean_iomap_ret;
}
- regs = ioremap(priv->cpsw_wr_res->start,
+ wr_regs = ioremap(priv->cpsw_wr_res->start,
resource_size(priv->cpsw_wr_res));
- if (!regs) {
+ if (!wr_regs) {
dev_err(priv->dev, "unable to map i/o region\n");
goto clean_cpsw_wr_iores_ret;
}
- priv->wr_regs = regs;
-
- for_each_slave(priv, cpsw_slave_init, priv);
+ priv->wr_regs = wr_regs;
memset(&dma_params, 0, sizeof(dma_params));
+ memset(&ale_params, 0, sizeof(ale_params));
+
+ switch (priv->version) {
+ case CPSW_VERSION_1:
+ priv->host_port_regs = ss_regs + CPSW1_HOST_PORT_OFFSET;
+ priv->cpts.reg = ss_regs + CPSW1_CPTS_OFFSET;
+ dma_params.dmaregs = ss_regs + CPSW1_CPDMA_OFFSET;
+ dma_params.txhdp = ss_regs + CPSW1_STATERAM_OFFSET;
+ ale_params.ale_regs = ss_regs + CPSW1_ALE_OFFSET;
+ slave_offset = CPSW1_SLAVE_OFFSET;
+ slave_size = CPSW1_SLAVE_SIZE;
+ sliver_offset = CPSW1_SLIVER_OFFSET;
+ dma_params.desc_mem_phys = 0;
+ break;
+ case CPSW_VERSION_2:
+ priv->host_port_regs = ss_regs + CPSW2_HOST_PORT_OFFSET;
+ priv->cpts.reg = ss_regs + CPSW2_CPTS_OFFSET;
+ dma_params.dmaregs = ss_regs + CPSW2_CPDMA_OFFSET;
+ dma_params.txhdp = ss_regs + CPSW2_STATERAM_OFFSET;
+ ale_params.ale_regs = ss_regs + CPSW2_ALE_OFFSET;
+ slave_offset = CPSW2_SLAVE_OFFSET;
+ slave_size = CPSW2_SLAVE_SIZE;
+ sliver_offset = CPSW2_SLIVER_OFFSET;
+ dma_params.desc_mem_phys =
+ (u32 __force) priv->cpsw_res->start + CPSW2_BD_OFFSET;
+ break;
+ default:
+ dev_err(priv->dev, "unknown version 0x%08x\n", priv->version);
+ ret = -ENODEV;
+ goto clean_cpsw_wr_iores_ret;
+ }
+ for (i = 0; i < priv->data.slaves; i++) {
+ struct cpsw_slave *slave = &priv->slaves[i];
+ cpsw_slave_init(slave, priv, slave_offset, sliver_offset);
+ slave_offset += slave_size;
+ sliver_offset += SLIVER_SIZE;
+ }
+
dma_params.dev = &pdev->dev;
- dma_params.dmaregs = cpsw_dma_regs((u32)priv->regs,
- data->cpdma_reg_ofs);
- dma_params.rxthresh = cpsw_dma_rxthresh((u32)priv->regs,
- data->cpdma_reg_ofs);
- dma_params.rxfree = cpsw_dma_rxfree((u32)priv->regs,
- data->cpdma_reg_ofs);
- dma_params.txhdp = cpsw_dma_txhdp((u32)priv->regs,
- data->cpdma_sram_ofs);
- dma_params.rxhdp = cpsw_dma_rxhdp((u32)priv->regs,
- data->cpdma_sram_ofs);
- dma_params.txcp = cpsw_dma_txcp((u32)priv->regs,
- data->cpdma_sram_ofs);
- dma_params.rxcp = cpsw_dma_rxcp((u32)priv->regs,
- data->cpdma_sram_ofs);
+ dma_params.rxthresh = dma_params.dmaregs + CPDMA_RXTHRESH;
+ dma_params.rxfree = dma_params.dmaregs + CPDMA_RXFREE;
+ dma_params.rxhdp = dma_params.txhdp + CPDMA_RXHDP;
+ dma_params.txcp = dma_params.txhdp + CPDMA_TXCP;
+ dma_params.rxcp = dma_params.txhdp + CPDMA_RXCP;
dma_params.num_chan = data->channels;
dma_params.has_soft_reset = true;
@@ -1325,10 +1286,7 @@ static int __devinit cpsw_probe(struct platform_device *pdev)
dma_params.desc_mem_size = data->bd_ram_size;
dma_params.desc_align = 16;
dma_params.has_ext_regs = true;
- dma_params.desc_mem_phys = data->no_bd_ram ? 0 :
- (u32 __force)priv->cpsw_res->start + data->bd_ram_ofs;
- dma_params.desc_hw_addr = data->hw_ram_addr ?
- data->hw_ram_addr : dma_params.desc_mem_phys ;
+ dma_params.desc_hw_addr = dma_params.desc_mem_phys;
priv->dma = cpdma_ctlr_create(&dma_params);
if (!priv->dma) {
@@ -1348,10 +1306,7 @@ static int __devinit cpsw_probe(struct platform_device *pdev)
goto clean_dma_ret;
}
- memset(&ale_params, 0, sizeof(ale_params));
ale_params.dev = &ndev->dev;
- ale_params.ale_regs = (void *)((u32)priv->regs) +
- ((u32)data->ale_reg_ofs);
ale_params.ale_ageout = ale_ageout;
ale_params.ale_entries = data->ale_entries;
ale_params.ale_ports = data->slaves;
diff --git a/include/linux/platform_data/cpsw.h b/include/linux/platform_data/cpsw.h
index b5c16c3..4b0ed74 100644
--- a/include/linux/platform_data/cpsw.h
+++ b/include/linux/platform_data/cpsw.h
@@ -18,8 +18,6 @@
#include <linux/if_ether.h>
struct cpsw_slave_data {
- u32 slave_reg_ofs;
- u32 sliver_reg_ofs;
const char *phy_id;
int phy_if;
u8 mac_addr[ETH_ALEN];
@@ -28,31 +26,14 @@ struct cpsw_slave_data {
struct cpsw_platform_data {
u32 ss_reg_ofs; /* Subsystem control register offset */
u32 channels; /* number of cpdma channels (symmetric) */
- u32 cpdma_reg_ofs; /* cpdma register offset */
- u32 cpdma_sram_ofs; /* cpdma sram offset */
-
u32 slaves; /* number of slave cpgmac ports */
struct cpsw_slave_data *slave_data;
u32 cpts_active_slave; /* time stamping slave */
u32 cpts_clock_mult; /* convert input clock ticks to nanoseconds */
u32 cpts_clock_shift; /* convert input clock ticks to nanoseconds */
-
- u32 ale_reg_ofs; /* address lookup engine reg offset */
u32 ale_entries; /* ale table size */
-
- u32 host_port_reg_ofs; /* cpsw cpdma host port registers */
- u32 host_port_num; /* The port number for the host port */
-
- u32 hw_stats_reg_ofs; /* cpsw hardware statistics counters */
- u32 cpts_reg_ofs; /* cpts registers */
-
- u32 bd_ram_ofs; /* embedded buffer descriptor RAM offset*/
u32 bd_ram_size; /*buffer descriptor ram size */
- u32 hw_ram_addr; /*if the HW address for BD RAM is different */
- bool no_bd_ram; /* no embedded BD ram*/
-
u32 rx_descs; /* Number of Rx Descriptios */
-
u32 mac_control; /* Mac control register */
};
--
1.7.0.4
^ permalink raw reply related
* [PATCH V4 5/7] arm/dts: am33xx: Add CPSW and MDIO module nodes for AM33XX
From: Mugunthan V N @ 2012-11-06 17:32 UTC (permalink / raw)
To: netdev
Cc: davem, devicetree-discuss, linux-arm-kernel, linux-omap,
Mugunthan V N, Vaibhav Hiremath, Benoit Cousson
In-Reply-To: <1352223159-27504-1-git-send-email-mugunthanvnm@ti.com>
Add CPSW and MDIO related device tree data for AM33XX.
Also enable them into board/evm dts files by providing
respective phy-id.
Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com>
Cc: Benoit Cousson <b-cousson@ti.com>
Acked-by: Peter Korsgaard <jacmet@sunsite.dk>
Acked-by: Richard Cochran <richardcochran@gmail.com>
---
arch/arm/boot/dts/am335x-bone.dts | 8 +++++++
arch/arm/boot/dts/am335x-evm.dts | 8 +++++++
arch/arm/boot/dts/am33xx.dtsi | 42 +++++++++++++++++++++++++++++++++++++
3 files changed, 58 insertions(+), 0 deletions(-)
diff --git a/arch/arm/boot/dts/am335x-bone.dts b/arch/arm/boot/dts/am335x-bone.dts
index c634f87..e233cfa 100644
--- a/arch/arm/boot/dts/am335x-bone.dts
+++ b/arch/arm/boot/dts/am335x-bone.dts
@@ -78,3 +78,11 @@
};
};
};
+
+&cpsw_emac0 {
+ phy_id = "4a101000.mdio:00";
+};
+
+&cpsw_emac1 {
+ phy_id = "4a101000.mdio:01";
+};
diff --git a/arch/arm/boot/dts/am335x-evm.dts b/arch/arm/boot/dts/am335x-evm.dts
index 185d632..415c3b3 100644
--- a/arch/arm/boot/dts/am335x-evm.dts
+++ b/arch/arm/boot/dts/am335x-evm.dts
@@ -118,3 +118,11 @@
};
};
};
+
+&cpsw_emac0 {
+ phy_id = "4a101000.mdio:00";
+};
+
+&cpsw_emac1 {
+ phy_id = "4a101000.mdio:01";
+};
diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
index bb31bff..2a0c8fe 100644
--- a/arch/arm/boot/dts/am33xx.dtsi
+++ b/arch/arm/boot/dts/am33xx.dtsi
@@ -210,5 +210,47 @@
interrupt-parent = <&intc>;
interrupts = <91>;
};
+
+ mac: ethernet@4A100000 {
+ compatible = "ti,cpsw";
+ ti,hwmods = "cpgmac0";
+ cpdma_channels = <8>;
+ ale_entries = <1024>;
+ bd_ram_size = <0x2000>;
+ no_bd_ram = <0>;
+ rx_descs = <64>;
+ mac_control = <0x20>;
+ slaves = <2>;
+ cpts_active_slave = <0>;
+ cpts_clock_mult = <0x80000000>;
+ cpts_clock_shift = <29>;
+ reg = <0x4a100000 0x800
+ 0x4a101200 0x100
+ 0x4a101000 0x100>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ interrupt-parent = <&intc>;
+ /* c0_rx_thresh_pend c0_rx_pend c0_tx_pend c0_misc_pend*/
+ interrupts = <40 41 42 43>;
+ ranges;
+ cpsw_emac0: slave@0 {
+ phy_id = "davinci_mdio.16:00";
+ /* Filled in by U-Boot */
+ mac-address = [ 00 00 00 00 00 00 ];
+ };
+ cpsw_emac1: slave@1 {
+ /* Filled in by U-Boot */
+ mac-address = [ 00 00 00 00 00 00 ];
+ };
+
+ davinci_mdio: mdio@4a101000 {
+ compatible = "ti,davinci_mdio";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ ti,hwmods = "davinci_mdio";
+ bus_freq = <1000000>;
+ reg = <0x4a101000 0x100>;
+ };
+ };
};
};
--
1.7.0.4
^ permalink raw reply related
* [PATCH V4 7/7] net: cpsw: halt network stack before halting the device during suspend
From: Mugunthan V N @ 2012-11-06 17:32 UTC (permalink / raw)
To: netdev
Cc: davem, devicetree-discuss, linux-arm-kernel, linux-omap,
Mugunthan V N
In-Reply-To: <1352223159-27504-1-git-send-email-mugunthanvnm@ti.com>
Move network stack halt APIs before halting the hardware to ensure no
packets are queued to hardware during closing the device during
suspend sequence.
Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
Acked-by: Richard Cochran <richardcochran@gmail.com>
---
drivers/net/ethernet/ti/cpsw.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index f94aa8f..b46dbb4 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -698,12 +698,12 @@ static int cpsw_ndo_stop(struct net_device *ndev)
struct cpsw_priv *priv = netdev_priv(ndev);
cpsw_info(priv, ifdown, "shutting down cpsw device\n");
- cpsw_intr_disable(priv);
- cpdma_ctlr_int_ctrl(priv->dma, false);
- cpdma_ctlr_stop(priv->dma);
netif_stop_queue(priv->ndev);
napi_disable(&priv->napi);
netif_carrier_off(priv->ndev);
+ cpsw_intr_disable(priv);
+ cpdma_ctlr_int_ctrl(priv->dma, false);
+ cpdma_ctlr_stop(priv->dma);
cpsw_ale_stop(priv->ale);
for_each_slave(priv, cpsw_slave_stop, priv);
pm_runtime_put_sync(&priv->pdev->dev);
--
1.7.0.4
^ permalink raw reply related
* [PATCH V4 6/7] ARM: OMAP2+: omap2plus_defconfig: Enable CPSW support
From: Mugunthan V N @ 2012-11-06 17:32 UTC (permalink / raw)
To: netdev
Cc: davem, devicetree-discuss, linux-arm-kernel, linux-omap,
Mugunthan V N
In-Reply-To: <1352223159-27504-1-git-send-email-mugunthanvnm@ti.com>
Enable CPSW support in defconfig which is present in AM33xx SoC
Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
Acked-by: Richard Cochran <richardcochran@gmail.com>
---
arch/arm/configs/omap2plus_defconfig | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/arch/arm/configs/omap2plus_defconfig b/arch/arm/configs/omap2plus_defconfig
index a4b330e..41b595e 100644
--- a/arch/arm/configs/omap2plus_defconfig
+++ b/arch/arm/configs/omap2plus_defconfig
@@ -242,3 +242,6 @@ CONFIG_CRC_ITU_T=y
CONFIG_CRC7=y
CONFIG_LIBCRC32C=y
CONFIG_SOC_OMAP5=y
+CONFIG_TI_DAVINCI_MDIO=y
+CONFIG_TI_DAVINCI_CPDMA=y
+CONFIG_TI_CPSW=y
--
1.7.0.4
^ permalink raw reply related
* [PATCH V4 3/7] ARM: OMAP3+: hwmod: Add AM33XX HWMOD data for davinci_mdio module
From: Mugunthan V N @ 2012-11-06 17:32 UTC (permalink / raw)
To: netdev
Cc: davem, devicetree-discuss, linux-arm-kernel, linux-omap,
Mugunthan V N, Vaibhav Hiremath, Paul Walmsley
In-Reply-To: <1352223159-27504-1-git-send-email-mugunthanvnm@ti.com>
This patch adds hwmod entry for davinci MDIO module,
creating parent<->child relationship between CPSW and MDIO module.
This Parent-child relation is required in order to use common resources
like, clock, but still maintaining the logical separation between them.
CPGMAC SubSystem consist of various sub-modules, like, mdio, cpdma,
cpsw, etc... These sub-modules are also used in some of Davinci
family of devices, so separate and independent platform devices &
drivers for CPSW and MDIO is implemented.
In case of AM33XX, the resources are shared and common register
bit-field is provided to control module/clock enable/disable,
makes it difficult to handle common resources from both drivers.
So the solution is, create parent<->child relationship between
CPGMAC & MDIO modules.
Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com>
Cc: Paul Walmsley <paul@pwsan.com>
Acked-by: Peter Korsgaard <jacmet@sunsite.dk>
Acked-by: Richard Cochran <richardcochran@gmail.com>
---
arch/arm/mach-omap2/omap_hwmod_33xx_data.c | 31 ++++++++++++++++++++++++++++
1 files changed, 31 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-omap2/omap_hwmod_33xx_data.c b/arch/arm/mach-omap2/omap_hwmod_33xx_data.c
index 59d5c1c..3125835 100644
--- a/arch/arm/mach-omap2/omap_hwmod_33xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_33xx_data.c
@@ -674,6 +674,7 @@ static struct omap_hwmod am33xx_cpgmac0_hwmod = {
.name = "cpgmac0",
.class = &am33xx_cpgmac0_hwmod_class,
.clkdm_name = "cpsw_125mhz_clkdm",
+ .flags = (HWMOD_SWSUP_SIDLE | HWMOD_SWSUP_MSTANDBY),
.mpu_irqs = am33xx_cpgmac0_irqs,
.main_clk = "cpsw_125mhz_gclk",
.prcm = {
@@ -685,6 +686,20 @@ static struct omap_hwmod am33xx_cpgmac0_hwmod = {
};
/*
+ * mdio class
+ */
+static struct omap_hwmod_class am33xx_mdio_hwmod_class = {
+ .name = "davinci_mdio",
+};
+
+static struct omap_hwmod am33xx_mdio_hwmod = {
+ .name = "davinci_mdio",
+ .class = &am33xx_mdio_hwmod_class,
+ .clkdm_name = "cpsw_125mhz_clkdm",
+ .main_clk = "cpsw_125mhz_gclk",
+};
+
+/*
* dcan class
*/
static struct omap_hwmod_class am33xx_dcan_hwmod_class = {
@@ -2501,6 +2516,21 @@ static struct omap_hwmod_ocp_if am33xx_l4_hs__cpgmac0 = {
.user = OCP_USER_MPU,
};
+struct omap_hwmod_addr_space am33xx_mdio_addr_space[] = {
+ {
+ .pa_start = 0x4A101000,
+ .pa_end = 0x4A101000 + SZ_256 - 1,
+ },
+ { }
+};
+
+struct omap_hwmod_ocp_if am33xx_cpgmac0__mdio = {
+ .master = &am33xx_cpgmac0_hwmod,
+ .slave = &am33xx_mdio_hwmod,
+ .addr = am33xx_mdio_addr_space,
+ .user = OCP_USER_MPU,
+};
+
static struct omap_hwmod_addr_space am33xx_elm_addr_space[] = {
{
.pa_start = 0x48080000,
@@ -3371,6 +3401,7 @@ static struct omap_hwmod_ocp_if *am33xx_hwmod_ocp_ifs[] __initdata = {
&am33xx_l3_main__tptc2,
&am33xx_l3_s__usbss,
&am33xx_l4_hs__cpgmac0,
+ &am33xx_cpgmac0__mdio,
NULL,
};
--
1.7.0.4
^ permalink raw reply related
* [PATCH V4 1/7] net: davinci_mdio: Fix typo mistake in calling runtime-pm api
From: Mugunthan V N @ 2012-11-06 17:32 UTC (permalink / raw)
To: netdev
Cc: davem, devicetree-discuss, linux-arm-kernel, linux-omap,
Vaibhav Hiremath, Mugunthan V N
In-Reply-To: <1352223159-27504-1-git-send-email-mugunthanvnm@ti.com>
From: Vaibhav Hiremath <hvaibhav@ti.com>
By mistake (most likely a copy-paste), instead of pm_runtime_get_sync()
api, driver is calling pm_runtime_put_sync() api in resume callback
function. The bug was introduced by commit id (ae2c07aaf74:
davinci_mdio: runtime PM support).
Now, the reason why it didn't impact functionality is, the patch has
been tested on AM335x-EVM and BeagleBone platform while submitting;
and in case of AM335x the MDIO driver doesn't control the module
enable/disable part, which is handled by CPSW driver.
Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com>
Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
Acked-by: Peter Korsgaard <jacmet@sunsite.dk>
Acked-by: Richard Cochran <richardcochran@gmail.com>
---
drivers/net/ethernet/ti/davinci_mdio.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ethernet/ti/davinci_mdio.c b/drivers/net/ethernet/ti/davinci_mdio.c
index 51a96db..ae74280 100644
--- a/drivers/net/ethernet/ti/davinci_mdio.c
+++ b/drivers/net/ethernet/ti/davinci_mdio.c
@@ -465,7 +465,7 @@ static int davinci_mdio_resume(struct device *dev)
u32 ctrl;
spin_lock(&data->lock);
- pm_runtime_put_sync(data->dev);
+ pm_runtime_get_sync(data->dev);
/* restart the scan state machine */
ctrl = __raw_readl(&data->regs->control);
--
1.7.0.4
^ permalink raw reply related
* [PATCH V4 2/7] net: cpsw: Add parent<->child relation support between cpsw and mdio
From: Mugunthan V N @ 2012-11-06 17:32 UTC (permalink / raw)
To: netdev
Cc: davem, devicetree-discuss, linux-arm-kernel, linux-omap,
Vaibhav Hiremath, Mugunthan V N
In-Reply-To: <1352223159-27504-1-git-send-email-mugunthanvnm@ti.com>
From: Vaibhav Hiremath <hvaibhav@ti.com>
CPGMAC SubSystem consist of various sub-modules, like, mdio, cpdma,
cpsw, etc... These sub-modules are also used in some of Davinci family
of devices. Now based on requirement, use-case and available technology
nodes the integration of these sub-modules varies across devices.
So coming back to Linux net driver, currently separate and independent
platform devices & drivers for CPSW and MDIO is implemented. In case of
Davinci they both has separate control, from resources perspective,
like clock.
In case of AM33XX, the resources are shared and only one register
bit-field is provided to control module/clock enable/disable, makes it
difficult to handle common resource.
So the solution here implemented in this patch is,
Create parent<->child relationship between both the drivers, making
CPSW as a parent and MDIO as its child and enumerate all the child nodes
under CPSW module.
Both the drivers will function exactly the way it was operating before,
including runtime-pm functionality. No change is required in MDIO driver
(for that matter to any child driver).
As this is only supported during DT boot, the parent<->child relationship
is created and populated in DT execution flow. The only required change
is inside DTS file, making MDIO as a child to CPSW node.
Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com>
Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
Acked-by: Peter Korsgaard <jacmet@sunsite.dk>
Acked-by: Richard Cochran <richardcochran@gmail.com>
---
drivers/net/ethernet/ti/cpsw.c | 16 ++++++++++++++--
1 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 7654a62..7007aba 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -1144,7 +1144,7 @@ static int cpsw_probe_dt(struct cpsw_platform_data *data,
}
data->mac_control = prop;
- for_each_child_of_node(node, slave_node) {
+ for_each_node_by_name(slave_node, "slave") {
struct cpsw_slave_data *slave_data = data->slave_data + i;
const char *phy_id = NULL;
const void *mac_addr = NULL;
@@ -1179,6 +1179,14 @@ static int cpsw_probe_dt(struct cpsw_platform_data *data,
i++;
}
+ /*
+ * Populate all the child nodes here...
+ */
+ ret = of_platform_populate(node, NULL, NULL, &pdev->dev);
+ /* We do not want to force this, as in some cases may not have child */
+ if (ret)
+ pr_warn("Doesn't have any child node\n");
+
return 0;
error_ret:
@@ -1212,6 +1220,11 @@ static int __devinit cpsw_probe(struct platform_device *pdev)
priv->msg_enable = netif_msg_init(debug_level, CPSW_DEBUG);
priv->rx_packet_max = max(rx_packet_max, 128);
+ /*
+ * This may be required here for child devices.
+ */
+ pm_runtime_enable(&pdev->dev);
+
if (cpsw_probe_dt(&priv->data, pdev)) {
pr_err("cpsw: platform data missing\n");
ret = -ENODEV;
@@ -1238,7 +1251,6 @@ static int __devinit cpsw_probe(struct platform_device *pdev)
for (i = 0; i < data->slaves; i++)
priv->slaves[i].slave_num = i;
- pm_runtime_enable(&pdev->dev);
priv->clk = clk_get(&pdev->dev, "fck");
if (IS_ERR(priv->clk)) {
dev_err(&pdev->dev, "fck is not found\n");
--
1.7.0.4
^ permalink raw reply related
* [PATCH V4 0/7] ARM: AM33XX: net: Add DT support to CPGMAC and MDIO driver
From: Mugunthan V N @ 2012-11-06 17:32 UTC (permalink / raw)
To: netdev
Cc: Mugunthan V N, devicetree-discuss, linux-omap, davem,
linux-arm-kernel
This patch-series adds support for,
[1/7]: Typo mistake in CPSW driver while invoking runtime_pm api's
[2/7]: Adds parent<->child relation between CPSW & MDIO module inside cpsw
driver, as in case of AM33XX, the resources are shared and common
register bit-field is provided to control module/clock enable/disable,
makes it difficult to handle common resource.
So the solution here is, to create parent<->child relation between them.
[3/7]: Add hwmod entry for MDIO module, required for MDIO driver.
[4/7]: cpsw: simplify the setup of the register pointers
[5/7]: Add DT device nodes for both CPSW and MDIO modules in am33xx.dtsi,
am335x-evm.dts and am335x-bone.dts file
[6/7]: Enable CPSW support to omap2plus_defconfig
[7/7]: cpsw: Kernel warn fix during suspend
This patch series has been created on top of net-next/master and tested
on BeagleBone platform for NFS boot and basic ping test cases.
Changes from V3:
* Removed unnecessary flags in Davinci MDIO Hwmod entry.
Mugunthan V N (4):
ARM: OMAP3+: hwmod: Add AM33XX HWMOD data for davinci_mdio module
arm/dts: am33xx: Add CPSW and MDIO module nodes for AM33XX
ARM: OMAP2+: omap2plus_defconfig: Enable CPSW support
net: cpsw: halt network stack before halting the device during
suspend
Richard Cochran (1):
cpsw: simplify the setup of the register pointers
Vaibhav Hiremath (2):
net: davinci_mdio: Fix typo mistake in calling runtime-pm api
net: cpsw: Add parent<->child relation support between cpsw and mdio
Documentation/devicetree/bindings/net/cpsw.txt | 34 ----
arch/arm/boot/dts/am335x-bone.dts | 8 +
arch/arm/boot/dts/am335x-evm.dts | 8 +
arch/arm/boot/dts/am33xx.dtsi | 42 +++++
arch/arm/configs/omap2plus_defconfig | 3 +
arch/arm/mach-omap2/omap_hwmod_33xx_data.c | 31 ++++
drivers/net/ethernet/ti/cpsw.c | 231 ++++++++++--------------
drivers/net/ethernet/ti/davinci_mdio.c | 2 +-
include/linux/platform_data/cpsw.h | 19 --
9 files changed, 192 insertions(+), 186 deletions(-)
^ permalink raw reply
* LETTER FROM MRS KATE
From: kate johnson @ 2012-11-06 17:02 UTC (permalink / raw)
[-- Attachment #1: Type: text/plain, Size: 241 bytes --]
--
I'm Mrs Kate Johnson from Ireland, suffering from breast cancer wish has got to
stage 4 and my doctor has made understand that i will not be able to live for
more than a month please read the attach file to know why i contacted you.
[-- Attachment #2: Document.docx --]
[-- Type: application/vnd.openxmlformats-officedocument.wordprocessingml.document, Size: 11720 bytes --]
^ permalink raw reply
* Re: [RFC PATCH v2] iproute2: bridge: add veb/vepa toggle
From: John Fastabend @ 2012-11-06 16:42 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: bhutchings, netdev
In-Reply-To: <20121105095039.3156a216@s6510.linuxnetplumber.net>
On 11/5/2012 12:50 AM, Stephen Hemminger wrote:
> On Fri, 02 Nov 2012 20:24:36 -0700
> John Fastabend <john.r.fastabend@intel.com> wrote:
>
>> Test bridge commands to set veb, vepa modes to iproute2.
[...]
>> +};
>> +#define IFLA_BRIDGE_MAX (__IFLA_BRIDGE_MAX - 1)
>> +
>> enum {
>> PORT_REQUEST_PREASSOCIATE = 0,
>> PORT_REQUEST_PREASSOCIATE_RR,
>>
>
> This looks ok. I will roll it into the resubmit of bridge changes
> when I get back from Linuxcon.
>
Sounds good thanks. I'll test what you come up with when I see
it.
^ permalink raw reply
* [PATCH v4 4/5] usbnet: smsc95xx: apply the introduced usbnet_{read|write}_cmd_nopm
From: Ming Lei @ 2012-11-06 14:53 UTC (permalink / raw)
To: David S. Miller, Greg Kroah-Hartman
Cc: Oliver Neukum, netdev, linux-usb, Ming Lei, Steve Glendinning
In-Reply-To: <1352213588-8948-1-git-send-email-ming.lei@canonical.com>
This patch applies the introduced usbnet_read_cmd_nopm() and
usbnet_write_cmd_nopm() in the callback of resume and suspend
to avoid deadlock if USB runtime PM is considered into
usbnet_read_cmd() and usbnet_write_cmd().
Cc: Steve Glendinning <steve.glendinning@shawell.net>
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
drivers/net/usb/smsc95xx.c | 134 ++++++++++++++++++++++++++++----------------
1 file changed, 85 insertions(+), 49 deletions(-)
diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index f69560c..e07f70b 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -73,20 +73,26 @@ static bool turbo_mode = true;
module_param(turbo_mode, bool, 0644);
MODULE_PARM_DESC(turbo_mode, "Enable multiple frames per Rx transaction");
-static int __must_check smsc95xx_read_reg(struct usbnet *dev, u32 index,
- u32 *data)
+static int __must_check __smsc95xx_read_reg(struct usbnet *dev, u32 index,
+ u32 *data, int in_pm)
{
u32 buf;
int ret;
+ int (*fn)(struct usbnet *, u8, u8, u16, u16, void *, u16);
BUG_ON(!dev);
- ret = usbnet_read_cmd(dev, USB_VENDOR_REQUEST_READ_REGISTER,
- USB_DIR_IN | USB_TYPE_VENDOR |
- USB_RECIP_DEVICE,
- 0, index, &buf, 4);
+ if (!in_pm)
+ fn = usbnet_read_cmd;
+ else
+ fn = usbnet_read_cmd_nopm;
+
+ ret = fn(dev, USB_VENDOR_REQUEST_READ_REGISTER, USB_DIR_IN
+ | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
+ 0, index, &buf, 4);
if (unlikely(ret < 0))
- netdev_warn(dev->net, "Failed to read register index 0x%08x\n", index);
+ netdev_warn(dev->net,
+ "Failed to read reg index 0x%08x: %d", index, ret);
le32_to_cpus(&buf);
*data = buf;
@@ -94,35 +100,64 @@ static int __must_check smsc95xx_read_reg(struct usbnet *dev, u32 index,
return ret;
}
-static int __must_check smsc95xx_write_reg(struct usbnet *dev, u32 index,
- u32 data)
+static int __must_check __smsc95xx_write_reg(struct usbnet *dev, u32 index,
+ u32 data, int in_pm)
{
u32 buf;
int ret;
+ int (*fn)(struct usbnet *, u8, u8, u16, u16, const void *, u16);
BUG_ON(!dev);
+ if (!in_pm)
+ fn = usbnet_write_cmd;
+ else
+ fn = usbnet_write_cmd_nopm;
+
buf = data;
cpu_to_le32s(&buf);
-
- ret = usbnet_write_cmd(dev, USB_VENDOR_REQUEST_WRITE_REGISTER,
- USB_DIR_OUT | USB_TYPE_VENDOR |
- USB_RECIP_DEVICE,
- 0, index, &buf, 4);
+ ret = fn(dev, USB_VENDOR_REQUEST_WRITE_REGISTER, USB_DIR_OUT
+ | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
+ 0, index, &buf, 4);
if (unlikely(ret < 0))
- netdev_warn(dev->net, "Failed to write register index 0x%08x\n", index);
+ netdev_warn(dev->net,
+ "Failed to write reg index 0x%08x: %d", index, ret);
return ret;
}
+static int __must_check smsc95xx_read_reg_nopm(struct usbnet *dev, u32 index,
+ u32 *data)
+{
+ return __smsc95xx_read_reg(dev, index, data, 1);
+}
+
+static int __must_check smsc95xx_write_reg_nopm(struct usbnet *dev, u32 index,
+ u32 data)
+{
+ return __smsc95xx_write_reg(dev, index, data, 1);
+}
+
+static int __must_check smsc95xx_read_reg(struct usbnet *dev, u32 index,
+ u32 *data)
+{
+ return __smsc95xx_read_reg(dev, index, data, 0);
+}
+
+static int __must_check smsc95xx_write_reg(struct usbnet *dev, u32 index,
+ u32 data)
+{
+ return __smsc95xx_write_reg(dev, index, data, 0);
+}
static int smsc95xx_set_feature(struct usbnet *dev, u32 feature)
{
if (WARN_ON_ONCE(!dev))
return -EINVAL;
- return usbnet_write_cmd(dev, USB_REQ_SET_FEATURE,
- USB_RECIP_DEVICE, feature, 0, NULL, 0);
+ return usbnet_write_cmd_nopm(dev, USB_REQ_SET_FEATURE,
+ USB_RECIP_DEVICE, feature, 0,
+ NULL, 0);
}
static int smsc95xx_clear_feature(struct usbnet *dev, u32 feature)
@@ -130,8 +165,9 @@ static int smsc95xx_clear_feature(struct usbnet *dev, u32 feature)
if (WARN_ON_ONCE(!dev))
return -EINVAL;
- return usbnet_write_cmd(dev, USB_REQ_CLEAR_FEATURE,
- USB_RECIP_DEVICE, feature, 0, NULL, 0);
+ return usbnet_write_cmd_nopm(dev, USB_REQ_CLEAR_FEATURE,
+ USB_RECIP_DEVICE, feature,
+ 0, NULL, 0);
}
/* Loop until the read is completed with timeout
@@ -708,7 +744,7 @@ static int smsc95xx_start_tx_path(struct usbnet *dev)
}
/* Starts the Receive path */
-static int smsc95xx_start_rx_path(struct usbnet *dev)
+static int smsc95xx_start_rx_path(struct usbnet *dev, int in_pm)
{
struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
unsigned long flags;
@@ -718,7 +754,7 @@ static int smsc95xx_start_rx_path(struct usbnet *dev)
pdata->mac_cr |= MAC_CR_RXEN_;
spin_unlock_irqrestore(&pdata->mac_cr_lock, flags);
- ret = smsc95xx_write_reg(dev, MAC_CR, pdata->mac_cr);
+ ret = __smsc95xx_write_reg(dev, MAC_CR, pdata->mac_cr, in_pm);
check_warn_return(ret, "Failed to write MAC_CR: %d\n", ret);
return 0;
@@ -937,7 +973,7 @@ static int smsc95xx_reset(struct usbnet *dev)
ret = smsc95xx_start_tx_path(dev);
check_warn_return(ret, "Failed to start TX path");
- ret = smsc95xx_start_rx_path(dev);
+ ret = smsc95xx_start_rx_path(dev, 0);
check_warn_return(ret, "Failed to start RX path");
netif_dbg(dev, ifup, dev->net, "smsc95xx_reset, return 0\n");
@@ -1039,30 +1075,30 @@ static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
netdev_info(dev->net, "entering SUSPEND2 mode");
/* disable energy detect (link up) & wake up events */
- ret = smsc95xx_read_reg(dev, WUCSR, &val);
+ ret = smsc95xx_read_reg_nopm(dev, WUCSR, &val);
check_warn_return(ret, "Error reading WUCSR");
val &= ~(WUCSR_MPEN_ | WUCSR_WAKE_EN_);
- ret = smsc95xx_write_reg(dev, WUCSR, val);
+ ret = smsc95xx_write_reg_nopm(dev, WUCSR, val);
check_warn_return(ret, "Error writing WUCSR");
- ret = smsc95xx_read_reg(dev, PM_CTRL, &val);
+ ret = smsc95xx_read_reg_nopm(dev, PM_CTRL, &val);
check_warn_return(ret, "Error reading PM_CTRL");
val &= ~(PM_CTL_ED_EN_ | PM_CTL_WOL_EN_);
- ret = smsc95xx_write_reg(dev, PM_CTRL, val);
+ ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
check_warn_return(ret, "Error writing PM_CTRL");
/* enter suspend2 mode */
- ret = smsc95xx_read_reg(dev, PM_CTRL, &val);
+ ret = smsc95xx_read_reg_nopm(dev, PM_CTRL, &val);
check_warn_return(ret, "Error reading PM_CTRL");
val &= ~(PM_CTL_SUS_MODE_ | PM_CTL_WUPS_ | PM_CTL_PHY_RST_);
val |= PM_CTL_SUS_MODE_2;
- ret = smsc95xx_write_reg(dev, PM_CTRL, val);
+ ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
check_warn_return(ret, "Error writing PM_CTRL");
return 0;
@@ -1131,7 +1167,7 @@ static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
}
for (i = 0; i < (pdata->wuff_filter_count * 4); i++) {
- ret = smsc95xx_write_reg(dev, WUFF, filter_mask[i]);
+ ret = smsc95xx_write_reg_nopm(dev, WUFF, filter_mask[i]);
if (ret < 0)
kfree(filter_mask);
check_warn_return(ret, "Error writing WUFF");
@@ -1139,43 +1175,43 @@ static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
kfree(filter_mask);
for (i = 0; i < (pdata->wuff_filter_count / 4); i++) {
- ret = smsc95xx_write_reg(dev, WUFF, command[i]);
+ ret = smsc95xx_write_reg_nopm(dev, WUFF, command[i]);
check_warn_return(ret, "Error writing WUFF");
}
for (i = 0; i < (pdata->wuff_filter_count / 4); i++) {
- ret = smsc95xx_write_reg(dev, WUFF, offset[i]);
+ ret = smsc95xx_write_reg_nopm(dev, WUFF, offset[i]);
check_warn_return(ret, "Error writing WUFF");
}
for (i = 0; i < (pdata->wuff_filter_count / 2); i++) {
- ret = smsc95xx_write_reg(dev, WUFF, crc[i]);
+ ret = smsc95xx_write_reg_nopm(dev, WUFF, crc[i]);
check_warn_return(ret, "Error writing WUFF");
}
/* clear any pending pattern match packet status */
- ret = smsc95xx_read_reg(dev, WUCSR, &val);
+ ret = smsc95xx_read_reg_nopm(dev, WUCSR, &val);
check_warn_return(ret, "Error reading WUCSR");
val |= WUCSR_WUFR_;
- ret = smsc95xx_write_reg(dev, WUCSR, val);
+ ret = smsc95xx_write_reg_nopm(dev, WUCSR, val);
check_warn_return(ret, "Error writing WUCSR");
}
if (pdata->wolopts & WAKE_MAGIC) {
/* clear any pending magic packet status */
- ret = smsc95xx_read_reg(dev, WUCSR, &val);
+ ret = smsc95xx_read_reg_nopm(dev, WUCSR, &val);
check_warn_return(ret, "Error reading WUCSR");
val |= WUCSR_MPR_;
- ret = smsc95xx_write_reg(dev, WUCSR, val);
+ ret = smsc95xx_write_reg_nopm(dev, WUCSR, val);
check_warn_return(ret, "Error writing WUCSR");
}
/* enable/disable wakeup sources */
- ret = smsc95xx_read_reg(dev, WUCSR, &val);
+ ret = smsc95xx_read_reg_nopm(dev, WUCSR, &val);
check_warn_return(ret, "Error reading WUCSR");
if (pdata->wolopts & (WAKE_BCAST | WAKE_MCAST | WAKE_ARP | WAKE_UCAST)) {
@@ -1194,41 +1230,41 @@ static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
val &= ~WUCSR_MPEN_;
}
- ret = smsc95xx_write_reg(dev, WUCSR, val);
+ ret = smsc95xx_write_reg_nopm(dev, WUCSR, val);
check_warn_return(ret, "Error writing WUCSR");
/* enable wol wakeup source */
- ret = smsc95xx_read_reg(dev, PM_CTRL, &val);
+ ret = smsc95xx_read_reg_nopm(dev, PM_CTRL, &val);
check_warn_return(ret, "Error reading PM_CTRL");
val |= PM_CTL_WOL_EN_;
- ret = smsc95xx_write_reg(dev, PM_CTRL, val);
+ ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
check_warn_return(ret, "Error writing PM_CTRL");
/* enable receiver to enable frame reception */
- smsc95xx_start_rx_path(dev);
+ smsc95xx_start_rx_path(dev, 1);
/* some wol options are enabled, so enter SUSPEND0 */
netdev_info(dev->net, "entering SUSPEND0 mode");
- ret = smsc95xx_read_reg(dev, PM_CTRL, &val);
+ ret = smsc95xx_read_reg_nopm(dev, PM_CTRL, &val);
check_warn_return(ret, "Error reading PM_CTRL");
val &= (~(PM_CTL_SUS_MODE_ | PM_CTL_WUPS_ | PM_CTL_PHY_RST_));
val |= PM_CTL_SUS_MODE_0;
- ret = smsc95xx_write_reg(dev, PM_CTRL, val);
+ ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
check_warn_return(ret, "Error writing PM_CTRL");
/* clear wol status */
val &= ~PM_CTL_WUPS_;
val |= PM_CTL_WUPS_WOL_;
- ret = smsc95xx_write_reg(dev, PM_CTRL, val);
+ ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
check_warn_return(ret, "Error writing PM_CTRL");
/* read back PM_CTRL */
- ret = smsc95xx_read_reg(dev, PM_CTRL, &val);
+ ret = smsc95xx_read_reg_nopm(dev, PM_CTRL, &val);
check_warn_return(ret, "Error reading PM_CTRL");
smsc95xx_set_feature(dev, USB_DEVICE_REMOTE_WAKEUP);
@@ -1249,22 +1285,22 @@ static int smsc95xx_resume(struct usb_interface *intf)
smsc95xx_clear_feature(dev, USB_DEVICE_REMOTE_WAKEUP);
/* clear wake-up sources */
- ret = smsc95xx_read_reg(dev, WUCSR, &val);
+ ret = smsc95xx_read_reg_nopm(dev, WUCSR, &val);
check_warn_return(ret, "Error reading WUCSR");
val &= ~(WUCSR_WAKE_EN_ | WUCSR_MPEN_);
- ret = smsc95xx_write_reg(dev, WUCSR, val);
+ ret = smsc95xx_write_reg_nopm(dev, WUCSR, val);
check_warn_return(ret, "Error writing WUCSR");
/* clear wake-up status */
- ret = smsc95xx_read_reg(dev, PM_CTRL, &val);
+ ret = smsc95xx_read_reg_nopm(dev, PM_CTRL, &val);
check_warn_return(ret, "Error reading PM_CTRL");
val &= ~PM_CTL_WOL_EN_;
val |= PM_CTL_WUPS_;
- ret = smsc95xx_write_reg(dev, PM_CTRL, val);
+ ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
check_warn_return(ret, "Error writing PM_CTRL");
}
--
1.7.9.5
^ permalink raw reply related
* [PATCH v4 2/5] usbnet: smsc75xx: apply the introduced usbnet_{read|write}_cmd_nopm
From: Ming Lei @ 2012-11-06 14:53 UTC (permalink / raw)
To: David S. Miller, Greg Kroah-Hartman
Cc: Oliver Neukum, netdev, linux-usb, Ming Lei, Steve Glendinning
In-Reply-To: <1352213588-8948-1-git-send-email-ming.lei@canonical.com>
This patch applies the introduced usbnet_read_cmd_nopm() and
usbnet_write_cmd_nopm() in the callback of resume and suspend
to avoid deadlock if USB runtime PM is considered into
usbnet_read_cmd() and usbnet_write_cmd().
Cc: Steve Glendinning <steve.glendinning@shawell.net>
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
drivers/net/usb/smsc75xx.c | 147 +++++++++++++++++++++++++++-----------------
1 file changed, 90 insertions(+), 57 deletions(-)
diff --git a/drivers/net/usb/smsc75xx.c b/drivers/net/usb/smsc75xx.c
index 85d70c2..c5353cf 100644
--- a/drivers/net/usb/smsc75xx.c
+++ b/drivers/net/usb/smsc75xx.c
@@ -85,18 +85,23 @@ static bool turbo_mode = true;
module_param(turbo_mode, bool, 0644);
MODULE_PARM_DESC(turbo_mode, "Enable multiple frames per Rx transaction");
-static int __must_check smsc75xx_read_reg(struct usbnet *dev, u32 index,
- u32 *data)
+static int __must_check __smsc75xx_read_reg(struct usbnet *dev, u32 index,
+ u32 *data, int in_pm)
{
u32 buf;
int ret;
+ int (*fn)(struct usbnet *, u8, u8, u16, u16, void *, u16);
BUG_ON(!dev);
- ret = usbnet_read_cmd(dev, USB_VENDOR_REQUEST_READ_REGISTER,
- USB_DIR_IN | USB_TYPE_VENDOR |
- USB_RECIP_DEVICE,
- 0, index, &buf, 4);
+ if (!in_pm)
+ fn = usbnet_read_cmd;
+ else
+ fn = usbnet_read_cmd_nopm;
+
+ ret = fn(dev, USB_VENDOR_REQUEST_READ_REGISTER, USB_DIR_IN
+ | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
+ 0, index, &buf, 4);
if (unlikely(ret < 0))
netdev_warn(dev->net,
"Failed to read reg index 0x%08x: %d", index, ret);
@@ -107,21 +112,26 @@ static int __must_check smsc75xx_read_reg(struct usbnet *dev, u32 index,
return ret;
}
-static int __must_check smsc75xx_write_reg(struct usbnet *dev, u32 index,
- u32 data)
+static int __must_check __smsc75xx_write_reg(struct usbnet *dev, u32 index,
+ u32 data, int in_pm)
{
u32 buf;
int ret;
+ int (*fn)(struct usbnet *, u8, u8, u16, u16, const void *, u16);
BUG_ON(!dev);
+ if (!in_pm)
+ fn = usbnet_write_cmd;
+ else
+ fn = usbnet_write_cmd_nopm;
+
buf = data;
cpu_to_le32s(&buf);
- ret = usbnet_write_cmd(dev, USB_VENDOR_REQUEST_WRITE_REGISTER,
- USB_DIR_OUT | USB_TYPE_VENDOR |
- USB_RECIP_DEVICE,
- 0, index, &buf, 4);
+ ret = fn(dev, USB_VENDOR_REQUEST_WRITE_REGISTER, USB_DIR_OUT
+ | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
+ 0, index, &buf, 4);
if (unlikely(ret < 0))
netdev_warn(dev->net,
"Failed to write reg index 0x%08x: %d", index, ret);
@@ -129,16 +139,38 @@ static int __must_check smsc75xx_write_reg(struct usbnet *dev, u32 index,
return ret;
}
+static int __must_check smsc75xx_read_reg_nopm(struct usbnet *dev, u32 index,
+ u32 *data)
+{
+ return __smsc75xx_read_reg(dev, index, data, 1);
+}
+
+static int __must_check smsc75xx_write_reg_nopm(struct usbnet *dev, u32 index,
+ u32 data)
+{
+ return __smsc75xx_write_reg(dev, index, data, 1);
+}
+
+static int __must_check smsc75xx_read_reg(struct usbnet *dev, u32 index,
+ u32 *data)
+{
+ return __smsc75xx_read_reg(dev, index, data, 0);
+}
+
+static int __must_check smsc75xx_write_reg(struct usbnet *dev, u32 index,
+ u32 data)
+{
+ return __smsc75xx_write_reg(dev, index, data, 0);
+}
+
static int smsc75xx_set_feature(struct usbnet *dev, u32 feature)
{
if (WARN_ON_ONCE(!dev))
return -EINVAL;
- cpu_to_le32s(&feature);
-
- return usb_control_msg(dev->udev, usb_sndctrlpipe(dev->udev, 0),
- USB_REQ_SET_FEATURE, USB_RECIP_DEVICE, feature, 0, NULL, 0,
- USB_CTRL_SET_TIMEOUT);
+ return usbnet_write_cmd_nopm(dev, USB_REQ_SET_FEATURE,
+ USB_DIR_OUT | USB_RECIP_DEVICE,
+ feature, 0, NULL, 0);
}
static int smsc75xx_clear_feature(struct usbnet *dev, u32 feature)
@@ -146,11 +178,9 @@ static int smsc75xx_clear_feature(struct usbnet *dev, u32 feature)
if (WARN_ON_ONCE(!dev))
return -EINVAL;
- cpu_to_le32s(&feature);
-
- return usb_control_msg(dev->udev, usb_sndctrlpipe(dev->udev, 0),
- USB_REQ_CLEAR_FEATURE, USB_RECIP_DEVICE, feature, 0, NULL, 0,
- USB_CTRL_SET_TIMEOUT);
+ return usbnet_write_cmd_nopm(dev, USB_REQ_CLEAR_FEATURE,
+ USB_DIR_OUT | USB_RECIP_DEVICE,
+ feature, 0, NULL, 0);
}
/* Loop until the read is completed with timeout
@@ -796,13 +826,16 @@ static int smsc75xx_set_features(struct net_device *netdev,
return 0;
}
-static int smsc75xx_wait_ready(struct usbnet *dev)
+static int smsc75xx_wait_ready(struct usbnet *dev, int in_pm)
{
int timeout = 0;
do {
u32 buf;
- int ret = smsc75xx_read_reg(dev, PMT_CTL, &buf);
+ int ret;
+
+ ret = __smsc75xx_read_reg(dev, PMT_CTL, &buf, in_pm);
+
check_warn_return(ret, "Failed to read PMT_CTL: %d", ret);
if (buf & PMT_CTL_DEV_RDY)
@@ -824,7 +857,7 @@ static int smsc75xx_reset(struct usbnet *dev)
netif_dbg(dev, ifup, dev->net, "entering smsc75xx_reset");
- ret = smsc75xx_wait_ready(dev);
+ ret = smsc75xx_wait_ready(dev, 0);
check_warn_return(ret, "device not ready in smsc75xx_reset");
ret = smsc75xx_read_reg(dev, HW_CFG, &buf);
@@ -1191,30 +1224,30 @@ static int smsc75xx_suspend(struct usb_interface *intf, pm_message_t message)
netdev_info(dev->net, "entering SUSPEND2 mode");
/* disable energy detect (link up) & wake up events */
- ret = smsc75xx_read_reg(dev, WUCSR, &val);
+ ret = smsc75xx_read_reg_nopm(dev, WUCSR, &val);
check_warn_return(ret, "Error reading WUCSR");
val &= ~(WUCSR_MPEN | WUCSR_WUEN);
- ret = smsc75xx_write_reg(dev, WUCSR, val);
+ ret = smsc75xx_write_reg_nopm(dev, WUCSR, val);
check_warn_return(ret, "Error writing WUCSR");
- ret = smsc75xx_read_reg(dev, PMT_CTL, &val);
+ ret = smsc75xx_read_reg_nopm(dev, PMT_CTL, &val);
check_warn_return(ret, "Error reading PMT_CTL");
val &= ~(PMT_CTL_ED_EN | PMT_CTL_WOL_EN);
- ret = smsc75xx_write_reg(dev, PMT_CTL, val);
+ ret = smsc75xx_write_reg_nopm(dev, PMT_CTL, val);
check_warn_return(ret, "Error writing PMT_CTL");
/* enter suspend2 mode */
- ret = smsc75xx_read_reg(dev, PMT_CTL, &val);
+ ret = smsc75xx_read_reg_nopm(dev, PMT_CTL, &val);
check_warn_return(ret, "Error reading PMT_CTL");
val &= ~(PMT_CTL_SUS_MODE | PMT_CTL_WUPS | PMT_CTL_PHY_RST);
val |= PMT_CTL_SUS_MODE_2;
- ret = smsc75xx_write_reg(dev, PMT_CTL, val);
+ ret = smsc75xx_write_reg_nopm(dev, PMT_CTL, val);
check_warn_return(ret, "Error writing PMT_CTL");
return 0;
@@ -1225,7 +1258,7 @@ static int smsc75xx_suspend(struct usb_interface *intf, pm_message_t message)
/* disable all filters */
for (i = 0; i < WUF_NUM; i++) {
- ret = smsc75xx_write_reg(dev, WUF_CFGX + i * 4, 0);
+ ret = smsc75xx_write_reg_nopm(dev, WUF_CFGX + i * 4, 0);
check_warn_return(ret, "Error writing WUF_CFGX");
}
@@ -1250,95 +1283,95 @@ static int smsc75xx_suspend(struct usb_interface *intf, pm_message_t message)
}
/* clear any pending pattern match packet status */
- ret = smsc75xx_read_reg(dev, WUCSR, &val);
+ ret = smsc75xx_read_reg_nopm(dev, WUCSR, &val);
check_warn_return(ret, "Error reading WUCSR");
val |= WUCSR_WUFR;
- ret = smsc75xx_write_reg(dev, WUCSR, val);
+ ret = smsc75xx_write_reg_nopm(dev, WUCSR, val);
check_warn_return(ret, "Error writing WUCSR");
netdev_info(dev->net, "enabling packet match detection");
- ret = smsc75xx_read_reg(dev, WUCSR, &val);
+ ret = smsc75xx_read_reg_nopm(dev, WUCSR, &val);
check_warn_return(ret, "Error reading WUCSR");
val |= WUCSR_WUEN;
- ret = smsc75xx_write_reg(dev, WUCSR, val);
+ ret = smsc75xx_write_reg_nopm(dev, WUCSR, val);
check_warn_return(ret, "Error writing WUCSR");
} else {
netdev_info(dev->net, "disabling packet match detection");
- ret = smsc75xx_read_reg(dev, WUCSR, &val);
+ ret = smsc75xx_read_reg_nopm(dev, WUCSR, &val);
check_warn_return(ret, "Error reading WUCSR");
val &= ~WUCSR_WUEN;
- ret = smsc75xx_write_reg(dev, WUCSR, val);
+ ret = smsc75xx_write_reg_nopm(dev, WUCSR, val);
check_warn_return(ret, "Error writing WUCSR");
}
/* disable magic, bcast & unicast wakeup sources */
- ret = smsc75xx_read_reg(dev, WUCSR, &val);
+ ret = smsc75xx_read_reg_nopm(dev, WUCSR, &val);
check_warn_return(ret, "Error reading WUCSR");
val &= ~(WUCSR_MPEN | WUCSR_BCST_EN | WUCSR_PFDA_EN);
- ret = smsc75xx_write_reg(dev, WUCSR, val);
+ ret = smsc75xx_write_reg_nopm(dev, WUCSR, val);
check_warn_return(ret, "Error writing WUCSR");
if (pdata->wolopts & WAKE_MAGIC) {
netdev_info(dev->net, "enabling magic packet wakeup");
- ret = smsc75xx_read_reg(dev, WUCSR, &val);
+ ret = smsc75xx_read_reg_nopm(dev, WUCSR, &val);
check_warn_return(ret, "Error reading WUCSR");
/* clear any pending magic packet status */
val |= WUCSR_MPR | WUCSR_MPEN;
- ret = smsc75xx_write_reg(dev, WUCSR, val);
+ ret = smsc75xx_write_reg_nopm(dev, WUCSR, val);
check_warn_return(ret, "Error writing WUCSR");
}
if (pdata->wolopts & WAKE_BCAST) {
netdev_info(dev->net, "enabling broadcast detection");
- ret = smsc75xx_read_reg(dev, WUCSR, &val);
+ ret = smsc75xx_read_reg_nopm(dev, WUCSR, &val);
check_warn_return(ret, "Error reading WUCSR");
val |= WUCSR_BCAST_FR | WUCSR_BCST_EN;
- ret = smsc75xx_write_reg(dev, WUCSR, val);
+ ret = smsc75xx_write_reg_nopm(dev, WUCSR, val);
check_warn_return(ret, "Error writing WUCSR");
}
if (pdata->wolopts & WAKE_UCAST) {
netdev_info(dev->net, "enabling unicast detection");
- ret = smsc75xx_read_reg(dev, WUCSR, &val);
+ ret = smsc75xx_read_reg_nopm(dev, WUCSR, &val);
check_warn_return(ret, "Error reading WUCSR");
val |= WUCSR_WUFR | WUCSR_PFDA_EN;
- ret = smsc75xx_write_reg(dev, WUCSR, val);
+ ret = smsc75xx_write_reg_nopm(dev, WUCSR, val);
check_warn_return(ret, "Error writing WUCSR");
}
/* enable receiver to enable frame reception */
- ret = smsc75xx_read_reg(dev, MAC_RX, &val);
+ ret = smsc75xx_read_reg_nopm(dev, MAC_RX, &val);
check_warn_return(ret, "Failed to read MAC_RX: %d", ret);
val |= MAC_RX_RXEN;
- ret = smsc75xx_write_reg(dev, MAC_RX, val);
+ ret = smsc75xx_write_reg_nopm(dev, MAC_RX, val);
check_warn_return(ret, "Failed to write MAC_RX: %d", ret);
/* some wol options are enabled, so enter SUSPEND0 */
netdev_info(dev->net, "entering SUSPEND0 mode");
- ret = smsc75xx_read_reg(dev, PMT_CTL, &val);
+ ret = smsc75xx_read_reg_nopm(dev, PMT_CTL, &val);
check_warn_return(ret, "Error reading PMT_CTL");
val &= (~(PMT_CTL_SUS_MODE | PMT_CTL_PHY_RST));
val |= PMT_CTL_SUS_MODE_0 | PMT_CTL_WOL_EN | PMT_CTL_WUPS;
- ret = smsc75xx_write_reg(dev, PMT_CTL, val);
+ ret = smsc75xx_write_reg_nopm(dev, PMT_CTL, val);
check_warn_return(ret, "Error writing PMT_CTL");
smsc75xx_set_feature(dev, USB_DEVICE_REMOTE_WAKEUP);
@@ -1359,37 +1392,37 @@ static int smsc75xx_resume(struct usb_interface *intf)
smsc75xx_clear_feature(dev, USB_DEVICE_REMOTE_WAKEUP);
/* Disable wakeup sources */
- ret = smsc75xx_read_reg(dev, WUCSR, &val);
+ ret = smsc75xx_read_reg_nopm(dev, WUCSR, &val);
check_warn_return(ret, "Error reading WUCSR");
val &= ~(WUCSR_WUEN | WUCSR_MPEN | WUCSR_PFDA_EN
| WUCSR_BCST_EN);
- ret = smsc75xx_write_reg(dev, WUCSR, val);
+ ret = smsc75xx_write_reg_nopm(dev, WUCSR, val);
check_warn_return(ret, "Error writing WUCSR");
/* clear wake-up status */
- ret = smsc75xx_read_reg(dev, PMT_CTL, &val);
+ ret = smsc75xx_read_reg_nopm(dev, PMT_CTL, &val);
check_warn_return(ret, "Error reading PMT_CTL");
val &= ~PMT_CTL_WOL_EN;
val |= PMT_CTL_WUPS;
- ret = smsc75xx_write_reg(dev, PMT_CTL, val);
+ ret = smsc75xx_write_reg_nopm(dev, PMT_CTL, val);
check_warn_return(ret, "Error writing PMT_CTL");
} else {
netdev_info(dev->net, "resuming from SUSPEND2");
- ret = smsc75xx_read_reg(dev, PMT_CTL, &val);
+ ret = smsc75xx_read_reg_nopm(dev, PMT_CTL, &val);
check_warn_return(ret, "Error reading PMT_CTL");
val |= PMT_CTL_PHY_PWRUP;
- ret = smsc75xx_write_reg(dev, PMT_CTL, val);
+ ret = smsc75xx_write_reg_nopm(dev, PMT_CTL, val);
check_warn_return(ret, "Error writing PMT_CTL");
}
- ret = smsc75xx_wait_ready(dev);
+ ret = smsc75xx_wait_ready(dev, 1);
check_warn_return(ret, "device not ready in smsc75xx_resume");
return usbnet_resume(intf);
--
1.7.9.5
^ permalink raw reply related
* [PATCH v4 0/5] usbnet: avoiding access auto-suspended device
From: Ming Lei @ 2012-11-06 14:53 UTC (permalink / raw)
To: David S. Miller, Greg Kroah-Hartman; +Cc: Oliver Neukum, netdev, linux-usb
Hi,
This patchset avoids accessing auto-suspended device in ioctl path,
which is generally triggered by some network utility(ethtool, ifconfig,
...)
Most of network devices have the problem, but as discussed in the
thread:
http://marc.info/?t=135054860600003&r=1&w=2
the problem should be solved inside driver.
Considered that only smsc75xx and smsc95xx calls usbnet_read_cmd()
and usbnet_write_cmd() inside its resume and suspend callback, the
patcheset introduces the nopm version of the two functions which
should be called only in the resume and suspend callback. So we
can solve the problem by runtime resuming device before doing
control message things.
The patchset is against 3.7.0-rc4-next-20121105, and has been tested
OK on smsc95xx usbnet device.
Change logs:
V4:
- kfree in failure path, only patch 3/5 changed
V3:
- fix comment and code style reported by Sergei Shtylyov
V2:
- rebased on the latest net-next tree, only 2/5 changed
V1:
- rebased on 3.7.0-rc3-next-20121102, only patch 4/5 changed
- fix one memory leak during smsc95xx_suspend, patch 3/5 added
drivers/net/usb/smsc75xx.c | 147 +++++++++++++++++++++++++++-----------------
drivers/net/usb/smsc95xx.c | 147 ++++++++++++++++++++++++++++----------------
drivers/net/usb/usbnet.c | 74 ++++++++++++++++++++--
include/linux/usb/usbnet.h | 4 ++
4 files changed, 258 insertions(+), 114 deletions(-)
Thanks,
--
Ming Lei
^ permalink raw reply
* [PATCH v4 5/5] usbnet: runtime wake up device before calling usbnet_{read|write}_cmd
From: Ming Lei @ 2012-11-06 14:53 UTC (permalink / raw)
To: David S. Miller, Greg Kroah-Hartman
Cc: Oliver Neukum, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA, Ming Lei
In-Reply-To: <1352213588-8948-1-git-send-email-ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
This patch gets the runtime PM reference count before calling
usbnet_{read|write}_cmd, and puts it after completion of the
usbnet_{read|write}_cmd, so that the usb control message can always
be sent to one active device in the non-PM context.
Signed-off-by: Ming Lei <ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
---
drivers/net/usb/usbnet.c | 20 ++++++++++++++++----
1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index a7fb074..08c4759 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -1673,8 +1673,14 @@ out:
int usbnet_read_cmd(struct usbnet *dev, u8 cmd, u8 reqtype,
u16 value, u16 index, void *data, u16 size)
{
- return __usbnet_read_cmd(dev, cmd, reqtype, value, index,
- data, size);
+ int ret;
+
+ if (usb_autopm_get_interface(dev->intf) < 0)
+ return -ENODEV;
+ ret = __usbnet_read_cmd(dev, cmd, reqtype, value, index,
+ data, size);
+ usb_autopm_put_interface(dev->intf);
+ return ret;
}
EXPORT_SYMBOL_GPL(usbnet_read_cmd);
@@ -1685,8 +1691,14 @@ EXPORT_SYMBOL_GPL(usbnet_read_cmd);
int usbnet_write_cmd(struct usbnet *dev, u8 cmd, u8 reqtype,
u16 value, u16 index, const void *data, u16 size)
{
- return __usbnet_write_cmd(dev, cmd, reqtype, value, index,
- data, size);
+ int ret;
+
+ if (usb_autopm_get_interface(dev->intf) < 0)
+ return -ENODEV;
+ ret = __usbnet_write_cmd(dev, cmd, reqtype, value, index,
+ data, size);
+ usb_autopm_put_interface(dev->intf);
+ return ret;
}
EXPORT_SYMBOL_GPL(usbnet_write_cmd);
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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 related
* [PATCH v4 3/5] usbnet: smsc95xx: fix memory leak in smsc95xx_suspend
From: Ming Lei @ 2012-11-06 14:53 UTC (permalink / raw)
To: David S. Miller, Greg Kroah-Hartman
Cc: Oliver Neukum, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA, Ming Lei
In-Reply-To: <1352213588-8948-1-git-send-email-ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
This patch fixes memory leak in smsc95xx_suspend.
Also, it isn't necessary to bother mm to allocate 8bytes/16byte,
and we can use stack variable safely.
Acked-By: Steve Glendinning <steve.glendinning-nksJyM/082jR7s880joybQ@public.gmane.org>
Signed-off-by: Ming Lei <ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
---
drivers/net/usb/smsc95xx.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 34f2e78..f69560c 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -1070,11 +1070,15 @@ static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
if (pdata->wolopts & (WAKE_BCAST | WAKE_MCAST | WAKE_ARP | WAKE_UCAST)) {
u32 *filter_mask = kzalloc(32, GFP_KERNEL);
- u32 *command = kzalloc(2, GFP_KERNEL);
- u32 *offset = kzalloc(2, GFP_KERNEL);
- u32 *crc = kzalloc(4, GFP_KERNEL);
+ u32 command[2];
+ u32 offset[2];
+ u32 crc[4];
int i, filter = 0;
+ memset(command, 0, sizeof(command));
+ memset(offset, 0, sizeof(offset));
+ memset(crc, 0, sizeof(crc));
+
if (pdata->wolopts & WAKE_BCAST) {
const u8 bcast[] = {0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF};
netdev_info(dev->net, "enabling broadcast detection");
@@ -1128,8 +1132,11 @@ static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
for (i = 0; i < (pdata->wuff_filter_count * 4); i++) {
ret = smsc95xx_write_reg(dev, WUFF, filter_mask[i]);
+ if (ret < 0)
+ kfree(filter_mask);
check_warn_return(ret, "Error writing WUFF");
}
+ kfree(filter_mask);
for (i = 0; i < (pdata->wuff_filter_count / 4); i++) {
ret = smsc95xx_write_reg(dev, WUFF, command[i]);
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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 related
* [PATCH v4 1/5] usbnet: introduce usbnet_{read|write}_cmd_nopm
From: Ming Lei @ 2012-11-06 14:53 UTC (permalink / raw)
To: David S. Miller, Greg Kroah-Hartman
Cc: Oliver Neukum, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA, Ming Lei
In-Reply-To: <1352213588-8948-1-git-send-email-ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
This patch introduces the below two helpers to prepare for solving
the usbnet runtime PM problem, which may cause some network utilities
(ifconfig, ethtool,...) touch a suspended device.
usbnet_read_cmd_nopm()
usbnet_write_cmd_nopm()
The above two helpers should be called by usbnet resume/suspend
callback to avoid deadlock.
Signed-off-by: Ming Lei <ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
---
drivers/net/usb/usbnet.c | 62 ++++++++++++++++++++++++++++++++++++++++----
include/linux/usb/usbnet.h | 4 +++
2 files changed, 61 insertions(+), 5 deletions(-)
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 09ea47a..a7fb074 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -1614,8 +1614,8 @@ void usbnet_device_suggests_idle(struct usbnet *dev)
EXPORT_SYMBOL(usbnet_device_suggests_idle);
/*-------------------------------------------------------------------------*/
-int usbnet_read_cmd(struct usbnet *dev, u8 cmd, u8 reqtype,
- u16 value, u16 index, void *data, u16 size)
+static int __usbnet_read_cmd(struct usbnet *dev, u8 cmd, u8 reqtype,
+ u16 value, u16 index, void *data, u16 size)
{
void *buf = NULL;
int err = -ENOMEM;
@@ -1639,10 +1639,10 @@ int usbnet_read_cmd(struct usbnet *dev, u8 cmd, u8 reqtype,
out:
return err;
}
-EXPORT_SYMBOL_GPL(usbnet_read_cmd);
-int usbnet_write_cmd(struct usbnet *dev, u8 cmd, u8 reqtype,
- u16 value, u16 index, const void *data, u16 size)
+static int __usbnet_write_cmd(struct usbnet *dev, u8 cmd, u8 reqtype,
+ u16 value, u16 index, const void *data,
+ u16 size)
{
void *buf = NULL;
int err = -ENOMEM;
@@ -1665,8 +1665,56 @@ int usbnet_write_cmd(struct usbnet *dev, u8 cmd, u8 reqtype,
out:
return err;
}
+
+/*
+ * The function can't be called inside suspend/resume callback,
+ * otherwise deadlock will be caused.
+ */
+int usbnet_read_cmd(struct usbnet *dev, u8 cmd, u8 reqtype,
+ u16 value, u16 index, void *data, u16 size)
+{
+ return __usbnet_read_cmd(dev, cmd, reqtype, value, index,
+ data, size);
+}
+EXPORT_SYMBOL_GPL(usbnet_read_cmd);
+
+/*
+ * The function can't be called inside suspend/resume callback,
+ * otherwise deadlock will be caused.
+ */
+int usbnet_write_cmd(struct usbnet *dev, u8 cmd, u8 reqtype,
+ u16 value, u16 index, const void *data, u16 size)
+{
+ return __usbnet_write_cmd(dev, cmd, reqtype, value, index,
+ data, size);
+}
EXPORT_SYMBOL_GPL(usbnet_write_cmd);
+/*
+ * The function can be called inside suspend/resume callback safely
+ * and should only be called by suspend/resume callback generally.
+ */
+int usbnet_read_cmd_nopm(struct usbnet *dev, u8 cmd, u8 reqtype,
+ u16 value, u16 index, void *data, u16 size)
+{
+ return __usbnet_read_cmd(dev, cmd, reqtype, value, index,
+ data, size);
+}
+EXPORT_SYMBOL_GPL(usbnet_read_cmd_nopm);
+
+/*
+ * The function can be called inside suspend/resume callback safely
+ * and should only be called by suspend/resume callback generally.
+ */
+int usbnet_write_cmd_nopm(struct usbnet *dev, u8 cmd, u8 reqtype,
+ u16 value, u16 index, const void *data,
+ u16 size)
+{
+ return __usbnet_write_cmd(dev, cmd, reqtype, value, index,
+ data, size);
+}
+EXPORT_SYMBOL_GPL(usbnet_write_cmd_nopm);
+
static void usbnet_async_cmd_cb(struct urb *urb)
{
struct usb_ctrlrequest *req = (struct usb_ctrlrequest *)urb->context;
@@ -1680,6 +1728,10 @@ static void usbnet_async_cmd_cb(struct urb *urb)
usb_free_urb(urb);
}
+/*
+ * The caller must make sure that device can't be put into suspend
+ * state until the control URB completes.
+ */
int usbnet_write_cmd_async(struct usbnet *dev, u8 cmd, u8 reqtype,
u16 value, u16 index, const void *data, u16 size)
{
diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
index 4410e416..9bbeabf 100644
--- a/include/linux/usb/usbnet.h
+++ b/include/linux/usb/usbnet.h
@@ -167,6 +167,10 @@ extern int usbnet_read_cmd(struct usbnet *dev, u8 cmd, u8 reqtype,
u16 value, u16 index, void *data, u16 size);
extern int usbnet_write_cmd(struct usbnet *dev, u8 cmd, u8 reqtype,
u16 value, u16 index, const void *data, u16 size);
+extern int usbnet_read_cmd_nopm(struct usbnet *dev, u8 cmd, u8 reqtype,
+ u16 value, u16 index, void *data, u16 size);
+extern int usbnet_write_cmd_nopm(struct usbnet *dev, u8 cmd, u8 reqtype,
+ u16 value, u16 index, const void *data, u16 size);
extern int usbnet_write_cmd_async(struct usbnet *dev, u8 cmd, u8 reqtype,
u16 value, u16 index, const void *data, u16 size);
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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 related
* Re: [Pv-drivers] [PATCH 1/6] VSOCK: vsock protocol implementation.
From: Andy King @ 2012-11-06 14:31 UTC (permalink / raw)
To: Stephen Hemminger
Cc: pv-drivers, netdev, linux-kernel, virtualization, gregkh, davem,
George Zhang
In-Reply-To: <20121106101013.09806f81@s6510.linuxnetplumber.net>
Hi Stephen,
> You did some scrubbing of the macro's to support multiple kernel
> versions, but there are still some leftovers.
> This code seems to have a lot of "this added in version xxx"
> type comments. These are probably not a good idea to include
> in the mainline kernel code.
Thanks so much for taking a look. Sorry about that, we'll remove
all such occurrences and send out a new series of patches in a bit.
Thanks!
- Andy
^ permalink raw reply
* Re: [PATCH v3 3/5] usbnet: smsc95xx: fix memory leak in smsc95xx_suspend
From: Ming Lei @ 2012-11-06 14:08 UTC (permalink / raw)
To: Steve Glendinning
Cc: David S. Miller, Greg Kroah-Hartman, Oliver Neukum, netdev,
linux-usb, Steve Glendinning
In-Reply-To: <CAKh2mn4HvxeJ-RasNQX5RfJhHxw9SpJ7y-Sy-jhdVuPa_E7KnA@mail.gmail.com>
On Tue, Nov 6, 2012 at 9:58 PM, Steve Glendinning <steve@shawell.net> wrote:
>> BTW, I just saw the smsc95xx datasheet and the vendor's driver
>> source code, and found the chip supports runtime PM well
>> (remote wakeup on 'good packet' or link change), so do you
>> plan to implement that?
>
> Yes, I do plan to implement this.
Good news, :-)
> Note that this feature is only
> supported on some product variants, for example LAN9500A, so it won't
> benefit all smsc95xx users.
Right. Looks remote wakeup on “Link Status Change” (SUSPEND1)
can be supported by all smsc95xx , and remote wakeup on 'good
packets'(SUSPEND3) is only supported by LAN9500A.
Thanks,
--
Ming Lei
Thanks,
--
Ming Lei
^ permalink raw reply
* Re: [PATCH v3 3/5] usbnet: smsc95xx: fix memory leak in smsc95xx_suspend
From: Steve Glendinning @ 2012-11-06 13:58 UTC (permalink / raw)
To: Ming Lei
Cc: David S. Miller, Greg Kroah-Hartman, Oliver Neukum, netdev,
linux-usb, Steve Glendinning
In-Reply-To: <CACVXFVPhYaBcr+V4W1-fSrLFVMMVo8vUv=eaKu2y_LjZ-EtnwQ@mail.gmail.com>
> BTW, I just saw the smsc95xx datasheet and the vendor's driver
> source code, and found the chip supports runtime PM well
> (remote wakeup on 'good packet' or link change), so do you
> plan to implement that?
Yes, I do plan to implement this. Note that this feature is only
supported on some product variants, for example LAN9500A, so it won't
benefit all smsc95xx users.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox