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