Re: + via-pata-controller-xfer-fixes.patch added to -mm tree

[Albert Lee]

matthieu castet wrote:
> matthieu castet wrote:
> 
>> Hi Albert,
>>
>> Albert Lee wrote:
>>
>>> castet.matthieu@xxxxxxx wrote:
>>>
>>>>
>>>>> Could you please test the current libata-upstream tree and
>>>>> turn on ATA_DEBUG and ATA_VERBOSE_DEBUG in include/linux/libata.h.
>>>>>
>>>>
>>>> Is there a easy way to get libata-upstream tree ?
>>>> Do I need to install git for that or there are some snapshots
>>>> somewhere ?
>>>>
>>>>
>>>
>>>
>>> Hi Matthieu,
>>>
>>> Tejun has a patch against 2.6.17:
>>> http://home-tj.org/files/libata-tj-stable/libata-tj-2.6.17-20060625-1.tar.bz2
>>>
>>>
>> I don't know if I did someting wrong, but it didn't apply cleanly.
>> So I enable the trace on lastest -mm kernel and I disable the via quirk.
>>
>> But the printk in the interrupt handler takes some times and hides the
>> altstatus delay.
>>
>> I will try to send you a trace, where I move the printk at the end of
>> the interrupt handler.
>>
>>
> After apllying the following patch to -mm, I got
> http://castet.matthieu.free.fr/tmp/ata_log
> 
> Matthieu

Hi Matthieu,

Thanks for the log. But could you please keep the 
VPRINTK() in the entrance of ata_host_intr() and add some message to
distinguish the three VPRINTK()s?

(This will make the log easier to read. Detail below.)


> 
> 
> ------------------------------------------------------------------------
> 
> Index: linux-2.6.16/drivers/scsi/libata-core.c
> ===================================================================
> --- linux-2.6.16.orig/drivers/scsi/libata-core.c	2006-07-02 10:32:33.000000000 +0200
> +++ linux-2.6.16/drivers/scsi/libata-core.c	2006-07-02 10:38:03.000000000 +0200
> @@ -4722,9 +4722,6 @@
>  {
>  	u8 status, host_stat = 0;
>  
> -	VPRINTK("ata%u: protocol %d task_state %d\n",
> -		ap->id, qc->tf.protocol, ap->hsm_task_state);
> -

Please keep this VPRINTK() and add "ENTER" to it. Something like
"ata%u: ENTER protocol...".

>  	/* Check whether we are expecting interrupt in this state */
>  	switch (ap->hsm_task_state) {
>  	case HSM_ST_FIRST:
> @@ -4780,6 +4777,9 @@
>  	ap->ops->irq_clear(ap);
>  
>  	ata_hsm_move(ap, qc, status, 0);
> +	VPRINTK("ata%u: protocol %d task_state %d\n",
> +		ap->id, qc->tf.protocol, ap->hsm_task_state);
> +
>  	return 1;	/* irq handled */
>  

Please add additional message "HANDLED" to this VPRINTK(). Something like
"ata%u: HANDLED protocol...".

>  idle_irq:
> @@ -4792,6 +4792,9 @@
>  		return 1;
>  	}
>  #endif
> +	VPRINTK("ata%u: protocol %d task_state %d\n",
> +		ap->id, qc->tf.protocol, ap->hsm_task_state);
> +
>  	return 0;	/* irq not handled */
>  }

Please add additional message "IGNORED" to this VPRINTK(). Something like
"ata%u: IGNORED protocol ...". 



Thanks,

Albert


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/