Hi Simon, > +Stephen > > Hi Lukasz, > > On Thu, 31 Jan 2019 at 02:04, Lukasz Majewski <lu...@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 <lu...@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: lu...@denx.de
pgpToC_Qb4rOR.pgp
Description: OpenPGP digital signature
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot