[PATCH v2] scsi: sd_zbc: Clean up sd_zbc_parse_report() setting of wp

[PATCH v2] scsi: sd_zbc: Clean up sd_zbc_parse_report() setting of wp

[Niklas Cassel]

From: Niklas Cassel <niklas.cassel@wdc.com>

Make sd_zbc_parse_report() use if/else when setting the write pointer,
instead of setting it unconditionally and then conditionally updating it.

Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
Changes since V2:
-Picked up Johannes' tag.
-Dropped patch 1/2 from the series. The actual +/- lines of this patch is
 unchanged.

 drivers/scsi/sd_zbc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/sd_zbc.c b/drivers/scsi/sd_zbc.c
index ed06798983f8..20e849437687 100644
--- a/drivers/scsi/sd_zbc.c
+++ b/drivers/scsi/sd_zbc.c
@@ -61,10 +61,11 @@ static int sd_zbc_parse_report(struct scsi_disk *sdkp, u8 *buf,
 	zone.len = logical_to_sectors(sdp, get_unaligned_be64(&buf[8]));
 	zone.capacity = zone.len;
 	zone.start = logical_to_sectors(sdp, get_unaligned_be64(&buf[16]));
-	zone.wp = logical_to_sectors(sdp, get_unaligned_be64(&buf[24]));
 	if (zone.type != ZBC_ZONE_TYPE_CONV &&
 	    zone.cond == ZBC_ZONE_COND_FULL)
 		zone.wp = zone.start + zone.len;
+	else
+		zone.wp = logical_to_sectors(sdp, get_unaligned_be64(&buf[24]));
 
 	ret = cb(&zone, idx, data);
 	if (ret)
-- 
2.33.1

Re: [PATCH v2] scsi: sd_zbc: Clean up sd_zbc_parse_report() setting of wp

[Damien Le Moal]

On 2021/11/29 22:15, Niklas Cassel wrote:
> From: Niklas Cassel <niklas.cassel@wdc.com>
> 
> Make sd_zbc_parse_report() use if/else when setting the write pointer,
> instead of setting it unconditionally and then conditionally updating it.
> 
> Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> ---
> Changes since V2:
> -Picked up Johannes' tag.
> -Dropped patch 1/2 from the series. The actual +/- lines of this patch is
>  unchanged.
> 
>  drivers/scsi/sd_zbc.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/sd_zbc.c b/drivers/scsi/sd_zbc.c
> index ed06798983f8..20e849437687 100644
> --- a/drivers/scsi/sd_zbc.c
> +++ b/drivers/scsi/sd_zbc.c
> @@ -61,10 +61,11 @@ static int sd_zbc_parse_report(struct scsi_disk *sdkp, u8 *buf,
>  	zone.len = logical_to_sectors(sdp, get_unaligned_be64(&buf[8]));
>  	zone.capacity = zone.len;
>  	zone.start = logical_to_sectors(sdp, get_unaligned_be64(&buf[16]));
> -	zone.wp = logical_to_sectors(sdp, get_unaligned_be64(&buf[24]));
>  	if (zone.type != ZBC_ZONE_TYPE_CONV &&
>  	    zone.cond == ZBC_ZONE_COND_FULL)

Actually, thinking about this test again, I think we should simplify it to:

	if (zone.cond == ZBC_ZONE_COND_FULL)

since conventional zones always have the condition ZBC_ZONE_COND_NO_WP, checking
the zone type is useless.

With that changed, feel free to add:

Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>

>  		zone.wp = zone.start + zone.len;
> +	else
> +		zone.wp = logical_to_sectors(sdp, get_unaligned_be64(&buf[24]));
>  
>  	ret = cb(&zone, idx, data);
>  	if (ret)
> 



-- 
Damien Le Moal
Western Digital Research