
Hi Simon,
+Stephen
Hi Lukasz,
On Thu, 31 Jan 2019 at 02:04, Lukasz Majewski lukma@denx.de wrote:
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.
As written in the in-code comment - some clocks do not set clk->id (and require it to be set to 0) and hence the standard ckl_{request|get_rate| free} API is used.
Signed-off-by: Lukasz Majewski lukma@denx.de
Changes in v2: None
drivers/clk/clk-uclass.c | 41 +++++++++++++++++++++++++++++++++++++++++ include/clk.h | 9 +++++++++ 2 files changed, 50 insertions(+)
Can we please call this from test/dm/clk.c?
Ok.
diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c index 6d7a514006..f1640dda67 100644 --- a/drivers/clk/clk-uclass.c +++ b/drivers/clk/clk-uclass.c @@ -340,6 +340,47 @@ ulong clk_get_rate(struct clk *clk) return ops->get_rate(clk); }
+ulong clk_get_parent_rate(struct clk *clk) +{
const struct clk_ops *ops;
struct udevice *pdev;
struct clk pclk;
ulong rate;
int ret;
debug("%s(clk=%p)\n", __func__, clk);
pdev = clk->dev->parent;
dev_get_parent(clk->dev)
Yes, correct.
if (!pdev)
return -ENODEV;
Not needed, all devices have parents except the root, which is not in UCLASS_CLK
Ok.
ops = clk_dev_ops(pdev);
if (!ops->get_rate)
return -ENOSYS;
/*
* We do use memset, clk_{request|get_rate|free}
* as there are clocks - like the "fixed" ones, which
* doesn't posses the clk wrapper struct (just added to
* UCLASS_CLK) and explicitly check if clk->id = 0.
*
* In fact the "clock" resources (like ops, description)
* are accessed via udevice structure (pdev - parent's one)
*/
drop blank line. Also is that comment wrapped to use the full number of columns?
I will refactor the comment.
Also, this seems like a bug that should be fixed?
Yes, this seems strange to me:
The main structure passed in the clock API in U-boot is struct clk *clk pointer.
However, the fixed clocks (clk_fixed_rate.c) require the clk->id to be set to 0. This is strange as imposes in practice passing new, memset'ed to 0 clk structure pointer. Moreover, they doesn't have corresponding struct clk definition and exists only linked in UCLASS_CLK.
The pattern to get clock rate:
memset(clk, 0, sizeof(clk); clk_request(pdev, clk) --> clk->dev = pdev [*]; (this is _the_ defacto clock assignment) clk_get_rate(clk) clk_free(clk)
is also used in socfpga [1] and 'clk dump' command and IMHO is wrong (but I cannot understand why it was done in that way - we _shall_ pass pointer to struct clk, not rely on udevices - like in [*]).
Such approach causes the passed clk pointer to be useless as it is NULL when we call clk_get_rate() recursively. And for this reason the struct clk *clk pointer is stored in driver_data for udevice.
[1] - arch/arm/mach-socfpga/clock_manager_arria10.c
memset(&pclk, 0, sizeof(pclk));
ret = clk_request(pdev, &pclk);
if (ret) {
printf("%s: pclk: %s request failed!\n", __func__,
pdev->name);
return ret;
}
rate = clk_get_rate(&pclk);
clk_free(&pclk);
return 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 f6fbcc6634..8224295ec3 100644 --- a/include/clk.h +++ b/include/clk.h @@ -238,6 +238,15 @@ int clk_free(struct clk *clk); ulong clk_get_rate(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
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