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

Christophe JAILLET
2024-12-12T07:13:55Z

Le 12/12/2024 à 08:01, Ming Yu a écrit :
> Dear Christophe,
>
> Thank you for your comments,
>
> Christophe JAILLET <christophe.jaillet-39ZsbGIQGT5GWvitb5QawA@public.gmane.org> 於 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