linux-ide.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* How to cleanly setup legacy IDE irq ?
@ 2004-10-01  3:51 Benjamin Herrenschmidt
  2004-10-01 14:11 ` Bartlomiej Zolnierkiewicz
  0 siblings, 1 reply; 19+ messages in thread
From: Benjamin Herrenschmidt @ 2004-10-01  3:51 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz; +Cc: linux-ide

Hi Bart !

I noticed that you are about to absolete the whole lot of IDE
default stuff

# define ide_default_io_base(index)	(0)
# define ide_default_irq(base)		(0)
# define ide_init_default_irq(base)	(0)

and others...

So how do I do to "cleanly" tell the IDE layer that my PCI IDE
controller that shows on the PCI bus and is in legacy mode is
using some IRQs (14 & 15 typically) that are _not_ the PCI irq
line for this device ?

Ben.




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

* Re: How to cleanly setup legacy IDE irq ?
  2004-10-01  3:51 How to cleanly setup legacy IDE irq ? Benjamin Herrenschmidt
@ 2004-10-01 14:11 ` Bartlomiej Zolnierkiewicz
  2004-10-02  4:30   ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 19+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2004-10-01 14:11 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linux-ide

On Friday 01 October 2004 05:51, Benjamin Herrenschmidt wrote:
> Hi Bart !

Hi!
 
> I noticed that you are about to absolete the whole lot of IDE
> default stuff
> 
> # define ide_default_io_base(index)	(0)
> # define ide_default_irq(base)		(0)
> # define ide_init_default_irq(base)	(0)
> 
> and others...
> 
> So how do I do to "cleanly" tell the IDE layer that my PCI IDE
> controller that shows on the PCI bus and is in legacy mode is
> using some IRQs (14 & 15 typically) that are _not_ the PCI irq
> line for this device ?

->init_hwif() hook

i.e. piix.c is handling this with:

	if (!hwif->irq)
		hwif->irq = hwif->channel ? 15 : 14;

Bartlomiej

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

* Re: How to cleanly setup legacy IDE irq ?
  2004-10-01 14:11 ` Bartlomiej Zolnierkiewicz
@ 2004-10-02  4:30   ` Benjamin Herrenschmidt
  2004-10-02 14:56     ` Bartlomiej Zolnierkiewicz
  0 siblings, 1 reply; 19+ messages in thread
From: Benjamin Herrenschmidt @ 2004-10-02  4:30 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz; +Cc: linux-ide

On Sat, 2004-10-02 at 00:11, Bartlomiej Zolnierkiewicz wrote:
> On Friday 01 October 2004 05:51, Benjamin Herrenschmidt wrote:
> > Hi Bart !
> 
> Hi!
>  
> > I noticed that you are about to absolete the whole lot of IDE
> > default stuff
> > 
> > # define ide_default_io_base(index)	(0)
> > # define ide_default_irq(base)		(0)
> > # define ide_init_default_irq(base)	(0)
> > 
> > and others...
> > 
> > So how do I do to "cleanly" tell the IDE layer that my PCI IDE
> > controller that shows on the PCI bus and is in legacy mode is
> > using some IRQs (14 & 15 typically) that are _not_ the PCI irq
> > line for this device ?
> 
> ->init_hwif() hook
> 
> i.e. piix.c is handling this with:
> 
> 	if (!hwif->irq)
> 		hwif->irq = hwif->channel ? 15 : 14;
> 

But that is a totally ugly hack ! And requires putting platform specific
knowledge inside the drivers themselves which is even worse...

(In that case, irqs are 20 and 21 for example).

Ben.



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

* Re: How to cleanly setup legacy IDE irq ?
  2004-10-02  4:30   ` Benjamin Herrenschmidt
