| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
With commit 132f2d45fb23 ("can: c_can: add support to 64 message
objects") the number of message objects used for reception /
transmission depends on FIFO size.
The ethtools API support allows you to retrieve this info. Driver info
has been added too.
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Dario Binacchi <[email protected]>
Reviewed-by: Andrew Lunn <[email protected]>
Signed-off-by: Marc Kleine-Budde <[email protected]>
|
|
The member rxmasked of struct c_can_priv is initialized by
c_can_chip_config(), but's it's never used, so remove it.
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Dario Binacchi <[email protected]>
Signed-off-by: Marc Kleine-Budde <[email protected]>
|
|
D_CAN controller supports 16, 32, 64 or 128 message objects, comparing
to 32 on C_CAN. AM335x/AM437x Sitara processors and DRA7 SOC all
instantiate a D_CAN controller with 64 message objects, as described
in the "DCAN features" subsection of the CAN chapter of their
technical reference manuals.
The driver policy has been kept unchanged, and as in the previous
version, the first half of the message objects is used for reception
and the second for transmission.
The I/O load is increased only in the case of 64 message objects,
keeping it unchanged in the case of 32. Two 32-bit read accesses are
in fact required, which however remained at 16-bit for configurations
with 32 message objects.
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Dario Binacchi <[email protected]>
Signed-off-by: Marc Kleine-Budde <[email protected]>
|
|
As pointed by commit c0a9f4d396c9 ("can: c_can: Reduce register
access") the "driver casts the 16 message objects in stone, which is
completely braindead as contemporary hardware has up to 128 message
objects".
The patch prepares the module to extend the number of message objects
beyond the 32 currently managed. This was achieved by transforming the
constants used to manage RX/TX messages into variables without
changing the driver policy.
Reported-by: kernel test robot <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Dario Binacchi <[email protected]>
Signed-off-by: Marc Kleine-Budde <[email protected]>
|
|
The arbitration register is already set up with 32-bit writes in the
other parts of the code except for this point.
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Dario Binacchi <[email protected]>
Signed-off-by: Marc Kleine-Budde <[email protected]>
|
|
After reading the commit 640916db2bf7 ("can: c_can: Make it SMP safe")
it may sound strange to see the IF_RX interface used by the
can_inval_tx_object function. A comment was added to avoid any
misunderstanding.
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Dario Binacchi <[email protected]>
Signed-off-by: Marc Kleine-Budde <[email protected]>
|
|
Commit 524369e2391f ("can: c_can: remove obsolete STRICT_FRAME_ORDERING Kconfig option")
left behind wrong indentation, fix it.
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Dario Binacchi <[email protected]>
Signed-off-by: Marc Kleine-Budde <[email protected]>
|
|
Commit 9d23a9818cb1 ("can: c_can: Remove unused inline function") left
behind C_CAN_MSG_OBJ_TX_LAST constant.
Commit fa39b54ccf28 ("can: c_can: Get rid of pointless interrupts") left
behind C_CAN_MSG_RX_LOW_LAST and C_CAN_MSG_OBJ_RX_SPLIT constants.
The removed code also made a comment useless and misleading.
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Dario Binacchi <[email protected]>
Signed-off-by: Marc Kleine-Budde <[email protected]>
|
|
This patch fixes the remaining checkpatch warnings in the driver.
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Marc Kleine-Budde <[email protected]>
|
|
This patch replaces the double assignments by two single ones, to make
checkpatch happy.
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Marc Kleine-Budde <[email protected]>
|
|
This patch fixes the indention in the driver.
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Marc Kleine-Budde <[email protected]>
|
|
This patch removes unnecessary blank lines and add suggested ones, so
that checkpatch doesn't complain anymore.
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Marc Kleine-Budde <[email protected]>
|
|
This patch converts all block comments to network subsystem style
block comments.
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Marc Kleine-Budde <[email protected]>
|
|
Currently doing modprobe c_can_pci will make the kernel complain:
Unbalanced pm_runtime_enable!
this is caused by pm_runtime_enable() called before pm is initialized.
This fix is similar to 227619c3ff7c, move those pm_enable/disable code
to c_can_platform.
Fixes: 4cdd34b26826 ("can: c_can: Add runtime PM support to Bosch C_CAN/D_CAN controller")
Link: http://lore.kernel.org/r/[email protected]
Signed-off-by: Tong Zhang <[email protected]>
Tested-by: Uwe Kleine-König <[email protected]>
Signed-off-by: Marc Kleine-Budde <[email protected]>
|
|
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_get_echo_skb() to return
that value. Convert all users of this function, too.
Reviewed-by: Vincent Mailhol <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Marc Kleine-Budde <[email protected]>
|
|
Add a frame_len argument to can_put_echo_skb() which is used to save length of
the CAN frame into field frame_len of struct can_skb_priv so that it can be
later used after transmission completion. Convert all users of this function,
too.
Drivers which implement BQL call can_put_echo_skb() with the output of
can_skb_get_frame_len(skb) and drivers which do not simply pass zero as an
input (in the same way that NULL would be given to can_get_echo_skb()). This
way, we have a nice symmetry between the two echo functions.
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Marc Kleine-Budde <[email protected]>
Reviewed-by: Vincent Mailhol <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Vincent Mailhol <[email protected]>
|
|
Conflicts:
drivers/net/ethernet/ibm/ibmvnic.c
Signed-off-by: Jakub Kicinski <[email protected]>
|
|
In the error handling in c_can_power_up(), there are two bugs:
1) c_can_pm_runtime_get_sync() will increase usage counter if device is not
empty. Forgetting to call c_can_pm_runtime_put_sync() will result in a
reference leak here.
2) c_can_reset_ram() operation will set start bit when enable is true. We
should clear it in the error handling.
We fix it by adding c_can_pm_runtime_put_sync() for 1), and
c_can_reset_ram(enable is false) for 2) in the error handling.
Fixes: 8212003260c60 ("can: c_can: Add d_can suspend resume support")
Fixes: 52cde85acc23f ("can: c_can: Add d_can raminit support")
Signed-off-by: Zhang Qilong <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
[mkl: return "0" instead of "ret"]
Signed-off-by: Marc Kleine-Budde <[email protected]>
|
|
The naming of can_dlc as element of struct can_frame and also as variable
name is misleading as it claims to be a 'data length CODE' but in reality
it always was a plain data length.
With the indroduction of a new 'len' element in struct can_frame we can now
remove can_dlc as name and make clear which of the former uses was a plain
length (-> 'len') or a data length code (-> 'dlc') value.
Signed-off-by: Oliver Hartkopp <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
[mkl: gs_usb: keep struct gs_host_frame::can_dlc as is]
Signed-off-by: Marc Kleine-Budde <[email protected]>
|
|
The get_can_dlc() macro is used to ensure the payload length information of
the Classical CAN frame to be max 8 bytes (the CAN_MAX_DLEN).
Rename the macro and use the correct constant in preparation of the len/dlc
cleanup for Classical CAN frames.
Signed-off-by: Oliver Hartkopp <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Marc Kleine-Budde <[email protected]>
|
|
commit 524369e2391f ("can: c_can: remove obsolete STRICT_FRAME_ORDERING Kconfig option")
left behind this, remove it.
Signed-off-by: YueHaibing <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Marc Kleine-Budde <[email protected]>
|
|
While the state is updated when the error counters increase and
decrease, there is no event when the bus recovers and the error counters
decrease again. So add that event as well.
Change the state going downward to be ERROR_PASSIVE -> ERROR_WARNING ->
ERROR_ACTIVE instead of directly to ERROR_ACTIVE again.
Signed-off-by: Jeroen Hofstee <[email protected]>
Acked-by: Kurt Van Dijck <[email protected]>
Tested-by: Kurt Van Dijck <[email protected]>
Signed-off-by: Marc Kleine-Budde <[email protected]>
|
|
When the CAN interface is closed it the hardwre is put in power down
mode, but does not reset the error counters / state. Reset the D_CAN on
open, so the reported state and the actual state match.
According to [1], the C_CAN module doesn't have the software reset.
[1] http://www.bosch-semiconductors.com/media/ip_modules/pdf_2/c_can_fd8/users_manual_c_can_fd8_r210_1.pdf
Signed-off-by: Jeroen Hofstee <[email protected]>
Signed-off-by: Marc Kleine-Budde <[email protected]>
|
|
When the status register is read without the status IRQ pending, the
chip may not raise the interrupt line for an upcoming status interrupt
and the driver may miss a status interrupt.
It is critical that the BUSOFF status interrupt is forwarded to the
higher layers, since no more interrupts will follow without
intervention.
Thanks to Wolfgang and Joe for bringing up the first idea.
Signed-off-by: Kurt Van Dijck <[email protected]>
Cc: Wolfgang Grandegger <[email protected]>
Cc: Joe Burmeister <[email protected]>
Fixes: fa39b54ccf28 ("can: c_can: Get rid of pointless interrupts")
Cc: linux-stable <[email protected]>
Signed-off-by: Marc Kleine-Budde <[email protected]>
|
|
napi_complete_done() allows to opt-in for gro_flush_timeout,
added back in linux-3.19, commit 3b47d30396ba
("net: gro: add a per device gro flush timer")
This allows for more efficient GRO aggregation without
sacrifying latencies.
Signed-off-by: Eric Dumazet <[email protected]>
Signed-off-by: David S. Miller <[email protected]>
|
|
access
When testing CAN write floods on Altera's CycloneV, the first 2 bytes
are sometimes 0x00, 0x00 or corrupted instead of the values sent. Also
observed bytes 4 & 5 were corrupted in some cases.
The D_CAN Data registers are 32 bits and changing from 16 bit writes to
32 bit writes fixes the problem.
Testing performed on Altera CycloneV (D_CAN). Requesting tests on other
C_CAN & D_CAN platforms.
Reported-by: Richard Andrysek <[email protected]>
Signed-off-by: Thor Thayer <[email protected]>
Cc: <[email protected]>
Signed-off-by: Marc Kleine-Budde <[email protected]>
|
|
The assignment 'cf->data[2] |= CAN_ERR_PROT_UNSPEC' used at CAN error message
creation time is obsolete as CAN_ERR_PROT_UNSPEC is zero and cf->data[2] is
initialized with zero in alloc_can_err_skb() anyway.
So we could either assign 'cf->data[2] = CAN_ERR_PROT_UNSPEC' correctly or we
can remove the obsolete OR operation entirely.
Signed-off-by: Oliver Hartkopp <[email protected]>
Signed-off-by: Marc Kleine-Budde <[email protected]>
|
|
As Dan Carpenter reported in http://marc.info/?l=linux-can&m=144793696016187
the assignment of the error location in CAN error messages had some bit wise
overlaps. Indeed the value to be assigned in data[3] is no bitfield but defines
a single value which points to a location inside the CAN frame on the wire.
This patch fixes the assignments for the error locations in error messages.
Reported-by: Dan Carpenter <[email protected]>
Signed-off-by: Oliver Hartkopp <[email protected]>
Signed-off-by: Marc Kleine-Budde <[email protected]>
|
|
The previous change 3973c526ae9c (net: can: c_can: Disable pins when CAN
interface is down) causes a slight glitch on the pinctrl settings when used.
Since commit ab78029 (drivers/pinctrl: grab default handles from device core),
the device core will automatically set the default pins. This causes the pins
to be momentarily set to the default and then to the sleep state in
register_c_can_dev(). By adding an optional "enable" state, boards can set the
default pin state to be disabled and avoid the glitch when the switch from
default to sleep first occurs. If the "enable" state is not available
c_can_pinctrl_select_state() falls back to using the "default" pinctrl state.
[Roger Q] - Forward port to v4.2 and use pinctrl_get_select().
Signed-off-by: J.D. Schroeder <[email protected]>
Signed-off-by: Roger Quadros <[email protected]>
Reviewed-by: Grygorii Strashko <[email protected]>
Cc: linux-stable <[email protected]>
Signed-off-by: Marc Kleine-Budde <[email protected]>
|
|
Conflicts:
arch/arm/boot/dts/imx6sx-sdb.dts
net/sched/cls_bpf.c
Two simple sets of overlapping changes.
Signed-off-by: David S. Miller <[email protected]>
|
|
Put controller into init mode in network stop to end pending transmissions. The
issue is observed in cases when transmitted frame is not acked.
Signed-off-by: Viktor Babrian <[email protected]>
Cc: linux-stable <[email protected]>
Signed-off-by: Marc Kleine-Budde <[email protected]>
|
|
In order to be able to move the stats increment from can_bus_off() into
can_change_state(), the increment had to be moved back into code that was using
can_bus_off() but not can_change_state().
As a side-effect, this patch fixes the following bugs:
* Redundant call to can_bus_off() in c_can.
* Bus-off counted twice in xilinx_can.
Signed-off-by: Andri Yngvason <[email protected]>
Signed-off-by: Marc Kleine-Budde <[email protected]>
|
|
DRA7 CAN IP suffers from a problem which causes it to be prevented
from fully turning OFF (i.e. stuck in transition) if the module was
disabled while there was traffic on the CAN_RX line.
To work around this issue we select the SLEEP pin state by default
on probe and use the DEFAULT pin state on CAN up and back to the
SLEEP pin state on CAN down.
Signed-off-by: Roger Quadros <[email protected]>
Signed-off-by: Marc Kleine-Budde <[email protected]>
|
|
Conflicts:
drivers/net/bonding/bond_alb.c
drivers/net/ethernet/altera/altera_msgdma.c
drivers/net/ethernet/altera/altera_sgdma.c
net/ipv6/xfrm6_output.c
Several cases of overlapping changes.
The xfrm6_output.c has a bug fix which overlaps the renaming
of skb->local_df to skb->ignore_df.
In the Altera TSE driver cases, the register access cleanups
in net-next overlapped with bug fixes done in net.
Similarly a bug fix to send ALB packets in the bonding driver using
the right source address overlaps with cleanups in net-next.
Signed-off-by: David S. Miller <[email protected]>
|
|
Add helpers for 32-bit accesses and replace open-coded 32-bit access
with calls to helpers. Minimum changes are done to the pci case, as I
don't have access to that hardware.
Tested-by: Thor Thayer <[email protected]>
Signed-off-by: Thor Thayer <[email protected]>
Signed-off-by: Pavel Machek <[email protected]>
Signed-off-by: Marc Kleine-Budde <[email protected]>
|
|
In 2b9aecdce2 ("can: c_can: Disable rx split as workaround") a new Kconfig
option was introduced as a workaround. The tests performed by Alexander Stein
confirmed this option to be obsolete with all the other cleanups and fixes
that had been discussed that time:
http://marc.info/?l=linux-can&m=139746476821294&w=2
Both (author and tester) agreed to remove this Kconfig option again:
http://marc.info/?l=linux-can&m=139883820714229&w=2
As some more cleanups took place since then a simple revert is not possible.
This patch removes the entire option as it would behave when disabled.
Further beautification’s can be done later.
Signed-off-by: Oliver Hartkopp <[email protected]>
Tested-by: Alexander Stein <[email protected]>
Signed-off-by: Marc Kleine-Budde <[email protected]>
|
|
It's suffcient to kill the TXIE bit in the message control register
even if the documentation of C and D CAN says that it's not allowed to
do that while MSGVAL is set. Reality tells a different story and this
change gives us another 2% of CPU back for not waiting on I/O.
Signed-off-by: Thomas Gleixner <[email protected]>
Tested-by: Alexander Stein <[email protected]>
Signed-off-by: Marc Kleine-Budde <[email protected]>
|
|
Mark suggested to use one IF for the softirq and the other for the
xmit function to avoid the xmit lock.
That requires to write the frame into the interface first, then handle
the echo skb and store the dlc before committing the TX request to the
message ram.
We use an atomic to handle the active buffers instead of reading the
MSGVAL register as thats way faster especially on PCH/x86.
Suggested-by: Mark <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Tested-by: Alexander Stein <[email protected]>
Signed-off-by: Marc Kleine-Budde <[email protected]>
|
|
Instead of obfuscating the code by artificial 16 bit splits use the
proper 32 bit assignments and split the result when writing to the
interface.
Signed-off-by: Thomas Gleixner <[email protected]>
Tested-by: Alexander Stein <[email protected]>
Signed-off-by: Marc Kleine-Budde <[email protected]>
|
|
Remove the MASK from the TX transfer side.
Make the code readable and get rid of the annoying IFX_WRITE_XXX_16BIT
macros which are just obfuscating the code.
Signed-off-by: Thomas Gleixner <[email protected]>
Tested-by: Alexander Stein <[email protected]>
Signed-off-by: Marc Kleine-Budde <[email protected]>
|
|
Sigh!
Signed-off-by: Thomas Gleixner <[email protected]>
Tested-by: Alexander Stein <[email protected]>
Signed-off-by: Marc Kleine-Budde <[email protected]>
|
|
Signed-off-by: Thomas Gleixner <[email protected]>
Tested-by: Alexander Stein <[email protected]>
Signed-off-by: Marc Kleine-Budde <[email protected]>
|
|
Signed-off-by: Thomas Gleixner <[email protected]>
Tested-by: Alexander Stein <[email protected]>
Signed-off-by: Marc Kleine-Budde <[email protected]>
|
|
Signed-off-by: Thomas Gleixner <[email protected]>
Tested-by: Alexander Stein <[email protected]>
Signed-off-by: Marc Kleine-Budde <[email protected]>
|
|
Signed-off-by: Thomas Gleixner <[email protected]>
Tested-by: Alexander Stein <[email protected]>
Signed-off-by: Marc Kleine-Budde <[email protected]>
|
|
Alexander reported that the new optimized handling of the RX fifo
causes random packet loss on Intel PCH C_CAN hardware.
After a few fruitless debugging sessions I got hold of a PCH (eg20t)
afflicted system. That machine does not have the CAN interface wired
up, but it was possible to reproduce the issue with the HW loopback
mode.
As Alexander observed correctly, clearing the NewDat flag along with
reading out the message buffer causes that issue on C_CAN, while D_CAN
handles that correctly.
Instead of restoring the original message buffer handling horror the
following workaround solves the issue:
transfer buffer to IF without clearing the NewDat
handle the message
clear NewDat bit
That's similar to the original code but conditional for C_CAN.
I really wonder why all user manuals (C_CAN, Intel PCH and some more)
recommend to clear the NewDat bit right away. The knows it all Oracle
operated by Gurgle does not unearth any useful information either. I
simply cannot believe that we are the first to uncover that HW issue.
Reported-and-tested-by: Alexander Stein <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Signed-off-by: Marc Kleine-Budde <[email protected]>
|
|
The RX buffer split causes packet loss in the hardware:
What happens is:
RX Packet 1 --> message buffer 1 (newdat bit is not cleared)
RX Packet 2 --> message buffer 2 (newdat bit is not cleared)
RX Packet 3 --> message buffer 3 (newdat bit is not cleared)
RX Packet 4 --> message buffer 4 (newdat bit is not cleared)
RX Packet 5 --> message buffer 5 (newdat bit is not cleared)
RX Packet 6 --> message buffer 6 (newdat bit is not cleared)
RX Packet 7 --> message buffer 7 (newdat bit is not cleared)
RX Packet 8 --> message buffer 8 (newdat bit is not cleared)
Clear newdat bit in message buffer 1
Clear newdat bit in message buffer 2
Clear newdat bit in message buffer 3
Clear newdat bit in message buffer 4
Clear newdat bit in message buffer 5
Clear newdat bit in message buffer 6
Clear newdat bit in message buffer 7
Clear newdat bit in message buffer 8
Now if during that clearing of newdat bits, a new message comes in,
the HW gets confused and drops it.
It does not matter how many of them you clear. I put a delay between
clear of buffer 1 and buffer 2 which was long enough that the message
should have been queued either in buffer 1 or buffer 9. But it did not
show up anywhere. The next message ended up in buffer 1. So the
hardware lost a packet of course without telling it via one of the
error handlers.
That does not happen on all clear newdat bit events. I see one of 10k
packets dropped in the scenario which allows us to reproduce. But the
trace looks always the same.
Not splitting the RX Buffer avoids the packet loss but can cause
reordering. It's hard to trigger, but it CAN happen.
With that mode we use the HW as it was probably designed for. We read
from the buffer 1 upwards and clear the buffer as we get the
message. That's how all microcontrollers use it. So I assume that the
way we handle the buffers was never really tested. According to the
public documentation it should just work :)
Let the user decide which evil is the lesser one.
[ Oliver Hartkopp: Provided a sane config option and help text and
made me switch to favour potential and unlikely reordering over
packet loss ]
Signed-off-by: Thomas Gleixner <[email protected]>
Tested-by: Alexander Stein <[email protected]>
Signed-off-by: Marc Kleine-Budde <[email protected]>
|
|
The driver handles pointlessly TWO interrupts per packet. The reason
is that it enables the status interrupt which fires for each rx and tx
packet and it enables the per message object interrupts as well.
The status interrupt merily acks or in case of D_CAN ignores the TX/RX
state and then the message object interrupt fires.
The message objects interrupts are only useful if all message objects
have hardware filters activated.
But we don't have that and its not simple to implement in that driver
without rewriting it completely.
So we can ditch the message object interrupts and handle the RX/TX
right away from the status interrupt. Instead of TWO we handle ONE.
Note: We must keep the TXIE/RXIE bits in the message buffers because
the status interrupt alone is not reliable enough in corner cases.
If we ever have the need for HW filtering, then this code needs a
complete overhaul and we can think about it then. For now we prefer a
lower interrupt load.
Signed-off-by: Thomas Gleixner <[email protected]>
Tested-by: Alexander Stein <[email protected]>
Signed-off-by: Marc Kleine-Budde <[email protected]>
|
|
On D_CAN the RXOK, TXOK and LEC bits are cleared/set on read of the
status register. No need to update them.
Signed-off-by: Thomas Gleixner <[email protected]>
Tested-by: Alexander Stein <[email protected]>
Signed-off-by: Marc Kleine-Budde <[email protected]>
|
|
Instead of writing to the message object we can simply clear the
NewDat bit with the get method.
Signed-off-by: Thomas Gleixner <[email protected]>
Tested-by: Alexander Stein <[email protected]>
Signed-off-by: Marc Kleine-Budde <[email protected]>
|