Linux Input/HID development
 help / color / mirror / Atom feed
* [PATCH v5 3/3] input: pm8xxx-vibrator: add new SPMI vibrator support
From: Fenglin Wu @ 2023-08-15  6:03 UTC (permalink / raw)
  To: linux-arm-msm, linux-kernel, krzysztof.kozlowski+dt, robh+dt,
	agross, andersson, dmitry.baryshkov, Konrad Dybcio,
	Dmitry Torokhov, linux-input
  Cc: quic_collinsd, quic_subbaram, quic_fenglinw, quic_kamalw, jestar,
	Luca Weiss
In-Reply-To: <20230815060314.352103-1-quic_fenglinw@quicinc.com>

Add new SPMI vibrator module which is very similar to the SPMI vibrator
module inside PM8916 but just has a finer drive voltage step (1mV vs
100mV) hence its drive level control is expanded to across 2 registers.
The vibrator module can be found in Qualcomm PMIC PMI632, then following
PM7250B, PM7325B, PM7550BA PMICs.

Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
Tested-by: Luca Weiss <luca.weiss@fairphone.com> # sdm632-fairphone-fp3 (pmi632)
---
 drivers/input/misc/pm8xxx-vibrator.c | 55 +++++++++++++++++++++++++---
 1 file changed, 50 insertions(+), 5 deletions(-)

diff --git a/drivers/input/misc/pm8xxx-vibrator.c b/drivers/input/misc/pm8xxx-vibrator.c
index d6b468324c77..990e8a9ac018 100644
--- a/drivers/input/misc/pm8xxx-vibrator.c
+++ b/drivers/input/misc/pm8xxx-vibrator.c
@@ -21,6 +21,13 @@
 #define SPMI_VIB_DRV_LEVEL_MASK		GENMASK(4, 0)
 #define SPMI_VIB_DRV_SHIFT		0
 
+#define SPMI_VIB_GEN2_DRV_REG		0x40
+#define SPMI_VIB_GEN2_DRV_MASK		GENMASK(7, 0)
+#define SPMI_VIB_GEN2_DRV_SHIFT		0
+#define SPMI_VIB_GEN2_DRV2_REG		0x41
+#define SPMI_VIB_GEN2_DRV2_MASK		GENMASK(3, 0)
+#define SPMI_VIB_GEN2_DRV2_SHIFT	8
+
 #define SPMI_VIB_EN_REG			0x46
 #define SPMI_VIB_EN_BIT			BIT(7)
 
@@ -33,12 +40,14 @@
 enum vib_hw_type {
 	SSBI_VIB,
 	SPMI_VIB,
+	SPMI_VIB_GEN2
 };
 
 struct pm8xxx_vib_data {
 	enum vib_hw_type	hw_type;
 	unsigned int		enable_addr;
 	unsigned int		drv_addr;
+	unsigned int		drv2_addr;
 };
 
 static const struct pm8xxx_vib_data ssbi_vib_data = {
@@ -52,6 +61,13 @@ static const struct pm8xxx_vib_data spmi_vib_data = {
 	.drv_addr	= SPMI_VIB_DRV_REG,
 };
 
+static const struct pm8xxx_vib_data spmi_vib_gen2_data = {
+	.hw_type	= SPMI_VIB_GEN2,
+	.enable_addr	= SPMI_VIB_EN_REG,
+	.drv_addr	= SPMI_VIB_GEN2_DRV_REG,
+	.drv2_addr	= SPMI_VIB_GEN2_DRV2_REG,
+};
+
 /**
  * struct pm8xxx_vib - structure to hold vibrator data
  * @vib_input_dev: input device supporting force feedback
@@ -85,12 +101,24 @@ static int pm8xxx_vib_set(struct pm8xxx_vib *vib, bool on)
 {
 	int rc;
 	unsigned int val = vib->reg_vib_drv;
-	u32 mask = SPMI_VIB_DRV_LEVEL_MASK;
-	u32 shift = SPMI_VIB_DRV_SHIFT;
+	u32 mask, shift;
 
-	if (vib->data->hw_type == SSBI_VIB) {
+
+	switch (vib->data->hw_type) {
+	case SSBI_VIB:
 		mask = SSBI_VIB_DRV_LEVEL_MASK;
 		shift = SSBI_VIB_DRV_SHIFT;
+		break;
+	case SPMI_VIB:
+		mask = SPMI_VIB_DRV_LEVEL_MASK;
+		shift = SPMI_VIB_DRV_SHIFT;
+		break;
+	case SPMI_VIB_GEN2:
+		mask = SPMI_VIB_GEN2_DRV_MASK;
+		shift = SPMI_VIB_GEN2_DRV_SHIFT;
+		break;
+	default:
+		return -EINVAL;
 	}
 
 	if (on)
@@ -104,6 +132,19 @@ static int pm8xxx_vib_set(struct pm8xxx_vib *vib, bool on)
 
 	vib->reg_vib_drv = val;
 
+	if (vib->data->hw_type == SPMI_VIB_GEN2) {
+		mask = SPMI_VIB_GEN2_DRV2_MASK;
+		shift = SPMI_VIB_GEN2_DRV2_SHIFT;
+		if (on)
+			val = (vib->level >> shift) & mask;
+		else
+			val = 0;
+		rc = regmap_update_bits(vib->regmap,
+				vib->reg_base + vib->data->drv2_addr, mask, val);
+		if (rc < 0)
+			return rc;
+	}
+
 	if (vib->data->hw_type == SSBI_VIB)
 		return 0;
 
@@ -128,10 +169,13 @@ static void pm8xxx_work_handler(struct work_struct *work)
 		vib->active = true;
 		vib->level = ((VIB_MAX_LEVELS * vib->speed) / MAX_FF_SPEED) +
 						VIB_MIN_LEVEL_mV;
-		vib->level /= 100;
+		if (vib->data->hw_type != SPMI_VIB_GEN2)
+			vib->level /= 100;
 	} else {
 		vib->active = false;
-		vib->level = VIB_MIN_LEVEL_mV / 100;
+		vib->level = VIB_MIN_LEVEL_mV;
+		if (vib->data->hw_type != SPMI_VIB_GEN2)
+			vib->level /= 100;
 	}
 
 	pm8xxx_vib_set(vib, vib->active);
@@ -266,6 +310,7 @@ static const struct of_device_id pm8xxx_vib_id_table[] = {
 	{ .compatible = "qcom,pm8058-vib", .data = &ssbi_vib_data },
 	{ .compatible = "qcom,pm8921-vib", .data = &ssbi_vib_data },
 	{ .compatible = "qcom,pm8916-vib", .data = &spmi_vib_data },
+	{ .compatible = "qcom,pmi632-vib", .data = &spmi_vib_gen2_data },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, pm8xxx_vib_id_table);
-- 
2.25.1


^ permalink raw reply related

* Re: [PATCH v1] HID: i2c-hid: use print_hex_dump_debug to print report descriptor
From: Riwen Lu @ 2023-08-15  6:02 UTC (permalink / raw)
  To: Jiri Kosina
  Cc: benjamin.tissoires, dmitry.torokhov, linux, hdegoede, rrangel,
	u.kleine-koenig, linux-input, linux-kernel, Riwen Lu
In-Reply-To: <nycvar.YFH.7.76.2308141126330.14207@cbobk.fhfr.pm>

在 2023/8/14 17:26, Jiri Kosina 写道:
> On Mon, 3 Jul 2023, Riwen Lu wrote:
> 
>> From: Riwen Lu <luriwen@kylinos.cn>
>>
>> The format '%*ph' print up to 64 bytes long as a hex string with ' '
>> sepatator. Usually the size of report descriptor is larger than 64
>> bytes, so consider using print_hex_dump_debug to print out all of it for
>> better debugging.
>>
>> Signed-off-by: Riwen Lu <luriwen@kylinos.cn>
>> ---
>>   drivers/hid/i2c-hid/i2c-hid-core.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
>> index efbba0465eef..8e97fc01c852 100644
>> --- a/drivers/hid/i2c-hid/i2c-hid-core.c
>> +++ b/drivers/hid/i2c-hid/i2c-hid-core.c
>> @@ -772,7 +772,9 @@ static int i2c_hid_parse(struct hid_device *hid)
>>   		}
>>   	}
>>   
>> -	i2c_hid_dbg(ihid, "Report Descriptor: %*ph\n", rsize, rdesc);
>> +	i2c_hid_dbg(ihid, "Report Descriptor\n");
>> +	print_hex_dump_debug("  ", DUMP_PREFIX_OFFSET, 16, 1,
>> +			rdesc, rsize, false);
> 
> But that would dump it unconditionally, while i2c_hid_dbg() is
> conditional.
> 
Function print_hex_dump_debug() dump messages is as conditional as 
i2c_hid_dbg().

The function i2c_hid_dbg() defines as follows:
#define i2c_hid_dbg(ihid, ...) dev_dbg(&(ihid)->client->dev, __VA_ARGS__)

dev_dbg() depends on the same macro as print_hex_dump_debug().


^ permalink raw reply

* [PATCH v5 2/3] dt-bindings: input: qcom,pm8xxx-vib: add new SPMI vibrator module
From: Fenglin Wu @ 2023-08-15  6:03 UTC (permalink / raw)
  To: linux-arm-msm, linux-kernel, krzysztof.kozlowski+dt, robh+dt,
	agross, andersson, dmitry.baryshkov, Konrad Dybcio,
	Dmitry Torokhov, linux-input, devicetree
  Cc: quic_collinsd, quic_subbaram, quic_fenglinw, quic_kamalw, jestar
In-Reply-To: <20230815060314.352103-1-quic_fenglinw@quicinc.com>

Add compatible strings to support vibrator module inside PMI632,
PMI7250B, PM7325B, PM7550BA.

Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
---
 .../bindings/input/qcom,pm8xxx-vib.yaml           | 15 +++++++++++----
 1 file changed, 11 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..72b72c67a9b6 100644
--- a/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml
+++ b/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml
@@ -11,10 +11,17 @@ maintainers:
 
 properties:
   compatible:
-    enum:
-      - qcom,pm8058-vib
-      - qcom,pm8916-vib
-      - qcom,pm8921-vib
+    oneOf:
+      - enum:
+          - qcom,pm8058-vib
+          - qcom,pm8916-vib
+          - qcom,pm8921-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 v5 1/3] input: pm8xxx-vib: refactor to easily support new SPMI vibrator
From: Fenglin Wu @ 2023-08-15  6:03 UTC (permalink / raw)
  To: linux-arm-msm, linux-kernel, krzysztof.kozlowski+dt, robh+dt,
	agross, andersson, dmitry.baryshkov, Konrad Dybcio,
	Dmitry Torokhov, linux-input
  Cc: quic_collinsd, quic_subbaram, quic_fenglinw, quic_kamalw, jestar
In-Reply-To: <20230815060314.352103-1-quic_fenglinw@quicinc.com>

Currently, all vibrator control register addresses are hard coded,
including the base address and the offset, it's not flexible to support
new SPMI vibrator module which is usually included in different PMICs
with different base address. Refactor this by defining register offset
with HW type combination, and register base address which is defined
in 'reg' property is added for SPMI vibrators.

Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
---
 drivers/input/misc/pm8xxx-vibrator.c | 122 ++++++++++++++++-----------
 1 file changed, 73 insertions(+), 49 deletions(-)

diff --git a/drivers/input/misc/pm8xxx-vibrator.c b/drivers/input/misc/pm8xxx-vibrator.c
index 04cb87efd799..d6b468324c77 100644
--- a/drivers/input/misc/pm8xxx-vibrator.c
+++ b/drivers/input/misc/pm8xxx-vibrator.c
@@ -12,36 +12,44 @@
 #include <linux/regmap.h>
 #include <linux/slab.h>
 
+#define SSBL_VIB_DRV_REG		0x4A
+#define SSBI_VIB_DRV_EN_MANUAL_MASK	GENMASK(7, 2)
+#define SSBI_VIB_DRV_LEVEL_MASK		GENMASK(7, 3)
+#define SSBI_VIB_DRV_SHIFT		3
+
+#define SPMI_VIB_DRV_REG		0x41
+#define SPMI_VIB_DRV_LEVEL_MASK		GENMASK(4, 0)
+#define SPMI_VIB_DRV_SHIFT		0
+
+#define SPMI_VIB_EN_REG			0x46
+#define SPMI_VIB_EN_BIT			BIT(7)
+
 #define VIB_MAX_LEVEL_mV	(3100)
 #define VIB_MIN_LEVEL_mV	(1200)
 #define VIB_MAX_LEVELS		(VIB_MAX_LEVEL_mV - VIB_MIN_LEVEL_mV)
 
 #define MAX_FF_SPEED		0xff
 
-struct pm8xxx_regs {
-	unsigned int enable_addr;
-	unsigned int enable_mask;
+enum vib_hw_type {
+	SSBI_VIB,
+	SPMI_VIB,
+};
 
-	unsigned int drv_addr;
-	unsigned int drv_mask;
-	unsigned int drv_shift;
-	unsigned int drv_en_manual_mask;
+struct pm8xxx_vib_data {
+	enum vib_hw_type	hw_type;
+	unsigned int		enable_addr;
+	unsigned int		drv_addr;
 };
 
-static const struct pm8xxx_regs pm8058_regs = {
-	.drv_addr = 0x4A,
-	.drv_mask = 0xf8,
-	.drv_shift = 3,
-	.drv_en_manual_mask = 0xfc,
+static const struct pm8xxx_vib_data ssbi_vib_data = {
+	.hw_type	= SSBI_VIB,
+	.drv_addr	= SSBL_VIB_DRV_REG,
 };
 
-static struct pm8xxx_regs pm8916_regs = {
-	.enable_addr = 0xc046,
-	.enable_mask = BIT(7),
-	.drv_addr = 0xc041,
-	.drv_mask = 0x1F,
-	.drv_shift = 0,
-	.drv_en_manual_mask = 0,
+static const struct pm8xxx_vib_data spmi_vib_data = {
+	.hw_type	= SPMI_VIB,
+	.enable_addr	= SPMI_VIB_EN_REG,
+	.drv_addr	= SPMI_VIB_DRV_REG,
 };
 
 /**
@@ -49,7 +57,8 @@ static struct pm8xxx_regs pm8916_regs = {
  * @vib_input_dev: input device supporting force feedback
  * @work: work structure to set the vibration parameters
  * @regmap: regmap for register read/write
- * @regs: registers' info
+ * @data: vibrator HW info
+ * @reg_base: the register base of the module
  * @speed: speed of vibration set from userland
  * @active: state of vibrator
  * @level: level of vibration to set in the chip
@@ -59,7 +68,8 @@ struct pm8xxx_vib {
 	struct input_dev *vib_input_dev;
 	struct work_struct work;
 	struct regmap *regmap;
-	const struct pm8xxx_regs *regs;
+	const struct pm8xxx_vib_data *data;
+	unsigned int reg_base;
 	int speed;
 	int level;
 	bool active;
@@ -75,24 +85,31 @@ static int pm8xxx_vib_set(struct pm8xxx_vib *vib, bool on)
 {
 	int rc;
 	unsigned int val = vib->reg_vib_drv;
-	const struct pm8xxx_regs *regs = vib->regs;
+	u32 mask = SPMI_VIB_DRV_LEVEL_MASK;
+	u32 shift = SPMI_VIB_DRV_SHIFT;
+
+	if (vib->data->hw_type == SSBI_VIB) {
+		mask = SSBI_VIB_DRV_LEVEL_MASK;
+		shift = SSBI_VIB_DRV_SHIFT;
+	}
 
 	if (on)
-		val |= (vib->level << regs->drv_shift) & regs->drv_mask;
+		val |= (vib->level << shift) & mask;
 	else
-		val &= ~regs->drv_mask;
+		val &= ~mask;
 
-	rc = regmap_write(vib->regmap, regs->drv_addr, val);
+	rc = regmap_update_bits(vib->regmap, vib->reg_base + vib->data->drv_addr, mask, 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);
+	if (vib->data->hw_type == SSBI_VIB)
+		return 0;
 
-	return rc;
+	mask = SPMI_VIB_EN_BIT;
+	val = on ? SPMI_VIB_EN_BIT : 0;
+	return regmap_update_bits(vib->regmap, vib->reg_base + vib->data->enable_addr, mask, val);
 }
 
 /**
@@ -102,13 +119,6 @@ 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;
-
-	rc = regmap_read(vib->regmap, regs->drv_addr, &val);
-	if (rc < 0)
-		return;
 
 	/*
 	 * pmic vibrator supports voltage ranges from 1.2 to 3.1V, so
@@ -168,9 +178,9 @@ static int pm8xxx_vib_probe(struct platform_device *pdev)
 {
 	struct pm8xxx_vib *vib;
 	struct input_dev *input_dev;
+	const struct pm8xxx_vib_data *data;
 	int error;
-	unsigned int val;
-	const struct pm8xxx_regs *regs;
+	unsigned int val, reg_base;
 
 	vib = devm_kzalloc(&pdev->dev, sizeof(*vib), GFP_KERNEL);
 	if (!vib)
@@ -187,19 +197,33 @@ static int pm8xxx_vib_probe(struct platform_device *pdev)
 	INIT_WORK(&vib->work, pm8xxx_work_handler);
 	vib->vib_input_dev = input_dev;
 
-	regs = of_device_get_match_data(&pdev->dev);
+	data = of_device_get_match_data(&pdev->dev);
+	if (!data)
+		return -EINVAL;
 
-	/* operate in manual mode */
-	error = regmap_read(vib->regmap, regs->drv_addr, &val);
-	if (error < 0)
-		return error;
+	if (data->hw_type != SSBI_VIB) {
+		error = fwnode_property_read_u32(pdev->dev.fwnode, "reg", &reg_base);
+		if (error < 0) {
+			dev_err(&pdev->dev, "Failed to read reg address, rc=%d\n", error);
+			return error;
+		}
+
+		vib->reg_base += reg_base;
+	}
 
-	val &= regs->drv_en_manual_mask;
-	error = regmap_write(vib->regmap, regs->drv_addr, val);
+	error = regmap_read(vib->regmap, vib->reg_base + data->drv_addr, &val);
 	if (error < 0)
 		return error;
 
-	vib->regs = regs;
+	/* operate in manual mode */
+	if (data->hw_type == SSBI_VIB) {
+		val &= SSBI_VIB_DRV_EN_MANUAL_MASK;
+		error = regmap_write(vib->regmap, vib->reg_base + data->drv_addr, val);
+		if (error < 0)
+			return error;
+	}
+
+	vib->data = data;
 	vib->reg_vib_drv = val;
 
 	input_dev->name = "pm8xxx_vib_ffmemless";
@@ -239,9 +263,9 @@ static int pm8xxx_vib_suspend(struct device *dev)
 static DEFINE_SIMPLE_DEV_PM_OPS(pm8xxx_vib_pm_ops, pm8xxx_vib_suspend, NULL);
 
 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,pm8058-vib", .data = &ssbi_vib_data },
+	{ .compatible = "qcom,pm8921-vib", .data = &ssbi_vib_data },
+	{ .compatible = "qcom,pm8916-vib", .data = &spmi_vib_data },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, pm8xxx_vib_id_table);
-- 
2.25.1


^ permalink raw reply related

* Re: [PATCH v5 2/3] dt-bindings: input: qcom,pm8xxx-vib: add new SPMI vibrator module
From: Luca Weiss @ 2023-08-15  6:35 UTC (permalink / raw)
  To: Fenglin Wu, linux-arm-msm, linux-kernel, krzysztof.kozlowski+dt,
	robh+dt, agross, andersson, dmitry.baryshkov, Konrad Dybcio,
	Dmitry Torokhov, linux-input, devicetree
  Cc: quic_collinsd, quic_subbaram, quic_kamalw, jestar
In-Reply-To: <20230815060314.352103-3-quic_fenglinw@quicinc.com>

Hi Fenglin,

On Tue Aug 15, 2023 at 8:03 AM CEST, Fenglin Wu wrote:
> Add compatible strings to support vibrator module inside PMI632,
> PMI7250B, PM7325B, PM7550BA.
>
> Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
> ---
>  .../bindings/input/qcom,pm8xxx-vib.yaml           | 15 +++++++++++----
>  1 file changed, 11 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..72b72c67a9b6 100644
> --- a/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml
> +++ b/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml
> @@ -11,10 +11,17 @@ maintainers:
>  
>  properties:
>    compatible:
> -    enum:
> -      - qcom,pm8058-vib
> -      - qcom,pm8916-vib
> -      - qcom,pm8921-vib
> +    oneOf:
> +      - enum:
> +          - qcom,pm8058-vib
> +          - qcom,pm8916-vib
> +          - qcom,pm8921-vib
> +      - items:
> +          - enum:
> +              - qcom,pm7250b-vib
> +              - qcom,pm7325b-vib
> +              - qcom,pm7550ba-vib
> +          - const: qcom,pmi632-vib

With the new revision the standalone 'compatible = "qcom,pmi632-vib";'
doesn't pass validation anymore.

foo.dtb: vibrator@5700: compatible: 'oneOf' conditional failed, one must be fixed:
        ['qcom,pmi632-vib'] is too short
        'qcom,pmi632-vib' is not one of ['qcom,pm8058-vib', 'qcom,pm8916-vib', 'qcom,pm8921-vib']
        'qcom,pmi632-vib' is not one of ['qcom,pm7250b-vib', 'qcom,pm7325b-vib', 'qcom,pm7550ba-vib']
        from schema $id: http://devicetree.org/schemas/input/qcom,pm8xxx-vib.yaml#

I believe you need to add the compatible also like this:

diff --git a/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml b/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml
index 72b72c67a9b6..2025d6a5423e 100644
--- a/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml
+++ b/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml
@@ -16,6 +16,7 @@ properties:
           - qcom,pm8058-vib
           - qcom,pm8916-vib
           - qcom,pm8921-vib
+          - qcom,pmi632-vib
       - items:
           - enum:
               - qcom,pm7250b-vib


Regards
Luca

>  
>    reg:
>      maxItems: 1


^ permalink raw reply related

* Re: [PATCH v1] HID: i2c-hid: use print_hex_dump_debug to print report descriptor
From: Rahul Rameshbabu @ 2023-08-15  6:35 UTC (permalink / raw)
  To: Riwen Lu
  Cc: Jiri Kosina, benjamin.tissoires, dmitry.torokhov, linux, hdegoede,
	rrangel, u.kleine-koenig, linux-input, linux-kernel, Riwen Lu
In-Reply-To: <TYCP286MB260715E63D023C52591264C5B114A@TYCP286MB2607.JPNP286.PROD.OUTLOOK.COM>


On Tue, 15 Aug, 2023 14:02:40 +0800 "Riwen Lu" <luriwen@hotmail.com> wrote:
> 在 2023/8/14 17:26, Jiri Kosina 写道:
>> On Mon, 3 Jul 2023, Riwen Lu wrote:
>>
>>> From: Riwen Lu <luriwen@kylinos.cn>
>>>
>>> The format '%*ph' print up to 64 bytes long as a hex string with ' '
>>> sepatator. Usually the size of report descriptor is larger than 64
>>> bytes, so consider using print_hex_dump_debug to print out all of it for
>>> better debugging.
>>>
>>> Signed-off-by: Riwen Lu <luriwen@kylinos.cn>
>>> ---
>>>   drivers/hid/i2c-hid/i2c-hid-core.c | 4 +++-
>>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
>>> index efbba0465eef..8e97fc01c852 100644
>>> --- a/drivers/hid/i2c-hid/i2c-hid-core.c
>>> +++ b/drivers/hid/i2c-hid/i2c-hid-core.c
>>> @@ -772,7 +772,9 @@ static int i2c_hid_parse(struct hid_device *hid)
>>>   		}
>>>   	}
>>>
>>> -	i2c_hid_dbg(ihid, "Report Descriptor: %*ph\n", rsize, rdesc);
>>> +	i2c_hid_dbg(ihid, "Report Descriptor\n");
>>> +	print_hex_dump_debug("  ", DUMP_PREFIX_OFFSET, 16, 1,
>>> +			rdesc, rsize, false);

Maybe it makes sense to use a prefix for the hex dump that is easy to
trace rather than padding whitespace? This looks good when you do not
see any other kernel message log lines get interlaced when written.
However, if you have a lot of concurrent kernel message output, I think
it can be tough to piece together the lines of the dump with this
prefix. Just my opinion.

>>
>> But that would dump it unconditionally, while i2c_hid_dbg() is
>> conditional.
>>
> Function print_hex_dump_debug() dump messages is as conditional as
> i2c_hid_dbg().
>
> The function i2c_hid_dbg() defines as follows:
> #define i2c_hid_dbg(ihid, ...) dev_dbg(&(ihid)->client->dev, __VA_ARGS__)
>
> dev_dbg() depends on the same macro as print_hex_dump_debug().

I agree with this point. Both dev_dbg and print_hex_dump_debug are noops
if neither CONFIG_DYNAMIC_DEBUG is set or DEBUG is defined.

--
Thanks,

Rahul Rameshbabu


^ permalink raw reply

* Re: [PATCH v5 2/3] dt-bindings: input: qcom,pm8xxx-vib: add new SPMI vibrator module
From: Fenglin Wu @ 2023-08-15  6:42 UTC (permalink / raw)
  To: Luca Weiss, linux-arm-msm, linux-kernel, krzysztof.kozlowski+dt,
	robh+dt, agross, andersson, dmitry.baryshkov, Konrad Dybcio,
	Dmitry Torokhov, linux-input, devicetree
  Cc: quic_collinsd, quic_subbaram, quic_kamalw, jestar
In-Reply-To: <CUSWRRL6QOPU.1YM7S0F8F3V2D@otso>



On 8/15/2023 2:35 PM, Luca Weiss wrote:
> Hi Fenglin,
> 
> On Tue Aug 15, 2023 at 8:03 AM CEST, Fenglin Wu wrote:
>> Add compatible strings to support vibrator module inside PMI632,
>> PMI7250B, PM7325B, PM7550BA.
>>
>> Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
>> ---
>>   .../bindings/input/qcom,pm8xxx-vib.yaml           | 15 +++++++++++----
>>   1 file changed, 11 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..72b72c67a9b6 100644
>> --- a/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml
>> +++ b/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml
>> @@ -11,10 +11,17 @@ maintainers:
>>   
>>   properties:
>>     compatible:
>> -    enum:
>> -      - qcom,pm8058-vib
>> -      - qcom,pm8916-vib
>> -      - qcom,pm8921-vib
>> +    oneOf:
>> +      - enum:
>> +          - qcom,pm8058-vib
>> +          - qcom,pm8916-vib
>> +          - qcom,pm8921-vib
>> +      - items:
>> +          - enum:
>> +              - qcom,pm7250b-vib
>> +              - qcom,pm7325b-vib
>> +              - qcom,pm7550ba-vib
>> +          - const: qcom,pmi632-vib
> 
> With the new revision the standalone 'compatible = "qcom,pmi632-vib";'
> doesn't pass validation anymore.
> 
> foo.dtb: vibrator@5700: compatible: 'oneOf' conditional failed, one must be fixed:
>          ['qcom,pmi632-vib'] is too short
>          'qcom,pmi632-vib' is not one of ['qcom,pm8058-vib', 'qcom,pm8916-vib', 'qcom,pm8921-vib']
>          'qcom,pmi632-vib' is not one of ['qcom,pm7250b-vib', 'qcom,pm7325b-vib', 'qcom,pm7550ba-vib']
>          from schema $id: http://devicetree.org/schemas/input/qcom,pm8xxx-vib.yaml#
> 
> I believe you need to add the compatible also like this:
> 
> diff --git a/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml b/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml
> index 72b72c67a9b6..2025d6a5423e 100644
> --- a/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml
> +++ b/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml
> @@ -16,6 +16,7 @@ properties:
>             - qcom,pm8058-vib
>             - qcom,pm8916-vib
>             - qcom,pm8921-vib
> +          - qcom,pmi632-vib
>         - items:
>             - enum:
>                 - qcom,pm7250b-vib
> 
Yeah, thanks for catching this. I will update it soon.

> 
> Regards
> Luca
> 
>>   
>>     reg:
>>       maxItems: 1
> 

^ permalink raw reply

* [PATCH v6 1/3] input: pm8xxx-vib: refactor to easily support new SPMI vibrator
From: Fenglin Wu @ 2023-08-15  6:49 UTC (permalink / raw)
  To: linux-arm-msm, linux-kernel, krzysztof.kozlowski+dt, robh+dt,
	agross, andersson, dmitry.baryshkov, Konrad Dybcio,
	Dmitry Torokhov, linux-input
  Cc: quic_collinsd, quic_subbaram, quic_fenglinw, quic_kamalw, jestar
In-Reply-To: <20230815064917.387235-1-quic_fenglinw@quicinc.com>

Currently, all vibrator control register addresses are hard coded,
including the base address and the offset, it's not flexible to support
new SPMI vibrator module which is usually included in different PMICs
with different base address. Refactor this by defining register offset
with HW type combination, and register base address which is defined
in 'reg' property is added for SPMI vibrators.

Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
---
 drivers/input/misc/pm8xxx-vibrator.c | 122 ++++++++++++++++-----------
 1 file changed, 73 insertions(+), 49 deletions(-)

diff --git a/drivers/input/misc/pm8xxx-vibrator.c b/drivers/input/misc/pm8xxx-vibrator.c
index 04cb87efd799..d6b468324c77 100644
--- a/drivers/input/misc/pm8xxx-vibrator.c
+++ b/drivers/input/misc/pm8xxx-vibrator.c
@@ -12,36 +12,44 @@
 #include <linux/regmap.h>
 #include <linux/slab.h>
 
+#define SSBL_VIB_DRV_REG		0x4A
+#define SSBI_VIB_DRV_EN_MANUAL_MASK	GENMASK(7, 2)
+#define SSBI_VIB_DRV_LEVEL_MASK		GENMASK(7, 3)
+#define SSBI_VIB_DRV_SHIFT		3
+
+#define SPMI_VIB_DRV_REG		0x41
+#define SPMI_VIB_DRV_LEVEL_MASK		GENMASK(4, 0)
+#define SPMI_VIB_DRV_SHIFT		0
+
+#define SPMI_VIB_EN_REG			0x46
+#define SPMI_VIB_EN_BIT			BIT(7)
+
 #define VIB_MAX_LEVEL_mV	(3100)
 #define VIB_MIN_LEVEL_mV	(1200)
 #define VIB_MAX_LEVELS		(VIB_MAX_LEVEL_mV - VIB_MIN_LEVEL_mV)
 
 #define MAX_FF_SPEED		0xff
 
