pull-request: can-next 2017-02-06

pull-request: can-next 2017-02-06

[Marc Kleine-Budde]

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

Hello David,

this is a pull request of 16 patches for net-next/master.

The first two patches by David Jander and me add the rx-offload
framework for CAN devices to the kernel. The remaining 14 patches
convert the flexcan driver to make use of it.

regards,
Marc

---

The following changes since commit bd092ad1463ca0990581fa992e12a9b0ed295d25:

  Merge branch 'remove-__napi_complete_done' (2017-02-05 16:11:59 -0500)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git tags/linux-can-next-for-4.11-20170206

for you to fetch changes up to 096de07f1d7156e500dec5abe89e9b44314e38e5:

  can: flexcan: switch imx6 and vf610 to timestamp based offloading (2017-02-06 15:13:45 +0100)

----------------------------------------------------------------
linux-can-next-for-4.11-20170206

----------------------------------------------------------------
David Jander (1):
      can: rx-offload: Add support for HW fifo based irq offloading

Marc Kleine-Budde (15):
      can: rx-offload: Add support for timestamp based irq offloading
      can: flexcan: add missing register definitions
      can: flexcan: remove write-only member pdata of struct flexcan_priv
      can: flexcan: make declaration of devtype_data const
      can: flexcan: do_bus_err(): convert rx_,tx_errors into bool
      can: flexcan: flexcan_poll_state(): no need to initialize new_state, rx_state, tx_state
      can: flexcan: flexcan_poll_bus_err(): fold in do_bus_err()
      can: flexcan: flexcan_irq(): don't unconditionally return IRQ_HANDLED
      can: flexcan: calculate default value for imask1 during runtime
      can: flexcan: make TX mailbox selectable during runtime
      can: flexcan: make use of rx-offload's irq_offload_fifo
      can: flexcan: activate individual RX masking and initialize reg_rximr
      can: flexcan: add quirk FLEXCAN_QUIRK_ENABLE_EACEN_RRS
      can: flexcan: add support for timestamp based rx-offload
      can: flexcan: switch imx6 and vf610 to timestamp based offloading

 drivers/net/can/Makefile       |   3 +-
 drivers/net/can/flexcan.c      | 419 ++++++++++++++++++++++++-----------------
 drivers/net/can/rx-offload.c   | 289 ++++++++++++++++++++++++++++
 include/linux/can/rx-offload.h |  59 ++++++
 4 files changed, 593 insertions(+), 177 deletions(-)
 create mode 100644 drivers/net/can/rx-offload.c
 create mode 100644 include/linux/can/rx-offload.h

-- 
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 --]

Re: pull-request: can-next 2017-02-06,pull-request: can-next 2017-02-06

[David Miller]

From: Marc Kleine-Budde <mkl@pengutronix.de>
Date: Mon, 6 Feb 2017 15:50:48 +0100

> this is a pull request of 16 patches for net-next/master.
> 
> The first two patches by David Jander and me add the rx-offload
> framework for CAN devices to the kernel. The remaining 14 patches
> convert the flexcan driver to make use of it.

Pulled, but I wonder if your comparisons does the right thing when the
counters overflow.

I think you need to do the same thing we do for TCP sequence number
comparisons and code it like:

static inline bool before(__u32 seq1, __u32 seq2)
{
        return (__s32)(seq1-seq2) < 0;
}

Re: pull-request: can-next 2017-02-06,pull-request: can-next 2017-02-06

[Marc Kleine-Budde]

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

On 02/06/2017 05:12 PM, David Miller wrote:
> From: Marc Kleine-Budde <mkl@pengutronix.de>
> Date: Mon, 6 Feb 2017 15:50:48 +0100
> 
>> this is a pull request of 16 patches for net-next/master.
>>
>> The first two patches by David Jander and me add the rx-offload
>> framework for CAN devices to the kernel. The remaining 14 patches
>> convert the flexcan driver to make use of it.
> 
> Pulled, but I wonder if your comparisons does the right thing when the
> counters overflow.
> 
> I think you need to do the same thing we do for TCP sequence number
> comparisons and code it like:
> 
> static inline bool before(__u32 seq1, __u32 seq2)
> {
>         return (__s32)(seq1-seq2) < 0;
> }

Yes, I think it's basically the same as the TCP sequence number code,
but obviously less readable.

> static int can_rx_offload_compare(struct sk_buff *a, struct sk_buff *b)
> {
> 	const struct can_rx_offload_cb *cb_a, *cb_b;
> 
> 	cb_a = can_rx_offload_get_cb(a);
> 	cb_b = can_rx_offload_get_cb(b);
> 
> 	/* Substract two u32 and return result as int, to keep
> 	 * difference steady around the u32 overflow.
> 	 */
> 	return cb_b->timestamp - cb_a->timestamp;
> }

This does the "(__s32)(seq1-seq2)"

> 	skb_queue_reverse_walk(head, pos) {
> 		const struct can_rx_offload_cb *cb_pos, *cb_new;
> 
> 		cb_pos = can_rx_offload_get_cb(pos);
> 		cb_new = can_rx_offload_get_cb(new);
> 
> 		netdev_dbg(new->dev,
> 			   "%s: pos=0x%08x, new=0x%08x, diff=%10d, queue_len=%d\n",
> 			   __func__,
> 			   cb_pos->timestamp, cb_new->timestamp,
> 			   cb_new->timestamp - cb_pos->timestamp,
> 			   skb_queue_len(head));
> 
> 		if (compare(pos, new) < 0)

And here the "return ... < 0;"

> 			continue;
> 		insert = pos;
> 		break;
> 	}

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 --]

Re: pull-request: can-next 2017-02-06,pull-request: can-next 2017-02-06,Re: pull-request: can-next 2017-02-06,pull-request: can-next 2017-02-06

[David Miller]

From: Marc Kleine-Budde <mkl@pengutronix.de>
Date: Mon, 6 Feb 2017 17:43:43 +0100

> On 02/06/2017 05:12 PM, David Miller wrote:
>> From: Marc Kleine-Budde <mkl@pengutronix.de>
>> Date: Mon, 6 Feb 2017 15:50:48 +0100
>> 
>>> this is a pull request of 16 patches for net-next/master.
>>>
>>> The first two patches by David Jander and me add the rx-offload
>>> framework for CAN devices to the kernel. The remaining 14 patches
>>> convert the flexcan driver to make use of it.
>> 
>> Pulled, but I wonder if your comparisons does the right thing when the
>> counters overflow.
>> 
>> I think you need to do the same thing we do for TCP sequence number
>> comparisons and code it like:
>> 
>> static inline bool before(__u32 seq1, __u32 seq2)
>> {
>>         return (__s32)(seq1-seq2) < 0;
>> }
> 
> Yes, I think it's basically the same as the TCP sequence number code,
> but obviously less readable.
> 
>> static int can_rx_offload_compare(struct sk_buff *a, struct sk_buff *b)
>> {
>> 	const struct can_rx_offload_cb *cb_a, *cb_b;
>> 
>> 	cb_a = can_rx_offload_get_cb(a);
>> 	cb_b = can_rx_offload_get_cb(b);
>> 
>> 	/* Substract two u32 and return result as int, to keep
>> 	 * difference steady around the u32 overflow.
>> 	 */
>> 	return cb_b->timestamp - cb_a->timestamp;
>> }
> 
> This does the "(__s32)(seq1-seq2)"
 ...
> 
> And here the "return ... < 0;"

Sure but what about these "can_rx_offload_le()" comparisons?

Re: pull-request: can-next 2017-02-06,pull-request: can-next 2017-02-06,Re: pull-request: can-next 2017-02-06,pull-request: can-next 2017-02-06

[Marc Kleine-Budde]

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

On 02/06/2017 05:59 PM, David Miller wrote:
> Sure but what about these "can_rx_offload_le()" comparisons?

This is to scan the mailboxes according to priority. On some CAN IP
cores the mailboxes decrease in priority (0=high, 63=low) some increase
(63=high, 0=low). The can_rx_offload_le() (le means less or equal) and
can_rx_offload_inc() functions are used to abstract the order. We always
only scan from highest to lower prio (offload->mb_first ...
offload->mb_last):

> 	for (i = offload->mb_first;
> 	     can_rx_offload_le(offload, i, offload->mb_last);
> 	     can_rx_offload_inc(offload, &i)) {

So we never wrap around.

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 --]