diff mbox

[V2] ARM: dove: dt: revert PMU interrupt controller node

Message ID 1392667236-19126-1-git-send-email-jason@lakedaemon.net (mailing list archive)
State New, archived
Headers show

Commit Message

Jason Cooper Feb. 17, 2014, 8 p.m. UTC
The corresponding driver didn't make it into v3.14, so we need to remove
the node.  Dove systems fail to boot with the node present and no
driver.

This node will be re-added when the driver makes it to mainline.

Signed-off-by: Jason Cooper <jason@lakedaemon.net>
---
Changes from v1:

  - removed the reference to pmu_intc.

Destined for the mvebu/dt-fixes branch for v3.14, node will be re-added once
the driver hits mainline for v3.15

 arch/arm/boot/dts/dove.dtsi | 11 -----------
 1 file changed, 11 deletions(-)

Comments

Russell King - ARM Linux March 3, 2014, 3:02 p.m. UTC | #1
On Mon, Feb 17, 2014 at 08:00:36PM +0000, Jason Cooper wrote:
> The corresponding driver didn't make it into v3.14, so we need to remove
> the node.  Dove systems fail to boot with the node present and no
> driver.
> 
> This node will be re-added when the driver makes it to mainline.

I'm going to stick my oar in on this and ask what is a very fundamental
question.

If we're adding the PMU interrupt controller as a separate "device"
aren't we describing our implementation rather than the hardware?  It
isn't a separate device as far as the description of it in the reference
manuals.

Moreover, should the PMU interrupt controller be something which is
handled by a separate chunk of code to a driver for the PMU as a whole,
or are we storing up problems with resource clashes?  I can quite see
a PMU driver coming along in the future offering a pair of generic
power domains for the GPU and VPU, and such a driver would need to map
all the PMU registers so it can access the power control, reset and
isolator registers.
Andrew Lunn March 3, 2014, 5:37 p.m. UTC | #2
On Mon, Mar 03, 2014 at 03:02:15PM +0000, Russell King - ARM Linux wrote:
> On Mon, Feb 17, 2014 at 08:00:36PM +0000, Jason Cooper wrote:
> > The corresponding driver didn't make it into v3.14, so we need to remove
> > the node.  Dove systems fail to boot with the node present and no
> > driver.
> > 
> > This node will be re-added when the driver makes it to mainline.
> 
> I'm going to stick my oar in on this and ask what is a very fundamental
> question.
> 
> If we're adding the PMU interrupt controller as a separate "device"
> aren't we describing our implementation rather than the hardware?  It
> isn't a separate device as far as the description of it in the reference
> manuals.
> 
> Moreover, should the PMU interrupt controller be something which is
> handled by a separate chunk of code to a driver for the PMU as a whole,
> or are we storing up problems with resource clashes?  I can quite see
> a PMU driver coming along in the future offering a pair of generic
> power domains for the GPU and VPU, and such a driver would need to map
> all the PMU registers so it can access the power control, reset and
> isolator registers.

Hi Russell

I suspect you are right, we are storing up problems.

During the 4 months between submitting this driver and actually
getting it accepted, i've learned quite a bit. I tried to implement
cpufreq for Dove and ran into the problems you mention. The registers
in the PMU are interleaved so that you cannot cleanly separate out the
range needed for cpufreq. We probably need a PMU device, which exports
a register syscon and have the interrupt controller make use of it.

	Andrew
Russell King - ARM Linux March 3, 2014, 6:15 p.m. UTC | #3
On Mon, Mar 03, 2014 at 06:37:40PM +0100, Andrew Lunn wrote:
> On Mon, Mar 03, 2014 at 03:02:15PM +0000, Russell King - ARM Linux wrote:
> > On Mon, Feb 17, 2014 at 08:00:36PM +0000, Jason Cooper wrote:
> > > The corresponding driver didn't make it into v3.14, so we need to remove
> > > the node.  Dove systems fail to boot with the node present and no
> > > driver.
> > > 
> > > This node will be re-added when the driver makes it to mainline.
> > 
> > I'm going to stick my oar in on this and ask what is a very fundamental
> > question.
> > 
> > If we're adding the PMU interrupt controller as a separate "device"
> > aren't we describing our implementation rather than the hardware?  It
> > isn't a separate device as far as the description of it in the reference
> > manuals.
> > 
> > Moreover, should the PMU interrupt controller be something which is
> > handled by a separate chunk of code to a driver for the PMU as a whole,
> > or are we storing up problems with resource clashes?  I can quite see
> > a PMU driver coming along in the future offering a pair of generic
> > power domains for the GPU and VPU, and such a driver would need to map
> > all the PMU registers so it can access the power control, reset and
> > isolator registers.
> 
> Hi Russell
> 
> I suspect you are right, we are storing up problems.
> 
> During the 4 months between submitting this driver and actually
> getting it accepted, i've learned quite a bit. I tried to implement
> cpufreq for Dove and ran into the problems you mention. The registers
> in the PMU are interleaved so that you cannot cleanly separate out the
> range needed for cpufreq. We probably need a PMU device, which exports
> a register syscon and have the interrupt controller make use of it.

I know it's a pain to say this, but maybe we should hold off with the
PMU IRQ controller patch for a while longer until we get a proper idea
of what we're doing with the PMU?

If you have a look at my "Generic PM domains - too noisy?" email earlier
today on LAKML, you can see some of the code I'm talking about above - I
have that running at the moment on my CuBox.  I'm going to be doing some
power measurements soon with various different autosuspend delays to see
what effect it has.

If we do end up with a PMU driver, then I think that unless there's a
real need, it should just integrate things like the PM domains, IRQ
controller and maybe (depending on how complex it is) the cpufreq bits.
MFD is all very well, but I feel that sometimes it brings along
additional complexity at times when that complexity isn't needed.
Jason Cooper March 3, 2014, 10:24 p.m. UTC | #4
On Mon, Mar 03, 2014 at 06:15:30PM +0000, Russell King - ARM Linux wrote:
> On Mon, Mar 03, 2014 at 06:37:40PM +0100, Andrew Lunn wrote:
> > On Mon, Mar 03, 2014 at 03:02:15PM +0000, Russell King - ARM Linux wrote:
> > > On Mon, Feb 17, 2014 at 08:00:36PM +0000, Jason Cooper wrote:
> > > > The corresponding driver didn't make it into v3.14, so we need to remove
> > > > the node.  Dove systems fail to boot with the node present and no
> > > > driver.
> > > > 
> > > > This node will be re-added when the driver makes it to mainline.
> > > 
> > > I'm going to stick my oar in on this and ask what is a very fundamental
> > > question.

Better now than later ;-)

> > > If we're adding the PMU interrupt controller as a separate "device"
> > > aren't we describing our implementation rather than the hardware?  It
> > > isn't a separate device as far as the description of it in the reference
> > > manuals.

I could have sworn this was discussed with this particular patchset, but
I'm unable to find the conversation in my archives.  Neither during the
patch submission process, nor the (long) pull request thread.

Perhaps it was an irc conversation?  Andrew, Sebastian, can you find a
link?  iirc, one of the DT maintainers (Mark Rutland?) raised the same
concern and I thought we answered that sufficiently...

> > > Moreover, should the PMU interrupt controller be something which is
> > > handled by a separate chunk of code to a driver for the PMU as a whole,
> > > or are we storing up problems with resource clashes?  I can quite see
> > > a PMU driver coming along in the future offering a pair of generic
> > > power domains for the GPU and VPU, and such a driver would need to map
> > > all the PMU registers so it can access the power control, reset and
> > > isolator registers.
> > 
> > Hi Russell
> > 
> > I suspect you are right, we are storing up problems.
> > 
> > During the 4 months between submitting this driver and actually
> > getting it accepted, i've learned quite a bit. I tried to implement
> > cpufreq for Dove and ran into the problems you mention. The registers
> > in the PMU are interleaved so that you cannot cleanly separate out the
> > range needed for cpufreq. We probably need a PMU device, which exports
> > a register syscon and have the interrupt controller make use of it.

This isn't the first time we've had this problem with Marvell SoCs.  We
added

  c5ca95b507c8 ARM: 7930/1: Introduce atomic MMIO modify

To work around a much smaller version of this similar problem.

> I know it's a pain to say this, but maybe we should hold off with the
> PMU IRQ controller patch for a while longer until we get a proper idea
> of what we're doing with the PMU?

I've no issue with reverting this driver.  I'll ask arm-soc to hold off
on pulling the latest mvebu DT pull request which contains the DT node.
I _really_ don't want to do a revert of a revert of a revert. :-/

thx,

Jason.
Jason Cooper March 4, 2014, 3:08 a.m. UTC | #5
On Mon, Mar 03, 2014 at 05:24:06PM -0500, Jason Cooper wrote:
> I've no issue with reverting this driver.  I'll ask arm-soc to hold off
> on pulling the latest mvebu DT pull request which contains the DT node.
> I _really_ don't want to do a revert of a revert of a revert. :-/

Done.  I'll work on reverting the driver tomorrow.  It's currently in
tip/irq/core queued for v3.15.

thx,

Jason.
Andrew Lunn March 4, 2014, 9:26 a.m. UTC | #6
> I could have sworn this was discussed with this particular patchset, but
> I'm unable to find the conversation in my archives.  Neither during the
> patch submission process, nor the (long) pull request thread.
> 
> Perhaps it was an irc conversation?  Andrew, Sebastian, can you find a
> link?  iirc, one of the DT maintainers (Mark Rutland?) raised the same
> concern and I thought we answered that sufficiently...

Hi Jason

It was the cpufreq driver which caused the discussion. I looked at it
for a while, and then task swapped onto the kirkwood move into
mach-mvebu.

    Andrew
Sebastian Hesselbarth March 4, 2014, 10:39 a.m. UTC | #7
On 03/04/2014 10:26 AM, Andrew Lunn wrote:
>> I could have sworn this was discussed with this particular patchset, but
>> I'm unable to find the conversation in my archives.  Neither during the
>> patch submission process, nor the (long) pull request thread.
>>
>> Perhaps it was an irc conversation?  Andrew, Sebastian, can you find a
>> link?  iirc, one of the DT maintainers (Mark Rutland?) raised the same
>> concern and I thought we answered that sufficiently...
>
> It was the cpufreq driver which caused the discussion. I looked at it
> for a while, and then task swapped onto the kirkwood move into
> mach-mvebu.

I guess you are looking for this discussion

http://comments.gmane.org/gmane.linux.power-management.general/41053

and specifically Mark's remarks on PMU and DT in here

http://permalink.gmane.org/gmane.linux.ports.arm.kernel/285384

BTW, +1 for a single PMU node that either serves an mfd (or type
of) driver or that subsystem drivers derive their resources from.
Looking at Dove FS, that would also include clock gating, which
could be a mess to sort out.. anyway, let's get it on.

Sebastian
Russell King - ARM Linux March 4, 2014, 12:11 p.m. UTC | #8
On Tue, Mar 04, 2014 at 11:39:43AM +0100, Sebastian Hesselbarth wrote:
> On 03/04/2014 10:26 AM, Andrew Lunn wrote:
>>> I could have sworn this was discussed with this particular patchset, but
>>> I'm unable to find the conversation in my archives.  Neither during the
>>> patch submission process, nor the (long) pull request thread.
>>>
>>> Perhaps it was an irc conversation?  Andrew, Sebastian, can you find a
>>> link?  iirc, one of the DT maintainers (Mark Rutland?) raised the same
>>> concern and I thought we answered that sufficiently...
>>
>> It was the cpufreq driver which caused the discussion. I looked at it
>> for a while, and then task swapped onto the kirkwood move into
>> mach-mvebu.
>
> I guess you are looking for this discussion
>
> http://comments.gmane.org/gmane.linux.power-management.general/41053
>
> and specifically Mark's remarks on PMU and DT in here
>
> http://permalink.gmane.org/gmane.linux.ports.arm.kernel/285384
>
> BTW, +1 for a single PMU node that either serves an mfd (or type
> of) driver or that subsystem drivers derive their resources from.
> Looking at Dove FS, that would also include clock gating, which
> could be a mess to sort out.. anyway, let's get it on.

So we have cpufreq, pm domains and an irq controller.  What's the plan
for this, who's going to look at sorting this out?
Jason Cooper March 4, 2014, 1:53 p.m. UTC | #9
On Tue, Mar 04, 2014 at 12:11:36PM +0000, Russell King - ARM Linux wrote:
> On Tue, Mar 04, 2014 at 11:39:43AM +0100, Sebastian Hesselbarth wrote:
> > On 03/04/2014 10:26 AM, Andrew Lunn wrote:
> >>> I could have sworn this was discussed with this particular patchset, but
> >>> I'm unable to find the conversation in my archives.  Neither during the
> >>> patch submission process, nor the (long) pull request thread.
> >>>
> >>> Perhaps it was an irc conversation?  Andrew, Sebastian, can you find a
> >>> link?  iirc, one of the DT maintainers (Mark Rutland?) raised the same
> >>> concern and I thought we answered that sufficiently...
> >>
> >> It was the cpufreq driver which caused the discussion. I looked at it
> >> for a while, and then task swapped onto the kirkwood move into
> >> mach-mvebu.
> >
> > I guess you are looking for this discussion
> >
> > http://comments.gmane.org/gmane.linux.power-management.general/41053
> >
> > and specifically Mark's remarks on PMU and DT in here
> >
> > http://permalink.gmane.org/gmane.linux.ports.arm.kernel/285384
> >
> > BTW, +1 for a single PMU node that either serves an mfd (or type
> > of) driver or that subsystem drivers derive their resources from.
> > Looking at Dove FS, that would also include clock gating, which
> > could be a mess to sort out.. anyway, let's get it on.
> 
> So we have cpufreq, pm domains and an irq controller.  What's the plan
> for this, who's going to look at sorting this out?

