Re: [PATCH v2 2/2] pci: host: new driver for Axis ARTPEC-6 PCIe controller

2016-06-21 Thread Bjorn Helgaas
On Mon, Jun 20, 2016 at 03:50:07PM -0400, Paul Gortmaker wrote:
> On Mon, May 9, 2016 at 7:49 AM, Niklas Cassel  wrote:
> > From: Niklas Cassel 
> >
> > The Axis ARTPEC-6 SoC integrates a PCIe controller from Synopsys.
> > This commit adds a new driver that provides the small glue
> > needed to use the existing Designware driver to make it work on
> > the Axis ARTPEC-6 SoC.
> >
> > Signed-off-by: Niklas Cassel 
> 
> [...]
> 
> > --- a/drivers/pci/host/Kconfig
> > +++ b/drivers/pci/host/Kconfig
> > @@ -244,4 +244,10 @@ config PCIE_ARMADA_8K
> >   Designware hardware and therefore the driver re-uses the
> >   Designware core functions to implement the driver.
> >
> > +config PCIE_ARTPEC6
> > +   bool "Axis ARTPEC-6 PCIe controller"
> 
> Bjorn,
> 
> Can we please start requiring new PCI/PCI-e drivers to either:
> 
>   (a)  be merged and tested as tristate, or
> 
>   (b)  remain bool, but not gratuitously use module macros

That requires somebody to enforce it, which I suppose would end up
being me, and that's not really practical unless you help out by
watching for them.  I know that's not really practical for you either.

I hoped that we could convert them to be modular, but that hasn't
started happening, so I think we need to fall back to your original
proposal of making them bool and removing the modular junk.
Eventually people who feel the pain of building in all the drivers
might step up and make them modular.

Do you want to update your series that removed the gratuitous module
code?  I'll be on vacation June 25-July 17, but maybe we can still
squeeze it in this cycle.

Bjorn


Re: [PATCH v2 2/2] pci: host: new driver for Axis ARTPEC-6 PCIe controller

2016-06-21 Thread Bjorn Helgaas
On Mon, Jun 20, 2016 at 03:50:07PM -0400, Paul Gortmaker wrote:
> On Mon, May 9, 2016 at 7:49 AM, Niklas Cassel  wrote:
> > From: Niklas Cassel 
> >
> > The Axis ARTPEC-6 SoC integrates a PCIe controller from Synopsys.
> > This commit adds a new driver that provides the small glue
> > needed to use the existing Designware driver to make it work on
> > the Axis ARTPEC-6 SoC.
> >
> > Signed-off-by: Niklas Cassel 
> 
> [...]
> 
> > --- a/drivers/pci/host/Kconfig
> > +++ b/drivers/pci/host/Kconfig
> > @@ -244,4 +244,10 @@ config PCIE_ARMADA_8K
> >   Designware hardware and therefore the driver re-uses the
> >   Designware core functions to implement the driver.
> >
> > +config PCIE_ARTPEC6
> > +   bool "Axis ARTPEC-6 PCIe controller"
> 
> Bjorn,
> 
> Can we please start requiring new PCI/PCI-e drivers to either:
> 
>   (a)  be merged and tested as tristate, or
> 
>   (b)  remain bool, but not gratuitously use module macros

That requires somebody to enforce it, which I suppose would end up
being me, and that's not really practical unless you help out by
watching for them.  I know that's not really practical for you either.

I hoped that we could convert them to be modular, but that hasn't
started happening, so I think we need to fall back to your original
proposal of making them bool and removing the modular junk.
Eventually people who feel the pain of building in all the drivers
might step up and make them modular.

Do you want to update your series that removed the gratuitous module
code?  I'll be on vacation June 25-July 17, but maybe we can still
squeeze it in this cycle.

Bjorn


Re: [PATCH v2 2/2] pci: host: new driver for Axis ARTPEC-6 PCIe controller

2016-06-20 Thread Paul Gortmaker
On Mon, May 9, 2016 at 7:49 AM, Niklas Cassel  wrote:
> From: Niklas Cassel 
>
> The Axis ARTPEC-6 SoC integrates a PCIe controller from Synopsys.
> This commit adds a new driver that provides the small glue
> needed to use the existing Designware driver to make it work on
> the Axis ARTPEC-6 SoC.
>
> Signed-off-by: Niklas Cassel 

[...]

> --- a/drivers/pci/host/Kconfig
> +++ b/drivers/pci/host/Kconfig
> @@ -244,4 +244,10 @@ config PCIE_ARMADA_8K
>   Designware hardware and therefore the driver re-uses the
>   Designware core functions to implement the driver.
>
> +config PCIE_ARTPEC6
> +   bool "Axis ARTPEC-6 PCIe controller"

Bjorn,

Can we please start requiring new PCI/PCI-e drivers to either:

  (a)  be merged and tested as tristate, or

  (b)  remain bool, but not gratuitously use module macros

I was told there was a preference to convert the existing ones
to tristate, but that is near impossible for me to achieve, since
I can't run-time test them and/or it requires new exports that
have explicitly been NACK'd by other maintainers.

In the meantime, each new merge window adds several
more PCI drivers with the same bool Kconfig that semi
pretend to be modular in the code they use, which just
compounds the problem space.

Thanks,
Paul.
--

> +   depends on MACH_ARTPEC6
> +   select PCIE_DW
> +   select PCIEPORTBUS
> +
>  endmenu
> diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
> index 9c8698e..5bc0af2 100644
> --- a/drivers/pci/host/Makefile
> +++ b/drivers/pci/host/Makefile
> @@ -29,3 +29,4 @@ obj-$(CONFIG_PCIE_QCOM) += pcie-qcom.o
>  obj-$(CONFIG_PCI_HOST_THUNDER_ECAM) += pci-thunder-ecam.o
>  obj-$(CONFIG_PCI_HOST_THUNDER_PEM) += pci-thunder-pem.o
>  obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o
> +obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o
> diff --git a/drivers/pci/host/pcie-artpec6.c b/drivers/pci/host/pcie-artpec6.c
> new file mode 100644
> index 000..d53dbaf
> --- /dev/null
> +++ b/drivers/pci/host/pcie-artpec6.c
> @@ -0,0 +1,293 @@
> +/*
> + * PCIe host controller driver for Axis ARTPEC-6 SoC
> + *
> + * Based on work done by Phil Edworthy 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "pcie-designware.h"
> +
> +#define to_artpec6_pcie(x) container_of(x, struct artpec6_pcie, pp)
> +
> +struct artpec6_pcie {
> +   struct pcie_portpp;
> +   struct regmap   *regmap;
> +   void __iomem*phy_base;
> +};
> +
> +/* PCIe Port Logic registers (memory-mapped) */
> +#define PL_OFFSET  0x700
> +#define PCIE_PHY_DEBUG_R0  (PL_OFFSET + 0x28)
> +#define PCIE_PHY_DEBUG_R1  (PL_OFFSET + 0x2c)
> +
> +#define MISC_CONTROL_1_OFF (PL_OFFSET + 0x1bc)
> +#define  DBI_RO_WR_EN  1
> +
> +/* ARTPEC-6 specific registers */
> +#define PCIECFG0x18
> +#define  PCIECFG_DBG_OEN   (1 << 24)
> +#define  PCIECFG_CORE_RESET_REQ(1 << 21)
> +#define  PCIECFG_LTSSM_ENABLE  (1 << 20)
> +#define  PCIECFG_CLKREQ_B  (1 << 11)
> +#define  PCIECFG_REFCLK_ENABLE (1 << 10)
> +#define  PCIECFG_PLL_ENABLE(1 << 9)
> +#define  PCIECFG_PCLK_ENABLE   (1 << 8)
> +#define  PCIECFG_RISRCREN  (1 << 4)
> +#define  PCIECFG_MODE_TX_DRV_EN(1 << 3)
> +#define  PCIECFG_CISRREN   (1 << 2)
> +#define  PCIECFG_MACRO_ENABLE  (1 << 0)
> +
> +#define NOCCFG 0x40
> +#define NOCCFG_ENABLE_CLK_PCIE (1 << 4)
> +#define NOCCFG_POWER_PCIE_IDLEACK  (1 << 3)
> +#define NOCCFG_POWER_PCIE_IDLE (1 << 2)
> +#define NOCCFG_POWER_PCIE_IDLEREQ  (1 << 1)
> +
> +#define PHY_STATUS 0x118
> +#define PHY_COSPLLLOCK (1 << 0)
> +
> +#define ARTPEC6_CPU_TO_BUS_ADDR0x0FFF
> +
> +static int artpec6_pcie_establish_link(struct pcie_port *pp)
> +{
> +   struct artpec6_pcie *artpec6_pcie = to_artpec6_pcie(pp);
> +   u32 val;
> +   unsigned int retries;
> +
> +   /* Hold DW core in reset */
> +   regmap_read(artpec6_pcie->regmap, PCIECFG, );
> +   val |= PCIECFG_CORE_RESET_REQ;
> +   regmap_write(artpec6_pcie->regmap, PCIECFG, val);
> +
> +   regmap_read(artpec6_pcie->regmap, PCIECFG, );
> +   val |=  PCIECFG_RISRCREN |  /* Receiver term. 50 Ohm */
> +   PCIECFG_MODE_TX_DRV_EN |
> +   PCIECFG_CISRREN |   /* Reference clock term. 100 Ohm */
> +   PCIECFG_MACRO_ENABLE;
> +   

Re: [PATCH v2 2/2] pci: host: new driver for Axis ARTPEC-6 PCIe controller

2016-06-20 Thread Paul Gortmaker
On Mon, May 9, 2016 at 7:49 AM, Niklas Cassel  wrote:
> From: Niklas Cassel 
>
> The Axis ARTPEC-6 SoC integrates a PCIe controller from Synopsys.
> This commit adds a new driver that provides the small glue
> needed to use the existing Designware driver to make it work on
> the Axis ARTPEC-6 SoC.
>
> Signed-off-by: Niklas Cassel 

[...]

> --- a/drivers/pci/host/Kconfig
> +++ b/drivers/pci/host/Kconfig
> @@ -244,4 +244,10 @@ config PCIE_ARMADA_8K
>   Designware hardware and therefore the driver re-uses the
>   Designware core functions to implement the driver.
>
> +config PCIE_ARTPEC6
> +   bool "Axis ARTPEC-6 PCIe controller"

Bjorn,

Can we please start requiring new PCI/PCI-e drivers to either:

  (a)  be merged and tested as tristate, or

  (b)  remain bool, but not gratuitously use module macros

I was told there was a preference to convert the existing ones
to tristate, but that is near impossible for me to achieve, since
I can't run-time test them and/or it requires new exports that
have explicitly been NACK'd by other maintainers.

In the meantime, each new merge window adds several
more PCI drivers with the same bool Kconfig that semi
pretend to be modular in the code they use, which just
compounds the problem space.

Thanks,
Paul.
--

> +   depends on MACH_ARTPEC6
> +   select PCIE_DW
> +   select PCIEPORTBUS
> +
>  endmenu
> diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
> index 9c8698e..5bc0af2 100644
> --- a/drivers/pci/host/Makefile
> +++ b/drivers/pci/host/Makefile
> @@ -29,3 +29,4 @@ obj-$(CONFIG_PCIE_QCOM) += pcie-qcom.o
>  obj-$(CONFIG_PCI_HOST_THUNDER_ECAM) += pci-thunder-ecam.o
>  obj-$(CONFIG_PCI_HOST_THUNDER_PEM) += pci-thunder-pem.o
>  obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o
> +obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o
> diff --git a/drivers/pci/host/pcie-artpec6.c b/drivers/pci/host/pcie-artpec6.c
> new file mode 100644
> index 000..d53dbaf
> --- /dev/null
> +++ b/drivers/pci/host/pcie-artpec6.c
> @@ -0,0 +1,293 @@
> +/*
> + * PCIe host controller driver for Axis ARTPEC-6 SoC
> + *
> + * Based on work done by Phil Edworthy 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "pcie-designware.h"
> +
> +#define to_artpec6_pcie(x) container_of(x, struct artpec6_pcie, pp)
> +
> +struct artpec6_pcie {
> +   struct pcie_portpp;
> +   struct regmap   *regmap;
> +   void __iomem*phy_base;
> +};
> +
> +/* PCIe Port Logic registers (memory-mapped) */
> +#define PL_OFFSET  0x700
> +#define PCIE_PHY_DEBUG_R0  (PL_OFFSET + 0x28)
> +#define PCIE_PHY_DEBUG_R1  (PL_OFFSET + 0x2c)
> +
> +#define MISC_CONTROL_1_OFF (PL_OFFSET + 0x1bc)
> +#define  DBI_RO_WR_EN  1
> +
> +/* ARTPEC-6 specific registers */
> +#define PCIECFG0x18
> +#define  PCIECFG_DBG_OEN   (1 << 24)
> +#define  PCIECFG_CORE_RESET_REQ(1 << 21)
> +#define  PCIECFG_LTSSM_ENABLE  (1 << 20)
> +#define  PCIECFG_CLKREQ_B  (1 << 11)
> +#define  PCIECFG_REFCLK_ENABLE (1 << 10)
> +#define  PCIECFG_PLL_ENABLE(1 << 9)
> +#define  PCIECFG_PCLK_ENABLE   (1 << 8)
> +#define  PCIECFG_RISRCREN  (1 << 4)
> +#define  PCIECFG_MODE_TX_DRV_EN(1 << 3)
> +#define  PCIECFG_CISRREN   (1 << 2)
> +#define  PCIECFG_MACRO_ENABLE  (1 << 0)
> +
> +#define NOCCFG 0x40
> +#define NOCCFG_ENABLE_CLK_PCIE (1 << 4)
> +#define NOCCFG_POWER_PCIE_IDLEACK  (1 << 3)
> +#define NOCCFG_POWER_PCIE_IDLE (1 << 2)
> +#define NOCCFG_POWER_PCIE_IDLEREQ  (1 << 1)
> +
> +#define PHY_STATUS 0x118
> +#define PHY_COSPLLLOCK (1 << 0)
> +
> +#define ARTPEC6_CPU_TO_BUS_ADDR0x0FFF
> +
> +static int artpec6_pcie_establish_link(struct pcie_port *pp)
> +{
> +   struct artpec6_pcie *artpec6_pcie = to_artpec6_pcie(pp);
> +   u32 val;
> +   unsigned int retries;
> +
> +   /* Hold DW core in reset */
> +   regmap_read(artpec6_pcie->regmap, PCIECFG, );
> +   val |= PCIECFG_CORE_RESET_REQ;
> +   regmap_write(artpec6_pcie->regmap, PCIECFG, val);
> +
> +   regmap_read(artpec6_pcie->regmap, PCIECFG, );
> +   val |=  PCIECFG_RISRCREN |  /* Receiver term. 50 Ohm */
> +   PCIECFG_MODE_TX_DRV_EN |
> +   PCIECFG_CISRREN |   /* Reference clock term. 100 Ohm */
> +   PCIECFG_MACRO_ENABLE;
> +   val |= PCIECFG_REFCLK_ENABLE;
> +   val &= ~PCIECFG_DBG_OEN;
> +   val &= 

Re: [PATCH v2 2/2] pci: host: new driver for Axis ARTPEC-6 PCIe controller

2016-06-20 Thread Bjorn Helgaas
On Mon, Jun 20, 2016 at 02:56:58AM +0200, Niklas Cassel wrote:
> 
> On 06/13/2016 03:33 PM, Bjorn Helgaas wrote:
> > On Mon, Jun 13, 2016 at 03:12:13PM +0200, Niklas Cassel wrote:
> >> On 06/10/2016 12:41 AM, Bjorn Helgaas wrote:
> >>> On Mon, May 09, 2016 at 01:49:03PM +0200, Niklas Cassel wrote:
>  From: Niklas Cassel 
> 
>  The Axis ARTPEC-6 SoC integrates a PCIe controller from Synopsys.
>  This commit adds a new driver that provides the small glue
>  needed to use the existing Designware driver to make it work on
>  the Axis ARTPEC-6 SoC.
> 
>  Signed-off-by: Niklas Cassel 
> >>> Hi Niklas,
> >>>
> >>> I'll review this soon.  In the meantime, can you send /proc/iomem and
> >>> /proc/ioport contents?  I'm looking to avoid problems like this:
> >>> http://lkml.kernel.org/r/20160606230537.20936.2892.st...@bhelgaas-glaptop2.roam.corp.google.com
> >>>
> >>> It looks like this is based on DesignWare, so it probably has the
> >>> problem, and will probably be fixed by these:
> >>>
> >>> http://lkml.kernel.org/r/20160606230452.20936.28937.st...@bhelgaas-glaptop2.roam.corp.google.com
> >>> http://lkml.kernel.org/r/20160606230501.20936.71818.st...@bhelgaas-glaptop2.roam.corp.google.com
> >>> http://lkml.kernel.org/r/20160606230508.20936.81845.st...@bhelgaas-glaptop2.roam.corp.google.com
> >>>
> >>> If you wanted to apply those and then send /proc/iomem and
> >>> /proc/ioports, that would be even better.
> >>>
> >>> Bjorn
> >>>
> >> Output with your patches applied:
> > Great, thanks, Niklas!  Comments below:
> >
> >> [2.107320] PCI host bridge /pcie@f805 ranges:
> >> [2.112138]   No bus range found for /pcie@f805, using [bus 00-ff]
> > Your DT *should* provide a bus number range.  There is no way for the
> > PCI core to correctly determine the range.
> Ok, I will update the DT example to include bus-range.
> 
> >
> >> [2.118684]IO 0xc001..0xc001 -> 0x0001
> >> [2.123835]   MEM 0xc002..0xdfff -> 0xc002
> >> [2.245559] artpec6-pcie f805.pcie: link up
> >> [2.250228] artpec6-pcie f805.pcie: PCI host bridge to bus :00
> >> [2.256773] pci_bus :00: root bus resource [bus 00-ff]
> >> [2.262273] pci_bus :00: root bus resource [io  0x-0x] (bus 
> >> address [0x1-0x1])
> > This looks questionable.  This says we're using I/O ports
> > 0x1-0x1 on the PCI bus.  That's legal, but unusual.
> >
> > The conventional PCI spec allowed the upper 16 bits of I/O BARs
> > to be hard-wired to zero "for devices intended for 16-bit I/O
> > systems, such as PC compatibles."
> >
> > I don't know whether a PCIe device with an I/O BAR with the
> > upper bits hard-wired to zero exists.  The conservative approach
> > would be to use ports 0x-0x on PCI.
> Thank you for the detailed explanation.
> We simply chose a value during testing,
> there wasn't any deeper thought behind it.
> Since other DesignWare drivers seem to use 0x0, lets stick to that.
> 
> I'm currently on parental leave, but I'm back on work next week.

Congratulations!

> I will try the new settings and send out a patch that updates the DT example.
> 
> Hopefully there's no hurry, since both changes are in the DT example.

Nope, no hurry.  I'll be on vacation for a while myself.

Bjorn


Re: [PATCH v2 2/2] pci: host: new driver for Axis ARTPEC-6 PCIe controller

2016-06-20 Thread Bjorn Helgaas
On Mon, Jun 20, 2016 at 02:56:58AM +0200, Niklas Cassel wrote:
> 
> On 06/13/2016 03:33 PM, Bjorn Helgaas wrote:
> > On Mon, Jun 13, 2016 at 03:12:13PM +0200, Niklas Cassel wrote:
> >> On 06/10/2016 12:41 AM, Bjorn Helgaas wrote:
> >>> On Mon, May 09, 2016 at 01:49:03PM +0200, Niklas Cassel wrote:
>  From: Niklas Cassel 
> 
>  The Axis ARTPEC-6 SoC integrates a PCIe controller from Synopsys.
>  This commit adds a new driver that provides the small glue
>  needed to use the existing Designware driver to make it work on
>  the Axis ARTPEC-6 SoC.
> 
>  Signed-off-by: Niklas Cassel 
> >>> Hi Niklas,
> >>>
> >>> I'll review this soon.  In the meantime, can you send /proc/iomem and
> >>> /proc/ioport contents?  I'm looking to avoid problems like this:
> >>> http://lkml.kernel.org/r/20160606230537.20936.2892.st...@bhelgaas-glaptop2.roam.corp.google.com
> >>>
> >>> It looks like this is based on DesignWare, so it probably has the
> >>> problem, and will probably be fixed by these:
> >>>
> >>> http://lkml.kernel.org/r/20160606230452.20936.28937.st...@bhelgaas-glaptop2.roam.corp.google.com
> >>> http://lkml.kernel.org/r/20160606230501.20936.71818.st...@bhelgaas-glaptop2.roam.corp.google.com
> >>> http://lkml.kernel.org/r/20160606230508.20936.81845.st...@bhelgaas-glaptop2.roam.corp.google.com
> >>>
> >>> If you wanted to apply those and then send /proc/iomem and
> >>> /proc/ioports, that would be even better.
> >>>
> >>> Bjorn
> >>>
> >> Output with your patches applied:
> > Great, thanks, Niklas!  Comments below:
> >
> >> [2.107320] PCI host bridge /pcie@f805 ranges:
> >> [2.112138]   No bus range found for /pcie@f805, using [bus 00-ff]
> > Your DT *should* provide a bus number range.  There is no way for the
> > PCI core to correctly determine the range.
> Ok, I will update the DT example to include bus-range.
> 
> >
> >> [2.118684]IO 0xc001..0xc001 -> 0x0001
> >> [2.123835]   MEM 0xc002..0xdfff -> 0xc002
> >> [2.245559] artpec6-pcie f805.pcie: link up
> >> [2.250228] artpec6-pcie f805.pcie: PCI host bridge to bus :00
> >> [2.256773] pci_bus :00: root bus resource [bus 00-ff]
> >> [2.262273] pci_bus :00: root bus resource [io  0x-0x] (bus 
> >> address [0x1-0x1])
> > This looks questionable.  This says we're using I/O ports
> > 0x1-0x1 on the PCI bus.  That's legal, but unusual.
> >
> > The conventional PCI spec allowed the upper 16 bits of I/O BARs
> > to be hard-wired to zero "for devices intended for 16-bit I/O
> > systems, such as PC compatibles."
> >
> > I don't know whether a PCIe device with an I/O BAR with the
> > upper bits hard-wired to zero exists.  The conservative approach
> > would be to use ports 0x-0x on PCI.
> Thank you for the detailed explanation.
> We simply chose a value during testing,
> there wasn't any deeper thought behind it.
> Since other DesignWare drivers seem to use 0x0, lets stick to that.
> 
> I'm currently on parental leave, but I'm back on work next week.

Congratulations!

> I will try the new settings and send out a patch that updates the DT example.
> 
> Hopefully there's no hurry, since both changes are in the DT example.

Nope, no hurry.  I'll be on vacation for a while myself.

Bjorn


Re: [PATCH v2 2/2] pci: host: new driver for Axis ARTPEC-6 PCIe controller

2016-06-19 Thread Niklas Cassel

On 06/13/2016 03:33 PM, Bjorn Helgaas wrote:
> On Mon, Jun 13, 2016 at 03:12:13PM +0200, Niklas Cassel wrote:
>> On 06/10/2016 12:41 AM, Bjorn Helgaas wrote:
>>> On Mon, May 09, 2016 at 01:49:03PM +0200, Niklas Cassel wrote:
 From: Niklas Cassel 

 The Axis ARTPEC-6 SoC integrates a PCIe controller from Synopsys.
 This commit adds a new driver that provides the small glue
 needed to use the existing Designware driver to make it work on
 the Axis ARTPEC-6 SoC.

 Signed-off-by: Niklas Cassel 
>>> Hi Niklas,
>>>
>>> I'll review this soon.  In the meantime, can you send /proc/iomem and
>>> /proc/ioport contents?  I'm looking to avoid problems like this:
>>> http://lkml.kernel.org/r/20160606230537.20936.2892.st...@bhelgaas-glaptop2.roam.corp.google.com
>>>
>>> It looks like this is based on DesignWare, so it probably has the
>>> problem, and will probably be fixed by these:
>>>
>>> http://lkml.kernel.org/r/20160606230452.20936.28937.st...@bhelgaas-glaptop2.roam.corp.google.com
>>> http://lkml.kernel.org/r/20160606230501.20936.71818.st...@bhelgaas-glaptop2.roam.corp.google.com
>>> http://lkml.kernel.org/r/20160606230508.20936.81845.st...@bhelgaas-glaptop2.roam.corp.google.com
>>>
>>> If you wanted to apply those and then send /proc/iomem and
>>> /proc/ioports, that would be even better.
>>>
>>> Bjorn
>>>
>> Output with your patches applied:
> Great, thanks, Niklas!  Comments below:
>
>> [2.107320] PCI host bridge /pcie@f805 ranges:
>> [2.112138]   No bus range found for /pcie@f805, using [bus 00-ff]
> Your DT *should* provide a bus number range.  There is no way for the
> PCI core to correctly determine the range.
Ok, I will update the DT example to include bus-range.

>
>> [2.118684]IO 0xc001..0xc001 -> 0x0001
>> [2.123835]   MEM 0xc002..0xdfff -> 0xc002
>> [2.245559] artpec6-pcie f805.pcie: link up
>> [2.250228] artpec6-pcie f805.pcie: PCI host bridge to bus :00
>> [2.256773] pci_bus :00: root bus resource [bus 00-ff]
>> [2.262273] pci_bus :00: root bus resource [io  0x-0x] (bus 
>> address [0x1-0x1])
> This looks questionable.  This says we're using I/O ports
> 0x1-0x1 on the PCI bus.  That's legal, but unusual.
>
> The conventional PCI spec allowed the upper 16 bits of I/O BARs
> to be hard-wired to zero "for devices intended for 16-bit I/O
> systems, such as PC compatibles."
>
> I don't know whether a PCIe device with an I/O BAR with the
> upper bits hard-wired to zero exists.  The conservative approach
> would be to use ports 0x-0x on PCI.
Thank you for the detailed explanation.
We simply chose a value during testing,
there wasn't any deeper thought behind it.
Since other DesignWare drivers seem to use 0x0, lets stick to that.

I'm currently on parental leave, but I'm back on work next week.
I will try the new settings and send out a patch that updates the DT example.

Hopefully there's no hurry, since both changes are in the DT example.

>
>> [2.271250] pci_bus :00: root bus resource [mem 0xc002-0xdfff]
>> [2.278407] PCI: bus0: Fast back to back transfers disabled
>> [2.293358] PCI: bus1: Fast back to back transfers disabled
>> [2.299018] pci :00:00.0: BAR 0: assigned [mem 0xc010-0xc01f]
>> [2.305825] pci :00:00.0: BAR 1: assigned [mem 0xc020-0xc02f]
>> [2.312627] pci :00:00.0: BAR 8: assigned [mem 0xc030-0xc07f]
>> [2.319430] pci :01:00.0: BAR 1: assigned [mem 0xc040-0xc05f]
>> [2.326241] pci :01:00.0: BAR 2: assigned [mem 0xc060-0xc07f]
>> [2.333052] pci :01:00.0: BAR 0: assigned [mem 0xc030-0xc0303fff]
>> [2.339862] pci :00:00.0: PCI bridge to [bus 01]
>> [2.344838] pci :00:00.0:   bridge window [mem 0xc030-0xc07f]
>>
>> # cat /proc/iomem
>> -0fff : System RAM
>>   00208000-01071b2f : Kernel code
>>   0130-01465347 : Kernel data
>> c002-dfff : MEM
>>   c010-c01f : :00:00.0
>>   c020-c02f : :00:00.0
>>   c030-c07f : PCI Bus :01
>> c030-c0303fff : :01:00.0
>>   c0301000-c0301007 : serial
>>   c0301200-c0301207 : serial
>> c040-c05f : :01:00.0
>> c060-c07f : :01:00.0
>> f801-f8013fff : /amba@0/ethernet@f801
>> f8036000-f8036fff : /amba@0/serial@f8036000
>>   f8036000-f8036fff : /amba@0/serial@f8036000
>> f8037000-f8037fff : /amba@0/serial@f8037000
>>   f8037000-f8037fff : /amba@0/serial@f8037000
>> f8038000-f8038fff : /amba@0/serial@f8038000
>>   f8038000-f8038fff : /amba@0/serial@f8038000
>> f8039000-f8039fff : /amba@0/serial@f8039000
>>   f8039000-f8039fff : /amba@0/serial@f8039000
>> f804-f8040fff : phy
>> f805-f8051fff : dbi
>>
>> # cat /proc/ioports
>> - : I/O
>>
>>
>> Without your patches applied:
>> /proc/ioports is 

Re: [PATCH v2 2/2] pci: host: new driver for Axis ARTPEC-6 PCIe controller

2016-06-19 Thread Niklas Cassel

On 06/13/2016 03:33 PM, Bjorn Helgaas wrote:
> On Mon, Jun 13, 2016 at 03:12:13PM +0200, Niklas Cassel wrote:
>> On 06/10/2016 12:41 AM, Bjorn Helgaas wrote:
>>> On Mon, May 09, 2016 at 01:49:03PM +0200, Niklas Cassel wrote:
 From: Niklas Cassel 

 The Axis ARTPEC-6 SoC integrates a PCIe controller from Synopsys.
 This commit adds a new driver that provides the small glue
 needed to use the existing Designware driver to make it work on
 the Axis ARTPEC-6 SoC.

 Signed-off-by: Niklas Cassel 
>>> Hi Niklas,
>>>
>>> I'll review this soon.  In the meantime, can you send /proc/iomem and
>>> /proc/ioport contents?  I'm looking to avoid problems like this:
>>> http://lkml.kernel.org/r/20160606230537.20936.2892.st...@bhelgaas-glaptop2.roam.corp.google.com
>>>
>>> It looks like this is based on DesignWare, so it probably has the
>>> problem, and will probably be fixed by these:
>>>
>>> http://lkml.kernel.org/r/20160606230452.20936.28937.st...@bhelgaas-glaptop2.roam.corp.google.com
>>> http://lkml.kernel.org/r/20160606230501.20936.71818.st...@bhelgaas-glaptop2.roam.corp.google.com
>>> http://lkml.kernel.org/r/20160606230508.20936.81845.st...@bhelgaas-glaptop2.roam.corp.google.com
>>>
>>> If you wanted to apply those and then send /proc/iomem and
>>> /proc/ioports, that would be even better.
>>>
>>> Bjorn
>>>
>> Output with your patches applied:
> Great, thanks, Niklas!  Comments below:
>
>> [2.107320] PCI host bridge /pcie@f805 ranges:
>> [2.112138]   No bus range found for /pcie@f805, using [bus 00-ff]
> Your DT *should* provide a bus number range.  There is no way for the
> PCI core to correctly determine the range.
Ok, I will update the DT example to include bus-range.

>
>> [2.118684]IO 0xc001..0xc001 -> 0x0001
>> [2.123835]   MEM 0xc002..0xdfff -> 0xc002
>> [2.245559] artpec6-pcie f805.pcie: link up
>> [2.250228] artpec6-pcie f805.pcie: PCI host bridge to bus :00
>> [2.256773] pci_bus :00: root bus resource [bus 00-ff]
>> [2.262273] pci_bus :00: root bus resource [io  0x-0x] (bus 
>> address [0x1-0x1])
> This looks questionable.  This says we're using I/O ports
> 0x1-0x1 on the PCI bus.  That's legal, but unusual.
>
> The conventional PCI spec allowed the upper 16 bits of I/O BARs
> to be hard-wired to zero "for devices intended for 16-bit I/O
> systems, such as PC compatibles."
>
> I don't know whether a PCIe device with an I/O BAR with the
> upper bits hard-wired to zero exists.  The conservative approach
> would be to use ports 0x-0x on PCI.
Thank you for the detailed explanation.
We simply chose a value during testing,
there wasn't any deeper thought behind it.
Since other DesignWare drivers seem to use 0x0, lets stick to that.

I'm currently on parental leave, but I'm back on work next week.
I will try the new settings and send out a patch that updates the DT example.

Hopefully there's no hurry, since both changes are in the DT example.

>
>> [2.271250] pci_bus :00: root bus resource [mem 0xc002-0xdfff]
>> [2.278407] PCI: bus0: Fast back to back transfers disabled
>> [2.293358] PCI: bus1: Fast back to back transfers disabled
>> [2.299018] pci :00:00.0: BAR 0: assigned [mem 0xc010-0xc01f]
>> [2.305825] pci :00:00.0: BAR 1: assigned [mem 0xc020-0xc02f]
>> [2.312627] pci :00:00.0: BAR 8: assigned [mem 0xc030-0xc07f]
>> [2.319430] pci :01:00.0: BAR 1: assigned [mem 0xc040-0xc05f]
>> [2.326241] pci :01:00.0: BAR 2: assigned [mem 0xc060-0xc07f]
>> [2.333052] pci :01:00.0: BAR 0: assigned [mem 0xc030-0xc0303fff]
>> [2.339862] pci :00:00.0: PCI bridge to [bus 01]
>> [2.344838] pci :00:00.0:   bridge window [mem 0xc030-0xc07f]
>>
>> # cat /proc/iomem
>> -0fff : System RAM
>>   00208000-01071b2f : Kernel code
>>   0130-01465347 : Kernel data
>> c002-dfff : MEM
>>   c010-c01f : :00:00.0
>>   c020-c02f : :00:00.0
>>   c030-c07f : PCI Bus :01
>> c030-c0303fff : :01:00.0
>>   c0301000-c0301007 : serial
>>   c0301200-c0301207 : serial
>> c040-c05f : :01:00.0
>> c060-c07f : :01:00.0
>> f801-f8013fff : /amba@0/ethernet@f801
>> f8036000-f8036fff : /amba@0/serial@f8036000
>>   f8036000-f8036fff : /amba@0/serial@f8036000
>> f8037000-f8037fff : /amba@0/serial@f8037000
>>   f8037000-f8037fff : /amba@0/serial@f8037000
>> f8038000-f8038fff : /amba@0/serial@f8038000
>>   f8038000-f8038fff : /amba@0/serial@f8038000
>> f8039000-f8039fff : /amba@0/serial@f8039000
>>   f8039000-f8039fff : /amba@0/serial@f8039000
>> f804-f8040fff : phy
>> f805-f8051fff : dbi
>>
>> # cat /proc/ioports
>> - : I/O
>>
>>
>> Without your patches applied:
>> /proc/ioports is empty.
>> /proc/iomem is missing the node 

Re: [PATCH v2 2/2] pci: host: new driver for Axis ARTPEC-6 PCIe controller

2016-06-13 Thread Bjorn Helgaas
On Mon, Jun 13, 2016 at 03:12:13PM +0200, Niklas Cassel wrote:
> On 06/10/2016 12:41 AM, Bjorn Helgaas wrote:
> > On Mon, May 09, 2016 at 01:49:03PM +0200, Niklas Cassel wrote:
> >> From: Niklas Cassel 
> >>
> >> The Axis ARTPEC-6 SoC integrates a PCIe controller from Synopsys.
> >> This commit adds a new driver that provides the small glue
> >> needed to use the existing Designware driver to make it work on
> >> the Axis ARTPEC-6 SoC.
> >>
> >> Signed-off-by: Niklas Cassel 
> > Hi Niklas,
> >
> > I'll review this soon.  In the meantime, can you send /proc/iomem and
> > /proc/ioport contents?  I'm looking to avoid problems like this:
> > http://lkml.kernel.org/r/20160606230537.20936.2892.st...@bhelgaas-glaptop2.roam.corp.google.com
> >
> > It looks like this is based on DesignWare, so it probably has the
> > problem, and will probably be fixed by these:
> >
> > http://lkml.kernel.org/r/20160606230452.20936.28937.st...@bhelgaas-glaptop2.roam.corp.google.com
> > http://lkml.kernel.org/r/20160606230501.20936.71818.st...@bhelgaas-glaptop2.roam.corp.google.com
> > http://lkml.kernel.org/r/20160606230508.20936.81845.st...@bhelgaas-glaptop2.roam.corp.google.com
> >
> > If you wanted to apply those and then send /proc/iomem and
> > /proc/ioports, that would be even better.
> >
> > Bjorn
> >
> 
> Output with your patches applied:

Great, thanks, Niklas!  Comments below:

> [2.107320] PCI host bridge /pcie@f805 ranges:
> [2.112138]   No bus range found for /pcie@f805, using [bus 00-ff]

Your DT *should* provide a bus number range.  There is no way for the
PCI core to correctly determine the range.

> [2.118684]IO 0xc001..0xc001 -> 0x0001
> [2.123835]   MEM 0xc002..0xdfff -> 0xc002
> [2.245559] artpec6-pcie f805.pcie: link up
> [2.250228] artpec6-pcie f805.pcie: PCI host bridge to bus :00
> [2.256773] pci_bus :00: root bus resource [bus 00-ff]
> [2.262273] pci_bus :00: root bus resource [io  0x-0x] (bus 
> address [0x1-0x1])

This looks questionable.  This says we're using I/O ports
0x1-0x1 on the PCI bus.  That's legal, but unusual.

The conventional PCI spec allowed the upper 16 bits of I/O BARs
to be hard-wired to zero "for devices intended for 16-bit I/O
systems, such as PC compatibles."

I don't know whether a PCIe device with an I/O BAR with the
upper bits hard-wired to zero exists.  The conservative approach
would be to use ports 0x-0x on PCI.

> [2.271250] pci_bus :00: root bus resource [mem 0xc002-0xdfff]
> [2.278407] PCI: bus0: Fast back to back transfers disabled
> [2.293358] PCI: bus1: Fast back to back transfers disabled
> [2.299018] pci :00:00.0: BAR 0: assigned [mem 0xc010-0xc01f]
> [2.305825] pci :00:00.0: BAR 1: assigned [mem 0xc020-0xc02f]
> [2.312627] pci :00:00.0: BAR 8: assigned [mem 0xc030-0xc07f]
> [2.319430] pci :01:00.0: BAR 1: assigned [mem 0xc040-0xc05f]
> [2.326241] pci :01:00.0: BAR 2: assigned [mem 0xc060-0xc07f]
> [2.333052] pci :01:00.0: BAR 0: assigned [mem 0xc030-0xc0303fff]
> [2.339862] pci :00:00.0: PCI bridge to [bus 01]
> [2.344838] pci :00:00.0:   bridge window [mem 0xc030-0xc07f]
> 
> # cat /proc/iomem
> -0fff : System RAM
>   00208000-01071b2f : Kernel code
>   0130-01465347 : Kernel data
> c002-dfff : MEM
>   c010-c01f : :00:00.0
>   c020-c02f : :00:00.0
>   c030-c07f : PCI Bus :01
> c030-c0303fff : :01:00.0
>   c0301000-c0301007 : serial
>   c0301200-c0301207 : serial
> c040-c05f : :01:00.0
> c060-c07f : :01:00.0
> f801-f8013fff : /amba@0/ethernet@f801
> f8036000-f8036fff : /amba@0/serial@f8036000
>   f8036000-f8036fff : /amba@0/serial@f8036000
> f8037000-f8037fff : /amba@0/serial@f8037000
>   f8037000-f8037fff : /amba@0/serial@f8037000
> f8038000-f8038fff : /amba@0/serial@f8038000
>   f8038000-f8038fff : /amba@0/serial@f8038000
> f8039000-f8039fff : /amba@0/serial@f8039000
>   f8039000-f8039fff : /amba@0/serial@f8039000
> f804-f8040fff : phy
> f805-f8051fff : dbi
> 
> # cat /proc/ioports
> - : I/O
> 
> 
> Without your patches applied:
> /proc/ioports is empty.
> /proc/iomem is missing the node "c002-dfff : MEM"
> and all of its child nodes.

Good, that's exactly what I expected.  We could use better names for
the resources, so they're connected to the PCIe controller, but at
least we have the entries.

Bjorn


Re: [PATCH v2 2/2] pci: host: new driver for Axis ARTPEC-6 PCIe controller

2016-06-13 Thread Bjorn Helgaas
On Mon, Jun 13, 2016 at 03:12:13PM +0200, Niklas Cassel wrote:
> On 06/10/2016 12:41 AM, Bjorn Helgaas wrote:
> > On Mon, May 09, 2016 at 01:49:03PM +0200, Niklas Cassel wrote:
> >> From: Niklas Cassel 
> >>
> >> The Axis ARTPEC-6 SoC integrates a PCIe controller from Synopsys.
> >> This commit adds a new driver that provides the small glue
> >> needed to use the existing Designware driver to make it work on
> >> the Axis ARTPEC-6 SoC.
> >>
> >> Signed-off-by: Niklas Cassel 
> > Hi Niklas,
> >
> > I'll review this soon.  In the meantime, can you send /proc/iomem and
> > /proc/ioport contents?  I'm looking to avoid problems like this:
> > http://lkml.kernel.org/r/20160606230537.20936.2892.st...@bhelgaas-glaptop2.roam.corp.google.com
> >
> > It looks like this is based on DesignWare, so it probably has the
> > problem, and will probably be fixed by these:
> >
> > http://lkml.kernel.org/r/20160606230452.20936.28937.st...@bhelgaas-glaptop2.roam.corp.google.com
> > http://lkml.kernel.org/r/20160606230501.20936.71818.st...@bhelgaas-glaptop2.roam.corp.google.com
> > http://lkml.kernel.org/r/20160606230508.20936.81845.st...@bhelgaas-glaptop2.roam.corp.google.com
> >
> > If you wanted to apply those and then send /proc/iomem and
> > /proc/ioports, that would be even better.
> >
> > Bjorn
> >
> 
> Output with your patches applied:

Great, thanks, Niklas!  Comments below:

> [2.107320] PCI host bridge /pcie@f805 ranges:
> [2.112138]   No bus range found for /pcie@f805, using [bus 00-ff]

Your DT *should* provide a bus number range.  There is no way for the
PCI core to correctly determine the range.

> [2.118684]IO 0xc001..0xc001 -> 0x0001
> [2.123835]   MEM 0xc002..0xdfff -> 0xc002
> [2.245559] artpec6-pcie f805.pcie: link up
> [2.250228] artpec6-pcie f805.pcie: PCI host bridge to bus :00
> [2.256773] pci_bus :00: root bus resource [bus 00-ff]
> [2.262273] pci_bus :00: root bus resource [io  0x-0x] (bus 
> address [0x1-0x1])

This looks questionable.  This says we're using I/O ports
0x1-0x1 on the PCI bus.  That's legal, but unusual.

The conventional PCI spec allowed the upper 16 bits of I/O BARs
to be hard-wired to zero "for devices intended for 16-bit I/O
systems, such as PC compatibles."

I don't know whether a PCIe device with an I/O BAR with the
upper bits hard-wired to zero exists.  The conservative approach
would be to use ports 0x-0x on PCI.

> [2.271250] pci_bus :00: root bus resource [mem 0xc002-0xdfff]
> [2.278407] PCI: bus0: Fast back to back transfers disabled
> [2.293358] PCI: bus1: Fast back to back transfers disabled
> [2.299018] pci :00:00.0: BAR 0: assigned [mem 0xc010-0xc01f]
> [2.305825] pci :00:00.0: BAR 1: assigned [mem 0xc020-0xc02f]
> [2.312627] pci :00:00.0: BAR 8: assigned [mem 0xc030-0xc07f]
> [2.319430] pci :01:00.0: BAR 1: assigned [mem 0xc040-0xc05f]
> [2.326241] pci :01:00.0: BAR 2: assigned [mem 0xc060-0xc07f]
> [2.333052] pci :01:00.0: BAR 0: assigned [mem 0xc030-0xc0303fff]
> [2.339862] pci :00:00.0: PCI bridge to [bus 01]
> [2.344838] pci :00:00.0:   bridge window [mem 0xc030-0xc07f]
> 
> # cat /proc/iomem
> -0fff : System RAM
>   00208000-01071b2f : Kernel code
>   0130-01465347 : Kernel data
> c002-dfff : MEM
>   c010-c01f : :00:00.0
>   c020-c02f : :00:00.0
>   c030-c07f : PCI Bus :01
> c030-c0303fff : :01:00.0
>   c0301000-c0301007 : serial
>   c0301200-c0301207 : serial
> c040-c05f : :01:00.0
> c060-c07f : :01:00.0
> f801-f8013fff : /amba@0/ethernet@f801
> f8036000-f8036fff : /amba@0/serial@f8036000
>   f8036000-f8036fff : /amba@0/serial@f8036000
> f8037000-f8037fff : /amba@0/serial@f8037000
>   f8037000-f8037fff : /amba@0/serial@f8037000
> f8038000-f8038fff : /amba@0/serial@f8038000
>   f8038000-f8038fff : /amba@0/serial@f8038000
> f8039000-f8039fff : /amba@0/serial@f8039000
>   f8039000-f8039fff : /amba@0/serial@f8039000
> f804-f8040fff : phy
> f805-f8051fff : dbi
> 
> # cat /proc/ioports
> - : I/O
> 
> 
> Without your patches applied:
> /proc/ioports is empty.
> /proc/iomem is missing the node "c002-dfff : MEM"
> and all of its child nodes.

Good, that's exactly what I expected.  We could use better names for
the resources, so they're connected to the PCIe controller, but at
least we have the entries.

Bjorn


Re: [PATCH v2 2/2] pci: host: new driver for Axis ARTPEC-6 PCIe controller

2016-06-13 Thread Niklas Cassel

On 06/10/2016 12:41 AM, Bjorn Helgaas wrote:
> On Mon, May 09, 2016 at 01:49:03PM +0200, Niklas Cassel wrote:
>> From: Niklas Cassel 
>>
>> The Axis ARTPEC-6 SoC integrates a PCIe controller from Synopsys.
>> This commit adds a new driver that provides the small glue
>> needed to use the existing Designware driver to make it work on
>> the Axis ARTPEC-6 SoC.
>>
>> Signed-off-by: Niklas Cassel 
> Hi Niklas,
>
> I'll review this soon.  In the meantime, can you send /proc/iomem and
> /proc/ioport contents?  I'm looking to avoid problems like this:
> http://lkml.kernel.org/r/20160606230537.20936.2892.st...@bhelgaas-glaptop2.roam.corp.google.com
>
> It looks like this is based on DesignWare, so it probably has the
> problem, and will probably be fixed by these:
>
> http://lkml.kernel.org/r/20160606230452.20936.28937.st...@bhelgaas-glaptop2.roam.corp.google.com
> http://lkml.kernel.org/r/20160606230501.20936.71818.st...@bhelgaas-glaptop2.roam.corp.google.com
> http://lkml.kernel.org/r/20160606230508.20936.81845.st...@bhelgaas-glaptop2.roam.corp.google.com
>
> If you wanted to apply those and then send /proc/iomem and
> /proc/ioports, that would be even better.
>
> Bjorn
>

Output with your patches applied:

[2.107320] PCI host bridge /pcie@f805 ranges:
[2.112138]   No bus range found for /pcie@f805, using [bus 00-ff]
[2.118684]IO 0xc001..0xc001 -> 0x0001
[2.123835]   MEM 0xc002..0xdfff -> 0xc002
[2.245559] artpec6-pcie f805.pcie: link up
[2.250228] artpec6-pcie f805.pcie: PCI host bridge to bus :00
[2.256773] pci_bus :00: root bus resource [bus 00-ff]
[2.262273] pci_bus :00: root bus resource [io  0x-0x] (bus 
address [0x1-0x1])
[2.271250] pci_bus :00: root bus resource [mem 0xc002-0xdfff]
[2.278407] PCI: bus0: Fast back to back transfers disabled
[2.293358] PCI: bus1: Fast back to back transfers disabled
[2.299018] pci :00:00.0: BAR 0: assigned [mem 0xc010-0xc01f]
[2.305825] pci :00:00.0: BAR 1: assigned [mem 0xc020-0xc02f]
[2.312627] pci :00:00.0: BAR 8: assigned [mem 0xc030-0xc07f]
[2.319430] pci :01:00.0: BAR 1: assigned [mem 0xc040-0xc05f]
[2.326241] pci :01:00.0: BAR 2: assigned [mem 0xc060-0xc07f]
[2.333052] pci :01:00.0: BAR 0: assigned [mem 0xc030-0xc0303fff]
[2.339862] pci :00:00.0: PCI bridge to [bus 01]
[2.344838] pci :00:00.0:   bridge window [mem 0xc030-0xc07f]

# cat /proc/iomem
-0fff : System RAM
  00208000-01071b2f : Kernel code
  0130-01465347 : Kernel data
c002-dfff : MEM
  c010-c01f : :00:00.0
  c020-c02f : :00:00.0
  c030-c07f : PCI Bus :01
c030-c0303fff : :01:00.0
  c0301000-c0301007 : serial
  c0301200-c0301207 : serial
c040-c05f : :01:00.0
c060-c07f : :01:00.0
f801-f8013fff : /amba@0/ethernet@f801
f8036000-f8036fff : /amba@0/serial@f8036000
  f8036000-f8036fff : /amba@0/serial@f8036000
f8037000-f8037fff : /amba@0/serial@f8037000
  f8037000-f8037fff : /amba@0/serial@f8037000
f8038000-f8038fff : /amba@0/serial@f8038000
  f8038000-f8038fff : /amba@0/serial@f8038000
f8039000-f8039fff : /amba@0/serial@f8039000
  f8039000-f8039fff : /amba@0/serial@f8039000
f804-f8040fff : phy
f805-f8051fff : dbi

# cat /proc/ioports
- : I/O


Without your patches applied:
/proc/ioports is empty.
/proc/iomem is missing the node "c002-dfff : MEM"
and all of its child nodes.

>> ---
>> Changes since v1:
>>  - Rename syscon DT node to be more descriptive
>>  - Use module_platform_driver macro
>>
>>  MAINTAINERS |   9 ++
>>  drivers/pci/host/Kconfig|   6 +
>>  drivers/pci/host/Makefile   |   1 +
>>  drivers/pci/host/pcie-artpec6.c | 293 
>> 
>>  4 files changed, 309 insertions(+)
>>  create mode 100644 drivers/pci/host/pcie-artpec6.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index c18feb5..88d5443 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -8772,6 +8772,15 @@ S:Maintained
>>  F:  Documentation/devicetree/bindings/pci/xgene-pci-msi.txt
>>  F:  drivers/pci/host/pci-xgene-msi.c
>>  
>> +PCIE DRIVER FOR AXIS ARTPEC
>> +M:  Niklas Cassel 
>> +M:  Jesper Nilsson 
>> +L:  linux-arm-ker...@axis.com
>> +L:  linux-...@vger.kernel.org
>> +S:  Maintained
>> +F:  Documentation/devicetree/bindings/pci/axis,artpec*
>> +F:  drivers/pci/host/*artpec*
>> +
>>  PCIE DRIVER FOR HISILICON
>>  M:  Zhou Wang 
>>  M:  Gabriele Paoloni 
>> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
>> index 5855f85..29e159a 100644
>> --- a/drivers/pci/host/Kconfig
>> +++ b/drivers/pci/host/Kconfig
>> @@ 

Re: [PATCH v2 2/2] pci: host: new driver for Axis ARTPEC-6 PCIe controller

2016-06-13 Thread Niklas Cassel

On 06/10/2016 12:41 AM, Bjorn Helgaas wrote:
> On Mon, May 09, 2016 at 01:49:03PM +0200, Niklas Cassel wrote:
>> From: Niklas Cassel 
>>
>> The Axis ARTPEC-6 SoC integrates a PCIe controller from Synopsys.
>> This commit adds a new driver that provides the small glue
>> needed to use the existing Designware driver to make it work on
>> the Axis ARTPEC-6 SoC.
>>
>> Signed-off-by: Niklas Cassel 
> Hi Niklas,
>
> I'll review this soon.  In the meantime, can you send /proc/iomem and
> /proc/ioport contents?  I'm looking to avoid problems like this:
> http://lkml.kernel.org/r/20160606230537.20936.2892.st...@bhelgaas-glaptop2.roam.corp.google.com
>
> It looks like this is based on DesignWare, so it probably has the
> problem, and will probably be fixed by these:
>
> http://lkml.kernel.org/r/20160606230452.20936.28937.st...@bhelgaas-glaptop2.roam.corp.google.com
> http://lkml.kernel.org/r/20160606230501.20936.71818.st...@bhelgaas-glaptop2.roam.corp.google.com
> http://lkml.kernel.org/r/20160606230508.20936.81845.st...@bhelgaas-glaptop2.roam.corp.google.com
>
> If you wanted to apply those and then send /proc/iomem and
> /proc/ioports, that would be even better.
>
> Bjorn
>

Output with your patches applied:

[2.107320] PCI host bridge /pcie@f805 ranges:
[2.112138]   No bus range found for /pcie@f805, using [bus 00-ff]
[2.118684]IO 0xc001..0xc001 -> 0x0001
[2.123835]   MEM 0xc002..0xdfff -> 0xc002
[2.245559] artpec6-pcie f805.pcie: link up
[2.250228] artpec6-pcie f805.pcie: PCI host bridge to bus :00
[2.256773] pci_bus :00: root bus resource [bus 00-ff]
[2.262273] pci_bus :00: root bus resource [io  0x-0x] (bus 
address [0x1-0x1])
[2.271250] pci_bus :00: root bus resource [mem 0xc002-0xdfff]
[2.278407] PCI: bus0: Fast back to back transfers disabled
[2.293358] PCI: bus1: Fast back to back transfers disabled
[2.299018] pci :00:00.0: BAR 0: assigned [mem 0xc010-0xc01f]
[2.305825] pci :00:00.0: BAR 1: assigned [mem 0xc020-0xc02f]
[2.312627] pci :00:00.0: BAR 8: assigned [mem 0xc030-0xc07f]
[2.319430] pci :01:00.0: BAR 1: assigned [mem 0xc040-0xc05f]
[2.326241] pci :01:00.0: BAR 2: assigned [mem 0xc060-0xc07f]
[2.333052] pci :01:00.0: BAR 0: assigned [mem 0xc030-0xc0303fff]
[2.339862] pci :00:00.0: PCI bridge to [bus 01]
[2.344838] pci :00:00.0:   bridge window [mem 0xc030-0xc07f]

# cat /proc/iomem
-0fff : System RAM
  00208000-01071b2f : Kernel code
  0130-01465347 : Kernel data
c002-dfff : MEM
  c010-c01f : :00:00.0
  c020-c02f : :00:00.0
  c030-c07f : PCI Bus :01
c030-c0303fff : :01:00.0
  c0301000-c0301007 : serial
  c0301200-c0301207 : serial
c040-c05f : :01:00.0
c060-c07f : :01:00.0
f801-f8013fff : /amba@0/ethernet@f801
f8036000-f8036fff : /amba@0/serial@f8036000
  f8036000-f8036fff : /amba@0/serial@f8036000
f8037000-f8037fff : /amba@0/serial@f8037000
  f8037000-f8037fff : /amba@0/serial@f8037000
f8038000-f8038fff : /amba@0/serial@f8038000
  f8038000-f8038fff : /amba@0/serial@f8038000
f8039000-f8039fff : /amba@0/serial@f8039000
  f8039000-f8039fff : /amba@0/serial@f8039000
f804-f8040fff : phy
f805-f8051fff : dbi

# cat /proc/ioports
- : I/O


Without your patches applied:
/proc/ioports is empty.
/proc/iomem is missing the node "c002-dfff : MEM"
and all of its child nodes.

>> ---
>> Changes since v1:
>>  - Rename syscon DT node to be more descriptive
>>  - Use module_platform_driver macro
>>
>>  MAINTAINERS |   9 ++
>>  drivers/pci/host/Kconfig|   6 +
>>  drivers/pci/host/Makefile   |   1 +
>>  drivers/pci/host/pcie-artpec6.c | 293 
>> 
>>  4 files changed, 309 insertions(+)
>>  create mode 100644 drivers/pci/host/pcie-artpec6.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index c18feb5..88d5443 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -8772,6 +8772,15 @@ S:Maintained
>>  F:  Documentation/devicetree/bindings/pci/xgene-pci-msi.txt
>>  F:  drivers/pci/host/pci-xgene-msi.c
>>  
>> +PCIE DRIVER FOR AXIS ARTPEC
>> +M:  Niklas Cassel 
>> +M:  Jesper Nilsson 
>> +L:  linux-arm-ker...@axis.com
>> +L:  linux-...@vger.kernel.org
>> +S:  Maintained
>> +F:  Documentation/devicetree/bindings/pci/axis,artpec*
>> +F:  drivers/pci/host/*artpec*
>> +
>>  PCIE DRIVER FOR HISILICON
>>  M:  Zhou Wang 
>>  M:  Gabriele Paoloni 
>> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
>> index 5855f85..29e159a 100644
>> --- a/drivers/pci/host/Kconfig
>> +++ b/drivers/pci/host/Kconfig
>> @@ -244,4 +244,10 @@ config PCIE_ARMADA_8K
>>Designware hardware and therefore the driver re-uses the
>>Designware core functions to 

Re: [PATCH v2 2/2] pci: host: new driver for Axis ARTPEC-6 PCIe controller

2016-06-09 Thread Bjorn Helgaas
On Mon, May 09, 2016 at 01:49:03PM +0200, Niklas Cassel wrote:
> From: Niklas Cassel 
> 
> The Axis ARTPEC-6 SoC integrates a PCIe controller from Synopsys.
> This commit adds a new driver that provides the small glue
> needed to use the existing Designware driver to make it work on
> the Axis ARTPEC-6 SoC.
> 
> Signed-off-by: Niklas Cassel 

Hi Niklas,

I'll review this soon.  In the meantime, can you send /proc/iomem and
/proc/ioport contents?  I'm looking to avoid problems like this:
http://lkml.kernel.org/r/20160606230537.20936.2892.st...@bhelgaas-glaptop2.roam.corp.google.com

It looks like this is based on DesignWare, so it probably has the
problem, and will probably be fixed by these:

http://lkml.kernel.org/r/20160606230452.20936.28937.st...@bhelgaas-glaptop2.roam.corp.google.com
http://lkml.kernel.org/r/20160606230501.20936.71818.st...@bhelgaas-glaptop2.roam.corp.google.com
http://lkml.kernel.org/r/20160606230508.20936.81845.st...@bhelgaas-glaptop2.roam.corp.google.com

If you wanted to apply those and then send /proc/iomem and
/proc/ioports, that would be even better.

Bjorn

> ---
> Changes since v1:
>  - Rename syscon DT node to be more descriptive
>  - Use module_platform_driver macro
> 
>  MAINTAINERS |   9 ++
>  drivers/pci/host/Kconfig|   6 +
>  drivers/pci/host/Makefile   |   1 +
>  drivers/pci/host/pcie-artpec6.c | 293 
> 
>  4 files changed, 309 insertions(+)
>  create mode 100644 drivers/pci/host/pcie-artpec6.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index c18feb5..88d5443 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -8772,6 +8772,15 @@ S: Maintained
>  F:   Documentation/devicetree/bindings/pci/xgene-pci-msi.txt
>  F:   drivers/pci/host/pci-xgene-msi.c
>  
> +PCIE DRIVER FOR AXIS ARTPEC
> +M:   Niklas Cassel 
> +M:   Jesper Nilsson 
> +L:   linux-arm-ker...@axis.com
> +L:   linux-...@vger.kernel.org
> +S:   Maintained
> +F:   Documentation/devicetree/bindings/pci/axis,artpec*
> +F:   drivers/pci/host/*artpec*
> +
>  PCIE DRIVER FOR HISILICON
>  M:   Zhou Wang 
>  M:   Gabriele Paoloni 
> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
> index 5855f85..29e159a 100644
> --- a/drivers/pci/host/Kconfig
> +++ b/drivers/pci/host/Kconfig
> @@ -244,4 +244,10 @@ config PCIE_ARMADA_8K
> Designware hardware and therefore the driver re-uses the
> Designware core functions to implement the driver.
>  
> +config PCIE_ARTPEC6
> + bool "Axis ARTPEC-6 PCIe controller"
> + depends on MACH_ARTPEC6
> + select PCIE_DW
> + select PCIEPORTBUS
> +
>  endmenu
> diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
> index 9c8698e..5bc0af2 100644
> --- a/drivers/pci/host/Makefile
> +++ b/drivers/pci/host/Makefile
> @@ -29,3 +29,4 @@ obj-$(CONFIG_PCIE_QCOM) += pcie-qcom.o
>  obj-$(CONFIG_PCI_HOST_THUNDER_ECAM) += pci-thunder-ecam.o
>  obj-$(CONFIG_PCI_HOST_THUNDER_PEM) += pci-thunder-pem.o
>  obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o
> +obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o
> diff --git a/drivers/pci/host/pcie-artpec6.c b/drivers/pci/host/pcie-artpec6.c
> new file mode 100644
> index 000..d53dbaf
> --- /dev/null
> +++ b/drivers/pci/host/pcie-artpec6.c
> @@ -0,0 +1,293 @@
> +/*
> + * PCIe host controller driver for Axis ARTPEC-6 SoC
> + *
> + * Based on work done by Phil Edworthy 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "pcie-designware.h"
> +
> +#define to_artpec6_pcie(x)   container_of(x, struct artpec6_pcie, pp)
> +
> +struct artpec6_pcie {
> + struct pcie_portpp;
> + struct regmap   *regmap;
> + void __iomem*phy_base;
> +};
> +
> +/* PCIe Port Logic registers (memory-mapped) */
> +#define PL_OFFSET0x700
> +#define PCIE_PHY_DEBUG_R0(PL_OFFSET + 0x28)
> +#define PCIE_PHY_DEBUG_R1(PL_OFFSET + 0x2c)
> +
> +#define MISC_CONTROL_1_OFF   (PL_OFFSET + 0x1bc)
> +#define  DBI_RO_WR_EN1
> +
> +/* ARTPEC-6 specific registers */
> +#define PCIECFG  0x18
> +#define  PCIECFG_DBG_OEN (1 << 24)
> +#define  PCIECFG_CORE_RESET_REQ  (1 << 21)
> +#define  PCIECFG_LTSSM_ENABLE(1 << 20)
> +#define  PCIECFG_CLKREQ_B(1 << 11)
> +#define  PCIECFG_REFCLK_ENABLE   (1 << 10)
> +#define  PCIECFG_PLL_ENABLE  (1 << 9)
> +#define  PCIECFG_PCLK_ENABLE (1 << 8)
> 

Re: [PATCH v2 2/2] pci: host: new driver for Axis ARTPEC-6 PCIe controller

2016-06-09 Thread Bjorn Helgaas
On Mon, May 09, 2016 at 01:49:03PM +0200, Niklas Cassel wrote:
> From: Niklas Cassel 
> 
> The Axis ARTPEC-6 SoC integrates a PCIe controller from Synopsys.
> This commit adds a new driver that provides the small glue
> needed to use the existing Designware driver to make it work on
> the Axis ARTPEC-6 SoC.
> 
> Signed-off-by: Niklas Cassel 

Hi Niklas,

I'll review this soon.  In the meantime, can you send /proc/iomem and
/proc/ioport contents?  I'm looking to avoid problems like this:
http://lkml.kernel.org/r/20160606230537.20936.2892.st...@bhelgaas-glaptop2.roam.corp.google.com

It looks like this is based on DesignWare, so it probably has the
problem, and will probably be fixed by these:

http://lkml.kernel.org/r/20160606230452.20936.28937.st...@bhelgaas-glaptop2.roam.corp.google.com
http://lkml.kernel.org/r/20160606230501.20936.71818.st...@bhelgaas-glaptop2.roam.corp.google.com
http://lkml.kernel.org/r/20160606230508.20936.81845.st...@bhelgaas-glaptop2.roam.corp.google.com

If you wanted to apply those and then send /proc/iomem and
/proc/ioports, that would be even better.

Bjorn

> ---
> Changes since v1:
>  - Rename syscon DT node to be more descriptive
>  - Use module_platform_driver macro
> 
>  MAINTAINERS |   9 ++
>  drivers/pci/host/Kconfig|   6 +
>  drivers/pci/host/Makefile   |   1 +
>  drivers/pci/host/pcie-artpec6.c | 293 
> 
>  4 files changed, 309 insertions(+)
>  create mode 100644 drivers/pci/host/pcie-artpec6.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index c18feb5..88d5443 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -8772,6 +8772,15 @@ S: Maintained
>  F:   Documentation/devicetree/bindings/pci/xgene-pci-msi.txt
>  F:   drivers/pci/host/pci-xgene-msi.c
>  
> +PCIE DRIVER FOR AXIS ARTPEC
> +M:   Niklas Cassel 
> +M:   Jesper Nilsson 
> +L:   linux-arm-ker...@axis.com
> +L:   linux-...@vger.kernel.org
> +S:   Maintained
> +F:   Documentation/devicetree/bindings/pci/axis,artpec*
> +F:   drivers/pci/host/*artpec*
> +
>  PCIE DRIVER FOR HISILICON
>  M:   Zhou Wang 
>  M:   Gabriele Paoloni 
> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
> index 5855f85..29e159a 100644
> --- a/drivers/pci/host/Kconfig
> +++ b/drivers/pci/host/Kconfig
> @@ -244,4 +244,10 @@ config PCIE_ARMADA_8K
> Designware hardware and therefore the driver re-uses the
> Designware core functions to implement the driver.
>  
> +config PCIE_ARTPEC6
> + bool "Axis ARTPEC-6 PCIe controller"
> + depends on MACH_ARTPEC6
> + select PCIE_DW
> + select PCIEPORTBUS
> +
>  endmenu
> diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
> index 9c8698e..5bc0af2 100644
> --- a/drivers/pci/host/Makefile
> +++ b/drivers/pci/host/Makefile
> @@ -29,3 +29,4 @@ obj-$(CONFIG_PCIE_QCOM) += pcie-qcom.o
>  obj-$(CONFIG_PCI_HOST_THUNDER_ECAM) += pci-thunder-ecam.o
>  obj-$(CONFIG_PCI_HOST_THUNDER_PEM) += pci-thunder-pem.o
>  obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o
> +obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o
> diff --git a/drivers/pci/host/pcie-artpec6.c b/drivers/pci/host/pcie-artpec6.c
> new file mode 100644
> index 000..d53dbaf
> --- /dev/null
> +++ b/drivers/pci/host/pcie-artpec6.c
> @@ -0,0 +1,293 @@
> +/*
> + * PCIe host controller driver for Axis ARTPEC-6 SoC
> + *
> + * Based on work done by Phil Edworthy 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "pcie-designware.h"
> +
> +#define to_artpec6_pcie(x)   container_of(x, struct artpec6_pcie, pp)
> +
> +struct artpec6_pcie {
> + struct pcie_portpp;
> + struct regmap   *regmap;
> + void __iomem*phy_base;
> +};
> +
> +/* PCIe Port Logic registers (memory-mapped) */
> +#define PL_OFFSET0x700
> +#define PCIE_PHY_DEBUG_R0(PL_OFFSET + 0x28)
> +#define PCIE_PHY_DEBUG_R1(PL_OFFSET + 0x2c)
> +
> +#define MISC_CONTROL_1_OFF   (PL_OFFSET + 0x1bc)
> +#define  DBI_RO_WR_EN1
> +
> +/* ARTPEC-6 specific registers */
> +#define PCIECFG  0x18
> +#define  PCIECFG_DBG_OEN (1 << 24)
> +#define  PCIECFG_CORE_RESET_REQ  (1 << 21)
> +#define  PCIECFG_LTSSM_ENABLE(1 << 20)
> +#define  PCIECFG_CLKREQ_B(1 << 11)
> +#define  PCIECFG_REFCLK_ENABLE   (1 << 10)
> +#define  PCIECFG_PLL_ENABLE  (1 << 9)
> +#define  PCIECFG_PCLK_ENABLE (1 << 8)
> +#define  PCIECFG_RISRCREN(1 << 4)
> +#define  PCIECFG_MODE_TX_DRV_EN  (1 << 3)
> +#define  PCIECFG_CISRREN (1 << 2)
> +#define