Linux Input/HID development
 help / color / mirror / Atom feed
* Re: [PATCH v4 8/8] dt-bindings: mfd: dlg,da9063: Convert da9062 to json-schema
From: Krzysztof Kozlowski @ 2023-12-07  8:43 UTC (permalink / raw)
  To: Biju Das, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Lee Jones
  Cc: Support Opensource, Rafael J. Wysocki, Daniel Lezcano, Zhang Rui,
	Lukasz Luba, Steve Twiss, linux-input, devicetree, linux-pm,
	Geert Uytterhoeven, Prabhakar Mahadev Lad, Biju Das,
	linux-renesas-soc
In-Reply-To: <20231206155740.5278-9-biju.das.jz@bp.renesas.com>

On 06/12/2023 16:57, Biju Das wrote:
> Convert the da9062 PMIC device tree binding documentation to json-schema.
> 
> Update the example to match reality.
> 
> While at it, update description with link to product information.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
> v3->v4:
>  * Split the thermal binding patch separate.
>  * Updated the description.
> v2->v3:
>  * Fixed bot errors related to MAINTAINERS entry, invalid doc
>    references and thermal examples by merging patch#4.
> v2:
>  * New patch
> ---
>  .../bindings/input/dlg,da9062-onkey.yaml      |   3 +-
>  .../devicetree/bindings/mfd/da9062.txt        | 124 ------------
>  .../devicetree/bindings/mfd/dlg,da9063.yaml   | 186 +++++++++++++++++-
>  .../bindings/thermal/dlg,da9062-thermal.yaml  |   2 +-
>  4 files changed, 183 insertions(+), 132 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/mfd/da9062.txt
> 
> diff --git a/Documentation/devicetree/bindings/input/dlg,da9062-onkey.yaml b/Documentation/devicetree/bindings/input/dlg,da9062-onkey.yaml
> index 4cff91f4bd34..18b6a3f02c07 100644
> --- a/Documentation/devicetree/bindings/input/dlg,da9062-onkey.yaml
> +++ b/Documentation/devicetree/bindings/input/dlg,da9062-onkey.yaml
> @@ -11,8 +11,7 @@ maintainers:
>  
>  description: |
>    This module is part of the DA9061/DA9062/DA9063. For more details about entire
> -  DA9062 and DA9061 chips see Documentation/devicetree/bindings/mfd/da9062.txt
> -  For DA9063 see Documentation/devicetree/bindings/mfd/dlg,da9063.yaml
> +  DA906{1,2,3} chips see Documentation/devicetree/bindings/mfd/dlg,da9063.yaml
>  
>    This module provides the KEY_POWER event.
>  
> diff --git a/Documentation/devicetree/bindings/mfd/da9062.txt b/Documentation/devicetree/bindings/mfd/da9062.txt
> deleted file mode 100644
> index c8a7f119ac84..000000000000
> --- a/Documentation/devicetree/bindings/mfd/da9062.txt
> +++ /dev/null
> @@ -1,124 +0,0 @@
> -* Dialog DA9062 Power Management Integrated Circuit (PMIC)
> -
> -Product information for the DA9062 and DA9061 devices can be found here:
> -- https://www.dialog-semiconductor.com/products/da9062
> -- https://www.dialog-semiconductor.com/products/da9061
> -
> -The DA9062 PMIC consists of:
> -
> -Device                   Supply Names    Description
> -------                   ------------    -----------
> -da9062-regulator        :               : LDOs & BUCKs
> -da9062-rtc              :               : Real-Time Clock
> -da9062-onkey            :               : On Key
> -da9062-watchdog         :               : Watchdog Timer
> -da9062-thermal          :               : Thermal
> -da9062-gpio             :               : GPIOs
> -
> -The DA9061 PMIC consists of:
> -
> -Device                   Supply Names    Description
> -------                   ------------    -----------
> -da9062-regulator        :               : LDOs & BUCKs
> -da9062-onkey            :               : On Key
> -da9062-watchdog         :               : Watchdog Timer
> -da9062-thermal          :               : Thermal
> -
> -======
> -
> -Required properties:
> -
> -- compatible : Should be
> -    "dlg,da9062" for DA9062
> -    "dlg,da9061" for DA9061
> -- reg : Specifies the I2C slave address (this defaults to 0x58 but it can be
> -  modified to match the chip's OTP settings).
> -
> -Optional properties:
> -
> -- gpio-controller : Marks the device as a gpio controller.
> -- #gpio-cells     : Should be two. The first cell is the pin number and the
> -                    second cell is used to specify the gpio polarity.
> -
> -See Documentation/devicetree/bindings/gpio/gpio.txt for further information on
> -GPIO bindings.
> -
> -- interrupts : IRQ line information.
> -- interrupt-controller
> -
> -See Documentation/devicetree/bindings/interrupt-controller/interrupts.txt for
> -further information on IRQ bindings.
> -
> -Sub-nodes:
> -
> -- regulators : This node defines the settings for the LDOs and BUCKs.
> -  The DA9062 regulators are bound using their names listed below:
> -
> -    buck1    : BUCK_1
> -    buck2    : BUCK_2
> -    buck3    : BUCK_3
> -    buck4    : BUCK_4
> -    ldo1     : LDO_1
> -    ldo2     : LDO_2
> -    ldo3     : LDO_3
> -    ldo4     : LDO_4
> -
> -  The DA9061 regulators are bound using their names listed below:
> -
> -    buck1    : BUCK_1
> -    buck2    : BUCK_2
> -    buck3    : BUCK_3
> -    ldo1     : LDO_1
> -    ldo2     : LDO_2
> -    ldo3     : LDO_3
> -    ldo4     : LDO_4
> -
> -  The component follows the standard regulator framework and the bindings
> -  details of individual regulator device can be found in:
> -  Documentation/devicetree/bindings/regulator/regulator.txt
> -
> -  regulator-initial-mode may be specified for buck regulators using mode values
> -  from include/dt-bindings/regulator/dlg,da9063-regulator.h.
> -
> -- rtc : This node defines settings required for the Real-Time Clock associated
> -  with the DA9062. There are currently no entries in this binding, however
> -  compatible = "dlg,da9062-rtc" should be added if a node is created.
> -
> -- onkey : See ../input/dlg,da9062-onkey.yaml
> -
> -- watchdog: See ../watchdog/dlg,da9062-watchdog.yaml
> -
> -- thermal : See ../thermal/dlg,da9062-thermal.yaml
> -
> -Example:
> -
> -	pmic0: da9062@58 {
> -		compatible = "dlg,da9062";
> -		reg = <0x58>;
> -		interrupt-parent = <&gpio6>;
> -		interrupts = <11 IRQ_TYPE_LEVEL_LOW>;
> -		interrupt-controller;
> -
> -		rtc {
> -			compatible = "dlg,da9062-rtc";
> -		};
> -
> -		regulators {
> -			DA9062_BUCK1: buck1 {
> -				regulator-name = "BUCK1";
> -				regulator-min-microvolt = <300000>;
> -				regulator-max-microvolt = <1570000>;
> -				regulator-min-microamp = <500000>;
> -				regulator-max-microamp = <2000000>;
> -				regulator-initial-mode = <DA9063_BUCK_MODE_SYNC>;
> -				regulator-boot-on;
> -			};
> -			DA9062_LDO1: ldo1 {
> -				regulator-name = "LDO_1";
> -				regulator-min-microvolt = <900000>;
> -				regulator-max-microvolt = <3600000>;
> -				regulator-boot-on;
> -			};
> -		};
> -	};
> -
> diff --git a/Documentation/devicetree/bindings/mfd/dlg,da9063.yaml b/Documentation/devicetree/bindings/mfd/dlg,da9063.yaml
> index 676b4f2566ae..54bb23dbc73f 100644
> --- a/Documentation/devicetree/bindings/mfd/dlg,da9063.yaml
> +++ b/Documentation/devicetree/bindings/mfd/dlg,da9063.yaml
> @@ -4,7 +4,7 @@
>  $id: http://devicetree.org/schemas/mfd/dlg,da9063.yaml#
>  $schema: http://devicetree.org/meta-schemas/core.yaml#
>  
> -title: Dialog DA9063/DA9063L Power Management Integrated Circuit (PMIC)
> +title: Dialog DA906{3L,3,2,1} Power Management Integrated Circuit (PMIC)
>  
>  maintainers:
>    - Steve Twiss <stwiss.opensource@diasemi.com>
> @@ -17,10 +17,17 @@ description: |
>    moment where all voltage monitors are disabled. Next, as da9063 only supports
>    UV *and* OV monitoring, both must be set to the same severity and value
>    (0: disable, 1: enable).
> +  Product information for the DA906{3L,3,2,1} devices can be found here:
> +  - https://www.dialog-semiconductor.com/products/da9063l
> +  - https://www.dialog-semiconductor.com/products/da9063
> +  - https://www.dialog-semiconductor.com/products/da9062
> +  - https://www.dialog-semiconductor.com/products/da9061
>  
>  properties:
>    compatible:
>      enum:
> +      - dlg,da9061
> +      - dlg,da9062
>        - dlg,da9063
>        - dlg,da9063l
>  
> @@ -35,6 +42,19 @@ properties:
>    "#interrupt-cells":
>      const: 2
>  
> +  gpio:

Old binding did not have such node and nothing in commit msg explained
changes from pure conversion.

> +    type: object
> +    $ref: /schemas/gpio/gpio.yaml#

?!? Why? First: It's always selected. Second, so you have two gpio
controllers? And original binding had 0? Why this is not explained at
all? Open the binding and look at its properties.


> +    unevaluatedProperties: false
> +    properties:
> +      compatible:
> +        const: dlg,da9062-gpio
> +
> +  gpio-controller: true

And here is the second gpio-controller...

> +
> +  "#gpio-cells":
> +    const: 2
> +
>    onkey:
>      $ref: /schemas/input/dlg,da9062-onkey.yaml
>  
> @@ -42,7 +62,7 @@ properties:
>      type: object
>      additionalProperties: false
>      patternProperties:
> -      "^(ldo([1-9]|1[01])|bcore([1-2]|s-merged)|b(pro|mem|io|peri)|bmem-bio-merged)$":
> +      "^(ldo([1-9]|1[01])|bcore([1-2]|s-merged)|b(pro|mem|io|peri)|bmem-bio-merged|buck[1-4])$":
>          $ref: /schemas/regulator/regulator.yaml
>          unevaluatedProperties: false
>  
> @@ -52,7 +72,12 @@ properties:
>      unevaluatedProperties: false
>      properties:
>        compatible:
> -        const: dlg,da9063-rtc
> +        enum:
> +          - dlg,da9063-rtc
> +          - dlg,da9062-rtc
> +
> +  thermal:
> +    $ref: /schemas/thermal/dlg,da9062-thermal.yaml
>  
>    watchdog:
>      $ref: /schemas/watchdog/dlg,da9062-watchdog.yaml
> @@ -60,8 +85,65 @@ properties:
>  required:
>    - compatible
>    - reg
> -  - interrupts
> -  - interrupt-controller
> +
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - dlg,da9063
> +              - dlg,da9063l
> +    then:
> +      properties:
> +        thermal: false
> +        gpio: false
> +        gpio-controller: false
> +        "#gpio-cells": false
> +        regulators:
> +          patternProperties:
> +            "^buck[1-4]$": false
> +      required:
> +        - interrupts
> +        - interrupt-controller
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - dlg,da9062
> +    then:
> +      properties:
> +        regulators:
> +          patternProperties:
> +            "^(ldo([5-9]|10|11)|bcore([1-2]|s-merged)|b(pro|mem|io|peri)|bmem-bio-merged)$": false
> +      required:
> +        - gpio
> +        - onkey
> +        - rtc
> +        - thermal
> +        - watchdog
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - dlg,da9061
> +    then:
> +      properties:
> +        gpio: false
> +        gpio-controller: false
> +        "#gpio-cells": false
> +        regulators:
> +          patternProperties:
> +            "^(ldo([5-9]|10|11)|bcore([1-2]|s-merged)|b(pro|mem|io|peri)|bmem-bio-merged|buck4)$": false
> +        rtc: false
> +      required:
> +        - onkey
> +        - thermal
> +        - watchdog
>  
>  additionalProperties: false
>  
> @@ -118,4 +200,98 @@ examples:
>          };
>        };
>      };
> +
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    #include <dt-bindings/regulator/dlg,da9063-regulator.h>
> +    i2c {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +      pmic@58 {
> +        compatible = "dlg,da9062";
> +        reg = <0x58>;
> +        #interrupt-cells = <2>;
> +        interrupt-parent = <&gpio1>;
> +        interrupts = <2 IRQ_TYPE_LEVEL_LOW>;
> +        interrupt-controller;
> +
> +        gpio {
> +          compatible = "dlg,da9062-gpio";
> +          status = "disabled";

Why?
Where are gpio-controller and cells? For this node and for parent? Why
this example is incomplete?

> +        };
> +
> +        onkey {
> +          compatible = "dlg,da9062-onkey";
> +        };


Best regards,
Krzysztof


^ permalink raw reply

* Re: [PATCH v4 0/8] Convert DA906{1,2} bindings to json-schema
From: Krzysztof Kozlowski @ 2023-12-07  8:38 UTC (permalink / raw)
  To: Biju Das, Lee Jones, Dmitry Torokhov, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: Support Opensource, Rafael J. Wysocki, Daniel Lezcano, Zhang Rui,
	Lukasz Luba, Steve Twiss, linux-input, devicetree, linux-pm,
	Geert Uytterhoeven, Prabhakar Mahadev Lad, Biju Das,
	linux-renesas-soc
In-Reply-To: <20231206155740.5278-1-biju.das.jz@bp.renesas.com>

On 06/12/2023 16:57, Biju Das wrote:
> Convert the below bindings to json-schema
> 1) DA906{1,2} mfd bindings
> 2) DA906{1,2,3} onkey bindings
> 3) DA906{1,2,3} thermal bindings
> 
> Also add fallback for DA9061 watchdog device and document
> DA9063 watchdog device.

