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

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:

--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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)) {
..

Mmm.. that test is failing.  I'll see if I can find where
those bits should be getting set.

Also, the driver now never recovers from a failed TRIM.
It just sits there retrying it over and over and over and over,
even after I unplug the drive, or unplug the controller.
But that might be old behaviour.  :)

Cheers
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Mark Lord wrote:
>
>> @@ -846,6 +846,17 @@ static void sil24_qc_prep(struct ata_queued_cmd *qc)
>>      if (!ata_is_atapi(qc->tf.protocol)) {
> ..
> 
> Mmm.. that test is failing.  I'll see if I can find where
> those bits should be getting set.
..

Pilot error.  Rebuilding the kernel again now.  :)

Thanks!
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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!
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Thu, Jul 30, 2009 at 1:30 PM, Mark Lord<liml@rtr.ca> wrote:
> 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!

OK, I'll submit with sign-off.
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html