Linux CAN drivers development
 help / color / mirror / Atom feed
* [PATCH 2/4] can: esd_usb2: cancel urb on -EPIPE and -EPROTO
From: Martin Kelly @ 2017-12-05 19:15 UTC (permalink / raw)
  To: linux-can
  Cc: Marc Kleine-Budde, Wolfgang Grandegger, Stefan Mätje,
	Jimmy Assarsson, Martin Kelly
In-Reply-To: <20171205191550.703-1-mkelly@xevo.com>

In mcba_usb, we have observed that when you unplug the device, the driver will
endlessly resubmit failing URBs, which can cause CPU stalls. This issue
is fixed in mcba_usb by catching the codes seen on device disconnect
(-EPIPE and -EPROTO).

This driver also resubmits in the case of -EPIPE and -EPROTO, so fix it
in the same way.

Signed-off-by: Martin Kelly <mkelly@xevo.com>
---
 drivers/net/can/usb/esd_usb2.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/can/usb/esd_usb2.c b/drivers/net/can/usb/esd_usb2.c
index 9fdb0f0bfa06..c6dcf93675c0 100644
--- a/drivers/net/can/usb/esd_usb2.c
+++ b/drivers/net/can/usb/esd_usb2.c
@@ -393,6 +393,8 @@ static void esd_usb2_read_bulk_callback(struct urb *urb)
 		break;
 
 	case -ENOENT:
+	case -EPIPE:
+	case -EPROTO:
 	case -ESHUTDOWN:
 		return;
 
-- 
2.11.0


^ permalink raw reply related

* [PATCH 1/4] can: ems_usb: cancel urb on -EPIPE and -EPROTO
From: Martin Kelly @ 2017-12-05 19:15 UTC (permalink / raw)
  To: linux-can
  Cc: Marc Kleine-Budde, Wolfgang Grandegger, Stefan Mätje,
	Jimmy Assarsson, Martin Kelly
In-Reply-To: <20171205191550.703-1-mkelly@xevo.com>

In mcba_usb, we have observed that when you unplug the device, the driver will
endlessly resubmit failing URBs, which can cause CPU stalls. This issue
is fixed in mcba_usb by catching the codes seen on device disconnect
(-EPIPE and -EPROTO).

This driver also resubmits in the case of -EPIPE and -EPROTO, so fix it
in the same way.

Signed-off-by: Martin Kelly <mkelly@xevo.com>
---
 drivers/net/can/usb/ems_usb.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/can/usb/ems_usb.c b/drivers/net/can/usb/ems_usb.c
index b3d02759c226..b00358297424 100644
--- a/drivers/net/can/usb/ems_usb.c
+++ b/drivers/net/can/usb/ems_usb.c
@@ -288,6 +288,8 @@ static void ems_usb_read_interrupt_callback(struct urb *urb)
 
 	case -ECONNRESET: /* unlink */
 	case -ENOENT:
+	case -EPIPE:
+	case -EPROTO:
 	case -ESHUTDOWN:
 		return;
 
-- 
2.11.0


^ permalink raw reply related

* [PATCH 0/4] URB disconnect bug fixes
From: Martin Kelly @ 2017-12-05 19:15 UTC (permalink / raw)
  To: linux-can
  Cc: Marc Kleine-Budde, Wolfgang Grandegger, Stefan Mätje,
	Jimmy Assarsson, Martin Kelly

This patch series is to fix a common bug I found in mcba_usb that appears to
also exist in these drivers. Because I don't own the hardware, I cannot test the
changes, but I wanted to send the change so that others could do so.

The bug is that, when you disconnect the device, you get -EPIPE and -EPROTO
error codes (exactly which depends on the timing and the system) in the URB read
callback. The driver then resubmits the URB, which immediately fails because the
device is unplugged. On a slower system like a Raspberry Pi, this can cause a
kernel stall, while on a desktop, the disconnect code runs fast enough to
prevent a stall and it's easy not to notice the issue.

Fix this by not resubmitting an URB that receives an -EPIPE/-EPROTO code.

Martin Kelly (4):
  can: ems_usb: cancel urb on -EPIPE and -EPROTO
  can: esd_usb2: cancel urb on -EPIPE and -EPROTO
  can: kvaser_usb: cancel urb on -EPIPE and -EPROTO
  can: usb_8dev: cancel urb on -EPIPE and -EPROTO

 drivers/net/can/usb/ems_usb.c    | 2 ++
 drivers/net/can/usb/esd_usb2.c   | 2 ++
 drivers/net/can/usb/kvaser_usb.c | 2 ++
 drivers/net/can/usb/usb_8dev.c   | 2 ++
 4 files changed, 8 insertions(+)

--
2.11.0


^ permalink raw reply

* [PATCH] can: mcba_usb: cancel urb on -EPROTO
From: Martin Kelly @ 2017-12-05 18:34 UTC (permalink / raw)
  To: linux-can
  Cc: Remigiusz Kołłątaj, Marc Kleine-Budde,
	Wolfgang Grandegger, Martin Kelly

When we unplug the device, we can see both -EPIPE and -EPROTO depending
on exact timing and what system we run on. If we continue to resubmit
URBs, they will immediately fail, and they can cause stalls, especially
on slower CPUs.

Fix this by not resubmitting on -EPROTO, as we already do on -EPIPE.

Signed-off-by: Martin Kelly <mkelly@xevo.com>
---
 drivers/net/can/usb/mcba_usb.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/can/usb/mcba_usb.c b/drivers/net/can/usb/mcba_usb.c
index ef417dcddbf7..8d8c2086424d 100644
--- a/drivers/net/can/usb/mcba_usb.c
+++ b/drivers/net/can/usb/mcba_usb.c
@@ -593,6 +593,7 @@ static void mcba_usb_read_bulk_callback(struct urb *urb)
 
 	case -ENOENT:
 	case -EPIPE:
+	case -EPROTO:
 	case -ESHUTDOWN:
 		return;
 
-- 
2.11.0


^ permalink raw reply related

* Re: [PATCH 1/2] dt-binding: can: mcp2517fd: document device tree bindings
From: Martin Sperl @ 2017-12-05 10:26 UTC (permalink / raw)
  To: Patrick Menschel; +Cc: linux-can-u79uwXL29TY76Z2rM5mHXA, devicetree
In-Reply-To: <1ac487f7-17e3-c8a0-0f99-8138fe867373-1KBjaw7Xf1+zQB+pC5nmwQ@public.gmane.org>

Hi Patrick!


I had a quick look starting to implement gpiolib,

but I believe I would need to implement pin-ctrl on top to

make the Alternate functions work propperly.

I wonder if this effort is really worth it.

Martin

On 11/30/2017 06:49 PM, Patrick Menschel wrote:
> Am 30.11.2017 um 17:58 schriebkernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org:
>>> drivers/tty/serial/max310x.c
>>> Documentation/devicetree/bindings/serial/maxim,max310x.txt
>>> Look for "#ifdef CONFIG_GPIOLIB”.
>> This is a gpio-controller, for which this is what I would implement.
>>
> Hi,
>
> you are mistaken about that, max310x chips are spi to uart bridges with
> strong ties to rs485 which is very similar to CAN except for the missing
> arbitration in hardware. Max14830 has GPIOs with clocking support, so
> this chip's driver also fused the main function with secondary gpio
> functions.
> https://datasheets.maximintegrated.com/en/ds/MAX14830.pdf
>
> Anyway I'm curious what is best practice for this sort of configuration.
>
> Regards,
> Patrick
>

--
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: pull-request: can-next 2017-12-01,Re: pull-request: can-next 2017-12-01
From: David Miller @ 2017-12-04 18:20 UTC (permalink / raw)
  To: mkl; +Cc: netdev, kernel, linux-can
In-Reply-To: <85177a44-2bf7-80f8-08c4-df133cd48917@pengutronix.de>

From: Marc Kleine-Budde <mkl@pengutronix.de>
Date: Fri, 1 Dec 2017 14:50:04 +0100

> On 12/01/2017 02:37 PM, Marc Kleine-Budde wrote:
>> Doh, I had an error in my script which produced the wrong Linux version
>> in the tag. The correct name of the tag is:
> 
> First test - than post - here's the correct tag:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git
> tags/linux-can-next-for-4.16-20171201

Pulled, thanks!

^ permalink raw reply

* Re: CAN-USB adapter unplug
From: Jimmy Assarsson @ 2017-12-04  8:39 UTC (permalink / raw)
  To: Martin Kelly, Marc Kleine-Budde, linux-can@vger.kernel.org
  Cc: Stefan Mätje, Remigiusz Kołłątaj,
	Wolfgang Grandegger
In-Reply-To: <4f6687db-269d-d8ff-7a47-5094ad63043a@xevo.com>

On 2017-11-30 19:19, Martin Kelly wrote:
> On 11/30/2017 02:18 AM, Jimmy Assarsson wrote:
>> On 2017-11-29 18:07, Martin Kelly wrote:
>>> On 11/29/2017 07:21 AM, Stefan Mätje wrote:
>>>> Am 29.11.2017 um 13:20 schrieb Marc Kleine-Budde:
>>>>> On 11/28/2017 10:09 PM, Martin Kelly wrote:
>>>>>>> Both applied to can.
>>>>>>
>>>>>> Thanks! By the way as far as I can tell from code inspection, it 
>>>>>> appears
>>>>>> that most of the other drivers in net/can/usb should have the same
>>>>>> disconnect bug. gs_usb appears to be clear, as it returns in its 
>>>>>> default
>>>>>> case. Unfortunately mcba_usb is the only device I have to test 
>>>>>> with, but
>>>>>> those with other devices may want to check for this.
>>>>>
>>>>> Can you create patches for the affected drivers and send them to the
>>>>> list and the maintainers of the driver on Cc?
>>>>>
>>>>> I don't have access to every USB adapter neither.
>>>>>
>>>>> Marc
>>>>>
>>>>
>>>> Hi,
>>>>
>>>> I have seen Martin's emails these days and tried to reproduce the 
>>>> error here
>>>> with an esd CAN-USB/2 device (handled by esd_usb2.c). The only thing 
>>>> I get
>>>> in the log are some messages like this:
>>>>
>>>> kernel: [14776.152459] usb 2-1: Rx URB aborted (-71)
>>>>
>>>> There is no endless loop. How could I reproduce the bad behavior? 
>>>> For the
>>>> quick test I used an Ubuntu 4.4.0-101-generic kernel.
>>
>> Hi,
>>
>> I got same result for kvaser_usb, a single "Rx URB aborted (-71)". 
>> When tested on Ubuntu with 4.4.0-93-generic.
>>
>>> Interesting. Do you see just a few -71 RX URB aborted messages (one 
>>> per outstanding URB) rather than an endless loop? If so, then I think 
>>> everything is OK on that device, as the URBs are not being resubmitted.
>>>
>>> In case it helps, my test case for the mcba_usb is on a Raspberry Pi 
>>> 3. I don't know whether or not that could influence the USB error 
>>> code we see, since you are seeing EPROTO instead of EPIPE when the 
>>> device gets unplugged.
>>
>> However, I do get lots of (500+) "Rx URB aborted (-71)" printouts, on 
>> Raspberry Pi 3, running Raspbian with kernel 4.9.35-v7. But no EPIPE 
>> seen.
>>
> 
> Very interesting; this means that there are multiple possible error 
> codes from a USB disconnect. If that's the case, is it possible that the 
> best thing to do is what gs_usb does? In gs_usb's receive callback, we 
> have:
> 
>      switch (urb->status) {
>      case 0: /* success */
>          break;
>      case -ENOENT:
>      case -ESHUTDOWN:
>          return;
>      default:
>          /* do not resubmit aborted urbs. eg: when device goes down */
>          return;
>      }
> 
> The default case is to *not* resubmit URBs, rather than to resubmit is a 
> default and try to catch all possible error codes in the non-default 
> case, as the other drivers are doing.
> 
> If we don't do something like this, then we need to understand all the 
> possible error codes that could occur and catch them all.

