Dne Út 24. srpna 2010 16:04:00 Vasily Khoruzhick napsal(a):
> В сообщении от 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.
Defining S3C2410_GPH_GPIONAME for each bit might be better then.
>
> > > +#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.
Well this isn't included in any assembly file. So it's not. Also, I'd be happier
if you could fix it the way I outlined.
Check for example mach-pxa/balloon3.c and how NAND access is done there. Cheers
>
> Thanks for review :)
You're welcome.
>
> Regards
> Vasily
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
