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:   Robin Gong <yibin.gong@nxp.com>
To:     Robin van der Gracht <robin@protonic.nl>
CC:     Rob Herring <robh+dt@kernel.org>,
        Mark Rutland <mark.rutland@arm.com>,
        Shawn Guo <shawnguo@kernel.org>,
        Sascha Hauer <s.hauer@pengutronix.de>,
        Pengutronix Kernel Team <kernel@pengutronix.de>,
        Fabio Estevam <festevam@gmail.com>,
        dl-linux-imx <linux-imx@nxp.com>,
        Dmitry Torokhov <dmitry.torokhov@gmail.com>,
        "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
        "linux-arm-kernel@lists.infradead.org" 
        <linux-arm-kernel@lists.infradead.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-input@vger.kernel.org" <linux-input@vger.kernel.org>,
        Adam Ford <aford173@gmail.com>
Subject: RE: [PATCH] input: keyboard: snvs_pwrkey: Send press and release
 event for i.MX6 S,DL and Q
Thread-Topic: [PATCH] input: keyboard: snvs_pwrkey: Send press and release
 event for i.MX6 S,DL and Q
Thread-Index: AQHVWa5ZFk23j3f7OEWRDzQvDh9iTqcOfq3w
Date:   Tue, 27 Aug 2019 06:17:52 +0000
Message-ID: <VE1PR04MB6638754916357F551502102589A00@VE1PR04MB6638.eurprd04.prod.outlook.com>
References: <20190823123002.10448-1-robin@protonic.nl>
In-Reply-To: <20190823123002.10448-1-robin@protonic.nl>
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="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 8f6b6e0a-19a3-4317-a10d-08d72ab64b40
X-MS-Exchange-CrossTenant-originalarrivaltime: 27 Aug 2019 06:17:52.9726
 (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: +/XtjRz0VVFRYrVLkWVXbf5U9772++bqT/D0GKEaAlazE1VzXnmPk4R0u7FwMP2PYVVctoPw1adsF/GeShXuaA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: VE1PR04MB6638
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 Fri, Aug 23, 2019 at 02:30:02PM +0200, Robin van der Gracht wrote:>=20
> The older generation i.MX6 processors send a powerdown request interrupt
> if the powerkey is released before a hard shutdown (5 second press). This
> should allow software to bring down the SoC safely.
>=20
> For this driver to work as a regular powerkey with the older SoCs, we nee=
d to
> send a keypress AND release when we get the powerdown request interrupt.
Please clarify here more clearly that because there is NO press interrupt t=
riggered
but only release interrupt on elder i.mx6 processors and that HW issue fixe=
d from
i.mx6sx.
>=20
> Signed-off-by: Robin van der Gracht <robin@protonic.nl>
> ---
>  arch/arm/boot/dts/imx6qdl.dtsi       |  2 +-
>  arch/arm/boot/dts/imx6sll.dtsi       |  2 +-
>  arch/arm/boot/dts/imx6sx.dtsi        |  2 +-
>  arch/arm/boot/dts/imx6ul.dtsi        |  2 +-
>  arch/arm/boot/dts/imx7s.dtsi         |  2 +-
As Shawn talked, please keep the original "fsl,sec-v4.0-pwrkey", just add
'imx6qdl-snvs-pwrkey' for elder i.mx6 processor i.mx6q/dl/sl, thus no need
to touch other newer processor's dts.

>=20
>  static void imx_imx_snvs_check_for_events(struct timer_list *t) @@ -67,1=
3
> +85,23 @@ static irqreturn_t imx_snvs_pwrkey_interrupt(int irq, void
> *dev_id)  {
>  	struct platform_device *pdev =3D dev_id;
>  	struct pwrkey_drv_data *pdata =3D platform_get_drvdata(pdev);
> +	struct input_dev *input =3D pdata->input;
>  	u32 lp_status;
>=20
> -	pm_wakeup_event(pdata->input->dev.parent, 0);
> +	pm_wakeup_event(input->dev.parent, 0);
>=20
>  	regmap_read(pdata->snvs, SNVS_LPSR_REG, &lp_status);
> -	if (lp_status & SNVS_LPSR_SPO)
> -		mod_timer(&pdata->check_timer, jiffies +
> msecs_to_jiffies(DEBOUNCE_TIME));
> +	if (lp_status & SNVS_LPSR_SPO) {
> +		if (pdata->hwtype =3D=3D IMX6QDL_SNVS) {
> +			input_report_key(input, pdata->keycode, 1);
> +			input_report_key(input, pdata->keycode, 0);
> +			input_sync(input);
> +			pm_relax(input->dev.parent);
Could you move the above input event report steps into imx_imx_snvs_check_f=
or_events()
as before? That make code better to understand and less operation in ISR.
> +		} else {
> +			mod_timer(&pdata->check_timer,
> +				jiffies + msecs_to_jiffies(DEBOUNCE_TIME));
> +		}
> +	}
>=20
>  	/* clear SPO status */
>  	regmap_write(pdata->snvs, SNVS_LPSR_REG, SNVS_LPSR_SPO); @@
> -88,11 +116,24 @@ static void imx_snvs_pwrkey_act(void *pdata)
>  	del_timer_sync(&pd->check_timer);
>  }
>=20
> +static const struct of_device_id imx_snvs_pwrkey_ids[] =3D {
> +	{
> +		.compatible =3D "fsl,imx6sx-sec-v4.0-pwrkey",
> +		.data =3D &imx_snvs_devtype[IMX6SX_SNVS],
> +	}, {
> +		.compatible =3D "fsl,imx6qdl-sec-v4.0-pwrkey",
> +		.data =3D &imx_snvs_devtype[IMX6QDL_SNVS],
No ' IMX6QDL_SNVS ' defined in your patch or am I missing?
> +	},
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, imx_snvs_pwrkey_ids);
> --
> 2.20.1