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/20210111061335.39983-1-mailhol.vincent@wanadoo.fr
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Reviewed-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Link: https://lore.kernel.org/r/20210111141930.693847-13-mkl@pengutronix.de
Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
The helper functions can_len2dlc and can_dlc2len are only relevant for
CAN FD data length code (DLC) conversion.
To fit the introduced can_cc_dlc2len for Classical CAN we rename:
can_dlc2len -> can_fd_dlc2len to get the payload length from the DLC
can_len2dlc -> can_fd_len2dlc to get the DLC from the payload length
Suggested-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Link: https://lore.kernel.org/r/20201110101852.1973-6-socketcan@hartkopp.net
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
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 <socketcan@hartkopp.net>
Link: https://lore.kernel.org/r/20201120100444.3199-1-socketcan@hartkopp.net
[mkl: gs_usb: keep struct gs_host_frame::can_dlc as is]
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
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 <socketcan@hartkopp.net>
Link: https://lore.kernel.org/r/20201110101852.1973-3-socketcan@hartkopp.net
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Calling pm_runtime_get_sync increments the counter even in case of
failure, causing incorrect ref count. Call pm_runtime_put if
pm_runtime_get_sync fails.
Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
Link: https://lore.kernel.org/r/20200605033239.60664-1-navid.emamdoost@gmail.com
Fixes: 4716620d1b ("can: xilinx: Convert to runtime_pm")
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Some variables with incorrect type were passed to "of_property_read_u32"
API, "of_property_read_u32" API was expecting an "u32 *" but the formal
parameter that was passed was of type "int *". Fixed the issue by
changing the variable types from "int" to "u32" and initialized with a
default value. Fixed sparse warning.
Addresses-Coverity: "incompatible_param"
Addresses-Coverity: "UNINIT(Using uninitialized value)"
Signed-off-by: Srinivas Neeli <srinivas.neeli@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Link: https://lore.kernel.org/r/0651544d22f3c25893ca9d445b14823f0dfddfc8.1600073396.git.michal.simek@xilinx.com
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
CANFD2.0 core uses BRAM for storing acceptance filter ID(AFID) and MASK
(AFMASK)registers. So by default AFID and AFMASK registers contain random
data. Due to random data, we are not able to receive all CAN ids.
Initializing AFID and AFMASK registers with Zero before enabling
acceptance filter to receive all packets irrespective of ID and Mask.
Fixes: 0db9071353 ("can: xilinx: add can 2.0 support")
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Signed-off-by: Srinivas Neeli <srinivas.neeli@xilinx.com>
Reviewed-by: Naga Sureshkumar Relli <naga.sureshkumar.relli@xilinx.com>
Cc: linux-stable <stable@vger.kernel.org> # >= v5.0
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
As per linux can framework, driver not allowed to touch the skb memory
after can_put_echo_skb() call.
This patch fixes the same.
https://www.spinics.net/lists/linux-can/msg02199.html
Signed-off-by: Srinivas Neeli <srinivas.neeli@xilinx.com>
Reviewed-by: Appana Durga Kedareswara Rao <appana.durga.rao@xilinx.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
When the CAN bus clock is provided from the clock wizard, clock wizard
driver may not be available when can driver probes resulting to the
error message "bus clock not found error".
As this error message is not very useful to the end user, skip printing
in the case of deferred probe.
Signed-off-by: Venkatesh Yadav Abbarapu <venkatesh.abbarapu@xilinx.com>
Signed-off-by: Srinivas Neeli <srinivas.neeli@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Reviewed-by: Appana Durga Kedareswara Rao <appana.durga.rao@xilinx.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Use devm_platform_ioremap_resource() to simplify the code a bit.
This is detected by coccinelle.
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Userspace can signal with CAN_CTRLMODE_BERR_REPORTING whether they need
reporting of bus errors (CAN_ERR_BUSERROR) or not.
However, xilinx_can driver currently always sends CAN_ERR_BUSERROR
frames to userspace on bus errors.
To improve performance on error conditions when bus error reporting is
not needed, avoid sending CAN_ERR_BUSERROR frames unless requested via
CAN_CTRLMODE_BERR_REPORTING.
The error interrupt is still kept enabled as there is no dedicated state
transition interrupt, but just disabling error frame submission still
yields a significant performance improvement. In a simple test with
continuous bus errors and no userspace programs reading/writing CAN I
saw system CPU load reduced by 1/3.
Tested on a ZynqMP board with CAN-FD v1.0.
Signed-off-by: Anssi Hannula <anssi.hannula@bitwise.fi>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
AXI CANIP doesn't support tx fifo empty interrupt feature(TXFEMP),
update the flags filed in the driver for AXI CAN case accordingly.
Fixes: 3281b380ec ("can: xilinx_can: Fix flags field initialization for axi can and canps")
Reported-by: Anssi Hannula <anssi.hannula@bitwise.fi>
Signed-off-by: Appana Durga Kedareswara rao <appana.durga.rao@xilinx.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
While calculating bitrate for the data phase, the driver is using phase
segment 1 of the arbitration phase instead of the data phase.
Fixes: c223da6 ("can: xilinx_can: Add support for CANFD FD frames")
Signed-off-by: Appana Durga Kedareswara rao <appana.durga.rao@xilinx.com>
Signed-off-by: Srinivas Neeli <srinivas.neeli@xilinx.com>
Acked-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
For CANFD 2.0 IP configuration existing driver is using incorrect mask
values for FSR register FL and RI fields.
Fixes: c223da6 ("can: xilinx_can: Add support for CANFD FD frames")
Signed-off-by: Appana Durga Kedareswara rao <appana.durga.rao@xilinx.com>
Acked-by: Shubhrajyoti Datta <shubhrajyoti.datta@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
commit c223da6893 ("can: xilinx_can: Add support for CANFD FD frames")
is writing data to a wrong offset for FD frames.
This patch fixes this issue.
Fixes: c223da6 ("can: xilinx_can: Add support for CANFD FD frames")
Reviewed-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
Reviewed-by: Shubhrajyoti Datta <Shubhrajyoti.datta@xilinx.com>
Signed-off-by: Appana Durga Kedareswara rao <appana.durga.rao@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
After commit c223da6893 ("can: xilinx_can: Add support for CANFD FD
frames") the driver is updating the FSR IRI index multiple times (i.e in
xcanfd_rx() and xcan_rx_fifo_get_next_frame()), It should be updated
once per RX packet. This patch fixes this issue, also this patch removes
the unnecessary fsr register checks in xcanfd_rx() API.
Fixes: c223da6 ("can: xilinx_can: Add support for CANFD FD frames")
Reviewed-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
Reviewed-by: Shubhrajyoti Datta <Shubhrajyoti.datta@xilinx.com>
Signed-off-by: Appana Durga Kedareswara rao <appana.durga.rao@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
When can clock is provided from the clock wizard, clock wizard driver
may not be available when can driver probes resulting to the error
message "Device clock not found error".
As this error message is not very userful to the end user, skip printing
it in the case of deferred probe.
Fixes: b1201e44 ("can: xilinx CAN controller support")
Signed-off-by: Appana Durga Kedareswara rao <appana.durga.rao@xilinx.com>
Signed-off-by: Venkatesh Yadav Abbarapu <venkatesh.abbarapu@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Currently the xilinx_can xcan_chip_start() function, called from
.ndo_open() and via CAN_MODE_START (bus-off restart), waits for the SR
register to show the wanted operating state, with a 1 sec timeout.
However, that register bit will only be set once the HW has observed 11
consecutive recessive bits (BusIdle) on the bus.
If the bus will not see the 11 bits (e.g. it is stuck dominant), the
function will timeout and return an error. If this was done as part of a
scheduled restart from bus-off, the interface will stay in bus-off state
forever even if the bus recovers later.
According to M_CAN and FLEXCAN documentation they also wait for 11
consecutive recessive bits, but their drivers do not seem to wait for
that.
To make the behavior consistent, modify xilinx_can to also not wait for
the synchronization to complete.
The only way for users to know for sure that the bus has been joined
successfully is to see successfully received or transmitted frames. That
does not seem optimal, but it is consistent with other drivers and we
should have a properly working restart-ms with xilinx_can.
Tested on ZynqMP with Xilinx CAN-FD 1.0.
Fixes: b1201e44f5 ("can: xilinx CAN controller support")
Signed-off-by: Anssi Hannula <anssi.hannula@bitwise.fi>
Tested-by: Appana Durga Kedareswara Rao <appana.durga.rao@xilinx.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
A statement is indented one level too deep, fix this.
Signed-off-by: Colin Ian King <colin.king@canonical.com>
Reviewed-by: Michal Simek <michal.simek@xilinx.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
CANFD IP supports both CAN and CAN FD frames,
Existing driver supports only CAN frames, This patch
adds support for CAN FD frames.
Signed-off-by: Naga Sureshkumar Relli <naga.sureshkumar.relli@xilinx.com>
Signed-off-by: Appana Durga Kedareswara rao <appana.durga.rao@xilinx.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
To differentiate between different CAN IP's this patch adds
cantype enum variable in the xcan_devtype_data structure
Signed-off-by: Appana Durga Kedareswara rao <appana.durga.rao@xilinx.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
AXI CAN IP and CANPS IP supports tx fifo empty feature, this patch updates
the flags field for the same.
Signed-off-by: Appana Durga Kedareswara rao <appana.durga.rao@xilinx.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
This patch fixes below kernel doc warnings
warning: Function parameter or member 'priv' not described in
'xcan_write_frame'
warning: Function parameter or member 'skb' not described in
'xcan_start_xmit_fifo'
warning: Function parameter or member 'ndev' not described in
'xcan_start_xmit_fifo'
warning: Function parameter or member 'skb' not described in
'xcan_start_xmit_mailbox'
warning: Function parameter or member 'ndev' not described in
'xcan_start_xmit_mailbox'
warning: Function parameter or member 'priv' not described in
'xcan_rx_fifo_get_next_frame'
Signed-off-by: Appana Durga Kedareswara rao <appana.durga.rao@xilinx.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
This patch fixes below checkpatch warnings and checks in the driver.
CHECK: Alignment should match open parenthesis
+ void (*write_reg)(const struct xcan_priv *priv, enum xcan_reg reg,
+ u32 val);
CHECK: Alignment should match open parenthesis
+static void xcan_write_reg_le(const struct xcan_priv *priv, enum xcan_reg reg,
+ u32 val)
CHECK: Alignment should match open parenthesis
+static void xcan_write_reg_be(const struct xcan_priv *priv, enum xcan_reg reg,
+ u32 val)
CHECK: Alignment should match open parenthesis
+ netdev_dbg(ndev, "BRPR=0x%08x, BTR=0x%08x\n",
+ priv->read_reg(priv, XCAN_BRPR_OFFSET),
CHECK: Alignment should match open parenthesis
+ netdev_warn(ndev,
+ "timed out for correct mode\n");
CHECK: Alignment should match open parenthesis
+ netdev_dbg(ndev, "status:#x%08x\n",
+ priv->read_reg(priv, XCAN_SR_OFFSET));
CHECK: spaces preferred around that '-' (ctx:VxV)
+ (CAN_EFF_ID_BITS-CAN_SFF_ID_BITS)) <<
^
CHECK: Alignment should match open parenthesis
+ netdev_dbg(ndev, "%s: error status register:0x%x\n",
+ __func__, priv->read_reg(priv, XCAN_ESR_OFFSET));
WARNING: line over 80 characters
+ offset = XCAN_RXMSG_2_FRAME_OFFSET(fsr & XCAN_FSR_RI_MASK);
WARNING: line over 80 characters
+ offset = XCAN_RXMSG_FRAME_OFFSET(fsr & XCAN_FSR_RI_MASK);
WARNING: line over 80 characters
+ while ((isr & XCAN_IXR_TXOK_MASK) && !WARN_ON(++retries == 100)) {
WARNING: line over 80 characters
+ priv->write_reg(priv, XCAN_ICR_OFFSET, XCAN_IXR_TXOK_MASK);
CHECK: Alignment should match open parenthesis
+ netdev_err(ndev, "%s: pm_runtime_get failed(%d)\n",
+ __func__, ret);
CHECK: Alignment should match open parenthesis
+ ret = request_irq(ndev->irq, xcan_interrupt, priv->irq_flags,
+ ndev->name, ndev);
CHECK: Alignment should match open parenthesis
+static int xcan_get_berr_counter(const struct net_device *ndev,
+ struct can_berr_counter *bec)
CHECK: Alignment should match open parenthesis
+ netdev_err(ndev, "%s: pm_runtime_get failed(%d)\n",
+ __func__, ret);
CHECK: Please don't use multiple blank lines
+
+
CHECK: Alignment should match open parenthesis
+ netdev_err(ndev, "%s: pm_runtime_get failed(%d)\n",
+ __func__, ret);`
Signed-off-by: Appana Durga Kedareswara rao <appana.durga.rao@xilinx.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Commit 9e5f1b273e ("can: xilinx_can: add support for Xilinx CAN FD
core") added a new can_bittiming_const structure for CAN FD cores that
support larger values for tseg1, tseg2, and sjw than previous Xilinx CAN
cores, but the commit did not actually take that into use.
Fix that.
Tested with CAN FD core on a ZynqMP board.
Fixes: 9e5f1b273e ("can: xilinx_can: add support for Xilinx CAN FD core")
Reported-by: Shubhrajyoti Datta <shubhrajyoti.datta@gmail.com>
Signed-off-by: Anssi Hannula <anssi.hannula@bitwise.fi>
Cc: Michal Simek <michal.simek@xilinx.com>
Reviewed-by: Shubhrajyoti Datta <shubhrajyoti.datta@gmail.com>
Cc: linux-stable <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Based on 3 normalized pattern(s):
this program is free software you can redistribute it and or modify
it under the terms of the gnu general public license as published by
the free software foundation either version 2 of the license or at
your option any later version this program is distributed in the
hope that it will be useful but without any warranty without even
the implied warranty of merchantability or fitness for a particular
purpose see the gnu general public license for more details
this program is free software you can redistribute it and or modify
it under the terms of the gnu general public license as published by
the free software foundation either version 2 of the license or at
your option any later version [author] [kishon] [vijay] [abraham]
[i] [kishon]@[ti] [com] this program is distributed in the hope that
it will be useful but without any warranty without even the implied
warranty of merchantability or fitness for a particular purpose see
the gnu general public license for more details
this program is free software you can redistribute it and or modify
it under the terms of the gnu general public license as published by
the free software foundation either version 2 of the license or at
your option any later version [author] [graeme] [gregory]
[gg]@[slimlogic] [co] [uk] [author] [kishon] [vijay] [abraham] [i]
[kishon]@[ti] [com] [based] [on] [twl6030]_[usb] [c] [author] [hema]
[hk] [hemahk]@[ti] [com] this program is distributed in the hope
that it will be useful but without any warranty without even the
implied warranty of merchantability or fitness for a particular
purpose see the gnu general public license for more details
extracted by the scancode license scanner the SPDX license identifier
GPL-2.0-or-later
has been chosen to replace the boilerplate/reference in 1105 file(s).
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Allison Randal <allison@lohutok.net>
Reviewed-by: Richard Fontana <rfontana@redhat.com>
Reviewed-by: Kate Stewart <kstewart@linuxfoundation.org>
Cc: linux-spdx@vger.kernel.org
Link: https://lkml.kernel.org/r/20190527070033.202006027@linutronix.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The method ndo_start_xmit() is defined as returning an 'netdev_tx_t',
which is a typedef for an enum type, so make sure the implementation in
this driver has returns 'netdev_tx_t' value, and change the function
return type to netdev_tx_t.
Found by coccinelle.
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Add support for Xilinx CAN FD core.
The major difference from the previously supported cores is that there
are TX mailboxes instead of a TX FIFO and the RX FIFO access method is
different.
We only transmit one frame at a time to prevent the HW from reordering
frames (it uses CAN ID priority order).
Support for CAN FD protocol is not added yet.
v2: Removed unnecessary "rx-mode" DT property and wrapped some long
lines.
Signed-off-by: Anssi Hannula <anssi.hannula@bitwise.fi>
Cc: Michal Simek <michal.simek@xilinx.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Xilinx CAN FD cores are different enough from the previous Zynq and AXI
CAN cores that some refactoring of the driver is needed.
This commit contains most of the required refactoring to existing code
and should not alter behavior on existing supported HW.
The changes are:
- Reading and writing to frame registers is parametrized to allow
reading/writing a different frame in the future.
- Slightly misleading (as it did not specify *all* the interrupts
supported by the HW) XCAN_INTR_ALL is replaced with specifying the
interrupts inline in interrupt enabling code.
- xcan_devtype_data.caps is renamed to xcan_devtype_data.flags to allow
for flags that define alternative functionality (e.g. mailboxes vs.
FIFO) instead of purely additive capabilities.
- can_bittiming_const is added to xcan_devtype_data as CAN FD cores will
have wider setting ranges.
- bus_clk clock name is now determined through xcan_devtype_data instead
of comparing compatible string in probe().
- xcan_devtype_data is added to xcan_priv to allow flag checks after
probe().
- XCAN_CAP_WATERMARK is now XCAN_FLAG_TXFEMP. CAN FD cores have
watermark support but no TXFEMP interrupt, which is what we are
actually interested in.
- xcan_start_xmit() is split to in two parts to prepare for TX mailboxes
instead of FIFO in CAN FD cores.
v2: Wrapped some long lines in xcan_write_frame().
Signed-off-by: Anssi Hannula <anssi.hannula@bitwise.fi>
Cc: Michal Simek <michal.simek@xilinx.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
The driver updates stats.tx_bytes in start_xmit() even though it could
do so in TX interrupt handler.
Change the code to update tx_bytes in the interrupt handler, using the
return value of can_get_echo_skb().
Signed-off-by: Anssi Hannula <anssi.hannula@bitwise.fi>
Cc: Michal Simek <michal.simek@xilinx.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Replace some custom code with a call to can_change_state().
This subtly changes the error reporting behavior when both RX and TX
error counters indicate the same state.
Previously, if both RX and TX counters indicated the same state:
- if overall state is PASSIVE, report CAN_ERR_CRTL_RX_PASSIVE
- if overall state is WARNING, report CAN_ERR_CRTL_TX_WARNING or
CAN_ERR_CRTL_RX_WARNING depending on which counter is higher,
or CAN_ERR_CRTL_RX_WARNING if the counters have the same value.
After this commit:
- report RX_* or TX_* depending on which counter is higher, or both if
the counters have exactly the same value.
This behavior is consistent with many other CAN drivers that use this
same code pattern.
Tested with the integrated CAN on Zynq-7000 SoC.
v2: Simplify resolving states as suggested by Andri Yngvason.
Signed-off-by: Anssi Hannula <anssi.hannula@bitwise.fi>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
The xilinx_can driver currently increments error-warning and
error-passive statistics on every error interrupt regardless of whether
the interface was already in the same state. Similarly, the error frame
sent on error interrupts is always sent with
CAN_ERR_CRTL_(RX|TX)_(PASSIVE|WARNING) bit set.
To make the error-warning and error-passive statistics more useful, add
a check to only set the error state when the state has actually been
changed.
Tested with the integrated CAN on Zynq-7000 SoC.
Signed-off-by: Anssi Hannula <anssi.hannula@bitwise.fi>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
The method ndo_start_xmit() is defined as returning an 'netdev_tx_t',
which is a typedef for an enum type, but the implementation in this
driver returns an 'int'.
Fix this by returning 'netdev_tx_t' in this driver too.
Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
Acked-by: Michal Simek <michal.simek@xilinx.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
There are several issues with the suspend/resume handling code of the
driver:
- The device is attached and detached in the runtime_suspend() and
runtime_resume() callbacks if the interface is running. However,
during xcan_chip_start() the interface is considered running,
causing the resume handler to incorrectly call netif_start_queue()
at the beginning of xcan_chip_start(), and on xcan_chip_start() error
return the suspend handler detaches the device leaving the user
unable to bring-up the device anymore.
- The device is not brought properly up on system resume. A reset is
done and the code tries to determine the bus state after that.
However, after reset the device is always in Configuration mode
(down), so the state checking code does not make sense and
communication will also not work.
- The suspend callback tries to set the device to sleep mode (low-power
mode which monitors the bus and brings the device back to normal mode
on activity), but then immediately disables the clocks (possibly
before the device reaches the sleep mode), which does not make sense
to me. If a clean shutdown is wanted before disabling clocks, we can
just bring it down completely instead of only sleep mode.
Reorganize the PM code so that only the clock logic remains in the
runtime PM callbacks and the system PM callbacks contain the device
bring-up/down logic. This makes calling the runtime PM callbacks during
e.g. xcan_chip_start() safe.
The system PM callbacks now simply call common code to start/stop the
HW if the interface was running, replacing the broken code from before.
xcan_chip_stop() is updated to use the common reset code so that it will
wait for the reset to complete. Reset also disables all interrupts so do
not do that separately.
Also, the device_may_wakeup() checks are removed as the driver does not
have wakeup support.
Tested on Zynq-7000 integrated CAN.
Signed-off-by: Anssi Hannula <anssi.hannula@bitwise.fi>
Cc: Michal Simek <michal.simek@xilinx.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
xcan_interrupt() clears ERROR|RXOFLV|BSOFF|ARBLST interrupts if any of
them is asserted. This does not take into account that some of them
could have been asserted between interrupt status read and interrupt
clear, therefore clearing them without handling them.
Fix the code to only clear those interrupts that it knows are asserted
and therefore going to be processed in xcan_err_interrupt().
Fixes: b1201e44f5 ("can: xilinx CAN controller support")
Signed-off-by: Anssi Hannula <anssi.hannula@bitwise.fi>
Cc: Michal Simek <michal.simek@xilinx.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
RX overflow interrupt (RXOFLW) is disabled even though xcan_interrupt()
processes it. This means that an RX overflow interrupt will only be
processed when another interrupt gets asserted (e.g. for RX/TX).
Fix that by enabling the RXOFLW interrupt.
Fixes: b1201e44f5 ("can: xilinx CAN controller support")
Signed-off-by: Anssi Hannula <anssi.hannula@bitwise.fi>
Cc: Michal Simek <michal.simek@xilinx.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
The xilinx_can driver assumes that the TXOK interrupt only clears after
it has been acknowledged as many times as there have been successfully
sent frames.
However, the documentation does not mention such behavior, instead
saying just that the interrupt is cleared when the clear bit is set.
Similarly, testing seems to also suggest that it is immediately cleared
regardless of the amount of frames having been sent. Performing some
heavy TX load and then going back to idle has the tx_head drifting
further away from tx_tail over time, steadily reducing the amount of
frames the driver keeps in the TX FIFO (but not to zero, as the TXOK
interrupt always frees up space for 1 frame from the driver's
perspective, so frames continue to be sent) and delaying the local echo
frames.
The TX FIFO tracking is also otherwise buggy as it does not account for
TX FIFO being cleared after software resets, causing
BUG!, TX FIFO full when queue awake!
messages to be output.
There does not seem to be any way to accurately track the state of the
TX FIFO for local echo support while using the full TX FIFO.
The Zynq version of the HW (but not the soft-AXI version) has watermark
programming support and with it an additional TX-FIFO-empty interrupt
bit.
Modify the driver to only put 1 frame into TX FIFO at a time on soft-AXI
and 2 frames at a time on Zynq. On Zynq the TXFEMP interrupt bit is used
to detect whether 1 or 2 frames have been sent at interrupt processing
time.
Tested with the integrated CAN on Zynq-7000 SoC. The 1-frame-FIFO mode
was also tested.
An alternative way to solve this would be to drop local echo support but
keep using the full TX FIFO.
v2: Add FIFO space check before TX queue wake with locking to
synchronize with queue stop. This avoids waking the queue when xmit()
had just filled it.
v3: Keep local echo support and reduce the amount of frames in FIFO
instead as suggested by Marc Kleine-Budde.
Fixes: b1201e44f5 ("can: xilinx CAN controller support")
Signed-off-by: Anssi Hannula <anssi.hannula@bitwise.fi>
Cc: <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
The xilinx_can driver contains no mechanism for propagating recovery
from CAN_STATE_ERROR_WARNING and CAN_STATE_ERROR_PASSIVE.
Add such a mechanism by factoring the handling of
XCAN_STATE_ERROR_PASSIVE and XCAN_STATE_ERROR_WARNING out of
xcan_err_interrupt and checking for recovery after RX and TX if the
interface is in one of those states.
Tested with the integrated CAN on Zynq-7000 SoC.
Fixes: b1201e44f5 ("can: xilinx CAN controller support")
Signed-off-by: Anssi Hannula <anssi.hannula@bitwise.fi>
Cc: <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
If the device gets into a state where RXNEMP (RX FIFO not empty)
interrupt is asserted without RXOK (new frame received successfully)
interrupt being asserted, xcan_rx_poll() will continue to try to clear
RXNEMP without actually reading frames from RX FIFO. If the RX FIFO is
not empty, the interrupt will not be cleared and napi_schedule() will
just be called again.
This situation can occur when:
(a) xcan_rx() returns without reading RX FIFO due to an error condition.
The code tries to clear both RXOK and RXNEMP but RXNEMP will not clear
due to a frame still being in the FIFO. The frame will never be read
from the FIFO as RXOK is no longer set.
(b) A frame is received between xcan_rx_poll() reading interrupt status
and clearing RXOK. RXOK will be cleared, but RXNEMP will again remain
set as the new message is still in the FIFO.
I'm able to trigger case (b) by flooding the bus with frames under load.
There does not seem to be any benefit in using both RXNEMP and RXOK in
the way the driver does, and the polling example in the reference manual
(UG585 v1.10 18.3.7 Read Messages from RxFIFO) also says that either
RXOK or RXNEMP can be used for detecting incoming messages.
Fix the issue and simplify the RX processing by only using RXNEMP
without RXOK.
Tested with the integrated CAN on Zynq-7000 SoC.
Fixes: b1201e44f5 ("can: xilinx CAN controller support")
Signed-off-by: Anssi Hannula <anssi.hannula@bitwise.fi>
Cc: <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
The xilinx_can driver performs a software reset when an RX overrun is
detected. This causes the device to enter Configuration mode where no
messages are received or transmitted.
The documentation does not mention any need to perform a reset on an RX
overrun, and testing by inducing an RX overflow also indicated that the
device continues to work just fine without a reset.
Remove the software reset.
Tested with the integrated CAN on Zynq-7000 SoC.
Fixes: b1201e44f5 ("can: xilinx CAN controller support")
Signed-off-by: Anssi Hannula <anssi.hannula@bitwise.fi>
Cc: <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
napi_complete_done() allows to opt-in for gro_flush_timeout,
added back in linux-3.19, commit 3b47d30396
("net: gro: add a per device gro flush timer")
This allows for more efficient GRO aggregation without
sacrifying latencies.
Signed-off-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Conflicts:
drivers/net/ethernet/renesas/ravb_main.c
kernel/bpf/syscall.c
net/ipv4/ipmr.c
All three conflicts were cases of overlapping changes.
Signed-off-by: David S. Miller <davem@davemloft.net>
Instead of enabling/disabling clocks at several locations in the driver,
Use the runtime_pm framework. This consolidates the actions for runtime PM
In the appropriate callbacks and makes the driver more readable and mantainable.
Signed-off-by: Kedareswara rao Appana <appanad@xilinx.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
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 <socketcan@hartkopp.net>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
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 <dan.carpenter@oracle.com>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Vincent Mailhol