mbox series

[RESEND,v2,0/2] Convert Atari Falcon IDE driver to platform device

Message ID 1569470064-3977-1-git-send-email-schmitzmic@gmail.com
Headers show
Series Convert Atari Falcon IDE driver to platform device | expand

Message

Michael Schmitz Sept. 26, 2019, 3:54 a.m. UTC
[Resend because linux-m68k was dropped from the recipient list ...]

As suggested by Geert, at least one of the drivers available for the Falcon
IDE interface should be converted to a platform device driver (to enable
module autoloading by the Debian installer).

Add platform device for Falcon IDE (patch 1), and rewrite the present
libata driver to make use of that device (patch 2).

Changes from v1: 

Incorporated review comments by Geert; corrected silly mismatch between
platform device name and platform driver name that caused loading driver
to fail locating the related resource; check return code of platform device
register call.

Tested on ARAnyM with only the pata_falcon driver builtin.

Cheers,

	Michael

Comments

Jens Axboe Oct. 25, 2019, 8:33 p.m. UTC | #1
On 9/25/19 9:54 PM, Michael Schmitz wrote:
> [Resend because linux-m68k was dropped from the recipient list ...]
> 
> As suggested by Geert, at least one of the drivers available for the Falcon
> IDE interface should be converted to a platform device driver (to enable
> module autoloading by the Debian installer).
> 
> Add platform device for Falcon IDE (patch 1), and rewrite the present
> libata driver to make use of that device (patch 2).
> 
> Changes from v1:
> 
> Incorporated review comments by Geert; corrected silly mismatch between
> platform device name and platform driver name that caused loading driver
> to fail locating the related resource; check return code of platform device
> register call.
> 
> Tested on ARAnyM with only the pata_falcon driver builtin.

Who's going to pick this one up? I can do it, but it'd be nice to have
m68k on patch 1 first.
Geert Uytterhoeven Oct. 26, 2019, 6:17 p.m. UTC | #2
On Fri, Oct 25, 2019 at 10:33 PM Jens Axboe <axboe@kernel.dk> wrote:
> On 9/25/19 9:54 PM, Michael Schmitz wrote:
> > [Resend because linux-m68k was dropped from the recipient list ...]
> >
> > As suggested by Geert, at least one of the drivers available for the Falcon
> > IDE interface should be converted to a platform device driver (to enable
> > module autoloading by the Debian installer).
> >
> > Add platform device for Falcon IDE (patch 1), and rewrite the present
> > libata driver to make use of that device (patch 2).
> >
> > Changes from v1:
> >
> > Incorporated review comments by Geert; corrected silly mismatch between
> > platform device name and platform driver name that caused loading driver
> > to fail locating the related resource; check return code of platform device
> > register call.
> >
> > Tested on ARAnyM with only the pata_falcon driver builtin.
>
> Who's going to pick this one up? I can do it, but it'd be nice to have
> m68k on patch 1 first.

Sorry for the late reply.  I'll have a closer look after ELC-E, and will apply
to the m68k tree if it passes.

BTW, I believe v1 of both patches has been acked by Bartlomiej?

Gr{oetje,eeting}s,

                        Geert


--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Michael Schmitz Oct. 28, 2019, 7:03 a.m. UTC | #3
Hi Geert,

Am 27.10.2019 um 07:17 schrieb Geert Uytterhoeven:
>>
>> Who's going to pick this one up? I can do it, but it'd be nice to have
>> m68k on patch 1 first.
>
> Sorry for the late reply.  I'll have a closer look after ELC-E, and will apply
> to the m68k tree if it passes.
>
> BTW, I believe v1 of both patches has been acked by Bartlomiej?

Correct - on July 3rd. I totally forgot about that, and didn't add his 
Acked-by in v2, sorry.

Message-IDs are <89366005-c1f1-4a0c-9917-720bb9e9e100@samsung.com> and
<69dd2d85-c9f2-23b9-b45c-34147e4dab86@samsung.com> FWIW.

Cheers,

	Michael

>
> Gr{oetje,eeting}s,
>
>                         Geert
>
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
>
Geert Uytterhoeven Nov. 4, 2019, 11:04 a.m. UTC | #4
Hi Michael,