Andrew, Sebastian?  I'm currently task-saturated...

thx,

Jason.
Andrew Lunn March 4, 2014, 1:54 p.m. UTC | #10
> > So we have cpufreq, pm domains and an irq controller.  What's the plan
> > for this, who's going to look at sorting this out?
> 
> Andrew, Sebastian?  I'm currently task-saturated...

I doubt i will be doing anything with it for the remainder of this
cycle. I would like to finish converting kirkwood to DT before
starting on anything new.

	 Andrew
Russell King - ARM Linux March 4, 2014, 2:01 p.m. UTC | #11
On Tue, Mar 04, 2014 at 02:54:52PM +0100, Andrew Lunn wrote:
> > > So we have cpufreq, pm domains and an irq controller.  What's the plan
> > > for this, who's going to look at sorting this out?
> > 
> > Andrew, Sebastian?  I'm currently task-saturated...
> 
> I doubt i will be doing anything with it for the remainder of this
> cycle. I would like to finish converting kirkwood to DT before
> starting on anything new.

Okay, can someone send me the cpufreq code then please?
Sebastian Hesselbarth March 4, 2014, 2:02 p.m. UTC | #12
On 03/04/2014 02:53 PM, Jason Cooper wrote:
> On Tue, Mar 04, 2014 at 12:11:36PM +0000, Russell King - ARM Linux wrote:
>> On Tue, Mar 04, 2014 at 11:39:43AM +0100, Sebastian Hesselbarth wrote:
>>> On 03/04/2014 10:26 AM, Andrew Lunn wrote:
>>>>> I could have sworn this was discussed with this particular patchset, but
>>>>> I'm unable to find the conversation in my archives.  Neither during the
>>>>> patch submission process, nor the (long) pull request thread.
>>>>>
>>>>> Perhaps it was an irc conversation?  Andrew, Sebastian, can you find a
>>>>> link?  iirc, one of the DT maintainers (Mark Rutland?) raised the same
>>>>> concern and I thought we answered that sufficiently...
>>>>
>>>> It was the cpufreq driver which caused the discussion. I looked at it
>>>> for a while, and then task swapped onto the kirkwood move into
>>>> mach-mvebu.
>>>
>>> I guess you are looking for this discussion
>>>
>>> http://comments.gmane.org/gmane.linux.power-management.general/41053
>>>
>>> and specifically Mark's remarks on PMU and DT in here
>>>
>>> http://permalink.gmane.org/gmane.linux.ports.arm.kernel/285384
>>>
>>> BTW, +1 for a single PMU node that either serves an mfd (or type
>>> of) driver or that subsystem drivers derive their resources from.
>>> Looking at Dove FS, that would also include clock gating, which
>>> could be a mess to sort out.. anyway, let's get it on.
>>
>> So we have cpufreq, pm domains and an irq controller.  What's the plan
>> for this, who's going to look at sorting this out?
>
> Andrew, Sebastian?  I'm currently task-saturated...

Phew, looks like I'll have to take it?

Are you guys ok with having a single PMU node with syscon provided
regmap and make all drivers depend on it? I'd like to get a go from
Russell here, as he has clearly something in mind.

We can consolidate drivers later if required. If we start that now,
we definitely risk running out of time for v3.15.

Sebastian
Jason Cooper March 4, 2014, 2:18 p.m. UTC | #13
On Tue, Mar 04, 2014 at 03:02:25PM +0100, Sebastian Hesselbarth wrote:
> On 03/04/2014 02:53 PM, Jason Cooper wrote:
> >On Tue, Mar 04, 2014 at 12:11:36PM +0000, Russell King - ARM Linux wrote:
> >>On Tue, Mar 04, 2014 at 11:39:43AM +0100, Sebastian Hesselbarth wrote:
> >>>On 03/04/2014 10:26 AM, Andrew Lunn wrote:
> >>>>>I could have sworn this was discussed with this particular patchset, but
> >>>>>I'm unable to find the conversation in my archives.  Neither during the
> >>>>>patch submission process, nor the (long) pull request thread.
> >>>>>
> >>>>>Perhaps it was an irc conversation?  Andrew, Sebastian, can you find a
> >>>>>link?  iirc, one of the DT maintainers (Mark Rutland?) raised the same
> >>>>>concern and I thought we answered that sufficiently...
> >>>>
> >>>>It was the cpufreq driver which caused the discussion. I looked at it
> >>>>for a while, and then task swapped onto the kirkwood move into
> >>>>mach-mvebu.
> >>>
> >>>I guess you are looking for this discussion
> >>>
> >>>http://comments.gmane.org/gmane.linux.power-management.general/41053
> >>>
> >>>and specifically Mark's remarks on PMU and DT in here
> >>>
> >>>http://permalink.gmane.org/gmane.linux.ports.arm.kernel/285384
> >>>
> >>>BTW, +1 for a single PMU node that either serves an mfd (or type
> >>>of) driver or that subsystem drivers derive their resources from.
> >>>Looking at Dove FS, that would also include clock gating, which
> >>>could be a mess to sort out.. anyway, let's get it on.
> >>
> >>So we have cpufreq, pm domains and an irq controller.  What's the plan
> >>for this, who's going to look at sorting this out?
> >
> >Andrew, Sebastian?  I'm currently task-saturated...
> 
> Phew, looks like I'll have to take it?
> 
> Are you guys ok with having a single PMU node with syscon provided
> regmap and make all drivers depend on it? I'd like to get a go from
> Russell here, as he has clearly something in mind.
> 
> We can consolidate drivers later if required. If we start that now,
> we definitely risk running out of time for v3.15.

Honestly, mvebu has enough going on atm.  I would target v3.16 and take
our time.  At least with the binding.  If the driver comes together
easily, we could always do like we did for mbus.  Do a legacy init from
the board (soc) file, then switch to the dt binding once everyone is
happy with it.

v3.14-rc6 is less than a week away, so I'll be sending final mvebu pull
requests late Wednesday/Thursday...

v3.14-rcX has been awful quiet, there might not be an -rc7.

thx,

Jason.
Andrew Lunn March 4, 2014, 2:41 p.m. UTC | #14
On Tue, Mar 04, 2014 at 02:01:36PM +0000, Russell King - ARM Linux wrote:
> On Tue, Mar 04, 2014 at 02:54:52PM +0100, Andrew Lunn wrote:
> > > > So we have cpufreq, pm domains and an irq controller.  What's the plan
> > > > for this, who's going to look at sorting this out?
> > > 
> > > Andrew, Sebastian?  I'm currently task-saturated...
> > 
> > I doubt i will be doing anything with it for the remainder of this
> > cycle. I would like to finish converting kirkwood to DT before
> > starting on anything new.
> 
> Okay, can someone send me the cpufreq code then please?

Hi Russell

Take a look at:

https://github.com/lunn/linux.git v3.13-rc2-rafael-next-dove-cpufreq

The cpufreq drivers/framework have been going through quite a bit of
refactoring recently, so maybe some work will be needed on it.

	    Andrew
diff mbox

Patch

diff --git a/arch/arm/boot/dts/dove.dtsi b/arch/arm/boot/dts/dove.dtsi
index 2b76524f4aa7..187fd46b7b5e 100644
--- a/arch/arm/boot/dts/dove.dtsi
+++ b/arch/arm/boot/dts/dove.dtsi
@@ -379,15 +379,6 @@ 
 				#clock-cells = <1>;
 			};
 
-			pmu_intc: pmu-interrupt-ctrl@d0050 {
-				compatible = "marvell,dove-pmu-intc";
-				interrupt-controller;
-				#interrupt-cells = <1>;
-				reg = <0xd0050 0x8>;
-				interrupts = <33>;
-				marvell,#interrupts = <7>;
-			};
-
 			pinctrl: pin-ctrl@d0200 {
 				compatible = "marvell,dove-pinctrl";
 				reg = <0xd0200 0x10>;
@@ -610,8 +601,6 @@ 
 			rtc: real-time-clock@d8500 {
 				compatible = "marvell,orion-rtc";
 				reg = <0xd8500 0x20>;
-				interrupt-parent = <&pmu_intc>;
-				interrupts = <5>;
 			};
 
 			gpio2: gpio-ctrl@e8400 {