Re: [PATCH RFC] libata: FUA updates

[Tejun Heo]

Robert Hancock wrote:
> This updates libata FUA support to be more more in line with reality.
> FUA support remains off by default.
> 
> Add a setting for the fua command-line parameter on libata which enables
> FUA only on NCQ-supporting disks.
> 
> Update the ata_dev_supports_fua function to remove the blacklisting of
> Maxtor BANC1G10 firmware, as there is no conclusive evidence it was ever
> needed.
> 
> Centralize all of the FUA on/off decisions in this function.
> 
> Bypass the checks for FUA command support bit for NCQ disks, since that
> bit only applies to non-NCQ FUA, and FUA is part of the spec for NCQ
> commands.
> 
> When queue depth is set by the user resulting in enabling/disabling
> NCQ, trigger a revalidate so that the FUA state will be rechecked,
> since disabling/enabling NCQ may also affect FUA support on some disks.
> 
> Add a controller flag to indicate it doesn't support FUA commands, and
> set this flag for Silicon Image 311x (since that one is known to have
> problems) as well as for ITE821x when in smart mode.
> 
> Signed-off-by: Robert Hancock <hancockr@xxxxxxx>

Yeap, I like the clean up.  Just a few comments.

* s/if(/if (/

* The patch adds a number of trailing white spaces.  I suggest using git
or quilt for patch management.  They complain when they see such things.

I'm still not so sure about NCQ FUA.  I still think that separate opcode
for FUA ops is the right thing to do (tm), well, or the safer thing to
do (tm).  Regarding drives which support NCQ but not separate opcode for
FUA, unless many drives are like that, I don't think the loss is too
great.  Also, I'm more uncomfortable with trusting those drives doing
NCQ FUA properly.

Let's talk about this on the other thread.

Thanks.

-- 
tejun
-
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/