Re: [PATCH 2/2] [POWERPC] MPC8349E-mITX: use platform IDE driver for CF interface
The hardware is called (E)IDE, the protocol is called ATA. Or that's what I was told -- I think there's some historic revisionism involved, too. ATA is the interface and standards for the ANSI standards based disk attachment. IDE Integrated Drive Electronics is a marketing name used to cover all sorts of ST412 compatible-ish early interfaces that moved the brains onto the disk. IDE doesn't really mean much but brains on disk, ATA is a real standard. Thanks for refreshing my memory. We will have to support both names in OF device tree nodes, since both names are used in many existing device trees. For new nodes with no precedent, like this mmio-ide, let's require the more correct ata name. Segher - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] [POWERPC] MPC8349E-mITX: use platform IDE driver for CF interface
Hello.
Segher Boessenkool wrote:
+[EMAIL PROTECTED] {
+compatible = mmio-ide;
+device_type = ide;
Why not ata?
The hardware is called (E)IDE, the protocol is called ATA.
Sorry for not denouncing this earlier. :-)
ATA is the name of ANSI standard describing IDE.
Or that's what I was told --
Re-check your sources. ;-)
I think there's some historic revisionism involved, too.
IDE was probably an initial name of the infamous disk hardware/protocol
later standardized as ATA, EIDE (being more of a trademark) more or less
equals to ATA-2.
Also, what mmio-ide in the compat properly means in the context of
ide_platform which is able to handle both port and memory mapped IDE.
I think
we must get rid with this crap, and since this IDE register mapping is
pretty
much board specific, call it something like mpc8349emitx-ide instead.
mmio-ide simply is not specific enough. The device_type
Yes.
should go, too.
If this IDE interface is board-specific, thee compatible
It's thy, not thee. ;-)
property should include the board vendor name and board
name. Oh, that's what emitx tries to do -- it could be
a bit clearer perhaps ;-)
Yeah, I forgot about the vondor's fsl, prefix.
Segher
MBR, Sergei
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] [POWERPC] MPC8349E-mITX: use platform IDE driver for CF interface
+ [EMAIL PROTECTED] {
+ compatible = mmio-ide;
+ device_type = ide;
Why not ata?
The hardware is called (E)IDE, the protocol is called ATA.
Or that's what I was told -- I think there's some historic
revisionism involved, too.
Also, what mmio-ide in the compat properly means in the context of
ide_platform which is able to handle both port and memory mapped IDE.
I think
we must get rid with this crap, and since this IDE register mapping is
pretty
much board specific, call it something like mpc8349emitx-ide instead.
mmio-ide simply is not specific enough. The device_type
should go, too.
If this IDE interface is board-specific, thee compatible
property should include the board vendor name and board
name. Oh, that's what emitx tries to do -- it could be
a bit clearer perhaps ;-)
Segher
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] [POWERPC] MPC8349E-mITX: use platform IDE driver for CF interface
The hardware is called (E)IDE, the protocol is called ATA. Or that's what I was told -- I think there's some historic revisionism involved, too. ATA is the interface and standards for the ANSI standards based disk attachment. IDE Integrated Drive Electronics is a marketing name used to cover all sorts of ST412 compatible-ish early interfaces that moved the brains onto the disk. IDE doesn't really mean much but brains on disk, ATA is a real standard. - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] [POWERPC] MPC8349E-mITX: use platform IDE driver for CF interface
Hello. Vitaly Bordug wrote: I acn undertand your complaint in the context of an OF driver (which we don't have yet) but mmio-ide just means nothing to the current driver, and it doesn't convery enough info on the programming interface for the conceivable OF driver, it also does need to know at least reg-stride (and maybe reg-size in case only 16/32-bit accesses can be used). Well, if such driver will be written, I/O mapping support will probably be dropped from it, so indeed, calling it mmio-ide.c would make sense. But that can be added when this driver is done, and for now I don't think the details of what Linux code currently exists should drive the device tree binding. That the current patches use platform_device glue code is an implementation detail (and one I'd rather see go away, in favor of a driver that supports both platform_device and of_device). I'd really prefer the board name to appear in the compatible prop (to which mmio-ide can be appended)... Sure, that's always good... it was the instead that I objected to. Hmmm. So what is finally suggested devicetree node for this beast - can somebody refine? I am a little bit confused about decided device_type My understanding is that ata has been already used, so there's no sense in introducing ide. Anyway, Segher will just say that device_type shoudn't matter and even be present. ;-) and compatible fields... In my understanding, as mmio-ide currectly makes no sense, it shouldn't even appear there. And since mpc8349emitx-cf (or whatever would be most generic name for those boards with the same type of CF IDE mapping) should be imply the shift value, this property should also be optional, i.e. passing hard coded value with platform_device would do. MBR, Sergei - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] [POWERPC] MPC8349E-mITX: use platform IDE driver for CF interface
On Wed, 25 Jul 2007 21:06:42 +0400
Sergei Shtylyov wrote:
Hello.
Vitaly Bordug wrote:
This updates relevant platform code (freescale mpc8349itx target)
to make the CompactFlash work in TrueIDE mode.
Erm, I'm not sure it's worth submitting the platform device driver
for PowerPC at this point, but well...
As Ben already confirmed, it is OK to have constructor insert platform device
if such is
required by design.
Signed-off-by: Anton Vorontsov [EMAIL PROTECTED]
Signed-off-by: Vitaly Bordug [EMAIL PROTECTED]
arch/powerpc/boot/dts/mpc8349emitx.dts|9
arch/powerpc/platforms/83xx/mpc834x_itx.c | 70
+ 2 files changed, 79 insertions(+), 0
deletions(-)
diff --git a/arch/powerpc/boot/dts/mpc8349emitx.dts
b/arch/powerpc/boot/dts/mpc8349emitx.dts index db0d003..f8f0e8a
100644 --- a/arch/powerpc/boot/dts/mpc8349emitx.dts
+++ b/arch/powerpc/boot/dts/mpc8349emitx.dts
@@ -37,6 +37,15 @@
reg = 1000;
};
+ [EMAIL PROTECTED] {
+ compatible = mmio-ide;
+ device_type = ide;
Why not ata?
we already argued around this iirc. I have no preferences and will put whatever
agreed appropriate finally in here.
+ reg = f000 10 f20c 4;
+ ioport_shift = 1;
Please use hyphen, not underscore in the property names
(device_type seems an only exception from this rule).
And since using shift instead of size buys you nothing is this
case, I'd prefer this property to be called reg-size or reg-stride.
[...]
diff --git a/arch/powerpc/platforms/83xx/mpc834x_itx.c
b/arch/powerpc/platforms/83xx/mpc834x_itx.c index 40a0194..d63a104
100644 --- a/arch/powerpc/platforms/83xx/mpc834x_itx.c
+++ b/arch/powerpc/platforms/83xx/mpc834x_itx.c
[...]
@@ -43,6 +44,75 @@ unsigned long isa_io_base = 0;
unsigned long isa_mem_base = 0;
#endif
+static int __init mmio_ide_of_init(void)
+{
+ struct device_node *np;
+ unsigned int i;
+
+ for (np = NULL, i = 0;
+(np = of_find_compatible_node(np, ide,
mmio-ide)) != NULL;
+i++) {
+ int ret = 0;
Unneeded initialization.
+ struct resource res[3];
+ struct platform_device *pdev = NULL;
Another one.
ok
+ static struct pata_platform_info pdata;
+
+ memset(res, 0, sizeof(res));
+
+ ret = of_address_to_resource(np, 0, res[0]);
+ if (ret) {
+ printk(KERN_ERR mmio-ide.%d: unable to
get
+ resource from OF\n, i);
+ goto err0;
+ }
+
+ ret = of_address_to_resource(np, 1, res[1]);
+ if (ret) {
+ printk(KERN_ERR mmio-ide.%d: unable to
get
+ resource from OF\n, i);
+ goto err0;
Erm, these printk's are repetitive, isn't it better to put them
under err0 label?
+ }
+
+ res[2].start = res[2].end =
irq_of_parse_and_map(np, 0);
+ if (res[2].start == NO_IRQ) {
+ printk(KERN_ERR mmio-ide.%d: no IRQ\n,
i);
+ goto err0;
+ }
+ res[2].name = pata_platform;
+ res[2].flags = IORESOURCE_IRQ;
+
+ pdata.ioport_shift = *((u32 *)of_get_property(np,
+ ioport_shift, NULL));
+
+ pdev = platform_device_alloc(pata_platform, i);
+ if (!pdev)
+ goto err1;
Hm, not err0?
agreed.
+
+ ret = platform_device_add_data(pdev, pdata,
sizeof(pdata));
+ if (ret)
+ goto err1;
+
+ ret = platform_device_add_resources(pdev, res,
ARRAY_SIZE(res));
+ if (ret)
+ goto err1;
+
+ ret = platform_device_register(pdev);
+ if (ret)
+ goto err1;
+
+ continue;
+err1:
+ printk(KERN_ERR mmio-ide.%d: registration
failed\n, i);
+ platform_device_del(pdev); /* it will free
everything */ +err0:
+ /* Even if some device failed, try others */
+ continue;
+ }
+
+ return 0;
+}
+device_initcall(mmio_ide_of_init);
+
/*
*
* Setup the architecture
MBR, Sergei
--
Sincerely, Vitaly
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] [POWERPC] MPC8349E-mITX: use platform IDE driver for CF interface
Hello, I wrote: Also, what mmio-ide in the compat properly means in the context of ide_platform which is able to handle both port and memory mapped IDE. I/O-space is only valid in the context of PCI, ISA, or similar buses, and the bus-specific reg format indicates whether it's mmio-space or io-space. You could save time on lecturing me (and use it to look on the driver ;-). Sorry, I misread the question as being a mismatch between the capabilities of the device binding and the driver, not about the specific compatible name. That too. :-) Something like generic-ide would probably be better. I strongly disagree with generic part. The generic IDE could only be said of 1:1 I/O mapped IDE ports, not about this fancy mapping. BTW, there's already something called drivers/ide/ide-generic.c... :-) MBR, Sergei - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] [POWERPC] MPC8349E-mITX: use platform IDE driver for CF interface
On Wed, 25 Jul 2007 13:46:57 -0500 Scott Wood wrote: Sergei Shtylyov wrote: I acn undertand your complaint in the context of an OF driver (which we don't have yet) but mmio-ide just means nothing to the current driver, and it doesn't convery enough info on the programming interface for the conceivable OF driver, it also does need to know at least reg-stride (and maybe reg-size in case only 16/32-bit accesses can be used). Well, if such driver will be written, I/O mapping support will probably be dropped from it, so indeed, calling it mmio-ide.c would make sense. But that can be added when this driver is done, and for now I don't think the details of what Linux code currently exists should drive the device tree binding. That the current patches use platform_device glue code is an implementation detail (and one I'd rather see go away, in favor of a driver that supports both platform_device and of_device). I'd really prefer the board name to appear in the compatible prop (to which mmio-ide can be appended)... Sure, that's always good... it was the instead that I objected to. Hmmm. So what is finally suggested devicetree node for this beast - can somebody refine? I am a little bit confused about decided device_type and compatible fields... -- Sincerely, Vitaly - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] [POWERPC] MPC8349E-mITX: use platform IDE driver for CF interface
Vitaly Bordug wrote:
diff --git a/arch/powerpc/boot/dts/mpc8349emitx.dts
b/arch/powerpc/boot/dts/mpc8349emitx.dts
index db0d003..f8f0e8a 100644
--- a/arch/powerpc/boot/dts/mpc8349emitx.dts
+++ b/arch/powerpc/boot/dts/mpc8349emitx.dts
@@ -37,6 +37,15 @@
reg = 1000;
};
+ [EMAIL PROTECTED] {
+ compatible = mmio-ide;
+ device_type = ide;
+ reg = f000 10 f20c 4;
+ ioport_shift = 1;
+ interrupts = 17 8;
+ interrupt-parent = ipic ;
+ };
+
Is this binding documented anywhere? If it's a new binding, could we
use reg-shift instead of ioport_shift? This is what ns16550 uses, and
in general dashes are preferable to underscores in device trees
(device_type is an ugly, historical exception).
diff --git a/arch/powerpc/platforms/83xx/mpc834x_itx.c
b/arch/powerpc/platforms/83xx/mpc834x_itx.c
index 40a0194..d63a104 100644
--- a/arch/powerpc/platforms/83xx/mpc834x_itx.c
+++ b/arch/powerpc/platforms/83xx/mpc834x_itx.c
@@ -23,6 +23,7 @@
#include linux/delay.h
#include linux/seq_file.h
#include linux/root_dev.h
+#include linux/pata_platform.h
#include asm/system.h
#include asm/atomic.h
@@ -43,6 +44,75 @@ unsigned long isa_io_base = 0;
unsigned long isa_mem_base = 0;
#endif
+static int __init mmio_ide_of_init(void)
+{
+ struct device_node *np;
+ unsigned int i;
+
+ for (np = NULL, i = 0;
+(np = of_find_compatible_node(np, ide, mmio-ide)) != NULL;
+i++) {
+ int ret = 0;
+ struct resource res[3];
+ struct platform_device *pdev = NULL;
+ static struct pata_platform_info pdata;
We really don't want code like this for every board with an MMIO IDE
controller. At the very least, make it a generic function in sysdev;
better would be to make the driver also support of_platform devices.
-Scott
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] [POWERPC] MPC8349E-mITX: use platform IDE driver for CF interface
Hello, I wrote:
diff --git a/arch/powerpc/boot/dts/mpc8349emitx.dts
b/arch/powerpc/boot/dts/mpc8349emitx.dts
index db0d003..f8f0e8a 100644
--- a/arch/powerpc/boot/dts/mpc8349emitx.dts
+++ b/arch/powerpc/boot/dts/mpc8349emitx.dts
@@ -37,6 +37,15 @@
reg = 1000;
};
+ [EMAIL PROTECTED] {
+ compatible = mmio-ide;
+ device_type = ide;
Why not ata?
Also, what mmio-ide in the compat properly means in the context of
ide_platform which is able to handle both port and memory mapped IDE. I think
we must get rid with this crap, and since this IDE register mapping is pretty
much board specific, call it something like mpc8349emitx-ide instead.
MBR, Sergei
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] [POWERPC] MPC8349E-mITX: use platform IDE driver for CF interface
Scott Wood wrote: Scott Wood wrote: Also, what mmio-ide in the compat properly means in the context of ide_platform which is able to handle both port and memory mapped IDE. I/O-space is only valid in the context of PCI, ISA, or similar buses, and the bus-specific reg format indicates whether it's mmio-space or io-space. You could save time on lecturing me (and use it to look on the driver ;-). Sorry, I misread the question as being a mismatch between the capabilities of the device binding and the driver, not about the specific compatible name. That too. :-) Something like generic-ide would probably be better. I strongly disagree with generic part. The generic IDE could only be said of 1:1 I/O mapped IDE ports, not about this fancy mapping. What is board specific about a set of standard IDE registers at a given The regisrer mapping used is highly non-standard. The gap between registers is nonstandard, but that's a fairly common type of noncompliance in embedded-land, and probably merits being That is only a common variation of embedded non-compliancy (which doesn't make it a compliancy. ;-) There are worse cases in the bi-endian land, even with the standard 8-bit regs and 1-byte stride. *Hopefully*, this driver could also support those... supported in a generic way. I wouldn't call it highly nonstandard. Yeah, there are also 8250 compatible UARTs that use 32-bit memory accesses, and even worse -- with some registers mapped differently than on 8250 (those can't be called compatible by any means), yet 8250.c drives all of them. I'm not really sure it was such a good idea to merge, say Alchemy UART support into 8250.c. Is there some other non-standardness that I'm missing? *Hopefully*, none. The original Kumar's driver pretended to handle byte-lane swapping too (but that was ugly :-). We're already in board specific code, so why the heck not? :-) various ns16550-compatibles out there as well? I never suggested that -- what I did suggest was make of_serial.c recognize certain chip types and register them with 8250 driver. What would be the advantage of maintaining a list of chips whose only Nobody's talking about the advantages, just about the device tree accepted practices (which we've already tried to bypass with MTD node -- causing a lot of bashing until David Woodhouse came to help :-). difference is register spacing, rather than just using reg-shift and being done with it? Please read the linuxppc-dev archive's threads following form David's patches. Or maybe Segher could repeat this for you. ;-) -Scott MBR, Sergei - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] [POWERPC] MPC8349E-mITX: use platform IDE driver for CF interface
On Wed, Jul 25, 2007 at 09:54:07PM +0400, Sergei Shtylyov wrote: Also, what mmio-ide in the compat properly means in the context of ide_platform which is able to handle both port and memory mapped IDE. I/O-space is only valid in the context of PCI, ISA, or similar buses, and the bus-specific reg format indicates whether it's mmio-space or io-space. I think we must get rid with this crap, and since this IDE register mapping is pretty much board specific, call it something like mpc8349emitx-ide instead. What is board specific about a set of standard IDE registers at a given address? Do we need to make board-specific glue code for all of the various ns16550-compatibles out there as well? -Scott - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] [POWERPC] MPC8349E-mITX: use platform IDE driver for CF interface
Sergei Shtylyov wrote: Hello. Scott Wood wrote: Also, what mmio-ide in the compat properly means in the context of ide_platform which is able to handle both port and memory mapped IDE. I/O-space is only valid in the context of PCI, ISA, or similar buses, and the bus-specific reg format indicates whether it's mmio-space or io-space. You could save time on lecturing me (and use it to look on the driver ;-). Sorry, I misread the question as being a mismatch between the capabilities of the device binding and the driver, not about the specific compatible name. Something like generic-ide would probably be better. What is board specific about a set of standard IDE registers at a given The regisrer mapping used is highly non-standard. The gap between registers is nonstandard, but that's a fairly common type of noncompliance in embedded-land, and probably merits being supported in a generic way. I wouldn't call it highly nonstandard. Is there some other non-standardness that I'm missing? We're already in board specific code, so why the heck not? :-) various ns16550-compatibles out there as well? I never suggested that -- what I did suggest was make of_serial.c recognize certain chip types and register them with 8250 driver. What would be the advantage of maintaining a list of chips whose only difference is register spacing, rather than just using reg-shift and being done with it? -Scott - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] [POWERPC] MPC8349E-mITX: use platform IDE driver for CF interface
Hello. Scott Wood wrote: Also, what mmio-ide in the compat properly means in the context of ide_platform which is able to handle both port and memory mapped IDE. I/O-space is only valid in the context of PCI, ISA, or similar buses, and the bus-specific reg format indicates whether it's mmio-space or io-space. You could save time on lecturing me (and use it to look on the driver ;-). I think we must get rid with this crap, and since this IDE register mapping is pretty much board specific, call it something like mpc8349emitx-ide instead. What is board specific about a set of standard IDE registers at a given The regisrer mapping used is highly non-standard. address? Do we need to make board-specific glue code for all of the We're already in board specific code, so why the heck not? :-) various ns16550-compatibles out there as well? I never suggested that -- what I did suggest was make of_serial.c recognize certain chip types and register them with 8250 driver. -Scott MBR, Sergei - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] [POWERPC] MPC8349E-mITX: use platform IDE driver for CF interface
Sergei Shtylyov wrote: I acn undertand your complaint in the context of an OF driver (which we don't have yet) but mmio-ide just means nothing to the current driver, and it doesn't convery enough info on the programming interface for the conceivable OF driver, it also does need to know at least reg-stride (and maybe reg-size in case only 16/32-bit accesses can be used). Well, if such driver will be written, I/O mapping support will probably be dropped from it, so indeed, calling it mmio-ide.c would make sense. But that can be added when this driver is done, and for now I don't think the details of what Linux code currently exists should drive the device tree binding. That the current patches use platform_device glue code is an implementation detail (and one I'd rather see go away, in favor of a driver that supports both platform_device and of_device). I'd really prefer the board name to appear in the compatible prop (to which mmio-ide can be appended)... Sure, that's always good... it was the instead that I objected to. -Scott - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
