lkml.org 
[lkml]   [2009]   [Dec]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: GPIO support for HTC Dream
    Hi!

    > > +int gpio_to_irq(unsigned gpio)
    > > +{
    > > + return -EINVAL;
    > > +}
    >
    > This should probably just be an inline function in
    > arch/arm/mach-msm/include/mach/gpio.h

    Well, it is not performance critical in any way and it is likely to
    change in future. I'd leave it here.

    > > diff --git a/arch/arm/mach-msm/board-dream.h b/arch/arm/mach-msm/board-dream.h
    > > index 4f345a5..dbd78b9 100644
    > > --- a/arch/arm/mach-msm/board-dream.h
    > > +++ b/arch/arm/mach-msm/board-dream.h
    > > @@ -1,5 +1,58 @@
    > >
    > > -#define TROUT_CPLD_BASE 0xE8100000
    > > -#define TROUT_CPLD_START 0x98000000
    > > -#define TROUT_CPLD_SIZE SZ_4K
    > > +#define MSM_SMI_BASE 0x00000000
    > > +#define MSM_SMI_SIZE 0x00800000
    ...
    > > +#define DREAM_CPLD_BASE 0xE8100000
    > > +#define DREAM_CPLD_START 0x98000000
    > > +#define DREAM_CPLD_SIZE SZ_4K
    > > +
    >
    > This header might need to be a separate patch. The only thing in it
    > related to the rest of this is DREAM_CPLD_BASE.

    Yep.

    > > +#ifndef __ASM_ARCH_MSM_GPIO_H
    > > +#define __ASM_ARCH_MSM_GPIO_H
    > > +
    > > +extern int gpio_to_irq(unsigned gpio);
    >
    > This should probably be an inline as mentioned above.
    >
    > For completeness you should probably also add:
    >
    > static inline int irq_to_gpio(unsigned irq)
    > {
    > return -EINVAL;
    > }

    I'd say that would be overdoing it.

    > And, nitpick, move both of them after the gpio_cansleep below.

    I'll do the move.

    > > +#define DREAM_GPIO_CABLE_IN1 (83)
    > > +#define DREAM_GPIO_CABLE_IN2 (49)
    > > +
    > > +#define DREAM_GPIO_START (128)
    >
    > Nitpick. Tab align these three with the ones below.

    Ok.

    > > +#define DREAM_GPIO_SDMC_CD_N (DREAM_GPIO_VIRTUAL_BASE + 0)
    > > +#define DREAM_GPIO_END (DREAM_GPIO_SDMC_CD_N)
    > > +#define DREAM_GPIO_BANK1_FIRST_INT_SOURCE (DREAM_GPIO_SDMC_CD_N)
    > > +#define DREAM_GPIO_BANK1_LAST_INT_SOURCE (DREAM_GPIO_SDMC_CD_N)
    > > +
    > > +#endif
    >
    > Otherwise, looks good to me. Just test it to make sure it works :-).
    >
    > Since I have no way of compiling or testing this...
    >
    > Reviewed-by: H Hartley Sweeten <hsweeten@visionengravers.com>

    I believe inlining that function would be bad change. Can I still use
    reviewed-by tag?
    Pavel

    --
    (english) http://www.livejournal.com/~pavelmachek
    (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


    \
     
     \ /
      Last update: 2009-12-15 20:49    [W:3.252 / U:0.232 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site