diff mbox series

[3/5] clk: qcom: clk-rpm: convert to parent_data API

Message ID 20220705202837.667-4-ansuelsmth@gmail.com (mailing list archive)
State Superseded, archived
Headers show
Series clk: qcom: Drop use of pxo/cxo_board for rpm devices | expand

Commit Message

Christian Marangi July 5, 2022, 8:28 p.m. UTC
Convert clk-rpm driver to parent_data API and change parent names to cxo
and pxo instead of the board variant as pxo_board is an ancient name and
pxo should be used instead.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 drivers/clk/qcom/clk-rpm.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

Comments

Dmitry Baryshkov July 6, 2022, 8:27 a.m. UTC | #1
On Tue, 5 Jul 2022 at 23:56, Christian Marangi <ansuelsmth@gmail.com> wrote:
>
> Convert clk-rpm driver to parent_data API and change parent names to cxo
> and pxo instead of the board variant as pxo_board is an ancient name and
> pxo should be used instead.
>
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> ---
>  drivers/clk/qcom/clk-rpm.c | 24 ++++++++++++++++--------
>  1 file changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/clk/qcom/clk-rpm.c b/drivers/clk/qcom/clk-rpm.c
> index a18811c38018..d45a8b0fc97c 100644
> --- a/drivers/clk/qcom/clk-rpm.c
> +++ b/drivers/clk/qcom/clk-rpm.c
> @@ -23,6 +23,14 @@
>  #define QCOM_RPM_SCALING_ENABLE_ID                     0x2
>  #define QCOM_RPM_XO_MODE_ON                            0x2
>
> +static const struct clk_parent_data gcc_pxo[] = {
> +       { .fw_name = "pxo", .name = "pxo" },

This means that the core clock would look for the DT clock named 'pxo'
with the fallback to the global clock named 'pxo'. Fallback is
required to keep compatibility with older DT files, not having the
clocks/clock-names properties.
For all other platforms we were using as a fallback  the 'pxo_board'
clock (defined in the DT) rather than the 'pxo' (hackily injected by
the gcc driver).
I'd suggest following this pattern and using .fw_name = "pxo', .name =
"pxo_board".

Same applies to the CXO clock.

> +};
> +
Christian Marangi July 6, 2022, 10:21 a.m. UTC | #2
On Wed, Jul 06, 2022 at 11:27:34AM +0300, Dmitry Baryshkov wrote:
> On Tue, 5 Jul 2022 at 23:56, Christian Marangi <ansuelsmth@gmail.com> wrote:
> >
> > Convert clk-rpm driver to parent_data API and change parent names to cxo
> > and pxo instead of the board variant as pxo_board is an ancient name and
> > pxo should be used instead.
> >
> > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> > ---
> >  drivers/clk/qcom/clk-rpm.c | 24 ++++++++++++++++--------
> >  1 file changed, 16 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/clk/qcom/clk-rpm.c b/drivers/clk/qcom/clk-rpm.c
> > index a18811c38018..d45a8b0fc97c 100644
> > --- a/drivers/clk/qcom/clk-rpm.c
> > +++ b/drivers/clk/qcom/clk-rpm.c
> > @@ -23,6 +23,14 @@
> >  #define QCOM_RPM_SCALING_ENABLE_ID                     0x2
> >  #define QCOM_RPM_XO_MODE_ON                            0x2
> >
> > +static const struct clk_parent_data gcc_pxo[] = {
> > +       { .fw_name = "pxo", .name = "pxo" },
> 
> This means that the core clock would look for the DT clock named 'pxo'
> with the fallback to the global clock named 'pxo'. Fallback is
> required to keep compatibility with older DT files, not having the
> clocks/clock-names properties.
> For all other platforms we were using as a fallback  the 'pxo_board'
> clock (defined in the DT) rather than the 'pxo' (hackily injected by
> the gcc driver).
> I'd suggest following this pattern and using .fw_name = "pxo', .name =
> "pxo_board".

We were discussing this to the lcc driver. Problematic topic, yes will
follow the pattern just to not change things.

> 
> Same applies to the CXO clock.
> 
> > +};
> > +
> 
> 
> -- 
> With best wishes
> Dmitry
Christian Marangi July 6, 2022, 1:16 p.m. UTC | #3
On Wed, Jul 06, 2022 at 03:26:53PM +0200, Konrad Dybcio wrote:
> 
> 
> On 5.07.2022 22:28, Christian Marangi wrote:
> > Convert clk-rpm driver to parent_data API and change parent names to cxo
> > and pxo instead of the board variant as pxo_board is an ancient name and
> > pxo should be used instead.
> > 
> > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> > ---
> >  drivers/clk/qcom/clk-rpm.c | 24 ++++++++++++++++--------
> >  1 file changed, 16 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/clk/qcom/clk-rpm.c b/drivers/clk/qcom/clk-rpm.c
> > index a18811c38018..d45a8b0fc97c 100644
> > --- a/drivers/clk/qcom/clk-rpm.c
> > +++ b/drivers/clk/qcom/clk-rpm.c
> > @@ -23,6 +23,14 @@
> >  #define QCOM_RPM_SCALING_ENABLE_ID			0x2
> >  #define QCOM_RPM_XO_MODE_ON				0x2
> >  
> > +static const struct clk_parent_data gcc_pxo[] = {
> > +	{ .fw_name = "pxo", .name = "pxo" },
> The .name lookup should include _board, as the old DTs expect that.
> If I understand correctly though, you need to preserve both pxo and
> pxo_board, as they were separate, not sure though..
>

pxo_board is what comes first... then qcom gcc driver hacks it and
create a factor clk with 1/1 div and provide pxo.
In practice they are the same clk. But yhea I should not change the
name, will resent with .name = pxo_board and be done with it.

> Konrad
> > +};
> > +
> > +static const struct clk_parent_data gcc_cxo[] = {
> > +	{ .fw_name = "cxo", .name = "cxo" },
> > +};
> > +
> >  #define DEFINE_CLK_RPM(_platform, _name, _active, r_id)			      \
> >  	static struct clk_rpm _platform##_##_active;			      \
> >  	static struct clk_rpm _platform##_##_name = {			      \
> > @@ -32,8 +40,8 @@
> >  		.hw.init = &(struct clk_init_data){			      \
> >  			.ops = &clk_rpm_ops,				      \
> >  			.name = #_name,					      \
> > -			.parent_names = (const char *[]){ "pxo_board" },      \
> > -			.num_parents = 1,				      \
> > +			.parent_data = gcc_pxo,				      \
> > +			.num_parents = ARRAY_SIZE(gcc_pxo),		      \
> >  		},							      \
> >  	};								      \
> >  	static struct clk_rpm _platform##_##_active = {			      \
> > @@ -44,8 +52,8 @@
> >  		.hw.init = &(struct clk_init_data){			      \
> >  			.ops = &clk_rpm_ops,				      \
> >  			.name = #_active,				      \
> > -			.parent_names = (const char *[]){ "pxo_board" },      \
> > -			.num_parents = 1,				      \
> > +			.parent_data = gcc_pxo,				      \
> > +			.num_parents = ARRAY_SIZE(gcc_pxo),		      \
> >  		},							      \
> >  	}
> >  
> > @@ -56,8 +64,8 @@
> >  		.hw.init = &(struct clk_init_data){			      \
> >  			.ops = &clk_rpm_xo_ops,			      \
> >  			.name = #_name,					      \
> > -			.parent_names = (const char *[]){ "cxo_board" },      \
> > -			.num_parents = 1,				      \
> > +			.parent_data = gcc_cxo,				      \
> > +			.num_parents = ARRAY_SIZE(gcc_cxo),		      \
> >  		},							      \
> >  	}
> >  
> > @@ -68,8 +76,8 @@
> >  		.hw.init = &(struct clk_init_data){			      \
> >  			.ops = &clk_rpm_fixed_ops,			      \
> >  			.name = #_name,					      \
> > -			.parent_names = (const char *[]){ "pxo" },	      \
> > -			.num_parents = 1,				      \
> > +			.parent_data = gcc_pxo,				      \
> > +			.num_parents = ARRAY_SIZE(gcc_pxo),		      \
> >  		},							      \
> >  	}
> >
Konrad Dybcio July 6, 2022, 1:26 p.m. UTC | #4
On 5.07.2022 22:28, Christian Marangi wrote:
> Convert clk-rpm driver to parent_data API and change parent names to cxo
> and pxo instead of the board variant as pxo_board is an ancient name and
> pxo should be used instead.
> 
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> ---
>  drivers/clk/qcom/clk-rpm.c | 24 ++++++++++++++++--------
>  1 file changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/clk/qcom/clk-rpm.c b/drivers/clk/qcom/clk-rpm.c
> index a18811c38018..d45a8b0fc97c 100644
> --- a/drivers/clk/qcom/clk-rpm.c
> +++ b/drivers/clk/qcom/clk-rpm.c
> @@ -23,6 +23,14 @@
>  #define QCOM_RPM_SCALING_ENABLE_ID			0x2
>  #define QCOM_RPM_XO_MODE_ON				0x2
>  
> +static const struct clk_parent_data gcc_pxo[] = {
> +	{ .fw_name = "pxo", .name = "pxo" },
The .name lookup should include _board, as the old DTs expect that.
If I understand correctly though, you need to preserve both pxo and
pxo_board, as they were separate, not sure though..

Konrad
> +};
> +
> +static const struct clk_parent_data gcc_cxo[] = {
> +	{ .fw_name = "cxo", .name = "cxo" },
> +};
> +
>  #define DEFINE_CLK_RPM(_platform, _name, _active, r_id)			      \
>  	static struct clk_rpm _platform##_##_active;			      \
>  	static struct clk_rpm _platform##_##_name = {			      \
> @@ -32,8 +40,8 @@
>  		.hw.init = &(struct clk_init_data){			      \
>  			.ops = &clk_rpm_ops,				      \
>  			.name = #_name,					      \
> -			.parent_names = (const char *[]){ "pxo_board" },      \
> -			.num_parents = 1,				      \
> +			.parent_data = gcc_pxo,				      \
> +			.num_parents = ARRAY_SIZE(gcc_pxo),		      \
>  		},							      \
>  	};								      \
>  	static struct clk_rpm _platform##_##_active = {			      \
> @@ -44,8 +52,8 @@
>  		.hw.init = &(struct clk_init_data){			      \
>  			.ops = &clk_rpm_ops,				      \
>  			.name = #_active,				      \
> -			.parent_names = (const char *[]){ "pxo_board" },      \
> -			.num_parents = 1,				      \
> +			.parent_data = gcc_pxo,				      \
> +			.num_parents = ARRAY_SIZE(gcc_pxo),		      \
>  		},							      \
>  	}
>  
> @@ -56,8 +64,8 @@
>  		.hw.init = &(struct clk_init_data){			      \
>  			.ops = &clk_rpm_xo_ops,			      \
>  			.name = #_name,					      \
> -			.parent_names = (const char *[]){ "cxo_board" },      \
> -			.num_parents = 1,				      \
> +			.parent_data = gcc_cxo,				      \
> +			.num_parents = ARRAY_SIZE(gcc_cxo),		      \
>  		},							      \
>  	}
>  
> @@ -68,8 +76,8 @@
>  		.hw.init = &(struct clk_init_data){			      \
>  			.ops = &clk_rpm_fixed_ops,			      \
>  			.name = #_name,					      \
> -			.parent_names = (const char *[]){ "pxo" },	      \
> -			.num_parents = 1,				      \
> +			.parent_data = gcc_pxo,				      \
> +			.num_parents = ARRAY_SIZE(gcc_pxo),		      \
>  		},							      \
>  	}
>
diff mbox series

Patch

diff --git a/drivers/clk/qcom/clk-rpm.c b/drivers/clk/qcom/clk-rpm.c
index a18811c38018..d45a8b0fc97c 100644
--- a/drivers/clk/qcom/clk-rpm.c
+++ b/drivers/clk/qcom/clk-rpm.c
@@ -23,6 +23,14 @@ 
 #define QCOM_RPM_SCALING_ENABLE_ID			0x2
 #define QCOM_RPM_XO_MODE_ON				0x2
 
+static const struct clk_parent_data gcc_pxo[] = {
+	{ .fw_name = "pxo", .name = "pxo" },
+};
+
+static const struct clk_parent_data gcc_cxo[] = {
+	{ .fw_name = "cxo", .name = "cxo" },
+};
+
 #define DEFINE_CLK_RPM(_platform, _name, _active, r_id)			      \
 	static struct clk_rpm _platform##_##_active;			      \
 	static struct clk_rpm _platform##_##_name = {			      \
@@ -32,8 +40,8 @@ 
 		.hw.init = &(struct clk_init_data){			      \
 			.ops = &clk_rpm_ops,				      \
 			.name = #_name,					      \
-			.parent_names = (const char *[]){ "pxo_board" },      \
-			.num_parents = 1,				      \
+			.parent_data = gcc_pxo,				      \
+			.num_parents = ARRAY_SIZE(gcc_pxo),		      \
 		},							      \
 	};								      \
 	static struct clk_rpm _platform##_##_active = {			      \
