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

[Jingoo Han <jg1.han@samsung.com>]

> -----Original Message-----
> From: Darius Augulis [mailto:augulis.darius@gmail.com]
> Sent: Thursday, March 15, 2012 4:42 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; kgene.kim@samsung.com; ben-
> linux@fluff.org; patches@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
> 
> Hi,
> 
> >
> > Yes, only single LCD resolution will be left.
> >
> >> I have mini6410 with both 4.3" and 7" LCDs and real6410 with 7" LCD. Now
> >> we have possibility to choose LCD size dynamically - leave it there.
> >> What you mean "default" 4.3" size LCD? The 7" size LCD is also provided
> >> by board sellers - I've bought it.
> >
> > OK, I see.
> > Both mini6410 and real6410 provide both 4.3" and 7" LCDs,
> > so you needs to select both LCDs.
> >
> > Um, usually, single LCD is provided on the single board.
> > Also, the daughter board with another kind LCD can be connected to the board.
> 
> 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.
So, your code can be removed.


> If you want to rework s3c-fb platform data and driver framework, you
> should not drop any functionality created by other people.

Augulis, Thomas Abraham wants to rework s3c-fb platform data and driver framework, not me. :)
I am just reviewing the Thomas's patchset.

> 
> >
> > However, .win_mode is used not for LCD, but for windows of FIMD IP.
> > Actually, our current framework does not support to choose multi LCD,
> > so, I understand that you use .win_mode[1] as second LCD.
> > However, that's not accurate way to select multi LCD.
> >
> > Thomas, can you consider Augulis's opinion?
> > I think that the method to select multi LCDs is necessary.
> >
> > Ideal process is such as:
> >  1. add the patch to support to select multi LCDs
> >  2. apply above patch to make the mini6410 and real6410 to select multi LCDs.
> >  3. apply Thomas's patchset to remove timing value from .win_mode variable.
> 
> Yes, this would be the right way to go.

However, skipping step 1 & 2 is also available, because your code is wrong.
As I'm mentioned above, your code will make the problem when 2~5 windows of FIMD IP are used.


> 
> regards,
> Darius A.