Re: [PATCH] input: keyboard: snvs_pwrkey: Send press and release event for i.MX6 S,DL and Q

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2019-08-27 08:17, Robin Gong wrote:
On Fri, Aug 23, 2019 at 02:30:02PM +0200, Robin van der Gracht wrote:>
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.

For this driver to work as a regular powerkey with the older SoCs, we need 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 triggered
but only release interrupt on elder i.mx6 processors and that HW issue
fixed from
i.mx6sx.

ACK


Signed-off-by: Robin van der Gracht <robin@xxxxxxxxxxx>
---
 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.

ACK



static void imx_imx_snvs_check_for_events(struct timer_list *t) @@ -67,13
+85,23 @@ static irqreturn_t imx_snvs_pwrkey_interrupt(int irq, void
*dev_id)  {
 	struct platform_device *pdev = dev_id;
 	struct pwrkey_drv_data *pdata = platform_get_drvdata(pdev);
+	struct input_dev *input = pdata->input;
 	u32 lp_status;

-	pm_wakeup_event(pdata->input->dev.parent, 0);
+	pm_wakeup_event(input->dev.parent, 0);

 	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 == 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_for_events()
as before? That make code better to understand and less operation in ISR.

I placed it here to avoid the unnessesairy debounce delay (since thats already
implemented in hardware).

I do agree with your arguments so I'll move emitting the events to
imx_imx_snvs_check_for_events().

Is it ok if I keep the conditional, but instead of emitting the events,
schedule imx_imx_snvs_check_for_events() immidiatly to avoid the debounce,
or should I choose clarity over the 30 ms delay?

+		} else {
+			mod_timer(&pdata->check_timer,
+				jiffies + msecs_to_jiffies(DEBOUNCE_TIME));
+		}
+	}

 	/* 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);
 }

+static const struct of_device_id imx_snvs_pwrkey_ids[] = {
+	{
+		.compatible = "fsl,imx6sx-sec-v4.0-pwrkey",
+		.data = &imx_snvs_devtype[IMX6SX_SNVS],
+	}, {
+		.compatible = "fsl,imx6qdl-sec-v4.0-pwrkey",
+		.data = &imx_snvs_devtype[IMX6QDL_SNVS],
No ' IMX6QDL_SNVS ' defined in your patch or am I missing?

I added an enum 'imx_snvs_hwtype' that defines both IMX6SX_SNVS and IMX6QDL_SNVS.

+	},
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, imx_snvs_pwrkey_ids);
--
2.20.1



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux