* [PATCH 0/3] HID: sony: more cleanup
From: Rosalie Wanders @ 2026-05-08 4:51 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: linux-input, linux-kernel, Rosalie Wanders
This series cleans up hid-sony by utilizing modern helper functions.
This series depends on the following patches:
- HID: sony: use input_dev from sc struct in sony_init_ff()
- HID: sony: use dedicated raw_event() handlers in sony_raw_event()
Rosalie Wanders (3):
HID: sony: use guard() and scoped_guard()
HID: sony: remove unneeded which argument from sony_schedule_work()
HID: sony: use devm_kasprintf()
drivers/hid/hid-sony.c | 100 +++++++++++++++--------------------------
1 file changed, 36 insertions(+), 64 deletions(-)
--
2.54.0
^ permalink raw reply
* Re: (subset) [PATCH v4 0/4] Add support for Awinic AW86938 haptic driver
From: Bjorn Andersson @ 2026-05-07 20:34 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Konrad Dybcio, Luca Weiss, Griffin Kroah-Hartman
Cc: linux-input, devicetree, linux-kernel, linux-arm-msm,
Krzysztof Kozlowski, Dmitry Baryshkov, Konrad Dybcio
In-Reply-To: <20260302-aw86938-driver-v4-0-92c865df9cca@fairphone.com>
On Mon, 02 Mar 2026 11:50:24 +0100, Griffin Kroah-Hartman wrote:
> Add devicetree bindings and driver support for the AW86938 haptic driver
> chip, and add it to the devicetree for the Fairphone (Gen. 6) smartphone.
>
> This haptics chip is quite similar to the AW86927, and shares many core
> features but has some notable differences, including some extra
> features.
>
> [...]
Applied, thanks!
[4/4] arm64: dts: qcom: milos-fairphone-fp6: Add vibrator support
commit: 66fb209e6035ed90cbff71c48c60124803da5c63
Best regards,
--
Bjorn Andersson <andersson@kernel.org>
^ permalink raw reply
* Re: [PATCH] Input: ads7846 - consolidate coordinate filtering logic
From: Dmitry Torokhov @ 2026-05-07 19:35 UTC (permalink / raw)
To: Kris Bahnsen
Cc: linux-input, Aaro Koskinen, Mark Featherston, Marek Vasut,
linux-kernel
In-Reply-To: <cf9e30d8-f51b-4a19-ae7d-914e6bccb265@embeddedTS.com>
Hi Kris,
On Thu, May 07, 2026 at 10:48:57AM -0700, Kris Bahnsen wrote:
> On 5/4/26 9:54 PM, Dmitry Torokhov wrote:
> > The ads7846 driver has two separate filtering functions,
> > ads7846_filter() and ads7846_filter_one(), for the full-duplex and
> > half-duplex SPI paths, respectively.
> >
> > They can be consolidated by extracting the core filtering logic for a
> > single command into a helper function, ads7846_filter_cmd(), which
> > iterates from l->skip to l->count. The half-duplex setup function is
> > updated to set l->skip = l->count - 1 so that the helper only processes
> > the last sample, preserving the original half-duplex behavior.
> >
> > Assisted-by: Gemini:gemini-3.1-pro
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > ---
> >
> > Not tested so will appreciate if someone could give it a spin.
>
> Forgive my ignorance, but I am unsure of the base commit this applies
> to and I'm unable to apply it to current HEAD of linux, linux-next,
> or your input tree.
>
> I also am not easily able to test this if it doesn't cleanly apply
> to the latest LTS, but will do what I can once I know what other
> patches I would need to pull in to our tree.
I think if you pull 'next' branch of my tree, then the following patches
seem to apply cleanly on 7.0:
c68bc840f06c ("Input: ads7846 - restore half-duplex support")
011bdf9f3a9d ("Input: ads7846 - consolidate coordinate filtering logic")
I hope they also build ;)
c68bc840f06c should appear in linux-next soon too.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v4 2/2] Input: isa1200 - new driver for Imagis ISA1200
From: Dmitry Torokhov @ 2026-05-07 19:25 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Linus Walleij,
linux-input, devicetree, linux-kernel
In-Reply-To: <20260507133948.75704-3-clamor95@gmail.com>
Hi Svyatoslav,
On Thu, May 07, 2026 at 04:39:48PM +0300, Svyatoslav Ryhel wrote:
> From: Linus Walleij <linusw@kernel.org>
>
> The ISA1200 is a haptic feedback unit from Imagis Technology using two
> motors for haptic feedback in mobile phones. Used in many mobile devices
> c. 2012 including Samsung Galxy S Advance GT-I9070 (Janice), Samsung Beam
> GT-I8350 (Gavini), LG Optimus 4X P880 and LG Optimus Vu P895.
>
> The exact datasheet for the ISA1200 is not available; all data was modeled
> based on available downstream kernel sources for various devices and
> fragments of information scattered across the internet.
>
> Tested-by: Linus Walleij <linusw@kernel.org> # GT-I9070 Janice
> Signed-off-by: Linus Walleij <linusw@kernel.org>
> Co-developed-by: Svyatoslav Ryhel <clamor95@gmail.com>
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> ---
> drivers/input/misc/Kconfig | 12 +
> drivers/input/misc/Makefile | 1 +
> drivers/input/misc/isa1200.c | 540 +++++++++++++++++++++++++++++++++++
> 3 files changed, 553 insertions(+)
> create mode 100644 drivers/input/misc/isa1200.c
>
> diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
> index 94a753fcb64f..52f192104ee2 100644
> --- a/drivers/input/misc/Kconfig
> +++ b/drivers/input/misc/Kconfig
> @@ -852,6 +852,18 @@ config INPUT_IQS7222
> To compile this driver as a module, choose M here: the
> module will be called iqs7222.
>
> +config INPUT_ISA1200_HAPTIC
> + tristate "Imagis ISA1200 haptic feedback unit"
> + depends on I2C
> + select INPUT_FF_MEMLESS
> + select REGMAP_I2C
> + help
> + Say Y to enable support for the Imagis ISA1200 haptic
> + feedback unit.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called isa1200.
> +
> config INPUT_CMA3000
> tristate "VTI CMA3000 Tri-axis accelerometer"
> help
> diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
> index 415fc4e2918b..d62bf2e9d85f 100644
> --- a/drivers/input/misc/Makefile
> +++ b/drivers/input/misc/Makefile
> @@ -49,6 +49,7 @@ obj-$(CONFIG_INPUT_IMS_PCU) += ims-pcu.o
> obj-$(CONFIG_INPUT_IQS269A) += iqs269a.o
> obj-$(CONFIG_INPUT_IQS626A) += iqs626a.o
> obj-$(CONFIG_INPUT_IQS7222) += iqs7222.o
> +obj-$(CONFIG_INPUT_ISA1200_HAPTIC) += isa1200.o
> obj-$(CONFIG_INPUT_KEYSPAN_REMOTE) += keyspan_remote.o
> obj-$(CONFIG_INPUT_KXTJ9) += kxtj9.o
> obj-$(CONFIG_INPUT_M68K_BEEP) += m68kspkr.o
> diff --git a/drivers/input/misc/isa1200.c b/drivers/input/misc/isa1200.c
> new file mode 100644
> index 000000000000..f8dba8a95c7d
> --- /dev/null
> +++ b/drivers/input/misc/isa1200.c
> @@ -0,0 +1,540 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +#include <linux/array_size.h>
> +#include <linux/bitmap.h>
> +#include <linux/bits.h>
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/devm-helpers.h>
> +#include <linux/err.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/i2c.h>
> +#include <linux/input.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/property.h>
> +#include <linux/pwm.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/units.h>
> +
> +/*
> + * System control (LDO regulator)
> + *
> + * LDO voltage to register mapping is linear, but it is split in two parts:
> + * 2.3V - 3.0V map to 0x08 - 0x0f; 3.1V - 3.8V map to 0x00 - 0x7
> + */
> +
> +#define ISA1200_SCTRL 0x00
> +#define ISA1200_LDO_VOLTAGE_BASE 0x08
> +#define ISA1200_LDO_VOLTAGE_STEP 100000
> +#define ISA1200_LDO_VOLTAGE_2V3 23
> +#define ISA1200_LDO_VOLTAGE_3V1 31
> +#define ISA1200_LDO_VOLTAGE_MIN 2300000
> +#define ISA1200_LDO_VOLTAGE_MAX 3800000
> +
> +/*
> + * The output frequency is calculated with this formula:
> + *
> + * base clock frequency
> + * fout = -----------------------------------------
> + * (128 - PWM_FREQ) * 2 * PLLDIV * PWM_PERIOD
> + *
> + * The base clock frequency is the clock frequency provided on the
> + * clock input to the chip, divided by the value in HCTRL0
> + *
> + * PWM_FREQ is configured in register HCTRL4, it is common to set this
> + * to 0 to get only two variables to calculate.
> + *
> + * PLLDIV is configured in register HCTRL3 (bits 7..4, so 0..15)
> + * PWM_PERIOD is configured in register HCTRL6
> + * Further the duty cycle can be configured in HCTRL5
> + */
> +
> +/*
> + * HCTRL0 configures clock or PWM input and selects the divider for
> + * the clock input.
> + */
> +#define ISA1200_HCTRL0 0x30
> +#define ISA1200_HCTRL0_HAP_ENABLE BIT(7)
> +#define ISA1200_HCTRL0_PWM_GEN_MODE BIT(4)
> +#define ISA1200_HCTRL0_PWM_INPUT_MODE BIT(3)
> +#define ISA1200_HCTRL0_CLKDIV_128 128
> +
> +/*
> + * HCTRL1 configures the motor type and clock sourse
> + */
> +#define ISA1200_HCTRL1 0x31
> +#define ISA1200_HCTRL1_EXT_CLOCK BIT(7)
> +#define ISA1200_HCTRL1_DAC_INVERT BIT(6)
> +#define ISA1200_HCTRL1_MODE(n) (((n) & 1) << 5)
I wonder if this should simply be BIT(5) and you conditionally use it in
the code. The macro is not really usable to disable the setting...
> +
> +/* HCTRL2 controls software reset of the chip */
> +#define ISA1200_HCTRL2 0x32
> +#define ISA1200_HCTRL2_SW_RESET BIT(0)
> +
> +/*
> + * HCTRL3 controls the PLL divisor
> + *
> + * Bits [0,1] are always set to 1 (we don't know what they are
> + * used for) and bit 4 and upward control the PLL divisor.
> + */
> +#define ISA1200_HCTRL3 0x33
> +#define ISA1200_HCTRL3_DEFAULT 0x03
> +#define ISA1200_HCTRL3_PLLDIV(n) (((n) & 0xf) << 4)
> +
> +/* HCTRL4 controls the PWM frequency of external channel */
> +#define ISA1200_HCTRL4 0x34
> +
> +/* HCTRL5 controls the PWM high duty cycle of internal channel */
> +#define ISA1200_HCTRL5 0x35
> +
> +/* HCTRL6 controls the PWM period of internal channel */
> +#define ISA1200_HCTRL6 0x36
> +#define ISA1200_HCTRL6_PERIOD_SCALE 100
> +
> +/* The use for these registers is unknown but they exist */
> +#define ISA1200_HCTRL7 0x37
> +#define ISA1200_HCTRL8 0x38
> +#define ISA1200_HCTRL9 0x39
> +#define ISA1200_HCTRLA 0x3a
> +#define ISA1200_HCTRLB 0x3b
> +#define ISA1200_HCTRLC 0x3c
> +#define ISA1200_HCTRLD 0x3d
> +
> +#define ISA1200_EN_PINS_MAX 2
> +
> +struct isa1200_config {
> + u32 ldo_voltage;
> + u32 mode;
> + u32 clkdiv;
> + u32 plldiv;
> + u32 freq;
> + u32 period;
> + u32 duty;
> +};
> +
> +struct isa1200 {
> + struct input_dev *input;
> + struct regmap *map;
> +
> + struct clk *clk;
> + struct pwm_device *pwm;
> + struct gpio_descs *enable_gpios;
> +
> + struct work_struct play_work;
> + struct isa1200_config config;
> +
> + int level;
> + bool clk_on;
I think you need not only clk_on, but general "active" flag that you
would set at the end of isa1200_start().
> +};
> +
> +static const struct regmap_config isa1200_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 8,
> + .max_register = ISA1200_HCTRLD,
> +};
> +
> +static void isa1200_start(struct isa1200 *isa)
> +{
> + struct isa1200_config *config = &isa->config;
> + struct pwm_state state;
> + u8 hctrl0 = 0, hctrl1 = 0;
> + DECLARE_BITMAP(values, ISA1200_EN_PINS_MAX);
> + int ret;
Please use "error" or "err" for all variables that only hold error codes
(or 0) instead of a real value that is used for something.
> +
> + if (!isa->clk_on) {
> + ret = clk_prepare_enable(isa->clk);
This return 0 on success so
if (error)
return;
> + if (ret < 0)
> + return;
> +
> + isa->clk_on = true;
> + }
> +
> + bitmap_fill(values, ISA1200_EN_PINS_MAX);
> + gpiod_multi_set_value_cansleep(isa->enable_gpios, values);
> +
> + usleep_range(200, 300);
> +
> + regmap_write(isa->map, ISA1200_SCTRL, config->ldo_voltage);
> +
> + if (isa->clk) {
> + hctrl0 = ISA1200_HCTRL0_PWM_GEN_MODE;
> + hctrl1 = ISA1200_HCTRL1_EXT_CLOCK;
> + }
> +
> + if (isa->pwm) {
> + hctrl0 = ISA1200_HCTRL0_PWM_INPUT_MODE;
> + hctrl1 = 0;
> + }
> +
> + hctrl0 |= __ffs(config->clkdiv / ISA1200_HCTRL0_CLKDIV_128);
> + hctrl1 |= ISA1200_HCTRL1_DAC_INVERT;
> + hctrl1 |= ISA1200_HCTRL1_MODE(config->mode);
> +
> + regmap_write(isa->map, ISA1200_HCTRL0, hctrl0);
> + regmap_write(isa->map, ISA1200_HCTRL1, hctrl1);
> +
> + /* Make sure to de-assert software reset */
> + regmap_write(isa->map, ISA1200_HCTRL2, 0x00);
> +
> + /* PLL divisor */
> + regmap_write(isa->map, ISA1200_HCTRL3,
> + ISA1200_HCTRL3_PLLDIV(config->plldiv) |
> + ISA1200_HCTRL3_DEFAULT);
> +
> + /* Frequency */
> + regmap_write(isa->map, ISA1200_HCTRL4, config->freq);
> + /* Duty cycle */
> + regmap_write(isa->map, ISA1200_HCTRL5, config->period >> 1);
> + /* Period */
> + regmap_write(isa->map, ISA1200_HCTRL6, config->period);
> +
> + hctrl0 |= ISA1200_HCTRL0_HAP_ENABLE;
> + regmap_write(isa->map, ISA1200_HCTRL0, hctrl0);
> +
> + if (isa->clk)
> + regmap_write(isa->map, ISA1200_HCTRL5, config->duty);
> +
> + if (isa->pwm) {
> + pwm_get_state(isa->pwm, &state);
> + state.duty_cycle = config->duty;
> + state.enabled = true;
> + pwm_apply_might_sleep(isa->pwm, &state);
> + }
> +}
> +
> +static void isa1200_power_off(void *data)
> +{
> + struct isa1200 *isa = data;
> +
> + DECLARE_BITMAP(values, ISA1200_EN_PINS_MAX);
> +
> + bitmap_zero(values, ISA1200_EN_PINS_MAX);
> + gpiod_multi_set_value_cansleep(isa->enable_gpios, values);
> +
> + if (isa->clk_on) {
> + clk_disable_unprepare(isa->clk);
> + isa->clk_on = false;
> + }
> +}
> +
> +static void isa1200_stop(struct isa1200 *isa)
> +{
> + struct pwm_state state;
> +
> + if (isa->pwm) {
> + pwm_get_state(isa->pwm, &state);
> + state.duty_cycle = 0;
> + state.enabled = false;
> + pwm_apply_might_sleep(isa->pwm, &state);
> + }
> +
> + regmap_write(isa->map, ISA1200_HCTRL0, 0x00);
> + isa1200_power_off(isa);
> +}
> +
> +static void isa1200_play_work(struct work_struct *work)
> +{
> + struct isa1200 *isa =
> + container_of(work, struct isa1200, play_work);
> +
> + guard(mutex)(&isa->input->mutex);
I think this mutex is dangerous here. You really want to stop/cancel
work in suspend() and close() instead of blocking on the mutex and then
continuing.
> +
> + if (isa->level)
> + isa1200_start(isa);
> + else
> + isa1200_stop(isa);
> +}
> +
> +static int isa1200_vibrator_play_effect(struct input_dev *input, void *data,
> + struct ff_effect *effect)
> +{
> + struct isa1200 *isa = input_get_drvdata(input);
> + int level;
> +
> + /*
> + * TODO: we currently only support rumble.
> + * The ISA1200 can control two motors and some devices
> + * also have two motors mounted.
> + */
> + level = effect->u.rumble.strong_magnitude;
> + if (!level)
> + level = effect->u.rumble.weak_magnitude;
> +
> + dev_dbg(&input->dev, "FF effect type %d level %d\n",
> + effect->type, level);
> +
> + if (isa->level != level) {
> + isa->level = level;
> + schedule_work(&isa->play_work);
> + }
> +
> + return 0;
> +}
> +
> +static void isa1200_vibrator_close(struct input_dev *input)
> +{
> + struct isa1200 *isa = input_get_drvdata(input);
> +
> + cancel_work_sync(&isa->play_work);
> +
> + if (isa->level)
So here I think you need to check that "active" flag that would represent
the committed state.
> + isa1200_stop(isa);
> +
> + isa->level = 0;
> +}
> +
> +static int isa1200_of_probe(struct i2c_client *client)
> +{
> + static const char * const isa1200_supplies[] = { "vdd", "vddp" };
> + struct isa1200 *isa = i2c_get_clientdata(client);
> + struct isa1200_config *config = &isa->config;
> + struct device *dev = &client->dev;
> + struct fwnode_handle *ldo_node;
> + int ret;
> +
> + isa->clk = devm_clk_get_optional(dev, NULL);
> + if (IS_ERR(isa->clk))
> + return dev_err_probe(dev, PTR_ERR(isa->clk),
> + "failed to get clock\n");
> +
> + isa->pwm = devm_pwm_get(dev, NULL);
> + if (IS_ERR(isa->pwm)) {
> + ret = PTR_ERR(isa->pwm);
> + if (ret == -ENODEV || ret == -EINVAL)
> + isa->pwm = NULL;
> + else
> + return dev_err_probe(dev, ret, "getting PWM\n");
> + }
> +
> + if (!isa->clk && !isa->pwm)
> + return dev_err_probe(dev, -EINVAL,
> + "clock or PWM are required, none were provided\n");
> +
> + ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(isa1200_supplies),
> + isa1200_supplies);
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to set up supplies\n");
> +
> + isa->enable_gpios = devm_gpiod_get_array_optional(dev, "control",
> + GPIOD_OUT_LOW);
> + if (IS_ERR(isa->enable_gpios))
> + return dev_err_probe(dev, PTR_ERR(isa->enable_gpios),
> + "failed to get enable gpios\n");
> +
> + ldo_node = device_get_named_child_node(dev, "ldo");
> + if (!ldo_node)
> + return dev_err_probe(dev, -ENODEV,
> + "failed to get embedded LDO node\n");
> +
> + ret = fwnode_property_read_u32(ldo_node, "regulator-min-microvolt",
> + &config->ldo_voltage);
> + fwnode_handle_put(ldo_node);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "failed to get ldo voltage\n");
> +
> + config->ldo_voltage = clamp(config->ldo_voltage,
> + ISA1200_LDO_VOLTAGE_MIN,
> + ISA1200_LDO_VOLTAGE_MAX);
> +
> + config->ldo_voltage /= ISA1200_LDO_VOLTAGE_STEP;
> + if (config->ldo_voltage < ISA1200_LDO_VOLTAGE_3V1)
> + config->ldo_voltage = config->ldo_voltage -
> + ISA1200_LDO_VOLTAGE_2V3 +
> + ISA1200_LDO_VOLTAGE_BASE;
> + else
> + config->ldo_voltage -= ISA1200_LDO_VOLTAGE_3V1;
> +
> + config->mode = 0; /* LRA_MODE */
> + device_property_read_u32(dev, "imagis,mode", &config->mode);
> +
> + config->clkdiv = ISA1200_HCTRL0_CLKDIV_128;
> + device_property_read_u32(dev, "imagis,clk-div", &config->clkdiv);
> + if (!config->clkdiv)
> + return dev_err_probe(dev, -EINVAL, "clk-div cannot be zero\n");
> +
> + config->clkdiv = clamp(config->clkdiv, ISA1200_HCTRL0_CLKDIV_128,
> + ISA1200_HCTRL0_CLKDIV_128 << 3);
> +
> + ret = device_property_read_u32(dev, "imagis,pll-div", &config->plldiv);
> + if (ret < 0 || !config->plldiv)
> + config->plldiv = 1;
> +
> + config->period = 0;
> + config->freq = 0;
> + config->duty = 0;
> +
> + if (isa->clk) {
> + ret = device_property_read_u32(dev, "imagis,period-ns",
> + &config->period);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "failed to get period\n");
> +
> + /*
> + * TODO: The scale value is arbitrary, but it fits observations
> + * quite well, and the exact conversion method is unknown.
> + * The period property value returned above is the HCTRL6
> + * register value set by the vendor code, multiplied by 100.
> + */
> + config->period /= ISA1200_HCTRL6_PERIOD_SCALE;
> + config->duty = config->period >> 1;
> + }
> +
> + if (isa->pwm) {
> + struct pwm_state state;
> +
> + pwm_init_state(isa->pwm, &state);
> +
> + if (!state.period)
> + return dev_err_probe(dev, -EINVAL,
> + "PWM period cannot be zero\n");
> +
> + config->freq = div64_u64(NANO, state.period * config->clkdiv);
> + config->duty = state.period >> 1;
> +
> + ret = pwm_apply_might_sleep(isa->pwm, &state);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "failed to apply initial PWM state\n");
> + }
> +
> + /*
> + * TODO: If device is using a clock, this property should return the
> + * value written to the HCTRL5 register by downstrem code. It likely
> + * needs to be converted into a meaningful duty cycle value, though
> + * unfortunately the exact conversion mechanism is unknown. If the
> + * device uses PWM, this property will return the correct duty cycle
> + * in nanoseconds.
> + */
> + device_property_read_u32(dev, "imagis,duty-cycle-ns", &config->duty);
> +
> + return 0;
> +}
> +
> +static int isa1200_probe(struct i2c_client *client)
> +{
> + struct isa1200 *isa;
> + struct device *dev = &client->dev;
> + DECLARE_BITMAP(values, ISA1200_EN_PINS_MAX);
> + u32 val;
> + int ret;
> +
> + isa = devm_kzalloc(dev, sizeof(*isa), GFP_KERNEL);
> + if (!isa)
> + return -ENOMEM;
> +
> + isa->input = devm_input_allocate_device(dev);
> + if (!isa->input)
> + return -ENOMEM;
> +
> + i2c_set_clientdata(client, isa);
> +
> + ret = isa1200_of_probe(client);
> + if (ret)
> + return ret;
> +
> + isa->map = devm_regmap_init_i2c(client, &isa1200_regmap_config);
> + if (IS_ERR(isa->map))
> + return dev_err_probe(dev, PTR_ERR(isa->map),
> + "failed to initialize register map\n");
> +
> + ret = clk_prepare_enable(isa->clk);
> + if (ret < 0)
> + return dev_err_probe(dev, ret, "failed to enable clock\n");
> +
> + isa->clk_on = true;
> +
> + bitmap_fill(values, ISA1200_EN_PINS_MAX);
> + gpiod_multi_set_value_cansleep(isa->enable_gpios, values);
You should factor our isa1200_power_on() from isa1200_start() and use it
here.
> +
> + ret = devm_add_action_or_reset(dev, isa1200_power_off, isa);
> + if (ret)
> + return ret;
close() is taking care of powering off the device.
> +
> + usleep_range(200, 300);
> +
> + /* Read a register so we know that regmap and I2C transport works */
> + ret = regmap_read(isa->map, ISA1200_SCTRL, &val);
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to read SCTRL\n");
You should call isa1200_power_off() here. You do not know when the first
effect will play. In the meantime the device should be in low power
state.
> +
> + ret = devm_work_autocancel(dev, &isa->play_work, isa1200_play_work);
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to init work\n");
I do not think you need it here. You supply input->close() and that
should take care of stopping/canceling the work. Input core guarantees
to call close() if the device is "open" whne unregistering.
> +
> + isa->input->name = "isa1200-haptic";
> + isa->input->id.bustype = BUS_HOST;
Not BUS_I2C?
> + isa->input->dev.parent = dev;
Parent is already set by devm_input_allocate_device().
> + isa->input->close = isa1200_vibrator_close;
> +
> + input_set_drvdata(isa->input, isa);
> +
> + /* TODO: this hardware can likely support more than rumble */
> + input_set_capability(isa->input, EV_FF, FF_RUMBLE);
> +
> + ret = input_ff_create_memless(isa->input, NULL,
> + isa1200_vibrator_play_effect);
> + if (ret)
> + return dev_err_probe(dev, ret, "couldn't create FF dev\n");
"failed ..."
> +
> + ret = input_register_device(isa->input);
> + if (ret)
> + return dev_err_probe(dev, ret, "couldn't register input dev\n");
"failed ..."
> +
> + return ret;
return 0;
> +}
> +
> +static int isa1200_suspend(struct device *dev)
> +{
> + struct isa1200 *isa = dev_get_drvdata(dev);
> +
> + cancel_work_sync(&isa->play_work);
Move it under input_device_enabled().
> +
> + guard(mutex)(&isa->input->mutex);
> +
> + if (input_device_enabled(isa->input))
> + if (isa->level)
> + isa1200_stop(isa);
> +
> + return 0;
> +}
> +
> +static int isa1200_resume(struct device *dev)
> +{
> + struct isa1200 *isa = dev_get_drvdata(dev);
> +
> + guard(mutex)(&isa->input->mutex);
> +
> + if (input_device_enabled(isa->input))
> + if (isa->level)
> + isa1200_start(isa);
> +
> + return 0;
> +}
> +
> +static DEFINE_SIMPLE_DEV_PM_OPS(isa1200_pm_ops, isa1200_suspend, isa1200_resume);
> +
> +static const struct of_device_id isa1200_of_match[] = {
> + { .compatible = "imagis,isa1200" },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, isa1200_of_match);
> +
> +static struct i2c_driver isa1200_i2c_driver = {
> + .driver = {
> + .name = "isa1200",
> + .of_match_table = isa1200_of_match,
> + .pm = pm_sleep_ptr(&isa1200_pm_ops),
> + },
> + .probe = isa1200_probe,
> +};
> +module_i2c_driver(isa1200_i2c_driver);
> +
> +MODULE_AUTHOR("Linus Walleij <linusw@kernel.org>");
> +MODULE_AUTHOR("Svyatoslav Ryhel <clamor95@gmail.com>");
> +MODULE_DESCRIPTION("Imagis ISA1200 haptic feedback unit");
> +MODULE_LICENSE("GPL");
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] Input: ads7846 - consolidate coordinate filtering logic
From: Kris Bahnsen @ 2026-05-07 17:48 UTC (permalink / raw)
To: Dmitry Torokhov, linux-input
Cc: Aaro Koskinen, Mark Featherston, Marek Vasut, linux-kernel
In-Reply-To: <afl3WtbabMFNjE24@google.com>
Dmitry,
On 5/4/26 9:54 PM, Dmitry Torokhov wrote:
> The ads7846 driver has two separate filtering functions,
> ads7846_filter() and ads7846_filter_one(), for the full-duplex and
> half-duplex SPI paths, respectively.
>
> They can be consolidated by extracting the core filtering logic for a
> single command into a helper function, ads7846_filter_cmd(), which
> iterates from l->skip to l->count. The half-duplex setup function is
> updated to set l->skip = l->count - 1 so that the helper only processes
> the last sample, preserving the original half-duplex behavior.
>
> Assisted-by: Gemini:gemini-3.1-pro
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>
> Not tested so will appreciate if someone could give it a spin.
Forgive my ignorance, but I am unsure of the base commit this applies
to and I'm unable to apply it to current HEAD of linux, linux-next,
or your input tree.
I also am not easily able to test this if it doesn't cleanly apply
to the latest LTS, but will do what I can once I know what other
patches I would need to pull in to our tree.
> drivers/input/touchscreen/ads7846.c | 74 +++++++++++++++--------------
> 1 file changed, 38 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c
> index d3b529333ca2..093f4b56cc18 100644
> --- a/drivers/input/touchscreen/ads7846.c
> +++ b/drivers/input/touchscreen/ads7846.c
> @@ -759,52 +759,54 @@ static bool ads7846_cmd_need_settle(enum ads7846_cmds cmd_idx)
> return false;
> }
>
> -static int ads7846_filter(struct ads7846 *ts)
> +static int ads7846_filter_cmd(struct ads7846 *ts, unsigned int cmd_idx)
> {
> struct ads7846_packet *packet = ts->packet;
> - int action;
> - int val;
> - unsigned int cmd_idx, b;
> + struct ads7846_buf_layout *l = &packet->l[cmd_idx];
> + unsigned int b;
>
> - packet->ignore = false;
> - for (cmd_idx = packet->last_cmd_idx; cmd_idx < packet->cmds - 1; cmd_idx++) {
> - struct ads7846_buf_layout *l = &packet->l[cmd_idx];
> + for (b = l->skip; b < l->count; b++) {
> + int val = ads7846_get_value(&packet->rx[l->offset + b]);
>
> - packet->last_cmd_idx = cmd_idx;
> + switch (ts->filter(ts->filter_data, cmd_idx, &val)) {
> + case ADS7846_FILTER_REPEAT:
> + if (b == l->count - 1)
> + return -EAGAIN;
> + break;
>
> - for (b = l->skip; b < l->count; b++) {
> - val = ads7846_get_value(&packet->rx[l->offset + b]);
> -
> - action = ts->filter(ts->filter_data, cmd_idx, &val);
> - if (action == ADS7846_FILTER_REPEAT) {
> - if (b == l->count - 1)
> - return -EAGAIN;
> - } else if (action == ADS7846_FILTER_OK) {
> - ads7846_set_cmd_val(ts, cmd_idx, val);
> - break;
> - } else {
> - packet->ignore = true;
> - return 0;
> - }
> + case ADS7846_FILTER_OK:
> + ads7846_set_cmd_val(ts, cmd_idx, val);
> + return 0;
> +
> + case ADS7846_FILTER_IGNORE:
> + default:
> + return -EIO;
> }
> }
>
> - return 0;
> + return -EIO;
> }
>
> -static int ads7846_filter_one(struct ads7846 *ts, unsigned int cmd_idx)
> +static int ads7846_filter(struct ads7846 *ts)
> {
> struct ads7846_packet *packet = ts->packet;
> - struct ads7846_buf_layout *l = &packet->l[cmd_idx];
> - int action, val;
> -
> - val = ads7846_get_value(&packet->rx[l->offset + l->count - 1]);
> - action = ts->filter(ts->filter_data, cmd_idx, &val);
> - if (action == ADS7846_FILTER_REPEAT)
> - return -EAGAIN;
> - else if (action != ADS7846_FILTER_OK)
> - return -EIO;
> - ads7846_set_cmd_val(ts, cmd_idx, val);
> + unsigned int cmd_idx;
> + int error;
> +
> + packet->ignore = false;
> + for (cmd_idx = packet->last_cmd_idx; cmd_idx < packet->cmds - 1; cmd_idx++) {
> + packet->last_cmd_idx = cmd_idx;
> +
> + error = ads7846_filter_cmd(ts, cmd_idx);
> + if (error) {
> + if (error == -EAGAIN)
> + return -EAGAIN;
> +
> + packet->ignore = true;
> + return 0;
> + }
> + }
> +
> return 0;
> }
>
> @@ -857,7 +859,7 @@ static void ads7846_halfd_read_state(struct ads7846 *ts)
> if (msg_idx == ts->msg_count - 1)
> break;
>
> - error = ads7846_filter_one(ts, msg_idx);
> + error = ads7846_filter_cmd(ts, msg_idx);
> if (error == -EAGAIN) {
> continue;
> } else if (error) {
> @@ -1119,7 +1121,7 @@ static int ads7846_halfd_spi_msg(struct ads7846 *ts,
> l->offset = offset;
> offset += max_count;
> l->count = max_count;
> - l->skip = 0;
> + l->skip = max_count - 1;
> size += sizeof(*packet->rx) * max_count;
> }
>
--
Kris Bahnsen
Software Engineer
embeddedTS
^ permalink raw reply
* Re: [PATCH] Input: gameport: fm801-gp - Simplify initialisation of pci_device_id array
From: Dmitry Torokhov @ 2026-05-07 17:07 UTC (permalink / raw)
To: Uwe Kleine-König (The Capable Hub)
Cc: Kees Cook, linux-input, linux-kernel, Markus Schneider-Pargmann
In-Reply-To: <20260507160051.3315630-2-u.kleine-koenig@baylibre.com>
On Thu, May 07, 2026 at 06:00:51PM +0200, Uwe Kleine-König (The Capable Hub) wrote:
> Instead of assigning the pci_device_id members using a list (which is
> hard to read as you need to look at the order of the members in that
> struct in parallel) use the PCI_VDEVICE() convenience macro to compact
> the initialisation while improving readability.
>
> Also drop trailing zeros that the compiler will care about then.
>
> The change doesn't introduce binary changes to the compiled driver,
> verified on both ARCH=x86 and ARCH=arm64.
>
> Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>
Applied, thank you.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v4] Input: ads7846 - don't use scratch for tx_buf when clearing register
From: Dmitry Torokhov @ 2026-05-07 17:00 UTC (permalink / raw)
To: Kris Bahnsen, Marek Vasut
Cc: stable, Mark Featherston, linux-input, linux-kernel
In-Reply-To: <20260507164943.760009-1-kris@embeddedTS.com>
On Thu, May 07, 2026 at 04:49:43PM +0000, Kris Bahnsen wrote:
> The workaround for XPT2046 clears the command register, giving the
> touchscreen controller a NOP. The change incorrectly re-uses the
> req->scratch variable which is used as rx_buf for xfer[5], so by
> the time xfer[6] occurs, the contents of req->scratch may not be
> 0. It was found that the touchscreen controller can end up in
> a completely unresponsive state due to it being given a command
> the driver does not expect.
>
> Instead, rely on the spi_transfer behavior of tx_buf being NULL to
> transmit all 0 bits and use the scratch variable for the rx_buf for
> both the 1 byte command to and 2 byte response from the controller.
>
> Also relocates the scratch member of struct ser_req to force it
> into a different cache line to prevent any potential issues of
> DMA stepping on unrelated data in other struct members due to
> sharing the same cache line.
>
> This change was tested on real TSC2046 and ADS7843 controllers,
> but not the XPT2046 the workaround was originally created for.
> Confirming that the original modification to clear the command
> register does not impact either real controller.
>
> Fixes: 781a07da9bb94 ("Input: ads7846 - add dummy command register clearing cycle")
> Cc: stable@vger.kernel.org
> Co-developed-by: Mark Featherston <mark@embeddedTS.com>
> Signed-off-by: Mark Featherston <mark@embeddedTS.com>
> Signed-off-by: Kris Bahnsen <kris@embeddedTS.com>
> ---
>
> V1 -> V2: Don't use rx_buf when clearing command reg
> V2 -> V3: Modify original 2 xfer command to eliminate dev_err()
> output on xfer with len and NULL buffers
> V3 -> V4: Move scratch to end of ser_req to force it to a new
> cache line.
>
> V4 Note: Change to moving scratch was tested against an SPI
> controller without DMA. We do not currently have a
> platform using this controller on an SPI bus supporting
> DMA.
Marek, any chance you could give it a quick spin?
Thanks!
--
Dmitry
^ permalink raw reply
* [PATCH v4] Input: ads7846 - don't use scratch for tx_buf when clearing register
From: Kris Bahnsen @ 2026-05-07 16:49 UTC (permalink / raw)
To: Dmitry Torokhov, Marek Vasut
Cc: Kris Bahnsen, stable, Mark Featherston, linux-input, linux-kernel
The workaround for XPT2046 clears the command register, giving the
touchscreen controller a NOP. The change incorrectly re-uses the
req->scratch variable which is used as rx_buf for xfer[5], so by
the time xfer[6] occurs, the contents of req->scratch may not be
0. It was found that the touchscreen controller can end up in
a completely unresponsive state due to it being given a command
the driver does not expect.
Instead, rely on the spi_transfer behavior of tx_buf being NULL to
transmit all 0 bits and use the scratch variable for the rx_buf for
both the 1 byte command to and 2 byte response from the controller.
Also relocates the scratch member of struct ser_req to force it
into a different cache line to prevent any potential issues of
DMA stepping on unrelated data in other struct members due to
sharing the same cache line.
This change was tested on real TSC2046 and ADS7843 controllers,
but not the XPT2046 the workaround was originally created for.
Confirming that the original modification to clear the command
register does not impact either real controller.
Fixes: 781a07da9bb94 ("Input: ads7846 - add dummy command register clearing cycle")
Cc: stable@vger.kernel.org
Co-developed-by: Mark Featherston <mark@embeddedTS.com>
Signed-off-by: Mark Featherston <mark@embeddedTS.com>
Signed-off-by: Kris Bahnsen <kris@embeddedTS.com>
---
V1 -> V2: Don't use rx_buf when clearing command reg
V2 -> V3: Modify original 2 xfer command to eliminate dev_err()
output on xfer with len and NULL buffers
V3 -> V4: Move scratch to end of ser_req to force it to a new
cache line.
V4 Note: Change to moving scratch was tested against an SPI
controller without DMA. We do not currently have a
platform using this controller on an SPI bus supporting
DMA.
drivers/input/touchscreen/ads7846.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c
index 4b39f7212d35..1ae1ae42582a 100644
--- a/drivers/input/touchscreen/ads7846.c
+++ b/drivers/input/touchscreen/ads7846.c
@@ -325,7 +325,6 @@ struct ser_req {
u8 ref_on;
u8 command;
u8 ref_off;
- u16 scratch;
struct spi_message msg;
struct spi_transfer xfer[8];
/*
@@ -333,6 +332,7 @@ struct ser_req {
* transfer buffers to live in their own cache lines.
*/
__be16 sample ____cacheline_aligned;
+ u16 scratch;
};
struct ads7845_ser_req {
@@ -403,8 +403,7 @@ static int ads7846_read12_ser(struct device *dev, unsigned command)
spi_message_add_tail(&req->xfer[5], &req->msg);
/* clear the command register */
- req->scratch = 0;
- req->xfer[6].tx_buf = &req->scratch;
+ req->xfer[6].rx_buf = &req->scratch;
req->xfer[6].len = 1;
spi_message_add_tail(&req->xfer[6], &req->msg);
base-commit: dd6c438c3e64a5ff0b5d7e78f7f9be547803ef1b
--
2.34.1
^ permalink raw reply related
* [PATCH] Input: gameport: fm801-gp - Simplify initialisation of pci_device_id array
From: Uwe Kleine-König (The Capable Hub) @ 2026-05-07 16:00 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Kees Cook, linux-input, linux-kernel, Markus Schneider-Pargmann
Instead of assigning the pci_device_id members using a list (which is
hard to read as you need to look at the order of the members in that
struct in parallel) use the PCI_VDEVICE() convenience macro to compact
the initialisation while improving readability.
Also drop trailing zeros that the compiler will care about then.
The change doesn't introduce binary changes to the compiled driver,
verified on both ARCH=x86 and ARCH=arm64.
Signed-off-by: Uwe Kleine-König (The Capable Hub) <u.kleine-koenig@baylibre.com>
---
Hello,
this is a preparing change for making struct pci_device_id::driver_data
an anonymous union (similar to
https://lore.kernel.org/all/cover.1776579304.git.u.kleine-koenig@baylibre.com/).
This requires named initializers for .driver_data. In this case the
initialization can be dropped as the driver doesn't make use of
.driver_data at all.
Best regards
Uwe
drivers/input/gameport/fm801-gp.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/input/gameport/fm801-gp.c b/drivers/input/gameport/fm801-gp.c
index 423cccdea34f..1e8c6c044844 100644
--- a/drivers/input/gameport/fm801-gp.c
+++ b/drivers/input/gameport/fm801-gp.c
@@ -125,8 +125,8 @@ static void fm801_gp_remove(struct pci_dev *pci)
}
static const struct pci_device_id fm801_gp_id_table[] = {
- { PCI_VENDOR_ID_FORTEMEDIA, PCI_DEVICE_ID_FM801_GP, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0 },
- { 0 }
+ { PCI_VDEVICE(FORTEMEDIA, PCI_DEVICE_ID_FM801_GP) },
+ { }
};
MODULE_DEVICE_TABLE(pci, fm801_gp_id_table);
base-commit: 254f49634ee16a731174d2ae34bc50bd5f45e731
--
2.47.3
^ permalink raw reply related
* Re: [PATCH v4 5/6 RESEND] mfd: motorola-cpcap: diverge configuration per-board
From: Svyatoslav Ryhel @ 2026-05-07 14:42 UTC (permalink / raw)
To: Lee Jones
Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Pavel Machek, David Lechner, Tony Lindgren, linux-input,
devicetree, linux-kernel, linux-leds
In-Reply-To: <20260507140519.GO305027@google.com>
чт, 7 трав. 2026 р. о 17:05 Lee Jones <lee@kernel.org> пише:
>
> On Tue, 28 Apr 2026, Svyatoslav Ryhel wrote:
>
> > MFD have rigid subdevice structure which does not allow flexible dynamic
> > subdevice linking. Address this by diverging CPCAP subdevice composition
> > to take into account board specific configuration.
> >
> > Create a common default subdevice composition, rename existing subdevice
> > composition into cpcap_mapphone_mfd_devices since it targets mainly
> > Mapphone board.
> >
> > Removed st,6556002 as it is no longer applicable to all cases and
> > duplicates motorola,cpcap, which is used as the default composition.
> >
> > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > ---
>
> Changelog?
>
> > drivers/mfd/motorola-cpcap.c | 101 ++++++++++++++++++++++++++++-------
> > 1 file changed, 83 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/mfd/motorola-cpcap.c b/drivers/mfd/motorola-cpcap.c
> > index d8243b956f87..516d1e33affa 100644
> > --- a/drivers/mfd/motorola-cpcap.c
> > +++ b/drivers/mfd/motorola-cpcap.c
> > @@ -12,6 +12,7 @@
> > #include <linux/kernel.h>
> > #include <linux/module.h>
> > #include <linux/mod_devicetable.h>
> > +#include <linux/property.h>
> > #include <linux/regmap.h>
> > #include <linux/sysfs.h>
> >
> > @@ -24,10 +25,16 @@
> > #define CPCAP_REGISTER_SIZE 4
> > #define CPCAP_REGISTER_BITS 16
> >
> > +struct cpcap_chip_data {
> > + const struct mfd_cell *mfd_devices;
> > + unsigned int num_devices;
> > +};
>
> This is a red flag.
>
> > struct cpcap_ddata {
> > struct spi_device *spi;
> > struct regmap_irq *irqs;
> > struct regmap_irq_chip_data *irqdata[CPCAP_NR_IRQ_CHIPS];
> > + const struct cpcap_chip_data *cdata;
> > const struct regmap_config *regmap_conf;
> > struct regmap *regmap;
> > };
> > @@ -195,20 +202,6 @@ static int cpcap_init_irq(struct cpcap_ddata *cpcap)
> > return 0;
> > }
> >
> > -static const struct of_device_id cpcap_of_match[] = {
> > - { .compatible = "motorola,cpcap", },
> > - { .compatible = "st,6556002", },
> > - {},
> > -};
> > -MODULE_DEVICE_TABLE(of, cpcap_of_match);
> > -
> > -static const struct spi_device_id cpcap_spi_ids[] = {
> > - { .name = "cpcap", },
> > - { .name = "6556002", },
> > - {},
> > -};
> > -MODULE_DEVICE_TABLE(spi, cpcap_spi_ids);
> > -
> > static const struct regmap_config cpcap_regmap_config = {
> > .reg_bits = 16,
> > .reg_stride = 4,
> > @@ -241,7 +234,56 @@ static int cpcap_resume(struct device *dev)
> >
> > static DEFINE_SIMPLE_DEV_PM_OPS(cpcap_pm, cpcap_suspend, cpcap_resume);
> >
> > -static const struct mfd_cell cpcap_mfd_devices[] = {
> > +static const struct mfd_cell cpcap_default_mfd_devices[] = {
> > + {
> > + .name = "cpcap_adc",
> > + .of_compatible = "motorola,cpcap-adc",
> > + }, {
> > + .name = "cpcap_battery",
> > + .of_compatible = "motorola,cpcap-battery",
> > + }, {
> > + .name = "cpcap-regulator",
> > + .of_compatible = "motorola,cpcap-regulator",
> > + }, {
> > + .name = "cpcap-rtc",
> > + .of_compatible = "motorola,cpcap-rtc",
> > + }, {
> > + .name = "cpcap-pwrbutton",
> > + .of_compatible = "motorola,cpcap-pwrbutton",
> > + }, {
> > + .name = "cpcap-usb-phy",
> > + .of_compatible = "motorola,cpcap-usb-phy",
> > + }, {
> > + .name = "cpcap-led",
> > + .id = 0,
> > + .of_compatible = "motorola,cpcap-led-red",
> > + }, {
> > + .name = "cpcap-led",
> > + .id = 1,
> > + .of_compatible = "motorola,cpcap-led-green",
> > + }, {
> > + .name = "cpcap-led",
> > + .id = 2,
> > + .of_compatible = "motorola,cpcap-led-blue",
> > + }, {
> > + .name = "cpcap-led",
> > + .id = 3,
> > + .of_compatible = "motorola,cpcap-led-adl",
> > + }, {
> > + .name = "cpcap-led",
> > + .id = 4,
> > + .of_compatible = "motorola,cpcap-led-cp",
> > + }, {
> > + .name = "cpcap-codec",
> > + },
> > +};
Lee, would you mind if I convert these mfd_cell structures to use
macros in this commit since I am refactoring them anyway?
> > +
> > +static const struct cpcap_chip_data cpcap_default_data = {
> > + .mfd_devices = cpcap_default_mfd_devices,
> > + .num_devices = ARRAY_SIZE(cpcap_default_mfd_devices),
> > +};
> > +
> > +static const struct mfd_cell cpcap_mapphone_mfd_devices[] = {
> > {
> > .name = "cpcap_adc",
> > .of_compatible = "motorola,mapphone-cpcap-adc",
> > @@ -285,7 +327,12 @@ static const struct mfd_cell cpcap_mfd_devices[] = {
> > .of_compatible = "motorola,cpcap-led-cp",
> > }, {
> > .name = "cpcap-codec",
> > - }
> > + },
> > +};
> > +
> > +static const struct cpcap_chip_data cpcap_mapphone_data = {
> > + .mfd_devices = cpcap_mapphone_mfd_devices,
> > + .num_devices = ARRAY_SIZE(cpcap_mapphone_mfd_devices),
> > };
> >
> > static int cpcap_probe(struct spi_device *spi)
> > @@ -297,9 +344,17 @@ static int cpcap_probe(struct spi_device *spi)
> > if (!cpcap)
> > return -ENOMEM;
> >
> > + cpcap->cdata = device_get_match_data(&spi->dev);
> > + if (!cpcap->cdata)
> > + return -ENODEV;
> > +
> > cpcap->spi = spi;
> > spi_set_drvdata(spi, cpcap);
> >
> > @@ -331,16 +382,24 @@ static int cpcap_probe(struct spi_device *spi)
> > spi->dev.coherent_dma_mask = 0;
> > spi->dev.dma_mask = &spi->dev.coherent_dma_mask;
> >
> > - return devm_mfd_add_devices(&spi->dev, 0, cpcap_mfd_devices,
> > - ARRAY_SIZE(cpcap_mfd_devices), NULL, 0, NULL);
> > + return devm_mfd_add_devices(&spi->dev, 0, cpcap->cdata->mfd_devices,
> > + cpcap->cdata->num_devices, NULL, 0, NULL);
> > }
> >
> > +static const struct of_device_id cpcap_of_match[] = {
> > + { .compatible = "motorola,cpcap", .data = &cpcap_default_data },
> > + { .compatible = "motorola,mapphone-cpcap", .data = &cpcap_mapphone_data },
>
> We don't allow data from one device registration API (MFD) to be passed
> through another (OF) because it tends to lead to all sorts of "creative
> solutions". Pass a value instead and match on that in a switch()
> statement like all of the other MFD drivers do.
>
> > + { /* sentinel */ }
> > +};
> > +MODULE_DEVICE_TABLE(of, cpcap_of_match);
> > +
> > +static const struct spi_device_id cpcap_spi_ids[] = {
> > + { .name = "cpcap", .driver_data = (kernel_ulong_t)&cpcap_default_data },
> > + { .name = "mapphone-cpcap", .driver_data = (kernel_ulong_t)&cpcap_mapphone_data },
> > + { /* sentinel */ }
> > +};
> > +MODULE_DEVICE_TABLE(spi, cpcap_spi_ids);
> > +
> > static struct spi_driver cpcap_driver = {
> > .driver = {
> > .name = "cpcap-core",
> > --
> > 2.51.0
> >
> >
>
> --
> Lee Jones
^ permalink raw reply
* Re: [PATCH v4 6/6 RESEND] mfd: motorola-cpcap: add support for Mot CPCAP composition
From: Svyatoslav Ryhel @ 2026-05-07 14:36 UTC (permalink / raw)
To: Lee Jones
Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Pavel Machek, David Lechner, Tony Lindgren, linux-input,
devicetree, linux-kernel, linux-leds
In-Reply-To: <20260507140715.GP305027@google.com>
чт, 7 трав. 2026 р. о 17:07 Lee Jones <lee@kernel.org> пише:
>
> On Tue, 28 Apr 2026, Svyatoslav Ryhel wrote:
>
> > Add a MFD subdevice composition used in Tegra20 based Mot board
> > (Motorola Atrix 4G and Droid X2).
> >
> > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > ---
> > drivers/mfd/motorola-cpcap.c | 50 ++++++++++++++++++++++++++++++++++++
> > 1 file changed, 50 insertions(+)
> >
> > diff --git a/drivers/mfd/motorola-cpcap.c b/drivers/mfd/motorola-cpcap.c
> > index 516d1e33affa..fdec92f5c6b0 100644
> > --- a/drivers/mfd/motorola-cpcap.c
> > +++ b/drivers/mfd/motorola-cpcap.c
> > @@ -335,6 +335,54 @@ static const struct cpcap_chip_data cpcap_mapphone_data = {
> > .num_devices = ARRAY_SIZE(cpcap_mapphone_mfd_devices),
> > };
> >
> > +/*
> > + * The Mot board features a USB-PHY and charger similar to the ones in
> > + * Mapphone; however, because Mot is based on Tegra20, it is incompatible
> > + * with the existing implementation, which is tightly interconnected with
> > + * the OMAP USB PHY.
> > + */
> > +static const struct mfd_cell cpcap_mot_mfd_devices[] = {
> > + {
> > + .name = "cpcap_adc",
> > + .of_compatible = "motorola,mot-cpcap-adc",
> > + }, {
> > + .name = "cpcap_battery",
> > + .of_compatible = "motorola,cpcap-battery",
> > + }, {
> > + .name = "cpcap-regulator",
> > + .of_compatible = "motorola,mot-cpcap-regulator",
> > + }, {
> > + .name = "cpcap-rtc",
> > + .of_compatible = "motorola,cpcap-rtc",
> > + }, {
> > + .name = "cpcap-pwrbutton",
> > + .of_compatible = "motorola,cpcap-pwrbutton",
> > + }, {
> > + .name = "cpcap-led",
> > + .id = 0,
> > + .of_compatible = "motorola,cpcap-led-red",
> > + }, {
> > + .name = "cpcap-led",
> > + .id = 1,
> > + .of_compatible = "motorola,cpcap-led-green",
> > + }, {
> > + .name = "cpcap-led",
> > + .id = 2,
> > + .of_compatible = "motorola,cpcap-led-blue",
> > + }, {
> > + .name = "cpcap-led",
> > + .id = 3,
> > + .of_compatible = "motorola,cpcap-led-adl",
>
> MFD_CELL_OF() for all.
>
> > + }, {
> > + .name = "cpcap-codec",
> > + },
>
> MFD_CELL_NAME()
>
I was not aware these macros exist. Thank you for pointing to them.
> > +};
> > +
> > +static const struct cpcap_chip_data cpcap_mot_data = {
> > + .mfd_devices = cpcap_mot_mfd_devices,
> > + .num_devices = ARRAY_SIZE(cpcap_mot_mfd_devices),
> > +};
> > +
> > static int cpcap_probe(struct spi_device *spi)
> > {
> > struct cpcap_ddata *cpcap;
> > @@ -389,6 +437,7 @@ static int cpcap_probe(struct spi_device *spi)
> > static const struct of_device_id cpcap_of_match[] = {
> > { .compatible = "motorola,cpcap", .data = &cpcap_default_data },
> > { .compatible = "motorola,mapphone-cpcap", .data = &cpcap_mapphone_data },
> > + { .compatible = "motorola,mot-cpcap", .data = &cpcap_mot_data },
> > { /* sentinel */ }
> > };
> > MODULE_DEVICE_TABLE(of, cpcap_of_match);
> > @@ -396,6 +445,7 @@ MODULE_DEVICE_TABLE(of, cpcap_of_match);
> > static const struct spi_device_id cpcap_spi_ids[] = {
> > { .name = "cpcap", .driver_data = (kernel_ulong_t)&cpcap_default_data },
> > { .name = "mapphone-cpcap", .driver_data = (kernel_ulong_t)&cpcap_mapphone_data },
> > + { .name = "mot-cpcap", .driver_data = (kernel_ulong_t)&cpcap_mot_data },
> > { /* sentinel */ }
> > };
> > MODULE_DEVICE_TABLE(spi, cpcap_spi_ids);
> > --
> > 2.51.0
> >
>
> --
> Lee Jones
^ permalink raw reply
* Re: [PATCH v4 5/6 RESEND] mfd: motorola-cpcap: diverge configuration per-board
From: Svyatoslav Ryhel @ 2026-05-07 14:33 UTC (permalink / raw)
To: Lee Jones
Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Pavel Machek, David Lechner, Tony Lindgren, linux-input,
devicetree, linux-kernel, linux-leds
In-Reply-To: <20260507140519.GO305027@google.com>
чт, 7 трав. 2026 р. о 17:05 Lee Jones <lee@kernel.org> пише:
>
> On Tue, 28 Apr 2026, Svyatoslav Ryhel wrote:
>
> > MFD have rigid subdevice structure which does not allow flexible dynamic
> > subdevice linking. Address this by diverging CPCAP subdevice composition
> > to take into account board specific configuration.
> >
> > Create a common default subdevice composition, rename existing subdevice
> > composition into cpcap_mapphone_mfd_devices since it targets mainly
> > Mapphone board.
> >
> > Removed st,6556002 as it is no longer applicable to all cases and
> > duplicates motorola,cpcap, which is used as the default composition.
> >
> > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > ---
>
> Changelog?
>
Changelog is in the cover.
> > drivers/mfd/motorola-cpcap.c | 101 ++++++++++++++++++++++++++++-------
> > 1 file changed, 83 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/mfd/motorola-cpcap.c b/drivers/mfd/motorola-cpcap.c
> > index d8243b956f87..516d1e33affa 100644
> > --- a/drivers/mfd/motorola-cpcap.c
> > +++ b/drivers/mfd/motorola-cpcap.c
> > @@ -12,6 +12,7 @@
> > #include <linux/kernel.h>
> > #include <linux/module.h>
> > #include <linux/mod_devicetable.h>
> > +#include <linux/property.h>
> > #include <linux/regmap.h>
> > #include <linux/sysfs.h>
> >
> > @@ -24,10 +25,16 @@
> > #define CPCAP_REGISTER_SIZE 4
> > #define CPCAP_REGISTER_BITS 16
> >
> > +struct cpcap_chip_data {
> > + const struct mfd_cell *mfd_devices;
> > + unsigned int num_devices;
> > +};
>
> This is a red flag.
>
> > struct cpcap_ddata {
> > struct spi_device *spi;
> > struct regmap_irq *irqs;
> > struct regmap_irq_chip_data *irqdata[CPCAP_NR_IRQ_CHIPS];
> > + const struct cpcap_chip_data *cdata;
> > const struct regmap_config *regmap_conf;
> > struct regmap *regmap;
> > };
> > @@ -195,20 +202,6 @@ static int cpcap_init_irq(struct cpcap_ddata *cpcap)
> > return 0;
> > }
> >
> > -static const struct of_device_id cpcap_of_match[] = {
> > - { .compatible = "motorola,cpcap", },
> > - { .compatible = "st,6556002", },
> > - {},
> > -};
> > -MODULE_DEVICE_TABLE(of, cpcap_of_match);
> > -
> > -static const struct spi_device_id cpcap_spi_ids[] = {
> > - { .name = "cpcap", },
> > - { .name = "6556002", },
> > - {},
> > -};
> > -MODULE_DEVICE_TABLE(spi, cpcap_spi_ids);
> > -
> > static const struct regmap_config cpcap_regmap_config = {
> > .reg_bits = 16,
> > .reg_stride = 4,
> > @@ -241,7 +234,56 @@ static int cpcap_resume(struct device *dev)
> >
> > static DEFINE_SIMPLE_DEV_PM_OPS(cpcap_pm, cpcap_suspend, cpcap_resume);
> >
> > -static const struct mfd_cell cpcap_mfd_devices[] = {
> > +static const struct mfd_cell cpcap_default_mfd_devices[] = {
> > + {
> > + .name = "cpcap_adc",
> > + .of_compatible = "motorola,cpcap-adc",
> > + }, {
> > + .name = "cpcap_battery",
> > + .of_compatible = "motorola,cpcap-battery",
> > + }, {
> > + .name = "cpcap-regulator",
> > + .of_compatible = "motorola,cpcap-regulator",
> > + }, {
> > + .name = "cpcap-rtc",
> > + .of_compatible = "motorola,cpcap-rtc",
> > + }, {
> > + .name = "cpcap-pwrbutton",
> > + .of_compatible = "motorola,cpcap-pwrbutton",
> > + }, {
> > + .name = "cpcap-usb-phy",
> > + .of_compatible = "motorola,cpcap-usb-phy",
> > + }, {
> > + .name = "cpcap-led",
> > + .id = 0,
> > + .of_compatible = "motorola,cpcap-led-red",
> > + }, {
> > + .name = "cpcap-led",
> > + .id = 1,
> > + .of_compatible = "motorola,cpcap-led-green",
> > + }, {
> > + .name = "cpcap-led",
> > + .id = 2,
> > + .of_compatible = "motorola,cpcap-led-blue",
> > + }, {
> > + .name = "cpcap-led",
> > + .id = 3,
> > + .of_compatible = "motorola,cpcap-led-adl",
> > + }, {
> > + .name = "cpcap-led",
> > + .id = 4,
> > + .of_compatible = "motorola,cpcap-led-cp",
> > + }, {
> > + .name = "cpcap-codec",
> > + },
> > +};
> > +
> > +static const struct cpcap_chip_data cpcap_default_data = {
> > + .mfd_devices = cpcap_default_mfd_devices,
> > + .num_devices = ARRAY_SIZE(cpcap_default_mfd_devices),
> > +};
> > +
> > +static const struct mfd_cell cpcap_mapphone_mfd_devices[] = {
> > {
> > .name = "cpcap_adc",
> > .of_compatible = "motorola,mapphone-cpcap-adc",
> > @@ -285,7 +327,12 @@ static const struct mfd_cell cpcap_mfd_devices[] = {
> > .of_compatible = "motorola,cpcap-led-cp",
> > }, {
> > .name = "cpcap-codec",
> > - }
> > + },
> > +};
> > +
> > +static const struct cpcap_chip_data cpcap_mapphone_data = {
> > + .mfd_devices = cpcap_mapphone_mfd_devices,
> > + .num_devices = ARRAY_SIZE(cpcap_mapphone_mfd_devices),
> > };
> >
> > static int cpcap_probe(struct spi_device *spi)
> > @@ -297,9 +344,17 @@ static int cpcap_probe(struct spi_device *spi)
> > if (!cpcap)
> > return -ENOMEM;
> >
> > + cpcap->cdata = device_get_match_data(&spi->dev);
> > + if (!cpcap->cdata)
> > + return -ENODEV;
> > +
> > cpcap->spi = spi;
> > spi_set_drvdata(spi, cpcap);
> >
> > @@ -331,16 +382,24 @@ static int cpcap_probe(struct spi_device *spi)
> > spi->dev.coherent_dma_mask = 0;
> > spi->dev.dma_mask = &spi->dev.coherent_dma_mask;
> >
> > - return devm_mfd_add_devices(&spi->dev, 0, cpcap_mfd_devices,
> > - ARRAY_SIZE(cpcap_mfd_devices), NULL, 0, NULL);
> > + return devm_mfd_add_devices(&spi->dev, 0, cpcap->cdata->mfd_devices,
> > + cpcap->cdata->num_devices, NULL, 0, NULL);
> > }
> >
> > +static const struct of_device_id cpcap_of_match[] = {
> > + { .compatible = "motorola,cpcap", .data = &cpcap_default_data },
> > + { .compatible = "motorola,mapphone-cpcap", .data = &cpcap_mapphone_data },
>
> We don't allow data from one device registration API (MFD) to be passed
> through another (OF) because it tends to lead to all sorts of "creative
> solutions". Pass a value instead and match on that in a switch()
> statement like all of the other MFD drivers do.
>
You don't allow this. I have not seen this enforced anywhere in the
kernel except the mfd subsystem. Fine, does not matter, if this makes
you happy I will adjust.
> > + { /* sentinel */ }
> > +};
> > +MODULE_DEVICE_TABLE(of, cpcap_of_match);
> > +
> > +static const struct spi_device_id cpcap_spi_ids[] = {
> > + { .name = "cpcap", .driver_data = (kernel_ulong_t)&cpcap_default_data },
> > + { .name = "mapphone-cpcap", .driver_data = (kernel_ulong_t)&cpcap_mapphone_data },
> > + { /* sentinel */ }
> > +};
> > +MODULE_DEVICE_TABLE(spi, cpcap_spi_ids);
> > +
> > static struct spi_driver cpcap_driver = {
> > .driver = {
> > .name = "cpcap-core",
> > --
> > 2.51.0
> >
> >
>
> --
> Lee Jones
^ permalink raw reply
* Re: [PATCH v4 6/6 RESEND] mfd: motorola-cpcap: add support for Mot CPCAP composition
From: Lee Jones @ 2026-05-07 14:07 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Pavel Machek, David Lechner, Tony Lindgren, linux-input,
devicetree, linux-kernel, linux-leds
In-Reply-To: <20260428153611.142816-7-clamor95@gmail.com>
On Tue, 28 Apr 2026, Svyatoslav Ryhel wrote:
> Add a MFD subdevice composition used in Tegra20 based Mot board
> (Motorola Atrix 4G and Droid X2).
>
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> ---
> drivers/mfd/motorola-cpcap.c | 50 ++++++++++++++++++++++++++++++++++++
> 1 file changed, 50 insertions(+)
>
> diff --git a/drivers/mfd/motorola-cpcap.c b/drivers/mfd/motorola-cpcap.c
> index 516d1e33affa..fdec92f5c6b0 100644
> --- a/drivers/mfd/motorola-cpcap.c
> +++ b/drivers/mfd/motorola-cpcap.c
> @@ -335,6 +335,54 @@ static const struct cpcap_chip_data cpcap_mapphone_data = {
> .num_devices = ARRAY_SIZE(cpcap_mapphone_mfd_devices),
> };
>
> +/*
> + * The Mot board features a USB-PHY and charger similar to the ones in
> + * Mapphone; however, because Mot is based on Tegra20, it is incompatible
> + * with the existing implementation, which is tightly interconnected with
> + * the OMAP USB PHY.
> + */
> +static const struct mfd_cell cpcap_mot_mfd_devices[] = {
> + {
> + .name = "cpcap_adc",
> + .of_compatible = "motorola,mot-cpcap-adc",
> + }, {
> + .name = "cpcap_battery",
> + .of_compatible = "motorola,cpcap-battery",
> + }, {
> + .name = "cpcap-regulator",
> + .of_compatible = "motorola,mot-cpcap-regulator",
> + }, {
> + .name = "cpcap-rtc",
> + .of_compatible = "motorola,cpcap-rtc",
> + }, {
> + .name = "cpcap-pwrbutton",
> + .of_compatible = "motorola,cpcap-pwrbutton",
> + }, {
> + .name = "cpcap-led",
> + .id = 0,
> + .of_compatible = "motorola,cpcap-led-red",
> + }, {
> + .name = "cpcap-led",
> + .id = 1,
> + .of_compatible = "motorola,cpcap-led-green",
> + }, {
> + .name = "cpcap-led",
> + .id = 2,
> + .of_compatible = "motorola,cpcap-led-blue",
> + }, {
> + .name = "cpcap-led",
> + .id = 3,
> + .of_compatible = "motorola,cpcap-led-adl",
MFD_CELL_OF() for all.
> + }, {
> + .name = "cpcap-codec",
> + },
MFD_CELL_NAME()
> +};
> +
> +static const struct cpcap_chip_data cpcap_mot_data = {
> + .mfd_devices = cpcap_mot_mfd_devices,
> + .num_devices = ARRAY_SIZE(cpcap_mot_mfd_devices),
> +};
> +
> static int cpcap_probe(struct spi_device *spi)
> {
> struct cpcap_ddata *cpcap;
> @@ -389,6 +437,7 @@ static int cpcap_probe(struct spi_device *spi)
> static const struct of_device_id cpcap_of_match[] = {
> { .compatible = "motorola,cpcap", .data = &cpcap_default_data },
> { .compatible = "motorola,mapphone-cpcap", .data = &cpcap_mapphone_data },
> + { .compatible = "motorola,mot-cpcap", .data = &cpcap_mot_data },
> { /* sentinel */ }
> };
> MODULE_DEVICE_TABLE(of, cpcap_of_match);
> @@ -396,6 +445,7 @@ MODULE_DEVICE_TABLE(of, cpcap_of_match);
> static const struct spi_device_id cpcap_spi_ids[] = {
> { .name = "cpcap", .driver_data = (kernel_ulong_t)&cpcap_default_data },
> { .name = "mapphone-cpcap", .driver_data = (kernel_ulong_t)&cpcap_mapphone_data },
> + { .name = "mot-cpcap", .driver_data = (kernel_ulong_t)&cpcap_mot_data },
> { /* sentinel */ }
> };
> MODULE_DEVICE_TABLE(spi, cpcap_spi_ids);
> --
> 2.51.0
>
--
Lee Jones
^ permalink raw reply
* Re: [PATCH v4 5/6 RESEND] mfd: motorola-cpcap: diverge configuration per-board
From: Lee Jones @ 2026-05-07 14:05 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Pavel Machek, David Lechner, Tony Lindgren, linux-input,
devicetree, linux-kernel, linux-leds
In-Reply-To: <20260428153611.142816-6-clamor95@gmail.com>
On Tue, 28 Apr 2026, Svyatoslav Ryhel wrote:
> MFD have rigid subdevice structure which does not allow flexible dynamic
> subdevice linking. Address this by diverging CPCAP subdevice composition
> to take into account board specific configuration.
>
> Create a common default subdevice composition, rename existing subdevice
> composition into cpcap_mapphone_mfd_devices since it targets mainly
> Mapphone board.
>
> Removed st,6556002 as it is no longer applicable to all cases and
> duplicates motorola,cpcap, which is used as the default composition.
>
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> ---
Changelog?
> drivers/mfd/motorola-cpcap.c | 101 ++++++++++++++++++++++++++++-------
> 1 file changed, 83 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/mfd/motorola-cpcap.c b/drivers/mfd/motorola-cpcap.c
> index d8243b956f87..516d1e33affa 100644
> --- a/drivers/mfd/motorola-cpcap.c
> +++ b/drivers/mfd/motorola-cpcap.c
> @@ -12,6 +12,7 @@
> #include <linux/kernel.h>
> #include <linux/module.h>
> #include <linux/mod_devicetable.h>
> +#include <linux/property.h>
> #include <linux/regmap.h>
> #include <linux/sysfs.h>
>
> @@ -24,10 +25,16 @@
> #define CPCAP_REGISTER_SIZE 4
> #define CPCAP_REGISTER_BITS 16
>
> +struct cpcap_chip_data {
> + const struct mfd_cell *mfd_devices;
> + unsigned int num_devices;
> +};
This is a red flag.
> struct cpcap_ddata {
> struct spi_device *spi;
> struct regmap_irq *irqs;
> struct regmap_irq_chip_data *irqdata[CPCAP_NR_IRQ_CHIPS];
> + const struct cpcap_chip_data *cdata;
> const struct regmap_config *regmap_conf;
> struct regmap *regmap;
> };
> @@ -195,20 +202,6 @@ static int cpcap_init_irq(struct cpcap_ddata *cpcap)
> return 0;
> }
>
> -static const struct of_device_id cpcap_of_match[] = {
> - { .compatible = "motorola,cpcap", },
> - { .compatible = "st,6556002", },
> - {},
> -};
> -MODULE_DEVICE_TABLE(of, cpcap_of_match);
> -
> -static const struct spi_device_id cpcap_spi_ids[] = {
> - { .name = "cpcap", },
> - { .name = "6556002", },
> - {},
> -};
> -MODULE_DEVICE_TABLE(spi, cpcap_spi_ids);
> -
> static const struct regmap_config cpcap_regmap_config = {
> .reg_bits = 16,
> .reg_stride = 4,
> @@ -241,7 +234,56 @@ static int cpcap_resume(struct device *dev)
>
> static DEFINE_SIMPLE_DEV_PM_OPS(cpcap_pm, cpcap_suspend, cpcap_resume);
>
> -static const struct mfd_cell cpcap_mfd_devices[] = {
> +static const struct mfd_cell cpcap_default_mfd_devices[] = {
> + {
> + .name = "cpcap_adc",
> + .of_compatible = "motorola,cpcap-adc",
> + }, {
> + .name = "cpcap_battery",
> + .of_compatible = "motorola,cpcap-battery",
> + }, {
> + .name = "cpcap-regulator",
> + .of_compatible = "motorola,cpcap-regulator",
> + }, {
> + .name = "cpcap-rtc",
> + .of_compatible = "motorola,cpcap-rtc",
> + }, {
> + .name = "cpcap-pwrbutton",
> + .of_compatible = "motorola,cpcap-pwrbutton",
> + }, {
> + .name = "cpcap-usb-phy",
> + .of_compatible = "motorola,cpcap-usb-phy",
> + }, {
> + .name = "cpcap-led",
> + .id = 0,
> + .of_compatible = "motorola,cpcap-led-red",
> + }, {
> + .name = "cpcap-led",
> + .id = 1,
> + .of_compatible = "motorola,cpcap-led-green",
> + }, {
> + .name = "cpcap-led",
> + .id = 2,
> + .of_compatible = "motorola,cpcap-led-blue",
> + }, {
> + .name = "cpcap-led",
> + .id = 3,
> + .of_compatible = "motorola,cpcap-led-adl",
> + }, {
> + .name = "cpcap-led",
> + .id = 4,
> + .of_compatible = "motorola,cpcap-led-cp",
> + }, {
> + .name = "cpcap-codec",
> + },
> +};
> +
> +static const struct cpcap_chip_data cpcap_default_data = {
> + .mfd_devices = cpcap_default_mfd_devices,
> + .num_devices = ARRAY_SIZE(cpcap_default_mfd_devices),
> +};
> +
> +static const struct mfd_cell cpcap_mapphone_mfd_devices[] = {
> {
> .name = "cpcap_adc",
> .of_compatible = "motorola,mapphone-cpcap-adc",
> @@ -285,7 +327,12 @@ static const struct mfd_cell cpcap_mfd_devices[] = {
> .of_compatible = "motorola,cpcap-led-cp",
> }, {
> .name = "cpcap-codec",
> - }
> + },
> +};
> +
> +static const struct cpcap_chip_data cpcap_mapphone_data = {
> + .mfd_devices = cpcap_mapphone_mfd_devices,
> + .num_devices = ARRAY_SIZE(cpcap_mapphone_mfd_devices),
> };
>
> static int cpcap_probe(struct spi_device *spi)
> @@ -297,9 +344,17 @@ static int cpcap_probe(struct spi_device *spi)
> if (!cpcap)
> return -ENOMEM;
>
> + cpcap->cdata = device_get_match_data(&spi->dev);
> + if (!cpcap->cdata)
> + return -ENODEV;
> +
> cpcap->spi = spi;
> spi_set_drvdata(spi, cpcap);
>
> @@ -331,16 +382,24 @@ static int cpcap_probe(struct spi_device *spi)
> spi->dev.coherent_dma_mask = 0;
> spi->dev.dma_mask = &spi->dev.coherent_dma_mask;
>
> - return devm_mfd_add_devices(&spi->dev, 0, cpcap_mfd_devices,
> - ARRAY_SIZE(cpcap_mfd_devices), NULL, 0, NULL);
> + return devm_mfd_add_devices(&spi->dev, 0, cpcap->cdata->mfd_devices,
> + cpcap->cdata->num_devices, NULL, 0, NULL);
> }
>
> +static const struct of_device_id cpcap_of_match[] = {
> + { .compatible = "motorola,cpcap", .data = &cpcap_default_data },
> + { .compatible = "motorola,mapphone-cpcap", .data = &cpcap_mapphone_data },
We don't allow data from one device registration API (MFD) to be passed
through another (OF) because it tends to lead to all sorts of "creative
solutions". Pass a value instead and match on that in a switch()
statement like all of the other MFD drivers do.
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, cpcap_of_match);
> +
> +static const struct spi_device_id cpcap_spi_ids[] = {
> + { .name = "cpcap", .driver_data = (kernel_ulong_t)&cpcap_default_data },
> + { .name = "mapphone-cpcap", .driver_data = (kernel_ulong_t)&cpcap_mapphone_data },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(spi, cpcap_spi_ids);
> +
> static struct spi_driver cpcap_driver = {
> .driver = {
> .name = "cpcap-core",
> --
> 2.51.0
>
>
--
Lee Jones
^ permalink raw reply
* [PATCH v4 2/2] Input: isa1200 - new driver for Imagis ISA1200
From: Svyatoslav Ryhel @ 2026-05-07 13:39 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Linus Walleij, Svyatoslav Ryhel
Cc: linux-input, devicetree, linux-kernel
In-Reply-To: <20260507133948.75704-1-clamor95@gmail.com>
From: Linus Walleij <linusw@kernel.org>
The ISA1200 is a haptic feedback unit from Imagis Technology using two
motors for haptic feedback in mobile phones. Used in many mobile devices
c. 2012 including Samsung Galxy S Advance GT-I9070 (Janice), Samsung Beam
GT-I8350 (Gavini), LG Optimus 4X P880 and LG Optimus Vu P895.
The exact datasheet for the ISA1200 is not available; all data was modeled
based on available downstream kernel sources for various devices and
fragments of information scattered across the internet.
Tested-by: Linus Walleij <linusw@kernel.org> # GT-I9070 Janice
Signed-off-by: Linus Walleij <linusw@kernel.org>
Co-developed-by: Svyatoslav Ryhel <clamor95@gmail.com>
Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
drivers/input/misc/Kconfig | 12 +
drivers/input/misc/Makefile | 1 +
drivers/input/misc/isa1200.c | 540 +++++++++++++++++++++++++++++++++++
3 files changed, 553 insertions(+)
create mode 100644 drivers/input/misc/isa1200.c
diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 94a753fcb64f..52f192104ee2 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -852,6 +852,18 @@ config INPUT_IQS7222
To compile this driver as a module, choose M here: the
module will be called iqs7222.
+config INPUT_ISA1200_HAPTIC
+ tristate "Imagis ISA1200 haptic feedback unit"
+ depends on I2C
+ select INPUT_FF_MEMLESS
+ select REGMAP_I2C
+ help
+ Say Y to enable support for the Imagis ISA1200 haptic
+ feedback unit.
+
+ To compile this driver as a module, choose M here: the
+ module will be called isa1200.
+
config INPUT_CMA3000
tristate "VTI CMA3000 Tri-axis accelerometer"
help
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index 415fc4e2918b..d62bf2e9d85f 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -49,6 +49,7 @@ obj-$(CONFIG_INPUT_IMS_PCU) += ims-pcu.o
obj-$(CONFIG_INPUT_IQS269A) += iqs269a.o
obj-$(CONFIG_INPUT_IQS626A) += iqs626a.o
obj-$(CONFIG_INPUT_IQS7222) += iqs7222.o
+obj-$(CONFIG_INPUT_ISA1200_HAPTIC) += isa1200.o
obj-$(CONFIG_INPUT_KEYSPAN_REMOTE) += keyspan_remote.o
obj-$(CONFIG_INPUT_KXTJ9) += kxtj9.o
obj-$(CONFIG_INPUT_M68K_BEEP) += m68kspkr.o
diff --git a/drivers/input/misc/isa1200.c b/drivers/input/misc/isa1200.c
new file mode 100644
index 000000000000..f8dba8a95c7d
--- /dev/null
+++ b/drivers/input/misc/isa1200.c
@@ -0,0 +1,540 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+#include <linux/array_size.h>
+#include <linux/bitmap.h>
+#include <linux/bits.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/devm-helpers.h>
+#include <linux/err.h>
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/input.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/property.h>
+#include <linux/pwm.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/units.h>
+
+/*
+ * System control (LDO regulator)
+ *
+ * LDO voltage to register mapping is linear, but it is split in two parts:
+ * 2.3V - 3.0V map to 0x08 - 0x0f; 3.1V - 3.8V map to 0x00 - 0x7
+ */
+
+#define ISA1200_SCTRL 0x00
+#define ISA1200_LDO_VOLTAGE_BASE 0x08
+#define ISA1200_LDO_VOLTAGE_STEP 100000
+#define ISA1200_LDO_VOLTAGE_2V3 23
+#define ISA1200_LDO_VOLTAGE_3V1 31
+#define ISA1200_LDO_VOLTAGE_MIN 2300000
+#define ISA1200_LDO_VOLTAGE_MAX 3800000
+
+/*
+ * The output frequency is calculated with this formula:
+ *
+ * base clock frequency
+ * fout = -----------------------------------------
+ * (128 - PWM_FREQ) * 2 * PLLDIV * PWM_PERIOD
+ *
+ * The base clock frequency is the clock frequency provided on the
+ * clock input to the chip, divided by the value in HCTRL0
+ *
+ * PWM_FREQ is configured in register HCTRL4, it is common to set this
+ * to 0 to get only two variables to calculate.
+ *
+ * PLLDIV is configured in register HCTRL3 (bits 7..4, so 0..15)
+ * PWM_PERIOD is configured in register HCTRL6
+ * Further the duty cycle can be configured in HCTRL5
+ */
+
+/*
+ * HCTRL0 configures clock or PWM input and selects the divider for
+ * the clock input.
+ */
+#define ISA1200_HCTRL0 0x30
+#define ISA1200_HCTRL0_HAP_ENABLE BIT(7)
+#define ISA1200_HCTRL0_PWM_GEN_MODE BIT(4)
+#define ISA1200_HCTRL0_PWM_INPUT_MODE BIT(3)
+#define ISA1200_HCTRL0_CLKDIV_128 128
+
+/*
+ * HCTRL1 configures the motor type and clock sourse
+ */
+#define ISA1200_HCTRL1 0x31
+#define ISA1200_HCTRL1_EXT_CLOCK BIT(7)
+#define ISA1200_HCTRL1_DAC_INVERT BIT(6)
+#define ISA1200_HCTRL1_MODE(n) (((n) & 1) << 5)
+
+/* HCTRL2 controls software reset of the chip */
+#define ISA1200_HCTRL2 0x32
+#define ISA1200_HCTRL2_SW_RESET BIT(0)
+
+/*
+ * HCTRL3 controls the PLL divisor
+ *
+ * Bits [0,1] are always set to 1 (we don't know what they are
+ * used for) and bit 4 and upward control the PLL divisor.
+ */
+#define ISA1200_HCTRL3 0x33
+#define ISA1200_HCTRL3_DEFAULT 0x03
+#define ISA1200_HCTRL3_PLLDIV(n) (((n) & 0xf) << 4)
+
+/* HCTRL4 controls the PWM frequency of external channel */
+#define ISA1200_HCTRL4 0x34
+
+/* HCTRL5 controls the PWM high duty cycle of internal channel */
+#define ISA1200_HCTRL5 0x35
+
+/* HCTRL6 controls the PWM period of internal channel */
+#define ISA1200_HCTRL6 0x36
+#define ISA1200_HCTRL6_PERIOD_SCALE 100
+
+/* The use for these registers is unknown but they exist */
+#define ISA1200_HCTRL7 0x37
+#define ISA1200_HCTRL8 0x38
+#define ISA1200_HCTRL9 0x39
+#define ISA1200_HCTRLA 0x3a
+#define ISA1200_HCTRLB 0x3b
+#define ISA1200_HCTRLC 0x3c
+#define ISA1200_HCTRLD 0x3d
+
+#define ISA1200_EN_PINS_MAX 2
+
+struct isa1200_config {
+ u32 ldo_voltage;
+ u32 mode;
+ u32 clkdiv;
+ u32 plldiv;
+ u32 freq;
+ u32 period;
+ u32 duty;
+};
+
+struct isa1200 {
+ struct input_dev *input;
+ struct regmap *map;
+
+ struct clk *clk;
+ struct pwm_device *pwm;
+ struct gpio_descs *enable_gpios;
+
+ struct work_struct play_work;
+ struct isa1200_config config;
+
+ int level;
+ bool clk_on;
+};
+
+static const struct regmap_config isa1200_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .max_register = ISA1200_HCTRLD,
+};
+
+static void isa1200_start(struct isa1200 *isa)
+{
+ struct isa1200_config *config = &isa->config;
+ struct pwm_state state;
+ u8 hctrl0 = 0, hctrl1 = 0;
+ DECLARE_BITMAP(values, ISA1200_EN_PINS_MAX);
+ int ret;
+
+ if (!isa->clk_on) {
+ ret = clk_prepare_enable(isa->clk);
+ if (ret < 0)
+ return;
+
+ isa->clk_on = true;
+ }
+
+ bitmap_fill(values, ISA1200_EN_PINS_MAX);
+ gpiod_multi_set_value_cansleep(isa->enable_gpios, values);
+
+ usleep_range(200, 300);
+
+ regmap_write(isa->map, ISA1200_SCTRL, config->ldo_voltage);
+
+ if (isa->clk) {
+ hctrl0 = ISA1200_HCTRL0_PWM_GEN_MODE;
+ hctrl1 = ISA1200_HCTRL1_EXT_CLOCK;
+ }
+
+ if (isa->pwm) {
+ hctrl0 = ISA1200_HCTRL0_PWM_INPUT_MODE;
+ hctrl1 = 0;
+ }
+
+ hctrl0 |= __ffs(config->clkdiv / ISA1200_HCTRL0_CLKDIV_128);
+ hctrl1 |= ISA1200_HCTRL1_DAC_INVERT;
+ hctrl1 |= ISA1200_HCTRL1_MODE(config->mode);
+
+ regmap_write(isa->map, ISA1200_HCTRL0, hctrl0);
+ regmap_write(isa->map, ISA1200_HCTRL1, hctrl1);
+
+ /* Make sure to de-assert software reset */
+ regmap_write(isa->map, ISA1200_HCTRL2, 0x00);
+
+ /* PLL divisor */
+ regmap_write(isa->map, ISA1200_HCTRL3,
+ ISA1200_HCTRL3_PLLDIV(config->plldiv) |
+ ISA1200_HCTRL3_DEFAULT);
+
+ /* Frequency */
+ regmap_write(isa->map, ISA1200_HCTRL4, config->freq);
+ /* Duty cycle */
+ regmap_write(isa->map, ISA1200_HCTRL5, config->period >> 1);
+ /* Period */
+ regmap_write(isa->map, ISA1200_HCTRL6, config->period);
+
+ hctrl0 |= ISA1200_HCTRL0_HAP_ENABLE;
+ regmap_write(isa->map, ISA1200_HCTRL0, hctrl0);
+
+ if (isa->clk)
+ regmap_write(isa->map, ISA1200_HCTRL5, config->duty);
+
+ if (isa->pwm) {
+ pwm_get_state(isa->pwm, &state);
+ state.duty_cycle = config->duty;
+ state.enabled = true;
+ pwm_apply_might_sleep(isa->pwm, &state);
+ }
+}
+
+static void isa1200_power_off(void *data)
+{
+ struct isa1200 *isa = data;
+
+ DECLARE_BITMAP(values, ISA1200_EN_PINS_MAX);
+
+ bitmap_zero(values, ISA1200_EN_PINS_MAX);
+ gpiod_multi_set_value_cansleep(isa->enable_gpios, values);
+
+ if (isa->clk_on) {
+ clk_disable_unprepare(isa->clk);
+ isa->clk_on = false;
+ }
+}
+
+static void isa1200_stop(struct isa1200 *isa)
+{
+ struct pwm_state state;
+
+ if (isa->pwm) {
+ pwm_get_state(isa->pwm, &state);
+ state.duty_cycle = 0;
+ state.enabled = false;
+ pwm_apply_might_sleep(isa->pwm, &state);
+ }
+
+ regmap_write(isa->map, ISA1200_HCTRL0, 0x00);
+ isa1200_power_off(isa);
+}
+
+static void isa1200_play_work(struct work_struct *work)
+{
+ struct isa1200 *isa =
+ container_of(work, struct isa1200, play_work);
+
+ guard(mutex)(&isa->input->mutex);
+
+ if (isa->level)
+ isa1200_start(isa);
+ else
+ isa1200_stop(isa);
+}
+
+static int isa1200_vibrator_play_effect(struct input_dev *input, void *data,
+ struct ff_effect *effect)
+{
+ struct isa1200 *isa = input_get_drvdata(input);
+ int level;
+
+ /*
+ * TODO: we currently only support rumble.
+ * The ISA1200 can control two motors and some devices
+ * also have two motors mounted.
+ */
+ level = effect->u.rumble.strong_magnitude;
+ if (!level)
+ level = effect->u.rumble.weak_magnitude;
+
+ dev_dbg(&input->dev, "FF effect type %d level %d\n",
+ effect->type, level);
+
+ if (isa->level != level) {
+ isa->level = level;
+ schedule_work(&isa->play_work);
+ }
+
+ return 0;
+}
+
+static void isa1200_vibrator_close(struct input_dev *input)
+{
+ struct isa1200 *isa = input_get_drvdata(input);
+
+ cancel_work_sync(&isa->play_work);
+
+ if (isa->level)
+ isa1200_stop(isa);
+
+ isa->level = 0;
+}
+
+static int isa1200_of_probe(struct i2c_client *client)
+{
+ static const char * const isa1200_supplies[] = { "vdd", "vddp" };
+ struct isa1200 *isa = i2c_get_clientdata(client);
+ struct isa1200_config *config = &isa->config;
+ struct device *dev = &client->dev;
+ struct fwnode_handle *ldo_node;
+ int ret;
+
+ isa->clk = devm_clk_get_optional(dev, NULL);
+ if (IS_ERR(isa->clk))
+ return dev_err_probe(dev, PTR_ERR(isa->clk),
+ "failed to get clock\n");
+
+ isa->pwm = devm_pwm_get(dev, NULL);
+ if (IS_ERR(isa->pwm)) {
+ ret = PTR_ERR(isa->pwm);
+ if (ret == -ENODEV || ret == -EINVAL)
+ isa->pwm = NULL;
+ else
+ return dev_err_probe(dev, ret, "getting PWM\n");
+ }
+
+ if (!isa->clk && !isa->pwm)
+ return dev_err_probe(dev, -EINVAL,
+ "clock or PWM are required, none were provided\n");
+
+ ret = devm_regulator_bulk_get_enable(dev, ARRAY_SIZE(isa1200_supplies),
+ isa1200_supplies);
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to set up supplies\n");
+
+ isa->enable_gpios = devm_gpiod_get_array_optional(dev, "control",
+ GPIOD_OUT_LOW);
+ if (IS_ERR(isa->enable_gpios))
+ return dev_err_probe(dev, PTR_ERR(isa->enable_gpios),
+ "failed to get enable gpios\n");
+
+ ldo_node = device_get_named_child_node(dev, "ldo");
+ if (!ldo_node)
+ return dev_err_probe(dev, -ENODEV,
+ "failed to get embedded LDO node\n");
+
+ ret = fwnode_property_read_u32(ldo_node, "regulator-min-microvolt",
+ &config->ldo_voltage);
+ fwnode_handle_put(ldo_node);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "failed to get ldo voltage\n");
+
+ config->ldo_voltage = clamp(config->ldo_voltage,
+ ISA1200_LDO_VOLTAGE_MIN,
+ ISA1200_LDO_VOLTAGE_MAX);
+
+ config->ldo_voltage /= ISA1200_LDO_VOLTAGE_STEP;
+ if (config->ldo_voltage < ISA1200_LDO_VOLTAGE_3V1)
+ config->ldo_voltage = config->ldo_voltage -
+ ISA1200_LDO_VOLTAGE_2V3 +
+ ISA1200_LDO_VOLTAGE_BASE;
+ else
+ config->ldo_voltage -= ISA1200_LDO_VOLTAGE_3V1;
+
+ config->mode = 0; /* LRA_MODE */
+ device_property_read_u32(dev, "imagis,mode", &config->mode);
+
+ config->clkdiv = ISA1200_HCTRL0_CLKDIV_128;
+ device_property_read_u32(dev, "imagis,clk-div", &config->clkdiv);
+ if (!config->clkdiv)
+ return dev_err_probe(dev, -EINVAL, "clk-div cannot be zero\n");
+
+ config->clkdiv = clamp(config->clkdiv, ISA1200_HCTRL0_CLKDIV_128,
+ ISA1200_HCTRL0_CLKDIV_128 << 3);
+
+ ret = device_property_read_u32(dev, "imagis,pll-div", &config->plldiv);
+ if (ret < 0 || !config->plldiv)
+ config->plldiv = 1;
+
+ config->period = 0;
+ config->freq = 0;
+ config->duty = 0;
+
+ if (isa->clk) {
+ ret = device_property_read_u32(dev, "imagis,period-ns",
+ &config->period);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "failed to get period\n");
+
+ /*
+ * TODO: The scale value is arbitrary, but it fits observations
+ * quite well, and the exact conversion method is unknown.
+ * The period property value returned above is the HCTRL6
+ * register value set by the vendor code, multiplied by 100.
+ */
+ config->period /= ISA1200_HCTRL6_PERIOD_SCALE;
+ config->duty = config->period >> 1;
+ }
+
+ if (isa->pwm) {
+ struct pwm_state state;
+
+ pwm_init_state(isa->pwm, &state);
+
+ if (!state.period)
+ return dev_err_probe(dev, -EINVAL,
+ "PWM period cannot be zero\n");
+
+ config->freq = div64_u64(NANO, state.period * config->clkdiv);
+ config->duty = state.period >> 1;
+
+ ret = pwm_apply_might_sleep(isa->pwm, &state);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "failed to apply initial PWM state\n");
+ }
+
+ /*
+ * TODO: If device is using a clock, this property should return the
+ * value written to the HCTRL5 register by downstrem code. It likely
+ * needs to be converted into a meaningful duty cycle value, though
+ * unfortunately the exact conversion mechanism is unknown. If the
+ * device uses PWM, this property will return the correct duty cycle
+ * in nanoseconds.
+ */
+ device_property_read_u32(dev, "imagis,duty-cycle-ns", &config->duty);
+
+ return 0;
+}
+
+static int isa1200_probe(struct i2c_client *client)
+{
+ struct isa1200 *isa;
+ struct device *dev = &client->dev;
+ DECLARE_BITMAP(values, ISA1200_EN_PINS_MAX);
+ u32 val;
+ int ret;
+
+ isa = devm_kzalloc(dev, sizeof(*isa), GFP_KERNEL);
+ if (!isa)
+ return -ENOMEM;
+
+ isa->input = devm_input_allocate_device(dev);
+ if (!isa->input)
+ return -ENOMEM;
+
+ i2c_set_clientdata(client, isa);
+
+ ret = isa1200_of_probe(client);
+ if (ret)
+ return ret;
+
+ isa->map = devm_regmap_init_i2c(client, &isa1200_regmap_config);
+ if (IS_ERR(isa->map))
+ return dev_err_probe(dev, PTR_ERR(isa->map),
+ "failed to initialize register map\n");
+
+ ret = clk_prepare_enable(isa->clk);
+ if (ret < 0)
+ return dev_err_probe(dev, ret, "failed to enable clock\n");
+
+ isa->clk_on = true;
+
+ bitmap_fill(values, ISA1200_EN_PINS_MAX);
+ gpiod_multi_set_value_cansleep(isa->enable_gpios, values);
+
+ ret = devm_add_action_or_reset(dev, isa1200_power_off, isa);
+ if (ret)
+ return ret;
+
+ usleep_range(200, 300);
+
+ /* Read a register so we know that regmap and I2C transport works */
+ ret = regmap_read(isa->map, ISA1200_SCTRL, &val);
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to read SCTRL\n");
+
+ ret = devm_work_autocancel(dev, &isa->play_work, isa1200_play_work);
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to init work\n");
+
+ isa->input->name = "isa1200-haptic";
+ isa->input->id.bustype = BUS_HOST;
+ isa->input->dev.parent = dev;
+ isa->input->close = isa1200_vibrator_close;
+
+ input_set_drvdata(isa->input, isa);
+
+ /* TODO: this hardware can likely support more than rumble */
+ input_set_capability(isa->input, EV_FF, FF_RUMBLE);
+
+ ret = input_ff_create_memless(isa->input, NULL,
+ isa1200_vibrator_play_effect);
+ if (ret)
+ return dev_err_probe(dev, ret, "couldn't create FF dev\n");
+
+ ret = input_register_device(isa->input);
+ if (ret)
+ return dev_err_probe(dev, ret, "couldn't register input dev\n");
+
+ return ret;
+}
+
+static int isa1200_suspend(struct device *dev)
+{
+ struct isa1200 *isa = dev_get_drvdata(dev);
+
+ cancel_work_sync(&isa->play_work);
+
+ guard(mutex)(&isa->input->mutex);
+
+ if (input_device_enabled(isa->input))
+ if (isa->level)
+ isa1200_stop(isa);
+
+ return 0;
+}
+
+static int isa1200_resume(struct device *dev)
+{
+ struct isa1200 *isa = dev_get_drvdata(dev);
+
+ guard(mutex)(&isa->input->mutex);
+
+ if (input_device_enabled(isa->input))
+ if (isa->level)
+ isa1200_start(isa);
+
+ return 0;
+}
+
+static DEFINE_SIMPLE_DEV_PM_OPS(isa1200_pm_ops, isa1200_suspend, isa1200_resume);
+
+static const struct of_device_id isa1200_of_match[] = {
+ { .compatible = "imagis,isa1200" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, isa1200_of_match);
+
+static struct i2c_driver isa1200_i2c_driver = {
+ .driver = {
+ .name = "isa1200",
+ .of_match_table = isa1200_of_match,
+ .pm = pm_sleep_ptr(&isa1200_pm_ops),
+ },
+ .probe = isa1200_probe,
+};
+module_i2c_driver(isa1200_i2c_driver);
+
+MODULE_AUTHOR("Linus Walleij <linusw@kernel.org>");
+MODULE_AUTHOR("Svyatoslav Ryhel <clamor95@gmail.com>");
+MODULE_DESCRIPTION("Imagis ISA1200 haptic feedback unit");
+MODULE_LICENSE("GPL");
--
2.51.0
^ permalink raw reply related
* [PATCH v4 1/2] dt-bindings: input: Document Imagis ISA1200 haptic motor driver
From: Svyatoslav Ryhel @ 2026-05-07 13:39 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Linus Walleij, Svyatoslav Ryhel
Cc: linux-input, devicetree, linux-kernel
In-Reply-To: <20260507133948.75704-1-clamor95@gmail.com>
Document the Imagis ISA1200 haptic motor driver, used primarily in mobile
handheld devices and capable of supporting up to two motors.
The exact datasheet for the ISA1200 is not available; all data was modeled
based on available downstream kernel sources for various devices and
fragments of information scattered across the internet.
Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
---
.../bindings/input/imagis,isa1200.yaml | 140 ++++++++++++++++++
1 file changed, 140 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/imagis,isa1200.yaml
diff --git a/Documentation/devicetree/bindings/input/imagis,isa1200.yaml b/Documentation/devicetree/bindings/input/imagis,isa1200.yaml
new file mode 100644
index 000000000000..bbe6f99d39c1
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/imagis,isa1200.yaml
@@ -0,0 +1,140 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/imagis,isa1200.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Imagis ISA1200 haptic motor driver
+
+maintainers:
+ - Svyatoslav Ryhel <clamor95@gmail.com>
+ - Linus Walleij <linusw@kernel.org>
+
+description:
+ The ISA1200 is a high-performance enhanced haptic motor driver designed
+ for mobile hand-held devices. It supports various voltages for both ERM
+ (Eccentric Rotating Mass) and LRA (Linear Resonant Actuator) type
+ actuators. Thanks to an embedded LDO, battery power can be used directly
+ in handheld applications.
+
+properties:
+ compatible:
+ const: imagis,isa1200
+
+ reg:
+ maxItems: 1
+
+ control-gpios:
+ description:
+ One or two GPIOs flagged as active high linked to HEN and LEN pins
+ maxItems: 2
+
+ clocks:
+ maxItems: 1
+
+ pwms:
+ maxItems: 1
+
+ vdd-supply:
+ description:
+ Regulator for 2.4V - 5.5V power supply
+
+ vddp-supply:
+ description:
+ Regulator for 2.4V - 3.6V IO power supply
+
+ imagis,clk-div:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ Divider for the external input clock/PWM
+ enum: [128, 256, 512, 1024]
+ default: 128
+
+ imagis,pll-div:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ Divider for the internal PLL clock
+ minimum: 1
+ maximum: 15
+ default: 1
+
+ imagis,mode:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: |
+ Defines the motor type isa1200 drives
+ 0 - LRA (Linear Resonant Actuator)
+ 1 - ERM (Eccentric Rotating Mass)
+ enum: [0, 1]
+ default: 0
+
+ imagis,period-ns:
+ description:
+ Period of the internal PWM channel in nanoseconds.
+ minimum: 10000
+ maximum: 30000
+
+ imagis,duty-cycle-ns:
+ description:
+ Duty cycle of the external/internal PWM channel in nanoseconds,
+ defaults to 50% of the channel's period
+
+ ldo:
+ $ref: /schemas/regulator/regulator.yaml#
+ type: object
+ description:
+ Embedded LDO regulator with voltage range 2.3V - 3.8V
+ unevaluatedProperties: false
+
+ required:
+ - regulator-min-microvolt
+ - regulator-max-microvolt
+
+required:
+ - compatible
+ - reg
+ - ldo
+
+anyOf:
+ - required:
+ - clocks
+ - imagis,period-ns
+ - required:
+ - pwms
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ haptic-engine@49 {
+ compatible = "imagis,isa1200";
+ reg = <0x49>;
+
+ clocks = <&isa1200_refclk>;
+
+ control-gpios = <&gpio 22 GPIO_ACTIVE_HIGH>,
+ <&gpio 23 GPIO_ACTIVE_HIGH>;
+
+ vdd-supply = <&vdd_3v3_vbat>;
+ vddp-supply = <&vdd_2v8_vvib>;
+
+ imagis,clk-div = <256>;
+ imagis,pll-div = <2>;
+
+ imagis,mode = <0>; /* LRA_MODE */
+
+ imagis,period-ns = <13400>;
+ imagis,duty-cycle-ns = <100>;
+
+ ldo {
+ regulator-name = "vdd_vib";
+ regulator-min-microvolt = <2300000>;
+ regulator-max-microvolt = <2300000>;
+ };
+ };
+ };
--
2.51.0
^ permalink raw reply related
* [PATCH v4 0/2] input: misc: add support for Imagis ISA1200 haptic motor driver
From: Svyatoslav Ryhel @ 2026-05-07 13:39 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Linus Walleij, Svyatoslav Ryhel
Cc: linux-input, devicetree, linux-kernel
The ISA1200 is a haptic feedback unit from Imagis Technology using two
motors for haptic feedback in mobile phones. Used in many mobile devices
c. 2012 including Samsung Galxy S Advance GT-I9070 (Janice), Samsung Beam
GT-I8350 (Gavini), LG Optimus 4X P880 and LG Optimus Vu P895.
The exact datasheet for the ISA1200 is not available; all data was modeled
based on available downstream kernel sources for various devices and
fragments of information scattered across the internet.
---
Changes in v4:
- added INPUT_FF_MEMLESS option selection
- fixed missing clock status set
- guard start/stop calls in isa1200_play_work with lock
- clamp ldo voltages to allowed range
- fixed imagis,pll-div parsing
- dropped Tested-by from schema adding commit
Changes in v3:
- added clock state tracking
- dropped level check in vibrator close
- added clkdiv clamping
- added comments regarding registers 5 and 6
Changes in v2:
- imagis,clk-div switched to accept actual divider value
- dropped DT header
- adjusted imagis,period-ns range
- initiated hctrl0 and hctrl1 values in isa1200_start
- fixed situation when PWM might return -EPROBE_DEFER to be
treated properly
- added chech a clock or PWM is available
- fixed regulator voltages check being off by 10
- added chech if state.period is not zero
- added action call to disable clock and gpios on error
- used managed version of work init
- added work cancel on suspend
- PW calls are done under mutex lock
---
Linus Walleij (1):
Input: isa1200 - new driver for Imagis ISA1200
Svyatoslav Ryhel (1):
dt-bindings: input: Document Imagis ISA1200 haptic motor driver
.../bindings/input/imagis,isa1200.yaml | 140 +++++
drivers/input/misc/Kconfig | 12 +
drivers/input/misc/Makefile | 1 +
drivers/input/misc/isa1200.c | 540 ++++++++++++++++++
4 files changed, 693 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/imagis,isa1200.yaml
create mode 100644 drivers/input/misc/isa1200.c
--
2.51.0
^ permalink raw reply
* [PATCH v3 3/3] platform/x86: asus-armoury: add fn_lock firmware attribute
From: Marcus Grenängen @ 2026-05-07 9:29 UTC (permalink / raw)
To: platform-driver-x86, denis.benato
Cc: linux-input, linux-kernel, luke, hansg, ilpo.jarvinen, jikos,
bentiss, corentin.chary, marcus
In-Reply-To: <20260507092911.8855-1-marcus@grenangen.se>
Add a fn_lock attribute to the asus-armoury firmware-attributes interface,
allowing userspace to read and set the Fn-lock state (whether F1-F12 keys
are primary or media/system keys are primary).
On most ASUS laptops fn-lock is backed by WMI DEVID 0x00100023. On
platforms where that DEVS call is a no-op (fnlock_use_hid quirk), the
store path dispatches via asus_wmi_fnlock_set(), which selects the HID
feature-report path internally. The show path returns -EOPNOTSUPP on
such platforms as the hardware provides no readback.
The attribute is only registered when the platform actually supports
fn-lock, either via the HID quirk (asus_wmi_fnlock_use_hid()) or a
functional WMI DEVID (armoury_has_devstate(ASUS_WMI_DEVID_FNLOCK)).
Registration state is tracked in fn_lock_registered so that removal
in the exit path is symmetric.
Signed-off-by: Marcus Grenängen <marcus@grenangen.se>
---
drivers/platform/x86/asus-armoury.c | 70 +++++++++++++++++++++++++++++
1 file changed, 70 insertions(+)
diff --git a/drivers/platform/x86/asus-armoury.c b/drivers/platform/x86/asus-armoury.c
index 5b0987ccc270..fb8ad3b14aad 100644
--- a/drivers/platform/x86/asus-armoury.c
+++ b/drivers/platform/x86/asus-armoury.c
@@ -93,6 +93,7 @@ struct asus_armoury_priv {
u32 mini_led_dev_id;
u32 gpu_mux_dev_id;
+ bool fn_lock_registered;
};
static struct asus_armoury_priv asus_armoury = {
@@ -778,6 +779,58 @@ ASUS_ATTR_GROUP_ROG_TUNABLE(nv_tgp, "nv_tgp", ASUS_WMI_DEVID_DGPU_SET_TGP,
ASUS_ATTR_GROUP_INT_VALUE_ONLY_RO(nv_base_tgp, ATTR_NV_BASE_TGP, ASUS_WMI_DEVID_DGPU_BASE_TGP,
"Read the base TGP value");
+/*
+ * fn_lock: toggle whether Fn key is locked (F1-F12 primary) or unlocked
+ * (media/system keys primary).
+ *
+ * On most ASUS laptops this is backed by WMI DEVID 0x00100023. On some
+ * platforms (e.g. ProArt P16) that DEVS call is a no-op and the state must
+ * be sent as a HID feature report to the N-Key keyboard via hid-asus.
+ */
+static ssize_t fn_lock_current_value_show(struct kobject *kobj,
+ struct kobj_attribute *attr, char *buf)
+{
+ u32 result;
+ int err;
+
+ if (asus_wmi_fnlock_use_hid())
+ return -EOPNOTSUPP;
+
+ err = armoury_get_devstate(attr, &result, ASUS_WMI_DEVID_FNLOCK);
+ if (err)
+ return err;
+
+ return sysfs_emit(buf, "%u\n", result & 1);
+}
+
+static ssize_t fn_lock_current_value_store(struct kobject *kobj,
+ struct kobj_attribute *attr,
+ const char *buf, size_t count)
+{
+ bool enable;
+ int err;
+
+ err = kstrtobool(buf, &enable);
+ if (err)
+ return err;
+
+ if (asus_wmi_fnlock_use_hid()) {
+ err = asus_hid_fnlock_set(enable);
+ if (err)
+ return err;
+ } else {
+ err = armoury_set_devstate(attr, enable ? 1 : 0, NULL,
+ ASUS_WMI_DEVID_FNLOCK);
+ if (err)
+ return err;
+ }
+
+ sysfs_notify(kobj, NULL, attr->attr.name);
+ return count;
+}
+
+ASUS_ATTR_GROUP_BOOL(fn_lock, "fn_lock", "Set the Fn-lock state");
+
/* If an attribute does not require any special case handling add it here */
static const struct asus_attr_group armoury_attr_groups[] = {
{ &egpu_connected_attr_group, ASUS_WMI_DEVID_EGPU_CONNECTED },
@@ -926,6 +979,17 @@ static int asus_fw_attr_add(void)
}
}
+ if (asus_wmi_fnlock_use_hid() ||
+ armoury_has_devstate(ASUS_WMI_DEVID_FNLOCK)) {
+ err = sysfs_create_group(&asus_armoury.fw_attr_kset->kobj,
+ &fn_lock_attr_group);
+ if (err) {
+ pr_err("Failed to create sysfs-group for fn_lock\n");
+ goto err_remove_gpu_mux_group;
+ }
+ asus_armoury.fn_lock_registered = true;
+ }
+
for (i = 0; i < ARRAY_SIZE(armoury_attr_groups); i++) {
if (!armoury_has_devstate(armoury_attr_groups[i].wmi_devid))
continue;
@@ -963,6 +1027,9 @@ static int asus_fw_attr_add(void)
sysfs_remove_group(&asus_armoury.fw_attr_kset->kobj,
armoury_attr_groups[i].attr_group);
}
+ if (asus_armoury.fn_lock_registered)
+ sysfs_remove_group(&asus_armoury.fw_attr_kset->kobj, &fn_lock_attr_group);
+err_remove_gpu_mux_group:
if (asus_armoury.gpu_mux_dev_id)
sysfs_remove_group(&asus_armoury.fw_attr_kset->kobj, &gpu_mux_mode_attr_group);
err_remove_mini_led_group:
@@ -1138,6 +1205,9 @@ static void __exit asus_fw_exit(void)
if (asus_armoury.gpu_mux_dev_id)
sysfs_remove_group(&asus_armoury.fw_attr_kset->kobj, &gpu_mux_mode_attr_group);
+ if (asus_armoury.fn_lock_registered)
+ sysfs_remove_group(&asus_armoury.fw_attr_kset->kobj, &fn_lock_attr_group);
+
if (asus_armoury.mini_led_dev_id)
sysfs_remove_group(&asus_armoury.fw_attr_kset->kobj, &mini_led_mode_attr_group);
--
2.54.0
^ permalink raw reply related
* [PATCH v3 2/3] platform/x86: asus-nb-wmi: add fnlock_use_hid quirk and asus_wmi_fnlock_use_hid()
From: Marcus Grenängen @ 2026-05-07 9:29 UTC (permalink / raw)
To: platform-driver-x86, denis.benato
Cc: linux-input, linux-kernel, luke, hansg, ilpo.jarvinen, jikos,
bentiss, corentin.chary, marcus
In-Reply-To: <20260507092911.8855-1-marcus@grenangen.se>
The ASUS ProArt P16 (N-Key keyboard 0B05:19B6) advertises the WMI fn-lock
DEVID (0x00100023) as present via DSTS, but the DEVS call has no effect.
Fn-lock must instead be toggled via a HID feature report sent to the N-Key
keyboard (handled by hid-asus).
Add a fnlock_use_hid flag to struct quirk_entry and set it for the ProArt
P16 via a DMI match on DMI_PRODUCT_FAMILY.
Export asus_wmi_fnlock_use_hid() so that asus-armoury can query whether
the HID path is required without reading the quirk struct directly. This
keeps the DMI and quirk knowledge inside asus-wmi.
Signed-off-by: Marcus Grenängen <marcus@grenangen.se>
---
drivers/platform/x86/asus-nb-wmi.c | 13 +++++++++++++
drivers/platform/x86/asus-wmi.c | 22 ++++++++++++++++++++++
drivers/platform/x86/asus-wmi.h | 5 +++++
include/linux/platform_data/x86/asus-wmi.h | 6 +++---
4 files changed, 43 insertions(+), 3 deletions(-)
diff --git a/drivers/platform/x86/asus-nb-wmi.c b/drivers/platform/x86/asus-nb-wmi.c
index b4677c5bba5b..44e4cf68ff70 100644
--- a/drivers/platform/x86/asus-nb-wmi.c
+++ b/drivers/platform/x86/asus-nb-wmi.c
@@ -155,6 +155,10 @@ static struct quirk_entry quirk_asus_z13 = {
.tablet_switch_mode = asus_wmi_kbd_dock_devid,
};
+static struct quirk_entry quirk_asus_proart_p16 = {
+ .fnlock_use_hid = true,
+};
+
static int dmi_matched(const struct dmi_system_id *dmi)
{
pr_info("Identified laptop model '%s'\n", dmi->ident);
@@ -553,6 +557,15 @@ static const struct dmi_system_id asus_quirks[] = {
},
.driver_data = &quirk_asus_z13,
},
+ {
+ .callback = dmi_matched,
+ .ident = "ASUS ProArt P16",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
+ DMI_MATCH(DMI_PRODUCT_FAMILY, "ProArt P16"),
+ },
+ .driver_data = &quirk_asus_proart_p16,
+ },
{},
};
diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 80144c412b90..d4d742b9983d 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -1759,6 +1759,28 @@ int asus_hid_event(enum asus_hid_event event)
}
EXPORT_SYMBOL_GPL(asus_hid_event);
+/**
+ * asus_wmi_fnlock_use_hid() - Return true if fn-lock must use the HID path.
+ *
+ * On some platforms (e.g. ASUS ProArt P16) the WMI DEVS call for fn-lock is
+ * silently a no-op. The fnlock_use_hid quirk flag marks these platforms so
+ * that callers can select the HID feature-report path instead.
+ *
+ * Returns: true if the HID path should be used, false otherwise.
+ */
+bool asus_wmi_fnlock_use_hid(void)
+{
+ struct asus_wmi *asus;
+
+ guard(spinlock_irqsave)(&asus_ref.lock);
+ asus = asus_ref.asus;
+ if (!asus)
+ return false;
+
+ return asus->driver->quirks->fnlock_use_hid;
+}
+EXPORT_SYMBOL_NS_GPL(asus_wmi_fnlock_use_hid, "ASUS_WMI");
+
/*
* These functions actually update the LED's, and are called from a
* workqueue. By doing this as separate work rather than when the LED
diff --git a/drivers/platform/x86/asus-wmi.h b/drivers/platform/x86/asus-wmi.h
index 5cd4392b964e..6c50b11860e8 100644
--- a/drivers/platform/x86/asus-wmi.h
+++ b/drivers/platform/x86/asus-wmi.h
@@ -52,6 +52,11 @@ struct quirk_entry {
*/
int no_display_toggle;
u32 xusb2pr;
+ /*
+ * Some platforms report WMI DEVID_FNLOCK as present but the DEVS call
+ * is a no-op. Force the HID feature report path via hid-asus instead.
+ */
+ bool fnlock_use_hid;
};
struct asus_wmi_driver {
diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
index a88bf03f9c4d..199179266363 100644
--- a/include/linux/platform_data/x86/asus-wmi.h
+++ b/include/linux/platform_data/x86/asus-wmi.h
@@ -205,7 +205,7 @@ int asus_wmi_evaluate_method(u32 method_id, u32 arg0, u32 arg1, u32 *retval);
int asus_hid_register_listener(struct asus_hid_listener *cdev);
void asus_hid_unregister_listener(struct asus_hid_listener *cdev);
int asus_hid_event(enum asus_hid_event event);
-int asus_hid_fnlock_set(bool enabled);
+bool asus_wmi_fnlock_use_hid(void);
#else
static inline void set_ally_mcu_hack(enum asus_ally_mcu_hack status)
{
@@ -238,9 +238,9 @@ static inline int asus_hid_event(enum asus_hid_event event)
return -ENODEV;
}
-static inline int asus_hid_fnlock_set(bool enabled)
+static inline bool asus_wmi_fnlock_use_hid(void)
{
- return -ENODEV;
+ return false;
}
#endif
--
2.54.0
^ permalink raw reply related
* [PATCH v3 1/3] HID: asus: export asus_hid_fnlock_set() for direct fn-lock control
From: Marcus Grenängen @ 2026-05-07 9:29 UTC (permalink / raw)
To: platform-driver-x86, denis.benato
Cc: linux-input, linux-kernel, luke, hansg, ilpo.jarvinen, jikos,
bentiss, corentin.chary, marcus
In-Reply-To: <20260507092911.8855-1-marcus@grenangen.se>
Some ASUS platforms cannot control fn-lock via WMI DEVS and must send a
HID feature report directly to the N-Key keyboard device instead.
Add a module-level fnlock_hdev pointer (protected by a mutex) that is set
at probe time for devices with QUIRK_HID_FN_LOCK and cleared at remove.
Export asus_hid_fnlock_set(bool) so that asus-armoury can call into
hid-asus without a circular dependency.
Signed-off-by: Marcus Grenängen <marcus@grenangen.se>
---
drivers/hid/hid-asus.c | 44 +++++++++++++++++++++-
include/linux/platform_data/x86/asus-wmi.h | 15 ++++++++
2 files changed, 58 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index d34d74df3dc0..402ba9d5e982 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -584,6 +584,39 @@ static void asus_sync_fn_lock(struct work_struct *work)
asus_kbd_set_fn_lock(drvdata->hdev, drvdata->fn_lock);
}
+/*
+ * Module-level reference to the HID device that handles fn-lock via feature
+ * report. Set at probe and cleared at remove for QUIRK_HID_FN_LOCK devices.
+ * Protected by fnlock_hdev_lock.
+ */
+static DEFINE_MUTEX(fnlock_hdev_lock);
+static struct hid_device *fnlock_hdev;
+
+/**
+ * asus_hid_fnlock_set() - Set fn-lock state directly via HID feature report.
+ * @enabled: true to lock fn (F1-F12 primary), false to unlock.
+ *
+ * Called by asus-armoury on platforms where the WMI DEVS path for fn-lock is
+ * non-functional (e.g. ASUS ProArt P16, N-Key keyboard product ID 0x19B6).
+ *
+ * Returns: 0 on success, -ENODEV if no fn-lock capable HID device is present.
+ */
+int asus_hid_fnlock_set(bool enabled)
+{
+ int ret;
+
+ guard(mutex)(&fnlock_hdev_lock);
+ if (!fnlock_hdev)
+ return -ENODEV;
+
+ ret = asus_kbd_set_fn_lock(fnlock_hdev, enabled);
+ if (ret < 0)
+ return ret;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(asus_hid_fnlock_set);
+
static void asus_schedule_work(struct asus_kbd_leds *led)
{
unsigned long flags;
@@ -969,6 +1002,8 @@ static int asus_input_configured(struct hid_device *hdev, struct hid_input *hi)
drvdata->fn_lock = true;
INIT_WORK(&drvdata->fn_lock_sync_work, asus_sync_fn_lock);
asus_kbd_set_fn_lock(hdev, true);
+ guard(mutex)(&fnlock_hdev_lock);
+ fnlock_hdev = hdev;
}
if (drvdata->tp) {
@@ -1008,6 +1043,8 @@ static int asus_input_configured(struct hid_device *hdev, struct hid_input *hi)
drvdata->fn_lock = true;
INIT_WORK(&drvdata->fn_lock_sync_work, asus_sync_fn_lock);
asus_kbd_set_fn_lock(hdev, true);
+ guard(mutex)(&fnlock_hdev_lock);
+ fnlock_hdev = hdev;
}
return 0;
@@ -1362,8 +1399,13 @@ static void asus_remove(struct hid_device *hdev)
cancel_work_sync(&drvdata->kbd_backlight->work);
}
- if (drvdata->quirks & QUIRK_HID_FN_LOCK)
+ if (drvdata->quirks & QUIRK_HID_FN_LOCK) {
+ scoped_guard(mutex, &fnlock_hdev_lock) {
+ if (fnlock_hdev == hdev)
+ fnlock_hdev = NULL;
+ }
cancel_work_sync(&drvdata->fn_lock_sync_work);
+ }
hid_hw_stop(hdev);
}
diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
index 554f41b827e1..a88bf03f9c4d 100644
--- a/include/linux/platform_data/x86/asus-wmi.h
+++ b/include/linux/platform_data/x86/asus-wmi.h
@@ -187,6 +187,15 @@ enum asus_hid_event {
#define ASUS_EV_MAX_BRIGHTNESS 3
+#if IS_REACHABLE(CONFIG_HID_ASUS)
+int asus_hid_fnlock_set(bool enabled);
+#else
+static inline int asus_hid_fnlock_set(bool enabled)
+{
+ return -ENODEV;
+}
+#endif
+
#if IS_REACHABLE(CONFIG_ASUS_WMI)
void set_ally_mcu_hack(enum asus_ally_mcu_hack status);
void set_ally_mcu_powersave(bool enabled);
@@ -196,6 +205,7 @@ int asus_wmi_evaluate_method(u32 method_id, u32 arg0, u32 arg1, u32 *retval);
int asus_hid_register_listener(struct asus_hid_listener *cdev);
void asus_hid_unregister_listener(struct asus_hid_listener *cdev);
int asus_hid_event(enum asus_hid_event event);
+int asus_hid_fnlock_set(bool enabled);
#else
static inline void set_ally_mcu_hack(enum asus_ally_mcu_hack status)
{
@@ -227,6 +237,11 @@ static inline int asus_hid_event(enum asus_hid_event event)
{
return -ENODEV;
}
+
+static inline int asus_hid_fnlock_set(bool enabled)
+{
+ return -ENODEV;
+}
#endif
#endif /* __PLATFORM_DATA_X86_ASUS_WMI_H */
--
2.54.0
^ permalink raw reply related
* [PATCH v3 0/3] platform/x86: fix fn-lock on ASUS ProArt P16 (WMI DEVS no-op)
From: Marcus Grenängen @ 2026-05-07 9:29 UTC (permalink / raw)
To: platform-driver-x86, denis.benato
Cc: linux-input, linux-kernel, luke, hansg, ilpo.jarvinen, jikos,
bentiss, corentin.chary, marcus
In-Reply-To: <9b568ce0-93f7-4a7f-98e4-625e910f8a1d@linux.dev>
Changes since v2 (addressing Denis's and Randy's review):
Patch 1 (HID: asus):
- Renamed asus_hid_fnlock_notify() to asus_hid_fnlock_set() (Denis)
- Replaced "if (ret > 0) ret = 0" with "if (ret < 0) return ret; return 0;"
pattern (Denis)
- Fixed Returns tag format to "Returns:" (Randy)
- Added #if IS_REACHABLE(CONFIG_HID_ASUS) guard for asus_hid_fnlock_set()
declaration in asus-wmi.h so asus-wmi.c can call it without a missing
prototype warning
Patch 2 (asus-nb-wmi):
- Added asus_wmi_fnlock_use_hid() export so asus-armoury can query the
quirk flag without reading the quirk struct directly, keeping DMI and
quirk knowledge inside asus-wmi (Denis)
- Fixed Returns tag format to "Returns:" (Randy)
Patch 3 (asus-armoury):
- Replaced the stored fnlock_use_hid flag and dmi_match() call with
asus_wmi_fnlock_use_hid(), routing the DMI/quirk check through asus-wmi
as Denis suggested
- Added fn_lock_registered bool to properly guard sysfs_remove_group in
both the error unwind path and __exit, mirroring the gpu_mux/mini_led
pattern (Denis)
- NOTE/Question: Since we have proper fn+esc hardware key handling working
now we could eliminate this patch completely if we don't care about
being able to control the fn state from user space
eg. asusctl and/or rog control center?
The nice thing of having it controllable via asusctl is the
scripting possibilibilities like setting prefered mode when
starting a DE as one example.
Regarding the if/else dispatch in fn_lock_current_value_store: Denis
suggested routing everything through a single asus_wmi_fnlock_set()
exported from asus-wmi. This was implemented but had to be reverted: it
introduced a module dependency cycle (hid_asus -> asus_wmi -> hid_asus)
that depmod detects and rejects. asus-armoury therefore retains the
if/else, calling asus_hid_fnlock_set() on HID-path platforms and
armoury_set_devstate() on WMI-path platforms. The asus-armoury -> hid-asus
dependency is a soft one (the stub in asus-wmi.h returns -ENODEV when
CONFIG_HID_ASUS is not reachable). But since I'm new to this maybe I'm
missing something critical here?
Marcus Grenängen (3):
HID: asus: export asus_hid_fnlock_set() for direct fn-lock control
platform/x86: asus-nb-wmi: add fnlock_use_hid quirk and asus_wmi_fnlock_use_hid()
platform/x86: asus-armoury: add fn_lock firmware attribute
drivers/hid/hid-asus.c | 44 ++++++++++++++++++-
drivers/platform/x86/asus-armoury.c | 70 ++++++++++++++++++++++++++++++
drivers/platform/x86/asus-nb-wmi.c | 13 ++++++
drivers/platform/x86/asus-wmi.c | 22 ++++++++++
drivers/platform/x86/asus-wmi.h | 5 +++
include/linux/platform_data/x86/asus-wmi.h | 15 +++++++
6 files changed, 168 insertions(+), 1 deletion(-)
--
2.54.0
^ permalink raw reply
* [PATCH] HID: remove duplicate hid_warn_ratelimited definition
From: 刘 凯 @ 2026-05-07 8:41 UTC (permalink / raw)
To: jikos@kernel.org, bentiss@kernel.org
Cc: vi@endrift.com, linux-input@vger.kernel.org,
linux-kernel@vger.kernel.org
From 565c81e1b2e1aaad675d9844428af7b35f4328c6 Mon Sep 17 00:00:00 2001
From: Liu Kai <lukace97@outlook.com>
Date: Thu, 7 May 2026 16:32:04 +0800
Subject: [PATCH] HID: remove duplicate hid_warn_ratelimited definition
The hid_warn_ratelimited macro is defined twice in include/linux/hid.h:
at line 1317 (added by commit 4051ead99888) and at line 1339 (added by
commit 1d64624243af).
The second definition is correctly grouped with other ratelimited macros.
Remove the duplicate definition at line 1317.
Fixes: 1d64624243af ("HID: core: Add printk_ratelimited variants to hid_warn() etc")
Signed-off-by: Liu Kai <lukace97@outlook.com>
---
include/linux/hid.h | 2 --
1 file changed, 2 deletions(-)
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 442a80d79e89..e1877a9a58e2 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -1314,8 +1314,6 @@ void hid_quirks_exit(__u16 bus);
dev_notice(&(hid)->dev, fmt, ##__VA_ARGS__)
#define hid_warn(hid, fmt, ...) \
dev_warn(&(hid)->dev, fmt, ##__VA_ARGS__)
-#define hid_warn_ratelimited(hid, fmt, ...) \
- dev_warn_ratelimited(&(hid)->dev, fmt, ##__VA_ARGS__)
#define hid_info(hid, fmt, ...) \
dev_info(&(hid)->dev, fmt, ##__VA_ARGS__)
#define hid_dbg(hid, fmt, ...) \
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v2 1/3] HID: asus: export asus_hid_fnlock_notify() for direct fn-lock control
From: Denis Benato @ 2026-05-06 22:17 UTC (permalink / raw)
To: Marcus Grenängen, platform-driver-x86
Cc: linux-input, linux-kernel, luke, hansg, ilpo.jarvinen, jikos,
bentiss, corentin.chary
In-Reply-To: <20260506193326.5862-2-marcus@grenangen.se>
On 5/6/26 21:33, Marcus Grenängen wrote:
> Some ASUS platforms cannot control fn-lock via WMI DEVS and must send a
> HID feature report directly to the N-Key keyboard device instead.
>
> Add a module-level fnlock_hdev pointer (protected by a mutex) that is set
> at probe time for devices with QUIRK_HID_FN_LOCK and cleared at remove.
> Export asus_hid_fnlock_notify(bool) so that asus-armoury can call into
> hid-asus without a circular dependency.
>
> Signed-off-by: Marcus Grenängen <marcus@grenangen.se>
> ---
> drivers/hid/hid-asus.c | 43 +++++++++++++++++++++-
> include/linux/platform_data/x86/asus-wmi.h | 5 +++
> 2 files changed, 47 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index d34d74df3dc0..8a51dacf35eb 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -584,6 +584,38 @@ static void asus_sync_fn_lock(struct work_struct *work)
> asus_kbd_set_fn_lock(drvdata->hdev, drvdata->fn_lock);
> }
>
> +/*
> + * Module-level reference to the HID device that handles fn-lock via feature
> + * report. Set at probe and cleared at remove for QUIRK_HID_FN_LOCK devices.
> + * Protected by fnlock_hdev_lock.
> + */
> +static DEFINE_MUTEX(fnlock_hdev_lock);
> +static struct hid_device *fnlock_hdev;
> +
> +/**
> + * asus_hid_fnlock_notify() - Set fn-lock state directly via HID feature report.
> + * @enabled: true to lock fn (F1-F12 primary), false to unlock.
> + *
> + * Called by asus-armoury on platforms where the WMI DEVS path for fn-lock is
> + * non-functional (e.g. ASUS ProArt P16, N-Key keyboard product ID 0x19B6).
> + *
> + * Returns 0 on success, -ENODEV if no fn-lock capable HID device is present.
> + */
> +int asus_hid_fnlock_notify(bool enabled)
Generally I see _notify naming being used for internal kernel
messaging or to notify the sysfs of a change, can you change
the name of this to something like asus_hid_fnlock_enable()
or asys_hid_fnlock_set()?
> +{
> + int ret = -ENODEV;
> +
> + guard(mutex)(&fnlock_hdev_lock);
> + if (fnlock_hdev) {
> + ret = asus_kbd_set_fn_lock(fnlock_hdev, enabled);
> + /* hid_hw_raw_request returns byte count on success; normalise to 0 */
The pattern I see most ofter used is
ret = operation()
if (ret < 0)
return ret;
return 0;
IMHO much easier to read and doesn't need a comment to explain.
> + if (ret > 0)
> + ret = 0;
> + }
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(asus_hid_fnlock_notify);
> +
> static void asus_schedule_work(struct asus_kbd_leds *led)
> {
> unsigned long flags;
> @@ -969,6 +1001,8 @@ static int asus_input_configured(struct hid_device *hdev, struct hid_input *hi)
> drvdata->fn_lock = true;
> INIT_WORK(&drvdata->fn_lock_sync_work, asus_sync_fn_lock);
> asus_kbd_set_fn_lock(hdev, true);
> + guard(mutex)(&fnlock_hdev_lock);
> + fnlock_hdev = hdev;
> }
>
> if (drvdata->tp) {
> @@ -1008,6 +1042,8 @@ static int asus_input_configured(struct hid_device *hdev, struct hid_input *hi)
> drvdata->fn_lock = true;
> INIT_WORK(&drvdata->fn_lock_sync_work, asus_sync_fn_lock);
> asus_kbd_set_fn_lock(hdev, true);
> + guard(mutex)(&fnlock_hdev_lock);
> + fnlock_hdev = hdev;
> }
>
> return 0;
> @@ -1362,8 +1398,13 @@ static void asus_remove(struct hid_device *hdev)
> cancel_work_sync(&drvdata->kbd_backlight->work);
> }
>
> - if (drvdata->quirks & QUIRK_HID_FN_LOCK)
> + if (drvdata->quirks & QUIRK_HID_FN_LOCK) {
> + scoped_guard(mutex, &fnlock_hdev_lock) {
> + if (fnlock_hdev == hdev)
> + fnlock_hdev = NULL;
> + }
> cancel_work_sync(&drvdata->fn_lock_sync_work);
> + }
>
> hid_hw_stop(hdev);
> }
> diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
> index 554f41b827e1..20facd5da74e 100644
> --- a/include/linux/platform_data/x86/asus-wmi.h
> +++ b/include/linux/platform_data/x86/asus-wmi.h
> @@ -196,6 +196,7 @@ int asus_wmi_evaluate_method(u32 method_id, u32 arg0, u32 arg1, u32 *retval);
> int asus_hid_register_listener(struct asus_hid_listener *cdev);
> void asus_hid_unregister_listener(struct asus_hid_listener *cdev);
> int asus_hid_event(enum asus_hid_event event);
> +int asus_hid_fnlock_notify(bool enabled);
> #else
> static inline void set_ally_mcu_hack(enum asus_ally_mcu_hack status)
> {
> @@ -227,6 +228,10 @@ static inline int asus_hid_event(enum asus_hid_event event)
> {
> return -ENODEV;
> }
> +static inline int asus_hid_fnlock_notify(bool enabled)
> +{
> + return -ENODEV;
> +}
> #endif
>
> #endif /* __PLATFORM_DATA_X86_ASUS_WMI_H */
^ permalink raw reply
* Re: [PATCH v2 3/3] platform/x86: asus-armoury: add fn_lock firmware attribute
From: Denis Benato @ 2026-05-06 22:10 UTC (permalink / raw)
To: Marcus Grenängen, platform-driver-x86
Cc: linux-input, linux-kernel, luke, hansg, ilpo.jarvinen, jikos,
bentiss, corentin.chary
In-Reply-To: <20260506193326.5862-4-marcus@grenangen.se>
On 5/6/26 21:33, Marcus Grenängen wrote:
> Add a fn_lock attribute to the asus-armoury firmware-attributes interface,
> allowing userspace to read and set the Fn-lock state (whether F1-F12 keys
> are primary or media/system keys are primary).
>
> On most ASUS laptops fn-lock is backed by WMI DEVID 0x00100023 and the
> attribute uses armoury_get/set_devstate() as normal. On platforms where
> the WMI DEVS call is a no-op (fnlock_use_hid quirk, e.g. ProArt P16), the
> store path calls asus_hid_fnlock_notify() to send the feature report
> directly to the N-Key keyboard via hid-asus. The show path returns
> -EOPNOTSUPP on such platforms as the hardware provides no readback.
>
> The fnlock_use_hid flag is detected at init time via dmi_match() on
> DMI_PRODUCT_FAMILY. A direct DMI check is used rather than reading the
> asus-nb-wmi quirk flag because asus-armoury and asus-nb-wmi are both
> loadable modules at the same init level, so the asus_ref pointer set by
> asus-wmi may not yet be valid when asus-armoury initialises.
>
> Signed-off-by: Marcus Grenängen <marcus@grenangen.se>
> ---
> drivers/platform/x86/asus-armoury.c | 69 +++++++++++++++++++++++++++++
> 1 file changed, 69 insertions(+)
>
> diff --git a/drivers/platform/x86/asus-armoury.c b/drivers/platform/x86/asus-armoury.c
> index 5b0987ccc270..9d7646eff944 100644
> --- a/drivers/platform/x86/asus-armoury.c
> +++ b/drivers/platform/x86/asus-armoury.c
> @@ -93,6 +93,7 @@ struct asus_armoury_priv {
>
> u32 mini_led_dev_id;
> u32 gpu_mux_dev_id;
> + bool fnlock_use_hid;
> };
>
> static struct asus_armoury_priv asus_armoury = {
> @@ -778,6 +779,58 @@ ASUS_ATTR_GROUP_ROG_TUNABLE(nv_tgp, "nv_tgp", ASUS_WMI_DEVID_DGPU_SET_TGP,
> ASUS_ATTR_GROUP_INT_VALUE_ONLY_RO(nv_base_tgp, ATTR_NV_BASE_TGP, ASUS_WMI_DEVID_DGPU_BASE_TGP,
> "Read the base TGP value");
>
> +/*
> + * fn_lock: toggle whether Fn key is locked (F1-F12 primary) or unlocked
> + * (media/system keys primary).
> + *
> + * On most ASUS laptops this is backed by WMI DEVID 0x00100023. On some
> + * platforms (e.g. ProArt P16) that DEVS call is a no-op and the state must
> + * be sent as a HID feature report to the N-Key keyboard via hid-asus.
> + */
> +static ssize_t fn_lock_current_value_show(struct kobject *kobj,
> + struct kobj_attribute *attr, char *buf)
> +{
> + u32 result;
> + int err;
> +
> + if (asus_armoury.fnlock_use_hid)
> + return -EOPNOTSUPP;
> +
> + err = armoury_get_devstate(attr, &result, ASUS_WMI_DEVID_FNLOCK);
> + if (err)
> + return err;
> +
> + return sysfs_emit(buf, "%u\n", result & 1);
> +}
> +
> +static ssize_t fn_lock_current_value_store(struct kobject *kobj,
> + struct kobj_attribute *attr,
> + const char *buf, size_t count)
> +{
> + bool enable;
> + int err;
> +
> + err = kstrtobool(buf, &enable);
> + if (err)
> + return err;
> +
> + if (asus_armoury.fnlock_use_hid) {
> + err = asus_hid_fnlock_notify(enable);
Doing this would introduce a dependency from asus-armoury to hid-asus,
let's not do that.
Instead only show this attribute if it's actually doing something,
you can check it from asus-wmi: asus-armoury already depends on it.
Edit: you are actually already registering this only if fnlock_use_hid is
true, so the else looks dead code to me.
I think there is something not right here.
> + if (err)
> + return err;
> + } else {
> + err = armoury_set_devstate(attr, enable ? 1 : 0, NULL,
> + ASUS_WMI_DEVID_FNLOCK);
> + if (err)
> + return err;
> + }
> +
> + sysfs_notify(kobj, NULL, attr->attr.name);
> + return count;
> +}
> +
> +ASUS_ATTR_GROUP_BOOL(fn_lock, "fn_lock", "Set the Fn-lock state");
> +
> /* If an attribute does not require any special case handling add it here */
> static const struct asus_attr_group armoury_attr_groups[] = {
> { &egpu_connected_attr_group, ASUS_WMI_DEVID_EGPU_CONNECTED },
> @@ -926,6 +979,16 @@ static int asus_fw_attr_add(void)
> }
> }
>
> + if (asus_armoury.fnlock_use_hid ||
> + armoury_has_devstate(ASUS_WMI_DEVID_FNLOCK)) {
> + err = sysfs_create_group(&asus_armoury.fw_attr_kset->kobj,
> + &fn_lock_attr_group);
> + if (err) {
> + pr_err("Failed to create sysfs-group for fn_lock\n");
> + goto err_remove_gpu_mux_group;
> + }
> + }
> +
> for (i = 0; i < ARRAY_SIZE(armoury_attr_groups); i++) {
> if (!armoury_has_devstate(armoury_attr_groups[i].wmi_devid))
> continue;
> @@ -963,6 +1026,8 @@ static int asus_fw_attr_add(void)
> sysfs_remove_group(&asus_armoury.fw_attr_kset->kobj,
> armoury_attr_groups[i].attr_group);
> }
> + sysfs_remove_group(&asus_armoury.fw_attr_kset->kobj, &fn_lock_attr_group);
> +err_remove_gpu_mux_group:
> if (asus_armoury.gpu_mux_dev_id)
> sysfs_remove_group(&asus_armoury.fw_attr_kset->kobj, &gpu_mux_mode_attr_group);
> err_remove_mini_led_group:
> @@ -1121,6 +1186,8 @@ static int __init asus_fw_init(void)
>
> init_rog_tunables();
>
> + asus_armoury.fnlock_use_hid = dmi_match(DMI_PRODUCT_FAMILY, "ProArt P16");
Perhaps you can reuse something from asus-wmi instead of re-doing the dmi_match again
in this driver.
> +
> /* Must always be last step to ensure data is available */
> return asus_fw_attr_add();
> }
> @@ -1138,6 +1205,8 @@ static void __exit asus_fw_exit(void)
> if (asus_armoury.gpu_mux_dev_id)
> sysfs_remove_group(&asus_armoury.fw_attr_kset->kobj, &gpu_mux_mode_attr_group);
>
> + sysfs_remove_group(&asus_armoury.fw_attr_kset->kobj, &fn_lock_attr_group);
Not guarded the same way as the sysfs_create_group therefore will trigger
on hardware that doesn't need this.
> +
> if (asus_armoury.mini_led_dev_id)
> sysfs_remove_group(&asus_armoury.fw_attr_kset->kobj, &mini_led_mode_attr_group);
>
^ permalink raw reply
* Re: [PATCH v2 1/3] HID: asus: export asus_hid_fnlock_notify() for direct fn-lock control
From: Randy Dunlap @ 2026-05-06 22:00 UTC (permalink / raw)
To: Marcus Grenängen, platform-driver-x86, denis.benato
Cc: linux-input, linux-kernel, luke, hansg, ilpo.jarvinen, jikos,
bentiss, corentin.chary
In-Reply-To: <20260506193326.5862-2-marcus@grenangen.se>
On 5/6/26 12:33 PM, Marcus Grenängen wrote:
> +/**
> + * asus_hid_fnlock_notify() - Set fn-lock state directly via HID feature report.
> + * @enabled: true to lock fn (F1-F12 primary), false to unlock.
> + *
> + * Called by asus-armoury on platforms where the WMI DEVS path for fn-lock is
> + * non-functional (e.g. ASUS ProArt P16, N-Key keyboard product ID 0x19B6).
> + *
> + * Returns 0 on success, -ENODEV if no fn-lock capable HID device is present.
* Returns: ...
> + */
> +int asus_hid_fnlock_notify(bool enabled)
> +{
--
~Randy
^ 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