With the introduction of CAN XL, a new can_xl_tdc_is_enabled() helper
function will be introduced later on. Rename can_tdc_is_enabled() into
can_fd_tdc_is_enabled() to make it more explicit that this helper is
meant for CAN FD.
Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Link: https://patch.msgid.link/20241112165118.586613-11-mailhol.vincent@wanadoo.fr
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
With the introduction of CAN XL, a new CAN_CTRLMODE_XL_TDC_MASK will
be introduced later on. Because CAN_CTRLMODE_TDC_MASK is not part of
the uapi, rename it to CAN_CTRLMODE_FD_TDC_MASK to make it more
explicit that this mask is meant for CAN FD.
Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Link: https://patch.msgid.link/20241112165118.586613-10-mailhol.vincent@wanadoo.fr
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
commit cfd98c838c ("can: netlink: move '=' operators back to
previous line (checkpatch fix)") inadvertently introduced a tabulation
between the IFLA_CAN_DATA_BITTIMING_CONST array index and the equal
sign.
Remove it.
Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Link: https://patch.msgid.link/20241112165118.586613-9-mailhol.vincent@wanadoo.fr
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
This is a preparation patch for the introduction of CAN XL.
CAN FD and CAN XL uses similar bittiming parameters. Add one level of
nesting for all the CAN FD parameters. Typically:
priv->can.data_bittiming;
becomes:
priv->can.fd.data_bittiming;
This way, the CAN XL equivalent (to be introduced later) would be:
priv->can.xl.data_bittiming;
Add the new struct data_bittiming_params which contains all the data
bittiming parameters, including the TDC and the callback functions.
This done, update all the CAN FD drivers to make use of the new
layout.
Acked-by: Oliver Hartkopp <socketcan@hartkopp.net>
Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Link: https://patch.msgid.link/20250501171213.2161572-2-mailhol.vincent@wanadoo.fr
[mkl: fix rcar_canfd]
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
There are 4 net namespaces involved when creating links:
- source netns - where the netlink socket resides,
- target netns - where to put the device being created,
- link netns - netns associated with the device (backend),
- peer netns - netns of peer device.
Currently, two nets are passed to newlink() callback - "src_net"
parameter and "dev_net" (implicitly in net_device). They are set as
follows, depending on netlink attributes in the request.
+------------+-------------------+---------+---------+
| peer netns | IFLA_LINK_NETNSID | src_net | dev_net |
+------------+-------------------+---------+---------+
| | absent | source | target |
| absent +-------------------+---------+---------+
| | present | link | link |
+------------+-------------------+---------+---------+
| | absent | peer | target |
| present +-------------------+---------+---------+
| | present | peer | link |
+------------+-------------------+---------+---------+
When IFLA_LINK_NETNSID is present, the device is created in link netns
first and then moved to target netns. This has some side effects,
including extra ifindex allocation, ifname validation and link events.
These could be avoided if we create it in target netns from
the beginning.
On the other hand, the meaning of src_net parameter is ambiguous. It
varies depending on how parameters are passed. It is the effective
link (or peer netns) by design, but some drivers ignore it and use
dev_net instead.
To provide more netns context for drivers, this patch packs existing
newlink() parameters, along with the source netns, link netns and peer
netns, into a struct. The old "src_net" is renamed to "net" to avoid
confusion with real source netns, and will be deprecated later. The use
of src_net are converted to params->net trivially.
Signed-off-by: Xiao Liang <shaw.leon@gmail.com>
Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>
Link: https://patch.msgid.link/20250219125039.18024-3-shaw.leon@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
-----BEGIN PGP SIGNATURE-----
iQFHBAABCgAxFiEEUEC6huC2BN0pvD5fKDiiPnotvG8FAmeA/ocTHG1rbEBwZW5n
dXRyb25peC5kZQAKCRAoOKI+ei28b7FlB/9c3ThnLVI/eezSZGE2UpvjN3eZWemk
UkTxypJ9NpJkd1UovpOGVTNjAmlN+6+yjATJm8GBpSg99cN5Y8gcu7lWnsnidopm
mkUNE+ZPPmkxe/ay1d6BfqJKzZTqkdJ6w1QlIXFfDgbRr4YaX9bn3egdtoNhq4fR
0qRt6++7V3hhhlr3x0hrhB+3nrTkL8eVBVWNEKFPCnCn9E3p7izXwZAG/y/IC4sP
swTk1BbSZg9ajds/1DE2AVeUvqz63FnzjY7YChU5WdNY5bq/cFHGoCWn8aZEX/kC
YFrQC3lonlNa/A5IFfb32HaFT0qFFbOKBHXru/wFuuwJCKVLUKsmLsd/
=c3SH
-----END PGP SIGNATURE-----
Merge tag 'linux-can-next-for-6.14-20250110' of git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next
Marc Kleine-Budde says:
====================
pull-request: can-next 2025-01-10
Pierre-Henry Moussay adds PIC64GX compatibility to the DT bindings for
Microchip's mpfs-can IP core.
The next 3 patches are by Sean Nyekjaer and target the tcan4x5x
driver. First the DT bindings is converted to DT schema, then nWKRQ
voltage selection is added to the driver.
Dario Binacchi's patch for the sun4i_can makes the driver more
consistent by adding a likely() to the driver.
Another patch by Sean Nyekjaer for the tcan4x5x driver gets rid of a
false error message.
Charan Pedumuru converts the atmel-can DT bindings to DT schema.
The next 2 patches are by Oliver Hartkopp. The first one maps Oliver's
former mail addresses to a dedicated CAN mail address. The second one
assigns net/sched/em_canid.c additionally to the CAN maintainers.
Ariel Otilibili's patch removes dead code from the CAN dev helper.
The next 3 patches are by Sean Nyekjaer and add HW standby support to
the tcan4x5x driver.
A patch by Dario Binacchi fixes the DT bindings for the st,stm32-bxcan
driver.
The last 4 patches are by Jimmy Assarsson and target the kvaser_usb
and the kvaser_pciefd driver: error statistics are improved and
CAN_CTRLMODE_BERR_REPORTING is added.
* tag 'linux-can-next-for-6.14-20250110' of git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next:
can: kvaser_pciefd: Add support for CAN_CTRLMODE_BERR_REPORTING
can: kvaser_pciefd: Update stats and state even if alloc_can_err_skb() fails
can: kvaser_usb: Add support for CAN_CTRLMODE_BERR_REPORTING
can: kvaser_usb: Update stats and state even if alloc_can_err_skb() fails
dt-bindings: can: st,stm32-bxcan: fix st,gcan property type
can: m_can: call deinit/init callback when going into suspend/resume
can: tcan4x5x: add deinit callback to set standby mode
can: m_can: add deinit callback
can: dev: can_get_state_str(): Remove dead code
MAINTAINERS: assign em_canid.c additionally to CAN maintainers
mailmap: add an entry for Oliver Hartkopp
dt-bindings: net: can: atmel: Convert to json schema
can: tcan4x5x: get rid of false clock errors
can: sun4i_can: continue to use likely() to check skb
can: tcan4x5x: add option for selecting nWKRQ voltage
dt-bindings: can: tcan4x5x: Document the ti,nwkrq-voltage-vio option
dt-bindings: can: convert tcan4x5x.txt to DT schema
dt-bindings: can: mpfs: add PIC64GX CAN compatibility
====================
Link: https://patch.msgid.link/20250110112712.3214173-1-mkl@pengutronix.de
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
In commit 6e86a1543c ("can: dev: provide optional GPIO based
termination support") GPIO based termination support was added.
For no particular reason that patch uses gpiod_set_value() to set the
GPIO. This leads to the following warning, if the systems uses a
sleeping GPIO, i.e. behind an I2C port expander:
| WARNING: CPU: 0 PID: 379 at /drivers/gpio/gpiolib.c:3496 gpiod_set_value+0x50/0x6c
| CPU: 0 UID: 0 PID: 379 Comm: ip Not tainted 6.11.0-20241016-1 #1 823affae360cc91126e4d316d7a614a8bf86236c
Replace gpiod_set_value() by gpiod_set_value_cansleep() to allow the
use of sleeping GPIOs.
Cc: Nicolai Buchwitz <nb@tipi-net.de>
Cc: Lino Sanfilippo <l.sanfilippo@kunbus.com>
Cc: stable@vger.kernel.org
Reported-by: Leonard Göhrs <l.goehrs@pengutronix.de>
Tested-by: Leonard Göhrs <l.goehrs@pengutronix.de>
Fixes: 6e86a1543c ("can: dev: provide optional GPIO based termination support")
Link: https://patch.msgid.link/20241121-dev-fix-can_set_termination-v1-1-41fa6e29216d@pengutronix.de
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
The responsibility for reporting of RX software timestamp has moved to
the core layer (see __ethtool_get_ts_info()), remove usage from the
device drivers.
Reviewed-by: Carolina Jubran <cjubran@nvidia.com>
Reviewed-by: Rahul Rameshbabu <rrameshbabu@nvidia.com>
Signed-off-by: Gal Pressman <gal@nvidia.com>
Reviewed-by: Marc Kleine-Budde <mkl@pengutronix.de>
Link: https://patch.msgid.link/20240901112803.212753-3-gal@nvidia.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
This patch moves the evaluation of data[IFLA_CAN_CTRLMODE] in function
can_changelink in front of the evaluation of data[IFLA_CAN_BITTIMING].
This avoids a call to do_set_data_bittiming providing a stale
can_priv::ctrlmode with a CAN_CTRLMODE_FD flag not matching the
requested state when switching between a CAN Classic and CAN-FD bitrate.
In the same manner the evaluation of data[IFLA_CAN_CTRLMODE] in function
can_validate is also moved in front of the evaluation of
data[IFLA_CAN_BITTIMING].
This is a preparation for patches where the nominal and data bittiming
may have interdependencies on the driver side depending on the
CAN_CTRLMODE_FD flag state.
Signed-off-by: Stefan Mätje <stefan.maetje@esd.eu>
Link: https://patch.msgid.link/20240808164224.213522-1-stefan.maetje@esd.eu
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
In prevision to add new UAPI for hwtstamp we will be limited to the struct
ethtool_ts_info that is currently passed in fixed binary format through the
ETHTOOL_GET_TS_INFO ethtool ioctl. It would be good if new kernel code
already started operating on an extensible kernel variant of that
structure, similar in concept to struct kernel_hwtstamp_config vs struct
hwtstamp_config.
Since struct ethtool_ts_info is in include/uapi/linux/ethtool.h, here
we introduce the kernel-only structure in include/linux/ethtool.h.
The manual copy is then made in the function called by ETHTOOL_GET_TS_INFO.
Acked-by: Shannon Nelson <shannon.nelson@amd.com>
Acked-by: Alexandra Winter <wintera@linux.ibm.com>
Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
Link: https://patch.msgid.link/20240709-feature_ptp_netnext-v17-6-b5317f50df2a@bootlin.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Simon reported that ndo_change_mtu() methods were never
updated to use WRITE_ONCE(dev->mtu, new_mtu) as hinted
in commit 501a90c945 ("inet: protect against too small
mtu values.")
We read dev->mtu without holding RTNL in many places,
with READ_ONCE() annotations.
It is time to take care of ndo_change_mtu() methods
to use corresponding WRITE_ONCE()
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: Simon Horman <horms@kernel.org>
Closes: https://lore.kernel.org/netdev/20240505144608.GB67882@kernel.org/
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Reviewed-by: Sabrina Dubroca <sd@queasysnail.net>
Reviewed-by: Simon Horman <horms@kernel.org>
Acked-by: Shannon Nelson <shannon.nelson@amd.com>
Link: https://lore.kernel.org/r/20240506102812.3025432-1-edumazet@google.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
The TDCO calculation was done using the currently applied data bittiming,
instead of the newly computed data bittiming, which means that the TDCO
had an invalid value unless setting the same data bittiming twice.
Fixes: d99755f71a ("can: netlink: add interface for CAN-FD Transmitter Delay Compensation (TDC)")
Signed-off-by: Maxime Jayat <maxime.jayat@mobile-devices.fr>
Reviewed-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Link: https://lore.kernel.org/all/40579c18-63c0-43a4-8d4c-f3a6c1c0b417@munic.io
Cc: stable@vger.kernel.org
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Now that napi_schedule return a bool, we can drop napi_reschedule that
does the same exact function. The function comes from a very old commit
bfe13f54f5 ("ibm_emac: Convert to use napi_struct independent of struct
net_device") and the purpose is actually deprecated in favour of
different logic.
Convert every user of napi_reschedule to napi_schedule.
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Acked-by: Jeff Johnson <quic_jjohnson@quicinc.com> # ath10k
Acked-by: Nick Child <nnac123@linux.ibm.com> # ibm
Acked-by: Marc Kleine-Budde <mkl@pengutronix.de> # for can/dev/rx-offload.c
Reviewed-by: Eric Dumazet <edumazet@google.com>
Acked-by: Tariq Toukan <tariqt@nvidia.com>
Link: https://lore.kernel.org/r/20231009133754.9834-3-ansuelsmth@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Some CAN controllers do not have a register that contains the current
CAN state, but only a register that contains the error counters.
Introduce a new function can_state_get_by_berr_counter() that returns
the current TX and RX state depending on the provided CAN bit error
counters.
Link: https://lore.kernel.org/all/20231005-at91_can-rx_offload-v2-1-9987d53600e0@pengutronix.de
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
If the "struct can_priv::echoo_skb" is accessed out of bounds, this
would cause a kernel crash. Instead, issue a meaningful warning
message and return with an error.
Fixes: a6e4bc5304 ("can: make the number of echo skb's configurable")
Link: https://lore.kernel.org/all/20231005-can-dev-fix-can-restart-v2-5-91b5c1fd922c@pengutronix.de
Reviewed-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Move the debug message "restarted" and the CAN restart stats_after_
the successful restart of the CAN device, because the restart may
fail.
While there update the error message from printing the error number to
printing symbolic error names.
Link: https://lore.kernel.org/all/20231005-can-dev-fix-can-restart-v2-4-91b5c1fd922c@pengutronix.de
Reviewed-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
[mkl: mention stats in subject and description, too]
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
This race condition was discovered while updating the at91_can driver
to use can_bus_off(). The following scenario describes how the
converted at91_can driver would behave.
When a CAN device goes into BUS-OFF state, the driver usually
stops/resets the CAN device and calls can_bus_off().
This function sets the netif carrier to off, and (if configured by
user space) schedules a delayed work that calls can_restart() to
restart the CAN device.
The can_restart() function first checks if the carrier is off and
triggers an error message if the carrier is OK.
Then it calls the driver's do_set_mode() function to restart the
device, then it sets the netif carrier to on. There is a race window
between these two calls.
The at91 CAN controller (observed on the sama5d3, a single core 32 bit
ARM CPU) has a hardware limitation. If the device goes into bus-off
while sending a CAN frame, there is no way to abort the sending of
this frame. After the controller is enabled again, another attempt is
made to send it.
If the bus is still faulty, the device immediately goes back to the
bus-off state. The driver calls can_bus_off(), the netif carrier is
switched off and another can_restart is scheduled. This occurs within
the race window before the original can_restart() handler marks the
netif carrier as OK. This would cause the 2nd can_restart() to be
called with an OK netif carrier, resulting in an error message.
The flow of the 1st can_restart() looks like this:
can_restart()
// bail out if netif_carrier is OK
netif_carrier_ok(dev)
priv->do_set_mode(dev, CAN_MODE_START)
// enable CAN controller
// sama5d3 restarts sending old message
// CAN devices goes into BUS_OFF, triggers IRQ
// IRQ handler start
at91_irq()
at91_irq_err_line()
can_bus_off()
netif_carrier_off()
schedule_delayed_work()
// IRQ handler end
netif_carrier_on()
The 2nd can_restart() will be called with an OK netif carrier and the
error message will be printed.
To close the race window, first set the netif carrier to on, then
restart the controller. In case the restart fails with an error code,
roll back the netif carrier to off.
Fixes: 39549eef35 ("can: CAN Network device driver and Netlink interface")
Link: https://lore.kernel.org/all/20231005-can-dev-fix-can-restart-v2-2-91b5c1fd922c@pengutronix.de
Reviewed-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
During testing, I triggered a can_restart() with the netif carrier
being OK [1]. The BUG_ON, which checks if the carrier is OK, results
in a fatal kernel crash. This is neither helpful for debugging nor for
a production system.
[1] The root cause is a race condition in can_restart() which will be
fixed in the next patch.
Do not crash the kernel, issue an error message instead, and continue
restarting the CAN device anyway.
Fixes: 39549eef35 ("can: CAN Network device driver and Netlink interface")
Link: https://lore.kernel.org/all/20231005-can-dev-fix-can-restart-v2-1-91b5c1fd922c@pengutronix.de
Reviewed-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Rename the rx_offload_get_echo_skb() function to
can_rx_offload_get_echo_skb_queue_timestamp(), since it inserts the
echo skb into the rx-offload queue sorted by timestamp.
This is a preparation for adding
can_rx_offload_get_echo_skb_queue_tail(), which adds the echo skb to
the end of the queue. This is intended for devices that do not support
timestamps.
Link: https://lore.kernel.org/all/20230718-gs_usb-rx-offload-v2-1-716e542d14d5@pengutronix.de
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Introduce a method to calculate the exact size in bits of a CAN(-FD)
frame with or without dynamic bitstuffing.
These are all the possible combinations taken into account:
- Classical CAN or CAN-FD
- Standard or Extended frame format
- CAN-FD CRC17 or CRC21
- Include or not intermission
Instead of doing several individual macro definitions, declare the
can_frame_bits() function-like macro. To this extent, do a full
refactoring of the length definitions.
In addition add the can_frame_bytes(). This function-like macro
replaces the existing macro:
- CAN_FRAME_OVERHEAD_SFF: can_frame_bytes(false, false, 0)
- CAN_FRAME_OVERHEAD_EFF: can_frame_bytes(false, true, 0)
- CANFD_FRAME_OVERHEAD_SFF: can_frame_bytes(true, false, 0)
- CANFD_FRAME_OVERHEAD_EFF: can_frame_bytes(true, true, 0)
Function-like macros were chosen over inline functions because they
can be used to initialize const struct fields.
The different maximum frame lengths (maximum data length, including
intermission) are as follow:
Frame type bits bytes
-------------------------------------------------------
Classic CAN SFF no bitstuffing 111 14
Classic CAN EFF no bitstuffing 131 17
Classic CAN SFF bitstuffing 135 17
Classic CAN EFF bitstuffing 160 20
CAN-FD SFF no bitstuffing 579 73
CAN-FD EFF no bitstuffing 598 75
CAN-FD SFF bitstuffing 712 89
CAN-FD EFF bitstuffing 736 92
The macro CAN_FRAME_LEN_MAX and CANFD_FRAME_LEN_MAX are kept as an
alias to, respectively, can_frame_bytes(false, true, CAN_MAX_DLEN) and
can_frame_bytes(true, true, CANFD_MAX_DLEN).
In addition to the above:
- Use ISO 11898-1:2015 definitions for the names of the CAN frame
fields.
- Include linux/bits.h for use of BITS_PER_BYTE.
- Include linux/math.h for use of mult_frac() and
DIV_ROUND_UP(). N.B: the use of DIV_ROUND_UP() is not new to this
patch, but the include was previously omitted.
- Add copyright 2023 for myself.
Suggested-by: Thomas Kopp <Thomas.Kopp@microchip.com>
Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Reviewed-by: Thomas Kopp <Thomas.Kopp@microchip.com>
Link: https://lore.kernel.org/all/20230611025728.450837-4-mailhol.vincent@wanadoo.fr
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
can_put_echo_skb() checks for the enabled IFF_ECHO flag and the
correct ETH_P type of the given skbuff. When implementing the CAN XL
support the new check for ETH_P_CANXL has been forgotten.
Fixes: fb08cba12b ("can: canxl: update CAN infrastructure for CAN XL frames")
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Link: https://lore.kernel.org/all/20230506184515.39241-1-socketcan@hartkopp.net
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Replace the netdev_err() by NL_SET_ERR_MSG_FMT() to better inform the
user about the problem. While there, use %u to print unsigned values
and improve error message a bit.
In case of an error, return -EINVAL instead of -EDOM, this corresponds
better to the actual meaning of the error value.
Link: https://lore.kernel.org/all/20230202110854.2318594-17-mkl@pengutronix.de
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
"The (Re-)Synchronization Jump Width (SJW) defines how far a
resynchronization may move the Sample Point inside the limits defined
by the Phase Buffer Segments to compensate for edge phase errors." [1]
In other words, this means that the SJW parameter controls the
tolerance of the CAN controller to frequency errors compared to other
CAN controllers.
If the user space does not provide an SJW parameter, the kernel
chooses a default value of 1. This has proven to be a good default
value for classic CAN controllers, but no longer for modern CAN-FD
controllers.
In the past there were CAN controllers like the sja1000 with a rather
limited range of bit timing parameters. For the standard bit rates
this results in the following bit timing parameters:
| Bit timing parameters for sja1000 with 8.000000 MHz ref clock
| _----+--------------=> tseg1: 1 … 16
| / / _---------=> tseg2: 1 … 8
| | | / _-----=> sjw: 1 … 4
| | | | / _-=> brp: 1 … 64 (inc: 1)
| | | | | /
| nominal | | | | | real Bitrt nom real SampP
| Bitrate TQ[ns] PrS PhS1 PhS2 SJW BRP Bitrate Error SampP SampP Error BTR0 BTR1
| 1000000 125 2 3 2 1 1 1000000 0.0% 75.0% 75.0% 0.0% 0x00 0x14
| 800000 125 3 4 2 1 1 800000 0.0% 80.0% 80.0% 0.0% 0x00 0x16
| 666666 125 4 4 3 1 1 666666 0.0% 80.0% 75.0% 6.2% 0x00 0x27
| 500000 125 6 7 2 1 1 500000 0.0% 87.5% 87.5% 0.0% 0x00 0x1c
| 250000 250 6 7 2 1 2 250000 0.0% 87.5% 87.5% 0.0% 0x01 0x1c
| 125000 500 6 7 2 1 4 125000 0.0% 87.5% 87.5% 0.0% 0x03 0x1c
| 100000 625 6 7 2 1 5 100000 0.0% 87.5% 87.5% 0.0% 0x04 0x1c
| 83333 750 6 7 2 1 6 83333 0.0% 87.5% 87.5% 0.0% 0x05 0x1c
| 50000 1250 6 7 2 1 10 50000 0.0% 87.5% 87.5% 0.0% 0x09 0x1c
| 33333 1875 6 7 2 1 15 33333 0.0% 87.5% 87.5% 0.0% 0x0e 0x1c
| 20000 3125 6 7 2 1 25 20000 0.0% 87.5% 87.5% 0.0% 0x18 0x1c
| 10000 6250 6 7 2 1 50 10000 0.0% 87.5% 87.5% 0.0% 0x31 0x1c
The attentive reader will notice that the SJW is 1 in most cases,
while the Seg2 phase is 2. Both values are given in TQ units, which in
turn is a duration in nanoseconds.
For example the 500 kbit/s configuration:
| nominal real Bitrt nom real SampP
| Bitrate TQ[ns] PrS PhS1 PhS2 SJW BRP Bitrate Error SampP SampP Error BTR0 BTR1
| 500000 125 6 7 2 1 1 500000 0.0% 87.5% 87.5% 0.0% 0x00 0x1c
the TQ is 125ns, the Phase Seg2 is "2" (== 250ns), the SJW is "1" (==
125 ns).
Looking at a more modern CAN controller like a mcp2518fd, it has wider
bit timing registers.
| Bit timing parameters for mcp251xfd with 40.000000 MHz ref clock
| _----+--------------=> tseg1: 2 … 256
| / / _---------=> tseg2: 1 … 128
| | | / _-----=> sjw: 1 … 128
| | | | / _-=> brp: 1 … 256 (inc: 1)
| | | | | /
| nominal | | | | | real Bitrt nom real SampP
| Bitrate TQ[ns] PrS PhS1 PhS2 SJW BRP Bitrate Error SampP SampP Error NBTCFG
| 500000 25 34 35 10 1 1 500000 0.0% 87.5% 87.5% 0.0% 0x00440900
The TQ is 25ns, the Phase Seg 2 is "10" (== 250ns), the SJW is "1" (==
25ns).
Since the kernel chooses a default SJW of 1 regardless of the TQ, this
leads to a much smaller SJW and thus much smaller tolerances to
frequency errors.
To maintain the same oscillator tolerances on controllers with wide
bit timing registers, select a default SJW value of Phase Seg2 / 2
unless Phase Seg 1 is less. This results in the following bit timing
parameters:
| Bit timing parameters for mcp251xfd with 40.000000 MHz ref clock
| _----+--------------=> tseg1: 2 … 256
| / / _---------=> tseg2: 1 … 128
| | | / _-----=> sjw: 1 … 128
| | | | / _-=> brp: 1 … 256 (inc: 1)
| | | | | /
| nominal | | | | | real Bitrt nom real SampP
| Bitrate TQ[ns] PrS PhS1 PhS2 SJW BRP Bitrate Error SampP SampP Error NBTCFG
| 500000 25 34 35 10 5 1 500000 0.0% 87.5% 87.5% 0.0% 0x00440904
The TQ is 25ns, the Phase Seg 2 is "10" (== 250ns), the SJW is "5" (==
125ns). Which is the same as on the sja1000 controller.
[1] http://web.archive.org/http://www.oertel-halle.de/files/cia99paper.pdf
Link: https://lore.kernel.org/all/20230202110854.2318594-15-mkl@pengutronix.de
Cc: Mark Bath <mark@baggywrinkle.co.uk>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
If the user space has supplied an invalid SJW value (greater than the
maximum SJW value), report -EINVAL instead of -ERANGE, this better
matches the actual meaning of the error value.
Additionally report an error message via netlink to the user space.
Link: https://lore.kernel.org/all/20230202110854.2318594-13-mkl@pengutronix.de
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Check each bit timing parameter first individually against their
limits and report a meaningful error message via netlink to the user
space.
In case of an error, return -EINVAL instead of -ERANGE, this
corresponds better to the actual meaning of the error value.
Link: https://lore.kernel.org/all/20230202110854.2318594-12-mkl@pengutronix.de
Suggested-by: Vincent Mailhol <vincent.mailhol@gmail.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Factor out the functionality of assigning a SJW default value into
can_sjw_set_default() and the checking the SJW limits into
can_sjw_check().
This functions will be improved and called from a different function
in the following patches.
Link: https://lore.kernel.org/all/20230202110854.2318594-11-mkl@pengutronix.de
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
This is a preparation patch.
In order to pass warning/error messages during netlink calls back to
user space, pass the extack struct down the callstack of
can_changelink(), the actual error messages will be added in the
following ptaches.
Link: https://lore.kernel.org/all/20230202110854.2318594-10-mkl@pengutronix.de
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Since commit 51c352bdbc ("netlink: add support for formatted extack
messages") formatted extack messages are supported to inform the user
space or warnings/errors during netlink calls.
Replace the netdev_err() by NL_SET_ERR_MSG_FMT() to better inform the
user about the problem. While there, use %u to print unsigned values
and improve error message a bit.
Link: https://lore.kernel.org/all/20230202110854.2318594-9-mkl@pengutronix.de
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
The sample point is a value in tenths of a percent. Meaningful values
are between 0 and 1000. Invalid values are rejected and an error
message is returned to user space via netlink.
Link: https://lore.kernel.org/all/20230202110854.2318594-8-mkl@pengutronix.de
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Implement the function can_bittiming_const_valid() to check the
validity of the specified bit timing constant. Call this function from
register_candev() to check the bit timing constants during the
registration of the CAN interface.
Link: https://lore.kernel.org/all/20230202110854.2318594-6-mkl@pengutronix.de
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
The can_fixup_bittiming() function is used to validate the
user-supplied low-level bit timing parameters and calculate the
bitrate prescaler (brp) from the requested time quanta (tq) and the
CAN clock of the controller.
can_fixup_bittiming() selects the best matching integer bit rate
prescaler, which may result in a different time quantum than the value
specified by the user.
Calculate the resulting time quantum and assign it so that the user
sees the effective time quantum.
Link: https://lore.kernel.org/all/20230202110854.2318594-4-mkl@pengutronix.de
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Commit 1c47fa6b31 ("can: dev: add a helper function to calculate the
duration of one bit") made the constant CAN_SYNC_SEG available in a
header file.
The magic number 1 in can_fixup_bittiming() represents the width of
the sync segment, replace it by CAN_SYNC_SEG to make the code more
readable.
Link: https://lore.kernel.org/all/20230202110854.2318594-3-mkl@pengutronix.de
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Commit 1c47fa6b31 ("can: dev: add a helper function to calculate the
duration of one bit") added the helper function can_bit_time().
Replace open coded variants of can_bit_time() by the helper function.
Link: https://lore.kernel.org/all/20230202110854.2318594-2-mkl@pengutronix.de
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
In commit a6d190f8c7 ("can: skb: drop tx skb if in listen only
mode") the priv->ctrlmode element is read even on virtual CAN
interfaces that do not create the struct can_priv at startup. This
out-of-bounds read may lead to CAN frame drops for virtual CAN
interfaces like vcan and vxcan.
This patch mainly reverts the original commit and adds a new helper
for CAN interface drivers that provide the required information in
struct can_priv.
Fixes: a6d190f8c7 ("can: skb: drop tx skb if in listen only mode")
Reported-by: Dariusz Stojaczyk <Dariusz.Stojaczyk@opensynergy.com>
Cc: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Cc: Max Staudt <max@enpas.org>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Acked-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Link: https://lore.kernel.org/all/20221102095431.36831-1-socketcan@hartkopp.net
Cc: stable@vger.kernel.org # 6.0.x
[mkl: patch pch_can, too]
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
- add new ETH_P_CANXL ethernet protocol type
- update skb checks for CAN XL
- add alloc_canxl_skb() which now needs a data length parameter
- introduce init_can_skb_reserve() to reduce code duplication
Acked-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Link: https://lore.kernel.org/all/20220912170725.120748-6-socketcan@hartkopp.net
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
To simplify the testing in user space all struct canfd_frame's provided by
the CAN subsystem of the Linux kernel now have the CANFD_FDF flag set in
canfd_frame::flags.
NB: Handcrafted ETH_P_CANFD frames introduced via PF_PACKET socket might
not set this bit correctly. During the check for sufficient headroom in
PF_PACKET sk_buffs the uninitialized CAN sk_buff data structures are filled.
In the case of a CAN FD frame the CANFD_FDF flag is set accordingly.
As the CAN frame content is already zero initialized in alloc_canfd_skb()
the obsolete initialization of cf->flags in the CTU CAN FD driver has been
removed as it would overwrite the already set CANFD_FDF flag.
Acked-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Link: https://lore.kernel.org/all/20220912170725.120748-4-socketcan@hartkopp.net
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Add two helpers to retrieve the data length from CAN sk_buffs and prepare
the length information to be a uint16 value for the CAN XL support.
Acked-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Link: https://lore.kernel.org/all/20220912170725.120748-3-socketcan@hartkopp.net
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Replace open coded checks for sk_buffs containing Classical CAN and
CAN FD frame structures as a preparation for CAN XL support.
With the added length check the unintended processing of CAN XL frames
having the CANXL_XLF bit set can be suppressed even when the skb->len
fits to non CAN XL frames.
The CAN_RAW socket needs a rework to use these helpers. Therefore the
use of these helpers is postponed to the CAN_RAW CAN XL integration.
The J1939 protocol gets a check for Classical CAN frames too.
Acked-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Link: https://lore.kernel.org/all/20220912170725.120748-2-socketcan@hartkopp.net
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Tools based on libpcap (such as tcpdump) expect the SIOCSHWTSTAMP
ioctl call to be supported. This is also specified in the kernel doc
[1]. The purpose of this ioctl is to toggle the hardware timestamps.
Currently, CAN devices which support hardware timestamping have those
always activated. can_eth_ioctl_hwts() is a dumb function that will
always succeed when requested to set tx_type to HWTSTAMP_TX_ON or
rx_filter to HWTSTAMP_FILTER_ALL.
[1] Kernel doc: Timestamping, section 3.1 "Hardware Timestamping
Implementation: Device Drivers"
Link: https://docs.kernel.org/networking/timestamping.html#hardware-timestamping-implementation-device-drivers
Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Link: https://lore.kernel.org/all/20220727101641.198847-9-mailhol.vincent@wanadoo.fr
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Add function can_ethtool_op_get_ts_info_hwts(). This function will be
used by CAN devices with hardware TX/RX timestamping support to
implement ethtool_ops::get_ts_info. This function does not offer
support to activate/deactivate hardware timestamps at device level nor
support the filter options (which is currently the case for all CAN
devices with hardware timestamping support).
The fact that hardware timestamp can not be deactivated at hardware
level does not impact the userland. As long as the user do not set
SO_TIMESTAMPING using a setsockopt() or ioctl(), the kernel will not
emit TX timestamps (RX timestamps will still be reproted as it is the
case currently).
Drivers which need more fine grained control remains free to implement
their own function, but we foresee that the generic function
introduced here will be sufficient for the majority.
Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Link: https://lore.kernel.org/all/20220727101641.198847-8-mailhol.vincent@wanadoo.fr
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Because of the loopback feature of socket CAN, hardware TX timestamps
are nothing else than the hardware RX timespamp of the corresponding
loopback packet. This patch simply reuses the hardware RX timestamp.
The rationale to clone this timestamp value is that existing tools
which rely of libpcap (such as tcpdump) expect support for both TX and
RX hardware timestamps in order to activate the feature (i.e. no
granular control to activate either of TX or RX hardware timestamps).
Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Link: https://lore.kernel.org/all/20220727101641.198847-7-mailhol.vincent@wanadoo.fr
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Upcoming changes on slcan driver will require you to specify a bitrate
of value -1 to prevent the open_candev() from failing but at the same
time highlighting that it is a fake value. In this case the command
`ip --details -s -s link show' would print 4294967295 as the bitrate
value. The patch change this value in 0.
Link: https://lore.kernel.org/all/20220628163137.413025-5-dario.binacchi@amarulasolutions.com
Suggested-by: Marc Kleine-Budde <mkl@pengutronix.de>
Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
Tested-by: Jeroen Hofstee <jhofstee@victronenergy.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
This patch is similar to 7e193a42c3 ("can: netlink: allow
configuring of fixed bit rates without need for do_set_bittiming
callback") but for data bit rates instead of bit rates.
Usually CAN devices support configurable data bit rates. The limits
are defined by struct can_priv::data_bittiming_const. Another way is
to implement the struct can_priv::do_set_data_bittiming callback.
If the bit rate is configured via netlink, the can_changelink()
function checks that either can_priv::data_bittiming_const or struct
can_priv::do_set_data_bittiming is implemented.
In commit 431af77925 ("can: dev: add CAN interface API for fixed
bitrates") an API for configuring bit rates on CAN interfaces that
only support fixed bit rates was added. The supported bit rates are
defined by struct can_priv::bitrate_const.
However the above mentioned commit forgot to add the struct
can_priv::data_bitrate_const to the check in can_changelink().
In order to avoid to implement a no-op can_priv::do_set_data_bittiming
callback on devices with fixed data bit rates, extend the check in
can_changelink() accordingly.
Link: https://lore.kernel.org/all/20220613143633.4151884-1-mkl@pengutronix.de
Fixes: 431af77925 ("can: dev: add CAN interface API for fixed bitrates")
Acked-by: Max Staudt <max@enpas.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Frames can be directly injected to a can driver via the packet
socket. By doing so, it is possible to reach the
net_device_ops::ndo_start_xmit function even if the driver is
configured in listen only mode.
Add a check in can_dropped_invalid_skb() to discard the skb if
CAN_CTRLMODE_LISTENONLY is set.
Link: https://lore.kernel.org/all/20220610143009.323579-8-mailhol.vincent@wanadoo.fr
Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Acked-by: Max Staudt <max@enpas.org>
Tested-by: Oliver Hartkopp <socketcan@hartkopp.net>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
The functions can_dropped_invalid_skb() and can_skb_headroom_valid()
grew a lot over the years to a point which it does not make much sense
to have them defined as static inline in header files. Move those two
functions to the .c counterpart of skb.h.
can_skb_headroom_valid()'s only caller being
can_dropped_invalid_skb(), the declaration is removed from the
header. Only can_dropped_invalid_skb() gets its symbol exported.
While doing so, do a small cleanup: add brackets around the else block
in can_dropped_invalid_skb().
Link: https://lore.kernel.org/all/20220610143009.323579-7-mailhol.vincent@wanadoo.fr
Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Reported-by: kernel test robot <lkp@intel.com>
Acked-by: Max Staudt <max@enpas.org>
Tested-by: Oliver Hartkopp <socketcan@hartkopp.net>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Only a few drivers rely on the CAN rx offload framework (as of the
writing of this patch, only four: flexcan, m_can, mcp251xfd and
ti_hecc). Split it out of can-dev and add a new config symbol:
CAN_RX_OFFLOAD.
The drivers relying on CAN rx offload are in different sub
folders. Make CAN_RX_OFFLOAD an hidden option and tag all the drivers
depending on that feature with "select CAN_RX_OFFLOAD" so that the
option gets automatically enabled if and only if one of those drivers
is chosen.
Link: https://lore.kernel.org/all/20220610143009.323579-5-mailhol.vincent@wanadoo.fr
Suggested-by: Geert Uytterhoeven <geert@linux-m68k.org>
Suggested-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Acked-by: Max Staudt <max@enpas.org>
Tested-by: Oliver Hartkopp <socketcan@hartkopp.net>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
The canonical way to select or deselect an object during compilation
is to use this pattern in the relevant Makefile:
bar-$(CONFIG_FOO) := foo.o
bittiming.c instead uses some #ifdef CONFIG_CAN_CALC_BITTIMG.
Create a new file named calc_bittiming.c with all the functions which
are conditionally compiled with CONFIG_CAN_CALC_BITTIMG and modify the
Makefile according to above pattern.
Link: https://lore.kernel.org/all/20220610143009.323579-4-mailhol.vincent@wanadoo.fr
Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Acked-by: Max Staudt <max@enpas.org>
Tested-by: Oliver Hartkopp <socketcan@hartkopp.net>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
In the next patches, the software/virtual drivers (slcan, v(x)can)
will depend on drivers/net/can/dev/skb.o.
This patch changes the scope of the can-dev module to include the
above mentioned drivers.
To do so, we reuse the menu "CAN Device Drivers" and turn it into a
configmenu using the config symbol CAN_DEV (which we released in
previous patch). Also, add a description to this new CAN_DEV
menuconfig.
The symbol CAN_DEV now only triggers the build of skb.o. For this
reasons, all the macros from linux/module.h are deported from
drivers/net/can/dev/dev.c to drivers/net/can/dev/skb.c.
Finally, drivers/net/can/dev/Makefile is adjusted accordingly.
Link: https://lore.kernel.org/all/20220610143009.323579-3-mailhol.vincent@wanadoo.fr
Suggested-by: Oliver Hartkopp <socketcan@hartkopp.net>
Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Acked-by: Max Staudt <max@enpas.org>
Tested-by: Oliver Hartkopp <socketcan@hartkopp.net>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
In the next patches, the scope of the can-dev module will grow to
engloble the software/virtual drivers (slcan, v(x)can). To this
extent, release CAN_DEV by renaming it into CAN_NETLINK. The config
symbol CAN_DEV will be reused to cover this extended scope.
The rationale for the name CAN_NETLINK is that netlink is the
predominant feature added here.
The current description only mentions platform drivers despite the
fact that this symbol is also required by "normal" devices (e.g. USB
or PCI) which do not fall under the platform devices category. The
description is updated accordingly to fix this gap.
Link: https://lore.kernel.org/all/20220610143009.323579-2-mailhol.vincent@wanadoo.fr
Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Acked-by: Max Staudt <max@enpas.org>
Tested-by: Oliver Hartkopp <socketcan@hartkopp.net>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Usually CAN devices support configurable bit rates. The limits are
defined by struct can_priv::bittiming_const. Another way is to
implement the struct can_priv::do_set_bittiming callback.
If the bit rate is configured via netlink, the can_changelink()
function checks that either can_priv::bittiming_const or struct
can_priv::do_set_bittiming is implemented.
In commit 431af77925 ("can: dev: add CAN interface API for fixed
bitrates") an API for configuring bit rates on CAN interfaces that
only support fixed bit rates was added. The supported bit rates are
defined by struct can_priv::bitrate_const.
However the above mentioned commit forgot to add the struct
can_priv::bitrate_const to the check in can_changelink().
In order to avoid to implement a no-op can_priv::do_set_bittiming
callback on devices with fixed bit rates, extend the check in
can_changelink() accordingly.
Link: https://lore.kernel.org/all/20220611144248.3924903-1-mkl@pengutronix.de
Fixes: 431af77925 ("can: dev: add CAN interface API for fixed bitrates")
Reported-by: Max Staudt <max@enpas.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Since commit 30f3b42147 ("can: mark led trigger as broken") the
CAN specific LED support was disabled and marked as BROKEN. As the
common LED support with CONFIG_LEDS_TRIGGER_NETDEV should do this work
now the code can be removed as preparation for a CAN netdevice Kconfig
rework.
Link: https://lore.kernel.org/all/20220518154527.29046-1-socketcan@hartkopp.net
Suggested-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
[mkl: remove led.h from MAINTAINERS]
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
We want to remove the weight argument from the basic version of the
netif_napi_add() call. Move all the callers in drivers/net/can that
pass a custom weight (i.e. not NAPI_POLL_WEIGHT or 64) to the
netif_napi_add_weight() API.
Link: https://lore.kernel.org/all/20220517002345.1812104-1-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
The CiA (CAN in Automation) lists in their Newsletter 1/2018 in the
"Recommendation for the CAN FD bit-timing" [1] article several
recommendations, one of them is:
| Recommendation 3: Choose BRPA and BRPD as low as possible
[1] https://can-newsletter.org/uploads/media/raw/f6a36d1461371a2f86ef0011a513712c.pdf
With the current bit timing algorithm Srinivas Neeli noticed that on
the Xilinx Versal ACAP board the CAN data bit timing parameters are
not calculated optimally. For most bit rates, the bit rate
prescaler (BRP) is != 1, although it's possible to configure the
requested with a bit rate with a prescaler of 1:
| Data Bit timing parameters for xilinx_can_fd2i with 79.999999 MHz ref clock (cmd-line) using algo 'v4.8'
| nominal real Bitrt nom real SampP
| Bitrate TQ[ns] PrS PhS1 PhS2 SJW BRP Bitrate Error SampP SampP Error
| 12000000 12 2 2 2 1 1 11428571 4.8% 75.0% 71.4% 4.8%
| 10000000 25 1 1 1 1 2 9999999 0.0% 75.0% 75.0% 0.0%
| 8000000 12 3 3 3 1 1 7999999 0.0% 75.0% 70.0% 6.7%
| 5000000 50 1 1 1 1 4 4999999 0.0% 75.0% 75.0% 0.0%
| 4000000 62 1 1 1 1 5 3999999 0.0% 75.0% 75.0% 0.0%
| 2000000 125 1 1 1 1 10 1999999 0.0% 75.0% 75.0% 0.0%
| 1000000 250 1 1 1 1 20 999999 0.0% 75.0% 75.0% 0.0%
The bit timing parameter calculation algorithm iterates effectively
from low to high BRP values. It selects a new best parameter set, if
the sample point error of the current parameter set is equal or less
to old best parameter set.
If the given hardware constraints (clock rate and bit timing parameter
constants) don't allow a sample point error of 0, the algorithm will
first find a valid bit timing parameter set with a low BRP, but then
will accept parameter sets with higher BRPs that have the same sample
point error.
This patch changes the algorithm to only accept a new parameter set,
if the resulting sample point error is lower. This leads to the
following data bit timing parameter for the Versal ACAP board:
| Data Bit timing parameters for xilinx_can_fd2i with 79.999999 MHz ref clock (cmd-line) using algo 'can-next'
| nominal real Bitrt nom real SampP
| Bitrate TQ[ns] PrS PhS1 PhS2 SJW BRP Bitrate Error SampP SampP Error
| 12000000 12 2 2 2 1 1 11428571 4.8% 75.0% 71.4% 4.8%
| 10000000 12 2 3 2 1 1 9999999 0.0% 75.0% 75.0% 0.0%
| 8000000 12 3 3 3 1 1 7999999 0.0% 75.0% 70.0% 6.7%
| 5000000 12 5 6 4 1 1 4999999 0.0% 75.0% 75.0% 0.0%
| 4000000 12 7 7 5 1 1 3999999 0.0% 75.0% 75.0% 0.0%
| 2000000 12 14 15 10 1 1 1999999 0.0% 75.0% 75.0% 0.0%
| 1000000 25 14 15 10 1 2 999999 0.0% 75.0% 75.0% 0.0%
Note: Due to HW constraints a data bit rate of 1 MBit/s with BRP = 1 is not possible.
Link: https://lore.kernel.org/all/20220318144913.873614-1-mkl@pengutronix.de
Link: https://lore.kernel.org/all/20220113203004.jf2rqj2pirhgx72i@pengutronix.de
Cc: Srinivas Neeli <sneeli@xilinx.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
This patch renames the function can_rx_offload_queue_sorted() to
can_rx_offload_queue_timestamp(). This better describes what the
function does, it adds a newly RX'ed skb to the sorted queue by its
timestamp.
Link: https://lore.kernel.org/all/20220417194327.2699059-1-mkl@pengutronix.de
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Since commit
baebdf48c3 ("net: dev: Makes sure netif_rx() can be invoked in any context.")
the function netif_rx() can be used in preemptible/thread context as
well as in interrupt context.
Use netif_rx().
Cc: Marc Kleine-Budde <mkl@pengutronix.de>
Cc: Oliver Hartkopp <socketcan@hartkopp.net>
Cc: Wolfgang Grandegger <wg@grandegger.com>
Cc: linux-can@vger.kernel.org
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Acked-by: Oliver Hartkopp <socketcan@hartkopp.net>
Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>
Signed-off-by: David S. Miller <davem@davemloft.net>
This patch simplifies the validation of the fixed bit rates. If a
supported bit rate is found, directly return 0.
If no valid bit rate is found return -EINVAL;
Link: https://lore.kernel.org/all/20220124215642.3474154-6-mkl@pengutronix.de
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Currently, the CAN netlink interface provides no easy ways to check
the capabilities of a given controller. The only method from the
command line is to try each CAN_CTRLMODE_* individually to check
whether the netlink interface returns an -EOPNOTSUPP error or not
(alternatively, one may find it easier to directly check the source
code of the driver instead...)
This patch introduces a method for the user to check both the
supported and the static capabilities. The proposed method introduces
a new IFLA nest: IFLA_CAN_CTRLMODE_EXT which extends the current
IFLA_CAN_CTRLMODE. This is done to guaranty a full forward and
backward compatibility between the kernel and the user land
applications.
The IFLA_CAN_CTRLMODE_EXT nest contains one single entry:
IFLA_CAN_CTRLMODE_SUPPORTED. Because this entry is only used in one
direction: kernel to userland, no new struct nla_policy are
introduced.
Below table explains how IFLA_CAN_CTRLMODE_SUPPORTED (hereafter:
"supported") and can_ctrlmode::flags (hereafter: "flags") allow us to
identify both the supported and the static capabilities, when masked
with any of the CAN_CTRLMODE_* bit flags:
supported & flags & Controller capabilities
CAN_CTRLMODE_* CAN_CTRLMODE_*
-----------------------------------------------------------------------
false false Feature not supported (always disabled)
false true Static feature (always enabled)
true false Feature supported but disabled
true true Feature supported and enabled
Link: https://lore.kernel.org/all/20211213160226.56219-5-mailhol.vincent@wanadoo.fr
Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
The statically enabled features of a CAN controller can be retrieved
using below formula:
| u32 ctrlmode_static = priv->ctrlmode & ~priv->ctrlmode_supported;
As such, there is no need to store this information. This patch remove
the field ctrlmode_static of struct can_priv and provides, in
replacement, the inline function can_get_static_ctrlmode() which
returns the same value.
Link: https://lore.kernel.org/all/20211213160226.56219-2-mailhol.vincent@wanadoo.fr
Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
The actual payload length of the CAN Remote Transmission Request (RTR)
frames is always 0, i.e. no payload is transmitted on the wire.
However, those RTR frames still use the DLC to indicate the length of
the requested frame.
As such, net_device_stats::rx_bytes should not be increased for the
RTR frames.
This patch fixes all the CAN drivers.
Link: https://lore.kernel.org/all/20211207121531.42941-5-mailhol.vincent@wanadoo.fr
Cc: Marc Kleine-Budde <mkl@pengutronix.de>
Cc: Nicolas Ferre <nicolas.ferre@microchip.com>
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Ludovic Desroches <ludovic.desroches@microchip.com>
Cc: Chandrasekar Ramakrishnan <rcsekar@samsung.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Chen-Yu Tsai <wens@csie.org>
Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
Cc: Yasushi SHOJI <yashi@spacecubics.com>
Cc: Appana Durga Kedareswara rao <appana.durga.rao@xilinx.com>
Cc: Naga Sureshkumar Relli <naga.sureshkumar.relli@xilinx.com>
Cc: Michal Simek <michal.simek@xilinx.com>
Cc: Stephane Grosjean <s.grosjean@peak-system.com>
Tested-by: Jimmy Assarsson <extja@kvaser.com> # kvaser
Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Acked-by: Stefan Mätje <stefan.maetje@esd.eu> # esd_usb2
Tested-by: Stefan Mätje <stefan.maetje@esd.eu> # esd_usb2
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
The CAN error message frames (i.e. error skb) are an interface
specific to socket CAN. The payload of the CAN error message frames
does not correspond to any actual data sent on the wire. Only an error
flag and a delimiter are transmitted when an error occurs (c.f. ISO
11898-1 section 10.4.4.2 "Error flag").
For this reason, it makes no sense to increment the rx_packets and
rx_bytes fields of struct net_device_stats because no actual payload
were transmitted on the wire.
This patch fixes all the CAN drivers.
Link: https://lore.kernel.org/all/20211207121531.42941-2-mailhol.vincent@wanadoo.fr
CC: Marc Kleine-Budde <mkl@pengutronix.de>
CC: Nicolas Ferre <nicolas.ferre@microchip.com>
CC: Alexandre Belloni <alexandre.belloni@bootlin.com>
CC: Ludovic Desroches <ludovic.desroches@microchip.com>
CC: Chandrasekar Ramakrishnan <rcsekar@samsung.com>
CC: Maxime Ripard <mripard@kernel.org>
CC: Chen-Yu Tsai <wens@csie.org>
CC: Jernej Skrabec <jernej.skrabec@gmail.com>
CC: Appana Durga Kedareswara rao <appana.durga.rao@xilinx.com>
CC: Naga Sureshkumar Relli <naga.sureshkumar.relli@xilinx.com>
CC: Michal Simek <michal.simek@xilinx.com>
CC: Stephane Grosjean <s.grosjean@peak-system.com>
Tested-by: Jimmy Assarsson <extja@kvaser.com> # kvaser
Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Acked-by: Stefan Mätje <stefan.maetje@esd.eu> # esd_usb2
Tested-by: Stefan Mätje <stefan.maetje@esd.eu> # esd_usb2
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
In [1], we introduced a set of units in linux/can/bittiming.h. Since
then, generic SI prefixes were added to linux/units.h in [2]. Those
new prefixes can perfectly replace CAN specific ones.
This patch replaces all occurrences of the CAN units with their
corresponding prefix (from linux/units) and the unit (as a comment)
according to below table.
CAN units SI metric prefix (from linux/units) + unit (as a comment)
------------------------------------------------------------------------
CAN_KBPS KILO /* BPS */
CAN_MBPS MEGA /* BPS */
CAM_MHZ MEGA /* Hz */
The definition are then removed from linux/can/bittiming.h
[1] commit 1d7750760b ("can: bittiming: add CAN_KBPS, CAN_MBPS and
CAN_MHZ macros")
[2] commit 26471d4a6c ("units: Add SI metric prefix definitions")
Link: https://lore.kernel.org/all/20211124014536.782550-1-mailhol.vincent@wanadoo.fr
Suggested-by: Jimmy Assarsson <extja@kvaser.com>
Suggested-by: Oliver Hartkopp <socketcan@hartkopp.net>
Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Some CAN device can measure the TDCV (Transmission Delay Compensation
Value) automatically for each transmitted CAN frames.
A callback function do_get_auto_tdcv() is added to retrieve that
value. This function is used only if CAN_CTRLMODE_TDC_AUTO is enabled
(if CAN_CTRLMODE_TDC_MANUAL is selected, the TDCV value is provided by
the user).
If the device does not support reporting of TDCV, do_get_auto_tdcv()
should be set to NULL and TDCV will not be reported by the netlink
interface.
On success, do_get_auto_tdcv() shall return 0. If the value can not be
measured by the device, for example because network is down or because
no frames were transmitted yet, can_priv::do_get_auto_tdcv() shall
return a negative error code (e.g. -EINVAL) to signify that the value
is not yet available. In such cases, TDCV is not reported by the
netlink interface.
Link: https://lore.kernel.org/all/20210918095637.20108-6-mailhol.vincent@wanadoo.fr
CC: Stefan Mätje <stefan.maetje@esd.eu>
Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Add the netlink interface for TDC parameters of struct can_tdc_const
and can_tdc.
Contrary to the can_bittiming(_const) structures for which there is
just a single IFLA_CAN(_DATA)_BITTMING(_CONST) entry per structure,
here, we create a nested entry IFLA_CAN_TDC. Within this nested entry,
additional IFLA_CAN_TDC_TDC* entries are added for each of the TDC
parameters of the newly introduced struct can_tdc_const and struct
can_tdc.
For struct can_tdc_const, these are:
IFLA_CAN_TDC_TDCV_MIN
IFLA_CAN_TDC_TDCV_MAX
IFLA_CAN_TDC_TDCO_MIN
IFLA_CAN_TDC_TDCO_MAX
IFLA_CAN_TDC_TDCF_MIN
IFLA_CAN_TDC_TDCF_MAX
For struct can_tdc, these are:
IFLA_CAN_TDC_TDCV
IFLA_CAN_TDC_TDCO
IFLA_CAN_TDC_TDCF
This is done so that changes can be applied in the future to the
structures without breaking the netlink interface.
The TDC netlink logic works as follow:
* CAN_CTRLMODE_FD is not provided:
- if any TDC parameters are provided: error.
- TDC parameters not provided: TDC parameters unchanged.
* CAN_CTRLMODE_FD is provided and is false:
- TDC is deactivated: both the structure and the
CAN_CTRLMODE_TDC_{AUTO,MANUAL} flags are flushed.
* CAN_CTRLMODE_FD provided and is true:
- CAN_CTRLMODE_TDC_{AUTO,MANUAL} and tdc{v,o,f} not provided: call
can_calc_tdco() to automatically decide whether TDC should be
activated and, if so, set CAN_CTRLMODE_TDC_AUTO and uses the
calculated tdco value.
- CAN_CTRLMODE_TDC_AUTO and tdco provided: set
CAN_CTRLMODE_TDC_AUTO and use the provided tdco value. Here,
tdcv is illegal and tdcf is optional.
- CAN_CTRLMODE_TDC_MANUAL and both of tdcv and tdco provided: set
CAN_CTRLMODE_TDC_MANUAL and use the provided tdcv and tdco
value. Here, tdcf is optional.
- CAN_CTRLMODE_TDC_{AUTO,MANUAL} are mutually exclusive. Whenever
one flag is turned on, the other will automatically be turned
off. Providing both returns an error.
- Combination other than the one listed above are illegal and will
return an error.
N.B. above rules mean that whenever CAN_CTRLMODE_FD is provided, the
previous TDC values will be overwritten. The only option to reuse
previous TDC value is to not provide CAN_CTRLMODE_FD.
All the new parameters are defined as u32. This arbitrary choice is
done to mimic the other bittiming values with are also all of type
u32. An u16 would have been sufficient to hold the TDC values.
This patch completes below series (c.f. [1]):
- commit 289ea9e4ae ("can: add new CAN FD bittiming parameters:
Transmitter Delay Compensation (TDC)")
- commit c25cc79932 ("can: bittiming: add calculation for CAN FD
Transmitter Delay Compensation (TDC)")
[1] https://lore.kernel.org/linux-can/20210224002008.4158-1-mailhol.vincent@wanadoo.fr/T/#t
Link: https://lore.kernel.org/all/20210918095637.20108-5-mailhol.vincent@wanadoo.fr
Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
The function can_calc_tdco() directly retrieves can_priv from the
net_device and directly modifies it.
This is annoying for the upcoming patch. In
drivers/net/can/dev/netlink.c:can_changelink(), the data bittiming are
written to a temporary structure and memcpyed to can_priv only after
everything succeeded. In the next patch, where we will introduce the
netlink interface for TDC parameters, we will add a new TDC block
which can potentially fail. For this reason, the data bittiming
temporary structure has to be copied after that to-be-introduced TDC
block. However, TDC also needs to access data bittiming information.
We change the prototype so that the data bittiming structure is passed
to can_calc_tdco() as an argument instead of retrieving it from
priv. This way can_calc_tdco() can access the data bittiming before it
gets memcpyed to priv.
Link: https://lore.kernel.org/all/20210918095637.20108-4-mailhol.vincent@wanadoo.fr
Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
In the current implementation, all Transmission Delay Compensation
(TDC) parameters are expressed in time quantum. However, ISO 11898-1
actually specifies that these should be expressed in *minimum* time
quantum.
Furthermore, the minimum time quantum is specified to be "one node
clock period long" (c.f. paragraph 11.3.1.1 "Bit time"). For sake of
simplicity, we prefer to use the "clock period" term instead of
"minimum time quantum" because we believe that it is more broadly
understood.
This patch fixes that discrepancy by updating the documentation and
the formula for TDCO calculation.
N.B. In can_calc_tdco(), the sample point (in time quantum) was
calculated using a division, thus introducing a risk of rounding and
truncation errors. On top of changing the unit to clock period, we
also modified the formula to use only additions.
Link: https://lore.kernel.org/all/20210918095637.20108-3-mailhol.vincent@wanadoo.fr
Suggested-by: Stefan Mätje <Stefan.Maetje@esd.eu>
Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
ISO 11898-1 specifies in section 11.3.3 "Transmitter delay
compensation" that "the configuration range for [the] SSP position
shall be at least 0 to 63 minimum time quanta."
Because SSP = TDCV + TDCO, it means that we should allow both TDCV and
TDCO to hold zero value in order to honor SSP's minimum possible
value.
However, current implementation assigned special meaning to TDCV and
TDCO's zero values:
* TDCV = 0 -> TDCV is automatically measured by the transceiver.
* TDCO = 0 -> TDC is off.
In order to allow for those values to really be zero and to maintain
current features, we introduce two new flags:
* CAN_CTRLMODE_TDC_AUTO indicates that the controller support
automatic measurement of TDCV.
* CAN_CTRLMODE_TDC_MANUAL indicates that the controller support
manual configuration of TDCV. N.B.: current implementation failed
to provide an option for the driver to indicate that only manual
mode was supported.
TDC is disabled if both CAN_CTRLMODE_TDC_AUTO and
CAN_CTRLMODE_TDC_MANUAL flags are off, c.f. the helper function
can_tdc_is_enabled() which is also introduced in this patch.
Also, this patch adds three fields: tdcv_min, tdco_min and tdcf_min to
struct can_tdc_const. While we are not convinced that those three
fields could be anything else than zero, we can imagine that some
controllers might specify a lower bound on these. Thus, those minimums
are really added "just in case".
Comments of struct can_tdc and can_tdc_const are updated accordingly.
Finally, the changes are applied to the etas_es58x driver.
Link: https://lore.kernel.org/all/20210918095637.20108-2-mailhol.vincent@wanadoo.fr
Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
The sanity checks on the control modes will reject any request related
to an unsupported features, even turning it off.
Example on an interface which does not support CAN-FD:
$ ip link set can0 type can bitrate 500000 fd off
RTNETLINK answers: Operation not supported
This patch lets such command go through (but requests to turn on an
unsupported feature are, of course, still denied).
Link: https://lore.kernel.org/r/20210815033248.98111-2-mailhol.vincent@wanadoo.fr
Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
For CAN buses to work, a termination resistor has to be present at both
ends of the bus. This resistor is usually 120 Ohms, other values may be
required for special bus topologies.
This patch adds support for a generic GPIO based CAN termination. The
resistor value has to be specified via device tree, and it can only be
attached to or detached from the bus. By default the termination is not
active.
Link: https://lore.kernel.org/r/20210818071232.20585-4-o.rempel@pengutronix.de
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
can_validate() does a first check:
| if (is_can_fd) {
| if (!data[IFLA_CAN_BITTIMING] || !data[IFLA_CAN_DATA_BITTIMING])
| return -EOPNOTSUPP;
| }
If that first if succeeds, we know that if is_can_fd is true then
data[IFLA_CAN_BITTIMING is set.
However, the next if switch does not leverage on above knowledge and
redoes the check:
| if (data[IFLA_CAN_DATA_BITTIMING]) {
| if (!is_can_fd || !data[IFLA_CAN_BITTIMING])
| ^~~~~~~~~~~~~~~~~~~~~~~~
| return -EOPNOTSUPP;
| }
This patch removes that redundant check.
Link: https://lore.kernel.org/r/20210603151550.140727-2-mailhol.vincent@wanadoo.fr
Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
When the FD is turned off through the netlink interface, the data bit
timing values still remain in data_bittiming and are displayed despite
of the feature being disabled.
Example:
| $ ip link set can0 type can bitrate 500000 dbitrate 2000000 fd on
| $ ip --details link show can0
| 1: can0: <NOARP,ECHO> mtu 72 qdisc pfifo_fast state DOWN mode DEFAULT group default qlen 10
| link/can promiscuity 0 minmtu 0 maxmtu 0
| can <FD> state STOPPED restart-ms 0
| bitrate 500000 sample-point 0.875
| tq 12 prop-seg 69 phase-seg1 70 phase-seg2 20 sjw 1
| ES582.1/ES584.1: tseg1 2..256 tseg2 2..128 sjw 1..128 brp 1..512 brp-inc 1
| dbitrate 2000000 dsample-point 0.750
| dtq 12 dprop-seg 14 dphase-seg1 15 dphase-seg2 10 dsjw 1
| ES582.1/ES584.1: dtseg1 2..32 dtseg2 1..16 dsjw 1..8 dbrp 1..32 dbrp-inc 1
| clock 80000000 numtxqueues 1 numrxqueues 1 gso_max_size 65536 gso_max_segs 65535
|
| $ ip link set can0 type can bitrate 500000 fd off
| $ ip --details link show can0
| 1: can0: <NOARP,ECHO> mtu 16 qdisc pfifo_fast state DOWN mode DEFAULT group default qlen 10
| link/can promiscuity 0 minmtu 0 maxmtu 0
| can state STOPPED restart-ms 0
| bitrate 500000 sample-point 0.875
| tq 12 prop-seg 69 phase-seg1 70 phase-seg2 20 sjw 1
| ES582.1/ES584.1: tseg1 2..256 tseg2 2..128 sjw 1..128 brp 1..512 brp-inc 1
| dbitrate 2000000 dsample-point 0.750
| dtq 12 dprop-seg 14 dphase-seg1 15 dphase-seg2 10 dsjw 1
| ES582.1/ES584.1: dtseg1 2..32 dtseg2 1..16 dsjw 1..8 dbrp 1..32 dbrp-inc 1
| clock 80000000 numtxqueues 1 numrxqueues 1 gso_max_size 65536 gso_max_segs 65535
Remark: once FD is turned off, it is not possible to turn fd back on
and reuse the previously input data bit timing values:
| $ ip link set can0 type can bitrate 500000 fd on
| RTNETLINK answers: Operation not supported
This means that the user will need to re-configure the data bit timing
in order to turn fd on again.
Because old data bit timing values cannot be reused, this patch clears
priv->data_bit timing whenever FD is turned off. This way, the data
bit timing variables are not displayed anymore.
Link: https://lore.kernel.org/r/20210618081904.141114-2-mailhol.vincent@wanadoo.fr
Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
After reading all CAN frames from the controller in the IRQ handler
and storing them into a skb_queue, the driver calls napi_schedule().
In the napi poll function the skb from the skb_queue are then pushed
into the networking stack.
However if napi_schedule() is called from a threaded IRQ handler this
triggers the following error:
| NOHZ tick-stop error: Non-RCU local softirq work is pending, handler #08!!!
To avoid this, create a new rx-offload
function (can_rx_offload_threaded_irq_finish()) with a call to
local_bh_disable()/local_bh_enable() around the napi_schedule() call.
Convert all drivers that call can_rx_offload_irq_finish() from
threaded IRQ context to can_rx_offload_threaded_irq_finish().
Link: https://lore.kernel.org/r/20210724204745.736053-4-mkl@pengutronix.de
Suggested-by: Daniel Glöckner <dg@emlix.com>
Tested-by: Oleksij Rempel <o.rempel@pengutronix.de>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Instead of calling can_rx_offload_schedule() call napi_schedule()
directly. As this was the last use of can_rx_offload_schedule() remove
this helper function.
Link: https://lore.kernel.org/r/20210724204745.736053-3-mkl@pengutronix.de
Tested-by: Oleksij Rempel <o.rempel@pengutronix.de>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Adding a skb to the skb_queue in rx-offload requires to take a lock.
This commit avoids this by adding an unlocked skb queue that is
appended at the end of the ISR. Having one lock at the end of the ISR
should be OK as the HW is empty, not about to overflow.
Link: https://lore.kernel.org/r/20210724204745.736053-2-mkl@pengutronix.de
Tested-by: Oleksij Rempel <o.rempel@pengutronix.de>
Co-developed-by: Kurt Van Dijck <dev.kurt@vandijck-laurijssen.be>
Signed-off-by: Kurt Van Dijck <dev.kurt@vandijck-laurijssen.be>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
The handling of CAN bus errors typically consist of allocating a CAN
error SKB using alloc_can_err_skb() followed by stats handling and
filling the error details in the newly allocated CAN error SKB. Even
if the allocation of the SKB fails the stats handling should not be
skipped.
The common pattern in CAN drivers is to allocate the skb and work on
the struct can_frame pointer "cf", if it has been assigned by
alloc_can_err_skb().
| skb = alloc_can_err_skb(priv->ndev, &cf);
|
| /* RX errors */
| if (bdiag1 & (MCP251XFD_REG_BDIAG1_DCRCERR |
| MCP251XFD_REG_BDIAG1_NCRCERR)) {
| netdev_dbg(priv->ndev, "CRC error\n");
|
| stats->rx_errors++;
| if (cf)
| cf->data[3] |= CAN_ERR_PROT_LOC_CRC_SEQ;
| }
In case of an OOM alloc_can_err_skb() returns NULL, but doesn't set
"cf" to NULL as well. For the above pattern to work the "cf" has to be
initialized to NULL, which is easily forgotten.
To solve this kind of problems, set "cf" to NULL if
alloc_can_err_skb() returns NULL.
Link: https://lore.kernel.org/r/20210402102245.1512583-1-mkl@pengutronix.de
Suggested-by: Vincent MAILHOL <mailhol.vincent@wanadoo.fr>
Reviewed-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Add three macro to simplify the readability of big bit timing numbers:
- CAN_KBPS: kilobits per second (one thousand)
- CAN_MBPS: megabits per second (one million)
- CAN_MHZ: megahertz per second (one million)
Example:
u32 bitrate_max = 8 * CAN_MBPS;
struct can_clock clock = {.freq = 80 * CAN_MHZ};
instead of:
u32 bitrate_max = 8000000;
struct can_clock clock = {.freq = 80000000};
Apply the new macro to driver/net/can/dev/bittiming.c.
Link: https://lore.kernel.org/r/20210306054040.76483-1-mailhol.vincent@wanadoo.fr
Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
The logic for the tdco calculation is to just reuse the normal sample
point: tdco = sp. Because the sample point is expressed in tenth of
percent and the tdco is expressed in time quanta, a conversion is
needed.
At the end,
ssp = tdcv + tdco
= tdcv + sp.
Another popular method is to set tdco to the middle of the bit:
tdc->tdco = can_bit_time(dbt) / 2
During benchmark tests, we could not find a clear advantages for one
of the two methods.
The tdco calculation is triggered each time the data_bittiming is
changed so that users relying on automated calculation can use the
netlink interface the exact same way without need of new parameters.
For example, a command such as:
ip link set canX type can bitrate 500000 dbitrate 4000000 fd on
would trigger the calculation.
The user using CONFIG_CAN_CALC_BITTIMING who does not want automated
calculation needs to manually set tdco to zero.
For example with:
ip link set canX type can tdco 0 bitrate 500000 dbitrate 4000000 fd on
(if the tdco parameter is provided in a previous command, it will be
overwritten).
If tdcv is set to zero (default), it is automatically calculated by
the transiver for each frame. As such, there is no code in the kernel
to calculate it.
tdcf has no automated calculation functions because we could not
figure out a formula for this parameter.
Link: https://lore.kernel.org/r/20210224002008.4158-6-mailhol.vincent@wanadoo.fr
Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Fix the warning triggered by having an '=' at the beginning of the
line by moving it back to the previous line. Also replace all
indentations with a single space so that future entries can be more
easily added.
Extract of ./scripts/checkpatch.pl -f drivers/net/can/dev/netlink.c:
CHECK: Assignment operator '=' should be on the previous line
+ [IFLA_CAN_BITTIMING_CONST]
+ = { .len = sizeof(struct can_bittiming_const) },
CHECK: Assignment operator '=' should be on the previous line
+ [IFLA_CAN_DATA_BITTIMING]
+ = { .len = sizeof(struct can_bittiming) },
CHECK: Assignment operator '=' should be on the previous line
+ [IFLA_CAN_DATA_BITTIMING_CONST]
+ = { .len = sizeof(struct can_bittiming_const) },
Link: https://lore.kernel.org/r/20210224002008.4158-4-mailhol.vincent@wanadoo.fr
Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
In order to implement byte queue limits (bql) in CAN drivers, the
length of the CAN frame needs to be passed into the networking stack
even if the transmission failed for some reason.
To avoid to calculate this length twice, extend can_free_echo_skb() to
return that value. Convert all users of this function, too.
This patch is the natural extension of commit:
| 9420e1d495 ("can: dev: can_get_echo_skb(): extend to return can
| frame length")
Link: https://lore.kernel.org/r/20210319142700.305648-3-mkl@pengutronix.de
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
A out of bounds access to "struct can_priv::echo_skb" leads to a
kernel crash. Better print a sensible warning message instead and try
to recover.
This patch is similar to:
| e7a6994d04 ("can: dev: __can_get_echo_skb(): Don't crash the kernel
| if can_priv::echo_skb is accessed out of bounds")
Link: https://lore.kernel.org/r/20210319142700.305648-2-mkl@pengutronix.de
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
So far the creation of the TX echo skb was optional and can be
controlled by the local sender of a CAN frame.
It turns out that the TX echo CAN skb can be piggybacked to carry
information in the driver from the TX- to the TX-complete handler.
Several drivers already use the return value of
can_get_echo_skb() (which is the length of the data field in the CAN
frame) for their number of transferred bytes statistics. The
statistics are not working if CAN echo skbs are disabled.
Another use case is to calculate and set the CAN frame length on the
wire, which is needed for BQL support in both the TX and TX-completion
handler.
For now in can_put_echo_skb(), which is called from the TX handler,
the skb carrying the CAN frame is discarded if no TX echo is
requested, leading to the above illustrated problems.
This patch changes the can_put_echo_skb() function, so that the echo
skb is always generated. If the sender requests no echo, the echo skb
is consumed in __can_get_echo_skb() without being passed into the RX
handler of the networking stack, but the CAN data length and CAN frame
length information is properly returned.
Link: https://lore.kernel.org/r/20210309211904.3348700-1-mkl@pengutronix.de
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
When a non-initial netns is destroyed, the usual policy is to delete
all virtual network interfaces contained, but move physical interfaces
back to the initial netns. This keeps the physical interface visible
on the system.
CAN devices are somewhat special, as they define rtnl_link_ops even
if they are physical devices. If a CAN interface is moved into a
non-initial netns, destroying that netns lets the interface vanish
instead of moving it back to the initial netns. default_device_exit()
skips CAN interfaces due to having rtnl_link_ops set. Reproducer:
ip netns add foo
ip link set can0 netns foo
ip netns delete foo
WARNING: CPU: 1 PID: 84 at net/core/dev.c:11030 ops_exit_list+0x38/0x60
CPU: 1 PID: 84 Comm: kworker/u4:2 Not tainted 5.10.19 #1
Workqueue: netns cleanup_net
[<c010e700>] (unwind_backtrace) from [<c010a1d8>] (show_stack+0x10/0x14)
[<c010a1d8>] (show_stack) from [<c086dc10>] (dump_stack+0x94/0xa8)
[<c086dc10>] (dump_stack) from [<c086b938>] (__warn+0xb8/0x114)
[<c086b938>] (__warn) from [<c086ba10>] (warn_slowpath_fmt+0x7c/0xac)
[<c086ba10>] (warn_slowpath_fmt) from [<c0629f20>] (ops_exit_list+0x38/0x60)
[<c0629f20>] (ops_exit_list) from [<c062a5c4>] (cleanup_net+0x230/0x380)
[<c062a5c4>] (cleanup_net) from [<c0142c20>] (process_one_work+0x1d8/0x438)
[<c0142c20>] (process_one_work) from [<c0142ee4>] (worker_thread+0x64/0x5a8)
[<c0142ee4>] (worker_thread) from [<c0148a98>] (kthread+0x148/0x14c)
[<c0148a98>] (kthread) from [<c0100148>] (ret_from_fork+0x14/0x2c)
To properly restore physical CAN devices to the initial netns on owning
netns exit, introduce a flag on rtnl_link_ops that can be set by drivers.
For CAN devices setting this flag, default_device_exit() considers them
non-virtual, applying the usual namespace move.
The issue was introduced in the commit mentioned below, as at that time
CAN devices did not have a dellink() operation.
Fixes: e008b5fc8d ("net: Simplfy default_device_exit and improve batching.")
Link: https://lore.kernel.org/r/20210302122423.872326-1-martin@strongswan.org
Signed-off-by: Martin Willi <martin@strongswan.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Since 20dd3850bc ("can: Speed up CAN frame receiption by using
ml_priv") the CAN framework uses per device specific data in the AF_CAN
protocol. For this purpose the struct net_device->ml_priv is used. Later
the ml_priv usage in CAN was extended for other users, one of them being
CAN_J1939.
Later in the kernel ml_priv was converted to an union, used by other
drivers. E.g. the tun driver started storing it's stats pointer.
Since tun devices can claim to be a CAN device, CAN specific protocols
will wrongly interpret this pointer, which will cause system crashes.
Mostly this issue is visible in the CAN_J1939 stack.
To fix this issue, we request a dedicated CAN pointer within the
net_device struct.
Reported-by: syzbot+5138c4dd15a0401bec7b@syzkaller.appspotmail.com
Fixes: 20dd3850bc ("can: Speed up CAN frame receiption by using ml_priv")
Fixes: ffd956eef6 ("can: introduce CAN midlayer private and allocate it automatically")
Fixes: 9d71dd0c70 ("can: add support of SAE J1939 protocol")
Fixes: 497a5757ce ("tun: switch to net core provided statistics counters")
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Link: https://lore.kernel.org/r/20210223070127.4538-1-o.rempel@pengutronix.de
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
drivers/net/can/dev.c
b552766c87 ("can: dev: prevent potential information leak in can_fill_info()")
3e77f70e73 ("can: dev: move driver related infrastructure into separate subdir")
0a042c6ec9 ("can: dev: move netlink related code into seperate file")
Code move.
drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
57ac4a31c4 ("net/mlx5e: Correctly handle changing the number of queues when the interface is down")
214baf2287 ("net/mlx5e: Support HTB offload")
Adjacent code changes
net/switchdev/switchdev.c
20776b465c ("net: switchdev: don't set port_obj_info->handled true when -EOPNOTSUPP")
ffb68fc58e ("net: switchdev: remove the transaction structure from port object notifiers")
bae33f2b5a ("net: switchdev: remove the transaction structure from port attributes")
Transaction parameter gets dropped otherwise keep the fix.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
In commit 652562e5ff ("can: length: can_fd_len2dlc(): simplify length
calculcation") the readability of the code degraded and became more error
prone. To counteract this, partially convert that patch and replace open coded
values (of the original code) with proper defines.
Fixes: 652562e5ff ("can: length: can_fd_len2dlc(): simplify length calculcation")
Cc: Vincent MAILHOL <mailhol.vincent@wanadoo.fr>
Link: https://lore.kernel.org/r/20210118201346.79422-1-socketcan@hartkopp.net
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
The can_get_state_str() function is also relevant to the drivers. Export the
symbol and make it visible in the can/dev.h header.
Link: https://lore.kernel.org/r/20210119170355.12040-1-mailhol.vincent@wanadoo.fr
Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Conflicts:
drivers/net/can/dev.c
commit 03f16c5075 ("can: dev: can_restart: fix use after free bug")
commit 3e77f70e73 ("can: dev: move driver related infrastructure into separate subdir")
Code move.
drivers/net/dsa/b53/b53_common.c
commit 8e4052c32d ("net: dsa: b53: fix an off by one in checking "vlan->vid"")
commit b7a9e0da2d ("net: switchdev: remove vid_begin -> vid_end range from VLAN objects")
Field rename.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Call skb_tx_timestamp() within can_put_echo_skb() so that a software tx
timestamp gets attached to the skb.
There two main reasons to include this call in can_put_echo_skb():
* It easily allow to enable the tx timestamp on all devices with
just one small change.
* According to Documentation/networking/timestamping.rst, the tx
timestamps should be generated in the device driver as close as possible,
but always prior to passing the packet to the network interface. During the
call to can_put_echo_skb(), the skb gets cloned meaning that the driver
should not dereference the skb variable anymore after can_put_echo_skb()
returns. This makes can_put_echo_skb() the very last place we can use the
skb without having to access the echo_skb[] array.
Remark: by default, skb_tx_timestamp() does nothing. It needs to be activated
by passing the SOF_TIMESTAMPING_TX_SOFTWARE flag either through socket options
or control messages.
References:
* Support for the error queue in CAN RAW sockets (which is needed for
tx timestamps) was introduced in:
https://git.kernel.org//torvalds/c/eb88531bdbfaafb827192d1fc6c5a3fcc4fadd96
* Put the call to skb_tx_timestamp() just before adding it to the
array:
https://lore.kernel.org/r/043c3ea1-6bdd-59c0-0269-27b2b5b36cec@victronenergy.com
* About Tx hardware timestamps
https://lore.kernel.org/r/20210111171152.GB11715@hoboy.vegasvil.org
Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Link: https://lore.kernel.org/r/20210112095437.6488-2-mailhol.vincent@wanadoo.fr
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
In order to implement byte queue limits (bql) in CAN drivers, the length of the
CAN frame needs to be passed into the networking stack after queueing and after
transmission completion.
To avoid to calculate this length twice, extend can_rx_offload_get_echo_skb()
to return that value. Convert all users of this function, too.
Reviewed-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Link: https://lore.kernel.org/r/20210111141930.693847-15-mkl@pengutronix.de
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
