($INBOX_DIR/description missing)
 help / color / Atom feed
* Re: [PATCH RESEND v2 0/2] Convert Atari Falcon IDE driver to platform device
       [not found] <1569470064-3977-1-git-send-email-schmitzmic@gmail.com>
@ 2019-10-25 20:33 ` Jens Axboe
  2019-10-26 18:17   ` Geert Uytterhoeven
       [not found] ` <1569470064-3977-2-git-send-email-schmitzmic@gmail.com>
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 26+ messages in thread
From: Jens Axboe @ 2019-10-25 20:33 UTC (permalink / raw)
  To: Michael Schmitz, linux-m68k, linux-ide; +Cc: geert, b.zolnierkie

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.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH RESEND v2 0/2] Convert Atari Falcon IDE driver to platform device
  2019-10-25 20:33 ` [PATCH RESEND v2 0/2] Convert Atari Falcon IDE driver to platform device Jens Axboe
@ 2019-10-26 18:17   ` Geert Uytterhoeven
  2019-10-28  7:03     ` Michael Schmitz
  0 siblings, 1 reply; 26+ messages in thread
From: Geert Uytterhoeven @ 2019-10-26 18:17 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Michael Schmitz, Linux/m68k, linux-ide, Bartlomiej Zolnierkiewicz

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH RESEND v2 0/2] Convert Atari Falcon IDE driver to platform device
  2019-10-26 18:17   ` Geert Uytterhoeven
@ 2019-10-28  7:03     ` Michael Schmitz
  2019-11-04 11:04       ` Geert Uytterhoeven
  0 siblings, 1 reply; 26+ messages in thread
From: Michael Schmitz @ 2019-10-28  7:03 UTC (permalink / raw)
  To: Geert Uytterhoeven, Jens Axboe
  Cc: Linux/m68k, linux-ide, Bartlomiej Zolnierkiewicz

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
>

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH RESEND v2 1/2] m68k/atari: add platform device for Falcon IDE port
       [not found] ` <1569470064-3977-2-git-send-email-schmitzmic@gmail.com>
@ 2019-11-04 10:56   ` Geert Uytterhoeven
  2019-11-04 21:09     ` Michael Schmitz
  0 siblings, 1 reply; 26+ messages in thread
From: Geert Uytterhoeven @ 2019-11-04 10:56 UTC (permalink / raw)
  To: Michael Schmitz; +Cc: Linux/m68k, linux-ide, Bartlomiej Zolnierkiewicz

Hi Michael,

On Thu, Sep 26, 2019 at 5:54 AM Michael Schmitz <schmitzmic@gmail.com> wrote:
> Autoloading of Falcon IDE driver modules requires converting
> these drivers to platform drivers.
>
> Add platform device for Falcon IDE interface in Atari platform
> setup code in preparation for this.
>
> Signed-off-by: Michael Schmitz <schmitzmic@gmail.com>
>
> --

This should be a triple dash.

>
> Changes from RFC
>
> - fix region size (spotted by Szymon Bieganski <S.Bieganski@chello.nl>)
> - define IDE interface address in atari/config.c, create platform device
>   always (suggested by Geert Uytterhoeven <geert@linux-m68k.org>)
>
> Changes from v1
>
> - add error checking for Falcon IDE platform device register

Thanks for the update!

> --- a/arch/m68k/atari/config.c
> +++ b/arch/m68k/atari/config.c
> @@ -939,6 +959,13 @@ int __init atari_platform_init(void)
>                         atari_scsi_tt_rsrc, ARRAY_SIZE(atari_scsi_tt_rsrc));
>  #endif
>
> +       if (ATARIHW_PRESENT(IDE)) {
> +               pdev = platform_device_register_simple("atari-falcon-ide", -1,
> +                       atari_falconide_rsrc, ARRAY_SIZE(atari_falconide_rsrc));
> +               if (IS_ERR(pdev))
> +                       rv = PTR_ERR(pdev);
> +       }
> +
>         return rv;
>  }

This breaks both falconide and pata_falcon, as it marks the resource
busy:

    ide: Falcon IDE controller
    falconide: resources busy

and

    pata_falcon: Atari Falcon PATA controller
    pata_falcon: resources busy

For pata_falcon, that regression can easily be fixed by merging both patches.
For falconide, I think the sensible thing to do is just remove the driver.
But before that, the defconfigs should be updated to use pata_falcon
instead of falconide.

For the actual code changes:
Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH RESEND v2 2/2] drivers/ata: convert pata_falcon to arch platform device
       [not found] ` <1569470064-3977-3-git-send-email-schmitzmic@gmail.com>
@ 2019-11-04 10:58   ` Geert Uytterhoeven
  0 siblings, 0 replies; 26+ messages in thread
From: Geert Uytterhoeven @ 2019-11-04 10:58 UTC (permalink / raw)
  To: Michael Schmitz; +Cc: Linux/m68k, linux-ide, Bartlomiej Zolnierkiewicz

Hi Michael,

On Thu, Sep 26, 2019 at 5:54 AM Michael Schmitz <schmitzmic@gmail.com> wrote:
> The Atari platform device setup now provides a platform device
> for the Falcon IDE interface. Use this in place of the simple platform
> device set up in the old pata_falcon probe code.
>
> Signed-off-by: Michael Schmitz <schmitzmic@gmail.com>
>
> --

Should be a triple dash.

Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>
Minor nit below.

> --- a/drivers/ata/pata_falcon.c
> +++ b/drivers/ata/pata_falcon.c
> @@ -120,24 +119,22 @@ static int pata_falcon_set_mode(struct ata_link *link,
>         .set_mode       = pata_falcon_set_mode,
>  };
>
> -static int pata_falcon_init_one(void)
> +static int __init pata_falcon_init_one(struct platform_device *pdev)
>  {
> +       struct resource *res;
>         struct ata_host *host;
>         struct ata_port *ap;
> -       struct platform_device *pdev;
>         void __iomem *base;
>
> -       if (!MACH_IS_ATARI || !ATARIHW_PRESENT(IDE))
> -               return -ENODEV;
> -
> -       pr_info(DRV_NAME ": Atari Falcon PATA controller\n");
> +       dev_info(&pdev->dev, ": Atari Falcon PATA controller\n");

Please drop the ": ".

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH RESEND v2 0/2] Convert Atari Falcon IDE driver to platform device
  2019-10-28  7:03     ` Michael Schmitz
@ 2019-11-04 11:04       ` Geert Uytterhoeven
  2019-11-04 19:17         ` Michael Schmitz
  0 siblings, 1 reply; 26+ messages in thread
From: Geert Uytterhoeven @ 2019-11-04 11:04 UTC (permalink / raw)
  To: Michael Schmitz
  Cc: Jens Axboe, Linux/m68k, linux-ide, Bartlomiej Zolnierkiewicz

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

-- 
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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH RESEND v2 0/2] Convert Atari Falcon IDE driver to platform device
  2019-11-04 11:04       ` Geert Uytterhoeven
@ 2019-11-04 19:17         ` Michael Schmitz
  2019-11-04 20:06           ` Geert Uytterhoeven
  2019-11-04 21:10           ` John Paul Adrian Glaubitz
  0 siblings, 2 replies; 26+ messages in thread
From: Michael Schmitz @ 2019-11-04 19:17 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Jens Axboe, Linux/m68k, linux-ide, Bartlomiej Zolnierkiewicz

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
>

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH RESEND v2 0/2] Convert Atari Falcon IDE driver to platform device
  2019-11-04 19:17         ` Michael Schmitz
@ 2019-11-04 20:06           ` Geert Uytterhoeven
  2019-11-04 21:10           ` John Paul Adrian Glaubitz
  1 sibling, 0 replies; 26+ messages in thread
From: Geert Uytterhoeven @ 2019-11-04 20:06 UTC (permalink / raw)
  To: Michael Schmitz
  Cc: Jens Axboe, Linux/m68k, linux-ide, Bartlomiej Zolnierkiewicz

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

-- 
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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH RESEND v2 1/2] m68k/atari: add platform device for Falcon IDE port
  2019-11-04 10:56   ` [PATCH RESEND v2 1/2] m68k/atari: add platform device for Falcon IDE port Geert Uytterhoeven
@ 2019-11-04 21:09     ` Michael Schmitz
  2019-11-05  6:37       ` [PATCH] ide: falconide: convert to platform driver Michael Schmitz
  2019-11-05  6:43       ` [PATCH RESEND v2 1/2] m68k/atari: add platform device for Falcon IDE port Michael Schmitz
  0 siblings, 2 replies; 26+ messages in thread
From: Michael Schmitz @ 2019-11-04 21:09 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: Linux/m68k, linux-ide, Bartlomiej Zolnierkiewicz

Hi Geert,

thanks for your review!

On 4/11/19 11:56 PM, Geert Uytterhoeven wrote:
> Hi Michael,
>
> On Thu, Sep 26, 2019 at 5:54 AM Michael Schmitz <schmitzmic@gmail.com> wrote:
>> Autoloading of Falcon IDE driver modules requires converting
>> these drivers to platform drivers.
>>
>> Add platform device for Falcon IDE interface in Atari platform
>> setup code in preparation for this.
>>
>> Signed-off-by: Michael Schmitz <schmitzmic@gmail.com>
>>
>> --
> This should be a triple dash.
>
>> Changes from RFC
>>
>> - fix region size (spotted by Szymon Bieganski <S.Bieganski@chello.nl>)
>> - define IDE interface address in atari/config.c, create platform device
>>    always (suggested by Geert Uytterhoeven <geert@linux-m68k.org>)
>>
>> Changes from v1
>>
>> - add error checking for Falcon IDE platform device register
> Thanks for the update!
>
>> --- a/arch/m68k/atari/config.c
>> +++ b/arch/m68k/atari/config.c
>> @@ -939,6 +959,13 @@ int __init atari_platform_init(void)
>>                          atari_scsi_tt_rsrc, ARRAY_SIZE(atari_scsi_tt_rsrc));
>>   #endif
>>
>> +       if (ATARIHW_PRESENT(IDE)) {
>> +               pdev = platform_device_register_simple("atari-falcon-ide", -1,
>> +                       atari_falconide_rsrc, ARRAY_SIZE(atari_falconide_rsrc));
>> +               if (IS_ERR(pdev))
>> +                       rv = PTR_ERR(pdev);
>> +       }
>> +
>>          return rv;
>>   }
> This breaks both falconide and pata_falcon, as it marks the resource
> busy:
>
>      ide: Falcon IDE controller
>      falconide: resources busy
>
> and
>
>      pata_falcon: Atari Falcon PATA controller
>      pata_falcon: resources busy
>
> For pata_falcon, that regression can easily be fixed by merging both patches.

I obviously need to test this again, but from what I remember from my 
last testing, falconide still worked OK after applying both patches. 
That would have been without loading pata_falcon at all.

I'll rewrite falconide to use the same platform device as pata_falcon.

Cheers,

     Michael

> For falconide, I think the sensible thing to do is just remove the driver.
> But before that, the defconfigs should be updated to use pata_falcon
> instead of falconide.
>
> For the actual code changes:
> Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>
>
> Gr{oetje,eeting}s,
>
>                          Geert
>

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH RESEND v2 0/2] Convert Atari Falcon IDE driver to platform device
  2019-11-04 19:17         ` Michael Schmitz
  2019-11-04 20:06           ` Geert Uytterhoeven
@ 2019-11-04 21:10           ` John Paul Adrian Glaubitz
  2019-11-04 21:21             ` Michael Schmitz
  1 sibling, 1 reply; 26+ messages in thread
From: John Paul Adrian Glaubitz @ 2019-11-04 21:10 UTC (permalink / raw)
  To: Michael Schmitz, Geert Uytterhoeven
  Cc: Jens Axboe, Linux/m68k, linux-ide, Bartlomiej Zolnierkiewicz

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
: :' :  Debian Developer - glaubitz@debian.org
`. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH RESEND v2 0/2] Convert Atari Falcon IDE driver to platform device
  2019-11-04 21:10           ` John Paul Adrian Glaubitz
@ 2019-11-04 21:21             ` Michael Schmitz
  2019-11-04 21:42               ` John Paul Adrian Glaubitz
  0 siblings, 1 reply; 26+ messages in thread
From: Michael Schmitz @ 2019-11-04 21:21 UTC (permalink / raw)
  To: John Paul Adrian Glaubitz, Geert Uytterhoeven
  Cc: Linux/m68k, linux-ide, Bartlomiej Zolnierkiewicz

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
>

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH RESEND v2 0/2] Convert Atari Falcon IDE driver to platform device
  2019-11-04 21:21             ` Michael Schmitz
@ 2019-11-04 21:42               ` John Paul Adrian Glaubitz
  2019-11-05  6:57                 ` Michael Schmitz
  0 siblings, 1 reply; 26+ messages in thread
From: John Paul Adrian Glaubitz @ 2019-11-04 21:42 UTC (permalink / raw)
  To: Michael Schmitz, Geert Uytterhoeven
  Cc: Linux/m68k, linux-ide, Bartlomiej Zolnierkiewicz

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

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz@debian.org
`. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH] ide: falconide: convert to platform driver
  2019-11-04 21:09     ` Michael Schmitz
@ 2019-11-05  6:37       ` Michael Schmitz
  2019-11-05  8:11         ` Geert Uytterhoeven
  2019-11-05  6:43       ` [PATCH RESEND v2 1/2] m68k/atari: add platform device for Falcon IDE port Michael Schmitz
  1 sibling, 1 reply; 26+ messages in thread
From: Michael Schmitz @ 2019-11-05  6:37 UTC (permalink / raw)
  To: linux-m68k, linux-ide, b.zolnierkie
  Cc: schmitz, Michael Schmitz, Geert Uytterhoeven

With the introduction of a platform device for the Atari Falcon IDE
interface, the old Falcon IDE driver no longer loads (resource already
claimed by the platform device).

Convert falconide driver to use the same platform device that is used
by pata_falcon also.

Tested (as built-in driver) on my Atari Falcon.

Signed-off-by: Michael Schmitz <schmitzmic@gmail.com>
CC: Geert Uytterhoeven <geert@linux-m68k.org>
---
 drivers/ide/falconide.c |   45 ++++++++++++++++++++++++++++++++-------------
 1 files changed, 32 insertions(+), 13 deletions(-)

diff --git a/drivers/ide/falconide.c b/drivers/ide/falconide.c
index a5a07cc..d6dd772 100644
--- a/drivers/ide/falconide.c
+++ b/drivers/ide/falconide.c
@@ -15,6 +15,7 @@
 #include <linux/blkdev.h>
 #include <linux/ide.h>
 #include <linux/init.h>
+#include <linux/platform_device.h>
 
 #include <asm/setup.h>
 #include <asm/atarihw.h>
@@ -23,6 +24,7 @@
 #include <asm/ide.h>
 
 #define DRV_NAME "falconide"
+#define DRV_VERSION "0.1.0"
 
     /*
      *  Base of the IDE interface
@@ -114,18 +116,18 @@ static void falconide_output_data(ide_drive_t *drive, struct ide_cmd *cmd,
 	.chipset		= ide_generic,
 };
 
-static void __init falconide_setup_ports(struct ide_hw *hw)
+static void __init falconide_setup_ports(struct ide_hw *hw, unsigned long base)
 {
 	int i;
 
 	memset(hw, 0, sizeof(*hw));
 
-	hw->io_ports.data_addr = ATA_HD_BASE;
+	hw->io_ports.data_addr = base;
 
 	for (i = 1; i < 8; i++)
-		hw->io_ports_array[i] = ATA_HD_BASE + 1 + i * 4;
+		hw->io_ports_array[i] = base + 1 + i * 4;
 
-	hw->io_ports.ctl_addr = ATA_HD_BASE + ATA_HD_CONTROL;
+	hw->io_ports.ctl_addr = base + ATA_HD_CONTROL;
 
 	hw->irq = IRQ_MFP_IDE;
 }
@@ -134,23 +136,29 @@ static void __init falconide_setup_ports(struct ide_hw *hw)
      *  Probe for a Falcon IDE interface
      */
 
-static int __init falconide_init(void)
+static int __init falconide_init(struct platform_device *pdev)
 {
+	struct resource *res;
 	struct ide_host *host;
 	struct ide_hw hw, *hws[] = { &hw };
+	unsigned long base;
 	int rc;
 
-	if (!MACH_IS_ATARI || !ATARIHW_PRESENT(IDE))
-		return -ENODEV;
+	dev_info(&pdev->dev, "Atari Falcon IDE controller\n");
 
-	printk(KERN_INFO "ide: Falcon IDE controller\n");
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res)
+		return -ENODEV;
 
-	if (!request_mem_region(ATA_HD_BASE, 0x40, DRV_NAME)) {
-		printk(KERN_ERR "%s: resources busy\n", DRV_NAME);
+	if (!devm_request_mem_region(&pdev->dev, res->start,
+				     resource_size(res), DRV_NAME)) {
+		dev_err(&pdev->dev, "resources busy\n");
 		return -EBUSY;
 	}
 
-	falconide_setup_ports(&hw);
+	base = (unsigned long)res->start;
+
+	falconide_setup_ports(&hw, base);
 
 	host = ide_host_alloc(&falconide_port_info, hws, 1);
 	if (host == NULL) {
@@ -169,10 +177,21 @@ static int __init falconide_init(void)
 err_free:
 	ide_host_free(host);
 err:
-	release_mem_region(ATA_HD_BASE, 0x40);
+	release_mem_region(res->start, resource_size(res));
 	return rc;
 }
 
-module_init(falconide_init);
+static struct platform_driver ide_falcon_driver = {
+	.driver   = {
+		.name	= "atari-falcon-ide",
+	},
+};
+
+module_platform_driver_probe(ide_falcon_driver, falconide_init);
+
 
+MODULE_AUTHOR("Geert Uytterhoeven");
+MODULE_DESCRIPTION("low-level driver for Atari Falcon IDE");
 MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:atari-falcon-ide");
+MODULE_VERSION(DRV_VERSION);
-- 
1.7.0.4


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH RESEND v2 1/2] m68k/atari: add platform device for Falcon IDE port
  2019-11-04 21:09     ` Michael Schmitz
  2019-11-05  6:37       ` [PATCH] ide: falconide: convert to platform driver Michael Schmitz
@ 2019-11-05  6:43       ` Michael Schmitz
  1 sibling, 0 replies; 26+ messages in thread
From: Michael Schmitz @ 2019-11-05  6:43 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: Linux/m68k, linux-ide, Bartlomiej Zolnierkiewicz

Hi Geert,

Am 05.11.2019 um 10:09 schrieb Michael Schmitz:
>>> --- a/arch/m68k/atari/config.c
>>> +++ b/arch/m68k/atari/config.c
>>> @@ -939,6 +959,13 @@ int __init atari_platform_init(void)
>>>                          atari_scsi_tt_rsrc,
>>> ARRAY_SIZE(atari_scsi_tt_rsrc));
>>>   #endif
>>>
>>> +       if (ATARIHW_PRESENT(IDE)) {
>>> +               pdev =
>>> platform_device_register_simple("atari-falcon-ide", -1,
>>> +                       atari_falconide_rsrc,
>>> ARRAY_SIZE(atari_falconide_rsrc));
>>> +               if (IS_ERR(pdev))
>>> +                       rv = PTR_ERR(pdev);
>>> +       }
>>> +
>>>          return rv;
>>>   }
>> This breaks both falconide and pata_falcon, as it marks the resource
>> busy:
>>
>>      ide: Falcon IDE controller
>>      falconide: resources busy
>>
>> and
>>
>>      pata_falcon: Atari Falcon PATA controller
>>      pata_falcon: resources busy
>>
>> For pata_falcon, that regression can easily be fixed by merging both
>> patches.
>
> I obviously need to test this again, but from what I remember from my
> last testing, falconide still worked OK after applying both patches.

No idea what happened on that test, but booting a kernel with these 
patches applied on real hardware indeed does see the (builtin) driver 
fail to load.

> That would have been without loading pata_falcon at all.
>
> I'll rewrite falconide to use the same platform device as pata_falcon.

Just sent a patch for that in order to get some review - will combine 
with the two earlier patches once it's in acceptable shape.

Cheers,

	Michael

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH RESEND v2 0/2] Convert Atari Falcon IDE driver to platform device
  2019-11-04 21:42               ` John Paul Adrian Glaubitz
