Linux CAN drivers development
 help / color / mirror / Atom feed
* Re: CAN-USB adapter unplug
From: Jimmy Assarsson @ 2017-11-30 10:18 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: <e5a2cbc8-f1be-1bed-a0af-9b5fa6fd35ca@xevo.com>

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.

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 1/2] dt-binding: can: mcp2517fd: document device tree bindings
From: kernel @ 2017-11-30  7:24 UTC (permalink / raw)
  To: Patrick Menschel; +Cc: linux-can, devicetree
In-Reply-To: <0a7e4416-332a-5c23-bda3-8c7561410314@posteo.de>

Hi Patrick!

> On 29.11.2017, at 21:35, Patrick Menschel <menschel.p@posteo.de> wrote:
> 
> Hello Martin,
> 
> 
> I didn't catch the whole discussion but you may want to check
> 
> include/dt-bindings/gpio/gpio.h
> 
>> #define GPIO_OPEN_DRAIN (GPIO_SINGLE_ENDED | GPIO_ACTIVE_LOW)
> 
> This corresponds to
> 
> Documentation/devicetree/bindings/gpio/gpio.txt

I understand, but the question still is: how to
present the information in a valid way.

To use gpio propperly it would require that the driver
implements a “sub-driver” pinctrl with all the extra
(boilerplate) code overhead.

Also this would mean mixing different types of
logical drivers into a single source - I doubt that
would be easy to get accepted...

Here again a summary of all the GPIOs that the mcp2517fd has:
* TXCAN: dedicated GPIO with single function, 
         individually conigurable as push/pull or open drain
* INT: main interrupt line - configurable as push/pull or 
         individually conigurable as push/pull or open drain
* GPIO0: general GPIO with in/out option, but 2 special “cases”:
         tx-irq and TX-disable
         group configurable as push/pull or open drain
* GPIO1: general GPIO with in/out potion, but 1 special “cases”:
         rx-irq
         group configurable as push/pull or open drain
* CLKO/SOF: clock output at (1/10th, 1/5th, 1/2, 1 of the 
            core frequency) or start of frame output
            possibly group configurable as push/pull or open drain
	    (not explicitly specified in datasheet)

How would you try to present that HW-configuration in the 
device tree instead?
How would it impact the driver design?

Thanks,
	Martin


^ permalink raw reply

* Re: [PATCH 1/2] dt-binding: can: mcp2517fd: document device tree bindings
From: Patrick Menschel @ 2017-11-29 20:35 UTC (permalink / raw)
  To: kernel-TqfNSX0MhmxHKSADF0wUEw
  Cc: linux-can-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <6EBDD798-8632-4F42-A138-369BCD36DF68-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>

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

Hello Martin,


I didn't catch the whole discussion but you may want to check

include/dt-bindings/gpio/gpio.h

> #define GPIO_OPEN_DRAIN (GPIO_SINGLE_ENDED | GPIO_ACTIVE_LOW)

This corresponds to

Documentation/devicetree/bindings/gpio/gpio.txt

> Most controllers are however specifying a generic flag bitfield
> in the last cell, so for these, use the macros defined in
> include/dt-bindings/gpio/gpio.h whenever possible:
>
> Example of a node using GPIOs:
>
>     node {
>         enable-gpios = <&qe_pio_e 18 GPIO_ACTIVE_HIGH>;
>     };
>
> GPIO_ACTIVE_HIGH is 0, so in this example gpio-specifier is "18 0" and
> encodes
> GPIO pin number, and GPIO flags as accepted by the "qe_pio_e"
> gpio-controller.
>
> Optional standard bitfield specifiers for the last cell:
>
> - Bit 0: 0 means active high, 1 means active low
> - Bit 1: 1 means single-ended wiring, see:
>            https://en.wikipedia.org/wiki/Single-ended_triode
>        When used with active-low, this means open drain/collector, see:
>            https://en.wikipedia.org/wiki/Open_collector
>        When used with active-high, this means open source/emitter

Regards,

Patrick



Am 29.11.2017 um 12:55 schrieb kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org:
> Hi Rob!
>
> Thanks for all the effort - I shall incorporate those changes into V2.
>
>> s/Microcip/Microchip/
>> s/_/-/
>>
>>> +					   0 = Start of Frame output
>>> +					   default: 10
>>> + - microchip,clock_div = <1|2>: internal clock divider - default 1
>>> + - microchip,gpio_opendrain: gpio (int0,1) in open drain mode
>>> +			     instead of default push/pull
>>> + - microchip,int_opendrain: int pin in open drain mode
>>> +			    instead of default push/pull
>> IIRC, we have a standard property for this.
> Would you know what that could be - I did a quick search for
> standard properties, but could not find a definitive list…
>
> The only thing I found in:
>   Documentation/devicetree/bindings/w1/w1-gpio.txt
> is:
>   linux,open-drain
>
> But there is also:  
>   nvidia,open-drain
>   open_drain
>   open-drain
>   drive-open-drain (pinctrl)
>   st,irq-open-drain
>
> Only the last is specific to the interrupt line and none to the other
> GPIOs of the chip (txcan, GPIO) - and each can get set differently.
>
> So how shall I implement that?
>
> Thanks,
> 	Martin--
> To unsubscribe from this list: send the line "unsubscribe linux-can" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 3992 bytes --]

^ permalink raw reply

* Re: CAN-USB adapter unplug
From: Martin Kelly @ 2017-11-29 17:07 UTC (permalink / raw)
  To: Stefan Mätje, Marc Kleine-Budde, linux-can@vger.kernel.org
  Cc: Remigiusz Kołłątaj, Wolfgang Grandegger
In-Reply-To: <2f999f02-028e-1e6c-9020-beb5a76889cc@esd.eu>

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.

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.

> 
> 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: CAN-USB adapter unplug
From: Martin Kelly @ 2017-11-29 17:04 UTC (permalink / raw)
  To: Marc Kleine-Budde, linux-can
  Cc: Remigiusz Kołłątaj, Wolfgang Grandegger
In-Reply-To: <48029f4a-5137-5f1f-6970-5668fb8da599@pengutronix.de>

On 11/29/2017 04:20 AM, Marc Kleine-Budde wrote:
> 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?
> 

Yes, I can do this, although they will be untested.

> I don't have access to every USB adapter neither.
> 
> Marc
> 

^ permalink raw reply

* [PATCH v3 30/36] can/bcm: Replace hrtimer_tasklet with softirq based hrtimer
From: Anna-Maria Gleixner @ 2017-11-29 15:30 UTC (permalink / raw)
  To: LKML
  Cc: Thomas Gleixner, Peter Zijlstra, Ingo Molnar, keescook,
	Christoph Hellwig, John Stultz, Anna-Maria Gleixner,
	Oliver Hartkopp, Marc Kleine-Budde, linux-can
In-Reply-To: <20171129153101.27297-1-anna-maria@linutronix.de>

From: Thomas Gleixner <tglx@linutronix.de>

Switch the timer to HRTIMER_MODE_SOFT, which executed the timer
callback in softirq context and remove the hrtimer_tasklet.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Anna-Maria Gleixner <anna-maria@linutronix.de>
Acked-by: Oliver Hartkopp <socketcan@hartkopp.net>
Cc: Oliver Hartkopp <socketcan@hartkopp.net>
Cc: Marc Kleine-Budde <mkl@pengutronix.de>
Cc: linux-can@vger.kernel.org
---
 net/can/bcm.c | 156 ++++++++++++++++++++--------------------------------------
 1 file changed, 52 insertions(+), 104 deletions(-)

diff --git a/net/can/bcm.c b/net/can/bcm.c
index 13690334efa3..9cc67ac257f1 100644
--- a/net/can/bcm.c
+++ b/net/can/bcm.c
@@ -102,7 +102,6 @@ struct bcm_op {
 	unsigned long frames_abs, frames_filtered;
 	struct bcm_timeval ival1, ival2;
 	struct hrtimer timer, thrtimer;
-	struct tasklet_struct tsklet, thrtsklet;
 	ktime_t rx_stamp, kt_ival1, kt_ival2, kt_lastmsg;
 	int rx_ifindex;
 	int cfsiz;
@@ -364,25 +363,34 @@ static void bcm_send_to_user(struct bcm_op *op, struct bcm_msg_head *head,
 	}
 }
 
-static void bcm_tx_start_timer(struct bcm_op *op)
+static bool bcm_tx_set_expiry(struct bcm_op *op, struct hrtimer *hrt)
 {
+	ktime_t ival;
+
 	if (op->kt_ival1 && op->count)
-		hrtimer_start(&op->timer,
-			      ktime_add(ktime_get(), op->kt_ival1),
-			      HRTIMER_MODE_ABS);
+		ival = op->kt_ival1;
 	else if (op->kt_ival2)
-		hrtimer_start(&op->timer,
-			      ktime_add(ktime_get(), op->kt_ival2),
-			      HRTIMER_MODE_ABS);
+		ival = op->kt_ival2;
+	else
+		return false;
+
+	hrtimer_set_expires(hrt, ktime_add(ktime_get(), ival));
+	return true;
 }
 
-static void bcm_tx_timeout_tsklet(unsigned long data)
+static void bcm_tx_start_timer(struct bcm_op *op)
 {
-	struct bcm_op *op = (struct bcm_op *)data;
+	if (bcm_tx_set_expiry(op, &op->timer))
+		hrtimer_start_expires(&op->timer, HRTIMER_MODE_ABS_SOFT);
+}
+
+/* bcm_tx_timeout_handler - performs cyclic CAN frame transmissions */
+static enum hrtimer_restart bcm_tx_timeout_handler(struct hrtimer *hrtimer)
+{
+	struct bcm_op *op = container_of(hrtimer, struct bcm_op, timer);
 	struct bcm_msg_head msg_head;
 
 	if (op->kt_ival1 && (op->count > 0)) {
-
 		op->count--;
 		if (!op->count && (op->flags & TX_COUNTEVT)) {
 
@@ -399,22 +407,12 @@ static void bcm_tx_timeout_tsklet(unsigned long data)
 		}
 		bcm_can_tx(op);
 
-	} else if (op->kt_ival2)
+	} else if (op->kt_ival2) {
 		bcm_can_tx(op);
+	}
 
-	bcm_tx_start_timer(op);
-}
-
-/*
- * bcm_tx_timeout_handler - performs cyclic CAN frame transmissions
- */
-static enum hrtimer_restart bcm_tx_timeout_handler(struct hrtimer *hrtimer)
-{
-	struct bcm_op *op = container_of(hrtimer, struct bcm_op, timer);
-
-	tasklet_schedule(&op->tsklet);
-
-	return HRTIMER_NORESTART;
+	return bcm_tx_set_expiry(op, &op->timer) ?
+		HRTIMER_RESTART : HRTIMER_NORESTART;
 }
 
 /*
@@ -480,7 +478,7 @@ static void bcm_rx_update_and_send(struct bcm_op *op,
 		/* do not send the saved data - only start throttle timer */
 		hrtimer_start(&op->thrtimer,
 			      ktime_add(op->kt_lastmsg, op->kt_ival2),
-			      HRTIMER_MODE_ABS);
+			      HRTIMER_MODE_ABS_SOFT);
 		return;
 	}
 
@@ -539,14 +537,21 @@ static void bcm_rx_starttimer(struct bcm_op *op)
 		return;
 
 	if (op->kt_ival1)
-		hrtimer_start(&op->timer, op->kt_ival1, HRTIMER_MODE_REL);
+		hrtimer_start(&op->timer, op->kt_ival1, HRTIMER_MODE_REL_SOFT);
 }
 
-static void bcm_rx_timeout_tsklet(unsigned long data)
+/* bcm_rx_timeout_handler - when the (cyclic) CAN frame reception timed out */
+static enum hrtimer_restart bcm_rx_timeout_handler(struct hrtimer *hrtimer)
 {
-	struct bcm_op *op = (struct bcm_op *)data;
+	struct bcm_op *op = container_of(hrtimer, struct bcm_op, timer);
 	struct bcm_msg_head msg_head;
 
+	/* if user wants to be informed, when cyclic CAN-Messages come back */
+	if ((op->flags & RX_ANNOUNCE_RESUME) && op->last_frames) {
+		/* clear received CAN frames to indicate 'nothing received' */
+		memset(op->last_frames, 0, op->nframes * op->cfsiz);
+	}
+
 	/* create notification to user */
 	msg_head.opcode  = RX_TIMEOUT;
 	msg_head.flags   = op->flags;
@@ -557,25 +562,6 @@ static void bcm_rx_timeout_tsklet(unsigned long data)
 	msg_head.nframes = 0;
 
 	bcm_send_to_user(op, &msg_head, NULL, 0);
-}
-
-/*
- * bcm_rx_timeout_handler - when the (cyclic) CAN frame reception timed out
- */
-static enum hrtimer_restart bcm_rx_timeout_handler(struct hrtimer *hrtimer)
-{
-	struct bcm_op *op = container_of(hrtimer, struct bcm_op, timer);
-
-	/* schedule before NET_RX_SOFTIRQ */
-	tasklet_hi_schedule(&op->tsklet);
-
-	/* no restart of the timer is done here! */
-
-	/* if user wants to be informed, when cyclic CAN-Messages come back */
-	if ((op->flags & RX_ANNOUNCE_RESUME) && op->last_frames) {
-		/* clear received CAN frames to indicate 'nothing received' */
-		memset(op->last_frames, 0, op->nframes * op->cfsiz);
-	}
 
 	return HRTIMER_NORESTART;
 }
@@ -583,14 +569,12 @@ static enum hrtimer_restart bcm_rx_timeout_handler(struct hrtimer *hrtimer)
 /*
  * bcm_rx_do_flush - helper for bcm_rx_thr_flush
  */
-static inline int bcm_rx_do_flush(struct bcm_op *op, int update,
-				  unsigned int index)
+static inline int bcm_rx_do_flush(struct bcm_op *op, unsigned int index)
 {
 	struct canfd_frame *lcf = op->last_frames + op->cfsiz * index;
 
 	if ((op->last_frames) && (lcf->flags & RX_THR)) {
-		if (update)
-			bcm_rx_changed(op, lcf);
+		bcm_rx_changed(op, lcf);
 		return 1;
 	}
 	return 0;
@@ -598,11 +582,8 @@ static inline int bcm_rx_do_flush(struct bcm_op *op, int update,
 
 /*
  * bcm_rx_thr_flush - Check for throttled data and send it to the userspace
- *
- * update == 0 : just check if throttled data is available  (any irq context)
- * update == 1 : check and send throttled data to userspace (soft_irq context)
  */
-static int bcm_rx_thr_flush(struct bcm_op *op, int update)
+static int bcm_rx_thr_flush(struct bcm_op *op)
 {
 	int updated = 0;
 
@@ -611,24 +592,16 @@ static int bcm_rx_thr_flush(struct bcm_op *op, int update)
 
 		/* for MUX filter we start at index 1 */
 		for (i = 1; i < op->nframes; i++)
-			updated += bcm_rx_do_flush(op, update, i);
+			updated += bcm_rx_do_flush(op, i);
 
 	} else {
 		/* for RX_FILTER_ID and simple filter */
-		updated += bcm_rx_do_flush(op, update, 0);
+		updated += bcm_rx_do_flush(op, 0);
 	}
 
 	return updated;
 }
 
-static void bcm_rx_thr_tsklet(unsigned long data)
-{
-	struct bcm_op *op = (struct bcm_op *)data;
-
-	/* push the changed data to the userspace */
-	bcm_rx_thr_flush(op, 1);
-}
-
 /*
  * bcm_rx_thr_handler - the time for blocked content updates is over now:
  *                      Check for throttled data and send it to the userspace
@@ -637,9 +610,7 @@ static enum hrtimer_restart bcm_rx_thr_handler(struct hrtimer *hrtimer)
 {
 	struct bcm_op *op = container_of(hrtimer, struct bcm_op, thrtimer);
 
-	tasklet_schedule(&op->thrtsklet);
-
-	if (bcm_rx_thr_flush(op, 0)) {
+	if (bcm_rx_thr_flush(op)) {
 		hrtimer_forward(hrtimer, ktime_get(), op->kt_ival2);
 		return HRTIMER_RESTART;
 	} else {
@@ -735,23 +706,8 @@ static struct bcm_op *bcm_find_op(struct list_head *ops,
 
 static void bcm_remove_op(struct bcm_op *op)
 {
-	if (op->tsklet.func) {
-		while (test_bit(TASKLET_STATE_SCHED, &op->tsklet.state) ||
-		       test_bit(TASKLET_STATE_RUN, &op->tsklet.state) ||
-		       hrtimer_active(&op->timer)) {
-			hrtimer_cancel(&op->timer);
-			tasklet_kill(&op->tsklet);
-		}
-	}
-
-	if (op->thrtsklet.func) {
-		while (test_bit(TASKLET_STATE_SCHED, &op->thrtsklet.state) ||
-		       test_bit(TASKLET_STATE_RUN, &op->thrtsklet.state) ||
-		       hrtimer_active(&op->thrtimer)) {
-			hrtimer_cancel(&op->thrtimer);
-			tasklet_kill(&op->thrtsklet);
-		}
-	}
+	hrtimer_cancel(&op->timer);
+	hrtimer_cancel(&op->thrtimer);
 
 	if ((op->frames) && (op->frames != &op->sframe))
 		kfree(op->frames);
@@ -979,15 +935,13 @@ static int bcm_tx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
 		op->ifindex = ifindex;
 
 		/* initialize uninitialized (kzalloc) structure */
-		hrtimer_init(&op->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+		hrtimer_init(&op->timer, CLOCK_MONOTONIC,
+			     HRTIMER_MODE_REL_SOFT);
 		op->timer.function = bcm_tx_timeout_handler;
 
-		/* initialize tasklet for tx countevent notification */
-		tasklet_init(&op->tsklet, bcm_tx_timeout_tsklet,
-			     (unsigned long) op);
-
 		/* currently unused in tx_ops */
-		hrtimer_init(&op->thrtimer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+		hrtimer_init(&op->thrtimer, CLOCK_MONOTONIC,
+			     HRTIMER_MODE_REL_SOFT);
 
 		/* add this bcm_op to the list of the tx_ops */
 		list_add(&op->list, &bo->tx_ops);
@@ -1150,20 +1104,14 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
 		op->rx_ifindex = ifindex;
 
 		/* initialize uninitialized (kzalloc) structure */
-		hrtimer_init(&op->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+		hrtimer_init(&op->timer, CLOCK_MONOTONIC,
+			     HRTIMER_MODE_REL_SOFT);
 		op->timer.function = bcm_rx_timeout_handler;
 
-		/* initialize tasklet for rx timeout notification */
-		tasklet_init(&op->tsklet, bcm_rx_timeout_tsklet,
-			     (unsigned long) op);
-
-		hrtimer_init(&op->thrtimer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+		hrtimer_init(&op->thrtimer, CLOCK_MONOTONIC,
+			     HRTIMER_MODE_REL_SOFT);
 		op->thrtimer.function = bcm_rx_thr_handler;
 
-		/* initialize tasklet for rx throttle handling */
-		tasklet_init(&op->thrtsklet, bcm_rx_thr_tsklet,
-			     (unsigned long) op);
-
 		/* add this bcm_op to the list of the rx_ops */
 		list_add(&op->list, &bo->rx_ops);
 
@@ -1209,12 +1157,12 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
 			 */
 			op->kt_lastmsg = 0;
 			hrtimer_cancel(&op->thrtimer);
-			bcm_rx_thr_flush(op, 1);
+			bcm_rx_thr_flush(op);
 		}
 
 		if ((op->flags & STARTTIMER) && op->kt_ival1)
 			hrtimer_start(&op->timer, op->kt_ival1,
-				      HRTIMER_MODE_REL);
+				      HRTIMER_MODE_REL_SOFT);
 	}
 
 	/* now we can register for can_ids, if we added a new bcm_op */
-- 
2.11.0

^ permalink raw reply related

* Re: CAN-USB adapter unplug
From: Stefan Mätje @ 2017-11-29 15:21 UTC (permalink / raw)
  To: Marc Kleine-Budde, Martin Kelly, linux-can@vger.kernel.org
  Cc: Remigiusz Kołłątaj, Wolfgang Grandegger
In-Reply-To: <48029f4a-5137-5f1f-6970-5668fb8da599@pengutronix.de>

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.

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

------------------------------------------------------------------------
Dipl.-Ing. Stefan Mätje
System-Design

esd electronics gmbh                                                    
Vahrenwalder Str. 207 - 30165 Hannover - GERMANY                        
Telefon: 0511-37298-146 - Fax: 0511-37298-68                            
E-Mail: Stefan.Maetje@esd.eu                                            
Bitte besuchen Sie uns im Internet unter http://www.esd.eu              
Quality Products - Made in Germany                                      
------------------------------------------------------------------------
Geschäftsführer: Klaus Detering, Norbert Gemmeke                        
Amtsgericht Hannover HRB 51373 - VAT-ID DE 115672832                    
------------------------------------------------------------------------

^ permalink raw reply

* CAN-USB adapter unplug (was: Re: [PATCH v2 1/2] can: mcba_usb: fix typo)
From: Marc Kleine-Budde @ 2017-11-29 12:20 UTC (permalink / raw)
  To: Martin Kelly, linux-can
  Cc: Remigiusz Kołłątaj, Wolfgang Grandegger
In-Reply-To: <1c246910-a55b-f16c-bff7-b93f3310ce7b@xevo.com>


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

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

-- 
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 1/2] dt-binding: can: mcp2517fd: document device tree bindings
From: kernel @ 2017-11-29 11:55 UTC (permalink / raw)
  To: Rob Herring
  Cc: Wolfgang Grandegger, Marc Kleine-Budde, Mark Rutland, linux-can,
	devicetree
In-Reply-To: <20171126222528.i6quueqdmuj6le6g@rob-hp-laptop>

Hi Rob!

Thanks for all the effort - I shall incorporate those changes into V2.

> s/Microcip/Microchip/
> s/_/-/
> 
>> +					   0 = Start of Frame output
>> +					   default: 10
>> + - microchip,clock_div = <1|2>: internal clock divider - default 1
>> + - microchip,gpio_opendrain: gpio (int0,1) in open drain mode
>> +			     instead of default push/pull
>> + - microchip,int_opendrain: int pin in open drain mode
>> +			    instead of default push/pull
> 
> IIRC, we have a standard property for this.

Would you know what that could be - I did a quick search for
standard properties, but could not find a definitive list…

The only thing I found in:
  Documentation/devicetree/bindings/w1/w1-gpio.txt
is:
  linux,open-drain

But there is also:  
  nvidia,open-drain
  open_drain
  open-drain
  drive-open-drain (pinctrl)
  st,irq-open-drain

Only the last is specific to the interrupt line and none to the other
GPIOs of the chip (txcan, GPIO) - and each can get set differently.

So how shall I implement that?

Thanks,
	Martin

^ permalink raw reply

* Re: [PATCH v2 2/2] can: flexcan: adding platform specific details for LS1021A
From: Mirza Krak @ 2017-11-28 22:11 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: <CALw8SCVNqN0SM1e=bxXZFMVL0VN0iy0LgFj9Hv4BeRwAbb9Y6A@mail.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
>
>
> Great thanks. I will not have access to hardware until Friday this week,
> hope you are patient :)

