Re: [PATCH v10 1/2] PCI: Introduce named defines for pci rom

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

On Fri, 12 Dec 2025, Guixin Liu wrote:

The subject still seems to have lowercase "pci rom" :-(.

-- 
 i.

> Convert the magic numbers associated with PCI ROM into named
> definitions. Some of these definitions will be used in the second
> fix patch.
> 
> Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
> ---
>  drivers/pci/rom.c | 38 +++++++++++++++++++++++++++-----------
>  1 file changed, 27 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/pci/rom.c b/drivers/pci/rom.c
> index e18d3a4383ba..4f7641b93b4b 100644
> --- a/drivers/pci/rom.c
> +++ b/drivers/pci/rom.c
> @@ -5,13 +5,28 @@
>   * (C) Copyright 2004 Jon Smirl <jonsmirl@yahoo.com>
>   * (C) Copyright 2004 Silicon Graphics, Inc. Jesse Barnes <jbarnes@sgi.com>
>   */
> +
> +#include <linux/bits.h>
>  #include <linux/kernel.h>
>  #include <linux/export.h>
>  #include <linux/pci.h>
> +#include <linux/sizes.h>
>  #include <linux/slab.h>
>  
>  #include "pci.h"
>  
> +#define PCI_ROM_HEADER_SIZE			0x1A
> +#define PCI_ROM_POINTER_TO_DATA_STRUCT		0x18
> +#define PCI_ROM_LAST_IMAGE_INDICATOR		0x15
> +#define PCI_ROM_LAST_IMAGE_INDICATOR_BIT	BIT(7)
> +#define PCI_ROM_IMAGE_LEN			0x10
> +#define PCI_ROM_IMAGE_SECTOR_SIZE		SZ_512
> +#define PCI_ROM_IMAGE_SIGNATURE			0xAA55
> +
> +/* Data structure signature is "PCIR" in ASCII representation */
> +#define PCI_ROM_DATA_STRUCT_SIGNATURE		0x52494350
> +#define PCI_ROM_DATA_STRUCT_LEN			0x0A
> +
>  /**
>   * pci_enable_rom - enable ROM decoding for a PCI device
>   * @pdev: PCI device to enable
> @@ -91,26 +106,27 @@ static size_t pci_get_rom_size(struct pci_dev *pdev, void __iomem *rom,
>  	do {
>  		void __iomem *pds;
>  		/* Standard PCI ROMs start out with these bytes 55 AA */
> -		if (readw(image) != 0xAA55) {
> -			pci_info(pdev, "Invalid PCI ROM header signature: expecting 0xaa55, got %#06x\n",
> -				 readw(image));
> +		if (readw(image) != PCI_ROM_IMAGE_SIGNATURE) {
> +			pci_info(pdev, "Invalid PCI ROM header signature: expecting %#06x, got %#06x\n",
> +				 PCI_ROM_IMAGE_SIGNATURE, readw(image));
>  			break;
>  		}
>  		/* get the PCI data structure and check its "PCIR" signature */
> -		pds = image + readw(image + 24);
> -		if (readl(pds) != 0x52494350) {
> -			pci_info(pdev, "Invalid PCI ROM data signature: expecting 0x52494350, got %#010x\n",
> -				 readl(pds));
> +		pds = image + readw(image + PCI_ROM_POINTER_TO_DATA_STRUCT);
> +		if (readl(pds) != PCI_ROM_DATA_STRUCT_SIGNATURE) {
> +			pci_info(pdev, "Invalid PCI ROM data signature: expecting %#010x, got %#010x\n",
> +				 PCI_ROM_DATA_STRUCT_SIGNATURE, readl(pds));
>  			break;
>  		}
> -		last_image = readb(pds + 21) & 0x80;
> -		length = readw(pds + 16);
> -		image += length * 512;
> +		last_image = readb(pds + PCI_ROM_LAST_IMAGE_INDICATOR) &
> +				   PCI_ROM_LAST_IMAGE_INDICATOR_BIT;
> +		length = readw(pds + PCI_ROM_IMAGE_LEN);
> +		image += length * PCI_ROM_IMAGE_SECTOR_SIZE;
>  		/* Avoid iterating through memory outside the resource window */
>  		if (image >= rom + size)
>  			break;
>  		if (!last_image) {
> -			if (readw(image) != 0xAA55) {
> +			if (readw(image) != PCI_ROM_IMAGE_SIGNATURE) {
>  				pci_info(pdev, "No more image in the PCI ROM\n");
>  				break;
>  			}
>