@ 2019-11-05  6:57                 ` Michael Schmitz
  2019-11-06  1:34                   ` Michael Schmitz
  0 siblings, 1 reply; 26+ messages in thread
From: Michael Schmitz @ 2019-11-05  6:57 UTC (permalink / raw)
  To: John Paul Adrian Glaubitz, Geert Uytterhoeven
  Cc: Linux/m68k, linux-ide, Bartlomiej Zolnierkiewicz

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
>

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] ide: falconide: convert to platform driver
  2019-11-05  6:37       ` [PATCH] ide: falconide: convert to platform driver Michael Schmitz
@ 2019-11-05  8:11         ` Geert Uytterhoeven
  2019-11-05 18:31           ` Michael Schmitz
  0 siblings, 1 reply; 26+ messages in thread
From: Geert Uytterhoeven @ 2019-11-05  8:11 UTC (permalink / raw)
  To: Michael Schmitz
  Cc: Linux/m68k, linux-ide, Bartlomiej Zolnierkiewicz, Michael Schmitz

Hi Michael,

On Tue, Nov 5, 2019 at 7:38 AM Michael Schmitz <schmitzmic@gmail.com> wrote:
> With the introduction of a platform device for the Atari Falcon IDE
> interface, the old Falcon IDE driver no longer loads (resource already
> claimed by the platform device).
>
> Convert falconide driver to use the same platform device that is used
> by pata_falcon also.
>
> Tested (as built-in driver) on my Atari Falcon.
>
> Signed-off-by: Michael Schmitz <schmitzmic@gmail.com>
> CC: Geert Uytterhoeven <geert@linux-m68k.org>

Thanks for your patch!

> --- a/drivers/ide/falconide.c
> +++ b/drivers/ide/falconide.c
> @@ -15,6 +15,7 @@
>  #include <linux/blkdev.h>
>  #include <linux/ide.h>
>  #include <linux/init.h>
> +#include <linux/platform_device.h>
>
>  #include <asm/setup.h>
>  #include <asm/atarihw.h>
> @@ -23,6 +24,7 @@
>  #include <asm/ide.h>
>
>  #define DRV_NAME "falconide"
> +#define DRV_VERSION "0.1.0"

Does anyone care about that version?
Will it ever be updated?

> @@ -169,10 +177,21 @@ static int __init falconide_init(void)
>  err_free:
>         ide_host_free(host);
>  err:
> -       release_mem_region(ATA_HD_BASE, 0x40);
> +       release_mem_region(res->start, resource_size(res));
>         return rc;
>  }
>
> -module_init(falconide_init);
> +static struct platform_driver ide_falcon_driver = {
> +       .driver   = {
> +               .name   = "atari-falcon-ide",
> +       },
> +};

Missing .remove() callback.

> +
> +module_platform_driver_probe(ide_falcon_driver, falconide_init);
> +
>
> +MODULE_AUTHOR("Geert Uytterhoeven");
> +MODULE_DESCRIPTION("low-level driver for Atari Falcon IDE");
>  MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:atari-falcon-ide");
> +MODULE_VERSION(DRV_VERSION);

I'd drop the MODULE_VERSION().

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] ide: falconide: convert to platform driver
  2019-11-05  8:11         ` Geert Uytterhoeven
@ 2019-11-05 18:31           ` Michael Schmitz
  2019-11-05 18:46             ` Geert Uytterhoeven
  0 siblings, 1 reply; 26+ messages in thread
From: Michael Schmitz @ 2019-11-05 18:31 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Linux/m68k, linux-ide, Bartlomiej Zolnierkiewicz, Michael Schmitz

Hi Geert,

thanks for the review!

On Tue, Nov 5, 2019 at 9:11 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Michael,
>
> On Tue, Nov 5, 2019 at 7:38 AM Michael Schmitz <schmitzmic@gmail.com> wrote:
> > With the introduction of a platform device for the Atari Falcon IDE
> > interface, the old Falcon IDE driver no longer loads (resource already
> > claimed by the platform device).
> >
> > Convert falconide driver to use the same platform device that is used
> > by pata_falcon also.
> >
> > Tested (as built-in driver) on my Atari Falcon.
> >
> > Signed-off-by: Michael Schmitz <schmitzmic@gmail.com>
> > CC: Geert Uytterhoeven <geert@linux-m68k.org>
>
> Thanks for your patch!
>
> > --- a/drivers/ide/falconide.c
> > +++ b/drivers/ide/falconide.c
> > @@ -15,6 +15,7 @@
> >  #include <linux/blkdev.h>
> >  #include <linux/ide.h>
> >  #include <linux/init.h>
> > +#include <linux/platform_device.h>
> >
> >  #include <asm/setup.h>
> >  #include <asm/atarihw.h>
> > @@ -23,6 +24,7 @@
> >  #include <asm/ide.h>
> >
> >  #define DRV_NAME "falconide"
> > +#define DRV_VERSION "0.1.0"
>
> Does anyone care about that version?

Not likely.

> Will it ever be updated?

You ask me? You're still listed as driver author!

I'll remove the version.

>
> > @@ -169,10 +177,21 @@ static int __init falconide_init(void)

Should I remove the __init here? Doesn't hurt in the built-in use
case, what about use as a module?

> >  err_free:
> >         ide_host_free(host);
> >  err:
> > -       release_mem_region(ATA_HD_BASE, 0x40);
> > +       release_mem_region(res->start, resource_size(res));
> >         return rc;
> >  }
> >
> > -module_init(falconide_init);
> > +static struct platform_driver ide_falcon_driver = {
> > +       .driver   = {
> > +               .name   = "atari-falcon-ide",
> > +       },
> > +};
>
> Missing .remove() callback.

Can't easily test driver remove, but I can certainly add a callback for that.

ide_unregister does the Right Thing (i.e. leaves the ST-DMA interrupt
registered) so no reason why it shouldn't work.

>
> > +
> > +module_platform_driver_probe(ide_falcon_driver, falconide_init);
> > +
> >
> > +MODULE_AUTHOR("Geert Uytterhoeven");
> > +MODULE_DESCRIPTION("low-level driver for Atari Falcon IDE");
> >  MODULE_LICENSE("GPL");
> > +MODULE_ALIAS("platform:atari-falcon-ide");
> > +MODULE_VERSION(DRV_VERSION);
>
> I'd drop the MODULE_VERSION().

Done.

Shall I merge this one with part one of the old series so there's no
chance of a bisection going wrong?

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] ide: falconide: convert to platform driver
  2019-11-05 18:31           ` Michael Schmitz
@ 2019-11-05 18:46             ` Geert Uytterhoeven
  2019-11-05 20:02               ` Michael Schmitz
  0 siblings, 1 reply; 26+ messages in thread
From: Geert Uytterhoeven @ 2019-11-05 18:46 UTC (permalink / raw)
  To: Michael Schmitz
  Cc: Linux/m68k, linux-ide, Bartlomiej Zolnierkiewicz, Michael Schmitz

Hi Michael,

On Tue, Nov 5, 2019 at 7:31 PM Michael Schmitz <schmitzmic@gmail.com> wrote:
> On Tue, Nov 5, 2019 at 9:11 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Tue, Nov 5, 2019 at 7:38 AM Michael Schmitz <schmitzmic@gmail.com> wrote:
> > > With the introduction of a platform device for the Atari Falcon IDE
> > > interface, the old Falcon IDE driver no longer loads (resource already
> > > claimed by the platform device).
> > >
> > > Convert falconide driver to use the same platform device that is used
> > > by pata_falcon also.
> > >
> > > Tested (as built-in driver) on my Atari Falcon.
> > >
> > > Signed-off-by: Michael Schmitz <schmitzmic@gmail.com>

> > > --- a/drivers/ide/falconide.c
> > > +++ b/drivers/ide/falconide.c

> > > @@ -23,6 +24,7 @@
> > >  #include <asm/ide.h>
> > >
> > >  #define DRV_NAME "falconide"
> > > +#define DRV_VERSION "0.1.0"
> >
> > Does anyone care about that version?
>
> Not likely.
>
> > Will it ever be updated?
>
> You ask me? You're still listed as driver author!

Yeah, I wrote a "it compiles, so it must work" driver in the IDE framework,
based on a driver in an even older framework. I never ran it on hardware ;-)

> > > @@ -169,10 +177,21 @@ static int __init falconide_init(void)
>
> Should I remove the __init here? Doesn't hurt in the built-in use
> case, what about use as a module?

Should be fine.

>
> > >  err_free:
> > >         ide_host_free(host);
> > >  err:
> > > -       release_mem_region(ATA_HD_BASE, 0x40);
> > > +       release_mem_region(res->start, resource_size(res));
> > >         return rc;
> > >  }
> > >
> > > -module_init(falconide_init);
> > > +static struct platform_driver ide_falcon_driver = {
> > > +       .driver   = {
> > > +               .name   = "atari-falcon-ide",
> > > +       },
> > > +};
> >
> > Missing .remove() callback.
>
> Can't easily test driver remove, but I can certainly add a callback for that.
>
> ide_unregister does the Right Thing (i.e. leaves the ST-DMA interrupt
> registered) so no reason why it shouldn't work.

gayle.c uses ide_host_remove().

> > > +
> > > +module_platform_driver_probe(ide_falcon_driver, falconide_init);
> > > +
> > >
> > > +MODULE_AUTHOR("Geert Uytterhoeven");
> > > +MODULE_DESCRIPTION("low-level driver for Atari Falcon IDE");
> > >  MODULE_LICENSE("GPL");
> > > +MODULE_ALIAS("platform:atari-falcon-ide");
> > > +MODULE_VERSION(DRV_VERSION);
> >
> > I'd drop the MODULE_VERSION().
>
> Done.
>
> Shall I merge this one with part one of the old series so there's no
> chance of a bisection going wrong?

Yes please.
Thanks!

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] ide: falconide: convert to platform driver
  2019-11-05 18:46             ` Geert Uytterhoeven
@ 2019-11-05 20:02               ` Michael Schmitz
  2019-11-05 21:13                 ` Michael Schmitz
  0 siblings, 1 reply; 26+ messages in thread
From: Michael Schmitz @ 2019-11-05 20:02 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Linux/m68k, linux-ide, Bartlomiej Zolnierkiewicz, Michael Schmitz

Hi Geert,

On Wed, Nov 6, 2019 at 7:46 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> > > > @@ -169,10 +177,21 @@ static int __init falconide_init(void)
> >
> > Should I remove the __init here? Doesn't hurt in the built-in use
> > case, what about use as a module?
>
> Should be fine.

OK, I'll leave it.

>
> >
> > > >  err_free:
> > > >         ide_host_free(host);
> > > >  err:
> > > > -       release_mem_region(ATA_HD_BASE, 0x40);
> > > > +       release_mem_region(res->start, resource_size(res));
> > > >         return rc;
> > > >  }
> > > >
> > > > -module_init(falconide_init);
> > > > +static struct platform_driver ide_falcon_driver = {
> > > > +       .driver   = {
> > > > +               .name   = "atari-falcon-ide",
> > > > +       },
> > > > +};
> > >
> > > Missing .remove() callback.
> >
> > Can't easily test driver remove, but I can certainly add a callback for that.
> >
> > ide_unregister does the Right Thing (i.e. leaves the ST-DMA interrupt
> > registered) so no reason why it shouldn't work.
>
> gayle.c uses ide_host_remove().

I'm using that, too. But  ide_host_remove() is not where free_irq() is called.

>
> > > > +
> > > > +module_platform_driver_probe(ide_falcon_driver, falconide_init);
> > > > +
> > > >
> > > > +MODULE_AUTHOR("Geert Uytterhoeven");
> > > > +MODULE_DESCRIPTION("low-level driver for Atari Falcon IDE");
> > > >  MODULE_LICENSE("GPL");
> > > > +MODULE_ALIAS("platform:atari-falcon-ide");
> > > > +MODULE_VERSION(DRV_VERSION);
> > >
> > > I'd drop the MODULE_VERSION().
> >
> > Done.
> >
> > Shall I merge this one with part one of the old series so there's no
> > chance of a bisection going wrong?
>
> Yes please.
> Thanks!

Thanks, I'll send a new version shortly.

Cheers,

    Michael

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] ide: falconide: convert to platform driver
  2019-11-05 20:02               ` Michael Schmitz
@ 2019-11-05 21:13                 ` Michael Schmitz
  2019-11-05 21:43                   ` Geert Uytterhoeven
  0 siblings, 1 reply; 26+ messages in thread
From: Michael Schmitz @ 2019-11-05 21:13 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Linux/m68k, linux-ide, Bartlomiej Zolnierkiewicz, Michael Schmitz

Hi Geert,


> On Wed, Nov 6, 2019 at 7:46 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > Shall I merge this one with part one of the old series so there's no
> > > chance of a bisection going wrong?
> >
> > Yes please.
> > Thanks!
>
> Thanks, I'll send a new version shortly.

Just confirming - the changes to pata_falcon.c will remain as a
separate patch which should be applied together with the patch that
will introduce the new platform device, and rewrite the legacy driver
to use it. That would require Bartlomiej and you to coordinate
closely.

If that's too onerous, I can merge the lot and you just ack the m68k
bits? Please let me know what you'd prefer.

Cheers,

    Michael

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] ide: falconide: convert to platform driver
  2019-11-05 21:13                 ` Michael Schmitz
