* [PATCH v11 0/3] Add support for vibrator in multiple PMICs
From: Fenglin Wu via B4 Relay @ 2024-04-16 2:44 UTC (permalink / raw)
To: kernel, Andy Gross, Bjorn Andersson, Konrad Dybcio,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Dmitry Baryshkov
Cc: linux-arm-msm, linux-input, linux-kernel, devicetree, Fenglin Wu,
Krzysztof Kozlowski
Add SW support for the vibrator module inside PMI632, PM7250B, PM7325B, PM7550BA.
It is very similar to the vibrator module inside PM8916 which is supported in
pm8xxx-vib driver but just the drive amplitude is controlled with 2 registers,
and the register base offset in each PMIC is different.
Changes in v11:
1. Drop the 1st patch since it has been applied
2. Update to address review comments
Link to v10: https://lore.kernel.org/r/20240412-pm8xxx-vibrator-new-design-v10-0-0ec0ad133866@quicinc.com
Changes in v10:
1. Add Fixes tag
2. Update SSBI vibrator to use DT 'reg' value
3. Add drv_in_step flag for programming vibrator level in steps
Link to v9: https://lore.kernel.org/r/20240411-pm8xxx-vibrator-new-design-v9-0-7bf56cb92b28@quicinc.com
Changes in v9:
1. Add a preceding change to correct VIB_MAX_LEVELS calculation
2. Address review comments from Konrad
Link to v8: https://lore.kernel.org/r/20240401-pm8xxx-vibrator-new-design-v8-0-6f2b8b03b4c7@quicinc.com
Changes in v8:
1. Remove hw_type, and still keep the register info in match data
2. Update to use register offset in pm8xxx_regs, and the base address
defined in DT for SPMI vibrator will be added in register access
3. Update voltage output range for SPMI vibrator which has 2 bytes drive
registers
Changes in v7:
1. Fix a typo: SSBL_VIB_DRV_REG --> SSBI_VIB_DRV_REG
2. Move the hw_type switch case in pm8xxx_vib_set() to the refactoring
change.
Changes in v6:
1. Add "qcom,pmi632-vib" as a standalone compatible string.
Changes in v5:
1. Drop "qcom,spmi-vib-gen2" generic compatible string as requested
and use device specific compatible strings only.
Changes in v4:
1. Update to use the combination of the HW type and register offset
as the constant match data, the register base address defined in
'reg' property will be added when accessing SPMI registers using
regmap APIs.
2. Remove 'qcom,spmi-vib-gen1' generic compatible string.
Changes in v3:
1. Refactor the driver to support different type of the vibrators with
better flexibility by introducing the HW type with corresponding
register fields definitions.
2. Add 'qcom,spmi-vib-gen1' and 'qcom,spmi-vib-gen2' compatible
strings, and add PMI632, PM7250B, PM7325B, PM7550BA as compatbile as
spmi-vib-gen2.
Changes in v2:
Remove the "pm7550ba-vib" compatible string as it's compatible with pm7325b.
Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
---
Fenglin Wu (3):
input: pm8xxx-vibrator: refactor to support new SPMI vibrator
dt-bindings: input: qcom,pm8xxx-vib: add new SPMI vibrator module
input: pm8xxx-vibrator: add new SPMI vibrator support
.../devicetree/bindings/input/qcom,pm8xxx-vib.yaml | 16 +++-
drivers/input/misc/pm8xxx-vibrator.c | 93 ++++++++++++++++------
2 files changed, 80 insertions(+), 29 deletions(-)
---
base-commit: 48c0687a322d54ac7e7a685c0b6db78d78f593af
change-id: 20240328-pm8xxx-vibrator-new-design-e5811ad59e8a
Best regards,
--
Fenglin Wu <quic_fenglinw@quicinc.com>
^ permalink raw reply
* [PATCH v11 1/3] input: pm8xxx-vibrator: refactor to support new SPMI vibrator
From: Fenglin Wu via B4 Relay @ 2024-04-16 2:44 UTC (permalink / raw)
To: kernel, Andy Gross, Bjorn Andersson, Konrad Dybcio,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Dmitry Baryshkov
Cc: linux-arm-msm, linux-input, linux-kernel, devicetree, Fenglin Wu
In-Reply-To: <20240416-pm8xxx-vibrator-new-design-v11-0-7b1c951e1515@quicinc.com>
From: Fenglin Wu <quic_fenglinw@quicinc.com>
Currently, vibrator control register addresses are hard coded,
including the base address and offsets, it's not flexible to
support new SPMI vibrator module which is usually included in
different PMICs with different base address. Refactor it by using
the base address defined in devicetree.
Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
---
drivers/input/misc/pm8xxx-vibrator.c | 41 ++++++++++++++++++++++--------------
1 file changed, 25 insertions(+), 16 deletions(-)
diff --git a/drivers/input/misc/pm8xxx-vibrator.c b/drivers/input/misc/pm8xxx-vibrator.c
index 79f478d3a9b3..2bcfa7ed3d6b 100644
--- a/drivers/input/misc/pm8xxx-vibrator.c
+++ b/drivers/input/misc/pm8xxx-vibrator.c
@@ -19,27 +19,27 @@
#define MAX_FF_SPEED 0xff
struct pm8xxx_regs {
- unsigned int enable_addr;
+ unsigned int enable_offset;
unsigned int enable_mask;
- unsigned int drv_addr;
+ unsigned int drv_offset;
unsigned int drv_mask;
unsigned int drv_shift;
unsigned int drv_en_manual_mask;
};
static const struct pm8xxx_regs pm8058_regs = {
- .drv_addr = 0x4A,
- .drv_mask = 0xf8,
+ .drv_offset = 0,
+ .drv_mask = GENMASK(7, 3),
.drv_shift = 3,
.drv_en_manual_mask = 0xfc,
};
static struct pm8xxx_regs pm8916_regs = {
- .enable_addr = 0xc046,
+ .enable_offset = 0x46,
.enable_mask = BIT(7),
- .drv_addr = 0xc041,
- .drv_mask = 0x1F,
+ .drv_offset = 0x41,
+ .drv_mask = GENMASK(4, 0),
.drv_shift = 0,
.drv_en_manual_mask = 0,
};
@@ -50,6 +50,8 @@ static struct pm8xxx_regs pm8916_regs = {
* @work: work structure to set the vibration parameters
* @regmap: regmap for register read/write
* @regs: registers' info
+ * @enable_addr: vibrator enable register
+ * @drv_addr: vibrator drive strength register
* @speed: speed of vibration set from userland
* @active: state of vibrator
* @level: level of vibration to set in the chip
@@ -60,6 +62,8 @@ struct pm8xxx_vib {
struct work_struct work;
struct regmap *regmap;
const struct pm8xxx_regs *regs;
+ unsigned int enable_addr;
+ unsigned int drv_addr;
int speed;
int level;
bool active;
@@ -82,15 +86,15 @@ static int pm8xxx_vib_set(struct pm8xxx_vib *vib, bool on)
else
val &= ~regs->drv_mask;
- rc = regmap_write(vib->regmap, regs->drv_addr, val);
+ rc = regmap_write(vib->regmap, vib->drv_addr, val);
if (rc < 0)
return rc;
vib->reg_vib_drv = val;
if (regs->enable_mask)
- rc = regmap_update_bits(vib->regmap, regs->enable_addr,
- regs->enable_mask, on ? ~0 : 0);
+ rc = regmap_update_bits(vib->regmap, vib->enable_addr,
+ regs->enable_mask, on ? regs->enable_mask : 0);
return rc;
}
@@ -102,11 +106,10 @@ static int pm8xxx_vib_set(struct pm8xxx_vib *vib, bool on)
static void pm8xxx_work_handler(struct work_struct *work)
{
struct pm8xxx_vib *vib = container_of(work, struct pm8xxx_vib, work);
- const struct pm8xxx_regs *regs = vib->regs;
- int rc;
unsigned int val;
+ int rc;
- rc = regmap_read(vib->regmap, regs->drv_addr, &val);
+ rc = regmap_read(vib->regmap, vib->drv_addr, &val);
if (rc < 0)
return;
@@ -169,7 +172,7 @@ static int pm8xxx_vib_probe(struct platform_device *pdev)
struct pm8xxx_vib *vib;
struct input_dev *input_dev;
int error;
- unsigned int val;
+ unsigned int val, reg_base = 0;
const struct pm8xxx_regs *regs;
vib = devm_kzalloc(&pdev->dev, sizeof(*vib), GFP_KERNEL);
@@ -187,15 +190,21 @@ static int pm8xxx_vib_probe(struct platform_device *pdev)
INIT_WORK(&vib->work, pm8xxx_work_handler);
vib->vib_input_dev = input_dev;
+ error = fwnode_property_read_u32(pdev->dev.fwnode, "reg", ®_base);
+ if (error < 0)
+ return dev_err_probe(&pdev->dev, error, "Failed to read reg address\n");
+
regs = of_device_get_match_data(&pdev->dev);
+ vib->enable_addr = reg_base + regs->enable_offset;
+ vib->drv_addr = reg_base + regs->drv_offset;
/* operate in manual mode */
- error = regmap_read(vib->regmap, regs->drv_addr, &val);
+ error = regmap_read(vib->regmap, vib->drv_addr, &val);
if (error < 0)
return error;
val &= regs->drv_en_manual_mask;
- error = regmap_write(vib->regmap, regs->drv_addr, val);
+ error = regmap_write(vib->regmap, vib->drv_addr, val);
if (error < 0)
return error;
--
2.25.1
^ permalink raw reply related
* [PATCH v11 2/3] dt-bindings: input: qcom,pm8xxx-vib: add new SPMI vibrator module
From: Fenglin Wu via B4 Relay @ 2024-04-16 2:44 UTC (permalink / raw)
To: kernel, Andy Gross, Bjorn Andersson, Konrad Dybcio,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Dmitry Baryshkov
Cc: linux-arm-msm, linux-input, linux-kernel, devicetree, Fenglin Wu,
Krzysztof Kozlowski
In-Reply-To: <20240416-pm8xxx-vibrator-new-design-v11-0-7b1c951e1515@quicinc.com>
From: Fenglin Wu <quic_fenglinw@quicinc.com>
Add compatible strings to support vibrator module inside PMI632,
PMI7250B, PM7325B, PM7550BA.
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
---
.../devicetree/bindings/input/qcom,pm8xxx-vib.yaml | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml b/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml
index c8832cd0d7da..2025d6a5423e 100644
--- a/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml
+++ b/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml
@@ -11,10 +11,18 @@ maintainers:
properties:
compatible:
- enum:
- - qcom,pm8058-vib
- - qcom,pm8916-vib
- - qcom,pm8921-vib
+ oneOf:
+ - enum:
+ - qcom,pm8058-vib
+ - qcom,pm8916-vib
+ - qcom,pm8921-vib
+ - qcom,pmi632-vib
+ - items:
+ - enum:
+ - qcom,pm7250b-vib
+ - qcom,pm7325b-vib
+ - qcom,pm7550ba-vib
+ - const: qcom,pmi632-vib
reg:
maxItems: 1
--
2.25.1
^ permalink raw reply related
* [PATCH v11 3/3] input: pm8xxx-vibrator: add new SPMI vibrator support
From: Fenglin Wu via B4 Relay @ 2024-04-16 2:44 UTC (permalink / raw)
To: kernel, Andy Gross, Bjorn Andersson, Konrad Dybcio,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Dmitry Baryshkov
Cc: linux-arm-msm, linux-input, linux-kernel, devicetree, Fenglin Wu
In-Reply-To: <20240416-pm8xxx-vibrator-new-design-v11-0-7b1c951e1515@quicinc.com>
From: Fenglin Wu <quic_fenglinw@quicinc.com>
Add support for a new SPMI vibrator module which is very similar
to the vibrator module inside PM8916 but has a finer drive voltage
step and different output voltage range, its drive level control
is expanded across 2 registers. The vibrator module can be found
in following Qualcomm PMICs: PMI632, PM7250B, PM7325B, PM7550BA.
Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
---
drivers/input/misc/pm8xxx-vibrator.c | 52 +++++++++++++++++++++++++++++-------
1 file changed, 43 insertions(+), 9 deletions(-)
diff --git a/drivers/input/misc/pm8xxx-vibrator.c b/drivers/input/misc/pm8xxx-vibrator.c
index 2bcfa7ed3d6b..381b06473279 100644
--- a/drivers/input/misc/pm8xxx-vibrator.c
+++ b/drivers/input/misc/pm8xxx-vibrator.c
@@ -11,10 +11,11 @@
#include <linux/regmap.h>
#include <linux/slab.h>
-#define VIB_MAX_LEVEL_mV (3100)
-#define VIB_MIN_LEVEL_mV (1200)
-#define VIB_PER_STEP_mV (100)
-#define VIB_MAX_LEVELS (VIB_MAX_LEVEL_mV - VIB_MIN_LEVEL_mV + VIB_PER_STEP_mV)
+#define VIB_MAX_LEVEL_mV(vib) (vib->drv2_addr ? 3544 : 3100)
+#define VIB_MIN_LEVEL_mV(vib) (vib->drv2_addr ? 1504 : 1200)
+#define VIB_PER_STEP_mV(vib) (vib->drv2_addr ? 8 : 100)
+#define VIB_MAX_LEVELS(vib) \
+ (VIB_MAX_LEVEL_mV(vib) - VIB_MIN_LEVEL_mV(vib) + VIB_PER_STEP_mV(vib))
#define MAX_FF_SPEED 0xff
@@ -25,7 +26,11 @@ struct pm8xxx_regs {
unsigned int drv_offset;
unsigned int drv_mask;
unsigned int drv_shift;
+ unsigned int drv2_offset;
+ unsigned int drv2_mask;
+ unsigned int drv2_shift;
unsigned int drv_en_manual_mask;
+ bool drv_in_step;
};
static const struct pm8xxx_regs pm8058_regs = {
@@ -33,6 +38,7 @@ static const struct pm8xxx_regs pm8058_regs = {
.drv_mask = GENMASK(7, 3),
.drv_shift = 3,
.drv_en_manual_mask = 0xfc,
+ .drv_in_step = true,
};
static struct pm8xxx_regs pm8916_regs = {
@@ -42,6 +48,20 @@ static struct pm8xxx_regs pm8916_regs = {
.drv_mask = GENMASK(4, 0),
.drv_shift = 0,
.drv_en_manual_mask = 0,
+ .drv_in_step = true,
+};
+
+static struct pm8xxx_regs pmi632_regs = {
+ .enable_offset = 0x46,
+ .enable_mask = BIT(7),
+ .drv_offset = 0x40,
+ .drv_mask = GENMASK(7, 0),
+ .drv_shift = 0,
+ .drv2_offset = 0x41,
+ .drv2_mask = GENMASK(3, 0),
+ .drv2_shift = 8,
+ .drv_en_manual_mask = 0,
+ .drv_in_step = false,
};
/**
@@ -52,6 +72,7 @@ static struct pm8xxx_regs pm8916_regs = {
* @regs: registers' info
* @enable_addr: vibrator enable register
* @drv_addr: vibrator drive strength register
+ * @drv2_addr: vibrator drive strength upper byte register
* @speed: speed of vibration set from userland
* @active: state of vibrator
* @level: level of vibration to set in the chip
@@ -64,6 +85,7 @@ struct pm8xxx_vib {
const struct pm8xxx_regs *regs;
unsigned int enable_addr;
unsigned int drv_addr;
+ unsigned int drv2_addr;
int speed;
int level;
bool active;
@@ -81,6 +103,9 @@ static int pm8xxx_vib_set(struct pm8xxx_vib *vib, bool on)
unsigned int val = vib->reg_vib_drv;
const struct pm8xxx_regs *regs = vib->regs;
+ if (regs->drv_in_step)
+ vib->level /= VIB_PER_STEP_mV(vib);
+
if (on)
val |= (vib->level << regs->drv_shift) & regs->drv_mask;
else
@@ -92,6 +117,14 @@ static int pm8xxx_vib_set(struct pm8xxx_vib *vib, bool on)
vib->reg_vib_drv = val;
+ if (regs->drv2_mask) {
+ val = vib->level << regs->drv2_shift;
+ rc = regmap_write_bits(vib->regmap, vib->drv2_addr,
+ regs->drv2_mask, on ? val : 0);
+ if (rc < 0)
+ return rc;
+ }
+
if (regs->enable_mask)
rc = regmap_update_bits(vib->regmap, vib->enable_addr,
regs->enable_mask, on ? regs->enable_mask : 0);
@@ -114,17 +147,16 @@ static void pm8xxx_work_handler(struct work_struct *work)
return;
/*
- * pmic vibrator supports voltage ranges from 1.2 to 3.1V, so
+ * pmic vibrator supports voltage ranges from MIN_LEVEL to MAX_LEVEL, so
* scale the level to fit into these ranges.
*/
if (vib->speed) {
vib->active = true;
- vib->level = ((VIB_MAX_LEVELS * vib->speed) / MAX_FF_SPEED) +
- VIB_MIN_LEVEL_mV;
- vib->level /= VIB_PER_STEP_mV;
+ vib->level = VIB_MIN_LEVEL_mV(vib);
+ vib->level += mult_frac(VIB_MAX_LEVELS(vib), vib->speed, MAX_FF_SPEED);
} else {
vib->active = false;
- vib->level = VIB_MIN_LEVEL_mV / VIB_PER_STEP_mV;
+ vib->level = VIB_MIN_LEVEL_mV(vib);
}
pm8xxx_vib_set(vib, vib->active);
@@ -197,6 +229,7 @@ static int pm8xxx_vib_probe(struct platform_device *pdev)
regs = of_device_get_match_data(&pdev->dev);
vib->enable_addr = reg_base + regs->enable_offset;
vib->drv_addr = reg_base + regs->drv_offset;
+ vib->drv2_addr = reg_base + regs->drv2_offset;
/* operate in manual mode */
error = regmap_read(vib->regmap, vib->drv_addr, &val);
@@ -251,6 +284,7 @@ static const struct of_device_id pm8xxx_vib_id_table[] = {
{ .compatible = "qcom,pm8058-vib", .data = &pm8058_regs },
{ .compatible = "qcom,pm8921-vib", .data = &pm8058_regs },
{ .compatible = "qcom,pm8916-vib", .data = &pm8916_regs },
+ { .compatible = "qcom,pmi632-vib", .data = &pmi632_regs },
{ }
};
MODULE_DEVICE_TABLE(of, pm8xxx_vib_id_table);
--
2.25.1
^ permalink raw reply related
* Re: [PATCH v3 1/4] dt-bindings: input: Add Himax HX83102J touchscreen
From: Rob Herring @ 2024-04-16 3:31 UTC (permalink / raw)
To: Allen_Lin
Cc: benjamin.tissoires, krzysztof.kozlowski+dt, jikos, linux-input,
conor, dmitry.torokhov, devicetree, linux-kernel
In-Reply-To: <TY0PR06MB56112682F500F8761F76DBA99E082@TY0PR06MB5611.apcprd06.prod.outlook.com>
On Tue, 16 Apr 2024 10:12:25 +0800, Allen_Lin wrote:
> Add the HX83102j touchscreen device tree bindings documents.
> HX83102j is a Himax TDDI touchscreen controller.
> It's power sequence should be bound with a lcm driver, thus it
> needs to be a panel follower. Others are the same as normal SPI
> touchscreen controller.
>
> Reviewed-by: Rob Herring <robh@kernel.org>
>
> Signed-off-by: Allen_Lin <allencl_lin@hotmail.com>
> ---
> .../input/touchscreen/himax,hx83102j.yaml | 93 +++++++++++++++++++
> MAINTAINERS | 6 ++
> 2 files changed, 99 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/input/touchscreen/himax,hx83102j.yaml
>
My bot found errors running 'make dt_binding_check' on your patch:
yamllint warnings/errors:
./Documentation/devicetree/bindings/input/touchscreen/himax,hx83102j.yaml:93:7: [error] no new line character at the end of file (new-line-at-end-of-file)
dtschema/dtc warnings/errors:
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/TY0PR06MB56112682F500F8761F76DBA99E082@TY0PR06MB5611.apcprd06.prod.outlook.com
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
^ permalink raw reply
* Re: [PATCH RESEND] HID: sony: Remove usage of the deprecated ida_simple_xx() API
From: Jiri Kosina @ 2024-04-16 7:51 UTC (permalink / raw)
To: Christophe JAILLET
Cc: akpm, Benjamin Tissoires, linux-kernel, kernel-janitors,
linux-input
In-Reply-To: <9b7684381f9d09a7cd5840caa2a160d7764d6403.1713088684.git.christophe.jaillet@wanadoo.fr>
On Sun, 14 Apr 2024, Christophe JAILLET wrote:
> ida_alloc() and ida_free() should be preferred to the deprecated
> ida_simple_get() and ida_simple_remove().
>
> This is less verbose.
>
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Applied to hid.git#for-6.10/sony.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH 15/18] HID: picoLCD: Constify lcd_ops
From: Bruno Prémont @ 2024-04-16 8:07 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Lee Jones, Daniel Thompson, Jingoo Han, Helge Deller, Jiri Kosina,
Benjamin Tissoires, Alexander Shiyan, Sascha Hauer,
Pengutronix Kernel Team, Shawn Guo, Fabio Estevam, dri-devel,
linux-fbdev, linux-kernel, linux-input, linux-arm-kernel, imx,
linux-omap
In-Reply-To: <20240414-video-backlight-lcd-ops-v1-15-9b37fcbf546a@kernel.org>
On Sun, 14 Apr 2024 18:36:13 +0200 Krzysztof Kozlowski wrote:
> 'struct lcd_ops' is not modified by core backlight code, so it can be
> made const for increased code safety.
>
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
Reviewed-by: Bruno Prémont <bonbons@linux-vserver.org>
> ---
>
> Depends on the first patch in the series.
> ---
> drivers/hid/hid-picolcd_lcd.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/hid/hid-picolcd_lcd.c b/drivers/hid/hid-picolcd_lcd.c
> index 0c4b76de8ae5..061a33ba7b1d 100644
> --- a/drivers/hid/hid-picolcd_lcd.c
> +++ b/drivers/hid/hid-picolcd_lcd.c
> @@ -46,7 +46,7 @@ static int picolcd_check_lcd_fb(struct lcd_device *ldev, struct fb_info *fb)
> return fb && fb == picolcd_fbinfo((struct picolcd_data *)lcd_get_data(ldev));
> }
>
> -static struct lcd_ops picolcd_lcdops = {
> +static const struct lcd_ops picolcd_lcdops = {
> .get_contrast = picolcd_get_contrast,
> .set_contrast = picolcd_set_contrast,
> .check_fb = picolcd_check_lcd_fb,
>
^ permalink raw reply
* Re: [PATCH 1/4] Input: Add trackpoint doubletap and system debug info keycodes
From: Hans de Goede @ 2024-04-16 8:33 UTC (permalink / raw)
To: Mark Pearson, Dmitry Torokhov
Cc: Peter Hutterer, Ilpo Järvinen, Henrique de Moraes Holschuh,
ibm-acpi-devel, platform-driver-x86@vger.kernel.org, linux-input,
linux-kernel, Nitin Joshi1, Vishnu Sankar
In-Reply-To: <7de52ec3-86f3-4a1d-ac87-a106ae1acb5d@app.fastmail.com>
Hi Mark,
On 4/16/24 1:57 AM, Mark Pearson wrote:
> Hi Dmitry,
>
> On Mon, Apr 15, 2024, at 6:54 PM, Dmitry Torokhov wrote:
>> On Mon, Apr 15, 2024 at 04:28:19PM -0400, Mark Pearson wrote:
>>> Hi
>>>
>>> On Mon, Apr 15, 2024, at 3:58 PM, Dmitry Torokhov wrote:
>>>> On Mon, Apr 15, 2024 at 09:50:37PM +0200, Hans de Goede wrote:
>>>>> Hi,
>>>>>
>>>>> On 4/15/24 9:40 PM, Dmitry Torokhov wrote:
>>>>>> On Wed, Apr 10, 2024 at 10:48:10PM -0400, Mark Pearson wrote:
>>>>>>>
>>>>>>> I have a stronger preference to keep the KEY_DOUBLECLICK - that one seems less controversial as a genuine new input event.
>>>>>>
>>>>>> Please see my response to Peter's letter. I think it very much depends
>>>>>> on how it will be used (associated with the pointer or standalone as it
>>>>>> is now).
>>>>>>
>>>>>> For standalone application, recalling your statement that on Win you
>>>>>> have this gesture invoke configuration utility I would argue for
>>>>>> KEY_CONFIG for it.
>>>>>
>>>>> KEY_CONFIG is already generated by Fn + F# on some ThinkPads to launch
>>>>> the GNOME/KDE control center/panel and I believe that at least GNOME
>>>>> comes with a default binding to map KEY_CONFIG to the control-center.
>>>>
>>>> Not KEY_CONTROLPANEL?
>>>>
>>>> Are there devices that use both Fn+# and the doubleclick? Would it be an
>>>> acceptable behavior for the users to have them behave the same?
>>>>
>>> Catching up with the thread, thanks for all the comments.
>>>
>>> For FN+N (originally KEY_DEBUG_SYS_INFO) the proposal was to now use
>>> KEY_VENDOR there. My conclusion was that this is targeting vendor
>>> specific functionality, and that was the closest fit, if a new keycode
>>> was not preferred.
>>
>> Fn+N -> KEY_VENDOR mapping sounds good to me.
>>
>>>
>>> For the doubletap (which is a unique input event - not related to the
>>> pointer) I would like to keep it as a new unique event.
>>>
>>> I think it's most likely use would be for control panel, but I don't
>>> think it should be limited to that. I can see it being useful if users
>>> are able to reconfigure it to launch something different (browser or
>>> music player maybe?), hence it would be best if it did not conflict
>>> with an existing keycode function. I also can't confirm it doesn't
>>> clash on existing or future systems - it's possible.
>>
>> So here is the problem. Keycodes in linux input are not mere
>> placeholders for something that will be decided later how it is to be
>> used, they are supposed to communicate intent and userspace ideally does
>> not need to have any additional knowledge about where the event is
>> coming from. A keyboard either internal or external sends KEY_SCREENLOCK
>> and the system should lock the screen. It should not be aware that one
>> device was a generic USB external keyboard while another had an internal
>> sensor that recognized hovering palm making swiping motion to the right
>> because a vendor decided to make it. Otherwise you have millions of
>> input devices all generating unique codes and you need userspace to
>> decide how to interpret data coming from each device individually.
>>
>> If you truly do not have a defined use case for it you have a couple
>> options:
>>
>> - assign it KEY_RESERVED, ensure your driver allows remapping to an
>> arbitrary keycode, let user or distro assign desired keycode to it
>>
>> - assign KEY_PROG1 .. KEY_PROG4 - pretty much the same - leave it in the
>> hand of the user to define a shortcut in their DE to make it useful
>>
>>>
>>> FWIW - I wouldn't be surprised with some of the new gaming systems
>>> we're seeing (Steamdeck, Legion-Go, etc), that a doubletap event on a
>>> joystick might be useful to have, if the HW supports it?
>>
>> What would it do exactly? Once we have this answer we can define key or
>> button code (although I do agree that game controller buttons are
>> different from "normal" keys because they map to the geometry of the
>> controller which in turn defines their commonly understood function).
>>
>> But in any case you would not reuse the same keycode for something that
>> is supposed to invoke a configuration utility and also to let's say
>> drop a flash grenade in a game.
>>
>
> Understood.
>
> I don't see a path forward within your stated parameters. I did not realise that there were such limitations, so my apologies for wasting everybody's time, and thank you for your patience and guidance.
>
> I will drop this patch from the series and proceed using existing defined codes only.
>
> Hans, I'll need to rejig things a bits but I have some ideas and I think I can make it work and stay within the pdx86 tree, which will make it simpler.
Ok this sounds good to me. For Fn + N using KEY_VENDOR sounds good for
me and for the doubletap any one of KEY_CONFIG/KEY_CONTROLPANEL/KEY_INFO/KEY_PROG1
or some other suitable KEY_foo define works for me.
Regards,
Hans
^ permalink raw reply
* Re: [PATCH 1/4] Input: Add trackpoint doubletap and system debug info keycodes
From: Hans de Goede @ 2024-04-16 8:35 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Mark Pearson, Peter Hutterer, Ilpo Järvinen,
Henrique de Moraes Holschuh, ibm-acpi-devel,
platform-driver-x86@vger.kernel.org, linux-input, linux-kernel,
Nitin Joshi1, Vishnu Sankar
In-Reply-To: <Zh2G85df29tPP6OK@google.com>
Hi,
On 4/15/24 9:58 PM, Dmitry Torokhov wrote:
> On Mon, Apr 15, 2024 at 09:50:37PM +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 4/15/24 9:40 PM, Dmitry Torokhov wrote:
>>> On Wed, Apr 10, 2024 at 10:48:10PM -0400, Mark Pearson wrote:
>>>>
>>>> I have a stronger preference to keep the KEY_DOUBLECLICK - that one seems less controversial as a genuine new input event.
>>>
>>> Please see my response to Peter's letter. I think it very much depends
>>> on how it will be used (associated with the pointer or standalone as it
>>> is now).
>>>
>>> For standalone application, recalling your statement that on Win you
>>> have this gesture invoke configuration utility I would argue for
>>> KEY_CONFIG for it.
>>
>> KEY_CONFIG is already generated by Fn + F# on some ThinkPads to launch
>> the GNOME/KDE control center/panel and I believe that at least GNOME
>> comes with a default binding to map KEY_CONFIG to the control-center.
>
> Not KEY_CONTROLPANEL?
No when this was added most distros were still using X11 not Wayland so
it was preferable to use KEY_foo symbols with codes below 240.
> Are there devices that use both Fn+# and the doubleclick? Would it be an
> acceptable behavior for the users to have them behave the same?
That is a good question, at least my current ThinkPad does not have
the Fn + F## combo for the control center anymore. So I guess we could
indeed re-use KEY_CONFIG for the double-tap.
Regards,
Hans
^ permalink raw reply
* [PATCH 0/4] HID: Asus
From: Luke D. Jones @ 2024-04-16 9:03 UTC (permalink / raw)
To: jikos; +Cc: benjamin.tissoires, linux-input, linux-kernel, Luke D. Jones
This is a small series of patches that does some cleanup and also adds some
initialisation support for the ROG Ally and the ROG X13 devices. The series is
a partial prep fro a *much* larger 2k line patch adding full feature support
for the ROG Ally - this patch isn't included as I don't want to hold up this
small series during review.
Luke D. Jones (4):
HID: asus: fix more n-key report descriptors if n-key quirked
HID: asus: make asus_kbd_init() generic, remove rog_nkey_led_init()
HID: asus: add ROG Ally N-Key ID and keycodes
HID: asus: add ROG Z13 lightbar
drivers/hid/hid-asus.c | 132 ++++++++++++++++-------------------------
drivers/hid/hid-ids.h | 2 +
2 files changed, 54 insertions(+), 80 deletions(-)
--
2.44.0
^ permalink raw reply
* [PATCH 1/4] HID: asus: fix more n-key report descriptors if n-key quirked
From: Luke D. Jones @ 2024-04-16 9:03 UTC (permalink / raw)
To: jikos; +Cc: benjamin.tissoires, linux-input, linux-kernel, Luke D. Jones
In-Reply-To: <20240416090402.31057-1-luke@ljones.dev>
Adjusts the report descriptor for N-Key devices to
make the output count 0x01 which completely avoids
the need for a block of filtering.
Signed-off-by: Luke D. Jones <luke@ljones.dev>
---
drivers/hid/hid-asus.c | 51 ++++++++++++++++++++----------------------
1 file changed, 24 insertions(+), 27 deletions(-)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 78cdfb8b9a7a..d6d8a028623a 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -335,36 +335,20 @@ static int asus_raw_event(struct hid_device *hdev,
if (drvdata->quirks & QUIRK_MEDION_E1239T)
return asus_e1239t_event(drvdata, data, size);
- if (drvdata->quirks & QUIRK_USE_KBD_BACKLIGHT) {
+ /*
+ * Skip these report ID, the device emits a continuous stream associated
+ * with the AURA mode it is in which looks like an 'echo'.
+ */
+ if (report->id == FEATURE_KBD_LED_REPORT_ID1 || report->id == FEATURE_KBD_LED_REPORT_ID2)
+ return -1;
+ if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD) {
/*
- * Skip these report ID, the device emits a continuous stream associated
- * with the AURA mode it is in which looks like an 'echo'.
+ * G713 and G733 send these codes on some keypresses, depending on
+ * the key pressed it can trigger a shutdown event if not caught.
*/
- if (report->id == FEATURE_KBD_LED_REPORT_ID1 ||
- report->id == FEATURE_KBD_LED_REPORT_ID2) {
+ if (data[0] == 0x02 && data[1] == 0x30) {
return -1;
- /* Additional report filtering */
- } else if (report->id == FEATURE_KBD_REPORT_ID) {
- /*
- * G14 and G15 send these codes on some keypresses with no
- * discernable reason for doing so. We'll filter them out to avoid
- * unmapped warning messages later.
- */
- if (data[1] == 0xea || data[1] == 0xec || data[1] == 0x02 ||
- data[1] == 0x8a || data[1] == 0x9e) {
- return -1;
- }
}
- if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD) {
- /*
- * G713 and G733 send these codes on some keypresses, depending on
- * the key pressed it can trigger a shutdown event if not caught.
- */
- if(data[0] == 0x02 && data[1] == 0x30) {
- return -1;
- }
- }
-
}
if (drvdata->quirks & QUIRK_ROG_CLAYMORE_II_KEYBOARD) {
@@ -1250,6 +1234,19 @@ static __u8 *asus_report_fixup(struct hid_device *hdev, __u8 *rdesc,
rdesc[205] = 0x01;
}
+ /* match many more n-key devices */
+ if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD) {
+ for (int i = 0; i < *rsize + 1; i++) {
+ /* offset to the count from 0x5a report part always 14 */
+ if (rdesc[i] == 0x85 && rdesc[i + 1] == 0x5a &&
+ rdesc[i + 14] == 0x95 && rdesc[i + 15] == 0x05) {
+ hid_info(hdev, "Fixing up Asus N-Key report descriptor\n");
+ rdesc[i + 15] = 0x01;
+ break;
+ }
+ }
+ }
+
return rdesc;
}
@@ -1319,4 +1316,4 @@ static struct hid_driver asus_driver = {
};
module_hid_driver(asus_driver);
-MODULE_LICENSE("GPL");
\ No newline at end of file
+MODULE_LICENSE("GPL");
--
2.44.0
^ permalink raw reply related
* [PATCH 2/4] HID: asus: make asus_kbd_init() generic, remove rog_nkey_led_init()
From: Luke D. Jones @ 2024-04-16 9:04 UTC (permalink / raw)
To: jikos; +Cc: benjamin.tissoires, linux-input, linux-kernel, Luke D. Jones
In-Reply-To: <20240416090402.31057-1-luke@ljones.dev>
Some of the n-key stuff is old and outdated, so
make asus_kbd_init() generic to use with other
report ID and remove rog_nkey_led_init().
Signed-off-by: Luke D. Jones <luke@ljones.dev>
---
drivers/hid/hid-asus.c | 70 +++++++++++-------------------------------
1 file changed, 18 insertions(+), 52 deletions(-)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index d6d8a028623a..18062f9804fb 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -386,9 +386,9 @@ static int asus_kbd_set_report(struct hid_device *hdev, const u8 *buf, size_t bu
return ret;
}
-static int asus_kbd_init(struct hid_device *hdev)
+static int asus_kbd_init(struct hid_device *hdev, u8 report_id)
{
- const u8 buf[] = { FEATURE_KBD_REPORT_ID, 0x41, 0x53, 0x55, 0x53, 0x20, 0x54,
+ const u8 buf[] = { report_id, 0x41, 0x53, 0x55, 0x53, 0x20, 0x54,
0x65, 0x63, 0x68, 0x2e, 0x49, 0x6e, 0x63, 0x2e, 0x00 };
int ret;
@@ -400,9 +400,10 @@ static int asus_kbd_init(struct hid_device *hdev)
}
static int asus_kbd_get_functions(struct hid_device *hdev,
- unsigned char *kbd_func)
+ unsigned char *kbd_func,
+ u8 report_id)
{
- const u8 buf[] = { FEATURE_KBD_REPORT_ID, 0x05, 0x20, 0x31, 0x00, 0x08 };
+ const u8 buf[] = { report_id, 0x05, 0x20, 0x31, 0x00, 0x08 };
u8 *readbuf;
int ret;
@@ -431,51 +432,6 @@ static int asus_kbd_get_functions(struct hid_device *hdev,
return ret;
}
-static int rog_nkey_led_init(struct hid_device *hdev)
-{
- const u8 buf_init_start[] = { FEATURE_KBD_LED_REPORT_ID1, 0xB9 };
- u8 buf_init2[] = { FEATURE_KBD_LED_REPORT_ID1, 0x41, 0x53, 0x55, 0x53, 0x20,
- 0x54, 0x65, 0x63, 0x68, 0x2e, 0x49, 0x6e, 0x63, 0x2e, 0x00 };
- u8 buf_init3[] = { FEATURE_KBD_LED_REPORT_ID1,
- 0x05, 0x20, 0x31, 0x00, 0x08 };
- int ret;
-
- hid_info(hdev, "Asus initialise N-KEY Device");
- /* The first message is an init start */
- ret = asus_kbd_set_report(hdev, buf_init_start, sizeof(buf_init_start));
- if (ret < 0) {
- hid_warn(hdev, "Asus failed to send init start command: %d\n", ret);
- return ret;
- }
- /* Followed by a string */
- ret = asus_kbd_set_report(hdev, buf_init2, sizeof(buf_init2));
- if (ret < 0) {
- hid_warn(hdev, "Asus failed to send init command 1.0: %d\n", ret);
- return ret;
- }
- /* Followed by a string */
- ret = asus_kbd_set_report(hdev, buf_init3, sizeof(buf_init3));
- if (ret < 0) {
- hid_warn(hdev, "Asus failed to send init command 1.1: %d\n", ret);
- return ret;
- }
-
- /* begin second report ID with same data */
- buf_init2[0] = FEATURE_KBD_LED_REPORT_ID2;
- buf_init3[0] = FEATURE_KBD_LED_REPORT_ID2;
-
- ret = asus_kbd_set_report(hdev, buf_init2, sizeof(buf_init2));
- if (ret < 0) {
- hid_warn(hdev, "Asus failed to send init command 2.0: %d\n", ret);
- return ret;
- }
- ret = asus_kbd_set_report(hdev, buf_init3, sizeof(buf_init3));
- if (ret < 0)
- hid_warn(hdev, "Asus failed to send init command 2.1: %d\n", ret);
-
- return ret;
-}
-
static void asus_schedule_work(struct asus_kbd_leds *led)
{
unsigned long flags;
@@ -558,17 +514,27 @@ static int asus_kbd_register_leds(struct hid_device *hdev)
int ret;
if (drvdata->quirks & QUIRK_ROG_NKEY_KEYBOARD) {
- ret = rog_nkey_led_init(hdev);
+ /* Initialize keyboard */
+ ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
+ if (ret < 0)
+ return ret;
+
+ /* The LED endpoint is initialised in two HID */
+ ret = asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID1);
+ if (ret < 0)
+ return ret;
+
+ ret = asus_kbd_init(hdev, FEATURE_KBD_LED_REPORT_ID2);
if (ret < 0)
return ret;
} else {
/* Initialize keyboard */
- ret = asus_kbd_init(hdev);
+ ret = asus_kbd_init(hdev, FEATURE_KBD_REPORT_ID);
if (ret < 0)
return ret;
/* Get keyboard functions */
- ret = asus_kbd_get_functions(hdev, &kbd_func);
+ ret = asus_kbd_get_functions(hdev, &kbd_func, FEATURE_KBD_REPORT_ID);
if (ret < 0)
return ret;
--
2.44.0
^ permalink raw reply related
* [PATCH 3/4] HID: asus: add ROG Ally N-Key ID and keycodes
From: Luke D. Jones @ 2024-04-16 9:04 UTC (permalink / raw)
To: jikos; +Cc: benjamin.tissoires, linux-input, linux-kernel, Luke D. Jones
In-Reply-To: <20240416090402.31057-1-luke@ljones.dev>
A handful of buttons on the ROG Ally are not actually part of the xpad
device and are instead keyboard keys (a typical use of the MCU that asus
uses). We attach a group of F<num> key codes which aren't used much and
which the handheld community has already accepted as defaults here.
Signed-off-by: Luke D. Jones <luke@ljones.dev>
---
drivers/hid/hid-asus.c | 8 +++++++-
drivers/hid/hid-ids.h | 1 +
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 18062f9804fb..61820f7a6b25 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -847,7 +847,10 @@ static int asus_input_mapping(struct hid_device *hdev,
case 0xb3: asus_map_key_clear(KEY_PROG3); break; /* Fn+Left next aura */
case 0x6a: asus_map_key_clear(KEY_F13); break; /* Screenpad toggle */
case 0x4b: asus_map_key_clear(KEY_F14); break; /* Arrows/Pg-Up/Dn toggle */
-
+ case 0xa5: asus_map_key_clear(KEY_F15); break; /* ROG Ally left back */
+ case 0xa6: asus_map_key_clear(KEY_F16); break; /* ROG Ally QAM button */
+ case 0xa7: asus_map_key_clear(KEY_F17); break; /* ROG Ally ROG long-press */
+ case 0xa8: asus_map_key_clear(KEY_F18); break; /* ROG Ally ROG long-press-release */
default:
/* ASUS lazily declares 256 usages, ignore the rest,
@@ -1239,6 +1242,9 @@ static const struct hid_device_id asus_devices[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD3),
QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
+ { HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
+ USB_DEVICE_ID_ASUSTEK_ROG_NKEY_ALLY),
+ QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
USB_DEVICE_ID_ASUSTEK_ROG_CLAYMORE_II_KEYBOARD),
QUIRK_ROG_CLAYMORE_II_KEYBOARD },
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 8376fb5e2d0b..f1e508a7ef06 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -208,6 +208,7 @@
#define USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD 0x1866
#define USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD2 0x19b6
#define USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD3 0x1a30
+#define USB_DEVICE_ID_ASUSTEK_ROG_NKEY_ALLY 0x1abe
#define USB_DEVICE_ID_ASUSTEK_ROG_CLAYMORE_II_KEYBOARD 0x196b
#define USB_DEVICE_ID_ASUSTEK_FX503VD_KEYBOARD 0x1869
--
2.44.0
^ permalink raw reply related
* [PATCH 4/4] HID: asus: add ROG Z13 lightbar
From: Luke D. Jones @ 2024-04-16 9:04 UTC (permalink / raw)
To: jikos; +Cc: benjamin.tissoires, linux-input, linux-kernel, Luke D. Jones
In-Reply-To: <20240416090402.31057-1-luke@ljones.dev>
Add init of the lightbar which is a small panel on the back of the ASUS
ROG Z13 and uses the same MCU as keyboards.
Signed-off-by: Luke D. Jones <luke@ljones.dev>
---
drivers/hid/hid-asus.c | 3 +++
drivers/hid/hid-ids.h | 1 +
2 files changed, 4 insertions(+)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 61820f7a6b25..02de2bf4f790 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -1242,6 +1242,9 @@ static const struct hid_device_id asus_devices[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD3),
QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
+ { HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
+ USB_DEVICE_ID_ASUSTEK_ROG_Z13_LIGHTBAR),
+ QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
USB_DEVICE_ID_ASUSTEK_ROG_NKEY_ALLY),
QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index f1e508a7ef06..94501dbdd463 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -208,6 +208,7 @@
#define USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD 0x1866
#define USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD2 0x19b6
#define USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD3 0x1a30
+#define USB_DEVICE_ID_ASUSTEK_ROG_Z13_LIGHTBAR 0x18c6
#define USB_DEVICE_ID_ASUSTEK_ROG_NKEY_ALLY 0x1abe
#define USB_DEVICE_ID_ASUSTEK_ROG_CLAYMORE_II_KEYBOARD 0x196b
#define USB_DEVICE_ID_ASUSTEK_FX503VD_KEYBOARD 0x1869
--
2.44.0
^ permalink raw reply related
* Re: [PATCH 00/19] amba: store owner from modules with amba_driver_register()
From: Suzuki K Poulose @ 2024-04-16 10:41 UTC (permalink / raw)
To: Krzysztof Kozlowski, Russell King, Mike Leach, James Clark,
Alexander Shishkin, Maxime Coquelin, Alexandre Torgue,
Linus Walleij, Andi Shyti, Olivia Mackall, Herbert Xu, Vinod Koul,
Dmitry Torokhov, Miquel Raynal, Michal Simek, Eric Auger,
Alex Williamson, Greg Kroah-Hartman
Cc: linux-kernel, coresight, linux-arm-kernel, linux-stm32, linux-i2c,
linux-crypto, dmaengine, linux-input, kvm
In-Reply-To: <cfa5aa01-44ef-4eb1-9ca6-541ed5908db4@linaro.org>
+ Greg
Hi Krzysztof,
On 30/03/2024 18:00, Krzysztof Kozlowski wrote:
> On 30/03/2024 18:58, Krzysztof Kozlowski wrote:
>>
>> On Tue, 26 Mar 2024 21:23:30 +0100, Krzysztof Kozlowski wrote:
>>> Merging
>>> =======
>>> All further patches depend on the first amba patch, therefore please ack
>>> and this should go via one tree.
>>>
>>> Description
>>> ===========
>>> Modules registering driver with amba_driver_register() often forget to
>>> set .owner field.
>>>
>>> [...]
>>
>> Applied, thanks!
>>
>> [01/19] amba: store owner from modules with amba_driver_register()
>> (no commit info)
>
> Patchset applied here:
> https://git.kernel.org/pub/scm/linux/kernel/git/krzk/linux-dt.git/log/?h=for-v6.10/module-owner-amba
How do you plan to push this ? Given this affects most of the drivers/,
do you plan to send this to Greg ? We have changes in the coresight
tree that would conflict with this "tag" ( I haven't merged them yet,
but is in my local queue). I want to make sure we can avoid the
conflicts. I am happy to merge this to my local tree and base the
changes on this, if this is going in for v6.10 and all are in agreement.
Kind regards
Suzuki
>
> Best regards,
> Krzysztof
>
^ permalink raw reply
* Re: [PATCH 00/18] backlight: Constify lcd_ops
From: Lee Jones @ 2024-04-16 12:17 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Thomas Zimmermann, Daniel Thompson, Jingoo Han, Helge Deller,
Bruno Prémont, Jiri Kosina, Benjamin Tissoires,
Alexander Shiyan, Sascha Hauer, Pengutronix Kernel Team,
Shawn Guo, Fabio Estevam, dri-devel, linux-fbdev, linux-kernel,
linux-input, linux-arm-kernel, imx, linux-omap
In-Reply-To: <65adee30-fd12-4cc7-a227-9a586bb9e6d5@kernel.org>
On Mon, 15 Apr 2024, Krzysztof Kozlowski wrote:
> On 15/04/2024 08:56, Thomas Zimmermann wrote:
> > Hi
> >
> > for patches 16, 17 and 18:
> >
> > Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
>
> This does not work like this. Toolset will apply your review tag for
> EVERYTHING. You must provide tag under each individual patch.
And for that to not happen now, you have to resubmit the set. :)
--
Lee Jones [李琼斯]
^ permalink raw reply
* Re: [PATCH v11 0/3] Add support for vibrator in multiple PMICs
From: Dmitry Baryshkov @ 2024-04-16 12:35 UTC (permalink / raw)
To: quic_fenglinw
Cc: kernel, Andy Gross, Bjorn Andersson, Konrad Dybcio,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, linux-arm-msm,
linux-input, linux-kernel, devicetree, Krzysztof Kozlowski
In-Reply-To: <20240416-pm8xxx-vibrator-new-design-v11-0-7b1c951e1515@quicinc.com>
On Tue, 16 Apr 2024 at 05:44, Fenglin Wu via B4 Relay
<devnull+quic_fenglinw.quicinc.com@kernel.org> wrote:
>
> Add SW support for the vibrator module inside PMI632, PM7250B, PM7325B, PM7550BA.
> It is very similar to the vibrator module inside PM8916 which is supported in
> pm8xxx-vib driver but just the drive amplitude is controlled with 2 registers,
> and the register base offset in each PMIC is different.
>
> Changes in v11:
> 1. Drop the 1st patch since it has been applied
> 2. Update to address review comments
Please abstain from such changelog entries. Which comments were
addressed? How were they addressed?
> Link to v10: https://lore.kernel.org/r/20240412-pm8xxx-vibrator-new-design-v10-0-0ec0ad133866@quicinc.com
>
> Changes in v10:
> 1. Add Fixes tag
> 2. Update SSBI vibrator to use DT 'reg' value
> 3. Add drv_in_step flag for programming vibrator level in steps
> Link to v9: https://lore.kernel.org/r/20240411-pm8xxx-vibrator-new-design-v9-0-7bf56cb92b28@quicinc.com
>
> Changes in v9:
> 1. Add a preceding change to correct VIB_MAX_LEVELS calculation
> 2. Address review comments from Konrad
> Link to v8: https://lore.kernel.org/r/20240401-pm8xxx-vibrator-new-design-v8-0-6f2b8b03b4c7@quicinc.com
>
> Changes in v8:
> 1. Remove hw_type, and still keep the register info in match data
> 2. Update to use register offset in pm8xxx_regs, and the base address
> defined in DT for SPMI vibrator will be added in register access
> 3. Update voltage output range for SPMI vibrator which has 2 bytes drive
> registers
>
> Changes in v7:
> 1. Fix a typo: SSBL_VIB_DRV_REG --> SSBI_VIB_DRV_REG
> 2. Move the hw_type switch case in pm8xxx_vib_set() to the refactoring
> change.
>
> Changes in v6:
> 1. Add "qcom,pmi632-vib" as a standalone compatible string.
>
> Changes in v5:
> 1. Drop "qcom,spmi-vib-gen2" generic compatible string as requested
> and use device specific compatible strings only.
>
> Changes in v4:
> 1. Update to use the combination of the HW type and register offset
> as the constant match data, the register base address defined in
> 'reg' property will be added when accessing SPMI registers using
> regmap APIs.
> 2. Remove 'qcom,spmi-vib-gen1' generic compatible string.
>
> Changes in v3:
> 1. Refactor the driver to support different type of the vibrators with
> better flexibility by introducing the HW type with corresponding
> register fields definitions.
> 2. Add 'qcom,spmi-vib-gen1' and 'qcom,spmi-vib-gen2' compatible
> strings, and add PMI632, PM7250B, PM7325B, PM7550BA as compatbile as
> spmi-vib-gen2.
>
> Changes in v2:
> Remove the "pm7550ba-vib" compatible string as it's compatible with pm7325b.
>
> Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
> ---
> Fenglin Wu (3):
> input: pm8xxx-vibrator: refactor to support new SPMI vibrator
> dt-bindings: input: qcom,pm8xxx-vib: add new SPMI vibrator module
> input: pm8xxx-vibrator: add new SPMI vibrator support
>
> .../devicetree/bindings/input/qcom,pm8xxx-vib.yaml | 16 +++-
> drivers/input/misc/pm8xxx-vibrator.c | 93 ++++++++++++++++------
> 2 files changed, 80 insertions(+), 29 deletions(-)
> ---
> base-commit: 48c0687a322d54ac7e7a685c0b6db78d78f593af
> change-id: 20240328-pm8xxx-vibrator-new-design-e5811ad59e8a
>
> Best regards,
> --
> Fenglin Wu <quic_fenglinw@quicinc.com>
>
>
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [PATCH v11 1/3] input: pm8xxx-vibrator: refactor to support new SPMI vibrator
From: Dmitry Baryshkov @ 2024-04-16 12:37 UTC (permalink / raw)
To: quic_fenglinw
Cc: kernel, Andy Gross, Bjorn Andersson, Konrad Dybcio,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, linux-arm-msm,
linux-input, linux-kernel, devicetree
In-Reply-To: <20240416-pm8xxx-vibrator-new-design-v11-1-7b1c951e1515@quicinc.com>
On Tue, 16 Apr 2024 at 05:44, Fenglin Wu via B4 Relay
<devnull+quic_fenglinw.quicinc.com@kernel.org> wrote:
>
> From: Fenglin Wu <quic_fenglinw@quicinc.com>
>
> Currently, vibrator control register addresses are hard coded,
> including the base address and offsets, it's not flexible to
> support new SPMI vibrator module which is usually included in
> different PMICs with different base address. Refactor it by using
> the base address defined in devicetree.
>
> Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
> ---
> drivers/input/misc/pm8xxx-vibrator.c | 41 ++++++++++++++++++++++--------------
> 1 file changed, 25 insertions(+), 16 deletions(-)
>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [PATCH v11 3/3] input: pm8xxx-vibrator: add new SPMI vibrator support
From: Dmitry Baryshkov @ 2024-04-16 12:39 UTC (permalink / raw)
To: quic_fenglinw
Cc: kernel, Andy Gross, Bjorn Andersson, Konrad Dybcio,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, linux-arm-msm,
linux-input, linux-kernel, devicetree
In-Reply-To: <20240416-pm8xxx-vibrator-new-design-v11-3-7b1c951e1515@quicinc.com>
On Tue, 16 Apr 2024 at 05:44, Fenglin Wu via B4 Relay
<devnull+quic_fenglinw.quicinc.com@kernel.org> wrote:
>
> From: Fenglin Wu <quic_fenglinw@quicinc.com>
>
> Add support for a new SPMI vibrator module which is very similar
> to the vibrator module inside PM8916 but has a finer drive voltage
> step and different output voltage range, its drive level control
> is expanded across 2 registers. The vibrator module can be found
> in following Qualcomm PMICs: PMI632, PM7250B, PM7325B, PM7550BA.
>
> Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
> ---
> drivers/input/misc/pm8xxx-vibrator.c | 52 +++++++++++++++++++++++++++++-------
> 1 file changed, 43 insertions(+), 9 deletions(-)
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [PATCH 1/4] Input: Add trackpoint doubletap and system debug info keycodes
From: Mark Pearson @ 2024-04-16 12:48 UTC (permalink / raw)
To: Hans de Goede, Dmitry Torokhov
Cc: Peter Hutterer, Ilpo Järvinen, Henrique de Moraes Holschuh,
ibm-acpi-devel, platform-driver-x86@vger.kernel.org, linux-input,
linux-kernel, Nitin Joshi1, Vishnu Sankar
In-Reply-To: <27b1b6cf-759c-4778-a53c-5d01442120b7@redhat.com>
Hi Hans
On Tue, Apr 16, 2024, at 4:33 AM, Hans de Goede wrote:
> Hi Mark,
>
> On 4/16/24 1:57 AM, Mark Pearson wrote:
>> Hi Dmitry,
>>
>> On Mon, Apr 15, 2024, at 6:54 PM, Dmitry Torokhov wrote:
>>> On Mon, Apr 15, 2024 at 04:28:19PM -0400, Mark Pearson wrote:
>>>> Hi
>>>>
>>>> On Mon, Apr 15, 2024, at 3:58 PM, Dmitry Torokhov wrote:
>>>>> On Mon, Apr 15, 2024 at 09:50:37PM +0200, Hans de Goede wrote:
>>>>>> Hi,
>>>>>>
>>>>>> On 4/15/24 9:40 PM, Dmitry Torokhov wrote:
>>>>>>> On Wed, Apr 10, 2024 at 10:48:10PM -0400, Mark Pearson wrote:
>>>>>>>>
>>>>>>>> I have a stronger preference to keep the KEY_DOUBLECLICK - that one seems less controversial as a genuine new input event.
>>>>>>>
>>>>>>> Please see my response to Peter's letter. I think it very much depends
>>>>>>> on how it will be used (associated with the pointer or standalone as it
>>>>>>> is now).
>>>>>>>
>>>>>>> For standalone application, recalling your statement that on Win you
>>>>>>> have this gesture invoke configuration utility I would argue for
>>>>>>> KEY_CONFIG for it.
>>>>>>
>>>>>> KEY_CONFIG is already generated by Fn + F# on some ThinkPads to launch
>>>>>> the GNOME/KDE control center/panel and I believe that at least GNOME
>>>>>> comes with a default binding to map KEY_CONFIG to the control-center.
>>>>>
>>>>> Not KEY_CONTROLPANEL?
>>>>>
>>>>> Are there devices that use both Fn+# and the doubleclick? Would it be an
>>>>> acceptable behavior for the users to have them behave the same?
>>>>>
>>>> Catching up with the thread, thanks for all the comments.
>>>>
>>>> For FN+N (originally KEY_DEBUG_SYS_INFO) the proposal was to now use
>>>> KEY_VENDOR there. My conclusion was that this is targeting vendor
>>>> specific functionality, and that was the closest fit, if a new keycode
>>>> was not preferred.
>>>
>>> Fn+N -> KEY_VENDOR mapping sounds good to me.
>>>
>>>>
>>>> For the doubletap (which is a unique input event - not related to the
>>>> pointer) I would like to keep it as a new unique event.
>>>>
>>>> I think it's most likely use would be for control panel, but I don't
>>>> think it should be limited to that. I can see it being useful if users
>>>> are able to reconfigure it to launch something different (browser or
>>>> music player maybe?), hence it would be best if it did not conflict
>>>> with an existing keycode function. I also can't confirm it doesn't
>>>> clash on existing or future systems - it's possible.
>>>
>>> So here is the problem. Keycodes in linux input are not mere
>>> placeholders for something that will be decided later how it is to be
>>> used, they are supposed to communicate intent and userspace ideally does
>>> not need to have any additional knowledge about where the event is
>>> coming from. A keyboard either internal or external sends KEY_SCREENLOCK
>>> and the system should lock the screen. It should not be aware that one
>>> device was a generic USB external keyboard while another had an internal
>>> sensor that recognized hovering palm making swiping motion to the right
>>> because a vendor decided to make it. Otherwise you have millions of
>>> input devices all generating unique codes and you need userspace to
>>> decide how to interpret data coming from each device individually.
>>>
>>> If you truly do not have a defined use case for it you have a couple
>>> options:
>>>
>>> - assign it KEY_RESERVED, ensure your driver allows remapping to an
>>> arbitrary keycode, let user or distro assign desired keycode to it
>>>
>>> - assign KEY_PROG1 .. KEY_PROG4 - pretty much the same - leave it in the
>>> hand of the user to define a shortcut in their DE to make it useful
>>>
>>>>
>>>> FWIW - I wouldn't be surprised with some of the new gaming systems
>>>> we're seeing (Steamdeck, Legion-Go, etc), that a doubletap event on a
>>>> joystick might be useful to have, if the HW supports it?
>>>
>>> What would it do exactly? Once we have this answer we can define key or
>>> button code (although I do agree that game controller buttons are
>>> different from "normal" keys because they map to the geometry of the
>>> controller which in turn defines their commonly understood function).
>>>
>>> But in any case you would not reuse the same keycode for something that
>>> is supposed to invoke a configuration utility and also to let's say
>>> drop a flash grenade in a game.
>>>
>>
>> Understood.
>>
>> I don't see a path forward within your stated parameters. I did not realise that there were such limitations, so my apologies for wasting everybody's time, and thank you for your patience and guidance.
>>
>> I will drop this patch from the series and proceed using existing defined codes only.
>>
>> Hans, I'll need to rejig things a bits but I have some ideas and I think I can make it work and stay within the pdx86 tree, which will make it simpler.
>
> Ok this sounds good to me. For Fn + N using KEY_VENDOR sounds good for
> me and for the doubletap any one of
> KEY_CONFIG/KEY_CONTROLPANEL/KEY_INFO/KEY_PROG1
> or some other suitable KEY_foo define works for me.
>
I think this should be a configurable input, by design. So my preference (if not allowed a new keycode, which I personally think is the better option) is for PROG1.
I discussed with Peter last night and it looks likely OK on their side. I do plan on doing some testing first, so it might take a few days to get the next set of patches out.
Mark
^ permalink raw reply
* Re: [PATCH 1/4] Input: Add trackpoint doubletap and system debug info keycodes
From: Hans de Goede @ 2024-04-16 13:03 UTC (permalink / raw)
To: Mark Pearson, Dmitry Torokhov
Cc: Peter Hutterer, Ilpo Järvinen, Henrique de Moraes Holschuh,
ibm-acpi-devel, platform-driver-x86@vger.kernel.org, linux-input,
linux-kernel, Nitin Joshi1, Vishnu Sankar
In-Reply-To: <ce28b0e5-a867-458e-bcb4-cc327be5f19e@app.fastmail.com>
Hi,
On 4/16/24 2:48 PM, Mark Pearson wrote:
> Hi Hans
>
> On Tue, Apr 16, 2024, at 4:33 AM, Hans de Goede wrote:
>> Hi Mark,
>>
>> On 4/16/24 1:57 AM, Mark Pearson wrote:
>>> Hi Dmitry,
>>>
>>> On Mon, Apr 15, 2024, at 6:54 PM, Dmitry Torokhov wrote:
>>>> On Mon, Apr 15, 2024 at 04:28:19PM -0400, Mark Pearson wrote:
>>>>> Hi
>>>>>
>>>>> On Mon, Apr 15, 2024, at 3:58 PM, Dmitry Torokhov wrote:
>>>>>> On Mon, Apr 15, 2024 at 09:50:37PM +0200, Hans de Goede wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> On 4/15/24 9:40 PM, Dmitry Torokhov wrote:
>>>>>>>> On Wed, Apr 10, 2024 at 10:48:10PM -0400, Mark Pearson wrote:
>>>>>>>>>
>>>>>>>>> I have a stronger preference to keep the KEY_DOUBLECLICK - that one seems less controversial as a genuine new input event.
>>>>>>>>
>>>>>>>> Please see my response to Peter's letter. I think it very much depends
>>>>>>>> on how it will be used (associated with the pointer or standalone as it
>>>>>>>> is now).
>>>>>>>>
>>>>>>>> For standalone application, recalling your statement that on Win you
>>>>>>>> have this gesture invoke configuration utility I would argue for
>>>>>>>> KEY_CONFIG for it.
>>>>>>>
>>>>>>> KEY_CONFIG is already generated by Fn + F# on some ThinkPads to launch
>>>>>>> the GNOME/KDE control center/panel and I believe that at least GNOME
>>>>>>> comes with a default binding to map KEY_CONFIG to the control-center.
>>>>>>
>>>>>> Not KEY_CONTROLPANEL?
>>>>>>
>>>>>> Are there devices that use both Fn+# and the doubleclick? Would it be an
>>>>>> acceptable behavior for the users to have them behave the same?
>>>>>>
>>>>> Catching up with the thread, thanks for all the comments.
>>>>>
>>>>> For FN+N (originally KEY_DEBUG_SYS_INFO) the proposal was to now use
>>>>> KEY_VENDOR there. My conclusion was that this is targeting vendor
>>>>> specific functionality, and that was the closest fit, if a new keycode
>>>>> was not preferred.
>>>>
>>>> Fn+N -> KEY_VENDOR mapping sounds good to me.
>>>>
>>>>>
>>>>> For the doubletap (which is a unique input event - not related to the
>>>>> pointer) I would like to keep it as a new unique event.
>>>>>
>>>>> I think it's most likely use would be for control panel, but I don't
>>>>> think it should be limited to that. I can see it being useful if users
>>>>> are able to reconfigure it to launch something different (browser or
>>>>> music player maybe?), hence it would be best if it did not conflict
>>>>> with an existing keycode function. I also can't confirm it doesn't
>>>>> clash on existing or future systems - it's possible.
>>>>
>>>> So here is the problem. Keycodes in linux input are not mere
>>>> placeholders for something that will be decided later how it is to be
>>>> used, they are supposed to communicate intent and userspace ideally does
>>>> not need to have any additional knowledge about where the event is
>>>> coming from. A keyboard either internal or external sends KEY_SCREENLOCK
>>>> and the system should lock the screen. It should not be aware that one
>>>> device was a generic USB external keyboard while another had an internal
>>>> sensor that recognized hovering palm making swiping motion to the right
>>>> because a vendor decided to make it. Otherwise you have millions of
>>>> input devices all generating unique codes and you need userspace to
>>>> decide how to interpret data coming from each device individually.
>>>>
>>>> If you truly do not have a defined use case for it you have a couple
>>>> options:
>>>>
>>>> - assign it KEY_RESERVED, ensure your driver allows remapping to an
>>>> arbitrary keycode, let user or distro assign desired keycode to it
>>>>
>>>> - assign KEY_PROG1 .. KEY_PROG4 - pretty much the same - leave it in the
>>>> hand of the user to define a shortcut in their DE to make it useful
>>>>
>>>>>
>>>>> FWIW - I wouldn't be surprised with some of the new gaming systems
>>>>> we're seeing (Steamdeck, Legion-Go, etc), that a doubletap event on a
>>>>> joystick might be useful to have, if the HW supports it?
>>>>
>>>> What would it do exactly? Once we have this answer we can define key or
>>>> button code (although I do agree that game controller buttons are
>>>> different from "normal" keys because they map to the geometry of the
>>>> controller which in turn defines their commonly understood function).
>>>>
>>>> But in any case you would not reuse the same keycode for something that
>>>> is supposed to invoke a configuration utility and also to let's say
>>>> drop a flash grenade in a game.
>>>>
>>>
>>> Understood.
>>>
>>> I don't see a path forward within your stated parameters. I did not realise that there were such limitations, so my apologies for wasting everybody's time, and thank you for your patience and guidance.
>>>
>>> I will drop this patch from the series and proceed using existing defined codes only.
>>>
>>> Hans, I'll need to rejig things a bits but I have some ideas and I think I can make it work and stay within the pdx86 tree, which will make it simpler.
>>
>> Ok this sounds good to me. For Fn + N using KEY_VENDOR sounds good for
>> me and for the doubletap any one of
>> KEY_CONFIG/KEY_CONTROLPANEL/KEY_INFO/KEY_PROG1
>> or some other suitable KEY_foo define works for me.
>>
> I think this should be a configurable input, by design. So my preference (if not allowed a new keycode, which I personally think is the better option) is for PROG1.
>
> I discussed with Peter last night and it looks likely OK on their side. I do plan on doing some testing first, so it might take a few days to get the next set of patches out.
Ok, PROG1 works for me.
Regards,
Hans
^ permalink raw reply
* Re: [PATCH v2 1/1] mm: change inlined allocation helpers to account at the call site
From: Jan Kara @ 2024-04-16 13:55 UTC (permalink / raw)
To: Suren Baghdasaryan
Cc: akpm, willy, jack, joro, will, trond.myklebust, anna, arnd,
herbert, davem, jikos, benjamin.tissoires, tytso, jack, dennis,
tj, cl, jakub, penberg, rientjes, iamjoonsoo.kim, vbabka,
edumazet, kuba, pabeni, iommu, linux-kernel, linux-nfs,
linux-acpi, acpica-devel, linux-arch, linux-crypto, bpf,
linux-input, linux-ext4, linux-mm, netdev, linux-security-module,
kent.overstreet
In-Reply-To: <20240415020731.1152108-1-surenb@google.com>
On Sun 14-04-24 19:07:31, Suren Baghdasaryan wrote:
> Main goal of memory allocation profiling patchset is to provide accounting
> that is cheap enough to run in production. To achieve that we inject
> counters using codetags at the allocation call sites to account every time
> allocation is made. This injection allows us to perform accounting
> efficiently because injected counters are immediately available as opposed
> to the alternative methods, such as using _RET_IP_, which would require
> counter lookup and appropriate locking that makes accounting much more
> expensive. This method requires all allocation functions to inject
> separate counters at their call sites so that their callers can be
> individually accounted. Counter injection is implemented by allocation
> hooks which should wrap all allocation functions.
>
> Inlined functions which perform allocations but do not use allocation
> hooks are directly charged for the allocations they perform. In most
> cases these functions are just specialized allocation wrappers used
> from multiple places to allocate objects of a specific type. It would
> be more useful to do the accounting at their call sites instead.
> Instrument these helpers to do accounting at the call site. Simple
> inlined allocation wrappers are converted directly into macros. More
> complex allocators or allocators with documentation are converted into
> _noprof versions and allocation hooks are added. This allows memory
> allocation profiling mechanism to charge allocations to the callers
> of these functions.
>
> Signed-off-by: Suren Baghdasaryan <surenb@google.com>
Feel free to add:
Acked-by: Jan Kara <jack@suse.cz>
for the jbd2 bits.
Honza
> ---
> Changes since v1 [1]:
> - Added explicit type casts in macro replacements to force type checks,
> per Matthew Wilcox
> - Expanded the changelog to explain the reasons codetags are used and
> this change is neeed, per Jan Kara
>
> [1] https://lore.kernel.org/all/20240404165404.3805498-1-surenb@google.com/
>
> drivers/iommu/amd/amd_iommu.h | 5 ++--
> fs/nfs/iostat.h | 5 +---
> include/acpi/platform/aclinuxex.h | 19 +++++---------
> include/asm-generic/pgalloc.h | 35 +++++++++++++++----------
> include/crypto/hash.h | 7 ++---
> include/crypto/internal/acompress.h | 5 ++--
> include/crypto/skcipher.h | 7 ++---
> include/linux/bpf.h | 33 ++++++-----------------
> include/linux/bpfptr.h | 5 ++--
> include/linux/dma-fence-chain.h | 6 ++---
> include/linux/hid_bpf.h | 6 ++---
> include/linux/jbd2.h | 12 +++------
> include/linux/mm.h | 5 ++--
> include/linux/mm_types.h | 5 ++--
> include/linux/percpu.h | 3 +++
> include/linux/ptr_ring.h | 28 +++++++++++---------
> include/linux/skb_array.h | 19 ++++++++------
> include/linux/skbuff.h | 20 ++++++--------
> include/linux/skmsg.h | 8 +++---
> include/linux/slab.h | 5 ++++
> include/linux/sockptr.h | 10 ++++---
> include/net/netlabel.h | 16 ++++++-----
> include/net/netlink.h | 5 ++--
> include/net/request_sock.h | 5 ++--
> include/net/tcx.h | 5 ++--
> net/sunrpc/auth_gss/auth_gss_internal.h | 6 +++--
> 26 files changed, 142 insertions(+), 143 deletions(-)
>
> diff --git a/drivers/iommu/amd/amd_iommu.h b/drivers/iommu/amd/amd_iommu.h
> index f482aab420f7..52575ba9a141 100644
> --- a/drivers/iommu/amd/amd_iommu.h
> +++ b/drivers/iommu/amd/amd_iommu.h
> @@ -134,13 +134,14 @@ static inline int get_pci_sbdf_id(struct pci_dev *pdev)
> return PCI_SEG_DEVID_TO_SBDF(seg, devid);
> }
>
> -static inline void *alloc_pgtable_page(int nid, gfp_t gfp)
> +static inline void *alloc_pgtable_page_noprof(int nid, gfp_t gfp)
> {
> struct page *page;
>
> - page = alloc_pages_node(nid, gfp | __GFP_ZERO, 0);
> + page = alloc_pages_node_noprof(nid, gfp | __GFP_ZERO, 0);
> return page ? page_address(page) : NULL;
> }
> +#define alloc_pgtable_page(...) alloc_hooks(alloc_pgtable_page_noprof(__VA_ARGS__))
>
> /*
> * This must be called after device probe completes. During probe
> diff --git a/fs/nfs/iostat.h b/fs/nfs/iostat.h
> index 5aa776b5a3e7..b17a9eb9b148 100644
> --- a/fs/nfs/iostat.h
> +++ b/fs/nfs/iostat.h
> @@ -46,10 +46,7 @@ static inline void nfs_add_stats(const struct inode *inode,
> nfs_add_server_stats(NFS_SERVER(inode), stat, addend);
> }
>
> -static inline struct nfs_iostats __percpu *nfs_alloc_iostats(void)
> -{
> - return alloc_percpu(struct nfs_iostats);
> -}
> +#define nfs_alloc_iostats() alloc_percpu(struct nfs_iostats)
>
> static inline void nfs_free_iostats(struct nfs_iostats __percpu *stats)
> {
> diff --git a/include/acpi/platform/aclinuxex.h b/include/acpi/platform/aclinuxex.h
> index 600d4e2641da..62cac266a1c8 100644
> --- a/include/acpi/platform/aclinuxex.h
> +++ b/include/acpi/platform/aclinuxex.h
> @@ -47,26 +47,19 @@ acpi_status acpi_os_terminate(void);
> * However, boot has (system_state != SYSTEM_RUNNING)
> * to quiet __might_sleep() in kmalloc() and resume does not.
> */
> -static inline void *acpi_os_allocate(acpi_size size)
> -{
> - return kmalloc(size, irqs_disabled()? GFP_ATOMIC : GFP_KERNEL);
> -}
> +#define acpi_os_allocate(_size) \
> + kmalloc(_size, irqs_disabled() ? GFP_ATOMIC : GFP_KERNEL)
>
> -static inline void *acpi_os_allocate_zeroed(acpi_size size)
> -{
> - return kzalloc(size, irqs_disabled()? GFP_ATOMIC : GFP_KERNEL);
> -}
> +#define acpi_os_allocate_zeroed(_size) \
> + kzalloc(_size, irqs_disabled() ? GFP_ATOMIC : GFP_KERNEL)
>
> static inline void acpi_os_free(void *memory)
> {
> kfree(memory);
> }
>
> -static inline void *acpi_os_acquire_object(acpi_cache_t * cache)
> -{
> - return kmem_cache_zalloc(cache,
> - irqs_disabled()? GFP_ATOMIC : GFP_KERNEL);
> -}
> +#define acpi_os_acquire_object(_cache) \
> + kmem_cache_zalloc(_cache, irqs_disabled() ? GFP_ATOMIC : GFP_KERNEL)
>
> static inline acpi_thread_id acpi_os_get_thread_id(void)
> {
> diff --git a/include/asm-generic/pgalloc.h b/include/asm-generic/pgalloc.h
> index 879e5f8aa5e9..7c48f5fbf8aa 100644
> --- a/include/asm-generic/pgalloc.h
> +++ b/include/asm-generic/pgalloc.h
> @@ -16,15 +16,16 @@
> *
> * Return: pointer to the allocated memory or %NULL on error
> */
> -static inline pte_t *__pte_alloc_one_kernel(struct mm_struct *mm)
> +static inline pte_t *__pte_alloc_one_kernel_noprof(struct mm_struct *mm)
> {
> - struct ptdesc *ptdesc = pagetable_alloc(GFP_PGTABLE_KERNEL &
> + struct ptdesc *ptdesc = pagetable_alloc_noprof(GFP_PGTABLE_KERNEL &
> ~__GFP_HIGHMEM, 0);
>
> if (!ptdesc)
> return NULL;
> return ptdesc_address(ptdesc);
> }
> +#define __pte_alloc_one_kernel(...) alloc_hooks(__pte_alloc_one_kernel_noprof(__VA_ARGS__))
>
> #ifndef __HAVE_ARCH_PTE_ALLOC_ONE_KERNEL
> /**
> @@ -33,10 +34,11 @@ static inline pte_t *__pte_alloc_one_kernel(struct mm_struct *mm)
> *
> * Return: pointer to the allocated memory or %NULL on error
> */
> -static inline pte_t *pte_alloc_one_kernel(struct mm_struct *mm)
> +static inline pte_t *pte_alloc_one_kernel_noprof(struct mm_struct *mm)
> {
> - return __pte_alloc_one_kernel(mm);
> + return __pte_alloc_one_kernel_noprof(mm);
> }
> +#define pte_alloc_one_kernel(...) alloc_hooks(pte_alloc_one_kernel_noprof(__VA_ARGS__))
> #endif
>
> /**
> @@ -61,11 +63,11 @@ static inline void pte_free_kernel(struct mm_struct *mm, pte_t *pte)
> *
> * Return: `struct page` referencing the ptdesc or %NULL on error
> */
> -static inline pgtable_t __pte_alloc_one(struct mm_struct *mm, gfp_t gfp)
> +static inline pgtable_t __pte_alloc_one_noprof(struct mm_struct *mm, gfp_t gfp)
> {
> struct ptdesc *ptdesc;
>
> - ptdesc = pagetable_alloc(gfp, 0);
> + ptdesc = pagetable_alloc_noprof(gfp, 0);
> if (!ptdesc)
> return NULL;
> if (!pagetable_pte_ctor(ptdesc)) {
> @@ -75,6 +77,7 @@ static inline pgtable_t __pte_alloc_one(struct mm_struct *mm, gfp_t gfp)
>
> return ptdesc_page(ptdesc);
> }
> +#define __pte_alloc_one(...) alloc_hooks(__pte_alloc_one_noprof(__VA_ARGS__))
>
> #ifndef __HAVE_ARCH_PTE_ALLOC_ONE
> /**
> @@ -85,10 +88,11 @@ static inline pgtable_t __pte_alloc_one(struct mm_struct *mm, gfp_t gfp)
> *
> * Return: `struct page` referencing the ptdesc or %NULL on error
> */
> -static inline pgtable_t pte_alloc_one(struct mm_struct *mm)
> +static inline pgtable_t pte_alloc_one_noprof(struct mm_struct *mm)
> {
> - return __pte_alloc_one(mm, GFP_PGTABLE_USER);
> + return __pte_alloc_one_noprof(mm, GFP_PGTABLE_USER);
> }
> +#define pte_alloc_one(...) alloc_hooks(pte_alloc_one_noprof(__VA_ARGS__))
> #endif
>
> /*
> @@ -124,14 +128,14 @@ static inline void pte_free(struct mm_struct *mm, struct page *pte_page)
> *
> * Return: pointer to the allocated memory or %NULL on error
> */
> -static inline pmd_t *pmd_alloc_one(struct mm_struct *mm, unsigned long addr)
> +static inline pmd_t *pmd_alloc_one_noprof(struct mm_struct *mm, unsigned long addr)
> {
> struct ptdesc *ptdesc;
> gfp_t gfp = GFP_PGTABLE_USER;
>
> if (mm == &init_mm)
> gfp = GFP_PGTABLE_KERNEL;
> - ptdesc = pagetable_alloc(gfp, 0);
> + ptdesc = pagetable_alloc_noprof(gfp, 0);
> if (!ptdesc)
> return NULL;
> if (!pagetable_pmd_ctor(ptdesc)) {
> @@ -140,6 +144,7 @@ static inline pmd_t *pmd_alloc_one(struct mm_struct *mm, unsigned long addr)
> }
> return ptdesc_address(ptdesc);
> }
> +#define pmd_alloc_one(...) alloc_hooks(pmd_alloc_one_noprof(__VA_ARGS__))
> #endif
>
> #ifndef __HAVE_ARCH_PMD_FREE
> @@ -157,7 +162,7 @@ static inline void pmd_free(struct mm_struct *mm, pmd_t *pmd)
>
> #if CONFIG_PGTABLE_LEVELS > 3
>
> -static inline pud_t *__pud_alloc_one(struct mm_struct *mm, unsigned long addr)
> +static inline pud_t *__pud_alloc_one_noprof(struct mm_struct *mm, unsigned long addr)
> {
> gfp_t gfp = GFP_PGTABLE_USER;
> struct ptdesc *ptdesc;
> @@ -166,13 +171,14 @@ static inline pud_t *__pud_alloc_one(struct mm_struct *mm, unsigned long addr)
> gfp = GFP_PGTABLE_KERNEL;
> gfp &= ~__GFP_HIGHMEM;
>
> - ptdesc = pagetable_alloc(gfp, 0);
> + ptdesc = pagetable_alloc_noprof(gfp, 0);
> if (!ptdesc)
> return NULL;
>
> pagetable_pud_ctor(ptdesc);
> return ptdesc_address(ptdesc);
> }
> +#define __pud_alloc_one(...) alloc_hooks(__pud_alloc_one_noprof(__VA_ARGS__))
>
> #ifndef __HAVE_ARCH_PUD_ALLOC_ONE
> /**
> @@ -184,10 +190,11 @@ static inline pud_t *__pud_alloc_one(struct mm_struct *mm, unsigned long addr)
> *
> * Return: pointer to the allocated memory or %NULL on error
> */
> -static inline pud_t *pud_alloc_one(struct mm_struct *mm, unsigned long addr)
> +static inline pud_t *pud_alloc_one_noprof(struct mm_struct *mm, unsigned long addr)
> {
> - return __pud_alloc_one(mm, addr);
> + return __pud_alloc_one_noprof(mm, addr);
> }
> +#define pud_alloc_one(...) alloc_hooks(pud_alloc_one_noprof(__VA_ARGS__))
> #endif
>
> static inline void __pud_free(struct mm_struct *mm, pud_t *pud)
> diff --git a/include/crypto/hash.h b/include/crypto/hash.h
> index 5d61f576cfc8..e5181cc9b7c5 100644
> --- a/include/crypto/hash.h
> +++ b/include/crypto/hash.h
> @@ -578,19 +578,20 @@ static inline void ahash_request_set_tfm(struct ahash_request *req,
> *
> * Return: allocated request handle in case of success, or NULL if out of memory
> */
> -static inline struct ahash_request *ahash_request_alloc(
> +static inline struct ahash_request *ahash_request_alloc_noprof(
> struct crypto_ahash *tfm, gfp_t gfp)
> {
> struct ahash_request *req;
>
> - req = kmalloc(sizeof(struct ahash_request) +
> - crypto_ahash_reqsize(tfm), gfp);
> + req = kmalloc_noprof(sizeof(struct ahash_request) +
> + crypto_ahash_reqsize(tfm), gfp);
>
> if (likely(req))
> ahash_request_set_tfm(req, tfm);
>
> return req;
> }
> +#define ahash_request_alloc(...) alloc_hooks(ahash_request_alloc_noprof(__VA_ARGS__))
>
> /**
> * ahash_request_free() - zeroize and free the request data structure
> diff --git a/include/crypto/internal/acompress.h b/include/crypto/internal/acompress.h
> index 4ac46bafba9d..2a67793f52ad 100644
> --- a/include/crypto/internal/acompress.h
> +++ b/include/crypto/internal/acompress.h
> @@ -69,15 +69,16 @@ static inline void acomp_request_complete(struct acomp_req *req,
> crypto_request_complete(&req->base, err);
> }
>
> -static inline struct acomp_req *__acomp_request_alloc(struct crypto_acomp *tfm)
> +static inline struct acomp_req *__acomp_request_alloc_noprof(struct crypto_acomp *tfm)
> {
> struct acomp_req *req;
>
> - req = kzalloc(sizeof(*req) + crypto_acomp_reqsize(tfm), GFP_KERNEL);
> + req = kzalloc_noprof(sizeof(*req) + crypto_acomp_reqsize(tfm), GFP_KERNEL);
> if (likely(req))
> acomp_request_set_tfm(req, tfm);
> return req;
> }
> +#define __acomp_request_alloc(...) alloc_hooks(__acomp_request_alloc_noprof(__VA_ARGS__))
>
> static inline void __acomp_request_free(struct acomp_req *req)
> {
> diff --git a/include/crypto/skcipher.h b/include/crypto/skcipher.h
> index c8857d7bdb37..6c5330e316b0 100644
> --- a/include/crypto/skcipher.h
> +++ b/include/crypto/skcipher.h
> @@ -861,19 +861,20 @@ static inline struct skcipher_request *skcipher_request_cast(
> *
> * Return: allocated request handle in case of success, or NULL if out of memory
> */
> -static inline struct skcipher_request *skcipher_request_alloc(
> +static inline struct skcipher_request *skcipher_request_alloc_noprof(
> struct crypto_skcipher *tfm, gfp_t gfp)
> {
> struct skcipher_request *req;
>
> - req = kmalloc(sizeof(struct skcipher_request) +
> - crypto_skcipher_reqsize(tfm), gfp);
> + req = kmalloc_noprof(sizeof(struct skcipher_request) +
> + crypto_skcipher_reqsize(tfm), gfp);
>
> if (likely(req))
> skcipher_request_set_tfm(req, tfm);
>
> return req;
> }
> +#define skcipher_request_alloc(...) alloc_hooks(skcipher_request_alloc_noprof(__VA_ARGS__))
>
> /**
> * skcipher_request_free() - zeroize and free request data structure
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 4f20f62f9d63..a63fa48ab80d 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -2230,31 +2230,14 @@ void *bpf_map_kvcalloc(struct bpf_map *map, size_t n, size_t size,
> void __percpu *bpf_map_alloc_percpu(const struct bpf_map *map, size_t size,
> size_t align, gfp_t flags);
> #else
> -static inline void *
> -bpf_map_kmalloc_node(const struct bpf_map *map, size_t size, gfp_t flags,
> - int node)
> -{
> - return kmalloc_node(size, flags, node);
> -}
> -
> -static inline void *
> -bpf_map_kzalloc(const struct bpf_map *map, size_t size, gfp_t flags)
> -{
> - return kzalloc(size, flags);
> -}
> -
> -static inline void *
> -bpf_map_kvcalloc(struct bpf_map *map, size_t n, size_t size, gfp_t flags)
> -{
> - return kvcalloc(n, size, flags);
> -}
> -
> -static inline void __percpu *
> -bpf_map_alloc_percpu(const struct bpf_map *map, size_t size, size_t align,
> - gfp_t flags)
> -{
> - return __alloc_percpu_gfp(size, align, flags);
> -}
> +#define bpf_map_kmalloc_node(_map, _size, _flags, _node) \
> + kmalloc_node(_size, _flags, _node)
> +#define bpf_map_kzalloc(_map, _size, _flags) \
> + kzalloc(_size, _flags)
> +#define bpf_map_kvcalloc(_map, _n, _size, _flags) \
> + kvcalloc(_n, _size, _flags)
> +#define bpf_map_alloc_percpu(_map, _size, _align, _flags) \
> + __alloc_percpu_gfp(_size, _align, _flags)
> #endif
>
> static inline int
> diff --git a/include/linux/bpfptr.h b/include/linux/bpfptr.h
> index 79b2f78eec1a..1af241525a17 100644
> --- a/include/linux/bpfptr.h
> +++ b/include/linux/bpfptr.h
> @@ -65,9 +65,9 @@ static inline int copy_to_bpfptr_offset(bpfptr_t dst, size_t offset,
> return copy_to_sockptr_offset((sockptr_t) dst, offset, src, size);
> }
>
> -static inline void *kvmemdup_bpfptr(bpfptr_t src, size_t len)
> +static inline void *kvmemdup_bpfptr_noprof(bpfptr_t src, size_t len)
> {
> - void *p = kvmalloc(len, GFP_USER | __GFP_NOWARN);
> + void *p = kvmalloc_noprof(len, GFP_USER | __GFP_NOWARN);
>
> if (!p)
> return ERR_PTR(-ENOMEM);
> @@ -77,6 +77,7 @@ static inline void *kvmemdup_bpfptr(bpfptr_t src, size_t len)
> }
> return p;
> }
> +#define kvmemdup_bpfptr(...) alloc_hooks(kvmemdup_bpfptr_noprof(__VA_ARGS__))
>
> static inline long strncpy_from_bpfptr(char *dst, bpfptr_t src, size_t count)
> {
> diff --git a/include/linux/dma-fence-chain.h b/include/linux/dma-fence-chain.h
> index 4bdf0b96da28..ad9e2506c2f4 100644
> --- a/include/linux/dma-fence-chain.h
> +++ b/include/linux/dma-fence-chain.h
> @@ -86,10 +86,8 @@ dma_fence_chain_contained(struct dma_fence *fence)
> *
> * Returns a new struct dma_fence_chain object or NULL on failure.
> */
> -static inline struct dma_fence_chain *dma_fence_chain_alloc(void)
> -{
> - return kmalloc(sizeof(struct dma_fence_chain), GFP_KERNEL);
> -};
> +#define dma_fence_chain_alloc() \
> + ((struct dma_fence_chain *)kmalloc(sizeof(struct dma_fence_chain), GFP_KERNEL))
>
> /**
> * dma_fence_chain_free
> diff --git a/include/linux/hid_bpf.h b/include/linux/hid_bpf.h
> index 7118ac28d468..ca70eeb6393e 100644
> --- a/include/linux/hid_bpf.h
> +++ b/include/linux/hid_bpf.h
> @@ -149,10 +149,8 @@ static inline int hid_bpf_connect_device(struct hid_device *hdev) { return 0; }
> static inline void hid_bpf_disconnect_device(struct hid_device *hdev) {}
> static inline void hid_bpf_destroy_device(struct hid_device *hid) {}
> static inline void hid_bpf_device_init(struct hid_device *hid) {}
> -static inline u8 *call_hid_bpf_rdesc_fixup(struct hid_device *hdev, u8 *rdesc, unsigned int *size)
> -{
> - return kmemdup(rdesc, *size, GFP_KERNEL);
> -}
> +#define call_hid_bpf_rdesc_fixup(_hdev, _rdesc, _size) \
> + ((u8 *)kmemdup(_rdesc, *(_size), GFP_KERNEL))
>
> #endif /* CONFIG_HID_BPF */
>
> diff --git a/include/linux/jbd2.h b/include/linux/jbd2.h
> index 971f3e826e15..9512fe332668 100644
> --- a/include/linux/jbd2.h
> +++ b/include/linux/jbd2.h
> @@ -1586,10 +1586,8 @@ void jbd2_journal_put_journal_head(struct journal_head *jh);
> */
> extern struct kmem_cache *jbd2_handle_cache;
>
> -static inline handle_t *jbd2_alloc_handle(gfp_t gfp_flags)
> -{
> - return kmem_cache_zalloc(jbd2_handle_cache, gfp_flags);
> -}
> +#define jbd2_alloc_handle(_gfp_flags) \
> + ((handle_t *)kmem_cache_zalloc(jbd2_handle_cache, _gfp_flags))
>
> static inline void jbd2_free_handle(handle_t *handle)
> {
> @@ -1602,10 +1600,8 @@ static inline void jbd2_free_handle(handle_t *handle)
> */
> extern struct kmem_cache *jbd2_inode_cache;
>
> -static inline struct jbd2_inode *jbd2_alloc_inode(gfp_t gfp_flags)
> -{
> - return kmem_cache_alloc(jbd2_inode_cache, gfp_flags);
> -}
> +#define jbd2_alloc_inode(_gfp_flags) \
> + ((struct jbd2_inode *)kmem_cache_alloc(jbd2_inode_cache, _gfp_flags))
>
> static inline void jbd2_free_inode(struct jbd2_inode *jinode)
> {
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 07c73451d42f..d261e45bb29b 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -2899,12 +2899,13 @@ static inline bool pagetable_is_reserved(struct ptdesc *pt)
> *
> * Return: The ptdesc describing the allocated page tables.
> */
> -static inline struct ptdesc *pagetable_alloc(gfp_t gfp, unsigned int order)
> +static inline struct ptdesc *pagetable_alloc_noprof(gfp_t gfp, unsigned int order)
> {
> - struct page *page = alloc_pages(gfp | __GFP_COMP, order);
> + struct page *page = alloc_pages_noprof(gfp | __GFP_COMP, order);
>
> return page_ptdesc(page);
> }
> +#define pagetable_alloc(...) alloc_hooks(pagetable_alloc_noprof(__VA_ARGS__))
>
> /**
> * pagetable_free - Free pagetables
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index c432add95913..db0adf5721cc 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -1167,14 +1167,15 @@ static inline void mm_init_cid(struct mm_struct *mm)
> cpumask_clear(mm_cidmask(mm));
> }
>
> -static inline int mm_alloc_cid(struct mm_struct *mm)
> +static inline int mm_alloc_cid_noprof(struct mm_struct *mm)
> {
> - mm->pcpu_cid = alloc_percpu(struct mm_cid);
> + mm->pcpu_cid = alloc_percpu_noprof(struct mm_cid);
> if (!mm->pcpu_cid)
> return -ENOMEM;
> mm_init_cid(mm);
> return 0;
> }
> +#define mm_alloc_cid(...) alloc_hooks(mm_alloc_cid_noprof(__VA_ARGS__))
>
> static inline void mm_destroy_cid(struct mm_struct *mm)
> {
> diff --git a/include/linux/percpu.h b/include/linux/percpu.h
> index 13a82f11e4fd..03053de557cf 100644
> --- a/include/linux/percpu.h
> +++ b/include/linux/percpu.h
> @@ -151,6 +151,9 @@ extern size_t pcpu_alloc_size(void __percpu *__pdata);
> #define alloc_percpu(type) \
> (typeof(type) __percpu *)__alloc_percpu(sizeof(type), \
> __alignof__(type))
> +#define alloc_percpu_noprof(type) \
> + ((typeof(type) __percpu *)pcpu_alloc_noprof(sizeof(type), \
> + __alignof__(type), false, GFP_KERNEL))
>
> extern void free_percpu(void __percpu *__pdata);
>
> diff --git a/include/linux/ptr_ring.h b/include/linux/ptr_ring.h
> index 808f9d3ee546..fd037c127bb0 100644
> --- a/include/linux/ptr_ring.h
> +++ b/include/linux/ptr_ring.h
> @@ -464,11 +464,11 @@ static inline int ptr_ring_consume_batched_bh(struct ptr_ring *r,
> /* Not all gfp_t flags (besides GFP_KERNEL) are allowed. See
> * documentation for vmalloc for which of them are legal.
> */
> -static inline void **__ptr_ring_init_queue_alloc(unsigned int size, gfp_t gfp)
> +static inline void **__ptr_ring_init_queue_alloc_noprof(unsigned int size, gfp_t gfp)
> {
> if (size > KMALLOC_MAX_SIZE / sizeof(void *))
> return NULL;
> - return kvmalloc_array(size, sizeof(void *), gfp | __GFP_ZERO);
> + return kvmalloc_array_noprof(size, sizeof(void *), gfp | __GFP_ZERO);
> }
>
> static inline void __ptr_ring_set_size(struct ptr_ring *r, int size)
> @@ -484,9 +484,9 @@ static inline void __ptr_ring_set_size(struct ptr_ring *r, int size)
> r->batch = 1;
> }
>
> -static inline int ptr_ring_init(struct ptr_ring *r, int size, gfp_t gfp)
> +static inline int ptr_ring_init_noprof(struct ptr_ring *r, int size, gfp_t gfp)
> {
> - r->queue = __ptr_ring_init_queue_alloc(size, gfp);
> + r->queue = __ptr_ring_init_queue_alloc_noprof(size, gfp);
> if (!r->queue)
> return -ENOMEM;
>
> @@ -497,6 +497,7 @@ static inline int ptr_ring_init(struct ptr_ring *r, int size, gfp_t gfp)
>
> return 0;
> }
> +#define ptr_ring_init(...) alloc_hooks(ptr_ring_init_noprof(__VA_ARGS__))
>
> /*
> * Return entries into ring. Destroy entries that don't fit.
> @@ -587,11 +588,11 @@ static inline void **__ptr_ring_swap_queue(struct ptr_ring *r, void **queue,
> * In particular if you consume ring in interrupt or BH context, you must
> * disable interrupts/BH when doing so.
> */
> -static inline int ptr_ring_resize(struct ptr_ring *r, int size, gfp_t gfp,
> +static inline int ptr_ring_resize_noprof(struct ptr_ring *r, int size, gfp_t gfp,
> void (*destroy)(void *))
> {
> unsigned long flags;
> - void **queue = __ptr_ring_init_queue_alloc(size, gfp);
> + void **queue = __ptr_ring_init_queue_alloc_noprof(size, gfp);
> void **old;
>
> if (!queue)
> @@ -609,6 +610,7 @@ static inline int ptr_ring_resize(struct ptr_ring *r, int size, gfp_t gfp,
>
> return 0;
> }
> +#define ptr_ring_resize(...) alloc_hooks(ptr_ring_resize_noprof(__VA_ARGS__))
>
> /*
> * Note: producer lock is nested within consumer lock, so if you
> @@ -616,21 +618,21 @@ static inline int ptr_ring_resize(struct ptr_ring *r, int size, gfp_t gfp,
> * In particular if you consume ring in interrupt or BH context, you must
> * disable interrupts/BH when doing so.
> */
> -static inline int ptr_ring_resize_multiple(struct ptr_ring **rings,
> - unsigned int nrings,
> - int size,
> - gfp_t gfp, void (*destroy)(void *))
> +static inline int ptr_ring_resize_multiple_noprof(struct ptr_ring **rings,
> + unsigned int nrings,
> + int size,
> + gfp_t gfp, void (*destroy)(void *))
> {
> unsigned long flags;
> void ***queues;
> int i;
>
> - queues = kmalloc_array(nrings, sizeof(*queues), gfp);
> + queues = kmalloc_array_noprof(nrings, sizeof(*queues), gfp);
> if (!queues)
> goto noqueues;
>
> for (i = 0; i < nrings; ++i) {
> - queues[i] = __ptr_ring_init_queue_alloc(size, gfp);
> + queues[i] = __ptr_ring_init_queue_alloc_noprof(size, gfp);
> if (!queues[i])
> goto nomem;
> }
> @@ -660,6 +662,8 @@ static inline int ptr_ring_resize_multiple(struct ptr_ring **rings,
> noqueues:
> return -ENOMEM;
> }
> +#define ptr_ring_resize_multiple(...) \
> + alloc_hooks(ptr_ring_resize_multiple_noprof(__VA_ARGS__))
>
> static inline void ptr_ring_cleanup(struct ptr_ring *r, void (*destroy)(void *))
> {
> diff --git a/include/linux/skb_array.h b/include/linux/skb_array.h
> index e2d45b7cb619..926496c9cc9c 100644
> --- a/include/linux/skb_array.h
> +++ b/include/linux/skb_array.h
> @@ -177,10 +177,11 @@ static inline int skb_array_peek_len_any(struct skb_array *a)
> return PTR_RING_PEEK_CALL_ANY(&a->ring, __skb_array_len_with_tag);
> }
>
> -static inline int skb_array_init(struct skb_array *a, int size, gfp_t gfp)
> +static inline int skb_array_init_noprof(struct skb_array *a, int size, gfp_t gfp)
> {
> - return ptr_ring_init(&a->ring, size, gfp);
> + return ptr_ring_init_noprof(&a->ring, size, gfp);
> }
> +#define skb_array_init(...) alloc_hooks(skb_array_init_noprof(__VA_ARGS__))
>
> static void __skb_array_destroy_skb(void *ptr)
> {
> @@ -198,15 +199,17 @@ static inline int skb_array_resize(struct skb_array *a, int size, gfp_t gfp)
> return ptr_ring_resize(&a->ring, size, gfp, __skb_array_destroy_skb);
> }
>
> -static inline int skb_array_resize_multiple(struct skb_array **rings,
> - int nrings, unsigned int size,
> - gfp_t gfp)
> +static inline int skb_array_resize_multiple_noprof(struct skb_array **rings,
> + int nrings, unsigned int size,
> + gfp_t gfp)
> {
> BUILD_BUG_ON(offsetof(struct skb_array, ring));
> - return ptr_ring_resize_multiple((struct ptr_ring **)rings,
> - nrings, size, gfp,
> - __skb_array_destroy_skb);
> + return ptr_ring_resize_multiple_noprof((struct ptr_ring **)rings,
> + nrings, size, gfp,
> + __skb_array_destroy_skb);
> }
> +#define skb_array_resize_multiple(...) \
> + alloc_hooks(skb_array_resize_multiple_noprof(__VA_ARGS__))
>
> static inline void skb_array_cleanup(struct skb_array *a)
> {
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 9d24aec064e8..b72920c9887b 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -3371,7 +3371,7 @@ void __napi_kfree_skb(struct sk_buff *skb, enum skb_drop_reason reason);
> *
> * %NULL is returned if there is no free memory.
> */
> -static inline struct page *__dev_alloc_pages(gfp_t gfp_mask,
> +static inline struct page *__dev_alloc_pages_noprof(gfp_t gfp_mask,
> unsigned int order)
> {
> /* This piece of code contains several assumptions.
> @@ -3384,13 +3384,11 @@ static inline struct page *__dev_alloc_pages(gfp_t gfp_mask,
> */
> gfp_mask |= __GFP_COMP | __GFP_MEMALLOC;
>
> - return alloc_pages_node(NUMA_NO_NODE, gfp_mask, order);
> + return alloc_pages_node_noprof(NUMA_NO_NODE, gfp_mask, order);
> }
> +#define __dev_alloc_pages(...) alloc_hooks(__dev_alloc_pages_noprof(__VA_ARGS__))
>
> -static inline struct page *dev_alloc_pages(unsigned int order)
> -{
> - return __dev_alloc_pages(GFP_ATOMIC | __GFP_NOWARN, order);
> -}
> +#define dev_alloc_pages(_order) __dev_alloc_pages(GFP_ATOMIC | __GFP_NOWARN, _order)
>
> /**
> * __dev_alloc_page - allocate a page for network Rx
> @@ -3400,15 +3398,13 @@ static inline struct page *dev_alloc_pages(unsigned int order)
> *
> * %NULL is returned if there is no free memory.
> */
> -static inline struct page *__dev_alloc_page(gfp_t gfp_mask)
> +static inline struct page *__dev_alloc_page_noprof(gfp_t gfp_mask)
> {
> - return __dev_alloc_pages(gfp_mask, 0);
> + return __dev_alloc_pages_noprof(gfp_mask, 0);
> }
> +#define __dev_alloc_page(...) alloc_hooks(__dev_alloc_page_noprof(__VA_ARGS__))
>
> -static inline struct page *dev_alloc_page(void)
> -{
> - return dev_alloc_pages(0);
> -}
> +#define dev_alloc_page() dev_alloc_pages(0)
>
> /**
> * dev_page_is_reusable - check whether a page can be reused for network Rx
> diff --git a/include/linux/skmsg.h b/include/linux/skmsg.h
> index e65ec3fd2799..78efc5b20284 100644
> --- a/include/linux/skmsg.h
> +++ b/include/linux/skmsg.h
> @@ -410,11 +410,9 @@ void sk_psock_stop_verdict(struct sock *sk, struct sk_psock *psock);
> int sk_psock_msg_verdict(struct sock *sk, struct sk_psock *psock,
> struct sk_msg *msg);
>
> -static inline struct sk_psock_link *sk_psock_init_link(void)
> -{
> - return kzalloc(sizeof(struct sk_psock_link),
> - GFP_ATOMIC | __GFP_NOWARN);
> -}
> +#define sk_psock_init_link() \
> + ((struct sk_psock_link *)kzalloc(sizeof(struct sk_psock_link), \
> + GFP_ATOMIC | __GFP_NOWARN))
>
> static inline void sk_psock_free_link(struct sk_psock_link *link)
> {
> diff --git a/include/linux/slab.h b/include/linux/slab.h
> index c0be1cd03cf6..4cc37ef22aae 100644
> --- a/include/linux/slab.h
> +++ b/include/linux/slab.h
> @@ -744,6 +744,9 @@ void *kmalloc_node_track_caller_noprof(size_t size, gfp_t flags, int node,
> */
> #define kmalloc_track_caller(...) kmalloc_node_track_caller(__VA_ARGS__, NUMA_NO_NODE)
>
> +#define kmalloc_track_caller_noprof(...) \
> + kmalloc_node_track_caller_noprof(__VA_ARGS__, NUMA_NO_NODE, _RET_IP_)
> +
> static inline __alloc_size(1, 2) void *kmalloc_array_node_noprof(size_t n, size_t size, gfp_t flags,
> int node)
> {
> @@ -781,6 +784,7 @@ extern void *kvmalloc_node_noprof(size_t size, gfp_t flags, int node) __alloc_si
> #define kvmalloc_node(...) alloc_hooks(kvmalloc_node_noprof(__VA_ARGS__))
>
> #define kvmalloc(_size, _flags) kvmalloc_node(_size, _flags, NUMA_NO_NODE)
> +#define kvmalloc_noprof(_size, _flags) kvmalloc_node_noprof(_size, _flags, NUMA_NO_NODE)
> #define kvzalloc(_size, _flags) kvmalloc(_size, _flags|__GFP_ZERO)
>
> #define kvzalloc_node(_size, _flags, _node) kvmalloc_node(_size, _flags|__GFP_ZERO, _node)
> @@ -797,6 +801,7 @@ static inline __alloc_size(1, 2) void *kvmalloc_array_noprof(size_t n, size_t si
>
> #define kvmalloc_array(...) alloc_hooks(kvmalloc_array_noprof(__VA_ARGS__))
> #define kvcalloc(_n, _size, _flags) kvmalloc_array(_n, _size, _flags|__GFP_ZERO)
> +#define kvcalloc_noprof(_n, _size, _flags) kvmalloc_array_noprof(_n, _size, _flags|__GFP_ZERO)
>
> extern void *kvrealloc_noprof(const void *p, size_t oldsize, size_t newsize, gfp_t flags)
> __realloc_size(3);
> diff --git a/include/linux/sockptr.h b/include/linux/sockptr.h
> index 307961b41541..ba703b22e7d8 100644
> --- a/include/linux/sockptr.h
> +++ b/include/linux/sockptr.h
> @@ -92,9 +92,9 @@ static inline int copy_to_sockptr(sockptr_t dst, const void *src, size_t size)
> return copy_to_sockptr_offset(dst, 0, src, size);
> }
>
> -static inline void *memdup_sockptr(sockptr_t src, size_t len)
> +static inline void *memdup_sockptr_noprof(sockptr_t src, size_t len)
> {
> - void *p = kmalloc_track_caller(len, GFP_USER | __GFP_NOWARN);
> + void *p = kmalloc_track_caller_noprof(len, GFP_USER | __GFP_NOWARN);
>
> if (!p)
> return ERR_PTR(-ENOMEM);
> @@ -104,10 +104,11 @@ static inline void *memdup_sockptr(sockptr_t src, size_t len)
> }
> return p;
> }
> +#define memdup_sockptr(...) alloc_hooks(memdup_sockptr_noprof(__VA_ARGS__))
>
> -static inline void *memdup_sockptr_nul(sockptr_t src, size_t len)
> +static inline void *memdup_sockptr_nul_noprof(sockptr_t src, size_t len)
> {
> - char *p = kmalloc_track_caller(len + 1, GFP_KERNEL);
> + char *p = kmalloc_track_caller_noprof(len + 1, GFP_KERNEL);
>
> if (!p)
> return ERR_PTR(-ENOMEM);
> @@ -118,6 +119,7 @@ static inline void *memdup_sockptr_nul(sockptr_t src, size_t len)
> p[len] = '\0';
> return p;
> }
> +#define memdup_sockptr_nul(...) alloc_hooks(memdup_sockptr_nul_noprof(__VA_ARGS__))
>
> static inline long strncpy_from_sockptr(char *dst, sockptr_t src, size_t count)
> {
> diff --git a/include/net/netlabel.h b/include/net/netlabel.h
> index f3ab0b8a4b18..c31bd96dafdb 100644
> --- a/include/net/netlabel.h
> +++ b/include/net/netlabel.h
> @@ -274,15 +274,17 @@ struct netlbl_calipso_ops {
> * on success, NULL on failure.
> *
> */
> -static inline struct netlbl_lsm_cache *netlbl_secattr_cache_alloc(gfp_t flags)
> +static inline struct netlbl_lsm_cache *netlbl_secattr_cache_alloc_noprof(gfp_t flags)
> {
> struct netlbl_lsm_cache *cache;
>
> - cache = kzalloc(sizeof(*cache), flags);
> + cache = kzalloc_noprof(sizeof(*cache), flags);
> if (cache)
> refcount_set(&cache->refcount, 1);
> return cache;
> }
> +#define netlbl_secattr_cache_alloc(...) \
> + alloc_hooks(netlbl_secattr_cache_alloc_noprof(__VA_ARGS__))
>
> /**
> * netlbl_secattr_cache_free - Frees a netlbl_lsm_cache struct
> @@ -311,10 +313,11 @@ static inline void netlbl_secattr_cache_free(struct netlbl_lsm_cache *cache)
> * on failure.
> *
> */
> -static inline struct netlbl_lsm_catmap *netlbl_catmap_alloc(gfp_t flags)
> +static inline struct netlbl_lsm_catmap *netlbl_catmap_alloc_noprof(gfp_t flags)
> {
> - return kzalloc(sizeof(struct netlbl_lsm_catmap), flags);
> + return kzalloc_noprof(sizeof(struct netlbl_lsm_catmap), flags);
> }
> +#define netlbl_catmap_alloc(...) alloc_hooks(netlbl_catmap_alloc_noprof(__VA_ARGS__))
>
> /**
> * netlbl_catmap_free - Free a LSM secattr catmap
> @@ -376,10 +379,11 @@ static inline void netlbl_secattr_destroy(struct netlbl_lsm_secattr *secattr)
> * pointer on success, or NULL on failure.
> *
> */
> -static inline struct netlbl_lsm_secattr *netlbl_secattr_alloc(gfp_t flags)
> +static inline struct netlbl_lsm_secattr *netlbl_secattr_alloc_noprof(gfp_t flags)
> {
> - return kzalloc(sizeof(struct netlbl_lsm_secattr), flags);
> + return kzalloc_noprof(sizeof(struct netlbl_lsm_secattr), flags);
> }
> +#define netlbl_secattr_alloc(...) alloc_hooks(netlbl_secattr_alloc_noprof(__VA_ARGS__))
>
> /**
> * netlbl_secattr_free - Frees a netlbl_lsm_secattr struct
> diff --git a/include/net/netlink.h b/include/net/netlink.h
> index c19ff921b661..972b5484fa6f 100644
> --- a/include/net/netlink.h
> +++ b/include/net/netlink.h
> @@ -1891,10 +1891,11 @@ static inline struct nla_bitfield32 nla_get_bitfield32(const struct nlattr *nla)
> * @src: netlink attribute to duplicate from
> * @gfp: GFP mask
> */
> -static inline void *nla_memdup(const struct nlattr *src, gfp_t gfp)
> +static inline void *nla_memdup_noprof(const struct nlattr *src, gfp_t gfp)
> {
> - return kmemdup(nla_data(src), nla_len(src), gfp);
> + return kmemdup_noprof(nla_data(src), nla_len(src), gfp);
> }
> +#define nla_memdup(...) alloc_hooks(nla_memdup_noprof(__VA_ARGS__))
>
> /**
> * nla_nest_start_noflag - Start a new level of nested attributes
> diff --git a/include/net/request_sock.h b/include/net/request_sock.h
> index 004e651e6067..29495c331d20 100644
> --- a/include/net/request_sock.h
> +++ b/include/net/request_sock.h
> @@ -127,12 +127,12 @@ static inline struct sock *skb_steal_sock(struct sk_buff *skb,
> }
>
> static inline struct request_sock *
> -reqsk_alloc(const struct request_sock_ops *ops, struct sock *sk_listener,
> +reqsk_alloc_noprof(const struct request_sock_ops *ops, struct sock *sk_listener,
> bool attach_listener)
> {
> struct request_sock *req;
>
> - req = kmem_cache_alloc(ops->slab, GFP_ATOMIC | __GFP_NOWARN);
> + req = kmem_cache_alloc_noprof(ops->slab, GFP_ATOMIC | __GFP_NOWARN);
> if (!req)
> return NULL;
> req->rsk_listener = NULL;
> @@ -157,6 +157,7 @@ reqsk_alloc(const struct request_sock_ops *ops, struct sock *sk_listener,
>
> return req;
> }
> +#define reqsk_alloc(...) alloc_hooks(reqsk_alloc_noprof(__VA_ARGS__))
>
> static inline void __reqsk_free(struct request_sock *req)
> {
> diff --git a/include/net/tcx.h b/include/net/tcx.h
> index 04be9377785d..72a3e75e539f 100644
> --- a/include/net/tcx.h
> +++ b/include/net/tcx.h
> @@ -75,9 +75,9 @@ tcx_entry_fetch(struct net_device *dev, bool ingress)
> return rcu_dereference_rtnl(dev->tcx_egress);
> }
>
> -static inline struct bpf_mprog_entry *tcx_entry_create(void)
> +static inline struct bpf_mprog_entry *tcx_entry_create_noprof(void)
> {
> - struct tcx_entry *tcx = kzalloc(sizeof(*tcx), GFP_KERNEL);
> + struct tcx_entry *tcx = kzalloc_noprof(sizeof(*tcx), GFP_KERNEL);
>
> if (tcx) {
> bpf_mprog_bundle_init(&tcx->bundle);
> @@ -85,6 +85,7 @@ static inline struct bpf_mprog_entry *tcx_entry_create(void)
> }
> return NULL;
> }
> +#define tcx_entry_create(...) alloc_hooks(tcx_entry_create_noprof(__VA_ARGS__))
>
> static inline void tcx_entry_free(struct bpf_mprog_entry *entry)
> {
> diff --git a/net/sunrpc/auth_gss/auth_gss_internal.h b/net/sunrpc/auth_gss/auth_gss_internal.h
> index c53b329092d4..4ebc1b7043d9 100644
> --- a/net/sunrpc/auth_gss/auth_gss_internal.h
> +++ b/net/sunrpc/auth_gss/auth_gss_internal.h
> @@ -23,7 +23,7 @@ simple_get_bytes(const void *p, const void *end, void *res, size_t len)
> }
>
> static inline const void *
> -simple_get_netobj(const void *p, const void *end, struct xdr_netobj *dest)
> +simple_get_netobj_noprof(const void *p, const void *end, struct xdr_netobj *dest)
> {
> const void *q;
> unsigned int len;
> @@ -35,7 +35,7 @@ simple_get_netobj(const void *p, const void *end, struct xdr_netobj *dest)
> if (unlikely(q > end || q < p))
> return ERR_PTR(-EFAULT);
> if (len) {
> - dest->data = kmemdup(p, len, GFP_KERNEL);
> + dest->data = kmemdup_noprof(p, len, GFP_KERNEL);
> if (unlikely(dest->data == NULL))
> return ERR_PTR(-ENOMEM);
> } else
> @@ -43,3 +43,5 @@ simple_get_netobj(const void *p, const void *end, struct xdr_netobj *dest)
> dest->len = len;
> return q;
> }
> +
> +#define simple_get_netobj(...) alloc_hooks(simple_get_netobj_noprof(__VA_ARGS__))
>
> base-commit: 3aec6b2b34e219898883d1e9ea7e911b4d3762a9
> --
> 2.44.0.683.g7961c838ac-goog
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH v11 1/3] input: pm8xxx-vibrator: refactor to support new SPMI vibrator
From: Konrad Dybcio @ 2024-04-16 13:56 UTC (permalink / raw)
To: quic_fenglinw, kernel, Andy Gross, Bjorn Andersson,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Dmitry Baryshkov
Cc: linux-arm-msm, linux-input, linux-kernel, devicetree
In-Reply-To: <20240416-pm8xxx-vibrator-new-design-v11-1-7b1c951e1515@quicinc.com>
On 4/16/24 04:44, Fenglin Wu via B4 Relay wrote:
> From: Fenglin Wu <quic_fenglinw@quicinc.com>
>
> Currently, vibrator control register addresses are hard coded,
> including the base address and offsets, it's not flexible to
> support new SPMI vibrator module which is usually included in
> different PMICs with different base address. Refactor it by using
> the base address defined in devicetree.
>
> Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Konrad
^ permalink raw reply
* Re: [PATCH v11 3/3] input: pm8xxx-vibrator: add new SPMI vibrator support
From: Konrad Dybcio @ 2024-04-16 13:59 UTC (permalink / raw)
To: quic_fenglinw, kernel, Andy Gross, Bjorn Andersson,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Dmitry Baryshkov
Cc: linux-arm-msm, linux-input, linux-kernel, devicetree
In-Reply-To: <20240416-pm8xxx-vibrator-new-design-v11-3-7b1c951e1515@quicinc.com>
On 4/16/24 04:44, Fenglin Wu via B4 Relay wrote:
> From: Fenglin Wu <quic_fenglinw@quicinc.com>
>
> Add support for a new SPMI vibrator module which is very similar
> to the vibrator module inside PM8916 but has a finer drive voltage
> step and different output voltage range, its drive level control
> is expanded across 2 registers. The vibrator module can be found
> in following Qualcomm PMICs: PMI632, PM7250B, PM7325B, PM7550BA.
>
> Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Konrad
^ permalink raw reply
* BUG: palm recognition / confidence is not working properly
From: Jon Moeller @ 2024-04-16 18:21 UTC (permalink / raw)
To: linux-input
[-- Attachment #1: Type: text/plain, Size: 3922 bytes --]
Hello -
I think I've found a bug, either with hid-multitouch, or with the
quirks for my device.
I have an HP Spectre x360, which has a nice haptic touchpad, with
excellent finger tracking; the model is ELAN07CE:00 04F3:32C8
Touchpad.
I noticed that palm rejection wasn't working, and after some
investigation found that the touchpad uses "Confidence" as a way to
indicate whether a given touch is a finger or a palm. So I disabled
MT_QUIRK_ALWAYS_VALID and enabled MT_QUIRK_VALID_IS_CONFIDENCE.
While this worked, and palm touches were now not being sent via evdev,
occasionally the cursor will "lock up" after I have been typing or
doing other things that involve resting my palms on the touchpad. I've
examined the hidraw output during these situations, and the touchpad
is sending correct data.
I believe what is happening is that the kernel is holding a slot for a
finger/palm that is no longer there, and so all subsequent touches,
even if single-finger, are interpreted as "gestures" by libinput.
I've tried setting MT_QUIRK_SLOT_IS_CONTACTID, since the touchpad
reliably maps each touch to a contactID - this doesn't help.
I've tried using MT_QUIRK_CONFIDENCE instead, which seemed promising,
but never see any ABS_MT_TOOL_TYPE event codes (neither MT_TOOL_FINGER
or MT_TOOL_PALM) reported by evtest.
So my questions are:
1) Is the MT_QUIRK_VALID_IS_CONFIDENCE messing up event/slot tracking
somehow? The "lock up" doesn't happen when this quirk is not set.
I've attached a recording of the issue in the attached yaml file from
`libinput record --with-hidraw`. It includes the initial palm touches,
followed by a one finger motion that was ignored or misinterpreted
somewhere in the stack due to the prior palm touches, followed by a
6-finger touch on the touchpad, which reliably resets the tracking
state since the touchpad only supports 5 fingers.
2) Why am I unable to get MT_TOOL_FINGER or MT_TOOL_PALM reports to
show up anywhere? It seems like there is some kind of legacy emulation
going on (converting finger tools to BTN_TOOL?), but I haven't figured
out how or why this is happening.
Happy to provide more info if necessary.
Set of quirks I'm using:
default (no issues except false touches)
MT_QUIRK_ALWAYS_VALID | MT_QUIRK_CONTACT_CNT_ACCURATE
palm-reject (rejects palm, but appears to hold slots for fingers that
aren't there any more):
MT_QUIRK_VALID_IS_CONFIDENCE | MT_QUIRK_CONTACT_CNT_ACCURATE
use-tool (attempt to get "real" tools working - currently has the same
effect as default):
MT_QUIRK_CONFIDENCE | MT_QUIRK_CONTACT_CNT_ACCURATE
Thanks,
Jon
Attachments follow:
$ evtest /dev/input.event12
Input driver version is 1.0.1
Input device ID: bus 0x18 vendor 0x4f3 product 0x32c8 version 0x100
Input device name: "ELAN07CE:00 04F3:32C8 Touchpad"
Supported events:
Event type 0 (EV_SYN)
Event type 1 (EV_KEY)
Event code 272 (BTN_LEFT)
Event code 325 (BTN_TOOL_FINGER)
Event code 328 (BTN_TOOL_QUINTTAP)
Event code 330 (BTN_TOUCH)
Event code 333 (BTN_TOOL_DOUBLETAP)
Event code 334 (BTN_TOOL_TRIPLETAP)
Event code 335 (BTN_TOOL_QUADTAP)
Event type 3 (EV_ABS)
Event code 0 (ABS_X)
Value 2470
Min 0
Max 4936
Resolution 31
Event code 1 (ABS_Y)
Value 696
Min 0
Max 3045
Resolution 31
Event code 47 (ABS_MT_SLOT)
Value 1
Min 0
Max 4
Event code 53 (ABS_MT_POSITION_X)
Value 0
Min 0
Max 4936
Resolution 31
Event code 54 (ABS_MT_POSITION_Y)
Value 0
Min 0
Max 3045
Resolution 31
Event code 57 (ABS_MT_TRACKING_ID)
Value 0
Min 0
Max 65535
Event type 4 (EV_MSC)
Event code 5 (MSC_TIMESTAMP)
Properties:
Property type 0 (INPUT_PROP_POINTER)
Property type 2 (INPUT_PROP_BUTTONPAD)
[-- Attachment #2: touchbug3.yaml --]
[-- Type: application/yaml, Size: 288274 bytes --]
^ 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