Messages in this thread Patch in this message | | | Date | Tue, 26 Jul 2005 13:25:02 -0400 | From | Jeff Garzik <> | Subject | Re: [PATCH 3/3 RESEND] Add disk hotswap support to libata |
| |
Lukasz Kosewski wrote: > Hey Jeff, everyone. > > This is a resend of patch 3 of my libata hotswap series, wherein I > found a silly flaw in my logic. I don't know what kind of crack I was > smoking, but I somehow turned "a = a & ~b" in the Promise driver into > "a = a ^ b" in my hotswap code, which is clearly wrong. I don't know > how it worked through my testing, but I'm willing to bet this is a > coincidence. Please apply this patch instead of the previously sent > patch 03. > > Luke Kosewski > Human Cannonball > Net Integration Technologies > > > ------------------------------------------------------------------------ > > 21.07.05 Luke Kosewski <lkosewsk@nit.ca> > > * A full implementation of hotplug on a libata controller, this being > the Promise Tx4/Tx2 Plus controller line (both SATA150 and SATAII150). > Almost all of the code pertaining to how to talk to the hotplug > registers has been stolen from the pdc-ulsata2 and ultra-1.0.8 Promise > drivers. This involves detecting when we have an interrupt pending > and on what device, as well as the bit where a hard SATA reset gets > a SATAII150 controller to re-spew a plug interrupt. > * Note that the hotplug handling code comes AFTER the normal interrupt > handling code in pdc_interrupt_common; this is because we're much > more likely to receive normal interrupts, so this drops the AVERAGE > interrupt handling time down a lot. > > Signed-off-by: Luke Kosewski <lkosewsk@nit.ca> > > --- linux-2.6.13-rc3/drivers/scsi/sata_promise.c.old 2005-07-21 13:52:13.037895639 -0400 > +++ linux-2.6.13-rc3/drivers/scsi/sata_promise.c 2005-07-21 13:55:53.490964645 -0400 > @@ -84,6 +84,7 @@ static void pdc_eng_timeout(struct ata_p > static int pdc_port_start(struct ata_port *ap); > static void pdc_port_stop(struct ata_port *ap); > static void pdc_phy_reset(struct ata_port *ap); > +static void pdc2_phy_reset(struct ata_port *ap); > static void pdc_pata_phy_reset(struct ata_port *ap); > static void pdc_pata_cbl_detect(struct ata_port *ap); > static void pdc_qc_prep(struct ata_queued_cmd *qc); > @@ -139,7 +140,7 @@ static struct ata_port_operations pdc2_a > .check_status = ata_check_status, > .exec_command = pdc_exec_command_mmio, > .dev_select = ata_std_dev_select, > - .phy_reset = pdc_phy_reset, > + .phy_reset = pdc2_phy_reset, > .qc_prep = pdc_qc_prep, > .qc_issue = pdc_qc_issue_prot, > .eng_timeout = pdc_eng_timeout, > @@ -325,6 +326,48 @@ static void pdc_phy_reset(struct ata_por > pdc_pata_phy_reset(ap); > } > > +/* Mask hotplug interrupts for one channel (ap) */ > +static inline void pdc2_disable_channel_hotplug_interrupts(struct ata_port *ap) > +{ > + void *mmio = ap->host_set->mmio_base + PDC2_SATA_PLUG_CSR + 2; > + > + u8 maskflags = readb(mmio); > + maskflags |= (0x11 << (u8)ap->hard_port_no); > + writeb(maskflags, mmio); > +} > + > +static inline void pdc2_enable_channel_hotplug_interrupts(struct ata_port *ap) > +{ > + > + void *mmio = ap->host_set->mmio_base + PDC2_SATA_PLUG_CSR; > + > + //Clear channel hotplug interrupts > + u8 maskflags = readb(mmio); > + maskflags = (0x11 << (u8)ap->hard_port_no); > + writeb(maskflags, mmio); > + > + //Unmask channel hotplug interrupts > + maskflags = readb(mmio + 2); > + maskflags &= ~(0x11 << (u8)ap->hard_port_no); > + writeb(maskflags, mmio + 2); > +} > + > +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.
> static void pdc_pata_cbl_detect(struct ata_port *ap) > { > u8 tmp; > @@ -483,6 +526,7 @@ static inline unsigned int pdc_interrupt > struct ata_host_set *host_set = dev_instance; > struct ata_port *ap; > void *mmio_base; > + u8 plugdata, maskflags; > u32 mask = 0; > unsigned int i, tmp, handled = 0; > unsigned long flags; > @@ -492,21 +536,18 @@ static inline unsigned int pdc_interrupt > } > > mmio_base = host_set->mmio_base; > - > + > spin_lock_irqsave(&host_set->lock, flags); > > /* reading should also clear interrupts */ > mask = readl(mmio_base + PDC_INT_SEQMASK); > > - if (mask == 0xffffffff) { > - VPRINTK("QUICK EXIT 2\n"); > - goto done_irq; > - } > + 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.
> mask &= 0xffff; /* only 16 tags possible */ > - if (!mask) { > - VPRINTK("QUICK EXIT 3\n"); > - goto done_irq; > - } > + if (!mask) > + goto try_hotplug; > > writel(mask, mmio_base + PDC_INT_SEQMASK); > > @@ -522,7 +563,32 @@ static inline unsigned int pdc_interrupt > handled += pdc_host_intr(ap, qc); > } > } > - > + > + if (handled) > + goto done_irq; > + > +try_hotplug: > + plugdata = readb(mmio_base + hotplug_regs); > + maskflags = readb(mmio_base + hotplug_regs + 2); > + plugdata &= ~maskflags; > + 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.
> done_irq: > spin_unlock_irqrestore(&host_set->lock, flags); > return handled; > @@ -636,9 +702,9 @@ static void pdc_host_init(unsigned int c > tmp = readl(offset); > writel(tmp | 0xff, offset); > > - /* mask plug/unplug ints */ > + /* unmask plug/unplug ints */ > tmp = readl(offset); > - writel(tmp | 0xff0000, offset); > + writel(tmp & 0xff00ffff, offset); > > /* reduce TBG clock to 133 Mhz. */ > tmp = readl(mmio + PDC_TBG_MODE);
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |