2008-06-25 09:10:35

by Russell King

[permalink] [raw]
Subject: Removal of BAST IDE driver

Referring to this commit in mainline:

commit ac1623625c5818bbdf5c68973098ba386ba7a004
Author: Ben Dooks <[email protected]>
Date: Fri Jun 20 20:53:35 2008 +0200

BAST: Remove old IDE driver

Remove the old BAST IDE driver, as we are now using the platform-pata
support.

Signed-off-by: Ben Dooks <[email protected]>
Cc: Jeff Garzik <[email protected]>
Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>

I've recieved a couple of patches for the next merge window from Ben
which say:

> Subject: [patch 20/32] BAST/VR1000: Move to using ata_platform (libata)
>
> Use the pata_platform driver to provide the IDE port
> drivers on the Simntec BAST and Thorcom VR1000 machines
> as a precursor to removing drivers/ide/arm/bast-ide.c

And when I queried Ben on this, he responded thusly:

09:58 < fluffy> yes, bart was rather quicker at applying the removal patch
09:59 < fluffy> i send 'for next kernel release' and he shoved it in his -rc6
sub

So, quite clearly we have a regression - we have platforms which have
lost IDE support.

There's two ways to resolve this. Either the above commit can be
reverted restoring old IDE support, or the patches to add libata
support for these platforms can be submitted. Given where we are in
the -rc, I think reverting the bad commit would be more sensible.

The question also has to be asked - what are maintainers doing putting
driver removals into -rc kernels? Surely they are only merge-window
candidates?

--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of:


Subject: Re: Removal of BAST IDE driver


Hi,

On Wednesday 25 June 2008, Russell King wrote:
> Referring to this commit in mainline:
>
> commit ac1623625c5818bbdf5c68973098ba386ba7a004
> Author: Ben Dooks <[email protected]>
> Date: Fri Jun 20 20:53:35 2008 +0200
>
> BAST: Remove old IDE driver
>
> Remove the old BAST IDE driver, as we are now using the platform-pata
> support.
>
> Signed-off-by: Ben Dooks <[email protected]>
> Cc: Jeff Garzik <[email protected]>
> Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
>
> I've recieved a couple of patches for the next merge window from Ben
> which say:
>
> > Subject: [patch 20/32] BAST/VR1000: Move to using ata_platform (libata)
> >
> > Use the pata_platform driver to provide the IDE port
> > drivers on the Simntec BAST and Thorcom VR1000 machines
> > as a precursor to removing drivers/ide/arm/bast-ide.c
>
> And when I queried Ben on this, he responded thusly:
>
> 09:58 < fluffy> yes, bart was rather quicker at applying the removal patch
> 09:59 < fluffy> i send 'for next kernel release' and he shoved it in his -rc6
> sub
>
> So, quite clearly we have a regression - we have platforms which have
> lost IDE support.
>
> There's two ways to resolve this. Either the above commit can be
> reverted restoring old IDE support, or the patches to add libata

The libata part is already in, we just need an arch part.

[ Looks like we need this patch from 2008-05-29:

http://article.gmane.org/gmane.linux.ports.arm.kernel/42021 ]

> support for these platforms can be submitted. Given where we are in
> the -rc, I think reverting the bad commit would be more sensible.

I think that it is up to arch maintainer to decide, either way is fine
with me.

> The question also has to be asked - what are maintainers doing putting
> driver removals into -rc kernels? Surely they are only merge-window
> candidates?

This was a special case as indicated in pull request:

...
[ not strictly -rc stuff but as we are now using the platform-pata
support lets not confuse people with having this driver around ]
...

I somehow missed 'the next release' part (please indicate it more clearly
when sending patches and always tell us that there are some other patches
which should be merged before if you are not including them in the patch
series) so when later Jeff merged:

commit cc18e0fea7907e7a96b7df71b81838d518bc074e
Author: Ben Dooks <[email protected]>
Date: Mon Jun 16 12:16:26 2008 +0100

LIBATA: Add HAVE_PATA_PLATFORM to select PATA_PLATFORM driver
...

which was patch #1/2 in the series I was under impression that now we
have both new and old driver with the old driver being the default because
of the link order. Seems I was wrong after all as other things are moving
slower than IDE/ATA. ;)

Sorry for messing things up. Russell/Ben: how should we proceed with
fixing it (arch part in or ide part out)?

Thanks,
Bart

2008-06-25 10:32:04

by Jeff Garzik

[permalink] [raw]
Subject: Re: Removal of BAST IDE driver

Bartlomiej Zolnierkiewicz wrote:
> series) so when later Jeff merged:
>
> commit cc18e0fea7907e7a96b7df71b81838d518bc074e
> Author: Ben Dooks <[email protected]>
> Date: Mon Jun 16 12:16:26 2008 +0100
>
> LIBATA: Add HAVE_PATA_PLATFORM to select PATA_PLATFORM driver
> ...
>
> which was patch #1/2 in the series I was under impression that now we
> have both new and old driver with the old driver being the default because
> of the link order. Seems I was wrong after all as other things are moving
> slower than IDE/ATA. ;)
>
> Sorry for messing things up. Russell/Ben: how should we proceed with
> fixing it (arch part in or ide part out)?


Ben's change merely adds a knob that a platform may set, in order to
enable the existing pata_platform driver, which is pretty safe by itself.

The next piece -- once not in 2.6.26 AFAIK -- is arch-specific code
selecting HAVE_PATA_PLATFORM. BAST IDE, I assume, can go away once
relevant platforms start turning on HAVE_PATA_PLATFORM.

Jeff

2008-06-25 11:34:17

by Russell King

[permalink] [raw]
Subject: Re: Removal of BAST IDE driver

On Wed, Jun 25, 2008 at 06:31:31AM -0400, Jeff Garzik wrote:
> Ben's change merely adds a knob that a platform may set, in order to
> enable the existing pata_platform driver, which is pretty safe by itself.
>
> The next piece -- once not in 2.6.26 AFAIK -- is arch-specific code
> selecting HAVE_PATA_PLATFORM. BAST IDE, I assume, can go away once
> relevant platforms start turning on HAVE_PATA_PLATFORM.

There's a bit more than just turning on HAVE_PATA_PLATFORM - there's
also the relevant arch code to register the PATA platform device and
provide the resources and platform data to properly configure the
PATA platform driver.

So there's a number of patches required:
- the addition of HAVE_PATA_PLATFORM (already merged by Jeff)
- "BAST/VR1000: Move to using ata_platform (libata)" to add the
arch specific bits
- "ANUBIS: Move to using ata_platform driver (libata)" for another
arch bit (which the following patch needs)
- "LIBATA: update Kconfig to allow new ata_platform dependencies" so
that the converted platforms select HAVE_PATA_PLATFORM

then, and only then, should drivers/ide/arm/bast-ide.c be removed -
that's the earliest point at which PATA becomes usable in mainline.

A note for Ben: I think the libata patch above should be rolled into
the other two patches so that bast can use the PATA code as soon as
the arch support code has been merged, rather than the current approach
where the libata patch enables the feature for all platforms.

--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: