Re: [PATCH] cpuidle: kirkwood: Move out of mach directory, add DT.

[Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org>]

On Fri, Dec 28, 2012 at 08:55:31AM -0600, Rob Herring wrote:
> On 12/28/2012 08:35 AM, Andrew Lunn wrote:
> > On Fri, Dec 28, 2012 at 08:18:42AM -0600, Rob Herring wrote:
> >> On 12/28/2012 06:47 AM, Andrew Lunn wrote:
> >>> Move the Kirkwood cpuidle driver out of arch/arm/mach-kirkwood and
> >>> into drivers/cpuidle. Convert the driver into a platform driver and
> >>> add a device tree binding. Add a DT node to instantiate the driver for
> >>> boards converted to DT, and a platform data for old style boards.
> >>
> >> Is this an old comment? I don't see any platform data.
> > 
> > There is no platform data, since all the driver needs is an address of
> > the DDR control register. The code to create a platform device entry
> > is in common.c hunk.
> 
> So you should say "a platform device for old style boards".

Hi Rob

O.K. I will change it.
 
> >>> +		cpuidle@1418 {
> >>> +			compatible = "marvell,kirkwood-cpuidle";
> >>> +			reg = <0x1418 0x4>;
> >>> +		};
> >>> +
> >>
> >> This is describing what linux wants, not the hardware. This is a common
> >> problem with cpuidle drivers in that they use shared registers. I don't
> >> have a good solution, but this doesn't belong in DTS.
> > 
> > Do you have a bad solution?
> 
> Ha! :) I should say I don't have a clear, obvious solution.
> 
> Don't do a platform driver and just check the machine compatible
> property which is what I did for highbank.

Yes, i've seen your cpuidle-calxeda.c. I can follow that model, but i
still somehow need the address of the SDRAM controller "Operation
Register".

> Have the machine code create
> the platform device. Not *all* platform devices have to be created based
> on the DTB. Create an MFD driver for the whole block of registers.

The block of registers is for controlling the SDRAM. Its not really a
MFD. The cpuidle code is putting the SDRAM controller into self
refresh mode and then doing a WFI.

> 
> > I could just hard code the address, since its the same for all
> > kirkwood SoCs. Also, the register is not being used by any other
> > code on kirkwood, so its not shared.
> 
> Then describe it based on the reference manual, but you need to do so
> assuming you are using all the other registers. I assume there are other
> registers at say 0x1414 or 0x141c. You have to be careful if you create
> separate nodes for each register or sub-group of registers. It needs to
> work no matter what the OS expectation is.

I don't understand what you are try to explain here. It makes little
sense for the cpuidle driver to take all the SDRAM control registers.

Thanks

	Andrew