@ 2004-10-02 14:56     ` Bartlomiej Zolnierkiewicz
  2004-10-03  0:33       ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 19+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2004-10-02 14:56 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linux-ide

On Saturday 02 October 2004 06:30, Benjamin Herrenschmidt wrote:
> On Sat, 2004-10-02 at 00:11, Bartlomiej Zolnierkiewicz wrote:
> > On Friday 01 October 2004 05:51, Benjamin Herrenschmidt wrote:
> > > Hi Bart !
> > 
> > Hi!
> >  
> > > I noticed that you are about to absolete the whole lot of IDE
> > > default stuff
> > > 
> > > # define ide_default_io_base(index)	(0)
> > > # define ide_default_irq(base)		(0)
> > > # define ide_init_default_irq(base)	(0)
> > > 
> > > and others...
> > > 
> > > So how do I do to "cleanly" tell the IDE layer that my PCI IDE
> > > controller that shows on the PCI bus and is in legacy mode is
> > > using some IRQs (14 & 15 typically) that are _not_ the PCI irq
> > > line for this device ?
> > 
> > ->init_hwif() hook
> > 
> > i.e. piix.c is handling this with:
> > 
> > 	if (!hwif->irq)
> > 		hwif->irq = hwif->channel ? 15 : 14;
> > 
> 
> But that is a totally ugly hack ! And requires putting platform specific
> knowledge inside the drivers themselves which is even worse...
> 
> (In that case, irqs are 20 and 21 for example).

I think that if platform is using specific driver it is a
correct place to put such information but I may be wrong.

Well, at least it allows somebody knowing nothing about specific
platform (me) to see what IRQs are needed and why they are needed.
Following all IDE code paths using the default stuff and guessing
which parts are overridden by specific platform can get you insane.

I know that is not perfect and I'm open for better ideas.

Bartlomiej

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

* Re: How to cleanly setup legacy IDE irq ?
  2004-10-02 14:56     ` Bartlomiej Zolnierkiewicz
@ 2004-10-03  0:33       ` Benjamin Herrenschmidt
  2004-10-03 20:54         ` Bartlomiej Zolnierkiewicz
  0 siblings, 1 reply; 19+ messages in thread
From: Benjamin Herrenschmidt @ 2004-10-03  0:33 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz; +Cc: linux-ide

On Sun, 2004-10-03 at 00:56, Bartlomiej Zolnierkiewicz wrote:

> I think that if platform is using specific driver it is a
> correct place to put such information but I may be wrong.
> 
> Well, at least it allows somebody knowing nothing about specific
> platform (me) to see what IRQs are needed and why they are needed.
> Following all IDE code paths using the default stuff and guessing
> which parts are overridden by specific platform can get you insane.
> 
> I know that is not perfect and I'm open for better ideas.

I see your point, but I'd still rather have some generic way (since
the problem is actually common with controllers in legacy mode).

Something like

  if (hwif->irq == NO_IRQ) (*)
      hwif->irq = ide_get_legacy_irq(hwif);

Or maybe just de-obsolete one of the old "default" callbacks

(*) and please, don't compare with 0, 0 is a valid IRQ number on a
number of platforms !)




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

* Re: How to cleanly setup legacy IDE irq ?
  2004-10-03  0:33       ` Benjamin Herrenschmidt
@ 2004-10-03 20:54         ` Bartlomiej Zolnierkiewicz
  2004-10-03 23:42           ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 19+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2004-10-03 20:54 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linux-ide

On Sunday 03 October 2004 02:33, Benjamin Herrenschmidt wrote:
> On Sun, 2004-10-03 at 00:56, Bartlomiej Zolnierkiewicz wrote:
> 
> > I think that if platform is using specific driver it is a
> > correct place to put such information but I may be wrong.
> > 
> > Well, at least it allows somebody knowing nothing about specific
> > platform (me) to see what IRQs are needed and why they are needed.
> > Following all IDE code paths using the default stuff and guessing
> > which parts are overridden by specific platform can get you insane.
> > 
> > I know that is not perfect and I'm open for better ideas.
> 
> I see your point, but I'd still rather have some generic way (since
> the problem is actually common with controllers in legacy mode).
> 
> Something like
> 
>   if (hwif->irq == NO_IRQ) (*)
>       hwif->irq = ide_get_legacy_irq(hwif);

