[PATCH 1/2] Revert "spi/pl022: fix spi-pl022 pm enable at probe"

[PATCH 1/2] Revert "spi/pl022: fix spi-pl022 pm enable at probe"

[Ulf Hansson]

From: Ulf Hansson <ulf.hansson@linaro.org>

This reverts commit 6887237cd7da904184dab2750504040c68f3a080.

Commit "spi/pl022: enable runtime PM" introduced runtime PM issues.
Due to that commit, "spi/pl022: fix spi-pl022 pm enable at probe"
was merged to fix part of those issues.

Instead of adding another fix, let's clean up and revert everything
back to when it was already fine.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/spi/spi-pl022.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/spi/spi-pl022.c b/drivers/spi/spi-pl022.c
index 9194641..5cf0643 100644
--- a/drivers/spi/spi-pl022.c
+++ b/drivers/spi/spi-pl022.c
@@ -2186,6 +2186,7 @@ pl022_probe(struct amba_device *adev, const struct amba_id *id)
 	printk(KERN_INFO "pl022: mapped registers from 0x%08x to %p\n",
 	       adev->res.start, pl022->virtbase);
 
+	pm_runtime_enable(dev);
 	pm_runtime_resume(dev);
 
 	pl022->clk = devm_clk_get(&adev->dev, NULL);
-- 
1.7.10

[PATCH 2/2] Revert "spi/pl022: enable runtime PM"

[Ulf Hansson]

From: Ulf Hansson <ulf.hansson@linaro.org>

This reverts commit 2fb30d1147c599f5657e8c62c862f9a0f58d9d99.

This patch introduced runtime PM issues and is not needed at all.
Thus we should revert it.

Conflicts:

	drivers/spi/spi-pl022.c

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/spi/spi-pl022.c |    4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/spi/spi-pl022.c b/drivers/spi/spi-pl022.c
index 5cf0643..a1db91a 100644
--- a/drivers/spi/spi-pl022.c
+++ b/drivers/spi/spi-pl022.c
@@ -2186,9 +2186,6 @@ pl022_probe(struct amba_device *adev, const struct amba_id *id)
 	printk(KERN_INFO "pl022: mapped registers from 0x%08x to %p\n",
 	       adev->res.start, pl022->virtbase);
 
-	pm_runtime_enable(dev);
-	pm_runtime_resume(dev);
-
 	pl022->clk = devm_clk_get(&adev->dev, NULL);
 	if (IS_ERR(pl022->clk)) {
 		status = PTR_ERR(pl022->clk);
@@ -2293,7 +2290,6 @@ pl022_remove(struct amba_device *adev)
 
 	clk_disable(pl022->clk);
 	clk_unprepare(pl022->clk);
-	pm_runtime_disable(&adev->dev);
 	amba_release_regions(adev);
 	tasklet_disable(&pl022->pump_transfers);
 	spi_unregister_master(pl022->master);
-- 
1.7.10

[PATCH 2/2] Revert "spi/pl022: enable runtime PM"

[Ulf Hansson]

It seems like this patch did not get applied together with:
[PATCH 1/2] Revert "spi/pl022: fix spi-pl022 pm enable at probe"

The problem was likely my fault, since there were a lot of resends.
Anyway, it should still be possible to apply.

Kind regards
Ulf Hansson


On 5 October 2012 17:51, Ulf Hansson <ulf.hansson@stericsson.com> wrote:
> From: Ulf Hansson <ulf.hansson@linaro.org>
>
> This reverts commit 2fb30d1147c599f5657e8c62c862f9a0f58d9d99.
>
> This patch introduced runtime PM issues and is not needed at all.
> Thus we should revert it.
>
> Conflicts:
>
>         drivers/spi/spi-pl022.c
>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  drivers/spi/spi-pl022.c |    4 ----
>  1 file changed, 4 deletions(-)
>
> diff --git a/drivers/spi/spi-pl022.c b/drivers/spi/spi-pl022.c
> index 5cf0643..a1db91a 100644
> --- a/drivers/spi/spi-pl022.c
> +++ b/drivers/spi/spi-pl022.c
> @@ -2186,9 +2186,6 @@ pl022_probe(struct amba_device *adev, const struct amba_id *id)
>         printk(KERN_INFO "pl022: mapped registers from 0x%08x to %p\n",
>                adev->res.start, pl022->virtbase);
>
> -       pm_runtime_enable(dev);
> -       pm_runtime_resume(dev);
> -
>         pl022->clk = devm_clk_get(&adev->dev, NULL);
>         if (IS_ERR(pl022->clk)) {
>                 status = PTR_ERR(pl022->clk);
> @@ -2293,7 +2290,6 @@ pl022_remove(struct amba_device *adev)
>
>         clk_disable(pl022->clk);
>         clk_unprepare(pl022->clk);
> -       pm_runtime_disable(&adev->dev);
>         amba_release_regions(adev);
>         tasklet_disable(&pl022->pump_transfers);
>         spi_unregister_master(pl022->master);
> --
> 1.7.10
>

[PATCH 2/2] Revert "spi/pl022: enable runtime PM"

[Mark Brown]

On Tue, Oct 23, 2012 at 12:31:06PM +0200, Ulf Hansson wrote:
> It seems like this patch did not get applied together with:
> [PATCH 1/2] Revert "spi/pl022: fix spi-pl022 pm enable at probe"
> 
> The problem was likely my fault, since there were a lot of resends.
> Anyway, it should still be possible to apply.

I *really* should not need to remind you to:

 - Not top post
 - Send patches in a form which can be applied if you want them applying
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20121023/977c8645/attachment.sig>

[PATCH 2/2] Revert "spi/pl022: enable runtime PM"

[Russell King - ARM Linux]

On Tue, Oct 23, 2012 at 12:10:19PM +0100, Mark Brown wrote:
> On Tue, Oct 23, 2012 at 12:31:06PM +0200, Ulf Hansson wrote:
> > It seems like this patch did not get applied together with:
> > [PATCH 1/2] Revert "spi/pl022: fix spi-pl022 pm enable at probe"
> > 
> > The problem was likely my fault, since there were a lot of resends.
> > Anyway, it should still be possible to apply.
> 
> I *really* should not need to remind you to:
> 
>  - Not top post
>  - Send patches in a form which can be applied if you want them applying

How about actually applying the damned patch from the two-part patch set,
rather than only the first, both of which has been around for almost a
month now, and fixes what should never have gone into mainline in the
first place?

[PATCH 2/2] Revert "spi/pl022: enable runtime PM"

[Mark Brown]

On Tue, Oct 23, 2012 at 12:16:56PM +0100, Russell King - ARM Linux wrote:
> On Tue, Oct 23, 2012 at 12:10:19PM +0100, Mark Brown wrote:

> > I *really* should not need to remind you to:

> >  - Not top post
> >  - Send patches in a form which can be applied if you want them applying

> How about actually applying the damned patch from the two-part patch set,
> rather than only the first, both of which has been around for almost a
> month now, and fixes what should never have gone into mainline in the
> first place?

Well, the reason for the second point is that I don't have a copy of the
patch any more, I need a copy of it before I can do anything.  Half the
reason these contentless top posts are annoying is that even if there's
something to do it's not actionable.

I have to say I had been under the impression that Linus' series that I
applied the other day dealt with all the outstanding stuff here; the
issues with this have been the awful changelogs and the overalapping
sets of patches.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20121023/160443fc/attachment-0001.sig>

[PATCH 2/2] Revert "spi/pl022: enable runtime PM"

[Russell King - ARM Linux]

On Tue, Oct 23, 2012 at 12:41:31PM +0100, Mark Brown wrote:
> On Tue, Oct 23, 2012 at 12:16:56PM +0100, Russell King - ARM Linux wrote:
> > On Tue, Oct 23, 2012 at 12:10:19PM +0100, Mark Brown wrote:
> 
> > > I *really* should not need to remind you to:
> 
> > >  - Not top post
> > >  - Send patches in a form which can be applied if you want them applying
> 
> > How about actually applying the damned patch from the two-part patch set,
> > rather than only the first, both of which has been around for almost a
> > month now, and fixes what should never have gone into mainline in the
> > first place?
> 
> Well, the reason for the second point is that I don't have a copy of the
> patch any more, I need a copy of it before I can do anything.  Half the
> reason these contentless top posts are annoying is that even if there's
> something to do it's not actionable.
> 
> I have to say I had been under the impression that Linus' series that I
> applied the other day dealt with all the outstanding stuff here; the
> issues with this have been the awful changelogs and the overalapping
> sets of patches.

Given that Linus is the one who introduced the fuckups in the first place,
and afaics Linus' series has not been posted anywhere I can see, and I
have been through the issues and worked with Ulf to get them fixed, I find
that to be down right insulting by you.

You received *BOTH* of Ulf's patches on the 5th October.  Both needed
to be applied *TOGETHER* to reduce the window where things got worse
before they got better.  But for some goddamned unknown reason you decided
to only take the first but totally and utterly ignored the second.

Well done, you've probably crapped this driver up again.  Way to go Mark.
Now, tell us _exactly_ what patches you've applied to this driver.  Post
them to this mailing list so we can see them, and see what state this
driver is now in.

IMHO, you're being a terrible "maintainer" here - and this is being shown
by the need to get you to post all the patches you've currently queued up
for this.

[PATCH 2/2] Revert "spi/pl022: enable runtime PM"

[Russell King - ARM Linux]

On Tue, Oct 23, 2012 at 12:47:23PM +0100, Russell King - ARM Linux wrote:
> On Tue, Oct 23, 2012 at 12:41:31PM +0100, Mark Brown wrote:
> > On Tue, Oct 23, 2012 at 12:16:56PM +0100, Russell King - ARM Linux wrote:
> > > On Tue, Oct 23, 2012 at 12:10:19PM +0100, Mark Brown wrote:
> > 
> > > > I *really* should not need to remind you to:
> > 
> > > >  - Not top post
> > > >  - Send patches in a form which can be applied if you want them applying
> > 
> > > How about actually applying the damned patch from the two-part patch set,
> > > rather than only the first, both of which has been around for almost a
> > > month now, and fixes what should never have gone into mainline in the
> > > first place?
> > 
> > Well, the reason for the second point is that I don't have a copy of the
> > patch any more, I need a copy of it before I can do anything.  Half the
> > reason these contentless top posts are annoying is that even if there's
> > something to do it's not actionable.
> > 
> > I have to say I had been under the impression that Linus' series that I
> > applied the other day dealt with all the outstanding stuff here; the
> > issues with this have been the awful changelogs and the overalapping
> > sets of patches.
> 
> Given that Linus is the one who introduced the fuckups in the first place,
> and afaics Linus' series has not been posted anywhere I can see, and I
> have been through the issues and worked with Ulf to get them fixed, I find
> that to be down right insulting by you.
> 
> You received *BOTH* of Ulf's patches on the 5th October.  Both needed
> to be applied *TOGETHER* to reduce the window where things got worse
> before they got better.  But for some goddamned unknown reason you decided
> to only take the first but totally and utterly ignored the second.
> 
> Well done, you've probably crapped this driver up again.  Way to go Mark.
> Now, tell us _exactly_ what patches you've applied to this driver.  Post
> them to this mailing list so we can see them, and see what state this
> driver is now in.
> 
> IMHO, you're being a terrible "maintainer" here - and this is being shown
> by the need to get you to post all the patches you've currently queued up
> for this.

Oh, and if you're unwilling to do that, then I'm taking both of Ulf's
patches, committing them to my fixes branch, and sending them off to
Linus tonight - because they are _NEEDED_ for the -rc series to fix
this driver.

[PATCH 2/2] Revert "spi/pl022: enable runtime PM"

[Mark Brown]

On Tue, Oct 23, 2012 at 12:48:42PM +0100, Russell King - ARM Linux wrote:

> Oh, and if you're unwilling to do that, then I'm taking both of Ulf's
> patches, committing them to my fixes branch, and sending them off to
> Linus tonight - because they are _NEEDED_ for the -rc series to fix
> this driver.

As I said in my previous e-mail I've now done the revert myself; I'll
send the fixes to Linus.  If you wish to check the other patches that
are queued they're in -next like I said; I'd appreciate someone who's
actively using the driver taking a look at them.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20121023/8bb0e238/attachment.sig>

[PATCH 2/2] Revert "spi/pl022: enable runtime PM"

[Mark Brown]

On Tue, Oct 23, 2012 at 12:47:23PM +0100, Russell King - ARM Linux wrote:
> On Tue, Oct 23, 2012 at 12:41:31PM +0100, Mark Brown wrote:

> > I have to say I had been under the impression that Linus' series that I
> > applied the other day dealt with all the outstanding stuff here; the
> > issues with this have been the awful changelogs and the overalapping
> > sets of patches.

> Given that Linus is the one who introduced the fuckups in the first place,
> and afaics Linus' series has not been posted anywhere I can see, and I
> have been through the issues and worked with Ulf to get them fixed, I find
> that to be down right insulting by you.

Linus' patches were all posted to spi-devel-general.  I can understand
why you wouldn't subscribe to it given that it's a sourceforge list and
hence full of spam but it is the list of record for SPI unfortunately.

> You received *BOTH* of Ulf's patches on the 5th October.  Both needed
> to be applied *TOGETHER* to reduce the window where things got worse
> before they got better.  But for some goddamned unknown reason you decided
> to only take the first but totally and utterly ignored the second.

Unfortunately Ulf's original posting of the patches didn't include any
analysis of the problems or explanation as to why the reverts were
needed.  Since I had no idea what the patches were supposed to do having
not been involved in whatever discussion of the bugs were I didn't apply
them then.

Ulf did repost with a slightly clarified changelog and I *suspect* that
the second patch just got fat fingered due to the multiple different
revisions of the series that were floating around.

> Well done, you've probably crapped this driver up again.  Way to go Mark.
> Now, tell us _exactly_ what patches you've applied to this driver.  Post
> them to this mailing list so we can see them, and see what state this
> driver is now in.

> IMHO, you're being a terrible "maintainer" here - and this is being shown
> by the need to get you to post all the patches you've currently queued up
> for this.

As one would expect all the patches that are currently sitting there for
the driver are in -next already; one of the nice things about -next is
that it does tend to help remove doubt about things like this.

You just pointed me at the discussion of the original issue that
happened prior to Ulf's revert posting; I now see what's going on, your
analysis is crystal clear [1].  I'm just generating the extra revert of
2fb30d (spi/pl022: enable runtime PM) which I'll squash down into the
existing revert and send to Linus tomorrow.

[1] http://lists.arm.linux.org.uk/lurker/message/20120927.152412.d281a899.en.html
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20121023/7acb8549/attachment-0001.sig>

[PATCH 2/2] Revert "spi/pl022: enable runtime PM"

[Linus Walleij]

On Tue, Oct 23, 2012 at 1:41 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:

> I have to say I had been under the impression that Linus' series that I
> applied the other day dealt with all the outstanding stuff here; the
> issues with this have been the awful changelogs and the overalapping
> sets of patches.

It did I think it was patch [1/4] (same subject as this thread),
http://sourceforge.net/mailarchive/forum.php?thread_name=1350476818-13056-1-git-send-email-linus.walleij%40stericsson.com&forum_name=spi-devel-general
to which you replied:

> This was already applied, applied the rest thanks.

So it probably wasn't and it's just some mishap so
that 2,3,4 /4 were applied while 1/4 was actually needed
to.

Yours,
Linus Walleij

[PATCH 1/2] Revert "spi/pl022: fix spi-pl022 pm enable at probe"

[Mark Brown]

On Fri, Oct 05, 2012 at 05:51:11PM +0200, Ulf Hansson wrote:
> From: Ulf Hansson <ulf.hansson@linaro.org>
> 
> This reverts commit 6887237cd7da904184dab2750504040c68f3a080.

Applied, thanks.