| Message ID | 20220209202535.7679-3-s.shtylyov@omp.ru |
|---|---|
| State | New |
| Headers | show |
| Series | Use *switch*es instead of *if*s in the Artop PATA driver | expand |
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@omp.ru> > --- > > 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, ®); > 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) ? > } >
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@omp.ru> >> --- >> >> 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, ®); >> 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
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, ®); pci_write_config_byte(pdev, 0x4a, (reg & ~0x01) | 0x80); + break; } }
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@omp.ru> --- 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(-)