Message ID | 20190419051432.13538-1-kernel@martin.sperl.org |
---|---|
Headers | show |
Series | Microchip mcp25xxfd can controller driver | expand |
On 4/19/19 7:14 AM, kernel@martin.sperl.org wrote: > From: Martin Sperl <kernel@martin.sperl.org> > > Add un-optimized Can2.0 and CanFD support. > > CAN-Transmission and Optimizations and are separate patches > > On a Rasperry pi 3 it is already able to process Can2.0 Frames > with DLC=0 on a CAN bus with 1MHz. without losing any packets > on the SPI side. Packets still get lost inside the network stack. > > Signed-off-by: Martin Sperl <kernel@martin.sperl.org> [..] > diff --git a/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can.c b/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can.c > index f98b02ff057b..eabd7ca50645 100644 > --- a/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can.c > +++ b/drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can.c > @@ -38,23 +38,162 @@ > * for timestamping of RX frames as well as for TEF entries. > */ > > -/* Implementation notes: > - * > - * Right now we only use the CAN controller block to put us into deep sleep > - * this means that the oscillator clock is turned off. > - * So this is the only thing that we implement here right now > - */ > - > +#include <linux/can/core.h> > +#include <linux/can/dev.h> > #include <linux/device.h> > +#include <linux/interrupt.h> > #include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/regulator/consumer.h> > #include <linux/spi/spi.h> > > +#include "mcp25xxfd_base.h" > +#include "mcp25xxfd_can_debugfs.h" > +#include "mcp25xxfd_can_fifo.h" > +#include "mcp25xxfd_can_int.h" > +#include "mcp25xxfd_can_priv.h" > #include "mcp25xxfd_clock.h" > #include "mcp25xxfd_cmd.h" > +#include "mcp25xxfd_int.h" > #include "mcp25xxfd_priv.h" > #include "mcp25xxfd_regs.h" > > -static int mcp25xxfd_can_get_mode(struct mcp25xxfd_priv *priv, u32 *reg) > +#include <uapi/linux/can/netlink.h> > + > +/* module parameters */ > +unsigned int bw_sharing_log2bits; > +module_param(bw_sharing_log2bits, uint, 0664); > +MODULE_PARM_DESC(bw_sharing_log2bits, > + "Delay between 2 transmissions in number of arbitration bit times\n"); > +bool enable_edge_filter; > +module_param(enable_edge_filter, bool, 0664); > +MODULE_PARM_DESC(enable_edge_filter, > + "Enable ISO11898-1:2015 edge_filtering"); > +unsigned int tdc_mode = 2; > +module_param(tdc_mode, uint, 0664); > +MODULE_PARM_DESC(tdc_mode, > + "Transmitter Delay Mode - 0 = disabled, 1 = fixed, 2 = auto\n"); > +unsigned int tdc_value; > +module_param(tdc_value, uint, 0664); > +MODULE_PARM_DESC(tdc_value, > + "Transmission Delay Value - range: [0:63] SCLK"); > +int tdc_offset = 64; /* outside of range to use computed values */ > +module_param(tdc_offset, int, 0664); > +MODULE_PARM_DESC(tdc_offset, > + "Transmission Delay offset - range: [-64:63] SCLK"); > + > +/* everything related to bit timing */ > +static > +const struct can_bittiming_const mcp25xxfd_can_nominal_bittiming_const = { > + .name = DEVICE_NAME, > + .tseg1_min = 2, > + .tseg1_max = BIT(MCP25XXFD_CAN_NBTCFG_TSEG1_BITS), > + .tseg2_min = 1, > + .tseg2_max = BIT(MCP25XXFD_CAN_NBTCFG_TSEG2_BITS), > + .sjw_max = BIT(MCP25XXFD_CAN_NBTCFG_SJW_BITS), > + .brp_min = 1, > + .brp_max = BIT(MCP25XXFD_CAN_NBTCFG_BRP_BITS), > + .brp_inc = 1, > +}; > + > +static > +const struct can_bittiming_const mcp25xxfd_can_data_bittiming_const = { > + .name = DEVICE_NAME, > + .tseg1_min = 1, > + .tseg1_max = BIT(MCP25XXFD_CAN_DBTCFG_TSEG1_BITS), > + .tseg2_min = 1, > + .tseg2_max = BIT(MCP25XXFD_CAN_DBTCFG_TSEG2_BITS), > + .sjw_max = BIT(MCP25XXFD_CAN_DBTCFG_SJW_BITS), > + .brp_min = 1, > + .brp_max = BIT(MCP25XXFD_CAN_DBTCFG_BRP_BITS), > + .brp_inc = 1, > +}; > + > +static int mcp25xxfd_can_do_set_nominal_bittiming(struct net_device *net) > +{ > + struct mcp25xxfd_can_priv *cpriv = netdev_priv(net); > + struct can_bittiming *bt = &cpriv->can.bittiming; > + > + int sjw = bt->sjw; > + int pseg2 = bt->phase_seg2; > + int pseg1 = bt->phase_seg1; > + int propseg = bt->prop_seg; > + int brp = bt->brp; > + > + int tseg1 = propseg + pseg1; > + int tseg2 = pseg2; > + > + /* calculate nominal bit timing */ > + cpriv->regs.nbtcfg = ((sjw - 1) << MCP25XXFD_CAN_NBTCFG_SJW_SHIFT) | > + ((tseg2 - 1) << MCP25XXFD_CAN_NBTCFG_TSEG2_SHIFT) | > + ((tseg1 - 1) << MCP25XXFD_CAN_NBTCFG_TSEG1_SHIFT) | > + ((brp - 1) << MCP25XXFD_CAN_NBTCFG_BRP_SHIFT); > + > + return mcp25xxfd_cmd_write(cpriv->priv->spi, MCP25XXFD_CAN_NBTCFG, > + cpriv->regs.nbtcfg); > +} > + > +static int mcp25xxfd_can_do_set_data_bittiming(struct net_device *net) > +{ > + struct mcp25xxfd_can_priv *cpriv = netdev_priv(net); > + struct mcp25xxfd_priv *priv = cpriv->priv; > + struct can_bittiming *bt = &cpriv->can.data_bittiming; > + struct spi_device *spi = priv->spi; > + > + int sjw = bt->sjw; > + int pseg2 = bt->phase_seg2; > + int pseg1 = bt->phase_seg1; > + int propseg = bt->prop_seg; > + int brp = bt->brp; > + > + int tseg1 = propseg + pseg1; > + int tseg2 = pseg2; > + > + int tdco; > + int ret; > + > + /* set up Transmitter delay compensation */ > + cpriv->regs.tdc = 0; > + /* configure TDC mode */ > + if (tdc_mode < 4) > + cpriv->regs.tdc = tdc_mode << MCP25XXFD_CAN_TDC_TDCMOD_SHIFT; > + else > + cpriv->regs.tdc = MCP25XXFD_CAN_TDC_TDCMOD_AUTO << > + MCP25XXFD_CAN_TDC_TDCMOD_SHIFT; > + > + /* configure TDC offsets */ > + if ((tdc_offset >= -64) && tdc_offset < 64) > + tdco = tdc_offset; > + else > + tdco = clamp_t(int, bt->brp * tseg1, -64, 63); > + cpriv->regs.tdc |= (tdco << MCP25XXFD_CAN_TDC_TDCO_SHIFT) & > + MCP25XXFD_CAN_TDC_TDCO_MASK; > + > + /* configure TDC value */ > + if (tdc_value < 64) > + cpriv->regs.tdc |= tdc_value << MCP25XXFD_CAN_TDC_TDCV_SHIFT; > + > + /* enable edge filtering */ > + if (enable_edge_filter) > + cpriv->regs.tdc |= MCP25XXFD_CAN_TDC_EDGFLTEN; > + > + /* set TDC */ > + ret = mcp25xxfd_cmd_write(spi, MCP25XXFD_CAN_TDC, cpriv->regs.tdc); > + if (ret) > + return ret; > + > + /* calculate data bit timing */ > + cpriv->regs.dbtcfg = ((sjw - 1) << MCP25XXFD_CAN_DBTCFG_SJW_SHIFT) | > + ((tseg2 - 1) << MCP25XXFD_CAN_DBTCFG_TSEG2_SHIFT) | > + ((tseg1 - 1) << MCP25XXFD_CAN_DBTCFG_TSEG1_SHIFT) | > + ((brp - 1) << MCP25XXFD_CAN_DBTCFG_BRP_SHIFT); > + > + return mcp25xxfd_cmd_write(spi, MCP25XXFD_CAN_DBTCFG, > + cpriv->regs.dbtcfg); > +} > + > +int mcp25xxfd_can_get_mode(struct mcp25xxfd_priv *priv, u32 *reg) > { > int ret; > > @@ -66,11 +205,11 @@ static int mcp25xxfd_can_get_mode(struct mcp25xxfd_priv *priv, u32 *reg) > MCP25XXFD_CAN_CON_OPMOD_SHIFT; > } > > -static int mcp25xxfd_can_switch_mode(struct mcp25xxfd_priv *priv, > - u32 *reg, int mode) > +int mcp25xxfd_can_switch_mode_no_wait(struct mcp25xxfd_priv *priv, > + u32 *reg, int mode) > { > u32 dummy; > - int ret, i; > + int ret; > > /* get the current mode/register - if reg is NULL > * when the can controller is not setup yet > @@ -78,9 +217,11 @@ static int mcp25xxfd_can_switch_mode(struct mcp25xxfd_priv *priv, > * (this only happens during initialization phase) > */ > if (reg) { > - ret = mcp25xxfd_can_get_mode(priv, reg); > - if (ret < 0) > - return ret; > + if (!reg) { > + ret = mcp25xxfd_can_get_mode(priv, reg); > + if (ret < 0) > + return ret; > + } After this patch the function reads: > static int mcp25xxfd_can_switch_mode_no_wait(struct mcp25xxfd_priv *priv, > u32 *reg, int mode) > { > u32 dummy; > int ret; > > /* get the current mode/register - if reg is NULL > * when the can controller is not setup yet > * typically by calling mcp25xxfd_can_sleep_mode > * (this only happens during initialization phase) > */ > if (reg) { > if (!reg) { This looks wrong. > ret = mcp25xxfd_can_get_mode(priv, reg); > if (ret < 0) > return ret; > } > } else { > /* alternatively use dummy */ > dummy = 0; > reg = &dummy; > } > Marc
Hi Marc! > On 08.05.2019, at 15:12, Marc Kleine-Budde <mkl@pengutronix.de> wrote: >> static int mcp25xxfd_can_switch_mode_no_wait(struct mcp25xxfd_priv *priv, >> u32 *reg, int mode) >> { >> u32 dummy; >> int ret; >> >> /* get the current mode/register - if reg is NULL >> * when the can controller is not setup yet >> * typically by calling mcp25xxfd_can_sleep_mode >> * (this only happens during initialization phase) >> */ >> if (reg) { >> if (!reg) { > > This looks wrong. should be: >> if (!*reg) { Fixed that - thanks! Thanks, Martin
From: Martin Sperl <kernel@martin.sperl.org> This patchset adds a driver for the mcp25xxfd CanFD controller. Most of the features of the controller are supported by this driver release. The controller includes a few features that the current core Can(FD) implementation does not support: * Transmit Bandwidth Sharing bits - this waits for a configurable number of syncronization bits before atempting to transmit the next frame - for the moment controllable via a module parameter * SID11 with CanFD frames - at this moment not supported by driver * 3 transmit attempts in addition to ONE_SHOT * transmitter delay compensation configurations * micro second exact transmission and reception timestamps - used internally by driver * variable number of tx-fifos - exposed via module parameter at this moment The driver driver has been split into several patches - first just initial setup of each section and then driver optimization patches. As requested the driver has now also been split out into smaller files for individual components of the driver - in the hope that this has not been producing regressions... The driver is able to handle reception of 99.95% of all CAN frames of a 100% saturated 1MHz Can2.0 Bus with Frames with standard IDs and DLC=0 on a Raspberry Pi 3. Note that this statistics is without injection into the network stack, which then drops about 60% of all frames. At this time the SPI bus is utilized 75% (when counting CS asserted) and 60% from a SPI-Clock perspective. On the Rasberry pi 3 in such a situation we are using 100% of one CPU for the interrupt thread (as the spi driver is using polling for short transfers) and this has exposed a performance inefficiency in the spi framework that shows as the the spi-worker thread also consuming CPU cycles (30% of one CPU in this test case) unnecessarily. The driver also allows to use the INT0/1 pins as GPIOs. Lots of internal data/statistics is exposed via debugfs. On the HW side some errata related to SRAM access have been found during the development of the patchsets. Since then an errata for the controller has been shared by microchip. At this very moment there is no 100% mitigation for this (besides a newer version of the silicon). The only instance where this is probably not an issue is when using a SPI bus driver that reliably deasserts CS in the "requested" time-frame of a few Can clock cycles. When the spi-bus-driver is using GPIO-CS (which is nowadays the preferred implementation of spi-bus-drivers) then we have to rely on interrupt and interrupt worker thread scheduling latencies to be fast enough to fullfill the requirements of the mcp2517fd of SCK stop to CS-deassert. To achive this a faster CPU is needed and preferably more cores. A lot of possible mitigations have been investigated (Fifo ordering in SRAM, the use of CRC SPI commands,...), but none have been successful. That is why we are only able to reach 99.95% rate, because when this issue occurres then there is the need to reconfigure the controller and that takes some time which can not get used to process frames. Fortunately this only happens on highly congested systems, but it is more likely to occur on low end systems. So as an example almost everything related to can-transmission can get found in mcp25xxfd_can_tx.c. mcp25xxfd_can_rx.c is responsible for can-reception and mcp25xxfd_can_int.c for interrupt handling. Unfortunately the limit of the linux-can mailing-list seems lower than 128k (and thus patch 4 of the series seems not to have been accepted). Thus V7 is essentially splitting V6 patch 4 into basic Can functionality with Can RX working in V7 patch 4 and TX-support in V7 patch 5. Changelog: V1 -> V2: new more generic name based on feedback from microchip cleanup of code (checkpatch) address all feedback on code handle (HW) systemerrors in a much better/reliable way cleanup of dt custom properties removing (most) gpio related properties implemented GPIOLIB support as per feedback V2 -> V3: added vendor-prefix for gpio-opendrain to dt-binding added gpio-controller to dt-binding added feedback by Rob Herring waited for other feedback regarding the code V3 -> V4: resend Patch-1 (dt) added: Reviewed-by: Rob Herring <robh@kernel.org> V4 -> V5: reorganisation of the patchset into smaller patches review of the whole driver code for better modularization V5 -> V6: Major refactoring as per feedback from Wilhelm Grandegger Fixing bugs reported by several other parties Split out of optimizations into separate patches V6 -> V7: added include linux/irqreturn.h (feedback by Eric Scholz) move can transmission into a separate patch to reduce the size of the individual patches to make the linux-can mailing-list happy... Martin Sperl (10): dt-binding: can: mcp25xxfd: document device tree bindings can: mcp25xxfd: Add Microchip mcp25xxfd CAN FD driver basics can: mcp25xxfd: add gpiolib support for GPIO0/1 (aka. INT0/INT1) can: mcp25xxfd: Add Microchip mcp25xxfd CAN FD driver can: mcp25xxfd: Add Can transmission support can: mcp25xxfd: optimize TEF read avoiding unnecessary SPI transfers can: mcp25xxfd: optimize TEF reads reading multiple TEFs in one go can: mcp25xxfd: optimize SPI reads of FIFOs in can2.0 mode can: mcp25xxfd: add prediction of CanFD frames sizes based on history can: mcp25xxfd: optimize reception of big CanFD frame reception with BRS .../bindings/net/can/microchip,mcp25xxfd.txt | 32 + drivers/net/can/spi/Kconfig | 2 + drivers/net/can/spi/Makefile | 2 + drivers/net/can/spi/mcp25xxfd/Kconfig | 5 + drivers/net/can/spi/mcp25xxfd/Makefile | 18 + drivers/net/can/spi/mcp25xxfd/mcp25xxfd_base.c | 281 ++++++++ drivers/net/can/spi/mcp25xxfd/mcp25xxfd_base.h | 14 + drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can.c | 684 ++++++++++++++++++ drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can.h | 56 ++ .../net/can/spi/mcp25xxfd/mcp25xxfd_can_debugfs.c | 235 ++++++ .../net/can/spi/mcp25xxfd/mcp25xxfd_can_debugfs.h | 44 ++ drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_fifo.c | 347 +++++++++ drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_fifo.h | 16 + drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_id.h | 69 ++ drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_int.c | 706 ++++++++++++++++++ drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_int.h | 17 + drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_priv.h | 203 ++++++ drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_rx.c | 521 ++++++++++++++ drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_rx.h | 18 + drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_tx.c | 794 +++++++++++++++++++++ drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_tx.h | 86 +++ drivers/net/can/spi/mcp25xxfd/mcp25xxfd_clock.c | 485 +++++++++++++ drivers/net/can/spi/mcp25xxfd/mcp25xxfd_clock.h | 28 + drivers/net/can/spi/mcp25xxfd/mcp25xxfd_cmd.c | 312 ++++++++ drivers/net/can/spi/mcp25xxfd/mcp25xxfd_cmd.h | 84 +++ drivers/net/can/spi/mcp25xxfd/mcp25xxfd_crc.c | 31 + drivers/net/can/spi/mcp25xxfd/mcp25xxfd_crc.h | 15 + drivers/net/can/spi/mcp25xxfd/mcp25xxfd_debugfs.c | 110 +++ drivers/net/can/spi/mcp25xxfd/mcp25xxfd_debugfs.h | 30 + drivers/net/can/spi/mcp25xxfd/mcp25xxfd_ecc.c | 75 ++ drivers/net/can/spi/mcp25xxfd/mcp25xxfd_ecc.h | 16 + drivers/net/can/spi/mcp25xxfd/mcp25xxfd_gpio.c | 194 +++++ drivers/net/can/spi/mcp25xxfd/mcp25xxfd_gpio.h | 16 + drivers/net/can/spi/mcp25xxfd/mcp25xxfd_int.c | 73 ++ drivers/net/can/spi/mcp25xxfd/mcp25xxfd_int.h | 15 + drivers/net/can/spi/mcp25xxfd/mcp25xxfd_priv.h | 83 +++ drivers/net/can/spi/mcp25xxfd/mcp25xxfd_regs.h | 661 +++++++++++++++++ 37 files changed, 6378 insertions(+) create mode 100644 Documentation/devicetree/bindings/net/can/microchip,mcp25xxfd.txt create mode 100644 drivers/net/can/spi/mcp25xxfd/Kconfig create mode 100644 drivers/net/can/spi/mcp25xxfd/Makefile create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd_base.c create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd_base.h create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can.c create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can.h create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_debugfs.c create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_debugfs.h create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_fifo.c create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_fifo.h create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_id.h create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_int.c create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_int.h create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_priv.h create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_rx.c create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_rx.h create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_tx.c create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd_can_tx.h create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd_clock.c create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd_clock.h create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd_cmd.c create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd_cmd.h create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd_crc.c create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd_crc.h create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd_debugfs.c create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd_debugfs.h create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd_ecc.c create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd_ecc.h create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd_gpio.c create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd_gpio.h create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd_int.c create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd_int.h create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd_priv.h create mode 100644 drivers/net/can/spi/mcp25xxfd/mcp25xxfd_regs.h -- 2.11.0