Re: [PATCH 10/12] ide-floppy: remove atomic test_*bit macros

Bartlomiej Zolnierkiewicz
2008-01-14T21:44:53Z

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)