@ 2019-11-05 21:43                   ` Geert Uytterhoeven
  2019-11-06  1:35                     ` Michael Schmitz
  0 siblings, 1 reply; 26+ messages in thread
From: Geert Uytterhoeven @ 2019-11-05 21:43 UTC (permalink / raw)
  To: Michael Schmitz
  Cc: Linux/m68k, linux-ide, Bartlomiej Zolnierkiewicz, Michael Schmitz

Hi Michael,

On Tue, Nov 5, 2019 at 10:13 PM Michael Schmitz <schmitzmic@gmail.com> wrote:
> > On Wed, Nov 6, 2019 at 7:46 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > Shall I merge this one with part one of the old series so there's no
> > > > chance of a bisection going wrong?
> > >
> > > Yes please.
> > > Thanks!
> >
> > Thanks, I'll send a new version shortly.
>
> Just confirming - the changes to pata_falcon.c will remain as a
> separate patch which should be applied together with the patch that
> will introduce the new platform device, and rewrite the legacy driver
> to use it. That would require Bartlomiej and you to coordinate
> closely.

Bartlomiej already acked both patches, so they can go in through the m68k
tree.

To avoid bisection regressions, both patches should be merged into a
single patch...

> If that's too onerous, I can merge the lot and you just ack the m68k
> bits? Please let me know what you'd prefer.

... and with the falconide.c conversion, all three patches should be merged
into a single patch.

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH RESEND v2 0/2] Convert Atari Falcon IDE driver to platform device
  2019-11-05  6:57                 ` Michael Schmitz
@ 2019-11-06  1:34                   ` Michael Schmitz
  0 siblings, 0 replies; 26+ messages in thread
From: Michael Schmitz @ 2019-11-06  1:34 UTC (permalink / raw)
  To: John Paul Adrian Glaubitz, Geert Uytterhoeven
  Cc: Linux/m68k, linux-ide, Bartlomiej Zolnierkiewicz

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



^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH] ide: falconide: convert to platform driver
  2019-11-05 21:43                   ` Geert Uytterhoeven
@ 2019-11-06  1:35                     ` Michael Schmitz
  0 siblings, 0 replies; 26+ messages in thread
From: Michael Schmitz @ 2019-11-06  1:35 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Linux/m68k, linux-ide, Bartlomiej Zolnierkiewicz, Michael Schmitz

Hi Geert,

thanks for confirming, I'll send a merged version shortly.

Cheers,

     Michael

On 6/11/19 10:43 AM, Geert Uytterhoeven wrote:
> Hi Michael,
>
> On Tue, Nov 5, 2019 at 10:13 PM Michael Schmitz <schmitzmic@gmail.com> wrote:
>>> On Wed, Nov 6, 2019 at 7:46 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>>>>> Shall I merge this one with part one of the old series so there's no
>>>>> chance of a bisection going wrong?
>>>> Yes please.
>>>> Thanks!
>>> Thanks, I'll send a new version shortly.
>> Just confirming - the changes to pata_falcon.c will remain as a
>> separate patch which should be applied together with the patch that
>> will introduce the new platform device, and rewrite the legacy driver
>> to use it. That would require Bartlomiej and you to coordinate
>> closely.
> Bartlomiej already acked both patches, so they can go in through the m68k
> tree.
>
> To avoid bisection regressions, both patches should be merged into a
> single patch...
>
>> If that's too onerous, I can merge the lot and you just ack the m68k
>> bits? Please let me know what you'd prefer.
> ... and with the falconide.c conversion, all three patches should be merged
> into a single patch.
>
> Gr{oetje,eeting}s,
>
>                          Geert
>

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v3] m68k/atari: convert legacy Falcon IDE driver to platform driver
       [not found] <1569470064-3977-1-git-send-email-schmitzmic@gmail.com>
                   ` (2 preceding siblings ...)
       [not found] ` <1569470064-3977-3-git-send-email-schmitzmic@gmail.com>
@ 2019-11-06  2:47 ` Michael Schmitz
  2019-11-18  9:20   ` Geert Uytterhoeven
  3 siblings, 1 reply; 26+ messages in thread
From: Michael Schmitz @ 2019-11-06  2:47 UTC (permalink / raw)
  To: linux-ide, linux-m68k
  Cc: b.zolnierkie, axboe, Michael Schmitz, Geert Uytterhoeven

Autoloading of Falcon IDE driver modules requires converting these
drivers to platform drivers.

Add platform device for Falcon IDE interface in Atari platform setup
code. Use this in the pata_falcon driver in place of the simple
platform device set up on the fly.

Convert falconide driver to use the same platform device that is used
by pata_falcon also. (With the introduction of a platform device for
the Atari Falcon IDE interface, the old Falcon IDE driver no longer
loads (resource already claimed by the platform device)).

Tested (as built-in driver) on my Atari Falcon.

Signed-off-by: Michael Schmitz <schmitzmic@gmail.com>
Acked-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
CC: Geert Uytterhoeven <geert@linux-m68k.org>

---

Changes from RFC

- fix region size (spotted by Szymon Bieganski <S.Bieganski@chello.nl>)
- define IDE interface address in atari/config.c, create platform device
  always (suggested by Geert Uytterhoeven <geert@linux-m68k.org>)

Changes from v1

- drop obsolete ATA_HD_BASE define
- add error checking for Falcon IDE platform device register
- use dev_err() to report error obtaining platform resource im
  pata_falcon driver

Changes from v2

- rewrite legacy falconide driver to use new platform device to avoid
  problem with IDE resource already marked busy (reported by Geert
  Uytterhoeven <geert@linux-m68k.org>)
---
 arch/m68k/atari/config.c  |   27 ++++++++++++++++++++
 drivers/ata/pata_falcon.c |   42 +++++++++++++++++++++----------
 drivers/ide/falconide.c   |   60 ++++++++++++++++++++++++++++++---------------
 3 files changed, 95 insertions(+), 34 deletions(-)

diff --git a/arch/m68k/atari/config.c b/arch/m68k/atari/config.c
index 73bf5ea..b932da1 100644
--- a/arch/m68k/atari/config.c
+++ b/arch/m68k/atari/config.c
@@ -869,8 +869,28 @@ static void isp1160_delay(struct device *dev, int delay)
 };
 #endif
 
