Hi Bart, Alan !
I've been fighting with a problem with both 2.4 and 2.5 for which I
would have a workaround, but I'd like to have your point of
view on it first as this code is rather messy.
So the problem is all about deciding if an hwif slot is 'owned' by
a driver, even after ide_unregister is called, after probe, etc...
and problems with the way the current scheme works (and differences
between the way PCI interfaces "pick" free slots vs. ide_register_hw
does)
So, let's first explain the various cases I have to deal with in the
context of ide/ppc/pmac.c.
1 - Normal case: interface is non hotswap, drive is present at startup,
things just work normally
2 - Empty interface: at boot, en empty interface (no drive) is found.
This happens a lot since Apple's ASIC often have at least one spare
ATA/33 interface that isn't wired to anything. This hwif is filled
up for ide/ppc/pmac (MMIO iops, chipset = ide_pmac, etc...).
hwif->present is cleared by the common probe code
3 - Hotswap interface: This one may or may not have devices plugged
at boot (laptop media bay). The hwif is filled with MMIO iops etc...
but since it may not have the CD drive plugged at boot, hwif->present
may be cleared by the common probe code.
So now, the problems:
- In both case 2 and 3, hwif->present may be cleared. That means
that something calling ide_register_hw() (like ide-cs for example)
will eventually pick up that hwif slot and try to use it. However
the hwif beeing filled for ide/ppc/pmac.c (MMIO iops among other)
it will usually not work and just crash, since ide_register_hw()
will _not_ reinit the fields. Also, in the case of the hotswap
interface (3), It's simply not acceptable for the slot to be re-used
(while it is for case 2) since you don't want the CD-ROM location
to change, and because ide-pmac will have internally setup a data
structure for this hwif that is tied to a given index that was
obtained at boot.
So that leads to 2 actual needs: One, to deal with case 2 & re-use
of the hwif by ide-cs (among others) is to have ide_register_hw
actually clear the interface calling init_hwif_data() at some
point when picking a "new" slot. The other one is to be able to
"mark" an interface as "held" by the driver (hotswap bay that
don't want to change numbering).
The simplest solution I have in mind is to add an hwif flag,
called "hold" (or whatever better name you find). Drivers like
ide/ppc/pmac.c would set this flag for the "hotswap" media bay
interface, and not for others.
The only change to the core code would then be for ide_register_hw
to 'skip' those when searching for an available slot, and to call
init_hwif_data when (!hwif->present && !hwif->hold) to handle case 2
where the iops & other hwif fields (mmio among others) need to be
reset to initial/legacy state.
That would allow "empty" fixed pmac interfaces to be released for
use by ide-cs, while the hotswap media bay one stay reserved.
That would give us a patch like that:
===== include/linux/ide.h 1.47 vs edited =====
--- 1.47/include/linux/ide.h Sun Apr 27 00:11:51 2003
+++ edited/include/linux/ide.h Sat May 3 11:30:34 2003
@@ -1022,6 +1022,7 @@
unsigned noprobe : 1; /* don't probe for this interface */
unsigned present : 1; /* this interface exists */
+ unsigned hold : 1; /* this interface is always present */
unsigned serialized : 1; /* serialized all channel operation */
unsigned sharing_irq: 1; /* 1 = sharing irq with another hwif */
unsigned reset : 1; /* reset after probe */
===== drivers/ide/ide.c 1.45 vs edited =====
--- 1.45/drivers/ide/ide.c Sun Apr 27 00:11:50 2003
+++ edited/drivers/ide/ide.c Sat May 3 11:32:35 2003
@@ -920,8 +920,8 @@
}
for (index = 0; index < MAX_HWIFS; ++index) {
hwif = &ide_hwifs[index];
- if ((!hwif->present && !hwif->mate && !initializing) ||
- (!hwif->hw.io_ports[IDE_DATA_OFFSET] && initializing))
+ if (!hwif->hold && ((!hwif->present && !hwif->mate && !initializing) ||
+ (!hwif->hw.io_ports[IDE_DATA_OFFSET] && initializing)))
goto found;
}
for (index = 0; index < MAX_HWIFS; index++)
@@ -931,6 +931,8 @@
found:
if (hwif->present)
ide_unregister(index);
+ else if (!hwif->hold)
+ init_hwif_data(index);
if (hwif->present)
return -1;
memcpy(&hwif->hw, hw, sizeof(*hw));
On 3 May 2003, Benjamin Herrenschmidt wrote:
> Hi Bart, Alan !
Hi!
> I've been fighting with a problem with both 2.4 and 2.5 for which I
> would have a workaround, but I'd like to have your point of
> view on it first as this code is rather messy.
>
> So the problem is all about deciding if an hwif slot is 'owned' by
> a driver, even after ide_unregister is called, after probe, etc...
> and problems with the way the current scheme works (and differences
> between the way PCI interfaces "pick" free slots vs. ide_register_hw
> does)
Yeah, this is tricky.
[slightly off-topic]
Yesterday I was thinking about:
- default arch hwifs, added/probed in ide_init_defult_hwifs() if no
IDE PCI/PCI (depends on arch, should be IDE PCI?) support is compiled in
- PCI hwifs
- legacy hwifs probed in ide_setup()
- legacy hwifs probed after PCI
and about ide_register_hw() + initializing flag.
What a mess... ordering issues can make you crazy.
> So, let's first explain the various cases I have to deal with in the
> context of ide/ppc/pmac.c.
>
> 1 - Normal case: interface is non hotswap, drive is present at startup,
> things just work normally
>
> 2 - Empty interface: at boot, en empty interface (no drive) is found.
> This happens a lot since Apple's ASIC often have at least one spare
> ATA/33 interface that isn't wired to anything. This hwif is filled
> up for ide/ppc/pmac (MMIO iops, chipset = ide_pmac, etc...).
> hwif->present is cleared by the common probe code
>
> 3 - Hotswap interface: This one may or may not have devices plugged
> at boot (laptop media bay). The hwif is filled with MMIO iops etc...
> but since it may not have the CD drive plugged at boot, hwif->present
> may be cleared by the common probe code.
>
> So now, the problems:
>
> - In both case 2 and 3, hwif->present may be cleared. That means
> that something calling ide_register_hw() (like ide-cs for example)
> will eventually pick up that hwif slot and try to use it. However
> the hwif beeing filled for ide/ppc/pmac.c (MMIO iops among other)
> it will usually not work and just crash, since ide_register_hw()
> will _not_ reinit the fields. Also, in the case of the hotswap
> interface (3), It's simply not acceptable for the slot to be re-used
> (while it is for case 2) since you don't want the CD-ROM location
> to change, and because ide-pmac will have internally setup a data
> structure for this hwif that is tied to a given index that was
> obtained at boot.
>
> So that leads to 2 actual needs: One, to deal with case 2 & re-use
> of the hwif by ide-cs (among others) is to have ide_register_hw
> actually clear the interface calling init_hwif_data() at some
> point when picking a "new" slot. The other one is to be able to
> "mark" an interface as "held" by the driver (hotswap bay that
> don't want to change numbering).
>
> The simplest solution I have in mind is to add an hwif flag,
> called "hold" (or whatever better name you find). Drivers like
> ide/ppc/pmac.c would set this flag for the "hotswap" media bay
> interface, and not for others.
This change is obviously correct and it doesn't have influence on
any existing code.
> The only change to the core code would then be for ide_register_hw
> to 'skip' those when searching for an available slot, and to call
> init_hwif_data when (!hwif->present && !hwif->hold) to handle case 2
> where the iops & other hwif fields (mmio among others) need to be
> reset to initial/legacy state.
Less safe change but also okay, as callers .
btw, Can't "ghost ides" be dealed inside ppc specific code?
Do you know when interface is valid and when it is "ghost",
and what other OS-es do in this case?
--
Bartlomiej
> That would allow "empty" fixed pmac interfaces to be released for
> use by ide-cs, while the hotswap media bay one stay reserved.
>
> That would give us a patch like that:
>
> ===== include/linux/ide.h 1.47 vs edited =====
> --- 1.47/include/linux/ide.h Sun Apr 27 00:11:51 2003
> +++ edited/include/linux/ide.h Sat May 3 11:30:34 2003
> @@ -1022,6 +1022,7 @@
>
> unsigned noprobe : 1; /* don't probe for this interface */
> unsigned present : 1; /* this interface exists */
> + unsigned hold : 1; /* this interface is always present */
> unsigned serialized : 1; /* serialized all channel operation */
> unsigned sharing_irq: 1; /* 1 = sharing irq with another hwif */
> unsigned reset : 1; /* reset after probe */
> ===== drivers/ide/ide.c 1.45 vs edited =====
> --- 1.45/drivers/ide/ide.c Sun Apr 27 00:11:50 2003
> +++ edited/drivers/ide/ide.c Sat May 3 11:32:35 2003
> @@ -920,8 +920,8 @@
> }
> for (index = 0; index < MAX_HWIFS; ++index) {
> hwif = &ide_hwifs[index];
> - if ((!hwif->present && !hwif->mate && !initializing) ||
> - (!hwif->hw.io_ports[IDE_DATA_OFFSET] && initializing))
> + if (!hwif->hold && ((!hwif->present && !hwif->mate && !initializing) ||
> + (!hwif->hw.io_ports[IDE_DATA_OFFSET] && initializing)))
> goto found;
> }
> for (index = 0; index < MAX_HWIFS; index++)
> @@ -931,6 +931,8 @@
> found:
> if (hwif->present)
> ide_unregister(index);
> + else if (!hwif->hold)
> + init_hwif_data(index);
> if (hwif->present)
> return -1;
> memcpy(&hwif->hw, hw, sizeof(*hw));
> Yesterday I was thinking about:
> - default arch hwifs, added/probed in ide_init_defult_hwifs() if no
> IDE PCI/PCI (depends on arch, should be IDE PCI?) support is compiled in
> - PCI hwifs
> - legacy hwifs probed in ide_setup()
> - legacy hwifs probed after PCI
> and about ide_register_hw() + initializing flag.
>
> What a mess... ordering issues can make you crazy.
Yup, I'd suggest we think about re-writing all that stuff for 2.7 :)
> > The simplest solution I have in mind is to add an hwif flag,
> > called "hold" (or whatever better name you find). Drivers like
> > ide/ppc/pmac.c would set this flag for the "hotswap" media bay
> > interface, and not for others.
>
> This change is obviously correct and it doesn't have influence on
> any existing code.
>
> > The only change to the core code would then be for ide_register_hw
> > to 'skip' those when searching for an available slot, and to call
> > init_hwif_data when (!hwif->present && !hwif->hold) to handle case 2
> > where the iops & other hwif fields (mmio among others) need to be
> > reset to initial/legacy state.
>
> Less safe change but also okay, as callers .
> btw, Can't "ghost ides" be dealed inside ppc specific code?
> Do you know when interface is valid and when it is "ghost",
> and what other OS-es do in this case?
Not re-using the slot for an interface with the "hold" bit won't
affect anybody but setters of that bit, so we are ok. The act of
calling init_hwif_data when !present && !hold is the one bringing
a possible change of behaviour to existing code.
However, who calls ide_register_hw() dynamically ? ide-cs and ?
I don't think there would much harm in re-calling init_hwif_data
at this point since hwif->present is not set, we _are_ re-using
the hwif, wether it was previously initialized or not by somebody
else. In this case, we really want to "clean" it, don't we ?
Right now, the only problem with re-initializing this way that
I've found is with hotswap interfaces like ide-pmac, because
they will have preset special MMIO ops etc... and that call
would revert that pre-setting. That's exactly why such interfaces
should set the "hold" bit to "reserve" the hwif slot. You see
the point ? I don't think there are much drivers aroung playing
with such tricks though.
All I can do within ide pmac itself is set or not that "hold"
bit for those interfaces. I just need to set it on the hotswap
ones (wthr they have devices connected or not) that way they
stay around "reserved" for when a device gets plugged. Other
"fixed" interfaces will have hwif->present cleared automatically
by the probe code, and thus will be "freed" for other uses, if
they don't have any device attached.
(Which is why that init_hwif_data is needed to reset their hwif
to something good default, and not whatever ide pmac have set).
In 2.5, I can be slightly smarted since I'm calling the probe
myself and no longer rely on the automatic initial probe done
by the IDE layer, like for PCI devices, so I can actually
"clear" those "empty" interfaces myself after they are probed.
But still, it makes sense to have this "hold" flag to let a
hotswap interface reserve a slot, and it makes sense when the
interface isn't held by anybody to "clean it up" before giving
it to somebody else.
The only problem I see right now is for a dynamic interface
(like ide-cs) where the _controller_ itself is hotswap, so
the hwif slot cannot be reserved in advance _and_ that interface
needs special IOps (which is fortunately not the case of ide-cs)
Such an interface can't really know what slot will be
picked by ide_register_hw() and can't "prepare" the HWIF with
special iops, so it won't be much harmed by the fact we are
calling init_hwif_data, but still, we should ultimately think
about splitting completely the fact of allocating an hwif slot,
setting it up, and triggering a probe on it. Those are 3 different
things that are currently mixed in bad ways. I don't beleive
fixing that fits in the 2.6 timeframe though.
Ben.
> Such an interface can't really know what slot will be
> picked by ide_register_hw() and can't "prepare" the HWIF with
> special iops, so it won't be much harmed by the fact we are
> calling init_hwif_data, but still, we should ultimately think
> about splitting completely the fact of allocating an hwif slot,
> setting it up, and triggering a probe on it. Those are 3 different
> things that are currently mixed in bad ways. I don't beleive
> fixing that fits in the 2.6 timeframe though.
I just though about another possible crap, though I haven't looked
enough to be sure, but PCI interfaces with no device will trigger
a similar problem as "empty" ide-pmac interfaces in that sense that
they will change dma ops, possibly mmio ops, etc.... If they hold
no device, their hwif->present will not be set, and thus the hwif
slot can possibly get re-used by thing like ide-cs (or anybody else
that rely on ide_register_hw() to allocate a new slot) without
those changes to hwif done by the PCI interface beeing cleared.
So my patch may actually fix some cases there too.
Ben.
On 3 May 2003, Benjamin Herrenschmidt wrote:
>
> > Yesterday I was thinking about:
> > - default arch hwifs, added/probed in ide_init_defult_hwifs() if no
> > IDE PCI/PCI (depends on arch, should be IDE PCI?) support is compiled in
> > - PCI hwifs
> > - legacy hwifs probed in ide_setup()
> > - legacy hwifs probed after PCI
> > and about ide_register_hw() + initializing flag.
> >
> > What a mess... ordering issues can make you crazy.
>
> Yup, I'd suggest we think about re-writing all that stuff for 2.7 :)
Yes, only remaining question is how... :-)
> > > The simplest solution I have in mind is to add an hwif flag,
> > > called "hold" (or whatever better name you find). Drivers like
> > > ide/ppc/pmac.c would set this flag for the "hotswap" media bay
> > > interface, and not for others.
> >
> > This change is obviously correct and it doesn't have influence on
> > any existing code.
> >
> > > The only change to the core code would then be for ide_register_hw
> > > to 'skip' those when searching for an available slot, and to call
> > > init_hwif_data when (!hwif->present && !hwif->hold) to handle case 2
> > > where the iops & other hwif fields (mmio among others) need to be
> > > reset to initial/legacy state.
> >
> > Less safe change but also okay, as callers .
> > btw, Can't "ghost ides" be dealed inside ppc specific code?
> > Do you know when interface is valid and when it is "ghost",
> > and what other OS-es do in this case?
>
> Not re-using the slot for an interface with the "hold" bit won't
> affect anybody but setters of that bit, so we are ok. The act of
> calling init_hwif_data when !present && !hold is the one bringing
> a possible change of behaviour to existing code.
>
> However, who calls ide_register_hw() dynamically ? ide-cs and ?
> I don't think there would much harm in re-calling init_hwif_data
> at this point since hwif->present is not set, we _are_ re-using
> the hwif, wether it was previously initialized or not by somebody
> else. In this case, we really want to "clean" it, don't we ?
Yes.
> Right now, the only problem with re-initializing this way that
> I've found is with hotswap interfaces like ide-pmac, because
> they will have preset special MMIO ops etc... and that call
> would revert that pre-setting. That's exactly why such interfaces
> should set the "hold" bit to "reserve" the hwif slot. You see
> the point ? I don't think there are much drivers aroung playing
> with such tricks though.
>
> All I can do within ide pmac itself is set or not that "hold"
> bit for those interfaces. I just need to set it on the hotswap
> ones (wthr they have devices connected or not) that way they
> stay around "reserved" for when a device gets plugged. Other
> "fixed" interfaces will have hwif->present cleared automatically
> by the probe code, and thus will be "freed" for other uses, if
> they don't have any device attached.
> (Which is why that init_hwif_data is needed to reset their hwif
> to something good default, and not whatever ide pmac have set).
>
> In 2.5, I can be slightly smarted since I'm calling the probe
> myself and no longer rely on the automatic initial probe done
> by the IDE layer, like for PCI devices, so I can actually
> "clear" those "empty" interfaces myself after they are probed.
> But still, it makes sense to have this "hold" flag to let a
> hotswap interface reserve a slot, and it makes sense when the
> interface isn't held by anybody to "clean it up" before giving
> it to somebody else.
Fully agreed.
> The only problem I see right now is for a dynamic interface
> (like ide-cs) where the _controller_ itself is hotswap, so
> the hwif slot cannot be reserved in advance _and_ that interface
> needs special IOps (which is fortunately not the case of ide-cs)
>
> Such an interface can't really know what slot will be
> picked by ide_register_hw() and can't "prepare" the HWIF with
> special iops, so it won't be much harmed by the fact we are
> calling init_hwif_data, but still, we should ultimately think
> about splitting completely the fact of allocating an hwif slot,
> setting it up, and triggering a probe on it. Those are 3 different
> things that are currently mixed in bad ways. I don't beleive
> fixing that fits in the 2.6 timeframe though.
Yeah, to be done, probably 2.7 :\.
--
Bartlomiej
> Ben.
On 3 May 2003, Benjamin Herrenschmidt wrote:
> > Such an interface can't really know what slot will be
> > picked by ide_register_hw() and can't "prepare" the HWIF with
> > special iops, so it won't be much harmed by the fact we are
> > calling init_hwif_data, but still, we should ultimately think
> > about splitting completely the fact of allocating an hwif slot,
> > setting it up, and triggering a probe on it. Those are 3 different
> > things that are currently mixed in bad ways. I don't beleive
> > fixing that fits in the 2.6 timeframe though.
>
> I just though about another possible crap, though I haven't looked
> enough to be sure, but PCI interfaces with no device will trigger
> a similar problem as "empty" ide-pmac interfaces in that sense that
> they will change dma ops, possibly mmio ops, etc.... If they hold
> no device, their hwif->present will not be set, and thus the hwif
> slot can possibly get re-used by thing like ide-cs (or anybody else
> that rely on ide_register_hw() to allocate a new slot) without
> those changes to hwif done by the PCI interface beeing cleared.
>
> So my patch may actually fix some cases there too.
No, look at ide_match_hwif() in setup-pci.c .
PCI grabs only ide_unknown interfaces.
btw, I think the only real long-term solution for all ordering issues
is customizable device mapper... 2.7?
--
Bartlomiej
> Ben.
On Sat, 2003-05-03 at 19:21, Bartlomiej Zolnierkiewicz wrote:
> >
> > So my patch may actually fix some cases there too.
>
> No, look at ide_match_hwif() in setup-pci.c .
> PCI grabs only ide_unknown interfaces.
No, you missed my point.
1) setup-pci claims a free hwif slot
2) the driver sets some custom IOPs (MMIO PCI interface for example)
and/or does other tweaks to hwif
3) no device is attached to this interface, the IDE probe code leaves
hwif->present to 0, but the hwif fields (IOps etc... are still
set by the PCI driver)
4) later on, ide-cs gets in, and picks that slot since hwif->present
is 0 and ide-cs doesn't care about chipset. However, those IOps
fields (and possibly other, DMA stuff etc...) are still those of
the PCI interface. If the PCI interface set it to MMIO for example,
boom !
Note that I haven't actually tested the above scenario as I don't have
a box with such PCI IDE interfaces, but it seems the problem I have
with ide-pmac in this case is identical.
With my patch, since the PCI interface will not set the "hold" flag,
ide_register_hw() called by ide-cs will call init_hwif_data(), thus
putting back the hwif to a sane state
> btw, I think the only real long-term solution for all ordering issues
> is customizable device mapper... 2.7?
Or not relying on /dev/hdX entries for mounting ? (disk UUID etc..) ;)
On 3 May 2003, Benjamin Herrenschmidt wrote:
> On Sat, 2003-05-03 at 19:21, Bartlomiej Zolnierkiewicz wrote:
>
> > > So my patch may actually fix some cases there too.
> >
> > No, look at ide_match_hwif() in setup-pci.c .
> > PCI grabs only ide_unknown interfaces.
>
> No, you missed my point.
You are right, sorry.
> 1) setup-pci claims a free hwif slot
> 2) the driver sets some custom IOPs (MMIO PCI interface for example)
> and/or does other tweaks to hwif
> 3) no device is attached to this interface, the IDE probe code leaves
> hwif->present to 0, but the hwif fields (IOps etc... are still
> set by the PCI driver)
> 4) later on, ide-cs gets in, and picks that slot since hwif->present
> is 0 and ide-cs doesn't care about chipset. However, those IOps
> fields (and possibly other, DMA stuff etc...) are still those of
> the PCI interface. If the PCI interface set it to MMIO for example,
> boom !
>
> Note that I haven't actually tested the above scenario as I don't have
> a box with such PCI IDE interfaces, but it seems the problem I have
> with ide-pmac in this case is identical.
Yes, I was thinking about PCI IDE hwif and another PCI IDE hwif case.
> With my patch, since the PCI interface will not set the "hold" flag,
> ide_register_hw() called by ide-cs will call init_hwif_data(), thus
> putting back the hwif to a sane state
So every time you remove your disks from one of your PCI IDE controllers,
your ide-cs will get diffirent hwif and drives mappings and your RAID
on ide-cs won't be recognized ;-)
> > btw, I think the only real long-term solution for all ordering issues
> > is customizable device mapper... 2.7?
>
> Or not relying on /dev/hdX entries for mounting ? (disk UUID etc..) ;)
Yep, this is what I mean :-) ie. devlabel.
+ adding kernel parameter like idedev_hdX=model,serial for recovery
--
Bartlomiej
> > With my patch, since the PCI interface will not set the "hold" flag,
> > ide_register_hw() called by ide-cs will call init_hwif_data(), thus
> > putting back the hwif to a sane state
>
> So every time you remove your disks from one of your PCI IDE controllers,
> your ide-cs will get diffirent hwif and drives mappings and your RAID
> on ide-cs won't be recognized ;-)
Yes ;) Note that it's already the case today. Except that with the call
to init_hwif_data(), at least, it wont crash the kernel because of wrong
IOps, DMA ops or whatever in the hwif.
Ben.
> So every time you remove your disks from one of your PCI IDE controllers,
> your ide-cs will get diffirent hwif and drives mappings and your RAID
> on ide-cs won't be recognized ;-)
I don't know what happens with hotplug, but md with persistent
superblocks and autorun (partition type 0xfd) has stable names.
It only whines a bit when the underlying device names change...
On Sat, 3 May 2003, Bartlomiej Zolnierkiewicz wrote:
>
> On 3 May 2003, Benjamin Herrenschmidt wrote:
>
> >
> > > Yesterday I was thinking about:
> > > - default arch hwifs, added/probed in ide_init_defult_hwifs() if no
> > > IDE PCI/PCI (depends on arch, should be IDE PCI?) support is compiled in
> > > - PCI hwifs
> > > - legacy hwifs probed in ide_setup()
> > > - legacy hwifs probed after PCI
> > > and about ide_register_hw() + initializing flag.
> > >
> > > What a mess... ordering issues can make you crazy.
> >
> > Yup, I'd suggest we think about re-writing all that stuff for 2.7 :)
>
> Yes, only remaining question is how... :-)
I will reveal in time, as this is already done.
2.7 I will release the remainder of my voodoo.
> > > > The simplest solution I have in mind is to add an hwif flag,
> > > > called "hold" (or whatever better name you find). Drivers like
> > > > ide/ppc/pmac.c would set this flag for the "hotswap" media bay
> > > > interface, and not for others.
> > >
> > > This change is obviously correct and it doesn't have influence on
> > > any existing code.
> > >
> > > > The only change to the core code would then be for ide_register_hw
> > > > to 'skip' those when searching for an available slot, and to call
> > > > init_hwif_data when (!hwif->present && !hwif->hold) to handle case 2
> > > > where the iops & other hwif fields (mmio among others) need to be
> > > > reset to initial/legacy state.
> > >
> > > Less safe change but also okay, as callers .
> > > btw, Can't "ghost ides" be dealed inside ppc specific code?
> > > Do you know when interface is valid and when it is "ghost",
> > > and what other OS-es do in this case?
> >
> > Not re-using the slot for an interface with the "hold" bit won't
> > affect anybody but setters of that bit, so we are ok. The act of
> > calling init_hwif_data when !present && !hold is the one bringing
> > a possible change of behaviour to existing code.
> >
> > However, who calls ide_register_hw() dynamically ? ide-cs and ?
> > I don't think there would much harm in re-calling init_hwif_data
> > at this point since hwif->present is not set, we _are_ re-using
> > the hwif, wether it was previously initialized or not by somebody
> > else. In this case, we really want to "clean" it, don't we ?
>
> Yes.
>
> > Right now, the only problem with re-initializing this way that
> > I've found is with hotswap interfaces like ide-pmac, because
> > they will have preset special MMIO ops etc... and that call
> > would revert that pre-setting. That's exactly why such interfaces
> > should set the "hold" bit to "reserve" the hwif slot. You see
> > the point ? I don't think there are much drivers aroung playing
> > with such tricks though.
> >
> > All I can do within ide pmac itself is set or not that "hold"
> > bit for those interfaces. I just need to set it on the hotswap
> > ones (wthr they have devices connected or not) that way they
> > stay around "reserved" for when a device gets plugged. Other
> > "fixed" interfaces will have hwif->present cleared automatically
> > by the probe code, and thus will be "freed" for other uses, if
> > they don't have any device attached.
> > (Which is why that init_hwif_data is needed to reset their hwif
> > to something good default, and not whatever ide pmac have set).
> >
> > In 2.5, I can be slightly smarted since I'm calling the probe
> > myself and no longer rely on the automatic initial probe done
> > by the IDE layer, like for PCI devices, so I can actually
> > "clear" those "empty" interfaces myself after they are probed.
> > But still, it makes sense to have this "hold" flag to let a
> > hotswap interface reserve a slot, and it makes sense when the
> > interface isn't held by anybody to "clean it up" before giving
> > it to somebody else.
>
> Fully agreed.
>
> > The only problem I see right now is for a dynamic interface
> > (like ide-cs) where the _controller_ itself is hotswap, so
> > the hwif slot cannot be reserved in advance _and_ that interface
> > needs special IOps (which is fortunately not the case of ide-cs)
> >
> > Such an interface can't really know what slot will be
> > picked by ide_register_hw() and can't "prepare" the HWIF with
> > special iops, so it won't be much harmed by the fact we are
> > calling init_hwif_data, but still, we should ultimately think
> > about splitting completely the fact of allocating an hwif slot,
> > setting it up, and triggering a probe on it. Those are 3 different
> > things that are currently mixed in bad ways. I don't beleive
> > fixing that fits in the 2.6 timeframe though.
>
> Yeah, to be done, probably 2.7 :\.
> --
> Bartlomiej
>
> > Ben.
>
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
Andre Hedrick
LAD Storage Consulting Group