* Fix "initial_timing" structure initialisation. The "struct ata_timing" must
contain 10 members, but ".dmack_hold" member was not initialised.
* The AT91SAM9 microcontrollers use special coding scheme for SMC_SETUP,
SMC_PULSE, SMC_CYCLE registers.
Old driver operates correctly only with low master clock values, but
with high master clock it incorrectly calculates SMC values and stops
to operate, because SMC can be setup only to admissible ranges in special
format.
New code correctly calculates SMC registers values, adjusts calculated
to admissible ranges, enlarges cycles if required and converts values
into SMC's format.
* Old driver calculates CS_SETUP and CS_PULSE cycles incorrectly
because of wrong assumptions.
New code calculates:
CS_SETUP = SETUP/2
CS_PULSE = PULSE + SETUP/2 + HOLD/2
* This fixes allows to correctly operate with any master clock frequency.
Signed-off-by: Igor Plyatov <[email protected]>
---
drivers/ata/pata_at91.c | 223 ++++++++++++++++++++++++++++++++---------------
1 files changed, 154 insertions(+), 69 deletions(-)
diff --git a/drivers/ata/pata_at91.c b/drivers/ata/pata_at91.c
index 0da0dcc..b18249e 100644
--- a/drivers/ata/pata_at91.c
+++ b/drivers/ata/pata_at91.c
@@ -3,6 +3,7 @@
* with CompactFlash interface in True IDE mode
*
* Copyright (C) 2009 Matyukevich Sergey
+ * 2011 Igor Plyatov
*
* Based on:
* * generic platform driver by Paul Mundt: drivers/ata/pata_platform.c
@@ -31,26 +32,140 @@
#include <mach/board.h>
#include <mach/gpio.h>
+#define DRV_NAME "pata_at91"
+#define DRV_VERSION "0.2"
-#define DRV_NAME "pata_at91"
-#define DRV_VERSION "0.1"
-
-#define CF_IDE_OFFSET 0x00c00000
-#define CF_ALT_IDE_OFFSET 0x00e00000
-#define CF_IDE_RES_SIZE 0x08
+#define CF_IDE_OFFSET 0x00c00000
+#define CF_ALT_IDE_OFFSET 0x00e00000
+#define CF_IDE_RES_SIZE 0x08
struct at91_ide_info {
unsigned long mode;
unsigned int cs;
-
struct clk *mck;
-
void __iomem *ide_addr;
void __iomem *alt_addr;
};
-static const struct ata_timing initial_timing =
- {XFER_PIO_0, 70, 290, 240, 600, 165, 150, 600, 0};
+static const struct ata_timing initial_timing = {
+ .mode = XFER_PIO_0,
+ .setup = 70,
+ .act8b = 290,
+ .rec8b = 240,
+ .cyc8b = 600,
+ .active = 165,
+ .recover = 150,
+ .dmack_hold = 0,
+ .cycle = 600,
+ .udma = 0
+};
+
+/*
+ * Adjust value for one of SMC_SETUP, SMC_PULSE or SMC_CYCLE registers.
+ * Returns:
+ * difference between input and output value or
+ * negative value in case of invalid input value.
+ */
+static int adjust_smc_value(unsigned int *value, int *prange,
+ unsigned int size)
+{
+ unsigned char i;
+ int remainder;
+
+ for (i = 0; i < size; i = i + 2) {
+ if (*value < prange[i]) {
+ remainder = prange[i] - *value;
+ *value = prange[i]; /* nearest valid value */
+ return remainder;
+ }
+ else if ((prange[i] <= *value) && (*value <= prange[i+1])) {
+ return 0;
+ }
+ }
+ *value = prange[size - 1]; /* assign maximal possible value */
+
+ return -1; /* invalid value */
+}
+
+/*
+ * Calculate SMC_SETUP, SMC_PULSE, SMC_CYCLE register values.
+ *
+ * SMC use special coding scheme, see "Coding and Range of Timing
+ * Parameters" table from AT91SAM926x datasheets.
+ *
+ * SETUP = 128*setup[5] + setup[4:0]
+ * PULSE = 256*pulse[6] + pulse[5:0]
+ * CYCLE = 256*cycle[8:7] + cycle[6:0]
+ *
+ * cs_setup = setup/2
+ * cs_pulse = pulse + setup/2 + hold/2
+ */
+static void calc_smc_vals(struct device *dev,
+ unsigned int *setup, unsigned int *cs_setup,
+ unsigned int *pulse, unsigned int *cs_pulse,
+ unsigned int *cycle)
+{
+ int ret_val;
+ int cs_hold;
+ int range_setup[] = { /* SMC_SETUP valid ranges */
+ 0, 31, /* first range */
+ 128, 159, /* second range */
+ };
+ int range_pulse[] = { /* SMC_PULSE valid ranges */
+ 0, 63, /* first range */
+ 256, 319, /* second range */
+ };
+ int range_cycle[] = { /* SMC_CYCLE valid ranges */
+ 0, 127, /* first range */
+ 256, 383, /* second range */
+ 512, 639, /* third range */
+ 768, 895, /* fourth range */
+ };
+
+ ret_val = adjust_smc_value(setup, range_setup, sizeof(range_setup));
+ if (ret_val < 0)
+ dev_warn(dev, "maximal setup value\n");
+ else
+ *cycle += ret_val;
+
+ ret_val = adjust_smc_value(pulse, range_pulse, sizeof(range_pulse));
+ if (ret_val < 0)
+ dev_warn(dev, "maximal pulse value\n");
+ else
+ *cycle += ret_val;
+
+ ret_val = adjust_smc_value(cycle, range_cycle, sizeof(range_cycle));
+ if (ret_val < 0)
+ dev_warn(dev, "maximal cycle value\n");
+
+ *cs_setup = *setup / 2;
+ ret_val = adjust_smc_value(cs_setup, range_setup, sizeof(range_setup));
+ if (ret_val < 0)
+ dev_warn(dev, "maximal cs_setup value\n");
+
+ if (*cs_setup >= *setup) {
+ dev_warn(dev, "cs_setup value >= setup\n");
+ *cs_pulse = *pulse;
+ } else {
+ if ((*cs_setup == 0) && (*setup == 1))
+ *cs_setup = 1;
+
+ /* transformed pulse + setup/2 + hold/2 */
+ *cs_pulse = (*pulse + *cycle)/2;
+ ret_val = adjust_smc_value(cs_pulse, range_pulse,
+ sizeof(range_pulse));
+ if (ret_val < 0)
+ dev_warn(dev, "maximal cs_pulse value\n");
+ else if (ret_val > 0) {
+ cs_hold = *cycle - *cs_setup - *cs_pulse;
+ if (cs_hold < ret_val)
+ dev_warn(dev, "unable setup correct cs_pulse\n");
+ }
+ }
+
+ dev_dbg(dev, "setup=%u, cs_setup=%u, pulse=%u, cs_pulse=%u, cycle=%u\n",
+ *setup, *cs_setup, *pulse, *cs_pulse, *cycle);
+}
static unsigned long calc_mck_cycles(unsigned long ns, unsigned long mck_hz)
{
@@ -78,71 +193,43 @@ static void set_smc_mode(struct at91_ide_info *info)
static void set_smc_timing(struct device *dev,
struct at91_ide_info *info, const struct ata_timing *ata)
{
- unsigned long read_cycle, write_cycle, active, recover;
- unsigned long nrd_setup, nrd_pulse, nrd_recover;
- unsigned long nwe_setup, nwe_pulse;
-
- unsigned long ncs_write_setup, ncs_write_pulse;
- unsigned long ncs_read_setup, ncs_read_pulse;
-
- unsigned long mck_hz;
-
- read_cycle = ata->cyc8b;
- nrd_setup = ata->setup;
- nrd_pulse = ata->act8b;
- nrd_recover = ata->rec8b;
+ unsigned int cycle; /* ATA cycle width in MCK ticks */
+ unsigned int setup; /* setup width in MCK ticks */
+ unsigned int pulse; /* CFIOR and CFIOW pulse width in MCK ticks */
+ unsigned int cs_setup = 0;/* CS4 or CS5 setup width in MCK ticks */
+ unsigned int cs_pulse = 0;/* CS4 or CS5 pulse width in MCK ticks*/
+ unsigned long mck_hz; /* MCK frequency in Hz */
mck_hz = clk_get_rate(info->mck);
+ cycle = calc_mck_cycles(ata->cyc8b, mck_hz);
+ setup = calc_mck_cycles(ata->setup, mck_hz);
+ pulse = calc_mck_cycles(ata->act8b, mck_hz);
- read_cycle = calc_mck_cycles(read_cycle, mck_hz);
- nrd_setup = calc_mck_cycles(nrd_setup, mck_hz);
- nrd_pulse = calc_mck_cycles(nrd_pulse, mck_hz);
- nrd_recover = calc_mck_cycles(nrd_recover, mck_hz);
-
- active = nrd_setup + nrd_pulse;
- recover = read_cycle - active;
-
- /* Need at least two cycles recovery */
- if (recover < 2)
- read_cycle = active + 2;
-
- /* (CS0, CS1, DIR, OE) <= (CFCE1, CFCE2, CFRNW, NCSX) timings */
- ncs_read_setup = 1;
- ncs_read_pulse = read_cycle - 2;
-
- /* Write timings same as read timings */
- write_cycle = read_cycle;
- nwe_setup = nrd_setup;
- nwe_pulse = nrd_pulse;
- ncs_write_setup = ncs_read_setup;
- ncs_write_pulse = ncs_read_pulse;
-
- dev_dbg(dev, "ATA timings: nrd_setup = %lu nrd_pulse = %lu nrd_cycle = %lu\n",
- nrd_setup, nrd_pulse, read_cycle);
- dev_dbg(dev, "ATA timings: nwe_setup = %lu nwe_pulse = %lu nwe_cycle = %lu\n",
- nwe_setup, nwe_pulse, write_cycle);
- dev_dbg(dev, "ATA timings: ncs_read_setup = %lu ncs_read_pulse = %lu\n",
- ncs_read_setup, ncs_read_pulse);
- dev_dbg(dev, "ATA timings: ncs_write_setup = %lu ncs_write_pulse = %lu\n",
- ncs_write_setup, ncs_write_pulse);
+ calc_smc_vals(dev, &setup, &cs_setup, &pulse, &cs_pulse, &cycle);
+ /* Convert values into SMC format */
+ setup = (setup & 0x1f) | ((setup & 0x80) >> 2);
+ pulse = (pulse & 0x3f) | ((pulse & 0x100) >> 2);
+ cycle = (cycle & 0x7f) | ((cycle & 0x300) >> 1);
+ cs_setup = (cs_setup & 0x1f) | ((cs_setup & 0x80) >> 2);
+ cs_pulse = (cs_pulse & 0x3f) | ((cs_pulse & 0x100) >> 2);
+
+ /* SMC setup. Write timings are same as read timings */
at91_sys_write(AT91_SMC_SETUP(info->cs),
- AT91_SMC_NWESETUP_(nwe_setup) |
- AT91_SMC_NRDSETUP_(nrd_setup) |
- AT91_SMC_NCS_WRSETUP_(ncs_write_setup) |
- AT91_SMC_NCS_RDSETUP_(ncs_read_setup));
+ AT91_SMC_NWESETUP_(setup) |
+ AT91_SMC_NRDSETUP_(setup) |
+ AT91_SMC_NCS_WRSETUP_(cs_setup) |
+ AT91_SMC_NCS_RDSETUP_(cs_setup));
at91_sys_write(AT91_SMC_PULSE(info->cs),
- AT91_SMC_NWEPULSE_(nwe_pulse) |
- AT91_SMC_NRDPULSE_(nrd_pulse) |
- AT91_SMC_NCS_WRPULSE_(ncs_write_pulse) |
- AT91_SMC_NCS_RDPULSE_(ncs_read_pulse));
+ AT91_SMC_NWEPULSE_(pulse) |
+ AT91_SMC_NRDPULSE_(pulse) |
+ AT91_SMC_NCS_WRPULSE_(cs_pulse) |
+ AT91_SMC_NCS_RDPULSE_(cs_pulse));
at91_sys_write(AT91_SMC_CYCLE(info->cs),
- AT91_SMC_NWECYCLE_(write_cycle) |
- AT91_SMC_NRDCYCLE_(read_cycle));
-
- return;
+ AT91_SMC_NWECYCLE_(cycle) |
+ AT91_SMC_NRDCYCLE_(cycle));
}
static void pata_at91_set_piomode(struct ata_port *ap, struct ata_device *adev)
@@ -163,8 +250,6 @@ static void pata_at91_set_piomode(struct ata_port *ap, struct ata_device *adev)
/* Setup SMC mode */
set_smc_mode(info);
-
- return;
}
static unsigned int pata_at91_data_xfer_noirq(struct ata_device *dev,
@@ -330,7 +415,7 @@ static int __devexit pata_at91_remove(struct platform_device *pdev)
static struct platform_driver pata_at91_driver = {
.probe = pata_at91_probe,
.remove = __devexit_p(pata_at91_remove),
- .driver = {
+ .driver = {
.name = DRV_NAME,
.owner = THIS_MODULE,
},
--
1.7.1
Hi Igor
On Wed, Apr 20, 2011 at 03:44:37PM +0400, Igor Plyatov wrote:
> * Fix "initial_timing" structure initialisation. The "struct ata_timing" must
> contain 10 members, but ".dmack_hold" member was not initialised.
> * The AT91SAM9 microcontrollers use special coding scheme for SMC_SETUP,
> SMC_PULSE, SMC_CYCLE registers.
> Old driver operates correctly only with low master clock values, but
> with high master clock it incorrectly calculates SMC values and stops
> to operate, because SMC can be setup only to admissible ranges in special
> format.
> New code correctly calculates SMC registers values, adjusts calculated
> to admissible ranges, enlarges cycles if required and converts values
> into SMC's format.
> * Old driver calculates CS_SETUP and CS_PULSE cycles incorrectly
> because of wrong assumptions.
> New code calculates:
> CS_SETUP = SETUP/2
If you to this, then {RD,WR}_SETUP have to be equal SETUP + SETUP/2
to generate proper setup (t0) time on IDE bus. I think the best way is
set CS_SETUP and CS_HOLD to 0 (what your code do if SETUP and HOLD <=1),
but to do this you need to take care of data float (t6z)
> +static const struct ata_timing initial_timing = {
> + .mode = XFER_PIO_0,
> + .setup = 70,
> + .act8b = 290,
> + .rec8b = 240,
> + .cyc8b = 600,
> + .active = 165,
> + .recover = 150,
> + .dmack_hold = 0,
> + .cycle = 600,
> + .udma = 0
> +};
Is this really needed, why not use ata_timing_find_mode(XFER_PIO_0)?
> +static int adjust_smc_value(unsigned int *value, int *prange,
> + unsigned int size)
> +{
> + unsigned char i;
> + int remainder;
> +
> + for (i = 0; i < size; i = i + 2) {
> + if (*value < prange[i]) {
> + remainder = prange[i] - *value;
> + *value = prange[i]; /* nearest valid value */
> + return remainder;
> + }
> + else if ((prange[i] <= *value) && (*value <= prange[i+1])) {
> + return 0;
> + }
> + }
> + *value = prange[size - 1]; /* assign maximal possible value */
> +
> + return -1; /* invalid value */
> +}
[snip]
> +static void calc_smc_vals(struct device *dev,
> + unsigned int *setup, unsigned int *cs_setup,
> + unsigned int *pulse, unsigned int *cs_pulse,
> + unsigned int *cycle)
> +{
> + int ret_val;
> + int cs_hold;
> + int range_setup[] = { /* SMC_SETUP valid ranges */
> + 0, 31, /* first range */
> + 128, 159, /* second range */
> + };
> + int range_pulse[] = { /* SMC_PULSE valid ranges */
> + 0, 63, /* first range */
> + 256, 319, /* second range */
> + };
> + int range_cycle[] = { /* SMC_CYCLE valid ranges */
> + 0, 127, /* first range */
> + 256, 383, /* second range */
> + 512, 639, /* third range */
> + 768, 895, /* fourth range */
> + };
Introducing helper structure like
struct smc_range {
u16 min, max;
};
would be a bit cleaner way of coding that (hint: ARRAY_SIZE could be used
instead of sizeof then)
> - dev_dbg(dev, "ATA timings: nrd_setup = %lu nrd_pulse = %lu nrd_cycle = %lu\n",
> - nrd_setup, nrd_pulse, read_cycle);
> - dev_dbg(dev, "ATA timings: nwe_setup = %lu nwe_pulse = %lu nwe_cycle = %lu\n",
> - nwe_setup, nwe_pulse, write_cycle);
> - dev_dbg(dev, "ATA timings: ncs_read_setup = %lu ncs_read_pulse = %lu\n",
> - ncs_read_setup, ncs_read_pulse);
> - dev_dbg(dev, "ATA timings: ncs_write_setup = %lu ncs_write_pulse = %lu\n",
> - ncs_write_setup, ncs_write_pulse);
It's worth to have some debugging prints to check if values are calculated
properly.
Thanks
Stanislaw
On 04/20/2011 07:44 AM, Igor Plyatov wrote:
> * Fix "initial_timing" structure initialisation. The "struct ata_timing" must
> contain 10 members, but ".dmack_hold" member was not initialised.
> * The AT91SAM9 microcontrollers use special coding scheme for SMC_SETUP,
> SMC_PULSE, SMC_CYCLE registers.
> Old driver operates correctly only with low master clock values, but
> with high master clock it incorrectly calculates SMC values and stops
> to operate, because SMC can be setup only to admissible ranges in special
> format.
> New code correctly calculates SMC registers values, adjusts calculated
> to admissible ranges, enlarges cycles if required and converts values
> into SMC's format.
> * Old driver calculates CS_SETUP and CS_PULSE cycles incorrectly
> because of wrong assumptions.
> New code calculates:
> CS_SETUP = SETUP/2
> CS_PULSE = PULSE + SETUP/2 + HOLD/2
> * This fixes allows to correctly operate with any master clock frequency.
>
> Signed-off-by: Igor Plyatov<[email protected]>
> ---
> drivers/ata/pata_at91.c | 223 ++++++++++++++++++++++++++++++++---------------
> 1 files changed, 154 insertions(+), 69 deletions(-)
FYI, the two previous "v4" patches are now applied.
Based on the comments from Stanislaw, this patch would need to be
updated anyway.
Hi Stanislaw!
Thank you for patch review!
> Hi Igor
>
> On Wed, Apr 20, 2011 at 03:44:37PM +0400, Igor Plyatov wrote:
>> * Fix "initial_timing" structure initialisation. The "struct ata_timing" must
>> contain 10 members, but ".dmack_hold" member was not initialised.
>> * The AT91SAM9 microcontrollers use special coding scheme for SMC_SETUP,
>> SMC_PULSE, SMC_CYCLE registers.
>> Old driver operates correctly only with low master clock values, but
>> with high master clock it incorrectly calculates SMC values and stops
>> to operate, because SMC can be setup only to admissible ranges in special
>> format.
>> New code correctly calculates SMC registers values, adjusts calculated
>> to admissible ranges, enlarges cycles if required and converts values
>> into SMC's format.
>> * Old driver calculates CS_SETUP and CS_PULSE cycles incorrectly
>> because of wrong assumptions.
>> New code calculates:
>> CS_SETUP = SETUP/2
> If you to this, then {RD,WR}_SETUP have to be equal SETUP + SETUP/2
> to generate proper setup (t0) time on IDE bus. I think the best way is
> set CS_SETUP and CS_HOLD to 0 (what your code do if SETUP and HOLD<=1),
> but to do this you need to take care of data float (t6z)
Why so?
This is not clear for me. Maybe we talk about different things or I have
wrong
understanding of ATA timings.
Can you please look at "Standard Read Cycle" for AT91SAM9 MCU
http://www.kicad.ru/files/AT91SAM9G20%20bus%20timing.pdf
, CompactFlash connection schematic
http://www.kicad.ru/files/CF%20for%20AT91SAM9%20(True%20IDE%20mode).pdf
and comment my thoughts?
Here is a legend for "Standard Read Cycle" timing diagram.
------------------------------------------------------------------------------
Read (NRD) signal parameters:
* t0 = cycle = NRD_CYCLE
* t1 = setup = NRD_SETUP
* t2 = pulse = NRD_PULSE
* t9 = hold = NRD_HOLD
Chip Select (NCS) signal parameters:
* cs_setup = NCS_RD_SETUP
* cs_pulse = NCS_RD_PULSE
* cs_cycle = cycle
Notes:
* The NCS_RD_CYCLE is equal to the NRD_CYCLE for AT91SAM9, because they
start/finish simultaneously (HW related).
* The NCS signal is not the same as CS1, CS2 ATA signals! It used only to
enable data bus transceiver U2.
So NCS parameters calculated as
cs_setup = setup/2
cs_pulse = pulse + setup/2 + hold/2 = (pulse + cycle)/2
>> +static const struct ata_timing initial_timing = {
>> + .mode = XFER_PIO_0,
>> + .setup = 70,
>> + .act8b = 290,
>> + .rec8b = 240,
>> + .cyc8b = 600,
>> + .active = 165,
>> + .recover = 150,
>> + .dmack_hold = 0,
>> + .cycle = 600,
>> + .udma = 0
>> +};
> Is this really needed, why not use ata_timing_find_mode(XFER_PIO_0)?
OK. Fixed.
>> +static int adjust_smc_value(unsigned int *value, int *prange,
>> + unsigned int size)
>> +{
>> + unsigned char i;
>> + int remainder;
>> +
>> + for (i = 0; i< size; i = i + 2) {
>> + if (*value< prange[i]) {
>> + remainder = prange[i] - *value;
>> + *value = prange[i]; /* nearest valid value */
>> + return remainder;
>> + }
>> + else if ((prange[i]<= *value)&& (*value<= prange[i+1])) {
>> + return 0;
>> + }
>> + }
>> + *value = prange[size - 1]; /* assign maximal possible value */
>> +
>> + return -1; /* invalid value */
>> +}
> [snip]
>> +static void calc_smc_vals(struct device *dev,
>> + unsigned int *setup, unsigned int *cs_setup,
>> + unsigned int *pulse, unsigned int *cs_pulse,
>> + unsigned int *cycle)
>> +{
>> + int ret_val;
>> + int cs_hold;
>> + int range_setup[] = { /* SMC_SETUP valid ranges */
>> + 0, 31, /* first range */
>> + 128, 159, /* second range */
>> + };
>> + int range_pulse[] = { /* SMC_PULSE valid ranges */
>> + 0, 63, /* first range */
>> + 256, 319, /* second range */
>> + };
>> + int range_cycle[] = { /* SMC_CYCLE valid ranges */
>> + 0, 127, /* first range */
>> + 256, 383, /* second range */
>> + 512, 639, /* third range */
>> + 768, 895, /* fourth range */
>> + };
> Introducing helper structure like
>
> struct smc_range {
> u16 min, max;
> };
>
> would be a bit cleaner way of coding that (hint: ARRAY_SIZE could be used
> instead of sizeof then)
OK. Fixed.
>> - dev_dbg(dev, "ATA timings: nrd_setup = %lu nrd_pulse = %lu nrd_cycle = %lu\n",
>> - nrd_setup, nrd_pulse, read_cycle);
>> - dev_dbg(dev, "ATA timings: nwe_setup = %lu nwe_pulse = %lu nwe_cycle = %lu\n",
>> - nwe_setup, nwe_pulse, write_cycle);
>> - dev_dbg(dev, "ATA timings: ncs_read_setup = %lu ncs_read_pulse = %lu\n",
>> - ncs_read_setup, ncs_read_pulse);
>> - dev_dbg(dev, "ATA timings: ncs_write_setup = %lu ncs_write_pulse = %lu\n",
>> - ncs_write_setup, ncs_write_pulse);
> It's worth to have some debugging prints to check if values are calculated
> properly.
They exists already in the calc_smc_vals() function, but now I move them
to set_smc_timing().
> Thanks
> Stanislaw
Best regards!
--
Igor Plyatov
On Tue, Apr 26, 2011 at 10:38:04AM +0400, Igor Plyatov wrote:
> >On Wed, Apr 20, 2011 at 03:44:37PM +0400, Igor Plyatov wrote:
> >>* Fix "initial_timing" structure initialisation. The "struct ata_timing" must
> >> contain 10 members, but ".dmack_hold" member was not initialised.
> >>* The AT91SAM9 microcontrollers use special coding scheme for SMC_SETUP,
> >> SMC_PULSE, SMC_CYCLE registers.
> >> Old driver operates correctly only with low master clock values, but
> >> with high master clock it incorrectly calculates SMC values and stops
> >> to operate, because SMC can be setup only to admissible ranges in special
> >> format.
> >> New code correctly calculates SMC registers values, adjusts calculated
> >> to admissible ranges, enlarges cycles if required and converts values
> >> into SMC's format.
> >>* Old driver calculates CS_SETUP and CS_PULSE cycles incorrectly
> >> because of wrong assumptions.
> >> New code calculates:
> >> CS_SETUP = SETUP/2
> >If you to this, then {RD,WR}_SETUP have to be equal SETUP + SETUP/2
> >to generate proper setup (t0) time on IDE bus. I think the best way is
err, setup time is t1
> >set CS_SETUP and CS_HOLD to 0 (what your code do if SETUP and HOLD<=1),
> >but to do this you need to take care of data float (t6z)
>
> Why so?
>
> This is not clear for me. Maybe we talk about different things or I
> have wrong
> understanding of ATA timings.
> Can you please look at "Standard Read Cycle" for AT91SAM9 MCU
> http://www.kicad.ru/files/AT91SAM9G20%20bus%20timing.pdf
> , CompactFlash connection schematic
> http://www.kicad.ru/files/CF%20for%20AT91SAM9%20(True%20IDE%20mode).pdf
> and comment my thoughts?
Full "External Bus Interface" section from AT91 spec is important,
because is describe how SMC works internally in CF True IDE mode.
> Here is a legend for "Standard Read Cycle" timing diagram.
> ------------------------------------------------------------------------------
> Read (NRD) signal parameters:
> * t0 = cycle = NRD_CYCLE
> * t1 = setup = NRD_SETUP
> * t2 = pulse = NRD_PULSE
> * t9 = hold = NRD_HOLD
Correct. Also t0 = t1 + t2 + t9 relation must be true.
> Chip Select (NCS) signal parameters:
> * cs_setup = NCS_RD_SETUP
> * cs_pulse = NCS_RD_PULSE
> * cs_cycle = cycle
You can setup that, but they need to have proper relations with NRD.
> Notes:
> * The NCS_RD_CYCLE is equal to the NRD_CYCLE for AT91SAM9, because they
> start/finish simultaneously (HW related).
No, NCS and NRD are different signals, waveforms are different (if configured
differently in SMC controller).
> * The NCS signal is not the same as CS1, CS2 ATA signals! It used only to
> enable data bus transceiver U2.
Well, they are different signals but connected together. All of these are
controlled by NCS: CFCE1 (CS1), CFCE2 (CS2), CFRNW (DIR), CFCS0 (OE)
if SMC is configured in True IDE mode.
Stanislaw
> > * The NCS_RD_CYCLE is equal to the NRD_CYCLE for AT91SAM9, because they
> > start/finish simultaneously (HW related).
> No, NCS and NRD are different signals, waveforms are different (if
> configured differently in SMC controller).
Your sentence is true. NRD and NCS cycle time must be the same (otherwise
SMC behaviour is undefined), hence both signals start/finish simultaneously.
Anyway waveforms (setup, and pulse time) can be different. All of thise
are kinda obvious, no? :-)
Stanislaw
Hi Jeff!
> On 04/20/2011 07:44 AM, Igor Plyatov wrote:
>> * Fix "initial_timing" structure initialisation. The "struct
>> ata_timing" must
>> contain 10 members, but ".dmack_hold" member was not initialised.
>> * The AT91SAM9 microcontrollers use special coding scheme for SMC_SETUP,
>> SMC_PULSE, SMC_CYCLE registers.
>> Old driver operates correctly only with low master clock values, but
>> with high master clock it incorrectly calculates SMC values and stops
>> to operate, because SMC can be setup only to admissible ranges in
>> special
>> format.
>> New code correctly calculates SMC registers values, adjusts
>> calculated
>> to admissible ranges, enlarges cycles if required and converts values
>> into SMC's format.
>> * Old driver calculates CS_SETUP and CS_PULSE cycles incorrectly
>> because of wrong assumptions.
>> New code calculates:
>> CS_SETUP = SETUP/2
>> CS_PULSE = PULSE + SETUP/2 + HOLD/2
>> * This fixes allows to correctly operate with any master clock
>> frequency.
>>
>> Signed-off-by: Igor Plyatov<[email protected]>
>> ---
>> drivers/ata/pata_at91.c | 223
>> ++++++++++++++++++++++++++++++++---------------
>> 1 files changed, 154 insertions(+), 69 deletions(-)
>
> FYI, the two previous "v4" patches are now applied.
>
> Based on the comments from Stanislaw, this patch would need to be
> updated anyway.
Thank you for info!
I will update this patch.
Best regards!
--
Igor Plyatov