Well, gs_usb never resubmit any URBs if urb->status is non-zero. As long 
as any error (urb->status != 0) is the result of a disconnect, it should 
be safe to never resubmit? I doubt this is the case, but I really don't 
know.

>> Regards,
>> Jimmy
>>
>>>>
>>>> In any case I will add the patch handling EPIPE on a test system and 
>>>> have a
>>>> look what it might change.
>>>>
>>>> Regards,
>>>> Stefan Mätje

^ permalink raw reply

* Re: [PATCH 2/2] can: mcp2517fd: Add Microchip mcp2517fd CAN FD driver
From: kernel-TqfNSX0MhmxHKSADF0wUEw @ 2017-12-03 18:34 UTC (permalink / raw)
  To: Marc Kleine-Budde
  Cc: Wolfgang Grandegger, Rob Herring, Mark Rutland,
	linux-can-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <ff920dd7-4535-dcaa-27f9-57844ce66c7b-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

H Marc!


> On 30.11.2017, at 14:04, Marc Kleine-Budde <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> wrote:
> 
> On 11/24/2017 07:35 PM, kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org wrote:
>> From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
>> 
>> This patch adds support for the Microchip mcp2517fd CAN-FD controller.
>> The mcp2517fd is capable of transmitting and receiving standard data
>> frames, extended data frames, remote frames and Can-FD frames.
>> The mcp2517fd interfaces with the host over SPI.
> 
> I've review about the last 1/3 of the driver. See comments inline.

Thanks for all the feedback!

I shall incorporate those into V2.

>> +	switch (priv->config.gpio0_mode) {
>> +	case gpio_mode_standby:
>> +	case gpio_mode_int: /* asserted low on TXIF */
>> +	case gpio_mode_out_low:
>> +	case gpio_mode_out_high:
>> +	case gpio_mode_in:
> 
> Please add comments for fallthrough for all your switch-case.


I thought /* fall through */ is only needed for the cases
where there is code involved in each case block and you want
to fall through to the next block - checkpatch only  complains 
in such conditions (there is one location in the driver -
in mcp2517fd_can_ist_handle_status - that requires /* fall through */).

Also Documentation/process/coding-style.rst shows such an example
in the “indentation” section, so I would assume this is valid code.

>> 
>> +int mcp2517fd_of_parse(struct mcp2517fd_priv *priv)
>> +{
>> +#ifdef CONFIG_OF_DYNAMIC
> 
> Why does this code depend on OF_DYNAMIC?
> 

Well - this is only in case of Device Tree - no need to include
all the DT-parsing in case that there is no DT support in the
first place…

I may arrange it as a #ifdef outside of the function 
(like the other case you have mentioned).

>> 
>> +	if (!IS_ERR(clk)) {
>> +		ret = clk_prepare_enable(clk);
>> +		if (ret)
>> +			goto out_free;
>> +	}
> 
> Why do you keep the clock running after the device has been probed?
> Usually we enable the clock during open().

I took the mcp251x as a blue-print and I believe it follows the
same pattern… 
But I may have simplified things during early driver development, 
so I will recheck it.

> 
>> +
>> +	/* check in device tree for overrrides */
>> +	ret = mcp2517fd_of_parse(priv);
>> +	if (ret)
>> +		return ret;
> 
> You're keeping the clock running.
See above - I shall look into it...

>> +			dev_err(&spi->dev,
>> +				"PLL clock frequency %i would exceed limit\n",
>> +				priv->can.clock.freq
>> +				);
>> +			return -EINVAL;
> 
> same here

Thanks, Martin--
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: pull-request: can 2017-12-01
From: David Miller @ 2017-12-03 14:58 UTC (permalink / raw)
  To: mkl; +Cc: netdev, linux-can, kernel
In-Reply-To: <20171201141739.5816-1-mkl@pengutronix.de>

From: Marc Kleine-Budde <mkl@pengutronix.de>
Date: Fri,  1 Dec 2017 15:17:30 +0100

> this is a pull for net consisting of nine patches.
> 
> The first three patches are by Jimmy Assarsson for the kvaser_usb driver
> and add the missing free()s in some error path, a signed/unsigned
> comparison and ratelimit the error messages in case of incomplete
> messages. Oliver Stäbler's patch for the ti_hecc driver fix the napi
> poll function's return value. The return values of the probe function of
> the peak_canfd and peak_pci PCI drivers are fixed by Stephane Grosjean's
> patch. Two patches by me for the flexcan driver update the
> bugs/features/quirks overview table and fix the error state transition
> for the VF610 SoC. The two patches by Martin Kelly for the mcba_usb
> driver fix a typo and a device disconnect bug.

Pulled, thanks Marc.

^ permalink raw reply

* [PATCH] vxcan: improve handling of missing peer name attribute
From: Oliver Hartkopp @ 2017-12-02 17:48 UTC (permalink / raw)
  To: linux-can; +Cc: Oliver Hartkopp, Serhey Popovych

Picking up the patch from Serhey Popovych (commit 191cdb3822e5df6b3c8,
"veth: Be more robust on network device creation when no attributes").

When the peer name attribute is not provided the former implementation tries
to register the given device name twice ... which leads to -EEXIST.
If only one device name is given apply an automatic generated and valid name
for the peer.

CC: Serhey Popovych <serhe.popovych@gmail.com>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
 drivers/net/can/vxcan.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/can/vxcan.c b/drivers/net/can/vxcan.c
index 8404e8852a0f..b4c4a2c76437 100644
--- a/drivers/net/can/vxcan.c
+++ b/drivers/net/can/vxcan.c
@@ -194,7 +194,7 @@ static int vxcan_newlink(struct net *net, struct net_device *dev,
 		tbp = peer_tb;
 	}
 
-	if (tbp[IFLA_IFNAME]) {
+	if (ifmp && tbp[IFLA_IFNAME]) {
 		nla_strlcpy(ifname, tbp[IFLA_IFNAME], IFNAMSIZ);
 		name_assign_type = NET_NAME_USER;
 	} else {
-- 
2.15.0


^ permalink raw reply related

* (unknown), 
From: Rein Appeldoorn @ 2017-12-01 14:22 UTC (permalink / raw)
  To: linux-can

unsubscribe linux-can

^ permalink raw reply

* [PATCH 3/9] can: kvaser_usb: ratelimit errors if incomplete messages are received
From: Marc Kleine-Budde @ 2017-12-01 14:17 UTC (permalink / raw)
  To: netdev
  Cc: davem, linux-can, kernel, Jimmy Assarsson, linux-stable,
	Marc Kleine-Budde
In-Reply-To: <20171201141739.5816-1-mkl@pengutronix.de>

From: Jimmy Assarsson <jimmyassarsson@gmail.com>

Avoid flooding the kernel log with "Formate error", if incomplete message
are received.

Signed-off-by: Jimmy Assarsson <jimmyassarsson@gmail.com>
Cc: linux-stable <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/kvaser_usb.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
index d87e330a20b3..f95945915d20 100644
--- a/drivers/net/can/usb/kvaser_usb.c
+++ b/drivers/net/can/usb/kvaser_usb.c
@@ -609,8 +609,8 @@ static int kvaser_usb_wait_msg(const struct kvaser_usb *dev, u8 id,
 			}
 
 			if (pos + tmp->len > actual_len) {
-				dev_err(dev->udev->dev.parent,
-					"Format error\n");
+				dev_err_ratelimited(dev->udev->dev.parent,
+						    "Format error\n");
 				break;
 			}
 
@@ -1353,7 +1353,8 @@ static void kvaser_usb_read_bulk_callback(struct urb *urb)
 		}
 
 		if (pos + msg->len > urb->actual_length) {
-			dev_err(dev->udev->dev.parent, "Format error\n");
+			dev_err_ratelimited(dev->udev->dev.parent,
+					    "Format error\n");
 			break;
 		}
 
-- 
2.15.0


^ permalink raw reply related

* [PATCH 1/9] can: kvaser_usb: free buf in error paths
From: Marc Kleine-Budde @ 2017-12-01 14:17 UTC (permalink / raw)
  To: netdev
  Cc: davem, linux-can, kernel, Jimmy Assarsson, linux-stable,
	Marc Kleine-Budde
In-Reply-To: <20171201141739.5816-1-mkl@pengutronix.de>

From: Jimmy Assarsson <jimmyassarsson@gmail.com>

The allocated buffer was not freed if usb_submit_urb() failed.

Signed-off-by: Jimmy Assarsson <jimmyassarsson@gmail.com>
Cc: linux-stable <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/kvaser_usb.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
index 9b18d96ef526..075644591498 100644
--- a/drivers/net/can/usb/kvaser_usb.c
+++ b/drivers/net/can/usb/kvaser_usb.c
@@ -813,6 +813,7 @@ static int kvaser_usb_simple_msg_async(struct kvaser_usb_net_priv *priv,
 	if (err) {
 		netdev_err(netdev, "Error transmitting URB\n");
 		usb_unanchor_urb(urb);
+		kfree(buf);
 		usb_free_urb(urb);
 		return err;
 	}
@@ -1768,6 +1769,7 @@ static netdev_tx_t kvaser_usb_start_xmit(struct sk_buff *skb,
 		spin_unlock_irqrestore(&priv->tx_contexts_lock, flags);
 
 		usb_unanchor_urb(urb);
+		kfree(buf);
 
 		stats->tx_dropped++;
 
-- 
2.15.0


^ permalink raw reply related

* [PATCH 9/9] can: mcba_usb: fix device disconnect bug
From: Marc Kleine-Budde @ 2017-12-01 14:17 UTC (permalink / raw)
  To: netdev
  Cc: davem, linux-can, kernel, Martin Kelly, linux-stable,
	Marc Kleine-Budde
In-Reply-To: <20171201141739.5816-1-mkl@pengutronix.de>

From: Martin Kelly <mkelly@xevo.com>

Currently, when you disconnect the device, the driver infinitely
resubmits all URBs, so you see:

Rx URB aborted (-32)

in an infinite loop.

Fix this by catching -EPIPE (what we get in urb->status when the device
disconnects) and not resubmitting.

With this patch, I can plug and unplug many times and the driver
recovers correctly.

Signed-off-by: Martin Kelly <mkelly@xevo.com>
Cc: linux-stable <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/mcba_usb.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/can/usb/mcba_usb.c b/drivers/net/can/usb/mcba_usb.c
index c4355f0a20d5..ef417dcddbf7 100644
--- a/drivers/net/can/usb/mcba_usb.c
+++ b/drivers/net/can/usb/mcba_usb.c
@@ -592,6 +592,7 @@ static void mcba_usb_read_bulk_callback(struct urb *urb)
 		break;
 
 	case -ENOENT:
+	case -EPIPE:
 	case -ESHUTDOWN:
 		return;
 
-- 
2.15.0

^ permalink raw reply related

* [PATCH 8/9] can: mcba_usb: fix typo
From: Marc Kleine-Budde @ 2017-12-01 14:17 UTC (permalink / raw)
  To: netdev; +Cc: davem, linux-can, kernel, Martin Kelly, Marc Kleine-Budde
In-Reply-To: <20171201141739.5816-1-mkl@pengutronix.de>

From: Martin Kelly <mkelly@xevo.com>

Fix typo "analizer" --> "Analyzer".

Signed-off-by: Martin Kelly <mkelly@xevo.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/mcba_usb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/can/usb/mcba_usb.c b/drivers/net/can/usb/mcba_usb.c
index 7f0272558bef..c4355f0a20d5 100644
--- a/drivers/net/can/usb/mcba_usb.c
+++ b/drivers/net/can/usb/mcba_usb.c
@@ -862,7 +862,7 @@ static int mcba_usb_probe(struct usb_interface *intf,
 		goto cleanup_unregister_candev;
 	}
 
-	dev_info(&intf->dev, "Microchip CAN BUS analizer connected\n");
+	dev_info(&intf->dev, "Microchip CAN BUS Analyzer connected\n");
 
 	return 0;
 
-- 
2.15.0

^ permalink raw reply related

* [PATCH 7/9] can: flexcan: fix VF610 state transition issue
From: Marc Kleine-Budde @ 2017-12-01 14:17 UTC (permalink / raw)
  To: netdev; +Cc: davem, linux-can, kernel, Marc Kleine-Budde, linux-stable
In-Reply-To: <20171201141739.5816-1-mkl@pengutronix.de>

Enable FLEXCAN_QUIRK_BROKEN_PERR_STATE for VF610 to report correct state
transitions.

Tested-by: Mirza Krak <mirza.krak@gmail.com>
Cc: linux-stable <stable@vger.kernel.org> # >= v4.11
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/flexcan.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index eefddae2e99a..0626dcfd1f3d 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -189,7 +189,7 @@
  *   MX35  FlexCAN2  03.00.00.00     no        no        no       no        no
  *   MX53  FlexCAN2  03.00.00.00    yes        no        no       no        no
  *   MX6s  FlexCAN3  10.00.12.00    yes       yes        no       no       yes
- *   VF610 FlexCAN3  ?               no       yes         ?      yes       yes?
+ *   VF610 FlexCAN3  ?               no       yes        no      yes       yes?
  *
  * Some SOCs do not have the RX_WARN & TX_WARN interrupt line connected.
  */
@@ -297,7 +297,8 @@ static const struct flexcan_devtype_data fsl_imx6q_devtype_data = {
 
 static const struct flexcan_devtype_data fsl_vf610_devtype_data = {
 	.quirks = FLEXCAN_QUIRK_DISABLE_RXFG | FLEXCAN_QUIRK_ENABLE_EACEN_RRS |
-		FLEXCAN_QUIRK_DISABLE_MECR | FLEXCAN_QUIRK_USE_OFF_TIMESTAMP,
+		FLEXCAN_QUIRK_DISABLE_MECR | FLEXCAN_QUIRK_USE_OFF_TIMESTAMP |
+		FLEXCAN_QUIRK_BROKEN_PERR_STATE,
 };
 
 static const struct can_bittiming_const flexcan_bittiming_const = {
-- 
2.15.0

^ permalink raw reply related

* [PATCH 6/9] can: flexcan: Update IRQ Err Passive information
From: Marc Kleine-Budde @ 2017-12-01 14:17 UTC (permalink / raw)
  To: netdev; +Cc: davem, linux-can, kernel, Marc Kleine-Budde
In-Reply-To: <20171201141739.5816-1-mkl@pengutronix.de>

The flexcan IP cores used on MX25 and MX35 do not generate Error Passive
IRQs. Update the IP core overview table in the driver accordingly.

Suggested-by: ZHU Yi (ST-FIR/ENG1-Zhu) <Yi.Zhu5@cn.bosch.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/flexcan.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index a13a4896a8bd..eefddae2e99a 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -184,9 +184,9 @@
  * Below is some version info we got:
  *    SOC   Version   IP-Version  Glitch- [TR]WRN_INT IRQ Err Memory err RTR re-
  *                                Filter? connected?  Passive detection  ception in MB
- *   MX25  FlexCAN2  03.00.00.00     no        no         ?       no        no
+ *   MX25  FlexCAN2  03.00.00.00     no        no        no       no        no
  *   MX28  FlexCAN2  03.00.04.00    yes       yes        no       no        no
- *   MX35  FlexCAN2  03.00.00.00     no        no         ?       no        no
+ *   MX35  FlexCAN2  03.00.00.00     no        no        no       no        no
  *   MX53  FlexCAN2  03.00.00.00    yes        no        no       no        no
  *   MX6s  FlexCAN3  10.00.12.00    yes       yes        no       no       yes
  *   VF610 FlexCAN3  ?               no       yes         ?      yes       yes?
-- 
2.15.0

^ permalink raw reply related

* [PATCH 5/9] can: peak/pci: fix potential bug when probe() fails
From: Marc Kleine-Budde @ 2017-12-01 14:17 UTC (permalink / raw)
  To: netdev
  Cc: davem, linux-can, kernel, Stephane Grosjean, linux-stable,
	Marc Kleine-Budde
In-Reply-To: <20171201141739.5816-1-mkl@pengutronix.de>

From: Stephane Grosjean <s.grosjean@peak-system.com>

PCI/PCIe drivers for PEAK-System CAN/CAN-FD interfaces do some access to the
PCI config during probing. In case one of these accesses fails, a POSITIVE
PCIBIOS_xxx error code is returned back. This POSITIVE error code MUST be
converted into a NEGATIVE errno for the probe() function to indicate it
failed. Using the pcibios_err_to_errno() function, we make sure that the
return code will always be negative.

Signed-off-by: Stephane Grosjean <s.grosjean@peak-system.com>
Cc: linux-stable <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/peak_canfd/peak_pciefd_main.c | 5 ++++-
 drivers/net/can/sja1000/peak_pci.c            | 5 ++++-
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/peak_canfd/peak_pciefd_main.c b/drivers/net/can/peak_canfd/peak_pciefd_main.c
index b4efd711f824..788c3464a3b0 100644
--- a/drivers/net/can/peak_canfd/peak_pciefd_main.c
+++ b/drivers/net/can/peak_canfd/peak_pciefd_main.c
@@ -825,7 +825,10 @@ static int peak_pciefd_probe(struct pci_dev *pdev,
 err_disable_pci:
 	pci_disable_device(pdev);
 
-	return err;
+	/* pci_xxx_config_word() return positive PCIBIOS_xxx error codes while
+	 * the probe() function must return a negative errno in case of failure
+	 * (err is unchanged if negative) */
+	return pcibios_err_to_errno(err);
 }
 
 /* free the board structure object, as well as its resources: */
diff --git a/drivers/net/can/sja1000/peak_pci.c b/drivers/net/can/sja1000/peak_pci.c
index 131026fbc2d7..5adc95c922ee 100644
--- a/drivers/net/can/sja1000/peak_pci.c
+++ b/drivers/net/can/sja1000/peak_pci.c
@@ -717,7 +717,10 @@ static int peak_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 failure_disable_pci:
 	pci_disable_device(pdev);
 
-	return err;
+	/* pci_xxx_config_word() return positive PCIBIOS_xxx error codes while
+	 * the probe() function must return a negative errno in case of failure
+	 * (err is unchanged if negative) */
+	return pcibios_err_to_errno(err);
 }
 
 static void peak_pci_remove(struct pci_dev *pdev)
-- 
2.15.0

^ permalink raw reply related

* [PATCH 4/9] can: ti_hecc: Fix napi poll return value for repoll
From: Marc Kleine-Budde @ 2017-12-01 14:17 UTC (permalink / raw)
  To: netdev
  Cc: davem, linux-can, kernel, Oliver Stäbler, linux-stable,
	Marc Kleine-Budde
In-Reply-To: <20171201141739.5816-1-mkl@pengutronix.de>

From: Oliver Stäbler <oliver.staebler@bytesatwork.ch>

After commit d75b1ade567f ("net: less interrupt masking in NAPI") napi
repoll is done only when work_done == budget.
So we need to return budget if there are still packets to receive.

Signed-off-by: Oliver Stäbler <oliver.staebler@bytesatwork.ch>
Cc: linux-stable <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/ti_hecc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/can/ti_hecc.c b/drivers/net/can/ti_hecc.c
index 4d4941469cfc..db6ea936dc3f 100644
--- a/drivers/net/can/ti_hecc.c
+++ b/drivers/net/can/ti_hecc.c
@@ -637,6 +637,9 @@ static int ti_hecc_rx_poll(struct napi_struct *napi, int quota)
 		mbx_mask = hecc_read(priv, HECC_CANMIM);
 		mbx_mask |= HECC_TX_MBOX_MASK;
 		hecc_write(priv, HECC_CANMIM, mbx_mask);
+	} else {
+		/* repoll is done only if whole budget is used */
+		num_pkts = quota;
 	}
 
 	return num_pkts;
-- 
2.15.0

^ permalink raw reply related

* [PATCH 2/9] can: kvaser_usb: Fix comparison bug in kvaser_usb_read_bulk_callback()
From: Marc Kleine-Budde @ 2017-12-01 14:17 UTC (permalink / raw)
  To: netdev
  Cc: davem, linux-can, kernel, Jimmy Assarsson, linux-stable,
	Marc Kleine-Budde
In-Reply-To: <20171201141739.5816-1-mkl@pengutronix.de>

From: Jimmy Assarsson <jimmyassarsson@gmail.com>

The conditon in the while-loop becomes true when actual_length is less than
2 (MSG_HEADER_LEN). In best case we end up with a former, already
dispatched msg, that got msg->len greater than actual_length. This will
result in a "Format error" error printout.

Problem seen when unplugging a Kvaser USB device connected to a vbox guest.

warning: comparison between signed and unsigned integer expressions
[-Wsign-compare]

Signed-off-by: Jimmy Assarsson <jimmyassarsson@gmail.com>
Cc: linux-stable <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/kvaser_usb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
index 075644591498..d87e330a20b3 100644
--- a/drivers/net/can/usb/kvaser_usb.c
+++ b/drivers/net/can/usb/kvaser_usb.c
@@ -1334,7 +1334,7 @@ static void kvaser_usb_read_bulk_callback(struct urb *urb)
 		goto resubmit_urb;
 	}
 
-	while (pos <= urb->actual_length - MSG_HEADER_LEN) {
+	while (pos <= (int)(urb->actual_length - MSG_HEADER_LEN)) {
 		msg = urb->transfer_buffer + pos;
 
 		/* The Kvaser firmware can only read and write messages that
-- 
2.15.0

^ permalink raw reply related

* pull-request: can 2017-12-01
From: Marc Kleine-Budde @ 2017-12-01 14:17 UTC (permalink / raw)
  To: netdev; +Cc: davem, linux-can, kernel

Hello David,

this is a pull for net consisting of nine patches.

The first three patches are by Jimmy Assarsson for the kvaser_usb driver
and add the missing free()s in some error path, a signed/unsigned
comparison and ratelimit the error messages in case of incomplete
messages. Oliver Stäbler's patch for the ti_hecc driver fix the napi
poll function's return value. The return values of the probe function of
the peak_canfd and peak_pci PCI drivers are fixed by Stephane Grosjean's
patch. Two patches by me for the flexcan driver update the
bugs/features/quirks overview table and fix the error state transition
for the VF610 SoC. The two patches by Martin Kelly for the mcba_usb
driver fix a typo and a device disconnect bug.

regards,
Marc

---

The following changes since commit 6fef90c6b3f6a2b52018e66c0886944ea0c03fcc:

  net: dsa: bcm_sf2: Set correct CHAIN_ID and slice number mask (2017-11-30 14:21:35 -0500)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can.git tags/linux-can-fixes-for-4.15-20171201

for you to fetch changes up to 1cb35a33a28394fd711bb26ddf3a564f4e9d9125:

  can: mcba_usb: fix device disconnect bug (2017-12-01 11:27:14 +0100)

----------------------------------------------------------------
linux-can-fixes-for-4.15-20171201

----------------------------------------------------------------
Jimmy Assarsson (3):
      can: kvaser_usb: free buf in error paths
      can: kvaser_usb: Fix comparison bug in kvaser_usb_read_bulk_callback()
      can: kvaser_usb: ratelimit errors if incomplete messages are received

Marc Kleine-Budde (2):
      can: flexcan: Update IRQ Err Passive information
      can: flexcan: fix VF610 state transition issue

Martin Kelly (2):
      can: mcba_usb: fix typo
      can: mcba_usb: fix device disconnect bug

Oliver Stäbler (1):
      can: ti_hecc: Fix napi poll return value for repoll

Stephane Grosjean (1):
      can: peak/pci: fix potential bug when probe() fails

 drivers/net/can/flexcan.c                     |  9 +++++----
 drivers/net/can/peak_canfd/peak_pciefd_main.c |  5 ++++-
 drivers/net/can/sja1000/peak_pci.c            |  5 ++++-
 drivers/net/can/ti_hecc.c                     |  3 +++
 drivers/net/can/usb/kvaser_usb.c              | 11 +++++++----
 drivers/net/can/usb/mcba_usb.c                |  3 ++-
 6 files changed, 25 insertions(+), 11 deletions(-)

^ permalink raw reply

* Re: pull-request: can-next 2017-12-01
From: Marc Kleine-Budde @ 2017-12-01 13:50 UTC (permalink / raw)
  To: netdev; +Cc: David Miller, kernel@pengutronix.de, linux-can@vger.kernel.org
In-Reply-To: <40a21f74-a30c-7af6-9892-2fac4d15bf94@pengutronix.de>


[-- Attachment #1.1: Type: text/plain, Size: 620 bytes --]

On 12/01/2017 02:37 PM, Marc Kleine-Budde wrote:
> Doh, I had an error in my script which produced the wrong Linux version
> in the tag. The correct name of the tag is:

First test - than post - here's the correct tag:

git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git
tags/linux-can-next-for-4.16-20171201

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: 488 bytes --]

^ permalink raw reply

* Re: pull-request: can-next 2017-12-01
From: Marc Kleine-Budde @ 2017-12-01 13:37 UTC (permalink / raw)
  To: netdev; +Cc: David Miller, kernel@pengutronix.de, linux-can@vger.kernel.org
In-Reply-To: <b2a795da-bde5-0527-ff07-762d5485ce45@pengutronix.de>


[-- Attachment #1.1: Type: text/plain, Size: 818 bytes --]

On 12/01/2017 10:57 AM, Marc Kleine-Budde wrote:
> Hello David,

[...]

> ---
> 
> The following changes since commit 201c78e05c5adaffa163b022c9b3a4d30debe100:
> 
>   Merge branch 'macb-rx-packet-filtering' (2017-11-30 14:12:47 -0500)
> 
> are available in the Git repository at:

Doh, I had an error in my script which produced the wrong Linux version
in the tag. The correct name of the tag is:

git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git tags/linux-can-next-for-v4.16-20171201

regards,
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: 488 bytes --]

^ permalink raw reply

* Re: [PATCH v2 2/2] can: flexcan: adding platform specific details for LS1021A
From: Marc Kleine-Budde @ 2017-12-01 10:32 UTC (permalink / raw)
  To: Mirza Krak
  Cc: Wolfgang Grandegger, ZHU Yi (ST-FIR/ENG1-Zhu), Pankaj Bansal,
	linux-can@vger.kernel.org, Varun Sethi, Poonam Aggrwal,
	Stefan Agner
In-Reply-To: <CALw8SCWhBZnh8P5c0MzTw95-SHhROhi68gOnGRBr9K+6hb2A5w@mail.gmail.com>


[-- Attachment #1.1: Type: text/plain, Size: 1277 bytes --]

On 12/01/2017 11:12 AM, Mirza Krak wrote:
> 2017-11-28 23:11 GMT+01:00 Mirza Krak <mirza.krak@gmail.com>:
>> 2017-11-28 23:09 GMT+01:00 Mirza Krak <mirza.krak@gmail.com>:
>>> 2017-11-27 15:00 GMT+01:00 Marc Kleine-Budde <mkl@pengutronix.de>:
>>>>
>>>> On 11/26/2017 10:11 PM, Mirza Krak wrote:
>>>>>>> Will try again next week when I am back at the office.
>>>>>>>
>>>>>>> I ran my tests on 4.14 kernel.
>>>>>>
>>>>>> This Flexcan core needs the FLEXCAN_QUIRK_BROKEN_PERR_STATE as well,
>>>>>> like
>>>>>> all other cores. Adding that quirk for the vf610 will cure the
>>>>>> problems.
>>>>>
>>>>> I will probably get some time during the coming week to test adding
>>>>> FLEXCAN_QUIRK_BROKEN_PERR_STATE and I can send a patch if it all looks
>>>>> good.
>>>>
>>>> I've created a patch, waiting for your Tested-by.
>>>>
>>>>
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can.git/log/?h=testing
> 
> Tested-by: Mirza Krak <mirza.krak@gmail.com>

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: 488 bytes --]

^ permalink raw reply

* Re: [PATCH v2 2/2] can: flexcan: adding platform specific details for LS1021A
From: Mirza Krak @ 2017-12-01 10:12 UTC (permalink / raw)
  To: Marc Kleine-Budde
  Cc: Wolfgang Grandegger, ZHU Yi (ST-FIR/ENG1-Zhu), Pankaj Bansal,
	linux-can@vger.kernel.org, Varun Sethi, Poonam Aggrwal,
	Stefan Agner
In-Reply-To: <CALw8SCXqcuUn_70704TxQAZrOFMbXhhdoYap2B4AFNJYFVw3Qg@mail.gmail.com>

2017-11-28 23:11 GMT+01:00 Mirza Krak <mirza.krak@gmail.com>:
> 2017-11-28 23:09 GMT+01:00 Mirza Krak <mirza.krak@gmail.com>:
>> 2017-11-27 15:00 GMT+01:00 Marc Kleine-Budde <mkl@pengutronix.de>:
>>>
>>> On 11/26/2017 10:11 PM, Mirza Krak wrote:
>>> >>> Will try again next week when I am back at the office.
>>> >>>
>>> >>> I ran my tests on 4.14 kernel.
>>> >>
>>> >> This Flexcan core needs the FLEXCAN_QUIRK_BROKEN_PERR_STATE as well,
>>> >> like
>>> >> all other cores. Adding that quirk for the vf610 will cure the
>>> >> problems.
>>> >
>>> > I will probably get some time during the coming week to test adding
>>> > FLEXCAN_QUIRK_BROKEN_PERR_STATE and I can send a patch if it all looks
>>> > good.
>>>
>>> I've created a patch, waiting for your Tested-by.
>>>
>>>
>>> https://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can.git/log/?h=testing

Tested-by: Mirza Krak <mirza.krak@gmail.com>


Test results:

1. Not connected and then connected

    root@colibri-vf:~# ./candump -td -e any,0:0,#FFFFFFFF
    (000.000000)  can0  20000004   [8]  00 04 00 00 00 00 00 00   ERRORFRAME
              controller-problem{rx-error-warning}
    (000.000120)  can0  20000004   [8]  00 10 00 00 00 00 00 00   ERRORFRAME
              controller-problem{rx-error-passive}

And I also get the following:

    (000.000011)  can0  20000004   [8]  00 04 00 00 00 00 00 00   ERRORFRAME
              controller-problem{rx-error-warning}

and

(000.000008)  can0  20000004   [8]  00 40 00 00 00 00 00 00   ERRORFRAME
              controller-problem{back-to-error-active}

When I re-connect to the network.

2. Short on CAN_H and CAN_L

    root@colibri-vf:~# ./candump -td -e any,0:0,#FFFFFFFF
    (000.000000)  can0  20000004   [8]  00 08 00 00 00 00 00 00   ERRORFRAME
              controller-problem{tx-error-warning}
    (000.000043)  can0  20000004   [8]  00 20 00 00 00 00 00 00   ERRORFRAME
              controller-problem{tx-error-passive}
    (000.000745)  can0  20000040   [8]  00 00 00 00 00 00 00 00   ERRORFRAME
              bus-off
    (000.100306)  can0  20000100   [8]  00 00 00 00 00 00 00 00   ERRORFRAME
              restarted-after-bus-off
    (000.111280)  can0  20000004   [8]  00 08 00 00 00 00 00 00   ERRORFRAME
              controller-problem{tx-error-warning}
    (000.000034)  can0  20000004   [8]  00 20 00 00 00 00 00 00   ERRORFRAME
              controller-problem{tx-error-passive}
    (000.000759)  can0  20000040   [8]  00 00 00 00 00 00 00 00   ERRORFRAME
              bus-off

-- 
Med Vänliga Hälsningar / Best Regards

Mirza Krak

^ permalink raw reply


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