* Re: [PATCH] HID: google_hammer: Fix invalid ENOSYS warning and unsigned.
From: Jiri Kosina @ 2023-12-18 10:06 UTC (permalink / raw)
To: Guy Chronister; +Cc: benjamin.tissoires, linux-input, linux-kernel
In-Reply-To: <20231212182038.28530-1-guylovesbritt@gmail.com>
On Tue, 12 Dec 2023, Guy Chronister wrote:
> Fixed warnings about ENOSYS and bare unsigned without int.
>
> Signed-off-by: Guy Chronister <guylovesbritt@gmail.com>
> ---
> drivers/hid/hid-google-hammer.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hid/hid-google-hammer.c b/drivers/hid/hid-google-hammer.c
> index c6bdb9c4ef3e..d567f020bead 100644
> --- a/drivers/hid/hid-google-hammer.c
> +++ b/drivers/hid/hid-google-hammer.c
> @@ -324,7 +324,7 @@ static int hammer_kbd_brightness_set_blocking(struct led_classdev *cdev,
> }
>
> ret = hid_hw_output_report(led->hdev, led->buf, sizeof(led->buf));
> - if (ret == -ENOSYS)
> + if (ret == -EINVAL)
> ret = hid_hw_raw_request(led->hdev, 0, led->buf,
> sizeof(led->buf),
> HID_OUTPUT_REPORT,
Could you please elaborate why this is funcionally correct thing to do?
How are you now handling the special case when
hdev->ll_driver->output_report() callback doesn't exist for the specific
low-level driver?
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH 0/2] Fix regression in ALS
From: Jonathan Cameron @ 2023-12-18 10:12 UTC (permalink / raw)
To: Srinivas Pandruvada
Cc: jikos, jic23, lars, Basavaraj.Natikar, linux-input, linux-iio,
linux-kernel, gregkh
In-Reply-To: <20231217200703.719876-1-srinivas.pandruvada@linux.intel.com>
On Sun, 17 Dec 2023 12:07:01 -0800
Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> wrote:
> Addition of color temperature and chromaticity support breaks ALS sensor
> on several platforms. Till we have a good solution, revert these commits
> for 6.7 cycle.
>
> Srinivas Pandruvada (2):
> Revert "iio: hid-sensor-als: Add light chromaticity support"
> Revert "iio: hid-sensor-als: Add light color temperature support"
>
> drivers/iio/light/hid-sensor-als.c | 100 +----------------------------
> include/linux/hid-sensor-ids.h | 4 --
> 2 files changed, 2 insertions(+), 102 deletions(-)
+CC Greg KH.
Hi Greg,
This is a regression fix that I'd like to get in asap. Currently light sensors
on a wide range of laptops are broken. I was hoping we'd fix the the problem
rather than need to revert, but time is running out so revert it is.
I don't have anything else that needs to be rushed in before the merge cycle,
so if you are happy to pick these two reverts directly that would be great.
Message ID of the cover letter is
20231217200703.719876-1-srinivas.pandruvada@linux.intel.com
Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
If not I should be able to do a pull request in next couple of days
with these in.
Thanks,
Jonathan
>
^ permalink raw reply
* Re: [PATCH v7 11/12] platform/x86/amd/pmf: Add capability to sideload of policy binary
From: Hans de Goede @ 2023-12-18 11:58 UTC (permalink / raw)
To: Shyam Sundar S K, markgross, ilpo.jarvinen, basavaraj.natikar,
jikos, benjamin.tissoires
Cc: Patil.Reddy, mario.limonciello, platform-driver-x86, linux-input
In-Reply-To: <20231212014705.2017474-12-Shyam-sundar.S-k@amd.com>
Hi,
On 12/12/23 02:47, Shyam Sundar S K wrote:
> A policy binary is OS agnostic, and the same policies are expected to work
> across the OSes. At times it becomes difficult to debug when the policies
> inside the policy binaries starts to misbehave. Add a way to sideload such
> policies independently to debug them via a debugfs entry.
>
> Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> ---
> drivers/platform/x86/amd/pmf/pmf.h | 1 +
> drivers/platform/x86/amd/pmf/tee-if.c | 60 +++++++++++++++++++++++++++
> 2 files changed, 61 insertions(+)
>
> diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
> index 55cd2b301bbb..16999c5b334f 100644
> --- a/drivers/platform/x86/amd/pmf/pmf.h
> +++ b/drivers/platform/x86/amd/pmf/pmf.h
> @@ -219,6 +219,7 @@ struct amd_pmf_dev {
> bool cnqf_supported;
> struct notifier_block pwr_src_notifier;
> /* Smart PC solution builder */
> + struct dentry *esbin;
> unsigned char *policy_buf;
> u32 policy_sz;
> struct tee_context *tee_ctx;
> diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
> index 38b75198cc3f..cf95251741c7 100644
> --- a/drivers/platform/x86/amd/pmf/tee-if.c
> +++ b/drivers/platform/x86/amd/pmf/tee-if.c
> @@ -8,6 +8,7 @@
> * Author: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> */
>
> +#include <linux/debugfs.h>
> #include <linux/tee_drv.h>
> #include <linux/uuid.h>
> #include "pmf.h"
> @@ -16,9 +17,14 @@
>
> /* Policy binary actions sampling frequency (in ms) */
> static int pb_actions_ms = MSEC_PER_SEC;
> +/* Sideload policy binaries to debug policy failures */
> +static bool pb_side_load;
> +
> #ifdef CONFIG_AMD_PMF_DEBUG
> module_param(pb_actions_ms, int, 0644);
> MODULE_PARM_DESC(pb_actions_ms, "Policy binary actions sampling frequency (default = 1000ms)");
> +module_param(pb_side_load, bool, 0444);
> +MODULE_PARM_DESC(pb_side_load, "Sideload policy binaries debug policy failures");
> #endif
>
> static const uuid_t amd_pmf_ta_uuid = UUID_INIT(0x6fd93b77, 0x3fb8, 0x524d,
> @@ -269,6 +275,54 @@ static int amd_pmf_start_policy_engine(struct amd_pmf_dev *dev)
> return 0;
> }
>
> +#ifdef CONFIG_AMD_PMF_DEBUG
> +static ssize_t amd_pmf_get_pb_data(struct file *filp, const char __user *buf,
> + size_t length, loff_t *pos)
> +{
> + struct amd_pmf_dev *dev = filp->private_data;
> + int ret;
> +
> + /* Policy binary size cannot exceed POLICY_BUF_MAX_SZ */
> + if (length > POLICY_BUF_MAX_SZ || length == 0)
> + return -EINVAL;
> +
> + dev->policy_sz = length;
> +
> + /* re-alloc to the new buffer length of the policy binary */
You are leaking the original policy_buf here. Also by
storing length + the kzalloc result in policy_sz + policy_buf
before a successful kzalloc() + copy_from_usr() you are
leaving things in a state where there is no valid policy_buf
on error exits, where as other code assumes there always
is a valid policy_buf.
I have squashed in the following fix to fix both these issues:
diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
index 9a4757f4f521..502ce93d5cdd 100644
--- a/drivers/platform/x86/amd/pmf/tee-if.c
+++ b/drivers/platform/x86/amd/pmf/tee-if.c
@@ -286,22 +286,25 @@ static ssize_t amd_pmf_get_pb_data(struct file *filp, const char __user *buf,
size_t length, loff_t *pos)
{
struct amd_pmf_dev *dev = filp->private_data;
+ unsigned char *new_policy_buf;
int ret;
/* Policy binary size cannot exceed POLICY_BUF_MAX_SZ */
if (length > POLICY_BUF_MAX_SZ || length == 0)
return -EINVAL;
- dev->policy_sz = length;
-
/* re-alloc to the new buffer length of the policy binary */
- dev->policy_buf = kzalloc(dev->policy_sz, GFP_KERNEL);
- if (!dev->policy_buf)
+ new_policy_buf = kzalloc(length, GFP_KERNEL);
+ if (!new_policy_buf)
return -ENOMEM;
- if (copy_from_user(dev->policy_buf, buf, dev->policy_sz))
+ if (copy_from_user(new_policy_buf, buf, length))
return -EFAULT;
+ kfree(dev->policy_buf);
+ dev->policy_buf = new_policy_buf;
+ dev->policy_sz = length;
+
amd_pmf_hex_dump_pb(dev);
ret = amd_pmf_start_policy_engine(dev);
if (ret)
Regards,
Hans
> + dev->policy_buf = kzalloc(dev->policy_sz, GFP_KERNEL);
> + if (!dev->policy_buf)
> + return -ENOMEM;
> +
> + if (copy_from_user(dev->policy_buf, buf, dev->policy_sz))
> + return -EFAULT;
> +
> + ret = amd_pmf_start_policy_engine(dev);
> + if (ret)
> + return -EINVAL;
> +
> + return length;
> +}
> +
> +static const struct file_operations pb_fops = {
> + .write = amd_pmf_get_pb_data,
> + .open = simple_open,
> +};
> +
> +static void amd_pmf_open_pb(struct amd_pmf_dev *dev, struct dentry *debugfs_root)
> +{
> + dev->esbin = debugfs_create_dir("pb", debugfs_root);
> + debugfs_create_file("update_policy", 0644, dev->esbin, dev, &pb_fops);
> +}
> +
> +static void amd_pmf_remove_pb(struct amd_pmf_dev *dev)
> +{
> + debugfs_remove_recursive(dev->esbin);
> +}
> +#else
> +static void amd_pmf_open_pb(struct amd_pmf_dev *dev, struct dentry *debugfs_root) {}
> +static void amd_pmf_remove_pb(struct amd_pmf_dev *dev) {}
> +#endif
> +
> static int amd_pmf_get_bios_buffer(struct amd_pmf_dev *dev)
> {
> dev->policy_buf = kzalloc(dev->policy_sz, GFP_KERNEL);
> @@ -281,6 +335,9 @@ static int amd_pmf_get_bios_buffer(struct amd_pmf_dev *dev)
>
> memcpy(dev->policy_buf, dev->policy_base, dev->policy_sz);
>
> + if (pb_side_load)
> + amd_pmf_open_pb(dev, dev->dbgfs_dir);
> +
> return amd_pmf_start_policy_engine(dev);
> }
>
> @@ -393,6 +450,9 @@ int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev)
>
> void amd_pmf_deinit_smart_pc(struct amd_pmf_dev *dev)
> {
> + if (pb_side_load)
> + amd_pmf_remove_pb(dev);
> +
> kfree(dev->prev_data);
> kfree(dev->policy_buf);
> cancel_delayed_work_sync(&dev->pb_work);
^ permalink raw reply related
* Re: [PATCH v7 00/12] Introduce PMF Smart PC Solution Builder Feature
From: Hans de Goede @ 2023-12-18 12:05 UTC (permalink / raw)
To: Shyam Sundar S K, markgross, ilpo.jarvinen, basavaraj.natikar,
jikos, benjamin.tissoires
Cc: Patil.Reddy, mario.limonciello, platform-driver-x86, linux-input
In-Reply-To: <20231212014705.2017474-1-Shyam-sundar.S-k@amd.com>
Hi,
On 12/12/23 02:46, Shyam Sundar S K wrote:
> Smart PC Solutions Builder allows for OEM to define a large number of
> custom system states to dynamically switch to. The system states are
> referred to as policies, and multiple policies can be loaded onto the
> system at any given time, however only one policy can be active at a
> given time.
>
> Policy is a combination of PMF input and output capabilities. The inputs
> are the incoming information from the other kernel subsystems like LID
> state, Sensor info, GPU info etc and the actions are the updating the
> power limits of SMU etc.
>
> The policy binary is signed and encrypted by a special key from AMD. This
> policy binary shall have the inputs and outputs which the OEMs can build
> for the platform customization that can enhance the user experience and
> system behavior.
>
> This series adds the initial support for Smart PC solution to PMF driver.
>
> Note that, on platforms where CnQF and Smart PC is advertised, Smart PC
> shall have higher precedence and same applies for Auto Mode.
Thank you for your patch-series, I've applied the series to my
review-hans branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans
Once I've run some tests on this branch the patches there will be
added to the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.
Regards,
Hans
>
> v6->v7:
> ---------
> - handle buffer free during suspend/resume
> - Move Smart PC checks within Smart PC init function
> - realloc a updated buffer size during the side load.
> - Drop patches from 13/15 to 15/15 of V6 series
>
> v5->v6:
> ---------
> - Add Ilpo's and Mario's Reviewed-by tags
> - Drop 13/17 and 14/17 patches from this series which are GPU centric
> - Drop separate checks for battery handling.
> - Handle SFH failure cases
>
> v4->v5:
> ---------
> - Remove PMF-GPU interface from amdgpu driver and add DRM/backlight
> changes within PMF
> - Add module_softdep for AMDGPU
> - remove error checks for debugfs_create_file()
> - Add "Reviewed-by:" tags
> - Add kerneldoc for kernel-wide headers
> - Add checks for acpi_backlight_native
> - Add early return for SFH path
> - other cosmetic changes
>
> v3->v4:
> ---------
> - Split v3 9/16 into 2 patches, that addresses using generic fn names
> - Add softdep [Ilpo] instead of request_module()
> - return proper ACPI status [Mario]
> - Update comments in code [Mario]
> - Remove missed double _ remarks
> - handle battery status branches [Ilpo]
> - Address KASAN problems
>
> v2->v3:
> ---------
> - Remove pci_get_device() for getting gpu handle
> - add .suspend handler for pmf driver
> - remove unwanted type caste
> - Align comments, spaces etc.
> - add wrapper for print_hex_dump_debug()
> - Remove lkp tags in commit-msg
> - Add macros for magic numbers
> - use right format specifiers for printing
> - propagate error codes back to the caller
> - remove unwanted comments
>
> v1->v2:
> ---------
> - Remove __func__ macros
> - Remove manual function names inside prints
> - Handle tee_shm_get_va() failure
> - Remove double _
> - Add meaningful prints
> - pass amd_pmf_set_dram_addr() failure errors
> - Add more information to commit messages
> - use right format specifiers
> - use devm_ioremap() instead of ioremap()
> - address unsigned long vs u32 problems
> - Fix lkp reported issues
> - Add amd_pmf_remove_pb() to remove the debugfs files created(if any).
> - Make amd_pmf_open_pb() as static.
> - Add cooling device APIs for controlling amdgpu backlight
> - handle amd_pmf_apply_policies() failures
> - Split v1 14/15 into 2 patches further
> - use linux/units.h for better handling
> - add "depends on" AMD_SFH_HID for interaction with SFH
> - other cosmetic remarks
>
> Shyam Sundar S K (12):
> platform/x86/amd/pmf: Add PMF TEE interface
> platform/x86/amd/pmf: Add support for PMF-TA interaction
> platform/x86/amd/pmf: Change return type of amd_pmf_set_dram_addr()
> platform/x86/amd/pmf: Add support for PMF Policy Binary
> platform/x86/amd/pmf: change amd_pmf_init_features() call sequence
> platform/x86/amd/pmf: Add support to get inputs from other subsystems
> platform/x86/amd/pmf: Add support update p3t limit
> platform/x86/amd/pmf: Add support to update system state
> platform/x86/amd/pmf: Make source_as_str() as non-static
> platform/x86/amd/pmf: Add facility to dump TA inputs
> platform/x86/amd/pmf: Add capability to sideload of policy binary
> platform/x86/amd/pmf: dump policy binary data
>
> Documentation/admin-guide/index.rst | 1 +
> Documentation/admin-guide/pmf.rst | 24 ++
> drivers/platform/x86/amd/pmf/Kconfig | 1 +
> drivers/platform/x86/amd/pmf/Makefile | 3 +-
> drivers/platform/x86/amd/pmf/acpi.c | 37 ++
> drivers/platform/x86/amd/pmf/core.c | 52 ++-
> drivers/platform/x86/amd/pmf/pmf.h | 203 +++++++++++
> drivers/platform/x86/amd/pmf/spc.c | 158 +++++++++
> drivers/platform/x86/amd/pmf/sps.c | 5 +-
> drivers/platform/x86/amd/pmf/tee-if.c | 469 ++++++++++++++++++++++++++
> 10 files changed, 936 insertions(+), 17 deletions(-)
> create mode 100644 Documentation/admin-guide/pmf.rst
> create mode 100644 drivers/platform/x86/amd/pmf/spc.c
> create mode 100644 drivers/platform/x86/amd/pmf/tee-if.c
>
^ permalink raw reply
* [PATCH AUTOSEL 6.6 06/18] HID: Add quirk for Labtec/ODDOR/aikeec handbrake
From: Sasha Levin @ 2023-12-18 12:43 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Sebastian Parschauer, Jiri Kosina, Sasha Levin, jikos,
benjamin.tissoires, linux-input
In-Reply-To: <20231218124415.1379060-1-sashal@kernel.org>
From: Sebastian Parschauer <s.parschauer@gmx.de>
[ Upstream commit 31e52523267faab5ed8569b9d5c22c9a2283872f ]
This device needs ALWAYS_POLL quirk, otherwise it keeps reconnecting
indefinitely. It is a handbrake for sim racing detected as joystick.
Reported and tested by GitHub user N0th1ngM4tt3rs.
Link: https://github.com/sriemer/fix-linux-mouse issue 22
Signed-off-by: Sebastian Parschauer <s.parschauer@gmx.de>
Signed-off-by: Jiri Kosina <jkosina@suse.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/hid-ids.h | 1 +
drivers/hid/hid-quirks.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index d10ccfa17e168..97ab317570dd3 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -744,6 +744,7 @@
#define USB_VENDOR_ID_LABTEC 0x1020
#define USB_DEVICE_ID_LABTEC_WIRELESS_KEYBOARD 0x0006
+#define USB_DEVICE_ID_LABTEC_ODDOR_HANDBRAKE 0x8888
#define USB_VENDOR_ID_LCPOWER 0x1241
#define USB_DEVICE_ID_LCPOWER_LC1000 0xf767
diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
index 5a48fcaa32f00..d9a4f8f7bbb07 100644
--- a/drivers/hid/hid-quirks.c
+++ b/drivers/hid/hid-quirks.c
@@ -120,6 +120,7 @@ static const struct hid_device_id hid_quirks[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_KYE, USB_DEVICE_ID_KYE_EASYPEN_M406XE), HID_QUIRK_MULTI_INPUT },
{ HID_USB_DEVICE(USB_VENDOR_ID_KYE, USB_DEVICE_ID_KYE_MOUSEPEN_I608X_V2), HID_QUIRK_MULTI_INPUT },
{ HID_USB_DEVICE(USB_VENDOR_ID_KYE, USB_DEVICE_ID_KYE_PENSKETCH_T609A), HID_QUIRK_MULTI_INPUT },
+ { HID_USB_DEVICE(USB_VENDOR_ID_LABTEC, USB_DEVICE_ID_LABTEC_ODDOR_HANDBRAKE), HID_QUIRK_ALWAYS_POLL },
{ HID_USB_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_OPTICAL_USB_MOUSE_600E), HID_QUIRK_ALWAYS_POLL },
{ HID_USB_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_PIXART_USB_MOUSE_608D), HID_QUIRK_ALWAYS_POLL },
{ HID_USB_DEVICE(USB_VENDOR_ID_LENOVO, USB_DEVICE_ID_LENOVO_PIXART_USB_MOUSE_6019), HID_QUIRK_ALWAYS_POLL },
--
2.43.0
^ permalink raw reply related
* Re: [RFC PATCH 1/5] dt-bindings: mfd: add entry for the Marvell 88PM88X PMICs
From: Rob Herring @ 2023-12-18 15:17 UTC (permalink / raw)
To: Karel Balej
Cc: Karel Balej, Dmitry Torokhov, Krzysztof Kozlowski, Conor Dooley,
Lee Jones, linux-input, devicetree, linux-kernel,
Duje Mihanović, ~postmarketos/upstreaming, phone-devel
In-Reply-To: <20231217131838.7569-2-karelb@gimli.ms.mff.cuni.cz>
On Sun, Dec 17, 2023 at 02:16:59PM +0100, Karel Balej wrote:
> From: Karel Balej <balejk@matfyz.cz>
>
> Marvell 88PM880 and 88PM886 are two similar PMICs with mostly matching
> register mapping and subdevices such as onkey, regulators or battery and
> charger. Both seem to come in two revisions which seem to be handled
> slightly differently in some subdevice drivers.
>
> Signed-off-by: Karel Balej <balejk@matfyz.cz>
> ---
> .../bindings/mfd/marvell,88pm88x.yaml | 55 +++++++++++++++++++
> 1 file changed, 55 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml
>
> diff --git a/Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml b/Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml
> new file mode 100644
> index 000000000000..e075729c360f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml
> @@ -0,0 +1,55 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/marvell,88pm88x.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Marvell 88PM88X PMIC core MFD
Drop 'MFD'.
> +
> +maintainers:
> + - Karel Balej <balejk@matfyz.cz>
> +
> +description: |
Don't need '|' as there is no formatting to preserve.
> + Marvell 88PM880 and 88PM886 are two similar PMICs providing
> + several functions such as onkey, regulators or battery and
> + charger. Both seem to come in two revisions -- A0 and A1.
> +
> +properties:
> + compatible:
> + const: marvell,88pm886-a1
The description talks about 4 different devices, but only 1 here.
Do you expect to need A0 support? Devices with these PMICs should be
known and few, right?
> +
> + reg:
> + description: I2C device address
Drop.
> + maxItems: 1
> +
> + interrupt-controller: true
> +
> + interrupts:
> + maxItems: 1
> +
> + "#interrupt-cells":
> + const: 2
> +
> +required:
> + - compatible
> + - reg
> + - interrupt-controller
> + - interrupts
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + pmic0: 88pm886@30 {
pmic@30
Drop the unused label.
> + compatible = "marvell,88pm886-a1";
> + reg = <0x30>;
> + interrupts = <0 4 IRQ_TYPE_LEVEL_HIGH>;
You need the header for this.
You'll find the input binding fails too. Please test your bindings
before sending.
> + interrupt-parent = <&gic>;
> + interrupt-controller;
> + #interrupt-cells = <1>;
> + };
> + };
> +...
> --
> 2.43.0
>
^ permalink raw reply
* Re: [RFC PATCH 3/5] dt-bindings: input: add entry for 88pm88x-onkey
From: Rob Herring @ 2023-12-18 15:18 UTC (permalink / raw)
To: Karel Balej
Cc: Karel Balej, Dmitry Torokhov, Krzysztof Kozlowski, Conor Dooley,
Lee Jones, linux-input, devicetree, linux-kernel,
Duje Mihanović, ~postmarketos/upstreaming, phone-devel
In-Reply-To: <20231217131838.7569-4-karelb@gimli.ms.mff.cuni.cz>
On Sun, Dec 17, 2023 at 02:17:01PM +0100, Karel Balej wrote:
> From: Karel Balej <balejk@matfyz.cz>
>
> Marvell 88PM88X PMICs provide onkey functionality. Document it.
>
> Signed-off-by: Karel Balej <balejk@matfyz.cz>
> ---
> .../bindings/input/marvell,88pm88x-onkey.yaml | 30 +++++++++++++++++++
> .../bindings/mfd/marvell,88pm88x.yaml | 4 +++
> 2 files changed, 34 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/input/marvell,88pm88x-onkey.yaml
>
> diff --git a/Documentation/devicetree/bindings/input/marvell,88pm88x-onkey.yaml b/Documentation/devicetree/bindings/input/marvell,88pm88x-onkey.yaml
> new file mode 100644
> index 000000000000..aeb7673189f8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/marvell,88pm88x-onkey.yaml
> @@ -0,0 +1,30 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/input/marvell,88pm88x-onkey.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Onkey driver for Marvell 88PM88X PMICs.
> +
> +maintainers:
> + - Karel Balej <balejk@matfyz.cz>
> +
> +description: |
> + This module is part of the 88PM88X MFD device. For more details
> + see Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml.
> +
> + The onkey controller is represented as a sub-node of the PMIC node in
> + the device tree.
> +
> +allOf:
> + - $ref: input.yaml#
> +
> +properties:
> + compatible:
> + const: marvell,88pm88x-onkey
> +
> +required:
> + - compatible
> +
> +additionalProperties: false
> +...
> diff --git a/Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml b/Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml
> index e075729c360f..115b41c9f22c 100644
> --- a/Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml
> +++ b/Documentation/devicetree/bindings/mfd/marvell,88pm88x.yaml
> @@ -50,6 +50,10 @@ examples:
> interrupt-parent = <&gic>;
> interrupt-controller;
> #interrupt-cells = <1>;
> +
> + onkey {
> + compatible = "marvell,88pm88x-onkey";
> + };
Why do you need this? You have no properties for it. The parent driver
can instantiate child drivers. You don't need a DT node for that.
Rob
^ permalink raw reply
* Re: [PATCH] HID: nintendo: Prevent divide-by-zero on code
From: Jiri Kosina @ 2023-12-18 15:50 UTC (permalink / raw)
To: Guilherme G. Piccoli
Cc: djogorchock, linux-input, benjamin.tissoires, kernel, kernel-dev
In-Reply-To: <20231205211628.993129-1-gpiccoli@igalia.com>
On Tue, 5 Dec 2023, Guilherme G. Piccoli wrote:
> It was reported [0] that adding a generic joycon to the system caused
> a kernel crash on Steam Deck, with the below panic spew:
>
> divide error: 0000 [#1] PREEMPT SMP NOPTI
> [...]
> Hardware name: Valve Jupiter/Jupiter, BIOS F7A0119 10/24/2023
> RIP: 0010:nintendo_hid_event+0x340/0xcc1 [hid_nintendo]
> [...]
> Call Trace:
> [...]
> ? exc_divide_error+0x38/0x50
> ? nintendo_hid_event+0x340/0xcc1 [hid_nintendo]
> ? asm_exc_divide_error+0x1a/0x20
> ? nintendo_hid_event+0x307/0xcc1 [hid_nintendo]
> hid_input_report+0x143/0x160
> hidp_session_run+0x1ce/0x700 [hidp]
>
> Since it's a divide-by-0 error, by tracking the code for potential
> denominator issues, we've spotted 2 places in which this could happen;
> so let's guard against the possibility and log in the kernel if the
> condition happens. This is specially useful since some data that
> fills some denominators are read from the joycon HW in some cases,
> increasing the potential for flaws.
>
> [0] https://github.com/ValveSoftware/SteamOS/issues/1070
>
> Signed-off-by: Guilherme G. Piccoli <gpiccoli@igalia.com>
Thanks a lot for the fix. Is it confirmed to fix the issue by either of
the reporters? (that's not clear to me from the github issue).
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH 0/2] Fix regression in ALS
From: Jonathan Cameron @ 2023-12-18 16:22 UTC (permalink / raw)
To: Srinivas Pandruvada
Cc: jikos, jic23, lars, Basavaraj.Natikar, linux-input, linux-iio,
linux-kernel, gregkh
In-Reply-To: <20231217200703.719876-1-srinivas.pandruvada@linux.intel.com>
On Sun, 17 Dec 2023 12:07:01 -0800
Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> wrote:
> Addition of color temperature and chromaticity support breaks ALS sensor
> on several platforms. Till we have a good solution, revert these commits
> for 6.7 cycle.
>
> Srinivas Pandruvada (2):
> Revert "iio: hid-sensor-als: Add light chromaticity support"
> Revert "iio: hid-sensor-als: Add light color temperature support"
>
> drivers/iio/light/hid-sensor-als.c | 100 +----------------------------
> include/linux/hid-sensor-ids.h | 4 --
> 2 files changed, 2 insertions(+), 102 deletions(-)
+CC Greg KH. (resent as I messed up Greg's address first time around)
Hi Greg,
This is a regression fix that I'd like to get in asap. Currently light sensors
on a wide range of laptops are broken. I was hoping we'd fix the the problem
rather than need to revert, but time is running out so revert it is.
I don't have anything else that needs to be rushed in before the merge cycle,
so if you are happy to pick these two reverts directly that would be great.
Message ID of the cover letter is
20231217200703.719876-1-srinivas.pandruvada@linux.intel.com
Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
If not I should be able to do a pull request in next couple of days
with these in.
Thanks,
Jonathan
>
^ permalink raw reply
* Re: [RFC] dt-bindings: input: Clarify that abs_min must be less than abs_max
From: Chris Morgan @ 2023-12-18 17:11 UTC (permalink / raw)
To: Artur Rojek
Cc: Chris Morgan, linux-input, devicetree, conor+dt,
krzysztof.kozlowski+dt, robh+dt, dmitry.torokhov, Paul Cercueil
In-Reply-To: <03a9a56362b0559234d4a21a4de3e32e@artur-rojek.eu>
On Fri, Dec 15, 2023 at 12:19:51PM +0100, Artur Rojek wrote:
> On 2023-12-15 03:40, Chris Morgan wrote:
> > From: Chris Morgan <macromorgan@hotmail.com>
> >
> > uinput refuses to work with abs devices where the min value is greater
> > than the max value. uinput_validate_absinfo() returns -EINVAL if this
> > is the case and prevents using uinput on such a device. Since uinput
> > has worked this way since at least kernel 2.6 (or prior) I presume that
> > this is the correct way of doing things, and that this documentation
> > needs to be clarified that min must always be less than max.
> >
> > uinput is used in my use case to bind together adc-joystick devices
> > with gpio-keys devices to create a single unified gamepad for
> > userspace.
> >
> > Note that there are several boards that will need to be corrected,
> > all but a few of them I maintain. Submitting as an RFC for now to get
> > comments from the input team and the original author in case there is
> > something I am missing.
> >
> > Fixes: 7956b0d4694f ("dt-bindings: input: Add docs for ADC driven
> > joystick")
> >
> > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > ---
> > Documentation/devicetree/bindings/input/adc-joystick.yaml | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/input/adc-joystick.yaml
> > b/Documentation/devicetree/bindings/input/adc-joystick.yaml
> > index 6c244d66f8ce..8f5cdd5ef190 100644
> > --- a/Documentation/devicetree/bindings/input/adc-joystick.yaml
> > +++ b/Documentation/devicetree/bindings/input/adc-joystick.yaml
> > @@ -73,8 +73,9 @@ patternProperties:
> > description: >
> > Minimum and maximum values produced by the axis.
> > For an ABS_X axis this will be the left-most and right-most
> > - inclination of the joystick. If min > max, it is left to
> > userspace to
> > - treat the axis as inverted.
> > + inclination of the joystick. The axis must always be
> > expressed as
> > + min < max, if the axis is inverted it is left to userspace to
> > handle
> > + the inversion.
>
> Hi Chris,
>
> Device Tree is supposed to depict the actual state of the hardware.
> I worded the adc-joytick's adc-range property specifically, so that it
> covers a case of GCW Zero hardware [1], which has a joystick, where the
> ABS_X axis reports increasing values for the left-wards inclination of
> the joystick, and decreasing values for the right-wards inclination. You
> are saying that there are even more boards that need to be corrected -
> those are all situations, where DT depicts the actual behavior of the
> hardware.
> What you are trying to do is change hardware description, because of how
> a driver in an OS works. You should instead fix behavior of said driver,
> even if nobody stumbled upon that issue since 2.6 :) We fixed libSDL [2]
> for the same reason.
>
> Cheers,
> Artur
>
> PS. cc'd Paul to the conversation.
>
> [1] https://github.com/OpenDingux/linux/blob/jz-6.1/arch/mips/boot/dts/ingenic/gcw0.dts#L273C12-L273C12
> [2] https://github.com/libsdl-org/SDL-1.2/commit/46806790ad043
Thank you. Okay, I'll update uinput instead to drop checks for
min > max, since that's valid/allowed.
Chris
>
> > This property is interpreted as two signed 32 bit values.
> >
> > abs-fuzz:
>
^ permalink raw reply
* [PATCH] input: uinput: Drop checks for abs_min > abs_max
From: Chris Morgan @ 2023-12-18 17:16 UTC (permalink / raw)
To: linux-input
Cc: dmitry.torokhov, svv, biswarupp, peter.hutterer, paul, contact,
Chris Morgan
From: Chris Morgan <macromorgan@hotmail.com>
Stop checking if the minimum abs value is greater than the maximum abs
value. When the axis is inverted this condition is allowed. Without
relaxing this check, it is not possible to use uinput on devices in
userspace with an inverted axis, such as the adc-joystick found on
many handheld gaming devices.
Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
---
drivers/input/misc/uinput.c | 9 +--------
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
index d98212d55108..e90dbf2c0b34 100644
--- a/drivers/input/misc/uinput.c
+++ b/drivers/input/misc/uinput.c
@@ -403,14 +403,7 @@ static int uinput_validate_absinfo(struct input_dev *dev, unsigned int code,
min = abs->minimum;
max = abs->maximum;
- if ((min != 0 || max != 0) && max < min) {
- printk(KERN_DEBUG
- "%s: invalid abs[%02x] min:%d max:%d\n",
- UINPUT_NAME, code, min, max);
- return -EINVAL;
- }
-
- if (!check_sub_overflow(max, min, &range) && abs->flat > range) {
+ if (!check_sub_overflow(max, min, &range) && abs->flat > abs(range)) {
printk(KERN_DEBUG
"%s: abs_flat #%02x out of range: %d (min:%d/max:%d)\n",
UINPUT_NAME, code, abs->flat, min, max);
--
2.34.1
^ permalink raw reply related
* Re: [PATCH] HID: nintendo: Prevent divide-by-zero on code
From: Jiri Kosina @ 2023-12-18 19:47 UTC (permalink / raw)
To: Sam Lantinga
Cc: Guilherme G. Piccoli, djogorchock, linux-input,
benjamin.tissoires, kernel, kernel-dev
In-Reply-To: <CACC3sbFGHHONh=orX2+VuRu1SdGXu-jhhFVE-xZe1wOBodUzpQ@mail.gmail.com>
On Mon, 18 Dec 2023, Sam Lantinga wrote:
> Tested-by: Sam Lantinga <slouken@libsdl.org>
Thanks. Applied to hid.git#for-6.7/upstream-fixes
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] HID: nintendo: Prevent divide-by-zero on code
From: Guilherme G. Piccoli @ 2023-12-18 19:22 UTC (permalink / raw)
To: Jiri Kosina, slouken
Cc: djogorchock, linux-input, benjamin.tissoires, kernel, kernel-dev
In-Reply-To: <nycvar.YFH.7.76.2312181649360.24250@cbobk.fhfr.pm>
On 18/12/2023 12:50, Jiri Kosina wrote:
> [...]
>
> Thanks a lot for the fix. Is it confirmed to fix the issue by either of
> the reporters? (that's not clear to me from the github issue).
>
> Thanks,
>
Hi Jiri, thanks for the message!
Yes, not only in github but internally - Slouken got the same issue, and
this is fixed to him, so I'm CCing his email so we can have a Tested tag.
Cheers,
Guilherme
^ permalink raw reply
* [PATCH v2 0/3] Add support of color temperature and chromaticity
From: Srinivas Pandruvada @ 2023-12-18 20:30 UTC (permalink / raw)
To: jikos, jic23, lars, Basavaraj.Natikar
Cc: linux-input, linux-iio, linux-kernel, Srinivas Pandruvada
The original series submitted to 6.7 (before revert) is modified to
solve regression issues on several platforms. There is one change
introduced before adding support for new features to allow dynamic
addition of channels.
This series is for the kernel version 6.8+.
v2:
New change to add channels dynamically
Modified color temperature and chromaticity to skip in case
of failures
Basavaraj Natikar (2):
iio: hid-sensor-als: Add light color temperature support
iio: hid-sensor-als: Add light chromaticity support
Srinivas Pandruvada (1):
iio: hid-sensor-als: Allocate channels dynamically
drivers/iio/light/hid-sensor-als.c | 164 ++++++++++++++++++++++++-----
include/linux/hid-sensor-ids.h | 4 +
2 files changed, 144 insertions(+), 24 deletions(-)
--
2.43.0
^ permalink raw reply
* [PATCH v2 1/3] iio: hid-sensor-als: Allocate channels dynamically
From: Srinivas Pandruvada @ 2023-12-18 20:30 UTC (permalink / raw)
To: jikos, jic23, lars, Basavaraj.Natikar
Cc: linux-input, linux-iio, linux-kernel, Srinivas Pandruvada
In-Reply-To: <20231218203026.1156375-1-srinivas.pandruvada@linux.intel.com>
Instead of assuming that every channel defined statically by
als_channels[] is present, allocate dynamically based on presence of the
respective usage id in the descriptor. This will allow to register ALS
with limited channel support. Append the timestamp as the last channel.
There is no intentional function changes done.
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
v2:
New change
drivers/iio/light/hid-sensor-als.c | 57 ++++++++++++++++++------------
1 file changed, 35 insertions(+), 22 deletions(-)
diff --git a/drivers/iio/light/hid-sensor-als.c b/drivers/iio/light/hid-sensor-als.c
index 5cd27f04b45e..e57ad1946b56 100644
--- a/drivers/iio/light/hid-sensor-als.c
+++ b/drivers/iio/light/hid-sensor-als.c
@@ -236,14 +236,21 @@ static int als_capture_sample(struct hid_sensor_hub_device *hsdev,
/* Parse report which is specific to an usage id*/
static int als_parse_report(struct platform_device *pdev,
- struct hid_sensor_hub_device *hsdev,
- struct iio_chan_spec *channels,
- unsigned usage_id,
- struct als_state *st)
+ struct hid_sensor_hub_device *hsdev,
+ struct iio_chan_spec **channels_out,
+ int *size_channels_out,
+ unsigned int usage_id,
+ struct als_state *st)
{
- int ret;
+ struct iio_chan_spec *channels;
+ int ret, index = 0;
int i;
+ channels = devm_kcalloc(&pdev->dev, ARRAY_SIZE(als_channels),
+ sizeof(als_channels), GFP_KERNEL);
+ if (!channels)
+ return -ENOMEM;
+
for (i = 0; i <= CHANNEL_SCAN_INDEX_ILLUM; ++i) {
ret = sensor_hub_input_get_attribute_info(hsdev,
HID_INPUT_REPORT,
@@ -251,9 +258,11 @@ static int als_parse_report(struct platform_device *pdev,
HID_USAGE_SENSOR_LIGHT_ILLUM,
&st->als[i]);
if (ret < 0)
- return ret;
- als_adjust_channel_bit_mask(channels, i, st->als[i].size);
+ break;
+ channels[i] = als_channels[i];
+ als_adjust_channel_bit_mask(channels, i, st->als[i].size);
+ ++index;
dev_dbg(&pdev->dev, "als %x:%x\n", st->als[i].index,
st->als[i].report_id);
}
@@ -262,17 +271,24 @@ static int als_parse_report(struct platform_device *pdev,
&st->als[CHANNEL_SCAN_INDEX_INTENSITY],
&st->scale_pre_decml, &st->scale_post_decml);
- return ret;
+ *channels_out = channels;
+ *size_channels_out = index;
+
+ if (!index)
+ ret = -ENODEV;
+
+ return 0;
}
/* Function to initialize the processing for usage id */
static int hid_als_probe(struct platform_device *pdev)
{
- int ret = 0;
+ int ret = 0, max_channel_index;
static const char *name = "als";
struct iio_dev *indio_dev;
struct als_state *als_state;
struct hid_sensor_hub_device *hsdev = pdev->dev.platform_data;
+ struct iio_chan_spec *channels;
indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(struct als_state));
if (!indio_dev)
@@ -293,24 +309,21 @@ static int hid_als_probe(struct platform_device *pdev)
return ret;
}
- indio_dev->channels = devm_kmemdup(&pdev->dev, als_channels,
- sizeof(als_channels), GFP_KERNEL);
- if (!indio_dev->channels) {
- dev_err(&pdev->dev, "failed to duplicate channels\n");
- return -ENOMEM;
- }
-
- ret = als_parse_report(pdev, hsdev,
- (struct iio_chan_spec *)indio_dev->channels,
- hsdev->usage,
- als_state);
+ ret = als_parse_report(pdev, hsdev, &channels, &max_channel_index,
+ hsdev->usage, als_state);
if (ret) {
dev_err(&pdev->dev, "failed to setup attributes\n");
return ret;
}
- indio_dev->num_channels =
- ARRAY_SIZE(als_channels);
+ /* Add timestamp channel */
+ channels[max_channel_index] = als_channels[CHANNEL_SCAN_INDEX_TIMESTAMP];
+ channels[max_channel_index].scan_index = max_channel_index;
+
+ /* +1 for adding timestamp channel */
+ indio_dev->num_channels = max_channel_index + 1;
+
+ indio_dev->channels = channels;
indio_dev->info = &als_info;
indio_dev->name = name;
indio_dev->modes = INDIO_DIRECT_MODE;
--
2.43.0
^ permalink raw reply related
* [PATCH v2 2/3] iio: hid-sensor-als: Add light color temperature support
From: Srinivas Pandruvada @ 2023-12-18 20:30 UTC (permalink / raw)
To: jikos, jic23, lars, Basavaraj.Natikar
Cc: linux-input, linux-iio, linux-kernel, Srinivas Pandruvada
In-Reply-To: <20231218203026.1156375-1-srinivas.pandruvada@linux.intel.com>
From: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
In most cases, ambient color sensors also support light color temperature.
As a result, add support of light color temperature.
Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
v2:
Original patch from Basavaraj Natikar <Basavaraj.Natikar@amd.com> is
modified to prevent failure when the new usage id is not found in the
descriptor.
drivers/iio/light/hid-sensor-als.c | 41 ++++++++++++++++++++++++++++--
include/linux/hid-sensor-ids.h | 1 +
2 files changed, 40 insertions(+), 2 deletions(-)
diff --git a/drivers/iio/light/hid-sensor-als.c b/drivers/iio/light/hid-sensor-als.c
index e57ad1946b56..8d6beacc338a 100644
--- a/drivers/iio/light/hid-sensor-als.c
+++ b/drivers/iio/light/hid-sensor-als.c
@@ -14,8 +14,9 @@
#include "../common/hid-sensors/hid-sensor-trigger.h"
enum {
- CHANNEL_SCAN_INDEX_INTENSITY = 0,
- CHANNEL_SCAN_INDEX_ILLUM = 1,
+ CHANNEL_SCAN_INDEX_INTENSITY,
+ CHANNEL_SCAN_INDEX_ILLUM,
+ CHANNEL_SCAN_INDEX_COLOR_TEMP,
CHANNEL_SCAN_INDEX_MAX
};
@@ -65,6 +66,16 @@ static const struct iio_chan_spec als_channels[] = {
BIT(IIO_CHAN_INFO_HYSTERESIS_RELATIVE),
.scan_index = CHANNEL_SCAN_INDEX_ILLUM,
},
+ {
+ .type = IIO_COLORTEMP,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+ .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |
+ BIT(IIO_CHAN_INFO_SCALE) |
+ BIT(IIO_CHAN_INFO_SAMP_FREQ) |
+ BIT(IIO_CHAN_INFO_HYSTERESIS) |
+ BIT(IIO_CHAN_INFO_HYSTERESIS_RELATIVE),
+ .scan_index = CHANNEL_SCAN_INDEX_COLOR_TEMP,
+ },
IIO_CHAN_SOFT_TIMESTAMP(CHANNEL_SCAN_INDEX_TIMESTAMP)
};
@@ -103,6 +114,11 @@ static int als_read_raw(struct iio_dev *indio_dev,
min = als_state->als[chan->scan_index].logical_minimum;
address = HID_USAGE_SENSOR_LIGHT_ILLUM;
break;
+ case CHANNEL_SCAN_INDEX_COLOR_TEMP:
+ report_id = als_state->als[chan->scan_index].report_id;
+ min = als_state->als[chan->scan_index].logical_minimum;
+ address = HID_USAGE_SENSOR_LIGHT_COLOR_TEMPERATURE;
+ break;
default:
report_id = -1;
break;
@@ -223,6 +239,10 @@ static int als_capture_sample(struct hid_sensor_hub_device *hsdev,
als_state->scan.illum[CHANNEL_SCAN_INDEX_ILLUM] = sample_data;
ret = 0;
break;
+ case HID_USAGE_SENSOR_LIGHT_COLOR_TEMPERATURE:
+ als_state->scan.illum[CHANNEL_SCAN_INDEX_COLOR_TEMP] = sample_data;
+ ret = 0;
+ break;
case HID_USAGE_SENSOR_TIME_TIMESTAMP:
als_state->timestamp = hid_sensor_convert_timestamp(&als_state->common_attributes,
*(s64 *)raw_data);
@@ -267,10 +287,27 @@ static int als_parse_report(struct platform_device *pdev,
st->als[i].report_id);
}
+ ret = sensor_hub_input_get_attribute_info(hsdev, HID_INPUT_REPORT,
+ usage_id,
+ HID_USAGE_SENSOR_LIGHT_COLOR_TEMPERATURE,
+ &st->als[CHANNEL_SCAN_INDEX_COLOR_TEMP]);
+ if (ret < 0)
+ goto skip_temp_channel;
+
+ channels[index++] = als_channels[CHANNEL_SCAN_INDEX_COLOR_TEMP];
+
+ als_adjust_channel_bit_mask(channels, CHANNEL_SCAN_INDEX_COLOR_TEMP,
+ st->als[CHANNEL_SCAN_INDEX_COLOR_TEMP].size);
+
+ dev_dbg(&pdev->dev, "als %x:%x\n",
+ st->als[CHANNEL_SCAN_INDEX_COLOR_TEMP].index,
+ st->als[CHANNEL_SCAN_INDEX_COLOR_TEMP].report_id);
+
st->scale_precision = hid_sensor_format_scale(usage_id,
&st->als[CHANNEL_SCAN_INDEX_INTENSITY],
&st->scale_pre_decml, &st->scale_post_decml);
+skip_temp_channel:
*channels_out = channels;
*size_channels_out = index;
diff --git a/include/linux/hid-sensor-ids.h b/include/linux/hid-sensor-ids.h
index 13b1e65fbdcc..8af4fb3e0254 100644
--- a/include/linux/hid-sensor-ids.h
+++ b/include/linux/hid-sensor-ids.h
@@ -21,6 +21,7 @@
#define HID_USAGE_SENSOR_ALS 0x200041
#define HID_USAGE_SENSOR_DATA_LIGHT 0x2004d0
#define HID_USAGE_SENSOR_LIGHT_ILLUM 0x2004d1
+#define HID_USAGE_SENSOR_LIGHT_COLOR_TEMPERATURE 0x2004d2
/* PROX (200011) */
#define HID_USAGE_SENSOR_PROX 0x200011
--
2.43.0
^ permalink raw reply related
* [PATCH v2 3/3] iio: hid-sensor-als: Add light chromaticity support
From: Srinivas Pandruvada @ 2023-12-18 20:30 UTC (permalink / raw)
To: jikos, jic23, lars, Basavaraj.Natikar
Cc: linux-input, linux-iio, linux-kernel, Srinivas Pandruvada
In-Reply-To: <20231218203026.1156375-1-srinivas.pandruvada@linux.intel.com>
From: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
In most cases, ambient color sensors also support the x and y light
colors, which represent the coordinates on the CIE 1931 chromaticity
diagram. Thus, add light chromaticity x and y.
Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
v2:
Original patch from Basavaraj Natikar <Basavaraj.Natikar@amd.com> is
modified to prevent failure when the new usage id is not found in the
descriptor.
drivers/iio/light/hid-sensor-als.c | 68 +++++++++++++++++++++++++++++-
include/linux/hid-sensor-ids.h | 3 ++
2 files changed, 70 insertions(+), 1 deletion(-)
diff --git a/drivers/iio/light/hid-sensor-als.c b/drivers/iio/light/hid-sensor-als.c
index 8d6beacc338a..6e2793fa515c 100644
--- a/drivers/iio/light/hid-sensor-als.c
+++ b/drivers/iio/light/hid-sensor-als.c
@@ -17,6 +17,8 @@ enum {
CHANNEL_SCAN_INDEX_INTENSITY,
CHANNEL_SCAN_INDEX_ILLUM,
CHANNEL_SCAN_INDEX_COLOR_TEMP,
+ CHANNEL_SCAN_INDEX_CHROMATICITY_X,
+ CHANNEL_SCAN_INDEX_CHROMATICITY_Y,
CHANNEL_SCAN_INDEX_MAX
};
@@ -76,6 +78,30 @@ static const struct iio_chan_spec als_channels[] = {
BIT(IIO_CHAN_INFO_HYSTERESIS_RELATIVE),
.scan_index = CHANNEL_SCAN_INDEX_COLOR_TEMP,
},
+ {
+ .type = IIO_CHROMATICITY,
+ .modified = 1,
+ .channel2 = IIO_MOD_X,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+ .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |
+ BIT(IIO_CHAN_INFO_SCALE) |
+ BIT(IIO_CHAN_INFO_SAMP_FREQ) |
+ BIT(IIO_CHAN_INFO_HYSTERESIS) |
+ BIT(IIO_CHAN_INFO_HYSTERESIS_RELATIVE),
+ .scan_index = CHANNEL_SCAN_INDEX_CHROMATICITY_X,
+ },
+ {
+ .type = IIO_CHROMATICITY,
+ .modified = 1,
+ .channel2 = IIO_MOD_Y,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+ .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |
+ BIT(IIO_CHAN_INFO_SCALE) |
+ BIT(IIO_CHAN_INFO_SAMP_FREQ) |
+ BIT(IIO_CHAN_INFO_HYSTERESIS) |
+ BIT(IIO_CHAN_INFO_HYSTERESIS_RELATIVE),
+ .scan_index = CHANNEL_SCAN_INDEX_CHROMATICITY_Y,
+ },
IIO_CHAN_SOFT_TIMESTAMP(CHANNEL_SCAN_INDEX_TIMESTAMP)
};
@@ -119,6 +145,16 @@ static int als_read_raw(struct iio_dev *indio_dev,
min = als_state->als[chan->scan_index].logical_minimum;
address = HID_USAGE_SENSOR_LIGHT_COLOR_TEMPERATURE;
break;
+ case CHANNEL_SCAN_INDEX_CHROMATICITY_X:
+ report_id = als_state->als[chan->scan_index].report_id;
+ min = als_state->als[chan->scan_index].logical_minimum;
+ address = HID_USAGE_SENSOR_LIGHT_CHROMATICITY_X;
+ break;
+ case CHANNEL_SCAN_INDEX_CHROMATICITY_Y:
+ report_id = als_state->als[chan->scan_index].report_id;
+ min = als_state->als[chan->scan_index].logical_minimum;
+ address = HID_USAGE_SENSOR_LIGHT_CHROMATICITY_Y;
+ break;
default:
report_id = -1;
break;
@@ -243,6 +279,14 @@ static int als_capture_sample(struct hid_sensor_hub_device *hsdev,
als_state->scan.illum[CHANNEL_SCAN_INDEX_COLOR_TEMP] = sample_data;
ret = 0;
break;
+ case HID_USAGE_SENSOR_LIGHT_CHROMATICITY_X:
+ als_state->scan.illum[CHANNEL_SCAN_INDEX_CHROMATICITY_X] = sample_data;
+ ret = 0;
+ break;
+ case HID_USAGE_SENSOR_LIGHT_CHROMATICITY_Y:
+ als_state->scan.illum[CHANNEL_SCAN_INDEX_CHROMATICITY_Y] = sample_data;
+ ret = 0;
+ break;
case HID_USAGE_SENSOR_TIME_TIMESTAMP:
als_state->timestamp = hid_sensor_convert_timestamp(&als_state->common_attributes,
*(s64 *)raw_data);
@@ -303,11 +347,33 @@ static int als_parse_report(struct platform_device *pdev,
st->als[CHANNEL_SCAN_INDEX_COLOR_TEMP].index,
st->als[CHANNEL_SCAN_INDEX_COLOR_TEMP].report_id);
+skip_temp_channel:
+ for (i = 0; i < 2; i++) {
+ int next_scan_index = CHANNEL_SCAN_INDEX_CHROMATICITY_X + i;
+
+ ret = sensor_hub_input_get_attribute_info(hsdev,
+ HID_INPUT_REPORT, usage_id,
+ HID_USAGE_SENSOR_LIGHT_CHROMATICITY_X + i,
+ &st->als[next_scan_index]);
+ if (ret < 0)
+ goto skip_chromaticity_channel;
+
+ channels[index++] = als_channels[CHANNEL_SCAN_INDEX_CHROMATICITY_X + i];
+
+ als_adjust_channel_bit_mask(channels,
+ CHANNEL_SCAN_INDEX_CHROMATICITY_X + i,
+ st->als[next_scan_index].size);
+
+ dev_dbg(&pdev->dev, "als %x:%x\n",
+ st->als[next_scan_index].index,
+ st->als[next_scan_index].report_id);
+ }
+
st->scale_precision = hid_sensor_format_scale(usage_id,
&st->als[CHANNEL_SCAN_INDEX_INTENSITY],
&st->scale_pre_decml, &st->scale_post_decml);
-skip_temp_channel:
+skip_chromaticity_channel:
*channels_out = channels;
*size_channels_out = index;
diff --git a/include/linux/hid-sensor-ids.h b/include/linux/hid-sensor-ids.h
index 8af4fb3e0254..6730ee900ee1 100644
--- a/include/linux/hid-sensor-ids.h
+++ b/include/linux/hid-sensor-ids.h
@@ -22,6 +22,9 @@
#define HID_USAGE_SENSOR_DATA_LIGHT 0x2004d0
#define HID_USAGE_SENSOR_LIGHT_ILLUM 0x2004d1
#define HID_USAGE_SENSOR_LIGHT_COLOR_TEMPERATURE 0x2004d2
+#define HID_USAGE_SENSOR_LIGHT_CHROMATICITY 0x2004d3
+#define HID_USAGE_SENSOR_LIGHT_CHROMATICITY_X 0x2004d4
+#define HID_USAGE_SENSOR_LIGHT_CHROMATICITY_Y 0x2004d5
/* PROX (200011) */
#define HID_USAGE_SENSOR_PROX 0x200011
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] HID: nintendo: Prevent divide-by-zero on code
From: Rahul Rameshbabu @ 2023-12-18 20:39 UTC (permalink / raw)
To: Guilherme G. Piccoli
Cc: djogorchock, linux-input, jikos, benjamin.tissoires, kernel,
kernel-dev
In-Reply-To: <20231205211628.993129-1-gpiccoli@igalia.com>
On Tue, 05 Dec, 2023 18:15:51 -0300 "Guilherme G. Piccoli" <gpiccoli@igalia.com> wrote:
> It was reported [0] that adding a generic joycon to the system caused
> a kernel crash on Steam Deck, with the below panic spew:
>
> divide error: 0000 [#1] PREEMPT SMP NOPTI
> [...]
> Hardware name: Valve Jupiter/Jupiter, BIOS F7A0119 10/24/2023
> RIP: 0010:nintendo_hid_event+0x340/0xcc1 [hid_nintendo]
> [...]
> Call Trace:
> [...]
> ? exc_divide_error+0x38/0x50
> ? nintendo_hid_event+0x340/0xcc1 [hid_nintendo]
> ? asm_exc_divide_error+0x1a/0x20
> ? nintendo_hid_event+0x307/0xcc1 [hid_nintendo]
> hid_input_report+0x143/0x160
> hidp_session_run+0x1ce/0x700 [hidp]
>
> Since it's a divide-by-0 error, by tracking the code for potential
> denominator issues, we've spotted 2 places in which this could happen;
> so let's guard against the possibility and log in the kernel if the
> condition happens. This is specially useful since some data that
> fills some denominators are read from the joycon HW in some cases,
> increasing the potential for flaws.
>
> [0] https://github.com/ValveSoftware/SteamOS/issues/1070
>
> Signed-off-by: Guilherme G. Piccoli <gpiccoli@igalia.com>
> ---
> drivers/hid/hid-nintendo.c | 27 ++++++++++++++++++++-------
> 1 file changed, 20 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
> index 138f154fecef..23f3f96c8c85 100644
> --- a/drivers/hid/hid-nintendo.c
> +++ b/drivers/hid/hid-nintendo.c
[...]
> @@ -1163,16 +1176,16 @@ static void joycon_parse_imu_report(struct joycon_ctlr *ctlr,
> JC_IMU_SAMPLES_PER_DELTA_AVG) {
> ctlr->imu_avg_delta_ms = ctlr->imu_delta_samples_sum /
> ctlr->imu_delta_samples_count;
> - /* don't ever want divide by zero shenanigans */
> - if (ctlr->imu_avg_delta_ms == 0) {
> - ctlr->imu_avg_delta_ms = 1;
> - hid_warn(ctlr->hdev,
> - "calculated avg imu delta of 0\n");
> - }
> ctlr->imu_delta_samples_count = 0;
> ctlr->imu_delta_samples_sum = 0;
> }
>
> + /* don't ever want divide by zero shenanigans */
> + if (ctlr->imu_avg_delta_ms == 0) {
> + ctlr->imu_avg_delta_ms = 1;
> + hid_warn(ctlr->hdev, "calculated avg imu delta of 0\n");
> + }
> +
Hi Guilherme,
I agree with the previous hunks you added and can see how those could
trigger the divide-by-zero issue you were seeing. However, I am not sure
if this hunk that I have left makes sense.
Reason being, is that the hid-nintendo driver has a special conditional
to prevent divide-by-zero in this case without this change.
1. If the first packet has not been received by the IMU, set
imu_avg_delta_ms to JC_IMU_DFLT_AVG_DELTA_MS.
2. Only change imu_avg_delta_ms when imu_delta_samples_count >=
JC_IMU_SAMPLES_PER_DELTA_AVG.
3. If that change leads to imu_avg_delta_ms being 0, set it to 1.
With this logic as-is, I do not see how a divide by zero could have
occurred in this specific path without your change. I might be missing
something, but I wanted to make sure to avoid integrating a hunk that
does not help.
Would it be possible to retest without this hunk?
> /* useful for debugging IMU sample rate */
> hid_dbg(ctlr->hdev,
> "imu_report: ms=%u last_ms=%u delta=%u avg_delta=%u\n",
--
Thanks,
Rahul Rameshbabu
^ permalink raw reply
* Re: [PATCH] HID: nintendo: Prevent divide-by-zero on code
From: Guilherme G. Piccoli @ 2023-12-18 21:46 UTC (permalink / raw)
To: Rahul Rameshbabu, jikos
Cc: djogorchock, linux-input, benjamin.tissoires, kernel, kernel-dev
In-Reply-To: <87o7enxn1x.fsf@protonmail.com>
On 18/12/2023 17:39, Rahul Rameshbabu wrote:
> [...]
>
> Hi Guilherme,
>
> I agree with the previous hunks you added and can see how those could
> trigger the divide-by-zero issue you were seeing. However, I am not sure
> if this hunk that I have left makes sense.
>
> Reason being, is that the hid-nintendo driver has a special conditional
> to prevent divide-by-zero in this case without this change.
>
> 1. If the first packet has not been received by the IMU, set
> imu_avg_delta_ms to JC_IMU_DFLT_AVG_DELTA_MS.
> 2. Only change imu_avg_delta_ms when imu_delta_samples_count >=
> JC_IMU_SAMPLES_PER_DELTA_AVG.
> 3. If that change leads to imu_avg_delta_ms being 0, set it to 1.
>
> With this logic as-is, I do not see how a divide by zero could have
> occurred in this specific path without your change. I might be missing
> something, but I wanted to make sure to avoid integrating a hunk that
> does not help.
>
> Would it be possible to retest without this hunk?
>
Hi Rahul, thanks for your review.
I think I see ... I covered both bases in this patch, but IIUC after
your points above and better looking the code:
(a) imu_avg_delta_ms is set to JC_IMU_DFLT_AVG_DELTA_MS and it can only
change iff imu_delta_samples_count >= JC_IMU_SAMPLES_PER_DELTA_AVG.
(b) But the if path related with the imu_delta_samples_count is the one
that also guards the divide-by-zero, so effectively that error condition
cannot happen outside that if path, i.e., my hunk changed nothing.
Ugh...my bad.
At the same time, the hunk is harmless - it's up to Jiri to decide, we
can drop it (either directly by git rebasing or I can send a V2 if Jiri
prefers), or we can keep it.
I can try to test internally, github testing may be a bit uncertain in
the timeframe (specially during holidays season). If Jiri thinks the
hunk is harmless and no change is necessary, I'd rather not bother
people for testing now (I don't have the HW).
Cheers,
Guilherme
^ permalink raw reply
* Re: [PATCH] HID: nintendo: Prevent divide-by-zero on code
From: Rahul Rameshbabu @ 2023-12-18 21:56 UTC (permalink / raw)
To: Guilherme G. Piccoli
Cc: jikos, djogorchock, linux-input, benjamin.tissoires, kernel,
kernel-dev
In-Reply-To: <dcd91e66-11ce-c576-5eb7-8756a1b6f222@igalia.com>
On Mon, 18 Dec, 2023 18:46:09 -0300 "Guilherme G. Piccoli" <gpiccoli@igalia.com> wrote:
>
> Hi Rahul, thanks for your review.
>
> I think I see ... I covered both bases in this patch, but IIUC after
> your points above and better looking the code:
>
> (a) imu_avg_delta_ms is set to JC_IMU_DFLT_AVG_DELTA_MS and it can only
> change iff imu_delta_samples_count >= JC_IMU_SAMPLES_PER_DELTA_AVG.
>
> (b) But the if path related with the imu_delta_samples_count is the one
> that also guards the divide-by-zero, so effectively that error condition
> cannot happen outside that if path, i.e., my hunk changed nothing.
> Ugh...my bad.
Right, no worries. I really appreciate this patch though for covering
the cases involving the gyro and the accelerometer.
>
> At the same time, the hunk is harmless - it's up to Jiri to decide, we
> can drop it (either directly by git rebasing or I can send a V2 if Jiri
> prefers), or we can keep it.
Agreed.
>
> I can try to test internally, github testing may be a bit uncertain in
> the timeframe (specially during holidays season). If Jiri thinks the
> hunk is harmless and no change is necessary, I'd rather not bother
> people for testing now (I don't have the HW).
>
Makes sense. Like discussed above, the change here is harmless in nature
but has no functional change at the same time.
--
Thanks,
Rahul Rameshbabu
^ permalink raw reply
* Re: [PATCH] HID: nintendo: Prevent divide-by-zero on code
From: Jiri Kosina @ 2023-12-18 22:27 UTC (permalink / raw)
To: Guilherme G. Piccoli
Cc: Rahul Rameshbabu, djogorchock, linux-input, benjamin.tissoires,
kernel, kernel-dev
In-Reply-To: <dcd91e66-11ce-c576-5eb7-8756a1b6f222@igalia.com>
On Mon, 18 Dec 2023, Guilherme G. Piccoli wrote:
> I think I see ... I covered both bases in this patch, but IIUC after
> your points above and better looking the code:
>
> (a) imu_avg_delta_ms is set to JC_IMU_DFLT_AVG_DELTA_MS and it can only
> change iff imu_delta_samples_count >= JC_IMU_SAMPLES_PER_DELTA_AVG.
>
> (b) But the if path related with the imu_delta_samples_count is the one
> that also guards the divide-by-zero, so effectively that error condition
> cannot happen outside that if path, i.e., my hunk changed nothing.
> Ugh...my bad.
>
> At the same time, the hunk is harmless - it's up to Jiri to decide, we
> can drop it (either directly by git rebasing or I can send a V2 if Jiri
> prefers), or we can keep it.
>
> I can try to test internally, github testing may be a bit uncertain in
> the timeframe (specially during holidays season). If Jiri thinks the
> hunk is harmless and no change is necessary, I'd rather not bother
> people for testing now (I don't have the HW).
My plan is to send this to Linus in a day or two, to have this fixed for
good in 6.7 final to be on the safe side.
We can always remove the potentially superfluous check (thanks Rahul for
spotting that) later once we get the testing results.
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] HID: nintendo: Prevent divide-by-zero on code
From: Guilherme G. Piccoli @ 2023-12-18 23:29 UTC (permalink / raw)
To: Jiri Kosina, Rahul Rameshbabu
Cc: djogorchock, linux-input, benjamin.tissoires, kernel, kernel-dev
In-Reply-To: <nycvar.YFH.7.76.2312182325460.24250@cbobk.fhfr.pm>
On 18/12/2023 19:27, Jiri Kosina wrote:
> On Mon, 18 Dec 2023, Guilherme G. Piccoli wrote:
> [...]
> My plan is to send this to Linus in a day or two, to have this fixed for
> good in 6.7 final to be on the safe side.
>
> We can always remove the potentially superfluous check (thanks Rahul for
> spotting that) later once we get the testing results.
>
> Thanks,
>
Thanks Jiri and Rahul! I agree with the approach, better to have it
fixed ASAP indeed.
I understand no action is necessary from my side now.
Cheers,
Guilherme
^ permalink raw reply
* Re: [PATCH] HID: nintendo: Prevent divide-by-zero on code
From: Jiri Kosina @ 2023-12-18 23:49 UTC (permalink / raw)
To: Guilherme G. Piccoli
Cc: Rahul Rameshbabu, djogorchock, linux-input, benjamin.tissoires,
kernel, kernel-dev
In-Reply-To: <59290abe-c780-5287-d27a-745f4f00ab8a@igalia.com>
On Mon, 18 Dec 2023, Guilherme G. Piccoli wrote:
> > My plan is to send this to Linus in a day or two, to have this fixed for
> > good in 6.7 final to be on the safe side.
> >
> > We can always remove the potentially superfluous check (thanks Rahul for
> > spotting that) later once we get the testing results.
> >
> > Thanks,
> >
>
> Thanks Jiri and Rahul! I agree with the approach, better to have it
> fixed ASAP indeed.
>
> I understand no action is necessary from my side now.
Not immediately, but if you are able to eventually remove that
likely-superfluous hunk with a Tested-by: tag, I'll happily merge that
patch.
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] HID: nintendo: Prevent divide-by-zero on code
From: Guilherme G. Piccoli @ 2023-12-19 0:03 UTC (permalink / raw)
To: Jiri Kosina
Cc: Rahul Rameshbabu, djogorchock, linux-input, benjamin.tissoires,
kernel, kernel-dev
In-Reply-To: <nycvar.YFH.7.76.2312190048380.24250@cbobk.fhfr.pm>
On 18/12/2023 20:49, Jiri Kosina wrote:
> [...]
>
> Not immediately, but if you are able to eventually remove that
> likely-superfluous hunk with a Tested-by: tag, I'll happily merge that
> patch.
>
> Thanks,
>
Of course, I'll do that and mention that it was a suggestion from Rahul!
Thanks,
Guilherme
^ permalink raw reply
* Re: [PATCH v2 1/4] Input: da9063 - Simplify obtaining OF match data
From: Dmitry Torokhov @ 2023-12-19 1:48 UTC (permalink / raw)
To: Biju Das
Cc: Support Opensource, linux-input, Geert Uytterhoeven,
Prabhakar Mahadev Lad, Biju Das, linux-renesas-soc
In-Reply-To: <20231213214803.9931-2-biju.das.jz@bp.renesas.com>
On Wed, Dec 13, 2023 at 09:48:00PM +0000, Biju Das wrote:
> Simplify probe() by replacing of_match_node() for retrieving match data by
> device_get_match_data().
>
> Some minor cleanups:
> * Remove the trailing comma in the terminator entry for the OF
> table making code robust against (theoretical) misrebases or other
> similar things where the new entry goes _after_ the termination without
> the compiler noticing.
> * Move OF table near to the user.
> * Arrange variables in reverse xmas tree order in probe().
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Applied, thank you.
--
Dmitry
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox