[PATCH u-boot-marvell 0/5] pci_mvebu changes

[PATCH u-boot-marvell 0/5] pci_mvebu changes

[Marek Behún]

Hi Stefan,

this patchset continues to address the issue you tried to solve with
commit "pci: pci_mvebu: Disable config access to PCI host bridge ports".

Some code refactoring is done here so that the code looks more sane and
the underlying information is documenter.

The last patch changes the local device (which presents itself as a
"Memory controller" to device number 1, so that the pci command in
U-Boot lists the real PCIe card as device 0, as Linux does.

Marek

Marek Beh?n (5):
  pci: pci_mvebu: use dev_seq instead of static variable
  pci: pci_mvebu: cosmetic fix
  pci: pci_mvebu: debug rd/wr config as other drivers do
  pci: pci_mvebu: refactor validation of addresses for config access
  pci: pci_mvebu: set local dev to number 1

 drivers/pci/pci_mvebu.c | 76 ++++++++++++++++++++++-------------------
 1 file changed, 40 insertions(+), 36 deletions(-)

-- 
2.26.2

[PATCH u-boot-marvell 1/5] pci: pci_mvebu: use dev_seq instead of static variable

[Marek Behún]

PCI uclass maps PCI bus numbers to the seq member of struct udevice.
Use dev_seq(dev) as the bus number in mvebu_pcie_probe instead of an
incrementing a static variable.

Signed-off-by: Marek Beh?n <marek.behun@nic.cz>
Cc: Stefan Roese <sr@denx.de>
Cc: Phil Sutter <phil@nwl.cc>
Cc: Mario Six <mario.six@gdsys.cc>
Cc: Baruch Siach <baruch@tkos.co.il>
---
 drivers/pci/pci_mvebu.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/pci/pci_mvebu.c b/drivers/pci/pci_mvebu.c
index 3ab03e3675..5c55a76d0e 100644
--- a/drivers/pci/pci_mvebu.c
+++ b/drivers/pci/pci_mvebu.c
@@ -272,7 +272,7 @@ static int mvebu_pcie_probe(struct udevice *dev)
 	struct mvebu_pcie *pcie = dev_get_plat(dev);
 	struct udevice *ctlr = pci_get_controller(dev);
 	struct pci_controller *hose = dev_get_uclass_priv(ctlr);
-	static int bus;
+	int bus = dev_seq(dev);
 	u32 reg;
 
 	debug("%s: PCIe %d.%d - up, base %08x\n", __func__,
@@ -335,8 +335,6 @@ static int mvebu_pcie_probe(struct udevice *dev)
 	writel(SOC_REGS_PHY_BASE, pcie->base + PCIE_BAR_LO_OFF(0));
 	writel(0, pcie->base + PCIE_BAR_HI_OFF(0));
 
-	bus++;
-
 	return 0;
 }
 
-- 
2.26.2

[PATCH u-boot-marvell 2/5] pci: pci_mvebu: cosmetic fix

[Marek Behún]

Write bdf address in a same way in mvebu_pcie_read/write_config.

Signed-off-by: Marek Beh?n <marek.behun@nic.cz>
Cc: Stefan Roese <sr@denx.de>
Cc: Phil Sutter <phil@nwl.cc>
Cc: Mario Six <mario.six@gdsys.cc>
Cc: Baruch Siach <baruch@tkos.co.il>
---
 drivers/pci/pci_mvebu.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/pci_mvebu.c b/drivers/pci/pci_mvebu.c
index 5c55a76d0e..abc6e9e6e0 100644
--- a/drivers/pci/pci_mvebu.c
+++ b/drivers/pci/pci_mvebu.c
@@ -150,7 +150,6 @@ static int mvebu_pcie_read_config(const struct udevice *bus, pci_dev_t bdf,
 	struct mvebu_pcie *pcie = dev_get_plat(bus);
 	int local_bus = PCI_BUS(pcie->dev);
 	int local_dev = PCI_DEV(pcie->dev);
-	u32 reg;
 	u32 data;
 
 	debug("PCIE CFG read:  loc_bus=%d loc_dev=%d (b,d,f)=(%2d,%2d,%2d) ",
@@ -171,8 +170,9 @@ static int mvebu_pcie_read_config(const struct udevice *bus, pci_dev_t bdf,
 	}
 
 	/* write address */
-	reg = PCIE_CONF_ADDR(bdf, offset);
-	writel(reg, pcie->base + PCIE_CONF_ADDR_OFF);
+	writel(PCIE_CONF_ADDR(bdf, offset), pcie->base + PCIE_CONF_ADDR_OFF);
+
+	/* read data */
 	data = readl(pcie->base + PCIE_CONF_DATA_OFF);
 	debug("(addr,val)=(0x%04x, 0x%08x)\n", offset, data);
 	*valuep = pci_conv_32_to_size(data, offset, size);
@@ -205,7 +205,10 @@ static int mvebu_pcie_write_config(struct udevice *bus, pci_dev_t bdf,
 		return 0;
 	}
 
+	/* write address */
 	writel(PCIE_CONF_ADDR(bdf, offset), pcie->base + PCIE_CONF_ADDR_OFF);
+
+	/* write data */
 	data = pci_conv_size_to_32(0, value, offset, size);
 	writel(data, pcie->base + PCIE_CONF_DATA_OFF);
 
-- 
2.26.2

[PATCH u-boot-marvell 3/5] pci: pci_mvebu: debug rd/wr config as other drivers do

[Marek Behún]

Other drivers (aardvark, intel_fpga) print "(addr,size,val)" when
debugging is enabled. Print size for pci_mvebu as well.

Signed-off-by: Marek Beh?n <marek.behun@nic.cz>
Cc: Stefan Roese <sr@denx.de>
Cc: Phil Sutter <phil@nwl.cc>
Cc: Mario Six <mario.six@gdsys.cc>
Cc: Baruch Siach <baruch@tkos.co.il>
---
 drivers/pci/pci_mvebu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/pci_mvebu.c b/drivers/pci/pci_mvebu.c
index abc6e9e6e0..4ad73512a6 100644
--- a/drivers/pci/pci_mvebu.c
+++ b/drivers/pci/pci_mvebu.c
@@ -174,7 +174,7 @@ static int mvebu_pcie_read_config(const struct udevice *bus, pci_dev_t bdf,
 
 	/* read data */
 	data = readl(pcie->base + PCIE_CONF_DATA_OFF);
-	debug("(addr,val)=(0x%04x, 0x%08x)\n", offset, data);
+	debug("(addr,size,val)=(0x%04x, %d, 0x%08x)\n", offset, size, data);
 	*valuep = pci_conv_32_to_size(data, offset, size);
 
 	return 0;
@@ -191,7 +191,7 @@ static int mvebu_pcie_write_config(struct udevice *bus, pci_dev_t bdf,
 
 	debug("PCIE CFG write: loc_bus=%d loc_dev=%d (b,d,f)=(%2d,%2d,%2d) ",
 	      local_bus, local_dev, PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf));
-	debug("(addr,val)=(0x%04x, 0x%08lx)\n", offset, value);
+	debug("(addr,size,val)=(0x%04x, %d, 0x%08lx)\n", offset, size, value);
 
 	/* Don't access the local host controller via this API */
 	if (PCI_BUS(bdf) == local_bus && PCI_DEV(bdf) == local_dev) {
-- 
2.26.2

[PATCH u-boot-marvell 4/5] pci: pci_mvebu: refactor validation of addresses for config access

[Marek Behún]

Refactor validation of bdf parameter in mvebu_pcie_read/write_config
functions.

We can simplify the code by putting the validation into separate
function.

Also there are always only two devices visible on local bus:
* on slot configured by function mvebu_pcie_set_local_dev_nr()
  (by default this register is set to 0) there is a
  "Marvell Memory controller", which isn't useful in root complex
  mode,
* on all other slots the real PCIe card connected to the PCIe slot.

We can simplify the code even more by simply allowing access only to
the real PCIe card.

Signed-off-by: Marek Beh?n <marek.behun@nic.cz>
Cc: Stefan Roese <sr@denx.de>
Cc: Phil Sutter <phil@nwl.cc>
Cc: Mario Six <mario.six@gdsys.cc>
Cc: Baruch Siach <baruch@tkos.co.il>
---
 drivers/pci/pci_mvebu.c | 59 ++++++++++++++++++++++-------------------
 1 file changed, 31 insertions(+), 28 deletions(-)

diff --git a/drivers/pci/pci_mvebu.c b/drivers/pci/pci_mvebu.c
index 4ad73512a6..2d923b7d8d 100644
--- a/drivers/pci/pci_mvebu.c
+++ b/drivers/pci/pci_mvebu.c
@@ -78,7 +78,8 @@ struct mvebu_pcie {
 	u32 lane;
 	int devfn;
 	u32 lane_mask;
-	pci_dev_t dev;
+	int first_busno;
+	int local_dev;
 	char name[16];
 	unsigned int mem_target;
 	unsigned int mem_attr;
@@ -143,27 +144,36 @@ static inline struct mvebu_pcie *hose_to_pcie(struct pci_controller *hose)
 	return container_of(hose, struct mvebu_pcie, hose);
 }
 
+static int mvebu_pcie_valid_addr(struct mvebu_pcie *pcie, pci_dev_t bdf)
+{
+	/*
+	 * There are two devices visible on local bus:
+	 *   * on slot configured by function mvebu_pcie_set_local_dev_nr()
+	 *     (by default this register is set to 0) there is a
+	 *     "Marvell Memory controller", which isn't useful in root complex
+	 *     mode,
+	 *   * on all other slots the real PCIe card connected to the PCIe slot.
+	 *
+	 * We therefore allow access only to the real PCIe card.
+	 */
+	if (PCI_BUS(bdf) == pcie->first_busno &&
+	    PCI_DEV(bdf) != !pcie->local_dev)
+		return 0;
+
+	return 1;
+}
+
 static int mvebu_pcie_read_config(const struct udevice *bus, pci_dev_t bdf,
 				  uint offset, ulong *valuep,
 				  enum pci_size_t size)
 {
 	struct mvebu_pcie *pcie = dev_get_plat(bus);
-	int local_bus = PCI_BUS(pcie->dev);
-	int local_dev = PCI_DEV(pcie->dev);
 	u32 data;
 
-	debug("PCIE CFG read:  loc_bus=%d loc_dev=%d (b,d,f)=(%2d,%2d,%2d) ",
-	      local_bus, local_dev, PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf));
+	debug("PCIE CFG read: (b,d,f)=(%2d,%2d,%2d) ",
+	      PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf));
 
-	/* Don't access the local host controller via this API */
-	if (PCI_BUS(bdf) == local_bus && PCI_DEV(bdf) == local_dev) {
-		debug("- skipping host controller\n");
-		*valuep = pci_get_ff(size);
-		return 0;
-	}
-
-	/* If local dev is 0, the first other dev can only be 1 */
-	if (PCI_BUS(bdf) == local_bus && local_dev == 0 && PCI_DEV(bdf) != 1) {
+	if (!mvebu_pcie_valid_addr(pcie, bdf)) {
 		debug("- out of range\n");
 		*valuep = pci_get_ff(size);
 		return 0;
@@ -185,22 +195,13 @@ static int mvebu_pcie_write_config(struct udevice *bus, pci_dev_t bdf,
 				   enum pci_size_t size)
 {
 	struct mvebu_pcie *pcie = dev_get_plat(bus);
-	int local_bus = PCI_BUS(pcie->dev);
-	int local_dev = PCI_DEV(pcie->dev);
 	u32 data;
 
-	debug("PCIE CFG write: loc_bus=%d loc_dev=%d (b,d,f)=(%2d,%2d,%2d) ",
-	      local_bus, local_dev, PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf));
+	debug("PCIE CFG write: (b,d,f)=(%2d,%2d,%2d) ",
+	      PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf));
 	debug("(addr,size,val)=(0x%04x, %d, 0x%08lx)\n", offset, size, value);
 
-	/* Don't access the local host controller via this API */
-	if (PCI_BUS(bdf) == local_bus && PCI_DEV(bdf) == local_dev) {
-		debug("- skipping host controller\n");
-		return 0;
-	}
-
-	/* If local dev is 0, the first other dev can only be 1 */
-	if (PCI_BUS(bdf) == local_bus && local_dev == 0 && PCI_DEV(bdf) != 1) {
+	if (!mvebu_pcie_valid_addr(pcie, bdf)) {
 		debug("- out of range\n");
 		return 0;
 	}
@@ -286,9 +287,11 @@ static int mvebu_pcie_probe(struct udevice *dev)
 	      readl(pcie->base), mvebu_pcie_get_local_bus_nr(pcie),
 	      mvebu_pcie_get_local_dev_nr(pcie));
 
+	pcie->first_busno = bus;
+	pcie->local_dev = 0;
+
 	mvebu_pcie_set_local_bus_nr(pcie, bus);
-	mvebu_pcie_set_local_dev_nr(pcie, 0);
-	pcie->dev = PCI_BDF(bus, 0, 0);
+	mvebu_pcie_set_local_dev_nr(pcie, pcie->local_dev);
 
 	pcie->mem.start = (u32)mvebu_pcie_membase;
 	pcie->mem.end = pcie->mem.start + PCIE_MEM_SIZE - 1;
-- 
2.26.2

[PATCH u-boot-marvell 5/5] pci: pci_mvebu: set local dev to number 1

[Marek Behún]

Linux displays the real PCIe card connected to a mvebu PCIe slot as
device 0, not 1. This is done by setting local dev number to 1, so that
the local "Marvell Memory controller" device is on address 1.

Let's do it also in U-Boot.

With this commit the pci command in U-Boot prints something like:
  => pci
  Scanning PCI devices on bus 0
  BusDevFun  VendorId   DeviceId   Device Class       Sub-Class
  _____________________________________________________________
  00.00.00   0x168c     0x003c     Network controller      0x80

Signed-off-by: Marek Beh?n <marek.behun@nic.cz>
Cc: Stefan Roese <sr@denx.de>
Cc: Phil Sutter <phil@nwl.cc>
Cc: Mario Six <mario.six@gdsys.cc>
Cc: Baruch Siach <baruch@tkos.co.il>
---
 drivers/pci/pci_mvebu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/pci_mvebu.c b/drivers/pci/pci_mvebu.c
index 2d923b7d8d..11b590617d 100644
--- a/drivers/pci/pci_mvebu.c
+++ b/drivers/pci/pci_mvebu.c
@@ -288,7 +288,7 @@ static int mvebu_pcie_probe(struct udevice *dev)
 	      mvebu_pcie_get_local_dev_nr(pcie));
 
 	pcie->first_busno = bus;
-	pcie->local_dev = 0;
+	pcie->local_dev = 1;
 
 	mvebu_pcie_set_local_bus_nr(pcie, bus);
 	mvebu_pcie_set_local_dev_nr(pcie, pcie->local_dev);
-- 
2.26.2

[PATCH u-boot-marvell 0/5] pci_mvebu changes

[Marek Behún]

And also pinging this series, since we had our answer from Thomas
Petazzoni :)

[PATCH u-boot-marvell 1/5] pci: pci_mvebu: use dev_seq instead of static variable

[Stefan Roese]

On 08.02.21 23:01, Marek Beh?n wrote:
> PCI uclass maps PCI bus numbers to the seq member of struct udevice.
> Use dev_seq(dev) as the bus number in mvebu_pcie_probe instead of an
> incrementing a static variable.
> 
> Signed-off-by: Marek Beh?n <marek.behun@nic.cz>
> Cc: Stefan Roese <sr@denx.de>
> Cc: Phil Sutter <phil@nwl.cc>
> Cc: Mario Six <mario.six@gdsys.cc>
> Cc: Baruch Siach <baruch@tkos.co.il>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
>   drivers/pci/pci_mvebu.c | 4 +---
>   1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/pci_mvebu.c b/drivers/pci/pci_mvebu.c
> index 3ab03e3675..5c55a76d0e 100644
> --- a/drivers/pci/pci_mvebu.c
> +++ b/drivers/pci/pci_mvebu.c
> @@ -272,7 +272,7 @@ static int mvebu_pcie_probe(struct udevice *dev)
>   	struct mvebu_pcie *pcie = dev_get_plat(dev);
>   	struct udevice *ctlr = pci_get_controller(dev);
>   	struct pci_controller *hose = dev_get_uclass_priv(ctlr);
> -	static int bus;
> +	int bus = dev_seq(dev);
>   	u32 reg;
>   
>   	debug("%s: PCIe %d.%d - up, base %08x\n", __func__,
> @@ -335,8 +335,6 @@ static int mvebu_pcie_probe(struct udevice *dev)
>   	writel(SOC_REGS_PHY_BASE, pcie->base + PCIE_BAR_LO_OFF(0));
>   	writel(0, pcie->base + PCIE_BAR_HI_OFF(0));
>   
> -	bus++;
> -
>   	return 0;
>   }
>   
> 


Viele Gr??e,
Stefan

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de

[PATCH u-boot-marvell 2/5] pci: pci_mvebu: cosmetic fix

[Stefan Roese]

On 08.02.21 23:01, Marek Beh?n wrote:
> Write bdf address in a same way in mvebu_pcie_read/write_config.
> 
> Signed-off-by: Marek Beh?n <marek.behun@nic.cz>
> Cc: Stefan Roese <sr@denx.de>
> Cc: Phil Sutter <phil@nwl.cc>
> Cc: Mario Six <mario.six@gdsys.cc>
> Cc: Baruch Siach <baruch@tkos.co.il>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
>   drivers/pci/pci_mvebu.c | 9 ++++++---
>   1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/pci_mvebu.c b/drivers/pci/pci_mvebu.c
> index 5c55a76d0e..abc6e9e6e0 100644
> --- a/drivers/pci/pci_mvebu.c
> +++ b/drivers/pci/pci_mvebu.c
> @@ -150,7 +150,6 @@ static int mvebu_pcie_read_config(const struct udevice *bus, pci_dev_t bdf,
>   	struct mvebu_pcie *pcie = dev_get_plat(bus);
>   	int local_bus = PCI_BUS(pcie->dev);
>   	int local_dev = PCI_DEV(pcie->dev);
> -	u32 reg;
>   	u32 data;
>   
>   	debug("PCIE CFG read:  loc_bus=%d loc_dev=%d (b,d,f)=(%2d,%2d,%2d) ",
> @@ -171,8 +170,9 @@ static int mvebu_pcie_read_config(const struct udevice *bus, pci_dev_t bdf,
>   	}
>   
>   	/* write address */
> -	reg = PCIE_CONF_ADDR(bdf, offset);
> -	writel(reg, pcie->base + PCIE_CONF_ADDR_OFF);
> +	writel(PCIE_CONF_ADDR(bdf, offset), pcie->base + PCIE_CONF_ADDR_OFF);
> +
> +	/* read data */
>   	data = readl(pcie->base + PCIE_CONF_DATA_OFF);
>   	debug("(addr,val)=(0x%04x, 0x%08x)\n", offset, data);
>   	*valuep = pci_conv_32_to_size(data, offset, size);
> @@ -205,7 +205,10 @@ static int mvebu_pcie_write_config(struct udevice *bus, pci_dev_t bdf,
>   		return 0;
>   	}
>   
> +	/* write address */
>   	writel(PCIE_CONF_ADDR(bdf, offset), pcie->base + PCIE_CONF_ADDR_OFF);
> +
> +	/* write data */
>   	data = pci_conv_size_to_32(0, value, offset, size);
>   	writel(data, pcie->base + PCIE_CONF_DATA_OFF);
>   
> 


Viele Gr??e,
Stefan

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de

[PATCH u-boot-marvell 3/5] pci: pci_mvebu: debug rd/wr config as other drivers do

[Stefan Roese]

On 08.02.21 23:01, Marek Beh?n wrote:
> Other drivers (aardvark, intel_fpga) print "(addr,size,val)" when
> debugging is enabled. Print size for pci_mvebu as well.
> 
> Signed-off-by: Marek Beh?n <marek.behun@nic.cz>
> Cc: Stefan Roese <sr@denx.de>
> Cc: Phil Sutter <phil@nwl.cc>
> Cc: Mario Six <mario.six@gdsys.cc>
> Cc: Baruch Siach <baruch@tkos.co.il>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
>   drivers/pci/pci_mvebu.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/pci_mvebu.c b/drivers/pci/pci_mvebu.c
> index abc6e9e6e0..4ad73512a6 100644
> --- a/drivers/pci/pci_mvebu.c
> +++ b/drivers/pci/pci_mvebu.c
> @@ -174,7 +174,7 @@ static int mvebu_pcie_read_config(const struct udevice *bus, pci_dev_t bdf,
>   
>   	/* read data */
>   	data = readl(pcie->base + PCIE_CONF_DATA_OFF);
> -	debug("(addr,val)=(0x%04x, 0x%08x)\n", offset, data);
> +	debug("(addr,size,val)=(0x%04x, %d, 0x%08x)\n", offset, size, data);
>   	*valuep = pci_conv_32_to_size(data, offset, size);
>   
>   	return 0;
> @@ -191,7 +191,7 @@ static int mvebu_pcie_write_config(struct udevice *bus, pci_dev_t bdf,
>   
>   	debug("PCIE CFG write: loc_bus=%d loc_dev=%d (b,d,f)=(%2d,%2d,%2d) ",
>   	      local_bus, local_dev, PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf));
> -	debug("(addr,val)=(0x%04x, 0x%08lx)\n", offset, value);
> +	debug("(addr,size,val)=(0x%04x, %d, 0x%08lx)\n", offset, size, value);
>   
>   	/* Don't access the local host controller via this API */
>   	if (PCI_BUS(bdf) == local_bus && PCI_DEV(bdf) == local_dev) {
> 


Viele Gr??e,
Stefan

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de

[PATCH u-boot-marvell 4/5] pci: pci_mvebu: refactor validation of addresses for config access

[Stefan Roese]

On 08.02.21 23:01, Marek Beh?n wrote:
> Refactor validation of bdf parameter in mvebu_pcie_read/write_config
> functions.
> 
> We can simplify the code by putting the validation into separate
> function.
> 
> Also there are always only two devices visible on local bus:
> * on slot configured by function mvebu_pcie_set_local_dev_nr()
>    (by default this register is set to 0) there is a
>    "Marvell Memory controller", which isn't useful in root complex
>    mode,
> * on all other slots the real PCIe card connected to the PCIe slot.
> 
> We can simplify the code even more by simply allowing access only to
> the real PCIe card.
> 
> Signed-off-by: Marek Beh?n <marek.behun@nic.cz>
> Cc: Stefan Roese <sr@denx.de>
> Cc: Phil Sutter <phil@nwl.cc>
> Cc: Mario Six <mario.six@gdsys.cc>
> Cc: Baruch Siach <baruch@tkos.co.il>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
>   drivers/pci/pci_mvebu.c | 59 ++++++++++++++++++++++-------------------
>   1 file changed, 31 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/pci/pci_mvebu.c b/drivers/pci/pci_mvebu.c
> index 4ad73512a6..2d923b7d8d 100644
> --- a/drivers/pci/pci_mvebu.c
> +++ b/drivers/pci/pci_mvebu.c
> @@ -78,7 +78,8 @@ struct mvebu_pcie {
>   	u32 lane;
>   	int devfn;
>   	u32 lane_mask;
> -	pci_dev_t dev;
> +	int first_busno;
> +	int local_dev;
>   	char name[16];
>   	unsigned int mem_target;
>   	unsigned int mem_attr;
> @@ -143,27 +144,36 @@ static inline struct mvebu_pcie *hose_to_pcie(struct pci_controller *hose)
>   	return container_of(hose, struct mvebu_pcie, hose);
>   }
>   
> +static int mvebu_pcie_valid_addr(struct mvebu_pcie *pcie, pci_dev_t bdf)
> +{
> +	/*
> +	 * There are two devices visible on local bus:
> +	 *   * on slot configured by function mvebu_pcie_set_local_dev_nr()
> +	 *     (by default this register is set to 0) there is a
> +	 *     "Marvell Memory controller", which isn't useful in root complex
> +	 *     mode,
> +	 *   * on all other slots the real PCIe card connected to the PCIe slot.
> +	 *
> +	 * We therefore allow access only to the real PCIe card.
> +	 */
> +	if (PCI_BUS(bdf) == pcie->first_busno &&
> +	    PCI_DEV(bdf) != !pcie->local_dev)
> +		return 0;
> +
> +	return 1;
> +}
> +
>   static int mvebu_pcie_read_config(const struct udevice *bus, pci_dev_t bdf,
>   				  uint offset, ulong *valuep,
>   				  enum pci_size_t size)
>   {
>   	struct mvebu_pcie *pcie = dev_get_plat(bus);
> -	int local_bus = PCI_BUS(pcie->dev);
> -	int local_dev = PCI_DEV(pcie->dev);
>   	u32 data;
>   
> -	debug("PCIE CFG read:  loc_bus=%d loc_dev=%d (b,d,f)=(%2d,%2d,%2d) ",
> -	      local_bus, local_dev, PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf));
> +	debug("PCIE CFG read: (b,d,f)=(%2d,%2d,%2d) ",
> +	      PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf));
>   
> -	/* Don't access the local host controller via this API */
> -	if (PCI_BUS(bdf) == local_bus && PCI_DEV(bdf) == local_dev) {
> -		debug("- skipping host controller\n");
> -		*valuep = pci_get_ff(size);
> -		return 0;
> -	}
> -
> -	/* If local dev is 0, the first other dev can only be 1 */
> -	if (PCI_BUS(bdf) == local_bus && local_dev == 0 && PCI_DEV(bdf) != 1) {
> +	if (!mvebu_pcie_valid_addr(pcie, bdf)) {
>   		debug("- out of range\n");
>   		*valuep = pci_get_ff(size);
>   		return 0;
> @@ -185,22 +195,13 @@ static int mvebu_pcie_write_config(struct udevice *bus, pci_dev_t bdf,
>   				   enum pci_size_t size)
>   {
>   	struct mvebu_pcie *pcie = dev_get_plat(bus);
> -	int local_bus = PCI_BUS(pcie->dev);
> -	int local_dev = PCI_DEV(pcie->dev);
>   	u32 data;
>   
> -	debug("PCIE CFG write: loc_bus=%d loc_dev=%d (b,d,f)=(%2d,%2d,%2d) ",
> -	      local_bus, local_dev, PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf));
> +	debug("PCIE CFG write: (b,d,f)=(%2d,%2d,%2d) ",
> +	      PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf));
>   	debug("(addr,size,val)=(0x%04x, %d, 0x%08lx)\n", offset, size, value);
>   
> -	/* Don't access the local host controller via this API */
> -	if (PCI_BUS(bdf) == local_bus && PCI_DEV(bdf) == local_dev) {
> -		debug("- skipping host controller\n");
> -		return 0;
> -	}
> -
> -	/* If local dev is 0, the first other dev can only be 1 */
> -	if (PCI_BUS(bdf) == local_bus && local_dev == 0 && PCI_DEV(bdf) != 1) {
> +	if (!mvebu_pcie_valid_addr(pcie, bdf)) {
>   		debug("- out of range\n");
>   		return 0;
>   	}
> @@ -286,9 +287,11 @@ static int mvebu_pcie_probe(struct udevice *dev)
>   	      readl(pcie->base), mvebu_pcie_get_local_bus_nr(pcie),
>   	      mvebu_pcie_get_local_dev_nr(pcie));
>   
> +	pcie->first_busno = bus;
> +	pcie->local_dev = 0;
> +
>   	mvebu_pcie_set_local_bus_nr(pcie, bus);
> -	mvebu_pcie_set_local_dev_nr(pcie, 0);
> -	pcie->dev = PCI_BDF(bus, 0, 0);
> +	mvebu_pcie_set_local_dev_nr(pcie, pcie->local_dev);
>   
>   	pcie->mem.start = (u32)mvebu_pcie_membase;
>   	pcie->mem.end = pcie->mem.start + PCIE_MEM_SIZE - 1;
> 


Viele Gr??e,
Stefan

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de

[PATCH u-boot-marvell 5/5] pci: pci_mvebu: set local dev to number 1

[Stefan Roese]

On 08.02.21 23:01, Marek Beh?n wrote:
> Linux displays the real PCIe card connected to a mvebu PCIe slot as
> device 0, not 1. This is done by setting local dev number to 1, so that
> the local "Marvell Memory controller" device is on address 1.
> 
> Let's do it also in U-Boot.
> 
> With this commit the pci command in U-Boot prints something like:
>    => pci
>    Scanning PCI devices on bus 0
>    BusDevFun  VendorId   DeviceId   Device Class       Sub-Class
>    _____________________________________________________________
>    00.00.00   0x168c     0x003c     Network controller      0x80
> 
> Signed-off-by: Marek Beh?n <marek.behun@nic.cz>
> Cc: Stefan Roese <sr@denx.de>
> Cc: Phil Sutter <phil@nwl.cc>
> Cc: Mario Six <mario.six@gdsys.cc>
> Cc: Baruch Siach <baruch@tkos.co.il>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
>   drivers/pci/pci_mvebu.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/pci_mvebu.c b/drivers/pci/pci_mvebu.c
> index 2d923b7d8d..11b590617d 100644
> --- a/drivers/pci/pci_mvebu.c
> +++ b/drivers/pci/pci_mvebu.c
> @@ -288,7 +288,7 @@ static int mvebu_pcie_probe(struct udevice *dev)
>   	      mvebu_pcie_get_local_dev_nr(pcie));
>   
>   	pcie->first_busno = bus;
> -	pcie->local_dev = 0;
> +	pcie->local_dev = 1;
>   
>   	mvebu_pcie_set_local_bus_nr(pcie, bus);
>   	mvebu_pcie_set_local_dev_nr(pcie, pcie->local_dev);
> 


Viele Gr??e,
Stefan

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de

[PATCH u-boot-marvell 0/5] pci_mvebu changes

[Stefan Roese]

On 08.02.21 23:01, Marek Beh?n wrote:
> Hi Stefan,
> 
> this patchset continues to address the issue you tried to solve with
> commit "pci: pci_mvebu: Disable config access to PCI host bridge ports".
> 
> Some code refactoring is done here so that the code looks more sane and
> the underlying information is documenter.
> 
> The last patch changes the local device (which presents itself as a
> "Memory controller" to device number 1, so that the pci command in
> U-Boot lists the real PCIe card as device 0, as Linux does.
> 
> Marek
> 
> Marek Beh?n (5):
>    pci: pci_mvebu: use dev_seq instead of static variable
>    pci: pci_mvebu: cosmetic fix
>    pci: pci_mvebu: debug rd/wr config as other drivers do
>    pci: pci_mvebu: refactor validation of addresses for config access
>    pci: pci_mvebu: set local dev to number 1
> 
>   drivers/pci/pci_mvebu.c | 76 ++++++++++++++++++++++-------------------
>   1 file changed, 40 insertions(+), 36 deletions(-)
> 

Applied to u-boot-marvell/master

Thanks,
Stefan