>>>>> "Damien" == Damien Le Moal <damien.lemoal@xxxxxxxx> writes:
Damien,
The new stuff looks much cleaner. Thanks for doing that!
This hunk has an unintended side effect:
@@ -2836,14 +2896,14 @@ static int sd_revalidate_disk(struct gendisk *disk)
* react badly if we do.
*/
if (sdkp->media_present) {
- sd_read_capacity(sdkp, buffer);
-
if (scsi_device_supports_vpd(sdp)) {
sd_read_block_provisioning(sdkp);
sd_read_block_limits(sdkp);
sd_read_block_characteristics(sdkp);
}
+ sd_read_capacity(sdkp, buffer);
+
sd_read_write_protect_flag(sdkp, buffer);
sd_read_cache_type(sdkp, buffer);
sd_read_app_tag_own(sdkp, buffer);
LMBPE from READ CAPACITY(16) will not be set when calling
sd_read_block_provisioning() and thus we bail early (we catch it second
time around). You may want to split that vpd conditional to shuffle the
block_characteristics on top. Or even better: Split the capacity/block
size printing code from sd_read_capacity() into a separate function and
do:
if (sdkp->media_present) {
sd_read_capacity(sdkp, buffer);
if (sd_try_extended_inquiry(sdp)) {
sd_read_block_provisioning(sdkp);
sd_read_block_limits(sdkp);
sd_read_block_characteristics(sdkp);
}
+ sd_print_capacity(sdkp);
sd_read_write_protect_flag(sdkp, buffer);
sd_read_cache_type(sdkp, buffer);
sd_read_app_tag_own(sdkp, buffer);
sd_read_write_same(sdkp, buffer);
}
Typo:
static int sd_zbc_read_zoned_charateristics(struct scsi_disk *sdkp,
^^^^^^^^^^^^^^
My second comment is that I don't particularly like the notion of values
being stored and passed in units of block layer sectors in struct
scsi_disk and sd*. As a rule of thumb I prefer SCSI stuff to be counted
in logical blocks and block layer stuff in 512b sectors. You may want to
entertain having a sectors_to_zone() wrapper to facilitate that
conversion.
--
Martin K. Petersen Oracle Linux Engineering
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
