Hello!
On 2/17/22 3:22 AM, Damien Le Moal 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.
I have initially converted that one too but finally decided to keep
the order of the comparisons intact -- it makes more sense to 1st check
dev->devno in the last *if*...
>
>> /* determine if device is ATA or ATAPI */
>
> This comment is a bit weird since ATA_DEV_ATAPI is not used. Maybe
Why? A call ata_port_classify() should detect the ATAPI devices,
we just don't "post-process" that result...
> change that to something like:
>
> /* Check the device class */
No, I don't agree here. :-)
>
> 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 :)
OK. :-)
[...]
MBR, Sergey
