From: Daniel Baluta <daniel.baluta@nxp.com>
To: "dongas86@gmail.com" <dongas86@gmail.com>,
"shawnguo@kernel.org" <shawnguo@kernel.org>
Cc: dl-linux-imx <linux-imx@nxp.com>,
Aisheng Dong <aisheng.dong@nxp.com>,
"robh+dt@kernel.org" <robh+dt@kernel.org>,
"mturquette@baylibre.com" <mturquette@baylibre.com>,
"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
Fabio Estevam <fabio.estevam@nxp.com>,
"sboyd@kernel.org" <sboyd@kernel.org>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
"linux-clk@vger.kernel.org" <linux-clk@vger.kernel.org>,
Daniel Baluta <daniel.baluta@nxp.com>,
"kernel@pengutronix.de" <kernel@pengutronix.de>
Subject: Re: [PATCH v3 01/11] dt-bindings: firmware: imx-scu: new binding to parse clocks from device tree
Date: Mon, 19 Aug 2019 12:36:36 +0000 [thread overview]
Message-ID: <caab750569dc246c99e88bbe6d9ca137ae7f198f.camel@nxp.com> (raw)
In-Reply-To: <CAA+hA=Sm0MAHNwH1sZQfK8cO+3jLkue97u=ceFiUv34+qGos1Q@mail.gmail.com>
On Mon, 2019-08-05 at 11:48 +0800, Dong Aisheng wrote:
> On Sun, Aug 4, 2019 at 11:49 AM Shawn Guo <shawnguo@kernel.org>
> wrote:
> >
> > On Tue, Jul 16, 2019 at 11:00:55PM +0800, Dong Aisheng wrote:
> > > There's a few limitations on the original one cell clock binding
> > > (#clock-cells = <1>) that we have to define some SW clock IDs for
> > > device
> > > tree to reference. This may cause troubles if we want to use
> > > common
> > > clock IDs for multi platforms support when the clock of those
> > > platforms
> > > are mostly the same.
> > > e.g. Current clock IDs name are defined with SS prefix.
> > >
> > > However the device may reside in different SS across CPUs, that
> > > means the
> > > SS prefix may not valid anymore for a new SoC. Furthermore, the
> > > device
> > > availability of those clocks may also vary a bit.
> > >
> > > For such situation, we want to eliminate the using of SW Clock
> > > IDs and
> > > change to use a more close to HW one instead.
> > > For SCU clocks usage, only two params required: Resource id +
> > > Clock Type.
> >
> > If this is how SCU firmware addresses the clock, I agree that it's
> > worth
> > witching to this new bindings, which describes the hardware (SCU
> > firmware in this case) better, IMO.
> >
> > > Both parameters are platform independent. So we could use two
> > > cells binding
> > > to pass those parameters,
> > >
> > > Cc: Rob Herring <robh+dt@kernel.org>
> > > Cc: Stephen Boyd <sboyd@kernel.org>
> > > Cc: Shawn Guo <shawnguo@kernel.org>
> > > Cc: Sascha Hauer <kernel@pengutronix.de>
> > > Cc: Michael Turquette <mturquette@baylibre.com>
> > > Cc: devicetree@vger.kernel.org
> > > Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> > > ---
> > > ChangeLog:
> > > v2->v3:
> > > * Changed to two cells binding and register all clocks in driver
> > > instead of parse from device tree.
> > > v1->v2:
> > > * changed to one cell binding inspired by arm,scpi.txt
> > > Documentation/devicetree/bindings/arm/arm,scpi.txt
> > > Resource ID is encoded in 'reg' property.
> > > Clock type is encoded in generic clock-indices property.
> > > Then we don't have to search all the DT nodes to fetch
> > > those two value to construct clocks which is relatively
> > > low efficiency.
> > > * Add required power-domain property as well.
> > > ---
> > > .../devicetree/bindings/arm/freescale/fsl,scu.txt | 12
> > > +++++++-----
> > > include/dt-bindings/firmware/imx/rsrc.h | 17
> > > +++++++++++++++++
> > > 2 files changed, 24 insertions(+), 5 deletions(-)
> > >
> > > diff --git
> > > a/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
> > > b/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
> > > index 5d7dbab..351d335 100644
> > > --- a/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
> > > +++ b/Documentation/devicetree/bindings/arm/freescale/fsl,scu.txt
> > > @@ -89,7 +89,10 @@ Required properties:
> > > "fsl,imx8qm-clock"
> > > "fsl,imx8qxp-clock"
> > > followed by "fsl,scu-clk"
> > > -- #clock-cells: Should be 1. Contains the Clock ID
> > > value.
> > > +- #clock-cells: Should be either
> > > + 2: Contains the Resource and Clock ID
> > > value.
> > > + or
> > > + 1: Contains the Clock ID value.
> > > (DEPRECATED)
> > > - clocks: List of clock specifiers, must contain an
> > > entry for
> > > each required entry in clock-names
> > > - clock-names: Should include entries
> > > "xtal_32KHz", "xtal_24MHz"
> > > @@ -162,7 +165,7 @@ firmware {
> > >
> > > clk: clk {
> > > compatible = "fsl,imx8qxp-clk", "fsl,scu-
> > > clk";
> > > - #clock-cells = <1>;
> > > + #clock-cells = <2>;
> > > };
> > >
> > > iomuxc {
> > > @@ -192,8 +195,7 @@ serial@5a060000 {
> > > ...
> > > pinctrl-names = "default";
> > > pinctrl-0 = <&pinctrl_lpuart0>;
> > > - clocks = <&clk IMX8QXP_UART0_CLK>,
> > > - <&clk IMX8QXP_UART0_IPG_CLK>;
> > > - clock-names = "per", "ipg";
> > > + clocks = <&uart0_clk IMX_SC_R_UART_0 IMX_SC_PM_CLK_PER>;
> > > + clock-names = "ipg";
> > > power-domains = <&pd IMX_SC_R_UART_0>;
> > > };
> > > diff --git a/include/dt-bindings/firmware/imx/rsrc.h
> > > b/include/dt-bindings/firmware/imx/rsrc.h
> > > index 4e61f64..fbeaca7 100644
> > > --- a/include/dt-bindings/firmware/imx/rsrc.h
> > > +++ b/include/dt-bindings/firmware/imx/rsrc.h
> > > @@ -547,4 +547,21 @@
> > > #define IMX_SC_R_ATTESTATION 545
> > > #define IMX_SC_R_LAST 546
> > >
> > > +/*
> > > + * Defines for SC PM CLK
> > > + */
> > > +#define IMX_SC_PM_CLK_SLV_BUS 0 /* Slave
> > > bus clock */
> > > +#define IMX_SC_PM_CLK_MST_BUS 1 /* Master
> > > bus clock */
> > > +#define IMX_SC_PM_CLK_PER 2 /* Peripheral clock
> > > */
> > > +#define IMX_SC_PM_CLK_PHY 3 /* Phy clock */
> > > +#define IMX_SC_PM_CLK_MISC 4 /* Misc clock */
>
> This is for typical device resource.
>
> > > +#define IMX_SC_PM_CLK_MISC0 0 /* Misc 0 clock */
> > > +#define IMX_SC_PM_CLK_MISC1 1 /* Misc 1 clock */
> > > +#define IMX_SC_PM_CLK_MISC2 2 /* Misc 2 clock */
> > > +#define IMX_SC_PM_CLK_MISC3 3 /* Misc 3 clock */
> > > +#define IMX_SC_PM_CLK_MISC4 4 /* Misc 4 clock */
>
> This is for some special clock types which do not belong to above
> normal clock types.
> Used very rare in SCU firmware.
> e.g.
> enet0_mac0_rxclk SC_R_ENE T_0 / SC_PM_CL K_MISC0
>
> > > +#define IMX_SC_PM_CLK_CPU 2 /* CPU clock */
> > > +#define IMX_SC_PM_CLK_PLL 4 /* PLL */
> > > +#define IMX_SC_PM_CLK_BYPASS 4 /* Bypass clock */
>
> They're for specific clock types for CPU/PLL/BYPASS only.
Hi Aisheng,
Yes, please separate this types of clocks in their own sections with
proper description.
>
> >
> > It seems that there are several sets of clock type which apply to
> > different resources/devices? If so, can you separate them a bit
> > with
> > some comments to make the list easier for readers?
> >
>
>
So, please send v4 with all comments fixed. I can help with testing.
I am also intrested in seeing this get in!
thanks,
Daniel.
next prev parent reply other threads:[~2019-08-19 12:36 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-16 15:00 [PATCH v3 00/11] clk: imx8: add new clock binding for better pm support Dong Aisheng
2019-07-16 15:00 ` [PATCH v3 01/11] dt-bindings: firmware: imx-scu: new binding to parse clocks from device tree Dong Aisheng
2019-08-03 14:42 ` Shawn Guo
2019-08-05 3:48 ` Dong Aisheng
2019-08-19 12:36 ` Daniel Baluta [this message]
2019-08-29 10:01 ` Oliver Graute
2019-07-16 15:00 ` [PATCH v3 02/11] dt-bindings: clock: imx-lpcg: add support " Dong Aisheng
2019-08-03 13:50 ` Shawn Guo
2019-08-05 3:27 ` Dong Aisheng
2019-08-12 13:00 ` Shawn Guo
2019-08-12 14:41 ` Aisheng Dong
2019-08-12 15:54 ` Shawn Guo
2019-08-19 12:42 ` Daniel Baluta
2019-07-16 15:00 ` [PATCH v3 03/11] clk: imx: scu: add two cells binding support Dong Aisheng
2019-07-16 15:00 ` [PATCH v3 04/11] clk: imx: scu: bypass cpu power domains Dong Aisheng
2019-07-16 15:00 ` [PATCH v3 05/11] clk: imx: scu: allow scu clk to take device pointer Dong Aisheng
2019-07-16 15:01 ` [PATCH v3 06/11] clk: imx: scu: add runtime pm support Dong Aisheng
2019-07-16 15:01 ` [PATCH v3 07/11] clk: imx: scu: add suspend/resume support Dong Aisheng
2019-07-16 15:01 ` [PATCH v3 08/11] clk: imx: imx8qxp-lpcg: add parsing clocks from device tree Dong Aisheng
2019-07-16 15:01 ` [PATCH v3 09/11] clk: imx: lpcg: allow lpcg clk to take device pointer Dong Aisheng
2019-07-16 15:01 ` [PATCH v3 10/11] clk: imx: clk-imx8qxp-lpcg: add runtime pm support Dong Aisheng
2019-07-16 15:01 ` [PATCH v3 11/11] clk: imx: lpcg: add suspend/resume support Dong Aisheng
2019-07-25 12:48 ` [PATCH v3 00/11] clk: imx8: add new clock binding for better pm support Dong Aisheng
2019-07-30 11:55 ` 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=caab750569dc246c99e88bbe6d9ca137ae7f198f.camel@nxp.com \
--to=daniel.baluta@nxp.com \
--cc=aisheng.dong@nxp.com \
--cc=devicetree@vger.kernel.org \
--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=robh+dt@kernel.org \
--cc=sboyd@kernel.org \
--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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).