On 23.09.2022 21:45:04, dariobin@xxxxxxxxx wrote:
> I didn't understand if you applied two patches separately or not.
> This was the only patch I had thought of. Which was similar to Marc's
> for the interrupt part but differed in the c_can_start_xmit part.
>
> --- a/drivers/net/can/c_can/c_can_main.c
> +++ b/drivers/net/can/c_can/c_can_main.c
> @@ -464,13 +464,19 @@ static netdev_tx_t c_can_start_xmit(struct sk_buff *skb,
> return NETDEV_TX_BUSY;
>
> idx = c_can_get_tx_head(tx_ring);
> + if (idx < c_can_get_tx_tail(tx_ring)) {
> + if (priv->type == BOSCH_D_CAN) {
> + cmd &= ~IF_COMM_TXRQST; /* Cache the message */
> + } else {
> + netif_stop_queue(priv->dev);
> + return NETDEV_TX_BUSY;
This works, but it is frowned upon. You intentionally omit adequate flow
control for the C_CAN core. The overhead for starting the
ndo_start_xmit() callback and aborting it with ETDEV_TX_BUSY is
considered too high.
> + }
> + }
> +
> tx_ring->head++;
> if (c_can_get_tx_free(tx_ring) == 0)
> netif_stop_queue(dev);
>
> - if (idx < c_can_get_tx_tail(tx_ring))
> - cmd &= ~IF_COMM_TXRQST; /* Cache the message */
> -
> /* Store the message in the interface so we can call
> * can_put_echo_skb(). We must do this before we enable
> * transmit as we might race against do_tx().
> @@ -723,7 +729,6 @@ static void c_can_do_tx(struct net_device *dev)
> struct c_can_tx_ring *tx_ring = &priv->tx;
> struct net_device_stats *stats = &dev->stats;
> u32 idx, obj, pkts = 0, bytes = 0, pend;
> - u8 tail;
>
> if (priv->msg_obj_tx_last > 32)
> pend = priv->read_reg32(priv, C_CAN_INTPND3_REG);
> @@ -759,15 +764,20 @@ static void c_can_do_tx(struct net_device *dev)
> stats->tx_bytes += bytes;
> stats->tx_packets += pkts;
>
> - tail = c_can_get_tx_tail(tx_ring);
> + if (priv->type == BOSCH_D_CAN) {
> + u8 tail;
> +
> + tail = c_can_get_tx_tail(tx_ring);
>
> - if (tail == 0) {
> - u8 head = c_can_get_tx_head(tx_ring);
> + if (tail == 0) {
> + u8 head = c_can_get_tx_head(tx_ring);
>
> - /* Start transmission for all cached messages */
> - for (idx = tail; idx < head; idx++) {
> - obj = idx + priv->msg_obj_tx_first;
> - c_can_object_put(dev, IF_NAPI, obj, IF_COMM_TXRQST);
> + /* Start transmission for all cached messages */
> + for (idx = tail; idx < head; idx++) {
> + obj = idx + priv->msg_obj_tx_first;
> + c_can_object_put(dev, IF_NAPI, obj,
> + IF_COMM_TXRQST);
> + }
> }
> }
> }
>
> I changed it a little from the previous email because I noticed a problem.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung West/Dortmund | Phone: +49-231-2826-924 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Attachment:
signature.asc
Description: PGP signature