This is really bad from maintainability and sanity point of view ie.
you change something in IDE code and then suddenly have to grep
arch and platform specific code and worry about what these callbacks
wanted to achieve and watch out to not break them (or more likely,
don't know about this issue completely and break them silently :).
With placing IRQ setup in host drivers everything is clear _immediately_.

Also what happens when one architecture can use two different
drivers both needing different legacy IRQs?

Hm, why there are no such things in other (much more successful)
subsystems (SCSI, network) but some devices also use weird IRQs?

> Or maybe just de-obsolete one of the old "default" callbacks

grep kernel for occurrences of ide_default_irq() and then try to match
archs/platforms/IRQs to host drivers and code which setup IRQs vs code
which make use of it... really traumatic experience...

> (*) and please, don't compare with 0, 0 is a valid IRQ number on a
> number of platforms !)

ide-probe.c heavily depends on !hwif->irq but maybe somebody
will finally fix IDE to not use 0 to represent unknown IRQ.

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

* Re: How to cleanly setup legacy IDE irq ?
  2004-10-03 20:54         ` Bartlomiej Zolnierkiewicz
@ 2004-10-03 23:42           ` Benjamin Herrenschmidt
  2004-10-04  0:27             ` Bartlomiej Zolnierkiewicz
  0 siblings, 1 reply; 19+ messages in thread
From: Benjamin Herrenschmidt @ 2004-10-03 23:42 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz; +Cc: linux-ide

On Mon, 2004-10-04 at 06:54, Bartlomiej Zolnierkiewicz wrote:

> >   if (hwif->irq == NO_IRQ) (*)
> >       hwif->irq = ide_get_legacy_irq(hwif);
> 
> This is really bad from maintainability and sanity point of view ie.
> you change something in IDE code and then suddenly have to grep
> arch and platform specific code and worry about what these callbacks
> wanted to achieve and watch out to not break them (or more likely,
> don't know about this issue completely and break them silently :).
> With placing IRQ setup in host drivers everything is clear _immediately_.

Then you'll have a ton of

#ifdef CONFIG_****
	if (machine == blablabla)
		irq = something;
#endif
#if CONFIG_****
	irq = something else
#endif

and that sort of thing all over drivers... stinks as well.

> Also what happens when one architecture can use two different
> drivers both needing different legacy IRQs?

The port number will let the platform decide.

While I agree that the mess of callbacks we had so far was confusing at
best, I still think that a single callback for optionally letting the
arch provide the irq for controllers in legacy mode makes sense. We are
not talking about a mess of callbacks making the stuff unmaintainable,
but a single simple one. We could even add a printk when it's used to
make it easier to spot. "Legacy controller with arch provided irq %d\n"
or something like that.

> Hm, why there are no such things in other (much more successful)
> subsystems (SCSI, network) but some devices also use weird IRQs?

Most SCSI & network I've had to deal with for anything recent use _one_
irq per device, which makes things a lot easier. A lot of them are PCI,
which contains a single irq field. Other stuffs are probed in different
ways, but we tend to have an irq field around.

IDE is special in this regard, since even PCI and recent controllers end
up having this "legacy" mode that board vendors still sometimes strap
the chip in, which cause them to have IRQ on different pins than the
normal PCI interrupt of the chip.

> > Or maybe just de-obsolete one of the old "default" callbacks
> 
> grep kernel for occurrences of ide_default_irq() and then try to match
> archs/platforms/IRQs to host drivers and code which setup IRQs vs code
> which make use of it... really traumatic experience...

How so ? And why would you care anyway ? We are talking about a single
very clearly defined setup: a PCI controller with an existing driver
that is in legacy mode asking the arch for it's irqs.

> > (*) and please, don't compare with 0, 0 is a valid IRQ number on a
> > number of platforms !)
> 
> ide-probe.c heavily depends on !hwif->irq but maybe somebody
> will finally fix IDE to not use 0 to represent unknown IRQ.

It used to be fixed in ide-io.c at one point, but yes, ide-probe.c is broken.

Ben.



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

* Re: How to cleanly setup legacy IDE irq ?
  2004-10-03 23:42           ` Benjamin Herrenschmidt
