Re: [RFC] [PATCH] PCI mmconfig without ACPI

[Len Brown <lenb@kernel.org>]

> > +#ifdef CONFIG_PCI_MMCONFIG
> > +
> > +struct acpi_mcfg_allocation {
> > +	u64 address;		/* Base address, processor-relative */
> > +	u16 pci_segment;	/* PCI segment group number */
> > +	u8 start_bus_number;	/* Starting PCI Bus number */
> > +	u8 end_bus_number;	/* Final PCI Bus number */
> > +	u32 reserved;
> > +};
> 
> Please rename this to "struct pci_mcfg_allocation" - there's nothing ACPI 
> about it anymore - mmcfg is a PCI feature and ACPI is an enumeration method.
> 
> Also, while touching it, please also use the opportunity to align structure 
> fields vertically:
> 
>  struct pci_mcfg_allocation {
> 	u64	address;		/* Base address, processor-relative */
> 	u16	pci_segment;		/* PCI segment group number */
> 	u8	start_bus_number;	/* Starting PCI Bus number */
> 	u8	end_bus_number;		/* Final PCI Bus number */
> 	u32	__reserved;
>  };
> 
> The whole layout of this structure becomes easier to read and nicer to look 
> at as well.
> 
> Another small detail: note how i renamed reserved to __reserved - that is a 
> standard way to de-emphasise the signficance of a structure field.
> 
> The reserved field there is for future expansion and to pad the structure to 
> 16 bytes - it doesnt really mean much and the underscores move it a bit out 
> of the default line of sight.
> 
> With a 'reserved' field people end up wondering whether it's perhaps some 
> _semantic_ 'reserved area' kind of thing (like for e820 maps, etc.) - so 
> it's never bad to make that distinction explicit via the double underscores.

struct acpi_mcfg_allocation is the structure that maps onto the MCFG
ACPI table as defined in the PCI firmware spec and provided by the ACPI 
BIOS.

I'd like it to stay in actbl1.h -- as that is part of ACPICA, which
tracks the standard tables.  (and I see you did this with your updated 
patch, thanks.)

FWIW, "reserved" here really does have a specific definition.
On read-only tables, such as this one, reserved fields are defined
to return 0 on reads for this version of the table, but may
return non-zero on future revisions.

thanks,
-Len