[PATCH master 1/2] clk: implement clk_get_optional helper

[PATCH master 1/2] clk: implement clk_get_optional helper

[Ahmad Fatoum]

Lack of clock can be ok at times and thus the clk API accepts NULL
arguments and treats them as no-op.

Linux provides a clk_get_optional function to simplify code with such
optional code, so let's provide an equivalent for barebox.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 include/linux/clk.h | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/include/linux/clk.h b/include/linux/clk.h
index 8509d5ece9d5..82022e78e39d 100644
--- a/include/linux/clk.h
+++ b/include/linux/clk.h
@@ -932,4 +932,23 @@ static inline void clk_bulk_disable(int num_clks,
 
 #endif
 
+/**
+ * clk_get_optional - lookup and obtain a reference to an optional clock
+ *		      producer.
+ * @dev: device for clock "consumer"
+ * @id: clock consumer ID
+ *
+ * Behaves the same as clk_get() except where there is no clock producer. In
+ * this case, instead of returning -ENOENT, the function returns NULL.
+ */
+static inline struct clk *clk_get_optional(struct device *dev, const char *id)
+{
+	struct clk *clk = clk_get(dev, id);
+
+	if (clk == ERR_PTR(-ENOENT))
+		return NULL;
+
+	return clk;
+}
+
 #endif
-- 
2.39.2

[PATCH master 2/2] pwm: imx: enable clocks during PWM register accesses

[Ahmad Fatoum]

This is a port of Linux commit 9f4c8f9607c3147d291b70c13dd01c738ed41faf:

| Author:     Anson Huang <anson.huang@nxp.com>
| AuthorDate: Wed Dec 19 05:24:58 2018 +0000
| Commit:     Thierry Reding <thierry.reding@gmail.com>
| CommitDate: Mon Dec 24 12:06:56 2018 +0100
|
|     pwm: imx: Add ipg clock operation
|
|     i.MX PWM module's ipg_clk_s is for PWM register access, on most of i.MX
|     SoCs, this ipg_clk_s is from system ipg clock or perclk which is always
|     enabled, but on i.MX7D, the ipg_clk_s is from PWM1_CLK_ROOT which is
|     controlled by CCGR132, that means the CCGR132 MUST be enabled first
|     before accessing PWM registers on i.MX7D. This patch adds ipg clock
|     operation to make sure register access successfully on i.MX7D and it
|     fixes Linux kernel boot up hang during PWM driver probe.
|
|     Fixes: 4a23e6ee9f69 ("ARM: dts: imx7d-sdb: Restore pwm backlight support")
|     Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
|     Signed-off-by: Thierry Reding <thierry.reding@gmail.com>

Unlike the Linux version, we make clk_ipg optional to reduce changes for
older SoCs.

This fixes system hang during PWM access on i.MX8M and presumably i.MX7.

Reported-by: Bastian Krause <bst@pengutronix.de>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/pwm/pwm-imx.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 42 insertions(+), 1 deletion(-)

diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c
index 2dc7e4cfd64d..486ca962f96d 100644
--- a/drivers/pwm/pwm-imx.c
+++ b/drivers/pwm/pwm-imx.c
@@ -36,7 +36,7 @@
 #define MX3_PWMCR_EN              (1 << 0)
 
 struct imx_chip {
-	struct clk	*clk_per;
+	struct clk	*clk_per, *clk_ipg;
 
 	void __iomem	*mmio_base;
 
@@ -93,14 +93,42 @@ static void imx_pwm_set_enable_v1(struct pwm_chip *chip, bool enable)
 	writel(val, imx->mmio_base + MX1_PWMC);
 }
 
+static int imx_pwm_clk_enable_v2(struct imx_chip *imx)
+{
+	int ret;
+
+	ret = clk_enable(imx->clk_ipg);
+	if (ret)
+		return ret;
+
+	ret = clk_prepare_enable(imx->clk_per);
+	if (ret) {
+		clk_disable_unprepare(imx->clk_ipg);
+		return ret;
+	}
+
+	return 0;
+}
+
+static void imx_pwm_clk_disable_v2(struct imx_chip *imx)
+{
+	clk_disable_unprepare(imx->clk_per);
+	clk_disable_unprepare(imx->clk_ipg);
+}
+
 static int imx_pwm_config_v2(struct pwm_chip *chip,
 		int duty_ns, int period_ns)
 {
 	struct imx_chip *imx = to_imx_chip(chip);
 	unsigned long long c;
 	unsigned long period_cycles, duty_cycles, prescale;
+	int ret;
 	u32 cr;
 
+	ret = imx_pwm_clk_enable_v2(imx);
+	if (ret)
+		return ret;
+
 	c = clk_get_rate(imx->clk_per);
 	c = c * period_ns;
 	do_div(c, 1000000000);
@@ -134,6 +162,8 @@ static int imx_pwm_config_v2(struct pwm_chip *chip,
 
 	writel(cr, imx->mmio_base + MX3_PWMCR);
 
+	imx_pwm_clk_disable_v2(imx);
+
 	return 0;
 }
 
@@ -141,6 +171,11 @@ static void imx_pwm_set_enable_v2(struct pwm_chip *chip, bool enable)
 {
 	struct imx_chip *imx = to_imx_chip(chip);
 	u32 val;
+	int ret;
+
+	ret = imx_pwm_clk_enable_v2(imx);
+	if (WARN_ON(ret))
+		return;
 
 	val = readl(imx->mmio_base + MX3_PWMCR);
 
@@ -150,6 +185,8 @@ static void imx_pwm_set_enable_v2(struct pwm_chip *chip, bool enable)
 		val &= ~MX3_PWMCR_EN;
 
 	writel(val, imx->mmio_base + MX3_PWMCR);
+
+	imx_pwm_clk_disable_v2(imx);
 }
 
 static int imx_pwm_apply(struct pwm_chip *chip, const struct pwm_state *state)
@@ -215,6 +252,10 @@ static int imx_pwm_probe(struct device *dev)
 
 	imx = xzalloc(sizeof(*imx));
 
+	imx->clk_ipg = clk_get_optional(dev, "ipg");
+	if (IS_ERR(imx->clk_ipg))
+		return PTR_ERR(imx->clk_ipg);
+
 	imx->clk_per = clk_get(dev, "per");
 	if (IS_ERR(imx->clk_per))
 		return PTR_ERR(imx->clk_per);
-- 
2.39.2

Re: [PATCH master 2/2] pwm: imx: enable clocks during PWM register accesses

[Marco Felsch]

On 23-09-04, Ahmad Fatoum wrote:
> This is a port of Linux commit 9f4c8f9607c3147d291b70c13dd01c738ed41faf:
> 
> | Author:     Anson Huang <anson.huang@nxp.com>
> | AuthorDate: Wed Dec 19 05:24:58 2018 +0000
> | Commit:     Thierry Reding <thierry.reding@gmail.com>
> | CommitDate: Mon Dec 24 12:06:56 2018 +0100
> |
> |     pwm: imx: Add ipg clock operation
> |
> |     i.MX PWM module's ipg_clk_s is for PWM register access, on most of i.MX
> |     SoCs, this ipg_clk_s is from system ipg clock or perclk which is always
> |     enabled, but on i.MX7D, the ipg_clk_s is from PWM1_CLK_ROOT which is
> |     controlled by CCGR132, that means the CCGR132 MUST be enabled first
> |     before accessing PWM registers on i.MX7D. This patch adds ipg clock
> |     operation to make sure register access successfully on i.MX7D and it
> |     fixes Linux kernel boot up hang during PWM driver probe.
> |
> |     Fixes: 4a23e6ee9f69 ("ARM: dts: imx7d-sdb: Restore pwm backlight support")
> |     Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> |     Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
> 
> Unlike the Linux version, we make clk_ipg optional to reduce changes for
> older SoCs.
> 
> This fixes system hang during PWM access on i.MX8M and presumably i.MX7.
> 
> Reported-by: Bastian Krause <bst@pengutronix.de>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

Reviewed-by: Marco Felsch <m.felsch@pengutronix.de>

Re: [PATCH master 1/2] clk: implement clk_get_optional helper

[Marco Felsch]

On 23-09-04, Ahmad Fatoum wrote:
> Lack of clock can be ok at times and thus the clk API accepts NULL
> arguments and treats them as no-op.
> 
> Linux provides a clk_get_optional function to simplify code with such
> optional code, so let's provide an equivalent for barebox.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

Reviewed-by: Marco Felsch <m.felsch@pengutronix.de>

Re: [PATCH master 2/2] pwm: imx: enable clocks during PWM register accesses

[Bastian Krause]

On 9/4/23 17:49, Ahmad Fatoum wrote:
> This is a port of Linux commit 9f4c8f9607c3147d291b70c13dd01c738ed41faf:
> 
> | Author:     Anson Huang <anson.huang@nxp.com>
> | AuthorDate: Wed Dec 19 05:24:58 2018 +0000
> | Commit:     Thierry Reding <thierry.reding@gmail.com>
> | CommitDate: Mon Dec 24 12:06:56 2018 +0100
> |
> |     pwm: imx: Add ipg clock operation
> |
> |     i.MX PWM module's ipg_clk_s is for PWM register access, on most of i.MX
> |     SoCs, this ipg_clk_s is from system ipg clock or perclk which is always
> |     enabled, but on i.MX7D, the ipg_clk_s is from PWM1_CLK_ROOT which is
> |     controlled by CCGR132, that means the CCGR132 MUST be enabled first
> |     before accessing PWM registers on i.MX7D. This patch adds ipg clock
> |     operation to make sure register access successfully on i.MX7D and it
> |     fixes Linux kernel boot up hang during PWM driver probe.
> |
> |     Fixes: 4a23e6ee9f69 ("ARM: dts: imx7d-sdb: Restore pwm backlight support")
> |     Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> |     Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
> 
> Unlike the Linux version, we make clk_ipg optional to reduce changes for
> older SoCs.
> 
> This fixes system hang during PWM access on i.MX8M and presumably i.MX7.
> 
> Reported-by: Bastian Krause <bst@pengutronix.de>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

The port of the Linux commit looks good. I'm using this to enable some
PWM LEDs. Unfortunately the LEDs don't remain on with this, because
we're missing Linux commit 15d4dbd601591 ("pwm: imx27: Fix clock
handling in pwm_imx27_apply()").

See below for a suggested change.

> ---
>   drivers/pwm/pwm-imx.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 42 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c
> index 2dc7e4cfd64d..486ca962f96d 100644
> --- a/drivers/pwm/pwm-imx.c
> +++ b/drivers/pwm/pwm-imx.c
> @@ -36,7 +36,7 @@
>   #define MX3_PWMCR_EN              (1 << 0)
>   
>   struct imx_chip {
> -	struct clk	*clk_per;
> +	struct clk	*clk_per, *clk_ipg;
>   
>   	void __iomem	*mmio_base;
>   
> @@ -93,14 +93,42 @@ static void imx_pwm_set_enable_v1(struct pwm_chip *chip, bool enable)
>   	writel(val, imx->mmio_base + MX1_PWMC);
>   }
>   
> +static int imx_pwm_clk_enable_v2(struct imx_chip *imx)
> +{
> +	int ret;
> +
> +	ret = clk_enable(imx->clk_ipg);
> +	if (ret)
> +		return ret;
> +
> +	ret = clk_prepare_enable(imx->clk_per);
> +	if (ret) {
> +		clk_disable_unprepare(imx->clk_ipg);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static void imx_pwm_clk_disable_v2(struct imx_chip *imx)
> +{
> +	clk_disable_unprepare(imx->clk_per);
> +	clk_disable_unprepare(imx->clk_ipg);
> +}
> +
>   static int imx_pwm_config_v2(struct pwm_chip *chip,
>   		int duty_ns, int period_ns)
>   {
>   	struct imx_chip *imx = to_imx_chip(chip);
>   	unsigned long long c;
>   	unsigned long period_cycles, duty_cycles, prescale;
> +	int ret;
>   	u32 cr;
>   
> +	ret = imx_pwm_clk_enable_v2(imx);
> +	if (ret)
> +		return ret;
> +
>   	c = clk_get_rate(imx->clk_per);
>   	c = c * period_ns;
>   	do_div(c, 1000000000);
> @@ -134,6 +162,8 @@ static int imx_pwm_config_v2(struct pwm_chip *chip,
>   
>   	writel(cr, imx->mmio_base + MX3_PWMCR);
>   
> +	imx_pwm_clk_disable_v2(imx);
> +

I think this should be:

if (!chip->state.p_enable)
         imx_pwm_clk_disable_v2(imx);

>   	return 0;
>   }
>   
> @@ -141,6 +171,11 @@ static void imx_pwm_set_enable_v2(struct pwm_chip *chip, bool enable)
>   {
>   	struct imx_chip *imx = to_imx_chip(chip);
>   	u32 val;
> +	int ret;
> +
> +	ret = imx_pwm_clk_enable_v2(imx);
> +	if (WARN_ON(ret))
> +		return;
>   
>   	val = readl(imx->mmio_base + MX3_PWMCR);
>   
> @@ -150,6 +185,8 @@ static void imx_pwm_set_enable_v2(struct pwm_chip *chip, bool enable)
>   		val &= ~MX3_PWMCR_EN;
>   
>   	writel(val, imx->mmio_base + MX3_PWMCR);
> +
> +	imx_pwm_clk_disable_v2(imx);

I think this should be:

if (!enable)
         imx_pwm_clk_disable_v2(imx);

With these changes, the clocks remain on if the PWM is running and my
PWM LEDs keep their state.

With the suggested changes:

   Tested-by: Bastian Krause <bst@pengutronix.de>

Regards,
Bastian

>   }
>   
>   static int imx_pwm_apply(struct pwm_chip *chip, const struct pwm_state *state)
> @@ -215,6 +252,10 @@ static int imx_pwm_probe(struct device *dev)
>   
>   	imx = xzalloc(sizeof(*imx));
>   
> +	imx->clk_ipg = clk_get_optional(dev, "ipg");
> +	if (IS_ERR(imx->clk_ipg))
> +		return PTR_ERR(imx->clk_ipg);
> +
>   	imx->clk_per = clk_get(dev, "per");
>   	if (IS_ERR(imx->clk_per))
>   		return PTR_ERR(imx->clk_per);

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |