Kyungmin Park wrote:
>
> On Wed, Jul 14, 2010 at 4:30 PM, Marek Szyprowski
> <m.szyprowski@xxxxxxxxxxx> wrote:
> > Hello,
> >
> > On Wednesday, July 14, 2010 8:48 AM Kukjin Kim wrote:
> >
> >> Marek Szyprowski wrote:
> >> >
> >> > This patch adds required platform definitions to enable s3c-fb
> >> > driver on GONI board. One framebuffer window in 480x800x16bpp mode is
> >> > defined.
> >> >
> >> > Signed-off-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>
> >> > Signed-off-by: Kyungmin Park <kyungmin.park@xxxxxxxxxxx>
> >> > ---
> >> > arch/arm/mach-s5pv210/Kconfig | 2 +
> >> > arch/arm/mach-s5pv210/mach-goni.c | 39
> >> > +++++++++++++++++++++++++++++++++++++
> >> > 2 files changed, 41 insertions(+), 0 deletions(-)
> >> >
> >> > diff --git a/arch/arm/mach-s5pv210/Kconfig b/arch/arm/mach-
> >> s5pv210/Kconfig
> >> > index 5e88941..8ab4bb0 100644
> >> > --- a/arch/arm/mach-s5pv210/Kconfig
> >> > +++ b/arch/arm/mach-s5pv210/Kconfig
> >> > @@ -59,6 +59,8 @@ config MACH_GONI
> >> > bool "GONI"
> >> > select CPU_S5PV210
> >> > select ARCH_SPARSEMEM_ENABLE
> >> > + select S5PV210_SETUP_FB_24BPP
> >> > + select S3C_DEV_FB
> >> > select S5PC110_DEV_ONENAND
> >> > help
> >> > Machine support for Samsung GONI board
> >> > diff --git a/arch/arm/mach-s5pv210/mach-goni.c
> >> b/arch/arm/mach-s5pv210/mach-
> >> > goni.c
> >> > index 88c38e3..05b4a1a 100644
> >> > --- a/arch/arm/mach-s5pv210/mach-goni.c
> >> > +++ b/arch/arm/mach-s5pv210/mach-goni.c
> >> > @@ -12,6 +12,9 @@
> >> > #include <linux/types.h>
> >> > #include <linux/init.h>
> >> > #include <linux/serial_core.h>
> >> > +#include <linux/fb.h>
> >> > +#include <linux/delay.h>
> >>
> >> need linux/delay.h in here?
> >>
> >> > +#include <linux/clk.h>
> >>
> >> same...need linux/clk.h?
> >>
> >> >
> >> > #include <asm/mach/arch.h>
> >> > #include <asm/mach/map.h>
> >> > @@ -20,11 +23,15 @@
> >> >
> >> > #include <mach/map.h>
> >> > #include <mach/regs-clock.h>
> >> > +#include <mach/regs-fb.h>
> >> > +#include <mach/gpio.h>
> >>
> >> linux/gpio.h
> >
> > Ok.
> >
> >> >
> >> > +#include <plat/gpio-cfg.h>
> >> > #include <plat/regs-serial.h>
> >> > #include <plat/s5pv210.h>
> >> > #include <plat/devs.h>
> >> > #include <plat/cpu.h>
> >> > +#include <plat/fb.h>
> >> >
> >> > /* Following are default values for UCON, ULCON and UFCON UART
> registers
> >> */
> >> > #define S5PV210_UCON_DEFAULT (S3C2410_UCON_TXILEVEL
> | \
> >> > @@ -73,7 +80,35 @@ static struct s3c2410_uartcfg goni_uartcfgs[]
> >> __initdata = {
> >> > },
> >> > };
> >> >
> >> > +/* Frame Buffer */
> >>
> >> No need this comment because we know _fb_ means frame buffer...
> >
> > Comments in the source code are imho always welcome. As you know
mach-*.c
> files
> > grows to very large sizes and it is much easier to read them if all
definitions
> > and items are grouped and commented with a header on top of them (with
such
> > comments you easily can notice where one group starts and ends without
> reading
> > the code).
> >
> >> > +static struct s3c_fb_pd_win goni_fb_win0 = {
> >>
> >> How about to use goni_fb_win[] array so that can be extended easily...
> >
> > I've just followed the style used in the other mach-*.c files. No
problem to
> > change it.
> >
> >>
> >> > + .win_mode = {
> >> > + .pixclock = 1000000000000ULL /
> >> > ((16+16+2+480)*(28+3+2+800)*55),
>
> Marugu(?) send the patch remove pixclock at fb mailing list. So If
Yeah, I saw Maurus' patch, [PATCH v2] s3c-fb: Automatically calculate pixel
clock when none is given.
> >> > + .left_margin = 16,
> >> > + .right_margin = 16,
> >> > + .upper_margin = 3,
> >> > + .lower_margin = 28,
> >> > + .hsync_len = 2,
> >> > + .vsync_len = 2,
> >> > + .xres = 480,
> >> > + .yres = 800,
> >> > + .refresh = 55,
> >> > + },
> >> > + .max_bpp = 32,
> >> > + .default_bpp = 16,
> >>
> >> If possible, please keep the align like below...for easily reading...
> >> But...it depends on your taste...:-)
> >
> > Ok, no problem with this.
> >
> >>
> >> + .win_mode = {
> >> + .pixclock = 1000000000000ULL /
> >> ((16+16+2+480)*(28+3+2+800)*55),
>
> Maurus send the patch remove pixclock calculation so I hope his patch
> merge and we just remove it at here.
But as far as I know, not applied yet.
So I think, need .pixclock member now..and maybe can be removed later.
>
> Thank you,
> Kyungmin Park
(snip)
Thanks.
Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@xxxxxxxxxxx>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.
--
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
