[PATCH 1/5] Add HP iPAQ RX1950 machine support
Vasily Khoruzhick
anarsoul at gmail.com
Tue May 11 02:50:22 EDT 2010
В сообщении от 11 мая 2010 03:15:42 автор Ben Dooks написал:
>
> firstly, do you need this mapping for anything and secondly can
> you check the indentation on it if it is staying.
Ok, I don't need them.
> > +static struct s3c2410fb_display rx1950_display __initdata = {
>
> don't think this is allowed to be __initdata at the moment.
Ok
> > + /* Wait a bit here... */
> > + mdelay(100);
>
> would msleep() be a better thing to do herer.
Nope, I prefer to use mdelay here, msleep doesn't do well for
some reason (there's a garbage on screen in case of msleep usage).
And actually wait for hsync/vsync should replace
this mdelay, but it isn't implemented at the moment.
>
> i think S3C_GPIO_SFN(2) would do for all of these, and would allow
> you to loop over the range.
Ok
>
> at-least get a space between { and KEY_
> would prefer to see properly named initialisers.
Ok
> > + s3c_irq_wake(IRQ_EINT0, 1);
> > +}
>
> surely this should be propertyu of a key in the keys table?
Ok
> > + gpio_request(S3C2410_GPJ(1), "MMC power");
> > + gpio_direction_output(S3C2410_GPJ(1), 0);
>
> you might want to WARN_ON() the result of gpio_request().
Ok
Thanks for review.
Regards
Vasily
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20100511/75e3d1ce/attachment.sig>
More information about the linux-arm-kernel
mailing list