* Re: [PATCH] net: Linn Ethernet Packet Sniffer driver
From: Stathis Voukelatos @ 2015-01-26 9:49 UTC (permalink / raw)
To: Daniel Borkmann, Stathis Voukelatos
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
devicetree@vger.kernel.org, abrestic@chromium.org
In-Reply-To: <54C22E68.1080601@redhat.com>
On 23/01/15 11:20, Daniel Borkmann wrote:
> On 01/23/2015 11:07 AM, Stathis Voukelatos wrote:
>> This patch adds support the Ethernet Packet Sniffer H/W module
>> developed by Linn Products Ltd and found in the IMG Pistachio SoC.
>> The module allows Ethernet packets to be parsed, matched against
>> a user-defined pattern and timestamped. It sits between a 100M
>> Ethernet MAC and PHY and is completely passive with respect to
>> Ethernet frames.
>>
>> Matched packet bytes and timestamp values are returned through a
>> FIFO. Timestamps are provided to the module through an externally
>> generated Gray-encoded counter.
>>
>> The command pattern for packet matching is stored in module RAM
>> and consists of a sequence of 16-bit entries. Each entry includes
>> an 8-bit command code and and 8-bit data value. Valid command
>> codes are:
>> 0 - Don't care
>> 1 - Match: packet data must match command string byte
>> 2 - Copy: packet data will be copied to FIFO
>> 3 - Match/Stamp: if packet data matches string byte, a timestamp
>> is copied into the FIFO
>> 4 - Copy/Done: packet data will be copied into the FIFO.
>> This command terminates the command string.
>>
>> The driver consists of two modules:
>> - Core: it provides an API to user space using the Generic Netlink
>> framework. Specific backend implementations, like the
>> Ethernet Packet Sniffer, register one or more channels
>> with the Core. For each channel a Genl family is created.
>> User space can access a channel by sending Genl messages
>> to the Genl family associated with the channel. Packet
>> matching events are multicast.
>>
>> - Ethernet Packet Sniffer backend: provides the driver for the
>> Linn Ethernet Packet Sniffer H/W modules.
>>
>> The split between a core and backend modules allows software-only
>> implementations to be added for platforms where no H/W support
>> is available.
>>
>> Based on 3.19-rc5
>>
>> Signed-off-by: Stathis Voukelatos <stathis.voukelatos@linn.co.uk>
> Please have a look at packet sockets, they offer already all the
> functionality (if not more) your driver interface to the user space
> resembles, are transparent to the underlying hardware, and easily
> can cope with 100Mbit.
>
> If I understand this correctly, you are effectively introducing a
> parallel API *next* to packet sockets to user space that we have to
> maintain forever ...
>
> Thanks !
>
Hello Daniel. Thank you for your feedback.
Packet sockets could also be used for the driver interface to
user space, however I think that both approaches would require the same
amount of maintenance. We need to maintain a protocol consisting of
a set of messages or commands that user space can use to communicate
with the driver in order to configure the H/W and retrieve results.
We could use packet sockets to send those messages too, but I thought
netlink already provides a message exchange framework that we could
make use of.
^ permalink raw reply
* Re: [PATCH v6 0/7] can: kvaser_usb: Leaf bugfixes and USBCan-II support
From: Marc Kleine-Budde @ 2015-01-26 9:55 UTC (permalink / raw)
To: Ahmed S. Darwish, Olivier Sobrie, Oliver Hartkopp,
Wolfgang Grandegger, Andri Yngvason
Cc: Linux-CAN, netdev, LKML
In-Reply-To: <20150126051755.GA3014@Darwish.PC>
[-- Attachment #1: Type: text/plain, Size: 1060 bytes --]
On 01/26/2015 06:17 AM, Ahmed S. Darwish wrote:
> Hi!
>
> This is an updated patch series for the Kvaser CAN/USB devices:
>
> 1- Extra patches are now added to the series. Most importantly
> patch #1 which fixes a critical `sleep in atomic context' bug
> in the current upstream driver. Patch #2 fixes a corruption in
> the kernel logs, also affecting current upstream driver. Patch
> #4 was originally USBCan-II only, but since it's a generic fix,
> it's now retrofitted to the Leaf-only upstream driver first.
>
> 2- The series has been re-organized so that patches #1 -> #4
> inclusive can go to linux-can/origin, while the rest can move
> to -next.
Applied 1-4 to can/master, I'm adding stable on Cc before sending a
pull-request to David.
Thanks,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v6 0/7] can: kvaser_usb: Leaf bugfixes and USBCan-II support
From: Marc Kleine-Budde @ 2015-01-26 10:07 UTC (permalink / raw)
To: Ahmed S. Darwish, Olivier Sobrie, Oliver Hartkopp,
Wolfgang Grandegger, Andri Yngvason
Cc: Linux-CAN, netdev, LKML
In-Reply-To: <20150126051755.GA3014@Darwish.PC>
[-- Attachment #1: Type: text/plain, Size: 1645 bytes --]
On 01/26/2015 06:17 AM, Ahmed S. Darwish wrote:
> Hi!
>
> This is an updated patch series for the Kvaser CAN/USB devices:
>
> 1- Extra patches are now added to the series. Most importantly
> patch #1 which fixes a critical `sleep in atomic context' bug
> in the current upstream driver. Patch #2 fixes a corruption in
> the kernel logs, also affecting current upstream driver. Patch
> #4 was originally USBCan-II only, but since it's a generic fix,
> it's now retrofitted to the Leaf-only upstream driver first.
>
> 2- The series has been re-organized so that patches #1 -> #4
> inclusive can go to linux-can/origin, while the rest can move
> to -next.
>
> 3- There are some important updates regarding the USBCan-II
> error counters, and how really erratic their heaviour is. All
> the new details are covered at the bottom of this URL:
>
> http://article.gmane.org/gmane.linux.can/7481
>
> 4- Attached below is the new candump traces. Now
> `back-to-error-active' states appear _if_ the hardware was
> kind enough and decreased the error counters appropriately.
> The earlier code did not recognize the error counters going
> down, thus the `back-to-error-active' transitions did not
> appear.
For patches 5-7 I'm waiting for an Acked by Andri.
...and we have to wait until Patches 1-4 will show up on net-next/master.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] net: Linn Ethernet Packet Sniffer driver
From: Daniel Borkmann @ 2015-01-26 10:10 UTC (permalink / raw)
To: Stathis Voukelatos
Cc: Stathis Voukelatos, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
abrestic@chromium.org
In-Reply-To: <54C60DB8.1060900@linn.co.uk>
Hi Stathis,
On 01/26/2015 10:49 AM, Stathis Voukelatos wrote:
> On 23/01/15 11:20, Daniel Borkmann wrote:
>> On 01/23/2015 11:07 AM, Stathis Voukelatos wrote:
>>> This patch adds support the Ethernet Packet Sniffer H/W module
>>> developed by Linn Products Ltd and found in the IMG Pistachio SoC.
>>> The module allows Ethernet packets to be parsed, matched against
>>> a user-defined pattern and timestamped. It sits between a 100M
>>> Ethernet MAC and PHY and is completely passive with respect to
>>> Ethernet frames.
>>>
>>> Matched packet bytes and timestamp values are returned through a
>>> FIFO. Timestamps are provided to the module through an externally
>>> generated Gray-encoded counter.
>>>
>>> The command pattern for packet matching is stored in module RAM
>>> and consists of a sequence of 16-bit entries. Each entry includes
>>> an 8-bit command code and and 8-bit data value. Valid command
>>> codes are:
>>> 0 - Don't care
>>> 1 - Match: packet data must match command string byte
>>> 2 - Copy: packet data will be copied to FIFO
>>> 3 - Match/Stamp: if packet data matches string byte, a timestamp
>>> is copied into the FIFO
>>> 4 - Copy/Done: packet data will be copied into the FIFO.
>>> This command terminates the command string.
>>>
>>> The driver consists of two modules:
>>> - Core: it provides an API to user space using the Generic Netlink
>>> framework. Specific backend implementations, like the
>>> Ethernet Packet Sniffer, register one or more channels
>>> with the Core. For each channel a Genl family is created.
>>> User space can access a channel by sending Genl messages
>>> to the Genl family associated with the channel. Packet
>>> matching events are multicast.
>>>
>>> - Ethernet Packet Sniffer backend: provides the driver for the
>>> Linn Ethernet Packet Sniffer H/W modules.
>>>
>>> The split between a core and backend modules allows software-only
>>> implementations to be added for platforms where no H/W support
>>> is available.
>>>
>>> Based on 3.19-rc5
>>>
>>> Signed-off-by: Stathis Voukelatos <stathis.voukelatos@linn.co.uk>
>> Please have a look at packet sockets, they offer already all the
>> functionality (if not more) your driver interface to the user space
>> resembles, are transparent to the underlying hardware, and easily
>> can cope with 100Mbit.
>>
>> If I understand this correctly, you are effectively introducing a
>> parallel API *next* to packet sockets to user space that we have to
>> maintain forever ...
>>
>> Thanks !
>
> Hello Daniel. Thank you for your feedback.
> Packet sockets could also be used for the driver interface to
> user space, however I think that both approaches would require the same
> amount of maintenance. We need to maintain a protocol consisting of
> a set of messages or commands that user space can use to communicate
> with the driver in order to configure the H/W and retrieve results.
> We could use packet sockets to send those messages too, but I thought
> netlink already provides a message exchange framework that we could
> make use of.
When using packet sockets and your driver as a backend feeding them,
users can see that there's an extra capturing/monitoring netdev present,
all libpcap-based tools such as tcpdump et al would work out of the box
w/o adapting any code, and as an admin you can also see what users/tools
are making of use of the device through packet sockets. I couldn't parse
the exact motivation from the commit message of why avoiding all this is
better?
Thanks,
Daniel
^ permalink raw reply
* RE: [PATCH 1/2] rhashtable: Introduce rhashtable_walk_*
From: David Laight @ 2015-01-26 10:09 UTC (permalink / raw)
To: 'Herbert Xu', Thomas Graf, Ying Xue, davem@davemloft.net,
kaber@trash.net, paulmck@linux.vnet.ibm.com,
netdev@vger.kernel.org, netfilter-devel@vger.kernel.org
In-Reply-To: <E1YFWUl-0004gE-SH@gondolin.me.apana.org.au>
From: Herbert Xu
> Some existing rhashtable users get too intimate with it by walking
> the buckets directly. This prevents us from easily changing the
> internals of rhashtable.
>
> This patch adds the helpers rhashtable_walk_init/next/end which
> will replace these custom walkers.
>
> They are meant to be usable for both procfs seq_file walks as well
> as walking by a netlink dump. The iterator structure should fit
> inside a netlink dump cb structure, with at least one element to
> spare.
...
> +/**
> + * rhashtable_walk_start - Start a hash table walk
> + * @iter: Hash table iterator
> + *
> + * Start a hash table walk.
> + *
> + * Returns zero if successful. Returns -EINTR if we couldn't
> + * obtain the resize lock.
> + */
> +int rhashtable_walk_start(struct rhashtable_iter *iter)
> +{
> + struct rhashtable *ht = iter->ht;
> + int err;
> +
> + err = mutex_lock_interruptible(&ht->mutex);
> + rcu_read_lock();
> +
> + if (!err)
> + mutex_unlock(&ht->mutex);
> +
> + return err;
> +}
That doesn't look right to me.
Surely you shouldn't be calling rcu_read_lock() when the mutex
request is interrupted.
So maybe:
err = mutex_lock_interruptible(&ht->mutex);
if (err)
return err;
rcu_read_lock();
return 0;
}
David
^ permalink raw reply
* Re: [PATCH] net: Linn Ethernet Packet Sniffer driver
From: Stathis Voukelatos @ 2015-01-26 10:16 UTC (permalink / raw)
To: Mark Rutland, Stathis Voukelatos
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
devicetree@vger.kernel.org, abrestic@chromium.org
In-Reply-To: <20150123105114.GB23493@leverpostej>
On 23/01/15 10:51, Mark Rutland wrote:
>
> diff --git a/Documentation/devicetree/bindings/net/linn-ether-packet-sniffer.txt b/Documentation/devicetree/bindings/net/linn-ether-packet-sniffer.txt
> new file mode 100644
> index 0000000..6b6e105
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/linn-ether-packet-sniffer.txt
> @@ -0,0 +1,27 @@
> +* Linn Products Ethernet Packet Sniffer
> +
> +Required properties:
> +- compatible : must be "linn,eth-sniffer"
> +- reg : physical addresses and sizes of registers. Must contain 3 entries:
> + first entry: registers memory space
> + second entry: TX command memory
> + third entry: RX command memory
> Just to check: are those memories are part of the packet sniffer device,
> or are carveouts from other memory?
Yes, the 3 memory areas are part of the packet sniffer module.
>> +- fifo-block-words : number of words in one data FIFO entry
>> +
>> +Example:
>> +
>> +sniffer@1814a000 {
>> + compatible = "linn,eth-sniffer";
>> + reg = <0x1814a000 0x100>, <0x1814a400 0x400>, <0x1814a800 0x400>;
>> + reg-names = "regs", "tx-ram", "rx-ram";
>> + interrupts = <GIC_SHARED 58 IRQ_TYPE_LEVEL_HIGH>;
>> + interrupt-names = "eth-sniffer-irq";
>> + clocks = <&system_clk>;
>> + clock-names = "sys";
>> + fifo-block-words = <4>;
>> + };
> Surely the relationship between the sniffer, MAC, and PHY should be
> described, so we know which interface the sniffer is related to?
>
The packet sniffer sits between the MAC and the PHY and monitors TX or RX
packets, or both. Will add a description in the binding doc.
Your other suggestions too, will be incorporated in the next version of
the patch set.
Thank you,
Stathis
^ permalink raw reply
* Re: [PATCH v5 2/5] can: kvaser_usb: Consolidate and unify state change handling
From: Andri Yngvason @ 2015-01-26 10:21 UTC (permalink / raw)
To: Ahmed S. Darwish
Cc: Wolfgang Grandegger, Olivier Sobrie, Oliver Hartkopp,
Marc Kleine-Budde, Linux-CAN, netdev, LKML
In-Reply-To: <20150125024300.GA6028@Darwish.PC>
Quoting Ahmed S. Darwish (2015-01-25 02:43:00)
> On Fri, Jan 23, 2015 at 10:32:13AM +0000, Andri Yngvason wrote:
> > Quoting Ahmed S. Darwish (2015-01-23 06:07:34)
> > > On Wed, Jan 21, 2015 at 05:13:45PM +0100, Wolfgang Grandegger wrote:
> > > > On Wed, 21 Jan 2015 10:36:47 -0500, "Ahmed S. Darwish"
> > > > <darwish.07@gmail.com> wrote:
> > > > > On Wed, Jan 21, 2015 at 03:00:15PM +0000, Andri Yngvason wrote:
> > > > >> Quoting Ahmed S. Darwish (2015-01-21 14:43:23)
> > > > >> > Hi!
> > > > >
> > > > > ...
> > > > >
> > > > >> > <-- Unplug the cable -->
> > > > >> >
> > > > >> > (000.009106) can0 20000080 [8] 00 00 00 00 00 00 08 00
> > > > >> > ERRORFRAME
> > > > >> > bus-error
> > > > >> > error-counter-tx-rx{{8}{0}}
> > > > >> > (000.001872) can0 20000080 [8] 00 00 00 00 00 00 10 00
> > > >
> > > > For a bus-errors I would also expcect some more information in the
> > > > data[2..3] fields. But these are always zero.
> > > >
> > >
> > > M16C error factors made it possible to report things like
> > > CAN_ERR_PROT_FORM/STUFF/BIT0/BIT1/TX in data[2], and
> > > CAN_ERR_PROT_LOC_ACK/CRC_DEL in data[3].
> > >
> > > Unfortunately such error factors are only reported in Leaf, but
> > > not in USBCan-II due to the wire format change in the error event:
> > >
> > > struct leaf_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 usbcan_msg_error_event {
> > > u8 tid;
> > > u8 padding;
> > > u8 tx_errors_count_ch0;
> > > u8 rx_errors_count_ch0;
> > > u8 tx_errors_count_ch1;
> > > u8 rx_errors_count_ch1;
> > > u8 status_ch0;
> > > u8 status_ch1;
> > > __le16 time;
> > > } __packed;
> > >
> > > I speculate that the wire format was changed due to controller
> > > bugs in the USBCan-II, which was slightly mentioned in their
> > > data sheets here:
> > >
> > > http://www.kvaser.com/canlib-webhelp/page_hardware_specific_can_controllers.html
> > >
> > > So it seems there's really no way for filling such bus error
> > > info given the very limited amount of data exported :-(
> > >
> > We experienced similar problems with FlexCAN.
>
> Hmm, I'll have a look there then...
>
> Although my initial instincts imply that the FlexCAN driver has
> access to the raw CAN registers, something I'm unable to do here.
> But maybe there's some black magic I'm missing :-)
>
Yes, it's memory mapped.
>
> [...]
>
> > >
> > > I've dumped _every_ message I receive from the firmware while
> > > disconnecting the CAN bus, waiting a while, and connecting it again.
> > > I really received _nothing_ from the firmware when the CAN bus was
> > > reconnected and the data packets were flowing again. Not even a
> > > single CHIP_STATE_EVENT, even after waiting for a long time.
> > >
> > > So it's basically:
> > > ...
> > > ERR EVENT, txerr=128, rxerr=0
> > > ERR EVENT, txerr=128, rxerr=0
> > > ERR EVENT, txerr=128, rxerr=0
> > > ...
> > >
> > > then complete silence, except the data frames. I've even tried with
> > > different versions of the firmware, but the same behaviour persisted.
> > >
> > > > > So, What can the driver do given the above?
> > > >
> > > > Little if the notification does not come.
> > > >
> > >
> > > We can poll the state by sending CMD_GET_CHIP_STATE to the firmware,
> > > and it will hopefully reply with a CHIP_STATE_EVENT response
> > > containing the new txerr and rxerr values that we can use for
> > > reverse state transitions.
> > >
> > > But do we _really_ want to go through the path? I feel that it will
> > > open some cans of worms w.r.t. concurrent access to both the netdev
> > > and USB stacks from a single driver.
> > >
> > Honestly, I don't know.
> > >
> > > A possible solution can be setting up a kernel thread that queries
> > > for a CHIP_STATE_EVENT every second?
> > >
> > Have you considered polling in kvaser_usb_tx_acknowledge? You could do something
> > like:
> > if(unlikely(dev->can.state != CAN_STATE_ERROR_ACTIVE))
> > {
> > request_state();
> > }
> >
>
> OK, I have four important updates on this issue:
>
> a) My initial testing was done on high-speed channel, at a bitrate
> of 50K. After setting the bus to a more reasonable bitrate 500K
> or 1M, I was _consistently_ able to receive CHIP_STATE_EVENTs
> when plugging the CAN connector again after an unplug.
>
> b) The error counters on this device do not get reset on plugging
> after an unplug. I've setup a kernel thread [2] that queries
> the chip state event every second, and the error counters stays
> the same all the time. [1]
>
> c) There's a single case when the erro counters do indeed get
> reversed, and it happens only when introducing some noise in
> the bus after the re-plug. In that case, the new error events
> get raised with new error counters starting from 0/1 again.
>
> d) I've discovered a bug that forbids the CAN state from
> returning to ERROR_ACTIVE in case of the error counters
> numbers getting decreased. But independent from that bug, the
> verbose debugging messages clearly imply that we only get the
> error counters decreased in the case mentioned at `c)' above.
>
> So from [1] and [2], it's now clear that the device do not reset
> these counters back in the re-plug case. I'll give a check to
> flexcan as advised, but unfortunately I don't really think there's
> much I can do about this.
>
> [1]
>
> [ 877.207082] CAN_ERROR_: channel=0, txerr=88, rxerr=0
> [ 877.207090] CAN_ERROR_: channel=0, txerr=136, rxerr=0
> [ 877.207094] CAN_ERROR_: channel=0, txerr=144, rxerr=0
> [ 877.207098] CAN_ERROR_: channel=0, txerr=152, rxerr=0
> [ 877.207100] CAN_ERROR_: channel=0, txerr=160, rxerr=0
> [ 877.207102] CAN_ERROR_: channel=0, txerr=168, rxerr=0
> [ 877.208075] CAN_ERROR_: channel=0, txerr=200, rxerr=0
>
> (( The above error event, staying the same at txerr=200 keeps
> flooding the bus until the CAN cable is re-plugged ))
>
> [ 878.225116] CHIP_STATE: channel=0, txerr=200, rxerr=0
> [ 878.225143] CHIP_STATE: channel=1, txerr=0, rxerr=0
> [ 879.265167] CHIP_STATE: channel=0, txerr=200, rxerr=0
> [ 879.267152] CHIP_STATE: channel=1, txerr=0, rxerr=0
> [ 879.265167] CHIP_STATE: channel=0, txerr=200, rxerr=0
> [ 879.267152] CHIP_STATE: channel=1, txerr=0, rxerr=0
>
> (( The same counters get repeated every second ))
>
> [2] State was polled using:
>
> static int kvaser_usb_poll_chip_state(void *vpriv) {
> struct kvaser_usb_net_priv *priv = vpriv;
>
> while (!kthread_should_stop()) {
> kvaser_usb_simple_msg_async(priv, CMD_GET_CHIP_STATE);
> ssleep(1);
> }
>
> return 0;
> }
>
> > I don't think that anything beyond that would be worth pursuing.
> >
>
> I agree, but given the new input, it seems that our problem
> extends to the error counters themselves not getting decreased
> on re-plug. So, even polling will not solve the issue: we'll
> get the same txerr/rxerr values again and again :-(
>
Well, that's a damn shame. Not much you can do about that.
Best regards,
Andri
^ permalink raw reply
* Re: [PATCH v6 6/7] can: kvaser_usb: Consolidate and unify state change handling
From: Andri Yngvason @ 2015-01-26 10:26 UTC (permalink / raw)
To: Ahmed S. Darwish, Olivier Sobrie, Oliver Hartkopp,
Wolfgang Grandegger, Marc Kleine-Budde
Cc: Linux-CAN, netdev, LKML
In-Reply-To: <20150126052915.GG3014@Darwish.PC>
Quoting Ahmed S. Darwish (2015-01-26 05:29:15)
> From: Ahmed S. Darwish <ahmed.darwish@valeo.com>
>
> Replace most of the can interface's state and error counters
> handling with the new can-dev can_change_state() mechanism.
>
> Suggested-by: Andri Yngvason <andri.yngvason@marel.com>
> Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com>
> ---
> drivers/net/can/usb/kvaser_usb.c | 113 ++++++++++++++++---------------------
> 1 files changed, 49 insertions(+), 64 deletions(-)
>
> diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
> index f57ce55..ddc2954 100644
> --- a/drivers/net/can/usb/kvaser_usb.c
> +++ b/drivers/net/can/usb/kvaser_usb.c
> @@ -620,39 +620,44 @@ static void kvaser_usb_unlink_tx_urbs(struct kvaser_usb_net_priv *priv)
> }
>
> static void kvaser_usb_rx_error_update_can_state(struct kvaser_usb_net_priv *priv,
> - const struct kvaser_usb_error_summary *es)
> + const struct kvaser_usb_error_summary *es,
> + struct can_frame *cf)
> {
> struct net_device_stats *stats;
> - enum can_state new_state;
> -
> - stats = &priv->netdev->stats;
> - new_state = priv->can.state;
> + enum can_state cur_state, new_state, tx_state, rx_state;
>
> netdev_dbg(priv->netdev, "Error status: 0x%02x\n", es->status);
>
> - if (es->status & (M16C_STATE_BUS_OFF | M16C_STATE_BUS_RESET)) {
> - priv->can.can_stats.bus_off++;
> + stats = &priv->netdev->stats;
> + new_state = cur_state = priv->can.state;
> +
> + if (es->status & (M16C_STATE_BUS_OFF | M16C_STATE_BUS_RESET))
> new_state = CAN_STATE_BUS_OFF;
> - } else if (es->status & M16C_STATE_BUS_PASSIVE) {
> - if (priv->can.state != CAN_STATE_ERROR_PASSIVE)
> - priv->can.can_stats.error_passive++;
> + else if (es->status & M16C_STATE_BUS_PASSIVE)
> new_state = CAN_STATE_ERROR_PASSIVE;
> - } else if (es->status & M16C_STATE_BUS_ERROR) {
> - if ((priv->can.state < CAN_STATE_ERROR_WARNING) &&
> - ((es->txerr >= 96) || (es->rxerr >= 96))) {
> - priv->can.can_stats.error_warning++;
> + else if (es->status & M16C_STATE_BUS_ERROR) {
> + if ((es->txerr >= 256) || (es->rxerr >= 256))
> + new_state = CAN_STATE_BUS_OFF;
> + else if ((es->txerr >= 128) || (es->rxerr >= 128))
> + new_state = CAN_STATE_ERROR_PASSIVE;
> + else if ((es->txerr >= 96) || (es->rxerr >= 96))
> new_state = CAN_STATE_ERROR_WARNING;
> - } else if ((priv->can.state > CAN_STATE_ERROR_ACTIVE) &&
> - ((es->txerr < 96) && (es->rxerr < 96))) {
> + else if (cur_state > CAN_STATE_ERROR_ACTIVE)
> new_state = CAN_STATE_ERROR_ACTIVE;
> - }
> }
>
> if (!es->status)
> new_state = CAN_STATE_ERROR_ACTIVE;
>
> + if (new_state != cur_state) {
> + tx_state = (es->txerr >= es->rxerr) ? new_state : 0;
> + rx_state = (es->txerr <= es->rxerr) ? new_state : 0;
> +
> + can_change_state(priv->netdev, cf, tx_state, rx_state);
> + }
> +
> if (priv->can.restart_ms &&
> - (priv->can.state >= CAN_STATE_BUS_OFF) &&
> + (cur_state >= CAN_STATE_BUS_OFF) &&
> (new_state < CAN_STATE_BUS_OFF)) {
> priv->can.can_stats.restarts++;
> }
> @@ -664,18 +669,17 @@ static void kvaser_usb_rx_error_update_can_state(struct kvaser_usb_net_priv *pri
>
> priv->bec.txerr = es->txerr;
> priv->bec.rxerr = es->rxerr;
> - priv->can.state = new_state;
> }
>
> static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
> const struct kvaser_msg *msg)
> {
> - struct can_frame *cf;
> + struct can_frame *cf, tmp_cf = { .can_id = CAN_ERR_FLAG, .can_dlc = CAN_ERR_DLC };
> struct sk_buff *skb;
> struct net_device_stats *stats;
> struct kvaser_usb_net_priv *priv;
> struct kvaser_usb_error_summary es = { };
> - enum can_state old_state;
> + enum can_state old_state, new_state;
>
> switch (msg->id) {
> case CMD_CAN_ERROR_EVENT:
> @@ -715,59 +719,40 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
> stats = &priv->netdev->stats;
>
> /* Update all of the can interface's state and error counters before
> - * trying any skb allocation that can actually fail with -ENOMEM.
> + * trying any memory allocation that can actually fail with -ENOMEM.
> + *
> + * We send a temporary stack-allocated error can frame to
> + * can_change_state() for the very same reason.
> + *
> + * TODO: Split can_change_state() responsibility between updating the
> + * can interface's state and counters, and the setting up of can error
> + * frame ID and data to userspace. Remove stack allocation afterwards.
> */
> old_state = priv->can.state;
> - kvaser_usb_rx_error_update_can_state(priv, &es);
> + kvaser_usb_rx_error_update_can_state(priv, &es, &tmp_cf);
> + new_state = priv->can.state;
>
> skb = alloc_can_err_skb(priv->netdev, &cf);
> if (!skb) {
> stats->rx_dropped++;
> return;
> }
> -
> - if (es.status & (M16C_STATE_BUS_OFF | M16C_STATE_BUS_RESET)) {
> - cf->can_id |= CAN_ERR_BUSOFF;
> -
> - if (!priv->can.restart_ms)
> - kvaser_usb_simple_msg_async(priv, CMD_STOP_CHIP);
> - netif_carrier_off(priv->netdev);
> - } else if (es.status & M16C_STATE_BUS_PASSIVE) {
> - if (old_state != CAN_STATE_ERROR_PASSIVE) {
> - cf->can_id |= CAN_ERR_CRTL;
> -
> - if (es.txerr || es.rxerr)
> - cf->data[1] = (es.txerr > es.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;
> - }
> - } else if (es.status & M16C_STATE_BUS_ERROR) {
> - if ((old_state < CAN_STATE_ERROR_WARNING) &&
> - ((es.txerr >= 96) || (es.rxerr >= 96))) {
> - cf->can_id |= CAN_ERR_CRTL;
> - cf->data[1] = (es.txerr > es.rxerr)
> - ? CAN_ERR_CRTL_TX_WARNING
> - : CAN_ERR_CRTL_RX_WARNING;
> - } else if ((old_state > CAN_STATE_ERROR_ACTIVE) &&
> - ((es.txerr < 96) && (es.rxerr < 96))) {
> - cf->can_id |= CAN_ERR_PROT;
> - cf->data[2] = CAN_ERR_PROT_ACTIVE;
> + memcpy(cf, &tmp_cf, sizeof(*cf));
> +
> + if (new_state != old_state) {
> + if (es.status &
> + (M16C_STATE_BUS_OFF | M16C_STATE_BUS_RESET)) {
> + if (!priv->can.restart_ms)
> + kvaser_usb_simple_msg_async(priv, CMD_STOP_CHIP);
> + netif_carrier_off(priv->netdev);
> }
> - }
> -
> - if (!es.status) {
> - cf->can_id |= CAN_ERR_PROT;
> - cf->data[2] = CAN_ERR_PROT_ACTIVE;
> - }
>
> - if (priv->can.restart_ms &&
> - (old_state >= CAN_STATE_BUS_OFF) &&
> - (priv->can.state < CAN_STATE_BUS_OFF)) {
> - cf->can_id |= CAN_ERR_RESTARTED;
> - netif_carrier_on(priv->netdev);
> + if (priv->can.restart_ms &&
> + (old_state >= CAN_STATE_BUS_OFF) &&
> + (new_state < CAN_STATE_BUS_OFF)) {
> + cf->can_id |= CAN_ERR_RESTARTED;
> + netif_carrier_on(priv->netdev);
> + }
> }
>
> if (es.error_factor) {
> --
> 1.7.7.6
ACK, for real this time. ;)
--
Andri
^ permalink raw reply
* Re: [PATCH v6 5/7] can: kvaser_usb: Update interface state before exiting on OOM
From: Andri Yngvason @ 2015-01-26 10:28 UTC (permalink / raw)
To: Ahmed S. Darwish, Olivier Sobrie, Oliver Hartkopp,
Wolfgang Grandegger, Marc Kleine-Budde
Cc: Linux-CAN, netdev, LKML
In-Reply-To: <20150126052719.GF3014@Darwish.PC>
Quoting Ahmed S. Darwish (2015-01-26 05:27:19)
> From: Ahmed S. Darwish <ahmed.darwish@valeo.com>
>
> Update all of the can interface's state and error counters before
> trying any skb allocation that can actually fail with -ENOMEM.
>
> Suggested-by: Marc Kleine-Budde <mkl@pengutronix.de>
> Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com>
> ---
> drivers/net/can/usb/kvaser_usb.c | 181 ++++++++++++++++++++++----------------
> 1 files changed, 105 insertions(+), 76 deletions(-)
>
> diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
> index 7af379c..f57ce55 100644
> --- a/drivers/net/can/usb/kvaser_usb.c
> +++ b/drivers/net/can/usb/kvaser_usb.c
> @@ -273,6 +273,10 @@ struct kvaser_msg {
> } u;
> } __packed;
>
> +struct kvaser_usb_error_summary {
> + u8 channel, status, txerr, rxerr, error_factor;
> +};
> +
> struct kvaser_usb_tx_urb_context {
> struct kvaser_usb_net_priv *priv;
> u32 echo_index;
> @@ -615,6 +619,54 @@ static void kvaser_usb_unlink_tx_urbs(struct kvaser_usb_net_priv *priv)
> priv->tx_contexts[i].echo_index = MAX_TX_URBS;
> }
>
> +static void kvaser_usb_rx_error_update_can_state(struct kvaser_usb_net_priv *priv,
> + const struct kvaser_usb_error_summary *es)
> +{
> + struct net_device_stats *stats;
> + enum can_state new_state;
> +
> + stats = &priv->netdev->stats;
> + new_state = priv->can.state;
> +
> + netdev_dbg(priv->netdev, "Error status: 0x%02x\n", es->status);
> +
> + if (es->status & (M16C_STATE_BUS_OFF | M16C_STATE_BUS_RESET)) {
> + priv->can.can_stats.bus_off++;
> + new_state = CAN_STATE_BUS_OFF;
> + } else if (es->status & M16C_STATE_BUS_PASSIVE) {
> + if (priv->can.state != CAN_STATE_ERROR_PASSIVE)
> + priv->can.can_stats.error_passive++;
> + new_state = CAN_STATE_ERROR_PASSIVE;
> + } else if (es->status & M16C_STATE_BUS_ERROR) {
> + if ((priv->can.state < CAN_STATE_ERROR_WARNING) &&
> + ((es->txerr >= 96) || (es->rxerr >= 96))) {
> + priv->can.can_stats.error_warning++;
> + new_state = CAN_STATE_ERROR_WARNING;
> + } else if ((priv->can.state > CAN_STATE_ERROR_ACTIVE) &&
> + ((es->txerr < 96) && (es->rxerr < 96))) {
> + new_state = CAN_STATE_ERROR_ACTIVE;
> + }
> + }
> +
> + if (!es->status)
> + new_state = CAN_STATE_ERROR_ACTIVE;
> +
> + if (priv->can.restart_ms &&
> + (priv->can.state >= CAN_STATE_BUS_OFF) &&
> + (new_state < CAN_STATE_BUS_OFF)) {
> + priv->can.can_stats.restarts++;
> + }
> +
> + if (es->error_factor) {
> + priv->can.can_stats.bus_error++;
> + stats->rx_errors++;
> + }
> +
> + priv->bec.txerr = es->txerr;
> + priv->bec.rxerr = es->rxerr;
> + priv->can.state = new_state;
> +}
> +
> static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
> const struct kvaser_msg *msg)
> {
> @@ -622,30 +674,30 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
> 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;
> + struct kvaser_usb_error_summary es = { };
> + enum can_state old_state;
>
> 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;
> + es.channel = msg->u.error_event.channel;
> + es.status = msg->u.error_event.status;
> + es.txerr = msg->u.error_event.tx_errors_count;
> + es.rxerr = msg->u.error_event.rx_errors_count;
> + es.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];
> + es.channel = msg->u.log_message.channel;
> + es.status = msg->u.log_message.data[0];
> + es.txerr = msg->u.log_message.data[2];
> + es.rxerr = msg->u.log_message.data[3];
> + es.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;
> + es.channel = msg->u.chip_state_event.channel;
> + es.status = msg->u.chip_state_event.status;
> + es.txerr = msg->u.chip_state_event.tx_errors_count;
> + es.rxerr = msg->u.chip_state_event.rx_errors_count;
> + es.error_factor = 0;
> break;
> default:
> dev_err(dev->udev->dev.parent, "Invalid msg id (%d)\n",
> @@ -653,116 +705,93 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
> return;
> }
>
> - if (channel >= dev->nchannels) {
> + if (es.channel >= dev->nchannels) {
> dev_err(dev->udev->dev.parent,
> - "Invalid channel number (%d)\n", channel);
> + "Invalid channel number (%d)\n", es.channel);
> return;
> }
>
> - priv = dev->nets[channel];
> + priv = dev->nets[es.channel];
> stats = &priv->netdev->stats;
>
> + /* Update all of the can interface's state and error counters before
> + * trying any skb allocation that can actually fail with -ENOMEM.
> + */
> + old_state = priv->can.state;
> + kvaser_usb_rx_error_update_can_state(priv, &es);
> +
> skb = alloc_can_err_skb(priv->netdev, &cf);
> if (!skb) {
> stats->rx_dropped++;
> return;
> }
>
> - new_state = priv->can.state;
> -
> - netdev_dbg(priv->netdev, "Error status: 0x%02x\n", status);
> -
> - if (status & (M16C_STATE_BUS_OFF | M16C_STATE_BUS_RESET)) {
> + if (es.status & (M16C_STATE_BUS_OFF | M16C_STATE_BUS_RESET)) {
> 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;
> - } else if (status & M16C_STATE_BUS_PASSIVE) {
> - if (priv->can.state != CAN_STATE_ERROR_PASSIVE) {
> + } else if (es.status & M16C_STATE_BUS_PASSIVE) {
> + if (old_state != CAN_STATE_ERROR_PASSIVE) {
> cf->can_id |= CAN_ERR_CRTL;
>
> - if (txerr || rxerr)
> - cf->data[1] = (txerr > rxerr)
> + if (es.txerr || es.rxerr)
> + cf->data[1] = (es.txerr > es.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;
> - } else if (status & M16C_STATE_BUS_ERROR) {
> - if ((priv->can.state < CAN_STATE_ERROR_WARNING) &&
> - ((txerr >= 96) || (rxerr >= 96))) {
> + } else if (es.status & M16C_STATE_BUS_ERROR) {
> + if ((old_state < CAN_STATE_ERROR_WARNING) &&
> + ((es.txerr >= 96) || (es.rxerr >= 96))) {
> cf->can_id |= CAN_ERR_CRTL;
> - cf->data[1] = (txerr > rxerr)
> + cf->data[1] = (es.txerr > es.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) &&
> - ((txerr < 96) && (rxerr < 96))) {
> + } else if ((old_state > CAN_STATE_ERROR_ACTIVE) &&
> + ((es.txerr < 96) && (es.rxerr < 96))) {
> cf->can_id |= CAN_ERR_PROT;
> cf->data[2] = CAN_ERR_PROT_ACTIVE;
> -
> - new_state = CAN_STATE_ERROR_ACTIVE;
> }
> }
>
> - if (!status) {
> + if (!es.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)) {
> + (old_state >= CAN_STATE_BUS_OFF) &&
> + (priv->can.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++;
> -
> + if (es.error_factor) {
> cf->can_id |= CAN_ERR_BUSERROR | CAN_ERR_PROT;
>
> - if (error_factor & M16C_EF_ACKE)
> + if (es.error_factor & M16C_EF_ACKE)
> cf->data[3] |= (CAN_ERR_PROT_LOC_ACK);
> - if (error_factor & M16C_EF_CRCE)
> + if (es.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)
> + if (es.error_factor & M16C_EF_FORME)
> cf->data[2] |= CAN_ERR_PROT_FORM;
> - if (error_factor & M16C_EF_STFE)
> + if (es.error_factor & M16C_EF_STFE)
> cf->data[2] |= CAN_ERR_PROT_STUFF;
> - if (error_factor & M16C_EF_BITE0)
> + if (es.error_factor & M16C_EF_BITE0)
> cf->data[2] |= CAN_ERR_PROT_BIT0;
> - if (error_factor & M16C_EF_BITE1)
> + if (es.error_factor & M16C_EF_BITE1)
> cf->data[2] |= CAN_ERR_PROT_BIT1;
> - if (error_factor & M16C_EF_TRE)
> + if (es.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;
> + cf->data[6] = es.txerr;
> + cf->data[7] = es.rxerr;
>
> stats->rx_packets++;
> stats->rx_bytes += cf->can_dlc;
> @@ -786,6 +815,9 @@ static void kvaser_usb_rx_can_err(const struct kvaser_usb_net_priv *priv,
> }
>
> if (msg->u.rx_can.flag & MSG_FLAG_OVERRUN) {
> + stats->rx_over_errors++;
> + stats->rx_errors++;
> +
> skb = alloc_can_err_skb(priv->netdev, &cf);
> if (!skb) {
> stats->rx_dropped++;
> @@ -795,9 +827,6 @@ static void kvaser_usb_rx_can_err(const struct kvaser_usb_net_priv *priv,
> cf->can_id |= CAN_ERR_CRTL;
> cf->data[1] = CAN_ERR_CRTL_RX_OVERFLOW;
>
> - stats->rx_over_errors++;
> - stats->rx_errors++;
> -
> stats->rx_packets++;
> stats->rx_bytes += cf->can_dlc;
> netif_rx(skb);
> --
> 1.7.7.6
Looks good to me.
--
Andri
^ permalink raw reply
* Re: [PATCH v6 7/7] can: kvaser_usb: Add support for the USBcan-II family
From: Andri Yngvason @ 2015-01-26 10:34 UTC (permalink / raw)
To: Ahmed S. Darwish, Olivier Sobrie, Oliver Hartkopp,
Wolfgang Grandegger, Marc Kleine-Budde
Cc: Linux-CAN, netdev, LKML
In-Reply-To: <20150126053310.GH3014@Darwish.PC>
Quoting Ahmed S. Darwish (2015-01-26 05:33:10)
> From: Ahmed S. Darwish <ahmed.darwish@valeo.com>
>
> CAN to USB interfaces sold by the Swedish manufacturer Kvaser are
> divided into two major families: 'Leaf', and 'USBcanII'. From an
> Operating System perspective, the firmware of both families behave
> in a not too drastically different fashion.
>
> This patch adds support for the USBcanII family of devices to the
> current Kvaser Leaf-only driver.
>
> CAN frames sending, receiving, and error handling paths has been
> tested using the dual-channel "Kvaser USBcan II HS/LS" dongle. It
> should also work nicely with other products in the same category.
>
> List of new devices supported by this driver update:
>
> - Kvaser USBcan II HS/HS
> - Kvaser USBcan II HS/LS
> - Kvaser USBcan Rugged ("USBcan Rev B")
> - Kvaser Memorator HS/HS
> - Kvaser Memorator HS/LS
> - Scania VCI2 (if you have the Kvaser logo on top)
>
> Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com>
> ---
> drivers/net/can/usb/Kconfig | 8 +-
> drivers/net/can/usb/kvaser_usb.c | 590 ++++++++++++++++++++++++++++++--------
> 2 files changed, 474 insertions(+), 124 deletions(-)
>
> ** V6 Changelog:
> - Revert to the error-active state if the error counters were
> decreased by hardware
> - Rebase over a new set of upstream Leaf-driver bugfixes
>
> ** V5 Changelog:
> - Rebase on the new CAN error state changes added for the Leaf driver
> - Add minor changes (remove unused commands, constify poniters, etc.)
>
> ** V4 Changelog:
> - Use type-safe C methods instead of cpp macros
> - Remove defensive checks against non-existing families
> - Re-order methods to remove forward declarations
> - Smaller stuff spotted by earlier review (function prefexes, etc.)
>
> ** V3 Changelog:
> - Fix padding for the usbcan_msg_tx_acknowledge command
> - Remove kvaser_usb->max_channels and the MAX_NET_DEVICES macro
> - Rename commands to CMD_LEAF_xxx and CMD_USBCAN_xxx
> - Apply checkpatch.pl suggestions ('net/' comments, multi-line strings, etc.)
>
> ** V2 Changelog:
> - Update Kconfig entries
> - Use actual number of CAN channels (instead of max) where appropriate
> - Rebase over a new set of UsbcanII-independent driver fixes
>
> diff --git a/drivers/net/can/usb/Kconfig b/drivers/net/can/usb/Kconfig
> index a77db919..f6f5500 100644
> --- a/drivers/net/can/usb/Kconfig
> +++ b/drivers/net/can/usb/Kconfig
> @@ -25,7 +25,7 @@ config CAN_KVASER_USB
> tristate "Kvaser CAN/USB interface"
> ---help---
> This driver adds support for Kvaser CAN/USB devices like Kvaser
> - Leaf Light.
> + Leaf Light and Kvaser USBcan II.
>
> The driver provides support for the following devices:
> - Kvaser Leaf Light
> @@ -46,6 +46,12 @@ config CAN_KVASER_USB
> - Kvaser USBcan R
> - Kvaser Leaf Light v2
> - Kvaser Mini PCI Express HS
> + - Kvaser USBcan II HS/HS
> + - Kvaser USBcan II HS/LS
> + - Kvaser USBcan Rugged ("USBcan Rev B")
> + - Kvaser Memorator HS/HS
> + - Kvaser Memorator HS/LS
> + - Scania VCI2 (if you have the Kvaser logo on top)
>
> If unsure, say N.
>
> diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
> index ddc2954..17d28d7 100644
> --- a/drivers/net/can/usb/kvaser_usb.c
> +++ b/drivers/net/can/usb/kvaser_usb.c
> @@ -6,10 +6,12 @@
> * Parts of this driver are based on the following:
> * - Kvaser linux leaf driver (version 4.78)
> * - CAN driver for esd CAN-USB/2
> + * - Kvaser linux usbcanII driver (version 5.3)
> *
> * 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>
> + * Copyright (C) 2015 Valeo A.S.
> */
>
> #include <linux/completion.h>
> @@ -30,8 +32,9 @@
> #define RX_BUFFER_SIZE 3072
> #define CAN_USB_CLOCK 8000000
> #define MAX_NET_DEVICES 3
> +#define MAX_USBCAN_NET_DEVICES 2
>
> -/* Kvaser USB devices */
> +/* Kvaser Leaf USB devices */
> #define KVASER_VENDOR_ID 0x0bfd
> #define USB_LEAF_DEVEL_PRODUCT_ID 10
> #define USB_LEAF_LITE_PRODUCT_ID 11
> @@ -56,6 +59,24 @@
> #define USB_LEAF_LITE_V2_PRODUCT_ID 288
> #define USB_MINI_PCIE_HS_PRODUCT_ID 289
>
> +static inline bool kvaser_is_leaf(const struct usb_device_id *id)
> +{
> + return id->idProduct >= USB_LEAF_DEVEL_PRODUCT_ID &&
> + id->idProduct <= USB_MINI_PCIE_HS_PRODUCT_ID;
> +}
> +
> +/* Kvaser USBCan-II devices */
> +#define USB_USBCAN_REVB_PRODUCT_ID 2
> +#define USB_VCI2_PRODUCT_ID 3
> +#define USB_USBCAN2_PRODUCT_ID 4
> +#define USB_MEMORATOR_PRODUCT_ID 5
> +
> +static inline bool kvaser_is_usbcan(const struct usb_device_id *id)
> +{
> + return id->idProduct >= USB_USBCAN_REVB_PRODUCT_ID &&
> + id->idProduct <= USB_MEMORATOR_PRODUCT_ID;
> +}
> +
> /* USB devices features */
> #define KVASER_HAS_SILENT_MODE BIT(0)
> #define KVASER_HAS_TXRX_ERRORS BIT(1)
> @@ -73,7 +94,7 @@
> #define MSG_FLAG_TX_ACK BIT(6)
> #define MSG_FLAG_TX_REQUEST BIT(7)
>
> -/* Can states */
> +/* Can states (M16C CxSTRH register) */
> #define M16C_STATE_BUS_RESET BIT(0)
> #define M16C_STATE_BUS_ERROR BIT(4)
> #define M16C_STATE_BUS_PASSIVE BIT(5)
> @@ -98,7 +119,11 @@
> #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_LEAF_GET_CARD_INFO2 32
> +#define CMD_USBCAN_RESET_CLOCK 32
> +#define CMD_USBCAN_CLOCK_OVERFLOW_EVENT 33
> +
> #define CMD_GET_CARD_INFO 34
> #define CMD_GET_CARD_INFO_REPLY 35
> #define CMD_GET_SOFTWARE_INFO 38
> @@ -108,8 +133,9 @@
> #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
> +
> +#define CMD_LEAF_USB_THROTTLE 77
> +#define CMD_LEAF_LOG_MESSAGE 106
>
> /* error factors */
> #define M16C_EF_ACKE BIT(0)
> @@ -121,6 +147,14 @@
> #define M16C_EF_RCVE BIT(6)
> #define M16C_EF_TRE BIT(7)
>
> +/* Only Leaf-based devices can report M16C error factors,
> + * thus define our own error status flags for USBCANII
> + */
> +#define USBCAN_ERROR_STATE_NONE 0
> +#define USBCAN_ERROR_STATE_TX_ERROR BIT(0)
> +#define USBCAN_ERROR_STATE_RX_ERROR BIT(1)
> +#define USBCAN_ERROR_STATE_BUSERROR BIT(2)
> +
> /* bittiming parameters */
> #define KVASER_USB_TSEG1_MIN 1
> #define KVASER_USB_TSEG1_MAX 16
> @@ -137,9 +171,18 @@
> #define KVASER_CTRL_MODE_SELFRECEPTION 3
> #define KVASER_CTRL_MODE_OFF 4
>
> -/* log message */
> +/* Extended CAN identifier flag */
> #define KVASER_EXTENDED_FRAME BIT(31)
>
> +/* Kvaser USB CAN dongles are divided into two major families:
> + * - Leaf: Based on Renesas M32C, running firmware labeled as 'filo'
> + * - UsbcanII: Based on Renesas M16C, running firmware labeled as 'helios'
> + */
> +enum kvaser_usb_family {
> + KVASER_LEAF,
> + KVASER_USBCAN,
> +};
> +
> struct kvaser_msg_simple {
> u8 tid;
> u8 channel;
> @@ -148,30 +191,55 @@ struct kvaser_msg_simple {
> struct kvaser_msg_cardinfo {
> u8 tid;
> u8 nchannels;
> - __le32 serial_number;
> - __le32 padding;
> + union {
> + struct {
> + __le32 serial_number;
> + __le32 padding;
> + } __packed leaf0;
> + struct {
> + __le32 serial_number_low;
> + __le32 serial_number_high;
> + } __packed usbcan0;
> + } __packed;
> __le32 clock_resolution;
> __le32 mfgdate;
> u8 ean[8];
> u8 hw_revision;
> - u8 usb_hs_mode;
> - __le16 padding2;
> + union {
> + struct {
> + u8 usb_hs_mode;
> + } __packed leaf1;
> + struct {
> + u8 padding;
> + } __packed usbcan1;
> + } __packed;
> + __le16 padding;
> } __packed;
>
> struct kvaser_msg_cardinfo2 {
> u8 tid;
> - u8 channel;
> + u8 reserved;
> u8 pcb_id[24];
> __le32 oem_unlock_code;
> } __packed;
>
> -struct kvaser_msg_softinfo {
> +struct leaf_msg_softinfo {
> u8 tid;
> - u8 channel;
> + u8 padding0;
> __le32 sw_options;
> __le32 fw_version;
> __le16 max_outstanding_tx;
> - __le16 padding[9];
> + __le16 padding1[9];
> +} __packed;
> +
> +struct usbcan_msg_softinfo {
> + u8 tid;
> + u8 fw_name[5];
> + __le16 max_outstanding_tx;
> + u8 padding[6];
> + __le32 fw_version;
> + __le16 checksum;
> + __le16 sw_options;
> } __packed;
>
> struct kvaser_msg_busparams {
> @@ -188,36 +256,86 @@ struct kvaser_msg_tx_can {
> u8 channel;
> u8 tid;
> u8 msg[14];
> - u8 padding;
> - u8 flags;
> + union {
> + struct {
> + u8 padding;
> + u8 flags;
> + } __packed leaf;
> + struct {
> + u8 flags;
> + u8 padding;
> + } __packed usbcan;
> + } __packed;
> +} __packed;
> +
> +struct kvaser_msg_rx_can_header {
> + u8 channel;
> + u8 flag;
> } __packed;
>
> -struct kvaser_msg_rx_can {
> +struct leaf_msg_rx_can {
> u8 channel;
> u8 flag;
> +
> __le16 time[3];
> u8 msg[14];
> } __packed;
>
> -struct kvaser_msg_chip_state_event {
> +struct usbcan_msg_rx_can {
> + u8 channel;
> + u8 flag;
> +
> + u8 msg[14];
> + __le16 time;
> +} __packed;
> +
> +struct leaf_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 usbcan_msg_chip_state_event {
> + u8 tid;
> + u8 channel;
> +
> + u8 tx_errors_count;
> + u8 rx_errors_count;
> + __le16 time;
> +
> u8 status;
> u8 padding[3];
> } __packed;
>
> -struct kvaser_msg_tx_acknowledge {
> +struct kvaser_msg_tx_acknowledge_header {
> u8 channel;
> u8 tid;
> +} __packed;
> +
> +struct leaf_msg_tx_acknowledge {
> + u8 channel;
> + u8 tid;
> +
> __le16 time[3];
> u8 flags;
> u8 time_offset;
> } __packed;
>
> -struct kvaser_msg_error_event {
> +struct usbcan_msg_tx_acknowledge {
> + u8 channel;
> + u8 tid;
> +
> + __le16 time;
> + __le16 padding;
> +} __packed;
> +
> +struct leaf_msg_error_event {
> u8 tid;
> u8 flags;
> __le16 time[3];
> @@ -229,6 +347,18 @@ struct kvaser_msg_error_event {
> u8 error_factor;
> } __packed;
>
> +struct usbcan_msg_error_event {
> + u8 tid;
> + u8 padding;
> + u8 tx_errors_count_ch0;
> + u8 rx_errors_count_ch0;
> + u8 tx_errors_count_ch1;
> + u8 rx_errors_count_ch1;
> + u8 status_ch0;
> + u8 status_ch1;
> + __le16 time;
> +} __packed;
> +
> struct kvaser_msg_ctrl_mode {
> u8 tid;
> u8 channel;
> @@ -243,7 +373,7 @@ struct kvaser_msg_flush_queue {
> u8 padding[3];
> } __packed;
>
> -struct kvaser_msg_log_message {
> +struct leaf_msg_log_message {
> u8 channel;
> u8 flags;
> __le16 time[3];
> @@ -260,21 +390,55 @@ struct kvaser_msg {
> 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_rx_can_header rx_can_header;
> + struct kvaser_msg_tx_acknowledge_header tx_acknowledge_header;
> +
> + union {
> + struct leaf_msg_softinfo softinfo;
> + struct leaf_msg_rx_can rx_can;
> + struct leaf_msg_chip_state_event chip_state_event;
> + struct leaf_msg_tx_acknowledge tx_acknowledge;
> + struct leaf_msg_error_event error_event;
> + struct leaf_msg_log_message log_message;
> + } __packed leaf;
> +
> + union {
> + struct usbcan_msg_softinfo softinfo;
> + struct usbcan_msg_rx_can rx_can;
> + struct usbcan_msg_chip_state_event chip_state_event;
> + struct usbcan_msg_tx_acknowledge tx_acknowledge;
> + struct usbcan_msg_error_event error_event;
> + } __packed usbcan;
> +
> 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;
>
> +/* Summary of a kvaser error event, for a unified Leaf/Usbcan error
> + * handling. Some discrepancies between the two families exist:
> + *
> + * - USBCAN firmware does not report M16C "error factors"
> + * - USBCAN controllers has difficulties reporting if the raised error
> + * event is for ch0 or ch1. They leave such arbitration to the OS
> + * driver by letting it compare error counters with previous values
> + * and decide the error event's channel. Thus for USBCAN, the channel
> + * field is only advisory.
> + */
> struct kvaser_usb_error_summary {
> - u8 channel, status, txerr, rxerr, error_factor;
> + u8 channel, status, txerr, rxerr;
> + union {
> + struct {
> + u8 error_factor;
> + } leaf;
> + struct {
> + u8 other_ch_status;
> + u8 error_state;
> + } usbcan;
> + };
> };
>
> struct kvaser_usb_tx_urb_context {
> @@ -292,6 +456,7 @@ struct kvaser_usb {
>
> u32 fw_version;
> unsigned int nchannels;
> + enum kvaser_usb_family family;
>
> bool rxinitdone;
> void *rxbuf[MAX_RX_URBS];
> @@ -315,6 +480,7 @@ struct kvaser_usb_net_priv {
> };
>
> static const struct usb_device_id kvaser_usb_table[] = {
> + /* Leaf family IDs */
> { 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),
> @@ -364,6 +530,17 @@ static const struct usb_device_id kvaser_usb_table[] = {
> .driver_info = KVASER_HAS_TXRX_ERRORS },
> { USB_DEVICE(KVASER_VENDOR_ID, USB_LEAF_LITE_V2_PRODUCT_ID) },
> { USB_DEVICE(KVASER_VENDOR_ID, USB_MINI_PCIE_HS_PRODUCT_ID) },
> +
> + /* USBCANII family IDs */
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_USBCAN2_PRODUCT_ID),
> + .driver_info = KVASER_HAS_TXRX_ERRORS },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_USBCAN_REVB_PRODUCT_ID),
> + .driver_info = KVASER_HAS_TXRX_ERRORS },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_MEMORATOR_PRODUCT_ID),
> + .driver_info = KVASER_HAS_TXRX_ERRORS },
> + { USB_DEVICE(KVASER_VENDOR_ID, USB_VCI2_PRODUCT_ID),
> + .driver_info = KVASER_HAS_TXRX_ERRORS },
> +
> { }
> };
> MODULE_DEVICE_TABLE(usb, kvaser_usb_table);
> @@ -467,7 +644,14 @@ static int kvaser_usb_get_software_info(struct kvaser_usb *dev)
> if (err)
> return err;
>
> - dev->fw_version = le32_to_cpu(msg.u.softinfo.fw_version);
> + switch (dev->family) {
> + case KVASER_LEAF:
> + dev->fw_version = le32_to_cpu(msg.u.leaf.softinfo.fw_version);
> + break;
> + case KVASER_USBCAN:
> + dev->fw_version = le32_to_cpu(msg.u.usbcan.softinfo.fw_version);
> + break;
> + }
>
> return 0;
> }
> @@ -486,7 +670,9 @@ static int kvaser_usb_get_card_info(struct kvaser_usb *dev)
> return err;
>
> dev->nchannels = msg.u.cardinfo.nchannels;
> - if (dev->nchannels > MAX_NET_DEVICES)
> + if ((dev->nchannels > MAX_NET_DEVICES) ||
> + (dev->family == KVASER_USBCAN &&
> + dev->nchannels > MAX_USBCAN_NET_DEVICES))
> return -EINVAL;
>
> return 0;
> @@ -500,8 +686,10 @@ static void kvaser_usb_tx_acknowledge(const struct kvaser_usb *dev,
> 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;
> + u8 channel, tid;
> +
> + channel = msg->u.tx_acknowledge_header.channel;
> + tid = msg->u.tx_acknowledge_header.tid;
>
> if (channel >= dev->nchannels) {
> dev_err(dev->udev->dev.parent,
> @@ -623,12 +811,12 @@ static void kvaser_usb_rx_error_update_can_state(struct kvaser_usb_net_priv *pri
> const struct kvaser_usb_error_summary *es,
> struct can_frame *cf)
> {
> - struct net_device_stats *stats;
> + struct kvaser_usb *dev = priv->dev;
> + struct net_device_stats *stats = &priv->netdev->stats;
> enum can_state cur_state, new_state, tx_state, rx_state;
>
> netdev_dbg(priv->netdev, "Error status: 0x%02x\n", es->status);
>
> - stats = &priv->netdev->stats;
> new_state = cur_state = priv->can.state;
>
> if (es->status & (M16C_STATE_BUS_OFF | M16C_STATE_BUS_RESET))
> @@ -662,9 +850,22 @@ static void kvaser_usb_rx_error_update_can_state(struct kvaser_usb_net_priv *pri
> priv->can.can_stats.restarts++;
> }
>
> - if (es->error_factor) {
> - priv->can.can_stats.bus_error++;
> - stats->rx_errors++;
> + switch (dev->family) {
> + case KVASER_LEAF:
> + if (es->leaf.error_factor) {
> + priv->can.can_stats.bus_error++;
> + stats->rx_errors++;
> + }
> + break;
> + case KVASER_USBCAN:
> + if (es->usbcan.error_state & USBCAN_ERROR_STATE_TX_ERROR)
> + stats->tx_errors++;
> + if (es->usbcan.error_state & USBCAN_ERROR_STATE_RX_ERROR)
> + stats->rx_errors++;
> + if (es->usbcan.error_state & USBCAN_ERROR_STATE_BUSERROR) {
> + priv->can.can_stats.bus_error++;
> + }
> + break;
> }
>
> priv->bec.txerr = es->txerr;
> @@ -672,50 +873,21 @@ static void kvaser_usb_rx_error_update_can_state(struct kvaser_usb_net_priv *pri
> }
>
> static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
> - const struct kvaser_msg *msg)
> + const struct kvaser_usb_error_summary *es)
> {
> struct can_frame *cf, tmp_cf = { .can_id = CAN_ERR_FLAG, .can_dlc = CAN_ERR_DLC };
> struct sk_buff *skb;
> struct net_device_stats *stats;
> struct kvaser_usb_net_priv *priv;
> - struct kvaser_usb_error_summary es = { };
> enum can_state old_state, new_state;
>
> - switch (msg->id) {
> - case CMD_CAN_ERROR_EVENT:
> - es.channel = msg->u.error_event.channel;
> - es.status = msg->u.error_event.status;
> - es.txerr = msg->u.error_event.tx_errors_count;
> - es.rxerr = msg->u.error_event.rx_errors_count;
> - es.error_factor = msg->u.error_event.error_factor;
> - break;
> - case CMD_LOG_MESSAGE:
> - es.channel = msg->u.log_message.channel;
> - es.status = msg->u.log_message.data[0];
> - es.txerr = msg->u.log_message.data[2];
> - es.rxerr = msg->u.log_message.data[3];
> - es.error_factor = msg->u.log_message.data[1];
> - break;
> - case CMD_CHIP_STATE_EVENT:
> - es.channel = msg->u.chip_state_event.channel;
> - es.status = msg->u.chip_state_event.status;
> - es.txerr = msg->u.chip_state_event.tx_errors_count;
> - es.rxerr = msg->u.chip_state_event.rx_errors_count;
> - es.error_factor = 0;
> - break;
> - default:
> - dev_err(dev->udev->dev.parent, "Invalid msg id (%d)\n",
> - msg->id);
> - return;
> - }
> -
> - if (es.channel >= dev->nchannels) {
> + if (es->channel >= dev->nchannels) {
> dev_err(dev->udev->dev.parent,
> - "Invalid channel number (%d)\n", es.channel);
> + "Invalid channel number (%d)\n", es->channel);
> return;
> }
>
> - priv = dev->nets[es.channel];
> + priv = dev->nets[es->channel];
> stats = &priv->netdev->stats;
>
> /* Update all of the can interface's state and error counters before
> @@ -729,7 +901,7 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
> * frame ID and data to userspace. Remove stack allocation afterwards.
> */
> old_state = priv->can.state;
> - kvaser_usb_rx_error_update_can_state(priv, &es, &tmp_cf);
> + kvaser_usb_rx_error_update_can_state(priv, es, &tmp_cf);
> new_state = priv->can.state;
>
> skb = alloc_can_err_skb(priv->netdev, &cf);
> @@ -740,7 +912,7 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
> memcpy(cf, &tmp_cf, sizeof(*cf));
>
> if (new_state != old_state) {
> - if (es.status &
> + if (es->status &
> (M16C_STATE_BUS_OFF | M16C_STATE_BUS_RESET)) {
> if (!priv->can.restart_ms)
> kvaser_usb_simple_msg_async(priv, CMD_STOP_CHIP);
> @@ -755,34 +927,161 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
> }
> }
>
> - if (es.error_factor) {
> - cf->can_id |= CAN_ERR_BUSERROR | CAN_ERR_PROT;
> -
> - if (es.error_factor & M16C_EF_ACKE)
> - cf->data[3] |= (CAN_ERR_PROT_LOC_ACK);
> - if (es.error_factor & M16C_EF_CRCE)
> - cf->data[3] |= (CAN_ERR_PROT_LOC_CRC_SEQ |
> - CAN_ERR_PROT_LOC_CRC_DEL);
> - if (es.error_factor & M16C_EF_FORME)
> - cf->data[2] |= CAN_ERR_PROT_FORM;
> - if (es.error_factor & M16C_EF_STFE)
> - cf->data[2] |= CAN_ERR_PROT_STUFF;
> - if (es.error_factor & M16C_EF_BITE0)
> - cf->data[2] |= CAN_ERR_PROT_BIT0;
> - if (es.error_factor & M16C_EF_BITE1)
> - cf->data[2] |= CAN_ERR_PROT_BIT1;
> - if (es.error_factor & M16C_EF_TRE)
> - cf->data[2] |= CAN_ERR_PROT_TX;
> + switch (dev->family) {
> + case KVASER_LEAF:
> + if (es->leaf.error_factor) {
> + cf->can_id |= CAN_ERR_BUSERROR | CAN_ERR_PROT;
> +
> + if (es->leaf.error_factor & M16C_EF_ACKE)
> + cf->data[3] |= (CAN_ERR_PROT_LOC_ACK);
> + if (es->leaf.error_factor & M16C_EF_CRCE)
> + cf->data[3] |= (CAN_ERR_PROT_LOC_CRC_SEQ |
> + CAN_ERR_PROT_LOC_CRC_DEL);
> + if (es->leaf.error_factor & M16C_EF_FORME)
> + cf->data[2] |= CAN_ERR_PROT_FORM;
> + if (es->leaf.error_factor & M16C_EF_STFE)
> + cf->data[2] |= CAN_ERR_PROT_STUFF;
> + if (es->leaf.error_factor & M16C_EF_BITE0)
> + cf->data[2] |= CAN_ERR_PROT_BIT0;
> + if (es->leaf.error_factor & M16C_EF_BITE1)
> + cf->data[2] |= CAN_ERR_PROT_BIT1;
> + if (es->leaf.error_factor & M16C_EF_TRE)
> + cf->data[2] |= CAN_ERR_PROT_TX;
> + }
> + break;
> + case KVASER_USBCAN:
> + if (es->usbcan.error_state & USBCAN_ERROR_STATE_BUSERROR) {
> + cf->can_id |= CAN_ERR_BUSERROR;
> + }
> + break;
> }
>
> - cf->data[6] = es.txerr;
> - cf->data[7] = es.rxerr;
> + cf->data[6] = es->txerr;
> + cf->data[7] = es->rxerr;
>
> stats->rx_packets++;
> stats->rx_bytes += cf->can_dlc;
> netif_rx(skb);
> }
>
> +/* For USBCAN, report error to userspace iff the channels's errors counter
> + * has changed, or we're the only channel seeing a bus error state.
> + */
> +static void kvaser_usbcan_conditionally_rx_error(const struct kvaser_usb *dev,
> + struct kvaser_usb_error_summary *es)
> +{
> + struct kvaser_usb_net_priv *priv;
> + int channel;
> + bool report_error;
> +
> + channel = es->channel;
> + if (channel >= dev->nchannels) {
> + dev_err(dev->udev->dev.parent,
> + "Invalid channel number (%d)\n", channel);
> + return;
> + }
> +
> + priv = dev->nets[channel];
> + report_error = false;
> +
> + if (es->txerr != priv->bec.txerr) {
> + es->usbcan.error_state |= USBCAN_ERROR_STATE_TX_ERROR;
> + report_error = true;
> + }
> + if (es->rxerr != priv->bec.rxerr) {
> + es->usbcan.error_state |= USBCAN_ERROR_STATE_RX_ERROR;
> + report_error = true;
> + }
> + if ((es->status & M16C_STATE_BUS_ERROR) &&
> + !(es->usbcan.other_ch_status & M16C_STATE_BUS_ERROR)) {
> + es->usbcan.error_state |= USBCAN_ERROR_STATE_BUSERROR;
> + report_error = true;
> + }
> +
> + if (report_error)
> + kvaser_usb_rx_error(dev, es);
> +}
> +
> +static void kvaser_usbcan_rx_error(const struct kvaser_usb *dev,
> + const struct kvaser_msg *msg)
> +{
> + struct kvaser_usb_error_summary es = { };
> +
> + switch (msg->id) {
> + /* Sometimes errors are sent as unsolicited chip state events */
> + case CMD_CHIP_STATE_EVENT:
> + es.channel = msg->u.usbcan.chip_state_event.channel;
> + es.status = msg->u.usbcan.chip_state_event.status;
> + es.txerr = msg->u.usbcan.chip_state_event.tx_errors_count;
> + es.rxerr = msg->u.usbcan.chip_state_event.rx_errors_count;
> + kvaser_usbcan_conditionally_rx_error(dev, &es);
> + break;
> +
> + case CMD_CAN_ERROR_EVENT:
> + es.channel = 0;
> + es.status = msg->u.usbcan.error_event.status_ch0;
> + es.txerr = msg->u.usbcan.error_event.tx_errors_count_ch0;
> + es.rxerr = msg->u.usbcan.error_event.rx_errors_count_ch0;
> + es.usbcan.other_ch_status =
> + msg->u.usbcan.error_event.status_ch1;
> + kvaser_usbcan_conditionally_rx_error(dev, &es);
> +
> + /* The USBCAN firmware supports up to 2 channels.
> + * Now that ch0 was checked, check if ch1 has any errors.
> + */
> + if (dev->nchannels == MAX_USBCAN_NET_DEVICES) {
> + es.channel = 1;
> + es.status = msg->u.usbcan.error_event.status_ch1;
> + es.txerr = msg->u.usbcan.error_event.tx_errors_count_ch1;
> + es.rxerr = msg->u.usbcan.error_event.rx_errors_count_ch1;
> + es.usbcan.other_ch_status =
> + msg->u.usbcan.error_event.status_ch0;
> + kvaser_usbcan_conditionally_rx_error(dev, &es);
> + }
> + break;
> +
> + default:
> + dev_err(dev->udev->dev.parent, "Invalid msg id (%d)\n",
> + msg->id);
> + }
> +}
> +
> +static void kvaser_leaf_rx_error(const struct kvaser_usb *dev,
> + const struct kvaser_msg *msg)
> +{
> + struct kvaser_usb_error_summary es = { };
> +
> + switch (msg->id) {
> + case CMD_CAN_ERROR_EVENT:
> + es.channel = msg->u.leaf.error_event.channel;
> + es.status = msg->u.leaf.error_event.status;
> + es.txerr = msg->u.leaf.error_event.tx_errors_count;
> + es.rxerr = msg->u.leaf.error_event.rx_errors_count;
> + es.leaf.error_factor = msg->u.leaf.error_event.error_factor;
> + break;
> + case CMD_LEAF_LOG_MESSAGE:
> + es.channel = msg->u.leaf.log_message.channel;
> + es.status = msg->u.leaf.log_message.data[0];
> + es.txerr = msg->u.leaf.log_message.data[2];
> + es.rxerr = msg->u.leaf.log_message.data[3];
> + es.leaf.error_factor = msg->u.leaf.log_message.data[1];
> + break;
> + case CMD_CHIP_STATE_EVENT:
> + es.channel = msg->u.leaf.chip_state_event.channel;
> + es.status = msg->u.leaf.chip_state_event.status;
> + es.txerr = msg->u.leaf.chip_state_event.tx_errors_count;
> + es.rxerr = msg->u.leaf.chip_state_event.rx_errors_count;
> + es.leaf.error_factor = 0;
> + break;
> + default:
> + dev_err(dev->udev->dev.parent, "Invalid msg id (%d)\n",
> + msg->id);
> + return;
> + }
> +
> + kvaser_usb_rx_error(dev, &es);
> +}
> +
> static void kvaser_usb_rx_can_err(const struct kvaser_usb_net_priv *priv,
> const struct kvaser_msg *msg)
> {
> @@ -790,16 +1089,16 @@ static void kvaser_usb_rx_can_err(const struct kvaser_usb_net_priv *priv,
> struct sk_buff *skb;
> struct net_device_stats *stats = &priv->netdev->stats;
>
> - if (msg->u.rx_can.flag & (MSG_FLAG_ERROR_FRAME |
> + if (msg->u.rx_can_header.flag & (MSG_FLAG_ERROR_FRAME |
> MSG_FLAG_NERR)) {
> netdev_err(priv->netdev, "Unknow error (flags: 0x%02x)\n",
> - msg->u.rx_can.flag);
> + msg->u.rx_can_header.flag);
>
> stats->rx_errors++;
> return;
> }
>
> - if (msg->u.rx_can.flag & MSG_FLAG_OVERRUN) {
> + if (msg->u.rx_can_header.flag & MSG_FLAG_OVERRUN) {
> stats->rx_over_errors++;
> stats->rx_errors++;
>
> @@ -825,7 +1124,8 @@ static void kvaser_usb_rx_can_msg(const struct kvaser_usb *dev,
> struct can_frame *cf;
> struct sk_buff *skb;
> struct net_device_stats *stats;
> - u8 channel = msg->u.rx_can.channel;
> + u8 channel = msg->u.rx_can_header.channel;
> + const u8 *rx_msg = NULL; /* GCC */
>
> if (channel >= dev->nchannels) {
> dev_err(dev->udev->dev.parent,
> @@ -836,60 +1136,68 @@ static void kvaser_usb_rx_can_msg(const struct kvaser_usb *dev,
> priv = dev->nets[channel];
> stats = &priv->netdev->stats;
>
> - if ((msg->u.rx_can.flag & MSG_FLAG_ERROR_FRAME) &&
> - (msg->id == CMD_LOG_MESSAGE)) {
> - kvaser_usb_rx_error(dev, msg);
> + if ((msg->u.rx_can_header.flag & MSG_FLAG_ERROR_FRAME) &&
> + (dev->family == KVASER_LEAF && msg->id == CMD_LEAF_LOG_MESSAGE)) {
> + kvaser_leaf_rx_error(dev, msg);
> return;
> - } else if (msg->u.rx_can.flag & (MSG_FLAG_ERROR_FRAME |
> - MSG_FLAG_NERR |
> - MSG_FLAG_OVERRUN)) {
> + } else if (msg->u.rx_can_header.flag & (MSG_FLAG_ERROR_FRAME |
> + MSG_FLAG_NERR |
> + MSG_FLAG_OVERRUN)) {
> kvaser_usb_rx_can_err(priv, msg);
> return;
> - } else if (msg->u.rx_can.flag & ~MSG_FLAG_REMOTE_FRAME) {
> + } else if (msg->u.rx_can_header.flag & ~MSG_FLAG_REMOTE_FRAME) {
> netdev_warn(priv->netdev,
> "Unhandled frame (flags: 0x%02x)",
> - msg->u.rx_can.flag);
> + msg->u.rx_can_header.flag);
> return;
> }
>
> + switch (dev->family) {
> + case KVASER_LEAF:
> + rx_msg = msg->u.leaf.rx_can.msg;
> + break;
> + case KVASER_USBCAN:
> + rx_msg = msg->u.usbcan.rx_can.msg;
> + break;
> + }
> +
> skb = alloc_can_skb(priv->netdev, &cf);
> if (!skb) {
> stats->tx_dropped++;
> return;
> }
>
> - if (msg->id == CMD_LOG_MESSAGE) {
> - cf->can_id = le32_to_cpu(msg->u.log_message.id);
> + if (dev->family == KVASER_LEAF && msg->id == CMD_LEAF_LOG_MESSAGE) {
> + cf->can_id = le32_to_cpu(msg->u.leaf.log_message.id);
> if (cf->can_id & KVASER_EXTENDED_FRAME)
> cf->can_id &= CAN_EFF_MASK | CAN_EFF_FLAG;
> else
> cf->can_id &= CAN_SFF_MASK;
>
> - cf->can_dlc = get_can_dlc(msg->u.log_message.dlc);
> + cf->can_dlc = get_can_dlc(msg->u.leaf.log_message.dlc);
>
> - if (msg->u.log_message.flags & MSG_FLAG_REMOTE_FRAME)
> + if (msg->u.leaf.log_message.flags & MSG_FLAG_REMOTE_FRAME)
> cf->can_id |= CAN_RTR_FLAG;
> else
> - memcpy(cf->data, &msg->u.log_message.data,
> + memcpy(cf->data, &msg->u.leaf.log_message.data,
> cf->can_dlc);
> } else {
> - cf->can_id = ((msg->u.rx_can.msg[0] & 0x1f) << 6) |
> - (msg->u.rx_can.msg[1] & 0x3f);
> + cf->can_id = ((rx_msg[0] & 0x1f) << 6) | (rx_msg[1] & 0x3f);
>
> 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 |= ((rx_msg[2] & 0x0f) << 14) |
> + ((rx_msg[3] & 0xff) << 6) |
> + (rx_msg[4] & 0x3f);
> cf->can_id |= CAN_EFF_FLAG;
> }
>
> - cf->can_dlc = get_can_dlc(msg->u.rx_can.msg[5]);
> + cf->can_dlc = get_can_dlc(rx_msg[5]);
>
> - if (msg->u.rx_can.flag & MSG_FLAG_REMOTE_FRAME)
> + if (msg->u.rx_can_header.flag & MSG_FLAG_REMOTE_FRAME)
> cf->can_id |= CAN_RTR_FLAG;
> else
> - memcpy(cf->data, &msg->u.rx_can.msg[6],
> + memcpy(cf->data, &rx_msg[6],
> cf->can_dlc);
> }
>
> @@ -952,21 +1260,35 @@ static void kvaser_usb_handle_message(const struct kvaser_usb *dev,
>
> case CMD_RX_STD_MESSAGE:
> case CMD_RX_EXT_MESSAGE:
> - case CMD_LOG_MESSAGE:
> + kvaser_usb_rx_can_msg(dev, msg);
> + break;
> +
> + case CMD_LEAF_LOG_MESSAGE:
> + if (dev->family != KVASER_LEAF)
> + goto warn;
> kvaser_usb_rx_can_msg(dev, msg);
> break;
>
> case CMD_CHIP_STATE_EVENT:
> case CMD_CAN_ERROR_EVENT:
> - kvaser_usb_rx_error(dev, msg);
> + if (dev->family == KVASER_LEAF)
> + kvaser_leaf_rx_error(dev, msg);
> + else
> + kvaser_usbcan_rx_error(dev, msg);
> break;
>
> case CMD_TX_ACKNOWLEDGE:
> kvaser_usb_tx_acknowledge(dev, msg);
> break;
>
> + /* Ignored messages */
> + case CMD_USBCAN_CLOCK_OVERFLOW_EVENT:
> + if (dev->family != KVASER_USBCAN)
> + goto warn;
> + break;
> +
> default:
> - dev_warn(dev->udev->dev.parent,
> +warn: dev_warn(dev->udev->dev.parent,
> "Unhandled message (%d)\n", msg->id);
> break;
> }
> @@ -1186,7 +1508,7 @@ static void kvaser_usb_unlink_all_urbs(struct kvaser_usb *dev)
> dev->rxbuf[i],
> dev->rxbuf_dma[i]);
>
> - for (i = 0; i < MAX_NET_DEVICES; i++) {
> + for (i = 0; i < dev->nchannels; i++) {
> struct kvaser_usb_net_priv *priv = dev->nets[i];
>
> if (priv)
> @@ -1294,6 +1616,7 @@ static netdev_tx_t kvaser_usb_start_xmit(struct sk_buff *skb,
> struct kvaser_msg *msg;
> int i, err;
> int ret = NETDEV_TX_OK;
> + u8 *msg_tx_can_flags = NULL; /* GCC */
>
> if (can_dropped_invalid_skb(netdev, skb))
> return NETDEV_TX_OK;
> @@ -1315,9 +1638,19 @@ static netdev_tx_t kvaser_usb_start_xmit(struct sk_buff *skb,
>
> msg = buf;
> msg->len = MSG_HEADER_LEN + sizeof(struct kvaser_msg_tx_can);
> - msg->u.tx_can.flags = 0;
> msg->u.tx_can.channel = priv->channel;
>
> + switch (dev->family) {
> + case KVASER_LEAF:
> + msg_tx_can_flags = &msg->u.tx_can.leaf.flags;
> + break;
> + case KVASER_USBCAN:
> + msg_tx_can_flags = &msg->u.tx_can.usbcan.flags;
> + break;
> + }
> +
> + *msg_tx_can_flags = 0;
> +
> if (cf->can_id & CAN_EFF_FLAG) {
> msg->id = CMD_TX_EXT_MESSAGE;
> msg->u.tx_can.msg[0] = (cf->can_id >> 24) & 0x1f;
> @@ -1335,7 +1668,7 @@ static netdev_tx_t kvaser_usb_start_xmit(struct sk_buff *skb,
> 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;
> + *msg_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) {
> @@ -1604,6 +1937,17 @@ static int kvaser_usb_probe(struct usb_interface *intf,
> if (!dev)
> return -ENOMEM;
>
> + if (kvaser_is_leaf(id)) {
> + dev->family = KVASER_LEAF;
> + } else if (kvaser_is_usbcan(id)) {
> + dev->family = KVASER_USBCAN;
> + } else {
> + dev_err(&intf->dev,
> + "Product ID (%d) does not belong to any known Kvaser USB family",
> + id->idProduct);
> + return -ENODEV;
> + }
> +
> err = kvaser_usb_get_endpoints(intf, &dev->bulk_in, &dev->bulk_out);
> if (err) {
> dev_err(&intf->dev, "Cannot get usb endpoint(s)");
> --
> 1.7.7.6
There's a lot of changes here but as far as error state is concerned, I see
nothing out of the ordinary.
--
Andri
^ permalink raw reply
* Re: [PATCH v6 5/7] can: kvaser_usb: Update interface state before exiting on OOM
From: Marc Kleine-Budde @ 2015-01-26 10:50 UTC (permalink / raw)
To: Andri Yngvason, Ahmed S. Darwish, Olivier Sobrie, Oliver Hartkopp,
Wolfgang Grandegger
Cc: Linux-CAN, netdev, LKML
In-Reply-To: <20150126102843.17894.84372@shannon>
[-- Attachment #1: Type: text/plain, Size: 910 bytes --]
On 01/26/2015 11:28 AM, Andri Yngvason wrote:
> Quoting Ahmed S. Darwish (2015-01-26 05:27:19)
>> From: Ahmed S. Darwish <ahmed.darwish@valeo.com>
>>
>> Update all of the can interface's state and error counters before
>> trying any skb allocation that can actually fail with -ENOMEM.
>>
>> Suggested-by: Marc Kleine-Budde <mkl@pengutronix.de>
>> Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com>
>> ---
>> drivers/net/can/usb/kvaser_usb.c | 181 ++++++++++++++++++++++----------------
>> 1 files changed, 105 insertions(+), 76 deletions(-)
[..]
> Looks good to me.
Can I add your Acked-by to 5-7?
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v6 5/7] can: kvaser_usb: Update interface state before exiting on OOM
From: Andri Yngvason @ 2015-01-26 10:52 UTC (permalink / raw)
To: Marc Kleine-Budde, Ahmed S. Darwish, Olivier Sobrie,
Oliver Hartkopp, Wolfgang Grandegger
Cc: Linux-CAN, netdev, LKML
In-Reply-To: <54C61BE4.8070105@pengutronix.de>
Quoting Marc Kleine-Budde (2015-01-26 10:50:12)
> On 01/26/2015 11:28 AM, Andri Yngvason wrote:
> > Quoting Ahmed S. Darwish (2015-01-26 05:27:19)
> >> From: Ahmed S. Darwish <ahmed.darwish@valeo.com>
> >>
> >> Update all of the can interface's state and error counters before
> >> trying any skb allocation that can actually fail with -ENOMEM.
> >>
> >> Suggested-by: Marc Kleine-Budde <mkl@pengutronix.de>
> >> Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com>
> >> ---
> >> drivers/net/can/usb/kvaser_usb.c | 181 ++++++++++++++++++++++----------------
> >> 1 files changed, 105 insertions(+), 76 deletions(-)
> [..]
>
> > Looks good to me.
>
> Can I add your Acked-by to 5-7?
>
Yes.
--
Andri
^ permalink raw reply
* Re: [PATCH v6 5/7] can: kvaser_usb: Update interface state before exiting on OOM
From: Marc Kleine-Budde @ 2015-01-26 10:53 UTC (permalink / raw)
To: Andri Yngvason, Ahmed S. Darwish, Olivier Sobrie, Oliver Hartkopp,
Wolfgang Grandegger
Cc: Linux-CAN, netdev, LKML
In-Reply-To: <20150126105250.19943.75012@shannon>
[-- Attachment #1: Type: text/plain, Size: 1067 bytes --]
On 01/26/2015 11:52 AM, Andri Yngvason wrote:
> Quoting Marc Kleine-Budde (2015-01-26 10:50:12)
>> On 01/26/2015 11:28 AM, Andri Yngvason wrote:
>>> Quoting Ahmed S. Darwish (2015-01-26 05:27:19)
>>>> From: Ahmed S. Darwish <ahmed.darwish@valeo.com>
>>>>
>>>> Update all of the can interface's state and error counters before
>>>> trying any skb allocation that can actually fail with -ENOMEM.
>>>>
>>>> Suggested-by: Marc Kleine-Budde <mkl@pengutronix.de>
>>>> Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com>
>>>> ---
>>>> drivers/net/can/usb/kvaser_usb.c | 181 ++++++++++++++++++++++----------------
>>>> 1 files changed, 105 insertions(+), 76 deletions(-)
>> [..]
>>
>>> Looks good to me.
>>
>> Can I add your Acked-by to 5-7?
>>
> Yes.
Tnx.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v6 0/7] can: kvaser_usb: Leaf bugfixes and USBCan-II support
From: Marc Kleine-Budde @ 2015-01-26 10:56 UTC (permalink / raw)
To: Ahmed S. Darwish, Olivier Sobrie, Oliver Hartkopp,
Wolfgang Grandegger, Andri Yngvason
Cc: Linux-CAN, netdev, LKML
In-Reply-To: <54C611DA.8050500@pengutronix.de>
[-- Attachment #1: Type: text/plain, Size: 1736 bytes --]
On 01/26/2015 11:07 AM, Marc Kleine-Budde wrote:
> On 01/26/2015 06:17 AM, Ahmed S. Darwish wrote:
>> Hi!
>>
>> This is an updated patch series for the Kvaser CAN/USB devices:
>>
>> 1- Extra patches are now added to the series. Most importantly
>> patch #1 which fixes a critical `sleep in atomic context' bug
>> in the current upstream driver. Patch #2 fixes a corruption in
>> the kernel logs, also affecting current upstream driver. Patch
>> #4 was originally USBCan-II only, but since it's a generic fix,
>> it's now retrofitted to the Leaf-only upstream driver first.
>>
>> 2- The series has been re-organized so that patches #1 -> #4
>> inclusive can go to linux-can/origin, while the rest can move
>> to -next.
>>
>> 3- There are some important updates regarding the USBCan-II
>> error counters, and how really erratic their heaviour is. All
>> the new details are covered at the bottom of this URL:
>>
>> http://article.gmane.org/gmane.linux.can/7481
>>
>> 4- Attached below is the new candump traces. Now
>> `back-to-error-active' states appear _if_ the hardware was
>> kind enough and decreased the error counters appropriately.
>> The earlier code did not recognize the error counters going
>> down, thus the `back-to-error-active' transitions did not
>> appear.
>
> For patches 5-7 I'm waiting for an Acked by Andri.
Applied to can-next, it's available as can-next-testing in the
linux-can-next repo.
Marc.
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH net-next] vlan: take link netns into account in vlan_newlink()
From: Nicolas Dichtel @ 2015-01-26 11:02 UTC (permalink / raw)
To: netdev; +Cc: davem
In-Reply-To: <1422265455-4331-1-git-send-email-nicolas.dichtel@6wind.com>
Le 26/01/2015 10:44, Nicolas Dichtel a écrit :
> When IFLA_LINK_NETNSID is set, the ifindex from IFLA_LINK comes from another
> netns.
>
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Please, discard this patch. It's not the right fix.
I will send an update later.
^ permalink raw reply
* Re: [PATCH] net: Linn Ethernet Packet Sniffer driver
From: Stathis Voukelatos @ 2015-01-26 11:05 UTC (permalink / raw)
To: Stathis Voukelatos
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1817503.588aons8oi@radagast>
On 23/01/15 18:12, James Hogan wrote:
>> diff --git a/drivers/net/pkt-sniffer/Kconfig
>> b/drivers/net/pkt-sniffer/Kconfig
>> new file mode 100644
>> index 0000000..26b4f98
>> --- /dev/null
>> +++ b/drivers/net/pkt-sniffer/Kconfig
>> @@ -0,0 +1,23 @@
>> +menuconfig PKT_SNIFFER
>> + tristate "Linn packet sniffer support"
> Should the kconfig symbol have linn in the name, or should the prompt
> not have lin in the name?
No it should not actually, as this option enables the core (framework)
driver.
Anybody could add a backend using the framework. Will change the text.
>> +
>> +config PKT_SNIFFER_ETHER
>> + tristate "Ethernet packet sniffer"
>> + depends on MIPS
> worth adding || COMPILE_TEST to get compile coverage on x86 allmodconfig
> builds, or does it have hard dependencies on the MIPS arch?
>
No hard dependencies on MIPS arch. Will change as you suggest.
Will implement your other recommendations too in the next version
of the patchset.
Thank you,
Stathis
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: udp_diag: Fix socket skipping within chain
From: Pavel Emelyanov @ 2015-01-26 11:07 UTC (permalink / raw)
To: Herbert Xu, netdev
In-Reply-To: <20150123210240.GA1569@gondor.apana.org.au>
On 01/24/2015 12:02 AM, Herbert Xu wrote:
> While working on rhashtable walking I noticed that the UDP diag
> dumping code is buggy. In particular, the socket skipping within
> a chain never happens, even though we record the number of sockets
> that should be skipped.
>
> As this code was supposedly copied from TCP, this patch does what
> TCP does and resets num before we walk a chain.
>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Acked-by: Pavel Emelyanov <xemul@parallels.com>
Thanks,
Pavel
> diff --git a/net/ipv4/udp_diag.c b/net/ipv4/udp_diag.c
> index 7927db0..4a000f1 100644
> --- a/net/ipv4/udp_diag.c
> +++ b/net/ipv4/udp_diag.c
> @@ -99,11 +99,13 @@ static void udp_dump(struct udp_table *table, struct sk_buff *skb, struct netlin
> s_slot = cb->args[0];
> num = s_num = cb->args[1];
>
> - for (slot = s_slot; slot <= table->mask; num = s_num = 0, slot++) {
> + for (slot = s_slot; slot <= table->mask; s_num = 0, slot++) {
> struct sock *sk;
> struct hlist_nulls_node *node;
> struct udp_hslot *hslot = &table->hash[slot];
>
> + num = 0;
> +
> if (hlist_nulls_empty(&hslot->head))
> continue;
>
>
^ permalink raw reply
* Re: [PATCH] net: Linn Ethernet Packet Sniffer driver
From: Stathis Voukelatos @ 2015-01-26 11:11 UTC (permalink / raw)
To: Joe Perches; +Cc: netdev, linux-kernel, devicetree, abrestic
In-Reply-To: <1422135432.18650.2.camel@perches.com>
On 24/01/15 21:37, Joe Perches wrote:
> On Fri, 2015-01-23 at 10:07 +0000, Stathis Voukelatos wrote:
>> This patch adds support the Ethernet Packet Sniffer H/W module
>> developed by Linn Products Ltd and found in the IMG Pistachio SoC.
>> The module allows Ethernet packets to be parsed, matched against
>> a user-defined pattern and timestamped. It sits between a 100M
>> Ethernet MAC and PHY and is completely passive with respect to
>> Ethernet frames.
> []
>> include/linux/pkt_sniffer.h | 89 +++++
> Why should this file be here?
> Why not in the drivers/net/pkt-sniffer directory?
>
Hi Joe,
Thank you for the feedback.
This header file is the public API for the driver.
Should it not live under the 'include' directory?
Several other drivers seem to follow that convention.
I will include your other suggestions in the next version
of the patch set.
Thank you,
Stathis
^ permalink raw reply
* Re: [PATCH] net: macb: Remove CONFIG_PM ifdef because of compilation warning
From: Nicolas Ferre @ 2015-01-26 11:11 UTC (permalink / raw)
To: Michal Simek, linux-kernel, monstr; +Cc: netdev
In-Reply-To: <7970145ad4d864eaf31d694d22f2fecbcf6825c9.1422002161.git.michal.simek@xilinx.com>
Le 23/01/2015 09:36, Michal Simek a écrit :
> Fix compilation warning:
> drivers/net/ethernet/cadence/macb.c:2415:12: warning: 'macb_suspend'
> defined but not used [-Wunused-function]
> static int macb_suspend(struct device *dev)
> drivers/net/ethernet/cadence/macb.c:2432:12: warning: 'macb_resume'
> defined but not used [-Wunused-function]
> static int macb_resume(struct device *dev)
>
> when CONFIG_PM=y, CONFIG_PM_SLEEP=n are used.
>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
okay:
Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> ---
>
> drivers/net/ethernet/cadence/macb.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
> index 3767271c7667..23ae32f118c2 100644
> --- a/drivers/net/ethernet/cadence/macb.c
> +++ b/drivers/net/ethernet/cadence/macb.c
> @@ -2411,8 +2411,7 @@ static int __exit macb_remove(struct platform_device *pdev)
> return 0;
> }
>
> -#ifdef CONFIG_PM
> -static int macb_suspend(struct device *dev)
> +static int __maybe_unused macb_suspend(struct device *dev)
> {
> struct platform_device *pdev = to_platform_device(dev);
> struct net_device *netdev = platform_get_drvdata(pdev);
> @@ -2429,7 +2428,7 @@ static int macb_suspend(struct device *dev)
> return 0;
> }
>
> -static int macb_resume(struct device *dev)
> +static int __maybe_unused macb_resume(struct device *dev)
> {
> struct platform_device *pdev = to_platform_device(dev);
> struct net_device *netdev = platform_get_drvdata(pdev);
> @@ -2444,7 +2443,6 @@ static int macb_resume(struct device *dev)
>
> return 0;
> }
> -#endif
>
> static SIMPLE_DEV_PM_OPS(macb_pm_ops, macb_suspend, macb_resume);
>
> --
> 1.8.2.3
>
--
Nicolas Ferre
^ permalink raw reply
* [PATCH v2 0/2] Add ACPI support for NXP PN544
From: Robert Dolca @ 2015-01-26 11:13 UTC (permalink / raw)
To: linux-nfc-hn68Rpc1hR1g9hUCZPvPmw, Lauro Ramos Venancio,
Aloisio Almeida Jr, Samuel Ortiz
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA, David S. Miller, Berg Johannes,
Clement Perrochaud, Robert Dolca
This patch set introduces ACPI support for PN544.
gpio_set_value was replaced with gpio_set_value_cansleep in order
to allow GPIO access that may sleep. This is particularelly useful
when GPIO is accessed using busses like I2C, SPI, USB
Changes since v1:
- Added cover letter
- Removed debug define and Kconfig include
- Minor fixes to patch subjects
Links to v1:
- http://lkml.iu.edu/hypermail/linux/kernel/1501.1/00943.html
- http://lkml.iu.edu/hypermail/linux/kernel/1501.1/00944.html
Robert Dolca (2):
NFC: PN544: GPIO access that may sleep
NFC: Add ACPI support for NXP PN544
drivers/nfc/pn544/i2c.c | 137 +++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 124 insertions(+), 13 deletions(-)
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH v2 2/2] NFC: Add ACPI support for NXP PN544
From: Robert Dolca @ 2015-01-26 11:13 UTC (permalink / raw)
To: linux-nfc, Lauro Ramos Venancio, Aloisio Almeida Jr, Samuel Ortiz
Cc: linux-kernel, linux-wireless, netdev, David S. Miller,
Berg Johannes, Clement Perrochaud, Robert Dolca
In-Reply-To: <1422270817-10950-1-git-send-email-robert.dolca@intel.com>
Currently there is no support for ACPI.
This patch uses the following configuration:
- Device id: NXP5440
- Pin mapping:
- 0 IRQ pin
- 1 enable pin
- 2 firmware pin
Signed-off-by: Robert Dolca <robert.dolca@intel.com>
---
drivers/nfc/pn544/i2c.c | 115 ++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 112 insertions(+), 3 deletions(-)
diff --git a/drivers/nfc/pn544/i2c.c b/drivers/nfc/pn544/i2c.c
index d9018cd..ae817b0 100644
--- a/drivers/nfc/pn544/i2c.c
+++ b/drivers/nfc/pn544/i2c.c
@@ -24,6 +24,7 @@
#include <linux/gpio.h>
#include <linux/of_gpio.h>
#include <linux/of_irq.h>
+#include <linux/acpi.h>
#include <linux/miscdevice.h>
#include <linux/interrupt.h>
#include <linux/delay.h>
@@ -41,6 +42,11 @@
#define PN544_I2C_FRAME_HEADROOM 1
#define PN544_I2C_FRAME_TAILROOM 2
+/* GPIO names */
+#define PN544_GPIO_NAME_IRQ "pn544_irq"
+#define PN544_GPIO_NAME_FW "pn544_fw"
+#define PN544_GPIO_NAME_EN "pn544_en"
+
/* framing in HCI mode */
#define PN544_HCI_I2C_LLC_LEN 1
#define PN544_HCI_I2C_LLC_CRC 2
@@ -58,6 +64,13 @@ static struct i2c_device_id pn544_hci_i2c_id_table[] = {
MODULE_DEVICE_TABLE(i2c, pn544_hci_i2c_id_table);
+static const struct acpi_device_id pn544_hci_i2c_acpi_match[] = {
+ {"NXP5440", 0},
+ {}
+};
+
+MODULE_DEVICE_TABLE(acpi, pn544_hci_i2c_acpi_match);
+
#define PN544_HCI_I2C_DRIVER_NAME "pn544_hci_i2c"
/*
@@ -861,6 +874,90 @@ exit_state_wait_secure_write_answer:
}
}
+static int pn544_hci_i2c_acpi_request_resources(struct i2c_client *client)
+{
+ struct pn544_i2c_phy *phy = i2c_get_clientdata(client);
+ const struct acpi_device_id *id;
+ struct gpio_desc *gpiod_en, *gpiod_irq, *gpiod_fw;
+ struct device *dev;
+ int ret;
+
+ if (!client)
+ return -EINVAL;
+
+ dev = &client->dev;
+
+ /* Match the struct device against a given list of ACPI IDs */
+ id = acpi_match_device(dev->driver->acpi_match_table, dev);
+
+ if (!id)
+ return -ENODEV;
+
+ /* Get EN GPIO from ACPI */
+ gpiod_en = devm_gpiod_get_index(dev, PN544_GPIO_NAME_EN, 1);
+ if (IS_ERR(gpiod_en)) {
+ nfc_err(dev,
+ "Unable to get EN GPIO\n");
+ return -ENODEV;
+ }
+
+ phy->gpio_en = desc_to_gpio(gpiod_en);
+
+ /* Configuration EN GPIO */
+ ret = gpiod_direction_output(gpiod_en, 0);
+ if (ret) {
+ nfc_err(dev, "Fail EN pin direction\n");
+ return ret;
+ }
+
+ /* Get FW GPIO from ACPI */
+ gpiod_fw = devm_gpiod_get_index(dev, PN544_GPIO_NAME_FW, 2);
+ if (IS_ERR(gpiod_fw)) {
+ nfc_err(dev,
+ "Unable to get FW GPIO\n");
+ return -ENODEV;
+ }
+
+ phy->gpio_fw = desc_to_gpio(gpiod_fw);
+
+ /* Configuration FW GPIO */
+ ret = gpiod_direction_output(gpiod_fw, 0);
+ if (ret) {
+ nfc_err(dev, "Fail FW pin direction\n");
+ return ret;
+ }
+
+ /* Get IRQ GPIO */
+ gpiod_irq = devm_gpiod_get_index(dev, PN544_GPIO_NAME_IRQ, 0);
+ if (IS_ERR(gpiod_irq)) {
+ nfc_err(dev,
+ "Unable to get IRQ GPIO\n");
+ return -ENODEV;
+ }
+
+ phy->gpio_irq = desc_to_gpio(gpiod_irq);
+
+ /* Configure IRQ GPIO */
+ ret = gpiod_direction_input(gpiod_irq);
+ if (ret) {
+ nfc_err(dev, "Fail IRQ pin direction\n");
+ return ret;
+ }
+
+ /* Map the pin to an IRQ */
+ ret = gpiod_to_irq(gpiod_irq);
+ if (ret < 0) {
+ nfc_err(dev, "Fail pin IRQ mapping\n");
+ return ret;
+ }
+
+ nfc_info(dev, "GPIO resource, no:%d irq:%d\n",
+ desc_to_gpio(gpiod_irq), ret);
+ client->irq = ret;
+
+ return 0;
+}
+
#ifdef CONFIG_OF
static int pn544_hci_i2c_of_request_resources(struct i2c_client *client)
@@ -886,7 +983,7 @@ static int pn544_hci_i2c_of_request_resources(struct i2c_client *client)
phy->gpio_en = ret;
/* Configuration of EN GPIO */
- ret = gpio_request(phy->gpio_en, "pn544_en");
+ ret = gpio_request(phy->gpio_en, PN544_GPIO_NAME_EN);
if (ret) {
nfc_err(&client->dev, "Fail EN pin\n");
goto err_dt;
@@ -908,7 +1005,7 @@ static int pn544_hci_i2c_of_request_resources(struct i2c_client *client)
phy->gpio_fw = ret;
/* Configuration of FW GPIO */
- ret = gpio_request(phy->gpio_fw, "pn544_fw");
+ ret = gpio_request(phy->gpio_fw, PN544_GPIO_NAME_FW);
if (ret) {
nfc_err(&client->dev, "Fail FW pin\n");
goto err_gpio_en;
@@ -1003,6 +1100,14 @@ static int pn544_hci_i2c_probe(struct i2c_client *client,
phy->gpio_en = pdata->get_gpio(NFC_GPIO_ENABLE);
phy->gpio_fw = pdata->get_gpio(NFC_GPIO_FW_RESET);
phy->gpio_irq = pdata->get_gpio(NFC_GPIO_IRQ);
+ /* Using ACPI */
+ } else if (ACPI_HANDLE(&client->dev)) {
+ r = pn544_hci_i2c_acpi_request_resources(client);
+ if (r) {
+ nfc_err(&client->dev,
+ "Cannot get ACPI data\n");
+ return r;
+ }
} else {
nfc_err(&client->dev, "No platform data\n");
return -EINVAL;
@@ -1022,9 +1127,12 @@ static int pn544_hci_i2c_probe(struct i2c_client *client,
PN544_I2C_FRAME_HEADROOM, PN544_I2C_FRAME_TAILROOM,
PN544_HCI_I2C_LLC_MAX_PAYLOAD,
pn544_hci_i2c_fw_download, &phy->hdev);
- if (r < 0)
+ if (r < 0) {
+ nfc_err(&client->dev, "HCI Probing error\n");
goto err_hci;
+ }
+ nfc_info(&client->dev, "NFC I2C driver loaded\n");
return 0;
err_hci:
@@ -1082,6 +1190,7 @@ static struct i2c_driver pn544_hci_i2c_driver = {
.name = PN544_HCI_I2C_DRIVER_NAME,
.owner = THIS_MODULE,
.of_match_table = of_match_ptr(of_pn544_i2c_match),
+ .acpi_match_table = ACPI_PTR(pn544_hci_i2c_acpi_match),
},
.probe = pn544_hci_i2c_probe,
.id_table = pn544_hci_i2c_id_table,
--
1.9.1
^ permalink raw reply related
* [PATCH v2 1/2] NFC: PN544: GPIO access that may sleep
From: Robert Dolca @ 2015-01-26 11:13 UTC (permalink / raw)
To: linux-nfc, Lauro Ramos Venancio, Aloisio Almeida Jr, Samuel Ortiz
Cc: linux-kernel, linux-wireless, netdev, David S. Miller,
Berg Johannes, Clement Perrochaud, Robert Dolca
In-Reply-To: <1422270817-10950-1-git-send-email-robert.dolca@intel.com>
gpio_set_value was replaced with gpio_set_value_cansleep in order
to allow GPIO access that may sleep. This is particularelly useful
when GPIO is accessed using busses like I2C, SPI, USB
Signed-off-by: Robert Dolca <robert.dolca@intel.com>
---
drivers/nfc/pn544/i2c.c | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/drivers/nfc/pn544/i2c.c b/drivers/nfc/pn544/i2c.c
index fc02e8d..d9018cd 100644
--- a/drivers/nfc/pn544/i2c.c
+++ b/drivers/nfc/pn544/i2c.c
@@ -195,18 +195,19 @@ static void pn544_hci_i2c_platform_init(struct pn544_i2c_phy *phy)
nfc_info(&phy->i2c_dev->dev, "Detecting nfc_en polarity\n");
/* Disable fw download */
- gpio_set_value(phy->gpio_fw, 0);
+ gpio_set_value_cansleep(phy->gpio_fw, 0);
for (polarity = 0; polarity < 2; polarity++) {
phy->en_polarity = polarity;
retry = 3;
while (retry--) {
/* power off */
- gpio_set_value(phy->gpio_en, !phy->en_polarity);
+ gpio_set_value_cansleep(phy->gpio_en,
+ !phy->en_polarity);
usleep_range(10000, 15000);
/* power on */
- gpio_set_value(phy->gpio_en, phy->en_polarity);
+ gpio_set_value_cansleep(phy->gpio_en, phy->en_polarity);
usleep_range(10000, 15000);
/* send reset */
@@ -225,13 +226,14 @@ static void pn544_hci_i2c_platform_init(struct pn544_i2c_phy *phy)
"Could not detect nfc_en polarity, fallback to active high\n");
out:
- gpio_set_value(phy->gpio_en, !phy->en_polarity);
+ gpio_set_value_cansleep(phy->gpio_en, !phy->en_polarity);
}
static void pn544_hci_i2c_enable_mode(struct pn544_i2c_phy *phy, int run_mode)
{
- gpio_set_value(phy->gpio_fw, run_mode == PN544_FW_MODE ? 1 : 0);
- gpio_set_value(phy->gpio_en, phy->en_polarity);
+ gpio_set_value_cansleep(phy->gpio_fw,
+ run_mode == PN544_FW_MODE ? 1 : 0);
+ gpio_set_value_cansleep(phy->gpio_en, phy->en_polarity);
usleep_range(10000, 15000);
phy->run_mode = run_mode;
@@ -254,14 +256,14 @@ static void pn544_hci_i2c_disable(void *phy_id)
{
struct pn544_i2c_phy *phy = phy_id;
- gpio_set_value(phy->gpio_fw, 0);
- gpio_set_value(phy->gpio_en, !phy->en_polarity);
+ gpio_set_value_cansleep(phy->gpio_fw, 0);
+ gpio_set_value_cansleep(phy->gpio_en, !phy->en_polarity);
usleep_range(10000, 15000);
- gpio_set_value(phy->gpio_en, phy->en_polarity);
+ gpio_set_value_cansleep(phy->gpio_en, phy->en_polarity);
usleep_range(10000, 15000);
- gpio_set_value(phy->gpio_en, !phy->en_polarity);
+ gpio_set_value_cansleep(phy->gpio_en, !phy->en_polarity);
usleep_range(10000, 15000);
phy->powered = 0;
--
1.9.1
^ permalink raw reply related
* Re: [PATCH] net: Linn Ethernet Packet Sniffer driver
From: Stathis Voukelatos @ 2015-01-26 11:23 UTC (permalink / raw)
To: Arnd Bergmann
Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
abrestic-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org
In-Reply-To: <3515756.ch1oRd7PaS@wuerfel>
On 23/01/15 10:21, Arnd Bergmann wrote:
> On Friday 23 January 2015 10:07:01 Stathis Voukelatos wrote:
>> +- interrupts : sniffer interrupt specifier
>> +- clocks : specify the system clock for the peripheral
>> +- clock-names : must contain the "sys" entry
>> +- fifo-block-words : number of words in one data FIFO entry
>> +
>> +Example:
>> +
>> +sniffer@1814a000 {
>> + compatible = "linn,eth-sniffer";
>> + reg = <0x1814a000 0x100>, <0x1814a400 0x400>, <0x1814a800 0x400>;
>> + reg-names = "regs", "tx-ram", "rx-ram";
>> + interrupts = <GIC_SHARED 58 IRQ_TYPE_LEVEL_HIGH>;
>> + interrupt-names = "eth-sniffer-irq";
>> + clocks = <&system_clk>;
>> + clock-names = "sys";
>> + fifo-block-words = <4>;
>>
> The example contains an interrupt-names property that is not documented.
> If you want to name interrupts, the exact name strings need to
> be mandated by the binding. Alternatively just drop the name.
> I notice that the driver requests the first interrupt without giving
> a name anyway, and the description above suggests that there can
> only be one interrupt.
>
> Arnd
>
Hi Arnd,
Yes, we can probably just drop the interrupt name field.
It will be done in the next version of the patch set.
Thank you,
Stathis
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: Question on SCTP ABORT chunk is generated when the association_max_retrans is reached
From: Marcelo Ricardo Leitner @ 2015-01-26 11:46 UTC (permalink / raw)
To: Sun Paul, Daniel Borkmann
Cc: Michael Tuexen, Vlad Yasevich, linux-sctp, netdev, linux-kernel
In-Reply-To: <CAFXGft+XZCmhuwj2QTCiU0MD10Uo+WZ3WCC+rfbsd_Lxbhk3iw@mail.gmail.com>
Hi,
On 25-01-2015 23:27, Sun Paul wrote:
> Hi
>
> sorry for the late reply. I am a bit confused. when side-A sends a
> request to side-B, and side-B return the response, but side-A keep
> re-transmit the same request to side-B, why side-B needed to send a
> ABORT to side-A?
That happens on data transfers. When A pushes data to B, A has to retry it
until B finally acknowledges it and A receive this signal. If the ack from B
gets dropped, A has no way to know if a) the ack was lost or b) its initial
message never actually made it to A, thus it retransmits. If it reaches a
limit, it gives up..
> If it is used in order to reestablish the connection, shoudn't it
> should be side-A to send ABORT instead?
Meant to reestablish it? Not really.. just to keep both sides in sync, as A
has given up by then.
Marcelo
> - PS
>
> On Sat, Jan 24, 2015 at 3:05 AM, Daniel Borkmann <dborkman@redhat.com> wrote:
>> On 01/23/2015 07:36 PM, Michael Tuexen wrote:
>> ...
>>>
>>> Yepp. It might not reach the peer or it might. If it does it helps
>>> to keep the states in sync. If it doesn't it sometimes helps in
>>> analysing tracefiles. In BSD, we also send it. It is not required,
>>> doesn't harm and is useful in some cases...
>>
>>
>> Ok, as the TCB is destroyed in any case, should be fine then.
>>
>> Thanks,
>> Daniel
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH net-next v2 4/5] bonding: fix LACP PDU not sent on slave port sometimes
From: Sergei Shtylyov @ 2015-01-26 12:04 UTC (permalink / raw)
To: Jonathan Toppins, Jay Vosburgh, Veaceslav Falico, Andy Gospodarek
Cc: netdev, Satish Ashok, Andy Gospodarek
In-Reply-To: <1422253021-3798-5-git-send-email-jtoppins@cumulusnetworks.com>
Hello.
On 1/26/2015 9:17 AM, Jonathan Toppins wrote:
> From: Satish Ashok <sashok@cumulusnetworks.com>
> When a slave is added to a bond and it is not in full duplex mode,
> AD_PORT_LACP_ENABLED flag is cleared, due to this LACP PDU is not sent
s/is not/not being/.
> on slave. When the duplex is changed to full, the flag needs to be set
> to send LACP PDU.
> Cc: Andy Gospodarek <gospo@cumulusnetworks.com>
> Reviewed-by: Nikolay Aleksandrov <nikolay@redhat.com>
> Signed-off-by: Satish Ashok <sashok@cumulusnetworks.com>
> Signed-off-by: Jonathan Toppins <jtoppins@cumulusnetworks.com>
> ---
> drivers/net/bonding/bond_3ad.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
> index e3c96b2..cfc4a9c 100644
> --- a/drivers/net/bonding/bond_3ad.c
> +++ b/drivers/net/bonding/bond_3ad.c
> @@ -2219,8 +2219,10 @@ static int bond_3ad_rx_indication(struct lacpdu *lacpdu, struct slave *slave,
> switch (lacpdu->subtype) {
> case AD_TYPE_LACPDU:
> ret = RX_HANDLER_CONSUMED;
> - netdev_dbg(slave->bond->dev, "Received LACPDU on port %d\n",
> - port->actor_port_number);
> + netdev_dbg(slave->bond->dev,
> + "Received LACPDU on port %d slave %s\n",
> + port->actor_port_number,
> + slave->dev->name);
> /* Protect against concurrent state machines */
> spin_lock(&slave->bond->mode_lock);
> ad_rx_machine(lacpdu, port);
> @@ -2312,7 +2314,10 @@ void bond_3ad_adapter_duplex_changed(struct slave *slave)
> port->actor_admin_port_key &= ~AD_DUPLEX_KEY_MASKS;
> port->actor_oper_port_key = port->actor_admin_port_key |=
> __get_duplex(port);
> - netdev_dbg(slave->bond->dev, "Port %d changed duplex\n", port->actor_port_number);
> + netdev_dbg(slave->bond->dev, "Port %d slave %s changed duplex\n",
> + port->actor_port_number, slave->dev->name);
The above 2 changes seem unrelated/undocumented in the change log...
[...]
WBR, Sergei
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox