Hi Marc,
> > > +struct nct6694_can_priv {
> > > + struct can_priv can; /* must be the first member */
> > > + struct net_device *ndev;
> > > + struct nct6694 *nct6694;
> > > + struct mutex lock;
> >
> > What does lock protect?
> >
>
> The lock is used to protect tx_buf and rx_buf for each CAN device.
>
> > > + struct sk_buff *tx_skb;
> > > + struct workqueue_struct *wq;
> > > + struct work_struct tx_work;
> > > + unsigned char *tx_buf;
> > void *
> > > + unsigned char *rx_buf;
> > void *
> > > + unsigned char can_idx;
> > > + bool tx_busy;
> >
> > IMHO it makes no sense to have tx_skb and tx_busy
> >
>
> Okay! I will revisit these to evaluate whether they are still necessary.
>
> > > +};
> > > +
I think there needs to be a tx_skb to record the skb passed by
start_xmit(), otherwise it can't handle the can_frame in tx_work. If
this is not necessary, could you please explain?
In addition, the tx flow is based on the implementation in
https://elixir.bootlin.com/linux/v6.12.6/source/drivers/net/can/spi/mcp251x.c
Thanks,
Ming
