Re: [PATCH 5/5] ide: Force VIA IDE legacy interrupts for AmigaOne boards

Re: [PATCH 5/5] ide: Force VIA IDE legacy interrupts for AmigaOne boards

[Bartlomiej Zolnierkiewicz]

On Wednesday 07 January 2009, Gerhard Pircher wrote:
> 
> -------- Original-Nachricht --------
> > Datum: Wed, 7 Jan 2009 08:13:06 -0700
> > Von: "Grant Likely" <grant.likely@secretlab.ca>
> > An: "Gerhard Pircher" <gerhard_pircher@gmx.net>
> > CC: linuxppc-dev@ozlabs.org, bzolnier@gmail.com
> > Betreff: Re: [PATCH 5/5] ide: Force VIA IDE legacy interrupts for AmigaOne boards
> 
> > On Wed, Jan 7, 2009 at 7:12 AM, Gerhard Pircher <gerhard_pircher@gmx.net>
> > wrote:
> > > The AmigaOne uses the onboard VIA IDE controller in legacy mode (like
> > the
> > > Pegasos).
> > >
> > > Signed-off-by: Gerhard Pircher <gerhard_pircher@gmx.net>
> > > ---
> > >  drivers/ide/via82cxxx.c |    5 +++++
> > >  1 files changed, 5 insertions(+), 0 deletions(-)
> > 
> > This patch needs to also be posted on the linux-ide mailing list.
> Ouch, I only sent it to the maintainer. I'll fix that.

[ Please also keep all previous recipients on cc: when doing so. ]

> > > diff --git a/drivers/ide/via82cxxx.c b/drivers/ide/via82cxxx.c
> > > index 2a812d3..086f476 100644
> > > --- a/drivers/ide/via82cxxx.c
> > > +++ b/drivers/ide/via82cxxx.c
> > > @@ -450,6 +450,11 @@ static int __devinit via_init_one(struct pci_dev
> > *dev, const struct pci_device_i
> > >                d.host_flags |= IDE_HFLAG_FORCE_LEGACY_IRQS;
> > >  #endif
> > >
> > > +#ifdef CONFIG_AMIGAONE
> > > +       if (machine_is(amigaone))
> > > +               d.host_flags |= IDE_HFLAG_FORCE_LEGACY_IRQS;
> > > +#endif
> > > +
> > 
> > I know you're just following the example of the PEGASOS workaround
> > immediately above; but the #defines are really ugly.  I wonder if
> > there is there a cleaner way to manipulate the flags.
> AFAIK the via82cxxx driver doesn't make use of the pci_get_legacy_ide_irq
> approach.

I applied your patch for 2.6.29 but for 2.6.30 I would ask you to clean
up #ifdefs by using ide_pci_is_in_compatibility_mode() helper instead for
checking if IDE_HFLAG_FORCE_LEGACY_IRQS should be set.

[ Some time ago Pegasos got PCI quirk to put controller in the legacy mode
  (arch/powerpc/platforms/chrp/pci.c) so it is OK to also remove Pegasos'
  special case while at it. ]

Thanks,
Bart

Re: [PATCH 5/5] ide: Force VIA IDE legacy interrupts for AmigaOne boards

[Gerhard Pircher]

-------- Original-Nachricht --------
> Datum: Sun, 11 Jan 2009 17:51:55 +0100
> Von: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> An: "Gerhard Pircher" <gerhard_pircher@gmx.net>
> CC: "Grant Likely" <grant.likely@secretlab.ca>, linuxppc-dev@ozlabs.org, linux-ide@vger.kernel.org
> Betreff: Re: [PATCH 5/5] ide: Force VIA IDE legacy interrupts for AmigaOne boards

> On Wednesday 07 January 2009, Gerhard Pircher wrote:
> > 
> > -------- Original-Nachricht --------
> > > Datum: Wed, 7 Jan 2009 08:13:06 -0700
> > > Von: "Grant Likely" <grant.likely@secretlab.ca>
> > > An: "Gerhard Pircher" <gerhard_pircher@gmx.net>
> > > CC: linuxppc-dev@ozlabs.org, bzolnier@gmail.com
> > > Betreff: Re: [PATCH 5/5] ide: Force VIA IDE legacy interrupts for
> AmigaOne boards
> > 
> > > On Wed, Jan 7, 2009 at 7:12 AM, Gerhard Pircher
> <gerhard_pircher@gmx.net>
> > > wrote:
> > > > The AmigaOne uses the onboard VIA IDE controller in legacy mode
> > > >(like the Pegasos).
> > > >
> > > > Signed-off-by: Gerhard Pircher <gerhard_pircher@gmx.net>
> > > > ---
> > > >  drivers/ide/via82cxxx.c |    5 +++++
> > > >  1 files changed, 5 insertions(+), 0 deletions(-)
> > > 
> > > This patch needs to also be posted on the linux-ide mailing list.
> > Ouch, I only sent it to the maintainer. I'll fix that.
> 
> [ Please also keep all previous recipients on cc: when doing so. ]
Okay, I'll keep that in mind.

> > > > diff --git a/drivers/ide/via82cxxx.c b/drivers/ide/via82cxxx.c
> > > > index 2a812d3..086f476 100644
> > > > --- a/drivers/ide/via82cxxx.c
> > > > +++ b/drivers/ide/via82cxxx.c
> > > > @@ -450,6 +450,11 @@ static int __devinit via_init_one(struct
> pci_dev
> > > *dev, const struct pci_device_i
> > > >                d.host_flags |= IDE_HFLAG_FORCE_LEGACY_IRQS;
> > > >  #endif
> > > >
> > > > +#ifdef CONFIG_AMIGAONE
> > > > +       if (machine_is(amigaone))
> > > > +               d.host_flags |= IDE_HFLAG_FORCE_LEGACY_IRQS;
> > > > +#endif
> > > > +
> > > 
> > > I know you're just following the example of the PEGASOS workaround
> > > immediately above; but the #defines are really ugly.  I wonder if
> > > there is there a cleaner way to manipulate the flags.
> > AFAIK the via82cxxx driver doesn't make use of the
> > pci_get_legacy_ide_irq approach.
> 
> I applied your patch for 2.6.29 but for 2.6.30 I would ask you to clean
> up #ifdefs by using ide_pci_is_in_compatibility_mode() helper instead for
> checking if IDE_HFLAG_FORCE_LEGACY_IRQS should be set.
Wouldn't it be better, if I clean this up now? (I have to resend my AmigaOne
platform patches anyway).

> [ Some time ago Pegasos got PCI quirk to put controller in the legacy mode
>   (arch/powerpc/platforms/chrp/pci.c) so it is OK to also remove Pegasos'
>   special case while at it. ]
Okay, so the change shouldn't break IDE for Pegasos machines (I don't have
a Pegasos for testing).

Thanks!

Gerhard

-- 
Psssst! Schon vom neuen GMX MultiMessenger gehört? Der kann`s mit allen: http://www.gmx.net/de/go/multimessenger

Re: [PATCH 5/5] ide: Force VIA IDE legacy interrupts for AmigaOne boards

[Bartlomiej Zolnierkiewicz]

On Sunday 11 January 2009, Gerhard Pircher wrote:
> 
> -------- Original-Nachricht --------
> > Datum: Sun, 11 Jan 2009 17:51:55 +0100
> > Von: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> > An: "Gerhard Pircher" <gerhard_pircher@gmx.net>
> > CC: "Grant Likely" <grant.likely@secretlab.ca>, linuxppc-dev@ozlabs.org, linux-ide@vger.kernel.org
> > Betreff: Re: [PATCH 5/5] ide: Force VIA IDE legacy interrupts for AmigaOne boards
> 
> > On Wednesday 07 January 2009, Gerhard Pircher wrote:
> > > 
> > > -------- Original-Nachricht --------
> > > > Datum: Wed, 7 Jan 2009 08:13:06 -0700
> > > > Von: "Grant Likely" <grant.likely@secretlab.ca>
> > > > An: "Gerhard Pircher" <gerhard_pircher@gmx.net>
> > > > CC: linuxppc-dev@ozlabs.org, bzolnier@gmail.com
> > > > Betreff: Re: [PATCH 5/5] ide: Force VIA IDE legacy interrupts for
> > AmigaOne boards
> > > 
> > > > On Wed, Jan 7, 2009 at 7:12 AM, Gerhard Pircher
> > <gerhard_pircher@gmx.net>
> > > > wrote:
> > > > > The AmigaOne uses the onboard VIA IDE controller in legacy mode
> > > > >(like the Pegasos).
> > > > >
> > > > > Signed-off-by: Gerhard Pircher <gerhard_pircher@gmx.net>
> > > > > ---
> > > > >  drivers/ide/via82cxxx.c |    5 +++++
> > > > >  1 files changed, 5 insertions(+), 0 deletions(-)
> > > > 
> > > > This patch needs to also be posted on the linux-ide mailing list.
> > > Ouch, I only sent it to the maintainer. I'll fix that.
> > 
> > [ Please also keep all previous recipients on cc: when doing so. ]
> Okay, I'll keep that in mind.
> 
> > > > > diff --git a/drivers/ide/via82cxxx.c b/drivers/ide/via82cxxx.c
> > > > > index 2a812d3..086f476 100644
> > > > > --- a/drivers/ide/via82cxxx.c
> > > > > +++ b/drivers/ide/via82cxxx.c
> > > > > @@ -450,6 +450,11 @@ static int __devinit via_init_one(struct
> > pci_dev
> > > > *dev, const struct pci_device_i
> > > > >                d.host_flags |= IDE_HFLAG_FORCE_LEGACY_IRQS;
> > > > >  #endif
> > > > >
> > > > > +#ifdef CONFIG_AMIGAONE
> > > > > +       if (machine_is(amigaone))
> > > > > +               d.host_flags |= IDE_HFLAG_FORCE_LEGACY_IRQS;
> > > > > +#endif
> > > > > +
> > > > 
> > > > I know you're just following the example of the PEGASOS workaround
> > > > immediately above; but the #defines are really ugly.  I wonder if
> > > > there is there a cleaner way to manipulate the flags.
> > > AFAIK the via82cxxx driver doesn't make use of the
> > > pci_get_legacy_ide_irq approach.
> > 
> > I applied your patch for 2.6.29 but for 2.6.30 I would ask you to clean
> > up #ifdefs by using ide_pci_is_in_compatibility_mode() helper instead for
> > checking if IDE_HFLAG_FORCE_LEGACY_IRQS should be set.
> Wouldn't it be better, if I clean this up now? (I have to resend my AmigaOne
> platform patches anyway).

Replacement patch instead of incremental one is also fine with me -- given 
that it can wait for 2.6.30.

> > [ Some time ago Pegasos got PCI quirk to put controller in the legacy mode
> >   (arch/powerpc/platforms/chrp/pci.c) so it is OK to also remove Pegasos'
> >   special case while at it. ]
> Okay, so the change shouldn't break IDE for Pegasos machines (I don't have
> a Pegasos for testing).

Yes but there may be some other platforms (not necessarily powerpc ones)
that may be affected (i.e. they can depend indirectly on IRQ auto-probing
during IDE probe) so cleanup patch needs to spend some time in linux-next.

Thanks,
Bart

Re: [PATCH 5/5] ide: Force VIA IDE legacy interrupts for AmigaOne boards

[Gerhard Pircher]

-------- Original-Nachricht --------
> Datum: Mon, 12 Jan 2009 18:55:55 +0100
> Von: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> An: "Gerhard Pircher" <gerhard_pircher@gmx.net>
> CC: linux-ide@vger.kernel.org, linuxppc-dev@ozlabs.org, grant.likely@secretlab.ca
> Betreff: Re: [PATCH 5/5] ide: Force VIA IDE legacy interrupts for AmigaOne boards

> > > checking if IDE_HFLAG_FORCE_LEGACY_IRQS should be set.
> > Wouldn't it be better, if I clean this up now? (I have to resend my
> > AmigaOne platform patches anyway).
> 
> Replacement patch instead of incremental one is also fine with me -- given
> that it can wait for 2.6.30.
Yes, it can wait.
Although I would like to know from the powerpc maintainer, if my platform
patches could still go in 2.6.29, if I resend them in the next days? I guess it's
too late, right?

> > Okay, so the change shouldn't break IDE for Pegasos machines (I don't
> > have a Pegasos for testing).
> 
> Yes but there may be some other platforms (not necessarily powerpc ones)
> that may be affected (i.e. they can depend indirectly on IRQ auto-probing
> during IDE probe) so cleanup patch needs to spend some time in linux-next.
I think the VIA libata driver simply checks the progif register, too. I guess you
don't like the idea of a "#ifdef PPC32" or so around the
ide_pci_is_in_compatibility_mode() check?

Thanks!

Gerhard

-- 
Psssst! Schon vom neuen GMX MultiMessenger gehört? Der kann`s mit allen: http://www.gmx.net/de/go/multimessenger

