Re: [PATCH v7 3/5] clk: Supply the critical clock {init, enable, disable} framework

Re: [PATCH v7 3/5] clk: Supply the critical clock {init, enable, disable} framework

[Michael Turquette]

Hi Lee,

+ linux-clk ml

Quoting Lee Jones (2015-07-22 06:04:13)
> These new API calls will firstly provide a mechanisms to tag a clock as
> critical and secondly allow any knowledgeable driver to (un)gate clocks,
> even if they are marked as critical.
> 
> Suggested-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>  drivers/clk/clk.c            | 45 ++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/clk-provider.h |  2 ++
>  include/linux/clk.h          | 30 +++++++++++++++++++++++++++++
>  3 files changed, 77 insertions(+)
> 
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 61c3fc5..486b1da 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -46,6 +46,21 @@ static struct clk_core *clk_core_lookup(const char *name);
>  
>  /***    private data structures    ***/
>  
> +/**
> + * struct critical -   Provides 'play' over critical clocks.  A clock can be
> + *                     marked as critical, meaning that it should not be
> + *                     disabled.  However, if a driver which is aware of the
> + *                     critical behaviour wants to control it, it can do so
> + *                     using clk_enable_critical() and clk_disable_critical().
> + *
> + * @enabled    Is clock critical?  Once set, doesn't change
> + * @leave_on   Self explanatory.  Can be disabled by knowledgeable drivers

Not self explanatory. I need this explained to me. What does leave_on
do? Better yet, what would happen if leave_on did not exist?

> + */
> +struct critical {
> +       bool enabled;
> +       bool leave_on;
> +};
> +
>  struct clk_core {
>         const char              *name;
>         const struct clk_ops    *ops;
> @@ -75,6 +90,7 @@ struct clk_core {
>         struct dentry           *dentry;
>  #endif
>         struct kref             ref;
> +       struct critical         critical;
>  };
>  
>  struct clk {
> @@ -995,6 +1011,10 @@ static void clk_core_disable(struct clk_core *clk)
>         if (WARN_ON(clk->enable_count == 0))
>                 return;
>  
> +       /* Refuse to turn off a critical clock */
> +       if (clk->enable_count == 1 && clk->critical.leave_on)
> +               return;

How do we get to this point? clk_enable_critical actually calls
clk_enable, thus incrementing the enable_count. The only time that we
could hit the above case is if,

a) there is an imbalance in clk_enable and clk_disable calls. If this is
the case then the drivers need to be fixed. Or better yet some
infrastructure to catch that, now that we have per-user struct clk
cookies.

b) a driver knowingly calls clk_enable_critical(foo) and then regular,
old clk_disable(foo). But why would a driver do that?

It might be that I am missing the point here, so please feel free to
clue me in.

Regards,
Mike

> +
>         if (--clk->enable_count > 0)
>                 return;
>  
> @@ -1037,6 +1057,13 @@ void clk_disable(struct clk *clk)
>  }
>  EXPORT_SYMBOL_GPL(clk_disable);
>  
> +void clk_disable_critical(struct clk *clk)
> +{
> +       clk->core->critical.leave_on = false;
> +       clk_disable(clk);
> +}
> +EXPORT_SYMBOL_GPL(clk_disable_critical);
> +
>  static int clk_core_enable(struct clk_core *clk)
>  {
>         int ret = 0;
> @@ -1100,6 +1127,15 @@ int clk_enable(struct clk *clk)
>  }
>  EXPORT_SYMBOL_GPL(clk_enable);
>  
> +int clk_enable_critical(struct clk *clk)
> +{
> +       if (clk->core->critical.enabled)
> +               clk->core->critical.leave_on = true;
> +
> +       return clk_enable(clk);
> +}
> +EXPORT_SYMBOL_GPL(clk_enable_critical);
> +
>  static unsigned long clk_core_round_rate_nolock(struct clk_core *clk,
>                                                 unsigned long rate,
>                                                 unsigned long min_rate,
> @@ -2482,6 +2518,15 @@ fail_out:
>  }
>  EXPORT_SYMBOL_GPL(clk_register);
>  
> +void clk_init_critical(struct clk *clk)
> +{
> +       struct critical *critical = &clk->core->critical;
> +
> +       critical->enabled = true;
> +       critical->leave_on = true;
> +}
> +EXPORT_SYMBOL_GPL(clk_init_critical);
> +
>  /*
>   * Free memory allocated for a clock.
>   * Caller must hold prepare_lock.
> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
> index 5591ea7..15ef8c9 100644
> --- a/include/linux/clk-provider.h
> +++ b/include/linux/clk-provider.h
> @@ -563,6 +563,8 @@ struct clk *devm_clk_register(struct device *dev, struct clk_hw *hw);
>  void clk_unregister(struct clk *clk);
>  void devm_clk_unregister(struct device *dev, struct clk *clk);
>  
> +void clk_init_critical(struct clk *clk);
> +
>  /* helper functions */
>  const char *__clk_get_name(struct clk *clk);
>  struct clk_hw *__clk_get_hw(struct clk *clk);
> diff --git a/include/linux/clk.h b/include/linux/clk.h
> index 8381bbf..9807f3b 100644
> --- a/include/linux/clk.h
> +++ b/include/linux/clk.h
> @@ -231,6 +231,19 @@ struct clk *devm_clk_get(struct device *dev, const char *id);
>  int clk_enable(struct clk *clk);
>  
>  /**
> + * clk_enable_critical - inform the system when the clock source should be
> + *                      running, even if clock is critical.
> + * @clk: clock source
> + *
> + * If the clock can not be enabled/disabled, this should return success.
> + *
> + * May be called from atomic contexts.
> + *
> + * Returns success (0) or negative errno.
> + */
> +int clk_enable_critical(struct clk *clk);
> +
> +/**
>   * clk_disable - inform the system when the clock source is no longer required.
>   * @clk: clock source
>   *
> @@ -247,6 +260,23 @@ int clk_enable(struct clk *clk);
>  void clk_disable(struct clk *clk);
>  
>  /**
> + * clk_disable_critical - inform the system when the clock source is no
> + *                       longer required, even if clock is critical.
> + * @clk: clock source
> + *
> + * Inform the system that a clock source is no longer required by
> + * a driver and may be shut down.
> + *
> + * May be called from atomic contexts.
> + *
> + * Implementation detail: if the clock source is shared between
> + * multiple drivers, clk_enable_critical() calls must be balanced
> + * by the same number of clk_disable_critical() calls for the clock
> + * source to be disabled.
> + */
> +void clk_disable_critical(struct clk *clk);
> +
> +/**
>   * clk_get_rate - obtain the current clock rate (in Hz) for a clock source.
>   *               This is only valid once the clock source has been enabled.
>   * @clk: clock source
> -- 
> 1.9.1
> 

Re: [PATCH v7 3/5] clk: Supply the critical clock {init, enable, disable} framework

[Lee Jones]

On Wed, 29 Jul 2015, Michael Turquette wrote:

> Hi Lee,
> 
> + linux-clk ml
> 
> Quoting Lee Jones (2015-07-22 06:04:13)
> > These new API calls will firstly provide a mechanisms to tag a clock as
> > critical and secondly allow any knowledgeable driver to (un)gate clocks,
> > even if they are marked as critical.
> > 
> > Suggested-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > ---
> >  drivers/clk/clk.c            | 45 ++++++++++++++++++++++++++++++++++++++++++++
> >  include/linux/clk-provider.h |  2 ++
> >  include/linux/clk.h          | 30 +++++++++++++++++++++++++++++
> >  3 files changed, 77 insertions(+)
> > 
> > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> > index 61c3fc5..486b1da 100644
> > --- a/drivers/clk/clk.c
> > +++ b/drivers/clk/clk.c
> > @@ -46,6 +46,21 @@ static struct clk_core *clk_core_lookup(const char *name);
> >  
> >  /***    private data structures    ***/
> >  
> > +/**
> > + * struct critical -   Provides 'play' over critical clocks.  A clock can be
> > + *                     marked as critical, meaning that it should not be
> > + *                     disabled.  However, if a driver which is aware of the
> > + *                     critical behaviour wants to control it, it can do so
> > + *                     using clk_enable_critical() and clk_disable_critical().
> > + *
> > + * @enabled    Is clock critical?  Once set, doesn't change
> > + * @leave_on   Self explanatory.  Can be disabled by knowledgeable drivers
> 
> Not self explanatory. I need this explained to me. What does leave_on
> do? Better yet, what would happen if leave_on did not exist?
> 
> > + */
> > +struct critical {
> > +       bool enabled;
> > +       bool leave_on;
> > +};
> > +
> >  struct clk_core {
> >         const char              *name;
> >         const struct clk_ops    *ops;
> > @@ -75,6 +90,7 @@ struct clk_core {
> >         struct dentry           *dentry;
> >  #endif
> >         struct kref             ref;
> > +       struct critical         critical;
> >  };
> >  
> >  struct clk {
> > @@ -995,6 +1011,10 @@ static void clk_core_disable(struct clk_core *clk)
> >         if (WARN_ON(clk->enable_count == 0))
> >                 return;
> >  
> > +       /* Refuse to turn off a critical clock */
> > +       if (clk->enable_count == 1 && clk->critical.leave_on)
> > +               return;
> 
> How do we get to this point? clk_enable_critical actually calls
> clk_enable, thus incrementing the enable_count. The only time that we
> could hit the above case is if,
> 
> a) there is an imbalance in clk_enable and clk_disable calls. If this is
> the case then the drivers need to be fixed. Or better yet some
> infrastructure to catch that, now that we have per-user struct clk
> cookies.
> 
> b) a driver knowingly calls clk_enable_critical(foo) and then regular,
> old clk_disable(foo). But why would a driver do that?
> 
> It might be that I am missing the point here, so please feel free to
> clue me in.