Grrr, stupid email client (last mail went out as HTML), sorry.

Re-send as plain-text for it to reach mailing list as well.

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

Mirza Krak

^ permalink raw reply

* Re: [PATCH v2 1/2] can: mcba_usb: fix typo
From: Martin Kelly @ 2017-11-28 21:09 UTC (permalink / raw)
  To: Marc Kleine-Budde, linux-can
  Cc: Remigiusz Kołłątaj, Wolfgang Grandegger
In-Reply-To: <31e3532f-5899-5d0a-f27a-378fe2ee5124@pengutronix.de>

On 11/28/2017 12:46 AM, Marc Kleine-Budde wrote:
> On 11/28/2017 12:49 AM, Martin Kelly wrote:
>> Fix typo "analizer" --> "Analyzer".
>>
>> Signed-off-by: Martin Kelly <mkelly@xevo.com>
> 
> 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.

^ permalink raw reply

* Re: [PATCH v2 1/2] can: mcba_usb: fix typo
From: Marc Kleine-Budde @ 2017-11-28  8:46 UTC (permalink / raw)
  To: Martin Kelly, linux-can
  Cc: Remigiusz Kołłątaj, Wolfgang Grandegger
In-Reply-To: <20171127234916.25865-1-mkelly@xevo.com>


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

On 11/28/2017 12:49 AM, Martin Kelly wrote:
> Fix typo "analizer" --> "Analyzer".
> 
> Signed-off-by: Martin Kelly <mkelly@xevo.com>

Both applied to can.

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 1/2] can: mcba_usb: fix typo
From: Martin Kelly @ 2017-11-27 23:49 UTC (permalink / raw)
  To: Marc Kleine-Budde, linux-can
  Cc: Remigiusz Kołłątaj, Wolfgang Grandegger
In-Reply-To: <2f7df48e-ee2a-f6b8-3251-e839b6bbdc73@pengutronix.de>

On 11/27/2017 03:17 PM, Marc Kleine-Budde wrote:
> On 11/27/2017 09:41 PM, Martin Kelly wrote:
>> Signed-off-by: Martin Kelly <mkelly@xevo.com>
> 
> Please add a patch description. Let's have every patch one.
> 
> Marc
> 

Alright, I sent a v2 series with a description.

^ permalink raw reply

* [PATCH v2 2/2] can: mcba_usb: fix device disconnect bug
From: Martin Kelly @ 2017-11-27 23:49 UTC (permalink / raw)
  To: linux-can
  Cc: Remigiusz Kołłątaj, Marc Kleine-Budde,
	Wolfgang Grandegger, Martin Kelly
In-Reply-To: <20171127234916.25865-1-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>
---
 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.11.0


^ permalink raw reply related

* [PATCH v2 1/2] can: mcba_usb: fix typo
From: Martin Kelly @ 2017-11-27 23:49 UTC (permalink / raw)
  To: linux-can
  Cc: Remigiusz Kołłątaj, Marc Kleine-Budde,
	Wolfgang Grandegger, Martin Kelly

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

Signed-off-by: Martin Kelly <mkelly@xevo.com>
---
 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.11.0


^ permalink raw reply related

* Re: [PATCH 1/2] can: mcba_usb: fix typo
From: Marc Kleine-Budde @ 2017-11-27 23:17 UTC (permalink / raw)
  To: Martin Kelly, linux-can
  Cc: Remigiusz Kołłątaj, Wolfgang Grandegger
In-Reply-To: <20171127204200.32599-1-mkelly@xevo.com>


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

On 11/27/2017 09:41 PM, Martin Kelly wrote:
> Signed-off-by: Martin Kelly <mkelly@xevo.com>

Please add a patch description. Let's have every patch one.

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

* [PATCH 2/2] can: mcba_usb: fix device disconnect bug
From: Martin Kelly @ 2017-11-27 20:42 UTC (permalink / raw)
  To: linux-can
  Cc: Remigiusz Kołłątaj, Marc Kleine-Budde,
	Wolfgang Grandegger, Martin Kelly
In-Reply-To: <20171127204200.32599-1-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>
---
 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.11.0


^ permalink raw reply related

* [PATCH 1/2] can: mcba_usb: fix typo
From: Martin Kelly @ 2017-11-27 20:41 UTC (permalink / raw)
  To: linux-can
  Cc: Remigiusz Kołłątaj, Marc Kleine-Budde,
	Wolfgang Grandegger, Martin Kelly

Signed-off-by: Martin Kelly <mkelly@xevo.com>
---
 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.11.0


^ permalink raw reply related

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

On 2017-11-23 21:17, Mirza Krak wrote:
> 2017-11-23 21:12 GMT+01:00 Mirza Krak <mirza.krak@gmail.com>:
>> On Nov 22, 2017 13:00, "Marc Kleine-Budde" <mkl@pengutronix.de> wrote:
>>
> 
> < snip >
> 
>>
>> Cc'ed Stefan Agner - maybe he has access to this SoC.
>>

