Re: [PATCH v3 1/7] mfd: Add core driver for Nuvoton NCT6694

[Christophe JAILLET]

Le 12/12/2024 à 08:01, Ming Yu a écrit :
> Dear Christophe,
>
> Thank you for your comments,
>
> Christophe JAILLET <christophe.jaillet-39ZsbGIQGT5GWvitb5QawA@xxxxxxxxxxxxxxxx> 於 2024年12月12日 週四 上午12:44寫道：
> > > +int nct6694_read_msg(struct nct6694 *nct6694, u8 mod, u16 offset,
> > > +                  u16 length, void *buf)
> > > +{
> > > +     struct nct6694_cmd_header *cmd_header = nct6694->cmd_header;
> > > +     struct nct6694_response_header *response_header = nct6694->response_header;
> > > +     struct usb_device *udev = nct6694->udev;
> > > +     int tx_len, rx_len, ret;
> > > +
> > > +     guard(mutex)(&nct6694->access_lock);
> >
> > Nitpick: This could be moved a few lines below, should it still comply
> > with your coding style.
>
> I think the lock should be placed here to prevent the cmd_header from
> being overwritten by another caller.
> Could you share your perspective on this?

You are right, I misread the code :(
(I though cmd_header was a local structure)

> > > +
> > > +     /* Send command packet to USB device */
> > > +     cmd_header->mod = mod;
> > > +     cmd_header->cmd = offset & 0xFF;
> > > +     cmd_header->sel = (offset >> 8) & 0xFF;
> > > +     cmd_header->hctrl = NCT6694_HCTRL_GET;
> > > +     cmd_header->len = length;

CJ