On 2025/6/24 16:44, Jan Beulich wrote:
> On 24.06.2025 10:32, Chen, Jiqian wrote:
>> 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)
>
> Well, yes, in the helper macro you may need to take precautions. However, I
> was
> wondering anyway why
>
> REGISTER_VPCI_CAP(PCI_CAP_ID_MSI, init_msi, NULL);
>
> would be necessary, when
>
> REGISTER_VPCI_CAP(MSI, init_msi, NULL);
>
> could do, using e.g.
>
> #define REGISTER_VPCI_CAP(cap, finit, fclean) \
> REGISTER_VPCI_CAPABILITY(PCI_CAP_ID_##cap, finit, fclean, false)
> #define REGISTER_VPCI_EXTCAP(cap, finit, fclean) \
> REGISTER_VPCI_CAPABILITY(PCI_EXT_CAP_ID_##cap, finit, fclean, true)
>
> (other variations are possible, of course). Then you could easily derive the
> identifier wanted (requiring another parameter to REGISTER_VPCI_CAPABILITY(),
> yes).
Seems better.
Thank you very much!
>
> Jan
--
Best regards,
Jiqian Chen.
