В сообщении от 24 августа 2010 16:49:51 автор Marek Vasut написал:
> > + ret = gpio_request(H1940_LATCH_BLUETOOTH_POWER, dev_name(&pdev-
>dev));
>
> This should contain the name of the GPIO, not dev_name I assume.
Ok
> > + if (ret) {
> > + gpio_free(S3C2410_GPH(1));
>
> What's this constant (the 1) here ? Maybe some sane #define wont hurt or
> comment around it.
GPH(1) specifies gpio bit, that's not magic constant.
> > +#ifndef __ASSEMBLY__
> > +#define H1940_LATCH ((void __force __iomem *)0xF8000000)
> > +#else
> > +#define H1940_LATCH 0xF8000000
> > +#endif
>
> Is the __ASSEMBLY__ really needed ? You can establish mapping when the
> kernel boots (looks like you're doing that already) but then use
> __raw_readX __raw_writeX to access that space instead of this stuff above.
Uh, I just moved this code from another file, not sure if __ASSEMBLY__ is
necessary, I'll remove it if it is not.
Thanks for review :)
Regards
Vasily
Attachment:
signature.asc
Description: This is a digitally signed message part.
