Re: [PATCH] 2.5.8 IDE 36

From: Norbert Kiesel (nkiesel@tbdnetworks.com)
Date: Tue Apr 16 2002 - 05:06:54 EST


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)

--nk

 

-- 
Key fingerprint = 6C58 F18D 4747 3295 F2DB  15C1 3882 4302 F8B4 C11C


- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Tue Apr 23 2002 - 22:00:14 EST