[PATCH 1/4] pxafb: introduce lcd_{read,write}l() to wrap the __raw_{read,write}l() - Kernel

This is a discussion on [PATCH 1/4] pxafb: introduce lcd_{read,write}l() to wrap the __raw_{read,write}l() - Kernel ; From 917ed27ddcaca455f7799f74a1db9a4497725de8 Mon Sep 17 00:00:00 2001 From: Eric Miao Date: Thu, 27 Mar 2008 09:28:32 +0800 Subject: [PATCH] pxafb: introduce lcd_{read,write}l() to wrap the __raw_{read,write}l() using __raw_{read,write}l() everywhere looks messy, introduce lcd_{read,write}l() to get this cleaned up a bit ...

+ Reply to Thread
Results 1 to 3 of 3

Thread: [PATCH 1/4] pxafb: introduce lcd_{read,write}l() to wrap the __raw_{read,write}l()

  1. [PATCH 1/4] pxafb: introduce lcd_{read,write}l() to wrap the __raw_{read,write}l()

    From 917ed27ddcaca455f7799f74a1db9a4497725de8 Mon Sep 17 00:00:00 2001
    From: Eric Miao
    Date: Thu, 27 Mar 2008 09:28:32 +0800
    Subject: [PATCH] pxafb: introduce lcd_{read,write}l() to wrap the
    __raw_{read,write}l()

    using __raw_{read,write}l() everywhere looks messy, introduce
    lcd_{read,write}l() to get this cleaned up a bit

    Signed-off-by: eric miao
    ---
    drivers/video/pxafb.c | 50 +++++++++++++++++++++++++-----------------------
    1 files changed, 26 insertions(+), 24 deletions(-)

    diff --git a/drivers/video/pxafb.c b/drivers/video/pxafb.c
    index 0031a5f..27d9fed 100644
    --- a/drivers/video/pxafb.c
    +++ b/drivers/video/pxafb.c
    @@ -64,6 +64,9 @@
    #define LCCR3_INVALID_CONFIG_MASK (LCCR3_HSP | LCCR3_VSP |\
    LCCR3_PCD | LCCR3_BPP)

    +#define lcd_readl(f, off) __raw_readl((f)->mmio_base + (off))
    +#define lcd_writel(f, off, v) __raw_writel((v), (f)->mmio_base + (off))
    +
    static void (*pxafb_backlight_power)(int);
    static void (*pxafb_lcd_power)(int, struct fb_var_screeninfo *);

    @@ -684,8 +687,7 @@ static int pxafb_activate_var(struct fb_var_screeninfo *var,
    fbi->reg_lccr1 = new_regs.lccr1;
    fbi->reg_lccr2 = new_regs.lccr2;
    fbi->reg_lccr3 = new_regs.lccr3;
    - fbi->reg_lccr4 = __raw_readl(fbi->mmio_base + LCCR4) &
    - (~LCCR4_PAL_FOR_MASK);
    + fbi->reg_lccr4 = lcd_readl(fbi, LCCR4) & ~LCCR4_PAL_FOR_MASK;
    fbi->reg_lccr4 |= (fbi->lccr4 & LCCR4_PAL_FOR_MASK);
    set_hsync_time(fbi, pcd);
    local_irq_restore(flags);
    @@ -694,12 +696,12 @@ static int pxafb_activate_var(struct
    fb_var_screeninfo *var,
    * Only update the registers if the controller is enabled
    * and something has changed.
    */
    - if ((__raw_readl(fbi->mmio_base + LCCR0) != fbi->reg_lccr0) ||
    - (__raw_readl(fbi->mmio_base + LCCR1) != fbi->reg_lccr1) ||
    - (__raw_readl(fbi->mmio_base + LCCR2) != fbi->reg_lccr2) ||
    - (__raw_readl(fbi->mmio_base + LCCR3) != fbi->reg_lccr3) ||
    - (__raw_readl(fbi->mmio_base + FDADR0) != fbi->fdadr[0]) ||
    - (__raw_readl(fbi->mmio_base + FDADR1) != fbi->fdadr[1]))
    + if ((lcd_readl(fbi, LCCR0) != fbi->reg_lccr0) ||
    + (lcd_readl(fbi, LCCR1) != fbi->reg_lccr1) ||
    + (lcd_readl(fbi, LCCR2) != fbi->reg_lccr2) ||
    + (lcd_readl(fbi, LCCR3) != fbi->reg_lccr3) ||
    + (lcd_readl(fbi, FDADR0) != fbi->fdadr[0]) ||
    + (lcd_readl(fbi, FDADR1) != fbi->fdadr[1]))
    pxafb_schedule_work(fbi, C_REENABLE);

    return 0;
    @@ -785,14 +787,14 @@ static void pxafb_enable_controller(struct
    pxafb_info *fbi)
    clk_enable(fbi->clk);

    /* Sequence from 11.7.10 */
    - __raw_writel(fbi->reg_lccr3, fbi->mmio_base + LCCR3);
    - __raw_writel(fbi->reg_lccr2, fbi->mmio_base + LCCR2);
    - __raw_writel(fbi->reg_lccr1, fbi->mmio_base + LCCR1);
    - __raw_writel(fbi->reg_lccr0 & ~LCCR0_ENB, fbi->mmio_base + LCCR0);
    -
    - __raw_writel(fbi->fdadr[0], fbi->mmio_base + FDADR0);
    - __raw_writel(fbi->fdadr[1], fbi->mmio_base + FDADR1);
    - __raw_writel(fbi->reg_lccr0 | LCCR0_ENB, fbi->mmio_base + LCCR0);
    + lcd_writel(fbi, LCCR3, fbi->reg_lccr3);
    + lcd_writel(fbi, LCCR2, fbi->reg_lccr2);
    + lcd_writel(fbi, LCCR1, fbi->reg_lccr1);
    + lcd_writel(fbi, LCCR0, fbi->reg_lccr0 & ~LCCR0_ENB);
    +
    + lcd_writel(fbi, FDADR0, fbi->fdadr[0]);
    + lcd_writel(fbi, FDADR1, fbi->fdadr[1]);
    + lcd_writel(fbi, LCCR0, fbi->reg_lccr0 | LCCR0_ENB);
    }

    static void pxafb_disable_controller(struct pxafb_info *fbi)
    @@ -805,11 +807,11 @@ static void pxafb_disable_controller(struct
    pxafb_info *fbi)
    add_wait_queue(&fbi->ctrlr_wait, &wait);

    /* Clear LCD Status Register */
    - __raw_writel(0xffffffff, fbi->mmio_base + LCSR);
    + lcd_writel(fbi, LCSR, 0xffffffff);

    - lccr0 = __raw_readl(fbi->mmio_base + LCCR0) & ~LCCR0_LDM;
    - __raw_writel(lccr0, fbi->mmio_base + LCCR0);
    - __raw_writel(lccr0 | LCCR0_DIS, fbi->mmio_base + LCCR0);
    + lccr0 = lcd_readl(fbi, LCCR0) & ~LCCR0_LDM;
    + lcd_writel(fbi, LCCR0, lccr0);
    + lcd_writel(fbi, LCCR0, lccr0 | LCCR0_DIS);

    schedule_timeout(200 * HZ / 1000);
    remove_wait_queue(&fbi->ctrlr_wait, &wait);
    @@ -824,15 +826,15 @@ static void pxafb_disable_controller(struct
    pxafb_info *fbi)
    static irqreturn_t pxafb_handle_irq(int irq, void *dev_id)
    {
    struct pxafb_info *fbi = dev_id;
    - unsigned int lccr0, lcsr = __raw_readl(fbi->mmio_base + LCSR);
    + unsigned int lccr0, lcsr = lcd_readl(fbi, LCSR);

    if (lcsr & LCSR_LDD) {
    - lccr0 = __raw_readl(fbi->mmio_base + LCCR0) | LCCR0_LDM;
    - __raw_writel(lccr0, fbi->mmio_base + LCCR0);
    + lccr0 = lcd_readl(fbi, LCCR0);
    + lcd_writel(fbi, LCCR0, lccr0 | LCCR0_LDM);
    wake_up(&fbi->ctrlr_wait);
    }

    - __raw_writel(lcsr, fbi->mmio_base + LCSR);
    + lcd_writel(fbi, LCSR, lcsr);
    return IRQ_HANDLED;
    }

    --
    1.5.4.3



    --
    Cheers
    - eric
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  2. Re: [PATCH 1/4] pxafb: introduce lcd_{read,write}l() to wrap the __raw_{read,write}l()

    On Tue, 8 Apr 2008 12:03:16 +0800 "eric miao" wrote:

    > +#define lcd_readl(f, off) __raw_readl((f)->mmio_base + (off))
    > +#define lcd_writel(f, off, v) __raw_writel((v), (f)->mmio_base + (off))


    Please implement things like this in C. Probably inlined.

    Advantages:

    - C looks nicer

    - For some reason people are more likely to document their C than their macros

    - macros can sometimes reference their argument multiple times, causing
    bugs when they are passed experssions with side-effects.

    - C functions have typechecking

    - C functions (whether inlined or not) count as a reference to their
    argument, and can help to avoid unused-variable warnings.


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  3. Re: [PATCH 1/4] pxafb: introduce lcd_{read,write}l() to wrap the __raw_{read,write}l()

    Updated, sorry for the mailer I'm using (gmail web interface), I also
    attach the patch hereby:

    From f4d383f836fc0ccee2669836b0355b60a4d4e88a Mon Sep 17 00:00:00 2001
    From: Eric Miao
    Date: Thu, 27 Mar 2008 09:28:32 +0800
    Subject: [PATCH] pxafb: introduce lcd_{read,write}l() to wrap the
    __raw_{read,write}l()

    using __raw_{read,write}l() everywhere looks messy, introduce
    lcd_{read,write}l() to get this cleaned up a bit

    Signed-off-by: eric miao
    ---
    drivers/video/pxafb.c | 59 +++++++++++++++++++++++++++++--------------------
    1 files changed, 35 insertions(+), 24 deletions(-)

    diff --git a/drivers/video/pxafb.c b/drivers/video/pxafb.c
    index 0031a5f..4175617 100644
    --- a/drivers/video/pxafb.c
    +++ b/drivers/video/pxafb.c
    @@ -71,6 +71,18 @@ static int pxafb_activate_var(struct fb_var_screeninfo *var,
    struct pxafb_info *);
    static void set_ctrlr_state(struct pxafb_info *fbi, u_int state);

    +static inline unsigned long
    +lcd_readl(struct pxafb_info *fbi, unsigned int off)
    +{
    + return __raw_readl(fbi->mmio_base + off);
    +}
    +
    +static inline void
    +lcd_writel(struct pxafb_info *fbi, unsigned int off, unsigned long val)
    +{
    + __raw_writel(val, fbi->mmio_base + off);
    +}
    +
    static inline void pxafb_schedule_work(struct pxafb_info *fbi, u_int state)
    {
    unsigned long flags;
    @@ -684,8 +696,7 @@ static int pxafb_activate_var(struct fb_var_screeninfo *var,
    fbi->reg_lccr1 = new_regs.lccr1;
    fbi->reg_lccr2 = new_regs.lccr2;
    fbi->reg_lccr3 = new_regs.lccr3;
    - fbi->reg_lccr4 = __raw_readl(fbi->mmio_base + LCCR4) &
    - (~LCCR4_PAL_FOR_MASK);
    + fbi->reg_lccr4 = lcd_readl(fbi, LCCR4) & ~LCCR4_PAL_FOR_MASK;
    fbi->reg_lccr4 |= (fbi->lccr4 & LCCR4_PAL_FOR_MASK);
    set_hsync_time(fbi, pcd);
    local_irq_restore(flags);
    @@ -694,12 +705,12 @@ static int pxafb_activate_var(struct
    fb_var_screeninfo *var,
    * Only update the registers if the controller is enabled
    * and something has changed.
    */
    - if ((__raw_readl(fbi->mmio_base + LCCR0) != fbi->reg_lccr0) ||
    - (__raw_readl(fbi->mmio_base + LCCR1) != fbi->reg_lccr1) ||
    - (__raw_readl(fbi->mmio_base + LCCR2) != fbi->reg_lccr2) ||
    - (__raw_readl(fbi->mmio_base + LCCR3) != fbi->reg_lccr3) ||
    - (__raw_readl(fbi->mmio_base + FDADR0) != fbi->fdadr[0]) ||
    - (__raw_readl(fbi->mmio_base + FDADR1) != fbi->fdadr[1]))
    + if ((lcd_readl(fbi, LCCR0) != fbi->reg_lccr0) ||
    + (lcd_readl(fbi, LCCR1) != fbi->reg_lccr1) ||
    + (lcd_readl(fbi, LCCR2) != fbi->reg_lccr2) ||
    + (lcd_readl(fbi, LCCR3) != fbi->reg_lccr3) ||
    + (lcd_readl(fbi, FDADR0) != fbi->fdadr[0]) ||
    + (lcd_readl(fbi, FDADR1) != fbi->fdadr[1]))
    pxafb_schedule_work(fbi, C_REENABLE);

    return 0;
    @@ -785,14 +796,14 @@ static void pxafb_enable_controller(struct
    pxafb_info *fbi)
    clk_enable(fbi->clk);

    /* Sequence from 11.7.10 */
    - __raw_writel(fbi->reg_lccr3, fbi->mmio_base + LCCR3);
    - __raw_writel(fbi->reg_lccr2, fbi->mmio_base + LCCR2);
    - __raw_writel(fbi->reg_lccr1, fbi->mmio_base + LCCR1);
    - __raw_writel(fbi->reg_lccr0 & ~LCCR0_ENB, fbi->mmio_base + LCCR0);
    -
    - __raw_writel(fbi->fdadr[0], fbi->mmio_base + FDADR0);
    - __raw_writel(fbi->fdadr[1], fbi->mmio_base + FDADR1);
    - __raw_writel(fbi->reg_lccr0 | LCCR0_ENB, fbi->mmio_base + LCCR0);
    + lcd_writel(fbi, LCCR3, fbi->reg_lccr3);
    + lcd_writel(fbi, LCCR2, fbi->reg_lccr2);
    + lcd_writel(fbi, LCCR1, fbi->reg_lccr1);
    + lcd_writel(fbi, LCCR0, fbi->reg_lccr0 & ~LCCR0_ENB);
    +
    + lcd_writel(fbi, FDADR0, fbi->fdadr[0]);
    + lcd_writel(fbi, FDADR1, fbi->fdadr[1]);
    + lcd_writel(fbi, LCCR0, fbi->reg_lccr0 | LCCR0_ENB);
    }

    static void pxafb_disable_controller(struct pxafb_info *fbi)
    @@ -805,11 +816,11 @@ static void pxafb_disable_controller(struct
    pxafb_info *fbi)
    add_wait_queue(&fbi->ctrlr_wait, &wait);

    /* Clear LCD Status Register */
    - __raw_writel(0xffffffff, fbi->mmio_base + LCSR);
    + lcd_writel(fbi, LCSR, 0xffffffff);

    - lccr0 = __raw_readl(fbi->mmio_base + LCCR0) & ~LCCR0_LDM;
    - __raw_writel(lccr0, fbi->mmio_base + LCCR0);
    - __raw_writel(lccr0 | LCCR0_DIS, fbi->mmio_base + LCCR0);
    + lccr0 = lcd_readl(fbi, LCCR0) & ~LCCR0_LDM;
    + lcd_writel(fbi, LCCR0, lccr0);
    + lcd_writel(fbi, LCCR0, lccr0 | LCCR0_DIS);

    schedule_timeout(200 * HZ / 1000);
    remove_wait_queue(&fbi->ctrlr_wait, &wait);
    @@ -824,15 +835,15 @@ static void pxafb_disable_controller(struct
    pxafb_info *fbi)
    static irqreturn_t pxafb_handle_irq(int irq, void *dev_id)
    {
    struct pxafb_info *fbi = dev_id;
    - unsigned int lccr0, lcsr = __raw_readl(fbi->mmio_base + LCSR);
    + unsigned int lccr0, lcsr = lcd_readl(fbi, LCSR);

    if (lcsr & LCSR_LDD) {
    - lccr0 = __raw_readl(fbi->mmio_base + LCCR0) | LCCR0_LDM;
    - __raw_writel(lccr0, fbi->mmio_base + LCCR0);
    + lccr0 = lcd_readl(fbi, LCCR0);
    + lcd_writel(fbi, LCCR0, lccr0 | LCCR0_LDM);
    wake_up(&fbi->ctrlr_wait);
    }

    - __raw_writel(lcsr, fbi->mmio_base + LCSR);
    + lcd_writel(fbi, LCSR, lcsr);
    return IRQ_HANDLED;
    }

    --
    1.5.4.3



    On Tue, Apr 8, 2008 at 4:20 PM, Andrew Morton wrote:
    > On Tue, 8 Apr 2008 12:03:16 +0800 "eric miao" wrote:
    >
    > > +#define lcd_readl(f, off) __raw_readl((f)->mmio_base + (off))
    > > +#define lcd_writel(f, off, v) __raw_writel((v), (f)->mmio_base + (off))

    >
    > Please implement things like this in C. Probably inlined.
    >
    > Advantages:
    >
    > - C looks nicer
    >
    > - For some reason people are more likely to document their C than their macros
    >
    > - macros can sometimes reference their argument multiple times, causing
    > bugs when they are passed experssions with side-effects.
    >
    > - C functions have typechecking
    >
    > - C functions (whether inlined or not) count as a reference to their
    > argument, and can help to avoid unused-variable warnings.
    >
    >
    >




    --
    Cheers
    - eric


+ Reply to Thread