On Mon, Sep 6, 2010 at 5:46 PM, Marek Szyprowski <m.szyprowski@xxxxxxxxxxx> wrote: > Hello, > > On 2010-09-06 13:52, Jassi Brar wrote: >> >> On Mon, Sep 6, 2010 at 12:50 PM, Marek Szyprowski >> <m.szyprowski@xxxxxxxxxxx> wrote: >> .... >>> >>> + parent = clk_get(NULL, "mout_epll"); >>> + if (IS_ERR(parent)) >>> + return PTR_ERR(parent); >>> + >>> + for (i = 0; err == 0&& i< ARRAY_SIZE(fimc_devs); i++) { >>> + if (fimc_devs[i]) { >>> + clk_fimc = clk_get(fimc_devs[i], "sclk_fimc"); >>> + if (IS_ERR(clk_fimc)) { >>> + err = PTR_ERR(clk_fimc); >>> + break; >>> + } >>> + clk_set_parent(clk_fimc, parent); >>> + clk_put(clk_fimc); >>> + } >>> + } >> >> The sclk_fimc could source clock from a number of options out of a mux. >> mout_epll is just one of them. Different machines may want to source the >> clock differently. > > Right, I forgot about this case. > >> So, IMO the parent selection should not be done in platform code, but >> rather >> in machine init code. >> Not the best, but a better solution can be found in >> arch/arm/mach-s5pc100/dev-spi.c >> Give it a thought. > > I'm thinking of making the parent clock an argument to the > s5pv210_fimc_setup_clks(). Yes, that's better since the relevant clock is managed by the CMU > I really don't like the idea of passing clock name through the platform data > and letting driver to mess with clock's parents. In case of SPI the clock mux and scalar is present _within_ the SPI controller and having to touch SPI regs from outside the driver isn't what I prefer. > Machine startup code is the > last place where such things should be changed. Until I am enlightened, I'd like to think otherwise. I think the board designer would already have thought out the clock sourcing hierarchy. Setting appropriate parents once at boot-time and having drivers not worry about it, should be better. -- 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