On 2/17/22 06:13, Sergey Shtylyov wrote:
> In ata_sff_dev_classify(), replace a string of the *if* statements checking
> the device's class with the *switch* statement that fits better here...
>
> Signed-off-by: Sergey Shtylyov <s.shtylyov@xxxxxx>
>
> ---
> This patch is against the 'for-next' branch of Damien Le Moal's 'libata.git'
> repo.
>
> drivers/ata/libata-sff.c | 14 ++++++++------
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> Index: libata/drivers/ata/libata-sff.c
> ===================================================================
> --- libata.orig/drivers/ata/libata-sff.c
> +++ libata/drivers/ata/libata-sff.c
> @@ -1841,8 +1841,8 @@ unsigned int ata_sff_dev_classify(struct
>
The err check before the hunk below could use a switch too.
> /* determine if device is ATA or ATAPI */
This comment is a bit weird since ATA_DEV_ATAPI is not used. Maybe
change that to something like:
/* Check the device class */
Or just drop it... The code is clear enough I think.
> class = ata_port_classify(ap, &tf);
> -
> - if (class == ATA_DEV_UNKNOWN) {
> + switch (class) {
> + case ATA_DEV_UNKNOWN:
> /* If the device failed diagnostic, it's likely to
While at it, please correct the comment style here (start with a "/*"
line). There are a ton of these style problems all over, so when
touching code around them, let's fix them :)
> * have reported incorrect device signature too.
> * Assume ATA device if the device seems present but
> @@ -1853,10 +1853,12 @@ unsigned int ata_sff_dev_classify(struct
> class = ATA_DEV_ATA;
> else
> class = ATA_DEV_NONE;
> - } else if ((class == ATA_DEV_ATA) &&
> - (ap->ops->sff_check_status(ap) == 0))
> - class = ATA_DEV_NONE;
> -
> + break;
> + case ATA_DEV_ATA:
> + if (ap->ops->sff_check_status(ap) == 0)
> + class = ATA_DEV_NONE;
> + break;
> + }
> return class;
> }
> EXPORT_SYMBOL_GPL(ata_sff_dev_classify);
--
Damien Le Moal
Western Digital Research
