| Message ID | 1500364414-10021-3-git-send-email-gabriel.fernandez@st.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 07/18/2017 10:53 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 | 3 ++- > include/linux/clk-provider.h | 1 + > 2 files changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c > index 4e0c054a..dd82485 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); > @@ -101,6 +101,7 @@ static int clk_gate_is_enabled(struct clk_hw *hw) > > return reg ? 1 : 0; > } > +EXPORT_SYMBOL_GPL(clk_gate_is_enabled); > > const struct clk_ops clk_gate_ops = { > .enable = clk_gate_enable, > 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); Here the prefix does not reflect the type of its argument, it might be acceptable for a veiled function, but it is not wanted for the exported function. Something like clk_hw_gate_is_enabled() is expected here, but again, let's firstly come to an agreement, that the export is needed. > > struct clk_div_table { > unsigned int val; > -- With best wishes, Vladimir
On 07/18, Vladimir Zapolskiy wrote: > On 07/18/2017 10:53 AM, gabriel.fernandez@st.com wrote: > > From: Gabriel Fernandez <gabriel.fernandez@st.com> > > } > > +EXPORT_SYMBOL_GPL(clk_gate_is_enabled); > > > > const struct clk_ops clk_gate_ops = { > > .enable = clk_gate_enable, > > 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); > > Here the prefix does not reflect the type of its argument, it might be > acceptable for a veiled function, but it is not wanted for the exported > function. Something like clk_hw_gate_is_enabled() is expected here, but > again, let's firstly come to an agreement, that the export is needed. > I'd prefer clk_gate_is_enabled() as it's not a struct clk_hw_gate, it's a struct clk_gate and there isn't any requirement for function names to reflect the type of the argument. That's what we have type analysis for.
On 07/19/2017 01:52 AM, Stephen Boyd wrote: > On 07/18, Vladimir Zapolskiy wrote: >> On 07/18/2017 10:53 AM, gabriel.fernandez@st.com wrote: >>> From: Gabriel Fernandez <gabriel.fernandez@st.com> >>> } >>> +EXPORT_SYMBOL_GPL(clk_gate_is_enabled); >>> >>> const struct clk_ops clk_gate_ops = { >>> .enable = clk_gate_enable, >>> 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); >> >> Here the prefix does not reflect the type of its argument, it might be >> acceptable for a veiled function, but it is not wanted for the exported >> function. Something like clk_hw_gate_is_enabled() is expected here, but >> again, let's firstly come to an agreement, that the export is needed. >> > > I'd prefer clk_gate_is_enabled() as it's not a struct clk_hw_gate, > it's a struct clk_gate Formally it's a 'struct clk_hw', and 'struct clk_hw_gate' does not exist. > and there isn't any requirement for function names to reflect the type > of the argument. That's what we have type analysis for. Sure, a function name can be selected arbitrary, however because in this particular case type analysis operates on 'struct clk_hw', it would fail to separate 'struct clk_gate' from 'struct clk_divider', a little hint in the naming may be helpful. I don't insist on my preference, of course your acceptance is sufficient. -- With best wishes, Vladimir
diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c index 4e0c054a..dd82485 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); @@ -101,6 +101,7 @@ static int clk_gate_is_enabled(struct clk_hw *hw) return reg ? 1 : 0; } +EXPORT_SYMBOL_GPL(clk_gate_is_enabled); const struct clk_ops clk_gate_ops = { .enable = clk_gate_enable, 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;