| Message ID | 99d91d0e-d3fc-5492-ddca-6da6e65e1e89@omp.ru |
|---|---|
| State | New |
| Headers | show |
| Series | ata: pata_legacy: fix pdc20230_set_piomode() | expand |
Hello! On 10/29/22 12:07 AM, Sergey Shtylyov wrote: > Clang gives a warning when compiling pata_legacy.c with 'make W=1' about > the 'rt' local variable in pdc20230_set_piomode() being set but unused. > Quite obviously, there is an outb() call missing to write back the updated > variable. Moreover, checking the docs by Petr Soucek revealed that bitwise > AND should have been done with a negated timing mask and the master/slave > timing masks were swapped while updating... > > Fixes: 669a5db411d8 ("[libata] Add a bunch of PATA drivers.") > Reported-by: Damien Le Moal <damien.lemoal@opensource.wdc.com> > Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru> > > --- Forgot to mention that the patch was done against the 'master' branch of libata.git... [...] MBR, Sergey
On 10/29/22 06:07, Sergey Shtylyov wrote: > Clang gives a warning when compiling pata_legacy.c with 'make W=1' about > the 'rt' local variable in pdc20230_set_piomode() being set but unused. > Quite obviously, there is an outb() call missing to write back the updated > variable. Moreover, checking the docs by Petr Soucek revealed that bitwise > AND should have been done with a negated timing mask and the master/slave > timing masks were swapped while updating... > > Fixes: 669a5db411d8 ("[libata] Add a bunch of PATA drivers.") > Reported-by: Damien Le Moal <damien.lemoal@opensource.wdc.com> > Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru> Applied to for-6.1-fixes. Thanks ! > > --- > drivers/ata/pata_legacy.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > Index: libata/drivers/ata/pata_legacy.c > =================================================================== > --- libata.orig/drivers/ata/pata_legacy.c > +++ libata/drivers/ata/pata_legacy.c > @@ -315,9 +315,10 @@ static void pdc20230_set_piomode(struct > outb(inb(0x1F4) & 0x07, 0x1F4); > > rt = inb(0x1F3); > - rt &= 0x07 << (3 * adev->devno); > + rt &= ~(0x07 << (3 * !adev->devno)); > if (pio) > - rt |= (1 + 3 * pio) << (3 * adev->devno); > + rt |= (1 + 3 * pio) << (3 * !adev->devno); > + outb(rt, 0x1F3); > > udelay(100); > outb(inb(0x1F2) | 0x01, 0x1F2);
Index: libata/drivers/ata/pata_legacy.c =================================================================== --- libata.orig/drivers/ata/pata_legacy.c +++ libata/drivers/ata/pata_legacy.c @@ -315,9 +315,10 @@ static void pdc20230_set_piomode(struct outb(inb(0x1F4) & 0x07, 0x1F4); rt = inb(0x1F3); - rt &= 0x07 << (3 * adev->devno); + rt &= ~(0x07 << (3 * !adev->devno)); if (pio) - rt |= (1 + 3 * pio) << (3 * adev->devno); + rt |= (1 + 3 * pio) << (3 * !adev->devno); + outb(rt, 0x1F3); udelay(100); outb(inb(0x1F2) | 0x01, 0x1F2);
Clang gives a warning when compiling pata_legacy.c with 'make W=1' about the 'rt' local variable in pdc20230_set_piomode() being set but unused. Quite obviously, there is an outb() call missing to write back the updated variable. Moreover, checking the docs by Petr Soucek revealed that bitwise AND should have been done with a negated timing mask and the master/slave timing masks were swapped while updating... Fixes: 669a5db411d8 ("[libata] Add a bunch of PATA drivers.") Reported-by: Damien Le Moal <damien.lemoal@opensource.wdc.com> Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru> --- drivers/ata/pata_legacy.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)