Re: [PATCH 5/5] ide: Force VIA IDE legacy interrupts for AmigaOne boards

[Benjamin Herrenschmidt]

> Yes, it can wait.
> Although I would like to know from the powerpc maintainer, if my platform
> patches could still go in 2.6.29, if I resend them in the next days? I guess it's
> too late, right?

Yes it is. I'll put them in -next after -rc2 or later, when we are happy
with them. That gives us a bit of time to do extra polishing.

Ben.

Re: [PATCH 5/5] ide: Force VIA IDE legacy interrupts for AmigaOne boards

[Gerhard Pircher]

-------- Original-Nachricht --------
> Datum: Tue, 13 Jan 2009 16:02:38 +1100
> Von: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> An: Gerhard Pircher <gerhard_pircher@gmx.net>
> CC: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>, grant.likely@secretlab.ca, linuxppc-dev@ozlabs.org, linux-ide@vger.kernel.org
> Betreff: Re: [PATCH 5/5] ide: Force VIA IDE legacy interrupts for AmigaOne boards

> > Yes, it can wait.
> > Although I would like to know from the powerpc maintainer, if my
> > platform patches could still go in 2.6.29, if I resend them in the next days? I
> > guess it's too late, right?
> 
> Yes it is. I'll put them in -next after -rc2 or later, when we are happy
> with them. That gives us a bit of time to do extra polishing.
Good, then I'll send out a new patch for the IDE driver and the current one
can be reverted.

