clk: imx: imx8: Add .name for "acm_aud_clk0_sel" and "acm_aud_clk1_sel"

[PATCH] clk: imx: imx8: Add .name for "acm_aud_clk0_sel" and "acm_aud_clk1_sel"

Posted by Shengjiu Wang 1 year, 5 months ago

"acm_aud_clk0_sel" and "acm_aud_clk1_sel" are registered by this ACM
driver, but they are the parent clocks for other clocks, in order to
use assigned-clock-parents in device tree, they need to have the
global name.

Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
 drivers/clk/imx/clk-imx8-acm.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/clk/imx/clk-imx8-acm.c b/drivers/clk/imx/clk-imx8-acm.c
index 1bdb480cc96c..a1affcf6daff 100644
--- a/drivers/clk/imx/clk-imx8-acm.c
+++ b/drivers/clk/imx/clk-imx8-acm.c
@@ -114,8 +114,8 @@ static const struct clk_parent_data imx8qm_mclk_out_sels[] = {
 static const struct clk_parent_data imx8qm_mclk_sels[] = {
 	{ .fw_name = "aud_pll_div_clk0_lpcg_clk" },
 	{ .fw_name = "aud_pll_div_clk1_lpcg_clk" },
-	{ .fw_name = "acm_aud_clk0_sel" },
-	{ .fw_name = "acm_aud_clk1_sel" },
+	{ .fw_name = "acm_aud_clk0_sel", .name = "acm_aud_clk0_sel" },
+	{ .fw_name = "acm_aud_clk1_sel", .name = "acm_aud_clk1_sel" },
 };
 
 static const struct clk_parent_data imx8qm_asrc_mux_clk_sels[] = {
@@ -179,8 +179,8 @@ static const struct clk_parent_data imx8qxp_mclk_out_sels[] = {
 static const struct clk_parent_data imx8qxp_mclk_sels[] = {
 	{ .fw_name = "aud_pll_div_clk0_lpcg_clk" },
 	{ .fw_name = "aud_pll_div_clk1_lpcg_clk" },
-	{ .fw_name = "acm_aud_clk0_sel" },
-	{ .fw_name = "acm_aud_clk1_sel" },
+	{ .fw_name = "acm_aud_clk0_sel", .name = "acm_aud_clk0_sel" },
+	{ .fw_name = "acm_aud_clk1_sel", .name = "acm_aud_clk1_sel" },
 };
 
 static struct clk_imx8_acm_sel imx8qxp_sels[] = {
@@ -231,8 +231,8 @@ static const struct clk_parent_data imx8dxl_mclk_out_sels[] = {
 static const struct clk_parent_data imx8dxl_mclk_sels[] = {
 	{ .fw_name = "aud_pll_div_clk0_lpcg_clk" },
 	{ .fw_name = "aud_pll_div_clk1_lpcg_clk" },
-	{ .fw_name = "acm_aud_clk0_sel" },
-	{ .fw_name = "acm_aud_clk1_sel" },
+	{ .fw_name = "acm_aud_clk0_sel", .name = "acm_aud_clk0_sel" },
+	{ .fw_name = "acm_aud_clk1_sel", .name = "acm_aud_clk1_sel" },
 };
 
 static struct clk_imx8_acm_sel imx8dxl_sels[] = {
-- 
2.34.1

Re: [PATCH] clk: imx: imx8: Add .name for "acm_aud_clk0_sel" and "acm_aud_clk1_sel"

Posted by Stephen Boyd 1 year, 5 months ago

Quoting Shengjiu Wang (2024-07-03 01:52:51)
> "acm_aud_clk0_sel" and "acm_aud_clk1_sel" are registered by this ACM
> driver, but they are the parent clocks for other clocks, in order to
> use assigned-clock-parents in device tree, they need to have the
> global name.
> 
> Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> ---
>  drivers/clk/imx/clk-imx8-acm.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/clk/imx/clk-imx8-acm.c b/drivers/clk/imx/clk-imx8-acm.c
> index 1bdb480cc96c..a1affcf6daff 100644
> --- a/drivers/clk/imx/clk-imx8-acm.c
> +++ b/drivers/clk/imx/clk-imx8-acm.c
> @@ -114,8 +114,8 @@ static const struct clk_parent_data imx8qm_mclk_out_sels[] = {
>  static const struct clk_parent_data imx8qm_mclk_sels[] = {
>         { .fw_name = "aud_pll_div_clk0_lpcg_clk" },
>         { .fw_name = "aud_pll_div_clk1_lpcg_clk" },
> -       { .fw_name = "acm_aud_clk0_sel" },
> -       { .fw_name = "acm_aud_clk1_sel" },
> +       { .fw_name = "acm_aud_clk0_sel", .name = "acm_aud_clk0_sel" },
> +       { .fw_name = "acm_aud_clk1_sel", .name = "acm_aud_clk1_sel" },

This doesn't make any sense. Why are we adding fallback names?  Is
"acm_aud_clk0_sel" not part of the DT binding for this clk controller?

Re: [PATCH] clk: imx: imx8: Add .name for "acm_aud_clk0_sel" and "acm_aud_clk1_sel"

Posted by Shengjiu Wang 1 year, 5 months ago

On Tue, Jul 9, 2024 at 6:45 AM Stephen Boyd <sboyd@kernel.org> wrote:
>
> Quoting Shengjiu Wang (2024-07-03 01:52:51)
> > "acm_aud_clk0_sel" and "acm_aud_clk1_sel" are registered by this ACM
> > driver, but they are the parent clocks for other clocks, in order to
> > use assigned-clock-parents in device tree, they need to have the
> > global name.
> >
> > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> > ---
> >  drivers/clk/imx/clk-imx8-acm.c | 12 ++++++------
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/clk/imx/clk-imx8-acm.c b/drivers/clk/imx/clk-imx8-acm.c
> > index 1bdb480cc96c..a1affcf6daff 100644
> > --- a/drivers/clk/imx/clk-imx8-acm.c
> > +++ b/drivers/clk/imx/clk-imx8-acm.c
> > @@ -114,8 +114,8 @@ static const struct clk_parent_data imx8qm_mclk_out_sels[] = {
> >  static const struct clk_parent_data imx8qm_mclk_sels[] = {
> >         { .fw_name = "aud_pll_div_clk0_lpcg_clk" },
> >         { .fw_name = "aud_pll_div_clk1_lpcg_clk" },
> > -       { .fw_name = "acm_aud_clk0_sel" },
> > -       { .fw_name = "acm_aud_clk1_sel" },
> > +       { .fw_name = "acm_aud_clk0_sel", .name = "acm_aud_clk0_sel" },
> > +       { .fw_name = "acm_aud_clk1_sel", .name = "acm_aud_clk1_sel" },
>
> This doesn't make any sense. Why are we adding fallback names?  Is
> "acm_aud_clk0_sel" not part of the DT binding for this clk controller?

It is not part of DT binding for this clk controller.  it is registered by this
clk controller itself.  As it is a parent clock, so my understanding
is that we need to add a fallback name,  or change "fw_name" to "name",
please correct me if I am wrong.

Best regards
Shengjiu Wang

Re: [PATCH] clk: imx: imx8: Add .name for "acm_aud_clk0_sel" and "acm_aud_clk1_sel"

Posted by Stephen Boyd 1 year, 5 months ago

Quoting Shengjiu Wang (2024-07-08 20:20:56)
> On Tue, Jul 9, 2024 at 6:45 AM Stephen Boyd <sboyd@kernel.org> wrote:
> >
> > Quoting Shengjiu Wang (2024-07-03 01:52:51)
> > > "acm_aud_clk0_sel" and "acm_aud_clk1_sel" are registered by this ACM
> > > driver, but they are the parent clocks for other clocks, in order to
> > > use assigned-clock-parents in device tree, they need to have the
> > > global name.
> > >
> > > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> > > ---
> > >  drivers/clk/imx/clk-imx8-acm.c | 12 ++++++------
> > >  1 file changed, 6 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/drivers/clk/imx/clk-imx8-acm.c b/drivers/clk/imx/clk-imx8-acm.c
> > > index 1bdb480cc96c..a1affcf6daff 100644
> > > --- a/drivers/clk/imx/clk-imx8-acm.c
> > > +++ b/drivers/clk/imx/clk-imx8-acm.c
> > > @@ -114,8 +114,8 @@ static const struct clk_parent_data imx8qm_mclk_out_sels[] = {
> > >  static const struct clk_parent_data imx8qm_mclk_sels[] = {
> > >         { .fw_name = "aud_pll_div_clk0_lpcg_clk" },
> > >         { .fw_name = "aud_pll_div_clk1_lpcg_clk" },
> > > -       { .fw_name = "acm_aud_clk0_sel" },
> > > -       { .fw_name = "acm_aud_clk1_sel" },
> > > +       { .fw_name = "acm_aud_clk0_sel", .name = "acm_aud_clk0_sel" },
> > > +       { .fw_name = "acm_aud_clk1_sel", .name = "acm_aud_clk1_sel" },
> >
> > This doesn't make any sense. Why are we adding fallback names?  Is
> > "acm_aud_clk0_sel" not part of the DT binding for this clk controller?
> 
> It is not part of DT binding for this clk controller.  it is registered by this
> clk controller itself.  As it is a parent clock, so my understanding
> is that we need to add a fallback name,  or change "fw_name" to "name",
> please correct me if I am wrong.

If it's registered by this clk controller itself then it should be a
clk_hw pointer and not use any string name.

Re: [PATCH] clk: imx: imx8: Add .name for "acm_aud_clk0_sel" and "acm_aud_clk1_sel"

Posted by Shengjiu Wang 1 year, 5 months ago

On Wed, Jul 10, 2024 at 4:08 AM Stephen Boyd <sboyd@kernel.org> wrote:
>
> Quoting Shengjiu Wang (2024-07-08 20:20:56)
> > On Tue, Jul 9, 2024 at 6:45 AM Stephen Boyd <sboyd@kernel.org> wrote:
> > >
> > > Quoting Shengjiu Wang (2024-07-03 01:52:51)
> > > > "acm_aud_clk0_sel" and "acm_aud_clk1_sel" are registered by this ACM
> > > > driver, but they are the parent clocks for other clocks, in order to
> > > > use assigned-clock-parents in device tree, they need to have the
> > > > global name.
> > > >
> > > > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> > > > ---
> > > >  drivers/clk/imx/clk-imx8-acm.c | 12 ++++++------
> > > >  1 file changed, 6 insertions(+), 6 deletions(-)
> > > >
> > > > diff --git a/drivers/clk/imx/clk-imx8-acm.c b/drivers/clk/imx/clk-imx8-acm.c
> > > > index 1bdb480cc96c..a1affcf6daff 100644
> > > > --- a/drivers/clk/imx/clk-imx8-acm.c
> > > > +++ b/drivers/clk/imx/clk-imx8-acm.c
> > > > @@ -114,8 +114,8 @@ static const struct clk_parent_data imx8qm_mclk_out_sels[] = {
> > > >  static const struct clk_parent_data imx8qm_mclk_sels[] = {
> > > >         { .fw_name = "aud_pll_div_clk0_lpcg_clk" },
> > > >         { .fw_name = "aud_pll_div_clk1_lpcg_clk" },
> > > > -       { .fw_name = "acm_aud_clk0_sel" },
> > > > -       { .fw_name = "acm_aud_clk1_sel" },
> > > > +       { .fw_name = "acm_aud_clk0_sel", .name = "acm_aud_clk0_sel" },
> > > > +       { .fw_name = "acm_aud_clk1_sel", .name = "acm_aud_clk1_sel" },
> > >
> > > This doesn't make any sense. Why are we adding fallback names?  Is
> > > "acm_aud_clk0_sel" not part of the DT binding for this clk controller?
> >
> > It is not part of DT binding for this clk controller.  it is registered by this
> > clk controller itself.  As it is a parent clock, so my understanding
> > is that we need to add a fallback name,  or change "fw_name" to "name",
> > please correct me if I am wrong.
>
> If it's registered by this clk controller itself then it should be a
> clk_hw pointer and not use any string name.

ok, will update it.

Best regards
Shengjiu Wang