[U-Boot] [PATCH v3 00/11] clk: Port Linux common clock framework [CCF] to U-boot (tag: 5.0-rc3)
This patch series brings the files from Linux kernel to provide clocks support as it is used on the Linux kernel with common clock framework [CCF] setup.
This series also fixes several problems with current clocks and provides sandbox tests for functions addded to clk-uclass.c file.
Design decisions/issues: =========================
- U-boot's DM for clk differs from Linux CCF. The most notably difference is the lack of support for hierarchical clocks and "clock as a manager driver" (single clock DTS node acts as a starting point for all other clocks).
- The clk_get_rate() now caches the previously read data (no need for recursive access.
- On purpose the "manager" clk driver (clk-imx6q.c) is not using large table to store pointers to clocks - e.g. clk[IMX6QDL_CLK_USDHC2_SEL] = .... Instead we use udevice's linked list for the same class (UCLASS_CLK). The rationale - when porting the code as is from Linux, one would need ~1KiB of RAM to store it. This is way too much if we do plan to use this driver in SPL.
- The "central" structure of this patch series is struct udevice and its driver_data field contains the struct clk pointer (to the originally created one).
- Up till now U-boot's driver model's CLK operates on udevice (main access to clock is by udevice ops) In the CCF the access to struct clk (comprising pointer to *dev) is possible via dev_get_driver_data()
Storing back pointer (from udevice to struct clk) as driver_data is a convention for CCF.
- I could use *private_alloc_size to allocate driver's 'private" structures (dev->priv) for e.g. divider (struct clk_divider *divider) for IMX6Q clock, but this would change the original structure of the CCF code.
The question is if it would be better to use private_alloc_size (and dev->private) or stay with driver_data. The former requires some rewritting in CCF original code (to remove (c)malloc, etc), but comply with u-boot DM. The latter allows re-using the CCF code as is, but introduces some convention special for CCF (I'm not sure thought if dev->priv is NOT another convention as well).
- I've added the clk_get_parent(), which reads parent's dev->driver_data to provide parent's struct clk pointer. This seems the easiest way to get child/parent relationship for struct clk in U-boot's udevice based clocks.
- For tests I had to "emulate" CCF code structure to test functionality of clk_get_parent_rate() and clk_get_by_id(). Those functions will not work properly with "standard" (i.e. non CCF) clock setup(with not set dev->driver_data to struct clk).
Repository: https://github.com/lmajewski/u-boot-dfu/commits/CCF-v3
Changes in v3: - New patch - The rate information is now cached into struct clk field - The clk_get_parent() is used to get pointer to the parent struct clk - Replace -ENODEV with -ENOENT - Use **clkp instead of **c - New patch - New patch
Lukasz Majewski (11): dm: Fix documentation entry as there is no UCLASS_CLOCK uclass cmd: Do not show frequency for clocks which .get_rate() return error clk: Remove clock ID check in .get_rate() of clk_fixed_* clk: Extend struct clk to provide information regarding clock rate clk: Provide struct clk for fixed rate clock (clk_fixed_rate.c) dm: clk: Define clk_get_parent() for clk operations dm: clk: Define clk_get_parent_rate() for clk operations dm: clk: Define clk_get_by_id() for clk operations clk: test: Provide unit test for clk_get_by_id() method clk: test: Provide unit test for clk_get_parent_rate() method clk: Port Linux common clock framework [CCF] for imx6q to U-boot (tag: 5.0-rc3)
arch/sandbox/include/asm/clk.h | 16 ++++ cmd/clk.c | 5 +- drivers/clk/Kconfig | 14 ++++ drivers/clk/Makefile | 2 + drivers/clk/clk-divider.c | 148 ++++++++++++++++++++++++++++++++++ drivers/clk/clk-fixed-factor.c | 87 ++++++++++++++++++++ drivers/clk/clk-mux.c | 164 +++++++++++++++++++++++++++++++++++++ drivers/clk/clk-uclass.c | 59 ++++++++++++++ drivers/clk/clk.c | 56 +++++++++++++ drivers/clk/clk_fixed_factor.c | 3 - drivers/clk/clk_fixed_rate.c | 8 +- drivers/clk/clk_sandbox_test.c | 49 +++++++++++ drivers/clk/imx/Kconfig | 9 +++ drivers/clk/imx/Makefile | 2 + drivers/clk/imx/clk-gate2.c | 113 ++++++++++++++++++++++++++ drivers/clk/imx/clk-imx6q.c | 179 +++++++++++++++++++++++++++++++++++++++++ drivers/clk/imx/clk-pfd.c | 91 +++++++++++++++++++++ drivers/clk/imx/clk-pllv3.c | 83 +++++++++++++++++++ drivers/clk/imx/clk.h | 75 +++++++++++++++++ include/clk.h | 33 +++++++- include/linux/clk-provider.h | 94 ++++++++++++++++++++++ test/dm/clk.c | 4 +- 22 files changed, 1285 insertions(+), 9 deletions(-) create mode 100644 drivers/clk/clk-divider.c create mode 100644 drivers/clk/clk-fixed-factor.c create mode 100644 drivers/clk/clk-mux.c create mode 100644 drivers/clk/clk.c create mode 100644 drivers/clk/imx/clk-gate2.c create mode 100644 drivers/clk/imx/clk-imx6q.c create mode 100644 drivers/clk/imx/clk-pfd.c create mode 100644 drivers/clk/imx/clk-pllv3.c create mode 100644 drivers/clk/imx/clk.h create mode 100644 include/linux/clk-provider.h
There is no UCLASS_CLOCK uclass defined. Instead we do use the UCLASS_CLK.
Signed-off-by: Lukasz Majewski lukma@denx.de Reviewed-by: Simon Glass sjg@chromium.org ---
Changes in v3: None
include/clk.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/clk.h b/include/clk.h index 8e366163f9..f6fbcc6634 100644 --- a/include/clk.h +++ b/include/clk.h @@ -19,7 +19,7 @@ * clock provider. This API provides a standard means for drivers to enable and * disable clocks, and to set the rate at which they oscillate. * - * A driver that implements UCLASS_CLOCK is a clock provider. A provider will + * A driver that implements UCLASS_CLK is a clock provider. A provider will * often implement multiple separate clocks, since the hardware it manages * often has this capability. clk-uclass.h describes the interface which * clock providers must implement.
Subject: [PATCH v3 01/11] dm: Fix documentation entry as there is no UCLASS_CLOCK uclass
There is no UCLASS_CLOCK uclass defined. Instead we do use the UCLASS_CLK.
Signed-off-by: Lukasz Majewski lukma@denx.de Reviewed-by: Simon Glass sjg@chromium.org
Changes in v3: None
include/clk.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/clk.h b/include/clk.h index 8e366163f9..f6fbcc6634 100644 --- a/include/clk.h +++ b/include/clk.h @@ -19,7 +19,7 @@
- clock provider. This API provides a standard means for drivers to enable
and
- disable clocks, and to set the rate at which they oscillate.
- A driver that implements UCLASS_CLOCK is a clock provider. A provider will
- A driver that implements UCLASS_CLK is a clock provider. A provider
- will
- often implement multiple separate clocks, since the hardware it manages
- often has this capability. clk-uclass.h describes the interface which
- clock providers must implement.
Reviewed-by: Peng Fan peng.fan@nxp.com
-- 2.11.0
It may happen that some UCLASS_CLK clocks drivers work as a "managers", to call other, proper clocks. This situation is present in the iMX{6|8} clocks when supporting CONFIG_CLK (and CCF).
To avoid bogus output of "clk dump" we omit clocks which return error value - allowing reusing default implementation of this command.
Signed-off-by: Lukasz Majewski lukma@denx.de Reviewed-by: Simon Glass sjg@chromium.org ---
Changes in v3: None
cmd/clk.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/cmd/clk.c b/cmd/clk.c index fd4231589c..2ea82176aa 100644 --- a/cmd/clk.c +++ b/cmd/clk.c @@ -16,6 +16,7 @@ int __weak soc_clk_dump(void) struct udevice *dev; struct uclass *uc; struct clk clk; + ulong rate; int ret;
/* Device addresses start at 1 */ @@ -37,7 +38,9 @@ int __weak soc_clk_dump(void) continue; }
- printf("%-30.30s : %lu Hz\n", dev->name, clk_get_rate(&clk)); + rate = clk_get_rate(&clk); + if (!IS_ERR_VALUE(rate)) + printf("%-30.30s : %lu Hz\n", dev->name, rate);
clk_free(&clk); }
Subject: [PATCH v3 02/11] cmd: Do not show frequency for clocks which .get_rate() return error
It may happen that some UCLASS_CLK clocks drivers work as a "managers", to call other, proper clocks. This situation is present in the iMX{6|8} clocks when supporting CONFIG_CLK (and CCF).
To avoid bogus output of "clk dump" we omit clocks which return error value - allowing reusing default implementation of this command.
Signed-off-by: Lukasz Majewski lukma@denx.de Reviewed-by: Simon Glass sjg@chromium.org
Changes in v3: None
cmd/clk.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/cmd/clk.c b/cmd/clk.c index fd4231589c..2ea82176aa 100644 --- a/cmd/clk.c +++ b/cmd/clk.c @@ -16,6 +16,7 @@ int __weak soc_clk_dump(void) struct udevice *dev; struct uclass *uc; struct clk clk;
ulong rate; int ret;
/* Device addresses start at 1 */
@@ -37,7 +38,9 @@ int __weak soc_clk_dump(void) continue; }
printf("%-30.30s : %lu Hz\n", dev->name, clk_get_rate(&clk));
rate = clk_get_rate(&clk);if (!IS_ERR_VALUE(rate))printf("%-30.30s : %lu Hz\n", dev->name, rate);clk_free(&clk); }
Reviewed-by: Peng Fan peng.fan@nxp.com
-- 2.11.0
This check requires the struct clk passed to .get_rate() to be always cleared out as any clock with valid ID causes -EINVAL return value.
The return code of fixed clocks shall always be returned.
Signed-off-by: Lukasz Majewski lukma@denx.de ---
Changes in v3: None
drivers/clk/clk_fixed_factor.c | 3 --- drivers/clk/clk_fixed_rate.c | 3 --- 2 files changed, 6 deletions(-)
diff --git a/drivers/clk/clk_fixed_factor.c b/drivers/clk/clk_fixed_factor.c index 5fa20a84db..dcdb6ddf5c 100644 --- a/drivers/clk/clk_fixed_factor.c +++ b/drivers/clk/clk_fixed_factor.c @@ -24,9 +24,6 @@ static ulong clk_fixed_factor_get_rate(struct clk *clk) uint64_t rate; struct clk_fixed_factor *ff = to_clk_fixed_factor(clk->dev);
- if (clk->id != 0) - return -EINVAL; - rate = clk_get_rate(&ff->parent); if (IS_ERR_VALUE(rate)) return rate; diff --git a/drivers/clk/clk_fixed_rate.c b/drivers/clk/clk_fixed_rate.c index d8d9f86c86..50dbb13655 100644 --- a/drivers/clk/clk_fixed_rate.c +++ b/drivers/clk/clk_fixed_rate.c @@ -15,9 +15,6 @@ struct clk_fixed_rate {
static ulong clk_fixed_rate_get_rate(struct clk *clk) { - if (clk->id != 0) - return -EINVAL; - return to_clk_fixed_rate(clk->dev)->fixed_rate; }
Subject: [PATCH v3 03/11] clk: Remove clock ID check in .get_rate() of clk_fixed_*
This check requires the struct clk passed to .get_rate() to be always cleared out as any clock with valid ID causes -EINVAL return value.
The return code of fixed clocks shall always be returned.
Signed-off-by: Lukasz Majewski lukma@denx.de
Changes in v3: None
drivers/clk/clk_fixed_factor.c | 3 --- drivers/clk/clk_fixed_rate.c | 3 --- 2 files changed, 6 deletions(-)
diff --git a/drivers/clk/clk_fixed_factor.c b/drivers/clk/clk_fixed_factor.c index 5fa20a84db..dcdb6ddf5c 100644 --- a/drivers/clk/clk_fixed_factor.c +++ b/drivers/clk/clk_fixed_factor.c @@ -24,9 +24,6 @@ static ulong clk_fixed_factor_get_rate(struct clk *clk) uint64_t rate; struct clk_fixed_factor *ff = to_clk_fixed_factor(clk->dev);
- if (clk->id != 0)
return -EINVAL;- rate = clk_get_rate(&ff->parent); if (IS_ERR_VALUE(rate)) return rate;
diff --git a/drivers/clk/clk_fixed_rate.c b/drivers/clk/clk_fixed_rate.c index d8d9f86c86..50dbb13655 100644 --- a/drivers/clk/clk_fixed_rate.c +++ b/drivers/clk/clk_fixed_rate.c @@ -15,9 +15,6 @@ struct clk_fixed_rate {
static ulong clk_fixed_rate_get_rate(struct clk *clk) {
- if (clk->id != 0)
return -EINVAL;- return to_clk_fixed_rate(clk->dev)->fixed_rate;
}
Reviewed-by: Peng Fan peng.fan@nxp.com
-- 2.11.0
This commit extends the struct clk to provide information regarding the clock rate. As a result the clock tree traversal is performed at most once, and further reads are using the cached value.
Signed-off-by: Lukasz Majewski lukma@denx.de ---
Changes in v3: None
include/clk.h | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/include/clk.h b/include/clk.h index f6fbcc6634..f29ba02da1 100644 --- a/include/clk.h +++ b/include/clk.h @@ -39,6 +39,7 @@ struct udevice; * other clock APIs to identify which clock signal to operate upon. * * @dev: The device which implements the clock signal. + * @rate: The clock rate (in HZ). * @id: The clock signal ID within the provider. * @data: An optional data field for scenarios where a single integer ID is not * sufficient. If used, it can be populated through an .of_xlate op and @@ -54,6 +55,7 @@ struct udevice; */ struct clk { struct udevice *dev; + unsigned long rate; /* in HZ */ /* * Written by of_xlate. In the future, we might add more fields here. */
Hi Lukasz,
Subject: [PATCH v3 04/11] clk: Extend struct clk to provide information regarding clock rate
This commit extends the struct clk to provide information regarding the clock rate. As a result the clock tree traversal is performed at most once, and further reads are using the cached value.
I am thinking recalc is needed for some platforms, for clks without CLK_GET_RATE_NOCACHE, It is to use cached value. Since your test mostly on i.MX6Q, so it should be fine for now.
Reviewed-by: Peng Fan peng.fan@nxp.com
Signed-off-by: Lukasz Majewski lukma@denx.de
Changes in v3: None
include/clk.h | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/include/clk.h b/include/clk.h index f6fbcc6634..f29ba02da1 100644 --- a/include/clk.h +++ b/include/clk.h @@ -39,6 +39,7 @@ struct udevice;
- other clock APIs to identify which clock signal to operate upon.
- @dev: The device which implements the clock signal.
- @rate: The clock rate (in HZ).
- @id: The clock signal ID within the provider.
- @data: An optional data field for scenarios where a single integer ID is
not
sufficient. If used, it can be populated through an .of_xlate op and@@ -54,6 +55,7 @@ struct udevice; */ struct clk { struct udevice *dev;
- unsigned long rate; /* in HZ */ /*
*/
- Written by of_xlate. In the future, we might add more fields here.
-- 2.11.0
Up till now the fixed rate clock ('osc') has been added to UCLASS_CLK without declaring struct clk. As a result it was only accessible by iterating the udevice's uclass list.
This is a problem for clock code, which operates on pointers to struct clk (like clk_get_rate()), not udevices.
After this change struct clk is accessible from udevice and udevice from struct clk.
Signed-off-by: Lukasz Majewski lukma@denx.de ---
Changes in v3: None
drivers/clk/clk_fixed_rate.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/drivers/clk/clk_fixed_rate.c b/drivers/clk/clk_fixed_rate.c index 50dbb13655..089f060a23 100644 --- a/drivers/clk/clk_fixed_rate.c +++ b/drivers/clk/clk_fixed_rate.c @@ -8,6 +8,7 @@ #include <dm.h>
struct clk_fixed_rate { + struct clk clk; unsigned long fixed_rate; };
@@ -24,10 +25,14 @@ const struct clk_ops clk_fixed_rate_ops = {
static int clk_fixed_rate_ofdata_to_platdata(struct udevice *dev) { + struct clk *clk = &to_clk_fixed_rate(dev)->clk; #if !CONFIG_IS_ENABLED(OF_PLATDATA) to_clk_fixed_rate(dev)->fixed_rate = dev_read_u32_default(dev, "clock-frequency", 0); #endif + /* Make fixed rate clock accessible from higher level struct clk */ + dev->driver_data = (ulong)clk; + clk->dev = dev;
return 0; }
Subject: [PATCH v3 05/11] clk: Provide struct clk for fixed rate clock (clk_fixed_rate.c)
Up till now the fixed rate clock ('osc') has been added to UCLASS_CLK without declaring struct clk. As a result it was only accessible by iterating the udevice's uclass list.
This is a problem for clock code, which operates on pointers to struct clk (like clk_get_rate()), not udevices.
After this change struct clk is accessible from udevice and udevice from struct clk.
Signed-off-by: Lukasz Majewski lukma@denx.de
Changes in v3: None
drivers/clk/clk_fixed_rate.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/drivers/clk/clk_fixed_rate.c b/drivers/clk/clk_fixed_rate.c index 50dbb13655..089f060a23 100644 --- a/drivers/clk/clk_fixed_rate.c +++ b/drivers/clk/clk_fixed_rate.c @@ -8,6 +8,7 @@ #include <dm.h>
struct clk_fixed_rate {
- struct clk clk; unsigned long fixed_rate;
};
@@ -24,10 +25,14 @@ const struct clk_ops clk_fixed_rate_ops = {
static int clk_fixed_rate_ofdata_to_platdata(struct udevice *dev) {
- struct clk *clk = &to_clk_fixed_rate(dev)->clk;
#if !CONFIG_IS_ENABLED(OF_PLATDATA) to_clk_fixed_rate(dev)->fixed_rate = dev_read_u32_default(dev, "clock-frequency", 0); #endif
- /* Make fixed rate clock accessible from higher level struct clk */
- dev->driver_data = (ulong)clk;
- clk->dev = dev;
Reviewed-by: Peng Fan peng.fan@nxp.com
return 0; } -- 2.11.0
This commit adds the clk_get_parent() function, which is responsible for getting the parent's struct clock pointer.
U-boot's DM support for getting parent is different (the parent relationship is in udevice) than the one in common clock framework (CCF) in Linux. To obtain the pointer to struct clk of parent the pdev->driver_data field is read.
Signed-off-by: Lukasz Majewski lukma@denx.de
---
Changes in v3: - New patch
drivers/clk/clk-uclass.c | 15 +++++++++++++++ include/clk.h | 9 +++++++++ 2 files changed, 24 insertions(+)
diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c index 844b87cc33..7ebe4e79fe 100644 --- a/drivers/clk/clk-uclass.c +++ b/drivers/clk/clk-uclass.c @@ -340,6 +340,21 @@ ulong clk_get_rate(struct clk *clk) return ops->get_rate(clk); }
+struct clk *clk_get_parent(struct clk *clk) +{ + struct udevice *pdev; + struct clk *pclk; + + debug("%s(clk=%p)\n", __func__, clk); + + pdev = dev_get_parent(clk->dev); + pclk = (struct clk *)dev_get_driver_data(pdev); + if (!pclk) + return ERR_PTR(-ENODEV); + + return pclk; +} + ulong clk_set_rate(struct clk *clk, ulong rate) { const struct clk_ops *ops = clk_dev_ops(clk->dev); diff --git a/include/clk.h b/include/clk.h index f29ba02da1..b44ee3b158 100644 --- a/include/clk.h +++ b/include/clk.h @@ -240,6 +240,15 @@ int clk_free(struct clk *clk); ulong clk_get_rate(struct clk *clk);
/** + * clk_get_parent() - Get current clock's parent. + * + * @clk: A clock struct that was previously successfully requested by + * clk_request/get_by_*(). + * @return pointer to parent's struct clk, or error code passed as pointer + */ +struct clk *clk_get_parent(struct clk *clk); + +/** * clk_set_rate() - Set current clock rate. * * @clk: A clock struct that was previously successfully requested by
Subject: [PATCH v3 06/11] dm: clk: Define clk_get_parent() for clk operations
This commit adds the clk_get_parent() function, which is responsible for getting the parent's struct clock pointer.
U-boot's DM support for getting parent is different (the parent relationship is in udevice) than the one in common clock framework (CCF) in Linux. To obtain the pointer to struct clk of parent the pdev->driver_data field is read.
Signed-off-by: Lukasz Majewski lukma@denx.de
Changes in v3:
- New patch
drivers/clk/clk-uclass.c | 15 +++++++++++++++ include/clk.h | 9 +++++++++ 2 files changed, 24 insertions(+)
diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c index 844b87cc33..7ebe4e79fe 100644 --- a/drivers/clk/clk-uclass.c +++ b/drivers/clk/clk-uclass.c @@ -340,6 +340,21 @@ ulong clk_get_rate(struct clk *clk) return ops->get_rate(clk); }
+struct clk *clk_get_parent(struct clk *clk) {
- struct udevice *pdev;
- struct clk *pclk;
- debug("%s(clk=%p)\n", __func__, clk);
- pdev = dev_get_parent(clk->dev);
- pclk = (struct clk *)dev_get_driver_data(pdev);
- if (!pclk)
return ERR_PTR(-ENODEV);- return pclk;
+}
ulong clk_set_rate(struct clk *clk, ulong rate) { const struct clk_ops *ops = clk_dev_ops(clk->dev); diff --git a/include/clk.h b/include/clk.h index f29ba02da1..b44ee3b158 100644 --- a/include/clk.h +++ b/include/clk.h @@ -240,6 +240,15 @@ int clk_free(struct clk *clk); ulong clk_get_rate(struct clk *clk);
/**
- clk_get_parent() - Get current clock's parent.
- @clk: A clock struct that was previously successfully requested by
clk_request/get_by_*().
- @return pointer to parent's struct clk, or error code passed as
+pointer */ struct clk *clk_get_parent(struct clk *clk);
+/**
- clk_set_rate() - Set current clock rate.
- @clk: A clock struct that was previously successfully requested by
--
Reviewed-by: Peng Fan peng.fan@nxp.com
2.11.0
This commit adds the clk_get_parent_rate() function, which is responsible for getting the rate of parent clock. Unfortunately, u-boot's DM support for getting parent is different (the parent relationship is in udevice) than the one in common clock framework (CCF) in Linux.
To alleviate this problem - the clk_get_parent_rate() function has been introduced to clk-uclass.c.
Signed-off-by: Lukasz Majewski lukma@denx.de
---
Changes in v3: - The rate information is now cached into struct clk field - The clk_get_parent() is used to get pointer to the parent struct clk
drivers/clk/clk-uclass.c | 22 ++++++++++++++++++++++ include/clk.h | 9 +++++++++ 2 files changed, 31 insertions(+)
diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c index 7ebe4e79fe..da6624d4f2 100644 --- a/drivers/clk/clk-uclass.c +++ b/drivers/clk/clk-uclass.c @@ -355,6 +355,28 @@ struct clk *clk_get_parent(struct clk *clk) return pclk; }
+ulong clk_get_parent_rate(struct clk *clk) +{ + const struct clk_ops *ops; + struct clk *pclk; + + debug("%s(clk=%p)\n", __func__, clk); + + pclk = clk_get_parent(clk); + if (IS_ERR(pclk)) + return -ENODEV; + + ops = clk_dev_ops(pclk->dev); + if (!ops->get_rate) + return -ENOSYS; + + /* Read the 'rate' if not already set */ + if (!pclk->rate) + pclk->rate = clk_get_rate(pclk); + + return pclk->rate; +} + ulong clk_set_rate(struct clk *clk, ulong rate) { const struct clk_ops *ops = clk_dev_ops(clk->dev); diff --git a/include/clk.h b/include/clk.h index b44ee3b158..98c3e12fb4 100644 --- a/include/clk.h +++ b/include/clk.h @@ -249,6 +249,15 @@ ulong clk_get_rate(struct clk *clk); struct clk *clk_get_parent(struct clk *clk);
/** + * clk_get_parent_rate() - Get parent of current clock rate. + * + * @clk: A clock struct that was previously successfully requested by + * clk_request/get_by_*(). + * @return clock rate in Hz, or -ve error code. + */ +ulong clk_get_parent_rate(struct clk *clk); + +/** * clk_set_rate() - Set current clock rate. * * @clk: A clock struct that was previously successfully requested by
Subject: [PATCH v3 07/11] dm: clk: Define clk_get_parent_rate() for clk operations
This commit adds the clk_get_parent_rate() function, which is responsible for getting the rate of parent clock. Unfortunately, u-boot's DM support for getting parent is different (the parent relationship is in udevice) than the one in common clock framework (CCF) in Linux.
To alleviate this problem - the clk_get_parent_rate() function has been introduced to clk-uclass.c.
Signed-off-by: Lukasz Majewski lukma@denx.de
Changes in v3:
- The rate information is now cached into struct clk field
- The clk_get_parent() is used to get pointer to the parent struct clk
drivers/clk/clk-uclass.c | 22 ++++++++++++++++++++++ include/clk.h | 9 +++++++++ 2 files changed, 31 insertions(+)
diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c index 7ebe4e79fe..da6624d4f2 100644 --- a/drivers/clk/clk-uclass.c +++ b/drivers/clk/clk-uclass.c @@ -355,6 +355,28 @@ struct clk *clk_get_parent(struct clk *clk) return pclk; }
+ulong clk_get_parent_rate(struct clk *clk) {
- const struct clk_ops *ops;
- struct clk *pclk;
- debug("%s(clk=%p)\n", __func__, clk);
- pclk = clk_get_parent(clk);
- if (IS_ERR(pclk))
return -ENODEV;- ops = clk_dev_ops(pclk->dev);
- if (!ops->get_rate)
return -ENOSYS;- /* Read the 'rate' if not already set */
- if (!pclk->rate)
pclk->rate = clk_get_rate(pclk);
Suggest a flag NOCACHE like Linux, in clk_get_rate, check the flag, if not set, just read the cached value.
How do you think?
Regards, Peng.
- return pclk->rate;
+}
ulong clk_set_rate(struct clk *clk, ulong rate) { const struct clk_ops *ops = clk_dev_ops(clk->dev); diff --git a/include/clk.h b/include/clk.h index b44ee3b158..98c3e12fb4 100644 --- a/include/clk.h +++ b/include/clk.h @@ -249,6 +249,15 @@ ulong clk_get_rate(struct clk *clk); struct clk *clk_get_parent(struct clk *clk);
/**
- clk_get_parent_rate() - Get parent of current clock rate.
- @clk: A clock struct that was previously successfully requested by
clk_request/get_by_*().
- @return clock rate in Hz, or -ve error code.
- */
+ulong clk_get_parent_rate(struct clk *clk);
+/**
- clk_set_rate() - Set current clock rate.
- @clk: A clock struct that was previously successfully requested by
-- 2.11.0
Hi Peng,
Thank you for your feedback.
Subject: [PATCH v3 07/11] dm: clk: Define clk_get_parent_rate() for clk operations
This commit adds the clk_get_parent_rate() function, which is responsible for getting the rate of parent clock. Unfortunately, u-boot's DM support for getting parent is different (the parent relationship is in udevice) than the one in common clock framework (CCF) in Linux.
To alleviate this problem - the clk_get_parent_rate() function has been introduced to clk-uclass.c.
Signed-off-by: Lukasz Majewski lukma@denx.de
Changes in v3:
- The rate information is now cached into struct clk field
- The clk_get_parent() is used to get pointer to the parent struct
clk
drivers/clk/clk-uclass.c | 22 ++++++++++++++++++++++ include/clk.h | 9 +++++++++ 2 files changed, 31 insertions(+)
diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c index 7ebe4e79fe..da6624d4f2 100644 --- a/drivers/clk/clk-uclass.c +++ b/drivers/clk/clk-uclass.c @@ -355,6 +355,28 @@ struct clk *clk_get_parent(struct clk *clk) return pclk; }
+ulong clk_get_parent_rate(struct clk *clk) {
- const struct clk_ops *ops;
- struct clk *pclk;
- debug("%s(clk=%p)\n", __func__, clk);
- pclk = clk_get_parent(clk);
- if (IS_ERR(pclk))
return -ENODEV;- ops = clk_dev_ops(pclk->dev);
- if (!ops->get_rate)
return -ENOSYS;- /* Read the 'rate' if not already set */
- if (!pclk->rate)
pclk->rate = clk_get_rate(pclk);Suggest a flag NOCACHE like Linux, in clk_get_rate, check the flag, if not set, just read the cached value.
How do you think?
No problem from my side. I will wait a few more days for more feedback nad prepare next version of CCF port.
Please also look on the design decision described in the cover letter: http://patchwork.ozlabs.org/cover/1090669/
(There is also link to github repo with those patches). Maybe you would have other idea for solving mentioned problems?
I would also like to make the CCF code as much IMX SoC agnostic as possible - in other words - make it working on iMX6Q and iMX8 from the outset (so I would know that other SoCs would be easily added).
Regards, Peng.
- return pclk->rate;
+}
ulong clk_set_rate(struct clk *clk, ulong rate) { const struct clk_ops *ops = clk_dev_ops(clk->dev); diff --git a/include/clk.h b/include/clk.h index b44ee3b158..98c3e12fb4 100644 --- a/include/clk.h +++ b/include/clk.h @@ -249,6 +249,15 @@ ulong clk_get_rate(struct clk *clk); struct clk *clk_get_parent(struct clk *clk);
/**
- clk_get_parent_rate() - Get parent of current clock rate.
- @clk: A clock struct that was previously successfully
requested by
clk_request/get_by_*().
- @return clock rate in Hz, or -ve error code.
- */
+ulong clk_get_parent_rate(struct clk *clk);
+/**
- clk_set_rate() - Set current clock rate.
- @clk: A clock struct that was previously successfully
requested by -- 2.11.0
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
This commit adds the clk_get_by_id() function, which is responsible for getting the udevice with matching clk->id. Such approach allows re-usage of inherit DM list relationship for the same class (UCLASS_CLK). As a result - we don't need any other external list - it is just enough to look for UCLASS_CLK related udevices.
Signed-off-by: Lukasz Majewski lukma@denx.de
---
Changes in v3: - Replace -ENODEV with -ENOENT - Use **clkp instead of **c
drivers/clk/clk-uclass.c | 22 ++++++++++++++++++++++ include/clk.h | 11 +++++++++++ 2 files changed, 33 insertions(+)
diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c index da6624d4f2..85dae5321a 100644 --- a/drivers/clk/clk-uclass.c +++ b/drivers/clk/clk-uclass.c @@ -451,6 +451,28 @@ int clk_disable_bulk(struct clk_bulk *bulk) return 0; }
+int clk_get_by_id(ulong id, struct clk **clkp) +{ + struct udevice *dev; + struct uclass *uc; + int ret; + + ret = uclass_get(UCLASS_CLK, &uc); + if (ret) + return ret; + + uclass_foreach_dev(dev, uc) { + struct clk *clk = (struct clk *)dev_get_driver_data(dev); + + if (clk && clk->id == id) { + *clkp = clk; + return 0; + } + } + + return -ENOENT; +} + UCLASS_DRIVER(clk) = { .id = UCLASS_CLK, .name = "clk", diff --git a/include/clk.h b/include/clk.h index 98c3e12fb4..a4ecca9fbc 100644 --- a/include/clk.h +++ b/include/clk.h @@ -326,4 +326,15 @@ static inline bool clk_valid(struct clk *clk) { return !!clk->dev; } + +/** + * clk_get_by_id() - Get the clock by its ID + * + * @id: The clock ID to search for + * + * @clkp: A pointer to clock struct that has been found among added clocks + * to UCLASS_CLK + * @return zero on success, or -ENOENT on error + */ +int clk_get_by_id(ulong id, struct clk **clkp); #endif
Subject: [PATCH v3 08/11] dm: clk: Define clk_get_by_id() for clk operations
This commit adds the clk_get_by_id() function, which is responsible for getting the udevice with matching clk->id. Such approach allows re-usage of inherit DM list relationship for the same class (UCLASS_CLK). As a result - we don't need any other external list - it is just enough to look for UCLASS_CLK related udevices.
Signed-off-by: Lukasz Majewski lukma@denx.de
Changes in v3:
- Replace -ENODEV with -ENOENT
- Use **clkp instead of **c
drivers/clk/clk-uclass.c | 22 ++++++++++++++++++++++ include/clk.h | 11 +++++++++++ 2 files changed, 33 insertions(+)
diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c index da6624d4f2..85dae5321a 100644 --- a/drivers/clk/clk-uclass.c +++ b/drivers/clk/clk-uclass.c @@ -451,6 +451,28 @@ int clk_disable_bulk(struct clk_bulk *bulk) return 0; }
+int clk_get_by_id(ulong id, struct clk **clkp) {
- struct udevice *dev;
- struct uclass *uc;
- int ret;
- ret = uclass_get(UCLASS_CLK, &uc);
- if (ret)
return ret;- uclass_foreach_dev(dev, uc) {
struct clk *clk = (struct clk *)dev_get_driver_data(dev);if (clk && clk->id == id) {*clkp = clk;return 0;}- }
- return -ENOENT;
+}
UCLASS_DRIVER(clk) = { .id = UCLASS_CLK, .name = "clk", diff --git a/include/clk.h b/include/clk.h index 98c3e12fb4..a4ecca9fbc 100644 --- a/include/clk.h +++ b/include/clk.h @@ -326,4 +326,15 @@ static inline bool clk_valid(struct clk *clk) { return !!clk->dev; }
+/**
- clk_get_by_id() - Get the clock by its ID
- @id: The clock ID to search for
- @clkp: A pointer to clock struct that has been found among added
clocks
to UCLASS_CLK
- @return zero on success, or -ENOENT on error */ int
+clk_get_by_id(ulong id, struct clk **clkp);
Reviewed-by: Peng Fan peng.fan@nxp.com
#endif
2.11.0
This commit provides sandbox unit test for clk_get_by_id() method.
To test it default test clocks setup had to be adjusted to emulate structure similar to clocks in the Common Clock Framework [CCF] (for iMX devices).
The clk_get_by_id() relies on dev->driver_data having the pointer to struct clk.
Signed-off-by: Lukasz Majewski lukma@denx.de
---
Changes in v3: - New patch
arch/sandbox/include/asm/clk.h | 8 ++++++++ drivers/clk/clk_sandbox_test.c | 23 +++++++++++++++++++++++ test/dm/clk.c | 3 ++- 3 files changed, 33 insertions(+), 1 deletion(-)
diff --git a/arch/sandbox/include/asm/clk.h b/arch/sandbox/include/asm/clk.h index 2b1c49f783..90f925109f 100644 --- a/arch/sandbox/include/asm/clk.h +++ b/arch/sandbox/include/asm/clk.h @@ -63,6 +63,14 @@ int sandbox_clk_query_enable(struct udevice *dev, int id); */ int sandbox_clk_test_get(struct udevice *dev); /** + * sandbox_clk_test_get_by_id - Ask the sandbox clock test device to request its + * clocks by using clock id. + * + * @dev: The sandbox clock test (client) devivce. + * @return: 0 if OK, or a negative error code. + */ +int sandbox_clk_test_get_by_id(struct udevice *dev); +/** * sandbox_clk_test_get_bulk - Ask the sandbox clock test device to request its * clocks with the bulk clk API. * diff --git a/drivers/clk/clk_sandbox_test.c b/drivers/clk/clk_sandbox_test.c index e8465dbfad..4d276f55b9 100644 --- a/drivers/clk/clk_sandbox_test.c +++ b/drivers/clk/clk_sandbox_test.c @@ -34,6 +34,29 @@ int sandbox_clk_test_get(struct udevice *dev) return 0; }
+int sandbox_clk_test_get_by_id(struct udevice *dev) +{ + struct sandbox_clk_test *sbct = dev_get_priv(dev); + struct clk *clkp, *i2c_clk; + ulong driver_data_bkp; + const int id = 24; + int ret, id_bkp; + + i2c_clk = &sbct->clks[SANDBOX_CLK_TEST_ID_I2C]; + + id_bkp = i2c_clk->id; + i2c_clk->id = id; + driver_data_bkp = i2c_clk->dev->driver_data; + i2c_clk->dev->driver_data = (ulong)i2c_clk; + + ret = clk_get_by_id(id, &clkp); + + i2c_clk->id = id_bkp; + i2c_clk->dev->driver_data = driver_data_bkp; + + return ret; +} + int sandbox_clk_test_get_bulk(struct udevice *dev) { struct sandbox_clk_test *sbct = dev_get_priv(dev); diff --git a/test/dm/clk.c b/test/dm/clk.c index 112d5cbbc9..8b858cee01 100644 --- a/test/dm/clk.c +++ b/test/dm/clk.c @@ -99,8 +99,9 @@ static int dm_test_clk(struct unit_test_state *uts) ut_asserteq(0, sandbox_clk_query_enable(dev_clk, SANDBOX_CLK_ID_SPI)); ut_asserteq(0, sandbox_clk_query_enable(dev_clk, SANDBOX_CLK_ID_I2C));
- ut_assertok(sandbox_clk_test_free(dev_test)); + ut_asserteq(0, sandbox_clk_test_get_by_id(dev_test));
+ ut_assertok(sandbox_clk_test_free(dev_test)); return 0; } DM_TEST(dm_test_clk, DM_TESTF_SCAN_FDT);
This commit provides sandbox unit test for clk_get_parent_rate() method.
For testing the default test clocks setup had to be adjusted to emulate structure similar to clocks in the Common Clock Framework [CCF] (for iMX devices).
The clk_get_parent_rate() relies on dev->driver_data having the pointer to proper struct clk. It uses internally clk_get_parent() method also tested by this test.
Signed-off-by: Lukasz Majewski lukma@denx.de
---
Changes in v3: - New patch
arch/sandbox/include/asm/clk.h | 8 ++++++++ drivers/clk/clk_sandbox_test.c | 26 ++++++++++++++++++++++++++ test/dm/clk.c | 1 + 3 files changed, 35 insertions(+)
diff --git a/arch/sandbox/include/asm/clk.h b/arch/sandbox/include/asm/clk.h index 90f925109f..1df9534236 100644 --- a/arch/sandbox/include/asm/clk.h +++ b/arch/sandbox/include/asm/clk.h @@ -79,6 +79,14 @@ int sandbox_clk_test_get_by_id(struct udevice *dev); */ int sandbox_clk_test_get_bulk(struct udevice *dev); /** + * sandbox_clk_test_get_parent_rate - Ask the sandbox clock test device to + * query a passed clock's parent rate. + * + * @dev: The sandbox clock test (client) devivce. + * @return: The rate of the clock + */ +ulong sandbox_clk_test_get_parent_rate(struct udevice *dev); +/** * sandbox_clk_test_get_rate - Ask the sandbox clock test device to query a * clock's rate. * diff --git a/drivers/clk/clk_sandbox_test.c b/drivers/clk/clk_sandbox_test.c index 4d276f55b9..8ea15614c1 100644 --- a/drivers/clk/clk_sandbox_test.c +++ b/drivers/clk/clk_sandbox_test.c @@ -64,6 +64,32 @@ int sandbox_clk_test_get_bulk(struct udevice *dev) return clk_get_bulk(dev, &sbct->bulk); }
+ulong sandbox_clk_test_get_parent_rate(struct udevice *dev) +{ + struct sandbox_clk_test *sbct = dev_get_priv(dev); + struct clk *i2c_clk, *parent_clk; + struct udevice *parent_bkp; + ulong rate; + + parent_clk = &sbct->clks[SANDBOX_CLK_TEST_ID_FIXED]; + i2c_clk = &sbct->clks[SANDBOX_CLK_TEST_ID_I2C]; + + parent_clk->dev->driver_data = (ulong)parent_clk; + parent_bkp = i2c_clk->dev->parent; + i2c_clk->dev->parent = parent_clk->dev; + + rate = clk_get_parent_rate(i2c_clk); + + i2c_clk->dev->parent = parent_bkp; + parent_clk->dev->driver_data = 0; + + /* Check if cache'd value is correct */ + if (parent_clk->rate != 1234) + return 0; + + return rate; +} + ulong sandbox_clk_test_get_rate(struct udevice *dev, int id) { struct sandbox_clk_test *sbct = dev_get_priv(dev); diff --git a/test/dm/clk.c b/test/dm/clk.c index 8b858cee01..94f475bd8e 100644 --- a/test/dm/clk.c +++ b/test/dm/clk.c @@ -100,6 +100,7 @@ static int dm_test_clk(struct unit_test_state *uts) ut_asserteq(0, sandbox_clk_query_enable(dev_clk, SANDBOX_CLK_ID_I2C));
ut_asserteq(0, sandbox_clk_test_get_by_id(dev_test)); + ut_asserteq(1234, sandbox_clk_test_get_parent_rate(dev_test));
ut_assertok(sandbox_clk_test_free(dev_test)); return 0;
This commit brings the files from Linux kernel to provide clocks support as it is used on the Linux kernel with common clock framework [CCF] setup.
The directory structure has been preserved. The ported code only supports reading information from PLL, MUX, Divider, etc and enabling/disabling the clocks USDHCx/ECSPIx depending on used bus. Moreover, it is agnostic to the alias numbering as the information about the clock is read from device tree.
One needs to pay attention to the comments indicating necessary for U-boot's DM changes.
If needed the code can be extended to support the "set" part of the clock management.
Signed-off-by: Lukasz Majewski lukma@denx.de ---
Changes in v3: None
drivers/clk/Kconfig | 14 ++++ drivers/clk/Makefile | 2 + drivers/clk/clk-divider.c | 148 ++++++++++++++++++++++++++++++++++ drivers/clk/clk-fixed-factor.c | 87 ++++++++++++++++++++ drivers/clk/clk-mux.c | 164 +++++++++++++++++++++++++++++++++++++ drivers/clk/clk.c | 56 +++++++++++++ drivers/clk/imx/Kconfig | 9 +++ drivers/clk/imx/Makefile | 2 + drivers/clk/imx/clk-gate2.c | 113 ++++++++++++++++++++++++++ drivers/clk/imx/clk-imx6q.c | 179 +++++++++++++++++++++++++++++++++++++++++ drivers/clk/imx/clk-pfd.c | 91 +++++++++++++++++++++ drivers/clk/imx/clk-pllv3.c | 83 +++++++++++++++++++ drivers/clk/imx/clk.h | 75 +++++++++++++++++ include/linux/clk-provider.h | 94 ++++++++++++++++++++++ 14 files changed, 1117 insertions(+) create mode 100644 drivers/clk/clk-divider.c create mode 100644 drivers/clk/clk-fixed-factor.c create mode 100644 drivers/clk/clk-mux.c create mode 100644 drivers/clk/clk.c create mode 100644 drivers/clk/imx/clk-gate2.c create mode 100644 drivers/clk/imx/clk-imx6q.c create mode 100644 drivers/clk/imx/clk-pfd.c create mode 100644 drivers/clk/imx/clk-pllv3.c create mode 100644 drivers/clk/imx/clk.h create mode 100644 include/linux/clk-provider.h
diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig index ff60fc5c45..9df3bc731a 100644 --- a/drivers/clk/Kconfig +++ b/drivers/clk/Kconfig @@ -46,6 +46,20 @@ config CLK_BOSTON help Enable this to support the clocks
+config SPL_CLK_CCF + bool "SPL Common Clock Framework [CCF] support " + depends on SPL_CLK + help + Enable this option if you want to (re-)use the Linux kernel's Common + Clock Framework [CCF] code in U-Boot's SPL. + +config CLK_CCF + bool "Common Clock Framework [CCF] support " + depends on CLK + help + Enable this option if you want to (re-)use the Linux kernel's Common + Clock Framework [CCF] code in U-Boot's clock driver. + config CLK_STM32F bool "Enable clock driver support for STM32F family" depends on CLK && (STM32F7 || STM32F4) diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile index 1d9d725cae..9fcc75e0ea 100644 --- a/drivers/clk/Makefile +++ b/drivers/clk/Makefile @@ -7,6 +7,8 @@ obj-$(CONFIG_$(SPL_TPL_)CLK) += clk-uclass.o obj-$(CONFIG_$(SPL_TPL_)CLK) += clk_fixed_rate.o obj-$(CONFIG_$(SPL_TPL_)CLK) += clk_fixed_factor.o +obj-$(CONFIG_$(SPL_TPL_)CLK_CCF) += clk.o clk-divider.o clk-mux.o +obj-$(CONFIG_$(SPL_TPL_)CLK_CCF) += clk-fixed-factor.o
obj-y += imx/ obj-y += tegra/ diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c new file mode 100644 index 0000000000..3841d8bfbb --- /dev/null +++ b/drivers/clk/clk-divider.c @@ -0,0 +1,148 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2019 DENX Software Engineering + * Lukasz Majewski, DENX Software Engineering, lukma@denx.de + * + * Copyright (C) 2011 Sascha Hauer, Pengutronix s.hauer@pengutronix.de + * Copyright (C) 2011 Richard Zhao, Linaro richard.zhao@linaro.org + * Copyright (C) 2011-2012 Mike Turquette, Linaro Ltd mturquette@linaro.org + * + */ + +#include <common.h> +#include <asm/io.h> +#include <malloc.h> +#include <clk-uclass.h> +#include <dm/device.h> +#include <dm/uclass.h> +#include <dm/lists.h> +#include <dm/device-internal.h> +#include <linux/clk-provider.h> +#include <div64.h> +#include <clk.h> +#include "clk.h" + +#define UBOOT_DM_CLK_CCF_DIVIDER "ccf_clk_divider" + +static unsigned int _get_table_div(const struct clk_div_table *table, + unsigned int val) +{ + const struct clk_div_table *clkt; + + for (clkt = table; clkt->div; clkt++) + if (clkt->val == val) + return clkt->div; + return 0; +} + +static unsigned int _get_div(const struct clk_div_table *table, + unsigned int val, unsigned long flags, u8 width) +{ + if (flags & CLK_DIVIDER_ONE_BASED) + return val; + if (flags & CLK_DIVIDER_POWER_OF_TWO) + return 1 << val; + if (flags & CLK_DIVIDER_MAX_AT_ZERO) + return val ? val : clk_div_mask(width) + 1; + if (table) + return _get_table_div(table, val); + return val + 1; +} + +unsigned long divider_recalc_rate(struct clk *hw, unsigned long parent_rate, + unsigned int val, + const struct clk_div_table *table, + unsigned long flags, unsigned long width) +{ + unsigned int div; + + div = _get_div(table, val, flags, width); + if (!div) { + WARN(!(flags & CLK_DIVIDER_ALLOW_ZERO), + "%s: Zero divisor and CLK_DIVIDER_ALLOW_ZERO not set\n", + clk_hw_get_name(hw)); + return parent_rate; + } + + return DIV_ROUND_UP_ULL((u64)parent_rate, div); +} + +static ulong clk_divider_recalc_rate(struct clk *clk) +{ + struct clk_divider *divider = + (struct clk_divider *)dev_get_driver_data(clk->dev); + unsigned long parent_rate = clk_get_parent_rate(clk); + unsigned int val; + + val = readl(divider->reg) >> divider->shift; + val &= clk_div_mask(divider->width); + + return divider_recalc_rate(clk, parent_rate, val, divider->table, + divider->flags, divider->width); +} + +const struct clk_ops clk_divider_ops = { + .get_rate = clk_divider_recalc_rate, +}; + +static struct clk *_register_divider(struct device *dev, const char *name, + const char *parent_name, unsigned long flags, + void __iomem *reg, u8 shift, u8 width, + u8 clk_divider_flags, const struct clk_div_table *table) +{ + struct clk_divider *div; + struct clk *clk; + int ret; + + if (clk_divider_flags & CLK_DIVIDER_HIWORD_MASK) { + if (width + shift > 16) { + pr_warn("divider value exceeds LOWORD field\n"); + return ERR_PTR(-EINVAL); + } + } + + /* allocate the divider */ + div = kzalloc(sizeof(*div), GFP_KERNEL); + if (!div) + return ERR_PTR(-ENOMEM); + + /* struct clk_divider assignments */ + div->reg = reg; + div->shift = shift; + div->width = width; + div->flags = clk_divider_flags; + div->table = table; + + /* register the clock */ + clk = &div->clk; + + ret = clk_register(clk, UBOOT_DM_CLK_CCF_DIVIDER, (ulong)clk, + name, parent_name); + if (ret) { + kfree(div); + return ERR_PTR(ret); + } + + return clk; +} + +struct clk *clk_register_divider(struct device *dev, const char *name, + const char *parent_name, unsigned long flags, + void __iomem *reg, u8 shift, u8 width, + u8 clk_divider_flags) +{ + struct clk *clk; + + clk = _register_divider(dev, name, parent_name, flags, reg, shift, + width, clk_divider_flags, NULL); + if (IS_ERR(clk)) + return ERR_CAST(clk); + return clk; +} + +U_BOOT_DRIVER(ccf_clk_divider) = { + .name = UBOOT_DM_CLK_CCF_DIVIDER, + .id = UCLASS_CLK, + .ops = &clk_divider_ops, + .flags = DM_FLAG_PRE_RELOC, +}; diff --git a/drivers/clk/clk-fixed-factor.c b/drivers/clk/clk-fixed-factor.c new file mode 100644 index 0000000000..acbc0909b4 --- /dev/null +++ b/drivers/clk/clk-fixed-factor.c @@ -0,0 +1,87 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2019 DENX Software Engineering + * Lukasz Majewski, DENX Software Engineering, lukma@denx.de + * + * Copyright (C) 2011 Sascha Hauer, Pengutronix s.hauer@pengutronix.de + */ +#include <common.h> +#include <malloc.h> +#include <clk-uclass.h> +#include <dm/device.h> +#include <linux/clk-provider.h> +#include <div64.h> +#include <clk.h> +#include "clk.h" + +#define UBOOT_DM_CLK_IMX_FIXED_FACTOR "ccf_clk_fixed_factor" + +static ulong clk_factor_recalc_rate(struct clk *clk) +{ + struct clk_fixed_factor *fix = + (struct clk_fixed_factor *)dev_get_driver_data(clk->dev); + unsigned long parent_rate = clk_get_parent_rate(clk); + unsigned long long int rate; + + rate = (unsigned long long int)parent_rate * fix->mult; + do_div(rate, fix->div); + return (ulong)rate; +} + +const struct clk_ops ccf_clk_fixed_factor_ops = { + .get_rate = clk_factor_recalc_rate, +}; + +struct clk *clk_hw_register_fixed_factor(struct device *dev, + const char *name, const char *parent_name, unsigned long flags, + unsigned int mult, unsigned int div) +{ + struct clk_fixed_factor *fix; + struct clk *clk; + int ret; + + fix = kmalloc(sizeof(*fix), GFP_KERNEL); + if (!fix) + return ERR_PTR(-ENOMEM); + + /* struct clk_fixed_factor assignments */ + fix->mult = mult; + fix->div = div; + clk = &fix->clk; + + /* + * We pass the struct clk *clk pointer (which is the same as + * clk_fixed_factor *fix - by the struct elements alignment) to DM as a + * driver_data, so it can be easily accessible from the udevice level. + * Moreover, the struct clk is only a wrapper on udevice which + * corresponds to the "real" clock device. + */ + ret = clk_register(clk, UBOOT_DM_CLK_IMX_FIXED_FACTOR, (ulong)clk, + name, parent_name); + if (ret) { + kfree(fix); + return ERR_PTR(ret); + } + + return clk; +} + +struct clk *clk_register_fixed_factor(struct device *dev, const char *name, + const char *parent_name, unsigned long flags, + unsigned int mult, unsigned int div) +{ + struct clk *clk; + + clk = clk_hw_register_fixed_factor(dev, name, parent_name, flags, mult, + div); + if (IS_ERR(clk)) + return ERR_CAST(clk); + return clk; +} + +U_BOOT_DRIVER(imx_clk_fixed_factor) = { + .name = UBOOT_DM_CLK_IMX_FIXED_FACTOR, + .id = UCLASS_CLK, + .ops = &ccf_clk_fixed_factor_ops, + .flags = DM_FLAG_PRE_RELOC, +}; diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c new file mode 100644 index 0000000000..2c85f2052c --- /dev/null +++ b/drivers/clk/clk-mux.c @@ -0,0 +1,164 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2019 DENX Software Engineering + * Lukasz Majewski, DENX Software Engineering, lukma@denx.de + * + * Copyright (C) 2011 Sascha Hauer, Pengutronix s.hauer@pengutronix.de + * Copyright (C) 2011 Richard Zhao, Linaro richard.zhao@linaro.org + * Copyright (C) 2011-2012 Mike Turquette, Linaro Ltd mturquette@linaro.org + * + * Simple multiplexer clock implementation + */ + +/* + * U-Boot CCF porting node: + * + * The Linux kernel - as of tag: 5.0-rc3 is using also the imx_clk_fixup_mux() + * version of CCF mux. It is used on e.g. imx6q to provide fixes (like + * imx_cscmr1_fixup) for broken HW. + * + * At least for IMX6Q (but NOT IMX6QP) it is important when we set the parent + * clock. + */ + +#include <common.h> +#include <asm/io.h> +#include <malloc.h> +#include <clk-uclass.h> +#include <dm/device.h> +#include <linux/clk-provider.h> +#include <clk.h> +#include "clk.h" + +#define UBOOT_DM_CLK_CCF_MUX "ccf_clk_mux" + +int clk_mux_val_to_index(struct clk *clk, u32 *table, unsigned int flags, + unsigned int val) +{ + struct clk_mux *mux = to_clk_mux(clk); + int num_parents = mux->num_parents; + + if (table) { + int i; + + for (i = 0; i < num_parents; i++) + if (table[i] == val) + return i; + return -EINVAL; + } + + if (val && (flags & CLK_MUX_INDEX_BIT)) + val = ffs(val) - 1; + + if (val && (flags & CLK_MUX_INDEX_ONE)) + val--; + + if (val >= num_parents) + return -EINVAL; + + return val; +} + +static u8 clk_mux_get_parent(struct clk *clk) +{ + struct clk_mux *mux = to_clk_mux(clk); + u32 val; + + val = readl(mux->reg) >> mux->shift; + val &= mux->mask; + + return clk_mux_val_to_index(clk, mux->table, mux->flags, val); +} + +const struct clk_ops clk_mux_ops = { + .get_rate = clk_generic_get_rate, +}; + +struct clk *clk_hw_register_mux_table(struct device *dev, const char *name, + const char * const *parent_names, u8 num_parents, + unsigned long flags, + void __iomem *reg, u8 shift, u32 mask, + u8 clk_mux_flags, u32 *table) +{ + struct clk_mux *mux; + struct clk *clk; + u8 width = 0; + int ret; + + if (clk_mux_flags & CLK_MUX_HIWORD_MASK) { + width = fls(mask) - ffs(mask) + 1; + if (width + shift > 16) { + pr_err("mux value exceeds LOWORD field\n"); + return ERR_PTR(-EINVAL); + } + } + + /* allocate the mux */ + mux = kzalloc(sizeof(*mux), GFP_KERNEL); + if (!mux) + return ERR_PTR(-ENOMEM); + + /* U-boot specific assignments */ + mux->parent_names = parent_names; + mux->num_parents = num_parents; + + /* struct clk_mux assignments */ + mux->reg = reg; + mux->shift = shift; + mux->mask = mask; + mux->flags = clk_mux_flags; + mux->table = table; + + clk = &mux->clk; + + /* + * Read the current mux setup - so we assign correct parent. + * + * Changing parent would require changing internals of udevice struct + * for the corresponding clock (to do that define .set_parent() method. + */ + ret = clk_register(clk, UBOOT_DM_CLK_CCF_MUX, (ulong)clk, name, + parent_names[clk_mux_get_parent(clk)]); + if (ret) { + kfree(mux); + return ERR_PTR(ret); + } + + return clk; +} + +struct clk *clk_register_mux_table(struct device *dev, const char *name, + const char * const *parent_names, u8 num_parents, + unsigned long flags, + void __iomem *reg, u8 shift, u32 mask, + u8 clk_mux_flags, u32 *table) +{ + struct clk *clk; + + clk = clk_hw_register_mux_table(dev, name, parent_names, num_parents, + flags, reg, shift, mask, clk_mux_flags, + table); + if (IS_ERR(clk)) + return ERR_CAST(clk); + return clk; +} + +struct clk *clk_register_mux(struct device *dev, const char *name, + const char * const *parent_names, u8 num_parents, + unsigned long flags, + void __iomem *reg, u8 shift, u8 width, + u8 clk_mux_flags) +{ + u32 mask = BIT(width) - 1; + + return clk_register_mux_table(dev, name, parent_names, num_parents, + flags, reg, shift, mask, clk_mux_flags, + NULL); +} + +U_BOOT_DRIVER(ccf_clk_mux) = { + .name = UBOOT_DM_CLK_CCF_MUX, + .id = UCLASS_CLK, + .ops = &clk_mux_ops, + .flags = DM_FLAG_PRE_RELOC, +}; diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c new file mode 100644 index 0000000000..0a0fffb50b --- /dev/null +++ b/drivers/clk/clk.c @@ -0,0 +1,56 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (C) 2019 DENX Software Engineering + * Lukasz Majewski, DENX Software Engineering, lukma@denx.de + */ + +#include <common.h> +#include <clk-uclass.h> +#include <dm/device.h> +#include <dm/uclass.h> +#include <dm/lists.h> +#include <dm/device-internal.h> +#include <clk.h> + +int clk_register(struct clk *clk, const char *drv_name, + ulong drv_data, const char *name, + const char *parent_name) +{ + struct udevice *parent; + struct driver *drv; + int ret; + + ret = uclass_get_device_by_name(UCLASS_CLK, parent_name, &parent); + if (ret) + printf("%s: UCLASS parent: 0x%p\n", __func__, parent); + + debug("%s: name: %s parent: %s [0x%p]\n", __func__, name, parent->name, + parent); + + drv = lists_driver_lookup_name(drv_name); + if (!drv) { + printf("%s: %s is not a valid driver name\n", + __func__, drv_name); + return -ENOENT; + } + + ret = device_bind_with_driver_data(parent, drv, name, drv_data, + ofnode_null(), &clk->dev); + if (ret) { + printf("%s: CLK: %s driver bind error [%d]!\n", __func__, name, + ret); + return ret; + } + + return 0; +} + +ulong clk_generic_get_rate(struct clk *clk) +{ + return clk_get_parent_rate(clk); +} + +const char *clk_hw_get_name(const struct clk *hw) +{ + return hw->dev->name; +} diff --git a/drivers/clk/imx/Kconfig b/drivers/clk/imx/Kconfig index a6fb58d6cf..469768b5c3 100644 --- a/drivers/clk/imx/Kconfig +++ b/drivers/clk/imx/Kconfig @@ -1,3 +1,12 @@ +config CLK_IMX6Q + bool "Clock support for i.MX6Q" + depends on ARCH_MX6 + select CLK + select CLK_CCF + select SPL_CLK_CCF + help + This enables DM/DTS support for clock driver in i.MX6Q platforms. + config CLK_IMX8 bool "Clock support for i.MX8" depends on ARCH_IMX8 diff --git a/drivers/clk/imx/Makefile b/drivers/clk/imx/Makefile index 5505ae52e2..beba3bff39 100644 --- a/drivers/clk/imx/Makefile +++ b/drivers/clk/imx/Makefile @@ -2,4 +2,6 @@ # # SPDX-License-Identifier: GPL-2.0
+obj-$(CONFIG_$(SPL_TPL_)CLK_CCF) += clk-gate2.o clk-pllv3.o clk-pfd.o +obj-$(CONFIG_CLK_IMX6Q) += clk-imx6q.o obj-$(CONFIG_CLK_IMX8) += clk-imx8.o diff --git a/drivers/clk/imx/clk-gate2.c b/drivers/clk/imx/clk-gate2.c new file mode 100644 index 0000000000..1e53e4f9db --- /dev/null +++ b/drivers/clk/imx/clk-gate2.c @@ -0,0 +1,113 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (C) 2019 DENX Software Engineering + * Lukasz Majewski, DENX Software Engineering, lukma@denx.de + * + * Copyright (C) 2010-2011 Canonical Ltd jeremy.kerr@canonical.com + * Copyright (C) 2011-2012 Mike Turquette, Linaro Ltd mturquette@linaro.org + * + * 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. + * + * Gated clock implementation + * + */ + +#include <common.h> +#include <asm/io.h> +#include <malloc.h> +#include <clk-uclass.h> +#include <dm/device.h> +#include <linux/clk-provider.h> +#include <clk.h> +#include "clk.h" + +#define UBOOT_DM_CLK_IMX_GATE2 "imx_clk_gate2" + +struct clk_gate2 { + struct clk clk; + void __iomem *reg; + u8 bit_idx; + u8 cgr_val; + u8 flags; +}; + +#define to_clk_gate2(_clk) container_of(_clk, struct clk_gate2, clk) + +static int clk_gate2_enable(struct clk *clk) +{ + struct clk_gate2 *gate = + (struct clk_gate2 *)dev_get_driver_data(clk->dev); + u32 reg; + + reg = readl(gate->reg); + reg &= ~(3 << gate->bit_idx); + reg |= gate->cgr_val << gate->bit_idx; + writel(reg, gate->reg); + + return 0; +} + +static int clk_gate2_disable(struct clk *clk) +{ + struct clk_gate2 *gate = + (struct clk_gate2 *)dev_get_driver_data(clk->dev); + u32 reg; + + reg = readl(gate->reg); + reg &= ~(3 << gate->bit_idx); + writel(reg, gate->reg); + + return 0; +} + +static const struct clk_ops clk_gate2_ops = { + .enable = clk_gate2_enable, + .disable = clk_gate2_disable, + .get_rate = clk_generic_get_rate, +}; + +struct clk *clk_register_gate2(struct device *dev, const char *name, + const char *parent_name, unsigned long flags, + void __iomem *reg, u8 bit_idx, u8 cgr_val, + u8 clk_gate2_flags) +{ + struct clk_gate2 *gate; + struct clk *clk; + int ret; + + gate = kzalloc(sizeof(*gate), GFP_KERNEL); + if (!gate) + return ERR_PTR(-ENOMEM); + + /* struct clk_gate2 assignments */ + gate->reg = reg; + gate->bit_idx = bit_idx; + gate->cgr_val = cgr_val; + gate->flags = clk_gate2_flags; + + /* + * U-boot DM adjustments: + * + * clk and gate reslove to the same address - lets pass clock + * for better readability. + */ + clk = &gate->clk; + + ret = clk_register(clk, UBOOT_DM_CLK_IMX_GATE2, (ulong)clk, + name, parent_name); + if (ret) { + kfree(gate); + return ERR_PTR(ret); + } + + return clk; +} + +U_BOOT_DRIVER(clk_gate2) = { + .name = UBOOT_DM_CLK_IMX_GATE2, + .id = UCLASS_CLK, + .ops = &clk_gate2_ops, + .flags = DM_FLAG_PRE_RELOC, +}; diff --git a/drivers/clk/imx/clk-imx6q.c b/drivers/clk/imx/clk-imx6q.c new file mode 100644 index 0000000000..92e9337d44 --- /dev/null +++ b/drivers/clk/imx/clk-imx6q.c @@ -0,0 +1,179 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2019 DENX Software Engineering + * Lukasz Majewski, DENX Software Engineering, lukma@denx.de + */ + +#include <common.h> +#include <clk-uclass.h> +#include <dm.h> +#include <asm/arch/clock.h> +#include <asm/arch/imx-regs.h> +#include <dt-bindings/clock/imx6qdl-clock.h> + +#include "clk.h" + +static int imx6q_check_id(ulong id) +{ + if (id < IMX6QDL_CLK_DUMMY || id >= IMX6QDL_CLK_END) { + printf("%s: Invalid clk ID #%lu\n", __func__, id); + return -EINVAL; + } + + return 0; +} + +static ulong imx6q_clk_get_rate(struct clk *clk) +{ + struct clk *c; + int ret; + + debug("%s(#%lu)\n", __func__, clk->id); + + ret = imx6q_check_id(clk->id); + if (ret) + return ret; + + ret = clk_get_by_id(clk->id, &c); + if (ret) + return ret; + + return clk_get_rate(c); +} + +static ulong imx6q_clk_set_rate(struct clk *clk, unsigned long rate) +{ + debug("%s(#%lu), rate: %lu\n", __func__, clk->id, rate); + + return rate; +} + +static int __imx6q_clk_enable(struct clk *clk, bool enable) +{ + struct clk *c; + int ret = 0; + + debug("%s(#%lu) en: %d\n", __func__, clk->id, enable); + + ret = imx6q_check_id(clk->id); + if (ret) + return ret; + + ret = clk_get_by_id(clk->id, &c); + if (ret) + return ret; + + if (enable) + ret = clk_enable(c); + else + ret = clk_disable(c); + + return ret; +} + +static int imx6q_clk_disable(struct clk *clk) +{ + return __imx6q_clk_enable(clk, 0); +} + +static int imx6q_clk_enable(struct clk *clk) +{ + return __imx6q_clk_enable(clk, 1); +} + +static struct clk_ops imx6q_clk_ops = { + .set_rate = imx6q_clk_set_rate, + .get_rate = imx6q_clk_get_rate, + .enable = imx6q_clk_enable, + .disable = imx6q_clk_disable, +}; + +static const char *const usdhc_sels[] = { "pll2_pfd2_396m", "pll2_pfd0_352m", }; + +static int imx6q_clk_probe(struct udevice *dev) +{ + void *base; + + /* Anatop clocks */ + base = (void *)ANATOP_BASE_ADDR; + + clk_dm(IMX6QDL_CLK_PLL2, + imx_clk_pllv3(IMX_PLLV3_GENERIC, "pll2_bus", "osc", + base + 0x30, 0x1)); + clk_dm(IMX6QDL_CLK_PLL3_USB_OTG, + imx_clk_pllv3(IMX_PLLV3_USB, "pll3_usb_otg", "osc", + base + 0x10, 0x3)); + clk_dm(IMX6QDL_CLK_PLL3_60M, + imx_clk_fixed_factor("pll3_60m", "pll3_usb_otg", 1, 8)); + clk_dm(IMX6QDL_CLK_PLL2_PFD0_352M, + imx_clk_pfd("pll2_pfd0_352m", "pll2_bus", base + 0x100, 0)); + clk_dm(IMX6QDL_CLK_PLL2_PFD2_396M, + imx_clk_pfd("pll2_pfd2_396m", "pll2_bus", base + 0x100, 2)); + + /* CCM clocks */ + base = dev_read_addr_ptr(dev); + if (base == (void *)FDT_ADDR_T_NONE) + return -EINVAL; + + clk_dm(IMX6QDL_CLK_USDHC1_SEL, + imx_clk_mux("usdhc1_sel", base + 0x1c, 16, 1, + usdhc_sels, ARRAY_SIZE(usdhc_sels))); + clk_dm(IMX6QDL_CLK_USDHC2_SEL, + imx_clk_mux("usdhc2_sel", base + 0x1c, 17, 1, + usdhc_sels, ARRAY_SIZE(usdhc_sels))); + clk_dm(IMX6QDL_CLK_USDHC3_SEL, + imx_clk_mux("usdhc3_sel", base + 0x1c, 18, 1, + usdhc_sels, ARRAY_SIZE(usdhc_sels))); + clk_dm(IMX6QDL_CLK_USDHC4_SEL, + imx_clk_mux("usdhc4_sel", base + 0x1c, 19, 1, + usdhc_sels, ARRAY_SIZE(usdhc_sels))); + + clk_dm(IMX6QDL_CLK_USDHC1_PODF, + imx_clk_divider("usdhc1_podf", "usdhc1_sel", + base + 0x24, 11, 3)); + clk_dm(IMX6QDL_CLK_USDHC2_PODF, + imx_clk_divider("usdhc2_podf", "usdhc2_sel", + base + 0x24, 16, 3)); + clk_dm(IMX6QDL_CLK_USDHC3_PODF, + imx_clk_divider("usdhc3_podf", "usdhc3_sel", + base + 0x24, 19, 3)); + clk_dm(IMX6QDL_CLK_USDHC4_PODF, + imx_clk_divider("usdhc4_podf", "usdhc4_sel", + base + 0x24, 22, 3)); + + clk_dm(IMX6QDL_CLK_ECSPI_ROOT, + imx_clk_divider("ecspi_root", "pll3_60m", base + 0x38, 19, 6)); + + clk_dm(IMX6QDL_CLK_ECSPI1, + imx_clk_gate2("ecspi1", "ecspi_root", base + 0x6c, 0)); + clk_dm(IMX6QDL_CLK_ECSPI2, + imx_clk_gate2("ecspi2", "ecspi_root", base + 0x6c, 2)); + clk_dm(IMX6QDL_CLK_ECSPI3, + imx_clk_gate2("ecspi3", "ecspi_root", base + 0x6c, 4)); + clk_dm(IMX6QDL_CLK_ECSPI4, + imx_clk_gate2("ecspi4", "ecspi_root", base + 0x6c, 6)); + clk_dm(IMX6QDL_CLK_USDHC1, + imx_clk_gate2("usdhc1", "usdhc1_podf", base + 0x80, 2)); + clk_dm(IMX6QDL_CLK_USDHC2, + imx_clk_gate2("usdhc2", "usdhc2_podf", base + 0x80, 4)); + clk_dm(IMX6QDL_CLK_USDHC3, + imx_clk_gate2("usdhc3", "usdhc3_podf", base + 0x80, 6)); + clk_dm(IMX6QDL_CLK_USDHC4, + imx_clk_gate2("usdhc4", "usdhc4_podf", base + 0x80, 8)); + + return 0; +} + +static const struct udevice_id imx6q_clk_ids[] = { + { .compatible = "fsl,imx6q-ccm" }, + { }, +}; + +U_BOOT_DRIVER(imx6q_clk) = { + .name = "clk_imx6q", + .id = UCLASS_CLK, + .of_match = imx6q_clk_ids, + .ops = &imx6q_clk_ops, + .probe = imx6q_clk_probe, + .flags = DM_FLAG_PRE_RELOC, +}; diff --git a/drivers/clk/imx/clk-pfd.c b/drivers/clk/imx/clk-pfd.c new file mode 100644 index 0000000000..2293d481d4 --- /dev/null +++ b/drivers/clk/imx/clk-pfd.c @@ -0,0 +1,91 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (C) 2019 DENX Software Engineering + * Lukasz Majewski, DENX Software Engineering, lukma@denx.de + * + * Copyright 2012 Freescale Semiconductor, Inc. + * Copyright 2012 Linaro Ltd. + * + * The code contained herein is licensed under the GNU General Public + * License. You may obtain a copy of the GNU General Public License + * Version 2 or later at the following locations: + * + * http://www.opensource.org/licenses/gpl-license.html + * http://www.gnu.org/copyleft/gpl.html + */ + +#include <common.h> +#include <asm/io.h> +#include <malloc.h> +#include <clk-uclass.h> +#include <dm/device.h> +#include <linux/clk-provider.h> +#include <div64.h> +#include <clk.h> +#include "clk.h" + +#define UBOOT_DM_CLK_IMX_PFD "imx_clk_pfd" + +struct clk_pfd { + struct clk clk; + void __iomem *reg; + u8 idx; +}; + +#define to_clk_pfd(_clk) container_of(_clk, struct clk_pfd, clk) + +#define SET 0x4 +#define CLR 0x8 +#define OTG 0xc + +static unsigned long clk_pfd_recalc_rate(struct clk *clk) +{ + struct clk_pfd *pfd = + (struct clk_pfd *)dev_get_driver_data(clk->dev); + unsigned long parent_rate = clk_get_parent_rate(clk); + u64 tmp = parent_rate; + u8 frac = (readl(pfd->reg) >> (pfd->idx * 8)) & 0x3f; + + tmp *= 18; + do_div(tmp, frac); + + return tmp; +} + +static const struct clk_ops clk_pfd_ops = { + .get_rate = clk_pfd_recalc_rate, +}; + +struct clk *imx_clk_pfd(const char *name, const char *parent_name, + void __iomem *reg, u8 idx) +{ + struct clk_pfd *pfd; + struct clk *clk; + int ret; + + pfd = kzalloc(sizeof(*pfd), GFP_KERNEL); + if (!pfd) + return ERR_PTR(-ENOMEM); + + pfd->reg = reg; + pfd->idx = idx; + + /* register the clock */ + clk = &pfd->clk; + + ret = clk_register(clk, UBOOT_DM_CLK_IMX_PFD, (ulong)clk, + name, parent_name); + if (ret) { + kfree(pfd); + return ERR_PTR(ret); + } + + return clk; +} + +U_BOOT_DRIVER(clk_pfd) = { + .name = UBOOT_DM_CLK_IMX_PFD, + .id = UCLASS_CLK, + .ops = &clk_pfd_ops, + .flags = DM_FLAG_PRE_RELOC, +}; diff --git a/drivers/clk/imx/clk-pllv3.c b/drivers/clk/imx/clk-pllv3.c new file mode 100644 index 0000000000..3fe9b7c03d --- /dev/null +++ b/drivers/clk/imx/clk-pllv3.c @@ -0,0 +1,83 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (C) 2019 DENX Software Engineering + * Lukasz Majewski, DENX Software Engineering, lukma@denx.de + */ + +#include <common.h> +#include <asm/io.h> +#include <malloc.h> +#include <clk-uclass.h> +#include <dm/device.h> +#include <dm/uclass.h> +#include <clk.h> +#include "clk.h" + +#define UBOOT_DM_CLK_IMX_PLLV3 "imx_clk_pllv3" + +struct clk_pllv3 { + struct clk clk; + void __iomem *base; + u32 div_mask; + u32 div_shift; +}; + +#define to_clk_pllv3(_clk) container_of(_clk, struct clk_pllv3, clk) + +static ulong clk_pllv3_get_rate(struct clk *clk) +{ + struct clk_pllv3 *pll = + (struct clk_pllv3 *)dev_get_driver_data(clk->dev); + unsigned long parent_rate = clk_get_parent_rate(clk); + + u32 div = (readl(pll->base) >> pll->div_shift) & pll->div_mask; + + return (div == 1) ? parent_rate * 22 : parent_rate * 20; +} + +static const struct clk_ops clk_pllv3_generic_ops = { + .get_rate = clk_pllv3_get_rate, +}; + +struct clk *imx_clk_pllv3(enum imx_pllv3_type type, const char *name, + const char *parent_name, void __iomem *base, + u32 div_mask) +{ + struct clk_pllv3 *pll; + struct clk *clk; + char *drv_name; + int ret; + + pll = kzalloc(sizeof(*pll), GFP_KERNEL); + if (!pll) + return ERR_PTR(-ENOMEM); + + switch (type) { + case IMX_PLLV3_GENERIC: + case IMX_PLLV3_USB: + drv_name = UBOOT_DM_CLK_IMX_PLLV3; + break; + default: + kfree(pll); + return ERR_PTR(-ENOTSUPP); + } + + pll->base = base; + pll->div_mask = div_mask; + clk = &pll->clk; + + ret = clk_register(clk, drv_name, (ulong)clk, name, parent_name); + if (ret) { + kfree(pll); + return ERR_PTR(ret); + } + + return clk; +} + +U_BOOT_DRIVER(clk_pllv3_generic) = { + .name = UBOOT_DM_CLK_IMX_PLLV3, + .id = UCLASS_CLK, + .ops = &clk_pllv3_generic_ops, + .flags = DM_FLAG_PRE_RELOC, +}; diff --git a/drivers/clk/imx/clk.h b/drivers/clk/imx/clk.h new file mode 100644 index 0000000000..864a215a22 --- /dev/null +++ b/drivers/clk/imx/clk.h @@ -0,0 +1,75 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (C) 2019 DENX Software Engineering + * Lukasz Majewski, DENX Software Engineering, lukma@denx.de + */ +#ifndef __MACH_IMX_CLK_H +#define __MACH_IMX_CLK_H + +#include <linux/clk-provider.h> + +static inline void clk_dm(ulong id, struct clk *clk) +{ + if (!IS_ERR(clk)) + clk->id = id; +} + +enum imx_pllv3_type { + IMX_PLLV3_GENERIC, + IMX_PLLV3_SYS, + IMX_PLLV3_USB, + IMX_PLLV3_USB_VF610, + IMX_PLLV3_AV, + IMX_PLLV3_ENET, + IMX_PLLV3_ENET_IMX7, + IMX_PLLV3_SYS_VF610, + IMX_PLLV3_DDR_IMX7, +}; + +struct clk *clk_register_gate2(struct device *dev, const char *name, + const char *parent_name, unsigned long flags, + void __iomem *reg, u8 bit_idx, u8 cgr_val, + u8 clk_gate_flags); + +struct clk *imx_clk_pllv3(enum imx_pllv3_type type, const char *name, + const char *parent_name, void __iomem *base, + u32 div_mask); + +static inline struct clk *imx_clk_gate2(const char *name, const char *parent, + void __iomem *reg, u8 shift) +{ + return clk_register_gate2(NULL, name, parent, CLK_SET_RATE_PARENT, reg, + shift, 0x3, 0); +} + +static inline struct clk *imx_clk_fixed_factor(const char *name, + const char *parent, unsigned int mult, unsigned int div) +{ + return clk_register_fixed_factor(NULL, name, parent, + CLK_SET_RATE_PARENT, mult, div); +} + +static inline struct clk *imx_clk_divider(const char *name, const char *parent, + void __iomem *reg, u8 shift, u8 width) +{ + return clk_register_divider(NULL, name, parent, CLK_SET_RATE_PARENT, + reg, shift, width, 0); +} + +struct clk *imx_clk_pfd(const char *name, const char *parent_name, + void __iomem *reg, u8 idx); + +struct clk *imx_clk_fixup_mux(const char *name, void __iomem *reg, + u8 shift, u8 width, const char * const *parents, + int num_parents, void (*fixup)(u32 *val)); + +static inline struct clk *imx_clk_mux(const char *name, void __iomem *reg, + u8 shift, u8 width, const char * const *parents, + int num_parents) +{ + return clk_register_mux(NULL, name, parents, num_parents, + CLK_SET_RATE_NO_REPARENT, reg, shift, + width, 0); +} + +#endif /* __MACH_IMX_CLK_H */ diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h new file mode 100644 index 0000000000..eac045c5f8 --- /dev/null +++ b/include/linux/clk-provider.h @@ -0,0 +1,94 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (C) 2019 DENX Software Engineering + * Lukasz Majewski, DENX Software Engineering, lukma@denx.de + * + * Copyright (c) 2010-2011 Jeremy Kerr jeremy.kerr@canonical.com + * Copyright (C) 2011-2012 Linaro Ltd mturquette@linaro.org + */ +#ifndef __LINUX_CLK_PROVIDER_H +#define __LINUX_CLK_PROVIDER_H + +#define CLK_SET_RATE_PARENT BIT(2) /* propagate rate change up one level */ +#define CLK_SET_RATE_NO_REPARENT BIT(7) /* don't re-parent on rate change */ + +#define CLK_MUX_INDEX_ONE BIT(0) +#define CLK_MUX_INDEX_BIT BIT(1) +#define CLK_MUX_HIWORD_MASK BIT(2) +#define CLK_MUX_READ_ONLY BIT(3) /* mux can't be changed */ +#define CLK_MUX_ROUND_CLOSEST BIT(4) + +struct clk_mux { + struct clk clk; + void __iomem *reg; + u32 *table; + u32 mask; + u8 shift; + u8 flags; + + /* + * Fields from struct clk_init_data - this struct has been + * omitted to avoid too deep level of CCF for bootloader + */ + const char * const *parent_names; + u8 num_parents; +}; + +#define to_clk_mux(_clk) container_of(_clk, struct clk_mux, clk) + +struct clk_div_table { + unsigned int val; + unsigned int div; +}; + +struct clk_divider { + struct clk clk; + void __iomem *reg; + u8 shift; + u8 width; + u8 flags; + const struct clk_div_table *table; +}; + +#define clk_div_mask(width) ((1 << (width)) - 1) +#define to_clk_divider(_clk) container_of(_clk, struct clk_divider, clk) + +#define CLK_DIVIDER_ONE_BASED BIT(0) +#define CLK_DIVIDER_POWER_OF_TWO BIT(1) +#define CLK_DIVIDER_ALLOW_ZERO BIT(2) +#define CLK_DIVIDER_HIWORD_MASK BIT(3) +#define CLK_DIVIDER_ROUND_CLOSEST BIT(4) +#define CLK_DIVIDER_READ_ONLY BIT(5) +#define CLK_DIVIDER_MAX_AT_ZERO BIT(6) + +struct clk_fixed_factor { + struct clk clk; + unsigned int mult; + unsigned int div; +}; + +#define to_clk_fixed_factor(_clk) container_of(_clk, struct clk_fixed_factor,\ + clk) + +int clk_register(struct clk *clk, const char *drv_name, + ulong drv_data, const char *name, + const char *parent_name); + +struct clk *clk_register_fixed_factor(struct device *dev, const char *name, + const char *parent_name, unsigned long flags, + unsigned int mult, unsigned int div); + +struct clk *clk_register_divider(struct device *dev, const char *name, + const char *parent_name, unsigned long flags, + void __iomem *reg, u8 shift, u8 width, + u8 clk_divider_flags); + +struct clk *clk_register_mux(struct device *dev, const char *name, + const char * const *parent_names, u8 num_parents, + unsigned long flags, + void __iomem *reg, u8 shift, u8 width, + u8 clk_mux_flags); + +const char *clk_hw_get_name(const struct clk *hw); +ulong clk_generic_get_rate(struct clk *clk); +#endif /* __LINUX_CLK_PROVIDER_H */
Subject: [PATCH v3 11/11] clk: Port Linux common clock framework [CCF] for imx6q to U-boot (tag: 5.0-rc3)
This commit brings the files from Linux kernel to provide clocks support as it is used on the Linux kernel with common clock framework [CCF] setup.
The directory structure has been preserved. The ported code only supports reading information from PLL, MUX, Divider, etc and enabling/disabling the clocks USDHCx/ECSPIx depending on used bus. Moreover, it is agnostic to the alias numbering as the information about the clock is read from device tree.
One needs to pay attention to the comments indicating necessary for U-boot's DM changes.
If needed the code can be extended to support the "set" part of the clock management.
Signed-off-by: Lukasz Majewski lukma@denx.de
Changes in v3: None
drivers/clk/Kconfig | 14 ++++ drivers/clk/Makefile | 2 + drivers/clk/clk-divider.c | 148 ++++++++++++++++++++++++++++++++++ drivers/clk/clk-fixed-factor.c | 87 ++++++++++++++++++++ drivers/clk/clk-mux.c | 164 +++++++++++++++++++++++++++++++++++++ drivers/clk/clk.c | 56 +++++++++++++ drivers/clk/imx/Kconfig | 9 +++ drivers/clk/imx/Makefile | 2 + drivers/clk/imx/clk-gate2.c | 113 ++++++++++++++++++++++++++ drivers/clk/imx/clk-imx6q.c | 179 +++++++++++++++++++++++++++++++++++++++++ drivers/clk/imx/clk-pfd.c | 91 +++++++++++++++++++++ drivers/clk/imx/clk-pllv3.c | 83 +++++++++++++++++++ drivers/clk/imx/clk.h | 75 +++++++++++++++++ include/linux/clk-provider.h | 94 ++++++++++++++++++++++ 14 files changed, 1117 insertions(+) create mode 100644 drivers/clk/clk-divider.c create mode 100644 drivers/clk/clk-fixed-factor.c create mode 100644 drivers/clk/clk-mux.c create mode 100644 drivers/clk/clk.c create mode 100644 drivers/clk/imx/clk-gate2.c create mode 100644 drivers/clk/imx/clk-imx6q.c create mode 100644 drivers/clk/imx/clk-pfd.c create mode 100644 drivers/clk/imx/clk-pllv3.c create mode 100644 drivers/clk/imx/clk.h create mode 100644 include/linux/clk-provider.h
diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig index ff60fc5c45..9df3bc731a 100644 --- a/drivers/clk/Kconfig +++ b/drivers/clk/Kconfig @@ -46,6 +46,20 @@ config CLK_BOSTON help Enable this to support the clocks
+config SPL_CLK_CCF
- bool "SPL Common Clock Framework [CCF] support "
- depends on SPL_CLK
- help
Enable this option if you want to (re-)use the Linux kernel's CommonClock Framework [CCF] code in U-Boot's SPL.+config CLK_CCF
- bool "Common Clock Framework [CCF] support "
- depends on CLK
- help
Enable this option if you want to (re-)use the Linux kernel's CommonClock Framework [CCF] code in U-Boot's clock driver.config CLK_STM32F bool "Enable clock driver support for STM32F family" depends on CLK && (STM32F7 || STM32F4) diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile index 1d9d725cae..9fcc75e0ea 100644 --- a/drivers/clk/Makefile +++ b/drivers/clk/Makefile @@ -7,6 +7,8 @@ obj-$(CONFIG_$(SPL_TPL_)CLK) += clk-uclass.o obj-$(CONFIG_$(SPL_TPL_)CLK) += clk_fixed_rate.o obj-$(CONFIG_$(SPL_TPL_)CLK) += clk_fixed_factor.o +obj-$(CONFIG_$(SPL_TPL_)CLK_CCF) += clk.o clk-divider.o clk-mux.o +obj-$(CONFIG_$(SPL_TPL_)CLK_CCF) += clk-fixed-factor.o
obj-y += imx/ obj-y += tegra/ diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c new file mode 100644 index 0000000000..3841d8bfbb --- /dev/null +++ b/drivers/clk/clk-divider.c @@ -0,0 +1,148 @@ +// SPDX-License-Identifier: GPL-2.0 +/*
- Copyright (C) 2019 DENX Software Engineering
- Lukasz Majewski, DENX Software Engineering, lukma@denx.de
- Copyright (C) 2011 Sascha Hauer, Pengutronix
- Copyright (C) 2011 Richard Zhao, Linaro richard.zhao@linaro.org
- Copyright (C) 2011-2012 Mike Turquette, Linaro Ltd
- */
+#include <common.h> +#include <asm/io.h> +#include <malloc.h> +#include <clk-uclass.h> +#include <dm/device.h> +#include <dm/uclass.h> +#include <dm/lists.h> +#include <dm/device-internal.h> +#include <linux/clk-provider.h> +#include <div64.h> +#include <clk.h> +#include "clk.h"
+#define UBOOT_DM_CLK_CCF_DIVIDER "ccf_clk_divider"
+static unsigned int _get_table_div(const struct clk_div_table *table,
unsigned int val)+{
- const struct clk_div_table *clkt;
- for (clkt = table; clkt->div; clkt++)
if (clkt->val == val)return clkt->div;- return 0;
+}
+static unsigned int _get_div(const struct clk_div_table *table,
unsigned int val, unsigned long flags, u8 width) {- if (flags & CLK_DIVIDER_ONE_BASED)
return val;- if (flags & CLK_DIVIDER_POWER_OF_TWO)
return 1 << val;- if (flags & CLK_DIVIDER_MAX_AT_ZERO)
return val ? val : clk_div_mask(width) + 1;- if (table)
return _get_table_div(table, val);- return val + 1;
+}
+unsigned long divider_recalc_rate(struct clk *hw, unsigned long parent_rate,
unsigned int val,const struct clk_div_table *table,unsigned long flags, unsigned long width) {- unsigned int div;
- div = _get_div(table, val, flags, width);
- if (!div) {
WARN(!(flags & CLK_DIVIDER_ALLOW_ZERO),"%s: Zero divisor and CLK_DIVIDER_ALLOW_ZERO not set\n",clk_hw_get_name(hw));return parent_rate;- }
- return DIV_ROUND_UP_ULL((u64)parent_rate, div); }
+static ulong clk_divider_recalc_rate(struct clk *clk) {
- struct clk_divider *divider =
(struct clk_divider *)dev_get_driver_data(clk->dev);- unsigned long parent_rate = clk_get_parent_rate(clk);
- unsigned int val;
- val = readl(divider->reg) >> divider->shift;
- val &= clk_div_mask(divider->width);
- return divider_recalc_rate(clk, parent_rate, val, divider->table,
divider->flags, divider->width); }+const struct clk_ops clk_divider_ops = {
- .get_rate = clk_divider_recalc_rate,
+};
+static struct clk *_register_divider(struct device *dev, const char *name,
const char *parent_name, unsigned long flags,void __iomem *reg, u8 shift, u8 width,u8 clk_divider_flags, const struct clk_div_table *table) {- struct clk_divider *div;
- struct clk *clk;
- int ret;
- if (clk_divider_flags & CLK_DIVIDER_HIWORD_MASK) {
if (width + shift > 16) {pr_warn("divider value exceeds LOWORD field\n");return ERR_PTR(-EINVAL);}- }
- /* allocate the divider */
- div = kzalloc(sizeof(*div), GFP_KERNEL);
- if (!div)
return ERR_PTR(-ENOMEM);- /* struct clk_divider assignments */
- div->reg = reg;
- div->shift = shift;
- div->width = width;
- div->flags = clk_divider_flags;
- div->table = table;
- /* register the clock */
- clk = &div->clk;
- ret = clk_register(clk, UBOOT_DM_CLK_CCF_DIVIDER, (ulong)clk,
name, parent_name);- if (ret) {
kfree(div);return ERR_PTR(ret);- }
- return clk;
+}
+struct clk *clk_register_divider(struct device *dev, const char *name,
const char *parent_name, unsigned long flags,void __iomem *reg, u8 shift, u8 width,u8 clk_divider_flags)+{
- struct clk *clk;
- clk = _register_divider(dev, name, parent_name, flags, reg, shift,
width, clk_divider_flags, NULL);- if (IS_ERR(clk))
return ERR_CAST(clk);- return clk;
+}
+U_BOOT_DRIVER(ccf_clk_divider) = {
- .name = UBOOT_DM_CLK_CCF_DIVIDER,
- .id = UCLASS_CLK,
- .ops = &clk_divider_ops,
- .flags = DM_FLAG_PRE_RELOC,
+}; diff --git a/drivers/clk/clk-fixed-factor.c b/drivers/clk/clk-fixed-factor.c new file mode 100644 index 0000000000..acbc0909b4 --- /dev/null +++ b/drivers/clk/clk-fixed-factor.c @@ -0,0 +1,87 @@ +// SPDX-License-Identifier: GPL-2.0 +/*
- Copyright (C) 2019 DENX Software Engineering
- Lukasz Majewski, DENX Software Engineering, lukma@denx.de
- Copyright (C) 2011 Sascha Hauer, Pengutronix
+s.hauer@pengutronix.de */ #include <common.h> #include <malloc.h> +#include <clk-uclass.h> #include <dm/device.h> #include +<linux/clk-provider.h> #include <div64.h> #include <clk.h> #include +"clk.h"
+#define UBOOT_DM_CLK_IMX_FIXED_FACTOR "ccf_clk_fixed_factor"
+static ulong clk_factor_recalc_rate(struct clk *clk) {
- struct clk_fixed_factor *fix =
(struct clk_fixed_factor *)dev_get_driver_data(clk->dev);- unsigned long parent_rate = clk_get_parent_rate(clk);
- unsigned long long int rate;
- rate = (unsigned long long int)parent_rate * fix->mult;
- do_div(rate, fix->div);
- return (ulong)rate;
+}
+const struct clk_ops ccf_clk_fixed_factor_ops = {
- .get_rate = clk_factor_recalc_rate,
+};
+struct clk *clk_hw_register_fixed_factor(struct device *dev,
const char *name, const char *parent_name, unsigned long flags,unsigned int mult, unsigned int div)+{
- struct clk_fixed_factor *fix;
- struct clk *clk;
- int ret;
- fix = kmalloc(sizeof(*fix), GFP_KERNEL);
- if (!fix)
return ERR_PTR(-ENOMEM);- /* struct clk_fixed_factor assignments */
- fix->mult = mult;
- fix->div = div;
- clk = &fix->clk;
- /*
* We pass the struct clk *clk pointer (which is the same as* clk_fixed_factor *fix - by the struct elements alignment) to DM as a* driver_data, so it can be easily accessible from the udevice level.* Moreover, the struct clk is only a wrapper on udevice which* corresponds to the "real" clock device.*/- ret = clk_register(clk, UBOOT_DM_CLK_IMX_FIXED_FACTOR, (ulong)clk,
name, parent_name);- if (ret) {
kfree(fix);return ERR_PTR(ret);- }
- return clk;
+}
+struct clk *clk_register_fixed_factor(struct device *dev, const char *name,
const char *parent_name, unsigned long flags,unsigned int mult, unsigned int div)+{
- struct clk *clk;
- clk = clk_hw_register_fixed_factor(dev, name, parent_name, flags, mult,
div);- if (IS_ERR(clk))
return ERR_CAST(clk);- return clk;
+}
+U_BOOT_DRIVER(imx_clk_fixed_factor) = {
- .name = UBOOT_DM_CLK_IMX_FIXED_FACTOR,
- .id = UCLASS_CLK,
- .ops = &ccf_clk_fixed_factor_ops,
- .flags = DM_FLAG_PRE_RELOC,
+}; diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c new file mode 100644 index 0000000000..2c85f2052c --- /dev/null +++ b/drivers/clk/clk-mux.c @@ -0,0 +1,164 @@ +// SPDX-License-Identifier: GPL-2.0 +/*
- Copyright (C) 2019 DENX Software Engineering
- Lukasz Majewski, DENX Software Engineering, lukma@denx.de
- Copyright (C) 2011 Sascha Hauer, Pengutronix
- Copyright (C) 2011 Richard Zhao, Linaro richard.zhao@linaro.org
- Copyright (C) 2011-2012 Mike Turquette, Linaro Ltd
- Simple multiplexer clock implementation */
+/*
- U-Boot CCF porting node:
- The Linux kernel - as of tag: 5.0-rc3 is using also the
+imx_clk_fixup_mux()
- version of CCF mux. It is used on e.g. imx6q to provide fixes (like
- imx_cscmr1_fixup) for broken HW.
- At least for IMX6Q (but NOT IMX6QP) it is important when we set the
+parent
- clock.
- */
+#include <common.h> +#include <asm/io.h> +#include <malloc.h> +#include <clk-uclass.h> +#include <dm/device.h> +#include <linux/clk-provider.h> +#include <clk.h> +#include "clk.h"
+#define UBOOT_DM_CLK_CCF_MUX "ccf_clk_mux"
+int clk_mux_val_to_index(struct clk *clk, u32 *table, unsigned int flags,
unsigned int val)+{
- struct clk_mux *mux = to_clk_mux(clk);
- int num_parents = mux->num_parents;
- if (table) {
int i;for (i = 0; i < num_parents; i++)if (table[i] == val)return i;return -EINVAL;- }
- if (val && (flags & CLK_MUX_INDEX_BIT))
val = ffs(val) - 1;- if (val && (flags & CLK_MUX_INDEX_ONE))
val--;- if (val >= num_parents)
return -EINVAL;- return val;
+}
+static u8 clk_mux_get_parent(struct clk *clk) {
- struct clk_mux *mux = to_clk_mux(clk);
- u32 val;
- val = readl(mux->reg) >> mux->shift;
- val &= mux->mask;
- return clk_mux_val_to_index(clk, mux->table, mux->flags, val); }
+const struct clk_ops clk_mux_ops = {
.get_rate = clk_generic_get_rate,+};
+struct clk *clk_hw_register_mux_table(struct device *dev, const char *name,
const char * const *parent_names, u8 num_parents,unsigned long flags,void __iomem *reg, u8 shift, u32 mask,u8 clk_mux_flags, u32 *table)+{
- struct clk_mux *mux;
- struct clk *clk;
- u8 width = 0;
- int ret;
- if (clk_mux_flags & CLK_MUX_HIWORD_MASK) {
width = fls(mask) - ffs(mask) + 1;if (width + shift > 16) {pr_err("mux value exceeds LOWORD field\n");return ERR_PTR(-EINVAL);}- }
- /* allocate the mux */
- mux = kzalloc(sizeof(*mux), GFP_KERNEL);
- if (!mux)
return ERR_PTR(-ENOMEM);- /* U-boot specific assignments */
- mux->parent_names = parent_names;
- mux->num_parents = num_parents;
- /* struct clk_mux assignments */
- mux->reg = reg;
- mux->shift = shift;
- mux->mask = mask;
- mux->flags = clk_mux_flags;
- mux->table = table;
- clk = &mux->clk;
- /*
* Read the current mux setup - so we assign correct parent.** Changing parent would require changing internals of udevice struct* for the corresponding clock (to do that define .set_parent() method.*/- ret = clk_register(clk, UBOOT_DM_CLK_CCF_MUX, (ulong)clk, name,
parent_names[clk_mux_get_parent(clk)]);- if (ret) {
kfree(mux);return ERR_PTR(ret);- }
- return clk;
+}
+struct clk *clk_register_mux_table(struct device *dev, const char *name,
const char * const *parent_names, u8 num_parents,unsigned long flags,void __iomem *reg, u8 shift, u32 mask,u8 clk_mux_flags, u32 *table)+{
- struct clk *clk;
- clk = clk_hw_register_mux_table(dev, name, parent_names,
num_parents,
flags, reg, shift, mask, clk_mux_flags,table);- if (IS_ERR(clk))
return ERR_CAST(clk);- return clk;
+}
+struct clk *clk_register_mux(struct device *dev, const char *name,
const char * const *parent_names, u8 num_parents,unsigned long flags,void __iomem *reg, u8 shift, u8 width,u8 clk_mux_flags)+{
- u32 mask = BIT(width) - 1;
- return clk_register_mux_table(dev, name, parent_names, num_parents,
flags, reg, shift, mask, clk_mux_flags,NULL);+}
+U_BOOT_DRIVER(ccf_clk_mux) = {
- .name = UBOOT_DM_CLK_CCF_MUX,
- .id = UCLASS_CLK,
- .ops = &clk_mux_ops,
- .flags = DM_FLAG_PRE_RELOC,
+}; diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c new file mode 100644 index 0000000000..0a0fffb50b --- /dev/null +++ b/drivers/clk/clk.c @@ -0,0 +1,56 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Copyright (C) 2019 DENX Software Engineering
- Lukasz Majewski, DENX Software Engineering, lukma@denx.de */
+#include <common.h> +#include <clk-uclass.h> +#include <dm/device.h> +#include <dm/uclass.h> +#include <dm/lists.h> +#include <dm/device-internal.h> +#include <clk.h>
+int clk_register(struct clk *clk, const char *drv_name,
ulong drv_data, const char *name,const char *parent_name)+{
- struct udevice *parent;
- struct driver *drv;
- int ret;
- ret = uclass_get_device_by_name(UCLASS_CLK, parent_name,
&parent);
- if (ret)
printf("%s: UCLASS parent: 0x%p\n", __func__, parent);- debug("%s: name: %s parent: %s [0x%p]\n", __func__, name,
parent->name,
parent);- drv = lists_driver_lookup_name(drv_name);
- if (!drv) {
printf("%s: %s is not a valid driver name\n",__func__, drv_name);return -ENOENT;- }
- ret = device_bind_with_driver_data(parent, drv, name, drv_data,
ofnode_null(), &clk->dev);- if (ret) {
printf("%s: CLK: %s driver bind error [%d]!\n", __func__, name,ret);return ret;- }
- return 0;
+}
+ulong clk_generic_get_rate(struct clk *clk) {
- return clk_get_parent_rate(clk);
+}
+const char *clk_hw_get_name(const struct clk *hw) {
- return hw->dev->name;
+} diff --git a/drivers/clk/imx/Kconfig b/drivers/clk/imx/Kconfig index a6fb58d6cf..469768b5c3 100644 --- a/drivers/clk/imx/Kconfig +++ b/drivers/clk/imx/Kconfig @@ -1,3 +1,12 @@ +config CLK_IMX6Q
- bool "Clock support for i.MX6Q"
- depends on ARCH_MX6
- select CLK
- select CLK_CCF
- select SPL_CLK_CCF
- help
This enables DM/DTS support for clock driver in i.MX6Q platforms.config CLK_IMX8 bool "Clock support for i.MX8" depends on ARCH_IMX8 diff --git a/drivers/clk/imx/Makefile b/drivers/clk/imx/Makefile index 5505ae52e2..beba3bff39 100644 --- a/drivers/clk/imx/Makefile +++ b/drivers/clk/imx/Makefile @@ -2,4 +2,6 @@ # # SPDX-License-Identifier: GPL-2.0
+obj-$(CONFIG_$(SPL_TPL_)CLK_CCF) += clk-gate2.o clk-pllv3.o clk-pfd.o +obj-$(CONFIG_CLK_IMX6Q) += clk-imx6q.o obj-$(CONFIG_CLK_IMX8) += clk-imx8.o diff --git a/drivers/clk/imx/clk-gate2.c b/drivers/clk/imx/clk-gate2.c new file mode 100644 index 0000000000..1e53e4f9db --- /dev/null +++ b/drivers/clk/imx/clk-gate2.c @@ -0,0 +1,113 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Copyright (C) 2019 DENX Software Engineering
- Lukasz Majewski, DENX Software Engineering, lukma@denx.de
- Copyright (C) 2010-2011 Canonical Ltd jeremy.kerr@canonical.com
- Copyright (C) 2011-2012 Mike Turquette, Linaro Ltd
- 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.
- Gated clock implementation
- */
+#include <common.h> +#include <asm/io.h> +#include <malloc.h> +#include <clk-uclass.h> +#include <dm/device.h> +#include <linux/clk-provider.h> +#include <clk.h> +#include "clk.h"
+#define UBOOT_DM_CLK_IMX_GATE2 "imx_clk_gate2"
+struct clk_gate2 {
- struct clk clk;
- void __iomem *reg;
- u8 bit_idx;
- u8 cgr_val;
- u8 flags;
+};
+#define to_clk_gate2(_clk) container_of(_clk, struct clk_gate2, clk)
+static int clk_gate2_enable(struct clk *clk) {
- struct clk_gate2 *gate =
(struct clk_gate2 *)dev_get_driver_data(clk->dev);- u32 reg;
- reg = readl(gate->reg);
- reg &= ~(3 << gate->bit_idx);
- reg |= gate->cgr_val << gate->bit_idx;
- writel(reg, gate->reg);
- return 0;
+}
+static int clk_gate2_disable(struct clk *clk) {
- struct clk_gate2 *gate =
(struct clk_gate2 *)dev_get_driver_data(clk->dev);- u32 reg;
- reg = readl(gate->reg);
- reg &= ~(3 << gate->bit_idx);
- writel(reg, gate->reg);
- return 0;
+}
+static const struct clk_ops clk_gate2_ops = {
- .enable = clk_gate2_enable,
- .disable = clk_gate2_disable,
- .get_rate = clk_generic_get_rate,
+};
+struct clk *clk_register_gate2(struct device *dev, const char *name,
const char *parent_name, unsigned long flags,void __iomem *reg, u8 bit_idx, u8 cgr_val,u8 clk_gate2_flags)+{
- struct clk_gate2 *gate;
- struct clk *clk;
- int ret;
- gate = kzalloc(sizeof(*gate), GFP_KERNEL);
- if (!gate)
return ERR_PTR(-ENOMEM);- /* struct clk_gate2 assignments */
- gate->reg = reg;
- gate->bit_idx = bit_idx;
- gate->cgr_val = cgr_val;
- gate->flags = clk_gate2_flags;
- /*
* U-boot DM adjustments:** clk and gate reslove to the same address - lets pass clock* for better readability.*/- clk = &gate->clk;
- ret = clk_register(clk, UBOOT_DM_CLK_IMX_GATE2, (ulong)clk,
name, parent_name);- if (ret) {
kfree(gate);return ERR_PTR(ret);- }
- return clk;
+}
+U_BOOT_DRIVER(clk_gate2) = {
- .name = UBOOT_DM_CLK_IMX_GATE2,
- .id = UCLASS_CLK,
- .ops = &clk_gate2_ops,
- .flags = DM_FLAG_PRE_RELOC,
+}; diff --git a/drivers/clk/imx/clk-imx6q.c b/drivers/clk/imx/clk-imx6q.c new file mode 100644 index 0000000000..92e9337d44 --- /dev/null +++ b/drivers/clk/imx/clk-imx6q.c @@ -0,0 +1,179 @@ +// SPDX-License-Identifier: GPL-2.0 +/*
- Copyright (C) 2019 DENX Software Engineering
- Lukasz Majewski, DENX Software Engineering, lukma@denx.de */
+#include <common.h> +#include <clk-uclass.h> +#include <dm.h> +#include <asm/arch/clock.h> +#include <asm/arch/imx-regs.h> +#include <dt-bindings/clock/imx6qdl-clock.h>
+#include "clk.h"
+static int imx6q_check_id(ulong id) +{
- if (id < IMX6QDL_CLK_DUMMY || id >= IMX6QDL_CLK_END) {
printf("%s: Invalid clk ID #%lu\n", __func__, id);return -EINVAL;- }
- return 0;
+}
+static ulong imx6q_clk_get_rate(struct clk *clk) {
- struct clk *c;
- int ret;
- debug("%s(#%lu)\n", __func__, clk->id);
- ret = imx6q_check_id(clk->id);
- if (ret)
return ret;- ret = clk_get_by_id(clk->id, &c);
- if (ret)
return ret;- return clk_get_rate(c);
+}
+static ulong imx6q_clk_set_rate(struct clk *clk, unsigned long rate) {
- debug("%s(#%lu), rate: %lu\n", __func__, clk->id, rate);
- return rate;
+}
+static int __imx6q_clk_enable(struct clk *clk, bool enable) {
- struct clk *c;
- int ret = 0;
- debug("%s(#%lu) en: %d\n", __func__, clk->id, enable);
- ret = imx6q_check_id(clk->id);
- if (ret)
return ret;- ret = clk_get_by_id(clk->id, &c);
- if (ret)
return ret;- if (enable)
ret = clk_enable(c);- else
ret = clk_disable(c);- return ret;
+}
+static int imx6q_clk_disable(struct clk *clk) {
- return __imx6q_clk_enable(clk, 0);
+}
+static int imx6q_clk_enable(struct clk *clk) {
- return __imx6q_clk_enable(clk, 1);
+}
+static struct clk_ops imx6q_clk_ops = {
- .set_rate = imx6q_clk_set_rate,
- .get_rate = imx6q_clk_get_rate,
- .enable = imx6q_clk_enable,
- .disable = imx6q_clk_disable,
+};
+static const char *const usdhc_sels[] = { "pll2_pfd2_396m", +"pll2_pfd0_352m", };
+static int imx6q_clk_probe(struct udevice *dev) {
- void *base;
- /* Anatop clocks */
- base = (void *)ANATOP_BASE_ADDR;
- clk_dm(IMX6QDL_CLK_PLL2,
imx_clk_pllv3(IMX_PLLV3_GENERIC, "pll2_bus", "osc",base + 0x30, 0x1));- clk_dm(IMX6QDL_CLK_PLL3_USB_OTG,
imx_clk_pllv3(IMX_PLLV3_USB, "pll3_usb_otg", "osc",base + 0x10, 0x3));- clk_dm(IMX6QDL_CLK_PLL3_60M,
imx_clk_fixed_factor("pll3_60m", "pll3_usb_otg", 1, 8));- clk_dm(IMX6QDL_CLK_PLL2_PFD0_352M,
imx_clk_pfd("pll2_pfd0_352m", "pll2_bus", base + 0x100, 0));- clk_dm(IMX6QDL_CLK_PLL2_PFD2_396M,
imx_clk_pfd("pll2_pfd2_396m", "pll2_bus", base + 0x100, 2));- /* CCM clocks */
- base = dev_read_addr_ptr(dev);
- if (base == (void *)FDT_ADDR_T_NONE)
return -EINVAL;- clk_dm(IMX6QDL_CLK_USDHC1_SEL,
imx_clk_mux("usdhc1_sel", base + 0x1c, 16, 1,usdhc_sels, ARRAY_SIZE(usdhc_sels)));- clk_dm(IMX6QDL_CLK_USDHC2_SEL,
imx_clk_mux("usdhc2_sel", base + 0x1c, 17, 1,usdhc_sels, ARRAY_SIZE(usdhc_sels)));- clk_dm(IMX6QDL_CLK_USDHC3_SEL,
imx_clk_mux("usdhc3_sel", base + 0x1c, 18, 1,usdhc_sels, ARRAY_SIZE(usdhc_sels)));- clk_dm(IMX6QDL_CLK_USDHC4_SEL,
imx_clk_mux("usdhc4_sel", base + 0x1c, 19, 1,usdhc_sels, ARRAY_SIZE(usdhc_sels)));- clk_dm(IMX6QDL_CLK_USDHC1_PODF,
imx_clk_divider("usdhc1_podf", "usdhc1_sel",base + 0x24, 11, 3));- clk_dm(IMX6QDL_CLK_USDHC2_PODF,
imx_clk_divider("usdhc2_podf", "usdhc2_sel",base + 0x24, 16, 3));- clk_dm(IMX6QDL_CLK_USDHC3_PODF,
imx_clk_divider("usdhc3_podf", "usdhc3_sel",base + 0x24, 19, 3));- clk_dm(IMX6QDL_CLK_USDHC4_PODF,
imx_clk_divider("usdhc4_podf", "usdhc4_sel",base + 0x24, 22, 3));- clk_dm(IMX6QDL_CLK_ECSPI_ROOT,
imx_clk_divider("ecspi_root", "pll3_60m", base + 0x38, 19, 6));- clk_dm(IMX6QDL_CLK_ECSPI1,
imx_clk_gate2("ecspi1", "ecspi_root", base + 0x6c, 0));- clk_dm(IMX6QDL_CLK_ECSPI2,
imx_clk_gate2("ecspi2", "ecspi_root", base + 0x6c, 2));- clk_dm(IMX6QDL_CLK_ECSPI3,
imx_clk_gate2("ecspi3", "ecspi_root", base + 0x6c, 4));- clk_dm(IMX6QDL_CLK_ECSPI4,
imx_clk_gate2("ecspi4", "ecspi_root", base + 0x6c, 6));- clk_dm(IMX6QDL_CLK_USDHC1,
imx_clk_gate2("usdhc1", "usdhc1_podf", base + 0x80, 2));- clk_dm(IMX6QDL_CLK_USDHC2,
imx_clk_gate2("usdhc2", "usdhc2_podf", base + 0x80, 4));- clk_dm(IMX6QDL_CLK_USDHC3,
imx_clk_gate2("usdhc3", "usdhc3_podf", base + 0x80, 6));- clk_dm(IMX6QDL_CLK_USDHC4,
imx_clk_gate2("usdhc4", "usdhc4_podf", base + 0x80, 8));- return 0;
+}
+static const struct udevice_id imx6q_clk_ids[] = {
- { .compatible = "fsl,imx6q-ccm" },
- { },
+};
+U_BOOT_DRIVER(imx6q_clk) = {
- .name = "clk_imx6q",
- .id = UCLASS_CLK,
- .of_match = imx6q_clk_ids,
- .ops = &imx6q_clk_ops,
- .probe = imx6q_clk_probe,
- .flags = DM_FLAG_PRE_RELOC,
+}; diff --git a/drivers/clk/imx/clk-pfd.c b/drivers/clk/imx/clk-pfd.c new file mode 100644 index 0000000000..2293d481d4 --- /dev/null +++ b/drivers/clk/imx/clk-pfd.c @@ -0,0 +1,91 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Copyright (C) 2019 DENX Software Engineering
- Lukasz Majewski, DENX Software Engineering, lukma@denx.de
- Copyright 2012 Freescale Semiconductor, Inc.
- Copyright 2012 Linaro Ltd.
- The code contained herein is licensed under the GNU General Public
- License. You may obtain a copy of the GNU General Public License
- Version 2 or later at the following locations:
+https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww. op +ensource.org%2Flicenses%2Fgpl-license.html&data=02%7C01%7Cpeng .fan% +40nxp.com%7C92b89511186641d8ecf508d6c969131f%7C686ea1d3bc2b4c 6fa92cd99c +5c301635%7C0%7C0%7C636917850484120681&sdata=4wXyWrLW9k NgQ7YfTNYDH1d +tO4AEHOS50suxlzBBugY%3D&reserved=0
+https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww. gn +u.org%2Fcopyleft%2Fgpl.html&data=02%7C01%7Cpeng.fan%40nxp.co m%7C92b +89511186641d8ecf508d6c969131f%7C686ea1d3bc2b4c6fa92cd99c5c3016 35%7C0%7C +0%7C636917850484120681&sdata=EV6piw9NY0H71%2BjCCVOwnKDz a%2FF33189dT +oqBsgUKn8%3D&reserved=0
- */
+#include <common.h> +#include <asm/io.h> +#include <malloc.h> +#include <clk-uclass.h> +#include <dm/device.h> +#include <linux/clk-provider.h> +#include <div64.h> +#include <clk.h> +#include "clk.h"
+#define UBOOT_DM_CLK_IMX_PFD "imx_clk_pfd"
+struct clk_pfd {
- struct clk clk;
- void __iomem *reg;
- u8 idx;
+};
+#define to_clk_pfd(_clk) container_of(_clk, struct clk_pfd, clk)
+#define SET 0x4 +#define CLR 0x8 +#define OTG 0xc
+static unsigned long clk_pfd_recalc_rate(struct clk *clk) {
- struct clk_pfd *pfd =
(struct clk_pfd *)dev_get_driver_data(clk->dev);- unsigned long parent_rate = clk_get_parent_rate(clk);
- u64 tmp = parent_rate;
- u8 frac = (readl(pfd->reg) >> (pfd->idx * 8)) & 0x3f;
- tmp *= 18;
- do_div(tmp, frac);
- return tmp;
+}
+static const struct clk_ops clk_pfd_ops = {
- .get_rate = clk_pfd_recalc_rate,
+};
+struct clk *imx_clk_pfd(const char *name, const char *parent_name,
void __iomem *reg, u8 idx)+{
- struct clk_pfd *pfd;
- struct clk *clk;
- int ret;
- pfd = kzalloc(sizeof(*pfd), GFP_KERNEL);
- if (!pfd)
return ERR_PTR(-ENOMEM);- pfd->reg = reg;
- pfd->idx = idx;
- /* register the clock */
- clk = &pfd->clk;
- ret = clk_register(clk, UBOOT_DM_CLK_IMX_PFD, (ulong)clk,
name, parent_name);- if (ret) {
kfree(pfd);return ERR_PTR(ret);- }
- return clk;
+}
+U_BOOT_DRIVER(clk_pfd) = {
- .name = UBOOT_DM_CLK_IMX_PFD,
- .id = UCLASS_CLK,
- .ops = &clk_pfd_ops,
- .flags = DM_FLAG_PRE_RELOC,
+}; diff --git a/drivers/clk/imx/clk-pllv3.c b/drivers/clk/imx/clk-pllv3.c new file mode 100644 index 0000000000..3fe9b7c03d --- /dev/null +++ b/drivers/clk/imx/clk-pllv3.c @@ -0,0 +1,83 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Copyright (C) 2019 DENX Software Engineering
- Lukasz Majewski, DENX Software Engineering, lukma@denx.de */
+#include <common.h> +#include <asm/io.h> +#include <malloc.h> +#include <clk-uclass.h> +#include <dm/device.h> +#include <dm/uclass.h> +#include <clk.h> +#include "clk.h"
+#define UBOOT_DM_CLK_IMX_PLLV3 "imx_clk_pllv3"
+struct clk_pllv3 {
- struct clk clk;
- void __iomem *base;
- u32 div_mask;
- u32 div_shift;
+};
+#define to_clk_pllv3(_clk) container_of(_clk, struct clk_pllv3, clk)
+static ulong clk_pllv3_get_rate(struct clk *clk) {
- struct clk_pllv3 *pll =
(struct clk_pllv3 *)dev_get_driver_data(clk->dev);- unsigned long parent_rate = clk_get_parent_rate(clk);
- u32 div = (readl(pll->base) >> pll->div_shift) & pll->div_mask;
- return (div == 1) ? parent_rate * 22 : parent_rate * 20; }
+static const struct clk_ops clk_pllv3_generic_ops = {
- .get_rate = clk_pllv3_get_rate,
+};
+struct clk *imx_clk_pllv3(enum imx_pllv3_type type, const char *name,
const char *parent_name, void __iomem *base,u32 div_mask)+{
- struct clk_pllv3 *pll;
- struct clk *clk;
- char *drv_name;
- int ret;
- pll = kzalloc(sizeof(*pll), GFP_KERNEL);
- if (!pll)
return ERR_PTR(-ENOMEM);- switch (type) {
- case IMX_PLLV3_GENERIC:
- case IMX_PLLV3_USB:
drv_name = UBOOT_DM_CLK_IMX_PLLV3;break;- default:
kfree(pll);return ERR_PTR(-ENOTSUPP);- }
- pll->base = base;
- pll->div_mask = div_mask;
- clk = &pll->clk;
- ret = clk_register(clk, drv_name, (ulong)clk, name, parent_name);
- if (ret) {
kfree(pll);return ERR_PTR(ret);- }
- return clk;
+}
+U_BOOT_DRIVER(clk_pllv3_generic) = {
- .name = UBOOT_DM_CLK_IMX_PLLV3,
- .id = UCLASS_CLK,
- .ops = &clk_pllv3_generic_ops,
- .flags = DM_FLAG_PRE_RELOC,
+}; diff --git a/drivers/clk/imx/clk.h b/drivers/clk/imx/clk.h new file mode 100644 index 0000000000..864a215a22 --- /dev/null +++ b/drivers/clk/imx/clk.h @@ -0,0 +1,75 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Copyright (C) 2019 DENX Software Engineering
- Lukasz Majewski, DENX Software Engineering, lukma@denx.de */
+#ifndef __MACH_IMX_CLK_H #define __MACH_IMX_CLK_H
+#include <linux/clk-provider.h>
+static inline void clk_dm(ulong id, struct clk *clk) {
- if (!IS_ERR(clk))
clk->id = id;+}
+enum imx_pllv3_type {
- IMX_PLLV3_GENERIC,
- IMX_PLLV3_SYS,
- IMX_PLLV3_USB,
- IMX_PLLV3_USB_VF610,
- IMX_PLLV3_AV,
- IMX_PLLV3_ENET,
- IMX_PLLV3_ENET_IMX7,
- IMX_PLLV3_SYS_VF610,
- IMX_PLLV3_DDR_IMX7,
+};
+struct clk *clk_register_gate2(struct device *dev, const char *name,
const char *parent_name, unsigned long flags,void __iomem *reg, u8 bit_idx, u8 cgr_val,u8 clk_gate_flags);+struct clk *imx_clk_pllv3(enum imx_pllv3_type type, const char *name,
const char *parent_name, void __iomem *base,u32 div_mask);+static inline struct clk *imx_clk_gate2(const char *name, const char *parent,
void __iomem *reg, u8 shift)+{
- return clk_register_gate2(NULL, name, parent, CLK_SET_RATE_PARENT,
reg,
shift, 0x3, 0);+}
+static inline struct clk *imx_clk_fixed_factor(const char *name,
const char *parent, unsigned int mult, unsigned int div) {- return clk_register_fixed_factor(NULL, name, parent,
CLK_SET_RATE_PARENT, mult, div);+}
+static inline struct clk *imx_clk_divider(const char *name, const char *parent,
void __iomem *reg, u8 shift, u8 width) {- return clk_register_divider(NULL, name, parent, CLK_SET_RATE_PARENT,
reg, shift, width, 0);+}
+struct clk *imx_clk_pfd(const char *name, const char *parent_name,
void __iomem *reg, u8 idx);+struct clk *imx_clk_fixup_mux(const char *name, void __iomem *reg,
u8 shift, u8 width, const char * const *parents,int num_parents, void (*fixup)(u32 *val));+static inline struct clk *imx_clk_mux(const char *name, void __iomem *reg,
u8 shift, u8 width, const char * const *parents,int num_parents)+{
- return clk_register_mux(NULL, name, parents, num_parents,
CLK_SET_RATE_NO_REPARENT, reg, shift,width, 0);+}
+#endif /* __MACH_IMX_CLK_H */ diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h new file mode 100644 index 0000000000..eac045c5f8 --- /dev/null +++ b/include/linux/clk-provider.h @@ -0,0 +1,94 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Copyright (C) 2019 DENX Software Engineering
- Lukasz Majewski, DENX Software Engineering, lukma@denx.de
- Copyright (c) 2010-2011 Jeremy Kerr jeremy.kerr@canonical.com
- Copyright (C) 2011-2012 Linaro Ltd mturquette@linaro.org */
+#ifndef __LINUX_CLK_PROVIDER_H #define __LINUX_CLK_PROVIDER_H
+#define CLK_SET_RATE_PARENT BIT(2) /* propagate rate change up one level */ +#define CLK_SET_RATE_NO_REPARENT BIT(7) /* don't re-parent on rate +change */
+#define CLK_MUX_INDEX_ONE BIT(0) +#define CLK_MUX_INDEX_BIT BIT(1) +#define CLK_MUX_HIWORD_MASK BIT(2) +#define CLK_MUX_READ_ONLY BIT(3) /* mux can't be changed */ +#define CLK_MUX_ROUND_CLOSEST BIT(4)
+struct clk_mux {
- struct clk clk;
- void __iomem *reg;
- u32 *table;
- u32 mask;
- u8 shift;
- u8 flags;
- /*
* Fields from struct clk_init_data - this struct has been* omitted to avoid too deep level of CCF for bootloader*/- const char * const *parent_names;
- u8 num_parents;
+};
+#define to_clk_mux(_clk) container_of(_clk, struct clk_mux, clk)
+struct clk_div_table {
- unsigned int val;
- unsigned int div;
+};
+struct clk_divider {
- struct clk clk;
- void __iomem *reg;
- u8 shift;
- u8 width;
- u8 flags;
- const struct clk_div_table *table;
+};
+#define clk_div_mask(width) ((1 << (width)) - 1) +#define to_clk_divider(_clk) container_of(_clk, struct clk_divider, +clk)
+#define CLK_DIVIDER_ONE_BASED BIT(0) +#define CLK_DIVIDER_POWER_OF_TWO BIT(1) +#define CLK_DIVIDER_ALLOW_ZERO BIT(2) +#define CLK_DIVIDER_HIWORD_MASK BIT(3) +#define CLK_DIVIDER_ROUND_CLOSEST BIT(4) +#define CLK_DIVIDER_READ_ONLY BIT(5) +#define CLK_DIVIDER_MAX_AT_ZERO BIT(6)
+struct clk_fixed_factor {
- struct clk clk;
- unsigned int mult;
- unsigned int div;
+};
+#define to_clk_fixed_factor(_clk) container_of(_clk, struct clk_fixed_factor,\
clk)+int clk_register(struct clk *clk, const char *drv_name,
ulong drv_data, const char *name,const char *parent_name);+struct clk *clk_register_fixed_factor(struct device *dev, const char *name,
const char *parent_name, unsigned long flags,unsigned int mult, unsigned int div);+struct clk *clk_register_divider(struct device *dev, const char *name,
const char *parent_name, unsigned long flags,void __iomem *reg, u8 shift, u8 width,u8 clk_divider_flags);+struct clk *clk_register_mux(struct device *dev, const char *name,
const char * const *parent_names, u8 num_parents,unsigned long flags,void __iomem *reg, u8 shift, u8 width,u8 clk_mux_flags);+const char *clk_hw_get_name(const struct clk *hw); ulong +clk_generic_get_rate(struct clk *clk); #endif /* __LINUX_CLK_PROVIDER_H +*/
I think it might be better to add new Kconfig for the clk files, such as PFD/GATE2 There are mixed usage hw and clk, do we need to provide hw, or just clk?
Set rate is not added, I could consider to add it in my patches. Also I am thinking to add round_rate, thoughts?
Regards, Peng.
-- 2.11.0
Dear All,
This patch series brings the files from Linux kernel to provide clocks support as it is used on the Linux kernel with common clock framework [CCF] setup.
This series also fixes several problems with current clocks and provides sandbox tests for functions addded to clk-uclass.c file.
Design decisions/issues:
- U-boot's DM for clk differs from Linux CCF. The most notably
difference is the lack of support for hierarchical clocks and "clock as a manager driver" (single clock DTS node acts as a starting point for all other clocks).
- The clk_get_rate() now caches the previously read data (no need for
recursive access.
- On purpose the "manager" clk driver (clk-imx6q.c) is not using large
table to store pointers to clocks - e.g. clk[IMX6QDL_CLK_USDHC2_SEL] = .... Instead we use udevice's linked list for the same class (UCLASS_CLK). The rationale - when porting the code as is from Linux, one would need ~1KiB of RAM to store it. This is way too much if we do plan to use this driver in SPL.
- The "central" structure of this patch series is struct udevice and
its driver_data field contains the struct clk pointer (to the originally created one).
- Up till now U-boot's driver model's CLK operates on udevice (main
access to clock is by udevice ops) In the CCF the access to struct clk (comprising pointer to *dev) is possible via dev_get_driver_data()
Storing back pointer (from udevice to struct clk) as driver_data is a convention for CCF.
- I could use *private_alloc_size to allocate driver's 'private" structures (dev->priv) for e.g. divider (struct clk_divider
*divider) for IMX6Q clock, but this would change the original structure of the CCF code.
The question is if it would be better to use private_alloc_size (and dev->private) or stay with driver_data. The former requires some rewritting in CCF original code (to remove (c)malloc, etc), but comply with u-boot DM. The latter allows re-using the CCF code as is, but introduces some convention special for CCF (I'm not sure thought if dev->priv is NOT another convention as well).
- I've added the clk_get_parent(), which reads parent's
dev->driver_data to provide parent's struct clk pointer. This seems the easiest way to get child/parent relationship for struct clk in U-boot's udevice based clocks.
- For tests I had to "emulate" CCF code structure to test
functionality of clk_get_parent_rate() and clk_get_by_id(). Those functions will not work properly with "standard" (i.e. non CCF) clock setup(with not set dev->driver_data to struct clk).
I would be very grateful for comments regarding described above design decisions.
Thanks in advance.
Repository: https://github.com/lmajewski/u-boot-dfu/commits/CCF-v3
Changes in v3:
- New patch
- The rate information is now cached into struct clk field
- The clk_get_parent() is used to get pointer to the parent struct clk
- Replace -ENODEV with -ENOENT
- Use **clkp instead of **c
- New patch
- New patch
Lukasz Majewski (11): dm: Fix documentation entry as there is no UCLASS_CLOCK uclass cmd: Do not show frequency for clocks which .get_rate() return error clk: Remove clock ID check in .get_rate() of clk_fixed_* clk: Extend struct clk to provide information regarding clock rate clk: Provide struct clk for fixed rate clock (clk_fixed_rate.c) dm: clk: Define clk_get_parent() for clk operations dm: clk: Define clk_get_parent_rate() for clk operations dm: clk: Define clk_get_by_id() for clk operations clk: test: Provide unit test for clk_get_by_id() method clk: test: Provide unit test for clk_get_parent_rate() method clk: Port Linux common clock framework [CCF] for imx6q to U-boot (tag: 5.0-rc3)
arch/sandbox/include/asm/clk.h | 16 ++++ cmd/clk.c | 5 +- drivers/clk/Kconfig | 14 ++++ drivers/clk/Makefile | 2 + drivers/clk/clk-divider.c | 148 ++++++++++++++++++++++++++++++++++ drivers/clk/clk-fixed-factor.c | 87 ++++++++++++++++++++ drivers/clk/clk-mux.c | 164 +++++++++++++++++++++++++++++++++++++ drivers/clk/clk-uclass.c | 59 ++++++++++++++ drivers/clk/clk.c | 56 +++++++++++++ drivers/clk/clk_fixed_factor.c | 3 - drivers/clk/clk_fixed_rate.c | 8 +- drivers/clk/clk_sandbox_test.c | 49 +++++++++++ drivers/clk/imx/Kconfig | 9 +++ drivers/clk/imx/Makefile | 2 + drivers/clk/imx/clk-gate2.c | 113 ++++++++++++++++++++++++++ drivers/clk/imx/clk-imx6q.c | 179 +++++++++++++++++++++++++++++++++++++++++ drivers/clk/imx/clk-pfd.c | 91 +++++++++++++++++++++ drivers/clk/imx/clk-pllv3.c | 83 +++++++++++++++++++ drivers/clk/imx/clk.h | 75 +++++++++++++++++ include/clk.h | 33 +++++++- include/linux/clk-provider.h | 94 ++++++++++++++++++++++ test/dm/clk.c | 4 +- 22 files changed, 1285 insertions(+), 9 deletions(-) create mode 100644 drivers/clk/clk-divider.c create mode 100644 drivers/clk/clk-fixed-factor.c create mode 100644 drivers/clk/clk-mux.c create mode 100644 drivers/clk/clk.c create mode 100644 drivers/clk/imx/clk-gate2.c create mode 100644 drivers/clk/imx/clk-imx6q.c create mode 100644 drivers/clk/imx/clk-pfd.c create mode 100644 drivers/clk/imx/clk-pllv3.c create mode 100644 drivers/clk/imx/clk.h create mode 100644 include/linux/clk-provider.h
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
Hi Lukasz,
On Thu, 25 Apr 2019 at 04:30, Lukasz Majewski lukma@denx.de wrote:
This patch series brings the files from Linux kernel to provide clocks support as it is used on the Linux kernel with common clock framework [CCF] setup.
This series also fixes several problems with current clocks and provides sandbox tests for functions addded to clk-uclass.c file.
Design decisions/issues:
- U-boot's DM for clk differs from Linux CCF. The most notably difference
is the lack of support for hierarchical clocks and "clock as a manager driver" (single clock DTS node acts as a starting point for all other clocks).
- The clk_get_rate() now caches the previously read data (no need for
recursive access.
- On purpose the "manager" clk driver (clk-imx6q.c) is not using large
table to store pointers to clocks - e.g. clk[IMX6QDL_CLK_USDHC2_SEL] = .... Instead we use udevice's linked list for the same class (UCLASS_CLK). The rationale - when porting the code as is from Linux, one would need ~1KiB of RAM to store it. This is way too much if we do plan to use this driver in SPL.
- The "central" structure of this patch series is struct udevice and its
driver_data field contains the struct clk pointer (to the originally created one).
Up till now U-boot's driver model's CLK operates on udevice (main access to clock is by udevice ops) In the CCF the access to struct clk (comprising pointer to *dev) is possible via dev_get_driver_data()
Storing back pointer (from udevice to struct clk) as driver_data is a convention for CCF.
Ick. Why not use uclass-private data to store this, since every UCLASS_CLK device can have a parent.
- I could use *private_alloc_size to allocate driver's 'private" structures (dev->priv) for e.g. divider (struct clk_divider *divider) for IMX6Q clock, but this would change the original structure of the CCF code.
The question is if it would be better to use private_alloc_size (and dev->private) or stay with driver_data. The former requires some rewritting in CCF original code (to remove (c)malloc, etc), but comply with u-boot DM. The latter allows re-using the CCF code as is, but introduces some convention special for CCF (I'm not sure thought if dev->priv is NOT another convention as well).
Yes I would like to avoid malloc() calls in drivers and use the in-built mechanism.
I've added the clk_get_parent(), which reads parent's dev->driver_data to provide parent's struct clk pointer. This seems the easiest way to get child/parent relationship for struct clk in U-boot's udevice based clocks.
For tests I had to "emulate" CCF code structure to test functionality of clk_get_parent_rate() and clk_get_by_id(). Those functions will not work properly with "standard" (i.e. non CCF) clock setup(with not set dev->driver_data to struct clk).
Well I think we need a better approach for that anywat. driver_data is used for getting something from the DT.
Regards, Simon
Hi Simon,
This is not the newest patch set version of CCF (v3 vs. v4), but the comments/issues apply.
Hi Lukasz,
On Thu, 25 Apr 2019 at 04:30, Lukasz Majewski lukma@denx.de wrote:
This patch series brings the files from Linux kernel to provide clocks support as it is used on the Linux kernel with common clock framework [CCF] setup.
This series also fixes several problems with current clocks and provides sandbox tests for functions addded to clk-uclass.c file.
Design decisions/issues:
- U-boot's DM for clk differs from Linux CCF. The most notably
difference is the lack of support for hierarchical clocks and "clock as a manager driver" (single clock DTS node acts as a starting point for all other clocks).
- The clk_get_rate() now caches the previously read data (no need
for recursive access.
- On purpose the "manager" clk driver (clk-imx6q.c) is not using
large table to store pointers to clocks - e.g. clk[IMX6QDL_CLK_USDHC2_SEL] = .... Instead we use udevice's linked list for the same class (UCLASS_CLK). The rationale - when porting the code as is from Linux, one would need ~1KiB of RAM to store it. This is way too much if we do plan to use this driver in SPL.
- The "central" structure of this patch series is struct udevice
and its driver_data field contains the struct clk pointer (to the originally created one).
- Up till now U-boot's driver model's CLK operates on udevice (main
access to clock is by udevice ops) In the CCF the access to struct clk (comprising pointer to *dev) is possible via dev_get_driver_data()
Storing back pointer (from udevice to struct clk) as driver_data is a convention for CCF.
Ick. Why not use uclass-private data to store this, since every UCLASS_CLK device can have a parent.
The "private_data" field would be also a good place to store the back pointer from udevice to struct clk [*]. The problem with CCF and udevice's priv pointer is explained just below:
- I could use *private_alloc_size to allocate driver's 'private" structures (dev->priv) for e.g. divider (struct clk_divider
*divider) for IMX6Q clock, but this would change the original structure of the CCF code.
The original Linux's CCF code for iMX relies on using kmalloc internally:
https://elixir.bootlin.com/linux/v5.1.2/source/drivers/clk/imx/clk-gate2.c#L... https://elixir.bootlin.com/linux/v5.1.2/source/drivers/clk/clk-divider.c#L47...
By using driver_data I've avoided the need to make more changes to the original Linux code.
I could use udevice's priv with automatic data allocation but then the CCF ported code would require more changes and considering the (from the outset) need to "fit" this code into U-Boot's DM, it drives away from the original Linux code.
The question is if it would be better to use private_alloc_size (and dev->private) or stay with driver_data. The former requires some rewritting in CCF original code (to remove (c)malloc, etc), but comply with u-boot DM. The latter allows re-using the CCF code as is, but introduces some convention special for CCF (I'm not sure thought if dev->priv is NOT another convention as well).
Yes I would like to avoid malloc() calls in drivers and use the in-built mechanism.
I see your point.
If the community agrees - I can rewrite the code to use such approach (but issues pointed out in [*] still apply).
- I've added the clk_get_parent(), which reads parent's
dev->driver_data to provide parent's struct clk pointer. This seems the easiest way to get child/parent relationship for struct clk in U-boot's udevice based clocks.
- For tests I had to "emulate" CCF code structure to test
functionality of clk_get_parent_rate() and clk_get_by_id(). Those functions will not work properly with "standard" (i.e. non CCF) clock setup(with not set dev->driver_data to struct clk).
Well I think we need a better approach for that anywat. driver_data is used for getting something from the DT.
Maybe the name (driver_data) was a bit misleading then. For CCF it stores the back pointer to struct clk (as in fact it is a CCF's "driver data").
NOTE:
[*] - I do have a hard time to understand how struct clk shall work with struct udevice?
In Linux or Barebox the struct clk is the "main" structure to hold the clock management data (like freq, ops, flags, parent/sibling relation, etc).
A side observation - we now have three different implementations of struct clk in U-Boot :-) (two of which have *ops inside :-) )
In the case of U-Boot's DM (./include/clk.h) it only has a _pointer_ to udevice (which means that I cannot get the struct clk easily from udevice with container_of()). The struct udevice has instead the *ops and *parent pointer (to another udevice).
Two problems:
- Linux CCF code uses massively "struct clk" to handle clock operations (but not udevice)
- There is no clear idea of how to implement struct clk *clk_get_parent(struct clk *) in U-Boot.
The reason is that we lack clear information about which udevice's data field shall be used to store the back pointer from udevice to struct clk.
Any hints and ideas are more than welcome.
Regards, Simon
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
Hi Lukasz,
On Sat, 18 May 2019 at 15:28, Lukasz Majewski lukma@denx.de wrote:
Hi Simon,
This is not the newest patch set version of CCF (v3 vs. v4), but the comments/issues apply.
Hi Lukasz,
On Thu, 25 Apr 2019 at 04:30, Lukasz Majewski lukma@denx.de wrote:
This patch series brings the files from Linux kernel to provide clocks support as it is used on the Linux kernel with common clock framework [CCF] setup.
This series also fixes several problems with current clocks and provides sandbox tests for functions addded to clk-uclass.c file.
Design decisions/issues:
- U-boot's DM for clk differs from Linux CCF. The most notably
difference is the lack of support for hierarchical clocks and "clock as a manager driver" (single clock DTS node acts as a starting point for all other clocks).
- The clk_get_rate() now caches the previously read data (no need
for recursive access.
- On purpose the "manager" clk driver (clk-imx6q.c) is not using
large table to store pointers to clocks - e.g. clk[IMX6QDL_CLK_USDHC2_SEL] = .... Instead we use udevice's linked list for the same class (UCLASS_CLK). The rationale - when porting the code as is from Linux, one would need ~1KiB of RAM to store it. This is way too much if we do plan to use this driver in SPL.
- The "central" structure of this patch series is struct udevice
and its driver_data field contains the struct clk pointer (to the originally created one).
- Up till now U-boot's driver model's CLK operates on udevice (main
access to clock is by udevice ops) In the CCF the access to struct clk (comprising pointer to *dev) is possible via dev_get_driver_data()
Storing back pointer (from udevice to struct clk) as driver_data is a convention for CCF.
Ick. Why not use uclass-private data to store this, since every UCLASS_CLK device can have a parent.
The "private_data" field would be also a good place to store the back pointer from udevice to struct clk [*]. The problem with CCF and udevice's priv pointer is explained just below:
- I could use *private_alloc_size to allocate driver's 'private" structures (dev->priv) for e.g. divider (struct clk_divider
*divider) for IMX6Q clock, but this would change the original structure of the CCF code.
The original Linux's CCF code for iMX relies on using kmalloc internally:
https://elixir.bootlin.com/linux/v5.1.2/source/drivers/clk/imx/clk-gate2.c#L... https://elixir.bootlin.com/linux/v5.1.2/source/drivers/clk/clk-divider.c#L47...
By using driver_data I've avoided the need to make more changes to the original Linux code.
I could use udevice's priv with automatic data allocation but then the CCF ported code would require more changes and considering the (from the outset) need to "fit" this code into U-Boot's DM, it drives away from the original Linux code.
Is the main change the need to cast driver_data? Perhaps that could be hidden in a helper function/macro, so that in U-Boot it can hide the use of (struct clk_uc_priv *)dev_get_uclass_priv(clk->dev))>parent ?
The question is if it would be better to use private_alloc_size (and dev->private) or stay with driver_data. The former requires some rewritting in CCF original code (to remove (c)malloc, etc), but comply with u-boot DM. The latter allows re-using the CCF code as is, but introduces some convention special for CCF (I'm not sure thought if dev->priv is NOT another convention as well).
Yes I would like to avoid malloc() calls in drivers and use the in-built mechanism.
I see your point.
If the community agrees - I can rewrite the code to use such approach (but issues pointed out in [*] still apply).
- I've added the clk_get_parent(), which reads parent's
dev->driver_data to provide parent's struct clk pointer. This seems the easiest way to get child/parent relationship for struct clk in U-boot's udevice based clocks.
- For tests I had to "emulate" CCF code structure to test
functionality of clk_get_parent_rate() and clk_get_by_id(). Those functions will not work properly with "standard" (i.e. non CCF) clock setup(with not set dev->driver_data to struct clk).
Well I think we need a better approach for that anywat. driver_data is used for getting something from the DT.
Maybe the name (driver_data) was a bit misleading then. For CCF it stores the back pointer to struct clk (as in fact it is a CCF's "driver data").
Well it seems like a hack to me. Perhaps there is a good reason for it in Linux? Or is it just convenience?
NOTE:
[*] - I do have a hard time to understand how struct clk shall work with struct udevice?
In Linux or Barebox the struct clk is the "main" structure to hold the clock management data (like freq, ops, flags, parent/sibling relation, etc).
Yes U-Boot has a uniform struct udevice for every device and struct uclass for every class.
But the interesting thing here is that clocks have their own parent/sibling relationships, quite independent from the device tree.
A side observation - we now have three different implementations of struct clk in U-Boot :-) (two of which have *ops inside :-) )
Oh dear.
The broadcom iMX ones needs to be converted.
In the case of U-Boot's DM (./include/clk.h) it only has a _pointer_ to udevice (which means that I cannot get the struct clk easily from udevice with container_of()). The struct udevice has instead the *ops and *parent pointer (to another udevice).
Yes that's correct. The struct clk is actually a handle to the clock, and includes an ID number.
Two problems:
- Linux CCF code uses massively "struct clk" to handle clock operations (but not udevice)
OK.
- There is no clear idea of how to implement
struct clk *clk_get_parent(struct clk *) in U-Boot.
As above, it seems that this might need to be implemented. I don't think the DM parent/child relationships are good enough for clk, since they are not aware of the ID.
The reason is that we lack clear information about which udevice's data field shall be used to store the back pointer from udevice to struct clk.
Any hints and ideas are more than welcome.
I think it would be good to get Stephen Warren's thoughts on this as he made the change to introduce struct clk.
But at present clk_set_parent() is implemented by calling into the driver. The uclass itself does not maintain information about what is a parent of what.
Do we *need* to maintain this information in the uclass?
I think it would be prohibitively expensive to separate out each individual clock into a separate device (udevice), but that would work.
The only other option I see is to create a sibling list and parent pointer inside struct clk.
I suspect this will affect power domain also, although we don't have that yet.
Do you think there is a case for building this into DM itself, such that devices can have a list of IDs for each device, each with independent parent/child relationships?
Regards, Simon
Hi Simon,
Hi Lukasz,
On Sat, 18 May 2019 at 15:28, Lukasz Majewski lukma@denx.de wrote:
Hi Simon,
This is not the newest patch set version of CCF (v3 vs. v4), but the comments/issues apply.
Hi Lukasz,
On Thu, 25 Apr 2019 at 04:30, Lukasz Majewski lukma@denx.de wrote:
This patch series brings the files from Linux kernel to provide clocks support as it is used on the Linux kernel with common clock framework [CCF] setup.
This series also fixes several problems with current clocks and provides sandbox tests for functions addded to clk-uclass.c file.
Design decisions/issues:
- U-boot's DM for clk differs from Linux CCF. The most notably
difference is the lack of support for hierarchical clocks and "clock as a manager driver" (single clock DTS node acts as a starting point for all other clocks).
- The clk_get_rate() now caches the previously read data (no
need for recursive access.
- On purpose the "manager" clk driver (clk-imx6q.c) is not using
large table to store pointers to clocks - e.g. clk[IMX6QDL_CLK_USDHC2_SEL] = .... Instead we use udevice's linked list for the same class (UCLASS_CLK). The rationale - when porting the code as is from Linux, one would need ~1KiB of RAM to store it. This is way too much if we do plan to use this driver in SPL.
- The "central" structure of this patch series is struct udevice
and its driver_data field contains the struct clk pointer (to the originally created one).
- Up till now U-boot's driver model's CLK operates on udevice
(main access to clock is by udevice ops) In the CCF the access to struct clk (comprising pointer to *dev) is possible via dev_get_driver_data()
Storing back pointer (from udevice to struct clk) as driver_data is a convention for CCF.
Ick. Why not use uclass-private data to store this, since every UCLASS_CLK device can have a parent.
The "private_data" field would be also a good place to store the back pointer from udevice to struct clk [*]. The problem with CCF and udevice's priv pointer is explained just below:
- I could use *private_alloc_size to allocate driver's 'private" structures (dev->priv) for e.g. divider (struct clk_divider
*divider) for IMX6Q clock, but this would change the original structure of the CCF code.
The original Linux's CCF code for iMX relies on using kmalloc internally:
https://elixir.bootlin.com/linux/v5.1.2/source/drivers/clk/imx/clk-gate2.c#L... https://elixir.bootlin.com/linux/v5.1.2/source/drivers/clk/clk-divider.c#L47...
By using driver_data I've avoided the need to make more changes to the original Linux code.
I could use udevice's priv with automatic data allocation but then the CCF ported code would require more changes and considering the (from the outset) need to "fit" this code into U-Boot's DM, it drives away from the original Linux code.
Is the main change the need to cast driver_data?
The main change would be to remove the per clock device memory allocation code (with exit paths) from the original CCF code.
This shall not be so difficult.
Perhaps that could be hidden in a helper function/macro, so that in U-Boot it can hide the use of (struct clk_uc_priv *)dev_get_uclass_priv(clk->dev))>parent ?
Helper function would help to some extend as we operate on structures similar to:
struct clk_gate2 { struct clk clk; void __iomem *reg; u8 bit_idx; u8 cgr_val; u8 flags; };
The helper would return struct clk's address which is the same as struct's clk_gate2 (this is assured by C standard).
The question is if it would be better to use private_alloc_size (and dev->private) or stay with driver_data. The former requires some rewritting in CCF original code (to remove (c)malloc, etc), but comply with u-boot DM. The latter allows re-using the CCF code as is, but introduces some convention special for CCF (I'm not sure thought if dev->priv is NOT another convention as well).
Yes I would like to avoid malloc() calls in drivers and use the in-built mechanism.
I see your point.
If the community agrees - I can rewrite the code to use such approach (but issues pointed out in [*] still apply).
- I've added the clk_get_parent(), which reads parent's
dev->driver_data to provide parent's struct clk pointer. This seems the easiest way to get child/parent relationship for struct clk in U-boot's udevice based clocks.
- For tests I had to "emulate" CCF code structure to test
functionality of clk_get_parent_rate() and clk_get_by_id(). Those functions will not work properly with "standard" (i.e. non CCF) clock setup(with not set dev->driver_data to struct clk).
Well I think we need a better approach for that anywat. driver_data is used for getting something from the DT.
Maybe the name (driver_data) was a bit misleading then. For CCF it stores the back pointer to struct clk (as in fact it is a CCF's "driver data").
Well it seems like a hack to me. Perhaps there is a good reason for it in Linux?
In Linux there is another approach - namely the struct clk (which is the main struct for clock management) has pointer to struct clk_core, which has pointer to parent(s).
https://elixir.bootlin.com/linux/v5.1.2/source/drivers/clk/clk.c#L43
In the case of U-Boot - the CCF wants to work on struct clk, but the _main_ data structure for U-Boot is struct udevice. Hence the need to have a back pointer (or force struct clk to have NOT pointer to udevice, but the udevice itself - then container_of would then do the trick).
Or is it just convenience?
As stated above - Linux all necessary information has accessible from struct clk.
NOTE:
[*] - I do have a hard time to understand how struct clk shall work with struct udevice?
In Linux or Barebox the struct clk is the "main" structure to hold the clock management data (like freq, ops, flags, parent/sibling relation, etc).
Yes U-Boot has a uniform struct udevice for every device and struct uclass for every class.
But the interesting thing here is that clocks have their own parent/sibling relationships, quite independent from the device tree.
But there would be no harm if we could re-use udevice for it. In the current CCF (v4) patch set each clk IP block (like mux or gate) is modelled as udevice:
A side observation - we now have three different implementations of struct clk in U-Boot :-) (two of which have *ops inside :-) )
Oh dear.
The broadcom iMX ones needs to be converted.
In the case of U-Boot's DM (./include/clk.h) it only has a _pointer_ to udevice (which means that I cannot get the struct clk easily from udevice with container_of()). The struct udevice has instead the *ops and *parent pointer (to another udevice).
Yes that's correct. The struct clk is actually a handle to the clock, and includes an ID number.
You mean the ID number of the clock ?
Two problems:
- Linux CCF code uses massively "struct clk" to handle clock
operations (but not udevice)
OK.
- There is no clear idea of how to implement
struct clk *clk_get_parent(struct clk *) in U-Boot.
As above, it seems that this might need to be implemented. I don't think the DM parent/child relationships are good enough for clk, since they are not aware of the ID.
The reason is that we lack clear information about which udevice's data field shall be used to store the back pointer from udevice to struct clk.
Any hints and ideas are more than welcome.
I think it would be good to get Stephen Warren's thoughts on this as he made the change to introduce struct clk.
Ok.
But at present clk_set_parent() is implemented by calling into the driver. The uclass itself does not maintain information about what is a parent of what.
Linux struct clk has easy access to its parent's struct clk. This is what is missing in U-Boot's DM.
Do we *need* to maintain this information in the uclass?
I think that we don't need. It would be enough to modify struct clk to has struct udevice embedded in it (as Linux has struct clk_core), not the pointer. Then we can use container_of to get clock and re-use struct udevice's parent pointer (and maybe UCLASS_CLK list of devices).
I think it would be prohibitively expensive to separate out each individual clock into a separate device (udevice), but that would work.
This is the approach as I use now in CCF v4: https://pastebin.com/uVmwv5FT
It is expensive but logically correct as each mux, gate, pll is the other CLK IP block (device).
The only other option I see is to create a sibling list and parent pointer inside struct clk.
This would be the approach similar to Linux kernel approach.
However, I don't know what were original needs of struct clk (as it did not have it). Maybe Stephen can shed some light on it?
I suspect this will affect power domain also, although we don't have that yet.
iMX8 has some clocks which needs to be always recalculated as they depend on power domains which can be disabled.
Do you think there is a case for building this into DM itself, such that devices can have a list of IDs for each device, each with independent parent/child relationships?
For iMX6Q the ID of the clock is used to get proper clock in drivers (or from DTS). All clock's udevices are stored into UCLASS_CLK list. With the ID we get proper udevice and from it and via driver_data the struct clk, which is then used in the CCF code to operate on clock devices (PLL, gate, mux, etc).
I simply re-used the DM facilities (only missing is the back pointer from udevice to struct clk).
Regards, Simon
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
Hi Lukasz,
On Sun, 19 May 2019 at 15:03, Lukasz Majewski lukma@denx.de wrote:
Hi Simon,
Hi Lukasz,
On Sat, 18 May 2019 at 15:28, Lukasz Majewski lukma@denx.de wrote:
Hi Simon,
This is not the newest patch set version of CCF (v3 vs. v4), but the comments/issues apply.
Hi Lukasz,
On Thu, 25 Apr 2019 at 04:30, Lukasz Majewski lukma@denx.de wrote:
This patch series brings the files from Linux kernel to provide clocks support as it is used on the Linux kernel with common clock framework [CCF] setup.
This series also fixes several problems with current clocks and provides sandbox tests for functions addded to clk-uclass.c file.
Design decisions/issues:
- U-boot's DM for clk differs from Linux CCF. The most notably
difference is the lack of support for hierarchical clocks and "clock as a manager driver" (single clock DTS node acts as a starting point for all other clocks).
- The clk_get_rate() now caches the previously read data (no
need for recursive access.
- On purpose the "manager" clk driver (clk-imx6q.c) is not using
large table to store pointers to clocks - e.g. clk[IMX6QDL_CLK_USDHC2_SEL] = .... Instead we use udevice's linked list for the same class (UCLASS_CLK). The rationale - when porting the code as is from Linux, one would need ~1KiB of RAM to store it. This is way too much if we do plan to use this driver in SPL.
- The "central" structure of this patch series is struct udevice
and its driver_data field contains the struct clk pointer (to the originally created one).
- Up till now U-boot's driver model's CLK operates on udevice
(main access to clock is by udevice ops) In the CCF the access to struct clk (comprising pointer to *dev) is possible via dev_get_driver_data()
Storing back pointer (from udevice to struct clk) as driver_data is a convention for CCF.
Ick. Why not use uclass-private data to store this, since every UCLASS_CLK device can have a parent.
The "private_data" field would be also a good place to store the back pointer from udevice to struct clk [*]. The problem with CCF and udevice's priv pointer is explained just below:
- I could use *private_alloc_size to allocate driver's 'private" structures (dev->priv) for e.g. divider (struct clk_divider
*divider) for IMX6Q clock, but this would change the original structure of the CCF code.
The original Linux's CCF code for iMX relies on using kmalloc internally:
https://elixir.bootlin.com/linux/v5.1.2/source/drivers/clk/imx/clk-gate2.c#L... https://elixir.bootlin.com/linux/v5.1.2/source/drivers/clk/clk-divider.c#L47...
By using driver_data I've avoided the need to make more changes to the original Linux code.
I could use udevice's priv with automatic data allocation but then the CCF ported code would require more changes and considering the (from the outset) need to "fit" this code into U-Boot's DM, it drives away from the original Linux code.
Is the main change the need to cast driver_data?
The main change would be to remove the per clock device memory allocation code (with exit paths) from the original CCF code.
This shall not be so difficult.
Perhaps that could be hidden in a helper function/macro, so that in U-Boot it can hide the use of (struct clk_uc_priv *)dev_get_uclass_priv(clk->dev))>parent ?
Helper function would help to some extend as we operate on structures similar to:
struct clk_gate2 { struct clk clk; void __iomem *reg; u8 bit_idx; u8 cgr_val; u8 flags; };
The helper would return struct clk's address which is the same as struct's clk_gate2 (this is assured by C standard).
The question is if it would be better to use private_alloc_size (and dev->private) or stay with driver_data. The former requires some rewritting in CCF original code (to remove (c)malloc, etc), but comply with u-boot DM. The latter allows re-using the CCF code as is, but introduces some convention special for CCF (I'm not sure thought if dev->priv is NOT another convention as well).
Yes I would like to avoid malloc() calls in drivers and use the in-built mechanism.
I see your point.
If the community agrees - I can rewrite the code to use such approach (but issues pointed out in [*] still apply).
- I've added the clk_get_parent(), which reads parent's
dev->driver_data to provide parent's struct clk pointer. This seems the easiest way to get child/parent relationship for struct clk in U-boot's udevice based clocks.
- For tests I had to "emulate" CCF code structure to test
functionality of clk_get_parent_rate() and clk_get_by_id(). Those functions will not work properly with "standard" (i.e. non CCF) clock setup(with not set dev->driver_data to struct clk).
Well I think we need a better approach for that anywat. driver_data is used for getting something from the DT.
Maybe the name (driver_data) was a bit misleading then. For CCF it stores the back pointer to struct clk (as in fact it is a CCF's "driver data").
Well it seems like a hack to me. Perhaps there is a good reason for it in Linux?
In Linux there is another approach - namely the struct clk (which is the main struct for clock management) has pointer to struct clk_core, which has pointer to parent(s).
https://elixir.bootlin.com/linux/v5.1.2/source/drivers/clk/clk.c#L43
In the case of U-Boot - the CCF wants to work on struct clk, but the _main_ data structure for U-Boot is struct udevice. Hence the need to have a back pointer (or force struct clk to have NOT pointer to udevice, but the udevice itself - then container_of would then do the trick).
The thing I don't understand is that I assumed there is no 1:1 correspondence from struct clk to struct udevice. I thought that we could have one clock device which supports lots of clk IDs (e.g. 0-23).
Or is it just convenience?
As stated above - Linux all necessary information has accessible from struct clk.
Sure, but we can always find the udevice from the clk.
If we require that clk == udevice then we can go back the other way too, by using uclass-private data attached to each device.
NOTE:
[*] - I do have a hard time to understand how struct clk shall work with struct udevice?
In Linux or Barebox the struct clk is the "main" structure to hold the clock management data (like freq, ops, flags, parent/sibling relation, etc).
Yes U-Boot has a uniform struct udevice for every device and struct uclass for every class.
But the interesting thing here is that clocks have their own parent/sibling relationships, quite independent from the device tree.
But there would be no harm if we could re-use udevice for it. In the current CCF (v4) patch set each clk IP block (like mux or gate) is modelled as udevice:
I don't see how you can do this...doesn't it mean changing the parents of existing devices? E.g. if a SPI clock can come from one of 4 parents, do you need to changes its parent in the driver-model tree?
A side observation - we now have three different implementations of struct clk in U-Boot :-) (two of which have *ops inside :-) )
Oh dear.
The broadcom iMX ones needs to be converted.
In the case of U-Boot's DM (./include/clk.h) it only has a _pointer_ to udevice (which means that I cannot get the struct clk easily from udevice with container_of()). The struct udevice has instead the *ops and *parent pointer (to another udevice).
Yes that's correct. The struct clk is actually a handle to the clock, and includes an ID number.
You mean the ID number of the clock ?
Yes:
struct clk { struct udevice *dev; /* * Written by of_xlate. We assume a single id is enough for now. In the * future, we might add more fields here. */ unsigned long id; };
Two problems:
- Linux CCF code uses massively "struct clk" to handle clock
operations (but not udevice)
OK.
- There is no clear idea of how to implement
struct clk *clk_get_parent(struct clk *) in U-Boot.
As above, it seems that this might need to be implemented. I don't think the DM parent/child relationships are good enough for clk, since they are not aware of the ID.
The reason is that we lack clear information about which udevice's data field shall be used to store the back pointer from udevice to struct clk.
Any hints and ideas are more than welcome.
I think it would be good to get Stephen Warren's thoughts on this as he made the change to introduce struct clk.
Ok.
But at present clk_set_parent() is implemented by calling into the driver. The uclass itself does not maintain information about what is a parent of what.
Linux struct clk has easy access to its parent's struct clk. This is what is missing in U-Boot's DM.
Do we *need* to maintain this information in the uclass?
I think that we don't need. It would be enough to modify struct clk to has struct udevice embedded in it (as Linux has struct clk_core), not the pointer. Then we can use container_of to get clock and re-use struct udevice's parent pointer (and maybe UCLASS_CLK list of devices).
I think it would be prohibitively expensive to separate out each individual clock into a separate device (udevice), but that would work.
This is the approach as I use now in CCF v4: https://pastebin.com/uVmwv5FT
It is expensive but logically correct as each mux, gate, pll is the other CLK IP block (device).
OK I see. What is the cost? Is it acceptable for a boot loader?
The only other option I see is to create a sibling list and parent pointer inside struct clk.
This would be the approach similar to Linux kernel approach.
However, I don't know what were original needs of struct clk (as it did not have it). Maybe Stephen can shed some light on it?
Hopefully.
I suspect this will affect power domain also, although we don't have that yet.
iMX8 has some clocks which needs to be always recalculated as they depend on power domains which can be disabled.
OK
Do you think there is a case for building this into DM itself, such that devices can have a list of IDs for each device, each with independent parent/child relationships?
For iMX6Q the ID of the clock is used to get proper clock in drivers (or from DTS). All clock's udevices are stored into UCLASS_CLK list. With the ID we get proper udevice and from it and via driver_data the struct clk, which is then used in the CCF code to operate on clock devices (PLL, gate, mux, etc).
I simply re-used the DM facilities (only missing is the back pointer from udevice to struct clk).
Well then you can use dev_get_uclass_priv() for that.
Regards, Simon
Hi Simon,
Hi Lukasz,
On Sun, 19 May 2019 at 15:03, Lukasz Majewski lukma@denx.de wrote:
Hi Simon,
Hi Lukasz,
On Sat, 18 May 2019 at 15:28, Lukasz Majewski lukma@denx.de wrote:
Hi Simon,
This is not the newest patch set version of CCF (v3 vs. v4), but the comments/issues apply.
Hi Lukasz,
On Thu, 25 Apr 2019 at 04:30, Lukasz Majewski lukma@denx.de wrote:
This patch series brings the files from Linux kernel to provide clocks support as it is used on the Linux kernel with common clock framework [CCF] setup.
This series also fixes several problems with current clocks and provides sandbox tests for functions addded to clk-uclass.c file.
Design decisions/issues:
- U-boot's DM for clk differs from Linux CCF. The most
notably difference is the lack of support for hierarchical clocks and "clock as a manager driver" (single clock DTS node acts as a starting point for all other clocks).
- The clk_get_rate() now caches the previously read data (no
need for recursive access.
- On purpose the "manager" clk driver (clk-imx6q.c) is not
using large table to store pointers to clocks - e.g. clk[IMX6QDL_CLK_USDHC2_SEL] = .... Instead we use udevice's linked list for the same class (UCLASS_CLK). The rationale - when porting the code as is from Linux, one would need ~1KiB of RAM to store it. This is way too much if we do plan to use this driver in SPL.
- The "central" structure of this patch series is struct
udevice and its driver_data field contains the struct clk pointer (to the originally created one).
- Up till now U-boot's driver model's CLK operates on
udevice (main access to clock is by udevice ops) In the CCF the access to struct clk (comprising pointer to *dev) is possible via dev_get_driver_data()
Storing back pointer (from udevice to struct clk) as driver_data is a convention for CCF.
Ick. Why not use uclass-private data to store this, since every UCLASS_CLK device can have a parent.
The "private_data" field would be also a good place to store the back pointer from udevice to struct clk [*]. The problem with CCF and udevice's priv pointer is explained just below:
- I could use *private_alloc_size to allocate driver's
'private" structures (dev->priv) for e.g. divider (struct clk_divider *divider) for IMX6Q clock, but this would change the original structure of the CCF code.
The original Linux's CCF code for iMX relies on using kmalloc internally:
https://elixir.bootlin.com/linux/v5.1.2/source/drivers/clk/imx/clk-gate2.c#L... https://elixir.bootlin.com/linux/v5.1.2/source/drivers/clk/clk-divider.c#L47...
By using driver_data I've avoided the need to make more changes to the original Linux code.
I could use udevice's priv with automatic data allocation but then the CCF ported code would require more changes and considering the (from the outset) need to "fit" this code into U-Boot's DM, it drives away from the original Linux code.
Is the main change the need to cast driver_data?
The main change would be to remove the per clock device memory allocation code (with exit paths) from the original CCF code.
This shall not be so difficult.
Perhaps that could be hidden in a helper function/macro, so that in U-Boot it can hide the use of (struct clk_uc_priv *)dev_get_uclass_priv(clk->dev))>parent ?
Helper function would help to some extend as we operate on structures similar to:
struct clk_gate2 { struct clk clk; void __iomem *reg; u8 bit_idx; u8 cgr_val; u8 flags; };
The helper would return struct clk's address which is the same as struct's clk_gate2 (this is assured by C standard).
The question is if it would be better to use private_alloc_size (and dev->private) or stay with driver_data. The former requires some rewritting in CCF original code (to remove (c)malloc, etc), but comply with u-boot DM. The latter allows re-using the CCF code as is, but introduces some convention special for CCF (I'm not sure thought if dev->priv is NOT another convention as well).
Yes I would like to avoid malloc() calls in drivers and use the in-built mechanism.
I see your point.
If the community agrees - I can rewrite the code to use such approach (but issues pointed out in [*] still apply).
- I've added the clk_get_parent(), which reads parent's
dev->driver_data to provide parent's struct clk pointer. This seems the easiest way to get child/parent relationship for struct clk in U-boot's udevice based clocks.
- For tests I had to "emulate" CCF code structure to test
functionality of clk_get_parent_rate() and clk_get_by_id(). Those functions will not work properly with "standard" (i.e. non CCF) clock setup(with not set dev->driver_data to struct clk).
Well I think we need a better approach for that anywat. driver_data is used for getting something from the DT.
Maybe the name (driver_data) was a bit misleading then. For CCF it stores the back pointer to struct clk (as in fact it is a CCF's "driver data").
Well it seems like a hack to me. Perhaps there is a good reason for it in Linux?
In Linux there is another approach - namely the struct clk (which is the main struct for clock management) has pointer to struct clk_core, which has pointer to parent(s).
https://elixir.bootlin.com/linux/v5.1.2/source/drivers/clk/clk.c#L43
In the case of U-Boot - the CCF wants to work on struct clk, but the _main_ data structure for U-Boot is struct udevice. Hence the need to have a back pointer (or force struct clk to have NOT pointer to udevice, but the udevice itself - then container_of would then do the trick).
The thing I don't understand is that I assumed there is no 1:1 correspondence from struct clk to struct udevice.
For CCF there is 1:1 match - i.e. each struct udevice has a matching struct clk.
I thought that we could have one clock device which supports lots of clk IDs (e.g. 0-23).
On IMX CCF the clock ID is a unique number to refer to a clock.
For example 163 is spi0, 164 is spi1, 165 is spi2, etc. The CCF uses this ID to get proper struct clk.
In case of CCF port in U-Boot we use clk ID to get proper udevice by searching UCLASS_CLK devices linked together. Then the struct clk is read with: struct clk *clk = (struct clk *)dev_get_driver_data(dev); And we do have match when clk->id == id.
Having one device supporting many IDs would be not compatible with CCF where each clock has its own instance of device and clock specific structure.
Or is it just convenience?
As stated above - Linux all necessary information has accessible from struct clk.
Sure, but we can always find the udevice from the clk.
Yes, correct.
However clocks (represented as struct udevices) are linked in a single, common uclass (UCLASS_CLK).
If we require that clk == udevice then we can go back the other way too, by using uclass-private data attached to each device.
That may work.
The struct udevice's priv would hold clock specific data structure (like struct clk_gate2 - automatically allocated and released).
And the uclass_priv would contain the pointer to struct clk itself (however, in CCF it is always the same as clock specific data structure - being the first field in the structure).
NOTE:
[*] - I do have a hard time to understand how struct clk shall work with struct udevice?
In Linux or Barebox the struct clk is the "main" structure to hold the clock management data (like freq, ops, flags, parent/sibling relation, etc).
Yes U-Boot has a uniform struct udevice for every device and struct uclass for every class.
But the interesting thing here is that clocks have their own parent/sibling relationships, quite independent from the device tree.
But there would be no harm if we could re-use udevice for it. In the current CCF (v4) patch set each clk IP block (like mux or gate) is modelled as udevice:
I don't see how you can do this...doesn't it mean changing the parents of existing devices? E.g. if a SPI clock can come from one of 4 parents, do you need to changes its parent in the driver-model tree?
Yes. The hierarchy is build when the "generic" clock driver is probed (@ clk/imx/clk-imx6q.c) by using the udevice's parent pointer.
If afterwards I need to change clock parent, then I simply change parent pointers in udevices.
A side observation - we now have three different implementations of struct clk in U-Boot :-) (two of which have *ops inside :-) )
Oh dear.
The broadcom iMX ones needs to be converted.
In the case of U-Boot's DM (./include/clk.h) it only has a _pointer_ to udevice (which means that I cannot get the struct clk easily from udevice with container_of()). The struct udevice has instead the *ops and *parent pointer (to another udevice).
Yes that's correct. The struct clk is actually a handle to the clock, and includes an ID number.
You mean the ID number of the clock ?
Yes:
struct clk { struct udevice *dev; /*
- Written by of_xlate. We assume a single id is enough for now. In the
- future, we might add more fields here.
*/ unsigned long id; };
Two problems:
- Linux CCF code uses massively "struct clk" to handle clock
operations (but not udevice)
OK.
- There is no clear idea of how to implement
struct clk *clk_get_parent(struct clk *) in U-Boot.
As above, it seems that this might need to be implemented. I don't think the DM parent/child relationships are good enough for clk, since they are not aware of the ID.
The reason is that we lack clear information about which udevice's data field shall be used to store the back pointer from udevice to struct clk.
Any hints and ideas are more than welcome.
I think it would be good to get Stephen Warren's thoughts on this as he made the change to introduce struct clk.
Ok.
But at present clk_set_parent() is implemented by calling into the driver. The uclass itself does not maintain information about what is a parent of what.
Linux struct clk has easy access to its parent's struct clk. This is what is missing in U-Boot's DM.
Do we *need* to maintain this information in the uclass?
I think that we don't need. It would be enough to modify struct clk to has struct udevice embedded in it (as Linux has struct clk_core), not the pointer. Then we can use container_of to get clock and re-use struct udevice's parent pointer (and maybe UCLASS_CLK list of devices).
I think it would be prohibitively expensive to separate out each individual clock into a separate device (udevice), but that would work.
This is the approach as I use now in CCF v4: https://pastebin.com/uVmwv5FT
It is expensive but logically correct as each mux, gate, pll is the other CLK IP block (device).
OK I see. What is the cost? Is it acceptable for a boot loader?
To make it really small we would need to use OF_PLATDATA.
For e.g. iMX6Q - I'm able to boot with this CCF port running in both SPL and U-Boot proper.
But, yes the cost is to have the larger binary as we do have larger section with udevices linker list.
Original Linux CCF code uses ~1KiB dynamic table to store pointers to struct clk addressed by ID number. In U-Boot instead - I add those devices to UCLASS_CLK list of devices (as 1KiB is a lot for SPL).
The only other option I see is to create a sibling list and parent pointer inside struct clk.
This would be the approach similar to Linux kernel approach.
However, I don't know what were original needs of struct clk (as it did not have it). Maybe Stephen can shed some light on it?
Hopefully.
I suspect this will affect power domain also, although we don't have that yet.
iMX8 has some clocks which needs to be always recalculated as they depend on power domains which can be disabled.
OK
Do you think there is a case for building this into DM itself, such that devices can have a list of IDs for each device, each with independent parent/child relationships?
For iMX6Q the ID of the clock is used to get proper clock in drivers (or from DTS). All clock's udevices are stored into UCLASS_CLK list. With the ID we get proper udevice and from it and via driver_data the struct clk, which is then used in the CCF code to operate on clock devices (PLL, gate, mux, etc).
I simply re-used the DM facilities (only missing is the back pointer from udevice to struct clk).
Well then you can use dev_get_uclass_priv() for that.
I think that it might be enough to use udevice's priv as clock specific structure (struct clk_gate2) has the struct clock embedded in it.
Regards, Simon
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
Hi Lukasz,
On Tue, 21 May 2019 at 08:48, Lukasz Majewski lukma@denx.de wrote:
Hi Simon,
Hi Lukasz,
On Sun, 19 May 2019 at 15:03, Lukasz Majewski lukma@denx.de wrote:
Hi Simon,
Hi Lukasz,
On Sat, 18 May 2019 at 15:28, Lukasz Majewski lukma@denx.de wrote:
Hi Simon,
This is not the newest patch set version of CCF (v3 vs. v4), but the comments/issues apply.
Hi Lukasz,
On Thu, 25 Apr 2019 at 04:30, Lukasz Majewski lukma@denx.de wrote: > > This patch series brings the files from Linux kernel to > provide clocks support as it is used on the Linux kernel > with common clock framework [CCF] setup. > > This series also fixes several problems with current clocks > and provides sandbox tests for functions addded to > clk-uclass.c file. > > Design decisions/issues: > ========================= > > - U-boot's DM for clk differs from Linux CCF. The most > notably difference is the lack of support for hierarchical > clocks and "clock as a manager driver" (single clock DTS > node acts as a starting point for all other clocks). > > - The clk_get_rate() now caches the previously read data (no > need for recursive access. > > - On purpose the "manager" clk driver (clk-imx6q.c) is not > using large table to store pointers to clocks - e.g. > clk[IMX6QDL_CLK_USDHC2_SEL] = .... Instead we use udevice's > linked list for the same class (UCLASS_CLK). The rationale - > when porting the code as is from Linux, one would need > ~1KiB of RAM to store it. This is way too much if we do > plan to use this driver in SPL. > > - The "central" structure of this patch series is struct > udevice and its driver_data field contains the struct clk > pointer (to the originally created one). > > - Up till now U-boot's driver model's CLK operates on > udevice (main access to clock is by udevice ops) > In the CCF the access to struct clk (comprising pointer to > *dev) is possible via dev_get_driver_data() > > Storing back pointer (from udevice to struct clk) as > driver_data is a convention for CCF.
Ick. Why not use uclass-private data to store this, since every UCLASS_CLK device can have a parent.
The "private_data" field would be also a good place to store the back pointer from udevice to struct clk [*]. The problem with CCF and udevice's priv pointer is explained just below:
> > - I could use *private_alloc_size to allocate driver's > 'private" structures (dev->priv) for e.g. divider (struct > clk_divider *divider) for IMX6Q clock, but this would > change the original structure of the CCF code.
The original Linux's CCF code for iMX relies on using kmalloc internally:
https://elixir.bootlin.com/linux/v5.1.2/source/drivers/clk/imx/clk-gate2.c#L... https://elixir.bootlin.com/linux/v5.1.2/source/drivers/clk/clk-divider.c#L47...
By using driver_data I've avoided the need to make more changes to the original Linux code.
I could use udevice's priv with automatic data allocation but then the CCF ported code would require more changes and considering the (from the outset) need to "fit" this code into U-Boot's DM, it drives away from the original Linux code.
Is the main change the need to cast driver_data?
The main change would be to remove the per clock device memory allocation code (with exit paths) from the original CCF code.
This shall not be so difficult.
Perhaps that could be hidden in a helper function/macro, so that in U-Boot it can hide the use of (struct clk_uc_priv *)dev_get_uclass_priv(clk->dev))>parent ?
Helper function would help to some extend as we operate on structures similar to:
struct clk_gate2 { struct clk clk; void __iomem *reg; u8 bit_idx; u8 cgr_val; u8 flags; };
The helper would return struct clk's address which is the same as struct's clk_gate2 (this is assured by C standard).
> > The question is if it would be better to use > private_alloc_size (and dev->private) or stay with > driver_data. The former requires some rewritting in CCF > original code (to remove (c)malloc, etc), but comply with > u-boot DM. The latter allows re-using the CCF code as is, > but introduces some convention special for CCF (I'm not > sure thought if dev->priv is NOT another convention as > well).
Yes I would like to avoid malloc() calls in drivers and use the in-built mechanism.
I see your point.
If the community agrees - I can rewrite the code to use such approach (but issues pointed out in [*] still apply).
> > - I've added the clk_get_parent(), which reads parent's > dev->driver_data to provide parent's struct clk pointer. > This seems the easiest way to get child/parent relationship > for struct clk in U-boot's udevice based clocks. > > - For tests I had to "emulate" CCF code structure to test > functionality of clk_get_parent_rate() and clk_get_by_id(). > Those functions will not work properly with "standard" (i.e. > non CCF) clock setup(with not set dev->driver_data to struct > clk).
Well I think we need a better approach for that anywat. driver_data is used for getting something from the DT.
Maybe the name (driver_data) was a bit misleading then. For CCF it stores the back pointer to struct clk (as in fact it is a CCF's "driver data").
Well it seems like a hack to me. Perhaps there is a good reason for it in Linux?
In Linux there is another approach - namely the struct clk (which is the main struct for clock management) has pointer to struct clk_core, which has pointer to parent(s).
https://elixir.bootlin.com/linux/v5.1.2/source/drivers/clk/clk.c#L43
In the case of U-Boot - the CCF wants to work on struct clk, but the _main_ data structure for U-Boot is struct udevice. Hence the need to have a back pointer (or force struct clk to have NOT pointer to udevice, but the udevice itself - then container_of would then do the trick).
The thing I don't understand is that I assumed there is no 1:1 correspondence from struct clk to struct udevice.
For CCF there is 1:1 match - i.e. each struct udevice has a matching struct clk.
I thought that we could have one clock device which supports lots of clk IDs (e.g. 0-23).
On IMX CCF the clock ID is a unique number to refer to a clock.
For example 163 is spi0, 164 is spi1, 165 is spi2, etc. The CCF uses this ID to get proper struct clk.
In case of CCF port in U-Boot we use clk ID to get proper udevice by searching UCLASS_CLK devices linked together. Then the struct clk is read with: struct clk *clk = (struct clk *)dev_get_driver_data(dev); And we do have match when clk->id == id.
Having one device supporting many IDs would be not compatible with CCF where each clock has its own instance of device and clock specific structure.
OK good.
Or is it just convenience?
As stated above - Linux all necessary information has accessible from struct clk.
Sure, but we can always find the udevice from the clk.
Yes, correct.
However clocks (represented as struct udevices) are linked in a single, common uclass (UCLASS_CLK).
OK good.
If we require that clk == udevice then we can go back the other way too, by using uclass-private data attached to each device.
That may work.
The struct udevice's priv would hold clock specific data structure (like struct clk_gate2 - automatically allocated and released).
And the uclass_priv would contain the pointer to struct clk itself (however, in CCF it is always the same as clock specific data structure
- being the first field in the structure).
Yes, or struct clk_uc_priv could contain a *member* which is a pointer to struct clk.
NOTE:
[*] - I do have a hard time to understand how struct clk shall work with struct udevice?
In Linux or Barebox the struct clk is the "main" structure to hold the clock management data (like freq, ops, flags, parent/sibling relation, etc).
Yes U-Boot has a uniform struct udevice for every device and struct uclass for every class.
But the interesting thing here is that clocks have their own parent/sibling relationships, quite independent from the device tree.
But there would be no harm if we could re-use udevice for it. In the current CCF (v4) patch set each clk IP block (like mux or gate) is modelled as udevice:
I don't see how you can do this...doesn't it mean changing the parents of existing devices? E.g. if a SPI clock can come from one of 4 parents, do you need to changes its parent in the driver-model tree?
Yes. The hierarchy is build when the "generic" clock driver is probed (@ clk/imx/clk-imx6q.c) by using the udevice's parent pointer.
If afterwards I need to change clock parent, then I simply change parent pointers in udevices.
Well if you do that, you need to update the sibling lists (device->sibling_node).
A side observation - we now have three different implementations of struct clk in U-Boot :-) (two of which have *ops inside :-) )
Oh dear.
The broadcom iMX ones needs to be converted.
In the case of U-Boot's DM (./include/clk.h) it only has a _pointer_ to udevice (which means that I cannot get the struct clk easily from udevice with container_of()). The struct udevice has instead the *ops and *parent pointer (to another udevice).
Yes that's correct. The struct clk is actually a handle to the clock, and includes an ID number.
You mean the ID number of the clock ?
Yes:
struct clk { struct udevice *dev; /*
- Written by of_xlate. We assume a single id is enough for now. In the
- future, we might add more fields here.
*/ unsigned long id; };
Two problems:
- Linux CCF code uses massively "struct clk" to handle clock
operations (but not udevice)
OK.
- There is no clear idea of how to implement
struct clk *clk_get_parent(struct clk *) in U-Boot.
As above, it seems that this might need to be implemented. I don't think the DM parent/child relationships are good enough for clk, since they are not aware of the ID.
The reason is that we lack clear information about which udevice's data field shall be used to store the back pointer from udevice to struct clk.
Any hints and ideas are more than welcome.
I think it would be good to get Stephen Warren's thoughts on this as he made the change to introduce struct clk.
Ok.
But at present clk_set_parent() is implemented by calling into the driver. The uclass itself does not maintain information about what is a parent of what.
Linux struct clk has easy access to its parent's struct clk. This is what is missing in U-Boot's DM.
Do we *need* to maintain this information in the uclass?
I think that we don't need. It would be enough to modify struct clk to has struct udevice embedded in it (as Linux has struct clk_core), not the pointer. Then we can use container_of to get clock and re-use struct udevice's parent pointer (and maybe UCLASS_CLK list of devices).
I think it would be prohibitively expensive to separate out each individual clock into a separate device (udevice), but that would work.
This is the approach as I use now in CCF v4: https://pastebin.com/uVmwv5FT
It is expensive but logically correct as each mux, gate, pll is the other CLK IP block (device).
OK I see. What is the cost? Is it acceptable for a boot loader?
To make it really small we would need to use OF_PLATDATA.
For e.g. iMX6Q - I'm able to boot with this CCF port running in both SPL and U-Boot proper.
But, yes the cost is to have the larger binary as we do have larger section with udevices linker list.
Original Linux CCF code uses ~1KiB dynamic table to store pointers to struct clk addressed by ID number. In U-Boot instead - I add those devices to UCLASS_CLK list of devices (as 1KiB is a lot for SPL).
OK.
The only other option I see is to create a sibling list and parent pointer inside struct clk.
This would be the approach similar to Linux kernel approach.
However, I don't know what were original needs of struct clk (as it did not have it). Maybe Stephen can shed some light on it?
Hopefully.
I suspect this will affect power domain also, although we don't have that yet.
iMX8 has some clocks which needs to be always recalculated as they depend on power domains which can be disabled.
OK
Do you think there is a case for building this into DM itself, such that devices can have a list of IDs for each device, each with independent parent/child relationships?
For iMX6Q the ID of the clock is used to get proper clock in drivers (or from DTS). All clock's udevices are stored into UCLASS_CLK list. With the ID we get proper udevice and from it and via driver_data the struct clk, which is then used in the CCF code to operate on clock devices (PLL, gate, mux, etc).
I simply re-used the DM facilities (only missing is the back pointer from udevice to struct clk).
Well then you can use dev_get_uclass_priv() for that.
I think that it might be enough to use udevice's priv as clock specific structure (struct clk_gate2) has the struct clock embedded in it.
Why do that? The uclass is specifically designed to hold data that is common to the uclass.
Regards, Simon
Hi All
Will we force to use CCF in future? And Is there possibility this feature would land in v2019.07? If not, I'll try to convert i.MX8MM CLK to no-CCF DM CLK version, since i.MX8MM has been pending for long time.
Thanks, Peng.
Subject: Re: [PATCH v3 00/11] clk: Port Linux common clock framework [CCF] to U-boot (tag: 5.0-rc3)
Hi Lukasz,
On Tue, 21 May 2019 at 08:48, Lukasz Majewski lukma@denx.de wrote:
Hi Simon,
Hi Lukasz,
On Sun, 19 May 2019 at 15:03, Lukasz Majewski lukma@denx.de
wrote:
Hi Simon,
Hi Lukasz,
On Sat, 18 May 2019 at 15:28, Lukasz Majewski lukma@denx.de wrote:
Hi Simon,
This is not the newest patch set version of CCF (v3 vs. v4), but the comments/issues apply.
> Hi Lukasz, > > On Thu, 25 Apr 2019 at 04:30, Lukasz Majewski > lukma@denx.de > wrote: > > > > This patch series brings the files from Linux kernel to > > provide clocks support as it is used on the Linux kernel > > with common clock framework [CCF] setup. > > > > This series also fixes several problems with current > > clocks and provides sandbox tests for functions addded to > > clk-uclass.c file. > > > > Design decisions/issues: > > ========================= > > > > - U-boot's DM for clk differs from Linux CCF. The most > > notably difference is the lack of support for hierarchical > > clocks and "clock as a manager driver" (single clock DTS > > node acts as a starting point for all other clocks). > > > > - The clk_get_rate() now caches the previously read data > > (no need for recursive access. > > > > - On purpose the "manager" clk driver (clk-imx6q.c) is not > > using large table to store pointers to clocks - e.g. > > clk[IMX6QDL_CLK_USDHC2_SEL] = .... Instead we use > > udevice's linked list for the same class (UCLASS_CLK). The > > rationale - when porting the code as is from Linux, one > > would need ~1KiB of RAM to store it. This is way too much > > if we do plan to use this driver in SPL. > > > > - The "central" structure of this patch series is struct > > udevice and its driver_data field contains the struct clk > > pointer (to the originally created one). > > > > - Up till now U-boot's driver model's CLK operates on > > udevice (main access to clock is by udevice ops) > > In the CCF the access to struct clk (comprising pointer > > to > > *dev) is possible via dev_get_driver_data() > > > > Storing back pointer (from udevice to struct clk) as > > driver_data is a convention for CCF. > > Ick. Why not use uclass-private data to store this, since > every UCLASS_CLK device can have a parent.
The "private_data" field would be also a good place to store the back pointer from udevice to struct clk [*]. The problem with CCF and udevice's priv pointer is explained just below:
> > > > > - I could use *private_alloc_size to allocate driver's > > 'private" structures (dev->priv) for e.g. divider (struct > > clk_divider *divider) for IMX6Q clock, but this would > > change the original structure of the CCF code.
The original Linux's CCF code for iMX relies on using kmalloc internally:
https://eur01.safelinks.protection.outlook.com/?url=https%3A%2 F%2Felixir.bootlin.com%2Flinux%2Fv5.1.2%2Fsource%2Fdrivers%2Fc
lk%2Fimx%2Fclk-gate2.c%23L139&data=02%7C01%7Cpeng.fan%40nx
p.com%7C995c4869e12e471bdb5108d6de4feda5%7C686ea1d3bc2b4c6fa92
cd99c5c301635%7C0%7C0%7C636940832249605030&sdata=NJdvX7JfV
g2Qd8H%2FEmnzeNS1v5xhz4AaMRhSUpo7MxI%3D&reserved=0
https://eur01.safelinks.protection.outlook.com/?url=https%3A%2 F%2Felixir.bootlin.com%2Flinux%2Fv5.1.2%2Fsource%2Fdrivers%2Fc
lk%2Fclk-divider.c%23L471&data=02%7C01%7Cpeng.fan%40nxp.co
m%7C995c4869e12e471bdb5108d6de4feda5%7C686ea1d3bc2b4c6fa92cd99
c5c301635%7C0%7C0%7C636940832249605030&sdata=iF66y5IKaQY69
vyFV%2BY5HSsZiKDb4s%2BN2yMPOBNDOqA%3D&reserved=0
By using driver_data I've avoided the need to make more changes to the original Linux code.
I could use udevice's priv with automatic data allocation but then the CCF ported code would require more changes and considering the (from the outset) need to "fit" this code into U-Boot's DM, it drives away from the original Linux code.
Is the main change the need to cast driver_data?
The main change would be to remove the per clock device memory allocation code (with exit paths) from the original CCF code.
This shall not be so difficult.
Perhaps that could be hidden in a helper function/macro, so that in U-Boot it can hide the use of (struct clk_uc_priv *)dev_get_uclass_priv(clk->dev))>parent ?
Helper function would help to some extend as we operate on structures similar to:
struct clk_gate2 { struct clk clk; void __iomem *reg; u8 bit_idx; u8 cgr_val; u8 flags; };
The helper would return struct clk's address which is the same as struct's clk_gate2 (this is assured by C standard).
> > > > The question is if it would be better to use > > private_alloc_size (and dev->private) or stay with > > driver_data. The former requires some rewritting in CCF > > original code (to remove (c)malloc, etc), but comply with > > u-boot DM. The latter allows re-using the CCF code as is, > > but introduces some convention special for CCF (I'm not > > sure thought if dev->priv is NOT another convention as > > well). > > Yes I would like to avoid malloc() calls in drivers and use > the in-built mechanism.
I see your point.
If the community agrees - I can rewrite the code to use such approach (but issues pointed out in [*] still apply).
> > > > > - I've added the clk_get_parent(), which reads parent's > > dev->driver_data to provide parent's struct clk pointer. > > This seems the easiest way to get child/parent > > relationship for struct clk in U-boot's udevice based clocks. > > > > - For tests I had to "emulate" CCF code structure to test > > functionality of clk_get_parent_rate() and clk_get_by_id(). > > Those functions will not work properly with "standard" (i.e. > > non CCF) clock setup(with not set dev->driver_data to > > struct clk). > > Well I think we need a better approach for that anywat. > driver_data is used for getting something from the DT.
Maybe the name (driver_data) was a bit misleading then. For CCF it stores the back pointer to struct clk (as in fact it is a CCF's "driver data").
Well it seems like a hack to me. Perhaps there is a good reason for it in Linux?
In Linux there is another approach - namely the struct clk (which is the main struct for clock management) has pointer to struct clk_core, which has pointer to parent(s).
https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2F elixir.bootlin.com%2Flinux%2Fv5.1.2%2Fsource%2Fdrivers%2Fclk%2Fclk .c%23L43&data=02%7C01%7Cpeng.fan%40nxp.com%7C995c4869
e12e471bd
b5108d6de4feda5%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C 63694
0832249605030&sdata=63VsTs6JusNw%2FT4mBpfsFLUZKXtyTpZlYx99jy 36
6Mk%3D&reserved=0
In the case of U-Boot - the CCF wants to work on struct clk, but the _main_ data structure for U-Boot is struct udevice. Hence the need to have a back pointer (or force struct clk to have NOT pointer to udevice, but the udevice itself - then container_of would then do the trick).
The thing I don't understand is that I assumed there is no 1:1 correspondence from struct clk to struct udevice.
For CCF there is 1:1 match - i.e. each struct udevice has a matching struct clk.
I thought that we could have one clock device which supports lots of clk IDs (e.g. 0-23).
On IMX CCF the clock ID is a unique number to refer to a clock.
For example 163 is spi0, 164 is spi1, 165 is spi2, etc. The CCF uses this ID to get proper struct clk.
In case of CCF port in U-Boot we use clk ID to get proper udevice by searching UCLASS_CLK devices linked together. Then the struct clk is read with: struct clk *clk = (struct clk *)dev_get_driver_data(dev); And we do have match when clk->id == id.
Having one device supporting many IDs would be not compatible with CCF where each clock has its own instance of device and clock specific structure.
OK good.
Or is it just convenience?
As stated above - Linux all necessary information has accessible from struct clk.
Sure, but we can always find the udevice from the clk.
Yes, correct.
However clocks (represented as struct udevices) are linked in a single, common uclass (UCLASS_CLK).
OK good.
If we require that clk == udevice then we can go back the other way too, by using uclass-private data attached to each device.
That may work.
The struct udevice's priv would hold clock specific data structure (like struct clk_gate2 - automatically allocated and released).
And the uclass_priv would contain the pointer to struct clk itself (however, in CCF it is always the same as clock specific data structure
- being the first field in the structure).
Yes, or struct clk_uc_priv could contain a *member* which is a pointer to struct clk.
NOTE:
[*] - I do have a hard time to understand how struct clk shall work with struct udevice?
In Linux or Barebox the struct clk is the "main" structure to hold the clock management data (like freq, ops, flags, parent/sibling relation, etc).
Yes U-Boot has a uniform struct udevice for every device and struct uclass for every class.
But the interesting thing here is that clocks have their own parent/sibling relationships, quite independent from the device tree.
But there would be no harm if we could re-use udevice for it. In the current CCF (v4) patch set each clk IP block (like mux or gate) is modelled as udevice:
https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2F
pastebin.com%2FuVmwv5FT&data=02%7C01%7Cpeng.fan%40nxp.com %7C99
5c4869e12e471bdb5108d6de4feda5%7C686ea1d3bc2b4c6fa92cd99c5c3016 35%
7C0%7C0%7C636940832249605030&sdata=XXjDjW9UBtSyuVpNZPeVzY AVCrz
fcnMokz46tqQ1QEo%3D&reserved=0
I don't see how you can do this...doesn't it mean changing the parents of existing devices? E.g. if a SPI clock can come from one of 4 parents, do you need to changes its parent in the driver-model tree?
Yes. The hierarchy is build when the "generic" clock driver is probed (@ clk/imx/clk-imx6q.c) by using the udevice's parent pointer.
If afterwards I need to change clock parent, then I simply change parent pointers in udevices.
Well if you do that, you need to update the sibling lists (device->sibling_node).
A side observation - we now have three different implementations of struct clk in U-Boot :-) (two of which have *ops inside :-) )
Oh dear.
The broadcom iMX ones needs to be converted.
In the case of U-Boot's DM (./include/clk.h) it only has a _pointer_ to udevice (which means that I cannot get the struct clk easily from udevice with container_of()). The struct udevice has instead the *ops and *parent pointer (to another udevice).
Yes that's correct. The struct clk is actually a handle to the clock, and includes an ID number.
You mean the ID number of the clock ?
Yes:
struct clk { struct udevice *dev; /*
- Written by of_xlate. We assume a single id is enough for now. In
the
- future, we might add more fields here.
*/ unsigned long id; };
Two problems:
- Linux CCF code uses massively "struct clk" to handle clock
operations (but not udevice)
OK.
- There is no clear idea of how to implement struct clk
*clk_get_parent(struct clk *) in U-Boot.
As above, it seems that this might need to be implemented. I don't think the DM parent/child relationships are good enough for clk, since they are not aware of the ID.
The reason is that we lack clear information about which udevice's data field shall be used to store the back pointer from udevice to struct clk.
Any hints and ideas are more than welcome.
I think it would be good to get Stephen Warren's thoughts on this as he made the change to introduce struct clk.
Ok.
But at present clk_set_parent() is implemented by calling into the driver. The uclass itself does not maintain information about what is a parent of what.
Linux struct clk has easy access to its parent's struct clk. This is what is missing in U-Boot's DM.
Do we *need* to maintain this information in the uclass?
I think that we don't need. It would be enough to modify struct clk to has struct udevice embedded in it (as Linux has struct clk_core), not the pointer. Then we can use container_of to get clock and re-use struct udevice's parent pointer (and maybe UCLASS_CLK list of devices).
I think it would be prohibitively expensive to separate out each individual clock into a separate device (udevice), but that would work.
This is the approach as I use now in CCF v4: https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2F
pastebin.com%2FuVmwv5FT&data=02%7C01%7Cpeng.fan%40nxp.com %7C99
5c4869e12e471bdb5108d6de4feda5%7C686ea1d3bc2b4c6fa92cd99c5c3016 35%
7C0%7C0%7C636940832249605030&sdata=XXjDjW9UBtSyuVpNZPeVzY AVCrz
fcnMokz46tqQ1QEo%3D&reserved=0
It is expensive but logically correct as each mux, gate, pll is the other CLK IP block (device).
OK I see. What is the cost? Is it acceptable for a boot loader?
To make it really small we would need to use OF_PLATDATA.
For e.g. iMX6Q - I'm able to boot with this CCF port running in both SPL and U-Boot proper.
But, yes the cost is to have the larger binary as we do have larger section with udevices linker list.
Original Linux CCF code uses ~1KiB dynamic table to store pointers to struct clk addressed by ID number. In U-Boot instead - I add those devices to UCLASS_CLK list of devices (as 1KiB is a lot for SPL).
OK.
The only other option I see is to create a sibling list and parent pointer inside struct clk.
This would be the approach similar to Linux kernel approach.
However, I don't know what were original needs of struct clk (as it did not have it). Maybe Stephen can shed some light on it?
Hopefully.
I suspect this will affect power domain also, although we don't have that yet.
iMX8 has some clocks which needs to be always recalculated as they depend on power domains which can be disabled.
OK
Do you think there is a case for building this into DM itself, such that devices can have a list of IDs for each device, each with independent parent/child relationships?
For iMX6Q the ID of the clock is used to get proper clock in drivers (or from DTS). All clock's udevices are stored into UCLASS_CLK
list.
With the ID we get proper udevice and from it and via driver_data the struct clk, which is then used in the CCF code to operate on clock devices (PLL, gate, mux, etc).
I simply re-used the DM facilities (only missing is the back pointer from udevice to struct clk).
Well then you can use dev_get_uclass_priv() for that.
I think that it might be enough to use udevice's priv as clock specific structure (struct clk_gate2) has the struct clock embedded in it.
Why do that? The uclass is specifically designed to hold data that is common to the uclass.
Regards, Simon
On 5/30/19 5:04 AM, Peng Fan wrote:
Hi All
Hi,
Will we force to use CCF in future?
Force how ?
And Is there possibility this feature would land in v2019.07?
No, it's rc3 already, no way.
If not, I'll try to convert i.MX8MM CLK to no-CCF DM CLK version, since i.MX8MM has been pending for long time.
Good
Also please do not top-post
Thanks, Peng.
Subject: Re: [PATCH v3 00/11] clk: Port Linux common clock framework [CCF] to U-boot (tag: 5.0-rc3)
Hi Lukasz,
On Tue, 21 May 2019 at 08:48, Lukasz Majewski lukma@denx.de wrote:
Hi Simon,
Hi Lukasz,
On Sun, 19 May 2019 at 15:03, Lukasz Majewski lukma@denx.de
wrote:
Hi Simon,
Hi Lukasz,
On Sat, 18 May 2019 at 15:28, Lukasz Majewski lukma@denx.de wrote: > > Hi Simon, > > This is not the newest patch set version of CCF (v3 vs. v4), > but the comments/issues apply. > >> Hi Lukasz, >> >> On Thu, 25 Apr 2019 at 04:30, Lukasz Majewski >> lukma@denx.de >> wrote: >>> >>> This patch series brings the files from Linux kernel to >>> provide clocks support as it is used on the Linux kernel >>> with common clock framework [CCF] setup. >>> >>> This series also fixes several problems with current >>> clocks and provides sandbox tests for functions addded to >>> clk-uclass.c file. >>> >>> Design decisions/issues: >>> ========================= >>> >>> - U-boot's DM for clk differs from Linux CCF. The most >>> notably difference is the lack of support for hierarchical >>> clocks and "clock as a manager driver" (single clock DTS >>> node acts as a starting point for all other clocks). >>> >>> - The clk_get_rate() now caches the previously read data >>> (no need for recursive access. >>> >>> - On purpose the "manager" clk driver (clk-imx6q.c) is not >>> using large table to store pointers to clocks - e.g. >>> clk[IMX6QDL_CLK_USDHC2_SEL] = .... Instead we use >>> udevice's linked list for the same class (UCLASS_CLK). The >>> rationale - when porting the code as is from Linux, one >>> would need ~1KiB of RAM to store it. This is way too much >>> if we do plan to use this driver in SPL. >>> >>> - The "central" structure of this patch series is struct >>> udevice and its driver_data field contains the struct clk >>> pointer (to the originally created one). >>> >>> - Up till now U-boot's driver model's CLK operates on >>> udevice (main access to clock is by udevice ops) >>> In the CCF the access to struct clk (comprising pointer >>> to >>> *dev) is possible via dev_get_driver_data() >>> >>> Storing back pointer (from udevice to struct clk) as >>> driver_data is a convention for CCF. >> >> Ick. Why not use uclass-private data to store this, since >> every UCLASS_CLK device can have a parent. > > The "private_data" field would be also a good place to store > the back pointer from udevice to struct clk [*]. The problem > with CCF and udevice's priv pointer is explained just below: > >> >>> >>> - I could use *private_alloc_size to allocate driver's >>> 'private" structures (dev->priv) for e.g. divider (struct >>> clk_divider *divider) for IMX6Q clock, but this would >>> change the original structure of the CCF code. > > The original Linux's CCF code for iMX relies on using kmalloc > internally: > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2 > F%2Felixir.bootlin.com%2Flinux%2Fv5.1.2%2Fsource%2Fdrivers%2Fc >
lk%2Fimx%2Fclk-gate2.c%23L139&data=02%7C01%7Cpeng.fan%40nx
>
p.com%7C995c4869e12e471bdb5108d6de4feda5%7C686ea1d3bc2b4c6fa92
>
cd99c5c301635%7C0%7C0%7C636940832249605030&sdata=NJdvX7JfV
>
g2Qd8H%2FEmnzeNS1v5xhz4AaMRhSUpo7MxI%3D&reserved=0
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2 > F%2Felixir.bootlin.com%2Flinux%2Fv5.1.2%2Fsource%2Fdrivers%2Fc >
lk%2Fclk-divider.c%23L471&data=02%7C01%7Cpeng.fan%40nxp.co
>
m%7C995c4869e12e471bdb5108d6de4feda5%7C686ea1d3bc2b4c6fa92cd99
>
c5c301635%7C0%7C0%7C636940832249605030&sdata=iF66y5IKaQY69
> vyFV%2BY5HSsZiKDb4s%2BN2yMPOBNDOqA%3D&reserved=0 > > By using driver_data I've avoided the need to make more > changes to the original Linux code. > > I could use udevice's priv with automatic data allocation but > then the CCF ported code would require more changes and > considering the (from the outset) need to "fit" this code into > U-Boot's DM, it drives away from the original Linux code.
Is the main change the need to cast driver_data?
The main change would be to remove the per clock device memory allocation code (with exit paths) from the original CCF code.
This shall not be so difficult.
Perhaps that could be hidden in a helper function/macro, so that in U-Boot it can hide the use of (struct clk_uc_priv *)dev_get_uclass_priv(clk->dev))>parent ?
Helper function would help to some extend as we operate on structures similar to:
struct clk_gate2 { struct clk clk; void __iomem *reg; u8 bit_idx; u8 cgr_val; u8 flags; };
The helper would return struct clk's address which is the same as struct's clk_gate2 (this is assured by C standard).
> > >>> >>> The question is if it would be better to use >>> private_alloc_size (and dev->private) or stay with >>> driver_data. The former requires some rewritting in CCF >>> original code (to remove (c)malloc, etc), but comply with >>> u-boot DM. The latter allows re-using the CCF code as is, >>> but introduces some convention special for CCF (I'm not >>> sure thought if dev->priv is NOT another convention as >>> well). >> >> Yes I would like to avoid malloc() calls in drivers and use >> the in-built mechanism. > > I see your point. > > If the community agrees - I can rewrite the code to use such > approach (but issues pointed out in [*] still apply). > >> >>> >>> - I've added the clk_get_parent(), which reads parent's >>> dev->driver_data to provide parent's struct clk pointer. >>> This seems the easiest way to get child/parent >>> relationship for struct clk in U-boot's udevice based clocks. >>> >>> - For tests I had to "emulate" CCF code structure to test >>> functionality of clk_get_parent_rate() and clk_get_by_id(). >>> Those functions will not work properly with "standard" (i.e. >>> non CCF) clock setup(with not set dev->driver_data to >>> struct clk). >> >> Well I think we need a better approach for that anywat. >> driver_data is used for getting something from the DT. > > Maybe the name (driver_data) was a bit misleading then. For > CCF it stores the back pointer to struct clk (as in fact it is > a CCF's "driver data").
Well it seems like a hack to me. Perhaps there is a good reason for it in Linux?
In Linux there is another approach - namely the struct clk (which is the main struct for clock management) has pointer to struct clk_core, which has pointer to parent(s).
https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2F elixir.bootlin.com%2Flinux%2Fv5.1.2%2Fsource%2Fdrivers%2Fclk%2Fclk .c%23L43&data=02%7C01%7Cpeng.fan%40nxp.com%7C995c4869
e12e471bd
b5108d6de4feda5%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C 63694
0832249605030&sdata=63VsTs6JusNw%2FT4mBpfsFLUZKXtyTpZlYx99jy 36
6Mk%3D&reserved=0
In the case of U-Boot - the CCF wants to work on struct clk, but the _main_ data structure for U-Boot is struct udevice. Hence the need to have a back pointer (or force struct clk to have NOT pointer to udevice, but the udevice itself - then container_of would then do the trick).
The thing I don't understand is that I assumed there is no 1:1 correspondence from struct clk to struct udevice.
For CCF there is 1:1 match - i.e. each struct udevice has a matching struct clk.
I thought that we could have one clock device which supports lots of clk IDs (e.g. 0-23).
On IMX CCF the clock ID is a unique number to refer to a clock.
For example 163 is spi0, 164 is spi1, 165 is spi2, etc. The CCF uses this ID to get proper struct clk.
In case of CCF port in U-Boot we use clk ID to get proper udevice by searching UCLASS_CLK devices linked together. Then the struct clk is read with: struct clk *clk = (struct clk *)dev_get_driver_data(dev); And we do have match when clk->id == id.
Having one device supporting many IDs would be not compatible with CCF where each clock has its own instance of device and clock specific structure.
OK good.
Or is it just convenience?
As stated above - Linux all necessary information has accessible from struct clk.
Sure, but we can always find the udevice from the clk.
Yes, correct.
However clocks (represented as struct udevices) are linked in a single, common uclass (UCLASS_CLK).
OK good.
If we require that clk == udevice then we can go back the other way too, by using uclass-private data attached to each device.
That may work.
The struct udevice's priv would hold clock specific data structure (like struct clk_gate2 - automatically allocated and released).
And the uclass_priv would contain the pointer to struct clk itself (however, in CCF it is always the same as clock specific data structure
- being the first field in the structure).
Yes, or struct clk_uc_priv could contain a *member* which is a pointer to struct clk.
> > > > NOTE: > > [*] - I do have a hard time to understand how struct clk shall > work with struct udevice? > > In Linux or Barebox the struct clk is the "main" structure to > hold the clock management data (like freq, ops, flags, > parent/sibling relation, etc).
Yes U-Boot has a uniform struct udevice for every device and struct uclass for every class.
But the interesting thing here is that clocks have their own parent/sibling relationships, quite independent from the device tree.
But there would be no harm if we could re-use udevice for it. In the current CCF (v4) patch set each clk IP block (like mux or gate) is modelled as udevice:
https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2F
pastebin.com%2FuVmwv5FT&data=02%7C01%7Cpeng.fan%40nxp.com %7C99
5c4869e12e471bdb5108d6de4feda5%7C686ea1d3bc2b4c6fa92cd99c5c3016 35%
7C0%7C0%7C636940832249605030&sdata=XXjDjW9UBtSyuVpNZPeVzY AVCrz
fcnMokz46tqQ1QEo%3D&reserved=0
I don't see how you can do this...doesn't it mean changing the parents of existing devices? E.g. if a SPI clock can come from one of 4 parents, do you need to changes its parent in the driver-model tree?
Yes. The hierarchy is build when the "generic" clock driver is probed (@ clk/imx/clk-imx6q.c) by using the udevice's parent pointer.
If afterwards I need to change clock parent, then I simply change parent pointers in udevices.
Well if you do that, you need to update the sibling lists (device->sibling_node).
> > A side observation - we now have three different > implementations of struct clk in U-Boot :-) (two of which have > *ops inside :-) )
Oh dear.
The broadcom iMX ones needs to be converted.
> > In the case of U-Boot's DM (./include/clk.h) it only has a > _pointer_ to udevice (which means that I cannot get the struct > clk easily from udevice with container_of()). The struct > udevice has instead the *ops and *parent pointer (to another > udevice).
Yes that's correct. The struct clk is actually a handle to the clock, and includes an ID number.
You mean the ID number of the clock ?
Yes:
struct clk { struct udevice *dev; /*
- Written by of_xlate. We assume a single id is enough for now. In
the
- future, we might add more fields here.
*/ unsigned long id; };
> > Two problems: > > - Linux CCF code uses massively "struct clk" to handle clock > operations (but not udevice)
OK.
> > - There is no clear idea of how to implement struct clk > *clk_get_parent(struct clk *) in U-Boot.
As above, it seems that this might need to be implemented. I don't think the DM parent/child relationships are good enough for clk, since they are not aware of the ID.
> > The reason is that we lack clear information about which > udevice's data field shall be used to store the back pointer > from udevice to struct clk. > > Any hints and ideas are more than welcome.
I think it would be good to get Stephen Warren's thoughts on this as he made the change to introduce struct clk.
Ok.
But at present clk_set_parent() is implemented by calling into the driver. The uclass itself does not maintain information about what is a parent of what.
Linux struct clk has easy access to its parent's struct clk. This is what is missing in U-Boot's DM.
Do we *need* to maintain this information in the uclass?
I think that we don't need. It would be enough to modify struct clk to has struct udevice embedded in it (as Linux has struct clk_core), not the pointer. Then we can use container_of to get clock and re-use struct udevice's parent pointer (and maybe UCLASS_CLK list of devices).
I think it would be prohibitively expensive to separate out each individual clock into a separate device (udevice), but that would work.
This is the approach as I use now in CCF v4: https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2F
pastebin.com%2FuVmwv5FT&data=02%7C01%7Cpeng.fan%40nxp.com %7C99
5c4869e12e471bdb5108d6de4feda5%7C686ea1d3bc2b4c6fa92cd99c5c3016 35%
7C0%7C0%7C636940832249605030&sdata=XXjDjW9UBtSyuVpNZPeVzY AVCrz
fcnMokz46tqQ1QEo%3D&reserved=0
It is expensive but logically correct as each mux, gate, pll is the other CLK IP block (device).
OK I see. What is the cost? Is it acceptable for a boot loader?
To make it really small we would need to use OF_PLATDATA.
For e.g. iMX6Q - I'm able to boot with this CCF port running in both SPL and U-Boot proper.
But, yes the cost is to have the larger binary as we do have larger section with udevices linker list.
Original Linux CCF code uses ~1KiB dynamic table to store pointers to struct clk addressed by ID number. In U-Boot instead - I add those devices to UCLASS_CLK list of devices (as 1KiB is a lot for SPL).
OK.
The only other option I see is to create a sibling list and parent pointer inside struct clk.
This would be the approach similar to Linux kernel approach.
However, I don't know what were original needs of struct clk (as it did not have it). Maybe Stephen can shed some light on it?
Hopefully.
I suspect this will affect power domain also, although we don't have that yet.
iMX8 has some clocks which needs to be always recalculated as they depend on power domains which can be disabled.
OK
Do you think there is a case for building this into DM itself, such that devices can have a list of IDs for each device, each with independent parent/child relationships?
For iMX6Q the ID of the clock is used to get proper clock in drivers (or from DTS). All clock's udevices are stored into UCLASS_CLK
list.
With the ID we get proper udevice and from it and via driver_data the struct clk, which is then used in the CCF code to operate on clock devices (PLL, gate, mux, etc).
I simply re-used the DM facilities (only missing is the back pointer from udevice to struct clk).
Well then you can use dev_get_uclass_priv() for that.
I think that it might be enough to use udevice's priv as clock specific structure (struct clk_gate2) has the struct clock embedded in it.
Why do that? The uclass is specifically designed to hold data that is common to the uclass.
Regards, Simon
Hi Peng,
Hi All
Will we force to use CCF in future?
I do think yes (as fair as I can tell this is what the community needs/agreed).
And Is there possibility this feature would land in v2019.07?
Do you mean this release? We are -rc3 now so it is IMHO too late. (The v2019.10-rc1 seems reasonable).
The status is that Simon, had some comments for it and I need to address them (however, recently I got less time to do that).
If not, I'll try to convert i.MX8MM CLK to no-CCF DM CLK version, since i.MX8MM has been pending for long time.
The CCF was under discussion for some time for a reason - I would like to avoid adding ad-hoc code and make it DM compliant (thanks Simon for your feedback) and re-usable as much as possible.
(I will try to do some work on it during this week).
Thanks, Peng.
Subject: Re: [PATCH v3 00/11] clk: Port Linux common clock framework [CCF] to U-boot (tag: 5.0-rc3)
Hi Lukasz,
On Tue, 21 May 2019 at 08:48, Lukasz Majewski lukma@denx.de wrote:
Hi Simon,
Hi Lukasz,
On Sun, 19 May 2019 at 15:03, Lukasz Majewski lukma@denx.de
wrote:
Hi Simon,
Hi Lukasz,
On Sat, 18 May 2019 at 15:28, Lukasz Majewski lukma@denx.de wrote: > > Hi Simon, > > This is not the newest patch set version of CCF (v3 vs. > v4), but the comments/issues apply. > > > Hi Lukasz, > > > > On Thu, 25 Apr 2019 at 04:30, Lukasz Majewski > > lukma@denx.de > > wrote: > > > > > > This patch series brings the files from Linux kernel > > > to provide clocks support as it is used on the Linux > > > kernel with common clock framework [CCF] setup. > > > > > > This series also fixes several problems with current > > > clocks and provides sandbox tests for functions > > > addded to clk-uclass.c file. > > > > > > Design decisions/issues: > > > ========================= > > > > > > - U-boot's DM for clk differs from Linux CCF. The most > > > notably difference is the lack of support for > > > hierarchical clocks and "clock as a manager > > > driver" (single clock DTS node acts as a starting > > > point for all other clocks). > > > > > > - The clk_get_rate() now caches the previously read > > > data (no need for recursive access. > > > > > > - On purpose the "manager" clk driver (clk-imx6q.c) > > > is not using large table to store pointers to clocks > > > - e.g. clk[IMX6QDL_CLK_USDHC2_SEL] = .... Instead we > > > use udevice's linked list for the same class > > > (UCLASS_CLK). The rationale - when porting the code > > > as is from Linux, one would need ~1KiB of RAM to > > > store it. This is way too much if we do plan to use > > > this driver in SPL. > > > > > > - The "central" structure of this patch series is > > > struct udevice and its driver_data field contains the > > > struct clk pointer (to the originally created one). > > > > > > - Up till now U-boot's driver model's CLK operates on > > > udevice (main access to clock is by udevice ops) > > > In the CCF the access to struct clk (comprising > > > pointer to > > > *dev) is possible via dev_get_driver_data() > > > > > > Storing back pointer (from udevice to struct clk) as > > > driver_data is a convention for CCF. > > > > Ick. Why not use uclass-private data to store this, > > since every UCLASS_CLK device can have a parent. > > The "private_data" field would be also a good place to > store the back pointer from udevice to struct clk [*]. > The problem with CCF and udevice's priv pointer is > explained just below: > > > > > > > > - I could use *private_alloc_size to allocate driver's > > > 'private" structures (dev->priv) for e.g. divider > > > (struct clk_divider *divider) for IMX6Q clock, but > > > this would change the original structure of the CCF > > > code. > > The original Linux's CCF code for iMX relies on using > kmalloc internally: > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2 > F%2Felixir.bootlin.com%2Flinux%2Fv5.1.2%2Fsource%2Fdrivers%2Fc >
lk%2Fimx%2Fclk-gate2.c%23L139&data=02%7C01%7Cpeng.fan%40nx
>
p.com%7C995c4869e12e471bdb5108d6de4feda5%7C686ea1d3bc2b4c6fa92
>
cd99c5c301635%7C0%7C0%7C636940832249605030&sdata=NJdvX7JfV
>
g2Qd8H%2FEmnzeNS1v5xhz4AaMRhSUpo7MxI%3D&reserved=0
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2 > F%2Felixir.bootlin.com%2Flinux%2Fv5.1.2%2Fsource%2Fdrivers%2Fc >
lk%2Fclk-divider.c%23L471&data=02%7C01%7Cpeng.fan%40nxp.co
>
m%7C995c4869e12e471bdb5108d6de4feda5%7C686ea1d3bc2b4c6fa92cd99
>
c5c301635%7C0%7C0%7C636940832249605030&sdata=iF66y5IKaQY69
> vyFV%2BY5HSsZiKDb4s%2BN2yMPOBNDOqA%3D&reserved=0 > > By using driver_data I've avoided the need to make more > changes to the original Linux code. > > I could use udevice's priv with automatic data allocation > but then the CCF ported code would require more changes > and considering the (from the outset) need to "fit" this > code into U-Boot's DM, it drives away from the original > Linux code.
Is the main change the need to cast driver_data?
The main change would be to remove the per clock device memory allocation code (with exit paths) from the original CCF code.
This shall not be so difficult.
Perhaps that could be hidden in a helper function/macro, so that in U-Boot it can hide the use of (struct clk_uc_priv *)dev_get_uclass_priv(clk->dev))>parent ?
Helper function would help to some extend as we operate on structures similar to:
struct clk_gate2 { struct clk clk; void __iomem *reg; u8 bit_idx; u8 cgr_val; u8 flags; };
The helper would return struct clk's address which is the same as struct's clk_gate2 (this is assured by C standard).
> > > > > > > > The question is if it would be better to use > > > private_alloc_size (and dev->private) or stay with > > > driver_data. The former requires some rewritting in > > > CCF original code (to remove (c)malloc, etc), but > > > comply with u-boot DM. The latter allows re-using the > > > CCF code as is, but introduces some convention > > > special for CCF (I'm not sure thought if dev->priv is > > > NOT another convention as well). > > > > Yes I would like to avoid malloc() calls in drivers and > > use the in-built mechanism. > > I see your point. > > If the community agrees - I can rewrite the code to use > such approach (but issues pointed out in [*] still apply). > > > > > > > > > - I've added the clk_get_parent(), which reads > > > parent's dev->driver_data to provide parent's struct > > > clk pointer. This seems the easiest way to get > > > child/parent relationship for struct clk in U-boot's > > > udevice based clocks. > > > > > > - For tests I had to "emulate" CCF code structure to > > > test functionality of clk_get_parent_rate() and > > > clk_get_by_id(). Those functions will not work > > > properly with "standard" (i.e. non CCF) clock > > > setup(with not set dev->driver_data to struct clk). > > > > Well I think we need a better approach for that anywat. > > driver_data is used for getting something from the DT. > > Maybe the name (driver_data) was a bit misleading then. > For CCF it stores the back pointer to struct clk (as in > fact it is a CCF's "driver data").
Well it seems like a hack to me. Perhaps there is a good reason for it in Linux?
In Linux there is another approach - namely the struct clk (which is the main struct for clock management) has pointer to struct clk_core, which has pointer to parent(s).
https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2F elixir.bootlin.com%2Flinux%2Fv5.1.2%2Fsource%2Fdrivers%2Fclk%2Fclk .c%23L43&data=02%7C01%7Cpeng.fan%40nxp.com%7C995c4869
e12e471bd
b5108d6de4feda5%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C 63694
0832249605030&sdata=63VsTs6JusNw%2FT4mBpfsFLUZKXtyTpZlYx99jy 36
6Mk%3D&reserved=0
In the case of U-Boot - the CCF wants to work on struct clk, but the _main_ data structure for U-Boot is struct udevice. Hence the need to have a back pointer (or force struct clk to have NOT pointer to udevice, but the udevice itself - then container_of would then do the trick).
The thing I don't understand is that I assumed there is no 1:1 correspondence from struct clk to struct udevice.
For CCF there is 1:1 match - i.e. each struct udevice has a matching struct clk.
I thought that we could have one clock device which supports lots of clk IDs (e.g. 0-23).
On IMX CCF the clock ID is a unique number to refer to a clock.
For example 163 is spi0, 164 is spi1, 165 is spi2, etc. The CCF uses this ID to get proper struct clk.
In case of CCF port in U-Boot we use clk ID to get proper udevice by searching UCLASS_CLK devices linked together. Then the struct clk is read with: struct clk *clk = (struct clk *)dev_get_driver_data(dev); And we do have match when clk->id == id.
Having one device supporting many IDs would be not compatible with CCF where each clock has its own instance of device and clock specific structure.
OK good.
Or is it just convenience?
As stated above - Linux all necessary information has accessible from struct clk.
Sure, but we can always find the udevice from the clk.
Yes, correct.
However clocks (represented as struct udevices) are linked in a single, common uclass (UCLASS_CLK).
OK good.
If we require that clk == udevice then we can go back the other way too, by using uclass-private data attached to each device.
That may work.
The struct udevice's priv would hold clock specific data structure (like struct clk_gate2 - automatically allocated and released).
And the uclass_priv would contain the pointer to struct clk itself (however, in CCF it is always the same as clock specific data structure
- being the first field in the structure).
Yes, or struct clk_uc_priv could contain a *member* which is a pointer to struct clk.
> > > > NOTE: > > [*] - I do have a hard time to understand how struct clk > shall work with struct udevice? > > In Linux or Barebox the struct clk is the "main" > structure to hold the clock management data (like freq, > ops, flags, parent/sibling relation, etc).
Yes U-Boot has a uniform struct udevice for every device and struct uclass for every class.
But the interesting thing here is that clocks have their own parent/sibling relationships, quite independent from the device tree.
But there would be no harm if we could re-use udevice for it. In the current CCF (v4) patch set each clk IP block (like mux or gate) is modelled as udevice:
https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2F
pastebin.com%2FuVmwv5FT&data=02%7C01%7Cpeng.fan%40nxp.com %7C99
5c4869e12e471bdb5108d6de4feda5%7C686ea1d3bc2b4c6fa92cd99c5c3016 35%
7C0%7C0%7C636940832249605030&sdata=XXjDjW9UBtSyuVpNZPeVzY AVCrz
fcnMokz46tqQ1QEo%3D&reserved=0
I don't see how you can do this...doesn't it mean changing the parents of existing devices? E.g. if a SPI clock can come from one of 4 parents, do you need to changes its parent in the driver-model tree?
Yes. The hierarchy is build when the "generic" clock driver is probed (@ clk/imx/clk-imx6q.c) by using the udevice's parent pointer.
If afterwards I need to change clock parent, then I simply change parent pointers in udevices.
Well if you do that, you need to update the sibling lists (device->sibling_node).
> > A side observation - we now have three different > implementations of struct clk in U-Boot :-) (two of which > have *ops inside :-) )
Oh dear.
The broadcom iMX ones needs to be converted.
> > In the case of U-Boot's DM (./include/clk.h) it only has a > _pointer_ to udevice (which means that I cannot get the > struct clk easily from udevice with container_of()). The > struct udevice has instead the *ops and *parent pointer > (to another udevice).
Yes that's correct. The struct clk is actually a handle to the clock, and includes an ID number.
You mean the ID number of the clock ?
Yes:
struct clk { struct udevice *dev; /*
- Written by of_xlate. We assume a single id is enough for now.
In the
- future, we might add more fields here.
*/ unsigned long id; };
> > Two problems: > > - Linux CCF code uses massively "struct clk" to handle > clock operations (but not udevice)
OK.
> > - There is no clear idea of how to implement struct clk > *clk_get_parent(struct clk *) in U-Boot.
As above, it seems that this might need to be implemented. I don't think the DM parent/child relationships are good enough for clk, since they are not aware of the ID.
> > The reason is that we lack clear information about which > udevice's data field shall be used to store the back > pointer from udevice to struct clk. > > Any hints and ideas are more than welcome.
I think it would be good to get Stephen Warren's thoughts on this as he made the change to introduce struct clk.
Ok.
But at present clk_set_parent() is implemented by calling into the driver. The uclass itself does not maintain information about what is a parent of what.
Linux struct clk has easy access to its parent's struct clk. This is what is missing in U-Boot's DM.
Do we *need* to maintain this information in the uclass?
I think that we don't need. It would be enough to modify struct clk to has struct udevice embedded in it (as Linux has struct clk_core), not the pointer. Then we can use container_of to get clock and re-use struct udevice's parent pointer (and maybe UCLASS_CLK list of devices).
I think it would be prohibitively expensive to separate out each individual clock into a separate device (udevice), but that would work.
This is the approach as I use now in CCF v4: https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2F
pastebin.com%2FuVmwv5FT&data=02%7C01%7Cpeng.fan%40nxp.com %7C99
5c4869e12e471bdb5108d6de4feda5%7C686ea1d3bc2b4c6fa92cd99c5c3016 35%
7C0%7C0%7C636940832249605030&sdata=XXjDjW9UBtSyuVpNZPeVzY AVCrz
fcnMokz46tqQ1QEo%3D&reserved=0
It is expensive but logically correct as each mux, gate, pll is the other CLK IP block (device).
OK I see. What is the cost? Is it acceptable for a boot loader?
To make it really small we would need to use OF_PLATDATA.
For e.g. iMX6Q - I'm able to boot with this CCF port running in both SPL and U-Boot proper.
But, yes the cost is to have the larger binary as we do have larger section with udevices linker list.
Original Linux CCF code uses ~1KiB dynamic table to store pointers to struct clk addressed by ID number. In U-Boot instead
- I add those devices to UCLASS_CLK list of devices (as 1KiB is a
lot for SPL).
OK.
The only other option I see is to create a sibling list and parent pointer inside struct clk.
This would be the approach similar to Linux kernel approach.
However, I don't know what were original needs of struct clk (as it did not have it). Maybe Stephen can shed some light on it?
Hopefully.
I suspect this will affect power domain also, although we don't have that yet.
iMX8 has some clocks which needs to be always recalculated as they depend on power domains which can be disabled.
OK
Do you think there is a case for building this into DM itself, such that devices can have a list of IDs for each device, each with independent parent/child relationships?
For iMX6Q the ID of the clock is used to get proper clock in drivers (or from DTS). All clock's udevices are stored into UCLASS_CLK
list.
With the ID we get proper udevice and from it and via driver_data the struct clk, which is then used in the CCF code to operate on clock devices (PLL, gate, mux, etc).
I simply re-used the DM facilities (only missing is the back pointer from udevice to struct clk).
Well then you can use dev_get_uclass_priv() for that.
I think that it might be enough to use udevice's priv as clock specific structure (struct clk_gate2) has the struct clock embedded in it.
Why do that? The uclass is specifically designed to hold data that is common to the uclass.
Regards, Simon
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
Hi Lukasz,
On Mon, 3 Jun 2019 at 08:22, Lukasz Majewski lukma@denx.de wrote:
Hi Peng,
Hi All
Will we force to use CCF in future?
I do think yes (as fair as I can tell this is what the community needs/agreed).
And Is there possibility this feature would land in v2019.07?
Do you mean this release? We are -rc3 now so it is IMHO too late. (The v2019.10-rc1 seems reasonable).
The status is that Simon, had some comments for it and I need to address them (however, recently I got less time to do that).
If not, I'll try to convert i.MX8MM CLK to no-CCF DM CLK version, since i.MX8MM has been pending for long time.
The CCF was under discussion for some time for a reason - I would like to avoid adding ad-hoc code and make it DM compliant (thanks Simon for your feedback) and re-usable as much as possible.
(I will try to do some work on it during this week).
Just checking on the state of this since I have been a bit out of touch for a while.
Regards, Simon
participants (4)
-
Lukasz Majewski -
Marek Vasut -
Peng Fan -
Simon Glass