[Intel-gfx] [PATCH 2/2] drm/i915: reword documentation of possible pci_device_id struct

[Lucas De Marchi]

On Tue, Aug 28, 2018 at 09:06:15PM +0300, Ville Syrjälä wrote:
> On Tue, Aug 28, 2018 at 10:41:46AM -0700, Lucas De Marchi wrote:
> > Document it like a real struct for ease of copy and paste, remove
> > comment of C99 compatibility and document that in some cases the first 2
> > fields can be u16.
> > 
> > Cc: Chris Wilson <chris at chris-wilson.co.uk>
> > Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
> > ---
> >  include/drm/i915_pciids.h | 16 +++++++++-------
> >  1 file changed, 9 insertions(+), 7 deletions(-)
> > 
> > diff --git a/include/drm/i915_pciids.h b/include/drm/i915_pciids.h
> > index 754ce4b10129..0c2cc43f916c 100644
> > --- a/include/drm/i915_pciids.h
> > +++ b/include/drm/i915_pciids.h
> > @@ -26,14 +26,16 @@
> >  #define _I915_PCIIDS_H
> >  
> >  /*
> > - * A pci_device_id struct {
> > - *	__u32 vendor, device;
> > - *      __u32 subvendor, subdevice;
> > - *	__u32 class, class_mask;
> > - *	kernel_ulong_t driver_data;
> > + * These macros can be used with a struct declared like this:
> > + *
> > + * struct pci_device_id {
> > + * 	__u32 vendor, device;
> > + * 	__u32 subvendor, subdevice;
> > + * 	__u32 class, class_mask;
> > + * 	kernel_ulong_t driver_data;
> >   * };
> > - * Don't use C99 here because "class" is reserved and we want to
> > - * give userspace flexibility.
> > + *
> > + * First two fields may be __u16 if PCI_DEVICE_ANY is not used
> 
> PCI_DEVICE_ANY undefined?

this should be PCI_ANY_ID, but now I'm not sure if I should even mention
it since it's defined somewhere that's private to kernel.

> 
> Also you can surely use u16 just fine as long as you're careful when
> comparing with ~0?

nops, since ~0u is unsigned int (~0 being int before patch 1), it will
overflow and cause a warning due to -Woverflow. This is what happens in
igt if I do what you said:

../intel/i915_pciids.h:40:2: warning: conversion from ‘unsigned int’ to ‘short unsigned int’ changes value from ‘4294967295’ to ‘65535’ [-Woverflow]
  ~0u, ~0u,    \
  ^


Lucas De Marchi

> 
> >   */
> >  #define INTEL_VGA_DEVICE(id, info) {		\
> >  	0x8086,	id,				\
> > -- 
> > 2.17.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel