From: Stephen Boyd <sboyd@kernel.org> To: Dong Aisheng <dongas86@gmail.com> Cc: Dong Aisheng <aisheng.dong@nxp.com>, linux-clk <linux-clk@vger.kernel.org>, "moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE" <linux-arm-kernel@lists.infradead.org>, Michael Turquette <mturquette@baylibre.com>, Shawn Guo <shawnguo@kernel.org>, Fabio Estevam <fabio.estevam@nxp.com>, dl-linux-imx <linux-imx@nxp.com>, Sascha Hauer <kernel@pengutronix.de> Subject: Re: [PATCH V4 03/11] clk: imx: scu: add two cells binding support Date: Mon, 16 Sep 2019 11:44:07 -0700 [thread overview] Message-ID: <20190916184408.8A55720665@mail.kernel.org> (raw) In-Reply-To: <CAA+hA=QoZFFb_EVfxcDuJB-9VobVd-1-RyhWeNTSePxW50P8Eg@mail.gmail.com> Quoting Dong Aisheng (2019-09-09 03:23:25) > Hi Stephen, > > Thanks for the review. > > On Sat, Sep 7, 2019 at 5:29 PM Stephen Boyd <sboyd@kernel.org> wrote: > > > > Quoting Dong Aisheng (2019-08-20 04:13:17) > > > 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. > > 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.
WARNING: multiple messages have this Message-ID (diff)
From: Stephen Boyd <sboyd@kernel.org> To: Dong Aisheng <dongas86@gmail.com> Cc: Dong Aisheng <aisheng.dong@nxp.com>, Michael Turquette <mturquette@baylibre.com>, dl-linux-imx <linux-imx@nxp.com>, Sascha Hauer <kernel@pengutronix.de>, Fabio Estevam <fabio.estevam@nxp.com>, Shawn Guo <shawnguo@kernel.org>, linux-clk <linux-clk@vger.kernel.org>, "moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE" <linux-arm-kernel@lists.infradead.org> Subject: Re: [PATCH V4 03/11] clk: imx: scu: add two cells binding support Date: Mon, 16 Sep 2019 11:44:07 -0700 [thread overview] Message-ID: <20190916184408.8A55720665@mail.kernel.org> (raw) In-Reply-To: <CAA+hA=QoZFFb_EVfxcDuJB-9VobVd-1-RyhWeNTSePxW50P8Eg@mail.gmail.com> Quoting Dong Aisheng (2019-09-09 03:23:25) > Hi Stephen, > > Thanks for the review. > > On Sat, Sep 7, 2019 at 5:29 PM Stephen Boyd <sboyd@kernel.org> wrote: > > > > Quoting Dong Aisheng (2019-08-20 04:13:17) > > > 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. > > 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
next prev parent reply other threads:[~2019-09-16 18:44 UTC|newest] Thread overview: 83+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-08-20 11:13 [PATCH V4 00/11] clk: imx8: add new clock binding for better pm support Dong Aisheng 2019-08-20 11:13 ` Dong Aisheng 2019-08-20 11:13 ` [PATCH V4 01/11] dt-bindings: firmware: imx-scu: new binding to parse clocks from device tree Dong Aisheng 2019-08-20 11:13 ` Dong Aisheng 2019-08-20 11:13 ` Dong Aisheng 2019-08-24 19:19 ` Shawn Guo 2019-08-24 19:19 ` Shawn Guo 2019-08-24 19:19 ` Shawn Guo 2019-08-26 3:24 ` Aisheng Dong 2019-08-26 3:24 ` Aisheng Dong 2019-08-26 3:24 ` Aisheng Dong 2019-08-27 17:04 ` Rob Herring 2019-08-27 17:04 ` Rob Herring 2019-08-27 17:04 ` Rob Herring 2019-09-06 16:56 ` Stephen Boyd 2019-09-06 16:56 ` Stephen Boyd 2019-09-06 16:56 ` Stephen Boyd 2019-08-20 11:13 ` [PATCH V4 02/11] dt-bindings: clock: imx-lpcg: add support " Dong Aisheng 2019-08-20 11:13 ` Dong Aisheng 2019-08-20 11:13 ` Dong Aisheng 2019-08-24 19:21 ` Shawn Guo 2019-08-24 19:21 ` Shawn Guo 2019-08-24 19:21 ` Shawn Guo 2019-08-26 3:14 ` Aisheng Dong 2019-08-26 3:14 ` Aisheng Dong 2019-08-26 3:14 ` Aisheng Dong 2019-08-26 3:21 ` Aisheng Dong 2019-08-26 3:21 ` Aisheng Dong 2019-08-26 3:21 ` Aisheng Dong 2019-08-27 17:05 ` Rob Herring 2019-08-27 17:05 ` Rob Herring 2019-08-27 17:05 ` Rob Herring 2019-09-06 17:00 ` Stephen Boyd 2019-09-06 17:00 ` Stephen Boyd 2019-09-06 17:00 ` Stephen Boyd 2019-08-20 11:13 ` [PATCH V4 03/11] clk: imx: scu: add two cells binding support Dong Aisheng 2019-08-20 11:13 ` Dong Aisheng 2019-09-06 17:06 ` Stephen Boyd 2019-09-06 17:06 ` Stephen Boyd 2019-09-09 10:23 ` Dong Aisheng 2019-09-09 10:23 ` Dong Aisheng 2019-09-16 18:44 ` Stephen Boyd [this message] 2019-09-16 18:44 ` Stephen Boyd 2019-11-17 12:07 ` Dong Aisheng 2019-11-17 12:07 ` Dong Aisheng 2019-08-20 11:13 ` [PATCH V4 04/11] clk: imx: scu: bypass cpu power domains Dong Aisheng 2019-08-20 11:13 ` Dong Aisheng 2019-09-06 17:07 ` Stephen Boyd 2019-09-06 17:07 ` Stephen Boyd 2019-09-09 10:24 ` Dong Aisheng 2019-09-09 10:24 ` Dong Aisheng 2019-08-20 11:13 ` [PATCH V4 05/11] clk: imx: scu: allow scu clk to take device pointer Dong Aisheng 2019-08-20 11:13 ` Dong Aisheng 2019-08-20 11:13 ` [PATCH V4 06/11] clk: imx: scu: add runtime pm support Dong Aisheng 2019-08-20 11:13 ` Dong Aisheng 2019-08-20 11:13 ` [PATCH V4 07/11] clk: imx: scu: add suspend/resume support Dong Aisheng 2019-08-20 11:13 ` Dong Aisheng 2019-09-06 17:09 ` Stephen Boyd 2019-09-06 17:09 ` Stephen Boyd 2019-09-09 10:35 ` Dong Aisheng 2019-09-09 10:35 ` Dong Aisheng 2019-08-20 11:13 ` [PATCH V4 08/11] clk: imx: imx8qxp-lpcg: add parsing clocks from device tree Dong Aisheng 2019-08-20 11:13 ` Dong Aisheng 2019-09-06 17:13 ` Stephen Boyd 2019-09-06 17:13 ` Stephen Boyd 2019-09-09 11:23 ` Dong Aisheng 2019-09-09 11:23 ` Dong Aisheng 2019-09-16 18:45 ` Stephen Boyd 2019-09-16 18:45 ` Stephen Boyd 2019-11-17 12:08 ` Dong Aisheng 2019-11-17 12:08 ` Dong Aisheng 2019-08-20 11:13 ` [PATCH V4 09/11] clk: imx: lpcg: allow lpcg clk to take device pointer Dong Aisheng 2019-08-20 11:13 ` Dong Aisheng 2019-08-20 11:13 ` [PATCH V4 10/11] clk: imx: clk-imx8qxp-lpcg: add runtime pm support Dong Aisheng 2019-08-20 11:13 ` Dong Aisheng 2019-08-20 11:13 ` [PATCH V4 11/11] clk: imx: lpcg: add suspend/resume support Dong Aisheng 2019-08-20 11:13 ` Dong Aisheng 2019-09-06 17:14 ` Stephen Boyd 2019-09-06 17:14 ` Stephen Boyd 2019-09-09 11:39 ` Dong Aisheng 2019-09-09 11:39 ` Dong Aisheng 2019-09-09 12:21 ` [PATCH V4 00/11] clk: imx8: add new clock binding for better pm support Oliver Graute 2019-09-09 12:21 ` Oliver Graute
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20190916184408.8A55720665@mail.kernel.org \ --to=sboyd@kernel.org \ --cc=aisheng.dong@nxp.com \ --cc=dongas86@gmail.com \ --cc=fabio.estevam@nxp.com \ --cc=kernel@pengutronix.de \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-clk@vger.kernel.org \ --cc=linux-imx@nxp.com \ --cc=mturquette@baylibre.com \ --cc=shawnguo@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.