Re: [PATCH v6 1/5] PCI: Unify ECAM constants in native PCI Express drivers
On Sun, Dec 6, 2020 at 10:25 PM Florian Fainelli wrote:
>
> +JimQ,
>
> On 12/6/2020 12:16 PM, Krzysztof Wilczyński wrote:
> > Hello Nicolas, Florian and Florian,
> >
> > [...]
> >> -/* Configuration space read/write support */
> >> -static inline int brcm_pcie_cfg_index(int busnr, int devfn, int reg)
> >> -{
> >> -return ((PCI_SLOT(devfn) & 0x1f) << PCIE_EXT_SLOT_SHIFT)
> >> -| ((PCI_FUNC(devfn) & 0x07) << PCIE_EXT_FUNC_SHIFT)
> >> -| (busnr << PCIE_EXT_BUSNUM_SHIFT)
> >> -| (reg & ~3);
> >> -}
> >> -
> >> static void __iomem *brcm_pcie_map_conf(struct pci_bus *bus, unsigned int
> >> devfn,
> >> int where)
> >> {
> >> @@ -716,7 +704,7 @@ static void __iomem *brcm_pcie_map_conf(struct pci_bus
> >> *bus, unsigned int devfn,
> >> return PCI_SLOT(devfn) ? NULL : base + where;
> >>
> >> /* For devices, write to the config space index register */
> >> -idx = brcm_pcie_cfg_index(bus->number, devfn, 0);
> >> +idx = PCIE_ECAM_OFFSET(bus->number, devfn, 0);
> >> writel(idx, pcie->base + PCIE_EXT_CFG_INDEX);
> >> return base + PCIE_EXT_CFG_DATA + where;
> >> }
> > [...]
> >
> > Passing the hard-coded 0 as the "reg" argument here never actually did
> > anything, thus the 32 bit alignment was never correctly enforced.
> >
> > My question would be: should this be 32 bit aligned? It seems like the
> > intention was to perhaps make the alignment? I am sadly not intimately
> > familiar with his hardware, so I am not sure if there is something to
> > fix here or not.
Hello Krzystzof,
The value gets assigned to our config-space index register, which has
the lower two bits marked "unused".We're making sure that we are
putting zeroes there but it is most likely not necessary.
> >
> > Also, I wonder whether it would be safe to pass the offset (the "where"
> > variable) rather than hard-coded 0?
The answer is "no" for this code but "maybe" in the future -- allow me
to explain. We have two methods to access the config space:
(1) Set a designated index register to map to the base of a device's
config-space. From then we can access a 4k register set. This is the
method you see in the code and is why we set reg=0 for the index value
and then add "where" to the return address.
(2) Set our index register to the bus/slot/func/reg value, and then we
access a single data register. In this case we do set the "reg" to
the register value to set the index and then only add "where & 0x3" to
the return address.
As it turns out, (1) is not compatible with some MIPs SOCs that we
still support as they do not have the 4k data register set. So I may
be changing to (1) in a future pullreq, and if so, I will invoke
PCIE_ECAM_OFFSET(bus->number, devfn, where & ~3);
Regards,
Jim Quinlan
Broadcom STB
> >
> > Thank you for help in advance!
> >
> > Bjorn also asked the same question:
> >
> > https://lore.kernel.org/linux-pci/20201120203428.GA272511@bjorn-Precision-5520/
> >
> > Krzysztof
> >
>
> --
> Florian
--
This electronic communication and the information and any files transmitted
with it, or attached to it, are confidential and are intended solely for
the use of the individual or entity to whom it is addressed and may contain
information that is confidential, legally privileged, protected by privacy
laws, or otherwise restricted from disclosure to anyone else. If you are
not the intended recipient or the person responsible for delivering the
e-mail to the intended recipient, you are hereby notified that any use,
copying, distributing, dissemination, forwarding, printing, or copying of
this e-mail is strictly prohibited. If you received this e-mail in error,
please return the e-mail to the sender, delete it from your computer, and
destroy any printed copy of it.
smime.p7s
Description: S/MIME Cryptographic Signature
Re: [PATCH v6 1/5] PCI: Unify ECAM constants in native PCI Express drivers
+JimQ,
On 12/6/2020 12:16 PM, Krzysztof Wilczyński wrote:
> Hello Nicolas, Florian and Florian,
>
> [...]
>> -/* Configuration space read/write support */
>> -static inline int brcm_pcie_cfg_index(int busnr, int devfn, int reg)
>> -{
>> -return ((PCI_SLOT(devfn) & 0x1f) << PCIE_EXT_SLOT_SHIFT)
>> -| ((PCI_FUNC(devfn) & 0x07) << PCIE_EXT_FUNC_SHIFT)
>> -| (busnr << PCIE_EXT_BUSNUM_SHIFT)
>> -| (reg & ~3);
>> -}
>> -
>> static void __iomem *brcm_pcie_map_conf(struct pci_bus *bus, unsigned int
>> devfn,
>> int where)
>> {
>> @@ -716,7 +704,7 @@ static void __iomem *brcm_pcie_map_conf(struct pci_bus
>> *bus, unsigned int devfn,
>> return PCI_SLOT(devfn) ? NULL : base + where;
>>
>> /* For devices, write to the config space index register */
>> -idx = brcm_pcie_cfg_index(bus->number, devfn, 0);
>> +idx = PCIE_ECAM_OFFSET(bus->number, devfn, 0);
>> writel(idx, pcie->base + PCIE_EXT_CFG_INDEX);
>> return base + PCIE_EXT_CFG_DATA + where;
>> }
> [...]
>
> Passing the hard-coded 0 as the "reg" argument here never actually did
> anything, thus the 32 bit alignment was never correctly enforced.
>
> My question would be: should this be 32 bit aligned? It seems like the
> intention was to perhaps make the alignment? I am sadly not intimately
> familiar with his hardware, so I am not sure if there is something to
> fix here or not.
>
> Also, I wonder whether it would be safe to pass the offset (the "where"
> variable) rather than hard-coded 0?
>
> Thank you for help in advance!
>
> Bjorn also asked the same question:
>
> https://lore.kernel.org/linux-pci/20201120203428.GA272511@bjorn-Precision-5520/
>
> Krzysztof
>
--
Florian
Re: [PATCH v6 1/5] PCI: Unify ECAM constants in native PCI Express drivers
Hello Nicolas, Florian and Florian,
[...]
> -/* Configuration space read/write support */
> -static inline int brcm_pcie_cfg_index(int busnr, int devfn, int reg)
> -{
> - return ((PCI_SLOT(devfn) & 0x1f) << PCIE_EXT_SLOT_SHIFT)
> - | ((PCI_FUNC(devfn) & 0x07) << PCIE_EXT_FUNC_SHIFT)
> - | (busnr << PCIE_EXT_BUSNUM_SHIFT)
> - | (reg & ~3);
> -}
> -
> static void __iomem *brcm_pcie_map_conf(struct pci_bus *bus, unsigned int
> devfn,
> int where)
> {
> @@ -716,7 +704,7 @@ static void __iomem *brcm_pcie_map_conf(struct pci_bus
> *bus, unsigned int devfn,
> return PCI_SLOT(devfn) ? NULL : base + where;
>
> /* For devices, write to the config space index register */
> - idx = brcm_pcie_cfg_index(bus->number, devfn, 0);
> + idx = PCIE_ECAM_OFFSET(bus->number, devfn, 0);
> writel(idx, pcie->base + PCIE_EXT_CFG_INDEX);
> return base + PCIE_EXT_CFG_DATA + where;
> }
[...]
Passing the hard-coded 0 as the "reg" argument here never actually did
anything, thus the 32 bit alignment was never correctly enforced.
My question would be: should this be 32 bit aligned? It seems like the
intention was to perhaps make the alignment? I am sadly not intimately
familiar with his hardware, so I am not sure if there is something to
fix here or not.
Also, I wonder whether it would be safe to pass the offset (the "where"
variable) rather than hard-coded 0?
Thank you for help in advance!
Bjorn also asked the same question:
https://lore.kernel.org/linux-pci/20201120203428.GA272511@bjorn-Precision-5520/
Krzysztof
Re: [PATCH v6 1/5] PCI: Unify ECAM constants in native PCI Express drivers
On Sun, 2020-11-29 at 23:07 +, Krzysztof Wilczyński wrote:
> Add ECAM-related constants to provide a set of standard constants
> defining memory address shift values to the byte-level address that can
> be used to access the PCI Express Configuration Space, and then move
> native PCI Express controller drivers to use the newly introduced
> definitions retiring driver-specific ones.
>
> Refactor pci_ecam_map_bus() function to use newly added constants so
> that limits to the bus, device function and offset (now limited to 4K as
> per the specification) are in place to prevent the defective or
> malicious caller from supplying incorrect configuration offset and thus
> targeting the wrong device when accessing extended configuration space.
> This refactor also allows for the ".bus_shit" initialisers to be dropped
> when the user is not using a custom value as a default value will be
> used as per the PCI Express Specification.
>
> Suggested-by: Bjorn Helgaas
> Signed-off-by: Krzysztof Wilczyński
> ---
> drivers/pci/controller/dwc/pcie-al.c| 12 ++---
> drivers/pci/controller/dwc/pcie-hisi.c | 2 --
> drivers/pci/controller/pci-aardvark.c | 13 +++---
> drivers/pci/controller/pci-host-generic.c | 1 -
> drivers/pci/controller/pci-thunder-ecam.c | 1 -
> drivers/pci/controller/pcie-brcmstb.c | 16 ++--
> drivers/pci/controller/pcie-rockchip-host.c | 27 ++---
> drivers/pci/controller/pcie-rockchip.h | 8 +-
> drivers/pci/controller/pcie-tango.c | 1 -
> drivers/pci/controller/pcie-xilinx-nwl.c| 9 ++-
> drivers/pci/controller/pcie-xilinx.c| 11 ++---
> drivers/pci/controller/vmd.c| 11 -
> drivers/pci/ecam.c | 23 --
> include/linux/pci-ecam.h| 27 +
> 14 files changed, 73 insertions(+), 89 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-al.c
> b/drivers/pci/controller/dwc/pcie-al.c
> index f973fbca90cf..af9e51ab1af8 100644
> --- a/drivers/pci/controller/dwc/pcie-al.c
> +++ b/drivers/pci/controller/dwc/pcie-al.c
> @@ -76,7 +76,6 @@ static int al_pcie_init(struct pci_config_window *cfg)
> }
>
> const struct pci_ecam_ops al_pcie_ops = {
> - .bus_shift= 20,
> .init = al_pcie_init,
> .pci_ops = {
> .map_bus= al_pcie_map_bus,
> @@ -138,8 +137,6 @@ struct al_pcie {
> struct al_pcie_target_bus_cfg target_bus_cfg;
> };
>
> -#define PCIE_ECAM_DEVFN(x) (((x) & 0xff) << 12)
> -
> #define to_al_pcie(x)dev_get_drvdata((x)->dev)
>
> static inline u32 al_pcie_controller_readl(struct al_pcie *pcie, u32 offset)
> @@ -226,11 +223,6 @@ static void __iomem *al_pcie_conf_addr_map_bus(struct
> pci_bus *bus,
> struct al_pcie_target_bus_cfg *target_bus_cfg = >target_bus_cfg;
> unsigned int busnr_ecam = busnr & target_bus_cfg->ecam_mask;
> unsigned int busnr_reg = busnr & target_bus_cfg->reg_mask;
> - void __iomem *pci_base_addr;
> -
> - pci_base_addr = (void __iomem *)((uintptr_t)pp->va_cfg0_base +
> - (busnr_ecam << 20) +
> - PCIE_ECAM_DEVFN(devfn));
>
> if (busnr_reg != target_bus_cfg->reg_val) {
> dev_dbg(pcie->pci->dev, "Changing target bus busnum val from
> 0x%x to 0x%x\n",
> @@ -241,7 +233,7 @@ static void __iomem *al_pcie_conf_addr_map_bus(struct
> pci_bus *bus,
> target_bus_cfg->reg_mask);
> }
>
> - return pci_base_addr + where;
> + return pp->va_cfg0_base + PCIE_ECAM_OFFSET(busnr_ecam, devfn, where);
> }
>
> static struct pci_ops al_child_pci_ops = {
> @@ -264,7 +256,7 @@ static void al_pcie_config_prepare(struct al_pcie *pcie)
>
> target_bus_cfg = >target_bus_cfg;
>
> - ecam_bus_mask = (pcie->ecam_size >> 20) - 1;
> + ecam_bus_mask = (pcie->ecam_size >> PCIE_ECAM_BUS_SHIFT) - 1;
> if (ecam_bus_mask > 255) {
> dev_warn(pcie->dev, "ECAM window size is larger than 256MB.
> Cutting off at 256\n");
> ecam_bus_mask = 255;
> diff --git a/drivers/pci/controller/dwc/pcie-hisi.c
> b/drivers/pci/controller/dwc/pcie-hisi.c
> index 5ca86796d43a..8fc5960faf28 100644
> --- a/drivers/pci/controller/dwc/pcie-hisi.c
> +++ b/drivers/pci/controller/dwc/pcie-hisi.c
> @@ -100,7 +100,6 @@ static int hisi_pcie_init(struct pci_config_window *cfg)
> }
>
> const struct pci_ecam_ops hisi_pcie_ops = {
> - .bus_shift= 20,
> .init = hisi_pcie_init,
> .pci_ops = {
> .map_bus= hisi_pcie_map_bus,
> @@ -135,7 +134,6 @@ static int hisi_pcie_platform_init(struct
> pci_config_window *cfg)
> }
>
> static const struct pci_ecam_ops hisi_pcie_platform_ops = {
> - .bus_shift= 20,
> .init = hisi_pcie_platform_init,
>
Re: [PATCH v6 1/5] PCI: Unify ECAM constants in native PCI Express drivers
Hi Lorenzo! On 20-11-30 11:08:58, Lorenzo Pieralisi wrote: [...] > > Refactor pci_ecam_map_bus() function to use newly added constants so > > that limits to the bus, device function and offset (now limited to 4K as > > per the specification) are in place to prevent the defective or > > malicious caller from supplying incorrect configuration offset and thus > > targeting the wrong device when accessing extended configuration space. > > This refactor also allows for the ".bus_shit" initialisers to be dropped > > > Nice typo, I'd fix it while applying it though if you don't mind ;-), > no need to resend it. Oh doh! Apologies. :) > Jokes aside, nice piece of work, thanks for that. > > > when the user is not using a custom value as a default value will be > > used as per the PCI Express Specification. > > > > Suggested-by: Bjorn Helgaas > > Signed-off-by: Krzysztof Wilczyński > > I think Bjorn's reviewed-by still stands so I will apply it. [...] Thank you! Krzysztof
Re: [PATCH v6 1/5] PCI: Unify ECAM constants in native PCI Express drivers
On Sun, Nov 29, 2020 at 11:07:39PM +, Krzysztof Wilczyński wrote:
> Add ECAM-related constants to provide a set of standard constants
> defining memory address shift values to the byte-level address that can
> be used to access the PCI Express Configuration Space, and then move
> native PCI Express controller drivers to use the newly introduced
> definitions retiring driver-specific ones.
>
> Refactor pci_ecam_map_bus() function to use newly added constants so
> that limits to the bus, device function and offset (now limited to 4K as
> per the specification) are in place to prevent the defective or
> malicious caller from supplying incorrect configuration offset and thus
> targeting the wrong device when accessing extended configuration space.
> This refactor also allows for the ".bus_shit" initialisers to be dropped
Nice typo, I'd fix it while applying it though if you don't mind ;-),
no need to resend it.
Jokes aside, nice piece of work, thanks for that.
> when the user is not using a custom value as a default value will be
> used as per the PCI Express Specification.
>
> Suggested-by: Bjorn Helgaas
> Signed-off-by: Krzysztof Wilczyński
I think Bjorn's reviewed-by still stands so I will apply it.
Thanks !
Lorenzo
> ---
> drivers/pci/controller/dwc/pcie-al.c| 12 ++---
> drivers/pci/controller/dwc/pcie-hisi.c | 2 --
> drivers/pci/controller/pci-aardvark.c | 13 +++---
> drivers/pci/controller/pci-host-generic.c | 1 -
> drivers/pci/controller/pci-thunder-ecam.c | 1 -
> drivers/pci/controller/pcie-brcmstb.c | 16 ++--
> drivers/pci/controller/pcie-rockchip-host.c | 27 ++---
> drivers/pci/controller/pcie-rockchip.h | 8 +-
> drivers/pci/controller/pcie-tango.c | 1 -
> drivers/pci/controller/pcie-xilinx-nwl.c| 9 ++-
> drivers/pci/controller/pcie-xilinx.c| 11 ++---
> drivers/pci/controller/vmd.c| 11 -
> drivers/pci/ecam.c | 23 --
> include/linux/pci-ecam.h| 27 +
> 14 files changed, 73 insertions(+), 89 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-al.c
> b/drivers/pci/controller/dwc/pcie-al.c
> index f973fbca90cf..af9e51ab1af8 100644
> --- a/drivers/pci/controller/dwc/pcie-al.c
> +++ b/drivers/pci/controller/dwc/pcie-al.c
> @@ -76,7 +76,6 @@ static int al_pcie_init(struct pci_config_window *cfg)
> }
>
> const struct pci_ecam_ops al_pcie_ops = {
> - .bus_shift= 20,
> .init = al_pcie_init,
> .pci_ops = {
> .map_bus= al_pcie_map_bus,
> @@ -138,8 +137,6 @@ struct al_pcie {
> struct al_pcie_target_bus_cfg target_bus_cfg;
> };
>
> -#define PCIE_ECAM_DEVFN(x) (((x) & 0xff) << 12)
> -
> #define to_al_pcie(x)dev_get_drvdata((x)->dev)
>
> static inline u32 al_pcie_controller_readl(struct al_pcie *pcie, u32 offset)
> @@ -226,11 +223,6 @@ static void __iomem *al_pcie_conf_addr_map_bus(struct
> pci_bus *bus,
> struct al_pcie_target_bus_cfg *target_bus_cfg = >target_bus_cfg;
> unsigned int busnr_ecam = busnr & target_bus_cfg->ecam_mask;
> unsigned int busnr_reg = busnr & target_bus_cfg->reg_mask;
> - void __iomem *pci_base_addr;
> -
> - pci_base_addr = (void __iomem *)((uintptr_t)pp->va_cfg0_base +
> - (busnr_ecam << 20) +
> - PCIE_ECAM_DEVFN(devfn));
>
> if (busnr_reg != target_bus_cfg->reg_val) {
> dev_dbg(pcie->pci->dev, "Changing target bus busnum val from
> 0x%x to 0x%x\n",
> @@ -241,7 +233,7 @@ static void __iomem *al_pcie_conf_addr_map_bus(struct
> pci_bus *bus,
> target_bus_cfg->reg_mask);
> }
>
> - return pci_base_addr + where;
> + return pp->va_cfg0_base + PCIE_ECAM_OFFSET(busnr_ecam, devfn, where);
> }
>
> static struct pci_ops al_child_pci_ops = {
> @@ -264,7 +256,7 @@ static void al_pcie_config_prepare(struct al_pcie *pcie)
>
> target_bus_cfg = >target_bus_cfg;
>
> - ecam_bus_mask = (pcie->ecam_size >> 20) - 1;
> + ecam_bus_mask = (pcie->ecam_size >> PCIE_ECAM_BUS_SHIFT) - 1;
> if (ecam_bus_mask > 255) {
> dev_warn(pcie->dev, "ECAM window size is larger than 256MB.
> Cutting off at 256\n");
> ecam_bus_mask = 255;
> diff --git a/drivers/pci/controller/dwc/pcie-hisi.c
> b/drivers/pci/controller/dwc/pcie-hisi.c
> index 5ca86796d43a..8fc5960faf28 100644
> --- a/drivers/pci/controller/dwc/pcie-hisi.c
> +++ b/drivers/pci/controller/dwc/pcie-hisi.c
> @@ -100,7 +100,6 @@ static int hisi_pcie_init(struct pci_config_window *cfg)
> }
>
> const struct pci_ecam_ops hisi_pcie_ops = {
> - .bus_shift= 20,
> .init = hisi_pcie_init,
> .pci_ops = {
> .map_bus
[PATCH v6 1/5] PCI: Unify ECAM constants in native PCI Express drivers
Add ECAM-related constants to provide a set of standard constants
defining memory address shift values to the byte-level address that can
be used to access the PCI Express Configuration Space, and then move
native PCI Express controller drivers to use the newly introduced
definitions retiring driver-specific ones.
Refactor pci_ecam_map_bus() function to use newly added constants so
that limits to the bus, device function and offset (now limited to 4K as
per the specification) are in place to prevent the defective or
malicious caller from supplying incorrect configuration offset and thus
targeting the wrong device when accessing extended configuration space.
This refactor also allows for the ".bus_shit" initialisers to be dropped
when the user is not using a custom value as a default value will be
used as per the PCI Express Specification.
Suggested-by: Bjorn Helgaas
Signed-off-by: Krzysztof Wilczyński
---
drivers/pci/controller/dwc/pcie-al.c| 12 ++---
drivers/pci/controller/dwc/pcie-hisi.c | 2 --
drivers/pci/controller/pci-aardvark.c | 13 +++---
drivers/pci/controller/pci-host-generic.c | 1 -
drivers/pci/controller/pci-thunder-ecam.c | 1 -
drivers/pci/controller/pcie-brcmstb.c | 16 ++--
drivers/pci/controller/pcie-rockchip-host.c | 27 ++---
drivers/pci/controller/pcie-rockchip.h | 8 +-
drivers/pci/controller/pcie-tango.c | 1 -
drivers/pci/controller/pcie-xilinx-nwl.c| 9 ++-
drivers/pci/controller/pcie-xilinx.c| 11 ++---
drivers/pci/controller/vmd.c| 11 -
drivers/pci/ecam.c | 23 --
include/linux/pci-ecam.h| 27 +
14 files changed, 73 insertions(+), 89 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-al.c
b/drivers/pci/controller/dwc/pcie-al.c
index f973fbca90cf..af9e51ab1af8 100644
--- a/drivers/pci/controller/dwc/pcie-al.c
+++ b/drivers/pci/controller/dwc/pcie-al.c
@@ -76,7 +76,6 @@ static int al_pcie_init(struct pci_config_window *cfg)
}
const struct pci_ecam_ops al_pcie_ops = {
- .bus_shift= 20,
.init = al_pcie_init,
.pci_ops = {
.map_bus= al_pcie_map_bus,
@@ -138,8 +137,6 @@ struct al_pcie {
struct al_pcie_target_bus_cfg target_bus_cfg;
};
-#define PCIE_ECAM_DEVFN(x) (((x) & 0xff) << 12)
-
#define to_al_pcie(x) dev_get_drvdata((x)->dev)
static inline u32 al_pcie_controller_readl(struct al_pcie *pcie, u32 offset)
@@ -226,11 +223,6 @@ static void __iomem *al_pcie_conf_addr_map_bus(struct
pci_bus *bus,
struct al_pcie_target_bus_cfg *target_bus_cfg = >target_bus_cfg;
unsigned int busnr_ecam = busnr & target_bus_cfg->ecam_mask;
unsigned int busnr_reg = busnr & target_bus_cfg->reg_mask;
- void __iomem *pci_base_addr;
-
- pci_base_addr = (void __iomem *)((uintptr_t)pp->va_cfg0_base +
-(busnr_ecam << 20) +
-PCIE_ECAM_DEVFN(devfn));
if (busnr_reg != target_bus_cfg->reg_val) {
dev_dbg(pcie->pci->dev, "Changing target bus busnum val from
0x%x to 0x%x\n",
@@ -241,7 +233,7 @@ static void __iomem *al_pcie_conf_addr_map_bus(struct
pci_bus *bus,
target_bus_cfg->reg_mask);
}
- return pci_base_addr + where;
+ return pp->va_cfg0_base + PCIE_ECAM_OFFSET(busnr_ecam, devfn, where);
}
static struct pci_ops al_child_pci_ops = {
@@ -264,7 +256,7 @@ static void al_pcie_config_prepare(struct al_pcie *pcie)
target_bus_cfg = >target_bus_cfg;
- ecam_bus_mask = (pcie->ecam_size >> 20) - 1;
+ ecam_bus_mask = (pcie->ecam_size >> PCIE_ECAM_BUS_SHIFT) - 1;
if (ecam_bus_mask > 255) {
dev_warn(pcie->dev, "ECAM window size is larger than 256MB.
Cutting off at 256\n");
ecam_bus_mask = 255;
diff --git a/drivers/pci/controller/dwc/pcie-hisi.c
b/drivers/pci/controller/dwc/pcie-hisi.c
index 5ca86796d43a..8fc5960faf28 100644
--- a/drivers/pci/controller/dwc/pcie-hisi.c
+++ b/drivers/pci/controller/dwc/pcie-hisi.c
@@ -100,7 +100,6 @@ static int hisi_pcie_init(struct pci_config_window *cfg)
}
const struct pci_ecam_ops hisi_pcie_ops = {
- .bus_shift= 20,
.init = hisi_pcie_init,
.pci_ops = {
.map_bus= hisi_pcie_map_bus,
@@ -135,7 +134,6 @@ static int hisi_pcie_platform_init(struct pci_config_window
*cfg)
}
static const struct pci_ecam_ops hisi_pcie_platform_ops = {
- .bus_shift= 20,
.init = hisi_pcie_platform_init,
.pci_ops = {
.map_bus= hisi_pcie_map_bus,
diff --git a/drivers/pci/controller/pci-aardvark.c
b/drivers/pci/controller/pci-aardvark.c
index 0be485a25327..1043e54c73bd 100644
---
