Re: [PATCH] ide-disk.c rev 1.13 killed CONFIG_IDEDISK_STROKE

[Bartlomiej Zolnierkiewicz <B.Zolnierkiewicz@elka.pw.edu.pl>]

On Thu, 7 Aug 2003, Andries Brouwer wrote:

> On Wed, Aug 06, 2003 at 08:32:23PM +0200, Bartlomiej Zolnierkiewicz wrote:
>
> > diff -puN drivers/ide/ide-disk.c~ide-disk-capacity-init-cleanup drivers/ide/ide-disk.c
> > --- linux-2.6.0-test2-bk5/drivers/ide/ide-disk.c~ide-disk-capacity-init-cleanup	2003-08-06 02:48:33.000000000 +0200
>
> Ha - and you didnt even tell me you had this patch out.

:-)

> Looks good.
> You forgot to correct the do_div.

Yep, Erik noticed this already.

> The part I most object to are things like
>
> > +	id->lba_capacity_2 = capacity_2 = set_max_ext;
>
> There have been many problems in the past, and it is a bad idea to add
> more of this. We should be eliminating all cases.

What problems?  This reflects real change in drive's identify
and I think should be replaced by rereading drive->id from a drive.

> I mean:
>
> We have info from BIOS, user, disk etc and conclude to a certain geometry.

I can't spot place when we get info from a BIOS.
If we get it from a user, user should know what she/he is doing.

We can deal with HPA before our geometry info is set
(by moving code that is in idedisk_setup() right before call to
init_idedisk_capacity()).

> Sneakily changing what the disk reported is very ugly. I recall a case
> where a disk bounced between two capacities because the value that this
> computation concluded to was not a fixed point. Also, the user gets an
> incorrect report from HDIO_GET_IDENTITY.

User gets correct report from HDIO_GET_IDENTIFY as drive's identify was
really changed.  Moreover HDIO_GET_IDENTIFY needs fixing to actually
reread drive->id from a drive (similarly like /proc identify was fixed).

> So, the clean way is to examine what the disk reported, never change it

Even if disk's info changes?  I don't think so.
--
Bartlomiej