Messages in this thread | | | From | Bartlomiej Zolnierkiewicz <> | Subject | Re: [PATCH 10/12] ide-floppy: remove atomic test_*bit macros | Date | Mon, 14 Jan 2008 22:50:58 +0100 |
| |
On Sunday 13 January 2008, Borislav Petkov wrote: > ..and replace them with flag enums. > > Signed-off-by: Borislav Petkov <bbpetkov@yahoo.de> > --- > drivers/ide/ide-floppy.c | 132 +++++++++++++++++++++++++-------------------- > 1 files changed, 73 insertions(+), 59 deletions(-)
[...]
> @@ -506,14 +516,14 @@ static ide_startstop_t idefloppy_pc_intr(ide_drive_t *drive) > > debug_log("Reached %s interrupt handler\n", __FUNCTION__); > > - if (test_bit(PC_DMA_IN_PROGRESS, &pc->flags)) { > + if (PC_FLAG_DMA_IN_PROGRESS & pc->flags) {
the usual kernel convention is to put flag last, i.e.
pc->flags & PC_FLAG_DMA_IN_PROGRESS
[...]
> @@ -570,7 +581,7 @@ static ide_startstop_t idefloppy_pc_intr(ide_drive_t *drive) > printk(KERN_ERR "ide-floppy: CoD != 0 in %s\n", __FUNCTION__); > return ide_do_reset(drive); > } > - if (((ireason & IO) == IO) == test_bit(PC_WRITING, &pc->flags)) { > + if (((ireason & IO) == IO) == (PC_FLAG_WRITING & pc->flags)) {
- test_bit() returns 1 or 0 (=> boolean) - (pc->flags & PC_FLAG_WRITING) is 0x10 or 0
so the above comparison will fail
> @@ -607,7 +618,7 @@ static ide_startstop_t idefloppy_pc_intr(ide_drive_t *drive) > xferfunc(drive, pc->current_position, bcount); > else > ide_floppy_io_buffers(drive, pc, bcount, > - test_bit(PC_WRITING, &pc->flags)); > + (PC_FLAG_WRITING & pc->flags));
ditto, this may actually work but '(pc->flags & PC_FLAG_WRITING) ? 1 : 0' would be much safer from maintainability POV
[...]
> @@ -1720,13 +1731,16 @@ static int idefloppy_media_changed(struct gendisk *disk) > { > struct ide_floppy_obj *floppy = ide_floppy_g(disk); > ide_drive_t *drive = floppy->drive; > + int ret; > > /* do not scan partitions twice if this is a removable device */ > if (drive->attach) { > drive->attach = 0; > return 0; > } > - return test_and_clear_bit(IDEFLOPPY_MEDIA_CHANGED, &floppy->flags); > + ret = IDEFLOPPY_FLAG_MEDIA_CHANGED & floppy->flags; > + floppy->flags &= ~IDEFLOPPY_FLAG_MEDIA_CHANGED; > + return ret; > }
same issue
otherwise looks good, please fix/resubmit (together with patch #12)
| |