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 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
 |