pata-generic/of: Make probing via device tree non-powerpc-specific

PATCH Superseded Dave Martin 7 participants 13 messages Sep 16, 2011 – Sep 20, 2011 libata

lore

Share & tools

lore

Dave Martin

author

2011-09-16 14:38 UTC #0 lore raw

PATCH 1/1 pata-generic/of: Make probing via device tree non-powerpc-specific

This patch enables device-tree-based probing of the pata-generic platform driver across all architectures:

* make the pata_of_generic module depend on OF instead of PPC_OF;
* supply some missing inclues;
* replace endianness-sensitive raw access to device tree data with of_property_read_u32() calls.

Signed-off-by Dave Martin <[email protected]>

---

Tested on ARM Versatile Express, with my soon-to-be-posted device tree support patches.

I'm not in a position to build/test this for powerpc easily -- if anyone is able to do that, it would be appreciated.

Grant, does this require similar cleanup to the isp1760 USB hcd driver?

Cheers
---Dave

 drivers/ata/Kconfig            |    2 +-
 drivers/ata/pata_of_platform.c |   16 +++++++---------
 2 files changed, 8 insertions(+), 10 deletions(-)

drivers/ata/Kconfig +1 -1

index 5987e0b..c6ef9d0 100644
@@ -820,7 +820,7 @@ config PATA_PLATFORM
 
 config PATA_OF_PLATFORM
 	tristate "OpenFirmware platform device PATA support"
-	depends on PATA_PLATFORM && PPC_OF
+	depends on PATA_PLATFORM && OF
 	help
 	  This option enables support for generic directly connected ATA
 	  devices commonly found on embedded systems with OpenFirmware

drivers/ata/pata_of_platform.c +7 -9

index f305400..ac7c9f7 100644
@@ -11,6 +11,9 @@
 
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
 #include <linux/of_platform.h>
 #include <linux/ata_platform.h>

@@ -21,10 +24,9 @@ static int __devinit pata_of_platform_probe(struct platform_device *ofdev)
 	struct resource io_res;
 	struct resource ctl_res;
 	struct resource irq_res;
-	unsigned int reg_shift = 0;
-	int pio_mode = 0;
+	u32 reg_shift = 0;
+	u32 pio_mode = 0;
 	int pio_mask;
