diff mbox

[v5,1/2] clk: gate: expose clk_gate_ops::is_enabled

Message ID 1499954552-20075-2-git-send-email-gabriel.fernandez@st.com (mailing list archive)
State Superseded, archived
Delegated to: Stephen Boyd
Headers show

Commit Message

Gabriel FERNANDEZ July 13, 2017, 2:02 p.m. UTC
From: Gabriel Fernandez <gabriel.fernandez@st.com>

This patch exposes clk_gate_ops::is_enabled as functions
that can be directly called and assigned in places like this so
we don't need wrapper functions that do nothing besides forward
the call.

Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
Sugested by Stephen Boyd <sboyd@codeaurora.org>
---
 drivers/clk/clk-gate.c       | 2 +-
 include/linux/clk-provider.h | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

Comments

Florian Fainelli July 13, 2017, 5:22 p.m. UTC | #1
On 07/13/2017 07:02 AM, gabriel.fernandez@st.com wrote:
> From: Gabriel Fernandez <gabriel.fernandez@st.com>
> 
> This patch exposes clk_gate_ops::is_enabled as functions
> that can be directly called and assigned in places like this so
> we don't need wrapper functions that do nothing besides forward
> the call.
> 
> Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
> Sugested by Stephen Boyd <sboyd@codeaurora.org>
> ---
>  drivers/clk/clk-gate.c       | 2 +-
>  include/linux/clk-provider.h | 1 +
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c
> index 4e0c054a..e27e28f 100644
> --- a/drivers/clk/clk-gate.c
> +++ b/drivers/clk/clk-gate.c
> @@ -86,7 +86,7 @@ static void clk_gate_disable(struct clk_hw *hw)
>  	clk_gate_endisable(hw, 0);
>  }
>  
> -static int clk_gate_is_enabled(struct clk_hw *hw)
> +int clk_gate_is_enabled(struct clk_hw *hw)
>  {
>  	u32 reg;
>  	struct clk_gate *gate = to_clk_gate(hw);

Don't you need to add an EXPORT_SYMBOL_GPL(clk_gate_is_enabled) as well
in case this gets used by modules?

> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
> index c59c625..e9587ab 100644
> --- a/include/linux/clk-provider.h
> +++ b/include/linux/clk-provider.h
> @@ -343,6 +343,7 @@ struct clk_hw *clk_hw_register_gate(struct device *dev, const char *name,
>  		u8 clk_gate_flags, spinlock_t *lock);
>  void clk_unregister_gate(struct clk *clk);
>  void clk_hw_unregister_gate(struct clk_hw *hw);
> +int clk_gate_is_enabled(struct clk_hw *hw);
>  
>  struct clk_div_table {
>  	unsigned int	val;
>
Stephen Boyd July 14, 2017, 1:16 a.m. UTC | #2
On 07/13, Florian Fainelli wrote:
> On 07/13/2017 07:02 AM, gabriel.fernandez@st.com wrote:
> > From: Gabriel Fernandez <gabriel.fernandez@st.com>
> > 
> > This patch exposes clk_gate_ops::is_enabled as functions
> > that can be directly called and assigned in places like this so
> > we don't need wrapper functions that do nothing besides forward
> > the call.
> > 
> > Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
> > Sugested by Stephen Boyd <sboyd@codeaurora.org>
> > ---
> >  drivers/clk/clk-gate.c       | 2 +-
> >  include/linux/clk-provider.h | 1 +
> >  2 files changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c
> > index 4e0c054a..e27e28f 100644
> > --- a/drivers/clk/clk-gate.c
> > +++ b/drivers/clk/clk-gate.c
> > @@ -86,7 +86,7 @@ static void clk_gate_disable(struct clk_hw *hw)
> >  	clk_gate_endisable(hw, 0);
> >  }
> >  
> > -static int clk_gate_is_enabled(struct clk_hw *hw)
> > +int clk_gate_is_enabled(struct clk_hw *hw)
> >  {
> >  	u32 reg;
> >  	struct clk_gate *gate = to_clk_gate(hw);
> 
> Don't you need to add an EXPORT_SYMBOL_GPL(clk_gate_is_enabled) as well
> in case this gets used by modules?

It would be needed in the future if someone uses it from a
module. The only user in this patch series looks to be builtin
only. I can add it when applying the patch if there aren't other
comments on the series.
kernel test robot July 14, 2017, 6:52 p.m. UTC | #3
Hi Gabriel,

[auto build test ERROR on clk/clk-next]
[also build test ERROR on v4.12 next-20170714]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/gabriel-fernandez-st-com/clk-stm32h7-Add-stm32h743-clock-driver/20170714-170518
base:   https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next
config: arm-lpc32xx_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

All errors (new ones prefixed by >>):

>> drivers/clk/nxp/clk-lpc32xx.c:906:12: error: static declaration of 'clk_gate_is_enabled' follows non-static declaration
    static int clk_gate_is_enabled(struct clk_hw *hw)
               ^~~~~~~~~~~~~~~~~~~
   In file included from drivers/clk/nxp/clk-lpc32xx.c:13:0:
   include/linux/clk-provider.h:346:5: note: previous declaration of 'clk_gate_is_enabled' was here
    int clk_gate_is_enabled(struct clk_hw *hw);
        ^~~~~~~~~~~~~~~~~~~

vim +/clk_gate_is_enabled +906 drivers/clk/nxp/clk-lpc32xx.c

f7c82a60 Vladimir Zapolskiy 2015-12-06  905  
f7c82a60 Vladimir Zapolskiy 2015-12-06 @906  static int clk_gate_is_enabled(struct clk_hw *hw)
f7c82a60 Vladimir Zapolskiy 2015-12-06  907  {
f7c82a60 Vladimir Zapolskiy 2015-12-06  908  	struct lpc32xx_clk_gate *clk = to_lpc32xx_gate(hw);
f7c82a60 Vladimir Zapolskiy 2015-12-06  909  	u32 val;
f7c82a60 Vladimir Zapolskiy 2015-12-06  910  	bool is_set;
f7c82a60 Vladimir Zapolskiy 2015-12-06  911  
f7c82a60 Vladimir Zapolskiy 2015-12-06  912  	regmap_read(clk_regmap, clk->reg, &val);
f7c82a60 Vladimir Zapolskiy 2015-12-06  913  	is_set = val & BIT(clk->bit_idx);
f7c82a60 Vladimir Zapolskiy 2015-12-06  914  
f7c82a60 Vladimir Zapolskiy 2015-12-06  915  	return (clk->flags & CLK_GATE_SET_TO_DISABLE ? !is_set : is_set);
f7c82a60 Vladimir Zapolskiy 2015-12-06  916  }
f7c82a60 Vladimir Zapolskiy 2015-12-06  917  

:::::: The code at line 906 was first introduced by commit
:::::: f7c82a60ba26c2f003662bcb2cff131021c1e828 clk: lpc32xx: add common clock framework driver

:::::: TO: Vladimir Zapolskiy <vz@mleia.com>
:::::: CC: Michael Turquette <mturquette@baylibre.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Gabriel FERNANDEZ July 17, 2017, 7:30 a.m. UTC | #4
Hi Stephen,


On 07/14/2017 08:52 PM, kbuild test robot wrote:
> Hi Gabriel,
>
> [auto build test ERROR on clk/clk-next]
> [also build test ERROR on v4.12 next-20170714]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>
> url:    https://github.com/0day-ci/linux/commits/gabriel-fernandez-st-com/clk-stm32h7-Add-stm32h743-clock-driver/20170714-170518
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next
> config: arm-lpc32xx_defconfig (attached as .config)
> compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
> reproduce:
>          wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>          chmod +x ~/bin/make.cross
>          # save the attached .config to linux build tree
>          make.cross ARCH=arm
>
> All errors (new ones prefixed by >>):
>
>>> drivers/clk/nxp/clk-lpc32xx.c:906:12: error: static declaration of 'clk_gate_is_enabled' follows non-static declaration
>      static int clk_gate_is_enabled(struct clk_hw *hw)
>                 ^~~~~~~~~~~~~~~~~~~
>     In file included from drivers/clk/nxp/clk-lpc32xx.c:13:0:
>     include/linux/clk-provider.h:346:5: note: previous declaration of 'clk_gate_is_enabled' was here
>      int clk_gate_is_enabled(struct clk_hw *hw);
>          ^~~~~~~~~~~~~~~~~~~
>
> vim +/clk_gate_is_enabled +906 drivers/clk/nxp/clk-lpc32xx.c
>
> f7c82a60 Vladimir Zapolskiy 2015-12-06  905
> f7c82a60 Vladimir Zapolskiy 2015-12-06 @906  static int clk_gate_is_enabled(struct clk_hw *hw)
> f7c82a60 Vladimir Zapolskiy 2015-12-06  907  {
> f7c82a60 Vladimir Zapolskiy 2015-12-06  908  	struct lpc32xx_clk_gate *clk = to_lpc32xx_gate(hw);
> f7c82a60 Vladimir Zapolskiy 2015-12-06  909  	u32 val;
> f7c82a60 Vladimir Zapolskiy 2015-12-06  910  	bool is_set;
> f7c82a60 Vladimir Zapolskiy 2015-12-06  911
> f7c82a60 Vladimir Zapolskiy 2015-12-06  912  	regmap_read(clk_regmap, clk->reg, &val);
> f7c82a60 Vladimir Zapolskiy 2015-12-06  913  	is_set = val & BIT(clk->bit_idx);
> f7c82a60 Vladimir Zapolskiy 2015-12-06  914
> f7c82a60 Vladimir Zapolskiy 2015-12-06  915  	return (clk->flags & CLK_GATE_SET_TO_DISABLE ? !is_set : is_set);
> f7c82a60 Vladimir Zapolskiy 2015-12-06  916  }
> f7c82a60 Vladimir Zapolskiy 2015-12-06  917  EXPORT_SYMBOL_GPL(__clk_gate_is_enabled);
>
>
> :::::: The code at line 906 was first introduced by commit
> :::::: f7c82a60ba26c2f003662bcb2cff131021c1e828 clk: lpc32xx: add common clock framework driver
>
> :::::: TO: Vladimir Zapolskiy <vz@mleia.com>
> :::::: CC: Michael Turquette <mturquette@baylibre.com>
>
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Rename 'clk_gate_is_enabled' into'__clk_gate_is_enabled' from clk-gate.c 
file, is it a good solution for you ?

i could add also EXPORT_SYMBOL_GPL(__clk_gate_is_enabled) if you are ok.


Best Regards
Gabriel--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephen Boyd July 18, 2017, 1:17 a.m. UTC | #5
On 07/17, Gabriel FERNANDEZ wrote:
> Hi Stephen,
> 
> 
> On 07/14/2017 08:52 PM, kbuild test robot wrote:
> > Hi Gabriel,
> >
> > [auto build test ERROR on clk/clk-next]
> > [also build test ERROR on v4.12 next-20170714]
> > [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
> >
> > url:    https://github.com/0day-ci/linux/commits/gabriel-fernandez-st-com/clk-stm32h7-Add-stm32h743-clock-driver/20170714-170518
> > base:   https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next
> > config: arm-lpc32xx_defconfig (attached as .config)
> > compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
> > reproduce:
> >          wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> >          chmod +x ~/bin/make.cross
> >          # save the attached .config to linux build tree
> >          make.cross ARCH=arm
> >
> > All errors (new ones prefixed by >>):
> >
> >>> drivers/clk/nxp/clk-lpc32xx.c:906:12: error: static declaration of 'clk_gate_is_enabled' follows non-static declaration
> >      static int clk_gate_is_enabled(struct clk_hw *hw)
> >                 ^~~~~~~~~~~~~~~~~~~
> >     In file included from drivers/clk/nxp/clk-lpc32xx.c:13:0:
> >     include/linux/clk-provider.h:346:5: note: previous declaration of 'clk_gate_is_enabled' was here
> >      int clk_gate_is_enabled(struct clk_hw *hw);
> >          ^~~~~~~~~~~~~~~~~~~
> >
> > https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
> 
> Rename 'clk_gate_is_enabled' into'__clk_gate_is_enabled' from clk-gate.c 
> file, is it a good solution for you ?
> 
> i could add also EXPORT_SYMBOL_GPL(__clk_gate_is_enabled) if you are ok.

No. We should rename the lpc32xx one to be less generic as it's
in a specific driver. Double underscore usually means internal or
unlocked (and we poorly did it this way for
__clk_mux_determine_rate function already).
diff mbox

Patch

diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c
index 4e0c054a..e27e28f 100644
--- a/drivers/clk/clk-gate.c
+++ b/drivers/clk/clk-gate.c
@@ -86,7 +86,7 @@  static void clk_gate_disable(struct clk_hw *hw)
 	clk_gate_endisable(hw, 0);
 }
 
-static int clk_gate_is_enabled(struct clk_hw *hw)
+int clk_gate_is_enabled(struct clk_hw *hw)
 {
 	u32 reg;
 	struct clk_gate *gate = to_clk_gate(hw);
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index c59c625..e9587ab 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -343,6 +343,7 @@  struct clk_hw *clk_hw_register_gate(struct device *dev, const char *name,
 		u8 clk_gate_flags, spinlock_t *lock);
 void clk_unregister_gate(struct clk *clk);
 void clk_hw_unregister_gate(struct clk_hw *hw);
+int clk_gate_is_enabled(struct clk_hw *hw);
 
 struct clk_div_table {
 	unsigned int	val;