This check behaves in a very similar to the WARN() above.  It's more
of a fail-safe.  If all drivers are behaving properly, then it
shouldn't ever be true.  If they're not, it prevents an incorrectly
written driver from irrecoverably crippling the system.

As I said in the other mail.  We can do without these 3 new wrappers.
We _could_ just write a driver which only calls clk_enable() _after_
it calls clk_disable(), a kind of intentional unbalance and it would
do that same thing.  However, what we're trying to do here is provide
a proper API, so we can see at first glance what the 'knowledgeable'
driver is trying to do and not have someone attempt to submit a 'fix'
which calls clk_enable() or something.

> > +
> >         if (--clk->enable_count > 0)
> >                 return;
> >  
> > @@ -1037,6 +1057,13 @@ void clk_disable(struct clk *clk)
> >  }
> >  EXPORT_SYMBOL_GPL(clk_disable);
> >  
> > +void clk_disable_critical(struct clk *clk)
> > +{
> > +       clk->core->critical.leave_on = false;
> > +       clk_disable(clk);
> > +}
> > +EXPORT_SYMBOL_GPL(clk_disable_critical);
> > +
> >  static int clk_core_enable(struct clk_core *clk)
> >  {
> >         int ret = 0;
> > @@ -1100,6 +1127,15 @@ int clk_enable(struct clk *clk)
> >  }
> >  EXPORT_SYMBOL_GPL(clk_enable);
> >  
> > +int clk_enable_critical(struct clk *clk)
> > +{
> > +       if (clk->core->critical.enabled)
> > +               clk->core->critical.leave_on = true;
> > +
> > +       return clk_enable(clk);
> > +}
> > +EXPORT_SYMBOL_GPL(clk_enable_critical);
> > +
> >  static unsigned long clk_core_round_rate_nolock(struct clk_core *clk,
> >                                                 unsigned long rate,
> >                                                 unsigned long min_rate,
> > @@ -2482,6 +2518,15 @@ fail_out:
> >  }
> >  EXPORT_SYMBOL_GPL(clk_register);
> >  
> > +void clk_init_critical(struct clk *clk)
> > +{
> > +       struct critical *critical = &clk->core->critical;
> > +
> > +       critical->enabled = true;
> > +       critical->leave_on = true;
> > +}
> > +EXPORT_SYMBOL_GPL(clk_init_critical);
> > +
> >  /*
> >   * Free memory allocated for a clock.
> >   * Caller must hold prepare_lock.
> > diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
> > index 5591ea7..15ef8c9 100644
> > --- a/include/linux/clk-provider.h
> > +++ b/include/linux/clk-provider.h
> > @@ -563,6 +563,8 @@ struct clk *devm_clk_register(struct device *dev, struct clk_hw *hw);
> >  void clk_unregister(struct clk *clk);
> >  void devm_clk_unregister(struct device *dev, struct clk *clk);
> >  
> > +void clk_init_critical(struct clk *clk);
> > +
> >  /* helper functions */
> >  const char *__clk_get_name(struct clk *clk);
> >  struct clk_hw *__clk_get_hw(struct clk *clk);
> > diff --git a/include/linux/clk.h b/include/linux/clk.h
> > index 8381bbf..9807f3b 100644
> > --- a/include/linux/clk.h
> > +++ b/include/linux/clk.h
> > @@ -231,6 +231,19 @@ struct clk *devm_clk_get(struct device *dev, const char *id);
> >  int clk_enable(struct clk *clk);
> >  
> >  /**
> > + * clk_enable_critical - inform the system when the clock source should be
> > + *                      running, even if clock is critical.
> > + * @clk: clock source
> > + *
> > + * If the clock can not be enabled/disabled, this should return success.
> > + *
> > + * May be called from atomic contexts.
> > + *
> > + * Returns success (0) or negative errno.
> > + */
> > +int clk_enable_critical(struct clk *clk);
> > +
> > +/**
> >   * clk_disable - inform the system when the clock source is no longer required.
> >   * @clk: clock source
> >   *
> > @@ -247,6 +260,23 @@ int clk_enable(struct clk *clk);
> >  void clk_disable(struct clk *clk);
> >  
> >  /**
> > + * clk_disable_critical - inform the system when the clock source is no
> > + *                       longer required, even if clock is critical.
> > + * @clk: clock source
> > + *
> > + * Inform the system that a clock source is no longer required by
> > + * a driver and may be shut down.
> > + *
> > + * May be called from atomic contexts.
> > + *
> > + * Implementation detail: if the clock source is shared between
> > + * multiple drivers, clk_enable_critical() calls must be balanced
> > + * by the same number of clk_disable_critical() calls for the clock
> > + * source to be disabled.
> > + */
> > +void clk_disable_critical(struct clk *clk);
> > +
> > +/**
> >   * clk_get_rate - obtain the current clock rate (in Hz) for a clock source.
> >   *               This is only valid once the clock source has been enabled.
> >   * @clk: clock source

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v7 3/5] clk: Supply the critical clock {init, enable, disable} framework

[Michael Turquette]

Quoting Lee Jones (2015-07-30 04:17:47)
> On Wed, 29 Jul 2015, Michael Turquette wrote:
> =

> > Hi Lee,
> > =

> > + linux-clk ml
> > =

> > Quoting Lee Jones (2015-07-22 06:04:13)
> > > These new API calls will firstly provide a mechanisms to tag a clock =
as
> > > critical and secondly allow any knowledgeable driver to (un)gate cloc=
ks,
> > > even if they are marked as critical.
> > > =

> > > Suggested-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> > > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > > ---
> > >  drivers/clk/clk.c            | 45 ++++++++++++++++++++++++++++++++++=
++++++++++
> > >  include/linux/clk-provider.h |  2 ++
> > >  include/linux/clk.h          | 30 +++++++++++++++++++++++++++++
> > >  3 files changed, 77 insertions(+)
> > > =

> > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> > > index 61c3fc5..486b1da 100644
> > > --- a/drivers/clk/clk.c
> > > +++ b/drivers/clk/clk.c
> > > @@ -46,6 +46,21 @@ static struct clk_core *clk_core_lookup(const char=
 *name);
> > >  =

> > >  /***    private data structures    ***/
> > >  =

