diff mbox series

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

Message ID 20211126104252.5443-13-pali@kernel.org
State Accepted
Commit 2a67bf65dd6c362487f416878348398d1842ae6b
Delegated to: Tom Rini
Headers show
Series Common U-Boot macros for PCI Configuration Mechanism #1 | expand

Commit Message

Pali Rohár Nov. 26, 2021, 10:42 a.m. UTC
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(-)

Comments

Simon Glass Dec. 28, 2021, 8:32 a.m. UTC | #1
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>
Tom Rini Jan. 13, 2022, 1:52 a.m. UTC | #2
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!
diff mbox series

Patch

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