Re: [PATCH v5 2/2] ata: pata_artop: use *switch* in atp8xx_fixup()

[Sergey Shtylyov <s.shtylyov@xxxxxx>]

On 2/10/22 2:29 AM, Damien Le Moal wrote:

>> This driver uses a string of the *if* statements in atp8xx_fixup() where
>> a *switch* statement would fit better...
>>
>> Signed-off-by: Sergey Shtylyov <s.shtylyov@xxxxxx>
>> ---
>>
>> Changes in version 5:
>> - fixed up #define DRV_VERSION;
>> - added "ata: " prefix to the patch subject.
>>
>> Changes in version 4:
>> - new patch.
>>
>>  drivers/ata/pata_artop.c | 14 +++++++++-----
>>  1 file changed, 9 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/ata/pata_artop.c b/drivers/ata/pata_artop.c
>> index 08e88403d0ab..20a8f31a3f57 100644
>> --- a/drivers/ata/pata_artop.c
>> +++ b/drivers/ata/pata_artop.c
>> @@ -28,7 +28,7 @@
>>  #include <linux/ata.h>
>>  
>>  #define DRV_NAME	"pata_artop"
>> -#define DRV_VERSION	"0.4.7"
>> +#define DRV_VERSION	"0.4.8"
>>  
>>  /*
>>   *	The ARTOP has 33 Mhz and "over clocked" timing tables. Until we
>> @@ -315,12 +315,15 @@ static struct ata_port_operations artop6260_ops = {
>>  
>>  static void atp8xx_fixup(struct pci_dev *pdev)
>>  {
>> -	if (pdev->device == 0x0005)
>> +	u8 reg;
>> +
>> +	switch (pdev->device) {
>> +	case 0x0005:
>>  		/* BIOS may have left us in UDMA, clear it before libata probe */
>>  		pci_write_config_byte(pdev, 0x54, 0);
>> -	else if (pdev->device == 0x0008 || pdev->device == 0x0009) {
>> -		u8 reg;
>> -
>> +		break;
>> +	case 0x0008:
>> +	case 0x0009:
>>  		/* Mac systems come up with some registers not set as we
>>  		   will need them */
>>  
>> @@ -338,6 +341,7 @@ static void atp8xx_fixup(struct pci_dev *pdev)
>>  		/* Enable IRQ output and burst mode */
>>  		pci_read_config_byte(pdev, 0x4a, &reg);
>>  		pci_write_config_byte(pdev, 0x4a, (reg & ~0x01) | 0x80);
>> +		break;
>>  	}
> 
> without a default case, isn't there a warning with make W=1

   No. I thought *default* makes sense only for *enum*s...

> or make C=1
> (sparse) ?

   Still no. :-P

[...]

MNR.Sergey