can: m_can: Create a m_can platform framework
Subsystems (5)
PATCH v7 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.
---
v7 - Fixed remaining new checkpatch issues, removed CSR setting, fixed tx hard
start function to return tx_busy, and renamed device callbacks - https://lore.kernel.org/patchwork/patch/1047220/
v6 - Squashed platform patch to this patch for bissectablity, fixed coding style issues, updated Kconfig help, placed mcan reg offsets back into c file, renamed priv->skb to priv->tx_skb and cleared perp interrupts at ISR start - Patch 1 comments - https://lore.kernel.org/patchwork/patch/1042446/
Patch 2 comments - https://lore.kernel.org/patchwork/patch/1042442/
drivers/net/can/m_can/Kconfig | 13 +- drivers/net/can/m_can/Makefile | 1 + drivers/net/can/m_can/m_can.c | 700 +++++++++++++------------ drivers/net/can/m_can/m_can.h | 110 ++++ drivers/net/can/m_can/m_can_platform.c | 202 +++++++ 5 files changed, 682 insertions(+), 344 deletions(-) create mode 100644 drivers/net/can/m_can/m_can.h create mode 100644 drivers/net/can/m_can/m_can_platform.c
drivers/net/can/m_can/Kconfig +11 -2
@@ -1,5 +1,14 @@ config CAN_M_CAN + tristate "Bosch M_CAN support" + ---help--- + Say Y here if you want support for Bosch M_CAN controller framework. + This is common support for devices that embed the Bosch M_CAN IP. + +config CAN_M_CAN_PLATFORM + tristate "Bosch M_CAN support for io-mapped devices" depends on HAS_IOMEM - tristate "Bosch M_CAN devices" + depends on CAN_M_CAN ---help--- - Say Y here if you want to support for Bosch M_CAN controller. + Say Y here if you want support for IO Mapped Bosch M_CAN controller. + This support is for devices that have the Bosch M_CAN controller + IP embedded into the device and the IP is IO Mapped to the processor.
drivers/net/can/m_can/Makefile +1 -0
@@ -3,3 +3,4 @@ # obj-$(CONFIG_CAN_M_CAN) += m_can.o +obj-$(CONFIG_CAN_M_CAN_PLATFORM) += m_can_platform.o
drivers/net/can/m_can/m_can.c +358 -342
994 lines not on this page · raw message
drivers/net/can/m_can/m_can.h new file +110 -0
110 lines not on this page · raw message
drivers/net/can/m_can/m_can_platform.c new file +202 -0
202 lines not on this page · raw message
Signature · 1 line
2.20.1.390.gb5101f9297
PATCH v7 2/4 can: m_can: Rename m_can_priv to m_can_classdev
Rename the common m_can_priv class structure tom_can_classdev as this is more descriptive.
---
v7 - Fixed remaining checkpatch issues, renamed priv to cdev - https://lore.kernel.org/patchwork/patch/1047219/
v6 - No changes only rebase changes possibly can squash into the first patch -
https://lore.kernel.org/patchwork/patch/1042444/
drivers/net/can/m_can/m_can.c | 524 ++++++++++++------------- drivers/net/can/m_can/m_can.h | 28 +- drivers/net/can/m_can/m_can_platform.c | 16 +- 3 files changed, 284 insertions(+), 284 deletions(-)
drivers/net/can/m_can/m_can.c +262 -262
@@ -320,94 +320,94 @@ enum m_can_reg {#define TX_EVENT_MM_SHIFT TX_BUF_MM_SHIFT#define TX_EVENT_MM_MASK (0xff << TX_EVENT_MM_SHIFT) -static u32 m_can_read (struct m_can_priv *priv,enum m_can_reg reg) +static u32 m_can_read (struct m_can_classdev *cdev,enum m_can_reg reg) { -if (priv->ops->read_reg) -return priv->ops->read_reg (priv, reg); +if (cdev->ops->read_reg) +return cdev->ops->read_reg (cdev, reg);else return -EINVAL ; } -static int m_can_write (struct m_can_priv *priv,enum m_can_reg reg,u32 val) +static int m_can_write (struct m_can_classdev *cdev,enum m_can_reg reg,u32 val) { -if (priv->ops->write_reg) -return priv->ops->write_reg (priv, reg, val); +if (cdev->ops->write_reg) +return cdev->ops->write_reg (cdev, reg, val);else return -EINVAL ; } -static u32 m_can_fifo_read (struct m_can_priv *priv, +static u32 m_can_fifo_read (struct m_can_classdev *cdev,u32 fgi,unsigned int offset) { -u32 addr_offset = priv->mcfg[MRAM_RXF0 ].off + fgi *RXF0_ELEMENT_SIZE + +u32 addr_offset = cdev->mcfg[MRAM_RXF0 ].off + fgi *RXF0_ELEMENT_SIZE + offset; -if (priv->ops->read_fifo) -return priv->ops->read_fifo (priv, addr_offset); +if (cdev->ops->read_fifo) +return cdev->ops->read_fifo (cdev, addr_offset);else return -EINVAL ; } -static u32 m_can_fifo_write (struct m_can_priv *priv, +static u32 m_can_fifo_write (struct m_can_classdev *cdev,u32 fpi,unsigned int offset,u32 val) { -u32 addr_offset = priv->mcfg[MRAM_TXB ].off + fpi *TXB_ELEMENT_SIZE + +u32 addr_offset = cdev->mcfg[MRAM_TXB ].off + fpi *TXB_ELEMENT_SIZE + offset; -if (priv->ops->write_fifo) -return priv->ops->write_fifo (priv, addr_offset, val); +if (cdev->ops->write_fifo) +return cdev->ops->write_fifo (cdev, addr_offset, val);else return -EINVAL ; } -static u32 m_can_fifo_write_no_off (struct m_can_priv *priv, +static u32 m_can_fifo_write_no_off (struct m_can_classdev *cdev,u32 fpi,u32 val) { -if (priv->ops->write_fifo) -return priv->ops->write_fifo (priv, fpi, val); +if (cdev->ops->write_fifo) +return cdev->ops->write_fifo (cdev, fpi, val);else return 0 ; } -static u32 m_can_txe_fifo_read (struct m_can_priv *priv,u32 fgi,u32 offset) +static u32 m_can_txe_fifo_read (struct m_can_classdev *cdev,u32 fgi,u32 offset) { -u32 addr_offset = priv->mcfg[MRAM_TXE ].off + fgi *TXE_ELEMENT_SIZE + +u32 addr_offset = cdev->mcfg[MRAM_TXE ].off + fgi *TXE_ELEMENT_SIZE + offset; -if (priv->ops->read_fifo) -return priv->ops->read_fifo (priv, addr_offset); +if (cdev->ops->read_fifo) +return cdev->ops->read_fifo (cdev, addr_offset);else return -EINVAL ; } -static inline bool m_can_tx_fifo_full (struct m_can_priv *priv) +static inline bool m_can_tx_fifo_full (struct m_can_classdev *cdev) { -return !!(m_can_read (priv,M_CAN_TXFQS ) &TXFQS_TFQF ); +return !!(m_can_read (cdev,M_CAN_TXFQS ) &TXFQS_TFQF ); } -void m_can_config_endisable (struct m_can_priv *priv,bool enable) +void m_can_config_endisable (struct m_can_classdev *cdev,bool enable) { -u32 cccr =m_can_read (priv,M_CAN_CCCR ); +u32 cccr =m_can_read (cdev,M_CAN_CCCR );u32 timeout =10 ;u32 val =0 ;if (enable) {/* enable m_can configuration */ -m_can_write (priv,M_CAN_CCCR , cccr |CCCR_INIT ); +m_can_write (cdev,M_CAN_CCCR , cccr |CCCR_INIT );udelay (5 );/* CCCR.CCE can only be set/reset while CCCR.INIT = '1' */ -m_can_write (priv,M_CAN_CCCR , cccr |CCCR_INIT |CCCR_CCE ); +m_can_write (cdev,M_CAN_CCCR , cccr |CCCR_INIT |CCCR_CCE ); }else { -m_can_write (priv,M_CAN_CCCR , cccr & ~(CCCR_INIT |CCCR_CCE )); +m_can_write (cdev,M_CAN_CCCR , cccr & ~(CCCR_INIT |CCCR_CCE )); }/* there's a delay for module initialization */ if (enable) val =CCCR_INIT |CCCR_CCE ; -while ((m_can_read (priv,M_CAN_CCCR ) & (CCCR_INIT |CCCR_CCE )) != val) { +while ((m_can_read (cdev,M_CAN_CCCR ) & (CCCR_INIT |CCCR_CCE )) != val) {if (timeout ==0 ) { -netdev_warn (priv->net,"Failed to init module\n" ); +netdev_warn (cdev->net,"Failed to init module\n" );return ; } timeout--;
@@ -415,21 +415,21 @@ void m_can_config_endisable(struct m_can_priv *priv, bool enable) } } -static inline void m_can_enable_all_interrupts (struct m_can_priv *priv) +static inline void m_can_enable_all_interrupts (struct m_can_classdev *cdev) {/* Only interrupt line 0 is used in this driver */ -m_can_write (priv,M_CAN_ILE ,ILE_EINT0 ); +m_can_write (cdev,M_CAN_ILE ,ILE_EINT0 ); } -static inline void m_can_disable_all_interrupts (struct m_can_priv *priv) +static inline void m_can_disable_all_interrupts (struct m_can_classdev *cdev) { -m_can_write (priv,M_CAN_ILE ,0x0 ); +m_can_write (cdev,M_CAN_ILE ,0x0 ); }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 *cdev =netdev_priv (dev);struct canfd_frame *cf;struct sk_buff *skb;u32 id, fgi, dlc;
@@ -437,7 +437,7 @@ static void m_can_read_fifo(struct net_device *dev, u32 rxfs)/* calculate the fifo get index for where to read data */ fgi = (rxfs &RXFS_FGI_MASK ) >>RXFS_FGI_SHIFT ; - dlc =m_can_fifo_read (priv, fgi,M_CAN_FIFO_DLC ); + dlc =m_can_fifo_read (cdev, fgi,M_CAN_FIFO_DLC );if (dlc &RX_BUF_FDF ) skb =alloc_canfd_skb (dev, &cf);else
@@ -452,7 +452,7 @@ static void m_can_read_fifo(struct net_device *dev, u32 rxfs)else cf->len =get_can_dlc ((dlc >>16 ) &0x0F ); - id =m_can_fifo_read (priv, fgi,M_CAN_FIFO_ID ); + id =m_can_fifo_read (cdev, fgi,M_CAN_FIFO_ID );if (id &RX_BUF_XTD ) cf->can_id = (id &CAN_EFF_MASK ) |CAN_EFF_FLAG ;else
@@ -471,12 +471,12 @@ static void m_can_read_fifo(struct net_device *dev, u32 rxfs)for (i =0 ; i < cf->len; i +=4 ) *(u32 *)(cf->data + i) = -m_can_fifo_read (priv, fgi, +m_can_fifo_read (cdev, fgi,M_CAN_FIFO_DATA (i /4 )); }/* acknowledge rx fifo 0 */ -m_can_write (priv,M_CAN_RXF0A , fgi); +m_can_write (cdev,M_CAN_RXF0A , fgi); stats->rx_packets++; stats->rx_bytes += cf->len;
@@ -486,11 +486,11 @@ 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 *cdev =netdev_priv (dev);u32 pkts =0 ;u32 rxfs; - rxfs =m_can_read (priv,M_CAN_RXF0S ); + rxfs =m_can_read (cdev,M_CAN_RXF0S );if (!(rxfs &RXFS_FFL_MASK )) {netdev_dbg (dev,"no messages in fifo0\n" );return 0 ;
@@ -504,7 +504,7 @@ static int m_can_do_rx_poll(struct net_device *dev, int quota) quota--; pkts++; - rxfs =m_can_read (priv,M_CAN_RXF0S ); + rxfs =m_can_read (cdev,M_CAN_RXF0S ); }if (pkts)
@@ -539,12 +539,12 @@ 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 *cdev =netdev_priv (dev);struct net_device_stats *stats = &dev->stats;struct can_frame *cf;struct sk_buff *skb; - priv->can.can_stats.bus_error++; + cdev->can.can_stats.bus_error++; stats->rx_errors++;/* propagate the error condition to the CAN stack */
@@ -596,51 +596,51 @@ 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 *cdev =netdev_priv (dev);unsigned int ecr; - ecr =m_can_read (priv,M_CAN_ECR ); + ecr =m_can_read (cdev,M_CAN_ECR ); bec->rxerr = (ecr &ECR_REC_MASK ) >>ECR_REC_SHIFT ; bec->txerr = (ecr &ECR_TEC_MASK ) >>ECR_TEC_SHIFT ;return 0 ; } -static int m_can_clk_start (struct m_can_priv *priv) +static int m_can_clk_start (struct m_can_classdev *cdev) {int err; -if (priv->pm_clock_support ==0 ) +if (cdev->pm_clock_support ==0 )return 0 ; - err =pm_runtime_get_sync (priv->dev); + err =pm_runtime_get_sync (cdev->dev);if (err <0 ) { -pm_runtime_put_noidle (priv->dev); +pm_runtime_put_noidle (cdev->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 *cdev) { -if (priv->pm_clock_support) -pm_runtime_put_sync (priv->dev); +if (cdev->pm_clock_support) +pm_runtime_put_sync (cdev->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 *cdev =netdev_priv (dev);int err; - err =m_can_clk_start (priv); + err =m_can_clk_start (cdev);if (err)return err;__m_can_get_berr_counter (dev, bec); -m_can_clk_stop (priv); +m_can_clk_stop (cdev);return 0 ; }
@@ -648,7 +648,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 *cdev =netdev_priv (dev);struct net_device_stats *stats = &dev->stats;struct can_frame *cf;struct sk_buff *skb;
@@ -658,19 +658,19 @@ static int m_can_handle_state_change(struct net_device *dev,switch (new_state) {case CAN_STATE_ERROR_ACTIVE :/* error warning state */ - priv->can.can_stats.error_warning++; - priv->can.state =CAN_STATE_ERROR_WARNING ; + cdev->can.can_stats.error_warning++; + cdev->can.state =CAN_STATE_ERROR_WARNING ;break ;case CAN_STATE_ERROR_PASSIVE :/* error passive state */ - priv->can.can_stats.error_passive++; - priv->can.state =CAN_STATE_ERROR_PASSIVE ; + cdev->can.can_stats.error_passive++; + cdev->can.state =CAN_STATE_ERROR_PASSIVE ;break ;case CAN_STATE_BUS_OFF :/* bus-off state */ - priv->can.state =CAN_STATE_BUS_OFF ; -m_can_disable_all_interrupts (priv); - priv->can.can_stats.bus_off++; + cdev->can.state =CAN_STATE_BUS_OFF ; +m_can_disable_all_interrupts (cdev); + cdev->can.can_stats.bus_off++;can_bus_off (dev);break ;default :
@@ -697,7 +697,7 @@ static int m_can_handle_state_change(struct net_device *dev,case CAN_STATE_ERROR_PASSIVE :/* error passive state */ cf->can_id |=CAN_ERR_CRTL ; - ecr =m_can_read (priv,M_CAN_ECR ); + ecr =m_can_read (cdev,M_CAN_ECR );if (ecr &ECR_RP ) cf->data[1 ] |=CAN_ERR_CRTL_RX_PASSIVE ;if (bec.txerr >127 )
@@ -722,25 +722,25 @@ 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 *cdev =netdev_priv (dev);int work_done =0 ;if ((psr &PSR_EW ) && - (priv->can.state !=CAN_STATE_ERROR_WARNING )) { + (cdev->can.state !=CAN_STATE_ERROR_WARNING )) {netdev_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 )) { + (cdev->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 )) { + (cdev->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 );
@@ -775,14 +775,14 @@ 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 *cdev =netdev_priv (dev);int work_done =0 ;if (irqstatus &IR_RF0L ) work_done +=m_can_handle_lost_msg (dev);/* handle lec errors on the bus */ -if ((priv->can.ctrlmode &CAN_CTRLMODE_BERR_REPORTING ) && +if ((cdev->can.ctrlmode &CAN_CTRLMODE_BERR_REPORTING ) &&is_lec_err (psr)) work_done +=m_can_handle_lec_err (dev, psr &LEC_UNUSED );
@@ -794,15 +794,15 @@ static int m_can_handle_bus_errors(struct net_device *dev, u32 irqstatus,static int m_can_rx_handler (struct net_device *dev,int quota) { -struct m_can_priv *priv =netdev_priv (dev); +struct m_can_classdev *cdev =netdev_priv (dev);int work_done =0 ;u32 irqstatus, psr; - irqstatus = priv->irqstatus |m_can_read (priv,M_CAN_IR ); + irqstatus = cdev->irqstatus |m_can_read (cdev,M_CAN_IR );if (!irqstatus) goto end; - psr =m_can_read (priv,M_CAN_PSR ); + psr =m_can_read (cdev,M_CAN_PSR );if (irqstatus &IR_ERR_STATE ) work_done +=m_can_handle_state_errors (dev, psr);
@@ -817,11 +817,11 @@ static int m_can_rx_handler(struct net_device *dev, int quota)static int m_can_rx_peripherial (struct net_device *dev) { -struct m_can_priv *priv =netdev_priv (dev); +struct m_can_classdev *cdev =netdev_priv (dev);m_can_rx_handler (dev,1 ); -m_can_enable_all_interrupts (priv); +m_can_enable_all_interrupts (cdev);return 0 ; }
@@ -829,13 +829,13 @@ static int m_can_rx_peripherial(struct net_device *dev)static int m_can_poll (struct napi_struct *napi,int quota) {struct net_device *dev = napi->dev; -struct m_can_priv *priv =netdev_priv (dev); +struct m_can_classdev *cdev =netdev_priv (dev);int work_done; work_done =m_can_rx_handler (dev, quota);if (work_done < quota) {napi_complete_done (napi, work_done); -m_can_enable_all_interrupts (priv); +m_can_enable_all_interrupts (cdev); }return work_done;
@@ -849,11 +849,11 @@ 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 *cdev =netdev_priv (dev);struct net_device_stats *stats = &dev->stats;/* read tx event fifo status */ - m_can_txefs =m_can_read (priv,M_CAN_TXEFS ); + m_can_txefs =m_can_read (cdev,M_CAN_TXEFS );/* Get Tx Event fifo element count */ txe_count = (m_can_txefs &TXEFS_EFFL_MASK )
@@ -862,15 +862,15 @@ static void m_can_echo_tx_event(struct net_device *dev)/* Get and process all sent elements */ for (i =0 ; i < txe_count; i++) {/* retrieve get index */ - fgi = (m_can_read (priv,M_CAN_TXEFS ) &TXEFS_EFGI_MASK ) + fgi = (m_can_read (cdev,M_CAN_TXEFS ) &TXEFS_EFGI_MASK ) >>TXEFS_EFGI_SHIFT ;/* get message marker */ - msg_mark = (m_can_txe_fifo_read (priv, fgi,4 ) & + msg_mark = (m_can_txe_fifo_read (cdev, fgi,4 ) &TX_EVENT_MM_MASK ) >>TX_EVENT_MM_SHIFT ;/* ack txe element */ -m_can_write (priv,M_CAN_TXEFA , (TXEFA_EFAI_MASK & +m_can_write (cdev,M_CAN_TXEFA , (TXEFA_EFAI_MASK & (fgi <<TXEFA_EFAI_SHIFT )));/* update stats */
@@ -882,20 +882,20 @@ 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 *cdev =netdev_priv (dev);struct net_device_stats *stats = &dev->stats;u32 ir; - ir =m_can_read (priv,M_CAN_IR ); + ir =m_can_read (cdev,M_CAN_IR );if (!ir)return IRQ_NONE ;/* ACK all irqs */ if (ir &IR_ALL_INT ) -m_can_write (priv,M_CAN_IR , ir); +m_can_write (cdev,M_CAN_IR , ir); -if (priv->ops->clear_interrupts) - priv->ops->clear_interrupts (priv); +if (cdev->ops->clear_interrupts) + cdev->ops->clear_interrupts (cdev);/* schedule NAPI in case of * - rx IRQ
@@ -903,15 +903,15 @@ static irqreturn_t m_can_isr(int irq, void *dev_id)* - bus error IRQ and bus error reporting */ if ((ir &IR_RF0N ) || (ir &IR_ERR_ALL_30X )) { - priv->irqstatus = ir; -m_can_disable_all_interrupts (priv); -if (!priv->is_peripherial) -napi_schedule (&priv->napi); + cdev->irqstatus = ir; +m_can_disable_all_interrupts (cdev); +if (!cdev->is_peripherial) +napi_schedule (&cdev->napi);else m_can_rx_peripherial (dev); } -if (priv->version ==30 ) { +if (cdev->version ==30 ) {if (ir &IR_TC ) {/* Transmission Complete Interrupt*/ stats->tx_bytes +=can_get_echo_skb (dev,0 );
@@ -925,7 +925,7 @@ static irqreturn_t m_can_isr(int irq, void *dev_id)m_can_echo_tx_event (dev);can_led_event (dev,CAN_LED_EVENT_TX );if (netif_queue_stopped (dev) && - !m_can_tx_fifo_full (priv)) + !m_can_tx_fifo_full (cdev))netif_wake_queue (dev); } }
@@ -983,9 +983,9 @@ 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); -const struct can_bittiming *bt = &priv->can.bittiming; -const struct can_bittiming *dbt = &priv->can.data_bittiming; +struct m_can_classdev *cdev =netdev_priv (dev); +const struct can_bittiming *bt = &cdev->can.bittiming; +const struct can_bittiming *dbt = &cdev->can.data_bittiming;u16 brp, sjw, tseg1, tseg2;u32 reg_btp;
@@ -995,9 +995,9 @@ static int m_can_set_bittiming(struct net_device *dev) tseg2 = bt->phase_seg2 -1 ; reg_btp = (brp <<NBTP_NBRP_SHIFT ) | (sjw <<NBTP_NSJW_SHIFT ) | (tseg1 <<NBTP_NTSEG1_SHIFT ) | (tseg2 <<NBTP_NTSEG2_SHIFT ); -m_can_write (priv,M_CAN_NBTP , reg_btp); +m_can_write (cdev,M_CAN_NBTP , reg_btp); -if (priv->can.ctrlmode &CAN_CTRLMODE_FD ) { +if (cdev->can.ctrlmode &CAN_CTRLMODE_FD ) { reg_btp =0 ; brp = dbt->brp -1 ; sjw = dbt->sjw -1 ;
@@ -1019,7 +1019,7 @@ static int m_can_set_bittiming(struct net_device *dev)/* Equation based on Bosch's M_CAN User Manual's * Transmitter Delay Compensation Section */ - tdco = (priv->can.clock.freq /1000 ) * + tdco = (cdev->can.clock.freq /1000 ) * ssp / dbt->bitrate;/* Max valid TDCO value is 127 */
@@ -1030,7 +1030,7 @@ static int m_can_set_bittiming(struct net_device *dev) } reg_btp |=DBTP_TDC ; -m_can_write (priv,M_CAN_TDCR , +m_can_write (cdev,M_CAN_TDCR , tdco <<TDCR_TDCO_SHIFT ); }
@@ -1039,7 +1039,7 @@ static int m_can_set_bittiming(struct net_device *dev) (tseg1 <<DBTP_DTSEG1_SHIFT ) | (tseg2 <<DBTP_DTSEG2_SHIFT ); -m_can_write (priv,M_CAN_DBTP , reg_btp); +m_can_write (cdev,M_CAN_DBTP , reg_btp); }return 0 ;
@@ -1056,63 +1056,63 @@ 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 *cdev =netdev_priv (dev);u32 cccr, test; -m_can_config_endisable (priv, true); +m_can_config_endisable (cdev, true);/* RX Buffer/FIFO Element Size 64 bytes data field */ -m_can_write (priv,M_CAN_RXESC ,M_CAN_RXESC_64BYTES ); +m_can_write (cdev,M_CAN_RXESC ,M_CAN_RXESC_64BYTES );/* Accept Non-matching Frames Into FIFO 0 */ -m_can_write (priv,M_CAN_GFC ,0x0 ); +m_can_write (cdev,M_CAN_GFC ,0x0 ); -if (priv->version ==30 ) { +if (cdev->version ==30 ) {/* only support one Tx Buffer currently */ -m_can_write (priv,M_CAN_TXBC , (1 <<TXBC_NDTB_SHIFT ) | - priv->mcfg[MRAM_TXB ].off); +m_can_write (cdev,M_CAN_TXBC , (1 <<TXBC_NDTB_SHIFT ) | + cdev->mcfg[MRAM_TXB ].off); }else {/* TX FIFO is used for newer IP Core versions */ -m_can_write (priv,M_CAN_TXBC , - (priv->mcfg[MRAM_TXB ].num <<TXBC_TFQS_SHIFT ) | - (priv->mcfg[MRAM_TXB ].off)); +m_can_write (cdev,M_CAN_TXBC , + (cdev->mcfg[MRAM_TXB ].num <<TXBC_TFQS_SHIFT ) | + (cdev->mcfg[MRAM_TXB ].off)); }/* support 64 bytes payload */ -m_can_write (priv,M_CAN_TXESC ,TXESC_TBDS_64BYTES ); +m_can_write (cdev,M_CAN_TXESC ,TXESC_TBDS_64BYTES );/* TX Event FIFO */ -if (priv->version ==30 ) { -m_can_write (priv,M_CAN_TXEFC , (1 <<TXEFC_EFS_SHIFT ) | - priv->mcfg[MRAM_TXE ].off); +if (cdev->version ==30 ) { +m_can_write (cdev,M_CAN_TXEFC , (1 <<TXEFC_EFS_SHIFT ) | + cdev->mcfg[MRAM_TXE ].off); }else {/* Full TX Event FIFO is used */ -m_can_write (priv,M_CAN_TXEFC , - ((priv->mcfg[MRAM_TXE ].num <<TXEFC_EFS_SHIFT ) +m_can_write (cdev,M_CAN_TXEFC , + ((cdev->mcfg[MRAM_TXE ].num <<TXEFC_EFS_SHIFT ) &TXEFC_EFS_MASK ) | - priv->mcfg[MRAM_TXE ].off); + cdev->mcfg[MRAM_TXE ].off); }/* rx fifo configuration, blocking mode, fifo size 1 */ -m_can_write (priv,M_CAN_RXF0C , - (priv->mcfg[MRAM_RXF0 ].num <<RXFC_FS_SHIFT ) | - priv->mcfg[MRAM_RXF0 ].off); +m_can_write (cdev,M_CAN_RXF0C , + (cdev->mcfg[MRAM_RXF0 ].num <<RXFC_FS_SHIFT ) | + cdev->mcfg[MRAM_RXF0 ].off); -m_can_write (priv,M_CAN_RXF1C , - (priv->mcfg[MRAM_RXF1 ].num <<RXFC_FS_SHIFT ) | - priv->mcfg[MRAM_RXF1 ].off); +m_can_write (cdev,M_CAN_RXF1C , + (cdev->mcfg[MRAM_RXF1 ].num <<RXFC_FS_SHIFT ) | + cdev->mcfg[MRAM_RXF1 ].off); - cccr =m_can_read (priv,M_CAN_CCCR ); - test =m_can_read (priv,M_CAN_TEST ); + cccr =m_can_read (cdev,M_CAN_CCCR ); + test =m_can_read (cdev,M_CAN_TEST ); test &= ~TEST_LBCK ; -if (priv->version ==30 ) { +if (cdev->version ==30 ) {/* Version 3.0.x */ cccr &= ~(CCCR_TEST |CCCR_MON | (CCCR_CMR_MASK <<CCCR_CMR_SHIFT ) | (CCCR_CME_MASK <<CCCR_CME_SHIFT )); -if (priv->can.ctrlmode &CAN_CTRLMODE_FD ) +if (cdev->can.ctrlmode &CAN_CTRLMODE_FD ) cccr |=CCCR_CME_CANFD_BRS <<CCCR_CME_SHIFT ; }else {
@@ -1121,61 +1121,61 @@ static void m_can_chip_config(struct net_device *dev) CCCR_NISO);/* Only 3.2.x has NISO Bit implemented */ -if (priv->can.ctrlmode &CAN_CTRLMODE_FD_NON_ISO ) +if (cdev->can.ctrlmode &CAN_CTRLMODE_FD_NON_ISO ) cccr |=CCCR_NISO ; -if (priv->can.ctrlmode &CAN_CTRLMODE_FD ) +if (cdev->can.ctrlmode &CAN_CTRLMODE_FD ) cccr |= (CCCR_BRSE |CCCR_FDOE ); }/* Loopback Mode */ -if (priv->can.ctrlmode &CAN_CTRLMODE_LOOPBACK ) { +if (cdev->can.ctrlmode &CAN_CTRLMODE_LOOPBACK ) { cccr |=CCCR_TEST |CCCR_MON ; test |=TEST_LBCK ; }/* Enable Monitoring (all versions) */ -if (priv->can.ctrlmode &CAN_CTRLMODE_LISTENONLY ) +if (cdev->can.ctrlmode &CAN_CTRLMODE_LISTENONLY ) cccr |=CCCR_MON ;/* Write config */ -m_can_write (priv,M_CAN_CCCR , cccr); -m_can_write (priv,M_CAN_TEST , test); +m_can_write (cdev,M_CAN_CCCR , cccr); +m_can_write (cdev,M_CAN_TEST , test);/* Enable interrupts */ -m_can_write (priv,M_CAN_IR ,IR_ALL_INT ); -if (!(priv->can.ctrlmode &CAN_CTRLMODE_BERR_REPORTING )) -if (priv->version ==30 ) -m_can_write (priv,M_CAN_IE ,IR_ALL_INT & +m_can_write (cdev,M_CAN_IR ,IR_ALL_INT ); +if (!(cdev->can.ctrlmode &CAN_CTRLMODE_BERR_REPORTING )) +if (cdev->version ==30 ) +m_can_write (cdev,M_CAN_IE ,IR_ALL_INT & ~(IR_ERR_LEC_30X ));else -m_can_write (priv,M_CAN_IE ,IR_ALL_INT & +m_can_write (cdev,M_CAN_IE ,IR_ALL_INT & ~(IR_ERR_LEC_31X ));else -m_can_write (priv,M_CAN_IE ,IR_ALL_INT ); +m_can_write (cdev,M_CAN_IE ,IR_ALL_INT );/* route all interrupts to INT0 */ -m_can_write (priv,M_CAN_ILS ,ILS_ALL_INT0 ); +m_can_write (cdev,M_CAN_ILS ,ILS_ALL_INT0 );/* set bittiming params */ m_can_set_bittiming (dev); -m_can_config_endisable (priv, false); +m_can_config_endisable (cdev, false); -if (priv->ops->init) - priv->ops->init (priv); +if (cdev->ops->init) + cdev->ops->init (cdev); }static void m_can_start (struct net_device *dev) { -struct m_can_priv *priv =netdev_priv (dev); +struct m_can_classdev *cdev =netdev_priv (dev);/* basic m_can configuration */ m_can_chip_config (dev); - priv->can.state =CAN_STATE_ERROR_ACTIVE ; + cdev->can.state =CAN_STATE_ERROR_ACTIVE ; -m_can_enable_all_interrupts (priv); +m_can_enable_all_interrupts (cdev); }static int m_can_set_mode (struct net_device *dev,enum can_mode mode)
@@ -1197,7 +1197,7 @@ 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 (struct m_can_priv *priv) +static int m_can_check_core_release (struct m_can_classdev *cdev) {u32 crel_reg;u8 rel;
@@ -1207,7 +1207,7 @@ static int m_can_check_core_release(struct m_can_priv *priv)/* 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 (priv,M_CAN_CREL ); + crel_reg =m_can_read (cdev,M_CAN_CREL ); rel = (u8)((crel_reg &CREL_REL_MASK ) >>CREL_REL_SHIFT ); step = (u8)((crel_reg &CREL_STEP_MASK ) >>CREL_STEP_SHIFT );
@@ -1225,19 +1225,19 @@ static int m_can_check_core_release(struct m_can_priv *priv)/* Selectable Non ISO support only in version 3.2.x * This function checks if the bit is writable. */ -static bool m_can_niso_supported (struct m_can_priv *priv) +static bool m_can_niso_supported (struct m_can_classdev *cdev) {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 ); +m_can_config_endisable (cdev, true); + cccr_reg =m_can_read (cdev,M_CAN_CCCR ); cccr_reg |=CCCR_NISO ; -m_can_write (priv,M_CAN_CCCR , cccr_reg); +m_can_write (cdev,M_CAN_CCCR , cccr_reg);for (i =0 ; i <=10 ; i++) { - cccr_poll =m_can_read (priv,M_CAN_CCCR ); + cccr_poll =m_can_read (cdev,M_CAN_CCCR );if (cccr_poll == cccr_reg) { niso_timeout =0 ;break ;
@@ -1248,15 +1248,15 @@ static bool m_can_niso_supported(struct m_can_priv *priv)/* Clear NISO */ cccr_reg &= ~(CCCR_NISO ); -m_can_write (priv,M_CAN_CCCR , cccr_reg); +m_can_write (cdev,M_CAN_CCCR , cccr_reg); -m_can_config_endisable (priv, false); +m_can_config_endisable (cdev, false);/* return false if time out (-ETIMEDOUT), else return true */ return !niso_timeout; } -static int m_can_dev_setup (struct m_can_priv *m_can_dev) +static int m_can_dev_setup (struct m_can_classdev *m_can_dev) {struct net_device *dev = m_can_dev->net;int m_can_version;
@@ -1333,30 +1333,30 @@ static int m_can_dev_setup(struct m_can_priv *m_can_dev)static void m_can_stop (struct net_device *dev) { -struct m_can_priv *priv =netdev_priv (dev); +struct m_can_classdev *cdev =netdev_priv (dev);/* disable all interrupts */ -m_can_disable_all_interrupts (priv); +m_can_disable_all_interrupts (cdev);/* set the state as STOPPED */ - priv->can.state =CAN_STATE_STOPPED ; + cdev->can.state =CAN_STATE_STOPPED ; }static int m_can_close (struct net_device *dev) { -struct m_can_priv *priv =netdev_priv (dev); +struct m_can_classdev *cdev =netdev_priv (dev);netif_stop_queue (dev); -if (!priv->is_peripherial) -napi_disable (&priv->napi); +if (!cdev->is_peripherial) +napi_disable (&cdev->napi);m_can_stop (dev); -m_can_clk_stop (priv); +m_can_clk_stop (cdev);free_irq (dev->irq, dev); -if (priv->is_peripherial) { - priv->tx_skb =NULL ; -destroy_workqueue (priv->tx_wq); - priv->tx_wq =NULL ; +if (cdev->is_peripherial) { + cdev->tx_skb =NULL ; +destroy_workqueue (cdev->tx_wq); + cdev->tx_wq =NULL ; }close_candev (dev);
@@ -1367,23 +1367,23 @@ 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 *cdev =netdev_priv (dev);/*get wrap around for loopback skb index */ -unsigned int wrap = priv->can.echo_skb_max; +unsigned int wrap = cdev->can.echo_skb_max;int next_idx;/* calculate next index */ next_idx = (++putidx >= wrap ?0 : putidx);/* check if occupied */ -return !!priv->can.echo_skb[next_idx]; +return !!cdev->can.echo_skb[next_idx]; } -static netdev_tx_t m_can_tx_handler (struct m_can_priv *priv) +static netdev_tx_t m_can_tx_handler (struct m_can_classdev *cdev) { -struct canfd_frame *cf = (struct canfd_frame *)priv->tx_skb->data; -struct net_device *dev = priv->net; -struct sk_buff *skb = priv->tx_skb; +struct canfd_frame *cf = (struct canfd_frame *)cdev->tx_skb->data; +struct net_device *dev = cdev->net; +struct sk_buff *skb = cdev->tx_skb;u32 id, cccr, fdflags;int i;int putidx;
@@ -1400,23 +1400,23 @@ static netdev_tx_t m_can_tx_handler(struct m_can_priv *priv)if (cf->can_id &CAN_RTR_FLAG ) id |=TX_BUF_RTR ; -if (priv->version ==30 ) { +if (cdev->version ==30 ) {netif_stop_queue (dev);/* message ram configuration */ -m_can_fifo_write (priv,0 ,M_CAN_FIFO_ID , id); -m_can_fifo_write (priv,0 ,M_CAN_FIFO_DLC , +m_can_fifo_write (cdev,0 ,M_CAN_FIFO_ID , id); +m_can_fifo_write (cdev,0 ,M_CAN_FIFO_DLC ,can_len2dlc (cf->len) <<16 );for (i =0 ; i < cf->len; i +=4 ) -m_can_fifo_write (priv,0 , +m_can_fifo_write (cdev,0 ,M_CAN_FIFO_DATA (i /4 ), *(u32 *)(cf->data + i));can_put_echo_skb (skb, dev,0 ); -if (priv->can.ctrlmode &CAN_CTRLMODE_FD ) { - cccr =m_can_read (priv,M_CAN_CCCR ); +if (cdev->can.ctrlmode &CAN_CTRLMODE_FD ) { + cccr =m_can_read (cdev,M_CAN_CCCR ); cccr &= ~(CCCR_CMR_MASK <<CCCR_CMR_SHIFT );if (can_is_canfd_skb (skb)) {if (cf->flags &CANFD_BRS )
@@ -1428,16 +1428,16 @@ static netdev_tx_t m_can_tx_handler(struct m_can_priv *priv) }else { cccr |=CCCR_CMR_CAN <<CCCR_CMR_SHIFT ; } -m_can_write (priv,M_CAN_CCCR , cccr); +m_can_write (cdev,M_CAN_CCCR , cccr); } -m_can_write (priv,M_CAN_TXBTIE ,0x1 ); -m_can_write (priv,M_CAN_TXBAR ,0x1 ); +m_can_write (cdev,M_CAN_TXBTIE ,0x1 ); +m_can_write (cdev,M_CAN_TXBAR ,0x1 );/* End of xmit function for version 3.0.x */ }else {/* Transmit routine for version >= v3.1.x */ /* Check if FIFO full */ -if (m_can_tx_fifo_full (priv)) { +if (m_can_tx_fifo_full (cdev)) {/* This shouldn't happen */ netif_stop_queue (dev); netdev_warn(dev,
@@ -1446,10 +1446,10 @@ static netdev_tx_t m_can_tx_handler(struct m_can_priv *priv) }/* get put index for frame */ - putidx = ((m_can_read (priv,M_CAN_TXFQS ) &TXFQS_TFQPI_MASK ) + putidx = ((m_can_read (cdev,M_CAN_TXFQS ) &TXFQS_TFQPI_MASK ) >>TXFQS_TFQPI_SHIFT );/* Write ID Field to FIFO Element */ -m_can_fifo_write (priv, putidx,M_CAN_FIFO_ID , id); +m_can_fifo_write (cdev, putidx,M_CAN_FIFO_ID , id);/* get CAN FD configuration of frame */ fdflags =0 ;
@@ -1464,14 +1464,14 @@ static netdev_tx_t m_can_tx_handler(struct m_can_priv *priv)* it is used in TX interrupt for * sending the correct echo frame */ -m_can_fifo_write (priv, putidx,M_CAN_FIFO_DLC , +m_can_fifo_write (cdev, putidx,M_CAN_FIFO_DLC , ((putidx <<TX_BUF_MM_SHIFT ) &TX_BUF_MM_MASK ) | (can_len2dlc (cf->len) <<16 ) | fdflags |TX_BUF_EFC );for (i =0 ; i < cf->len; i +=4 ) -m_can_fifo_write (priv, putidx,M_CAN_FIFO_DATA (i /4 ), +m_can_fifo_write (cdev, putidx,M_CAN_FIFO_DATA (i /4 ), *(u32 *)(cf->data + i));/* Push loopback echo.
@@ -1480,10 +1480,10 @@ static netdev_tx_t m_can_tx_handler(struct m_can_priv *priv)can_put_echo_skb (skb, dev, putidx);/* Enable TX FIFO element to start transfer */ -m_can_write (priv,M_CAN_TXBAR , (1 << putidx)); +m_can_write (cdev,M_CAN_TXBAR , (1 << putidx));/* stop network queue if fifo full */ -if (m_can_tx_fifo_full (priv) || +if (m_can_tx_fifo_full (cdev) ||m_can_next_echo_skb_occupied (dev, putidx))netif_stop_queue (dev); }
@@ -1493,35 +1493,35 @@ static netdev_tx_t m_can_tx_handler(struct m_can_priv *priv)static void m_can_tx_work_queue (struct work_struct *ws) { - struct m_can_priv *priv = container_of(ws, struct m_can_priv, + struct m_can_classdev *cdev = container_of(ws, struct m_can_classdev, tx_work);netdev_tx_t ret; - ret =m_can_tx_handler (priv); + ret =m_can_tx_handler (cdev);if (ret ==NETDEV_TX_OK ) - priv->tx_skb =NULL ; + cdev->tx_skb =NULL ; }static netdev_tx_t m_can_start_xmit (struct sk_buff *skb,struct net_device *dev) { -struct m_can_priv *priv =netdev_priv (dev); +struct m_can_classdev *cdev =netdev_priv (dev);if (can_dropped_invalid_skb (dev, skb))return NETDEV_TX_OK ; -if (priv->is_peripherial) { -if (priv->tx_skb) { +if (cdev->is_peripherial) { +if (cdev->tx_skb) {netdev_err (dev,"hard_xmit called while tx busy\n" );return NETDEV_TX_BUSY ; } - priv->tx_skb = skb; -netif_stop_queue (priv->net); -queue_work (priv->tx_wq, &priv->tx_work); + cdev->tx_skb = skb; +netif_stop_queue (cdev->net); +queue_work (cdev->tx_wq, &cdev->tx_work); }else { - priv->tx_skb = skb; -return m_can_tx_handler (priv); + cdev->tx_skb = skb; +return m_can_tx_handler (cdev); }return NETDEV_TX_OK ;
@@ -1529,10 +1529,10 @@ static netdev_tx_t m_can_start_xmit(struct sk_buff *skb,static int m_can_open (struct net_device *dev) { -struct m_can_priv *priv =netdev_priv (dev); +struct m_can_classdev *cdev =netdev_priv (dev);int err; - err =m_can_clk_start (priv); + err =m_can_clk_start (cdev);if (err)return err;
@@ -1544,16 +1544,16 @@ static int m_can_open(struct net_device *dev) }/* register interrupt handler */ -if (priv->is_peripherial) { - priv->tx_skb =NULL ; - priv->tx_wq =alloc_workqueue ("mcan_wq" , +if (cdev->is_peripherial) { + cdev->tx_skb =NULL ; + cdev->tx_wq =alloc_workqueue ("mcan_wq" ,WQ_FREEZABLE |WQ_MEM_RECLAIM ,0 ); -if (!priv->tx_wq) { +if (!cdev->tx_wq) { err = -ENOMEM ; goto out_wq_fail; } -INIT_WORK (&priv->tx_work, m_can_tx_work_queue); +INIT_WORK (&cdev->tx_work, m_can_tx_work_queue); err = request_threaded_irq(dev->irq, NULL, m_can_isr, IRQF_ONESHOT | IRQF_TRIGGER_FALLING,
@@ -1573,20 +1573,20 @@ static int m_can_open(struct net_device *dev)can_led_event (dev,CAN_LED_EVENT_OPEN ); -if (!priv->is_peripherial) -napi_enable (&priv->napi); +if (!cdev->is_peripherial) +napi_enable (&cdev->napi);netif_start_queue (dev);return 0 ; exit_irq_fail: -if (priv->is_peripherial) -destroy_workqueue (priv->tx_wq); +if (cdev->is_peripherial) +destroy_workqueue (cdev->tx_wq); out_wq_fail:close_candev (dev); exit_disable_clks: -m_can_clk_stop (priv); +m_can_clk_stop (cdev);return err; }
@@ -1605,61 +1605,61 @@ static int register_m_can_dev(struct net_device *dev)return register_candev (dev); } -static void m_can_of_parse_mram (struct m_can_priv *priv, +static void m_can_of_parse_mram (struct m_can_classdev *cdev,const u32 *mram_config_vals) { - priv->mcfg[MRAM_SIDF ].off = mram_config_vals[0 ]; - priv->mcfg[MRAM_SIDF ].num = mram_config_vals[1 ]; - priv->mcfg[MRAM_XIDF ].off = priv->mcfg[MRAM_SIDF ].off + - priv->mcfg[MRAM_SIDF ].num *SIDF_ELEMENT_SIZE ; - priv->mcfg[MRAM_XIDF ].num = mram_config_vals[2 ]; - priv->mcfg[MRAM_RXF0 ].off = priv->mcfg[MRAM_XIDF ].off + - priv->mcfg[MRAM_XIDF ].num *XIDF_ELEMENT_SIZE ; - priv->mcfg[MRAM_RXF0 ].num = mram_config_vals[3 ] & + cdev->mcfg[MRAM_SIDF ].off = mram_config_vals[0 ]; + cdev->mcfg[MRAM_SIDF ].num = mram_config_vals[1 ]; + cdev->mcfg[MRAM_XIDF ].off = cdev->mcfg[MRAM_SIDF ].off + + cdev->mcfg[MRAM_SIDF ].num *SIDF_ELEMENT_SIZE ; + cdev->mcfg[MRAM_XIDF ].num = mram_config_vals[2 ]; + cdev->mcfg[MRAM_RXF0 ].off = cdev->mcfg[MRAM_XIDF ].off + + cdev->mcfg[MRAM_XIDF ].num *XIDF_ELEMENT_SIZE ; + cdev->mcfg[MRAM_RXF0 ].num = mram_config_vals[3 ] & (RXFC_FS_MASK >>RXFC_FS_SHIFT ); - priv->mcfg[MRAM_RXF1 ].off = priv->mcfg[MRAM_RXF0 ].off + - priv->mcfg[MRAM_RXF0 ].num *RXF0_ELEMENT_SIZE ; - priv->mcfg[MRAM_RXF1 ].num = mram_config_vals[4 ] & + cdev->mcfg[MRAM_RXF1 ].off = cdev->mcfg[MRAM_RXF0 ].off + + cdev->mcfg[MRAM_RXF0 ].num *RXF0_ELEMENT_SIZE ; + cdev->mcfg[MRAM_RXF1 ].num = mram_config_vals[4 ] & (RXFC_FS_MASK >>RXFC_FS_SHIFT ); - priv->mcfg[MRAM_RXB ].off = priv->mcfg[MRAM_RXF1 ].off + - priv->mcfg[MRAM_RXF1 ].num *RXF1_ELEMENT_SIZE ; - priv->mcfg[MRAM_RXB ].num = mram_config_vals[5 ]; - priv->mcfg[MRAM_TXE ].off = priv->mcfg[MRAM_RXB ].off + - priv->mcfg[MRAM_RXB ].num *RXB_ELEMENT_SIZE ; - priv->mcfg[MRAM_TXE ].num = mram_config_vals[6 ]; - priv->mcfg[MRAM_TXB ].off = priv->mcfg[MRAM_TXE ].off + - priv->mcfg[MRAM_TXE ].num *TXE_ELEMENT_SIZE ; - priv->mcfg[MRAM_TXB ].num = mram_config_vals[7 ] & + cdev->mcfg[MRAM_RXB ].off = cdev->mcfg[MRAM_RXF1 ].off + + cdev->mcfg[MRAM_RXF1 ].num *RXF1_ELEMENT_SIZE ; + cdev->mcfg[MRAM_RXB ].num = mram_config_vals[5 ]; + cdev->mcfg[MRAM_TXE ].off = cdev->mcfg[MRAM_RXB ].off + + cdev->mcfg[MRAM_RXB ].num *RXB_ELEMENT_SIZE ; + cdev->mcfg[MRAM_TXE ].num = mram_config_vals[6 ]; + cdev->mcfg[MRAM_TXB ].off = cdev->mcfg[MRAM_TXE ].off + + cdev->mcfg[MRAM_TXE ].num *TXE_ELEMENT_SIZE ; + cdev->mcfg[MRAM_TXB ].num = mram_config_vals[7 ] & (TXBC_NDTB_MASK >>TXBC_NDTB_SHIFT ); -dev_dbg (priv->dev, +dev_dbg (cdev->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, - priv->mcfg[MRAM_RXF1 ].off, priv->mcfg[MRAM_RXF1 ].num, - 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); + cdev->mcfg[MRAM_SIDF ].off, cdev->mcfg[MRAM_SIDF ].num, + cdev->mcfg[MRAM_XIDF ].off, cdev->mcfg[MRAM_XIDF ].num, + cdev->mcfg[MRAM_RXF0 ].off, cdev->mcfg[MRAM_RXF0 ].num, + cdev->mcfg[MRAM_RXF1 ].off, cdev->mcfg[MRAM_RXF1 ].num, + cdev->mcfg[MRAM_RXB ].off, cdev->mcfg[MRAM_RXB ].num, + cdev->mcfg[MRAM_TXE ].off, cdev->mcfg[MRAM_TXE ].num, + cdev->mcfg[MRAM_TXB ].off, cdev->mcfg[MRAM_TXB ].num); } -void m_can_init_ram (struct m_can_priv *priv) +void m_can_init_ram (struct m_can_classdev *cdev) {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 ; + start = cdev->mcfg[MRAM_SIDF ].off; + end = cdev->mcfg[MRAM_TXB ].off + + cdev->mcfg[MRAM_TXB ].num *TXB_ELEMENT_SIZE ;for (i = start; i < end; i +=4 ) -m_can_fifo_write_no_off (priv, i,0x0 ); +m_can_fifo_write_no_off (cdev, i,0x0 ); }EXPORT_SYMBOL_GPL (m_can_init_ram); -int m_can_class_get_clocks (struct m_can_priv *m_can_dev) +int m_can_class_get_clocks (struct m_can_classdev *m_can_dev) {int ret =0 ;
@@ -1675,9 +1675,9 @@ int m_can_class_get_clocks(struct m_can_priv *m_can_dev) }EXPORT_SYMBOL_GPL (m_can_class_get_clocks); -struct m_can_priv *m_can_class_allocate_dev (struct device *dev) +struct m_can_classdev *m_can_class_allocate_dev (struct device *dev) { -struct m_can_priv *class_dev =NULL ; +struct m_can_classdev *class_dev =NULL ;u32 mram_config_vals[MRAM_CFG_LEN ];struct net_device *net_dev;u32 tx_fifo_size;
@@ -1706,7 +1706,7 @@ struct m_can_priv *m_can_class_allocate_dev(struct device *dev) class_dev =netdev_priv (net_dev);if (!class_dev) { -dev_err (dev,"Failed to init netdev private" ); +dev_err (dev,"Failed to init netdev cdevate" ); goto out; }
@@ -1720,7 +1720,7 @@ struct m_can_priv *m_can_class_allocate_dev(struct device *dev) }EXPORT_SYMBOL_GPL (m_can_class_allocate_dev); -int m_can_class_register (struct m_can_priv *m_can_dev) +int m_can_class_register (struct m_can_classdev *m_can_dev) {int ret;
@@ -1768,18 +1768,18 @@ EXPORT_SYMBOL_GPL(m_can_class_register);int m_can_class_suspend (struct device *dev) {struct net_device *ndev =dev_get_drvdata (dev); -struct m_can_priv *priv =netdev_priv (ndev); +struct m_can_classdev *cdev =netdev_priv (ndev);if (netif_running (ndev)) {netif_stop_queue (ndev);netif_device_detach (ndev);m_can_stop (ndev); -m_can_clk_stop (priv); +m_can_clk_stop (cdev); }pinctrl_pm_select_sleep_state (dev); - priv->can.state =CAN_STATE_SLEEPING ; + cdev->can.state =CAN_STATE_SLEEPING ;return 0 ; }
@@ -1788,20 +1788,20 @@ EXPORT_SYMBOL_GPL(m_can_class_suspend);int m_can_class_resume (struct device *dev) {struct net_device *ndev =dev_get_drvdata (dev); -struct m_can_priv *priv =netdev_priv (ndev); +struct m_can_classdev *cdev =netdev_priv (ndev);pinctrl_pm_select_default_state (dev); - priv->can.state =CAN_STATE_ERROR_ACTIVE ; + cdev->can.state =CAN_STATE_ERROR_ACTIVE ;if (netif_running (ndev)) {int ret; - ret =m_can_clk_start (priv); + ret =m_can_clk_start (cdev);if (ret)return ret; -m_can_init_ram (priv); +m_can_init_ram (cdev);m_can_start (ndev);netif_device_attach (ndev);netif_start_queue (ndev);
@@ -1811,7 +1811,7 @@ int m_can_class_resume(struct device *dev) }EXPORT_SYMBOL_GPL (m_can_class_resume); -void m_can_class_unregister (struct m_can_priv *m_can_dev) +void m_can_class_unregister (struct m_can_classdev *m_can_dev) {unregister_candev (m_can_dev->net);
drivers/net/can/m_can/m_can.h +14 -14
@@ -57,19 +57,19 @@ struct mram_cfg {u8 num; }; -struct m_can_priv ; +struct m_can_classdev ;struct m_can_ops {/* Device specific call backs */ -int (*clear_interrupts)(struct m_can_priv *m_can_class); -u32 (*read_reg)(struct m_can_priv *m_can_class,int reg); -int (*write_reg)(struct m_can_priv *m_can_class,int reg,int val); -u32 (*read_fifo)(struct m_can_priv *m_can_class,int addr_offset); -int (*write_fifo)(struct m_can_priv *m_can_class,int addr_offset, +int (*clear_interrupts)(struct m_can_classdev *m_can_class); +u32 (*read_reg)(struct m_can_classdev *m_can_class,int reg); +int (*write_reg)(struct m_can_classdev *m_can_class,int reg,int val); +u32 (*read_fifo)(struct m_can_classdev *m_can_class,int addr_offset); +int (*write_fifo)(struct m_can_classdev *m_can_class,int addr_offset,int val); -int (*init)(struct m_can_priv *m_can_class); +int (*init)(struct m_can_classdev *m_can_class); }; -struct m_can_priv { +struct m_can_classdev {struct can_priv can;struct napi_struct napi;struct net_device *net;
@@ -98,12 +98,12 @@ struct m_can_priv {struct mram_cfg mcfg[MRAM_CFG_NUM ]; }; -struct m_can_priv *m_can_class_allocate_dev (struct device *dev); -int m_can_class_register (struct m_can_priv *m_can_dev); -void m_can_class_unregister (struct m_can_priv *m_can_dev); -int m_can_class_get_clocks (struct m_can_priv *m_can_dev); -void m_can_init_ram (struct m_can_priv *priv); -void m_can_config_endisable (struct m_can_priv *priv,bool enable); +struct m_can_classdev *m_can_class_allocate_dev (struct device *dev); +int m_can_class_register (struct m_can_classdev *m_can_dev); +void m_can_class_unregister (struct m_can_classdev *m_can_dev); +int m_can_class_get_clocks (struct m_can_classdev *m_can_dev); +void m_can_init_ram (struct m_can_classdev *priv); +void m_can_config_endisable (struct m_can_classdev *priv,bool enable);int m_can_class_suspend (struct device *dev);int m_can_class_resume (struct device *dev);
drivers/net/can/m_can/m_can_platform.c +8 -8
@@ -14,7 +14,7 @@ struct m_can_plat_priv {void __iomem *mram_base; }; -static u32 iomap_read_reg (struct m_can_priv *cdev,int reg) +static u32 iomap_read_reg (struct m_can_classdev *cdev,int reg) {struct m_can_plat_priv *priv = (struct m_can_plat_priv *)cdev->device_data;
@@ -22,7 +22,7 @@ static u32 iomap_read_reg(struct m_can_priv *cdev, int reg)return readl (priv->base + reg); } -static u32 iomap_read_fifo (struct m_can_priv *cdev,int offset) +static u32 iomap_read_fifo (struct m_can_classdev *cdev,int offset) {struct m_can_plat_priv *priv = (struct m_can_plat_priv *)cdev->device_data;
@@ -30,7 +30,7 @@ static u32 iomap_read_fifo(struct m_can_priv *cdev, int offset)return readl (priv->mram_base + offset); } -static int iomap_write_reg (struct m_can_priv *cdev,int reg,int val) +static int iomap_write_reg (struct m_can_classdev *cdev,int reg,int val) {struct m_can_plat_priv *priv = (struct m_can_plat_priv *)cdev->device_data;
@@ -40,7 +40,7 @@ static int iomap_write_reg(struct m_can_priv *cdev, int reg, int val)return 0 ; } -static int iomap_write_fifo (struct m_can_priv *cdev,int offset,int val) +static int iomap_write_fifo (struct m_can_classdev *cdev,int offset,int val) {struct m_can_plat_priv *priv = (struct m_can_plat_priv *)cdev->device_data;
@@ -59,7 +59,7 @@ static struct m_can_ops m_can_plat_ops = {static int m_can_plat_probe (struct platform_device *pdev) { -struct m_can_priv *mcan_class; +struct m_can_classdev *mcan_class;struct m_can_plat_priv *priv;struct resource *res;void __iomem *addr;
@@ -131,7 +131,7 @@ static __maybe_unused int m_can_resume(struct device *dev)static int m_can_plat_remove (struct platform_device *pdev) {struct net_device *dev =platform_get_drvdata (pdev); -struct m_can_priv *mcan_class =netdev_priv (dev); +struct m_can_classdev *mcan_class =netdev_priv (dev);m_can_class_unregister (mcan_class);
@@ -143,7 +143,7 @@ static int m_can_plat_remove(struct platform_device *pdev)static int __maybe_unused m_can_runtime_suspend (struct device *dev) {struct net_device *ndev =dev_get_drvdata (dev); -struct m_can_priv *mcan_class =netdev_priv (ndev); +struct m_can_classdev *mcan_class =netdev_priv (ndev);m_can_class_suspend (dev);
@@ -156,7 +156,7 @@ static int __maybe_unused m_can_runtime_suspend(struct device *dev)static int __maybe_unused m_can_runtime_resume (struct device *dev) {struct net_device *ndev =dev_get_drvdata (dev); -struct m_can_priv *mcan_class =netdev_priv (ndev); +struct m_can_classdev *mcan_class =netdev_priv (ndev);int err; err =clk_prepare_enable (mcan_class->hclk);
Signature · 1 line
2.20.1.390.gb5101f9297
Re: [PATCH v7 2/4] can: m_can: Rename m_can_priv to m_can_classdev
Am 05.03.19 um 16:52 schrieb Dan Murphy:
Quoted 15 lines
Rename the common
m_can_privclass structure tom_can_classdevas this is more descriptive.Signed-off-by Dan Murphy <[email protected]>---
v7 - Fixed remaining checkpatch issues, renamed priv to cdev - https://lore.kernel.org/patchwork/patch/1047219/
v6 - No changes only rebase changes possibly can squash into the first patch -
https://lore.kernel.org/patchwork/patch/1042444/drivers/net/can/m_can/m_can.c | 524 ++++++++++++------------- drivers/net/can/m_can/m_can.h | 28 +- drivers/net/can/m_can/m_can_platform.c | 16 +- 3 files changed, 284 insertions(+), 284 deletions(-)
...snip...
Quoted 12 lines
drivers/net/can/m_can/m_can.h +1
-1@@ -57,19 +57,19 @@ struct mram_cfg {u8 num; }; -struct m_can_priv ; +struct m_can_classdev ;struct m_can_ops {/* Device specific call backs */
drivers/net/can/m_can/m_can.h struct mram_cfg { · line 67
Quoted 10 lines
drivers/net/can/m_can/m_can.h +5
-5-int (*clear_interrupts)(struct m_can_priv *m_can_class); -u32 (*read_reg)(struct m_can_priv *m_can_class,int reg); -int (*write_reg)(struct m_can_priv *m_can_class,int reg,int val); -u32 (*read_fifo)(struct m_can_priv *m_can_class,int addr_offset); - int (*write_fifo)(struct m_can_priv *m_can_class, int addr_offset, +int (*clear_interrupts)(struct m_can_classdev *m_can_class); +u32 (*read_reg)(struct m_can_classdev *m_can_class,int reg); +int (*write_reg)(struct m_can_classdev *m_can_class,int reg,int val); +u32 (*read_fifo)(struct m_can_classdev *m_can_class,int addr_offset); + int (*write_fifo)(struct m_can_classdev *m_can_class, int addr_offset,
Could you please replace the variable name "m_can_class" with "cdev" here and below and in "c_can_platform.c " and "tcan4x5x.c" as well.
Thanks,
Wolfgang.
Re: [PATCH v7 2/4] can: m_can: Rename m_can_priv to m_can_classdev
Wolfgang
Quoted 38 lines · Wolfgang Grandegger
Am 05.03.19 um 16:52 schrieb Dan Murphy:
Quoted 15 lines
Rename the common
m_can_privclass structure tom_can_classdevas this is more descriptive.Signed-off-by Dan Murphy <[email protected]>---
v7 - Fixed remaining checkpatch issues, renamed priv to cdev - https://lore.kernel.org/patchwork/patch/1047219/
v6 - No changes only rebase changes possibly can squash into the first patch -
https://lore.kernel.org/patchwork/patch/1042444/drivers/net/can/m_can/m_can.c | 524 ++++++++++++------------- drivers/net/can/m_can/m_can.h | 28 +- drivers/net/can/m_can/m_can_platform.c | 16 +- 3 files changed, 284 insertions(+), 284 deletions(-)...snip...
Quoted 17 lines
drivers/net/can/m_can/m_can.h +1
-6@@ -57,19 +57,19 @@ struct mram_cfg {u8 num; }; -struct m_can_priv ; +struct m_can_classdev ;struct m_can_ops {/* Device specific call backs */ -int (*clear_interrupts)(struct m_can_priv *m_can_class); -u32 (*read_reg)(struct m_can_priv *m_can_class,int reg); -int (*write_reg)(struct m_can_priv *m_can_class,int reg,int val); -u32 (*read_fifo)(struct m_can_priv *m_can_class,int addr_offset); - int (*write_fifo)(struct m_can_priv *m_can_class, int addr_offset,
Quoted 9 lines
drivers/net/can/m_can/m_can.h struct mram_cfg { · line 67
Quoted 5 lines
drivers/net/can/m_can/m_can.h +5
-0+int (*clear_interrupts)(struct m_can_classdev *m_can_class); +u32 (*read_reg)(struct m_can_classdev *m_can_class,int reg); +int (*write_reg)(struct m_can_classdev *m_can_class,int reg,int val); +u32 (*read_fifo)(struct m_can_classdev *m_can_class,int addr_offset); + int (*write_fifo)(struct m_can_classdev *m_can_class, int addr_offset,Could you please replace the variable name "
m_can_class" with "cdev" here and below and in "c_can_platform.c" and "tcan4x5x.c" as well.
OK I will update the header
The tcan and m_can_platform source files are converted to cdev as opposed to m_can_class variable.
Dan
Quoted 4 lines
Thanks,
Wolfgang.
Signature · 2 lines
------------------ Dan Murphy
PATCH v7 3/4 dt-bindings: can: tcan4x5x: Add DT bindings for TCAN4x5X driver
In reply to Dan MurphyDT binding documentation for TI TCAN4x5x driver.
---
v7 - Made device state optional - https://lore.kernel.org/patchwork/patch/1047218/
v6 - No changes - https://lore.kernel.org/patchwork/patch/1042445/
.../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. + +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. + - device-state-gpios: Input GPIO that indicates if the device is in + a sleep state or if the device is active. + +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.390.gb5101f9297
PATCH v7 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. Register to 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.
---
v7 - Fixed remaining checkpatch issues and renamed callbacks for m_can change -
https://lore.kernel.org/patchwork/patch/1047221/
v6 - Updated Kconfig help and added REGMAP_SPI dependency, fixed coding style issues, removed setting GPIO high in wake routine, change -EIO to return ret in init and setup functions - https://lore.kernel.org/patchwork/patch/1042443/
drivers/net/can/m_can/Kconfig | 9 + drivers/net/can/m_can/Makefile | 1 + drivers/net/can/m_can/tcan4x5x.c | 533 +++++++++++++++++++++++++++++++ 3 files changed, 543 insertions(+) create mode 100644 drivers/net/can/m_can/tcan4x5x.c
drivers/net/can/m_can/Kconfig +9 -0
@@ -12,3 +12,12 @@ config CAN_M_CAN_PLATFORM Say Y here if you want support for IO Mapped Bosch M_CAN controller. This support is for devices that have the Bosch M_CAN controller IP embedded into the device and the IP is IO Mapped to the processor. + +config CAN_M_CAN_TCAN4X5X + depends on CAN_M_CAN + depends on REGMAP_SPI + tristate "TCAN4X5X M_CAN device" + ---help--- + Say Y here if you want support for Texas Instruments TCAN4x5x + M_CAN controller. This device is a peripherial device that uses the + SPI bus for communication.
drivers/net/can/m_can/Makefile +1 -0
@@ -4,3 +4,4 @@ obj-$(CONFIG_CAN_M_CAN) += 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/tcan4x5x.c new file +533 -0
533 lines not on this page · raw message
Signature · 1 line
2.20.1.390.gb5101f9297
Hallo Dan,
Am 05.03.19 um 16:52 schrieb Dan Murphy:
Quoted 61 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]>---
v7 - Fixed remaining new checkpatch issues, removed CSR setting, fixed tx hard
start function to returntx_busy, and renamed device callbacks - https://lore.kernel.org/patchwork/patch/1047220/v6 - Squashed platform patch to this patch for bissectablity, fixed coding style issues, updated
Kconfighelp, placed mcan reg offsets back into c file, renamedpriv->skbtopriv->tx_skband cleared perp interrupts at ISR start - Patch 1 comments - https://lore.kernel.org/patchwork/patch/1042446/
Patch 2 comments - https://lore.kernel.org/patchwork/patch/1042442/drivers/net/can/m_can/Kconfig | 13 +- drivers/net/can/m_can/Makefile | 1 + drivers/net/can/m_can/m_can.c | 700 +++++++++++++------------ drivers/net/can/m_can/m_can.h | 110 ++++ drivers/net/can/m_can/m_can_platform.c | 202 +++++++ 5 files changed, 682 insertions(+), 344 deletions(-) create mode 100644 drivers/net/can/m_can/m_can.h create mode 100644 drivers/net/can/m_can/m_can_platform.cdrivers/net/can/m_can/Kconfig +11
-2@@ -1,5 +1,14 @@ config CAN_M_CAN + tristate "Bosch M_CAN support" + ---help--- + Say Y here if you want support for Bosch M_CAN controller framework. + This is common support for devices that embed the Bosch M_CAN IP. + +config CAN_M_CAN_PLATFORM + tristate "Bosch M_CAN support for io-mapped devices" depends on HAS_IOMEM - tristate "Bosch M_CAN devices" + depends on CAN_M_CAN ---help--- - Say Y here if you want to support for Bosch M_CAN controller. + Say Y here if you want support for IO Mapped Bosch M_CAN controller. + This support is for devices that have the Bosch M_CAN controller + IP embedded into the device and the IP is IO Mapped to the processor.drivers/net/can/m_can/Makefile +1
-0@@ -3,3 +3,4 @@ # obj-$(CONFIG_CAN_M_CAN) += m_can.o +obj-$(CONFIG_CAN_M_CAN_PLATFORM) += m_can_platform.odrivers/net/can/m_can/m_can.c +0
-0
... snip...
drivers/net/can/m_can/m_can.c static netdev_tx_t m_can_start_xmit(struct sk_buff *skb, · line 1517
Quoted 13 lines
drivers/net/can/m_can/m_can.c +13
-0+static netdev_tx_t m_can_start_xmit (struct sk_buff *skb, +struct net_device *dev) +{ +struct m_can_priv *priv =netdev_priv (dev); + +if (can_dropped_invalid_skb (dev, skb)) +return NETDEV_TX_OK ; + +if (priv->is_peripherial) { +if (priv->tx_skb) { +netdev_err (dev,"hard_xmit called while tx busy\n" ); +return NETDEV_TX_BUSY ; + }
The problem with that approach is, that the upper layer will try to resubmit the current "skb" but not the previous "tx_skb". And the previous "tx_skb" has not been freed yet. I would just drop and free the skb and return NETDEV_TX_OK in m_can_tx_handler() for peripheral devices (like can_dropped_invalid_skb() does).
drivers/net/can/m_can/m_can.c static netdev_tx_t m_can_start_xmit(struct sk_buff *skb, · line 1528
Quoted 11 lines
drivers/net/can/m_can/m_can.c +7
-0+ + priv->tx_skb = skb; +netif_stop_queue (priv->net); +queue_work (priv->tx_wq, &priv->tx_work); + }else { + priv->tx_skb = skb; +return m_can_tx_handler (priv); }return NETDEV_TX_OK ; }
Wolfgang.
Wolfgang
Quoted 78 lines · Wolfgang Grandegger
Hallo Dan,
Am 05.03.19 um 16:52 schrieb Dan Murphy:
Quoted 61 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]>---
v7 - Fixed remaining new checkpatch issues, removed CSR setting, fixed tx hard
start function to returntx_busy, and renamed device callbacks - https://lore.kernel.org/patchwork/patch/1047220/v6 - Squashed platform patch to this patch for bissectablity, fixed coding style issues, updated
Kconfighelp, placed mcan reg offsets back into c file, renamedpriv->skbtopriv->tx_skband cleared perp interrupts at ISR start - Patch 1 comments - https://lore.kernel.org/patchwork/patch/1042446/
Patch 2 comments - https://lore.kernel.org/patchwork/patch/1042442/drivers/net/can/m_can/Kconfig | 13 +- drivers/net/can/m_can/Makefile | 1 + drivers/net/can/m_can/m_can.c | 700 +++++++++++++------------ drivers/net/can/m_can/m_can.h | 110 ++++ drivers/net/can/m_can/m_can_platform.c | 202 +++++++ 5 files changed, 682 insertions(+), 344 deletions(-) create mode 100644 drivers/net/can/m_can/m_can.h create mode 100644 drivers/net/can/m_can/m_can_platform.cdrivers/net/can/m_can/Kconfig +11
-2@@ -1,5 +1,14 @@ config CAN_M_CAN + tristate "Bosch M_CAN support" + ---help--- + Say Y here if you want support for Bosch M_CAN controller framework. + This is common support for devices that embed the Bosch M_CAN IP. + +config CAN_M_CAN_PLATFORM + tristate "Bosch M_CAN support for io-mapped devices" depends on HAS_IOMEM - tristate "Bosch M_CAN devices" + depends on CAN_M_CAN ---help--- - Say Y here if you want to support for Bosch M_CAN controller. + Say Y here if you want support for IO Mapped Bosch M_CAN controller. + This support is for devices that have the Bosch M_CAN controller + IP embedded into the device and the IP is IO Mapped to the processor.drivers/net/can/m_can/Makefile +1
-0@@ -3,3 +3,4 @@ # obj-$(CONFIG_CAN_M_CAN) += m_can.o +obj-$(CONFIG_CAN_M_CAN_PLATFORM) += m_can_platform.odrivers/net/can/m_can/m_can.c +0
-0... snip...
Quoted 11 lines
drivers/net/can/m_can/m_can.c +11
-0+static netdev_tx_t m_can_start_xmit (struct sk_buff *skb, +struct net_device *dev) +{ +struct m_can_priv *priv =netdev_priv (dev); + +if (can_dropped_invalid_skb (dev, skb)) +return NETDEV_TX_OK ; + +if (priv->is_peripherial) { +if (priv->tx_skb) { +netdev_err (dev,"hard_xmit called while tx busy\n" );
Quoted 9 lines
drivers/net/can/m_can/m_can.c
Quoted 2 lines
drivers/net/can/m_can/m_can.c +2
-0+return NETDEV_TX_BUSY ; + }The problem with that approach is, that the upper layer will try to resubmit the current "skb" but not the previous "
tx_skb". And the previous "tx_skb" has not been freed yet. I would just drop and free the skb and returnNETDEV_TX_OKinm_can_tx_handler()for peripheral devices (likecan_dropped_invalid_skb()does).
OK.
So would this also be a bug in the hi3110 and mcp251x drivers (line 521) as well because besides checking tx_length this is how these drivers are written.
In addition in the peripheral context the work queue does not report up to the upper layer the status. Again the hi3110 and mcp251x drivers are written this way.
The only issue I see here is that the dropped and invalid check needs to come after the tx_skb check.
Dan
Quoted 14 lines
drivers/net/can/m_can/m_can.c static netdev_tx_t m_can_start_xmit(struct sk_buff *skb, · line 1528
Quoted 11 lines
drivers/net/can/m_can/m_can.c +7
-0+ + priv->tx_skb = skb; +netif_stop_queue (priv->net); +queue_work (priv->tx_wq, &priv->tx_work); + }else { + priv->tx_skb = skb; +return m_can_tx_handler (priv); }return NETDEV_TX_OK ; }Wolfgang.
Signature · 2 lines
------------------ Dan Murphy
Hello Dan,
Am 08.03.19 um 13:44 schrieb Dan Murphy:
Quoted 91 lines
Wolfgang
Quoted 78 lines · Wolfgang Grandegger
Hallo Dan,
Am 05.03.19 um 16:52 schrieb Dan Murphy:
Quoted 61 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]>---
v7 - Fixed remaining new checkpatch issues, removed CSR setting, fixed tx hard
start function to returntx_busy, and renamed device callbacks - https://lore.kernel.org/patchwork/patch/1047220/v6 - Squashed platform patch to this patch for bissectablity, fixed coding style issues, updated
Kconfighelp, placed mcan reg offsets back into c file, renamedpriv->skbtopriv->tx_skband cleared perp interrupts at ISR start - Patch 1 comments - https://lore.kernel.org/patchwork/patch/1042446/
Patch 2 comments - https://lore.kernel.org/patchwork/patch/1042442/drivers/net/can/m_can/Kconfig | 13 +- drivers/net/can/m_can/Makefile | 1 + drivers/net/can/m_can/m_can.c | 700 +++++++++++++------------ drivers/net/can/m_can/m_can.h | 110 ++++ drivers/net/can/m_can/m_can_platform.c | 202 +++++++ 5 files changed, 682 insertions(+), 344 deletions(-) create mode 100644 drivers/net/can/m_can/m_can.h create mode 100644 drivers/net/can/m_can/m_can_platform.cdrivers/net/can/m_can/Kconfig +11
-2@@ -1,5 +1,14 @@ config CAN_M_CAN + tristate "Bosch M_CAN support" + ---help--- + Say Y here if you want support for Bosch M_CAN controller framework. + This is common support for devices that embed the Bosch M_CAN IP. + +config CAN_M_CAN_PLATFORM + tristate "Bosch M_CAN support for io-mapped devices" depends on HAS_IOMEM - tristate "Bosch M_CAN devices" + depends on CAN_M_CAN ---help--- - Say Y here if you want to support for Bosch M_CAN controller. + Say Y here if you want support for IO Mapped Bosch M_CAN controller. + This support is for devices that have the Bosch M_CAN controller + IP embedded into the device and the IP is IO Mapped to the processor.drivers/net/can/m_can/Makefile +1
-0@@ -3,3 +3,4 @@ # obj-$(CONFIG_CAN_M_CAN) += m_can.o +obj-$(CONFIG_CAN_M_CAN_PLATFORM) += m_can_platform.odrivers/net/can/m_can/m_can.c +0
-0... snip...
Quoted 11 lines
drivers/net/can/m_can/m_can.c +11
-0+static netdev_tx_t m_can_start_xmit (struct sk_buff *skb, +struct net_device *dev) +{ +struct m_can_priv *priv =netdev_priv (dev); + +if (can_dropped_invalid_skb (dev, skb)) +return NETDEV_TX_OK ; + +if (priv->is_peripherial) { +if (priv->tx_skb) { +netdev_err (dev,"hard_xmit called while tx busy\n" );Quoted 9 lines
drivers/net/can/m_can/m_can.c
Quoted 2 lines
drivers/net/can/m_can/m_can.c +2
-0+return NETDEV_TX_BUSY ; + }The problem with that approach is, that the upper layer will try to resubmit the current "skb" but not the previous "
tx_skb". And the previous "tx_skb" has not been freed yet. I would just drop and free the skb and returnNETDEV_TX_OKinm_can_tx_handler()for peripheral devices (likecan_dropped_invalid_skb()does).
Quoted 4 lines
OK.
So would this also be a bug in the hi3110 and mcp251x drivers (line 521) as well because besides checking
tx_lengththis is how these drivers are written.
This is different. When entering the "start_xmit" routine, the previous TX is still in progress. It will (hopefully) complete soon. Therefore returning NETDEV_TX_BUSY is OK. The "start_xmit" routine will be recalled soon with the same "skb". That scenario should/could also not happen.
In contrast, in "m_can_tx_handler()", the skb could not be handled because the FIFO is full. The "start_xmit" routine for peripheral devices for that skb already returned NETDEV_TX_OK. Therefore the only meaningful action is to drop the skb. Also this error should not happen and if, something is going really wrong. Therefore I think, a WARN_ONCE() would be even more appropriate. But that should be a separate patch.
Quoted 5 lines
In addition in the peripheral context the work queue does not report up to the upper layer the status. Again the hi3110 and mcp251x drivers are written this way.
The only issue I see here is that the dropped and invalid check needs to come after the
tx_skbcheck.
See above.
Wolfgang.
Hello Dan,
thinking more about it...
Am 08.03.19 um 14:29 schrieb Wolfgang Grandegger:
Quoted 96 lines
Hello Dan,
Am 08.03.19 um 13:44 schrieb Dan Murphy:
Quoted 93 lines
Wolfgang
Quoted 78 lines · Wolfgang Grandegger
Hallo Dan,
Am 05.03.19 um 16:52 schrieb Dan Murphy:
Quoted 61 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]>---
v7 - Fixed remaining new checkpatch issues, removed CSR setting, fixed tx hard
start function to returntx_busy, and renamed device callbacks - https://lore.kernel.org/patchwork/patch/1047220/v6 - Squashed platform patch to this patch for bissectablity, fixed coding style issues, updated
Kconfighelp, placed mcan reg offsets back into c file, renamedpriv->skbtopriv->tx_skband cleared perp interrupts at ISR start - Patch 1 comments - https://lore.kernel.org/patchwork/patch/1042446/
Patch 2 comments - https://lore.kernel.org/patchwork/patch/1042442/drivers/net/can/m_can/Kconfig | 13 +- drivers/net/can/m_can/Makefile | 1 + drivers/net/can/m_can/m_can.c | 700 +++++++++++++------------ drivers/net/can/m_can/m_can.h | 110 ++++ drivers/net/can/m_can/m_can_platform.c | 202 +++++++ 5 files changed, 682 insertions(+), 344 deletions(-) create mode 100644 drivers/net/can/m_can/m_can.h create mode 100644 drivers/net/can/m_can/m_can_platform.cdrivers/net/can/m_can/Kconfig +11
-2@@ -1,5 +1,14 @@ config CAN_M_CAN + tristate "Bosch M_CAN support" + ---help--- + Say Y here if you want support for Bosch M_CAN controller framework. + This is common support for devices that embed the Bosch M_CAN IP. + +config CAN_M_CAN_PLATFORM + tristate "Bosch M_CAN support for io-mapped devices" depends on HAS_IOMEM - tristate "Bosch M_CAN devices" + depends on CAN_M_CAN ---help--- - Say Y here if you want to support for Bosch M_CAN controller. + Say Y here if you want support for IO Mapped Bosch M_CAN controller. + This support is for devices that have the Bosch M_CAN controller + IP embedded into the device and the IP is IO Mapped to the processor.drivers/net/can/m_can/Makefile +1
-0@@ -3,3 +3,4 @@ # obj-$(CONFIG_CAN_M_CAN) += m_can.o +obj-$(CONFIG_CAN_M_CAN_PLATFORM) += m_can_platform.odrivers/net/can/m_can/m_can.c +0
-0... snip...
Quoted 11 lines
drivers/net/can/m_can/m_can.c +11
-0+static netdev_tx_t m_can_start_xmit (struct sk_buff *skb, +struct net_device *dev) +{ +struct m_can_priv *priv =netdev_priv (dev); + +if (can_dropped_invalid_skb (dev, skb)) +return NETDEV_TX_OK ; + +if (priv->is_peripherial) { +if (priv->tx_skb) { +netdev_err (dev,"hard_xmit called while tx busy\n" );Quoted 9 lines
drivers/net/can/m_can/m_can.c
Quoted 2 lines
drivers/net/can/m_can/m_can.c +2
-0+return NETDEV_TX_BUSY ; + }The problem with that approach is, that the upper layer will try to resubmit the current "skb" but not the previous "
tx_skb". And the previous "tx_skb" has not been freed yet. I would just drop and free the skb and returnNETDEV_TX_OKinm_can_tx_handler()for peripheral devices (likecan_dropped_invalid_skb()does).OK.
Quoted 8 lines
Quoted 2 lines
So would this also be a bug in the hi3110 and mcp251x drivers (line 521) as well because besides checking
tx_lengththis is how these drivers are written.This is different. When entering the "
start_xmit" routine, the previous TX is still in progress. It will (hopefully) complete soon. Therefore returningNETDEV_TX_BUSYis OK. The "start_xmit" routine will be recalled soon with the same "skb". That scenario should/could also not happen.
In principle, this also applies to the m_can peripheral devices. If tx_skb is not NULL, the TX is still in progress and returning NETDEV_TX_BUSY is just fine.
Quoted 8 lines
In contrast, in "
m_can_tx_handler()", the skb could not be handled because the FIFO is full. The "start_xmit" routine for peripheral devices for that skb already returnedNETDEV_TX_OK. Therefore the only meaningful action is to drop the skb. Also this error should not happen and if, something is going really wrong. Therefore I think, aWARN_ONCE()would be even more appropriate. But that should be a separate patch.
But that's a different issue/error. The tx_skb cannot be processed in "m_can_tx_handler()". Either we drop it or we re-queue it (retry later).
Quoted 7 lines
Quoted 5 lines
In addition in the peripheral context the work queue does not report up to the upper layer the status. Again the hi3110 and mcp251x drivers are written this way.
The only issue I see here is that the dropped and invalid check needs to come after the
tx_skbcheck.See above.
Wolfgang.
Wolfgang
Quoted 123 lines · Wolfgang Grandegger
Hello Dan,
thinking more about it...
Am 08.03.19 um 14:29 schrieb Wolfgang Grandegger:
Quoted 96 lines
Hello Dan,
Am 08.03.19 um 13:44 schrieb Dan Murphy:
Quoted 93 lines
Wolfgang
Quoted 78 lines · Wolfgang Grandegger
Hallo Dan,
Am 05.03.19 um 16:52 schrieb Dan Murphy:
Quoted 61 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]>---
v7 - Fixed remaining new checkpatch issues, removed CSR setting, fixed tx hard
start function to returntx_busy, and renamed device callbacks - https://lore.kernel.org/patchwork/patch/1047220/v6 - Squashed platform patch to this patch for bissectablity, fixed coding style issues, updated
Kconfighelp, placed mcan reg offsets back into c file, renamedpriv->skbtopriv->tx_skband cleared perp interrupts at ISR start - Patch 1 comments - https://lore.kernel.org/patchwork/patch/1042446/
Patch 2 comments - https://lore.kernel.org/patchwork/patch/1042442/drivers/net/can/m_can/Kconfig | 13 +- drivers/net/can/m_can/Makefile | 1 + drivers/net/can/m_can/m_can.c | 700 +++++++++++++------------ drivers/net/can/m_can/m_can.h | 110 ++++ drivers/net/can/m_can/m_can_platform.c | 202 +++++++ 5 files changed, 682 insertions(+), 344 deletions(-) create mode 100644 drivers/net/can/m_can/m_can.h create mode 100644 drivers/net/can/m_can/m_can_platform.cdrivers/net/can/m_can/Kconfig +11
-2@@ -1,5 +1,14 @@ config CAN_M_CAN + tristate "Bosch M_CAN support" + ---help--- + Say Y here if you want support for Bosch M_CAN controller framework. + This is common support for devices that embed the Bosch M_CAN IP. + +config CAN_M_CAN_PLATFORM + tristate "Bosch M_CAN support for io-mapped devices" depends on HAS_IOMEM - tristate "Bosch M_CAN devices" + depends on CAN_M_CAN ---help--- - Say Y here if you want to support for Bosch M_CAN controller. + Say Y here if you want support for IO Mapped Bosch M_CAN controller. + This support is for devices that have the Bosch M_CAN controller + IP embedded into the device and the IP is IO Mapped to the processor.drivers/net/can/m_can/Makefile +1
-0@@ -3,3 +3,4 @@ # obj-$(CONFIG_CAN_M_CAN) += m_can.o +obj-$(CONFIG_CAN_M_CAN_PLATFORM) += m_can_platform.odrivers/net/can/m_can/m_can.c +0
-0... snip...
Quoted 11 lines
drivers/net/can/m_can/m_can.c +11
-0+static netdev_tx_t m_can_start_xmit (struct sk_buff *skb, +struct net_device *dev) +{ +struct m_can_priv *priv =netdev_priv (dev); + +if (can_dropped_invalid_skb (dev, skb)) +return NETDEV_TX_OK ; + +if (priv->is_peripherial) { +if (priv->tx_skb) { +netdev_err (dev,"hard_xmit called while tx busy\n" );Quoted 9 lines
drivers/net/can/m_can/m_can.c
Quoted 2 lines
drivers/net/can/m_can/m_can.c +2
-0+return NETDEV_TX_BUSY ; + }The problem with that approach is, that the upper layer will try to resubmit the current "skb" but not the previous "
tx_skb". And the previous "tx_skb" has not been freed yet. I would just drop and free the skb and returnNETDEV_TX_OKinm_can_tx_handler()for peripheral devices (likecan_dropped_invalid_skb()does).OK.
Quoted 8 lines
Quoted 2 lines
So would this also be a bug in the hi3110 and mcp251x drivers (line 521) as well because besides checking
tx_lengththis is how these drivers are written.This is different. When entering the "
start_xmit" routine, the previous TX is still in progress. It will (hopefully) complete soon. Therefore returningNETDEV_TX_BUSYis OK. The "start_xmit" routine will be recalled soon with the same "skb". That scenario should/could also not happen.In principle, this also applies to the
m_canperipheral devices. Iftx_skbis not NULL, the TX is still in progress and returningNETDEV_TX_BUSYis just fine.Quoted 8 lines
In contrast, in "
m_can_tx_handler()", the skb could not be handled because the FIFO is full. The "start_xmit" routine for peripheral devices for that skb already returnedNETDEV_TX_OK. Therefore the only meaningful action is to drop the skb. Also this error should not happen and if, something is going really wrong. Therefore I think, aWARN_ONCE()would be even more appropriate. But that should be a separate patch.
Quoted 3 lines
But that's a different issue/error. The
tx_skbcannot be processed in "m_can_tx_handler()". Either we drop it or we re-queue it (retry later).
OK I am a bit confused on this. Are you saying this is not an issue? Or are you saying I need to check for tx_len like the other code?
Again if this code is an issue here I believe this is an issue in the hi3110 and mcp251x
Dan
Quoted 10 lines
Quoted 7 lines
Quoted 5 lines
In addition in the peripheral context the work queue does not report up to the upper layer the status. Again the hi3110 and mcp251x drivers are written this way.
The only issue I see here is that the dropped and invalid check needs to come after the
tx_skbcheck.See above.
Wolfgang.
Signature · 2 lines
------------------ Dan Murphy
Hello,
Am 08.03.19 um 16:48 schrieb Dan Murphy:
Quoted 126 lines
Wolfgang
Quoted 123 lines · Wolfgang Grandegger
Hello Dan,
thinking more about it...
Am 08.03.19 um 14:29 schrieb Wolfgang Grandegger:
Quoted 96 lines
Hello Dan,
Am 08.03.19 um 13:44 schrieb Dan Murphy:
Quoted 93 lines
Wolfgang
Quoted 78 lines · Wolfgang Grandegger
Hallo Dan,
Am 05.03.19 um 16:52 schrieb Dan Murphy:
Quoted 61 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]>---
v7 - Fixed remaining new checkpatch issues, removed CSR setting, fixed tx hard
start function to returntx_busy, and renamed device callbacks - https://lore.kernel.org/patchwork/patch/1047220/v6 - Squashed platform patch to this patch for bissectablity, fixed coding style issues, updated
Kconfighelp, placed mcan reg offsets back into c file, renamedpriv->skbtopriv->tx_skband cleared perp interrupts at ISR start - Patch 1 comments - https://lore.kernel.org/patchwork/patch/1042446/
Patch 2 comments - https://lore.kernel.org/patchwork/patch/1042442/drivers/net/can/m_can/Kconfig | 13 +- drivers/net/can/m_can/Makefile | 1 + drivers/net/can/m_can/m_can.c | 700 +++++++++++++------------ drivers/net/can/m_can/m_can.h | 110 ++++ drivers/net/can/m_can/m_can_platform.c | 202 +++++++ 5 files changed, 682 insertions(+), 344 deletions(-) create mode 100644 drivers/net/can/m_can/m_can.h create mode 100644 drivers/net/can/m_can/m_can_platform.cdrivers/net/can/m_can/Kconfig +11
-2@@ -1,5 +1,14 @@ config CAN_M_CAN + tristate "Bosch M_CAN support" + ---help--- + Say Y here if you want support for Bosch M_CAN controller framework. + This is common support for devices that embed the Bosch M_CAN IP. + +config CAN_M_CAN_PLATFORM + tristate "Bosch M_CAN support for io-mapped devices" depends on HAS_IOMEM - tristate "Bosch M_CAN devices" + depends on CAN_M_CAN ---help--- - Say Y here if you want to support for Bosch M_CAN controller. + Say Y here if you want support for IO Mapped Bosch M_CAN controller. + This support is for devices that have the Bosch M_CAN controller + IP embedded into the device and the IP is IO Mapped to the processor.drivers/net/can/m_can/Makefile +1
-0@@ -3,3 +3,4 @@ # obj-$(CONFIG_CAN_M_CAN) += m_can.o +obj-$(CONFIG_CAN_M_CAN_PLATFORM) += m_can_platform.odrivers/net/can/m_can/m_can.c +0
-0... snip...
Quoted 11 lines
drivers/net/can/m_can/m_can.c +11
-0+static netdev_tx_t m_can_start_xmit (struct sk_buff *skb, +struct net_device *dev) +{ +struct m_can_priv *priv =netdev_priv (dev); + +if (can_dropped_invalid_skb (dev, skb)) +return NETDEV_TX_OK ; + +if (priv->is_peripherial) { +if (priv->tx_skb) { +netdev_err (dev,"hard_xmit called while tx busy\n" );Quoted 9 lines
drivers/net/can/m_can/m_can.c
Quoted 2 lines
drivers/net/can/m_can/m_can.c +2
-0+return NETDEV_TX_BUSY ; + }The problem with that approach is, that the upper layer will try to resubmit the current "skb" but not the previous "
tx_skb". And the previous "tx_skb" has not been freed yet. I would just drop and free the skb and returnNETDEV_TX_OKinm_can_tx_handler()for peripheral devices (likecan_dropped_invalid_skb()does).OK.
Quoted 8 lines
Quoted 2 lines
So would this also be a bug in the hi3110 and mcp251x drivers (line 521) as well because besides checking
tx_lengththis is how these drivers are written.This is different. When entering the "
start_xmit" routine, the previous TX is still in progress. It will (hopefully) complete soon. Therefore returningNETDEV_TX_BUSYis OK. The "start_xmit" routine will be recalled soon with the same "skb". That scenario should/could also not happen.In principle, this also applies to the
m_canperipheral devices. Iftx_skbis not NULL, the TX is still in progress and returningNETDEV_TX_BUSYis just fine.Quoted 8 lines
In contrast, in "
m_can_tx_handler()", the skb could not be handled because the FIFO is full. The "start_xmit" routine for peripheral devices for that skb already returnedNETDEV_TX_OK. Therefore the only meaningful action is to drop the skb. Also this error should not happen and if, something is going really wrong. Therefore I think, aWARN_ONCE()would be even more appropriate. But that should be a separate patch.
Quoted 6 lines
Quoted 3 lines
But that's a different issue/error. The
tx_skbcannot be processed in "m_can_tx_handler()". Either we drop it or we re-queue it (retry later).OK I am a bit confused on this. Are you saying this is not an issue? Or are you saying I need to check for
tx_lenlike the other code?
If you check for tx_skb in the "start_xmit" routine like the hi3110 and mcp251x, it will work the same way. But only, if the "tx_handler()" has fully processed the message. It simple means, the TX is still in progress and will complete soon. But in "m_can_tx_handler()" we return without handling the message! It will never be sent and freed. Or will the "m_can_tx_handler()" retry?
Quoted 1 line
Again if this code is an issue here I believe this is an issue in the hi3110 and mcp251x
I don't think so.
Sorry for confusion.
Wolfgang.
Quoted 136 lines · Wolfgang Grandegger
Hello,
Am 08.03.19 um 16:48 schrieb Dan Murphy:
Quoted 126 lines
Wolfgang
Quoted 123 lines · Wolfgang Grandegger
Hello Dan,
thinking more about it...
Am 08.03.19 um 14:29 schrieb Wolfgang Grandegger:
Quoted 96 lines
Hello Dan,
Am 08.03.19 um 13:44 schrieb Dan Murphy:
Quoted 93 lines
Wolfgang
Quoted 78 lines · Wolfgang Grandegger
Hallo Dan,
Am 05.03.19 um 16:52 schrieb Dan Murphy:
Quoted 61 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]>---
v7 - Fixed remaining new checkpatch issues, removed CSR setting, fixed tx hard
start function to returntx_busy, and renamed device callbacks - https://lore.kernel.org/patchwork/patch/1047220/v6 - Squashed platform patch to this patch for bissectablity, fixed coding style issues, updated
Kconfighelp, placed mcan reg offsets back into c file, renamedpriv->skbtopriv->tx_skband cleared perp interrupts at ISR start - Patch 1 comments - https://lore.kernel.org/patchwork/patch/1042446/
Patch 2 comments - https://lore.kernel.org/patchwork/patch/1042442/drivers/net/can/m_can/Kconfig | 13 +- drivers/net/can/m_can/Makefile | 1 + drivers/net/can/m_can/m_can.c | 700 +++++++++++++------------ drivers/net/can/m_can/m_can.h | 110 ++++ drivers/net/can/m_can/m_can_platform.c | 202 +++++++ 5 files changed, 682 insertions(+), 344 deletions(-) create mode 100644 drivers/net/can/m_can/m_can.h create mode 100644 drivers/net/can/m_can/m_can_platform.cdrivers/net/can/m_can/Kconfig +11
-2@@ -1,5 +1,14 @@ config CAN_M_CAN + tristate "Bosch M_CAN support" + ---help--- + Say Y here if you want support for Bosch M_CAN controller framework. + This is common support for devices that embed the Bosch M_CAN IP. + +config CAN_M_CAN_PLATFORM + tristate "Bosch M_CAN support for io-mapped devices" depends on HAS_IOMEM - tristate "Bosch M_CAN devices" + depends on CAN_M_CAN ---help--- - Say Y here if you want to support for Bosch M_CAN controller. + Say Y here if you want support for IO Mapped Bosch M_CAN controller. + This support is for devices that have the Bosch M_CAN controller + IP embedded into the device and the IP is IO Mapped to the processor.drivers/net/can/m_can/Makefile +1
-0@@ -3,3 +3,4 @@ # obj-$(CONFIG_CAN_M_CAN) += m_can.o +obj-$(CONFIG_CAN_M_CAN_PLATFORM) += m_can_platform.odrivers/net/can/m_can/m_can.c +0
-0... snip...
Quoted 11 lines
drivers/net/can/m_can/m_can.c +11
-0+static netdev_tx_t m_can_start_xmit (struct sk_buff *skb, +struct net_device *dev) +{ +struct m_can_priv *priv =netdev_priv (dev); + +if (can_dropped_invalid_skb (dev, skb)) +return NETDEV_TX_OK ; + +if (priv->is_peripherial) { +if (priv->tx_skb) { +netdev_err (dev,"hard_xmit called while tx busy\n" );Quoted 9 lines
drivers/net/can/m_can/m_can.c
Quoted 2 lines
drivers/net/can/m_can/m_can.c +2
-0+return NETDEV_TX_BUSY ; + }The problem with that approach is, that the upper layer will try to resubmit the current "skb" but not the previous "
tx_skb". And the previous "tx_skb" has not been freed yet. I would just drop and free the skb and returnNETDEV_TX_OKinm_can_tx_handler()for peripheral devices (likecan_dropped_invalid_skb()does).OK.
Quoted 8 lines
Quoted 2 lines
So would this also be a bug in the hi3110 and mcp251x drivers (line 521) as well because besides checking
tx_lengththis is how these drivers are written.This is different. When entering the "
start_xmit" routine, the previous TX is still in progress. It will (hopefully) complete soon. Therefore returningNETDEV_TX_BUSYis OK. The "start_xmit" routine will be recalled soon with the same "skb". That scenario should/could also not happen.In principle, this also applies to the
m_canperipheral devices. Iftx_skbis not NULL, the TX is still in progress and returningNETDEV_TX_BUSYis just fine.Quoted 8 lines
In contrast, in "
m_can_tx_handler()", the skb could not be handled because the FIFO is full. The "start_xmit" routine for peripheral devices for that skb already returnedNETDEV_TX_OK. Therefore the only meaningful action is to drop the skb. Also this error should not happen and if, something is going really wrong. Therefore I think, aWARN_ONCE()would be even more appropriate. But that should be a separate patch.Quoted 6 lines
Quoted 3 lines
But that's a different issue/error. The
tx_skbcannot be processed in "m_can_tx_handler()". Either we drop it or we re-queue it (retry later).OK I am a bit confused on this. Are you saying this is not an issue? Or are you saying I need to check for
tx_lenlike the other code?
Quoted 7 lines
If you check for
tx_skbin the "start_xmit" routine like the hi3110 and mcp251x, it will work the same way. But only, if the "tx_handler()" has fully processed the message. It simple means, the TX is still in progress and will complete soon. But in "m_can_tx_handler()" we return without handling the message! It will never be sent and freed. Or will the "m_can_tx_handler()" retry?
I am not seeing where we are not handling the message in the m_can_tx_handler()
In the peripheral code the work is queued up. And the work thread is m_can_tx_work_queue.
This in turn calls the m_can_tx_handler and the worker is blocked until return which means the message would have been processed.
If there is no issue and the handler returns OK then the skb is set to null. Otherwise the only other time that the skb will not be null is if the FIFO was full.
Plus there can only be one work queue at a time so the processing is synchronous. If the upper layer decides to send another packet before the prior one is complete then it will get a TX busy return.
IOmapped calls are blocked on return so this is not an issue. We cannot do it the same way with peripherals due to the atomic context of the request.
Dan
Quoted 8 lines
Quoted 1 line
Again if this code is an issue here I believe this is an issue in the hi3110 and mcp251x
I don't think so.
Sorry for confusion.
Wolfgang.
Signature · 2 lines
------------------ Dan Murphy
Hello Dan,
Am 08.03.19 um 18:25 schrieb Dan Murphy:
Quoted 145 lines · Wolfgang Grandegger
Quoted 136 lines
Hello,
Am 08.03.19 um 16:48 schrieb Dan Murphy:
Quoted 126 lines
Wolfgang
Quoted 123 lines · Wolfgang Grandegger
Hello Dan,
thinking more about it...
Am 08.03.19 um 14:29 schrieb Wolfgang Grandegger:
Quoted 96 lines
Hello Dan,
Am 08.03.19 um 13:44 schrieb Dan Murphy:
Quoted 93 lines
Wolfgang
Quoted 78 lines · Wolfgang Grandegger
Hallo Dan,
Am 05.03.19 um 16:52 schrieb Dan Murphy:
Quoted 61 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]>---
v7 - Fixed remaining new checkpatch issues, removed CSR setting, fixed tx hard
start function to returntx_busy, and renamed device callbacks - https://lore.kernel.org/patchwork/patch/1047220/v6 - Squashed platform patch to this patch for bissectablity, fixed coding style issues, updated
Kconfighelp, placed mcan reg offsets back into c file, renamedpriv->skbtopriv->tx_skband cleared perp interrupts at ISR start - Patch 1 comments - https://lore.kernel.org/patchwork/patch/1042446/
Patch 2 comments - https://lore.kernel.org/patchwork/patch/1042442/drivers/net/can/m_can/Kconfig | 13 +- drivers/net/can/m_can/Makefile | 1 + drivers/net/can/m_can/m_can.c | 700 +++++++++++++------------ drivers/net/can/m_can/m_can.h | 110 ++++ drivers/net/can/m_can/m_can_platform.c | 202 +++++++ 5 files changed, 682 insertions(+), 344 deletions(-) create mode 100644 drivers/net/can/m_can/m_can.h create mode 100644 drivers/net/can/m_can/m_can_platform.cdrivers/net/can/m_can/Kconfig +11
-2@@ -1,5 +1,14 @@ config CAN_M_CAN + tristate "Bosch M_CAN support" + ---help--- + Say Y here if you want support for Bosch M_CAN controller framework. + This is common support for devices that embed the Bosch M_CAN IP. + +config CAN_M_CAN_PLATFORM + tristate "Bosch M_CAN support for io-mapped devices" depends on HAS_IOMEM - tristate "Bosch M_CAN devices" + depends on CAN_M_CAN ---help--- - Say Y here if you want to support for Bosch M_CAN controller. + Say Y here if you want support for IO Mapped Bosch M_CAN controller. + This support is for devices that have the Bosch M_CAN controller + IP embedded into the device and the IP is IO Mapped to the processor.drivers/net/can/m_can/Makefile +1
-0@@ -3,3 +3,4 @@ # obj-$(CONFIG_CAN_M_CAN) += m_can.o +obj-$(CONFIG_CAN_M_CAN_PLATFORM) += m_can_platform.odrivers/net/can/m_can/m_can.c +0
-0... snip...
Quoted 11 lines
drivers/net/can/m_can/m_can.c +11
-0+static netdev_tx_t m_can_start_xmit (struct sk_buff *skb, +struct net_device *dev) +{ +struct m_can_priv *priv =netdev_priv (dev); + +if (can_dropped_invalid_skb (dev, skb)) +return NETDEV_TX_OK ; + +if (priv->is_peripherial) { +if (priv->tx_skb) { +netdev_err (dev,"hard_xmit called while tx busy\n" );Quoted 9 lines
drivers/net/can/m_can/m_can.c
Quoted 2 lines
drivers/net/can/m_can/m_can.c +2
-0+return NETDEV_TX_BUSY ; + }The problem with that approach is, that the upper layer will try to resubmit the current "skb" but not the previous "
tx_skb". And the previous "tx_skb" has not been freed yet. I would just drop and free the skb and returnNETDEV_TX_OKinm_can_tx_handler()for peripheral devices (likecan_dropped_invalid_skb()does).OK.
Quoted 8 lines
Quoted 2 lines
So would this also be a bug in the hi3110 and mcp251x drivers (line 521) as well because besides checking
tx_lengththis is how these drivers are written.This is different. When entering the "
start_xmit" routine, the previous TX is still in progress. It will (hopefully) complete soon. Therefore returningNETDEV_TX_BUSYis OK. The "start_xmit" routine will be recalled soon with the same "skb". That scenario should/could also not happen.In principle, this also applies to the
m_canperipheral devices. Iftx_skbis not NULL, the TX is still in progress and returningNETDEV_TX_BUSYis just fine.Quoted 8 lines
In contrast, in "
m_can_tx_handler()", the skb could not be handled because the FIFO is full. The "start_xmit" routine for peripheral devices for that skb already returnedNETDEV_TX_OK. Therefore the only meaningful action is to drop the skb. Also this error should not happen and if, something is going really wrong. Therefore I think, aWARN_ONCE()would be even more appropriate. But that should be a separate patch.Quoted 6 lines
Quoted 3 lines
But that's a different issue/error. The
tx_skbcannot be processed in "m_can_tx_handler()". Either we drop it or we re-queue it (retry later).OK I am a bit confused on this. Are you saying this is not an issue? Or are you saying I need to check for
tx_lenlike the other code?Quoted 7 lines
If you check for
tx_skbin the "start_xmit" routine like the hi3110 and mcp251x, it will work the same way. But only, if the "tx_handler()" has fully processed the message. It simple means, the TX is still in progress and will complete soon. But in "m_can_tx_handler()" we return without handling the message! It will never be sent and freed. Or will the "m_can_tx_handler()" retry?
Quoted 1 line
I am not seeing where we are not handling the message in the
m_can_tx_handler()
static void m_can_tx_handler(struct m_can_classdev *priv)
{
...
/* Check if FIFO full */
if (m_can_tx_fifo_full(priv)) {
/* This shouldn't happen */
netif_stop_queue(dev);
netdev_warn(dev,
"TX queue active although FIFO is full.");
return;
}We simply return here. When is the message (tx_skb) processed (sent or freed)? What happens with tx_skb?
For the hi3110, we have:
static void hi3110_tx_work_handler(struct work_struct *ws)
{
struct hi3110_priv *priv = container_of(ws, struct hi3110_priv,
tx_work);
struct spi_device *spi = priv->spi;
struct net_device *net = priv->net;
struct can_frame *frame;
mutex_lock(&priv->hi3110_lock);
if (priv->tx_skb) {
if (priv->can.state == CAN_STATE_BUS_OFF) {
hi3110_clean(net);
} else {
frame = (struct can_frame *)priv->tx_skb->data;
hi3110_hw_tx(spi, frame);
priv->tx_len = 1 + frame->can_dlc;
can_put_echo_skb(priv->tx_skb, net, 0);
priv->tx_skb = NULL;
}
}
mutex_unlock(&priv->hi3110_lock);
}Either the tx_skb is sent or cleanup (dropped and freed) in case of bus-off. Also "hi3110_clean" sets "priv->tx_skb = NULL"! The "tx_len" handles a pending "echo_skb".
Quoted 9 lines
In the peripheral code the work is queued up. And the work thread is
m_can_tx_work_queue.This in turn calls the
m_can_tx_handlerand the worker is blocked until return which means the message would have been processed.If there is no issue and the handler returns OK then the skb is set to null. Otherwise the only other time that the skb will not be null is if the FIFO was full.
Quoted 6 lines
Plus there can only be one work queue at a time so the processing is synchronous. If the upper layer decides to send another packet before the prior one is complete then it will get a TX busy return.
IOmapped calls are blocked on return so this is not an issue. We cannot do it the same way with peripherals due to the atomic context of the request.
Wolfgang.
Quoted 162 lines · Wolfgang Grandegger
Hello Dan,
Am 08.03.19 um 18:25 schrieb Dan Murphy:
Quoted 145 lines · Wolfgang Grandegger
Quoted 136 lines
Hello,
Am 08.03.19 um 16:48 schrieb Dan Murphy:
Quoted 126 lines
Wolfgang
Quoted 123 lines · Wolfgang Grandegger
Hello Dan,
thinking more about it...
Am 08.03.19 um 14:29 schrieb Wolfgang Grandegger:
Quoted 96 lines
Hello Dan,
Am 08.03.19 um 13:44 schrieb Dan Murphy:
Quoted 93 lines
Wolfgang
Quoted 78 lines · Wolfgang Grandegger
Hallo Dan,
Am 05.03.19 um 16:52 schrieb Dan Murphy:
Quoted 61 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]>---
v7 - Fixed remaining new checkpatch issues, removed CSR setting, fixed tx hard
start function to returntx_busy, and renamed device callbacks - https://lore.kernel.org/patchwork/patch/1047220/v6 - Squashed platform patch to this patch for bissectablity, fixed coding style issues, updated
Kconfighelp, placed mcan reg offsets back into c file, renamedpriv->skbtopriv->tx_skband cleared perp interrupts at ISR start - Patch 1 comments - https://lore.kernel.org/patchwork/patch/1042446/
Patch 2 comments - https://lore.kernel.org/patchwork/patch/1042442/drivers/net/can/m_can/Kconfig | 13 +- drivers/net/can/m_can/Makefile | 1 + drivers/net/can/m_can/m_can.c | 700 +++++++++++++------------ drivers/net/can/m_can/m_can.h | 110 ++++ drivers/net/can/m_can/m_can_platform.c | 202 +++++++ 5 files changed, 682 insertions(+), 344 deletions(-) create mode 100644 drivers/net/can/m_can/m_can.h create mode 100644 drivers/net/can/m_can/m_can_platform.cdrivers/net/can/m_can/Kconfig +11
-2@@ -1,5 +1,14 @@ config CAN_M_CAN + tristate "Bosch M_CAN support" + ---help--- + Say Y here if you want support for Bosch M_CAN controller framework. + This is common support for devices that embed the Bosch M_CAN IP. + +config CAN_M_CAN_PLATFORM + tristate "Bosch M_CAN support for io-mapped devices" depends on HAS_IOMEM - tristate "Bosch M_CAN devices" + depends on CAN_M_CAN ---help--- - Say Y here if you want to support for Bosch M_CAN controller. + Say Y here if you want support for IO Mapped Bosch M_CAN controller. + This support is for devices that have the Bosch M_CAN controller + IP embedded into the device and the IP is IO Mapped to the processor.drivers/net/can/m_can/Makefile +1
-0@@ -3,3 +3,4 @@ # obj-$(CONFIG_CAN_M_CAN) += m_can.o +obj-$(CONFIG_CAN_M_CAN_PLATFORM) += m_can_platform.odrivers/net/can/m_can/m_can.c +0
-0... snip...
Quoted 11 lines
drivers/net/can/m_can/m_can.c +11
-0+static netdev_tx_t m_can_start_xmit (struct sk_buff *skb, +struct net_device *dev) +{ +struct m_can_priv *priv =netdev_priv (dev); + +if (can_dropped_invalid_skb (dev, skb)) +return NETDEV_TX_OK ; + +if (priv->is_peripherial) { +if (priv->tx_skb) { +netdev_err (dev,"hard_xmit called while tx busy\n" );Quoted 9 lines
drivers/net/can/m_can/m_can.c
Quoted 2 lines
drivers/net/can/m_can/m_can.c +2
-0+return NETDEV_TX_BUSY ; + }The problem with that approach is, that the upper layer will try to resubmit the current "skb" but not the previous "
tx_skb". And the previous "tx_skb" has not been freed yet. I would just drop and free the skb and returnNETDEV_TX_OKinm_can_tx_handler()for peripheral devices (likecan_dropped_invalid_skb()does).OK.
Quoted 8 lines
Quoted 2 lines
So would this also be a bug in the hi3110 and mcp251x drivers (line 521) as well because besides checking
tx_lengththis is how these drivers are written.This is different. When entering the "
start_xmit" routine, the previous TX is still in progress. It will (hopefully) complete soon. Therefore returningNETDEV_TX_BUSYis OK. The "start_xmit" routine will be recalled soon with the same "skb". That scenario should/could also not happen.In principle, this also applies to the
m_canperipheral devices. Iftx_skbis not NULL, the TX is still in progress and returningNETDEV_TX_BUSYis just fine.Quoted 8 lines
In contrast, in "
m_can_tx_handler()", the skb could not be handled because the FIFO is full. The "start_xmit" routine for peripheral devices for that skb already returnedNETDEV_TX_OK. Therefore the only meaningful action is to drop the skb. Also this error should not happen and if, something is going really wrong. Therefore I think, aWARN_ONCE()would be even more appropriate. But that should be a separate patch.Quoted 6 lines
Quoted 3 lines
But that's a different issue/error. The
tx_skbcannot be processed in "m_can_tx_handler()". Either we drop it or we re-queue it (retry later).OK I am a bit confused on this. Are you saying this is not an issue? Or are you saying I need to check for
tx_lenlike the other code?Quoted 7 lines
If you check for
tx_skbin the "start_xmit" routine like the hi3110 and mcp251x, it will work the same way. But only, if the "tx_handler()" has fully processed the message. It simple means, the TX is still in progress and will complete soon. But in "m_can_tx_handler()" we return without handling the message! It will never be sent and freed. Or will the "m_can_tx_handler()" retry?Quoted 1 line
I am not seeing where we are not handling the message in the
m_can_tx_handler()static void
m_can_tx_handler(struct m_can_classdev *priv){ ... /* Check if FIFO full */ if (m_can_tx_fifo_full(priv)) { /* This shouldn't happen */ netif_stop_queue(dev); netdev_warn(dev, "TX queue active although FIFO is full."); return; }
Quoted 3 lines
We simply return here. When is the message (
tx_skb) processed (sent or freed)? What happens withtx_skb?
Are you sure you are looking at the right code?
For patch version v7 I have the following
/* Check if FIFO full */
if (m_can_tx_fifo_full(cdev)) {
/* This shouldn't happen */
netif_stop_queue(dev);
netdev_warn(dev,
"TX queue active although FIFO is full.");
return NETDEV_TX_BUSY;
}Which is no change from the original source code.
Quoted 47 lines
For the hi3110, we have:
static void
hi3110_tx_work_handler(struct work_struct *ws){ struct hi3110_priv *priv = container_of(ws, struct hi3110_priv, tx_work); struct spi_device *spi = priv->spi; struct net_device *net = priv->net; struct can_frame *frame; mutex_lock(&priv->hi3110_lock); if (priv->tx_skb) { if (priv->can.state == CAN_STATE_BUS_OFF) { hi3110_clean(net); } else { frame = (struct can_frame *)priv->tx_skb->data; hi3110_hw_tx(spi, frame); priv->tx_len = 1 + frame->can_dlc; can_put_echo_skb(priv->tx_skb, net, 0); priv->tx_skb = NULL; } } mutex_unlock(&priv->hi3110_lock); }Either the
tx_skbis sent or cleanup (dropped and freed) in case of bus-off. Also "hi3110_clean" sets "priv->tx_skb= NULL"! The "tx_len" handles a pending "echo_skb".Quoted 9 lines
In the peripheral code the work is queued up. And the work thread is
m_can_tx_work_queue.This in turn calls the
m_can_tx_handlerand the worker is blocked until return which means the message would have been processed.If there is no issue and the handler returns OK then the skb is set to null. Otherwise the only other time that the skb will not be null is if the FIFO was full.
Quoted 6 lines
Plus there can only be one work queue at a time so the processing is synchronous. If the upper layer decides to send another packet before the prior one is complete then it will get a TX busy return.
IOmapped calls are blocked on return so this is not an issue. We cannot do it the same way with peripherals due to the atomic context of the request.
Wolfgang.
Signature · 2 lines
------------------ Dan Murphy
Am 08.03.19 um 18:52 schrieb Dan Murphy:
Quoted 180 lines · Wolfgang Grandegger
Quoted 162 lines
Hello Dan,
Am 08.03.19 um 18:25 schrieb Dan Murphy:
Quoted 145 lines · Wolfgang Grandegger
Quoted 136 lines
Hello,
Am 08.03.19 um 16:48 schrieb Dan Murphy:
Quoted 126 lines
Wolfgang
Quoted 123 lines · Wolfgang Grandegger
Hello Dan,
thinking more about it...
Am 08.03.19 um 14:29 schrieb Wolfgang Grandegger:
Quoted 96 lines
Hello Dan,
Am 08.03.19 um 13:44 schrieb Dan Murphy:
Quoted 93 lines
Wolfgang
Quoted 78 lines · Wolfgang Grandegger
Hallo Dan,
Am 05.03.19 um 16:52 schrieb Dan Murphy:
Quoted 61 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]>---
v7 - Fixed remaining new checkpatch issues, removed CSR setting, fixed tx hard
start function to returntx_busy, and renamed device callbacks - https://lore.kernel.org/patchwork/patch/1047220/v6 - Squashed platform patch to this patch for bissectablity, fixed coding style issues, updated
Kconfighelp, placed mcan reg offsets back into c file, renamedpriv->skbtopriv->tx_skband cleared perp interrupts at ISR start - Patch 1 comments - https://lore.kernel.org/patchwork/patch/1042446/
Patch 2 comments - https://lore.kernel.org/patchwork/patch/1042442/drivers/net/can/m_can/Kconfig | 13 +- drivers/net/can/m_can/Makefile | 1 + drivers/net/can/m_can/m_can.c | 700 +++++++++++++------------ drivers/net/can/m_can/m_can.h | 110 ++++ drivers/net/can/m_can/m_can_platform.c | 202 +++++++ 5 files changed, 682 insertions(+), 344 deletions(-) create mode 100644 drivers/net/can/m_can/m_can.h create mode 100644 drivers/net/can/m_can/m_can_platform.cdrivers/net/can/m_can/Kconfig +11
-2@@ -1,5 +1,14 @@ config CAN_M_CAN + tristate "Bosch M_CAN support" + ---help--- + Say Y here if you want support for Bosch M_CAN controller framework. + This is common support for devices that embed the Bosch M_CAN IP. + +config CAN_M_CAN_PLATFORM + tristate "Bosch M_CAN support for io-mapped devices" depends on HAS_IOMEM - tristate "Bosch M_CAN devices" + depends on CAN_M_CAN ---help--- - Say Y here if you want to support for Bosch M_CAN controller. + Say Y here if you want support for IO Mapped Bosch M_CAN controller. + This support is for devices that have the Bosch M_CAN controller + IP embedded into the device and the IP is IO Mapped to the processor.drivers/net/can/m_can/Makefile +1
-0@@ -3,3 +3,4 @@ # obj-$(CONFIG_CAN_M_CAN) += m_can.o +obj-$(CONFIG_CAN_M_CAN_PLATFORM) += m_can_platform.odrivers/net/can/m_can/m_can.c +0
-0... snip...
Quoted 11 lines
drivers/net/can/m_can/m_can.c +11
-0+static netdev_tx_t m_can_start_xmit (struct sk_buff *skb, +struct net_device *dev) +{ +struct m_can_priv *priv =netdev_priv (dev); + +if (can_dropped_invalid_skb (dev, skb)) +return NETDEV_TX_OK ; + +if (priv->is_peripherial) { +if (priv->tx_skb) { +netdev_err (dev,"hard_xmit called while tx busy\n" );Quoted 9 lines
drivers/net/can/m_can/m_can.c
Quoted 2 lines
drivers/net/can/m_can/m_can.c +2
-0+return NETDEV_TX_BUSY ; + }The problem with that approach is, that the upper layer will try to resubmit the current "skb" but not the previous "
tx_skb". And the previous "tx_skb" has not been freed yet. I would just drop and free the skb and returnNETDEV_TX_OKinm_can_tx_handler()for peripheral devices (likecan_dropped_invalid_skb()does).OK.
Quoted 8 lines
Quoted 2 lines
So would this also be a bug in the hi3110 and mcp251x drivers (line 521) as well because besides checking
tx_lengththis is how these drivers are written.This is different. When entering the "
start_xmit" routine, the previous TX is still in progress. It will (hopefully) complete soon. Therefore returningNETDEV_TX_BUSYis OK. The "start_xmit" routine will be recalled soon with the same "skb". That scenario should/could also not happen.In principle, this also applies to the
m_canperipheral devices. Iftx_skbis not NULL, the TX is still in progress and returningNETDEV_TX_BUSYis just fine.Quoted 8 lines
In contrast, in "
m_can_tx_handler()", the skb could not be handled because the FIFO is full. The "start_xmit" routine for peripheral devices for that skb already returnedNETDEV_TX_OK. Therefore the only meaningful action is to drop the skb. Also this error should not happen and if, something is going really wrong. Therefore I think, aWARN_ONCE()would be even more appropriate. But that should be a separate patch.Quoted 6 lines
Quoted 3 lines
But that's a different issue/error. The
tx_skbcannot be processed in "m_can_tx_handler()". Either we drop it or we re-queue it (retry later).OK I am a bit confused on this. Are you saying this is not an issue? Or are you saying I need to check for
tx_lenlike the other code?Quoted 7 lines
If you check for
tx_skbin the "start_xmit" routine like the hi3110 and mcp251x, it will work the same way. But only, if the "tx_handler()" has fully processed the message. It simple means, the TX is still in progress and will complete soon. But in "m_can_tx_handler()" we return without handling the message! It will never be sent and freed. Or will the "m_can_tx_handler()" retry?Quoted 1 line
I am not seeing where we are not handling the message in the
m_can_tx_handler()static void
m_can_tx_handler(struct m_can_classdev *priv){ ... /* Check if FIFO full */ if (m_can_tx_fifo_full(priv)) { /* This shouldn't happen */ netif_stop_queue(dev); netdev_warn(dev, "TX queue active although FIFO is full."); return; }Quoted 3 lines
We simply return here. When is the message (
tx_skb) processed (sent or freed)? What happens withtx_skb?Are you sure you are looking at the right code?
For patch version v7 I have the following
/* Check if FIFO full */ if (m_can_tx_fifo_full(cdev)) { /* This shouldn't happen */ netif_stop_queue(dev); netdev_warn(dev, "TX queue active although FIFO is full."); return NETDEV_TX_BUSY; }
Quoted 1 line
Which is no change from the original source code.
I know, but for the peripheral devices you have:
static void m_can_tx_work_queue(struct work_struct *ws)
{
struct m_can_priv *priv = container_of(ws, struct m_can_priv,
tx_work);
netdev_tx_t ret;
ret = m_can_tx_handler(priv);
if (ret == NETDEV_TX_OK)
priv->tx_skb = NULL;}
What will happen with tx_skb if NETDEV_TX_BUSY? It has not been dropped/freed yet?
Wolfgang.
Quoted 196 lines · Wolfgang Grandegger
Am 08.03.19 um 18:52 schrieb Dan Murphy:
Quoted 180 lines · Wolfgang Grandegger
Quoted 162 lines
Hello Dan,
Am 08.03.19 um 18:25 schrieb Dan Murphy:
Quoted 145 lines · Wolfgang Grandegger
Quoted 136 lines
Hello,
Am 08.03.19 um 16:48 schrieb Dan Murphy:
Quoted 126 lines
Wolfgang
Quoted 123 lines · Wolfgang Grandegger
Hello Dan,
thinking more about it...
Am 08.03.19 um 14:29 schrieb Wolfgang Grandegger:
Quoted 96 lines
Hello Dan,
Am 08.03.19 um 13:44 schrieb Dan Murphy:
Quoted 93 lines
Wolfgang
Quoted 78 lines · Wolfgang Grandegger
Hallo Dan,
Am 05.03.19 um 16:52 schrieb Dan Murphy:
Quoted 61 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]>---
v7 - Fixed remaining new checkpatch issues, removed CSR setting, fixed tx hard
start function to returntx_busy, and renamed device callbacks - https://lore.kernel.org/patchwork/patch/1047220/v6 - Squashed platform patch to this patch for bissectablity, fixed coding style issues, updated
Kconfighelp, placed mcan reg offsets back into c file, renamedpriv->skbtopriv->tx_skband cleared perp interrupts at ISR start - Patch 1 comments - https://lore.kernel.org/patchwork/patch/1042446/
Patch 2 comments - https://lore.kernel.org/patchwork/patch/1042442/drivers/net/can/m_can/Kconfig | 13 +- drivers/net/can/m_can/Makefile | 1 + drivers/net/can/m_can/m_can.c | 700 +++++++++++++------------ drivers/net/can/m_can/m_can.h | 110 ++++ drivers/net/can/m_can/m_can_platform.c | 202 +++++++ 5 files changed, 682 insertions(+), 344 deletions(-) create mode 100644 drivers/net/can/m_can/m_can.h create mode 100644 drivers/net/can/m_can/m_can_platform.cdrivers/net/can/m_can/Kconfig +11
-2@@ -1,5 +1,14 @@ config CAN_M_CAN + tristate "Bosch M_CAN support" + ---help--- + Say Y here if you want support for Bosch M_CAN controller framework. + This is common support for devices that embed the Bosch M_CAN IP. + +config CAN_M_CAN_PLATFORM + tristate "Bosch M_CAN support for io-mapped devices" depends on HAS_IOMEM - tristate "Bosch M_CAN devices" + depends on CAN_M_CAN ---help--- - Say Y here if you want to support for Bosch M_CAN controller. + Say Y here if you want support for IO Mapped Bosch M_CAN controller. + This support is for devices that have the Bosch M_CAN controller + IP embedded into the device and the IP is IO Mapped to the processor.drivers/net/can/m_can/Makefile +1
-0@@ -3,3 +3,4 @@ # obj-$(CONFIG_CAN_M_CAN) += m_can.o +obj-$(CONFIG_CAN_M_CAN_PLATFORM) += m_can_platform.odrivers/net/can/m_can/m_can.c +0
-0... snip...
Quoted 11 lines
drivers/net/can/m_can/m_can.c +11
-0+static netdev_tx_t m_can_start_xmit (struct sk_buff *skb, +struct net_device *dev) +{ +struct m_can_priv *priv =netdev_priv (dev); + +if (can_dropped_invalid_skb (dev, skb)) +return NETDEV_TX_OK ; + +if (priv->is_peripherial) { +if (priv->tx_skb) { +netdev_err (dev,"hard_xmit called while tx busy\n" );Quoted 9 lines
drivers/net/can/m_can/m_can.c
Quoted 2 lines
drivers/net/can/m_can/m_can.c +2
-0+return NETDEV_TX_BUSY ; + }The problem with that approach is, that the upper layer will try to resubmit the current "skb" but not the previous "
tx_skb". And the previous "tx_skb" has not been freed yet. I would just drop and free the skb and returnNETDEV_TX_OKinm_can_tx_handler()for peripheral devices (likecan_dropped_invalid_skb()does).OK.
Quoted 8 lines
Quoted 2 lines
So would this also be a bug in the hi3110 and mcp251x drivers (line 521) as well because besides checking
tx_lengththis is how these drivers are written.This is different. When entering the "
start_xmit" routine, the previous TX is still in progress. It will (hopefully) complete soon. Therefore returningNETDEV_TX_BUSYis OK. The "start_xmit" routine will be recalled soon with the same "skb". That scenario should/could also not happen.In principle, this also applies to the
m_canperipheral devices. Iftx_skbis not NULL, the TX is still in progress and returningNETDEV_TX_BUSYis just fine.Quoted 8 lines
In contrast, in "
m_can_tx_handler()", the skb could not be handled because the FIFO is full. The "start_xmit" routine for peripheral devices for that skb already returnedNETDEV_TX_OK. Therefore the only meaningful action is to drop the skb. Also this error should not happen and if, something is going really wrong. Therefore I think, aWARN_ONCE()would be even more appropriate. But that should be a separate patch.Quoted 6 lines
Quoted 3 lines
But that's a different issue/error. The
tx_skbcannot be processed in "m_can_tx_handler()". Either we drop it or we re-queue it (retry later).OK I am a bit confused on this. Are you saying this is not an issue? Or are you saying I need to check for
tx_lenlike the other code?Quoted 7 lines
If you check for
tx_skbin the "start_xmit" routine like the hi3110 and mcp251x, it will work the same way. But only, if the "tx_handler()" has fully processed the message. It simple means, the TX is still in progress and will complete soon. But in "m_can_tx_handler()" we return without handling the message! It will never be sent and freed. Or will the "m_can_tx_handler()" retry?Quoted 1 line
I am not seeing where we are not handling the message in the
m_can_tx_handler()static void
m_can_tx_handler(struct m_can_classdev *priv){ ... /* Check if FIFO full */ if (m_can_tx_fifo_full(priv)) { /* This shouldn't happen */ netif_stop_queue(dev); netdev_warn(dev, "TX queue active although FIFO is full."); return; }Quoted 3 lines
We simply return here. When is the message (
tx_skb) processed (sent or freed)? What happens withtx_skb?Are you sure you are looking at the right code?
For patch version v7 I have the following
/* Check if FIFO full */ if (m_can_tx_fifo_full(cdev)) { /* This shouldn't happen */ netif_stop_queue(dev); netdev_warn(dev, "TX queue active although FIFO is full."); return NETDEV_TX_BUSY; }Quoted 1 line
Which is no change from the original source code.
I know, but for the peripheral devices you have:
static void
m_can_tx_work_queue(struct work_struct *ws){ struct m_can_priv *priv = container_of(ws, struct m_can_priv, tx_work); netdev_tx_t ret; ret = m_can_tx_handler(priv); if (ret == NETDEV_TX_OK) priv->tx_skb = NULL;
Quoted 5 lines
}
What will happen with
tx_skbifNETDEV_TX_BUSY? It has not been dropped/freed yet?
OK I think I see the issue there.
I should probably add can_put_echo_skb if NETDEV_TX_BUSY and always NULL out the SKB.
This appears to be the way the other perp drivers do it as they just put and null the skb regardless of the return of the handlers.
And clean is called when the BUS is off or coming out of suspend.
Dan
Quoted 2 lines
Wolfgang.
Signature · 2 lines
------------------ Dan Murphy
Hello Dan,
Am 08.03.19 um 21:36 schrieb Dan Murphy:
Quoted 199 lines · Wolfgang Grandegger
Quoted 198 lines
Am 08.03.19 um 18:52 schrieb Dan Murphy:
Quoted 180 lines · Wolfgang Grandegger
Quoted 162 lines
Hello Dan,
Am 08.03.19 um 18:25 schrieb Dan Murphy:
Quoted 145 lines · Wolfgang Grandegger
Quoted 136 lines
Hello,
Am 08.03.19 um 16:48 schrieb Dan Murphy:
Quoted 126 lines
Wolfgang
Quoted 123 lines · Wolfgang Grandegger
Hello Dan,
thinking more about it...
Am 08.03.19 um 14:29 schrieb Wolfgang Grandegger:
Quoted 96 lines
Hello Dan,
Am 08.03.19 um 13:44 schrieb Dan Murphy:
Quoted 93 lines
Wolfgang
Quoted 78 lines · Wolfgang Grandegger
Hallo Dan,
Am 05.03.19 um 16:52 schrieb Dan Murphy:
Quoted 61 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]>---
v7 - Fixed remaining new checkpatch issues, removed CSR setting, fixed tx hard
start function to returntx_busy, and renamed device callbacks - https://lore.kernel.org/patchwork/patch/1047220/v6 - Squashed platform patch to this patch for bissectablity, fixed coding style issues, updated
Kconfighelp, placed mcan reg offsets back into c file, renamedpriv->skbtopriv->tx_skband cleared perp interrupts at ISR start - Patch 1 comments - https://lore.kernel.org/patchwork/patch/1042446/
Patch 2 comments - https://lore.kernel.org/patchwork/patch/1042442/drivers/net/can/m_can/Kconfig | 13 +- drivers/net/can/m_can/Makefile | 1 + drivers/net/can/m_can/m_can.c | 700 +++++++++++++------------ drivers/net/can/m_can/m_can.h | 110 ++++ drivers/net/can/m_can/m_can_platform.c | 202 +++++++ 5 files changed, 682 insertions(+), 344 deletions(-) create mode 100644 drivers/net/can/m_can/m_can.h create mode 100644 drivers/net/can/m_can/m_can_platform.cdrivers/net/can/m_can/Kconfig +11
-2@@ -1,5 +1,14 @@ config CAN_M_CAN + tristate "Bosch M_CAN support" + ---help--- + Say Y here if you want support for Bosch M_CAN controller framework. + This is common support for devices that embed the Bosch M_CAN IP. + +config CAN_M_CAN_PLATFORM + tristate "Bosch M_CAN support for io-mapped devices" depends on HAS_IOMEM - tristate "Bosch M_CAN devices" + depends on CAN_M_CAN ---help--- - Say Y here if you want to support for Bosch M_CAN controller. + Say Y here if you want support for IO Mapped Bosch M_CAN controller. + This support is for devices that have the Bosch M_CAN controller + IP embedded into the device and the IP is IO Mapped to the processor.drivers/net/can/m_can/Makefile +1
-0@@ -3,3 +3,4 @@ # obj-$(CONFIG_CAN_M_CAN) += m_can.o +obj-$(CONFIG_CAN_M_CAN_PLATFORM) += m_can_platform.odrivers/net/can/m_can/m_can.c +0
-0... snip...
Quoted 11 lines
drivers/net/can/m_can/m_can.c +11
-0+static netdev_tx_t m_can_start_xmit (struct sk_buff *skb, +struct net_device *dev) +{ +struct m_can_priv *priv =netdev_priv (dev); + +if (can_dropped_invalid_skb (dev, skb)) +return NETDEV_TX_OK ; + +if (priv->is_peripherial) { +if (priv->tx_skb) { +netdev_err (dev,"hard_xmit called while tx busy\n" );Quoted 9 lines
drivers/net/can/m_can/m_can.c
Quoted 2 lines
drivers/net/can/m_can/m_can.c +2
-0+return NETDEV_TX_BUSY ; + }The problem with that approach is, that the upper layer will try to resubmit the current "skb" but not the previous "
tx_skb". And the previous "tx_skb" has not been freed yet. I would just drop and free the skb and returnNETDEV_TX_OKinm_can_tx_handler()for peripheral devices (likecan_dropped_invalid_skb()does).OK.
Quoted 8 lines
Quoted 2 lines
So would this also be a bug in the hi3110 and mcp251x drivers (line 521) as well because besides checking
tx_lengththis is how these drivers are written.This is different. When entering the "
start_xmit" routine, the previous TX is still in progress. It will (hopefully) complete soon. Therefore returningNETDEV_TX_BUSYis OK. The "start_xmit" routine will be recalled soon with the same "skb". That scenario should/could also not happen.In principle, this also applies to the
m_canperipheral devices. Iftx_skbis not NULL, the TX is still in progress and returningNETDEV_TX_BUSYis just fine.Quoted 8 lines
In contrast, in "
m_can_tx_handler()", the skb could not be handled because the FIFO is full. The "start_xmit" routine for peripheral devices for that skb already returnedNETDEV_TX_OK. Therefore the only meaningful action is to drop the skb. Also this error should not happen and if, something is going really wrong. Therefore I think, aWARN_ONCE()would be even more appropriate. But that should be a separate patch.Quoted 6 lines
Quoted 3 lines
But that's a different issue/error. The
tx_skbcannot be processed in "m_can_tx_handler()". Either we drop it or we re-queue it (retry later).OK I am a bit confused on this. Are you saying this is not an issue? Or are you saying I need to check for
tx_lenlike the other code?Quoted 7 lines
If you check for
tx_skbin the "start_xmit" routine like the hi3110 and mcp251x, it will work the same way. But only, if the "tx_handler()" has fully processed the message. It simple means, the TX is still in progress and will complete soon. But in "m_can_tx_handler()" we return without handling the message! It will never be sent and freed. Or will the "m_can_tx_handler()" retry?Quoted 1 line
I am not seeing where we are not handling the message in the
m_can_tx_handler()static void
m_can_tx_handler(struct m_can_classdev *priv){ ... /* Check if FIFO full */ if (m_can_tx_fifo_full(priv)) { /* This shouldn't happen */ netif_stop_queue(dev); netdev_warn(dev, "TX queue active although FIFO is full."); return; }Quoted 3 lines
We simply return here. When is the message (
tx_skb) processed (sent or freed)? What happens withtx_skb?Are you sure you are looking at the right code?
For patch version v7 I have the following
/* Check if FIFO full */ if (m_can_tx_fifo_full(cdev)) { /* This shouldn't happen */ netif_stop_queue(dev); netdev_warn(dev, "TX queue active although FIFO is full."); return NETDEV_TX_BUSY; }Quoted 1 line
Which is no change from the original source code.
I know, but for the peripheral devices you have:
static void
m_can_tx_work_queue(struct work_struct *ws){ struct m_can_priv *priv = container_of(ws, struct m_can_priv, tx_work); netdev_tx_t ret; ret = m_can_tx_handler(priv); if (ret == NETDEV_TX_OK) priv->tx_skb = NULL;}
Quoted 5 lines
Quoted 3 lines
What will happen with
tx_skbifNETDEV_TX_BUSY? It has not been dropped/freed yet?OK I think I see the issue there.
The key point is that the "skb" entered by the "start_xmit" must be released/free when it's processed (with NETDEV_TX_OK). This is more tricky for perp devices because the "skb" is handled deferred.
Quoted 2 lines
I should probably add
can_put_echo_skbifNETDEV_TX_BUSYand always NULL out the SKB.
can_put_echo_skb() should only be called after the TX has been initiated. The normal flow for the skb is:
start-xmit -> initiate tx -> can_put_echo_skb -> return NETDEV_TX_OK...
tx done interrupt -> can_get_echo_skb -> free skb
I would just drop the message/skb:
/* Check if FIFO full */
if (m_can_tx_fifo_full(cdev)) {
/* This shouldn't happen */
netif_stop_queue(dev);
netdev_warn(dev,
"TX queue active although FIFO is full.");
if (cdev->is_peripherial) {
kfree_skb(skb);
dev->stats.tx_dropped++;
return NETDEV_TX_OK;
} else {
return NETDEV_TX_BUSY;
}
}Quoted 2 lines
This appears to be the way the other perp drivers do it as they just put and null the skb regardless of the return of the handlers.
You can also use:
if (cdev->tx_skb) {
netdev_err(dev, "hard_xmit called while tx busy\n");
return NETDEV_TX_BUSY;
}to check for "hard_xmit called while tx busy". But you still need to handle the "m_can_tx_fifo_full(cdev)" case properly. See above.
Quoted 1 line
And clean is called when the BUS is off or coming out of suspend.
Probably we need that as well even if other drivers don't care if the device goes bus-off while TX messages are pending.
Wolfgang