mbox series

[v2,0/2] PCI: mvebu: add support for orion soc

Message ID 20220802173423.47230-1-maukka@ext.kapsi.fi
Headers show
Series PCI: mvebu: add support for orion soc | expand

Message

Mauri Sandberg Aug. 2, 2022, 5:34 p.m. UTC 
Hello all,

Here a cleaned up version of the previous series.

Changes
 - instead of removing PCIe related mvebu windows add them in pcie_setup()

I intentionally left the ORION5X_PCIE_WA_VIRT_BASE in place as all pieces
in the puzzle have not found their place yet. It will be replaced with
ioremap() or similar when the problem of passing the address to
configuration space from a device tree is solved.

Tested with DNS323 both DT and non-DT builds.

Thanks,
Mauri

Mauri Sandberg (2):
  dt-bindings: PCI: mvebu: Add orion5x compatible
  PCI: mvebu: add support for orion5x

 .../devicetree/bindings/pci/mvebu-pci.txt     |  1 +
 arch/arm/mach-orion5x/common.c                | 13 ----
 arch/arm/mach-orion5x/pci.c                   | 14 +++++
 drivers/pci/controller/Kconfig                |  2 +-
 drivers/pci/controller/pci-mvebu.c            | 59 +++++++++++++++++++
 5 files changed, 75 insertions(+), 14 deletions(-)


base-commit: 7d0d3fa7339ed5a06d6608b7cde9f079eba62bb1
--
2.25.1

Comments

Pali Rohár Aug. 2, 2022, 5:49 p.m. UTC | #1 
On Tuesday 02 August 2022 20:34:21 Mauri Sandberg wrote:
> Hello all,
> 
> Here a cleaned up version of the previous series.
> 
> Changes
>  - instead of removing PCIe related mvebu windows add them in pcie_setup()
> 
> I intentionally left the ORION5X_PCIE_WA_VIRT_BASE in place as all pieces
> in the puzzle have not found their place yet. It will be replaced with
> ioremap() or similar when the problem of passing the address to
> configuration space from a device tree is solved.

Hello all, could you please look at the issue how to write
representation of PCIe config space in device tree?
https://lore.kernel.org/linux-devicetree/20220710225108.bgedria6igtqpz5l@pali/

Without it we cannot finish this conversion of orion5x pcie code from
arch/arm to pci-mvebu.c

> Tested with DNS323 both DT and non-DT builds.
> 
> Thanks,
> Mauri
> 
> Mauri Sandberg (2):
>   dt-bindings: PCI: mvebu: Add orion5x compatible
>   PCI: mvebu: add support for orion5x
> 
>  .../devicetree/bindings/pci/mvebu-pci.txt     |  1 +
>  arch/arm/mach-orion5x/common.c                | 13 ----
>  arch/arm/mach-orion5x/pci.c                   | 14 +++++
>  drivers/pci/controller/Kconfig                |  2 +-
>  drivers/pci/controller/pci-mvebu.c            | 59 +++++++++++++++++++
>  5 files changed, 75 insertions(+), 14 deletions(-)
> 
> 
> base-commit: 7d0d3fa7339ed5a06d6608b7cde9f079eba62bb1
> --
> 2.25.1
Lorenzo Pieralisi Aug. 25, 2022, 3:15 p.m. UTC | #2 
On Tue, Aug 02, 2022 at 08:34:23PM +0300, Mauri Sandberg wrote:
> Add support for orion5x PCIe controller.
> 
> There is Orion-specific errata that config space via CF8/CFC registers
> is broken. Workaround documented in errata documented (linked from above
> documentation) does not work when DMA is used and instead other

Linked to which documentation ?

> undocumented workaround is needed which maps config space to memory
> (and therefore avoids usage of broken CF8/CFC memory mapped registers).
> 
> Signed-off-by: Mauri Sandberg <maukka@ext.kapsi.fi>
> Cc: Pali Rohár <pali@kernel.org>
> ---
> v1 -> v2:
>  - do pcie related mvebu windows and remaps in pcie_setup()
> ---
>  arch/arm/mach-orion5x/common.c     | 13 -------
>  arch/arm/mach-orion5x/pci.c        | 14 +++++++
>  drivers/pci/controller/Kconfig     |  2 +-
>  drivers/pci/controller/pci-mvebu.c | 59 ++++++++++++++++++++++++++++++
>  4 files changed, 74 insertions(+), 14 deletions(-)
> 
> diff --git a/arch/arm/mach-orion5x/common.c b/arch/arm/mach-orion5x/common.c
> index 7bcb41137bbf..9d8be5ce1266 100644
> --- a/arch/arm/mach-orion5x/common.c
> +++ b/arch/arm/mach-orion5x/common.c
> @@ -231,19 +231,6 @@ void __init orion5x_init_early(void)
>  
>  void orion5x_setup_wins(void)
>  {
> -	/*
> -	 * The PCIe windows will no longer be statically allocated
> -	 * here once Orion5x is migrated to the pci-mvebu driver.
> -	 */
> -	mvebu_mbus_add_window_remap_by_id(ORION_MBUS_PCIE_IO_TARGET,
> -					  ORION_MBUS_PCIE_IO_ATTR,
> -					  ORION5X_PCIE_IO_PHYS_BASE,
> -					  ORION5X_PCIE_IO_SIZE,
> -					  ORION5X_PCIE_IO_BUS_BASE);
> -	mvebu_mbus_add_window_by_id(ORION_MBUS_PCIE_MEM_TARGET,
> -				    ORION_MBUS_PCIE_MEM_ATTR,
> -				    ORION5X_PCIE_MEM_PHYS_BASE,
> -				    ORION5X_PCIE_MEM_SIZE);
>  	mvebu_mbus_add_window_remap_by_id(ORION_MBUS_PCI_IO_TARGET,
>  					  ORION_MBUS_PCI_IO_ATTR,
>  					  ORION5X_PCI_IO_PHYS_BASE,
> diff --git a/arch/arm/mach-orion5x/pci.c b/arch/arm/mach-orion5x/pci.c
> index 9574c73f3c03..7c4e2f355cc7 100644
> --- a/arch/arm/mach-orion5x/pci.c
> +++ b/arch/arm/mach-orion5x/pci.c
> @@ -150,6 +150,20 @@ static int __init pcie_setup(struct pci_sys_data *sys)
>  	 */
>  	orion_pcie_setup(PCIE_BASE);
>  
> +	/*
> +	 * The PCIe windows will no longer be statically allocated
> +	 * here once Orion5x is migrated to the pci-mvebu driver.
> +	 */

Is this comment still relevant ? And more importantly, may I ask
you why this code move in this hunk ? I think, whatever the reason
is, that deserves a comment.

> +	mvebu_mbus_add_window_remap_by_id(ORION_MBUS_PCIE_IO_TARGET,
> +					  ORION_MBUS_PCIE_IO_ATTR,
> +					  ORION5X_PCIE_IO_PHYS_BASE,
> +					  ORION5X_PCIE_IO_SIZE,
> +					  ORION5X_PCIE_IO_BUS_BASE);
> +	mvebu_mbus_add_window_by_id(ORION_MBUS_PCIE_MEM_TARGET,
> +				    ORION_MBUS_PCIE_MEM_ATTR,
> +				    ORION5X_PCIE_MEM_PHYS_BASE,
> +				    ORION5X_PCIE_MEM_SIZE);
> +
>  	/*
>  	 * Check whether to apply Orion-1/Orion-NAS PCIe config
>  	 * read transaction workaround.
> diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
> index b8d96d38064d..a249375837f0 100644
> --- a/drivers/pci/controller/Kconfig
> +++ b/drivers/pci/controller/Kconfig
> @@ -5,7 +5,7 @@ menu "PCI controller drivers"
>  
>  config PCI_MVEBU
>  	tristate "Marvell EBU PCIe controller"
> -	depends on ARCH_MVEBU || ARCH_DOVE || COMPILE_TEST
> +	depends on ARCH_MVEBU || ARCH_DOVE || ARCH_ORION5X || COMPILE_TEST
>  	depends on MVEBU_MBUS
>  	depends on ARM
>  	depends on OF
> diff --git a/drivers/pci/controller/pci-mvebu.c b/drivers/pci/controller/pci-mvebu.c
> index c1ffdb06c971..1d3052aa7e49 100644
> --- a/drivers/pci/controller/pci-mvebu.c
> +++ b/drivers/pci/controller/pci-mvebu.c
> @@ -1487,6 +1487,54 @@ static int mvebu_pcie_parse_request_resources(struct mvebu_pcie *pcie)
>  	return 0;
>  }
>  
> +static int orion_pcie_rd_conf_wa(void __iomem *wa_base, struct pci_bus *bus,
> +			  u32 devfn, int where, int size, u32 *val)
> +{
> +	*val = readl(wa_base + (PCIE_CONF_BUS(bus->number) |
> +				PCIE_CONF_DEV(PCI_SLOT(devfn)) |
> +				PCIE_CONF_FUNC(PCI_FUNC(devfn)) |
> +				PCIE_CONF_REG(where)));
> +
> +	if (size == 1)
> +		*val = (*val >> (8 * (where & 3))) & 0xff;
> +	else if (size == 2)
> +		*val = (*val >> (8 * (where & 3))) & 0xffff;
> +
> +	return PCIBIOS_SUCCESSFUL;
> +}
> +
> +/* Relevant only for Orion-1/Orion-NAS */
> +#define ORION5X_PCIE_WA_PHYS_BASE	0xf0000000
> +#define ORION5X_PCIE_WA_VIRT_BASE	IOMEM(0xfd000000)
> +#define ORION5X_PCIE_WA_SIZE		SZ_16M
> +#define ORION_MBUS_PCIE_WA_TARGET	0x04
> +#define ORION_MBUS_PCIE_WA_ATTR		0x79
> +
> +static int mvebu_pcie_child_rd_conf_wa(struct pci_bus *bus, u32 devfn, int where, int size, u32 *val)
> +{
> +	struct mvebu_pcie *pcie = bus->sysdata;
> +	struct mvebu_pcie_port *port;
> +
> +	port = mvebu_pcie_find_port(pcie, bus, devfn);
> +	if (!port)
> +		return PCIBIOS_DEVICE_NOT_FOUND;
> +
> +	if (!mvebu_pcie_link_up(port))
> +		return PCIBIOS_DEVICE_NOT_FOUND;
> +
> +	/*
> +	 * We only support access to the non-extended configuration
> +	 * space when using the WA access method (or we would have to
> +	 * sacrifice 256M of CPU virtual address space.)

Please expand the comment - future reviewers and developers may need
this information to understand this choice, me included.

Lorenzo

> +	 */
> +	if (where >= 0x100) {
> +		*val = 0xffffffff;
> +		return PCIBIOS_DEVICE_NOT_FOUND;
> +	}
> +
> +	return orion_pcie_rd_conf_wa(ORION5X_PCIE_WA_VIRT_BASE, bus, devfn, where, size, val);
> +}
> +
>  static int mvebu_pcie_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> @@ -1663,6 +1711,16 @@ static int mvebu_pcie_probe(struct platform_device *pdev)
>  	bridge->align_resource = mvebu_pcie_align_resource;
>  	bridge->map_irq = mvebu_pcie_map_irq;
>  
> +	if (of_machine_is_compatible("marvell,orion5x-88f5181")) {
> +		dev_info(dev, "Applying Orion-1/Orion-NAS PCIe config read transaction workaround\n");
> +
> +		mvebu_pcie_child_ops.read = mvebu_pcie_child_rd_conf_wa;
> +		mvebu_mbus_add_window_by_id(ORION_MBUS_PCIE_WA_TARGET,
> +					    ORION_MBUS_PCIE_WA_ATTR,
> +					    ORION5X_PCIE_WA_PHYS_BASE,
> +					    ORION5X_PCIE_WA_SIZE);
> +	}
> +
>  	return pci_host_probe(bridge);
>  }
>  
> @@ -1733,6 +1791,7 @@ static const struct of_device_id mvebu_pcie_of_match_table[] = {
>  	{ .compatible = "marvell,armada-370-pcie", },
>  	{ .compatible = "marvell,dove-pcie", },
>  	{ .compatible = "marvell,kirkwood-pcie", },
> +	{ .compatible = "marvell,orion5x-pcie", },
>  	{},
>  };
>  
> -- 
> 2.25.1
>
Pali Rohár Aug. 25, 2022, 4 p.m. UTC | #3 
On Thursday 25 August 2022 17:15:36 Lorenzo Pieralisi wrote:
> On Tue, Aug 02, 2022 at 08:34:23PM +0300, Mauri Sandberg wrote:
> > Add support for orion5x PCIe controller.
> > 
> > There is Orion-specific errata that config space via CF8/CFC registers
> > is broken. Workaround documented in errata documented (linked from above
> > documentation) does not work when DMA is used and instead other
> 
> Linked to which documentation ?

Hello! Orion Errata document is linked from kernel doc:
https://www.kernel.org/doc/html/latest/arm/marvell.html
Lorenzo Pieralisi Aug. 26, 2022, 8:42 a.m. UTC | #4 
On Thu, Aug 25, 2022 at 06:00:51PM +0200, Pali Rohár wrote:
> On Thursday 25 August 2022 17:15:36 Lorenzo Pieralisi wrote:
> > On Tue, Aug 02, 2022 at 08:34:23PM +0300, Mauri Sandberg wrote:
> > > Add support for orion5x PCIe controller.
> > > 
> > > There is Orion-specific errata that config space via CF8/CFC registers
> > > is broken. Workaround documented in errata documented (linked from above
> > > documentation) does not work when DMA is used and instead other
> > 
> > Linked to which documentation ?
> 
> Hello! Orion Errata document is linked from kernel doc:
> https://www.kernel.org/doc/html/latest/arm/marvell.html

Add a Link: tag to the patch, thanks.

Lorenzo