On Mon, Oct 28, 2019 at 8:03 AM Michael Schmitz <schmitzmic@gmail.com> wrote:
> Am 27.10.2019 um 07:17 schrieb Geert Uytterhoeven:
> >>
> >> Who's going to pick this one up? I can do it, but it'd be nice to have
> >> m68k on patch 1 first.
> >
> > Sorry for the late reply.  I'll have a closer look after ELC-E, and will apply
> > to the m68k tree if it passes.
> >
> > BTW, I believe v1 of both patches has been acked by Bartlomiej?
>
> Correct - on July 3rd. I totally forgot about that, and didn't add his
> Acked-by in v2, sorry.

OK.

I was about to queue the combined patch, until I realized the defconfigs
default to falconide, which is broken by patch 1/2.
My proposed solution for that is:
  1. Switch the defconfigs from falconide to pata_falcon,
  2. Remove the legacy falconide driver.

Does that sound OK? Thanks!

Gr{oetje,eeting}s,

                        Geert
Michael Schmitz Nov. 4, 2019, 7:17 p.m. UTC | #5
Hi Geert,

On 5/11/19 12:04 AM, Geert Uytterhoeven wrote:
> Hi Michael,
>
> On Mon, Oct 28, 2019 at 8:03 AM Michael Schmitz <schmitzmic@gmail.com> wrote:
>> Am 27.10.2019 um 07:17 schrieb Geert Uytterhoeven:
>>>> Who's going to pick this one up? I can do it, but it'd be nice to have
>>>> m68k on patch 1 first.
>>> Sorry for the late reply.  I'll have a closer look after ELC-E, and will apply
>>> to the m68k tree if it passes.
>>>
>>> BTW, I believe v1 of both patches has been acked by Bartlomiej?
>> Correct - on July 3rd. I totally forgot about that, and didn't add his
>> Acked-by in v2, sorry.
> OK.
>
> I was about to queue the combined patch, until I realized the defconfigs
> default to falconide, which is broken by patch 1/2.
> My proposed solution for that is:
>    1. Switch the defconfigs from falconide to pata_falcon,

Ack.

>    2. Remove the legacy falconide driver.

Nack - I still use that one (because pata_falcon has no support for 
using interrupts with the Falcon IDE interface, and I'm unsure how much 
more kernel bloat libata will add). Need to check the impact of 
switching to pata_falcon first.

Cheers,

     Michael

>
> Does that sound OK? Thanks!
>
> Gr{oetje,eeting}s,
>
>                          Geert
>
Geert Uytterhoeven Nov. 4, 2019, 8:06 p.m. UTC | #6
Hi Michael,

On Mon, Nov 4, 2019 at 8:17 PM Michael Schmitz <schmitzmic@gmail.com> wrote:
> On 5/11/19 12:04 AM, Geert Uytterhoeven wrote:
> > On Mon, Oct 28, 2019 at 8:03 AM Michael Schmitz <schmitzmic@gmail.com> wrote:
> >> Am 27.10.2019 um 07:17 schrieb Geert Uytterhoeven:
> >>>> Who's going to pick this one up? I can do it, but it'd be nice to have
> >>>> m68k on patch 1 first.
> >>> Sorry for the late reply.  I'll have a closer look after ELC-E, and will apply
> >>> to the m68k tree if it passes.
> >>>
> >>> BTW, I believe v1 of both patches has been acked by Bartlomiej?
> >> Correct - on July 3rd. I totally forgot about that, and didn't add his
> >> Acked-by in v2, sorry.
> > OK.
> >
> > I was about to queue the combined patch, until I realized the defconfigs
> > default to falconide, which is broken by patch 1/2.
> > My proposed solution for that is:
> >    1. Switch the defconfigs from falconide to pata_falcon,
>
> Ack.
>
> >    2. Remove the legacy falconide driver.
>
> Nack - I still use that one (because pata_falcon has no support for
> using interrupts with the Falcon IDE interface, and I'm unsure how much
> more kernel bloat libata will add). Need to check the impact of
> switching to pata_falcon first.

Oh, I forgot about that.
So yes, in that case pata_falcon is not a viable alternative yet.
However, that means we can only avoid regressions by converting
falconide to the new platform device, too, and doing that together, atomically,
with your 2 patches in this series.

Gr{oetje,eeting}s,

                        Geert
John Paul Adrian Glaubitz Nov. 4, 2019, 9:10 p.m. UTC | #7
Hi!

On 11/4/19 8:17 PM, Michael Schmitz wrote:
>>    2. Remove the legacy falconide driver.
> 
> Nack - I still use that one (because pata_falcon has no support for using interrupts with the Falcon IDE interface, and I'm unsure how much more kernel bloat libata will add). Need to check the impact of switching to pata_falcon first.

I think we have to switch over to libata sooner or later as the old IDE
stack has officially been deprecated now and will probably be removed
at some point.

Adrian
Michael Schmitz Nov. 4, 2019, 9:21 p.m. UTC | #8
Hi Adrian,

fine - that'll be the time when I gladly hand over testing of 030 m68k 
stuff (at least on Atari) to someone else.

Any takers?

Cheers,

     Michael

On 5/11/19 10:10 AM, John Paul Adrian Glaubitz wrote:
> Hi!
>
> On 11/4/19 8:17 PM, Michael Schmitz wrote:
>>>     2. Remove the legacy falconide driver.
>> Nack - I still use that one (because pata_falcon has no support for using interrupts with the Falcon IDE interface, and I'm unsure how much more kernel bloat libata will add). Need to check the impact of switching to pata_falcon first.
> I think we have to switch over to libata sooner or later as the old IDE
> stack has officially been deprecated now and will probably be removed
> at some point.
>
> Adrian
>
John Paul Adrian Glaubitz Nov. 4, 2019, 9:42 p.m. UTC | #9
On 11/4/19 10:21 PM, Michael Schmitz wrote:
> fine - that'll be the time when I gladly hand over testing of 030 m68k stuff (at least on Atari) to someone else.
> 
> Any takers?

I'm not sure I understand the reasoning. Does the pata_falcon driver not
work on a real Atari?

Adrian
Michael Schmitz Nov. 5, 2019, 6:57 a.m. UTC | #10
Adrian,

Am 05.11.2019 um 10:42 schrieb John Paul Adrian Glaubitz:
> On 11/4/19 10:21 PM, Michael Schmitz wrote:
>> fine - that'll be the time when I gladly hand over testing of 030 m68k stuff (at least on Atari) to someone else.
>>
>> Any takers?
>
> I'm not sure I understand the reasoning. Does the pata_falcon driver not
> work on a real Atari?

I honestly don't know. I never tried that. With only 14 MB of RAM, 
keeping code size to the absolute minimum is quite important to me.

Aside from the lack of interrupt support for the Falcon IDE adapter in 
pata_falcon: I recall some criticism regarding the size of libata a few 
years back, combined with suggestion to allow libata to be built in a 
more modular fashion so features not requried to support what is 
essentially a dumb PIO mode IDE interface could be excluded. Not sure 
what became of that.

I'd have to test both the impact of missing IDE interrupt support, and 
that of code size when using either the old IDE code or libata at some 
stage. Having a stable SCSI driver for the Falcon is one of the 
prerequisites to that. We've just had a lot of fun with some of the m68k 
SCSI drivers breaking in the 5.x kernel series, so I'd rather leave IDE 
alone for now, or someone else step in and sort that particular mess out.

Cheers,

	Michael


>
> Adrian
>
Michael Schmitz Nov. 6, 2019, 1:34 a.m. UTC | #11
Hi Adrian,

On 5/11/19 7:57 PM, Michael Schmitz wrote:
>
>
> I'd have to test both the impact of missing IDE interrupt support, and 
> that of code size when using either the old 

Code size comparison (replacing legacy IDE by libata): Total: 
Before=2979865, After=3101692, chg +4.09%

120k increase is a little much for me to want to try with my current RAM 
size. I regularly get processes kicked off by the OOM reaper when doing 
a bit of IO loading as is.

Cheers,

     Michael