mbox series

[v3,0/2] PCI: imx: Initial imx7d pm support

Message ID cover.1532448745.git.leonard.crestez@nxp.com (mailing list archive)
Headers show
Series PCI: imx: Initial imx7d pm support | expand

Message

Leonard Crestez July 24, 2018, 4:14 p.m. UTC
Changes since v2:
 * Print with dev_info if link fails on resume (Lucas)
 * Add a comment on imx7d pci irq mappings (Andrey)
 * Make imx6_pcie_ltssm_disable print an error on IMX6Q (Lucas)

The ltssm_disable does not return an error because it can't be usefully
handled, reversing partial suspend is a nightmare and unlikely to work.

 * Drop "reset: imx7: Fix always writing bits as 0 (accepted by Philipp)

Series is against linux-next tag next-20180724 where the reset patch was
already accepted. The imx7d.dtsi patch is also useful standalone.

Link to v2: https://lkml.org/lkml/2018/7/20/472

Leonard Crestez (2):
  Revert "ARM: dts: imx7d: Invert legacy PCI irq mapping"
  PCI: imx: Initial imx7d pm support

 arch/arm/boot/dts/imx7d.dtsi          | 12 ++--
 drivers/pci/controller/dwc/pci-imx6.c | 97 +++++++++++++++++++++++++--
 2 files changed, 100 insertions(+), 9 deletions(-)

Comments

Leonard Crestez Aug. 8, 2018, 10:53 a.m. UTC | #1
On Tue, 2018-07-24 at 19:14 +0300, Leonard Crestez wrote:
> Changes since v2:
>  * Print with dev_info if link fails on resume (Lucas)
>  * Add a comment on imx7d pci irq mappings (Andrey)
>  * Make imx6_pcie_ltssm_disable print an error on IMX6Q (Lucas)
> 
> The ltssm_disable does not return an error because it can't be usefully
> handled, reversing partial suspend is a nightmare and unlikely to work.
> 
>  * Drop "reset: imx7: Fix always writing bits as 0 (accepted by Philipp)
> 
> Series is against linux-next tag next-20180724 where the reset patch was
> already accepted. The imx7d.dtsi patch is also useful standalone.
> 
> Link to v2: https://lkml.org/lkml/2018/7/20/472

This is a gentle reminder that this series was reviewed by Lucas two
weeks ago but not yet included.

--
Regards,
Leonard
Lorenzo Pieralisi Aug. 8, 2018, 11:14 a.m. UTC | #2
On Wed, Aug 08, 2018 at 10:53:52AM +0000, Leonard Crestez wrote:
> On Tue, 2018-07-24 at 19:14 +0300, Leonard Crestez wrote:
> > Changes since v2:
> >  * Print with dev_info if link fails on resume (Lucas)
> >  * Add a comment on imx7d pci irq mappings (Andrey)
> >  * Make imx6_pcie_ltssm_disable print an error on IMX6Q (Lucas)
> > 
> > The ltssm_disable does not return an error because it can't be usefully
> > handled, reversing partial suspend is a nightmare and unlikely to work.
> > 
> >  * Drop "reset: imx7: Fix always writing bits as 0 (accepted by Philipp)
> > 
> > Series is against linux-next tag next-20180724 where the reset patch was
> > already accepted. The imx7d.dtsi patch is also useful standalone.
> > 
> > Link to v2: https://lkml.org/lkml/2018/7/20/472
> 
> This is a gentle reminder that this series was reviewed by Lucas two
> weeks ago but not yet included.

Does this series have a functional dependency on the reset fix ? If yes
we can have a bisection proplem depending on which tree gets merged
first.

Please let me know.

Thanks,
Lorenzo
Leonard Crestez Aug. 8, 2018, 11:37 a.m. UTC | #3
On Wed, 2018-08-08 at 12:14 +0100, Lorenzo Pieralisi wrote:
> On Wed, Aug 08, 2018 at 10:53:52AM +0000, Leonard Crestez wrote:
> > On Tue, 2018-07-24 at 19:14 +0300, Leonard Crestez wrote:
> > > Changes since v2:
> > >  * Print with dev_info if link fails on resume (Lucas)
> > >  * Add a comment on imx7d pci irq mappings (Andrey)
> > >  * Make imx6_pcie_ltssm_disable print an error on IMX6Q (Lucas)
> > > 
> > > The ltssm_disable does not return an error because it can't be usefully
> > > handled, reversing partial suspend is a nightmare and unlikely to work.
> > > 
> > >  * Drop "reset: imx7: Fix always writing bits as 0 (accepted by Philipp)
> > > 
> > > Series is against linux-next tag next-20180724 where the reset patch was
> > > already accepted. The imx7d.dtsi patch is also useful standalone.
> > > 
> > > Link to v2: https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.org%2Flkml%2F2018%2F7%2F20%2F472&data=02%7C01%7Cleonard.crestez%40nxp.com%7Cc1e98512a50246c457a208d5fd2021dc%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636693236811248777&sdata=gxs1%2BfevIaBmQCVcJLUK41ml8CK2zLg%2FFKGGV%2F%2FHSLQ%3D&reserved=0
> > 
> > This is a gentle reminder that this series was reviewed by Lucas two
> > weeks ago but not yet included.
> 
> Does this series have a functional dependency on the reset fix ? If yes
> we can have a bisection proplem depending on which tree gets merged
> first.

Yes, without the reset fix I expect hangs.

Maybe the reset fix should be pulled in the pci tree? I don't know how
these issues are sorted out.
Lorenzo Pieralisi Aug. 8, 2018, 2:19 p.m. UTC | #4
On Wed, Aug 08, 2018 at 11:37:14AM +0000, Leonard Crestez wrote:
> On Wed, 2018-08-08 at 12:14 +0100, Lorenzo Pieralisi wrote:
> > On Wed, Aug 08, 2018 at 10:53:52AM +0000, Leonard Crestez wrote:
> > > On Tue, 2018-07-24 at 19:14 +0300, Leonard Crestez wrote:
> > > > Changes since v2:
> > > >  * Print with dev_info if link fails on resume (Lucas)
> > > >  * Add a comment on imx7d pci irq mappings (Andrey)
> > > >  * Make imx6_pcie_ltssm_disable print an error on IMX6Q (Lucas)
> > > > 
> > > > The ltssm_disable does not return an error because it can't be usefully
> > > > handled, reversing partial suspend is a nightmare and unlikely to work.
> > > > 
> > > >  * Drop "reset: imx7: Fix always writing bits as 0 (accepted by Philipp)
> > > > 
> > > > Series is against linux-next tag next-20180724 where the reset patch was
> > > > already accepted. The imx7d.dtsi patch is also useful standalone.
> > > > 
> > > > Link to v2: https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.org%2Flkml%2F2018%2F7%2F20%2F472&data=02%7C01%7Cleonard.crestez%40nxp.com%7Cc1e98512a50246c457a208d5fd2021dc%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636693236811248777&sdata=gxs1%2BfevIaBmQCVcJLUK41ml8CK2zLg%2FFKGGV%2F%2FHSLQ%3D&reserved=0
> > > 
> > > This is a gentle reminder that this series was reviewed by Lucas two
> > > weeks ago but not yet included.
> > 
> > Does this series have a functional dependency on the reset fix ? If yes
> > we can have a bisection proplem depending on which tree gets merged
> > first.
> 
> Yes, without the reset fix I expect hangs.
> 
> Maybe the reset fix should be pulled in the pci tree? I don't know how
> these issues are sorted out.

Well either I pull that fix into the PCI tree (but it is already -rc8
and I can't guarantee v4.19 inclusion - I shall try) or I can ack these
patches and Philippe will send them upstream on my behalf, atop the
reset fix above.

When will the reset fix will be sent to Linus ? Please let me know
how you want to proceed.

Thanks,
Lorenzo
Leonard Crestez Aug. 8, 2018, 2:58 p.m. UTC | #5
On Wed, 2018-08-08 at 15:19 +0100, Lorenzo Pieralisi wrote:
> On Wed, Aug 08, 2018 at 11:37:14AM +0000, Leonard Crestez wrote:
> > On Wed, 2018-08-08 at 12:14 +0100, Lorenzo Pieralisi wrote:
> > > On Wed, Aug 08, 2018 at 10:53:52AM +0000, Leonard Crestez wrote:
> > > > On Tue, 2018-07-24 at 19:14 +0300, Leonard Crestez wrote:
> > > > > Changes since v2:
> > > > >  * Print with dev_info if link fails on resume (Lucas)
> > > > >  * Add a comment on imx7d pci irq mappings (Andrey)
> > > > >  * Make imx6_pcie_ltssm_disable print an error on IMX6Q (Lucas)
> > > > > 
> > > > > The ltssm_disable does not return an error because it can't be usefully
> > > > > handled, reversing partial suspend is a nightmare and unlikely to work.
> > > > > 
> > > > >  * Drop "reset: imx7: Fix always writing bits as 0 (accepted by Philipp)
> > > > > 
> > > > > Series is against linux-next tag next-20180724 where the reset patch was
> > > > > already accepted. The imx7d.dtsi patch is also useful standalone.
> > > > 
> > > > This is a gentle reminder that this series was reviewed by Lucas two
> > > > weeks ago but not yet included.
> > > 
> > > Does this series have a functional dependency on the reset fix ? If yes
> > > we can have a bisection proplem depending on which tree gets merged
> > > first.
> > 
> > Yes, without the reset fix I expect hangs.
> > 
> > Maybe the reset fix should be pulled in the pci tree? I don't know how
> > these issues are sorted out.
> 
> Well either I pull that fix into the PCI tree (but it is already -rc8
> and I can't guarantee v4.19 inclusion - I shall try) or I can ack these
> patches and Philippe will send them upstream on my behalf, atop the
> reset fix above.
> 
> When will the reset fix will be sent to Linus ? Please let me know
> how you want to proceed.

I have some other unsent changes for imx pci+dts+reset standing by.
Since these changes qualify more as "pm features" than fixes I don't
really mind skipping 4.19.

Pulling pci features into the reset tree doesn't make much sense to me.

Maybe this kind of stuff could go through Shawn's tree? There shouldn't
be any interaction with non-imx stuff.
Lorenzo Pieralisi Aug. 8, 2018, 3:27 p.m. UTC | #6
On Wed, Aug 08, 2018 at 02:58:15PM +0000, Leonard Crestez wrote:
> On Wed, 2018-08-08 at 15:19 +0100, Lorenzo Pieralisi wrote:
> > On Wed, Aug 08, 2018 at 11:37:14AM +0000, Leonard Crestez wrote:
> > > On Wed, 2018-08-08 at 12:14 +0100, Lorenzo Pieralisi wrote:
> > > > On Wed, Aug 08, 2018 at 10:53:52AM +0000, Leonard Crestez wrote:
> > > > > On Tue, 2018-07-24 at 19:14 +0300, Leonard Crestez wrote:
> > > > > > Changes since v2:
> > > > > >  * Print with dev_info if link fails on resume (Lucas)
> > > > > >  * Add a comment on imx7d pci irq mappings (Andrey)
> > > > > >  * Make imx6_pcie_ltssm_disable print an error on IMX6Q (Lucas)
> > > > > > 
> > > > > > The ltssm_disable does not return an error because it can't be usefully
> > > > > > handled, reversing partial suspend is a nightmare and unlikely to work.
> > > > > > 
> > > > > >  * Drop "reset: imx7: Fix always writing bits as 0 (accepted by Philipp)
> > > > > > 
> > > > > > Series is against linux-next tag next-20180724 where the reset patch was
> > > > > > already accepted. The imx7d.dtsi patch is also useful standalone.
> > > > > 
> > > > > This is a gentle reminder that this series was reviewed by Lucas two
> > > > > weeks ago but not yet included.
> > > > 
> > > > Does this series have a functional dependency on the reset fix ? If yes
> > > > we can have a bisection proplem depending on which tree gets merged
> > > > first.
> > > 
> > > Yes, without the reset fix I expect hangs.
> > > 
> > > Maybe the reset fix should be pulled in the pci tree? I don't know how
> > > these issues are sorted out.
> > 
> > Well either I pull that fix into the PCI tree (but it is already -rc8
> > and I can't guarantee v4.19 inclusion - I shall try) or I can ack these
> > patches and Philippe will send them upstream on my behalf, atop the
> > reset fix above.
> > 
> > When will the reset fix will be sent to Linus ? Please let me know
> > how you want to proceed.
> 
> I have some other unsent changes for imx pci+dts+reset standing by.
> Since these changes qualify more as "pm features" than fixes I don't
> really mind skipping 4.19.

OK that's fine by me, resend this series and the new patches at
v4.19-rc1 and I will pull the changes into the pci tree then.

> Pulling pci features into the reset tree doesn't make much sense to me.

I was just giving you an option.

> Maybe this kind of stuff could go through Shawn's tree? There shouldn't
> be any interaction with non-imx stuff.

As I said, I can ack the changes for Shawn to pick them up but if
Shawn's tree goes to Linus via eg arm-soc we are back to square
one, it is just too late.

I have no problem taking these changes in the PCI tree for v4.20 as long
as we plan the dependencies in advance.

Lorenzo
Leonard Crestez Aug. 10, 2018, 11:11 a.m. UTC | #7
On Wed, 2018-08-08 at 16:27 +0100, Lorenzo Pieralisi wrote:
> On Wed, Aug 08, 2018 at 02:58:15PM +0000, Leonard Crestez wrote:
> > On Wed, 2018-08-08 at 15:19 +0100, Lorenzo Pieralisi wrote:
> > > On Wed, Aug 08, 2018 at 11:37:14AM +0000, Leonard Crestez wrote:
> > > > On Wed, 2018-08-08 at 12:14 +0100, Lorenzo Pieralisi wrote:
> > > > > On Wed, Aug 08, 2018 at 10:53:52AM +0000, Leonard Crestez wrote:
> > > > > > On Tue, 2018-07-24 at 19:14 +0300, Leonard Crestez wrote:
> > > > > > > Changes since v2:
> > > > > > >  * Print with dev_info if link fails on resume (Lucas)
> > > > > > >  * Add a comment on imx7d pci irq mappings (Andrey)
> > > > > > >  * Make imx6_pcie_ltssm_disable print an error on IMX6Q (Lucas)
> > > > > > > 
> > > > > > > The ltssm_disable does not return an error because it can't be usefully
> > > > > > > handled, reversing partial suspend is a nightmare and unlikely to work.
> > > > > > > 
> > > > > > >  * Drop "reset: imx7: Fix always writing bits as 0 (accepted by Philipp)
> > > > > > > 
> > > > > > > Series is against linux-next tag next-20180724 where the reset patch was
> > > > > > > already accepted. The imx7d.dtsi patch is also useful standalone.
> > > > > > 
> > > > > > This is a gentle reminder that this series was reviewed by Lucas two
> > > > > > weeks ago but not yet included.
> > > > > 
> > > > > Does this series have a functional dependency on the reset fix ? If yes
> > > > > we can have a bisection proplem depending on which tree gets merged
> > > > > first.
> > > > 
> > > > Yes, without the reset fix I expect hangs.

This needs further clarification: without the reset patch this will
hang on imx7 suspend/resume but this is the current behavior anyway.

Both patches are required for imx7 pci suspend and including them out
of order shouldn't break unrelated functionality.

So it should actually be fine to just include the pci changes now,
right?
Lorenzo Pieralisi Aug. 10, 2018, 3:13 p.m. UTC | #8
On Fri, Aug 10, 2018 at 11:11:55AM +0000, Leonard Crestez wrote:

[...]

> This needs further clarification: without the reset patch this will
> hang on imx7 suspend/resume but this is the current behavior anyway.
> 
> Both patches are required for imx7 pci suspend and including them out
> of order shouldn't break unrelated functionality.
> 
> So it should actually be fine to just include the pci changes now,
> right?

It is a bit late for v4.19 but before considering that, this series
is actually a set of fixes, correct ? If that's the case I think
Shawn can send them upstream for a v4.19-rc*, I can ACK them if that
sounds reasonable.

Patch (2) subject must make clear you are actually fixing a problem if
we choose this course of action.

Lorenzo
Shawn Guo Aug. 21, 2018, 3:26 p.m. UTC | #9
On Fri, Aug 10, 2018 at 04:13:41PM +0100, Lorenzo Pieralisi wrote:
> On Fri, Aug 10, 2018 at 11:11:55AM +0000, Leonard Crestez wrote:
> 
> [...]
> 
> > This needs further clarification: without the reset patch this will
> > hang on imx7 suspend/resume but this is the current behavior anyway.
> > 
> > Both patches are required for imx7 pci suspend and including them out
> > of order shouldn't break unrelated functionality.
> > 
> > So it should actually be fine to just include the pci changes now,
> > right?
> 
> It is a bit late for v4.19 but before considering that, this series
> is actually a set of fixes, correct ? If that's the case I think
> Shawn can send them upstream for a v4.19-rc*, I can ACK them if that
> sounds reasonable.
> 
> Patch (2) subject must make clear you are actually fixing a problem if
> we choose this course of action.

This is something never worked before, so it's a new feature to me.
That said, I'm not in the position to send the PCI patch for 4.19-rc as
a fix though IMX (arm-soc) tree.

I will send the DTS patch for 4.19-rc inclusion though, since that's
indeed a fix.

Shawn