On 15.03.2023 22:10:40, Dario Binacchi wrote:
> Add support for the basic extended CAN controller (bxCAN) found in many
> low- to middle-end STM32 SoCs. It supports the Basic Extended CAN
> protocol versions 2.0A and B with a maximum bit rate of 1 Mbit/s.
>
> The controller supports two channels (CAN1 as master and CAN2 as slave)
> and the driver can enable either or both of the channels. They share
> some of the required logic (e. g. clocks and filters), and that means
> you cannot use the slave CAN without enabling some hardware resources
> managed by the master CAN.
>
> Each channel has 3 transmit mailboxes, 2 receive FIFOs with 3 stages and
> 28 scalable filter banks.
> It also manages 4 dedicated interrupt vectors:
> - transmit interrupt
> - FIFO 0 receive interrupt
> - FIFO 1 receive interrupt
> - status change error interrupt
>
> Driver uses all 3 available mailboxes for transmission and FIFO 0 for
> reception. Rx filter rules are configured to the minimum. They accept
> all messages and assign filter 0 to CAN1 and filter 14 to CAN2 in
> identifier mask mode with 32 bits width. It enables and uses transmit,
> receive buffers for FIFO 0 and error and status change interrupts.
>
> Signed-off-by: Dario Binacchi <dario.binacchi@xxxxxxxxxxxxxxxxxxxx>
> Reviewed-by: Vincent Mailhol <mailhol.vincent@xxxxxxxxxx>
[...]
> diff --git a/drivers/net/can/bxcan.c b/drivers/net/can/bxcan.c
> new file mode 100644
> index 000000000000..daf4d7ef00e7
> --- /dev/null
> +++ b/drivers/net/can/bxcan.c
[...]
> +
> +static inline void bxcan_rmw(struct bxcan_priv *priv, void __iomem *addr,
> + u32 clear, u32 set)
> +{
> + unsigned long flags;
> + u32 old, val;
> +
> + spin_lock_irqsave(&priv->rmw_lock, flags);
> + old = readl(addr);
> + val = (old & ~clear) | set;
> + if (val != old)
> + writel(val, addr);
> +
> + spin_unlock_irqrestore(&priv->rmw_lock, flags);
> +}
I think you don't need the spin_lock anymore, but it's not in the hot
path, so leave it this way.
> +
[...]
> +static irqreturn_t bxcan_rx_isr(int irq, void *dev_id)
> +{
> + struct net_device *ndev = dev_id;
> + struct bxcan_priv *priv = netdev_priv(ndev);
> +
> + can_rx_offload_irq_offload_fifo(&priv->offload);
> + can_rx_offload_irq_finish(&priv->offload);
> +
> + return IRQ_HANDLED;
This handler is not 100% shared IRQ safe, you have to return IRQ_NONE if
no IRQ is active.
> +}
> +
> +static irqreturn_t bxcan_tx_isr(int irq, void *dev_id)
> +{
> + struct net_device *ndev = dev_id;
> + struct bxcan_priv *priv = netdev_priv(ndev);
> + struct bxcan_regs __iomem *regs = priv->regs;
> + struct net_device_stats *stats = &ndev->stats;
> + u32 tsr, rqcp_bit;
> + int idx;
> +
> + tsr = readl(®s->tsr);
> + if (!(tsr & (BXCAN_TSR_RQCP0 | BXCAN_TSR_RQCP1 | BXCAN_TSR_RQCP2)))
> + return IRQ_HANDLED;
Is this a check for no IRQ? Then return IRQ_NONE.
> +
> + while (priv->tx_head - priv->tx_tail > 0) {
> + idx = bxcan_get_tx_tail(priv);
> + rqcp_bit = BXCAN_TSR_RQCP0 << (idx << 3);
> + if (!(tsr & rqcp_bit))
> + break;
> +
> + stats->tx_packets++;
> + stats->tx_bytes += can_get_echo_skb(ndev, idx, NULL);
> + priv->tx_tail++;
> + }
> +
> + writel(tsr, ®s->tsr);
> +
> + if (bxcan_get_tx_free(priv)) {
> + /* Make sure that anybody stopping the queue after
> + * this sees the new tx_ring->tail.
> + */
> + smp_mb();
> + netif_wake_queue(ndev);
> + }
> +
> + return IRQ_HANDLED;
> +}
> +
> +static void bxcan_handle_state_change(struct net_device *ndev, u32 esr)
> +{
> + struct bxcan_priv *priv = netdev_priv(ndev);
> + enum can_state new_state = priv->can.state;
> + struct can_berr_counter bec;
> + enum can_state rx_state, tx_state;
> + struct sk_buff *skb;
> + struct can_frame *cf;
> +
> + /* Early exit if no error flag is set */
> + if (!(esr & (BXCAN_ESR_EWGF | BXCAN_ESR_EPVF | BXCAN_ESR_BOFF)))
> + return;
> +
> + bec.txerr = FIELD_GET(BXCAN_ESR_TEC_MASK, esr);
> + bec.rxerr = FIELD_GET(BXCAN_ESR_REC_MASK, esr);
> +
> + if (esr & BXCAN_ESR_BOFF)
> + new_state = CAN_STATE_BUS_OFF;
> + else if (esr & BXCAN_ESR_EPVF)
> + new_state = CAN_STATE_ERROR_PASSIVE;
> + else if (esr & BXCAN_ESR_EWGF)
> + new_state = CAN_STATE_ERROR_WARNING;
> +
> + /* state hasn't changed */
> + if (unlikely(new_state == priv->can.state))
> + return;
> +
> + skb = alloc_can_err_skb(ndev, &cf);
> +
> + tx_state = bec.txerr >= bec.rxerr ? new_state : 0;
> + rx_state = bec.txerr <= bec.rxerr ? new_state : 0;
> + can_change_state(ndev, cf, tx_state, rx_state);
> +
> + if (new_state == CAN_STATE_BUS_OFF) {
> + can_bus_off(ndev);
> + } else if (skb) {
> + cf->can_id |= CAN_ERR_CNT;
> + cf->data[6] = bec.txerr;
> + cf->data[7] = bec.rxerr;
> + }
> +
> + if (skb) {
> + int err;
> +
> + err = can_rx_offload_queue_timestamp(&priv->offload, skb,
> + priv->timestamp);
> + if (err)
> + ndev->stats.rx_fifo_errors++;
> + }
> +}
> +
> +static void bxcan_handle_bus_err(struct net_device *ndev, u32 esr)
> +{
> + struct bxcan_priv *priv = netdev_priv(ndev);
> + enum bxcan_lec_code lec_code;
> + struct can_frame *cf;
> + struct sk_buff *skb;
> +
> + lec_code = FIELD_GET(BXCAN_ESR_LEC_MASK, esr);
> +
> + /* Early exit if no lec update or no error.
> + * No lec update means that no CAN bus event has been detected
> + * since CPU wrote BXCAN_LEC_UNUSED value to status reg.
> + */
> + if (lec_code == BXCAN_LEC_UNUSED || lec_code == BXCAN_LEC_NO_ERROR)
> + return;
> +
> + /* Common for all type of bus errors */
> + priv->can.can_stats.bus_error++;
> +
> + /* Propagate the error condition to the CAN stack */
> + skb = alloc_can_err_skb(ndev, &cf);
> + if (skb)
> + cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
> +
> + switch (lec_code) {
> + case BXCAN_LEC_STUFF_ERROR:
> + netdev_dbg(ndev, "Stuff error\n");
> + ndev->stats.rx_errors++;
> + if (skb)
> + cf->data[2] |= CAN_ERR_PROT_STUFF;
> + break;
> +
> + case BXCAN_LEC_FORM_ERROR:
> + netdev_dbg(ndev, "Form error\n");
> + ndev->stats.rx_errors++;
> + if (skb)
> + cf->data[2] |= CAN_ERR_PROT_FORM;
> + break;
> +
> + case BXCAN_LEC_ACK_ERROR:
> + netdev_dbg(ndev, "Ack error\n");
> + ndev->stats.tx_errors++;
> + if (skb) {
> + cf->can_id |= CAN_ERR_ACK;
> + cf->data[3] = CAN_ERR_PROT_LOC_ACK;
> + }
> + break;
> +
> + case BXCAN_LEC_BIT1_ERROR:
> + netdev_dbg(ndev, "Bit error (recessive)\n");
> + ndev->stats.tx_errors++;
> + if (skb)
> + cf->data[2] |= CAN_ERR_PROT_BIT1;
> + break;
> +
> + case BXCAN_LEC_BIT0_ERROR:
> + netdev_dbg(ndev, "Bit error (dominant)\n");
> + ndev->stats.tx_errors++;
> + if (skb)
> + cf->data[2] |= CAN_ERR_PROT_BIT0;
> + break;
> +
> + case BXCAN_LEC_CRC_ERROR:
> + netdev_dbg(ndev, "CRC error\n");
> + ndev->stats.rx_errors++;
> + if (skb) {
> + cf->data[2] |= CAN_ERR_PROT_BIT;
> + cf->data[3] = CAN_ERR_PROT_LOC_CRC_SEQ;
> + }
> + break;
> +
> + default:
> + break;
> + }
> +
> + if (skb) {
> + int err;
> +
> + err = can_rx_offload_queue_timestamp(&priv->offload, skb,
> + priv->timestamp);
> + if (err)
> + ndev->stats.rx_fifo_errors++;
> + }
> +}
> +
> +static irqreturn_t bxcan_state_change_isr(int irq, void *dev_id)
> +{
> + struct net_device *ndev = dev_id;
> + struct bxcan_priv *priv = netdev_priv(ndev);
> + struct bxcan_regs __iomem *regs = priv->regs;
> + u32 msr, esr;
> +
> + msr = readl(®s->msr);
> + if (!(msr & BXCAN_MSR_ERRI))
> + return IRQ_NONE;
No IRQ, the driver returns IRQ_NONE here? Looks good!
> +
> + esr = readl(®s->esr);
> + bxcan_handle_state_change(ndev, esr);
> +
> + if (priv->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING)
> + bxcan_handle_bus_err(ndev, esr);
> +
> + msr |= BXCAN_MSR_ERRI;
> + writel(msr, ®s->msr);
> + can_rx_offload_irq_finish(&priv->offload);
> +
> + return IRQ_HANDLED;
> +}
regards,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung Nürnberg | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Attachment:
signature.asc
Description: PGP signature