Gerhard

Re: [PATCH 5/5] ide: Force VIA IDE legacy interrupts for AmigaOne boards

[Bartlomiej Zolnierkiewicz]

On Tuesday 13 January 2009, Gerhard Pircher wrote:
> 
> -------- Original-Nachricht --------
> > Datum: Tue, 13 Jan 2009 16:02:38 +1100
> > Von: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > An: Gerhard Pircher <gerhard_pircher@gmx.net>
> > CC: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>, grant.likely@secretlab.ca, linuxppc-dev@ozlabs.org, linux-ide@vger.kernel.org
> > Betreff: Re: [PATCH 5/5] ide: Force VIA IDE legacy interrupts for AmigaOne boards
> 
> > > Yes, it can wait.
> > > Although I would like to know from the powerpc maintainer, if my
> > > platform patches could still go in 2.6.29, if I resend them in the next days? I
> > > guess it's too late, right?
> > 
> > Yes it is. I'll put them in -next after -rc2 or later, when we are happy
> > with them. That gives us a bit of time to do extra polishing.
> Good, then I'll send out a new patch for the IDE driver and the current one
> can be reverted.

The following patchset fixes core IDE PCI code to always use
pci_get_legacy_ide_irq() and ide_pci_is_in_compatibility_mode():

http://lkml.org/lkml/2009/1/19/163

so via82cxxx specific solution is no longer necessary.

[ IOW I'll keep your previous patch and the #ifdef issue will
  solve itself after the above patchset is merged. ]

Thanks,
Bart

Re: [PATCH 5/5] ide: Force VIA IDE legacy interrupts for AmigaOne boards

[Gerhard Pircher]

-------- Original-Nachricht --------
> Datum: Mon, 19 Jan 2009 19:28:35 +0100
> Von: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> An: "Gerhard Pircher" <gerhard_pircher@gmx.net>
> CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>, linux-ide@vger.kernel.org, linuxppc-dev@ozlabs.org, grant.likely@secretlab.ca
> Betreff: Re: [PATCH 5/5] ide: Force VIA IDE legacy interrupts for AmigaOne boards

> The following patchset fixes core IDE PCI code to always use
> pci_get_legacy_ide_irq() and ide_pci_is_in_compatibility_mode():
> 
> http://lkml.org/lkml/2009/1/19/163
> 
> so via82cxxx specific solution is no longer necessary.
> 
> [ IOW I'll keep your previous patch and the #ifdef issue will
>   solve itself after the above patchset is merged. ]
Thanks a lot! That's much better than the simple fix I had planned.

Gerhard

-- 
Psssst! Schon vom neuen GMX MultiMessenger gehört? Der kann`s mit allen: http://www.gmx.net/de/go/multimessenger