* [PATCH] Input: mcs-touchkey - Fix uninitialized use of error in mcs_touchkey_probe()
From: Nathan Chancellor @ 2023-07-25 15:37 UTC (permalink / raw)
To: dmitry.torokhov
Cc: ndesaulniers, trix, frank.li, linux-input, llvm, patches,
Nathan Chancellor
Clang warns (or errors with CONFIG_WERROR=y):
drivers/input/keyboard/mcs_touchkey.c:149:49: error: variable 'error' is uninitialized when used here [-Werror,-Wuninitialized]
149 | dev_err(&client->dev, "i2c read error[%d]\n", error);
| ^~~~~
include/linux/dev_printk.h:144:65: note: expanded from macro 'dev_err'
144 | dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~~~~~
include/linux/dev_printk.h:110:23: note: expanded from macro 'dev_printk_index_wrap'
110 | _p_func(dev, fmt, ##__VA_ARGS__); \
| ^~~~~~~~~~~
drivers/input/keyboard/mcs_touchkey.c:110:11: note: initialize the variable 'error' to silence this warning
110 | int error;
| ^
| = 0
1 error generated.
A refactoring updated the error handling in this block but did not
update the dev_err() call to use fw_ver instead of error. Do so now to
fix the warning and avoid printing uninitialized memory.
Closes: https://github.com/ClangBuiltLinux/linux/issues/1893
Fixes: e175eae16c1b ("Input: mcs-touchkey - convert to use devm_* api")
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
drivers/input/keyboard/mcs_touchkey.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/keyboard/mcs_touchkey.c b/drivers/input/keyboard/mcs_touchkey.c
index f25e2b20e271..2410f676c7f9 100644
--- a/drivers/input/keyboard/mcs_touchkey.c
+++ b/drivers/input/keyboard/mcs_touchkey.c
@@ -146,7 +146,7 @@ static int mcs_touchkey_probe(struct i2c_client *client)
fw_ver = i2c_smbus_read_byte_data(client, fw_reg);
if (fw_ver < 0) {
- dev_err(&client->dev, "i2c read error[%d]\n", error);
+ dev_err(&client->dev, "i2c read error[%d]\n", fw_ver);
return fw_ver;
}
dev_info(&client->dev, "Firmware version: %d\n", fw_ver);
---
base-commit: 447c09544275663e1082f796b26c7959915c922a
change-id: 20230725-mcs_touchkey-fix-wuninitialized-4ffe9a2a9aa9
Best regards,
--
Nathan Chancellor <nathan@kernel.org>
^ permalink raw reply related
* Re: [PATCH v3 1/3] input: pm8xxx-vib: refactor to easily support new SPMI vibrator
From: kernel test robot @ 2023-07-25 10:01 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
Cc: llvm, oe-kbuild-all, quic_collinsd, quic_subbaram, quic_fenglinw,
quic_kamalw, jestar
In-Reply-To: <20230725054138.129497-2-quic_fenglinw@quicinc.com>
Hi Fenglin,
kernel test robot noticed the following build warnings:
[auto build test WARNING on dtor-input/next]
[also build test WARNING on dtor-input/for-linus linus/master v6.5-rc3 next-20230725]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Fenglin-Wu/input-pm8xxx-vib-refactor-to-easily-support-new-SPMI-vibrator/20230725-134504
base: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
patch link: https://lore.kernel.org/r/20230725054138.129497-2-quic_fenglinw%40quicinc.com
patch subject: [PATCH v3 1/3] input: pm8xxx-vib: refactor to easily support new SPMI vibrator
config: x86_64-buildonly-randconfig-r002-20230725 (https://download.01.org/0day-ci/archive/20230725/202307251741.PMtlVAgD-lkp@intel.com/config)
compiler: clang version 15.0.7 (https://github.com/llvm/llvm-project.git 8dfdcc7b7bf66834a761bd8de445840ef68e4d1a)
reproduce: (https://download.01.org/0day-ci/archive/20230725/202307251741.PMtlVAgD-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202307251741.PMtlVAgD-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/input/misc/pm8xxx-vibrator.c:190:17: warning: cast to smaller integer type 'enum pm8xxx_vib_type' from 'const void *' [-Wvoid-pointer-to-enum-cast]
vib->hw_type = (enum pm8xxx_vib_type)of_device_get_match_data(dev);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
vim +190 drivers/input/misc/pm8xxx-vibrator.c
163
164 static int pm8xxx_vib_probe(struct platform_device *pdev)
165 {
166 struct pm8xxx_vib *vib;
167 struct input_dev *input_dev;
168 struct device *dev = &pdev->dev;
169 struct regmap *regmap;
170 struct reg_field *regs;
171 int error, i;
172 unsigned int val;
173 u32 reg_base;
174
175 vib = devm_kzalloc(dev, sizeof(*vib), GFP_KERNEL);
176 if (!vib)
177 return -ENOMEM;
178
179 regmap = dev_get_regmap(dev->parent, NULL);
180 if (!regmap)
181 return -ENODEV;
182
183 input_dev = devm_input_allocate_device(dev);
184 if (!input_dev)
185 return -ENOMEM;
186
187 INIT_WORK(&vib->work, pm8xxx_work_handler);
188 vib->vib_input_dev = input_dev;
189
> 190 vib->hw_type = (enum pm8xxx_vib_type)of_device_get_match_data(dev);
191
192 regs = ssbi_vib_regs;
193 if (vib->hw_type != SSBI_VIB) {
194 error = fwnode_property_read_u32(dev->fwnode, "reg", ®_base);
195 if (error < 0) {
196 dev_err(dev, "Failed to read reg address, rc=%d\n", error);
197 return error;
198 }
199
200 if (vib->hw_type == SPMI_VIB_GEN1)
201 regs = spmi_vib_gen1_regs;
202
203 for (i = 0; i < VIB_MAX_REG; i++)
204 if (regs[i].reg != 0)
205 regs[i].reg += reg_base;
206 }
207
208 error = devm_regmap_field_bulk_alloc(dev, regmap, vib->r_fields, regs, VIB_MAX_REG);
209 if (error < 0)
210 {
211 dev_err(dev, "Failed to allocate regmap failed, rc=%d\n", error);
212 return error;
213 }
214
215 error = regmap_field_read(vib->r_fields[VIB_DRV_REG], &val);
216 if (error < 0)
217 return error;
218
219 /* operate in manual mode */
220 if (vib->hw_type == SSBI_VIB) {
221 val &= SSBI_VIB_DRV_EN_MANUAL_MASK;
222 error = regmap_field_write(vib->r_fields[VIB_DRV_REG], val);
223 if (error < 0)
224 return error;
225 }
226
227 vib->reg_vib_drv = val;
228
229 input_dev->name = "pm8xxx_vib_ffmemless";
230 input_dev->id.version = 1;
231 input_dev->close = pm8xxx_vib_close;
232 input_set_drvdata(input_dev, vib);
233 input_set_capability(vib->vib_input_dev, EV_FF, FF_RUMBLE);
234
235 error = input_ff_create_memless(input_dev, NULL,
236 pm8xxx_vib_play_effect);
237 if (error) {
238 dev_err(dev, "couldn't register vibrator as FF device\n");
239 return error;
240 }
241
242 error = input_register_device(input_dev);
243 if (error) {
244 dev_err(dev, "couldn't register input device\n");
245 return error;
246 }
247
248 platform_set_drvdata(pdev, vib);
249 return 0;
250 }
251
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Słowa kluczowe do wypozycjonowania
From: Adam Charachuta @ 2023-07-25 7:50 UTC (permalink / raw)
To: linux-input
Dzień dobry,
zapoznałem się z Państwa ofertą i z przyjemnością przyznaję, że przyciąga uwagę i zachęca do dalszych rozmów.
Pomyślałem, że może mógłbym mieć swój wkład w Państwa rozwój i pomóc dotrzeć z tą ofertą do większego grona odbiorców. Pozycjonuję strony www, dzięki czemu generują świetny ruch w sieci.
Możemy porozmawiać w najbliższym czasie?
Pozdrawiam
Adam Charachuta
^ permalink raw reply
* Re: [PATCH v2 4/5] dt-bindings: clock: imx6ul: Support optional enet*_ref_pad clocks
From: Abel Vesa @ 2023-07-25 7:34 UTC (permalink / raw)
To: Oleksij Rempel
Cc: Abel Vesa, Michael Turquette, Stephen Boyd, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
Herbert Xu, David S. Miller, Dmitry Torokhov, Ulf Hansson, kernel,
Peng Fan, Fabio Estevam, NXP Linux Team, Daniel Lezcano,
Thomas Gleixner, Michael Trimarchi, Mark Brown, Dario Binacchi,
Marek Vasut, linux-clk, devicetree, linux-arm-kernel,
linux-kernel, linux-crypto, linux-input, linux-mmc
In-Reply-To: <20230621093245.78130-5-o.rempel@pengutronix.de>
On 23-06-21 11:32:44, Oleksij Rempel wrote:
> Extend the 'clocks' and 'clock-names' properties to support optional
> 'enet1_ref_pad' and 'enet2_ref_pad' clocks to resolve the following
> dtbs_check warning:
> imx6ul-prti6g.dtb: clock-controller@20c4000: clocks: [[17], [18], [19], [20], [21]] is too long
> imx6ul-prti6g.dtb: clock-controller@20c4000: clock-names: ['ckil', 'osc', 'ipp_di0', 'ipp_di1', 'enet1_ref_pad'] is too long
>
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
> Documentation/devicetree/bindings/clock/imx6ul-clock.yaml | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/clock/imx6ul-clock.yaml b/Documentation/devicetree/bindings/clock/imx6ul-clock.yaml
> index be54d4df5afa2..3b71ebc100bf6 100644
> --- a/Documentation/devicetree/bindings/clock/imx6ul-clock.yaml
> +++ b/Documentation/devicetree/bindings/clock/imx6ul-clock.yaml
> @@ -28,18 +28,24 @@ properties:
> const: 1
>
> clocks:
> + minItems: 4
> items:
> - description: 32k osc
> - description: 24m osc
> - description: ipp_di0 clock input
> - description: ipp_di1 clock input
> + - description: Optional lenet1_ref_pad or enet2_ref_pad clocks
> + - description: Optional lenet1_ref_pad or enet2_ref_pad clocks
s/lenet1_ref_pad/enet1_ref_pad/g
with that:
Reviewed-by: Abel Vesa <abel.vesa@linaro.org>
>
> clock-names:
> + minItems: 4
> items:
> - const: ckil
> - const: osc
> - const: ipp_di0
> - const: ipp_di1
> + - pattern: '^enet[12]_ref_pad$'
> + - pattern: '^enet[12]_ref_pad$'
>
> required:
> - compatible
> --
> 2.39.2
>
^ permalink raw reply
* [dtor-input:next] BUILD SUCCESS 447c09544275663e1082f796b26c7959915c922a
From: kernel test robot @ 2023-07-25 6:49 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
branch HEAD: 447c09544275663e1082f796b26c7959915c922a Input: qt1070 - convert to use devm_* api
elapsed time: 1455m
configs tested: 220
configs skipped: 15
The following configs have been built successfully.
More configs may be tested in the coming days.
tested configs:
alpha allyesconfig gcc
alpha defconfig gcc
alpha randconfig-r001-20230724 gcc
alpha randconfig-r016-20230724 gcc
alpha randconfig-r031-20230724 gcc
arc allyesconfig gcc
arc defconfig gcc
arc haps_hs_defconfig gcc
arc randconfig-r043-20230724 gcc
arc randconfig-r043-20230725 gcc
arm allmodconfig gcc
arm allyesconfig gcc
arm axm55xx_defconfig gcc
arm defconfig gcc
arm lpc18xx_defconfig gcc
arm qcom_defconfig gcc
arm randconfig-r002-20230724 clang
arm randconfig-r011-20230724 gcc
arm randconfig-r012-20230724 gcc
arm randconfig-r046-20230724 gcc
arm stm32_defconfig gcc
arm64 allyesconfig gcc
arm64 defconfig gcc
arm64 randconfig-r011-20230724 clang
arm64 randconfig-r014-20230725 gcc
arm64 randconfig-r022-20230724 clang
arm64 randconfig-r032-20230725 clang
csky defconfig gcc
csky randconfig-r012-20230724 gcc
csky randconfig-r016-20230724 gcc
csky randconfig-r021-20230724 gcc
hexagon randconfig-r002-20230724 clang
hexagon randconfig-r014-20230724 clang
hexagon randconfig-r041-20230724 clang
hexagon randconfig-r045-20230724 clang
i386 allyesconfig gcc
i386 buildonly-randconfig-r004-20230723 clang
i386 buildonly-randconfig-r004-20230724 gcc
i386 buildonly-randconfig-r005-20230723 clang
i386 buildonly-randconfig-r005-20230724 gcc
i386 buildonly-randconfig-r006-20230723 clang
i386 buildonly-randconfig-r006-20230724 gcc
i386 debian-10.3 gcc
i386 defconfig gcc
i386 randconfig-i001-20230724 gcc
i386 randconfig-i002-20230724 gcc
i386 randconfig-i003-20230724 gcc
i386 randconfig-i004-20230724 gcc
i386 randconfig-i005-20230724 gcc
i386 randconfig-i006-20230724 gcc
i386 randconfig-i011-20230724 clang
i386 randconfig-i011-20230725 gcc
i386 randconfig-i012-20230724 clang
i386 randconfig-i012-20230725 gcc
i386 randconfig-i013-20230724 clang
i386 randconfig-i013-20230725 gcc
i386 randconfig-i014-20230724 clang
i386 randconfig-i014-20230725 gcc
i386 randconfig-i015-20230724 clang
i386 randconfig-i015-20230725 gcc
i386 randconfig-i016-20230724 clang
i386 randconfig-i016-20230725 gcc
i386 randconfig-r012-20230725 gcc
i386 randconfig-r025-20230724 clang
i386 randconfig-r033-20230724 gcc
loongarch allmodconfig gcc
loongarch allnoconfig gcc
loongarch defconfig gcc
loongarch loongson3_defconfig gcc
loongarch randconfig-r005-20230724 gcc
loongarch randconfig-r013-20230724 gcc
loongarch randconfig-r015-20230725 gcc
loongarch randconfig-r016-20230724 gcc
loongarch randconfig-r025-20230724 gcc
m68k allmodconfig gcc
m68k allyesconfig gcc
m68k defconfig gcc
m68k m5272c3_defconfig gcc
m68k randconfig-r012-20230724 gcc
m68k randconfig-r014-20230724 gcc
m68k randconfig-r015-20230724 gcc
m68k randconfig-r023-20230724 gcc
m68k randconfig-r034-20230724 gcc
microblaze randconfig-r001-20230724 gcc
microblaze randconfig-r004-20230724 gcc
microblaze randconfig-r015-20230724 gcc
microblaze randconfig-r016-20230724 gcc
microblaze randconfig-r023-20230724 gcc
microblaze randconfig-r033-20230724 gcc
mips allmodconfig gcc
mips allyesconfig gcc
mips db1xxx_defconfig gcc
mips randconfig-r004-20230724 clang
mips randconfig-r031-20230724 clang
mips randconfig-r034-20230725 gcc
nios2 3c120_defconfig gcc
nios2 defconfig gcc
nios2 randconfig-r001-20230724 gcc
nios2 randconfig-r023-20230725 gcc
nios2 randconfig-r025-20230724 gcc
openrisc randconfig-r012-20230724 gcc
openrisc randconfig-r021-20230724 gcc
openrisc randconfig-r026-20230724 gcc
openrisc randconfig-r033-20230725 gcc
parisc allyesconfig gcc
parisc defconfig gcc
parisc randconfig-r001-20230724 gcc
parisc randconfig-r005-20230724 gcc
parisc randconfig-r006-20230724 gcc
parisc randconfig-r032-20230724 gcc
parisc64 defconfig gcc
powerpc akebono_defconfig clang
powerpc allmodconfig gcc
powerpc allnoconfig gcc
powerpc chrp32_defconfig gcc
powerpc ppc40x_defconfig gcc
powerpc randconfig-r004-20230725 clang
powerpc randconfig-r006-20230724 gcc
powerpc randconfig-r023-20230724 clang
powerpc randconfig-r024-20230724 clang
powerpc stx_gp3_defconfig gcc
riscv allmodconfig gcc
riscv allnoconfig gcc
riscv allyesconfig gcc
riscv defconfig gcc
riscv randconfig-r004-20230724 gcc
riscv randconfig-r014-20230724 clang
riscv randconfig-r042-20230724 clang
riscv randconfig-r042-20230725 gcc
riscv rv32_defconfig gcc
s390 allmodconfig gcc
s390 allyesconfig gcc
s390 defconfig gcc
s390 randconfig-r006-20230725 clang
s390 randconfig-r011-20230725 gcc
s390 randconfig-r032-20230724 gcc
s390 randconfig-r033-20230724 gcc
s390 randconfig-r044-20230724 clang
s390 randconfig-r044-20230725 gcc
sh allmodconfig gcc
sh ecovec24_defconfig gcc
sh j2_defconfig gcc
sh randconfig-r004-20230724 gcc
sh randconfig-r013-20230724 gcc
sh randconfig-r016-20230724 gcc
sh randconfig-r024-20230724 gcc
sh randconfig-r035-20230724 gcc
sh randconfig-r036-20230724 gcc
sh sh03_defconfig gcc
sh titan_defconfig gcc
sparc allyesconfig gcc
sparc defconfig gcc
sparc randconfig-r003-20230724 gcc
sparc randconfig-r005-20230724 gcc
sparc randconfig-r011-20230724 gcc
sparc randconfig-r012-20230724 gcc
sparc randconfig-r013-20230724 gcc
sparc randconfig-r015-20230724 gcc
sparc randconfig-r021-20230725 gcc
sparc randconfig-r036-20230724 gcc
sparc64 randconfig-r016-20230725 gcc
sparc64 randconfig-r035-20230724 gcc
um allmodconfig clang
um allnoconfig clang
um allyesconfig clang
um defconfig gcc
um i386_defconfig gcc
um randconfig-r003-20230724 clang
um randconfig-r006-20230724 clang
um randconfig-r013-20230724 gcc
um randconfig-r031-20230724 clang
um x86_64_defconfig gcc
x86_64 allyesconfig gcc
x86_64 buildonly-randconfig-r001-20230723 clang
x86_64 buildonly-randconfig-r001-20230724 gcc
x86_64 buildonly-randconfig-r002-20230723 clang
x86_64 buildonly-randconfig-r002-20230724 gcc
x86_64 buildonly-randconfig-r003-20230723 clang
x86_64 buildonly-randconfig-r003-20230724 gcc
x86_64 defconfig gcc
x86_64 kexec gcc
x86_64 randconfig-r006-20230724 gcc
x86_64 randconfig-r011-20230724 clang
x86_64 randconfig-r013-20230724 clang
x86_64 randconfig-r022-20230724 clang
x86_64 randconfig-r022-20230725 gcc
x86_64 randconfig-r025-20230725 gcc
x86_64 randconfig-r035-20230724 gcc
x86_64 randconfig-x001-20230724 clang
x86_64 randconfig-x001-20230725 gcc
x86_64 randconfig-x002-20230724 clang
x86_64 randconfig-x002-20230725 gcc
x86_64 randconfig-x003-20230724 clang
x86_64 randconfig-x003-20230725 gcc
x86_64 randconfig-x004-20230724 clang
x86_64 randconfig-x004-20230725 gcc
x86_64 randconfig-x005-20230724 clang
x86_64 randconfig-x005-20230725 gcc
x86_64 randconfig-x006-20230724 clang
x86_64 randconfig-x006-20230725 gcc
x86_64 randconfig-x011-20230724 gcc
x86_64 randconfig-x012-20230724 gcc
x86_64 randconfig-x013-20230724 gcc
x86_64 randconfig-x014-20230724 gcc
x86_64 randconfig-x015-20230724 gcc
x86_64 randconfig-x016-20230724 gcc
x86_64 rhel-8.3-bpf gcc
x86_64 rhel-8.3-func gcc
x86_64 rhel-8.3-kselftests gcc
x86_64 rhel-8.3-kunit gcc
x86_64 rhel-8.3-ltp gcc
x86_64 rhel-8.3-rust clang
x86_64 rhel-8.3 gcc
xtensa alldefconfig gcc
xtensa randconfig-r002-20230724 gcc
xtensa randconfig-r003-20230724 gcc
xtensa randconfig-r012-20230724 gcc
xtensa randconfig-r015-20230724 gcc
xtensa randconfig-r026-20230724 gcc
xtensa randconfig-r036-20230724 gcc
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH v3 2/3] dt-bindings: input: qcom,pm8xxx-vib: add new SPMI vibrator module
From: Fenglin Wu @ 2023-07-25 6:26 UTC (permalink / raw)
To: Krzysztof Kozlowski, 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: <b2ad4863-a38b-7fb6-65b1-ea336c4fc876@linaro.org>
On 7/25/2023 1:53 PM, Krzysztof Kozlowski wrote:
> On 25/07/2023 07:41, Fenglin Wu wrote:
>> Add compatible string 'qcom,spmi-vib-gen2' for vibrator module inside
>> PMI632, PMI7250B, PM7325B, PM7550BA. Also, add 'qcom,spmi-vib-gen1'
>> string for the SPMI vibrator inside PM8916 to maintain the completeness
>> of the hardware version history for SPMI vibrators.
>>
>> Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
>> ---
>> .../bindings/input/qcom,pm8xxx-vib.yaml | 18 ++++++++++++++----
>> 1 file changed, 14 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..ab778714ad29 100644
>> --- a/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml
>> +++ b/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml
>> @@ -11,10 +11,20 @@ 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,spmi-vib-gen1
>> + - qcom,spmi-vib-gen2
>
> Generic compatibles should not be alone. Drop both lines.
Sure. I will remove 'qcom,spmi-vib-gen2'.
Should I also keep 'qcom,spmi-vib-gen1' as generic compatible and move
'qcom,pm8916-vib' as its fallback as following?
compatible:
oneOf:
- enum:
- qcom,pm8058-vib
- qcom,pm8921-vib
- items:
- enum:
- qcom,pm8916-vib
- const: qcom,spmi-vib-gen1
- items:
- enum:
- qcom,pmi632-vib
- qcom,pm7250b-vib
- qcom,pm7325b-vib
- qcom,pm7550b-vib
- const: qcom,spmi-vib-gen2
I saw 'qcom,pm8916-vib' has been used in multiple DTS files and updating
it as a fallback will result updating those DTS files as well.
Or please help to suggest if there is any way to keep 'qcom,pm8916-vib'
and 'qcom,spmi-vib-gen1' compatible without updating existing DTS nodes.
Thanks
>
>> + - items:
>> + - enum:
>> + - qcom,pmi632-vib
>> + - qcom,pm7250b-vib
>> + - qcom,pm7325b-vib
>> + - qcom,pm7550b-vib
>> + - const: qcom,spmi-vib-gen2
>
>
> Best regards,
> Krzysztof
>
^ permalink raw reply
* Re: [PATCH v3 1/3] input: pm8xxx-vib: refactor to easily support new SPMI vibrator
From: Fenglin Wu @ 2023-07-25 6:16 UTC (permalink / raw)
To: Krzysztof Kozlowski, 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_kamalw, jestar
In-Reply-To: <5dd56c31-7ca3-dd39-0623-e4fd18ac6f68@linaro.org>
On 7/25/2023 1:52 PM, Krzysztof Kozlowski wrote:
> On 25/07/2023 07:41, Fenglin Wu wrote:
>> 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 introducing the HW type
>> terminology and contain the register offsets/masks/shifts in reg_filed
>> data structures corresponding to each vibrator type, and the base address
>> value defined in 'reg' property will be added for SPMI vibrators.
>>
>> Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
>> ---
>> drivers/input/misc/pm8xxx-vibrator.c | 130 ++++++++++++++++-----------
>> 1 file changed, 77 insertions(+), 53 deletions(-)
>>
>> diff --git a/drivers/input/misc/pm8xxx-vibrator.c b/drivers/input/misc/pm8xxx-vibrator.c
>> index 04cb87efd799..77bb0018d4e1 100644
>> --- a/drivers/input/misc/pm8xxx-vibrator.c
>> +++ b/drivers/input/misc/pm8xxx-vibrator.c
>> @@ -12,36 +12,36 @@
>> #include <linux/regmap.h>
>> #include <linux/slab.h>
>>
>> +#define SSBI_VIB_DRV_EN_MANUAL_MASK 0xfc
>> +#define SSBI_VIB_DRV_LEVEL_MASK 0xf8
>> +#define SSBI_VIB_DRV_SHIFT 3
>> +#define SPMI_VIB_DRV_LEVEL_MASK 0xff
>> +#define SPMI_VIB_DRV_SHIFT 0
>> +
>> #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 pm8xxx_vib_type {
>> + SSBI_VIB,
>> + SPMI_VIB_GEN1,
>> +};
>>
>> - unsigned int drv_addr;
>> - unsigned int drv_mask;
>> - unsigned int drv_shift;
>> - unsigned int drv_en_manual_mask;
>> +enum {
>> + VIB_DRV_REG,
>> + VIB_EN_REG,
>> + VIB_MAX_REG,
>> };
>>
>> -static const struct pm8xxx_regs pm8058_regs = {
>> - .drv_addr = 0x4A,
>> - .drv_mask = 0xf8,
>> - .drv_shift = 3,
>> - .drv_en_manual_mask = 0xfc,
>> +static struct reg_field ssbi_vib_regs[VIB_MAX_REG] = {
>
> Change from const to non-const is wrong. How do you support multiple
> devices? No, this is way too fragile now.
>
The register definition is no longer used as the match data, hw_type is
used.
The last suggestion was getting the register base address from the DT
and it has to be added into the offset of SPMI vibrator registers
(either in the previous hard-coded format or the later the reg_filed
data structure), so it's not appropriated to make it constant.
I don't understand this question: "How do you support multiple devices?"
For SSBI vibrator, since all the registers are fixed, and I would assume
that there is no chance to support multiple vibrator devices on the same
SSBI bus. If they are not on the same bus, the regmap device will be
different while the registers definition is the same, and we are still
able to support multiple devices, right?
The similar story for SPMI vibrators and it can support multiple devices
if they are located on different SPMI bus, or even if they are on the
same SPMI bus but just having different SID or PID.
> ...
>
>>
>> /*
>> * pmic vibrator supports voltage ranges from 1.2 to 3.1V, so
>> @@ -168,38 +166,65 @@ static int pm8xxx_vib_probe(struct platform_device *pdev)
>> {
>> struct pm8xxx_vib *vib;
>> struct input_dev *input_dev;
>> - int error;
>> + struct device *dev = &pdev->dev;
>> + struct regmap *regmap;
>> + struct reg_field *regs;
>> + int error, i;
>> unsigned int val;
>> - const struct pm8xxx_regs *regs;
>> + u32 reg_base;
>>
>> - vib = devm_kzalloc(&pdev->dev, sizeof(*vib), GFP_KERNEL);
>> + vib = devm_kzalloc(dev, sizeof(*vib), GFP_KERNEL);
>
> Not really related. Split cleanup from new features.
Okay, will keep as it is.
>
>> if (!vib)
>> return -ENOMEM;
>>
>> - vib->regmap = dev_get_regmap(pdev->dev.parent, NULL);
>> - if (!vib->regmap)
>> + regmap = dev_get_regmap(dev->parent, NULL);
>> + if (!regmap)
>> return -ENODEV;
>>
>> - input_dev = devm_input_allocate_device(&pdev->dev);
>> + input_dev = devm_input_allocate_device(dev);
>> if (!input_dev)
>> return -ENOMEM;
>>
>> INIT_WORK(&vib->work, pm8xxx_work_handler);
>> vib->vib_input_dev = input_dev;
>>
>> - regs = of_device_get_match_data(&pdev->dev);
>> + vib->hw_type = (enum pm8xxx_vib_type)of_device_get_match_data(dev);
>>
>> - /* operate in manual mode */
>> - error = regmap_read(vib->regmap, regs->drv_addr, &val);
>> + regs = ssbi_vib_regs;
>> + if (vib->hw_type != SSBI_VIB) {
>> + error = fwnode_property_read_u32(dev->fwnode, "reg", ®_base);
>> + if (error < 0) {
>> + dev_err(dev, "Failed to read reg address, rc=%d\n", error);
>> + return error;
>> + }
>> +
>> + if (vib->hw_type == SPMI_VIB_GEN1)
>> + regs = spmi_vib_gen1_regs;
>> +
>> + for (i = 0; i < VIB_MAX_REG; i++)
>> + if (regs[i].reg != 0)
>> + regs[i].reg += reg_base;
>> + }
>> +
>> + error = devm_regmap_field_bulk_alloc(dev, regmap, vib->r_fields, regs, VIB_MAX_REG);
>> if (error < 0)
>> + {
>
> That's not a Linux coding style.
>
> Please run scripts/checkpatch.pl and fix reported warnings. Some
> warnings can be ignored, but the code here looks like it needs a fix.
> Feel free to get in touch if the warning is not clear.
>
>> + dev_err(dev, "Failed to allocate regmap failed, rc=%d\n", error);
>
> No need to print errors on allocation failures.
>
Will fix it.
Thanks
>> return error;
>> + }
>>
>> - val &= regs->drv_en_manual_mask;
>> - error = regmap_write(vib->regmap, regs->drv_addr, val);
>> + error = regmap_field_read(vib->r_fields[VIB_DRV_REG], &val);
>> if (error < 0)
>> return error;
>
>
> Best regards,
> Krzysztof
>
^ permalink raw reply
* Re: Fwd: Kernel 6.4.4 breaks Elan Touchpad 1206
From: Bagas Sanjaya @ 2023-07-25 5:53 UTC (permalink / raw)
To: Dmitry Torokhov, Henrik Rydberg, Verot
Cc: Linux Input Devices, Linux Kernel Mailing List, Linux Regressions,
Linux Stable
In-Reply-To: <42bc8e02-5ee0-f1c8-610e-e16391e54ee2@gmail.com>
On 7/25/23 07:00, Bagas Sanjaya wrote:
> #regzbot introduced: 7b63a88bb62ba2 https://bugzilla.kernel.org/show_bug.cgi?id=217701
> #regzbot title: OOB protocol access fix breaks Elan Touchpad 1206
>
Looks like the wrong culprit (see [1]), thus:
#regzbot introduced: v6.1..v6.4
Thanks.
[1]: https://lore.kernel.org/linux-input/ZL8S2%2FUjprk4KgzT@google.com/
--
An old man doll... just what I always wanted! - Clara
^ permalink raw reply
* Re: [PATCH v3 2/3] dt-bindings: input: qcom,pm8xxx-vib: add new SPMI vibrator module
From: Krzysztof Kozlowski @ 2023-07-25 5:53 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: <20230725054138.129497-3-quic_fenglinw@quicinc.com>
On 25/07/2023 07:41, Fenglin Wu wrote:
> Add compatible string 'qcom,spmi-vib-gen2' for vibrator module inside
> PMI632, PMI7250B, PM7325B, PM7550BA. Also, add 'qcom,spmi-vib-gen1'
> string for the SPMI vibrator inside PM8916 to maintain the completeness
> of the hardware version history for SPMI vibrators.
>
> Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
> ---
> .../bindings/input/qcom,pm8xxx-vib.yaml | 18 ++++++++++++++----
> 1 file changed, 14 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..ab778714ad29 100644
> --- a/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml
> +++ b/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml
> @@ -11,10 +11,20 @@ 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,spmi-vib-gen1
> + - qcom,spmi-vib-gen2
Generic compatibles should not be alone. Drop both lines.
> + - items:
> + - enum:
> + - qcom,pmi632-vib
> + - qcom,pm7250b-vib
> + - qcom,pm7325b-vib
> + - qcom,pm7550b-vib
> + - const: qcom,spmi-vib-gen2
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v3 1/3] input: pm8xxx-vib: refactor to easily support new SPMI vibrator
From: Krzysztof Kozlowski @ 2023-07-25 5:52 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
Cc: quic_collinsd, quic_subbaram, quic_kamalw, jestar
In-Reply-To: <20230725054138.129497-2-quic_fenglinw@quicinc.com>
On 25/07/2023 07:41, Fenglin Wu wrote:
> 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 introducing the HW type
> terminology and contain the register offsets/masks/shifts in reg_filed
> data structures corresponding to each vibrator type, and the base address
> value defined in 'reg' property will be added for SPMI vibrators.
>
> Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
> ---
> drivers/input/misc/pm8xxx-vibrator.c | 130 ++++++++++++++++-----------
> 1 file changed, 77 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/input/misc/pm8xxx-vibrator.c b/drivers/input/misc/pm8xxx-vibrator.c
> index 04cb87efd799..77bb0018d4e1 100644
> --- a/drivers/input/misc/pm8xxx-vibrator.c
> +++ b/drivers/input/misc/pm8xxx-vibrator.c
> @@ -12,36 +12,36 @@
> #include <linux/regmap.h>
> #include <linux/slab.h>
>
> +#define SSBI_VIB_DRV_EN_MANUAL_MASK 0xfc
> +#define SSBI_VIB_DRV_LEVEL_MASK 0xf8
> +#define SSBI_VIB_DRV_SHIFT 3
> +#define SPMI_VIB_DRV_LEVEL_MASK 0xff
> +#define SPMI_VIB_DRV_SHIFT 0
> +
> #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 pm8xxx_vib_type {
> + SSBI_VIB,
> + SPMI_VIB_GEN1,
> +};
>
> - unsigned int drv_addr;
> - unsigned int drv_mask;
> - unsigned int drv_shift;
> - unsigned int drv_en_manual_mask;
> +enum {
> + VIB_DRV_REG,
> + VIB_EN_REG,
> + VIB_MAX_REG,
> };
>
> -static const struct pm8xxx_regs pm8058_regs = {
> - .drv_addr = 0x4A,
> - .drv_mask = 0xf8,
> - .drv_shift = 3,
> - .drv_en_manual_mask = 0xfc,
> +static struct reg_field ssbi_vib_regs[VIB_MAX_REG] = {
Change from const to non-const is wrong. How do you support multiple
devices? No, this is way too fragile now.
...
>
> /*
> * pmic vibrator supports voltage ranges from 1.2 to 3.1V, so
> @@ -168,38 +166,65 @@ static int pm8xxx_vib_probe(struct platform_device *pdev)
> {
> struct pm8xxx_vib *vib;
> struct input_dev *input_dev;
> - int error;
> + struct device *dev = &pdev->dev;
> + struct regmap *regmap;
> + struct reg_field *regs;
> + int error, i;
> unsigned int val;
> - const struct pm8xxx_regs *regs;
> + u32 reg_base;
>
> - vib = devm_kzalloc(&pdev->dev, sizeof(*vib), GFP_KERNEL);
> + vib = devm_kzalloc(dev, sizeof(*vib), GFP_KERNEL);
Not really related. Split cleanup from new features.
> if (!vib)
> return -ENOMEM;
>
> - vib->regmap = dev_get_regmap(pdev->dev.parent, NULL);
> - if (!vib->regmap)
> + regmap = dev_get_regmap(dev->parent, NULL);
> + if (!regmap)
> return -ENODEV;
>
> - input_dev = devm_input_allocate_device(&pdev->dev);
> + input_dev = devm_input_allocate_device(dev);
> if (!input_dev)
> return -ENOMEM;
>
> INIT_WORK(&vib->work, pm8xxx_work_handler);
> vib->vib_input_dev = input_dev;
>
> - regs = of_device_get_match_data(&pdev->dev);
> + vib->hw_type = (enum pm8xxx_vib_type)of_device_get_match_data(dev);
>
> - /* operate in manual mode */
> - error = regmap_read(vib->regmap, regs->drv_addr, &val);
> + regs = ssbi_vib_regs;
> + if (vib->hw_type != SSBI_VIB) {
> + error = fwnode_property_read_u32(dev->fwnode, "reg", ®_base);
> + if (error < 0) {
> + dev_err(dev, "Failed to read reg address, rc=%d\n", error);
> + return error;
> + }
> +
> + if (vib->hw_type == SPMI_VIB_GEN1)
> + regs = spmi_vib_gen1_regs;
> +
> + for (i = 0; i < VIB_MAX_REG; i++)
> + if (regs[i].reg != 0)
> + regs[i].reg += reg_base;
> + }
> +
> + error = devm_regmap_field_bulk_alloc(dev, regmap, vib->r_fields, regs, VIB_MAX_REG);
> if (error < 0)
> + {
That's not a Linux coding style.
Please run scripts/checkpatch.pl and fix reported warnings. Some
warnings can be ignored, but the code here looks like it needs a fix.
Feel free to get in touch if the warning is not clear.
> + dev_err(dev, "Failed to allocate regmap failed, rc=%d\n", error);
No need to print errors on allocation failures.
> return error;
> + }
>
> - val &= regs->drv_en_manual_mask;
> - error = regmap_write(vib->regmap, regs->drv_addr, val);
> + error = regmap_field_read(vib->r_fields[VIB_DRV_REG], &val);
> if (error < 0)
> return error;
Best regards,
Krzysztof
^ permalink raw reply
* [PATCH v3 3/3] input: pm8xxx-vibrator: add new SPMI vibrator support
From: Fenglin Wu @ 2023-07-25 5:41 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: <20230725054138.129497-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.
Name the module as 'qcom,spmi-vib-gen2', and it can be found in
following Qualcomm PMICs: PMI632, PM7250B, PM7325B, PM7550BA. Also, add
a compatible string 'qcom,spmi-vib-gen1' for SPMI vibrator inside PM8916
to maintain the completeness of the hardware version history.
Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
---
drivers/input/misc/pm8xxx-vibrator.c | 37 ++++++++++++++++++++++++++--
1 file changed, 35 insertions(+), 2 deletions(-)
diff --git a/drivers/input/misc/pm8xxx-vibrator.c b/drivers/input/misc/pm8xxx-vibrator.c
index 77bb0018d4e1..2cbccc5c6cf3 100644
--- a/drivers/input/misc/pm8xxx-vibrator.c
+++ b/drivers/input/misc/pm8xxx-vibrator.c
@@ -17,6 +17,8 @@
#define SSBI_VIB_DRV_SHIFT 3
#define SPMI_VIB_DRV_LEVEL_MASK 0xff
#define SPMI_VIB_DRV_SHIFT 0
+#define SPMI_VIB_DRV2_LEVEL_MASK 0x0f
+#define SPMI_VIB_DRV2_SHIFT 8
#define VIB_MAX_LEVEL_mV (3100)
#define VIB_MIN_LEVEL_mV (1200)
@@ -27,11 +29,13 @@
enum pm8xxx_vib_type {
SSBI_VIB,
SPMI_VIB_GEN1,
+ SPMI_VIB_GEN2,
};
enum {
VIB_DRV_REG,
VIB_EN_REG,
+ VIB_DRV2_REG,
VIB_MAX_REG,
};
@@ -44,6 +48,12 @@ static struct reg_field spmi_vib_gen1_regs[VIB_MAX_REG] = {
REG_FIELD(0x46, 7, 7),
};
+static struct reg_field spmi_vib_gen2_regs[VIB_MAX_REG] = {
+ REG_FIELD(0x40, 0, 7),
+ REG_FIELD(0x46, 7, 7),
+ REG_FIELD(0x41, 0, 3),
+};
+
/**
* struct pm8xxx_vib - structure to hold vibrator data
* @vib_input_dev: input device supporting force feedback
@@ -94,6 +104,22 @@ static int pm8xxx_vib_set(struct pm8xxx_vib *vib, bool on)
vib->reg_vib_drv = val;
+ if (vib->hw_type == SPMI_VIB_GEN2) {
+ mask = SPMI_VIB_DRV2_LEVEL_MASK;
+ shift = SPMI_VIB_DRV2_SHIFT;
+
+ if (on)
+ val = (vib->level >> shift) & mask;
+ else
+ val = 0;
+
+ rc = regmap_field_write(vib->r_fields[VIB_DRV2_REG], val);
+ if (rc < 0)
+ return rc;
+
+ vib->reg_vib_drv |= val << shift;
+ }
+
if (vib->hw_type != SSBI_VIB)
rc = regmap_field_write(vib->r_fields[VIB_EN_REG], on);
@@ -116,10 +142,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->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->hw_type != SPMI_VIB_GEN2)
+ vib->level /= 100;
}
pm8xxx_vib_set(vib, vib->active);
@@ -200,6 +229,8 @@ static int pm8xxx_vib_probe(struct platform_device *pdev)
if (vib->hw_type == SPMI_VIB_GEN1)
regs = spmi_vib_gen1_regs;
+ else
+ regs = spmi_vib_gen2_regs;
for (i = 0; i < VIB_MAX_REG; i++)
if (regs[i].reg != 0)
@@ -266,6 +297,8 @@ static const struct of_device_id pm8xxx_vib_id_table[] = {
{ .compatible = "qcom,pm8058-vib", .data = (void *)SSBI_VIB },
{ .compatible = "qcom,pm8921-vib", .data = (void *)SSBI_VIB },
{ .compatible = "qcom,pm8916-vib", .data = (void *)SPMI_VIB_GEN1 },
+ { .compatible = "qcom,spmi-vib-gen1", .data = (void *)SPMI_VIB_GEN1 },
+ { .compatible = "qcom,spmi-vib-gen2", .data = (void *)SPMI_VIB_GEN2 },
{ }
};
MODULE_DEVICE_TABLE(of, pm8xxx_vib_id_table);
--
2.25.1
^ permalink raw reply related
* [PATCH v3 2/3] dt-bindings: input: qcom,pm8xxx-vib: add new SPMI vibrator module
From: Fenglin Wu @ 2023-07-25 5:41 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: <20230725054138.129497-1-quic_fenglinw@quicinc.com>
Add compatible string 'qcom,spmi-vib-gen2' for vibrator module inside
PMI632, PMI7250B, PM7325B, PM7550BA. Also, add 'qcom,spmi-vib-gen1'
string for the SPMI vibrator inside PM8916 to maintain the completeness
of the hardware version history for SPMI vibrators.
Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
---
.../bindings/input/qcom,pm8xxx-vib.yaml | 18 ++++++++++++++----
1 file changed, 14 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..ab778714ad29 100644
--- a/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml
+++ b/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml
@@ -11,10 +11,20 @@ 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,spmi-vib-gen1
+ - qcom,spmi-vib-gen2
+ - items:
+ - enum:
+ - qcom,pmi632-vib
+ - qcom,pm7250b-vib
+ - qcom,pm7325b-vib
+ - qcom,pm7550b-vib
+ - const: qcom,spmi-vib-gen2
reg:
maxItems: 1
--
2.25.1
^ permalink raw reply related
* [PATCH v3 1/3] input: pm8xxx-vib: refactor to easily support new SPMI vibrator
From: Fenglin Wu @ 2023-07-25 5:41 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: <20230725054138.129497-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 introducing the HW type
terminology and contain the register offsets/masks/shifts in reg_filed
data structures corresponding to each vibrator type, and the base address
value defined in 'reg' property will be added for SPMI vibrators.
Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
---
drivers/input/misc/pm8xxx-vibrator.c | 130 ++++++++++++++++-----------
1 file changed, 77 insertions(+), 53 deletions(-)
diff --git a/drivers/input/misc/pm8xxx-vibrator.c b/drivers/input/misc/pm8xxx-vibrator.c
index 04cb87efd799..77bb0018d4e1 100644
--- a/drivers/input/misc/pm8xxx-vibrator.c
+++ b/drivers/input/misc/pm8xxx-vibrator.c
@@ -12,36 +12,36 @@
#include <linux/regmap.h>
#include <linux/slab.h>
+#define SSBI_VIB_DRV_EN_MANUAL_MASK 0xfc
+#define SSBI_VIB_DRV_LEVEL_MASK 0xf8
+#define SSBI_VIB_DRV_SHIFT 3
+#define SPMI_VIB_DRV_LEVEL_MASK 0xff
+#define SPMI_VIB_DRV_SHIFT 0
+
#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 pm8xxx_vib_type {
+ SSBI_VIB,
+ SPMI_VIB_GEN1,
+};
- unsigned int drv_addr;
- unsigned int drv_mask;
- unsigned int drv_shift;
- unsigned int drv_en_manual_mask;
+enum {
+ VIB_DRV_REG,
+ VIB_EN_REG,
+ VIB_MAX_REG,
};
-static const struct pm8xxx_regs pm8058_regs = {
- .drv_addr = 0x4A,
- .drv_mask = 0xf8,
- .drv_shift = 3,
- .drv_en_manual_mask = 0xfc,
+static struct reg_field ssbi_vib_regs[VIB_MAX_REG] = {
+ REG_FIELD(0xf8, 0, 7),
};
-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 struct reg_field spmi_vib_gen1_regs[VIB_MAX_REG] = {
+ REG_FIELD(0x41, 0, 4),
+ REG_FIELD(0x46, 7, 7),
};
/**
@@ -58,12 +58,12 @@ static struct pm8xxx_regs pm8916_regs = {
struct pm8xxx_vib {
struct input_dev *vib_input_dev;
struct work_struct work;
- struct regmap *regmap;
- const struct pm8xxx_regs *regs;
+ struct regmap_field *r_fields[VIB_MAX_REG];
int speed;
int level;
bool active;
u8 reg_vib_drv;
+ enum pm8xxx_vib_type hw_type;
};
/**
@@ -75,22 +75,27 @@ 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->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_field_write(vib->r_fields[VIB_DRV_REG], 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->hw_type != SSBI_VIB)
+ rc = regmap_field_write(vib->r_fields[VIB_EN_REG], on);
return rc;
}
@@ -102,13 +107,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,38 +166,65 @@ static int pm8xxx_vib_probe(struct platform_device *pdev)
{
struct pm8xxx_vib *vib;
struct input_dev *input_dev;
- int error;
+ struct device *dev = &pdev->dev;
+ struct regmap *regmap;
+ struct reg_field *regs;
+ int error, i;
unsigned int val;
- const struct pm8xxx_regs *regs;
+ u32 reg_base;
- vib = devm_kzalloc(&pdev->dev, sizeof(*vib), GFP_KERNEL);
+ vib = devm_kzalloc(dev, sizeof(*vib), GFP_KERNEL);
if (!vib)
return -ENOMEM;
- vib->regmap = dev_get_regmap(pdev->dev.parent, NULL);
- if (!vib->regmap)
+ regmap = dev_get_regmap(dev->parent, NULL);
+ if (!regmap)
return -ENODEV;
- input_dev = devm_input_allocate_device(&pdev->dev);
+ input_dev = devm_input_allocate_device(dev);
if (!input_dev)
return -ENOMEM;
INIT_WORK(&vib->work, pm8xxx_work_handler);
vib->vib_input_dev = input_dev;
- regs = of_device_get_match_data(&pdev->dev);
+ vib->hw_type = (enum pm8xxx_vib_type)of_device_get_match_data(dev);
- /* operate in manual mode */
- error = regmap_read(vib->regmap, regs->drv_addr, &val);
+ regs = ssbi_vib_regs;
+ if (vib->hw_type != SSBI_VIB) {
+ error = fwnode_property_read_u32(dev->fwnode, "reg", ®_base);
+ if (error < 0) {
+ dev_err(dev, "Failed to read reg address, rc=%d\n", error);
+ return error;
+ }
+
+ if (vib->hw_type == SPMI_VIB_GEN1)
+ regs = spmi_vib_gen1_regs;
+
+ for (i = 0; i < VIB_MAX_REG; i++)
+ if (regs[i].reg != 0)
+ regs[i].reg += reg_base;
+ }
+
+ error = devm_regmap_field_bulk_alloc(dev, regmap, vib->r_fields, regs, VIB_MAX_REG);
if (error < 0)
+ {
+ dev_err(dev, "Failed to allocate regmap failed, rc=%d\n", error);
return error;
+ }
- val &= regs->drv_en_manual_mask;
- error = regmap_write(vib->regmap, regs->drv_addr, val);
+ error = regmap_field_read(vib->r_fields[VIB_DRV_REG], &val);
if (error < 0)
return error;
- vib->regs = regs;
+ /* operate in manual mode */
+ if (vib->hw_type == SSBI_VIB) {
+ val &= SSBI_VIB_DRV_EN_MANUAL_MASK;
+ error = regmap_field_write(vib->r_fields[VIB_DRV_REG], val);
+ if (error < 0)
+ return error;
+ }
+
vib->reg_vib_drv = val;
input_dev->name = "pm8xxx_vib_ffmemless";
@@ -211,14 +236,13 @@ static int pm8xxx_vib_probe(struct platform_device *pdev)
error = input_ff_create_memless(input_dev, NULL,
pm8xxx_vib_play_effect);
if (error) {
- dev_err(&pdev->dev,
- "couldn't register vibrator as FF device\n");
+ dev_err(dev, "couldn't register vibrator as FF device\n");
return error;
}
error = input_register_device(input_dev);
if (error) {
- dev_err(&pdev->dev, "couldn't register input device\n");
+ dev_err(dev, "couldn't register input device\n");
return error;
}
@@ -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 = (void *)SSBI_VIB },
+ { .compatible = "qcom,pm8921-vib", .data = (void *)SSBI_VIB },
+ { .compatible = "qcom,pm8916-vib", .data = (void *)SPMI_VIB_GEN1 },
{ }
};
MODULE_DEVICE_TABLE(of, pm8xxx_vib_id_table);
--
2.25.1
^ permalink raw reply related
* Re: Fwd: Kernel 6.4.4 breaks Elan Touchpad 1206
From: Dmitry Torokhov @ 2023-07-25 0:14 UTC (permalink / raw)
To: Bagas Sanjaya
Cc: Henrik Rydberg, Verot, Linux Input Devices,
Linux Kernel Mailing List, Linux Regressions, Linux Stable
In-Reply-To: <ZL8S2/Ujprk4KgzT@google.com>
On Mon, Jul 24, 2023 at 05:10:03PM -0700, Dmitry Torokhov wrote:
> On Tue, Jul 25, 2023 at 07:00:21AM +0700, Bagas Sanjaya wrote:
> > Hi,
> >
> > I notice a regression report on Bugzilla [1]. Quoting from it:
> >
> > > Description:
> > > When booting into Linux 6.4.4, system no longer recognizes touchpad input (confirmed with xinput). On the lts release, 6.1.39, the input is still recognized.
> > >
> > > Additional info:
> > > * package version(s): Linux 6.4.4, 6.1.39
> > > * Device: ELAN1206:00 04F3:30F1 Touchpad
> > >
> > > Steps to reproduce:
> > > - Install 6.4.4 with Elan Touchpad 1206
> > > - Reboot
> > >
> > > The issue might be related to bisected commit id: 7b63a88bb62ba2ddf5fcd956be85fe46624628b9
> > > This is the only recent commit related to Elantech drivers I've noticed that may have broken the input.
> >
> > See Bugzilla for the full thread:
> >
> > To the reporter (Verot): Can you attach dmesg and lspci output?
> >
> > Anyway, I'm adding this regression to be tracked by regzbot:
> >
> > #regzbot introduced: 7b63a88bb62ba2 https://bugzilla.kernel.org/show_bug.cgi?id=217701
> > #regzbot title: OOB protocol access fix breaks Elan Touchpad 1206
>
> Please don't as ELAN1206:00 04F3:30F1 looks like I2C-HID and not PS/2
> device that 7b63a88bb62ba2 would affect.
Also 7b63a88bb62ba2 is present in 6.1 since v6.1.34.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: Fwd: Kernel 6.4.4 breaks Elan Touchpad 1206
From: Dmitry Torokhov @ 2023-07-25 0:10 UTC (permalink / raw)
To: Bagas Sanjaya
Cc: Henrik Rydberg, Verot, Linux Input Devices,
Linux Kernel Mailing List, Linux Regressions, Linux Stable
In-Reply-To: <42bc8e02-5ee0-f1c8-610e-e16391e54ee2@gmail.com>
On Tue, Jul 25, 2023 at 07:00:21AM +0700, Bagas Sanjaya wrote:
> Hi,
>
> I notice a regression report on Bugzilla [1]. Quoting from it:
>
> > Description:
> > When booting into Linux 6.4.4, system no longer recognizes touchpad input (confirmed with xinput). On the lts release, 6.1.39, the input is still recognized.
> >
> > Additional info:
> > * package version(s): Linux 6.4.4, 6.1.39
> > * Device: ELAN1206:00 04F3:30F1 Touchpad
> >
> > Steps to reproduce:
> > - Install 6.4.4 with Elan Touchpad 1206
> > - Reboot
> >
> > The issue might be related to bisected commit id: 7b63a88bb62ba2ddf5fcd956be85fe46624628b9
> > This is the only recent commit related to Elantech drivers I've noticed that may have broken the input.
>
> See Bugzilla for the full thread:
>
> To the reporter (Verot): Can you attach dmesg and lspci output?
>
> Anyway, I'm adding this regression to be tracked by regzbot:
>
> #regzbot introduced: 7b63a88bb62ba2 https://bugzilla.kernel.org/show_bug.cgi?id=217701
> #regzbot title: OOB protocol access fix breaks Elan Touchpad 1206
Please don't as ELAN1206:00 04F3:30F1 looks like I2C-HID and not PS/2
device that 7b63a88bb62ba2 would affect.
Thanks.
--
Dmitry
^ permalink raw reply
* Fwd: Kernel 6.4.4 breaks Elan Touchpad 1206
From: Bagas Sanjaya @ 2023-07-25 0:00 UTC (permalink / raw)
To: Dmitry Torokhov, Henrik Rydberg, Verot
Cc: Linux Input Devices, Linux Kernel Mailing List, Linux Regressions,
Linux Stable
Hi,
I notice a regression report on Bugzilla [1]. Quoting from it:
> Description:
> When booting into Linux 6.4.4, system no longer recognizes touchpad input (confirmed with xinput). On the lts release, 6.1.39, the input is still recognized.
>
> Additional info:
> * package version(s): Linux 6.4.4, 6.1.39
> * Device: ELAN1206:00 04F3:30F1 Touchpad
>
> Steps to reproduce:
> - Install 6.4.4 with Elan Touchpad 1206
> - Reboot
>
> The issue might be related to bisected commit id: 7b63a88bb62ba2ddf5fcd956be85fe46624628b9
> This is the only recent commit related to Elantech drivers I've noticed that may have broken the input.
See Bugzilla for the full thread:
To the reporter (Verot): Can you attach dmesg and lspci output?
Anyway, I'm adding this regression to be tracked by regzbot:
#regzbot introduced: 7b63a88bb62ba2 https://bugzilla.kernel.org/show_bug.cgi?id=217701
#regzbot title: OOB protocol access fix breaks Elan Touchpad 1206
Thanks.
[1]: https://bugzilla.kernel.org/show_bug.cgi?id=217701
--
An old man doll... just what I always wanted! - Clara
^ permalink raw reply
* Re: [PATCH V2] Input: xpad - Add HyperX Clutch Gladiate Support
From: Rahul Rameshbabu @ 2023-07-24 23:47 UTC (permalink / raw)
To: HP Dev; +Cc: Dmitry Torokhov, linux-input, Chris Toledanes, Carl Ng,
Max Nguyen
In-Reply-To: <20230724224903.146092-1-hphyperxdev@gmail.com>
On Mon, 24 Jul, 2023 15:49:05 -0700 HP Dev <hphyperxdev@gmail.com> wrote:
> Add HyperX controller support to xpad_device and xpad_table.
>
> Suggested-by: Chris Toledanes <chris.toledanes@hp.com>
> Reviewed-by: Carl Ng <carl.ng@hp.com>
> Signed-off-by: Max Nguyen <maxwell.nguyen@hp.com>
> ---
> V1 -> V2: Remove Xbox 360 vendor-specific class support.
Thanks for making this change. I think it makes sense. I think if we
want to have universal support for probing both Xbox 360 and Xbox One
controller devices for a single vendorId in the xpad driver, the
XPAD_XBOX360_VENDOR and XPAD_XBOXONE_VENDOR macros would be consolidated
(one macro that instantiates support for both protocols). Since no
HyperX controller for supporting Xbox 360 exists currently, I do not
think it makes sense to introduce XPAD_XBOX360_VENDOR vendor-specific
class matching for the HyperX vendorId in this patch (what v2 resolves).
>
> drivers/input/joystick/xpad.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
> index cdb193317c3b..1e377d040c43 100644
> --- a/drivers/input/joystick/xpad.c
> +++ b/drivers/input/joystick/xpad.c
> @@ -130,6 +130,7 @@ static const struct xpad_device {
> { 0x0079, 0x18d4, "GPD Win 2 X-Box Controller", 0, XTYPE_XBOX360 },
> { 0x03eb, 0xff01, "Wooting One (Legacy)", 0, XTYPE_XBOX360 },
> { 0x03eb, 0xff02, "Wooting Two (Legacy)", 0, XTYPE_XBOX360 },
> + { 0x03f0, 0x0495, "HyperX Clutch Gladiate", 0, XTYPE_XBOXONE },
> { 0x044f, 0x0f00, "Thrustmaster Wheel", 0, XTYPE_XBOX },
> { 0x044f, 0x0f03, "Thrustmaster Wheel", 0, XTYPE_XBOX },
> { 0x044f, 0x0f07, "Thrustmaster, Inc. Controller", 0, XTYPE_XBOX },
> @@ -457,6 +458,7 @@ static const struct usb_device_id xpad_table[] = {
> { USB_INTERFACE_INFO('X', 'B', 0) }, /* Xbox USB-IF not-approved class */
> XPAD_XBOX360_VENDOR(0x0079), /* GPD Win 2 controller */
> XPAD_XBOX360_VENDOR(0x03eb), /* Wooting Keyboards (Legacy) */
> + XPAD_XBOXONE_VENDOR(0x03f0), /* HP HyperX Xbox One controllers */
> XPAD_XBOX360_VENDOR(0x044f), /* Thrustmaster Xbox 360 controllers */
> XPAD_XBOX360_VENDOR(0x045e), /* Microsoft Xbox 360 controllers */
> XPAD_XBOXONE_VENDOR(0x045e), /* Microsoft Xbox One controllers */
Reviewed-by: Rahul Rameshbabu <rrameshbabu@nvidia.com>
^ permalink raw reply
* [PATCH V2] Input: xpad - Add HyperX Clutch Gladiate Support
From: HP Dev @ 2023-07-24 22:49 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, HP Dev, Chris Toledanes, Carl Ng, Max Nguyen
Add HyperX controller support to xpad_device and xpad_table.
Suggested-by: Chris Toledanes <chris.toledanes@hp.com>
Reviewed-by: Carl Ng <carl.ng@hp.com>
Signed-off-by: Max Nguyen <maxwell.nguyen@hp.com>
---
V1 -> V2: Remove Xbox 360 vendor-specific class support.
drivers/input/joystick/xpad.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index cdb193317c3b..1e377d040c43 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -130,6 +130,7 @@ static const struct xpad_device {
{ 0x0079, 0x18d4, "GPD Win 2 X-Box Controller", 0, XTYPE_XBOX360 },
{ 0x03eb, 0xff01, "Wooting One (Legacy)", 0, XTYPE_XBOX360 },
{ 0x03eb, 0xff02, "Wooting Two (Legacy)", 0, XTYPE_XBOX360 },
+ { 0x03f0, 0x0495, "HyperX Clutch Gladiate", 0, XTYPE_XBOXONE },
{ 0x044f, 0x0f00, "Thrustmaster Wheel", 0, XTYPE_XBOX },
{ 0x044f, 0x0f03, "Thrustmaster Wheel", 0, XTYPE_XBOX },
{ 0x044f, 0x0f07, "Thrustmaster, Inc. Controller", 0, XTYPE_XBOX },
@@ -457,6 +458,7 @@ static const struct usb_device_id xpad_table[] = {
{ USB_INTERFACE_INFO('X', 'B', 0) }, /* Xbox USB-IF not-approved class */
XPAD_XBOX360_VENDOR(0x0079), /* GPD Win 2 controller */
XPAD_XBOX360_VENDOR(0x03eb), /* Wooting Keyboards (Legacy) */
+ XPAD_XBOXONE_VENDOR(0x03f0), /* HP HyperX Xbox One controllers */
XPAD_XBOX360_VENDOR(0x044f), /* Thrustmaster Xbox 360 controllers */
XPAD_XBOX360_VENDOR(0x045e), /* Microsoft Xbox 360 controllers */
XPAD_XBOXONE_VENDOR(0x045e), /* Microsoft Xbox One controllers */
--
2.39.3
^ permalink raw reply related
* Re: [PATCH 2/2] Input: lm8323 - convert to use devm_* api
From: Christophe JAILLET @ 2023-07-24 19:26 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, Yangtao Li, linux-kernel
In-Reply-To: <ZL7KghF8L+y1Fzcs@google.com>
Le 24/07/2023 à 21:01, Dmitry Torokhov a écrit :
> On Mon, Jul 24, 2023 at 08:53:11PM +0200, Christophe JAILLET wrote:
>> Le 24/07/2023 à 07:29, Dmitry Torokhov a écrit :
>>> From: Yangtao Li <frank.li@vivo.com>
>>> +
>>> + err = devm_led_classdev_register(dev, &pwm->cdev);
>
> ^^^^^^^^^^^^^^
Oops,
For some reason, I missed this hunk :(.
>
>>> -
>>> - for (i = 0; i < 3; i++)
>>> - if (lm->pwm[i].enabled)
>>> - led_classdev_unregister(&lm->pwm[i].cdev);
>>
>> this look wrong.
>
> Why? It will be cleared up by devm.
>
> Thanks.
>
Sorry for the noise.
I've been puzzled by [1].
CJ
[1]: https://lore.kernel.org/all/20230714080611.81302-8-frank.li@vivo.com/
^ permalink raw reply
* Re: [PATCH 2/2] Input: lm8323 - convert to use devm_* api
From: Dmitry Torokhov @ 2023-07-24 19:01 UTC (permalink / raw)
To: Christophe JAILLET; +Cc: linux-input, Yangtao Li, linux-kernel
In-Reply-To: <b83df292-6f7a-a8bf-895e-6df80a17029f@wanadoo.fr>
On Mon, Jul 24, 2023 at 08:53:11PM +0200, Christophe JAILLET wrote:
> Le 24/07/2023 à 07:29, Dmitry Torokhov a écrit :
> > From: Yangtao Li <frank.li@vivo.com>
> > +
> > + err = devm_led_classdev_register(dev, &pwm->cdev);
^^^^^^^^^^^^^^
...
> > -
> > - for (i = 0; i < 3; i++)
> > - if (lm->pwm[i].enabled)
> > - led_classdev_unregister(&lm->pwm[i].cdev);
>
> this look wrong.
Why? It will be cleared up by devm.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 2/2] Input: lm8323 - convert to use devm_* api
From: Christophe JAILLET @ 2023-07-24 18:53 UTC (permalink / raw)
To: Dmitry Torokhov, linux-input; +Cc: Yangtao Li, linux-kernel
In-Reply-To: <20230724052901.350240-2-dmitry.torokhov@gmail.com>
Le 24/07/2023 à 07:29, Dmitry Torokhov a écrit :
> From: Yangtao Li <frank.li@vivo.com>
>
> Use devm_* api to simplify code, this makes it unnecessary to explicitly
> release resources.
>
> Signed-off-by: Yangtao Li <frank.li@vivo.com>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> drivers/input/keyboard/lm8323.c | 77 +++++++++++----------------------
> 1 file changed, 26 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/input/keyboard/lm8323.c b/drivers/input/keyboard/lm8323.c
> index d5195415533a..7bee93e9b0f5 100644
> --- a/drivers/input/keyboard/lm8323.c
> +++ b/drivers/input/keyboard/lm8323.c
> @@ -556,6 +556,7 @@ static int init_pwm(struct lm8323_chip *lm, int id, struct device *dev,
> const char *name)
> {
> struct lm8323_pwm *pwm;
> + int err;
>
> BUG_ON(id > 3);
>
> @@ -575,9 +576,11 @@ static int init_pwm(struct lm8323_chip *lm, int id, struct device *dev,
> pwm->cdev.name = name;
> pwm->cdev.brightness_set = lm8323_pwm_set_brightness;
> pwm->cdev.groups = lm8323_pwm_groups;
> - if (led_classdev_register(dev, &pwm->cdev) < 0) {
> - dev_err(dev, "couldn't register PWM %d\n", id);
> - return -1;
> +
> + err = devm_led_classdev_register(dev, &pwm->cdev);
> + if (err) {
> + dev_err(dev, "couldn't register PWM %d: %d\n", id, err);
> + return err;
> }
> pwm->enabled = true;
> }
> @@ -585,8 +588,6 @@ static int init_pwm(struct lm8323_chip *lm, int id, struct device *dev,
> return 0;
> }
>
> -static struct i2c_driver lm8323_i2c_driver;
> -
> static ssize_t lm8323_show_disable(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> @@ -648,12 +649,13 @@ static int lm8323_probe(struct i2c_client *client)
> return -EINVAL;
> }
>
> - lm = kzalloc(sizeof *lm, GFP_KERNEL);
> - idev = input_allocate_device();
> - if (!lm || !idev) {
> - err = -ENOMEM;
> - goto fail1;
> - }
> + lm = devm_kzalloc(&client->dev, sizeof(*lm), GFP_KERNEL);
> + if (!lm)
> + return -ENOMEM;
> +
> + idev = devm_input_allocate_device(&client->dev);
> + if (!idev)
> + return -ENOMEM;
>
> lm->client = client;
> lm->idev = idev;
> @@ -669,8 +671,10 @@ static int lm8323_probe(struct i2c_client *client)
>
> lm8323_reset(lm);
>
> - /* Nothing's set up to service the IRQ yet, so just spin for max.
> - * 100ms until we can configure. */
> + /*
> + * Nothing's set up to service the IRQ yet, so just spin for max.
> + * 100ms until we can configure.
> + */
> tmo = jiffies + msecs_to_jiffies(100);
> while (lm8323_read(lm, LM8323_CMD_READ_INT, data, 1) == 1) {
> if (data[0] & INT_NOINIT)
> @@ -690,15 +694,14 @@ static int lm8323_probe(struct i2c_client *client)
> /* If a true probe check the device */
> if (lm8323_read_id(lm, data) != 0) {
> dev_err(&client->dev, "device not found\n");
> - err = -ENODEV;
> - goto fail1;
> + return -ENODEV;
> }
>
> for (pwm = 0; pwm < LM8323_NUM_PWMS; pwm++) {
> err = init_pwm(lm, pwm + 1, &client->dev,
> pdata->pwm_names[pwm]);
> - if (err < 0)
> - goto fail2;
> + if (err)
> + return err;
> }
>
> lm->kp_enabled = true;
> @@ -722,14 +725,16 @@ static int lm8323_probe(struct i2c_client *client)
> err = input_register_device(idev);
> if (err) {
> dev_dbg(&client->dev, "error registering input device\n");
> - goto fail2;
> + return err;
> }
>
> - err = request_threaded_irq(client->irq, NULL, lm8323_irq,
> - IRQF_TRIGGER_LOW|IRQF_ONESHOT, "lm8323", lm);
> + err = devm_request_threaded_irq(&client->dev, client->irq,
> + NULL, lm8323_irq,
> + IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> + "lm8323", lm);
> if (err) {
> dev_err(&client->dev, "could not get IRQ %d\n", client->irq);
> - goto fail3;
> + return err;
> }
>
> i2c_set_clientdata(client, lm);
> @@ -738,35 +743,6 @@ static int lm8323_probe(struct i2c_client *client)
> enable_irq_wake(client->irq);
>
> return 0;
> -
> -fail3:
> - input_unregister_device(idev);
> - idev = NULL;
> -fail2:
> - while (--pwm >= 0)
> - if (lm->pwm[pwm].enabled)
> - led_classdev_unregister(&lm->pwm[pwm].cdev);
This and...
> -fail1:
> - input_free_device(idev);
> - kfree(lm);
> - return err;
> -}
> -
> -static void lm8323_remove(struct i2c_client *client)
> -{
> - struct lm8323_chip *lm = i2c_get_clientdata(client);
> - int i;
> -
> - disable_irq_wake(client->irq);
> - free_irq(client->irq, lm);
> -
> - input_unregister_device(lm->idev);
> -
> - for (i = 0; i < 3; i++)
> - if (lm->pwm[i].enabled)
> - led_classdev_unregister(&lm->pwm[i].cdev);
this look wrong.
What you left for lm8323 looks correct.
CJ
> -
> - kfree(lm);
> }
>
> /*
> @@ -827,7 +803,6 @@ static struct i2c_driver lm8323_i2c_driver = {
> .dev_groups = lm8323_groups,
> },
> .probe = lm8323_probe,
> - .remove = lm8323_remove,
> .id_table = lm8323_id,
> };
> MODULE_DEVICE_TABLE(i2c, lm8323_id);
^ permalink raw reply
* Re: [PATCH] dt-bindings: input: convert syna,rmi4 to DT schema
From: Krzysztof Kozlowski @ 2023-07-24 17:15 UTC (permalink / raw)
To: Rob Herring
Cc: Dmitry Torokhov, Krzysztof Kozlowski, Conor Dooley,
Jason A. Donenfeld, Matthias Schiffer, Vincent Huang, linux-input,
devicetree, linux-kernel
In-Reply-To: <20230724155802.GA3609728-robh@kernel.org>
On 24/07/2023 17:58, Rob Herring wrote:
> On Thu, Jul 20, 2023 at 01:00:08PM +0200, Krzysztof Kozlowski wrote:
>
>> + required:
>> + - reg
>> +
>> + "^rmi4-f[0-9a-z]+@[0-9a-z]+$":
>
> a-f in both places.
>
Ack
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH] Input: xpad - Add HyperX Clutch Gladiate support
From: Rahul Rameshbabu @ 2023-07-24 17:02 UTC (permalink / raw)
To: Nguyen, Max
Cc: HP Dev, Dmitry Torokhov, linux-input@vger.kernel.org,
TOLEDANES, CHRIS, Ng, Carl
In-Reply-To: <MW4PR84MB178025C2FCDAE078762D0CCAEB02A@MW4PR84MB1780.NAMPRD84.PROD.OUTLOOK.COM>
On Mon, 24 Jul, 2023 16:40:14 +0000 "Nguyen, Max" <maxwell.nguyen@hyperx.com> wrote:
> Hi Rahul,
>
>
>
> My apologies for the resend. We resent due to white space issues in the previous patches. I will resend again and address the comments you
> mentioned below.
>
No worries. The point on resending was minor. If you make the changes to
the patch, be sure to make it a v2 rather than a resend.
>
>
> The purpose of adding Xbox 360 specified class is to cover future products that will need this protocol support ahead of project launch date.
> This is purely for testing during development.
>
Yeah, lets omit this from submission to the mainline kernel till the
future products with Xbox 360 support are implemented. Even then if they
support Xbox One devices, you do not need Xbox 360 vendor-specific class
support since you can use Xbox One class support. I have little knobs
like this too that I keep internal for testing purposes.
>
>
> Regards,
>
> Max
>
In general, this type of reply is considered top-posting and is looked
down upon in the kernel community. I see you are likely using Outlook
which is not the best mailing option for interacting with the kernel
mailing lists.
* https://people.kernel.org/tglx/
* https://www.kernel.org/doc/html/latest/process/email-clients.html
-- Rahul Rameshbabu
>
>
> From: Rahul Rameshbabu <rrameshbabu@nvidia.com>
> Sent: Friday, July 21, 2023 7:46 PM
> To: HP Dev <hphyperxdev@gmail.com>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>; linux-input@vger.kernel.org; TOLEDANES, CHRIS <chris.toledanes@hp.com>; Ng, Carl
> <carl.ng@hp.com>; Nguyen, Max <maxwell.nguyen@hp.com>
> Subject: Re: [PATCH] Input: xpad - Add HyperX Clutch Gladiate support
>
>
>
> CAUTION: External Email
>
> On Fri, 21 Jul, 2023 18:05:32 -0700 HP Dev <hphyperxdev@gmail.com> wrote:
>> Add HyperX controller support to xpad_device and xpad_table.
>
> This patch appears to be a resend of a previous patch but does not
> follow the right practice for patch resends.
>
> https://docs.kernel.org/process/submitting-patches.html#don-t-get-discouraged-or-impatient
>
> https://lore.kernel.org/linux-input/ZKxVBULWtM30ZJ7D@google.com/
>
>>
>> Reported-by: Chris Toledanes <chris.toledanes@hp.com>
>
> I think this may not be an accurate use of the Reported-by: git trailer.
> My guess is you would want to use the Suggested-by: trailer here. This
> patch does not solve a "regression" in the xpad driver that was reported
> by someone or some automation but rather adds a new controller to be
> probed by the driver.
>
> https://docs.kernel.org/process/submitting-patches.html#using-reported-by-tested-by-reviewed-by-suggested-by-and-fixes
>
>> Reviewed-by: Carl Ng <carl.ng@hp.com>
>> Signed-off-by: Max Nguyen <maxwell.nguyen@hp.com>
>> ---
>> drivers/input/joystick/xpad.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
>> index cdb193317c3b..dfddb0bba8d8 100644
>> --- a/drivers/input/joystick/xpad.c
>> +++ b/drivers/input/joystick/xpad.c
>> @@ -130,6 +130,7 @@ static const struct xpad_device {
>> { 0x0079, 0x18d4, "GPD Win 2 X-Box Controller", 0, XTYPE_XBOX360 },
>> { 0x03eb, 0xff01, "Wooting One (Legacy)", 0, XTYPE_XBOX360 },
>> { 0x03eb, 0xff02, "Wooting Two (Legacy)", 0, XTYPE_XBOX360 },
>> + { 0x03f0, 0x0495, "HyperX Clutch Gladiate", 0, XTYPE_XBOXONE },
>> { 0x044f, 0x0f00, "Thrustmaster Wheel", 0, XTYPE_XBOX },
>> { 0x044f, 0x0f03, "Thrustmaster Wheel", 0, XTYPE_XBOX },
>> { 0x044f, 0x0f07, "Thrustmaster, Inc. Controller", 0, XTYPE_XBOX },
>> @@ -457,6 +458,8 @@ static const struct usb_device_id xpad_table[] = {
>> { USB_INTERFACE_INFO('X', 'B', 0) }, /* Xbox USB-IF not-approved class */
>> XPAD_XBOX360_VENDOR(0x0079), /* GPD Win 2 controller */
>> XPAD_XBOX360_VENDOR(0x03eb), /* Wooting Keyboards (Legacy) */
>> + XPAD_XBOX360_VENDOR(0x03f0), /* HP HyperX XBox 360 Controllers */
>
> There are no HP HyperX Xbox 360 controllers previously supported or
> added in this patch. I think the above line should be removed since you
> only have a XTYPE_XBOXONE type controller. You probably do not need to
> match against Xbox 360 vendor-specific class since you only implement an
> Xbox One controller.
>
>> + XPAD_XBOXONE_VENDOR(0x03f0), /* HP HyperX Xbox One Controllers */
>> XPAD_XBOX360_VENDOR(0x044f), /* Thrustmaster Xbox 360 controllers */
>> XPAD_XBOX360_VENDOR(0x045e), /* Microsoft Xbox 360 controllers */
>> XPAD_XBOXONE_VENDOR(0x045e), /* Microsoft Xbox One controllers */
>
> -- Rahul Rameshbabu
^ permalink raw reply
* Re: [PATCH] HID: logitech-hidpp: add support for the Logitech G Pro X mouse
From: Hamza Mahfooz @ 2023-07-24 16:55 UTC (permalink / raw)
To: linux-kernel
Cc: Filipe Laíns, Bastien Nocera, Jiri Kosina,
Benjamin Tissoires, linux-input
In-Reply-To: <20230724163032.15226-1-someguy@effective-light.com>
On Mon, Jul 24 2023 at 12:30:32 PM -04:00:00, Hamza Mahfooz
<someguy@effective-light.com> wrote:
> The Logitech G Pro X mouse uses the HID++ protocol and supports
> battery
> reporting. So, add it to the list of supported devices.
>
> Signed-off-by: Hamza Mahfooz <someguy@effective-light.com>
Actually on further inspection there is already a series that address
this issue (i.e.
https://lore.kernel.org/r/20230716182320.85483-1-mavchatz@protonmail.com/),
so please ignore this patch.
> ---
> drivers/hid/hid-logitech-hidpp.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/hid/hid-logitech-hidpp.c
> b/drivers/hid/hid-logitech-hidpp.c
> index 129b01be488d..34fc4f7b254a 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c
> @@ -4620,6 +4620,8 @@ static const struct hid_device_id
> hidpp_devices[] = {
> .driver_data = HIDPP_QUIRK_CLASS_G920 |
> HIDPP_QUIRK_FORCE_OUTPUT_REPORTS },
> { /* Logitech G Pro Gaming Mouse over USB */
> HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC088) },
> + { /* Logitech G Pro X Superlight Gaming Mouse over USB */
> + HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC094) },
>
> { /* G935 Gaming Headset */
> HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0x0a87),
> --
> 2.41.0
>
^ permalink raw reply
* [PATCH] HID: logitech-hidpp: add support for the Logitech G Pro X mouse
From: Hamza Mahfooz @ 2023-07-24 16:30 UTC (permalink / raw)
To: linux-kernel
Cc: Hamza Mahfooz, Filipe Laíns, Bastien Nocera, Jiri Kosina,
Benjamin Tissoires, linux-input
The Logitech G Pro X mouse uses the HID++ protocol and supports battery
reporting. So, add it to the list of supported devices.
Signed-off-by: Hamza Mahfooz <someguy@effective-light.com>
---
drivers/hid/hid-logitech-hidpp.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 129b01be488d..34fc4f7b254a 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -4620,6 +4620,8 @@ static const struct hid_device_id hidpp_devices[] = {
.driver_data = HIDPP_QUIRK_CLASS_G920 | HIDPP_QUIRK_FORCE_OUTPUT_REPORTS },
{ /* Logitech G Pro Gaming Mouse over USB */
HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC088) },
+ { /* Logitech G Pro X Superlight Gaming Mouse over USB */
+ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC094) },
{ /* G935 Gaming Headset */
HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0x0a87),
--
2.41.0
^ permalink raw reply related
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