Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH 1/3 V3] phy/micrel: Implement support for KSZ8021
From: David Miller @ 2012-09-22 19:31 UTC (permalink / raw)
  To: marex; +Cc: netdev, david.choi, nobuhiro.iwamatsu.yj
In-Reply-To: <1348278110-11603-1-git-send-email-marex@denx.de>

From: Marek Vasut <marex@denx.de>
Date: Sat, 22 Sep 2012 03:41:47 +0200

> +	.name		= "Micrel KSZ8021",
> +	.features	= (PHY_BASIC_FEATURES | SUPPORTED_Pause |
> +				SUPPORTED_Asym_Pause),

You only fixed one of the two coding style problems I pointed out.

>From my original email:

====================
This is similarly not styled properly.

Besies being indented imporperly on the second line, the final "|"
character should be at the end of the first line, rather than
start the second line.
====================

Note "indented improperly on the second line"

Which is a clear reference to the first piece of
feedback I gave earlier in the same reply:

====================
This is not indented properly.  The goal is not to exclusively use
TAB characters to indent code until it sort-of looks fine.

Rather, the goal is to properly line up function arguments with
the first column after the openning parenthesis on the previous
line.  Using TAB and SPACE characters, as needed.
====================

^ permalink raw reply

* Re: pull-request: can-next 2012-09-22
From: David Miller @ 2012-09-22 19:26 UTC (permalink / raw)
  To: mkl; +Cc: netdev, linux-can
In-Reply-To: <505CE91C.1070504@pengutronix.de>

From: Marc Kleine-Budde <mkl@pengutronix.de>
Date: Sat, 22 Sep 2012 00:24:28 +0200

> this pull request is intended for net-next (v3.7 cycle), it consist of
> 5 patches by AnilKumar bringing device tree support, runtime PM, suspend
> resume and pinctrl support to the c_can/d_can driver.
> 
> Andreas Larsson improves the sja1000 driver (one-shot, listen only).
> Randy Dunlap fixes a function name conflict in the peak driver.
> Wei Yongjun fixes a return value check in the mscan-mpc5xxx driver.

Pulled, thanks Marc.

^ permalink raw reply

* Re: [PATCH] rds: Error on offset mismatch if not loopback
From: David Miller @ 2012-09-22 19:25 UTC (permalink / raw)
  To: jjolly; +Cc: linux-kernel, venkat.x.venkatsubra, netdev
In-Reply-To: <20120921213239.GJ14393@linux-tkdk.sfcn.org>

From: John Jolly <jjolly@suse.com>
Date: Fri, 21 Sep 2012 15:32:40 -0600

> Attempting an rds connection from the IP address of an IPoIB interface
> to itself causes a kernel panic due to a BUG_ON() being triggered.
> Making the test less strict allows rds-ping to work without crashing
> the machine.
> 
> A local unprivileged user could use this flaw to crash the system.
> 
> Signed-off-by: John Jolly <jjolly@suse.com>

Besides the questions being asked of you by Venkat Venkatsubra, this
patch has another issue.

It has been completely corrupted by your email client, it has
turned all TAB characters into spaces, making the patch useless.

Please learn how to send a patch unmolested in the body of your
email.  Test it by emailing the patch to yourself, and verifying
that you can in fact apply the patch you receive in that email.
Then, and only then, should you consider making a new submission
of this patch.

Use Documentation/email-clients.txt for guidance.

^ permalink raw reply

* Re: [PATCH V3 net-next 3/4] ptp: link the phc device to its parent device
From: Jeff Kirsher @ 2012-09-22 17:32 UTC (permalink / raw)
  To: Richard Cochran
  Cc: netdev, Ben Hutchings, David Miller, Jacob Keller, John Stultz,
	Matthew Vick
In-Reply-To: <5cbf85cdd74396119af11ba00b4b0bcd93f23ba0.1348332941.git.richardcochran@gmail.com>

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

On Sat, 2012-09-22 at 19:02 +0200, Richard Cochran wrote:
> PTP Hardware Clock devices appear as class devices in sysfs. This
> patch
> changes the registration API to use the parent device, clarifying the
> clock's relationship to the underlying device.
> 
> Signed-off-by: Richard Cochran <richardcochran@gmail.com> 

Acked-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH v3] lxt PHY: Support for the buggy LXT973 rev A2
From: Richard Cochran @ 2012-09-22 17:32 UTC (permalink / raw)
  To: Christophe Leroy; +Cc: David S Miller, netdev, linux-kernel
In-Reply-To: <201209221616.q8MGGnRc019864@localhost.localdomain>

On Sat, Sep 22, 2012 at 06:16:49PM +0200, Christophe Leroy wrote:
> This patch adds proper handling of the buggy revision A2 of LXT973 phy, adding
> precautions linked to ERRATA Item 4:
> 
> Revision A2 of LXT973 chip randomly returns the contents of the previous even
> register when you read a odd register regularly

This patch does not apply to net-next.

Also, I have just a few stylistic comments, below.

> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> 
> diff -u linux-3.5-vanilla/drivers/net/phy/lxt.c linux-3.5/drivers/net/phy/lxt.c
> --- linux-3.5-vanilla/drivers/net/phy/lxt.c	2012-07-21 22:58:29.000000000 +0200
> +++ linux-3.5/drivers/net/phy/lxt.c	2012-09-15 19:20:34.000000000 +0200

...