Please explain here dependencies and make clear merging strategy. The
patches cannot be taken independently.

Best regards,
Krzysztof


^ permalink raw reply

* Re: [PATCH v4 5/8] dt-bindings: input: Convert da906{1,2,3} onkey to json-schema
From: Krzysztof Kozlowski @ 2023-12-07  8:37 UTC (permalink / raw)
  To: Biju Das, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: Support Opensource, linux-input, devicetree, Geert Uytterhoeven,
	Prabhakar Mahadev Lad, Biju Das, linux-renesas-soc
In-Reply-To: <20231206155740.5278-6-biju.das.jz@bp.renesas.com>

On 06/12/2023 16:57, Biju Das wrote:
> Convert the da906{1,2,3} onkey device tree binding documentation to
> json-schema.
> 
> Update MAINTAINERS entries, description and onkey property by
> referring to dlg,da9062-onkey binding file.
> 

...

> +---
> +$id: http://devicetree.org/schemas/input/dlg,da9062-onkey.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Dialog DA9061/62/63 OnKey Module
> +
> +maintainers:
> +  - Biju Das <biju.das.jz@bp.renesas.com>
> +
> +description: |
> +  This module is part of the DA9061/DA9062/DA9063. For more details about entire
> +  DA9062 and DA9061 chips see Documentation/devicetree/bindings/mfd/da9062.txt
> +  For DA9063 see Documentation/devicetree/bindings/mfd/dlg,da9063.yaml
> +
> +  This module provides the KEY_POWER event.
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - items:

Drop items


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Best regards,
Krzysztof


^ permalink raw reply

* [PATCH 0/2] Input: uinput - Multiple concurrency fixes in ff request handling
From: Vicki Pfau @ 2023-12-07  6:34 UTC (permalink / raw)
  To: Dmitry Torokhov, linux-input; +Cc: Vicki Pfau

While investigating a report of a process hanging on submitting ff data to a
uinput-derived evdev handle I uncovered several issues regarding cross-thread
concurrency.

The first fix is simply making waiting on the completion object interruptible.
Without this, the submitting process cannot be interrupted, meaning it has to
either wait for the uinput-controlling process to read the data, or the timeout
being reached. While this is the usual flow, it being uninterruptible means
that if the uinput-controlling process is misbehaving, the submitting process
cannot be killed, suspended, or otherwise interrupted until the timeout is
reached, which could take an annoyingly long time for users.

The second fix is probably more controversial, and I'm unsure if it's really
the best way to solve this issue. Namely, there exists a small, but
reproducible window where closing a uinput device on the uinput side and
uploading ff data via an evdev handle in a separate process will lead to a
deadlock: the uinput ioctl will claim the mutex, flush requests, then try to
close the input device, which then tries to claim the evdev mutex. However,
when uploading the ff data, the evdev mutex will be claimed, try to claim the
uinput mutex, and hang indefinitely, leading to a deadlock. Since it can never
claim the uinput mutex, it doesn't notice that it should exit early, but since
it can't get the mutex at all, it can't release the evdev mutex.

My approach to solving this involves temporarily releasing the mutex after
flushing requests, allowing the upload to claim the mutex, then closing the
input device without the mutex being held, and finally reclaim the mutex to
rebalance the mutex_unlock later on.

I spent quite a while investigating other approaches while trying to come up
with the least hacky and simplest way to fix this. However, a proper fix might
be more involved and have to touch other subsystems, namely evdev, in which
case I would defer to Dmitry for a better fix, as he's a lot more familiar with
these subsystems.

I also suspect that there's a race condition with uinput_dev_event, as most
call sites are protected by the uinput device mutex, but not all of them.
Namely, it can be called via the input device's event function pointer, which
has no idea that the uinput mutex exists.  However, I haven't demonstrated that
there is actually an issue here, so I haven't attempted to fix it.

Vicki Pfau (2):
  Input: uinput - Allow uinput_request_submit wait interrupting
  Input: uinput - Release mutex while unregistering input device

 drivers/input/misc/uinput.c | 34 ++++++++++++++++++++++++++++++++--
 1 file changed, 32 insertions(+), 2 deletions(-)

-- 
2.43.0


^ permalink raw reply

* [PATCH 2/2] Input: uinput - Release mutex while unregistering input device
From: Vicki Pfau @ 2023-12-07  6:34 UTC (permalink / raw)
  To: Dmitry Torokhov, linux-input; +Cc: Vicki Pfau
In-Reply-To: <20231207063406.556770-1-vi@endrift.com>

Any pending requests may be holding a mutex from its own subsystem, e.g.
evdev, while waiting to be able to claim the uinput device mutex.
However, unregistering the device may try to claim that mutex, leading
to a deadlock. To prevent this from happening, we need to temporarily
give up the lock before calling input_unregister_device.

Fixes: e8b95728f724 ("Input: uinput - avoid FF flush when destroying device")
Signed-off-by: Vicki Pfau <vi@endrift.com>
---
 drivers/input/misc/uinput.c | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
index 0330e72798db..ac6e5baa2093 100644
--- a/drivers/input/misc/uinput.c
+++ b/drivers/input/misc/uinput.c
@@ -296,17 +296,34 @@ static void uinput_destroy_device(struct uinput_device *udev)
 	udev->state = UIST_NEW_DEVICE;
 
 	if (dev) {
+		udev->dev = NULL;
 		name = dev->name;
 		phys = dev->phys;
 		if (old_state == UIST_CREATED) {
 			uinput_flush_requests(udev);
+
+			/*
+			 * Any pending requests may be holding a mutex from its
+			 * own subsystem, e.g. evdev, while waiting to be able
+			 * to claim the uinput device mutex. However,
+			 * unregistering the device may try to claim that
+			 * mutex, leading to a deadlock. To prevent this from
+			 * happening, we need to temporarily give up the lock.
+			 *
+			 * Since this function is only called immediately
+			 * before the caller exits the critical section without
+			 * doing any further operations on the device, this
+			 * is safe and we can immediately reclaim the mutex
+			 * when done so the unlock is still balanced.
+			 */
+			mutex_unlock(&udev->mutex);
 			input_unregister_device(dev);
+			mutex_lock(&udev->mutex);
 		} else {
 			input_free_device(dev);
 		}
 		kfree(name);
 		kfree(phys);
-		udev->dev = NULL;
 	}
 }
 
@@ -745,7 +762,16 @@ static int uinput_release(struct inode *inode, struct file *file)
 {
 	struct uinput_device *udev = file->private_data;
 
+	int retval;
+
+	retval = mutex_lock_interruptible(&udev->mutex);
+	if (retval)
+		return retval;
+
 	uinput_destroy_device(udev);
+
+	mutex_unlock(&udev->mutex);
+
 	kfree(udev);
 
 	return 0;
-- 
2.43.0


^ permalink raw reply related

* [PATCH 1/2] Input: uinput - Allow uinput_request_submit wait interrupting
From: Vicki Pfau @ 2023-12-07  6:34 UTC (permalink / raw)
  To: Dmitry Torokhov, linux-input; +Cc: Vicki Pfau
In-Reply-To: <20231207063406.556770-1-vi@endrift.com>

Currently, uinput_request_submit will only fail if the request wait times out.
However, in other places this wait is interruptable, and in this specific
location it can lead to issues, such as causing system suspend to hang until
the request times out. Since the timeout is so long, this can cause the
appearance of a total system freeze. Making the wait interruptable resolves
this and possibly further issues.

Signed-off-by: Vicki Pfau <vi@endrift.com>
---
 drivers/input/misc/uinput.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
index d98212d55108..0330e72798db 100644
--- a/drivers/input/misc/uinput.c
+++ b/drivers/input/misc/uinput.c
@@ -183,7 +183,11 @@ static int uinput_request_submit(struct uinput_device *udev,
 	if (retval)
 		goto out;
 
-	if (!wait_for_completion_timeout(&request->done, 30 * HZ)) {
+	retval = wait_for_completion_interruptible_timeout(&request->done, 30 * HZ);
+	if (retval == -ERESTARTSYS)
+		goto out;
+
+	if (!retval) {
 		retval = -ETIMEDOUT;
 		goto out;
 	}
-- 
2.43.0


^ permalink raw reply related

* Re: [PATCH v4 3/3] Input: Add TouchNetix axiom i2c touchscreen driver
From: Dan Carpenter @ 2023-12-07  5:28 UTC (permalink / raw)
  To: oe-kbuild, Kamel Bouhara, Dmitry Torokhov, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Henrik Rydberg, linux-input,
	linux-kernel, devicetree, Marco Felsch, Jeff LaBundy
  Cc: lkp, oe-kbuild-all, catalin.popescu, mark.satterthwaite, bartp,
	hannah.rossiter, Thomas Petazzoni, Gregory Clement,
	bsp-development.geo, Kamel Bouhara
In-Reply-To: <20231204140505.2838916-4-kamel.bouhara@bootlin.com>

Hi Kamel,

kernel test robot noticed the following build warnings:

https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Kamel-Bouhara/dt-bindings-vendor-prefixes-Add-TouchNetix-AS/20231204-222017
base:   https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
patch link:    https://lore.kernel.org/r/20231204140505.2838916-4-kamel.bouhara%40bootlin.com
patch subject: [PATCH v4 3/3] Input: Add TouchNetix axiom i2c touchscreen driver
config: i386-randconfig-r071-20231206 (https://download.01.org/0day-ci/archive/20231207/202312070608.vxtNAYhk-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce: (https://download.01.org/0day-ci/archive/20231207/202312070608.vxtNAYhk-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202312070608.vxtNAYhk-lkp@intel.com/

smatch warnings:
drivers/input/touchscreen/touchnetix_axiom.c:565 axiom_i2c_probe() warn: passing zero to 'PTR_ERR'

vim +/PTR_ERR +565 drivers/input/touchscreen/touchnetix_axiom.c

a9f3450f06a026 Kamel Bouhara 2023-12-04  536  static int axiom_i2c_probe(struct i2c_client *client)
a9f3450f06a026 Kamel Bouhara 2023-12-04  537  {
a9f3450f06a026 Kamel Bouhara 2023-12-04  538  	struct device *dev = &client->dev;
a9f3450f06a026 Kamel Bouhara 2023-12-04  539  	struct input_dev *input_dev;
a9f3450f06a026 Kamel Bouhara 2023-12-04  540  	struct axiom_data *ts;
a9f3450f06a026 Kamel Bouhara 2023-12-04  541  	u32 startup_delay_ms;
a9f3450f06a026 Kamel Bouhara 2023-12-04  542  	u32 poll_interval;
a9f3450f06a026 Kamel Bouhara 2023-12-04  543  	int target;
a9f3450f06a026 Kamel Bouhara 2023-12-04  544  	int error;
a9f3450f06a026 Kamel Bouhara 2023-12-04  545  
a9f3450f06a026 Kamel Bouhara 2023-12-04  546  	ts = devm_kzalloc(dev, sizeof(*ts), GFP_KERNEL);
a9f3450f06a026 Kamel Bouhara 2023-12-04  547  	if (!ts)
a9f3450f06a026 Kamel Bouhara 2023-12-04  548  		return -ENOMEM;
a9f3450f06a026 Kamel Bouhara 2023-12-04  549  
a9f3450f06a026 Kamel Bouhara 2023-12-04  550  	ts->client = client;
a9f3450f06a026 Kamel Bouhara 2023-12-04  551  	i2c_set_clientdata(client, ts);
a9f3450f06a026 Kamel Bouhara 2023-12-04  552  	ts->dev = dev;
a9f3450f06a026 Kamel Bouhara 2023-12-04  553  
a9f3450f06a026 Kamel Bouhara 2023-12-04  554  	ts->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
a9f3450f06a026 Kamel Bouhara 2023-12-04  555  	if (IS_ERR(ts->reset_gpio))
a9f3450f06a026 Kamel Bouhara 2023-12-04  556  		return dev_err_probe(dev, PTR_ERR(ts->reset_gpio), "failed to get reset GPIO\n");
a9f3450f06a026 Kamel Bouhara 2023-12-04  557  
a9f3450f06a026 Kamel Bouhara 2023-12-04  558  	if (ts->reset_gpio)
a9f3450f06a026 Kamel Bouhara 2023-12-04  559  		axiom_reset(ts->reset_gpio);
a9f3450f06a026 Kamel Bouhara 2023-12-04  560  
a9f3450f06a026 Kamel Bouhara 2023-12-04  561  	ts->vddi = devm_regulator_get_optional(dev, "VDDI");
a9f3450f06a026 Kamel Bouhara 2023-12-04  562  	if (!IS_ERR(ts->vddi)) {
a9f3450f06a026 Kamel Bouhara 2023-12-04  563  		error = regulator_enable(ts->vddi);
a9f3450f06a026 Kamel Bouhara 2023-12-04  564  		if (error)
a9f3450f06a026 Kamel Bouhara 2023-12-04 @565  			return dev_err_probe(&client->dev, PTR_ERR(ts->vddi),
                                                                                                   ^^^^^^^^^^^^^^^^^^
s/PTR_ERR(ts->vddi)/error/

a9f3450f06a026 Kamel Bouhara 2023-12-04  566  					     "Failed to enable VDDI regulator\n");
a9f3450f06a026 Kamel Bouhara 2023-12-04  567  	}
a9f3450f06a026 Kamel Bouhara 2023-12-04  568  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki


^ permalink raw reply

* Re: [PATCH v2] HID: intel-ish-hid: ipc: Rework EHL OOB wakeup
From: Kai-Heng Feng @ 2023-12-07  2:44 UTC (permalink / raw)
  To: srinivas pandruvada
  Cc: jikos, benjamin.tissoires, linux-pm, linux-pci, Jian Hui Lee,
	Even Xu, linux-input, linux-kernel
In-Reply-To: <cc6c162407c69c53ec256bf887a0384361dd0516.camel@linux.intel.com>

On Tue, Dec 5, 2023 at 1:50 AM srinivas pandruvada
<srinivas.pandruvada@linux.intel.com> wrote:
>
> Hi Kai,
>
> Sorry for he delay in getting back on this. I have a question below:
>
> On Wed, 2023-11-08 at 14:19 +0200, Kai-Heng Feng wrote:
> > Since PCI core and ACPI core already handles PCI PME wake and GPE
> > wake
> > when the device has wakeup capability, use device_init_wakeup() to
> > let
> > them do the wakeup setting work.
> >
> > Also add a shutdown callback which uses pci_prepare_to_sleep() to let
> > PCI and ACPI set OOB wakeup for S5.
> >
> > Cc: Jian Hui Lee <jianhui.lee@canonical.com>
> > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> > ---
> > v2:
> >  Rebase on ("HID: intel-ish-hid: ipc: Disable and reenable ACPI GPE
> > bit")
> >
> >  drivers/hid/intel-ish-hid/ipc/pci-ish.c | 67 ++++++-----------------
> > --
> >  1 file changed, 15 insertions(+), 52 deletions(-)
> >
> > diff --git a/drivers/hid/intel-ish-hid/ipc/pci-ish.c
> > b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
> > index 710fda5f19e1..65e7eeb2fa64 100644
> > --- a/drivers/hid/intel-ish-hid/ipc/pci-ish.c
> > +++ b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
> > @@ -119,50 +119,6 @@ static inline bool ish_should_leave_d0i3(struct
> > pci_dev *pdev)
> >         return !pm_resume_via_firmware() || pdev->device ==
> > CHV_DEVICE_ID;
> >  }
> >
> > -static int enable_gpe(struct device *dev)
> > -{
> > -#ifdef CONFIG_ACPI
> > -       acpi_status acpi_sts;
> > -       struct acpi_device *adev;
> > -       struct acpi_device_wakeup *wakeup;
> > -
> > -       adev = ACPI_COMPANION(dev);
> > -       if (!adev) {
> > -               dev_err(dev, "get acpi handle failed\n");
> > -               return -ENODEV;
> > -       }
> > -       wakeup = &adev->wakeup;
> > -
> > -       /*
> > -        * Call acpi_disable_gpe(), so that reference count
> > -        * gpe_event_info->runtime_count doesn't overflow.
> > -        * When gpe_event_info->runtime_count = 0, the call
> > -        * to acpi_disable_gpe() simply return.
> > -        */
> > -       acpi_disable_gpe(wakeup->gpe_device, wakeup->gpe_number);
> > -
> > -       acpi_sts = acpi_enable_gpe(wakeup->gpe_device, wakeup-
> > >gpe_number);
> > -       if (ACPI_FAILURE(acpi_sts)) {
> > -               dev_err(dev, "enable ose_gpe failed\n");
> > -               return -EIO;
> > -       }
> > -
> > -       return 0;
> > -#else
> > -       return -ENODEV;
> > -#endif
> > -}
> > -
> > -static void enable_pme_wake(struct pci_dev *pdev)
> > -{
> > -       if ((pci_pme_capable(pdev, PCI_D0) ||
> > -            pci_pme_capable(pdev, PCI_D3hot) ||
> > -            pci_pme_capable(pdev, PCI_D3cold)) && !enable_gpe(&pdev-
> > >dev)) {
> > -               pci_pme_active(pdev, true);
> > -               dev_dbg(&pdev->dev, "ish ipc driver pme wake
> > enabled\n");
> > -       }
> > -}
> > -
> >  /**
> >   * ish_probe() - PCI driver probe callback
> >   * @pdev:      pci device
> > @@ -233,7 +189,7 @@ static int ish_probe(struct pci_dev *pdev, const
> > struct pci_device_id *ent)
> >
> >         /* Enable PME for EHL */
> >         if (pdev->device == EHL_Ax_DEVICE_ID)
> > -               enable_pme_wake(pdev);
> > +               device_init_wakeup(dev, true);
>
> For apple to apple comparison, which path will call
> https://elixir.bootlin.com/linux/latest/C/ident/__pci_enable_wake
> which will call pci_pme_active()?

Here's the flow:
device_shutdown()
  pci_device_shutdown()
    ish_shutdown()
      pci_prepare_to_sleep()
        __pci_enable_wake()
          pci_pme_active()
            __pci_pme_active()

Kai-Heng

>
> Thanks,
> Srinivas
>
> >
> >         ret = ish_init(ishtp);
> >         if (ret)
> > @@ -256,6 +212,19 @@ static void ish_remove(struct pci_dev *pdev)
> >         ish_device_disable(ishtp_dev);
> >  }
> >
> > +
> > +/**
> > + * ish_shutdown() - PCI driver shutdown callback
> > + * @pdev:      pci device
> > + *
> > + * This function sets up wakeup for S5
> > + */
> > +static void ish_shutdown(struct pci_dev *pdev)
> > +{
> > +       if (pdev->device == EHL_Ax_DEVICE_ID)
> > +               pci_prepare_to_sleep(pdev);
> > +}
> > +
> >  static struct device __maybe_unused *ish_resume_device;
> >
> >  /* 50ms to get resume response */
> > @@ -378,13 +347,6 @@ static int __maybe_unused ish_resume(struct
> > device *device)
> >         struct pci_dev *pdev = to_pci_dev(device);
> >         struct ishtp_device *dev = pci_get_drvdata(pdev);
> >
> > -       /* add this to finish power flow for EHL */
> > -       if (dev->pdev->device == EHL_Ax_DEVICE_ID) {
> > -               pci_set_power_state(pdev, PCI_D0);
> > -               enable_pme_wake(pdev);
> > -               dev_dbg(dev->devc, "set power state to D0 for
> > ehl\n");
> > -       }
> > -
> >         ish_resume_device = device;
> >         dev->resume_flag = 1;
> >
> > @@ -400,6 +362,7 @@ static struct pci_driver ish_driver = {
> >         .id_table = ish_pci_tbl,
> >         .probe = ish_probe,
> >         .remove = ish_remove,
> > +       .shutdown = ish_shutdown,
> >         .driver.pm = &ish_pm_ops,
> >  };
> >
>

^ permalink raw reply

* [PATCH] irq: Resolve that mask_irq/unmask_irq may not be called in pairs
From: xiongxin @ 2023-12-07  1:40 UTC (permalink / raw)
  To: tglx, jikos, benjamin.tissoires; +Cc: linux-input, xiongxin, stable, Riwen Lu

When an interrupt controller uses a function such as handle_level_irq()
as an interrupt handler and the controller implements the irq_disable()
callback, the following scenario will appear in the i2c-hid driver in
the sleep scenario:

in the sleep flow, while the user is still triggering the i2c-hid
interrupt, we get the following function call:

  handle_level_irq()
    -> mask_ack_irq()
      -> mask_irq()
				i2c_hid_core_suspend()
				  -> disable_irq()
				    -> __irq_disable()
				      -> irq_state_set_disabled()
				      -> irq_state_set_masked()

  irq_thread_fn()
    -> irq_finalize_oneshot()
      -> if (!desc->threads_oneshot && !irqd_irq_disabled() &&
	     irqd_irq_masked())
      	 	unmask_threaded_irq()
		  -> unmask_irq()

That is, when __irq_disable() is called between mask_irq() and
irq_finalize_oneshot(), the code in irq_finalize_oneshot() will cause
the !irqd_irq_disabled() fails to enter the unmask_irq() branch, which
causes mask_irq/unmask_irq to be called unpaired and the i2c-hid
interrupt to be masked.

Since mask_irq/unmask_irq and irq_disabled() belong to two different
hardware registers or policies, the !irqd_irq_disabled() assertion may
not be used to determine whether unmask_irq() needs to be called.

Cc: stable@vger.kernel.org
Signed-off-by: xiongxin <xiongxin@kylinos.cn>
Signed-off-by: Riwen Lu <luriwen@kylinos.cn>

diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index 1782f90cd8c6..9160fc9170b3 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -1120,8 +1120,7 @@ static void irq_finalize_oneshot(struct irq_desc *desc,
 
 	desc->threads_oneshot &= ~action->thread_mask;
 
-	if (!desc->threads_oneshot && !irqd_irq_disabled(&desc->irq_data) &&
-	    irqd_irq_masked(&desc->irq_data))
+	if (!desc->threads_oneshot && irqd_irq_masked(&desc->irq_data))
 		unmask_threaded_irq(desc);
 
 out_unlock:
-- 
2.34.1


^ permalink raw reply related

* Re: [PATCH v2 2/2] Input: Add Himax HX83102J touchscreen driver
From: kernel test robot @ 2023-12-07  0:27 UTC (permalink / raw)
  To: Allen_Lin, dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt,
	conor+dt, jikos, benjamin.tissoires, linux-input, devicetree,
	linux-kernel
  Cc: oe-kbuild-all, Allen_Lin
In-Reply-To: <TY0PR06MB56119F0D60142F4C1435767C9E84A@TY0PR06MB5611.apcprd06.prod.outlook.com>

Hi Allen_Lin,

kernel test robot noticed the following build warnings:

[auto build test WARNING on hid/for-next]
[also build test WARNING on dtor-input/next dtor-input/for-linus robh/for-next linus/master v6.7-rc4 next-20231206]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Allen_Lin/Input-Add-Himax-HX83102J-touchscreen-driver/20231206-183804
base:   https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-next
patch link:    https://lore.kernel.org/r/TY0PR06MB56119F0D60142F4C1435767C9E84A%40TY0PR06MB5611.apcprd06.prod.outlook.com
patch subject: [PATCH v2 2/2] Input: Add Himax HX83102J touchscreen driver
config: sh-randconfig-r053-20231207 (https://download.01.org/0day-ci/archive/20231207/202312070838.oJSWv4T3-lkp@intel.com/config)
compiler: sh4-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231207/202312070838.oJSWv4T3-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202312070838.oJSWv4T3-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/hid/hid-himax-83102j.c:2673:12: warning: 'himax_parse_dt' defined but not used [-Wunused-function]
    2673 | static int himax_parse_dt(struct device_node *dt, struct himax_platform_data *pdata)
         |            ^~~~~~~~~~~~~~


vim +/himax_parse_dt +2673 drivers/hid/hid-himax-83102j.c

  2672	
> 2673	static int himax_parse_dt(struct device_node *dt, struct himax_platform_data *pdata)
  2674	{
  2675		/* pid_fw_name size = length of default_fw_name + length of "_XXXX" +
  2676		 * length of ".bin" + null terminator.
  2677		 */
  2678		static char pid_fw_name[ARRAY_SIZE(default_fw_name) + 5 + 4 + 1] = {0};
  2679		int tmp = 0;
  2680		const int pid_prop_args = 2;
  2681		u32 data = 0;
  2682		int id_gpios[8] = {0};
  2683		int counter = 0;
  2684		int i = 0;
  2685		s16 id_value = -1;
  2686		int dts_irq = 0;
  2687		int err = 0;
  2688	
  2689		UNUSED(default_fw_name);
  2690		if (!dt || !pdata) {
  2691			E("DT: dev or pdata is NULL");
  2692			return -EINVAL;
  2693		}
  2694	
  2695		dts_irq = of_irq_get(dt, 0);
  2696		D("DT: dts_irq = %d", dts_irq);
  2697		if (dts_irq <= 0) {
  2698			if (dts_irq == -EPROBE_DEFER)
  2699				E("DT: dts_irq = -EPROBE_DEFER");
  2700			return -EIO;
  2701		}
  2702	
  2703		pdata->of_irq = dts_irq;
  2704		pdata->gpio_irq = -1;
  2705	
  2706		pdata->gpio_reset = of_get_named_gpio(dt, "reset-gpios", 0);
  2707		if (!gpio_is_valid(pdata->gpio_reset)) {
  2708			I(" DT:gpio-rst value is not valid");
  2709			return -EIO;
  2710		}
  2711	
  2712		I(" DT:interrupt=%d, reset=%d",
  2713		  pdata->of_irq, pdata->gpio_reset);
  2714		counter = gpiod_count(pdata->ts->dev, "himax,id");
  2715		if (counter > 0) {
  2716			for (i = 0 ; i < counter ; i++) {
  2717				id_gpios[i] = of_get_named_gpio(dt, "himax,id-gpios", i);
  2718				if (!gpio_is_valid(id_gpios[i])) {
  2719					I(" DT:gpio-id value is not valid");
  2720					return -EIO;
  2721				}
  2722				I(" DT:gpio-id[%d]=%d", i, id_gpios[i]);
  2723			}
  2724			id_value = 0;
  2725			for (i = 0 ; i < counter ; i++) {
  2726				gpio_direction_input(id_gpios[i]);
  2727				id_value |= gpio_get_value(id_gpios[i]) << i;
  2728			}
  2729			I(" DT:gpio-id value=%04X", id_value);
  2730			pdata->panel_id = id_value;
  2731		} else {
  2732			pdata->panel_id = -1;
  2733			D(" DT:gpio-id not found");
  2734		}
  2735	
  2736		// himax,ic_det_delay unit is millisecond
  2737		if (of_property_read_u32(dt, "himax,ic-det-delay-ms", &data)) {
  2738			pdata->ic_det_delay = 0;
  2739			D(" DT:himax,ic-det-delay-ms not found");
  2740		} else {
  2741			pdata->ic_det_delay = data;
  2742			I(" DT:himax,ic-det-delay-ms=%d", pdata->ic_det_delay);
  2743		}
  2744	
  2745		// himax,ic_resume_delay unit is millisecond
  2746		if (of_property_read_u32(dt, "himax,ic-resume-delay-ms", &data)) {
  2747			pdata->ic_resume_delay = 0;
  2748			D(" DT:himax,ic-resume-delay-ms not found");
  2749		} else {
  2750			pdata->ic_resume_delay = data;
  2751			I(" DT:himax,ic-resume-delay-ms=%d", pdata->ic_resume_delay);
  2752		}
  2753	
  2754		if (of_property_read_bool(dt, "himax,has-flash")) {
  2755			pdata->is_zf = false;
  2756			D(" DT:himax,has-flash");
  2757		} else {
  2758			pdata->is_zf = true;
  2759			I(" DT:himax,has-flash not found, load firmware from file");
  2760		}
  2761	
  2762		if (of_property_read_bool(dt, "vccd-supply")) {
  2763			pdata->vccd_supply = regulator_get(pdata->ts->dev, "vccd");
  2764			if (IS_ERR(pdata->vccd_supply)) {
  2765				E(" DT:failed to get vccd supply");
  2766				err = PTR_ERR(pdata->vccd_supply);
  2767				pdata->vccd_supply = NULL;
  2768				return err;
  2769			}
  2770			I(" DT:vccd-supply=%p", pdata->vccd_supply);
  2771		} else {
  2772			pdata->vccd_supply = NULL;
  2773		}
  2774	
  2775		if (of_property_read_bool(dt, "vcca-supply")) {
  2776			pdata->vcca_supply = regulator_get(pdata->ts->dev, "vcca");
  2777			if (IS_ERR(pdata->vcca_supply)) {
  2778				E(" DT:failed to get vcca supply");
  2779				err = PTR_ERR(pdata->vcca_supply);
  2780				pdata->vcca_supply = NULL;
  2781				return err;
  2782			}
  2783			I(" DT:vcca-supply=%p", pdata->vcca_supply);
  2784		} else {
  2785			pdata->vcca_supply = NULL;
  2786		}
  2787	
  2788		/*
  2789		 * check himax,pid first, if exist then check if it is single.
  2790		 * Single case: himax,pid = <0x1002>; // 0x1002 is pid value
  2791		 * Multiple case: himax,pid = <id_value0 00x1001>, <id_value1 0x1002>;
  2792		 * When id_value >= 0, check the mapping listed to use the pid value.
  2793		 */
  2794		if (of_get_property(dt, "himax,pid", &data)) {
  2795			counter = data / (sizeof(u32) * pid_prop_args);
  2796	
  2797			if (!counter) {
  2798				// default case, no id->pid mappings
  2799				if (of_property_read_u32(dt, "himax,pid", &data)) {
  2800					pdata->pid = 0;
  2801					D(" DT:himax,pid not found");
  2802					goto GET_PID_END;
  2803				} else {
  2804					goto GET_PID_VALUE;
  2805				}
  2806			}
  2807	
  2808			if (id_value < 0) {
  2809				E(" DT:himax,pid has no matched for id_value=%04X", id_value);
  2810				pdata->pid = 0;
  2811				goto GET_PID_END;
  2812			}
  2813	
  2814			for (i = 0; i < counter; i++) {
  2815				if (of_property_read_u32_index(dt, "himax,pid",
  2816							       i * pid_prop_args, &tmp)) {
  2817					D(" DT:himax,pid parsing error!");
  2818					pdata->pid = 0;
  2819					goto GET_PID_END;
  2820				}
  2821	
  2822				if (of_property_read_u32_index(dt, "himax,pid",
  2823							       i * pid_prop_args + 1, &data)) {
  2824					D(" DT:himax,pid parsing error!");
  2825					pdata->pid = 0;
  2826					goto GET_PID_END;
  2827				}
  2828	
  2829				if (tmp == id_value) {
  2830					I(" DT:himax,pid mapping: id=%04X => pid=%04X, matched!",
  2831					  tmp, data);
  2832					i = counter;
  2833				} else {
  2834					I(" DT:himax,pid mapping: id=%04X => pid=%04X", tmp, data);
  2835				}
  2836			}
  2837	
  2838			if (counter == i) {
  2839				E(" DT:himax,pid has no matched for id_value=%04X", id_value);
  2840				pdata->pid = 0;
  2841				goto GET_PID_END;
  2842			}
  2843	
  2844	GET_PID_VALUE:
  2845			g_fw_boot_upgrade_name = pid_fw_name;
  2846			pdata->pid = data;
  2847			snprintf(pid_fw_name, sizeof(pid_fw_name), "%s_%04X%s",
  2848				 BOOT_UPGRADE_FWNAME, pdata->pid, ".bin");
  2849			I(" DT:himax,pid=%04X, fw_name=%s",
  2850			  pdata->pid, pid_fw_name);
  2851		} else {
  2852			pdata->pid = 0;
  2853			D(" DT:himax,pid not found");
  2854		}
  2855	GET_PID_END:
  2856	
  2857		return 0;
  2858	}
  2859	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply

* Re: [PATCH v2 0/9] Support light color temperature and chromaticity
From: Thomas Weißschuh @ 2023-12-06 23:39 UTC (permalink / raw)
  To: Basavaraj Natikar
  Cc: jikos, benjamin.tissoires, jic23, lars, srinivas.pandruvada,
	linux-input, linux-iio, regressions
In-Reply-To: <20230919081054.2050714-1-Basavaraj.Natikar@amd.com>

Hi everybody,

On 2023-09-19 13:40:45+0530, Basavaraj Natikar wrote:
> This series adds support for light color temperature and chromaticity.
> 
> v1->v2:
> *Rename the series.
> *Rename als_illum to als channel as it supports other channels.
> *Update patch description to include same reading for the two existing
>  channels to use channel index to support more hub attributes.
> *Keep line length under 80chars in hid-sensor-als.
> *Add new channel type IIO_COLORTEMP.
> *Update patch description and its subject to add channel type for 
>  chromaticity. 
> 
> Basavaraj Natikar (9):
>   iio: hid-sensor-als: Use channel index to support more hub attributes
>   iio: Add channel type light color temperature
>   iio: hid-sensor-als: Add light color temperature support
>   HID: amd_sfh: Add support for light color temperature
>   HID: amd_sfh: Add support for SFH1.1 light color temperature
>   iio: Add channel type for chromaticity
>   iio: hid-sensor-als: Add light chromaticity support
>   HID: amd_sfh: Add light chromaticity support
>   HID: amd_sfh: Add light chromaticity for SFH1.1

This series is breaking probing of hid-sensor-als on Framework 13 AMD
laptops [0].
The problem is that the patches require hid-sensors-als sensors to also
report chromaticity and color temparature which they don't.

When I remove the 'if (ret < 0) return ret;' checks in
als_parse_report() probing works and the illuminance/intensity channels
that show up behave as expected.
Unfortunately this still leaves behind a bunch of unusable channels.
A nice fix would be to have something like sysfs/hwmon .is_visible()
callback but that's not supported by IIO.

One aproach would be to detect the usable channels in als_parse_report()
and then adapt the indio_dev->channels based on that information.

[0] https://bugzilla.kernel.org/show_bug.cgi?id=218223

#regzbot introduced: 5f05285df691b1e82108eead7165feae238c95ef
#regzbot monitor: https://bugzilla.kernel.org/show_bug.cgi?id=218223

^ permalink raw reply

* Re: [PATCH v2 00/15] selftests/hid: tablets fixes
From: Jiri Kosina @ 2023-12-06 23:32 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Benjamin Tissoires, Shuah Khan, Peter Hutterer, linux-input,
	linux-kselftest, linux-kernel
In-Reply-To: <20231206-wip-selftests-v2-0-c0350c2f5986@kernel.org>

On Wed, 6 Dec 2023, Benjamin Tissoires wrote:

> Hi,
> 
> the main trigger of this series was the XP-Pen issue[0].
> Basically, the tablets tests were good-ish but couldn't
> handle that tablet both in terms of emulation or in terms
> of detection of issues.
> 
> So rework the tablets test a bit to be able to include the
> XP-Pen patch later, once I have a kernel fix for it (right
> now I only have a HID-BPF fix, meaning that the test will
> fail if I include them).
> 
> Also, vmtest.sh needed a little bit of care, because
> boot2container moved, and I made it easier to reuse in a CI
> environment.
> 
> Cheers,
> Benjamin
> 
> Note: I got the confirmation off-list from Peter that his
> rev-by applied to the whole series.
> 
> [0] https://lore.kernel.org/all/nycvar.YFH.7.76.2311012033290.29220@cbobk.fhfr.pm/
> 
> Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>

FWIW

	Acked-by: Jiri Kosina <jkosina@suse.com>

As far as I am concerned, feel free to push it to hid.git right away. And 
thanks a lot for all the work.

-- 
Jiri Kosina
SUSE Labs


^ permalink raw reply

* Re: [PATCH v2 1/2] Input: i8042: Avoid probing if no keyboard and mouse are set in quirks
From: Rahul Rameshbabu @ 2023-12-06 21:55 UTC (permalink / raw)
  To: Mario Limonciello; +Cc: dmitry.torokhov, linux-input, linux-kernel
In-Reply-To: <20231206212140.7458-1-mario.limonciello@amd.com>

On Wed, 06 Dec, 2023 15:21:39 -0600 Mario Limonciello <mario.limonciello@amd.com> wrote:
> Some laptops have an i8042 controller in the SOC, nothing mentioned in
> ACPI PNP and nothing connected to the controller. Add the ability to
> skip probing in this case.
>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---

Thanks. I think this is a good choice for handling the issue you
presented with the Framework 16.

Reviewed-by: Rahul Rameshbabu <rrameshbabu@nvidia.com>

^ permalink raw reply

* Re: [PATCH v2 2/2] Input: i8042: Add a quirk for Framework 16" laptop
From: Rahul Rameshbabu @ 2023-12-06 21:54 UTC (permalink / raw)
  To: Mario Limonciello; +Cc: dmitry.torokhov, linux-input, linux-kernel
In-Reply-To: <20231206212140.7458-2-mario.limonciello@amd.com>

On Wed, 06 Dec, 2023 15:21:40 -0600 Mario Limonciello <mario.limonciello@amd.com> wrote:
> The Framework 16" laptop doesn't have a PS/2 keyboard. At bootup the
> following messages are emitted:
>
> i8042: PNP: No PS/2 controller found.
> i8042: PNP: Probing ports directly.
> i8042: Can't read CTR while initializing i8042
> i8042: probe of i8042 failed with error -5
>
> There are no PNP devices as those listed in `pnp_kbd_devids` but
> i8042_pnp_init() ignores this and still runs and will continue to
> try to probe.
>
> As there is no PS/2 keyboard or mouse in this laptop, set a quirk
> to avoid this behavior.
>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---

Reviewed-by: Rahul Rameshbabu <rrameshbabu@nvidia.com>

^ permalink raw reply

* Re: [PATCH] Input: omap4-keypad: react on keypresses if device is runtime-suspended
From: Andreas Kemnade @ 2023-12-06 21:28 UTC (permalink / raw)
  To: dmitry.torokhov, tony, u.kleine-koenig, robh, Jonathan.Cameron,
	andreas, frank.li, linux-input, linux-kernel
In-Reply-To: <20231126194319.111504-1-andreas@kemnade.info>

On Sun, 26 Nov 2023 20:43:19 +0100
Andreas Kemnade <andreas@kemnade.info> wrote:

> According to SWPU235AB, table 26-6, fclk is required to generate events
> at least on OMAP4460, so keep fclk enabled all the time the device
> is opened.
> 
> Suggested-by: Tony Lindgren <tony@atomide.com>
> Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> Reviewed-by: Tony Lindgren <tony@atomide.com>
> ---
> Changes since RFC:
> - add R-by:
> 
>  drivers/input/keyboard/omap4-keypad.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
a friendly ping...

Regards,
Andreas

^ permalink raw reply

* [PATCH v2 2/2] Input: i8042: Add a quirk for Framework 16" laptop
From: Mario Limonciello @ 2023-12-06 21:21 UTC (permalink / raw)
  To: dmitry.torokhov; +Cc: linux-input, linux-kernel, rrameshbabu, Mario Limonciello
In-Reply-To: <20231206212140.7458-1-mario.limonciello@amd.com>

The Framework 16" laptop doesn't have a PS/2 keyboard. At bootup the
following messages are emitted:

i8042: PNP: No PS/2 controller found.
i8042: PNP: Probing ports directly.
i8042: Can't read CTR while initializing i8042
i8042: probe of i8042 failed with error -5

There are no PNP devices as those listed in `pnp_kbd_devids` but
i8042_pnp_init() ignores this and still runs and will continue to
try to probe.

As there is no PS/2 keyboard or mouse in this laptop, set a quirk
to avoid this behavior.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/input/serio/i8042-acpipnpio.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/input/serio/i8042-acpipnpio.h b/drivers/input/serio/i8042-acpipnpio.h
index 0fd88bbfaee1..6a71416057a3 100644
--- a/drivers/input/serio/i8042-acpipnpio.h
+++ b/drivers/input/serio/i8042-acpipnpio.h
@@ -1310,6 +1310,14 @@ static const struct dmi_system_id i8042_dmi_quirk_table[] __initconst = {
 		.driver_data = (void *)(SERIO_QUIRK_NOMUX | SERIO_QUIRK_RESET_ALWAYS |
 					SERIO_QUIRK_NOLOOP | SERIO_QUIRK_NOPNP)
 	},
+	/* Framework 16" laptop uses an internal USB keyboard */
+	{
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Framework"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "Laptop 16"),
+		},
+		.driver_data = (void *)(SERIO_QUIRK_NOKBD | SERIO_QUIRK_NOAUX)
+	},
 	{ }
 };
 
-- 
2.34.1


^ permalink raw reply related

* [PATCH v2 1/2] Input: i8042: Avoid probing if no keyboard and mouse are set in quirks
From: Mario Limonciello @ 2023-12-06 21:21 UTC (permalink / raw)
  To: dmitry.torokhov; +Cc: linux-input, linux-kernel, rrameshbabu, Mario Limonciello

Some laptops have an i8042 controller in the SOC, nothing mentioned in
ACPI PNP and nothing connected to the controller. Add the ability to
skip probing in this case.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/input/serio/i8042-acpipnpio.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/input/serio/i8042-acpipnpio.h b/drivers/input/serio/i8042-acpipnpio.h
index 9c39553d30fa..0fd88bbfaee1 100644
--- a/drivers/input/serio/i8042-acpipnpio.h
+++ b/drivers/input/serio/i8042-acpipnpio.h
@@ -1707,6 +1707,9 @@ static int __init i8042_platform_init(void)
 		"");
 #endif
 
+	if (i8042_nokbd && i8042_noaux)
+		return -ENODEV;
+
 	retval = i8042_pnp_init();
 	if (retval)
 		return retval;
-- 
2.34.1


^ permalink raw reply related

* Re: [PATCH] Input: i8042: Quiet down probe failure messages
From: Mario Limonciello @ 2023-12-06 20:06 UTC (permalink / raw)
  To: Rahul Rameshbabu; +Cc: dmitry.torokhov, linux-input, linux-kernel
In-Reply-To: <87v89bgl7a.fsf@nvidia.com>

On 12/6/2023 13:55, Rahul Rameshbabu wrote:
> On Wed, 06 Dec, 2023 13:22:25 -0600 Mario Limonciello <mario.limonciello@amd.com> wrote:
>> On 12/6/2023 12:46, Rahul Rameshbabu wrote:
>>> On Wed, 06 Dec, 2023 11:58:18 -0600 Mario Limonciello <mario.limonciello@amd.com> wrote:
>>>> The Framework 16" laptop doesn't have a PS/2 keyboard. At bootup the
>>>> following messages are emitted:
>>>>
>>>> i8042: PNP: No PS/2 controller found.
>>>> i8042: PNP: Probing ports directly.
>>>> i8042: Can't read CTR while initializing i8042
>>>> i8042: probe of i8042 failed with error -5
>>>>
>>>> The last two messages are ERR and WARN respectively.  These messages
>>>> might be useful for one boot while diagnosing a problem for someone
>>>> but as there is no PS/2 controller in PNP or on the machine they're
>>>> needlessly noisy to emit every boot.
>>>>
>>>> Downgrade the CTR message to debug and change the error code for the
>>>> failure so that the base device code doesn't emit a warning.
>>>>
>>>> If someone has problems with i8042 and they need this information,
>>>> they can turn on dynamic debugging to get these messages.
>>>>
>>>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>>>> ---
>>> For the Framework 16, I think the following should be done.
>>> Use SERIO_QUIRK_NOPNP for the device to avoid the PS/2 controller
>>> probing. You can find examples in drivers/input/serio/i8042-acpipnpio.h
>>> under the i8042_dmi_quirk_table. This will prevent emitting the first
>>> two messages in the shared snippet.
>>
>> I had tried this initially, and yes those first two messages are removed.  But
>> TBH I'm not too worried about those as they're INFO. Adding a quirk just
>> switches them over to a new INFO message.
>>
> 
> Right, I can imagine someone owning this device panic-ing about the logs
> in red/yellow in journalctl or dmesg output. That said, since we know
> that the device should not bother with PNP, I think we should add the
> quirk, none the less.
> 
>> But the ERR and WARN messages still come up.  The 3 messages that show up are:
>>
>> i8042: PNP detection disabled
>> i8042: Can't read CTR while initializing i8042
>> i8042: probe of i8042 failed with error -5
>>
>> I'm more concerned that ERR and WARN messages show up making you think there is
>> a problem to look into.
>>
>>>
>>>>    drivers/input/serio/i8042.c | 4 ++--
>>>>    1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c
>>>> index 9fbb8d31575a..95dd585fdc1a 100644
>>>> --- a/drivers/input/serio/i8042.c
>>>> +++ b/drivers/input/serio/i8042.c
>>>> @@ -1008,8 +1008,8 @@ static int i8042_controller_init(void)
>>>>    			udelay(50);
>>>>      		if (i8042_command(&ctr[n++ % 2], I8042_CMD_CTL_RCTR)) {
>>>> -			pr_err("Can't read CTR while initializing i8042\n");
>>>> -			return i8042_probe_defer ? -EPROBE_DEFER : -EIO;
>>>> +			pr_debug("Can't read CTR while initializing\n");
>>> I also think this error message should be pr_err in the situation that
>>> the SERIO_QUIRK_PROBE_DEFER quirk is not used. I think what you are
>>> likely looking for is avoiding emitting this message when the
>>> SERIO_QUIRK_PROBE_DEFER quirk is used for noise reduction purposes.
>>
>> SERIO_QUIRK_PROBE_DEFER isn't set on this machine.
>>
> 
> Yeah, I would like to add this quirk as well for the device potentially
> along with SERIO_QUIRK_NOPNP. I'll get into why I think this might be a
> good idea later in this email.

OK if we end up with a quirk for this system in the end I'll add both.

> 
>>>
>>>> +			return i8042_probe_defer ? -EPROBE_DEFER : -ENXIO;
>>> I do not think this change makes sense to me personally. It is indeed an
>>> I/O issue with the i8042 controller on the Framework motherboard, so the
>>> error should be -EIO when i8042_probe_defer is not set.
>>
>> With i8042.debug enabled I can see that the error is a wait read timeout.  So I
>> had thought -ENXIO ("No such device or address") made sense here.
> 
> Right, I think that wait read timeout you are seeing is likely a symptom
> of something very similar to the experience on laptops like the ASUS
> ZenBook UX425UA, which inspired the introduction of the probe deferring
> in the i8042 driver.
> 
>    https://lore.kernel.org/lkml/20211112180022.10850-1-tiwai@suse.de/T/
> 
> In this case though, the ASUS ZenBook device did have a PS/2 device
> attached, while in the Framework device this shouldn't be the case. Will
> delve more into this nuance in my next section.

OK let me experiment with this and get back on what happens.
> 
>>
>> If you feel strongly that the errors and warnings should stay as is I I wonder
>> if this should be looked at from i8042_pnp_init().
>>
>> In the no PNP device declared case it still probes, why isn't PNP trusted?
>>
> 
> My understanding is this is due to some PS/2 devices not supporting PNP
> so this manual probe path must still be taken even when we add the
> SERIO_QUIRK_NOPNP quirk. I think if we add the quirk for deferred
> probing for the device, we can then also have a patch that does not omit
> the error to the logs when this quirk is enabled.
> 
> My question now becomes the following. If the Framework device does not
> want to expose its Intel 8042 controller to the host since its some
> unused hardware on the mainboard, why does it bother to advertise the
> 8042 over the ACPI table? Wouldn't it be better to have some UEFI update
> that fixes the ACPI table listing to avoid advertising the 8042?
> 
>    https://wiki.osdev.org/%228042%22_PS/2_Controller

You hit on the head my confusion.  The laptop doesn't advertise any of 
the PNP IDs listed in pnp_kbd_devids.

I double checked in /sys/bus/acpi/devices.

It seems that they're ignored anyway and will probe whether they are 
there or not.

So how could the firmware actually advertise this so the kernel would 
trust it?

> 
>>>
>>>>    		}
>>>>      	} while (n < 2 || ctr[0] != ctr[1]);
> 
> --
> Thanks,
> 
> Rahul Rameshbabu


^ permalink raw reply

* Re: [PATCH] Input: i8042: Quiet down probe failure messages
From: Rahul Rameshbabu @ 2023-12-06 19:55 UTC (permalink / raw)
  To: Mario Limonciello; +Cc: dmitry.torokhov, linux-input, linux-kernel
In-Reply-To: <abc96443-ed27-4021-afa9-280be027e355@amd.com>

On Wed, 06 Dec, 2023 13:22:25 -0600 Mario Limonciello <mario.limonciello@amd.com> wrote:
> On 12/6/2023 12:46, Rahul Rameshbabu wrote:
>> On Wed, 06 Dec, 2023 11:58:18 -0600 Mario Limonciello <mario.limonciello@amd.com> wrote:
>>> The Framework 16" laptop doesn't have a PS/2 keyboard. At bootup the
>>> following messages are emitted:
>>>
>>> i8042: PNP: No PS/2 controller found.
>>> i8042: PNP: Probing ports directly.
>>> i8042: Can't read CTR while initializing i8042
>>> i8042: probe of i8042 failed with error -5
>>>
>>> The last two messages are ERR and WARN respectively.  These messages
>>> might be useful for one boot while diagnosing a problem for someone
>>> but as there is no PS/2 controller in PNP or on the machine they're
>>> needlessly noisy to emit every boot.
>>>
>>> Downgrade the CTR message to debug and change the error code for the
>>> failure so that the base device code doesn't emit a warning.
>>>
>>> If someone has problems with i8042 and they need this information,
>>> they can turn on dynamic debugging to get these messages.
>>>
>>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>>> ---
>> For the Framework 16, I think the following should be done.
>> Use SERIO_QUIRK_NOPNP for the device to avoid the PS/2 controller
>> probing. You can find examples in drivers/input/serio/i8042-acpipnpio.h
>> under the i8042_dmi_quirk_table. This will prevent emitting the first
>> two messages in the shared snippet.
>
> I had tried this initially, and yes those first two messages are removed.  But
> TBH I'm not too worried about those as they're INFO. Adding a quirk just
> switches them over to a new INFO message.
>

Right, I can imagine someone owning this device panic-ing about the logs
in red/yellow in journalctl or dmesg output. That said, since we know
that the device should not bother with PNP, I think we should add the
quirk, none the less.

> But the ERR and WARN messages still come up.  The 3 messages that show up are:
>
> i8042: PNP detection disabled
> i8042: Can't read CTR while initializing i8042
> i8042: probe of i8042 failed with error -5
>
> I'm more concerned that ERR and WARN messages show up making you think there is
> a problem to look into.
>
>> 
>>>   drivers/input/serio/i8042.c | 4 ++--
>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c
>>> index 9fbb8d31575a..95dd585fdc1a 100644
>>> --- a/drivers/input/serio/i8042.c
>>> +++ b/drivers/input/serio/i8042.c
>>> @@ -1008,8 +1008,8 @@ static int i8042_controller_init(void)
>>>   			udelay(50);
>>>     		if (i8042_command(&ctr[n++ % 2], I8042_CMD_CTL_RCTR)) {
>>> -			pr_err("Can't read CTR while initializing i8042\n");
>>> -			return i8042_probe_defer ? -EPROBE_DEFER : -EIO;
>>> +			pr_debug("Can't read CTR while initializing\n");
>> I also think this error message should be pr_err in the situation that
>> the SERIO_QUIRK_PROBE_DEFER quirk is not used. I think what you are
>> likely looking for is avoiding emitting this message when the
>> SERIO_QUIRK_PROBE_DEFER quirk is used for noise reduction purposes.
>
> SERIO_QUIRK_PROBE_DEFER isn't set on this machine.
>

Yeah, I would like to add this quirk as well for the device potentially
along with SERIO_QUIRK_NOPNP. I'll get into why I think this might be a
good idea later in this email.

>> 
>>> +			return i8042_probe_defer ? -EPROBE_DEFER : -ENXIO;
>> I do not think this change makes sense to me personally. It is indeed an
>> I/O issue with the i8042 controller on the Framework motherboard, so the
>> error should be -EIO when i8042_probe_defer is not set.
>
> With i8042.debug enabled I can see that the error is a wait read timeout.  So I
> had thought -ENXIO ("No such device or address") made sense here.

Right, I think that wait read timeout you are seeing is likely a symptom
of something very similar to the experience on laptops like the ASUS
ZenBook UX425UA, which inspired the introduction of the probe deferring
in the i8042 driver.

  https://lore.kernel.org/lkml/20211112180022.10850-1-tiwai@suse.de/T/

In this case though, the ASUS ZenBook device did have a PS/2 device
attached, while in the Framework device this shouldn't be the case. Will
delve more into this nuance in my next section.

>
> If you feel strongly that the errors and warnings should stay as is I I wonder
> if this should be looked at from i8042_pnp_init().
>
> In the no PNP device declared case it still probes, why isn't PNP trusted?
>

My understanding is this is due to some PS/2 devices not supporting PNP
so this manual probe path must still be taken even when we add the
SERIO_QUIRK_NOPNP quirk. I think if we add the quirk for deferred
probing for the device, we can then also have a patch that does not omit
the error to the logs when this quirk is enabled.

My question now becomes the following. If the Framework device does not
want to expose its Intel 8042 controller to the host since its some
unused hardware on the mainboard, why does it bother to advertise the
8042 over the ACPI table? Wouldn't it be better to have some UEFI update
that fixes the ACPI table listing to avoid advertising the 8042?

  https://wiki.osdev.org/%228042%22_PS/2_Controller

>> 
>>>   		}
>>>     	} while (n < 2 || ctr[0] != ctr[1]);

--
Thanks,

Rahul Rameshbabu

^ permalink raw reply

* Re: [PATCH] Input: i8042: Quiet down probe failure messages
From: Mario Limonciello @ 2023-12-06 19:22 UTC (permalink / raw)
  To: Rahul Rameshbabu; +Cc: dmitry.torokhov, linux-input, linux-kernel
In-Reply-To: <87zfyngoe4.fsf@nvidia.com>

On 12/6/2023 12:46, Rahul Rameshbabu wrote:
> On Wed, 06 Dec, 2023 11:58:18 -0600 Mario Limonciello <mario.limonciello@amd.com> wrote:
>> The Framework 16" laptop doesn't have a PS/2 keyboard. At bootup the
>> following messages are emitted:
>>
>> i8042: PNP: No PS/2 controller found.
>> i8042: PNP: Probing ports directly.
>> i8042: Can't read CTR while initializing i8042
>> i8042: probe of i8042 failed with error -5
>>
>> The last two messages are ERR and WARN respectively.  These messages
>> might be useful for one boot while diagnosing a problem for someone
>> but as there is no PS/2 controller in PNP or on the machine they're
>> needlessly noisy to emit every boot.
>>
>> Downgrade the CTR message to debug and change the error code for the
>> failure so that the base device code doesn't emit a warning.
>>
>> If someone has problems with i8042 and they need this information,
>> they can turn on dynamic debugging to get these messages.
>>
>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>> ---
> 
> For the Framework 16, I think the following should be done.
> 
> Use SERIO_QUIRK_NOPNP for the device to avoid the PS/2 controller
> probing. You can find examples in drivers/input/serio/i8042-acpipnpio.h
> under the i8042_dmi_quirk_table. This will prevent emitting the first
> two messages in the shared snippet.

I had tried this initially, and yes those first two messages are 
removed.  But TBH I'm not too worried about those as they're INFO. 
Adding a quirk just switches them over to a new INFO message.

But the ERR and WARN messages still come up.  The 3 messages that show 
up are:

i8042: PNP detection disabled
i8042: Can't read CTR while initializing i8042
i8042: probe of i8042 failed with error -5

I'm more concerned that ERR and WARN messages show up making you think 
there is a problem to look into.

> 
> 
>>   drivers/input/serio/i8042.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c
>> index 9fbb8d31575a..95dd585fdc1a 100644
>> --- a/drivers/input/serio/i8042.c
>> +++ b/drivers/input/serio/i8042.c
>> @@ -1008,8 +1008,8 @@ static int i8042_controller_init(void)
>>   			udelay(50);
>>   
>>   		if (i8042_command(&ctr[n++ % 2], I8042_CMD_CTL_RCTR)) {
>> -			pr_err("Can't read CTR while initializing i8042\n");
>> -			return i8042_probe_defer ? -EPROBE_DEFER : -EIO;
>> +			pr_debug("Can't read CTR while initializing\n");
> 
> I also think this error message should be pr_err in the situation that
> the SERIO_QUIRK_PROBE_DEFER quirk is not used. I think what you are
> likely looking for is avoiding emitting this message when the
> SERIO_QUIRK_PROBE_DEFER quirk is used for noise reduction purposes.

SERIO_QUIRK_PROBE_DEFER isn't set on this machine.

> 
>> +			return i8042_probe_defer ? -EPROBE_DEFER : -ENXIO;
> 
> I do not think this change makes sense to me personally. It is indeed an
> I/O issue with the i8042 controller on the Framework motherboard, so the
> error should be -EIO when i8042_probe_defer is not set.

With i8042.debug enabled I can see that the error is a wait read 
timeout.  So I had thought -ENXIO ("No such device or address") made 
sense here.

If you feel strongly that the errors and warnings should stay as is I I 
wonder if this should be looked at from i8042_pnp_init().

In the no PNP device declared case it still probes, why isn't PNP trusted?

> 
>>   		}
>>   
>>   	} while (n < 2 || ctr[0] != ctr[1]);
> 
> --
> Thanks,
> 
> Rahul Rameshbabu



^ permalink raw reply

* Re: [PATCH] Input: i8042: Quiet down probe failure messages
From: Rahul Rameshbabu @ 2023-12-06 18:46 UTC (permalink / raw)
  To: Mario Limonciello; +Cc: dmitry.torokhov, linux-input, linux-kernel
In-Reply-To: <20231206175818.2568-1-mario.limonciello@amd.com>

On Wed, 06 Dec, 2023 11:58:18 -0600 Mario Limonciello <mario.limonciello@amd.com> wrote:
> The Framework 16" laptop doesn't have a PS/2 keyboard. At bootup the
> following messages are emitted:
>
> i8042: PNP: No PS/2 controller found.
> i8042: PNP: Probing ports directly.
> i8042: Can't read CTR while initializing i8042
> i8042: probe of i8042 failed with error -5
>
> The last two messages are ERR and WARN respectively.  These messages
> might be useful for one boot while diagnosing a problem for someone
> but as there is no PS/2 controller in PNP or on the machine they're
> needlessly noisy to emit every boot.
>
> Downgrade the CTR message to debug and change the error code for the
> failure so that the base device code doesn't emit a warning.
>
> If someone has problems with i8042 and they need this information,
> they can turn on dynamic debugging to get these messages.
>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---

For the Framework 16, I think the following should be done.

Use SERIO_QUIRK_NOPNP for the device to avoid the PS/2 controller
probing. You can find examples in drivers/input/serio/i8042-acpipnpio.h
under the i8042_dmi_quirk_table. This will prevent emitting the first
two messages in the shared snippet.


>  drivers/input/serio/i8042.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c
> index 9fbb8d31575a..95dd585fdc1a 100644
> --- a/drivers/input/serio/i8042.c
> +++ b/drivers/input/serio/i8042.c
> @@ -1008,8 +1008,8 @@ static int i8042_controller_init(void)
>  			udelay(50);
>  
>  		if (i8042_command(&ctr[n++ % 2], I8042_CMD_CTL_RCTR)) {
> -			pr_err("Can't read CTR while initializing i8042\n");
> -			return i8042_probe_defer ? -EPROBE_DEFER : -EIO;
> +			pr_debug("Can't read CTR while initializing\n");

I also think this error message should be pr_err in the situation that
the SERIO_QUIRK_PROBE_DEFER quirk is not used. I think what you are
likely looking for is avoiding emitting this message when the
SERIO_QUIRK_PROBE_DEFER quirk is used for noise reduction purposes.

> +			return i8042_probe_defer ? -EPROBE_DEFER : -ENXIO;

I do not think this change makes sense to me personally. It is indeed an
I/O issue with the i8042 controller on the Framework motherboard, so the
error should be -EIO when i8042_probe_defer is not set.

>  		}
>  
>  	} while (n < 2 || ctr[0] != ctr[1]);

--
Thanks,

Rahul Rameshbabu

^ permalink raw reply

* [PATCH] Input: i8042: Quiet down probe failure messages
From: Mario Limonciello @ 2023-12-06 17:58 UTC (permalink / raw)
  To: dmitry.torokhov; +Cc: linux-input, linux-kernel, Mario Limonciello

The Framework 16" laptop doesn't have a PS/2 keyboard. At bootup the
following messages are emitted:

i8042: PNP: No PS/2 controller found.
i8042: PNP: Probing ports directly.
i8042: Can't read CTR while initializing i8042
i8042: probe of i8042 failed with error -5

The last two messages are ERR and WARN respectively.  These messages
might be useful for one boot while diagnosing a problem for someone
but as there is no PS/2 controller in PNP or on the machine they're
needlessly noisy to emit every boot.

Downgrade the CTR message to debug and change the error code for the
failure so that the base device code doesn't emit a warning.

If someone has problems with i8042 and they need this information,
they can turn on dynamic debugging to get these messages.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/input/serio/i8042.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c
index 9fbb8d31575a..95dd585fdc1a 100644
--- a/drivers/input/serio/i8042.c
+++ b/drivers/input/serio/i8042.c
@@ -1008,8 +1008,8 @@ static int i8042_controller_init(void)
 			udelay(50);
 
 		if (i8042_command(&ctr[n++ % 2], I8042_CMD_CTL_RCTR)) {
-			pr_err("Can't read CTR while initializing i8042\n");
-			return i8042_probe_defer ? -EPROBE_DEFER : -EIO;
+			pr_debug("Can't read CTR while initializing\n");
+			return i8042_probe_defer ? -EPROBE_DEFER : -ENXIO;
 		}
 
 	} while (n < 2 || ctr[0] != ctr[1]);
-- 
2.34.1


^ permalink raw reply related

* Re: [RFC PATCH v3 4/5] arm64: dts: mediatek: mt8173-elm-hana: Mark touchscreens and trackpads as fail
From: Doug Anderson @ 2023-12-06 17:00 UTC (permalink / raw)
  To: AngeloGioacchino Del Regno
  Cc: Chen-Yu Tsai, Rob Herring, Frank Rowand, Krzysztof Kozlowski,
	Conor Dooley, Matthias Brugger, Wolfram Sang, Benson Leung,
	Tzung-Bi Shih, chrome-platform, devicetree, linux-arm-kernel,
	linux-mediatek, linux-kernel, Johan Hovold, Hsin-Yi Wang,
	Dmitry Torokhov, andriy.shevchenko, Jiri Kosina, linus.walleij,
	broonie, gregkh, hdegoede, james.clark, james, keescook, rafael,
	tglx, Jeff LaBundy, linux-input, linux-i2c
In-Reply-To: <4b9ea82c-d1a4-47b6-ba03-346cfdedef05@collabora.com>

Hi,

On Wed, Dec 6, 2023 at 2:02 AM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> Il 06/12/23 03:55, Chen-Yu Tsai ha scritto:
> > On Tue, Dec 5, 2023 at 6:22 PM AngeloGioacchino Del Regno
> > <angelogioacchino.delregno@collabora.com> wrote:
> >>
> >> Il 04/12/23 17:50, Doug Anderson ha scritto:
> >>> Hi,
> >>>
> >>> On Sun, Dec 3, 2023 at 10:59 PM Chen-Yu Tsai <wenst@chromium.org> wrote:
> >>>>
> >>>> On Sat, Dec 2, 2023 at 8:58 AM Doug Anderson <dianders@chromium.org> wrote:
> >>>>>
> >>>>> Hi,
> >>>>>
> >>>>> On Tue, Nov 28, 2023 at 12:45 AM Chen-Yu Tsai <wenst@chromium.org> wrote:
> >>>>>>
> >>>>>> @@ -44,6 +46,7 @@ trackpad2: trackpad@2c {
> >>>>>>                   reg = <0x2c>;
> >>>>>>                   hid-descr-addr = <0x0020>;
> >>>>>>                   wakeup-source;
> >>>>>> +               status = "fail-needs-probe";
> >>>>>
> >>>>> While doing this, you could also remove the hack where the trackpad
> >>>>> IRQ pinctrl is listed under i2c4.
> >>>>
> >>>> Sure. I do think we can do away with it though. According to at least one
> >>>> schematic, the interrupt line has pull-ups on both sides of the voltage
> >>>> shifter.
> >>>>
> >>>> BTW, The touchscreen doesn't have pinctrl entries. This has pull-ups on
> >>>> both sides of the voltage shifter as well.
> >>>
> >>> I dunno if the convention is different on Mediatek boards, but at
> >>> least on boards I've been involved with in the past we've always put
> >>> pinctrl entries just to make things explicit. This meant that we
> >>> didn't rely on the firmware/bootrom/defaults to leave pulls in any
> >>> particular state. ...otherwise those external pull-ups could be
> >>> fighting with internal pull-downs, right?
> >>>
> >>
> >> MediaTek boards aren't special and there's no good reason for those to rely on
> >> firmware/bootrom/defaults - so there is no good reason to avoid declaring any
> >> relevant pinctrl entry.
> >
> > I think this should be migrated to use the proper GPIO bindings: the
> > GPIO_PULL_UP / GPIO_PULL_DOWN / GPIO_BIAS_DISABLE flags.
> >
> > But that's a different discussion.
> >
>
> 100% agreed.

I guess I'd need to see patches as an example to see how this looks,
but I'm at least slightly skeptical. In this case the GPIO is
indirectly specified via "interrupts". Would you add these flags to
the interrupts specifier, too? There's another potential pull as well
(PIN_CONFIG_BIAS_BUS_HOLD) as well as other pin configuration
(PIN_CONFIG_INPUT_DEBOUNCE, for instance). Do we try to fit all of
these into the GPIO / interrupt specifier?

Certainly I will admit that this is a complicated topic, but it seems
weird to say that we use pinctrl to specify pin configuration / pulls
for all pins except ones that are configured as GPIOs or GPIO
interrupts.

-Doug

^ permalink raw reply

* [PATCH v4 8/8] dt-bindings: mfd: dlg,da9063: Convert da9062 to json-schema
From: Biju Das @ 2023-12-06 15:57 UTC (permalink / raw)
  To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Lee Jones
  Cc: Biju Das, Support Opensource, Rafael J. Wysocki, Daniel Lezcano,
	Zhang Rui, Lukasz Luba, Steve Twiss, linux-input, devicetree,
	linux-pm, Geert Uytterhoeven, Prabhakar Mahadev Lad, Biju Das,
	linux-renesas-soc
In-Reply-To: <20231206155740.5278-1-biju.das.jz@bp.renesas.com>

Convert the da9062 PMIC device tree binding documentation to json-schema.

Update the example to match reality.

While at it, update description with link to product information.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v3->v4:
 * Split the thermal binding patch separate.
 * Updated the description.
v2->v3:
 * Fixed bot errors related to MAINTAINERS entry, invalid doc
   references and thermal examples by merging patch#4.
v2:
 * New patch
---
 .../bindings/input/dlg,da9062-onkey.yaml      |   3 +-
 .../devicetree/bindings/mfd/da9062.txt        | 124 ------------
 .../devicetree/bindings/mfd/dlg,da9063.yaml   | 186 +++++++++++++++++-
 .../bindings/thermal/dlg,da9062-thermal.yaml  |   2 +-
 4 files changed, 183 insertions(+), 132 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/mfd/da9062.txt

diff --git a/Documentation/devicetree/bindings/input/dlg,da9062-onkey.yaml b/Documentation/devicetree/bindings/input/dlg,da9062-onkey.yaml
index 4cff91f4bd34..18b6a3f02c07 100644
--- a/Documentation/devicetree/bindings/input/dlg,da9062-onkey.yaml
+++ b/Documentation/devicetree/bindings/input/dlg,da9062-onkey.yaml
@@ -11,8 +11,7 @@ maintainers:
 
 description: |
   This module is part of the DA9061/DA9062/DA9063. For more details about entire
-  DA9062 and DA9061 chips see Documentation/devicetree/bindings/mfd/da9062.txt
-  For DA9063 see Documentation/devicetree/bindings/mfd/dlg,da9063.yaml
+  DA906{1,2,3} chips see Documentation/devicetree/bindings/mfd/dlg,da9063.yaml
 
   This module provides the KEY_POWER event.
 
diff --git a/Documentation/devicetree/bindings/mfd/da9062.txt b/Documentation/devicetree/bindings/mfd/da9062.txt
deleted file mode 100644
index c8a7f119ac84..000000000000
--- a/Documentation/devicetree/bindings/mfd/da9062.txt
+++ /dev/null
@@ -1,124 +0,0 @@
-* Dialog DA9062 Power Management Integrated Circuit (PMIC)
-
-Product information for the DA9062 and DA9061 devices can be found here:
-- https://www.dialog-semiconductor.com/products/da9062
-- https://www.dialog-semiconductor.com/products/da9061
-
-The DA9062 PMIC consists of:
-
-Device                   Supply Names    Description
-------                   ------------    -----------
-da9062-regulator        :               : LDOs & BUCKs
-da9062-rtc              :               : Real-Time Clock
-da9062-onkey            :               : On Key
-da9062-watchdog         :               : Watchdog Timer
-da9062-thermal          :               : Thermal
-da9062-gpio             :               : GPIOs
-
-The DA9061 PMIC consists of:
-
-Device                   Supply Names    Description
-------                   ------------    -----------
-da9062-regulator        :               : LDOs & BUCKs
-da9062-onkey            :               : On Key
-da9062-watchdog         :               : Watchdog Timer
-da9062-thermal          :               : Thermal
-
-======
-
-Required properties:
-
-- compatible : Should be
-    "dlg,da9062" for DA9062
-    "dlg,da9061" for DA9061
-- reg : Specifies the I2C slave address (this defaults to 0x58 but it can be
-  modified to match the chip's OTP settings).
-
-Optional properties:
-
-- gpio-controller : Marks the device as a gpio controller.
-- #gpio-cells     : Should be two. The first cell is the pin number and the
-                    second cell is used to specify the gpio polarity.
-
-See Documentation/devicetree/bindings/gpio/gpio.txt for further information on
-GPIO bindings.
-
-- interrupts : IRQ line information.
-- interrupt-controller
-
-See Documentation/devicetree/bindings/interrupt-controller/interrupts.txt for
-further information on IRQ bindings.
-
-Sub-nodes:
-
-- regulators : This node defines the settings for the LDOs and BUCKs.
-  The DA9062 regulators are bound using their names listed below:
-
-    buck1    : BUCK_1
-    buck2    : BUCK_2
-    buck3    : BUCK_3
-    buck4    : BUCK_4
-    ldo1     : LDO_1
-    ldo2     : LDO_2
-    ldo3     : LDO_3
-    ldo4     : LDO_4
-
-  The DA9061 regulators are bound using their names listed below:
-
-    buck1    : BUCK_1
-    buck2    : BUCK_2
-    buck3    : BUCK_3
-    ldo1     : LDO_1
-    ldo2     : LDO_2
-    ldo3     : LDO_3
-    ldo4     : LDO_4
-
-  The component follows the standard regulator framework and the bindings
-  details of individual regulator device can be found in:
-  Documentation/devicetree/bindings/regulator/regulator.txt
-
-  regulator-initial-mode may be specified for buck regulators using mode values
-  from include/dt-bindings/regulator/dlg,da9063-regulator.h.
-
-- rtc : This node defines settings required for the Real-Time Clock associated
-  with the DA9062. There are currently no entries in this binding, however
-  compatible = "dlg,da9062-rtc" should be added if a node is created.
-
-- onkey : See ../input/dlg,da9062-onkey.yaml
-
-- watchdog: See ../watchdog/dlg,da9062-watchdog.yaml
-
-- thermal : See ../thermal/dlg,da9062-thermal.yaml
-
-Example:
-
-	pmic0: da9062@58 {
-		compatible = "dlg,da9062";
-		reg = <0x58>;
-		interrupt-parent = <&gpio6>;
-		interrupts = <11 IRQ_TYPE_LEVEL_LOW>;
-		interrupt-controller;
-
-		rtc {
-			compatible = "dlg,da9062-rtc";
-		};
-
-		regulators {
-			DA9062_BUCK1: buck1 {
-				regulator-name = "BUCK1";
-				regulator-min-microvolt = <300000>;
-				regulator-max-microvolt = <1570000>;
-				regulator-min-microamp = <500000>;
-				regulator-max-microamp = <2000000>;
-				regulator-initial-mode = <DA9063_BUCK_MODE_SYNC>;
-				regulator-boot-on;
-			};
-			DA9062_LDO1: ldo1 {
-				regulator-name = "LDO_1";
-				regulator-min-microvolt = <900000>;
-				regulator-max-microvolt = <3600000>;
-				regulator-boot-on;
-			};
-		};
-	};
-
diff --git a/Documentation/devicetree/bindings/mfd/dlg,da9063.yaml b/Documentation/devicetree/bindings/mfd/dlg,da9063.yaml
index 676b4f2566ae..54bb23dbc73f 100644
--- a/Documentation/devicetree/bindings/mfd/dlg,da9063.yaml
+++ b/Documentation/devicetree/bindings/mfd/dlg,da9063.yaml
@@ -4,7 +4,7 @@
 $id: http://devicetree.org/schemas/mfd/dlg,da9063.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: Dialog DA9063/DA9063L Power Management Integrated Circuit (PMIC)
+title: Dialog DA906{3L,3,2,1} Power Management Integrated Circuit (PMIC)
 
 maintainers:
   - Steve Twiss <stwiss.opensource@diasemi.com>
@@ -17,10 +17,17 @@ description: |
   moment where all voltage monitors are disabled. Next, as da9063 only supports
   UV *and* OV monitoring, both must be set to the same severity and value
   (0: disable, 1: enable).
+  Product information for the DA906{3L,3,2,1} devices can be found here:
+  - https://www.dialog-semiconductor.com/products/da9063l
+  - https://www.dialog-semiconductor.com/products/da9063
+  - https://www.dialog-semiconductor.com/products/da9062
+  - https://www.dialog-semiconductor.com/products/da9061
 
 properties:
   compatible:
     enum:
+      - dlg,da9061
+      - dlg,da9062
       - dlg,da9063
       - dlg,da9063l
 
@@ -35,6 +42,19 @@ properties:
   "#interrupt-cells":
     const: 2
 
+  gpio:
+    type: object
+    $ref: /schemas/gpio/gpio.yaml#
+    unevaluatedProperties: false
+    properties:
+      compatible:
+        const: dlg,da9062-gpio
+
+  gpio-controller: true
+
+  "#gpio-cells":
+    const: 2
+
   onkey:
     $ref: /schemas/input/dlg,da9062-onkey.yaml
 
@@ -42,7 +62,7 @@ properties:
     type: object
     additionalProperties: false
     patternProperties:
-      "^(ldo([1-9]|1[01])|bcore([1-2]|s-merged)|b(pro|mem|io|peri)|bmem-bio-merged)$":
+      "^(ldo([1-9]|1[01])|bcore([1-2]|s-merged)|b(pro|mem|io|peri)|bmem-bio-merged|buck[1-4])$":
         $ref: /schemas/regulator/regulator.yaml
         unevaluatedProperties: false
 
@@ -52,7 +72,12 @@ properties:
     unevaluatedProperties: false
     properties:
       compatible:
-        const: dlg,da9063-rtc
+        enum:
+          - dlg,da9063-rtc
+          - dlg,da9062-rtc
+
+  thermal:
+    $ref: /schemas/thermal/dlg,da9062-thermal.yaml
 
   watchdog:
     $ref: /schemas/watchdog/dlg,da9062-watchdog.yaml
@@ -60,8 +85,65 @@ properties:
 required:
   - compatible
   - reg
-  - interrupts
-  - interrupt-controller
+
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - dlg,da9063
+              - dlg,da9063l
+    then:
+      properties:
+        thermal: false
+        gpio: false
+        gpio-controller: false
+        "#gpio-cells": false
+        regulators:
+          patternProperties:
+            "^buck[1-4]$": false
+      required:
+        - interrupts
+        - interrupt-controller
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - dlg,da9062
+    then:
+      properties:
+        regulators:
+          patternProperties:
+            "^(ldo([5-9]|10|11)|bcore([1-2]|s-merged)|b(pro|mem|io|peri)|bmem-bio-merged)$": false
+      required:
+        - gpio
+        - onkey
+        - rtc
+        - thermal
+        - watchdog
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - dlg,da9061
+    then:
+      properties:
+        gpio: false
+        gpio-controller: false
+        "#gpio-cells": false
+        regulators:
+          patternProperties:
+            "^(ldo([5-9]|10|11)|bcore([1-2]|s-merged)|b(pro|mem|io|peri)|bmem-bio-merged|buck4)$": false
+        rtc: false
+      required:
+        - onkey
+        - thermal
+        - watchdog
 
 additionalProperties: false
 
@@ -118,4 +200,98 @@ examples:
         };
       };
     };
+
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/regulator/dlg,da9063-regulator.h>
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+      pmic@58 {
+        compatible = "dlg,da9062";
+        reg = <0x58>;
+        #interrupt-cells = <2>;
+        interrupt-parent = <&gpio1>;
+        interrupts = <2 IRQ_TYPE_LEVEL_LOW>;
+        interrupt-controller;
+
+        gpio {
+          compatible = "dlg,da9062-gpio";
+          status = "disabled";
+        };
+
+        onkey {
+          compatible = "dlg,da9062-onkey";
+        };
+
+        regulators {
+          buck1 {
+            regulator-name = "vdd_arm";
+            regulator-min-microvolt = <925000>;
+            regulator-max-microvolt = <1380000>;
+            regulator-initial-mode = <DA9063_BUCK_MODE_SYNC>;
+            regulator-always-on;
+          };
+          buck2 {
+            regulator-name = "vdd_soc";
+            regulator-min-microvolt = <1150000>;
+            regulator-max-microvolt = <1380000>;
+            regulator-initial-mode = <DA9063_BUCK_MODE_SYNC>;
+            regulator-always-on;
+          };
+          buck3 {
+            regulator-name = "vdd_ddr3";
+            regulator-min-microvolt = <1500000>;
+            regulator-max-microvolt = <1500000>;
+            regulator-initial-mode = <DA9063_BUCK_MODE_SYNC>;
+            regulator-always-on;
+          };
+          buck4 {
+            regulator-name = "vdd_eth";
+            regulator-min-microvolt = <1200000>;
+            regulator-max-microvolt = <1200000>;
+            regulator-initial-mode = <DA9063_BUCK_MODE_SYNC>;
+            regulator-always-on;
+          };
+          ldo1 {
+            regulator-name = "vdd_snvs";
+            regulator-min-microvolt = <3000000>;
+            regulator-max-microvolt = <3000000>;
+            regulator-always-on;
+          };
+          ldo2 {
+            regulator-name = "vdd_high";
+            regulator-min-microvolt = <3000000>;
+            regulator-max-microvolt = <3000000>;
+            regulator-always-on;
+          };
+          ldo3 {
+            regulator-name = "vdd_eth_io";
+            regulator-min-microvolt = <2500000>;
+            regulator-max-microvolt = <2500000>;
+          };
+          ldo4 {
+            regulator-name = "vdd_emmc";
+            regulator-min-microvolt = <1800000>;
+            regulator-max-microvolt = <1800000>;
+            regulator-always-on;
+          };
+        };
+
+        rtc {
+          compatible = "dlg,da9062-rtc";
+          status = "disabled";
+        };
+
+        thermal {
+          compatible = "dlg,da9062-thermal";
+          status = "disabled";
+        };
+
+        watchdog {
+          compatible = "dlg,da9062-watchdog";
+          dlg,use-sw-pm;
+        };
+      };
+    };
 ...
diff --git a/Documentation/devicetree/bindings/thermal/dlg,da9062-thermal.yaml b/Documentation/devicetree/bindings/thermal/dlg,da9062-thermal.yaml
index 206635f74850..e8b2cac41084 100644
--- a/Documentation/devicetree/bindings/thermal/dlg,da9062-thermal.yaml
+++ b/Documentation/devicetree/bindings/thermal/dlg,da9062-thermal.yaml
@@ -11,7 +11,7 @@ maintainers:
 
 description: |
   This module is part of the DA9061/DA9062. For more details about entire
-  DA9062 and DA9061 chips see Documentation/devicetree/bindings/mfd/da9062.txt
+  DA906{1,2} chips see Documentation/devicetree/bindings/mfd/dlg,da9063.yaml
 
   Junction temperature thermal module uses an interrupt signal to identify
   high THERMAL_TRIP_HOT temperatures for the PMIC device.
-- 
2.39.2


^ permalink raw reply related

* [PATCH v4 5/8] dt-bindings: input: Convert da906{1,2,3} onkey to json-schema
From: Biju Das @ 2023-12-06 15:57 UTC (permalink / raw)
  To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: Biju Das, Support Opensource, linux-input, devicetree,
	Geert Uytterhoeven, Prabhakar Mahadev Lad, Biju Das,
	linux-renesas-soc