@@ -44,8 +52,8 @@ 
 		.hw.init = &(struct clk_init_data){			      \
 			.ops = &clk_rpm_ops,				      \
 			.name = #_active,				      \
-			.parent_names = (const char *[]){ "pxo_board" },      \
-			.num_parents = 1,				      \
+			.parent_data = gcc_pxo,				      \
+			.num_parents = ARRAY_SIZE(gcc_pxo),		      \
 		},							      \
 	}
 
@@ -56,8 +64,8 @@ 
 		.hw.init = &(struct clk_init_data){			      \
 			.ops = &clk_rpm_xo_ops,			      \
 			.name = #_name,					      \
-			.parent_names = (const char *[]){ "cxo_board" },      \
-			.num_parents = 1,				      \
+			.parent_data = gcc_cxo,				      \
+			.num_parents = ARRAY_SIZE(gcc_cxo),		      \
 		},							      \
 	}
 
@@ -68,8 +76,8 @@ 
 		.hw.init = &(struct clk_init_data){			      \
 			.ops = &clk_rpm_fixed_ops,			      \
 			.name = #_name,					      \
-			.parent_names = (const char *[]){ "pxo" },	      \
-			.num_parents = 1,				      \
+			.parent_data = gcc_pxo,				      \
+			.num_parents = ARRAY_SIZE(gcc_pxo),		      \
 		},							      \
 	}