[PATCH 4/5] libata/sff: Use ops->bmdma_stop instead of ata_bmdma_stop()

[PATCH 4/5] libata/sff: Use ops->bmdma_stop instead of ata_bmdma_stop()

[Benjamin Herrenschmidt]

In libata-sff, ata_sff_post_internal_cmd() directly calls ata_bmdma_stop()
instead of ap->ops->bmdma_stop(). This can be a problem for controllers
that use their own bmdma_stop for which the generic sff one isn't suitable

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---

 drivers/ata/libata-sff.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- linux-work.orig/drivers/ata/libata-sff.c	2009-12-01 17:48:27.000000000 +1100
+++ linux-work/drivers/ata/libata-sff.c	2009-12-01 17:48:48.000000000 +1100
@@ -2384,7 +2384,7 @@ void ata_sff_post_internal_cmd(struct at
 	ap->hsm_task_state = HSM_ST_IDLE;
 
 	if (ap->ioaddr.bmdma_addr)
-		ata_bmdma_stop(qc);
+		ap->ops->bmdma_stop(qc);
 
 	spin_unlock_irqrestore(ap->lock, flags);
 }

Re: [PATCH 4/5] libata/sff: Use ops->bmdma_stop instead of ata_bmdma_stop()

[Tejun Heo]

On 12/01/2009 04:08 PM, Benjamin Herrenschmidt wrote:
> In libata-sff, ata_sff_post_internal_cmd() directly calls ata_bmdma_stop()
> instead of ap->ops->bmdma_stop(). This can be a problem for controllers
> that use their own bmdma_stop for which the generic sff one isn't suitable
> 
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

Oh... that's a scary bug lurking around.  Thanks for catching it.

Acked-by: Tejun Heo <tj@kernel.org>

-- 
tejun

Re: [PATCH 4/5] libata/sff: Use ops->bmdma_stop instead of ata_bmdma_stop()

[Benjamin Herrenschmidt]

On Tue, 2009-12-01 at 16:25 +0900, Tejun Heo wrote:
> On 12/01/2009 04:08 PM, Benjamin Herrenschmidt wrote:
> > In libata-sff, ata_sff_post_internal_cmd() directly calls ata_bmdma_stop()
> > instead of ap->ops->bmdma_stop(). This can be a problem for controllers
> > that use their own bmdma_stop for which the generic sff one isn't suitable
> > 
> > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> 
> Oh... that's a scary bug lurking around.  Thanks for catching it.
> 
> Acked-by: Tejun Heo <tj@kernel.org>

Feel free to pick that one up earlier if you want (ie for 2.6.32), as
long as we manage to get it in in 2.6.33 -before- I push powerpc-next to
Linus, I'm happy :-) (or we can have it in both trees).

Cheers,
Ben.

Re: [PATCH 4/5] libata/sff: Use ops->bmdma_stop instead of ata_bmdma_stop()

[Tejun Heo]

On 12/01/2009 04:29 PM, Benjamin Herrenschmidt wrote:
> On Tue, 2009-12-01 at 16:25 +0900, Tejun Heo wrote:
>> On 12/01/2009 04:08 PM, Benjamin Herrenschmidt wrote:
>>> In libata-sff, ata_sff_post_internal_cmd() directly calls ata_bmdma_stop()
>>> instead of ap->ops->bmdma_stop(). This can be a problem for controllers
>>> that use their own bmdma_stop for which the generic sff one isn't suitable
>>>
>>> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>>
>> Oh... that's a scary bug lurking around.  Thanks for catching it.
>>
>> Acked-by: Tejun Heo <tj@kernel.org>
> 
> Feel free to pick that one up earlier if you want (ie for 2.6.32), as
> long as we manage to get it in in 2.6.33 -before- I push powerpc-next to
> Linus, I'm happy :-) (or we can have it in both trees).

As nasty as the bug might be, given that we haven't had too much
problem with that, I think it would be best to give it some time to be
tested before releasing it to users.  It causes behavior differences
for all bmdma drivers which implement custom bmdma_stop.  It's an
apparent bug fix but well the nasty ones are always apparent bug
fixes, right?  :-)

Thanks.

-- 
tejun

Re: [PATCH 4/5] libata/sff: Use ops->bmdma_stop instead of ata_bmdma_stop()

[Benjamin Herrenschmidt]

On Tue, 2009-12-01 at 16:33 +0900, Tejun Heo wrote:
> > Feel free to pick that one up earlier if you want (ie for 2.6.32),
> as
> > long as we manage to get it in in 2.6.33 -before- I push
> powerpc-next to
> > Linus, I'm happy :-) (or we can have it in both trees).
> 
> As nasty as the bug might be, given that we haven't had too much
> problem with that, I think it would be best to give it some time to be
> tested before releasing it to users.  It causes behavior differences
> for all bmdma drivers which implement custom bmdma_stop.  It's an
> apparent bug fix but well the nasty ones are always apparent bug
> fixes, right?  :-) 

Yeah, I agree. I wouldn't be comfortable pushing it into .32 to be
honest after a quick grep of how many drivers actually use that hook and
considering that no problem was reported so far...

I'll merge it with the rest of the powerpc stuff during the .33 merge
window if you are ok with that.

Cheers,
Ben.

[PATCH 4/5] libata/sff: Use ops->bmdma_stop instead of ata_bmdma_stop()

[Benjamin Herrenschmidt]

In libata-sff, ata_sff_post_internal_cmd() directly calls ata_bmdma_stop()
instead of ap->ops->bmdma_stop(). This can be a problem for controllers
that use their own bmdma_stop for which the generic sff one isn't suitable

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---

 drivers/ata/libata-sff.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- linux-work.orig/drivers/ata/libata-sff.c	2009-12-01 17:48:27.000000000 +1100
+++ linux-work/drivers/ata/libata-sff.c	2009-12-01 17:48:48.000000000 +1100
@@ -2384,7 +2384,7 @@ void ata_sff_post_internal_cmd(struct at
 	ap->hsm_task_state = HSM_ST_IDLE;
 
 	if (ap->ioaddr.bmdma_addr)
-		ata_bmdma_stop(qc);
+		ap->ops->bmdma_stop(qc);
 
 	spin_unlock_irqrestore(ap->lock, flags);
 }

Re: [PATCH 4/5] libata/sff: Use ops->bmdma_stop instead of ata_bmdma_stop()

[Jeff Garzik]

On 12/01/2009 07:36 PM, Benjamin Herrenschmidt wrote:
> In libata-sff, ata_sff_post_internal_cmd() directly calls ata_bmdma_stop()
> instead of ap->ops->bmdma_stop(). This can be a problem for controllers
> that use their own bmdma_stop for which the generic sff one isn't suitable
>
> Signed-off-by: Benjamin Herrenschmidt<benh@kernel.crashing.org>
> ---
>
>   drivers/ata/libata-sff.c |    2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> --- linux-work.orig/drivers/ata/libata-sff.c	2009-12-01 17:48:27.000000000 +1100
> +++ linux-work/drivers/ata/libata-sff.c	2009-12-01 17:48:48.000000000 +1100
> @@ -2384,7 +2384,7 @@ void ata_sff_post_internal_cmd(struct at
>   	ap->hsm_task_state = HSM_ST_IDLE;
>
>   	if (ap->ioaddr.bmdma_addr)
> -		ata_bmdma_stop(qc);
> +		ap->ops->bmdma_stop(qc);
>
>   	spin_unlock_irqrestore(ap->lock, flags);

applied

As we discussed, feel free to carry this in your tree as well.  git 
should be able to sort it out.

Acked-by: Jeff Garzik <jgarzik@redhat.com>