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

[Damien Le Moal <damien.lemoal@xxxxxxxxxxxxxxxxxx>]

On 2/10/22 05:25, Sergey Shtylyov 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 or make C=1
(sparse) ?


>  }
>  


-- 
Damien Le Moal
Western Digital Research