2.6.22-rc1-mm1: IDE compile error

2.6.22-rc1-mm1: IDE compile error

[Adrian Bunk]

On Tue, May 15, 2007 at 08:19:14PM -0700, Andrew Morton wrote:
>...
> - Added an i386 early-startup development tree, as git-newsetup.patch ("H. 
>   Peter Anvin" <hpa@zytor.com>)
>...
> Changes since 2.6.21-mm2:
>...
>  git-newsetup.patch
>...
>  git trees
>...

This breaks the compilation of the oldest of our IDE disk drivers:

<--  snip  -->

...
  LD      .tmp_vmlinux1
drivers/built-in.o: In function `hd_init':
hd.c:(.init.text+0x44a7d): undefined reference to `drive_info'
hd.c:(.init.text+0x44a89): undefined reference to `drive_info'
hd.c:(.init.text+0x44a95): undefined reference to `drive_info'
hd.c:(.init.text+0x44aa1): undefined reference to `drive_info'
hd.c:(.init.text+0x44aad): undefined reference to `drive_info'
drivers/built-in.o:hd.c:(.init.text+0x44ab9): more undefined references to `drive_info' follow
make[1]: *** [.tmp_vmlinux1] Error 1

<--  snip  -->

Considering the fact that we have two more recent drivers with the same 
functionality, it might be an option to simply remove this driver...

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: 2.6.22-rc1-mm1: IDE compile error

[Bartlomiej Zolnierkiewicz]

Hi,

On Wednesday 16 May 2007, Adrian Bunk wrote:
> On Tue, May 15, 2007 at 08:19:14PM -0700, Andrew Morton wrote:
> >...
> > - Added an i386 early-startup development tree, as git-newsetup.patch ("H. 
> >   Peter Anvin" <hpa@zytor.com>)
> >...
> > Changes since 2.6.21-mm2:
> >...
> >  git-newsetup.patch
> >...
> >  git trees
> >...
> 
> This breaks the compilation of the oldest of our IDE disk drivers:
> 
> <--  snip  -->
> 
> ...
>   LD      .tmp_vmlinux1
> drivers/built-in.o: In function `hd_init':
> hd.c:(.init.text+0x44a7d): undefined reference to `drive_info'
> hd.c:(.init.text+0x44a89): undefined reference to `drive_info'
> hd.c:(.init.text+0x44a95): undefined reference to `drive_info'
> hd.c:(.init.text+0x44aa1): undefined reference to `drive_info'
> hd.c:(.init.text+0x44aad): undefined reference to `drive_info'
> drivers/built-in.o:hd.c:(.init.text+0x44ab9): more undefined references to `drive_info' follow
> make[1]: *** [.tmp_vmlinux1] Error 1
> 
> <--  snip  -->
> 
> Considering the fact that we have two more recent drivers with the same 
> functionality, it might be an option to simply remove this driver...

Care to send a patch?

Thanks,
Bart

Re: 2.6.22-rc1-mm1: IDE compile error

[Alan Cox]

> > hd.c:(.init.text+0x44a7d): undefined reference to `drive_info'
> > hd.c:(.init.text+0x44a89): undefined reference to `drive_info'
> > hd.c:(.init.text+0x44a95): undefined reference to `drive_info'
> > hd.c:(.init.text+0x44aa1): undefined reference to `drive_info'
> > hd.c:(.init.text+0x44aad): undefined reference to `drive_info'
> > drivers/built-in.o:hd.c:(.init.text+0x44ab9): more undefined references to `drive_info' follow
> > make[1]: *** [.tmp_vmlinux1] Error 1
> > 
> > <--  snip  -->
> > 
> > Considering the fact that we have two more recent drivers with the same 
> > functionality, it might be an option to simply remove this driver...
> 
> Care to send a patch?

hd.c can drive MFM and RLL disks and drivers/ide cannot. Although it
really wants burying further down the config tree the ability to read MFM
and RLL disks when recovering ancient data is useful and people do
actually use this driver now and then rescuing stuff like twenty year old
datasets.

It thus needs fixing not removing.


Alan

Re: 2.6.22-rc1-mm1: IDE compile error

[H. Peter Anvin]

Alan Cox wrote:
>>> hd.c:(.init.text+0x44a7d): undefined reference to `drive_info'
>>> hd.c:(.init.text+0x44a89): undefined reference to `drive_info'
>>> hd.c:(.init.text+0x44a95): undefined reference to `drive_info'
>>> hd.c:(.init.text+0x44aa1): undefined reference to `drive_info'
>>> hd.c:(.init.text+0x44aad): undefined reference to `drive_info'
>>> drivers/built-in.o:hd.c:(.init.text+0x44ab9): more undefined references to `drive_info' follow
>>> make[1]: *** [.tmp_vmlinux1] Error 1
>>>
>>> <--  snip  -->
>>>
>>> Considering the fact that we have two more recent drivers with the same 
>>> functionality, it might be an option to simply remove this driver...
>> Care to send a patch?
> 
> hd.c can drive MFM and RLL disks and drivers/ide cannot. Although it
> really wants burying further down the config tree the ability to read MFM
> and RLL disks when recovering ancient data is useful and people do
> actually use this driver now and then rescuing stuff like twenty year old
> datasets.
> 
> It thus needs fixing not removing.

Why is this driver parked in drivers/ide/legacy when the companion
driver, xd.c, is in drivers/block (where hd.c used to be at one point,
too)?  Especially so since it's not really for IDE, but for ST-506.

HOWEVER, the code that fails above hard-assumes that the ST-506 disks
that you have are your primary system drives, which is obviously a wrong
assumption -- ST-506 drives were obsolete quite a while before Linux
existed[1].

xd.c, on the other hand, seems to actually go out and query the hardware
directly.  I guess this is understandably, since this controller would
never have been primary.

If hd.c is pure legacy, which it obviously is, should we remove the code
to assume the BIOS settings are the MFM/RLL settings (i.e. the __i386__
clause), and instead do something more like the __arm__ clause which
means that "if you really want to use this you have to specify the
parameters manually"?

	-hpa


[1] The 386-16 that I had access to at Northwestern, which with 0.59
BogoMIPS was the slowest Linux system in existence until Linux was
ported to other architectures, might have been an ST-506 drive, but I'm
not sure.

Re: 2.6.22-rc1-mm1: IDE compile error

[H. Peter Anvin]

Alan Cox wrote:
> 
> hd.c can drive MFM and RLL disks and drivers/ide cannot. Although it
> really wants burying further down the config tree the ability to read MFM
> and RLL disks when recovering ancient data is useful and people do
> actually use this driver now and then rescuing stuff like twenty year old
> datasets.
> 
> It thus needs fixing not removing.
> 

Opinions, anyone (especially Alan):

http://git.kernel.org/?p=linux/kernel/git/hpa/linux-2.6-newsetup.git;a=commitdiff;h=369f16fdd423d79640c4390915e6ab71189cb497

	-hpa

Re: 2.6.22-rc1-mm1: IDE compile error

[Alan Cox]

> > It thus needs fixing not removing.
> > 
> 
> Opinions, anyone (especially Alan):
> 
> http://git.kernel.org/?p=linux/kernel/git/hpa/linux-2.6-newsetup.git;a=commitdiff;h=369f16fdd423d79640c4390915e6ab71189cb497

I believe the technical description for the comment is "bullshit" 8)

Almost all MFM controllers and RLL controllers will only run at the
standard primary and secondary ATA address.

Given the intended use of the driver today I don't see a big problem in
requiring "hd=" although you have to question the point of this boot code
rewrite when it seems primarily to be removing features 

Alan

Re: 2.6.22-rc1-mm1: IDE compile error

[H. Peter Anvin]

Alan Cox wrote:
> I believe the technical description for the comment is "bullshit" 8)
> 
> Almost all MFM controllers and RLL controllers will only run at the
> standard primary and secondary ATA address.

Yes, but that doesn't (necessarily) apply to the controller that is
likely to be the primary controller in a modern system.

The whole point is that what the BIOS considers primary isn't
necessarily tied to the standard ATA addresses anymore, with SATA
controllers being primary.

The question I'm asking is: do you think it's better to remove this from
hd.c, or do you think it's better to add it back boot code BIOS
detection (and take the risk of poking an ST-506 disk with legacy data
with parameters which may belong to another disk -- keep in mind this
can permanently damage an ST-506 disk)?

> Given the intended use of the driver today I don't see a big problem in
> requiring "hd=" although you have to question the point of this boot code
> rewrite when it seems primarily to be removing features 

I've been trying to remove features that are obsolete and/or broken.  I
don't have access to this particular ancient hardware, nor any system
that can even host them.   It's very easy to add the stuff back in the
boot code; it's a much more tricky/annoying question if one *should* do
so.  That's part of a rewrite/cleanup.

	-hpa

Re: 2.6.22-rc1-mm1: IDE compile error

[Alan Cox]

> The question I'm asking is: do you think it's better to remove this from
> hd.c, or do you think it's better to add it back boot code BIOS
> detection (and take the risk of poking an ST-506 disk with legacy data
> with parameters which may belong to another disk -- keep in mind this
> can permanently damage an ST-506 disk)?

To set it up the user will have to know the parameters and have typed
them into the BIOS (if it even has an option for it). I see no problem

Re: 2.6.22-rc1-mm1: IDE compile error

[H. Peter Anvin]

Alan Cox wrote:
>> The question I'm asking is: do you think it's better to remove this from
>> hd.c, or do you think it's better to add it back boot code BIOS
>> detection (and take the risk of poking an ST-506 disk with legacy data
>> with parameters which may belong to another disk -- keep in mind this
>> can permanently damage an ST-506 disk)?
> 
> To set it up the user will have to know the parameters and have typed
> them into the BIOS (if it even has an option for it). I see no problem

Sorry, see no problem which way?  My concern here is with getting
incorrect data, not getting no data.  The BIOS probe amounts to pulling
data out of two tables (INT 0x41/0x46, corresponding to BIOS drives 0x80
and 0x81 -- the EDD 1.1 spec is quite specific that if implemented they
follow the BIOS drive numbers, not the ATA port addresses), and hoping
that they actually match the drives that hd.c uses.  That scares me,
since we're talking about old legacy data here.

I'm not concerned with what's easy, I'm concerned with what's the right
thing to do.

	-hpa

Re: 2.6.22-rc1-mm1: IDE compile error

[Alan Cox]

> > To set it up the user will have to know the parameters and have typed
> > them into the BIOS (if it even has an option for it). I see no problem
> 
> Sorry, see no problem which way?  

Forcing the user to provide the geometry. Historically that driver dealt
with the main disks the user had. Today its only use is specialist
recovery work. Anyone recovering a disk has to get the geometry data into
the BIOS (if the BIOS even allows it - many now don't) and will therefore
know it for hd= arguments as well

Alan