Re: [PATCH v6 2/8] vpci: Refactor REGISTER_VPCI_INIT

["Chen, Jiqian" <Jiqian.Chen@xxxxxxx>]

On 2025/6/24 16:28, Jan Beulich wrote:
> On 24.06.2025 10:26, Chen, Jiqian wrote:
>> On 2025/6/24 16:17, Jan Beulich wrote:
>>> On 24.06.2025 10:12, Chen, Jiqian wrote:
>>>> On 2025/6/20 14:34, Jan Beulich wrote:
>>>>> On 19.06.2025 08:14, Chen, Jiqian wrote:
>>>>>> On 2025/6/18 22:33, Jan Beulich wrote:
>>>>>>> On 12.06.2025 11:29, Jiqian Chen wrote:
>>>>>>>> +} vpci_capability_t;
>>>>>>>
>>>>>>> As you have it here, ...
>>>>>>>
>>>>>>>> @@ -29,9 +30,22 @@ typedef int vpci_register_init_t(struct pci_dev 
>>>>>>>> *dev);
>>>>>>>>   */
>>>>>>>>  #define VPCI_MAX_VIRT_DEV       (PCI_SLOT(~0) + 1)
>>>>>>>>  
>>>>>>>> -#define REGISTER_VPCI_INIT(x, p)                \
>>>>>>>> -  static vpci_register_init_t *const x##_entry  \
>>>>>>>> -               __used_section(".data.vpci." p) = (x)
>>>>>>>> +#define REGISTER_VPCI_CAPABILITY(cap, finit, fclean, ext) \
>>>>>>>> +    static const vpci_capability_t finit##_t = { \
>>>>>>>
>>>>>>> ... _t suffixes generally designate types. I don't think we should abuse
>>>>>>> that suffix for an identifier of a variable.
>>>>>> What do you think I should change to?
>>>>>
>>>>> Well, if you take my other advice, this question won't need answering, as
>>>>> then you only need the ..._entry one.
>>>>>
>>>>> Btw, noticing only now - why is it finit that's used to derive the 
>>>>> identifier?
>>>>> With that, it could as well be fclean (leaving aside the fact that that's
>>>>> optional). Imo the name would better be derived from cap, and it would 
>>>>> better
>>>>> also reflect the purpose of the variable.
>>>> I considered this.
>>>> I think it is easier to use finit, and finit contains the cap type, and 
>>>> the main purpose of this struct is to initialize the cap.
>>>
>>> Yet identifier names should make sense for the object they name.
>> OK. What's your suggestion about naming the entry?
> 
> cap##_init or _##cap##_init for example.
If so, I need to extend the parameter of REGISTER_VPCI_CAPABILITY since current 
cap is number, not string.
Maybe:
REGISTER_VPCI_CAPABILITY (cap, cap_id, finit, fclean, ext)

> 
> Jan

-- 
Best regards,
Jiqian Chen.