Re: [PATCH v2 1/5] ARM: EXYNOS4: Change clock name for FIMD

[daeinki <inki.dae@samsung.com>]

Hi Sylwester

You are right, no problem if clkdev is used instead of 
plat-samsung/clock.c. I didn't aware of recently trend.(I couldn't 
afford to having interest in open source) and I will look over clkdev 
feature Thomas introduced.

Thank you.

Sylwester Nawrocki 쓴 글:
> Hi Inki
> 
> On 06/20/2011 12:09 PM, daeinki wrote:
>> Hi, Mr. Han and Sylwester.
>> below is my opinion.
>>
>> JinGoo Han 쓴 글:
> ...
>>> 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)|
>>> ----------+------------------------+-----------------------+
> ...
>>>> 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.
> ...
> 
>> when someone adds new board file with new SoC, he doesn't need to know
>> this SoC chip has hclk and sclk_fimd or only sclk_fimd(such as exynos4).
>> using implicit clock means it should know that this SoC chip has both
>> clocks(bus clock, sclk_fimd) or only sclk_fimd.
>>
>> for example, if any driver needs fimd clock frequency then this driver
>> should know that this SoC chip is exynos4 or not and has both clock
>> source(bus clock, soure clock fimd) or not(only source clock fimd)
>> so I think we shoule see only a clock "lcd" regardless of which clock is
>> used and if exynos4 then sclk_fimd would be set by machine code.
>>
>> and Sylwester,
>> it appears that your patch has one issue about clk_get function call.
>> your patch adds "bus_ck" to list head "clocks" of plat-samsung/clock.c
> 
> Why do you think so ? In fact the "clocks" list in plat-samsung/clock.c
> is not used any more, AFAIU it should have been removed altogether with
> clk_get/clk_put functions in Thomas' clkdev patches. 
> When you comment out the line declaring the list everything compiles fine
> there.
> 
>> and "pix_ck" to list head "clocks" of drivers/clk/clkdev.c and I am
>> afraid that if some machine(such as s3c24xx, s3c64xx and s5pc1xx) has
>> CLKDEV_LOOKUP configuration then clk_get() would fail to get clock
>> object because in this case, clock lookup could be done through list
>> head "clocks" of driver/clk/clkdev.c.(it's right from
>> plat-samsung/clock.c) so I think it needs more patch for resolving this
> 
> As I indicated earlier the framebuffer driver would have to be modified
> to support newly introduced clock _connection_ names.
> We could (temporarily) name one of those clock connections "lcd",
> to avoid additional trouble on SoCs that still use a one-to-one
> platform clock name <-> clock connection id mapping.
> 
>> issue also and do you think it's a good way to use only one clock name
>> "lcd"?... in fact, this might be so.... much slight issue. :)
> 
> Do you mean using using one name in the code for different clock names
> in the datasheets ? I suppose it was because of the API limitations and
> hope it will change for the better. :)
> 
> Cheers,
> Sylwester
> 
>