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 _______________________________________________ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss