-----Original Message-----
From: Darius Augulis [mailto:augulis.darius at gmail.com]
Sent: Thursday, March 15, 2012 5:27 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; kgene.kim at samsung.com; ben-
linux at fluff.org; patches at linaro.org; Kyungmin Park; JeongHyeon Kim; Heiko Stuebner; Kwangwoo Lee; Mark
Brown; Peter Korsgaard; Maurus Cuelenaere
Subject: Re: [PATCH v2 3/3] ARM: Samsung: Rework platform data of s3c-fb driver

On Thu, Mar 15, 2012 at 10:10 AM, Jingoo Han [off-list ref] wrote:

quoted

quoted

There is single board - mini6410 (or real6410) and it's name doesn't
depend on connected LCD size.
We know, that this board is available with different sizes of LCD and
currently we have in kernel support for both sizes.
It might be so, that it's implemented not in perfect way, but it was
accepted and at least it's working.

I don't think so.
You argues that the wrong code should not be removed because it was accepted and at least it's working.
It is just wrong usage, which can just work.
Moreover, your code will make the problem, when 2~5 windows of FIMD IP are used.

Could please explain that? How does LCD feature selection for mini6410
and real6410 break that?
Timing for different sizes of LCD is located in two statically
allocated structures which are used at board init time depending on
kernel param line.
Data from one of these structures is copied to s3c-fb platform data
dynamically by board init code. Why it's a problem? What changes if
platform data isn't rewritten dynamically?

quoted

So, your code can be removed.

I don't think so. It does not break anything yet. One who make
changes, should ensure backwards compatibility, at least talking about
functionality and hardware (LCD) support, which was added few months
ago. Remember, we talk about kernel parameters line - now it lets
bootloader to select correct LCD size. After your changes, board with
7" LCD wont show anything.

[CC'ed Tushar Behera]

Yes, your code is working. However, your code is bug.
'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.
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
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
problem with your code. Why we should keep aberration such as your code? If you want to select two LCDs, please find other way.

Darius A.