Wednesday 04 February 2009 17:04:27 Sergei Shtylyov napisał(a):
> >>> extern void __init at91_add_device_cf(struct at91_cf_data *data);
> >>>
> >>>+ /* Compact Flash True IDE mode */
> >>>+struct at91_ide_data {
> >>>+ u8 irq_pin; /* the same meaning as for CF */
>
> >> I again have to express my dislike about not passing IRQ the usual
> >>way. Also, see my comments to the platform code.
>
> > Yes, I know, I don't like to argue. Only reasoning to use platform irq resource
> > seams to be: "because other drivers do". However we have exception - at91_cf
> > also use board->irq_pin, so maybe this driver could also do ?
>
> Then why have the memory resource when we can calculate it from the chip
> select?
Great idea, I very like it :) But memory is a platform (cpu) resource, however
board dependend.
> (I'm not asking you to do that, since the platfrom device resources
> are user-visible thru /proc/iomem -- even if the driver is not enabled.)
Let's distinguish platform (cpu) resources and board resources.
If you take a look at arch/arm/mach-at91/*_devices.c files,
IORESOURCE_IRQ are used for interrupts from devices that are integrated
on the chip. Board specific irq pins (like in at91_cf, at91_ether) are not
passed to platform driver via platform_resource but via board data.
> >>>diff --git a/drivers/ide/at91_ide.c b/drivers/ide/at91_ide.c
> >>>new file mode 100644
> >>>index 0000000..3a1f7e0
> >>>--- /dev/null
> >>>+++ b/drivers/ide/at91_ide.c
> >>>@@ -0,0 +1,496 @@
> >>>+/*
> >>>+ * IDE host driver for AT91SAM9 Static Memory Controller
>
> >> Why not call the driver 'at91sam9_ide'?
>
> >>>+/*
> >>>+ * AT91 Static Memory Controller
>
> >> AT91SAM9.
>
> > Ok, currently only SAM9 can be used with driver. However I think adding
> > support to AT91RM9200 to this driver will be not much effort.
>
> Can you answer the simple question: why we should try to support two
> incompatible chips with a single driver? Because the driver name will be
> shorter? :-)
Very funny. I think patch adding RM9200 support to this driver will have less
than 50 lines changeset, whereas writing new driver would be about 500
lines.
> > I don't think
> > someone will want to write new driver for RM9200 insted using this one.
>
> You're right, nobody will want that... because AT91RM9200 as is has *no
> support for True IDE mode*. ;-)
Atmel documents are confusing. AT91RM9200 datasheet tells there is no
True IDE support, but RM9200 hard drive application note (which I send you
a link before) tell it is.
> >> Frankly speaking, I don't see why you're reading this register back
> >>if you already know what needs to be set there -- as you've done it in
> >>init_smc_mode().
>
> > This function is not only used at initialization. It is also used when PIO mode
> > is changed.
>
> So what? You can just write the fixed mode bits into the register every
> time without readback.
Ok, I see now.
Cheers
Stanislaw Gruszka
--
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
