Re: [RFC][PATCH] at91_ide driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Stanislaw Gruszka wrote:

+/* Proper CS address space will be added */
+#define AT91_IDE_TASK_FILE	0x00c00000
+#define AT91_IDE_CTRL_REG	0x00e00000

Besides, I'm not sure: these 2 address range are decoded via 1 SMC chip select or 2?

These are in one chip select address space.

   Then some external circutry converts the single nCS into -CS0 and -CS1?

Well, we could add #ifdef with diffrent implementation of init_smc_mode(), set_8bit_mode(), etc...

  No, #ifdef'ery is certainly not an option.

Why?

It's totally ugly and unacceptable way of doing things. It seems also totally wrong to add support for totally incompatible SMC to this driver (especially with #ifdef's). Another driver should be written if CF support is required.

Other option is create some header files and implement and exporting these
functions from processor specific code. This add files dependencies and spread
things across sources, FWIW.

   I don't think it's feasible as that SMC is just too different.

+	/* values are rounded up so we need to assure cycle is larger than pulse */
+	if (t0 < t1 + t2 + t9)
+		t0 = t1 + t2 + t9;
+
+	/* setup calculated timings */
+	at91_sys_write(AT91_SMC_SETUP(chipselect), AT91_SMC_NWESETUP_(t1) |
+						   AT91_SMC_NCS_WRSETUP_(0) |
+						   AT91_SMC_NRDSETUP_(t1) |
+						   AT91_SMC_NCS_RDSETUP_(0));
+	at91_sys_write(AT91_SMC_PULSE(chipselect), AT91_SMC_NWEPULSE_(t2) |
+						   AT91_SMC_NCS_WRPULSE_(t1 + t2 + t9) |

 With zero address to CS setup time it's the same as t0.

Not true for slower PIO modes.

  Well, you're right probably...

They should better be the same and in any case t1 + t2 + t9 must be no less than t0min.

But CS pulse can be t0.

  Yes, it must be no less than t0.

Pulse time can be less than t0, cycle time no.

nCS pulse time cannot be less than t0min as by address valid ATA spec means both address and -CSx valid, not just address.

Maybe this is simpler, host have to react on IORDY signal but device
just not assert it.

I would like to remove this code and have allways NWAIT
READY mode to make flipping 8/16 simpler.

  I don't understand how these are connected.

Both set SMC MODE register. If I will not change NWAIT in MODE register,
I can write static values in set_8/16bit_mode and stop doing logical operation on it.

   No, that's undesirable.

  Why not pass it normally, via the platform device's resource?

Irq pin is board specific. When use resource I will need in processor code modify resource.

  So what? That's pretty normal and won't take much code.

I see only more memory and code usage. Let's reverse question:
Why use RESOURCE_IRQ when we have irq in board data?

Len't reverse it again: why have it in board data when the normal location is the resource? :-)

Cheers
Stanislaw Gruszka

MBR, Sergei
--
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

[Index of Archives]     [Linux Filesystems]     [Linux SCSI]     [Linux RAID]     [Git]     [Kernel Newbies]     [Linux Newbie]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Samba]     [Device Mapper]

  Powered by Linux