[PATCH u-boot-next 00/12] Common U-Boot macros for PCI Configuration Mechanism #1

[PATCH u-boot-next 00/12] Common U-Boot macros for PCI Configuration Mechanism #1

[Pali Rohár]

This patch series add new U-Boot macros for composing Config Address for
PCI Configuration Mechanism #1 as defined in PCI Local Bus Specification,
including extended variant (which do not have any formal specification)
and then use these new macros (PCI_CONF1_ADDRESS and PCI_CONF1_EXT_ADDRESS)
in all PCI drivers that use Config Address according to PCI Configuration
Mechanism #1.

PCI Configuration Mechanism #1 was originally specified for x86 platforms
and it is still supported on x86 together with PCIe ECAM. Nowadays lot of
non-ECAM-compliant ARM PCIe controllers use only extended variant of this
address construction and some of them requires cleared Enable bit. Extended
variant is also supported by x86 AMD Barcelona (and new) CPUs, but U-Boot
code does not provide this support yet.

Note that it exists also PCI Configuration Mechanism #2, but this one was
removed from PCI Local Bus Specification revision 3.0 and seems that it is
not used by any PCI driver in U-Boot. So I have not added macros for this
mechanism in this patch series.

Because lot of hardware still use this (rather old) mechanism, relevant
U-Boot PCI and PCIe drivers have own ad-hoc code address construction,
which is repeated and written in different ways.

This patch series is code cleanup of these PCIe drivers to use common
U-Boot macros for PCI Configuration Mechanism #1.

The last change is fixing construction of Config Address in PCI driver
sh7751. Construction with this change matches Linux kernel code and also
U-Boot code prior commit mentioned in commit message.

I have tested this patch series only for PCI mvebu driver on A385 board and
it is working fine. So Please properly review all other changes. Ideally
test them.

I have run CI tests with these changes on github and everything passed:
https://github.com/u-boot/u-boot/pull/101

Please let me know what do you think about this change.

Pali Rohár (12):
  pci: Add standard PCI Config Address macros
  pci: gt64120: Use PCI_CONF1_ADDRESS() macro
  pci: mpc85xx: Use PCI_CONF1_EXT_ADDRESS() macro
  pci: msc01: Use PCI_CONF1_ADDRESS() macro
  pci: mvebu: Use PCI_CONF1_EXT_ADDRESS() macro
  pci: tegra: Use PCI_CONF1_EXT_ADDRESS() macro
  pci: fsl: Use PCI_CONF1_EXT_ADDRESS() macro
  pci: mediatek: Use PCI_CONF1_EXT_ADDRESS() macro
  pci: sh7780: Use PCI_CONF1_ADDRESS() macro
  x86: pci: Use PCI_CONF1_ADDRESS() macro
  m68k: mcf5445x: pci: Use PCI_CONF1_ADDRESS() macro
  pci: sh7751: Fix access to config space via PCI_CONF1_ADDRESS() macro

 arch/m68k/cpu/mcf5445x/pci.c |  7 +++---
 arch/x86/cpu/pci.c           |  4 ++--
 drivers/pci/pci_gt64120.c    |  7 ++----
 drivers/pci/pci_mpc85xx.c    |  4 ++--
 drivers/pci/pci_msc01.c      |  7 ++----
 drivers/pci/pci_mvebu.c      | 17 ++++----------
 drivers/pci/pci_sh7751.c     | 29 ++---------------------
 drivers/pci/pci_sh7780.c     |  8 +++----
 drivers/pci/pci_tegra.c      | 11 +++------
 drivers/pci/pcie_fsl.c       | 10 ++++----
 drivers/pci/pcie_mediatek.c  | 17 +++++++-------
 include/gt64120.h            | 12 ----------
 include/msc01.h              |  9 --------
 include/pci.h                | 45 ++++++++++++++++++++++++++++++++++++
 14 files changed, 83 insertions(+), 104 deletions(-)

-- 
2.20.1

[PATCH u-boot-next 01/12] pci: Add standard PCI Config Address macros

[Pali Rohár]

Lot of PCI and PCIe controllers are using standard Config Address for PCI
Configuration Mechanism #1 or its extended version.

So add PCI_CONF1_ADDRESS() and PCI_CONF1_EXT_ADDRESS() macros into U-Boot's
pci.h header file which can be suitable for most PCI and PCIe controller
drivers. Drivers do not have to invent their own macros and can use these
new U-Boot macros.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 include/pci.h | 45 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/include/pci.h b/include/pci.h
index 6c1094d72998..0ea41a7e1ba2 100644
--- a/include/pci.h
+++ b/include/pci.h
@@ -522,6 +522,51 @@
 
 #include <pci_ids.h>
 
+/*
+ * Config Address for PCI Configuration Mechanism #1
+ *
+ * See PCI Local Bus Specification, Revision 3.0,
+ * Section 3.2.2.3.2, Figure 3-2, p. 50.
+ */
+
+#define PCI_CONF1_BUS_SHIFT	16 /* Bus number */
+#define PCI_CONF1_DEV_SHIFT	11 /* Device number */
+#define PCI_CONF1_FUNC_SHIFT	8  /* Function number */
+
+#define PCI_CONF1_BUS_MASK	0xff
+#define PCI_CONF1_DEV_MASK	0x1f
+#define PCI_CONF1_FUNC_MASK	0x7
+#define PCI_CONF1_REG_MASK	0xfc /* Limit aligned offset to a maximum of 256B */
+
+#define PCI_CONF1_ENABLE	BIT(31)
+#define PCI_CONF1_BUS(x)	(((x) & PCI_CONF1_BUS_MASK) << PCI_CONF1_BUS_SHIFT)
+#define PCI_CONF1_DEV(x)	(((x) & PCI_CONF1_DEV_MASK) << PCI_CONF1_DEV_SHIFT)
+#define PCI_CONF1_FUNC(x)	(((x) & PCI_CONF1_FUNC_MASK) << PCI_CONF1_FUNC_SHIFT)
+#define PCI_CONF1_REG(x)	((x) & PCI_CONF1_REG_MASK)
+
+#define PCI_CONF1_ADDRESS(bus, dev, func, reg) \
+	(PCI_CONF1_ENABLE | \
+	 PCI_CONF1_BUS(bus) | \
+	 PCI_CONF1_DEV(dev) | \
+	 PCI_CONF1_FUNC(func) | \
+	 PCI_CONF1_REG(reg))
+
+/*
+ * Extension of PCI Config Address for accessing extended PCIe registers
+ *
+ * No standardized specification, but used on lot of non-ECAM-compliant ARM SoCs
+ * or on AMD Barcelona and new CPUs. Reserved bits [27:24] of PCI Config Address
+ * are used for specifying additional 4 high bits of PCI Express register.
+ */
+
+#define PCI_CONF1_EXT_REG_SHIFT	16
+#define PCI_CONF1_EXT_REG_MASK	0xf00
+#define PCI_CONF1_EXT_REG(x)	(((x) & PCI_CONF1_EXT_REG_MASK) << PCI_CONF1_EXT_REG_SHIFT)
+
+#define PCI_CONF1_EXT_ADDRESS(bus, dev, func, reg) \
+	(PCI_CONF1_ADDRESS(bus, dev, func, reg) | \
+	 PCI_CONF1_EXT_REG(reg))
+
 /*
  * Enhanced Configuration Access Mechanism (ECAM)
  *
-- 
2.20.1

Re: [PATCH u-boot-next 01/12] pci: Add standard PCI Config Address macros

[Simon Glass]

Hi Paul,

On Fri, 26 Nov 2021 at 03:43, Pali Rohár <pali@kernel.org> wrote:
>
> Lot of PCI and PCIe controllers are using standard Config Address for PCI
> Configuration Mechanism #1 or its extended version.
>
> So add PCI_CONF1_ADDRESS() and PCI_CONF1_EXT_ADDRESS() macros into U-Boot's
> pci.h header file which can be suitable for most PCI and PCIe controller
> drivers. Drivers do not have to invent their own macros and can use these
> new U-Boot macros.
>
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
>  include/pci.h | 45 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 45 insertions(+)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

Thoughts below

> diff --git a/include/pci.h b/include/pci.h
> index 6c1094d72998..0ea41a7e1ba2 100644
> --- a/include/pci.h
> +++ b/include/pci.h
> @@ -522,6 +522,51 @@
>
>  #include <pci_ids.h>
>
> +/*
> + * Config Address for PCI Configuration Mechanism #1
> + *
> + * See PCI Local Bus Specification, Revision 3.0,
> + * Section 3.2.2.3.2, Figure 3-2, p. 50.
> + */
> +
> +#define PCI_CONF1_BUS_SHIFT    16 /* Bus number */
> +#define PCI_CONF1_DEV_SHIFT    11 /* Device number */
> +#define PCI_CONF1_FUNC_SHIFT   8  /* Function number */
> +
> +#define PCI_CONF1_BUS_MASK     0xff
> +#define PCI_CONF1_DEV_MASK     0x1f
> +#define PCI_CONF1_FUNC_MASK    0x7
> +#define PCI_CONF1_REG_MASK     0xfc /* Limit aligned offset to a maximum of 256B */
> +
> +#define PCI_CONF1_ENABLE       BIT(31)
> +#define PCI_CONF1_BUS(x)       (((x) & PCI_CONF1_BUS_MASK) << PCI_CONF1_BUS_SHIFT)
> +#define PCI_CONF1_DEV(x)       (((x) & PCI_CONF1_DEV_MASK) << PCI_CONF1_DEV_SHIFT)
> +#define PCI_CONF1_FUNC(x)      (((x) & PCI_CONF1_FUNC_MASK) << PCI_CONF1_FUNC_SHIFT)
> +#define PCI_CONF1_REG(x)       ((x) & PCI_CONF1_REG_MASK)
> +
> +#define PCI_CONF1_ADDRESS(bus, dev, func, reg) \

Just for brevity, how about _ADDR() instead of _ADDRESS() ?

> +       (PCI_CONF1_ENABLE | \
> +        PCI_CONF1_BUS(bus) | \
> +        PCI_CONF1_DEV(dev) | \
> +        PCI_CONF1_FUNC(func) | \
> +        PCI_CONF1_REG(reg))
> +
> +/*
> + * Extension of PCI Config Address for accessing extended PCIe registers
> + *
> + * No standardized specification, but used on lot of non-ECAM-compliant ARM SoCs
> + * or on AMD Barcelona and new CPUs. Reserved bits [27:24] of PCI Config Address
> + * are used for specifying additional 4 high bits of PCI Express register.
> + */
> +
> +#define PCI_CONF1_EXT_REG_SHIFT        16
> +#define PCI_CONF1_EXT_REG_MASK 0xf00

How about s/EXT_REG/EXT/ ?

For shifts and masks we normally like to define the mask in terms of
the shift, so:

> +#define PCI_CONF1_EXT_REG_MASK (0xf00 << PCI_CONF1_EXT_REG_SHIFT)

> +#define PCI_CONF1_EXT_REG(x)   (((x) & PCI_CONF1_EXT_REG_MASK) << PCI_CONF1_EXT_REG_SHIFT)
> +
> +#define PCI_CONF1_EXT_ADDRESS(bus, dev, func, reg) \
> +       (PCI_CONF1_ADDRESS(bus, dev, func, reg) | \
> +        PCI_CONF1_EXT_REG(reg))
> +
>  /*
>   * Enhanced Configuration Access Mechanism (ECAM)
>   *
> --
> 2.20.1
>

Regards,
Simon

Re: [PATCH u-boot-next 01/12] pci: Add standard PCI Config Address macros

[Pali Rohár]

On Tuesday 28 December 2021 01:32:02 Simon Glass wrote:
> Hi Paul,
> 
> On Fri, 26 Nov 2021 at 03:43, Pali Rohár <pali@kernel.org> wrote:
> >
> > Lot of PCI and PCIe controllers are using standard Config Address for PCI
> > Configuration Mechanism #1 or its extended version.
> >
> > So add PCI_CONF1_ADDRESS() and PCI_CONF1_EXT_ADDRESS() macros into U-Boot's
> > pci.h header file which can be suitable for most PCI and PCIe controller
> > drivers. Drivers do not have to invent their own macros and can use these
> > new U-Boot macros.
> >
> > Signed-off-by: Pali Rohár <pali@kernel.org>
> > ---
> >  include/pci.h | 45 +++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 45 insertions(+)
> >
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> Thoughts below
> 
> > diff --git a/include/pci.h b/include/pci.h
> > index 6c1094d72998..0ea41a7e1ba2 100644
> > --- a/include/pci.h
> > +++ b/include/pci.h
> > @@ -522,6 +522,51 @@
> >
> >  #include <pci_ids.h>
> >
> > +/*
> > + * Config Address for PCI Configuration Mechanism #1
> > + *
> > + * See PCI Local Bus Specification, Revision 3.0,
> > + * Section 3.2.2.3.2, Figure 3-2, p. 50.
> > + */
> > +
> > +#define PCI_CONF1_BUS_SHIFT    16 /* Bus number */
> > +#define PCI_CONF1_DEV_SHIFT    11 /* Device number */
> > +#define PCI_CONF1_FUNC_SHIFT   8  /* Function number */
> > +
> > +#define PCI_CONF1_BUS_MASK     0xff
> > +#define PCI_CONF1_DEV_MASK     0x1f
> > +#define PCI_CONF1_FUNC_MASK    0x7
> > +#define PCI_CONF1_REG_MASK     0xfc /* Limit aligned offset to a maximum of 256B */
> > +
> > +#define PCI_CONF1_ENABLE       BIT(31)
> > +#define PCI_CONF1_BUS(x)       (((x) & PCI_CONF1_BUS_MASK) << PCI_CONF1_BUS_SHIFT)
> > +#define PCI_CONF1_DEV(x)       (((x) & PCI_CONF1_DEV_MASK) << PCI_CONF1_DEV_SHIFT)
> > +#define PCI_CONF1_FUNC(x)      (((x) & PCI_CONF1_FUNC_MASK) << PCI_CONF1_FUNC_SHIFT)
> > +#define PCI_CONF1_REG(x)       ((x) & PCI_CONF1_REG_MASK)
> > +
> > +#define PCI_CONF1_ADDRESS(bus, dev, func, reg) \
> 
> Just for brevity, how about _ADDR() instead of _ADDRESS() ?

I do not know... I think both are OK.

> > +       (PCI_CONF1_ENABLE | \
> > +        PCI_CONF1_BUS(bus) | \
> > +        PCI_CONF1_DEV(dev) | \
> > +        PCI_CONF1_FUNC(func) | \
> > +        PCI_CONF1_REG(reg))
> > +
> > +/*
> > + * Extension of PCI Config Address for accessing extended PCIe registers
> > + *
> > + * No standardized specification, but used on lot of non-ECAM-compliant ARM SoCs
> > + * or on AMD Barcelona and new CPUs. Reserved bits [27:24] of PCI Config Address
> > + * are used for specifying additional 4 high bits of PCI Express register.
> > + */
> > +
> > +#define PCI_CONF1_EXT_REG_SHIFT        16
> > +#define PCI_CONF1_EXT_REG_MASK 0xf00
> 
> How about s/EXT_REG/EXT/ ?

I'm not sure... Name "EXT" does not say what it is. This macro defines
access to the extended part (higher bits) of register value. That is why
I called it "EXT_REG" so reader would see that it belongs to plain "REG"
macro (which defines lower bits of register value).

> For shifts and masks we normally like to define the mask in terms of
> the shift, so:
> 
> > +#define PCI_CONF1_EXT_REG_MASK (0xf00 << PCI_CONF1_EXT_REG_SHIFT)
> 
> > +#define PCI_CONF1_EXT_REG(x)   (((x) & PCI_CONF1_EXT_REG_MASK) << PCI_CONF1_EXT_REG_SHIFT)

Well, I wanted these PCI_CONF1_* macros to be "compatible" with
PCIE_ECAM_* macros which are already in next and which I reused from
Linux kernel header files. And these existing macros are using the style
which I used also for PCI_CONF1_* macros.

> > +#define PCI_CONF1_EXT_ADDRESS(bus, dev, func, reg) \
> > +       (PCI_CONF1_ADDRESS(bus, dev, func, reg) | \
> > +        PCI_CONF1_EXT_REG(reg))
> > +
> >  /*
> >   * Enhanced Configuration Access Mechanism (ECAM)
> >   *
> > --
> > 2.20.1
> >
> 
> Regards,
> Simon

Re: [PATCH u-boot-next 01/12] pci: Add standard PCI Config Address macros

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 600 bytes --]

On Fri, Nov 26, 2021 at 11:42:41AM +0100, Pali Rohár wrote:

> Lot of PCI and PCIe controllers are using standard Config Address for PCI
> Configuration Mechanism #1 or its extended version.
> 
> So add PCI_CONF1_ADDRESS() and PCI_CONF1_EXT_ADDRESS() macros into U-Boot's
> pci.h header file which can be suitable for most PCI and PCIe controller
> drivers. Drivers do not have to invent their own macros and can use these
> new U-Boot macros.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

[PATCH u-boot-next 02/12] pci: gt64120: Use PCI_CONF1_ADDRESS() macro

[Pali Rohár]

PCI gt64120 driver uses standard format of Config Address for PCI
Configuration Mechanism #1.

So use new U-Boot macro PCI_CONF1_ADDRESS() and remove old custom driver
address macros.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 drivers/pci/pci_gt64120.c |  7 ++-----
 include/gt64120.h         | 12 ------------
 2 files changed, 2 insertions(+), 17 deletions(-)

diff --git a/drivers/pci/pci_gt64120.c b/drivers/pci/pci_gt64120.c
index 153c65b119a4..2c2a80eeaa06 100644
--- a/drivers/pci/pci_gt64120.c
+++ b/drivers/pci/pci_gt64120.c
@@ -48,7 +48,7 @@ static int gt_config_access(struct gt64120_pci_controller *gt,
 {
 	unsigned int bus = PCI_BUS(bdf);
 	unsigned int dev = PCI_DEV(bdf);
-	unsigned int devfn = PCI_DEV(bdf) << 3 | PCI_FUNC(bdf);
+	unsigned int func = PCI_FUNC(bdf);
 	u32 intr;
 	u32 addr;
 	u32 val;
@@ -65,10 +65,7 @@ static int gt_config_access(struct gt64120_pci_controller *gt,
 	/* Clear cause register bits */
 	writel(~GT_INTRCAUSE_ABORT_BITS, &gt->regs->intrcause);
 
-	addr = GT_PCI0_CFGADDR_CONFIGEN_BIT;
-	addr |=	bus << GT_PCI0_CFGADDR_BUSNUM_SHF;
-	addr |=	devfn << GT_PCI0_CFGADDR_FUNCTNUM_SHF;
-	addr |= (where / 4) << GT_PCI0_CFGADDR_REGNUM_SHF;
+	addr = PCI_CONF1_ADDRESS(bus, dev, func, where);
 
 	/* Setup address */
 	writel(addr, &gt->regs->pci0_cfgaddr);
diff --git a/include/gt64120.h b/include/gt64120.h
index 0b577f3f44b9..b58afe3c4afe 100644
--- a/include/gt64120.h
+++ b/include/gt64120.h
@@ -491,18 +491,6 @@
 #define GT_INTRCAUSE_TARABORT0_BIT	GT_INTRCAUSE_TARABORT0_MSK
 
 
-#define GT_PCI0_CFGADDR_REGNUM_SHF	2
-#define GT_PCI0_CFGADDR_REGNUM_MSK	(MSK(6) << GT_PCI0_CFGADDR_REGNUM_SHF)
-#define GT_PCI0_CFGADDR_FUNCTNUM_SHF	8
-#define GT_PCI0_CFGADDR_FUNCTNUM_MSK	(MSK(3) << GT_PCI0_CFGADDR_FUNCTNUM_SHF)
-#define GT_PCI0_CFGADDR_DEVNUM_SHF	11
-#define GT_PCI0_CFGADDR_DEVNUM_MSK	(MSK(5) << GT_PCI0_CFGADDR_DEVNUM_SHF)
-#define GT_PCI0_CFGADDR_BUSNUM_SHF	16
-#define GT_PCI0_CFGADDR_BUSNUM_MSK	(MSK(8) << GT_PCI0_CFGADDR_BUSNUM_SHF)
-#define GT_PCI0_CFGADDR_CONFIGEN_SHF	31
-#define GT_PCI0_CFGADDR_CONFIGEN_MSK	(MSK(1) << GT_PCI0_CFGADDR_CONFIGEN_SHF)
-#define GT_PCI0_CFGADDR_CONFIGEN_BIT	GT_PCI0_CFGADDR_CONFIGEN_MSK
-
 #define GT_PCI0_CMD_MBYTESWAP_SHF	0
 #define GT_PCI0_CMD_MBYTESWAP_MSK	(MSK(1) << GT_PCI0_CMD_MBYTESWAP_SHF)
 #define GT_PCI0_CMD_MBYTESWAP_BIT	GT_PCI0_CMD_MBYTESWAP_MSK
-- 
2.20.1

Re: [PATCH u-boot-next 02/12] pci: gt64120: Use PCI_CONF1_ADDRESS() macro

[Simon Glass]

On Fri, 26 Nov 2021 at 03:43, Pali Rohár <pali@kernel.org> wrote:
>
> PCI gt64120 driver uses standard format of Config Address for PCI
> Configuration Mechanism #1.
>
> So use new U-Boot macro PCI_CONF1_ADDRESS() and remove old custom driver
> address macros.
>
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
>  drivers/pci/pci_gt64120.c |  7 ++-----
>  include/gt64120.h         | 12 ------------
>  2 files changed, 2 insertions(+), 17 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [PATCH u-boot-next 02/12] pci: gt64120: Use PCI_CONF1_ADDRESS() macro

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 409 bytes --]

On Fri, Nov 26, 2021 at 11:42:42AM +0100, Pali Rohár wrote:

> PCI gt64120 driver uses standard format of Config Address for PCI
> Configuration Mechanism #1.
> 
> So use new U-Boot macro PCI_CONF1_ADDRESS() and remove old custom driver
> address macros.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

[PATCH u-boot-next 03/12] pci: mpc85xx: Use PCI_CONF1_EXT_ADDRESS() macro

[Pali Rohár]

PCI mpc85xx driver uses extended format of Config Address for PCI
Configuration Mechanism #1.

So use new U-Boot macro PCI_CONF1_EXT_ADDRESS().

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 drivers/pci/pci_mpc85xx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/pci_mpc85xx.c b/drivers/pci/pci_mpc85xx.c
index 574cb784a893..1e180ee289b0 100644
--- a/drivers/pci/pci_mpc85xx.c
+++ b/drivers/pci/pci_mpc85xx.c
@@ -23,7 +23,7 @@ static int mpc85xx_pci_dm_read_config(const struct udevice *dev, pci_dev_t bdf,
 	struct mpc85xx_pci_priv *priv = dev_get_priv(dev);
 	u32 addr;
 
-	addr = bdf | (offset & 0xfc) | ((offset & 0xf00) << 16) | 0x80000000;
+	addr = PCI_CONF1_EXT_ADDRESS(PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf), offset);
 	out_be32(priv->cfg_addr, addr);
 	sync();
 	*value = pci_conv_32_to_size(in_le32(priv->cfg_data), offset, size);
@@ -38,7 +38,7 @@ static int mpc85xx_pci_dm_write_config(struct udevice *dev, pci_dev_t bdf,
 	struct mpc85xx_pci_priv *priv = dev_get_priv(dev);
 	u32 addr;
 
-	addr = bdf | (offset & 0xfc) | ((offset & 0xf00) << 16) | 0x80000000;
+	addr = PCI_CONF1_EXT_ADDRESS(PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf), offset);
 	out_be32(priv->cfg_addr, addr);
 	sync();
 	out_le32(priv->cfg_data, pci_conv_size_to_32(0, value, offset, size));
-- 
2.20.1

Re: [PATCH u-boot-next 03/12] pci: mpc85xx: Use PCI_CONF1_EXT_ADDRESS() macro

[Simon Glass]

Hi Pali,

On Fri, 26 Nov 2021 at 03:43, Pali Rohár <pali@kernel.org> wrote:
>
> PCI mpc85xx driver uses extended format of Config Address for PCI
> Configuration Mechanism #1.
>
> So use new U-Boot macro PCI_CONF1_EXT_ADDRESS().
>
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
>  drivers/pci/pci_mpc85xx.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

nit below

>
> diff --git a/drivers/pci/pci_mpc85xx.c b/drivers/pci/pci_mpc85xx.c
> index 574cb784a893..1e180ee289b0 100644
> --- a/drivers/pci/pci_mpc85xx.c
> +++ b/drivers/pci/pci_mpc85xx.c
> @@ -23,7 +23,7 @@ static int mpc85xx_pci_dm_read_config(const struct udevice *dev, pci_dev_t bdf,
>         struct mpc85xx_pci_priv *priv = dev_get_priv(dev);
>         u32 addr;
>
> -       addr = bdf | (offset & 0xfc) | ((offset & 0xf00) << 16) | 0x80000000;
> +       addr = PCI_CONF1_EXT_ADDRESS(PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf), offset);
>         out_be32(priv->cfg_addr, addr);
>         sync();
>         *value = pci_conv_32_to_size(in_le32(priv->cfg_data), offset, size);
> @@ -38,7 +38,7 @@ static int mpc85xx_pci_dm_write_config(struct udevice *dev, pci_dev_t bdf,
>         struct mpc85xx_pci_priv *priv = dev_get_priv(dev);
>         u32 addr;
>
> -       addr = bdf | (offset & 0xfc) | ((offset & 0xf00) << 16) | 0x80000000;
> +       addr = PCI_CONF1_EXT_ADDRESS(PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf), offset);

It seems annoying to have to separate these out just to have the macro
put them back together. Perhaps add a version of the macro that takes
bdf as a parameter?

>         out_be32(priv->cfg_addr, addr);
>         sync();
>         out_le32(priv->cfg_data, pci_conv_size_to_32(0, value, offset, size));
> --
> 2.20.1
>

Regards,
Simon

Re: [PATCH u-boot-next 03/12] pci: mpc85xx: Use PCI_CONF1_EXT_ADDRESS() macro

[Pali Rohár]

On Tuesday 28 December 2021 01:32:05 Simon Glass wrote:
> Hi Pali,
> 
> On Fri, 26 Nov 2021 at 03:43, Pali Rohár <pali@kernel.org> wrote:
> >
> > PCI mpc85xx driver uses extended format of Config Address for PCI
> > Configuration Mechanism #1.
> >
> > So use new U-Boot macro PCI_CONF1_EXT_ADDRESS().
> >
> > Signed-off-by: Pali Rohár <pali@kernel.org>
> > ---
> >  drivers/pci/pci_mpc85xx.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> nit below
> 
> >
> > diff --git a/drivers/pci/pci_mpc85xx.c b/drivers/pci/pci_mpc85xx.c
> > index 574cb784a893..1e180ee289b0 100644
> > --- a/drivers/pci/pci_mpc85xx.c
> > +++ b/drivers/pci/pci_mpc85xx.c
> > @@ -23,7 +23,7 @@ static int mpc85xx_pci_dm_read_config(const struct udevice *dev, pci_dev_t bdf,
> >         struct mpc85xx_pci_priv *priv = dev_get_priv(dev);
> >         u32 addr;
> >
> > -       addr = bdf | (offset & 0xfc) | ((offset & 0xf00) << 16) | 0x80000000;
> > +       addr = PCI_CONF1_EXT_ADDRESS(PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf), offset);
> >         out_be32(priv->cfg_addr, addr);
> >         sync();
> >         *value = pci_conv_32_to_size(in_le32(priv->cfg_data), offset, size);
> > @@ -38,7 +38,7 @@ static int mpc85xx_pci_dm_write_config(struct udevice *dev, pci_dev_t bdf,
> >         struct mpc85xx_pci_priv *priv = dev_get_priv(dev);
> >         u32 addr;
> >
> > -       addr = bdf | (offset & 0xfc) | ((offset & 0xf00) << 16) | 0x80000000;
> > +       addr = PCI_CONF1_EXT_ADDRESS(PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf), offset);
> 
> It seems annoying to have to separate these out just to have the macro
> put them back together. Perhaps add a version of the macro that takes
> bdf as a parameter?

Hello!

In most cases specifying the bus value directly from the "bdf" variable
is in U-Boot incorrect. It is because since new DM PCI API "bdf"
parameter in read_config and write_config callbacks is relative to the
DM PCI root bus. And it needs to be translated from DM PCI numbering to
the numbering on PCI bus. In most cases for single-domain or
single-host-bridge or single-PCIe-root-port buses is root bus number
zero and therefore translation is 1:1. Also it applies for the first
registered host bridge into DM. But my understanding of DM API is that
DM drivers should not expects order of loading and neither that exactly
one DM driver from PCI category would be loaded.

So in my opinion, passing bus number from "bdf" variable together with
device number from "bdf" variable to PCI_CONF1_EXT_ADDRESS() is
something which should be avoided in all new drivers. And so separating
bus and device is a good idea.

Lot of these existing PCI drivers were converted from old driver to DM
model without thinking about multidomain/multi-host-bridge support
(as I guess all these old drivers are running on platforms without
multidomain support), so nobody noticed any issue.

But nowadays, it is common that on ARM SoCs are PCIe ports connected to
different PCIe Root Complexes which represents different PCIe
controllers and therefore also different PCI domains. So I think it is a
good idea to have separated bus and domain and do not provide macro
which takes directly "bdf" as it could lead to issues that new drivers
would not provide correct PCIe multi-port support.

Look into pci-aardvark.c or pci_mvebu.c, correct way is to calculate bus
number as: int busno = PCI_BUS(bdf) - dev_seq(bus); (unless driver or HW
does not expect any more complicated translation or mapping).

> >         out_be32(priv->cfg_addr, addr);
> >         sync();
> >         out_le32(priv->cfg_data, pci_conv_size_to_32(0, value, offset, size));
> > --
> > 2.20.1
> >
> 
> Regards,
> Simon

Re: [PATCH u-boot-next 03/12] pci: mpc85xx: Use PCI_CONF1_EXT_ADDRESS() macro

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 366 bytes --]

On Fri, Nov 26, 2021 at 11:42:43AM +0100, Pali Rohár wrote:

> PCI mpc85xx driver uses extended format of Config Address for PCI
> Configuration Mechanism #1.
> 
> So use new U-Boot macro PCI_CONF1_EXT_ADDRESS().
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

[PATCH u-boot-next 04/12] pci: msc01: Use PCI_CONF1_ADDRESS() macro

[Pali Rohár]

PCI msc01 driver uses standard format of Config Address for PCI
Configuration Mechanism #1 but with cleared Enable bit.

So use new U-Boot macro PCI_CONF1_ADDRESS() with clearing PCI_CONF1_ENABLE
bit and remove old custom driver address macros.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 drivers/pci/pci_msc01.c | 7 ++-----
 include/msc01.h         | 9 ---------
 2 files changed, 2 insertions(+), 14 deletions(-)

diff --git a/drivers/pci/pci_msc01.c b/drivers/pci/pci_msc01.c
index 2f1b688fc321..8d363d60498b 100644
--- a/drivers/pci/pci_msc01.c
+++ b/drivers/pci/pci_msc01.c
@@ -34,16 +34,13 @@ static int msc01_config_access(struct msc01_pci_controller *msc01,
 	void *cfgdata = msc01->base + MSC01_PCI_CFGDATA_OFS;
 	unsigned int bus = PCI_BUS(bdf);
 	unsigned int dev = PCI_DEV(bdf);
-	unsigned int devfn = PCI_DEV(bdf) << 3 | PCI_FUNC(bdf);
+	unsigned int func = PCI_FUNC(bdf);
 
 	/* clear abort status */
 	__raw_writel(aborts, intstat);
 
 	/* setup address */
-	__raw_writel((bus << MSC01_PCI_CFGADDR_BNUM_SHF) |
-		     (dev << MSC01_PCI_CFGADDR_DNUM_SHF) |
-		     (devfn << MSC01_PCI_CFGADDR_FNUM_SHF) |
-		     ((where / 4) << MSC01_PCI_CFGADDR_RNUM_SHF),
+	__raw_writel((PCI_CONF1_ADDRESS(bus, dev, func, where) & ~PCI_CONF1_ENABLE),
 		     msc01->base + MSC01_PCI_CFGADDR_OFS);
 
 	/* perform access */
diff --git a/include/msc01.h b/include/msc01.h
index ec18a724eb93..20158123494a 100644
--- a/include/msc01.h
+++ b/include/msc01.h
@@ -71,15 +71,6 @@
 #define MSC01_PCI_INTSTAT_MA_SHF	7
 #define MSC01_PCI_INTSTAT_MA_MSK	(0x1 << MSC01_PCI_INTSTAT_MA_SHF)
 
-#define MSC01_PCI_CFGADDR_BNUM_SHF	16
-#define MSC01_PCI_CFGADDR_BNUM_MSK	(0xff << MSC01_PCI_CFGADDR_BNUM_SHF)
-#define MSC01_PCI_CFGADDR_DNUM_SHF	11
-#define MSC01_PCI_CFGADDR_DNUM_MSK	(0x1f << MSC01_PCI_CFGADDR_DNUM_SHF)
-#define MSC01_PCI_CFGADDR_FNUM_SHF	8
-#define MSC01_PCI_CFGADDR_FNUM_MSK	(0x3 << MSC01_PCI_CFGADDR_FNUM_SHF)
-#define MSC01_PCI_CFGADDR_RNUM_SHF	2
-#define MSC01_PCI_CFGADDR_RNUM_MSK	(0x3f << MSC01_PCI_CFGADDR_RNUM_SHF)
-
 #define MSC01_PCI_HEAD0_VENDORID_SHF	0
 #define MSC01_PCI_HEAD0_DEVICEID_SHF	16
 
-- 
2.20.1

Re: [PATCH u-boot-next 04/12] pci: msc01: Use PCI_CONF1_ADDRESS() macro

[Simon Glass]

On Fri, 26 Nov 2021 at 03:43, Pali Rohár <pali@kernel.org> wrote:
>
> PCI msc01 driver uses standard format of Config Address for PCI
> Configuration Mechanism #1 but with cleared Enable bit.
>
> So use new U-Boot macro PCI_CONF1_ADDRESS() with clearing PCI_CONF1_ENABLE
> bit and remove old custom driver address macros.
>
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
>  drivers/pci/pci_msc01.c | 7 ++-----
>  include/msc01.h         | 9 ---------
>  2 files changed, 2 insertions(+), 14 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [PATCH u-boot-next 04/12] pci: msc01: Use PCI_CONF1_ADDRESS() macro

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 470 bytes --]

On Fri, Nov 26, 2021 at 11:42:44AM +0100, Pali Rohár wrote:

> PCI msc01 driver uses standard format of Config Address for PCI
> Configuration Mechanism #1 but with cleared Enable bit.
> 
> So use new U-Boot macro PCI_CONF1_ADDRESS() with clearing PCI_CONF1_ENABLE
> bit and remove old custom driver address macros.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

[PATCH u-boot-next 05/12] pci: mvebu: Use PCI_CONF1_EXT_ADDRESS() macro

[Pali Rohár]

PCI mvebu driver uses extended format of Config Address for PCI
Configuration Mechanism #1.

So use new U-Boot macro PCI_CONF1_EXT_ADDRESS() and remove old custom
driver address macros.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 drivers/pci/pci_mvebu.c | 17 ++++-------------
 1 file changed, 4 insertions(+), 13 deletions(-)

diff --git a/drivers/pci/pci_mvebu.c b/drivers/pci/pci_mvebu.c
index 14cd82db6ff8..aa0d6bd5162c 100644
--- a/drivers/pci/pci_mvebu.c
+++ b/drivers/pci/pci_mvebu.c
@@ -48,15 +48,6 @@ DECLARE_GLOBAL_DATA_PTR;
 #define PCIE_WIN5_BASE_OFF		0x1884
 #define PCIE_WIN5_REMAP_OFF		0x188c
 #define PCIE_CONF_ADDR_OFF		0x18f8
-#define  PCIE_CONF_ADDR_EN		BIT(31)
-#define  PCIE_CONF_REG(r)		((((r) & 0xf00) << 16) | ((r) & 0xfc))
-#define  PCIE_CONF_BUS(b)		(((b) & 0xff) << 16)
-#define  PCIE_CONF_DEV(d)		(((d) & 0x1f) << 11)
-#define  PCIE_CONF_FUNC(f)		(((f) & 0x7) << 8)
-#define  PCIE_CONF_ADDR(b, d, f, reg) \
-	(PCIE_CONF_BUS(b) | PCIE_CONF_DEV(d)    | \
-	 PCIE_CONF_FUNC(f) | PCIE_CONF_REG(reg) | \
-	 PCIE_CONF_ADDR_EN)
 #define PCIE_CONF_DATA_OFF		0x18fc
 #define PCIE_MASK_OFF			0x1910
 #define  PCIE_MASK_ENABLE_INTS          (0xf << 24)
@@ -188,9 +179,9 @@ static int mvebu_pcie_read_config(const struct udevice *bus, pci_dev_t bdf,
 	 * secondary bus with device number 1.
 	 */
 	if (busno == pcie->first_busno)
-		addr = PCIE_CONF_ADDR(pcie->sec_busno, 1, 0, offset);
+		addr = PCI_CONF1_EXT_ADDRESS(pcie->sec_busno, 1, 0, offset);
 	else
-		addr = PCIE_CONF_ADDR(busno, PCI_DEV(bdf), PCI_FUNC(bdf), offset);
+		addr = PCI_CONF1_EXT_ADDRESS(busno, PCI_DEV(bdf), PCI_FUNC(bdf), offset);
 
 	/* write address */
 	writel(addr, pcie->base + PCIE_CONF_ADDR_OFF);
@@ -286,9 +277,9 @@ static int mvebu_pcie_write_config(struct udevice *bus, pci_dev_t bdf,
 	 * secondary bus with device number 1.
 	 */
 	if (busno == pcie->first_busno)
-		addr = PCIE_CONF_ADDR(pcie->sec_busno, 1, 0, offset);
+		addr = PCI_CONF1_EXT_ADDRESS(pcie->sec_busno, 1, 0, offset);
 	else
-		addr = PCIE_CONF_ADDR(busno, PCI_DEV(bdf), PCI_FUNC(bdf), offset);
+		addr = PCI_CONF1_EXT_ADDRESS(busno, PCI_DEV(bdf), PCI_FUNC(bdf), offset);
 
 	/* write address */
 	writel(addr, pcie->base + PCIE_CONF_ADDR_OFF);
-- 
2.20.1

Re: [PATCH u-boot-next 05/12] pci: mvebu: Use PCI_CONF1_EXT_ADDRESS() macro

[Simon Glass]

On Fri, 26 Nov 2021 at 03:43, Pali Rohár <pali@kernel.org> wrote:
>
> PCI mvebu driver uses extended format of Config Address for PCI
> Configuration Mechanism #1.
>
> So use new U-Boot macro PCI_CONF1_EXT_ADDRESS() and remove old custom
> driver address macros.
>
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
>  drivers/pci/pci_mvebu.c | 17 ++++-------------
>  1 file changed, 4 insertions(+), 13 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [PATCH u-boot-next 05/12] pci: mvebu: Use PCI_CONF1_EXT_ADDRESS() macro

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 411 bytes --]

On Fri, Nov 26, 2021 at 11:42:45AM +0100, Pali Rohár wrote:

> PCI mvebu driver uses extended format of Config Address for PCI
> Configuration Mechanism #1.
> 
> So use new U-Boot macro PCI_CONF1_EXT_ADDRESS() and remove old custom
> driver address macros.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

[PATCH u-boot-next 06/12] pci: tegra: Use PCI_CONF1_EXT_ADDRESS() macro

[Pali Rohár]

PCI tegra driver uses extended format of Config Address for PCI
Configuration Mechanism #1 but with cleared Enable bit.

So use new U-Boot macro PCI_CONF1_EXT_ADDRESS() with clearing
PCI_CONF1_ENABLE bit and remove old custom driver address function.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 drivers/pci/pci_tegra.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/pci/pci_tegra.c b/drivers/pci/pci_tegra.c
index 9cb4414836f8..fc05ee00f1fc 100644
--- a/drivers/pci/pci_tegra.c
+++ b/drivers/pci/pci_tegra.c
@@ -275,13 +275,6 @@ static void rp_writel(struct tegra_pcie_port *port, unsigned long value,
 	writel(value, port->regs.start + offset);
 }
 
-static unsigned long tegra_pcie_conf_offset(pci_dev_t bdf, int where)
-{
-	return ((where & 0xf00) << 16) | (PCI_BUS(bdf) << 16) |
-	       (PCI_DEV(bdf) << 11) | (PCI_FUNC(bdf) << 8) |
-	       (where & 0xfc);
-}
-
 static int tegra_pcie_conf_address(struct tegra_pcie *pcie, pci_dev_t bdf,
 				   int where, unsigned long *address)
 {
@@ -305,7 +298,9 @@ static int tegra_pcie_conf_address(struct tegra_pcie *pcie, pci_dev_t bdf,
 			return -EFAULT;
 #endif
 
-		*address = pcie->cs.start + tegra_pcie_conf_offset(bdf, where);
+		*address = pcie->cs.start +
+			   (PCI_CONF1_EXT_ADDRESS(PCI_BUS(bdf), PCI_DEV(bdf),
+			    PCI_FUNC(bdf), where) & ~PCI_CONF1_ENABLE);
 		return 0;
 	}
 }
-- 
2.20.1

Re: [PATCH u-boot-next 06/12] pci: tegra: Use PCI_CONF1_EXT_ADDRESS() macro

[Simon Glass]

On Fri, 26 Nov 2021 at 03:43, Pali Rohár <pali@kernel.org> wrote:
>
> PCI tegra driver uses extended format of Config Address for PCI
> Configuration Mechanism #1 but with cleared Enable bit.
>
> So use new U-Boot macro PCI_CONF1_EXT_ADDRESS() with clearing
> PCI_CONF1_ENABLE bit and remove old custom driver address function.
>
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
>  drivers/pci/pci_tegra.c | 11 +++--------
>  1 file changed, 3 insertions(+), 8 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [PATCH u-boot-next 06/12] pci: tegra: Use PCI_CONF1_EXT_ADDRESS() macro

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 476 bytes --]

On Fri, Nov 26, 2021 at 11:42:46AM +0100, Pali Rohár wrote:

> PCI tegra driver uses extended format of Config Address for PCI
> Configuration Mechanism #1 but with cleared Enable bit.
> 
> So use new U-Boot macro PCI_CONF1_EXT_ADDRESS() with clearing
> PCI_CONF1_ENABLE bit and remove old custom driver address function.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

[PATCH u-boot-next 07/12] pci: fsl: Use PCI_CONF1_EXT_ADDRESS() macro

[Pali Rohár]

PCI fsl driver uses extended format of Config Address for PCI
Configuration Mechanism #1.

So use new U-Boot macro PCI_CONF1_EXT_ADDRESS().

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 drivers/pci/pcie_fsl.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/pcie_fsl.c b/drivers/pci/pcie_fsl.c
index 3c2a2a476111..cc6efdd5b464 100644
--- a/drivers/pci/pcie_fsl.c
+++ b/drivers/pci/pcie_fsl.c
@@ -58,8 +58,9 @@ static int fsl_pcie_read_config(const struct udevice *bus, pci_dev_t bdf,
 		return 0;
 	}
 
-	bdf = bdf - PCI_BDF(dev_seq(bus), 0, 0);
-	val = bdf | (offset & 0xfc) | ((offset & 0xf00) << 16) | 0x80000000;
+	val = PCI_CONF1_EXT_ADDRESS(PCI_BUS(bdf) - dev_seq(bus),
+				    PCI_DEV(bdf), PCI_FUNC(bdf),
+				    offset);
 	out_be32(&regs->cfg_addr, val);
 
 	sync();
@@ -94,8 +95,9 @@ static int fsl_pcie_write_config(struct udevice *bus, pci_dev_t bdf,
 	if (fsl_pcie_addr_valid(pcie, bdf))
 		return 0;
 
-	bdf = bdf - PCI_BDF(dev_seq(bus), 0, 0);
-	val = bdf | (offset & 0xfc) | ((offset & 0xf00) << 16) | 0x80000000;
+	val = PCI_CONF1_EXT_ADDRESS(PCI_BUS(bdf) - dev_seq(bus),
+				    PCI_DEV(bdf), PCI_FUNC(bdf),
+				    offset);
 	out_be32(&regs->cfg_addr, val);
 
 	sync();
-- 
2.20.1

Re: [PATCH u-boot-next 07/12] pci: fsl: Use PCI_CONF1_EXT_ADDRESS() macro

[Simon Glass]

On Fri, 26 Nov 2021 at 03:43, Pali Rohár <pali@kernel.org> wrote:
>
> PCI fsl driver uses extended format of Config Address for PCI
> Configuration Mechanism #1.
>
> So use new U-Boot macro PCI_CONF1_EXT_ADDRESS().
>
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
>  drivers/pci/pcie_fsl.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [PATCH u-boot-next 07/12] pci: fsl: Use PCI_CONF1_EXT_ADDRESS() macro

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 362 bytes --]

On Fri, Nov 26, 2021 at 11:42:47AM +0100, Pali Rohár wrote:

> PCI fsl driver uses extended format of Config Address for PCI
> Configuration Mechanism #1.
> 
> So use new U-Boot macro PCI_CONF1_EXT_ADDRESS().
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

[PATCH u-boot-next 08/12] pci: mediatek: Use PCI_CONF1_EXT_ADDRESS() macro

[Pali Rohár]

PCI mediatek driver uses extended format of Config Address for PCI
Configuration Mechanism #1 but with cleared Enable bit.

So use new U-Boot macro PCI_CONF1_EXT_ADDRESS() with clearing
PCI_CONF1_ENABLE bit and remove old custom driver address macros.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 drivers/pci/pcie_mediatek.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/pci/pcie_mediatek.c b/drivers/pci/pcie_mediatek.c
index f5556713878f..051a3bc96935 100644
--- a/drivers/pci/pcie_mediatek.c
+++ b/drivers/pci/pcie_mediatek.c
@@ -41,10 +41,6 @@
 #define PCIE_BAR_ENABLE		BIT(0)
 #define PCIE_REVISION_ID	BIT(0)
 #define PCIE_CLASS_CODE		(0x60400 << 8)
-#define PCIE_CONF_REG(regn)	(((regn) & GENMASK(7, 2)) | \
-				((((regn) >> 8) & GENMASK(3, 0)) << 24))
-#define PCIE_CONF_ADDR(regn, bdf) \
-				(PCIE_CONF_REG(regn) | (bdf))
 
 /* MediaTek specific configuration registers */
 #define PCIE_FTS_NUM		0x70c
@@ -147,8 +143,11 @@ static int mtk_pcie_config_address(const struct udevice *udev, pci_dev_t bdf,
 				   uint offset, void **paddress)
 {
 	struct mtk_pcie *pcie = dev_get_priv(udev);
+	u32 val;
 
-	writel(PCIE_CONF_ADDR(offset, bdf), pcie->base + PCIE_CFG_ADDR);
+	val = PCI_CONF1_EXT_ADDRESS(PCI_BUS(bdf), PCI_DEV(bdf),
+				    PCI_FUNC(bdf), offset) & ~PCI_CONF1_ENABLE;
+	writel(val, pcie->base + PCIE_CFG_ADDR);
 	*paddress = pcie->base + PCIE_CFG_DATA + (offset & 3);
 
 	return 0;
@@ -330,7 +329,6 @@ static void mtk_pcie_port_free(struct mtk_pcie_port *port)
 static int mtk_pcie_startup_port(struct mtk_pcie_port *port)
 {
 	struct mtk_pcie *pcie = port->pcie;
-	u32 slot = PCI_DEV(port->slot << 11);
 	u32 val;
 	int err;
 
@@ -357,13 +355,14 @@ static int mtk_pcie_startup_port(struct mtk_pcie_port *port)
 	writel(PCIE_CLASS_CODE | PCIE_REVISION_ID, port->base + PCIE_CLASS);
 
 	/* configure FC credit */
-	writel(PCIE_CONF_ADDR(PCIE_FC_CREDIT, slot),
-	       pcie->base + PCIE_CFG_ADDR);
+	val = PCI_CONF1_EXT_ADDRESS(0, port->slot, 0, PCIE_FC_CREDIT) & ~PCI_CONF1_ENABLE;
+	writel(val, pcie->base + PCIE_CFG_ADDR);
 	clrsetbits_le32(pcie->base + PCIE_CFG_DATA, PCIE_FC_CREDIT_MASK,
 			PCIE_FC_CREDIT_VAL(0x806c));
 
 	/* configure RC FTS number to 250 when it leaves L0s */
-	writel(PCIE_CONF_ADDR(PCIE_FTS_NUM, slot), pcie->base + PCIE_CFG_ADDR);
+	val = PCI_CONF1_EXT_ADDRESS(0, port->slot, 0, PCIE_FTS_NUM) & ~PCI_CONF1_ENABLE;
+	writel(val, pcie->base + PCIE_CFG_ADDR);
 	clrsetbits_le32(pcie->base + PCIE_CFG_DATA, PCIE_FTS_NUM_MASK,
 			PCIE_FTS_NUM_L0(0x50));
 
-- 
2.20.1

Re: [PATCH u-boot-next 08/12] pci: mediatek: Use PCI_CONF1_EXT_ADDRESS() macro

[Simon Glass]

On Fri, 26 Nov 2021 at 03:43, Pali Rohár <pali@kernel.org> wrote:
>
> PCI mediatek driver uses extended format of Config Address for PCI
> Configuration Mechanism #1 but with cleared Enable bit.
>
> So use new U-Boot macro PCI_CONF1_EXT_ADDRESS() with clearing
> PCI_CONF1_ENABLE bit and remove old custom driver address macros.
>
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
>  drivers/pci/pcie_mediatek.c | 17 ++++++++---------
>  1 file changed, 8 insertions(+), 9 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [PATCH u-boot-next 08/12] pci: mediatek: Use PCI_CONF1_EXT_ADDRESS() macro

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 477 bytes --]

On Fri, Nov 26, 2021 at 11:42:48AM +0100, Pali Rohár wrote:

> PCI mediatek driver uses extended format of Config Address for PCI
> Configuration Mechanism #1 but with cleared Enable bit.
> 
> So use new U-Boot macro PCI_CONF1_EXT_ADDRESS() with clearing
> PCI_CONF1_ENABLE bit and remove old custom driver address macros.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

[PATCH u-boot-next 09/12] pci: sh7780: Use PCI_CONF1_ADDRESS() macro

[Pali Rohár]

PCI sh7780 driver uses standard format of Config Address for PCI
Configuration Mechanism #1.

So use new U-Boot macro PCI_CONF1_ADDRESS().

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 drivers/pci/pci_sh7780.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/pci_sh7780.c b/drivers/pci/pci_sh7780.c
index 06d711a6cb9e..7533286c0156 100644
--- a/drivers/pci/pci_sh7780.c
+++ b/drivers/pci/pci_sh7780.c
@@ -34,9 +34,9 @@
 int pci_sh4_read_config_dword(struct pci_controller *hose,
 				    pci_dev_t dev, int offset, u32 *value)
 {
-	u32 par_data = 0x80000000 | dev;
+	u32 par_data = PCI_CONF1_ADDRESS(PCI_BUS(dev), PCI_DEV(dev), PCI_FUNC(dev), offset);
 
-	p4_out(par_data | (offset & 0xfc), SH7780_PCIPAR);
+	p4_out(par_data, SH7780_PCIPAR);
 	*value = p4_in(SH7780_PCIPDR);
 
 	return 0;
@@ -45,9 +45,9 @@ int pci_sh4_read_config_dword(struct pci_controller *hose,
 int pci_sh4_write_config_dword(struct pci_controller *hose,
 				     pci_dev_t dev, int offset, u32 value)
 {
-	u32 par_data = 0x80000000 | dev;
+	u32 par_data = PCI_CONF1_ADDRESS(PCI_BUS(dev), PCI_DEV(dev), PCI_FUNC(dev), offset);
 
-	p4_out(par_data | (offset & 0xfc), SH7780_PCIPAR);
+	p4_out(par_data, SH7780_PCIPAR);
 	p4_out(value, SH7780_PCIPDR);
 	return 0;
 }
-- 
2.20.1

Re: [PATCH u-boot-next 09/12] pci: sh7780: Use PCI_CONF1_ADDRESS() macro

[Simon Glass]

On Fri, 26 Nov 2021 at 03:43, Pali Rohár <pali@kernel.org> wrote:
>
> PCI sh7780 driver uses standard format of Config Address for PCI
> Configuration Mechanism #1.
>
> So use new U-Boot macro PCI_CONF1_ADDRESS().
>
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
>  drivers/pci/pci_sh7780.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [PATCH u-boot-next 09/12] pci: sh7780: Use PCI_CONF1_ADDRESS() macro

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 361 bytes --]

On Fri, Nov 26, 2021 at 11:42:49AM +0100, Pali Rohár wrote:

> PCI sh7780 driver uses standard format of Config Address for PCI
> Configuration Mechanism #1.
> 
> So use new U-Boot macro PCI_CONF1_ADDRESS().
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

[PATCH u-boot-next 10/12] x86: pci: Use PCI_CONF1_ADDRESS() macro

[Pali Rohár]

x86 platform uses standard format of Config Address for PCI Configuration
Mechanism #1. So use new U-Boot macro PCI_CONF1_ADDRESS().

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 arch/x86/cpu/pci.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/cpu/pci.c b/arch/x86/cpu/pci.c
index d4f9290ca73b..8a992ed82339 100644
--- a/arch/x86/cpu/pci.c
+++ b/arch/x86/cpu/pci.c
@@ -20,7 +20,7 @@
 int pci_x86_read_config(pci_dev_t bdf, uint offset, ulong *valuep,
 			enum pci_size_t size)
 {
-	outl(bdf | (offset & 0xfc) | PCI_CFG_EN, PCI_REG_ADDR);
+	outl(PCI_CONF1_ADDRESS(PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf), offset), PCI_REG_ADDR);
 	switch (size) {
 	case PCI_SIZE_8:
 		*valuep = inb(PCI_REG_DATA + (offset & 3));
@@ -39,7 +39,7 @@ int pci_x86_read_config(pci_dev_t bdf, uint offset, ulong *valuep,
 int pci_x86_write_config(pci_dev_t bdf, uint offset, ulong value,
 			 enum pci_size_t size)
 {
-	outl(bdf | (offset & 0xfc) | PCI_CFG_EN, PCI_REG_ADDR);
+	outl(PCI_CONF1_ADDRESS(PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf), offset), PCI_REG_ADDR);
 	switch (size) {
 	case PCI_SIZE_8:
 		outb(value, PCI_REG_DATA + (offset & 3));
-- 
2.20.1

Re: [PATCH u-boot-next 10/12] x86: pci: Use PCI_CONF1_ADDRESS() macro

[Simon Glass]

On Fri, 26 Nov 2021 at 03:43, Pali Rohár <pali@kernel.org> wrote:
>
> x86 platform uses standard format of Config Address for PCI Configuration
> Mechanism #1. So use new U-Boot macro PCI_CONF1_ADDRESS().
>
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
>  arch/x86/cpu/pci.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [PATCH u-boot-next 10/12] x86: pci: Use PCI_CONF1_ADDRESS() macro

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 349 bytes --]

On Fri, Nov 26, 2021 at 11:42:50AM +0100, Pali Rohár wrote:

> x86 platform uses standard format of Config Address for PCI Configuration
> Mechanism #1. So use new U-Boot macro PCI_CONF1_ADDRESS().
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

[PATCH u-boot-next 11/12] m68k: mcf5445x: pci: Use PCI_CONF1_ADDRESS() macro

[Pali Rohár]

mcf5445x platform uses standard format of Config Address for PCI
Configuration Mechanism #1. So use new U-Boot macro PCI_CONF1_ADDRESS().

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 arch/m68k/cpu/mcf5445x/pci.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/m68k/cpu/mcf5445x/pci.c b/arch/m68k/cpu/mcf5445x/pci.c
index af02c4934c97..d487468d0bfa 100644
--- a/arch/m68k/cpu/mcf5445x/pci.c
+++ b/arch/m68k/cpu/mcf5445x/pci.c
@@ -26,12 +26,11 @@
 int pci_##rw##_cfg_##size(struct pci_controller *hose,			\
 	pci_dev_t dev, int offset, type val)				\
 {									\
-	u32 addr = 0;							\
-	u16 cfg_type = 0;						\
-	addr = ((offset & 0xfc) | cfg_type | (dev)  | 0x80000000);	\
+	u32 addr = PCI_CONF1_ADDRESS(PCI_BUS(dev), PCI_DEV(dev),	\
+				     PCI_FUNC(dev), offset);		\
 	out_be32(hose->cfg_addr, addr);					\
 	cfg_##rw(val, hose->cfg_data + (offset & mask), type, op);	\
-	out_be32(hose->cfg_addr, addr & 0x7fffffff);			\
+	out_be32(hose->cfg_addr, addr & ~PCI_CONF1_ENABLE);		\
 	return 0;							\
 }
 
-- 
2.20.1

Re: [PATCH u-boot-next 11/12] m68k: mcf5445x: pci: Use PCI_CONF1_ADDRESS() macro

[Simon Glass]

On Fri, 26 Nov 2021 at 03:43, Pali Rohár <pali@kernel.org> wrote:
>
> mcf5445x platform uses standard format of Config Address for PCI
> Configuration Mechanism #1. So use new U-Boot macro PCI_CONF1_ADDRESS().
>
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
>  arch/m68k/cpu/mcf5445x/pci.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [PATCH u-boot-next 11/12] m68k: mcf5445x: pci: Use PCI_CONF1_ADDRESS() macro

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 354 bytes --]

On Fri, Nov 26, 2021 at 11:42:51AM +0100, Pali Rohár wrote:

> mcf5445x platform uses standard format of Config Address for PCI
> Configuration Mechanism #1. So use new U-Boot macro PCI_CONF1_ADDRESS().
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

[PATCH u-boot-next 12/12] pci: sh7751: Fix access to config space via PCI_CONF1_ADDRESS() macro

[Pali Rohár]

sh7751 platform uses standard format of Config Address for PCI
Configuration Mechanism #1.

Commit 72c2f4acd76f ("pci: sh7751: Convert to DM and DT probing") which did
conversion of PCI sh7751 driver to DM, broke access to config space as that
commit somehow swapped device and function bits in config address.

Fix all these issues by using new U-Boot macro PCI_CONF1_ADDRESS() which
calculates Config Address correctly.

Also remove nonsense function sh7751_pci_addr_valid() which was introduced
in commit 72c2f4acd76f ("pci: sh7751: Convert to DM and DT probing")
probably due to workarounded issues with mixing/swapping device and
function bits of config address which probably resulted in non-working
access to some devices. With correct composing of config address there
should not be such issue anymore.

Signed-off-by: Pali Rohár <pali@kernel.org>
Fixes: 72c2f4acd76f ("pci: sh7751: Convert to DM and DT probing")
Cc: Marek Vasut <marek.vasut+renesas@gmail.com>
---
 drivers/pci/pci_sh7751.c | 29 ++---------------------------
 1 file changed, 2 insertions(+), 27 deletions(-)

diff --git a/drivers/pci/pci_sh7751.c b/drivers/pci/pci_sh7751.c
index e110550c71c8..d514c040344c 100644
--- a/drivers/pci/pci_sh7751.c
+++ b/drivers/pci/pci_sh7751.c
@@ -74,33 +74,13 @@
 #define p4_in(addr)	(*addr)
 #define p4_out(data, addr) (*addr) = (data)
 
-static int sh7751_pci_addr_valid(pci_dev_t d, uint offset)
-{
-	if (PCI_FUNC(d))
-		return -EINVAL;
-
-	return 0;
-}
-
-static u32 get_bus_address(const struct udevice *dev, pci_dev_t bdf, u32 offset)
-{
-	return BIT(31) | (PCI_DEV(bdf) << 8) | (offset & ~3);
-}
-
 static int sh7751_pci_read_config(const struct udevice *dev, pci_dev_t bdf,
 				  uint offset, ulong *value,
 				  enum pci_size_t size)
 {
 	u32 addr, reg;
-	int ret;
 
-	ret = sh7751_pci_addr_valid(bdf, offset);
-	if (ret) {
-		*value = pci_get_ff(size);
-		return 0;
-	}
-
-	addr = get_bus_address(dev, bdf, offset);
+	addr = PCI_CONF1_ADDRESS(PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf), offset);
 	p4_out(addr, SH7751_PCIPAR);
 	reg = p4_in(SH7751_PCIPDR);
 	*value = pci_conv_32_to_size(reg, offset, size);
@@ -113,13 +93,8 @@ static int sh7751_pci_write_config(struct udevice *dev, pci_dev_t bdf,
 				      enum pci_size_t size)
 {
 	u32 addr, reg, old;
-	int ret;
-
-	ret = sh7751_pci_addr_valid(bdf, offset);
-	if (ret)
-		return ret;
 
-	addr = get_bus_address(dev, bdf, offset);
+	addr = PCI_CONF1_ADDRESS(PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf), offset);
 	p4_out(addr, SH7751_PCIPAR);
 	old = p4_in(SH7751_PCIPDR);
 	reg = pci_conv_size_to_32(old, value, offset, size);
-- 
2.20.1

Re: [PATCH u-boot-next 12/12] pci: sh7751: Fix access to config space via PCI_CONF1_ADDRESS() macro

[Simon Glass]

On Fri, 26 Nov 2021 at 03:43, Pali Rohár <pali@kernel.org> wrote:
>
> sh7751 platform uses standard format of Config Address for PCI
> Configuration Mechanism #1.
>
> Commit 72c2f4acd76f ("pci: sh7751: Convert to DM and DT probing") which did
> conversion of PCI sh7751 driver to DM, broke access to config space as that
> commit somehow swapped device and function bits in config address.
>
> Fix all these issues by using new U-Boot macro PCI_CONF1_ADDRESS() which
> calculates Config Address correctly.
>
> Also remove nonsense function sh7751_pci_addr_valid() which was introduced
> in commit 72c2f4acd76f ("pci: sh7751: Convert to DM and DT probing")
> probably due to workarounded issues with mixing/swapping device and
> function bits of config address which probably resulted in non-working
> access to some devices. With correct composing of config address there
> should not be such issue anymore.
>
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Fixes: 72c2f4acd76f ("pci: sh7751: Convert to DM and DT probing")
> Cc: Marek Vasut <marek.vasut+renesas@gmail.com>
> ---
>  drivers/pci/pci_sh7751.c | 29 ++---------------------------
>  1 file changed, 2 insertions(+), 27 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [PATCH u-boot-next 12/12] pci: sh7751: Fix access to config space via PCI_CONF1_ADDRESS() macro

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1189 bytes --]

On Fri, Nov 26, 2021 at 11:42:52AM +0100, Pali Rohár wrote:

> sh7751 platform uses standard format of Config Address for PCI
> Configuration Mechanism #1.
> 
> Commit 72c2f4acd76f ("pci: sh7751: Convert to DM and DT probing") which did
> conversion of PCI sh7751 driver to DM, broke access to config space as that
> commit somehow swapped device and function bits in config address.
> 
> Fix all these issues by using new U-Boot macro PCI_CONF1_ADDRESS() which
> calculates Config Address correctly.
> 
> Also remove nonsense function sh7751_pci_addr_valid() which was introduced
> in commit 72c2f4acd76f ("pci: sh7751: Convert to DM and DT probing")
> probably due to workarounded issues with mixing/swapping device and
> function bits of config address which probably resulted in non-working
> access to some devices. With correct composing of config address there
> should not be such issue anymore.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Fixes: 72c2f4acd76f ("pci: sh7751: Convert to DM and DT probing")
> Cc: Marek Vasut <marek.vasut+renesas@gmail.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH u-boot-next 00/12] Common U-Boot macros for PCI Configuration Mechanism #1

[Pali Rohár]

Hello! Could you look and review at this patch series?

On Friday 26 November 2021 11:42:40 Pali Rohár wrote:
> This patch series add new U-Boot macros for composing Config Address for
> PCI Configuration Mechanism #1 as defined in PCI Local Bus Specification,
> including extended variant (which do not have any formal specification)
> and then use these new macros (PCI_CONF1_ADDRESS and PCI_CONF1_EXT_ADDRESS)
> in all PCI drivers that use Config Address according to PCI Configuration
> Mechanism #1.
> 
> PCI Configuration Mechanism #1 was originally specified for x86 platforms
> and it is still supported on x86 together with PCIe ECAM. Nowadays lot of
> non-ECAM-compliant ARM PCIe controllers use only extended variant of this
> address construction and some of them requires cleared Enable bit. Extended
> variant is also supported by x86 AMD Barcelona (and new) CPUs, but U-Boot
> code does not provide this support yet.
> 
> Note that it exists also PCI Configuration Mechanism #2, but this one was
> removed from PCI Local Bus Specification revision 3.0 and seems that it is
> not used by any PCI driver in U-Boot. So I have not added macros for this
> mechanism in this patch series.
> 
> Because lot of hardware still use this (rather old) mechanism, relevant
> U-Boot PCI and PCIe drivers have own ad-hoc code address construction,
> which is repeated and written in different ways.
> 
> This patch series is code cleanup of these PCIe drivers to use common
> U-Boot macros for PCI Configuration Mechanism #1.
> 
> The last change is fixing construction of Config Address in PCI driver
> sh7751. Construction with this change matches Linux kernel code and also
> U-Boot code prior commit mentioned in commit message.
> 
> I have tested this patch series only for PCI mvebu driver on A385 board and
> it is working fine. So Please properly review all other changes. Ideally
> test them.
> 
> I have run CI tests with these changes on github and everything passed:
> https://github.com/u-boot/u-boot/pull/101
> 
> Please let me know what do you think about this change.
> 
> Pali Rohár (12):
>   pci: Add standard PCI Config Address macros
>   pci: gt64120: Use PCI_CONF1_ADDRESS() macro
>   pci: mpc85xx: Use PCI_CONF1_EXT_ADDRESS() macro
>   pci: msc01: Use PCI_CONF1_ADDRESS() macro
>   pci: mvebu: Use PCI_CONF1_EXT_ADDRESS() macro
>   pci: tegra: Use PCI_CONF1_EXT_ADDRESS() macro
>   pci: fsl: Use PCI_CONF1_EXT_ADDRESS() macro
>   pci: mediatek: Use PCI_CONF1_EXT_ADDRESS() macro
>   pci: sh7780: Use PCI_CONF1_ADDRESS() macro
>   x86: pci: Use PCI_CONF1_ADDRESS() macro
>   m68k: mcf5445x: pci: Use PCI_CONF1_ADDRESS() macro
>   pci: sh7751: Fix access to config space via PCI_CONF1_ADDRESS() macro
> 
>  arch/m68k/cpu/mcf5445x/pci.c |  7 +++---
>  arch/x86/cpu/pci.c           |  4 ++--
>  drivers/pci/pci_gt64120.c    |  7 ++----
>  drivers/pci/pci_mpc85xx.c    |  4 ++--
>  drivers/pci/pci_msc01.c      |  7 ++----
>  drivers/pci/pci_mvebu.c      | 17 ++++----------
>  drivers/pci/pci_sh7751.c     | 29 ++---------------------
>  drivers/pci/pci_sh7780.c     |  8 +++----
>  drivers/pci/pci_tegra.c      | 11 +++------
>  drivers/pci/pcie_fsl.c       | 10 ++++----
>  drivers/pci/pcie_mediatek.c  | 17 +++++++-------
>  include/gt64120.h            | 12 ----------
>  include/msc01.h              |  9 --------
>  include/pci.h                | 45 ++++++++++++++++++++++++++++++++++++
>  14 files changed, 83 insertions(+), 104 deletions(-)
> 
> -- 
> 2.20.1
>