Re: [PATCH v3 4/7] can: Add Nuvoton NCT6694 CAN support

[Ming Yu <a0282524688@xxxxxxxxx>]

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