| Message ID | 1584279836-29825-13-git-send-email-aisheng.dong@nxp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | clk: imx8: add new clock binding for better pm support | expand |
Quoting Dong Aisheng (2020-03-15 06:43:56) > Unregister clocks if add provider failed > > Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com> > > --- Why isn't this squashed in to where it's needed? > ChangeLog: > v6: new patch > --- > drivers/clk/imx/clk-imx8qxp.c | 11 +++++++++-- > drivers/clk/imx/clk-scu.c | 13 +++++++++++++ > drivers/clk/imx/clk-scu.h | 2 ++ > 3 files changed, 24 insertions(+), 2 deletions(-) > > diff --git a/drivers/clk/imx/clk-imx8qxp.c b/drivers/clk/imx/clk-imx8qxp.c > index 2ec3e0c4749d..e615214495c0 100644 > --- a/drivers/clk/imx/clk-imx8qxp.c > +++ b/drivers/clk/imx/clk-imx8qxp.c > @@ -138,10 +138,17 @@ static int imx8qxp_clk_probe(struct platform_device *pdev) > i, PTR_ERR(clks[i])); > } > > - if (clk_cells == 2) > + if (clk_cells == 2) { > ret = of_clk_add_hw_provider(ccm_node, imx_scu_of_clk_src_get, imx_scu_clks); > - else > + if (ret) > + imx_clk_scu_unregister(); > + } else { > + /* > + * NOTE: we did not unregister clocks for the legacy way cause > + * it will be removed later. I got confused what 'it' was. I think it's the legacy way entirely. Maybe say "legacy binding code path doesn't unregister here because..." > + */ > ret = of_clk_add_hw_provider(ccm_node, of_clk_hw_onecell_get, clk_data); > + } > > return ret; > }
> From: Stephen Boyd <sboyd@kernel.org> > Sent: Tuesday, May 5, 2020 12:56 PM > > Quoting Dong Aisheng (2020-03-15 06:43:56) > > Unregister clocks if add provider failed > > > > Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com> > > > > --- > > Why isn't this squashed in to where it's needed? There were two reasons: 1. The original code also has the issue, so I thought maybe It could be an extra fix patch. 2. It saved some rebase conflicts. But anyway, if you'd like to see it was squashed, I can do it in next version. > > > ChangeLog: > > v6: new patch > > --- > > drivers/clk/imx/clk-imx8qxp.c | 11 +++++++++-- > > drivers/clk/imx/clk-scu.c | 13 +++++++++++++ > > drivers/clk/imx/clk-scu.h | 2 ++ > > 3 files changed, 24 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/clk/imx/clk-imx8qxp.c > > b/drivers/clk/imx/clk-imx8qxp.c index 2ec3e0c4749d..e615214495c0 > > 100644 > > --- a/drivers/clk/imx/clk-imx8qxp.c > > +++ b/drivers/clk/imx/clk-imx8qxp.c > > @@ -138,10 +138,17 @@ static int imx8qxp_clk_probe(struct > platform_device *pdev) > > i, PTR_ERR(clks[i])); > > } > > > > - if (clk_cells == 2) > > + if (clk_cells == 2) { > > ret = of_clk_add_hw_provider(ccm_node, > imx_scu_of_clk_src_get, imx_scu_clks); > > - else > > + if (ret) > > + imx_clk_scu_unregister(); > > + } else { > > + /* > > + * NOTE: we did not unregister clocks for the legacy way > cause > > + * it will be removed later. > > I got confused what 'it' was. I think it's the legacy way entirely. > Maybe say "legacy binding code path doesn't unregister here because..." Thanks for the suggestion. I will change to use your version
diff --git a/drivers/clk/imx/clk-imx8qxp.c b/drivers/clk/imx/clk-imx8qxp.c index 2ec3e0c4749d..e615214495c0 100644 --- a/drivers/clk/imx/clk-imx8qxp.c +++ b/drivers/clk/imx/clk-imx8qxp.c @@ -138,10 +138,17 @@ static int imx8qxp_clk_probe(struct platform_device *pdev) i, PTR_ERR(clks[i])); } - if (clk_cells == 2) + if (clk_cells == 2) { ret = of_clk_add_hw_provider(ccm_node, imx_scu_of_clk_src_get, imx_scu_clks); - else + if (ret) + imx_clk_scu_unregister(); + } else { + /* + * NOTE: we did not unregister clocks for the legacy way cause + * it will be removed later. + */ ret = of_clk_add_hw_provider(ccm_node, of_clk_hw_onecell_get, clk_data); + } return ret; } diff --git a/drivers/clk/imx/clk-scu.c b/drivers/clk/imx/clk-scu.c index 7b910922aecf..1933cf90ae30 100644 --- a/drivers/clk/imx/clk-scu.c +++ b/drivers/clk/imx/clk-scu.c @@ -595,3 +595,16 @@ struct clk_hw *imx_clk_scu_alloc_dev(const char *name, /* For API backwards compatiblilty, simply return NULL for success */ return NULL; } + +void imx_clk_scu_unregister(void) +{ + struct imx_scu_clk_node *clk; + int i; + + for (i = 0; i < IMX_SC_R_LAST; i++) { + list_for_each_entry(clk, &imx_scu_clks[i], node) { + clk_hw_unregister(clk->hw); + kfree(clk); + } + } +} diff --git a/drivers/clk/imx/clk-scu.h b/drivers/clk/imx/clk-scu.h index b1dfdaf0734e..e8352164923e 100644 --- a/drivers/clk/imx/clk-scu.h +++ b/drivers/clk/imx/clk-scu.h @@ -24,6 +24,8 @@ struct clk_hw *__imx_clk_scu(struct device *dev, const char *name, const char * const *parents, int num_parents, u32 rsrc_id, u8 clk_type); +void imx_clk_scu_unregister(void); + struct clk_hw *__imx_clk_lpcg_scu(struct device *dev, const char *name, const char *parent_name, unsigned long flags, void __iomem *reg, u8 bit_idx, bool hw_gate);
Unregister clocks if add provider failed Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com> --- ChangeLog: v6: new patch --- drivers/clk/imx/clk-imx8qxp.c | 11 +++++++++-- drivers/clk/imx/clk-scu.c | 13 +++++++++++++ drivers/clk/imx/clk-scu.h | 2 ++ 3 files changed, 24 insertions(+), 2 deletions(-)