Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Horia Geanta <horia.geanta@nxp.com>
To:     =?iso-8859-2?Q?Andr=E9_Draszik?= <git@andred.net>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
CC:     Anson Huang <anson.huang@nxp.com>,
        Dmitry Torokhov <dmitry.torokhov@gmail.com>,
        Aymen Sghaier <aymen.sghaier@nxp.com>,
        Herbert Xu <herbert@gondor.apana.org.au>,
        "David S. Miller" <davem@davemloft.net>,
        Rob Herring <robh+dt@kernel.org>,
        Mark Rutland <mark.rutland@arm.com>,
        "linux-crypto@vger.kernel.org" <linux-crypto@vger.kernel.org>,
        "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
        "linux-input@vger.kernel.org" <linux-input@vger.kernel.org>,
        Robin Gong <yibin.gong@nxp.com>,
        dl-linux-imx <linux-imx@nxp.com>
Subject: Re: [PATCH 2/3] Input: snvs_pwrkey - enable snvs clock as needed
Thread-Topic: [PATCH 2/3] Input: snvs_pwrkey - enable snvs clock as needed
Thread-Index: AQHV164yHhXoESmTiUqzHJu+3DHGMA==
Date:   Fri, 7 Feb 2020 08:10:22 +0000
Message-ID: <VI1PR0402MB3485EC2F82DDE52DC5CA0795981C0@VI1PR0402MB3485.eurprd04.prod.outlook.com>
References: <20200130204516.4760-1-git@andred.net>
 <20200130204516.4760-2-git@andred.net>
Accept-Language: en-US
Content-Language: en-US
received-spf: None (protection.outlook.com: nxp.com does not designate
 permitted sender hosts)
