[v4,1/5] clk: Introduce devm_clk_hw_register_gate_parent_data()

Commit Message

Marek Vasut Feb. 23, 2023, 5:11 p.m. UTC

Add an API for clock gate that uses parent_data for the parent instead of
a string parent_name.

Signed-off-by: Marek Vasut <[e-mail]>
---
Cc: Abel Vesa <[e-mail]>
Cc: Fabio Estevam <[e-mail]>
Cc: Jacky Bai <[e-mail]>
Cc: Krzysztof Kozlowski <[e-mail]>
Cc: Laurent Pinchart <[e-mail]>
Cc: Lucas Stach <[e-mail]>
Cc: Marco Felsch <[e-mail]>
Cc: Michael Turquette <[e-mail]>
Cc: NXP Linux Team <[e-mail]>
Cc: Peng Fan <[e-mail]>
Cc: Pengutronix Kernel Team <[e-mail]>
Cc: Richard Cochran <[e-mail]>
Cc: Rob Herring <[e-mail]>
Cc: Sascha Hauer <[e-mail]>
Cc: Shawn Guo <[e-mail]>
Cc: Stephen Boyd <[e-mail]>
Cc: [e-mail]
Cc: [e-mail]
Cc: [e-mail]
---
V3: New patch
V4: - Rebase on next 20230223
---
 include/linux/clk-provider.h | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

Comments

Adam Ford Feb. 24, 2023, 12:24 a.m. UTC | #1

On Thu, Feb 23, 2023 at 11:12 AM Marek Vasut <[e-mail]> wrote:
>
> Add an API for clock gate that uses parent_data for the parent instead of
> a string parent_name.
>

For the series:

Tested-by:  Adam Ford <[e-mail]> #imx8mp-beacon-kit

