From: Shawn Guo <shawnguo@kernel.org>
To: Dong Aisheng <aisheng.dong@nxp.com>
Cc: linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
sboyd@kernel.org, mturquette@baylibre.com, fabio.estevam@nxp.com,
linux-imx@nxp.com, kernel@pengutronix.de,
Rob Herring <robh+dt@kernel.org>,
devicetree@vger.kernel.org
Subject: Re: [PATCH v3 01/11] dt-bindings: firmware: imx-scu: new binding to parse clocks from device tree
Date: Sat, 3 Aug 2019 16:42:51 +0200 [thread overview]
Message-ID: <20190803144246.GM8870@X250.getinternet.no> (raw)
In-Reply-To: <1563289265-10977-2-git-send-email-aisheng.dong@nxp.com>
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 */
> +#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 */
> +#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 */
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?
Shawn
> +
> #endif /* __DT_BINDINGS_RSCRC_IMX_H */
> --
> 2.7.4
>
next prev parent reply other threads:[~2019-08-03 14:42 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 [this message]
2019-08-05 3:48 ` Dong Aisheng
2019-08-19 12:36 ` Daniel Baluta
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=20190803144246.GM8870@X250.getinternet.no \
--to=shawnguo@kernel.org \
--cc=aisheng.dong@nxp.com \
--cc=devicetree@vger.kernel.org \
--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 \
/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).