Hi Stephen,

Sorry for the delay due to a horrible busy months. Just a bit relax now.

On Tue, Sep 17, 2019 at 2:44 AM Stephen Boyd [off-list ref] wrote:

Quoting Dong Aisheng (2019-09-09 03:23:25)

quoted

Hi Stephen,

Thanks for the review.

On Sat, Sep 7, 2019 at 5:29 PM Stephen Boyd [off-list ref] wrote:

quoted

Quoting Dong Aisheng (2019-08-20 04:13:17)

quoted

diff --git a/drivers/clk/imx/clk-imx8qxp.c b/drivers/clk/imx/clk-imx8qxp.c
index 5e2903e..1ad3f2a 100644
--- a/drivers/clk/imx/clk-imx8qxp.c
+++ b/drivers/clk/imx/clk-imx8qxp.c

@@ -134,7 +134,12 @@ static int imx8qxp_clk_probe(struct platform_device *pdev)
                                i, PTR_ERR(clks[i]));
        }

-       return of_clk_add_hw_provider(ccm_node, of_clk_hw_onecell_get, clk_data);
+       if (clock_cells == 2)

Can you just read this from the DT node again instead of having a global
variable called "clock_cells" for this?

I tried thinking about it.
One problem is that we also need this information in the exist clk
registration API to
keep the backwards compatibility:
e.g.
 static inline struct clk_hw *imx_clk_scu(const char *name, u32 rsrc_id,
                                         u8 clk_type)
 {
-       return __imx_clk_scu(name, NULL, 0, rsrc_id, clk_type);
+       if (clock_cells == 2)
+               return imx_clk_scu_alloc_dev(name, NULL, 0, rsrc_id, clk_type);
+       else
+               return __imx_clk_scu(name, NULL, 0, rsrc_id, clk_type);
 }

Parsing it for all clocks seems not good.

Can you parse it once for the clock controller and then pass it to the
registration function as the number of cells? I dislike the global and
the name of the global.

Yes, i can do it.
Why i didn't do it before is because there're tens of APIs callers already
and finally we will back to the original API again after removing the
legacy users.
So i used a global variable as a temporarily workaround during transition phase.
But i do agree that make the code look ugly.

Regards
Aisheng

quoted

In the future, i planned to totally remove the legacy binding support which
is a premature one and missing continued support.
Then we will also remove this unneeded clock_cells.

Ok sure.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel