* [PATCH v5 0/2] can: Allwinner A10/A20 CAN Controller support - summary
@ 2015-09-13 11:43 Gerhard Bertelsmann
2015-09-13 11:43 ` [PATCH v5 1/2] can: Allwinner A10/A20 CAN Controller support - Devicetree bindings Gerhard Bertelsmann
` (2 more replies)
0 siblings, 3 replies; 15+ messages in thread
From: Gerhard Bertelsmann @ 2015-09-13 11:43 UTC (permalink / raw)
To: linux-can; +Cc: linux-sunxi, mkl, Gerhard Bertelsmann
Hi,
please find attached the next try. Thanks to Marc. I've worked on all mentioned
issues except one: I don't add clk_disable_unprepare in sunxi_can_start. Makes no
sense to me (driver fails).
For testers:
the patch will only apply wihtout modification to kernels 3.19+.
I've tested the driver (receipt, sending and basic errors). IMHO
everything is fine. The performance seems to be good - high rates
(>5000 packets/second) working fine.
Please test and report bugs if exists.
[PATCH v5 1/2] Device Tree Binding Documentation
[PATCH v5 2/2] Kernel Module
History:
V5: fix license
modify prefix to mode select defines
enable and disable clock in sunxican_get_berr_counter
delete set_normal_mode at the end of sunxi_can_start
removed sunxican_id_table
use devm_clk_get instead of clk_get
use devm_ioremap_resource to simplify probe and remove
make set-normal-mode and set-reset-mode more readable
v4: defines prefixed with SUNXI_
sunxi_can_write_cmdreg tweaked
loops in set_xxx_mode reworked
add return value to set_xxx_mode
sunxican_start_xmit reworked
struct platform_driver stripped
moved set_bittiming into open
moved clock start into open
add clock stop to close
suspend reworked
resume reworked
fixed double counting bug
v3: changed error state change handling (thx Andri for the hint)
use bittiming function correct (no need to call it)
strip down priv (suggested by Marc)
scripts/checkpatch.pl-> no matches anymore
sparse -> no errors or warnings anymore
v2: cleaning
v1: initial
Signed-off-by: Gerhard Bertelsmann <info@gerhard-bertelsmann.de>
---
.../devicetree/bindings/net/can/sunxi_can.txt | 40 +
drivers/net/can/Kconfig | 10 +
drivers/net/can/Makefile | 1 +
drivers/net/can/sunxi_can.c | 879 +++++++++++++++++++++
4 files changed, 930 insertions(+)
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v5 1/2] can: Allwinner A10/A20 CAN Controller support - Devicetree bindings
2015-09-13 11:43 [PATCH v5 0/2] can: Allwinner A10/A20 CAN Controller support - summary Gerhard Bertelsmann
@ 2015-09-13 11:43 ` Gerhard Bertelsmann
[not found] ` <1442144632-4541-2-git-send-email-info-La43T0Mi4bH5xCKuJOYmCvaTkwRoYoCU@public.gmane.org>
2015-09-13 11:43 ` [PATCH v5 2/2] can: Allwinner A10/A20 CAN Controller support - controller code Gerhard Bertelsmann
[not found] ` <1442144632-4541-1-git-send-email-info-La43T0Mi4bH5xCKuJOYmCvaTkwRoYoCU@public.gmane.org>
2 siblings, 1 reply; 15+ messages in thread
From: Gerhard Bertelsmann @ 2015-09-13 11:43 UTC (permalink / raw)
To: linux-can; +Cc: linux-sunxi, mkl, Gerhard Bertelsmann
Signed-off-by: Gerhard Bertelsmann <info@gerhard-bertelsmann.de>
---
.../devicetree/bindings/net/can/sunxi_can.txt | 40 +
1 files changed, 49 insertions(+)
diff --git a/Documentation/devicetree/bindings/net/can/sunxi_can.txt b/Documentation/devicetree/bindings/net/can/sunxi_can.txt
new file mode 100644
index 0000000..5db9abc
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/can/sunxi_can.txt
@@ -0,0 +1,40 @@
+Allwinner A10/A20 CAN controller Device Tree Bindings
+-----------------------------------------------------
+
+Required properties:
+- compatible: "allwinner,sunxican"
+- reg: physical base address and size of the Allwinner A10/A20 CAN register map.
+- interrupts: interrupt specifier for the sole interrupt.
+- clocks: phandle and clock specifier.
+- pinctrl-0: pin control group to be used for this controller.
+- pinctrl-names: must be "default".
+
+
+Example
+-------
+
+SoC common .dtsi file:
+
+ can0_pins_a: can0@0 {
+ allwinner,pins = "PH20","PH21";
+ allwinner,function = "can";
+ allwinner,drive = <0>;
+ allwinner,pull = <0>;
+ };
+...
+ can0: can@01c2bc00 {
+ compatible = "allwinner,sunxican";
+ reg = <0x01c2bc00 0x400>;
+ interrupts = <0 26 4>;
+ clocks = <&apb1_gates 4>;
+ status = "disabled";
+ };
+
+Board specific .dts file:
+
+ can0: can@01c2bc00 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&can0_pins_a>;
+ status = "okay";
+ };
+
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v5 2/2] can: Allwinner A10/A20 CAN Controller support - controller code
2015-09-13 11:43 [PATCH v5 0/2] can: Allwinner A10/A20 CAN Controller support - summary Gerhard Bertelsmann
2015-09-13 11:43 ` [PATCH v5 1/2] can: Allwinner A10/A20 CAN Controller support - Devicetree bindings Gerhard Bertelsmann
@ 2015-09-13 11:43 ` Gerhard Bertelsmann
[not found] ` <1442144632-4541-3-git-send-email-info-La43T0Mi4bH5xCKuJOYmCvaTkwRoYoCU@public.gmane.org>
[not found] ` <1442144632-4541-1-git-send-email-info-La43T0Mi4bH5xCKuJOYmCvaTkwRoYoCU@public.gmane.org>
2 siblings, 1 reply; 15+ messages in thread
From: Gerhard Bertelsmann @ 2015-09-13 11:43 UTC (permalink / raw)
To: linux-can; +Cc: linux-sunxi, mkl, Gerhard Bertelsmann
Signed-off-by: Gerhard Bertelsmann <info@gerhard-bertelsmann.de>
---
drivers/net/can/Kconfig | 10 +
drivers/net/can/Makefile | 1 +
drivers/net/can/sunxi_can.c | 879 +++++++++++++++++++++
3 files changed, 890 insertions(+)
diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
index e8c96b8..439f67c 100644
--- a/drivers/net/can/Kconfig
+++ b/drivers/net/can/Kconfig
@@ -129,6 +129,16 @@ config CAN_RCAR
To compile this driver as a module, choose M here: the module will
be called rcar_can.
+config CAN_SUNXI
+ tristate "Allwinner SUNXI CAN controller"
+ depends on MACH_SUN4I || MACH_SUN7I || COMPILE_TEST
+ ---help---
+ Say Y here if you want to use CAN controller found on Allwinner
+ A10/A20 SoCs.
+
+ To compile this driver as a module, choose M here: the module will
+ be called rcar_can.
+
config CAN_XILINXCAN
tristate "Xilinx CAN"
depends on ARCH_ZYNQ || ARM64 || MICROBLAZE || COMPILE_TEST
diff --git a/drivers/net/can/Makefile b/drivers/net/can/Makefile
index c533c62..b3e825c 100644
--- a/drivers/net/can/Makefile
+++ b/drivers/net/can/Makefile
@@ -27,6 +27,7 @@ obj-$(CONFIG_CAN_FLEXCAN) += flexcan.o
obj-$(CONFIG_PCH_CAN) += pch_can.o
obj-$(CONFIG_CAN_GRCAN) += grcan.o
obj-$(CONFIG_CAN_RCAR) += rcar_can.o
+obj-$(CONFIG_CAN_SUNXI) += sunxi_can.o
obj-$(CONFIG_CAN_XILINXCAN) += xilinx_can.o
subdir-ccflags-y += -D__CHECK_ENDIAN__
diff --git a/drivers/net/can/sunxi_can.c b/drivers/net/can/sunxi_can.c
new file mode 100644
index 0000000..569e6cd
--- /dev/null
+++ b/drivers/net/can/sunxi_can.c
@@ -0,0 +1,879 @@
+/*
+ * sunxi_can.c - CAN bus controller driver for Allwinner SUN4I&SUN7I based SoCs
+ *
+ * Copyright (C) 2013 Peter Chen
+ * Copyright (C) 2015 Gerhard Bertelsmann
+ * All rights reserved.
+ *
+ * Parts of this software are based on (derived from) the SJA1000 code by:
+ * Copyright (C) 2014 Oliver Hartkopp <oliver.hartkopp@volkswagen.de>
+ * Copyright (C) 2007 Wolfgang Grandegger <wg@grandegger.com>
+ * Copyright (C) 2002-2007 Volkswagen Group Electronic Research
+ * Copyright (C) 2003 Matthias Brukner, Trajet Gmbh, Rebenring 33,
+ * 38106 Braunschweig, GERMANY
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ * 3. Neither the name of Volkswagen nor the names of its contributors
+ * may be used to endorse or promote products derived from this software
+ * without specific prior written permission.
+ *
+ * Alternatively, provided that this notice is retained in full, this
+ * software may be distributed under the terms of the GNU General
+ * Public License ("GPL") version 2, in which case the provisions of the
+ * GPL apply INSTEAD OF those given above.
+ *
+ * The provided data structures and external interfaces from this code
+ * are not restricted to be used by modules with a GPL compatible license.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
+ * DAMAGE.
+ *
+ */
+
+#include <linux/netdevice.h>
+#include <linux/can.h>
+#include <linux/can/dev.h>
+#include <linux/can/error.h>
+#include <linux/can/led.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/interrupt.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+
+#define DRV_NAME "sunxi_can"
+#define DRV_MODULE_VERSION "0.99"
+
+/* Registers address (physical base address 0x01C2BC00) */
+#define SUNXI_REG_MSEL_ADDR 0x0000 /* CAN Mode Select */
+#define SUNXI_REG_CMD_ADDR 0x0004 /* CAN Command */
+#define SUNXI_REG_STA_ADDR 0x0008 /* CAN Status */
+#define SUNXI_REG_INT_ADDR 0x000c /* CAN Interrupt Flag */
+#define SUNXI_REG_INTEN_ADDR 0x0010 /* CAN Interrupt Enable */
+#define SUNXI_REG_BTIME_ADDR 0x0014 /* CAN Bus Timing 0 */
+#define SUNXI_REG_TEWL_ADDR 0x0018 /* CAN Tx Error Warning Limit */
+#define SUNXI_REG_ERRC_ADDR 0x001c /* CAN Error Counter */
+#define SUNXI_REG_RMCNT_ADDR 0x0020 /* CAN Receive Message Counter */
+#define SUNXI_REG_RBUFSA_ADDR 0x0024 /* CAN Receive Buffer Start Address */
+#define SUNXI_REG_BUF0_ADDR 0x0040 /* CAN Tx/Rx Buffer 0 */
+#define SUNXI_REG_BUF1_ADDR 0x0044 /* CAN Tx/Rx Buffer 1 */
+#define SUNXI_REG_BUF2_ADDR 0x0048 /* CAN Tx/Rx Buffer 2 */
+#define SUNXI_REG_BUF3_ADDR 0x004c /* CAN Tx/Rx Buffer 3 */
+#define SUNXI_REG_BUF4_ADDR 0x0050 /* CAN Tx/Rx Buffer 4 */
+#define SUNXI_REG_BUF5_ADDR 0x0054 /* CAN Tx/Rx Buffer 5 */
+#define SUNXI_REG_BUF6_ADDR 0x0058 /* CAN Tx/Rx Buffer 6 */
+#define SUNXI_REG_BUF7_ADDR 0x005c /* CAN Tx/Rx Buffer 7 */
+#define SUNXI_REG_BUF8_ADDR 0x0060 /* CAN Tx/Rx Buffer 8 */
+#define SUNXI_REG_BUF9_ADDR 0x0064 /* CAN Tx/Rx Buffer 9 */
+#define SUNXI_REG_BUF10_ADDR 0x0068 /* CAN Tx/Rx Buffer 10 */
+#define SUNXI_REG_BUF11_ADDR 0x006c /* CAN Tx/Rx Buffer 11 */
+#define SUNXI_REG_BUF12_ADDR 0x0070 /* CAN Tx/Rx Buffer 12 */
+#define SUNXI_REG_ACPC_ADDR 0x0040 /* CAN Acceptance Code 0 */
+#define SUNXI_REG_ACPM_ADDR 0x0044 /* CAN Acceptance Mask 0 */
+#define SUNXI_REG_RBUF_RBACK_START_ADDR 0x0180 /* CAN transmit buffer start */
+#define SUNXI_REG_RBUF_RBACK_END_ADDR 0x01b0 /* CAN transmit buffer end */
+
+/* Controller Register Description */
+
+/* mode select register (r/w)
+ * offset:0x0000 default:0x0000_0001
+ */
+#define SUNXI_MSEL_SLEEP_MODE (0x01 << 4) /* write in reset mode */
+#define SUNXI_MSEL_WAKE_UP (0x00 << 4)
+#define SUNXI_MSEL_SINGLE_FILTER (0x01 << 3) /* write in reset mode */
+#define SUNXI_MSEL_DUAL_FILTERS (0x00 << 3)
+#define SUNXI_MSEL_LOOPBACK_MODE BIT(2)
+#define SUNXI_MSEL_LISTEN_ONLY_MODE BIT(1)
+#define SUNXI_MSEL_RESET_MODE BIT(0)
+
+/* command register (w)
+ * offset:0x0004 default:0x0000_0000
+ */
+#define SUNXI_CMD_BUS_OFF_REQ BIT(5)
+#define SUNXI_CMD_SELF_RCV_REQ BIT(4)
+#define SUNXI_CMD_CLEAR_OR_FLAG BIT(3)
+#define SUNXI_CMD_RELEASE_RBUF BIT(2)
+#define SUNXI_CMD_ABORT_REQ BIT(1)
+#define SUNXI_CMD_TRANS_REQ BIT(0)
+
+/* status register (r)
+ * offset:0x0008 default:0x0000_003c
+ */
+#define SUNXI_STA_BIT_ERR (0x00 << 22)
+#define SUNXI_STA_FORM_ERR (0x01 << 22)
+#define SUNXI_STA_STUFF_ERR (0x02 << 22)
+#define SUNXI_STA_OTHER_ERR (0x03 << 22)
+#define SUNXI_STA_MASK_ERR (0x03 << 22)
+#define SUNXI_STA_ERR_DIR BIT(21)
+#define SUNXI_STA_ERR_SEG_CODE (0x1f << 16)
+#define SUNXI_STA_START (0x03 << 16)
+#define SUNXI_STA_ID28_21 (0x02 << 16)
+#define SUNXI_STA_ID20_18 (0x06 << 16)
+#define SUNXI_STA_SRTR (0x04 << 16)
+#define SUNXI_STA_IDE (0x05 << 16)
+#define SUNXI_STA_ID17_13 (0x07 << 16)
+#define SUNXI_STA_ID12_5 (0x0f << 16)
+#define SUNXI_STA_ID4_0 (0x0e << 16)
+#define SUNXI_STA_RTR (0x0c << 16)
+#define SUNXI_STA_RB1 (0x0d << 16)
+#define SUNXI_STA_RB0 (0x09 << 16)
+#define SUNXI_STA_DLEN (0x0b << 16)
+#define SUNXI_STA_DATA_FIELD (0x0a << 16)
+#define SUNXI_STA_CRC_SEQUENCE (0x08 << 16)
+#define SUNXI_STA_CRC_DELIMITER (0x18 << 16)
+#define SUNXI_STA_ACK (0x19 << 16)
+#define SUNXI_STA_ACK_DELIMITER (0x1b << 16)
+#define SUNXI_STA_END (0x1a << 16)
+#define SUNXI_STA_INTERMISSION (0x12 << 16)
+#define SUNXI_STA_ACTIVE_ERROR (0x11 << 16)
+#define SUNXI_STA_PASSIVE_ERROR (0x16 << 16)
+#define SUNXI_STA_TOLERATE_DOMINANT_BITS (0x13 << 16)
+#define SUNXI_STA_ERROR_DELIMITER (0x17 << 16)
+#define SUNXI_STA_OVERLOAD (0x1c << 16)
+#define SUNXI_STA_BUS_OFF BIT(7)
+#define SUNXI_STA_ERR_STA BIT(6)
+#define SUNXI_STA_TRANS_BUSY BIT(5)
+#define SUNXI_STA_RCV_BUSY BIT(4)
+#define SUNXI_STA_TRANS_OVER BIT(3)
+#define SUNXI_STA_TBUF_RDY BIT(2)
+#define SUNXI_STA_DATA_ORUN BIT(1)
+#define SUNXI_STA_RBUF_RDY BIT(0)
+
+/* interrupt register (r)
+ * offset:0x000c default:0x0000_0000
+ */
+#define SUNXI_INT_BUS_ERR BIT(7)
+#define SUNXI_INT_ARB_LOST BIT(6)
+#define SUNXI_INT_ERR_PASSIVE BIT(5)
+#define SUNXI_INT_WAKEUP BIT(4)
+#define SUNXI_INT_DATA_OR BIT(3)
+#define SUNXI_INT_ERR_WRN BIT(2)
+#define SUNXI_INT_TBUF_VLD BIT(1)
+#define SUNXI_INT_RBUF_VLD BIT(0)
+
+/* interrupt enable register (r/w)
+ * offset:0x0010 default:0x0000_0000
+ */
+#define SUNXI_INTEN_BERR BIT(7)
+#define SUNXI_INTEN_ARB_LOST BIT(6)
+#define SUNXI_INTEN_ERR_PASSIVE BIT(5)
+#define SUNXI_INTEN_WAKEUP BIT(4)
+#define SUNXI_INTEN_OR BIT(3)
+#define SUNXI_INTEN_ERR_WRN BIT(2)
+#define SUNXI_INTEN_TX BIT(1)
+#define SUNXI_INTEN_RX BIT(0)
+
+/* error code */
+#define SUNXI_ERR_INRCV (0x1 << 5)
+#define SUNXI_ERR_INTRANS (0x0 << 5)
+
+/* filter mode */
+#define SINXI_FILTER_CLOSE 0
+#define SUNXI_SINGLE_FLTER_MODE 1
+#define SUNXI_DUAL_FILTER_MODE 2
+
+/* message buffer flags */
+#define SUNXI_MSG_EFF_FLAG BIT(7)
+#define SUNXI_MSG_RTR_FLAG BIT(6)
+
+/* max. number of interrupts handled in ISR */
+#define SUNXI_CAN_MAX_IRQ 20
+#define SUNXI_MODE_MAX_RETRIES 100
+
+struct sunxican_priv {
+ struct can_priv can;
+ void __iomem *base;
+ struct clk *clk;
+ spinlock_t cmdreg_lock; /* lock for concurrent cmd register writes */
+};
+
+static const struct can_bittiming_const sunxican_bittiming_const = {
+ .name = DRV_NAME,
+ .tseg1_min = 1,
+ .tseg1_max = 16,
+ .tseg2_min = 1,
+ .tseg2_max = 8,
+ .sjw_max = 4,
+ .brp_min = 1,
+ .brp_max = 64,
+ .brp_inc = 1,
+};
+
+static void sunxi_can_write_cmdreg(struct sunxican_priv *priv, u8 val)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&priv->cmdreg_lock, flags);
+ writel(val, priv->base + SUNXI_REG_CMD_ADDR);
+ spin_unlock_irqrestore(&priv->cmdreg_lock, flags);
+}
+
+static int set_normal_mode(struct net_device *dev)
+{
+ struct sunxican_priv *priv = netdev_priv(dev);
+ int retry = SUNXI_MODE_MAX_RETRIES;
+ u32 mod_reg_val = 0;
+
+ do {
+ mod_reg_val = readl(priv->base + SUNXI_REG_MSEL_ADDR);
+ mod_reg_val &= ~SUNXI_MSEL_RESET_MODE;
+ writel(mod_reg_val, priv->base + SUNXI_REG_MSEL_ADDR);
+ } while (retry-- && (mod_reg_val & SUNXI_MSEL_RESET_MODE));
+
+ if (readl(priv->base + SUNXI_REG_MSEL_ADDR) & SUNXI_MSEL_RESET_MODE) {
+ netdev_err(dev,
+ "setting controller into normal mode failed!\n");
+ return -ETIMEDOUT;
+ }
+
+ priv->can.state = CAN_STATE_ERROR_ACTIVE;
+ /* enable interrupts */
+ if (priv->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING)
+ writel(0xFFFF, priv->base + SUNXI_REG_INTEN_ADDR);
+ else
+ writel(0xFFFF & ~SUNXI_INTEN_BERR,
+ priv->base + SUNXI_REG_INTEN_ADDR);
+
+ mod_reg_val = readl(priv->base + SUNXI_REG_MSEL_ADDR);
+
+ if (priv->can.ctrlmode & CAN_CTRLMODE_PRESUME_ACK)
+ mod_reg_val |= SUNXI_MSEL_LOOPBACK_MODE;
+ else if (priv->can.ctrlmode & CAN_CTRLMODE_LISTENONLY)
+ mod_reg_val |= SUNXI_MSEL_LISTEN_ONLY_MODE;
+
+ writel(mod_reg_val, priv->base + SUNXI_REG_MSEL_ADDR);
+ return 0;
+}
+
+static int set_reset_mode(struct net_device *dev)
+{
+ struct sunxican_priv *priv = netdev_priv(dev);
+ int retry = SUNXI_MODE_MAX_RETRIES;
+ u32 mod_reg_val = 0;
+
+ do {
+ mod_reg_val = readl(priv->base + SUNXI_REG_MSEL_ADDR);
+ mod_reg_val |= SUNXI_MSEL_RESET_MODE;
+ writel(mod_reg_val, priv->base + SUNXI_REG_MSEL_ADDR);
+ } while (retry-- && !(mod_reg_val & SUNXI_MSEL_RESET_MODE));
+
+ if (!(readl(priv->base + SUNXI_REG_MSEL_ADDR) &
+ SUNXI_MSEL_RESET_MODE)) {
+ netdev_err(dev, "setting controller into reset mode failed!\n");
+ return -ETIMEDOUT;
+ }
+
+ priv->can.state = CAN_STATE_STOPPED;
+
+ return 0;
+}
+
+static int sunxican_set_bittiming(struct net_device *dev)
+{
+ struct sunxican_priv *priv = netdev_priv(dev);
+ struct can_bittiming *bt = &priv->can.bittiming;
+ int err;
+ u32 cfg;
+
+ cfg = ((bt->brp - 1) & 0x3FF) |
+ (((bt->sjw - 1) & 0x3) << 14) |
+ (((bt->prop_seg + bt->phase_seg1 - 1) & 0xf) << 16) |
+ (((bt->phase_seg2 - 1) & 0x7) << 20);
+ if (priv->can.ctrlmode & CAN_CTRLMODE_3_SAMPLES)
+ cfg |= 0x800000;
+
+ netdev_info(dev, "setting BITTIMING=0x%08x\n", cfg);
+
+ /* CAN_BTIME_ADDR only writable in reset mode */
+ err = set_reset_mode(dev);
+ if (err)
+ return err;
+ writel(cfg, priv->base + SUNXI_REG_BTIME_ADDR);
+
+ return set_normal_mode(dev);
+}
+
+static int sunxican_get_berr_counter(const struct net_device *dev,
+ struct can_berr_counter *bec)
+{
+ struct sunxican_priv *priv = netdev_priv(dev);
+ u32 errors;
+ int err;
+
+ err = clk_prepare_enable(priv->clk);
+ if (err) {
+ netdev_err(dev, "could not enable clock\n");
+ return err;
+ }
+
+ errors = readl(priv->base + SUNXI_REG_ERRC_ADDR);
+
+ bec->txerr = errors & 0xFF;
+ bec->rxerr = (errors >> 16) & 0xFF;
+
+ clk_disable_unprepare(priv->clk);
+
+ return 0;
+}
+
+static int sunxi_can_start(struct net_device *dev)
+{
+ struct sunxican_priv *priv = netdev_priv(dev);
+ int err;
+ u32 temp_irqen;
+
+ /* turn on clocking for CAN peripheral block */
+ err = clk_prepare_enable(priv->clk);
+ if (err) {
+ netdev_err(dev, "could not enable clocking (apb1_can)\n");
+ goto exit;
+ }
+
+ /* leave reset mode */
+ if (priv->can.state != CAN_STATE_STOPPED)
+ set_reset_mode(dev);
+
+ /* set filters - we accept all */
+ writel(0x00000000, priv->base + SUNXI_REG_ACPC_ADDR);
+ writel(0xFFFFFFFF, priv->base + SUNXI_REG_ACPM_ADDR);
+
+ /* Clear error counters and error code capture */
+ writel(0x0, priv->base + SUNXI_REG_ERRC_ADDR);
+
+ /* enable CAN specific interrupts */
+ temp_irqen = SUNXI_INTEN_BERR | SUNXI_INTEN_ERR_PASSIVE |
+ SUNXI_INTEN_OR | SUNXI_INTEN_RX;
+ writel(readl(priv->base + SUNXI_REG_INTEN_ADDR) | temp_irqen,
+ priv->base + SUNXI_REG_INTEN_ADDR);
+
+ err = sunxican_set_bittiming(dev);
+
+ if (err)
+ return err;
+
+ return 0;
+
+exit:
+ return err;
+}
+
+static int sunxican_set_mode(struct net_device *dev, enum can_mode mode)
+{
+ int err;
+
+ switch (mode) {
+ case CAN_MODE_START:
+ err = sunxi_can_start(dev);
+ if (err) {
+ netdev_err(dev, "starting CAN controller failed!\n");
+ return err;
+ }
+ if (netif_queue_stopped(dev))
+ netif_wake_queue(dev);
+ break;
+
+ default:
+ return -EOPNOTSUPP;
+ }
+ return 0;
+}
+
+/* transmit a CAN message
+ * message layout in the sk_buff should be like this:
+ * xx xx xx xx ff ll 00 11 22 33 44 55 66 77
+ * [ can_id ] [flags] [len] [can data (up to 8 bytes]
+ */
+static int sunxican_start_xmit(struct sk_buff *skb, struct net_device *dev)
+{
+ struct sunxican_priv *priv = netdev_priv(dev);
+ struct can_frame *cf = (struct can_frame *)skb->data;
+ u8 dlc;
+ u32 dreg, msg_flag_n;
+ canid_t id;
+ int i;
+
+ if (can_dropped_invalid_skb(dev, skb))
+ return NETDEV_TX_OK;
+
+ netif_stop_queue(dev);
+
+ id = cf->can_id;
+ dlc = cf->can_dlc;
+ msg_flag_n = dlc;
+
+ if (id & CAN_RTR_FLAG)
+ msg_flag_n |= SUNXI_MSG_RTR_FLAG;
+
+ if (id & CAN_EFF_FLAG) {
+ msg_flag_n |= SUNXI_MSG_EFF_FLAG;
+ dreg = SUNXI_REG_BUF5_ADDR;
+ writel((id >> 21) & 0xFF, priv->base + SUNXI_REG_BUF1_ADDR);
+ writel((id >> 13) & 0xFF, priv->base + SUNXI_REG_BUF2_ADDR);
+ writel((id >> 5) & 0xFF, priv->base + SUNXI_REG_BUF3_ADDR);
+ writel((id << 3) & 0xF8, priv->base + SUNXI_REG_BUF4_ADDR);
+ } else {
+ dreg = SUNXI_REG_BUF3_ADDR;
+ writel((id >> 3) & 0xFF, priv->base + SUNXI_REG_BUF1_ADDR);
+ writel((id << 5) & 0xE0, priv->base + SUNXI_REG_BUF2_ADDR);
+ }
+
+ for (i = 0; i < dlc; i++)
+ writel(cf->data[i], priv->base + (dreg + i * 4));
+
+ writel(msg_flag_n, priv->base + SUNXI_REG_BUF0_ADDR);
+
+ can_put_echo_skb(skb, dev, 0);
+
+ if (priv->can.ctrlmode & CAN_CTRLMODE_LOOPBACK)
+ sunxi_can_write_cmdreg(priv, SUNXI_CMD_SELF_RCV_REQ);
+ else
+ sunxi_can_write_cmdreg(priv, SUNXI_CMD_TRANS_REQ);
+
+ return NETDEV_TX_OK;
+}
+
+static void sunxi_can_rx(struct net_device *dev)
+{
+ struct sunxican_priv *priv = netdev_priv(dev);
+ struct net_device_stats *stats = &dev->stats;
+ struct can_frame *cf;
+ struct sk_buff *skb;
+ u8 fi;
+ u32 dreg;
+ canid_t id;
+ int i;
+
+ /* create zero'ed CAN frame buffer */
+ skb = alloc_can_skb(dev, &cf);
+ if (!skb)
+ return;
+
+ fi = readl(priv->base + SUNXI_REG_BUF0_ADDR);
+ cf->can_dlc = get_can_dlc(fi & 0x0F);
+ if (fi & SUNXI_MSG_EFF_FLAG) {
+ dreg = SUNXI_REG_BUF5_ADDR;
+ id = (readl(priv->base + SUNXI_REG_BUF1_ADDR) << 21) |
+ (readl(priv->base + SUNXI_REG_BUF2_ADDR) << 13) |
+ (readl(priv->base + SUNXI_REG_BUF3_ADDR) << 5) |
+ ((readl(priv->base + SUNXI_REG_BUF4_ADDR) >> 3) & 0x1f);
+ id |= CAN_EFF_FLAG;
+ } else {
+ dreg = SUNXI_REG_BUF3_ADDR;
+ id = (readl(priv->base + SUNXI_REG_BUF1_ADDR) << 3) |
+ ((readl(priv->base + SUNXI_REG_BUF2_ADDR) >> 5) & 0x7);
+ }
+
+ /* remote frame ? */
+ if (fi & SUNXI_MSG_RTR_FLAG)
+ id |= CAN_RTR_FLAG;
+ else
+ for (i = 0; i < cf->can_dlc; i++)
+ cf->data[i] = readl(priv->base + dreg + i * 4);
+
+ cf->can_id = id;
+
+ sunxi_can_write_cmdreg(priv, SUNXI_CMD_RELEASE_RBUF);
+
+ stats->rx_packets++;
+ stats->rx_bytes += cf->can_dlc;
+ netif_rx(skb);
+
+ can_led_event(dev, CAN_LED_EVENT_RX);
+}
+
+static int sunxi_can_err(struct net_device *dev, u8 isrc, u8 status)
+{
+ struct sunxican_priv *priv = netdev_priv(dev);
+ struct net_device_stats *stats = &dev->stats;
+ struct can_frame *cf;
+ struct sk_buff *skb;
+ enum can_state state = priv->can.state;
+ enum can_state rx_state, tx_state;
+ unsigned int rxerr, txerr, errc;
+ u32 ecc, alc;
+
+ skb = alloc_can_err_skb(dev, &cf);
+ if (!skb)
+ return -ENOMEM;
+
+ errc = readl(priv->base + SUNXI_REG_ERRC_ADDR);
+ rxerr = (errc >> 16) & 0xFF;
+ txerr = errc & 0xFF;
+
+ cf->data[6] = txerr;
+ cf->data[7] = rxerr;
+
+ if (isrc & SUNXI_INT_DATA_OR) {
+ /* data overrun interrupt */
+ netdev_dbg(dev, "data overrun interrupt\n");
+ cf->can_id |= CAN_ERR_CRTL;
+ cf->data[1] = CAN_ERR_CRTL_RX_OVERFLOW;
+ stats->rx_over_errors++;
+ stats->rx_errors++;
+ /* clear bit */
+ sunxi_can_write_cmdreg(priv, SUNXI_CMD_CLEAR_OR_FLAG);
+ }
+ if (isrc & SUNXI_INT_ERR_WRN) {
+ /* error warning interrupt */
+ netdev_dbg(dev, "error warning interrupt\n");
+
+ if (status & SUNXI_STA_BUS_OFF)
+ state = CAN_STATE_BUS_OFF;
+ else if (status & SUNXI_STA_ERR_STA)
+ state = CAN_STATE_ERROR_WARNING;
+ else
+ state = CAN_STATE_ERROR_ACTIVE;
+ }
+ if (isrc & SUNXI_INT_BUS_ERR) {
+ /* bus error interrupt */
+ netdev_dbg(dev, "bus error interrupt\n");
+ priv->can.can_stats.bus_error++;
+ stats->rx_errors++;
+
+ ecc = readl(priv->base + SUNXI_REG_STA_ADDR);
+
+ cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
+
+ switch (ecc & SUNXI_STA_MASK_ERR) {
+ case SUNXI_STA_BIT_ERR:
+ cf->data[2] |= CAN_ERR_PROT_BIT;
+ break;
+ case SUNXI_STA_FORM_ERR:
+ cf->data[2] |= CAN_ERR_PROT_FORM;
+ break;
+ case SUNXI_STA_STUFF_ERR:
+ cf->data[2] |= CAN_ERR_PROT_STUFF;
+ break;
+ default:
+ cf->data[2] |= CAN_ERR_PROT_UNSPEC;
+ cf->data[3] = (ecc & SUNXI_STA_ERR_SEG_CODE) >> 16;
+ break;
+ }
+ /* error occurred during transmission? */
+ if ((ecc & SUNXI_STA_ERR_DIR) == 0)
+ cf->data[2] |= CAN_ERR_PROT_TX;
+ }
+ if (isrc & SUNXI_INT_ERR_PASSIVE) {
+ /* error passive interrupt */
+ netdev_dbg(dev, "error passive interrupt\n");
+ if (state == CAN_STATE_ERROR_PASSIVE)
+ state = CAN_STATE_ERROR_WARNING;
+ else
+ state = CAN_STATE_ERROR_PASSIVE;
+ }
+ if (isrc & SUNXI_INT_ARB_LOST) {
+ /* arbitration lost interrupt */
+ netdev_dbg(dev, "arbitration lost interrupt\n");
+ alc = readl(priv->base + SUNXI_REG_STA_ADDR);
+ priv->can.can_stats.arbitration_lost++;
+ stats->tx_errors++;
+ cf->can_id |= CAN_ERR_LOSTARB;
+ cf->data[0] = (alc & 0x1f) >> 8;
+ }
+
+ if (state != priv->can.state) {
+ tx_state = txerr >= rxerr ? state : 0;
+ rx_state = txerr <= rxerr ? state : 0;
+
+ can_change_state(dev, cf, tx_state, rx_state);
+ if (state == CAN_STATE_BUS_OFF)
+ can_bus_off(dev);
+ }
+
+ stats->rx_packets++;
+ stats->rx_bytes += cf->can_dlc;
+ netif_rx(skb);
+
+ return 0;
+}
+
+static irqreturn_t sunxi_can_interrupt(int irq, void *dev_id)
+{
+ struct net_device *dev = (struct net_device *)dev_id;
+ struct sunxican_priv *priv = netdev_priv(dev);
+ struct net_device_stats *stats = &dev->stats;
+ u8 isrc, status;
+ int n = 0;
+
+ while ((isrc = readl(priv->base + SUNXI_REG_INT_ADDR)) &&
+ (n < SUNXI_CAN_MAX_IRQ)) {
+ n++;
+ status = readl(priv->base + SUNXI_REG_STA_ADDR);
+
+ if (isrc & SUNXI_INT_WAKEUP)
+ netdev_warn(dev, "wakeup interrupt\n");
+
+ if (isrc & SUNXI_INT_TBUF_VLD) {
+ /* transmission complete interrupt */
+ stats->tx_bytes +=
+ readl(priv->base +
+ SUNXI_REG_RBUF_RBACK_START_ADDR) & 0xf;
+ stats->tx_packets++;
+ can_get_echo_skb(dev, 0);
+ netif_wake_queue(dev);
+ can_led_event(dev, CAN_LED_EVENT_TX);
+ }
+ if (isrc & SUNXI_INT_RBUF_VLD) {
+ /* receive interrupt */
+ while (status & SUNXI_STA_RBUF_RDY) {
+ /* RX buffer is not empty */
+ sunxi_can_rx(dev);
+ status = readl(priv->base + SUNXI_REG_STA_ADDR);
+ }
+ }
+ if (isrc &
+ (SUNXI_INT_DATA_OR | SUNXI_INT_ERR_WRN | SUNXI_INT_BUS_ERR |
+ SUNXI_INT_ERR_PASSIVE | SUNXI_INT_ARB_LOST)) {
+ /* error interrupt */
+ if (sunxi_can_err(dev, isrc, status))
+ break;
+ }
+ /* clear interrupts */
+ writel(isrc, priv->base + SUNXI_REG_INT_ADDR);
+ readl(priv->base + SUNXI_REG_INT_ADDR);
+ }
+ if (n >= SUNXI_CAN_MAX_IRQ)
+ netdev_dbg(dev, "%d messages handled in ISR", n);
+
+ return (n) ? IRQ_HANDLED : IRQ_NONE;
+}
+
+static int sunxican_open(struct net_device *dev)
+{
+ int err;
+
+ /* common open */
+ err = open_candev(dev);
+ if (err)
+ return err;
+
+ /* register interrupt handler */
+ err = request_irq(dev->irq, sunxi_can_interrupt, IRQF_SHARED,
+ dev->name, dev);
+ if (err) {
+ netdev_err(dev, "request_irq err: %d\n", err);
+ close_candev(dev);
+ return -EAGAIN;
+ }
+
+ sunxi_can_start(dev);
+
+ can_led_event(dev, CAN_LED_EVENT_OPEN);
+
+ netif_start_queue(dev);
+
+ return 0;
+}
+
+static int sunxican_close(struct net_device *dev)
+{
+ struct sunxican_priv *priv = netdev_priv(dev);
+
+ netif_stop_queue(dev);
+ set_reset_mode(dev);
+ clk_disable_unprepare(priv->clk);
+
+ free_irq(dev->irq, dev);
+ close_candev(dev);
+ can_led_event(dev, CAN_LED_EVENT_STOP);
+
+ return 0;
+}
+
+static const struct net_device_ops sunxican_netdev_ops = {
+ .ndo_open = sunxican_open,
+ .ndo_stop = sunxican_close,
+ .ndo_start_xmit = sunxican_start_xmit,
+};
+
+static const struct of_device_id sunxican_of_match[] = {
+ {.compatible = "allwinner,sunxican"},
+ {},
+};
+
+MODULE_DEVICE_TABLE(of, sunxican_of_match);
+
+static int sunxican_remove(struct platform_device *pdev)
+{
+ struct net_device *dev = platform_get_drvdata(pdev);
+
+ unregister_netdev(dev);
+ free_candev(dev);
+
+ return 0;
+}
+
+static int sunxican_probe(struct platform_device *pdev)
+{
+ struct resource *mem;
+ struct clk *clk;
+ void __iomem *addr;
+ int err, irq;
+ struct net_device *dev;
+ struct sunxican_priv *priv;
+
+ clk = devm_clk_get(&pdev->dev, "apb1_can");
+ if (IS_ERR(clk)) {
+ dev_err(&pdev->dev, "no clock defined\n");
+ err = -ENODEV;
+ goto exit;
+ }
+
+ irq = platform_get_irq(pdev, 0);
+ if (irq < 0) {
+ dev_err(&pdev->dev, "could not get a valid irq\n");
+ err = -ENODEV;
+ goto exit;
+ }
+
+ mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ addr = devm_ioremap_resource(&pdev->dev, mem);
+ if (IS_ERR(addr)) {
+ err = -EBUSY;
+ goto exit;
+ }
+
+ dev = alloc_candev(sizeof(struct sunxican_priv), 1);
+ if (!dev) {
+ dev_err(&pdev->dev,
+ "could not allocate memory for CAN device\n");
+ err = -ENOMEM;
+ goto exit;
+ }
+
+ dev->netdev_ops = &sunxican_netdev_ops;
+ dev->irq = irq;
+ dev->flags |= IFF_ECHO;
+
+ priv = netdev_priv(dev);
+ priv->can.clock.freq = clk_get_rate(clk);
+ priv->can.bittiming_const = &sunxican_bittiming_const;
+ priv->can.do_set_mode = sunxican_set_mode;
+ priv->can.do_get_berr_counter = sunxican_get_berr_counter;
+ priv->can.ctrlmode_supported = CAN_CTRLMODE_BERR_REPORTING |
+ CAN_CTRLMODE_LISTENONLY |
+ CAN_CTRLMODE_LOOPBACK |
+ CAN_CTRLMODE_3_SAMPLES;
+ priv->base = addr;
+ priv->clk = clk;
+ spin_lock_init(&priv->cmdreg_lock);
+
+ platform_set_drvdata(pdev, dev);
+ SET_NETDEV_DEV(dev, &pdev->dev);
+
+ err = register_candev(dev);
+ if (err) {
+ dev_err(&pdev->dev, "registering %s failed (err=%d)\n",
+ DRV_NAME, err);
+ goto exit_free;
+ }
+ devm_can_led_init(dev);
+
+ dev_info(&pdev->dev, "device registered (base=%p, irq=%d)\n",
+ priv->base, dev->irq);
+
+ return 0;
+
+exit_free:
+ free_candev(dev);
+exit:
+ return err;
+}
+
+static int __maybe_unused sunxi_can_suspend(struct device *device)
+{
+ struct net_device *dev = dev_get_drvdata(device);
+ struct sunxican_priv *priv = netdev_priv(dev);
+ u32 mode;
+ int err;
+
+ if (netif_running(dev)) {
+ netif_stop_queue(dev);
+ netif_device_detach(dev);
+ }
+
+ err = set_reset_mode(dev);
+ if (err)
+ return err;
+
+ mode = readl(priv->base + SUNXI_REG_MSEL_ADDR);
+ writel(mode | SUNXI_MSEL_SLEEP_MODE, priv->base + SUNXI_REG_MSEL_ADDR);
+
+ priv->can.state = CAN_STATE_SLEEPING;
+
+ clk_disable(priv->clk);
+ return 0;
+}
+
+static int __maybe_unused sunxi_can_resume(struct device *device)
+{
+ struct net_device *dev = dev_get_drvdata(device);
+ struct sunxican_priv *priv = netdev_priv(dev);
+ u32 mode;
+ int err;
+
+ err = clk_enable(priv->clk);
+ if (err) {
+ netdev_err(dev, "clk_enable() failed, error %d\n", err);
+ return err;
+ }
+
+ err = set_reset_mode(dev);
+ if (err)
+ return err;
+
+ mode = readl(priv->base + SUNXI_REG_MSEL_ADDR);
+ writel(mode & ~(SUNXI_MSEL_SLEEP_MODE),
+ priv->base + SUNXI_REG_MSEL_ADDR);
+ err = set_normal_mode(dev);
+ if (err)
+ return err;
+
+ priv->can.state = CAN_STATE_ERROR_ACTIVE;
+ if (netif_running(dev)) {
+ netif_device_attach(dev);
+ netif_start_queue(dev);
+ }
+ return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(sunxi_can_pm_ops, sunxi_can_suspend, sunxi_can_resume);
+
+static struct platform_driver sunxi_can_driver = {
+ .driver = {
+ .name = DRV_NAME,
+ .pm = &sunxi_can_pm_ops,
+ .of_match_table = sunxican_of_match,
+ },
+ .probe = sunxican_probe,
+ .remove = sunxican_remove,
+};
+
+module_platform_driver(sunxi_can_driver);
+
+MODULE_AUTHOR("Peter Chen <xingkongcp@gmail.com>");
+MODULE_AUTHOR("Gerhard Bertelsmann <info@gerhard-bertelsmann.de>");
+MODULE_LICENSE("Dual BSD/GPL");
+MODULE_DESCRIPTION(DRV_NAME "CAN driver for Allwinner SoCs (A10/A20)");
+MODULE_VERSION(DRV_MODULE_VERSION);
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v5 0/2] can: Allwinner A10/A20 CAN Controller support - summary
[not found] ` <1442144632-4541-1-git-send-email-info-La43T0Mi4bH5xCKuJOYmCvaTkwRoYoCU@public.gmane.org>
@ 2015-09-13 12:22 ` Maxime Ripard
2015-09-13 12:57 ` [linux-sunxi] " Marc Kleine-Budde
0 siblings, 1 reply; 15+ messages in thread
From: Maxime Ripard @ 2015-09-13 12:22 UTC (permalink / raw)
To: Gerhard Bertelsmann
Cc: linux-can-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, mkl-bIcnvbaLZ9MEGnE8C9+IrQ
[-- Attachment #1: Type: text/plain, Size: 2253 bytes --]
Hi Gerhard,
Thanks for working on this!
A few questions / remarks though:
On Sun, Sep 13, 2015 at 01:43:50PM +0200, Gerhard Bertelsmann wrote:
> Hi,
>
> please find attached the next try. Thanks to Marc. I've worked on
> all mentioned issues except one: I don't add clk_disable_unprepare
> in sunxi_can_start. Makes no sense to me (driver fails).
>
> For testers:
> the patch will only apply wihtout modification to kernels 3.19+.
Please always make sure to send a patch that have been tested and
applies at least on top of the latest rc. This would be 4.3-rc1 now.
> I've tested the driver (receipt, sending and basic errors). IMHO
> everything is fine. The performance seems to be good - high rates
> (>5000 packets/second) working fine.
>
> Please test and report bugs if exists.
>
>
> [PATCH v5 1/2] Device Tree Binding Documentation
> [PATCH v5 2/2] Kernel Module
>
> History:
> V5: fix license
> modify prefix to mode select defines
> enable and disable clock in sunxican_get_berr_counter
> delete set_normal_mode at the end of sunxi_can_start
> removed sunxican_id_table
> use devm_clk_get instead of clk_get
> use devm_ioremap_resource to simplify probe and remove
> make set-normal-mode and set-reset-mode more readable
>
> v4: defines prefixed with SUNXI_
> sunxi_can_write_cmdreg tweaked
> loops in set_xxx_mode reworked
> add return value to set_xxx_mode
> sunxican_start_xmit reworked
> struct platform_driver stripped
> moved set_bittiming into open
> moved clock start into open
> add clock stop to close
> suspend reworked
> resume reworked
> fixed double counting bug
>
> v3: changed error state change handling (thx Andri for the hint)
> use bittiming function correct (no need to call it)
> strip down priv (suggested by Marc)
> scripts/checkpatch.pl-> no matches anymore
> sparse -> no errors or warnings anymore
> v2: cleaning
> v1: initial
Where were those versions posted ?
Make sure to run get_maintainer before sending your patches to mail
them to the right recipients.
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v5 1/2] can: Allwinner A10/A20 CAN Controller support - Devicetree bindings
[not found] ` <1442144632-4541-2-git-send-email-info-La43T0Mi4bH5xCKuJOYmCvaTkwRoYoCU@public.gmane.org>
@ 2015-09-13 12:25 ` Maxime Ripard
0 siblings, 0 replies; 15+ messages in thread
From: Maxime Ripard @ 2015-09-13 12:25 UTC (permalink / raw)
To: Gerhard Bertelsmann
Cc: linux-can-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, mkl-bIcnvbaLZ9MEGnE8C9+IrQ
[-- Attachment #1: Type: text/plain, Size: 1465 bytes --]
Hi,
On Sun, Sep 13, 2015 at 01:43:51PM +0200, Gerhard Bertelsmann wrote:
> Signed-off-by: Gerhard Bertelsmann <info-La43T0Mi4bH5xCKuJOYmCvaTkwRoYoCU@public.gmane.org>
A commit log would be nice.
> ---
> .../devicetree/bindings/net/can/sunxi_can.txt | 40 +
> 1 files changed, 49 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/net/can/sunxi_can.txt b/Documentation/devicetree/bindings/net/can/sunxi_can.txt
> new file mode 100644
> index 0000000..5db9abc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/can/sunxi_can.txt
> @@ -0,0 +1,40 @@
> +Allwinner A10/A20 CAN controller Device Tree Bindings
> +-----------------------------------------------------
> +
> +Required properties:
> +- compatible: "allwinner,sunxican"
The compatible should be made using the earliest SoC using this IP. In
our case, that would be the A10.
The format we use is sun4i-a10-<ip>, which would make it sun4i-a10-can
in this case.
> +- reg: physical base address and size of the Allwinner A10/A20 CAN register map.
> +- interrupts: interrupt specifier for the sole interrupt.
> +- clocks: phandle and clock specifier
Which clock?
> +- pinctrl-0: pin control group to be used for this controller.
> +- pinctrl-names: must be "default".
Those are generic standalone properties. There's no really need to
mention them.
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v5 2/2] can: Allwinner A10/A20 CAN Controller support - controller code
[not found] ` <1442144632-4541-3-git-send-email-info-La43T0Mi4bH5xCKuJOYmCvaTkwRoYoCU@public.gmane.org>
@ 2015-09-13 12:45 ` Maxime Ripard
2015-09-13 12:50 ` [linux-sunxi] " Marc Kleine-Budde
2015-09-13 13:42 ` Gerhard Bertelsmann
0 siblings, 2 replies; 15+ messages in thread
From: Maxime Ripard @ 2015-09-13 12:45 UTC (permalink / raw)
To: Gerhard Bertelsmann
Cc: linux-can-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, mkl-bIcnvbaLZ9MEGnE8C9+IrQ
[-- Attachment #1: Type: text/plain, Size: 30695 bytes --]
Hi!
On Sun, Sep 13, 2015 at 01:43:52PM +0200, Gerhard Bertelsmann wrote:
> Signed-off-by: Gerhard Bertelsmann <info-La43T0Mi4bH5xCKuJOYmCvaTkwRoYoCU@public.gmane.org>
And one commit log here too please :)
> ---
> drivers/net/can/Kconfig | 10 +
> drivers/net/can/Makefile | 1 +
> drivers/net/can/sunxi_can.c | 879 +++++++++++++++++++++
> 3 files changed, 890 insertions(+)
>
> diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
> index e8c96b8..439f67c 100644
> --- a/drivers/net/can/Kconfig
> +++ b/drivers/net/can/Kconfig
> @@ -129,6 +129,16 @@ config CAN_RCAR
> To compile this driver as a module, choose M here: the module will
> be called rcar_can.
>
> +config CAN_SUNXI
> + tristate "Allwinner SUNXI CAN controller"
It would be better if we could mention that it's a driver for the
sun4i family and derived.
We have no guarantee that there won't be any IP on other sunxi SoCs
incompatible with this one that would need a new driver.
Something like CAN_SUN4I (and the driver sun4i_can.c) would be great.
(This also applies to all the symbols/defines/etc inside the driver,
for consistency).
> + depends on MACH_SUN4I || MACH_SUN7I || COMPILE_TEST
> + ---help---
> + Say Y here if you want to use CAN controller found on Allwinner
> + A10/A20 SoCs.
> +
> + To compile this driver as a module, choose M here: the module will
> + be called rcar_can.
You're sure about the name of the compiled module?
> +
> config CAN_XILINXCAN
> tristate "Xilinx CAN"
> depends on ARCH_ZYNQ || ARM64 || MICROBLAZE || COMPILE_TEST
> diff --git a/drivers/net/can/Makefile b/drivers/net/can/Makefile
> index c533c62..b3e825c 100644
> --- a/drivers/net/can/Makefile
> +++ b/drivers/net/can/Makefile
> @@ -27,6 +27,7 @@ obj-$(CONFIG_CAN_FLEXCAN) += flexcan.o
> obj-$(CONFIG_PCH_CAN) += pch_can.o
> obj-$(CONFIG_CAN_GRCAN) += grcan.o
> obj-$(CONFIG_CAN_RCAR) += rcar_can.o
> +obj-$(CONFIG_CAN_SUNXI) += sunxi_can.o
> obj-$(CONFIG_CAN_XILINXCAN) += xilinx_can.o
>
> subdir-ccflags-y += -D__CHECK_ENDIAN__
> diff --git a/drivers/net/can/sunxi_can.c b/drivers/net/can/sunxi_can.c
> new file mode 100644
> index 0000000..569e6cd
> --- /dev/null
> +++ b/drivers/net/can/sunxi_can.c
> @@ -0,0 +1,879 @@
> +/*
> + * sunxi_can.c - CAN bus controller driver for Allwinner SUN4I&SUN7I based SoCs
> + *
> + * Copyright (C) 2013 Peter Chen
> + * Copyright (C) 2015 Gerhard Bertelsmann
> + * All rights reserved.
> + *
> + * Parts of this software are based on (derived from) the SJA1000 code by:
> + * Copyright (C) 2014 Oliver Hartkopp <oliver.hartkopp-l29pVbxQd1IUtdQbppsyvg@public.gmane.org>
> + * Copyright (C) 2007 Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
> + * Copyright (C) 2002-2007 Volkswagen Group Electronic Research
> + * Copyright (C) 2003 Matthias Brukner, Trajet Gmbh, Rebenring 33,
> + * 38106 Braunschweig, GERMANY
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + * 1. Redistributions of source code must retain the above copyright
> + * notice, this list of conditions and the following disclaimer.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + * notice, this list of conditions and the following disclaimer in the
> + * documentation and/or other materials provided with the distribution.
> + * 3. Neither the name of Volkswagen nor the names of its contributors
> + * may be used to endorse or promote products derived from this software
> + * without specific prior written permission.
> + *
> + * Alternatively, provided that this notice is retained in full, this
> + * software may be distributed under the terms of the GNU General
> + * Public License ("GPL") version 2, in which case the provisions of the
> + * GPL apply INSTEAD OF those given above.
> + *
> + * The provided data structures and external interfaces from this code
> + * are not restricted to be used by modules with a GPL compatible license.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
> + * DAMAGE.
> + *
> + */
> +
> +#include <linux/netdevice.h>
> +#include <linux/can.h>
> +#include <linux/can/dev.h>
> +#include <linux/can/error.h>
> +#include <linux/can/led.h>
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/interrupt.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +
> +#define DRV_NAME "sunxi_can"
> +#define DRV_MODULE_VERSION "0.99"
> +
> +/* Registers address (physical base address 0x01C2BC00) */
> +#define SUNXI_REG_MSEL_ADDR 0x0000 /* CAN Mode Select */
> +#define SUNXI_REG_CMD_ADDR 0x0004 /* CAN Command */
> +#define SUNXI_REG_STA_ADDR 0x0008 /* CAN Status */
> +#define SUNXI_REG_INT_ADDR 0x000c /* CAN Interrupt Flag */
> +#define SUNXI_REG_INTEN_ADDR 0x0010 /* CAN Interrupt Enable */
> +#define SUNXI_REG_BTIME_ADDR 0x0014 /* CAN Bus Timing 0 */
> +#define SUNXI_REG_TEWL_ADDR 0x0018 /* CAN Tx Error Warning Limit */
> +#define SUNXI_REG_ERRC_ADDR 0x001c /* CAN Error Counter */
> +#define SUNXI_REG_RMCNT_ADDR 0x0020 /* CAN Receive Message Counter */
> +#define SUNXI_REG_RBUFSA_ADDR 0x0024 /* CAN Receive Buffer Start Address */
> +#define SUNXI_REG_BUF0_ADDR 0x0040 /* CAN Tx/Rx Buffer 0 */
> +#define SUNXI_REG_BUF1_ADDR 0x0044 /* CAN Tx/Rx Buffer 1 */
> +#define SUNXI_REG_BUF2_ADDR 0x0048 /* CAN Tx/Rx Buffer 2 */
> +#define SUNXI_REG_BUF3_ADDR 0x004c /* CAN Tx/Rx Buffer 3 */
> +#define SUNXI_REG_BUF4_ADDR 0x0050 /* CAN Tx/Rx Buffer 4 */
> +#define SUNXI_REG_BUF5_ADDR 0x0054 /* CAN Tx/Rx Buffer 5 */
> +#define SUNXI_REG_BUF6_ADDR 0x0058 /* CAN Tx/Rx Buffer 6 */
> +#define SUNXI_REG_BUF7_ADDR 0x005c /* CAN Tx/Rx Buffer 7 */
> +#define SUNXI_REG_BUF8_ADDR 0x0060 /* CAN Tx/Rx Buffer 8 */
> +#define SUNXI_REG_BUF9_ADDR 0x0064 /* CAN Tx/Rx Buffer 9 */
> +#define SUNXI_REG_BUF10_ADDR 0x0068 /* CAN Tx/Rx Buffer 10 */
> +#define SUNXI_REG_BUF11_ADDR 0x006c /* CAN Tx/Rx Buffer 11 */
> +#define SUNXI_REG_BUF12_ADDR 0x0070 /* CAN Tx/Rx Buffer 12 */
> +#define SUNXI_REG_ACPC_ADDR 0x0040 /* CAN Acceptance Code 0 */
> +#define SUNXI_REG_ACPM_ADDR 0x0044 /* CAN Acceptance Mask 0 */
> +#define SUNXI_REG_RBUF_RBACK_START_ADDR 0x0180 /* CAN transmit buffer start */
> +#define SUNXI_REG_RBUF_RBACK_END_ADDR 0x01b0 /* CAN transmit buffer end */
> +
> +/* Controller Register Description */
> +
> +/* mode select register (r/w)
> + * offset:0x0000 default:0x0000_0001
> + */
> +#define SUNXI_MSEL_SLEEP_MODE (0x01 << 4) /* write in reset mode */
> +#define SUNXI_MSEL_WAKE_UP (0x00 << 4)
> +#define SUNXI_MSEL_SINGLE_FILTER (0x01 << 3) /* write in reset mode */
> +#define SUNXI_MSEL_DUAL_FILTERS (0x00 << 3)
> +#define SUNXI_MSEL_LOOPBACK_MODE BIT(2)
> +#define SUNXI_MSEL_LISTEN_ONLY_MODE BIT(1)
> +#define SUNXI_MSEL_RESET_MODE BIT(0)
> +
> +/* command register (w)
> + * offset:0x0004 default:0x0000_0000
> + */
> +#define SUNXI_CMD_BUS_OFF_REQ BIT(5)
> +#define SUNXI_CMD_SELF_RCV_REQ BIT(4)
> +#define SUNXI_CMD_CLEAR_OR_FLAG BIT(3)
> +#define SUNXI_CMD_RELEASE_RBUF BIT(2)
> +#define SUNXI_CMD_ABORT_REQ BIT(1)
> +#define SUNXI_CMD_TRANS_REQ BIT(0)
> +
> +/* status register (r)
> + * offset:0x0008 default:0x0000_003c
> + */
> +#define SUNXI_STA_BIT_ERR (0x00 << 22)
> +#define SUNXI_STA_FORM_ERR (0x01 << 22)
> +#define SUNXI_STA_STUFF_ERR (0x02 << 22)
> +#define SUNXI_STA_OTHER_ERR (0x03 << 22)
> +#define SUNXI_STA_MASK_ERR (0x03 << 22)
> +#define SUNXI_STA_ERR_DIR BIT(21)
> +#define SUNXI_STA_ERR_SEG_CODE (0x1f << 16)
> +#define SUNXI_STA_START (0x03 << 16)
> +#define SUNXI_STA_ID28_21 (0x02 << 16)
> +#define SUNXI_STA_ID20_18 (0x06 << 16)
> +#define SUNXI_STA_SRTR (0x04 << 16)
> +#define SUNXI_STA_IDE (0x05 << 16)
> +#define SUNXI_STA_ID17_13 (0x07 << 16)
> +#define SUNXI_STA_ID12_5 (0x0f << 16)
> +#define SUNXI_STA_ID4_0 (0x0e << 16)
> +#define SUNXI_STA_RTR (0x0c << 16)
> +#define SUNXI_STA_RB1 (0x0d << 16)
> +#define SUNXI_STA_RB0 (0x09 << 16)
> +#define SUNXI_STA_DLEN (0x0b << 16)
> +#define SUNXI_STA_DATA_FIELD (0x0a << 16)
> +#define SUNXI_STA_CRC_SEQUENCE (0x08 << 16)
> +#define SUNXI_STA_CRC_DELIMITER (0x18 << 16)
> +#define SUNXI_STA_ACK (0x19 << 16)
> +#define SUNXI_STA_ACK_DELIMITER (0x1b << 16)
> +#define SUNXI_STA_END (0x1a << 16)
> +#define SUNXI_STA_INTERMISSION (0x12 << 16)
> +#define SUNXI_STA_ACTIVE_ERROR (0x11 << 16)
> +#define SUNXI_STA_PASSIVE_ERROR (0x16 << 16)
> +#define SUNXI_STA_TOLERATE_DOMINANT_BITS (0x13 << 16)
> +#define SUNXI_STA_ERROR_DELIMITER (0x17 << 16)
> +#define SUNXI_STA_OVERLOAD (0x1c << 16)
> +#define SUNXI_STA_BUS_OFF BIT(7)
> +#define SUNXI_STA_ERR_STA BIT(6)
> +#define SUNXI_STA_TRANS_BUSY BIT(5)
> +#define SUNXI_STA_RCV_BUSY BIT(4)
> +#define SUNXI_STA_TRANS_OVER BIT(3)
> +#define SUNXI_STA_TBUF_RDY BIT(2)
> +#define SUNXI_STA_DATA_ORUN BIT(1)
> +#define SUNXI_STA_RBUF_RDY BIT(0)
> +
> +/* interrupt register (r)
> + * offset:0x000c default:0x0000_0000
> + */
> +#define SUNXI_INT_BUS_ERR BIT(7)
> +#define SUNXI_INT_ARB_LOST BIT(6)
> +#define SUNXI_INT_ERR_PASSIVE BIT(5)
> +#define SUNXI_INT_WAKEUP BIT(4)
> +#define SUNXI_INT_DATA_OR BIT(3)
> +#define SUNXI_INT_ERR_WRN BIT(2)
> +#define SUNXI_INT_TBUF_VLD BIT(1)
> +#define SUNXI_INT_RBUF_VLD BIT(0)
> +
> +/* interrupt enable register (r/w)
> + * offset:0x0010 default:0x0000_0000
> + */
> +#define SUNXI_INTEN_BERR BIT(7)
> +#define SUNXI_INTEN_ARB_LOST BIT(6)
> +#define SUNXI_INTEN_ERR_PASSIVE BIT(5)
> +#define SUNXI_INTEN_WAKEUP BIT(4)
> +#define SUNXI_INTEN_OR BIT(3)
> +#define SUNXI_INTEN_ERR_WRN BIT(2)
> +#define SUNXI_INTEN_TX BIT(1)
> +#define SUNXI_INTEN_RX BIT(0)
> +
> +/* error code */
> +#define SUNXI_ERR_INRCV (0x1 << 5)
> +#define SUNXI_ERR_INTRANS (0x0 << 5)
> +
> +/* filter mode */
> +#define SINXI_FILTER_CLOSE 0
> +#define SUNXI_SINGLE_FLTER_MODE 1
> +#define SUNXI_DUAL_FILTER_MODE 2
> +
> +/* message buffer flags */
> +#define SUNXI_MSG_EFF_FLAG BIT(7)
> +#define SUNXI_MSG_RTR_FLAG BIT(6)
> +
> +/* max. number of interrupts handled in ISR */
> +#define SUNXI_CAN_MAX_IRQ 20
> +#define SUNXI_MODE_MAX_RETRIES 100
> +
> +struct sunxican_priv {
> + struct can_priv can;
> + void __iomem *base;
> + struct clk *clk;
> + spinlock_t cmdreg_lock; /* lock for concurrent cmd register writes */
> +};
> +
> +static const struct can_bittiming_const sunxican_bittiming_const = {
> + .name = DRV_NAME,
> + .tseg1_min = 1,
> + .tseg1_max = 16,
> + .tseg2_min = 1,
> + .tseg2_max = 8,
> + .sjw_max = 4,
> + .brp_min = 1,
> + .brp_max = 64,
> + .brp_inc = 1,
> +};
> +
> +static void sunxi_can_write_cmdreg(struct sunxican_priv *priv, u8 val)
> +{
> + unsigned long flags;
> +
> + spin_lock_irqsave(&priv->cmdreg_lock, flags);
> + writel(val, priv->base + SUNXI_REG_CMD_ADDR);
> + spin_unlock_irqrestore(&priv->cmdreg_lock, flags);
> +}
> +
> +static int set_normal_mode(struct net_device *dev)
> +{
> + struct sunxican_priv *priv = netdev_priv(dev);
> + int retry = SUNXI_MODE_MAX_RETRIES;
> + u32 mod_reg_val = 0;
> +
> + do {
> + mod_reg_val = readl(priv->base + SUNXI_REG_MSEL_ADDR);
> + mod_reg_val &= ~SUNXI_MSEL_RESET_MODE;
> + writel(mod_reg_val, priv->base + SUNXI_REG_MSEL_ADDR);
> + } while (retry-- && (mod_reg_val & SUNXI_MSEL_RESET_MODE));
> +
> + if (readl(priv->base + SUNXI_REG_MSEL_ADDR) & SUNXI_MSEL_RESET_MODE) {
> + netdev_err(dev,
> + "setting controller into normal mode failed!\n");
> + return -ETIMEDOUT;
> + }
> +
> + priv->can.state = CAN_STATE_ERROR_ACTIVE;
> + /* enable interrupts */
> + if (priv->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING)
> + writel(0xFFFF, priv->base + SUNXI_REG_INTEN_ADDR);
> + else
> + writel(0xFFFF & ~SUNXI_INTEN_BERR,
> + priv->base + SUNXI_REG_INTEN_ADDR);
> +
> + mod_reg_val = readl(priv->base + SUNXI_REG_MSEL_ADDR);
> +
> + if (priv->can.ctrlmode & CAN_CTRLMODE_PRESUME_ACK)
> + mod_reg_val |= SUNXI_MSEL_LOOPBACK_MODE;
> + else if (priv->can.ctrlmode & CAN_CTRLMODE_LISTENONLY)
> + mod_reg_val |= SUNXI_MSEL_LISTEN_ONLY_MODE;
> +
> + writel(mod_reg_val, priv->base + SUNXI_REG_MSEL_ADDR);
> + return 0;
> +}
> +
> +static int set_reset_mode(struct net_device *dev)
> +{
> + struct sunxican_priv *priv = netdev_priv(dev);
> + int retry = SUNXI_MODE_MAX_RETRIES;
> + u32 mod_reg_val = 0;
> +
> + do {
> + mod_reg_val = readl(priv->base + SUNXI_REG_MSEL_ADDR);
> + mod_reg_val |= SUNXI_MSEL_RESET_MODE;
> + writel(mod_reg_val, priv->base + SUNXI_REG_MSEL_ADDR);
> + } while (retry-- && !(mod_reg_val & SUNXI_MSEL_RESET_MODE));
> +
> + if (!(readl(priv->base + SUNXI_REG_MSEL_ADDR) &
> + SUNXI_MSEL_RESET_MODE)) {
> + netdev_err(dev, "setting controller into reset mode failed!\n");
> + return -ETIMEDOUT;
> + }
> +
> + priv->can.state = CAN_STATE_STOPPED;
> +
> + return 0;
> +}
> +
> +static int sunxican_set_bittiming(struct net_device *dev)
> +{
> + struct sunxican_priv *priv = netdev_priv(dev);
> + struct can_bittiming *bt = &priv->can.bittiming;
> + int err;
> + u32 cfg;
> +
> + cfg = ((bt->brp - 1) & 0x3FF) |
> + (((bt->sjw - 1) & 0x3) << 14) |
> + (((bt->prop_seg + bt->phase_seg1 - 1) & 0xf) << 16) |
> + (((bt->phase_seg2 - 1) & 0x7) << 20);
> + if (priv->can.ctrlmode & CAN_CTRLMODE_3_SAMPLES)
> + cfg |= 0x800000;
> +
> + netdev_info(dev, "setting BITTIMING=0x%08x\n", cfg);
> +
> + /* CAN_BTIME_ADDR only writable in reset mode */
> + err = set_reset_mode(dev);
> + if (err)
> + return err;
> + writel(cfg, priv->base + SUNXI_REG_BTIME_ADDR);
> +
> + return set_normal_mode(dev);
> +}
> +
> +static int sunxican_get_berr_counter(const struct net_device *dev,
> + struct can_berr_counter *bec)
> +{
> + struct sunxican_priv *priv = netdev_priv(dev);
> + u32 errors;
> + int err;
> +
> + err = clk_prepare_enable(priv->clk);
> + if (err) {
> + netdev_err(dev, "could not enable clock\n");
> + return err;
> + }
> +
> + errors = readl(priv->base + SUNXI_REG_ERRC_ADDR);
> +
> + bec->txerr = errors & 0xFF;
> + bec->rxerr = (errors >> 16) & 0xFF;
> +
> + clk_disable_unprepare(priv->clk);
> +
> + return 0;
> +}
> +
> +static int sunxi_can_start(struct net_device *dev)
> +{
> + struct sunxican_priv *priv = netdev_priv(dev);
> + int err;
> + u32 temp_irqen;
> +
> + /* turn on clocking for CAN peripheral block */
> + err = clk_prepare_enable(priv->clk);
> + if (err) {
> + netdev_err(dev, "could not enable clocking (apb1_can)\n");
> + goto exit;
> + }
> +
> + /* leave reset mode */
> + if (priv->can.state != CAN_STATE_STOPPED)
> + set_reset_mode(dev);
> +
> + /* set filters - we accept all */
> + writel(0x00000000, priv->base + SUNXI_REG_ACPC_ADDR);
> + writel(0xFFFFFFFF, priv->base + SUNXI_REG_ACPM_ADDR);
> +
> + /* Clear error counters and error code capture */
> + writel(0x0, priv->base + SUNXI_REG_ERRC_ADDR);
> +
> + /* enable CAN specific interrupts */
> + temp_irqen = SUNXI_INTEN_BERR | SUNXI_INTEN_ERR_PASSIVE |
> + SUNXI_INTEN_OR | SUNXI_INTEN_RX;
> + writel(readl(priv->base + SUNXI_REG_INTEN_ADDR) | temp_irqen,
> + priv->base + SUNXI_REG_INTEN_ADDR);
> +
> + err = sunxican_set_bittiming(dev);
> +
> + if (err)
> + return err;
You're leaving the clock enabled if you have an error.
> +
> + return 0;
> +
> +exit:
> + return err;
> +}
> +
> +static int sunxican_set_mode(struct net_device *dev, enum can_mode mode)
> +{
> + int err;
> +
> + switch (mode) {
> + case CAN_MODE_START:
> + err = sunxi_can_start(dev);
> + if (err) {
> + netdev_err(dev, "starting CAN controller failed!\n");
> + return err;
> + }
> + if (netif_queue_stopped(dev))
> + netif_wake_queue(dev);
> + break;
> +
> + default:
> + return -EOPNOTSUPP;
> + }
> + return 0;
> +}
> +
> +/* transmit a CAN message
> + * message layout in the sk_buff should be like this:
> + * xx xx xx xx ff ll 00 11 22 33 44 55 66 77
> + * [ can_id ] [flags] [len] [can data (up to 8 bytes]
> + */
> +static int sunxican_start_xmit(struct sk_buff *skb, struct net_device *dev)
> +{
> + struct sunxican_priv *priv = netdev_priv(dev);
> + struct can_frame *cf = (struct can_frame *)skb->data;
> + u8 dlc;
> + u32 dreg, msg_flag_n;
> + canid_t id;
> + int i;
> +
> + if (can_dropped_invalid_skb(dev, skb))
> + return NETDEV_TX_OK;
> +
> + netif_stop_queue(dev);
> +
> + id = cf->can_id;
> + dlc = cf->can_dlc;
> + msg_flag_n = dlc;
> +
> + if (id & CAN_RTR_FLAG)
> + msg_flag_n |= SUNXI_MSG_RTR_FLAG;
> +
> + if (id & CAN_EFF_FLAG) {
> + msg_flag_n |= SUNXI_MSG_EFF_FLAG;
> + dreg = SUNXI_REG_BUF5_ADDR;
> + writel((id >> 21) & 0xFF, priv->base + SUNXI_REG_BUF1_ADDR);
> + writel((id >> 13) & 0xFF, priv->base + SUNXI_REG_BUF2_ADDR);
> + writel((id >> 5) & 0xFF, priv->base + SUNXI_REG_BUF3_ADDR);
> + writel((id << 3) & 0xF8, priv->base + SUNXI_REG_BUF4_ADDR);
> + } else {
> + dreg = SUNXI_REG_BUF3_ADDR;
> + writel((id >> 3) & 0xFF, priv->base + SUNXI_REG_BUF1_ADDR);
> + writel((id << 5) & 0xE0, priv->base + SUNXI_REG_BUF2_ADDR);
> + }
> +
> + for (i = 0; i < dlc; i++)
> + writel(cf->data[i], priv->base + (dreg + i * 4));
> +
> + writel(msg_flag_n, priv->base + SUNXI_REG_BUF0_ADDR);
> +
> + can_put_echo_skb(skb, dev, 0);
> +
> + if (priv->can.ctrlmode & CAN_CTRLMODE_LOOPBACK)
> + sunxi_can_write_cmdreg(priv, SUNXI_CMD_SELF_RCV_REQ);
> + else
> + sunxi_can_write_cmdreg(priv, SUNXI_CMD_TRANS_REQ);
> +
> + return NETDEV_TX_OK;
> +}
> +
> +static void sunxi_can_rx(struct net_device *dev)
> +{
> + struct sunxican_priv *priv = netdev_priv(dev);
> + struct net_device_stats *stats = &dev->stats;
> + struct can_frame *cf;
> + struct sk_buff *skb;
> + u8 fi;
> + u32 dreg;
> + canid_t id;
> + int i;
> +
> + /* create zero'ed CAN frame buffer */
> + skb = alloc_can_skb(dev, &cf);
> + if (!skb)
> + return;
> +
> + fi = readl(priv->base + SUNXI_REG_BUF0_ADDR);
> + cf->can_dlc = get_can_dlc(fi & 0x0F);
> + if (fi & SUNXI_MSG_EFF_FLAG) {
> + dreg = SUNXI_REG_BUF5_ADDR;
> + id = (readl(priv->base + SUNXI_REG_BUF1_ADDR) << 21) |
> + (readl(priv->base + SUNXI_REG_BUF2_ADDR) << 13) |
> + (readl(priv->base + SUNXI_REG_BUF3_ADDR) << 5) |
> + ((readl(priv->base + SUNXI_REG_BUF4_ADDR) >> 3) & 0x1f);
> + id |= CAN_EFF_FLAG;
> + } else {
> + dreg = SUNXI_REG_BUF3_ADDR;
> + id = (readl(priv->base + SUNXI_REG_BUF1_ADDR) << 3) |
> + ((readl(priv->base + SUNXI_REG_BUF2_ADDR) >> 5) & 0x7);
> + }
> +
> + /* remote frame ? */
> + if (fi & SUNXI_MSG_RTR_FLAG)
> + id |= CAN_RTR_FLAG;
> + else
> + for (i = 0; i < cf->can_dlc; i++)
> + cf->data[i] = readl(priv->base + dreg + i * 4);
> +
> + cf->can_id = id;
> +
> + sunxi_can_write_cmdreg(priv, SUNXI_CMD_RELEASE_RBUF);
> +
> + stats->rx_packets++;
> + stats->rx_bytes += cf->can_dlc;
> + netif_rx(skb);
> +
> + can_led_event(dev, CAN_LED_EVENT_RX);
> +}
> +
> +static int sunxi_can_err(struct net_device *dev, u8 isrc, u8 status)
> +{
> + struct sunxican_priv *priv = netdev_priv(dev);
> + struct net_device_stats *stats = &dev->stats;
> + struct can_frame *cf;
> + struct sk_buff *skb;
> + enum can_state state = priv->can.state;
> + enum can_state rx_state, tx_state;
> + unsigned int rxerr, txerr, errc;
> + u32 ecc, alc;
> +
> + skb = alloc_can_err_skb(dev, &cf);
> + if (!skb)
> + return -ENOMEM;
> +
> + errc = readl(priv->base + SUNXI_REG_ERRC_ADDR);
> + rxerr = (errc >> 16) & 0xFF;
> + txerr = errc & 0xFF;
> +
> + cf->data[6] = txerr;
> + cf->data[7] = rxerr;
> +
> + if (isrc & SUNXI_INT_DATA_OR) {
> + /* data overrun interrupt */
> + netdev_dbg(dev, "data overrun interrupt\n");
> + cf->can_id |= CAN_ERR_CRTL;
> + cf->data[1] = CAN_ERR_CRTL_RX_OVERFLOW;
> + stats->rx_over_errors++;
> + stats->rx_errors++;
> + /* clear bit */
> + sunxi_can_write_cmdreg(priv, SUNXI_CMD_CLEAR_OR_FLAG);
> + }
> + if (isrc & SUNXI_INT_ERR_WRN) {
> + /* error warning interrupt */
> + netdev_dbg(dev, "error warning interrupt\n");
> +
> + if (status & SUNXI_STA_BUS_OFF)
> + state = CAN_STATE_BUS_OFF;
> + else if (status & SUNXI_STA_ERR_STA)
> + state = CAN_STATE_ERROR_WARNING;
> + else
> + state = CAN_STATE_ERROR_ACTIVE;
> + }
> + if (isrc & SUNXI_INT_BUS_ERR) {
> + /* bus error interrupt */
> + netdev_dbg(dev, "bus error interrupt\n");
> + priv->can.can_stats.bus_error++;
> + stats->rx_errors++;
> +
> + ecc = readl(priv->base + SUNXI_REG_STA_ADDR);
> +
> + cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
> +
> + switch (ecc & SUNXI_STA_MASK_ERR) {
> + case SUNXI_STA_BIT_ERR:
> + cf->data[2] |= CAN_ERR_PROT_BIT;
> + break;
> + case SUNXI_STA_FORM_ERR:
> + cf->data[2] |= CAN_ERR_PROT_FORM;
> + break;
> + case SUNXI_STA_STUFF_ERR:
> + cf->data[2] |= CAN_ERR_PROT_STUFF;
> + break;
> + default:
> + cf->data[2] |= CAN_ERR_PROT_UNSPEC;
> + cf->data[3] = (ecc & SUNXI_STA_ERR_SEG_CODE) >> 16;
> + break;
> + }
> + /* error occurred during transmission? */
> + if ((ecc & SUNXI_STA_ERR_DIR) == 0)
> + cf->data[2] |= CAN_ERR_PROT_TX;
> + }
> + if (isrc & SUNXI_INT_ERR_PASSIVE) {
> + /* error passive interrupt */
> + netdev_dbg(dev, "error passive interrupt\n");
> + if (state == CAN_STATE_ERROR_PASSIVE)
> + state = CAN_STATE_ERROR_WARNING;
> + else
> + state = CAN_STATE_ERROR_PASSIVE;
> + }
> + if (isrc & SUNXI_INT_ARB_LOST) {
> + /* arbitration lost interrupt */
> + netdev_dbg(dev, "arbitration lost interrupt\n");
> + alc = readl(priv->base + SUNXI_REG_STA_ADDR);
> + priv->can.can_stats.arbitration_lost++;
> + stats->tx_errors++;
> + cf->can_id |= CAN_ERR_LOSTARB;
> + cf->data[0] = (alc & 0x1f) >> 8;
> + }
> +
> + if (state != priv->can.state) {
> + tx_state = txerr >= rxerr ? state : 0;
> + rx_state = txerr <= rxerr ? state : 0;
> +
> + can_change_state(dev, cf, tx_state, rx_state);
> + if (state == CAN_STATE_BUS_OFF)
> + can_bus_off(dev);
> + }
> +
> + stats->rx_packets++;
> + stats->rx_bytes += cf->can_dlc;
> + netif_rx(skb);
> +
> + return 0;
> +}
> +
> +static irqreturn_t sunxi_can_interrupt(int irq, void *dev_id)
> +{
> + struct net_device *dev = (struct net_device *)dev_id;
> + struct sunxican_priv *priv = netdev_priv(dev);
> + struct net_device_stats *stats = &dev->stats;
> + u8 isrc, status;
> + int n = 0;
> +
> + while ((isrc = readl(priv->base + SUNXI_REG_INT_ADDR)) &&
> + (n < SUNXI_CAN_MAX_IRQ)) {
> + n++;
> + status = readl(priv->base + SUNXI_REG_STA_ADDR);
> +
> + if (isrc & SUNXI_INT_WAKEUP)
> + netdev_warn(dev, "wakeup interrupt\n");
> +
> + if (isrc & SUNXI_INT_TBUF_VLD) {
> + /* transmission complete interrupt */
> + stats->tx_bytes +=
> + readl(priv->base +
> + SUNXI_REG_RBUF_RBACK_START_ADDR) & 0xf;
> + stats->tx_packets++;
> + can_get_echo_skb(dev, 0);
> + netif_wake_queue(dev);
> + can_led_event(dev, CAN_LED_EVENT_TX);
> + }
> + if (isrc & SUNXI_INT_RBUF_VLD) {
> + /* receive interrupt */
> + while (status & SUNXI_STA_RBUF_RDY) {
> + /* RX buffer is not empty */
> + sunxi_can_rx(dev);
> + status = readl(priv->base + SUNXI_REG_STA_ADDR);
> + }
> + }
> + if (isrc &
> + (SUNXI_INT_DATA_OR | SUNXI_INT_ERR_WRN | SUNXI_INT_BUS_ERR |
> + SUNXI_INT_ERR_PASSIVE | SUNXI_INT_ARB_LOST)) {
> + /* error interrupt */
> + if (sunxi_can_err(dev, isrc, status))
> + break;
> + }
> + /* clear interrupts */
> + writel(isrc, priv->base + SUNXI_REG_INT_ADDR);
> + readl(priv->base + SUNXI_REG_INT_ADDR);
> + }
Why not simply handle whatever you have in the status register, and
let the interrupt retrigger if there's something new?
> + if (n >= SUNXI_CAN_MAX_IRQ)
> + netdev_dbg(dev, "%d messages handled in ISR", n);
> +
> + return (n) ? IRQ_HANDLED : IRQ_NONE;
> +}
> +
> +static int sunxican_open(struct net_device *dev)
> +{
> + int err;
> +
> + /* common open */
> + err = open_candev(dev);
> + if (err)
> + return err;
> +
> + /* register interrupt handler */
> + err = request_irq(dev->irq, sunxi_can_interrupt, IRQF_SHARED,
> + dev->name, dev);
> + if (err) {
> + netdev_err(dev, "request_irq err: %d\n", err);
> + close_candev(dev);
> + return -EAGAIN;
> + }
> +
> + sunxi_can_start(dev);
> +
> + can_led_event(dev, CAN_LED_EVENT_OPEN);
> +
> + netif_start_queue(dev);
> +
> + return 0;
> +}
> +
> +static int sunxican_close(struct net_device *dev)
> +{
> + struct sunxican_priv *priv = netdev_priv(dev);
> +
> + netif_stop_queue(dev);
> + set_reset_mode(dev);
> + clk_disable_unprepare(priv->clk);
Are you sure you have the right use count on that clock? You're
calling sunxi_can_start from several places, but you call
clk_disable_unprepare only from here.
Even if it does start, it is really confusing. Please move the
clk_prepare_enable in the open function.
> +
> + free_irq(dev->irq, dev);
> + close_candev(dev);
> + can_led_event(dev, CAN_LED_EVENT_STOP);
> +
> + return 0;
> +}
> +
> +static const struct net_device_ops sunxican_netdev_ops = {
> + .ndo_open = sunxican_open,
> + .ndo_stop = sunxican_close,
> + .ndo_start_xmit = sunxican_start_xmit,
> +};
> +
> +static const struct of_device_id sunxican_of_match[] = {
> + {.compatible = "allwinner,sunxican"},
> + {},
> +};
> +
> +MODULE_DEVICE_TABLE(of, sunxican_of_match);
> +
> +static int sunxican_remove(struct platform_device *pdev)
> +{
> + struct net_device *dev = platform_get_drvdata(pdev);
> +
> + unregister_netdev(dev);
> + free_candev(dev);
> +
> + return 0;
> +}
> +
> +static int sunxican_probe(struct platform_device *pdev)
> +{
> + struct resource *mem;
> + struct clk *clk;
> + void __iomem *addr;
> + int err, irq;
> + struct net_device *dev;
> + struct sunxican_priv *priv;
> +
> + clk = devm_clk_get(&pdev->dev, "apb1_can");
If you pass NULL, it will automatically grab the first clock in the
DT, which is what you want here, otherwise the clocks property is
useless.
> + if (IS_ERR(clk)) {
> + dev_err(&pdev->dev, "no clock defined\n");
> + err = -ENODEV;
> + goto exit;
> + }
> +
> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0) {
> + dev_err(&pdev->dev, "could not get a valid irq\n");
> + err = -ENODEV;
> + goto exit;
> + }
> +
> + mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + addr = devm_ioremap_resource(&pdev->dev, mem);
> + if (IS_ERR(addr)) {
> + err = -EBUSY;
> + goto exit;
> + }
> +
> + dev = alloc_candev(sizeof(struct sunxican_priv), 1);
> + if (!dev) {
> + dev_err(&pdev->dev,
> + "could not allocate memory for CAN device\n");
> + err = -ENOMEM;
> + goto exit;
> + }
> +
> + dev->netdev_ops = &sunxican_netdev_ops;
> + dev->irq = irq;
> + dev->flags |= IFF_ECHO;
> +
> + priv = netdev_priv(dev);
> + priv->can.clock.freq = clk_get_rate(clk);
> + priv->can.bittiming_const = &sunxican_bittiming_const;
> + priv->can.do_set_mode = sunxican_set_mode;
> + priv->can.do_get_berr_counter = sunxican_get_berr_counter;
> + priv->can.ctrlmode_supported = CAN_CTRLMODE_BERR_REPORTING |
> + CAN_CTRLMODE_LISTENONLY |
> + CAN_CTRLMODE_LOOPBACK |
> + CAN_CTRLMODE_3_SAMPLES;
> + priv->base = addr;
> + priv->clk = clk;
> + spin_lock_init(&priv->cmdreg_lock);
> +
> + platform_set_drvdata(pdev, dev);
> + SET_NETDEV_DEV(dev, &pdev->dev);
> +
> + err = register_candev(dev);
> + if (err) {
> + dev_err(&pdev->dev, "registering %s failed (err=%d)\n",
> + DRV_NAME, err);
> + goto exit_free;
> + }
> + devm_can_led_init(dev);
> +
> + dev_info(&pdev->dev, "device registered (base=%p, irq=%d)\n",
> + priv->base, dev->irq);
> +
> + return 0;
> +
> +exit_free:
> + free_candev(dev);
> +exit:
> + return err;
> +}
> +
> +static int __maybe_unused sunxi_can_suspend(struct device *device)
> +{
> + struct net_device *dev = dev_get_drvdata(device);
> + struct sunxican_priv *priv = netdev_priv(dev);
> + u32 mode;
> + int err;
> +
> + if (netif_running(dev)) {
> + netif_stop_queue(dev);
> + netif_device_detach(dev);
> + }
> +
> + err = set_reset_mode(dev);
> + if (err)
> + return err;
> +
> + mode = readl(priv->base + SUNXI_REG_MSEL_ADDR);
> + writel(mode | SUNXI_MSEL_SLEEP_MODE, priv->base + SUNXI_REG_MSEL_ADDR);
> +
> + priv->can.state = CAN_STATE_SLEEPING;
> +
> + clk_disable(priv->clk);
> + return 0;
> +}
> +
> +static int __maybe_unused sunxi_can_resume(struct device *device)
> +{
> + struct net_device *dev = dev_get_drvdata(device);
> + struct sunxican_priv *priv = netdev_priv(dev);
> + u32 mode;
> + int err;
> +
> + err = clk_enable(priv->clk);
> + if (err) {
> + netdev_err(dev, "clk_enable() failed, error %d\n", err);
> + return err;
> + }
> +
> + err = set_reset_mode(dev);
> + if (err)
> + return err;
> +
> + mode = readl(priv->base + SUNXI_REG_MSEL_ADDR);
> + writel(mode & ~(SUNXI_MSEL_SLEEP_MODE),
> + priv->base + SUNXI_REG_MSEL_ADDR);
> + err = set_normal_mode(dev);
> + if (err)
> + return err;
> +
> + priv->can.state = CAN_STATE_ERROR_ACTIVE;
> + if (netif_running(dev)) {
> + netif_device_attach(dev);
> + netif_start_queue(dev);
> + }
> + return 0;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(sunxi_can_pm_ops, sunxi_can_suspend, sunxi_can_resume);
How did you test those hooks? There's no suspend support yet.
> +static struct platform_driver sunxi_can_driver = {
> + .driver = {
> + .name = DRV_NAME,
> + .pm = &sunxi_can_pm_ops,
> + .of_match_table = sunxican_of_match,
> + },
> + .probe = sunxican_probe,
> + .remove = sunxican_remove,
> +};
> +
> +module_platform_driver(sunxi_can_driver);
> +
> +MODULE_AUTHOR("Peter Chen <xingkongcp-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>");
> +MODULE_AUTHOR("Gerhard Bertelsmann <info-La43T0Mi4bH5xCKuJOYmCvaTkwRoYoCU@public.gmane.org>");
> +MODULE_LICENSE("Dual BSD/GPL");
> +MODULE_DESCRIPTION(DRV_NAME "CAN driver for Allwinner SoCs (A10/A20)");
> +MODULE_VERSION(DRV_MODULE_VERSION);
Is this going to be useful?
The module version is already redundant with the kernel version, and
there's a good chance it will never reach 1.0.
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [linux-sunxi] [PATCH v5 2/2] can: Allwinner A10/A20 CAN Controller support - controller code
2015-09-13 12:45 ` Maxime Ripard
@ 2015-09-13 12:50 ` Marc Kleine-Budde
2015-09-13 13:42 ` Gerhard Bertelsmann
1 sibling, 0 replies; 15+ messages in thread
From: Marc Kleine-Budde @ 2015-09-13 12:50 UTC (permalink / raw)
To: Maxime Ripard, Gerhard Bertelsmann; +Cc: linux-can, linux-sunxi
[-- Attachment #1: Type: text/plain, Size: 2169 bytes --]
On 09/13/2015 02:45 PM, Maxime Ripard wrote:
>> > +static irqreturn_t sunxi_can_interrupt(int irq, void *dev_id)
>> > +{
>> > + struct net_device *dev = (struct net_device *)dev_id;
>> > + struct sunxican_priv *priv = netdev_priv(dev);
>> > + struct net_device_stats *stats = &dev->stats;
>> > + u8 isrc, status;
>> > + int n = 0;
>> > +
>> > + while ((isrc = readl(priv->base + SUNXI_REG_INT_ADDR)) &&
>> > + (n < SUNXI_CAN_MAX_IRQ)) {
>> > + n++;
>> > + status = readl(priv->base + SUNXI_REG_STA_ADDR);
>> > +
>> > + if (isrc & SUNXI_INT_WAKEUP)
>> > + netdev_warn(dev, "wakeup interrupt\n");
>> > +
>> > + if (isrc & SUNXI_INT_TBUF_VLD) {
>> > + /* transmission complete interrupt */
>> > + stats->tx_bytes +=
>> > + readl(priv->base +
>> > + SUNXI_REG_RBUF_RBACK_START_ADDR) & 0xf;
>> > + stats->tx_packets++;
>> > + can_get_echo_skb(dev, 0);
>> > + netif_wake_queue(dev);
>> > + can_led_event(dev, CAN_LED_EVENT_TX);
>> > + }
>> > + if (isrc & SUNXI_INT_RBUF_VLD) {
>> > + /* receive interrupt */
>> > + while (status & SUNXI_STA_RBUF_RDY) {
>> > + /* RX buffer is not empty */
>> > + sunxi_can_rx(dev);
>> > + status = readl(priv->base + SUNXI_REG_STA_ADDR);
>> > + }
>> > + }
>> > + if (isrc &
>> > + (SUNXI_INT_DATA_OR | SUNXI_INT_ERR_WRN | SUNXI_INT_BUS_ERR |
>> > + SUNXI_INT_ERR_PASSIVE | SUNXI_INT_ARB_LOST)) {
>> > + /* error interrupt */
>> > + if (sunxi_can_err(dev, isrc, status))
>> > + break;
>> > + }
>> > + /* clear interrupts */
>> > + writel(isrc, priv->base + SUNXI_REG_INT_ADDR);
>> > + readl(priv->base + SUNXI_REG_INT_ADDR);
>> > + }
> Why not simply handle whatever you have in the status register, and
> let the interrupt retrigger if there's something new?
Latency. On CAN Networks you can have an interrupt rate of sevel KHz.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [linux-sunxi] [PATCH v5 0/2] can: Allwinner A10/A20 CAN Controller support - summary
2015-09-13 12:22 ` [PATCH v5 0/2] can: Allwinner A10/A20 CAN Controller support - summary Maxime Ripard
@ 2015-09-13 12:57 ` Marc Kleine-Budde
2015-09-13 13:42 ` Maxime Ripard
0 siblings, 1 reply; 15+ messages in thread
From: Marc Kleine-Budde @ 2015-09-13 12:57 UTC (permalink / raw)
To: Maxime Ripard, Gerhard Bertelsmann; +Cc: linux-can, linux-sunxi
[-- Attachment #1: Type: text/plain, Size: 1684 bytes --]
On 09/13/2015 02:22 PM, Maxime Ripard wrote:
>> History:
>> V5: fix license
>> modify prefix to mode select defines
>> enable and disable clock in sunxican_get_berr_counter
>> delete set_normal_mode at the end of sunxi_can_start
>> removed sunxican_id_table
>> use devm_clk_get instead of clk_get
>> use devm_ioremap_resource to simplify probe and remove
>> make set-normal-mode and set-reset-mode more readable
>>
>> v4: defines prefixed with SUNXI_
>> sunxi_can_write_cmdreg tweaked
>> loops in set_xxx_mode reworked
>> add return value to set_xxx_mode
>> sunxican_start_xmit reworked
>> struct platform_driver stripped
>> moved set_bittiming into open
>> moved clock start into open
>> add clock stop to close
>> suspend reworked
>> resume reworked
>> fixed double counting bug
>>
>> v3: changed error state change handling (thx Andri for the hint)
>> use bittiming function correct (no need to call it)
>> strip down priv (suggested by Marc)
>> scripts/checkpatch.pl-> no matches anymore
>> sparse -> no errors or warnings anymore
>> v2: cleaning
>> v1: initial
>
> Where were those versions posted ?
On the linux-can mailinglist.
> Make sure to run get_maintainer before sending your patches to mail
> them to the right recipients.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [linux-sunxi] [PATCH v5 0/2] can: Allwinner A10/A20 CAN Controller support - summary
2015-09-13 12:57 ` [linux-sunxi] " Marc Kleine-Budde
@ 2015-09-13 13:42 ` Maxime Ripard
0 siblings, 0 replies; 15+ messages in thread
From: Maxime Ripard @ 2015-09-13 13:42 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: Gerhard Bertelsmann, linux-can, linux-sunxi
[-- Attachment #1: Type: text/plain, Size: 1663 bytes --]
On Sun, Sep 13, 2015 at 02:57:24PM +0200, Marc Kleine-Budde wrote:
> On 09/13/2015 02:22 PM, Maxime Ripard wrote:
> >> History:
> >> V5: fix license
> >> modify prefix to mode select defines
> >> enable and disable clock in sunxican_get_berr_counter
> >> delete set_normal_mode at the end of sunxi_can_start
> >> removed sunxican_id_table
> >> use devm_clk_get instead of clk_get
> >> use devm_ioremap_resource to simplify probe and remove
> >> make set-normal-mode and set-reset-mode more readable
> >>
> >> v4: defines prefixed with SUNXI_
> >> sunxi_can_write_cmdreg tweaked
> >> loops in set_xxx_mode reworked
> >> add return value to set_xxx_mode
> >> sunxican_start_xmit reworked
> >> struct platform_driver stripped
> >> moved set_bittiming into open
> >> moved clock start into open
> >> add clock stop to close
> >> suspend reworked
> >> resume reworked
> >> fixed double counting bug
> >>
> >> v3: changed error state change handling (thx Andri for the hint)
> >> use bittiming function correct (no need to call it)
> >> strip down priv (suggested by Marc)
> >> scripts/checkpatch.pl-> no matches anymore
> >> sparse -> no errors or warnings anymore
> >> v2: cleaning
> >> v1: initial
> >
> > Where were those versions posted ?
>
> On the linux-can mailinglist.
Hmm, ok. My comment about using get_maintainer to have the right
recipient list is even more valid then.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v5 2/2] can: Allwinner A10/A20 CAN Controller support - controller code
2015-09-13 12:45 ` Maxime Ripard
2015-09-13 12:50 ` [linux-sunxi] " Marc Kleine-Budde
@ 2015-09-13 13:42 ` Gerhard Bertelsmann
2015-09-13 13:49 ` [linux-sunxi] " Marc Kleine-Budde
[not found] ` <f8eea0f544eb6367142bbc1c2ab35964-6XUBc1GShDsOIzVOb1FTxg@public.gmane.org>
1 sibling, 2 replies; 15+ messages in thread
From: Gerhard Bertelsmann @ 2015-09-13 13:42 UTC (permalink / raw)
To: Maxime Ripard
Cc: linux-can-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, mkl-bIcnvbaLZ9MEGnE8C9+IrQ,
linux-can-owner-u79uwXL29TY76Z2rM5mHXA
Hi Maxime,
Am 2015-09-13 14:45, schrieb Maxime Ripard:
> Hi!
>
> On Sun, Sep 13, 2015 at 01:43:52PM +0200, Gerhard Bertelsmann wrote:
>> Signed-off-by: Gerhard Bertelsmann <info-La43T0Mi4bH5xCKuJOYmCvaTkwRoYoCU@public.gmane.org>
>
> And one commit log here too please :)
>
>> ---
>> drivers/net/can/Kconfig | 10 +
>> drivers/net/can/Makefile | 1 +
>> drivers/net/can/sunxi_can.c | 879
>> +++++++++++++++++++++
>> 3 files changed, 890 insertions(+)
>>
>> diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
>> index e8c96b8..439f67c 100644
>> --- a/drivers/net/can/Kconfig
>> +++ b/drivers/net/can/Kconfig
>> @@ -129,6 +129,16 @@ config CAN_RCAR
>> To compile this driver as a module, choose M here: the module will
>> be called rcar_can.
>>
>> +config CAN_SUNXI
>> + tristate "Allwinner SUNXI CAN controller"
>
> It would be better if we could mention that it's a driver for the
> sun4i family and derived.
I'm using it on Bpi with A20. AFAIK a SUN7I. Anyway I'm not aware
of all Allwinner SoCs. I will change it to any text you want.
>
> We have no guarantee that there won't be any IP on other sunxi SoCs
> incompatible with this one that would need a new driver.
Right. IMHO A10 and A20 are working - maybe Fxx(?).
>
> Something like CAN_SUN4I (and the driver sun4i_can.c) would be great.
>
> (This also applies to all the symbols/defines/etc inside the driver,
> for consistency).
>
>> + depends on MACH_SUN4I || MACH_SUN7I || COMPILE_TEST
>> + ---help---
>> + Say Y here if you want to use CAN controller found on Allwinner
>> + A10/A20 SoCs.
>> +
>> + To compile this driver as a module, choose M here: the module will
>> + be called rcar_can.
>
> You're sure about the name of the compiled module?
Oops. Will be correct the in the next patch.
>
>> +
>> config CAN_XILINXCAN
>> tristate "Xilinx CAN"
>> depends on ARCH_ZYNQ || ARM64 || MICROBLAZE || COMPILE_TEST
>> diff --git a/drivers/net/can/Makefile b/drivers/net/can/Makefile
>> index c533c62..b3e825c 100644
>> --- a/drivers/net/can/Makefile
>> +++ b/drivers/net/can/Makefile
>> @@ -27,6 +27,7 @@ obj-$(CONFIG_CAN_FLEXCAN) += flexcan.o
>> obj-$(CONFIG_PCH_CAN) += pch_can.o
>> obj-$(CONFIG_CAN_GRCAN) += grcan.o
>> obj-$(CONFIG_CAN_RCAR) += rcar_can.o
>> +obj-$(CONFIG_CAN_SUNXI) += sunxi_can.o
>> obj-$(CONFIG_CAN_XILINXCAN) += xilinx_can.o
>>
>> subdir-ccflags-y += -D__CHECK_ENDIAN__
>> diff --git a/drivers/net/can/sunxi_can.c b/drivers/net/can/sunxi_can.c
>> new file mode 100644
>> index 0000000..569e6cd
>> --- /dev/null
>> +++ b/drivers/net/can/sunxi_can.c
>> @@ -0,0 +1,879 @@
>> +/*
>> + * sunxi_can.c - CAN bus controller driver for Allwinner SUN4I&SUN7I
>> based SoCs
>> + *
>> + * Copyright (C) 2013 Peter Chen
>> + * Copyright (C) 2015 Gerhard Bertelsmann
>> + * All rights reserved.
>> + *
>> + * Parts of this software are based on (derived from) the SJA1000
>> code by:
>> + * Copyright (C) 2014 Oliver Hartkopp
>> <oliver.hartkopp-l29pVbxQd1IUtdQbppsyvg@public.gmane.org>
>> + * Copyright (C) 2007 Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
>> + * Copyright (C) 2002-2007 Volkswagen Group Electronic Research
>> + * Copyright (C) 2003 Matthias Brukner, Trajet Gmbh, Rebenring 33,
>> + * 38106 Braunschweig, GERMANY
>> + *
>> + * Redistribution and use in source and binary forms, with or without
>> + * modification, are permitted provided that the following conditions
>> + * are met:
>> + * 1. Redistributions of source code must retain the above copyright
>> + * notice, this list of conditions and the following disclaimer.
>> + * 2. Redistributions in binary form must reproduce the above
>> copyright
>> + * notice, this list of conditions and the following disclaimer in
>> the
>> + * documentation and/or other materials provided with the
>> distribution.
>> + * 3. Neither the name of Volkswagen nor the names of its
>> contributors
>> + * may be used to endorse or promote products derived from this
>> software
>> + * without specific prior written permission.
>> + *
>> + * Alternatively, provided that this notice is retained in full, this
>> + * software may be distributed under the terms of the GNU General
>> + * Public License ("GPL") version 2, in which case the provisions of
>> the
>> + * GPL apply INSTEAD OF those given above.
>> + *
>> + * The provided data structures and external interfaces from this
>> code
>> + * are not restricted to be used by modules with a GPL compatible
>> license.
>> + *
>> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
>> CONTRIBUTORS
>> + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
>> + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
>> FOR
>> + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
>> COPYRIGHT
>> + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
>> INCIDENTAL,
>> + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
>> + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
>> USE,
>> + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
>> ANY
>> + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR
>> TORT
>> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
>> USE
>> + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
>> + * DAMAGE.
>> + *
>> + */
>> +
>> +#include <linux/netdevice.h>
>> +#include <linux/can.h>
>> +#include <linux/can/dev.h>
>> +#include <linux/can/error.h>
>> +#include <linux/can/led.h>
>> +#include <linux/clk.h>
>> +#include <linux/delay.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/init.h>
>> +#include <linux/io.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_device.h>
>> +#include <linux/platform_device.h>
>> +
>> +#define DRV_NAME "sunxi_can"
>> +#define DRV_MODULE_VERSION "0.99"
>> +
>> +/* Registers address (physical base address 0x01C2BC00) */
>> +#define SUNXI_REG_MSEL_ADDR 0x0000 /* CAN Mode Select */
>> +#define SUNXI_REG_CMD_ADDR 0x0004 /* CAN Command */
>> +#define SUNXI_REG_STA_ADDR 0x0008 /* CAN Status */
>> +#define SUNXI_REG_INT_ADDR 0x000c /* CAN Interrupt Flag */
>> +#define SUNXI_REG_INTEN_ADDR 0x0010 /* CAN Interrupt Enable */
>> +#define SUNXI_REG_BTIME_ADDR 0x0014 /* CAN Bus Timing 0 */
>> +#define SUNXI_REG_TEWL_ADDR 0x0018 /* CAN Tx Error Warning Limit */
>> +#define SUNXI_REG_ERRC_ADDR 0x001c /* CAN Error Counter */
>> +#define SUNXI_REG_RMCNT_ADDR 0x0020 /* CAN Receive Message Counter */
>> +#define SUNXI_REG_RBUFSA_ADDR 0x0024 /* CAN Receive Buffer Start
>> Address */
>> +#define SUNXI_REG_BUF0_ADDR 0x0040 /* CAN Tx/Rx Buffer 0 */
>> +#define SUNXI_REG_BUF1_ADDR 0x0044 /* CAN Tx/Rx Buffer 1 */
>> +#define SUNXI_REG_BUF2_ADDR 0x0048 /* CAN Tx/Rx Buffer 2 */
>> +#define SUNXI_REG_BUF3_ADDR 0x004c /* CAN Tx/Rx Buffer 3 */
>> +#define SUNXI_REG_BUF4_ADDR 0x0050 /* CAN Tx/Rx Buffer 4 */
>> +#define SUNXI_REG_BUF5_ADDR 0x0054 /* CAN Tx/Rx Buffer 5 */
>> +#define SUNXI_REG_BUF6_ADDR 0x0058 /* CAN Tx/Rx Buffer 6 */
>> +#define SUNXI_REG_BUF7_ADDR 0x005c /* CAN Tx/Rx Buffer 7 */
>> +#define SUNXI_REG_BUF8_ADDR 0x0060 /* CAN Tx/Rx Buffer 8 */
>> +#define SUNXI_REG_BUF9_ADDR 0x0064 /* CAN Tx/Rx Buffer 9 */
>> +#define SUNXI_REG_BUF10_ADDR 0x0068 /* CAN Tx/Rx Buffer 10 */
>> +#define SUNXI_REG_BUF11_ADDR 0x006c /* CAN Tx/Rx Buffer 11 */
>> +#define SUNXI_REG_BUF12_ADDR 0x0070 /* CAN Tx/Rx Buffer 12 */
>> +#define SUNXI_REG_ACPC_ADDR 0x0040 /* CAN Acceptance Code 0 */
>> +#define SUNXI_REG_ACPM_ADDR 0x0044 /* CAN Acceptance Mask 0 */
>> +#define SUNXI_REG_RBUF_RBACK_START_ADDR 0x0180 /* CAN transmit buffer
>> start */
>> +#define SUNXI_REG_RBUF_RBACK_END_ADDR 0x01b0 /* CAN transmit buffer
>> end */
>> +
>> +/* Controller Register Description */
>> +
>> +/* mode select register (r/w)
>> + * offset:0x0000 default:0x0000_0001
>> + */
>> +#define SUNXI_MSEL_SLEEP_MODE (0x01 << 4) /* write in reset mode
>> */
>> +#define SUNXI_MSEL_WAKE_UP (0x00 << 4)
>> +#define SUNXI_MSEL_SINGLE_FILTER (0x01 << 3) /* write in reset mode
>> */
>> +#define SUNXI_MSEL_DUAL_FILTERS (0x00 << 3)
>> +#define SUNXI_MSEL_LOOPBACK_MODE BIT(2)
>> +#define SUNXI_MSEL_LISTEN_ONLY_MODE BIT(1)
>> +#define SUNXI_MSEL_RESET_MODE BIT(0)
>> +
>> +/* command register (w)
>> + * offset:0x0004 default:0x0000_0000
>> + */
>> +#define SUNXI_CMD_BUS_OFF_REQ BIT(5)
>> +#define SUNXI_CMD_SELF_RCV_REQ BIT(4)
>> +#define SUNXI_CMD_CLEAR_OR_FLAG BIT(3)
>> +#define SUNXI_CMD_RELEASE_RBUF BIT(2)
>> +#define SUNXI_CMD_ABORT_REQ BIT(1)
>> +#define SUNXI_CMD_TRANS_REQ BIT(0)
>> +
>> +/* status register (r)
>> + * offset:0x0008 default:0x0000_003c
>> + */
>> +#define SUNXI_STA_BIT_ERR (0x00 << 22)
>> +#define SUNXI_STA_FORM_ERR (0x01 << 22)
>> +#define SUNXI_STA_STUFF_ERR (0x02 << 22)
>> +#define SUNXI_STA_OTHER_ERR (0x03 << 22)
>> +#define SUNXI_STA_MASK_ERR (0x03 << 22)
>> +#define SUNXI_STA_ERR_DIR BIT(21)
>> +#define SUNXI_STA_ERR_SEG_CODE (0x1f << 16)
>> +#define SUNXI_STA_START (0x03 << 16)
>> +#define SUNXI_STA_ID28_21 (0x02 << 16)
>> +#define SUNXI_STA_ID20_18 (0x06 << 16)
>> +#define SUNXI_STA_SRTR (0x04 << 16)
>> +#define SUNXI_STA_IDE (0x05 << 16)
>> +#define SUNXI_STA_ID17_13 (0x07 << 16)
>> +#define SUNXI_STA_ID12_5 (0x0f << 16)
>> +#define SUNXI_STA_ID4_0 (0x0e << 16)
>> +#define SUNXI_STA_RTR (0x0c << 16)
>> +#define SUNXI_STA_RB1 (0x0d << 16)
>> +#define SUNXI_STA_RB0 (0x09 << 16)
>> +#define SUNXI_STA_DLEN (0x0b << 16)
>> +#define SUNXI_STA_DATA_FIELD (0x0a << 16)
>> +#define SUNXI_STA_CRC_SEQUENCE (0x08 << 16)
>> +#define SUNXI_STA_CRC_DELIMITER (0x18 << 16)
>> +#define SUNXI_STA_ACK (0x19 << 16)
>> +#define SUNXI_STA_ACK_DELIMITER (0x1b << 16)
>> +#define SUNXI_STA_END (0x1a << 16)
>> +#define SUNXI_STA_INTERMISSION (0x12 << 16)
>> +#define SUNXI_STA_ACTIVE_ERROR (0x11 << 16)
>> +#define SUNXI_STA_PASSIVE_ERROR (0x16 << 16)
>> +#define SUNXI_STA_TOLERATE_DOMINANT_BITS (0x13 << 16)
>> +#define SUNXI_STA_ERROR_DELIMITER (0x17 << 16)
>> +#define SUNXI_STA_OVERLOAD (0x1c << 16)
>> +#define SUNXI_STA_BUS_OFF BIT(7)
>> +#define SUNXI_STA_ERR_STA BIT(6)
>> +#define SUNXI_STA_TRANS_BUSY BIT(5)
>> +#define SUNXI_STA_RCV_BUSY BIT(4)
>> +#define SUNXI_STA_TRANS_OVER BIT(3)
>> +#define SUNXI_STA_TBUF_RDY BIT(2)
>> +#define SUNXI_STA_DATA_ORUN BIT(1)
>> +#define SUNXI_STA_RBUF_RDY BIT(0)
>> +
>> +/* interrupt register (r)
>> + * offset:0x000c default:0x0000_0000
>> + */
>> +#define SUNXI_INT_BUS_ERR BIT(7)
>> +#define SUNXI_INT_ARB_LOST BIT(6)
>> +#define SUNXI_INT_ERR_PASSIVE BIT(5)
>> +#define SUNXI_INT_WAKEUP BIT(4)
>> +#define SUNXI_INT_DATA_OR BIT(3)
>> +#define SUNXI_INT_ERR_WRN BIT(2)
>> +#define SUNXI_INT_TBUF_VLD BIT(1)
>> +#define SUNXI_INT_RBUF_VLD BIT(0)
>> +
>> +/* interrupt enable register (r/w)
>> + * offset:0x0010 default:0x0000_0000
>> + */
>> +#define SUNXI_INTEN_BERR BIT(7)
>> +#define SUNXI_INTEN_ARB_LOST BIT(6)
>> +#define SUNXI_INTEN_ERR_PASSIVE BIT(5)
>> +#define SUNXI_INTEN_WAKEUP BIT(4)
>> +#define SUNXI_INTEN_OR BIT(3)
>> +#define SUNXI_INTEN_ERR_WRN BIT(2)
>> +#define SUNXI_INTEN_TX BIT(1)
>> +#define SUNXI_INTEN_RX BIT(0)
>> +
>> +/* error code */
>> +#define SUNXI_ERR_INRCV (0x1 << 5)
>> +#define SUNXI_ERR_INTRANS (0x0 << 5)
>> +
>> +/* filter mode */
>> +#define SINXI_FILTER_CLOSE 0
>> +#define SUNXI_SINGLE_FLTER_MODE 1
>> +#define SUNXI_DUAL_FILTER_MODE 2
>> +
>> +/* message buffer flags */
>> +#define SUNXI_MSG_EFF_FLAG BIT(7)
>> +#define SUNXI_MSG_RTR_FLAG BIT(6)
>> +
>> +/* max. number of interrupts handled in ISR */
>> +#define SUNXI_CAN_MAX_IRQ 20
>> +#define SUNXI_MODE_MAX_RETRIES 100
>> +
>> +struct sunxican_priv {
>> + struct can_priv can;
>> + void __iomem *base;
>> + struct clk *clk;
>> + spinlock_t cmdreg_lock; /* lock for concurrent cmd register writes
>> */
>> +};
>> +
>> +static const struct can_bittiming_const sunxican_bittiming_const = {
>> + .name = DRV_NAME,
>> + .tseg1_min = 1,
>> + .tseg1_max = 16,
>> + .tseg2_min = 1,
>> + .tseg2_max = 8,
>> + .sjw_max = 4,
>> + .brp_min = 1,
>> + .brp_max = 64,
>> + .brp_inc = 1,
>> +};
>> +
>> +static void sunxi_can_write_cmdreg(struct sunxican_priv *priv, u8
>> val)
>> +{
>> + unsigned long flags;
>> +
>> + spin_lock_irqsave(&priv->cmdreg_lock, flags);
>> + writel(val, priv->base + SUNXI_REG_CMD_ADDR);
>> + spin_unlock_irqrestore(&priv->cmdreg_lock, flags);
>> +}
>> +
>> +static int set_normal_mode(struct net_device *dev)
>> +{
>> + struct sunxican_priv *priv = netdev_priv(dev);
>> + int retry = SUNXI_MODE_MAX_RETRIES;
>> + u32 mod_reg_val = 0;
>> +
>> + do {
>> + mod_reg_val = readl(priv->base + SUNXI_REG_MSEL_ADDR);
>> + mod_reg_val &= ~SUNXI_MSEL_RESET_MODE;
>> + writel(mod_reg_val, priv->base + SUNXI_REG_MSEL_ADDR);
>> + } while (retry-- && (mod_reg_val & SUNXI_MSEL_RESET_MODE));
>> +
>> + if (readl(priv->base + SUNXI_REG_MSEL_ADDR) & SUNXI_MSEL_RESET_MODE)
>> {
>> + netdev_err(dev,
>> + "setting controller into normal mode failed!\n");
>> + return -ETIMEDOUT;
>> + }
>> +
>> + priv->can.state = CAN_STATE_ERROR_ACTIVE;
>> + /* enable interrupts */
>> + if (priv->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING)
>> + writel(0xFFFF, priv->base + SUNXI_REG_INTEN_ADDR);
>> + else
>> + writel(0xFFFF & ~SUNXI_INTEN_BERR,
>> + priv->base + SUNXI_REG_INTEN_ADDR);
>> +
>> + mod_reg_val = readl(priv->base + SUNXI_REG_MSEL_ADDR);
>> +
>> + if (priv->can.ctrlmode & CAN_CTRLMODE_PRESUME_ACK)
>> + mod_reg_val |= SUNXI_MSEL_LOOPBACK_MODE;
>> + else if (priv->can.ctrlmode & CAN_CTRLMODE_LISTENONLY)
>> + mod_reg_val |= SUNXI_MSEL_LISTEN_ONLY_MODE;
>> +
>> + writel(mod_reg_val, priv->base + SUNXI_REG_MSEL_ADDR);
>> + return 0;
>> +}
>> +
>> +static int set_reset_mode(struct net_device *dev)
>> +{
>> + struct sunxican_priv *priv = netdev_priv(dev);
>> + int retry = SUNXI_MODE_MAX_RETRIES;
>> + u32 mod_reg_val = 0;
>> +
>> + do {
>> + mod_reg_val = readl(priv->base + SUNXI_REG_MSEL_ADDR);
>> + mod_reg_val |= SUNXI_MSEL_RESET_MODE;
>> + writel(mod_reg_val, priv->base + SUNXI_REG_MSEL_ADDR);
>> + } while (retry-- && !(mod_reg_val & SUNXI_MSEL_RESET_MODE));
>> +
>> + if (!(readl(priv->base + SUNXI_REG_MSEL_ADDR) &
>> + SUNXI_MSEL_RESET_MODE)) {
>> + netdev_err(dev, "setting controller into reset mode failed!\n");
>> + return -ETIMEDOUT;
>> + }
>> +
>> + priv->can.state = CAN_STATE_STOPPED;
>> +
>> + return 0;
>> +}
>> +
>> +static int sunxican_set_bittiming(struct net_device *dev)
>> +{
>> + struct sunxican_priv *priv = netdev_priv(dev);
>> + struct can_bittiming *bt = &priv->can.bittiming;
>> + int err;
>> + u32 cfg;
>> +
>> + cfg = ((bt->brp - 1) & 0x3FF) |
>> + (((bt->sjw - 1) & 0x3) << 14) |
>> + (((bt->prop_seg + bt->phase_seg1 - 1) & 0xf) << 16) |
>> + (((bt->phase_seg2 - 1) & 0x7) << 20);
>> + if (priv->can.ctrlmode & CAN_CTRLMODE_3_SAMPLES)
>> + cfg |= 0x800000;
>> +
>> + netdev_info(dev, "setting BITTIMING=0x%08x\n", cfg);
>> +
>> + /* CAN_BTIME_ADDR only writable in reset mode */
>> + err = set_reset_mode(dev);
>> + if (err)
>> + return err;
>> + writel(cfg, priv->base + SUNXI_REG_BTIME_ADDR);
>> +
>> + return set_normal_mode(dev);
>> +}
>> +
>> +static int sunxican_get_berr_counter(const struct net_device *dev,
>> + struct can_berr_counter *bec)
>> +{
>> + struct sunxican_priv *priv = netdev_priv(dev);
>> + u32 errors;
>> + int err;
>> +
>> + err = clk_prepare_enable(priv->clk);
>> + if (err) {
>> + netdev_err(dev, "could not enable clock\n");
>> + return err;
>> + }
>> +
>> + errors = readl(priv->base + SUNXI_REG_ERRC_ADDR);
>> +
>> + bec->txerr = errors & 0xFF;
>> + bec->rxerr = (errors >> 16) & 0xFF;
>> +
>> + clk_disable_unprepare(priv->clk);
>> +
>> + return 0;
>> +}
>> +
>> +static int sunxi_can_start(struct net_device *dev)
>> +{
>> + struct sunxican_priv *priv = netdev_priv(dev);
>> + int err;
>> + u32 temp_irqen;
>> +
>> + /* turn on clocking for CAN peripheral block */
>> + err = clk_prepare_enable(priv->clk);
>> + if (err) {
>> + netdev_err(dev, "could not enable clocking (apb1_can)\n");
>> + goto exit;
>> + }
>> +
>> + /* leave reset mode */
>> + if (priv->can.state != CAN_STATE_STOPPED)
>> + set_reset_mode(dev);
>> +
>> + /* set filters - we accept all */
>> + writel(0x00000000, priv->base + SUNXI_REG_ACPC_ADDR);
>> + writel(0xFFFFFFFF, priv->base + SUNXI_REG_ACPM_ADDR);
>> +
>> + /* Clear error counters and error code capture */
>> + writel(0x0, priv->base + SUNXI_REG_ERRC_ADDR);
>> +
>> + /* enable CAN specific interrupts */
>> + temp_irqen = SUNXI_INTEN_BERR | SUNXI_INTEN_ERR_PASSIVE |
>> + SUNXI_INTEN_OR | SUNXI_INTEN_RX;
>> + writel(readl(priv->base + SUNXI_REG_INTEN_ADDR) | temp_irqen,
>> + priv->base + SUNXI_REG_INTEN_ADDR);
>> +
>> + err = sunxican_set_bittiming(dev);
>> +
>> + if (err)
>> + return err;
>
> You're leaving the clock enabled if you have an error.
>
I will have a look at this.
>> +
>> + return 0;
>> +
>> +exit:
>> + return err;
>> +}
>> +
>> +static int sunxican_set_mode(struct net_device *dev, enum can_mode
>> mode)
>> +{
>> + int err;
>> +
>> + switch (mode) {
>> + case CAN_MODE_START:
>> + err = sunxi_can_start(dev);
>> + if (err) {
>> + netdev_err(dev, "starting CAN controller failed!\n");
>> + return err;
>> + }
>> + if (netif_queue_stopped(dev))
>> + netif_wake_queue(dev);
>> + break;
>> +
>> + default:
>> + return -EOPNOTSUPP;
>> + }
>> + return 0;
>> +}
>> +
>> +/* transmit a CAN message
>> + * message layout in the sk_buff should be like this:
>> + * xx xx xx xx ff ll 00 11 22 33 44 55 66 77
>> + * [ can_id ] [flags] [len] [can data (up to 8 bytes]
>> + */
>> +static int sunxican_start_xmit(struct sk_buff *skb, struct net_device
>> *dev)
>> +{
>> + struct sunxican_priv *priv = netdev_priv(dev);
>> + struct can_frame *cf = (struct can_frame *)skb->data;
>> + u8 dlc;
>> + u32 dreg, msg_flag_n;
>> + canid_t id;
>> + int i;
>> +
>> + if (can_dropped_invalid_skb(dev, skb))
>> + return NETDEV_TX_OK;
>> +
>> + netif_stop_queue(dev);
>> +
>> + id = cf->can_id;
>> + dlc = cf->can_dlc;
>> + msg_flag_n = dlc;
>> +
>> + if (id & CAN_RTR_FLAG)
>> + msg_flag_n |= SUNXI_MSG_RTR_FLAG;
>> +
>> + if (id & CAN_EFF_FLAG) {
>> + msg_flag_n |= SUNXI_MSG_EFF_FLAG;
>> + dreg = SUNXI_REG_BUF5_ADDR;
>> + writel((id >> 21) & 0xFF, priv->base + SUNXI_REG_BUF1_ADDR);
>> + writel((id >> 13) & 0xFF, priv->base + SUNXI_REG_BUF2_ADDR);
>> + writel((id >> 5) & 0xFF, priv->base + SUNXI_REG_BUF3_ADDR);
>> + writel((id << 3) & 0xF8, priv->base + SUNXI_REG_BUF4_ADDR);
>> + } else {
>> + dreg = SUNXI_REG_BUF3_ADDR;
>> + writel((id >> 3) & 0xFF, priv->base + SUNXI_REG_BUF1_ADDR);
>> + writel((id << 5) & 0xE0, priv->base + SUNXI_REG_BUF2_ADDR);
>> + }
>> +
>> + for (i = 0; i < dlc; i++)
>> + writel(cf->data[i], priv->base + (dreg + i * 4));
>> +
>> + writel(msg_flag_n, priv->base + SUNXI_REG_BUF0_ADDR);
>> +
>> + can_put_echo_skb(skb, dev, 0);
>> +
>> + if (priv->can.ctrlmode & CAN_CTRLMODE_LOOPBACK)
>> + sunxi_can_write_cmdreg(priv, SUNXI_CMD_SELF_RCV_REQ);
>> + else
>> + sunxi_can_write_cmdreg(priv, SUNXI_CMD_TRANS_REQ);
>> +
>> + return NETDEV_TX_OK;
>> +}
>> +
>> +static void sunxi_can_rx(struct net_device *dev)
>> +{
>> + struct sunxican_priv *priv = netdev_priv(dev);
>> + struct net_device_stats *stats = &dev->stats;
>> + struct can_frame *cf;
>> + struct sk_buff *skb;
>> + u8 fi;
>> + u32 dreg;
>> + canid_t id;
>> + int i;
>> +
>> + /* create zero'ed CAN frame buffer */
>> + skb = alloc_can_skb(dev, &cf);
>> + if (!skb)
>> + return;
>> +
>> + fi = readl(priv->base + SUNXI_REG_BUF0_ADDR);
>> + cf->can_dlc = get_can_dlc(fi & 0x0F);
>> + if (fi & SUNXI_MSG_EFF_FLAG) {
>> + dreg = SUNXI_REG_BUF5_ADDR;
>> + id = (readl(priv->base + SUNXI_REG_BUF1_ADDR) << 21) |
>> + (readl(priv->base + SUNXI_REG_BUF2_ADDR) << 13) |
>> + (readl(priv->base + SUNXI_REG_BUF3_ADDR) << 5) |
>> + ((readl(priv->base + SUNXI_REG_BUF4_ADDR) >> 3) & 0x1f);
>> + id |= CAN_EFF_FLAG;
>> + } else {
>> + dreg = SUNXI_REG_BUF3_ADDR;
>> + id = (readl(priv->base + SUNXI_REG_BUF1_ADDR) << 3) |
>> + ((readl(priv->base + SUNXI_REG_BUF2_ADDR) >> 5) & 0x7);
>> + }
>> +
>> + /* remote frame ? */
>> + if (fi & SUNXI_MSG_RTR_FLAG)
>> + id |= CAN_RTR_FLAG;
>> + else
>> + for (i = 0; i < cf->can_dlc; i++)
>> + cf->data[i] = readl(priv->base + dreg + i * 4);
>> +
>> + cf->can_id = id;
>> +
>> + sunxi_can_write_cmdreg(priv, SUNXI_CMD_RELEASE_RBUF);
>> +
>> + stats->rx_packets++;
>> + stats->rx_bytes += cf->can_dlc;
>> + netif_rx(skb);
>> +
>> + can_led_event(dev, CAN_LED_EVENT_RX);
>> +}
>> +
>> +static int sunxi_can_err(struct net_device *dev, u8 isrc, u8 status)
>> +{
>> + struct sunxican_priv *priv = netdev_priv(dev);
>> + struct net_device_stats *stats = &dev->stats;
>> + struct can_frame *cf;
>> + struct sk_buff *skb;
>> + enum can_state state = priv->can.state;
>> + enum can_state rx_state, tx_state;
>> + unsigned int rxerr, txerr, errc;
>> + u32 ecc, alc;
>> +
>> + skb = alloc_can_err_skb(dev, &cf);
>> + if (!skb)
>> + return -ENOMEM;
>> +
>> + errc = readl(priv->base + SUNXI_REG_ERRC_ADDR);
>> + rxerr = (errc >> 16) & 0xFF;
>> + txerr = errc & 0xFF;
>> +
>> + cf->data[6] = txerr;
>> + cf->data[7] = rxerr;
>> +
>> + if (isrc & SUNXI_INT_DATA_OR) {
>> + /* data overrun interrupt */
>> + netdev_dbg(dev, "data overrun interrupt\n");
>> + cf->can_id |= CAN_ERR_CRTL;
>> + cf->data[1] = CAN_ERR_CRTL_RX_OVERFLOW;
>> + stats->rx_over_errors++;
>> + stats->rx_errors++;
>> + /* clear bit */
>> + sunxi_can_write_cmdreg(priv, SUNXI_CMD_CLEAR_OR_FLAG);
>> + }
>> + if (isrc & SUNXI_INT_ERR_WRN) {
>> + /* error warning interrupt */
>> + netdev_dbg(dev, "error warning interrupt\n");
>> +
>> + if (status & SUNXI_STA_BUS_OFF)
>> + state = CAN_STATE_BUS_OFF;
>> + else if (status & SUNXI_STA_ERR_STA)
>> + state = CAN_STATE_ERROR_WARNING;
>> + else
>> + state = CAN_STATE_ERROR_ACTIVE;
>> + }
>> + if (isrc & SUNXI_INT_BUS_ERR) {
>> + /* bus error interrupt */
>> + netdev_dbg(dev, "bus error interrupt\n");
>> + priv->can.can_stats.bus_error++;
>> + stats->rx_errors++;
>> +
>> + ecc = readl(priv->base + SUNXI_REG_STA_ADDR);
>> +
>> + cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
>> +
>> + switch (ecc & SUNXI_STA_MASK_ERR) {
>> + case SUNXI_STA_BIT_ERR:
>> + cf->data[2] |= CAN_ERR_PROT_BIT;
>> + break;
>> + case SUNXI_STA_FORM_ERR:
>> + cf->data[2] |= CAN_ERR_PROT_FORM;
>> + break;
>> + case SUNXI_STA_STUFF_ERR:
>> + cf->data[2] |= CAN_ERR_PROT_STUFF;
>> + break;
>> + default:
>> + cf->data[2] |= CAN_ERR_PROT_UNSPEC;
>> + cf->data[3] = (ecc & SUNXI_STA_ERR_SEG_CODE) >> 16;
>> + break;
>> + }
>> + /* error occurred during transmission? */
>> + if ((ecc & SUNXI_STA_ERR_DIR) == 0)
>> + cf->data[2] |= CAN_ERR_PROT_TX;
>> + }
>> + if (isrc & SUNXI_INT_ERR_PASSIVE) {
>> + /* error passive interrupt */
>> + netdev_dbg(dev, "error passive interrupt\n");
>> + if (state == CAN_STATE_ERROR_PASSIVE)
>> + state = CAN_STATE_ERROR_WARNING;
>> + else
>> + state = CAN_STATE_ERROR_PASSIVE;
>> + }
>> + if (isrc & SUNXI_INT_ARB_LOST) {
>> + /* arbitration lost interrupt */
>> + netdev_dbg(dev, "arbitration lost interrupt\n");
>> + alc = readl(priv->base + SUNXI_REG_STA_ADDR);
>> + priv->can.can_stats.arbitration_lost++;
>> + stats->tx_errors++;
>> + cf->can_id |= CAN_ERR_LOSTARB;
>> + cf->data[0] = (alc & 0x1f) >> 8;
>> + }
>> +
>> + if (state != priv->can.state) {
>> + tx_state = txerr >= rxerr ? state : 0;
>> + rx_state = txerr <= rxerr ? state : 0;
>> +
>> + can_change_state(dev, cf, tx_state, rx_state);
>> + if (state == CAN_STATE_BUS_OFF)
>> + can_bus_off(dev);
>> + }
>> +
>> + stats->rx_packets++;
>> + stats->rx_bytes += cf->can_dlc;
>> + netif_rx(skb);
>> +
>> + return 0;
>> +}
>> +
>> +static irqreturn_t sunxi_can_interrupt(int irq, void *dev_id)
>> +{
>> + struct net_device *dev = (struct net_device *)dev_id;
>> + struct sunxican_priv *priv = netdev_priv(dev);
>> + struct net_device_stats *stats = &dev->stats;
>> + u8 isrc, status;
>> + int n = 0;
>> +
>> + while ((isrc = readl(priv->base + SUNXI_REG_INT_ADDR)) &&
>> + (n < SUNXI_CAN_MAX_IRQ)) {
>> + n++;
>> + status = readl(priv->base + SUNXI_REG_STA_ADDR);
>> +
>> + if (isrc & SUNXI_INT_WAKEUP)
>> + netdev_warn(dev, "wakeup interrupt\n");
>> +
>> + if (isrc & SUNXI_INT_TBUF_VLD) {
>> + /* transmission complete interrupt */
>> + stats->tx_bytes +=
>> + readl(priv->base +
>> + SUNXI_REG_RBUF_RBACK_START_ADDR) & 0xf;
>> + stats->tx_packets++;
>> + can_get_echo_skb(dev, 0);
>> + netif_wake_queue(dev);
>> + can_led_event(dev, CAN_LED_EVENT_TX);
>> + }
>> + if (isrc & SUNXI_INT_RBUF_VLD) {
>> + /* receive interrupt */
>> + while (status & SUNXI_STA_RBUF_RDY) {
>> + /* RX buffer is not empty */
>> + sunxi_can_rx(dev);
>> + status = readl(priv->base + SUNXI_REG_STA_ADDR);
>> + }
>> + }
>> + if (isrc &
>> + (SUNXI_INT_DATA_OR | SUNXI_INT_ERR_WRN | SUNXI_INT_BUS_ERR |
>> + SUNXI_INT_ERR_PASSIVE | SUNXI_INT_ARB_LOST)) {
>> + /* error interrupt */
>> + if (sunxi_can_err(dev, isrc, status))
>> + break;
>> + }
>> + /* clear interrupts */
>> + writel(isrc, priv->base + SUNXI_REG_INT_ADDR);
>> + readl(priv->base + SUNXI_REG_INT_ADDR);
>> + }
>
> Why not simply handle whatever you have in the status register, and
> let the interrupt retrigger if there's something new?
>
Performance(less latency) as Marc mentioned.
>> + if (n >= SUNXI_CAN_MAX_IRQ)
>> + netdev_dbg(dev, "%d messages handled in ISR", n);
>> +
>> + return (n) ? IRQ_HANDLED : IRQ_NONE;
>> +}
>> +
>> +static int sunxican_open(struct net_device *dev)
>> +{
>> + int err;
>> +
>> + /* common open */
>> + err = open_candev(dev);
>> + if (err)
>> + return err;
>> +
>> + /* register interrupt handler */
>> + err = request_irq(dev->irq, sunxi_can_interrupt, IRQF_SHARED,
>> + dev->name, dev);
>> + if (err) {
>> + netdev_err(dev, "request_irq err: %d\n", err);
>> + close_candev(dev);
>> + return -EAGAIN;
>> + }
>> +
>> + sunxi_can_start(dev);
>> +
>> + can_led_event(dev, CAN_LED_EVENT_OPEN);
>> +
>> + netif_start_queue(dev);
>> +
>> + return 0;
>> +}
>> +
>> +static int sunxican_close(struct net_device *dev)
>> +{
>> + struct sunxican_priv *priv = netdev_priv(dev);
>> +
>> + netif_stop_queue(dev);
>> + set_reset_mode(dev);
>> + clk_disable_unprepare(priv->clk);
>
> Are you sure you have the right use count on that clock? You're
> calling sunxi_can_start from several places, but you call
> clk_disable_unprepare only from here.
I will check this (again).
>
> Even if it does start, it is really confusing. Please move the
> clk_prepare_enable in the open function.
>
That was my first approach. IMHO it's useful to enable/disable
according to the state of the CAN interface to save power.
>> +
>> + free_irq(dev->irq, dev);
>> + close_candev(dev);
>> + can_led_event(dev, CAN_LED_EVENT_STOP);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct net_device_ops sunxican_netdev_ops = {
>> + .ndo_open = sunxican_open,
>> + .ndo_stop = sunxican_close,
>> + .ndo_start_xmit = sunxican_start_xmit,
>> +};
>> +
>> +static const struct of_device_id sunxican_of_match[] = {
>> + {.compatible = "allwinner,sunxican"},
>> + {},
>> +};
>> +
>> +MODULE_DEVICE_TABLE(of, sunxican_of_match);
>> +
>> +static int sunxican_remove(struct platform_device *pdev)
>> +{
>> + struct net_device *dev = platform_get_drvdata(pdev);
>> +
>> + unregister_netdev(dev);
>> + free_candev(dev);
>> +
>> + return 0;
>> +}
>> +
>> +static int sunxican_probe(struct platform_device *pdev)
>> +{
>> + struct resource *mem;
>> + struct clk *clk;
>> + void __iomem *addr;
>> + int err, irq;
>> + struct net_device *dev;
>> + struct sunxican_priv *priv;
>> +
>> + clk = devm_clk_get(&pdev->dev, "apb1_can");
>
> If you pass NULL, it will automatically grab the first clock in the
> DT, which is what you want here, otherwise the clocks property is
> useless.
Hmm - I will check this.
>
>> + if (IS_ERR(clk)) {
>> + dev_err(&pdev->dev, "no clock defined\n");
>> + err = -ENODEV;
>> + goto exit;
>> + }
>> +
>> + irq = platform_get_irq(pdev, 0);
>> + if (irq < 0) {
>> + dev_err(&pdev->dev, "could not get a valid irq\n");
>> + err = -ENODEV;
>> + goto exit;
>> + }
>> +
>> + mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + addr = devm_ioremap_resource(&pdev->dev, mem);
>> + if (IS_ERR(addr)) {
>> + err = -EBUSY;
>> + goto exit;
>> + }
>> +
>> + dev = alloc_candev(sizeof(struct sunxican_priv), 1);
>> + if (!dev) {
>> + dev_err(&pdev->dev,
>> + "could not allocate memory for CAN device\n");
>> + err = -ENOMEM;
>> + goto exit;
>> + }
>> +
>> + dev->netdev_ops = &sunxican_netdev_ops;
>> + dev->irq = irq;
>> + dev->flags |= IFF_ECHO;
>> +
>> + priv = netdev_priv(dev);
>> + priv->can.clock.freq = clk_get_rate(clk);
>> + priv->can.bittiming_const = &sunxican_bittiming_const;
>> + priv->can.do_set_mode = sunxican_set_mode;
>> + priv->can.do_get_berr_counter = sunxican_get_berr_counter;
>> + priv->can.ctrlmode_supported = CAN_CTRLMODE_BERR_REPORTING |
>> + CAN_CTRLMODE_LISTENONLY |
>> + CAN_CTRLMODE_LOOPBACK |
>> + CAN_CTRLMODE_3_SAMPLES;
>> + priv->base = addr;
>> + priv->clk = clk;
>> + spin_lock_init(&priv->cmdreg_lock);
>> +
>> + platform_set_drvdata(pdev, dev);
>> + SET_NETDEV_DEV(dev, &pdev->dev);
>> +
>> + err = register_candev(dev);
>> + if (err) {
>> + dev_err(&pdev->dev, "registering %s failed (err=%d)\n",
>> + DRV_NAME, err);
>> + goto exit_free;
>> + }
>> + devm_can_led_init(dev);
>> +
>> + dev_info(&pdev->dev, "device registered (base=%p, irq=%d)\n",
>> + priv->base, dev->irq);
>> +
>> + return 0;
>> +
>> +exit_free:
>> + free_candev(dev);
>> +exit:
>> + return err;
>> +}
>> +
>> +static int __maybe_unused sunxi_can_suspend(struct device *device)
>> +{
>> + struct net_device *dev = dev_get_drvdata(device);
>> + struct sunxican_priv *priv = netdev_priv(dev);
>> + u32 mode;
>> + int err;
>> +
>> + if (netif_running(dev)) {
>> + netif_stop_queue(dev);
>> + netif_device_detach(dev);
>> + }
>> +
>> + err = set_reset_mode(dev);
>> + if (err)
>> + return err;
>> +
>> + mode = readl(priv->base + SUNXI_REG_MSEL_ADDR);
>> + writel(mode | SUNXI_MSEL_SLEEP_MODE, priv->base +
>> SUNXI_REG_MSEL_ADDR);
>> +
>> + priv->can.state = CAN_STATE_SLEEPING;
>> +
>> + clk_disable(priv->clk);
>> + return 0;
>> +}
>> +
>> +static int __maybe_unused sunxi_can_resume(struct device *device)
>> +{
>> + struct net_device *dev = dev_get_drvdata(device);
>> + struct sunxican_priv *priv = netdev_priv(dev);
>> + u32 mode;
>> + int err;
>> +
>> + err = clk_enable(priv->clk);
>> + if (err) {
>> + netdev_err(dev, "clk_enable() failed, error %d\n", err);
>> + return err;
>> + }
>> +
>> + err = set_reset_mode(dev);
>> + if (err)
>> + return err;
>> +
>> + mode = readl(priv->base + SUNXI_REG_MSEL_ADDR);
>> + writel(mode & ~(SUNXI_MSEL_SLEEP_MODE),
>> + priv->base + SUNXI_REG_MSEL_ADDR);
>> + err = set_normal_mode(dev);
>> + if (err)
>> + return err;
>> +
>> + priv->can.state = CAN_STATE_ERROR_ACTIVE;
>> + if (netif_running(dev)) {
>> + netif_device_attach(dev);
>> + netif_start_queue(dev);
>> + }
>> + return 0;
>> +}
>> +
>> +static SIMPLE_DEV_PM_OPS(sunxi_can_pm_ops, sunxi_can_suspend,
>> sunxi_can_resume);
>
> How did you test those hooks? There's no suspend support yet.
>
I didn't test them.
>> +static struct platform_driver sunxi_can_driver = {
>> + .driver = {
>> + .name = DRV_NAME,
>> + .pm = &sunxi_can_pm_ops,
>> + .of_match_table = sunxican_of_match,
>> + },
>> + .probe = sunxican_probe,
>> + .remove = sunxican_remove,
>> +};
>> +
>> +module_platform_driver(sunxi_can_driver);
>> +
>> +MODULE_AUTHOR("Peter Chen <xingkongcp-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>");
>> +MODULE_AUTHOR("Gerhard Bertelsmann <info-La43T0Mi4bH5xCKuJOYmCvaTkwRoYoCU@public.gmane.org>");
>> +MODULE_LICENSE("Dual BSD/GPL");
>> +MODULE_DESCRIPTION(DRV_NAME "CAN driver for Allwinner SoCs
>> (A10/A20)");
>> +MODULE_VERSION(DRV_MODULE_VERSION);
>
> Is this going to be useful?
>
> The module version is already redundant with the kernel version, and
> there's a good chance it will never reach 1.0.
Will remove the version number.
General remark:
Somebody asked me to add linux-sunxi dev list. Seems to be not a good
idea
before the driver was ready. Seems to be hard to fulfill the
differentness.
I will remove linux-sunxi for the next patch set. I will try to get
linux-can
people satisfied and then I'm done with it. I'm doing this in my spare
time,
but it seems to get an endless story. I'm not offended in any kind, my
spare
time is just limited.
The driver is working so far and the patches also apply to 4.3.
>
> Thanks!
> Maxime
Greetings
Gerd
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [linux-sunxi] [PATCH v5 2/2] can: Allwinner A10/A20 CAN Controller support - controller code
2015-09-13 13:42 ` Gerhard Bertelsmann
@ 2015-09-13 13:49 ` Marc Kleine-Budde
[not found] ` <f8eea0f544eb6367142bbc1c2ab35964-6XUBc1GShDsOIzVOb1FTxg@public.gmane.org>
1 sibling, 0 replies; 15+ messages in thread
From: Marc Kleine-Budde @ 2015-09-13 13:49 UTC (permalink / raw)
To: Gerhard Bertelsmann, Maxime Ripard
Cc: linux-can, linux-sunxi, linux-can-owner
[-- Attachment #1: Type: text/plain, Size: 737 bytes --]
On 09/13/2015 03:42 PM, Gerhard Bertelsmann wrote:
>> > Even if it does start, it is really confusing. Please move the
>> > clk_prepare_enable in the open function.
>> >
> That was my first approach. IMHO it's useful to enable/disable
> according to the state of the CAN interface to save power.
The open() function is perfectly right, enabling the clock in the probe
function keeps the clock powered even if the CAN controller is not used.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v5 2/2] can: Allwinner A10/A20 CAN Controller support - controller code
[not found] ` <f8eea0f544eb6367142bbc1c2ab35964-6XUBc1GShDsOIzVOb1FTxg@public.gmane.org>
@ 2015-09-13 23:51 ` Julian Calaby
2015-09-14 8:17 ` Maxime Ripard
1 sibling, 0 replies; 15+ messages in thread
From: Julian Calaby @ 2015-09-13 23:51 UTC (permalink / raw)
To: info-La43T0Mi4bH5xCKuJOYmCvaTkwRoYoCU
Cc: Maxime Ripard, linux-can-u79uwXL29TY76Z2rM5mHXA, linux-sunxi,
mkl-bIcnvbaLZ9MEGnE8C9+IrQ,
linux-can-owner-u79uwXL29TY76Z2rM5mHXA
Hi Gerd,
On Sun, Sep 13, 2015 at 11:42 PM, Gerhard Bertelsmann
<info-La43T0Mi4bH5xCKuJOYmCvaTkwRoYoCU@public.gmane.org> wrote:
> General remark:
>
> Somebody asked me to add linux-sunxi dev list. Seems to be not a good idea
> before the driver was ready. Seems to be hard to fulfill the differentness.
>
> I will remove linux-sunxi for the next patch set. I will try to get
> linux-can
> people satisfied and then I'm done with it. I'm doing this in my spare time,
> but it seems to get an endless story. I'm not offended in any kind, my spare
> time is just limited.
>
> The driver is working so far and the patches also apply to 4.3.
You misunderstand, the sunxi people (who's mailing list I follow) are
the experts on the hardware platform you're writing this driver for.
If you don't include them and don't solicit their advice, your driver
may end up doing things which aren't correct for the platform (e.g. it
might work for you but break something else) and this will seriously
impact it's chances of getting included in the kernel at some point.
You also can't get the driver "ready" before asking for their comments
as what might have been small tweaks to get stuff "right" at the start
may end up being big changes at the end. (Stitch in time and all
that.)
Including the linux-sunxi mailing list also means that there's more
visibility to your efforts, more people looking over the code,
(potentially) more testers and less duplicated effort as all sunxi
related stuff is referred to in one place. (Thankfully in your case
nobody else was working on a CAN bus driver, but there have been cases
of multiple people independently working on drivers for the same
hardware and only discovering this late in the development process.)
As an occasional Linux developer, I don't see Maxime's questions as
being "hard": he makes some good points about your clock handling -
which is important for power management - and asks a couple of other
good questions. Those questions and issues are stuff that has to be
answered to get this driver into the kernel. In particular, the
question about the interrupt handling was an "explain why you're doing
this" question not a "this is broken and needs changing" question. And
as for the changing things only to change them back issue, this is
likely to be the last time that happens.
My reading of his comments indicates that you should check over what
and how stuff is handled over the "lifetime" of the device /
connection / socket. I.e. are all your resources "freed" after they're
no-longer needed? Are you switching off everything you turn on? etc.
This is the sort of analysis that I'm sure you've done already, but it
never hurts to do it again. I find issues like this all the time in
the development I do.
Finally, I strongly urge you to stick with the driver at least until
it's merged. The fact that Maxime only had a few comments indicates
that you've written a good driver which is a few tweaks away from
being merged - which is something you should be very proud of. I see
drivers being submitted that have much more serious issues and that
prompt much more debate than yours has. (For instance I've seen some
changes on another mailing list go through 6 full rewrites without
everyone being happy.)
Excellent work, keep it up.
Thanks,
--
Julian Calaby
Email: julian.calaby-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
Profile: http://www.google.com/profiles/julian.calaby/
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v5 2/2] can: Allwinner A10/A20 CAN Controller support - controller code
[not found] ` <f8eea0f544eb6367142bbc1c2ab35964-6XUBc1GShDsOIzVOb1FTxg@public.gmane.org>
2015-09-13 23:51 ` Julian Calaby
@ 2015-09-14 8:17 ` Maxime Ripard
2015-09-14 12:41 ` Gerhard Bertelsmann
1 sibling, 1 reply; 15+ messages in thread
From: Maxime Ripard @ 2015-09-14 8:17 UTC (permalink / raw)
To: Gerhard Bertelsmann
Cc: linux-can-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, mkl-bIcnvbaLZ9MEGnE8C9+IrQ,
linux-can-owner-u79uwXL29TY76Z2rM5mHXA
[-- Attachment #1: Type: text/plain, Size: 7397 bytes --]
Hi,
On Sun, Sep 13, 2015 at 03:42:41PM +0200, Gerhard Bertelsmann wrote:
> >> drivers/net/can/Kconfig | 10 +
> >> drivers/net/can/Makefile | 1 +
> >> drivers/net/can/sunxi_can.c | 879
> >>+++++++++++++++++++++
> >> 3 files changed, 890 insertions(+)
> >>
> >>diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
> >>index e8c96b8..439f67c 100644
> >>--- a/drivers/net/can/Kconfig
> >>+++ b/drivers/net/can/Kconfig
> >>@@ -129,6 +129,16 @@ config CAN_RCAR
> >> To compile this driver as a module, choose M here: the module will
> >> be called rcar_can.
> >>
> >>+config CAN_SUNXI
> >>+ tristate "Allwinner SUNXI CAN controller"
> >
> >It would be better if we could mention that it's a driver for the
> >sun4i family and derived.
>
> I'm using it on Bpi with A20. AFAIK a SUN7I. Anyway I'm not aware
> of all Allwinner SoCs. I will change it to any text you want.
CAN_SUN4I and Allwinner A10 CAN controller will do just fine.
Now that I'm thinking of it, can you also enable this driver in the
sunxi and multi_v7 defconfigs please (in separate patches)?
> >We have no guarantee that there won't be any IP on other sunxi SoCs
> >incompatible with this one that would need a new driver.
>
> Right. IMHO A10 and A20 are working - maybe Fxx(?).
I was more thinking of the A31, A23, A80 and alike, but yeah, it might
be true for the F20 too.
> >>+static int sunxican_open(struct net_device *dev)
> >>+{
> >>+ int err;
> >>+
> >>+ /* common open */
> >>+ err = open_candev(dev);
> >>+ if (err)
> >>+ return err;
> >>+
> >>+ /* register interrupt handler */
> >>+ err = request_irq(dev->irq, sunxi_can_interrupt, IRQF_SHARED,
> >>+ dev->name, dev);
> >>+ if (err) {
> >>+ netdev_err(dev, "request_irq err: %d\n", err);
> >>+ close_candev(dev);
> >>+ return -EAGAIN;
> >>+ }
> >>+
> >>+ sunxi_can_start(dev);
> >>+
> >>+ can_led_event(dev, CAN_LED_EVENT_OPEN);
> >>+
> >>+ netif_start_queue(dev);
> >>+
> >>+ return 0;
> >>+}
> >>+
> >>+static int sunxican_close(struct net_device *dev)
> >>+{
> >>+ struct sunxican_priv *priv = netdev_priv(dev);
> >>+
> >>+ netif_stop_queue(dev);
> >>+ set_reset_mode(dev);
> >>+ clk_disable_unprepare(priv->clk);
> >
> >Are you sure you have the right use count on that clock? You're
> >calling sunxi_can_start from several places, but you call
> >clk_disable_unprepare only from here.
>
> I will check this (again).
>
> >Even if it does start, it is really confusing. Please move the
> >clk_prepare_enable in the open function.
>
> That was my first approach. IMHO it's useful to enable/disable
> according to the state of the CAN interface to save power.
Yes, it makes sense to have the clock enabled/disabled in open/close.
I was just saying that these calls should be balanced, and having the
clk_prepare_enable in the sunxi_can_start doesn't help that.
> >>+static int __maybe_unused sunxi_can_suspend(struct device *device)
> >>+{
> >>+ struct net_device *dev = dev_get_drvdata(device);
> >>+ struct sunxican_priv *priv = netdev_priv(dev);
> >>+ u32 mode;
> >>+ int err;
> >>+
> >>+ if (netif_running(dev)) {
> >>+ netif_stop_queue(dev);
> >>+ netif_device_detach(dev);
> >>+ }
> >>+
> >>+ err = set_reset_mode(dev);
> >>+ if (err)
> >>+ return err;
> >>+
> >>+ mode = readl(priv->base + SUNXI_REG_MSEL_ADDR);
> >>+ writel(mode | SUNXI_MSEL_SLEEP_MODE, priv->base +
> >>SUNXI_REG_MSEL_ADDR);
> >>+
> >>+ priv->can.state = CAN_STATE_SLEEPING;
> >>+
> >>+ clk_disable(priv->clk);
> >>+ return 0;
> >>+}
> >>+
> >>+static int __maybe_unused sunxi_can_resume(struct device *device)
> >>+{
> >>+ struct net_device *dev = dev_get_drvdata(device);
> >>+ struct sunxican_priv *priv = netdev_priv(dev);
> >>+ u32 mode;
> >>+ int err;
> >>+
> >>+ err = clk_enable(priv->clk);
> >>+ if (err) {
> >>+ netdev_err(dev, "clk_enable() failed, error %d\n", err);
> >>+ return err;
> >>+ }
> >>+
> >>+ err = set_reset_mode(dev);
> >>+ if (err)
> >>+ return err;
> >>+
> >>+ mode = readl(priv->base + SUNXI_REG_MSEL_ADDR);
> >>+ writel(mode & ~(SUNXI_MSEL_SLEEP_MODE),
> >>+ priv->base + SUNXI_REG_MSEL_ADDR);
> >>+ err = set_normal_mode(dev);
> >>+ if (err)
> >>+ return err;
> >>+
> >>+ priv->can.state = CAN_STATE_ERROR_ACTIVE;
> >>+ if (netif_running(dev)) {
> >>+ netif_device_attach(dev);
> >>+ netif_start_queue(dev);
> >>+ }
> >>+ return 0;
> >>+}
> >>+
> >>+static SIMPLE_DEV_PM_OPS(sunxi_can_pm_ops, sunxi_can_suspend,
> >>sunxi_can_resume);
> >
> >How did you test those hooks? There's no suspend support yet.
>
> I didn't test them.
Then remove them.
> >>+static struct platform_driver sunxi_can_driver = {
> >>+ .driver = {
> >>+ .name = DRV_NAME,
> >>+ .pm = &sunxi_can_pm_ops,
> >>+ .of_match_table = sunxican_of_match,
> >>+ },
> >>+ .probe = sunxican_probe,
> >>+ .remove = sunxican_remove,
> >>+};
> >>+
> >>+module_platform_driver(sunxi_can_driver);
> >>+
> >>+MODULE_AUTHOR("Peter Chen <xingkongcp-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>");
> >>+MODULE_AUTHOR("Gerhard Bertelsmann <info-La43T0Mi4bH5xCKuJOYmCvaTkwRoYoCU@public.gmane.org>");
> >>+MODULE_LICENSE("Dual BSD/GPL");
> >>+MODULE_DESCRIPTION(DRV_NAME "CAN driver for Allwinner SoCs (A10/A20)");
> >>+MODULE_VERSION(DRV_MODULE_VERSION);
> >
> >Is this going to be useful?
> >
> >The module version is already redundant with the kernel version, and
> >there's a good chance it will never reach 1.0.
>
> Will remove the version number.
>
> General remark:
>
> Somebody asked me to add linux-sunxi dev list. Seems to be not a
> good idea before the driver was ready. Seems to be hard to fulfill
> the differentness.
>
> I will remove linux-sunxi for the next patch set. I will try to get
> linux-can people satisfied and then I'm done with it. I'm doing this
> in my spare time, but it seems to get an endless story. I'm not
> offended in any kind, my spare time is just limited.
>
> The driver is working so far and the patches also apply to 4.3.
Cc'ing linux-sunxi is great, but it's an orthogonal issue.
Whenever you post a patch, you're supposed to Cc all the maintainers
listed for the MAINTAINERS file, and the get_maintainers.pl script
helps you with that.
If you use that script on your driver, you'll get:
./scripts/get_maintainer.pl -f drivers/net/can/sunxi_can.c
Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org> (maintainer:CAN NETWORK DRIVERS)
Marc Kleine-Budde <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> (maintainer:CAN NETWORK DRIVERS)
Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> (maintainer:ARM/Allwinner A1X SoC support)
linux-can-u79uwXL29TY76Z2rM5mHXA@public.gmane.org (open list:CAN NETWORK DRIVERS)
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org (open list:NETWORKING DRIVERS)
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org (moderated list:ARM/Allwinner A1X SoC support)
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org (open list)
All these people are supposed to be reviewing your driver, and while
Marc will have a look at how your driver interact with the CAN
framework, I'll have a look at how your driver interact with the rest
of the code we did for the Allwinner SoCs. We look at different
aspects, we have different kind of comments.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v5 2/2] can: Allwinner A10/A20 CAN Controller support - controller code
2015-09-14 8:17 ` Maxime Ripard
@ 2015-09-14 12:41 ` Gerhard Bertelsmann
2015-09-14 12:45 ` [linux-sunxi] " Marc Kleine-Budde
0 siblings, 1 reply; 15+ messages in thread
From: Gerhard Bertelsmann @ 2015-09-14 12:41 UTC (permalink / raw)
To: Maxime Ripard
Cc: linux-can-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, mkl-bIcnvbaLZ9MEGnE8C9+IrQ,
linux-can-owner-u79uwXL29TY76Z2rM5mHXA
Hi Maxime,
thanks for your response.
Am 2015-09-14 10:17, schrieb Maxime Ripard:
> Hi,
>
> On Sun, Sep 13, 2015 at 03:42:41PM +0200, Gerhard Bertelsmann wrote:
>> >> drivers/net/can/Kconfig | 10 +
>> >> drivers/net/can/Makefile | 1 +
>> >> drivers/net/can/sunxi_can.c | 879
>> >>+++++++++++++++++++++
>> >> 3 files changed, 890 insertions(+)
>> >>
>> >>diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
>> >>index e8c96b8..439f67c 100644
>> >>--- a/drivers/net/can/Kconfig
>> >>+++ b/drivers/net/can/Kconfig
>> >>@@ -129,6 +129,16 @@ config CAN_RCAR
>> >> To compile this driver as a module, choose M here: the module will
>> >> be called rcar_can.
>> >>
>> >>+config CAN_SUNXI
>> >>+ tristate "Allwinner SUNXI CAN controller"
>> >
>> >It would be better if we could mention that it's a driver for the
>> >sun4i family and derived.
>>
>> I'm using it on Bpi with A20. AFAIK a SUN7I. Anyway I'm not aware
>> of all Allwinner SoCs. I will change it to any text you want.
>
> CAN_SUN4I and Allwinner A10 CAN controller will do just fine.
>
> Now that I'm thinking of it, can you also enable this driver in the
> sunxi and multi_v7 defconfigs please (in separate patches)?
>
>> >We have no guarantee that there won't be any IP on other sunxi SoCs
>> >incompatible with this one that would need a new driver.
>>
>> Right. IMHO A10 and A20 are working - maybe Fxx(?).
>
> I was more thinking of the A31, A23, A80 and alike, but yeah, it might
> be true for the F20 too.
>
>> >>+static int sunxican_open(struct net_device *dev)
>> >>+{
>> >>+ int err;
>> >>+
>> >>+ /* common open */
>> >>+ err = open_candev(dev);
>> >>+ if (err)
>> >>+ return err;
>> >>+
>> >>+ /* register interrupt handler */
>> >>+ err = request_irq(dev->irq, sunxi_can_interrupt, IRQF_SHARED,
>> >>+ dev->name, dev);
>> >>+ if (err) {
>> >>+ netdev_err(dev, "request_irq err: %d\n", err);
>> >>+ close_candev(dev);
>> >>+ return -EAGAIN;
>> >>+ }
>> >>+
>> >>+ sunxi_can_start(dev);
>> >>+
>> >>+ can_led_event(dev, CAN_LED_EVENT_OPEN);
>> >>+
>> >>+ netif_start_queue(dev);
>> >>+
>> >>+ return 0;
>> >>+}
>> >>+
>> >>+static int sunxican_close(struct net_device *dev)
>> >>+{
>> >>+ struct sunxican_priv *priv = netdev_priv(dev);
>> >>+
>> >>+ netif_stop_queue(dev);
>> >>+ set_reset_mode(dev);
>> >>+ clk_disable_unprepare(priv->clk);
>> >
>> >Are you sure you have the right use count on that clock? You're
>> >calling sunxi_can_start from several places, but you call
>> >clk_disable_unprepare only from here.
>>
>> I will check this (again).
>>
>> >Even if it does start, it is really confusing. Please move the
>> >clk_prepare_enable in the open function.
>>
>> That was my first approach. IMHO it's useful to enable/disable
>> according to the state of the CAN interface to save power.
>
> Yes, it makes sense to have the clock enabled/disabled in open/close.
>
> I was just saying that these calls should be balanced, and having the
> clk_prepare_enable in the sunxi_can_start doesn't help that.
>
>> >>+static int __maybe_unused sunxi_can_suspend(struct device *device)
>> >>+{
>> >>+ struct net_device *dev = dev_get_drvdata(device);
>> >>+ struct sunxican_priv *priv = netdev_priv(dev);
>> >>+ u32 mode;
>> >>+ int err;
>> >>+
>> >>+ if (netif_running(dev)) {
>> >>+ netif_stop_queue(dev);
>> >>+ netif_device_detach(dev);
>> >>+ }
>> >>+
>> >>+ err = set_reset_mode(dev);
>> >>+ if (err)
>> >>+ return err;
>> >>+
>> >>+ mode = readl(priv->base + SUNXI_REG_MSEL_ADDR);
>> >>+ writel(mode | SUNXI_MSEL_SLEEP_MODE, priv->base +
>> >>SUNXI_REG_MSEL_ADDR);
>> >>+
>> >>+ priv->can.state = CAN_STATE_SLEEPING;
>> >>+
>> >>+ clk_disable(priv->clk);
>> >>+ return 0;
>> >>+}
>> >>+
>> >>+static int __maybe_unused sunxi_can_resume(struct device *device)
>> >>+{
>> >>+ struct net_device *dev = dev_get_drvdata(device);
>> >>+ struct sunxican_priv *priv = netdev_priv(dev);
>> >>+ u32 mode;
>> >>+ int err;
>> >>+
>> >>+ err = clk_enable(priv->clk);
>> >>+ if (err) {
>> >>+ netdev_err(dev, "clk_enable() failed, error %d\n", err);
>> >>+ return err;
>> >>+ }
>> >>+
>> >>+ err = set_reset_mode(dev);
>> >>+ if (err)
>> >>+ return err;
>> >>+
>> >>+ mode = readl(priv->base + SUNXI_REG_MSEL_ADDR);
>> >>+ writel(mode & ~(SUNXI_MSEL_SLEEP_MODE),
>> >>+ priv->base + SUNXI_REG_MSEL_ADDR);
>> >>+ err = set_normal_mode(dev);
>> >>+ if (err)
>> >>+ return err;
>> >>+
>> >>+ priv->can.state = CAN_STATE_ERROR_ACTIVE;
>> >>+ if (netif_running(dev)) {
>> >>+ netif_device_attach(dev);
>> >>+ netif_start_queue(dev);
>> >>+ }
>> >>+ return 0;
>> >>+}
>> >>+
>> >>+static SIMPLE_DEV_PM_OPS(sunxi_can_pm_ops, sunxi_can_suspend,
>> >>sunxi_can_resume);
>> >
>> >How did you test those hooks? There's no suspend support yet.
>>
>> I didn't test them.
>
> Then remove them.
>
>> >>+static struct platform_driver sunxi_can_driver = {
>> >>+ .driver = {
>> >>+ .name = DRV_NAME,
>> >>+ .pm = &sunxi_can_pm_ops,
>> >>+ .of_match_table = sunxican_of_match,
>> >>+ },
>> >>+ .probe = sunxican_probe,
>> >>+ .remove = sunxican_remove,
>> >>+};
>> >>+
>> >>+module_platform_driver(sunxi_can_driver);
>> >>+
>> >>+MODULE_AUTHOR("Peter Chen <xingkongcp-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>");
>> >>+MODULE_AUTHOR("Gerhard Bertelsmann <info-La43T0Mi4bH5xCKuJOYmCvaTkwRoYoCU@public.gmane.org>");
>> >>+MODULE_LICENSE("Dual BSD/GPL");
>> >>+MODULE_DESCRIPTION(DRV_NAME "CAN driver for Allwinner SoCs (A10/A20)");
>> >>+MODULE_VERSION(DRV_MODULE_VERSION);
>> >
>> >Is this going to be useful?
>> >
>> >The module version is already redundant with the kernel version, and
>> >there's a good chance it will never reach 1.0.
>>
>> Will remove the version number.
>>
>> General remark:
>>
>> Somebody asked me to add linux-sunxi dev list. Seems to be not a
>> good idea before the driver was ready. Seems to be hard to fulfill
>> the differentness.
>>
>> I will remove linux-sunxi for the next patch set. I will try to get
>> linux-can people satisfied and then I'm done with it. I'm doing this
>> in my spare time, but it seems to get an endless story. I'm not
>> offended in any kind, my spare time is just limited.
>>
>> The driver is working so far and the patches also apply to 4.3.
>
> Cc'ing linux-sunxi is great, but it's an orthogonal issue.
>
> Whenever you post a patch, you're supposed to Cc all the maintainers
> listed for the MAINTAINERS file, and the get_maintainers.pl script
> helps you with that.
>
> If you use that script on your driver, you'll get:
>
> ./scripts/get_maintainer.pl -f drivers/net/can/sunxi_can.c
> Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org> (maintainer:CAN NETWORK
> DRIVERS)
> Marc Kleine-Budde <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> (maintainer:CAN NETWORK DRIVERS)
> Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> (maintainer:ARM/Allwinner A1X SoC support)
> linux-can-u79uwXL29TY76Z2rM5mHXA@public.gmane.org (open list:CAN NETWORK DRIVERS)
> netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org (open list:NETWORKING DRIVERS)
> linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org (moderated list:ARM/Allwinner A1X
> SoC support)
> linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org (open list)
>
> All these people are supposed to be reviewing your driver, and while
> Marc will have a look at how your driver interact with the CAN
> framework, I'll have a look at how your driver interact with the rest
> of the code we did for the Allwinner SoCs. We look at different
> aspects, we have different kind of comments.
>
> Maxime
I think I got it.
IMHO Marc did a very good job reviewing the code so far. If the
SocketCAN
is happy with the code I will send the patch to the maintainer if Marc
agree.
Regards
Gerd
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [linux-sunxi] [PATCH v5 2/2] can: Allwinner A10/A20 CAN Controller support - controller code
2015-09-14 12:41 ` Gerhard Bertelsmann
@ 2015-09-14 12:45 ` Marc Kleine-Budde
0 siblings, 0 replies; 15+ messages in thread
From: Marc Kleine-Budde @ 2015-09-14 12:45 UTC (permalink / raw)
To: Gerhard Bertelsmann, Maxime Ripard
Cc: linux-can, linux-sunxi, linux-can-owner
[-- Attachment #1: Type: text/plain, Size: 1001 bytes --]
On 09/14/2015 02:41 PM, Gerhard Bertelsmann wrote:
>> All these people are supposed to be reviewing your driver, and while
>> Marc will have a look at how your driver interact with the CAN
>> framework, I'll have a look at how your driver interact with the rest
>> of the code we did for the Allwinner SoCs. We look at different
>> aspects, we have different kind of comments.
>>
>> Maxime
>
> I think I got it.
>
> IMHO Marc did a very good job reviewing the code so far. If the
> SocketCAN
> is happy with the code I will send the patch to the maintainer if Marc
> agree.
I think for the following patch iterations you can send to all parties
that show up in the get_maintainer.pl's output.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2015-09-14 12:45 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-13 11:43 [PATCH v5 0/2] can: Allwinner A10/A20 CAN Controller support - summary Gerhard Bertelsmann
2015-09-13 11:43 ` [PATCH v5 1/2] can: Allwinner A10/A20 CAN Controller support - Devicetree bindings Gerhard Bertelsmann
[not found] ` <1442144632-4541-2-git-send-email-info-La43T0Mi4bH5xCKuJOYmCvaTkwRoYoCU@public.gmane.org>
2015-09-13 12:25 ` Maxime Ripard
2015-09-13 11:43 ` [PATCH v5 2/2] can: Allwinner A10/A20 CAN Controller support - controller code Gerhard Bertelsmann
[not found] ` <1442144632-4541-3-git-send-email-info-La43T0Mi4bH5xCKuJOYmCvaTkwRoYoCU@public.gmane.org>
2015-09-13 12:45 ` Maxime Ripard
2015-09-13 12:50 ` [linux-sunxi] " Marc Kleine-Budde
2015-09-13 13:42 ` Gerhard Bertelsmann
2015-09-13 13:49 ` [linux-sunxi] " Marc Kleine-Budde
[not found] ` <f8eea0f544eb6367142bbc1c2ab35964-6XUBc1GShDsOIzVOb1FTxg@public.gmane.org>
2015-09-13 23:51 ` Julian Calaby
2015-09-14 8:17 ` Maxime Ripard
2015-09-14 12:41 ` Gerhard Bertelsmann
2015-09-14 12:45 ` [linux-sunxi] " Marc Kleine-Budde
[not found] ` <1442144632-4541-1-git-send-email-info-La43T0Mi4bH5xCKuJOYmCvaTkwRoYoCU@public.gmane.org>
2015-09-13 12:22 ` [PATCH v5 0/2] can: Allwinner A10/A20 CAN Controller support - summary Maxime Ripard
2015-09-13 12:57 ` [linux-sunxi] " Marc Kleine-Budde
2015-09-13 13:42 ` Maxime Ripard
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.