Skip to contentSkip to conversation

pasemi: electra IDE/pata_platform glue

PATCH In review Olof Johansson 5 participants 10 messages May 12, 2007 – May 13, 2007

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 pasemi: electra IDE/pata_platform glue

Glue code to hook up the pata_platform on the PA Semi Electra eval board. CFE sets up device tree entries for the IDE interface, with device type 'ide' and compatible field 'electra-ide'.

We unfortunately need to modify the resources before calling the generic platform driver, since the device tree only has one register window in it and the driver expects two. Adding this as an of_platform driver instead doesn't give us any benefit, it just adds one more layer of register/probe functions.

Since CONFIG_PATA_PLATFORM depends on CONFIG_EMBEDDED, add that as a default for PPC_PASEMI.

arch/powerpc/platforms/pasemi/Makefile +1 -0
Index: 2.6.21/arch/powerpc/platforms/pasemi/Makefile
===================================================================
@@ -1,3 +1,4 @@
 obj-y	+= setup.o pci.o time.o idle.o powersave.o iommu.o
 obj-$(CONFIG_PPC_PASEMI_MDIO)	+= gpio_mdio.o
+obj-$(CONFIG_ELECTRA_IDE) += electra_ide.o
 obj-$(CONFIG_PPC_PASEMI_CPUFREQ) += cpufreq.o
arch/powerpc/platforms/pasemi/electra_ide.c new file +96 -0

96 lines not on this page · raw message

arch/powerpc/platforms/pasemi/Kconfig +10 -0
Index: 2.6.21/arch/powerpc/platforms/pasemi/Kconfig
===================================================================
@@ -6,6 +6,7 @@ config PPC_PASEMI
 	select PPC_UDBG_16550
 	select GENERIC_TBSYNC
 	select PPC_NATIVE
+	select EMBEDDED
 	help
 	  This option enables support for PA Semi's PWRficient line
 	  of SoC processors, including PA6T-1682M
@@ -26,4 +27,13 @@ config PPC_PASEMI_MDIO
 	help
 	  Driver for MDIO via GPIO on PWRficient platforms
 
+config ELECTRA_IDE
+      tristate "Electra IDE driver"
+      default y
+      depends on PPC_PASEMI && ATA
+      select PATA_PLATFORM
+      help
+	This includes driver support for the Electra on-board IDE
+	interface.
+
 endmenu
#1 lore raw
Quoted 6 lines · Olof Johansson

We unfortunately need to modify the resources before calling the generic platform driver, since the device tree only has one register window in it and the driver expects two. Adding this as an of_platform driver instead doesn't give us any benefit, it just adds one more layer of register/probe functions.

Why not provide a proper pata_of.c driver based on ata_generic? That will help the next person that has a builtin ata controller and wants to get it running as an of_device.

If you simply export generic_sht (rename that to ata_generic_sht) and generic_port_ops (-> ata_generic_port_ops), then the resulting of_platform_driver should be no more complicated than the one you are proposing here.

	Arnd <><  
#2 lore raw
Quoted 3 lines

Why not provide a proper pata_of.c driver based on ata_generic? That will help the next person that has a builtin ata controller and wants to get it running as an of_device.

Easier to use pata_platform I would think ? Just create the OF device and bind it to pata_platform.

Alan

#3 lore raw
Quoted 6 lines · Alan Cox
Quoted 3 lines

Why not provide a proper pata_of.c driver based on ata_generic? That will help the next person that has a builtin ata controller and wants to get it running as an of_device.

Easier to use pata_platform I would think ? Just create the OF device and bind it to pata_platform.

Not sure I understand what you mean. pata_platform expects a platform_device, which cannot be cast from an of_device.

Actually, it might be possible to merge the common parts of pata_platform, ata_generic and ata_of into one module, and have one driver for each of the three bus_types on top of it. Currently, the only difference between ata_generic and pata_platform is the ->set_mode function, which can probably be made generic (controlled by a flag in the ata_port).

Olof, have you looked at which of the two ->set_mode functions is more appropriate for electra? I would guess that you actually want to use the ata_generic one.

	Arnd <><
#4 lore raw
Quoted 10 lines · Arnd Bergmann
Quoted 6 lines · Alan Cox
Quoted 3 lines

Why not provide a proper pata_of.c driver based on ata_generic? That will help the next person that has a builtin ata controller and wants to get it running as an of_device.

Easier to use pata_platform I would think ? Just create the OF device and bind it to pata_platform.

Not sure I understand what you mean. pata_platform expects a platform_device, which cannot be cast from an of_device.

Note that in the long run, we might have less problem with that sort of thing.

First, every device now has an OF node optionally attached to it (since I introduced dev_sysdata). This of_device doesn't really bring much more than OF type/name/compatible properties based probing.

Then, the plans I have in mind for the future of that stuff are around the idea of registering "constructors" based on bus matches and device matches respectively.

The kernel will then walk the whole OF device-tree at boot, and will instanciate devices using those constructors, passing them the struct device of whatever was the parent.

Thus we could easily have platforms registers constructors for specific devices that build a platform device off an OF node. We can have a generic constructor that builds the PCI devices, etc... and we can have bus constructors to generate of_device's for things where they are useful, like plb4/5 on 4xx etc...

On top of that, I want to add some resource management to of_device and maybe kill things like ebus, vio, etc... device if possible (that is if the only difference to the base OF device can be resolved at construction time).

Anyway, still only thoughts but that gives you an idea to where I'm heading.

Ben.

#5 lore raw
Quoted 11 lines · Benjamin Herrenschmidt

Then, the plans I have in mind for the future of that stuff are around the idea of registering "constructors" based on bus matches and device matches respectively.

The kernel will then walk the whole OF device-tree at boot, and will instanciate devices using those constructors, passing them the struct device of whatever was the parent.

Thus we could easily have platforms registers constructors for specific devices that build a platform device off an OF node.

Great. That should fit perfectly with the glue I have now, it'll turn into the constructor instead.

I honestly don't see what the benefit is of this recent obsession with creating of_platform devices for every new device that's not PCI, especially when there's an already well-fitting driver in the traditional platform model. It's not like the code to link the two is large and complex.

Also, in this particular case, the bindings are not standardized, and there's a good chance that whatever new platform uses a similar device will need to do something slightly different.

-Olof

#6 lore raw
Quoted 9 lines

Great. That should fit perfectly with the glue I have now, it'll turn into the constructor instead.

I honestly don't see what the benefit is of this recent obsession with creating of_platform devices for every new device that's not PCI, especially when there's an already well-fitting driver in the traditional platform model. It's not like the code to link the two is large and complex.

I agree.

Quoted 3 lines

Also, in this particular case, the bindings are not standardized, and there's a good chance that whatever new platform uses a similar device will need to do something slightly different.

Yup.

Ben.

#7 lore raw
In reply to Olof Johansson
Quoted 3 lines

Also, in this particular case, the bindings are not standardized, and there's a good chance that whatever new platform uses a similar device will need to do something slightly different.

Yeah, everyone else will do the sane thing, and describe the two register ranges the IDE uses, not a much bigger range full of register shadows like you have. It is one thing for the hardware to do partial address decoding; the device tree shouldn't normally expose this though.

But that's what you've got now, so you have your own special OF device matching code, which is exactly as it should be. No need to have "generic" ide OF matching code until device trees containing such devices show up :-)

Segher

#8 lore raw
In reply to Arnd Bergmann
Quoted 5 lines
Quoted 2 lines

Easier to use pata_platform I would think ? Just create the OF device and bind it to pata_platform.

Not sure I understand what you mean. pata_platform expects a platform_device, which cannot be cast from an of_device.

Other than the binding and wanting to use the standard ->set_mode they appear to be identical so a copy of pata_platform ought to turn into a pata_of very nicely.

Quoted 3 lines

Olof, have you looked at which of the two ->set_mode functions is more appropriate for electra? I would guess that you actually want to use the ata_generic one.

You almost certainly simply don't want to provide one. I'm not sure the pata_platform default one is right at all as it will assume the firmware tuned everything. ata_generic does this purely because it is a "last resort" driver for chips we can't tune/program at all.

Alan

#9 lore raw
In reply to Arnd Bergmann

Hi,

Quoted 3 lines · Arnd Bergmann

Olof, have you looked at which of the two ->set_mode functions is more appropriate for electra? I would guess that you actually want to use the ata_generic one.

Actually, the pata_platform driver is _exactly_ what I want at this time, since we can't do bus master dma and it assumes that. It's a perfect fit, I see no reason to retool it.

Another reason for why I'm not that excited about doing a generic of_platform driver is that it's likely that I'll do a new driver that uses one of the simple dma engines on our SoC to get the data over PIO (i.e still not bus master dma, but at least offloaded). That'd make the resulting number of users of a pata_of driver 0 until the next one comes around, with whatever that means with respect to bit-rot, etc.

That driver will, on the other hand, likely be an of_platform one. But until then there's not much use in doing one.

-Olof