Re: [PATCH] PCI: Add PCI_RID_MASK macro

[Aksh Garg]

Marc, Lukas,

On 16/02/26 18:56, Marc Zyngier wrote:
> On Mon, 16 Feb 2026 12:06:00 +0000,
> Aksh Garg <a-garg7@xxxxxx> wrote:
> > Routing ID (RID) is a 16-bit value composed of bus number and devfn.
>
> No. RID here means "Requester ID". Please lookup the PCI spec.
>
> > Add a PCI_RID_MASK macro to eliminate the use of magic number 0xffff
> > when masking for RIDs.
>
> I'm not convinced this is universally true. Some RIDs (or RID-like
> identifiers, such as Stream IDs and Device IDs) are larger than 16
> bit, depending on the integration.
>
> The fact that this particular integration uses a 16bit RID without any
> extra qualifier in the upper bits doesn't make it universal.
>
> > This provides a standard way for drivers to extract RIDs without
> > resorting to magic numbers.
> >
> > Signed-off-by: Aksh Garg <a-garg7@xxxxxx>
> > ---
> >   drivers/pci/controller/pcie-apple.c | 2 +-
> >   include/linux/pci.h                 | 1 +
> >   2 files changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/pci/controller/pcie-apple.c b/drivers/pci/controller/pcie-apple.c
> > index 2d92fc79f6dd..6bf9fe102e2f 100644
> > --- a/drivers/pci/controller/pcie-apple.c
> > +++ b/drivers/pci/controller/pcie-apple.c
> > @@ -802,7 +802,7 @@ static void apple_pcie_disable_device(struct pci_host_bridge *bridge, struct pci
> >   		u32 val;
> >   
> >   		val = readl_relaxed(port_rid2sid_addr(port, idx));
> > -		if ((val & 0xffff) == rid) {
> > +		if ((val & PCI_RID_MASK) == rid) {
> >   			apple_pcie_rid2sid_write(port, idx, 0);
> >   			bitmap_release_region(port->sid_map, idx, 0);
> >   			dev_dbg(&pdev->dev, "Released %x (%d)\n", val, idx);
> > diff --git a/include/linux/pci.h b/include/linux/pci.h
> > index 864775651c6f..e3d9730121c8 100644
> > --- a/include/linux/pci.h
> > +++ b/include/linux/pci.h
> > @@ -71,6 +71,7 @@
> >   #define PCI_DEVID(bus, devfn)	((((u16)(bus)) << 8) | (devfn))
> >   /* return bus from PCI devid = ((u16)bus_number) << 8) | devfn */
> >   #define PCI_BUS_NUM(x) (((x) >> 8) & 0xff)
> > +#define PCI_RID_MASK	0xffff
>
> Even the naming of this macro clashes with the current nomenclature,
> where we use PCI_DEVID()/pci_dev_id() to manipulate this information.

Is the below proposed macro acceptable at the same place:
#define PCI_GET_DEVID(x) ((x) & 0xffff)

Regards,
Aksh Garg

> Thanks,
>
> 	M.