[PATCH v2 3/3] ARM: Samsung: Rework platform data of s3c-fb driver

[Jingoo Han]

> -----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 <jg1.han at samsung.com> wrote:
> 
> >> > 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.
> 
> 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.

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.


> 
> > '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.
> >