> +int lxt973a2_read_status(struct phy_device *phydev)
> +{
> +	int adv;
> +	int err;
> +	int lpa;
> +	int lpagb = 0;
> +
> +	/* Update the link, but return if there was an error */
> +	err = lxt973a2_update_link(phydev);
> +	if (err)
> +		return err;
> +
> +	if (AUTONEG_ENABLE == phydev->autoneg) {
> +		int retry = 1;
> +
> +		adv = phy_read(phydev, MII_ADVERTISE);
> +
> +		if (adv < 0)
> +			return adv;
> +
> +		do {
> +			lpa = phy_read(phydev, MII_LPA);
> +
> +			if (lpa < 0)
> +				return lpa;
> +
> +			/* If both registers are equal, it is suspect but not
> +			* impossible, hence a new try
> +			*/
> +		} while (lpa == adv && retry--);
> +
> +		lpa &= adv;
> +
> +		phydev->speed = SPEED_10;
> +		phydev->duplex = DUPLEX_HALF;
> +		phydev->pause = phydev->asym_pause = 0;
> +
> +		if (lpagb & (LPA_1000FULL | LPA_1000HALF)) {
> +			phydev->speed = SPEED_1000;
> +
> +			if (lpagb & LPA_1000FULL)
> +				phydev->duplex = DUPLEX_FULL;
> +		} else if (lpa & (LPA_100FULL | LPA_100HALF)) {
> +			phydev->speed = SPEED_100;
> +
> +			if (lpa & LPA_100FULL)
> +				phydev->duplex = DUPLEX_FULL;
> +		} else
> +			if (lpa & LPA_10FULL)
> +				phydev->duplex = DUPLEX_FULL;

This last 'else' could use braces.

> +
> +		if (phydev->duplex == DUPLEX_FULL) {
> +			phydev->pause = lpa & LPA_PAUSE_CAP ? 1 : 0;
> +			phydev->asym_pause = lpa & LPA_PAUSE_ASYM ? 1 : 0;
> +		}
> +	} else {
> +		int bmcr = phy_read(phydev, MII_BMCR);
> +
> +		if (bmcr < 0)
> +			return bmcr;
> +
> +		if (bmcr & BMCR_FULLDPLX)
> +			phydev->duplex = DUPLEX_FULL;
> +		else
> +			phydev->duplex = DUPLEX_HALF;
> +
> +		if (bmcr & BMCR_SPEED1000)
> +			phydev->speed = SPEED_1000;
> +		else if (bmcr & BMCR_SPEED100)
> +			phydev->speed = SPEED_100;
> +		else
> +			phydev->speed = SPEED_10;
> +
> +		phydev->pause = phydev->asym_pause = 0;
> +	}
> +
> +	return 0;
> +}
> +
> +static int lxt973_read_status(struct phy_device *phydev)
> +{
> +	return (int)phydev->priv&PHYDEV_PRIV_REVA2 ?
> +			lxt973a2_read_status(phydev) :
> +			genphy_read_status(phydev);

Needs spacing, like this:

	return (int) phydev->priv & PHYDEV_PRIV_REVA2 ?
		lxt973a2_read_status(phydev) : genphy_read_status(phydev);

> +}
> +
>  static int lxt973_probe(struct phy_device *phydev)
>  {
>  	int val = phy_read(phydev, MII_LXT973_PCR);
> +	int priv = 0;
> +
> +	phydev->priv = NULL;
> +
> +	if (val < 0)
> +		return val;
>  
>  	if (val & PCR_FIBER_SELECT) {
>  		/*
> @@ -136,17 +272,26 @@
>  		val &= ~BMCR_ANENABLE;
>  		phy_write(phydev, MII_BMCR, val);
>  		/* Remember that the port is in fiber mode. */
> -		phydev->priv = lxt973_probe;
> -	} else {
> -		phydev->priv = NULL;
> +		priv |= PHYDEV_PRIV_FIBER;
> +	}
> +	val = phy_read(phydev, MII_PHYSID2);
> +
> +	if (val < 0)
> +		return val;
> +
> +	if ((val & 0xf) == 0) { /* rev A2 */
> +		dev_info(&phydev->dev, " LXT973 revision A2 has bugs\n");
> +		priv |= PHYDEV_PRIV_REVA2;
>  	}
> +	phydev->priv = (void *)priv;

One space after cast, please: (void *) priv;

>  	return 0;
>  }
>  
>  static int lxt973_config_aneg(struct phy_device *phydev)
>  {
>  	/* Do nothing if port is in fiber mode. */
> -	return phydev->priv ? 0 : genphy_config_aneg(phydev);
> +	return (int)phydev->priv&PHYDEV_PRIV_FIBER ?
> +			0 : genphy_config_aneg(phydev);

Same spacing issue again.

Thanks,
Richard

^ permalink raw reply

* Re: [RFC PATCHv2 bridge 7/7] bridge: Add the ability to show dump the vlan map from a bridge port
From: David Miller @ 2012-09-22 17:27 UTC (permalink / raw)
  To: bhutchings; +Cc: vyasevic, netdev, shemminger
In-Reply-To: <1348334132.2521.100.camel@bwh-desktop.uk.solarflarecom.com>

From: Ben Hutchings <bhutchings@solarflare.com>
Date: Sat, 22 Sep 2012 18:15:32 +0100

> On Wed, 2012-09-19 at 08:42 -0400, Vlad Yasevich wrote:
>> Using the RTM_GETLINK dump the vlan map of a given bridge port.
> [...]
> 
> This enlarges the RTM_GETLINK response quite a bit.  I think perhaps
> this should be optional, like IFLA_VFINFO_LIST is now.

Completely agreed.

^ permalink raw reply

* Re: [PATCH V3 net-next 4/4] ptp: clarify the clock_name sysfs attribute
From: Ben Hutchings @ 2012-09-22 17:20 UTC (permalink / raw)
  To: Richard Cochran
  Cc: netdev, David Miller, Jacob Keller, Jeff Kirsher, John Stultz,
	Matthew Vick
In-Reply-To: <149b5cc1b5813bb4052f8dff4cbe0b5be62d086e.1348332941.git.richardcochran@gmail.com>

On Sat, 2012-09-22 at 19:02 +0200, Richard Cochran wrote:
> There has been some confusion among PHC driver authors about the
> intended purpose of the clock_name attribute. This patch expands the
> documation in order to clarify how the clock_name field should be
> understood.
> 
> Signed-off-by: Richard Cochran <richardcochran@gmail.com>
[...]

Looks good, thanks.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

^ permalink raw reply

* Re: [PATCH V3 net-next 3/4] ptp: link the phc device to its parent device
From: Ben Hutchings @ 2012-09-22 17:18 UTC (permalink / raw)
  To: Richard Cochran
  Cc: netdev, David Miller, Jacob Keller, Jeff Kirsher, John Stultz,
	Matthew Vick
In-Reply-To: <5cbf85cdd74396119af11ba00b4b0bcd93f23ba0.1348332941.git.richardcochran@gmail.com>

On Sat, 2012-09-22 at 19:02 +0200, Richard Cochran wrote:
> PTP Hardware Clock devices appear as class devices in sysfs. This patch
> changes the registration API to use the parent device, clarifying the
> clock's relationship to the underlying device.
> 
> Signed-off-by: Richard Cochran <richardcochran@gmail.com>
[...]

Acked-by: Ben Hutchings <bhutchings@solarflare.com>

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

^ permalink raw reply

* Re: [RFC PATCHv2 bridge 4/7] bridge: Add netlink interface to configure vlans on bridge ports
From: Ben Hutchings @ 2012-09-22 17:17 UTC (permalink / raw)
  To: Vlad Yasevich; +Cc: netdev, shemminger
In-Reply-To: <1348058536-22607-5-git-send-email-vyasevic@redhat.com>

On Wed, 2012-09-19 at 08:42 -0400, Vlad Yasevich wrote:
> Add a netlink interface to add and remove vlan configuration on bridge port.
> The interface uses the RTM_SETLINK message and encodes the vlan
> configuration inside the IFLA_AF_SPEC.  It is possble to include multiple
> vlans to either add or remove in a single message.
[...]
> --- a/net/bridge/br_if.c
> +++ b/net/bridge/br_if.c
> @@ -23,6 +23,7 @@
>  #include <linux/if_ether.h>
>  #include <linux/slab.h>
>  #include <net/sock.h>
> +#include <linux/if_vlan.h>
>  
>  #include "br_private.h"
>  
> @@ -445,6 +446,79 @@ int br_del_if(struct net_bridge *br, struct net_device *dev)
>  	return 0;
>  }
>  
> +/* Called with RTNL */
> +int br_set_port_vlan(struct net_bridge_port *p, unsigned short vlan)
> +{
> +	unsigned long table_size = BITS_TO_LONGS(br_vid(VLAN_N_VID));
> +	unsigned long *vid_map = NULL;
> +	__u16 vid = br_vid(vlan);
> +	int ret = 0;
> +
> +	/* The vlan map is indexed by vid+1.  This way we can store
> +	 * vid 0 (untagged) into the map as well.
> +	 */

So bit 1 is for untagged, bits 2-4096 are for tagged, and bit 0 is
for...?

> +	if (!p->vlan_map) {
> +		vid_map = kzalloc(table_size, GFP_KERNEL);
> +		if (!vid_map) {
> +			return -ENOMEM;
> +		}
> +
> +		set_bit(vid, vid_map);
> +		rcu_assign_pointer(p->vlan_map, vid_map);
> +		synchronize_net();
> +	} else {
> +		/* Map is already allocated */
> +		set_bit(vid, rcu_dereference_rtnl(p->vlan_map));
> +	}
> +
> +	return ret;
> +}
> +
> +
> +/* Called with RTNL */
> +int br_del_port_vlan(struct net_bridge_port *p, unsigned short vlan)
> +{
> +	unsigned long first_bit;
> +	unsigned long next_bit;
> +	__u16 vid = br_vid(vlan);

Which is the bit number, not really the VID - a little confusing...

> +	unsigned long tbl_len = BITS_TO_LONGS(br_vid(VLAN_N_VID));
> +
> +	if (!p->vlan_map) {
> +		return -EINVAL;
> +	}
> +
> +	if (!test_bit(vlan, p->vlan_map)) {
> +		return -EINVAL;
> +	}
> +
> +	/* Check to see if any other vlans are in this table.  If this
> +	 * is the last vlan, delete the whole table.  If this is not the
> +	 * last vlan, just clear the bit.
> +	 */
> +	first_bit = find_first_bit(p->vlan_map, tbl_len);
> +	next_bit = find_next_bit(p->vlan_map, tbl_len, (tbl_len - vid));
[...]

Last parameter to find_next_bit is the starting offset, which should
presumably be vid + 1.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

^ permalink raw reply

* Re: [RFC PATCHv2 bridge 2/7] bridge: Add vlan to unicast fdb entries
From: Ben Hutchings @ 2012-09-22 17:17 UTC (permalink / raw)
  To: Vlad Yasevich; +Cc: netdev, shemminger
In-Reply-To: <1348058536-22607-3-git-send-email-vyasevic@redhat.com>

On Wed, 2012-09-19 at 08:42 -0400, Vlad Yasevich wrote:
> This patch adds vlan to unicast fdb entries that are created for
> learned addresses (not the manually configured ones).  It adds
> vlan id into the hash mix and uses vlan as an addditional parameter
> for an entry match.
[...]
> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
> index 9ce430b..e17f9f2 100644
> --- a/net/bridge/br_fdb.c
> +++ b/net/bridge/br_fdb.c
[...]
> @@ -67,11 +68,12 @@ static inline int has_expired(const struct net_bridge *br,
>                 time_before_eq(fdb->updated + hold_time(br), jiffies);
>  }
>  
> -static inline int br_mac_hash(const unsigned char *mac)
> +static inline int br_mac_hash(const unsigned char *mac, __u16 vlan_tci)
>  {
> -       /* use 1 byte of OUI cnd 3 bytes of NIC */
> +       /* use 1 byte of OUI and 3 bytes of NIC */
>         u32 key = get_unaligned((u32 *)(mac + 2));
> -       return jhash_1word(key, fdb_salt) & (BR_HASH_SIZE - 1);
> +       return jhash_2words(key, (vlan_tci & VLAN_VID_MASK),
> +                               fdb_salt) & (BR_HASH_SIZE - 1);
>  }

Why do you add a vlan_tci parameter to so many functions, which they
then mask to get the VID?  Would it not make more sense to pass only
VIDs around?

[...]
> @@ -628,11 +640,12 @@ int br_fdb_add(struct ndmsg *ndm, struct net_device *dev,
>  
>         if (ndm->ndm_flags & NTF_USE) {
>                 rcu_read_lock();
> -               br_fdb_update(p->br, p, addr);
> +               br_fdb_update(p->br, p, addr, 0);
>                 rcu_read_unlock();
>         } else {
>                 spin_lock_bh(&p->br->hash_lock);
> -               err = fdb_add_entry(p, addr, ndm->ndm_state, nlh_flags);
> +               err = fdb_add_entry(p, addr, ndm->ndm_state, nlh_flags,
> +                               0);
>                 spin_unlock_bh(&p->br->hash_lock);
>         }
>  
> @@ -642,10 +655,10 @@ int br_fdb_add(struct ndmsg *ndm, struct net_device *dev,
>  static int fdb_delete_by_addr(struct net_bridge_port *p, u8 *addr)
>  {
>         struct net_bridge *br = p->br;
> -       struct hlist_head *head = &br->hash[br_mac_hash(addr)];
> +       struct hlist_head *head = &br->hash[br_mac_hash(addr, 0)];
>         struct net_bridge_fdb_entry *fdb;
>  
> -       fdb = fdb_find(head, addr);
> +       fdb = fdb_find(head, addr, 0);
>         if (!fdb)
>                 return -ENOENT;
> 

So current tools will only be able to manipulate forwarding entries for
untagged frames?  Surely they should still insert and delete forwarding
entries that affect all VLANs, and new tools will be able to restrict
forwarding entries to specific VLANs?

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

^ permalink raw reply

* Re: [RFC PATCHv2 bridge 7/7] bridge:  Add the ability to show dump the vlan map from a bridge port
From: Ben Hutchings @ 2012-09-22 17:15 UTC (permalink / raw)
  To: Vlad Yasevich; +Cc: netdev, shemminger
In-Reply-To: <1348058536-22607-8-git-send-email-vyasevic@redhat.com>

On Wed, 2012-09-19 at 08:42 -0400, Vlad Yasevich wrote:
> Using the RTM_GETLINK dump the vlan map of a given bridge port.
[...]

This enlarges the RTM_GETLINK response quite a bit.  I think perhaps
this should be optional, like IFLA_VFINFO_LIST is now.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

^ permalink raw reply

* pull request: wireless 2012-09-22
From: John W. Linville @ 2012-09-22 17:13 UTC (permalink / raw)
  To: davem-fT/PcQaiUtIeIZ0/mPfg9Q
  Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

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

commit 1199992df2417dc9a1db1b19930ea4d0a697a61e

Dave,

Please pull this last(?) batch of fixes intended for 3.6...

For the Bluetooth bits, Gustavo says this:

"Here goes probably my last update to 3.6. It includes the two patches
you were ok last week(from Andrzej Kaczmarek), those are critical
ones, and two other fixes one for a system crash and the other for
a missing lockdep annotation."

The referenced fixes from Andrzej prevent attempts to configure devices
that are powered-off.

Along with the Bluetooth fixes, there are a couple of 802.11 fixes.
Emmanuel Grumbach gives us an iwlwifi fix to prevent releasing an
interrupt twice.  Luis R. Rodriguez provides a fix for a possible
circular lock dependency in the cfg80211 regulatory enforcement code.

All of these have been in linux-next for a few days.  I hope they are
not too late to make the 3.6 release!

Thanks,

John

---

The following changes since commit abef3bd71029b80ec1bdd6c6244b5b0b99f56633:

  Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net (2012-09-21 14:32:55 -0700)

are available in the git repository at:


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

for you to fetch changes up to 1199992df2417dc9a1db1b19930ea4d0a697a61e:

  Merge branch 'master' of git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless into for-davem (2012-09-22 12:19:22 -0400)

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

Andrei Emeltchenko (1):
      Bluetooth: Fix freeing uninitialized delayed works

Andrzej Kaczmarek (2):
      Bluetooth: mgmt: Fix enabling SSP while powered off
      Bluetooth: mgmt: Fix enabling LE while powered off

Emmanuel Grumbach (1):
      iwlwifi: don't double free the interrupt in failure path

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

Luis R. Rodriguez (1):
      cfg80211: fix possible circular lock on reg_regdb_search()

Vinicius Costa Gomes (1):
      Bluetooth: Fix not removing power_off delayed work

 drivers/net/wireless/iwlwifi/pcie/trans.c |  1 +
 net/bluetooth/hci_core.c                  |  2 ++
 net/bluetooth/l2cap_core.c                |  2 +-
 net/bluetooth/mgmt.c                      | 16 ++++++++++++++++
 net/wireless/reg.c                        | 12 +++++++++---
 5 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/pcie/trans.c b/drivers/net/wireless/iwlwifi/pcie/trans.c
index 1e86ea2..dbeebef 100644
--- a/drivers/net/wireless/iwlwifi/pcie/trans.c
+++ b/drivers/net/wireless/iwlwifi/pcie/trans.c
@@ -1442,6 +1442,7 @@ static int iwl_trans_pcie_start_hw(struct iwl_trans *trans)
 	return err;
 
 err_free_irq:
+	trans_pcie->irq_requested = false;
 	free_irq(trans_pcie->irq, trans);
 error:
 	iwl_free_isr_ict(trans);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index d4de5db..0b997c8 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -734,6 +734,8 @@ static int hci_dev_do_close(struct hci_dev *hdev)
 
 	cancel_work_sync(&hdev->le_scan);
 
+	cancel_delayed_work(&hdev->power_off);
+
 	hci_req_cancel(hdev, ENODEV);
 	hci_req_lock(hdev);
 
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 4ea1710..38c00f1 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1008,7 +1008,7 @@ static void l2cap_send_disconn_req(struct l2cap_conn *conn, struct l2cap_chan *c
 	if (!conn)
 		return;
 
-	if (chan->mode == L2CAP_MODE_ERTM) {
+	if (chan->mode == L2CAP_MODE_ERTM && chan->state == BT_CONNECTED) {
 		__clear_retrans_timer(chan);
 		__clear_monitor_timer(chan);
 		__clear_ack_timer(chan);
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index ad6613d..eba022d 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -2875,6 +2875,22 @@ int mgmt_powered(struct hci_dev *hdev, u8 powered)
 		if (scan)
 			hci_send_cmd(hdev, HCI_OP_WRITE_SCAN_ENABLE, 1, &scan);
 
+		if (test_bit(HCI_SSP_ENABLED, &hdev->dev_flags)) {
+			u8 ssp = 1;
+
+			hci_send_cmd(hdev, HCI_OP_WRITE_SSP_MODE, 1, &ssp);
+		}
+
+		if (test_bit(HCI_LE_ENABLED, &hdev->dev_flags)) {
+			struct hci_cp_write_le_host_supported cp;
+
+			cp.le = 1;
+			cp.simul = !!(hdev->features[6] & LMP_SIMUL_LE_BR);
+
+			hci_send_cmd(hdev, HCI_OP_WRITE_LE_HOST_SUPPORTED,
+				     sizeof(cp), &cp);
+		}
+
 		update_class(hdev);
 		update_name(hdev, hdev->dev_name);
 		update_eir(hdev);
diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 2ded3c7..72d170c 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -350,6 +350,9 @@ static void reg_regdb_search(struct work_struct *work)
 	struct reg_regdb_search_request *request;
 	const struct ieee80211_regdomain *curdom, *regdom;
 	int i, r;
+	bool set_reg = false;
+
+	mutex_lock(&cfg80211_mutex);
 
 	mutex_lock(&reg_regdb_search_mutex);
 	while (!list_empty(&reg_regdb_search_list)) {
@@ -365,9 +368,7 @@ static void reg_regdb_search(struct work_struct *work)
 				r = reg_copy_regd(&regdom, curdom);
 				if (r)
 					break;
-				mutex_lock(&cfg80211_mutex);
-				set_regdom(regdom);
-				mutex_unlock(&cfg80211_mutex);
+				set_reg = true;
 				break;
 			}
 		}
@@ -375,6 +376,11 @@ static void reg_regdb_search(struct work_struct *work)
 		kfree(request);
 	}
 	mutex_unlock(&reg_regdb_search_mutex);
+
+	if (set_reg)
+		set_regdom(regdom);
+
+	mutex_unlock(&cfg80211_mutex);
 }
 
 static DECLARE_WORK(reg_regdb_work, reg_regdb_search);
-- 
John W. Linville		Someday the world will need a hero, and you
linville-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org			might be all we have.  Be ready.

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

^ permalink raw reply related

* [patch net] team: send port changed when added
From: Jiri Pirko @ 2012-09-22 17:07 UTC (permalink / raw)
  To: netdev; +Cc: davem

On some hw, link is not up during adding iface to team. That causes event
not being sent to userspace and that may cause confusion.
Fix this bug by sending port changed event once it's added to team.

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
 drivers/net/team/team.c | 32 ++++++++++++++++++++++++--------
 1 file changed, 24 insertions(+), 8 deletions(-)

diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
index 341b65d..3ffe8a6 100644
--- a/drivers/net/team/team.c
+++ b/drivers/net/team/team.c
@@ -848,7 +848,7 @@ static struct netpoll_info *team_netpoll_info(struct team *team)
 }
 #endif
 
-static void __team_port_change_check(struct team_port *port, bool linkup);
+static void __team_port_change_port_added(struct team_port *port, bool linkup);
 
 static int team_port_add(struct team *team, struct net_device *port_dev)
 {
@@ -948,7 +948,7 @@ static int team_port_add(struct team *team, struct net_device *port_dev)
 	team_port_enable(team, port);
 	list_add_tail_rcu(&port->list, &team->port_list);
 	__team_compute_features(team);
-	__team_port_change_check(port, !!netif_carrier_ok(port_dev));
+	__team_port_change_port_added(port, !!netif_carrier_ok(port_dev));
 	__team_options_change_check(team);
 
 	netdev_info(dev, "Port device %s added\n", portname);
@@ -983,6 +983,8 @@ err_set_mtu:
 	return err;
 }
 
+static void __team_port_change_port_removed(struct team_port *port);
+
 static int team_port_del(struct team *team, struct net_device *port_dev)
 {
 	struct net_device *dev = team->dev;
@@ -999,8 +1001,7 @@ static int team_port_del(struct team *team, struct net_device *port_dev)
 	__team_option_inst_mark_removed_port(team, port);
 	__team_options_change_check(team);
 	__team_option_inst_del_port(team, port);
-	port->removed = true;
-	__team_port_change_check(port, false);
+	__team_port_change_port_removed(port);
 	team_port_disable(team, port);
 	list_del_rcu(&port->list);
 	netdev_rx_handler_unregister(port_dev);
@@ -2251,13 +2252,11 @@ static void __team_options_change_check(struct team *team)
 }
 
 /* rtnl lock is held */
-static void __team_port_change_check(struct team_port *port, bool linkup)
+
+static void __team_port_change_send(struct team_port *port, bool linkup)
 {
 	int err;
 
-	if (!port->removed && port->state.linkup == linkup)
-		return;
-
 	port->changed = true;
 	port->state.linkup = linkup;
 	team_refresh_port_linkup(port);
@@ -2282,6 +2281,23 @@ send_event:
 
 }
 
+static void __team_port_change_check(struct team_port *port, bool linkup)
+{
+	if (port->state.linkup != linkup)
+		__team_port_change_send(port, linkup);
+}
+
+static void __team_port_change_port_added(struct team_port *port, bool linkup)
+{
+	__team_port_change_send(port, linkup);
+}
+
+static void __team_port_change_port_removed(struct team_port *port)
+{
+	port->removed = true;
+	__team_port_change_send(port, false);
+}
+
 static void team_port_change_check(struct team_port *port, bool linkup)
 {
 	struct team *team = port->team;
-- 
1.7.11.4

^ permalink raw reply related

* Re: [patch net-next] team: send port changed when added
From: Jiri Pirko @ 2012-09-22 17:04 UTC (permalink / raw)
  To: David Miller; +Cc: netdev
In-Reply-To: <20120921.150142.1121078984466626799.davem@davemloft.net>

Fri, Sep 21, 2012 at 09:01:42PM CEST, davem@davemloft.net wrote:
>From: Jiri Pirko <jiri@resnulli.us>
>Date: Fri, 21 Sep 2012 13:51:59 +0200
>
>> On some hw, link is not up during adding iface to team. That causes event
>> not being sent to userspace and that may cause confusion.
>> Fix this bug by sending port changed event once it's added to team.
>> 
>> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>
>Applied, thanks a lot Jiri.

David, I just realized this would be good to have fixed in net tree as
well. Unfortunately this patch does not apply cleanly. I will send you
backported patch for net tree in a jiffy.

Thanks!

Jiri

^ permalink raw reply

* [PATCH V3 net-next 4/4] ptp: clarify the clock_name sysfs attribute
From: Richard Cochran @ 2012-09-22 17:02 UTC (permalink / raw)
  To: netdev
  Cc: Ben Hutchings, David Miller, Jacob Keller, Jeff Kirsher,
	John Stultz, Matthew Vick
In-Reply-To: <cover.1348332940.git.richardcochran@gmail.com>

There has been some confusion among PHC driver authors about the
intended purpose of the clock_name attribute. This patch expands the
documation in order to clarify how the clock_name field should be
understood.

Signed-off-by: Richard Cochran <richardcochran@gmail.com>
---
 Documentation/ABI/testing/sysfs-ptp |    6 +++++-
 include/linux/ptp_clock_kernel.h    |    4 +++-
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-ptp b/Documentation/ABI/testing/sysfs-ptp
index d40d2b5..05aeedf 100644
--- a/Documentation/ABI/testing/sysfs-ptp
+++ b/Documentation/ABI/testing/sysfs-ptp
@@ -19,7 +19,11 @@ Date:		September 2010
 Contact:	Richard Cochran <richardcochran@gmail.com>
 Description:
 		This file contains the name of the PTP hardware clock
-		as a human readable string.
+		as a human readable string. The purpose of this
+		attribute is to provide the user with a "friendly
+		name" and to help distinguish PHY based devices from
+		MAC based ones. The string does not necessarily have
+		to be any kind of unique id.
 
 What:		/sys/class/ptp/ptpN/max_adjustment
 Date:		September 2010
diff --git a/include/linux/ptp_clock_kernel.h b/include/linux/ptp_clock_kernel.h
index 56c71b2..f2dc6d8 100644
--- a/include/linux/ptp_clock_kernel.h
+++ b/include/linux/ptp_clock_kernel.h
@@ -42,7 +42,9 @@ struct ptp_clock_request {
  * struct ptp_clock_info - decribes a PTP hardware clock
  *
  * @owner:     The clock driver should set to THIS_MODULE.
- * @name:      A short name to identify the clock.
+ * @name:      A short "friendly name" to identify the clock and to
+ *             help distinguish PHY based devices from MAC based ones.
+ *             The string is not meant to be a unique id.
  * @max_adj:   The maximum possible frequency adjustment, in parts per billon.
  * @n_alarm:   The number of programmable alarms.
  * @n_ext_ts:  The number of external time stamp channels.
-- 
1.7.2.5

^ permalink raw reply related

* [PATCH V3 net-next 3/4] ptp: link the phc device to its parent device
From: Richard Cochran @ 2012-09-22 17:02 UTC (permalink / raw)
  To: netdev
  Cc: Ben Hutchings, David Miller, Jacob Keller, Jeff Kirsher,
	John Stultz, Matthew Vick
In-Reply-To: <cover.1348332940.git.richardcochran@gmail.com>

PTP Hardware Clock devices appear as class devices in sysfs. This patch
changes the registration API to use the parent device, clarifying the
clock's relationship to the underlying device.

Signed-off-by: Richard Cochran <richardcochran@gmail.com>
---
 drivers/net/ethernet/freescale/gianfar_ptp.c |    2 +-
 drivers/net/ethernet/intel/igb/igb_ptp.c     |    3 ++-
 drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c |    3 ++-
 drivers/net/ethernet/sfc/ptp.c               |    3 ++-
 drivers/net/phy/dp83640.c                    |    2 +-
 drivers/ptp/ptp_clock.c                      |    5 +++--
 drivers/ptp/ptp_ixp46x.c                     |    2 +-
 drivers/ptp/ptp_pch.c                        |    2 +-
 include/linux/ptp_clock_kernel.h             |    7 +++++--
 9 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/freescale/gianfar_ptp.c b/drivers/net/ethernet/freescale/gianfar_ptp.c
index c08e5d4..18762a3 100644
--- a/drivers/net/ethernet/freescale/gianfar_ptp.c
+++ b/drivers/net/ethernet/freescale/gianfar_ptp.c
@@ -510,7 +510,7 @@ static int gianfar_ptp_probe(struct platform_device *dev)
 
 	spin_unlock_irqrestore(&etsects->lock, flags);
 
-	etsects->clock = ptp_clock_register(&etsects->caps);
+	etsects->clock = ptp_clock_register(&etsects->caps, &dev->dev);
 	if (IS_ERR(etsects->clock)) {
 		err = PTR_ERR(etsects->clock);
 		goto no_clock;
diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c
index e13ba1d..ee21445 100644
--- a/drivers/net/ethernet/intel/igb/igb_ptp.c
+++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
@@ -752,7 +752,8 @@ void igb_ptp_init(struct igb_adapter *adapter)
 		wr32(E1000_IMS, E1000_IMS_TS);
 	}
 
-	adapter->ptp_clock = ptp_clock_register(&adapter->ptp_caps);
+	adapter->ptp_clock = ptp_clock_register(&adapter->ptp_caps,
+						&adapter->pdev->dev);
 	if (IS_ERR(adapter->ptp_clock)) {
 		adapter->ptp_clock = NULL;
 		dev_err(&adapter->pdev->dev, "ptp_clock_register failed\n");
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c
index 3456d56..39881cb 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c
@@ -960,7 +960,8 @@ void ixgbe_ptp_init(struct ixgbe_adapter *adapter)
 	/* (Re)start the overflow check */
 	adapter->flags2 |= IXGBE_FLAG2_OVERFLOW_CHECK_ENABLED;
 
-	adapter->ptp_clock = ptp_clock_register(&adapter->ptp_caps);
+	adapter->ptp_clock = ptp_clock_register(&adapter->ptp_caps,
+						&adapter->pdev->dev);
 	if (IS_ERR(adapter->ptp_clock)) {
 		adapter->ptp_clock = NULL;
 		e_dev_err("ptp_clock_register failed\n");
diff --git a/drivers/net/ethernet/sfc/ptp.c b/drivers/net/ethernet/sfc/ptp.c
index 2b07a4e..5b3dd02 100644
--- a/drivers/net/ethernet/sfc/ptp.c
+++ b/drivers/net/ethernet/sfc/ptp.c
@@ -931,7 +931,8 @@ static int efx_ptp_probe_channel(struct efx_channel *channel)
 	ptp->phc_clock_info.settime = efx_phc_settime;
 	ptp->phc_clock_info.enable = efx_phc_enable;
 
-	ptp->phc_clock = ptp_clock_register(&ptp->phc_clock_info);
+	ptp->phc_clock = ptp_clock_register(&ptp->phc_clock_info,
+					    &efx->pci_dev->dev);
 	if (!ptp->phc_clock)
 		goto fail3;
 
diff --git a/drivers/net/phy/dp83640.c b/drivers/net/phy/dp83640.c
index b0da022..24e05c4 100644
--- a/drivers/net/phy/dp83640.c
+++ b/drivers/net/phy/dp83640.c
@@ -980,7 +980,7 @@ static int dp83640_probe(struct phy_device *phydev)
 
 	if (choose_this_phy(clock, phydev)) {
 		clock->chosen = dp83640;
-		clock->ptp_clock = ptp_clock_register(&clock->caps);
+		clock->ptp_clock = ptp_clock_register(&clock->caps, &phydev->dev);
 		if (IS_ERR(clock->ptp_clock)) {
 			err = PTR_ERR(clock->ptp_clock);
 			goto no_register;
diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c
index 6f7009a..b15a376 100644
--- a/drivers/ptp/ptp_clock.c
+++ b/drivers/ptp/ptp_clock.c
@@ -186,7 +186,8 @@ static void delete_ptp_clock(struct posix_clock *pc)
 
 /* public interface */
 
-struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info)
+struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info,
+				     struct device *parent)
 {
 	struct ptp_clock *ptp;
 	int err = 0, index, major = MAJOR(ptp_devt);
@@ -219,7 +220,7 @@ struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info)
 	init_waitqueue_head(&ptp->tsev_wq);
 
 	/* Create a new device in our class. */
-	ptp->dev = device_create(ptp_class, NULL, ptp->devid, ptp,
+	ptp->dev = device_create(ptp_class, parent, ptp->devid, ptp,
 				 "ptp%d", ptp->index);
 	if (IS_ERR(ptp->dev))
 		goto no_device;
diff --git a/drivers/ptp/ptp_ixp46x.c b/drivers/ptp/ptp_ixp46x.c
index e03c406..d49b851 100644
--- a/drivers/ptp/ptp_ixp46x.c
+++ b/drivers/ptp/ptp_ixp46x.c
@@ -298,7 +298,7 @@ static int __init ptp_ixp_init(void)
 
 	ixp_clock.caps = ptp_ixp_caps;
 
-	ixp_clock.ptp_clock = ptp_clock_register(&ixp_clock.caps);
+	ixp_clock.ptp_clock = ptp_clock_register(&ixp_clock.caps, NULL);
 
 	if (IS_ERR(ixp_clock.ptp_clock))
 		return PTR_ERR(ixp_clock.ptp_clock);
diff --git a/drivers/ptp/ptp_pch.c b/drivers/ptp/ptp_pch.c
index 3a9c17e..e624e4d 100644
--- a/drivers/ptp/ptp_pch.c
+++ b/drivers/ptp/ptp_pch.c
@@ -627,7 +627,7 @@ pch_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	}
 
 	chip->caps = ptp_pch_caps;
-	chip->ptp_clock = ptp_clock_register(&chip->caps);
+	chip->ptp_clock = ptp_clock_register(&chip->caps, &pdev->dev);
 
 	if (IS_ERR(chip->ptp_clock))
 		return PTR_ERR(chip->ptp_clock);
diff --git a/include/linux/ptp_clock_kernel.h b/include/linux/ptp_clock_kernel.h
index a644b29..56c71b2 100644
--- a/include/linux/ptp_clock_kernel.h
+++ b/include/linux/ptp_clock_kernel.h
@@ -21,6 +21,7 @@
 #ifndef _PTP_CLOCK_KERNEL_H_
 #define _PTP_CLOCK_KERNEL_H_
 
+#include <linux/device.h>
 #include <linux/pps_kernel.h>
 #include <linux/ptp_clock.h>
 
@@ -93,10 +94,12 @@ struct ptp_clock;
 /**
  * ptp_clock_register() - register a PTP hardware clock driver
  *
- * @info:  Structure describing the new clock.
+ * @info:   Structure describing the new clock.
+ * @parent: Pointer to the parent device of the new clock.
  */
 
-extern struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info);
+extern struct ptp_clock *ptp_clock_register(struct ptp_clock_info *info,
+					    struct device *parent);
 
 /**
  * ptp_clock_unregister() - unregister a PTP hardware clock driver
-- 
1.7.2.5

^ permalink raw reply related

* [PATCH V3 net-next 2/4] ptp: provide the clock's adjusted frequency
From: Richard Cochran @ 2012-09-22 17:02 UTC (permalink / raw)
  To: netdev
  Cc: Ben Hutchings, David Miller, Jacob Keller, Jeff Kirsher,
	John Stultz, Matthew Vick
In-Reply-To: <cover.1348332940.git.richardcochran@gmail.com>

If the timex.mode field indicates a query, then we provide the value of
the current frequency adjustment.

Signed-off-by: Richard Cochran <richardcochran@gmail.com>
---
 drivers/ptp/ptp_clock.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c
index 67e628e..6f7009a 100644
--- a/drivers/ptp/ptp_clock.c
+++ b/drivers/ptp/ptp_clock.c
@@ -148,6 +148,11 @@ static int ptp_clock_adjtime(struct posix_clock *pc, struct timex *tx)
 
 		err = ops->adjfreq(ops, scaled_ppm_to_ppb(tx->freq));
 		ptp->dialed_frequency = tx->freq;
+
+	} else if (tx->modes == 0) {
+
+		tx->freq = ptp->dialed_frequency;
+		err = 0;
 	}
 
 	return err;
-- 
1.7.2.5

^ permalink raw reply related

* [PATCH V3 net-next 1/4] ptp: remember the adjusted frequency
From: Richard Cochran @ 2012-09-22 17:02 UTC (permalink / raw)
  To: netdev
  Cc: Ben Hutchings, David Miller, Jacob Keller, Jeff Kirsher,
	John Stultz, Matthew Vick
In-Reply-To: <cover.1348332940.git.richardcochran@gmail.com>

This patch adds a field to the representation of a PTP hardware clock in
order to remember the frequency adjustment value dialed by the user.

Adding this field will let us answer queries in the manner of adjtimex
in a follow on patch.

Signed-off-by: Richard Cochran <richardcochran@gmail.com>
---
 drivers/ptp/ptp_clock.c   |    1 +
 drivers/ptp/ptp_private.h |    1 +
 2 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c
index 966875d..67e628e 100644
--- a/drivers/ptp/ptp_clock.c
+++ b/drivers/ptp/ptp_clock.c
@@ -147,6 +147,7 @@ static int ptp_clock_adjtime(struct posix_clock *pc, struct timex *tx)
 	} else if (tx->modes & ADJ_FREQUENCY) {
 
 		err = ops->adjfreq(ops, scaled_ppm_to_ppb(tx->freq));
+		ptp->dialed_frequency = tx->freq;
 	}
 
 	return err;
diff --git a/drivers/ptp/ptp_private.h b/drivers/ptp/ptp_private.h
index 4d5b508..69d3207 100644
--- a/drivers/ptp/ptp_private.h
+++ b/drivers/ptp/ptp_private.h
@@ -45,6 +45,7 @@ struct ptp_clock {
 	dev_t devid;
 	int index; /* index into clocks.map */
 	struct pps_device *pps_source;
+	long dialed_frequency; /* remembers the frequency adjustment */
 	struct timestamp_event_queue tsevq; /* simple fifo for time stamps */
 	struct mutex tsevq_mux; /* one process at a time reading the fifo */
 	wait_queue_head_t tsev_wq;
-- 
1.7.2.5

^ permalink raw reply related

* [PATCH V3 net-next 0/4] Two new PTP Hardware Clock features
From: Richard Cochran @ 2012-09-22 17:02 UTC (permalink / raw)
  To: netdev
  Cc: Ben Hutchings, David Miller, Jacob Keller, Jeff Kirsher,
	John Stultz, Matthew Vick

This patch series adds two new features to the PHC code.

* ChangeLog
** V3
   - Use the correct parent device in the solarflare driver.
   - Expand the sysfs documentation of clock_name.
   - Also document the clock_name field in the header file.
** V2 
   - Preserves the clock_name attribute as it was meant to be, instead
     of making any changes to it.
   - Covers the registration API change in the brand new solarflare phc
     device, which was overlooked in V1.

The first two patches let a user program find out the previously
dialed frequency adjustment. This is primarily useful when restarting
a PTP service, since without this information, the presumably correct
adjustment will bias the new frequency estimation.

The third patch links the phc class device to its parent device within
the driver model and sysfs.

The fourth patch adds a bit more documentation of the sysfs clock_name
attribute. This should help clarify the naming scheme.

Thanks,
Richard


Richard Cochran (4):
  ptp: remember the adjusted frequency
  ptp: provide the clock's adjusted frequency
  ptp: link the phc device to its parent device
  ptp: clarify the clock_name sysfs attribute

 Documentation/ABI/testing/sysfs-ptp          |    6 +++++-
 drivers/net/ethernet/freescale/gianfar_ptp.c |    2 +-
 drivers/net/ethernet/intel/igb/igb_ptp.c     |    3 ++-
 drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c |    3 ++-
 drivers/net/ethernet/sfc/ptp.c               |    3 ++-
 drivers/net/phy/dp83640.c                    |    2 +-
 drivers/ptp/ptp_clock.c                      |   11 +++++++++--
 drivers/ptp/ptp_ixp46x.c                     |    2 +-
 drivers/ptp/ptp_pch.c                        |    2 +-
 drivers/ptp/ptp_private.h                    |    1 +
 include/linux/ptp_clock_kernel.h             |   11 ++++++++---
 11 files changed, 33 insertions(+), 13 deletions(-)

-- 
1.7.2.5

^ permalink raw reply

* Re: [PATCH net-next v2] Take care of xfrm policy when checking dst entries
From: Jan Engelhardt @ 2012-09-22 16:49 UTC (permalink / raw)
  To: David Miller
  Cc: vyasevich, nicolas.dichtel, eric.dumazet, sri, linux-sctp, netdev
In-Reply-To: <20120910.140105.1099041309239526456.davem@davemloft.net>


On Monday 2012-09-10 20:01, David Miller wrote:
>> 
>> So you are perfectly ok with invalidating IPv6 cache when IPv4 table
>> changes, but not invalidating IPv4 cache if IPv6 table changes?
>
>Due to tunneling I can't see how this is avoidable?
>
>We do ipv6 over ipv4, but not vice-versa.

I have a setup here where 6 machines are connected with one another 
(most of them) to form 9 IPsec sessions, all of which are ESP6 links - 
since native IPv6 is provided - that also handle the site-to-site IPv4 
traffic. Does that count?

^ permalink raw reply

* Re: [PATCH v2] lxt PHY: Support for the buggy LXT973 rev A2
From: leroy christophe @ 2012-09-22 16:21 UTC (permalink / raw)
  To: Lutz Jaenicke; +Cc: David S Miller, netdev, linux-kernel
In-Reply-To: <20120910181747.GA5960@lutz.bln.innominate.local>


Le 10/09/2012 20:17, Lutz Jaenicke a écrit :
> On Mon, Sep 10, 2012 at 05:45:49PM +0200, Christophe Leroy wrote:
>> This patch adds proper handling of the buggy revision A2 of LXT973 phy, adding
>> precautions linked to ERRATA Item 4:
>>
>> Item 4: MDIO Interface and Repeated Polling
>> Problem: Repeated polling of odd-numbered registers via the MDIO interface
>> 	randomly returns the contents of the previous even register.
>> Implication: Managed applications may not obtain the correct register contents
>> 	when a particular register is monitored for device status.
>> Workaround: None.
>> Status: This erratum has been previously fixed (in rev A3)
> Hmm. Were did you get the hardware from? We have been using LXT973 in
> our products and the A2 was replaced by A3 in 2003.
>
> Best regards,
> 	Lutz
They are custom boards that my company manufactures. Most boards 
manufactured before 2006 or 2007 have this buggy chip.

Regards
Christophe

^ permalink raw reply

* [PATCH v3] lxt PHY: Support for the buggy LXT973 rev A2
From: Christophe Leroy @ 2012-09-22 16:16 UTC (permalink / raw)
  To: David S Miller, Richard Cochran; +Cc: netdev, linux-kernel

This patch adds proper handling of the buggy revision A2 of LXT973 phy, adding
precautions linked to ERRATA Item 4:

Revision A2 of LXT973 chip randomly returns the contents of the previous even
register when you read a odd register regularly

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>

diff -u linux-3.5-vanilla/drivers/net/phy/lxt.c linux-3.5/drivers/net/phy/lxt.c
--- linux-3.5-vanilla/drivers/net/phy/lxt.c	2012-07-21 22:58:29.000000000 +0200
+++ linux-3.5/drivers/net/phy/lxt.c	2012-09-15 19:20:34.000000000 +0200
@@ -54,8 +54,12 @@
 #define MII_LXT971_ISR		19  /* Interrupt Status Register */
 
 /* register definitions for the 973 */
-#define MII_LXT973_PCR 16 /* Port Configuration Register */
+#define MII_LXT973_PCR		16 /* Port Configuration Register */
 #define PCR_FIBER_SELECT 1
+#define MII_LXT973_SFR		27  /* Special Function Register */
+
+#define PHYDEV_PRIV_FIBER	1
+#define PHYDEV_PRIV_REVA2	2
 
 MODULE_DESCRIPTION("Intel LXT PHY driver");
 MODULE_AUTHOR("Andy Fleming");
@@ -99,6 +103,9 @@
 	return err;
 }
 
+/* register definitions for the 973 */
+#define MII_LXT973_PCR 16 /* Port Configuration Register */
+#define PCR_FIBER_SELECT 1
 
 static int lxt971_ack_interrupt(struct phy_device *phydev)
 {
@@ -122,9 +129,138 @@
 	return err;
 }
 
+/*
+ * A2 version of LXT973 chip has an ERRATA: it randomly return the contents
+ * of the previous even register when you read a odd register regularly
+ */
+
+static int lxt973a2_update_link(struct phy_device *phydev)
+{
+	int status;
+	int control;
+	int retry = 8; /* we try 8 times */
+
+	/* Do a fake read */
+	status = phy_read(phydev, MII_BMSR);
+
+	if (status < 0)
+		return status;
+
+	control = phy_read(phydev, MII_BMCR);
+	if (control < 0)
+		return control;
+
+	do {
+		/* Read link and autonegotiation status */
+		status = phy_read(phydev, MII_BMSR);
+	} while (status >= 0 && retry-- && status == control);
+
+	if (status < 0)
+		return status;
+
+	if ((status & BMSR_LSTATUS) == 0)
+		phydev->link = 0;
+	else
+		phydev->link = 1;
+
+	return 0;
+}
+
+int lxt973a2_read_status(struct phy_device *phydev)
+{
+	int adv;
+	int err;
+	int lpa;
+	int lpagb = 0;
+
+	/* Update the link, but return if there was an error */
+	err = lxt973a2_update_link(phydev);
+	if (err)
+		return err;
+
+	if (AUTONEG_ENABLE == phydev->autoneg) {
+		int retry = 1;
+
+		adv = phy_read(phydev, MII_ADVERTISE);
+
+		if (adv < 0)
+			return adv;
+
+		do {
+			lpa = phy_read(phydev, MII_LPA);
+
+			if (lpa < 0)
+				return lpa;
+
+			/* If both registers are equal, it is suspect but not
+			* impossible, hence a new try
+			*/
+		} while (lpa == adv && retry--);
+
+		lpa &= adv;
+
+		phydev->speed = SPEED_10;
+		phydev->duplex = DUPLEX_HALF;
+		phydev->pause = phydev->asym_pause = 0;
+
+		if (lpagb & (LPA_1000FULL | LPA_1000HALF)) {
+			phydev->speed = SPEED_1000;
+
+			if (lpagb & LPA_1000FULL)
+				phydev->duplex = DUPLEX_FULL;
+		} else if (lpa & (LPA_100FULL | LPA_100HALF)) {
+			phydev->speed = SPEED_100;
+
+			if (lpa & LPA_100FULL)
+				phydev->duplex = DUPLEX_FULL;
+		} else
+			if (lpa & LPA_10FULL)
+				phydev->duplex = DUPLEX_FULL;
+
+		if (phydev->duplex == DUPLEX_FULL) {
+			phydev->pause = lpa & LPA_PAUSE_CAP ? 1 : 0;
+			phydev->asym_pause = lpa & LPA_PAUSE_ASYM ? 1 : 0;
+		}
+	} else {
+		int bmcr = phy_read(phydev, MII_BMCR);
+
+		if (bmcr < 0)
+			return bmcr;
+
+		if (bmcr & BMCR_FULLDPLX)
+			phydev->duplex = DUPLEX_FULL;
+		else
+			phydev->duplex = DUPLEX_HALF;
+
+		if (bmcr & BMCR_SPEED1000)
+			phydev->speed = SPEED_1000;
+		else if (bmcr & BMCR_SPEED100)
+			phydev->speed = SPEED_100;
+		else
+			phydev->speed = SPEED_10;
+
+		phydev->pause = phydev->asym_pause = 0;
+	}
+
+	return 0;
+}
+
+static int lxt973_read_status(struct phy_device *phydev)
+{
+	return (int)phydev->priv&PHYDEV_PRIV_REVA2 ?
+			lxt973a2_read_status(phydev) :
+			genphy_read_status(phydev);
+}
+
 static int lxt973_probe(struct phy_device *phydev)
 {
 	int val = phy_read(phydev, MII_LXT973_PCR);
+	int priv = 0;
+
+	phydev->priv = NULL;
+
+	if (val < 0)
+		return val;
 
 	if (val & PCR_FIBER_SELECT) {
 		/*
@@ -136,17 +272,26 @@
 		val &= ~BMCR_ANENABLE;
 		phy_write(phydev, MII_BMCR, val);
 		/* Remember that the port is in fiber mode. */
-		phydev->priv = lxt973_probe;
-	} else {
-		phydev->priv = NULL;
+		priv |= PHYDEV_PRIV_FIBER;
+	}
+	val = phy_read(phydev, MII_PHYSID2);
+
+	if (val < 0)
+		return val;
+
+	if ((val & 0xf) == 0) { /* rev A2 */
+		dev_info(&phydev->dev, " LXT973 revision A2 has bugs\n");
+		priv |= PHYDEV_PRIV_REVA2;
 	}
+	phydev->priv = (void *)priv;
 	return 0;
 }
 
 static int lxt973_config_aneg(struct phy_device *phydev)
 {
 	/* Do nothing if port is in fiber mode. */
-	return phydev->priv ? 0 : genphy_config_aneg(phydev);
+	return (int)phydev->priv&PHYDEV_PRIV_FIBER ?
+			0 : genphy_config_aneg(phydev);
 }
 
 static struct phy_driver lxt970_driver = {
@@ -184,7 +329,10 @@
 	.flags		= 0,
 	.probe		= lxt973_probe,
 	.config_aneg	= lxt973_config_aneg,
-	.read_status	= genphy_read_status,
+	.read_status	= lxt973_read_status,
+	.suspend	= genphy_suspend,
+	.resume		= genphy_resume,
+	.isolate	= genphy_isolate,
 	.driver 	= { .owner = THIS_MODULE,},
 };
 

^ permalink raw reply

* Re: [PATCH V2 net-next 4/4] ptp: clarify the clock_name sysfs attribute
From: Jeff Kirsher @ 2012-09-22 16:03 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: Richard Cochran, netdev, David Miller, Jacob Keller, Jeff Kirsher,
	John Stultz, Matthew Vick
In-Reply-To: <1348328688.2521.81.camel@bwh-desktop.uk.solarflarecom.com>

On 09/22/2012 08:44 AM, Ben Hutchings wrote:
> On Sat, 2012-09-22 at 09:42 +0200, Richard Cochran wrote:
>> There has been some confusion among PHC driver authors about the
>> intended purpose of the clock_name attribute. This patch expands the
>> documation in order to clarify how the clock_name field should be
>> understood.
>>
>> Signed-off-by: Richard Cochran <richardcochran@gmail.com>
>> ---
>>   Documentation/ABI/testing/sysfs-ptp |    5 ++++-
>>   1 files changed, 4 insertions(+), 1 deletions(-)
>>
>> diff --git a/Documentation/ABI/testing/sysfs-ptp b/Documentation/ABI/testing/sysfs-ptp
>> index d40d2b5..c906488 100644
>> --- a/Documentation/ABI/testing/sysfs-ptp
>> +++ b/Documentation/ABI/testing/sysfs-ptp
>> @@ -19,7 +19,10 @@ Date:		September 2010
>>   Contact:	Richard Cochran <richardcochran@gmail.com>
>>   Description:
>>   		This file contains the name of the PTP hardware clock
>> -		as a human readable string.
>> +		as a human readable string. The purpose of this
>> +		attribute is to provide the user with a "friendly
>> +		name" and to help distinguish PHY based devices from
>> +		MAC based ones.
> Might also be worth saying that it is not required to be unique.  And
> this explanation should also go in the kernel-doc in ptp_clock_kernel.h,
> which is what driver writers are most likely to read.
>
> Ben
FWIW, I agree with Ben.

^ permalink raw reply

* Re: [PATCH v4] can: kvaser_usb: Add support for Kvaser CAN/USB devices
From: Wolfgang Grandegger @ 2012-09-22 16:02 UTC (permalink / raw)
  To: Marc Kleine-Budde
  Cc: Olivier Sobrie, linux-can, Kvaser Support, netdev, linux-usb
In-Reply-To: <505C395C.6000504@pengutronix.de>

On 09/21/2012 11:54 AM, Marc Kleine-Budde wrote:
> On 09/20/2012 07:06 AM, Olivier Sobrie wrote:
>> This driver provides support for several Kvaser CAN/USB devices.
>> Such kind of devices supports up to three CAN network interfaces.
>>
>> It has been tested with a Kvaser USB Leaf Light (one network interface)
>> connected to a pch_can interface.
>> The firmware version of the Kvaser device was 2.5.205.
> 
> I don't remember, have the USB people already had a look on your driver
> and gave some comments?

IIRC, Oliver Neukum commented on v2. Actually we ignored v3 :(, sorry!

> From the CAN and network point of view looks good, some comments inline.
> Would be fine, if Wolfgang can give comments or Ack about the error
> frame generation.

Olivier already sent candump traces for no cable connected and
short-circuiting CAN high and low. The error handling looked good, IIRC,
at least as good as the firmware can do... more comments inline...

>> List of Kvaser devices supported by the driver:
>>   - Kvaser Leaf prototype (P010v2 and v3)
>>   - Kvaser Leaf Light (P010v3)
>>   - Kvaser Leaf Professional HS
>>   - Kvaser Leaf SemiPro HS
>>   - Kvaser Leaf Professional LS
>>   - Kvaser Leaf Professional SWC
>>   - Kvaser Leaf Professional LIN
>>   - Kvaser Leaf SemiPro LS
>>   - Kvaser Leaf SemiPro SWC
>>   - Kvaser Memorator II, Prototype
>>   - Kvaser Memorator II HS/HS
>>   - Kvaser USBcan Professional HS/HS
>>   - Kvaser Leaf Light GI
>>   - Kvaser Leaf Professional HS (OBD-II connector)
>>   - Kvaser Memorator Professional HS/LS
>>   - Kvaser Leaf Light "China"
>>   - Kvaser BlackBird SemiPro
>>   - Kvaser OEM Mercury
>>   - Kvaser OEM Leaf
>>   - Kvaser USBcan R
>>
>> Signed-off-by: Olivier Sobrie <olivier@sobrie.be>
>> ---
>> Changes since v3:
>>  - add support for CMD_LOG_MESSAGE
>>
>>  drivers/net/can/usb/Kconfig      |   33 +
>>  drivers/net/can/usb/Makefile     |    1 +
>>  drivers/net/can/usb/kvaser_usb.c | 1555 ++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 1589 insertions(+)
>>  create mode 100644 drivers/net/can/usb/kvaser_usb.c
>>
>> diff --git a/drivers/net/can/usb/Kconfig b/drivers/net/can/usb/Kconfig
>> index 0a68768..578955f 100644
>> --- a/drivers/net/can/usb/Kconfig
>> +++ b/drivers/net/can/usb/Kconfig
>> @@ -13,6 +13,39 @@ config CAN_ESD_USB2
>>            This driver supports the CAN-USB/2 interface
>>            from esd electronic system design gmbh (http://www.esd.eu).
>>  
>> +config CAN_KVASER_USB
>> +	tristate "Kvaser CAN/USB interface"
>> +	---help---
>> +	  This driver adds support for Kvaser CAN/USB devices like Kvaser
>> +	  Leaf Light.
>> +
>> +	  The driver gives support for the following devices:
>> +	    - Kvaser Leaf prototype (P010v2 and v3)
>> +	    - Kvaser Leaf Light (P010v3)
>> +	    - Kvaser Leaf Professional HS
>> +	    - Kvaser Leaf SemiPro HS
>> +	    - Kvaser Leaf Professional LS
>> +	    - Kvaser Leaf Professional SWC
>> +	    - Kvaser Leaf Professional LIN
>> +	    - Kvaser Leaf SemiPro LS
>> +	    - Kvaser Leaf SemiPro SWC
>> +	    - Kvaser Memorator II, Prototype
>> +	    - Kvaser Memorator II HS/HS
>> +	    - Kvaser USBcan Professional HS/HS
>> +	    - Kvaser Leaf Light GI
>> +	    - Kvaser Leaf Professional HS (OBD-II connector)
>> +	    - Kvaser Memorator Professional HS/LS
>> +	    - Kvaser Leaf Light "China"
>> +	    - Kvaser BlackBird SemiPro
>> +	    - Kvaser OEM Mercury
>> +	    - Kvaser OEM Leaf
>> +	    - Kvaser USBcan R
>> +
>> +	  If unsure, say N.
>> +
>> +	  To compile this driver as a module, choose M here: the
>> +	  module will be called kvaser_usb.
>> +
>>  config CAN_PEAK_USB
>>  	tristate "PEAK PCAN-USB/USB Pro interfaces"
>>  	---help---
>> diff --git a/drivers/net/can/usb/Makefile b/drivers/net/can/usb/Makefile
>> index da6d1d3..80a2ee4 100644
>> --- a/drivers/net/can/usb/Makefile
>> +++ b/drivers/net/can/usb/Makefile
>> @@ -4,6 +4,7 @@
>>  
>>  obj-$(CONFIG_CAN_EMS_USB) += ems_usb.o
>>  obj-$(CONFIG_CAN_ESD_USB2) += esd_usb2.o
>> +obj-$(CONFIG_CAN_KVASER_USB) += kvaser_usb.o
>>  obj-$(CONFIG_CAN_PEAK_USB) += peak_usb/
>>  
>>  ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG
>> diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
>> new file mode 100644
>> index 0000000..3509ca5
>> --- /dev/null
>> +++ b/drivers/net/can/usb/kvaser_usb.c
>> @@ -0,0 +1,1555 @@
>> +/*
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License as
>> + * published by the Free Software Foundation version 2.
>> + *
>> + * Parts of this driver are based on the following:
>> + *  - Kvaser linux leaf driver (version 4.78)
>> + *  - CAN driver for esd CAN-USB/2
>> + *
>> + * Copyright (C) 2002-2006 KVASER AB, Sweden. All rights reserved.
>> + * Copyright (C) 2010 Matthias Fuchs <matthias.fuchs@esd.eu>, esd gmbh
>> + * Copyright (C) 2012 Olivier Sobrie <olivier@sobrie.be>
>> + */
>> +
>> +#include <linux/init.h>
>> +#include <linux/completion.h>
>> +#include <linux/module.h>
>> +#include <linux/netdevice.h>
>> +#include <linux/usb.h>
>> +
>> +#include <linux/can.h>
>> +#include <linux/can/dev.h>
>> +#include <linux/can/error.h>
>> +
>> +#define MAX_TX_URBS			16
>> +#define MAX_RX_URBS			4
>> +#define START_TIMEOUT			1000 /* msecs */
>> +#define STOP_TIMEOUT			1000 /* msecs */
>> +#define USB_SEND_TIMEOUT		1000 /* msecs */
>> +#define USB_RECV_TIMEOUT		1000 /* msecs */
>> +#define RX_BUFFER_SIZE			3072
>> +#define CAN_USB_CLOCK			8000000
>> +#define MAX_NET_DEVICES			3
>> +
>> +/* Kvaser USB devices */
>> +#define KVASER_VENDOR_ID		0x0bfd
>> +#define USB_LEAF_DEVEL_PRODUCT_ID	10
>> +#define USB_LEAF_LITE_PRODUCT_ID	11
>> +#define USB_LEAF_PRO_PRODUCT_ID		12
>> +#define USB_LEAF_SPRO_PRODUCT_ID	14
>> +#define USB_LEAF_PRO_LS_PRODUCT_ID	15
>> +#define USB_LEAF_PRO_SWC_PRODUCT_ID	16
>> +#define USB_LEAF_PRO_LIN_PRODUCT_ID	17
>> +#define USB_LEAF_SPRO_LS_PRODUCT_ID	18
>> +#define USB_LEAF_SPRO_SWC_PRODUCT_ID	19
>> +#define USB_MEMO2_DEVEL_PRODUCT_ID	22
>> +#define USB_MEMO2_HSHS_PRODUCT_ID	23
>> +#define USB_UPRO_HSHS_PRODUCT_ID	24
>> +#define USB_LEAF_LITE_GI_PRODUCT_ID	25
>> +#define USB_LEAF_PRO_OBDII_PRODUCT_ID	26
>> +#define USB_MEMO2_HSLS_PRODUCT_ID	27
>> +#define USB_LEAF_LITE_CH_PRODUCT_ID	28
>> +#define USB_BLACKBIRD_SPRO_PRODUCT_ID	29
>> +#define USB_OEM_MERCURY_PRODUCT_ID	34
>> +#define USB_OEM_LEAF_PRODUCT_ID		35
>> +#define USB_CAN_R_PRODUCT_ID		39
>> +
>> +/* USB devices features */
>> +#define KVASER_HAS_SILENT_MODE		BIT(0)
>> +#define KVASER_HAS_TXRX_ERRORS		BIT(1)
>> +
>> +/* Message header size */
>> +#define MSG_HEADER_LEN			2
>> +
>> +/* Can message flags */
>> +#define MSG_FLAG_ERROR_FRAME		BIT(0)
>> +#define MSG_FLAG_OVERRUN		BIT(1)
>> +#define MSG_FLAG_NERR			BIT(2)
>> +#define MSG_FLAG_WAKEUP			BIT(3)
>> +#define MSG_FLAG_REMOTE_FRAME		BIT(4)
>> +#define MSG_FLAG_RESERVED		BIT(5)
>> +#define MSG_FLAG_TX_ACK			BIT(6)
>> +#define MSG_FLAG_TX_REQUEST		BIT(7)
>> +
>> +/* Can states */
>> +#define M16C_STATE_BUS_RESET		BIT(0)
>> +#define M16C_STATE_BUS_ERROR		BIT(4)
>> +#define M16C_STATE_BUS_PASSIVE		BIT(5)
>> +#define M16C_STATE_BUS_OFF		BIT(6)
>> +
>> +/* Can msg ids */
>> +#define CMD_RX_STD_MESSAGE		12
>> +#define CMD_TX_STD_MESSAGE		13
>> +#define CMD_RX_EXT_MESSAGE		14
>> +#define CMD_TX_EXT_MESSAGE		15
>> +#define CMD_SET_BUS_PARAMS		16
>> +#define CMD_GET_BUS_PARAMS		17
>> +#define CMD_GET_BUS_PARAMS_REPLY	18
>> +#define CMD_GET_CHIP_STATE		19
>> +#define CMD_CHIP_STATE_EVENT		20
>> +#define CMD_SET_CTRL_MODE		21
>> +#define CMD_GET_CTRL_MODE		22
>> +#define CMD_GET_CTRL_MODE_REPLY		23
>> +#define CMD_RESET_CHIP			24
>> +#define CMD_RESET_CARD			25
>> +#define CMD_START_CHIP			26
>> +#define CMD_START_CHIP_REPLY		27
>> +#define CMD_STOP_CHIP			28
>> +#define CMD_STOP_CHIP_REPLY		29
>> +#define CMD_GET_CARD_INFO2		32
>> +#define CMD_GET_CARD_INFO		34
>> +#define CMD_GET_CARD_INFO_REPLY		35
>> +#define CMD_GET_SOFTWARE_INFO		38
>> +#define CMD_GET_SOFTWARE_INFO_REPLY	39
>> +#define CMD_ERROR_EVENT			45
>> +#define CMD_FLUSH_QUEUE			48
>> +#define CMD_RESET_ERROR_COUNTER		49
>> +#define CMD_TX_ACKNOWLEDGE		50
>> +#define CMD_CAN_ERROR_EVENT		51
>> +#define CMD_USB_THROTTLE		77
>> +#define CMD_LOG_MESSAGE			106
>> +
>> +/* error factors */
>> +#define M16C_EF_ACKE			BIT(0)
>> +#define M16C_EF_CRCE			BIT(1)
>> +#define M16C_EF_FORME			BIT(2)
>> +#define M16C_EF_STFE			BIT(3)
>> +#define M16C_EF_BITE0			BIT(4)
>> +#define M16C_EF_BITE1			BIT(5)
>> +#define M16C_EF_RCVE			BIT(6)
>> +#define M16C_EF_TRE			BIT(7)
>> +
>> +/* bittiming parameters */
>> +#define KVASER_USB_TSEG1_MIN		1
>> +#define KVASER_USB_TSEG1_MAX		16
>> +#define KVASER_USB_TSEG2_MIN		1
>> +#define KVASER_USB_TSEG2_MAX		8
>> +#define KVASER_USB_SJW_MAX		4
>> +#define KVASER_USB_BRP_MIN		1
>> +#define KVASER_USB_BRP_MAX		64
>> +#define KVASER_USB_BRP_INC		1
>> +
>> +/* ctrl modes */
>> +#define KVASER_CTRL_MODE_NORMAL		1
>> +#define KVASER_CTRL_MODE_SILENT		2
>> +#define KVASER_CTRL_MODE_SELFRECEPTION	3
>> +#define KVASER_CTRL_MODE_OFF		4
>> +
>> +struct kvaser_msg_simple {
>> +	u8 tid;
>> +	u8 channel;
>> +} __packed;
>> +
>> +struct kvaser_msg_cardinfo {
>> +	u8 tid;
>> +	u8 nchannels;
>> +	__le32 serial_number;
>> +	__le32 padding;
>> +	__le32 clock_resolution;
>> +	__le32 mfgdate;
>> +	u8 ean[8];
>> +	u8 hw_revision;
>> +	u8 usb_hs_mode;
>> +	__le16 padding2;
>> +} __packed;
>> +
>> +struct kvaser_msg_cardinfo2 {
>> +	u8 tid;
>> +	u8 channel;
>> +	u8 pcb_id[24];
>> +	__le32 oem_unlock_code;
>> +} __packed;
>> +
>> +struct kvaser_msg_softinfo {
>> +	u8 tid;
>> +	u8 channel;
>> +	__le32 sw_options;
>> +	__le32 fw_version;
>> +	__le16 max_outstanding_tx;
>> +	__le16 padding[9];
>> +} __packed;
>> +
>> +struct kvaser_msg_busparams {
>> +	u8 tid;
>> +	u8 channel;
>> +	__le32 bitrate;
>> +	u8 tseg1;
>> +	u8 tseg2;
>> +	u8 sjw;
>> +	u8 no_samp;
>> +} __packed;
>> +
>> +struct kvaser_msg_tx_can {
>> +	u8 channel;
>> +	u8 tid;
>> +	u8 msg[14];
>> +	u8 padding;
>> +	u8 flags;
>> +} __packed;
>> +
>> +struct kvaser_msg_rx_can {
>> +	u8 channel;
>> +	u8 flag;
>> +	__le16 time[3];
>> +	u8 msg[14];
>> +} __packed;
>> +
>> +struct kvaser_msg_chip_state_event {
>> +	u8 tid;
>> +	u8 channel;
>> +	__le16 time[3];
>> +	u8 tx_errors_count;
>> +	u8 rx_errors_count;
>> +	u8 status;
>> +	u8 padding[3];
>> +} __packed;
>> +
>> +struct kvaser_msg_tx_acknowledge {
>> +	u8 channel;
>> +	u8 tid;
>> +	__le16 time[3];
>> +	u8 flags;
>> +	u8 time_offset;
>> +} __packed;
>> +
>> +struct kvaser_msg_error_event {
>> +	u8 tid;
>> +	u8 flags;
>> +	__le16 time[3];
>> +	u8 channel;
>> +	u8 padding;
>> +	u8 tx_errors_count;
>> +	u8 rx_errors_count;
>> +	u8 status;
>> +	u8 error_factor;
>> +} __packed;
>> +
>> +struct kvaser_msg_ctrl_mode {
>> +	u8 tid;
>> +	u8 channel;
>> +	u8 ctrl_mode;
>> +	u8 padding[3];
>> +} __packed;
>> +
>> +struct kvaser_msg_flush_queue {
>> +	u8 tid;
>> +	u8 channel;
>> +	u8 flags;
>> +	u8 padding[3];
>> +} __packed;
>> +
>> +struct kvaser_msg_log_message {
>> +	u8 channel;
>> +	u8 flags;
>> +	__le16 time[3];
>> +	u8 dlc;
>> +	u8 time_offset;
>> +	__le32 id;
>> +	u8 data[8];
>> +} __packed;
>> +
>> +struct kvaser_msg {
>> +	u8 len;
>> +	u8 id;
>> +	union	{
>> +		struct kvaser_msg_simple simple;
>> +		struct kvaser_msg_cardinfo cardinfo;
>> +		struct kvaser_msg_cardinfo2 cardinfo2;
>> +		struct kvaser_msg_softinfo softinfo;
>> +		struct kvaser_msg_busparams busparams;
>> +		struct kvaser_msg_tx_can tx_can;
>> +		struct kvaser_msg_rx_can rx_can;
>> +		struct kvaser_msg_chip_state_event chip_state_event;
>> +		struct kvaser_msg_tx_acknowledge tx_acknowledge;
>> +		struct kvaser_msg_error_event error_event;
>> +		struct kvaser_msg_ctrl_mode ctrl_mode;
>> +		struct kvaser_msg_flush_queue flush_queue;
>> +		struct kvaser_msg_log_message log_message;
>> +	} u;
>> +} __packed;
>> +
>> +struct kvaser_usb_tx_urb_context {
>> +	struct kvaser_usb_net_priv *priv;
>> +	u32 echo_index;
>> +	int dlc;
>> +};
>> +
>> +struct kvaser_usb {
>> +	struct usb_device *udev;
>> +	struct kvaser_usb_net_priv *nets[MAX_NET_DEVICES];
>> +
>> +	struct usb_endpoint_descriptor *bulk_in, *bulk_out;
>> +	struct usb_anchor rx_submitted;
>> +
>> +	u32 fw_version;
>> +	unsigned int nchannels;
>> +
>> +	bool rxinitdone;
>> +	void *rxbuf[MAX_RX_URBS];
>> +	dma_addr_t rxbuf_dma[MAX_RX_URBS];
>> +};
>> +
>> +struct kvaser_usb_net_priv {
>> +	struct can_priv can;
>> +
>> +	atomic_t active_tx_urbs;
>> +	struct usb_anchor tx_submitted;
>> +	struct kvaser_usb_tx_urb_context tx_contexts[MAX_TX_URBS];
>> +
>> +	struct completion start_comp, stop_comp;
>> +
>> +	struct kvaser_usb *dev;
>> +	struct net_device *netdev;
>> +	int channel;
>> +
>> +	struct can_berr_counter bec;
>> +};
>> +
>> +static struct usb_device_id kvaser_usb_table[] = {
>> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_DEVEL_PRODUCT_ID) },
>> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_LITE_PRODUCT_ID) },
>> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_PRO_PRODUCT_ID),
>> +		.driver_info = KVASER_HAS_TXRX_ERRORS |
>> +			       KVASER_HAS_SILENT_MODE },
>> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_SPRO_PRODUCT_ID),
>> +		.driver_info = KVASER_HAS_TXRX_ERRORS |
>> +			       KVASER_HAS_SILENT_MODE },
>> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_PRO_LS_PRODUCT_ID),
>> +		.driver_info = KVASER_HAS_TXRX_ERRORS |
>> +			       KVASER_HAS_SILENT_MODE },
>> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_PRO_SWC_PRODUCT_ID),
>> +		.driver_info = KVASER_HAS_TXRX_ERRORS |
>> +			       KVASER_HAS_SILENT_MODE },
>> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_PRO_LIN_PRODUCT_ID),
>> +		.driver_info = KVASER_HAS_TXRX_ERRORS |
>> +			       KVASER_HAS_SILENT_MODE },
>> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_SPRO_LS_PRODUCT_ID),
>> +		.driver_info = KVASER_HAS_TXRX_ERRORS |
>> +			       KVASER_HAS_SILENT_MODE },
>> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_SPRO_SWC_PRODUCT_ID),
>> +		.driver_info = KVASER_HAS_TXRX_ERRORS |
>> +			       KVASER_HAS_SILENT_MODE },
>> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_MEMO2_DEVEL_PRODUCT_ID),
>> +		.driver_info = KVASER_HAS_TXRX_ERRORS |
>> +			       KVASER_HAS_SILENT_MODE },
>> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_MEMO2_HSHS_PRODUCT_ID),
>> +		.driver_info = KVASER_HAS_TXRX_ERRORS |
>> +			       KVASER_HAS_SILENT_MODE },
>> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_UPRO_HSHS_PRODUCT_ID),
>> +		.driver_info = KVASER_HAS_TXRX_ERRORS },
>> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_LITE_GI_PRODUCT_ID) },
>> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_PRO_OBDII_PRODUCT_ID),
>> +		.driver_info = KVASER_HAS_TXRX_ERRORS |
>> +			       KVASER_HAS_SILENT_MODE },
>> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_MEMO2_HSLS_PRODUCT_ID),
>> +		.driver_info = KVASER_HAS_TXRX_ERRORS },
>> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_LITE_CH_PRODUCT_ID),
>> +		.driver_info = KVASER_HAS_TXRX_ERRORS },
>> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_BLACKBIRD_SPRO_PRODUCT_ID),
>> +		.driver_info = KVASER_HAS_TXRX_ERRORS },
>> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_OEM_MERCURY_PRODUCT_ID),
>> +		.driver_info = KVASER_HAS_TXRX_ERRORS },
>> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_OEM_LEAF_PRODUCT_ID),
>> +		.driver_info = KVASER_HAS_TXRX_ERRORS },
>> +	{ USB_DEVICE(KVASER_VENDOR_ID, USB_CAN_R_PRODUCT_ID),
>> +		.driver_info = KVASER_HAS_TXRX_ERRORS },
>> +	{ }
>> +};
>> +MODULE_DEVICE_TABLE(usb, kvaser_usb_table);
>> +
>> +static inline int kvaser_usb_send_msg(const struct kvaser_usb *dev,
>> +				      struct kvaser_msg *msg)
>> +{
>> +	int actual_len;
>> +
>> +	return usb_bulk_msg(dev->udev,
>> +			    usb_sndbulkpipe(dev->udev,
>> +					dev->bulk_out->bEndpointAddress),
>> +			    msg, msg->len, &actual_len,
>> +			    USB_SEND_TIMEOUT);
>> +}
>> +
>> +static int kvaser_usb_wait_msg(const struct kvaser_usb *dev, u8 id,
>> +			       struct kvaser_msg *msg)
>> +{
>> +	struct kvaser_msg *tmp;
>> +	void *buf;
>> +	int actual_len;
>> +	int err;
>> +	int pos = 0;
>> +
>> +	buf = kzalloc(RX_BUFFER_SIZE, GFP_KERNEL);
>> +	if (!buf)
>> +		return -ENOMEM;
>> +
>> +	err = usb_bulk_msg(dev->udev,
>> +			   usb_rcvbulkpipe(dev->udev,
>> +					   dev->bulk_in->bEndpointAddress),
>> +			   buf, RX_BUFFER_SIZE, &actual_len,
>> +			   USB_RECV_TIMEOUT);
>> +	if (err < 0)
>> +		goto end;
>> +
>> +	while (pos <= actual_len - MSG_HEADER_LEN) {
>> +		tmp = buf + pos;
>> +
>> +		if (!tmp->len)
>> +			break;
>> +
>> +		if (pos + tmp->len > actual_len) {
>> +			dev_err(dev->udev->dev.parent, "Format error\n");
>> +			break;
>> +		}
>> +
>> +		if (tmp->id == id) {
>> +			memcpy(msg, tmp, tmp->len);
>> +			goto end;
>> +		}
>> +
>> +		pos += tmp->len;
>> +	}
>> +
>> +	err = -EINVAL;
>> +
>> +end:
>> +	kfree(buf);
>> +
>> +	return err;
>> +}
>> +
>> +static int kvaser_usb_send_simple_msg(const struct kvaser_usb *dev,
>> +				      u8 msg_id, int channel)
>> +{
>> +	struct kvaser_msg msg = {
>> +		.len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_simple),
>> +		.id = msg_id,
>> +		.u.simple.channel = channel,
>> +		.u.simple.tid = 0xff,
>> +	};
>> +
>> +	return kvaser_usb_send_msg(dev, &msg);
>> +}
>> +
>> +static int kvaser_usb_get_software_info(struct kvaser_usb *dev)
>> +{
>> +	struct kvaser_msg msg;
>> +	int err;
>> +
>> +	err = kvaser_usb_send_simple_msg(dev, CMD_GET_SOFTWARE_INFO, 0);
>> +	if (err)
>> +		return err;
>> +
>> +	err = kvaser_usb_wait_msg(dev, CMD_GET_SOFTWARE_INFO_REPLY, &msg);
>> +	if (err)
>> +		return err;
>> +
>> +	dev->fw_version = le32_to_cpu(msg.u.softinfo.fw_version);
>> +
>> +	return 0;
>> +}
>> +
>> +static int kvaser_usb_get_card_info(struct kvaser_usb *dev)
>> +{
>> +	struct kvaser_msg msg;
>> +	int err;
>> +
>> +	err = kvaser_usb_send_simple_msg(dev, CMD_GET_CARD_INFO, 0);
>> +	if (err)
>> +		return err;
>> +
>> +	err = kvaser_usb_wait_msg(dev, CMD_GET_CARD_INFO_REPLY, &msg);
>> +	if (err)
>> +		return err;
>> +
>> +	dev->nchannels = msg.u.cardinfo.nchannels;
>> +
>> +	return 0;
>> +}
>> +
>> +static void kvaser_usb_tx_acknowledge(const struct kvaser_usb *dev,
>> +				      const struct kvaser_msg *msg)
>> +{
>> +	struct net_device_stats *stats;
>> +	struct kvaser_usb_tx_urb_context *context;
>> +	struct kvaser_usb_net_priv *priv;
>> +	struct sk_buff *skb;
>> +	struct can_frame *cf;
>> +	u8 channel = msg->u.tx_acknowledge.channel;
>> +	u8 tid = msg->u.tx_acknowledge.tid;
>> +
>> +	if (channel >= dev->nchannels) {
>> +		dev_err(dev->udev->dev.parent,
>> +			"Invalid channel number (%d)\n", channel);
>> +		return;
>> +	}
>> +
>> +	priv = dev->nets[channel];
>> +
>> +	if (!netif_device_present(priv->netdev))
>> +		return;
>> +
>> +	stats = &priv->netdev->stats;
>> +
>> +	context = &priv->tx_contexts[tid % MAX_TX_URBS];
>> +
>> +	/* Sometimes the state change doesn't come after a bus-off event */
>> +	if (priv->can.restart_ms &&
>> +	    (priv->can.state >= CAN_STATE_BUS_OFF)) {
>> +		skb = alloc_can_err_skb(priv->netdev, &cf);
>> +		if (skb) {
>> +			cf->can_id |= CAN_ERR_RESTARTED;
>> +			netif_rx(skb);
>> +
>> +			stats->rx_packets++;
>> +			stats->rx_bytes += cf->can_dlc;
>> +		} else {
>> +			netdev_err(priv->netdev,
>> +				   "No memory left for err_skb\n");
>> +		}
>> +
>> +		priv->can.can_stats.restarts++;
>> +		netif_carrier_on(priv->netdev);
>> +
>> +		priv->can.state = CAN_STATE_ERROR_ACTIVE;
>> +	}
>> +
>> +	stats->tx_packets++;
>> +	stats->tx_bytes += context->dlc;
>> +	can_get_echo_skb(priv->netdev, context->echo_index);
>> +
>> +	context->echo_index = MAX_TX_URBS;
>> +	atomic_dec(&priv->active_tx_urbs);
>> +
>> +	netif_wake_queue(priv->netdev);
>> +}
>> +
>> +static void kvaser_usb_simple_msg_callback(struct urb *urb)
>> +{
>> +	struct net_device *netdev = urb->context;
>> +
>> +	kfree(urb->transfer_buffer);
>> +
>> +	if (urb->status)
>> +		netdev_warn(netdev, "urb status received: %d\n",
>> +			    urb->status);
>> +}
>> +
>> +static int kvaser_usb_simple_msg_async(struct kvaser_usb_net_priv *priv,
>> +				       u8 msg_id)
>> +{
>> +	struct kvaser_usb *dev = priv->dev;
>> +	struct net_device *netdev = priv->netdev;
>> +	struct kvaser_msg *msg;
>> +	struct urb *urb;
>> +	void *buf;
>> +	int err;
>> +
>> +	urb = usb_alloc_urb(0, GFP_ATOMIC);
>> +	if (!urb) {
>> +		netdev_err(netdev, "No memory left for URBs\n");
>> +		return -ENOMEM;
>> +	}
>> +
>> +	buf = kmalloc(sizeof(struct kvaser_msg), GFP_ATOMIC);
>> +	if (!buf) {
> 
> Do you have to free the usb you just allocated?
> 
>> +		netdev_err(netdev, "No memory left for USB buffer\n");
>> +		return -ENOMEM;
>> +	}
>> +
>> +	msg = (struct kvaser_msg *)buf;
>> +	msg->len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_simple);
>> +	msg->id = msg_id;
>> +	msg->u.simple.channel = priv->channel;
>> +
>> +	usb_fill_bulk_urb(urb, dev->udev,
>> +			  usb_sndbulkpipe(dev->udev,
>> +					  dev->bulk_out->bEndpointAddress),
>> +			  buf, msg->len,
>> +			  kvaser_usb_simple_msg_callback, priv);
>> +	usb_anchor_urb(urb, &priv->tx_submitted);
>> +
>> +	err = usb_submit_urb(urb, GFP_ATOMIC);
>> +	if (err) {
>> +		netdev_err(netdev, "Error transmitting URB\n");
>> +		usb_unanchor_urb(urb);
>> +		kfree(buf);
>> +		return err;
> 
> and here?
> 
>> +	}
>> +
>> +	usb_free_urb(urb);
>> +
>> +	return 0;
>> +}
>> +
>> +static void kvaser_usb_unlink_tx_urbs(struct kvaser_usb_net_priv *priv)
>> +{
>> +	int i;
>> +
>> +	usb_kill_anchored_urbs(&priv->tx_submitted);
>> +	atomic_set(&priv->active_tx_urbs, 0);
>> +
>> +	for (i = 0; i < MAX_TX_URBS; i++)
>> +		priv->tx_contexts[i].echo_index = MAX_TX_URBS;
>> +}
>> +
>> +static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
>> +				const struct kvaser_msg *msg)
>> +{
>> +	struct can_frame *cf;
>> +	struct sk_buff *skb;
>> +	struct net_device_stats *stats;
>> +	struct kvaser_usb_net_priv *priv;
>> +	unsigned int new_state;
>> +	u8 channel, status, txerr, rxerr, error_factor;
>> +
>> +	switch (msg->id) {
>> +	case CMD_CAN_ERROR_EVENT:
>> +		channel = msg->u.error_event.channel;
>> +		status =  msg->u.error_event.status;
>> +		txerr = msg->u.error_event.tx_errors_count;
>> +		rxerr = msg->u.error_event.rx_errors_count;
>> +		error_factor = msg->u.error_event.error_factor;
>> +		break;
>> +	case CMD_LOG_MESSAGE:
>> +		channel = msg->u.log_message.channel;
>> +		status = msg->u.log_message.data[0];
>> +		txerr = msg->u.log_message.data[2];
>> +		rxerr = msg->u.log_message.data[3];
>> +		error_factor = msg->u.log_message.data[1];
>> +		break;
>> +	case CMD_CHIP_STATE_EVENT:
>> +		channel = msg->u.chip_state_event.channel;
>> +		status =  msg->u.chip_state_event.status;
>> +		txerr = msg->u.chip_state_event.tx_errors_count;
>> +		rxerr = msg->u.chip_state_event.rx_errors_count;
>> +		error_factor = 0;
>> +		break;
>> +	default:
>> +		dev_err(dev->udev->dev.parent, "Invalid msg id (%d)\n",
>> +			msg->id);
>> +		return;
>> +	}
>> +
>> +	if (channel >= dev->nchannels) {
>> +		dev_err(dev->udev->dev.parent,
>> +			"Invalid channel number (%d)\n", channel);
>> +		return;
>> +	}
>> +
>> +	priv = dev->nets[channel];
>> +	stats = &priv->netdev->stats;
>> +
>> +	if (status & M16C_STATE_BUS_RESET) {
>> +		kvaser_usb_unlink_tx_urbs(priv);
>> +		return;
>> +	}
>> +
>> +	skb = alloc_can_err_skb(priv->netdev, &cf);
>> +	if (!skb) {
>> +		stats->rx_dropped++;
>> +		return;
>> +	}
>> +
>> +	cf->can_id |= CAN_ERR_BUSERROR;

At state change is *not* a bus error. The line above should e moved into
the "if (error_factor)" block below.

>> +
>> +	new_state = priv->can.state;
>> +
>> +	netdev_dbg(priv->netdev, "Error status: 0x%02x\n", status);
>> +
>> +	if (status & M16C_STATE_BUS_OFF) {
>> +		cf->can_id |= CAN_ERR_BUSOFF;
>> +
>> +		priv->can.can_stats.bus_off++;
>> +		if (!priv->can.restart_ms)
>> +			kvaser_usb_simple_msg_async(priv, CMD_STOP_CHIP);
>> +
>> +		netif_carrier_off(priv->netdev);
>> +
>> +		new_state = CAN_STATE_BUS_OFF;
>> +	}
>> +
>> +	if (status & M16C_STATE_BUS_PASSIVE) {
> 
> else if ()
> 
> as bus passive and bus off is mutually exclusive.
> 
>> +		if (priv->can.state != CAN_STATE_ERROR_PASSIVE) {
>> +			cf->can_id |= CAN_ERR_CRTL;
>> +
>> +			if ((txerr > 0) || (rxerr > 0))

if (txerr | rxerr) ?

>> +				cf->data[1] = (txerr > rxerr)
>> +						? CAN_ERR_CRTL_TX_PASSIVE
>> +						: CAN_ERR_CRTL_RX_PASSIVE;
>> +			else
>> +				cf->data[1] = CAN_ERR_CRTL_TX_PASSIVE |
>> +					      CAN_ERR_CRTL_RX_PASSIVE;
>> +
>> +			priv->can.can_stats.error_passive++;
>> +		}
>> +
>> +		new_state = CAN_STATE_ERROR_PASSIVE;
>> +	}
>> +
>> +	if (status == M16C_STATE_BUS_ERROR) {
>> +		if ((priv->can.state < CAN_STATE_ERROR_WARNING) &&
>> +		    ((txerr > 96) || (rxerr > 96))) {
> 
> Is is >= 96 ?

Yep.

>> +			cf->can_id |= CAN_ERR_CRTL;
>> +			cf->data[1] = (txerr > rxerr)
>> +					? CAN_ERR_CRTL_TX_WARNING
>> +					: CAN_ERR_CRTL_RX_WARNING;
>> +
>> +			priv->can.can_stats.error_warning++;
>> +			new_state = CAN_STATE_ERROR_WARNING;
>> +		} else if (priv->can.state > CAN_STATE_ERROR_ACTIVE) {
>> +			cf->can_id |= CAN_ERR_PROT;
>> +			cf->data[2] = CAN_ERR_PROT_ACTIVE;
>> +
>> +			new_state = CAN_STATE_ERROR_ACTIVE;
>> +		}
>> +	}
>> +
>> +	if (!status) {
>> +		cf->can_id |= CAN_ERR_PROT;
>> +		cf->data[2] = CAN_ERR_PROT_ACTIVE;
>> +
>> +		new_state = CAN_STATE_ERROR_ACTIVE;
>> +	}
>> +
>> +	if (priv->can.restart_ms &&
>> +	    (priv->can.state >= CAN_STATE_BUS_OFF) &&
>> +	    (new_state < CAN_STATE_BUS_OFF)) {
>> +		cf->can_id |= CAN_ERR_RESTARTED;
>> +		netif_carrier_on(priv->netdev);
>> +
>> +		priv->can.can_stats.restarts++;
>> +	}
>> +
>> +	if (error_factor) {
>> +		priv->can.can_stats.bus_error++;
>> +		stats->rx_errors++;
>> +
>> +		cf->can_id |= CAN_ERR_PROT;
>> +
>> +		if (error_factor & M16C_EF_ACKE)
>> +			cf->data[3] |= (CAN_ERR_PROT_LOC_ACK);
>> +		if (error_factor & M16C_EF_CRCE)
>> +			cf->data[3] |= (CAN_ERR_PROT_LOC_CRC_SEQ |
>> +					CAN_ERR_PROT_LOC_CRC_DEL);
>> +		if (error_factor & M16C_EF_FORME)
>> +			cf->data[2] |= CAN_ERR_PROT_FORM;
>> +		if (error_factor & M16C_EF_STFE)
>> +			cf->data[2] |= CAN_ERR_PROT_STUFF;
>> +		if (error_factor & M16C_EF_BITE0)
>> +			cf->data[2] |= CAN_ERR_PROT_BIT0;
>> +		if (error_factor & M16C_EF_BITE1)
>> +			cf->data[2] |= CAN_ERR_PROT_BIT1;
>> +		if (error_factor & M16C_EF_TRE)
>> +			cf->data[2] |= CAN_ERR_PROT_TX;
>> +	}
>> +
>> +	cf->data[6] = txerr;
>> +	cf->data[7] = rxerr;
>> +
>> +	priv->bec.txerr = txerr;
>> +	priv->bec.rxerr = rxerr;
>> +
>> +	priv->can.state = new_state;
>> +
>> +	netif_rx(skb);
>> +
>> +	stats->rx_packets++;
>> +	stats->rx_bytes += cf->can_dlc;
>> +}
>> +
>> +static void kvaser_usb_rx_can_msg(const struct kvaser_usb *dev,
>> +				  const struct kvaser_msg *msg)
>> +{
>> +	struct kvaser_usb_net_priv *priv;
>> +	struct can_frame *cf;
>> +	struct sk_buff *skb;
>> +	struct net_device_stats *stats;
>> +	u8 channel = msg->u.rx_can.channel;
>> +
>> +	if (channel >= dev->nchannels) {
>> +		dev_err(dev->udev->dev.parent,
>> +			"Invalid channel number (%d)\n", channel);
>> +		return;
>> +	}
>> +
>> +	priv = dev->nets[channel];
>> +	stats = &priv->netdev->stats;
>> +
>> +	skb = alloc_can_skb(priv->netdev, &cf);
>> +	if (!skb) {
>> +		stats->tx_dropped++;
>> +		return;
>> +	}
>> +
>> +	cf->can_id = ((msg->u.rx_can.msg[0] & 0x1f) << 6) |
>> +		     (msg->u.rx_can.msg[1] & 0x3f);
>> +	cf->can_dlc = get_can_dlc(msg->u.rx_can.msg[5]);
>> +
>> +	if (msg->id == CMD_RX_EXT_MESSAGE) {
>> +		cf->can_id <<= 18;
>> +		cf->can_id |= ((msg->u.rx_can.msg[2] & 0x0f) << 14) |
>> +			      ((msg->u.rx_can.msg[3] & 0xff) << 6) |
>> +			      (msg->u.rx_can.msg[4] & 0x3f);
>> +		cf->can_id |= CAN_EFF_FLAG;
>> +	}
>> +
>> +	if (msg->u.rx_can.flag & MSG_FLAG_REMOTE_FRAME) {
>> +		cf->can_id |= CAN_RTR_FLAG;
>> +	} else if (msg->u.rx_can.flag & (MSG_FLAG_ERROR_FRAME |
>> +					 MSG_FLAG_NERR)) {
>> +		cf->can_id |= CAN_ERR_FLAG;
>> +		cf->can_dlc = CAN_ERR_DLC;

> Please move the error skb creation handling into a subfunction, use
> can_alloc_err_skb() in that function. Please move the:
> 
>     if (msg->u.rx_can.flag & ...)
> 
> up in this function, before the alloc_can_skb().
> 
>> +
>> +		netdev_err(priv->netdev, "Unknow error (flags: 0x%02x)\n",
>> +			   msg->u.rx_can.flag);
>> +
>> +		stats->rx_errors++;

This an *error* which does normally not happen, IIRC. Therefore I do not
see a need to pass it to the user as error message.

>> +	} else if (msg->u.rx_can.flag & MSG_FLAG_OVERRUN) {
>> +		cf->can_id = CAN_ERR_FLAG | CAN_ERR_CRTL;
>> +		cf->can_dlc = CAN_ERR_DLC;
>> +		cf->data[1] = CAN_ERR_CRTL_RX_OVERFLOW;
>> +
>> +		stats->rx_over_errors++;
>> +		stats->rx_errors++;
> 
> This should go into the error skb generation function, too.
> 
>> +	} else if (!msg->u.rx_can.flag) {
>> +		memcpy(cf->data, &msg->u.rx_can.msg[6], cf->can_dlc);
> 
> Please don't copy the contents of RTR frames.
> 
>> +	} else {
>> +		kfree_skb(skb);
> 
> After you have moved the error skb generation into a seperate function,
> you should get rid of the kfree(skb), too. The function should look like
> this (pseude code):
> 
> static void kvaser_usb_rx_can_msg(const struct kvaser_usb *dev,
> 				  const struct kvaser_msg *msg)
> {
> 	if (channel_invalid())
> 		return;
> 
> 	if (msg->u.rx_can.flag & ERROR) {
> 		kvaser_usb_rx_can_err_msg();
> 		return;
> 	} else if (msg->u.rx_can.flag & INVALID_FRAME) {
> 		return;
> 	}
> 
> 	skb = alloc_can_skb();
> 
> 	/* existing dlc, rtr and data handling code */
> 	...
> }
> 
> 
>> +		return;
>> +	}
>> +
>> +	netif_rx(skb);
>> +
>> +	stats->rx_packets++;
>> +	stats->rx_bytes += cf->can_dlc;
>> +}
>> +
>> +static void kvaser_usb_start_chip_reply(const struct kvaser_usb *dev,
>> +					const struct kvaser_msg *msg)
>> +{
>> +	struct kvaser_usb_net_priv *priv;
>> +	u8 channel = msg->u.simple.channel;
>> +
>> +	if (channel >= dev->nchannels) {
>> +		dev_err(dev->udev->dev.parent,
>> +			"Invalid channel number (%d)\n", channel);
>> +		return;
>> +	}
>> +
>> +	priv = dev->nets[channel];
>> +
>> +	if (completion_done(&priv->start_comp) &&
>> +	    netif_queue_stopped(priv->netdev)) {
>> +		netif_wake_queue(priv->netdev);
>> +	} else {
>> +		netif_start_queue(priv->netdev);
>> +		complete(&priv->start_comp);
>> +	}
>> +}
>> +
>> +static void kvaser_usb_stop_chip_reply(const struct kvaser_usb *dev,
>> +				       const struct kvaser_msg *msg)
>> +{
>> +	struct kvaser_usb_net_priv *priv;
>> +	u8 channel = msg->u.simple.channel;
>> +
>> +	if (channel >= dev->nchannels) {
>> +		dev_err(dev->udev->dev.parent,
>> +			"Invalid channel number (%d)\n", channel);
>> +		return;
>> +	}
>> +
>> +	priv = dev->nets[channel];
>> +
>> +	complete(&priv->stop_comp);
>> +}
>> +
>> +static void kvaser_usb_handle_message(const struct kvaser_usb *dev,
>> +				      const struct kvaser_msg *msg)
>> +{
>> +	switch (msg->id) {
>> +	case CMD_START_CHIP_REPLY:
>> +		kvaser_usb_start_chip_reply(dev, msg);
>> +		break;
>> +
>> +	case CMD_STOP_CHIP_REPLY:
>> +		kvaser_usb_stop_chip_reply(dev, msg);
>> +		break;
>> +
>> +	case CMD_RX_STD_MESSAGE:
>> +	case CMD_RX_EXT_MESSAGE:
>> +		kvaser_usb_rx_can_msg(dev, msg);
>> +		break;
>> +
>> +	case CMD_CHIP_STATE_EVENT:
>> +	case CMD_CAN_ERROR_EVENT:
>> +		kvaser_usb_rx_error(dev, msg);
>> +		break;
>> +
>> +	case CMD_LOG_MESSAGE:
>> +		if (msg->u.log_message.flags & MSG_FLAG_ERROR_FRAME)
>> +			kvaser_usb_rx_error(dev, msg);
>> +		break;
>> +
>> +	case CMD_TX_ACKNOWLEDGE:
>> +		kvaser_usb_tx_acknowledge(dev, msg);
>> +		break;
>> +
>> +	default:
>> +		dev_warn(dev->udev->dev.parent,
>> +			 "Unhandled message (%d)\n", msg->id);
>> +		break;
>> +	}
>> +}
>> +
>> +static void kvaser_usb_read_bulk_callback(struct urb *urb)
>> +{
>> +	struct kvaser_usb *dev = urb->context;
>> +	struct kvaser_msg *msg;
>> +	int pos = 0;
>> +	int err, i;
>> +
>> +	switch (urb->status) {
>> +	case 0:
>> +		break;
>> +	case -ENOENT:
>> +	case -ESHUTDOWN:
>> +		return;
>> +	default:
>> +		dev_info(dev->udev->dev.parent, "Rx URB aborted (%d)\n",
>> +			 urb->status);
>> +		goto resubmit_urb;
>> +	}
>> +
>> +	while (pos <= urb->actual_length - MSG_HEADER_LEN) {
>> +		msg = urb->transfer_buffer + pos;
>> +
>> +		if (!msg->len)
>> +			break;
>> +
>> +		if (pos + msg->len > urb->actual_length) {
>> +			dev_err(dev->udev->dev.parent, "Format error\n");
>> +			break;
>> +		}
>> +
>> +		kvaser_usb_handle_message(dev, msg);
>> +
>> +		pos += msg->len;
>> +	}
>> +
>> +resubmit_urb:
>> +	usb_fill_bulk_urb(urb, dev->udev,
>> +			  usb_rcvbulkpipe(dev->udev,
>> +					  dev->bulk_in->bEndpointAddress),
>> +			  urb->transfer_buffer, RX_BUFFER_SIZE,
>> +			  kvaser_usb_read_bulk_callback, dev);
>> +
>> +	err = usb_submit_urb(urb, GFP_ATOMIC);
>> +	if (err == -ENODEV) {
>> +		for (i = 0; i < dev->nchannels; i++) {
>> +			if (!dev->nets[i])
>> +				continue;
>> +
>> +			netif_device_detach(dev->nets[i]->netdev);
>> +		}
>> +	} else if (err) {
>> +		dev_err(dev->udev->dev.parent,
>> +			"Failed resubmitting read bulk urb: %d\n", err);
>> +	}
>> +
>> +	return;
>> +}
>> +
>> +static int kvaser_usb_setup_rx_urbs(struct kvaser_usb *dev)
>> +{
>> +	int i, err = 0;
>> +
>> +	if (dev->rxinitdone)
>> +		return 0;
>> +
>> +	for (i = 0; i < MAX_RX_URBS; i++) {
>> +		struct urb *urb = NULL;
>> +		u8 *buf = NULL;
>> +		dma_addr_t buf_dma;
>> +
>> +		urb = usb_alloc_urb(0, GFP_KERNEL);
>> +		if (!urb) {
>> +			dev_warn(dev->udev->dev.parent,
>> +				 "No memory left for URBs\n");
>> +			err = -ENOMEM;
>> +			break;
>> +		}
>> +
>> +		buf = usb_alloc_coherent(dev->udev, RX_BUFFER_SIZE,
>> +					 GFP_KERNEL, &buf_dma);
>> +		if (!buf) {
>> +			dev_warn(dev->udev->dev.parent,
>> +				 "No memory left for USB buffer\n");
>> +			usb_free_urb(urb);
>> +			err = -ENOMEM;
>> +			break;
>> +		}
>> +
>> +		usb_fill_bulk_urb(urb, dev->udev,
>> +				  usb_rcvbulkpipe(dev->udev,
>> +					  dev->bulk_in->bEndpointAddress),
>> +				  buf, RX_BUFFER_SIZE,
>> +				  kvaser_usb_read_bulk_callback,
>> +				  dev);
>> +		urb->transfer_dma = buf_dma;
>> +		urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
>> +		usb_anchor_urb(urb, &dev->rx_submitted);
>> +
>> +		err = usb_submit_urb(urb, GFP_KERNEL);
>> +		if (err) {
>> +			usb_unanchor_urb(urb);
>> +			usb_free_coherent(dev->udev, RX_BUFFER_SIZE, buf,
>> +					  buf_dma);
> 
> Do you have to call usb_free_usb() here, or does the usb frame work take
> care of this?
> 
>> +			break;
>> +		}
>> +
>> +		dev->rxbuf[i] = buf;
>> +		dev->rxbuf_dma[i] = buf_dma;
>> +
>> +		usb_free_urb(urb);
>> +	}
>> +
>> +	if (i == 0) {
>> +		dev_warn(dev->udev->dev.parent,
>> +			 "Cannot setup read URBs, error %d\n", err);
>> +		return err;
>> +	} else if (i < MAX_RX_URBS) {
>> +		dev_warn(dev->udev->dev.parent,
>> +			 "RX performances may be slow\n");
>> +	}
>> +
>> +	dev->rxinitdone = true;
>> +
>> +	return 0;
>> +}
>> +
>> +static int kvaser_usb_set_opt_mode(const struct kvaser_usb_net_priv *priv)
>> +{
>> +	struct kvaser_msg msg = {
>> +		.id = CMD_SET_CTRL_MODE,
>> +		.len = MSG_HEADER_LEN +
>> +		       sizeof(struct kvaser_msg_ctrl_mode),
>> +		.u.ctrl_mode.tid = 0xff,
>> +		.u.ctrl_mode.channel = priv->channel,
>> +	};
>> +
>> +	if (priv->can.ctrlmode & CAN_CTRLMODE_LISTENONLY)
>> +		msg.u.ctrl_mode.ctrl_mode = KVASER_CTRL_MODE_SILENT;
>> +	else
>> +		msg.u.ctrl_mode.ctrl_mode = KVASER_CTRL_MODE_NORMAL;
>> +
>> +	return kvaser_usb_send_msg(priv->dev, &msg);
>> +}
>> +
>> +static int kvaser_usb_start_chip(struct kvaser_usb_net_priv *priv)
>> +{
>> +	int err;
>> +
>> +	init_completion(&priv->start_comp);
>> +
>> +	err = kvaser_usb_send_simple_msg(priv->dev, CMD_START_CHIP,
>> +					 priv->channel);
>> +	if (err)
>> +		return err;
>> +
>> +	if (!wait_for_completion_timeout(&priv->start_comp,
>> +					 msecs_to_jiffies(START_TIMEOUT)))
>> +		return -ETIMEDOUT;
>> +
>> +	return 0;
>> +}
>> +
>> +static int kvaser_usb_open(struct net_device *netdev)
>> +{
>> +	struct kvaser_usb_net_priv *priv = netdev_priv(netdev);
>> +	struct kvaser_usb *dev = priv->dev;
>> +	int err;
>> +
>> +	err = open_candev(netdev);
>> +	if (err)
>> +		return err;
>> +
>> +	err = kvaser_usb_setup_rx_urbs(dev);
>> +	if (err)
>> +		goto error;
>> +
>> +	err = kvaser_usb_set_opt_mode(priv);
>> +	if (err)
>> +		goto error;
>> +
>> +	err = kvaser_usb_start_chip(priv);
>> +	if (err) {
>> +		netdev_warn(netdev, "Cannot start device, error %d\n", err);
>> +		goto error;
>> +	}
>> +
>> +	priv->can.state = CAN_STATE_ERROR_ACTIVE;
>> +
>> +	return 0;
>> +
>> +error:
>> +	close_candev(netdev);
>> +	return err;
>> +}
>> +
>> +static void kvaser_usb_unlink_all_urbs(struct kvaser_usb *dev)
>> +{
>> +	int i;
>> +
>> +	usb_kill_anchored_urbs(&dev->rx_submitted);
>> +
>> +	for (i = 0; i < MAX_RX_URBS; i++)
>> +		usb_free_coherent(dev->udev, RX_BUFFER_SIZE,
>> +				  dev->rxbuf[i],
>> +				  dev->rxbuf_dma[i]);
>> +
>> +	for (i = 0; i < MAX_NET_DEVICES; i++) {
>> +		struct kvaser_usb_net_priv *priv = dev->nets[i];
>> +
>> +		if (priv)
>> +			kvaser_usb_unlink_tx_urbs(priv);
>> +	}
>> +}
>> +
>> +static int kvaser_usb_stop_chip(struct kvaser_usb_net_priv *priv)
>> +{
>> +	int err;
>> +
>> +	init_completion(&priv->stop_comp);
>> +
>> +	err = kvaser_usb_send_simple_msg(priv->dev, CMD_STOP_CHIP,
>> +					 priv->channel);
>> +	if (err)
>> +		return err;
>> +
>> +	if (!wait_for_completion_timeout(&priv->stop_comp,
>> +					 msecs_to_jiffies(STOP_TIMEOUT)))
>> +		return -ETIMEDOUT;
>> +
>> +	return 0;
>> +}
>> +
>> +static int kvaser_usb_flush_queue(struct kvaser_usb_net_priv *priv)
>> +{
>> +	struct kvaser_msg msg = {
>> +		.id = CMD_FLUSH_QUEUE,
>> +		.len = MSG_HEADER_LEN +
>> +		       sizeof(struct kvaser_msg_flush_queue),
>> +		.u.flush_queue.channel = priv->channel,
>> +		.u.flush_queue.flags = 0x00,
>> +	};
>> +
>> +	return kvaser_usb_send_msg(priv->dev, &msg);
>> +}
>> +
>> +static int kvaser_usb_close(struct net_device *netdev)
>> +{
>> +	struct kvaser_usb_net_priv *priv = netdev_priv(netdev);
>> +	struct kvaser_usb *dev = priv->dev;
>> +	int err;
>> +
>> +	netif_stop_queue(netdev);
>> +
>> +	err = kvaser_usb_flush_queue(priv);
>> +	if (err)
>> +		netdev_warn(netdev, "Cannot flush queue, error %d\n", err);
>> +
>> +	if (kvaser_usb_send_simple_msg(dev, CMD_RESET_CHIP, priv->channel))
>> +		netdev_warn(netdev, "Cannot reset card, error %d\n", err);
>> +
>> +	err = kvaser_usb_stop_chip(priv);
>> +	if (err)
>> +		netdev_warn(netdev, "Cannot stop device, error %d\n", err);
>> +
>> +	priv->can.state = CAN_STATE_STOPPED;
>> +	close_candev(priv->netdev);
>> +
>> +	return 0;
>> +}
>> +
>> +static void kvaser_usb_write_bulk_callback(struct urb *urb)
>> +{
>> +	struct kvaser_usb_tx_urb_context *context = urb->context;
>> +	struct kvaser_usb_net_priv *priv;
>> +	struct net_device *netdev;
>> +
>> +	if (WARN_ON(!context))
>> +		return;
>> +
>> +	priv = context->priv;
>> +	netdev = priv->netdev;
>> +
>> +	kfree(urb->transfer_buffer);
>> +
>> +	if (!netif_device_present(netdev))
>> +		return;
>> +
>> +	if (urb->status)
>> +		netdev_info(netdev, "Tx URB aborted (%d)\n", urb->status);
>> +}
>> +
>> +static netdev_tx_t kvaser_usb_start_xmit(struct sk_buff *skb,
>> +					 struct net_device *netdev)
>> +{
>> +	struct kvaser_usb_net_priv *priv = netdev_priv(netdev);
>> +	struct kvaser_usb *dev = priv->dev;
>> +	struct net_device_stats *stats = &netdev->stats;
>> +	struct can_frame *cf = (struct can_frame *)skb->data;
>> +	struct kvaser_usb_tx_urb_context *context = NULL;
>> +	struct urb *urb;
>> +	void *buf;
>> +	struct kvaser_msg *msg;
>> +	int i, err;
>> +	int ret = NETDEV_TX_OK;
>> +
>> +	if (can_dropped_invalid_skb(netdev, skb))
>> +		return NETDEV_TX_OK;
>> +
>> +	urb = usb_alloc_urb(0, GFP_ATOMIC);
>> +	if (!urb) {
>> +		netdev_err(netdev, "No memory left for URBs\n");
>> +		stats->tx_dropped++;
>> +		dev_kfree_skb(skb);
>> +		return NETDEV_TX_OK;
>> +	}
>> +
>> +	buf = kmalloc(sizeof(struct kvaser_msg), GFP_ATOMIC);
>> +	if (!buf) {
>> +		netdev_err(netdev, "No memory left for USB buffer\n");
>> +		stats->tx_dropped++;
>> +		dev_kfree_skb(skb);
> What about the urb?
>> +		goto nobufmem;
>> +	}
>> +
>> +	msg = (struct kvaser_msg *)buf;
> 
> nitpick: cast is not needed, as buf is void *
> 
>> +	msg->len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_tx_can);
>> +	msg->u.tx_can.flags = 0;
>> +	msg->u.tx_can.channel = priv->channel;
>> +
>> +	if (cf->can_id & CAN_EFF_FLAG) {
>> +		msg->id = CMD_TX_EXT_MESSAGE;
>> +		msg->u.tx_can.msg[0] = (cf->can_id >> 24) & 0x1f;
>> +		msg->u.tx_can.msg[1] = (cf->can_id >> 18) & 0x3f;
>> +		msg->u.tx_can.msg[2] = (cf->can_id >> 14) & 0x0f;
>> +		msg->u.tx_can.msg[3] = (cf->can_id >> 6) & 0xff;
>> +		msg->u.tx_can.msg[4] = cf->can_id & 0x3f;
>> +	} else {
>> +		msg->id = CMD_TX_STD_MESSAGE;
>> +		msg->u.tx_can.msg[0] = (cf->can_id >> 6) & 0x1f;
>> +		msg->u.tx_can.msg[1] = cf->can_id & 0x3f;
>> +	}
>> +
>> +	msg->u.tx_can.msg[5] = cf->can_dlc;
>> +	memcpy(&msg->u.tx_can.msg[6], cf->data, cf->can_dlc);
>> +
>> +	if (cf->can_id & CAN_RTR_FLAG)
>> +		msg->u.tx_can.flags |= MSG_FLAG_REMOTE_FRAME;
>> +
>> +	for (i = 0; i < ARRAY_SIZE(priv->tx_contexts); i++) {
>> +		if (priv->tx_contexts[i].echo_index == MAX_TX_URBS) {
>> +			context = &priv->tx_contexts[i];
>> +			break;
>> +		}
>> +	}
>> +
>> +	if (!context) {
>> +		netdev_warn(netdev, "cannot find free context\n");
>> +		ret =  NETDEV_TX_BUSY;
>> +		goto releasebuf;
>> +	}
>> +
>> +	context->priv = priv;
>> +	context->echo_index = i;
>> +	context->dlc = cf->can_dlc;
>> +
>> +	msg->u.tx_can.tid = context->echo_index;
>> +
>> +	usb_fill_bulk_urb(urb, dev->udev,
>> +			  usb_sndbulkpipe(dev->udev,
>> +					  dev->bulk_out->bEndpointAddress),
>> +			  buf, msg->len,
>> +			  kvaser_usb_write_bulk_callback, context);
>> +	usb_anchor_urb(urb, &priv->tx_submitted);
>> +
>> +	can_put_echo_skb(skb, netdev, context->echo_index);
>> +
>> +	atomic_inc(&priv->active_tx_urbs);
>> +
>> +	if (atomic_read(&priv->active_tx_urbs) >= MAX_TX_URBS)
>> +		netif_stop_queue(netdev);
>> +
>> +	err = usb_submit_urb(urb, GFP_ATOMIC);
>> +	if (unlikely(err)) {
>> +		can_free_echo_skb(netdev, context->echo_index);
>> +
>> +		atomic_dec(&priv->active_tx_urbs);
>> +		usb_unanchor_urb(urb);
>> +
>> +		stats->tx_dropped++;
>> +
>> +		if (err == -ENODEV)
>> +			netif_device_detach(netdev);
>> +		else
>> +			netdev_warn(netdev, "Failed tx_urb %d\n", err);
>> +
>> +		goto releasebuf;
>> +	}
>> +
>> +	netdev->trans_start = jiffies;
> 
> Is this still needed?
> 
>> +
>> +	usb_free_urb(urb);
>> +
>> +	return NETDEV_TX_OK;
>> +
>> +releasebuf:
>> +	kfree(buf);
>> +nobufmem:
>> +	usb_free_urb(urb);
>> +	return ret;
>> +}
>> +
>> +static const struct net_device_ops kvaser_usb_netdev_ops = {
>> +	.ndo_open = kvaser_usb_open,
>> +	.ndo_stop = kvaser_usb_close,
>> +	.ndo_start_xmit = kvaser_usb_start_xmit,
>> +};
>> +
>> +static struct can_bittiming_const kvaser_usb_bittiming_const = {
>> +	.name = "kvaser_usb",
>> +	.tseg1_min = KVASER_USB_TSEG1_MIN,
>> +	.tseg1_max = KVASER_USB_TSEG1_MAX,
>> +	.tseg2_min = KVASER_USB_TSEG2_MIN,
>> +	.tseg2_max = KVASER_USB_TSEG2_MAX,
>> +	.sjw_max = KVASER_USB_SJW_MAX,
>> +	.brp_min = KVASER_USB_BRP_MIN,
>> +	.brp_max = KVASER_USB_BRP_MAX,
>> +	.brp_inc = KVASER_USB_BRP_INC,
>> +};
>> +
>> +static int kvaser_usb_set_bittiming(struct net_device *netdev)
>> +{
>> +	struct kvaser_usb_net_priv *priv = netdev_priv(netdev);
>> +	struct can_bittiming *bt = &priv->can.bittiming;
>> +	struct kvaser_usb *dev = priv->dev;
>> +	struct kvaser_msg msg = {
>> +		.id = CMD_SET_BUS_PARAMS,
>> +		.len = MSG_HEADER_LEN +
>> +		       sizeof(struct kvaser_msg_busparams),
>> +		.u.busparams.channel = priv->channel,
>> +		.u.busparams.tid = 0xff,
>> +		.u.busparams.bitrate = cpu_to_le32(bt->bitrate),
>> +		.u.busparams.sjw = bt->sjw,
>> +		.u.busparams.tseg1 = bt->prop_seg + bt->phase_seg1,
>> +		.u.busparams.tseg2 = bt->phase_seg2,
>> +	};
>> +
>> +	if (priv->can.ctrlmode & CAN_CTRLMODE_3_SAMPLES)
>> +		msg.u.busparams.no_samp = 3;
>> +	else
>> +		msg.u.busparams.no_samp = 1;
>> +
>> +	return kvaser_usb_send_msg(dev, &msg);
>> +}
>> +
>> +static int kvaser_usb_set_mode(struct net_device *netdev,
>> +			       enum can_mode mode)
>> +{
>> +	struct kvaser_usb_net_priv *priv = netdev_priv(netdev);
>> +	int err;
>> +
>> +	switch (mode) {
>> +	case CAN_MODE_START:
>> +		err = kvaser_usb_simple_msg_async(priv, CMD_START_CHIP);
>> +		if (err)
>> +			return err;
>> +		break;
>> +	default:
>> +		return -EOPNOTSUPP;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int kvaser_usb_get_berr_counter(const struct net_device *netdev,
>> +				       struct can_berr_counter *bec)
>> +{
>> +	struct kvaser_usb_net_priv *priv = netdev_priv(netdev);
>> +
>> +	*bec = priv->bec;
>> +
>> +	return 0;
>> +}
>> +
>> +static int kvaser_usb_init_one(struct usb_interface *intf,
>> +			       const struct usb_device_id *id, int channel)
>> +{
>> +	struct kvaser_usb *dev = usb_get_intfdata(intf);
>> +	struct net_device *netdev;
>> +	struct kvaser_usb_net_priv *priv;
>> +	int i, err;
>> +
>> +	netdev = alloc_candev(sizeof(*priv), MAX_TX_URBS);
>> +	if (!netdev) {
>> +		dev_err(&intf->dev, "Cannot alloc candev\n");
>> +		return -ENOMEM;
>> +	}
>> +
>> +	priv = netdev_priv(netdev);
>> +
>> +	init_completion(&priv->start_comp);
>> +	init_completion(&priv->stop_comp);
>> +
>> +	init_usb_anchor(&priv->tx_submitted);
>> +	atomic_set(&priv->active_tx_urbs, 0);
>> +
>> +	for (i = 0; i < ARRAY_SIZE(priv->tx_contexts); i++)
>> +		priv->tx_contexts[i].echo_index = MAX_TX_URBS;
>> +
>> +	priv->dev = dev;
>> +	priv->netdev = netdev;
>> +	priv->channel = channel;
>> +
>> +	priv->can.state = CAN_STATE_STOPPED;
>> +	priv->can.clock.freq = CAN_USB_CLOCK;
>> +	priv->can.bittiming_const = &kvaser_usb_bittiming_const;
>> +	priv->can.do_set_bittiming = kvaser_usb_set_bittiming;
>> +	priv->can.do_set_mode = kvaser_usb_set_mode;
>> +	if (id->driver_info & KVASER_HAS_TXRX_ERRORS)
>> +		priv->can.do_get_berr_counter = kvaser_usb_get_berr_counter;
>> +	priv->can.ctrlmode_supported = CAN_CTRLMODE_3_SAMPLES;
>> +	if (id->driver_info & KVASER_HAS_SILENT_MODE)
>> +		priv->can.ctrlmode_supported |= CAN_CTRLMODE_LISTENONLY;
>> +
>> +	netdev->flags |= IFF_ECHO;
>> +
>> +	netdev->netdev_ops = &kvaser_usb_netdev_ops;
>> +
>> +	SET_NETDEV_DEV(netdev, &intf->dev);
>> +
>> +	dev->nets[channel] = priv;
>> +
>> +	err = register_candev(netdev);
>> +	if (err) {
>> +		dev_err(&intf->dev, "Failed to register can device\n");
>> +		free_candev(netdev);
>> +		return err;
>> +	}
>> +
>> +	netdev_dbg(netdev, "device registered\n");
>> +
>> +	return 0;
>> +}
>> +
>> +static void kvaser_usb_get_endpoints(const struct usb_interface *intf,
>> +				     struct usb_endpoint_descriptor **in,
>> +				     struct usb_endpoint_descriptor **out)
>> +{
>> +	const struct usb_host_interface *iface_desc;
>> +	struct usb_endpoint_descriptor *endpoint;
>> +	int i;
>> +
>> +	iface_desc = &intf->altsetting[0];
>> +
>> +	for (i = 0; i < iface_desc->desc.bNumEndpoints; ++i) {
>> +		endpoint = &iface_desc->endpoint[i].desc;
>> +
>> +		if (usb_endpoint_is_bulk_in(endpoint))
>> +			*in = endpoint;
>> +
>> +		if (usb_endpoint_is_bulk_out(endpoint))
>> +			*out = endpoint;
>> +	}
>> +}
>> +
>> +static int kvaser_usb_probe(struct usb_interface *intf,
>> +			    const struct usb_device_id *id)
>> +{
>> +	struct kvaser_usb *dev;
>> +	int err = -ENOMEM;
>> +	int i;
>> +
>> +	dev = devm_kzalloc(&intf->dev, sizeof(*dev), GFP_KERNEL);
>> +	if (!dev)
>> +		return -ENOMEM;
>> +
>> +	kvaser_usb_get_endpoints(intf, &dev->bulk_in, &dev->bulk_out);
>> +	if (!dev->bulk_in || !dev->bulk_out) {
>> +		dev_err(&intf->dev, "Cannot get usb endpoint(s)");
>> +		return err;
>> +	}
>> +
>> +	dev->udev = interface_to_usbdev(intf);
>> +
>> +	init_usb_anchor(&dev->rx_submitted);
>> +
>> +	usb_set_intfdata(intf, dev);
>> +
>> +	for (i = 0; i < MAX_NET_DEVICES; i++)
>> +		kvaser_usb_send_simple_msg(dev, CMD_RESET_CHIP, i);
>> +
>> +	err = kvaser_usb_get_software_info(dev);
>> +	if (err) {
>> +		dev_err(&intf->dev,
>> +			"Cannot get software infos, error %d\n", err);
>> +		return err;
>> +	}
>> +
>> +	err = kvaser_usb_get_card_info(dev);
>> +	if (err) {
>> +		dev_err(&intf->dev,
>> +			"Cannot get card infos, error %d\n", err);
>> +		return err;
>> +	}
>> +
>> +	dev_dbg(&intf->dev, "Firmware version: %d.%d.%d\n",
>> +		((dev->fw_version >> 24) & 0xff),
>> +		((dev->fw_version >> 16) & 0xff),
>> +		(dev->fw_version & 0xffff));
>> +
>> +	for (i = 0; i < dev->nchannels; i++)
>> +		kvaser_usb_init_one(intf, id, i);

Error checking is not needed?

>> +
>> +	return 0;
>> +}
>> +
>> +static void kvaser_usb_disconnect(struct usb_interface *intf)
>> +{
>> +	struct kvaser_usb *dev = usb_get_intfdata(intf);
>> +	int i;
>> +
>> +	usb_set_intfdata(intf, NULL);
>> +
>> +	if (!dev)
>> +		return;
>> +
>> +	for (i = 0; i < dev->nchannels; i++) {
>> +		if (!dev->nets[i])
>> +			continue;
>> +
>> +		unregister_netdev(dev->nets[i]->netdev);
>> +	}
>> +
>> +	kvaser_usb_unlink_all_urbs(dev);
>> +
>> +	for (i = 0; i < dev->nchannels; i++)
>> +		free_candev(dev->nets[i]->netdev);
>> +}
>> +
>> +static struct usb_driver kvaser_usb_driver = {
>> +	.name = "kvaser_usb",
>> +	.probe = kvaser_usb_probe,
>> +	.disconnect = kvaser_usb_disconnect,
>> +	.id_table = kvaser_usb_table
>> +};
>> +
>> +module_usb_driver(kvaser_usb_driver);
>> +
>> +MODULE_AUTHOR("Olivier Sobrie <olivier@sobrie.be>");
>> +MODULE_DESCRIPTION("CAN driver for Kvaser CAN/USB devices");
>> +MODULE_LICENSE("GPL v2");

Wolfgang.


^ permalink raw reply

* Re: [PATCH V2 net-next 4/4] ptp: clarify the clock_name sysfs attribute
From: Ben Hutchings @ 2012-09-22 15:44 UTC (permalink / raw)
  To: Richard Cochran
  Cc: netdev, David Miller, Jacob Keller, Jeff Kirsher, John Stultz,
	Matthew Vick
In-Reply-To: <6bc914f0607428ab0d8f5d5fc42a5e87bedc95c5.1348299094.git.richardcochran@gmail.com>

On Sat, 2012-09-22 at 09:42 +0200, Richard Cochran wrote:
> There has been some confusion among PHC driver authors about the
> intended purpose of the clock_name attribute. This patch expands the
> documation in order to clarify how the clock_name field should be
> understood.
> 
> Signed-off-by: Richard Cochran <richardcochran@gmail.com>
> ---
>  Documentation/ABI/testing/sysfs-ptp |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-ptp b/Documentation/ABI/testing/sysfs-ptp
> index d40d2b5..c906488 100644
> --- a/Documentation/ABI/testing/sysfs-ptp
> +++ b/Documentation/ABI/testing/sysfs-ptp
> @@ -19,7 +19,10 @@ Date:		September 2010
>  Contact:	Richard Cochran <richardcochran@gmail.com>
>  Description:
>  		This file contains the name of the PTP hardware clock
> -		as a human readable string.
> +		as a human readable string. The purpose of this
> +		attribute is to provide the user with a "friendly
> +		name" and to help distinguish PHY based devices from
> +		MAC based ones.

Might also be worth saying that it is not required to be unique.  And
this explanation should also go in the kernel-doc in ptp_clock_kernel.h,
which is what driver writers are most likely to read.

Ben.

>  What:		/sys/class/ptp/ptpN/max_adjustment
>  Date:		September 2010

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

^ permalink raw reply


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