On 27/01/15 22:42, Arnd Bergmann wrote: > On Tuesday 27 January 2015 23:36:36 Arnd Bergmann wrote: >> On Tuesday 27 January 2015 14:19:09 Guenter Roeck wrote: >>> On Tue, Jan 27, 2015 at 10:29:49PM +0100, Arnd Bergmann wrote: >>>> On Tuesday 27 January 2015 12:52:29 Guenter Roeck wrote: >>>>> Driver does this (today): >>>>> >>>>> drvdata->rtc_clk = clk_get(&pdev->dev, "rtc"); >>>>> >>>>> Isn't that the name to use ? Just wondering. >>>> >>>> Just because the driver uses it at the moment does not mean it's the name >>>> that the IP block uses. >>>> >>>> clk_get() has the unpleasant property of doing fuzzy matching >>>> on the name that is passed. It first tries to use the string >>>> as the name of the clock input of the device, but if that is >>>> not there, it falls back to looking for a global clk with a con_id. >>>> >>>> In DT, we only support the first kind, but if a driver currently >>>> uses the second, you get the wrong name. >>>> >>>> Looking at arch/mips/jz4740/clock.c now, this seems to be exactly >>>> what is going on here: there is no clkdev_add call to associate >>>> the device clocks, so it can only match a global clock entry. >>>> >>> Me confused :-(. >>> >>> Does that mean the driver needs to be fixed, that the DT property >>> needs to change (to what ?), or both ? >> >> Both. >> >> The jz47xx clock driver should register a clkdev lookup table with >> proper clock input names for each clock that is referenced by a >> device, and then the drivers can use the right names. >> >> In a lot of cases, the best name for a clock is no name so you >> just use an anonymous clock like >> >> clk = clk_get(dev, NULL); >> >> but this still requires a clock lookup table. > > To illustrate why the current approach is wrong, think of a driver > that handles a device that can be used on two different SoCs. > > The name used by the clock provider is SoC specific, so the driver > would need to know which SoC it's being used on in order to pass > the right clock signal name. clkdev is meant to solve this by providing > a lookup between the device/clock-input pair and the actual clock. > > Apparently jz4740 does not share any devices with another SoC at > the moment, so this has not been a problem, but if jz4780 has > a slightly different clock tree, it's already broken. > > Arnd > There is on-going work to fix jz4740, add jz4780 and shake the entire clock tree as well. Patch 14 onwards in this series http://patchwork.linux-mips.org/bundle/paulburton/ci20-v3.20/ Instead of lumping things out in huge changesets, I intended to push out the minor patches that are disjoint. That was the purpose of sending these two patches. Current binding requires the clock-name to be "rtc". Hence the name at the moment. Regards, ZubairLK -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
