Hi Sakari Ailus,
Thanks for the feedback.
> -----Original Message-----
> From: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> Sent: Monday, March 18, 2024 6:29 PM
> Subject: Re: [PATCH v3 1/3] clk: Update API documentation related to clock disable
>
> 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@xxxxxxxxxxxxxxxx>
> > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > ---
> > 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.
Driver abstract hardware. But a driver supports different hardware's.
One hardware may doesn't care about the sync/async disabling of the clock
while another one does.
Not sure how other people's thoughts on this change??
>
> > *
> > * @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.
If we elaborate " driver dependent", it matches with your explanation.
Not sure we should elaborate it or it is understood from "driver " dependent??
Cheers,
Biju
>
> > *
> > * May be called from atomic contexts.
> > *
>
> --
> Kind regards,
>
> Sakari Ailus