Content-Type: text/plain; charset="iso-8859-2"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 8580592e-cffa-4e3d-d394-08d7aba52de4
X-MS-Exchange-CrossTenant-originalarrivaltime: 07 Feb 2020 08:10:22.3029
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: Xb7V0opFCYwMk2eE4+4oWGRLomzYNRU0HQQnyoOW28xmUxdsbRHOSjCbWA2Z3PXxsZ0MDZ40JCigQ3+cVkeoow==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR0402MB2831
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 1/30/2020 10:45 PM, Andr=E9 Draszik wrote:=0A=
> At the moment, enabling this driver without the SNVS RTC driver=0A=
> being active will hang the kernel as soon as the power button=0A=
> is pressed.=0A=
> =0A=
> The reason is that in that case the SNVS isn't enabled, and=0A=
> any attempt to read the SNVS registers will simply hang forever.=0A=
> =0A=
> Ensure the clock is enabled (during the interrupt handler) to=0A=
> make this driver work.=0A=
> =0A=
> Also see commit 7f8993995410 ("drivers/rtc/rtc-snvs: add clock support")=
=0A=
> and commit edb190cb1734=0A=
> ("rtc: snvs: make sure clock is enabled for interrupt handle")=0A=
> for similar updates to the snvs rtc driver.=0A=
> =0A=
> Signed-off-by: Andr=E9 Draszik <git@andred.net>=0A=
> Cc: Anson Huang <Anson.Huang@nxp.com>=0A=
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>=0A=
> Cc: "Horia Geant=E3" <horia.geanta@nxp.com>=0A=
> Cc: Aymen Sghaier <aymen.sghaier@nxp.com>=0A=
> Cc: Herbert Xu <herbert@gondor.apana.org.au>=0A=
> Cc: "David S. Miller" <davem@davemloft.net>=0A=
> Cc: Rob Herring <robh+dt@kernel.org>=0A=
> Cc: Mark Rutland <mark.rutland@arm.com>=0A=
> Cc: linux-crypto@vger.kernel.org=0A=
> Cc: devicetree@vger.kernel.org=0A=
> Cc: linux-input@vger.kernel.org=0A=
> ---=0A=
>  drivers/input/keyboard/snvs_pwrkey.c | 27 +++++++++++++++++++++++++++=0A=
>  1 file changed, 27 insertions(+)=0A=
> =0A=
> diff --git a/drivers/input/keyboard/snvs_pwrkey.c b/drivers/input/keyboar=
d/snvs_pwrkey.c=0A=
> index 2f5e3ab5ed63..c29711d8735c 100644=0A=
> --- a/drivers/input/keyboard/snvs_pwrkey.c=0A=
> +++ b/drivers/input/keyboard/snvs_pwrkey.c=0A=
> @@ -16,6 +16,7 @@=0A=
>  #include <linux/of_address.h>=0A=
>  #include <linux/platform_device.h>=0A=
>  #include <linux/pm_wakeirq.h>=0A=
> +#include <linux/clk.h>=0A=
>  #include <linux/mfd/syscon.h>=0A=
>  #include <linux/regmap.h>=0A=
>  =0A=
> @@ -38,6 +39,7 @@ struct pwrkey_drv_data {=0A=
>  	int wakeup;=0A=
>  	struct timer_list check_timer;=0A=
>  	struct input_dev *input;=0A=
> +	struct clk *clk;=0A=
>  	u8 minor_rev;=0A=
>  };=0A=
>  =0A=
> @@ -72,6 +74,9 @@ static irqreturn_t imx_snvs_pwrkey_interrupt(int irq, v=
oid *dev_id)=0A=
>  	struct input_dev *input =3D pdata->input;=0A=
>  	u32 lp_status;=0A=
>  =0A=
> +	if (pdata->clk)=0A=
> +		clk_enable(pdata->clk);=0A=
> +=0A=
clk framework handles NULL pointers internally, the check is redundant.=0A=
=0A=
>  	pm_wakeup_event(input->dev.parent, 0);=0A=
>  =0A=
>  	regmap_read(pdata->snvs, SNVS_LPSR_REG, &lp_status);=0A=
> @@ -96,6 +101,9 @@ static irqreturn_t imx_snvs_pwrkey_interrupt(int irq, =
void *dev_id)=0A=
>  	/* clear SPO status */=0A=
>  	regmap_write(pdata->snvs, SNVS_LPSR_REG, SNVS_LPSR_SPO);=0A=
>  =0A=
> +	if (pdata->clk)=0A=
> +		clk_disable(pdata->clk);=0A=
> +=0A=
>  	return IRQ_HANDLED;=0A=
>  }=0A=
>  =0A=
> @@ -140,6 +148,25 @@ static int imx_snvs_pwrkey_probe(struct platform_dev=
ice *pdev)=0A=
>  	if (pdata->irq < 0)=0A=
>  		return -EINVAL;=0A=
>  =0A=
> +	pdata->clk =3D devm_clk_get(&pdev->dev, "snvs-pwrkey");=0A=
> +	if (IS_ERR(pdata->clk)) {=0A=
> +		pdata->clk =3D NULL;=0A=
Using devm_clk_get_optional() would simplify error handling.=0A=
=0A=
> +	} else {=0A=
> +		error =3D clk_prepare_enable(pdata->clk);=0A=
> +		if (error) {=0A=
> +			dev_err(&pdev->dev,=0A=
> +				"Could not prepare or enable the snvs clock\n");=0A=
> +			return error;=0A=
> +		}=0A=
> +		error =3D devm_add_action_or_reset(&pdev->dev,=0A=
> +				(void(*)(void *))clk_disable_unprepare,=0A=
> +				pdata->clk);=0A=
> +		if (error) {=0A=
> +			dev_err(&pdev->dev, "failed to add reset action on 'snvs-pwrkey'");=
=0A=
> +			return error;=0A=
> +		}=0A=
> +	}=0A=
> +=0A=
>  	regmap_read(pdata->snvs, SNVS_HPVIDR1_REG, &vid);=0A=
>  	pdata->minor_rev =3D vid & 0xff;=0A=
>  =0A=
> =0A=