From: Patrice CHOTARD <patrice.chotard@foss.st.com>
To: Alain Volmat <avolmat@me.com>,
Michael Turquette <mturquette@baylibre.com>,
Stephen Boyd <sboyd@kernel.org>, Rob Herring <robh+dt@kernel.org>
Cc: Lee Jones <lee.jones@linaro.org>, <linux-clk@vger.kernel.org>,
<devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH v3 2/7] clk: st: flexgen: embed soc clock outputs within compatible data
Date: Wed, 31 Mar 2021 18:06:22 +0200 [thread overview]
Message-ID: <c1595010-c001-807c-b109-a3d8e572dd4d@foss.st.com> (raw)
In-Reply-To: <20210330205125.25708-3-avolmat@me.com>
Hi Alain
I have tested your series on STiH410-B2260 board and it misses a critical flag, see below
On 3/30/21 10:51 PM, Alain Volmat wrote:
> In order to avoid relying on the old style description via the DT
> clock-output-names, add compatible data describing the flexgen
> outputs clocks for all STiH407/STiH410 and STiH418 SOCs.
>
> In order to ease transition between the two methods, this commit
> introduce the new compatible without removing the old method.
> Once DTs will be fixed, the method relying on DT clock-output-names
> will be removed from this driver as well as old compatibles.
>
> Signed-off-by: Alain Volmat <avolmat@me.com>
> ---
> v3: add comments about CLK_IS_CRITICAL reason
> don't put clk-proc-stfe as CLK_IS_CRITICAL
> v2: add some missing clock as CLK_IS_CRITICAL
>
> drivers/clk/st/clk-flexgen.c | 366 +++++++++++++++++++++++++++++++++--
> 1 file changed, 352 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/clk/st/clk-flexgen.c b/drivers/clk/st/clk-flexgen.c
> index 55873d4b7603..69711c6c47a4 100644
> --- a/drivers/clk/st/clk-flexgen.c
> +++ b/drivers/clk/st/clk-flexgen.c
> @@ -16,9 +16,16 @@
> #include <linux/of.h>
> #include <linux/of_address.h>
>
> +struct clkgen_clk_out {
> + const char *name;
> + unsigned long flags;
> +};
> +
> struct clkgen_data {
> unsigned long flags;
> bool mode;
> + const struct clkgen_clk_out *outputs;
> + const unsigned int outputs_nb;
> };
>
> struct flexgen {
> @@ -295,6 +302,289 @@ static const struct clkgen_data clkgen_video = {
> .mode = 1,
> };
>
> +static const struct clkgen_clk_out clkgen_stih407_a0_clk_out[] = {
> + /* This clk needs to be on so that memory interface is accessible */
> + { .name = "clk-ic-lmi0", .flags = CLK_IS_CRITICAL },
> +};
> +
> +static const struct clkgen_data clkgen_stih407_a0 = {
> + .outputs = clkgen_stih407_a0_clk_out,
> + .outputs_nb = ARRAY_SIZE(clkgen_stih407_a0_clk_out),
> +};
> +
> +static const struct clkgen_clk_out clkgen_stih410_a0_clk_out[] = {
> + /* Those clks need to be on so that memory interface is accessible */
> + { .name = "clk-ic-lmi0", .flags = CLK_IS_CRITICAL },
> + { .name = "clk-ic-lmi1", .flags = CLK_IS_CRITICAL },
> +};
> +
> +static const struct clkgen_data clkgen_stih410_a0 = {
> + .outputs = clkgen_stih410_a0_clk_out,
> + .outputs_nb = ARRAY_SIZE(clkgen_stih410_a0_clk_out),
> +};
> +
> +static const struct clkgen_clk_out clkgen_stih407_c0_clk_out[] = {
> + { .name = "clk-icn-gpu", },
> + { .name = "clk-fdma", },
> + { .name = "clk-nand", },
> + { .name = "clk-hva", },
> + { .name = "clk-proc-stfe", },
> + { .name = "clk-proc-tp", },
> + { .name = "clk-rx-icn-dmu", },
> + { .name = "clk-rx-icn-hva", },
> + /* This clk needs to be on to keep bus interconnect alive */
> + { .name = "clk-icn-cpu", .flags = CLK_IS_CRITICAL },
> + /* This clk needs to be on to keep bus interconnect alive */
> + { .name = "clk-tx-icn-dmu", .flags = CLK_IS_CRITICAL },
> + { .name = "clk-mmc-0", },
> + { .name = "clk-mmc-1", },
> + { .name = "clk-jpegdec", },
> + /* This clk needs to be on to keep A9 running */
> + { .name = "clk-ext2fa9", .flags = CLK_IS_CRITICAL },
> + { .name = "clk-ic-bdisp-0", },
> + { .name = "clk-ic-bdisp-1", },
> + { .name = "clk-pp-dmu", },
> + { .name = "clk-vid-dmu", },
> + { .name = "clk-dss-lpc", },
> + { .name = "clk-st231-aud-0", },
> + { .name = "clk-st231-gp-1", },
> + { .name = "clk-st231-dmu", },
> + /* This clk needs to be on to keep bus interconnect alive */
> + { .name = "clk-icn-lmi", .flags = CLK_IS_CRITICAL },
> + { .name = "clk-tx-icn-disp-1", },
> + /* This clk needs to be on to keep bus interconnect alive */
> + { .name = "clk-icn-sbc", .flags = CLK_IS_CRITICAL },
> + { .name = "clk-stfe-frc2", },
> + { .name = "clk-eth-phy", },
> + { .name = "clk-eth-ref-phyclk", },
> + { .name = "clk-flash-promip", },
> + { .name = "clk-main-disp", },
> + { .name = "clk-aux-disp", },
> + { .name = "clk-compo-dvp", },
> +};
> +
> +static const struct clkgen_data clkgen_stih407_c0 = {
> + .outputs = clkgen_stih407_c0_clk_out,
> + .outputs_nb = ARRAY_SIZE(clkgen_stih407_c0_clk_out),
> +};
> +
> +static const struct clkgen_clk_out clkgen_stih410_c0_clk_out[] = {
> + { .name = "clk-icn-gpu", },
> + { .name = "clk-fdma", },
> + { .name = "clk-nand", },
> + { .name = "clk-hva", },
> + { .name = "clk-proc-stfe", },
> + { .name = "clk-proc-tp", },
> + { .name = "clk-rx-icn-dmu", },
> + { .name = "clk-rx-icn-hva", },
> + /* This clk needs to be on to keep bus interconnect alive */
> + { .name = "clk-icn-cpu", .flags = CLK_IS_CRITICAL },
> + /* This clk needs to be on to keep bus interconnect alive */
> + { .name = "clk-tx-icn-dmu", .flags = CLK_IS_CRITICAL },
> + { .name = "clk-mmc-0", },
> + { .name = "clk-mmc-1", },
> + { .name = "clk-jpegdec", },
> + /* This clk needs to be on to keep A9 running */
> + { .name = "clk-ext2fa9", .flags = CLK_IS_CRITICAL },
> + { .name = "clk-ic-bdisp-0", },
> + { .name = "clk-ic-bdisp-1", },
> + { .name = "clk-pp-dmu", },
> + { .name = "clk-vid-dmu", },
> + { .name = "clk-dss-lpc", },
> + { .name = "clk-st231-aud-0", },
> + { .name = "clk-st231-gp-1", },
> + { .name = "clk-st231-dmu", },
> + /* This clk needs to be on to keep bus interconnect alive */
> + { .name = "clk-icn-lmi", .flags = CLK_IS_CRITICAL },
> + { .name = "clk-tx-icn-disp-1", },
> + /* This clk needs to be on to keep bus interconnect alive */
> + { .name = "clk-icn-sbc", .flags = CLK_IS_CRITICAL },
> + { .name = "clk-stfe-frc2", },
> + { .name = "clk-eth-phy", },
> + { .name = "clk-eth-ref-phyclk", },
> + { .name = "clk-flash-promip", },
> + { .name = "clk-main-disp", },
> + { .name = "clk-aux-disp", },
> + { .name = "clk-compo-dvp", },
> + { .name = "clk-tx-icn-hades", },
> + { .name = "clk-rx-icn-hades", },
> + { .name = "clk-icn-reg-16", },
Need to add CLK_IS_CRITICAL for clk-icn-reg-16
This was a long time bug which is hidden by usage of clk_ignore_unused in bootargs.
With that fix, you can add
Reviewed-by: Patrice Chotard <patrice.chotard@foss.st.com>
Thanks
Patrice
> + { .name = "clk-pp-hades", },
> + { .name = "clk-clust-hades", },
> + { .name = "clk-hwpe-hades", },
> + { .name = "clk-fc-hades", },
> +};
> +
> +static const struct clkgen_data clkgen_stih410_c0 = {
> + .outputs = clkgen_stih410_c0_clk_out,
> + .outputs_nb = ARRAY_SIZE(clkgen_stih410_c0_clk_out),
> +};
> +
> +static const struct clkgen_clk_out clkgen_stih418_c0_clk_out[] = {
> + { .name = "clk-icn-gpu", },
> + { .name = "clk-fdma", },
> + { .name = "clk-nand", },
> + { .name = "clk-hva", },
> + { .name = "clk-proc-stfe", },
> + { .name = "clk-tp", },
> + /* This clk needs to be on to keep bus interconnect alive */
> + { .name = "clk-rx-icn-dmu", .flags = CLK_IS_CRITICAL },
> + /* This clk needs to be on to keep bus interconnect alive */
> + { .name = "clk-rx-icn-hva", .flags = CLK_IS_CRITICAL },
> + { .name = "clk-icn-cpu", .flags = CLK_IS_CRITICAL },
> + /* This clk needs to be on to keep bus interconnect alive */
> + { .name = "clk-tx-icn-dmu", .flags = CLK_IS_CRITICAL },
> + { .name = "clk-mmc-0", },
> + { .name = "clk-mmc-1", },
> + { .name = "clk-jpegdec", },
> + /* This clk needs to be on to keep bus interconnect alive */
> + { .name = "clk-icn-reg", .flags = CLK_IS_CRITICAL },
> + { .name = "clk-proc-bdisp-0", },
> + { .name = "clk-proc-bdisp-1", },
> + { .name = "clk-pp-dmu", },
> + { .name = "clk-vid-dmu", },
> + { .name = "clk-dss-lpc", },
> + { .name = "clk-st231-aud-0", },
> + { .name = "clk-st231-gp-1", },
> + { .name = "clk-st231-dmu", },
> + /* This clk needs to be on to keep bus interconnect alive */
> + { .name = "clk-icn-lmi", .flags = CLK_IS_CRITICAL },
> + /* This clk needs to be on to keep bus interconnect alive */
> + { .name = "clk-tx-icn-1", .flags = CLK_IS_CRITICAL },
> + /* This clk needs to be on to keep bus interconnect alive */
> + { .name = "clk-icn-sbc", .flags = CLK_IS_CRITICAL },
> + { .name = "clk-stfe-frc2", },
> + { .name = "clk-eth-phyref", },
> + { .name = "clk-eth-ref-phyclk", },
> + { .name = "clk-flash-promip", },
> + { .name = "clk-main-disp", },
> + { .name = "clk-aux-disp", },
> + { .name = "clk-compo-dvp", },
> + /* This clk needs to be on to keep bus interconnect alive */
> + { .name = "clk-tx-icn-hades", .flags = CLK_IS_CRITICAL },
> + /* This clk needs to be on to keep bus interconnect alive */
> + { .name = "clk-rx-icn-hades", .flags = CLK_IS_CRITICAL },
> + /* This clk needs to be on to keep bus interconnect alive */
> + { .name = "clk-icn-reg-16", .flags = CLK_IS_CRITICAL },
> + { .name = "clk-pp-hevc", },
> + { .name = "clk-clust-hevc", },
> + { .name = "clk-hwpe-hevc", },
> + { .name = "clk-fc-hevc", },
> + { .name = "clk-proc-mixer", },
> + { .name = "clk-proc-sc", },
> + { .name = "clk-avsp-hevc", },
> +};
> +
> +static const struct clkgen_data clkgen_stih418_c0 = {
> + .outputs = clkgen_stih418_c0_clk_out,
> + .outputs_nb = ARRAY_SIZE(clkgen_stih418_c0_clk_out),
> +};
> +
> +static const struct clkgen_clk_out clkgen_stih407_d0_clk_out[] = {
> + { .name = "clk-pcm-0", },
> + { .name = "clk-pcm-1", },
> + { .name = "clk-pcm-2", },
> + { .name = "clk-spdiff", },
> +};
> +
> +static const struct clkgen_data clkgen_stih407_d0 = {
> + .flags = CLK_SET_RATE_PARENT,
> + .outputs = clkgen_stih407_d0_clk_out,
> + .outputs_nb = ARRAY_SIZE(clkgen_stih407_d0_clk_out),
> +};
> +
> +static const struct clkgen_clk_out clkgen_stih410_d0_clk_out[] = {
> + { .name = "clk-pcm-0", },
> + { .name = "clk-pcm-1", },
> + { .name = "clk-pcm-2", },
> + { .name = "clk-spdiff", },
> + { .name = "clk-pcmr10-master", },
> + { .name = "clk-usb2-phy", },
> +};
> +
> +static const struct clkgen_data clkgen_stih410_d0 = {
> + .flags = CLK_SET_RATE_PARENT,
> + .outputs = clkgen_stih410_d0_clk_out,
> + .outputs_nb = ARRAY_SIZE(clkgen_stih410_d0_clk_out),
> +};
> +
> +static const struct clkgen_clk_out clkgen_stih407_d2_clk_out[] = {
> + { .name = "clk-pix-main-disp", },
> + { .name = "clk-pix-pip", },
> + { .name = "clk-pix-gdp1", },
> + { .name = "clk-pix-gdp2", },
> + { .name = "clk-pix-gdp3", },
> + { .name = "clk-pix-gdp4", },
> + { .name = "clk-pix-aux-disp", },
> + { .name = "clk-denc", },
> + { .name = "clk-pix-hddac", },
> + { .name = "clk-hddac", },
> + { .name = "clk-sddac", },
> + { .name = "clk-pix-dvo", },
> + { .name = "clk-dvo", },
> + { .name = "clk-pix-hdmi", },
> + { .name = "clk-tmds-hdmi", },
> + { .name = "clk-ref-hdmiphy", },
> +};
> +
> +static const struct clkgen_data clkgen_stih407_d2 = {
> + .outputs = clkgen_stih407_d2_clk_out,
> + .outputs_nb = ARRAY_SIZE(clkgen_stih407_d2_clk_out),
> + .flags = CLK_SET_RATE_PARENT,
> + .mode = 1,
> +};
> +
> +static const struct clkgen_clk_out clkgen_stih418_d2_clk_out[] = {
> + { .name = "clk-pix-main-disp", },
> + { .name = "", },
> + { .name = "", },
> + { .name = "", },
> + { .name = "", },
> + { .name = "clk-tmds-hdmi-div2", },
> + { .name = "clk-pix-aux-disp", },
> + { .name = "clk-denc", },
> + { .name = "clk-pix-hddac", },
> + { .name = "clk-hddac", },
> + { .name = "clk-sddac", },
> + { .name = "clk-pix-dvo", },
> + { .name = "clk-dvo", },
> + { .name = "clk-pix-hdmi", },
> + { .name = "clk-tmds-hdmi", },
> + { .name = "clk-ref-hdmiphy", },
> + { .name = "", }, { .name = "", }, { .name = "", }, { .name = "", },
> + { .name = "", }, { .name = "", }, { .name = "", }, { .name = "", },
> + { .name = "", }, { .name = "", }, { .name = "", }, { .name = "", },
> + { .name = "", }, { .name = "", }, { .name = "", }, { .name = "", },
> + { .name = "", }, { .name = "", }, { .name = "", }, { .name = "", },
> + { .name = "", }, { .name = "", }, { .name = "", }, { .name = "", },
> + { .name = "", }, { .name = "", }, { .name = "", }, { .name = "", },
> + { .name = "", }, { .name = "", }, { .name = "", },
> + { .name = "clk-vp9", },
> +};
> +
> +static const struct clkgen_data clkgen_stih418_d2 = {
> + .outputs = clkgen_stih418_d2_clk_out,
> + .outputs_nb = ARRAY_SIZE(clkgen_stih418_d2_clk_out),
> + .flags = CLK_SET_RATE_PARENT,
> + .mode = 1,
> +};
> +
> +static const struct clkgen_clk_out clkgen_stih407_d3_clk_out[] = {
> + { .name = "clk-stfe-frc1", },
> + { .name = "clk-tsout-0", },
> + { .name = "clk-tsout-1", },
> + { .name = "clk-mchi", },
> + { .name = "clk-vsens-compo", },
> + { .name = "clk-frc1-remote", },
> + { .name = "clk-lpc-0", },
> + { .name = "clk-lpc-1", },
> +};
> +
> +static const struct clkgen_data clkgen_stih407_d3 = {
> + .outputs = clkgen_stih407_d3_clk_out,
> + .outputs_nb = ARRAY_SIZE(clkgen_stih407_d3_clk_out),
> +};
> +
> static const struct of_device_id flexgen_of_match[] = {
> {
> .compatible = "st,flexgen-audio",
> @@ -304,6 +594,46 @@ static const struct of_device_id flexgen_of_match[] = {
> .compatible = "st,flexgen-video",
> .data = &clkgen_video,
> },
> + {
> + .compatible = "st,flexgen-stih407-a0",
> + .data = &clkgen_stih407_a0,
> + },
> + {
> + .compatible = "st,flexgen-stih410-a0",
> + .data = &clkgen_stih410_a0,
> + },
> + {
> + .compatible = "st,flexgen-stih407-c0",
> + .data = &clkgen_stih407_c0,
> + },
> + {
> + .compatible = "st,flexgen-stih410-c0",
> + .data = &clkgen_stih410_c0,
> + },
> + {
> + .compatible = "st,flexgen-stih418-c0",
> + .data = &clkgen_stih418_c0,
> + },
> + {
> + .compatible = "st,flexgen-stih407-d0",
> + .data = &clkgen_stih407_d0,
> + },
> + {
> + .compatible = "st,flexgen-stih410-d0",
> + .data = &clkgen_stih410_d0,
> + },
> + {
> + .compatible = "st,flexgen-stih407-d2",
> + .data = &clkgen_stih407_d2,
> + },
> + {
> + .compatible = "st,flexgen-stih418-d2",
> + .data = &clkgen_stih418_d2,
> + },
> + {
> + .compatible = "st,flexgen-stih407-d3",
> + .data = &clkgen_stih407_d3,
> + },
> {}
> };
>
> @@ -320,6 +650,7 @@ static void __init st_of_flexgen_setup(struct device_node *np)
> unsigned long flex_flags = 0;
> int ret;
> bool clk_mode = 0;
> + const char *clk_name;
>
> pnode = of_get_parent(np);
> if (!pnode)
> @@ -347,13 +678,17 @@ static void __init st_of_flexgen_setup(struct device_node *np)
> if (!clk_data)
> goto err;
>
> - ret = of_property_count_strings(np, "clock-output-names");
> - if (ret <= 0) {
> - pr_err("%s: Failed to get number of output clocks (%d)",
> - __func__, clk_data->clk_num);
> - goto err;
> - }
> - clk_data->clk_num = ret;
> + /* First try to get output information from the compatible data */
> + if (!data || !data->outputs_nb || !data->outputs) {
> + ret = of_property_count_strings(np, "clock-output-names");
> + if (ret <= 0) {
> + pr_err("%s: Failed to get number of output clocks (%d)",
> + __func__, clk_data->clk_num);
> + goto err;
> + }
> + clk_data->clk_num = ret;
> + } else
> + clk_data->clk_num = data->outputs_nb;
>
> clk_data->clks = kcalloc(clk_data->clk_num, sizeof(struct clk *),
> GFP_KERNEL);
> @@ -368,16 +703,19 @@ static void __init st_of_flexgen_setup(struct device_node *np)
>
> for (i = 0; i < clk_data->clk_num; i++) {
> struct clk *clk;
> - const char *clk_name;
>
> - if (of_property_read_string_index(np, "clock-output-names",
> - i, &clk_name)) {
> - break;
> + if (!data || !data->outputs_nb || !data->outputs) {
> + if (of_property_read_string_index(np,
> + "clock-output-names",
> + i, &clk_name))
> + break;
> + flex_flags &= ~CLK_IS_CRITICAL;
> + of_clk_detect_critical(np, i, &flex_flags);
> + } else {
> + clk_name = data->outputs[i].name;
> + flex_flags = data->flags | data->outputs[i].flags;
> }
>
> - flex_flags &= ~CLK_IS_CRITICAL;
> - of_clk_detect_critical(np, i, &flex_flags);
> -
> /*
> * If we read an empty clock name then the output is unused
> */
>
next prev parent reply other threads:[~2021-03-31 16:07 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-30 20:51 [PATCH v3 0/7] clk: st: embed clock outputs within drivers Alain Volmat
2021-03-30 20:51 ` [PATCH v3 1/7] clk: st: clkgen-pll: remove unused variable of struct clkgen_pll Alain Volmat
2021-03-31 16:07 ` Patrice CHOTARD
2021-03-30 20:51 ` [PATCH v3 2/7] clk: st: flexgen: embed soc clock outputs within compatible data Alain Volmat
2021-03-31 16:06 ` Patrice CHOTARD [this message]
2021-03-30 20:51 ` [PATCH v3 3/7] dt-bindings: clock: st: flexgen: add new introduced compatible Alain Volmat
2021-03-30 20:51 ` [PATCH v3 4/7] clk: st: clkgen-pll: embed soc clock outputs within compatible data Alain Volmat
2021-03-31 16:08 ` Patrice CHOTARD
2021-03-30 20:51 ` [PATCH v3 5/7] dt-bindings: clock: st: clkgen-pll: add new introduced compatible Alain Volmat
2021-03-30 20:51 ` [PATCH v3 6/7] clk: st: clkgen-fsyn: embed soc clock outputs within compatible data Alain Volmat
2021-03-31 16:08 ` Patrice CHOTARD
2021-03-30 20:51 ` [PATCH v3 7/7] dt-bindings: clock: st: clkgen-fsyn: add new introduced compatible Alain Volmat
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=c1595010-c001-807c-b109-a3d8e572dd4d@foss.st.com \
--to=patrice.chotard@foss.st.com \
--cc=avolmat@me.com \
--cc=devicetree@vger.kernel.org \
--cc=lee.jones@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-clk@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--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).