Re: [PATCH v3 1/3] clk: Update API documentation related to clock disable

[Sakari Ailus <sakari.ailus@linux.intel.com>]

Hi Biju,

On Mon, Mar 18, 2024 at 11:08:40AM +0000, Biju Das wrote:
> 
> The API's related to clk disable operation does not explicitly
> states the synchoronous or asynchrous behaviour as it is driver
> dependent. So make this part clear in API documentation.
> 
> Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
> v2->v3:
>  * No change.
> v2:
>  * New patch.
> ---
>  drivers/clk/clk.c            | 3 ++-
>  include/linux/clk-provider.h | 3 ++-
>  include/linux/clk.h          | 3 ++-
>  3 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 25371c91a58f..f5fa91a339d7 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -1010,7 +1010,8 @@ static void clk_core_unprepare_lock(struct clk_core *core)
>   * if the operation may sleep.  One example is a clk which is accessed over
>   * I2c.  In the complex case a clk gate operation may require a fast and a slow
>   * part.  It is this reason that clk_unprepare and clk_disable are not mutually
> - * exclusive.  In fact clk_disable must be called before clk_unprepare.
> + * exclusive.  In fact clk_disable must be called before clk_unprepare.  The
> + * synchronous or asynchronous clock gating operation is driver dependent.
>   */
>  void clk_unprepare(struct clk *clk)
>  {
> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
> index 4a537260f655..5b493024e1ec 100644
> --- a/include/linux/clk-provider.h
> +++ b/include/linux/clk-provider.h
> @@ -113,7 +113,8 @@ struct clk_duty {
>   *		sleep.
>   *
>   * @disable:	Disable the clock atomically. Called with enable_lock held.
> - *		This function must not sleep.
> + *		This function must not sleep. The synchronous or asynchronous
> + *		disabling of the clock is driver dependent.

s/driver\K/ and hardware/

Same in the first chunk actually.

>   *
>   * @is_enabled:	Queries the hardware to determine if the clock is enabled.
>   *		This function must not sleep. Optional, if this op is not
> diff --git a/include/linux/clk.h b/include/linux/clk.h
> index 00623f4de5e1..84b02518791f 100644
> --- a/include/linux/clk.h
> +++ b/include/linux/clk.h
> @@ -681,7 +681,8 @@ int __must_check clk_bulk_enable(int num_clks,
>   * @clk: clock source
>   *
>   * Inform the system that a clock source is no longer required by
> - * a driver and may be shut down.
> + * a driver and may be shut down. It is not guaranteed to ever actually
> + * be stopped, that will be driver dependent.

I'd rephrase this, taking other users into account:

	There's no guarantee that the clock stops within a particular time
	window or at all, depending on other users of the clock as well as
	the driver and hardware implementation.

>   *
>   * May be called from atomic contexts.
>   *

-- 
Kind regards,

Sakari Ailus