> > > +/**
> > > + * struct critical -   Provides 'play' over critical clocks.  A cloc=
k can be
> > > + *                     marked as critical, meaning that it should no=
t be
> > > + *                     disabled.  However, if a driver which is awar=
e of the
> > > + *                     critical behaviour wants to control it, it ca=
n do so
> > > + *                     using clk_enable_critical() and clk_disable_c=
ritical().
> > > + *
> > > + * @enabled    Is clock critical?  Once set, doesn't change
> > > + * @leave_on   Self explanatory.  Can be disabled by knowledgeable d=
rivers
> > =

> > Not self explanatory. I need this explained to me. What does leave_on
> > do? Better yet, what would happen if leave_on did not exist?
> > =

> > > + */
> > > +struct critical {
> > > +       bool enabled;
> > > +       bool leave_on;
> > > +};
> > > +
> > >  struct clk_core {
> > >         const char              *name;
> > >         const struct clk_ops    *ops;
> > > @@ -75,6 +90,7 @@ struct clk_core {
> > >         struct dentry           *dentry;
> > >  #endif
> > >         struct kref             ref;
> > > +       struct critical         critical;
> > >  };
> > >  =

> > >  struct clk {
> > > @@ -995,6 +1011,10 @@ static void clk_core_disable(struct clk_core *c=
lk)
> > >         if (WARN_ON(clk->enable_count =3D=3D 0))
> > >                 return;
> > >  =

> > > +       /* Refuse to turn off a critical clock */
> > > +       if (clk->enable_count =3D=3D 1 && clk->critical.leave_on)
> > > +               return;
> > =

> > How do we get to this point? clk_enable_critical actually calls
> > clk_enable, thus incrementing the enable_count. The only time that we
> > could hit the above case is if,
> > =

> > a) there is an imbalance in clk_enable and clk_disable calls. If this is
> > the case then the drivers need to be fixed. Or better yet some
> > infrastructure to catch that, now that we have per-user struct clk
> > cookies.
> > =

> > b) a driver knowingly calls clk_enable_critical(foo) and then regular,
> > old clk_disable(foo). But why would a driver do that?
> > =

> > It might be that I am missing the point here, so please feel free to
> > clue me in.
> =

> This check behaves in a very similar to the WARN() above.  It's more
> of a fail-safe.  If all drivers are behaving properly, then it
> shouldn't ever be true.  If they're not, it prevents an incorrectly
> written driver from irrecoverably crippling the system.

Then this check should be replaced with a generic approach that refuses
to honor imbalances anyways. Below are two patches that probably resolve
the issue of badly behaving drivers that cause enable imbalances.

> =

> As I said in the other mail.  We can do without these 3 new wrappers.
> We _could_ just write a driver which only calls clk_enable() _after_
> it calls clk_disable(), a kind of intentional unbalance and it would
> do that same thing.

This naive approach will not work with per-user imbalance tracking.

> However, what we're trying to do here is provide
> a proper API, so we can see at first glance what the 'knowledgeable'
> driver is trying to do and not have someone attempt to submit a 'fix'
> which calls clk_enable() or something.

We'll need some type of api for sure for the handoff.

Regards,
Mike



From=203599ed206da9ce770bfafcfd95cbb9a03ac44473 Mon Sep 17 00:00:00 2001
From: Michael Turquette <mturquette@baylibre.com>
Date: Wed, 29 Jul 2015 18:22:45 -0700
Subject: [PATCH 1/2] clk: per-user clk prepare & enable ref counts

This patch adds prepare and enable reference counts for the per-user
handles that clock consumers have for a clock node. This patch warns if
an imbalance occurs while trying to disable or unprepare a clock and
aborts, leaving the hardware unaffected.

Signed-off-by: Michael Turquette <mturquette@baylibre.com>
---
 drivers/clk/clk.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 898052e..72feee9 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -84,6 +84,8 @@ struct clk {
 	unsigned long min_rate;
 	unsigned long max_rate;
 	struct hlist_node clks_node;
+	unsigned int enable_count;
+	unsigned int prepare_count;
 };
 =

 /***           locking             ***/
@@ -600,6 +602,9 @@ void clk_unprepare(struct clk *clk)
 		return;
 =

 	clk_prepare_lock();
+	if (WARN_ON(clk->prepare_count =3D=3D 0))
+		return;
+	clk->prepare_count--;
 	clk_core_unprepare(clk->core);
 	clk_prepare_unlock();
 }
@@ -657,6 +662,7 @@ int clk_prepare(struct clk *clk)
 		return 0;
 =

 	clk_prepare_lock();
+	clk->prepare_count++;
 	ret =3D clk_core_prepare(clk->core);
 	clk_prepare_unlock();
 =

@@ -707,6 +713,9 @@ void clk_disable(struct clk *clk)
 		return;
 =

 	flags =3D clk_enable_lock();
+	if (WARN_ON(clk->enable_count =3D=3D 0))
+		return;
+	clk->enable_count--;
 	clk_core_disable(clk->core);
 	clk_enable_unlock(flags);
 }
@@ -769,6 +778,7 @@ int clk_enable(struct clk *clk)
 		return 0;
 =

 	flags =3D clk_enable_lock();
+	clk->enable_count++;
 	ret =3D clk_core_enable(clk->core);
 	clk_enable_unlock(flags);
 =

-- =

1.9.1

From=20ace76f6ed634a69c499f8440a98d4b5a54d78368 Mon Sep 17 00:00:00 2001
From: Michael Turquette <mturquette@baylibre.com>
Date: Thu, 30 Jul 2015 12:52:26 -0700
Subject: [PATCH 2/2] clk: clk_put WARNs if user has not disabled clk

From=20the clk_put kerneldoc in include/linux/clk.h:

"""
Note: drivers must ensure that all clk_enable calls made on this clock
source are balanced by clk_disable calls prior to calling this function.
"""

The common clock framework implementation of the clk.h api has per-user
reference counts for calls to clk_prepare and clk_disable. As such it
can enforce the requirement to properly call clk_disable and
clk_unprepare before calling clk_put.

Because this requirement is probably violated in many places, this patch
starts with a simple warning. Once offending code has been fixed this
check could additionally release the reference counts automatically.

Signed-off-by: Michael Turquette <mturquette@baylibre.com>
---
 drivers/clk/clk.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 72feee9..6ec0f77 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2764,6 +2764,14 @@ void __clk_put(struct clk *clk)
 	    clk->max_rate < clk->core->req_rate)
 		clk_core_set_rate_nolock(clk->core, clk->core->req_rate);
 =

+	/*
+	 * before calling clk_put, all calls to clk_prepare and clk_enable from
+	 * a given user must be balanced with calls to clk_disable and
+	 * clk_unprepare by that same user
+	 */
+	WARN_ON(clk->prepare_count);
+	WARN_ON(clk->enable_count);
+
 	owner =3D clk->core->owner;
 	kref_put(&clk->core->ref, __clk_release);
 =

-- =

1.9.1

Re: [PATCH v7 3/5] clk: Supply the critical clock {init, enable, disable} framework

[Lee Jones]

On Thu, 30 Jul 2015, Michael Turquette wrote:

> Quoting Lee Jones (2015-07-30 04:17:47)
> > On Wed, 29 Jul 2015, Michael Turquette wrote:
> > 
> > > Hi Lee,
> > > 
> > > + linux-clk ml
> > > 
> > > Quoting Lee Jones (2015-07-22 06:04:13)
> > > > These new API calls will firstly provide a mechanisms to tag a clock as
> > > > critical and secondly allow any knowledgeable driver to (un)gate clocks,
> > > > even if they are marked as critical.
> > > > 
> > > > Suggested-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> > > > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > > > ---
> > > >  drivers/clk/clk.c            | 45 ++++++++++++++++++++++++++++++++++++++++++++
> > > >  include/linux/clk-provider.h |  2 ++
> > > >  include/linux/clk.h          | 30 +++++++++++++++++++++++++++++
> > > >  3 files changed, 77 insertions(+)
> > > > 
> > > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> > > > index 61c3fc5..486b1da 100644
> > > > --- a/drivers/clk/clk.c
> > > > +++ b/drivers/clk/clk.c
> > > > @@ -46,6 +46,21 @@ static struct clk_core *clk_core_lookup(const char *name);
> > > >  
> > > >  /***    private data structures    ***/
> > > >  
> > > > +/**
> > > > + * struct critical -   Provides 'play' over critical clocks.  A clock can be
> > > > + *                     marked as critical, meaning that it should not be
> > > > + *                     disabled.  However, if a driver which is aware of the
> > > > + *                     critical behaviour wants to control it, it can do so
> > > > + *                     using clk_enable_critical() and clk_disable_critical().
> > > > + *
> > > > + * @enabled    Is clock critical?  Once set, doesn't change
> > > > + * @leave_on   Self explanatory.  Can be disabled by knowledgeable drivers
> > > 
> > > Not self explanatory. I need this explained to me. What does leave_on
> > > do? Better yet, what would happen if leave_on did not exist?
> > > 
> > > > + */
> > > > +struct critical {
> > > > +       bool enabled;
> > > > +       bool leave_on;
> > > > +};
> > > > +
> > > >  struct clk_core {
> > > >         const char              *name;
> > > >         const struct clk_ops    *ops;
> > > > @@ -75,6 +90,7 @@ struct clk_core {
> > > >         struct dentry           *dentry;
> > > >  #endif
> > > >         struct kref             ref;
> > > > +       struct critical         critical;
> > > >  };
> > > >  
> > > >  struct clk {
> > > > @@ -995,6 +1011,10 @@ static void clk_core_disable(struct clk_core *clk)
> > > >         if (WARN_ON(clk->enable_count == 0))
> > > >                 return;
> > > >  
> > > > +       /* Refuse to turn off a critical clock */
> > > > +       if (clk->enable_count == 1 && clk->critical.leave_on)
> > > > +               return;
> > > 
> > > How do we get to this point? clk_enable_critical actually calls
> > > clk_enable, thus incrementing the enable_count. The only time that we
> > > could hit the above case is if,
> > > 
> > > a) there is an imbalance in clk_enable and clk_disable calls. If this is
> > > the case then the drivers need to be fixed. Or better yet some
> > > infrastructure to catch that, now that we have per-user struct clk
> > > cookies.
> > > 
> > > b) a driver knowingly calls clk_enable_critical(foo) and then regular,
> > > old clk_disable(foo). But why would a driver do that?
> > > 
> > > It might be that I am missing the point here, so please feel free to
> > > clue me in.
> > 
> > This check behaves in a very similar to the WARN() above.  It's more
> > of a fail-safe.  If all drivers are behaving properly, then it
> > shouldn't ever be true.  If they're not, it prevents an incorrectly
> > written driver from irrecoverably crippling the system.
> 
> Then this check should be replaced with a generic approach that refuses
> to honor imbalances anyways. Below are two patches that probably resolve
> the issue of badly behaving drivers that cause enable imbalances.

Your patch should make the requirement for this check moot, so it can
probably be removed.

> > As I said in the other mail.  We can do without these 3 new wrappers.
> > We _could_ just write a driver which only calls clk_enable() _after_
> > it calls clk_disable(), a kind of intentional unbalance and it would
> > do that same thing.
> 
> This naive approach will not work with per-user imbalance tracking.

Steady on.  I said we "_could_", that that I think it's a good idea.

I think it's a bad idea, which is why I wrote this set. ;)

> > However, what we're trying to do here is provide
> > a proper API, so we can see at first glance what the 'knowledgeable'
> > driver is trying to do and not have someone attempt to submit a 'fix'
> > which calls clk_enable() or something.
> 
> We'll need some type of api for sure for the handoff.

This set will not trigger your new checks.  The clocks will be in
perfect ballance becuase a reference will be taken at start-up.

Again:

start-up:
  clk_prepare_enable()

knowlegable_driver_probe:
  clk_get()

knowlegable_driver_gate_clk:
  clk_disable_critical()

knowlegable_driver_ungate_clk:
  clk_enable_critical()

knowlegable_driver_remove:
  clk_put()

