Linux CAN drivers development
 help / color / mirror / Atom feed
* Mailing list archive is down?
From: Rémy DZIEMIASZKO @ 2017-12-11 16:10 UTC (permalink / raw)
  To: linux-can

Hello,

I am trying to access the linux-can mailing list archive at 
http://dir.gmane.org/gmane.linux.can

But none of the sub-links seems to work...

- http://news.gmane.org/gmane.linux.can
- http://blog.gmane.org/gmane.linux.can

Is the server down or I am wrong?

Cheers,

Rémy Dziemiaszko

PS: Sorry, I just subscribed to the mailing list today so maybe this 
issue is already known?

^ permalink raw reply

* Re: [PATCH] vxcan: improve handling of missing peer name attribute
From: Marc Kleine-Budde @ 2017-12-11 13:45 UTC (permalink / raw)
  To: Oliver Hartkopp, linux-can; +Cc: Serhey Popovych
In-Reply-To: <20171202174852.9618-1-socketcan@hartkopp.net>


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

On 12/02/2017 06:48 PM, Oliver Hartkopp wrote:
> 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>

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: Can: FD: configure transmitter delay
From: Martin Sperl @ 2017-12-09 18:48 UTC (permalink / raw)
  To: Oliver Hartkopp; +Cc: linux-can
In-Reply-To: <eb6253a6-706b-db3b-9701-3eef2e408d9c@hartkopp.net>


As far as I can tell dsample-point is the definition of the first
sample point of a data bit (in % of the bit length). It would not 
define the 2nd sample point that would be defined by 
transmitter delay.

Also how would one parametrize: off, fixed or auto with this
parameter(where with auto part of the parameter would be
Automatic, but parts would be fixed?

I understand we need to come to a common denominator.

So maybe it would help summarize the parameters for each
Device to figure out what is common.

Here the info for the mcp2517fd:

bit 17-16 TDCMOD<1:0>: Transmitter Delay Compensation Mode bits; Secondary Sample Point (SSP) 10-11 = Auto; measure delay and add TDCO. 01 = Manual; Don't measure, use TDCV + TDCO from register 00 = TDC Disabled 
bit 14-8 TDCO<6:0>: Transmitter Delay Compensation Offset bits; Secondary Sample Point (SSP) Two's complement; offset can be positive, zero, or negative. 011 1111 =  63 x TSYSCLK ... 000 0000 =  0 x TSYSCLK ... 111 1111 =  –64 x TSYSCLK 
bit 5-0 TDCV<5:0>: Transmitter Delay Compensation Value bits; Secondary Sample Point (SSP) 11 1111 =  63 x TSYSCLK ... 00 0000 =  0 x TSYSCLK

What the microchip reference implementation does is define auto
 and setting vdco while keeping vdcv=0

My initial implementation did set tdc to disabled, but then the 
communication with (for example) PCAN-usb is failing -  have 
this information from someone who has access to this (which 
makes it hard for me to reproduce...)

Martin

> On 09.12.2017, at 19:01, Oliver Hartkopp <socketcan@hartkopp.net> wrote:
> 
> Hello Martin,
> 
> we had a discussion on this topic recently which ended here:
> 
> https://marc.info/?l=linux-can&m=150850171310828&w=2
> 
> There was no further discussion since then.
> 
> What is you opinion about using the 'data sample-point' aka dsample-point for transmitter delay configuration?
> 
>   $ ip link set can0 type can help
>    Usage: ip link set DEVICE type can
>        [ bitrate BITRATE [ sample-point SAMPLE-POINT] ] |
>        [ tq TQ prop-seg PROP_SEG phase-seg1 PHASE-SEG1
>          phase-seg2 PHASE-SEG2 [ sjw SJW ] ]
> 
>        [ dbitrate BITRATE [ dsample-point SAMPLE-POINT] ] | <<---
>        [ dtq TQ dprop-seg PROP_SEG dphase-seg1 PHASE-SEG1
>          dphase-seg2 PHASE-SEG2 [ dsjw SJW ] ]
> 
> Best regards,
> Oliver
> 
>> On 12/09/2017 06:01 PM, Martin Sperl wrote:
>> Hi!
>> The mcp2517fd that I am finishing the driver for has some specific
>> settings to configure the transmitter delay which is used with can fd
>> frames (essentially it configures 2nd sample point that takes the
>> delay /latency of the transceiver into account -  can be off, fixed, or
>> automatic).
>> Compatibility tests have already shown that some can fd communication
>> is quite sensitive to these settings - especially between different types
>> of devices - which may result in BUS off situations...
>> So I am wondering how to best allow for configuration of these
>> parameters - module parameters is ugly similarly debugfs or sysfs.
>> I guess there must be other fd devices with similar requirements,
>> so I wonder how it is handled there...
>> Thanks,
>>              Martin
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-can" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html


^ permalink raw reply

* Re: Can: FD: configure transmitter delay
From: Oliver Hartkopp @ 2017-12-09 18:01 UTC (permalink / raw)
  To: Martin Sperl; +Cc: linux-can
In-Reply-To: <B4DF4571-1790-4F09-8DEC-C0812D0F34B1@martin.sperl.org>

Hello Martin,

we had a discussion on this topic recently which ended here:

https://marc.info/?l=linux-can&m=150850171310828&w=2

There was no further discussion since then.

What is you opinion about using the 'data sample-point' aka 
dsample-point for transmitter delay configuration?

    $ ip link set can0 type can help
     Usage: ip link set DEVICE type can
         [ bitrate BITRATE [ sample-point SAMPLE-POINT] ] |
         [ tq TQ prop-seg PROP_SEG phase-seg1 PHASE-SEG1
           phase-seg2 PHASE-SEG2 [ sjw SJW ] ]

         [ dbitrate BITRATE [ dsample-point SAMPLE-POINT] ] | <<---
         [ dtq TQ dprop-seg PROP_SEG dphase-seg1 PHASE-SEG1
           dphase-seg2 PHASE-SEG2 [ dsjw SJW ] ]

Best regards,
Oliver

On 12/09/2017 06:01 PM, Martin Sperl wrote:
> Hi!
> 
> The mcp2517fd that I am finishing the driver for has some specific
> settings to configure the transmitter delay which is used with can fd
> frames (essentially it configures 2nd sample point that takes the
> delay /latency of the transceiver into account -  can be off, fixed, or
> automatic).
> 
> Compatibility tests have already shown that some can fd communication
> is quite sensitive to these settings - especially between different types
> of devices - which may result in BUS off situations...
> 
> So I am wondering how to best allow for configuration of these
> parameters - module parameters is ugly similarly debugfs or sysfs.
> 
> I guess there must be other fd devices with similar requirements,
> so I wonder how it is handled there...
> 
> Thanks,
>               Martin
> --
> To unsubscribe from this list: send the line "unsubscribe linux-can" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply

* Re: [PATCH] slip: sl_alloc(): remove unused parameter "dev_t line"
From: Oliver Hartkopp @ 2017-12-09 17:28 UTC (permalink / raw)
  To: Marc Kleine-Budde, netdev; +Cc: kernel, linux-can
In-Reply-To: <39d83463-b20d-3739-7e84-f1f51919e9e5@pengutronix.de>



On 12/08/2017 12:22 PM, Marc Kleine-Budde wrote:
> Hello Oliver,
> 
> I've the corresponding slcan patch already in my queue.

Excellent :-)

Thanks,
Oliver

> 
> Marc
> 
> On 12/08/2017 12:18 PM, Marc Kleine-Budde wrote:
>> The first and only parameter of sl_alloc() is unused, so remove it.
>>
>> Fixes: 5342b77c4123 slip: ("Clean up create and destroy")
>> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
>> ---
>>   drivers/net/slip/slip.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/slip/slip.c b/drivers/net/slip/slip.c
>> index cc63102ca96e..8940417c30e5 100644
>> --- a/drivers/net/slip/slip.c
>> +++ b/drivers/net/slip/slip.c
>> @@ -731,7 +731,7 @@ static void sl_sync(void)
>>   
>>   
>>   /* Find a free SLIP channel, and link in this `tty' line. */
>> -static struct slip *sl_alloc(dev_t line)
>> +static struct slip *sl_alloc(void)
>>   {
>>   	int i;
>>   	char name[IFNAMSIZ];
>> @@ -809,7 +809,7 @@ static int slip_open(struct tty_struct *tty)
>>   
>>   	/* OK.  Find a free SLIP channel to use. */
>>   	err = -ENFILE;
>> -	sl = sl_alloc(tty_devnum(tty));
>> +	sl = sl_alloc();
>>   	if (sl == NULL)
>>   		goto err_exit;
>>   
>>
> 
> 

^ permalink raw reply

* Can: FD: configure transmitter delay
From: Martin Sperl @ 2017-12-09 17:01 UTC (permalink / raw)
  To: linux-can

Hi!

The mcp2517fd that I am finishing the driver for has some specific
settings to configure the transmitter delay which is used with can fd 
frames (essentially it configures 2nd sample point that takes the
delay /latency of the transceiver into account -  can be off, fixed, or 
automatic).

Compatibility tests have already shown that some can fd communication
is quite sensitive to these settings - especially between different types
of devices - which may result in BUS off situations...

So I am wondering how to best allow for configuration of these
parameters - module parameters is ugly similarly debugfs or sysfs.

I guess there must be other fd devices with similar requirements,
so I wonder how it is handled there...

Thanks,
             Martin

^ permalink raw reply

* Re: pull-request: can 2017-12-08
From: David Miller @ 2017-12-08 19:54 UTC (permalink / raw)
  To: mkl; +Cc: netdev, linux-can, kernel
In-Reply-To: <20171208155817.9364-1-mkl@pengutronix.de>

From: Marc Kleine-Budde <mkl@pengutronix.de>
Date: Fri,  8 Dec 2017 16:58:11 +0100

> this is a pull request of 6 patches for net/master.
> 
> Martin Kelly provides 5 patches for various USB based CAN drivers, that
> properly cancel the URBs on adapter unplug, so that the driver doesn't
> end up in an endless loop. Stephane Grosjean provides a patch to restart
> the tx queue if zero length packages are transmitted.

Pulled, thanks Marc.

^ permalink raw reply

* [PATCH 2/6] can: ems_usb: cancel urb on -EPIPE and -EPROTO
From: Marc Kleine-Budde @ 2017-12-08 15:58 UTC (permalink / raw)
  To: netdev
  Cc: davem, linux-can, kernel, Martin Kelly, linux-stable,
	Marc Kleine-Budde
In-Reply-To: <20171208155817.9364-1-mkl@pengutronix.de>

From: Martin Kelly <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>
Cc: linux-stable <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 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.15.0


^ permalink raw reply related

* [PATCH 6/6] can: peak/pcie_fd: fix potential bug in restarting tx queue
From: Marc Kleine-Budde @ 2017-12-08 15:58 UTC (permalink / raw)
  To: netdev
  Cc: davem, linux-can, kernel, Stephane Grosjean, linux-stable,
	Marc Kleine-Budde
In-Reply-To: <20171208155817.9364-1-mkl@pengutronix.de>

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

Don't rely on can_get_echo_skb() return value to wake the network tx
queue up: can_get_echo_skb() returns 0 if the echo array slot was not
occupied, but also when the DLC of the released echo frame was 0.

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_canfd.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/net/can/peak_canfd/peak_canfd.c b/drivers/net/can/peak_canfd/peak_canfd.c
index 85268be0c913..55513411a82e 100644
--- a/drivers/net/can/peak_canfd/peak_canfd.c
+++ b/drivers/net/can/peak_canfd/peak_canfd.c
@@ -258,21 +258,18 @@ static int pucan_handle_can_rx(struct peak_canfd_priv *priv,
 	/* if this frame is an echo, */
 	if ((rx_msg_flags & PUCAN_MSG_LOOPED_BACK) &&
 	    !(rx_msg_flags & PUCAN_MSG_SELF_RECEIVE)) {
-		int n;
 		unsigned long flags;
 
 		spin_lock_irqsave(&priv->echo_lock, flags);
-		n = can_get_echo_skb(priv->ndev, msg->client);
+		can_get_echo_skb(priv->ndev, msg->client);
 		spin_unlock_irqrestore(&priv->echo_lock, flags);
 
 		/* count bytes of the echo instead of skb */
 		stats->tx_bytes += cf_len;
 		stats->tx_packets++;
 
-		if (n) {
-			/* restart tx queue only if a slot is free */
-			netif_wake_queue(priv->ndev);
-		}
+		/* restart tx queue (a slot is free) */
+		netif_wake_queue(priv->ndev);
 
 		return 0;
 	}
-- 
2.15.0

^ permalink raw reply related

* [PATCH 5/6] can: usb_8dev: cancel urb on -EPIPE and -EPROTO
From: Marc Kleine-Budde @ 2017-12-08 15:58 UTC (permalink / raw)
  To: netdev
  Cc: davem, linux-can, kernel, Martin Kelly, linux-stable,
	Marc Kleine-Budde
In-Reply-To: <20171208155817.9364-1-mkl@pengutronix.de>

From: Martin Kelly <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>
Cc: linux-stable <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/usb_8dev.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/can/usb/usb_8dev.c b/drivers/net/can/usb/usb_8dev.c
index d000cb62d6ae..27861c417c94 100644
--- a/drivers/net/can/usb/usb_8dev.c
+++ b/drivers/net/can/usb/usb_8dev.c
@@ -524,6 +524,8 @@ static void usb_8dev_read_bulk_callback(struct urb *urb)
 		break;
 
 	case -ENOENT:
+	case -EPIPE:
+	case -EPROTO:
 	case -ESHUTDOWN:
 		return;
 
-- 
2.15.0

^ permalink raw reply related

* [PATCH 4/6] can: kvaser_usb: cancel urb on -EPIPE and -EPROTO
From: Marc Kleine-Budde @ 2017-12-08 15:58 UTC (permalink / raw)
  To: netdev
  Cc: davem, linux-can, kernel, Martin Kelly, linux-stable,
	Marc Kleine-Budde
In-Reply-To: <20171208155817.9364-1-mkl@pengutronix.de>

From: Martin Kelly <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>
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 f95945915d20..63587b8e6825 100644
--- a/drivers/net/can/usb/kvaser_usb.c
+++ b/drivers/net/can/usb/kvaser_usb.c
@@ -1326,6 +1326,8 @@ static void kvaser_usb_read_bulk_callback(struct urb *urb)
 	case 0:
 		break;
 	case -ENOENT:
+	case -EPIPE:
+	case -EPROTO:
 	case -ESHUTDOWN:
 		return;
 	default:
-- 
2.15.0

^ permalink raw reply related

* [PATCH 3/6] can: esd_usb2: cancel urb on -EPIPE and -EPROTO
From: Marc Kleine-Budde @ 2017-12-08 15:58 UTC (permalink / raw)
  To: netdev
  Cc: davem, linux-can, kernel, Martin Kelly, linux-stable,
	Marc Kleine-Budde
In-Reply-To: <20171208155817.9364-1-mkl@pengutronix.de>

From: Martin Kelly <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>
Cc: linux-stable <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 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.15.0

^ permalink raw reply related

* [PATCH 1/6] can: mcba_usb: cancel urb on -EPROTO
From: Marc Kleine-Budde @ 2017-12-08 15:58 UTC (permalink / raw)
  To: netdev
  Cc: davem, linux-can, kernel, Martin Kelly, linux-stable,
	Marc Kleine-Budde
In-Reply-To: <20171208155817.9364-1-mkl@pengutronix.de>

From: Martin Kelly <mkelly@xevo.com>

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>
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 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.15.0

^ permalink raw reply related

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

Hello David,

this is a pull request of 6 patches for net/master.

Martin Kelly provides 5 patches for various USB based CAN drivers, that
properly cancel the URBs on adapter unplug, so that the driver doesn't
end up in an endless loop. Stephane Grosjean provides a patch to restart
the tx queue if zero length packages are transmitted.

regards,
Marc

---

The following changes since commit 195bd525d5f6e338b948d9a6b25bfaae86291353:

  Merge git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf (2017-12-07 16:22:51 -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-20171208

for you to fetch changes up to 91785de6f94b58c3fb6664609e3682f011bd28d2:

  can: peak/pcie_fd: fix potential bug in restarting tx queue (2017-12-08 10:51:53 +0100)

----------------------------------------------------------------
linux-can-fixes-for-4.15-20171208

----------------------------------------------------------------
Martin Kelly (5):
      can: mcba_usb: cancel urb on -EPROTO
      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

Stephane Grosjean (1):
      can: peak/pcie_fd: fix potential bug in restarting tx queue

 drivers/net/can/peak_canfd/peak_canfd.c | 9 +++------
 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/mcba_usb.c          | 1 +
 drivers/net/can/usb/usb_8dev.c          | 2 ++
 6 files changed, 12 insertions(+), 6 deletions(-)

^ permalink raw reply

* Re: [PATCH] slip: sl_alloc(): remove unused parameter "dev_t line"
From: Marc Kleine-Budde @ 2017-12-08 11:22 UTC (permalink / raw)
  To: netdev; +Cc: kernel, linux-can, Oliver Hartkopp
In-Reply-To: <20171208111859.6090-1-mkl@pengutronix.de>


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

Hello Oliver,

I've the corresponding slcan patch already in my queue.

Marc

On 12/08/2017 12:18 PM, Marc Kleine-Budde wrote:
> The first and only parameter of sl_alloc() is unused, so remove it.
> 
> Fixes: 5342b77c4123 slip: ("Clean up create and destroy")
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> ---
>  drivers/net/slip/slip.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/slip/slip.c b/drivers/net/slip/slip.c
> index cc63102ca96e..8940417c30e5 100644
> --- a/drivers/net/slip/slip.c
> +++ b/drivers/net/slip/slip.c
> @@ -731,7 +731,7 @@ static void sl_sync(void)
>  
>  
>  /* Find a free SLIP channel, and link in this `tty' line. */
> -static struct slip *sl_alloc(dev_t line)
> +static struct slip *sl_alloc(void)
>  {
>  	int i;
>  	char name[IFNAMSIZ];
> @@ -809,7 +809,7 @@ static int slip_open(struct tty_struct *tty)
>  
>  	/* OK.  Find a free SLIP channel to use. */
>  	err = -ENFILE;
> -	sl = sl_alloc(tty_devnum(tty));
> +	sl = sl_alloc();
>  	if (sl == NULL)
>  		goto err_exit;
>  
> 


-- 
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: peak-usb: peak_usb_netif_rx() ts_high ununsed
From: Marc Kleine-Budde @ 2017-12-08  9:59 UTC (permalink / raw)
  To: Stéphane Grosjean, linux-can
In-Reply-To: <VI1PR0302MB33255A4E1224FB34C7B70572D6320@VI1PR0302MB3325.eurprd03.prod.outlook.com>


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

On 12/06/2017 04:04 PM, Stéphane Grosjean wrote:
> Yep, you're right: peak_usb_get_ts_time() handles 32-bit wrapping
> when computing delay between two consecutives events, so the ts_high
> argument is useless.

ts_high is never used in the driver now.

> Consequently, each call to "peak_usb_netif_rx(a, b, c, d);" in
> pcan_usb_fd.c can be changed into "peak_usb_netif_rx(a, b, c);"
ACK.

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/pcie_fd: fix potential bug in restarting tx queue
From: Marc Kleine-Budde @ 2017-12-08  9:46 UTC (permalink / raw)
  To: Stephane Grosjean, Oliver Hartkopp; +Cc: linux-can Mailing List
In-Reply-To: <20171207151343.26923-1-s.grosjean@peak-system.com>


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

On 12/07/2017 04:13 PM, Stephane Grosjean wrote:
> Don't rely on can_get_echo_skb() return value to wake the network tx
> queue up: can_get_echo_skb() returns 0 if the echo array slot was not
> occupied, but also when the DLC of the released echo frame was 0.
> 
> 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

* [PATCH] can: peak/pcie_fd: fix potential bug in restarting tx queue
From: Stephane Grosjean @ 2017-12-07 15:13 UTC (permalink / raw)
  To: Oliver Hartkopp; +Cc: linux-can Mailing List, Stephane Grosjean

Don't rely on can_get_echo_skb() return value to wake the network tx
queue up: can_get_echo_skb() returns 0 if the echo array slot was not
occupied, but also when the DLC of the released echo frame was 0.

Signed-off-by: Stephane Grosjean <s.grosjean@peak-system.com>
---
 drivers/net/can/peak_canfd/peak_canfd.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/net/can/peak_canfd/peak_canfd.c b/drivers/net/can/peak_canfd/peak_canfd.c
index 85268be0c913..55513411a82e 100644
--- a/drivers/net/can/peak_canfd/peak_canfd.c
+++ b/drivers/net/can/peak_canfd/peak_canfd.c
@@ -258,21 +258,18 @@ static int pucan_handle_can_rx(struct peak_canfd_priv *priv,
 	/* if this frame is an echo, */
 	if ((rx_msg_flags & PUCAN_MSG_LOOPED_BACK) &&
 	    !(rx_msg_flags & PUCAN_MSG_SELF_RECEIVE)) {
-		int n;
 		unsigned long flags;
 
 		spin_lock_irqsave(&priv->echo_lock, flags);
-		n = can_get_echo_skb(priv->ndev, msg->client);
+		can_get_echo_skb(priv->ndev, msg->client);
 		spin_unlock_irqrestore(&priv->echo_lock, flags);
 
 		/* count bytes of the echo instead of skb */
 		stats->tx_bytes += cf_len;
 		stats->tx_packets++;
 
-		if (n) {
-			/* restart tx queue only if a slot is free */
-			netif_wake_queue(priv->ndev);
-		}
+		/* restart tx queue (a slot is free) */
+		netif_wake_queue(priv->ndev);
 
 		return 0;
 	}
-- 
2.14.1


^ permalink raw reply related

* RE: peak-usb: peak_usb_netif_rx() ts_high ununsed
From: Stéphane Grosjean @ 2017-12-06 15:04 UTC (permalink / raw)
  To: Marc Kleine-Budde, linux-can
In-Reply-To: <ee2e4702-5d2d-b6fc-a90c-5fc9b0d3f362@pengutronix.de>

Hello,

Yep, you're right: peak_usb_get_ts_time() handles 32-bit wrapping when computing delay between two consecutives events, so the ts_high argument is useless.

Consequently, each call to "peak_usb_netif_rx(a, b, c, d);" in pcan_usb_fd.c can be changed into "peak_usb_netif_rx(a, b, c);"

Stéphane.

-----Original Message-----
From: Marc Kleine-Budde [mailto:mkl@pengutronix.de]
Sent: mercredi 6 décembre 2017 15:09
To: linux-can <linux-can@vger.kernel.org>; Stéphane Grosjean <s.grosjean@peak-system.com>
Subject: peak-usb: peak_usb_netif_rx() ts_high ununsed

Hello,

we just found out that ts_high is never used:

> /*
>  * post received skb after having set any hw timestamp  */ int
> peak_usb_netif_rx(struct sk_buff *skb,
>       struct peak_time_ref *time_ref, u32 ts_low, u32 ts_high) {
> struct skb_shared_hwtstamps *hwts = skb_hwtstamps(skb);
>
> peak_usb_get_ts_time(time_ref, ts_low, &hwts->hwtstamp);
>
> return netif_rx(skb);
> }

Is that expected? If yes, let's remove ts_high.

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   |


--
PEAK-System Technik GmbH
Sitz der Gesellschaft Darmstadt - HRB 9183
Geschaeftsfuehrung: Alexander Gach / Uwe Wilhelm
--

^ permalink raw reply

* [mkl-can-next:af_can 5/18] net/can/af_can.c:890:25: error: 'struct netns_can' has no member named 'can_stattimer'
From: kbuild test robot @ 2017-12-06 14:32 UTC (permalink / raw)
  To: Marc Kleine-Budde; +Cc: kbuild-all, linux-can

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

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git af_can
head:   f0c2db14e47365b89dd8e76a365f0f9d5a48a914
commit: 50f0b88d236c1404945f5707dd6eceb8564f0290 [5/18] can: netns: remove "can_" prefix from members struct netns_can
config: i386-randconfig-a0-201749 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
        git checkout 50f0b88d236c1404945f5707dd6eceb8564f0290
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   In file included from include/linux/workqueue.h:9:0,
                    from include/linux/srcu.h:34,
                    from include/linux/notifier.h:16,
                    from include/linux/memory_hotplug.h:7,
                    from include/linux/mmzone.h:775,
                    from include/linux/gfp.h:6,
                    from include/linux/umh.h:4,
                    from include/linux/kmod.h:22,
                    from include/linux/module.h:13,
                    from net/can/af_can.c:43:
   net/can/af_can.c: In function 'can_pernet_init':
>> net/can/af_can.c:890:25: error: 'struct netns_can' has no member named 'can_stattimer'
       timer_setup(&net->can.can_stattimer, can_stat_update,
                            ^
   include/linux/timer.h:105:19: note: in definition of macro '__init_timer'
      init_timer_key((_timer), (_fn), (_flags), #_timer, &__key);\
                      ^
   net/can/af_can.c:890:4: note: in expansion of macro 'timer_setup'
       timer_setup(&net->can.can_stattimer, can_stat_update,
       ^
   net/can/af_can.c:892:23: error: 'struct netns_can' has no member named 'can_stattimer'
       mod_timer(&net->can.can_stattimer,
                          ^
--
   In file included from include/linux/kernel.h:10:0,
                    from include/linux/list.h:9,
                    from include/linux/module.h:9,
                    from net/can/proc.c:42:
   net/can/proc.c: In function 'can_stat_update':
>> net/can/proc.c:120:42: error: 'struct netns_can' has no member named 'can_stattimer'
     struct net *net = from_timer(net, t, can.can_stattimer);
                                             ^
   include/linux/compiler.h:301:19: note: in definition of macro '__compiletime_assert'
      bool __cond = !(condition);    \
                      ^
   include/linux/compiler.h:324:2: note: in expansion of macro '_compiletime_assert'
     _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
     ^
   include/linux/build_bug.h:47:37: note: in expansion of macro 'compiletime_assert'
    #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
                                        ^
   include/linux/kernel.h:930:2: note: in expansion of macro 'BUILD_BUG_ON_MSG'
     BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
     ^
   include/linux/kernel.h:930:20: note: in expansion of macro '__same_type'
     BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
                       ^
   include/linux/timer.h:144:2: note: in expansion of macro 'container_of'
     container_of(callback_timer, typeof(*var), timer_fieldname)
     ^
   net/can/proc.c:120:20: note: in expansion of macro 'from_timer'
     struct net *net = from_timer(net, t, can.can_stattimer);
                       ^
   In file included from include/linux/compiler_types.h:58:0,
                    from include/uapi/linux/stddef.h:2,
                    from include/linux/stddef.h:5,
                    from include/uapi/linux/posix_types.h:5,
                    from include/uapi/linux/types.h:14,
                    from include/linux/types.h:6,
                    from include/linux/list.h:5,
                    from include/linux/module.h:9,
                    from net/can/proc.c:42:
   include/linux/compiler-gcc.h:166:2: error: 'struct netns_can' has no member named 'can_stattimer'
     __builtin_offsetof(a, b)
     ^
   include/linux/stddef.h:17:32: note: in expansion of macro '__compiler_offsetof'
    #define offsetof(TYPE, MEMBER) __compiler_offsetof(TYPE, MEMBER)
                                   ^
   include/linux/kernel.h:933:21: note: in expansion of macro 'offsetof'
     ((type *)(__mptr - offsetof(type, member))); })
                        ^
   include/linux/timer.h:144:2: note: in expansion of macro 'container_of'
     container_of(callback_timer, typeof(*var), timer_fieldname)
     ^
   net/can/proc.c:120:20: note: in expansion of macro 'from_timer'
     struct net *net = from_timer(net, t, can.can_stattimer);
                       ^
   net/can/proc.c: In function 'can_stats_proc_show':
   net/can/proc.c:224:14: error: 'struct netns_can' has no member named 'can_stattimer'
     if (net->can.can_stattimer.function == can_stat_update) {
                 ^
   net/can/proc.c: In function 'can_reset_stats_proc_show':
   net/can/proc.c:294:14: error: 'struct netns_can' has no member named 'can_stattimer'
     if (net->can.can_stattimer.function == can_stat_update) {
                 ^

vim +890 net/can/af_can.c

0d66548a Oliver Hartkopp   2007-11-16  872  
8e8cda6d Mario Kicherer    2017-02-21  873  static int can_pernet_init(struct net *net)
8e8cda6d Mario Kicherer    2017-02-21  874  {
50f0b88d Marc Kleine-Budde 2017-08-28  875  	spin_lock_init(&net->can.rcvlists_lock);
50f0b88d Marc Kleine-Budde 2017-08-28  876  	net->can.rx_alldev_list =
8e8cda6d Mario Kicherer    2017-02-21  877  		kzalloc(sizeof(struct dev_rcv_lists), GFP_KERNEL);
50f0b88d Marc Kleine-Budde 2017-08-28  878  	if (!net->can.rx_alldev_list)
5a606223 Marc Kleine-Budde 2017-07-29  879  		goto out;
54c8b3c2 Marc Kleine-Budde 2017-08-28  880  	net->can.pkg_stats = kzalloc(sizeof(struct can_pkg_stats), GFP_KERNEL);
54c8b3c2 Marc Kleine-Budde 2017-08-28  881  	if (!net->can.pkg_stats)
5a606223 Marc Kleine-Budde 2017-07-29  882  		goto out_free_alldev_list;
54c8b3c2 Marc Kleine-Budde 2017-08-28  883  	net->can.rcv_lists_stats = kzalloc(sizeof(struct can_rcv_lists_stats), GFP_KERNEL);
54c8b3c2 Marc Kleine-Budde 2017-08-28  884  	if (!net->can.rcv_lists_stats)
5317dd6d Marc Kleine-Budde 2017-08-28  885  		goto out_free_pkg_stats;
cb5635a3 Oliver Hartkopp   2017-04-25  886  
cb5635a3 Oliver Hartkopp   2017-04-25  887  	if (IS_ENABLED(CONFIG_PROC_FS)) {
cb5635a3 Oliver Hartkopp   2017-04-25  888  		/* the statistics are updated every second (timer triggered) */
cb5635a3 Oliver Hartkopp   2017-04-25  889  		if (stats_timer) {
1fccb565 Kees Cook         2017-10-16 @890  			timer_setup(&net->can.can_stattimer, can_stat_update,
1fccb565 Kees Cook         2017-10-16  891  				    0);
cb5635a3 Oliver Hartkopp   2017-04-25  892  			mod_timer(&net->can.can_stattimer,
cb5635a3 Oliver Hartkopp   2017-04-25  893  				  round_jiffies(jiffies + HZ));
cb5635a3 Oliver Hartkopp   2017-04-25  894  		}
54c8b3c2 Marc Kleine-Budde 2017-08-28  895  		net->can.pkg_stats->jiffies_init = jiffies;
8e8cda6d Mario Kicherer    2017-02-21  896  		can_init_proc(net);
cb5635a3 Oliver Hartkopp   2017-04-25  897  	}
8e8cda6d Mario Kicherer    2017-02-21  898  
8e8cda6d Mario Kicherer    2017-02-21  899  	return 0;
5a606223 Marc Kleine-Budde 2017-07-29  900  
5317dd6d Marc Kleine-Budde 2017-08-28  901   out_free_pkg_stats:
54c8b3c2 Marc Kleine-Budde 2017-08-28  902  	kfree(net->can.pkg_stats);
5a606223 Marc Kleine-Budde 2017-07-29  903   out_free_alldev_list:
50f0b88d Marc Kleine-Budde 2017-08-28  904  	kfree(net->can.rx_alldev_list);
5a606223 Marc Kleine-Budde 2017-07-29  905   out:
5a606223 Marc Kleine-Budde 2017-07-29  906  	return -ENOMEM;
8e8cda6d Mario Kicherer    2017-02-21  907  }
8e8cda6d Mario Kicherer    2017-02-21  908  

:::::: The code at line 890 was first introduced by commit
:::::: 1fccb565e8b09e54467d41111f6faf08fcc9c3c1 net: can: Convert timers to use timer_setup()

:::::: TO: Kees Cook <keescook@chromium.org>
:::::: CC: David S. Miller <davem@davemloft.net>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 29240 bytes --]

^ permalink raw reply

* [mkl-can-next:af_can 5/18] include/linux/compiler-gcc.h:166:2: error: 'struct netns_can' has no member named 'can_stattimer'
From: kbuild test robot @ 2017-12-06 14:29 UTC (permalink / raw)
  To: Marc Kleine-Budde; +Cc: kbuild-all, linux-can

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

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git af_can
head:   f0c2db14e47365b89dd8e76a365f0f9d5a48a914
commit: 50f0b88d236c1404945f5707dd6eceb8564f0290 [5/18] can: netns: remove "can_" prefix from members struct netns_can
config: i386-randconfig-sb0-12061831 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
        git checkout 50f0b88d236c1404945f5707dd6eceb8564f0290
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   In file included from include/linux/kernel.h:10:0,
                    from include/linux/list.h:9,
                    from include/linux/module.h:9,
                    from net/can/proc.c:42:
   net/can/proc.c: In function 'can_stat_update':
   net/can/proc.c:120:42: error: 'struct netns_can' has no member named 'can_stattimer'
     struct net *net = from_timer(net, t, can.can_stattimer);
                                             ^
   include/linux/compiler.h:301:19: note: in definition of macro '__compiletime_assert'
      bool __cond = !(condition);    \
                      ^
   include/linux/compiler.h:324:2: note: in expansion of macro '_compiletime_assert'
     _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
     ^
   include/linux/build_bug.h:47:37: note: in expansion of macro 'compiletime_assert'
    #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
                                        ^
   include/linux/kernel.h:930:2: note: in expansion of macro 'BUILD_BUG_ON_MSG'
     BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
     ^
   include/linux/kernel.h:930:20: note: in expansion of macro '__same_type'
     BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
                       ^
   include/linux/timer.h:144:2: note: in expansion of macro 'container_of'
     container_of(callback_timer, typeof(*var), timer_fieldname)
     ^
   net/can/proc.c:120:20: note: in expansion of macro 'from_timer'
     struct net *net = from_timer(net, t, can.can_stattimer);
                       ^
   In file included from include/linux/compiler_types.h:58:0,
                    from include/uapi/linux/stddef.h:2,
                    from include/linux/stddef.h:5,
                    from include/uapi/linux/posix_types.h:5,
                    from include/uapi/linux/types.h:14,
                    from include/linux/types.h:6,
                    from include/linux/list.h:5,
                    from include/linux/module.h:9,
                    from net/can/proc.c:42:
>> include/linux/compiler-gcc.h:166:2: error: 'struct netns_can' has no member named 'can_stattimer'
     __builtin_offsetof(a, b)
     ^
   include/linux/stddef.h:17:32: note: in expansion of macro '__compiler_offsetof'
    #define offsetof(TYPE, MEMBER) __compiler_offsetof(TYPE, MEMBER)
                                   ^
   include/linux/kernel.h:933:21: note: in expansion of macro 'offsetof'
     ((type *)(__mptr - offsetof(type, member))); })
                        ^
   include/linux/timer.h:144:2: note: in expansion of macro 'container_of'
     container_of(callback_timer, typeof(*var), timer_fieldname)
     ^
   net/can/proc.c:120:20: note: in expansion of macro 'from_timer'
     struct net *net = from_timer(net, t, can.can_stattimer);
                       ^
   net/can/proc.c: In function 'can_stats_proc_show':
   net/can/proc.c:224:14: error: 'struct netns_can' has no member named 'can_stattimer'
     if (net->can.can_stattimer.function == can_stat_update) {
                 ^
   net/can/proc.c: In function 'can_reset_stats_proc_show':
   net/can/proc.c:294:14: error: 'struct netns_can' has no member named 'can_stattimer'
     if (net->can.can_stattimer.function == can_stat_update) {
                 ^
--
   In file included from include/linux/kernel.h:10:0,
                    from include/linux/list.h:9,
                    from include/linux/module.h:9,
                    from net//can/proc.c:42:
   net//can/proc.c: In function 'can_stat_update':
   net//can/proc.c:120:42: error: 'struct netns_can' has no member named 'can_stattimer'
     struct net *net = from_timer(net, t, can.can_stattimer);
                                             ^
   include/linux/compiler.h:301:19: note: in definition of macro '__compiletime_assert'
      bool __cond = !(condition);    \
                      ^
   include/linux/compiler.h:324:2: note: in expansion of macro '_compiletime_assert'
     _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
     ^
   include/linux/build_bug.h:47:37: note: in expansion of macro 'compiletime_assert'
    #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
                                        ^
   include/linux/kernel.h:930:2: note: in expansion of macro 'BUILD_BUG_ON_MSG'
     BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
     ^
   include/linux/kernel.h:930:20: note: in expansion of macro '__same_type'
     BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
                       ^
   include/linux/timer.h:144:2: note: in expansion of macro 'container_of'
     container_of(callback_timer, typeof(*var), timer_fieldname)
     ^
   net//can/proc.c:120:20: note: in expansion of macro 'from_timer'
     struct net *net = from_timer(net, t, can.can_stattimer);
                       ^
   In file included from include/linux/compiler_types.h:58:0,
                    from include/uapi/linux/stddef.h:2,
                    from include/linux/stddef.h:5,
                    from include/uapi/linux/posix_types.h:5,
                    from include/uapi/linux/types.h:14,
                    from include/linux/types.h:6,
                    from include/linux/list.h:5,
                    from include/linux/module.h:9,
                    from net//can/proc.c:42:
>> include/linux/compiler-gcc.h:166:2: error: 'struct netns_can' has no member named 'can_stattimer'
     __builtin_offsetof(a, b)
     ^
   include/linux/stddef.h:17:32: note: in expansion of macro '__compiler_offsetof'
    #define offsetof(TYPE, MEMBER) __compiler_offsetof(TYPE, MEMBER)
                                   ^
   include/linux/kernel.h:933:21: note: in expansion of macro 'offsetof'
     ((type *)(__mptr - offsetof(type, member))); })
                        ^
   include/linux/timer.h:144:2: note: in expansion of macro 'container_of'
     container_of(callback_timer, typeof(*var), timer_fieldname)
     ^
   net//can/proc.c:120:20: note: in expansion of macro 'from_timer'
     struct net *net = from_timer(net, t, can.can_stattimer);
                       ^
   net//can/proc.c: In function 'can_stats_proc_show':
   net//can/proc.c:224:14: error: 'struct netns_can' has no member named 'can_stattimer'
     if (net->can.can_stattimer.function == can_stat_update) {
                 ^
   net//can/proc.c: In function 'can_reset_stats_proc_show':
   net//can/proc.c:294:14: error: 'struct netns_can' has no member named 'can_stattimer'
     if (net->can.can_stattimer.function == can_stat_update) {
                 ^

vim +166 include/linux/compiler-gcc.h

cb984d10 Joe Perches 2015-06-25  163  
cb984d10 Joe Perches 2015-06-25  164  #define __used			__attribute__((__used__))
cb984d10 Joe Perches 2015-06-25  165  #define __compiler_offsetof(a, b)					\
cb984d10 Joe Perches 2015-06-25 @166  	__builtin_offsetof(a, b)
cb984d10 Joe Perches 2015-06-25  167  

:::::: The code at line 166 was first introduced by commit
:::::: cb984d101b30eb7478d32df56a0023e4603cba7f compiler-gcc: integrate the various compiler-gcc[345].h files

:::::: TO: Joe Perches <joe@perches.com>
:::::: CC: Linus Torvalds <torvalds@linux-foundation.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 27019 bytes --]

^ permalink raw reply

* peak-usb: peak_usb_netif_rx() ts_high ununsed
From: Marc Kleine-Budde @ 2017-12-06 14:09 UTC (permalink / raw)
  To: linux-can, Stephane Grosjean


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

Hello,

we just found out that ts_high is never used:

> /*
>  * post received skb after having set any hw timestamp
>  */
> int peak_usb_netif_rx(struct sk_buff *skb,
> 		      struct peak_time_ref *time_ref, u32 ts_low, u32 ts_high)
> {
> 	struct skb_shared_hwtstamps *hwts = skb_hwtstamps(skb);
> 
> 	peak_usb_get_ts_time(time_ref, ts_low, &hwts->hwtstamp);
> 
> 	return netif_rx(skb);
> }

Is that expected? If yes, let's remove ts_high.

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: [mkl-can-next:af_can 16/18] drivers/net/can/slcan.c:531:51: error: invalid application of 'sizeof' to incomplete type 'struct can_ml_priv'
From: Marc Kleine-Budde @ 2017-12-06 13:13 UTC (permalink / raw)
  To: kbuild test robot; +Cc: kbuild-all, linux-can
In-Reply-To: <201712062055.bgtxdnoo%fengguang.wu@intel.com>


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

On 12/06/2017 01:49 PM, kbuild test robot wrote:
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git af_can
> head:   f0c2db14e47365b89dd8e76a365f0f9d5a48a914
> commit: b6ecae37409c9654a3d6c4908ebfd6e0c1939c0c [16/18] can: introduce CAN midlayer private and allocate it automatically
> config: x86_64-randconfig-x001-201749 (attached as .config)
> compiler: gcc-7 (Debian 7.2.0-12) 7.2.1 20171025
> reproduce:
>         git checkout b6ecae37409c9654a3d6c4908ebfd6e0c1939c0c
>         # save the attached .config to linux build tree
>         make ARCH=x86_64 
> 
> All errors (new ones prefixed by >>):
> 
>    drivers/net/can/slcan.c: In function 'slc_alloc':
>>> drivers/net/can/slcan.c:531:51: error: invalid application of 'sizeof' to incomplete type 'struct can_ml_priv'
>      size = ALIGN(sizeof(*sl), NETDEV_ALIGN) + sizeof(struct can_ml_priv);

Should be fixed already.

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

* [mkl-can-next:af_can 16/18] drivers/net/can/slcan.c:531:51: error: invalid application of 'sizeof' to incomplete type 'struct can_ml_priv'
From: kbuild test robot @ 2017-12-06 12:49 UTC (permalink / raw)
  To: Marc Kleine-Budde; +Cc: kbuild-all, linux-can

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

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git af_can
head:   f0c2db14e47365b89dd8e76a365f0f9d5a48a914
commit: b6ecae37409c9654a3d6c4908ebfd6e0c1939c0c [16/18] can: introduce CAN midlayer private and allocate it automatically
config: x86_64-randconfig-x001-201749 (attached as .config)
compiler: gcc-7 (Debian 7.2.0-12) 7.2.1 20171025
reproduce:
        git checkout b6ecae37409c9654a3d6c4908ebfd6e0c1939c0c
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/net/can/slcan.c: In function 'slc_alloc':
>> drivers/net/can/slcan.c:531:51: error: invalid application of 'sizeof' to incomplete type 'struct can_ml_priv'
     size = ALIGN(sizeof(*sl), NETDEV_ALIGN) + sizeof(struct can_ml_priv);
                                                      ^~~~~~
--
>> drivers/net/can/vxcan.c:295:71: error: invalid application of 'sizeof' to incomplete type 'struct can_ml_priv'
     .priv_size = ALIGN(sizeof(struct vxcan_priv), NETDEV_ALIGN) + sizeof(struct can_ml_priv);
                                                                          ^~~~~~
>> drivers/net/can/vxcan.c:295:90: error: expected '}' before ';' token
     .priv_size = ALIGN(sizeof(struct vxcan_priv), NETDEV_ALIGN) + sizeof(struct can_ml_priv);
                                                                                             ^
   drivers/net/can/vxcan.c:285:20: warning: 'vxcan_get_link_net' defined but not used [-Wunused-function]
    static struct net *vxcan_get_link_net(const struct net_device *dev)
                       ^~~~~~~~~~~~~~~~~~
   drivers/net/can/vxcan.c:259:13: warning: 'vxcan_dellink' defined but not used [-Wunused-function]
    static void vxcan_dellink(struct net_device *dev, struct list_head *head)
                ^~~~~~~~~~~~~
   drivers/net/can/vxcan.c:165:12: warning: 'vxcan_newlink' defined but not used [-Wunused-function]
    static int vxcan_newlink(struct net *net, struct net_device *dev,
               ^~~~~~~~~~~~~
   drivers/net/can/vxcan.c:150:13: warning: 'vxcan_setup' defined but not used [-Wunused-function]
    static void vxcan_setup(struct net_device *dev)
                ^~~~~~~~~~~

vim +531 drivers/net/can/slcan.c

   509	
   510	/* Find a free SLCAN channel, and link in this `tty' line. */
   511	static struct slcan *slc_alloc(dev_t line)
   512	{
   513		int i;
   514		char name[IFNAMSIZ];
   515		struct net_device *dev = NULL;
   516		struct slcan       *sl;
   517		int size;
   518	
   519		for (i = 0; i < maxdev; i++) {
   520			dev = slcan_devs[i];
   521			if (dev == NULL)
   522				break;
   523	
   524		}
   525	
   526		/* Sorry, too many, all slots in use */
   527		if (i >= maxdev)
   528			return NULL;
   529	
   530		sprintf(name, "slcan%d", i);
 > 531		size = ALIGN(sizeof(*sl), NETDEV_ALIGN) + sizeof(struct can_ml_priv);
   532		dev = alloc_netdev(size, name, NET_NAME_UNKNOWN, slc_setup);
   533		if (!dev)
   534			return NULL;
   535	
   536		dev->base_addr  = i;
   537		sl = netdev_priv(dev);
   538	
   539		/* Initialize channel control data */
   540		sl->magic = SLCAN_MAGIC;
   541		sl->dev	= dev;
   542		spin_lock_init(&sl->lock);
   543		INIT_WORK(&sl->tx_work, slcan_transmit);
   544		slcan_devs[i] = dev;
   545	
   546		return sl;
   547	}
   548	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 27492 bytes --]

^ permalink raw reply

* Re: CAN-USB adapter unplug
From: Jimmy Assarsson @ 2017-12-06  9:30 UTC (permalink / raw)
  To: Martin Kelly, linux-can@vger.kernel.org
  Cc: Marc Kleine-Budde, Stefan Mätje,
	Remigiusz Kołłątaj, Wolfgang Grandegger
In-Reply-To: <f28efe8a-e700-d6e8-6b2a-f95e92396bb6@xevo.com>

On 2017-12-06 00:42, Martin Kelly wrote:
> On 12/04/2017 12:39 AM, Jimmy Assarsson wrote:
>> 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.
>>
> 
> Yeah, it's unclear to me whether what gs_usb does is safe. I have kept 
> my patches just using -EPIPE/-EPROTO rather than a catch-all default, 
> just to be safe.

Sounds good :)

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


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