RE: [PATCH v2 3/3] ARM: Samsung: Rework platform data of s3c-fb driver
From: Jingoo Han <hidden>
Date: 2012-03-16 08:07:03
Also in:
linux-arm-kernel, linux-samsung-soc
-----Original Message----- From: Darius Augulis [mailto:augulis.darius@gmail.com] Sent: Friday, March 16, 2012 4:47 PM To: Jingoo Han Cc: Thomas Abraham; linux-fbdev@vger.kernel.org; FlorianSchandinat@gmx.de; linux-arm- kernel@lists.infradead.org; linux-samsung-soc@vger.kernel.org; ben-linux@fluff.org; patches@linaro.org; kgene.kim@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:quoted
quoted
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.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.
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 makeproblem with Thomas'squoted
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 wouldnot make the compatibilityquoted
problem with your code. Why we should keep aberration such as your code? If you want to select twoLCDs, please find other way.quoted
quoted
Darius A.