diff mbox series

clk: qcom: gcc-ipq806x: use parent_data for the last remaining entry

Message ID 20220620215150.1875557-1-dmitry.baryshkov@linaro.org
State Superseded
Headers show
Series clk: qcom: gcc-ipq806x: use parent_data for the last remaining entry | expand

Commit Message

Dmitry Baryshkov June 20, 2022, 9:51 p.m. UTC 
Use parent_data for the last remaining entry (pll4). This clock is
provided by the lcc device.

Fixes: cb02866f9a74 ("clk: qcom: gcc-ipq806x: convert parent_names to parent_data")
Cc: Ansuel Smith <ansuelsmth@gmail.com>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/clk/qcom/gcc-ipq806x.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Christian Marangi June 21, 2022, 3:21 p.m. UTC | #1 
On Tue, Jun 21, 2022 at 12:51:50AM +0300, Dmitry Baryshkov wrote:
> Use parent_data for the last remaining entry (pll4). This clock is
> provided by the lcc device.
> 
> Fixes: cb02866f9a74 ("clk: qcom: gcc-ipq806x: convert parent_names to parent_data")
> Cc: Ansuel Smith <ansuelsmth@gmail.com>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  drivers/clk/qcom/gcc-ipq806x.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/qcom/gcc-ipq806x.c b/drivers/clk/qcom/gcc-ipq806x.c
> index 718de17a1e60..6447f3e81b55 100644
> --- a/drivers/clk/qcom/gcc-ipq806x.c
> +++ b/drivers/clk/qcom/gcc-ipq806x.c
> @@ -79,7 +79,9 @@ static struct clk_regmap pll4_vote = {
>  	.enable_mask = BIT(4),
>  	.hw.init = &(struct clk_init_data){
>  		.name = "pll4_vote",
> -		.parent_names = (const char *[]){ "pll4" },
> +		.parent_data = &(const struct clk_parent_data){
> +			.fw_name = "pll4", .name = "pll4",
> +		},
>  		.num_parents = 1,
>  		.ops = &clk_pll_vote_ops,
>  	},
> -- 
> 2.35.1
> 

Hi my intention was finding a way to directly reference the hw clk from
the lcc driver instead of using fw_name/name parent data. Wonder if that
would be a better solution... Seems wrong to me to eventually add also
the pll4 clk in the dts to correctly use the fw_name definition (when
that will be fixed in the ipq8064 dtsi)
Dmitry Baryshkov June 21, 2022, 5 p.m. UTC | #2 
On Tue, 21 Jun 2022 at 18:21, Christian Marangi <ansuelsmth@gmail.com> wrote:
>
> On Tue, Jun 21, 2022 at 12:51:50AM +0300, Dmitry Baryshkov wrote:
> > Use parent_data for the last remaining entry (pll4). This clock is
> > provided by the lcc device.
> >
> > Fixes: cb02866f9a74 ("clk: qcom: gcc-ipq806x: convert parent_names to parent_data")
> > Cc: Ansuel Smith <ansuelsmth@gmail.com>
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > ---
> >  drivers/clk/qcom/gcc-ipq806x.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/clk/qcom/gcc-ipq806x.c b/drivers/clk/qcom/gcc-ipq806x.c
> > index 718de17a1e60..6447f3e81b55 100644
> > --- a/drivers/clk/qcom/gcc-ipq806x.c
> > +++ b/drivers/clk/qcom/gcc-ipq806x.c
> > @@ -79,7 +79,9 @@ static struct clk_regmap pll4_vote = {
> >       .enable_mask = BIT(4),
> >       .hw.init = &(struct clk_init_data){
> >               .name = "pll4_vote",
> > -             .parent_names = (const char *[]){ "pll4" },
> > +             .parent_data = &(const struct clk_parent_data){
> > +                     .fw_name = "pll4", .name = "pll4",
> > +             },
> >               .num_parents = 1,
> >               .ops = &clk_pll_vote_ops,
> >       },
> > --
> > 2.35.1
> >
>
> Hi my intention was finding a way to directly reference the hw clk from
> the lcc driver instead of using fw_name/name parent data. Wonder if that
> would be a better solution... Seems wrong to me to eventually add also
> the pll4 clk in the dts to correctly use the fw_name definition (when
> that will be fixed in the ipq8064 dtsi)

Please don't. They are two separate hardware pieces, two separate
drivers. Please don't invent anything fancy. Let the OF clk code
handle it.
Yes, this will result in "pll4" clock being referenced in DT. We
already have binding for that, <&lcc PLL4>.
Christian Marangi June 21, 2022, 5:06 p.m. UTC | #3 
On Tue, Jun 21, 2022 at 08:00:26PM +0300, Dmitry Baryshkov wrote:
> On Tue, 21 Jun 2022 at 18:21, Christian Marangi <ansuelsmth@gmail.com> wrote:
> >
> > On Tue, Jun 21, 2022 at 12:51:50AM +0300, Dmitry Baryshkov wrote:
> > > Use parent_data for the last remaining entry (pll4). This clock is
> > > provided by the lcc device.
> > >
> > > Fixes: cb02866f9a74 ("clk: qcom: gcc-ipq806x: convert parent_names to parent_data")
> > > Cc: Ansuel Smith <ansuelsmth@gmail.com>
> > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > > ---
> > >  drivers/clk/qcom/gcc-ipq806x.c | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/clk/qcom/gcc-ipq806x.c b/drivers/clk/qcom/gcc-ipq806x.c
> > > index 718de17a1e60..6447f3e81b55 100644
> > > --- a/drivers/clk/qcom/gcc-ipq806x.c
> > > +++ b/drivers/clk/qcom/gcc-ipq806x.c
> > > @@ -79,7 +79,9 @@ static struct clk_regmap pll4_vote = {
> > >       .enable_mask = BIT(4),
> > >       .hw.init = &(struct clk_init_data){
> > >               .name = "pll4_vote",
> > > -             .parent_names = (const char *[]){ "pll4" },
> > > +             .parent_data = &(const struct clk_parent_data){
> > > +                     .fw_name = "pll4", .name = "pll4",
> > > +             },
> > >               .num_parents = 1,
> > >               .ops = &clk_pll_vote_ops,
> > >       },
> > > --
> > > 2.35.1
> > >
> >
> > Hi my intention was finding a way to directly reference the hw clk from
> > the lcc driver instead of using fw_name/name parent data. Wonder if that
> > would be a better solution... Seems wrong to me to eventually add also
> > the pll4 clk in the dts to correctly use the fw_name definition (when
> > that will be fixed in the ipq8064 dtsi)
> 
> Please don't. They are two separate hardware pieces, two separate
> drivers. Please don't invent anything fancy. Let the OF clk code
> handle it.
> Yes, this will result in "pll4" clock being referenced in DT. We
> already have binding for that, <&lcc PLL4>.
>

Oh ok. Thanks for the clarification, no problem for me. Just another clk to
add to the gcc node in the dtsi.

> -- 
> With best wishes
> Dmitry
Stephen Boyd June 24, 2022, 12:27 a.m. UTC | #4 
Quoting Dmitry Baryshkov (2022-06-20 14:51:50)
> Use parent_data for the last remaining entry (pll4). This clock is
> provided by the lcc device.
>
> Fixes: cb02866f9a74 ("clk: qcom: gcc-ipq806x: convert parent_names to parent_data")
> Cc: Ansuel Smith <ansuelsmth@gmail.com>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  drivers/clk/qcom/gcc-ipq806x.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clk/qcom/gcc-ipq806x.c b/drivers/clk/qcom/gcc-ipq806x.c
> index 718de17a1e60..6447f3e81b55 100644
> --- a/drivers/clk/qcom/gcc-ipq806x.c
> +++ b/drivers/clk/qcom/gcc-ipq806x.c
> @@ -79,7 +79,9 @@ static struct clk_regmap pll4_vote = {
>         .enable_mask = BIT(4),
>         .hw.init = &(struct clk_init_data){
>                 .name = "pll4_vote",
> -               .parent_names = (const char *[]){ "pll4" },
> +               .parent_data = &(const struct clk_parent_data){
> +                       .fw_name = "pll4", .name = "pll4",

Is there a DT binding update?
Stephen Boyd June 24, 2022, 12:37 a.m. UTC | #5 
Quoting Stephen Boyd (2022-06-23 17:27:18)
> Quoting Dmitry Baryshkov (2022-06-20 14:51:50)
> > Use parent_data for the last remaining entry (pll4). This clock is
> > provided by the lcc device.
> > 
> > Fixes: cb02866f9a74 ("clk: qcom: gcc-ipq806x: convert parent_names to parent_data")
> > Cc: Ansuel Smith <ansuelsmth@gmail.com>
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > ---
> >  drivers/clk/qcom/gcc-ipq806x.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/clk/qcom/gcc-ipq806x.c b/drivers/clk/qcom/gcc-ipq806x.c
> > index 718de17a1e60..6447f3e81b55 100644
> > --- a/drivers/clk/qcom/gcc-ipq806x.c
> > +++ b/drivers/clk/qcom/gcc-ipq806x.c
> > @@ -79,7 +79,9 @@ static struct clk_regmap pll4_vote = {
> >         .enable_mask = BIT(4),
> >         .hw.init = &(struct clk_init_data){
> >                 .name = "pll4_vote",
> > -               .parent_names = (const char *[]){ "pll4" },
> > +               .parent_data = &(const struct clk_parent_data){
> > +                       .fw_name = "pll4", .name = "pll4",
> 
> Is there a DT binding update?

Also I'd appreciate clk patches be sent to my kernel.org email instead
of chromium to help my filters.
Christian Marangi June 24, 2022, 12:42 a.m. UTC | #6 
On Thu, Jun 23, 2022 at 05:37:12PM -0700, Stephen Boyd wrote:
> Quoting Stephen Boyd (2022-06-23 17:27:18)
> > Quoting Dmitry Baryshkov (2022-06-20 14:51:50)
> > > Use parent_data for the last remaining entry (pll4). This clock is
> > > provided by the lcc device.
> > > 
> > > Fixes: cb02866f9a74 ("clk: qcom: gcc-ipq806x: convert parent_names to parent_data")
> > > Cc: Ansuel Smith <ansuelsmth@gmail.com>
> > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > > ---
> > >  drivers/clk/qcom/gcc-ipq806x.c | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/clk/qcom/gcc-ipq806x.c b/drivers/clk/qcom/gcc-ipq806x.c
> > > index 718de17a1e60..6447f3e81b55 100644
> > > --- a/drivers/clk/qcom/gcc-ipq806x.c
> > > +++ b/drivers/clk/qcom/gcc-ipq806x.c
> > > @@ -79,7 +79,9 @@ static struct clk_regmap pll4_vote = {
> > >         .enable_mask = BIT(4),
> > >         .hw.init = &(struct clk_init_data){
> > >                 .name = "pll4_vote",
> > > -               .parent_names = (const char *[]){ "pll4" },
> > > +               .parent_data = &(const struct clk_parent_data){
> > > +                       .fw_name = "pll4", .name = "pll4",
> > 
> > Is there a DT binding update?
> 
> Also I'd appreciate clk patches be sent to my kernel.org email instead
> of chromium to help my filters.

Sorry for the OT but as you mention... there are many series for ipq806x
all reviewed. Wonder if they can be picked or should I RESEND them?

Some of them are blocking me from sending other fixes as the current
dtsi have wrong definition that would cause kernel panic (if things are
correctly implemented on the driver side)
Stephen Boyd June 24, 2022, 12:49 a.m. UTC | #7 
Quoting Christian Marangi (2022-06-23 17:42:05)
> On Thu, Jun 23, 2022 at 05:37:12PM -0700, Stephen Boyd wrote:
> > 
> > Also I'd appreciate clk patches be sent to my kernel.org email instead
> > of chromium to help my filters.
> 
> Sorry for the OT but as you mention... there are many series for ipq806x
> all reviewed. Wonder if they can be picked or should I RESEND them?
> 
> Some of them are blocking me from sending other fixes as the current
> dtsi have wrong definition that would cause kernel panic (if things are
> correctly implemented on the driver side)
> 

Bjorn has agreed to pick up qcom clk patches and send them off as a pull
request to clk maintainers (i.e. me). Please don't RESEND patches unless
requested. When that happens I have to mark the old series as
"superseded" and then look at the resends. Then I review code slower and
everyone is sad.
Dmitry Baryshkov June 24, 2022, 5:23 a.m. UTC | #8 
On Fri, 24 Jun 2022 at 03:27, Stephen Boyd <swboyd@chromium.org> wrote:
>
> Quoting Dmitry Baryshkov (2022-06-20 14:51:50)
> > Use parent_data for the last remaining entry (pll4). This clock is
> > provided by the lcc device.
> >
> > Fixes: cb02866f9a74 ("clk: qcom: gcc-ipq806x: convert parent_names to parent_data")
> > Cc: Ansuel Smith <ansuelsmth@gmail.com>
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > ---
> >  drivers/clk/qcom/gcc-ipq806x.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/clk/qcom/gcc-ipq806x.c b/drivers/clk/qcom/gcc-ipq806x.c
> > index 718de17a1e60..6447f3e81b55 100644
> > --- a/drivers/clk/qcom/gcc-ipq806x.c
> > +++ b/drivers/clk/qcom/gcc-ipq806x.c
> > @@ -79,7 +79,9 @@ static struct clk_regmap pll4_vote = {
> >         .enable_mask = BIT(4),
> >         .hw.init = &(struct clk_init_data){
> >                 .name = "pll4_vote",
> > -               .parent_names = (const char *[]){ "pll4" },
> > +               .parent_data = &(const struct clk_parent_data){
> > +                       .fw_name = "pll4", .name = "pll4",
>
> Is there a DT binding update?

I will include it in v2.
Dmitry Baryshkov June 24, 2022, 5:23 a.m. UTC | #9 
On Fri, 24 Jun 2022 at 03:37, Stephen Boyd <sboyd@kernel.org> wrote:
>
> Quoting Stephen Boyd (2022-06-23 17:27:18)
> > Quoting Dmitry Baryshkov (2022-06-20 14:51:50)
> > > Use parent_data for the last remaining entry (pll4). This clock is
> > > provided by the lcc device.
> > >
> > > Fixes: cb02866f9a74 ("clk: qcom: gcc-ipq806x: convert parent_names to parent_data")
> > > Cc: Ansuel Smith <ansuelsmth@gmail.com>
> > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > > ---
> > >  drivers/clk/qcom/gcc-ipq806x.c | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/clk/qcom/gcc-ipq806x.c b/drivers/clk/qcom/gcc-ipq806x.c
> > > index 718de17a1e60..6447f3e81b55 100644
> > > --- a/drivers/clk/qcom/gcc-ipq806x.c
> > > +++ b/drivers/clk/qcom/gcc-ipq806x.c
> > > @@ -79,7 +79,9 @@ static struct clk_regmap pll4_vote = {
> > >         .enable_mask = BIT(4),
> > >         .hw.init = &(struct clk_init_data){
> > >                 .name = "pll4_vote",
> > > -               .parent_names = (const char *[]){ "pll4" },
> > > +               .parent_data = &(const struct clk_parent_data){
> > > +                       .fw_name = "pll4", .name = "pll4",
> >
> > Is there a DT binding update?
>
> Also I'd appreciate clk patches be sent to my kernel.org email instead
> of chromium to help my filters.

Ack, excuse me.
Bjorn Andersson June 27, 2022, 11:45 p.m. UTC | #10 
On Thu 23 Jun 19:42 CDT 2022, Christian Marangi wrote:

> On Thu, Jun 23, 2022 at 05:37:12PM -0700, Stephen Boyd wrote:
> > Quoting Stephen Boyd (2022-06-23 17:27:18)
> > > Quoting Dmitry Baryshkov (2022-06-20 14:51:50)
> > > > Use parent_data for the last remaining entry (pll4). This clock is
> > > > provided by the lcc device.
> > > > 
> > > > Fixes: cb02866f9a74 ("clk: qcom: gcc-ipq806x: convert parent_names to parent_data")
> > > > Cc: Ansuel Smith <ansuelsmth@gmail.com>
> > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > > > ---
> > > >  drivers/clk/qcom/gcc-ipq806x.c | 4 +++-
> > > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/clk/qcom/gcc-ipq806x.c b/drivers/clk/qcom/gcc-ipq806x.c
> > > > index 718de17a1e60..6447f3e81b55 100644
> > > > --- a/drivers/clk/qcom/gcc-ipq806x.c
> > > > +++ b/drivers/clk/qcom/gcc-ipq806x.c
> > > > @@ -79,7 +79,9 @@ static struct clk_regmap pll4_vote = {
> > > >         .enable_mask = BIT(4),
> > > >         .hw.init = &(struct clk_init_data){
> > > >                 .name = "pll4_vote",
> > > > -               .parent_names = (const char *[]){ "pll4" },
> > > > +               .parent_data = &(const struct clk_parent_data){
> > > > +                       .fw_name = "pll4", .name = "pll4",
> > > 
> > > Is there a DT binding update?
> > 
> > Also I'd appreciate clk patches be sent to my kernel.org email instead
> > of chromium to help my filters.
> 
> Sorry for the OT but as you mention... there are many series for ipq806x
> all reviewed. Wonder if they can be picked or should I RESEND them?
> 

Sorry about that, I'm hoping to catch up on the pending patches in the
coming few days...

Regards,
Bjorn

> Some of them are blocking me from sending other fixes as the current
> dtsi have wrong definition that would cause kernel panic (if things are
> correctly implemented on the driver side)
> 
> -- 
> 	Ansuel
diff mbox series

Patch

diff --git a/drivers/clk/qcom/gcc-ipq806x.c b/drivers/clk/qcom/gcc-ipq806x.c
index 718de17a1e60..6447f3e81b55 100644
--- a/drivers/clk/qcom/gcc-ipq806x.c
+++ b/drivers/clk/qcom/gcc-ipq806x.c
@@ -79,7 +79,9 @@  static struct clk_regmap pll4_vote = {
 	.enable_mask = BIT(4),
 	.hw.init = &(struct clk_init_data){
 		.name = "pll4_vote",
-		.parent_names = (const char *[]){ "pll4" },
+		.parent_data = &(const struct clk_parent_data){
+			.fw_name = "pll4", .name = "pll4",
+		},
 		.num_parents = 1,
 		.ops = &clk_pll_vote_ops,
 	},