Hi,
On Wednesday 10 September 2008, Borislav Petkov wrote:
> Hi,
>
> On Mon, Sep 08, 2008 at 12:15:19AM +0200, Bartlomiej Zolnierkiewicz wrote:
> > * Use drive->capacity64 for caching current capacity.
> >
> > * Switch ide_floppy_capacity() to use drive->capacity64.
> >
> > * Call set_capacity() in idefloppy_open() and ide_floppy_probe()
> > instead of ide_floppy_get_capacity().
> >
> > There should be no functional changes caused by this patch.
> >
> > Cc: Borislav Petkov <petkovbb@xxxxxxxxx>
> > Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@xxxxxxxxx>
> > ---
> > drivers/ide/ide-floppy.c | 23 +++++++++++++----------
> > 1 file changed, 13 insertions(+), 10 deletions(-)
> >
> > Index: b/drivers/ide/ide-floppy.c
> > ===================================================================
> > --- a/drivers/ide/ide-floppy.c
> > +++ b/drivers/ide/ide-floppy.c
> > @@ -445,7 +445,9 @@ static int ide_floppy_get_flexible_disk_
> > drive->name, lba_capacity, capacity);
> > floppy->blocks = floppy->block_size ?
> > capacity / floppy->block_size : 0;
> > + drive->capacity64 = floppy->blocks * floppy->bs_factor;
>
> why do we do the assignment only in the capacity < lba_capacity case?
> drive->capacity64 is the total number of sectors, shouldn't we do
>
> drive->capacity64 = floppy->blocks;
>
> in the floppy->bs_factor == 1 case? Otherwise you have the case of calling
Probably we should...
> idefloppy_setup()
> |-> ide_floppy_get_capacity()
> |-> ide_floppy_get_flexible_disk_page()
>
> and having drive->capacity64 == 0 in the (capacity >= lba_capacity) case which
> assigns a capacity of 0 to disk->capacity in the set_capacity() call later ...
>
> or am I missing something?
...my patch just modified the code to also set ->capacity64 in places
which previously were modifying ->blocks and/or ->bs_factor (since
->capacity64 replaced open-coded ->blocks * ->bs_factor calculation),
so I think that the above problem is as an orthogonal issue and it
is the best to address it in separate pre- or post- patch (could you
please take care of it?).
[...]
> > @@ -547,17 +551,12 @@ static int ide_floppy_get_capacity(ide_d
> > if (!(drive->atapi_flags & IDE_AFLAG_CLIK_DRIVE))
> > (void) ide_floppy_get_flexible_disk_page(drive);
> >
> > - set_capacity(disk, floppy->blocks * floppy->bs_factor);
> > -
> > return rc;
> > }
> >
> > sector_t ide_floppy_capacity(ide_drive_t *drive)
> > {
> > - idefloppy_floppy_t *floppy = drive->driver_data;
> > - unsigned long capacity = floppy->blocks * floppy->bs_factor;
> > -
> > - return capacity;
> > + return drive->capacity64;
>
> you can simplify this one even further by killing ide_floppy_capacity() and
> doing
>
> set_capacity(disk, floppy->drive->capacity64);
I did it ide_floppy_capacity()-way to match ide_disk_capacity()
and ease the merge (probably ide_gd_capacity() can be removed now).
Thanks,
Bart
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
