* [PATCH] can: dev: fix deadlock reported after bus-off
From: Marc Kleine-Budde @ 2016-09-22 12:42 UTC (permalink / raw)
To: netdev
Cc: davem, linux-can, kernel, Sergei Miroshnichenko, linux-stable,
Marc Kleine-Budde
In-Reply-To: <20160922124222.31598-1-mkl@pengutronix.de>
From: Sergei Miroshnichenko <sergeimir@emcraft.com>
A timer was used to restart after the bus-off state, leading to a
relatively large can_restart() executed in an interrupt context,
which in turn sets up pinctrl. When this happens during system boot,
there is a high probability of grabbing the pinctrl_list_mutex,
which is locked already by the probe() of other device, making the
kernel suspect a deadlock condition [1].
To resolve this issue, the restart_timer is replaced by a delayed
work.
[1] https://github.com/victronenergy/venus/issues/24
Signed-off-by: Sergei Miroshnichenko <sergeimir@emcraft.com>
Cc: linux-stable <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
drivers/net/can/dev.c | 27 +++++++++++++++++----------
include/linux/can/dev.h | 3 ++-
2 files changed, 19 insertions(+), 11 deletions(-)
diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
index e21f7cc5ae4d..8d6208c0b400 100644
--- a/drivers/net/can/dev.c
+++ b/drivers/net/can/dev.c
@@ -21,6 +21,7 @@
#include <linux/slab.h>
#include <linux/netdevice.h>
#include <linux/if_arp.h>
+#include <linux/workqueue.h>
#include <linux/can.h>
#include <linux/can/dev.h>
#include <linux/can/skb.h>
@@ -501,9 +502,8 @@ EXPORT_SYMBOL_GPL(can_free_echo_skb);
/*
* CAN device restart for bus-off recovery
*/
-static void can_restart(unsigned long data)
+static void can_restart(struct net_device *dev)
{
- struct net_device *dev = (struct net_device *)data;
struct can_priv *priv = netdev_priv(dev);
struct net_device_stats *stats = &dev->stats;
struct sk_buff *skb;
@@ -543,6 +543,14 @@ restart:
netdev_err(dev, "Error %d during restart", err);
}
+static void can_restart_work(struct work_struct *work)
+{
+ struct delayed_work *dwork = to_delayed_work(work);
+ struct can_priv *priv = container_of(dwork, struct can_priv, restart_work);
+
+ can_restart(priv->dev);
+}
+
int can_restart_now(struct net_device *dev)
{
struct can_priv *priv = netdev_priv(dev);
@@ -556,8 +564,8 @@ int can_restart_now(struct net_device *dev)
if (priv->state != CAN_STATE_BUS_OFF)
return -EBUSY;
- /* Runs as soon as possible in the timer context */
- mod_timer(&priv->restart_timer, jiffies);
+ cancel_delayed_work_sync(&priv->restart_work);
+ can_restart(dev);
return 0;
}
@@ -578,8 +586,8 @@ void can_bus_off(struct net_device *dev)
netif_carrier_off(dev);
if (priv->restart_ms)
- mod_timer(&priv->restart_timer,
- jiffies + (priv->restart_ms * HZ) / 1000);
+ schedule_delayed_work(&priv->restart_work,
+ msecs_to_jiffies(priv->restart_ms));
}
EXPORT_SYMBOL_GPL(can_bus_off);
@@ -688,6 +696,7 @@ struct net_device *alloc_candev(int sizeof_priv, unsigned int echo_skb_max)
return NULL;
priv = netdev_priv(dev);
+ priv->dev = dev;
if (echo_skb_max) {
priv->echo_skb_max = echo_skb_max;
@@ -697,7 +706,7 @@ struct net_device *alloc_candev(int sizeof_priv, unsigned int echo_skb_max)
priv->state = CAN_STATE_STOPPED;
- init_timer(&priv->restart_timer);
+ INIT_DELAYED_WORK(&priv->restart_work, can_restart_work);
return dev;
}
@@ -778,8 +787,6 @@ int open_candev(struct net_device *dev)
if (!netif_carrier_ok(dev))
netif_carrier_on(dev);
- setup_timer(&priv->restart_timer, can_restart, (unsigned long)dev);
-
return 0;
}
EXPORT_SYMBOL_GPL(open_candev);
@@ -794,7 +801,7 @@ void close_candev(struct net_device *dev)
{
struct can_priv *priv = netdev_priv(dev);
- del_timer_sync(&priv->restart_timer);
+ cancel_delayed_work_sync(&priv->restart_work);
can_flush_echo_skb(dev);
}
EXPORT_SYMBOL_GPL(close_candev);
diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h
index 5261751f6bd4..5f5270941ba0 100644
--- a/include/linux/can/dev.h
+++ b/include/linux/can/dev.h
@@ -32,6 +32,7 @@ enum can_mode {
* CAN common private data
*/
struct can_priv {
+ struct net_device *dev;
struct can_device_stats can_stats;
struct can_bittiming bittiming, data_bittiming;
@@ -47,7 +48,7 @@ struct can_priv {
u32 ctrlmode_static; /* static enabled options for driver/hardware */
int restart_ms;
- struct timer_list restart_timer;
+ struct delayed_work restart_work;
int (*do_set_bittiming)(struct net_device *dev);
int (*do_set_data_bittiming)(struct net_device *dev);
--
2.9.3
^ permalink raw reply related
* pull-request: can 2016-09-22
From: Marc Kleine-Budde @ 2016-09-22 12:42 UTC (permalink / raw)
To: netdev; +Cc: davem, linux-can, kernel
Hello David,
this is a pull request of one patch for the upcoming linux-4.8 release.
The patch by Sergei Miroshnichenko fixes a potential deadlock in the generic
CAN device code that cann occour after a bus-off.
regards,
Marc
---
The following changes since commit 7e32b44361abc77fbc01f2b97b045c405b2583e5:
tcp: properly account Fast Open SYN-ACK retrans (2016-09-22 03:33:01 -0400)
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.8-20160922
for you to fetch changes up to 9abefcb1aaa58b9d5aa40a8bb12c87d02415e4c8:
can: dev: fix deadlock reported after bus-off (2016-09-22 10:01:21 +0200)
----------------------------------------------------------------
linux-can-fixes-for-4.8-20160922
----------------------------------------------------------------
Sergei Miroshnichenko (1):
can: dev: fix deadlock reported after bus-off
drivers/net/can/dev.c | 27 +++++++++++++++++----------
include/linux/can/dev.h | 3 ++-
2 files changed, 19 insertions(+), 11 deletions(-)
^ permalink raw reply
* Re: [PATCH 1/1] can: fix deadlock reported after bus-off
From: Marc Kleine-Budde @ 2016-09-22 8:02 UTC (permalink / raw)
To: Sergei Miroshnichenko, linux-can; +Cc: Alexander Dyachenko, Stefano Babic
In-Reply-To: <1473256272-30283-1-git-send-email-sergeimir@emcraft.com>
[-- Attachment #1.1: Type: text/plain, Size: 964 bytes --]
On 09/07/2016 03:51 PM, Sergei Miroshnichenko wrote:
> A timer was used to restart after the bus-off state, leading to a
> relatively large can_restart() executed in an interrupt context,
> which in turn sets up pinctrl. When this happens during system boot,
> there is a high probability of grabbing the pinctrl_list_mutex,
> which is locked already by the probe() of other device, making the
> kernel suspect a deadlock condition [1].
>
> To resolve this issue, the restart_timer is replaced by a delayed
> work.
>
> [1] https://github.com/victronenergy/venus/issues/24
>
> Signed-off-by: Sergei Miroshnichenko <sergeimir@emcraft.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: 455 bytes --]
^ permalink raw reply
* Re: pull-request: can 2016-09-21
From: David Miller @ 2016-09-22 6:48 UTC (permalink / raw)
To: mkl; +Cc: netdev, linux-can, kernel
In-Reply-To: <20160921084355.20972-1-mkl@pengutronix.de>
From: Marc Kleine-Budde <mkl@pengutronix.de>
Date: Wed, 21 Sep 2016 10:43:54 +0200
> this is another pull request of one patch for the upcoming linux-4.8 release.
>
> Marek Vasut fixes the CAN-FD bit rate switch in the ifi driver by configuring
> the transmitter delay.
Pulled, thanks.
^ permalink raw reply
* RE: BCM loopback - Information request
From: Angeloni Andrea @ 2016-09-21 15:03 UTC (permalink / raw)
To: Oliver Hartkopp, Alexander Stein; +Cc: linux-can@vger.kernel.org
In-Reply-To: <79dbb349-8bf5-ea85-e0dc-80cd5114e41a@hartkopp.net>
As usual, problem exists between chair and keyboard.
I changed during debug the CAN filters. Same message was matching two of them, thus returning two different reads. Nothing related to recvmsg as wrongly reported by me.
Regards,
Andrea
-----Original Message-----
From: Angeloni Andrea
Sent: mercoledì 21 settembre 2016 15:35
To: 'Oliver Hartkopp' <socketcan@hartkopp.net>; 'Alexander Stein' <alexander.stein@systec-electronic.com>
Cc: 'linux-can@vger.kernel.org' <linux-can@vger.kernel.org>
Subject: RE: BCM loopback - Information request
Dears,
sorry to bother you again with newby questions.
I changed my code so that "read" is replaced by "recvmsg". After reading, flag MSG_DONTROUTE is checked to understand if data is coming from same CAN device. Now I correctly do not see on CAN[i] messages sent by CAN[i], but whatever I send, via socket RAW or BCM, is read (i.e. n = recvmsg() > 0) twice by recvmsg.
Any hint?
Thanks,
Andrea
-----Original Message-----
From: Angeloni Andrea
Sent: martedì 20 settembre 2016 17:44
To: 'Oliver Hartkopp' <socketcan@hartkopp.net>; Alexander Stein <alexander.stein@systec-electronic.com>
Cc: linux-can@vger.kernel.org
Subject: RE: BCM loopback - Information request
It solves perfectly my issue.
Thanks for your piece of software, good luck sirs.
Andrea
-----Original Message-----
From: Oliver Hartkopp [mailto:socketcan@hartkopp.net]
Sent: martedì 20 settembre 2016 17:29
To: Angeloni Andrea <andrea.angeloni@alfamationglobal.com>; Alexander Stein <alexander.stein@systec-electronic.com>
Cc: linux-can@vger.kernel.org
Subject: Re: BCM loopback - Information request
On 09/20/2016 05:19 PM, Angeloni Andrea wrote:
> Ok clear.
>
> With local host you mean same Linux system
Yes.
"MSG_DONTROUTE: set when the received frame was created on the local host."
or same CAN interface? On the uPC I'm currently using there are two CAN interfaces, and it could happen that one sends data to be processed to the other.
When the CAN frame is physically received by the CAN controller it comes 'from outer space' and not from the local host.
When you sent a frame with CAN ID 0x123 with the CAN_BCM on can0 and you have can0 and can1 connected together the received frame on can0 will have MSG_DONTROUTE set and the frame received from can1 will have MSG_DONTROUTE unset.
Regards,
Oliver
>
> Regards,
> Andrea
>
> -----Original Message-----
> From: Oliver Hartkopp [mailto:socketcan@hartkopp.net]
> Sent: martedì 20 settembre 2016 17:07
> To: Angeloni Andrea <andrea.angeloni@alfamationglobal.com>; Alexander
> Stein <alexander.stein@systec-electronic.com>
> Cc: linux-can@vger.kernel.org
> Subject: Re: BCM loopback - Information request
>
> Hi Andrea,
>
> the CAN_BCM sets the loopback by default:
>
> http://lxr.linux.no/#linux+v4.7.4/net/can/bcm.c#L1229
>
> So it is not configurable by now.
>
> An possibility could be to implement socket options for the CAN_BCM so that a setsockopt() with a new CAN_BCM_LOOPBACK becomes available.
>
> But this is not in mainline Linux now.
>
> Alternatively you can check for the fames you received on a CAN_RAW socket whether these frames have been created on the local host:
>
> http://lxr.linux.no/#linux+v4.7.4/Documentation/networking/can.txt#L63
> 2
>
> Which means that you have to use recvmsg() to read from CAN_RAW.
>
> 'candump' does it here:
> https://github.com/linux-can/can-utils/blob/master/candump.c#L793
>
> Regards,
> Oliver
>
> On 09/20/2016 12:19 PM, Angeloni Andrea wrote:
>> Ciao Alexander,
>>
>> thanks for your answer.
>>
>> Unfortunately, CAN_RAW_LOOPBACK is not working. Option is indeed referring to socket and not device, while BCM messages are coming from a different socket.
>>
>> Andrea
>>
>> -----Original Message-----
>> From: Alexander Stein [mailto:alexander.stein@systec-electronic.com]
>> Sent: martedì 20 settembre 2016 11:54
>> To: Angeloni Andrea <andrea.angeloni@alfamationglobal.com>
>> Cc: linux-can@vger.kernel.org
>> Subject: Re: BCM loopback - Information request
>>
>> Hi Andrea,
>>
>> On Tuesday 20 September 2016 09:42:53, Angeloni Andrea wrote:
>>> In my application there is a thread for each initialized CAN
>>> interface that sends over TCP all the read data to a Windows Client.
>>> Data are read using Socket RAW. Application is also capable to send
>>> cyclic messages using Broadcast manager.
>>>
>>> Here is my question: how can I configure my application so that
>>> reading thread does not get messages sent by BCM on the same interface?
>>
>> I think the socket option CAN_RAW_LOOPBACK is what you need. Please
>> refer to
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/
>> D
>> ocumentation/networking/can.txt#n536
>> You will not receive CAN frames sent from interface (BCM and other CAN frames), thus only messages from other CAN nodes.
>>
>> HTH
>> Alexander
>>
>>
>> --
>> 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: BCM loopback - Information request
From: Angeloni Andrea @ 2016-09-21 13:35 UTC (permalink / raw)
To: Oliver Hartkopp, Alexander Stein; +Cc: linux-can@vger.kernel.org
In-Reply-To: <79dbb349-8bf5-ea85-e0dc-80cd5114e41a@hartkopp.net>
Dears,
sorry to bother you again with newby questions.
I changed my code so that "read" is replaced by "recvmsg". After reading, flag MSG_DONTROUTE is checked to understand if data is coming from same CAN device. Now I correctly do not see on CAN[i] messages sent by CAN[i], but whatever I send, via socket RAW or BCM, is read (i.e. n = recvmsg() > 0) twice by recvmsg.
Any hint?
Thanks,
Andrea
-----Original Message-----
From: Angeloni Andrea
Sent: martedì 20 settembre 2016 17:44
To: 'Oliver Hartkopp' <socketcan@hartkopp.net>; Alexander Stein <alexander.stein@systec-electronic.com>
Cc: linux-can@vger.kernel.org
Subject: RE: BCM loopback - Information request
It solves perfectly my issue.
Thanks for your piece of software, good luck sirs.
Andrea
-----Original Message-----
From: Oliver Hartkopp [mailto:socketcan@hartkopp.net]
Sent: martedì 20 settembre 2016 17:29
To: Angeloni Andrea <andrea.angeloni@alfamationglobal.com>; Alexander Stein <alexander.stein@systec-electronic.com>
Cc: linux-can@vger.kernel.org
Subject: Re: BCM loopback - Information request
On 09/20/2016 05:19 PM, Angeloni Andrea wrote:
> Ok clear.
>
> With local host you mean same Linux system
Yes.
"MSG_DONTROUTE: set when the received frame was created on the local host."
or same CAN interface? On the uPC I'm currently using there are two CAN interfaces, and it could happen that one sends data to be processed to the other.
When the CAN frame is physically received by the CAN controller it comes 'from outer space' and not from the local host.
When you sent a frame with CAN ID 0x123 with the CAN_BCM on can0 and you have can0 and can1 connected together the received frame on can0 will have MSG_DONTROUTE set and the frame received from can1 will have MSG_DONTROUTE unset.
Regards,
Oliver
>
> Regards,
> Andrea
>
> -----Original Message-----
> From: Oliver Hartkopp [mailto:socketcan@hartkopp.net]
> Sent: martedì 20 settembre 2016 17:07
> To: Angeloni Andrea <andrea.angeloni@alfamationglobal.com>; Alexander
> Stein <alexander.stein@systec-electronic.com>
> Cc: linux-can@vger.kernel.org
> Subject: Re: BCM loopback - Information request
>
> Hi Andrea,
>
> the CAN_BCM sets the loopback by default:
>
> http://lxr.linux.no/#linux+v4.7.4/net/can/bcm.c#L1229
>
> So it is not configurable by now.
>
> An possibility could be to implement socket options for the CAN_BCM so that a setsockopt() with a new CAN_BCM_LOOPBACK becomes available.
>
> But this is not in mainline Linux now.
>
> Alternatively you can check for the fames you received on a CAN_RAW socket whether these frames have been created on the local host:
>
> http://lxr.linux.no/#linux+v4.7.4/Documentation/networking/can.txt#L63
> 2
>
> Which means that you have to use recvmsg() to read from CAN_RAW.
>
> 'candump' does it here:
> https://github.com/linux-can/can-utils/blob/master/candump.c#L793
>
> Regards,
> Oliver
>
> On 09/20/2016 12:19 PM, Angeloni Andrea wrote:
>> Ciao Alexander,
>>
>> thanks for your answer.
>>
>> Unfortunately, CAN_RAW_LOOPBACK is not working. Option is indeed referring to socket and not device, while BCM messages are coming from a different socket.
>>
>> Andrea
>>
>> -----Original Message-----
>> From: Alexander Stein [mailto:alexander.stein@systec-electronic.com]
>> Sent: martedì 20 settembre 2016 11:54
>> To: Angeloni Andrea <andrea.angeloni@alfamationglobal.com>
>> Cc: linux-can@vger.kernel.org
>> Subject: Re: BCM loopback - Information request
>>
>> Hi Andrea,
>>
>> On Tuesday 20 September 2016 09:42:53, Angeloni Andrea wrote:
>>> In my application there is a thread for each initialized CAN
>>> interface that sends over TCP all the read data to a Windows Client.
>>> Data are read using Socket RAW. Application is also capable to send
>>> cyclic messages using Broadcast manager.
>>>
>>> Here is my question: how can I configure my application so that
>>> reading thread does not get messages sent by BCM on the same interface?
>>
>> I think the socket option CAN_RAW_LOOPBACK is what you need. Please
>> refer to
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/
>> D
>> ocumentation/networking/can.txt#n536
>> You will not receive CAN frames sent from interface (BCM and other CAN frames), thus only messages from other CAN nodes.
>>
>> HTH
>> Alexander
>>
>>
>> --
>> 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
* [PATCH] net: can: ifi: Configure transmitter delay
From: Marc Kleine-Budde @ 2016-09-21 8:43 UTC (permalink / raw)
To: netdev
Cc: davem, linux-can, kernel, Marek Vasut, Marc Kleine-Budde,
Mark Rutland, Oliver Hartkopp, Wolfgang Grandegger, linux-stable
In-Reply-To: <20160921084355.20972-1-mkl@pengutronix.de>
From: Marek Vasut <marex@denx.de>
Configure the transmitter delay register at +0x1c to correctly handle
the CAN FD bitrate switch (BRS). This moves the SSP (secondary sample
point) to a proper offset, so that the TDC mechanism works and won't
generate error frames on the CAN link.
Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Marc Kleine-Budde <mkl@pengutronix.de>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Oliver Hartkopp <socketcan@hartkopp.net>
Cc: Wolfgang Grandegger <wg@grandegger.com>
Cc: linux-stable <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
drivers/net/can/ifi_canfd/ifi_canfd.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/net/can/ifi_canfd/ifi_canfd.c b/drivers/net/can/ifi_canfd/ifi_canfd.c
index 2d1d22eec750..368bb0710d8f 100644
--- a/drivers/net/can/ifi_canfd/ifi_canfd.c
+++ b/drivers/net/can/ifi_canfd/ifi_canfd.c
@@ -81,6 +81,10 @@
#define IFI_CANFD_TIME_SET_TIMEA_4_12_6_6 BIT(15)
#define IFI_CANFD_TDELAY 0x1c
+#define IFI_CANFD_TDELAY_DEFAULT 0xb
+#define IFI_CANFD_TDELAY_MASK 0x3fff
+#define IFI_CANFD_TDELAY_ABS BIT(14)
+#define IFI_CANFD_TDELAY_EN BIT(15)
#define IFI_CANFD_ERROR 0x20
#define IFI_CANFD_ERROR_TX_OFFSET 0
@@ -641,7 +645,7 @@ static void ifi_canfd_set_bittiming(struct net_device *ndev)
struct ifi_canfd_priv *priv = netdev_priv(ndev);
const struct can_bittiming *bt = &priv->can.bittiming;
const struct can_bittiming *dbt = &priv->can.data_bittiming;
- u16 brp, sjw, tseg1, tseg2;
+ u16 brp, sjw, tseg1, tseg2, tdc;
/* Configure bit timing */
brp = bt->brp - 2;
@@ -664,6 +668,11 @@ static void ifi_canfd_set_bittiming(struct net_device *ndev)
(brp << IFI_CANFD_TIME_PRESCALE_OFF) |
(sjw << IFI_CANFD_TIME_SJW_OFF_7_9_8_8),
priv->base + IFI_CANFD_FTIME);
+
+ /* Configure transmitter delay */
+ tdc = (dbt->brp * (dbt->phase_seg1 + 1)) & IFI_CANFD_TDELAY_MASK;
+ writel(IFI_CANFD_TDELAY_EN | IFI_CANFD_TDELAY_ABS | tdc,
+ priv->base + IFI_CANFD_TDELAY);
}
static void ifi_canfd_set_filter(struct net_device *ndev, const u32 id,
--
2.9.3
^ permalink raw reply related
* pull-request: can 2016-09-21
From: Marc Kleine-Budde @ 2016-09-21 8:43 UTC (permalink / raw)
To: netdev; +Cc: davem, linux-can, kernel
Hello David,
this is another pull request of one patch for the upcoming linux-4.8 release.
Marek Vasut fixes the CAN-FD bit rate switch in the ifi driver by configuring
the transmitter delay.
regards,
Marc
---
The following changes since commit b5036cd4ed3173ab8cdbc85e2ba74acf46bafb51:
ipmr, ip6mr: return lastuse relative to now (2016-09-21 00:58:23 -0400)
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.8-20160921
for you to fetch changes up to 8d58790b832e13d6006d842037732304af357c3c:
net: can: ifi: Configure transmitter delay (2016-09-21 10:32:24 +0200)
----------------------------------------------------------------
linux-can-fixes-for-4.8-20160921
----------------------------------------------------------------
Marek Vasut (1):
net: can: ifi: Configure transmitter delay
drivers/net/can/ifi_canfd/ifi_canfd.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
^ permalink raw reply
* Re: [PATCH] net: can: ifi: Configure transmitter delay
From: Marc Kleine-Budde @ 2016-09-21 8:36 UTC (permalink / raw)
To: Marek Vasut, linux-can; +Cc: Mark Rutland, Oliver Hartkopp, Wolfgang Grandegger
In-Reply-To: <20160919193401.8320-1-marex@denx.de>
[-- Attachment #1.1: Type: text/plain, Size: 857 bytes --]
On 09/19/2016 09:34 PM, Marek Vasut wrote:
> Configure the transmitter delay register at +0x1c to correctly handle
> the CAN FD bitrate switch (BRS). This moves the SSP (secondary sample
> point) to a proper offset, so that the TDC mechanism works and won't
> generate error frames on the CAN link.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Marc Kleine-Budde <mkl@pengutronix.de>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Oliver Hartkopp <socketcan@hartkopp.net>
> Cc: Wolfgang Grandegger <wg@grandegger.com>
Applied to linux-can.
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: 455 bytes --]
^ permalink raw reply
* Re: pull-request: can 2016-09-19
From: David Miller @ 2016-09-21 2:48 UTC (permalink / raw)
To: mkl; +Cc: netdev, linux-can, kernel
In-Reply-To: <20160919141949.5826-1-mkl@pengutronix.de>
From: Marc Kleine-Budde <mkl@pengutronix.de>
Date: Mon, 19 Sep 2016 16:19:48 +0200
> this is a pull request of one patch for the upcoming linux-4.8 release.
>
> The patch by Fabio Estevam fixes the pm handling in the flexcan driver.
Pulled, thanks.
^ permalink raw reply
* RE: BCM loopback - Information request
From: Angeloni Andrea @ 2016-09-20 15:43 UTC (permalink / raw)
To: Oliver Hartkopp, Alexander Stein; +Cc: linux-can@vger.kernel.org
In-Reply-To: <79dbb349-8bf5-ea85-e0dc-80cd5114e41a@hartkopp.net>
It solves perfectly my issue.
Thanks for your piece of software, good luck sirs.
Andrea
-----Original Message-----
From: Oliver Hartkopp [mailto:socketcan@hartkopp.net]
Sent: martedì 20 settembre 2016 17:29
To: Angeloni Andrea <andrea.angeloni@alfamationglobal.com>; Alexander Stein <alexander.stein@systec-electronic.com>
Cc: linux-can@vger.kernel.org
Subject: Re: BCM loopback - Information request
On 09/20/2016 05:19 PM, Angeloni Andrea wrote:
> Ok clear.
>
> With local host you mean same Linux system
Yes.
"MSG_DONTROUTE: set when the received frame was created on the local host."
or same CAN interface? On the uPC I'm currently using there are two CAN interfaces, and it could happen that one sends data to be processed to the other.
When the CAN frame is physically received by the CAN controller it comes 'from outer space' and not from the local host.
When you sent a frame with CAN ID 0x123 with the CAN_BCM on can0 and you have can0 and can1 connected together the received frame on can0 will have MSG_DONTROUTE set and the frame received from can1 will have MSG_DONTROUTE unset.
Regards,
Oliver
>
> Regards,
> Andrea
>
> -----Original Message-----
> From: Oliver Hartkopp [mailto:socketcan@hartkopp.net]
> Sent: martedì 20 settembre 2016 17:07
> To: Angeloni Andrea <andrea.angeloni@alfamationglobal.com>; Alexander
> Stein <alexander.stein@systec-electronic.com>
> Cc: linux-can@vger.kernel.org
> Subject: Re: BCM loopback - Information request
>
> Hi Andrea,
>
> the CAN_BCM sets the loopback by default:
>
> http://lxr.linux.no/#linux+v4.7.4/net/can/bcm.c#L1229
>
> So it is not configurable by now.
>
> An possibility could be to implement socket options for the CAN_BCM so that a setsockopt() with a new CAN_BCM_LOOPBACK becomes available.
>
> But this is not in mainline Linux now.
>
> Alternatively you can check for the fames you received on a CAN_RAW socket whether these frames have been created on the local host:
>
> http://lxr.linux.no/#linux+v4.7.4/Documentation/networking/can.txt#L63
> 2
>
> Which means that you have to use recvmsg() to read from CAN_RAW.
>
> 'candump' does it here:
> https://github.com/linux-can/can-utils/blob/master/candump.c#L793
>
> Regards,
> Oliver
>
> On 09/20/2016 12:19 PM, Angeloni Andrea wrote:
>> Ciao Alexander,
>>
>> thanks for your answer.
>>
>> Unfortunately, CAN_RAW_LOOPBACK is not working. Option is indeed referring to socket and not device, while BCM messages are coming from a different socket.
>>
>> Andrea
>>
>> -----Original Message-----
>> From: Alexander Stein [mailto:alexander.stein@systec-electronic.com]
>> Sent: martedì 20 settembre 2016 11:54
>> To: Angeloni Andrea <andrea.angeloni@alfamationglobal.com>
>> Cc: linux-can@vger.kernel.org
>> Subject: Re: BCM loopback - Information request
>>
>> Hi Andrea,
>>
>> On Tuesday 20 September 2016 09:42:53, Angeloni Andrea wrote:
>>> In my application there is a thread for each initialized CAN
>>> interface that sends over TCP all the read data to a Windows Client.
>>> Data are read using Socket RAW. Application is also capable to send
>>> cyclic messages using Broadcast manager.
>>>
>>> Here is my question: how can I configure my application so that
>>> reading thread does not get messages sent by BCM on the same interface?
>>
>> I think the socket option CAN_RAW_LOOPBACK is what you need. Please
>> refer to
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/
>> D
>> ocumentation/networking/can.txt#n536
>> You will not receive CAN frames sent from interface (BCM and other CAN frames), thus only messages from other CAN nodes.
>>
>> HTH
>> Alexander
>>
>>
>> --
>> 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: BCM loopback - Information request
From: Oliver Hartkopp @ 2016-09-20 15:29 UTC (permalink / raw)
To: Angeloni Andrea, Alexander Stein; +Cc: linux-can@vger.kernel.org
In-Reply-To: <BECC71D675C45C4F912901A0BBE9A3A41493CDCC@srv-mail.DominioAlfa.local>
On 09/20/2016 05:19 PM, Angeloni Andrea wrote:
> Ok clear.
>
> With local host you mean same Linux system
Yes.
"MSG_DONTROUTE: set when the received frame was created on the local host."
or same CAN interface? On the uPC I'm currently using there are two CAN
interfaces, and it could happen that one sends data to be processed to
the other.
When the CAN frame is physically received by the CAN controller it comes
'from outer space' and not from the local host.
When you sent a frame with CAN ID 0x123 with the CAN_BCM on can0 and you
have can0 and can1 connected together the received frame on can0 will
have MSG_DONTROUTE set and the frame received from can1 will have
MSG_DONTROUTE unset.
Regards,
Oliver
>
> Regards,
> Andrea
>
> -----Original Message-----
> From: Oliver Hartkopp [mailto:socketcan@hartkopp.net]
> Sent: martedì 20 settembre 2016 17:07
> To: Angeloni Andrea <andrea.angeloni@alfamationglobal.com>; Alexander Stein <alexander.stein@systec-electronic.com>
> Cc: linux-can@vger.kernel.org
> Subject: Re: BCM loopback - Information request
>
> Hi Andrea,
>
> the CAN_BCM sets the loopback by default:
>
> http://lxr.linux.no/#linux+v4.7.4/net/can/bcm.c#L1229
>
> So it is not configurable by now.
>
> An possibility could be to implement socket options for the CAN_BCM so that a setsockopt() with a new CAN_BCM_LOOPBACK becomes available.
>
> But this is not in mainline Linux now.
>
> Alternatively you can check for the fames you received on a CAN_RAW socket whether these frames have been created on the local host:
>
> http://lxr.linux.no/#linux+v4.7.4/Documentation/networking/can.txt#L632
>
> Which means that you have to use recvmsg() to read from CAN_RAW.
>
> 'candump' does it here:
> https://github.com/linux-can/can-utils/blob/master/candump.c#L793
>
> Regards,
> Oliver
>
> On 09/20/2016 12:19 PM, Angeloni Andrea wrote:
>> Ciao Alexander,
>>
>> thanks for your answer.
>>
>> Unfortunately, CAN_RAW_LOOPBACK is not working. Option is indeed referring to socket and not device, while BCM messages are coming from a different socket.
>>
>> Andrea
>>
>> -----Original Message-----
>> From: Alexander Stein [mailto:alexander.stein@systec-electronic.com]
>> Sent: martedì 20 settembre 2016 11:54
>> To: Angeloni Andrea <andrea.angeloni@alfamationglobal.com>
>> Cc: linux-can@vger.kernel.org
>> Subject: Re: BCM loopback - Information request
>>
>> Hi Andrea,
>>
>> On Tuesday 20 September 2016 09:42:53, Angeloni Andrea wrote:
>>> In my application there is a thread for each initialized CAN
>>> interface that sends over TCP all the read data to a Windows Client.
>>> Data are read using Socket RAW. Application is also capable to send
>>> cyclic messages using Broadcast manager.
>>>
>>> Here is my question: how can I configure my application so that
>>> reading thread does not get messages sent by BCM on the same interface?
>>
>> I think the socket option CAN_RAW_LOOPBACK is what you need. Please
>> refer to
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/D
>> ocumentation/networking/can.txt#n536
>> You will not receive CAN frames sent from interface (BCM and other CAN frames), thus only messages from other CAN nodes.
>>
>> HTH
>> Alexander
>>
>>
>> --
>> 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: BCM loopback - Information request
From: Angeloni Andrea @ 2016-09-20 15:19 UTC (permalink / raw)
To: Oliver Hartkopp, Alexander Stein; +Cc: linux-can@vger.kernel.org
In-Reply-To: <0cbe9cab-e983-0439-e11d-bd59b8e2f32e@hartkopp.net>
Ok clear.
With local host you mean same Linux system or same CAN interface? On the uPC I'm currently using there are two CAN interfaces, and it could happen that one sends data to be processed to the other.
Regards,
Andrea
-----Original Message-----
From: Oliver Hartkopp [mailto:socketcan@hartkopp.net]
Sent: martedì 20 settembre 2016 17:07
To: Angeloni Andrea <andrea.angeloni@alfamationglobal.com>; Alexander Stein <alexander.stein@systec-electronic.com>
Cc: linux-can@vger.kernel.org
Subject: Re: BCM loopback - Information request
Hi Andrea,
the CAN_BCM sets the loopback by default:
http://lxr.linux.no/#linux+v4.7.4/net/can/bcm.c#L1229
So it is not configurable by now.
An possibility could be to implement socket options for the CAN_BCM so that a setsockopt() with a new CAN_BCM_LOOPBACK becomes available.
But this is not in mainline Linux now.
Alternatively you can check for the fames you received on a CAN_RAW socket whether these frames have been created on the local host:
http://lxr.linux.no/#linux+v4.7.4/Documentation/networking/can.txt#L632
Which means that you have to use recvmsg() to read from CAN_RAW.
'candump' does it here:
https://github.com/linux-can/can-utils/blob/master/candump.c#L793
Regards,
Oliver
On 09/20/2016 12:19 PM, Angeloni Andrea wrote:
> Ciao Alexander,
>
> thanks for your answer.
>
> Unfortunately, CAN_RAW_LOOPBACK is not working. Option is indeed referring to socket and not device, while BCM messages are coming from a different socket.
>
> Andrea
>
> -----Original Message-----
> From: Alexander Stein [mailto:alexander.stein@systec-electronic.com]
> Sent: martedì 20 settembre 2016 11:54
> To: Angeloni Andrea <andrea.angeloni@alfamationglobal.com>
> Cc: linux-can@vger.kernel.org
> Subject: Re: BCM loopback - Information request
>
> Hi Andrea,
>
> On Tuesday 20 September 2016 09:42:53, Angeloni Andrea wrote:
>> In my application there is a thread for each initialized CAN
>> interface that sends over TCP all the read data to a Windows Client.
>> Data are read using Socket RAW. Application is also capable to send
>> cyclic messages using Broadcast manager.
>>
>> Here is my question: how can I configure my application so that
>> reading thread does not get messages sent by BCM on the same interface?
>
> I think the socket option CAN_RAW_LOOPBACK is what you need. Please
> refer to
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/D
> ocumentation/networking/can.txt#n536
> You will not receive CAN frames sent from interface (BCM and other CAN frames), thus only messages from other CAN nodes.
>
> HTH
> Alexander
>
>
> --
> 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: BCM loopback - Information request
From: Oliver Hartkopp @ 2016-09-20 15:06 UTC (permalink / raw)
To: Angeloni Andrea, Alexander Stein; +Cc: linux-can@vger.kernel.org
In-Reply-To: <BECC71D675C45C4F912901A0BBE9A3A41493CDA1@srv-mail.DominioAlfa.local>
Hi Andrea,
the CAN_BCM sets the loopback by default:
http://lxr.linux.no/#linux+v4.7.4/net/can/bcm.c#L1229
So it is not configurable by now.
An possibility could be to implement socket options for the CAN_BCM so
that a setsockopt() with a new CAN_BCM_LOOPBACK becomes available.
But this is not in mainline Linux now.
Alternatively you can check for the fames you received on a CAN_RAW
socket whether these frames have been created on the local host:
http://lxr.linux.no/#linux+v4.7.4/Documentation/networking/can.txt#L632
Which means that you have to use recvmsg() to read from CAN_RAW.
'candump' does it here:
https://github.com/linux-can/can-utils/blob/master/candump.c#L793
Regards,
Oliver
On 09/20/2016 12:19 PM, Angeloni Andrea wrote:
> Ciao Alexander,
>
> thanks for your answer.
>
> Unfortunately, CAN_RAW_LOOPBACK is not working. Option is indeed referring to socket and not device, while BCM messages are coming from a different socket.
>
> Andrea
>
> -----Original Message-----
> From: Alexander Stein [mailto:alexander.stein@systec-electronic.com]
> Sent: martedì 20 settembre 2016 11:54
> To: Angeloni Andrea <andrea.angeloni@alfamationglobal.com>
> Cc: linux-can@vger.kernel.org
> Subject: Re: BCM loopback - Information request
>
> Hi Andrea,
>
> On Tuesday 20 September 2016 09:42:53, Angeloni Andrea wrote:
>> In my application there is a thread for each initialized CAN interface
>> that sends over TCP all the read data to a Windows Client. Data are
>> read using Socket RAW. Application is also capable to send cyclic
>> messages using Broadcast manager.
>>
>> Here is my question: how can I configure my application so that
>> reading thread does not get messages sent by BCM on the same interface?
>
> I think the socket option CAN_RAW_LOOPBACK is what you need. Please refer to
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/networking/can.txt#n536
> You will not receive CAN frames sent from interface (BCM and other CAN frames), thus only messages from other CAN nodes.
>
> HTH
> Alexander
>
>
> --
> 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
* [mkl-can-next:j1939 16/24] net/can/j1939/transport.c:300:4: error: expected identifier or '(' before 'else'
From: kbuild test robot @ 2016-09-20 13:02 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: kbuild-all, linux-can
[-- Attachment #1: Type: text/plain, Size: 4023 bytes --]
tree: https://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git j1939
head: fddc023c49cba0e69c2ee75170d4d2a6ba842d0f
commit: 9b5f44d7911c357101559eff8c39f41066f49c74 [16/24] j1939: add empty lines to make code more readable
config: i386-randconfig-s1-09191616 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
git checkout 9b5f44d7911c357101559eff8c39f41066f49c74
# save the attached .config to linux build tree
make ARCH=i386
Note: the mkl-can-next/j1939 HEAD fddc023c49cba0e69c2ee75170d4d2a6ba842d0f builds fine.
It only hurts bisectibility.
All error/warnings (new ones prefixed by >>):
>> net/can/j1939/transport.c:300:4: error: expected identifier or '(' before 'else'
} else {
^~~~
>> net/can/j1939/transport.c:313:2: error: expected identifier or '(' before 'return'
return 1;
^~~~~~
>> net/can/j1939/transport.c:314:1: error: expected identifier or '(' before '}' token
}
^
>> net/can/j1939/transport.c:604:2: warning: data definition has no type or storage class
put_session(session); /* ~j1939tp_find */
^~~~~~~~~~~
>> net/can/j1939/transport.c:604:2: error: type defaults to 'int' in declaration of 'put_session' [-Werror=implicit-int]
>> net/can/j1939/transport.c:604:2: warning: parameter names (without types) in function declaration
>> net/can/j1939/transport.c:604:2: error: conflicting types for 'put_session'
net/can/j1939/transport.c:168:13: note: previous definition of 'put_session' was here
static void put_session(struct session *session)
^~~~~~~~~~~
net/can/j1939/transport.c:605:1: error: expected identifier or '(' before '}' token
}
^
net/can/j1939/transport.c: In function 'j1939tp_match':
>> net/can/j1939/transport.c:300:2: warning: control reaches end of non-void function [-Wreturn-type]
} else {
^
cc1: some warnings being treated as errors
vim +300 net/can/j1939/transport.c
9b5f44d7 Marc Kleine-Budde 2016-09-08 294
126ad779 Kurt Van Dijck 2011-05-23 295 if (session->cb->dstname) {
126ad779 Kurt Van Dijck 2011-05-23 296 if (session->cb->dstname != cb->dstname)
126ad779 Kurt Van Dijck 2011-05-23 297 return 0;
126ad779 Kurt Van Dijck 2011-05-23 298 } else if (session->cb->dstaddr != cb->dstaddr)
126ad779 Kurt Van Dijck 2011-05-23 299 return 0;
126ad779 Kurt Van Dijck 2011-05-23 @300 } else {
126ad779 Kurt Van Dijck 2011-05-23 301 if (session->cb->srcname) {
126ad779 Kurt Van Dijck 2011-05-23 302 if (session->cb->srcname != cb->dstname)
126ad779 Kurt Van Dijck 2011-05-23 303 return 0;
126ad779 Kurt Van Dijck 2011-05-23 304 } else if (session->cb->srcaddr != cb->dstaddr)
126ad779 Kurt Van Dijck 2011-05-23 305 return 0;
126ad779 Kurt Van Dijck 2011-05-23 306 if (session->cb->dstname) {
126ad779 Kurt Van Dijck 2011-05-23 307 if (session->cb->dstname != cb->srcname)
126ad779 Kurt Van Dijck 2011-05-23 308 return 0;
126ad779 Kurt Van Dijck 2011-05-23 309 } else if (session->cb->dstaddr != cb->srcaddr)
126ad779 Kurt Van Dijck 2011-05-23 310 return 0;
126ad779 Kurt Van Dijck 2011-05-23 311 }
9b5f44d7 Marc Kleine-Budde 2016-09-08 312
126ad779 Kurt Van Dijck 2011-05-23 @313 return 1;
126ad779 Kurt Van Dijck 2011-05-23 @314 }
126ad779 Kurt Van Dijck 2011-05-23 315
126ad779 Kurt Van Dijck 2011-05-23 316 static struct session *_j1939tp_find(struct list_head *root,
126ad779 Kurt Van Dijck 2011-05-23 317 struct sk_buff *skb, int reverse)
:::::: The code at line 300 was first introduced by commit
:::::: 126ad77985046d48218f25ed6f18d06bf1fa78dd can-j1939: Import SAE J1939
:::::: TO: Kurt Van Dijck <kurt.van.dijck@eia.be>
:::::: CC: Marc Kleine-Budde <mkl@pengutronix.de>
---
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: 26141 bytes --]
^ permalink raw reply
* RE: BCM loopback - Information request
From: Angeloni Andrea @ 2016-09-20 10:19 UTC (permalink / raw)
To: Alexander Stein; +Cc: linux-can@vger.kernel.org
In-Reply-To: <1509366.LC0YPQ3zn0@ws-stein>
Ciao Alexander,
thanks for your answer.
Unfortunately, CAN_RAW_LOOPBACK is not working. Option is indeed referring to socket and not device, while BCM messages are coming from a different socket.
Andrea
-----Original Message-----
From: Alexander Stein [mailto:alexander.stein@systec-electronic.com]
Sent: martedì 20 settembre 2016 11:54
To: Angeloni Andrea <andrea.angeloni@alfamationglobal.com>
Cc: linux-can@vger.kernel.org
Subject: Re: BCM loopback - Information request
Hi Andrea,
On Tuesday 20 September 2016 09:42:53, Angeloni Andrea wrote:
> In my application there is a thread for each initialized CAN interface
> that sends over TCP all the read data to a Windows Client. Data are
> read using Socket RAW. Application is also capable to send cyclic
> messages using Broadcast manager.
>
> Here is my question: how can I configure my application so that
> reading thread does not get messages sent by BCM on the same interface?
I think the socket option CAN_RAW_LOOPBACK is what you need. Please refer to
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/networking/can.txt#n536
You will not receive CAN frames sent from interface (BCM and other CAN frames), thus only messages from other CAN nodes.
HTH
Alexander
^ permalink raw reply
* Re: BCM loopback - Information request
From: Alexander Stein @ 2016-09-20 9:53 UTC (permalink / raw)
To: Angeloni Andrea; +Cc: linux-can@vger.kernel.org
In-Reply-To: <BECC71D675C45C4F912901A0BBE9A3A41493CD7D@srv-mail.DominioAlfa.local>
Hi Andrea,
On Tuesday 20 September 2016 09:42:53, Angeloni Andrea wrote:
> In my application there is a thread for each initialized CAN interface that
> sends over TCP all the read data to a Windows Client. Data are read using
> Socket RAW. Application is also capable to send cyclic messages using
> Broadcast manager.
>
> Here is my question: how can I configure my application so that reading
> thread does not get messages sent by BCM on the same interface?
I think the socket option CAN_RAW_LOOPBACK is what you need. Please refer to
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/networking/can.txt#n536
You will not receive CAN frames sent from interface (BCM and other CAN
frames), thus only messages from other CAN nodes.
HTH
Alexander
^ permalink raw reply
* BCM loopback - Information request
From: Angeloni Andrea @ 2016-09-20 9:42 UTC (permalink / raw)
To: linux-can@vger.kernel.org
Good morning,
my name is Andrea Angeloni, I'm working for an Italian company manufacturing test benches for several industries.
At the moment I'm developing a C application on ARM Linux to communicate with DUT over CAN. I'm using SocketCAN APIs.
In my application there is a thread for each initialized CAN interface that sends over TCP all the read data to a Windows Client. Data are read using Socket RAW.
Application is also capable to send cyclic messages using Broadcast manager.
Here is my question: how can I configure my application so that reading thread does not get messages sent by BCM on the same interface?
Filtering by ID is not a viable solution because a priori I'm not able to know whether or not someone on the network will send data over the same IDs of BCM.
Thanks for a feedback, regards.
Andrea Angeloni
^ permalink raw reply
* Re: j1939
From: Marc Kleine-Budde @ 2016-09-20 9:09 UTC (permalink / raw)
To: Austin Schuh, Oliver Hartkopp, linux-can; +Cc: David Jander, Kurt Van Dijck
In-Reply-To: <CANGgnMaxNRBDG9WWAanHEem-wDJ1ujxYdqgko7nAx3iJNcPUGA@mail.gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 1520 bytes --]
On 09/19/2016 11:37 PM, Austin Schuh wrote:
>>>> What about putting them into socket depended sockopts with setsockopt()?
>>>
>>> Is it a per interface or a per socket setting?
>>
>> At least for iso-tp these kind of settings are transport channel
>> specific. Don't know if this can be applied to j1939 too.
>>
>> This is something Kurt or others can answer better than me.
>
> These parameters should be defined by the standard. For the padding,
> I'm not aware of any messages (besides the RQST message which should
> be 3 bytes long) which are not already 8 bytes. If you want to
> enforce that, I would think it would be better to refuse to accept the
> packet rather than pad it out to surprise the user less.
So an option is to check if PGN is 0xea00 then length must be 3,
otherwise length must be 8?
> The unused bits should be set to 1, but that's hard to do at the
> driver level when you don't know which bits are used and which are
> unused in a request.
The padding is done in the j1939_send() function. So padding is no
problem here.
> SAE J1939-21 defines all the timing requirements and other constraints
> pretty well. Is there a reason to make them configurable?
Dunno...Kurt?
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: 455 bytes --]
^ permalink raw reply
* Re: j1939
From: Marc Kleine-Budde @ 2016-09-20 8:01 UTC (permalink / raw)
To: David Jander; +Cc: linux-can, Kurt Van Dijck
In-Reply-To: <20160920092234.7accf5f8@erd980>
[-- Attachment #1.1: Type: text/plain, Size: 7421 bytes --]
On 09/20/2016 09:22 AM, David Jander wrote:
>> Is here someone with insight and can tell if the code is correct this way?
> Looks correct to me. I have yet to test it though.
Thanks for your input, the diff to the previous version is this:
> diff --git a/net/can/j1939/transport.c b/net/can/j1939/transport.c
> index 56bc5bf36090..5d878b5aaf0a 100644
> --- a/net/can/j1939/transport.c
> +++ b/net/can/j1939/transport.c
> @@ -979,7 +979,7 @@ static int j1939tp_txnext(struct session *session)
> j1939tp_set_rxtimeout(session, 1250);
> break;
> case tp_cmd_rts:
> - case etp_cmd_rts:
> + case etp_cmd_rts: /* fallthrough */
> if (!j1939tp_im_receiver(session->skb))
> break;
> tx_cts:
> @@ -1028,9 +1028,10 @@ static int j1939tp_txnext(struct session *session)
> j1939tp_set_rxtimeout(session, 1250);
> session->pkt.tx = session->pkt.done;
> }
> - case tp_cmd_cts:
> - case 0xff: /* did some data */
> - case etp_cmd_dpo:
> + /* fallthrough */
> + case tp_cmd_cts: /* fallthrough */
> + case 0xff: /* did some data */ /* FIXME: let David Jander recheck this */
> + case etp_cmd_dpo: /* fallthrough */
> if ((session->extd || !j1939cb_is_broadcast(session->cb)) &&
> j1939tp_im_receiver(session->skb)) {
> if (session->pkt.done >= session->pkt.total) {
> @@ -1061,6 +1062,7 @@ static int j1939tp_txnext(struct session *session)
> goto tx_cts;
> }
> }
> + break;
> case tp_cmd_bam:
> if (!j1939tp_im_transmitter(session->skb))
> break;
For completeness, the whole function:
> /* transmit function */
> static int j1939tp_txnext(struct session *session)
> {
> u8 dat[8];
> const u8 *tpdat;
> int ret, offset, pkt_done, pkt_end;
> unsigned int pkt, len, pdelay;
>
> memset(dat, 0xff, sizeof(dat));
> get_session(session); /* do not loose it */
>
> switch (session->last_cmd) {
> case 0:
> if (!j1939tp_im_transmitter(session->skb))
> break;
> dat[1] = (session->skb->len >> 0) & 0xff;
> dat[2] = (session->skb->len >> 8) & 0xff;
> dat[3] = session->pkt.total;
> if (session->extd) {
> dat[0] = etp_cmd_rts;
> dat[1] = (session->skb->len >> 0) & 0xff;
> dat[2] = (session->skb->len >> 8) & 0xff;
> dat[3] = (session->skb->len >> 16) & 0xff;
> dat[4] = (session->skb->len >> 24) & 0xff;
> } else if (j1939cb_is_broadcast(session->cb)) {
> dat[0] = tp_cmd_bam;
> /* fake cts for broadcast */
> session->pkt.tx = 0;
> } else {
> dat[0] = tp_cmd_rts;
> dat[4] = dat[3];
> }
> if (dat[0] == session->last_txcmd)
> /* done already */
> break;
> ret = j1939tp_tx_ctl(session, 0, dat);
> if (ret < 0)
> goto failed;
> session->last_txcmd = dat[0];
> /* must lock? */
> if (tp_cmd_bam == dat[0])
> j1939tp_schedule_txtimer(session, 50);
> j1939tp_set_rxtimeout(session, 1250);
> break;
> case tp_cmd_rts:
> case etp_cmd_rts: /* fallthrough */
> if (!j1939tp_im_receiver(session->skb))
> break;
> tx_cts:
> ret = 0;
> len = session->pkt.total - session->pkt.done;
> len = min(max(len, session->pkt.block), block ?: 255);
>
> if (session->extd) {
> pkt = session->pkt.done + 1;
> dat[0] = etp_cmd_cts;
> dat[1] = len;
> dat[2] = (pkt >> 0) & 0xff;
> dat[3] = (pkt >> 8) & 0xff;
> dat[4] = (pkt >> 16) & 0xff;
> } else {
> dat[0] = tp_cmd_cts;
> dat[1] = len;
> dat[2] = session->pkt.done + 1;
> }
> if (dat[0] == session->last_txcmd)
> /* done already */
> break;
> ret = j1939tp_tx_ctl(session, 1, dat);
> if (ret < 0)
> goto failed;
> if (len)
> /* only mark cts done when len is set */
> session->last_txcmd = dat[0];
> j1939tp_set_rxtimeout(session, 1250);
> break;
> case etp_cmd_cts:
> if (j1939tp_im_transmitter(session->skb) && session->extd &&
> (etp_cmd_dpo != session->last_txcmd)) {
> /* do dpo */
> dat[0] = etp_cmd_dpo;
> session->pkt.dpo = session->pkt.done;
> pkt = session->pkt.dpo;
> dat[1] = session->pkt.last - session->pkt.done;
> dat[2] = (pkt >> 0) & 0xff;
> dat[3] = (pkt >> 8) & 0xff;
> dat[4] = (pkt >> 16) & 0xff;
> ret = j1939tp_tx_ctl(session, 0, dat);
> if (ret < 0)
> goto failed;
> session->last_txcmd = dat[0];
> j1939tp_set_rxtimeout(session, 1250);
> session->pkt.tx = session->pkt.done;
> }
> /* fallthrough */
> case tp_cmd_cts: /* fallthrough */
> case 0xff: /* did some data */ /* FIXME: let David Jander recheck this */
> case etp_cmd_dpo: /* fallthrough */
> if ((session->extd || !j1939cb_is_broadcast(session->cb)) &&
> j1939tp_im_receiver(session->skb)) {
> if (session->pkt.done >= session->pkt.total) {
> if (session->extd) {
> dat[0] = etp_cmd_eof;
> dat[1] = session->skb->len >> 0;
> dat[2] = session->skb->len >> 8;
> dat[3] = session->skb->len >> 16;
> dat[4] = session->skb->len >> 24;
> } else {
> dat[0] = tp_cmd_eof;
> dat[1] = session->skb->len;
> dat[2] = session->skb->len >> 8;
> dat[3] = session->pkt.total;
> }
> if (dat[0] == session->last_txcmd)
> /* done already */
> break;
> ret = j1939tp_tx_ctl(session, 1, dat);
> if (ret < 0)
> goto failed;
> session->last_txcmd = dat[0];
> j1939tp_set_rxtimeout(session, 1250);
> /* wait for the EOF packet to come in */
> break;
> } else if (session->pkt.done >= session->pkt.last) {
> session->last_txcmd = 0;
> goto tx_cts;
> }
> }
> break;
> case tp_cmd_bam:
> if (!j1939tp_im_transmitter(session->skb))
> break;
> tpdat = session->skb->data;
> ret = 0;
> pkt_done = 0;
> pkt_end = (!session->extd && j1939cb_is_broadcast(session->cb))
> ? session->pkt.total : session->pkt.last;
>
> while (session->pkt.tx < pkt_end) {
> dat[0] = session->pkt.tx - session->pkt.dpo + 1;
> offset = session->pkt.tx * 7;
> len = session->skb->len - offset;
> if (len > 7)
> len = 7;
> memcpy(&dat[1], &tpdat[offset], len);
> ret = j1939tp_tx_dat(session->skb, session->extd,
> dat, len + 1);
> if (ret < 0)
> break;
> session->last_txcmd = 0xff;
> ++pkt_done;
> ++session->pkt.tx;
> pdelay = j1939cb_is_broadcast(session->cb) ? 50 :
> packet_delay;
> if ((session->pkt.tx < session->pkt.total) && pdelay) {
> j1939tp_schedule_txtimer(session, pdelay);
> break;
> }
> }
> if (pkt_done)
> j1939tp_set_rxtimeout(session, 250);
> if (ret)
> goto failed;
> break;
> }
> put_session(session);
> return 0;
> failed:
> put_session(session);
> return ret;
> }
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: 455 bytes --]
^ permalink raw reply
* Re: j1939
From: David Jander @ 2016-09-20 7:15 UTC (permalink / raw)
To: Austin Schuh
Cc: Oliver Hartkopp, Marc Kleine-Budde, linux-can, Kurt Van Dijck
In-Reply-To: <CANGgnMaxNRBDG9WWAanHEem-wDJ1ujxYdqgko7nAx3iJNcPUGA@mail.gmail.com>
On Mon, 19 Sep 2016 14:37:15 -0700
Austin Schuh <austin@peloton-tech.com> wrote:
> On Mon, Sep 19, 2016 at 12:45 PM Oliver Hartkopp <socketcan@hartkopp.net> wrote:
> >
> >
> >
> > On 09/19/2016 08:52 PM, Marc Kleine-Budde wrote:
> > > On 09/19/2016 08:27 PM, Oliver Hartkopp wrote:
> > >> On 09/19/2016 06:54 PM, Marc Kleine-Budde wrote:
> > >>
> > >>>
> > >>> Another thing that IMHO has to be sorted out is the use of module
> > >>> parameters:
> > >>>
> > >>>> ./main.c:49:MODULE_PARM_DESC(padding, "Pad all packets to 8 bytes, and stuff with 0xff");
This is true only for 99% of all J1939 messages. I need to investigate some
more, but I fear this option can better be removed. I couldn't think of a
situation where I'd ponder setting this parameter to true.
> > >>>> ./transport.c:60:MODULE_PARM_DESC(transport_burst_count, "Number of packets to send in burst between flow control (1..255, default 255)");
Usually you'd want this parameter to be 255, except for some special
situations, and I think this should be a per-socket setting.
> > >>>> ./transport.c:61:MODULE_PARM_DESC(transport_max_size, "Maximum packet size (default 100k)");
No idea if this limit makes any sense to the kernel driver... for the protocol
it doesn't. For ISOBus applications this limit is much too low. ISOBus VT
clients for example can easily send packets of several times that amount.
IMHO, this parameter should either be removed or set to infinite per default.
> > >>>> ./transport.c:62:MODULE_PARM_DESC(transport_retry_time, "Packet retransmission timeout in msecs, used in case of buffer full. (default 20)");
> > >>>> ./transport.c:63:MODULE_PARM_DESC(transport_packet_delay, "Delay between packets to avoid buffer overruns (default 0)");
These two parameters could be a per interface setting I believe.
> > >>>
> > >>> Better convert them to netlink options.
> > >>>
> > >>
> > >> What about putting them into socket depended sockopts with setsockopt()?
> > >
> > > Is it a per interface or a per socket setting?
> > >
> >
> > At least for iso-tp these kind of settings are transport channel
> > specific. Don't know if this can be applied to j1939 too.
> >
> > This is something Kurt or others can answer better than me.
>
> These parameters should be defined by the standard. For the padding,
> I'm not aware of any messages (besides the RQST message which should
> be 3 bytes long) which are not already 8 bytes. If you want to
> enforce that, I would think it would be better to refuse to accept the
> packet rather than pad it out to surprise the user less. The unused
> bits should be set to 1, but that's hard to do at the driver level
> when you don't know which bits are used and which are unused in a
> request.
>
> SAE J1939-21 defines all the timing requirements and other constraints
> pretty well. Is there a reason to make them configurable?
>
> Austin
Best regards,
--
David Jander
Protonic Holland.
^ permalink raw reply
* Re: j1939
From: David Jander @ 2016-09-20 7:22 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: linux-can, Kurt Van Dijck
In-Reply-To: <a6ad3606-b295-dc72-c557-098c45769cb9@pengutronix.de>
On Mon, 19 Sep 2016 18:54:43 +0200
Marc Kleine-Budde <mkl@pengutronix.de> wrote:
> Hello,
>
> I've ported Kurt's latest j1939 patches to linux-4.7. I've squashed all
> patches into one. Then a patch to undo unused modifications and a bunch
> of patches to make checkpatch happy.
>
> I've pushed the result to
>
> https://git.kernel.org/cgit/linux/kernel/git/mkl/linux-can-next.git/log/?h=j1939
>
> The remaining warning is about a case statement without fall through
> annotations and/or breaks:
>
> > WARNING: Possible switch case/default not preceeded by break or fallthrough comment
> > #4017: FILE: net/can/j1939/transport.c:1031:
> > + case tp_cmd_cts:
> >
> > WARNING: Possible switch case/default not preceeded by break or fallthrough comment
> > #4018: FILE: net/can/j1939/transport.c:1032:
> > + case 0xff: /* did some data */
> >
> > WARNING: Possible switch case/default not preceeded by break or fallthrough comment
> > #4019: FILE: net/can/j1939/transport.c:1033:
> > + case etp_cmd_dpo:
>
> The code in question is:
>
> > /* transmit function */
> > static int j1939tp_txnext(struct session *session)
> > {
> > u8 dat[8];
> > const u8 *tpdat;
> > int ret, offset, pkt_done, pkt_end;
> > unsigned int pkt, len, pdelay;
> >
> > memset(dat, 0xff, sizeof(dat));
> > get_session(session); /* do not loose it */
> >
> > switch (session->last_cmd) {
> > case 0:
> > if (!j1939tp_im_transmitter(session->skb))
> > break;
> > dat[1] = (session->skb->len >> 0) & 0xff;
> > dat[2] = (session->skb->len >> 8) & 0xff;
> > dat[3] = session->pkt.total;
> > if (session->extd) {
> > dat[0] = etp_cmd_rts;
> > dat[1] = (session->skb->len >> 0) & 0xff;
> > dat[2] = (session->skb->len >> 8) & 0xff;
> > dat[3] = (session->skb->len >> 16) & 0xff;
> > dat[4] = (session->skb->len >> 24) & 0xff;
> > } else if (j1939cb_is_broadcast(session->cb)) {
> > dat[0] = tp_cmd_bam;
> > /* fake cts for broadcast */
> > session->pkt.tx = 0;
> > } else {
> > dat[0] = tp_cmd_rts;
> > dat[4] = dat[3];
> > }
> > if (dat[0] == session->last_txcmd)
> > /* done already */
> > break;
> > ret = j1939tp_tx_ctl(session, 0, dat);
> > if (ret < 0)
> > goto failed;
> > session->last_txcmd = dat[0];
> > /* must lock? */
> > if (tp_cmd_bam == dat[0])
> > j1939tp_schedule_txtimer(session, 50);
> > j1939tp_set_rxtimeout(session, 1250);
> > break;
> > case tp_cmd_rts:
> > case etp_cmd_rts:
This fall-through looks correct.
> > if (!j1939tp_im_receiver(session->skb))
> > break;
> > tx_cts:
> > ret = 0;
> > len = session->pkt.total - session->pkt.done;
> > len = min(max(len, session->pkt.block), block ?: 255);
> >
> > if (session->extd) {
> > pkt = session->pkt.done + 1;
> > dat[0] = etp_cmd_cts;
> > dat[1] = len;
> > dat[2] = (pkt >> 0) & 0xff;
> > dat[3] = (pkt >> 8) & 0xff;
> > dat[4] = (pkt >> 16) & 0xff;
> > } else {
> > dat[0] = tp_cmd_cts;
> > dat[1] = len;
> > dat[2] = session->pkt.done + 1;
> > }
> > if (dat[0] == session->last_txcmd)
> > /* done already */
> > break;
> > ret = j1939tp_tx_ctl(session, 1, dat);
> > if (ret < 0)
> > goto failed;
> > if (len)
> > /* only mark cts done when len is set */
> > session->last_txcmd = dat[0];
> > j1939tp_set_rxtimeout(session, 1250);
> > break;
> > case etp_cmd_cts:
> > if (j1939tp_im_transmitter(session->skb) && session->extd &&
> > (etp_cmd_dpo != session->last_txcmd)) {
> > /* do dpo */
> > dat[0] = etp_cmd_dpo;
> > session->pkt.dpo = session->pkt.done;
> > pkt = session->pkt.dpo;
> > dat[1] = session->pkt.last - session->pkt.done;
> > dat[2] = (pkt >> 0) & 0xff;
> > dat[3] = (pkt >> 8) & 0xff;
> > dat[4] = (pkt >> 16) & 0xff;
> > ret = j1939tp_tx_ctl(session, 0, dat);
> > if (ret < 0)
> > goto failed;
> > session->last_txcmd = dat[0];
> > j1939tp_set_rxtimeout(session, 1250);
> > session->pkt.tx = session->pkt.done;
> > }
> > case tp_cmd_cts:
> > case 0xff: /* did some data */
> > case etp_cmd_dpo:
Also in this case, a tp_cmd_cts has almost the same function as etp_cmd_dpo,
so this should be ok. Not yet sure how the 0xff thing is supposed to work...
> > if ((session->extd || !j1939cb_is_broadcast(session->cb)) &&
> > j1939tp_im_receiver(session->skb)) {
> > if (session->pkt.done >= session->pkt.total) {
> > if (session->extd) {
> > dat[0] = etp_cmd_eof;
> > dat[1] = session->skb->len >> 0;
> > dat[2] = session->skb->len >> 8;
> > dat[3] = session->skb->len >> 16;
> > dat[4] = session->skb->len >> 24;
> > } else {
> > dat[0] = tp_cmd_eof;
> > dat[1] = session->skb->len;
> > dat[2] = session->skb->len >> 8;
> > dat[3] = session->pkt.total;
> > }
> > if (dat[0] == session->last_txcmd)
> > /* done already */
> > break;
> > ret = j1939tp_tx_ctl(session, 1, dat);
> > if (ret < 0)
> > goto failed;
> > session->last_txcmd = dat[0];
> > j1939tp_set_rxtimeout(session, 1250);
> > /* wait for the EOF packet to come in */
> > break;
> > } else if (session->pkt.done >= session->pkt.last) {
> > session->last_txcmd = 0;
> > goto tx_cts;
> > }
> > }
Here we could better have break to be clear I guess. No code paths get to this
point AFAICS, but clarity is always better IMHO.
> > case tp_cmd_bam:
> > if (!j1939tp_im_transmitter(session->skb))
> > break;
> > tpdat = session->skb->data;
> > ret = 0;
> > pkt_done = 0;
> > pkt_end = (!session->extd && j1939cb_is_broadcast(session->cb))
> > ? session->pkt.total : session->pkt.last;
> >
> > while (session->pkt.tx < pkt_end) {
> > dat[0] = session->pkt.tx - session->pkt.dpo + 1;
> > offset = session->pkt.tx * 7;
> > len = session->skb->len - offset;
> > if (len > 7)
> > len = 7;
> > memcpy(&dat[1], &tpdat[offset], len);
> > ret = j1939tp_tx_dat(session->skb, session->extd,
> > dat, len + 1);
> > if (ret < 0)
> > break;
> > session->last_txcmd = 0xff;
> > ++pkt_done;
> > ++session->pkt.tx;
> > pdelay = j1939cb_is_broadcast(session->cb) ? 50 :
> > packet_delay;
> > if ((session->pkt.tx < session->pkt.total) && pdelay) {
> > j1939tp_schedule_txtimer(session, pdelay);
> > break;
> > }
> > }
> > if (pkt_done)
> > j1939tp_set_rxtimeout(session, 250);
> > if (ret)
> > goto failed;
> > break;
Does coding-style suggest a default:break; here?
> > }
> > put_session(session);
> > return 0;
> > failed:
> > put_session(session);
> > return ret;
> > }
>
> Is here someone with insight and can tell if the code is correct this way?
Looks correct to me. I have yet to test it though.
> Another thing that IMHO has to be sorted out is the use of module
> parameters:
>
> > ./main.c:49:MODULE_PARM_DESC(padding, "Pad all packets to 8 bytes, and stuff with 0xff");
> > ./transport.c:60:MODULE_PARM_DESC(transport_burst_count, "Number of packets to send in burst between flow control (1..255, default 255)");
> > ./transport.c:61:MODULE_PARM_DESC(transport_max_size, "Maximum packet size (default 100k)");
> > ./transport.c:62:MODULE_PARM_DESC(transport_retry_time, "Packet retransmission timeout in msecs, used in case of buffer full. (default 20)");
> > ./transport.c:63:MODULE_PARM_DESC(transport_packet_delay, "Delay between packets to avoid buffer overruns (default 0)");
>
> Better convert them to netlink options.
>
> Next think I'll do is to establish some communication with an external
> j1939 component. I'll keep you updated.
Thanks!
Best regards,
--
David Jander
Protonic Holland.
^ permalink raw reply
* Re: j1939
From: Austin Schuh @ 2016-09-19 21:37 UTC (permalink / raw)
To: Oliver Hartkopp, Marc Kleine-Budde, linux-can
Cc: David Jander, Kurt Van Dijck
In-Reply-To: <1be622f4-1376-189b-9180-487aef3dc636@hartkopp.net>
On Mon, Sep 19, 2016 at 12:45 PM Oliver Hartkopp <socketcan@hartkopp.net> wrote:
>
>
>
> On 09/19/2016 08:52 PM, Marc Kleine-Budde wrote:
> > On 09/19/2016 08:27 PM, Oliver Hartkopp wrote:
> >> On 09/19/2016 06:54 PM, Marc Kleine-Budde wrote:
> >>
> >>>
> >>> Another thing that IMHO has to be sorted out is the use of module
> >>> parameters:
> >>>
> >>>> ./main.c:49:MODULE_PARM_DESC(padding, "Pad all packets to 8 bytes, and stuff with 0xff");
> >>>> ./transport.c:60:MODULE_PARM_DESC(transport_burst_count, "Number of packets to send in burst between flow control (1..255, default 255)");
> >>>> ./transport.c:61:MODULE_PARM_DESC(transport_max_size, "Maximum packet size (default 100k)");
> >>>> ./transport.c:62:MODULE_PARM_DESC(transport_retry_time, "Packet retransmission timeout in msecs, used in case of buffer full. (default 20)");
> >>>> ./transport.c:63:MODULE_PARM_DESC(transport_packet_delay, "Delay between packets to avoid buffer overruns (default 0)");
> >>>
> >>> Better convert them to netlink options.
> >>>
> >>
> >> What about putting them into socket depended sockopts with setsockopt()?
> >
> > Is it a per interface or a per socket setting?
> >
>
> At least for iso-tp these kind of settings are transport channel
> specific. Don't know if this can be applied to j1939 too.
>
> This is something Kurt or others can answer better than me.
These parameters should be defined by the standard. For the padding,
I'm not aware of any messages (besides the RQST message which should
be 3 bytes long) which are not already 8 bytes. If you want to
enforce that, I would think it would be better to refuse to accept the
packet rather than pad it out to surprise the user less. The unused
bits should be set to 1, but that's hard to do at the driver level
when you don't know which bits are used and which are unused in a
request.
SAE J1939-21 defines all the timing requirements and other constraints
pretty well. Is there a reason to make them configurable?
Austin
^ permalink raw reply
* Re: j1939
From: Oliver Hartkopp @ 2016-09-19 19:44 UTC (permalink / raw)
To: Marc Kleine-Budde, linux-can; +Cc: David Jander, Kurt Van Dijck
In-Reply-To: <9a40e851-4bc8-f57c-ffc5-478f3ea93acb@pengutronix.de>
On 09/19/2016 08:52 PM, Marc Kleine-Budde wrote:
> On 09/19/2016 08:27 PM, Oliver Hartkopp wrote:
>> On 09/19/2016 06:54 PM, Marc Kleine-Budde wrote:
>>
>>>
>>> Another thing that IMHO has to be sorted out is the use of module
>>> parameters:
>>>
>>>> ./main.c:49:MODULE_PARM_DESC(padding, "Pad all packets to 8 bytes, and stuff with 0xff");
>>>> ./transport.c:60:MODULE_PARM_DESC(transport_burst_count, "Number of packets to send in burst between flow control (1..255, default 255)");
>>>> ./transport.c:61:MODULE_PARM_DESC(transport_max_size, "Maximum packet size (default 100k)");
>>>> ./transport.c:62:MODULE_PARM_DESC(transport_retry_time, "Packet retransmission timeout in msecs, used in case of buffer full. (default 20)");
>>>> ./transport.c:63:MODULE_PARM_DESC(transport_packet_delay, "Delay between packets to avoid buffer overruns (default 0)");
>>>
>>> Better convert them to netlink options.
>>>
>>
>> What about putting them into socket depended sockopts with setsockopt()?
>
> Is it a per interface or a per socket setting?
>
At least for iso-tp these kind of settings are transport channel
specific. Don't know if this can be applied to j1939 too.
This is something Kurt or others can answer better than me.
Regards,
Oliver
^ permalink raw reply
* [PATCH] net: can: ifi: Configure transmitter delay
From: Marek Vasut @ 2016-09-19 19:34 UTC (permalink / raw)
To: linux-can
Cc: Marek Vasut, Marc Kleine-Budde, Mark Rutland, Oliver Hartkopp,
Wolfgang Grandegger
Configure the transmitter delay register at +0x1c to correctly handle
the CAN FD bitrate switch (BRS). This moves the SSP (secondary sample
point) to a proper offset, so that the TDC mechanism works and won't
generate error frames on the CAN link.
Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Marc Kleine-Budde <mkl@pengutronix.de>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Oliver Hartkopp <socketcan@hartkopp.net>
Cc: Wolfgang Grandegger <wg@grandegger.com>
---
drivers/net/can/ifi_canfd/ifi_canfd.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/net/can/ifi_canfd/ifi_canfd.c b/drivers/net/can/ifi_canfd/ifi_canfd.c
index 2d1d22e..368bb07 100644
--- a/drivers/net/can/ifi_canfd/ifi_canfd.c
+++ b/drivers/net/can/ifi_canfd/ifi_canfd.c
@@ -81,6 +81,10 @@
#define IFI_CANFD_TIME_SET_TIMEA_4_12_6_6 BIT(15)
#define IFI_CANFD_TDELAY 0x1c
+#define IFI_CANFD_TDELAY_DEFAULT 0xb
+#define IFI_CANFD_TDELAY_MASK 0x3fff
+#define IFI_CANFD_TDELAY_ABS BIT(14)
+#define IFI_CANFD_TDELAY_EN BIT(15)
#define IFI_CANFD_ERROR 0x20
#define IFI_CANFD_ERROR_TX_OFFSET 0
@@ -641,7 +645,7 @@ static void ifi_canfd_set_bittiming(struct net_device *ndev)
struct ifi_canfd_priv *priv = netdev_priv(ndev);
const struct can_bittiming *bt = &priv->can.bittiming;
const struct can_bittiming *dbt = &priv->can.data_bittiming;
- u16 brp, sjw, tseg1, tseg2;
+ u16 brp, sjw, tseg1, tseg2, tdc;
/* Configure bit timing */
brp = bt->brp - 2;
@@ -664,6 +668,11 @@ static void ifi_canfd_set_bittiming(struct net_device *ndev)
(brp << IFI_CANFD_TIME_PRESCALE_OFF) |
(sjw << IFI_CANFD_TIME_SJW_OFF_7_9_8_8),
priv->base + IFI_CANFD_FTIME);
+
+ /* Configure transmitter delay */
+ tdc = (dbt->brp * (dbt->phase_seg1 + 1)) & IFI_CANFD_TDELAY_MASK;
+ writel(IFI_CANFD_TDELAY_EN | IFI_CANFD_TDELAY_ABS | tdc,
+ priv->base + IFI_CANFD_TDELAY);
}
static void ifi_canfd_set_filter(struct net_device *ndev, const u32 id,
--
2.9.3
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox