2009-04-08 20:05:31

by Jeff Garzik

[permalink] [raw]
Subject: [PATCH] sata_sx4: refresh and convert to new EH


I reviewed and refreshed libata-dev.git#new-eh, now renamed to #sx4.

Several bugs and missing pieces in the new-EH conversion were addressed
(often by copying highly similar code from sata_promise, a highly
similar chip)

The only thing left is some "it still works" testing... any takers?




commit 51aca756268d405fb7cbfc887c55c92185e03b53
Author: Jeff Garzik <[email protected]>
Date: Wed Apr 8 16:02:18 2009 -0400

[libata] sata_sx4: convert to new exception handling methods

Signed-off-by: Jeff Garzik <[email protected]>

diff --git a/drivers/ata/sata_sx4.c b/drivers/ata/sata_sx4.c
index dce3dcc..1ce98af 100644
--- a/drivers/ata/sata_sx4.c
+++ b/drivers/ata/sata_sx4.c
@@ -213,8 +213,9 @@ struct pdc_host_priv {


static int pdc_sata_init_one(struct pci_dev *pdev, const struct pci_device_id *ent);
-static void pdc_eng_timeout(struct ata_port *ap);
-static void pdc_20621_phy_reset(struct ata_port *ap);
+static void pdc_error_handler(struct ata_port *ap);
+static void pdc_freeze(struct ata_port *ap);
+static void pdc_thaw(struct ata_port *ap);
static int pdc_port_start(struct ata_port *ap);
static void pdc20621_qc_prep(struct ata_queued_cmd *qc);
static void pdc_tf_load_mmio(struct ata_port *ap, const struct ata_taskfile *tf);
@@ -233,6 +234,10 @@ static void pdc20621_put_to_dimm(struct ata_host *host,
void *psource, u32 offset, u32 size);
static void pdc20621_irq_clear(struct ata_port *ap);
static unsigned int pdc20621_qc_issue(struct ata_queued_cmd *qc);
+static int pdc_softreset(struct ata_link *link, unsigned int *class,
+ unsigned long deadline);
+static void pdc_post_internal_cmd(struct ata_queued_cmd *qc);
+static int pdc_check_atapi_dma(struct ata_queued_cmd *qc);


static struct scsi_host_template pdc_sata_sht = {
@@ -243,20 +248,24 @@ static struct scsi_host_template pdc_sata_sht = {

/* TODO: inherit from base port_ops after converting to new EH */
static struct ata_port_operations pdc_20621_ops = {
- .sff_tf_load = pdc_tf_load_mmio,
- .sff_tf_read = ata_sff_tf_read,
- .sff_check_status = ata_sff_check_status,
- .sff_exec_command = pdc_exec_command_mmio,
- .sff_dev_select = ata_sff_dev_select,
- .phy_reset = pdc_20621_phy_reset,
+ .inherits = &ata_sff_port_ops,
+
+ .check_atapi_dma = pdc_check_atapi_dma,
.qc_prep = pdc20621_qc_prep,
.qc_issue = pdc20621_qc_issue,
- .qc_fill_rtf = ata_sff_qc_fill_rtf,
- .sff_data_xfer = ata_sff_data_xfer,
- .eng_timeout = pdc_eng_timeout,
- .sff_irq_clear = pdc20621_irq_clear,
- .sff_irq_on = ata_sff_irq_on,
+
+ .freeze = pdc_freeze,
+ .thaw = pdc_thaw,
+ .softreset = pdc_softreset,
+ .error_handler = pdc_error_handler,
+ .lost_interrupt = ATA_OP_NULL,
+ .post_internal_cmd = pdc_post_internal_cmd,
+
.port_start = pdc_port_start,
+
+ .sff_tf_load = pdc_tf_load_mmio,
+ .sff_exec_command = pdc_exec_command_mmio,
+ .sff_irq_clear = pdc20621_irq_clear,
};

static const struct ata_port_info pdc_port_info[] = {
@@ -310,14 +319,6 @@ static int pdc_port_start(struct ata_port *ap)
return 0;
}

-static void pdc_20621_phy_reset(struct ata_port *ap)
-{
- VPRINTK("ENTER\n");
- ap->cbl = ATA_CBL_SATA;
- ata_port_probe(ap);
- ata_bus_reset(ap);
-}
-
static inline void pdc20621_ata_sg(struct ata_taskfile *tf, u8 *buf,
unsigned int portno,
unsigned int total_len)
@@ -859,40 +860,115 @@ static irqreturn_t pdc20621_interrupt(int irq, void *dev_instance)
return IRQ_RETVAL(handled);
}

-static void pdc_eng_timeout(struct ata_port *ap)
+static void pdc_freeze(struct ata_port *ap)
{
- u8 drv_stat;
- struct ata_host *host = ap->host;
- struct ata_queued_cmd *qc;
- unsigned long flags;
+ void __iomem *mmio = ap->ioaddr.cmd_addr;
+ u32 tmp;

- DPRINTK("ENTER\n");
+ /* FIXME: if all 4 ATA engines are stopped, also stop HDMA engine */

- spin_lock_irqsave(&host->lock, flags);
+ tmp = readl(mmio + PDC_CTLSTAT);
+ tmp |= PDC_MASK_INT;
+ tmp &= ~PDC_DMA_ENABLE;
+ writel(tmp, mmio + PDC_CTLSTAT);
+ readl(mmio + PDC_CTLSTAT); /* flush */
+}

- qc = ata_qc_from_tag(ap, ap->link.active_tag);
+static void pdc_thaw(struct ata_port *ap)
+{
+ void __iomem *mmio = ap->ioaddr.cmd_addr;
+ void __iomem *mmio_base;
+ u32 tmp;

- switch (qc->tf.protocol) {
- case ATA_PROT_DMA:
- case ATA_PROT_NODATA:
- ata_port_printk(ap, KERN_ERR, "command timeout\n");
- qc->err_mask |= __ac_err_mask(ata_wait_idle(ap));
- break;
+ /* FIXME: start HDMA engine, if zero ATA engines running */

- default:
- drv_stat = ata_sff_busy_wait(ap, ATA_BUSY | ATA_DRQ, 1000);
+ /* reading SEQ mask register clears IRQ */
+ mmio_base = ap->host->iomap[PDC_MMIO_BAR] + PDC_CHIP0_OFS;
+ readl(mmio_base + PDC_20621_SEQMASK);

- ata_port_printk(ap, KERN_ERR,
- "unknown timeout, cmd 0x%x stat 0x%x\n",
- qc->tf.command, drv_stat);
+ /* turn IRQ back on */
+ tmp = readl(mmio + PDC_CTLSTAT);
+ tmp &= ~PDC_MASK_INT;
+ writel(tmp, mmio + PDC_CTLSTAT);
+ readl(mmio + PDC_CTLSTAT); /* flush */
+}

- qc->err_mask |= ac_err_mask(drv_stat);
- break;
+static void pdc_reset_port(struct ata_port *ap)
+{
+ void __iomem *mmio = ap->ioaddr.cmd_addr + PDC_CTLSTAT;
+ unsigned int i;
+ u32 tmp;
+
+ /* FIXME: handle HDMA copy engine */
+
+ for (i = 11; i > 0; i--) {
+ tmp = readl(mmio);
+ if (tmp & PDC_RESET)
+ break;
+
+ udelay(100);
+
+ tmp |= PDC_RESET;
+ writel(tmp, mmio);
}

- spin_unlock_irqrestore(&host->lock, flags);
- ata_eh_qc_complete(qc);
- DPRINTK("EXIT\n");
+ tmp &= ~PDC_RESET;
+ writel(tmp, mmio);
+ readl(mmio); /* flush */
+}
+
+static int pdc_softreset(struct ata_link *link, unsigned int *class,
+ unsigned long deadline)
+{
+ pdc_reset_port(link->ap);
+ return ata_sff_softreset(link, class, deadline);
+}
+
+static void pdc_error_handler(struct ata_port *ap)
+{
+ if (!(ap->pflags & ATA_PFLAG_FROZEN))
+ pdc_reset_port(ap);
+
+ ata_std_error_handler(ap);
+}
+
+static void pdc_post_internal_cmd(struct ata_queued_cmd *qc)
+{
+ struct ata_port *ap = qc->ap;
+
+ /* make DMA engine forget about the failed command */
+ if (qc->flags & ATA_QCFLAG_FAILED)
+ pdc_reset_port(ap);
+}
+
+static int pdc_check_atapi_dma(struct ata_queued_cmd *qc)
+{
+ u8 *scsicmd = qc->scsicmd->cmnd;
+ int pio = 1; /* atapi dma off by default */
+
+ /* Whitelist commands that may use DMA. */
+ switch (scsicmd[0]) {
+ case WRITE_12:
+ case WRITE_10:
+ case WRITE_6:
+ case READ_12:
+ case READ_10:
+ case READ_6:
+ case 0xad: /* READ_DVD_STRUCTURE */
+ case 0xbe: /* READ_CD */
+ pio = 0;
+ }
+ /* -45150 (FFFF4FA2) to -1 (FFFFFFFF) shall use PIO mode */
+ if (scsicmd[0] == WRITE_10) {
+ unsigned int lba =
+ (scsicmd[2] << 24) |
+ (scsicmd[3] << 16) |
+ (scsicmd[4] << 8) |
+ scsicmd[5];
+ if (lba >= 0xFFFF4FA2)
+ pio = 1;
+ }
+ return pio;
}

static void pdc_tf_load_mmio(struct ata_port *ap, const struct ata_taskfile *tf)


2009-04-12 02:46:46

by Alexander Beregalov

[permalink] [raw]
Subject: Re: [PATCH] sata_sx4: refresh and convert to new EH

2009/4/9 Jeff Garzik <[email protected]>:
>
> I reviewed and refreshed libata-dev.git#new-eh, now renamed to #sx4.
>
> Several bugs and missing pieces in the new-EH conversion were addressed
> (often by copying highly similar code from sata_promise, a highly
> similar chip)
>
> The only thing left is some "it still works" testing...  any takers?
>

Hi Jeff

I have SX4000 card, it is based on the same PDC20621 chip, but has PATA ports
(without PATA-SATA bridges).
Can I help?

I am newbie here, but I will try to modify the driver to support PATA
card as well
and test your patch.

2009-04-12 11:19:23

by Jeff Garzik

[permalink] [raw]
Subject: Re: [PATCH] sata_sx4: refresh and convert to new EH

Alexander Beregalov wrote:
> 2009/4/9 Jeff Garzik <[email protected]>:
>> I reviewed and refreshed libata-dev.git#new-eh, now renamed to #sx4.
>>
>> Several bugs and missing pieces in the new-EH conversion were addressed
>> (often by copying highly similar code from sata_promise, a highly
>> similar chip)
>>
>> The only thing left is some "it still works" testing... any takers?
>>
>
> Hi Jeff
>
> I have SX4000 card, it is based on the same PDC20621 chip, but has PATA ports
> (without PATA-SATA bridges).
> Can I help?
>
> I am newbie here, but I will try to modify the driver to support PATA
> card as well
> and test your patch.

It's unknown how well, or what is needed, to support the SX4000. You
are welcome to dive in and figure out what is needed, but you'd be
asking questions to which we wouldn't have answers.

Jeff



2009-04-13 02:24:10

by Alexander Beregalov

[permalink] [raw]
Subject: Re: [PATCH] sata_sx4: refresh and convert to new EH

2009/4/12 Jeff Garzik <[email protected]>:
> Alexander Beregalov wrote:
>>
>> 2009/4/9 Jeff Garzik <[email protected]>:
>>>
>>> I reviewed and refreshed libata-dev.git#new-eh, now renamed to #sx4.
>>>
>>> Several bugs and missing pieces in the new-EH conversion were addressed
>>> (often by copying highly similar code from sata_promise, a highly
>>> similar chip)
>>>
>>> The only thing left is some "it still works" testing...  any takers?
>>>
>>
>> Hi Jeff
>>
>> I have SX4000 card, it is based on the same PDC20621 chip, but has PATA
>> ports
>> (without PATA-SATA bridges).
>> Can I help?
>>
>> I am newbie here, but I will try to modify the driver to support PATA
>> card as well
>> and test your patch.
>
> It's unknown how well, or what is needed, to support the SX4000.  You are
> welcome to dive in and figure out what is needed, but you'd be asking
> questions to which we wouldn't have answers.

I found Promise provided docs for FreeBSD team.
http://unix.derkeiler.com/Mailing-Lists/FreeBSD/performance/2003-07/0043.html
>I do have the sx4000 in the works (thanks to Promise who has provided docs
>and HW to make this happen)..
Can we ask them too? :)

Interesting, I cut off all unrelated lines from freebsd/ata/ata-chipset.c
and it seems SX4/SX4000 driver is only about 700 lines long.

2009-04-13 09:09:22

by Jeff Garzik

[permalink] [raw]
Subject: Re: [PATCH] sata_sx4: refresh and convert to new EH

Alexander Beregalov wrote:
> 2009/4/12 Jeff Garzik <[email protected]>:
>> Alexander Beregalov wrote:
>>> 2009/4/9 Jeff Garzik <[email protected]>:
>>>> I reviewed and refreshed libata-dev.git#new-eh, now renamed to #sx4.
>>>>
>>>> Several bugs and missing pieces in the new-EH conversion were addressed
>>>> (often by copying highly similar code from sata_promise, a highly
>>>> similar chip)
>>>>
>>>> The only thing left is some "it still works" testing... any takers?
>>>>
>>> Hi Jeff
>>>
>>> I have SX4000 card, it is based on the same PDC20621 chip, but has PATA
>>> ports
>>> (without PATA-SATA bridges).
>>> Can I help?
>>>
>>> I am newbie here, but I will try to modify the driver to support PATA
>>> card as well
>>> and test your patch.
>> It's unknown how well, or what is needed, to support the SX4000. You are
>> welcome to dive in and figure out what is needed, but you'd be asking
>> questions to which we wouldn't have answers.
>
> I found Promise provided docs for FreeBSD team.
> http://unix.derkeiler.com/Mailing-Lists/FreeBSD/performance/2003-07/0043.html

Well, you can find several docs at
http://gkernel.sourceforge.net/specs/promise/


>> I do have the sx4000 in the works (thanks to Promise who has provided docs
>> and HW to make this happen)..
> Can we ask them too? :)
>
> Interesting, I cut off all unrelated lines from freebsd/ata/ata-chipset.c
> and it seems SX4/SX4000 driver is only about 700 lines long.

The current FreeBSD has split up things into multiple files. You can
check out the kernel via

cd $repo
mkdir -p freebsd/src
cd freebsd
svn checkout svn://svn.freebsd.org/base/head/sys src/sys
cd src/sys/dev/ata/chipsets

and indeed ata-promise.c has plenty of SX4[000] goodies to mess around with!

Also, check out this stuff from sata_promise maintainer Mikael:
http://marc.info/?l=linux-ide&m=121144512109826&w=2
http://marc.info/?l=linux-ide&m=121135828227724&w=2

My revised sata_sx4.c is currently stored on the "sx4" branch of
git://git.kernel.org/pub/scm/linux/kernel/git/jgarzik/libata-dev.git

Jeff