@ 2004-10-04  0:27             ` Bartlomiej Zolnierkiewicz
  2004-10-04  1:05               ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 19+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2004-10-04  0:27 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linux-ide

On Monday 04 October 2004 01:42, Benjamin Herrenschmidt wrote:
> On Mon, 2004-10-04 at 06:54, Bartlomiej Zolnierkiewicz wrote:
> 
> > >   if (hwif->irq == NO_IRQ) (*)
> > >       hwif->irq = ide_get_legacy_irq(hwif);
> > 
> > This is really bad from maintainability and sanity point of view ie.
> > you change something in IDE code and then suddenly have to grep
> > arch and platform specific code and worry about what these callbacks
> > wanted to achieve and watch out to not break them (or more likely,
> > don't know about this issue completely and break them silently :).
> > With placing IRQ setup in host drivers everything is clear _immediately_.
> 
> Then you'll have a ton of
> 
> #ifdef CONFIG_****
> 	if (machine == blablabla)
> 		irq = something;
> #endif
> #if CONFIG_****
> 	irq = something else
> #endif
> 
> and that sort of thing all over drivers... stinks as well.

Actually it is fine with me.  I find it very informative. :)
Also host driver specific code is where is should belong.

> > Also what happens when one architecture can use two different
> > drivers both needing different legacy IRQs?
> 
> The port number will let the platform decide.

What do you mean by "the port number"? hwif->index?
In this situation both drivers have to be compiled in
(no modules because they change ordering) and probing 
order must be strictly ordered (ie. if non-PCI controllers
possible) or wrong IRQs will be set.

Do you suggest that we should stick to legacy ordering instead
of going in the direction of user-space solutions (udev)?

> While I agree that the mess of callbacks we had so far was confusing at
> best, I still think that a single callback for optionally letting the
> arch provide the irq for controllers in legacy mode makes sense. We are
> not talking about a mess of callbacks making the stuff unmaintainable,
> but a single simple one. We could even add a printk when it's used to
> make it easier to spot. "Legacy controller with arch provided irq %d\n"
> or something like that.
> 
> > Hm, why there are no such things in other (much more successful)
> > subsystems (SCSI, network) but some devices also use weird IRQs?
> 
> Most SCSI & network I've had to deal with for anything recent use _one_
> irq per device, which makes things a lot easier. A lot of them are PCI,
> which contains a single irq field. Other stuffs are probed in different
> ways, but we tend to have an irq field around.
> 
> IDE is special in this regard, since even PCI and recent controllers end
> up having this "legacy" mode that board vendors still sometimes strap
> the chip in, which cause them to have IRQ on different pins than the
> normal PCI interrupt of the chip.
> 
> > > Or maybe just de-obsolete one of the old "default" callbacks
> > 
> > grep kernel for occurrences of ide_default_irq() and then try to match
> > archs/platforms/IRQs to host drivers and code which setup IRQs vs code
> > which make use of it... really traumatic experience...
> 
> How so ? And why would you care anyway ? We are talking about a single

dynamic objects, sysfs (add your wishlist here)

ide_defualt_irq() is not the main problem here
(ide_init_default_hwifs() is) but I would prefer
not to unobsolete it.

> very clearly defined setup: a PCI controller with an existing driver
> that is in legacy mode asking the arch for it's irqs.

If this is going to be limited to this single case - PCI device
in legacy mode asking for IRQ it makes sense (as long as it
doesn't need to include <linux/ide.h>) otherwise rather not.

Actual code showing pros/cons of both solutions should make
it more clear which one is better (well, both may be wrong). 

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

* Re: How to cleanly setup legacy IDE irq ?
  2004-10-04  0:27             ` Bartlomiej Zolnierkiewicz
