Hi, Sylwester Nawrocki.
I appreciate your review and suggestion.
Please, refer to the LCD contoller clock table as follows:
- s3c2440 uses 's3c2410fb.c', not 's3c-fb.c' since LCD controller IP is
different.
However, s3c2443 uses 's3c-fb.c'. So I add s3c2443 to table instead of
s3c2440.
- s3c6410 has SCLK_LCD, but, clock name is not defined.
- Exynos4 does not use name "HCLK".
| LCD controller | |
| (IP core) clock | LCD pixel clock |
----------+------------------------+-----------------------+
s3c2443 | HCLK (lcd) | x | DISPCLK (display-if) |
----------+------------------------+-----------------------+
s3c6410 | HCLK (lcd) | x | SCLK_LCD (N/A) |
----------+------------------------+-----------------------+
s5pc100 | HCLK (lcd) | x | SCLK_LCD (sclk_lcd) |
----------+------------------------+-----------------------+
s5pv210 | HCLK_DSYS (lcd) | x | SCLK_FIMD (sclk_fimd)|
----------+-----------------------+------------------------+
exynos4 | ACLK_160 (fimd) | O | SCLK_FIMD (sclk_fimd)|
----------+------------------------+-----------------------+
s3c2443, s3c6410, s5pc100 and s5pv210 don't use 'sclk_lcd' or 'sclk_fimd'.
'lcd' clock is also used to generate the LCD pixel clock.
My point is that LCD contoroller clock should be named "lcd" for consistence.
If there is not mux for lcd pixel clock in case of exynos4, "sclk_fimd" will be
set
in machine directory.
As you mentioned, I also think that we need to create two clock connection ids
such as "bus_ck", "pix_ck" in order to use SCLK_LCD or SCLK_FIMD.
Moreover, 'lcd' in s5pv210 should be changed to 'fimd' according to s5pv210
datasheet.
However, it requres many works to convert.
So, I think that 'two clock connection ids' patch would be submitted later,
after committing the patches that I submitted on last Friday.
On 06/19/2011 23:39 AM, Sylwester Nawrocki wrote:
> Hi Jingoo,
>
> On 06/17/2011 03:01 PM, Jingoo Han wrote:
> > This patch changes clock name for FIMD from "fimd" to "lcd".
> >
> > Signed-off-by: Jingoo Han<jg1....@samsung.com>
> > ---
> > arch/arm/mach-exynos4/clock.c | 4 ++--
> > 1 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm/mach-exynos4/clock.c
> > b/arch/arm/mach-exynos4/clock.c index 871f9d5..12e6853 100644
> > --- a/arch/arm/mach-exynos4/clock.c
> > +++ b/arch/arm/mach-exynos4/clock.c
> > @@ -433,12 +433,12 @@ static struct clk init_clocks_off[] = {
> > .enable = exynos4_clk_ip_cam_ctrl,
> > .ctrlbit = (1<< 3),
> > }, {
> > - .name = "fimd",
> > + .name = "lcd",
> > .id = 0,
> > .enable = exynos4_clk_ip_lcd0_ctrl,
>
> I think we're inevitably heading to disaster with this kind of implicit
> clocks mapping across various SoCs. It is getting harder to figure out
> what's going on with every new SoC support added.
> The "fimd" clock in this case (exynos4) is not really a HCLK clock, like,
> for instance, in case of s5pv210. But after this patch they would both be
> named "lcd". You cannot set frequency on "fimd" clock, it is only for
> gating the bus clock to the LCD controller, right ? Whereas "lcd"
> indicates HCLK on s5pv210 and can also be used to generate the LCD pixel
> clock.
> I know you are not going to use "lcd" clock in the driver for setting up
> the pixel clock frequency on exynos4 but it's all confusing this way.
>
> I dug in the datasheets and it looks like the LCD controller's IP main
> (bus) clock is named HCLK there and "lcd" throughout the code.
>
> | LCD controller | |
> | (IP core) clock | LCD pixel clock |
> ----------+------------------------+-----------------------+
> s3c2440 | HCLK (lcd) | x | N/A ? |
> ----------+------------------------+-----------------------+
> s3c6410 | HCLK (lcd) | x | LCD |
> ----------+------------------------+-----------------------+
> s5pc100 | HCLK (lcd) | x | SCLK_LCD (sclk_lcd) |
> ----------+------------------------+-----------------------+
> s5pv210 | HCLK(_DSYS) (lcd)| x | SCLK_FIMD (sclk_fimd)|
> ----------+-----------------------+-----------------------+
> exynos4 | ? | - | SCLK_FIMD?(sclk_fimd)|
> ----------+------------------------+-----------------------+
>
> I think we could try to create two clock connection ids to the framebuffer
> device in the first place, e.g. "bus_ck", "pix_ck".
> And then think about how handle that in the driver.
>
> But this requires conversion to the omap-style clock registration method,
> something like in the attached patch. The patch is only for s5pv210 and
> and compile tested only as I didn't have any board to test it here.
> It's based on for-next branch at http://tinyurl.com/6yzravy I think there
> might be more issues to convert the old s3c24xx platforms, nevertheless
> the attached patch should not affect them.
>
> --
> Regards,
> SylwesterN떑꿩�r툤y鉉싕b쾊Ф푤v�^�)頻{.n�+돴쪐{굇ß틏,″㎍썳變}찠꼿쟺�&j:+v돣�쳭喩zZ+�+zf＂톒쉱�~넮녬i鎬z� 췿ⅱ�?솳鈺�&�)刪f
