[PATCH i-g-t] lib: sync PCI ID macros with kernel
Zbigniew Kempczyński
zbigniew.kempczynski at intel.com
Wed Nov 13 06:48:49 UTC 2024
On Tue, Nov 12, 2024 at 03:02:10PM +0200, Jani Nikula wrote:
<cut>
> >
> > I see no __KERNEL__ in current xe_pciids.h and i915_pciids.h so
> > there's some change from my perspective. intel_vbt_defs.h also
> > doesn't contain any kernel conditional inside. I think author
> > of this change should provide additional documentation about this
> > (not me) - commit message for me is not enough. Till this change
> > I haven't noticed any __KERNEL__ usage in igt codebase and I think
> > if it appears it should be documented what's for dead code is
> > imported to the project.
>
> See igt commit e966143f5c5f ("lib/intel_device_info: use dedicated macro
> for struct pci_id_match init"). INTEL_VGA_DEVICE has been unused in igt
> since then.
>
> I wrapped the macro in #ifdef __KERNEL__ in kernel commit fc9cb46bdca8
> ("drm/i915/pciids: use designated initializers in INTEL_VGA_DEVICE()")
> to avoid it being used in igt again.
Rationale of your change is clear to me - isolation of INTEL_VGA_DEVICE()
to use in the kernel only. My problems are when I see such thing in the
igt header
#ifdef __KERNEL__
... some definitions ...
#endif
1. is someone really is setting up __KERNEL__ in igt build system?
What's for it is here? (cognitive problem)
2. this conditional is not necessary here, let's remove it.
(dead code problem)
I guess you will not accept change in which I would remove __KERNEL__
conditional from the header (2).
>
> I'm not going to document this further in igt.
I'm asking for the documenting of this change not for me, but for
other developers who might be confused by this conditional in the
future. Especially we want to copy this header from the kernel.
If you still think this is not necessary I have no other arguments
to convince you to add some note to README.md.
--
Zbigniew
>
>
> BR,
> Jani.
>
>
> --
> Jani Nikula, Intel
More information about the igt-dev
mailing list