lkml.org 
[lkml]   [2008]   [Jan]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [PATCH 10/12] ide-floppy: remove atomic test_*bit macros
    Date
    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)


    \
     
     \ /
      Last update: 2008-01-14 22:47    [W:4.185 / U:0.128 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site