| Message ID | 1292099846-24528-1-git-send-email-plyatov@gmail.com |
|---|---|
| State | Not Applicable |
| Delegated to: | David Miller |
| Headers | show |
On 12/11/2010 03:37 PM, 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 ATA device. > > Signed-off-by: Igor Plyatov<plyatov@gmail.com> Thanks, queued (absent further comments...) -- 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
Hello. On 11-12-2010 23:37, 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 ATA device. > Signed-off-by: Igor Plyatov<plyatov@gmail.com> [...] > diff --git a/drivers/ata/pata_at91.c b/drivers/ata/pata_at91.c > index 0da0dcc..a462405 100644 > --- a/drivers/ata/pata_at91.c > +++ b/drivers/ata/pata_at91.c [...] > @@ -50,7 +52,7 @@ struct at91_ide_info { > }; > > static const struct ata_timing initial_timing = > - {XFER_PIO_0, 70, 290, 240, 600, 165, 150, 600, 0}; > + {XFER_PIO_0, 70, 290, 240, 600, 165, 150, 0, 600, 0}; BTW, you haven't described this in the changelog... WBR, Sergei -- 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
Dear Sergei, > Hello. > > On 11-12-2010 23:37, 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 ATA device. > > > Signed-off-by: Igor Plyatov<plyatov@gmail.com> > [...] Please use more descriptive comments, because it is not clear what you mean here. Maybe here you just cut a text... It is so strange to cut such small amount of text and I does not have any idea why here is "[...]" exists. > > diff --git a/drivers/ata/pata_at91.c b/drivers/ata/pata_at91.c > > index 0da0dcc..a462405 100644 > > --- a/drivers/ata/pata_at91.c > > +++ b/drivers/ata/pata_at91.c > [...] The same as above. Nobody are able to read you mind on the distance :-) > > @@ -50,7 +52,7 @@ struct at91_ide_info { > > }; > > > > static const struct ata_timing initial_timing = > > - {XFER_PIO_0, 70, 290, 240, 600, 165, 150, 600, 0}; > > + {XFER_PIO_0, 70, 290, 240, 600, 165, 150, 0, 600, 0}; > > BTW, you haven't described this in the changelog... Here is just a typo fixed for the ata_timing structure. This typo does not have influence on the driver operation, but I do not like to leave such a negligible problem as it is. > WBR, Sergei 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
Hello. On 12-12-2010 22:02, 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 ATA device. >> >>> Signed-off-by: Igor Plyatov<plyatov@gmail.com> >> [...] > Please use more descriptive comments, because it is not clear what you > mean here. Maybe here you just cut a text... Exactly, this is how I mark the cut out text. > It is so strange to cut such small amount of text and I does not have > any idea why here is "[...]" exists. Well, it's still better than leaving large patches quoted and uncommented, having only one comment at the top, as some people do. :-) >>> diff --git a/drivers/ata/pata_at91.c b/drivers/ata/pata_at91.c >>> index 0da0dcc..a462405 100644 >>> --- a/drivers/ata/pata_at91.c >>> +++ b/drivers/ata/pata_at91.c >> [...] > The same as above. Nobody are able to read you mind on the distance :-) It's the first time anybody tried to iterpret my [...] marks as comments. :-) >>> @@ -50,7 +52,7 @@ struct at91_ide_info { >>> }; >>> >>> static const struct ata_timing initial_timing = >>> - {XFER_PIO_0, 70, 290, 240, 600, 165, 150, 600, 0}; >>> + {XFER_PIO_0, 70, 290, 240, 600, 165, 150, 0, 600, 0}; >> BTW, you haven't described this in the changelog... > Here is just a typo fixed for the ata_timing structure. This typo does > not have influence on the driver operation, but I do not like to leave > such a negligible problem as it is. You at least should have noted that in the changelog. And as the change is unrelated to the other changes you're doing, it's a good practice to put it into a separate patch. >> WBR, Sergei > Best regards! > -- > Igor Plyatov WBR, Sergei -- 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
On 12/13/2010 06:02 AM, Sergei Shtylyov wrote: > On 12-12-2010 22:02, Igor Plyatov wrote: >>>> static const struct ata_timing initial_timing = >>>> - {XFER_PIO_0, 70, 290, 240, 600, 165, 150, 600, 0}; >>>> + {XFER_PIO_0, 70, 290, 240, 600, 165, 150, 0, 600, 0}; > >>> BTW, you haven't described this in the changelog... > >> Here is just a typo fixed for the ata_timing structure. This typo does >> not have influence on the driver operation, but I do not like to leave >> such a negligible problem as it is. > > You at least should have noted that in the changelog. And as the change > is unrelated to the other changes you're doing, it's a good practice to > put it into a separate patch. Agreed... -- 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/ata/pata_at91.c b/drivers/ata/pata_at91.c index 0da0dcc..a462405 100644 --- a/drivers/ata/pata_at91.c +++ b/drivers/ata/pata_at91.c @@ -33,12 +33,14 @@ #define DRV_NAME "pata_at91" -#define DRV_VERSION "0.1" +#define DRV_VERSION "0.2" #define CF_IDE_OFFSET 0x00c00000 #define CF_ALT_IDE_OFFSET 0x00e00000 #define CF_IDE_RES_SIZE 0x08 +#define NCS_RD_PULSE_LIMIT 0x3f /* maximal value for pulse bitfields */ + struct at91_ide_info { unsigned long mode; unsigned int cs; @@ -50,7 +52,7 @@ struct at91_ide_info { }; static const struct ata_timing initial_timing = - {XFER_PIO_0, 70, 290, 240, 600, 165, 150, 600, 0}; + {XFER_PIO_0, 70, 290, 240, 600, 165, 150, 0, 600, 0}; static unsigned long calc_mck_cycles(unsigned long ns, unsigned long mck_hz) { @@ -109,6 +111,11 @@ static void set_smc_timing(struct device *dev, /* (CS0, CS1, DIR, OE) <= (CFCE1, CFCE2, CFRNW, NCSX) timings */ ncs_read_setup = 1; ncs_read_pulse = read_cycle - 2; + if (ncs_read_pulse > NCS_RD_PULSE_LIMIT) { + ncs_read_pulse = NCS_RD_PULSE_LIMIT; + dev_warn(dev, "ncs_read_pulse limited to maximal value %lu\n", + ncs_read_pulse); + } /* Write timings same as read timings */ write_cycle = read_cycle;
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 ATA device. Signed-off-by: Igor Plyatov <plyatov@gmail.com> --- drivers/ata/pata_at91.c | 11 +++++++++-- 1 files changed, 9 insertions(+), 2 deletions(-)