-	const u32 *prop;
 
 	ret = of_address_to_resource(dn, 0, &io_res);
 	if (ret) {

@@ -55,13 +57,9 @@ static int __devinit pata_of_platform_probe(struct platform_device *ofdev)
 	else
 		irq_res.flags = 0;
 
-	prop = of_get_property(dn, "reg-shift", NULL);
-	if (prop)
-		reg_shift = *prop;
+	of_property_read_u32(dn, "reg-shift", &reg_shift);
 
-	prop = of_get_property(dn, "pio-mode", NULL);
-	if (prop) {
-		pio_mode = *prop;
+	if (of_property_read_u32(dn, "pio-mode", &pio_mode)) {
 		if (pio_mode > 6) {
 			dev_err(&ofdev->dev, "invalid pio-mode\n");
 			return -EINVAL;

Signature · 1 line

1.7.4.1

Dave Martin

author

2011-09-16 16:22 UTC #1 lore raw

PATCH v2 1/1 pata-generic/of: Make probing via device tree non-powerpc-specific

This patch enables device-tree-based probing of the pata-generic platform driver across all architectures:

* make the pata_of_generic module depend on OF instead of PPC_OF;
* supply some missing inclues;
* replace endianness-sensitive raw access to device tree data with of_property_read_u32() calls.

Signed-off-by Dave Martin <[email protected]>

---
v2: correct sense of the check of_property_read_u32(dn, "pio-mode", &pio_mode). Somehow I posted an old version of this patch, depite having already fixed this...

Tested on ARM Versatile Express, with my soon-to-be-posted device tree support patches.

I'm not in a position to build/test this for powerpc easily -- if anyone is able to do that, it would be appreciated.

Grant, does this require similar cleanup to the isp1760 USB hcd driver?

 drivers/ata/Kconfig            |    2 +-
 drivers/ata/pata_of_platform.c |   16 +++++++---------
 2 files changed, 8 insertions(+), 10 deletions(-)

drivers/ata/Kconfig +1 -1

index 5987e0b..c6ef9d0 100644
@@ -820,7 +820,7 @@ config PATA_PLATFORM
 
 config PATA_OF_PLATFORM
 	tristate "OpenFirmware platform device PATA support"
-	depends on PATA_PLATFORM && PPC_OF
+	depends on PATA_PLATFORM && OF
 	help
 	  This option enables support for generic directly connected ATA
 	  devices commonly found on embedded systems with OpenFirmware

drivers/ata/pata_of_platform.c +7 -9

index f305400..e6e9aa9 100644
@@ -11,6 +11,9 @@
 
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
 #include <linux/of_platform.h>
 #include <linux/ata_platform.h>

@@ -21,10 +24,9 @@ static int __devinit pata_of_platform_probe(struct platform_device *ofdev)
 	struct resource io_res;
 	struct resource ctl_res;
 	struct resource irq_res;
-	unsigned int reg_shift = 0;
-	int pio_mode = 0;
+	u32 reg_shift = 0;
+	u32 pio_mode = 0;
 	int pio_mask;
-	const u32 *prop;
 
 	ret = of_address_to_resource(dn, 0, &io_res);
 	if (ret) {

@@ -55,13 +57,9 @@ static int __devinit pata_of_platform_probe(struct platform_device *ofdev)
 	else
 		irq_res.flags = 0;
 
-	prop = of_get_property(dn, "reg-shift", NULL);
-	if (prop)
-		reg_shift = *prop;
+	of_property_read_u32(dn, "reg-shift", &reg_shift);
 
-	prop = of_get_property(dn, "pio-mode", NULL);
-	if (prop) {
-		pio_mode = *prop;
+	if (!of_property_read_u32(dn, "pio-mode", &pio_mode)) {
 		if (pio_mode > 6) {
 			dev_err(&ofdev->dev, "invalid pio-mode\n");
 			return -EINVAL;

Signature · 1 line

1.7.4.1

Rob Herring

2011-09-16 21:34 UTC #2 lore raw

PATCH v2 1/1 pata-generic/of: Make probing via device tree non-powerpc-specific

Dave,

Quoted 14 lines · Dave Martin

This patch enables device-tree-based probing of the pata-generic platform driver across all architectures:
* make the pata_of_generic module depend on OF instead of PPC_OF;
* supply some missing inclues;
* replace endianness-sensitive raw access to device tree data with of_property_read_u32() calls.
Signed-off-by Dave Martin <[email protected]>
---
v2: correct sense of the check of_property_read_u32(dn, "pio-mode", &pio_mode). Somehow I posted an old version of this patch, depite having already fixed this...

Quoted 6 lines

Tested on ARM Versatile Express, with my soon-to-be-posted device tree support patches.
I'm not in a position to build/test this for powerpc easily -- if anyone is able to do that, it would be appreciated.

Building just requires getting Codesourcery PPC toolchain...

You also have to be aware that you are enabling this for all OF-enabled arches which could break with an allyesconfig. So really you need sparc, x86, mips, and microblaze, but a subset is probably sufficient.

Quoted 2 lines

Grant, does this require similar cleanup to the isp1760 USB hcd driver?

Yes. It really should be merged with pata_platform.c. If you're willing to combine them, I'll do ppc and sparc builds as I already have those on my system.

Rob

Quoted 60 lines

 drivers/ata/Kconfig            |    2 +-
 drivers/ata/pata_of_platform.c |   16 +++++++---------
 2 files changed, 8 insertions(+), 10 deletions(-)

drivers/ata/Kconfig +1 -1

index 5987e0b..c6ef9d0 100644
@@ -820,7 +820,7 @@ config PATA_PLATFORM
 
 config PATA_OF_PLATFORM
 	tristate "OpenFirmware platform device PATA support"
-	depends on PATA_PLATFORM && PPC_OF
+	depends on PATA_PLATFORM && OF
 	help
 	  This option enables support for generic directly connected ATA
 	  devices commonly found on embedded systems with OpenFirmware

drivers/ata/pata_of_platform.c +7 -9

index f305400..e6e9aa9 100644
@@ -11,6 +11,9 @@
 
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
 #include <linux/of_platform.h>
 #include <linux/ata_platform.h>

@@ -21,10 +24,9 @@ static int __devinit pata_of_platform_probe(struct platform_device *ofdev)
 	struct resource io_res;
 	struct resource ctl_res;
 	struct resource irq_res;
-	unsigned int reg_shift = 0;
-	int pio_mode = 0;
+	u32 reg_shift = 0;
+	u32 pio_mode = 0;
 	int pio_mask;
-	const u32 *prop;
 
 	ret = of_address_to_resource(dn, 0, &io_res);
 	if (ret) {

@@ -55,13 +57,9 @@ static int __devinit pata_of_platform_probe(struct platform_device *ofdev)
 	else
 		irq_res.flags = 0;
 
-	prop = of_get_property(dn, "reg-shift", NULL);
-	if (prop)
-		reg_shift = *prop;
+	of_property_read_u32(dn, "reg-shift", &reg_shift);
 
-	prop = of_get_property(dn, "pio-mode", NULL);
-	if (prop) {
-		pio_mode = *prop;
+	if (!of_property_read_u32(dn, "pio-mode", &pio_mode)) {
 		if (pio_mode > 6) {
 			dev_err(&ofdev->dev, "invalid pio-mode\n");
 			return -EINVAL;

Dave Martin

author

2011-09-19 10:10 UTC #3 lore raw

PATCH v2 1/1 pata-generic/of: Make probing via device tree non-powerpc-specific

Quoted 17 lines · Rob Herring

Dave,
Quoted 14 lines · Dave Martin
This patch enables device-tree-based probing of the pata-generic platform driver across all architectures:
? * make the pata_of_generic module depend on OF instead of PPC_OF; ? * supply some missing inclues;
? * replace endianness-sensitive raw access to device tree data ? ? with of_property_read_u32() calls.
Signed-off-by Dave Martin <[email protected]>
---
v2: correct sense of the check of_property_read_u32(dn, "pio-mode", &pio_mode). ?Somehow I posted an old version of this patch, depite having already fixed this...

Quoted 7 lines

Quoted 6 lines
Tested on ARM Versatile Express, with my soon-to-be-posted device tree support patches.
I'm not in a position to build/test this for powerpc easily -- if anyone is able to do that, it would be appreciated.
Building just requires getting Codesourcery PPC toolchain...

True, but although I can try to build it, I don't have a PPC platform to do the actual testing.

Quoted 3 lines

You also have to be aware that you are enabling this for all OF-enabled arches which could break with an allyesconfig. So really you need sparc, x86, mips, and microblaze, but a subset is probably sufficient.

Fair point.

Since this has now been superseded by Grant's patch anyway, I'll leave it for others to judge.

Cheers
---Dave

Jeff Garzik

2011-09-20 19:30 UTC #4 lore raw

PATCH v2 1/1 pata-generic/of: Make probing via device tree non-powerpc-specific

Quoted 2 lines · Dave Martin

Since this has now been superseded by Grant's patch anyway, I'll leave it for others to judge.

Grant's patch is certainly preferable, all other things being equal...

Jeff

Grant Likely

2011-09-17 15:37 UTC #5 lore raw

PATCH v2 1/1 pata-generic/of: Make probing via device tree non-powerpc-specific

In reply to Dave Martin

Quoted 14 lines · Dave Martin

This patch enables device-tree-based probing of the pata-generic platform driver across all architectures:
* make the pata_of_generic module depend on OF instead of PPC_OF;
* supply some missing inclues;
* replace endianness-sensitive raw access to device tree data with of_property_read_u32() calls.
Signed-off-by Dave Martin <[email protected]>
---
v2: correct sense of the check of_property_read_u32(dn, "pio-mode", &pio_mode). Somehow I posted an old version of this patch, depite having already fixed this...

Quoted 5 lines

Tested on ARM Versatile Express, with my soon-to-be-posted device tree support patches.
I'm not in a position to build/test this for powerpc easily -- if anyone is able to do that, it would be appreciated.

What driver is normally used for versatile express pata? This driver is kind of legacy in that it was created when there was a split between platform_device and of_platform_devices. But that split was a bad idea and the same driver should be used regardless of whether or not DT is enabled. pata_of_platform.c really should be removed.

So, instead of removing the PPC_OF restriction from pata_of_platform.c, you should look at adding DT support to the existing binding. Bonus points if you can roll pata_of_platform.c support into pata_platform.c (assuming that is the correct driver).

Arnd Bergmann

2011-09-17 18:40 UTC #6 lore raw

PATCH v2 1/1 pata-generic/of: Make probing via device tree non-powerpc-specific

Quoted 5 lines · On Saturday 17 September 2011 09:37:32 Grant Likely wrote:

What driver is normally used for versatile express pata? This driver is kind of legacy in that it was created when there was a split between platform_device and of_platform_devices. But that split was a bad idea and the same driver should be used regardless of whether or not DT is enabled. pata_of_platform.c really should be removed.

It normally uses the plain pata_platform.c driver.

Note that the pata_of_platform driver is already just a shim on top of the regular pata_platform driver. They could easily be combined, but the current state is also ok, since there is very little code duplication.

Arnd

Grant Likely

2011-09-17 21:30 UTC #7 lore raw

PATCH v2 1/1 pata-generic/of: Make probing via device tree non-powerpc-specific

Quoted 7 lines · Arnd Bergmann

Quoted 5 lines · On Saturday 17 September 2011 09:37:32 Grant Likely wrote:
What driver is normally used for versatile express pata? ?This driver is kind of legacy in that it was created when there was a split between platform_device and of_platform_devices. ?But that split was a bad idea and the same driver should be used regardless of whether or not DT is enabled. ?pata_of_platform.c really should be removed.

Quoted 6 lines

It normally uses the plain pata_platform.c driver.
Note that the pata_of_platform driver is already just a shim on top of the regular pata_platform driver. They could easily be combined, but the current state is also ok, since there is very little code duplication.

A bunch of the code is actually redundant since the resource table is no populated for DT devices. I also see some directly references to reg-shift and pio-mode property values without using be32_to_cpu(), so that will also need to be fixed. The of_irq_to_resource() and of_address_to_resource() calls are now redundant since platform_get_*() works for DT sourced platform device (with one quirk for the electra-ide device). Given that the conversion is straight forward, I'd rather see pata_of_platform.c dropped and rolled into pata_platform.c. I've hacked together a patch to do so, but I've only compile tested it. Dave, if I send it to you, can you take care of testing it?

Thanks,
g.

Russell King - ARM Linux

2011-09-16 21:43 UTC #8 lore raw

PATCH 1/1 pata-generic/of: Make probing via device tree non-powerpc-specific

In reply to Dave Martin

Quoted 8 lines · Dave Martin

This patch enables device-tree-based probing of the pata-generic platform driver across all architectures:
* make the pata_of_generic module depend on OF instead of PPC_OF;
* supply some missing inclues;
* replace endianness-sensitive raw access to device tree data with of_property_read_u32() calls.

Quoted 5 lines

Signed-off-by Dave Martin <[email protected]>
---
Tested on ARM Versatile Express, with my soon-to-be-posted device tree support patches.

This may not be the correct way to support the CF slot on Versatile Express - it depends whether the CF slot on VE supports just CF memory cards or whether it can take any CF card.

If the latter, then what may be inserted could be a CF network card, and that means it's probably wrong to tell the kernel that what's there is a PATA interface.

Arnd Bergmann

2011-09-17 18:35 UTC #9 lore raw

PATCH 1/1 pata-generic/of: Make probing via device tree non-powerpc-specific

Quoted 15 lines · On Friday 16 September 2011 22:43:13 Russell King - ARM Linux wrote:

Quoted 8 lines · Dave Martin
This patch enables device-tree-based probing of the pata-generic platform driver across all architectures:
* make the pata_of_generic module depend on OF instead of PPC_OF;
* supply some missing inclues;
* replace endianness-sensitive raw access to device tree data with of_property_read_u32() calls.
Quoted 5 lines
Signed-off-by Dave Martin <[email protected]>
---
Tested on ARM Versatile Express, with my soon-to-be-posted device tree support patches.

Quoted 7 lines

This may not be the correct way to support the CF slot on Versatile Express - it depends whether the CF slot on VE supports just CF memory cards or whether it can take any CF card.
If the latter, then what may be inserted could be a CF network card, and that means it's probably wrong to tell the kernel that what's there is a PATA interface.

The documentation at
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0447e/CACECHFJ.html
claims that the slot only supports TrueIDE and PIO/Taskfile mode, which are the two modes for CF memory cards. Of course this is highly dependent on what you load into the FPGA that implements the CF interface on the versatile express, but at least for the currect Versatile Express FPGA load it should be right.

Also, the patch makes sense regardless of versatile express, it's just basic cleanup.

One extension we might want to add eventually is support for the UDMA modes that are present on most of the current CF cards but not supported by this driver (AFAICT).

Arnd

Dave Martin

author

2011-09-19 10:05 UTC #10 lore raw

PATCH 1/1 pata-generic/of: Make probing via device tree non-powerpc-specific

In reply to Russell King - ARM Linux

Hi,

Quoted 3 lines · Russell King - ARM Linux

Quoted 2 lines · Dave Martin
This patch enables device-tree-based probing of the pata-generic platform driver across all architectures:

[...]

Quoted 8 lines

This may not be the correct way to support the CF slot on Versatile Express - it depends whether the CF slot on VE supports just CF memory cards or whether it can take any CF card.
If the latter, then what may be inserted could be a CF network card, and that means it's probably wrong to tell the kernel that what's there is a PATA interface.

Interesting point. I don't know whether we've tried plugging anything else in...

Pawel, can you comment?

Cheers
---Dave

Pawel Moll

2011-09-19 10:23 UTC #11 lore raw

PATCH 1/1 pata-generic/of: Make probing via device tree non-powerpc-specific

Quoted 4 lines

Quoted 4 lines
This may not be the correct way to support the CF slot on Versatile Express - it depends whether the CF slot on VE supports just CF memory cards or whether it can take any CF card.

Quoted 8 lines

Quoted 3 lines
If the latter, then what may be inserted could be a CF network card, and that means it's probably wrong to tell the kernel that what's there is a PATA interface.
Interesting point. I don't know whether we've tried plugging anything else in...
Pawel, can you comment?

VE officially supports CF memory cards only (Arnd already referenced the spec). So in this case CF == PATA.

Cheers!

Pawe?

Dave Martin

author

2011-09-19 10:56 UTC #12 lore raw

PATCH 1/1 pata-generic/of: Make probing via device tree non-powerpc-specific

Quoted 8 lines · Pawel Moll

Quoted 8 lines
Quoted 7 lines
This may not be the correct way to support the CF slot on Versatile Express - it depends whether the CF slot on VE supports just CF memory cards or whether it can take any CF card.
If the latter, then what may be inserted could be a CF network card, and that means it's probably wrong to tell the kernel that what's there is a PATA interface.

Quoted 7 lines

Quoted 4 lines
Interesting point. ?I don't know whether we've tried plugging anything else in...
Pawel, can you comment?
VE officially supports CF memory cards only (Arnd already referenced the spec). So in this case CF == PATA.

OK, Il'l stick with this for now.

Cheers
---Dave