[PATCH V2] Input: pm8941-pwrkey: add resin key capabilities

[PATCH V2] Input: pm8941-pwrkey: add resin key capabilities

[Tirupathi Reddy]

Add resin key support to handle different types of key events
defined in different platforms.

Signed-off-by: Tirupathi Reddy <tirupath@codeaurora.org>
---
 .../bindings/input/qcom,pm8941-pwrkey.txt          | 20 ++++++-
 drivers/input/misc/pm8941-pwrkey.c                 | 65 +++++++++++++++++++++-
 2 files changed, 83 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/input/qcom,pm8941-pwrkey.txt b/Documentation/devicetree/bindings/input/qcom,pm8941-pwrkey.txt
index 07bf55f..f499cf8 100644
--- a/Documentation/devicetree/bindings/input/qcom,pm8941-pwrkey.txt
+++ b/Documentation/devicetree/bindings/input/qcom,pm8941-pwrkey.txt
@@ -20,6 +20,14 @@ PROPERTIES
 		    defined by the binding document describing the node's
 		    interrupt parent.
 
+- interrupt-names:
+	Usage: required
+	Value type: <stringlist>
+	Definition: Interrupt names.  This list must match up 1-to-1 with the
+		    interrupts specified in the 'interrupts' property. "kpdpwr"
+		    must be specified and should be the first entry of the list.
+		    "resin" may be specified for some platforms.
+
 - debounce:
 	Usage: optional
 	Value type: <u32>
@@ -32,12 +40,22 @@ PROPERTIES
 	Definition: presence of this property indicates that the KPDPWR_N pin
 		    should be configured for pull up.
 
+- linux,code:
+	Usage: required for "resin" key
+	Value type: <u32>
+	Definition: The input key-code associated with the resin key.
+		    Use the linux event codes defined in
+		    include/dt-bindings/input/linux-event-codes.h
+
 EXAMPLE
 
 	pwrkey@800 {
 		compatible = "qcom,pm8941-pwrkey";
 		reg = <0x800>;
-		interrupts = <0x0 0x8 0 IRQ_TYPE_EDGE_BOTH>;
+		interrupts = <0x0 0x8 0 IRQ_TYPE_EDGE_BOTH>,
+			     <0x0 0x8 1 IRQ_TYPE_EDGE_BOTH>;
+		interrupt-names = "kpdpwr", "resin";
 		debounce = <15625>;
 		bias-pull-up;
+		linux,code = <KEY_VOLUMEDOWN>;
 	};
diff --git a/drivers/input/misc/pm8941-pwrkey.c b/drivers/input/misc/pm8941-pwrkey.c
index 18ad956..2fffc7c 100644
--- a/drivers/input/misc/pm8941-pwrkey.c
+++ b/drivers/input/misc/pm8941-pwrkey.c
@@ -28,6 +28,7 @@
 
 #define PON_RT_STS			0x10
 #define  PON_KPDPWR_N_SET		BIT(0)
+#define  PON_RESIN_N_SET		BIT(1)
 
 #define PON_PS_HOLD_RST_CTL		0x5a
 #define PON_PS_HOLD_RST_CTL2		0x5b
@@ -46,6 +47,7 @@
 struct pm8941_pwrkey {
 	struct device *dev;
 	int irq;
+	u32 resin_key_code;
 	u32 baseaddr;
 	struct regmap *regmap;
 	struct input_dev *input;
@@ -130,6 +132,24 @@ static irqreturn_t pm8941_pwrkey_irq(int irq, void *_data)
 	return IRQ_HANDLED;
 }
 
+static irqreturn_t pm8941_resinkey_irq(int irq, void *_data)
+{
+	struct pm8941_pwrkey *pwrkey = _data;
+	unsigned int sts;
+	int error;
+
+	error = regmap_read(pwrkey->regmap,
+			    pwrkey->baseaddr + PON_RT_STS, &sts);
+	if (error)
+		return IRQ_HANDLED;
+
+	input_report_key(pwrkey->input, pwrkey->resin_key_code,
+			 !!(sts & PON_RESIN_N_SET));
+	input_sync(pwrkey->input);
+
+	return IRQ_HANDLED;
+}
+
 static int __maybe_unused pm8941_pwrkey_suspend(struct device *dev)
 {
 	struct pm8941_pwrkey *pwrkey = dev_get_drvdata(dev);
@@ -153,12 +173,40 @@ static int __maybe_unused pm8941_pwrkey_resume(struct device *dev)
 static SIMPLE_DEV_PM_OPS(pm8941_pwr_key_pm_ops,
 			 pm8941_pwrkey_suspend, pm8941_pwrkey_resume);
 
+static int pm8941_resin_key_init(struct pm8941_pwrkey *pwrkey, int irq)
+{
+	int error;
+
+	/*
+	 * Get the standard-key parameters. This might not be
+	 * specified if there is no key mapping on the reset line.
+	 */
+	error = of_property_read_u32(pwrkey->dev->of_node, "linux,code",
+			&pwrkey->resin_key_code);
+	if (error) {
+		dev_err(pwrkey->dev, "failed to read key-code for resin key\n");
+		return error;
+	}
+
+	/* Register key configuration */
+	input_set_capability(pwrkey->input, EV_KEY, pwrkey->resin_key_code);
+
+	error = devm_request_threaded_irq(pwrkey->dev, irq, NULL,
+					  pm8941_resinkey_irq, IRQF_ONESHOT,
+					  "pm8941_resinkey", pwrkey);
+	if (error)
+		dev_err(pwrkey->dev, "failed requesting resin key IRQ: %d\n",
+			error);
+
+	return error;
+}
+
 static int pm8941_pwrkey_probe(struct platform_device *pdev)
 {
 	struct pm8941_pwrkey *pwrkey;
 	bool pull_up;
 	u32 req_delay;
-	int error;
+	int error, resin_irq;
 
 	if (of_property_read_u32(pdev->dev.of_node, "debounce", &req_delay))
 		req_delay = 15625;
@@ -241,6 +289,21 @@ static int pm8941_pwrkey_probe(struct platform_device *pdev)
 		return error;
 	}
 
+	resin_irq = platform_get_irq_byname(pdev, "resin");
+	if (resin_irq < 0 && resin_irq != -ENXIO) {
+		if (resin_irq != -EPROBE_DEFER)
+			dev_err(&pdev->dev, "failed to get resin irq\n");
+		return resin_irq;
+	} else if (resin_irq >= 0) {
+		/* resin key capabilities are defined in device node */
+		error = pm8941_resin_key_init(pwrkey, resin_irq);
+		if (error) {
+			dev_err(&pdev->dev, "failed resin key initialization: %d\n",
+				error);
+			return error;
+		}
+	}
+
 	error = input_register_device(pwrkey->input);
 	if (error) {
 		dev_err(&pdev->dev, "failed to register input device: %d\n",
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH V2] Input: pm8941-pwrkey: add resin key capabilities

[Trilok Soni]

Hi Tirupathi,

On 3/13/2018 1:35 AM, Tirupathi Reddy wrote:
> Add resin key support to handle different types of key events
> defined in different platforms.

Describe "resin"  in the commit text? or just call it same as power key 
or different purpose?

>   
> +static irqreturn_t pm8941_resinkey_irq(int irq, void *_data)
> +{
> +	struct pm8941_pwrkey *pwrkey = _data;
> +	unsigned int sts;
> +	int error;
> +
> +	error = regmap_read(pwrkey->regmap,
> +			    pwrkey->baseaddr + PON_RT_STS, &sts);
> +	if (error)
> +		return IRQ_HANDLED;

You may want to document here why you still return as IRQ_HANDLED on the 
error after reading the status register?

>   
> +	resin_irq = platform_get_irq_byname(pdev, "resin");
> +	if (resin_irq < 0 && resin_irq != -ENXIO) {
> +		if (resin_irq != -EPROBE_DEFER)
> +			dev_err(&pdev->dev, "failed to get resin irq\n");
> +		return resin_irq;
> +	} else if (resin_irq >= 0) {
> +		/* resin key capabilities are defined in device node */
> +		error = pm8941_resin_key_init(pwrkey, resin_irq);
> +		if (error) {
> +			dev_err(&pdev->dev, "failed resin key initialization: %d\n",
> +				error);
> +			return error;
> +		}
> +	}
> +

Any reason you don't want to create different driver for "resin" key? Is 
this going to be physically different key on the platform? Please 
describe why it needs to be part of the power key driver.

---Trilok Soni

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project

Re: [PATCH V2] Input: pm8941-pwrkey: add resin key capabilities

[Bjorn Andersson]

On Tue 13 Mar 01:35 PDT 2018, Tirupathi Reddy wrote:

> Add resin key support to handle different types of key events
> defined in different platforms.
> 
> Signed-off-by: Tirupathi Reddy <tirupath@codeaurora.org>
> ---
>  .../bindings/input/qcom,pm8941-pwrkey.txt          | 20 ++++++-
>  drivers/input/misc/pm8941-pwrkey.c                 | 65 +++++++++++++++++++++-
>  2 files changed, 83 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/input/qcom,pm8941-pwrkey.txt b/Documentation/devicetree/bindings/input/qcom,pm8941-pwrkey.txt
> index 07bf55f..f499cf8 100644
> --- a/Documentation/devicetree/bindings/input/qcom,pm8941-pwrkey.txt
> +++ b/Documentation/devicetree/bindings/input/qcom,pm8941-pwrkey.txt
> @@ -20,6 +20,14 @@ PROPERTIES
>  		    defined by the binding document describing the node's
>  		    interrupt parent.
>  
> +- interrupt-names:
> +	Usage: required
> +	Value type: <stringlist>
> +	Definition: Interrupt names.  This list must match up 1-to-1 with the
> +		    interrupts specified in the 'interrupts' property. "kpdpwr"
> +		    must be specified and should be the first entry of the list.
> +		    "resin" may be specified for some platforms.
> +
>  - debounce:
>  	Usage: optional
>  	Value type: <u32>
> @@ -32,12 +40,22 @@ PROPERTIES
>  	Definition: presence of this property indicates that the KPDPWR_N pin
>  		    should be configured for pull up.
>  
> +- linux,code:
> +	Usage: required for "resin" key
> +	Value type: <u32>
> +	Definition: The input key-code associated with the resin key.
> +		    Use the linux event codes defined in
> +		    include/dt-bindings/input/linux-event-codes.h

For completeness sake I think this should list both the key code for the
power key and for the RESIN key.

> +
>  EXAMPLE
>  
>  	pwrkey@800 {
>  		compatible = "qcom,pm8941-pwrkey";
>  		reg = <0x800>;
> -		interrupts = <0x0 0x8 0 IRQ_TYPE_EDGE_BOTH>;
> +		interrupts = <0x0 0x8 0 IRQ_TYPE_EDGE_BOTH>,
> +			     <0x0 0x8 1 IRQ_TYPE_EDGE_BOTH>;
> +		interrupt-names = "kpdpwr", "resin";
>  		debounce = <15625>;
>  		bias-pull-up;

Do we need to support defining the bias for the resin as well?

Perhaps it makes more sense to describe the RESIN key with a separate
compatible and define that in a node of its own (we can still implement
this in the same driver - if there's any chance of reuse...).

> +		linux,code = <KEY_VOLUMEDOWN>;
>  	};
> diff --git a/drivers/input/misc/pm8941-pwrkey.c b/drivers/input/misc/pm8941-pwrkey.c
> index 18ad956..2fffc7c 100644
> --- a/drivers/input/misc/pm8941-pwrkey.c
> +++ b/drivers/input/misc/pm8941-pwrkey.c
> @@ -28,6 +28,7 @@
>  
>  #define PON_RT_STS			0x10
>  #define  PON_KPDPWR_N_SET		BIT(0)
> +#define  PON_RESIN_N_SET		BIT(1)
>  
>  #define PON_PS_HOLD_RST_CTL		0x5a
>  #define PON_PS_HOLD_RST_CTL2		0x5b
> @@ -46,6 +47,7 @@
>  struct pm8941_pwrkey {
>  	struct device *dev;
>  	int irq;
> +	u32 resin_key_code;
>  	u32 baseaddr;
>  	struct regmap *regmap;
>  	struct input_dev *input;
> @@ -130,6 +132,24 @@ static irqreturn_t pm8941_pwrkey_irq(int irq, void *_data)
>  	return IRQ_HANDLED;
>  }
>  
> +static irqreturn_t pm8941_resinkey_irq(int irq, void *_data)
> +{
> +	struct pm8941_pwrkey *pwrkey = _data;
> +	unsigned int sts;
> +	int error;
> +
> +	error = regmap_read(pwrkey->regmap,
> +			    pwrkey->baseaddr + PON_RT_STS, &sts);

This needs to be broken because the line is 81 chars long, if you use a
shorter name for the return value (e.g. "ret") you don't need to do
this.

> +	if (error)
> +		return IRQ_HANDLED;
> +
> +	input_report_key(pwrkey->input, pwrkey->resin_key_code,
> +			 !!(sts & PON_RESIN_N_SET));

Put pwrkey->resin_key_code in a local variable named "key" (or even
key_code) and you don't need to line wrap this one.

> +	input_sync(pwrkey->input);
> +
> +	return IRQ_HANDLED;
> +}
> +

Regards,
Bjorn

Re: [PATCH V2] Input: pm8941-pwrkey: add resin key capabilities

[Dmitry Torokhov]

On Tue, Mar 13, 2018 at 07:40:48PM -0700, Bjorn Andersson wrote:
> On Tue 13 Mar 01:35 PDT 2018, Tirupathi Reddy wrote:
> 
> > Add resin key support to handle different types of key events
> > defined in different platforms.
> > 
> > Signed-off-by: Tirupathi Reddy <tirupath@codeaurora.org>
> > ---
> >  .../bindings/input/qcom,pm8941-pwrkey.txt          | 20 ++++++-
> >  drivers/input/misc/pm8941-pwrkey.c                 | 65 +++++++++++++++++++++-
> >  2 files changed, 83 insertions(+), 2 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/input/qcom,pm8941-pwrkey.txt b/Documentation/devicetree/bindings/input/qcom,pm8941-pwrkey.txt
> > index 07bf55f..f499cf8 100644
> > --- a/Documentation/devicetree/bindings/input/qcom,pm8941-pwrkey.txt
> > +++ b/Documentation/devicetree/bindings/input/qcom,pm8941-pwrkey.txt
> > @@ -20,6 +20,14 @@ PROPERTIES
> >  		    defined by the binding document describing the node's
> >  		    interrupt parent.
> >  
> > +- interrupt-names:
> > +	Usage: required
> > +	Value type: <stringlist>
> > +	Definition: Interrupt names.  This list must match up 1-to-1 with the
> > +		    interrupts specified in the 'interrupts' property. "kpdpwr"
> > +		    must be specified and should be the first entry of the list.
> > +		    "resin" may be specified for some platforms.
> > +
> >  - debounce:
> >  	Usage: optional
> >  	Value type: <u32>
> > @@ -32,12 +40,22 @@ PROPERTIES
> >  	Definition: presence of this property indicates that the KPDPWR_N pin
> >  		    should be configured for pull up.
> >  
> > +- linux,code:
> > +	Usage: required for "resin" key
> > +	Value type: <u32>
> > +	Definition: The input key-code associated with the resin key.
> > +		    Use the linux event codes defined in
> > +		    include/dt-bindings/input/linux-event-codes.h
> 
> For completeness sake I think this should list both the key code for the
> power key and for the RESIN key.
> 
> > +
> >  EXAMPLE
> >  
> >  	pwrkey@800 {
> >  		compatible = "qcom,pm8941-pwrkey";
> >  		reg = <0x800>;
> > -		interrupts = <0x0 0x8 0 IRQ_TYPE_EDGE_BOTH>;
> > +		interrupts = <0x0 0x8 0 IRQ_TYPE_EDGE_BOTH>,
> > +			     <0x0 0x8 1 IRQ_TYPE_EDGE_BOTH>;
> > +		interrupt-names = "kpdpwr", "resin";
> >  		debounce = <15625>;
> >  		bias-pull-up;
> 
> Do we need to support defining the bias for the resin as well?
> 
> Perhaps it makes more sense to describe the RESIN key with a separate
> compatible and define that in a node of its own (we can still implement
> this in the same driver - if there's any chance of reuse...).
> 
> > +		linux,code = <KEY_VOLUMEDOWN>;
> >  	};
> > diff --git a/drivers/input/misc/pm8941-pwrkey.c b/drivers/input/misc/pm8941-pwrkey.c
> > index 18ad956..2fffc7c 100644
> > --- a/drivers/input/misc/pm8941-pwrkey.c
> > +++ b/drivers/input/misc/pm8941-pwrkey.c
> > @@ -28,6 +28,7 @@
> >  
> >  #define PON_RT_STS			0x10
> >  #define  PON_KPDPWR_N_SET		BIT(0)
> > +#define  PON_RESIN_N_SET		BIT(1)
> >  
> >  #define PON_PS_HOLD_RST_CTL		0x5a
> >  #define PON_PS_HOLD_RST_CTL2		0x5b
> > @@ -46,6 +47,7 @@
> >  struct pm8941_pwrkey {
> >  	struct device *dev;
> >  	int irq;
> > +	u32 resin_key_code;
> >  	u32 baseaddr;
> >  	struct regmap *regmap;
> >  	struct input_dev *input;
> > @@ -130,6 +132,24 @@ static irqreturn_t pm8941_pwrkey_irq(int irq, void *_data)
> >  	return IRQ_HANDLED;
> >  }
> >  
> > +static irqreturn_t pm8941_resinkey_irq(int irq, void *_data)
> > +{
> > +	struct pm8941_pwrkey *pwrkey = _data;
> > +	unsigned int sts;
> > +	int error;
> > +
> > +	error = regmap_read(pwrkey->regmap,
> > +			    pwrkey->baseaddr + PON_RT_STS, &sts);
> 
> This needs to be broken because the line is 81 chars long, if you use a
> shorter name for the return value (e.g. "ret") you don't need to do
> this.

I prefer keeping it as "error" please.

> 
> > +	if (error)
> > +		return IRQ_HANDLED;
> > +
> > +	input_report_key(pwrkey->input, pwrkey->resin_key_code,
> > +			 !!(sts & PON_RESIN_N_SET));
> 
> Put pwrkey->resin_key_code in a local variable named "key" (or even
> key_code) and you don't need to line wrap this one.
> 
> > +	input_sync(pwrkey->input);
> > +
> > +	return IRQ_HANDLED;
> > +}
> > +
> 
> Regards,
> Bjorn

-- 
Dmitry

Re: [PATCH V2] Input: pm8941-pwrkey: add resin key capabilities

[Tirupathi Reddy T]

Hi,


On 3/14/2018 10:13 PM, Dmitry Torokhov wrote:
> On Tue, Mar 13, 2018 at 07:40:48PM -0700, Bjorn Andersson wrote:
>> On Tue 13 Mar 01:35 PDT 2018, Tirupathi Reddy wrote:
>>
>>> Add resin key support to handle different types of key events
>>> defined in different platforms.
>>>
>>> Signed-off-by: Tirupathi Reddy <tirupath@codeaurora.org>
>>> ---
>>>   .../bindings/input/qcom,pm8941-pwrkey.txt          | 20 ++++++-
>>>   drivers/input/misc/pm8941-pwrkey.c                 | 65 +++++++++++++++++++++-
>>>   2 files changed, 83 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/input/qcom,pm8941-pwrkey.txt b/Documentation/devicetree/bindings/input/qcom,pm8941-pwrkey.txt
>>> index 07bf55f..f499cf8 100644
>>> --- a/Documentation/devicetree/bindings/input/qcom,pm8941-pwrkey.txt
>>> +++ b/Documentation/devicetree/bindings/input/qcom,pm8941-pwrkey.txt
>>> @@ -20,6 +20,14 @@ PROPERTIES
>>>   		    defined by the binding document describing the node's
>>>   		    interrupt parent.
>>>   
>>> +- interrupt-names:
>>> +	Usage: required
>>> +	Value type: <stringlist>
>>> +	Definition: Interrupt names.  This list must match up 1-to-1 with the
>>> +		    interrupts specified in the 'interrupts' property. "kpdpwr"
>>> +		    must be specified and should be the first entry of the list.
>>> +		    "resin" may be specified for some platforms.
>>> +
>>>   - debounce:
>>>   	Usage: optional
>>>   	Value type: <u32>
>>> @@ -32,12 +40,22 @@ PROPERTIES
>>>   	Definition: presence of this property indicates that the KPDPWR_N pin
>>>   		    should be configured for pull up.
>>>   
>>> +- linux,code:
>>> +	Usage: required for "resin" key
>>> +	Value type: <u32>
>>> +	Definition: The input key-code associated with the resin key.
>>> +		    Use the linux event codes defined in
>>> +		    include/dt-bindings/input/linux-event-codes.h
>> For completeness sake I think this should list both the key code for the
>> power key and for the RESIN key.
Then it breaks existing DT nodes.
Since pwrkey functionality is fixed across platforms, "linux,code"
specification through device tree is not required for it.
>>> +
>>>   EXAMPLE
>>>   
>>>   	pwrkey@800 {
>>>   		compatible = "qcom,pm8941-pwrkey";
>>>   		reg = <0x800>;
>>> -		interrupts = <0x0 0x8 0 IRQ_TYPE_EDGE_BOTH>;
>>> +		interrupts = <0x0 0x8 0 IRQ_TYPE_EDGE_BOTH>,
>>> +			     <0x0 0x8 1 IRQ_TYPE_EDGE_BOTH>;
>>> +		interrupt-names = "kpdpwr", "resin";
>>>   		debounce = <15625>;
>>>   		bias-pull-up;
>> Do we need to support defining the bias for the resin as well?
Good point, will add this support in Patch V3. Also, the debounce 
configuration
in HW is common across power-key and resin.
>>
>> Perhaps it makes more sense to describe the RESIN key with a separate
>> compatible and define that in a node of its own (we can still implement
>> this in the same driver - if there's any chance of reuse...).
The PMIC peripheral/module called PON(Power  on) generates these key events.
Pwrkey(KPDPWR_N) is the primary supported key by this module, the others are
optional. The resin may be supported a "defined" key. How about using 
the below
logic for resin key without breaking the existing DTs as shown below:

     pwrkey@800 {
            compatible = "qcom,pm8941-pwrkey";
            reg = <0x800>;
            interrupts = <0x0 0x8 0 IRQ_TYPE_EDGE_BOTH>;
            debounce = <15625>;
            bias-pull-up;

            resin {
                    interrupts = <0x0 0x8 1 IRQ_TYPE_EDGE_BOTH>;
                    linux,code = <KEY_VOLUMEDOWN>;
                    bias-pull-up;
            };
     };

>>> +		linux,code = <KEY_VOLUMEDOWN>;
>>>   	};
>>> diff --git a/drivers/input/misc/pm8941-pwrkey.c b/drivers/input/misc/pm8941-pwrkey.c
>>> index 18ad956..2fffc7c 100644
>>> --- a/drivers/input/misc/pm8941-pwrkey.c
>>> +++ b/drivers/input/misc/pm8941-pwrkey.c
>>> @@ -28,6 +28,7 @@
>>>   
>>>   #define PON_RT_STS			0x10
>>>   #define  PON_KPDPWR_N_SET		BIT(0)
>>> +#define  PON_RESIN_N_SET		BIT(1)
>>>   
>>>   #define PON_PS_HOLD_RST_CTL		0x5a
>>>   #define PON_PS_HOLD_RST_CTL2		0x5b
>>> @@ -46,6 +47,7 @@
>>>   struct pm8941_pwrkey {
>>>   	struct device *dev;
>>>   	int irq;
>>> +	u32 resin_key_code;
>>>   	u32 baseaddr;
>>>   	struct regmap *regmap;
>>>   	struct input_dev *input;
>>> @@ -130,6 +132,24 @@ static irqreturn_t pm8941_pwrkey_irq(int irq, void *_data)
>>>   	return IRQ_HANDLED;
>>>   }
>>>   
>>> +static irqreturn_t pm8941_resinkey_irq(int irq, void *_data)
>>> +{
>>> +	struct pm8941_pwrkey *pwrkey = _data;
>>> +	unsigned int sts;
>>> +	int error;
>>> +
>>> +	error = regmap_read(pwrkey->regmap,
>>> +			    pwrkey->baseaddr + PON_RT_STS, &sts);
>> This needs to be broken because the line is 81 chars long, if you use a
>> shorter name for the return value (e.g. "ret") you don't need to do
>> this.
> I prefer keeping it as "error" please.
Okay. I will keep it as "error".
>>> +	if (error)
>>> +		return IRQ_HANDLED;
>>> +
>>> +	input_report_key(pwrkey->input, pwrkey->resin_key_code,
>>> +			 !!(sts & PON_RESIN_N_SET));
>> Put pwrkey->resin_key_code in a local variable named "key" (or even
>> key_code) and you don't need to line wrap this one.
Okay. I will address it in Patch V3.
>>> +	input_sync(pwrkey->input);
>>> +
>>> +	return IRQ_HANDLED;
>>> +}
>>> +
>> Regards,
>> Bjorn