-----Original Message----- From: Lukasz Majewski [mailto:lukma@denx.de] Sent: 2019年5月8日 15:31 To: Peng Fan peng.fan@nxp.com Cc: sbabic@denx.de; festevam@gmail.com; dl-uboot-imx uboot-imx@nxp.com; sjg@chromium.org; jagan@amarulasolutions.com; sr@denx.de; u-boot@lists.denx.de; trini@konsulko.com Subject: Re: [i.MX8MM+CCF 03/41] clk: introduce clk_dev_binded
On Tue, 7 May 2019 13:22:24 +0000 Peng Fan peng.fan@nxp.com wrote:
Subject: Re: [i.MX8MM+CCF 03/41] clk: introduce clk_dev_binded
On Tue, 30 Apr 2019 10:17:40 +0000 Peng Fan peng.fan@nxp.com wrote:
When support Clock Common Framework, U-Boot use dev for clk tree information, there is no clk->parent.
There is a function in clk uclass named: clk_get_parent() to provide parent of the clock.
When support composite clk, it contains mux/gate/divider, but the mux/gate/divider is not binded with device.
There is a binding: struct clk_pllv3 { struct clk clk; ... };
The clk.dev points to corresponding device.
In the opposite direction we do have dev->driver_data, which points to struct clk embedded in for example struct clk_pllv3. (as struct clk_pllv3 and struct clk share the same address it is up to us to cast it properly).
I've written my thoughts and considerations about using dev->private and dev->driver_data in the patch cover letter [1]
So we could not use dev_get_driver_data to get the correct clk_mux/gate/divider.
Maybe I've overlooked something, but dev_get_driver_data() shall provide correct reference to udevice.
A composite clk contains a mux/gate/divider clk. Only the composite clk needs to binded with a udevice.
So, if I understood correctly we do need to have another reference to udevice (or clock)?
The composite clk structure struct clk_composite { struct clk clk; struct clk_ops ops;
struct clk *mux; struct clk *rate; struct clk *gate;
const struct clk_ops *mux_ops; const struct clk_ops *rate_ops; const struct clk_ops *gate_ops; };
Only the first clk needs to be binded with a udevice. The mux/rate/gate pointer is only used for composite clk internal usage and no udevice will be binded with them.
Please correct my understanding:
- Composite clock would use clk->dev (to have pointer to struct udevice) and dev->driver_data to have pointer to clk.
Yes, for the composite clk.
- Then we use udevice flag DM_FLAG_BOUND to indicate if this device has other udevices (i.e. clk) bound.
Yes, DM_FLAG_BOUND means the clk has a udevice binded.
- If it has bounded device then we use dev->driver_data to get it's struct clk (or clock IP - like mux). If not, then we just use this clk.
Yes.
Considering the above - the commit message needs detailed explanation of the "binding" concept.
The mux/gate/divider inside a composite clk should not bind a device, because they needs to be hidden from dm tree or clk dumps.
If bind the mux/gate/divider insides a composite clk, that will be mess.
But the composite clock itself would be printed in 'dm tree' ?
Yes.
If yes, maybe we can add some kind of indication that it is a "container" and that it has some clocks "binded" ?
Looking at clk summary from Linux /sys/kernel/debug/clk/clk_summary, There is no flag shows that it is a composite clk or not.
I think only add a flag to show the clk is a container does not make much sense.
Thanks, Peng.
The reason to introduce composite clk is to make clk tree cleaner/simplier.
Ok. No problem with this.
Regards, Peng.
So add clk_dev_binded to let choose the correct method.
[1] - http://patchwork.ozlabs.org/cover/1090669/
Signed-off-by: Peng Fan peng.fan@nxp.com
drivers/clk/clk.c | 8 ++++++++ include/clk.h | 9 +++++++++ 2 files changed, 17 insertions(+)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index 0a0fffb50b..025bb99ecc 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -54,3 +54,11 @@ const char *clk_hw_get_name(const struct clk *hw) { return hw->dev->name; }
+bool clk_dev_binded(struct clk *clk) {
- if (clk->dev && (clk->dev->flags & DM_FLAG_BOUND))
return true;- return false;
+} diff --git a/include/clk.h b/include/clk.h index a4ecca9fbc..8199119d01 100644 --- a/include/clk.h +++ b/include/clk.h @@ -337,4 +337,13 @@ static inline bool clk_valid(struct clk *clk)
- @return zero on success, or -ENOENT on error
*/ int clk_get_by_id(ulong id, struct clk **clkp);
+/**
- clk_dev_binded() - Check whether the clk has a device binded
- @clk A pointer to the clk
- @return true on binded, or false on no */ bool
+clk_dev_binded(struct clk *clk); #endif
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
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