Re: hdparm-9.17 released, with experimental trim/wiper scripts for SSDs

[Mark Lord <liml@rtr.ca>]

Robert Hancock wrote:
> On 07/30/2009 06:54 AM, Mark Lord wrote:
>>> Yeah, according to the datasheet "The SiI3124 will decode the 8-bit ATA
>>> command at PRB offset 0x0a and automatically execute the default
>>> protocol for the command. In certain cases it might be desirable to
>>> specify a non-default protocol to be used, such as with vendor
>>> specific device commands." The DSM command seems to be DMA data-out
>>> and the chip likely doesn't know that command. I have to wonder why
>>> they decided to use that design instead of just making the driver
>>> indicate the protocol explicitly. In any case, it looks like the
>>> driver needs code to override the protocol setting for this command.
>>> (Maybe we should just set the protocol override for what we know the
>>> command is supposed to be in all cases?)
>> ..
>>
>> If you can puzzle out how to do that, and post a quick(?) patch,
>> it would certainly make testing SSDs easier for me here.
>>
>> I would like to use the Sil3124 ExpressCard controller with my notebook
>> for this stuff, rather than having to power up one of the noisy
>> full-size systems under the table.  ;)
> 
> You can try this patch (totally untested) which basically just bludgeons it
> into doing what we want for all non-packet commands:
> 
> diff --git a/drivers/ata/sata_sil24.c b/drivers/ata/sata_sil24.c
> index 77aa8d7..e6946fc 100644
> --- a/drivers/ata/sata_sil24.c
> +++ b/drivers/ata/sata_sil24.c
> @@ -846,6 +846,17 @@ static void sil24_qc_prep(struct ata_queued_cmd *qc)
>  	if (!ata_is_atapi(qc->tf.protocol)) {
>  		prb = &cb->ata.prb;
>  		sge = cb->ata.sge;
> +		if (ata_is_data(qc->tf.protocol)) {
> +			u16 prot = 0;
> +			ctrl = PRB_CTRL_PROTOCOL;
> +			if (ata_is_ncq(qc->tf.protocol))
> +				prot |= PRB_PROT_NCQ;
> +			if (qc->tf.flags & ATA_TFLAG_WRITE)
> +				prot |= PRB_PROT_WRITE;
> +			else
> +				prot |= PRB_PROT_READ;
> +			prb->prot = cpu_to_le16(prot);
> +		}
>  	} else {
>  		prb = &cb->atapi.prb;
>  		sge = cb->atapi.sge;
..

Okay, that patch works perfectly here on my Sil24 controller card.
I've run all sorts of commands through it now, including TRIM,
and everything works without any hitches or glitches.

Can we push this upstream for Jeff now ?

Thanks again!