adam
> Signed-off-by: Marek Vasut <[e-mail]>
> ---
> Cc: Abel Vesa <[e-mail]>
> Cc: Fabio Estevam <[e-mail]>
> Cc: Jacky Bai <[e-mail]>
> Cc: Krzysztof Kozlowski <[e-mail]>
> Cc: Laurent Pinchart <[e-mail]>
> Cc: Lucas Stach <[e-mail]>
> Cc: Marco Felsch <[e-mail]>
> Cc: Michael Turquette <[e-mail]>
> Cc: NXP Linux Team <[e-mail]>
> Cc: Peng Fan <[e-mail]>
> Cc: Pengutronix Kernel Team <[e-mail]>
> Cc: Richard Cochran <[e-mail]>
> Cc: Rob Herring <[e-mail]>
> Cc: Sascha Hauer <[e-mail]>
> Cc: Shawn Guo <[e-mail]>
> Cc: Stephen Boyd <[e-mail]>
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> ---
> V3: New patch
> V4: - Rebase on next 20230223
> ---
>  include/linux/clk-provider.h | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
>
> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
> index 842e72a5348fa..92b7c794c6272 100644
> --- a/include/linux/clk-provider.h
> +++ b/include/linux/clk-provider.h
> @@ -608,6 +608,25 @@ struct clk *clk_register_gate(struct device *dev, const char *name,
>         __devm_clk_hw_register_gate((dev), NULL, (name), (parent_name), NULL, \
>                                NULL, (flags), (reg), (bit_idx),               \
>                                (clk_gate_flags), (lock))
> +
> +/**
> + * devm_clk_hw_register_gate - register a gate clock with the clock framework
> + * @dev: device that is registering this clock
> + * @name: name of this clock
> + * @parent_data: parent clk data
> + * @flags: framework-specific flags for this clock
> + * @reg: register address to control gating of this clock
> + * @bit_idx: which bit in the register controls gating of this clock
> + * @clk_gate_flags: gate-specific flags for this clock
> + * @lock: shared register lock for this clock
> + */
> +#define devm_clk_hw_register_gate_parent_data(dev, name, parent_data, flags,  \
> +                                             reg, bit_idx, clk_gate_flags,   \
> +                                             lock)                           \
> +       __devm_clk_hw_register_gate((dev), NULL, (name), NULL, NULL,          \
> +                                   (parent_data), (flags), (reg), (bit_idx), \
> +                                   (clk_gate_flags), (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);
> --
> 2.39.1
>

Alexander Stein Feb. 27, 2023, 8:38 a.m. UTC | #2

Hi Marek,

thanks for respinning this series.

Am Donnerstag, 23. Februar 2023, 18:11:10 CET schrieb Marek Vasut:
> Add an API for clock gate that uses parent_data for the parent instead of
> a string parent_name.
> 
> Signed-off-by: Marek Vasut <[e-mail]>
> ---
> Cc: Abel Vesa <[e-mail]>
> Cc: Fabio Estevam <[e-mail]>
> Cc: Jacky Bai <[e-mail]>
> Cc: Krzysztof Kozlowski <[e-mail]>
> Cc: Laurent Pinchart <[e-mail]>
> Cc: Lucas Stach <[e-mail]>
> Cc: Marco Felsch <[e-mail]>
> Cc: Michael Turquette <[e-mail]>
> Cc: NXP Linux Team <[e-mail]>
> Cc: Peng Fan <[e-mail]>
> Cc: Pengutronix Kernel Team <[e-mail]>
> Cc: Richard Cochran <[e-mail]>
> Cc: Rob Herring <[e-mail]>
> Cc: Sascha Hauer <[e-mail]>
> Cc: Shawn Guo <[e-mail]>
> Cc: Stephen Boyd <[e-mail]>
> Cc: [e-mail]
> Cc: [e-mail]
> Cc: [e-mail]
> ---
> V3: New patch
> V4: - Rebase on next 20230223
> ---
>  include/linux/clk-provider.h | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
> index 842e72a5348fa..92b7c794c6272 100644
> --- a/include/linux/clk-provider.h
> +++ b/include/linux/clk-provider.h
> @@ -608,6 +608,25 @@ struct clk *clk_register_gate(struct device *dev, const
> char *name, __devm_clk_hw_register_gate((dev), NULL, (name), (parent_name),
> NULL, \ NULL, (flags), (reg), (bit_idx),		      \
>  			       (clk_gate_flags), (lock))
> +
> +/**
> + * devm_clk_hw_register_gate - register a gate clock with the clock
> framework + * @dev: device that is registering this clock
> + * @name: name of this clock
> + * @parent_data: parent clk data
> + * @flags: framework-specific flags for this clock
> + * @reg: register address to control gating of this clock
> + * @bit_idx: which bit in the register controls gating of this clock
> + * @clk_gate_flags: gate-specific flags for this clock
> + * @lock: shared register lock for this clock
> + */
> +#define devm_clk_hw_register_gate_parent_data(dev, name, parent_data,
> flags,  \ +					      reg, 
bit_idx, clk_gate_flags,   \
> +					      lock)		
	      \
> +	__devm_clk_hw_register_gate((dev), NULL, (name), NULL, NULL,	      \
> +				    (parent_data), (flags), (reg), 
(bit_idx), \
> +				    (clk_gate_flags), (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);

Is it worth to add the parent_data paremeter into existing 
devm_clk_hw_register_gate macro? Just adding this new macro for setting 
parent_data instead of parent_name seems a bit too much. What if someone wants 
to set parent_hw? Add another macro?

Best regards,
Alexander

Marek Vasut Feb. 27, 2023, 1:47 p.m. UTC | #3

On 2/27/23 09:38, Alexander Stein wrote:

[...]

>> +/**
>> + * devm_clk_hw_register_gate - register a gate clock with the clock
>> framework + * @dev: device that is registering this clock
>> + * @name: name of this clock
>> + * @parent_data: parent clk data
>> + * @flags: framework-specific flags for this clock
>> + * @reg: register address to control gating of this clock
>> + * @bit_idx: which bit in the register controls gating of this clock
>> + * @clk_gate_flags: gate-specific flags for this clock
>> + * @lock: shared register lock for this clock
>> + */
>> +#define devm_clk_hw_register_gate_parent_data(dev, name, parent_data,
>> flags,  \ +					      reg,
> bit_idx, clk_gate_flags,   \
>> +					      lock)		
> 	      \
>> +	__devm_clk_hw_register_gate((dev), NULL, (name), NULL, NULL,	      \
>> +				    (parent_data), (flags), (reg),
> (bit_idx), \
>> +				    (clk_gate_flags), (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);
> 
> Is it worth to add the parent_data paremeter into existing
> devm_clk_hw_register_gate macro? Just adding this new macro for setting
> parent_data instead of parent_name seems a bit too much. What if someone wants
> to set parent_hw? Add another macro?

Yes, another macro seems to be the accepted practice here.

Patch

diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 842e72a5348fa..92b7c794c6272 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -608,6 +608,25 @@  struct clk *clk_register_gate(struct device *dev, const char *name,
 	__devm_clk_hw_register_gate((dev), NULL, (name), (parent_name), NULL, \
 			       NULL, (flags), (reg), (bit_idx),		      \
 			       (clk_gate_flags), (lock))
+
+/**
+ * devm_clk_hw_register_gate - register a gate clock with the clock framework
+ * @dev: device that is registering this clock
+ * @name: name of this clock
+ * @parent_data: parent clk data
+ * @flags: framework-specific flags for this clock
+ * @reg: register address to control gating of this clock
+ * @bit_idx: which bit in the register controls gating of this clock
+ * @clk_gate_flags: gate-specific flags for this clock
+ * @lock: shared register lock for this clock
+ */
+#define devm_clk_hw_register_gate_parent_data(dev, name, parent_data, flags,  \
+					      reg, bit_idx, clk_gate_flags,   \
+					      lock)			      \
+	__devm_clk_hw_register_gate((dev), NULL, (name), NULL, NULL,	      \
+				    (parent_data), (flags), (reg), (bit_idx), \
+				    (clk_gate_flags), (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);