On Wed, Mar 15, 2023 at 12:05:43PM +0100, Markus Schneider-Pargmann wrote:
> Keep track of the number of transmits in flight.
>
> This patch prepares the driver to control the network interface queue
> based on this counter. By itself this counter be
> implemented with an atomic, but as we need to do other things in the
> critical sections later I am using a spinlock instead.
>
> Signed-off-by: Markus Schneider-Pargmann <msp@xxxxxxxxxxxx>
Nit, assuming the values are always positive, I think
that unsigned might be a more appropriate type than int
for the tx_fifo_in_flight field, and associated function
parameters and local variables.
That notwithstanding,
Reviewed-by: Simon Horman <simon.horman@xxxxxxxxxxxx>
> ---
> drivers/net/can/m_can/m_can.c | 41 ++++++++++++++++++++++++++++++++++-
> drivers/net/can/m_can/m_can.h | 4 ++++
> 2 files changed, 44 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
> index 27d36bcc094c..4ad8f08f8284 100644
> --- a/drivers/net/can/m_can/m_can.c
> +++ b/drivers/net/can/m_can/m_can.c
> @@ -442,6 +442,7 @@ static u32 m_can_get_timestamp(struct m_can_classdev *cdev)
> static void m_can_clean(struct net_device *net)
> {
> struct m_can_classdev *cdev = netdev_priv(net);
> + unsigned long irqflags;
>
> for (int i = 0; i != cdev->tx_fifo_size; ++i) {
> if (!cdev->tx_ops[i].skb)
> @@ -453,6 +454,10 @@ static void m_can_clean(struct net_device *net)
>
> for (int i = 0; i != cdev->can.echo_skb_max; ++i)
> can_free_echo_skb(cdev->net, i, NULL);
> +
> + spin_lock_irqsave(&cdev->tx_handling_spinlock, irqflags);
> + cdev->tx_fifo_in_flight = 0;
> + spin_unlock_irqrestore(&cdev->tx_handling_spinlock, irqflags);
> }
>
> /* For peripherals, pass skb to rx-offload, which will push skb from
> @@ -1023,6 +1028,24 @@ static void m_can_tx_update_stats(struct m_can_classdev *cdev,
> stats->tx_packets++;
> }
>
> +static void m_can_finish_tx(struct m_can_classdev *cdev, int transmitted)
> +{
> + unsigned long irqflags;
> +
> + spin_lock_irqsave(&cdev->tx_handling_spinlock, irqflags);
> + cdev->tx_fifo_in_flight -= transmitted;
> + spin_unlock_irqrestore(&cdev->tx_handling_spinlock, irqflags);
> +}
> +
> +static void m_can_start_tx(struct m_can_classdev *cdev)
> +{
> + unsigned long irqflags;
> +
> + spin_lock_irqsave(&cdev->tx_handling_spinlock, irqflags);
> + ++cdev->tx_fifo_in_flight;
> + spin_unlock_irqrestore(&cdev->tx_handling_spinlock, irqflags);
> +}
> +
> static int m_can_echo_tx_event(struct net_device *dev)
> {
> u32 txe_count = 0;
> @@ -1032,6 +1055,7 @@ static int m_can_echo_tx_event(struct net_device *dev)
> int i = 0;
> int err = 0;
> unsigned int msg_mark;
> + int processed = 0;
>
> struct m_can_classdev *cdev = netdev_priv(dev);
>
> @@ -1061,12 +1085,15 @@ static int m_can_echo_tx_event(struct net_device *dev)
>
> /* update stats */
> m_can_tx_update_stats(cdev, msg_mark, timestamp);
> + ++processed;
> }
>
> if (ack_fgi != -1)
> m_can_write(cdev, M_CAN_TXEFA, FIELD_PREP(TXEFA_EFAI_MASK,
> ack_fgi));
>
> + m_can_finish_tx(cdev, processed);
> +
> return err;
> }
>
> @@ -1161,6 +1188,7 @@ static irqreturn_t m_can_isr(int irq, void *dev_id)
> timestamp = m_can_get_timestamp(cdev);
> m_can_tx_update_stats(cdev, 0, timestamp);
> netif_wake_queue(dev);
> + m_can_finish_tx(cdev, 1);
> }
> } else {
> if (ir & (IR_TEFN | IR_TEFW)) {
> @@ -1846,11 +1874,22 @@ static netdev_tx_t m_can_start_peripheral_xmit(struct m_can_classdev *cdev,
> }
>
> netif_stop_queue(cdev->net);
> +
> + m_can_start_tx(cdev);
> +
> m_can_tx_queue_skb(cdev, skb);
>
> return NETDEV_TX_OK;
> }
>
> +static netdev_tx_t m_can_start_fast_xmit(struct m_can_classdev *cdev,
> + struct sk_buff *skb)
> +{
> + m_can_start_tx(cdev);
> +
> + return m_can_tx_handler(cdev, skb);
> +}
> +
> static netdev_tx_t m_can_start_xmit(struct sk_buff *skb,
> struct net_device *dev)
> {
> @@ -1862,7 +1901,7 @@ static netdev_tx_t m_can_start_xmit(struct sk_buff *skb,
> if (cdev->is_peripheral)
> return m_can_start_peripheral_xmit(cdev, skb);
> else
> - return m_can_tx_handler(cdev, skb);
> + return m_can_start_fast_xmit(cdev, skb);
> }
>
> static int m_can_open(struct net_device *dev)
> diff --git a/drivers/net/can/m_can/m_can.h b/drivers/net/can/m_can/m_can.h
> index 2e1a52980a18..e230cf320a6c 100644
> --- a/drivers/net/can/m_can/m_can.h
> +++ b/drivers/net/can/m_can/m_can.h
> @@ -109,6 +109,10 @@ struct m_can_classdev {
> // Store this internally to avoid fetch delays on peripheral chips
> int tx_fifo_putidx;
>
> + /* Protects shared state between start_xmit and m_can_isr */
> + spinlock_t tx_handling_spinlock;
> + int tx_fifo_in_flight;
> +
> struct m_can_tx_op *tx_ops;
> int tx_fifo_size;
> int next_tx_op;
> --
> 2.39.2
>
