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

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

[Pali Rohár]

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

[PATCH u-boot 1/3] pci: mpc85xx: Add missing sync() after writing to PCI config space

[Pali Rohár]

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;
 }
-- 
2.20.1

Re: [PATCH u-boot 1/3] pci: mpc85xx: Add missing sync() after writing to PCI config space

[Heiko Schocher]

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
-- 
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

Re: [PATCH u-boot 1/3] pci: mpc85xx: Add missing sync() after writing to PCI config space

[Tom Rini]

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

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!

-- 
Tom

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

[PATCH u-boot 2/3] pci: mpc85xx: Allow 8/16-bit access to PCI config space

[Pali Rohár]

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;
-- 
2.20.1

Re: [PATCH u-boot 2/3] pci: mpc85xx: Allow 8/16-bit access to PCI config space

[Heiko Schocher]

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
-- 
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

Re: [PATCH u-boot 2/3] pci: mpc85xx: Allow 8/16-bit access to PCI config space

[Tom Rini]

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

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!

-- 
Tom

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

[PATCH u-boot 3/3] pci: mpc85xx: Do not try to access extended PCIe registers

[Pali Rohár]

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();
 
-- 
2.20.1

Re: [PATCH u-boot 3/3] pci: mpc85xx: Do not try to access extended PCIe registers

[Heiko Schocher]

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
-- 
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

Re: [PATCH u-boot 3/3] pci: mpc85xx: Do not try to access extended PCIe registers

[Tom Rini]

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

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!

-- 
Tom

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

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

[Heiko Schocher]

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
-- 
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

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

[Heiko Schocher]

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
-- 
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

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

[Pali Rohár]

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

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

[Pali Rohár]

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.

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

[Heiko Schocher]

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
-- 
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