* Re: [PATCH 1/2] Input: cs40l26: Support for CS40L26 Boosted Haptic Amplifier
From: Fred Treven @ 2023-04-14 20:51 UTC (permalink / raw)
To: Jeff LaBundy
Cc: Charles Keepax, dmitry.torokhov@gmail.com, Ben Bright,
James Ogletree, lee@kernel.org, jdelvare@suse.de, joel@jms.id.au,
cy_huang@richtek.com, rdunlap@infradead.org,
eajames@linux.ibm.com, ping.bai@nxp.com, msp@baylibre.com,
arnd@arndb.de, bartosz.golaszewski@linaro.org,
linux-kernel@vger.kernel.org, linux-input@vger.kernel.org,
patches@opensource.cirrus.com
In-Reply-To: <ZDYakQMOPsPTbGe0@nixie71>
>> +
>> + return cs40l26_probe(cs40l26, pdata);
>> +}
>> +
>> +static void cs40l26_i2c_remove(struct i2c_client *client)
>> +{
>> + struct cs40l26_private *cs40l26 = i2c_get_clientdata(client);
>> +
>> + cs40l26_remove(cs40l26);
>> +}
>> +
>> +static struct i2c_driver cs40l26_i2c_driver = {
>> + .driver = {
>> + .name = "cs40l26",
>> + .of_match_table = cs40l26_of_match,
>> + .pm = &cs40l26_pm_ops,
>
> Please guard this with the new pm_sleep_ptr(), as not all platforms would
> define CONFIG_PM. More comments in the core driver.
Understood, and I certainly agree with this change. One thing I’m unsure of is what the effect would be on the driver if CONFIG_PM is not set in the .config. Surely, this driver would not work as expected right? Should I add a dependency in the kconfig to avoid building the driver without CONFIG_PM?
>
>> +{
>> + size_t len_words = len_bytes / sizeof(__be32);
>> + struct cs_dsp_coeff_ctl *ctl;
>> + __be32 *val;
>> + int i, ret;
>> +
>> + ctl = cs_dsp_get_ctl(dsp, name, WMFW_ADSP2_XM, algo_id);
>> + if (IS_ERR_OR_NULL(ctl)) {
>> + dev_err(dsp->dev, "Failed to find fw ctl %s\n", name);
>> + return -ENOENT;
>> + }
>> +
>> + val = kzalloc(len_bytes, GFP_KERNEL);
>> + if (!val)
>> + return -ENOMEM;
>> +
>> + for (i = 0; i < len_words; i++)
>> + val[i] = cpu_to_be32(buf[i]);
>> +
>> + ret = cs_dsp_coeff_write_ctrl(ctl, off_words, val, len_bytes);
>> + if (ret)
>> + dev_err(dsp->dev, "Failed to write fw ctl %s: %d\n", name, ret);
>> +
>> + kfree(val);
>> +
>> + return ret;
>> +}
>> +
>> +static inline int cs40l26_fw_ctl_write(struct cs_dsp *dsp, const char * const name,
>> + unsigned int algo_id, u32 val)
>> +{
>> + return cs40l26_fw_ctl_write_raw(dsp, name, algo_id, 0, sizeof(u32), &val);
>> +}
>> +
>> +static int cs40l26_fw_ctl_read_raw(struct cs_dsp *dsp, const char * const name,
>> + unsigned int algo_id, unsigned int off_words, size_t len_bytes, u32 *buf)
>> +{
>> + size_t len_words = len_bytes / sizeof(u32);
>> + struct cs_dsp_coeff_ctl *ctl;
>> + int i, ret;
>> +
>> + ctl = cs_dsp_get_ctl(dsp, name, WMFW_ADSP2_XM, algo_id);
>> + if (IS_ERR_OR_NULL(ctl)) {
>> + dev_err(dsp->dev, "Failed to find fw ctl %s\n", name);
>> + return -ENOENT;
>> + }
>> +
>> + ret = cs_dsp_coeff_read_ctrl(ctl, off_words, buf, len_bytes);
>> + if (ret) {
>> + dev_err(dsp->dev, "Failed to read fw ctl %s: %d\n", name, ret);
>> + return ret;
>> + }
>> +
>> + for (i = 0; i < len_words; i++)
>> + buf[i] = be32_to_cpu(buf[i]);
>> +
>> + return 0;
>> +}
>> +
>> +static inline int cs40l26_fw_ctl_read(struct cs_dsp *dsp, const char * const name,
>> + unsigned int algo_id, u32 *buf)
>> +{
>> + return cs40l26_fw_ctl_read_raw(dsp, name, algo_id, 0, sizeof(u32), buf);
>> +}
>
> None of these four functions seem particularly specific to L26; is there any
> reason they don't belong in cs_dsp or wm_adsp? In fact, some of the functions
> throughout those drivers seem to be doing similar work.
>
> Maybe out of scope for this particular submission, but is there not any room
> for re-use here?
>
I wanted to avoid making too many changes to the firmware drivers in this initial submission, and I think I’d like to keep it as-is for now, but I think moving this combination to cs_dsp is the right move. Let me know if you feel that it would be better to make the change now rather than later.
> On Tue, Apr 11, 2023 at 09:27:08AM +0000, Charles Keepax wrote:
>> On Mon, Apr 10, 2023 at 07:31:56PM -0500, Jeff LaBundy wrote:
>>> On Mon, Apr 10, 2023 at 08:56:34AM +0000, Charles Keepax wrote:
>>>> On Sat, Apr 08, 2023 at 10:44:39PM -0500, Jeff LaBundy wrote:
>>>> I would far rather not have every single attempt to communicate
>>>> with the device wrapped in a retry if the communication failed
>>>> incase the device is hibernating. It seems much cleaner, and less
>>>> likely to risk odd behaviour, to know we have brought the device
>>>> out of hibernation.
>>
>>> A common way to deal with this is that of [1], where the bus calls
>>> are simply wrapped with all retry logic limited to two places (read
>>> and write). These functions could also print the register address
>>> in case of failure, solving the problem of having dozens of custom
>>> error messages thorughout the driver.
>>
>> I suspect this really comes down to a matter of taste, but my
>> thoughts would be that the code is shorter that way, but not
>> necessarily simpler. This feels far more error prone and likely
>> to encounter issues where the device hibernates at a time someone
>> hadn't properly thought through. I am far more comfortable with
>> the device is blocked from hibernating whilst the driver is
>> actively engaged with it and it keeps any special handling for
>> exiting hibernate in one place.
>
> Fair enough. I do concede that having this control in the driver as
> opposed to DSP FW is more nimble and makes it easier to respond to
> customer issues; I'm sure your battle scars will agree :)
I concur with Charles here, and it seems like you’re also ok with this so I will leave it as-is.
>>> +static int cs40l26_irq_update_mask(struct cs40l26_private *cs40l26, u32 reg, u32 val, u32 bit_mask)
>>> +{
>>> + u32 eint_reg, cur_mask, new_mask;
>>> + int ret;
>>> +
>>> + if (reg == CS40L26_IRQ1_MASK_1) {
>>> + eint_reg = CS40L26_IRQ1_EINT_1;
>>> + } else if (reg == CS40L26_IRQ1_MASK_2) {
>>> + eint_reg = CS40L26_IRQ1_EINT_2;
>>> + } else {
>>> + dev_err(cs40l26->dev, "Invalid IRQ mask reg: 0x%08X\n", reg);
>>> + return -EINVAL;
>>> + }
>>> +
>>> + ret = regmap_read(cs40l26->regmap, reg, &cur_mask);
>>> + if (ret) {
>>> + dev_err(cs40l26->dev, "Failed to get IRQ mask\n");
>>
>> Having a custom error message for every possible failed register read
>> does not ultimately aid in debugging and unnecessarily grows the size
>> of the driver.
>>
>> If a specific message is absolutely necessary, then add wrappers around
>> regmap_read/write and print the failed address. However, this does not
>> seem necessary either. Simply propagate the error code all the way up
>> to the caller, whether it is probe or a sysfs attribute.
>>
>> Stated another way:
>>
>> error = regmap_...(...);
>> if (error)
>> return error;
>>
>
> Not sure I feel super strongly on this one, but I do find when
> debugging issues on drivers that do this that I usually end up
> adding the printks back in.
I went ahead and implemented the change Jeff suggested here; it does streamline the driver to a good degree, and I think it’s worth making the adjustment.
>>> +static int cs40l26_dsp_state_get(struct cs40l26_private *cs40l26, u8 *state)
>>> +{
>>> + bool mutex_available = !mutex_is_locked(&cs40l26->dsp.pwr_lock);
>>
>> This is dangerous and a sign that locks are not properly managed. What would
>> be a case where you do not know the state of the lock upon entering this function?
>> If you do not know whether the mutex is locked inside this function, it is not the
>> proper place to grab it.
>>
>
> Yes I whole heartedly agree here this should not be done this
> way.
I certainly understand the concern here, but I wanted to provide some context. Since cs40l26_dsp_state_get() is called both in the cs_dsp_pre_run() callback as well as outside of this, there are instances where pwr_lock needs to be grabbed and times when it is already taken (in the instance of the callback invocation). Because of this, attempting to take the lock during pre_run causes a deadlock. I felt that it was quite clear when the lock would be available and when it wouldn’t be and to avoid the deadlock, I checked whether or not the lock was available. What do y’all feel is the best way to handle this? Some separate variable to determine if we are in the pre_run sequence or not? A separate function for dsp_state_get() for when it is not invoked from the callback?
>
>> +
>> + cs40l26_pm_runtime_teardown(cs40l26);
>> +
>> + if (cs40l26->dsp.running)
>> + cs_dsp_stop(&cs40l26->dsp);
>> + if (cs40l26->dsp.booted)
>> + cs_dsp_power_down(&cs40l26->dsp);
>> + if (&cs40l26->dsp)
>> + cs_dsp_remove(&cs40l26->dsp);
>> +
>> + if (cs40l26->vibe_workqueue) {
>> + cancel_work_sync(&cs40l26->erase_work);
>> + cancel_work_sync(&cs40l26->set_gain_work);
>> + cancel_work_sync(&cs40l26->upload_work);
>> + cancel_work_sync(&cs40l26->vibe_start_work);
>> + cancel_work_sync(&cs40l26->vibe_stop_work);
>> + destroy_workqueue(cs40l26->vibe_workqueue);
>> + }
>> +
>> + mutex_destroy(&cs40l26->lock);
>
> This ultimately does nothing.
Could you please clarify a bit what you mean here? Does the mutex not need to be destroyed explicitly? What about the work queue? Is it canceled/destroyed automatically when the module is removed? Are the cs_dsp functions not necessary to gracefully stop the DSP etc.?
In v2 of this patch, I plan to implement all of the suggestions in your comments that I didn’t question or disagree with here. Thanks for your in-depth analysis.
Best regards,
Fred
^ permalink raw reply
* [dtor-input:next] BUILD SUCCESS c55d84fb2bd89fe2ad56768ead90eb1050581d29
From: kernel test robot @ 2023-04-14 19:57 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: c55d84fb2bd89fe2ad56768ead90eb1050581d29 dt-bindings: input: pwm-beeper: convert to dt schema
elapsed time: 722m
configs tested: 231
configs skipped: 18
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-r021-20230414 gcc
alpha randconfig-r023-20230414 gcc
alpha randconfig-r026-20230414 gcc
alpha randconfig-r034-20230412 gcc
arc allyesconfig gcc
arc defconfig gcc
arc randconfig-r005-20230409 gcc
arc randconfig-r013-20230410 gcc
arc randconfig-r025-20230414 gcc
arc randconfig-r032-20230410 gcc
arc randconfig-r033-20230409 gcc
arc randconfig-r034-20230413 gcc
arc randconfig-r043-20230409 gcc
arc randconfig-r043-20230410 gcc
arc randconfig-r043-20230412 gcc
arc randconfig-r043-20230413 gcc
arm allmodconfig gcc
arm allyesconfig gcc
arm buildonly-randconfig-r005-20230410 clang
arm defconfig gcc
arm pxa3xx_defconfig gcc
arm randconfig-r021-20230413 gcc
arm randconfig-r022-20230414 clang
arm randconfig-r031-20230413 clang
arm randconfig-r033-20230412 gcc
arm randconfig-r046-20230409 clang
arm randconfig-r046-20230410 clang
arm randconfig-r046-20230412 clang
arm randconfig-r046-20230413 gcc
arm rpc_defconfig gcc
arm64 allyesconfig gcc
arm64 buildonly-randconfig-r005-20230409 clang
arm64 defconfig gcc
arm64 randconfig-r002-20230410 clang
arm64 randconfig-r004-20230414 clang
arm64 randconfig-r035-20230409 clang
arm64 randconfig-r036-20230410 clang
csky buildonly-randconfig-r002-20230409 gcc
csky buildonly-randconfig-r006-20230409 gcc
csky defconfig gcc
csky randconfig-r001-20230414 gcc
csky randconfig-r005-20230414 gcc
csky randconfig-r026-20230413 gcc
csky randconfig-r031-20230409 gcc
hexagon buildonly-randconfig-r003-20230413 clang
hexagon buildonly-randconfig-r004-20230413 clang
hexagon randconfig-r002-20230414 clang
hexagon randconfig-r011-20230410 clang
hexagon randconfig-r033-20230409 clang
hexagon randconfig-r041-20230409 clang
hexagon randconfig-r041-20230410 clang
hexagon randconfig-r041-20230412 clang
hexagon randconfig-r041-20230413 clang
hexagon randconfig-r045-20230409 clang
hexagon randconfig-r045-20230410 clang
hexagon randconfig-r045-20230412 clang
hexagon randconfig-r045-20230413 clang
i386 allnoconfig clang
i386 allyesconfig gcc
i386 buildonly-randconfig-r003-20230410 clang
i386 debian-10.3 gcc
i386 defconfig gcc
i386 randconfig-a001-20230410 clang
i386 randconfig-a002-20230410 clang
i386 randconfig-a003-20230410 clang
i386 randconfig-a004-20230410 clang
i386 randconfig-a005-20230410 clang
i386 randconfig-a006-20230410 clang
i386 randconfig-a011-20230410 gcc
i386 randconfig-a012-20230410 gcc
i386 randconfig-a013-20230410 gcc
i386 randconfig-a014-20230410 gcc
i386 randconfig-a015-20230410 gcc
i386 randconfig-a016-20230410 gcc
i386 randconfig-r036-20230410 clang
ia64 allmodconfig gcc
ia64 defconfig gcc
ia64 randconfig-r001-20230410 gcc
ia64 randconfig-r015-20230410 gcc
ia64 randconfig-r024-20230410 gcc
ia64 randconfig-r025-20230409 gcc
ia64 randconfig-r031-20230409 gcc
ia64 randconfig-r034-20230410 gcc
ia64 randconfig-r036-20230413 gcc
loongarch allmodconfig gcc
loongarch allnoconfig gcc
loongarch buildonly-randconfig-r001-20230413 gcc
loongarch defconfig gcc
loongarch randconfig-r004-20230409 gcc
loongarch randconfig-r016-20230410 gcc
loongarch randconfig-r021-20230410 gcc
loongarch randconfig-r032-20230409 gcc
m68k allmodconfig gcc
m68k buildonly-randconfig-r006-20230413 gcc
m68k defconfig gcc
m68k randconfig-r002-20230409 gcc
m68k randconfig-r005-20230414 gcc
m68k randconfig-r021-20230410 gcc
m68k randconfig-r024-20230409 gcc
m68k randconfig-r036-20230409 gcc
m68k sun3x_defconfig gcc
microblaze randconfig-r002-20230413 gcc
microblaze randconfig-r005-20230410 gcc
microblaze randconfig-r023-20230410 gcc
microblaze randconfig-r024-20230414 gcc
microblaze randconfig-r025-20230413 gcc
microblaze randconfig-r032-20230410 gcc
microblaze randconfig-r034-20230409 gcc
mips allmodconfig gcc
mips allyesconfig gcc
mips cu1000-neo_defconfig clang
mips omega2p_defconfig clang
mips randconfig-r023-20230409 clang
mips randconfig-r024-20230413 gcc
mips randconfig-r035-20230410 gcc
nios2 defconfig gcc
nios2 randconfig-r002-20230414 gcc
nios2 randconfig-r006-20230413 gcc
nios2 randconfig-r011-20230409 gcc
nios2 randconfig-r022-20230413 gcc
nios2 randconfig-r033-20230413 gcc
nios2 randconfig-r035-20230409 gcc
openrisc buildonly-randconfig-r002-20230410 gcc
openrisc buildonly-randconfig-r004-20230410 gcc
openrisc randconfig-r003-20230409 gcc
openrisc randconfig-r003-20230413 gcc
openrisc randconfig-r006-20230410 gcc
openrisc randconfig-r023-20230413 gcc
openrisc randconfig-r024-20230410 gcc
parisc defconfig gcc
parisc randconfig-r001-20230413 gcc
parisc randconfig-r012-20230409 gcc
parisc randconfig-r022-20230409 gcc
parisc randconfig-r031-20230412 gcc
parisc randconfig-r032-20230409 gcc
parisc randconfig-r033-20230409 gcc
parisc64 defconfig gcc
powerpc allmodconfig gcc
powerpc allnoconfig gcc
powerpc kilauea_defconfig clang
powerpc makalu_defconfig gcc
powerpc mpc7448_hpc2_defconfig gcc
powerpc mvme5100_defconfig clang
powerpc ppc44x_defconfig clang
powerpc randconfig-r003-20230410 clang
powerpc randconfig-r026-20230410 gcc
powerpc randconfig-r034-20230409 clang
powerpc randconfig-r035-20230413 gcc
powerpc wii_defconfig gcc
riscv allmodconfig gcc
riscv allnoconfig gcc
riscv buildonly-randconfig-r004-20230409 gcc
riscv defconfig gcc
riscv randconfig-r014-20230410 gcc
riscv randconfig-r031-20230410 clang
riscv randconfig-r042-20230409 gcc
riscv randconfig-r042-20230410 gcc
riscv randconfig-r042-20230412 gcc
riscv randconfig-r042-20230413 clang
riscv rv32_defconfig gcc
s390 allmodconfig gcc
s390 allyesconfig gcc
s390 defconfig gcc
s390 randconfig-r001-20230409 clang
s390 randconfig-r003-20230414 clang
s390 randconfig-r004-20230410 clang
s390 randconfig-r004-20230414 clang
s390 randconfig-r005-20230413 gcc
s390 randconfig-r023-20230409 gcc
s390 randconfig-r033-20230410 clang
s390 randconfig-r035-20230410 clang
s390 randconfig-r044-20230409 gcc
s390 randconfig-r044-20230410 gcc
s390 randconfig-r044-20230412 gcc
s390 randconfig-r044-20230413 clang
sh allmodconfig gcc
sh buildonly-randconfig-r001-20230410 gcc
sh buildonly-randconfig-r005-20230413 gcc
sh randconfig-r003-20230414 gcc
sh randconfig-r006-20230414 gcc
sh randconfig-r012-20230410 gcc
sh randconfig-r014-20230409 gcc
sh randconfig-r015-20230409 gcc
sh randconfig-r023-20230410 gcc
sh randconfig-r026-20230409 gcc
sh randconfig-r032-20230410 gcc
sh randconfig-r032-20230412 gcc
sh randconfig-r034-20230410 gcc
sh se7722_defconfig gcc
sh secureedge5410_defconfig gcc
sh sh7785lcr_32bit_defconfig gcc
sh titan_defconfig gcc
sparc defconfig gcc
sparc randconfig-r031-20230409 gcc
sparc randconfig-r035-20230409 gcc
sparc randconfig-r036-20230409 gcc
sparc64 buildonly-randconfig-r001-20230409 gcc
sparc64 buildonly-randconfig-r002-20230413 gcc
sparc64 randconfig-r006-20230409 gcc
sparc64 randconfig-r021-20230409 gcc
sparc64 randconfig-r022-20230410 gcc
sparc64 randconfig-r031-20230410 gcc
um i386_defconfig gcc
um x86_64_defconfig gcc
x86_64 allnoconfig gcc
x86_64 allyesconfig gcc
x86_64 defconfig gcc
x86_64 kexec gcc
x86_64 randconfig-a001-20230410 clang
x86_64 randconfig-a001 clang
x86_64 randconfig-a002-20230410 clang
x86_64 randconfig-a003-20230410 clang
x86_64 randconfig-a003 clang
x86_64 randconfig-a004-20230410 clang
x86_64 randconfig-a005-20230410 clang
x86_64 randconfig-a005 clang
x86_64 randconfig-a006-20230410 clang
x86_64 randconfig-a011-20230410 gcc
x86_64 randconfig-a012-20230410 gcc
x86_64 randconfig-a013-20230410 gcc
x86_64 randconfig-a014-20230410 gcc
x86_64 randconfig-a015-20230410 gcc
x86_64 randconfig-a016-20230410 gcc
x86_64 randconfig-r025-20230410 gcc
x86_64 randconfig-r034-20230410 clang
x86_64 rhel-8.3 gcc
xtensa buildonly-randconfig-r006-20230410 gcc
xtensa randconfig-r033-20230410 gcc
xtensa randconfig-r035-20230410 gcc
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
^ permalink raw reply
* [dtor-input:for-linus] BUILD SUCCESS 5dc63e56a9cf8df0b59c234a505a1653f1bdf885
From: kernel test robot @ 2023-04-14 19:57 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git for-linus
branch HEAD: 5dc63e56a9cf8df0b59c234a505a1653f1bdf885 Input: cyttsp5 - fix sensing configuration data structure
elapsed time: 723m
configs tested: 142
configs skipped: 5
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-r025-20230410 gcc
alpha randconfig-r034-20230412 gcc
arc allyesconfig gcc
arc defconfig gcc
arc randconfig-r026-20230409 gcc
arc randconfig-r031-20230410 gcc
arc randconfig-r032-20230410 gcc
arc randconfig-r033-20230409 gcc
arc randconfig-r036-20230413 gcc
arc randconfig-r043-20230409 gcc
arc randconfig-r043-20230410 gcc
arc randconfig-r043-20230412 gcc
arc randconfig-r043-20230413 gcc
arm allmodconfig gcc
arm allyesconfig gcc
arm defconfig gcc
arm randconfig-r025-20230409 clang
arm randconfig-r033-20230412 gcc
arm randconfig-r046-20230409 clang
arm randconfig-r046-20230410 clang
arm randconfig-r046-20230412 clang
arm randconfig-r046-20230413 gcc
arm64 allyesconfig gcc
arm64 defconfig gcc
arm64 randconfig-r024-20230409 gcc
csky defconfig gcc
csky randconfig-r031-20230409 gcc
csky randconfig-r034-20230414 gcc
hexagon randconfig-r033-20230414 clang
hexagon randconfig-r041-20230409 clang
hexagon randconfig-r041-20230410 clang
hexagon randconfig-r041-20230412 clang
hexagon randconfig-r041-20230413 clang
hexagon randconfig-r045-20230409 clang
hexagon randconfig-r045-20230410 clang
hexagon randconfig-r045-20230412 clang
hexagon randconfig-r045-20230413 clang
i386 allnoconfig clang
i386 allyesconfig gcc
i386 debian-10.3 gcc
i386 defconfig gcc
i386 randconfig-a001-20230410 clang
i386 randconfig-a002-20230410 clang
i386 randconfig-a003-20230410 clang
i386 randconfig-a004-20230410 clang
i386 randconfig-a005-20230410 clang
i386 randconfig-a006-20230410 clang
i386 randconfig-a011-20230410 gcc
i386 randconfig-a012-20230410 gcc
i386 randconfig-a013-20230410 gcc
i386 randconfig-a014-20230410 gcc
i386 randconfig-a015-20230410 gcc
i386 randconfig-a016-20230410 gcc
ia64 allmodconfig gcc
ia64 defconfig gcc
ia64 randconfig-r034-20230410 gcc
loongarch allmodconfig gcc
loongarch allnoconfig gcc
loongarch defconfig gcc
loongarch randconfig-r021-20230409 gcc
loongarch randconfig-r026-20230410 gcc
loongarch randconfig-r032-20230414 gcc
loongarch randconfig-r034-20230410 gcc
loongarch randconfig-r035-20230409 gcc
m68k allmodconfig gcc
m68k defconfig gcc
m68k randconfig-r022-20230410 gcc
m68k randconfig-r036-20230409 gcc
microblaze randconfig-r023-20230409 gcc
microblaze randconfig-r036-20230410 gcc
mips allmodconfig gcc
mips allyesconfig gcc
mips randconfig-r032-20230413 clang
mips randconfig-r034-20230409 gcc
nios2 defconfig gcc
nios2 randconfig-r034-20230413 gcc
nios2 randconfig-r035-20230409 gcc
openrisc randconfig-r036-20230414 gcc
parisc defconfig gcc
parisc randconfig-r031-20230412 gcc
parisc randconfig-r032-20230409 gcc
parisc64 defconfig gcc
powerpc allmodconfig gcc
powerpc allnoconfig gcc
powerpc mvme5100_defconfig clang
powerpc randconfig-r021-20230410 gcc
powerpc randconfig-r033-20230413 gcc
riscv allmodconfig gcc
riscv allnoconfig gcc
riscv defconfig gcc
riscv randconfig-r024-20230410 gcc
riscv randconfig-r035-20230410 clang
riscv randconfig-r042-20230409 gcc
riscv randconfig-r042-20230410 gcc
riscv randconfig-r042-20230412 gcc
riscv randconfig-r042-20230413 clang
riscv rv32_defconfig gcc
s390 allmodconfig gcc
s390 allyesconfig gcc
s390 defconfig gcc
s390 randconfig-r022-20230409 gcc
s390 randconfig-r035-20230414 clang
s390 randconfig-r044-20230409 gcc
s390 randconfig-r044-20230410 gcc
s390 randconfig-r044-20230412 gcc
s390 randconfig-r044-20230413 clang
sh allmodconfig gcc
sh randconfig-r032-20230412 gcc
sh randconfig-r035-20230413 gcc
sparc defconfig gcc
sparc randconfig-r031-20230409 gcc
sparc64 randconfig-r032-20230409 gcc
sparc64 randconfig-r033-20230409 gcc
sparc64 randconfig-r036-20230409 gcc
um i386_defconfig gcc
um x86_64_defconfig gcc
x86_64 allnoconfig gcc
x86_64 allyesconfig gcc
x86_64 defconfig gcc
x86_64 kexec gcc
x86_64 randconfig-a001-20230410 clang
x86_64 randconfig-a001 clang
x86_64 randconfig-a002-20230410 clang
x86_64 randconfig-a003-20230410 clang
x86_64 randconfig-a003 clang
x86_64 randconfig-a004-20230410 clang
x86_64 randconfig-a005-20230410 clang
x86_64 randconfig-a005 clang
x86_64 randconfig-a006-20230410 clang
x86_64 randconfig-a011-20230410 gcc
x86_64 randconfig-a012-20230410 gcc
x86_64 randconfig-a013-20230410 gcc
x86_64 randconfig-a014-20230410 gcc
x86_64 randconfig-a015-20230410 gcc
x86_64 randconfig-a016-20230410 gcc
x86_64 rhel-8.3 gcc
xtensa randconfig-r031-20230413 gcc
xtensa randconfig-r031-20230414 gcc
xtensa randconfig-r033-20230410 gcc
xtensa randconfig-r035-20230410 gcc
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
^ permalink raw reply
* [PATCH] HID: wacom: Check for string overflow from strscpy calls
From: Jason Gerecke @ 2023-04-14 18:22 UTC (permalink / raw)
To: linux-input, Benjamin Tissoires, Jiri Kosina
Cc: Ping Cheng, Aaron Armstrong Skomra, Joshua Dickens, Jason Gerecke,
Jason Gerecke, Ping Cheng
From: Jason Gerecke <killertofu@gmail.com>
The strscpy function is able to return an error code when a copy would
overflow the size of the destination. The copy is stopped and the buffer
terminated before overflow actually occurs so it is safe to continue
execution, but we should still produce a warning should this occur.
Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
Reviewed-by: Ping Cheng <ping.cheng@wacom.com>
---
drivers/hid/wacom_sys.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 8214896adadad..7192970d199a0 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -2224,7 +2224,9 @@ static void wacom_update_name(struct wacom *wacom, const char *suffix)
} else if (strstr(product_name, "Wacom") ||
strstr(product_name, "wacom") ||
strstr(product_name, "WACOM")) {
- strscpy(name, product_name, sizeof(name));
+ if (strscpy(name, product_name, sizeof(name)) < 0) {
+ hid_warn(wacom->hdev, "String overflow while assembling device name");
+ }
} else {
snprintf(name, sizeof(name), "Wacom %s", product_name);
}
@@ -2242,7 +2244,9 @@ static void wacom_update_name(struct wacom *wacom, const char *suffix)
if (name[strlen(name)-1] == ' ')
name[strlen(name)-1] = '\0';
} else {
- strscpy(name, features->name, sizeof(name));
+ if (strscpy(name, features->name, sizeof(name)) < 0) {
+ hid_warn(wacom->hdev, "String overflow while assembling device name");
+ }
}
snprintf(wacom_wac->name, sizeof(wacom_wac->name), "%s%s",
@@ -2500,8 +2504,10 @@ static void wacom_wireless_work(struct work_struct *work)
goto fail;
}
- strscpy(wacom_wac->name, wacom_wac1->name,
- sizeof(wacom_wac->name));
+ if (strscpy(wacom_wac->name, wacom_wac1->name,
+ sizeof(wacom_wac->name)) < 0) {
+ hid_warn(wacom->hdev, "String overflow while assembling device name");
+ }
}
return;
--
2.40.0
^ permalink raw reply related
* [PATCH] Input: support pre-stored effects
From: James Ogletree @ 2023-04-14 16:58 UTC (permalink / raw)
To: dmitry.torokhov; +Cc: linux-input, linux-kernel, James Ogletree
At present, the best way to define effects that
pre-exist in device memory is by utilizing
the custom_data field, which it was not intended
for, and requires arbitrary interpretation by
the driver to make meaningful.
Provide option for defining pre-stored effects in
device memory.
Signed-off-by: James Ogletree <james.ogletree@cirrus.com>
---
include/uapi/linux/input.h | 32 ++++++++++++++++++++++----------
1 file changed, 22 insertions(+), 10 deletions(-)
diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
index 2557eb7b0561..689e5fa10647 100644
--- a/include/uapi/linux/input.h
+++ b/include/uapi/linux/input.h
@@ -428,17 +428,27 @@ struct ff_rumble_effect {
__u16 weak_magnitude;
};
+/**
+ * struct ff_prestored_effect - defines parameters of a pre-stored force-feedback effect
+ * @index: index of effect
+ * @bank: memory bank of effect
+ */
+struct ff_prestored_effect {
+ __u16 index;
+ __u16 bank;
+};
+
/**
* struct ff_effect - defines force feedback effect
* @type: type of the effect (FF_CONSTANT, FF_PERIODIC, FF_RAMP, FF_SPRING,
- * FF_FRICTION, FF_DAMPER, FF_RUMBLE, FF_INERTIA, or FF_CUSTOM)
+ * FF_FRICTION, FF_DAMPER, FF_RUMBLE, FF_INERTIA, FF_PRESTORED, or FF_CUSTOM)
* @id: an unique id assigned to an effect
* @direction: direction of the effect
* @trigger: trigger conditions (struct ff_trigger)
* @replay: scheduling of the effect (struct ff_replay)
* @u: effect-specific structure (one of ff_constant_effect, ff_ramp_effect,
- * ff_periodic_effect, ff_condition_effect, ff_rumble_effect) further
- * defining effect parameters
+ * ff_periodic_effect, ff_condition_effect, ff_rumble_effect, ff_prestored_effect)
+ * further defining effect parameters
*
* This structure is sent through ioctl from the application to the driver.
* To create a new effect application should set its @id to -1; the kernel
@@ -464,6 +474,7 @@ struct ff_effect {
struct ff_periodic_effect periodic;
struct ff_condition_effect condition[2]; /* One for each axis */
struct ff_rumble_effect rumble;
+ struct ff_prestored_effect prestored;
} u;
};
@@ -479,20 +490,21 @@ struct ff_effect {
#define FF_DAMPER 0x55
#define FF_INERTIA 0x56
#define FF_RAMP 0x57
+#define FF_PRESTORED 0x58
#define FF_EFFECT_MIN FF_RUMBLE
-#define FF_EFFECT_MAX FF_RAMP
+#define FF_EFFECT_MAX FF_PRESTORED
/*
* Force feedback periodic effect types
*/
-#define FF_SQUARE 0x58
-#define FF_TRIANGLE 0x59
-#define FF_SINE 0x5a
-#define FF_SAW_UP 0x5b
-#define FF_SAW_DOWN 0x5c
-#define FF_CUSTOM 0x5d
+#define FF_SQUARE 0x59
+#define FF_TRIANGLE 0x5a
+#define FF_SINE 0x5b
+#define FF_SAW_UP 0x5c
+#define FF_SAW_DOWN 0x5d
+#define FF_CUSTOM 0x5e
#define FF_WAVEFORM_MIN FF_SQUARE
#define FF_WAVEFORM_MAX FF_CUSTOM
--
2.25.1
^ permalink raw reply related
* [PATCH v2] Input: synaptics - disable intertouch for Lenovo L440
From: Jonathan Denose @ 2023-04-14 16:41 UTC (permalink / raw)
To: lyude
Cc: aduggan, amandhoot12, dmitry.torokhov, jdenose, jdenose,
linux-input, linux-kernel, markpearson, wsa+renesas
In-Reply-To: <063c8f77c216ffac463532023009124542d54c19.camel@redhat.com>
When intertouch is enabled for the L440 a (deep)sleep/resume
cycle causes the touchpad driver to hang which causes the
touchpad to become unresponsive. Disable intertouch resolves
this issue and the touchpad is fine after resume from sleep.
Additionally, when the PNP id for the L440 is only removed
from the topbuttonpad_pnp_ids list, a message is logged to
enable psmouse.synaptics_intertouch, which would cause the
sleep/resume issue again. By removing the PNP id from
topbutton_pnp_ids and then adding it to the
forcepad_pnp_ids array, intertouch is disabled and the
message is not logged.
Signed-off-by: Jonathan Denose <jdenose@google.com>
---
Changes in v2:
- remove debug statement
drivers/input/mouse/synaptics.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index fa021af8506e4..b7218b7652c20 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -150,7 +150,6 @@ static const char * const topbuttonpad_pnp_ids[] = {
"LEN2001", /* Edge E431 */
"LEN2002", /* Edge E531 */
"LEN2003",
- "LEN2004", /* L440 */
"LEN2005",
"LEN2006", /* Edge E440/E540 */
"LEN2007",
@@ -198,6 +197,7 @@ static const char * const smbus_pnp_ids[] = {
static const char * const forcepad_pnp_ids[] = {
"SYN300D",
"SYN3014",
+ "LEN2004", /* L440 */
NULL
};
--
2.39.2
^ permalink raw reply related
* Re: [PATCH] Input: vmmouse - add macros to enable vmmouse relative mode
From: Zack Rusin @ 2023-04-14 15:32 UTC (permalink / raw)
To: dmitry.torokhov@gmail.com, Pv-drivers, zhouzongmin@kylinos.cn,
Linux-graphics-maintainer
Cc: linux-input@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <a576df84ad459d13b18377bb64309fa80f1ea7f2.camel@kylinos.cn>
On Fri, 2023-04-14 at 16:58 +0800, zongmin zhou wrote:
> On Fri, 2023-04-14 at 01:22 +0000, Zack Rusin wrote:
> > On Thu, 2023-04-13 at 16:56 +0800, Zongmin Zhou wrote:
> > > Add macros to enable request relative mode.
> > >
> > > Change the REL_Y value passed by input_report_rel function,
> > > to match the direction of mouse movement.
> >
> > Thanks for the patch, but in its current form it's a nack. First of
> > all we don't
> > want any defines in the driver code that affect compilation, it's
> > never going to be
> > tested or compiled in. Either a kconfig or a module parameter would
> > be acceptable
> > but that's only if you can actually explain what it is that you're
> > fixing. The
> > current single line description just mentions what the effect it has
> > (not completely
> > correctly either because for merged packets absolute will still be
> > x=x, y=y, but
> > relative will be x += dx, y -= dy) but not why it's done, what it's
> > fixing and how
> > to reproduce.
> >
> > z
> >
> Dear zack:
>
> Firstly,thanks for your reply.
>
> The reason I want to add macros to request different vmmouse
> modes(relative or absolute) is that the vmmouse drivers currently only
> supports request absolute mode.But in some case we want request
> relative mode so that Pointer acceleration feature can be used.(as I
> know,libinput module only support Pointer acceleration feature in
> relative mode.)
> So I think we can provide two vmmouse modes to facilitate the use of
> different needs.
> If need,I can change it to a kconfig or a module parameter.
>
> The reasons of fix for REL_Y value，are as follows:
> When I request relative vmmouse mode,and let mouse move up,the mouse
> pointer moved down instead.
> Similarly, when I move the mouse down, the mouse pointer moved up.
> it obviously with a wrong motion direction in y.
>
> Actually，I understand that the value of y here is the end calculation
> result of relative coordinate movement，the real calculation is in
> motion_event() of spice-gtk and legacy_mouse_event() of qemu.
>
> Test scenario:
> 1) start virtual machine with qemu command "vmport=on"，also with spice
> protocal.
> 2) modify guest vmmouse drivers to request relative mode.
> 3) move the mouse,will observe the pointer freezed,it's because driver
> not match the condition 'status & VMMOUSE_RELATIVE_PACKET',can't find
> correct input device.need merge this patch in qemu:
> https://lore.kernel.org/all/20230413081526.2229916-1-zhouzongmin@kylinos.cn/
> 4) after merge the patch in qemu,we can observe the issue of wrong
> motion direction in y.
Sounds like you have a bug in the monitor code to me. The mouse might request
relative mode, but that doesn't mean that it's going to be switched, it's a hint. On
enable we definitely want to request the default absolute mode.
Ultimately it's the delivery mechanism (i.e. whether the delivered event is
VMMOUSE_RELATIVE_PACKET or not) that defines what an event is. This sounds to me
like the monitor delivers VMMOUSE_RELATIVE_PACKET events but doesn't mark them as
such. You can confirm by putting some debugging output in the vmmouse_report_events.
z
^ permalink raw reply
* Re: [PATCH v2 1/1] HID: shield: Initial driver implementation with Thunderstrike support
From: Jiri Kosina @ 2023-04-14 14:12 UTC (permalink / raw)
To: Rahul Rameshbabu; +Cc: Benjamin Tissoires, linux-input, linux-kernel
In-Reply-To: <87a5zbhrdf.fsf@nvidia.com>
On Fri, 14 Apr 2023, Rahul Rameshbabu wrote:
> The reason why I disable interrupts is because I call
> thunderstrike_update_haptics from the play_effect callback I pass to
> input_ff_create_memless. From there, it is used in ml_effect_timer
> drivers/input/ff-memless.c, which is a timer interrupt context so we
> should disable interrupts when taking haptics_update_lock.
>
> static void ml_effect_timer(struct timer_list *t)
> {
> struct ml_device *ml = from_timer(ml, t, timer);
> struct input_dev *dev = ml->dev;
> unsigned long flags;
>
> pr_debug("timer: updating effects\n");
>
> spin_lock_irqsave(&dev->event_lock, flags);
> ml_play_effects(ml);
> spin_unlock_irqrestore(&dev->event_lock, flags);
> }
Ah, right, I missed that, it indeed runs in softirq context. Please
disregard my comment.
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH 1/2] HID: wacom: Lazy-init batteries
From: Jiri Kosina @ 2023-04-14 14:10 UTC (permalink / raw)
To: Jason Gerecke
Cc: linux-input, Benjamin Tissoires, Ping Cheng,
Aaron Armstrong Skomra, Joshua Dickens, Jason Gerecke,
Mario Limonciello
In-Reply-To: <20230413181743.7849-1-jason.gerecke@wacom.com>
On Thu, 13 Apr 2023, Jason Gerecke wrote:
> From: Jason Gerecke <killertofu@gmail.com>
>
> Rather than creating batteries as part of the initial device probe, let's
> make the process lazy. This gives us the opportunity to prevent batteries
> from being created in situations where they are unnecessary.
>
> There are two cases in particular where batteries are being unnecessarily
> created at initialization. These are AES sensors (for which we don't know
> any battery status information until a battery-powered pen actually comes
> into prox) peripheral tablets which share HID descriptors between the
> wired-only and wireless-capable SKUs of a family of devices.
>
> This patch will delay battery initialization of the former until a pen
> actually comes into prox. It will delay battery initialization of the
> latter until either a pen comes into prox or a "heartbeat" packet is
> processed.
>
> Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
> Tested-by: Mario Limonciello <mario.limonciello@amd.com>
I have added the Link: tags suggested by Mario and applied, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* [PATCH] Input: xpad - fix GPF in xpad_probe
From: Dongliang Mu @ 2023-04-14 12:55 UTC (permalink / raw)
To: Dmitry Torokhov, Pavel Rojtberg, Vicki Pfau, Nate Yocom,
Mattijs Korpershoek, John Butler, Matthias Benkmann,
Christopher Crockett, Santosh De Massari
Cc: hust-os-kernel-patches, Dongliang Mu, syzbot+a3f758b8d8cb7e49afec,
Pierre-Loup A. Griffais, linux-input, linux-kernel
In xpad_probe(), it does not allocate xpad->dev with input_dev type.
Then, when it invokes dev_warn with 1st argument - &xpad->dev->dev, it
would trigger GPF.
Fix this by allocating xpad->dev, its error handling and cleanup
operations in the remove function.
Note that this crash does not have any reproducer, so the patch
only passes compilation testing.
Reported-by: syzbot+a3f758b8d8cb7e49afec@syzkaller.appspotmail.com
Signed-off-by: Dongliang Mu <dzm91@hust.edu.cn>
---
drivers/input/joystick/xpad.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index 66a92691a047..2e077b52f46a 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -1944,6 +1944,7 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
{
struct usb_device *udev = interface_to_usbdev(intf);
struct usb_xpad *xpad;
+ struct input_dev *input_dev;
struct usb_endpoint_descriptor *ep_irq_in, *ep_irq_out;
int i, error;
@@ -1957,9 +1958,13 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
}
xpad = kzalloc(sizeof(struct usb_xpad), GFP_KERNEL);
- if (!xpad)
- return -ENOMEM;
+ input_dev = input_allocate_device();
+ if (!xpad || !input_dev) {
+ error = -ENOMEM;
+ goto err_free_mem;
+ }
+ xpad->dev = input_dev;
usb_make_path(udev, xpad->phys, sizeof(xpad->phys));
strlcat(xpad->phys, "/input0", sizeof(xpad->phys));
@@ -2134,6 +2139,7 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
err_free_idata:
usb_free_coherent(udev, XPAD_PKT_LEN, xpad->idata, xpad->idata_dma);
err_free_mem:
+ input_free_device(input_dev);
kfree(xpad);
return error;
}
@@ -2159,6 +2165,7 @@ static void xpad_disconnect(struct usb_interface *intf)
usb_free_coherent(xpad->udev, XPAD_PKT_LEN,
xpad->idata, xpad->idata_dma);
+ input_free_device(xpad->dev);
kfree(xpad);
usb_set_intfdata(intf, NULL);
--
2.39.2
^ permalink raw reply related
* Re: [PATCH] Input: vmmouse - add macros to enable vmmouse relative mode
From: zongmin zhou @ 2023-04-14 8:58 UTC (permalink / raw)
To: Zack Rusin, dmitry.torokhov@gmail.com, Pv-drivers,
Linux-graphics-maintainer
Cc: linux-input@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <36d0b466c96b42f5fd364c2a80379d6d7be9cb48.camel@vmware.com>
On Fri, 2023-04-14 at 01:22 +0000, Zack Rusin wrote:
> On Thu, 2023-04-13 at 16:56 +0800, Zongmin Zhou wrote:
> > Add macros to enable request relative mode.
> >
> > Change the REL_Y value passed by input_report_rel function,
> > to match the direction of mouse movement.
>
> Thanks for the patch, but in its current form it's a nack. First of
> all we don't
> want any defines in the driver code that affect compilation, it's
> never going to be
> tested or compiled in. Either a kconfig or a module parameter would
> be acceptable
> but that's only if you can actually explain what it is that you're
> fixing. The
> current single line description just mentions what the effect it has
> (not completely
> correctly either because for merged packets absolute will still be
> x=x, y=y, but
> relative will be x += dx, y -= dy) but not why it's done, what it's
> fixing and how
> to reproduce.
>
> z
>
Dear zack:
Firstly,thanks for your reply.
The reason I want to add macros to request different vmmouse
modes(relative or absolute) is that the vmmouse drivers currently only
supports request absolute mode.But in some case we want request
relative mode so that Pointer acceleration feature can be used.(as I
know,libinput module only support Pointer acceleration feature in
relative mode.)
So I think we can provide two vmmouse modes to facilitate the use of
different needs.
If need,I can change it to a kconfig or a module parameter.
The reasons of fix for REL_Y value，are as follows:
When I request relative vmmouse mode,and let mouse move up,the mouse
pointer moved down instead.
Similarly, when I move the mouse down, the mouse pointer moved up.
it obviously with a wrong motion direction in y.
Actually，I understand that the value of y here is the end calculation
result of relative coordinate movement，the real calculation is in
motion_event() of spice-gtk and legacy_mouse_event() of qemu.
Test scenario:
1) start virtual machine with qemu command "vmport=on"，also with spice
protocal.
2) modify guest vmmouse drivers to request relative mode.
3) move the mouse,will observe the pointer freezed,it's because driver
not match the condition 'status & VMMOUSE_RELATIVE_PACKET',can't find
correct input device.need merge this patch in qemu:
https://lore.kernel.org/all/20230413081526.2229916-1-zhouzongmin@kylinos.cn/
4) after merge the patch in qemu,we can observe the issue of wrong
motion direction in y.
Looking forward to your reply.
^ permalink raw reply
* Re: [PATCH v2 1/5] dt-bindings: input: touchscreen: add bindings for focaltech,fts5452
From: Krzysztof Kozlowski @ 2023-04-14 7:44 UTC (permalink / raw)
To: Joel Selvaraj, Caleb Connolly, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Andy Gross, Bjorn Andersson, Konrad Dybcio,
Henrik Rydberg, Arnd Bergmann, Robert Jarzmik, Jeff LaBundy,
Markuss Broks, Jean Delvare, Job Noorman, Alistair Francis,
Chris Morgan, Hans de Goede
Cc: linux-input, devicetree, linux-kernel, linux-arm-msm,
~postmarketos/upstreaming, phone-devel
In-Reply-To: <b89c39af-da87-8138-9899-fb631ebe76e1@gmail.com>
On 14/04/2023 02:32, Joel Selvaraj wrote:
> Hi Krzysztof Kozlowski,
>
> Konrad Dybcio suggested to use interrupts-extended instead interrupts.
Sorry,
I have no idea what this email is about.
There is no context here, no reply, it just appeared alone in my inbox
without any reference. Please respond inline to existing messages,
keeping necessary context you are replying to.
> So in my WIP v3, I have updated it in the dts and bindings example.
> However, I am confused if I should replace the "interrupts" with
> "interrupts-extended" property in the schema too? I see a lot of schemas
No.
> specifying "interrupts", with examples using "interrupts" or
> "interrupts-extended". At the same time, I see some specifying both
> "interrupts" and "interrupts-extended" (like one of these two) and very
> few others specify only "interrupts-extended" in the schema. Which is
> the currently recommended way to do this?
>
> In between, the interrupt property should be a required property as the
> driver will not function without an interrupt. I will fix that in v3.
>
> Thanks,
> Joel Selvaraj
Best regards,
Krzysztof
^ permalink raw reply
* [PATCH] Input: soc_button_array - Add invalid acpi_index DMI quirk handling
From: Hans de Goede @ 2023-04-14 7:21 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Hans de Goede, linux-input
Some devices have a wrong entry in their button array which points to
a GPIO which is required in another driver, so soc_button_array must
not claim it.
A specific example of this is the Lenovo Yoga Book X90F / X90L,
where the PNP0C40 home button entry points to a GPIO which is not
a home button and which is required by the lenovo-yogabook driver.
Add a DMI quirk table which can specify an ACPI GPIO resource index which
should be skipped; and add an entry for the Lenovo Yoga Book X90F / X90L
to this new DMI quirk table.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/input/misc/soc_button_array.c | 30 +++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/drivers/input/misc/soc_button_array.c b/drivers/input/misc/soc_button_array.c
index 09489380afda..e79f5497948b 100644
--- a/drivers/input/misc/soc_button_array.c
+++ b/drivers/input/misc/soc_button_array.c
@@ -108,6 +108,27 @@ static const struct dmi_system_id dmi_use_low_level_irq[] = {
{} /* Terminating entry */
};
+/*
+ * Some devices have a wrong entry which points to a GPIO which is
+ * required in another driver, so this driver must not claim it.
+ */
+static const struct dmi_system_id dmi_invalid_acpi_index[] = {
+ {
+ /*
+ * Lenovo Yoga Book X90F / X90L, the PNP0C40 home button entry
+ * points to a GPIO which is not a home button and which is
+ * required by the lenovo-yogabook driver.
+ */
+ .matches = {
+ DMI_EXACT_MATCH(DMI_SYS_VENDOR, "Intel Corporation"),
+ DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "CHERRYVIEW D1 PLATFORM"),
+ DMI_EXACT_MATCH(DMI_PRODUCT_VERSION, "YETI-11"),
+ },
+ .driver_data = (void *)1l,
+ },
+ {} /* Terminating entry */
+};
+
/*
* Get the Nth GPIO number from the ACPI object.
*/
@@ -137,6 +158,8 @@ soc_button_device_create(struct platform_device *pdev,
struct platform_device *pd;
struct gpio_keys_button *gpio_keys;
struct gpio_keys_platform_data *gpio_keys_pdata;
+ const struct dmi_system_id *dmi_id;
+ int invalid_acpi_index = -1;
int error, gpio, irq;
int n_buttons = 0;
@@ -154,10 +177,17 @@ soc_button_device_create(struct platform_device *pdev,
gpio_keys = (void *)(gpio_keys_pdata + 1);
n_buttons = 0;
+ dmi_id = dmi_first_match(dmi_invalid_acpi_index);
+ if (dmi_id)
+ invalid_acpi_index = (long)dmi_id->driver_data;
+
for (info = button_info; info->name; info++) {
if (info->autorepeat != autorepeat)
continue;
+ if (info->acpi_index == invalid_acpi_index)
+ continue;
+
error = soc_button_lookup_gpio(&pdev->dev, info->acpi_index, &gpio, &irq);
if (error || irq < 0) {
/*
--
2.39.1
^ permalink raw reply related
* Re: [PATCH v2 1/1] HID: shield: Initial driver implementation with Thunderstrike support
From: Rahul Rameshbabu @ 2023-04-14 7:09 UTC (permalink / raw)
To: Jiri Kosina; +Cc: Benjamin Tissoires, linux-input, linux-kernel
In-Reply-To: <nycvar.YFH.7.76.2304131705350.29760@cbobk.fhfr.pm>
Hi Jiri,
Thanks for the feedback.
On Thu, 13 Apr, 2023 17:07:00 +0200 Jiri Kosina <jikos@kernel.org> wrote:
> On Mon, 10 Apr 2023, Rahul Rameshbabu wrote:
>
>> Supports the Thunderstrike (SHIELD 2017) controller. Implements support for
>> the Thunderstrike HOSTCMD firmware interface. Adds sysfs attributes about a
>> SHIELD device and introduces haptics support for controllers.
>>
>> Signed-off-by: Rahul Rameshbabu <rrameshbabu@nvidia.com>
>
> Thanks a lot for submitting the driver. I have one minor question:
>
> [ ... snip ... ]
>> +thunderstrike_update_haptics(struct thunderstrike *ts,
>> + struct thunderstrike_hostcmd_haptics *motors)
>> +{
>> + unsigned long flags;
>> +
>> + spin_lock_irqsave(&ts->haptics_update_lock, flags);
>> + ts->haptics_val = *motors;
>> + spin_unlock_irqrestore(&ts->haptics_update_lock, flags);
>
> Do we really have to disable interrupts when taking haptics_update_lock?
> Is it ever being taken from interrupt context?
The reason why I disable interrupts is because I call
thunderstrike_update_haptics from the play_effect callback I pass to
input_ff_create_memless. From there, it is used in ml_effect_timer
drivers/input/ff-memless.c, which is a timer interrupt context so we
should disable interrupts when taking haptics_update_lock.
static void ml_effect_timer(struct timer_list *t)
{
struct ml_device *ml = from_timer(ml, t, timer);
struct input_dev *dev = ml->dev;
unsigned long flags;
pr_debug("timer: updating effects\n");
spin_lock_irqsave(&dev->event_lock, flags);
ml_play_effects(ml);
spin_unlock_irqrestore(&dev->event_lock, flags);
}
Let me know if this seems off.
>
> Thanks,
-- Rahul Rameshbabu
^ permalink raw reply
* Re: [PATCH v2 2/2] Input: xpad - fix PowerA EnWired Controller guide button
From: Dmitry Torokhov @ 2023-04-14 7:05 UTC (permalink / raw)
To: Vicki Pfau; +Cc: linux-input, Pavel Rojtberg
In-Reply-To: <20230411031650.960322-3-vi@endrift.com>
On Mon, Apr 10, 2023 at 08:16:50PM -0700, Vicki Pfau wrote:
> This commit explicitly disables the audio interface the same way the official
> driver does. This is needed for some controllers, such as the PowerA Enhanced
> Wired Controller for Series X|S (0x20d6:0x200e) to report the guide button.
>
> Signed-off-by: Vicki Pfau <vi@endrift.com>
Applied, thank you.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2 1/2] Input: xpad - Add constants for GIP interface numbers
From: Dmitry Torokhov @ 2023-04-14 7:05 UTC (permalink / raw)
To: Vicki Pfau; +Cc: linux-input, Pavel Rojtberg
In-Reply-To: <20230411031650.960322-2-vi@endrift.com>
On Mon, Apr 10, 2023 at 08:16:49PM -0700, Vicki Pfau wrote:
> Wired GIP devices present multiple interfaces with the same USB identification
> other than the interface number. This adds constants for differentiating two of
> them and uses them where appropriate
>
> Signed-off-by: Vicki Pfau <vi@endrift.com>
Applied, thank you.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2 1/1] HID: shield: Initial driver implementation with Thunderstrike support
From: Rahul Rameshbabu @ 2023-04-14 6:47 UTC (permalink / raw)
To: Benjamin Tissoires; +Cc: Jiri Kosina, linux-input, linux-kernel
In-Reply-To: <20230413151621.py34io57wcrfofo2@mail.corp.redhat.com>
On Thu, 13 Apr, 2023 17:16:21 +0200 Benjamin Tissoires <benjamin.tissoires@redhat.com> wrote:
> Hi,
>
> On Apr 10 2023, Rahul Rameshbabu wrote:
>> Supports the Thunderstrike (SHIELD 2017) controller. Implements support for
>> the Thunderstrike HOSTCMD firmware interface. Adds sysfs attributes about a
>> SHIELD device and introduces haptics support for controllers.
>>
>> Signed-off-by: Rahul Rameshbabu <rrameshbabu@nvidia.com>
>> ---
>> MAINTAINERS | 6 +
>> drivers/hid/Kconfig | 18 ++
>> drivers/hid/Makefile | 1 +
>> drivers/hid/hid-ids.h | 3 +
>> drivers/hid/hid-shield.c | 587 +++++++++++++++++++++++++++++++++++++++
>
> In addition to what Jiri said, would you mind changing the name to
> hid-nvidia-shield.c or just hid-nvidia.c?
> The "normal" naming scheme in the hid tree is to group devices by
> vendors, and TBH, knowing that the "shield" is from Nvidia is not
> necessarily obvious.
Ack. I will make the change to hid-nvidia.c in my next submission of
this patch.
>
> Cheers,
> Benjamin
>
^ permalink raw reply
* Re: [PATCH] Input: cyttsp5 - fix sensing configuration data structure
From: Dmitry Torokhov @ 2023-04-14 6:41 UTC (permalink / raw)
To: hrdl; +Cc: Linus Walleij, linux-input, linux-kernel, Alistair Francis
In-Reply-To: <20230411211651.3791304-1-git@hrdl.eu>
On Tue, Apr 11, 2023 at 11:16:51PM +0200, hrdl wrote:
> Prior to this patch, the sensing configuration data was not parsed
> correctly, breaking detection of max_tch. The vendor driver includes
> this field. This change informs the driver about the correct maximum
> number of simultaneous touch inputs.
>
> Tested on a Pine64 PineNote with a modified touch screen controller
> firmware.
>
> Signed-off-by: hrdl <git@hrdl.eu>
Applied, thank you.
--
Dmitry
^ permalink raw reply
* Re: [PATCH V3] dt-bindings: input: pwm-beeper: convert to dt schema
From: Dmitry Torokhov @ 2023-04-14 6:36 UTC (permalink / raw)
To: Peng Fan (OSS)
Cc: robh+dt, krzysztof.kozlowski+dt, s.hauer, linux-input, devicetree,
linux-kernel, Peng Fan
In-Reply-To: <20230407075259.1593739-1-peng.fan@oss.nxp.com>
On Fri, Apr 07, 2023 at 03:52:59PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
>
> Convert the binding doc to dt schema, and also fixed the
> example from fixed-regulator to regulator-fixed.
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
Applied, thank you.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] Input: synaptics-rmi4 - Modify mismatched function name
From: Dmitry Torokhov @ 2023-04-14 6:24 UTC (permalink / raw)
To: Jiapeng Chong; +Cc: linux-input, linux-kernel, Abaci Robot
In-Reply-To: <20230209040710.111456-1-jiapeng.chong@linux.alibaba.com>
On Thu, Feb 09, 2023 at 12:07:10PM +0800, Jiapeng Chong wrote:
> No functional modification involved.
>
> drivers/input/rmi4/rmi_bus.c:300: warning: expecting prototype for rmi_register_function_handler(). Prototype was for __rmi_register_function_handler() instead.
>
> Reported-by: Abaci Robot <abaci@linux.alibaba.com>
> Link: https://bugzilla.openanolis.cn/show_bug.cgi?id=4012
> Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>
Applied, thank you.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2] input: raspberrypi-ts: Fix refcount leak in rpi_ts_probe
From: Dmitry Torokhov @ 2023-04-14 6:06 UTC (permalink / raw)
To: Miaoqian Lin
Cc: Florian Fainelli, Broadcom internal kernel review list,
Rob Herring, Eric Anholt, Nicolas Saenz Julienne, linux-input,
linux-rpi-kernel, linux-arm-kernel, linux-kernel
In-Reply-To: <20221223074657.810346-1-linmq006@gmail.com>
On Fri, Dec 23, 2022 at 11:46:53AM +0400, Miaoqian Lin wrote:
> rpi_firmware_get() take reference, we need to release it in error paths
> as well. Use devm_rpi_firmware_get() helper to handling the resources.
> Also remove the existing rpi_firmware_put().
>
> Fixes: 0b9f28fed3f7 ("Input: add official Raspberry Pi's touchscreen driver")
> Fixes: 3b8ddff780b7 ("input: raspberrypi-ts: Release firmware handle when not needed")
> Signed-off-by: Miaoqian Lin <linmq006@gmail.com>
Applied, thank you.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] Input: vmmouse - add macros to enable vmmouse relative mode
From: Zack Rusin @ 2023-04-14 1:22 UTC (permalink / raw)
To: dmitry.torokhov@gmail.com, Pv-drivers, zhouzongmin@kylinos.cn,
Linux-graphics-maintainer
Cc: linux-input@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20230413085635.2515647-1-zhouzongmin@kylinos.cn>
On Thu, 2023-04-13 at 16:56 +0800, Zongmin Zhou wrote:
> Add macros to enable request relative mode.
>
> Change the REL_Y value passed by input_report_rel function,
> to match the direction of mouse movement.
Thanks for the patch, but in its current form it's a nack. First of all we don't
want any defines in the driver code that affect compilation, it's never going to be
tested or compiled in. Either a kconfig or a module parameter would be acceptable
but that's only if you can actually explain what it is that you're fixing. The
current single line description just mentions what the effect it has (not completely
correctly either because for merged packets absolute will still be x=x, y=y, but
relative will be x += dx, y -= dy) but not why it's done, what it's fixing and how
to reproduce.
z
^ permalink raw reply
* Re: [PATCH v2 1/5] dt-bindings: input: touchscreen: add bindings for focaltech,fts5452
From: Joel Selvaraj @ 2023-04-14 0:32 UTC (permalink / raw)
To: Krzysztof Kozlowski, Caleb Connolly, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Andy Gross, Bjorn Andersson, Konrad Dybcio,
Henrik Rydberg, Arnd Bergmann, Robert Jarzmik, Jeff LaBundy,
Markuss Broks, Jean Delvare, Job Noorman, Alistair Francis,
Chris Morgan, Hans de Goede
Cc: linux-input, devicetree, linux-kernel, linux-arm-msm,
~postmarketos/upstreaming, phone-devel, Konrad Dybcio
In-Reply-To: <f9552bb6-ea73-93b4-f15d-d5d7c326c708@linaro.org>
Hi Krzysztof Kozlowski,
Konrad Dybcio suggested to use interrupts-extended instead interrupts.
So in my WIP v3, I have updated it in the dts and bindings example.
However, I am confused if I should replace the "interrupts" with
"interrupts-extended" property in the schema too? I see a lot of schemas
specifying "interrupts", with examples using "interrupts" or
"interrupts-extended". At the same time, I see some specifying both
"interrupts" and "interrupts-extended" (like one of these two) and very
few others specify only "interrupts-extended" in the schema. Which is
the currently recommended way to do this?
In between, the interrupt property should be a required property as the
driver will not function without an interrupt. I will fix that in v3.
Thanks,
Joel Selvaraj
^ permalink raw reply
* Re: [PATCH] Input: synaptics - disable intertouch for Lenovo L440
From: Lyude Paul @ 2023-04-13 20:47 UTC (permalink / raw)
To: Jonathan Denose, LKML
Cc: Dmitry Torokhov, Wolfram Sang, linux-input, Jonathan Denose,
Aman Dhoot, Mark Pearson, Andrew Duggan
In-Reply-To: <20230412175311.1.Ieb687047a5b75c7b7ee5dd258207ef5ca9a3b728@changeid>
This patch looks fine to me, I'm a bit curious though whether the folks at
Synaptics have any idea of some other workaround we might be able to do rather
than disabling intertouch? Added Andrew Duggan to CC to ask.
If we don't get any response from them after a while, feel free to consider
this:
Reviewed-by: Lyude Paul <lyude@redhat.com>
On Wed, 2023-04-12 at 17:54 -0500, Jonathan Denose wrote:
> When intertouch is enabled for the L440 a (deep)sleep/resume
> cycle causes the touchpad driver to hang which causes the
> touchpad to become unresponsive. Disable intertouch resolves
> this issue and the touchpad is fine after resume from sleep.
>
> Additionally, when the PNP id for the L440 is only removed
> from the topbuttonpad_pnp_ids list, a message is logged to
> enable psmouse.synaptics_intertouch, which would cause the
> sleep/resume issue again. By removing the PNP id from
> topbutton_pnp_ids and then adding it to the
> forcepad_pnp_ids array, intertouch is disabled and the
> message is not logged.
>
> Signed-off-by: Jonathan Denose <jdenose@google.com>
> ---
>
> drivers/input/mouse/synaptics.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
> index fa021af8506e4..77a4f58128e84 100644
> --- a/drivers/input/mouse/synaptics.c
> +++ b/drivers/input/mouse/synaptics.c
> @@ -150,7 +150,6 @@ static const char * const topbuttonpad_pnp_ids[] = {
> "LEN2001", /* Edge E431 */
> "LEN2002", /* Edge E531 */
> "LEN2003",
> - "LEN2004", /* L440 */
> "LEN2005",
> "LEN2006", /* Edge E440/E540 */
> "LEN2007",
> @@ -198,6 +197,7 @@ static const char * const smbus_pnp_ids[] = {
> static const char * const forcepad_pnp_ids[] = {
> "SYN300D",
> "SYN3014",
> + "LEN2004", /* L440 */
> NULL
> };
>
> @@ -1769,6 +1769,8 @@ static int synaptics_create_intertouch(struct psmouse *psmouse,
> .flags = I2C_CLIENT_HOST_NOTIFY,
> };
>
> + psmouse_dbg(psmouse, "topbuttonpad is: %s\n", topbuttonpad ? "true" : "false");
> +
> return psmouse_smbus_init(psmouse, &intertouch_board,
> &pdata, sizeof(pdata), true,
> leave_breadcrumbs);
--
Cheers,
Lyude Paul (she/her)
Software Engineer at Red Hat
^ permalink raw reply
* RE: [PATCH 1/2] HID: wacom: Lazy-init batteries
From: Limonciello, Mario @ 2023-04-13 19:03 UTC (permalink / raw)
To: Jason Gerecke, linux-input@vger.kernel.org, Benjamin Tissoires,
Jiri Kosina
Cc: Ping Cheng, Aaron Armstrong Skomra, Joshua Dickens, Jason Gerecke,
Bastien Nocera
In-Reply-To: <20230413181743.7849-1-jason.gerecke@wacom.com>
[Public]
> From: Jason Gerecke <killertofu@gmail.com>
>
> Rather than creating batteries as part of the initial device probe, let's
> make the process lazy. This gives us the opportunity to prevent batteries
> from being created in situations where they are unnecessary.
>
> There are two cases in particular where batteries are being unnecessarily
> created at initialization. These are AES sensors (for which we don't know
> any battery status information until a battery-powered pen actually comes
> into prox) peripheral tablets which share HID descriptors between the
> wired-only and wireless-capable SKUs of a family of devices.
>
> This patch will delay battery initialization of the former until a pen
> actually comes into prox. It will delay battery initialization of the
> latter until either a pen comes into prox or a "heartbeat" packet is
> processed.
>
> Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
> Tested-by: Mario Limonciello <mario.limonciello@amd.com>
Some other tags to add:
Link: https://bugzilla.kernel.org/show_bug.cgi?id=217062
Link: https://gitlab.gnome.org/GNOME/gnome-control-center/-/issues/2354
> ---
> drivers/hid/wacom_sys.c | 10 ----------
> drivers/hid/wacom_wac.c | 13 ++++++-------
> 2 files changed, 6 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index fb538a6c4add8..8214896adadad 100644
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c
> @@ -2372,13 +2372,6 @@ static int wacom_parse_and_register(struct
> wacom *wacom, bool wireless)
> if (error)
> goto fail;
>
> - if (!(features->device_type & WACOM_DEVICETYPE_WL_MONITOR)
> &&
> - (features->quirks & WACOM_QUIRK_BATTERY)) {
> - error = wacom_initialize_battery(wacom);
> - if (error)
> - goto fail;
> - }
> -
> error = wacom_register_inputs(wacom);
> if (error)
> goto fail;
> @@ -2509,9 +2502,6 @@ static void wacom_wireless_work(struct
> work_struct *work)
>
> strscpy(wacom_wac->name, wacom_wac1->name,
> sizeof(wacom_wac->name));
> - error = wacom_initialize_battery(wacom);
> - if (error)
> - goto fail;
> }
>
> return;
> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> index 4cfa51416dbcb..391fde5bf6024 100644
> --- a/drivers/hid/wacom_wac.c
> +++ b/drivers/hid/wacom_wac.c
> @@ -113,6 +113,11 @@ static void wacom_notify_battery(struct
> wacom_wac *wacom_wac,
> bool bat_connected, bool ps_connected)
> {
> struct wacom *wacom = container_of(wacom_wac, struct wacom,
> wacom_wac);
> + bool bat_initialized = wacom->battery.battery;
> + bool has_quirk = wacom_wac->features.quirks &
> WACOM_QUIRK_BATTERY;
> +
> + if (bat_initialized != has_quirk)
> + wacom_schedule_work(wacom_wac,
> WACOM_WORKER_BATTERY);
>
> __wacom_notify_battery(&wacom->battery, bat_status,
> bat_capacity,
> bat_charging, bat_connected, ps_connected);
> @@ -3391,19 +3396,13 @@ static int wacom_status_irq(struct wacom_wac
> *wacom_wac, size_t len)
> int battery = (data[8] & 0x3f) * 100 / 31;
> bool charging = !!(data[8] & 0x80);
>
> + features->quirks |= WACOM_QUIRK_BATTERY;
> wacom_notify_battery(wacom_wac,
> WACOM_POWER_SUPPLY_STATUS_AUTO,
> battery, charging, battery || charging, 1);
> -
> - if (!wacom->battery.battery &&
> - !(features->quirks & WACOM_QUIRK_BATTERY)) {
> - features->quirks |= WACOM_QUIRK_BATTERY;
> - wacom_schedule_work(wacom_wac,
> WACOM_WORKER_BATTERY);
> - }
> }
> else if ((features->quirks & WACOM_QUIRK_BATTERY) &&
> wacom->battery.battery) {
> features->quirks &= ~WACOM_QUIRK_BATTERY;
> - wacom_schedule_work(wacom_wac,
> WACOM_WORKER_BATTERY);
> wacom_notify_battery(wacom_wac,
> POWER_SUPPLY_STATUS_UNKNOWN, 0, 0, 0, 0);
> }
> return 0;
> --
> 2.40.0
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox