lkml.org 
[lkml]   [2019]   [Aug]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] input: keyboard: snvs_pwrkey: Send press and release event for i.MX6 S,DL and Q
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@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.

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

\
 
 \ /
  Last update: 2019-08-27 10:15    [W:0.045 / U:0.528 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site