lkml.org 
[lkml]   [2010]   [Jun]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH] Staging: d53155_drv.c: cleanup fbuffer usage
    From
    Date
    On Thu, 2010-06-24 at 09:31 -0700, H Hartley Sweeten wrote:
    > The global symbol dt3155_fbuffer[], declared in dt3155_isr.c, is really
    > just a pointer to dt3155_status[].fbuffer. To improve readability, make
    > some of the really long lines shorter, and make the buffer access more
    > consistent, use &dt3155_status[].fbuffer to access the buffer structure.

    I think this would be more consistent/intelligible
    if the temporary wasn't
    struct dt3155_fbuffer *fb = &dt3155_status[minor].fbuffer;
    but was
    struct dt3155_status *dts = &dt3155_status[minor];

    > @@ -146,11 +148,11 @@ static void quick_stop (int minor)
    > dt3155_status[minor].state &= ~(DT3155_STATE_STOP|0xff);
    > /* mark the system stopped: */
    > dt3155_status[minor].state |= DT3155_STATE_IDLE;
    > - dt3155_fbuffer[minor]->stop_acquire = 0;
    > - dt3155_fbuffer[minor]->even_stopped = 0;
    > + fb->stop_acquire = 0;
    > + fb->even_stopped = 0;
    > #else
    > dt3155_status[minor].state |= DT3155_STATE_STOP;
    > - dt3155_status[minor].fbuffer.stop_acquire = 1;
    > + fb->stop_acquire = 1;
    > #endif

    becomes

    dts->state &= ~(DT3155_STATE_STOP|0xff);
    /* mark the system stopped: */
    dts->state |= DT3155_STATE_IDLE;
    dts->fbuffer.stop_acquire = 0;
    dts->fbuffer.even_stopped = 0;
    #else
    dts->fbuffer.stop_acquire = 1;
    #endif

    Another option might be to use dereferencing inlines.

    static inline struct dt3155_status *dts(int index)
    {
    return &dt3155_status[index];
    }
    static inline struct dt3155_fbuffer *fb(int index)
    {
    return &(dts(index)->fbuffer);
    }

    I think this isn't as good as the temporary because the
    compiler sometimes doesn't reuse the intermediate addresses
    and the inlines have file rather than function scope.

    It becomes something like:

    dts(minor)->state &= ~(DT3155_STATE_STOP|0xff);
    /* mark the system stopped: */
    dts(minor)->state |= DT3155_STATE_IDLE;
    fb(minor)->stop_acquire = 0;
    fb(minor)->even_stopped = 0;
    #else
    fb(minor)->stop_acquire = 1;
    #endif

    I think this isn't as good as the temporary because the
    compiler sometimes doesn't reuse the intermediate addresses
    and the macros have file rather than function scope.




    \
     
     \ /
      Last update: 2010-06-24 19:39    [W:0.022 / U:0.468 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site