On 04/02/2025 at 12:24, Ming Yu wrote:
> Dear Vincent,
>
> Thank you for reviewing,
> I will address the issues you mentioned in the next patch.
>
> Vincent Mailhol <mailhol.vincent@xxxxxxxxxx> 於 2025年1月26日 週日 下午4:47寫道：
>>
> ...
>>> +static int nct6694_can_get_clock(struct nct6694_can_priv *priv)
>>> +{
>>> + struct nct6694_can_information *info;
>>> + struct nct6694_cmd_header cmd_hd = {
>>
>> If the variable only has constant initializer, make it static const:
>>
>> static const struct nct6694_cmd_header cmd_hd = {
>>
>> Apply this at other locations in your different modules.
>>
>>> + .mod = NCT6694_CAN_MOD,
>>> + .cmd = NCT6694_CAN_INFORMATION,
>>> + .sel = NCT6694_CAN_INFORMATION_SEL,
>>> + .len = cpu_to_le16(sizeof(*info))
>>> + };
>>> + int ret, can_clk;
>>> +
>>> + info = kzalloc(sizeof(*info), GFP_KERNEL);
>>> + if (!info)
>>> + return -ENOMEM;
>>> +
>
> Excuse me, I would like to confirm, if the variable is constant
> initializer, should the declaration be written as:
> static const struct nct6694_cmd_header cmd_hd = {
> .mod = NCT6694_CAN_MOD,
> .cmd = NCT6694_CAN_INFORMATION,
> .sel = NCT6694_CAN_INFORMATION_SEL,
> .len = cpu_to_le16(sizeof(struct nct6694_can_information))
> };
> instead of:
> static const struct nct6694_cmd_header cmd_hd = {
> .mod = NCT6694_CAN_MOD,
> .cmd = NCT6694_CAN_INFORMATION,
> .sel = NCT6694_CAN_INFORMATION_SEL,
> .len = cpu_to_le16(sizeof(*info))
> };
> , correct?
The sizeof() keyword returns a integer constant expression even if
applied on to a variable (unless that variable is a variable length
array, but these are banned in the kernel anyway). This is because only
the type of the variable matters, and that is known at compile time.
So, cpu_to_le16(sizeof(*info)) should work fine. Or are you getting any
error?
> In addition, does this mean that the parameter in nct6694_read_msg()
> and nct6694_write_msg() should be changed to const struct
> nct6694_cmd_header *cmd_hd?
Yes!
Yours sincerely,
Vincent Mailhol
