Messages in this thread | | | Date | Fri, 17 Jun 2011 14:07:04 +0200 | From | Tejun Heo <> | Subject | Re: [PATCH v3] ahci: move ahci_sb600_softreset to libahci.c and rename it |
| |
Hello, Yuan-Hsin.
Yeap, we're getting almost there. :)
On Fri, Jun 17, 2011 at 06:39:18PM +0800, Yuan-Hsin Chen wrote: > diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h > index 12c5282..e20f8d7 100644 > --- a/drivers/ata/ahci.h > +++ b/drivers/ata/ahci.h > @@ -312,6 +312,7 @@ extern struct device_attribute *ahci_sdev_attrs[]; > .sdev_attrs = ahci_sdev_attrs > > extern struct ata_port_operations ahci_ops; > +extern struct ata_port_operations ahci_pmp_ops;
But I don't like ahci_pmp_ops name. ahci_ops works just fine for pmp. It's a workaround ops. Maybe something like ahci_pmp_retry_srst_ops?
> +static int ahci_pmp_softreset(struct ata_link *link, unsigned int *class, > + unsigned long deadline);
Similarly, ahci_pmp_retry_srst_softreset()
> +struct ata_port_operations ahci_pmp_ops = { > + .inherits = &ahci_ops, > + .softreset = ahci_pmp_softreset, > + .pmp_softreset = ahci_pmp_softreset, > +};
Isn't overriding .softreset enough? .pmp_softreset is used only for downstream links.
> +static int ahci_pmp_check_ready(struct ata_link *link) > +{ > + void __iomem *port_mmio = ahci_port_base(link->ap); > + u8 status = readl(port_mmio + PORT_TFDATA) & 0xFF; > + u32 irq_status = readl(port_mmio + PORT_IRQ_STAT); > + > + /* > + * There is no need to check TFDATA if BAD PMP is found due to HW bug, > + * which can save timeout delay. > + */ > + if (irq_status & PORT_IRQ_BAD_PMP) > + return -EIO; > + > + printk("%s:TFT 0x%x\n", __func__, status);
You probably want ata_link_printk(link, KERN_DEBUG, ...);
Thanks.
-- tejun
| |