[U-Boot] [PATCH 1/3] clk: introduce enable_cnt

[Lukasz Majewski]

Hi Peng,

> Introduce enable_cnt to track the clk enable/disable count 

As fair as I remember there was no reference counters for Linux original
CCF (to be precise - they are in devices, but not explicitly used in
CCF).

As the commit message is very short I would like to explicitly ask
what problem do you try to solve here (and if the same problem is
available on Linux)? [*]

The whole parch series misses a few things:

- Detailed explanation and rationale for this code [*]

- If you introduce new variable/functionality in clk-uclass - there
  shall be a sandbox test code for it. This is missing in this series.


Note:

[*] - please write detailed commit messages / readme for your patches
(this is the note for the whole NXP's u-boot developers' team).

Detailed commit message provides:

- Better understanding of the issue for reviewers from the outset

- It is a very good documentation which stays with the code (in git)

- After e.g. 2 years it is still possible to get the grasp of the
  problem (when bisecting/ fixing bugs) quickly.

> when
> clk_enable/disable for CCF.
> And Initialize enable_cnt to 0 when register the clk.
> 
> Signed-off-by: Peng Fan <peng.fan at nxp.com>
> ---
>  drivers/clk/clk.c            | 1 +
>  drivers/clk/clk_fixed_rate.c | 1 +
>  include/clk.h                | 1 +
>  3 files changed, 3 insertions(+)
> 
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 39b3087067..7effd410ee 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -40,6 +40,7 @@ int clk_register(struct clk *clk, const char
> *drv_name, return ret;
>  	}
>  
> +	clk->enable_cnt = 0;
>  	/* Store back pointer to clk from udevice */
>  	clk->dev->uclass_priv = clk;
>  
> diff --git a/drivers/clk/clk_fixed_rate.c
> b/drivers/clk/clk_fixed_rate.c index 08cce0d79b..6b89fad46b 100644
> --- a/drivers/clk/clk_fixed_rate.c
> +++ b/drivers/clk/clk_fixed_rate.c
> @@ -27,6 +27,7 @@ static int clk_fixed_rate_ofdata_to_platdata(struct
> udevice *dev) /* Make fixed rate clock accessible from higher level
> struct clk */ dev->uclass_priv = clk;
>  	clk->dev = dev;
> +	clk->enable_cnt = 0;
>  
>  	return 0;
>  }
> diff --git a/include/clk.h b/include/clk.h
> index 2ebc905e04..58e456898f 100644
> --- a/include/clk.h
> +++ b/include/clk.h
> @@ -61,6 +61,7 @@ struct clk {
>  	struct udevice *dev;
>  	long long rate;	/* in HZ */
>  	u32 flags;
> +	int enable_cnt;
>  	/*
>  	 * Written by of_xlate. In the future, we might add more
> fields here. */



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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190816/82716981/attachment.sig>