lkml.org 
[lkml]   [2002]   [Apr]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH] 2.5.8 IDE 36
    From
    Date
    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
    [unhandled content-type:application/pgp-signature]
    \
     
     \ /
      Last update: 2005-03-22 13:25    [W:0.025 / U:0.664 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site