Without reading the full thread, I did some testing back when we added
vf610 support:
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-July/275903.html

>>> @Marc, @Wolfgang,
>>> What do you think? Or shall we fix first and then wait?
>>
>> Let's wait if Stefan can test, otherwise just fix.
>>
>>
>> I have access to an vf610 and can test CAN stuff.
>>
>> What is it more specifically we want to test? I read the thread but not
>> completely clear to me.
> 

Thanks Mirza for your testing!

--
Stefan

^ permalink raw reply

* Re: [PATCH] can: ti_hecc: Fix napi poll return value for repoll
From: Marc Kleine-Budde @ 2017-11-27 14:18 UTC (permalink / raw)
  To: Oliver Stäbler, Wolfgang Grandegger, linux-can, netdev
In-Reply-To: <20171120134515.2658-1-oliver.staebler@bytesatwork.ch>


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

On 11/20/2017 02:45 PM, Oliver Stäbler wrote:
> 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>

Applied to can.

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] can: peak/pci: fix potential bug when probe() fails
From: Marc Kleine-Budde @ 2017-11-27 14:16 UTC (permalink / raw)
  To: Stephane Grosjean, Oliver Hartkopp; +Cc: linux-can Mailing List
In-Reply-To: <20171123144435.12282-1-s.grosjean@peak-system.com>


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

On 11/23/2017 03:44 PM, Stephane Grosjean wrote:
> 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>

Applied to can.

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 v4 0/6] Remodel FlexCAN register r/w APIs for big endian
From: Marc Kleine-Budde @ 2017-11-27 14:07 UTC (permalink / raw)
  To: Pankaj Bansal, wg-5Yr1BZd7O62+XT7JhA+gdA,
	linux-can-u79uwXL29TY76Z2rM5mHXA, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA
  Cc: V.Sethi-3arQi8VN3Tc, poonam.aggrwal-3arQi8VN3Tc
In-Reply-To: <1511529733-27942-1-git-send-email-pankaj.bansal-3arQi8VN3Tc@public.gmane.org>


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

On 11/24/2017 02:22 PM, Pankaj Bansal wrote:
> This patch series remodels the FlexCAN register r/w APIs for big endian.
> The endianness is checked based on optional big-endian property in
> device tree. if this property is not present in device tree node then 
> controller is assumed to be little endian. if this property is present then
> controller is assumed to be big endian.
> 
> An exception to this rule is powerpc P1010RDB, which is always
> big-endian, even if big-endian is not present in dts. This is
> checked using p1010-flexcan compatible in dts.
> 
> Therefore, remove p1010-flexcan compatible from imx series dts,
> as their flexcan core is little endian.
> 
> Finally this series adds support for NXP LS1021A SOC in flexcan,
> which is arm based SOC having big-endian FlexCAN controller.
> 
> Pankaj Bansal (6):
>   can: flexcan: Remodel FlexCAN register r/w APIs for big endian FlexCAN
>     controllers.
>   can: flexcan: adding platform specific details for LS1021A
>   Documentation : can : flexcan : Add big-endian property to device tree
>   powerpc: dts: P1010: Add endianness property to flexcan node
>   arm: dts: Remove p1010-flexcan compatible from imx series dts
>   arm/dts: Add nodes for flexcan devices present on LS1021A-Rev2 SoC

Applied to can-next.

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

^ permalink raw reply

* Re: [PATCH v4 1/6] can: flexcan: Remodel FlexCAN register r/w APIs for big endian FlexCAN controllers.
From: Marc Kleine-Budde @ 2017-11-27 14:06 UTC (permalink / raw)
  To: Pankaj Bansal, wg@grandegger.com, linux-can@vger.kernel.org,
	robh+dt@kernel.org, mark.rutland@arm.com,
	devicetree@vger.kernel.org
  Cc: Varun Sethi, Poonam Aggrwal, Bhupesh Sharma, Sakar Arora
In-Reply-To: <AM0PR0402MB39405B9B5E5C04B2492221D6F1240@AM0PR0402MB3940.eurprd04.prod.outlook.com>


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

On 11/26/2017 03:20 AM, Pankaj Bansal wrote:
>>> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
>>> index a13a489..4c873fb 100644
>>> --- a/drivers/net/can/flexcan.c
>>> +++ b/drivers/net/can/flexcan.c
>>
>> [...]
>> I think this will select LE for non DT devices, right?
>>
> 
> Yes.  As per below code snippet:
> 
> static struct property *__of_find_property(const struct device_node *np,
> 					   const char *name, int *lenp)
> {
> 	struct property *pp;
> 
> 	if (!np)
> 		return NULL;
>               ....
> }
> 
> If no device node is present null is returned.
> So we select le as default.

This is what I found out, too.

>>> +	if (of_property_read_bool(pdev->dev.of_node, "big-endian")) {
>>> +		priv->read = flexcan_read_be;
>>> +		priv->write = flexcan_write_be;
>>> +	} else {
>>> +		if (of_device_is_compatible(pdev->dev.of_node,
>>> +					    "fsl,p1010-flexcan")) {
>>> +			priv->read = flexcan_read_be;
>>> +			priv->write = flexcan_write_be;
>>> +		} else {
>>> +			priv->read = flexcan_read_le;
>>> +			priv->write = flexcan_write_le;
>>> +		}
>>> +	}
>>> +
>>>  	priv->can.clock.freq = clock_freq;
>>>  	priv->can.bittiming_const = &flexcan_bittiming_const;
>>>  	priv->can.do_set_mode = flexcan_set_mode;

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-11-27 14:00 UTC (permalink / raw)
  To: Mirza Krak, Wolfgang Grandegger
  Cc: ZHU Yi (ST-FIR/ENG1-Zhu), Pankaj Bansal,
	linux-can@vger.kernel.org, Varun Sethi, Poonam Aggrwal,
	Stefan Agner
In-Reply-To: <CALw8SCWK55Byqackx3YQ_h8c5Jo5JFLtt3-WJryHwOEtP-Rtgw@mail.gmail.com>


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

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

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 1/2] dt-binding: can: mcp2517fd: document device tree bindings
From: Rob Herring @ 2017-11-26 22:28 UTC (permalink / raw)
  To: kernel-TqfNSX0MhmxHKSADF0wUEw
  Cc: Wolfgang Grandegger, Marc Kleine-Budde, Mark Rutland,
	linux-can-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20171124183509.12810-2-kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>

On Fri, Nov 24, 2017 at 06:35:08PM +0000, kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org wrote:
> From: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
> 
> Add device-tree bindings for Microcip CanFD Controller mcp2517fd

s/Microcip/Microchip/

> 
> Signed-off-by: Martin Sperl <kernel-TqfNSX0MhmxHKSADF0wUEw@public.gmane.org>
> ---
>  .../bindings/net/can/microchip,mcp2517fd.txt       | 47 ++++++++++++++++++++++
>  1 file changed, 47 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/can/microchip,mcp2517fd.txt
> 
> diff --git a/Documentation/devicetree/bindings/net/can/microchip,mcp2517fd.txt b/Documentation/devicetree/bindings/net/can/microchip,mcp2517fd.txt
> new file mode 100644
> index 000000000000..96cbf0c96895
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/can/microchip,mcp2517fd.txt
> @@ -0,0 +1,47 @@
> +* Microchip MCP2517 stand-alone CAN controller device tree bindings
> +
> +Required properties:
> + - compatible: Should be one of the following:
> +   - "microchip,mcp2517fd" for MCP2517fd.
> + - reg: SPI chip select.
> + - clocks: The clock feeding the CAN controller.
> + - interrupt-parent: The parent interrupt controller.
> + - interrupts: Should contain IRQ line for the CAN controller.
> +
> +Optional properties:
> + - vdd-supply: Regulator that powers the CAN controller.
> + - xceiver-supply: Regulator that powers the CAN transceiver.
> + - microchip,clock_out_div = <0|1|2|4|10>: Clock output pin divider

s/_/-/

And on the rest. Don't use underscores...

> +					   0 = Start of Frame output
> +					   default: 10
> + - microchip,clock_div = <1|2>: internal clock divider - default 1
> + - microchip,gpio_opendrain: gpio (int0,1) in open drain mode
> +			     instead of default push/pull
> + - microchip,int_opendrain: int pin in open drain mode
> +			    instead of default push/pull

IIRC, we have a standard property for this.

> + - microchip,txcan_opendrain: txcan pin in open drain mode
> +			      instead of default push/pull
> + - microchip,gpio0_mode : gpio mode functionality
> +			  0 = input
> +			  1 = TX interrupt output - default
> +			  2 = output default low
> +			  3 = output default high
> +			  4 = (tx) transceiver standby
> + - microchip,gpio1_mode : gpio mode functionality
> +			  0 = input - default
> +			  1 = RX interrupt output - default
> +			  2 = output default low
> +			  3 = output default high
> +
> +Example:
> +	can0: can@1 {
> +		compatible = "microchip,mcp2515";
> +		reg = <1>;
> +		clocks = <&clk24m>;
> +		interrupt-parent = <&gpio4>;
> +		interrupts = <13 0x8>;
> +		vdd-supply = <&reg5v0>;
> +		xceiver-supply = <&reg5v0>;
> +		microchip,gpio0_mode = <4>;
> +		microchip,gpio0_mode = <1>;
> +	};
> -- 
> 2.11.0
> 
--
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


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