On Fri, Mar 16, 2012 at 10:07 AM, Jingoo Han [off-list ref] wrote:

quoted

-----Original Message-----
From: Darius Augulis [mailto:augulis.darius at gmail.com]
Sent: Friday, March 16, 2012 4:47 PM
To: Jingoo Han
Cc: Thomas Abraham; linux-fbdev at vger.kernel.org; FlorianSchandinat at gmx.de; linux-arm-
kernel at lists.infradead.org; linux-samsung-soc at vger.kernel.org; ben-linux at fluff.org; patches at linaro.org;
kgene.kim at samsung.com; Kyungmin Park; JeongHyeon Kim; Heiko Stuebner; Kwangwoo Lee; Mark Brown; Peter
Korsgaard; Maurus Cuelenaere; Tushar Behera
Subject: Re: [PATCH v2 3/3] ARM: Samsung: Rework platform data of s3c-fb driver

On Fri, Mar 16, 2012 at 3:39 AM, Jingoo Han [off-list ref] wrote:

it's not a bug, because it's working like it was intended to. it was
reviewed and accepted by maintainers before merging to main line.
you do not have rights to drop it because you don't like it.
You are doing changes - please do it correctly and do not break
existing functionality.
btw, you still not answered my question: why these two s3c_fb_pd_win
structures in mach-mini6410.c is a problem? They are declared on the
stack, but platform data uses only single one. mini6410 rewrites its
s3c-fb platform data with data from one of these two structures
dynamically. Why it's a bug? You want to remove this dynamic selection
which could be leaved there with reworked framework too.

It's just bug.

struct s3c_fb_pd_win should be used for windows of FIMD IP.
It should not be used for selecting multi-LCDs.

why not? s3c_fb_pd_win contains timing values which depend directly on
LCD size and resolution.
Please look into the code again - mini6410 doesn't use platform data
so select different LCD sizes.
It has only single window in platform data - exactly what you want.
Now tell my, why you are arguing, that this platform data cannot be
rewritten dynamically by board setup routines at kernel startup time?
Many ARM architectures are doing the same - takes kernel command line
argument to select LCD and passes necessary data for its fb drivers
via platform data structures. It not a bug, it's correct approach to
support different sizes of LCD display for the same board.
You are simply lazy to deal with little bit different code of mini6410
and real6410 because they have support for multiple LCDs and other
s3c-fb boards doesn't.
This is a reason why you want to drop it and not because it's a bug.
Hope other maintainers and code reviewers understand this situation correctly.

struct s3c_fb_pd_win should include information of windows of FIMD IP, s3c_fb_pd_win[0], s3c_fb_pd_win[1] represents window 0,
window 1 of FIMD IP. However, your code includes information of LCD, it means that struct s3c_fb_pd_win represents 4.3" LCD, 7.0"
LCD. It is wrong usage.

quoted

quoted

'struct s3c_fb_pd_win' is used not for LCD, but for windows of FIMD IP. So, 'struct s3c_fb_pd_win'

should not be used to select LCD.

quoted

The mach-aquila.c uses two 'struct s3c_fb_pd_win' like you, however 'struct s3c_fb_pd_win' don't make

problem with Thomas's

quoted

patchset. It is because mach-aquila.c use 'struct s3c_fb_pd_win' for only windows of FIMD IP.
We don't have to responsibility to keep wrong code. If you have kept the rule, Thomas's patch would

not make the compatibility

quoted

problem with your code. Why we should keep aberration such as your code? If you want to select two

LCDs, please find other way.

quoted

quoted

Darius A.