Re: [PATCH] dt-bindings: PCI: rcar: Add device tree support for r8a7744

[Lorenzo Pieralisi]

On Mon, Oct 08, 2018 at 09:34:45AM +0200, Simon Horman wrote:
> On Thu, Oct 04, 2018 at 05:07:47PM +0100, Biju Das wrote:
> > Add support for r8a7744. The Renesas RZ/G1N (R8A7744) PCIe controller
> > is identical to the R-Car Gen2 family.
> > 
> > Signed-off-by: Biju Das 
> > Reviewed-by: Chris Paterson 
> 
> Reviewed-by: Simon Horman 

Simon, Biju,

I pulled the same binding for rcar2, I would pull this in the PCI tree
too unless you object, please let me know.

Thanks,
Lorenzo

Re: [PATCH] DT: pci: rcar-pci: document R8A77990 bindings

[Lorenzo Pieralisi]

On Sat, Sep 22, 2018 at 05:02:52PM +0200, Marek Vasut wrote:
> From: Tho Vu 
> 
> Document the R-Car E3 (R8A77990) SoC in the R-Car PCIe bindings.
> 
> Signed-off-by: Tho Vu 
> Signed-off-by: Kazuya Mizuguchi 
> Signed-off-by: Marek Vasut 
> Cc: Geert Uytterhoeven 
> Cc: Lorenzo Pieralisi 
> Cc: Phil Edworthy 
> Cc: Rob Herring 
> Cc: Sergei Shtylyov 
> Cc: Simon Horman 
> Cc: Wolfram Sang 
> Cc: linux-renesas-soc@vger.kernel.org
> ---
>  Documentation/devicetree/bindings/pci/rcar-pci.txt | 1 +
>  1 file changed, 1 insertion(+)

Applied to pci/controller-misc for v4.20, thanks.

Lorenzo

> diff --git a/Documentation/devicetree/bindings/pci/rcar-pci.txt 
> b/Documentation/devicetree/bindings/pci/rcar-pci.txt
> index a5f7fc62d10e..5f2f9e380efb 100644
> --- a/Documentation/devicetree/bindings/pci/rcar-pci.txt
> +++ b/Documentation/devicetree/bindings/pci/rcar-pci.txt
> @@ -9,6 +9,7 @@ compatible: "renesas,pcie-r8a7743" for the R8A7743 SoC;
>   "renesas,pcie-r8a7795" for the R8A7795 SoC;
>   "renesas,pcie-r8a7796" for the R8A7796 SoC;
>   "renesas,pcie-r8a77980" for the R8A77980 SoC;
> + "renesas,pcie-r8a77990" for the R8A77990 SoC;
>   "renesas,pcie-rcar-gen2" for a generic R-Car Gen2 or
>RZ/G1 compatible device.
>   "renesas,pcie-rcar-gen3" for a generic R-Car Gen3 compatible device.
> -- 
> 2.18.0
> 

Re: [PATCH][RFC] PCI: rcar: Add bus notifier so we can limit the DMA range

[Lorenzo Pieralisi]

On Tue, May 22, 2018 at 12:05:14AM +0200, Marek Vasut wrote:
> From: Phil Edworthy 
> 
> The PCIe DMA controller on RCar Gen2 and earlier is on 32bit bus,
> so limit the DMA range to 32bit.
> 
> Signed-off-by: Phil Edworthy 
> Signed-off-by: Marek Vasut 
> Cc: Arnd Bergmann 
> Cc: Geert Uytterhoeven 
> Cc: Phil Edworthy 
> Cc: Simon Horman 
> Cc: Wolfram Sang 
> Cc: linux-renesas-soc@vger.kernel.org
> To: linux-...@vger.kernel.org
> ---
> NOTE: I'm aware of https://patchwork.kernel.org/patch/9495895/ , but the
>   discussion seems to have gone way off, so I'm sending this as a
>   RFC. Any feedback on how to do this limiting properly would be nice.
> ---
>  drivers/pci/host/pcie-rcar.c | 28 
>  1 file changed, 28 insertions(+)

The issue solved by this patch was solved in a more generic way through
this series:

https://lists.linuxfoundation.org/pipermail/iommu/2018-July/028792.html

I will therefore drop this patch from the PCI patch queue.

Thanks,
Lorenzo

> diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c
> index c3eab0b95290..db2b16f40bc1 100644
> --- a/drivers/pci/host/pcie-rcar.c
> +++ b/drivers/pci/host/pcie-rcar.c
> @@ -1325,3 +1325,31 @@ static struct platform_driver rcar_pcie_driver = {
>   .probe = rcar_pcie_probe,
>  };
>  builtin_platform_driver(rcar_pcie_driver);
> +
> +static int rcar_pcie_pci_notifier(struct notifier_block *nb,
> + unsigned long action, void *data)
> +{
> + struct device *dev = data;
> +
> + switch (action) {
> + case BUS_NOTIFY_BOUND_DRIVER:
> + /* Force the DMA mask to lower 32-bits */
> + dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
> + break;
> + default:
> + return NOTIFY_DONE;
> + }
> +
> + return NOTIFY_OK;
> +}
> +
> +static struct notifier_block device_nb = {
> + .notifier_call = rcar_pcie_pci_notifier,
> +};
> +
> +static int __init register_rcar_pcie_pci_notifier(void)
> +{
> + return bus_register_notifier(_bus_type, _nb);
> +}
> +
> +arch_initcall(register_rcar_pcie_pci_notifier);
> -- 
> 2.16.2
> 

Re: [PATCH V2 4/5] PCI: rcar: Support runtime PM, link state L1 handling

[Lorenzo Pieralisi]

On Tue, Aug 21, 2018 at 08:58:38AM +, Phil Edworthy wrote:
> Hi Lorenzo,
> 
> On 20 August 2018 15:48 Lorenzo Pieralisi wrote:
> > On Mon, Aug 20, 2018 at 01:44:48PM +, Phil Edworthy wrote:
> > 
> > [...]
> > 
> > > However, both before and after this patch, the RP does not transition
> > > L1 when the endpoints change to L1.
> > > This patch only transitions the RP to L1 during accessing a card's
> > > config registers, if the RP is not in L1 link state and has received
> > > PM_ENTER_L1 DLLP (e.g. resume). After this, the hardware will handle
> > > the transition out of L1.
> > >
> > > The relevant part of the rcar manual says: "After a recovery to L0, if
> > > the device is in the Non-D0 state and PM_Enter_L1 DLLP is transmitted
> > > from the downstream device, software should confirm that hardware is
> > > in the L0 state (PMSR.PMSTATE = L0) and initiate the L1 transition
> > > sequence again (write 1 to PMCTLR.L1IATN). In this case, the sequence
> > > is: L0 ??? L1 ??? L0 recovery ??? L1 again."
> > 
> > Can you map these FSM steps to this patch code please ? I would like to
> > understand what Link state maps to which command written and when.
> I don't think I can because we are not initially entering L1. Looking at this
> again, I think this section of the manual only helps in indicating how to 
> detect we should have gone into L1 and how to poke the HW to initiate the
> transition to L1.
> 
> On system suspend, the EP sends PM_Enter_L1 DLLP and enters L1 state.

I am still struggling to understand what "EP enters L1 state" means. A
link in L1 means both ends of the link are in electrical idle, it is a
two-way handshake, see PCI express specifications 5.3.2.1 "Entry into
the L1 state".

> The rcar RP cannot enter L1 by HW alone, so is still in L0.

See above.

> The only way out of this from the PCIe spec FSM is for both EP and RP
> to enter the Recovery state.
> The patch simply detects that we should have gone into L1, and so initiates
> that state change, and the HW will then handle the transition from L1 to
> Recovery and then on to L0.

That I can understand, I reckon it is to "reset" the RP link state
machine to a "sane" state.

> > > I don't think the potential issue that Bjorn talked about can happen
> > > because the RP does go into L1. I could be wrong though...
> > 
> > I do not understand this paragraph, mind elaborating on it ?
> As rcar RP only supports D0 and D3hot/cold, (the manual says it supports
> D3cold, but I cannot see how if it doesn't support L2 or L3 states), if you
> force the link to D3, we can only be in L1 state.

D3 is a device state, not a link state. I still do not understand this
statement.

The link between RP and EP can enter L1 when all functions in the EP
are in a device state != D0 but, as I mentioned above, it is still
unclear what happens in this platform since I do not get what state
in the PCI spec 5.3.2.1 state machine the RP Link state machine is
in.

If we programme the device into any D-state and the device wants to
send a PME message _before_ we reset the RP state machine with the
procedure described in this thread, what happens ? Or, more explicitly,
what are in _HW_ the states of upstream and downstream link state
machines when the EP is put in, say, D1 ?

That's in short our question. I would be happy to get to the bottom
of this since it is an interesting issue we are facing, we need
HW details, I can apply Marek's patch but I would be happier if I get
the whole picture first.

Thanks,
Lorenzo

> 
> 
> > > The driver should also have a runtime-PM hook to transition to L1 on
> > > suspend in order to save power. However, that is somewhat separate to
> > > the problem the patch fixes.
> > 
> > Yes that's a separate patch.
> > 
> > Thanks for chiming in, let's try to get to the bottom of this thread.
> > 
> > Lorenzo
> 
> Thanks
> Phil

Re: [PATCH V2 4/5] PCI: rcar: Support runtime PM, link state L1 handling

[Lorenzo Pieralisi]

On Mon, Aug 20, 2018 at 01:44:48PM +, Phil Edworthy wrote:

[...]

> However, both before and after this patch, the RP does not transition L1
> when the endpoints change to L1.
> This patch only transitions the RP to L1 during accessing a card's
> config registers, if the RP is not in L1 link state and has received
> PM_ENTER_L1 DLLP (e.g. resume). After this, the hardware will handle
> the transition out of L1.
> 
> The relevant part of the rcar manual says: "After a recovery to L0, if
> the device is in the Non-D0 state and PM_Enter_L1 DLLP is transmitted
> from the downstream device, software should confirm that hardware is
> in the L0 state (PMSR.PMSTATE = L0) and initiate the L1 transition
> sequence again (write 1 to PMCTLR.L1IATN). In this case, the sequence
> is: L0 → L1 → L0 recovery → L1 again."

Can you map these FSM steps to this patch code please ? I would like
to understand what Link state maps to which command written and when.

> I don’t think the potential issue that Bjorn talked about can happen
> because the RP does go into L1. I could be wrong though...

I do not understand this paragraph, mind elaborating on it ?

> The driver should also have a runtime-PM hook to transition to L1 on 
> suspend in order to save power. However, that is somewhat separate
> to the problem the patch fixes.

Yes that's a separate patch.

Thanks for chiming in, let's try to get to the bottom of this thread.

Lorenzo

Re: [PATCH V2 4/5] PCI: rcar: Support runtime PM, link state L1 handling

[Lorenzo Pieralisi]

On Wed, Jun 13, 2018 at 12:25:59PM -0500, Bjorn Helgaas wrote:



> Putting this workaround in the config accessor makes sense to me
> because in this situation the endpoint thinks it's in L1 and it won't
> receive TLPs for config accesses.  Apparently forcing the RP to L1
> completes the L1 entry, and the RP correctly handles the "Exit from L1
> State" (sec 5.3.2.2) that's required when the RP needs to send a TLP
> to the endpoint.
> 
> I think there's still a potential issue if the endpoint goes to a
> non-D0 state, the link is stuck in this transitional state (endpoint
> thinks it's L1, RP thinks it's L0), and the *endpoint* wants to exit
> L1, e.g., so it can send a PME message for a wakeup.  I don't know
> what happens then.

What is not clear to me is whether the endpoint link state can really be
in electrical idle (so ready for L1) if it has not received a
PM_Request_Ack DLLP from the host controller.

It is not clear whether the host controller sends it upon PM_Enter_L1
DLLP reception (ie does it actually carry out step 9 in PCIe specs
5.3.2.1 "Entry into L1 state") or it has to be coerced into L1 to send
it.

I agree with Bjorn that this is not clear at all and it boils down to
how HW is designed. We need to understand what "Endpoint in L1, RP in
L0" means in this context and for that we need an errata document
otherwise that's impossible to untangle.

Lorenzo

> If there were a real erratum writeup for this, it would probably
> discuss this situation.
> 
> > > > > If that's the case, we shouldn't enable L1
> > > > > entry on a card.  Is the core ASPM code doing something wrong here?
> > > > 
> > > > I can double-check, am I looking for some particular register in the
> > > > PCIe space ?
> > > > 
> > > >  PM_ENTER_L1 DLLP to the host controller. At this point, we can no 
> > > >  longer
> > > >  access the card's config registers.
> > > > 
> > > >  The R-Car host controller will handle taking cards out of L1 as 
> > > >  long as
> > > >  the host controller has also been transitioned to L1 link state.
> > > > >>>
> > > > >>> I wonder why this can't be done in a PM restore hook but that's not
> > > > >>> really where my question is.
> > > > >>
> > > > >> I suspect because the link can be in L1 during startup too?
> > > > >>
> > > >  Ideally, we would detect the PM_ENTER_L1 DLLP using an interrupt 
> > > >  and
> > > >  transition the host to L1 immediately. However, this patch just 
> > > >  ensures
> > > >  that we can talk to cards after they have gone into L1.
> > > > >>>
> > > >  When attempting a config access, it checks to see if the card has 
> > > >  gone
> > > >  into L1, and if so, does the same for the host controller.
> > > > 
> > > >  This is based on a patch by Hien Dang 
> > > >  
> > > > 
> > > >  Signed-off-by: Phil Edworthy 
> > > >  Signed-off-by: Marek Vasut 
> > > >  Cc: Geert Uytterhoeven 
> > > >  Cc: Phil Edworthy 
> > > >  Cc: Simon Horman 
> > > >  Cc: Wolfram Sang 
> > > >  Cc: linux-renesas-soc@vger.kernel.org
> > > >  ---
> > > >  V2: - Drop extra parenthesis
> > > >  - Use GENMASK()
> > > >  - Fix comment "The HW will handle coming of of L1.", s/of 
> > > >  of/out of/
> > > >  ---
> > > >   drivers/pci/host/pcie-rcar.c | 24 
> > > >   1 file changed, 24 insertions(+)
> > > > 
> > > >  diff --git a/drivers/pci/host/pcie-rcar.c 
> > > >  b/drivers/pci/host/pcie-rcar.c
> > > >  index ab61829db389..068bf9067ec1 100644
> > > >  --- a/drivers/pci/host/pcie-rcar.c
> > > >  +++ b/drivers/pci/host/pcie-rcar.c
> > > >  @@ -92,6 +92,13 @@
> > > >   #define MACCTLR   0x011058
> > > >   #define  SPEED_CHANGE BIT(24)
> > > >   #define  SCRAMBLE_DISABLE BIT(27)
> > > >  +#define PMSR  0x01105c
> > > >  +#define  L1FAEG   BIT(31)
> > > >  +#define  PM_ENTER_L1RXBIT(23)
> > > >  +#define  PMSTATE  GENMASK(18, 16)
> > > >  +#define  PMSTATE_L1   GENMASK(17, 16)
> > > >  +#define PMCTLR0x011060
> > > >  +#define  L1_INIT  BIT(31)
> > > >   #define MACS2R0x011078
> > > >   #define MACCGSPSETR   0x011084
> > > >   #define  SPCNGRSN BIT(31)
> > > >  @@ -191,6 +198,7 @@ static int rcar_pcie_config_access(struct 
> > > >  rcar_pcie *pcie,
> > > > unsigned int devfn, int where, u32 *data)
> > > >   {
> > > > int dev, func, reg, index;
> > > >  +  u32 val;
> > > >   
> > > > dev = PCI_SLOT(devfn);
> > > > func = PCI_FUNC(devfn);
> > > >  @@ -232,6 +240,22 @@ static int rcar_pcie_config_access(struct 
> > > >  rcar_pcie *pcie,
> > > > if (pcie->root_bus_nr < 0)
> > > > return 

Re: [PATCH v2] drivers/firmware: psci_checker: stash and use topology_core_cpumask for hotplug tests

[Lorenzo Pieralisi]

On Thu, Jul 19, 2018 at 02:35:49PM +0100, Sudeep Holla wrote:
> Commit 7f9545aa1a91 ("arm64: smp: remove cpu and numa topology
> information when hotplugging out CPU") updates the cpu topology when
> the CPU is hotplugged out. However the PSCI checker code uses the
> topology_core_cpumask pointers for some of the cpu hotplug testing.
> Since the pointer to the core_cpumask of the first CPU in the group
> is used, which when that CPU itself is hotpugged out is just set to
> itself, the testing terminates after that particular CPU is tested out.
> But the intention of this tests is to cover all the CPU in the group.
> 
> In order to support that, we need to stash the topology_core_cpumask
> before the start of the test and use that value instead of pointer to
> a cpumask which will be updated on CPU hotplug.
> 
> Fixes: 7f9545aa1a91a9a4 ("arm64: smp: remove cpu and numa topology
>   information when hotplugging out CPU")
> Reported-by: Geert Uytterhoeven 
> Tested-by: Geert Uytterhoeven 
> Cc: Mark Rutland 
> Cc: Lorenzo Pieralisi 
> Signed-off-by: Sudeep Holla 
> ---
>  drivers/firmware/psci_checker.c | 53 
> -
>  1 file changed, 42 insertions(+), 11 deletions(-)
> 
> v1->v2:
>   - Move allocation and freeing of the cpumasks to separate
> routines as suggested by Lorenzo
>   - Reduced the allocation to number of groups instead of number
> of cpus in the system by making 2 pass
> 
> diff --git a/drivers/firmware/psci_checker.c b/drivers/firmware/psci_checker.c
> index bb1c068bff19..7e6f66b588fd 100644
> --- a/drivers/firmware/psci_checker.c
> +++ b/drivers/firmware/psci_checker.c
> @@ -77,21 +77,23 @@ static int psci_ops_check(void)
>   return 0;
>  }
> 
> -static int find_cpu_groups(const struct cpumask *cpus,
> -const struct cpumask **cpu_groups)
> +static int find_cpu_groups(cpumask_var_t *cpu_groups)
>  {
>   unsigned int nb = 0;
>   cpumask_var_t tmp;
> 
>   if (!alloc_cpumask_var(, GFP_KERNEL))
>   return -ENOMEM;
> - cpumask_copy(tmp, cpus);
> + cpumask_copy(tmp, cpu_online_mask);
> 
>   while (!cpumask_empty(tmp)) {
>   const struct cpumask *cpu_group =
>   topology_core_cpumask(cpumask_any(tmp));
> 
> - cpu_groups[nb++] = cpu_group;
> + if (cpu_groups && cpu_groups[nb])
> + cpumask_copy(cpu_groups[nb], cpu_group);
> +
> + nb++;
>   cpumask_andnot(tmp, tmp, cpu_group);
>   }
> 
> @@ -166,20 +168,48 @@ static unsigned int down_and_up_cpus(const struct 
> cpumask *cpus,
>   return err;
>  }
> 
> +static void free_cpu_groups(int num, cpumask_var_t *cpu_groups)
> +{
> + int i;
> +
> + for (i = 0; i < num; ++i)
> + free_cpumask_var(cpu_groups[i]);
> + kfree(cpu_groups);
> +}
> +
> +static cpumask_var_t *alloc_cpu_groups(int num)
> +{
> + int i;
> + cpumask_var_t *cpu_groups;
> +
> + cpu_groups = kcalloc(num, sizeof(cpu_groups), GFP_KERNEL);
> + if (!cpu_groups)
> + return NULL;
> +
> + for (i = 0; i < num; ++i)
> + if (!alloc_cpumask_var(_groups[i], GFP_KERNEL)) {
> + free_cpu_groups(num, cpu_groups);
> + return NULL;
> +         }
> +
> + return cpu_groups;
> +}

Sorry for being a PITA - I meant we could remove find_cpu_groups()
entirely and embed it in alloc_cpu_groups(), that takes a cpumask_t
pointer and return the number of groups, again, to make it more
readable but that's just my opinion.

Regardless:

Acked-by: Lorenzo Pieralisi 

>  static int hotplug_tests(void)
>  {
>   int err;
> - cpumask_var_t offlined_cpus;
> + cpumask_var_t offlined_cpus, *cpu_groups;
>   int i, nb_cpu_group;
> - const struct cpumask **cpu_groups;
>   char *page_buf;
> 
> + /* first run to just get the number of cpu groups */
> + nb_cpu_group = find_cpu_groups(NULL);
> +
>   err = -ENOMEM;
>   if (!alloc_cpumask_var(_cpus, GFP_KERNEL))
>   return err;
> - /* We may have up to nb_available_cpus cpu_groups. */
> - cpu_groups = kmalloc_array(nb_available_cpus, sizeof(*cpu_groups),
> -GFP_KERNEL);
> +
> + cpu_groups = alloc_cpu_groups(nb_cpu_group);
>   if (!cpu_groups)
>   goto out_free_cpus;
>   page_buf = (char *)__get_free_page(GFP_KERNEL);
> @@ -187,7 +217,8 @@ static int hotplug_tests(void)
>   goto out_free_cpu_groups;
> 
>   err = 0;
> 

Re: [PATCH] drivers/firmware: psci_checker: stash and use topology_core_cpumask for hotplug tests

[Lorenzo Pieralisi]

On Wed, Jul 18, 2018 at 12:25:32PM +0100, Sudeep Holla wrote:
> Commit 7f9545aa1a91 ("arm64: smp: remove cpu and numa topology
> information when hotplugging out CPU") updates the cpu topology when
> the CPU is hotplugged out. However the PSCI checker code uses the
> topology_core_cpumask pointers for some of the cpu hotplug testing.
> Since the pointer to the core_cpumask of the first CPU in the group
> is used, which when that CPU itself is hotpugged out is just set to
> itself, the testing terminates after that particular CPU is tested out.
> But the intention of this tests is to cover all the CPU in the group.
> 
> In order to support that, we need to stash the topology_core_cpumask
> before the start of the test and use that value instead of pointer to
> a cpumask which will be updated on CPU hotplug.
> 
> Fixes: 7f9545aa1a91a9a4 ("arm64: smp: remove cpu and numa topology
>   information when hotplugging out CPU")
> Reported-by: Geert Uytterhoeven 
> Tested-by: Geert Uytterhoeven 
> Cc: Mark Rutland 
> Cc: Lorenzo Pieralisi 
> Signed-off-by: Sudeep Holla 
> ---
>  drivers/firmware/psci_checker.c | 30 --
>  1 file changed, 20 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/firmware/psci_checker.c b/drivers/firmware/psci_checker.c
> index bb1c068bff19..e330c6cb45f5 100644
> --- a/drivers/firmware/psci_checker.c
> +++ b/drivers/firmware/psci_checker.c
> @@ -77,21 +77,23 @@ static int psci_ops_check(void)
>   return 0;
>  }
>  
> -static int find_cpu_groups(const struct cpumask *cpus,
> -const struct cpumask **cpu_groups)
> +static int find_cpu_groups(cpumask_var_t *cpu_groups)
>  {
>   unsigned int nb = 0;
>   cpumask_var_t tmp;
>  
>   if (!alloc_cpumask_var(, GFP_KERNEL))
>   return -ENOMEM;
> - cpumask_copy(tmp, cpus);
> + cpumask_copy(tmp, cpu_online_mask);
>  
>   while (!cpumask_empty(tmp)) {
>   const struct cpumask *cpu_group =
>   topology_core_cpumask(cpumask_any(tmp));
>  
> - cpu_groups[nb++] = cpu_group;
> + if (cpu_groups && cpu_groups[nb])
> + cpumask_copy(cpu_groups[nb], cpu_group);
> +
> + nb++;
>   cpumask_andnot(tmp, tmp, cpu_group);
>   }
>  
> @@ -169,25 +171,31 @@ static unsigned int down_and_up_cpus(const struct 
> cpumask *cpus,
>  static int hotplug_tests(void)
>  {
>   int err;
> - cpumask_var_t offlined_cpus;
> + cpumask_var_t offlined_cpus, *cpu_groups;
>   int i, nb_cpu_group;
> - const struct cpumask **cpu_groups;
>   char *page_buf;
>  
> + /* first run to just get the number of cpu groups */
> + nb_cpu_group = find_cpu_groups(NULL);
> +
>   err = -ENOMEM;
>   if (!alloc_cpumask_var(_cpus, GFP_KERNEL))
>   return err;
> - /* We may have up to nb_available_cpus cpu_groups. */
> - cpu_groups = kmalloc_array(nb_available_cpus, sizeof(*cpu_groups),
> -GFP_KERNEL);
> + cpu_groups = kcalloc(nb_cpu_group, sizeof(cpu_groups), GFP_KERNEL);
>   if (!cpu_groups)
>   goto out_free_cpus;
> +
> + for (i = 0; i < nb_cpu_group; ++i)
> + if (!alloc_cpumask_var(_groups[i], GFP_KERNEL))
> + goto out_free_cpu_groups;
> +
>   page_buf = (char *)__get_free_page(GFP_KERNEL);
>   if (!page_buf)
>   goto out_free_cpu_groups;
>  
>   err = 0;
> - nb_cpu_group = find_cpu_groups(cpu_online_mask, cpu_groups);
> + /* second run to populate/copy the cpumask */
> + nb_cpu_group = find_cpu_groups(cpu_groups);
>  
>   /*
>* Of course the last CPU cannot be powered down and cpu_down() should
> @@ -212,6 +220,8 @@ static int hotplug_tests(void)
>  
>   free_page((unsigned long)page_buf);
>  out_free_cpu_groups:
> + for (i = 0; i < nb_cpu_group; ++i)
> + free_cpumask_var(cpu_groups[i]);
>   kfree(cpu_groups);
>  out_free_cpus:
>   free_cpumask_var(offlined_cpus);

Hi Sudeep,

thanks for the patch. I reckon that adding two functions, say,
alloc_cpu_groups() and free_cpu_groups() would make the code
more readable instead of relying on find_cpu_groups() to first
count then copy; it is for readability rather than correctness.

Thanks,
Lorenzo

Re: [PATCH 0/8] Fix PCI I/O space page leaks

[Lorenzo Pieralisi]

On Sun, Jul 15, 2018 at 07:00:21PM +0300, Sergei Shtylyov wrote:
> Hello!
> 
> Here's a set of 8 patches against the 'pci/controller-fixes' branch of Lorenzo
> Pieralisi's 'pci.git' repo. They are the fixes for the PCI I/O space page 
> leaks
> (and the kernel BUG caused by them on deferred probe); those were 1st found
> testing the R-Car PCIe driver. The patches are in the chronological order
> (considering the date/time of the commits specified in the Fixes: tag), 
> patches
> #2..#8 depend on the patch #1 in order to build/work as it introduces the 
> managed
> device API that they all use...
> 
> [1/8] PCI: xgene: Fix I/O space page leak
> [2/8] PCI: of: Fix I/O space page leak
> [3/8] PCI: versatile: Fix I/O space page leak
> [4/8] PCI: designware: Fix I/O space page leak
> [5/8] PCI: aardvark: Fix I/O space page leak
> [6/8] PCI: faraday: Fix I/O space page leak
> [7/8] PCI: mediatek: Fix I/O space page leak
> [8/8] PCI: v3-semi: Fix I/O space page leak
> 
> MBR, Sergei

Hi Sergei,

I reshuffled the series, rewrote commit logs and move the devm_
API to commit 2 which is the one you tested AFAIK.

I pushed them out to my pci/controller-fixes branch which I hope
we can still aim for an -rc, please do have a look and let me know
what you think, I have not asked Bjorn to pull it yet.

Thanks,
Lorenzo

Re: [PATCH] pci: fix I/O space page leak

[Lorenzo Pieralisi]

On Mon, Jul 02, 2018 at 01:08:45PM +0200, Geert Uytterhoeven wrote:
> Hi Lorenzo,
> 
> On Mon, Jul 2, 2018 at 12:31 PM Lorenzo Pieralisi
>  wrote:
> > On Sat, Jun 30, 2018 at 01:37:18PM +0300, Sergei Shtylyov wrote:
> > > On 6/28/2018 5:26 PM, Lorenzo Pieralisi wrote:
> > > >>>When testing the R-Car PCIe driver on the Condor board, I noticed that 
> > > >>>iff
> > > >>>I  left the PCIe PHY driver disabled, the kernel crashed  with this 
> > > >>>BUG:
> > > >>>
> > > >>>[1.225819] kernel BUG at lib/ioremap.c:72!
> 
> > > >>>It turned out that pci_remap_iospace() wasn't undone when the driver's
> > > >>>probe failed, and since devm_phy_optional_get() returned -EPROBE_DEFER,
> > > >>>the probe was retried,  finally causing the BUG due to trying to remap
> > > >>>already remapped pages.
> > > >>>
> > > >>>The most feasible solution seems to introduce devm_pci_remap_iospace()
> > > >>>and call it instead of pci_remap_iospace(), so that the pages get 
> > > >>>unmapped
> > > >>>automagically on any probe failure.
> > > >>>
> > > >>>And  while fixing pci_parse_request_of_pci_ranges(), aslo fix the other
> > > >>>drivers that have probably copied the bad example...
> > > >>>
> > > >>>Fixes: 4e64dbe226e7 ("PCI: generic: Expose pci_host_common_probe() for 
> > > >>>use by other drivers")
> > > >>>Fixes: cbce7900598c ("PCI: designware: Make driver arch-agnostic")
> > > >>>Fixes: 8c39d710363c ("PCI: aardvark: Add Aardvark PCI host controller 
> > > >>>driver")
> > > >>>Fixes: d3c68e0a7e34 ("PCI: faraday: Add Faraday Technology FTPCI100 
> > > >>>PCI Host Bridge driver")
> > > >>>Fixes: 68a15eb7bd0c ("PCI: v3-semi: Add V3 Semiconductor PCI host 
> > > >>>driver")
> > > >>>Fixes: b7e78170efd4 ("PCI: versatile: Add DT-based ARM Versatile PB 
> > > >>>PCIe host driver")
> > > >>>Fixes: 5f6b6ccdbe1c ("PCI: xgene: Add APM X-Gene PCIe driver")
> > > >>>Fixes: 637cfacae96f ("PCI: mediatek: Add MediaTek PCIe host controller 
> > > >>>support")
> > > >>>Signed-off-by: Sergei Shtylyov 
> > > >>>Cc: sta...@vger.kernel.org
> > > >>
> > > >>Let me know if you want me to take this, Lorenzo, otherwise:
> > > >>s/pci: fix/PCI: Fix/ and
> > > >>
> > > >>Acked-by: Bjorn Helgaas 
> > > >
> > > >Thank you Bjorn, yes it could go in as a fix but IMO it has to be split,
> > > >more so given the stable tag (and I think that each "Fixes" tag should
> > > >be self-contained),
> > >
> > >It cannot be self-contained because it'll depend on the initial
> > > commit adding devm_pci_remap_iobase(). If you mean finding the
> > > earliest broken driver and introduce the deviec managed API while
> > > fixing it and then make use of that
> > > API in the subsequent patches, that surely can be done.
> >
> > Yes I think that's the best course of action.
> >
> > > >merging it as-is would give Greg (and us) a
> > > >headache when it comes to backporting it.
> > >
> > >The patch interdependency would give him headache too, and I was
> > > hoping to relieve those with the monilitic patch. :-)
> >
> > The problem is that if any of the fixes has to be reverted we have
> > to revert the whole thing instead of just the problematic patch,
> > which, given that we are sending this to stable kernels may easily
> > turn out quite complicated.
> >
> > So, I would add the new API along with the earliest broken driver
> > and mark it for stable.
> >
> > In the same thread, add all other fixes (one per patch) without the
> > stable tag. When the first fix gets merged into the mainline (and
> > consequently goes to stable) we can send the stable backports for the
> > remainder of fixes.
> >
> > How does that sound ?
> 
> If you want to be prepared for reverting, why not split off the first fix
> from the patch that provides the new API as well?
> Yes, it does mean a dependency for backporting, but it avoids the mess if
> it turns out the first fix is the (only) one that needs to be reverted.

Problem is that that would give Bjorn a hard time asking to merge the
new API as a fix as a standalone patch for an -rc, hence I suggested we
merged it for v4.19 but Sergei is not happy with that and I understand
his point.

To be honest I'd rather start with fixing RCAR (by "fixing" the
pci_parse_request_of_pci_ranges() call with the new devm_ API) without
any stable tag (with commit log reporting the RCAR regression), we will
work out how to fix and backport the rest of the drivers later.

I am not even sure this fix per-se is stable kernel material, that's
moot given that there exists an pci_unmap_iospace() API that could be
used in drivers before they were converted to
pci_parse_request_of_pci_ranges().

Please let me know if that's reasonable, thanks.

Lorenzo

Re: [PATCH] pci: fix I/O space page leak

[Lorenzo Pieralisi]

On Sat, Jun 30, 2018 at 01:37:18PM +0300, Sergei Shtylyov wrote:
> Hello!
> 
> On 6/28/2018 5:26 PM, Lorenzo Pieralisi wrote:
> 
> >>>When testing the R-Car PCIe driver on the Condor board, I noticed that iff
> >>>I  left the PCIe PHY driver disabled, the kernel crashed  with this BUG:
> >>>
> >>>[1.225819] kernel BUG at lib/ioremap.c:72!
> >>>[1.230007] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
> >>>[1.235496] Modules linked in:
> >>>[1.238561] CPU: 0 PID: 39 Comm: kworker/0:1 Not tainted 4.17.0-dirty 
> >>>#1092
> >>>[1.245526] Hardware name: Renesas Condor board based on r8a77980 (DT)
> >>>[1.252075] Workqueue: events deferred_probe_work_func
> >>>[1.257220] pstate: 8005 (Nzcv daif -PAN -UAO)
> >>>[1.262024] pc : ioremap_page_range+0x370/0x3c8
> >>>[1.266558] lr : ioremap_page_range+0x40/0x3c8
> >>>[1.271002] sp : 08da39e0
> >>>[1.274317] x29: 08da39e0 x28: 00e80f07
> >>>[1.279636] x27: 7dfffee0 x26: 0140
> >>>[1.284954] x25: 7dfffef0 x24: 000fe100
> >>>[1.290272] x23: 80007b906000 x22: 08ab8000
> >>>[1.295590] x21: 08bb1d58 x20: 7dfffef0
> >>>[1.300909] x19: 89c30fb8 x18: 0001
> >>>[1.306226] x17: 000152d0 x16: 014012d0
> >>>[1.311544] x15:  x14: 0720072007200720
> >>>[1.316862] x13: 0720072007200720 x12: 0720072007200720
> >>>[1.322180] x11: 0720072007300730 x10: 00ae
> >>>[1.327498] x9 :  x8 : 7d00
> >>>[1.332816] x7 :  x6 : 0100
> >>>[1.338134] x5 :  x4 : 7b906000
> >>>[1.343452] x3 : 80007c61a880 x2 : 7dfffeef
> >>>[1.348770] x1 : 4000 x0 : 00e8fe100f07
> >>>[1.354090] Process kworker/0:1 (pid: 39, stack limit = 0x
> >>>(ptrval))
> >>>[1.361056] Call trace:
> >>>[1.363504]  ioremap_page_range+0x370/0x3c8
> >>>[1.367695]  pci_remap_iospace+0x7c/0xac
> >>>[1.371624]  pci_parse_request_of_pci_ranges+0x13c/0x190
> >>>[1.376945]  rcar_pcie_probe+0x4c/0xb04
> >>>[1.380786]  platform_drv_probe+0x50/0xbc
> >>>[1.384799]  driver_probe_device+0x21c/0x308
> >>>[1.389072]  __device_attach_driver+0x98/0xc8
> >>>[1.393431]  bus_for_each_drv+0x54/0x94
> >>>[1.397269]  __device_attach+0xc4/0x12c
> >>>[1.401107]  device_initial_probe+0x10/0x18
> >>>[1.405292]  bus_probe_device+0x90/0x98
> >>>[1.409130]  deferred_probe_work_func+0xb0/0x150
> >>>[1.413756]  process_one_work+0x12c/0x29c
> >>>[1.417768]  worker_thread+0x200/0x3fc
> >>>[1.421522]  kthread+0x108/0x134
> >>>[1.424755]  ret_from_fork+0x10/0x18
> >>>[1.428334] Code: f9004ba2 5480 aa0003fb 1748 (d421)
> >>>
> >>>It turned out that pci_remap_iospace() wasn't undone when the driver's
> >>>probe failed, and since devm_phy_optional_get() returned -EPROBE_DEFER,
> >>>the probe was retried,  finally causing the BUG due to trying to remap
> >>>already remapped pages.
> >>>
> >>>The most feasible solution seems to introduce devm_pci_remap_iospace()
> >>>and call it instead of pci_remap_iospace(), so that the pages get unmapped
> >>>automagically on any probe failure.
> >>>
> >>>And  while fixing pci_parse_request_of_pci_ranges(), aslo fix the other
> >>>drivers that have probably copied the bad example...
> >>>
> >>>Fixes: 4e64dbe226e7 ("PCI: generic: Expose pci_host_common_probe() for use 
> >>>by other drivers")
> >>>Fixes: cbce7900598c ("PCI: designware: Make driver arch-agnostic")
> >>>Fixes: 8c39d710363c ("PCI: aardvark: Add Aardvark PCI host controller 
> >>>driver")
> >>>Fixes: d3c68e0a7e34 ("PCI: faraday: Add Faraday Technology FTPCI100 PCI 
> >>>Host Bridge driver")
> >>>Fixes: 68a15eb7bd0c ("PCI: v3-semi: Add V3 Semiconductor PCI host driver")
> >>>Fixes: b7e78170efd4 ("PCI: versatile: Add DT-based ARM Versatile PB PCIe 
> >>>host driver")
> >>>Fixes: 5f6b6ccdbe1c (&

Re: [PATCH] PCI: rcar: Clean up PHY init on failure

[Lorenzo Pieralisi]

On Fri, May 25, 2018 at 08:33:26PM +0200, Marek Vasut wrote:
> If the Gen3 PHY fails to power up, the code does not undo the
> initialization caused by phy_init(). Add the missing failure
> handling to the rcar_pcie_phy_init_gen3() function.
> 
> Signed-off-by: Marek Vasut 
> Reported-by: Geert Uytterhoeven 
> Cc: Geert Uytterhoeven 
> Cc: Lorenzo Pieralisi 
> Cc: Phil Edworthy 
> Cc: Simon Horman 
> Cc: Wolfram Sang 
> Cc: linux-renesas-soc@vger.kernel.org
> Fixes: 517ca93a7159 ("PCI: rcar: Add R-Car gen3 PHY support")
> ---
>  drivers/pci/host/pcie-rcar.c | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)

Applied to pci/controller-fixes to be tentatively merged for
-rc4, thanks.

Lorenzo

> diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c
> index 695781934f0a..477bf40cc031 100644
> --- a/drivers/pci/host/pcie-rcar.c
> +++ b/drivers/pci/host/pcie-rcar.c
> @@ -678,7 +678,11 @@ static int rcar_pcie_phy_init_gen3(struct rcar_pcie 
> *pcie)
>   if (err)
>   return err;
>  
> - return phy_power_on(pcie->phy);
> + err = phy_power_on(pcie->phy);
> + if (err)
> + phy_exit(pcie->phy);
> +
> + return err;
>  }
>  
>  static int rcar_msi_alloc(struct rcar_msi *chip)
> -- 
> 2.16.2
> 

Re: [PATCH v4 6/6] PCI: rcar: Shut the PHY down in failpath

[Lorenzo Pieralisi]

On Thu, May 24, 2018 at 04:36:24PM +0200, Marek Vasut wrote:
> If anything fails past phy_init_fn() and the system is a Gen3 with
> a PHY, the PHY will be left on and inited. This is caused by the
> phy_init_fn, which is in fact a pointer to rcar_pcie_phy_init_gen3()
> function, which starts the PHY, yet has no counterpart in the failpath.
> Add that counterpart.
> 
> Signed-off-by: Marek Vasut 
> Cc: Geert Uytterhoeven 
> Cc: Lorenzo Pieralisi 
> Cc: Phil Edworthy 
> Cc: Simon Horman 
> Cc: Wolfram Sang 
> Cc: linux-renesas-soc@vger.kernel.org
> Fixes: 517ca93a7159 ("PCI: rcar: Add R-Car gen3 PHY support")
> ---
> V4: New patch
> ---
>  drivers/pci/host/pcie-rcar.c | 10 --
>  1 file changed, 8 insertions(+), 2 deletions(-)

Applied to pci/controller-fixes to be tentatively merged at -rc4,
thanks.

Lorenzo

> diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c
> index 636c3c5095d2..695781934f0a 100644
> --- a/drivers/pci/host/pcie-rcar.c
> +++ b/drivers/pci/host/pcie-rcar.c
> @@ -1163,7 +1163,7 @@ static int rcar_pcie_probe(struct platform_device *pdev)
>   if (rcar_pcie_hw_init(pcie)) {
>   dev_info(dev, "PCIe link down\n");
>   err = -ENODEV;
> - goto err_clk_disable;
> + goto err_phy_shutdown;
>   }
>  
>   data = rcar_pci_read_reg(pcie, MACSR);
> @@ -1175,7 +1175,7 @@ static int rcar_pcie_probe(struct platform_device *pdev)
>   dev_err(dev,
>   "failed to enable MSI support: %d\n",
>   err);
> - goto err_clk_disable;
> + goto err_phy_shutdown;
>   }
>   }
>  
> @@ -1189,6 +1189,12 @@ static int rcar_pcie_probe(struct platform_device 
> *pdev)
>   if (IS_ENABLED(CONFIG_PCI_MSI))
>   rcar_pcie_teardown_msi(pcie);
>  
> +err_phy_shutdown:
> + if (pcie->phy) {
> + phy_power_off(pcie->phy);
> + phy_exit(pcie->phy);
> + }
> +
>  err_clk_disable:
>   clk_disable_unprepare(pcie->bus_clk);
>  
> -- 
> 2.16.2
> 

Re: [PATCH] pci: fix I/O space page leak

[Lorenzo Pieralisi]

On Thu, Jun 28, 2018 at 08:22:59AM -0500, Bjorn Helgaas wrote:
> On Wed, Jun 20, 2018 at 08:51:37PM +0300, Sergei Shtylyov wrote:
> > When testing the R-Car PCIe driver on the Condor board, I noticed that iff
> > I  left the PCIe PHY driver disabled, the kernel crashed  with this BUG:
> > 
> > [1.225819] kernel BUG at lib/ioremap.c:72!
> > [1.230007] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
> > [1.235496] Modules linked in:
> > [1.238561] CPU: 0 PID: 39 Comm: kworker/0:1 Not tainted 4.17.0-dirty 
> > #1092
> > [1.245526] Hardware name: Renesas Condor board based on r8a77980 (DT)
> > [1.252075] Workqueue: events deferred_probe_work_func
> > [1.257220] pstate: 8005 (Nzcv daif -PAN -UAO)
> > [1.262024] pc : ioremap_page_range+0x370/0x3c8
> > [1.266558] lr : ioremap_page_range+0x40/0x3c8
> > [1.271002] sp : 08da39e0
> > [1.274317] x29: 08da39e0 x28: 00e80f07
> > [1.279636] x27: 7dfffee0 x26: 0140
> > [1.284954] x25: 7dfffef0 x24: 000fe100
> > [1.290272] x23: 80007b906000 x22: 08ab8000
> > [1.295590] x21: 08bb1d58 x20: 7dfffef0
> > [1.300909] x19: 89c30fb8 x18: 0001
> > [1.306226] x17: 000152d0 x16: 014012d0
> > [1.311544] x15:  x14: 0720072007200720
> > [1.316862] x13: 0720072007200720 x12: 0720072007200720
> > [1.322180] x11: 0720072007300730 x10: 00ae
> > [1.327498] x9 :  x8 : 7d00  
> > 
> > [1.332816] x7 :  x6 : 0100
> > [1.338134] x5 :  x4 : 7b906000
> > [1.343452] x3 : 80007c61a880 x2 : 7dfffeef
> > [1.348770] x1 : 4000 x0 : 00e8fe100f07
> > [1.354090] Process kworker/0:1 (pid: 39, stack limit = 0x
> > (ptrval))  
> > [1.361056] Call trace:
> > [1.363504]  ioremap_page_range+0x370/0x3c8
> > [1.367695]  pci_remap_iospace+0x7c/0xac
> > [1.371624]  pci_parse_request_of_pci_ranges+0x13c/0x190
> > [1.376945]  rcar_pcie_probe+0x4c/0xb04
> > [1.380786]  platform_drv_probe+0x50/0xbc
> > [1.384799]  driver_probe_device+0x21c/0x308
> > [1.389072]  __device_attach_driver+0x98/0xc8
> > [1.393431]  bus_for_each_drv+0x54/0x94
> > [1.397269]  __device_attach+0xc4/0x12c
> > [1.401107]  device_initial_probe+0x10/0x18
> > [1.405292]  bus_probe_device+0x90/0x98
> > [1.409130]  deferred_probe_work_func+0xb0/0x150
> > [1.413756]  process_one_work+0x12c/0x29c
> > [1.417768]  worker_thread+0x200/0x3fc
> > [1.421522]  kthread+0x108/0x134
> > [1.424755]  ret_from_fork+0x10/0x18
> > [1.428334] Code: f9004ba2 5480 aa0003fb 1748 (d421)
> > 
> > It turned out that pci_remap_iospace() wasn't undone when the driver's
> > probe failed, and since devm_phy_optional_get() returned -EPROBE_DEFER,
> > the probe was retried,  finally causing the BUG due to trying to remap
> > already remapped pages.
> > 
> > The most feasible solution seems to introduce devm_pci_remap_iospace()
> > and call it instead of pci_remap_iospace(), so that the pages get unmapped
> > automagically on any probe failure.
> > 
> > And  while fixing pci_parse_request_of_pci_ranges(), aslo fix the other
> > drivers that have probably copied the bad example...
> > 
> > Fixes: 4e64dbe226e7 ("PCI: generic: Expose pci_host_common_probe() for use 
> > by other drivers")
> > Fixes: cbce7900598c ("PCI: designware: Make driver arch-agnostic")
> > Fixes: 8c39d710363c ("PCI: aardvark: Add Aardvark PCI host controller 
> > driver")
> > Fixes: d3c68e0a7e34 ("PCI: faraday: Add Faraday Technology FTPCI100 PCI 
> > Host Bridge driver")
> > Fixes: 68a15eb7bd0c ("PCI: v3-semi: Add V3 Semiconductor PCI host driver")
> > Fixes: b7e78170efd4 ("PCI: versatile: Add DT-based ARM Versatile PB PCIe 
> > host driver")
> > Fixes: 5f6b6ccdbe1c ("PCI: xgene: Add APM X-Gene PCIe driver")
> > Fixes: 637cfacae96f ("PCI: mediatek: Add MediaTek PCIe host controller 
> > support")
> > Signed-off-by: Sergei Shtylyov 
> > Cc: sta...@vger.kernel.org
> 
> Let me know if you want me to take this, Lorenzo, otherwise:
> s/pci: fix/PCI: Fix/ and
> 
> Acked-by: Bjorn Helgaas 

Thank you Bjorn, yes it could go in as a fix but IMO it has to be split,
more so given the stable tag (and I think that each "Fixes" tag should
be self-contained), merging it as-is would give Greg (and us) a
headache when it comes to backporting it.

Honestly I think it is best to split it up and send it for v4.19 but
I am happy to hear other options.

Lorenzo

> > ---
> > The patch is against the 'master' branch of Bjorn Helgaas' 'pci.git' repo...
> > It  has only been tested with the R-Car PCIe driver...
> > 
> >  drivers/pci/controller/dwc/pcie-designware-host.c |3 +
> >  drivers/pci/controller/pci-aardvark.c |2 -
> >  

Re: [PATCH V2 4/5] PCI: rcar: Support runtime PM, link state L1 handling

[Lorenzo Pieralisi]

On Wed, Jun 13, 2018 at 12:25:59PM -0500, Bjorn Helgaas wrote:
> On Wed, Jun 13, 2018 at 04:52:52PM +0100, Lorenzo Pieralisi wrote:
> > On Wed, Jun 13, 2018 at 08:53:08AM -0500, Bjorn Helgaas wrote:
> > > On Wed, Jun 13, 2018 at 01:54:51AM +0200, Marek Vasut wrote:
> > > > On 06/11/2018 03:59 PM, Bjorn Helgaas wrote:
> > > > > On Sun, Jun 10, 2018 at 03:57:10PM +0200, Marek Vasut wrote:
> > > > >> On 11/17/2017 06:49 PM, Lorenzo Pieralisi wrote:
> > > > >>> On Fri, Nov 10, 2017 at 10:58:42PM +0100, Marek Vasut wrote:
> > > > >>>> From: Phil Edworthy 
> > > > >>>>
> > > > >>>> Most PCIe host controllers support L0s and L1 power states via 
> > > > >>>> ASPM.
> > > > >>>> The R-Car hardware only supports L0s, so when the system suspends 
> > > > >>>> and
> > > > >>>> resumes we have to manually handle L1.
> > > > >>>> When the system suspends, cards can put themselves into L1 and 
> > > > >>>> send a
> > > > >>>
> > > > >>> I assumed L1 entry has to be negotiated depending upon the PCIe
> > > > >>> hierarchy capabilities, I would appreciate if you can explain to
> > > > >>> me what's the root cause of the issue please.
> > > > >>
> > > > >> You should probably ignore the suspend/resume part altogether. The 
> > > > >> issue
> > > > >> here is that the cards can enter L1 state, while the controller 
> > > > >> won't do
> > > > >> that automatically, it can only detect that the link went into L1 
> > > > >> state.
> > > > >> If that happens,the driver must manually put the controller to L1 
> > > > >> state.
> > > > >> The controller can transition out of L1 state automatically though.
> > > > > 
> > > > > From earlier discussion I thought the R-Car root port did not
> > > > > advertise L1 support.
> > > > 
> > > > Which discussion ? This one or somewhere else ?
> > > 
> > > https://lkml.kernel.org/r/hk2pr0601mb1393d917d343e6363484ca68f5...@hk2pr0601mb1393.apcprd06.prod.outlook.com
> > > 
> > > Re-reading that, I think I see my misunderstanding.  I was only
> > > considering L1 in the ASPM context.  I didn't realize the L1
> > > implications of devices being in states other than D0.
> > > 
> > > Obviously L1 support for ASPM is optional and advertised via Link
> > > Capabilities.  But per PCIe r4.0, sec 5.2, L1 support is required for
> > > PCI-PM compatible power management, and is entered "whenever all
> > > Functions ... are programmed to a D-state other than D0."
> > > 
> > > So I guess this means *every* device is supposed to support L1 when it
> > > is in a non-D0 power state.  I think *this* is the case you're
> > > solving.
> > > 
> > > A little more of this detail, e.g., that this issue has nothing to do
> > > with ASPM, it's probably an R-Car erratum that the RC can't transition
> > > from L1 to L0, etc., in the changelog would really help clear things
> > > up for me.
> > 
> > I think that the issue is related to the L0->L1 transition upon system
> > suspend (ie the kernel must force the controller into L1 when all
> > devices are in a sleep state) and for this specific reason I still think
> > that checking for a PM_Enter_L1 DLLP reception and doing the L0->L1
> > transition within a config access is wrong and prone to error (what's
> > the rationale behind that ?), this ought to be done using PM methods in
> > the host controller driver.
> 
> But doesn't the problem happen whenever the link goes to L1, for any
> reason?  E.g., runtime power management might put an endpoint in D3
> even if we're not doing a whole system suspend.  A user could even
> force the endpoint to D3 by writing to PCI_PM_CTRL with "setpci".  If
> that's the case, I don't think the host controller PM methods will be
> enough to work around the issue.

By PM methods I included runtime PM (and related device dependencies)
but you are right there, I missed some use cases (which are not
necessarily the most common but we have to cope with them anyway).

> The comment in the patch ("If we are not in L1 link state and we have
> received PM_ENTER_L1 DLLP, transition to L1 link state") suggests that
> the R-C

Re: [PATCH V2 4/5] PCI: rcar: Support runtime PM, link state L1 handling

[Lorenzo Pieralisi]

On Wed, Jun 13, 2018 at 08:53:08AM -0500, Bjorn Helgaas wrote:
> On Wed, Jun 13, 2018 at 01:54:51AM +0200, Marek Vasut wrote:
> > On 06/11/2018 03:59 PM, Bjorn Helgaas wrote:
> > > On Sun, Jun 10, 2018 at 03:57:10PM +0200, Marek Vasut wrote:
> > >> On 11/17/2017 06:49 PM, Lorenzo Pieralisi wrote:
> > >>> On Fri, Nov 10, 2017 at 10:58:42PM +0100, Marek Vasut wrote:
> > >>>> From: Phil Edworthy 
> > >>>>
> > >>>> Most PCIe host controllers support L0s and L1 power states via ASPM.
> > >>>> The R-Car hardware only supports L0s, so when the system suspends and
> > >>>> resumes we have to manually handle L1.
> > >>>> When the system suspends, cards can put themselves into L1 and send a
> > >>>
> > >>> I assumed L1 entry has to be negotiated depending upon the PCIe
> > >>> hierarchy capabilities, I would appreciate if you can explain to
> > >>> me what's the root cause of the issue please.
> > >>
> > >> You should probably ignore the suspend/resume part altogether. The issue
> > >> here is that the cards can enter L1 state, while the controller won't do
> > >> that automatically, it can only detect that the link went into L1 state.
> > >> If that happens,the driver must manually put the controller to L1 state.
> > >> The controller can transition out of L1 state automatically though.
> > > 
> > > From earlier discussion I thought the R-Car root port did not
> > > advertise L1 support.
> > 
> > Which discussion ? This one or somewhere else ?
> 
> https://lkml.kernel.org/r/hk2pr0601mb1393d917d343e6363484ca68f5...@hk2pr0601mb1393.apcprd06.prod.outlook.com
> 
> Re-reading that, I think I see my misunderstanding.  I was only
> considering L1 in the ASPM context.  I didn't realize the L1
> implications of devices being in states other than D0.
> 
> Obviously L1 support for ASPM is optional and advertised via Link
> Capabilities.  But per PCIe r4.0, sec 5.2, L1 support is required for
> PCI-PM compatible power management, and is entered "whenever all
> Functions ... are programmed to a D-state other than D0."
> 
> So I guess this means *every* device is supposed to support L1 when it
> is in a non-D0 power state.  I think *this* is the case you're
> solving.
> 
> A little more of this detail, e.g., that this issue has nothing to do
> with ASPM, it's probably an R-Car erratum that the RC can't transition
> from L1 to L0, etc., in the changelog would really help clear things
> up for me.

I think that the issue is related to the L0->L1 transition upon system
suspend (ie the kernel must force the controller into L1 when all
devices are in a sleep state) and for this specific reason I still think
that checking for a PM_Enter_L1 DLLP reception and doing the L0->L1
transition within a config access is wrong and prone to error (what's
the rationale behind that ?), this ought to be done using PM methods in
the host controller driver.

And yes, adding more details to the commit log would help everybody
understand where the problem lies.

Thanks,
Lorenzo

> > > If that's the case, we shouldn't enable L1
> > > entry on a card.  Is the core ASPM code doing something wrong here?
> > 
> > I can double-check, am I looking for some particular register in the
> > PCIe space ?
> > 
> > >>>> PM_ENTER_L1 DLLP to the host controller. At this point, we can no 
> > >>>> longer
> > >>>> access the card's config registers.
> > >>>>
> > >>>> The R-Car host controller will handle taking cards out of L1 as long as
> > >>>> the host controller has also been transitioned to L1 link state.
> > >>>
> > >>> I wonder why this can't be done in a PM restore hook but that's not
> > >>> really where my question is.
> > >>
> > >> I suspect because the link can be in L1 during startup too?
> > >>
> > >>>> Ideally, we would detect the PM_ENTER_L1 DLLP using an interrupt and
> > >>>> transition the host to L1 immediately. However, this patch just ensures
> > >>>> that we can talk to cards after they have gone into L1.
> > >>>
> > >>>> When attempting a config access, it checks to see if the card has gone
> > >>>> into L1, and if so, does the same for the host controller.
> > >>>>
> > >>>> This is based on a patch by Hien Dang 
> > >>>>
> > >>>> Signed-of

Re: [PATCH v4 0/6] PCI: rcar: Failpath fixes

[Lorenzo Pieralisi]

On Thu, May 24, 2018 at 04:36:18PM +0200, Marek Vasut wrote:
> Multiple minor failpath fixes for the R-Car PCIe driver.
> 
> V4: Sync up the version numbers
> Rebase on top of Lorenzo's tree
> Add new patch fixing bug in the PHY code
> 
> Marek Vasut (6):
>   PCI: rcar: Pull bus clock enable/disable from
> rcar_pcie_get_resources()
>   PCI: rcar: Add missing irq_dispose_mapping() into failpath
>   PCI: rcar: Teardown MSI setup if rcar_pcie_enable() fails
>   PCI: rcar: Poll more often in rcar_pcie_wait_for_dl()
>   PCI: rcar: Remove IRQ mappings in rcar_pcie_enable_msi failpath
>   PCI: rcar: Shut the PHY down in failpath
> 
>  drivers/pci/host/pcie-rcar.c | 82 
> +++-
>  1 file changed, 66 insertions(+), 16 deletions(-)

Marek,

I have applied patches [1-5] to pci/rcar for v4.18, I could not
apply patch 6 since it is missing Simon's ACK (please have a look
at what I queued to check it is OK).

Bjorn will be merging the branch into -next shortly, I do not know if
there is room for adding additional patches on top of it, as I mentioned
on linux-pci I will be offline for two weeks so it is possible that
what's in pci/rcar at present will be what gets into v4.18, apologies,
that's all I can do.

Lorenzo

> Signed-off-by: Marek Vasut <marek.vasut+rene...@gmail.com>
> Cc: Geert Uytterhoeven <geert+rene...@glider.be>
> Cc: Lorenzo Pieralisi <lorenzo.pieral...@arm.com>
> Cc: Phil Edworthy <phil.edwor...@renesas.com>
> Cc: Simon Horman <horms+rene...@verge.net.au>
> Cc: Wolfram Sang <w...@the-dreams.de>
> Cc: linux-renesas-soc@vger.kernel.org
> 
> -- 
> 2.16.2
> 

Re: [PATCH v4 0/6] PCI: rcar: Failpath fixes

[Lorenzo Pieralisi]

On Fri, May 25, 2018 at 11:39:08AM +0200, Geert Uytterhoeven wrote:
> Hi Lorenzo,
> 
> On Fri, May 25, 2018 at 11:35 AM, Lorenzo Pieralisi
> <lorenzo.pieral...@arm.com> wrote:
> > Can I retain your review tags on this series so that I can queue
> > the patches ? I already added them apart from the last patch that
> > is new, please let me know asap.
> 
> Yes, please.

I assume that's valid also for the last patch, I need Simon's ACK
to proceed too.

Lorenzo

Re: [PATCH v4 0/6] PCI: rcar: Failpath fixes

[Lorenzo Pieralisi]

Geert, Simon,

Can I retain your review tags on this series so that I can queue
the patches ? I already added them apart from the last patch that
is new, please let me know asap.

Thanks,
Lorenzo

On Thu, May 24, 2018 at 04:36:18PM +0200, Marek Vasut wrote:
> Multiple minor failpath fixes for the R-Car PCIe driver.
> 
> V4: Sync up the version numbers
> Rebase on top of Lorenzo's tree
> Add new patch fixing bug in the PHY code
> 
> Marek Vasut (6):
>   PCI: rcar: Pull bus clock enable/disable from
> rcar_pcie_get_resources()
>   PCI: rcar: Add missing irq_dispose_mapping() into failpath
>   PCI: rcar: Teardown MSI setup if rcar_pcie_enable() fails
>   PCI: rcar: Poll more often in rcar_pcie_wait_for_dl()
>   PCI: rcar: Remove IRQ mappings in rcar_pcie_enable_msi failpath
>   PCI: rcar: Shut the PHY down in failpath
> 
>  drivers/pci/host/pcie-rcar.c | 82 
> +++-
>  1 file changed, 66 insertions(+), 16 deletions(-)
> 
> Signed-off-by: Marek Vasut <marek.vasut+rene...@gmail.com>
> Cc: Geert Uytterhoeven <geert+rene...@glider.be>
> Cc: Lorenzo Pieralisi <lorenzo.pieral...@arm.com>
> Cc: Phil Edworthy <phil.edwor...@renesas.com>
> Cc: Simon Horman <horms+rene...@verge.net.au>
> Cc: Wolfram Sang <w...@the-dreams.de>
> Cc: linux-renesas-soc@vger.kernel.org
> 
> -- 
> 2.16.2
> 

Re: [PATCH V3] PCI: rcar: Poll more often in rcar_pcie_wait_for_dl()

[Lorenzo Pieralisi]

On Tue, May 22, 2018 at 02:24:20PM +0200, Marek Vasut wrote:
> The data link active signal usually takes ~20 uSec to be asserted,
> poll the bit more often to avoid useless delays in this function.
> Use udelay() instead of usleep() for such a small delay as suggested
> by the timer documentation and because this will be used in atomic
> context later on when the suspend/resume patches land.
> 
> Signed-off-by: Marek Vasut 
> Cc: Geert Uytterhoeven 
> Cc: Phil Edworthy 
> Cc: Simon Horman 
> Cc: Wolfram Sang 
> Cc: linux-renesas-soc@vger.kernel.org
> ---
> V2: s/content/context in commit message
> V3: Add cpu_relax()
> ---
>  drivers/pci/host/pcie-rcar.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)

Applied to pci/rcar for v4.18, thanks.

Lorenzo

> diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c
> index 5c365f743df5..3aa6fe5f2d64 100644
> --- a/drivers/pci/host/pcie-rcar.c
> +++ b/drivers/pci/host/pcie-rcar.c
> @@ -529,13 +529,14 @@ static void phy_write_reg(struct rcar_pcie *pcie,
>  
>  static int rcar_pcie_wait_for_dl(struct rcar_pcie *pcie)
>  {
> - unsigned int timeout = 10;
> + unsigned int timeout = 1;
>  
>   while (timeout--) {
>   if ((rcar_pci_read_reg(pcie, PCIETSTR) & DATA_LINK_ACTIVE))
>   return 0;
>  
> - msleep(5);
> + udelay(5);
> + cpu_relax();
>   }
>  
>   return -ETIMEDOUT;
> -- 
> 2.16.2
> 

Re: [PATCH 1/4] PCI: rcar: Rename rcar_pcie_parse_request_of_pci_ranges()

[Lorenzo Pieralisi]

On Thu, May 24, 2018 at 09:24:27AM +0200, Marek Vasut wrote:
> On 05/23/2018 11:56 PM, Bjorn Helgaas wrote:
> > On Wed, May 23, 2018 at 07:05:06PM +0200, Marek Vasut wrote:
> >> On 05/23/2018 06:17 PM, Lorenzo Pieralisi wrote:
> >>> On Mon, May 21, 2018 at 03:11:20PM +0200, Marek Vasut wrote:
> >>>> The function name is just too confusing, rename it, no functional change.
> >>>> Rename the function to rcar_pcie_alloc_and_parse_pci_resource_list() as
> >>>> it's matching failpath function is pci_free_resource_list() so the names
> >>>> align much better and the new name also describes what the function does
> >>>> much better.
> >>>>
> >>>> Signed-off-by: Marek Vasut <marek.vasut+rene...@gmail.com>
> >>>> Cc: Geert Uytterhoeven <geert+rene...@glider.be>
> >>>> Cc: Phil Edworthy <phil.edwor...@renesas.com>
> >>>> Cc: Simon Horman <horms+rene...@verge.net.au>
> >>>> Cc: Wolfram Sang <w...@the-dreams.de>
> >>>> Cc: linux-renesas-soc@vger.kernel.org
> >>>> ---
> >>>>  drivers/pci/host/pcie-rcar.c | 4 ++--
> >>>>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>>
> >>> Can you rebase this series against my pci/rcar branch please ?
> >>>
> >>> I will merge it then, thanks.
> >>
> >> Where is that tree/branch located ?
> >>
> >> It applies fine on current next 20180517, is there a problem ?
> > https://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/pci.git/log/?h=pci/rcar
> 
> OK, 1/4 is dropped, the remaining patches are resubmitted. I added one
> more (6/6) since the phy patches in that tree added a bug into the fail
> path, so I fixed that too.

Please CC me if you want me to merge your code, I monitor linux-pci
but that would help me, thanks.

You did not add the review tags you received to the new series (also a
cover letter - however minimal - and a version number would help),
please add them and post a v2 so that I can merge them, I can do that
for you but it is good practice for the future.

Thanks,
Lorenzo

> > I don't think next 20180517 includes Lorenzo's pci/rcar branch, so
> > there might be conflicts.  I think Stephen is on vacation until next
> > week, so there isn't a newer -next tree yet.
> 
> OK
> 
> -- 
> Best regards,
> Marek Vasut

Re: [PATCH 1/4] PCI: rcar: Rename rcar_pcie_parse_request_of_pci_ranges()

[Lorenzo Pieralisi]

On Mon, May 21, 2018 at 03:11:20PM +0200, Marek Vasut wrote:
> The function name is just too confusing, rename it, no functional change.
> Rename the function to rcar_pcie_alloc_and_parse_pci_resource_list() as
> it's matching failpath function is pci_free_resource_list() so the names
> align much better and the new name also describes what the function does
> much better.
> 
> Signed-off-by: Marek Vasut 
> Cc: Geert Uytterhoeven 
> Cc: Phil Edworthy 
> Cc: Simon Horman 
> Cc: Wolfram Sang 
> Cc: linux-renesas-soc@vger.kernel.org
> ---
>  drivers/pci/host/pcie-rcar.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Can you rebase this series against my pci/rcar branch please ?

I will merge it then, thanks.

Lorenzo

> diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c
> index e403c5206b24..dbc80e457f95 100644
> --- a/drivers/pci/host/pcie-rcar.c
> +++ b/drivers/pci/host/pcie-rcar.c
> @@ -1051,7 +1051,7 @@ static const struct of_device_id rcar_pcie_of_match[] = 
> {
>   {},
>  };
>  
> -static int rcar_pcie_parse_request_of_pci_ranges(struct rcar_pcie *pci)
> +static int rcar_pcie_alloc_and_parse_pci_resource_list(struct rcar_pcie *pci)
>  {
>   int err;
>   struct device *dev = pci->dev;
> @@ -1108,7 +1108,7 @@ static int rcar_pcie_probe(struct platform_device *pdev)
>  
>   INIT_LIST_HEAD(>resources);
>  
> - err = rcar_pcie_parse_request_of_pci_ranges(pcie);
> + err = rcar_pcie_alloc_and_parse_pci_resource_list(pcie);
>   if (err)
>   goto err_free_bridge;
>  
> -- 
> 2.16.2
> 

Re: [PATCH V5] PCI: rcar: Use runtime PM to control controller clock

[Lorenzo Pieralisi]

On Mon, May 21, 2018 at 01:08:36PM +0200, Marek Vasut wrote:
> On 05/14/2018 05:49 PM, Lorenzo Pieralisi wrote:
> > On Mon, May 14, 2018 at 05:32:04PM +0200, Marek Vasut wrote:
> >> On 05/01/2018 12:55 PM, Lorenzo Pieralisi wrote:
> >>> On Fri, Apr 13, 2018 at 02:48:19PM +0200, Simon Horman wrote:
> >>>> On Tue, Apr 10, 2018 at 06:17:04PM +0200, Marek Vasut wrote:
> >>>>> On 04/10/2018 05:28 PM, Geert Uytterhoeven wrote:
> >>>>
> >>>> ...
> >>>>
> >>>>>>>> rcar_pcie_get_resources() is called while the device is
> >>>>>>>> runtime-enabled/resumed,
> >>>>>>>> pci_free_resource_list() is called while the device is 
> >>>>>>>> runtime-disabled.
> >>>>>
> >>>>> rcar_pcie_get_resources() is NOT a pair function for
> >>>>> pci_free_resource_list() . rcar_pcie_parse_request_of_pci_ranges() is a
> >>>>> pair function for pci_free_resource_list().
> >>>>>
> >>>>> rcar_pcie_parse_request_of_pci_ranges() calls
> >>>>> of_pci_get_host_bridge_resources() internally, so every single function
> >>>>> called after successful call of rcar_pcie_parse_request_of_pci_ranges()
> >>>>> must call pci_free_resource_list().
> >>>>>
> >>>>> Both of_pci_get_host_bridge_resources() and pci_free_resource_list() are
> >>>>> called with runtime PM disabled.
> >>>>>
> >>>>> The naming of the functions is confusing though.
> >>>>
> >>>> Hi,
> >>>>
> >>>> thanks everyone for their efforts in preparing/reviewing this patch.
> >>>>
> >>>> It seems there are some differences of opinion on how best to handle the
> >>>> error paths but unlike earlier versions this one seems correct to me. If
> >>>> that turns out to be false we can address it. But I don't think its 
> >>>> likely
> >>>> things will be enhanced by continuing this review.
> >>>>
> >>>> Lorenzo, please consider taking this patch in its current form.
> >>>>
> >>>> Reviewed-by: Simon Horman <horms+rene...@verge.net.au>
> >>>
> >>> Applied to pci/rcar for v4.18, thanks.
> >>
> >> Is there any reason why this patch isnt in next yet ?
> > 
> > Bjorn will merge it into -next this week.
> 
> Seems this got missed last week ?

It will go into -next as soon as a new -next release will
be available:

https://marc.info/?l=linux-next=152654084627146=2

Lorenzo

Re: [PATCH V5] PCI: rcar: Use runtime PM to control controller clock

[Lorenzo Pieralisi]

On Mon, May 14, 2018 at 05:32:04PM +0200, Marek Vasut wrote:
> On 05/01/2018 12:55 PM, Lorenzo Pieralisi wrote:
> > On Fri, Apr 13, 2018 at 02:48:19PM +0200, Simon Horman wrote:
> >> On Tue, Apr 10, 2018 at 06:17:04PM +0200, Marek Vasut wrote:
> >>> On 04/10/2018 05:28 PM, Geert Uytterhoeven wrote:
> >>
> >> ...
> >>
> >>>>>> rcar_pcie_get_resources() is called while the device is
> >>>>>> runtime-enabled/resumed,
> >>>>>> pci_free_resource_list() is called while the device is 
> >>>>>> runtime-disabled.
> >>>
> >>> rcar_pcie_get_resources() is NOT a pair function for
> >>> pci_free_resource_list() . rcar_pcie_parse_request_of_pci_ranges() is a
> >>> pair function for pci_free_resource_list().
> >>>
> >>> rcar_pcie_parse_request_of_pci_ranges() calls
> >>> of_pci_get_host_bridge_resources() internally, so every single function
> >>> called after successful call of rcar_pcie_parse_request_of_pci_ranges()
> >>> must call pci_free_resource_list().
> >>>
> >>> Both of_pci_get_host_bridge_resources() and pci_free_resource_list() are
> >>> called with runtime PM disabled.
> >>>
> >>> The naming of the functions is confusing though.
> >>
> >> Hi,
> >>
> >> thanks everyone for their efforts in preparing/reviewing this patch.
> >>
> >> It seems there are some differences of opinion on how best to handle the
> >> error paths but unlike earlier versions this one seems correct to me. If
> >> that turns out to be false we can address it. But I don't think its likely
> >> things will be enhanced by continuing this review.
> >>
> >> Lorenzo, please consider taking this patch in its current form.
> >>
> >> Reviewed-by: Simon Horman <horms+rene...@verge.net.au>
> > 
> > Applied to pci/rcar for v4.18, thanks.
> 
> Is there any reason why this patch isnt in next yet ?

Bjorn will merge it into -next this week.

Thanks,
Lorenzo

Re: [PATCH] PCI: rcar: Reuse generic pci_parse_request_of_pci_ranges() function

[Lorenzo Pieralisi]

On Wed, Apr 25, 2018 at 06:21:25PM +0300, Vladimir Zapolskiy wrote:
> The non-functional change removes a custom function to parse and
> allocate PCI resources in favour of pci_parse_request_of_pci_ranges().
> 
> Signed-off-by: Vladimir Zapolskiy 
> ---
>  drivers/pci/host/pcie-rcar.c | 42 +-
>  1 file changed, 1 insertion(+), 41 deletions(-)
> 
> diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c
> index 6ab28f29ac6a..66863b587380 100644
> --- a/drivers/pci/host/pcie-rcar.c
> +++ b/drivers/pci/host/pcie-rcar.c
> @@ -1063,44 +1063,6 @@ static const struct of_device_id rcar_pcie_of_match[] 
> = {
>   {},
>  };

Applied to pci/rcar for v4.18, thanks.

Lorenzo

> -static int rcar_pcie_parse_request_of_pci_ranges(struct rcar_pcie *pci)
> -{
> - int err;
> - struct device *dev = pci->dev;
> - struct device_node *np = dev->of_node;
> - resource_size_t iobase;
> - struct resource_entry *win, *tmp;
> -
> - err = of_pci_get_host_bridge_resources(np, 0, 0xff, >resources,
> -);
> - if (err)
> - return err;
> -
> - err = devm_request_pci_bus_resources(dev, >resources);
> - if (err)
> - goto out_release_res;
> -
> - resource_list_for_each_entry_safe(win, tmp, >resources) {
> - struct resource *res = win->res;
> -
> - if (resource_type(res) == IORESOURCE_IO) {
> - err = pci_remap_iospace(res, iobase);
> - if (err) {
> - dev_warn(dev, "error %d: failed to map resource 
> %pR\n",
> -  err, res);
> -
> - resource_list_destroy_entry(win);
> - }
> - }
> - }
> -
> - return 0;
> -
> -out_release_res:
> - pci_free_resource_list(>resources);
> - return err;
> -}
> -
>  static int rcar_pcie_probe(struct platform_device *pdev)
>  {
>   struct device *dev = >dev;
> @@ -1118,9 +1080,7 @@ static int rcar_pcie_probe(struct platform_device *pdev)
>  
>   pcie->dev = dev;
>  
> - INIT_LIST_HEAD(>resources);
> -
> - err = rcar_pcie_parse_request_of_pci_ranges(pcie);
> + err = pci_parse_request_of_pci_ranges(dev, >resources, NULL);
>   if (err)
>   goto err_free_bridge;
>  
> -- 
> 2.8.1
> 

Re: [PATCH v3 0/5] Add R8A77980 PCIe support & some driver cleanups

[Lorenzo Pieralisi]

On Thu, May 03, 2018 at 10:32:41PM +0300, Sergei Shtylyov wrote:
> Hello!
> 
> Here's a set of 5 patches against the 'pci/rcar' branch of Lorenzo Pieralisi's
> 'pci.git' repo. These are the changes needed for better R-Car gen3 support
> (namely for R8A77980 support) plus some PCIe driver re-factoring done in
> the process...
> 
> [1/5] pcie-rcar: poll PHYRDY in rcar_pcie_hw_init()
> [2/5] pcie-rcar: remove PHYRDY polling from rcar_pcie_hw_init_h1()
> [3/5] pcie-rcar: add R-Car gen3 PHY support
> [4/5] pcie-rcar: factor out rcar_pcie_hw_init() call
> [5/5] DT: pci: rcar-pci: document R8A77980 bindings

I applied the series, with some commit log changes, I can comment
on those please have a look at my updated pci/rcar branch.

Thanks,
Lorenzo

Re: [PATCH v2 0/5] Add R8A77980 PCIe support & some driver cleanups

[Lorenzo Pieralisi]

On Thu, May 03, 2018 at 06:26:43PM +0300, Sergei Shtylyov wrote:
> On 05/01/2018 01:57 PM, Lorenzo Pieralisi wrote:
> 
> >>> Hello!
> >>>
> >>> Here's a set of 5 patches against the 'pci/rcar' branch of Lorenzo 
> >>> Pieralisi's
> >>> 'pci.git' repo. These are the changes needed for better R-Car gen3 support
> >>> (namely for R8A77980 support) plus some PCIe driver re-factoring done in
> >>> the process...
> >>>
> >>> [1/5] pcie-rcar: poll PHYRDY in rcar_pcie_hw_init()
> >>> [2/5] pcie-rcar: remove PHYRDY polling from rcar_pcie_hw_init_h1()
> >>> [3/5] pcie-rcar: add R-Car gen3 PHY support
> >>> [4/5] pcie-rcar: factor out rcar_pcie_hw_init() call
> >>> [5/5] DT: pci: rcar-pci: document R8A77980 bindings
> >>
> >> Reviewed-by: Simon Horman <horms+rene...@verge.net.au>
> > 
> > Sergei, would you mind rebasing this series against my pci/rcar
> > branch please ?
> 
>You mean against your updated 'pci/rcar' branch? (The patches were
>against that branch in the 1st place, and they still do apply,
>though with fuzz in one of the patches)...

Yes the current pci/rcar branch, thanks a lot.

Lorenzo

Re: [PATCH v2 0/5] Add R8A77980 PCIe support & some driver cleanups

[Lorenzo Pieralisi]

On Tue, May 01, 2018 at 07:55:53AM +0200, Simon Horman wrote:
> On Sun, Apr 08, 2018 at 08:57:13PM +0300, Sergei Shtylyov wrote:
> > Hello!
> > 
> > Here's a set of 5 patches against the 'pci/rcar' branch of Lorenzo 
> > Pieralisi's
> > 'pci.git' repo. These are the changes needed for better R-Car gen3 support
> > (namely for R8A77980 support) plus some PCIe driver re-factoring done in
> > the process...
> > 
> > [1/5] pcie-rcar: poll PHYRDY in rcar_pcie_hw_init()
> > [2/5] pcie-rcar: remove PHYRDY polling from rcar_pcie_hw_init_h1()
> > [3/5] pcie-rcar: add R-Car gen3 PHY support
> > [4/5] pcie-rcar: factor out rcar_pcie_hw_init() call
> > [5/5] DT: pci: rcar-pci: document R8A77980 bindings
> 
> Reviewed-by: Simon Horman 

Sergei, would you mind rebasing this series against my pci/rcar
branch please ? I will apply it then.

Thanks,
Lorenzo

Re: [PATCH V5] PCI: rcar: Use runtime PM to control controller clock

[Lorenzo Pieralisi]

On Fri, Apr 13, 2018 at 02:48:19PM +0200, Simon Horman wrote:
> On Tue, Apr 10, 2018 at 06:17:04PM +0200, Marek Vasut wrote:
> > On 04/10/2018 05:28 PM, Geert Uytterhoeven wrote:
> 
> ...
> 
> > >>> rcar_pcie_get_resources() is called while the device is
> > >>> runtime-enabled/resumed,
> > >>> pci_free_resource_list() is called while the device is runtime-disabled.
> > 
> > rcar_pcie_get_resources() is NOT a pair function for
> > pci_free_resource_list() . rcar_pcie_parse_request_of_pci_ranges() is a
> > pair function for pci_free_resource_list().
> > 
> > rcar_pcie_parse_request_of_pci_ranges() calls
> > of_pci_get_host_bridge_resources() internally, so every single function
> > called after successful call of rcar_pcie_parse_request_of_pci_ranges()
> > must call pci_free_resource_list().
> > 
> > Both of_pci_get_host_bridge_resources() and pci_free_resource_list() are
> > called with runtime PM disabled.
> > 
> > The naming of the functions is confusing though.
> 
> Hi,
> 
> thanks everyone for their efforts in preparing/reviewing this patch.
> 
> It seems there are some differences of opinion on how best to handle the
> error paths but unlike earlier versions this one seems correct to me. If
> that turns out to be false we can address it. But I don't think its likely
> things will be enhanced by continuing this review.
> 
> Lorenzo, please consider taking this patch in its current form.
> 
> Reviewed-by: Simon Horman 

Applied to pci/rcar for v4.18, thanks.

Lorenzo

Re: [PATCH V3] PCI: rcar: Clean up the macros

[Lorenzo Pieralisi]

On Tue, May 01, 2018 at 07:53:19AM +0200, Simon Horman wrote:
> On Sun, Apr 08, 2018 at 08:04:31PM +0200, Marek Vasut wrote:
> > This patch replaces the (1 << n) with BIT(n) and cleans up whitespace,
> > no functional change.
> > 
> > Signed-off-by: Marek Vasut 
> > Cc: Geert Uytterhoeven 
> > Cc: Phil Edworthy 
> > Cc: Simon Horman 
> > Cc: Wolfram Sang 
> > Cc: linux-renesas-soc@vger.kernel.org
> > Reviewed-by: Niklas S??derlund 
> 
> Marek, thanks for your patch.
> 
> Lorenzo, this seems pretty well baked.
> Could you consider applying it?
> 
> Reviewed-by: Simon Horman 

Applied to pci/rcar for v4.18, thanks.

Lorenzo

Re: [PATCH V5] PCI: rcar: Use runtime PM to control controller clock

[Lorenzo Pieralisi]

On Fri, Apr 13, 2018 at 02:48:19PM +0200, Simon Horman wrote:
> On Tue, Apr 10, 2018 at 06:17:04PM +0200, Marek Vasut wrote:
> > On 04/10/2018 05:28 PM, Geert Uytterhoeven wrote:
> 
> ...
> 
> > >>> rcar_pcie_get_resources() is called while the device is
> > >>> runtime-enabled/resumed,
> > >>> pci_free_resource_list() is called while the device is runtime-disabled.
> > 
> > rcar_pcie_get_resources() is NOT a pair function for
> > pci_free_resource_list() . rcar_pcie_parse_request_of_pci_ranges() is a
> > pair function for pci_free_resource_list().
> > 
> > rcar_pcie_parse_request_of_pci_ranges() calls
> > of_pci_get_host_bridge_resources() internally, so every single function
> > called after successful call of rcar_pcie_parse_request_of_pci_ranges()
> > must call pci_free_resource_list().
> > 
> > Both of_pci_get_host_bridge_resources() and pci_free_resource_list() are
> > called with runtime PM disabled.
> > 
> > The naming of the functions is confusing though.
> 
> Hi,
> 
> thanks everyone for their efforts in preparing/reviewing this patch.
> 
> It seems there are some differences of opinion on how best to handle the
> error paths but unlike earlier versions this one seems correct to me. If
> that turns out to be false we can address it. But I don't think its likely
> things will be enhanced by continuing this review.
> 
> Lorenzo, please consider taking this patch in its current form.

I will as soon as we restart queueing patches for v4.18, thanks for
the heads-up.

Lorenzo

Re: [PATCH] PCI: fix semicolon.cocci warnings

[Lorenzo Pieralisi]

On Wed, Mar 07, 2018 at 09:34:29AM +0100, Julia Lawall wrote:
> From: Fengguang Wu 
> 
>  Remove unneeded semicolon.
> 
> Generated by: scripts/coccinelle/misc/semicolon.cocci
> 
> Signed-off-by: Fengguang Wu 
> Signed-off-by: Julia Lawall 
> ---
> 
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git
> pci-header-cleanup
> head:   40fb7646c6ec16d2fd3de422be876baca9edf86a
> commit: c44f2aed62c2314f1c0ecda606716090b73cc730 [4/5] PCI: improve
> compile test coverage
> :: branch date: 9 hours ago
> :: commit date: 9 hours ago
> 
> 
>  pcie-rcar.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Hi Julia,

FYI, I marked this as superseded in patchwork since Rob already posted
it as part of this series, I will pick it up from there:

https://patchwork.ozlabs.org/project/linux-pci/list/?series=32449

Thanks,
Lorenzo

> --- a/drivers/pci/host/pcie-rcar.c
> +++ b/drivers/pci/host/pcie-rcar.c
> @@ -435,7 +435,7 @@ static void rcar_pcie_force_speedup(stru
>   }
> 
>   msleep(1);
> - };
> + }
> 
>   dev_err(dev, "Speed change timed out\n");
> 

Re: [PATCH V3] PCI: rcar: Use runtime PM to control controller clock

[Lorenzo Pieralisi]

Hi Marek,

On Fri, Nov 10, 2017 at 10:54:11PM +0100, Marek Vasut wrote:
> From: Dien Pham 
> 
> The controller clock can be switched off during suspend/resume,
> let runtime PM take care of that.
> 
> Signed-off-by: Dien Pham 
> Signed-off-by: Hien Dang 
> Signed-off-by: Marek Vasut 
> Cc: Geert Uytterhoeven 
> Cc: Phil Edworthy 
> Cc: Simon Horman 
> Cc: Wolfram Sang 
> Cc: linux-renesas-soc@vger.kernel.org
> To: linux-...@vger.kernel.org
> ---
> V2: - Reorder the fail path in rcar_pcie_probe() to cater for the
>   reordering of function calls in probe
> - Dispose of fail_clk in rcar_pcie_get_resources()
> V3: - Fix up the failpath in probe function
> ---
>  drivers/pci/host/pcie-rcar.c | 40 
>  1 file changed, 12 insertions(+), 28 deletions(-)

I would like to ask you if this patch is still applicable and if so
whether you want me to apply it stand-alone (ie does it depend on
other patches ?), please let me know.

Thanks,
Lorenzo

> diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c
> index 12796eccb2be..e00f865952d5 100644
> --- a/drivers/pci/host/pcie-rcar.c
> +++ b/drivers/pci/host/pcie-rcar.c
> @@ -145,7 +145,6 @@ struct rcar_pcie {
>   void __iomem*base;
>   struct list_headresources;
>   int root_bus_nr;
> - struct clk  *clk;
>   struct clk  *bus_clk;
>   struct  rcar_msi msi;
>  };
> @@ -917,24 +916,14 @@ static int rcar_pcie_get_resources(struct rcar_pcie 
> *pcie)
>   if (IS_ERR(pcie->base))
>   return PTR_ERR(pcie->base);
>  
> - pcie->clk = devm_clk_get(dev, "pcie");
> - if (IS_ERR(pcie->clk)) {
> - dev_err(dev, "cannot get platform clock\n");
> - return PTR_ERR(pcie->clk);
> - }
> - err = clk_prepare_enable(pcie->clk);
> - if (err)
> - return err;
> -
>   pcie->bus_clk = devm_clk_get(dev, "pcie_bus");
>   if (IS_ERR(pcie->bus_clk)) {
>   dev_err(dev, "cannot get pcie bus clock\n");
> - err = PTR_ERR(pcie->bus_clk);
> - goto fail_clk;
> + return PTR_ERR(pcie->bus_clk);
>   }
>   err = clk_prepare_enable(pcie->bus_clk);
>   if (err)
> - goto fail_clk;
> + return err;
>  
>   i = irq_of_parse_and_map(dev->of_node, 0);
>   if (!i) {
> @@ -956,8 +945,6 @@ static int rcar_pcie_get_resources(struct rcar_pcie *pcie)
>  
>  err_map_reg:
>   clk_disable_unprepare(pcie->bus_clk);
> -fail_clk:
> - clk_disable_unprepare(pcie->clk);
>  
>   return err;
>  }
> @@ -1125,22 +1112,22 @@ static int rcar_pcie_probe(struct platform_device 
> *pdev)
>  
>   rcar_pcie_parse_request_of_pci_ranges(pcie);
>  
> + pm_runtime_enable(pcie->dev);
> + err = pm_runtime_get_sync(pcie->dev);
> + if (err < 0) {
> + dev_err(pcie->dev, "pm_runtime_get_sync failed\n");
> + goto err_pm_disable;
> + }
> +
>   err = rcar_pcie_get_resources(pcie);
>   if (err < 0) {
>   dev_err(dev, "failed to request resources: %d\n", err);
> - goto err_free_bridge;
> + goto err_pm_put;
>   }
>  
>   err = rcar_pcie_parse_map_dma_ranges(pcie, dev->of_node);
>   if (err)
> - goto err_free_bridge;
> -
> - pm_runtime_enable(dev);
> - err = pm_runtime_get_sync(dev);
> - if (err < 0) {
> - dev_err(dev, "pm_runtime_get_sync failed\n");
> - goto err_pm_disable;
> - }
> + goto err_pm_put;
>  
>   /* Failure to get a link might just be that no cards are inserted */
>   hw_init_fn = of_device_get_match_data(dev);
> @@ -1172,13 +1159,10 @@ static int rcar_pcie_probe(struct platform_device 
> *pdev)
>  
>  err_pm_put:
>   pm_runtime_put(dev);
> -
>  err_pm_disable:
>   pm_runtime_disable(dev);
> -
> -err_free_bridge:
> - pci_free_host_bridge(bridge);
>   pci_free_resource_list(>resources);
> + pci_free_host_bridge(bridge);
>  
>   return err;
>  }
> -- 
> 2.11.0
> 

Re: [PATCH v2] dt-bindings: PCI: rcar: Add device tree support for r8a7743

[Lorenzo Pieralisi]

On Tue, Nov 14, 2017 at 09:59:03AM +, Biju Das wrote:
> Add support for r8a7743. The Renesas RZ/G1M(R8A7743)PCIe controller
> is identical to the R-Car Gen2 family.
> 
> No driver change is needed due to the fallback compatible value
> "renesas,pcie-rcar-gen2".
> Adding the SoC-specific compatible values here has three purposes:
> 1. Document which SoCs have this hardware module,
> 2. Allow checkpatch to validate compatible values.
> 3. Allow the driver to support SoC specific implementations in future
>as necessary.
> 
> Signed-off-by: Biju Das 
> Reviewed-by: Fabrizio Castro 
> Reviewed-by: Geert Uytterhoeven 
> ---
> v1-->v2
>Corrected RZ-G1 to RZ/G1.
> 
>  Documentation/devicetree/bindings/pci/rcar-pci.txt | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)

Applied to pci/rcar for v4.17, apologies for the delay.

Thanks,
Lorenzo

> diff --git a/Documentation/devicetree/bindings/pci/rcar-pci.txt 
> b/Documentation/devicetree/bindings/pci/rcar-pci.txt
> index 76ba3a6..1fb614e 100644
> --- a/Documentation/devicetree/bindings/pci/rcar-pci.txt
> +++ b/Documentation/devicetree/bindings/pci/rcar-pci.txt
> @@ -1,13 +1,15 @@
>  * Renesas R-Car PCIe interface
>  
>  Required properties:
> -compatible: "renesas,pcie-r8a7779" for the R8A7779 SoC;
> +compatible: "renesas,pcie-r8a7743" for the R8A7743 SoC;
> + "renesas,pcie-r8a7779" for the R8A7779 SoC;
>   "renesas,pcie-r8a7790" for the R8A7790 SoC;
>   "renesas,pcie-r8a7791" for the R8A7791 SoC;
>   "renesas,pcie-r8a7793" for the R8A7793 SoC;
>   "renesas,pcie-r8a7795" for the R8A7795 SoC;
>   "renesas,pcie-r8a7796" for the R8A7796 SoC;
> - "renesas,pcie-rcar-gen2" for a generic R-Car Gen2 compatible device.
> + "renesas,pcie-rcar-gen2" for a generic R-Car Gen2 or
> +  RZ/G1 compatible device.
>   "renesas,pcie-rcar-gen3" for a generic R-Car Gen3 compatible device.
>  
>   When compatible with the generic version, nodes must list the
> -- 
> 1.9.1
> 

Re: [PATCH] PCI: rcar-gen2: remove duplicated bit-wise or of RCAR_PCI_INT_SIGRETABORT

[Lorenzo Pieralisi]

On Fri, Feb 23, 2018 at 12:29:49PM +, Colin King wrote:
> From: Colin Ian King 
> 
> Bit pattern RCAR_PCI_INT_SIGRETABORT is being bit-wise or'd twice;
> remove the redundant 2nd RCAR_PCI_INT_SIGRETABORT.
> 
> Signed-off-by: Colin Ian King 
> ---
>  drivers/pci/host/pci-rcar-gen2.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/pci/host/pci-rcar-gen2.c 
> b/drivers/pci/host/pci-rcar-gen2.c

Applied to pci/rcar for v4.17, thanks !

Lorenzo

> index a28370bb2b2a..dd4f1a6b57c5 100644
> --- a/drivers/pci/host/pci-rcar-gen2.c
> +++ b/drivers/pci/host/pci-rcar-gen2.c
> @@ -52,7 +52,6 @@
>  #define RCAR_PCI_INT_B   (1 << 17)
>  #define RCAR_PCI_INT_PME (1 << 19)
>  #define RCAR_PCI_INT_ALLERRORS (RCAR_PCI_INT_SIGTABORT   | \
> - RCAR_PCI_INT_SIGRETABORT| \
>   RCAR_PCI_INT_SIGRETABORT| \
>   RCAR_PCI_INT_REMABORT   | \
>   RCAR_PCI_INT_PERR   | \
> -- 
> 2.15.1
> 

Re: [PATCH 2/2] PCI: rcar: Handle rcar_pcie_parse_request_of_pci_ranges() failures

[Lorenzo Pieralisi]

On Thu, Dec 07, 2017 at 11:15:20AM +0100, Geert Uytterhoeven wrote:
> rcar_pcie_parse_request_of_pci_ranges() can fail and return an error
> code, but this is not checked nor handled.
> 
> Fix this by adding the missing error handling.
> 
> Fixes: 5d2917d469faab72 ("PCI: rcar: Convert to DT resource parsing API")
> Signed-off-by: Geert Uytterhoeven 
> ---
>  drivers/pci/host/pcie-rcar.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)

Applied to pci/rcar for v4.16.

Thanks,
Lorenzo

> diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c
> index 52ab3cb0a0bfe065..95ca4a1feba4b759 100644
> --- a/drivers/pci/host/pcie-rcar.c
> +++ b/drivers/pci/host/pcie-rcar.c
> @@ -1123,7 +1123,9 @@ static int rcar_pcie_probe(struct platform_device *pdev)
>  
>   INIT_LIST_HEAD(>resources);
>  
> - rcar_pcie_parse_request_of_pci_ranges(pcie);
> + err = rcar_pcie_parse_request_of_pci_ranges(pcie);
> + if (err)
> + goto err_free_bridge;
>  
>   err = rcar_pcie_get_resources(pcie);
>   if (err < 0) {
> @@ -1178,6 +1180,7 @@ static int rcar_pcie_probe(struct platform_device *pdev)
>  
>  err_free_resource_list:
>   pci_free_resource_list(>resources);
> +err_free_bridge:
>   pci_free_host_bridge(bridge);
>  
>   return err;
> -- 
> 2.7.4
> 

Re: [PATCH 0/2] PCI: rcar: Misc error path fixes

[Lorenzo Pieralisi]

On Tue, Dec 12, 2017 at 11:16:58AM -0600, Bjorn Helgaas wrote:
> On Thu, Dec 07, 2017 at 11:15:18AM +0100, Geert Uytterhoeven wrote:
> > Hi Simon, Lorenzo, Bjorn,
> > 
> > This patch series fixes two issues in the error path for the R-Car PCIe
> > host bridge driver.
> > 
> > The first issue is triggered easily by not having a PCIe card inserted,
> > and may cause a crash.
> > 
> > Thanks!
> > 
> > Geert Uytterhoeven (2):
> >   PCI: rcar: Fix use-after-free in probe error path
> >   PCI: rcar: Handle rcar_pcie_parse_request_of_pci_ranges() failures
> > 
> >  drivers/pci/host/pcie-rcar.c | 11 +++
> >  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> The first fixes ddd535f1ea3eb27e, which appeared in v4.14-rc1.  The
> second fixes 5d2917d469faab72, which appeared in v4.5-rc1.
> 
> After the merge window I normally just pull in critical fixes and
> fixes to things we merged during the window.
> 
> I think the first makes sense for v4.15 because it can cause a crash
> with no obvious way to debug it.  The second one feels like v4.16
> material to me.
> 
> Thoughts?  We're trying to sort out how to handle this sort of thing
> between Lorenzo and myself, so I apologize for the confusion here.

This sounds absolutely reasonable for me.

Thanks !
Lorenzo

Re: [PATCH 2/2] PCI: rcar: Handle rcar_pcie_parse_request_of_pci_ranges() failures

[Lorenzo Pieralisi]

On Thu, Dec 07, 2017 at 11:15:20AM +0100, Geert Uytterhoeven wrote:
> rcar_pcie_parse_request_of_pci_ranges() can fail and return an error
> code, but this is not checked nor handled.
> 
> Fix this by adding the missing error handling.
> 
> Fixes: 5d2917d469faab72 ("PCI: rcar: Convert to DT resource parsing API")
> Signed-off-by: Geert Uytterhoeven <geert+rene...@glider.be>
> ---
>  drivers/pci/host/pcie-rcar.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)

Acked-by: Lorenzo Pieralisi <lorenzo.pieral...@arm.com>

> diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c
> index 52ab3cb0a0bfe065..95ca4a1feba4b759 100644
> --- a/drivers/pci/host/pcie-rcar.c
> +++ b/drivers/pci/host/pcie-rcar.c
> @@ -1123,7 +1123,9 @@ static int rcar_pcie_probe(struct platform_device *pdev)
>  
>   INIT_LIST_HEAD(>resources);
>  
> - rcar_pcie_parse_request_of_pci_ranges(pcie);
> + err = rcar_pcie_parse_request_of_pci_ranges(pcie);
> + if (err)
> + goto err_free_bridge;
>  
>   err = rcar_pcie_get_resources(pcie);
>   if (err < 0) {
> @@ -1178,6 +1180,7 @@ static int rcar_pcie_probe(struct platform_device *pdev)
>  
>  err_free_resource_list:
>   pci_free_resource_list(>resources);
> +err_free_bridge:
>   pci_free_host_bridge(bridge);
>  
>   return err;
> -- 
> 2.7.4
> 

Re: [PATCH 1/2] PCI: rcar: Fix use-after-free in probe error path

[Lorenzo Pieralisi]

On Thu, Dec 07, 2017 at 11:15:19AM +0100, Geert Uytterhoeven wrote:
> If CONFIG_DEBUG_SLAB=y, and no PCIe card is inserted, the kernel crashes
> during probe on r8a7791/koelsch:
> 
> rcar-pcie fe00.pcie: PCIe link down
> Unable to handle kernel paging request at virtual address 6b6b6b6b
> 
> (seeing this message requires earlycon and keep_bootcon).
> 
> Indeed, pci_free_host_bridge() frees the PCI host bridge, including the
> embedded rcar_pcie object, so pci_free_resource_list() must not be
> called afterwards.
> 
> To fix this, move the call to pci_free_resource_list() up, and update the
> label name accordingly.
> 
> Fixes: ddd535f1ea3eb27e ("PCI: rcar: Fix memory leak when no PCIe card is 
> inserted")
> Signed-off-by: Geert Uytterhoeven <geert+rene...@glider.be>
> ---
>  drivers/pci/host/pcie-rcar.c | 8 ++++
>  1 file changed, 4 insertions(+), 4 deletions(-)

Acked-by: Lorenzo Pieralisi <lorenzo.pieral...@arm.com>

> diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c
> index 12796eccb2befd91..52ab3cb0a0bfe065 100644
> --- a/drivers/pci/host/pcie-rcar.c
> +++ b/drivers/pci/host/pcie-rcar.c
> @@ -1128,12 +1128,12 @@ static int rcar_pcie_probe(struct platform_device 
> *pdev)
>   err = rcar_pcie_get_resources(pcie);
>   if (err < 0) {
>   dev_err(dev, "failed to request resources: %d\n", err);
> - goto err_free_bridge;
> + goto err_free_resource_list;
>   }
>  
>   err = rcar_pcie_parse_map_dma_ranges(pcie, dev->of_node);
>   if (err)
> - goto err_free_bridge;
> + goto err_free_resource_list;
>  
>   pm_runtime_enable(dev);
>   err = pm_runtime_get_sync(dev);
> @@ -1176,9 +1176,9 @@ static int rcar_pcie_probe(struct platform_device *pdev)
>  err_pm_disable:
>   pm_runtime_disable(dev);
>  
> -err_free_bridge:
> - pci_free_host_bridge(bridge);
> +err_free_resource_list:
>   pci_free_resource_list(>resources);
> + pci_free_host_bridge(bridge);
>  
>   return err;
>  }
> -- 
> 2.7.4
> 

Re: [PATCH V2 4/5] PCI: rcar: Support runtime PM, link state L1 handling

[Lorenzo Pieralisi]

Hi Marek,

On Fri, Nov 10, 2017 at 10:58:42PM +0100, Marek Vasut wrote:
> From: Phil Edworthy 
> 
> Most PCIe host controllers support L0s and L1 power states via ASPM.
> The R-Car hardware only supports L0s, so when the system suspends and
> resumes we have to manually handle L1.
> When the system suspends, cards can put themselves into L1 and send a

I assumed L1 entry has to be negotiated depending upon the PCIe
hierarchy capabilities, I would appreciate if you can explain to
me what's the root cause of the issue please.

> PM_ENTER_L1 DLLP to the host controller. At this point, we can no longer
> access the card's config registers.
> 
> The R-Car host controller will handle taking cards out of L1 as long as
> the host controller has also been transitioned to L1 link state.

I wonder why this can't be done in a PM restore hook but that's not
really where my question is.

> Ideally, we would detect the PM_ENTER_L1 DLLP using an interrupt and
> transition the host to L1 immediately. However, this patch just ensures
> that we can talk to cards after they have gone into L1.

> When attempting a config access, it checks to see if the card has gone
> into L1, and if so, does the same for the host controller.
> 
> This is based on a patch by Hien Dang 
> 
> Signed-off-by: Phil Edworthy 
> Signed-off-by: Marek Vasut 
> Cc: Geert Uytterhoeven 
> Cc: Phil Edworthy 
> Cc: Simon Horman 
> Cc: Wolfram Sang 
> Cc: linux-renesas-soc@vger.kernel.org
> ---
> V2: - Drop extra parenthesis
> - Use GENMASK()
> - Fix comment "The HW will handle coming of of L1.", s/of of/out of/
> ---
>  drivers/pci/host/pcie-rcar.c | 24 
>  1 file changed, 24 insertions(+)
> 
> diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c
> index ab61829db389..068bf9067ec1 100644
> --- a/drivers/pci/host/pcie-rcar.c
> +++ b/drivers/pci/host/pcie-rcar.c
> @@ -92,6 +92,13 @@
>  #define MACCTLR  0x011058
>  #define  SPEED_CHANGEBIT(24)
>  #define  SCRAMBLE_DISABLEBIT(27)
> +#define PMSR 0x01105c
> +#define  L1FAEG  BIT(31)
> +#define  PM_ENTER_L1RX   BIT(23)
> +#define  PMSTATE GENMASK(18, 16)
> +#define  PMSTATE_L1  GENMASK(17, 16)
> +#define PMCTLR   0x011060
> +#define  L1_INIT BIT(31)
>  #define MACS2R   0x011078
>  #define MACCGSPSETR  0x011084
>  #define  SPCNGRSNBIT(31)
> @@ -191,6 +198,7 @@ static int rcar_pcie_config_access(struct rcar_pcie *pcie,
>   unsigned int devfn, int where, u32 *data)
>  {
>   int dev, func, reg, index;
> + u32 val;
>  
>   dev = PCI_SLOT(devfn);
>   func = PCI_FUNC(devfn);
> @@ -232,6 +240,22 @@ static int rcar_pcie_config_access(struct rcar_pcie 
> *pcie,
>   if (pcie->root_bus_nr < 0)
>   return PCIBIOS_DEVICE_NOT_FOUND;
>  
> + /*
> +  * If we are not in L1 link state and we have received PM_ENTER_L1 DLLP,
> +  * transition to L1 link state. The HW will handle coming out of L1.
> +  */
> + val = rcar_pci_read_reg(pcie, PMSR);
> + if (val & PM_ENTER_L1RX && (val & PMSTATE) != PMSTATE_L1) {
> + rcar_pci_write_reg(pcie, L1_INIT, PMCTLR);
> +
> + /* Wait until we are in L1 */
> + while (!(val & L1FAEG))
> + val = rcar_pci_read_reg(pcie, PMSR);
> +
> + /* Clear flags indicating link has transitioned to L1 */
> + rcar_pci_write_reg(pcie, L1FAEG | PM_ENTER_L1RX, PMSR);
> + }

I do not get why you need to add the DLLP check for _every_ given config
access and how/why it is just related to suspend/resume and not eg cold
boot (I supposed it is because devices can enter L1 upon suspend(?)), I
would ask you please to provide a thorough explanation so that I can
actually review this patch (the commit log must be rewritten nonetheless,
I do not think it is clear, at least it is not for me).

Thanks,
Lorenzo

> +
>   /* Clear errors */
>   rcar_pci_write_reg(pcie, rcar_pci_read_reg(pcie, PCIEERRFR), PCIEERRFR);
>  
> -- 
> 2.11.0
> 

Re: [1/2] drivers: firmware: psci: Add psci_is_available()

[Lorenzo Pieralisi]

On Wed, Oct 11, 2017 at 10:03:01AM +0200, Geert Uytterhoeven wrote:
> PSCI support may be disabled at build time (by configuration) or at
> run-time (PSCI firmware not present).  While CONFIG_ARM_PSCI_FW can be
> used to check for build time enablement, there is currently no simple
> way to check if PSCI is actually available and used.
> 
> Hence add a helper function to check if PSCI is available.

Hi Geert,

excuse us the delay in responding. I think it would be better if the
check just carries out a DT/ACPI matching check rather than being based
on PSCI ops initialization but before doing that I would like first to
understand what this function can be actually used for (ie I do not
think the usage in the PSCI checker is relevant to this discussion).

Thanks,
Lorenzo

> This is useful for e.g. drivers that are used on platforms with and
> without PSCI.  Such drivers may need to take provisions for proper
> operation when PSCI is used, and/or to implement functionality that is
> usually provided by PSCI.
> 
> Signed-off-by: Geert Uytterhoeven 
> ---
>  drivers/firmware/psci.c | 5 +
>  include/linux/psci.h| 2 ++
>  2 files changed, 7 insertions(+)
> 
> diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
> index da469c972b503f83..a3a11e2d8aaa 100644
> --- a/drivers/firmware/psci.c
> +++ b/drivers/firmware/psci.c
> @@ -670,6 +670,11 @@ int __init psci_dt_init(void)
>   return init_fn(np);
>  }
>  
> +bool psci_is_available(void)
> +{
> + return psci_ops.cpu_off && psci_ops.cpu_on && psci_ops.cpu_suspend;
> +}
> +
>  #ifdef CONFIG_ACPI
>  /*
>   * We use PSCI 0.2+ when ACPI is deployed on ARM64 and it's
> diff --git a/include/linux/psci.h b/include/linux/psci.h
> index bdea1cb5e1db142b..2bdee325aeb80cf6 100644
> --- a/include/linux/psci.h
> +++ b/include/linux/psci.h
> @@ -39,8 +39,10 @@ extern struct psci_operations psci_ops;
>  
>  #if defined(CONFIG_ARM_PSCI_FW)
>  int __init psci_dt_init(void);
> +bool psci_is_available(void);
>  #else
>  static inline int psci_dt_init(void) { return 0; }
> +static inline bool psci_is_available(void) { return false; }
>  #endif
>  
>  #if defined(CONFIG_ARM_PSCI_FW) && defined(CONFIG_ACPI)

Re: [PATCH] PCI: rcar-pcie: Fix memory leak when no PCIe card is inserted

[Lorenzo Pieralisi]

On Wed, Aug 02, 2017 at 04:13:20PM +0900, Harunobu Kurokawa wrote:
> When no PCIe card is inserted, there is a memory leak as
> pci_free_resource_list is not called before returning.
> 
> Signed-off-by: Harunobu Kurokawa <harunobu.kurokawa...@renesas.com>
> ---
>  drivers/pci/host/pcie-rcar.c | 7 ---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c
> index ac80fbb..9b06de6 100644
> --- a/drivers/pci/host/pcie-rcar.c
> +++ b/drivers/pci/host/pcie-rcar.c
> @@ -1190,14 +1190,15 @@ static int rcar_pcie_probe(struct platform_device 
> *pdev)
>  
>   return 0;
>  
> -err_free_bridge:
> - pci_free_host_bridge(bridge);
> -
>  err_pm_put:
>   pm_runtime_put(dev);
>  
>  err_pm_disable:
>   pm_runtime_disable(dev);
> +
> +err_free_bridge:
> + pci_free_host_bridge(bridge);
> + pci_free_resource_list(>resources);
>   return err;
>  }

The error code path needs fixing but not like this (with a single
patch lumping all fixes - well, actually it does not - together).

Step one, patch below. Step two free the >resources list
(which IIUC was not freed even before commit 90634e854079) in
a separate patch.

Am I missing something ?

Please apply, check and repost the patch if it is ok, thanks.

-- >8 --
Subject: [PATCH] PCI: rcar: Fix error exit path

Commit 90634e854079 ("PCI: rcar: Convert PCI scan API to
pci_scan_root_bus_bridge()") converted PCI root bus scan API
to the new pci_scan_root_bus_bridge() API; in the process
some error paths were not updated correctly which may
cause memory leaks.

Fix the driver error exit path reinstating the previous
correct error exit behaviour.

Fixes: 90634e854079 ("PCI: rcar: Convert PCI scan API to 
pci_scan_root_bus_bridge()")
Signed-off-by: Lorenzo Pieralisi <lorenzo.pieral...@arm.com>
---
 drivers/pci/host/pcie-rcar.c | 11 +--
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c
index 246d485..007523e 100644
--- a/drivers/pci/host/pcie-rcar.c
+++ b/drivers/pci/host/pcie-rcar.c
@@ -471,10 +471,8 @@ static int rcar_pcie_enable(struct rcar_pcie *pcie)
bridge->msi = >msi.chip;
 
ret = pci_scan_root_bus_bridge(bridge);
-   if (ret < 0) {
-   kfree(bridge);
+   if (ret < 0)
return ret;
-   }
 
bus = bridge->bus;
 
@@ -1190,14 +1188,15 @@ static int rcar_pcie_probe(struct platform_device *pdev)
 
return 0;
 
-err_free_bridge:
-   pci_free_host_bridge(bridge);
-
 err_pm_put:
pm_runtime_put(dev);
 
 err_pm_disable:
pm_runtime_disable(dev);
+
+err_free_bridge:
+   pci_free_host_bridge(bridge);
+
return err;
 }
 
-- 
2.10.0

Re: [RFC] pci: Provide a domain limited version of pdev_fixup_irq

[Lorenzo Pieralisi]

Hi Bjorn, all,

On Mon, Aug 08, 2016 at 06:19:22PM -0500, Bjorn Helgaas wrote:
> [+cc Matthew]
> 
> On Mon, Aug 08, 2016 at 09:31:05AM +, Phil Edworthy wrote:
> > Hi Lorenzo,
> > 
> > Thanks for the link, I'll wait to see how this pans out.
> 
> I'm not sure anything is happening there.  Last time I looked
> (http://lkml.kernel.org/r/20151223230433.GA27708@localhost), I thought
> there were some issues.
> 
> From Lorenzo's response, it sounds like he thought maybe I should
> apply it anyway, but I dropped the ball and haven't done anything with
> it.

There are pending issues to be sorted in particular for x86, but
given how Matthew's patchset is structured we could merge the code
as an opt-in (we could initialize the bridge->map_irq function pointer
only for the cases we know that are safe; granted we won't be able to
remove pci_fixup_irqs() till all its users are removed, but at least
on ARM/ARM64 we would be able to switch over to the new bridge->map_irq
allocation approach instead of relying on pci_fixup_irqs()).

I am not sure we need to wait for all arches to be converted before
merging Matt's code, that was my point, but I need to have another
thorough look myself before commenting any further.

Thanks,
Lorenzo

> I'll take another look at it.
> 
> > > -Original Message-
> > > From: linux-pci-ow...@vger.kernel.org [mailto:linux-pci-
> > > ow...@vger.kernel.org] On Behalf Of Lorenzo Pieralisi
> > > Sent: 21 July 2016 10:35
> > > To: Phil Edworthy <phil.edwor...@renesas.com>
> > > Cc: Bjorn Helgaas <bhelg...@google.com>; linux-...@vger.kernel.org; linux-
> > > renesas-...@vger.kernel.org
> > > Subject: Re: [RFC] pci: Provide a domain limited version of pdev_fixup_irq
> > > 
> > > Hi Phil,
> > > 
> > > On Fri, Jul 08, 2016 at 12:49:55PM +0100, Phil Edworthy wrote:
> > > > Hi Bjorn,
> > > >
> > > > I've marked this as RFC as I guess there might be a better way to do
> > > > this, but I'm not sure how. I would appreciate your thoughts on this.
> > > 
> > > There is a more generic solution to this issue, that has been
> > > hanging in the balance for a while:
> > > 
> > > http://marc.info/?l=linux-pci=144557674601373=2
> > > 
> > > We should bring it to completion since pci_fixup_irqs() should
> > > really be replaced, this is just another example of the bugs
> > > it can trigger.
> > > 
> > > Thanks,
> > > Lorenzo
> > > 
> > > > Thanks
> > > > Phil
> > > >
> > > > ---
> > > > pdev_fixup_irq() performs interrupt swizzling on all PCI devices,
> > > > no matter that the device may be on a completely unrelated PCI
> > > > Host controller.
> > > >
> > > > When you have multiple PCI Host controllers, pdev_fixup_irq() can
> > > > clobber the dev->irq of devices it should not be changing.
> > > > This has been seen when performing suspend/resume on boards with
> > > > two R-Car PCIe Host controllers, resulting in a NULL ptr access
> > > > in __pci_restore_msi_state. This happens because dev->irq has been
> > > > overwritten with 0, and irq_get_msi_desc(dev->irq) returns NULL.
> > > >
> > > > This patch introduces a new function, pci_fixup_irqs_local(), that
> > > > performs the same operation as pdev_fixup_irq(), but only changes
> > > > the dev->irq of device on the same domain.
> > > >
> > > > Signed-off-by: Phil Edworthy <phil.edwor...@renesas.com>
> > > > ---
> > > >  drivers/pci/setup-irq.c | 25 ++---
> > > >  include/linux/pci.h |  3 +++
> > > >  2 files changed, 25 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/drivers/pci/setup-irq.c b/drivers/pci/setup-irq.c
> > > > index 95c225b..90ea8fa 100644
> > > > --- a/drivers/pci/setup-irq.c
> > > > +++ b/drivers/pci/setup-irq.c
> > > > @@ -22,7 +22,8 @@ void __weak pcibios_update_irq(struct pci_dev *dev, 
> > > > int
> > > irq)
> > > > pci_write_config_byte(dev, PCI_INTERRUPT_LINE, irq);
> > > >  }
> > > >
> > > > -static void pdev_fixup_irq(struct pci_dev *dev,
> > > > +static void pdev_fixup_irq(int domain_nr,
> > > > +  struct pci_dev *dev,
> > > >u8 (*swizzle)(struct pci_dev *, u8 *),
> > > > 

Re: [RFC] pci: Provide a domain limited version of pdev_fixup_irq

[Lorenzo Pieralisi]

Hi Phil,

On Fri, Jul 08, 2016 at 12:49:55PM +0100, Phil Edworthy wrote:
> Hi Bjorn,
> 
> I've marked this as RFC as I guess there might be a better way to do
> this, but I'm not sure how. I would appreciate your thoughts on this.

There is a more generic solution to this issue, that has been
hanging in the balance for a while:

http://marc.info/?l=linux-pci=144557674601373=2

We should bring it to completion since pci_fixup_irqs() should
really be replaced, this is just another example of the bugs
it can trigger.

Thanks,
Lorenzo

> Thanks
> Phil
> 
> ---
> pdev_fixup_irq() performs interrupt swizzling on all PCI devices,
> no matter that the device may be on a completely unrelated PCI
> Host controller.
> 
> When you have multiple PCI Host controllers, pdev_fixup_irq() can
> clobber the dev->irq of devices it should not be changing.
> This has been seen when performing suspend/resume on boards with
> two R-Car PCIe Host controllers, resulting in a NULL ptr access
> in __pci_restore_msi_state. This happens because dev->irq has been
> overwritten with 0, and irq_get_msi_desc(dev->irq) returns NULL.
> 
> This patch introduces a new function, pci_fixup_irqs_local(), that
> performs the same operation as pdev_fixup_irq(), but only changes
> the dev->irq of device on the same domain.
> 
> Signed-off-by: Phil Edworthy 
> ---
>  drivers/pci/setup-irq.c | 25 ++---
>  include/linux/pci.h |  3 +++
>  2 files changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/setup-irq.c b/drivers/pci/setup-irq.c
> index 95c225b..90ea8fa 100644
> --- a/drivers/pci/setup-irq.c
> +++ b/drivers/pci/setup-irq.c
> @@ -22,7 +22,8 @@ void __weak pcibios_update_irq(struct pci_dev *dev, int irq)
>   pci_write_config_byte(dev, PCI_INTERRUPT_LINE, irq);
>  }
>  
> -static void pdev_fixup_irq(struct pci_dev *dev,
> +static void pdev_fixup_irq(int domain_nr,
> +struct pci_dev *dev,
>  u8 (*swizzle)(struct pci_dev *, u8 *),
>  int (*map_irq)(const struct pci_dev *, u8, u8))
>  {
> @@ -48,8 +49,15 @@ static void pdev_fixup_irq(struct pci_dev *dev,
>   if (irq == -1)
>   irq = 0;
>   }
> - dev->irq = irq;
> + /* Since pci_fixup_irqs() can be called more than once due to multiple
> +  * host controllers, and we scan all PCI devices, not just those
> +  * attached to this controller, make sure we don't clobber dev->irq
> +  * that has nothing to do with this domain.
> +  */
> + if (domain_nr >= 0 && dev->bus->domain_nr != domain_nr)
> + return;
>  
> + dev->irq = irq;
>   dev_dbg(>dev, "fixup irq: got %d\n", dev->irq);
>  
>   /* Always tell the device, so the driver knows what is
> @@ -63,6 +71,17 @@ void pci_fixup_irqs(u8 (*swizzle)(struct pci_dev *, u8 *),
>   struct pci_dev *dev = NULL;
>  
>   for_each_pci_dev(dev)
> - pdev_fixup_irq(dev, swizzle, map_irq);
> + pdev_fixup_irq(-1, dev, swizzle, map_irq);
>  }
>  EXPORT_SYMBOL_GPL(pci_fixup_irqs);
> +
> +void pci_fixup_irqs_local(struct pci_bus *bus,
> + u8 (*swizzle)(struct pci_dev *, u8 *),
> + int (*map_irq)(const struct pci_dev *, u8, u8))
> +{
> + struct pci_dev *dev = NULL;
> +
> + for_each_pci_dev(dev)
> + pdev_fixup_irq(bus->domain_nr, dev, swizzle, map_irq);
> +}
> +EXPORT_SYMBOL_GPL(pci_fixup_irqs_local);
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 8badb66..37a97df 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1134,6 +1134,9 @@ void pdev_enable_device(struct pci_dev *);
>  int pci_enable_resources(struct pci_dev *, int mask);
>  void pci_fixup_irqs(u8 (*)(struct pci_dev *, u8 *),
>   int (*)(const struct pci_dev *, u8, u8));
> +void pci_fixup_irqs_local(struct pci_bus *,
> + u8 (*)(struct pci_dev *, u8 *),
> + int (*)(const struct pci_dev *, u8, u8));
>  #define HAVE_PCI_REQ_REGIONS 2
>  int __must_check pci_request_regions(struct pci_dev *, const char *);
>  int __must_check pci_request_regions_exclusive(struct pci_dev *, const char 
> *);
> -- 
> 2.5.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH v1 15/25] PCI: generic: Free resource list close to where it's allocated

[Lorenzo Pieralisi]

On Mon, Jun 20, 2016 at 09:56:45AM -0700, Tyler Baker wrote:
> Hi Bjorn,
> 
> On 6 June 2016 at 16:06, Bjorn Helgaas  wrote:
> > Previously we allocated the PCI resource list in
> > gen_pci_parse_request_of_pci_ranges(), but if we had an error, we freed it
> > on error in gen_pci_init().
> >
> > Reorder gen_pci_init() so we can take care of error path cleanup in
> > gen_pci_parse_request_of_pci_ranges() instead.
> >
> > Signed-off-by: Bjorn Helgaas 
> 
> The kernelci.org bot has reported[0] new qemu-aarch64
> (arm64-defconfig) boot failures[1][2] in next-20160620. I've
> bisected[3] this boot failure down to this patch, and confirmed
> reverting it on top of next-20160620 resolves the boot issue.
> 
> I have not investigated further, but you can easily reproduce[4] the
> boot failure on an x86 host running qemu-system-aarch64 (I'm running
> qemu-system 2.6).

That's most likely because pci_ecam_create() requires the bus_range
resource (its busr parameter) to be initialized when it is called
and that's not the case after this patch is applied if I read it
correctly.

It is probably a NULL pointer dereference in pci_ecam_create().

Thanks,
Lorenzo

> 
> Cheers,
> 
> Tyler
> 
> [0] https://kernelci.org/boot/all/job/next/kernel/next-20160620/
> [1] 
> https://storage.kernelci.org/next/next-20160620/arm64-defconfig/lab-cambridge/boot-apm-mustang-kvm-guest.txt
> [2] 
> https://storage.kernelci.org/next/next-20160620/arm64-defconfig/lab-tbaker/boot-qemu-aarch64,legacy.txt
> [3] http://hastebin.com/segiruribu.vbs
> [4] http://hastebin.com/dafuzicuyi.avrasm
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>