In-Reply-To: <20231206155740.5278-1-biju.das.jz@bp.renesas.com>

Convert the da906{1,2,3} onkey device tree binding documentation to
json-schema.

Update MAINTAINERS entries, description and onkey property by
referring to dlg,da9062-onkey binding file.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v3->v4:
 * Squashed with patch#6 and patch#9 from v2.
 * Replaced enum->const for dlg,da9061-onkey and its fallback.
 * Dropped example
v2->v3:
 * Updated MAINTAINERS entries.
v2:
 * New patch
---
 .../bindings/input/da9062-onkey.txt           | 47 -------------------
 .../bindings/input/dlg,da9062-onkey.yaml      | 40 ++++++++++++++++
 .../devicetree/bindings/mfd/da9062.txt        |  2 +-
 .../devicetree/bindings/mfd/dlg,da9063.yaml   | 15 +-----
 MAINTAINERS                                   |  2 +-
 5 files changed, 43 insertions(+), 63 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/input/da9062-onkey.txt
 create mode 100644 Documentation/devicetree/bindings/input/dlg,da9062-onkey.yaml

diff --git a/Documentation/devicetree/bindings/input/da9062-onkey.txt b/Documentation/devicetree/bindings/input/da9062-onkey.txt
deleted file mode 100644
index e5eef59a93dc..000000000000
--- a/Documentation/devicetree/bindings/input/da9062-onkey.txt
+++ /dev/null
@@ -1,47 +0,0 @@
-* Dialog DA9061/62/63 OnKey Module
-
-This module is part of the DA9061/DA9062/DA9063. For more details about entire
-DA9062 and DA9061 chips see Documentation/devicetree/bindings/mfd/da9062.txt
-For DA9063 see Documentation/devicetree/bindings/mfd/dlg,da9063.yaml
-
-This module provides the KEY_POWER event.
-
-Required properties:
-
-- compatible: should be one of the following valid compatible string lines:
-	"dlg,da9061-onkey", "dlg,da9062-onkey"
-	"dlg,da9062-onkey"
-	"dlg,da9063-onkey"
-
-Optional properties:
-
-- dlg,disable-key-power : Disable power-down using a long key-press. If this
-    entry exists the OnKey driver will remove support for the KEY_POWER key
-    press when triggered using a long press of the OnKey.
-
-Example: DA9063
-
-	pmic0: da9063@58 {
-		onkey {
-			compatible = "dlg,da9063-onkey";
-			dlg,disable-key-power;
-		};
-	};
-
-Example: DA9062
-
-	pmic0: da9062@58 {
-		onkey {
-			compatible = "dlg,da9062-onkey";
-			dlg,disable-key-power;
-		};
-	};
-
-Example: DA9061 using a fall-back compatible for the DA9062 onkey driver
-
-	pmic0: da9061@58 {
-		onkey {
-			compatible = "dlg,da9061-onkey", "dlg,da9062-onkey";
-			dlg,disable-key-power;
-		};
-	};
diff --git a/Documentation/devicetree/bindings/input/dlg,da9062-onkey.yaml b/Documentation/devicetree/bindings/input/dlg,da9062-onkey.yaml
new file mode 100644
index 000000000000..4cff91f4bd34
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/dlg,da9062-onkey.yaml
@@ -0,0 +1,40 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/dlg,da9062-onkey.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Dialog DA9061/62/63 OnKey Module
+
+maintainers:
+  - Biju Das <biju.das.jz@bp.renesas.com>
+
+description: |
+  This module is part of the DA9061/DA9062/DA9063. For more details about entire
+  DA9062 and DA9061 chips see Documentation/devicetree/bindings/mfd/da9062.txt
+  For DA9063 see Documentation/devicetree/bindings/mfd/dlg,da9063.yaml
+
+  This module provides the KEY_POWER event.
+
+properties:
+  compatible:
+    oneOf:
+      - items:
+          - enum:
+              - dlg,da9062-onkey
+              - dlg,da9063-onkey
+      - items:
+          - const: dlg,da9061-onkey
+          - const: dlg,da9062-onkey
+
+  dlg,disable-key-power:
+    type: boolean
+    description:
+      Disable power-down using a long key-press. If this entry exists
+      the OnKey driver will remove support for the KEY_POWER key press
+      when triggered using a long press of the OnKey.
+
+required:
+  - compatible
+
+additionalProperties: false
diff --git a/Documentation/devicetree/bindings/mfd/da9062.txt b/Documentation/devicetree/bindings/mfd/da9062.txt
index 18463b7fbb42..154c31fa4443 100644
--- a/Documentation/devicetree/bindings/mfd/da9062.txt
+++ b/Documentation/devicetree/bindings/mfd/da9062.txt
@@ -84,7 +84,7 @@ Sub-nodes:
   with the DA9062. There are currently no entries in this binding, however
   compatible = "dlg,da9062-rtc" should be added if a node is created.
 
