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

login
register
mail settings
Submitter Dave Martin
Date Sept. 16, 2011, 2:38 p.m.
Message ID <1316183890-13677-1-git-send-email-dave.martin@linaro.org>
Download mbox | patch
Permalink /patch/114936/
State Not Applicable
Delegated to: David Miller
Headers show

Comments

Dave Martin - Sept. 16, 2011, 2:38 p.m.
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 <dave.martin@linaro.org>
---

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(-)
Russell King - ARM Linux - Sept. 16, 2011, 9:43 p.m.
On Fri, Sep 16, 2011 at 03:38:10PM +0100, Dave Martin wrote:
> 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 <dave.martin@linaro.org>
> ---
> 
> 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.
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Arnd Bergmann - Sept. 17, 2011, 6:35 p.m.
On Friday 16 September 2011 22:43:13 Russell King - ARM Linux wrote:
> On Fri, Sep 16, 2011 at 03:38:10PM +0100, Dave Martin wrote:
> > 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 <dave.martin@linaro.org>
> > ---
> > 
> > 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.

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
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dave Martin - Sept. 19, 2011, 10:05 a.m.
Hi,

On Fri, Sep 16, 2011 at 10:43 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Fri, Sep 16, 2011 at 03:38:10PM +0100, Dave Martin wrote:
>> This patch enables device-tree-based probing of the pata-generic
>> platform driver across all architectures:

[...]

>
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pawel Moll - Sept. 19, 2011, 10:23 a.m.
> > 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?

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

Cheers!

Paweł


--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dave Martin - Sept. 19, 2011, 10:56 a.m.
On Mon, Sep 19, 2011 at 11:23 AM, Pawel Moll <pawel.moll@arm.com> wrote:
>> > 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?
>
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index 5987e0b..c6ef9d0 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -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
diff --git a/drivers/ata/pata_of_platform.c b/drivers/ata/pata_of_platform.c
index f305400..ac7c9f7 100644
--- a/drivers/ata/pata_of_platform.c
+++ b/drivers/ata/pata_of_platform.c
@@ -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;