[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 at nic.cz>
> Cc: Stefan Roese <sr at denx.de>
> Cc: Phil Sutter <phil at nwl.cc>
> Cc: Mario Six <mario.six at gdsys.cc>
> Cc: Baruch Siach <baruch at tkos.co.il>

Reviewed-by: Stefan Roese <sr at 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