Re: [PATCH] Input: snvs_pwrkey - Add clk handling

["Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>]

[-- Attachment #1: Type: text/plain, Size: 3210 bytes --]

Hello,

On Wed, Sep 22, 2021 at 11:43:00AM +0200, Uwe Kleine-König wrote:
> On i.MX7S and i.MX8M* (but not i.MX6*) the pwrkey device has an
> associated clock. Accessing the registers requires that this clock is
> enabled. Binding the driver on at least i.MX7S and i.MX8MP while not
> having the clock enabled results in a complete hang of the machine.
> (This usually only happens if snvs_pwrkey is built as a module and the
> rtc-snvs driver isn't already bound because at bootup the required clk
> is on and only gets disabled when the clk framework disables unused clks
> late during boot.)
> 
> This completes the fix in commit 135be16d3505 ("ARM: dts: imx7s: add
> snvs clock to pwrkey").
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

This patch fixes a hard machine hang that occurs on an i.MX8MP based
machine in ~10% of the boot ups. In my eyes it's suitable to be applied
before v5.14 even.

Any feedback on it?

Best regards
Uwe

> ---
>  drivers/input/keyboard/snvs_pwrkey.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/input/keyboard/snvs_pwrkey.c b/drivers/input/keyboard/snvs_pwrkey.c
> index 2f5e3ab5ed63..b79a559b8329 100644
> --- a/drivers/input/keyboard/snvs_pwrkey.c
> +++ b/drivers/input/keyboard/snvs_pwrkey.c
> @@ -3,6 +3,7 @@
>  // Driver for the IMX SNVS ON/OFF Power Key
>  // Copyright (C) 2015 Freescale Semiconductor, Inc. All Rights Reserved.
>  
> +#include <linux/clk.h>
>  #include <linux/device.h>
>  #include <linux/err.h>
>  #include <linux/init.h>
> @@ -99,6 +100,11 @@ static irqreturn_t imx_snvs_pwrkey_interrupt(int irq, void *dev_id)
>  	return IRQ_HANDLED;
>  }
>  
> +static void imx_snvs_pwrkey_disable_clk(void *data)
> +{
> +	clk_disable_unprepare(data);
> +}
> +
>  static void imx_snvs_pwrkey_act(void *pdata)
>  {
>  	struct pwrkey_drv_data *pd = pdata;
> @@ -111,6 +117,7 @@ static int imx_snvs_pwrkey_probe(struct platform_device *pdev)
>  	struct pwrkey_drv_data *pdata;
>  	struct input_dev *input;
>  	struct device_node *np;
> +	struct clk *clk;
>  	int error;
>  	u32 vid;
>  
> @@ -134,6 +141,19 @@ static int imx_snvs_pwrkey_probe(struct platform_device *pdev)
>  		dev_warn(&pdev->dev, "KEY_POWER without setting in dts\n");
>  	}
>  
> +	clk = devm_clk_get_optional(&pdev->dev, NULL);
> +	if (IS_ERR(clk))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(clk), "Failed to get clock\n");
> +
> +	error = clk_prepare_enable(clk);
> +	if (error)
> +		return dev_err_probe(&pdev->dev, PTR_ERR(clk), "Failed to enable clock\n");
> +
> +	error = devm_add_action_or_reset(&pdev->dev, imx_snvs_pwrkey_disable_clk, clk);
> +	if (error)
> +		return dev_err_probe(&pdev->dev, PTR_ERR(clk),
> +				     "Failed to register clock cleanup handler\n");
> +
>  	pdata->wakeup = of_property_read_bool(np, "wakeup-source");
>  
>  	pdata->irq = platform_get_irq(pdev, 0);
> 
> base-commit: 7d2a07b769330c34b4deabeed939325c77a7ec2f
> -- 
> 2.30.2
> 
> 
> 

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]