can: m_can: Create a m_can platform framework
Subsystems (5)
PATCH v4 1/4 can: m_can: Create a m_can platform framework
Create a m_can platform framework that peripherial devices can register to and use common code and register sets. The peripherial devices may provide read/write and configuration support of the IP.
--- drivers/net/can/m_can/m_can.c | 6 + drivers/net/can/m_can/m_can_platform.c | 209 +++++++++++++++++++++++++ drivers/net/can/m_can/m_can_platform.h | 163 +++++++++++++++++++ 3 files changed, 378 insertions(+) create mode 100644 drivers/net/can/m_can/m_can_platform.c create mode 100644 drivers/net/can/m_can/m_can_platform.h
drivers/net/can/m_can/m_can.c +6 -0
@@ -414,6 +414,9 @@ static inline void m_can_config_endisable(const struct m_can_priv *priv,u32 timeout =10 ;u32 val =0 ; +if (cccr &CCCR_CSR ) + cccr &= ~CCCR_CSR ; +if (enable) {/* enable m_can configuration */ m_can_write (priv,M_CAN_CCCR , cccr |CCCR_INIT );
@@ -1155,6 +1158,9 @@ static void m_can_chip_config(struct net_device *dev)m_can_set_bittiming (dev);m_can_config_endisable (priv, false); + +if (priv->device_init) + priv->device_init (priv); }static void m_can_start (struct net_device *dev)
drivers/net/can/m_can/m_can_platform.c new file +209 -0
209 lines not on this page · raw message
drivers/net/can/m_can/m_can_platform.h new file +163 -0
163 lines not on this page · raw message
Signature · 1 line
2.20.1.98.gecbdaf0899
PATCH v4 2/4 can: m_can: Migrate the m_can code to use the framework
Migrate the m_can code to use the m_can_platform framework code.
--- drivers/net/can/m_can/Kconfig | 12 + drivers/net/can/m_can/Makefile | 4 +- drivers/net/can/m_can/m_can.c | 764 ++++++++++++------------- drivers/net/can/m_can/m_can_platform.h | 2 +- 4 files changed, 374 insertions(+), 408 deletions(-)
drivers/net/can/m_can/Kconfig +12 -0
@@ -1,5 +1,17 @@ config CAN_M_CAN + tristate "Bosch M_CAN support" + ---help--- + Say Y here if you want to support for Bosch M_CAN controller. + +config CAN_M_CAN_CORE + depends on CAN_M_CAN + tristate "Bosch M_CAN Core support" + ---help--- + Say Y here if you want to support for Bosch M_CAN controller. + +config CAN_M_CAN_PLATFORM depends on HAS_IOMEM + depends on CAN_M_CAN_CORE tristate "Bosch M_CAN devices" ---help--- Say Y here if you want to support for Bosch M_CAN controller.
drivers/net/can/m_can/Makefile +3 -1
@@ -2,4 +2,6 @@ # Makefile for the Bosch M_CAN controller driver. # -obj-$(CONFIG_CAN_M_CAN) += m_can.o +obj-$(CONFIG_CAN_M_CAN_CORE) += m_can.o +obj-$(CONFIG_CAN_M_CAN_PLATFORM) += m_can_platform.o +obj-$(CONFIG_CAN_M_CAN_TCAN4X5X) += tcan4x5x.o
drivers/net/can/m_can/m_can.c +358 -406
1,170 lines not on this page · raw message
drivers/net/can/m_can/m_can_platform.h +1 -1
@@ -156,7 +156,7 @@ int m_can_core_register(struct m_can_classdev *m_can_dev);void m_can_core_unregister (struct m_can_classdev *m_can_dev);int m_can_core_get_clocks (struct m_can_classdev *m_can_dev);void m_can_init_ram (struct m_can_classdev *priv); -void m_can_config_endisable (const struct m_can_classdev *priv,bool enable); +void m_can_config_endisable (struct m_can_classdev *priv,bool enable);int m_can_core_suspend (struct device *dev);int m_can_core_resume (struct device *dev);
Signature · 1 line
2.20.1.98.gecbdaf0899
Re: [PATCH v4 2/4] can: m_can: Migrate the m_can code to use the framework
Hello,
Am 17.01.19 um 21:05 schrieb Dan Murphy:
Quoted 16 lines
Migrate the
m_cancode to use them_can_platformframework code.Signed-off-by Dan Murphy <[email protected]>--- drivers/net/can/m_can/Kconfig | 12 + drivers/net/can/m_can/Makefile | 4 +- drivers/net/can/m_can/m_can.c | 764 ++++++++++++------------- drivers/net/can/m_can/m_can_platform.h | 2 +- 4 files changed, 374 insertions(+), 408 deletions(-)drivers/net/can/m_can/Kconfig +0
-0
drivers/net/can/m_can/Kconfig line 10
Quoted 10 lines
drivers/net/can/m_can/Kconfig +9
-0@@ -1,5 +1,17 @@ config CAN_M_CAN + tristate "Bosch M_CAN support" + ---help--- + Say Y here if you want to support for Bosch M_CAN controller. + +config CAN_M_CAN_CORE + depends on CAN_M_CAN + tristate "Bosch M_CAN Core support" + ---help--- + Say Y here if you want to support for Bosch M_CAN controller.
Do you need that extra config? I think "CAN_M_CAN" is just fine.
Quoted 11 lines
drivers/net/can/m_can/Kconfig +2
-0+config CAN_M_CAN_PLATFORM depends on HAS_IOMEM + depends on CAN_M_CAN_CORE tristate "Bosch M_CAN devices" ---help--- Say Y here if you want to support for Bosch M_CAN controller.drivers/net/can/m_can/Makefile +0
-0
drivers/net/can/m_can/Makefile line 7
Quoted 7 lines
drivers/net/can/m_can/Makefile +3
-1@@ -2,4 +2,6 @@ # Makefile for the Bosch M_CAN controller driver. # -obj-$(CONFIG_CAN_M_CAN) += m_can.o +obj-$(CONFIG_CAN_M_CAN_CORE) += m_can.o +obj-$(CONFIG_CAN_M_CAN_PLATFORM) += m_can_platform.o +obj-$(CONFIG_CAN_M_CAN_TCAN4X5X) += tcan4x5x.o
This file is provided in a sub-sequent patched! The code *must* compile for every single patch applied for bisect'ing. Looking to your first patch, I just realize that this is also not the case!
I think it makes sense to squash patch 1 and 2.
Quoted 80 lines
drivers/net/can/m_can/m_can.c +2
-64@@ -28,87 +28,14 @@#include <linux/can/dev.h> #include <linux/pinctrl/consumer.h> +#include "m_can_platform.h" +/* napi related */ #define M_CAN_NAPI_WEIGHT 64/* message ram configuration data length */ #define MRAM_CFG_LEN 8 -/* registers definition */ -enum m_can_reg { -M_CAN_CREL =0x0 , -M_CAN_ENDN =0x4 , -M_CAN_CUST =0x8 , -M_CAN_DBTP =0xc , -M_CAN_TEST =0x10 , -M_CAN_RWD =0x14 , -M_CAN_CCCR =0x18 , -M_CAN_NBTP =0x1c , -M_CAN_TSCC =0x20 , -M_CAN_TSCV =0x24 , -M_CAN_TOCC =0x28 , -M_CAN_TOCV =0x2c , -M_CAN_ECR =0x40 , -M_CAN_PSR =0x44 , -/* TDCR Register only available for version >=3.1.x */ -M_CAN_TDCR =0x48 , -M_CAN_IR =0x50 , -M_CAN_IE =0x54 , -M_CAN_ILS =0x58 , -M_CAN_ILE =0x5c , -M_CAN_GFC =0x80 , -M_CAN_SIDFC =0x84 , -M_CAN_XIDFC =0x88 , -M_CAN_XIDAM =0x90 , -M_CAN_HPMS =0x94 , -M_CAN_NDAT1 =0x98 , -M_CAN_NDAT2 =0x9c , -M_CAN_RXF0C =0xa0 , -M_CAN_RXF0S =0xa4 , -M_CAN_RXF0A =0xa8 , -M_CAN_RXBC =0xac , -M_CAN_RXF1C =0xb0 , -M_CAN_RXF1S =0xb4 , -M_CAN_RXF1A =0xb8 , -M_CAN_RXESC =0xbc , -M_CAN_TXBC =0xc0 , -M_CAN_TXFQS =0xc4 , -M_CAN_TXESC =0xc8 , -M_CAN_TXBRP =0xcc , -M_CAN_TXBAR =0xd0 , -M_CAN_TXBCR =0xd4 , -M_CAN_TXBTO =0xd8 , -M_CAN_TXBCF =0xdc , -M_CAN_TXBTIE =0xe0 , -M_CAN_TXBCIE =0xe4 , -M_CAN_TXEFC =0xf0 , -M_CAN_TXEFS =0xf4 , -M_CAN_TXEFA =0xf8 , -}; - -/* m_can lec values */ -enum m_can_lec_type { -LEC_NO_ERROR =0 , -LEC_STUFF_ERROR , -LEC_FORM_ERROR , -LEC_ACK_ERROR , -LEC_BIT1_ERROR , -LEC_BIT0_ERROR , -LEC_CRC_ERROR , -LEC_UNUSED , -}; -
drivers/net/can/m_can/m_can.c line 110
Quoted 10 lines
drivers/net/can/m_can/m_can.c +0
-10-enum m_can_mram_cfg { -MRAM_SIDF =0 , -MRAM_XIDF , -MRAM_RXF0 , -MRAM_RXF1 , -MRAM_RXB , -MRAM_TXE , -MRAM_TXB , -MRAM_CFG_NUM , -};
Patch 1 should have already done that!
Quoted 73 lines
drivers/net/can/m_can/m_can.c +26
-29/* Core Release Register (CREL) */ #define CREL_REL_SHIFT 28#define CREL_REL_MASK (0xF << CREL_REL_SHIFT)@@ -343,72 +270,81 @@ enum m_can_mram_cfg {#define TX_BUF_MM_MASK (0xff << TX_BUF_MM_SHIFT)/* Tx event FIFO Element */ -/* E1 */ #define TX_EVENT_MM_SHIFT TX_BUF_MM_SHIFT#define TX_EVENT_MM_MASK (0xff << TX_EVENT_MM_SHIFT) -/* address offset and element number for each FIFO/Buffer in the Message RAM */ -struct mram_cfg { -u16 off; -u8 num; -}; +static u32 m_can_read (struct m_can_classdev *priv,enum m_can_reg reg) +{ +u32 ret = -EINVAL ; -/* m_can private data structure */ -struct m_can_priv { -struct can_priv can;/* must be the first member */ -struct napi_struct napi; -struct net_device *dev; -struct device *device; -struct clk *hclk; -struct clk *cclk; -void __iomem *base; -u32 irqstatus; -int version; - -/* message ram configuration */ -void __iomem *mram_base; -struct mram_cfg mcfg[MRAM_CFG_NUM ]; -}; +if (priv->read_reg) + ret = priv->read_reg (priv, reg); -static inline u32 m_can_read (const struct m_can_priv *priv,enum m_can_reg reg) +return ret; +} + +static int m_can_write (struct m_can_classdev *priv,enum m_can_reg reg,u32 val) { -return readl (priv->base + reg); +int ret = -EINVAL ; + +if (priv->write_reg) + ret = priv->write_reg (priv, reg, val); + +return ret; } -static inline void m_can_write (const struct m_can_priv *priv, -enum m_can_reg reg,u32 val) +static u32 m_can_fifo_read (struct m_can_classdev *priv, +u32 fgi,unsigned int offset) { -writel (val, priv->base + reg); +u32 addr_offset = priv->mcfg[MRAM_RXF0 ].off + fgi *RXF0_ELEMENT_SIZE + offset; +u32 ret = -EINVAL ; + +if (priv->read_fifo) + ret = priv->read_fifo (priv, addr_offset); + +return ret; } -static inline u32 m_can_fifo_read (const struct m_can_priv *priv, -u32 fgi,unsigned int offset) +static u32 m_can_fifo_write (struct m_can_classdev *priv, +u32 fpi,unsigned int offset,u32 val)
drivers/net/can/m_can/m_can.c enum m_can_mram_cfg { · line 317
Quoted 10 lines
drivers/net/can/m_can/m_can.c +7
-2{ -return readl (priv->mram_base + priv->mcfg[MRAM_RXF0 ].off + - fgi *RXF0_ELEMENT_SIZE + offset); +u32 addr_offset = priv->mcfg[MRAM_TXB ].off + fpi *TXB_ELEMENT_SIZE + offset; +u32 ret = -EINVAL ; + +if (priv->write_fifo) + ret = priv->write_fifo (priv, addr_offset, val); + +return ret;
Why not just:
if (priv->write_fifo) return priv->write_fifo(priv, addr_offset, val); else return -EINVAL;
Here and below...
Quoted 99 lines
drivers/net/can/m_can/m_can.c +26
-19} -static inline void m_can_fifo_write (const struct m_can_priv *priv, -u32 fpi,unsigned int offset,u32 val) +static u32 m_can_fifo_write_no_off (struct m_can_classdev *priv, +u32 fpi,u32 val) { -writel (val, priv->mram_base + priv->mcfg[MRAM_TXB ].off + - fpi *TXB_ELEMENT_SIZE + offset); +u32 ret =0 ; + +if (priv->write_fifo) + ret = priv->write_fifo (priv, fpi, val); + +return ret; } -static inline u32 m_can_txe_fifo_read (const struct m_can_priv *priv, -u32 fgi, -u32 offset) { -return readl (priv->mram_base + priv->mcfg[MRAM_TXE ].off + - fgi *TXE_ELEMENT_SIZE + offset); +static u32 m_can_txe_fifo_read (struct m_can_classdev *priv,u32 fgi,u32 offset) +{ +u32 addr_offset = priv->mcfg[MRAM_TXE ].off + fgi *TXE_ELEMENT_SIZE + offset; +u32 ret = -EINVAL ; + +if (priv->read_fifo) + ret = priv->read_fifo (priv, addr_offset); + +return ret; } -static inline bool m_can_tx_fifo_full (const struct m_can_priv *priv) +static inline bool m_can_tx_fifo_full (struct m_can_classdev *priv) { -return !!(m_can_read (priv,M_CAN_TXFQS ) &TXFQS_TFQF ); +return !!(m_can_read (priv,M_CAN_TXFQS ) &TXFQS_TFQF ); } -static inline void m_can_config_endisable (const struct m_can_priv *priv, -bool enable) +void m_can_config_endisable (struct m_can_classdev *priv,bool enable) {u32 cccr =m_can_read (priv,M_CAN_CCCR );u32 timeout =10 ;@@ -433,7 +369,7 @@ static inline void m_can_config_endisable(const struct m_can_priv *priv,while ((m_can_read (priv,M_CAN_CCCR ) & (CCCR_INIT |CCCR_CCE )) != val) {if (timeout ==0 ) { -netdev_warn (priv->dev,"Failed to init module\n" ); +netdev_warn (priv->net,"Failed to init module\n" );return ; } timeout--;@@ -441,13 +377,13 @@ static inline void m_can_config_endisable(const struct m_can_priv *priv, } } -static inline void m_can_enable_all_interrupts (const struct m_can_priv *priv) +static inline void m_can_enable_all_interrupts (struct m_can_classdev *priv) {/* Only interrupt line 0 is used in this driver */ m_can_write (priv,M_CAN_ILE ,ILE_EINT0 ); } -static inline void m_can_disable_all_interrupts (const struct m_can_priv *priv) +static inline void m_can_disable_all_interrupts (struct m_can_classdev *priv) {m_can_write (priv,M_CAN_ILE ,0x0 ); }@@ -455,7 +391,7 @@ static inline void m_can_disable_all_interrupts(const struct m_can_priv *priv)static void m_can_read_fifo (struct net_device *dev,u32 rxfs) {struct net_device_stats *stats = &dev->stats; -struct m_can_priv *priv =netdev_priv (dev); +struct m_can_classdev *priv =netdev_priv (dev);struct canfd_frame *cf;struct sk_buff *skb;u32 id, fgi, dlc;@@ -512,7 +448,7 @@ static void m_can_read_fifo(struct net_device *dev, u32 rxfs)static int m_can_do_rx_poll (struct net_device *dev,int quota) { -struct m_can_priv *priv =netdev_priv (dev); +struct m_can_classdev *priv =netdev_priv (dev);u32 pkts =0 ;u32 rxfs;@@ -565,7 +501,7 @@ static int m_can_handle_lost_msg(struct net_device *dev)static int m_can_handle_lec_err (struct net_device *dev,enum m_can_lec_type lec_type) { -struct m_can_priv *priv =netdev_priv (dev); +struct m_can_classdev *priv =netdev_priv (dev);struct net_device_stats *stats = &dev->stats;struct can_frame *cf;struct sk_buff *skb;
drivers/net/can/m_can/m_can.c static int m_can_handle_lec_err(struct net_device *dev, · line 561
Quoted 5 lines
drivers/net/can/m_can/m_can.c +1
-1@@ -622,7 +558,7 @@ static int m_can_handle_lec_err(struct net_device *dev,static int __m_can_get_berr_counter (const struct net_device *dev,struct can_berr_counter *bec) { -struct m_can_priv *priv =netdev_priv (dev); +struct m_can_classdev *priv =netdev_priv (dev);
Back to the naming: You use "priv" here because I only wanted to see minimal changes to the m_can code. Why not also re-using "struct m_can_priv" for the moment and use:
m_can_register(struct m_can_priv priv);
Later-on, when the development has settled, we could introduce betetr names. What do you think?
Quoted 52 lines
drivers/net/can/m_can/m_can.c +11
-7unsigned int ecr; ecr =m_can_read (priv,M_CAN_ECR );@@ -632,28 +568,32 @@ static int __m_can_get_berr_counter(const struct net_device *dev,return 0 ; } -static int m_can_clk_start (struct m_can_priv *priv) +static int m_can_clk_start (struct m_can_classdev *priv) {int err; - err =pm_runtime_get_sync (priv->device); +if (priv->pm_clock_support ==0 ) +return 0 ; + + err =pm_runtime_get_sync (priv->dev);if (err <0 ) { -pm_runtime_put_noidle (priv->device); +pm_runtime_put_noidle (priv->dev);return err; }return 0 ; } -static void m_can_clk_stop (struct m_can_priv *priv) +static void m_can_clk_stop (struct m_can_classdev *priv) { -pm_runtime_put_sync (priv->device); +if (priv->pm_clock_support) +pm_runtime_put_sync (priv->dev); }static int m_can_get_berr_counter (const struct net_device *dev,struct can_berr_counter *bec) { -struct m_can_priv *priv =netdev_priv (dev); +struct m_can_classdev *priv =netdev_priv (dev);int err; err =m_can_clk_start (priv);@@ -670,7 +610,7 @@ static int m_can_get_berr_counter(const struct net_device *dev,static int m_can_handle_state_change (struct net_device *dev,enum can_state new_state) { -struct m_can_priv *priv =netdev_priv (dev); +struct m_can_classdev *priv =netdev_priv (dev);struct net_device_stats *stats = &dev->stats;struct can_frame *cf;struct sk_buff *skb;
drivers/net/can/m_can/m_can.c static int m_can_handle_state_change(struct net_device *dev, · line 690
Quoted 10 lines
drivers/net/can/m_can/m_can.c +2
-3@@ -744,25 +684,22 @@ static int m_can_handle_state_change(struct net_device *dev,static int m_can_handle_state_errors (struct net_device *dev,u32 psr) { -struct m_can_priv *priv =netdev_priv (dev); +struct m_can_classdev *priv =netdev_priv (dev);int work_done =0 ; -if ((psr &PSR_EW ) && - (priv->can.state !=CAN_STATE_ERROR_WARNING )) { +if ((psr &PSR_EW ) && priv->can.state !=CAN_STATE_ERROR_WARNING ) {
This is an unrelated cosmetic change. We should avoid them here. Could be done later-on with an extra patch.
Quoted 42 lines
drivers/net/can/m_can/m_can.c +5
-8netdev_dbg (dev,"entered error warning state\n" ); work_done +=m_can_handle_state_change (dev,CAN_STATE_ERROR_WARNING ); } -if ((psr &PSR_EP ) && - (priv->can.state !=CAN_STATE_ERROR_PASSIVE )) { +if ((psr &PSR_EP ) && priv->can.state !=CAN_STATE_ERROR_PASSIVE ) {netdev_dbg (dev,"entered error passive state\n" ); work_done +=m_can_handle_state_change (dev,CAN_STATE_ERROR_PASSIVE ); } -if ((psr &PSR_BO ) && - (priv->can.state !=CAN_STATE_BUS_OFF )) { +if ((psr &PSR_BO ) && priv->can.state !=CAN_STATE_BUS_OFF ) {netdev_dbg (dev,"entered error bus off state\n" ); work_done +=m_can_handle_state_change (dev,CAN_STATE_BUS_OFF );@@ -797,7 +734,7 @@ static inline bool is_lec_err(u32 psr)static int m_can_handle_bus_errors (struct net_device *dev,u32 irqstatus,u32 psr) { -struct m_can_priv *priv =netdev_priv (dev); +struct m_can_classdev *priv =netdev_priv (dev);int work_done =0 ;if (irqstatus &IR_RF0L )@@ -814,10 +751,9 @@ static int m_can_handle_bus_errors(struct net_device *dev, u32 irqstatus,return work_done; } -static int m_can_poll (struct napi_struct *napi,int quota) +static int m_can_rx_handler (struct net_device *dev,int quota) { -struct net_device *dev = napi->dev; -struct m_can_priv *priv =netdev_priv (dev); +struct m_can_classdev *priv =netdev_priv (dev);int work_done =0 ;u32 irqstatus, psr;
drivers/net/can/m_can/m_can.c static int m_can_poll(struct napi_struct *napi, int quota) · line 777
Quoted 8 lines
drivers/net/can/m_can/m_can.c +5
-0@@ -834,13 +770,33 @@ static int m_can_poll(struct napi_struct *napi, int quota)if (irqstatus &IR_RF0N ) work_done +=m_can_do_rx_poll (dev, (quota - work_done)); +end: +return work_done; +} + +static int m_can_rx (struct net_device *dev)
m_can_rx_peripheral ?
Quoted 131 lines
drivers/net/can/m_can/m_can.c +34
-15+{ +struct m_can_classdev *priv =netdev_priv (dev); +m_can_rx_handler (dev,1 ); + +m_can_enable_all_interrupts (priv); + +return 0 ; +} + +static int m_can_poll (struct napi_struct *napi,int quota) +{ +struct net_device *dev = napi->dev; +struct m_can_classdev *priv =netdev_priv (dev); +int work_done =0 ; + + work_done =m_can_rx_handler (dev, quota);if (work_done < quota) {napi_complete_done (napi, work_done);m_can_enable_all_interrupts (priv); } -end:return work_done; }@@ -852,7 +808,7 @@ static void m_can_echo_tx_event(struct net_device *dev)int i =0 ;unsigned int msg_mark; -struct m_can_priv *priv =netdev_priv (dev); +struct m_can_classdev *priv =netdev_priv (dev);struct net_device_stats *stats = &dev->stats;/* read tx event fifo status */ @@ -885,7 +841,7 @@ static void m_can_echo_tx_event(struct net_device *dev)static irqreturn_t m_can_isr (int irq,void *dev_id) {struct net_device *dev = (struct net_device *)dev_id; -struct m_can_priv *priv =netdev_priv (dev); +struct m_can_classdev *priv =netdev_priv (dev);struct net_device_stats *stats = &dev->stats;u32 ir;@@ -905,7 +861,10 @@ static irqreturn_t m_can_isr(int irq, void *dev_id)if ((ir &IR_RF0N ) || (ir &IR_ERR_ALL_30X )) { priv->irqstatus = ir;m_can_disable_all_interrupts (priv); -napi_schedule (&priv->napi); +if (!priv->is_peripherial) +napi_schedule (&priv->napi); +else +m_can_rx (dev); }if (priv->version ==30 ) {@@ -927,6 +886,9 @@ static irqreturn_t m_can_isr(int irq, void *dev_id) } } +if (priv->clr_dev_interrupts) + priv->clr_dev_interrupts (priv); +return IRQ_HANDLED ; }@@ -980,7 +942,7 @@ static const struct can_bittiming_const m_can_data_bittiming_const_31X = {static int m_can_set_bittiming (struct net_device *dev) { -struct m_can_priv *priv =netdev_priv (dev); +struct m_can_classdev *priv =netdev_priv (dev);const struct can_bittiming *bt = &priv->can.bittiming;const struct can_bittiming *dbt = &priv->can.data_bittiming;u16 brp, sjw, tseg1, tseg2;@@ -1053,7 +1015,7 @@ static int m_can_set_bittiming(struct net_device *dev)*/ static void m_can_chip_config (struct net_device *dev) { -struct m_can_priv *priv =netdev_priv (dev); +struct m_can_classdev *priv =netdev_priv (dev);u32 cccr, test;m_can_config_endisable (priv, true);@@ -1165,7 +1127,7 @@ static void m_can_chip_config(struct net_device *dev)static void m_can_start (struct net_device *dev) { -struct m_can_priv *priv =netdev_priv (dev); +struct m_can_classdev *priv =netdev_priv (dev);/* basic m_can configuration */ m_can_chip_config (dev);@@ -1194,20 +1156,17 @@ static int m_can_set_mode(struct net_device *dev, enum can_mode mode)* else it returns the release and step coded as: * return value = 10 * <release> + 1 * <step> */ -static int m_can_check_core_release (void __iomem *m_can_base) +static int m_can_check_core_release (struct m_can_classdev *priv) {u32 crel_reg;u8 rel;u8 step;int res; -struct m_can_priv temp_priv = { - .base = m_can_base - };/* Read Core Release Version and split into version number * Example: Version 3.2.1 => rel = 3; step = 2; substep = 1; */ - crel_reg =m_can_read (&temp_priv,M_CAN_CREL ); + crel_reg =m_can_read (priv,M_CAN_CREL ); rel = (u8)((crel_reg &CREL_REL_MASK ) >>CREL_REL_SHIFT ); step = (u8)((crel_reg &CREL_STEP_MASK ) >>CREL_STEP_SHIFT );@@ -1225,18 +1184,22 @@ static int m_can_check_core_release(void __iomem *m_can_base)/* Selectable Non ISO support only in version 3.2.x * This function checks if the bit is writable. */ -static bool m_can_niso_supported (const struct m_can_priv *priv) +static bool m_can_niso_supported (struct m_can_classdev *priv) { -u32 cccr_reg, cccr_poll; -int niso_timeout; +u32 cccr_reg, cccr_poll =0 ; +int niso_timeout = -ETIMEDOUT ; +int i;m_can_config_endisable (priv, true); cccr_reg =m_can_read (priv,M_CAN_CCCR );
drivers/net/can/m_can/m_can.c static int m_can_check_core_release(void __iomem *m_can_base) · line 1202
Quoted 10 lines
drivers/net/can/m_can/m_can.c +5
-2cccr_reg |=CCCR_NISO ;m_can_write (priv,M_CAN_CCCR , cccr_reg); - niso_timeout =readl_poll_timeout ((priv->base +M_CAN_CCCR ), cccr_poll, - (cccr_poll == cccr_reg),0 ,10 ); +for (i =0 ; i <=10 ; i++) { + cccr_poll =m_can_read (priv,M_CAN_CCCR ); +if (cccr_poll == cccr_reg) + niso_timeout =0 ; + }
This change is also unrelated. Should be done in an extra patch.
Quoted 56 lines
drivers/net/can/m_can/m_can.c +13
-16/* Clear NISO */ cccr_reg &= ~(CCCR_NISO );@@ -1248,112 +1211,100 @@ static bool m_can_niso_supported(const struct m_can_priv *priv)return !niso_timeout; } -static int m_can_dev_setup (struct platform_device *pdev,struct net_device *dev, -void __iomem *addr) +static int m_can_dev_setup (struct m_can_classdev *m_can_dev) { -struct m_can_priv *priv; +struct net_device *dev = m_can_dev->net;int m_can_version; - m_can_version =m_can_check_core_release (addr); + m_can_version =m_can_check_core_release (m_can_dev);/* return if unsupported version */ if (!m_can_version) { -dev_err (&pdev->dev,"Unsupported version number: %2d" , +dev_err (m_can_dev->dev,"Unsupported version number: %2d" , m_can_version);return -EINVAL ; } - priv =netdev_priv (dev); -netif_napi_add (dev, &priv->napi, m_can_poll,M_CAN_NAPI_WEIGHT ); +if (!m_can_dev->is_peripherial) +netif_napi_add (dev, &m_can_dev->napi, + m_can_poll,M_CAN_NAPI_WEIGHT );/* Shared properties of all M_CAN versions */ - priv->version = m_can_version; - priv->dev = dev; - priv->base = addr; - priv->can.do_set_mode = m_can_set_mode; - priv->can.do_get_berr_counter = m_can_get_berr_counter; + m_can_dev->version = m_can_version; + m_can_dev->can.do_set_mode = m_can_set_mode; + m_can_dev->can.do_get_berr_counter = m_can_get_berr_counter;/* Set M_CAN supported operations */ - priv->can.ctrlmode_supported =CAN_CTRLMODE_LOOPBACK | + m_can_dev->can.ctrlmode_supported =CAN_CTRLMODE_LOOPBACK |CAN_CTRLMODE_LISTENONLY |CAN_CTRLMODE_BERR_REPORTING |CAN_CTRLMODE_FD ;/* Set properties depending on M_CAN version */ -switch (priv->version) { +switch (m_can_dev->version) {case 30 :/* CAN_CTRLMODE_FD_NON_ISO is fixed with M_CAN IP v3.0.x */ can_set_static_ctrlmode (dev,CAN_CTRLMODE_FD_NON_ISO ); - priv->can.bittiming_const = &m_can_bittiming_const_30X; - priv->can.data_bittiming_const = +if (m_can_dev->bit_timing)
drivers/net/can/m_can/m_can.c static bool m_can_niso_supported(const struct m_can_priv *priv) · line 1257
Quoted 10 lines
drivers/net/can/m_can/m_can.c +9
-0+ m_can_dev->can.bittiming_const = m_can_dev->bit_timing; + else + m_can_dev->can.bittiming_const = + &m_can_bittiming_const_30X; +if (m_can_dev->data_timing) + m_can_dev->can.data_bittiming_const = + m_can_dev->data_timing; +else + m_can_dev->can.data_bittiming_const = &m_can_data_bittiming_const_30X;
Should'nt that go to m_can_platform.c?
Quoted 29 lines
drivers/net/can/m_can/m_can.c +18
-4break ;case 31 :/* CAN_CTRLMODE_FD_NON_ISO is fixed with M_CAN IP v3.1.x */ can_set_static_ctrlmode (dev,CAN_CTRLMODE_FD_NON_ISO ); - priv->can.bittiming_const = &m_can_bittiming_const_31X; - priv->can.data_bittiming_const = +if (m_can_dev->bit_timing) + m_can_dev->can.bittiming_const = m_can_dev->bit_timing; +else + m_can_dev->can.bittiming_const = + &m_can_bittiming_const_31X; +if (m_can_dev->data_timing) + m_can_dev->can.data_bittiming_const = + m_can_dev->data_timing; +else + m_can_dev->can.data_bittiming_const = &m_can_data_bittiming_const_31X;break ;case 32 : - priv->can.bittiming_const = &m_can_bittiming_const_31X; - priv->can.data_bittiming_const = +if (m_can_dev->bit_timing) + m_can_dev->can.bittiming_const = m_can_dev->bit_timing; +else + m_can_dev->can.bittiming_const = + &m_can_bittiming_const_31X; + +if (m_can_dev->data_timing) + m_can_dev->can.data_bittiming_const =
drivers/net/can/m_can/m_can.c static bool m_can_niso_supported(const struct m_can_priv *priv) · line 1291
Quoted 10 lines
drivers/net/can/m_can/m_can.c +6
-1+ m_can_dev->data_timing; + else + m_can_dev->can.data_bittiming_const = &m_can_data_bittiming_const_31X; - priv->can.ctrlmode_supported |= (m_can_niso_supported (priv) + + m_can_dev->can.ctrlmode_supported |= + (m_can_niso_supported (m_can_dev) ?CAN_CTRLMODE_FD_NON_ISO :0 );
if (m_can_niso_supported(m_can_dev) m_can_dev->can.ctrlmode_supported |= CAN_CTRLMODE_FD_NON_ISO;
drivers/net/can/m_can/m_can.c static bool m_can_niso_supported(const struct m_can_priv *priv) · line 1314
Quoted 13 lines
drivers/net/can/m_can/m_can.c +2
-6break ;default : -dev_err (&pdev->dev,"Unsupported version number: %2d" , - priv->version); +dev_err (m_can_dev->dev,"Unsupported version number: %2d" , + m_can_dev->version);return -EINVAL ; } -return 0 ; -} - -static int m_can_open (struct net_device *dev)
This function has been moved around making it difficult to understand the diffs.
Quoted 53 lines
drivers/net/can/m_can/m_can.c +3
-37-{ -struct m_can_priv *priv =netdev_priv (dev); -int err; - - err =m_can_clk_start (priv); -if (err) -return err; - -/* open the can device */ - err =open_candev (dev); -if (err) { -netdev_err (dev,"failed to open can device\n" ); - goto exit_disable_clks; - } - -/* register interrupt handler */ - err =request_irq (dev->irq, m_can_isr,IRQF_SHARED , dev->name, - dev); -if (err <0 ) { -netdev_err (dev,"failed to request interrupt\n" ); - goto exit_irq_fail; - } - -/* start the m_can controller */ -m_can_start (dev); - -can_led_event (dev,CAN_LED_EVENT_OPEN ); -napi_enable (&priv->napi); -netif_start_queue (dev); +if (m_can_dev->device_init) + m_can_dev->device_init (m_can_dev);return 0 ; - -exit_irq_fail: -close_candev (dev); -exit_disable_clks: -m_can_clk_stop (priv); -return err; }static void m_can_stop (struct net_device *dev) { -struct m_can_priv *priv =netdev_priv (dev); +struct m_can_classdev *priv =netdev_priv (dev);/* disable all interrupts */ m_can_disable_all_interrupts (priv);@@ -1364,13 +1315,16 @@ static void m_can_stop(struct net_device *dev)static int m_can_close (struct net_device *dev) { -struct m_can_priv *priv =netdev_priv (dev);
drivers/net/can/m_can/m_can.c static void m_can_stop(struct net_device *dev) · line 1326
Quoted 10 lines
drivers/net/can/m_can/m_can.c +4
-1+struct m_can_classdev *priv =netdev_priv (dev);netif_stop_queue (dev); -napi_disable (&priv->napi); +if (!priv->is_peripherial) +napi_disable (&priv->napi);m_can_stop (dev);m_can_clk_stop (priv);free_irq (dev->irq, dev); +destroy_workqueue (priv->wq);
if (priv->is_peripherial) ?
Quoted 14 lines
drivers/net/can/m_can/m_can.c +2
-1+ priv->wq =NULL ;close_candev (dev);can_led_event (dev,CAN_LED_EVENT_STOP );@@ -1379,7 +1333,7 @@ static int m_can_close(struct net_device *dev)static int m_can_next_echo_skb_occupied (struct net_device *dev,int putidx) { -struct m_can_priv *priv =netdev_priv (dev); +struct m_can_classdev *priv =netdev_priv (dev);/*get wrap around for loopback skb index */ unsigned int wrap = priv->can.echo_skb_max;int next_idx;
drivers/net/can/m_can/m_can.c static int m_can_next_echo_skb_occupied(struct net_device *dev, int putidx) · line 1395
Quoted 5 lines
drivers/net/can/m_can/m_can.c +0
-2@@ -1391,18 +1345,17 @@ static int m_can_next_echo_skb_occupied(struct net_device *dev, int putidx)return !!priv->can.echo_skb[next_idx]; } -static netdev_tx_t m_can_start_xmit (struct sk_buff *skb, -struct net_device *dev)
Ditto
drivers/net/can/m_can/m_can.c static int m_can_next_echo_skb_occupied(struct net_device *dev, int putidx) · line 1348
Quoted 1 line
drivers/net/can/m_can/m_can.c +1
-0+static void m_can_tx_work_handler (struct work_struct *ws)
I think you need a common function to do the tx.
Quoted 19 lines
drivers/net/can/m_can/m_can.c +5
-5{ -struct m_can_priv *priv =netdev_priv (dev); -struct canfd_frame *cf = (struct canfd_frame *)skb->data; + struct m_can_classdev *priv = container_of(ws, struct m_can_classdev, + tx_work); +struct canfd_frame *cf = (struct canfd_frame *)priv->skb->data; +struct net_device *dev = priv->net; +struct sk_buff *skb = priv->skb;u32 id, cccr, fdflags;int i;int putidx; -if (can_dropped_invalid_skb (dev, skb)) -return NETDEV_TX_OK ; -/* Generate ID field for TX buffer Element */ /* Common to all supported M_CAN versions */ if (cf->can_id &CAN_EFF_FLAG ) {
drivers/net/can/m_can/m_can.c static netdev_tx_t m_can_start_xmit(struct sk_buff *skb, · line 1388
Quoted 6 lines
drivers/net/can/m_can/m_can.c +2
-1@@ -1431,7 +1384,8 @@ static netdev_tx_t m_can_start_xmit(struct sk_buff *skb,can_put_echo_skb (skb, dev,0 );if (priv->can.ctrlmode &CAN_CTRLMODE_FD ) { - cccr =m_can_read (priv,M_CAN_CCCR ); +/*cccr = m_can_read(priv, M_CAN_CCCR);*/ + cccr =0 ;
Unrelated change.
Quoted 24 lines
drivers/net/can/m_can/m_can.c +5
-4cccr &= ~(CCCR_CMR_MASK <<CCCR_CMR_SHIFT );if (can_is_canfd_skb (skb)) {if (cf->flags &CANFD_BRS )@@ -1457,7 +1411,7 @@ static netdev_tx_t m_can_start_xmit(struct sk_buff *skb,netif_stop_queue (dev);netdev_warn (dev,"TX queue active although FIFO is full." ); -return NETDEV_TX_BUSY ; +return ; }/* get put index for frame */ @@ -1498,14 +1452,87 @@ static netdev_tx_t m_can_start_xmit(struct sk_buff *skb,m_can_write (priv,M_CAN_TXBAR , (1 << putidx));/* stop network queue if fifo full */ -if (m_can_tx_fifo_full (priv) || -m_can_next_echo_skb_occupied (dev, putidx)) -netif_stop_queue (dev); +if (m_can_tx_fifo_full (priv) || +m_can_next_echo_skb_occupied (dev, putidx)) +netif_stop_queue (dev); } +}
drivers/net/can/m_can/m_can.c static netdev_tx_t m_can_start_xmit(struct sk_buff *skb, · line 1469
Quoted 10 lines
drivers/net/can/m_can/m_can.c +10
-0+ +static netdev_tx_t m_can_start_xmit (struct sk_buff *skb, +struct net_device *dev) +{ +struct m_can_classdev *priv =netdev_priv (dev); + +if (can_dropped_invalid_skb (dev, skb)) +return NETDEV_TX_BUSY ; + +netif_stop_queue (dev);
if (!priv->is_peripheral) {
m_can_tx(priv, skb);
} else {
netif_stop_queue(dev);
priv->skb = skb;
queue_work(priv->wq, &priv->tx_work);
}I think stopping the queue here is still experimental.
Quoted 9 lines
drivers/net/can/m_can/m_can.c +6
-0return NETDEV_TX_OK ; } +static int m_can_open (struct net_device *dev) +{ +struct m_can_classdev *priv =netdev_priv (dev); +int err; + + err =m_can_clk_start (priv);
drivers/net/can/m_can/m_can.c static netdev_tx_t m_can_start_xmit(struct sk_buff *skb, · line 1491
Quoted 10 lines
drivers/net/can/m_can/m_can.c +10
-0+if (err) +return err; + +/* open the can device */ + err =open_candev (dev); +if (err) { +netdev_err (dev,"failed to open can device\n" ); + goto exit_disable_clks; + } +
if (priv->is_peripheral) {drivers/net/can/m_can/m_can.c static netdev_tx_t m_can_start_xmit(struct sk_buff *skb, · line 1499
Quoted 8 lines
drivers/net/can/m_can/m_can.c +8
-0+ priv->wq =alloc_workqueue ("mcan_wq" ,WQ_FREEZABLE |WQ_MEM_RECLAIM , +0 ); +if (!priv->wq) { + err = -ENOMEM ; + goto out_wq_fail; + } + +INIT_WORK (&priv->tx_work, m_can_tx_work_handler);
}
Quoted 174 lines
drivers/net/can/m_can/m_can.c +74
-62+/* register interrupt handler */ +if (priv->is_peripherial) + err =request_threaded_irq (dev->irq,NULL , m_can_isr, +IRQF_ONESHOT |IRQF_TRIGGER_FALLING , + dev->name, dev); +else + err =request_irq (dev->irq, m_can_isr,IRQF_SHARED , dev->name, + dev); + +if (err <0 ) { +netdev_err (dev,"failed to request interrupt\n" ); + goto exit_irq_fail; + } + +/* start the m_can controller */ +m_can_start (dev); + +can_led_event (dev,CAN_LED_EVENT_OPEN ); + +if (!priv->is_peripherial) +napi_enable (&priv->napi); + +netif_start_queue (dev); + +return 0 ; + +exit_irq_fail: +destroy_workqueue (priv->wq); +out_wq_fail: +close_candev (dev); +exit_disable_clks: +m_can_clk_stop (priv); +return err; +} +static const struct net_device_ops m_can_netdev_ops = { .ndo_open = m_can_open, .ndo_stop = m_can_close,@@ -1521,21 +1548,7 @@ static int register_m_can_dev(struct net_device *dev)return register_candev (dev); } -static void m_can_init_ram (struct m_can_priv *priv) -{ -int end, i, start; - -/* initialize the entire Message RAM in use to avoid possible -* ECC/parity checksum errors when reading an uninitialized buffer -*/ - start = priv->mcfg[MRAM_SIDF ].off; - end = priv->mcfg[MRAM_TXB ].off + - priv->mcfg[MRAM_TXB ].num *TXB_ELEMENT_SIZE ; -for (i = start; i < end; i +=4 ) -writel (0x0 , priv->mram_base + i); -} - -static void m_can_of_parse_mram (struct m_can_priv *priv, +static void m_can_of_parse_mram (struct m_can_classdev *priv,const u32 *mram_config_vals) { priv->mcfg[MRAM_SIDF ].off = mram_config_vals[0 ];@@ -1562,9 +1575,8 @@ static void m_can_of_parse_mram(struct m_can_priv *priv, priv->mcfg[MRAM_TXB ].num = mram_config_vals[7 ] & (TXBC_NDTB_MASK >>TXBC_NDTB_SHIFT ); - dev_dbg(priv->device, - "mram_base %p sidf 0x%x %d xidf 0x%x %d rxf0 0x%x %d rxf1 0x%x %d rxb 0x%x %d txe 0x%x %d txb 0x%x %d\n", - priv->mram_base, + dev_dbg(priv->dev, + "sidf 0x%x %d xidf 0x%x %d rxf0 0x%x %d rxf1 0x%x %d rxb 0x%x %d txe 0x%x %d txb 0x%x %d\n", priv->mcfg[MRAM_SIDF].off, priv->mcfg[MRAM_SIDF].num, priv->mcfg[MRAM_XIDF].off, priv->mcfg[MRAM_XIDF].num, priv->mcfg[MRAM_RXF0].off, priv->mcfg[MRAM_RXF0].num,@@ -1572,63 +1584,55 @@ static void m_can_of_parse_mram(struct m_can_priv *priv, priv->mcfg[MRAM_RXB ].off, priv->mcfg[MRAM_RXB ].num, priv->mcfg[MRAM_TXE ].off, priv->mcfg[MRAM_TXE ].num, priv->mcfg[MRAM_TXB].off, priv->mcfg[MRAM_TXB].num); - -m_can_init_ram (priv); } -static int m_can_plat_probe (struct platform_device *pdev) +void m_can_init_ram (struct m_can_classdev *priv) { -struct net_device *dev; -struct m_can_priv *priv; -struct resource *res; -void __iomem *addr; -void __iomem *mram_addr; -struct clk *hclk, *cclk; -int irq, ret; -struct device_node *np; -u32 mram_config_vals[MRAM_CFG_LEN ]; -u32 tx_fifo_size; - - np = pdev->dev.of_node; +int end, i, start; - hclk =devm_clk_get (&pdev->dev,"hclk" ); - cclk =devm_clk_get (&pdev->dev,"cclk" ); +/* initialize the entire Message RAM in use to avoid possible +* ECC/parity checksum errors when reading an uninitialized buffer +*/ + start = priv->mcfg[MRAM_SIDF ].off; + end = priv->mcfg[MRAM_TXB ].off + + priv->mcfg[MRAM_TXB ].num *TXB_ELEMENT_SIZE ; -if (IS_ERR (hclk) ||IS_ERR (cclk)) { -dev_err (&pdev->dev,"no clock found\n" ); - ret = -ENODEV ; - goto failed_ret; - } +for (i = start; i < end; i +=4 ) +m_can_fifo_write_no_off (priv, i,0x0 ); +} +EXPORT_SYMBOL_GPL (m_can_init_ram); - res =platform_get_resource_byname (pdev,IORESOURCE_MEM ,"m_can" ); - addr =devm_ioremap_resource (&pdev->dev, res); - irq =platform_get_irq_byname (pdev,"int0" ); +int m_can_core_get_clocks (struct m_can_classdev *m_can_dev) +{ +int ret =0 ; -if (IS_ERR (addr) || irq <0 ) { - ret = -EINVAL ; - goto failed_ret; - } + m_can_dev->hclk =devm_clk_get (m_can_dev->dev,"hclk" ); + m_can_dev->cclk =devm_clk_get (m_can_dev->dev,"cclk" ); -/* message ram could be shared */ - res =platform_get_resource_byname (pdev,IORESOURCE_MEM ,"message_ram" ); -if (!res) { +if (IS_ERR (m_can_dev->cclk)) { +dev_err (m_can_dev->dev,"no clock found\n" ); ret = -ENODEV ; - goto failed_ret; } - mram_addr =devm_ioremap (&pdev->dev, res->start,resource_size (res)); -if (!mram_addr) { - ret = -ENOMEM ; - goto failed_ret; - } +return ret; +} +EXPORT_SYMBOL_GPL (m_can_core_get_clocks); -/* get message ram configuration */ - ret =of_property_read_u32_array (np,"bosch,mram-cfg" , - mram_config_vals, -sizeof (mram_config_vals) /4 ); +struct m_can_classdev *m_can_core_allocate_dev (struct device *dev) +{ +struct m_can_classdev *class_dev =NULL ; +u32 mram_config_vals[MRAM_CFG_LEN ]; +struct net_device *net_dev; +u32 tx_fifo_size; +int ret; + + ret =fwnode_property_read_u32_array (dev_fwnode (dev), +"bosch,mram-cfg" , + mram_config_vals, +sizeof (mram_config_vals) /4 );if (ret) { -dev_err (&pdev->dev,"Could not get Message RAM configuration." ); - goto failed_ret; +dev_err (dev,"Could not get Message RAM configuration." ); + goto out; }/* Get TX FIFO size
drivers/net/can/m_can/m_can.c static int m_can_plat_probe(struct platform_device *pdev) · line 1646
Quoted 10 lines
drivers/net/can/m_can/m_can.c +3
-4@@ -1637,69 +1641,77 @@ static int m_can_plat_probe(struct platform_device *pdev) tx_fifo_size = mram_config_vals[7 ];/* allocate the m_can device */ - dev =alloc_candev (sizeof (*priv), tx_fifo_size); -if (!dev) { - ret = -ENOMEM ; - goto failed_ret; + net_dev =alloc_candev (sizeof (*class_dev), tx_fifo_size); +if (!net_dev) { +dev_err (dev,"Failed to allocate CAN device" );
No error message in case of ENOMEM. It makes it worse. Also, it's an unrelated change.
Quoted 5 lines
drivers/net/can/m_can/m_can.c +1
-2+ goto out; } - priv =netdev_priv (dev); - dev->irq = irq;
drivers/net/can/m_can/m_can.c static int m_can_plat_probe(struct platform_device *pdev) · line 1654
Quoted 10 lines
drivers/net/can/m_can/m_can.c +5
-5- priv->device = &pdev->dev; - priv->hclk = hclk; - priv->cclk = cclk; - priv->can.clock.freq =clk_get_rate (cclk); - priv->mram_base = mram_addr; + class_dev =netdev_priv (net_dev); +if (!class_dev) { +dev_err (dev,"Failed to init netdev private" ); + goto out; + }
WARN_ON_ONECE() ?
Quoted 193 lines
drivers/net/can/m_can/m_can.c +44
-94-platform_set_drvdata (pdev, dev); -SET_NETDEV_DEV (dev, &pdev->dev); + class_dev->net = net_dev; + class_dev->dev = dev; +SET_NETDEV_DEV (net_dev, dev); -/* Enable clocks. Necessary to read Core Release in order to determine -* M_CAN version -*/ -pm_runtime_enable (&pdev->dev); - ret =m_can_clk_start (priv); -if (ret) - goto pm_runtime_fail; +m_can_of_parse_mram (class_dev, mram_config_vals); +out: +return class_dev; +} +EXPORT_SYMBOL_GPL (m_can_core_allocate_dev); + +int m_can_core_register (struct m_can_classdev *m_can_dev) +{ +int ret; + +if (m_can_dev->pm_clock_support) { +pm_runtime_enable (m_can_dev->dev); + ret =m_can_clk_start (m_can_dev); +if (ret) + goto pm_runtime_fail; + } - ret =m_can_dev_setup (pdev, dev, addr); + ret =m_can_dev_setup (m_can_dev);if (ret) goto clk_disable; - ret =register_m_can_dev (dev); + ret =register_m_can_dev (m_can_dev->net);if (ret) { -dev_err (&pdev->dev,"registering %s failed (err=%d)\n" , -KBUILD_MODNAME , ret); +dev_err (m_can_dev->dev,"registering %s failed (err=%d)\n" , + m_can_dev->net->name, ret); goto clk_disable; } -m_can_of_parse_mram (priv, mram_config_vals); - -devm_can_led_init (dev); +devm_can_led_init (m_can_dev->net); -of_can_transceiver (dev); +of_can_transceiver (m_can_dev->net); -dev_info (&pdev->dev,"%s device registered (irq=%d, version=%d)\n" , -KBUILD_MODNAME , dev->irq, priv->version); +dev_info (m_can_dev->dev,"%s device registered (irq=%d, version=%d)\n" , +KBUILD_MODNAME , m_can_dev->net->irq, m_can_dev->version);/* Probe finished * Stop clocks. They will be reactivated once the M_CAN device is opened */ clk_disable: -m_can_clk_stop (priv); +m_can_clk_stop (m_can_dev); pm_runtime_fail:if (ret) { -pm_runtime_disable (&pdev->dev); -free_candev (dev); +if (m_can_dev->pm_clock_support) +pm_runtime_disable (m_can_dev->dev); +free_candev (m_can_dev->net); } -failed_ret: +return ret; } +EXPORT_SYMBOL_GPL (m_can_core_register); -static __maybe_unused int m_can_suspend (struct device *dev) +int m_can_core_suspend (struct device *dev) {struct net_device *ndev =dev_get_drvdata (dev); -struct m_can_priv *priv =netdev_priv (ndev); +struct m_can_classdev *priv =netdev_priv (ndev);if (netif_running (ndev)) {netif_stop_queue (ndev);@@ -1714,11 +1726,12 @@ static __maybe_unused int m_can_suspend(struct device *dev)return 0 ; } +EXPORT_SYMBOL_GPL (m_can_core_suspend); -static __maybe_unused int m_can_resume (struct device *dev) +int m_can_core_resume (struct device *dev) {struct net_device *ndev =dev_get_drvdata (dev); -struct m_can_priv *priv =netdev_priv (ndev); +struct m_can_classdev *priv =netdev_priv (ndev);pinctrl_pm_select_default_state (dev);@@ -1739,78 +1752,17 @@ static __maybe_unused int m_can_resume(struct device *dev)return 0 ; } +EXPORT_SYMBOL_GPL (m_can_core_resume); -static void unregister_m_can_dev (struct net_device *dev) -{ -unregister_candev (dev); -} - -static int m_can_plat_remove (struct platform_device *pdev) -{ -struct net_device *dev =platform_get_drvdata (pdev); - -unregister_m_can_dev (dev); - -pm_runtime_disable (&pdev->dev); - -platform_set_drvdata (pdev,NULL ); - -free_candev (dev); - -return 0 ; -} - -static int __maybe_unused m_can_runtime_suspend (struct device *dev) +void m_can_core_unregister (struct m_can_classdev *m_can_dev) { -struct net_device *ndev =dev_get_drvdata (dev); -struct m_can_priv *priv =netdev_priv (ndev); +unregister_candev (m_can_dev->net); -clk_disable_unprepare (priv->cclk); -clk_disable_unprepare (priv->hclk); +m_can_clk_stop (m_can_dev); -return 0 ; +free_candev (m_can_dev->net); } - -static int __maybe_unused m_can_runtime_resume (struct device *dev) -{ -struct net_device *ndev =dev_get_drvdata (dev); -struct m_can_priv *priv =netdev_priv (ndev); -int err; - - err =clk_prepare_enable (priv->hclk); -if (err) -return err; - - err =clk_prepare_enable (priv->cclk); -if (err) -clk_disable_unprepare (priv->hclk); - -return err; -} - -static const struct dev_pm_ops m_can_pmops = { - SET_RUNTIME_PM_OPS(m_can_runtime_suspend, - m_can_runtime_resume, NULL) -SET_SYSTEM_SLEEP_PM_OPS (m_can_suspend, m_can_resume) -}; - -static const struct of_device_id m_can_of_table[] = { - { .compatible ="bosch,m_can" , .data =NULL }, - {/* sentinel */ }, -}; -MODULE_DEVICE_TABLE (of, m_can_of_table); - -static struct platform_driver m_can_plat_driver = { - .driver = { - .name =KBUILD_MODNAME , - .of_match_table = m_can_of_table, - .pm = &m_can_pmops, - }, - .probe = m_can_plat_probe, - .remove = m_can_plat_remove, -}; - -module_platform_driver (m_can_plat_driver); +EXPORT_SYMBOL_GPL (m_can_core_unregister);MODULE_AUTHOR ("Dong Aisheng <[email protected]>" );MODULE_LICENSE ("GPL v2" );drivers/net/can/m_can/m_can_platform.h +0
-0
drivers/net/can/m_can/m_can_platform.h int m_can_core_register(struct m_can_classdev *m_can_dev); · line 162
Quoted 8 lines
drivers/net/can/m_can/m_can_platform.h +1
-1@@ -156,7 +156,7 @@ int m_can_core_register(struct m_can_classdev *m_can_dev);void m_can_core_unregister (struct m_can_classdev *m_can_dev);int m_can_core_get_clocks (struct m_can_classdev *m_can_dev);void m_can_init_ram (struct m_can_classdev *priv); -void m_can_config_endisable (const struct m_can_classdev *priv,bool enable); +void m_can_config_endisable (struct m_can_classdev *priv,bool enable);int m_can_core_suspend (struct device *dev);int m_can_core_resume (struct device *dev);
If you fix the issues with "is_peripheral" and the TX function, it should already work on standard M_CAN devices as before... at least in theory!
Wolfgang.
Re: [PATCH v4 2/4] can: m_can: Migrate the m_can code to use the framework
Wolfgang
Quoted 23 lines · Wolfgang Grandegger
Hello,
Am 17.01.19 um 21:05 schrieb Dan Murphy:
Quoted 20 lines
Migrate the
m_cancode to use them_can_platformframework code.Signed-off-by Dan Murphy <[email protected]>--- drivers/net/can/m_can/Kconfig | 12 + drivers/net/can/m_can/Makefile | 4 +- drivers/net/can/m_can/m_can.c | 764 ++++++++++++------------- drivers/net/can/m_can/m_can_platform.h | 2 +- 4 files changed, 374 insertions(+), 408 deletions(-)drivers/net/can/m_can/Kconfig +3
-0@@ -1,5 +1,17 @@ config CAN_M_CAN + tristate "Bosch M_CAN support" + ---help--- + Say Y here if you want to support for Bosch M_CAN controller.
Quoted 9 lines
drivers/net/can/m_can/Kconfig line 10
Quoted 6 lines
drivers/net/can/m_can/Kconfig +6
-0+ +config CAN_M_CAN_CORE + depends on CAN_M_CAN + tristate "Bosch M_CAN Core support" + ---help--- + Say Y here if you want to support for Bosch M_CAN controller.Do you need that extra config? I think "
CAN_M_CAN" is just fine.
OK I can remove the CORE. That should be built if CAN_M_CAN is selected then
Quoted 23 lines
Quoted 11 lines
drivers/net/can/m_can/Kconfig +2
-0+config CAN_M_CAN_PLATFORM depends on HAS_IOMEM + depends on CAN_M_CAN_CORE tristate "Bosch M_CAN devices" ---help--- Say Y here if you want to support for Bosch M_CAN controller.drivers/net/can/m_can/Makefile +0
-0drivers/net/can/m_can/Makefile line 7
Quoted 7 lines
drivers/net/can/m_can/Makefile +3
-1@@ -2,4 +2,6 @@ # Makefile for the Bosch M_CAN controller driver. # -obj-$(CONFIG_CAN_M_CAN) += m_can.o +obj-$(CONFIG_CAN_M_CAN_CORE) += m_can.o +obj-$(CONFIG_CAN_M_CAN_PLATFORM) += m_can_platform.o +obj-$(CONFIG_CAN_M_CAN_TCAN4X5X) += tcan4x5x.oThis file is provided in a sub-sequent patched! The code *must* compile for every single patch applied for bisect'ing. Looking to your first patch, I just realize that this is also not the case!
Quoted 2 lines
I think it makes sense to squash patch 1 and 2.
Well. Patch 1 should compile on its own I will check that.
Squashing patch 1 and patch 2 made the review even more impossible.
So I made patch 1 to create the io-mapped code (which probably will not compile when enabled) and then patch 2 was the changes to the CORE to stitch in the io-mapped code.
Patch 4 was the addition of the peripherial.
I did this because there was a comment from the RFCs I sent in that there was to much change to review for a single patch conversion. I can squash patch 1 and patch 2 after we finish the review process prior to merging it into the tree.
That was also the comment I received from an internal review.
Quoted 84 lines
Quoted 84 lines
drivers/net/can/m_can/m_can.c +2
-68@@ -28,87 +28,14 @@#include <linux/can/dev.h> #include <linux/pinctrl/consumer.h> +#include "m_can_platform.h" +/* napi related */ #define M_CAN_NAPI_WEIGHT 64/* message ram configuration data length */ #define MRAM_CFG_LEN 8 -/* registers definition */ -enum m_can_reg { -M_CAN_CREL =0x0 , -M_CAN_ENDN =0x4 , -M_CAN_CUST =0x8 , -M_CAN_DBTP =0xc , -M_CAN_TEST =0x10 , -M_CAN_RWD =0x14 , -M_CAN_CCCR =0x18 , -M_CAN_NBTP =0x1c , -M_CAN_TSCC =0x20 , -M_CAN_TSCV =0x24 , -M_CAN_TOCC =0x28 , -M_CAN_TOCV =0x2c , -M_CAN_ECR =0x40 , -M_CAN_PSR =0x44 , -/* TDCR Register only available for version >=3.1.x */ -M_CAN_TDCR =0x48 , -M_CAN_IR =0x50 , -M_CAN_IE =0x54 , -M_CAN_ILS =0x58 , -M_CAN_ILE =0x5c , -M_CAN_GFC =0x80 , -M_CAN_SIDFC =0x84 , -M_CAN_XIDFC =0x88 , -M_CAN_XIDAM =0x90 , -M_CAN_HPMS =0x94 , -M_CAN_NDAT1 =0x98 , -M_CAN_NDAT2 =0x9c , -M_CAN_RXF0C =0xa0 , -M_CAN_RXF0S =0xa4 , -M_CAN_RXF0A =0xa8 , -M_CAN_RXBC =0xac , -M_CAN_RXF1C =0xb0 , -M_CAN_RXF1S =0xb4 , -M_CAN_RXF1A =0xb8 , -M_CAN_RXESC =0xbc , -M_CAN_TXBC =0xc0 , -M_CAN_TXFQS =0xc4 , -M_CAN_TXESC =0xc8 , -M_CAN_TXBRP =0xcc , -M_CAN_TXBAR =0xd0 , -M_CAN_TXBCR =0xd4 , -M_CAN_TXBTO =0xd8 , -M_CAN_TXBCF =0xdc , -M_CAN_TXBTIE =0xe0 , -M_CAN_TXBCIE =0xe4 , -M_CAN_TXEFC =0xf0 , -M_CAN_TXEFS =0xf4 , -M_CAN_TXEFA =0xf8 , -}; - -/* m_can lec values */ -enum m_can_lec_type { -LEC_NO_ERROR =0 , -LEC_STUFF_ERROR , -LEC_FORM_ERROR , -LEC_ACK_ERROR , -LEC_BIT1_ERROR , -LEC_BIT0_ERROR , -LEC_CRC_ERROR , -LEC_UNUSED , -}; - -enum m_can_mram_cfg { -MRAM_SIDF =0 , -MRAM_XIDF , -MRAM_RXF0 ,
Quoted 9 lines
drivers/net/can/m_can/m_can.c line 110
Quoted 6 lines
drivers/net/can/m_can/m_can.c +0
-6-MRAM_RXF1 , -MRAM_RXB , -MRAM_TXE , -MRAM_TXB , - MRAM_CFG_NUM, -};Patch 1 should have already done that!
Will look at it.
Quoted 86 lines
Quoted 73 lines
drivers/net/can/m_can/m_can.c +26
-29/* Core Release Register (CREL) */ #define CREL_REL_SHIFT 28#define CREL_REL_MASK (0xF << CREL_REL_SHIFT)@@ -343,72 +270,81 @@ enum m_can_mram_cfg {#define TX_BUF_MM_MASK (0xff << TX_BUF_MM_SHIFT)/* Tx event FIFO Element */ -/* E1 */ #define TX_EVENT_MM_SHIFT TX_BUF_MM_SHIFT#define TX_EVENT_MM_MASK (0xff << TX_EVENT_MM_SHIFT) -/* address offset and element number for each FIFO/Buffer in the Message RAM */ -struct mram_cfg { -u16 off; -u8 num; -}; +static u32 m_can_read (struct m_can_classdev *priv,enum m_can_reg reg) +{ +u32 ret = -EINVAL ; -/* m_can private data structure */ -struct m_can_priv { -struct can_priv can;/* must be the first member */ -struct napi_struct napi; -struct net_device *dev; -struct device *device; -struct clk *hclk; -struct clk *cclk; -void __iomem *base; -u32 irqstatus; -int version; - -/* message ram configuration */ -void __iomem *mram_base; -struct mram_cfg mcfg[MRAM_CFG_NUM ]; -}; +if (priv->read_reg) + ret = priv->read_reg (priv, reg); -static inline u32 m_can_read (const struct m_can_priv *priv,enum m_can_reg reg) +return ret; +} + +static int m_can_write (struct m_can_classdev *priv,enum m_can_reg reg,u32 val) { -return readl (priv->base + reg); +int ret = -EINVAL ; + +if (priv->write_reg) + ret = priv->write_reg (priv, reg, val); + +return ret; } -static inline void m_can_write (const struct m_can_priv *priv, -enum m_can_reg reg,u32 val) +static u32 m_can_fifo_read (struct m_can_classdev *priv, +u32 fgi,unsigned int offset) { -writel (val, priv->base + reg); +u32 addr_offset = priv->mcfg[MRAM_RXF0 ].off + fgi *RXF0_ELEMENT_SIZE + offset; +u32 ret = -EINVAL ; + +if (priv->read_fifo) + ret = priv->read_fifo (priv, addr_offset); + +return ret; } -static inline u32 m_can_fifo_read (const struct m_can_priv *priv, -u32 fgi,unsigned int offset) +static u32 m_can_fifo_write (struct m_can_classdev *priv, +u32 fpi,unsigned int offset,u32 val)drivers/net/can/m_can/m_can.c enum m_can_mram_cfg { · line 317
Quoted 10 lines
drivers/net/can/m_can/m_can.c +7
-2{ -return readl (priv->mram_base + priv->mcfg[MRAM_RXF0 ].off + - fgi *RXF0_ELEMENT_SIZE + offset); +u32 addr_offset = priv->mcfg[MRAM_TXB ].off + fpi *TXB_ELEMENT_SIZE + offset; +u32 ret = -EINVAL ; + +if (priv->write_fifo) + ret = priv->write_fifo (priv, addr_offset, val); + +return ret;Why not just:
Quoted 7 lines
if (priv->write_fifo) return priv->write_fifo(priv, addr_offset, val); else return -EINVAL;Here and below...
Ack.
Quoted 109 lines
Quoted 99 lines
drivers/net/can/m_can/m_can.c +26
-19} -static inline void m_can_fifo_write (const struct m_can_priv *priv, -u32 fpi,unsigned int offset,u32 val) +static u32 m_can_fifo_write_no_off (struct m_can_classdev *priv, +u32 fpi,u32 val) { -writel (val, priv->mram_base + priv->mcfg[MRAM_TXB ].off + - fpi *TXB_ELEMENT_SIZE + offset); +u32 ret =0 ; + +if (priv->write_fifo) + ret = priv->write_fifo (priv, fpi, val); + +return ret; } -static inline u32 m_can_txe_fifo_read (const struct m_can_priv *priv, -u32 fgi, -u32 offset) { -return readl (priv->mram_base + priv->mcfg[MRAM_TXE ].off + - fgi *TXE_ELEMENT_SIZE + offset); +static u32 m_can_txe_fifo_read (struct m_can_classdev *priv,u32 fgi,u32 offset) +{ +u32 addr_offset = priv->mcfg[MRAM_TXE ].off + fgi *TXE_ELEMENT_SIZE + offset; +u32 ret = -EINVAL ; + +if (priv->read_fifo) + ret = priv->read_fifo (priv, addr_offset); + +return ret; } -static inline bool m_can_tx_fifo_full (const struct m_can_priv *priv) +static inline bool m_can_tx_fifo_full (struct m_can_classdev *priv) { -return !!(m_can_read (priv,M_CAN_TXFQS ) &TXFQS_TFQF ); +return !!(m_can_read (priv,M_CAN_TXFQS ) &TXFQS_TFQF ); } -static inline void m_can_config_endisable (const struct m_can_priv *priv, -bool enable) +void m_can_config_endisable (struct m_can_classdev *priv,bool enable) {u32 cccr =m_can_read (priv,M_CAN_CCCR );u32 timeout =10 ;@@ -433,7 +369,7 @@ static inline void m_can_config_endisable(const struct m_can_priv *priv,while ((m_can_read (priv,M_CAN_CCCR ) & (CCCR_INIT |CCCR_CCE )) != val) {if (timeout ==0 ) { -netdev_warn (priv->dev,"Failed to init module\n" ); +netdev_warn (priv->net,"Failed to init module\n" );return ; } timeout--;@@ -441,13 +377,13 @@ static inline void m_can_config_endisable(const struct m_can_priv *priv, } } -static inline void m_can_enable_all_interrupts (const struct m_can_priv *priv) +static inline void m_can_enable_all_interrupts (struct m_can_classdev *priv) {/* Only interrupt line 0 is used in this driver */ m_can_write (priv,M_CAN_ILE ,ILE_EINT0 ); } -static inline void m_can_disable_all_interrupts (const struct m_can_priv *priv) +static inline void m_can_disable_all_interrupts (struct m_can_classdev *priv) {m_can_write (priv,M_CAN_ILE ,0x0 ); }@@ -455,7 +391,7 @@ static inline void m_can_disable_all_interrupts(const struct m_can_priv *priv)static void m_can_read_fifo (struct net_device *dev,u32 rxfs) {struct net_device_stats *stats = &dev->stats; -struct m_can_priv *priv =netdev_priv (dev); +struct m_can_classdev *priv =netdev_priv (dev);struct canfd_frame *cf;struct sk_buff *skb;u32 id, fgi, dlc;@@ -512,7 +448,7 @@ static void m_can_read_fifo(struct net_device *dev, u32 rxfs)static int m_can_do_rx_poll (struct net_device *dev,int quota) { -struct m_can_priv *priv =netdev_priv (dev); +struct m_can_classdev *priv =netdev_priv (dev);u32 pkts =0 ;u32 rxfs;@@ -565,7 +501,7 @@ static int m_can_handle_lost_msg(struct net_device *dev)static int m_can_handle_lec_err (struct net_device *dev,enum m_can_lec_type lec_type) { -struct m_can_priv *priv =netdev_priv (dev); +struct m_can_classdev *priv =netdev_priv (dev);struct net_device_stats *stats = &dev->stats;struct can_frame *cf;struct sk_buff *skb;drivers/net/can/m_can/m_can.c static int m_can_handle_lec_err(struct net_device *dev, · line 561
Quoted 5 lines
drivers/net/can/m_can/m_can.c +1
-1@@ -622,7 +558,7 @@ static int m_can_handle_lec_err(struct net_device *dev,static int __m_can_get_berr_counter (const struct net_device *dev,struct can_berr_counter *bec) { -struct m_can_priv *priv =netdev_priv (dev); +struct m_can_classdev *priv =netdev_priv (dev);Back to the naming: You use "priv" here because I only wanted to see minimal changes to the
m_cancode. Why not also re-using "structm_can_priv" for the moment and use:
Quoted 5 lines
m_can_register(struct m_can_priv priv);Later-on, when the development has settled, we could introduce betetr names. What do you think?
m_can_classdev is the common struct for the devices that register. m_can_priv is the private struct for the device itself. If you look at the m_can_priv struct and the tcan_priv struct they contain very different variables.
This is why I changed this to the common class struct so that the m_can common code has a common struct.
Quoted 59 lines
Quoted 59 lines
drivers/net/can/m_can/m_can.c +12
-8unsigned int ecr; ecr =m_can_read (priv,M_CAN_ECR );@@ -632,28 +568,32 @@ static int __m_can_get_berr_counter(const struct net_device *dev,return 0 ; } -static int m_can_clk_start (struct m_can_priv *priv) +static int m_can_clk_start (struct m_can_classdev *priv) {int err; - err =pm_runtime_get_sync (priv->device); +if (priv->pm_clock_support ==0 ) +return 0 ; + + err =pm_runtime_get_sync (priv->dev);if (err <0 ) { -pm_runtime_put_noidle (priv->device); +pm_runtime_put_noidle (priv->dev);return err; }return 0 ; } -static void m_can_clk_stop (struct m_can_priv *priv) +static void m_can_clk_stop (struct m_can_classdev *priv) { -pm_runtime_put_sync (priv->device); +if (priv->pm_clock_support) +pm_runtime_put_sync (priv->dev); }static int m_can_get_berr_counter (const struct net_device *dev,struct can_berr_counter *bec) { -struct m_can_priv *priv =netdev_priv (dev); +struct m_can_classdev *priv =netdev_priv (dev);int err; err =m_can_clk_start (priv);@@ -670,7 +610,7 @@ static int m_can_get_berr_counter(const struct net_device *dev,static int m_can_handle_state_change (struct net_device *dev,enum can_state new_state) { -struct m_can_priv *priv =netdev_priv (dev); +struct m_can_classdev *priv =netdev_priv (dev);struct net_device_stats *stats = &dev->stats;struct can_frame *cf;struct sk_buff *skb;@@ -744,25 +684,22 @@ static int m_can_handle_state_change(struct net_device *dev,static int m_can_handle_state_errors (struct net_device *dev,u32 psr) { -struct m_can_priv *priv =netdev_priv (dev); +struct m_can_classdev *priv =netdev_priv (dev);int work_done =0 ;
Quoted 7 lines
drivers/net/can/m_can/m_can.c static int m_can_handle_state_change(struct net_device *dev, · line 690
Quoted 3 lines
drivers/net/can/m_can/m_can.c +1
-2-if ((psr &PSR_EW ) && - (priv->can.state !=CAN_STATE_ERROR_WARNING )) { +if ((psr &PSR_EW ) && priv->can.state !=CAN_STATE_ERROR_WARNING ) {This is an unrelated cosmetic change. We should avoid them here. Could be done later-on with an extra patch.
Ack
Quoted 43 lines
Quoted 43 lines
drivers/net/can/m_can/m_can.c +5
-8netdev_dbg (dev,"entered error warning state\n" ); work_done +=m_can_handle_state_change (dev,CAN_STATE_ERROR_WARNING ); } -if ((psr &PSR_EP ) && - (priv->can.state !=CAN_STATE_ERROR_PASSIVE )) { +if ((psr &PSR_EP ) && priv->can.state !=CAN_STATE_ERROR_PASSIVE ) {netdev_dbg (dev,"entered error passive state\n" ); work_done +=m_can_handle_state_change (dev,CAN_STATE_ERROR_PASSIVE ); } -if ((psr &PSR_BO ) && - (priv->can.state !=CAN_STATE_BUS_OFF )) { +if ((psr &PSR_BO ) && priv->can.state !=CAN_STATE_BUS_OFF ) {netdev_dbg (dev,"entered error bus off state\n" ); work_done +=m_can_handle_state_change (dev,CAN_STATE_BUS_OFF );@@ -797,7 +734,7 @@ static inline bool is_lec_err(u32 psr)static int m_can_handle_bus_errors (struct net_device *dev,u32 irqstatus,u32 psr) { -struct m_can_priv *priv =netdev_priv (dev); +struct m_can_classdev *priv =netdev_priv (dev);int work_done =0 ;if (irqstatus &IR_RF0L )@@ -814,10 +751,9 @@ static int m_can_handle_bus_errors(struct net_device *dev, u32 irqstatus,return work_done; } -static int m_can_poll (struct napi_struct *napi,int quota) +static int m_can_rx_handler (struct net_device *dev,int quota) { -struct net_device *dev = napi->dev; -struct m_can_priv *priv =netdev_priv (dev); +struct m_can_classdev *priv =netdev_priv (dev);int work_done =0 ;u32 irqstatus, psr;@@ -834,13 +770,33 @@ static int m_can_poll(struct napi_struct *napi, int quota)
Quoted 10 lines
drivers/net/can/m_can/m_can.c static int m_can_poll(struct napi_struct *napi, int quota) · line 777
Quoted 7 lines
drivers/net/can/m_can/m_can.c +5
-0if (irqstatus &IR_RF0N ) work_done +=m_can_do_rx_poll (dev, (quota - work_done)); +end: +return work_done; +} + +static int m_can_rx (struct net_device *dev)
m_can_rx_peripheral?
Ack
Quoted 134 lines
Quoted 134 lines
drivers/net/can/m_can/m_can.c +34
-15+{ +struct m_can_classdev *priv =netdev_priv (dev); +m_can_rx_handler (dev,1 ); + +m_can_enable_all_interrupts (priv); + +return 0 ; +} + +static int m_can_poll (struct napi_struct *napi,int quota) +{ +struct net_device *dev = napi->dev; +struct m_can_classdev *priv =netdev_priv (dev); +int work_done =0 ; + + work_done =m_can_rx_handler (dev, quota);if (work_done < quota) {napi_complete_done (napi, work_done);m_can_enable_all_interrupts (priv); } -end:return work_done; }@@ -852,7 +808,7 @@ static void m_can_echo_tx_event(struct net_device *dev)int i =0 ;unsigned int msg_mark; -struct m_can_priv *priv =netdev_priv (dev); +struct m_can_classdev *priv =netdev_priv (dev);struct net_device_stats *stats = &dev->stats;/* read tx event fifo status */ @@ -885,7 +841,7 @@ static void m_can_echo_tx_event(struct net_device *dev)static irqreturn_t m_can_isr (int irq,void *dev_id) {struct net_device *dev = (struct net_device *)dev_id; -struct m_can_priv *priv =netdev_priv (dev); +struct m_can_classdev *priv =netdev_priv (dev);struct net_device_stats *stats = &dev->stats;u32 ir;@@ -905,7 +861,10 @@ static irqreturn_t m_can_isr(int irq, void *dev_id)if ((ir &IR_RF0N ) || (ir &IR_ERR_ALL_30X )) { priv->irqstatus = ir;m_can_disable_all_interrupts (priv); -napi_schedule (&priv->napi); +if (!priv->is_peripherial) +napi_schedule (&priv->napi); +else +m_can_rx (dev); }if (priv->version ==30 ) {@@ -927,6 +886,9 @@ static irqreturn_t m_can_isr(int irq, void *dev_id) } } +if (priv->clr_dev_interrupts) + priv->clr_dev_interrupts (priv); +return IRQ_HANDLED ; }@@ -980,7 +942,7 @@ static const struct can_bittiming_const m_can_data_bittiming_const_31X = {static int m_can_set_bittiming (struct net_device *dev) { -struct m_can_priv *priv =netdev_priv (dev); +struct m_can_classdev *priv =netdev_priv (dev);const struct can_bittiming *bt = &priv->can.bittiming;const struct can_bittiming *dbt = &priv->can.data_bittiming;u16 brp, sjw, tseg1, tseg2;@@ -1053,7 +1015,7 @@ static int m_can_set_bittiming(struct net_device *dev)*/ static void m_can_chip_config (struct net_device *dev) { -struct m_can_priv *priv =netdev_priv (dev); +struct m_can_classdev *priv =netdev_priv (dev);u32 cccr, test;m_can_config_endisable (priv, true);@@ -1165,7 +1127,7 @@ static void m_can_chip_config(struct net_device *dev)static void m_can_start (struct net_device *dev) { -struct m_can_priv *priv =netdev_priv (dev); +struct m_can_classdev *priv =netdev_priv (dev);/* basic m_can configuration */ m_can_chip_config (dev);@@ -1194,20 +1156,17 @@ static int m_can_set_mode(struct net_device *dev, enum can_mode mode)* else it returns the release and step coded as: * return value = 10 * <release> + 1 * <step> */ -static int m_can_check_core_release (void __iomem *m_can_base) +static int m_can_check_core_release (struct m_can_classdev *priv) {u32 crel_reg;u8 rel;u8 step;int res; -struct m_can_priv temp_priv = { - .base = m_can_base - };/* Read Core Release Version and split into version number * Example: Version 3.2.1 => rel = 3; step = 2; substep = 1; */ - crel_reg =m_can_read (&temp_priv,M_CAN_CREL ); + crel_reg =m_can_read (priv,M_CAN_CREL ); rel = (u8)((crel_reg &CREL_REL_MASK ) >>CREL_REL_SHIFT ); step = (u8)((crel_reg &CREL_STEP_MASK ) >>CREL_STEP_SHIFT );@@ -1225,18 +1184,22 @@ static int m_can_check_core_release(void __iomem *m_can_base)/* Selectable Non ISO support only in version 3.2.x * This function checks if the bit is writable. */ -static bool m_can_niso_supported (const struct m_can_priv *priv) +static bool m_can_niso_supported (struct m_can_classdev *priv) { -u32 cccr_reg, cccr_poll; -int niso_timeout; +u32 cccr_reg, cccr_poll =0 ; +int niso_timeout = -ETIMEDOUT ; +int i;m_can_config_endisable (priv, true); cccr_reg =m_can_read (priv,M_CAN_CCCR ); cccr_reg |=CCCR_NISO ;m_can_write (priv,M_CAN_CCCR , cccr_reg);
Quoted 10 lines
drivers/net/can/m_can/m_can.c static int m_can_check_core_release(void __iomem *m_can_base) · line 1202
Quoted 7 lines
drivers/net/can/m_can/m_can.c +5
-2- niso_timeout =readl_poll_timeout ((priv->base +M_CAN_CCCR ), cccr_poll, - (cccr_poll == cccr_reg),0 ,10 ); +for (i =0 ; i <=10 ; i++) { + cccr_poll =m_can_read (priv,M_CAN_CCCR ); +if (cccr_poll == cccr_reg) + niso_timeout =0 ; + }This change is also unrelated. Should be done in an extra patch.
Actually it is not. readl_poll_timeout is not supported in peripherial devices. I had to re-write this to poll the periherial devices. It worked fine for io-mapped devices.
Quoted 60 lines
Quoted 60 lines
drivers/net/can/m_can/m_can.c +17
-16/* Clear NISO */ cccr_reg &= ~(CCCR_NISO );@@ -1248,112 +1211,100 @@ static bool m_can_niso_supported(const struct m_can_priv *priv)return !niso_timeout; } -static int m_can_dev_setup (struct platform_device *pdev,struct net_device *dev, -void __iomem *addr) +static int m_can_dev_setup (struct m_can_classdev *m_can_dev) { -struct m_can_priv *priv; +struct net_device *dev = m_can_dev->net;int m_can_version; - m_can_version =m_can_check_core_release (addr); + m_can_version =m_can_check_core_release (m_can_dev);/* return if unsupported version */ if (!m_can_version) { -dev_err (&pdev->dev,"Unsupported version number: %2d" , +dev_err (m_can_dev->dev,"Unsupported version number: %2d" , m_can_version);return -EINVAL ; } - priv =netdev_priv (dev); -netif_napi_add (dev, &priv->napi, m_can_poll,M_CAN_NAPI_WEIGHT ); +if (!m_can_dev->is_peripherial) +netif_napi_add (dev, &m_can_dev->napi, + m_can_poll,M_CAN_NAPI_WEIGHT );/* Shared properties of all M_CAN versions */ - priv->version = m_can_version; - priv->dev = dev; - priv->base = addr; - priv->can.do_set_mode = m_can_set_mode; - priv->can.do_get_berr_counter = m_can_get_berr_counter; + m_can_dev->version = m_can_version; + m_can_dev->can.do_set_mode = m_can_set_mode; + m_can_dev->can.do_get_berr_counter = m_can_get_berr_counter;/* Set M_CAN supported operations */ - priv->can.ctrlmode_supported =CAN_CTRLMODE_LOOPBACK | + m_can_dev->can.ctrlmode_supported =CAN_CTRLMODE_LOOPBACK |CAN_CTRLMODE_LISTENONLY |CAN_CTRLMODE_BERR_REPORTING |CAN_CTRLMODE_FD ;/* Set properties depending on M_CAN version */ -switch (priv->version) { +switch (m_can_dev->version) {case 30 :/* CAN_CTRLMODE_FD_NON_ISO is fixed with M_CAN IP v3.0.x */ can_set_static_ctrlmode (dev,CAN_CTRLMODE_FD_NON_ISO ); - priv->can.bittiming_const = &m_can_bittiming_const_30X; - priv->can.data_bittiming_const = +if (m_can_dev->bit_timing) + m_can_dev->can.bittiming_const = m_can_dev->bit_timing; +else + m_can_dev->can.bittiming_const = + &m_can_bittiming_const_30X;
Quoted 9 lines
drivers/net/can/m_can/m_can.c static bool m_can_niso_supported(const struct m_can_priv *priv) · line 1257
Quoted 6 lines
drivers/net/can/m_can/m_can.c +5
-0+if (m_can_dev->data_timing) + m_can_dev->can.data_bittiming_const = + m_can_dev->data_timing; +else + m_can_dev->can.data_bittiming_const = &m_can_data_bittiming_const_30X;Should'nt that go to
m_can_platform.c?
No. This is the original code. I added the ability for the peripherials to over ride the default bit timings provided.
I had to do this because TCAN needed different bit and data timings.
Quoted 53 lines
Quoted 29 lines
drivers/net/can/m_can/m_can.c +18
-4break ;case 31 :/* CAN_CTRLMODE_FD_NON_ISO is fixed with M_CAN IP v3.1.x */ can_set_static_ctrlmode (dev,CAN_CTRLMODE_FD_NON_ISO ); - priv->can.bittiming_const = &m_can_bittiming_const_31X; - priv->can.data_bittiming_const = +if (m_can_dev->bit_timing) + m_can_dev->can.bittiming_const = m_can_dev->bit_timing; +else + m_can_dev->can.bittiming_const = + &m_can_bittiming_const_31X; +if (m_can_dev->data_timing) + m_can_dev->can.data_bittiming_const = + m_can_dev->data_timing; +else + m_can_dev->can.data_bittiming_const = &m_can_data_bittiming_const_31X;break ;case 32 : - priv->can.bittiming_const = &m_can_bittiming_const_31X; - priv->can.data_bittiming_const = +if (m_can_dev->bit_timing) + m_can_dev->can.bittiming_const = m_can_dev->bit_timing; +else + m_can_dev->can.bittiming_const = + &m_can_bittiming_const_31X; + +if (m_can_dev->data_timing) + m_can_dev->can.data_bittiming_const =drivers/net/can/m_can/m_can.c static bool m_can_niso_supported(const struct m_can_priv *priv) · line 1291
Quoted 10 lines
drivers/net/can/m_can/m_can.c +6
-1+ m_can_dev->data_timing; + else + m_can_dev->can.data_bittiming_const = &m_can_data_bittiming_const_31X; - priv->can.ctrlmode_supported |= (m_can_niso_supported (priv) + + m_can_dev->can.ctrlmode_supported |= + (m_can_niso_supported (m_can_dev) ?CAN_CTRLMODE_FD_NON_ISO :0 );if (m_can_niso_supported(m_can_dev) m_can_dev->can.ctrlmode_supported |= CAN_CTRLMODE_FD_NON_ISO;Quoted 9 lines
drivers/net/can/m_can/m_can.c +2
-2break ;default : -dev_err (&pdev->dev,"Unsupported version number: %2d" , - priv->version); +dev_err (m_can_dev->dev,"Unsupported version number: %2d" , + m_can_dev->version);return -EINVAL ; }
Quoted 8 lines
drivers/net/can/m_can/m_can.c static bool m_can_niso_supported(const struct m_can_priv *priv) · line 1314
Quoted 4 lines
drivers/net/can/m_can/m_can.c +0
-4-return 0 ; -} - -static int m_can_open (struct net_device *dev)This function has been moved around making it difficult to understand the diffs.
Yes it had to be broken up as some of the calls needed to be done in the peripherial and some are common and can be done here.
Quoted 55 lines
Quoted 55 lines
drivers/net/can/m_can/m_can.c +4
-37-{ -struct m_can_priv *priv =netdev_priv (dev); -int err; - - err =m_can_clk_start (priv); -if (err) -return err; - -/* open the can device */ - err =open_candev (dev); -if (err) { -netdev_err (dev,"failed to open can device\n" ); - goto exit_disable_clks; - } - -/* register interrupt handler */ - err =request_irq (dev->irq, m_can_isr,IRQF_SHARED , dev->name, - dev); -if (err <0 ) { -netdev_err (dev,"failed to request interrupt\n" ); - goto exit_irq_fail; - } - -/* start the m_can controller */ -m_can_start (dev); - -can_led_event (dev,CAN_LED_EVENT_OPEN ); -napi_enable (&priv->napi); -netif_start_queue (dev); +if (m_can_dev->device_init) + m_can_dev->device_init (m_can_dev);return 0 ; - -exit_irq_fail: -close_candev (dev); -exit_disable_clks: -m_can_clk_stop (priv); -return err; }static void m_can_stop (struct net_device *dev) { -struct m_can_priv *priv =netdev_priv (dev); +struct m_can_classdev *priv =netdev_priv (dev);/* disable all interrupts */ m_can_disable_all_interrupts (priv);@@ -1364,13 +1315,16 @@ static void m_can_stop(struct net_device *dev)static int m_can_close (struct net_device *dev) { -struct m_can_priv *priv =netdev_priv (dev); +struct m_can_classdev *priv =netdev_priv (dev);
Quoted 10 lines
drivers/net/can/m_can/m_can.c static void m_can_stop(struct net_device *dev) · line 1326
Quoted 8 lines
drivers/net/can/m_can/m_can.c +3
-1netif_stop_queue (dev); -napi_disable (&priv->napi); +if (!priv->is_peripherial) +napi_disable (&priv->napi);m_can_stop (dev);m_can_clk_stop (priv);free_irq (dev->irq, dev); +destroy_workqueue (priv->wq);if (
priv->is_peripherial) ?
This workqueue is created for both peripherial and io-mapped
Quoted 18 lines
Quoted 18 lines
drivers/net/can/m_can/m_can.c +2
-1+ priv->wq =NULL ;close_candev (dev);can_led_event (dev,CAN_LED_EVENT_STOP );@@ -1379,7 +1333,7 @@ static int m_can_close(struct net_device *dev)static int m_can_next_echo_skb_occupied (struct net_device *dev,int putidx) { -struct m_can_priv *priv =netdev_priv (dev); +struct m_can_classdev *priv =netdev_priv (dev);/*get wrap around for loopback skb index */ unsigned int wrap = priv->can.echo_skb_max;int next_idx;@@ -1391,18 +1345,17 @@ static int m_can_next_echo_skb_occupied(struct net_device *dev, int putidx)return !!priv->can.echo_skb[next_idx]; }
Quoted 4 lines
drivers/net/can/m_can/m_can.c static int m_can_next_echo_skb_occupied(struct net_device *dev, int putidx) · line 1395
Quoted 2 lines
drivers/net/can/m_can/m_can.c +0
-2-static netdev_tx_t m_can_start_xmit (struct sk_buff *skb, -struct net_device *dev)Ditto
Same as above
Quoted 5 lines
drivers/net/can/m_can/m_can.c static int m_can_next_echo_skb_occupied(struct net_device *dev, int putidx) · line 1348
Quoted 2 lines
drivers/net/can/m_can/m_can.c +1
-0+static void m_can_tx_work_handler (struct work_struct *ws)I think you need a common function to do the tx.
This is the common function that was in the original code.
Quoted 22 lines
Quoted 22 lines
drivers/net/can/m_can/m_can.c +5
-5{ -struct m_can_priv *priv =netdev_priv (dev); -struct canfd_frame *cf = (struct canfd_frame *)skb->data; + struct m_can_classdev *priv = container_of(ws, struct m_can_classdev, + tx_work); +struct canfd_frame *cf = (struct canfd_frame *)priv->skb->data; +struct net_device *dev = priv->net; +struct sk_buff *skb = priv->skb;u32 id, cccr, fdflags;int i;int putidx; -if (can_dropped_invalid_skb (dev, skb)) -return NETDEV_TX_OK ; -/* Generate ID field for TX buffer Element */ /* Common to all supported M_CAN versions */ if (cf->can_id &CAN_EFF_FLAG ) {@@ -1431,7 +1384,8 @@ static netdev_tx_t m_can_start_xmit(struct sk_buff *skb,can_put_echo_skb (skb, dev,0 );
Quoted 7 lines
drivers/net/can/m_can/m_can.c static netdev_tx_t m_can_start_xmit(struct sk_buff *skb, · line 1388
Quoted 4 lines
drivers/net/can/m_can/m_can.c +2
-1if (priv->can.ctrlmode &CAN_CTRLMODE_FD ) { - cccr =m_can_read (priv,M_CAN_CCCR ); +/*cccr = m_can_read(priv, M_CAN_CCCR);*/ + cccr =0 ;Unrelated change.
Ah yes debug code. Can be removed.
Quoted 58 lines
Quoted 24 lines
drivers/net/can/m_can/m_can.c +5
-4cccr &= ~(CCCR_CMR_MASK <<CCCR_CMR_SHIFT );if (can_is_canfd_skb (skb)) {if (cf->flags &CANFD_BRS )@@ -1457,7 +1411,7 @@ static netdev_tx_t m_can_start_xmit(struct sk_buff *skb,netif_stop_queue (dev);netdev_warn (dev,"TX queue active although FIFO is full." ); -return NETDEV_TX_BUSY ; +return ; }/* get put index for frame */ @@ -1498,14 +1452,87 @@ static netdev_tx_t m_can_start_xmit(struct sk_buff *skb,m_can_write (priv,M_CAN_TXBAR , (1 << putidx));/* stop network queue if fifo full */ -if (m_can_tx_fifo_full (priv) || -m_can_next_echo_skb_occupied (dev, putidx)) -netif_stop_queue (dev); +if (m_can_tx_fifo_full (priv) || +m_can_next_echo_skb_occupied (dev, putidx)) +netif_stop_queue (dev); } +}drivers/net/can/m_can/m_can.c static netdev_tx_t m_can_start_xmit(struct sk_buff *skb, · line 1469
Quoted 10 lines
drivers/net/can/m_can/m_can.c +10
-0+ +static netdev_tx_t m_can_start_xmit (struct sk_buff *skb, +struct net_device *dev) +{ +struct m_can_classdev *priv =netdev_priv (dev); + +if (can_dropped_invalid_skb (dev, skb)) +return NETDEV_TX_BUSY ; + +netif_stop_queue (dev);if (!priv->is_peripheral) { m_can_tx(priv, skb); } else { netif_stop_queue(dev); priv->skb = skb; queue_work(priv->wq, &priv->tx_work); }I think stopping the queue here is still experimental.
Quoted 13 lines
drivers/net/can/m_can/m_can.c +10
-0return NETDEV_TX_OK ; } +static int m_can_open (struct net_device *dev) +{ +struct m_can_classdev *priv =netdev_priv (dev); +int err; + + err =m_can_clk_start (priv); +if (err) +return err; + +/* open the can device */
Quoted 9 lines
drivers/net/can/m_can/m_can.c static netdev_tx_t m_can_start_xmit(struct sk_buff *skb, · line 1491
Quoted 6 lines
drivers/net/can/m_can/m_can.c +6
-0+ err =open_candev (dev); +if (err) { +netdev_err (dev,"failed to open can device\n" ); + goto exit_disable_clks; + } +if (priv->is_peripheral) {
Not here this is needed. It is original code
Quoted 187 lines
drivers/net/can/m_can/m_can.c static netdev_tx_t m_can_start_xmit(struct sk_buff *skb, · line 1499
Quoted 8 lines
drivers/net/can/m_can/m_can.c +8
-0+ priv->wq =alloc_workqueue ("mcan_wq" ,WQ_FREEZABLE |WQ_MEM_RECLAIM , +0 ); +if (!priv->wq) { + err = -ENOMEM ; + goto out_wq_fail; + } + +INIT_WORK (&priv->tx_work, m_can_tx_work_handler);}Quoted 176 lines
drivers/net/can/m_can/m_can.c +74
-62+/* register interrupt handler */ +if (priv->is_peripherial) + err =request_threaded_irq (dev->irq,NULL , m_can_isr, +IRQF_ONESHOT |IRQF_TRIGGER_FALLING , + dev->name, dev); +else + err =request_irq (dev->irq, m_can_isr,IRQF_SHARED , dev->name, + dev); + +if (err <0 ) { +netdev_err (dev,"failed to request interrupt\n" ); + goto exit_irq_fail; + } + +/* start the m_can controller */ +m_can_start (dev); + +can_led_event (dev,CAN_LED_EVENT_OPEN ); + +if (!priv->is_peripherial) +napi_enable (&priv->napi); + +netif_start_queue (dev); + +return 0 ; + +exit_irq_fail: +destroy_workqueue (priv->wq); +out_wq_fail: +close_candev (dev); +exit_disable_clks: +m_can_clk_stop (priv); +return err; +} +static const struct net_device_ops m_can_netdev_ops = { .ndo_open = m_can_open, .ndo_stop = m_can_close,@@ -1521,21 +1548,7 @@ static int register_m_can_dev(struct net_device *dev)return register_candev (dev); } -static void m_can_init_ram (struct m_can_priv *priv) -{ -int end, i, start; - -/* initialize the entire Message RAM in use to avoid possible -* ECC/parity checksum errors when reading an uninitialized buffer -*/ - start = priv->mcfg[MRAM_SIDF ].off; - end = priv->mcfg[MRAM_TXB ].off + - priv->mcfg[MRAM_TXB ].num *TXB_ELEMENT_SIZE ; -for (i = start; i < end; i +=4 ) -writel (0x0 , priv->mram_base + i); -} - -static void m_can_of_parse_mram (struct m_can_priv *priv, +static void m_can_of_parse_mram (struct m_can_classdev *priv,const u32 *mram_config_vals) { priv->mcfg[MRAM_SIDF ].off = mram_config_vals[0 ];@@ -1562,9 +1575,8 @@ static void m_can_of_parse_mram(struct m_can_priv *priv, priv->mcfg[MRAM_TXB ].num = mram_config_vals[7 ] & (TXBC_NDTB_MASK >>TXBC_NDTB_SHIFT ); - dev_dbg(priv->device, - "mram_base %p sidf 0x%x %d xidf 0x%x %d rxf0 0x%x %d rxf1 0x%x %d rxb 0x%x %d txe 0x%x %d txb 0x%x %d\n", - priv->mram_base, + dev_dbg(priv->dev, + "sidf 0x%x %d xidf 0x%x %d rxf0 0x%x %d rxf1 0x%x %d rxb 0x%x %d txe 0x%x %d txb 0x%x %d\n", priv->mcfg[MRAM_SIDF].off, priv->mcfg[MRAM_SIDF].num, priv->mcfg[MRAM_XIDF].off, priv->mcfg[MRAM_XIDF].num, priv->mcfg[MRAM_RXF0].off, priv->mcfg[MRAM_RXF0].num,@@ -1572,63 +1584,55 @@ static void m_can_of_parse_mram(struct m_can_priv *priv, priv->mcfg[MRAM_RXB ].off, priv->mcfg[MRAM_RXB ].num, priv->mcfg[MRAM_TXE ].off, priv->mcfg[MRAM_TXE ].num, priv->mcfg[MRAM_TXB].off, priv->mcfg[MRAM_TXB].num); - -m_can_init_ram (priv); } -static int m_can_plat_probe (struct platform_device *pdev) +void m_can_init_ram (struct m_can_classdev *priv) { -struct net_device *dev; -struct m_can_priv *priv; -struct resource *res; -void __iomem *addr; -void __iomem *mram_addr; -struct clk *hclk, *cclk; -int irq, ret; -struct device_node *np; -u32 mram_config_vals[MRAM_CFG_LEN ]; -u32 tx_fifo_size; - - np = pdev->dev.of_node; +int end, i, start; - hclk =devm_clk_get (&pdev->dev,"hclk" ); - cclk =devm_clk_get (&pdev->dev,"cclk" ); +/* initialize the entire Message RAM in use to avoid possible +* ECC/parity checksum errors when reading an uninitialized buffer +*/ + start = priv->mcfg[MRAM_SIDF ].off; + end = priv->mcfg[MRAM_TXB ].off + + priv->mcfg[MRAM_TXB ].num *TXB_ELEMENT_SIZE ; -if (IS_ERR (hclk) ||IS_ERR (cclk)) { -dev_err (&pdev->dev,"no clock found\n" ); - ret = -ENODEV ; - goto failed_ret; - } +for (i = start; i < end; i +=4 ) +m_can_fifo_write_no_off (priv, i,0x0 ); +} +EXPORT_SYMBOL_GPL (m_can_init_ram); - res =platform_get_resource_byname (pdev,IORESOURCE_MEM ,"m_can" ); - addr =devm_ioremap_resource (&pdev->dev, res); - irq =platform_get_irq_byname (pdev,"int0" ); +int m_can_core_get_clocks (struct m_can_classdev *m_can_dev) +{ +int ret =0 ; -if (IS_ERR (addr) || irq <0 ) { - ret = -EINVAL ; - goto failed_ret; - } + m_can_dev->hclk =devm_clk_get (m_can_dev->dev,"hclk" ); + m_can_dev->cclk =devm_clk_get (m_can_dev->dev,"cclk" ); -/* message ram could be shared */ - res =platform_get_resource_byname (pdev,IORESOURCE_MEM ,"message_ram" ); -if (!res) { +if (IS_ERR (m_can_dev->cclk)) { +dev_err (m_can_dev->dev,"no clock found\n" ); ret = -ENODEV ; - goto failed_ret; } - mram_addr =devm_ioremap (&pdev->dev, res->start,resource_size (res)); -if (!mram_addr) { - ret = -ENOMEM ; - goto failed_ret; - } +return ret; +} +EXPORT_SYMBOL_GPL (m_can_core_get_clocks); -/* get message ram configuration */ - ret =of_property_read_u32_array (np,"bosch,mram-cfg" , - mram_config_vals, -sizeof (mram_config_vals) /4 ); +struct m_can_classdev *m_can_core_allocate_dev (struct device *dev) +{ +struct m_can_classdev *class_dev =NULL ; +u32 mram_config_vals[MRAM_CFG_LEN ]; +struct net_device *net_dev; +u32 tx_fifo_size; +int ret; + + ret =fwnode_property_read_u32_array (dev_fwnode (dev), +"bosch,mram-cfg" , + mram_config_vals, +sizeof (mram_config_vals) /4 );if (ret) { -dev_err (&pdev->dev,"Could not get Message RAM configuration." ); - goto failed_ret; +dev_err (dev,"Could not get Message RAM configuration." ); + goto out; }/* Get TX FIFO size @@ -1637,69 +1641,77 @@ static int m_can_plat_probe(struct platform_device *pdev) tx_fifo_size = mram_config_vals[7 ];
Quoted 12 lines
drivers/net/can/m_can/m_can.c static int m_can_plat_probe(struct platform_device *pdev) · line 1646
Quoted 8 lines
drivers/net/can/m_can/m_can.c +3
-4/* allocate the m_can device */ - dev =alloc_candev (sizeof (*priv), tx_fifo_size); -if (!dev) { - ret = -ENOMEM ; - goto failed_ret; + net_dev =alloc_candev (sizeof (*class_dev), tx_fifo_size); +if (!net_dev) { +dev_err (dev,"Failed to allocate CAN device" );No error message in case of ENOMEM. It makes it worse. Also, it's an unrelated change.
Most likely a copy and paste from original code.
Quoted 18 lines
Quoted 5 lines
drivers/net/can/m_can/m_can.c +1
-2+ goto out; } - priv =netdev_priv (dev); - dev->irq = irq;drivers/net/can/m_can/m_can.c static int m_can_plat_probe(struct platform_device *pdev) · line 1654
Quoted 10 lines
drivers/net/can/m_can/m_can.c +5
-5- priv->device = &pdev->dev; - priv->hclk = hclk; - priv->cclk = cclk; - priv->can.clock.freq =clk_get_rate (cclk); - priv->mram_base = mram_addr; + class_dev =netdev_priv (net_dev); +if (!class_dev) { +dev_err (dev,"Failed to init netdev private" ); + goto out; + }
WARN_ON_ONECE()?
That would be an unrelated change this is a copy/paste from original code.
Quoted 199 lines
Quoted 199 lines
drivers/net/can/m_can/m_can.c +44
-94-platform_set_drvdata (pdev, dev); -SET_NETDEV_DEV (dev, &pdev->dev); + class_dev->net = net_dev; + class_dev->dev = dev; +SET_NETDEV_DEV (net_dev, dev); -/* Enable clocks. Necessary to read Core Release in order to determine -* M_CAN version -*/ -pm_runtime_enable (&pdev->dev); - ret =m_can_clk_start (priv); -if (ret) - goto pm_runtime_fail; +m_can_of_parse_mram (class_dev, mram_config_vals); +out: +return class_dev; +} +EXPORT_SYMBOL_GPL (m_can_core_allocate_dev); + +int m_can_core_register (struct m_can_classdev *m_can_dev) +{ +int ret; + +if (m_can_dev->pm_clock_support) { +pm_runtime_enable (m_can_dev->dev); + ret =m_can_clk_start (m_can_dev); +if (ret) + goto pm_runtime_fail; + } - ret =m_can_dev_setup (pdev, dev, addr); + ret =m_can_dev_setup (m_can_dev);if (ret) goto clk_disable; - ret =register_m_can_dev (dev); + ret =register_m_can_dev (m_can_dev->net);if (ret) { -dev_err (&pdev->dev,"registering %s failed (err=%d)\n" , -KBUILD_MODNAME , ret); +dev_err (m_can_dev->dev,"registering %s failed (err=%d)\n" , + m_can_dev->net->name, ret); goto clk_disable; } -m_can_of_parse_mram (priv, mram_config_vals); - -devm_can_led_init (dev); +devm_can_led_init (m_can_dev->net); -of_can_transceiver (dev); +of_can_transceiver (m_can_dev->net); -dev_info (&pdev->dev,"%s device registered (irq=%d, version=%d)\n" , -KBUILD_MODNAME , dev->irq, priv->version); +dev_info (m_can_dev->dev,"%s device registered (irq=%d, version=%d)\n" , +KBUILD_MODNAME , m_can_dev->net->irq, m_can_dev->version);/* Probe finished * Stop clocks. They will be reactivated once the M_CAN device is opened */ clk_disable: -m_can_clk_stop (priv); +m_can_clk_stop (m_can_dev); pm_runtime_fail:if (ret) { -pm_runtime_disable (&pdev->dev); -free_candev (dev); +if (m_can_dev->pm_clock_support) +pm_runtime_disable (m_can_dev->dev); +free_candev (m_can_dev->net); } -failed_ret: +return ret; } +EXPORT_SYMBOL_GPL (m_can_core_register); -static __maybe_unused int m_can_suspend (struct device *dev) +int m_can_core_suspend (struct device *dev) {struct net_device *ndev =dev_get_drvdata (dev); -struct m_can_priv *priv =netdev_priv (ndev); +struct m_can_classdev *priv =netdev_priv (ndev);if (netif_running (ndev)) {netif_stop_queue (ndev);@@ -1714,11 +1726,12 @@ static __maybe_unused int m_can_suspend(struct device *dev)return 0 ; } +EXPORT_SYMBOL_GPL (m_can_core_suspend); -static __maybe_unused int m_can_resume (struct device *dev) +int m_can_core_resume (struct device *dev) {struct net_device *ndev =dev_get_drvdata (dev); -struct m_can_priv *priv =netdev_priv (ndev); +struct m_can_classdev *priv =netdev_priv (ndev);pinctrl_pm_select_default_state (dev);@@ -1739,78 +1752,17 @@ static __maybe_unused int m_can_resume(struct device *dev)return 0 ; } +EXPORT_SYMBOL_GPL (m_can_core_resume); -static void unregister_m_can_dev (struct net_device *dev) -{ -unregister_candev (dev); -} - -static int m_can_plat_remove (struct platform_device *pdev) -{ -struct net_device *dev =platform_get_drvdata (pdev); - -unregister_m_can_dev (dev); - -pm_runtime_disable (&pdev->dev); - -platform_set_drvdata (pdev,NULL ); - -free_candev (dev); - -return 0 ; -} - -static int __maybe_unused m_can_runtime_suspend (struct device *dev) +void m_can_core_unregister (struct m_can_classdev *m_can_dev) { -struct net_device *ndev =dev_get_drvdata (dev); -struct m_can_priv *priv =netdev_priv (ndev); +unregister_candev (m_can_dev->net); -clk_disable_unprepare (priv->cclk); -clk_disable_unprepare (priv->hclk); +m_can_clk_stop (m_can_dev); -return 0 ; +free_candev (m_can_dev->net); } - -static int __maybe_unused m_can_runtime_resume (struct device *dev) -{ -struct net_device *ndev =dev_get_drvdata (dev); -struct m_can_priv *priv =netdev_priv (ndev); -int err; - - err =clk_prepare_enable (priv->hclk); -if (err) -return err; - - err =clk_prepare_enable (priv->cclk); -if (err) -clk_disable_unprepare (priv->hclk); - -return err; -} - -static const struct dev_pm_ops m_can_pmops = { - SET_RUNTIME_PM_OPS(m_can_runtime_suspend, - m_can_runtime_resume, NULL) -SET_SYSTEM_SLEEP_PM_OPS (m_can_suspend, m_can_resume) -}; - -static const struct of_device_id m_can_of_table[] = { - { .compatible ="bosch,m_can" , .data =NULL }, - {/* sentinel */ }, -}; -MODULE_DEVICE_TABLE (of, m_can_of_table); - -static struct platform_driver m_can_plat_driver = { - .driver = { - .name =KBUILD_MODNAME , - .of_match_table = m_can_of_table, - .pm = &m_can_pmops, - }, - .probe = m_can_plat_probe, - .remove = m_can_plat_remove, -}; - -module_platform_driver (m_can_plat_driver); +EXPORT_SYMBOL_GPL (m_can_core_unregister);MODULE_AUTHOR ("Dong Aisheng <[email protected]>" );MODULE_LICENSE ("GPL v2" );drivers/net/can/m_can/m_can_platform.h +1
-1@@ -156,7 +156,7 @@ int m_can_core_register(struct m_can_classdev *m_can_dev);void m_can_core_unregister (struct m_can_classdev *m_can_dev);int m_can_core_get_clocks (struct m_can_classdev *m_can_dev);void m_can_init_ram (struct m_can_classdev *priv); -void m_can_config_endisable (const struct m_can_classdev *priv,bool enable); +void m_can_config_endisable (struct m_can_classdev *priv,bool enable);
Quoted 7 lines
drivers/net/can/m_can/m_can_platform.h int m_can_core_register(struct m_can_classdev *m_can_dev); · line 162
Quoted 2 lines
drivers/net/can/m_can/m_can_platform.h +0
-0int m_can_core_suspend (struct device *dev);int m_can_core_resume (struct device *dev);If you fix the issues with "
is_peripheral" and the TX function, it should already work on standardM_CANdevices as before... at least in theory!
It does work on the io-mapped devices. There may be some issues but we did run the test and it at least did not bug check.
Still testing the io-mapped code though
Quoted 2 lines
Wolfgang.
Signature · 2 lines
------------------ Dan Murphy
Re: [PATCH v4 2/4] can: m_can: Migrate the m_can code to use the framework
Hello Dan,
Am 22.01.19 um 14:37 schrieb Dan Murphy:
Quoted 61 lines
Wolfgang
Quoted 23 lines · Wolfgang Grandegger
Hello,
Am 17.01.19 um 21:05 schrieb Dan Murphy:
Quoted 20 lines
Migrate the
m_cancode to use them_can_platformframework code.Signed-off-by Dan Murphy <[email protected]>--- drivers/net/can/m_can/Kconfig | 12 + drivers/net/can/m_can/Makefile | 4 +- drivers/net/can/m_can/m_can.c | 764 ++++++++++++------------- drivers/net/can/m_can/m_can_platform.h | 2 +- 4 files changed, 374 insertions(+), 408 deletions(-)drivers/net/can/m_can/Kconfig +3
-0@@ -1,5 +1,17 @@ config CAN_M_CAN + tristate "Bosch M_CAN support" + ---help--- + Say Y here if you want to support for Bosch M_CAN controller.Quoted 9 lines
drivers/net/can/m_can/Kconfig line 10
Quoted 6 lines
drivers/net/can/m_can/Kconfig +6
-0+ +config CAN_M_CAN_CORE + depends on CAN_M_CAN + tristate "Bosch M_CAN Core support" + ---help--- + Say Y here if you want to support for Bosch M_CAN controller.Do you need that extra config? I think "
CAN_M_CAN" is just fine.OK I can remove the CORE. That should be built if
CAN_M_CANis selected thenQuoted 23 lines
Quoted 11 lines
drivers/net/can/m_can/Kconfig +2
-0+config CAN_M_CAN_PLATFORM depends on HAS_IOMEM + depends on CAN_M_CAN_CORE tristate "Bosch M_CAN devices" ---help--- Say Y here if you want to support for Bosch M_CAN controller.drivers/net/can/m_can/Makefile +0
-0drivers/net/can/m_can/Makefile line 7
Quoted 7 lines
drivers/net/can/m_can/Makefile +3
-1@@ -2,4 +2,6 @@ # Makefile for the Bosch M_CAN controller driver. # -obj-$(CONFIG_CAN_M_CAN) += m_can.o +obj-$(CONFIG_CAN_M_CAN_CORE) += m_can.o +obj-$(CONFIG_CAN_M_CAN_PLATFORM) += m_can_platform.o +obj-$(CONFIG_CAN_M_CAN_TCAN4X5X) += tcan4x5x.oThis file is provided in a sub-sequent patched! The code *must* compile for every single patch applied for bisect'ing. Looking to your first patch, I just realize that this is also not the case!
Quoted 8 lines
Quoted 2 lines
I think it makes sense to squash patch 1 and 2.
Well. Patch 1 should compile on its own I will check that.
Squashing patch 1 and patch 2 made the review even more impossible.
So I made patch 1 to create the io-mapped code (which probably will not compile when enabled) and then patch 2 was the changes to the CORE to stitch in the io-mapped code.
Patch 1 just changes two small locations of "m_can.c". It does not use the new interface or even include "m_can_platform.h". Patch 2 is well readable, especially the common code. It would be even better if "struct can_priv" would be kept and all the functions in the original order. That's why I suggested to do so.
Quoted 6 lines
Patch 4 was the addition of the peripherial.
I did this because there was a comment from the RFCs I sent in that there was to much change to review for a single patch conversion. I can squash patch 1 and patch 2 after we finish the review process prior to merging it into the tree.
That was also the comment I received from an internal review.
Sorry, if I confused you! My concern was about the common code in m_can.c.
Quoted 94 lines
Quoted 84 lines
Quoted 84 lines
drivers/net/can/m_can/m_can.c +2
-68@@ -28,87 +28,14 @@#include <linux/can/dev.h> #include <linux/pinctrl/consumer.h> +#include "m_can_platform.h" +/* napi related */ #define M_CAN_NAPI_WEIGHT 64/* message ram configuration data length */ #define MRAM_CFG_LEN 8 -/* registers definition */ -enum m_can_reg { -M_CAN_CREL =0x0 , -M_CAN_ENDN =0x4 , -M_CAN_CUST =0x8 , -M_CAN_DBTP =0xc , -M_CAN_TEST =0x10 , -M_CAN_RWD =0x14 , -M_CAN_CCCR =0x18 , -M_CAN_NBTP =0x1c , -M_CAN_TSCC =0x20 , -M_CAN_TSCV =0x24 , -M_CAN_TOCC =0x28 , -M_CAN_TOCV =0x2c , -M_CAN_ECR =0x40 , -M_CAN_PSR =0x44 , -/* TDCR Register only available for version >=3.1.x */ -M_CAN_TDCR =0x48 , -M_CAN_IR =0x50 , -M_CAN_IE =0x54 , -M_CAN_ILS =0x58 , -M_CAN_ILE =0x5c , -M_CAN_GFC =0x80 , -M_CAN_SIDFC =0x84 , -M_CAN_XIDFC =0x88 , -M_CAN_XIDAM =0x90 , -M_CAN_HPMS =0x94 , -M_CAN_NDAT1 =0x98 , -M_CAN_NDAT2 =0x9c , -M_CAN_RXF0C =0xa0 , -M_CAN_RXF0S =0xa4 , -M_CAN_RXF0A =0xa8 , -M_CAN_RXBC =0xac , -M_CAN_RXF1C =0xb0 , -M_CAN_RXF1S =0xb4 , -M_CAN_RXF1A =0xb8 , -M_CAN_RXESC =0xbc , -M_CAN_TXBC =0xc0 , -M_CAN_TXFQS =0xc4 , -M_CAN_TXESC =0xc8 , -M_CAN_TXBRP =0xcc , -M_CAN_TXBAR =0xd0 , -M_CAN_TXBCR =0xd4 , -M_CAN_TXBTO =0xd8 , -M_CAN_TXBCF =0xdc , -M_CAN_TXBTIE =0xe0 , -M_CAN_TXBCIE =0xe4 , -M_CAN_TXEFC =0xf0 , -M_CAN_TXEFS =0xf4 , -M_CAN_TXEFA =0xf8 , -}; - -/* m_can lec values */ -enum m_can_lec_type { -LEC_NO_ERROR =0 , -LEC_STUFF_ERROR , -LEC_FORM_ERROR , -LEC_ACK_ERROR , -LEC_BIT1_ERROR , -LEC_BIT0_ERROR , -LEC_CRC_ERROR , -LEC_UNUSED , -}; - -enum m_can_mram_cfg { -MRAM_SIDF =0 , -MRAM_XIDF , -MRAM_RXF0 ,Quoted 9 lines
drivers/net/can/m_can/m_can.c line 110
Quoted 6 lines
drivers/net/can/m_can/m_can.c +0
-6-MRAM_RXF1 , -MRAM_RXB , -MRAM_TXE , -MRAM_TXB , - MRAM_CFG_NUM, -};Patch 1 should have already done that!
Quoted 1 line
Will look at it.
As I said above. Patch 1 did *not* touch "m_can.c". Therefore the definitions above are in both files, m_can.c *and* m_can_platform.h.
Quoted 212 lines
Quoted 87 lines
Quoted 74 lines
drivers/net/can/m_can/m_can.c +26
-29/* Core Release Register (CREL) */ #define CREL_REL_SHIFT 28#define CREL_REL_MASK (0xF << CREL_REL_SHIFT)@@ -343,72 +270,81 @@ enum m_can_mram_cfg {#define TX_BUF_MM_MASK (0xff << TX_BUF_MM_SHIFT)/* Tx event FIFO Element */ -/* E1 */ #define TX_EVENT_MM_SHIFT TX_BUF_MM_SHIFT#define TX_EVENT_MM_MASK (0xff << TX_EVENT_MM_SHIFT) -/* address offset and element number for each FIFO/Buffer in the Message RAM */ -struct mram_cfg { -u16 off; -u8 num; -}; +static u32 m_can_read (struct m_can_classdev *priv,enum m_can_reg reg) +{ +u32 ret = -EINVAL ; -/* m_can private data structure */ -struct m_can_priv { -struct can_priv can;/* must be the first member */ -struct napi_struct napi; -struct net_device *dev; -struct device *device; -struct clk *hclk; -struct clk *cclk; -void __iomem *base; -u32 irqstatus; -int version; - -/* message ram configuration */ -void __iomem *mram_base; -struct mram_cfg mcfg[MRAM_CFG_NUM ]; -}; +if (priv->read_reg) + ret = priv->read_reg (priv, reg); -static inline u32 m_can_read (const struct m_can_priv *priv,enum m_can_reg reg) +return ret; +} + +static int m_can_write (struct m_can_classdev *priv,enum m_can_reg reg,u32 val) { -return readl (priv->base + reg); +int ret = -EINVAL ; + +if (priv->write_reg) + ret = priv->write_reg (priv, reg, val); + +return ret; } -static inline void m_can_write (const struct m_can_priv *priv, -enum m_can_reg reg,u32 val) +static u32 m_can_fifo_read (struct m_can_classdev *priv, +u32 fgi,unsigned int offset) { -writel (val, priv->base + reg); +u32 addr_offset = priv->mcfg[MRAM_RXF0 ].off + fgi *RXF0_ELEMENT_SIZE + offset; +u32 ret = -EINVAL ; + +if (priv->read_fifo) + ret = priv->read_fifo (priv, addr_offset); + +return ret; } -static inline u32 m_can_fifo_read (const struct m_can_priv *priv, -u32 fgi,unsigned int offset) +static u32 m_can_fifo_write (struct m_can_classdev *priv, +u32 fpi,unsigned int offset,u32 val)drivers/net/can/m_can/m_can.c enum m_can_mram_cfg { · line 317
Quoted 10 lines
drivers/net/can/m_can/m_can.c +7
-2{ -return readl (priv->mram_base + priv->mcfg[MRAM_RXF0 ].off + - fgi *RXF0_ELEMENT_SIZE + offset); +u32 addr_offset = priv->mcfg[MRAM_TXB ].off + fpi *TXB_ELEMENT_SIZE + offset; +u32 ret = -EINVAL ; + +if (priv->write_fifo) + ret = priv->write_fifo (priv, addr_offset, val); + +return ret;Why not just:
Quoted 7 lines
if (priv->write_fifo) return priv->write_fifo(priv, addr_offset, val); else return -EINVAL;Here and below...
Ack.
Quoted 109 lines
Quoted 99 lines
drivers/net/can/m_can/m_can.c +26
-19} -static inline void m_can_fifo_write (const struct m_can_priv *priv, -u32 fpi,unsigned int offset,u32 val) +static u32 m_can_fifo_write_no_off (struct m_can_classdev *priv, +u32 fpi,u32 val) { -writel (val, priv->mram_base + priv->mcfg[MRAM_TXB ].off + - fpi *TXB_ELEMENT_SIZE + offset); +u32 ret =0 ; + +if (priv->write_fifo) + ret = priv->write_fifo (priv, fpi, val); + +return ret; } -static inline u32 m_can_txe_fifo_read (const struct m_can_priv *priv, -u32 fgi, -u32 offset) { -return readl (priv->mram_base + priv->mcfg[MRAM_TXE ].off + - fgi *TXE_ELEMENT_SIZE + offset); +static u32 m_can_txe_fifo_read (struct m_can_classdev *priv,u32 fgi,u32 offset) +{ +u32 addr_offset = priv->mcfg[MRAM_TXE ].off + fgi *TXE_ELEMENT_SIZE + offset; +u32 ret = -EINVAL ; + +if (priv->read_fifo) + ret = priv->read_fifo (priv, addr_offset); + +return ret; } -static inline bool m_can_tx_fifo_full (const struct m_can_priv *priv) +static inline bool m_can_tx_fifo_full (struct m_can_classdev *priv) { -return !!(m_can_read (priv,M_CAN_TXFQS ) &TXFQS_TFQF ); +return !!(m_can_read (priv,M_CAN_TXFQS ) &TXFQS_TFQF ); } -static inline void m_can_config_endisable (const struct m_can_priv *priv, -bool enable) +void m_can_config_endisable (struct m_can_classdev *priv,bool enable) {u32 cccr =m_can_read (priv,M_CAN_CCCR );u32 timeout =10 ;@@ -433,7 +369,7 @@ static inline void m_can_config_endisable(const struct m_can_priv *priv,while ((m_can_read (priv,M_CAN_CCCR ) & (CCCR_INIT |CCCR_CCE )) != val) {if (timeout ==0 ) { -netdev_warn (priv->dev,"Failed to init module\n" ); +netdev_warn (priv->net,"Failed to init module\n" );return ; } timeout--;@@ -441,13 +377,13 @@ static inline void m_can_config_endisable(const struct m_can_priv *priv, } } -static inline void m_can_enable_all_interrupts (const struct m_can_priv *priv) +static inline void m_can_enable_all_interrupts (struct m_can_classdev *priv) {/* Only interrupt line 0 is used in this driver */ m_can_write (priv,M_CAN_ILE ,ILE_EINT0 ); } -static inline void m_can_disable_all_interrupts (const struct m_can_priv *priv) +static inline void m_can_disable_all_interrupts (struct m_can_classdev *priv) {m_can_write (priv,M_CAN_ILE ,0x0 ); }@@ -455,7 +391,7 @@ static inline void m_can_disable_all_interrupts(const struct m_can_priv *priv)static void m_can_read_fifo (struct net_device *dev,u32 rxfs) {struct net_device_stats *stats = &dev->stats; -struct m_can_priv *priv =netdev_priv (dev); +struct m_can_classdev *priv =netdev_priv (dev);struct canfd_frame *cf;struct sk_buff *skb;u32 id, fgi, dlc;@@ -512,7 +448,7 @@ static void m_can_read_fifo(struct net_device *dev, u32 rxfs)static int m_can_do_rx_poll (struct net_device *dev,int quota) { -struct m_can_priv *priv =netdev_priv (dev); +struct m_can_classdev *priv =netdev_priv (dev);u32 pkts =0 ;u32 rxfs;@@ -565,7 +501,7 @@ static int m_can_handle_lost_msg(struct net_device *dev)static int m_can_handle_lec_err (struct net_device *dev,enum m_can_lec_type lec_type) { -struct m_can_priv *priv =netdev_priv (dev); +struct m_can_classdev *priv =netdev_priv (dev);struct net_device_stats *stats = &dev->stats;struct can_frame *cf;struct sk_buff *skb;drivers/net/can/m_can/m_can.c static int m_can_handle_lec_err(struct net_device *dev, · line 561
Quoted 5 lines
drivers/net/can/m_can/m_can.c +1
-1@@ -622,7 +558,7 @@ static int m_can_handle_lec_err(struct net_device *dev,static int __m_can_get_berr_counter (const struct net_device *dev,struct can_berr_counter *bec) { -struct m_can_priv *priv =netdev_priv (dev); +struct m_can_classdev *priv =netdev_priv (dev);Back to the naming: You use "priv" here because I only wanted to see minimal changes to the
m_cancode. Why not also re-using "structm_can_priv" for the moment and use:Quoted 5 lines
m_can_register(struct m_can_priv priv);Later-on, when the development has settled, we could introduce betetr names. What do you think?
Quoted 6 lines
m_can_classdevis the common struct for the devices that register.m_can_privis the private struct for the device itself. If you look at them_can_privstruct and thetcan_privstruct they contain very different variables.This is why I changed this to the common class struct so that the
m_cancommon code has a common struct.
I know and I agree that the old "stuct m_can_priv" is not a good name. The only reason why I suggest to keep it for the moment is to make the changes to the common code minimal, esepcially to get rid of:
- struct m_can_priv *priv = netdev_priv(dev);
+ struct m_can_classdev *priv = netdev_priv(dev);
Well, just leave it as it is now! I can follow the changes.
Quoted 270 lines
Quoted 59 lines
Quoted 59 lines
drivers/net/can/m_can/m_can.c +12
-8unsigned int ecr; ecr =m_can_read (priv,M_CAN_ECR );@@ -632,28 +568,32 @@ static int __m_can_get_berr_counter(const struct net_device *dev,return 0 ; } -static int m_can_clk_start (struct m_can_priv *priv) +static int m_can_clk_start (struct m_can_classdev *priv) {int err; - err =pm_runtime_get_sync (priv->device); +if (priv->pm_clock_support ==0 ) +return 0 ; + + err =pm_runtime_get_sync (priv->dev);if (err <0 ) { -pm_runtime_put_noidle (priv->device); +pm_runtime_put_noidle (priv->dev);return err; }return 0 ; } -static void m_can_clk_stop (struct m_can_priv *priv) +static void m_can_clk_stop (struct m_can_classdev *priv) { -pm_runtime_put_sync (priv->device); +if (priv->pm_clock_support) +pm_runtime_put_sync (priv->dev); }static int m_can_get_berr_counter (const struct net_device *dev,struct can_berr_counter *bec) { -struct m_can_priv *priv =netdev_priv (dev); +struct m_can_classdev *priv =netdev_priv (dev);int err; err =m_can_clk_start (priv);@@ -670,7 +610,7 @@ static int m_can_get_berr_counter(const struct net_device *dev,static int m_can_handle_state_change (struct net_device *dev,enum can_state new_state) { -struct m_can_priv *priv =netdev_priv (dev); +struct m_can_classdev *priv =netdev_priv (dev);struct net_device_stats *stats = &dev->stats;struct can_frame *cf;struct sk_buff *skb;@@ -744,25 +684,22 @@ static int m_can_handle_state_change(struct net_device *dev,static int m_can_handle_state_errors (struct net_device *dev,u32 psr) { -struct m_can_priv *priv =netdev_priv (dev); +struct m_can_classdev *priv =netdev_priv (dev);int work_done =0 ;Quoted 7 lines
drivers/net/can/m_can/m_can.c static int m_can_handle_state_change(struct net_device *dev, · line 690
Quoted 3 lines
drivers/net/can/m_can/m_can.c +1
-2-if ((psr &PSR_EW ) && - (priv->can.state !=CAN_STATE_ERROR_WARNING )) { +if ((psr &PSR_EW ) && priv->can.state !=CAN_STATE_ERROR_WARNING ) {This is an unrelated cosmetic change. We should avoid them here. Could be done later-on with an extra patch.
Ack
Quoted 43 lines
Quoted 43 lines
drivers/net/can/m_can/m_can.c +5
-8netdev_dbg (dev,"entered error warning state\n" ); work_done +=m_can_handle_state_change (dev,CAN_STATE_ERROR_WARNING ); } -if ((psr &PSR_EP ) && - (priv->can.state !=CAN_STATE_ERROR_PASSIVE )) { +if ((psr &PSR_EP ) && priv->can.state !=CAN_STATE_ERROR_PASSIVE ) {netdev_dbg (dev,"entered error passive state\n" ); work_done +=m_can_handle_state_change (dev,CAN_STATE_ERROR_PASSIVE ); } -if ((psr &PSR_BO ) && - (priv->can.state !=CAN_STATE_BUS_OFF )) { +if ((psr &PSR_BO ) && priv->can.state !=CAN_STATE_BUS_OFF ) {netdev_dbg (dev,"entered error bus off state\n" ); work_done +=m_can_handle_state_change (dev,CAN_STATE_BUS_OFF );@@ -797,7 +734,7 @@ static inline bool is_lec_err(u32 psr)static int m_can_handle_bus_errors (struct net_device *dev,u32 irqstatus,u32 psr) { -struct m_can_priv *priv =netdev_priv (dev); +struct m_can_classdev *priv =netdev_priv (dev);int work_done =0 ;if (irqstatus &IR_RF0L )@@ -814,10 +751,9 @@ static int m_can_handle_bus_errors(struct net_device *dev, u32 irqstatus,return work_done; } -static int m_can_poll (struct napi_struct *napi,int quota) +static int m_can_rx_handler (struct net_device *dev,int quota) { -struct net_device *dev = napi->dev; -struct m_can_priv *priv =netdev_priv (dev); +struct m_can_classdev *priv =netdev_priv (dev);int work_done =0 ;u32 irqstatus, psr;@@ -834,13 +770,33 @@ static int m_can_poll(struct napi_struct *napi, int quota)Quoted 10 lines
drivers/net/can/m_can/m_can.c static int m_can_poll(struct napi_struct *napi, int quota) · line 777
Quoted 7 lines
drivers/net/can/m_can/m_can.c +5
-0if (irqstatus &IR_RF0N ) work_done +=m_can_do_rx_poll (dev, (quota - work_done)); +end: +return work_done; +} + +static int m_can_rx (struct net_device *dev)
m_can_rx_peripheral?Ack
Quoted 134 lines
Quoted 134 lines
drivers/net/can/m_can/m_can.c +34
-15+{ +struct m_can_classdev *priv =netdev_priv (dev); +m_can_rx_handler (dev,1 ); + +m_can_enable_all_interrupts (priv); + +return 0 ; +} + +static int m_can_poll (struct napi_struct *napi,int quota) +{ +struct net_device *dev = napi->dev; +struct m_can_classdev *priv =netdev_priv (dev); +int work_done =0 ; + + work_done =m_can_rx_handler (dev, quota);if (work_done < quota) {napi_complete_done (napi, work_done);m_can_enable_all_interrupts (priv); } -end:return work_done; }@@ -852,7 +808,7 @@ static void m_can_echo_tx_event(struct net_device *dev)int i =0 ;unsigned int msg_mark; -struct m_can_priv *priv =netdev_priv (dev); +struct m_can_classdev *priv =netdev_priv (dev);struct net_device_stats *stats = &dev->stats;/* read tx event fifo status */ @@ -885,7 +841,7 @@ static void m_can_echo_tx_event(struct net_device *dev)static irqreturn_t m_can_isr (int irq,void *dev_id) {struct net_device *dev = (struct net_device *)dev_id; -struct m_can_priv *priv =netdev_priv (dev); +struct m_can_classdev *priv =netdev_priv (dev);struct net_device_stats *stats = &dev->stats;u32 ir;@@ -905,7 +861,10 @@ static irqreturn_t m_can_isr(int irq, void *dev_id)if ((ir &IR_RF0N ) || (ir &IR_ERR_ALL_30X )) { priv->irqstatus = ir;m_can_disable_all_interrupts (priv); -napi_schedule (&priv->napi); +if (!priv->is_peripherial) +napi_schedule (&priv->napi); +else +m_can_rx (dev); }if (priv->version ==30 ) {@@ -927,6 +886,9 @@ static irqreturn_t m_can_isr(int irq, void *dev_id) } } +if (priv->clr_dev_interrupts) + priv->clr_dev_interrupts (priv); +return IRQ_HANDLED ; }@@ -980,7 +942,7 @@ static const struct can_bittiming_const m_can_data_bittiming_const_31X = {static int m_can_set_bittiming (struct net_device *dev) { -struct m_can_priv *priv =netdev_priv (dev); +struct m_can_classdev *priv =netdev_priv (dev);const struct can_bittiming *bt = &priv->can.bittiming;const struct can_bittiming *dbt = &priv->can.data_bittiming;u16 brp, sjw, tseg1, tseg2;@@ -1053,7 +1015,7 @@ static int m_can_set_bittiming(struct net_device *dev)*/ static void m_can_chip_config (struct net_device *dev) { -struct m_can_priv *priv =netdev_priv (dev); +struct m_can_classdev *priv =netdev_priv (dev);u32 cccr, test;m_can_config_endisable (priv, true);@@ -1165,7 +1127,7 @@ static void m_can_chip_config(struct net_device *dev)static void m_can_start (struct net_device *dev) { -struct m_can_priv *priv =netdev_priv (dev); +struct m_can_classdev *priv =netdev_priv (dev);/* basic m_can configuration */ m_can_chip_config (dev);@@ -1194,20 +1156,17 @@ static int m_can_set_mode(struct net_device *dev, enum can_mode mode)* else it returns the release and step coded as: * return value = 10 * <release> + 1 * <step> */ -static int m_can_check_core_release (void __iomem *m_can_base) +static int m_can_check_core_release (struct m_can_classdev *priv) {u32 crel_reg;u8 rel;u8 step;int res; -struct m_can_priv temp_priv = { - .base = m_can_base - };/* Read Core Release Version and split into version number * Example: Version 3.2.1 => rel = 3; step = 2; substep = 1; */ - crel_reg =m_can_read (&temp_priv,M_CAN_CREL ); + crel_reg =m_can_read (priv,M_CAN_CREL ); rel = (u8)((crel_reg &CREL_REL_MASK ) >>CREL_REL_SHIFT ); step = (u8)((crel_reg &CREL_STEP_MASK ) >>CREL_STEP_SHIFT );@@ -1225,18 +1184,22 @@ static int m_can_check_core_release(void __iomem *m_can_base)/* Selectable Non ISO support only in version 3.2.x * This function checks if the bit is writable. */ -static bool m_can_niso_supported (const struct m_can_priv *priv) +static bool m_can_niso_supported (struct m_can_classdev *priv) { -u32 cccr_reg, cccr_poll; -int niso_timeout; +u32 cccr_reg, cccr_poll =0 ; +int niso_timeout = -ETIMEDOUT ; +int i;m_can_config_endisable (priv, true); cccr_reg =m_can_read (priv,M_CAN_CCCR ); cccr_reg |=CCCR_NISO ;m_can_write (priv,M_CAN_CCCR , cccr_reg);Quoted 10 lines
drivers/net/can/m_can/m_can.c static int m_can_check_core_release(void __iomem *m_can_base) · line 1202
Quoted 7 lines
drivers/net/can/m_can/m_can.c +5
-2- niso_timeout =readl_poll_timeout ((priv->base +M_CAN_CCCR ), cccr_poll, - (cccr_poll == cccr_reg),0 ,10 ); +for (i =0 ; i <=10 ; i++) { + cccr_poll =m_can_read (priv,M_CAN_CCCR ); +if (cccr_poll == cccr_reg) + niso_timeout =0 ; + }This change is also unrelated. Should be done in an extra patch.
Quoted 2 lines
Actually it is not.
readl_poll_timeoutis not supported in peripherial devices. I had to re-write this to poll the periherial devices. It worked fine for io-mapped devices.
OK.
Quoted 70 lines
Quoted 60 lines
Quoted 60 lines
drivers/net/can/m_can/m_can.c +17
-16/* Clear NISO */ cccr_reg &= ~(CCCR_NISO );@@ -1248,112 +1211,100 @@ static bool m_can_niso_supported(const struct m_can_priv *priv)return !niso_timeout; } -static int m_can_dev_setup (struct platform_device *pdev,struct net_device *dev, -void __iomem *addr) +static int m_can_dev_setup (struct m_can_classdev *m_can_dev) { -struct m_can_priv *priv; +struct net_device *dev = m_can_dev->net;int m_can_version; - m_can_version =m_can_check_core_release (addr); + m_can_version =m_can_check_core_release (m_can_dev);/* return if unsupported version */ if (!m_can_version) { -dev_err (&pdev->dev,"Unsupported version number: %2d" , +dev_err (m_can_dev->dev,"Unsupported version number: %2d" , m_can_version);return -EINVAL ; } - priv =netdev_priv (dev); -netif_napi_add (dev, &priv->napi, m_can_poll,M_CAN_NAPI_WEIGHT ); +if (!m_can_dev->is_peripherial) +netif_napi_add (dev, &m_can_dev->napi, + m_can_poll,M_CAN_NAPI_WEIGHT );/* Shared properties of all M_CAN versions */ - priv->version = m_can_version; - priv->dev = dev; - priv->base = addr; - priv->can.do_set_mode = m_can_set_mode; - priv->can.do_get_berr_counter = m_can_get_berr_counter; + m_can_dev->version = m_can_version; + m_can_dev->can.do_set_mode = m_can_set_mode; + m_can_dev->can.do_get_berr_counter = m_can_get_berr_counter;/* Set M_CAN supported operations */ - priv->can.ctrlmode_supported =CAN_CTRLMODE_LOOPBACK | + m_can_dev->can.ctrlmode_supported =CAN_CTRLMODE_LOOPBACK |CAN_CTRLMODE_LISTENONLY |CAN_CTRLMODE_BERR_REPORTING |CAN_CTRLMODE_FD ;/* Set properties depending on M_CAN version */ -switch (priv->version) { +switch (m_can_dev->version) {case 30 :/* CAN_CTRLMODE_FD_NON_ISO is fixed with M_CAN IP v3.0.x */ can_set_static_ctrlmode (dev,CAN_CTRLMODE_FD_NON_ISO ); - priv->can.bittiming_const = &m_can_bittiming_const_30X; - priv->can.data_bittiming_const = +if (m_can_dev->bit_timing) + m_can_dev->can.bittiming_const = m_can_dev->bit_timing; +else + m_can_dev->can.bittiming_const = + &m_can_bittiming_const_30X;Quoted 9 lines
drivers/net/can/m_can/m_can.c static bool m_can_niso_supported(const struct m_can_priv *priv) · line 1257
Quoted 6 lines
drivers/net/can/m_can/m_can.c +5
-0+if (m_can_dev->data_timing) + m_can_dev->can.data_bittiming_const = + m_can_dev->data_timing; +else + m_can_dev->can.data_bittiming_const = &m_can_data_bittiming_const_30X;Should'nt that go to
m_can_platform.c?
Quoted 4 lines
No. This is the original code. I added the ability for the peripherials to over ride the default bit timings provided.
I had to do this because TCAN needed different bit and data timings.
OK.
Quoted 131 lines
Quoted 53 lines
Quoted 29 lines
drivers/net/can/m_can/m_can.c +18
-4break ;case 31 :/* CAN_CTRLMODE_FD_NON_ISO is fixed with M_CAN IP v3.1.x */ can_set_static_ctrlmode (dev,CAN_CTRLMODE_FD_NON_ISO ); - priv->can.bittiming_const = &m_can_bittiming_const_31X; - priv->can.data_bittiming_const = +if (m_can_dev->bit_timing) + m_can_dev->can.bittiming_const = m_can_dev->bit_timing; +else + m_can_dev->can.bittiming_const = + &m_can_bittiming_const_31X; +if (m_can_dev->data_timing) + m_can_dev->can.data_bittiming_const = + m_can_dev->data_timing; +else + m_can_dev->can.data_bittiming_const = &m_can_data_bittiming_const_31X;break ;case 32 : - priv->can.bittiming_const = &m_can_bittiming_const_31X; - priv->can.data_bittiming_const = +if (m_can_dev->bit_timing) + m_can_dev->can.bittiming_const = m_can_dev->bit_timing; +else + m_can_dev->can.bittiming_const = + &m_can_bittiming_const_31X; + +if (m_can_dev->data_timing) + m_can_dev->can.data_bittiming_const =drivers/net/can/m_can/m_can.c static bool m_can_niso_supported(const struct m_can_priv *priv) · line 1291
Quoted 10 lines
drivers/net/can/m_can/m_can.c +6
-1+ m_can_dev->data_timing; + else + m_can_dev->can.data_bittiming_const = &m_can_data_bittiming_const_31X; - priv->can.ctrlmode_supported |= (m_can_niso_supported (priv) + + m_can_dev->can.ctrlmode_supported |= + (m_can_niso_supported (m_can_dev) ?CAN_CTRLMODE_FD_NON_ISO :0 );if (m_can_niso_supported(m_can_dev) m_can_dev->can.ctrlmode_supported |= CAN_CTRLMODE_FD_NON_ISO;Quoted 9 lines
drivers/net/can/m_can/m_can.c +2
-2break ;default : -dev_err (&pdev->dev,"Unsupported version number: %2d" , - priv->version); +dev_err (m_can_dev->dev,"Unsupported version number: %2d" , + m_can_dev->version);return -EINVAL ; }Quoted 8 lines
drivers/net/can/m_can/m_can.c static bool m_can_niso_supported(const struct m_can_priv *priv) · line 1314
Quoted 4 lines
drivers/net/can/m_can/m_can.c +0
-4-return 0 ; -} - -static int m_can_open (struct net_device *dev)This function has been moved around making it difficult to understand the diffs.
Yes it had to be broken up as some of the calls needed to be done in the peripherial and some are common and can be done here.
Quoted 55 lines
Quoted 55 lines
drivers/net/can/m_can/m_can.c +4
-37-{ -struct m_can_priv *priv =netdev_priv (dev); -int err; - - err =m_can_clk_start (priv); -if (err) -return err; - -/* open the can device */ - err =open_candev (dev); -if (err) { -netdev_err (dev,"failed to open can device\n" ); - goto exit_disable_clks; - } - -/* register interrupt handler */ - err =request_irq (dev->irq, m_can_isr,IRQF_SHARED , dev->name, - dev); -if (err <0 ) { -netdev_err (dev,"failed to request interrupt\n" ); - goto exit_irq_fail; - } - -/* start the m_can controller */ -m_can_start (dev); - -can_led_event (dev,CAN_LED_EVENT_OPEN ); -napi_enable (&priv->napi); -netif_start_queue (dev); +if (m_can_dev->device_init) + m_can_dev->device_init (m_can_dev);return 0 ; - -exit_irq_fail: -close_candev (dev); -exit_disable_clks: -m_can_clk_stop (priv); -return err; }static void m_can_stop (struct net_device *dev) { -struct m_can_priv *priv =netdev_priv (dev); +struct m_can_classdev *priv =netdev_priv (dev);/* disable all interrupts */ m_can_disable_all_interrupts (priv);@@ -1364,13 +1315,16 @@ static void m_can_stop(struct net_device *dev)static int m_can_close (struct net_device *dev) { -struct m_can_priv *priv =netdev_priv (dev); +struct m_can_classdev *priv =netdev_priv (dev);Quoted 10 lines
drivers/net/can/m_can/m_can.c static void m_can_stop(struct net_device *dev) · line 1326
Quoted 8 lines
drivers/net/can/m_can/m_can.c +3
-1netif_stop_queue (dev); -napi_disable (&priv->napi); +if (!priv->is_peripherial) +napi_disable (&priv->napi);m_can_stop (dev);m_can_clk_stop (priv);free_irq (dev->irq, dev); +destroy_workqueue (priv->wq);if (
priv->is_peripherial) ?
Quoted 1 line
This workqueue is created for both peripherial and io-mapped
Even if the io-mapped does not use it? Ah, you mean it does use it... which is a bad solution, I think. This adds extra latency!
Quoted 356 lines
Quoted 19 lines
Quoted 19 lines
drivers/net/can/m_can/m_can.c +2
-1+ priv->wq =NULL ;close_candev (dev);can_led_event (dev,CAN_LED_EVENT_STOP );@@ -1379,7 +1333,7 @@ static int m_can_close(struct net_device *dev)static int m_can_next_echo_skb_occupied (struct net_device *dev,int putidx) { -struct m_can_priv *priv =netdev_priv (dev); +struct m_can_classdev *priv =netdev_priv (dev);/*get wrap around for loopback skb index */ unsigned int wrap = priv->can.echo_skb_max;int next_idx;@@ -1391,18 +1345,17 @@ static int m_can_next_echo_skb_occupied(struct net_device *dev, int putidx)return !!priv->can.echo_skb[next_idx]; }Quoted 4 lines
drivers/net/can/m_can/m_can.c static int m_can_next_echo_skb_occupied(struct net_device *dev, int putidx) · line 1395
Quoted 2 lines
drivers/net/can/m_can/m_can.c +0
-2-static netdev_tx_t m_can_start_xmit (struct sk_buff *skb, -struct net_device *dev)Ditto
Same as above
Quoted 5 lines
drivers/net/can/m_can/m_can.c static int m_can_next_echo_skb_occupied(struct net_device *dev, int putidx) · line 1348
Quoted 2 lines
drivers/net/can/m_can/m_can.c +1
-0+static void m_can_tx_work_handler (struct work_struct *ws)I think you need a common function to do the tx.
This is the common function that was in the original code.
Quoted 22 lines
Quoted 22 lines
drivers/net/can/m_can/m_can.c +5
-5{ -struct m_can_priv *priv =netdev_priv (dev); -struct canfd_frame *cf = (struct canfd_frame *)skb->data; + struct m_can_classdev *priv = container_of(ws, struct m_can_classdev, + tx_work); +struct canfd_frame *cf = (struct canfd_frame *)priv->skb->data; +struct net_device *dev = priv->net; +struct sk_buff *skb = priv->skb;u32 id, cccr, fdflags;int i;int putidx; -if (can_dropped_invalid_skb (dev, skb)) -return NETDEV_TX_OK ; -/* Generate ID field for TX buffer Element */ /* Common to all supported M_CAN versions */ if (cf->can_id &CAN_EFF_FLAG ) {@@ -1431,7 +1384,8 @@ static netdev_tx_t m_can_start_xmit(struct sk_buff *skb,can_put_echo_skb (skb, dev,0 );Quoted 7 lines
drivers/net/can/m_can/m_can.c static netdev_tx_t m_can_start_xmit(struct sk_buff *skb, · line 1388
Quoted 4 lines
drivers/net/can/m_can/m_can.c +2
-1if (priv->can.ctrlmode &CAN_CTRLMODE_FD ) { - cccr =m_can_read (priv,M_CAN_CCCR ); +/*cccr = m_can_read(priv, M_CAN_CCCR);*/ + cccr =0 ;Unrelated change.
Ah yes debug code. Can be removed.
Quoted 58 lines
Quoted 24 lines
drivers/net/can/m_can/m_can.c +5
-4cccr &= ~(CCCR_CMR_MASK <<CCCR_CMR_SHIFT );if (can_is_canfd_skb (skb)) {if (cf->flags &CANFD_BRS )@@ -1457,7 +1411,7 @@ static netdev_tx_t m_can_start_xmit(struct sk_buff *skb,netif_stop_queue (dev);netdev_warn (dev,"TX queue active although FIFO is full." ); -return NETDEV_TX_BUSY ; +return ; }/* get put index for frame */ @@ -1498,14 +1452,87 @@ static netdev_tx_t m_can_start_xmit(struct sk_buff *skb,m_can_write (priv,M_CAN_TXBAR , (1 << putidx));/* stop network queue if fifo full */ -if (m_can_tx_fifo_full (priv) || -m_can_next_echo_skb_occupied (dev, putidx)) -netif_stop_queue (dev); +if (m_can_tx_fifo_full (priv) || +m_can_next_echo_skb_occupied (dev, putidx)) +netif_stop_queue (dev); } +}drivers/net/can/m_can/m_can.c static netdev_tx_t m_can_start_xmit(struct sk_buff *skb, · line 1469
Quoted 10 lines
drivers/net/can/m_can/m_can.c +10
-0+ +static netdev_tx_t m_can_start_xmit (struct sk_buff *skb, +struct net_device *dev) +{ +struct m_can_classdev *priv =netdev_priv (dev); + +if (can_dropped_invalid_skb (dev, skb)) +return NETDEV_TX_BUSY ; + +netif_stop_queue (dev);if (!priv->is_peripheral) { m_can_tx(priv, skb); } else { netif_stop_queue(dev); priv->skb = skb; queue_work(priv->wq, &priv->tx_work); }I think stopping the queue here is still experimental.
Quoted 13 lines
drivers/net/can/m_can/m_can.c +10
-0return NETDEV_TX_OK ; } +static int m_can_open (struct net_device *dev) +{ +struct m_can_classdev *priv =netdev_priv (dev); +int err; + + err =m_can_clk_start (priv); +if (err) +return err; + +/* open the can device */ Quoted 9 lines
drivers/net/can/m_can/m_can.c static netdev_tx_t m_can_start_xmit(struct sk_buff *skb, · line 1491
Quoted 6 lines
drivers/net/can/m_can/m_can.c +6
-0+ err =open_candev (dev); +if (err) { +netdev_err (dev,"failed to open can device\n" ); + goto exit_disable_clks; + } +if (priv->is_peripheral) {Not here this is needed. It is original code
Quoted 187 lines
drivers/net/can/m_can/m_can.c static netdev_tx_t m_can_start_xmit(struct sk_buff *skb, · line 1499
Quoted 8 lines
drivers/net/can/m_can/m_can.c +8
-0+ priv->wq =alloc_workqueue ("mcan_wq" ,WQ_FREEZABLE |WQ_MEM_RECLAIM , +0 ); +if (!priv->wq) { + err = -ENOMEM ; + goto out_wq_fail; + } + +INIT_WORK (&priv->tx_work, m_can_tx_work_handler);}Quoted 176 lines
drivers/net/can/m_can/m_can.c +74
-62+/* register interrupt handler */ +if (priv->is_peripherial) + err =request_threaded_irq (dev->irq,NULL , m_can_isr, +IRQF_ONESHOT |IRQF_TRIGGER_FALLING , + dev->name, dev); +else + err =request_irq (dev->irq, m_can_isr,IRQF_SHARED , dev->name, + dev); + +if (err <0 ) { +netdev_err (dev,"failed to request interrupt\n" ); + goto exit_irq_fail; + } + +/* start the m_can controller */ +m_can_start (dev); + +can_led_event (dev,CAN_LED_EVENT_OPEN ); + +if (!priv->is_peripherial) +napi_enable (&priv->napi); + +netif_start_queue (dev); + +return 0 ; + +exit_irq_fail: +destroy_workqueue (priv->wq); +out_wq_fail: +close_candev (dev); +exit_disable_clks: +m_can_clk_stop (priv); +return err; +} +static const struct net_device_ops m_can_netdev_ops = { .ndo_open = m_can_open, .ndo_stop = m_can_close,@@ -1521,21 +1548,7 @@ static int register_m_can_dev(struct net_device *dev)return register_candev (dev); } -static void m_can_init_ram (struct m_can_priv *priv) -{ -int end, i, start; - -/* initialize the entire Message RAM in use to avoid possible -* ECC/parity checksum errors when reading an uninitialized buffer -*/ - start = priv->mcfg[MRAM_SIDF ].off; - end = priv->mcfg[MRAM_TXB ].off + - priv->mcfg[MRAM_TXB ].num *TXB_ELEMENT_SIZE ; -for (i = start; i < end; i +=4 ) -writel (0x0 , priv->mram_base + i); -} - -static void m_can_of_parse_mram (struct m_can_priv *priv, +static void m_can_of_parse_mram (struct m_can_classdev *priv,const u32 *mram_config_vals) { priv->mcfg[MRAM_SIDF ].off = mram_config_vals[0 ];@@ -1562,9 +1575,8 @@ static void m_can_of_parse_mram(struct m_can_priv *priv, priv->mcfg[MRAM_TXB ].num = mram_config_vals[7 ] & (TXBC_NDTB_MASK >>TXBC_NDTB_SHIFT ); - dev_dbg(priv->device, - "mram_base %p sidf 0x%x %d xidf 0x%x %d rxf0 0x%x %d rxf1 0x%x %d rxb 0x%x %d txe 0x%x %d txb 0x%x %d\n", - priv->mram_base, + dev_dbg(priv->dev, + "sidf 0x%x %d xidf 0x%x %d rxf0 0x%x %d rxf1 0x%x %d rxb 0x%x %d txe 0x%x %d txb 0x%x %d\n", priv->mcfg[MRAM_SIDF].off, priv->mcfg[MRAM_SIDF].num, priv->mcfg[MRAM_XIDF].off, priv->mcfg[MRAM_XIDF].num, priv->mcfg[MRAM_RXF0].off, priv->mcfg[MRAM_RXF0].num,@@ -1572,63 +1584,55 @@ static void m_can_of_parse_mram(struct m_can_priv *priv, priv->mcfg[MRAM_RXB ].off, priv->mcfg[MRAM_RXB ].num, priv->mcfg[MRAM_TXE ].off, priv->mcfg[MRAM_TXE ].num, priv->mcfg[MRAM_TXB].off, priv->mcfg[MRAM_TXB].num); - -m_can_init_ram (priv); } -static int m_can_plat_probe (struct platform_device *pdev) +void m_can_init_ram (struct m_can_classdev *priv) { -struct net_device *dev; -struct m_can_priv *priv; -struct resource *res; -void __iomem *addr; -void __iomem *mram_addr; -struct clk *hclk, *cclk; -int irq, ret; -struct device_node *np; -u32 mram_config_vals[MRAM_CFG_LEN ]; -u32 tx_fifo_size; - - np = pdev->dev.of_node; +int end, i, start; - hclk =devm_clk_get (&pdev->dev,"hclk" ); - cclk =devm_clk_get (&pdev->dev,"cclk" ); +/* initialize the entire Message RAM in use to avoid possible +* ECC/parity checksum errors when reading an uninitialized buffer +*/ + start = priv->mcfg[MRAM_SIDF ].off; + end = priv->mcfg[MRAM_TXB ].off + + priv->mcfg[MRAM_TXB ].num *TXB_ELEMENT_SIZE ; -if (IS_ERR (hclk) ||IS_ERR (cclk)) { -dev_err (&pdev->dev,"no clock found\n" ); - ret = -ENODEV ; - goto failed_ret; - } +for (i = start; i < end; i +=4 ) +m_can_fifo_write_no_off (priv, i,0x0 ); +} +EXPORT_SYMBOL_GPL (m_can_init_ram); - res =platform_get_resource_byname (pdev,IORESOURCE_MEM ,"m_can" ); - addr =devm_ioremap_resource (&pdev->dev, res); - irq =platform_get_irq_byname (pdev,"int0" ); +int m_can_core_get_clocks (struct m_can_classdev *m_can_dev) +{ +int ret =0 ; -if (IS_ERR (addr) || irq <0 ) { - ret = -EINVAL ; - goto failed_ret; - } + m_can_dev->hclk =devm_clk_get (m_can_dev->dev,"hclk" ); + m_can_dev->cclk =devm_clk_get (m_can_dev->dev,"cclk" ); -/* message ram could be shared */ - res =platform_get_resource_byname (pdev,IORESOURCE_MEM ,"message_ram" ); -if (!res) { +if (IS_ERR (m_can_dev->cclk)) { +dev_err (m_can_dev->dev,"no clock found\n" ); ret = -ENODEV ; - goto failed_ret; } - mram_addr =devm_ioremap (&pdev->dev, res->start,resource_size (res)); -if (!mram_addr) { - ret = -ENOMEM ; - goto failed_ret; - } +return ret; +} +EXPORT_SYMBOL_GPL (m_can_core_get_clocks); -/* get message ram configuration */ - ret =of_property_read_u32_array (np,"bosch,mram-cfg" , - mram_config_vals, -sizeof (mram_config_vals) /4 ); +struct m_can_classdev *m_can_core_allocate_dev (struct device *dev) +{ +struct m_can_classdev *class_dev =NULL ; +u32 mram_config_vals[MRAM_CFG_LEN ]; +struct net_device *net_dev; +u32 tx_fifo_size; +int ret; + + ret =fwnode_property_read_u32_array (dev_fwnode (dev), +"bosch,mram-cfg" , + mram_config_vals, +sizeof (mram_config_vals) /4 );if (ret) { -dev_err (&pdev->dev,"Could not get Message RAM configuration." ); - goto failed_ret; +dev_err (dev,"Could not get Message RAM configuration." ); + goto out; }/* Get TX FIFO size @@ -1637,69 +1641,77 @@ static int m_can_plat_probe(struct platform_device *pdev) tx_fifo_size = mram_config_vals[7 ];Quoted 12 lines
drivers/net/can/m_can/m_can.c static int m_can_plat_probe(struct platform_device *pdev) · line 1646
Quoted 8 lines
drivers/net/can/m_can/m_can.c +3
-4/* allocate the m_can device */ - dev =alloc_candev (sizeof (*priv), tx_fifo_size); -if (!dev) { - ret = -ENOMEM ; - goto failed_ret; + net_dev =alloc_candev (sizeof (*class_dev), tx_fifo_size); +if (!net_dev) { +dev_err (dev,"Failed to allocate CAN device" );No error message in case of ENOMEM. It makes it worse. Also, it's an unrelated change.
Most likely a copy and paste from original code.
Quoted 18 lines
Quoted 5 lines
drivers/net/can/m_can/m_can.c +1
-2+ goto out; } - priv =netdev_priv (dev); - dev->irq = irq;drivers/net/can/m_can/m_can.c static int m_can_plat_probe(struct platform_device *pdev) · line 1654
Quoted 10 lines
drivers/net/can/m_can/m_can.c +5
-5- priv->device = &pdev->dev; - priv->hclk = hclk; - priv->cclk = cclk; - priv->can.clock.freq =clk_get_rate (cclk); - priv->mram_base = mram_addr; + class_dev =netdev_priv (net_dev); +if (!class_dev) { +dev_err (dev,"Failed to init netdev private" ); + goto out; + }
WARN_ON_ONECE()?
Quoted 1 line
That would be an unrelated change this is a copy/paste from original code.
OK.
Quoted 207 lines
Quoted 199 lines
Quoted 199 lines
drivers/net/can/m_can/m_can.c +44
-94-platform_set_drvdata (pdev, dev); -SET_NETDEV_DEV (dev, &pdev->dev); + class_dev->net = net_dev; + class_dev->dev = dev; +SET_NETDEV_DEV (net_dev, dev); -/* Enable clocks. Necessary to read Core Release in order to determine -* M_CAN version -*/ -pm_runtime_enable (&pdev->dev); - ret =m_can_clk_start (priv); -if (ret) - goto pm_runtime_fail; +m_can_of_parse_mram (class_dev, mram_config_vals); +out: +return class_dev; +} +EXPORT_SYMBOL_GPL (m_can_core_allocate_dev); + +int m_can_core_register (struct m_can_classdev *m_can_dev) +{ +int ret; + +if (m_can_dev->pm_clock_support) { +pm_runtime_enable (m_can_dev->dev); + ret =m_can_clk_start (m_can_dev); +if (ret) + goto pm_runtime_fail; + } - ret =m_can_dev_setup (pdev, dev, addr); + ret =m_can_dev_setup (m_can_dev);if (ret) goto clk_disable; - ret =register_m_can_dev (dev); + ret =register_m_can_dev (m_can_dev->net);if (ret) { -dev_err (&pdev->dev,"registering %s failed (err=%d)\n" , -KBUILD_MODNAME , ret); +dev_err (m_can_dev->dev,"registering %s failed (err=%d)\n" , + m_can_dev->net->name, ret); goto clk_disable; } -m_can_of_parse_mram (priv, mram_config_vals); - -devm_can_led_init (dev); +devm_can_led_init (m_can_dev->net); -of_can_transceiver (dev); +of_can_transceiver (m_can_dev->net); -dev_info (&pdev->dev,"%s device registered (irq=%d, version=%d)\n" , -KBUILD_MODNAME , dev->irq, priv->version); +dev_info (m_can_dev->dev,"%s device registered (irq=%d, version=%d)\n" , +KBUILD_MODNAME , m_can_dev->net->irq, m_can_dev->version);/* Probe finished * Stop clocks. They will be reactivated once the M_CAN device is opened */ clk_disable: -m_can_clk_stop (priv); +m_can_clk_stop (m_can_dev); pm_runtime_fail:if (ret) { -pm_runtime_disable (&pdev->dev); -free_candev (dev); +if (m_can_dev->pm_clock_support) +pm_runtime_disable (m_can_dev->dev); +free_candev (m_can_dev->net); } -failed_ret: +return ret; } +EXPORT_SYMBOL_GPL (m_can_core_register); -static __maybe_unused int m_can_suspend (struct device *dev) +int m_can_core_suspend (struct device *dev) {struct net_device *ndev =dev_get_drvdata (dev); -struct m_can_priv *priv =netdev_priv (ndev); +struct m_can_classdev *priv =netdev_priv (ndev);if (netif_running (ndev)) {netif_stop_queue (ndev);@@ -1714,11 +1726,12 @@ static __maybe_unused int m_can_suspend(struct device *dev)return 0 ; } +EXPORT_SYMBOL_GPL (m_can_core_suspend); -static __maybe_unused int m_can_resume (struct device *dev) +int m_can_core_resume (struct device *dev) {struct net_device *ndev =dev_get_drvdata (dev); -struct m_can_priv *priv =netdev_priv (ndev); +struct m_can_classdev *priv =netdev_priv (ndev);pinctrl_pm_select_default_state (dev);@@ -1739,78 +1752,17 @@ static __maybe_unused int m_can_resume(struct device *dev)return 0 ; } +EXPORT_SYMBOL_GPL (m_can_core_resume); -static void unregister_m_can_dev (struct net_device *dev) -{ -unregister_candev (dev); -} - -static int m_can_plat_remove (struct platform_device *pdev) -{ -struct net_device *dev =platform_get_drvdata (pdev); - -unregister_m_can_dev (dev); - -pm_runtime_disable (&pdev->dev); - -platform_set_drvdata (pdev,NULL ); - -free_candev (dev); - -return 0 ; -} - -static int __maybe_unused m_can_runtime_suspend (struct device *dev) +void m_can_core_unregister (struct m_can_classdev *m_can_dev) { -struct net_device *ndev =dev_get_drvdata (dev); -struct m_can_priv *priv =netdev_priv (ndev); +unregister_candev (m_can_dev->net); -clk_disable_unprepare (priv->cclk); -clk_disable_unprepare (priv->hclk); +m_can_clk_stop (m_can_dev); -return 0 ; +free_candev (m_can_dev->net); } - -static int __maybe_unused m_can_runtime_resume (struct device *dev) -{ -struct net_device *ndev =dev_get_drvdata (dev); -struct m_can_priv *priv =netdev_priv (ndev); -int err; - - err =clk_prepare_enable (priv->hclk); -if (err) -return err; - - err =clk_prepare_enable (priv->cclk); -if (err) -clk_disable_unprepare (priv->hclk); - -return err; -} - -static const struct dev_pm_ops m_can_pmops = { - SET_RUNTIME_PM_OPS(m_can_runtime_suspend, - m_can_runtime_resume, NULL) -SET_SYSTEM_SLEEP_PM_OPS (m_can_suspend, m_can_resume) -}; - -static const struct of_device_id m_can_of_table[] = { - { .compatible ="bosch,m_can" , .data =NULL }, - {/* sentinel */ }, -}; -MODULE_DEVICE_TABLE (of, m_can_of_table); - -static struct platform_driver m_can_plat_driver = { - .driver = { - .name =KBUILD_MODNAME , - .of_match_table = m_can_of_table, - .pm = &m_can_pmops, - }, - .probe = m_can_plat_probe, - .remove = m_can_plat_remove, -}; - -module_platform_driver (m_can_plat_driver); +EXPORT_SYMBOL_GPL (m_can_core_unregister);MODULE_AUTHOR ("Dong Aisheng <[email protected]>" );MODULE_LICENSE ("GPL v2" );drivers/net/can/m_can/m_can_platform.h +1
-1@@ -156,7 +156,7 @@ int m_can_core_register(struct m_can_classdev *m_can_dev);void m_can_core_unregister (struct m_can_classdev *m_can_dev);int m_can_core_get_clocks (struct m_can_classdev *m_can_dev);void m_can_init_ram (struct m_can_classdev *priv); -void m_can_config_endisable (const struct m_can_classdev *priv,bool enable); +void m_can_config_endisable (struct m_can_classdev *priv,bool enable);Quoted 7 lines
drivers/net/can/m_can/m_can_platform.h int m_can_core_register(struct m_can_classdev *m_can_dev); · line 162
Quoted 2 lines
drivers/net/can/m_can/m_can_platform.h +0
-0int m_can_core_suspend (struct device *dev);int m_can_core_resume (struct device *dev);If you fix the issues with "
is_peripheral" and the TX function, it should already work on standardM_CANdevices as before... at least in theory!
Quoted 4 lines
It does work on the io-mapped devices. There may be some issues but we did run the test and it at least did not bug check.
Still testing the io-mapped code though
Not too bad!
Thanks,
Wolfgang.
PATCH v4 3/4 dt-bindings: can: tcan4x5x: Add DT bindings for TCAN4x5X driver
In reply to Dan MurphyDT binding documentation for TI TCAN4x5x driver.
--- .../devicetree/bindings/net/can/tcan4x5x.txt | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 Documentation/devicetree/bindings/net/can/tcan4x5x.txt
Documentation/devicetree/bindings/net/can/tcan4x5x.txt new file +37 -0
@@ -0,0 +1,37 @@ +Texas Instruments TCAN4x5x CAN Controller +================================================ + +This file provides device node information for the TCAN4x5x interface contains. + +Required properties: + - compatible: "ti,tcan4x5x" + - reg: 0 + - #address-cells: 1 + - #size-cells: 0 + - spi-max-frequency: Maximum frequency of the SPI bus the chip can + operate at should be less than or equal to 18 MHz. + - data-ready-gpios: Interrupt GPIO for data and error reporting. + - device-wake-gpios: Wake up GPIO to wake up the TCAN device. + - device-state-gpios: Input GPIO that indicates if the device is in + a sleep state or if the device is active. + +See Documentation/devicetree/bindings/net/can/m_can.txt for additional +required property details. + +Optional properties: + - reset-gpios: Hardwired output GPIO. If not defined then software + reset. + +Example: +tcan4x5x: tcan4x5x@0 { + compatible = "ti,tcan4x5x"; + reg = <0>; + #address-cells = <1>; + #size-cells = <1>; + spi-max-frequency = <10000000>; + bosch,mram-cfg = <0x0 0 0 32 0 0 1 1>; + data-ready-gpios = <&gpio1 14 GPIO_ACTIVE_LOW>; + device-state-gpios = <&gpio3 21 GPIO_ACTIVE_HIGH>; + device-wake-gpios = <&gpio1 15 GPIO_ACTIVE_HIGH>; + reset-gpios = <&gpio1 27 GPIO_ACTIVE_LOW>; +};
Signature · 1 line
2.20.1.98.gecbdaf0899
PATCH v4 4/4 can: tcan4x5x: Add tcan4x5x driver to the kernel
In reply to Dan MurphyAdd the TCAN4x5x SPI CAN driver. This device
uses the Bosch MCAN IP core along with a SPI
interface map. Leverage the MCAN common core
code to manage the MCAN IP.
This device has a special method to indicate a
write/read operation on the data payload.
--- drivers/net/can/m_can/Kconfig | 6 + drivers/net/can/m_can/tcan4x5x.c | 529 +++++++++++++++++++++++++++++++ 2 files changed, 535 insertions(+) create mode 100644 drivers/net/can/m_can/tcan4x5x.c
drivers/net/can/m_can/Kconfig +6 -0
@@ -15,3 +15,9 @@ config CAN_M_CAN_PLATFORM tristate "Bosch M_CAN devices" ---help--- Say Y here if you want to support for Bosch M_CAN controller. + +config CAN_M_CAN_TCAN4X5X + depends on CAN_M_CAN + tristate "TCAN4X5X M_CAN device" + ---help--- + Say Y here if you want to support for TI M_CAN controller.
drivers/net/can/m_can/tcan4x5x.c new file +529 -0
529 lines not on this page · raw message
Signature · 1 line
2.20.1.98.gecbdaf0899
Re: [PATCH v4 4/4] can: tcan4x5x: Add tcan4x5x driver to the kernel
Hello,
Am 17.01.19 um 21:06 schrieb Dan Murphy:
Quoted 110 lines
Add the TCAN4x5x SPI CAN driver. This device
uses the Bosch MCAN IP core along with a SPI
interface map. Leverage the MCAN common core
code to manage the MCAN IP.This device has a special method to indicate a
write/read operation on the data payload.Signed-off-by Dan Murphy <[email protected]>--- drivers/net/can/m_can/Kconfig | 6 + drivers/net/can/m_can/tcan4x5x.c | 529 +++++++++++++++++++++++++++++++ 2 files changed, 535 insertions(+) create mode 100644 drivers/net/can/m_can/tcan4x5x.cdrivers/net/can/m_can/Kconfig +6
-0@@ -15,3 +15,9 @@ config CAN_M_CAN_PLATFORM tristate "Bosch M_CAN devices" ---help--- Say Y here if you want to support for Bosch M_CAN controller. + +config CAN_M_CAN_TCAN4X5X + depends on CAN_M_CAN + tristate "TCAN4X5X M_CAN device" + ---help--- + Say Y here if you want to support for TI M_CAN controller.drivers/net/can/m_can/tcan4x5x.c new file +75
-0@@ -0,0 +1,529 @@ +// SPDX-License-Identifier: GPL-2.0 +// SPI to CAN driver for the Texas Instruments TCAN4x5x +// Copyright (C) 2018-19 Texas Instruments Incorporated - http://www.ti.com/ + +#include <linux/regmap.h> +#include <linux/spi/spi.h> + +#include <linux/regulator/consumer.h> +#include <linux/gpio/consumer.h> + +#include "m_can_platform.h" + +#define DEVICE_NAME "tcan4x5x" +#define TCAN4X5X_EXT_CLK_DEF 40000000 + +#define TCAN4X5X_DEV_ID0 0x00 +#define TCAN4X5X_DEV_ID1 0x04 +#define TCAN4X5X_REV 0x08 +#define TCAN4X5X_STATUS 0x0C +#define TCAN4X5X_ERROR_STATUS 0x10 +#define TCAN4X5X_CONTROL 0x14 + +#define TCAN4X5X_CONFIG 0x800 +#define TCAN4X5X_TS_PRESCALE 0x804 +#define TCAN4X5X_TEST_REG 0x808 +#define TCAN4X5X_INT_FLAGS 0x820 +#define TCAN4X5X_MCAN_INT_REG 0x824 +#define TCAN4X5X_INT_EN 0x830 + + +/* Interrupt bits */ +#define TCAN4X5X_CANBUSTERMOPEN_INT_EN BIT(30) +#define TCAN4X5X_CANHCANL_INT_EN BIT(29) +#define TCAN4X5X_CANHBAT_INT_EN BIT(28) +#define TCAN4X5X_CANLGND_INT_EN BIT(27) +#define TCAN4X5X_CANBUSOPEN_INT_EN BIT(26) +#define TCAN4X5X_CANBUSGND_INT_EN BIT(25) +#define TCAN4X5X_CANBUSBAT_INT_EN BIT(24) +#define TCAN4X5X_UVSUP_INT_EN BIT(22) +#define TCAN4X5X_UVIO_INT_EN BIT(21) +#define TCAN4X5X_TSD_INT_EN BIT(19) +#define TCAN4X5X_ECCERR_INT_EN BIT(16) +#define TCAN4X5X_CANINT_INT_EN BIT(15) +#define TCAN4X5X_LWU_INT_EN BIT(14) +#define TCAN4X5X_CANSLNT_INT_EN BIT(10) +#define TCAN4X5X_CANDOM_INT_EN BIT(8) +#define TCAN4X5X_CANBUS_ERR_INT_EN BIT(5) +#define TCAN4X5X_BUS_FAULT BIT(4) +#define TCAN4X5X_MCAN_INT BIT(1) +#define TCAN4X5X_ENABLE_TCAN_INT (TCAN4X5X_MCAN_INT | \ + TCAN4X5X_BUS_FAULT | \ + TCAN4X5X_CANBUS_ERR_INT_EN | \ + TCAN4X5X_CANINT_INT_EN) + +/* MCAN Interrupt bits */ +#define TCAN4X5X_MCAN_IR_ARA BIT(29) +#define TCAN4X5X_MCAN_IR_PED BIT(28) +#define TCAN4X5X_MCAN_IR_PEA BIT(27) +#define TCAN4X5X_MCAN_IR_WD BIT(26) +#define TCAN4X5X_MCAN_IR_BO BIT(25) +#define TCAN4X5X_MCAN_IR_EW BIT(24) +#define TCAN4X5X_MCAN_IR_EP BIT(23) +#define TCAN4X5X_MCAN_IR_ELO BIT(22) +#define TCAN4X5X_MCAN_IR_BEU BIT(21) +#define TCAN4X5X_MCAN_IR_BEC BIT(20) +#define TCAN4X5X_MCAN_IR_DRX BIT(19) +#define TCAN4X5X_MCAN_IR_TOO BIT(18) +#define TCAN4X5X_MCAN_IR_MRAF BIT(17) +#define TCAN4X5X_MCAN_IR_TSW BIT(16) +#define TCAN4X5X_MCAN_IR_TEFL BIT(15) +#define TCAN4X5X_MCAN_IR_TEFF BIT(14) +#define TCAN4X5X_MCAN_IR_TEFW BIT(13) +#define TCAN4X5X_MCAN_IR_TEFN BIT(12) +#define TCAN4X5X_MCAN_IR_TFE BIT(11) +#define TCAN4X5X_MCAN_IR_TCF BIT(10)
drivers/net/can/m_can/tcan4x5x.c line 85
Quoted 10 lines
drivers/net/can/m_can/tcan4x5x.c new file +10
-0+#define TCAN4X5X_MCAN_IR_TC BIT(9) +#define TCAN4X5X_MCAN_IR_HPM BIT(8) +#define TCAN4X5X_MCAN_IR_RF1L BIT(7) +#define TCAN4X5X_MCAN_IR_RF1F BIT(6) +#define TCAN4X5X_MCAN_IR_RF1W BIT(5) +#define TCAN4X5X_MCAN_IR_RF1N BIT(4) +#define TCAN4X5X_MCAN_IR_RF0L BIT(3) +#define TCAN4X5X_MCAN_IR_RF0F BIT(2) +#define TCAN4X5X_MCAN_IR_RF0W BIT(1) +#define TCAN4X5X_MCAN_IR_RF0N BIT(0)
These bits are already defined in the common header file.
Quoted 434 lines
drivers/net/can/m_can/tcan4x5x.c new file +434
-0+#define TCAN4X5X_ENABLE_MCAN_INT (TCAN4X5X_MCAN_IR_TC | \ + TCAN4X5X_MCAN_IR_RF0N | \ + TCAN4X5X_MCAN_IR_RF1N | \ + TCAN4X5X_MCAN_IR_RF0F | \ + TCAN4X5X_MCAN_IR_RF1F) +#define TCAN4X5X_MRAM_START 0x8000 +#define TCAN4X5X_MCAN_OFFSET 0x1000 +#define TCAN4X5X_MAX_REGISTER 0x8fff + +#define TCAN4X5X_CLEAR_ALL_INT 0xffffffff +#define TCAN4X5X_SET_ALL_INT 0xffffffff + +#define TCAN4X5X_WRITE_CMD (0x61 << 24) +#define TCAN4X5X_READ_CMD (0x41 << 24) + +#define TCAN4X5X_MODE_SEL_MASK (BIT(7) | BIT(6)) +#define TCAN4X5X_MODE_SLEEP 0x00 +#define TCAN4X5X_MODE_STANDBY BIT(6) +#define TCAN4X5X_MODE_NORMAL BIT(7) + +#define TCAN4X5X_SW_RESET BIT(2) + +#define TCAN4X5X_MCAN_CONFIGURED BIT(5) +#define TCAN4X5X_WATCHDOG_EN BIT(3) +#define TCAN4X5X_WD_60_MS_TIMER 0 +#define TCAN4X5X_WD_600_MS_TIMER BIT(28) +#define TCAN4X5X_WD_3_S_TIMER BIT(29) +#define TCAN4X5X_WD_6_S_TIMER (BIT(28) | BIT(29)) + +struct tcan4x5x_priv { +struct regmap *regmap; +struct spi_device *spi; +struct mutex tcan4x5x_lock;/* SPI device lock */ + +struct m_can_classdev *mcan_dev; + +struct gpio_desc *reset_gpio; +struct gpio_desc *interrupt_gpio; +struct gpio_desc *device_wake_gpio; +struct gpio_desc *device_state_gpio; +struct regulator *power; + +/* Register based ip */ +int mram_start; +int reg_offset; +}; + +static struct can_bittiming_const tcan4x5x_bittiming_const = { + .name =DEVICE_NAME , + .tseg1_min =2 , + .tseg1_max =31 , + .tseg2_min =2 , + .tseg2_max =16 , + .sjw_max =16 , + .brp_min =1 , + .brp_max =32 , + .brp_inc =1 , +}; + +static struct can_bittiming_const tcan4x5x_data_bittiming_const = { + .name =DEVICE_NAME , + .tseg1_min =1 , + .tseg1_max =32 , + .tseg2_min =1 , + .tseg2_max =16 , + .sjw_max =16 , + .brp_min =1 , + .brp_max =32 , + .brp_inc =1 , +}; + +static void tcan4x5x_check_wake (struct tcan4x5x_priv *priv) +{ +int wake_state =0 ; + +if (priv->device_state_gpio) + wake_state =gpiod_get_value (priv->device_state_gpio); + +if (priv->device_wake_gpio && wake_state) { +gpiod_set_value (priv->device_wake_gpio,1 ); +udelay (100 ); +gpiod_set_value (priv->device_wake_gpio,0 ); +udelay (100 ); +gpiod_set_value (priv->device_wake_gpio,1 ); + } +} + +static int regmap_spi_gather_write (void *context,const void *reg, +size_t reg_len,const void *val, +size_t val_len) +{ +struct device *dev = context; +struct spi_device *spi =to_spi_device (dev); +struct spi_message m; +u32 addr; +struct spi_transfer t[2 ] = {{ .tx_buf = &addr, .len = reg_len, .cs_change =0 ,}, + { .tx_buf = val, .len = val_len, },}; + + addr =TCAN4X5X_WRITE_CMD | (*((u16 *)reg) <<8 ) | val_len >>3 ; + +spi_message_init (&m); +spi_message_add_tail (&t[0 ], &m); +spi_message_add_tail (&t[1 ], &m); + +return spi_sync (spi, &m); +} + +static int tcan4x5x_regmap_write (void *context,const void *data,size_t count) +{ +u16 *reg = (u16 *)(data); +const u32 *val = data +4 ; + +return regmap_spi_gather_write (context, reg,4 , val, count); +} + +static int regmap_spi_async_write (void *context, +const void *reg,size_t reg_len, +const void *val,size_t val_len, +struct regmap_async *a) +{ +return -ENOTSUPP ; +} + +static struct regmap_async *regmap_spi_async_alloc (void ) +{ +return NULL ; +} + +static int tcan4x5x_regmap_read (void *context, +const void *reg,size_t reg_size, +void *val,size_t val_size) +{ +struct device *dev = context; +struct spi_device *spi =to_spi_device (dev); + +u32 addr =TCAN4X5X_READ_CMD | (*((u16 *)reg) <<8 ) | val_size >>2 ; + +return spi_write_then_read (spi, &addr, reg_size, (u32 *)val, val_size); +} + +static struct regmap_bus tcan4x5x_bus = { + .write = tcan4x5x_regmap_write, + .gather_write = regmap_spi_gather_write, + .async_write = regmap_spi_async_write, + .async_alloc = regmap_spi_async_alloc, + .read = tcan4x5x_regmap_read, + .read_flag_mask =0x00 , + .reg_format_endian_default =REGMAP_ENDIAN_NATIVE , + .val_format_endian_default =REGMAP_ENDIAN_NATIVE , +}; + +static u32 tcan4x5x_read_reg (struct m_can_classdev *m_can_class,int reg) +{ +struct tcan4x5x_priv *priv = (struct tcan4x5x_priv *)m_can_class->device_data; +u32 val; + +tcan4x5x_check_wake (priv); + +regmap_read (priv->regmap, priv->reg_offset + reg, &val); + +return val; +} + +static u32 tcan4x5x_read_fifo (struct m_can_classdev *m_can_class, +int addr_offset) +{ +struct tcan4x5x_priv *priv = (struct tcan4x5x_priv *)m_can_class->device_data; +u32 val; + +tcan4x5x_check_wake (priv); + +regmap_read (priv->regmap, priv->mram_start + addr_offset, &val); + +return val; +} + +static int tcan4x5x_write_reg (struct m_can_classdev *m_can_class, +int reg,int val) +{ +struct tcan4x5x_priv *priv = (struct tcan4x5x_priv *)m_can_class->device_data; + +tcan4x5x_check_wake (priv); + +return regmap_write (priv->regmap, priv->reg_offset + reg, val); +} + +static int tcan4x5x_write_fifo (struct m_can_classdev *m_can_class, +int addr_offset,int val) +{ +struct tcan4x5x_priv *priv = (struct tcan4x5x_priv *)m_can_class->device_data; + +tcan4x5x_check_wake (priv); + +return regmap_write (priv->regmap, priv->mram_start + addr_offset, val); +} + +static int tcan4x5x_power_enable (struct regulator *reg,int enable) +{ +if (IS_ERR_OR_NULL (reg)) +return 0 ; + +if (enable) +return regulator_enable (reg); +else +return regulator_disable (reg); +} + +static int tcan4x5x_write_tcan_reg (struct m_can_classdev *m_can_class, +int reg,int val) +{ +struct tcan4x5x_priv *priv = (struct tcan4x5x_priv *)m_can_class->device_data; + +tcan4x5x_check_wake (priv); + +return regmap_write (priv->regmap, reg, val); +} + +static int tcan4x5x_clear_interrupts (struct m_can_classdev *class_dev) +{ +struct tcan4x5x_priv *tcan4x5x = (struct tcan4x5x_priv *)class_dev->device_data; +int ret; + +tcan4x5x_check_wake (tcan4x5x); + + ret =tcan4x5x_write_tcan_reg (class_dev,TCAN4X5X_STATUS , +TCAN4X5X_CLEAR_ALL_INT ); +if (ret) +return -EIO ; + + ret =tcan4x5x_write_tcan_reg (class_dev,TCAN4X5X_MCAN_INT_REG , +TCAN4X5X_ENABLE_MCAN_INT ); +if (ret) +return -EIO ; + + ret =tcan4x5x_write_tcan_reg (class_dev,TCAN4X5X_INT_FLAGS , +TCAN4X5X_CLEAR_ALL_INT ); +if (ret) +return -EIO ; + + + ret =tcan4x5x_write_tcan_reg (class_dev,TCAN4X5X_ERROR_STATUS , +TCAN4X5X_CLEAR_ALL_INT ); +if (ret) +return -EIO ; + +return ret; +} + +static int tcan4x5x_init (struct m_can_classdev *class_dev) +{ +struct tcan4x5x_priv *tcan4x5x = (struct tcan4x5x_priv *)class_dev->device_data; +int ret; + +tcan4x5x_check_wake (tcan4x5x); + + ret =tcan4x5x_clear_interrupts (class_dev); +if (ret) +return ret; + + ret =tcan4x5x_write_tcan_reg (class_dev,TCAN4X5X_INT_EN , +TCAN4X5X_ENABLE_TCAN_INT ); +if (ret) +return -EIO ; + + ret =regmap_update_bits (tcan4x5x->regmap,TCAN4X5X_CONFIG , +TCAN4X5X_MODE_SEL_MASK ,TCAN4X5X_MODE_NORMAL ); +if (ret) +return -EIO ; + +/* Zero out the MCAN buffers */ +m_can_init_ram (class_dev); + +return ret; +} + +static int tcan4x5x_parse_config (struct m_can_classdev *class_dev) +{ +struct tcan4x5x_priv *tcan4x5x = (struct tcan4x5x_priv *)class_dev->device_data; + + tcan4x5x->reset_gpio =devm_gpiod_get_optional (class_dev->dev, +"reset" ,GPIOD_OUT_LOW ); +if (IS_ERR (tcan4x5x->reset_gpio)) + tcan4x5x->reset_gpio =NULL ; + + tcan4x5x->device_wake_gpio =devm_gpiod_get_optional (class_dev->dev, +"device-wake" , +GPIOD_OUT_HIGH ); +if (IS_ERR (tcan4x5x->device_wake_gpio)) + tcan4x5x->device_wake_gpio =NULL ; + + tcan4x5x->device_state_gpio =devm_gpiod_get_optional (class_dev->dev, +"device-state" , +GPIOD_IN ); +if (IS_ERR (tcan4x5x->device_state_gpio)) + tcan4x5x->device_state_gpio =NULL ; + + tcan4x5x->interrupt_gpio =devm_gpiod_get (class_dev->dev, +"data-ready" ,GPIOD_IN ); +if (IS_ERR (tcan4x5x->interrupt_gpio)) { +dev_err (class_dev->dev,"data-ready gpio not defined\n" ); +return -EINVAL ; + } + + class_dev->net->irq =gpiod_to_irq (tcan4x5x->interrupt_gpio); + + tcan4x5x->power =devm_regulator_get_optional (class_dev->dev, +"vsup" ); +if (PTR_ERR (tcan4x5x->power) == -EPROBE_DEFER ) +return -EPROBE_DEFER ; + +return 0 ; +} + +static const struct regmap_config tcan4x5x_regmap = { + .reg_bits =32 , + .val_bits =32 , + .cache_type =REGCACHE_NONE , + .max_register =TCAN4X5X_MAX_REGISTER , +}; + +static int tcan4x5x_can_probe (struct spi_device *spi) +{ +struct tcan4x5x_priv *priv; +struct m_can_classdev *mcan_class; +int freq, ret; + + mcan_class =m_can_core_allocate_dev (&spi->dev); + priv =devm_kzalloc (&spi->dev,sizeof (*priv),GFP_KERNEL ); +if (!priv) +return -ENOMEM ; + + mcan_class->device_data = priv; + +m_can_core_get_clocks (mcan_class); +if (IS_ERR (mcan_class->cclk)) { +dev_err (&spi->dev,"no CAN clock source defined\n" ); + freq =TCAN4X5X_EXT_CLK_DEF ; + }else { + freq =clk_get_rate (mcan_class->cclk); + } + +/* Sanity check */ +if (freq <20000000 || freq >TCAN4X5X_EXT_CLK_DEF ) +return -ERANGE ; + + priv->reg_offset =TCAN4X5X_MCAN_OFFSET ; + priv->mram_start =TCAN4X5X_MRAM_START ; + priv->spi = spi; + priv->mcan_dev = mcan_class; + + mcan_class->pm_clock_support =0 ; + mcan_class->can.clock.freq = freq; + mcan_class->dev = &spi->dev; + + mcan_class->device_init = &tcan4x5x_init; + mcan_class->read_reg = &tcan4x5x_read_reg; + mcan_class->write_reg = &tcan4x5x_write_reg; + mcan_class->write_fifo = &tcan4x5x_write_fifo; + mcan_class->read_fifo = &tcan4x5x_read_fifo; + mcan_class->clr_dev_interrupts = &tcan4x5x_clear_interrupts; + mcan_class->is_peripherial = true; + + mcan_class->bit_timing = &tcan4x5x_bittiming_const; + mcan_class->data_timing = &tcan4x5x_data_bittiming_const; + +spi_set_drvdata (spi, priv); + + ret =tcan4x5x_parse_config (mcan_class); +if (ret) + goto out_clk; + +/* Configure the SPI bus */ + spi->bits_per_word =32 ; + ret =spi_setup (spi); +if (ret) + goto out_clk; + + priv->regmap =devm_regmap_init (&spi->dev, &tcan4x5x_bus, + &spi->dev, &tcan4x5x_regmap); + +mutex_init (&priv->tcan4x5x_lock); + +tcan4x5x_power_enable (priv->power,1 ); + + ret =m_can_core_register (mcan_class); +if (ret) + goto reg_err; + +netdev_info (mcan_class->net,"TCAN4X5X successfully initialized.\n" ); +return 0 ; + +reg_err: +tcan4x5x_power_enable (priv->power,0 ); +out_clk: +if (!IS_ERR (mcan_class->cclk)) { +clk_disable_unprepare (mcan_class->cclk); +clk_disable_unprepare (mcan_class->hclk); + } + +dev_err (&spi->dev,"Probe failed, err=%d\n" , -ret); +return ret; +} + +static int tcan4x5x_can_remove (struct spi_device *spi) +{ +struct tcan4x5x_priv *priv =spi_get_drvdata (spi); + +tcan4x5x_power_enable (priv->power,0 ); + +m_can_core_unregister (priv->mcan_dev); + +return 0 ; +} + +static const struct of_device_id tcan4x5x_of_match[] = { + { .compatible ="ti,tcan4x5x" , }, + { } +}; +MODULE_DEVICE_TABLE (of, tcan4x5x_of_match); + +static const struct spi_device_id tcan4x5x_id_table[] = { + { + .name ="tcan4x5x" , + .driver_data =0 , + }, + { } +}; +MODULE_DEVICE_TABLE (spi, tcan4x5x_id_table); + +static struct spi_driver tcan4x5x_can_driver = { + .driver = { + .name =DEVICE_NAME , + .of_match_table = tcan4x5x_of_match, + .pm =NULL ,
drivers/net/can/m_can/tcan4x5x.c line 529
Quoted 10 lines
drivers/net/can/m_can/tcan4x5x.c new file +10
-0+ }, + .id_table = tcan4x5x_id_table, + .probe = tcan4x5x_can_probe, + .remove = tcan4x5x_can_remove, +}; +module_spi_driver (tcan4x5x_can_driver); + +MODULE_AUTHOR ("Dan Murphy <[email protected]>" ); +MODULE_DESCRIPTION ("Texas Instruments TCAN4x5x CAN driver" ); +MODULE_LICENSE ("GPL v2" );
Curious to hear about the performance of M_CAN connected via SPI. Does it miss or drop messages?
Thanks for your patience,
Wolfgang.
Re: [PATCH v4 4/4] can: tcan4x5x: Add tcan4x5x driver to the kernel
Wolfgang
Sorry for the late reply
Quoted 117 lines · Wolfgang Grandegger
Hello,
Am 17.01.19 um 21:06 schrieb Dan Murphy:
Quoted 114 lines
Add the TCAN4x5x SPI CAN driver. This device
uses the Bosch MCAN IP core along with a SPI
interface map. Leverage the MCAN common core
code to manage the MCAN IP.This device has a special method to indicate a
write/read operation on the data payload.Signed-off-by Dan Murphy <[email protected]>--- drivers/net/can/m_can/Kconfig | 6 + drivers/net/can/m_can/tcan4x5x.c | 529 +++++++++++++++++++++++++++++++ 2 files changed, 535 insertions(+) create mode 100644 drivers/net/can/m_can/tcan4x5x.cdrivers/net/can/m_can/Kconfig +6
-0@@ -15,3 +15,9 @@ config CAN_M_CAN_PLATFORM tristate "Bosch M_CAN devices" ---help--- Say Y here if you want to support for Bosch M_CAN controller. + +config CAN_M_CAN_TCAN4X5X + depends on CAN_M_CAN + tristate "TCAN4X5X M_CAN device" + ---help--- + Say Y here if you want to support for TI M_CAN controller.drivers/net/can/m_can/tcan4x5x.c new file +79
-0@@ -0,0 +1,529 @@ +// SPDX-License-Identifier: GPL-2.0 +// SPI to CAN driver for the Texas Instruments TCAN4x5x +// Copyright (C) 2018-19 Texas Instruments Incorporated - http://www.ti.com/ + +#include <linux/regmap.h> +#include <linux/spi/spi.h> + +#include <linux/regulator/consumer.h> +#include <linux/gpio/consumer.h> + +#include "m_can_platform.h" + +#define DEVICE_NAME "tcan4x5x" +#define TCAN4X5X_EXT_CLK_DEF 40000000 + +#define TCAN4X5X_DEV_ID0 0x00 +#define TCAN4X5X_DEV_ID1 0x04 +#define TCAN4X5X_REV 0x08 +#define TCAN4X5X_STATUS 0x0C +#define TCAN4X5X_ERROR_STATUS 0x10 +#define TCAN4X5X_CONTROL 0x14 + +#define TCAN4X5X_CONFIG 0x800 +#define TCAN4X5X_TS_PRESCALE 0x804 +#define TCAN4X5X_TEST_REG 0x808 +#define TCAN4X5X_INT_FLAGS 0x820 +#define TCAN4X5X_MCAN_INT_REG 0x824 +#define TCAN4X5X_INT_EN 0x830 + + +/* Interrupt bits */ +#define TCAN4X5X_CANBUSTERMOPEN_INT_EN BIT(30) +#define TCAN4X5X_CANHCANL_INT_EN BIT(29) +#define TCAN4X5X_CANHBAT_INT_EN BIT(28) +#define TCAN4X5X_CANLGND_INT_EN BIT(27) +#define TCAN4X5X_CANBUSOPEN_INT_EN BIT(26) +#define TCAN4X5X_CANBUSGND_INT_EN BIT(25) +#define TCAN4X5X_CANBUSBAT_INT_EN BIT(24) +#define TCAN4X5X_UVSUP_INT_EN BIT(22) +#define TCAN4X5X_UVIO_INT_EN BIT(21) +#define TCAN4X5X_TSD_INT_EN BIT(19) +#define TCAN4X5X_ECCERR_INT_EN BIT(16) +#define TCAN4X5X_CANINT_INT_EN BIT(15) +#define TCAN4X5X_LWU_INT_EN BIT(14) +#define TCAN4X5X_CANSLNT_INT_EN BIT(10) +#define TCAN4X5X_CANDOM_INT_EN BIT(8) +#define TCAN4X5X_CANBUS_ERR_INT_EN BIT(5) +#define TCAN4X5X_BUS_FAULT BIT(4) +#define TCAN4X5X_MCAN_INT BIT(1) +#define TCAN4X5X_ENABLE_TCAN_INT (TCAN4X5X_MCAN_INT | \ + TCAN4X5X_BUS_FAULT | \ + TCAN4X5X_CANBUS_ERR_INT_EN | \ + TCAN4X5X_CANINT_INT_EN) + +/* MCAN Interrupt bits */ +#define TCAN4X5X_MCAN_IR_ARA BIT(29) +#define TCAN4X5X_MCAN_IR_PED BIT(28) +#define TCAN4X5X_MCAN_IR_PEA BIT(27) +#define TCAN4X5X_MCAN_IR_WD BIT(26) +#define TCAN4X5X_MCAN_IR_BO BIT(25) +#define TCAN4X5X_MCAN_IR_EW BIT(24) +#define TCAN4X5X_MCAN_IR_EP BIT(23) +#define TCAN4X5X_MCAN_IR_ELO BIT(22) +#define TCAN4X5X_MCAN_IR_BEU BIT(21) +#define TCAN4X5X_MCAN_IR_BEC BIT(20) +#define TCAN4X5X_MCAN_IR_DRX BIT(19) +#define TCAN4X5X_MCAN_IR_TOO BIT(18) +#define TCAN4X5X_MCAN_IR_MRAF BIT(17) +#define TCAN4X5X_MCAN_IR_TSW BIT(16) +#define TCAN4X5X_MCAN_IR_TEFL BIT(15) +#define TCAN4X5X_MCAN_IR_TEFF BIT(14) +#define TCAN4X5X_MCAN_IR_TEFW BIT(13) +#define TCAN4X5X_MCAN_IR_TEFN BIT(12) +#define TCAN4X5X_MCAN_IR_TFE BIT(11) +#define TCAN4X5X_MCAN_IR_TCF BIT(10) +#define TCAN4X5X_MCAN_IR_TC BIT(9) +#define TCAN4X5X_MCAN_IR_HPM BIT(8) +#define TCAN4X5X_MCAN_IR_RF1L BIT(7) +#define TCAN4X5X_MCAN_IR_RF1F BIT(6)
Quoted 9 lines
drivers/net/can/m_can/tcan4x5x.c line 85
Quoted 6 lines
drivers/net/can/m_can/tcan4x5x.c new file +6
-0+#define TCAN4X5X_MCAN_IR_RF1W BIT(5) +#define TCAN4X5X_MCAN_IR_RF1N BIT(4) +#define TCAN4X5X_MCAN_IR_RF0L BIT(3) +#define TCAN4X5X_MCAN_IR_RF0F BIT(2) +#define TCAN4X5X_MCAN_IR_RF0W BIT(1) +#define TCAN4X5X_MCAN_IR_RF0N BIT(0)These bits are already defined in the common header file.
These are TCAN specific interrupt enable bits there are not in the Bosch register set
Quoted 439 lines
Quoted 439 lines
drivers/net/can/m_can/tcan4x5x.c new file +439
-0+#define TCAN4X5X_ENABLE_MCAN_INT (TCAN4X5X_MCAN_IR_TC | \ + TCAN4X5X_MCAN_IR_RF0N | \ + TCAN4X5X_MCAN_IR_RF1N | \ + TCAN4X5X_MCAN_IR_RF0F | \ + TCAN4X5X_MCAN_IR_RF1F) +#define TCAN4X5X_MRAM_START 0x8000 +#define TCAN4X5X_MCAN_OFFSET 0x1000 +#define TCAN4X5X_MAX_REGISTER 0x8fff + +#define TCAN4X5X_CLEAR_ALL_INT 0xffffffff +#define TCAN4X5X_SET_ALL_INT 0xffffffff + +#define TCAN4X5X_WRITE_CMD (0x61 << 24) +#define TCAN4X5X_READ_CMD (0x41 << 24) + +#define TCAN4X5X_MODE_SEL_MASK (BIT(7) | BIT(6)) +#define TCAN4X5X_MODE_SLEEP 0x00 +#define TCAN4X5X_MODE_STANDBY BIT(6) +#define TCAN4X5X_MODE_NORMAL BIT(7) + +#define TCAN4X5X_SW_RESET BIT(2) + +#define TCAN4X5X_MCAN_CONFIGURED BIT(5) +#define TCAN4X5X_WATCHDOG_EN BIT(3) +#define TCAN4X5X_WD_60_MS_TIMER 0 +#define TCAN4X5X_WD_600_MS_TIMER BIT(28) +#define TCAN4X5X_WD_3_S_TIMER BIT(29) +#define TCAN4X5X_WD_6_S_TIMER (BIT(28) | BIT(29)) + +struct tcan4x5x_priv { +struct regmap *regmap; +struct spi_device *spi; +struct mutex tcan4x5x_lock;/* SPI device lock */ + +struct m_can_classdev *mcan_dev; + +struct gpio_desc *reset_gpio; +struct gpio_desc *interrupt_gpio; +struct gpio_desc *device_wake_gpio; +struct gpio_desc *device_state_gpio; +struct regulator *power; + +/* Register based ip */ +int mram_start; +int reg_offset; +}; + +static struct can_bittiming_const tcan4x5x_bittiming_const = { + .name =DEVICE_NAME , + .tseg1_min =2 , + .tseg1_max =31 , + .tseg2_min =2 , + .tseg2_max =16 , + .sjw_max =16 , + .brp_min =1 , + .brp_max =32 , + .brp_inc =1 , +}; + +static struct can_bittiming_const tcan4x5x_data_bittiming_const = { + .name =DEVICE_NAME , + .tseg1_min =1 , + .tseg1_max =32 , + .tseg2_min =1 , + .tseg2_max =16 , + .sjw_max =16 , + .brp_min =1 , + .brp_max =32 , + .brp_inc =1 , +}; + +static void tcan4x5x_check_wake (struct tcan4x5x_priv *priv) +{ +int wake_state =0 ; + +if (priv->device_state_gpio) + wake_state =gpiod_get_value (priv->device_state_gpio); + +if (priv->device_wake_gpio && wake_state) { +gpiod_set_value (priv->device_wake_gpio,1 ); +udelay (100 ); +gpiod_set_value (priv->device_wake_gpio,0 ); +udelay (100 ); +gpiod_set_value (priv->device_wake_gpio,1 ); + } +} + +static int regmap_spi_gather_write (void *context,const void *reg, +size_t reg_len,const void *val, +size_t val_len) +{ +struct device *dev = context; +struct spi_device *spi =to_spi_device (dev); +struct spi_message m; +u32 addr; +struct spi_transfer t[2 ] = {{ .tx_buf = &addr, .len = reg_len, .cs_change =0 ,}, + { .tx_buf = val, .len = val_len, },}; + + addr =TCAN4X5X_WRITE_CMD | (*((u16 *)reg) <<8 ) | val_len >>3 ; + +spi_message_init (&m); +spi_message_add_tail (&t[0 ], &m); +spi_message_add_tail (&t[1 ], &m); + +return spi_sync (spi, &m); +} + +static int tcan4x5x_regmap_write (void *context,const void *data,size_t count) +{ +u16 *reg = (u16 *)(data); +const u32 *val = data +4 ; + +return regmap_spi_gather_write (context, reg,4 , val, count); +} + +static int regmap_spi_async_write (void *context, +const void *reg,size_t reg_len, +const void *val,size_t val_len, +struct regmap_async *a) +{ +return -ENOTSUPP ; +} + +static struct regmap_async *regmap_spi_async_alloc (void ) +{ +return NULL ; +} + +static int tcan4x5x_regmap_read (void *context, +const void *reg,size_t reg_size, +void *val,size_t val_size) +{ +struct device *dev = context; +struct spi_device *spi =to_spi_device (dev); + +u32 addr =TCAN4X5X_READ_CMD | (*((u16 *)reg) <<8 ) | val_size >>2 ; + +return spi_write_then_read (spi, &addr, reg_size, (u32 *)val, val_size); +} + +static struct regmap_bus tcan4x5x_bus = { + .write = tcan4x5x_regmap_write, + .gather_write = regmap_spi_gather_write, + .async_write = regmap_spi_async_write, + .async_alloc = regmap_spi_async_alloc, + .read = tcan4x5x_regmap_read, + .read_flag_mask =0x00 , + .reg_format_endian_default =REGMAP_ENDIAN_NATIVE , + .val_format_endian_default =REGMAP_ENDIAN_NATIVE , +}; + +static u32 tcan4x5x_read_reg (struct m_can_classdev *m_can_class,int reg) +{ +struct tcan4x5x_priv *priv = (struct tcan4x5x_priv *)m_can_class->device_data; +u32 val; + +tcan4x5x_check_wake (priv); + +regmap_read (priv->regmap, priv->reg_offset + reg, &val); + +return val; +} + +static u32 tcan4x5x_read_fifo (struct m_can_classdev *m_can_class, +int addr_offset) +{ +struct tcan4x5x_priv *priv = (struct tcan4x5x_priv *)m_can_class->device_data; +u32 val; + +tcan4x5x_check_wake (priv); + +regmap_read (priv->regmap, priv->mram_start + addr_offset, &val); + +return val; +} + +static int tcan4x5x_write_reg (struct m_can_classdev *m_can_class, +int reg,int val) +{ +struct tcan4x5x_priv *priv = (struct tcan4x5x_priv *)m_can_class->device_data; + +tcan4x5x_check_wake (priv); + +return regmap_write (priv->regmap, priv->reg_offset + reg, val); +} + +static int tcan4x5x_write_fifo (struct m_can_classdev *m_can_class, +int addr_offset,int val) +{ +struct tcan4x5x_priv *priv = (struct tcan4x5x_priv *)m_can_class->device_data; + +tcan4x5x_check_wake (priv); + +return regmap_write (priv->regmap, priv->mram_start + addr_offset, val); +} + +static int tcan4x5x_power_enable (struct regulator *reg,int enable) +{ +if (IS_ERR_OR_NULL (reg)) +return 0 ; + +if (enable) +return regulator_enable (reg); +else +return regulator_disable (reg); +} + +static int tcan4x5x_write_tcan_reg (struct m_can_classdev *m_can_class, +int reg,int val) +{ +struct tcan4x5x_priv *priv = (struct tcan4x5x_priv *)m_can_class->device_data; + +tcan4x5x_check_wake (priv); + +return regmap_write (priv->regmap, reg, val); +} + +static int tcan4x5x_clear_interrupts (struct m_can_classdev *class_dev) +{ +struct tcan4x5x_priv *tcan4x5x = (struct tcan4x5x_priv *)class_dev->device_data; +int ret; + +tcan4x5x_check_wake (tcan4x5x); + + ret =tcan4x5x_write_tcan_reg (class_dev,TCAN4X5X_STATUS , +TCAN4X5X_CLEAR_ALL_INT ); +if (ret) +return -EIO ; + + ret =tcan4x5x_write_tcan_reg (class_dev,TCAN4X5X_MCAN_INT_REG , +TCAN4X5X_ENABLE_MCAN_INT ); +if (ret) +return -EIO ; + + ret =tcan4x5x_write_tcan_reg (class_dev,TCAN4X5X_INT_FLAGS , +TCAN4X5X_CLEAR_ALL_INT ); +if (ret) +return -EIO ; + + + ret =tcan4x5x_write_tcan_reg (class_dev,TCAN4X5X_ERROR_STATUS , +TCAN4X5X_CLEAR_ALL_INT ); +if (ret) +return -EIO ; + +return ret; +} + +static int tcan4x5x_init (struct m_can_classdev *class_dev) +{ +struct tcan4x5x_priv *tcan4x5x = (struct tcan4x5x_priv *)class_dev->device_data; +int ret; + +tcan4x5x_check_wake (tcan4x5x); + + ret =tcan4x5x_clear_interrupts (class_dev); +if (ret) +return ret; + + ret =tcan4x5x_write_tcan_reg (class_dev,TCAN4X5X_INT_EN , +TCAN4X5X_ENABLE_TCAN_INT ); +if (ret) +return -EIO ; + + ret =regmap_update_bits (tcan4x5x->regmap,TCAN4X5X_CONFIG , +TCAN4X5X_MODE_SEL_MASK ,TCAN4X5X_MODE_NORMAL ); +if (ret) +return -EIO ; + +/* Zero out the MCAN buffers */ +m_can_init_ram (class_dev); + +return ret; +} + +static int tcan4x5x_parse_config (struct m_can_classdev *class_dev) +{ +struct tcan4x5x_priv *tcan4x5x = (struct tcan4x5x_priv *)class_dev->device_data; + + tcan4x5x->reset_gpio =devm_gpiod_get_optional (class_dev->dev, +"reset" ,GPIOD_OUT_LOW ); +if (IS_ERR (tcan4x5x->reset_gpio)) + tcan4x5x->reset_gpio =NULL ; + + tcan4x5x->device_wake_gpio =devm_gpiod_get_optional (class_dev->dev, +"device-wake" , +GPIOD_OUT_HIGH ); +if (IS_ERR (tcan4x5x->device_wake_gpio)) + tcan4x5x->device_wake_gpio =NULL ; + + tcan4x5x->device_state_gpio =devm_gpiod_get_optional (class_dev->dev, +"device-state" , +GPIOD_IN ); +if (IS_ERR (tcan4x5x->device_state_gpio)) + tcan4x5x->device_state_gpio =NULL ; + + tcan4x5x->interrupt_gpio =devm_gpiod_get (class_dev->dev, +"data-ready" ,GPIOD_IN ); +if (IS_ERR (tcan4x5x->interrupt_gpio)) { +dev_err (class_dev->dev,"data-ready gpio not defined\n" ); +return -EINVAL ; + } + + class_dev->net->irq =gpiod_to_irq (tcan4x5x->interrupt_gpio); + + tcan4x5x->power =devm_regulator_get_optional (class_dev->dev, +"vsup" ); +if (PTR_ERR (tcan4x5x->power) == -EPROBE_DEFER ) +return -EPROBE_DEFER ; + +return 0 ; +} + +static const struct regmap_config tcan4x5x_regmap = { + .reg_bits =32 , + .val_bits =32 , + .cache_type =REGCACHE_NONE , + .max_register =TCAN4X5X_MAX_REGISTER , +}; + +static int tcan4x5x_can_probe (struct spi_device *spi) +{ +struct tcan4x5x_priv *priv; +struct m_can_classdev *mcan_class; +int freq, ret; + + mcan_class =m_can_core_allocate_dev (&spi->dev); + priv =devm_kzalloc (&spi->dev,sizeof (*priv),GFP_KERNEL ); +if (!priv) +return -ENOMEM ; + + mcan_class->device_data = priv; + +m_can_core_get_clocks (mcan_class); +if (IS_ERR (mcan_class->cclk)) { +dev_err (&spi->dev,"no CAN clock source defined\n" ); + freq =TCAN4X5X_EXT_CLK_DEF ; + }else { + freq =clk_get_rate (mcan_class->cclk); + } + +/* Sanity check */ +if (freq <20000000 || freq >TCAN4X5X_EXT_CLK_DEF ) +return -ERANGE ; + + priv->reg_offset =TCAN4X5X_MCAN_OFFSET ; + priv->mram_start =TCAN4X5X_MRAM_START ; + priv->spi = spi; + priv->mcan_dev = mcan_class; + + mcan_class->pm_clock_support =0 ; + mcan_class->can.clock.freq = freq; + mcan_class->dev = &spi->dev; + + mcan_class->device_init = &tcan4x5x_init; + mcan_class->read_reg = &tcan4x5x_read_reg; + mcan_class->write_reg = &tcan4x5x_write_reg; + mcan_class->write_fifo = &tcan4x5x_write_fifo; + mcan_class->read_fifo = &tcan4x5x_read_fifo; + mcan_class->clr_dev_interrupts = &tcan4x5x_clear_interrupts; + mcan_class->is_peripherial = true; + + mcan_class->bit_timing = &tcan4x5x_bittiming_const; + mcan_class->data_timing = &tcan4x5x_data_bittiming_const; + +spi_set_drvdata (spi, priv); + + ret =tcan4x5x_parse_config (mcan_class); +if (ret) + goto out_clk; + +/* Configure the SPI bus */ + spi->bits_per_word =32 ; + ret =spi_setup (spi); +if (ret) + goto out_clk; + + priv->regmap =devm_regmap_init (&spi->dev, &tcan4x5x_bus, + &spi->dev, &tcan4x5x_regmap); + +mutex_init (&priv->tcan4x5x_lock); + +tcan4x5x_power_enable (priv->power,1 ); + + ret =m_can_core_register (mcan_class); +if (ret) + goto reg_err; + +netdev_info (mcan_class->net,"TCAN4X5X successfully initialized.\n" ); +return 0 ; + +reg_err: +tcan4x5x_power_enable (priv->power,0 ); +out_clk: +if (!IS_ERR (mcan_class->cclk)) { +clk_disable_unprepare (mcan_class->cclk); +clk_disable_unprepare (mcan_class->hclk); + } + +dev_err (&spi->dev,"Probe failed, err=%d\n" , -ret); +return ret; +} + +static int tcan4x5x_can_remove (struct spi_device *spi) +{ +struct tcan4x5x_priv *priv =spi_get_drvdata (spi); + +tcan4x5x_power_enable (priv->power,0 ); + +m_can_core_unregister (priv->mcan_dev); + +return 0 ; +} + +static const struct of_device_id tcan4x5x_of_match[] = { + { .compatible ="ti,tcan4x5x" , }, + { } +}; +MODULE_DEVICE_TABLE (of, tcan4x5x_of_match); + +static const struct spi_device_id tcan4x5x_id_table[] = { + { + .name ="tcan4x5x" , + .driver_data =0 , + }, + { } +}; +MODULE_DEVICE_TABLE (spi, tcan4x5x_id_table); + +static struct spi_driver tcan4x5x_can_driver = { + .driver = { + .name =DEVICE_NAME , + .of_match_table = tcan4x5x_of_match, + .pm =NULL , + }, + .id_table = tcan4x5x_id_table, + .probe = tcan4x5x_can_probe, + .remove = tcan4x5x_can_remove, +};
Quoted 9 lines
drivers/net/can/m_can/tcan4x5x.c line 529
Quoted 5 lines
drivers/net/can/m_can/tcan4x5x.c new file +5
-0+module_spi_driver (tcan4x5x_can_driver); + +MODULE_AUTHOR ("Dan Murphy <[email protected]>" ); +MODULE_DESCRIPTION ("Texas Instruments TCAN4x5x CAN driver" ); +MODULE_LICENSE ("GPL v2" );Curious to hear about the performance of
M_CANconnected via SPI. Does it miss or drop messages?
Here is some cangen data we have.
We can probably speed it up but we want to get the code functional first.
/home/root/can/ip link set can0 up type can bitrate 1000000 dbitrate 10000000 fd on
root@am335x-evm:~/can# cat /proc/net/can/stats
5681 transmitted frames (TXF)
5681 received frames (RXF)
0 matched frames (RXMF) 0 % total match ratio (RXMR)
167 frames/s total tx rate (TXR)
167 frames/s total rx rate (RXR)
0 % current match ratio (CRXMR)
0 frames/s current tx rate (CTXR)
0 frames/s current rx rate (CRXR)
0 % max match ratio (MRXMR)
224 frames/s max tx rate (MTXR)
222 frames/s max rx rate (MRXR)
Quoted 4 lines
Thanks for your patience,
Wolfgang.
Signature · 2 lines
------------------ Dan Murphy
Wolfgang
Quoted 413 lines · Dan Murphy
Create a
m_canplatform framework that peripherial devices can register to and use common code and register sets. The peripherial devices may provide read/write and configuration support of the IP.Signed-off-by Dan Murphy <[email protected]>--- drivers/net/can/m_can/m_can.c | 6 + drivers/net/can/m_can/m_can_platform.c | 209 +++++++++++++++++++++++++ drivers/net/can/m_can/m_can_platform.h | 163 +++++++++++++++++++ 3 files changed, 378 insertions(+) create mode 100644 drivers/net/can/m_can/m_can_platform.c create mode 100644 drivers/net/can/m_can/m_can_platform.hdrivers/net/can/m_can/m_can.c +6
-0@@ -414,6 +414,9 @@ static inline void m_can_config_endisable(const struct m_can_priv *priv,u32 timeout =10 ;u32 val =0 ; +if (cccr &CCCR_CSR ) + cccr &= ~CCCR_CSR ; +if (enable) {/* enable m_can configuration */ m_can_write (priv,M_CAN_CCCR , cccr |CCCR_INIT );@@ -1155,6 +1158,9 @@ static void m_can_chip_config(struct net_device *dev)m_can_set_bittiming (dev);m_can_config_endisable (priv, false); + +if (priv->device_init) + priv->device_init (priv); }static void m_can_start (struct net_device *dev)drivers/net/can/m_can/m_can_platform.c new file +209
-0@@ -0,0 +1,209 @@ +/* +* CAN bus driver for Bosch M_CAN controller +* +* Copyright (C) 2014 Freescale Semiconductor, Inc. +* Dong Aisheng <[email protected]> +* +* Bosch M_CAN user manual can be obtained from: +* http://www.bosch-semiconductors.de/media/pdf_1/ipmodules_1/m_can/ +* mcan_users_manual_v302.pdf +* +* This file is licensed under the terms of the GNU General Public +* License version 2. This program is licensed "as is" without any +* warranty of any kind, whether express or implied. +*/ + +#include <linux/clk.h> +#include <linux/interrupt.h> +#include <linux/io.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/netdevice.h> +#include <linux/platform_device.h> +#include <linux/pm_runtime.h> +#include <linux/can/dev.h> +#include <linux/pinctrl/consumer.h> + +#include "m_can_platform.h" + +struct m_can_plat_priv { +void __iomem *base; +void __iomem *mram_base; +}; + +static u32 iomap_read_reg (struct m_can_classdev *m_can_class,int reg) +{ +struct m_can_plat_priv *priv = (struct m_can_plat_priv *)m_can_class->device_data; + +return readl (priv->base + reg); +} + +static u32 iomap_read_fifo (struct m_can_classdev *m_can_class,int addr_offset) +{ +struct m_can_plat_priv *priv = (struct m_can_plat_priv *)m_can_class->device_data; + +return readl (priv->mram_base + addr_offset); +} + +static int iomap_write_reg (struct m_can_classdev *m_can_class,int reg,int val) +{ +struct m_can_plat_priv *priv = (struct m_can_plat_priv *)m_can_class->device_data; + +writel (val, priv->base + reg); + +return 0 ; +} + +static int iomap_write_fifo (struct m_can_classdev *m_can_class,int addr_offset,int val) +{ +struct m_can_plat_priv *priv = (struct m_can_plat_priv *)m_can_class->device_data; + +writel (val, priv->base + addr_offset); + +return 0 ; +} + +static int m_can_plat_probe (struct platform_device *pdev) +{ +struct m_can_classdev *mcan_class; +struct m_can_plat_priv *priv; +struct resource *res; +void __iomem *addr; +void __iomem *mram_addr; +int irq, ret =0 ; + + mcan_class =m_can_core_allocate_dev (&pdev->dev); + priv =devm_kzalloc (&pdev->dev,sizeof (*priv),GFP_KERNEL ); +if (!priv) +return -ENOMEM ; + + mcan_class->device_data = priv; + +m_can_core_get_clocks (mcan_class); + + res =platform_get_resource_byname (pdev,IORESOURCE_MEM ,"m_can" ); + addr =devm_ioremap_resource (&pdev->dev, res); + irq =platform_get_irq_byname (pdev,"int0" ); +if (IS_ERR (addr) || irq <0 ) { + ret = -EINVAL ; + goto failed_ret; + } + +/* message ram could be shared */ + res =platform_get_resource_byname (pdev,IORESOURCE_MEM ,"message_ram" ); +if (!res) { + ret = -ENODEV ; + goto failed_ret; + } + + mram_addr =devm_ioremap (&pdev->dev, res->start,resource_size (res)); +if (!mram_addr) { + ret = -ENOMEM ; + goto failed_ret; + } + + priv->base = addr; + priv->mram_base = mram_addr; + + mcan_class->net->irq = irq; + mcan_class->pm_clock_support =1 ; + mcan_class->can.clock.freq =clk_get_rate (mcan_class->cclk); + mcan_class->dev = &pdev->dev; + + mcan_class->read_reg = &iomap_read_reg; + mcan_class->write_reg = &iomap_write_reg; + mcan_class->write_fifo = &iomap_write_fifo; + mcan_class->read_fifo = &iomap_read_fifo; + mcan_class->is_peripherial = false; + +platform_set_drvdata (pdev, mcan_class->dev); + +m_can_init_ram (mcan_class); + + ret =m_can_core_register (mcan_class); + +failed_ret: +return ret; +} + +static __maybe_unused int m_can_suspend (struct device *dev) +{ +return m_can_core_suspend (dev); +} + +static __maybe_unused int m_can_resume (struct device *dev) +{ +return m_can_core_resume (dev); +} + +static int m_can_plat_remove (struct platform_device *pdev) +{ +struct net_device *dev =platform_get_drvdata (pdev); +struct m_can_classdev *mcan_class =netdev_priv (dev); + +m_can_core_unregister (mcan_class); + +platform_set_drvdata (pdev,NULL ); + +return 0 ; +} + +static int __maybe_unused m_can_runtime_suspend (struct device *dev) +{ +struct net_device *ndev =dev_get_drvdata (dev); +struct m_can_classdev *mcan_class =netdev_priv (ndev); + +m_can_core_suspend (dev); + +clk_disable_unprepare (mcan_class->cclk); +clk_disable_unprepare (mcan_class->hclk); + +return 0 ; +} + +static int __maybe_unused m_can_runtime_resume (struct device *dev) +{ +struct net_device *ndev =dev_get_drvdata (dev); +struct m_can_classdev *mcan_class =netdev_priv (ndev); +int err; + + err =clk_prepare_enable (mcan_class->hclk); +if (err) +return err; + + err =clk_prepare_enable (mcan_class->cclk); +if (err) +clk_disable_unprepare (mcan_class->hclk); + +m_can_core_resume (dev); + +return err; +} + +static const struct dev_pm_ops m_can_pmops = { + SET_RUNTIME_PM_OPS(m_can_runtime_suspend, + m_can_runtime_resume, NULL) +SET_SYSTEM_SLEEP_PM_OPS (m_can_suspend, m_can_resume) +}; + +static const struct of_device_id m_can_of_table[] = { + { .compatible ="bosch,m_can" , .data =NULL }, + {/* sentinel */ }, +}; +MODULE_DEVICE_TABLE (of, m_can_of_table); + +static struct platform_driver m_can_plat_driver = { + .driver = { + .name =KBUILD_MODNAME , + .of_match_table = m_can_of_table, + .pm = &m_can_pmops, + }, + .probe = m_can_plat_probe, + .remove = m_can_plat_remove, +}; + +module_platform_driver (m_can_plat_driver); + +MODULE_AUTHOR ("Dong Aisheng <[email protected]>" ); +MODULE_LICENSE ("GPL v2" ); +MODULE_DESCRIPTION ("CAN bus driver for Bosch M_CAN controller" );drivers/net/can/m_can/m_can_platform.h new file +153
-0@@ -0,0 +1,163 @@ +// SPDX-License-Identifier: GPL-2.0 +// Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/ + +#ifndef _CAN_M_CAN_CORE_H_ +#define _CAN_M_CAN_CORE_H_ + +#include <linux/can/core.h> +#include <linux/can/led.h> +#include <linux/completion.h> +#include <linux/device.h> +#include <linux/dma-mapping.h> +#include <linux/freezer.h> +#include <linux/slab.h> +#include <linux/uaccess.h> +#include <linux/clk.h> +#include <linux/delay.h> +#include <linux/interrupt.h> +#include <linux/io.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/netdevice.h> +#include <linux/of.h> +#include <linux/of_device.h> +#include <linux/pm_runtime.h> +#include <linux/iopoll.h> +#include <linux/can/dev.h> +#include <linux/pinctrl/consumer.h> + +/* m_can lec values */ +enum m_can_lec_type { +LEC_NO_ERROR =0 , +LEC_STUFF_ERROR , +LEC_FORM_ERROR , +LEC_ACK_ERROR , +LEC_BIT1_ERROR , +LEC_BIT0_ERROR , +LEC_CRC_ERROR , +LEC_UNUSED , +}; + +enum m_can_mram_cfg { +MRAM_SIDF =0 , +MRAM_XIDF , +MRAM_RXF0 , +MRAM_RXF1 , +MRAM_RXB , +MRAM_TXE , +MRAM_TXB , +MRAM_CFG_NUM , +}; + +/* registers definition */ +enum m_can_reg { +M_CAN_CREL =0x0 , +M_CAN_ENDN =0x4 , +M_CAN_CUST =0x8 , +M_CAN_DBTP =0xc , +M_CAN_TEST =0x10 , +M_CAN_RWD =0x14 , +M_CAN_CCCR =0x18 , +M_CAN_NBTP =0x1c , +M_CAN_TSCC =0x20 , +M_CAN_TSCV =0x24 , +M_CAN_TOCC =0x28 , +M_CAN_TOCV =0x2c , +M_CAN_ECR =0x40 , +M_CAN_PSR =0x44 , +/* TDCR Register only available for version >=3.1.x */ +M_CAN_TDCR =0x48 , +M_CAN_IR =0x50 , +M_CAN_IE =0x54 , +M_CAN_ILS =0x58 , +M_CAN_ILE =0x5c , +M_CAN_GFC =0x80 , +M_CAN_SIDFC =0x84 , +M_CAN_XIDFC =0x88 , +M_CAN_XIDAM =0x90 , +M_CAN_HPMS =0x94 , +M_CAN_NDAT1 =0x98 , +M_CAN_NDAT2 =0x9c , +M_CAN_RXF0C =0xa0 , +M_CAN_RXF0S =0xa4 , +M_CAN_RXF0A =0xa8 , +M_CAN_RXBC =0xac , +M_CAN_RXF1C =0xb0 , +M_CAN_RXF1S =0xb4 , +M_CAN_RXF1A =0xb8 , +M_CAN_RXESC =0xbc , +M_CAN_TXBC =0xc0 , +M_CAN_TXFQS =0xc4 , +M_CAN_TXESC =0xc8 , +M_CAN_TXBRP =0xcc , +M_CAN_TXBAR =0xd0 , +M_CAN_TXBCR =0xd4 , +M_CAN_TXBTO =0xd8 , +M_CAN_TXBCF =0xdc , +M_CAN_TXBTIE =0xe0 , +M_CAN_TXBCIE =0xe4 , +M_CAN_TXEFC =0xf0 , +M_CAN_TXEFS =0xf4 , +M_CAN_TXEFA =0xf8 , +}; + +/* address offset and element number for each FIFO/Buffer in the Message RAM */ +struct mram_cfg { +u16 off; +u8 num; +}; + +struct m_can_classdev ; + +typedef int (*can_dev_init ) (struct m_can_classdev *m_can_class); +typedef int (*can_clr_dev_interrupts ) (struct m_can_classdev *m_can_class); +typedef u32 (*can_reg_read ) (struct m_can_classdev *m_can_class,int reg); +typedef int (*can_reg_write ) (struct m_can_classdev *m_can_class,int reg,int val); +typedef u32 (*can_fifo_read ) (struct m_can_classdev *m_can_class,int addr_offset); +typedef int (*can_fifo_write ) (struct m_can_classdev *m_can_class,int addr_offset,int val); + +struct m_can_classdev { +struct can_priv can; +struct napi_struct napi; +struct net_device *net; +struct device *dev; +struct clk *hclk; +struct clk *cclk; + +struct workqueue_struct *wq; +struct work_struct tx_work; +struct sk_buff *skb; + +struct can_bittiming_const *bit_timing; +struct can_bittiming_const *data_timing; + +void *device_data; + +/* Device specific call backs */ +can_dev_init device_init; +can_clr_dev_interrupts clr_dev_interrupts; +can_reg_read read_reg; +can_reg_write write_reg; +can_fifo_read read_fifo; +can_fifo_write write_fifo; + +int version; +int freq; +u32 irqstatus; + +int pm_clock_support; +bool is_peripherial; + +struct mram_cfg mcfg[MRAM_CFG_NUM ]; +}; +
drivers/net/can/m_can/m_can_platform.h line 163
Quoted 10 lines
drivers/net/can/m_can/m_can_platform.h new file +10
-0+struct m_can_classdev *m_can_core_allocate_dev (struct device *dev); +int m_can_core_register (struct m_can_classdev *m_can_dev); +void m_can_core_unregister (struct m_can_classdev *m_can_dev); +int m_can_core_get_clocks (struct m_can_classdev *m_can_dev); +void m_can_init_ram (struct m_can_classdev *priv); +void m_can_config_endisable (const struct m_can_classdev *priv,bool enable); + +int m_can_core_suspend (struct device *dev); +int m_can_core_resume (struct device *dev); +#endif /* _CAN_M_CAN_CORE_H_ */
This patch set is working for the TCAN and at least boots on io-mapped devices. We have a couple of customers who are looking at using this implementation and we would like to start moving this patch set along in the review.
I did not put a change log in here as there have been no comments for the last 3 patch sets I sent.
This v4 squashes a few bugs I found during testing across all the files.
Dan
Signature · 2 lines
------------------ Dan Murphy
Hello Dan,
Am 17.01.19 um 21:08 schrieb Dan Murphy:
Quoted 17 lines
Wolfgang
Quoted 14 lines · Dan Murphy
Create a
m_canplatform framework that peripherial devices can register to and use common code and register sets. The peripherial devices may provide read/write and configuration support of the IP.Signed-off-by Dan Murphy <[email protected]>--- drivers/net/can/m_can/m_can.c | 6 + drivers/net/can/m_can/m_can_platform.c | 209 +++++++++++++++++++++++++ drivers/net/can/m_can/m_can_platform.h | 163 +++++++++++++++++++ 3 files changed, 378 insertions(+) create mode 100644 drivers/net/can/m_can/m_can_platform.c create mode 100644 drivers/net/can/m_can/m_can_platform.h
... snip ...
Quoted 7 lines
This patch set is working for the TCAN and at least boots on io-mapped devices. We have a couple of customers who are looking at using this implementation and we would like to start moving this patch set along in the review.
I did not put a change log in here as there have been no comments for the last 3 patch sets I sent.
This v4 squashes a few bugs I found during testing across all the files.
OK, after a very busy week I will try to find time for the review... finally.
Thanks,
Wolfgang.
Wolfgang
Quoted 27 lines · Wolfgang Grandegger
Hello Dan,
Am 17.01.19 um 21:08 schrieb Dan Murphy:
Quoted 17 lines
Wolfgang
Quoted 14 lines · Dan Murphy
Create a
m_canplatform framework that peripherial devices can register to and use common code and register sets. The peripherial devices may provide read/write and configuration support of the IP.Signed-off-by Dan Murphy <[email protected]>--- drivers/net/can/m_can/m_can.c | 6 + drivers/net/can/m_can/m_can_platform.c | 209 +++++++++++++++++++++++++ drivers/net/can/m_can/m_can_platform.h | 163 +++++++++++++++++++ 3 files changed, 378 insertions(+) create mode 100644 drivers/net/can/m_can/m_can_platform.c create mode 100644 drivers/net/can/m_can/m_can_platform.h... snip ...
Quoted 4 lines
This patch set is working for the TCAN and at least boots on io-mapped devices. We have a couple of customers who are looking at using this implementation and we would like to start moving this patch set along in the review.
Quoted 7 lines
Quoted 3 lines
I did not put a change log in here as there have been no comments for the last 3 patch sets I sent.
This v4 squashes a few bugs I found during testing across all the files.
OK, after a very busy week I will try to find time for the review... finally.
Thanks. I anticipate a v5 as we debug the io mapped code and continue to test the TCAN code.
Dan
Quoted 4 lines
Thanks,
Wolfgang.
Signature · 2 lines
------------------ Dan Murphy
Hello Dan,
looks already quite good...
Am 17.01.19 um 21:05 schrieb Dan Murphy:
Quoted 19 lines
Create a
m_canplatform framework that peripherial devices can register to and use common code and register sets. The peripherial devices may provide read/write and configuration support of the IP.Signed-off-by Dan Murphy <[email protected]>--- drivers/net/can/m_can/m_can.c | 6 + drivers/net/can/m_can/m_can_platform.c | 209 +++++++++++++++++++++++++ drivers/net/can/m_can/m_can_platform.h | 163 +++++++++++++++++++ 3 files changed, 378 insertions(+) create mode 100644 drivers/net/can/m_can/m_can_platform.c create mode 100644 drivers/net/can/m_can/m_can_platform.hdrivers/net/can/m_can/m_can.c +0
-0
drivers/net/can/m_can/m_can.c static inline void m_can_config_endisable(const struct m_can_priv *priv, · line 419
Quoted 6 lines
drivers/net/can/m_can/m_can.c +3
-0@@ -414,6 +414,9 @@ static inline void m_can_config_endisable(const struct m_can_priv *priv,u32 timeout =10 ;u32 val =0 ; +if (cccr &CCCR_CSR ) + cccr &= ~CCCR_CSR ; +
This is an unrelated change/fix. Should go somewhere else.
Quoted 50 lines
drivers/net/can/m_can/m_can.c +3
-0if (enable) {/* enable m_can configuration */ m_can_write (priv,M_CAN_CCCR , cccr |CCCR_INIT );@@ -1155,6 +1158,9 @@ static void m_can_chip_config(struct net_device *dev)m_can_set_bittiming (dev);m_can_config_endisable (priv, false); + +if (priv->device_init) + priv->device_init (priv); }static void m_can_start (struct net_device *dev)drivers/net/can/m_can/m_can_platform.c new file +31
-0@@ -0,0 +1,209 @@ +/* +* CAN bus driver for Bosch M_CAN controller +* +* Copyright (C) 2014 Freescale Semiconductor, Inc. +* Dong Aisheng <[email protected]> +* +* Bosch M_CAN user manual can be obtained from: +* http://www.bosch-semiconductors.de/media/pdf_1/ipmodules_1/m_can/ +* mcan_users_manual_v302.pdf +* +* This file is licensed under the terms of the GNU General Public +* License version 2. This program is licensed "as is" without any +* warranty of any kind, whether express or implied. +*/ + +#include <linux/clk.h> +#include <linux/interrupt.h> +#include <linux/io.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/netdevice.h> +#include <linux/platform_device.h> +#include <linux/pm_runtime.h> +#include <linux/can/dev.h> +#include <linux/pinctrl/consumer.h> + +#include "m_can_platform.h" + +struct m_can_plat_priv { +void __iomem *base; +void __iomem *mram_base;
drivers/net/can/m_can/m_can_platform.c line 41
Quoted 10 lines
drivers/net/can/m_can/m_can_platform.c new file +10
-0+}; + +static u32 iomap_read_reg (struct m_can_classdev *m_can_class,int reg) +{ +struct m_can_plat_priv *priv = (struct m_can_plat_priv *)m_can_class->device_data; + +return readl (priv->base + reg); +} + +static u32 iomap_read_fifo (struct m_can_classdev *m_can_class,int addr_offset)
Why not just "offset".
Quoted 65 lines
drivers/net/can/m_can/m_can_platform.c new file +65
-0+{ +struct m_can_plat_priv *priv = (struct m_can_plat_priv *)m_can_class->device_data; + +return readl (priv->mram_base + addr_offset); +} + +static int iomap_write_reg (struct m_can_classdev *m_can_class,int reg,int val) +{ +struct m_can_plat_priv *priv = (struct m_can_plat_priv *)m_can_class->device_data; + +writel (val, priv->base + reg); + +return 0 ; +} + +static int iomap_write_fifo (struct m_can_classdev *m_can_class,int addr_offset,int val) +{ +struct m_can_plat_priv *priv = (struct m_can_plat_priv *)m_can_class->device_data; + +writel (val, priv->base + addr_offset); + +return 0 ; +} + +static int m_can_plat_probe (struct platform_device *pdev) +{ +struct m_can_classdev *mcan_class; +struct m_can_plat_priv *priv; +struct resource *res; +void __iomem *addr; +void __iomem *mram_addr; +int irq, ret =0 ; + + mcan_class =m_can_core_allocate_dev (&pdev->dev); + priv =devm_kzalloc (&pdev->dev,sizeof (*priv),GFP_KERNEL ); +if (!priv) +return -ENOMEM ; + + mcan_class->device_data = priv; + +m_can_core_get_clocks (mcan_class); + + res =platform_get_resource_byname (pdev,IORESOURCE_MEM ,"m_can" ); + addr =devm_ioremap_resource (&pdev->dev, res); + irq =platform_get_irq_byname (pdev,"int0" ); +if (IS_ERR (addr) || irq <0 ) { + ret = -EINVAL ; + goto failed_ret; + } + +/* message ram could be shared */ + res =platform_get_resource_byname (pdev,IORESOURCE_MEM ,"message_ram" ); +if (!res) { + ret = -ENODEV ; + goto failed_ret; + } + + mram_addr =devm_ioremap (&pdev->dev, res->start,resource_size (res)); +if (!mram_addr) { + ret = -ENOMEM ; + goto failed_ret; + } + + priv->base = addr; + priv->mram_base = mram_addr;
drivers/net/can/m_can/m_can_platform.c line 116
Quoted 10 lines
drivers/net/can/m_can/m_can_platform.c new file +10
-0+ + mcan_class->net->irq = irq; + mcan_class->pm_clock_support =1 ; + mcan_class->can.clock.freq =clk_get_rate (mcan_class->cclk); + mcan_class->dev = &pdev->dev; + + mcan_class->read_reg = &iomap_read_reg; + mcan_class->write_reg = &iomap_write_reg; + mcan_class->write_fifo = &iomap_write_fifo; + mcan_class->read_fifo = &iomap_read_fifo;
No "&" please!
Quoted 81 lines
drivers/net/can/m_can/m_can_platform.c new file +81
-0+ mcan_class->is_peripherial = false; + +platform_set_drvdata (pdev, mcan_class->dev); + +m_can_init_ram (mcan_class); + + ret =m_can_core_register (mcan_class); + +failed_ret: +return ret; +} + +static __maybe_unused int m_can_suspend (struct device *dev) +{ +return m_can_core_suspend (dev); +} + +static __maybe_unused int m_can_resume (struct device *dev) +{ +return m_can_core_resume (dev); +} + +static int m_can_plat_remove (struct platform_device *pdev) +{ +struct net_device *dev =platform_get_drvdata (pdev); +struct m_can_classdev *mcan_class =netdev_priv (dev); + +m_can_core_unregister (mcan_class); + +platform_set_drvdata (pdev,NULL ); + +return 0 ; +} + +static int __maybe_unused m_can_runtime_suspend (struct device *dev) +{ +struct net_device *ndev =dev_get_drvdata (dev); +struct m_can_classdev *mcan_class =netdev_priv (ndev); + +m_can_core_suspend (dev); + +clk_disable_unprepare (mcan_class->cclk); +clk_disable_unprepare (mcan_class->hclk); + +return 0 ; +} + +static int __maybe_unused m_can_runtime_resume (struct device *dev) +{ +struct net_device *ndev =dev_get_drvdata (dev); +struct m_can_classdev *mcan_class =netdev_priv (ndev); +int err; + + err =clk_prepare_enable (mcan_class->hclk); +if (err) +return err; + + err =clk_prepare_enable (mcan_class->cclk); +if (err) +clk_disable_unprepare (mcan_class->hclk); + +m_can_core_resume (dev); + +return err; +} + +static const struct dev_pm_ops m_can_pmops = { + SET_RUNTIME_PM_OPS(m_can_runtime_suspend, + m_can_runtime_resume, NULL) +SET_SYSTEM_SLEEP_PM_OPS (m_can_suspend, m_can_resume) +}; + +static const struct of_device_id m_can_of_table[] = { + { .compatible ="bosch,m_can" , .data =NULL }, + {/* sentinel */ }, +}; +MODULE_DEVICE_TABLE (of, m_can_of_table); + +static struct platform_driver m_can_plat_driver = { + .driver = { + .name =KBUILD_MODNAME ,
drivers/net/can/m_can/m_can_platform.c line 207
Quoted 10 lines
drivers/net/can/m_can/m_can_platform.c new file +10
-0+ .of_match_table = m_can_of_table, + .pm = &m_can_pmops, + }, + .probe = m_can_plat_probe, + .remove = m_can_plat_remove, +}; + +module_platform_driver (m_can_plat_driver); + +MODULE_AUTHOR ("Dong Aisheng <[email protected]>" );
Feel free to add yourself as second author.
drivers/net/can/m_can/m_can_platform.h
Quoted 7 lines
drivers/net/can/m_can/m_can_platform.c new file +2
-0+MODULE_LICENSE ("GPL v2" ); +MODULE_DESCRIPTION ("CAN bus driver for Bosch M_CAN controller" );drivers/net/can/m_can/m_can_platform.h new file +0
-0
These are common definitions, right? Therefore the filen name should be "m_can.h"!?
Quoted 18 lines
drivers/net/can/m_can/m_can_platform.h new file +17
-0@@ -0,0 +1,163 @@ +// SPDX-License-Identifier: GPL-2.0 +// Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/ + +#ifndef _CAN_M_CAN_CORE_H_ +#define _CAN_M_CAN_CORE_H_ + +#include <linux/can/core.h> +#include <linux/can/led.h> +#include <linux/completion.h> +#include <linux/device.h> +#include <linux/dma-mapping.h> +#include <linux/freezer.h> +#include <linux/slab.h> +#include <linux/uaccess.h> +#include <linux/clk.h> +#include <linux/delay.h> +#include <linux/interrupt.h>
drivers/net/can/m_can/m_can_platform.h line 27
Quoted 10 lines
drivers/net/can/m_can/m_can_platform.h new file +10
-0+#include <linux/io.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/netdevice.h> +#include <linux/of.h> +#include <linux/of_device.h> +#include <linux/pm_runtime.h> +#include <linux/iopoll.h> +#include <linux/can/dev.h> +#include <linux/pinctrl/consumer.h>
Do you really need them all in this header file?
Quoted 80 lines
drivers/net/can/m_can/m_can_platform.h new file +80
-0+ +/* m_can lec values */ +enum m_can_lec_type { +LEC_NO_ERROR =0 , +LEC_STUFF_ERROR , +LEC_FORM_ERROR , +LEC_ACK_ERROR , +LEC_BIT1_ERROR , +LEC_BIT0_ERROR , +LEC_CRC_ERROR , +LEC_UNUSED , +}; + +enum m_can_mram_cfg { +MRAM_SIDF =0 , +MRAM_XIDF , +MRAM_RXF0 , +MRAM_RXF1 , +MRAM_RXB , +MRAM_TXE , +MRAM_TXB , +MRAM_CFG_NUM , +}; + +/* registers definition */ +enum m_can_reg { +M_CAN_CREL =0x0 , +M_CAN_ENDN =0x4 , +M_CAN_CUST =0x8 , +M_CAN_DBTP =0xc , +M_CAN_TEST =0x10 , +M_CAN_RWD =0x14 , +M_CAN_CCCR =0x18 , +M_CAN_NBTP =0x1c , +M_CAN_TSCC =0x20 , +M_CAN_TSCV =0x24 , +M_CAN_TOCC =0x28 , +M_CAN_TOCV =0x2c , +M_CAN_ECR =0x40 , +M_CAN_PSR =0x44 , +/* TDCR Register only available for version >=3.1.x */ +M_CAN_TDCR =0x48 , +M_CAN_IR =0x50 , +M_CAN_IE =0x54 , +M_CAN_ILS =0x58 , +M_CAN_ILE =0x5c , +M_CAN_GFC =0x80 , +M_CAN_SIDFC =0x84 , +M_CAN_XIDFC =0x88 , +M_CAN_XIDAM =0x90 , +M_CAN_HPMS =0x94 , +M_CAN_NDAT1 =0x98 , +M_CAN_NDAT2 =0x9c , +M_CAN_RXF0C =0xa0 , +M_CAN_RXF0S =0xa4 , +M_CAN_RXF0A =0xa8 , +M_CAN_RXBC =0xac , +M_CAN_RXF1C =0xb0 , +M_CAN_RXF1S =0xb4 , +M_CAN_RXF1A =0xb8 , +M_CAN_RXESC =0xbc , +M_CAN_TXBC =0xc0 , +M_CAN_TXFQS =0xc4 , +M_CAN_TXESC =0xc8 , +M_CAN_TXBRP =0xcc , +M_CAN_TXBAR =0xd0 , +M_CAN_TXBCR =0xd4 , +M_CAN_TXBTO =0xd8 , +M_CAN_TXBCF =0xdc , +M_CAN_TXBTIE =0xe0 , +M_CAN_TXBCIE =0xe4 , +M_CAN_TXEFC =0xf0 , +M_CAN_TXEFS =0xf4 , +M_CAN_TXEFA =0xf8 , +}; + +/* address offset and element number for each FIFO/Buffer in the Message RAM */ +struct mram_cfg { +u16 off; +u8 num;
drivers/net/can/m_can/m_can_platform.h line 117
Quoted 10 lines
drivers/net/can/m_can/m_can_platform.h new file +10
-0+}; + +struct m_can_classdev ; + +typedef int (*can_dev_init ) (struct m_can_classdev *m_can_class); +typedef int (*can_clr_dev_interrupts ) (struct m_can_classdev *m_can_class); +typedef u32 (*can_reg_read ) (struct m_can_classdev *m_can_class,int reg); +typedef int (*can_reg_write ) (struct m_can_classdev *m_can_class,int reg,int val); +typedef u32 (*can_fifo_read ) (struct m_can_classdev *m_can_class,int addr_offset); +typedef int (*can_fifo_write ) (struct m_can_classdev *m_can_class,int addr_offset,int val);
No typedefs in the kernel, please!
Quoted 29 lines
drivers/net/can/m_can/m_can_platform.h new file +29
-0+struct m_can_classdev { +struct can_priv can; +struct napi_struct napi; +struct net_device *net; +struct device *dev; +struct clk *hclk; +struct clk *cclk; + +struct workqueue_struct *wq; +struct work_struct tx_work; +struct sk_buff *skb; + +struct can_bittiming_const *bit_timing; +struct can_bittiming_const *data_timing; + +void *device_data; + +/* Device specific call backs */ +can_dev_init device_init; +can_clr_dev_interrupts clr_dev_interrupts; +can_reg_read read_reg; +can_reg_write write_reg; +can_fifo_read read_fifo; +can_fifo_write write_fifo; + +int version; +int freq; +u32 irqstatus; +
drivers/net/can/m_can/m_can_platform.h line 157
Quoted 10 lines
drivers/net/can/m_can/m_can_platform.h new file +10
-0+int pm_clock_support; +bool is_peripherial; + +struct mram_cfg mcfg[MRAM_CFG_NUM ]; +}; + +struct m_can_classdev *m_can_core_allocate_dev (struct device *dev); +int m_can_core_register (struct m_can_classdev *m_can_dev); +void m_can_core_unregister (struct m_can_classdev *m_can_dev); +int m_can_core_get_clocks (struct m_can_classdev *m_can_dev);
You use here three different prefixes: "m_can_core", "m_can_classdev" and "m_can_dev". There should be just one principle name for the struct, func, args and vars, e.g.:
int m_can_device_register(struct m_can_device *mcan_dev);
drivers/net/can/m_can/m_can_platform.h line 163
Quoted 7 lines
drivers/net/can/m_can/m_can_platform.h new file +6
-0+void m_can_init_ram (struct m_can_classdev *priv); +void m_can_config_endisable (const struct m_can_classdev *priv,bool enable); + +int m_can_core_suspend (struct device *dev); +int m_can_core_resume (struct device *dev); +#endif /* _CAN_M_CAN_CORE_H_ */
Wolfgang
Wolfgang
Thanks for the review
Quoted 27 lines · Wolfgang Grandegger
Hello Dan,
looks already quite good...
Am 17.01.19 um 21:05 schrieb Dan Murphy:
Quoted 22 lines
Create a
m_canplatform framework that peripherial devices can register to and use common code and register sets. The peripherial devices may provide read/write and configuration support of the IP.Signed-off-by Dan Murphy <[email protected]>--- drivers/net/can/m_can/m_can.c | 6 + drivers/net/can/m_can/m_can_platform.c | 209 +++++++++++++++++++++++++ drivers/net/can/m_can/m_can_platform.h | 163 +++++++++++++++++++ 3 files changed, 378 insertions(+) create mode 100644 drivers/net/can/m_can/m_can_platform.c create mode 100644 drivers/net/can/m_can/m_can_platform.hdrivers/net/can/m_can/m_can.c +0
-0@@ -414,6 +414,9 @@ static inline void m_can_config_endisable(const struct m_can_priv *priv,u32 timeout =10 ;u32 val =0 ;
Quoted 6 lines
drivers/net/can/m_can/m_can.c static inline void m_can_config_endisable(const struct m_can_priv *priv, · line 419
Quoted 3 lines
drivers/net/can/m_can/m_can.c +3
-0+if (cccr &CCCR_CSR ) + cccr &= ~CCCR_CSR ; +This is an unrelated change/fix. Should go somewhere else.
I thought I pulled this and the change below out of of this patchset.
Quoted 54 lines
Quoted 54 lines
drivers/net/can/m_can/m_can.c +3
-0if (enable) {/* enable m_can configuration */ m_can_write (priv,M_CAN_CCCR , cccr |CCCR_INIT );@@ -1155,6 +1158,9 @@ static void m_can_chip_config(struct net_device *dev)m_can_set_bittiming (dev);m_can_config_endisable (priv, false); + +if (priv->device_init) + priv->device_init (priv); }static void m_can_start (struct net_device *dev)drivers/net/can/m_can/m_can_platform.c new file +35
-0@@ -0,0 +1,209 @@ +/* +* CAN bus driver for Bosch M_CAN controller +* +* Copyright (C) 2014 Freescale Semiconductor, Inc. +* Dong Aisheng <[email protected]> +* +* Bosch M_CAN user manual can be obtained from: +* http://www.bosch-semiconductors.de/media/pdf_1/ipmodules_1/m_can/ +* mcan_users_manual_v302.pdf +* +* This file is licensed under the terms of the GNU General Public +* License version 2. This program is licensed "as is" without any +* warranty of any kind, whether express or implied. +*/ + +#include <linux/clk.h> +#include <linux/interrupt.h> +#include <linux/io.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/netdevice.h> +#include <linux/platform_device.h> +#include <linux/pm_runtime.h> +#include <linux/can/dev.h> +#include <linux/pinctrl/consumer.h> + +#include "m_can_platform.h" + +struct m_can_plat_priv { +void __iomem *base; +void __iomem *mram_base; +}; + +static u32 iomap_read_reg (struct m_can_classdev *m_can_class,int reg) +{
Quoted 9 lines
drivers/net/can/m_can/m_can_platform.c line 41
Quoted 6 lines
drivers/net/can/m_can/m_can_platform.c new file +6
-0+struct m_can_plat_priv *priv = (struct m_can_plat_priv *)m_can_class->device_data; + +return readl (priv->base + reg); +} + +static u32 iomap_read_fifo (struct m_can_classdev *m_can_class,int addr_offset)Why not just "offset".
I can change the name
Quoted 69 lines
Quoted 69 lines
drivers/net/can/m_can/m_can_platform.c new file +69
-0+{ +struct m_can_plat_priv *priv = (struct m_can_plat_priv *)m_can_class->device_data; + +return readl (priv->mram_base + addr_offset); +} + +static int iomap_write_reg (struct m_can_classdev *m_can_class,int reg,int val) +{ +struct m_can_plat_priv *priv = (struct m_can_plat_priv *)m_can_class->device_data; + +writel (val, priv->base + reg); + +return 0 ; +} + +static int iomap_write_fifo (struct m_can_classdev *m_can_class,int addr_offset,int val) +{ +struct m_can_plat_priv *priv = (struct m_can_plat_priv *)m_can_class->device_data; + +writel (val, priv->base + addr_offset); + +return 0 ; +} + +static int m_can_plat_probe (struct platform_device *pdev) +{ +struct m_can_classdev *mcan_class; +struct m_can_plat_priv *priv; +struct resource *res; +void __iomem *addr; +void __iomem *mram_addr; +int irq, ret =0 ; + + mcan_class =m_can_core_allocate_dev (&pdev->dev); + priv =devm_kzalloc (&pdev->dev,sizeof (*priv),GFP_KERNEL ); +if (!priv) +return -ENOMEM ; + + mcan_class->device_data = priv; + +m_can_core_get_clocks (mcan_class); + + res =platform_get_resource_byname (pdev,IORESOURCE_MEM ,"m_can" ); + addr =devm_ioremap_resource (&pdev->dev, res); + irq =platform_get_irq_byname (pdev,"int0" ); +if (IS_ERR (addr) || irq <0 ) { + ret = -EINVAL ; + goto failed_ret; + } + +/* message ram could be shared */ + res =platform_get_resource_byname (pdev,IORESOURCE_MEM ,"message_ram" ); +if (!res) { + ret = -ENODEV ; + goto failed_ret; + } + + mram_addr =devm_ioremap (&pdev->dev, res->start,resource_size (res)); +if (!mram_addr) { + ret = -ENOMEM ; + goto failed_ret; + } + + priv->base = addr; + priv->mram_base = mram_addr; + + mcan_class->net->irq = irq; + mcan_class->pm_clock_support =1 ; + mcan_class->can.clock.freq =clk_get_rate (mcan_class->cclk);
Quoted 9 lines
drivers/net/can/m_can/m_can_platform.c line 116
Quoted 6 lines
drivers/net/can/m_can/m_can_platform.c new file +6
-0+ mcan_class->dev = &pdev->dev; + + mcan_class->read_reg = &iomap_read_reg; + mcan_class->write_reg = &iomap_write_reg; + mcan_class->write_fifo = &iomap_write_fifo; + mcan_class->read_fifo = &iomap_read_fifo;No "&" please!
OK. But can I ask why?
Quoted 94 lines
Quoted 81 lines
drivers/net/can/m_can/m_can_platform.c new file +81
-0+ mcan_class->is_peripherial = false; + +platform_set_drvdata (pdev, mcan_class->dev); + +m_can_init_ram (mcan_class); + + ret =m_can_core_register (mcan_class); + +failed_ret: +return ret; +} + +static __maybe_unused int m_can_suspend (struct device *dev) +{ +return m_can_core_suspend (dev); +} + +static __maybe_unused int m_can_resume (struct device *dev) +{ +return m_can_core_resume (dev); +} + +static int m_can_plat_remove (struct platform_device *pdev) +{ +struct net_device *dev =platform_get_drvdata (pdev); +struct m_can_classdev *mcan_class =netdev_priv (dev); + +m_can_core_unregister (mcan_class); + +platform_set_drvdata (pdev,NULL ); + +return 0 ; +} + +static int __maybe_unused m_can_runtime_suspend (struct device *dev) +{ +struct net_device *ndev =dev_get_drvdata (dev); +struct m_can_classdev *mcan_class =netdev_priv (ndev); + +m_can_core_suspend (dev); + +clk_disable_unprepare (mcan_class->cclk); +clk_disable_unprepare (mcan_class->hclk); + +return 0 ; +} + +static int __maybe_unused m_can_runtime_resume (struct device *dev) +{ +struct net_device *ndev =dev_get_drvdata (dev); +struct m_can_classdev *mcan_class =netdev_priv (ndev); +int err; + + err =clk_prepare_enable (mcan_class->hclk); +if (err) +return err; + + err =clk_prepare_enable (mcan_class->cclk); +if (err) +clk_disable_unprepare (mcan_class->hclk); + +m_can_core_resume (dev); + +return err; +} + +static const struct dev_pm_ops m_can_pmops = { + SET_RUNTIME_PM_OPS(m_can_runtime_suspend, + m_can_runtime_resume, NULL) +SET_SYSTEM_SLEEP_PM_OPS (m_can_suspend, m_can_resume) +}; + +static const struct of_device_id m_can_of_table[] = { + { .compatible ="bosch,m_can" , .data =NULL }, + {/* sentinel */ }, +}; +MODULE_DEVICE_TABLE (of, m_can_of_table); + +static struct platform_driver m_can_plat_driver = { + .driver = { + .name =KBUILD_MODNAME ,drivers/net/can/m_can/m_can_platform.c line 207
Quoted 10 lines
drivers/net/can/m_can/m_can_platform.c new file +10
-0+ .of_match_table = m_can_of_table, + .pm = &m_can_pmops, + }, + .probe = m_can_plat_probe, + .remove = m_can_plat_remove, +}; + +module_platform_driver (m_can_plat_driver); + +MODULE_AUTHOR ("Dong Aisheng <[email protected]>" );Feel free to add yourself as second author.
Quoted 11 lines
drivers/net/can/m_can/m_can_platform.h
Quoted 7 lines
drivers/net/can/m_can/m_can_platform.c new file +2
-0+MODULE_LICENSE ("GPL v2" ); +MODULE_DESCRIPTION ("CAN bus driver for Bosch M_CAN controller" );drivers/net/can/m_can/m_can_platform.h new file +0
-0These are common definitions, right? Therefore the filen name should be "
m_can.h"!?
Ah yes. My mistake common m_can definitions should be just m_can.
Quoted 22 lines
Quoted 22 lines
drivers/net/can/m_can/m_can_platform.h new file +21
-0@@ -0,0 +1,163 @@ +// SPDX-License-Identifier: GPL-2.0 +// Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/ + +#ifndef _CAN_M_CAN_CORE_H_ +#define _CAN_M_CAN_CORE_H_ + +#include <linux/can/core.h> +#include <linux/can/led.h> +#include <linux/completion.h> +#include <linux/device.h> +#include <linux/dma-mapping.h> +#include <linux/freezer.h> +#include <linux/slab.h> +#include <linux/uaccess.h> +#include <linux/clk.h> +#include <linux/delay.h> +#include <linux/interrupt.h> +#include <linux/io.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/netdevice.h>
Quoted 9 lines
drivers/net/can/m_can/m_can_platform.h line 27
Quoted 6 lines
drivers/net/can/m_can/m_can_platform.h new file +6
-0+#include <linux/of.h> +#include <linux/of_device.h> +#include <linux/pm_runtime.h> +#include <linux/iopoll.h> +#include <linux/can/dev.h> +#include <linux/pinctrl/consumer.h> Do you really need them all in this header file?
Probably not just a copy paste from the original m_can. I will go through them and keep the common headers.
Quoted 84 lines
Quoted 84 lines
drivers/net/can/m_can/m_can_platform.h new file +84
-0+ +/* m_can lec values */ +enum m_can_lec_type { +LEC_NO_ERROR =0 , +LEC_STUFF_ERROR , +LEC_FORM_ERROR , +LEC_ACK_ERROR , +LEC_BIT1_ERROR , +LEC_BIT0_ERROR , +LEC_CRC_ERROR , +LEC_UNUSED , +}; + +enum m_can_mram_cfg { +MRAM_SIDF =0 , +MRAM_XIDF , +MRAM_RXF0 , +MRAM_RXF1 , +MRAM_RXB , +MRAM_TXE , +MRAM_TXB , +MRAM_CFG_NUM , +}; + +/* registers definition */ +enum m_can_reg { +M_CAN_CREL =0x0 , +M_CAN_ENDN =0x4 , +M_CAN_CUST =0x8 , +M_CAN_DBTP =0xc , +M_CAN_TEST =0x10 , +M_CAN_RWD =0x14 , +M_CAN_CCCR =0x18 , +M_CAN_NBTP =0x1c , +M_CAN_TSCC =0x20 , +M_CAN_TSCV =0x24 , +M_CAN_TOCC =0x28 , +M_CAN_TOCV =0x2c , +M_CAN_ECR =0x40 , +M_CAN_PSR =0x44 , +/* TDCR Register only available for version >=3.1.x */ +M_CAN_TDCR =0x48 , +M_CAN_IR =0x50 , +M_CAN_IE =0x54 , +M_CAN_ILS =0x58 , +M_CAN_ILE =0x5c , +M_CAN_GFC =0x80 , +M_CAN_SIDFC =0x84 , +M_CAN_XIDFC =0x88 , +M_CAN_XIDAM =0x90 , +M_CAN_HPMS =0x94 , +M_CAN_NDAT1 =0x98 , +M_CAN_NDAT2 =0x9c , +M_CAN_RXF0C =0xa0 , +M_CAN_RXF0S =0xa4 , +M_CAN_RXF0A =0xa8 , +M_CAN_RXBC =0xac , +M_CAN_RXF1C =0xb0 , +M_CAN_RXF1S =0xb4 , +M_CAN_RXF1A =0xb8 , +M_CAN_RXESC =0xbc , +M_CAN_TXBC =0xc0 , +M_CAN_TXFQS =0xc4 , +M_CAN_TXESC =0xc8 , +M_CAN_TXBRP =0xcc , +M_CAN_TXBAR =0xd0 , +M_CAN_TXBCR =0xd4 , +M_CAN_TXBTO =0xd8 , +M_CAN_TXBCF =0xdc , +M_CAN_TXBTIE =0xe0 , +M_CAN_TXBCIE =0xe4 , +M_CAN_TXEFC =0xf0 , +M_CAN_TXEFS =0xf4 , +M_CAN_TXEFA =0xf8 , +}; + +/* address offset and element number for each FIFO/Buffer in the Message RAM */ +struct mram_cfg { +u16 off; +u8 num; +}; + +struct m_can_classdev ; +
Quoted 9 lines
drivers/net/can/m_can/m_can_platform.h line 117
Quoted 6 lines
drivers/net/can/m_can/m_can_platform.h new file +6
-0+typedef int (*can_dev_init ) (struct m_can_classdev *m_can_class); +typedef int (*can_clr_dev_interrupts ) (struct m_can_classdev *m_can_class); +typedef u32 (*can_reg_read ) (struct m_can_classdev *m_can_class,int reg); +typedef int (*can_reg_write ) (struct m_can_classdev *m_can_class,int reg,int val); +typedef u32 (*can_fifo_read ) (struct m_can_classdev *m_can_class,int addr_offset); +typedef int (*can_fifo_write ) (struct m_can_classdev *m_can_class,int addr_offset,int val);No typedefs in the kernel, please!
OK. Just following some other conventions. I can remove
Quoted 44 lines
Quoted 29 lines
drivers/net/can/m_can/m_can_platform.h new file +29
-0+struct m_can_classdev { +struct can_priv can; +struct napi_struct napi; +struct net_device *net; +struct device *dev; +struct clk *hclk; +struct clk *cclk; + +struct workqueue_struct *wq; +struct work_struct tx_work; +struct sk_buff *skb; + +struct can_bittiming_const *bit_timing; +struct can_bittiming_const *data_timing; + +void *device_data; + +/* Device specific call backs */ +can_dev_init device_init; +can_clr_dev_interrupts clr_dev_interrupts; +can_reg_read read_reg; +can_reg_write write_reg; +can_fifo_read read_fifo; +can_fifo_write write_fifo; + +int version; +int freq; +u32 irqstatus; +drivers/net/can/m_can/m_can_platform.h line 157
Quoted 10 lines
drivers/net/can/m_can/m_can_platform.h new file +10
-0+int pm_clock_support; +bool is_peripherial; + +struct mram_cfg mcfg[MRAM_CFG_NUM ]; +}; + +struct m_can_classdev *m_can_core_allocate_dev (struct device *dev); +int m_can_core_register (struct m_can_classdev *m_can_dev); +void m_can_core_unregister (struct m_can_classdev *m_can_dev); +int m_can_core_get_clocks (struct m_can_classdev *m_can_dev);You use here three different prefixes: "
m_can_core", "m_can_classdev" and "m_can_dev". There should be just one principle name for the struct, func, args and vars, e.g.:
Quoted 2 lines
int m_can_device_register(struct m_can_device *mcan_dev);
OK I will commonize the naming. I may go with the above or call it m_can_class.
Dan
Quoted 10 lines
drivers/net/can/m_can/m_can_platform.h line 163
Quoted 7 lines
drivers/net/can/m_can/m_can_platform.h new file +6
-0+void m_can_init_ram (struct m_can_classdev *priv); +void m_can_config_endisable (const struct m_can_classdev *priv,bool enable); + +int m_can_core_suspend (struct device *dev); +int m_can_core_resume (struct device *dev); +#endif /* _CAN_M_CAN_CORE_H_ */ Wolfgang
Signature · 2 lines
------------------ Dan Murphy
Hello Dan,
Am 22.01.19 um 14:04 schrieb Dan Murphy:
Quoted 186 lines
Wolfgang
Thanks for the review
Quoted 27 lines · Wolfgang Grandegger
Hello Dan,
looks already quite good...
Am 17.01.19 um 21:05 schrieb Dan Murphy:
Quoted 22 lines
Create a
m_canplatform framework that peripherial devices can register to and use common code and register sets. The peripherial devices may provide read/write and configuration support of the IP.Signed-off-by Dan Murphy <[email protected]>--- drivers/net/can/m_can/m_can.c | 6 + drivers/net/can/m_can/m_can_platform.c | 209 +++++++++++++++++++++++++ drivers/net/can/m_can/m_can_platform.h | 163 +++++++++++++++++++ 3 files changed, 378 insertions(+) create mode 100644 drivers/net/can/m_can/m_can_platform.c create mode 100644 drivers/net/can/m_can/m_can_platform.hdrivers/net/can/m_can/m_can.c +0
-0@@ -414,6 +414,9 @@ static inline void m_can_config_endisable(const struct m_can_priv *priv,u32 timeout =10 ;u32 val =0 ;Quoted 6 lines
drivers/net/can/m_can/m_can.c static inline void m_can_config_endisable(const struct m_can_priv *priv, · line 419
Quoted 3 lines
drivers/net/can/m_can/m_can.c +3
-0+if (cccr &CCCR_CSR ) + cccr &= ~CCCR_CSR ; +This is an unrelated change/fix. Should go somewhere else.
I thought I pulled this and the change below out of of this patchset.
Quoted 54 lines
Quoted 54 lines
drivers/net/can/m_can/m_can.c +3
-0if (enable) {/* enable m_can configuration */ m_can_write (priv,M_CAN_CCCR , cccr |CCCR_INIT );@@ -1155,6 +1158,9 @@ static void m_can_chip_config(struct net_device *dev)m_can_set_bittiming (dev);m_can_config_endisable (priv, false); + +if (priv->device_init) + priv->device_init (priv); }static void m_can_start (struct net_device *dev)drivers/net/can/m_can/m_can_platform.c new file +35
-0@@ -0,0 +1,209 @@ +/* +* CAN bus driver for Bosch M_CAN controller +* +* Copyright (C) 2014 Freescale Semiconductor, Inc. +* Dong Aisheng <[email protected]> +* +* Bosch M_CAN user manual can be obtained from: +* http://www.bosch-semiconductors.de/media/pdf_1/ipmodules_1/m_can/ +* mcan_users_manual_v302.pdf +* +* This file is licensed under the terms of the GNU General Public +* License version 2. This program is licensed "as is" without any +* warranty of any kind, whether express or implied. +*/ + +#include <linux/clk.h> +#include <linux/interrupt.h> +#include <linux/io.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/netdevice.h> +#include <linux/platform_device.h> +#include <linux/pm_runtime.h> +#include <linux/can/dev.h> +#include <linux/pinctrl/consumer.h> + +#include "m_can_platform.h" + +struct m_can_plat_priv { +void __iomem *base; +void __iomem *mram_base; +}; + +static u32 iomap_read_reg (struct m_can_classdev *m_can_class,int reg) +{Quoted 9 lines
drivers/net/can/m_can/m_can_platform.c line 41
Quoted 6 lines
drivers/net/can/m_can/m_can_platform.c new file +6
-0+struct m_can_plat_priv *priv = (struct m_can_plat_priv *)m_can_class->device_data; + +return readl (priv->base + reg); +} + +static u32 iomap_read_fifo (struct m_can_classdev *m_can_class,int addr_offset)Why not just "offset".
I can change the name
Quoted 69 lines
Quoted 69 lines
drivers/net/can/m_can/m_can_platform.c new file +69
-0+{ +struct m_can_plat_priv *priv = (struct m_can_plat_priv *)m_can_class->device_data; + +return readl (priv->mram_base + addr_offset); +} + +static int iomap_write_reg (struct m_can_classdev *m_can_class,int reg,int val) +{ +struct m_can_plat_priv *priv = (struct m_can_plat_priv *)m_can_class->device_data; + +writel (val, priv->base + reg); + +return 0 ; +} + +static int iomap_write_fifo (struct m_can_classdev *m_can_class,int addr_offset,int val) +{ +struct m_can_plat_priv *priv = (struct m_can_plat_priv *)m_can_class->device_data; + +writel (val, priv->base + addr_offset); + +return 0 ; +} + +static int m_can_plat_probe (struct platform_device *pdev) +{ +struct m_can_classdev *mcan_class; +struct m_can_plat_priv *priv; +struct resource *res; +void __iomem *addr; +void __iomem *mram_addr; +int irq, ret =0 ; + + mcan_class =m_can_core_allocate_dev (&pdev->dev); + priv =devm_kzalloc (&pdev->dev,sizeof (*priv),GFP_KERNEL ); +if (!priv) +return -ENOMEM ; + + mcan_class->device_data = priv; + +m_can_core_get_clocks (mcan_class); + + res =platform_get_resource_byname (pdev,IORESOURCE_MEM ,"m_can" ); + addr =devm_ioremap_resource (&pdev->dev, res); + irq =platform_get_irq_byname (pdev,"int0" ); +if (IS_ERR (addr) || irq <0 ) { + ret = -EINVAL ; + goto failed_ret; + } + +/* message ram could be shared */ + res =platform_get_resource_byname (pdev,IORESOURCE_MEM ,"message_ram" ); +if (!res) { + ret = -ENODEV ; + goto failed_ret; + } + + mram_addr =devm_ioremap (&pdev->dev, res->start,resource_size (res)); +if (!mram_addr) { + ret = -ENOMEM ; + goto failed_ret; + } + + priv->base = addr; + priv->mram_base = mram_addr; + + mcan_class->net->irq = irq; + mcan_class->pm_clock_support =1 ; + mcan_class->can.clock.freq =clk_get_rate (mcan_class->cclk);Quoted 9 lines
drivers/net/can/m_can/m_can_platform.c line 116
Quoted 6 lines
drivers/net/can/m_can/m_can_platform.c new file +6
-0+ mcan_class->dev = &pdev->dev; + + mcan_class->read_reg = &iomap_read_reg; + mcan_class->write_reg = &iomap_write_reg; + mcan_class->write_fifo = &iomap_write_fifo; + mcan_class->read_fifo = &iomap_read_fifo;No "&" please!
Quoted 1 line
OK. But can I ask why?
Well, to be honest, I cannot remember :(. I know that the convention in the Linux kernel is without "&". Maybe somebody else can explain it!?
Quoted 238 lines
Quoted 95 lines
Quoted 82 lines
drivers/net/can/m_can/m_can_platform.c new file +81
-0+ mcan_class->is_peripherial = false; + +platform_set_drvdata (pdev, mcan_class->dev); + +m_can_init_ram (mcan_class); + + ret =m_can_core_register (mcan_class); + +failed_ret: +return ret; +} + +static __maybe_unused int m_can_suspend (struct device *dev) +{ +return m_can_core_suspend (dev); +} + +static __maybe_unused int m_can_resume (struct device *dev) +{ +return m_can_core_resume (dev); +} + +static int m_can_plat_remove (struct platform_device *pdev) +{ +struct net_device *dev =platform_get_drvdata (pdev); +struct m_can_classdev *mcan_class =netdev_priv (dev); + +m_can_core_unregister (mcan_class); + +platform_set_drvdata (pdev,NULL ); + +return 0 ; +} + +static int __maybe_unused m_can_runtime_suspend (struct device *dev) +{ +struct net_device *ndev =dev_get_drvdata (dev); +struct m_can_classdev *mcan_class =netdev_priv (ndev); + +m_can_core_suspend (dev); + +clk_disable_unprepare (mcan_class->cclk); +clk_disable_unprepare (mcan_class->hclk); + +return 0 ; +} + +static int __maybe_unused m_can_runtime_resume (struct device *dev) +{ +struct net_device *ndev =dev_get_drvdata (dev); +struct m_can_classdev *mcan_class =netdev_priv (ndev); +int err; + + err =clk_prepare_enable (mcan_class->hclk); +if (err) +return err; + + err =clk_prepare_enable (mcan_class->cclk); +if (err) +clk_disable_unprepare (mcan_class->hclk); + +m_can_core_resume (dev); + +return err; +} + +static const struct dev_pm_ops m_can_pmops = { + SET_RUNTIME_PM_OPS(m_can_runtime_suspend, + m_can_runtime_resume, NULL) +SET_SYSTEM_SLEEP_PM_OPS (m_can_suspend, m_can_resume) +}; + +static const struct of_device_id m_can_of_table[] = { + { .compatible ="bosch,m_can" , .data =NULL }, + {/* sentinel */ }, +}; +MODULE_DEVICE_TABLE (of, m_can_of_table); + +static struct platform_driver m_can_plat_driver = { + .driver = { + .name =KBUILD_MODNAME ,drivers/net/can/m_can/m_can_platform.c line 207
Quoted 10 lines
drivers/net/can/m_can/m_can_platform.c new file +10
-0+ .of_match_table = m_can_of_table, + .pm = &m_can_pmops, + }, + .probe = m_can_plat_probe, + .remove = m_can_plat_remove, +}; + +module_platform_driver (m_can_plat_driver); + +MODULE_AUTHOR ("Dong Aisheng <[email protected]>" );Feel free to add yourself as second author.
Quoted 11 lines
drivers/net/can/m_can/m_can_platform.h
Quoted 7 lines
drivers/net/can/m_can/m_can_platform.c new file +2
-0+MODULE_LICENSE ("GPL v2" ); +MODULE_DESCRIPTION ("CAN bus driver for Bosch M_CAN controller" );drivers/net/can/m_can/m_can_platform.h new file +0
-0These are common definitions, right? Therefore the filen name should be "
m_can.h"!?Ah yes. My mistake common
m_candefinitions should be justm_can.Quoted 22 lines
Quoted 22 lines
drivers/net/can/m_can/m_can_platform.h new file +21
-0@@ -0,0 +1,163 @@ +// SPDX-License-Identifier: GPL-2.0 +// Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/ + +#ifndef _CAN_M_CAN_CORE_H_ +#define _CAN_M_CAN_CORE_H_ + +#include <linux/can/core.h> +#include <linux/can/led.h> +#include <linux/completion.h> +#include <linux/device.h> +#include <linux/dma-mapping.h> +#include <linux/freezer.h> +#include <linux/slab.h> +#include <linux/uaccess.h> +#include <linux/clk.h> +#include <linux/delay.h> +#include <linux/interrupt.h> +#include <linux/io.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/netdevice.h> Quoted 9 lines
drivers/net/can/m_can/m_can_platform.h line 27
Quoted 6 lines
drivers/net/can/m_can/m_can_platform.h new file +6
-0+#include <linux/of.h> +#include <linux/of_device.h> +#include <linux/pm_runtime.h> +#include <linux/iopoll.h> +#include <linux/can/dev.h> +#include <linux/pinctrl/consumer.h> Do you really need them all in this header file?
Probably not just a copy paste from the original
m_can. I will go through them and keep the common headers.Quoted 84 lines
Quoted 84 lines
drivers/net/can/m_can/m_can_platform.h new file +84
-0+ +/* m_can lec values */ +enum m_can_lec_type { +LEC_NO_ERROR =0 , +LEC_STUFF_ERROR , +LEC_FORM_ERROR , +LEC_ACK_ERROR , +LEC_BIT1_ERROR , +LEC_BIT0_ERROR , +LEC_CRC_ERROR , +LEC_UNUSED , +}; + +enum m_can_mram_cfg { +MRAM_SIDF =0 , +MRAM_XIDF , +MRAM_RXF0 , +MRAM_RXF1 , +MRAM_RXB , +MRAM_TXE , +MRAM_TXB , +MRAM_CFG_NUM , +}; + +/* registers definition */ +enum m_can_reg { +M_CAN_CREL =0x0 , +M_CAN_ENDN =0x4 , +M_CAN_CUST =0x8 , +M_CAN_DBTP =0xc , +M_CAN_TEST =0x10 , +M_CAN_RWD =0x14 , +M_CAN_CCCR =0x18 , +M_CAN_NBTP =0x1c , +M_CAN_TSCC =0x20 , +M_CAN_TSCV =0x24 , +M_CAN_TOCC =0x28 , +M_CAN_TOCV =0x2c , +M_CAN_ECR =0x40 , +M_CAN_PSR =0x44 , +/* TDCR Register only available for version >=3.1.x */ +M_CAN_TDCR =0x48 , +M_CAN_IR =0x50 , +M_CAN_IE =0x54 , +M_CAN_ILS =0x58 , +M_CAN_ILE =0x5c , +M_CAN_GFC =0x80 , +M_CAN_SIDFC =0x84 , +M_CAN_XIDFC =0x88 , +M_CAN_XIDAM =0x90 , +M_CAN_HPMS =0x94 , +M_CAN_NDAT1 =0x98 , +M_CAN_NDAT2 =0x9c , +M_CAN_RXF0C =0xa0 , +M_CAN_RXF0S =0xa4 , +M_CAN_RXF0A =0xa8 , +M_CAN_RXBC =0xac , +M_CAN_RXF1C =0xb0 , +M_CAN_RXF1S =0xb4 , +M_CAN_RXF1A =0xb8 , +M_CAN_RXESC =0xbc , +M_CAN_TXBC =0xc0 , +M_CAN_TXFQS =0xc4 , +M_CAN_TXESC =0xc8 , +M_CAN_TXBRP =0xcc , +M_CAN_TXBAR =0xd0 , +M_CAN_TXBCR =0xd4 , +M_CAN_TXBTO =0xd8 , +M_CAN_TXBCF =0xdc , +M_CAN_TXBTIE =0xe0 , +M_CAN_TXBCIE =0xe4 , +M_CAN_TXEFC =0xf0 , +M_CAN_TXEFS =0xf4 , +M_CAN_TXEFA =0xf8 , +}; + +/* address offset and element number for each FIFO/Buffer in the Message RAM */ +struct mram_cfg { +u16 off; +u8 num; +}; + +struct m_can_classdev ; +Quoted 9 lines
drivers/net/can/m_can/m_can_platform.h line 117
Quoted 6 lines
drivers/net/can/m_can/m_can_platform.h new file +6
-0+typedef int (*can_dev_init ) (struct m_can_classdev *m_can_class); +typedef int (*can_clr_dev_interrupts ) (struct m_can_classdev *m_can_class); +typedef u32 (*can_reg_read ) (struct m_can_classdev *m_can_class,int reg); +typedef int (*can_reg_write ) (struct m_can_classdev *m_can_class,int reg,int val); +typedef u32 (*can_fifo_read ) (struct m_can_classdev *m_can_class,int addr_offset); +typedef int (*can_fifo_write ) (struct m_can_classdev *m_can_class,int addr_offset,int val);No typedefs in the kernel, please!
Quoted 1 line
OK. Just following some other conventions. I can remove
See
https://elixir.bootlin.com/linux/latest/source/Documentation/process/coding-style.rst#L318
Quoted 45 lines
Quoted 45 lines
Quoted 30 lines
drivers/net/can/m_can/m_can_platform.h new file +29
-0+struct m_can_classdev { +struct can_priv can; +struct napi_struct napi; +struct net_device *net; +struct device *dev; +struct clk *hclk; +struct clk *cclk; + +struct workqueue_struct *wq; +struct work_struct tx_work; +struct sk_buff *skb; + +struct can_bittiming_const *bit_timing; +struct can_bittiming_const *data_timing; + +void *device_data; + +/* Device specific call backs */ +can_dev_init device_init; +can_clr_dev_interrupts clr_dev_interrupts; +can_reg_read read_reg; +can_reg_write write_reg; +can_fifo_read read_fifo; +can_fifo_write write_fifo; + +int version; +int freq; +u32 irqstatus; +drivers/net/can/m_can/m_can_platform.h line 157
Quoted 10 lines
drivers/net/can/m_can/m_can_platform.h new file +10
-0+int pm_clock_support; +bool is_peripherial; + +struct mram_cfg mcfg[MRAM_CFG_NUM ]; +}; + +struct m_can_classdev *m_can_core_allocate_dev (struct device *dev); +int m_can_core_register (struct m_can_classdev *m_can_dev); +void m_can_core_unregister (struct m_can_classdev *m_can_dev); +int m_can_core_get_clocks (struct m_can_classdev *m_can_dev);You use here three different prefixes: "
m_can_core", "m_can_classdev" and "m_can_dev". There should be just one principle name for the struct, func, args and vars, e.g.:
Quoted 4 lines
Quoted 2 lines
int m_can_device_register(struct m_can_device *mcan_dev);OK I will commonize the naming. I may go with the above or call it
m_can_class.
See also my related comment in the next patch.
Wolfgang.