-- onkey : See ../input/da9062-onkey.txt
+- onkey : See ../input/dlg,da9062-onkey.yaml
 
 - watchdog: See ../watchdog/dlg,da9062-watchdog.yaml
 
diff --git a/Documentation/devicetree/bindings/mfd/dlg,da9063.yaml b/Documentation/devicetree/bindings/mfd/dlg,da9063.yaml
index ce81e0b029cc..1e5a847a6be2 100644
--- a/Documentation/devicetree/bindings/mfd/dlg,da9063.yaml
+++ b/Documentation/devicetree/bindings/mfd/dlg,da9063.yaml
@@ -44,20 +44,7 @@ properties:
         const: dlg,da9063-rtc
 
   onkey:
-    type: object
-    $ref: /schemas/input/input.yaml#
-    unevaluatedProperties: false
-    properties:
-      compatible:
-        const: dlg,da9063-onkey
-
-      dlg,disable-key-power:
-        type: boolean
-        description: |
-          Disable power-down using a long key-press.
-          If this entry does not exist then by default the key-press triggered
-          power down is enabled and the OnKey will support both KEY_POWER and
-          KEY_SLEEP.
+    $ref: /schemas/input/dlg,da9062-onkey.yaml
 
   regulators:
     type: object
diff --git a/MAINTAINERS b/MAINTAINERS
index fa3965f1bf0e..ea171a18217c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6110,8 +6110,8 @@ DIALOG SEMICONDUCTOR DRIVERS
 M:	Support Opensource <support.opensource@diasemi.com>
 S:	Supported
 W:	http://www.dialog-semiconductor.com/products
-F:	Documentation/devicetree/bindings/input/da90??-onkey.txt
 F:	Documentation/devicetree/bindings/input/dlg,da72??.txt
+F:	Documentation/devicetree/bindings/input/dlg,da9062-onkey.yaml
 F:	Documentation/devicetree/bindings/mfd/da90*.txt
 F:	Documentation/devicetree/bindings/mfd/dlg,da90*.yaml
 F:	Documentation/devicetree/bindings/regulator/da92*.txt
-- 
2.39.2


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox