[PATCH u-boot 0/3] pci: mpc85xx: Fixes for PCI config space

This patch series contains small fixes for mpc85xx old PCI Local Bus driver.
Heiko: Are you able to test these changes? Has your Socrates board available old PCI Local Bus support?
Pali Rohár (3): pci: mpc85xx: Add missing sync() after writing to PCI config space pci: mpc85xx: Allow 8/16-bit access to PCI config space pci: mpc85xx: Do not try to access extended PCIe registers
drivers/pci/pci_mpc85xx.c | 39 +++++++++++++++++++++++++++++++++++---- 1 file changed, 35 insertions(+), 4 deletions(-)

On PowerPC we should use barrier after store operation to HW register.
Signed-off-by: Pali Rohár pali@kernel.org --- drivers/pci/pci_mpc85xx.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/pci/pci_mpc85xx.c b/drivers/pci/pci_mpc85xx.c index 8a81a74067e9..23f14db83018 100644 --- a/drivers/pci/pci_mpc85xx.c +++ b/drivers/pci/pci_mpc85xx.c @@ -41,6 +41,7 @@ static int mpc85xx_pci_dm_write_config(struct udevice *dev, pci_dev_t bdf, out_be32(priv->cfg_addr, addr); sync(); out_le32(priv->cfg_data, pci_conv_size_to_32(0, value, offset, size)); + sync();
return 0; }

Hello Pali,
On 13.04.23 22:41, Pali Rohár wrote:
On PowerPC we should use barrier after store operation to HW register.
Signed-off-by: Pali Rohár pali@kernel.org
drivers/pci/pci_mpc85xx.c | 1 + 1 file changed, 1 insertion(+)
Good catch, thanks!
Reviewed-by: Heiko Schocher hs@denx.de Tested-by: Heiko Schocher hs@denx.de
bye, Heiko

On Thu, Apr 13, 2023 at 10:41:44PM +0200, Pali Rohár wrote:
On PowerPC we should use barrier after store operation to HW register.
Signed-off-by: Pali Rohár pali@kernel.org Reviewed-by: Heiko Schocher hs@denx.de Tested-by: Heiko Schocher hs@denx.de
Applied to u-boot/master, thanks!

This Freescale mpc85xx PCI controller should support 8-bit and 16-bit read and write access to PCI config space as described in more Freescale reference manuals.
This change fixes issue that 8-bit and 16-bit write to PCI config space caused to clear adjacent bits of 32-bit PCI register.
Signed-off-by: Pali Rohár pali@kernel.org --- drivers/pci/pci_mpc85xx.c | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/pci_mpc85xx.c b/drivers/pci/pci_mpc85xx.c index 23f14db83018..d144f2b791b8 100644 --- a/drivers/pci/pci_mpc85xx.c +++ b/drivers/pci/pci_mpc85xx.c @@ -25,7 +25,18 @@ static int mpc85xx_pci_dm_read_config(const struct udevice *dev, pci_dev_t bdf, 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); + + switch (size) { + case PCI_SIZE_8: + *value = in_8(priv->cfg_data + (offset & 3)); + break; + case PCI_SIZE_16: + *value = in_le16(priv->cfg_data + (offset & 2)); + break; + case PCI_SIZE_32: + *value = in_le32(priv->cfg_data); + break; + }
return 0; } @@ -40,7 +51,18 @@ static int mpc85xx_pci_dm_write_config(struct udevice *dev, pci_dev_t bdf, 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)); + + switch (size) { + case PCI_SIZE_8: + out_8(priv->cfg_data + (offset & 3), value); + break; + case PCI_SIZE_16: + out_le16(priv->cfg_data + (offset & 2), value); + break; + case PCI_SIZE_32: + out_le32(priv->cfg_data, value); + break; + } sync();
return 0;

Hello Pali,
On 13.04.23 22:41, Pali Rohár wrote:
This Freescale mpc85xx PCI controller should support 8-bit and 16-bit read and write access to PCI config space as described in more Freescale reference manuals.
This change fixes issue that 8-bit and 16-bit write to PCI config space caused to clear adjacent bits of 32-bit PCI register.
Signed-off-by: Pali Rohár pali@kernel.org
drivers/pci/pci_mpc85xx.c | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-)
Thanks!
Reviewed-by: Heiko Schocher hs@denx.de Tested-by: Heiko Schocher hs@denx.de
bye, Heiko

On Thu, Apr 13, 2023 at 10:41:45PM +0200, Pali Rohár wrote:
This Freescale mpc85xx PCI controller should support 8-bit and 16-bit read and write access to PCI config space as described in more Freescale reference manuals.
This change fixes issue that 8-bit and 16-bit write to PCI config space caused to clear adjacent bits of 32-bit PCI register.
Signed-off-by: Pali Rohár pali@kernel.org Reviewed-by: Heiko Schocher hs@denx.de Tested-by: Heiko Schocher hs@denx.de
Applied to u-boot/master, thanks!

Driver pci_mpc85xx.c is PCI controller driver for old PCI Local Bus, which does not support access to extended PCIe registers (above 0xff), as opposite of the PCIe driver pcie_fsl.c for the same platform.
So do not try to access extended PCIe registers as it cannot work.
Signed-off-by: Pali Rohár pali@kernel.org --- drivers/pci/pci_mpc85xx.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/pci_mpc85xx.c b/drivers/pci/pci_mpc85xx.c index d144f2b791b8..833de816c459 100644 --- a/drivers/pci/pci_mpc85xx.c +++ b/drivers/pci/pci_mpc85xx.c @@ -22,7 +22,12 @@ 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 = PCI_CONF1_EXT_ADDRESS(PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf), offset); + if (offset > 0xff) { + *value = pci_get_ff(size); + return 0; + } + + addr = PCI_CONF1_ADDRESS(PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf), offset); out_be32(priv->cfg_addr, addr); sync();
@@ -48,7 +53,10 @@ 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 = PCI_CONF1_EXT_ADDRESS(PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf), offset); + if (offset > 0xff) + return 0; + + addr = PCI_CONF1_ADDRESS(PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf), offset); out_be32(priv->cfg_addr, addr); sync();

Hello Pali,
On 13.04.23 22:41, Pali Rohár wrote:
Driver pci_mpc85xx.c is PCI controller driver for old PCI Local Bus, which does not support access to extended PCIe registers (above 0xff), as opposite of the PCIe driver pcie_fsl.c for the same platform.
So do not try to access extended PCIe registers as it cannot work.
Signed-off-by: Pali Rohár pali@kernel.org
drivers/pci/pci_mpc85xx.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)
Thanks!
Reviewed-by: Heiko Schocher hs@denx.de Tested-by: Heiko Schocher hs@denx.de
bye, Heiko

On Thu, Apr 13, 2023 at 10:41:46PM +0200, Pali Rohár wrote:
Driver pci_mpc85xx.c is PCI controller driver for old PCI Local Bus, which does not support access to extended PCIe registers (above 0xff), as opposite of the PCIe driver pcie_fsl.c for the same platform.
So do not try to access extended PCIe registers as it cannot work.
Signed-off-by: Pali Rohár pali@kernel.org Reviewed-by: Heiko Schocher hs@denx.de Tested-by: Heiko Schocher hs@denx.de
Applied to u-boot/master, thanks!

Hello Pali,
On 13.04.23 22:41, Pali Rohár wrote:
This patch series contains small fixes for mpc85xx old PCI Local Bus driver.
Heiko: Are you able to test these changes? Has your Socrates board available old PCI Local Bus support?
Pali Rohár (3): pci: mpc85xx: Add missing sync() after writing to PCI config space pci: mpc85xx: Allow 8/16-bit access to PCI config space pci: mpc85xx: Do not try to access extended PCIe registers
drivers/pci/pci_mpc85xx.c | 39 +++++++++++++++++++++++++++++++++++---- 1 file changed, 35 insertions(+), 4 deletions(-)
I have to look, try your patches soon, thanks!
bye, Heiko

Hello Pali,
On 13.04.23 22:41, Pali Rohár wrote:
This patch series contains small fixes for mpc85xx old PCI Local Bus driver.
Heiko: Are you able to test these changes? Has your Socrates board available old PCI Local Bus support?
Pali Rohár (3): pci: mpc85xx: Add missing sync() after writing to PCI config space pci: mpc85xx: Allow 8/16-bit access to PCI config space pci: mpc85xx: Do not try to access extended PCIe registers
drivers/pci/pci_mpc85xx.c | 39 +++++++++++++++++++++++++++++++++++---- 1 file changed, 35 insertions(+), 4 deletions(-)
I just applied your patches to my U-Boot (based on 2023.04) running on the socrates board. I see no problems.
I have there:
=> pci BusDevFun VendorId DeviceId Device Class Sub-Class _____________________________________________________________ 00.00.00 0x1957 0x0033 Processor 0x20 00.11.00 0x1131 0x1561 Serial bus controller 0x03 00.11.01 0x1131 0x1561 Serial bus controller 0x03 00.11.02 0x1131 0x1562 Serial bus controller 0x03 00.12.00 0x1131 0x1561 Serial bus controller 0x03 00.12.01 0x1131 0x1561 Serial bus controller 0x03 00.12.02 0x1131 0x1562 Serial bus controller 0x03 =>
=> usb start starting USB... Bus ohci_pci: USB OHCI 1.0 Bus ohci_pci: USB OHCI 1.0 Bus ohci_pci: USB OHCI 1.0 Bus ohci_pci: USB OHCI 1.0 scanning bus ohci_pci for devices... 1 USB Device(s) found scanning bus ohci_pci for devices... 1 USB Device(s) found scanning bus ohci_pci for devices... 1 USB Device(s) found scanning bus ohci_pci for devices... 2 USB Device(s) found scanning usb for storage devices... 1 Storage Device(s) found => usb storage Device 0: Vendor: Mass Rev: 1.00 Prod: Storage Device Type: Removable Hard Disk Capacity: 30424.5 MB = 29.7 GB (62309376 x 512) =>
So, I see no problems with your patches.
bye, Heiko

On Monday 17 April 2023 07:12:16 Heiko Schocher wrote:
Hello Pali,
On 13.04.23 22:41, Pali Rohár wrote:
This patch series contains small fixes for mpc85xx old PCI Local Bus driver.
Heiko: Are you able to test these changes? Has your Socrates board available old PCI Local Bus support?
Pali Rohár (3): pci: mpc85xx: Add missing sync() after writing to PCI config space pci: mpc85xx: Allow 8/16-bit access to PCI config space pci: mpc85xx: Do not try to access extended PCIe registers
drivers/pci/pci_mpc85xx.c | 39 +++++++++++++++++++++++++++++++++++---- 1 file changed, 35 insertions(+), 4 deletions(-)
I just applied your patches to my U-Boot (based on 2023.04) running on the socrates board. I see no problems.
I have there:
=> pci BusDevFun VendorId DeviceId Device Class Sub-Class _____________________________________________________________ 00.00.00 0x1957 0x0033 Processor 0x20 00.11.00 0x1131 0x1561 Serial bus controller 0x03 00.11.01 0x1131 0x1561 Serial bus controller 0x03 00.11.02 0x1131 0x1562 Serial bus controller 0x03 00.12.00 0x1131 0x1561 Serial bus controller 0x03 00.12.01 0x1131 0x1561 Serial bus controller 0x03 00.12.02 0x1131 0x1562 Serial bus controller 0x03 =>
=> usb start starting USB... Bus ohci_pci: USB OHCI 1.0 Bus ohci_pci: USB OHCI 1.0 Bus ohci_pci: USB OHCI 1.0 Bus ohci_pci: USB OHCI 1.0 scanning bus ohci_pci for devices... 1 USB Device(s) found scanning bus ohci_pci for devices... 1 USB Device(s) found scanning bus ohci_pci for devices... 1 USB Device(s) found scanning bus ohci_pci for devices... 2 USB Device(s) found scanning usb for storage devices... 1 Storage Device(s) found => usb storage Device 0: Vendor: Mass Rev: 1.00 Prod: Storage Device Type: Removable Hard Disk Capacity: 30424.5 MB = 29.7 GB (62309376 x 512) =>
So, I see no problems with your patches.
Thanks for testing!
Could you send me dump of the PCI config space from u-boot and also from Linux? So I could compare it if U-Boot is doing everything correctly.
In U-Boot for first two devices:
=> pci display.b 00.00.00 0x0 0x1000 => pci display.b 00.11.00 0x0 0x1000
And in Linux for all devices:
# lspci -xxxx -nn
bye, Heiko -- DENX Software Engineering GmbH, Managing Director: Erika Unter HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-52 Fax: +49-8142-66989-80 Email: hs@denx.de

On Thursday 13 April 2023 22:41:43 Pali Rohár wrote:
This patch series contains small fixes for mpc85xx old PCI Local Bus driver.
Heiko: Are you able to test these changes? Has your Socrates board available old PCI Local Bus support?
Pali Rohár (3): pci: mpc85xx: Add missing sync() after writing to PCI config space pci: mpc85xx: Allow 8/16-bit access to PCI config space pci: mpc85xx: Do not try to access extended PCIe registers
drivers/pci/pci_mpc85xx.c | 39 +++++++++++++++++++++++++++++++++++---- 1 file changed, 35 insertions(+), 4 deletions(-)
-- 2.20.1
Anything more here?
Remaining issues are handled by the other patch series on the list.

Hello Pali,
On 29.04.23 13:10, Pali Rohár wrote:
On Thursday 13 April 2023 22:41:43 Pali Rohár wrote:
This patch series contains small fixes for mpc85xx old PCI Local Bus driver.
Heiko: Are you able to test these changes? Has your Socrates board available old PCI Local Bus support?
Pali Rohár (3): pci: mpc85xx: Add missing sync() after writing to PCI config space pci: mpc85xx: Allow 8/16-bit access to PCI config space pci: mpc85xx: Do not try to access extended PCIe registers
drivers/pci/pci_mpc85xx.c | 39 +++++++++++++++++++++++++++++++++++---- 1 file changed, 35 insertions(+), 4 deletions(-)
-- 2.20.1
Anything more here?
Remaining issues are handled by the other patch series on the list.
I see no more issues on my hardware, thanks!
bye, Heiko
participants (3)
-
Heiko Schocher
-
Pali Rohár
-
Tom Rini