> From 3599ed206da9ce770bfafcfd95cbb9a03ac44473 Mon Sep 17 00:00:00 2001
> From: Michael Turquette <mturquette@baylibre.com>
> Date: Wed, 29 Jul 2015 18:22:45 -0700
> Subject: [PATCH 1/2] clk: per-user clk prepare & enable ref counts
> 
> This patch adds prepare and enable reference counts for the per-user
> handles that clock consumers have for a clock node. This patch warns if
> an imbalance occurs while trying to disable or unprepare a clock and
> aborts, leaving the hardware unaffected.
> 
> Signed-off-by: Michael Turquette <mturquette@baylibre.com>
> ---
>  drivers/clk/clk.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 898052e..72feee9 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -84,6 +84,8 @@ struct clk {
>  	unsigned long min_rate;
>  	unsigned long max_rate;
>  	struct hlist_node clks_node;
> +	unsigned int enable_count;
> +	unsigned int prepare_count;
>  };
>  
>  /***           locking             ***/
> @@ -600,6 +602,9 @@ void clk_unprepare(struct clk *clk)
>  		return;
>  
>  	clk_prepare_lock();
> +	if (WARN_ON(clk->prepare_count == 0))
> +		return;
> +	clk->prepare_count--;
>  	clk_core_unprepare(clk->core);
>  	clk_prepare_unlock();
>  }
> @@ -657,6 +662,7 @@ int clk_prepare(struct clk *clk)
>  		return 0;
>  
>  	clk_prepare_lock();
> +	clk->prepare_count++;
>  	ret = clk_core_prepare(clk->core);
>  	clk_prepare_unlock();
>  
> @@ -707,6 +713,9 @@ void clk_disable(struct clk *clk)
>  		return;
>  
>  	flags = clk_enable_lock();
> +	if (WARN_ON(clk->enable_count == 0))
> +		return;
> +	clk->enable_count--;
>  	clk_core_disable(clk->core);
>  	clk_enable_unlock(flags);
>  }
> @@ -769,6 +778,7 @@ int clk_enable(struct clk *clk)
>  		return 0;
>  
>  	flags = clk_enable_lock();
> +	clk->enable_count++;
>  	ret = clk_core_enable(clk->core);
>  	clk_enable_unlock(flags);
>  

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v7 3/5] clk: Supply the critical clock {init, enable, disable} framework

[Michael Turquette]

UXVvdGluZyBMZWUgSm9uZXMgKDIwMTUtMDctMzEgMDI6MDI6MTkpCj4gT24gVGh1LCAzMCBKdWwg
MjAxNSwgTWljaGFlbCBUdXJxdWV0dGUgd3JvdGU6Cj4gCj4gPiBRdW90aW5nIExlZSBKb25lcyAo
MjAxNS0wNy0zMCAwNDoxNzo0NykKPiA+ID4gT24gV2VkLCAyOSBKdWwgMjAxNSwgTWljaGFlbCBU
dXJxdWV0dGUgd3JvdGU6Cj4gPiA+IAo+ID4gPiA+IEhpIExlZSwKPiA+ID4gPiAKPiA+ID4gPiAr
IGxpbnV4LWNsayBtbAo+ID4gPiA+IAo+ID4gPiA+IFF1b3RpbmcgTGVlIEpvbmVzICgyMDE1LTA3
LTIyIDA2OjA0OjEzKQo+ID4gPiA+ID4gVGhlc2UgbmV3IEFQSSBjYWxscyB3aWxsIGZpcnN0bHkg
cHJvdmlkZSBhIG1lY2hhbmlzbXMgdG8gdGFnIGEgY2xvY2sgYXMKPiA+ID4gPiA+IGNyaXRpY2Fs
IGFuZCBzZWNvbmRseSBhbGxvdyBhbnkga25vd2xlZGdlYWJsZSBkcml2ZXIgdG8gKHVuKWdhdGUg
Y2xvY2tzLAo+ID4gPiA+ID4gZXZlbiBpZiB0aGV5IGFyZSBtYXJrZWQgYXMgY3JpdGljYWwuCj4g
PiA+ID4gPiAKPiA+ID4gPiA+IFN1Z2dlc3RlZC1ieTogTWF4aW1lIFJpcGFyZCA8bWF4aW1lLnJp
cGFyZEBmcmVlLWVsZWN0cm9ucy5jb20+Cj4gPiA+ID4gPiBTaWduZWQtb2ZmLWJ5OiBMZWUgSm9u
ZXMgPGxlZS5qb25lc0BsaW5hcm8ub3JnPgo+ID4gPiA+ID4gLS0tCj4gPiA+ID4gPiAgZHJpdmVy
cy9jbGsvY2xrLmMgICAgICAgICAgICB8IDQ1ICsrKysrKysrKysrKysrKysrKysrKysrKysrKysr
KysrKysrKysrKysrKysrCj4gPiA+ID4gPiAgaW5jbHVkZS9saW51eC9jbGstcHJvdmlkZXIuaCB8
ICAyICsrCj4gPiA+ID4gPiAgaW5jbHVkZS9saW51eC9jbGsuaCAgICAgICAgICB8IDMwICsrKysr
KysrKysrKysrKysrKysrKysrKysrKysrCj4gPiA+ID4gPiAgMyBmaWxlcyBjaGFuZ2VkLCA3NyBp
bnNlcnRpb25zKCspCj4gPiA+ID4gPiAKPiA+ID4gPiA+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2Ns
ay9jbGsuYyBiL2RyaXZlcnMvY2xrL2Nsay5jCj4gPiA+ID4gPiBpbmRleCA2MWMzZmM1Li40ODZi
MWRhIDEwMDY0NAo+ID4gPiA+ID4gLS0tIGEvZHJpdmVycy9jbGsvY2xrLmMKPiA+ID4gPiA+ICsr
KyBiL2RyaXZlcnMvY2xrL2Nsay5jCj4gPiA+ID4gPiBAQCAtNDYsNiArNDYsMjEgQEAgc3RhdGlj
IHN0cnVjdCBjbGtfY29yZSAqY2xrX2NvcmVfbG9va3VwKGNvbnN0IGNoYXIgKm5hbWUpOwo+ID4g
PiA+ID4gIAo+ID4gPiA+ID4gIC8qKiogICAgcHJpdmF0ZSBkYXRhIHN0cnVjdHVyZXMgICAgKioq
Lwo+ID4gPiA+ID4gIAo+ID4gPiA+ID4gKy8qKgo+ID4gPiA+ID4gKyAqIHN0cnVjdCBjcml0aWNh
bCAtICAgUHJvdmlkZXMgJ3BsYXknIG92ZXIgY3JpdGljYWwgY2xvY2tzLiAgQSBjbG9jayBjYW4g
YmUKPiA+ID4gPiA+ICsgKiAgICAgICAgICAgICAgICAgICAgIG1hcmtlZCBhcyBjcml0aWNhbCwg
bWVhbmluZyB0aGF0IGl0IHNob3VsZCBub3QgYmUKPiA+ID4gPiA+ICsgKiAgICAgICAgICAgICAg
ICAgICAgIGRpc2FibGVkLiAgSG93ZXZlciwgaWYgYSBkcml2ZXIgd2hpY2ggaXMgYXdhcmUgb2Yg
dGhlCj4gPiA+ID4gPiArICogICAgICAgICAgICAgICAgICAgICBjcml0aWNhbCBiZWhhdmlvdXIg
d2FudHMgdG8gY29udHJvbCBpdCwgaXQgY2FuIGRvIHNvCj4gPiA+ID4gPiArICogICAgICAgICAg
ICAgICAgICAgICB1c2luZyBjbGtfZW5hYmxlX2NyaXRpY2FsKCkgYW5kIGNsa19kaXNhYmxlX2Ny
aXRpY2FsKCkuCj4gPiA+ID4gPiArICoKPiA+ID4gPiA+ICsgKiBAZW5hYmxlZCAgICBJcyBjbG9j
ayBjcml0aWNhbD8gIE9uY2Ugc2V0LCBkb2Vzbid0IGNoYW5nZQo+ID4gPiA+ID4gKyAqIEBsZWF2
ZV9vbiAgIFNlbGYgZXhwbGFuYXRvcnkuICBDYW4gYmUgZGlzYWJsZWQgYnkga25vd2xlZGdlYWJs
ZSBkcml2ZXJzCj4gPiA+ID4gCj4gPiA+ID4gTm90IHNlbGYgZXhwbGFuYXRvcnkuIEkgbmVlZCB0
aGlzIGV4cGxhaW5lZCB0byBtZS4gV2hhdCBkb2VzIGxlYXZlX29uCj4gPiA+ID4gZG8/IEJldHRl
ciB5ZXQsIHdoYXQgd291bGQgaGFwcGVuIGlmIGxlYXZlX29uIGRpZCBub3QgZXhpc3Q/Cj4gPiA+
ID4gCj4gPiA+ID4gPiArICovCj4gPiA+ID4gPiArc3RydWN0IGNyaXRpY2FsIHsKPiA+ID4gPiA+
ICsgICAgICAgYm9vbCBlbmFibGVkOwo+ID4gPiA+ID4gKyAgICAgICBib29sIGxlYXZlX29uOwo+
ID4gPiA+ID4gK307Cj4gPiA+ID4gPiArCj4gPiA+ID4gPiAgc3RydWN0IGNsa19jb3JlIHsKPiA+
ID4gPiA+ICAgICAgICAgY29uc3QgY2hhciAgICAgICAgICAgICAgKm5hbWU7Cj4gPiA+ID4gPiAg
ICAgICAgIGNvbnN0IHN0cnVjdCBjbGtfb3BzICAgICpvcHM7Cj4gPiA+ID4gPiBAQCAtNzUsNiAr
OTAsNyBAQCBzdHJ1Y3QgY2xrX2NvcmUgewo+ID4gPiA+ID4gICAgICAgICBzdHJ1Y3QgZGVudHJ5
ICAgICAgICAgICAqZGVudHJ5Owo+ID4gPiA+ID4gICNlbmRpZgo+ID4gPiA+ID4gICAgICAgICBz
dHJ1Y3Qga3JlZiAgICAgICAgICAgICByZWY7Cj4gPiA+ID4gPiArICAgICAgIHN0cnVjdCBjcml0
aWNhbCAgICAgICAgIGNyaXRpY2FsOwo+ID4gPiA+ID4gIH07Cj4gPiA+ID4gPiAgCj4gPiA+ID4g
PiAgc3RydWN0IGNsayB7Cj4gPiA+ID4gPiBAQCAtOTk1LDYgKzEwMTEsMTAgQEAgc3RhdGljIHZv
aWQgY2xrX2NvcmVfZGlzYWJsZShzdHJ1Y3QgY2xrX2NvcmUgKmNsaykKPiA+ID4gPiA+ICAgICAg
ICAgaWYgKFdBUk5fT04oY2xrLT5lbmFibGVfY291bnQgPT0gMCkpCj4gPiA+ID4gPiAgICAgICAg
ICAgICAgICAgcmV0dXJuOwo+ID4gPiA+ID4gIAo+ID4gPiA+ID4gKyAgICAgICAvKiBSZWZ1c2Ug
dG8gdHVybiBvZmYgYSBjcml0aWNhbCBjbG9jayAqLwo+ID4gPiA+ID4gKyAgICAgICBpZiAoY2xr
LT5lbmFibGVfY291bnQgPT0gMSAmJiBjbGstPmNyaXRpY2FsLmxlYXZlX29uKQo+ID4gPiA+ID4g
KyAgICAgICAgICAgICAgIHJldHVybjsKPiA+ID4gPiAKPiA+ID4gPiBIb3cgZG8gd2UgZ2V0IHRv
IHRoaXMgcG9pbnQ/IGNsa19lbmFibGVfY3JpdGljYWwgYWN0dWFsbHkgY2FsbHMKPiA+ID4gPiBj
bGtfZW5hYmxlLCB0aHVzIGluY3JlbWVudGluZyB0aGUgZW5hYmxlX2NvdW50LiBUaGUgb25seSB0
aW1lIHRoYXQgd2UKPiA+ID4gPiBjb3VsZCBoaXQgdGhlIGFib3ZlIGNhc2UgaXMgaWYsCj4gPiA+
ID4gCj4gPiA+ID4gYSkgdGhlcmUgaXMgYW4gaW1iYWxhbmNlIGluIGNsa19lbmFibGUgYW5kIGNs
a19kaXNhYmxlIGNhbGxzLiBJZiB0aGlzIGlzCj4gPiA+ID4gdGhlIGNhc2UgdGhlbiB0aGUgZHJp
dmVycyBuZWVkIHRvIGJlIGZpeGVkLiBPciBiZXR0ZXIgeWV0IHNvbWUKPiA+ID4gPiBpbmZyYXN0
cnVjdHVyZSB0byBjYXRjaCB0aGF0LCBub3cgdGhhdCB3ZSBoYXZlIHBlci11c2VyIHN0cnVjdCBj
bGsKPiA+ID4gPiBjb29raWVzLgo+ID4gPiA+IAo+ID4gPiA+IGIpIGEgZHJpdmVyIGtub3dpbmds
eSBjYWxscyBjbGtfZW5hYmxlX2NyaXRpY2FsKGZvbykgYW5kIHRoZW4gcmVndWxhciwKPiA+ID4g
PiBvbGQgY2xrX2Rpc2FibGUoZm9vKS4gQnV0IHdoeSB3b3VsZCBhIGRyaXZlciBkbyB0aGF0Pwo+
ID4gPiA+IAo+ID4gPiA+IEl0IG1pZ2h0IGJlIHRoYXQgSSBhbSBtaXNzaW5nIHRoZSBwb2ludCBo
ZXJlLCBzbyBwbGVhc2UgZmVlbCBmcmVlIHRvCj4gPiA+ID4gY2x1ZSBtZSBpbi4KPiA+ID4gCj4g
PiA+IFRoaXMgY2hlY2sgYmVoYXZlcyBpbiBhIHZlcnkgc2ltaWxhciB0byB0aGUgV0FSTigpIGFi
b3ZlLiAgSXQncyBtb3JlCj4gPiA+IG9mIGEgZmFpbC1zYWZlLiAgSWYgYWxsIGRyaXZlcnMgYXJl
IGJlaGF2aW5nIHByb3Blcmx5LCB0aGVuIGl0Cj4gPiA+IHNob3VsZG4ndCBldmVyIGJlIHRydWUu
ICBJZiB0aGV5J3JlIG5vdCwgaXQgcHJldmVudHMgYW4gaW5jb3JyZWN0bHkKPiA+ID4gd3JpdHRl
biBkcml2ZXIgZnJvbSBpcnJlY292ZXJhYmx5IGNyaXBwbGluZyB0aGUgc3lzdGVtLgo+ID4gCj4g
PiBUaGVuIHRoaXMgY2hlY2sgc2hvdWxkIGJlIHJlcGxhY2VkIHdpdGggYSBnZW5lcmljIGFwcHJv
YWNoIHRoYXQgcmVmdXNlcwo+ID4gdG8gaG9ub3IgaW1iYWxhbmNlcyBhbnl3YXlzLiBCZWxvdyBh
cmUgdHdvIHBhdGNoZXMgdGhhdCBwcm9iYWJseSByZXNvbHZlCj4gPiB0aGUgaXNzdWUgb2YgYmFk
bHkgYmVoYXZpbmcgZHJpdmVycyB0aGF0IGNhdXNlIGVuYWJsZSBpbWJhbGFuY2VzLgo+IAo+IFlv
dXIgcGF0Y2ggc2hvdWxkIG1ha2UgdGhlIHJlcXVpcmVtZW50IGZvciB0aGlzIGNoZWNrIG1vb3Qs
IHNvIGl0IGNhbgo+IHByb2JhYmx5IGJlIHJlbW92ZWQuCj4gCj4gPiA+IEFzIEkgc2FpZCBpbiB0
aGUgb3RoZXIgbWFpbC4gIFdlIGNhbiBkbyB3aXRob3V0IHRoZXNlIDMgbmV3IHdyYXBwZXJzLgo+
ID4gPiBXZSBfY291bGRfIGp1c3Qgd3JpdGUgYSBkcml2ZXIgd2hpY2ggb25seSBjYWxscyBjbGtf
ZW5hYmxlKCkgX2FmdGVyXwo+ID4gPiBpdCBjYWxscyBjbGtfZGlzYWJsZSgpLCBhIGtpbmQgb2Yg
aW50ZW50aW9uYWwgdW5iYWxhbmNlIGFuZCBpdCB3b3VsZAo+ID4gPiBkbyB0aGF0IHNhbWUgdGhp
bmcuCj4gPiAKPiA+IFRoaXMgbmFpdmUgYXBwcm9hY2ggd2lsbCBub3Qgd29yayB3aXRoIHBlci11
c2VyIGltYmFsYW5jZSB0cmFja2luZy4KPiAKPiBTdGVhZHkgb24uICBJIHNhaWQgd2UgIl9jb3Vs
ZF8iLCB0aGF0IHRoYXQgSSB0aGluayBpdCdzIGEgZ29vZCBpZGVhLgo+IAo+IEkgdGhpbmsgaXQn
cyBhIGJhZCBpZGVhLCB3aGljaCBpcyB3aHkgSSB3cm90ZSB0aGlzIHNldC4gOykKPiAKPiA+ID4g
SG93ZXZlciwgd2hhdCB3ZSdyZSB0cnlpbmcgdG8gZG8gaGVyZSBpcyBwcm92aWRlCj4gPiA+IGEg
cHJvcGVyIEFQSSwgc28gd2UgY2FuIHNlZSBhdCBmaXJzdCBnbGFuY2Ugd2hhdCB0aGUgJ2tub3ds
ZWRnZWFibGUnCj4gPiA+IGRyaXZlciBpcyB0cnlpbmcgdG8gZG8gYW5kIG5vdCBoYXZlIHNvbWVv
bmUgYXR0ZW1wdCB0byBzdWJtaXQgYSAnZml4Jwo+ID4gPiB3aGljaCBjYWxscyBjbGtfZW5hYmxl
KCkgb3Igc29tZXRoaW5nLgo+ID4gCj4gPiBXZSdsbCBuZWVkIHNvbWUgdHlwZSBvZiBhcGkgZm9y
IHN1cmUgZm9yIHRoZSBoYW5kb2ZmLgo+IAo+IFRoaXMgc2V0IHdpbGwgbm90IHRyaWdnZXIgeW91
ciBuZXcgY2hlY2tzLiAgVGhlIGNsb2NrcyB3aWxsIGJlIGluCj4gcGVyZmVjdCBiYWxsYW5jZSBi
ZWN1YXNlIGEgcmVmZXJlbmNlIHdpbGwgYmUgdGFrZW4gYXQgc3RhcnQtdXAuCj4gCj4gQWdhaW46
Cj4gCj4gc3RhcnQtdXA6Cj4gICBjbGtfcHJlcGFyZV9lbmFibGUoKQo+IAo+IGtub3dsZWdhYmxl
X2RyaXZlcl9wcm9iZToKPiAgIGNsa19nZXQoKQo+IAo+IGtub3dsZWdhYmxlX2RyaXZlcl9nYXRl
X2NsazoKPiAgIGNsa19kaXNhYmxlX2NyaXRpY2FsKCkKClRoZSBjYWxsIHRvIGNsa19kaXNhYmxl
KCkgbmVzdGVkIGluc2lkZSBjbGtfZGlzYWJsZV9jcml0aWNhbCB3aWxsIGZhaWwKd2l0aCB0aGUg
bmV3IGNoZWNrcy4gVGhpcyBpcyBiZWNhdXNlIHRoZSBzdHJ1Y3QgY2xrIGluc3RhbmNlIHdpbGwg
YmUKZGlmZmVyZW50IGZyb20gb25lIHVzZWQgaW4geW91ciAic3RhcnQtdXAiIHNlY3Rpb24gYWJv
dmUuIGNsa19nZXQoKQpjcmVhdGVzIGEgdW5pcXVlIHN0cnVjdCBjbGsgZXZlcnkgdGltZSB5b3Ug
Y2FsbCBpdC4KClB1dCBhbm90aGVyIHdheSwgYSB1bmlxdWUgdXNlciBvZiBhIGNsb2NrIGNhbm5v
dCBjYWxsIGNsa19kaXNhYmxlKCkgd2hlbgp0aGUgcGVyLXVzZXIgZW5hYmxlX2NvdW50IGlzIDAu
CgpGdXJ0aGVybW9yZSwgdGhlcmUgaXMgbm8gd2F5IHRoYXQgSSB3aWxsIGV2ZXIgYmUgaGFwcHkg
d2l0aCBhIHRlY2huaXF1ZQp0aGF0IHJlcXVpcmVzIGNhbGxpbmcgZGlzYWJsZSBwcmlvciB0byBh
biBlbmFibGUgd2l0aGluIGEgZHJpdmVyLiBUaGF0CmdvZXMgYWdhaW5zdCBhIGxvbmctc3RhbmRp
bmcgYXBpIGRlc2lnbnMgYW5kIGlzIGNvbmZ1c2luZyBhcyBoZWxsIHRvCmRyaXZlciBhdXRob3Jz
LgoKUmVnYXJkcywKTWlrZQoKPiAKPiBrbm93bGVnYWJsZV9kcml2ZXJfdW5nYXRlX2NsazoKPiAg
IGNsa19lbmFibGVfY3JpdGljYWwoKQo+IAo+IGtub3dsZWdhYmxlX2RyaXZlcl9yZW1vdmU6Cj4g
ICBjbGtfcHV0KCkKPiAKPiA+IEZyb20gMzU5OWVkMjA2ZGE5Y2U3NzBiZmFmY2ZkOTVjYmI5YTAz
YWM0NDQ3MyBNb24gU2VwIDE3IDAwOjAwOjAwIDIwMDEKPiA+IEZyb206IE1pY2hhZWwgVHVycXVl
dHRlIDxtdHVycXVldHRlQGJheWxpYnJlLmNvbT4KPiA+IERhdGU6IFdlZCwgMjkgSnVsIDIwMTUg
MTg6MjI6NDUgLTA3MDAKPiA+IFN1YmplY3Q6IFtQQVRDSCAxLzJdIGNsazogcGVyLXVzZXIgY2xr
IHByZXBhcmUgJiBlbmFibGUgcmVmIGNvdW50cwo+ID4gCj4gPiBUaGlzIHBhdGNoIGFkZHMgcHJl
cGFyZSBhbmQgZW5hYmxlIHJlZmVyZW5jZSBjb3VudHMgZm9yIHRoZSBwZXItdXNlcgo+ID4gaGFu
ZGxlcyB0aGF0IGNsb2NrIGNvbnN1bWVycyBoYXZlIGZvciBhIGNsb2NrIG5vZGUuIFRoaXMgcGF0
Y2ggd2FybnMgaWYKPiA+IGFuIGltYmFsYW5jZSBvY2N1cnMgd2hpbGUgdHJ5aW5nIHRvIGRpc2Fi
bGUgb3IgdW5wcmVwYXJlIGEgY2xvY2sgYW5kCj4gPiBhYm9ydHMsIGxlYXZpbmcgdGhlIGhhcmR3
YXJlIHVuYWZmZWN0ZWQuCj4gPiAKPiA+IFNpZ25lZC1vZmYtYnk6IE1pY2hhZWwgVHVycXVldHRl
IDxtdHVycXVldHRlQGJheWxpYnJlLmNvbT4KPiA+IC0tLQo+ID4gIGRyaXZlcnMvY2xrL2Nsay5j
IHwgMTAgKysrKysrKysrKwo+ID4gIDEgZmlsZSBjaGFuZ2VkLCAxMCBpbnNlcnRpb25zKCspCj4g
PiAKPiA+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2Nsay9jbGsuYyBiL2RyaXZlcnMvY2xrL2Nsay5j
Cj4gPiBpbmRleCA4OTgwNTJlLi43MmZlZWU5IDEwMDY0NAo+ID4gLS0tIGEvZHJpdmVycy9jbGsv
Y2xrLmMKPiA+ICsrKyBiL2RyaXZlcnMvY2xrL2Nsay5jCj4gPiBAQCAtODQsNiArODQsOCBAQCBz
dHJ1Y3QgY2xrIHsKPiA+ICAgICAgIHVuc2lnbmVkIGxvbmcgbWluX3JhdGU7Cj4gPiAgICAgICB1
bnNpZ25lZCBsb25nIG1heF9yYXRlOwo+ID4gICAgICAgc3RydWN0IGhsaXN0X25vZGUgY2xrc19u
b2RlOwo+ID4gKyAgICAgdW5zaWduZWQgaW50IGVuYWJsZV9jb3VudDsKPiA+ICsgICAgIHVuc2ln
bmVkIGludCBwcmVwYXJlX2NvdW50Owo+ID4gIH07Cj4gPiAgCj4gPiAgLyoqKiAgICAgICAgICAg
bG9ja2luZyAgICAgICAgICAgICAqKiovCj4gPiBAQCAtNjAwLDYgKzYwMiw5IEBAIHZvaWQgY2xr
X3VucHJlcGFyZShzdHJ1Y3QgY2xrICpjbGspCj4gPiAgICAgICAgICAgICAgIHJldHVybjsKPiA+
ICAKPiA+ICAgICAgIGNsa19wcmVwYXJlX2xvY2soKTsKPiA+ICsgICAgIGlmIChXQVJOX09OKGNs
ay0+cHJlcGFyZV9jb3VudCA9PSAwKSkKPiA+ICsgICAgICAgICAgICAgcmV0dXJuOwo+ID4gKyAg
ICAgY2xrLT5wcmVwYXJlX2NvdW50LS07Cj4gPiAgICAgICBjbGtfY29yZV91bnByZXBhcmUoY2xr
LT5jb3JlKTsKPiA+ICAgICAgIGNsa19wcmVwYXJlX3VubG9jaygpOwo+ID4gIH0KPiA+IEBAIC02
NTcsNiArNjYyLDcgQEAgaW50IGNsa19wcmVwYXJlKHN0cnVjdCBjbGsgKmNsaykKPiA+ICAgICAg
ICAgICAgICAgcmV0dXJuIDA7Cj4gPiAgCj4gPiAgICAgICBjbGtfcHJlcGFyZV9sb2NrKCk7Cj4g
PiArICAgICBjbGstPnByZXBhcmVfY291bnQrKzsKPiA+ICAgICAgIHJldCA9IGNsa19jb3JlX3By
ZXBhcmUoY2xrLT5jb3JlKTsKPiA+ICAgICAgIGNsa19wcmVwYXJlX3VubG9jaygpOwo+ID4gIAo+
ID4gQEAgLTcwNyw2ICs3MTMsOSBAQCB2b2lkIGNsa19kaXNhYmxlKHN0cnVjdCBjbGsgKmNsaykK
PiA+ICAgICAgICAgICAgICAgcmV0dXJuOwo+ID4gIAo+ID4gICAgICAgZmxhZ3MgPSBjbGtfZW5h
YmxlX2xvY2soKTsKPiA+ICsgICAgIGlmIChXQVJOX09OKGNsay0+ZW5hYmxlX2NvdW50ID09IDAp
KQo+ID4gKyAgICAgICAgICAgICByZXR1cm47Cj4gPiArICAgICBjbGstPmVuYWJsZV9jb3VudC0t
Owo+ID4gICAgICAgY2xrX2NvcmVfZGlzYWJsZShjbGstPmNvcmUpOwo+ID4gICAgICAgY2xrX2Vu
YWJsZV91bmxvY2soZmxhZ3MpOwo+ID4gIH0KPiA+IEBAIC03NjksNiArNzc4LDcgQEAgaW50IGNs
a19lbmFibGUoc3RydWN0IGNsayAqY2xrKQo+ID4gICAgICAgICAgICAgICByZXR1cm4gMDsKPiA+
ICAKPiA+ICAgICAgIGZsYWdzID0gY2xrX2VuYWJsZV9sb2NrKCk7Cj4gPiArICAgICBjbGstPmVu
YWJsZV9jb3VudCsrOwo+ID4gICAgICAgcmV0ID0gY2xrX2NvcmVfZW5hYmxlKGNsay0+Y29yZSk7
Cj4gPiAgICAgICBjbGtfZW5hYmxlX3VubG9jayhmbGFncyk7Cj4gPiAgCj4gCj4gLS0gCj4gTGVl
IEpvbmVzCj4gTGluYXJvIFNUTWljcm9lbGVjdHJvbmljcyBMYW5kaW5nIFRlYW0gTGVhZAo+IExp
bmFyby5vcmcg4pSCIE9wZW4gc291cmNlIHNvZnR3YXJlIGZvciBBUk0gU29Dcwo+IEZvbGxvdyBM
aW5hcm86IEZhY2Vib29rIHwgVHdpdHRlciB8IEJsb2cKPiAtLQo+IFRvIHVuc3Vic2NyaWJlIGZy
b20gdGhpcyBsaXN0OiBzZW5kIHRoZSBsaW5lICJ1bnN1YnNjcmliZSBsaW51eC1jbGsiIGluCj4g
dGhlIGJvZHkgb2YgYSBtZXNzYWdlIHRvIG1ham9yZG9tb0B2Z2VyLmtlcm5lbC5vcmcKPiBNb3Jl
IG1ham9yZG9tbyBpbmZvIGF0ICBodHRwOi8vdmdlci5rZXJuZWwub3JnL21ham9yZG9tby1pbmZv
Lmh0bWwKCl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmxp
bnV4LWFybS1rZXJuZWwgbWFpbGluZyBsaXN0CmxpbnV4LWFybS1rZXJuZWxAbGlzdHMuaW5mcmFk
ZWFkLm9yZwpodHRwOi8vbGlzdHMuaW5mcmFkZWFkLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2xpbnV4
LWFybS1rZXJuZWwK