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 <avorontsov@xxxxxxxxxxxxx>
> > Signed-off-by: Vitaly Bordug <vitb@xxxxxxxxxxxxxxxxxxx>
>
> > 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 = <00000000 10000000>;
> > };
> >
> > + ide@f0000000 {
> > + 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 = <f0000000 10 f000020c 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
