| Message ID | 20190425102953.5348-8-lukma@denx.de |
|---|---|
| State | Changes Requested |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | clk: Port Linux common clock framework [CCF] to U-boot (tag: 5.0-rc3) | expand |
> 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
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
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(+)