On Thu, Feb 22, 2018 at 1:22 PM, Marek Szyprowski <m.szyprowski@xxxxxxxxxxx> wrote: > Hi Krzysztof, > > On 2018-02-21 17:19, Krzysztof Kozlowski wrote: >> >> On Wed, Feb 21, 2018 at 11:15 AM, Marek Szyprowski >> <m.szyprowski@xxxxxxxxxxx> wrote: >>> >>> Exynos5250/5420/5800 have only one clock constroller, but some of their >>> clock depends on respective power domains. Handling integration of clock >>> controller and power domain can be done using runtime PM feature of CCF >>> framework. This however needs a separate struct device for each power >>> domain. This patch adds such separate driver for group such clocks, >> >> "driver to group"? >> >>> which can be instantiated more than once, each time for a different >>> power domain. >>> >>> Signed-off-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx> >>> --- >>> drivers/clk/samsung/clk-exynos5x-subcmu.c | 180 >>> ++++++++++++++++++++++++++++++ >> >> In some other places we call entire family "exynos5" so maybe let's >> follow the convention instead of "5x"? In the name of file and all the >> variables/names later? > > > Okay. > > >>> drivers/clk/samsung/clk-exynos5x-subcmu.h | 30 +++++ >>> 2 files changed, 210 insertions(+) >>> create mode 100644 drivers/clk/samsung/clk-exynos5x-subcmu.c >>> create mode 100644 drivers/clk/samsung/clk-exynos5x-subcmu.h >>> >>> diff --git a/drivers/clk/samsung/clk-exynos5x-subcmu.c >>> b/drivers/clk/samsung/clk-exynos5x-subcmu.c >>> new file mode 100644 >>> index 000000000000..9ff6d5d17f57 >>> --- /dev/null >>> +++ b/drivers/clk/samsung/clk-exynos5x-subcmu.c >>> @@ -0,0 +1,180 @@ >>> +/* >>> + * Copyright (c) 2018 Samsung Electronics Co., Ltd. >>> + * Author: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx> >>> + * >>> + * This program is free software; you can redistribute it and/or modify >>> + * it under the terms of the GNU General Public License version 2 as >>> + * published by the Free Software Foundation. >> >> Use SPDX identifier instead of license text. >> >>> + * >>> + * Common Clock Framework support for Exynos5x power-domain dependent >>> + * sub-CMUs >>> + */ >>> + >>> +#include <linux/of_platform.h> >>> +#include <linux/platform_device.h> >>> +#include <linux/pm_domain.h> >>> +#include <linux/pm_runtime.h> >>> + >>> +#include "clk.h" >>> +#include "clk-exynos5x-subcmu.h" >>> + >>> +static struct samsung_clk_provider *ctx; >>> +static const struct samsung_5x_subcmu_info *cmu; >>> +static int nr_cmus; >> >> core_initcall here will register this and subcmu drivers. The probe of >> this driver will populate devices for subcmus. From next patch - the >> initcall of clock driver - will initialize the subcmus, also touching >> these statics. >> >> Is my understanding correct? >> >> Quite complicated init system... plus static variables. Are you sure >> there are no concurrency issues? > > > Everything is okay. exynos5420 clock driver is initialized very early > via OF_CLK_DECLARE() method. It calls > > samsung_clk_subcmus_init() at the end, which passes the needed clk > context to this driver. Then there is core_init call, which registers > both platform drivers and then a bit later at arch_initcalls all > platform devices are populated from device tree. This triggers a probe > of exynos5x-clock platform driver, which in its probe creates child > platform devices for each supported power domain. Then those child > devices are probed by exynos5x-clock-subcmu platform driver, which > finally registers clocks and enables runtime pm for them. > > I know this is a bit complex, but frankly I didn't find any other way > of handling clocks by both OF_CLK_DECLARE and platform device based > driver(s). Maybe I will put the above explanation in the comment > somewhere in this driver. > > In theory exynos5x-clock platform driver is not really needed, as child > devices might have been created directly from OF_CLK_DECLARE()-based > code, but sadly it is called so early that it is not yet possible to > call any code related to platform bus (it is not yet initialized). The > good thing that all this is hidden inside this driver and there are no > external hacks needed elsewhere (like in DT) to get it working. Thanks for explanation - indeed it would be nice to see it somewhere in the code. Also as you pointed, it is nice that everything is driver-contained. Best regards, Krzysztof -- 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