Message ID | 20101216235153.GA3793@r2bh72.net.upc.cz |
---|---|
State | RFC |
Delegated to: | David Miller |
Headers | show |
Hello Stanislaw! > On Mon, Dec 13, 2010 at 10:35:49PM +0100, Stanislaw Gruszka wrote: >> On Sat, Dec 11, 2010 at 11:45:26PM +0300, Igor Plyatov wrote: >>> The AT91SAM9 microcontrollers with master clock higher then 105 MHz >>> and PIO0, have overflow of the NCS_RD_PULSE value in the MSB. This >>> lead to "NCS_RD_PULSE" pulse longer then "NRD_CYCLE" pulse and driver >>> does not detect IDE device. >> The overflow happens because MSB (bit 6) is multiplied by 256. >> NCS pulse length = 256*NCS_RD_PULSE[6] + NCS_RD_PULSE[5:0] clock >> cycles. So NCS_RD_PULSE 0x41 gives 257 clock cycles not 65 as we >> expected before. Static memory controller behaviour is undefined >> when pulse length is bigger than cycle length, so things not work. >> >>> + u16 ncs_rd_pulse; >>> unsigned long mode = AT91_SMC_READMODE | AT91_SMC_WRITEMODE | >>> AT91_SMC_BAT_SELECT; >>> >>> @@ -81,19 +84,29 @@ static void set_smc_timings(const u8 chipselect, const u16 cycle, >>> if (data_float) >>> mode |= AT91_SMC_TDF_(data_float); >>> >>> + ncs_rd_pulse = cycle; >>> + if (ncs_rd_pulse> NCS_RD_PULSE_LIMIT) { >>> + ncs_rd_pulse = NCS_RD_PULSE_LIMIT; >>> + pr_warn(DRV_NAME ": ncs_rd_pulse limited to maximal value %d\n", >>> + ncs_rd_pulse); >>> + } >> I'm fine with that fix. We can possibly still have problems with higher >> frequencies, but I'm not sure if someone will use that hardware with >> faster clocks. > I looked at patch again and change mind, I'm not ok with it. EBI NCS in > this case controls compact flash CS0, CS1 signals, so NCS pulse define > a cycle on IDE bus. IDE cycle length can not be smaller than t0 (from > PIO Mode Timing Diagram), otherwise we do not conform spec. > > IMHO what should be done in case of overflow is increase NCS pulse time > (IDE cycle) and in consequence overall SMC cycle time. Below draw patch > how this could be done. I do not even tried to compile it (give up with > that since I do not have hardware to test anyway), just idea how things > could be possibly done. > > diff --git a/drivers/ide/at91_ide.c b/drivers/ide/at91_ide.c > index 000a78e..277224c 100644 > --- a/drivers/ide/at91_ide.c > +++ b/drivers/ide/at91_ide.c > @@ -66,9 +66,8 @@ > > #define leave_16bit(cs, mode) at91_sys_write(AT91_SMC_MODE(cs), mode); > > -static void set_smc_timings(const u8 chipselect, const u16 cycle, > - const u16 setup, const u16 pulse, > - const u16 data_float, int use_iordy) > +static void set_smc_timings(const u8 chipselect, u16 cycle, u16 cs_cycle, > + u16 setup, u16 pulse, u16 data_float, int use_iordy) > { > unsigned long mode = AT91_SMC_READMODE | AT91_SMC_WRITEMODE | > AT91_SMC_BAT_SELECT; > @@ -89,9 +88,9 @@ static void set_smc_timings(const u8 chipselect, const u16 cycle, > AT91_SMC_NRDSETUP_(setup) | > AT91_SMC_NCS_RDSETUP_(0)); > at91_sys_write(AT91_SMC_PULSE(chipselect), AT91_SMC_NWEPULSE_(pulse) | > - AT91_SMC_NCS_WRPULSE_(cycle) | > + AT91_SMC_NCS_WRPULSE_(cs_cycle) | > AT91_SMC_NRDPULSE_(pulse) | > - AT91_SMC_NCS_RDPULSE_(cycle)); > + AT91_SMC_NCS_RDPULSE_(cs_cycle)); > at91_sys_write(AT91_SMC_CYCLE(chipselect), AT91_SMC_NWECYCLE_(cycle) | > AT91_SMC_NRDCYCLE_(cycle)); > } > @@ -106,11 +105,44 @@ static unsigned int calc_mck_cycles(unsigned int ns, unsigned int mck_hz) > return (unsigned int) tmp; > } > > +/* > + * In SMC registers values are coded in form: > + * > + * mul * ((val& (limit + 1)) ? 1 : 0) + (val& limit) > + * > + * where limit can be 0x1f, 0x3f or 0x7f, and mul can be 128 or 256 > + */ > +static int code_smc_values(int *val, const int limit, const int mul) > +{ > + int tmp; > + > + if (*val<= limit) > + return 0; > + > + /* limit< val< mul */ > + tmp = mul - *val; > + if (tmp> 0) { > + *val = limit + 1; > + return tmp; > + } > + > + /* mul + limit< val */ > + if (WARN_ON(*val - mul> limit)) { > + /* it's not possible to code that value - set max */ > + *val = (limit + 1) | limit; > + return 0; > + } > + > + /* mul<= val<= mul + limit */ > + *val = (limit + 1) | (*val - mul); > + return 0; > +} > + > static void apply_timings(const u8 chipselect, const u8 pio, > const struct ide_timing *timing, int use_iordy) > { > unsigned int t0, t1, t2, t6z; > - unsigned int cycle, setup, pulse, data_float; > + unsigned int cycle, cs_cycle, setup, pulse, data_float; > unsigned int mck_hz; > struct clk *mck; > > @@ -133,11 +165,29 @@ static void apply_timings(const u8 chipselect, const u8 pio, > setup = calc_mck_cycles(t1, mck_hz); > pulse = calc_mck_cycles(t2, mck_hz); > data_float = calc_mck_cycles(t6z, mck_hz); > - > - pdbg("cycle=%u setup=%u pulse=%u data_float=%u\n", > - cycle, setup, pulse, data_float); > - > - set_smc_timings(chipselect, cycle, setup, pulse, data_float, use_iordy); > + cs_cycle = cycle; > + > + pdbg("cycle=%u cs_cycle=%u setup=%u pulse=%u data_float=%u\n", > + cycle, cs_cycle, setup, pulse, data_float); > + > + /* 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] > + */ > + cycle += code_smc_values(&setup, 31, 128); > + cycle += code_smc_values(&pulse, 63, 256); > + /* cs_cycle is a full pulse wave, setup and hold times are 0 */ > + cycle += code_smc_values(&cs_cycle, 63, 256); > + code_cmc_values(&cycle, 127, 256); > + > + pdbg("cycle=0x%x cs_cycle=0x%x setup=0x%x pulse=0x%x data_float=0x%x\n", > + cycle, cs_cycle, setup, pulse, data_float); > + > + set_smc_timings(chipselect, cycle, cs_cycle, setup, pulse, data_float, > + use_iordy); > } > > static void at91_ide_input_data(ide_drive_t *drive, struct ide_cmd *cmd, Unfortunately, yours patch can't work correctly because it does not consider all details, but I understand yours idea and fix pata_at91.c driver. You can look to email with "[PATCH] ATA: pata_at91.c bugfixes" subject for details. Best regards! -- Igor Plyatov -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/ide/at91_ide.c b/drivers/ide/at91_ide.c index 000a78e..277224c 100644 --- a/drivers/ide/at91_ide.c +++ b/drivers/ide/at91_ide.c @@ -66,9 +66,8 @@ #define leave_16bit(cs, mode) at91_sys_write(AT91_SMC_MODE(cs), mode); -static void set_smc_timings(const u8 chipselect, const u16 cycle, - const u16 setup, const u16 pulse, - const u16 data_float, int use_iordy) +static void set_smc_timings(const u8 chipselect, u16 cycle, u16 cs_cycle, + u16 setup, u16 pulse, u16 data_float, int use_iordy) { unsigned long mode = AT91_SMC_READMODE | AT91_SMC_WRITEMODE | AT91_SMC_BAT_SELECT; @@ -89,9 +88,9 @@ static void set_smc_timings(const u8 chipselect, const u16 cycle, AT91_SMC_NRDSETUP_(setup) | AT91_SMC_NCS_RDSETUP_(0)); at91_sys_write(AT91_SMC_PULSE(chipselect), AT91_SMC_NWEPULSE_(pulse) | - AT91_SMC_NCS_WRPULSE_(cycle) | + AT91_SMC_NCS_WRPULSE_(cs_cycle) | AT91_SMC_NRDPULSE_(pulse) | - AT91_SMC_NCS_RDPULSE_(cycle)); + AT91_SMC_NCS_RDPULSE_(cs_cycle)); at91_sys_write(AT91_SMC_CYCLE(chipselect), AT91_SMC_NWECYCLE_(cycle) | AT91_SMC_NRDCYCLE_(cycle)); } @@ -106,11 +105,44 @@ static unsigned int calc_mck_cycles(unsigned int ns, unsigned int mck_hz) return (unsigned int) tmp; } +/* + * In SMC registers values are coded in form: + * + * mul * ((val & (limit + 1)) ? 1 : 0) + (val & limit) + * + * where limit can be 0x1f, 0x3f or 0x7f, and mul can be 128 or 256 + */ +static int code_smc_values(int *val, const int limit, const int mul) +{ + int tmp; + + if (*val <= limit) + return 0; + + /* limit < val < mul */ + tmp = mul - *val; + if (tmp > 0) { + *val = limit + 1; + return tmp; + } + + /* mul + limit < val */ + if (WARN_ON(*val - mul > limit)) { + /* it's not possible to code that value - set max */ + *val = (limit + 1) | limit; + return 0; + } + + /* mul <= val <= mul + limit */ + *val = (limit + 1) | (*val - mul); + return 0; +} + static void apply_timings(const u8 chipselect, const u8 pio, const struct ide_timing *timing, int use_iordy) { unsigned int t0, t1, t2, t6z; - unsigned int cycle, setup, pulse, data_float; + unsigned int cycle, cs_cycle, setup, pulse, data_float; unsigned int mck_hz; struct clk *mck; @@ -133,11 +165,29 @@ static void apply_timings(const u8 chipselect, const u8 pio, setup = calc_mck_cycles(t1, mck_hz); pulse = calc_mck_cycles(t2, mck_hz); data_float = calc_mck_cycles(t6z, mck_hz); - - pdbg("cycle=%u setup=%u pulse=%u data_float=%u\n", - cycle, setup, pulse, data_float); - - set_smc_timings(chipselect, cycle, setup, pulse, data_float, use_iordy); + cs_cycle = cycle; + + pdbg("cycle=%u cs_cycle=%u setup=%u pulse=%u data_float=%u\n", + cycle, cs_cycle, setup, pulse, data_float); + + /* 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] + */ + cycle += code_smc_values(&setup, 31, 128); + cycle += code_smc_values(&pulse, 63, 256); + /* cs_cycle is a full pulse wave, setup and hold times are 0 */ + cycle += code_smc_values(&cs_cycle, 63, 256); + code_cmc_values(&cycle, 127, 256); + + pdbg("cycle=0x%x cs_cycle=0x%x setup=0x%x pulse=0x%x data_float=0x%x\n", + cycle, cs_cycle, setup, pulse, data_float); + + set_smc_timings(chipselect, cycle, cs_cycle, setup, pulse, data_float, + use_iordy); } static void at91_ide_input_data(ide_drive_t *drive, struct ide_cmd *cmd,