Re: [PATCH] 2.5.8 IDE 36

Norbert Kiesel
2002-04-16T10:27:53Z

[-- Attachment #1: Type: text/plain, Size: 2788 bytes --]

I can for sure provide a patch, testing will take a bit longer because I
currently only use 2.4.x. Give me 24h...

--nk

On Tue, 2002-04-16 at 02:20, Martin Dalecki wrote:
> Norbert Kiesel wrote:
> > On Tue, 2002-04-16 at 01:21, Martin Dalecki wrote:
> >
> >>Norbert Kiesel wrote:
> >>
> >>>Hi,
> >>>
> >>>while trying to understand recent kernel changes I stumbled over
> >>>the following patch to
> >>>
> >>>diff -urN linux-2.5.8/drivers/ide/ide.c linux/drivers/ide/ide.c
> >>>--- linux-2.5.8/drivers/ide/ide.c Tue Apr 16 06:01:07 2002
> >>>+++ linux/drivers/ide/ide.c Tue Apr 16 05:38:37 2002
> >>>
> >>>...
> >>> while (i > 0) {
> >>>- u32 buffer[16];
> >>>- unsigned int wcount = (i > 16) ? 16 : i;
> >>>- i -= wcount;
> >>>- ata_input_data (drive, buffer, wcount);
> >>>+ u32 buffer[SECTOR_WORDS];
> >>>+ unsigned int count = (i > 1) ? 1 : i;
> >>>+
> >>>+ ata_read(drive, buffer, count * SECTOR_WORDS);
> >>>+ i -= count;
> >>> }
> >>> }
> >>>...
> >>>
> >>>While the old code called ata_input_read() with [0:16] as last param,
> >>>the new code calls the (renamed) ata_read() with either 0 or 16. Also,
> >>>the new code loops "i" times while the old code looped "i/16+1" times.
> >>>Was this intended or should the patch better read like:
> >>>
> >>>...
> >>> while (i > 0) {
> >>>- u32 buffer[16];
> >>>- unsigned int wcount = (i > 16) ? 16 : i;
> >>>- i -= wcount;
> >>>- ata_input_data (drive, buffer, wcount);
> >>>+ u32 buffer[SECTOR_WORDS];
> >>>+ unsigned int count = max(i, SECTOR_WORDS);
> >>>+
> >>>+ ata_read(drive, buffer, count);
> >>>+ i -= count;
> >>> }
> >>> }
> >>>...
> >>>
> >>>so long
> >>
> >>It's fine as it is I think. Please look up at the initialization of i.
> >>I have just divded the SECTROT_WORDS (== 16) factor out
> >>of all the places above ata_read.
> >>
> >
> >
> > You are right (assuming SECTOR_WORDS == 16. I was looking it up in
> > 2.4.18 where SECTOR_WORDS is 512/4 == 128). However, the new code looks
> > overly complicated (at least for me, easily proven by my wrong first
> > email :-), given that count is now always == 1. Would the following not
> > be nicer?
> >
> > int i;
> >
> > if (drive->type != ATA_DISK)
> > return;
> >
> > for (i = min(drive->mult_count, 1); i > 0; i--) {
> > u32 buffer[SECTOR_WORDS];
> >
> > ata_read(drive, buffer, SECTOR_WORDS);
> > }
> >
> > (This of course assumes that drive->mult_count is always non-negative)
>
> Yes this looks nicer. Would you mind to test it and drop me
> a patch?
>
--
Key fingerprint = 6C58 F18D 4747 3295 F2DB 15C1 3882 4302 F8B4 C11C

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 232 bytes --]