* [PATCH 2/7] sata_promise: pdc_freeze() semantic change
2007-07-07 6:57 [PATCH 0/7] libata: irq_on/off restructuring (take #2) Albert Lee
@ 2007-07-07 7:02 ` Albert Lee
2007-10-02 15:28 ` Jeff Garzik
0 siblings, 1 reply; 6+ messages in thread
From: Albert Lee @ 2007-07-07 7:02 UTC (permalink / raw)
To: Jeff Garzik
Cc: Alan Cox, Doug Maxey, Mark Lord, Tejun Heo, Linux IDE,
Mikael Pettersson
After checking the current implementations of freeze()/thaw(), it seems only pdc_freeze()
does more than simple irq masking. Remove the DMA stop code from pdc_freeze().
Signed-off-by: Albert Lee <albertcc@tw.ibm.com>
---
diff -Nrup 01_remove_leftover_irqon/drivers/ata/sata_promise.c 02_sata_pdc_freeze/drivers/ata/sata_promise.c
--- 01_remove_leftover_irqon/drivers/ata/sata_promise.c 2007-07-07 09:58:55.000000000 +0800
+++ 02_sata_pdc_freeze/drivers/ata/sata_promise.c 2007-07-07 10:39:22.000000000 +0800
@@ -578,7 +578,6 @@ static void pdc_freeze(struct ata_port *
tmp = readl(mmio + PDC_CTLSTAT);
tmp |= PDC_IRQ_DISABLE;
- tmp &= ~PDC_DMA_ENABLE;
writel(tmp, mmio + PDC_CTLSTAT);
readl(mmio + PDC_CTLSTAT); /* flush */
}
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/7] sata_promise: pdc_freeze() semantic change
@ 2007-07-07 19:08 Mikael Pettersson
2007-07-10 16:06 ` Tejun Heo
0 siblings, 1 reply; 6+ messages in thread
From: Mikael Pettersson @ 2007-07-07 19:08 UTC (permalink / raw)
To: albertl, jeff; +Cc: alan, dwm, htejun, linux-ide, mikpe, mlord
On Sat, 07 Jul 2007 15:02:49 +0800, Albert Lee wrote:
> After checking the current implementations of freeze()/thaw(), it seems only pdc_freeze()
>does more than simple irq masking. Remove the DMA stop code from pdc_freeze().
>
>Signed-off-by: Albert Lee <albertcc@tw.ibm.com>
>---
>
>diff -Nrup 01_remove_leftover_irqon/drivers/ata/sata_promise.c 02_sata_pdc_freeze/drivers/ata/sata_promise.c
>--- 01_remove_leftover_irqon/drivers/ata/sata_promise.c 2007-07-07 09:58:55.000000000 +0800
>+++ 02_sata_pdc_freeze/drivers/ata/sata_promise.c 2007-07-07 10:39:22.000000000 +0800
>@@ -578,7 +578,6 @@ static void pdc_freeze(struct ata_port *
>
> tmp = readl(mmio + PDC_CTLSTAT);
> tmp |= PDC_IRQ_DISABLE;
>- tmp &= ~PDC_DMA_ENABLE;
> writel(tmp, mmio + PDC_CTLSTAT);
> readl(mmio + PDC_CTLSTAT); /* flush */
> }
pdc_freeze() halts in-flight DMAs because that's what libata
specifices. E.g., Documentation/DocBook/libata.tmpl writes:
>void (*freeze) (struct ata_port *ap);
>void (*thaw) (struct ata_port *ap);
> </programlisting>
>
> <para>
>ata_port_freeze() is called when HSM violations or some other
>condition disrupts normal operation of the port. A frozen port
>is not allowed to perform any operation until the port is
>thawed, which usually follows a successful reset.
> </para>
>
> <para>
>The optional ->freeze() callback can be used for freezing the port
>hardware-wise (e.g. mask interrupt and stop DMA engine). If a
>port cannot be frozen hardware-wise, the interrupt handler
>must ack and clear interrupts unconditionally while the port
>is frozen.
> </para>
> <para>
>The optional ->thaw() callback is called to perform the opposite of ->freeze():
>prepare the port for normal operation once again. Unmask interrupts,
>start DMA engine, etc.
Similar wording exists in libata-eh.c above __ata_port_freeze().
Albert's patch is OK as far as sata_promise is concerned, but
I want to see an update of libata.tmpl and libata-eh.c to
indicate the new, weakened, specification of freeze/thaw before
I ACK this patch.
/Mikael
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/7] sata_promise: pdc_freeze() semantic change
2007-07-07 19:08 [PATCH 2/7] sata_promise: pdc_freeze() semantic change Mikael Pettersson
@ 2007-07-10 16:06 ` Tejun Heo
2007-07-13 9:07 ` Tejun Heo
0 siblings, 1 reply; 6+ messages in thread
From: Tejun Heo @ 2007-07-10 16:06 UTC (permalink / raw)
To: Mikael Pettersson; +Cc: albertl, jeff, alan, dwm, linux-ide, mlord
Mikael Pettersson wrote:
> Albert's patch is OK as far as sata_promise is concerned, but
> I want to see an update of libata.tmpl and libata-eh.c to
> indicate the new, weakened, specification of freeze/thaw before
> I ACK this patch.
Aye aye sir.
--
tejun
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/7] sata_promise: pdc_freeze() semantic change
2007-07-10 16:06 ` Tejun Heo
@ 2007-07-13 9:07 ` Tejun Heo
2007-07-16 9:19 ` Albert Lee
0 siblings, 1 reply; 6+ messages in thread
From: Tejun Heo @ 2007-07-13 9:07 UTC (permalink / raw)
To: Mikael Pettersson, albertl; +Cc: jeff, alan, dwm, linux-ide, mlord
Tejun Heo wrote:
> Mikael Pettersson wrote:
>> Albert's patch is OK as far as sata_promise is concerned, but
>> I want to see an update of libata.tmpl and libata-eh.c to
>> indicate the new, weakened, specification of freeze/thaw before
>> I ACK this patch.
Hmmm.... The docbook document is too stale at this point. It needs
major rewrite. Also, I don't really think docbook is a good format for
this type of documentation. Can we switch to formatted plain text which
can be converted to other formats? Or use some form of inlined comment
for callback documentation which can be gathered and formatted
automatically?
Albert, do you mind updating the comment in libata-eh.c?
--
tejun
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/7] sata_promise: pdc_freeze() semantic change
2007-07-13 9:07 ` Tejun Heo
@ 2007-07-16 9:19 ` Albert Lee
0 siblings, 0 replies; 6+ messages in thread
From: Albert Lee @ 2007-07-16 9:19 UTC (permalink / raw)
To: Tejun Heo; +Cc: Mikael Pettersson, albertl, jeff, alan, dwm, linux-ide, mlord
Tejun Heo wrote:
> Tejun Heo wrote:
>
>>Mikael Pettersson wrote:
>>
>>>Albert's patch is OK as far as sata_promise is concerned, but
>>>I want to see an update of libata.tmpl and libata-eh.c to
>>>indicate the new, weakened, specification of freeze/thaw before
>>>I ACK this patch.
>
>
> Hmmm.... The docbook document is too stale at this point. It needs
> major rewrite. Also, I don't really think docbook is a good format for
> this type of documentation. Can we switch to formatted plain text which
> can be converted to other formats? Or use some form of inlined comment
> for callback documentation which can be gathered and formatted
> automatically?
>
> Albert, do you mind updating the comment in libata-eh.c?
>
No problem. Will fix the comments in the next revision.
--
albert
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/7] sata_promise: pdc_freeze() semantic change
2007-07-07 7:02 ` [PATCH 2/7] sata_promise: pdc_freeze() semantic change Albert Lee
@ 2007-10-02 15:28 ` Jeff Garzik
0 siblings, 0 replies; 6+ messages in thread
From: Jeff Garzik @ 2007-10-02 15:28 UTC (permalink / raw)
To: albertl, Mikael Pettersson
Cc: Alan Cox, Doug Maxey, Mark Lord, Tejun Heo, Linux IDE
Albert Lee wrote:
> After checking the current implementations of freeze()/thaw(), it seems only pdc_freeze()
> does more than simple irq masking. Remove the DMA stop code from pdc_freeze().
>
> Signed-off-by: Albert Lee <albertcc@tw.ibm.com>
> ---
>
> diff -Nrup 01_remove_leftover_irqon/drivers/ata/sata_promise.c 02_sata_pdc_freeze/drivers/ata/sata_promise.c
> --- 01_remove_leftover_irqon/drivers/ata/sata_promise.c 2007-07-07 09:58:55.000000000 +0800
> +++ 02_sata_pdc_freeze/drivers/ata/sata_promise.c 2007-07-07 10:39:22.000000000 +0800
> @@ -578,7 +578,6 @@ static void pdc_freeze(struct ata_port *
>
> tmp = readl(mmio + PDC_CTLSTAT);
> tmp |= PDC_IRQ_DISABLE;
> - tmp &= ~PDC_DMA_ENABLE;
> writel(tmp, mmio + PDC_CTLSTAT);
> readl(mmio + PDC_CTLSTAT); /* flush */
Two comments:
1) I do not think it safe to simply remove this. Have you verified that
pdc_reset_port() without a disabled DMA engine works 100%? At the very
least I would say to -move- this code, rather the deleting it.
2) It was put there to disable the DMA engine at our first opportunity.
For some errors we really should take some preliminary
port/DMA-disable actions in the interrupt handler, before kicking off EH
and waiting to be scheduled in process context.
Jeff
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2007-10-02 15:28 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-07-07 19:08 [PATCH 2/7] sata_promise: pdc_freeze() semantic change Mikael Pettersson
2007-07-10 16:06 ` Tejun Heo
2007-07-13 9:07 ` Tejun Heo
2007-07-16 9:19 ` Albert Lee
-- strict thread matches above, loose matches on Subject: below --
2007-07-07 6:57 [PATCH 0/7] libata: irq_on/off restructuring (take #2) Albert Lee
2007-07-07 7:02 ` [PATCH 2/7] sata_promise: pdc_freeze() semantic change Albert Lee
2007-10-02 15:28 ` Jeff Garzik
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).