On 7/26/05, Jeff Garzik <jgarzik@xxxxxxxxx> wrote:
> > +static void pdc2_phy_reset(struct ata_port *ap)
> > +{
> > + /* As observed on the Promise SATAII150 Tx2 Plus/Tx4, giving the
> > + * controller a hard reset triggers another hotplug interrupt. So
> > + * disable them for the hard reset, and re-enable afterwards.
> > + *
> > + * No PATA support here yet
> > + */
> > + if (ap->flags & ATA_FLAG_SATA_RESET && ap->flags & ATA_FLAG_SATA) {
> > + pdc2_disable_channel_hotplug_interrupts(ap);
> > + pdc_phy_reset(ap);
> > + pdc2_enable_channel_hotplug_interrupts(ap);
> > + } else
> > + pdc_phy_reset(ap);
> > +}
>
> Don't special case this. disable/enable hotplug interrupts for all
> controllers.
>
OK, I've done this in my new patches.
<snip>
> > + if (mask == 0xffffffff)
> > + goto try_hotplug;
>
> you are confusing controller hotplug (mask == 0xffffffff) and device
> hotplug.
>
> controller hotplug should already be handled correct, by testing
> 0xffffffff and supporting the PCI ->remove hook.
>
Yes. Done this as well.
<snip>
> > + if (plugdata) {
> > + writeb(plugdata, mmio_base + hotplug_regs);
> > + for (i = 0; i < host_set->n_ports; ++i) {
> > + ap = host_set->ports[i];
> > + if (!(ap->flags & ATA_FLAG_SATA))
> > + continue; //No PATA support here... yet
> > + // Check unplug flag
> > + if (plugdata & 0x01) {
> > + ata_hotplug_unplug(ap);
> > + handled = 1;
> > + } else if ((plugdata >> 4) & 0x01) { //Check plug
> > flag
> > + ata_hotplug_plug(ap);
> > + handled = 1;
> > + }
> > + plugdata >>= 1;
>
> you probably need a debounce timer; see the SiI email I am about to send.
Keep in mind that I sent three patches:
- 01 - sata_promise properly supporting SATAII150 controllers
- 02 - libata hotswap infrastructure
- 03 - sata_promise supporting the infrastructure
I believe the debounce timer should really go into patch 02. This is
actually something that's probably needed by every controller... if
you're just plugging drives in/out of a controller, what you're doing
is connecting metal pins, they can be imprecise and trigger a whole
host of interrupts. Hence, I propose changing patch 02 (attached for
all of your convenience) to be as follows:
- on calling ata_hotplug_unplug: if a debounce timer is running, stop
it. Conditionally call ap->ops->unplug_janitor or something like
that, for cases (like the Sil driver, it would seem) that need to do
something special like hard-reset the channel. Start a new
debounce_timer (say, with an interval of 500ms, as suggested by the
Sil guys?
- on calling ata_hotplug_plug: if a debounce_timer is running, stop
it. Conditionally call ap->ops->plug_janitor, if any driver needs
anything special done on a plug action. Start a new one.
- when the debounce_timer goes off, the first thing it does is call
ap->ops->hotplug_action, which returns whether it should be plugging
or unplugging. On a plug, do an ata_bus_probe, an an ata_scsi_plug on
success, ata_scsi_unplug on failure. On an unplug event,
ata_port_disable and then call ata_scsi_unplug (kill any outstanding
taskfiles).
This means that patch 02 will no longer have a workqueue, but a timer.
Any suggestions/comments on this approach?
Luke Kosewski
02-libata_hotswap_infrastructure-2.6.13-rc3-mm1.diff
Description: Text Data
|
