On Sunday 13 January 2008, Borislav Petkov wrote:
> ..and replace them with flag enums.
> 
> Signed-off-by: Borislav Petkov <[EMAIL PROTECTED]>
> ---
>  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)
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to