-struct pm8xxx_regs {
-	unsigned int enable_addr;
-	unsigned int enable_mask;
+enum vib_hw_type {
+	SSBI_VIB,
+	SPMI_VIB,
+};
 
-	unsigned int drv_addr;
-	unsigned int drv_mask;
-	unsigned int drv_shift;
-	unsigned int drv_en_manual_mask;
+struct pm8xxx_vib_data {
+	enum vib_hw_type	hw_type;
+	unsigned int		enable_addr;
+	unsigned int		drv_addr;
 };
 
-static const struct pm8xxx_regs pm8058_regs = {
-	.drv_addr = 0x4A,
-	.drv_mask = 0xf8,
-	.drv_shift = 3,
-	.drv_en_manual_mask = 0xfc,
+static const struct pm8xxx_vib_data ssbi_vib_data = {
+	.hw_type	= SSBI_VIB,
+	.drv_addr	= SSBL_VIB_DRV_REG,
 };
 
-static struct pm8xxx_regs pm8916_regs = {
-	.enable_addr = 0xc046,
-	.enable_mask = BIT(7),
-	.drv_addr = 0xc041,
-	.drv_mask = 0x1F,
-	.drv_shift = 0,
-	.drv_en_manual_mask = 0,
+static const struct pm8xxx_vib_data spmi_vib_data = {
+	.hw_type	= SPMI_VIB,
+	.enable_addr	= SPMI_VIB_EN_REG,
+	.drv_addr	= SPMI_VIB_DRV_REG,
 };
 
 /**
@@ -49,7 +57,8 @@ static struct pm8xxx_regs pm8916_regs = {
  * @vib_input_dev: input device supporting force feedback
  * @work: work structure to set the vibration parameters
  * @regmap: regmap for register read/write
- * @regs: registers' info
+ * @data: vibrator HW info
+ * @reg_base: the register base of the module
  * @speed: speed of vibration set from userland
  * @active: state of vibrator
  * @level: level of vibration to set in the chip
@@ -59,7 +68,8 @@ struct pm8xxx_vib {
 	struct input_dev *vib_input_dev;
 	struct work_struct work;
 	struct regmap *regmap;
-	const struct pm8xxx_regs *regs;
+	const struct pm8xxx_vib_data *data;
+	unsigned int reg_base;
 	int speed;
 	int level;
 	bool active;
@@ -75,24 +85,31 @@ static int pm8xxx_vib_set(struct pm8xxx_vib *vib, bool on)
 {
 	int rc;
 	unsigned int val = vib->reg_vib_drv;
-	const struct pm8xxx_regs *regs = vib->regs;
+	u32 mask = SPMI_VIB_DRV_LEVEL_MASK;
+	u32 shift = SPMI_VIB_DRV_SHIFT;
+
+	if (vib->data->hw_type == SSBI_VIB) {
+		mask = SSBI_VIB_DRV_LEVEL_MASK;
+		shift = SSBI_VIB_DRV_SHIFT;
+	}
 
 	if (on)
-		val |= (vib->level << regs->drv_shift) & regs->drv_mask;
+		val |= (vib->level << shift) & mask;
 	else
-		val &= ~regs->drv_mask;
+		val &= ~mask;
 
-	rc = regmap_write(vib->regmap, regs->drv_addr, val);
+	rc = regmap_update_bits(vib->regmap, vib->reg_base + vib->data->drv_addr, mask, 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);
+	if (vib->data->hw_type == SSBI_VIB)
+		return 0;
 
-	return rc;
+	mask = SPMI_VIB_EN_BIT;
+	val = on ? SPMI_VIB_EN_BIT : 0;
+	return regmap_update_bits(vib->regmap, vib->reg_base + vib->data->enable_addr, mask, val);
 }
 
 /**
@@ -102,13 +119,6 @@ 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;
-
-	rc = regmap_read(vib->regmap, regs->drv_addr, &val);
-	if (rc < 0)
-		return;
 
 	/*
 	 * pmic vibrator supports voltage ranges from 1.2 to 3.1V, so
@@ -168,9 +178,9 @@ static int pm8xxx_vib_probe(struct platform_device *pdev)
 {
 	struct pm8xxx_vib *vib;
 	struct input_dev *input_dev;
+	const struct pm8xxx_vib_data *data;
 	int error;
-	unsigned int val;
-	const struct pm8xxx_regs *regs;
+	unsigned int val, reg_base;
 
 	vib = devm_kzalloc(&pdev->dev, sizeof(*vib), GFP_KERNEL);
 	if (!vib)
@@ -187,19 +197,33 @@ static int pm8xxx_vib_probe(struct platform_device *pdev)
 	INIT_WORK(&vib->work, pm8xxx_work_handler);
 	vib->vib_input_dev = input_dev;
 
-	regs = of_device_get_match_data(&pdev->dev);
+	data = of_device_get_match_data(&pdev->dev);
+	if (!data)
+		return -EINVAL;
 
-	/* operate in manual mode */
-	error = regmap_read(vib->regmap, regs->drv_addr, &val);
-	if (error < 0)
-		return error;
+	if (data->hw_type != SSBI_VIB) {
+		error = fwnode_property_read_u32(pdev->dev.fwnode, "reg", &reg_base);
+		if (error < 0) {
+			dev_err(&pdev->dev, "Failed to read reg address, rc=%d\n", error);
+			return error;
+		}
+
+		vib->reg_base += reg_base;
+	}
 
-	val &= regs->drv_en_manual_mask;
-	error = regmap_write(vib->regmap, regs->drv_addr, val);
+	error = regmap_read(vib->regmap, vib->reg_base + data->drv_addr, &val);
 	if (error < 0)
 		return error;
 
-	vib->regs = regs;
+	/* operate in manual mode */
+	if (data->hw_type == SSBI_VIB) {
+		val &= SSBI_VIB_DRV_EN_MANUAL_MASK;
+		error = regmap_write(vib->regmap, vib->reg_base + data->drv_addr, val);
+		if (error < 0)
+			return error;
+	}
+
+	vib->data = data;
 	vib->reg_vib_drv = val;
 
 	input_dev->name = "pm8xxx_vib_ffmemless";
@@ -239,9 +263,9 @@ static int pm8xxx_vib_suspend(struct device *dev)
 static DEFINE_SIMPLE_DEV_PM_OPS(pm8xxx_vib_pm_ops, pm8xxx_vib_suspend, NULL);
 
 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,pm8058-vib", .data = &ssbi_vib_data },
+	{ .compatible = "qcom,pm8921-vib", .data = &ssbi_vib_data },
+	{ .compatible = "qcom,pm8916-vib", .data = &spmi_vib_data },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, pm8xxx_vib_id_table);
-- 
2.25.1


^ permalink raw reply related

* [PATCH v6 2/3] dt-bindings: input: qcom,pm8xxx-vib: add new SPMI vibrator module
From: Fenglin Wu @ 2023-08-15  6:49 UTC (permalink / raw)
  To: linux-arm-msm, linux-kernel, krzysztof.kozlowski+dt, robh+dt,
	agross, andersson, dmitry.baryshkov, Konrad Dybcio,
	Dmitry Torokhov, linux-input, devicetree
  Cc: quic_collinsd, quic_subbaram, quic_fenglinw, quic_kamalw, jestar
In-Reply-To: <20230815064917.387235-1-quic_fenglinw@quicinc.com>

Add compatible strings to support vibrator module inside PMI632,
PMI7250B, PM7325B, PM7550BA.

Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
---
 .../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 v6 3/3] input: pm8xxx-vibrator: add new SPMI vibrator support
From: Fenglin Wu @ 2023-08-15  6:49 UTC (permalink / raw)
  To: linux-arm-msm, linux-kernel, krzysztof.kozlowski+dt, robh+dt,
	agross, andersson, dmitry.baryshkov, Konrad Dybcio,
	Dmitry Torokhov, linux-input
  Cc: quic_collinsd, quic_subbaram, quic_fenglinw, quic_kamalw, jestar,
	Luca Weiss
In-Reply-To: <20230815064917.387235-1-quic_fenglinw@quicinc.com>

Add new SPMI vibrator module which is very similar to the SPMI vibrator
module inside PM8916 but just has a finer drive voltage step (1mV vs
100mV) hence its drive level control is expanded to across 2 registers.
The vibrator module can be found in Qualcomm PMIC PMI632, then following
PM7250B, PM7325B, PM7550BA PMICs.

Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
Tested-by: Luca Weiss <luca.weiss@fairphone.com> # sdm632-fairphone-fp3 (pmi632)
---
 drivers/input/misc/pm8xxx-vibrator.c | 55 +++++++++++++++++++++++++---
 1 file changed, 50 insertions(+), 5 deletions(-)

diff --git a/drivers/input/misc/pm8xxx-vibrator.c b/drivers/input/misc/pm8xxx-vibrator.c
index d6b468324c77..990e8a9ac018 100644
--- a/drivers/input/misc/pm8xxx-vibrator.c
+++ b/drivers/input/misc/pm8xxx-vibrator.c
@@ -21,6 +21,13 @@
 #define SPMI_VIB_DRV_LEVEL_MASK		GENMASK(4, 0)
 #define SPMI_VIB_DRV_SHIFT		0
 
+#define SPMI_VIB_GEN2_DRV_REG		0x40
+#define SPMI_VIB_GEN2_DRV_MASK		GENMASK(7, 0)
+#define SPMI_VIB_GEN2_DRV_SHIFT		0
+#define SPMI_VIB_GEN2_DRV2_REG		0x41
+#define SPMI_VIB_GEN2_DRV2_MASK		GENMASK(3, 0)
+#define SPMI_VIB_GEN2_DRV2_SHIFT	8
+
 #define SPMI_VIB_EN_REG			0x46
 #define SPMI_VIB_EN_BIT			BIT(7)
 
@@ -33,12 +40,14 @@
 enum vib_hw_type {
 	SSBI_VIB,
 	SPMI_VIB,
+	SPMI_VIB_GEN2
 };
 
 struct pm8xxx_vib_data {
 	enum vib_hw_type	hw_type;
 	unsigned int		enable_addr;
 	unsigned int		drv_addr;
+	unsigned int		drv2_addr;
 };
 
 static const struct pm8xxx_vib_data ssbi_vib_data = {
@@ -52,6 +61,13 @@ static const struct pm8xxx_vib_data spmi_vib_data = {
 	.drv_addr	= SPMI_VIB_DRV_REG,
 };
 
+static const struct pm8xxx_vib_data spmi_vib_gen2_data = {
+	.hw_type	= SPMI_VIB_GEN2,
+	.enable_addr	= SPMI_VIB_EN_REG,
+	.drv_addr	= SPMI_VIB_GEN2_DRV_REG,
+	.drv2_addr	= SPMI_VIB_GEN2_DRV2_REG,
+};
+
 /**
  * struct pm8xxx_vib - structure to hold vibrator data
  * @vib_input_dev: input device supporting force feedback
@@ -85,12 +101,24 @@ static int pm8xxx_vib_set(struct pm8xxx_vib *vib, bool on)
 {
 	int rc;
 	unsigned int val = vib->reg_vib_drv;
-	u32 mask = SPMI_VIB_DRV_LEVEL_MASK;
-	u32 shift = SPMI_VIB_DRV_SHIFT;
+	u32 mask, shift;
 
-	if (vib->data->hw_type == SSBI_VIB) {
+
+	switch (vib->data->hw_type) {
+	case SSBI_VIB:
 		mask = SSBI_VIB_DRV_LEVEL_MASK;
 		shift = SSBI_VIB_DRV_SHIFT;
+		break;
+	case SPMI_VIB:
+		mask = SPMI_VIB_DRV_LEVEL_MASK;
+		shift = SPMI_VIB_DRV_SHIFT;
+		break;
+	case SPMI_VIB_GEN2:
+		mask = SPMI_VIB_GEN2_DRV_MASK;
+		shift = SPMI_VIB_GEN2_DRV_SHIFT;
+		break;
+	default:
+		return -EINVAL;
 	}
 
 	if (on)
@@ -104,6 +132,19 @@ static int pm8xxx_vib_set(struct pm8xxx_vib *vib, bool on)
 
 	vib->reg_vib_drv = val;
 
+	if (vib->data->hw_type == SPMI_VIB_GEN2) {
+		mask = SPMI_VIB_GEN2_DRV2_MASK;
+		shift = SPMI_VIB_GEN2_DRV2_SHIFT;
+		if (on)
+			val = (vib->level >> shift) & mask;
+		else
+			val = 0;
+		rc = regmap_update_bits(vib->regmap,
+				vib->reg_base + vib->data->drv2_addr, mask, val);
+		if (rc < 0)
+			return rc;
+	}
+
 	if (vib->data->hw_type == SSBI_VIB)
 		return 0;
 
@@ -128,10 +169,13 @@ static void pm8xxx_work_handler(struct work_struct *work)
 		vib->active = true;
 		vib->level = ((VIB_MAX_LEVELS * vib->speed) / MAX_FF_SPEED) +
 						VIB_MIN_LEVEL_mV;
-		vib->level /= 100;
+		if (vib->data->hw_type != SPMI_VIB_GEN2)
+			vib->level /= 100;
 	} else {
 		vib->active = false;
-		vib->level = VIB_MIN_LEVEL_mV / 100;
+		vib->level = VIB_MIN_LEVEL_mV;
+		if (vib->data->hw_type != SPMI_VIB_GEN2)
+			vib->level /= 100;
 	}
 
 	pm8xxx_vib_set(vib, vib->active);
@@ -266,6 +310,7 @@ static const struct of_device_id pm8xxx_vib_id_table[] = {
 	{ .compatible = "qcom,pm8058-vib", .data = &ssbi_vib_data },
 	{ .compatible = "qcom,pm8921-vib", .data = &ssbi_vib_data },
 	{ .compatible = "qcom,pm8916-vib", .data = &spmi_vib_data },
+	{ .compatible = "qcom,pmi632-vib", .data = &spmi_vib_gen2_data },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, pm8xxx_vib_id_table);
-- 
2.25.1


^ permalink raw reply related

* Re: [PATCH v1] HID: i2c-hid: use print_hex_dump_debug to print report descriptor
From: Riwen Lu @ 2023-08-15  6:58 UTC (permalink / raw)
  To: Rahul Rameshbabu
  Cc: Jiri Kosina, benjamin.tissoires, dmitry.torokhov, linux, hdegoede,
	rrangel, u.kleine-koenig, linux-input, linux-kernel, Riwen Lu
In-Reply-To: <87fs4kn77j.fsf@protonmail.com>

在 2023/8/15 14:35, Rahul Rameshbabu 写道:
> 
> On Tue, 15 Aug, 2023 14:02:40 +0800 "Riwen Lu" <luriwen@hotmail.com> wrote:
>> 在 2023/8/14 17:26, Jiri Kosina 写道:
>>> On Mon, 3 Jul 2023, Riwen Lu wrote:
>>>
>>>> From: Riwen Lu <luriwen@kylinos.cn>
>>>>
>>>> The format '%*ph' print up to 64 bytes long as a hex string with ' '
>>>> sepatator. Usually the size of report descriptor is larger than 64
>>>> bytes, so consider using print_hex_dump_debug to print out all of it for
>>>> better debugging.
>>>>
>>>> Signed-off-by: Riwen Lu <luriwen@kylinos.cn>
>>>> ---
>>>>    drivers/hid/i2c-hid/i2c-hid-core.c | 4 +++-
>>>>    1 file changed, 3 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
>>>> index efbba0465eef..8e97fc01c852 100644
>>>> --- a/drivers/hid/i2c-hid/i2c-hid-core.c
>>>> +++ b/drivers/hid/i2c-hid/i2c-hid-core.c
>>>> @@ -772,7 +772,9 @@ static int i2c_hid_parse(struct hid_device *hid)
>>>>    		}
>>>>    	}
>>>>
>>>> -	i2c_hid_dbg(ihid, "Report Descriptor: %*ph\n", rsize, rdesc);
>>>> +	i2c_hid_dbg(ihid, "Report Descriptor\n");
>>>> +	print_hex_dump_debug("  ", DUMP_PREFIX_OFFSET, 16, 1,
>>>> +			rdesc, rsize, false);
> 
> Maybe it makes sense to use a prefix for the hex dump that is easy to
> trace rather than padding whitespace? This looks good when you do not
> see any other kernel message log lines get interlaced when written.
> However, if you have a lot of concurrent kernel message output, I think
> it can be tough to piece together the lines of the dump with this
> prefix. Just my opinion.
> 
That's a good opinion. I'll make a v2 version.

Thanks.
>>>
>>> But that would dump it unconditionally, while i2c_hid_dbg() is
>>> conditional.
>>>
>> Function print_hex_dump_debug() dump messages is as conditional as
>> i2c_hid_dbg().
>>
>> The function i2c_hid_dbg() defines as follows:
>> #define i2c_hid_dbg(ihid, ...) dev_dbg(&(ihid)->client->dev, __VA_ARGS__)
>>
>> dev_dbg() depends on the same macro as print_hex_dump_debug().
> 
> I agree with this point. Both dev_dbg and print_hex_dump_debug are noops
> if neither CONFIG_DYNAMIC_DEBUG is set or DEBUG is defined.
> 
> --
> Thanks,
> 
> Rahul Rameshbabu
> 


^ permalink raw reply

* Re: [PATCH v6 2/3] dt-bindings: input: qcom,pm8xxx-vib: add new SPMI vibrator module
From: Krzysztof Kozlowski @ 2023-08-15  7:33 UTC (permalink / raw)
  To: Fenglin Wu, linux-arm-msm, linux-kernel, krzysztof.kozlowski+dt,
	robh+dt, agross, andersson, dmitry.baryshkov, Konrad Dybcio,
	Dmitry Torokhov, linux-input, devicetree
  Cc: quic_collinsd, quic_subbaram, quic_kamalw, jestar
In-Reply-To: <20230815064917.387235-3-quic_fenglinw@quicinc.com>

On 15/08/2023 08:49, Fenglin Wu wrote:
> Add compatible strings to support vibrator module inside PMI632,
> PMI7250B, PM7325B, PM7550BA.
> 
> Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
> ---

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

Best regards,
Krzysztof


^ permalink raw reply

* [PATCH -next] Input: walkera0701 - Use module_parport_driver macro to simplify the code
From: Li Zetao @ 2023-08-15  8:01 UTC (permalink / raw)
  To: dmitry.torokhov; +Cc: lizetao1, linux-input

Use the module_parport_driver macro to simplify the code, which is the
same as declaring with module_init() and module_exit().

Signed-off-by: Li Zetao <lizetao1@huawei.com>
---
 drivers/input/joystick/walkera0701.c | 13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/drivers/input/joystick/walkera0701.c b/drivers/input/joystick/walkera0701.c
index 56abc8c6c763..27d95d6cf56e 100644
--- a/drivers/input/joystick/walkera0701.c
+++ b/drivers/input/joystick/walkera0701.c
@@ -296,15 +296,4 @@ static struct parport_driver walkera0701_parport_driver = {
 	.devmodel = true,
 };
 
-static int __init walkera0701_init(void)
-{
-	return parport_register_driver(&walkera0701_parport_driver);
-}
-
-static void __exit walkera0701_exit(void)
-{
-	parport_unregister_driver(&walkera0701_parport_driver);
-}
-
-module_init(walkera0701_init);
-module_exit(walkera0701_exit);
+module_parport_driver(walkera0701_parport_driver);
-- 
2.34.1


^ permalink raw reply related

* [PATCH v1 2/3] iio: imu: st_lsm6dsx: Use aligned data type for timestamp
From: Andy Shevchenko @ 2023-08-15 15:40 UTC (permalink / raw)
  To: Andy Shevchenko, Jonathan Cameron, Lorenzo Bianconi,
	Philipp Jungkamp, Peter Zijlstra, linux-input, linux-iio,
	linux-kernel
  Cc: Jiri Kosina, Jonathan Cameron, Srinivas Pandruvada,
	Lars-Peter Clausen
In-Reply-To: <20230815154027.12468-1-andriy.shevchenko@linux.intel.com>

Use __aligned_s64 for the timestamp field.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
index c19237717e81..d3d4b3281aa6 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
@@ -442,10 +442,9 @@ struct st_lsm6dsx_hw {
 	const struct st_lsm6dsx_settings *settings;
 
 	struct iio_mount_matrix orientation;
-	/* Ensure natural alignment of buffer elements */
 	struct {
 		__le16 channels[3];
-		s64 ts __aligned(8);
+		__aligned_s64 ts;
 	} scan[ST_LSM6DSX_ID_MAX];
 };
 
-- 
2.40.0.1.gaa8946217a0b


^ permalink raw reply related

* [PATCH v1 1/3] types: Complement the aligned types with signed 64-bit one
From: Andy Shevchenko @ 2023-08-15 15:40 UTC (permalink / raw)
  To: Andy Shevchenko, Jonathan Cameron, Lorenzo Bianconi,
	Philipp Jungkamp, Peter Zijlstra, linux-input, linux-iio,
	linux-kernel
  Cc: Jiri Kosina, Jonathan Cameron, Srinivas Pandruvada,
	Lars-Peter Clausen

Some user may want to use aligned signed 64-bit type.
Provide it for them.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 include/linux/types.h      | 3 ++-
 include/uapi/linux/types.h | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/linux/types.h b/include/linux/types.h
index 253168bb3fe1..78d87c751ff5 100644
--- a/include/linux/types.h
+++ b/include/linux/types.h
@@ -115,8 +115,9 @@ typedef u64			u_int64_t;
 typedef s64			int64_t;
 #endif
 
-/* this is a special 64bit data type that is 8-byte aligned */
+/* These are the special 64-bit data types that are 8-byte aligned */
 #define aligned_u64		__aligned_u64
+#define aligned_s64		__aligned_s64
 #define aligned_be64		__aligned_be64
 #define aligned_le64		__aligned_le64
 
diff --git a/include/uapi/linux/types.h b/include/uapi/linux/types.h
index 6375a0684052..48b933938877 100644
--- a/include/uapi/linux/types.h
+++ b/include/uapi/linux/types.h
@@ -53,6 +53,7 @@ typedef __u32 __bitwise __wsum;
  * No conversions are necessary between 32-bit user-space and a 64-bit kernel.
  */
 #define __aligned_u64 __u64 __attribute__((aligned(8)))
+#define __aligned_s64 __s64 __attribute__((aligned(8)))
 #define __aligned_be64 __be64 __attribute__((aligned(8)))
 #define __aligned_le64 __le64 __attribute__((aligned(8)))
 
-- 
2.40.0.1.gaa8946217a0b


^ permalink raw reply related

* [PATCH v1 3/3] iio: hid-sensor: Use aligned data type for timestamp
From: Andy Shevchenko @ 2023-08-15 15:40 UTC (permalink / raw)
  To: Andy Shevchenko, Jonathan Cameron, Lorenzo Bianconi,
	Philipp Jungkamp, Peter Zijlstra, linux-input, linux-iio,
	linux-kernel
  Cc: Jiri Kosina, Jonathan Cameron, Srinivas Pandruvada,
	Lars-Peter Clausen
In-Reply-To: <20230815154027.12468-1-andriy.shevchenko@linux.intel.com>

Use aligned_s64 for the timestamp field.

Note, the actual data is signed, hence with this we also amend that.
While at it, drop redundant __alignment directive.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/iio/accel/hid-sensor-accel-3d.c              | 3 +--
 drivers/iio/gyro/hid-sensor-gyro-3d.c                | 2 +-
 drivers/iio/humidity/hid-sensor-humidity.c           | 2 +-
 drivers/iio/light/hid-sensor-als.c                   | 2 +-
 drivers/iio/orientation/hid-sensor-incl-3d.c         | 2 +-
 drivers/iio/orientation/hid-sensor-rotation.c        | 4 ++--
 drivers/iio/position/hid-sensor-custom-intel-hinge.c | 2 +-
 drivers/iio/pressure/hid-sensor-press.c              | 2 +-
 drivers/iio/temperature/hid-sensor-temperature.c     | 2 +-
 9 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/iio/accel/hid-sensor-accel-3d.c b/drivers/iio/accel/hid-sensor-accel-3d.c
index 5eac7ea19993..f739589564c5 100644
--- a/drivers/iio/accel/hid-sensor-accel-3d.c
+++ b/drivers/iio/accel/hid-sensor-accel-3d.c
@@ -25,10 +25,9 @@ struct accel_3d_state {
 	struct hid_sensor_hub_callbacks callbacks;
 	struct hid_sensor_common common_attributes;
 	struct hid_sensor_hub_attribute_info accel[ACCEL_3D_CHANNEL_MAX];
-	/* Ensure timestamp is naturally aligned */
 	struct {
 		u32 accel_val[3];
-		s64 timestamp __aligned(8);
+		aligned_s64 timestamp;
 	} scan;
 	int scale_pre_decml;
 	int scale_post_decml;
diff --git a/drivers/iio/gyro/hid-sensor-gyro-3d.c b/drivers/iio/gyro/hid-sensor-gyro-3d.c
index 698c50da1f10..a7050a6328d6 100644
--- a/drivers/iio/gyro/hid-sensor-gyro-3d.c
+++ b/drivers/iio/gyro/hid-sensor-gyro-3d.c
@@ -27,7 +27,7 @@ struct gyro_3d_state {
 	struct hid_sensor_hub_attribute_info gyro[GYRO_3D_CHANNEL_MAX];
 	struct {
 		u32 gyro_val[GYRO_3D_CHANNEL_MAX];
-		u64 timestamp __aligned(8);
+		aligned_s64 timestamp;
 	} scan;
 	int scale_pre_decml;
 	int scale_post_decml;
diff --git a/drivers/iio/humidity/hid-sensor-humidity.c b/drivers/iio/humidity/hid-sensor-humidity.c
index fa0fe404a70a..0e484b78b735 100644
--- a/drivers/iio/humidity/hid-sensor-humidity.c
+++ b/drivers/iio/humidity/hid-sensor-humidity.c
@@ -18,7 +18,7 @@ struct hid_humidity_state {
 	struct hid_sensor_hub_attribute_info humidity_attr;
 	struct {
 		s32 humidity_data;
-		u64 timestamp __aligned(8);
+		aligned_s64 timestamp;
 	} scan;
 	int scale_pre_decml;
 	int scale_post_decml;
diff --git a/drivers/iio/light/hid-sensor-als.c b/drivers/iio/light/hid-sensor-als.c
index eb1aedad7edc..8a906d95edd4 100644
--- a/drivers/iio/light/hid-sensor-als.c
+++ b/drivers/iio/light/hid-sensor-als.c
@@ -27,7 +27,7 @@ struct als_state {
 	struct hid_sensor_hub_attribute_info als_illum;
 	struct {
 		u32 illum[CHANNEL_SCAN_INDEX_MAX];
-		u64 timestamp __aligned(8);
+		aligned_s64 timestamp;
 	} scan;
 	int scale_pre_decml;
 	int scale_post_decml;
diff --git a/drivers/iio/orientation/hid-sensor-incl-3d.c b/drivers/iio/orientation/hid-sensor-incl-3d.c
index ba5b581d5b25..3e5f2c58dfa9 100644
--- a/drivers/iio/orientation/hid-sensor-incl-3d.c
+++ b/drivers/iio/orientation/hid-sensor-incl-3d.c
@@ -29,7 +29,7 @@ struct incl_3d_state {
 	struct hid_sensor_hub_attribute_info incl[INCLI_3D_CHANNEL_MAX];
 	struct {
 		u32 incl_val[INCLI_3D_CHANNEL_MAX];
-		u64 timestamp __aligned(8);
+		aligned_s64 timestamp;
 	} scan;
 	int scale_pre_decml;
 	int scale_post_decml;
diff --git a/drivers/iio/orientation/hid-sensor-rotation.c b/drivers/iio/orientation/hid-sensor-rotation.c
index a033699910e8..864ecbcad26e 100644
--- a/drivers/iio/orientation/hid-sensor-rotation.c
+++ b/drivers/iio/orientation/hid-sensor-rotation.c
@@ -19,8 +19,8 @@ struct dev_rot_state {
 	struct hid_sensor_common common_attributes;
 	struct hid_sensor_hub_attribute_info quaternion;
 	struct {
-		s32 sampled_vals[4] __aligned(16);
-		u64 timestamp __aligned(8);
+		s32 sampled_vals[4];
+		aligned_s64 timestamp;
 	} scan;
 	int scale_pre_decml;
 	int scale_post_decml;
diff --git a/drivers/iio/position/hid-sensor-custom-intel-hinge.c b/drivers/iio/position/hid-sensor-custom-intel-hinge.c
index 07c30d217255..48005b568dd9 100644
--- a/drivers/iio/position/hid-sensor-custom-intel-hinge.c
+++ b/drivers/iio/position/hid-sensor-custom-intel-hinge.c
@@ -39,7 +39,7 @@ struct hinge_state {
 	const char *labels[CHANNEL_SCAN_INDEX_MAX];
 	struct {
 		u32 hinge_val[3];
-		u64 timestamp __aligned(8);
+		aligned_s64 timestamp;
 	} scan;
 
 	int scale_pre_decml;
diff --git a/drivers/iio/pressure/hid-sensor-press.c b/drivers/iio/pressure/hid-sensor-press.c
index a9215eb32d70..a964c7b65402 100644
--- a/drivers/iio/pressure/hid-sensor-press.c
+++ b/drivers/iio/pressure/hid-sensor-press.c
@@ -24,7 +24,7 @@ struct press_state {
 	struct hid_sensor_hub_attribute_info press_attr;
 	struct {
 		u32 press_data;
-		u64 timestamp __aligned(8);
+		aligned_s64 timestamp;
 	} scan;
 	int scale_pre_decml;
 	int scale_post_decml;
diff --git a/drivers/iio/temperature/hid-sensor-temperature.c b/drivers/iio/temperature/hid-sensor-temperature.c
index d40f235af1d4..32f4b13fd554 100644
--- a/drivers/iio/temperature/hid-sensor-temperature.c
+++ b/drivers/iio/temperature/hid-sensor-temperature.c
@@ -18,7 +18,7 @@ struct temperature_state {
 	struct hid_sensor_hub_attribute_info temperature_attr;
 	struct {
 		s32 temperature_data;
-		u64 timestamp __aligned(8);
+		aligned_s64 timestamp;
 	} scan;
 	int scale_pre_decml;
 	int scale_post_decml;
-- 
2.40.0.1.gaa8946217a0b


^ permalink raw reply related

* Re: [PATCH v3 2/2] Input: cs40l50 - Initial support for Cirrus Logic CS40L50
From: James Ogletree @ 2023-08-15 15:56 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Dmitry Torokhov, Fred Treven, Ben Bright, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Lee Jones, Jeff LaBundy,
	Joel Stanley, Arnd Bergmann, Jacky Bai, Jean Delvare, Eddie James,
	Markus Schneider-Pargmann, ChiYuan Huang, Randy Dunlap,
	Wolfram Sang, patches@cirrus.com, linux-input@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <d1b9e063-54df-2dae-1487-c7c932fda145@linaro.org>



> On Aug 10, 2023, at 1:17 AM, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
> 
> On 09/08/2023 21:10, James Ogletree wrote:
> 
>> +
>> +static int cs40l50_cs_dsp_init(struct cs40l50_private *cs40l50)
>> +{
>> + cs40l50->dsp.num = 1;
>> + cs40l50->dsp.type = WMFW_HALO;
>> + cs40l50->dsp.dev = cs40l50->dev;
>> + cs40l50->dsp.regmap = cs40l50->regmap;
>> + cs40l50->dsp.base = CS40L50_DSP1_CORE_BASE;
>> + cs40l50->dsp.base_sysinfo = CS40L50_DSP1_SYS_INFO_ID;
>> + cs40l50->dsp.mem = cs40l50_dsp_regions;
>> + cs40l50->dsp.num_mems = ARRAY_SIZE(cs40l50_dsp_regions);
>> + cs40l50->dsp.lock_regions = 0xFFFFFFFF;
>> + cs40l50->dsp.no_core_startstop = true;
>> + cs40l50->dsp.client_ops = &cs40l50_cs_dsp_client_ops;
>> +
>> + return cs_dsp_halo_init(&cs40l50->dsp);
>> +}
>> +
>> +int cs40l50_probe(struct cs40l50_private *cs40l50)
>> +{
>> + int error, i, irq;
>> + u32 val;
>> +
>> + mutex_init(&cs40l50->lock);
>> +
>> + error = devm_regulator_bulk_get(cs40l50->dev, ARRAY_SIZE(cs40l50_supplies),
>> + cs40l50_supplies);
>> + if (error)
>> + return dev_err_probe(cs40l50->dev, error, "Failed to request supplies\n");
>> +
>> + error = regulator_bulk_enable(ARRAY_SIZE(cs40l50_supplies), cs40l50_supplies);
>> + if (error)
>> + return dev_err_probe(cs40l50->dev, error, "Failed to enable supplies\n");
>> +
>> + cs40l50->reset_gpio = devm_gpiod_get_optional(cs40l50->dev, "reset", GPIOD_OUT_HIGH);
> 
> None of the lines above or below seem to be wrapped according to Linux
> coding style (80).

This patch abides by the 100-column line length limit which checkpatch.pl enforces.
However, I can see how some of the lines might be less jarring to the eyes if wrapped.
That will be addressed in V4.

> 
>> + if (IS_ERR(cs40l50->reset_gpio)) {
>> + error = PTR_ERR(cs40l50->reset_gpio);
>> + goto err;
> 
> Why do you disable IRQ now?
> 
> I asked to test this. Your entire cleanup path is:
> 1. not tested.
> 2. buggy.
> 3. done differently than Linux style. Use proper cleanup calls and
> multiple goto labels.

Disabling IRQ there is a mistake. The probe error path will be tightened up and made
to utilize multiple goto labels instead in V4.

Regards,
James Ogletree


^ permalink raw reply

* Re: [PATCH] drivers/input/touchscreen/goodix.c: Add support for ACPI ID GDX9110
From: Dmitry Torokhov @ 2023-08-15 18:39 UTC (permalink / raw)
  To: Felix Engelhardt; +Cc: Bastien Nocera, Hans de Goede, linux-input, linux-kernel
In-Reply-To: <20230807124723.382899-1-felix.engelhardt@eidu.com>

On Mon, Aug 07, 2023 at 02:47:23PM +0200, Felix Engelhardt wrote:
> The Goodix touchscreen controller with ACPI ID GDX9110 was not recognized
> by the goodix driver. This patch adds this ID to the list of supported IDs,
> allowing the driver to be used with this device. The change will allow
> Linux to be used on ~1 million tablet devices used in Kenyan primary
> schools.
> 
> Signed-off-by: Felix Engelhardt <felix.engelhardt@eidu.com>

Applied, thank you.

-- 
Dmitry

^ permalink raw reply

* [PATCH 3/4] Input: zforce_ts: Accept standard touchscreen properties
From: Andreas Kemnade @ 2023-08-15 18:29 UTC (permalink / raw)
  To: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt,
	shawnguo, s.hauer, kernel, festevam, linux-imx, rydberg,
	u.kleine-koenig, linus.walleij, Jonathan.Cameron, linux-input,
	devicetree, linux-kernel, linux-arm-kernel
  Cc: Andreas Kemnade
In-Reply-To: <20230815182948.212575-1-andreas@kemnade.info>

Only driver-specific properties were accepted, change it
to use the now-available standard properties.

Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
 drivers/input/touchscreen/zforce_ts.c | 36 +++++++++++++--------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/drivers/input/touchscreen/zforce_ts.c b/drivers/input/touchscreen/zforce_ts.c
index 5be5112845e1..f4397497bbe9 100644
--- a/drivers/input/touchscreen/zforce_ts.c
+++ b/drivers/input/touchscreen/zforce_ts.c
@@ -20,6 +20,7 @@
 #include <linux/device.h>
 #include <linux/sysfs.h>
 #include <linux/input/mt.h>
+#include <linux/input/touchscreen.h>
 #include <linux/platform_data/zforce_ts.h>
 #include <linux/regulator/consumer.h>
 #include <linux/of.h>
@@ -106,6 +107,7 @@ struct zforce_point {
 struct zforce_ts {
 	struct i2c_client	*client;
 	struct input_dev	*input;
+	struct touchscreen_properties prop;
 	const struct zforce_ts_platdata *pdata;
 	char			phys[32];
 
@@ -266,7 +268,6 @@ static int zforce_setconfig(struct zforce_ts *ts, char b1)
 static int zforce_start(struct zforce_ts *ts)
 {
 	struct i2c_client *client = ts->client;
-	const struct zforce_ts_platdata *pdata = ts->pdata;
 	int ret;
 
 	dev_dbg(&client->dev, "starting device\n");
@@ -277,7 +278,7 @@ static int zforce_start(struct zforce_ts *ts)
 		return ret;
 	}
 
-	ret = zforce_resolution(ts, pdata->x_max, pdata->y_max);
+	ret = zforce_resolution(ts, ts->prop.max_x, ts->prop.max_y);
 	if (ret) {
 		dev_err(&client->dev, "Unable to set resolution, %d\n", ret);
 		goto error;
@@ -337,7 +338,6 @@ static int zforce_stop(struct zforce_ts *ts)
 static int zforce_touch_event(struct zforce_ts *ts, u8 *payload)
 {
 	struct i2c_client *client = ts->client;
-	const struct zforce_ts_platdata *pdata = ts->pdata;
 	struct zforce_point point;
 	int count, i, num = 0;
 
@@ -355,8 +355,8 @@ static int zforce_touch_event(struct zforce_ts *ts, u8 *payload)
 		point.coord_y =
 			payload[9 * i + 4] << 8 | payload[9 * i + 3];
 
-		if (point.coord_x > pdata->x_max ||
-		    point.coord_y > pdata->y_max) {
+		if (point.coord_x > ts->prop.max_x ||
+		    point.coord_y > ts->prop.max_y) {
 			dev_warn(&client->dev, "coordinates (%d,%d) invalid\n",
 				point.coord_x, point.coord_y);
 			point.coord_x = point.coord_y = 0;
@@ -390,10 +390,11 @@ static int zforce_touch_event(struct zforce_ts *ts, u8 *payload)
 						point.state != STATE_UP);
 
 		if (point.state != STATE_UP) {
-			input_report_abs(ts->input, ABS_MT_POSITION_X,
-					 point.coord_x);
-			input_report_abs(ts->input, ABS_MT_POSITION_Y,
-					 point.coord_y);
+			touchscreen_report_pos(ts->input,
+					       &ts->prop,
+					       point.coord_x,
+					       point.coord_y,
+					       true);
 			input_report_abs(ts->input, ABS_MT_TOUCH_MAJOR,
 					 point.area_major);
 			input_report_abs(ts->input, ABS_MT_TOUCH_MINOR,
@@ -719,15 +720,8 @@ static struct zforce_ts_platdata *zforce_parse_dt(struct device *dev)
 		return ERR_PTR(-ENOMEM);
 	}
 
-	if (of_property_read_u32(np, "x-size", &pdata->x_max)) {
-		dev_err(dev, "failed to get x-size property\n");
-		return ERR_PTR(-EINVAL);
-	}
-
-	if (of_property_read_u32(np, "y-size", &pdata->y_max)) {
-		dev_err(dev, "failed to get y-size property\n");
-		return ERR_PTR(-EINVAL);
-	}
+	of_property_read_u32(np, "x-size", &pdata->x_max);
+	of_property_read_u32(np, "y-size", &pdata->y_max);
 
 	return pdata;
 }
@@ -856,6 +850,12 @@ static int zforce_probe(struct i2c_client *client)
 	input_set_abs_params(input_dev, ABS_MT_POSITION_Y, 0,
 			     pdata->y_max, 0, 0);
 
+	touchscreen_parse_properties(input_dev, true, &ts->prop);
+	if (ts->prop.max_x == 0 || ts->prop.max_y == 0) {
+		dev_err(&client->dev, "no size specified\n");
+		return -EINVAL;
+	}
+
 	input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR, 0,
 			     ZFORCE_MAX_AREA, 0, 0);
 	input_set_abs_params(input_dev, ABS_MT_TOUCH_MINOR, 0,
-- 
2.39.2


^ permalink raw reply related

* [PATCH 2/4] dt-bindings: touchscreen: neonode,zforce: Use standard properties
From: Andreas Kemnade @ 2023-08-15 18:29 UTC (permalink / raw)
  To: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt,
	shawnguo, s.hauer, kernel, festevam, linux-imx, rydberg,
	u.kleine-koenig, linus.walleij, Jonathan.Cameron, linux-input,
	devicetree, linux-kernel, linux-arm-kernel
  Cc: Andreas Kemnade
In-Reply-To: <20230815182948.212575-1-andreas@kemnade.info>

Enable touchscreen orientation to be specified by using standard
properties.

Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
 .../bindings/input/touchscreen/neonode,zforce.yaml  | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/input/touchscreen/neonode,zforce.yaml b/Documentation/devicetree/bindings/input/touchscreen/neonode,zforce.yaml
index 1c45adb2407a..03513f38c9cc 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/neonode,zforce.yaml
+++ b/Documentation/devicetree/bindings/input/touchscreen/neonode,zforce.yaml
@@ -9,6 +9,9 @@ title: Neonode infrared touchscreen controller
 maintainers:
   - Heiko Stuebner <heiko.stuebner@bqreaders.com>
 
+allOf:
+  - $ref: touchscreen.yaml#
+
 properties:
   compatible:
     const: neonode,zforce
@@ -26,9 +29,11 @@ properties:
     maxItems: 1
 
   x-size:
+    deprecated: true
     $ref: /schemas/types.yaml#/definitions/uint32
 
   y-size:
+    deprecated: true
     $ref: /schemas/types.yaml#/definitions/uint32
 
   vdd-supply: true
@@ -38,8 +43,6 @@ required:
   - reg
   - interrupts
   - reset-gpios
-  - x-size
-  - y-size
 
 unevaluatedProperties: false
 
@@ -60,8 +63,10 @@ examples:
             reset-gpios = <&gpio5 9 0>; /* RST */
             irq-gpios = <&gpio5 6 0>; /* IRQ, optional */
 
-            x-size = <800>;
-            y-size = <600>;
+            touchscreen-min-x = <0>;
+            touchscreen-size-x = <800>;
+            touchscreen-min-y = <0>;
+            touchscreen-size-y = <600>;
         };
     };
 ...
-- 
2.39.2


^ permalink raw reply related

* [PATCH 0/4] Input: zforce_ts: standard properties
From: Andreas Kemnade @ 2023-08-15 18:29 UTC (permalink / raw)
  To: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt,
	shawnguo, s.hauer, kernel, festevam, linux-imx, rydberg,
	u.kleine-koenig, linus.walleij, Jonathan.Cameron, linux-input,
	devicetree, linux-kernel, linux-arm-kernel
  Cc: Andreas Kemnade

Accept standard touchscreen properties to also enable specification
of touchscreen orientation. 

Andreas Kemnade (4):
  dt-bindings: touchscreen: convert neonode,zforce to json-schema
  dt-bindings: touchscreen: neonode,zforce: Use standard properties
  Input: zforce_ts: Accept standard touchscreen properties
  ARM: dts: imx6sl-tolino-shine2hd: fix touchscreen rotation

 .../input/touchscreen/neonode,zforce.yaml     | 72 +++++++++++++++++++
 .../bindings/input/touchscreen/zforce_ts.txt  | 34 ---------
 .../dts/nxp/imx/imx6sl-tolino-shine2hd.dts    |  6 +-
 drivers/input/touchscreen/zforce_ts.c         | 36 +++++-----
 4 files changed, 94 insertions(+), 54 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/neonode,zforce.yaml
 delete mode 100644 Documentation/devicetree/bindings/input/touchscreen/zforce_ts.txt

-- 
2.39.2


^ permalink raw reply

* [PATCH 4/4] ARM: dts: imx6sl-tolino-shine2hd: fix touchscreen rotation
From: Andreas Kemnade @ 2023-08-15 18:29 UTC (permalink / raw)
  To: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt,
	shawnguo, s.hauer, kernel, festevam, linux-imx, rydberg,
	u.kleine-koenig, linus.walleij, Jonathan.Cameron, linux-input,
	devicetree, linux-kernel, linux-arm-kernel
  Cc: Andreas Kemnade
In-Reply-To: <20230815182948.212575-1-andreas@kemnade.info>

The display is in landscape orientation, but the touchscreen is in portrait
orientation. Specify that properly in the devicetree.

Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
 arch/arm/boot/dts/nxp/imx/imx6sl-tolino-shine2hd.dts | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/nxp/imx/imx6sl-tolino-shine2hd.dts b/arch/arm/boot/dts/nxp/imx/imx6sl-tolino-shine2hd.dts
index 815119c12bd4..5636fb3661e8 100644
--- a/arch/arm/boot/dts/nxp/imx/imx6sl-tolino-shine2hd.dts
+++ b/arch/arm/boot/dts/nxp/imx/imx6sl-tolino-shine2hd.dts
@@ -141,8 +141,10 @@ zforce: touchscreen@50 {
 		interrupts = <6 IRQ_TYPE_EDGE_FALLING>;
 		vdd-supply = <&ldo1_reg>;
 		reset-gpios = <&gpio5 9 GPIO_ACTIVE_LOW>;
-		x-size = <1072>;
-		y-size = <1448>;
+		touchscreen-size-x = <1072>;
+		touchscreen-size-y = <1448>;
+		touchscreen-swapped-x-y;
+		touchscreen-inverted-x;
 	};
 
 	/* TODO: TPS65185 PMIC for E Ink at 0x68 */
-- 
2.39.2


^ permalink raw reply related

* [PATCH 1/4] dt-bindings: touchscreen: convert neonode,zforce to json-schema
From: Andreas Kemnade @ 2023-08-15 18:29 UTC (permalink / raw)
  To: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt,
	shawnguo, s.hauer, kernel, festevam, linux-imx, rydberg,
	u.kleine-koenig, linus.walleij, Jonathan.Cameron, linux-input,
	devicetree, linux-kernel, linux-arm-kernel
  Cc: Andreas Kemnade
In-Reply-To: <20230815182948.212575-1-andreas@kemnade.info>

Convert Neonode infrared touchscreen controller binding to DT schema.

Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
 .../input/touchscreen/neonode,zforce.yaml     | 67 +++++++++++++++++++
 .../bindings/input/touchscreen/zforce_ts.txt  | 34 ----------
 2 files changed, 67 insertions(+), 34 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/neonode,zforce.yaml
 delete mode 100644 Documentation/devicetree/bindings/input/touchscreen/zforce_ts.txt

diff --git a/Documentation/devicetree/bindings/input/touchscreen/neonode,zforce.yaml b/Documentation/devicetree/bindings/input/touchscreen/neonode,zforce.yaml
new file mode 100644
index 000000000000..1c45adb2407a
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/neonode,zforce.yaml
@@ -0,0 +1,67 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/touchscreen/neonode,zforce.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Neonode infrared touchscreen controller
+
+maintainers:
+  - Heiko Stuebner <heiko.stuebner@bqreaders.com>
+
+properties:
+  compatible:
+    const: neonode,zforce
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  reset-gpios:
+    maxItems: 1
+
+  irq-gpios:
+    maxItems: 1
+
+  x-size:
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+  y-size:
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+  vdd-supply: true
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - reset-gpios
+  - x-size
+  - y-size
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        touchscreen@50 {
+            compatible = "neonode,zforce";
+            reg = <0x50>;
+            interrupts = <2 0>;
+            vdd-supply = <&reg_zforce_vdd>;
+
+            reset-gpios = <&gpio5 9 0>; /* RST */
+            irq-gpios = <&gpio5 6 0>; /* IRQ, optional */
+
+            x-size = <800>;
+            y-size = <600>;
+        };
+    };
+...
diff --git a/Documentation/devicetree/bindings/input/touchscreen/zforce_ts.txt b/Documentation/devicetree/bindings/input/touchscreen/zforce_ts.txt
deleted file mode 100644
index e3c27c4fd9c8..000000000000
--- a/Documentation/devicetree/bindings/input/touchscreen/zforce_ts.txt
+++ /dev/null
@@ -1,34 +0,0 @@
-* Neonode infrared touchscreen controller
-
-Required properties:
-- compatible: must be "neonode,zforce"
-- reg: I2C address of the chip
-- interrupts: interrupt to which the chip is connected
-- reset-gpios: reset gpio the chip is connected to
-- x-size: horizontal resolution of touchscreen
-- y-size: vertical resolution of touchscreen
-
-Optional properties:
-- irq-gpios : interrupt gpio the chip is connected to
-- vdd-supply: Regulator controlling the controller supply
-
-Example:
-
-	i2c@00000000 {
-		/* ... */
-
-		zforce_ts@50 {
-			compatible = "neonode,zforce";
-			reg = <0x50>;
-			interrupts = <2 0>;
-			vdd-supply = <&reg_zforce_vdd>;
-
-			reset-gpios = <&gpio5 9 0>; /* RST */
-			irq-gpios = <&gpio5 6 0>; /* IRQ, optional */
-
-			x-size = <800>;
-			y-size = <600>;
-		};
-
-		/* ... */
-	};
-- 
2.39.2


^ permalink raw reply related

* Re: [PATCH v3 2/2] Input: cs40l50 - Initial support for Cirrus Logic CS40L50
From: Krzysztof Kozlowski @ 2023-08-15 19:23 UTC (permalink / raw)
  To: James Ogletree
  Cc: Dmitry Torokhov, Fred Treven, Ben Bright, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Lee Jones, Jeff LaBundy,
	Joel Stanley, Arnd Bergmann, Jacky Bai, Jean Delvare, Eddie James,
	Markus Schneider-Pargmann, ChiYuan Huang, Randy Dunlap,
	Wolfram Sang, patches@cirrus.com, linux-input@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <D4B53566-9F7D-46DA-8BAE-0572B9A8AA7A@cirrus.com>

On 15/08/2023 17:56, James Ogletree wrote:
> 
> 
>> On Aug 10, 2023, at 1:17 AM, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 09/08/2023 21:10, James Ogletree wrote:
>>
>>> +
>>> +static int cs40l50_cs_dsp_init(struct cs40l50_private *cs40l50)
>>> +{
>>> + cs40l50->dsp.num = 1;
>>> + cs40l50->dsp.type = WMFW_HALO;
>>> + cs40l50->dsp.dev = cs40l50->dev;
>>> + cs40l50->dsp.regmap = cs40l50->regmap;
>>> + cs40l50->dsp.base = CS40L50_DSP1_CORE_BASE;
>>> + cs40l50->dsp.base_sysinfo = CS40L50_DSP1_SYS_INFO_ID;
>>> + cs40l50->dsp.mem = cs40l50_dsp_regions;
>>> + cs40l50->dsp.num_mems = ARRAY_SIZE(cs40l50_dsp_regions);
>>> + cs40l50->dsp.lock_regions = 0xFFFFFFFF;
>>> + cs40l50->dsp.no_core_startstop = true;
>>> + cs40l50->dsp.client_ops = &cs40l50_cs_dsp_client_ops;
>>> +
>>> + return cs_dsp_halo_init(&cs40l50->dsp);
>>> +}
>>> +
>>> +int cs40l50_probe(struct cs40l50_private *cs40l50)
>>> +{
>>> + int error, i, irq;
>>> + u32 val;
>>> +
>>> + mutex_init(&cs40l50->lock);
>>> +
>>> + error = devm_regulator_bulk_get(cs40l50->dev, ARRAY_SIZE(cs40l50_supplies),
>>> + cs40l50_supplies);
>>> + if (error)
>>> + return dev_err_probe(cs40l50->dev, error, "Failed to request supplies\n");
>>> +
>>> + error = regulator_bulk_enable(ARRAY_SIZE(cs40l50_supplies), cs40l50_supplies);
>>> + if (error)
>>> + return dev_err_probe(cs40l50->dev, error, "Failed to enable supplies\n");
>>> +
>>> + cs40l50->reset_gpio = devm_gpiod_get_optional(cs40l50->dev, "reset", GPIOD_OUT_HIGH);
>>
>> None of the lines above or below seem to be wrapped according to Linux
>> coding style (80).
> 
> This patch abides by the 100-column line length limit which checkpatch.pl enforces.

Checkpatch does not enforce 100, because it accepts also 80. It
complains if something is longer than 100, but coding style is still the
same. Format everything according to coding style and use 100 only if it
increases readability.

> However, I can see how some of the lines might be less jarring to the eyes if wrapped.
> That will be addressed in V4.
> 

Best regards,
Krzysztof


^ permalink raw reply

* [PATCH v2] HID: apple: Add "Hailuck" to the list of non-apple keyboards
From: Nils Tonnaett @ 2023-08-15 20:19 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires, linux-input, linux-kernel
  Cc: Rahul Rameshbabu, Nils Tonnaett
In-Reply-To: <20230811202932.30413-1-ntonnatt@ccrma.stanford.edu>

Powzan keyboards KB750 and KB770 identify as
"Hailuck Co.,Ltd USB Keyboard". Adding "Hailuck" to the list
of non-apple keyboards fixes function keys for these models.

Signed-off-by: Nils Tonnaett <ntonnatt@ccrma.stanford.edu>
---
V1 -> V2:
	- Start commit message subject with HID: apple: instead of hid:
	- Comma terminate last member of array

 drivers/hid/hid-apple.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
index d7b932925730..3ca45975c686 100644
--- a/drivers/hid/hid-apple.c
+++ b/drivers/hid/hid-apple.c
@@ -343,7 +343,8 @@ static const struct apple_non_apple_keyboard non_apple_keyboards[] = {
 	{ "SONiX USB DEVICE" },
 	{ "Keychron" },
 	{ "AONE" },
-	{ "GANSS" }
+	{ "GANSS" },
+	{ "Hailuck" },
 };
 
 static bool apple_is_non_apple_keyboard(struct hid_device *hdev)
-- 
2.41.0


^ permalink raw reply related


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