| Message ID | 1719996771-11220-1-git-send-email-shengjiu.wang@nxp.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | clk: imx: imx8: Add .name for "acm_aud_clk0_sel" and "acm_aud_clk1_sel" | expand |
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?
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
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.
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
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[] = {
"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(-)