lkml.org 
[lkml]   [2007]   [Jul]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [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 <avorontsov@ru.mvista.com>
    > > Signed-off-by: Vitaly Bordug <vitb@kernel.crashing.org>
    >
    > > 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-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2007-07-26 19:35    [W:0.034 / U:61.700 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site