@ 2004-10-04  1:05               ` Benjamin Herrenschmidt
  2004-10-04  1:20                 ` Benjamin Herrenschmidt
  2004-10-04  1:27                 ` Jeff Garzik
  0 siblings, 2 replies; 19+ messages in thread
From: Benjamin Herrenschmidt @ 2004-10-04  1:05 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz; +Cc: linux-ide

On Mon, 2004-10-04 at 10:27, Bartlomiej Zolnierkiewicz wrote:

> > Then you'll have a ton of
> > 
> > #ifdef CONFIG_****
> > 	if (machine == blablabla)
> > 		irq = something;
> > #endif
> > #if CONFIG_****
> > 	irq = something else
> > #endif
> > 
> > and that sort of thing all over drivers... stinks as well.
> 
> Actually it is fine with me.  I find it very informative. :)
> Also host driver specific code is where is should belong.

Ugh ? damn, that's a textboot example of crappy code ! You are
putting interrupt routing platform knowledge in non-platform specific
drivers, that is disgusting !

> > > Also what happens when one architecture can use two different
> > > drivers both needing different legacy IRQs?
> > 
> > The port number will let the platform decide.
> 
> What do you mean by "the port number"? hwif->index?
> In this situation both drivers have to be compiled in
> (no modules because they change ordering) and probing 
> order must be strictly ordered (ie. if non-PCI controllers
> possible) or wrong IRQs will be set.
> 
> Do you suggest that we should stick to legacy ordering instead
> of going in the direction of user-space solutions (udev)?

Nooooo, not at all... We are in a case where a controller that
was detected (we know the port numer / mmio address, the hwif
is almost fully setup already, only the interrupt number is missing).

Eventually, call it

  ide_get_pci_legacy_irq(pdev, channel);

That would make it even clearer.... It's just an arch helper to ask
for the interrupt routing of a known PCI host that happens to be
in "legacy" mode (and thus relies on platform specific interrupt
routing and does not respect normal PCI stuff).

> dynamic objects, sysfs (add your wishlist here)
> 
> ide_defualt_irq() is not the main problem here
> (ide_init_default_hwifs() is) but I would prefer
> not to unobsolete it.

I agree. The old callback stuff for setting up the whole legacy
interfaces has to go. I'm not talking about that. I'm talking about the
specific case of a known PCI chip used in legacy mode looking for an
interrupt.

I agree that setting up "legacy" controllers or controllers at
various random ports requires a specific driver.

In this case, this is an existing driver that works on all platforms
that do PCI, _but_ which happen to have been set to legacy mode by the
HW strapping or the firmware (unfortunately, some can't be changed back)
and thus requires some platform specific IRQ routing informations (ok,
I'm getting redundant here :)

> > very clearly defined setup: a PCI controller with an existing driver
> > that is in legacy mode asking the arch for it's irqs.
> 
> If this is going to be limited to this single case - PCI device
> in legacy mode asking for IRQ it makes sense (as long as it
> doesn't need to include <linux/ide.h>) otherwise rather not.
> 
> Actual code showing pros/cons of both solutions should make
> it more clear which one is better (well, both may be wrong). 

Well, then what about my proposal above ?

  int ide_get_pci_legacy_irq(struct pci_dev *pdev, int channel);

(with eventually an #ifdef ARCH_HAS.....)

We could have it either called by the chipset drivers themselves, or by
the generic code when the controller is in legacy mode.




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

* Re: How to cleanly setup legacy IDE irq ?
  2004-10-04  1:05               ` Benjamin Herrenschmidt
@ 2004-10-04  1:20                 ` Benjamin Herrenschmidt
  2004-10-04 21:35                   ` Bartlomiej Zolnierkiewicz
  2004-10-04  1:27                 ` Jeff Garzik
  1 sibling, 1 reply; 19+ messages in thread
From: Benjamin Herrenschmidt @ 2004-10-04  1:20 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz; +Cc: linux-ide


> Ugh ? damn, that's a textboot example of crappy code ! You are
> putting interrupt routing platform knowledge in non-platform specific
> drivers, that is disgusting !

Please, don't take my harsh language to seriously, I missed adding
a couple of ;)

Just to summarize, I totally agree with you that the existing callbacks
are ugly and the way of setting up on-board controllers, especially non
PCI ones, should be to have proper controller drivers.

But as I think I made clear in previous mail, I'm just talking about
the specific issue of IRQ routing for PCI controllers set in legacy mode,
to avoid cluttering "generic" drivers with all sort of platform ifdef's
for something that has really nothing to do with the driver logic itself...  

Ben.



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

* Re: How to cleanly setup legacy IDE irq ?
  2004-10-04  1:27                 ` Jeff Garzik
@ 2004-10-04  1:26                   ` Benjamin Herrenschmidt
  2004-10-04  1:33                     ` Jeff Garzik
  0 siblings, 1 reply; 19+ messages in thread
From: Benjamin Herrenschmidt @ 2004-10-04  1:26 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: Bartlomiej Zolnierkiewicz, linux-ide

On Mon, 2004-10-04 at 11:27, Jeff Garzik wrote:

> > We could have it either called by the chipset drivers themselves, or by
> > the generic code when the controller is in legacy mode.
> 
> How about a more generic, yet more specific pci_init_hwif()?

What would it do except from providing the interrupt  ?

I really don't want platforms to abuse that in the way the older
callbacks existed, that is to fully initialize an hwif.

Also, my proposal beeing sort-of "agnostic" to the IDE layer (only
pdev and channel arguments), it could be useable by libata too :)

Ben.



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

* Re: How to cleanly setup legacy IDE irq ?
  2004-10-04  1:05               ` Benjamin Herrenschmidt
  2004-10-04  1:20                 ` Benjamin Herrenschmidt
@ 2004-10-04  1:27                 ` Jeff Garzik
  2004-10-04  1:26                   ` Benjamin Herrenschmidt
  1 sibling, 1 reply; 19+ messages in thread
From: Jeff Garzik @ 2004-10-04  1:27 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: Bartlomiej Zolnierkiewicz, linux-ide

Benjamin Herrenschmidt wrote:
> On Mon, 2004-10-04 at 10:27, Bartlomiej Zolnierkiewicz wrote:
> 
> 
>>>Then you'll have a ton of
>>>
>>>#ifdef CONFIG_****
>>>	if (machine == blablabla)
>>>		irq = something;
>>>#endif
>>>#if CONFIG_****
>>>	irq = something else
>>>#endif
>>>
>>>and that sort of thing all over drivers... stinks as well.
>>
>>Actually it is fine with me.  I find it very informative. :)
>>Also host driver specific code is where is should belong.
> 
> 
> Ugh ? damn, that's a textboot example of crappy code ! You are
> putting interrupt routing platform knowledge in non-platform specific
> drivers, that is disgusting !

Yeah, I agree...  unless the driver is wholly platform specific (in 
which case no #ifdefs).



> Well, then what about my proposal above ?
> 
>   int ide_get_pci_legacy_irq(struct pci_dev *pdev, int channel);
> 
> (with eventually an #ifdef ARCH_HAS.....)
> 
> We could have it either called by the chipset drivers themselves, or by
> the generic code when the controller is in legacy mode.

How about a more generic, yet more specific pci_init_hwif()?

	Jeff




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

* Re: How to cleanly setup legacy IDE irq ?
  2004-10-04  1:26                   ` Benjamin Herrenschmidt
@ 2004-10-04  1:33                     ` Jeff Garzik
  2004-10-04  1:33                       ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 19+ messages in thread
From: Jeff Garzik @ 2004-10-04  1:33 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: Bartlomiej Zolnierkiewicz, linux-ide

On Mon, Oct 04, 2004 at 11:26:53AM +1000, Benjamin Herrenschmidt wrote:
> Also, my proposal beeing sort-of "agnostic" to the IDE layer (only
> pdev and channel arguments), it could be useable by libata too :)

Will, given that perspective, I wouldn't mind a pci_get_ide_irq()
in the PCI layer...  platforms that do not provide can easily return an
error code unconditionally.

	Jeff




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

* Re: How to cleanly setup legacy IDE irq ?
  2004-10-04  1:33                     ` Jeff Garzik
@ 2004-10-04  1:33                       ` Benjamin Herrenschmidt
  2004-10-04 21:21                         ` Bartlomiej Zolnierkiewicz
  0 siblings, 1 reply; 19+ messages in thread
From: Benjamin Herrenschmidt @ 2004-10-04  1:33 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: Bartlomiej Zolnierkiewicz, linux-ide

On Mon, 2004-10-04 at 11:33, Jeff Garzik wrote:
> On Mon, Oct 04, 2004 at 11:26:53AM +1000, Benjamin Herrenschmidt wrote:
> > Also, my proposal beeing sort-of "agnostic" to the IDE layer (only
> > pdev and channel arguments), it could be useable by libata too :)
> 
> Will, given that perspective, I wouldn't mind a pci_get_ide_irq()
> in the PCI layer...  platforms that do not provide can easily return an
> error code unconditionally.

Yup. Good idea.

I'd still call it pci_get_ide_legacy_irq() though as it's really
specific to controllers that are left in "legacy" mode. Controllers
in "fully native" mode use the normal PCI irq routing.

An negative error code vs. a positive irq number looks good ? Or can
irq numbers be legally negative on some platforms ? I'd rather have
it return NO_IRQ in fact if the platform can't help ;) Actually, in
80% of the cases, those numbers will be 14 and 15, so it could even
have a default implementation returning those....

I'll do a patch proposal either later today or tomorrow.
 
Ben.



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

* Re: How to cleanly setup legacy IDE irq ?
  2004-10-04  1:33                       ` Benjamin Herrenschmidt
@ 2004-10-04 21:21                         ` Bartlomiej Zolnierkiewicz
  2004-10-04 23:26                           ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 19+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2004-10-04 21:21 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: Jeff Garzik, linux-ide

On Monday 04 October 2004 03:33, Benjamin Herrenschmidt wrote:
> On Mon, 2004-10-04 at 11:33, Jeff Garzik wrote:
> > On Mon, Oct 04, 2004 at 11:26:53AM +1000, Benjamin Herrenschmidt wrote:
> > > Also, my proposal beeing sort-of "agnostic" to the IDE layer (only
> > > pdev and channel arguments), it could be useable by libata too :)
> > 
> > Will, given that perspective, I wouldn't mind a pci_get_ide_irq()
> > in the PCI layer...  platforms that do not provide can easily return an
> > error code unconditionally.
> 
> Yup. Good idea.
> 
> I'd still call it pci_get_ide_legacy_irq() though as it's really
> specific to controllers that are left in "legacy" mode. Controllers
> in "fully native" mode use the normal PCI irq routing.

What about controllers in "half native" mode
(legacy IRQs but native addressing)?

> An negative error code vs. a positive irq number looks good ? Or can
> irq numbers be legally negative on some platforms ? I'd rather have
> it return NO_IRQ in fact if the platform can't help ;) Actually, in
> 80% of the cases, those numbers will be 14 and 15, so it could even
> have a default implementation returning those....

Your proposal sounds fine but I worry that instead of crappy #ifdefs
it will end up having crappy switch (dev->device) statements.

> I'll do a patch proposal either later today or tomorrow.

OK.

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

* Re: How to cleanly setup legacy IDE irq ?
  2004-10-04  1:20                 ` Benjamin Herrenschmidt
@ 2004-10-04 21:35                   ` Bartlomiej Zolnierkiewicz
  2004-10-04 23:27                     ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 19+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2004-10-04 21:35 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linux-ide

On Monday 04 October 2004 03:20, Benjamin Herrenschmidt wrote:
> 
> > Ugh ? damn, that's a textboot example of crappy code ! You are
> > putting interrupt routing platform knowledge in non-platform specific
> > drivers, that is disgusting !
> 
> Please, don't take my harsh language to seriously, I missed adding
> a couple of ;)
>
> Just to summarize, I totally agree with you that the existing callbacks
> are ugly and the way of setting up on-board controllers, especially non
> PCI ones, should be to have proper controller drivers.

OK
 
> But as I think I made clear in previous mail, I'm just talking about
> the specific issue of IRQ routing for PCI controllers set in legacy mode,
> to avoid cluttering "generic" drivers with all sort of platform ifdef's
> for something that has really nothing to do with the driver logic itself...  

Maybe indeed cluttering PCI layer or arch includes with
information needed only by IDE driver is better...

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

* Re: How to cleanly setup legacy IDE irq ?
  2004-10-04 21:21                         ` Bartlomiej Zolnierkiewicz
@ 2004-10-04 23:26                           ` Benjamin Herrenschmidt
  2004-10-04 23:56                             ` Bartlomiej Zolnierkiewicz
  0 siblings, 1 reply; 19+ messages in thread
From: Benjamin Herrenschmidt @ 2004-10-04 23:26 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz; +Cc: Jeff Garzik, linux-ide

On Tue, 2004-10-05 at 07:21, Bartlomiej Zolnierkiewicz wrote:

> What about controllers in "half native" mode
> (legacy IRQs but native addressing)?

As far as IRQ routing is concerned, this is legacy mode, the controller
driver should call that function. But this is not a "standard" state, so
it would have done by the host controller driver, while the normal
"legacy" mode is well defined by the progif bits 0x1 and 0x4 and thus
the function for retreiving the irqs might be called by the generic ide
pci code no ?

> Your proposal sounds fine but I worry that instead of crappy #ifdefs
> it will end up having crappy switch (dev->device) statements.

Whatever we will have will be entirely in the arch. It will usually end
up beeing something like

	if (dev == onboard_ide_dev)
		return channel ? <chan_1_irq> : <chan_2_irq>;
	return NO_IRQ;

Since there will be only one of these on board on 99% of the time.

Ben.



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

* Re: How to cleanly setup legacy IDE irq ?
  2004-10-04 21:35                   ` Bartlomiej Zolnierkiewicz
@ 2004-10-04 23:27                     ` Benjamin Herrenschmidt
  0 siblings, 0 replies; 19+ messages in thread
From: Benjamin Herrenschmidt @ 2004-10-04 23:27 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz; +Cc: linux-ide

On Tue, 2004-10-05 at 07:35, Bartlomiej Zolnierkiewicz wrote:

> Maybe indeed cluttering PCI layer or arch includes with
> information needed only by IDE driver is better...

In this case it is. This is a platform provided information that
is of use of drivers that are platform agnostic. It's always a good
thing to keep platform specific stuff in the platform code outside
of drivers that are generic if we can...

Ben.
 


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

* Re: How to cleanly setup legacy IDE irq ?
  2004-10-04 23:26                           ` Benjamin Herrenschmidt
@ 2004-10-04 23:56                             ` Bartlomiej Zolnierkiewicz
  0 siblings, 0 replies; 19+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2004-10-04 23:56 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: Jeff Garzik, linux-ide

On Tuesday 05 October 2004 01:26, Benjamin Herrenschmidt wrote:
> On Tue, 2004-10-05 at 07:21, Bartlomiej Zolnierkiewicz wrote:
> 
> > What about controllers in "half native" mode
> > (legacy IRQs but native addressing)?
> 
> As far as IRQ routing is concerned, this is legacy mode, the controller
> driver should call that function. But this is not a "standard" state, so
> it would have done by the host controller driver, while the normal
> "legacy" mode is well defined by the progif bits 0x1 and 0x4 and thus
> the function for retreiving the irqs might be called by the generic ide
> pci code no ?

Yep.

> > Your proposal sounds fine but I worry that instead of crappy #ifdefs
> > it will end up having crappy switch (dev->device) statements.
> 
> Whatever we will have will be entirely in the arch. It will usually end
> up beeing something like
> 
> 	if (dev == onboard_ide_dev)
> 		return channel ? <chan_1_irq> : <chan_2_irq>;
> 	return NO_IRQ;
> 
> Since there will be only one of these on board on 99% of the time.

Yep.

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

end of thread, other threads:[~2004-10-04 23:55 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-10-01  3:51 How to cleanly setup legacy IDE irq ? Benjamin Herrenschmidt
2004-10-01 14:11 ` Bartlomiej Zolnierkiewicz
2004-10-02  4:30   ` Benjamin Herrenschmidt
2004-10-02 14:56     ` Bartlomiej Zolnierkiewicz
2004-10-03  0:33       ` Benjamin Herrenschmidt
2004-10-03 20:54         ` Bartlomiej Zolnierkiewicz
2004-10-03 23:42           ` Benjamin Herrenschmidt
2004-10-04  0:27             ` Bartlomiej Zolnierkiewicz
2004-10-04  1:05               ` Benjamin Herrenschmidt
2004-10-04  1:20                 ` Benjamin Herrenschmidt
2004-10-04 21:35                   ` Bartlomiej Zolnierkiewicz
2004-10-04 23:27                     ` Benjamin Herrenschmidt
2004-10-04  1:27                 ` Jeff Garzik
2004-10-04  1:26                   ` Benjamin Herrenschmidt
2004-10-04  1:33                     ` Jeff Garzik
2004-10-04  1:33                       ` Benjamin Herrenschmidt
2004-10-04 21:21                         ` Bartlomiej Zolnierkiewicz
2004-10-04 23:26                           ` Benjamin Herrenschmidt
2004-10-04 23:56                             ` Bartlomiej Zolnierkiewicz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).