+/*
+ * Falcon IDE interface
+ */
+
+#define FALCON_IDE_BASE	0xfff00000
+
+static const struct resource atari_falconide_rsrc[] __initconst = {
+	{
+		.flags = IORESOURCE_MEM,
+		.start = FALCON_IDE_BASE,
+		.end   = FALCON_IDE_BASE+0x39,
+	},
+	{
+		.flags = IORESOURCE_IRQ,
+		.start = IRQ_MFP_FSCSI,
+		.end   = IRQ_MFP_FSCSI,
+	},
+};
+
 int __init atari_platform_init(void)
 {
+	struct platform_device *pdev;
 	int rv = 0;
 
 	if (!MACH_IS_ATARI)
@@ -912,6 +932,13 @@ int __init atari_platform_init(void)
 			atari_scsi_tt_rsrc, ARRAY_SIZE(atari_scsi_tt_rsrc));
 #endif
 
+	if (ATARIHW_PRESENT(IDE)) {
+		pdev = platform_device_register_simple("atari-falcon-ide", -1,
+			atari_falconide_rsrc, ARRAY_SIZE(atari_falconide_rsrc));
+		if (IS_ERR(pdev))
+			rv = PTR_ERR(pdev);
+	}
+
 	return rv;
 }
 
diff --git a/drivers/ata/pata_falcon.c b/drivers/ata/pata_falcon.c
index 41e0d6a..27b0952 100644
--- a/drivers/ata/pata_falcon.c
+++ b/drivers/ata/pata_falcon.c
@@ -33,7 +33,6 @@
 #define DRV_NAME "pata_falcon"
 #define DRV_VERSION "0.1.0"
 
-#define ATA_HD_BASE	0xfff00000
 #define ATA_HD_CONTROL	0x39
 
 static struct scsi_host_template pata_falcon_sht = {
@@ -120,24 +119,22 @@ static int pata_falcon_set_mode(struct ata_link *link,
 	.set_mode	= pata_falcon_set_mode,
 };
 
-static int pata_falcon_init_one(void)
+static int __init pata_falcon_init_one(struct platform_device *pdev)
 {
+	struct resource *res;
 	struct ata_host *host;
 	struct ata_port *ap;
-	struct platform_device *pdev;
 	void __iomem *base;
 
-	if (!MACH_IS_ATARI || !ATARIHW_PRESENT(IDE))
-		return -ENODEV;
-
-	pr_info(DRV_NAME ": Atari Falcon PATA controller\n");
+	dev_info(&pdev->dev, "Atari Falcon PATA controller\n");
 
-	pdev = platform_device_register_simple(DRV_NAME, 0, NULL, 0);
-	if (IS_ERR(pdev))
-		return PTR_ERR(pdev);
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res)
+		return -ENODEV;
 
-	if (!devm_request_mem_region(&pdev->dev, ATA_HD_BASE, 0x40, DRV_NAME)) {
-		pr_err(DRV_NAME ": resources busy\n");
+	if (!devm_request_mem_region(&pdev->dev, res->start,
+				     resource_size(res), DRV_NAME)) {
+		dev_err(&pdev->dev, "resources busy\n");
 		return -EBUSY;
 	}
 
@@ -152,7 +149,7 @@ static int pata_falcon_init_one(void)
 	ap->flags |= ATA_FLAG_SLAVE_POSS | ATA_FLAG_NO_IORDY;
 	ap->flags |= ATA_FLAG_PIO_POLLING;
 
-	base = (void __iomem *)ATA_HD_BASE;
+	base = (void __iomem *)res->start;
 	ap->ioaddr.data_addr		= base;
 	ap->ioaddr.error_addr		= base + 1 + 1 * 4;
 	ap->ioaddr.feature_addr		= base + 1 + 1 * 4;
@@ -174,9 +171,26 @@ static int pata_falcon_init_one(void)
 	return ata_host_activate(host, 0, NULL, 0, &pata_falcon_sht);
 }
 
-module_init(pata_falcon_init_one);
+static int __exit pata_falcon_remove_one(struct platform_device *pdev)
+{
+	struct ata_host *host = platform_get_drvdata(pdev);
+
+	ata_host_detach(host);
+
+	return 0;
+}
+
+static struct platform_driver pata_falcon_driver = {
+	.remove = __exit_p(pata_falcon_remove_one),
+	.driver   = {
+		.name	= "atari-falcon-ide",
+	},
+};
+
+module_platform_driver_probe(pata_falcon_driver, pata_falcon_init_one);
 
 MODULE_AUTHOR("Bartlomiej Zolnierkiewicz");
 MODULE_DESCRIPTION("low-level driver for Atari Falcon PATA");
 MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:atari-falcon-ide");
 MODULE_VERSION(DRV_VERSION);
diff --git a/drivers/ide/falconide.c b/drivers/ide/falconide.c
index a5a07cc..dbeb260 100644
--- a/drivers/ide/falconide.c
+++ b/drivers/ide/falconide.c
@@ -15,6 +15,7 @@
 #include <linux/blkdev.h>
 #include <linux/ide.h>
 #include <linux/init.h>
+#include <linux/platform_device.h>
 
 #include <asm/setup.h>
 #include <asm/atarihw.h>
@@ -25,13 +26,7 @@
 #define DRV_NAME "falconide"
 
     /*
-     *  Base of the IDE interface
-     */
-
-#define ATA_HD_BASE	0xfff00000
-
-    /*
-     *  Offsets from the above base
+     *  Offsets from base address
      */
 
 #define ATA_HD_CONTROL	0x39
@@ -114,18 +109,18 @@ static void falconide_output_data(ide_drive_t *drive, struct ide_cmd *cmd,
 	.chipset		= ide_generic,
 };
 
-static void __init falconide_setup_ports(struct ide_hw *hw)
+static void __init falconide_setup_ports(struct ide_hw *hw, unsigned long base)
 {
 	int i;
 
 	memset(hw, 0, sizeof(*hw));
 
-	hw->io_ports.data_addr = ATA_HD_BASE;
+	hw->io_ports.data_addr = base;
 
 	for (i = 1; i < 8; i++)
-		hw->io_ports_array[i] = ATA_HD_BASE + 1 + i * 4;
+		hw->io_ports_array[i] = base + 1 + i * 4;
 
-	hw->io_ports.ctl_addr = ATA_HD_BASE + ATA_HD_CONTROL;
+	hw->io_ports.ctl_addr = base + ATA_HD_CONTROL;
 
 	hw->irq = IRQ_MFP_IDE;
 }
@@ -134,23 +129,29 @@ static void __init falconide_setup_ports(struct ide_hw *hw)
      *  Probe for a Falcon IDE interface
      */
 
-static int __init falconide_init(void)
+static int __init falconide_init(struct platform_device *pdev)
 {
+	struct resource *res;
 	struct ide_host *host;
 	struct ide_hw hw, *hws[] = { &hw };
+	unsigned long base;
 	int rc;
 
-	if (!MACH_IS_ATARI || !ATARIHW_PRESENT(IDE))
-		return -ENODEV;
+	dev_info(&pdev->dev, "Atari Falcon IDE controller\n");
 
-	printk(KERN_INFO "ide: Falcon IDE controller\n");
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res)
+		return -ENODEV;
 
-	if (!request_mem_region(ATA_HD_BASE, 0x40, DRV_NAME)) {
-		printk(KERN_ERR "%s: resources busy\n", DRV_NAME);
+	if (!devm_request_mem_region(&pdev->dev, res->start,
+				     resource_size(res), DRV_NAME)) {
+		dev_err(&pdev->dev, "resources busy\n");
 		return -EBUSY;
 	}
 
-	falconide_setup_ports(&hw);
+	base = (unsigned long)res->start;
+
+	falconide_setup_ports(&hw, base);
 
 	host = ide_host_alloc(&falconide_port_info, hws, 1);
 	if (host == NULL) {
@@ -169,10 +170,29 @@ static int __init falconide_init(void)
 err_free:
 	ide_host_free(host);
 err:
-	release_mem_region(ATA_HD_BASE, 0x40);
+	release_mem_region(res->start, resource_size(res));
 	return rc;
 }
 
-module_init(falconide_init);
+static int falconide_remove(struct platform_device *pdev)
+{
+	struct ide_host *host = dev_get_drvdata(&pdev->dev);
+
+	ide_host_remove(host);
+
+	return 0;
+}
+
+static struct platform_driver ide_falcon_driver = {
+	.remove = falconide_remove,
+	.driver   = {
+		.name	= "atari-falcon-ide",
+	},
+};
+
+module_platform_driver_probe(ide_falcon_driver, falconide_init);
 
+MODULE_AUTHOR("Geert Uytterhoeven");
+MODULE_DESCRIPTION("low-level driver for Atari Falcon IDE");
 MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:atari-falcon-ide");
-- 
1.7.0.4


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v3] m68k/atari: convert legacy Falcon IDE driver to platform driver
  2019-11-06  2:47 ` [PATCH v3] m68k/atari: convert legacy Falcon IDE driver to platform driver Michael Schmitz
@ 2019-11-18  9:20   ` Geert Uytterhoeven
  2019-11-18  9:41     ` John Paul Adrian Glaubitz
  0 siblings, 1 reply; 26+ messages in thread
From: Geert Uytterhoeven @ 2019-11-18  9:20 UTC (permalink / raw)
  To: Michael Schmitz
  Cc: linux-ide, Linux/m68k, Bartlomiej Zolnierkiewicz, Jens Axboe

On Wed, Nov 6, 2019 at 3:47 AM Michael Schmitz <schmitzmic@gmail.com> wrote:
> Autoloading of Falcon IDE driver modules requires converting these
> drivers to platform drivers.
>
> Add platform device for Falcon IDE interface in Atari platform setup
> code. Use this in the pata_falcon driver in place of the simple
> platform device set up on the fly.
>
> Convert falconide driver to use the same platform device that is used
> by pata_falcon also. (With the introduction of a platform device for
> the Atari Falcon IDE interface, the old Falcon IDE driver no longer
> loads (resource already claimed by the platform device)).
>
> Tested (as built-in driver) on my Atari Falcon.
>
> Signed-off-by: Michael Schmitz <schmitzmic@gmail.com>
> Acked-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> CC: Geert Uytterhoeven <geert@linux-m68k.org>

Thanks, applied as "m68k/atari: Convert Falcon IDE drivers to platform
drivers", and queued for v5.5.

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v3] m68k/atari: convert legacy Falcon IDE driver to platform driver
  2019-11-18  9:20   ` Geert Uytterhoeven
@ 2019-11-18  9:41     ` John Paul Adrian Glaubitz
  0 siblings, 0 replies; 26+ messages in thread
From: John Paul Adrian Glaubitz @ 2019-11-18  9:41 UTC (permalink / raw)
  To: Geert Uytterhoeven, Michael Schmitz
  Cc: linux-ide, Linux/m68k, Bartlomiej Zolnierkiewicz, Jens Axboe

On 11/18/19 10:20 AM, Geert Uytterhoeven wrote:
> Thanks, applied as "m68k/atari: Convert Falcon IDE drivers to platform
> drivers", and queued for v5.5.

Great. So this should finally resolve the issue that debian-installer won't
load the ATA drivers on Aranym automatically during installation.

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz@debian.org
`. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, back to index

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <1569470064-3977-1-git-send-email-schmitzmic@gmail.com>
2019-10-25 20:33 ` [PATCH RESEND v2 0/2] Convert Atari Falcon IDE driver to platform device Jens Axboe
2019-10-26 18:17   ` Geert Uytterhoeven
2019-10-28  7:03     ` Michael Schmitz
2019-11-04 11:04       ` Geert Uytterhoeven
2019-11-04 19:17         ` Michael Schmitz
2019-11-04 20:06           ` Geert Uytterhoeven
2019-11-04 21:10           ` John Paul Adrian Glaubitz
2019-11-04 21:21             ` Michael Schmitz
2019-11-04 21:42               ` John Paul Adrian Glaubitz
2019-11-05  6:57                 ` Michael Schmitz
2019-11-06  1:34                   ` Michael Schmitz
     [not found] ` <1569470064-3977-2-git-send-email-schmitzmic@gmail.com>
2019-11-04 10:56   ` [PATCH RESEND v2 1/2] m68k/atari: add platform device for Falcon IDE port Geert Uytterhoeven
2019-11-04 21:09     ` Michael Schmitz
2019-11-05  6:37       ` [PATCH] ide: falconide: convert to platform driver Michael Schmitz
2019-11-05  8:11         ` Geert Uytterhoeven
2019-11-05 18:31           ` Michael Schmitz
2019-11-05 18:46             ` Geert Uytterhoeven
2019-11-05 20:02               ` Michael Schmitz
2019-11-05 21:13                 ` Michael Schmitz
2019-11-05 21:43                   ` Geert Uytterhoeven
2019-11-06  1:35                     ` Michael Schmitz
2019-11-05  6:43       ` [PATCH RESEND v2 1/2] m68k/atari: add platform device for Falcon IDE port Michael Schmitz
     [not found] ` <1569470064-3977-3-git-send-email-schmitzmic@gmail.com>
2019-11-04 10:58   ` [PATCH RESEND v2 2/2] drivers/ata: convert pata_falcon to arch platform device Geert Uytterhoeven
2019-11-06  2:47 ` [PATCH v3] m68k/atari: convert legacy Falcon IDE driver to platform driver Michael Schmitz
2019-11-18  9:20   ` Geert Uytterhoeven
2019-11-18  9:41     ` John Paul Adrian Glaubitz

($INBOX_DIR/description missing)

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-m68k/0 linux-m68k/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-m68k linux-m68k/ https://lore.kernel.org/linux-m68k \
		linux-m68k@vger.kernel.org
	public-inbox-index linux-m68k

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-m68k


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git