Skip to contentSkip to conversation

CAN controller support for Allwinner A10/A20 SoCs

PATCH Superseded Gerhard Bertelsmann 2 participants 2 messages Sep 2, 2015

Subsystems (2)

v1 of 8 Superseded Latest is v8 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 CAN controller support for Allwinner A10/A20 SoCs

This driver add support for SUNXI CAN controllers found on Allwinner A10/A20 SoCs V1

Signed-of-by: Gerhard Bertelsmann <info <at> gerhard-bertelsmann>
Tested-by: Gerhard Bertelsmann <info <at> gerhard-bertelsmann>

 drivers/net/can/Kconfig     |  10 +
 drivers/net/can/Makefile    |   1 +
 drivers/net/can/sunxi_can.c | 883 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 894 insertions(+)
drivers/net/can/Kconfig +10 -0
index e8c96b8..29b62e6 100644
@@ -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
+	---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 sunxi_can.
+
 config CAN_XILINXCAN
 	tristate "Xilinx CAN"
 	depends on ARCH_ZYNQ || ARM64 || MICROBLAZE || COMPILE_TEST
drivers/net/can/Makefile +1 -0
index c533c62..b3e825c 100644
@@ -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__
drivers/net/can/sunxi_can.c new file +883 -0

883 lines not on this page · raw message

#1 lore raw
Quoted 2 lines · Gerhard Bertelsmann

This driver add support for SUNXI CAN controllers found on Allwinner A10/A20 SoCs V1

Thanks for the patch. I suggest to use git send-email to post the patch. Please test your patch with scripts/checkpatch.pl (included in the kernel tree) before submitting - you can even use the "--file" option to check a file, before creating the patch.

Here some general remarks:

- please remove the EXPORT_SYMBOL_GPL, as this is a stand alone driver.
- why are there udelay()s in the hot path of the code?
- please move the device tree bindings into a separate file in Documentation/devicetree/bindings/net/can/
please provide that binding doc as a separate patch
- please provide a proper Signed-off-by line, with a working non obfuscated email address

Your priv struct seems a bit bloated:

Quoted 3 lines
struct sunxican_priv {
	struct can_priv can;
	struct net_device *dev;

write only variable

Quoted 1 line
	struct napi_struct napi;

not used

Quoted 2 lines
	int open_time;

read only variable

Quoted 1 line
	struct sk_buff *echo_skb;

not used

Quoted 4 lines
	void __iomem *base;
	struct clk *clk;

	void *priv;		/* for board-specific data */

I think this is never used

Quoted 2 lines

unsigned long irq_flags; /* for request_irq() */

This is a read only variable

Quoted 3 lines
	spinlock_t cmdreg_lock;	/* lock for concurrent cmd register writes */

	u16 flags;		/* custom mode flags */

not used

Quoted 1 line

};

Please fix these sparse warning:

Quoted 2 lines

drivers/net/can/sunxi_can.c:596:13: warning: symbol 'sunxi_can_interrupt' was not declared. Should it be static? drivers/net/can/sunxi_can.c:690:6: warning: symbol 'free_sunxicandev' was not declared. Should it be static?

Compile with "make C=1" to test yourself. Install the "sparse" package of your distribution first.

Marc

Signature · 4 lines
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   |