[PATCH v2] driver: pwm: pwm-imx: fix probing on imx6
U-Boot 2024.07 drops on aristainetos2 board the following warning:
Failed to enable per_clk
and bootlogo is not seen on LVDS display.
This patch uses old behaviour for systems without clock framework if CONFIG_CLK is not enabled.
Fixes: bfc778cb93a3 ("driver: pwm: pwm-imx: get and enable per/ipg clock using dm")
Signed-off-by: Heiko Schocher hs@denx.de ---
Changes in v2: use CONFIG_IS_ENABLED instead of IS_ENABLED as Anatolij suggested.
drivers/pwm/pwm-imx.c | 56 ++++++++++++++++++++++++------------------- 1 file changed, 31 insertions(+), 25 deletions(-)
diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c index 8fbb40cc276..e69684b2167 100644 --- a/drivers/pwm/pwm-imx.c +++ b/drivers/pwm/pwm-imx.c @@ -160,7 +160,11 @@ int pwm_dm_imx_get_parms(struct imx_pwm_priv *priv, int period_ns, { unsigned long long c;
- c = clk_get_rate(&priv->per_clk); + if (CONFIG_IS_ENABLED(CLK)) + c = clk_get_rate(&priv->per_clk); + else + c = CFG_IMX6_PWM_PER_CLK; + c = c * period_ns; do_div(c, 1000000000); *period_c = c; @@ -227,43 +231,45 @@ static int imx_pwm_set_enable(struct udevice *dev, uint channel, bool enable)
static int imx_pwm_of_to_plat(struct udevice *dev) { - int ret; + int __maybe_unused ret; struct imx_pwm_priv *priv = dev_get_priv(dev);
priv->regs = dev_read_addr_ptr(dev);
- ret = clk_get_by_name(dev, "per", &priv->per_clk); - if (ret) { - printf("Failed to get per_clk\n"); - return ret; - } - - ret = clk_get_by_name(dev, "ipg", &priv->ipg_clk); - if (ret) { - printf("Failed to get ipg_clk\n"); - return ret; + if (CONFIG_IS_ENABLED(CLK)) { + ret = clk_get_by_name(dev, "per", &priv->per_clk); + if (ret) { + printf("Failed to get per_clk\n"); + return ret; + } + + ret = clk_get_by_name(dev, "ipg", &priv->ipg_clk); + if (ret) { + printf("Failed to get ipg_clk\n"); + return ret; + } } - return 0; }
static int imx_pwm_probe(struct udevice *dev) { - int ret; + int __maybe_unused ret; struct imx_pwm_priv *priv = dev_get_priv(dev);
- ret = clk_enable(&priv->per_clk); - if (ret) { - printf("Failed to enable per_clk\n"); - return ret; - } - - ret = clk_enable(&priv->ipg_clk); - if (ret) { - printf("Failed to enable ipg_clk\n"); - return ret; + if (CONFIG_IS_ENABLED(CLK)) { + ret = clk_enable(&priv->per_clk); + if (ret) { + printf("Failed to enable per_clk\n"); + return ret; + } + + ret = clk_enable(&priv->ipg_clk); + if (ret) { + printf("Failed to enable ipg_clk\n"); + return ret; + } } - return 0; }
Hi Heiko,
On Wed, Aug 7, 2024 at 12:53 PM Heiko Schocher hs@denx.de wrote:
U-Boot 2024.07 drops on aristainetos2 board the following warning:
Failed to enable per_clkand bootlogo is not seen on LVDS display.
This patch uses old behaviour for systems without clock framework if CONFIG_CLK is not enabled.
Fixes: bfc778cb93a3 ("driver: pwm: pwm-imx: get and enable per/ipg clock using dm")
Signed-off-by: Heiko Schocher hs@denx.de
Changes in v2: use CONFIG_IS_ENABLED instead of IS_ENABLED as Anatolij suggested.
This causes build failures:
https://source.denx.de/u-boot/custodians/u-boot-imx/-/jobs/887503
Please fix and resend.
Thanks
Hello Fabio, Tom,
On 13.08.24 14:27, Fabio Estevam wrote:
Hi Heiko,
On Wed, Aug 7, 2024 at 12:53 PM Heiko Schocher hs@denx.de wrote:
U-Boot 2024.07 drops on aristainetos2 board the following warning:
Failed to enable per_clkand bootlogo is not seen on LVDS display.
This patch uses old behaviour for systems without clock framework if CONFIG_CLK is not enabled.
Fixes: bfc778cb93a3 ("driver: pwm: pwm-imx: get and enable per/ipg clock using dm")
Signed-off-by: Heiko Schocher hs@denx.de
Changes in v2: use CONFIG_IS_ENABLED instead of IS_ENABLED as Anatolij suggested.
This causes build failures:
https://source.denx.de/u-boot/custodians/u-boot-imx/-/jobs/887503
Please fix and resend.
Hmm... yes ... I work/worked on aristainetos2 board update for 2024.07 (or later) ... and saw this
Failed to enable per_clk
warning when 2024.07 boots ... and bootlogo did not work anymore, as backlight is pwm driven on this hardware...
I bet, for all the boards which fail in the azure run with this patch, PWM do not work anymore... because CONFIG_CLK is not enabled for them!
I am really unhappy, that I have no automated U-Boot runtime tests anymore running on real hardware... setup with tbot was very easy, but had no time for looking at the bugs poping up...
Hmm... looking in git history, there comes this commit in my eyes:
commit 4eea76574062e9079b6a267229962d6ec0be910a Author: Tom Rini trini@konsulko.com Date: Sat Nov 19 18:45:18 2022 -0500
pwm: imx: Remove unused references to CONFIG_IMX6_PWM_PER_CLK
On platforms that use DM_PWM, we do not need to define this value anymore, so remove it from config files.
Signed-off-by: Tom Rini trini@konsulko.com
which exactly removes for all breaking boards this define...
Should we revert this patch?
I just can say for the aristianetos2 board, that I readded exactly this define in my work for the port to currentmainline ... and with my patch, pwm and so bootlogo works again ...
bye, Heiko
On Tue, Aug 13, 2024 at 03:57:08PM +0200, Heiko Schocher wrote:
Hello Fabio, Tom,
On 13.08.24 14:27, Fabio Estevam wrote:
Hi Heiko,
On Wed, Aug 7, 2024 at 12:53 PM Heiko Schocher hs@denx.de wrote:
U-Boot 2024.07 drops on aristainetos2 board the following warning:
Failed to enable per_clkand bootlogo is not seen on LVDS display.
This patch uses old behaviour for systems without clock framework if CONFIG_CLK is not enabled.
Fixes: bfc778cb93a3 ("driver: pwm: pwm-imx: get and enable per/ipg clock using dm")
Signed-off-by: Heiko Schocher hs@denx.de
Changes in v2: use CONFIG_IS_ENABLED instead of IS_ENABLED as Anatolij suggested.
This causes build failures:
https://source.denx.de/u-boot/custodians/u-boot-imx/-/jobs/887503
Please fix and resend.
Hmm... yes ... I work/worked on aristainetos2 board update for 2024.07 (or later) ... and saw this
Failed to enable per_clk
warning when 2024.07 boots ... and bootlogo did not work anymore, as backlight is pwm driven on this hardware...
I bet, for all the boards which fail in the azure run with this patch, PWM do not work anymore... because CONFIG_CLK is not enabled for them!
I am really unhappy, that I have no automated U-Boot runtime tests anymore running on real hardware... setup with tbot was very easy, but had no time for looking at the bugs poping up...
Hmm... looking in git history, there comes this commit in my eyes:
commit 4eea76574062e9079b6a267229962d6ec0be910a Author: Tom Rini trini@konsulko.com Date: Sat Nov 19 18:45:18 2022 -0500
pwm: imx: Remove unused references to CONFIG_IMX6_PWM_PER_CLK On platforms that use DM_PWM, we do not need to define this value anymore, so remove it from config files. Signed-off-by: Tom Rini <trini@konsulko.com>which exactly removes for all breaking boards this define...
Should we revert this patch?
I just can say for the aristianetos2 board, that I readded exactly this define in my work for the port to currentmainline ... and with my patch, pwm and so bootlogo works again ...
We can't do a strict revert, no, the symbol is now CFG_IMX6_PWM_PER_CLK. But also, isn't this board using DM_PWM? And if so, that value isn't used?
Hello Tom,
On 13.08.24 18:16, Tom Rini wrote:
On Tue, Aug 13, 2024 at 03:57:08PM +0200, Heiko Schocher wrote:
Hello Fabio, Tom,
On 13.08.24 14:27, Fabio Estevam wrote:
Hi Heiko,
On Wed, Aug 7, 2024 at 12:53 PM Heiko Schocher hs@denx.de wrote:
U-Boot 2024.07 drops on aristainetos2 board the following warning:
Failed to enable per_clkand bootlogo is not seen on LVDS display.
This patch uses old behaviour for systems without clock framework if CONFIG_CLK is not enabled.
Fixes: bfc778cb93a3 ("driver: pwm: pwm-imx: get and enable per/ipg clock using dm")
Signed-off-by: Heiko Schocher hs@denx.de
Changes in v2: use CONFIG_IS_ENABLED instead of IS_ENABLED as Anatolij suggested.
This causes build failures:
https://source.denx.de/u-boot/custodians/u-boot-imx/-/jobs/887503
Please fix and resend.
Hmm... yes ... I work/worked on aristainetos2 board update for 2024.07 (or later) ... and saw this
Failed to enable per_clk
warning when 2024.07 boots ... and bootlogo did not work anymore, as backlight is pwm driven on this hardware...
I bet, for all the boards which fail in the azure run with this patch, PWM do not work anymore... because CONFIG_CLK is not enabled for them!
I am really unhappy, that I have no automated U-Boot runtime tests anymore running on real hardware... setup with tbot was very easy, but had no time for looking at the bugs poping up...
Hmm... looking in git history, there comes this commit in my eyes:
commit 4eea76574062e9079b6a267229962d6ec0be910a Author: Tom Rini trini@konsulko.com Date: Sat Nov 19 18:45:18 2022 -0500
pwm: imx: Remove unused references to CONFIG_IMX6_PWM_PER_CLK On platforms that use DM_PWM, we do not need to define this value anymore, so remove it from config files. Signed-off-by: Tom Rini <trini@konsulko.com>which exactly removes for all breaking boards this define...
Should we revert this patch?
I just can say for the aristianetos2 board, that I readded exactly this define in my work for the port to currentmainline ... and with my patch, pwm and so bootlogo works again ...
We can't do a strict revert, no, the symbol is now CFG_IMX6_PWM_PER_CLK. But also, isn't this board using DM_PWM? And if so, that value isn't used?
Yes, of course, simple revert does not work.
Yes, the failing boards are converted to DM_PWM, but they do not work anymore as bfc778cb93a3 intrdocued clk framework functions, and the failing boards have no clk framwork enabled. So my patch simply readded the define for DM_PWM case.
bye, Heiko
On Wed, Aug 14, 2024 at 06:07:52AM +0200, Heiko Schocher wrote:
Hello Tom,
On 13.08.24 18:16, Tom Rini wrote:
On Tue, Aug 13, 2024 at 03:57:08PM +0200, Heiko Schocher wrote:
Hello Fabio, Tom,
On 13.08.24 14:27, Fabio Estevam wrote:
Hi Heiko,
On Wed, Aug 7, 2024 at 12:53 PM Heiko Schocher hs@denx.de wrote:
U-Boot 2024.07 drops on aristainetos2 board the following warning:
Failed to enable per_clkand bootlogo is not seen on LVDS display.
This patch uses old behaviour for systems without clock framework if CONFIG_CLK is not enabled.
Fixes: bfc778cb93a3 ("driver: pwm: pwm-imx: get and enable per/ipg clock using dm")
Signed-off-by: Heiko Schocher hs@denx.de
Changes in v2: use CONFIG_IS_ENABLED instead of IS_ENABLED as Anatolij suggested.
This causes build failures:
https://source.denx.de/u-boot/custodians/u-boot-imx/-/jobs/887503
Please fix and resend.
Hmm... yes ... I work/worked on aristainetos2 board update for 2024.07 (or later) ... and saw this
Failed to enable per_clk
warning when 2024.07 boots ... and bootlogo did not work anymore, as backlight is pwm driven on this hardware...
I bet, for all the boards which fail in the azure run with this patch, PWM do not work anymore... because CONFIG_CLK is not enabled for them!
I am really unhappy, that I have no automated U-Boot runtime tests anymore running on real hardware... setup with tbot was very easy, but had no time for looking at the bugs poping up...
Hmm... looking in git history, there comes this commit in my eyes:
commit 4eea76574062e9079b6a267229962d6ec0be910a Author: Tom Rini trini@konsulko.com Date: Sat Nov 19 18:45:18 2022 -0500
pwm: imx: Remove unused references to CONFIG_IMX6_PWM_PER_CLK On platforms that use DM_PWM, we do not need to define this value anymore, so remove it from config files. Signed-off-by: Tom Rini <trini@konsulko.com>which exactly removes for all breaking boards this define...
Should we revert this patch?
I just can say for the aristianetos2 board, that I readded exactly this define in my work for the port to currentmainline ... and with my patch, pwm and so bootlogo works again ...
We can't do a strict revert, no, the symbol is now CFG_IMX6_PWM_PER_CLK. But also, isn't this board using DM_PWM? And if so, that value isn't used?
Yes, of course, simple revert does not work.
Yes, the failing boards are converted to DM_PWM, but they do not work anymore as bfc778cb93a3 intrdocued clk framework functions, and the failing boards have no clk framwork enabled. So my patch simply readded the define for DM_PWM case.
OK. And why don't we just fix the missing clock migration next?
Hi Heiko,
On Tue, Aug 13, 2024 at 10:58 AM Heiko Schocher hs@denx.de wrote:
Should we revert this patch?
I just can say for the aristianetos2 board, that I readded exactly this define in my work for the port to currentmainline ... and with my patch, pwm and so bootlogo works again ...
What about the change below on top of your patch?
--- a/drivers/pwm/pwm-imx.c +++ b/drivers/pwm/pwm-imx.c @@ -14,6 +14,8 @@ #include <asm/io.h> #include <clk.h>
+#define IMX6_PWM_PER_CLK 66000000 + int pwm_config_internal(struct pwm_regs *pwm, unsigned long period_cycles, unsigned long duty_cycles, unsigned long prescale) { @@ -75,7 +77,7 @@ int pwm_imx_get_parms(int period_ns, int duty_ns, unsigned long *period_c, * value here as a define. Replace it when we have the clock * framework. */ - c = CFG_IMX6_PWM_PER_CLK; + c = IMX6_PWM_PER_CLK; c = c * period_ns; do_div(c, 1000000000); *period_c = c; @@ -162,7 +164,7 @@ int pwm_dm_imx_get_parms(struct imx_pwm_priv *priv, int period_ns, if (CONFIG_IS_ENABLED(CLK)) c = clk_get_rate(&priv->per_clk); else - c = CFG_IMX6_PWM_PER_CLK; + c = IMX6_PWM_PER_CLK;
c = c * period_ns; do_div(c, 1000000000);
Hello Fabio,
On 13.08.24 22:12, Fabio Estevam wrote:
Hi Heiko,
On Tue, Aug 13, 2024 at 10:58 AM Heiko Schocher hs@denx.de wrote:
Should we revert this patch?
I just can say for the aristianetos2 board, that I readded exactly this define in my work for the port to currentmainline ... and with my patch, pwm and so bootlogo works again ...
What about the change below on top of your patch?
--- a/drivers/pwm/pwm-imx.c +++ b/drivers/pwm/pwm-imx.c @@ -14,6 +14,8 @@ #include <asm/io.h> #include <clk.h>
+#define IMX6_PWM_PER_CLK 66000000
- int pwm_config_internal(struct pwm_regs *pwm, unsigned long period_cycles, unsigned long duty_cycles, unsigned long prescale) {
@@ -75,7 +77,7 @@ int pwm_imx_get_parms(int period_ns, int duty_ns, unsigned long *period_c, * value here as a define. Replace it when we have the clock * framework. */
c = CFG_IMX6_PWM_PER_CLK;
c = IMX6_PWM_PER_CLK; c = c * period_ns; do_div(c, 1000000000); *period_c = c;@@ -162,7 +164,7 @@ int pwm_dm_imx_get_parms(struct imx_pwm_priv *priv, int period_ns, if (CONFIG_IS_ENABLED(CLK)) c = clk_get_rate(&priv->per_clk); else
c = CFG_IMX6_PWM_PER_CLK;
c = IMX6_PWM_PER_CLK; c = c * period_ns; do_div(c, 1000000000);
Hmm.. yes, setting a default value is a good option, so I would add a check, if define is not set yet?
And we should simply let the name "CFG_IMX6_PWM_PER_CLK" ?
If okay, I can make a v3 if you want?
bye, Heiko
Hi Heiko,
On Wed, Aug 14, 2024 at 1:11 AM Heiko Schocher hs@denx.de wrote:
Hmm.. yes, setting a default value is a good option, so I would add a check, if define is not set yet?
And we should simply let the name "CFG_IMX6_PWM_PER_CLK" ?
Yes, that is OK.
If okay, I can make a v3 if you want?
Yes, please send a v3.
Thanks,
Fabio Estevam
participants (3)
-
Fabio Estevam -
Heiko Schocher -
Tom Rini