Re: [PATCH] PCI: Unify ECAM constants in native PCI Express drivers

From: "Krzysztof Wilczyński" <kw@linux.com>
To: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>,
	Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
	Heiko Stuebner <heiko@sntech.de>,
	linux-pci@vger.kernel.org, Shawn Lin <shawn.lin@rock-chips.com>,
	Michal Simek <michal.simek@xilinx.com>,
	linux-rockchip@lists.infradead.org,
	Zhou Wang <wangzhou1@hisilicon.com>,
	Robert Richter <rrichter@marvell.com>,
	Jonathan Chocron <jonnyc@amazon.com>,
	Will Deacon <will@kernel.org>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] PCI: Unify ECAM constants in native PCI Express drivers
Date: Sat, 5 Sep 2020 22:44:16 +0200	[thread overview]
Message-ID: <20200905204416.GA83847@rocinante> (raw)
In-Reply-To: <20200828100843.0000474e@Huawei.com>

Hello Jonathan,

Thank you for the review!  Also, apologies for late reply.

On 20-08-28 10:08:43, Jonathan Cameron wrote:
[...]
> 
> Might potentially be worth tidying up the masks as well?
> Or potentially drop them given I suspect that there are no cases
> in which the mask is actually doing anything...

Just to confirm - you have the following constants in mind?

drivers/pci/controller/pcie-rockchip.h:

#define PCIE_ECAM_BUS(x)	(((x) & 0xff) << 20)
#define PCIE_ECAM_DEV(x)	(((x) & 0x1f) << 15)
#define PCIE_ECAM_FUNC(x)	(((x) & 0x7) << 12)

drivers/pci/controller/dwc/pcie-al.c:

#define PCIE_ECAM_DEVFN(x)	(((x) & 0xff) << 12)

I can move PCIE_ECAM_BUS, PCIE_ECAM_DEV and PCIE_ECAM_FUNC (as
PCIE_ECAM_FUN) to the linux/pci-ecam.h file, as these seem useful, but
without the masks, and then update other files to use these.  We could
then leverage these, for example:

 	pci_base_addr = (void __iomem *)((uintptr_t)pp->va_cfg0_base +
-					 (busnr_ecam << 20) +
-					 PCIE_ECAM_DEVFN(devfn));
+					 PCIE_ECAM_BUS(busnr_ecam) +
+					 PCIE_ECAM_FUN(devfn));

What do you think?  Bjorn, would that be acceptable?

Krzysztof