[U-Boot] [i.MX8MM+CCF 03/41] clk: introduce clk_dev_binded

Peng Fan
2019-05-08T07:41:18Z

> -----Original Message-----
> From: Lukasz Majewski [mailto:lukma at denx.de]
> Sent: 2019年5月8日 15:31
> To: Peng Fan <peng.fan@nxp.com>
> Cc: sbabic at denx.de; festevam at gmail.com; dl-uboot-imx
> <uboot-imx@nxp.com>; sjg at chromium.org; jagan at amarulasolutions.com;
> sr at denx.de; u-boot at lists.denx.de; trini at 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 at 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 at denx.de