From: Chanwoo Choi <cwchoi00@gmail.com> To: Abel Vesa <abel.vesa@nxp.com>, Rob Herring <robh@kernel.org>, Dong Aisheng <aisheng.dong@nxp.com>, Shawn Guo <shawnguo@kernel.org>, Sascha Hauer <s.hauer@pengutronix.de>, Fabio Estevam <festevam@gmail.com>, "catalin.marinas@arm.com" <catalin.marinas@arm.com>, Will Deacon <will.deacon@arm.com>, MyungJoo Ham <myungjoo.ham@samsung.com>, Kyungmin Park <kyungmin.park@samsung.com>, Chanwoo Choi <cw00.choi@samsung.com>, Georgi Djakov <djakov@kernel.org>, Adrian Hunter <adrian.hunter@intel.com>, Ulf Hansson <ulf.hansson@linaro.org>, Ahmad Fatoum <a.fatoum@pengutronix.de> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>, linux-serial@vger.kernel.org, NXP Linux Team <linux-imx@nxp.com>, Linux Kernel Mailing List <linux-kernel@vger.kernel.org>, devicetree@vger.kernel.org, linux-pm@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: Re: [RFC 05/19] devfreq: imx8m-ddrc: Use the opps acquired from EL3 Date: Wed, 15 Sep 2021 12:29:20 +0900 [thread overview] Message-ID: <b52430cc-1216-8b3e-baec-7d3fffc1274a@gmail.com> (raw) In-Reply-To: <1631554694-9599-6-git-send-email-abel.vesa@nxp.com> Hi, OPP is mandatory for devfreq driver. Also, must need to add the OPP levels to devicetree file, it is better to show the supported OPP list for the developer who don't know the detailed background of driver. If there are no any critical issue. I prefer the existing approach for the readability. On 21. 9. 14. 오전 2:38, Abel Vesa wrote: > i.MX8M platforms get their dram OPPs from the EL3. > We don't need to duplicate that in the kernel dram dts node. > We should just trust the OPPs provided by the EL3. > > Signed-off-by: Abel Vesa <abel.vesa@nxp.com> > --- > drivers/devfreq/imx8m-ddrc.c | 50 +++--------------------------------- > 1 file changed, 3 insertions(+), 47 deletions(-) > > diff --git a/drivers/devfreq/imx8m-ddrc.c b/drivers/devfreq/imx8m-ddrc.c > index 583123bf2100..f18a5c3c1c03 100644 > --- a/drivers/devfreq/imx8m-ddrc.c > +++ b/drivers/devfreq/imx8m-ddrc.c > @@ -321,38 +321,9 @@ static int imx8m_ddrc_init_freq_info(struct device *dev) > if (freq->dram_core_parent_index == 2 && > freq->dram_alt_parent_index == 0) > return -ENODEV; > - } > - > - return 0; > -} > - > -static int imx8m_ddrc_check_opps(struct device *dev) > -{ > - struct imx8m_ddrc *priv = dev_get_drvdata(dev); > - struct imx8m_ddrc_freq *freq_info; > - struct dev_pm_opp *opp; > - unsigned long freq; > - int i, opp_count; > - > - /* Enumerate DT OPPs and disable those not supported by firmware */ > - opp_count = dev_pm_opp_get_opp_count(dev); > - if (opp_count < 0) > - return opp_count; > - for (i = 0, freq = 0; i < opp_count; ++i, ++freq) { > - opp = dev_pm_opp_find_freq_ceil(dev, &freq); > - if (IS_ERR(opp)) { > - dev_err(dev, "Failed enumerating OPPs: %ld\n", > - PTR_ERR(opp)); > - return PTR_ERR(opp); > - } > - dev_pm_opp_put(opp); > > - freq_info = imx8m_ddrc_find_freq(priv, freq); > - if (!freq_info) { > - dev_info(dev, "Disable unsupported OPP %luHz %luMT/s\n", > - freq, DIV_ROUND_CLOSEST(freq, 250000)); > - dev_pm_opp_disable(dev, freq); > - } > + if (dev_pm_opp_add(dev, freq->rate * 250000, 0)) > + return -ENODEV; > } > > return 0; > @@ -360,7 +331,6 @@ static int imx8m_ddrc_check_opps(struct device *dev) > > static void imx8m_ddrc_exit(struct device *dev) > { > - dev_pm_opp_of_remove_table(dev); > } > > static int imx8m_ddrc_probe(struct platform_device *pdev) > @@ -407,16 +377,7 @@ static int imx8m_ddrc_probe(struct platform_device *pdev) > return ret; > } > > - ret = dev_pm_opp_of_add_table(dev); > - if (ret < 0) { > - dev_err(dev, "failed to get OPP table\n"); > - return ret; > - } > - > - ret = imx8m_ddrc_check_opps(dev); > - if (ret < 0) > - goto err; > - > + priv->profile.polling_ms = 1000; This change is not related to role of this patch. Need to make the separate patch. > priv->profile.target = imx8m_ddrc_target; > priv->profile.exit = imx8m_ddrc_exit; > priv->profile.get_cur_freq = imx8m_ddrc_get_cur_freq; > @@ -427,13 +388,8 @@ static int imx8m_ddrc_probe(struct platform_device *pdev) > if (IS_ERR(priv->devfreq)) { > ret = PTR_ERR(priv->devfreq); > dev_err(dev, "failed to add devfreq device: %d\n", ret); > - goto err; > } > > - return 0; > - > -err: > - dev_pm_opp_of_remove_table(dev); > return ret; > } > > -- Best Regards, Samsung Electronics Chanwoo Choi
WARNING: multiple messages have this Message-ID (diff)
From: Chanwoo Choi <cwchoi00@gmail.com> To: Abel Vesa <abel.vesa@nxp.com>, Rob Herring <robh@kernel.org>, Dong Aisheng <aisheng.dong@nxp.com>, Shawn Guo <shawnguo@kernel.org>, Sascha Hauer <s.hauer@pengutronix.de>, Fabio Estevam <festevam@gmail.com>, "catalin.marinas@arm.com" <catalin.marinas@arm.com>, Will Deacon <will.deacon@arm.com>, MyungJoo Ham <myungjoo.ham@samsung.com>, Kyungmin Park <kyungmin.park@samsung.com>, Chanwoo Choi <cw00.choi@samsung.com>, Georgi Djakov <djakov@kernel.org>, Adrian Hunter <adrian.hunter@intel.com>, Ulf Hansson <ulf.hansson@linaro.org>, Ahmad Fatoum <a.fatoum@pengutronix.de> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>, linux-serial@vger.kernel.org, NXP Linux Team <linux-imx@nxp.com>, Linux Kernel Mailing List <linux-kernel@vger.kernel.org>, devicetree@vger.kernel.org, linux-pm@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: Re: [RFC 05/19] devfreq: imx8m-ddrc: Use the opps acquired from EL3 Date: Wed, 15 Sep 2021 12:29:20 +0900 [thread overview] Message-ID: <b52430cc-1216-8b3e-baec-7d3fffc1274a@gmail.com> (raw) In-Reply-To: <1631554694-9599-6-git-send-email-abel.vesa@nxp.com> Hi, OPP is mandatory for devfreq driver. Also, must need to add the OPP levels to devicetree file, it is better to show the supported OPP list for the developer who don't know the detailed background of driver. If there are no any critical issue. I prefer the existing approach for the readability. On 21. 9. 14. 오전 2:38, Abel Vesa wrote: > i.MX8M platforms get their dram OPPs from the EL3. > We don't need to duplicate that in the kernel dram dts node. > We should just trust the OPPs provided by the EL3. > > Signed-off-by: Abel Vesa <abel.vesa@nxp.com> > --- > drivers/devfreq/imx8m-ddrc.c | 50 +++--------------------------------- > 1 file changed, 3 insertions(+), 47 deletions(-) > > diff --git a/drivers/devfreq/imx8m-ddrc.c b/drivers/devfreq/imx8m-ddrc.c > index 583123bf2100..f18a5c3c1c03 100644 > --- a/drivers/devfreq/imx8m-ddrc.c > +++ b/drivers/devfreq/imx8m-ddrc.c > @@ -321,38 +321,9 @@ static int imx8m_ddrc_init_freq_info(struct device *dev) > if (freq->dram_core_parent_index == 2 && > freq->dram_alt_parent_index == 0) > return -ENODEV; > - } > - > - return 0; > -} > - > -static int imx8m_ddrc_check_opps(struct device *dev) > -{ > - struct imx8m_ddrc *priv = dev_get_drvdata(dev); > - struct imx8m_ddrc_freq *freq_info; > - struct dev_pm_opp *opp; > - unsigned long freq; > - int i, opp_count; > - > - /* Enumerate DT OPPs and disable those not supported by firmware */ > - opp_count = dev_pm_opp_get_opp_count(dev); > - if (opp_count < 0) > - return opp_count; > - for (i = 0, freq = 0; i < opp_count; ++i, ++freq) { > - opp = dev_pm_opp_find_freq_ceil(dev, &freq); > - if (IS_ERR(opp)) { > - dev_err(dev, "Failed enumerating OPPs: %ld\n", > - PTR_ERR(opp)); > - return PTR_ERR(opp); > - } > - dev_pm_opp_put(opp); > > - freq_info = imx8m_ddrc_find_freq(priv, freq); > - if (!freq_info) { > - dev_info(dev, "Disable unsupported OPP %luHz %luMT/s\n", > - freq, DIV_ROUND_CLOSEST(freq, 250000)); > - dev_pm_opp_disable(dev, freq); > - } > + if (dev_pm_opp_add(dev, freq->rate * 250000, 0)) > + return -ENODEV; > } > > return 0; > @@ -360,7 +331,6 @@ static int imx8m_ddrc_check_opps(struct device *dev) > > static void imx8m_ddrc_exit(struct device *dev) > { > - dev_pm_opp_of_remove_table(dev); > } > > static int imx8m_ddrc_probe(struct platform_device *pdev) > @@ -407,16 +377,7 @@ static int imx8m_ddrc_probe(struct platform_device *pdev) > return ret; > } > > - ret = dev_pm_opp_of_add_table(dev); > - if (ret < 0) { > - dev_err(dev, "failed to get OPP table\n"); > - return ret; > - } > - > - ret = imx8m_ddrc_check_opps(dev); > - if (ret < 0) > - goto err; > - > + priv->profile.polling_ms = 1000; This change is not related to role of this patch. Need to make the separate patch. > priv->profile.target = imx8m_ddrc_target; > priv->profile.exit = imx8m_ddrc_exit; > priv->profile.get_cur_freq = imx8m_ddrc_get_cur_freq; > @@ -427,13 +388,8 @@ static int imx8m_ddrc_probe(struct platform_device *pdev) > if (IS_ERR(priv->devfreq)) { > ret = PTR_ERR(priv->devfreq); > dev_err(dev, "failed to add devfreq device: %d\n", ret); > - goto err; > } > > - return 0; > - > -err: > - dev_pm_opp_of_remove_table(dev); > return ret; > } > > -- Best Regards, Samsung Electronics Chanwoo Choi _______________________________________________ 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:[~2021-09-15 3:29 UTC|newest] Thread overview: 72+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-09-13 17:37 [RFC 00/19] Add interconnect and devfreq support for i.MX8MQ Abel Vesa 2021-09-13 17:37 ` Abel Vesa 2021-09-13 17:37 ` [RFC 01/19] dt-bindings: interconnect: imx8mq: Add missing pl301 and SAI ids Abel Vesa 2021-09-13 17:37 ` Abel Vesa 2021-09-15 8:09 ` Georgi Djakov 2021-09-15 8:09 ` Georgi Djakov 2021-09-13 17:37 ` [RFC 02/19] devfreq: imx-bus: Switch governor to powersave Abel Vesa 2021-09-13 17:37 ` Abel Vesa 2021-09-13 17:37 ` [RFC 03/19] devfreq: imx-bus: Decouple imx-bus from icc made Abel Vesa 2021-09-13 17:37 ` Abel Vesa 2021-09-13 17:37 ` [RFC 04/19] devfreq: imx8m-ddrc: Change governor to powersave Abel Vesa 2021-09-13 17:37 ` Abel Vesa 2021-09-13 17:38 ` [RFC 05/19] devfreq: imx8m-ddrc: Use the opps acquired from EL3 Abel Vesa 2021-09-13 17:38 ` Abel Vesa 2021-09-15 3:29 ` Chanwoo Choi [this message] 2021-09-15 3:29 ` Chanwoo Choi 2021-09-15 18:12 ` Chanwoo Choi 2021-09-15 18:12 ` Chanwoo Choi 2021-09-13 17:38 ` [RFC 06/19] devfreq: imx8m-ddrc: Add late system sleep PM ops Abel Vesa 2021-09-13 17:38 ` Abel Vesa 2021-09-15 3:37 ` Chanwoo Choi 2021-09-15 3:37 ` Chanwoo Choi 2021-10-25 20:59 ` Abel Vesa 2021-10-25 20:59 ` Abel Vesa 2021-11-10 12:15 ` Martin Kepplinger 2021-11-10 12:15 ` Martin Kepplinger 2021-11-30 20:06 ` Abel Vesa 2021-11-30 20:06 ` Abel Vesa 2021-12-01 9:35 ` Martin Kepplinger 2021-12-01 9:35 ` Martin Kepplinger 2021-12-06 12:33 ` Martin Kepplinger 2021-12-06 12:33 ` Martin Kepplinger 2021-09-13 17:38 ` [RFC 07/19] interconnect: imx: Switch from imx_icc_node_adj_desc to fsl,icc-id node assignment Abel Vesa 2021-09-13 17:38 ` [RFC 07/19] interconnect: imx: Switch from imx_icc_node_adj_desc to fsl, icc-id " Abel Vesa 2021-09-13 17:38 ` [RFC 08/19] interconnect: imx8: Remove the imx_icc_node_adj_desc Abel Vesa 2021-09-13 17:38 ` Abel Vesa 2021-10-18 12:41 ` Adam Ford 2021-10-18 12:41 ` Adam Ford 2021-10-25 9:00 ` Abel Vesa 2021-10-25 9:00 ` Abel Vesa 2021-09-13 17:38 ` [RFC 09/19] interconnect: imx8mq: Add the pl301_per_m and pl301_wakeup nodes and subnodes Abel Vesa 2021-09-13 17:38 ` Abel Vesa 2021-09-13 17:38 ` [RFC 10/19] interconnect: imx8mq: Add of_match_table Abel Vesa 2021-09-13 17:38 ` Abel Vesa 2021-09-13 17:38 ` [RFC 11/19] interconnect: imx: Add imx_icc_get_bw and imx_icc_aggregate functions Abel Vesa 2021-09-13 17:38 ` Abel Vesa 2021-09-13 17:38 ` [RFC 12/19] arm64: dts: imx8mq: Add fsl,icc-id property to ddrc node Abel Vesa 2021-09-13 17:38 ` Abel Vesa 2021-09-13 17:38 ` [RFC 13/19] arm64: dts: imx8mq: Add fsl,icc-id to noc node Abel Vesa 2021-09-13 17:38 ` Abel Vesa 2021-09-13 17:38 ` [RFC 14/19] arm64: dts: imx8mq: Add all pl301 nodes Abel Vesa 2021-09-13 17:38 ` Abel Vesa 2021-09-13 17:38 ` [RFC 15/19] arm64: dts: imx8mq: Add the interconnect node Abel Vesa 2021-09-13 17:38 ` Abel Vesa 2021-09-13 17:38 ` [RFC 16/19] arm64: dts: imx8mq: Add interconnect properties to icc consumer nodes Abel Vesa 2021-09-13 17:38 ` Abel Vesa 2021-09-13 17:38 ` [RFC 17/19] net: ethernet: fec_main: Add interconnect support Abel Vesa 2021-09-13 17:38 ` Abel Vesa 2021-09-13 17:38 ` [RFC 18/19] mmc: sdhci-esdhc-imx: " Abel Vesa 2021-09-13 17:38 ` Abel Vesa 2021-09-13 17:38 ` [RFC 19/19] arm64: defconfig: Add necessary configs for icc+devfreq on i.MX8MQ Abel Vesa 2021-09-13 17:38 ` Abel Vesa 2021-09-24 10:20 ` [RFC 00/19] Add interconnect and devfreq support for i.MX8MQ Martin Kepplinger 2021-09-24 10:20 ` Martin Kepplinger 2021-09-29 11:44 ` Abel Vesa 2021-09-29 11:44 ` Abel Vesa 2021-09-30 8:03 ` Martin Kepplinger 2021-09-30 8:03 ` Martin Kepplinger 2021-09-30 14:22 ` Abel Vesa 2021-09-30 14:22 ` Abel Vesa 2021-10-05 13:34 ` Martin Kepplinger 2021-10-05 13:34 ` Martin Kepplinger
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=b52430cc-1216-8b3e-baec-7d3fffc1274a@gmail.com \ --to=cwchoi00@gmail.com \ --cc=a.fatoum@pengutronix.de \ --cc=abel.vesa@nxp.com \ --cc=adrian.hunter@intel.com \ --cc=aisheng.dong@nxp.com \ --cc=catalin.marinas@arm.com \ --cc=cw00.choi@samsung.com \ --cc=devicetree@vger.kernel.org \ --cc=djakov@kernel.org \ --cc=festevam@gmail.com \ --cc=kernel@pengutronix.de \ --cc=kyungmin.park@samsung.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-imx@nxp.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-pm@vger.kernel.org \ --cc=linux-serial@vger.kernel.org \ --cc=myungjoo.ham@samsung.com \ --cc=robh@kernel.org \ --cc=s.hauer@pengutronix.de \ --cc=shawnguo@kernel.org \ --cc=ulf.hansson@linaro.org \ --cc=will.deacon@arm.com \ /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.