В сообщении от 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.

Reply via email to