Add support for the I2C-connected Awinic AW86938 LRA haptic driver.
The AW86938 has a similar but slightly different register layout. In
particular, the boost mode register values.
The AW86938 also has some extra features that aren't implemented
in this driver yet.
Signed-off-by: Griffin Kroah-Hartman <griffin.kroah@fairphone.com>
---
drivers/input/misc/aw86927.c | 65 ++++++++++++++++++++++++++++++++++++--------
1 file changed, 53 insertions(+), 12 deletions(-)
diff --git a/drivers/input/misc/aw86927.c b/drivers/input/misc/aw86927.c
index 8ad361239cfe3a888628b15e4dbdeed0c9ca3d1a..28ec6e42fd452a0edf1e1b9a9614e2723c6d9f93 100644
--- a/drivers/input/misc/aw86927.c
+++ b/drivers/input/misc/aw86927.c
@@ -43,6 +43,12 @@
#define AW86927_PLAYCFG1_BST_VOUT_VREFSET_MASK GENMASK(6, 0)
#define AW86927_PLAYCFG1_BST_8500MV 0x50
+#define AW86938_PLAYCFG1_REG 0x06
+#define AW86938_PLAYCFG1_BST_MODE_MASK GENMASK(5, 5)
+#define AW86938_PLAYCFG1_BST_MODE_BYPASS 0
+#define AW86938_PLAYCFG1_BST_VOUT_VREFSET_MASK GENMASK(4, 0)
+#define AW86938_PLAYCFG1_BST_7000MV 0x11
+
#define AW86927_PLAYCFG2_REG 0x07
#define AW86927_PLAYCFG3_REG 0x08
@@ -140,6 +146,7 @@
#define AW86927_CHIPIDH_REG 0x57
#define AW86927_CHIPIDL_REG 0x58
#define AW86927_CHIPID 0x9270
+#define AW86938_CHIPID 0x9380
#define AW86927_TMCFG_REG 0x5b
#define AW86927_TMCFG_UNLOCK 0x7d
@@ -173,7 +180,13 @@ enum aw86927_work_mode {
AW86927_RAM_MODE,
};
+enum aw86927_model {
+ AW86927,
+ AW86938,
+};
+
struct aw86927_data {
+ enum aw86927_model model;
struct work_struct play_work;
struct device *dev;
struct input_dev *input_dev;
@@ -377,7 +390,7 @@ static int aw86927_play_sine(struct aw86927_data *haptics)
return err;
/* set gain to value lower than 0x80 to avoid distorted playback */
- err = regmap_write(haptics->regmap, AW86927_PLAYCFG2_REG, 0x7c);
+ err = regmap_write(haptics->regmap, AW86927_PLAYCFG2_REG, 0x45);
if (err)
return err;
@@ -565,13 +578,26 @@ static int aw86927_haptic_init(struct aw86927_data *haptics)
if (err)
return err;
- err = regmap_update_bits(haptics->regmap,
- AW86927_PLAYCFG1_REG,
- AW86927_PLAYCFG1_BST_VOUT_VREFSET_MASK,
- FIELD_PREP(AW86927_PLAYCFG1_BST_VOUT_VREFSET_MASK,
- AW86927_PLAYCFG1_BST_8500MV));
- if (err)
- return err;
+ switch (haptics->model) {
+ case AW86927:
+ err = regmap_update_bits(haptics->regmap,
+ AW86927_PLAYCFG1_REG,
+ AW86927_PLAYCFG1_BST_VOUT_VREFSET_MASK,
+ FIELD_PREP(AW86927_PLAYCFG1_BST_VOUT_VREFSET_MASK,
+ AW86927_PLAYCFG1_BST_8500MV));
+ if (err)
+ return err;
+ break;
+ case AW86938:
+ err = regmap_update_bits(haptics->regmap,
+ AW86938_PLAYCFG1_REG,
+ AW86938_PLAYCFG1_BST_VOUT_VREFSET_MASK,
+ FIELD_PREP(AW86938_PLAYCFG1_BST_VOUT_VREFSET_MASK,
+ AW86938_PLAYCFG1_BST_7000MV));
+ if (err)
+ return err;
+ break;
+ }
err = regmap_update_bits(haptics->regmap,
AW86927_PLAYCFG3_REG,
@@ -599,6 +625,9 @@ static int aw86927_ram_init(struct aw86927_data *haptics)
FIELD_PREP(AW86927_SYSCTRL3_EN_RAMINIT_MASK,
AW86927_SYSCTRL3_EN_RAMINIT_ON));
+ /* AW86938 wants a 1ms delay here */
+ usleep_range(1000, 1500);
+
/* Set base address for the start of the SRAM waveforms */
err = regmap_write(haptics->regmap,
AW86927_BASEADDRH_REG, AW86927_BASEADDRH_VAL);
@@ -717,9 +746,19 @@ static int aw86927_detect(struct aw86927_data *haptics)
chip_id = be16_to_cpu(read_buf);
- if (chip_id != AW86927_CHIPID) {
- dev_err(haptics->dev, "Unexpected CHIPID value 0x%x\n", chip_id);
- return -ENODEV;
+ switch (haptics->model) {
+ case AW86927:
+ if (chip_id != AW86927_CHIPID) {
+ dev_err(haptics->dev, "Unexpected CHIPID value 0x%x\n", chip_id);
+ return -ENODEV;
+ }
+ break;
+ case AW86938:
+ if (chip_id != AW86938_CHIPID) {
+ dev_err(haptics->dev, "Unexpected CHIPID value 0x%x\n", chip_id);
+ return -ENODEV;
+ }
+ break;
}
return 0;
@@ -736,6 +775,7 @@ static int aw86927_probe(struct i2c_client *client)
haptics->dev = &client->dev;
haptics->client = client;
+ haptics->model = (enum aw86927_model)device_get_match_data(&client->dev);
i2c_set_clientdata(client, haptics);
@@ -825,7 +865,8 @@ static int aw86927_probe(struct i2c_client *client)
}
static const struct of_device_id aw86927_of_id[] = {
- { .compatible = "awinic,aw86927" },
+ { .compatible = "awinic,aw86927", .data = (void *)AW86927 },
+ { .compatible = "awinic,aw86938", .data = (void *)AW86938 },
{ /* sentinel */ }
};
--
2.43.0Hi Griffin,
On Wed, Jan 28, 2026 at 04:51:14PM +0100, Griffin Kroah-Hartman wrote:
> @@ -717,9 +746,19 @@ static int aw86927_detect(struct aw86927_data *haptics)
>
> chip_id = be16_to_cpu(read_buf);
>
> - if (chip_id != AW86927_CHIPID) {
> - dev_err(haptics->dev, "Unexpected CHIPID value 0x%x\n", chip_id);
> - return -ENODEV;
> + switch (haptics->model) {
> + case AW86927:
> + if (chip_id != AW86927_CHIPID) {
> + dev_err(haptics->dev, "Unexpected CHIPID value 0x%x\n", chip_id);
> + return -ENODEV;
> + }
If we are able to query chip ID why do we need to have separate
compatibles? I would define chip data structure with differences between
variants and assign and use it instead of having separate compatible.
Thanks.
--
Dmitry
On 2/1/26 2:49 AM, Dmitry Torokhov wrote:
> Hi Griffin,
>
> On Wed, Jan 28, 2026 at 04:51:14PM +0100, Griffin Kroah-Hartman wrote:
>> @@ -717,9 +746,19 @@ static int aw86927_detect(struct aw86927_data *haptics)
>>
>> chip_id = be16_to_cpu(read_buf);
>>
>> - if (chip_id != AW86927_CHIPID) {
>> - dev_err(haptics->dev, "Unexpected CHIPID value 0x%x\n", chip_id);
>> - return -ENODEV;
>> + switch (haptics->model) {
>> + case AW86927:
>> + if (chip_id != AW86927_CHIPID) {
>> + dev_err(haptics->dev, "Unexpected CHIPID value 0x%x\n", chip_id);
>> + return -ENODEV;
>> + }
>
> If we are able to query chip ID why do we need to have separate
> compatibles? I would define chip data structure with differences between
> variants and assign and use it instead of having separate compatible.
dt-bindings guidelines explicitly call for this, a chipid comparison
then works as a safety net
Konrad
On Mon, Feb 02, 2026 at 11:12:26AM +0100, Konrad Dybcio wrote:
> On 2/1/26 2:49 AM, Dmitry Torokhov wrote:
> > Hi Griffin,
> >
> > On Wed, Jan 28, 2026 at 04:51:14PM +0100, Griffin Kroah-Hartman wrote:
> >> @@ -717,9 +746,19 @@ static int aw86927_detect(struct aw86927_data *haptics)
> >>
> >> chip_id = be16_to_cpu(read_buf);
> >>
> >> - if (chip_id != AW86927_CHIPID) {
> >> - dev_err(haptics->dev, "Unexpected CHIPID value 0x%x\n", chip_id);
> >> - return -ENODEV;
> >> + switch (haptics->model) {
> >> + case AW86927:
> >> + if (chip_id != AW86927_CHIPID) {
> >> + dev_err(haptics->dev, "Unexpected CHIPID value 0x%x\n", chip_id);
> >> + return -ENODEV;
> >> + }
> >
> > If we are able to query chip ID why do we need to have separate
> > compatibles? I would define chip data structure with differences between
> > variants and assign and use it instead of having separate compatible.
>
> dt-bindings guidelines explicitly call for this, a chipid comparison
No, they don't. If devices offer autodetection, then they are in fact
fully compatible for the SW.
Best regards,
Krzysztof
Hi Konrad,
On Mon Feb 2, 2026 at 11:12 AM CET, Konrad Dybcio wrote:
> On 2/1/26 2:49 AM, Dmitry Torokhov wrote:
>> Hi Griffin,
>>
>> On Wed, Jan 28, 2026 at 04:51:14PM +0100, Griffin Kroah-Hartman wrote:
>>> @@ -717,9 +746,19 @@ static int aw86927_detect(struct aw86927_data *haptics)
>>>
>>> chip_id = be16_to_cpu(read_buf);
>>>
>>> - if (chip_id != AW86927_CHIPID) {
>>> - dev_err(haptics->dev, "Unexpected CHIPID value 0x%x\n", chip_id);
>>> - return -ENODEV;
>>> + switch (haptics->model) {
>>> + case AW86927:
>>> + if (chip_id != AW86927_CHIPID) {
>>> + dev_err(haptics->dev, "Unexpected CHIPID value 0x%x\n", chip_id);
>>> + return -ENODEV;
>>> + }
>>
>> If we are able to query chip ID why do we need to have separate
>> compatibles? I would define chip data structure with differences between
>> variants and assign and use it instead of having separate compatible.
>
> dt-bindings guidelines explicitly call for this, a chipid comparison
> then works as a safety net
Are you saying, that
1. we should enforce dt-bindings == CHIP_ID (what's currently done)
or
2. we should have both compatibles with no handling based on compatible,
but only use CHIP_ID at runtime to change behavior
Regards
Luca
On 2/2/26 11:14 AM, Luca Weiss wrote:
> Hi Konrad,
>
> On Mon Feb 2, 2026 at 11:12 AM CET, Konrad Dybcio wrote:
>> On 2/1/26 2:49 AM, Dmitry Torokhov wrote:
>>> Hi Griffin,
>>>
>>> On Wed, Jan 28, 2026 at 04:51:14PM +0100, Griffin Kroah-Hartman wrote:
>>>> @@ -717,9 +746,19 @@ static int aw86927_detect(struct aw86927_data *haptics)
>>>>
>>>> chip_id = be16_to_cpu(read_buf);
>>>>
>>>> - if (chip_id != AW86927_CHIPID) {
>>>> - dev_err(haptics->dev, "Unexpected CHIPID value 0x%x\n", chip_id);
>>>> - return -ENODEV;
>>>> + switch (haptics->model) {
>>>> + case AW86927:
>>>> + if (chip_id != AW86927_CHIPID) {
>>>> + dev_err(haptics->dev, "Unexpected CHIPID value 0x%x\n", chip_id);
>>>> + return -ENODEV;
>>>> + }
>>>
>>> If we are able to query chip ID why do we need to have separate
>>> compatibles? I would define chip data structure with differences between
>>> variants and assign and use it instead of having separate compatible.
>>
>> dt-bindings guidelines explicitly call for this, a chipid comparison
>> then works as a safety net
>
> Are you saying, that
>
> 1. we should enforce dt-bindings == CHIP_ID (what's currently done)
This
>
> or
>
> 2. we should have both compatibles with no handling based on compatible,
> but only use CHIP_ID at runtime to change behavior
This is spaghetti
Konrad
On Mon, Feb 02, 2026 at 11:19:36AM +0100, Konrad Dybcio wrote:
> On 2/2/26 11:14 AM, Luca Weiss wrote:
> > Hi Konrad,
> >
> > On Mon Feb 2, 2026 at 11:12 AM CET, Konrad Dybcio wrote:
> >> On 2/1/26 2:49 AM, Dmitry Torokhov wrote:
> >>> Hi Griffin,
> >>>
> >>> On Wed, Jan 28, 2026 at 04:51:14PM +0100, Griffin Kroah-Hartman wrote:
> >>>> @@ -717,9 +746,19 @@ static int aw86927_detect(struct aw86927_data *haptics)
> >>>>
> >>>> chip_id = be16_to_cpu(read_buf);
> >>>>
> >>>> - if (chip_id != AW86927_CHIPID) {
> >>>> - dev_err(haptics->dev, "Unexpected CHIPID value 0x%x\n", chip_id);
> >>>> - return -ENODEV;
> >>>> + switch (haptics->model) {
> >>>> + case AW86927:
> >>>> + if (chip_id != AW86927_CHIPID) {
> >>>> + dev_err(haptics->dev, "Unexpected CHIPID value 0x%x\n", chip_id);
> >>>> + return -ENODEV;
> >>>> + }
> >>>
> >>> If we are able to query chip ID why do we need to have separate
> >>> compatibles? I would define chip data structure with differences between
> >>> variants and assign and use it instead of having separate compatible.
> >>
> >> dt-bindings guidelines explicitly call for this, a chipid comparison
> >> then works as a safety net
> >
> > Are you saying, that
> >
> > 1. we should enforce dt-bindings == CHIP_ID (what's currently done)
>
> This
No. If there is a compatible chip with different ID (for whatever reason
- maybe there is additional functionality that either board does not
need or the driver does not implement) we absolutely should not refuse
to bind the driver.
Hint: this thing is called _compatible_ for a reason.
>
> >
> > or
> >
> > 2. we should have both compatibles with no handling based on compatible,
> > but only use CHIP_ID at runtime to change behavior
>
> This is spaghetti
I really do not understand the aversion of DT maintainers to generic
compatibles. We see this in I2C HID where we keep adding compatibles
for what could be described via device properties.
Thanks.
--
Dmitry
On 2/2/26 12:04 PM, Dmitry Torokhov wrote:
> On Mon, Feb 02, 2026 at 11:19:36AM +0100, Konrad Dybcio wrote:
>> On 2/2/26 11:14 AM, Luca Weiss wrote:
>>> Hi Konrad,
>>>
>>> On Mon Feb 2, 2026 at 11:12 AM CET, Konrad Dybcio wrote:
>>>> On 2/1/26 2:49 AM, Dmitry Torokhov wrote:
>>>>> Hi Griffin,
>>>>>
>>>>> On Wed, Jan 28, 2026 at 04:51:14PM +0100, Griffin Kroah-Hartman wrote:
>>>>>> @@ -717,9 +746,19 @@ static int aw86927_detect(struct aw86927_data *haptics)
>>>>>>
>>>>>> chip_id = be16_to_cpu(read_buf);
>>>>>>
>>>>>> - if (chip_id != AW86927_CHIPID) {
>>>>>> - dev_err(haptics->dev, "Unexpected CHIPID value 0x%x\n", chip_id);
>>>>>> - return -ENODEV;
>>>>>> + switch (haptics->model) {
>>>>>> + case AW86927:
>>>>>> + if (chip_id != AW86927_CHIPID) {
>>>>>> + dev_err(haptics->dev, "Unexpected CHIPID value 0x%x\n", chip_id);
>>>>>> + return -ENODEV;
>>>>>> + }
>>>>>
>>>>> If we are able to query chip ID why do we need to have separate
>>>>> compatibles? I would define chip data structure with differences between
>>>>> variants and assign and use it instead of having separate compatible.
>>>>
>>>> dt-bindings guidelines explicitly call for this, a chipid comparison
>>>> then works as a safety net
>>>
>>> Are you saying, that
>>>
>>> 1. we should enforce dt-bindings == CHIP_ID (what's currently done)
>>
>> This
>
> No. If there is a compatible chip with different ID (for whatever reason
> - maybe there is additional functionality that either board does not
> need or the driver does not implement) we absolutely should not refuse
> to bind the driver.
>
> Hint: this thing is called _compatible_ for a reason.
Right, the reason you have in mind is fulfilled by fallback compatibles
(i.e. "vendor,actualchipname", "vendor,similarchipname" where the driver
only considers the latter becuase the software interface hasn't changed)
>
>>
>>>
>>> or
>>>
>>> 2. we should have both compatibles with no handling based on compatible,
>>> but only use CHIP_ID at runtime to change behavior
>>
>> This is spaghetti
>
> I really do not understand the aversion of DT maintainers to generic
> compatibles. We see this in I2C HID where we keep adding compatibles
> for what could be described via device properties.
This is because it's the only way to allow for retroactive changes that
do not require changing firmware. That's why ACPI carries new identifiers
for even very slightly different devices too. Once the firmware containing
(ACPI tables / DTB) is put on a production device, it is generally not
going to ever change.
CHIP_ID registers are a good tool to validate that the author of the
firmware table is doing the right thing, but solely relying on them
encourages creating a "vendor,haptic" compatible, which I'm sure you'll
agree is totally meaningless.
That's especially if the naming scheme makes no sense and you can't
even factor out a common wildcard-name (which also happens to be the case
quite often)
Plus a compatible is used to restrict/modify the set of allowed/required
properties, so having an "actual" compatible is required for schema
validation to work
Konrad
On Mon, Feb 02, 2026 at 04:11:51PM +0100, Konrad Dybcio wrote:
> On 2/2/26 12:04 PM, Dmitry Torokhov wrote:
> > On Mon, Feb 02, 2026 at 11:19:36AM +0100, Konrad Dybcio wrote:
> >> On 2/2/26 11:14 AM, Luca Weiss wrote:
> >>> Hi Konrad,
> >>>
> >>> On Mon Feb 2, 2026 at 11:12 AM CET, Konrad Dybcio wrote:
> >>>> On 2/1/26 2:49 AM, Dmitry Torokhov wrote:
> >>>>> Hi Griffin,
> >>>>>
> >>>>> On Wed, Jan 28, 2026 at 04:51:14PM +0100, Griffin Kroah-Hartman wrote:
> >>>>>> @@ -717,9 +746,19 @@ static int aw86927_detect(struct aw86927_data *haptics)
> >>>>>>
> >>>>>> chip_id = be16_to_cpu(read_buf);
> >>>>>>
> >>>>>> - if (chip_id != AW86927_CHIPID) {
> >>>>>> - dev_err(haptics->dev, "Unexpected CHIPID value 0x%x\n", chip_id);
> >>>>>> - return -ENODEV;
> >>>>>> + switch (haptics->model) {
> >>>>>> + case AW86927:
> >>>>>> + if (chip_id != AW86927_CHIPID) {
> >>>>>> + dev_err(haptics->dev, "Unexpected CHIPID value 0x%x\n", chip_id);
> >>>>>> + return -ENODEV;
> >>>>>> + }
> >>>>>
> >>>>> If we are able to query chip ID why do we need to have separate
> >>>>> compatibles? I would define chip data structure with differences between
> >>>>> variants and assign and use it instead of having separate compatible.
> >>>>
> >>>> dt-bindings guidelines explicitly call for this, a chipid comparison
> >>>> then works as a safety net
> >>>
> >>> Are you saying, that
> >>>
> >>> 1. we should enforce dt-bindings == CHIP_ID (what's currently done)
> >>
> >> This
> >
> > No. If there is a compatible chip with different ID (for whatever reason
> > - maybe there is additional functionality that either board does not
> > need or the driver does not implement) we absolutely should not refuse
> > to bind the driver.
> >
> > Hint: this thing is called _compatible_ for a reason.
>
> Right, the reason you have in mind is fulfilled by fallback compatibles
>
> (i.e. "vendor,actualchipname", "vendor,similarchipname" where the driver
> only considers the latter becuase the software interface hasn't changed)
And having chip_id checks will break this...
>
> >
> >>
> >>>
> >>> or
> >>>
> >>> 2. we should have both compatibles with no handling based on compatible,
> >>> but only use CHIP_ID at runtime to change behavior
> >>
> >> This is spaghetti
> >
> > I really do not understand the aversion of DT maintainers to generic
> > compatibles. We see this in I2C HID where we keep adding compatibles
> > for what could be described via device properties.
>
> This is because it's the only way to allow for retroactive changes that
> do not require changing firmware. That's why ACPI carries new identifiers
> for even very slightly different devices too. Once the firmware containing
> (ACPI tables / DTB) is put on a production device, it is generally not
> going to ever change.
They are actually solving slightly different problem. In ACPI world they
allocate a new ID to represent a peripheral in a given design, down to
it's firmware behavior. It encodes much more than chip ID that DT
maintainers want to key off of.
>
> CHIP_ID registers are a good tool to validate that the author of the
> firmware table is doing the right thing, but solely relying on them
> encourages creating a "vendor,haptic" compatible, which I'm sure you'll
> agree is totally meaningless.
Is it? If a piece of hardware speaks i2c-hid protocol why do I need to
know the exact chip that is being used? Depending on the chassis and the
size of the sensing element and the version of the firmware that is
loaded into it the behavior and timings of the same chip may be very
different.
>
> That's especially if the naming scheme makes no sense and you can't
> even factor out a common wildcard-name (which also happens to be the case
> quite often)
>
> Plus a compatible is used to restrict/modify the set of allowed/required
> properties, so having an "actual" compatible is required for schema
> validation to work
Yes, in cases where there is not a common set of properties having
different compatibles makes sense. But in cases when the device is
supposed to have vendor-agnostic behavior insisting on myriad
compatibles makes little sense.
Thanks.
--
Dmitry
On 2/3/26 10:49 AM, Dmitry Torokhov wrote:
> On Mon, Feb 02, 2026 at 04:11:51PM +0100, Konrad Dybcio wrote:
>> On 2/2/26 12:04 PM, Dmitry Torokhov wrote:
>>> On Mon, Feb 02, 2026 at 11:19:36AM +0100, Konrad Dybcio wrote:
>>>> On 2/2/26 11:14 AM, Luca Weiss wrote:
>>>>> Hi Konrad,
>>>>>
>>>>> On Mon Feb 2, 2026 at 11:12 AM CET, Konrad Dybcio wrote:
>>>>>> On 2/1/26 2:49 AM, Dmitry Torokhov wrote:
>>>>>>> Hi Griffin,
>>>>>>>
>>>>>>> On Wed, Jan 28, 2026 at 04:51:14PM +0100, Griffin Kroah-Hartman wrote:
>>>>>>>> @@ -717,9 +746,19 @@ static int aw86927_detect(struct aw86927_data *haptics)
>>>>>>>>
>>>>>>>> chip_id = be16_to_cpu(read_buf);
>>>>>>>>
>>>>>>>> - if (chip_id != AW86927_CHIPID) {
>>>>>>>> - dev_err(haptics->dev, "Unexpected CHIPID value 0x%x\n", chip_id);
>>>>>>>> - return -ENODEV;
>>>>>>>> + switch (haptics->model) {
>>>>>>>> + case AW86927:
>>>>>>>> + if (chip_id != AW86927_CHIPID) {
>>>>>>>> + dev_err(haptics->dev, "Unexpected CHIPID value 0x%x\n", chip_id);
>>>>>>>> + return -ENODEV;
>>>>>>>> + }
>>>>>>>
>>>>>>> If we are able to query chip ID why do we need to have separate
>>>>>>> compatibles? I would define chip data structure with differences between
>>>>>>> variants and assign and use it instead of having separate compatible.
>>>>>>
>>>>>> dt-bindings guidelines explicitly call for this, a chipid comparison
>>>>>> then works as a safety net
>>>>>
>>>>> Are you saying, that
>>>>>
>>>>> 1. we should enforce dt-bindings == CHIP_ID (what's currently done)
>>>>
>>>> This
>>>
>>> No. If there is a compatible chip with different ID (for whatever reason
>>> - maybe there is additional functionality that either board does not
>>> need or the driver does not implement) we absolutely should not refuse
>>> to bind the driver.
>>>
>>> Hint: this thing is called _compatible_ for a reason.
>>
>> Right, the reason you have in mind is fulfilled by fallback compatibles
>>
>> (i.e. "vendor,actualchipname", "vendor,similarchipname" where the driver
>> only considers the latter becuase the software interface hasn't changed)
>
> And having chip_id checks will break this...
Depends on how they're implemented and how different the chips are.
If the software interface is exactly 1:1, with the only difference in the
ID register, something like
if (chipid == SIMILARCHIPNAME_ID || chipid == ACTUALCHIPNAME_ID)
...
would be fitting.
However, more often than not, like in this case, there's actual differences
that need to be taken into account, meaning we already need to act upon
the "actual" compatible
>
>>
>>>
>>>>
>>>>>
>>>>> or
>>>>>
>>>>> 2. we should have both compatibles with no handling based on compatible,
>>>>> but only use CHIP_ID at runtime to change behavior
>>>>
>>>> This is spaghetti
>>>
>>> I really do not understand the aversion of DT maintainers to generic
>>> compatibles. We see this in I2C HID where we keep adding compatibles
>>> for what could be described via device properties.
>>
>> This is because it's the only way to allow for retroactive changes that
>> do not require changing firmware. That's why ACPI carries new identifiers
>> for even very slightly different devices too. Once the firmware containing
>> (ACPI tables / DTB) is put on a production device, it is generally not
>> going to ever change.
>
> They are actually solving slightly different problem. In ACPI world they
> allocate a new ID to represent a peripheral in a given design, down to
> it's firmware behavior. It encodes much more than chip ID that DT
> maintainers want to key off of.
DT sort of does this to. In the Qualcomm world, how you get to interact
with the platform changes dramatically depending on the firmware flavor
it's flashed with (which I'd hope to see go away one day..) - if you have
a Chrome firmware, you're basically free/required to configure everything
from Linux. With Android firmware, much of that heavy lifting must be done
by the hypervisor or the secure world. With Windows firmware, you get the
Android experience + UEFI services. And at the tail end of the scale,
there's the automotive firmware where you don't even get to toggle clocks,
but instead all peripherals' power and performance states are exposed
through dozens of SCMI servers..
Somehow we can try and be smart, deducing the behavior based on the
properties present in DT, but often times, a separate compatible for
"this SoC except with this firmware" needs to exist, as the OS-accessible
software interface is simply different, as if this wasn't the same SoC
anymore.
>
>>
>> CHIP_ID registers are a good tool to validate that the author of the
>> firmware table is doing the right thing, but solely relying on them
>> encourages creating a "vendor,haptic" compatible, which I'm sure you'll
>> agree is totally meaningless.
>
> Is it? If a piece of hardware speaks i2c-hid protocol why do I need to
> know the exact chip that is being used? Depending on the chassis and the
> size of the sensing element and the version of the firmware that is
> loaded into it the behavior and timings of the same chip may be very
> different.
I agree this argument gets overused at times
>>
>> That's especially if the naming scheme makes no sense and you can't
>> even factor out a common wildcard-name (which also happens to be the case
>> quite often)
>>
>> Plus a compatible is used to restrict/modify the set of allowed/required
>> properties, so having an "actual" compatible is required for schema
>> validation to work
>
> Yes, in cases where there is not a common set of properties having
> different compatibles makes sense. But in cases when the device is
> supposed to have vendor-agnostic behavior insisting on myriad
> compatibles makes little sense.
Some people may be thrown off by the golden rule of implementing standards,
which is to break or bend them immediately, claiming full compatibility.
But for cases like hid-over-i2c, I symphatize with the "no one needs
to know if it's a synaptics a00001 or a synaptics a00002" sentiment
Konrad
Hi Griffin,
kernel test robot noticed the following build warnings:
[auto build test WARNING on 0364de6be161e2360cbb1f26d5aff5b343ef7bb0]
url: https://github.com/intel-lab-lkp/linux/commits/Griffin-Kroah-Hartman/dt-bindings-input-awinic-aw86927-Add-Awinic-AW86938/20260129-000753
base: 0364de6be161e2360cbb1f26d5aff5b343ef7bb0
patch link: https://lore.kernel.org/r/20260128-aw86938-driver-v2-2-b51ee086aaf5%40fairphone.com
patch subject: [PATCH v2 2/3] Input: aw86938 - add driver for Awinic AW86938
config: sparc64-allmodconfig (https://download.01.org/0day-ci/archive/20260131/202601311117.t00gEixW-lkp@intel.com/config)
compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project 9b8addffa70cee5b2acc5454712d9cf78ce45710)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260131/202601311117.t00gEixW-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202601311117.t00gEixW-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/input/misc/aw86927.c:778:19: warning: cast to smaller integer type 'enum aw86927_model' from 'const void *' [-Wvoid-pointer-to-enum-cast]
778 | haptics->model = (enum aw86927_model)device_get_match_data(&client->dev);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
vim +778 drivers/input/misc/aw86927.c
766
767 static int aw86927_probe(struct i2c_client *client)
768 {
769 struct aw86927_data *haptics;
770 int err;
771
772 haptics = devm_kzalloc(&client->dev, sizeof(struct aw86927_data), GFP_KERNEL);
773 if (!haptics)
774 return -ENOMEM;
775
776 haptics->dev = &client->dev;
777 haptics->client = client;
> 778 haptics->model = (enum aw86927_model)device_get_match_data(&client->dev);
779
780 i2c_set_clientdata(client, haptics);
781
782 haptics->regmap = devm_regmap_init_i2c(client, &aw86927_regmap_config);
783 if (IS_ERR(haptics->regmap))
784 return dev_err_probe(haptics->dev, PTR_ERR(haptics->regmap),
785 "Failed to allocate register map\n");
786
787 haptics->input_dev = devm_input_allocate_device(haptics->dev);
788 if (!haptics->input_dev)
789 return -ENOMEM;
790
791 haptics->reset_gpio = devm_gpiod_get(haptics->dev, "reset", GPIOD_OUT_HIGH);
792 if (IS_ERR(haptics->reset_gpio))
793 return dev_err_probe(haptics->dev, PTR_ERR(haptics->reset_gpio),
794 "Failed to get reset gpio\n");
795
796 /* Hardware reset */
797 aw86927_hw_reset(haptics);
798
799 /* Software reset */
800 err = regmap_write(haptics->regmap, AW86927_RSTCFG_REG, AW86927_RSTCFG_SOFTRST);
801 if (err)
802 return dev_err_probe(haptics->dev, err, "Failed Software reset\n");
803
804 /* Wait ~3ms until I2C is accessible */
805 usleep_range(3000, 3500);
806
807 err = aw86927_detect(haptics);
808 if (err)
809 return dev_err_probe(haptics->dev, err, "Failed to find chip\n");
810
811 /* IRQ config */
812 err = regmap_write(haptics->regmap, AW86927_SYSCTRL4_REG,
813 FIELD_PREP(AW86927_SYSCTRL4_INT_MODE_MASK,
814 AW86927_SYSCTRL4_INT_MODE_EDGE) |
815 FIELD_PREP(AW86927_SYSCTRL4_INT_EDGE_MODE_MASK,
816 AW86927_SYSCTRL4_INT_EDGE_MODE_POS));
817 if (err)
818 return dev_err_probe(haptics->dev, err, "Failed to configure interrupt modes\n");
819
820 err = regmap_write(haptics->regmap, AW86927_SYSINTM_REG,
821 AW86927_SYSINTM_BST_OVPM |
822 AW86927_SYSINTM_FF_AEM |
823 AW86927_SYSINTM_FF_AFM |
824 AW86927_SYSINTM_DONEM);
825 if (err)
826 return dev_err_probe(haptics->dev, err, "Failed to configure interrupt masks\n");
827
828 err = devm_request_threaded_irq(haptics->dev, client->irq, NULL,
829 aw86927_irq, IRQF_ONESHOT, NULL, haptics);
830 if (err)
831 return dev_err_probe(haptics->dev, err, "Failed to request threaded irq\n");
832
833 INIT_WORK(&haptics->play_work, aw86927_haptics_play_work);
834
835 haptics->input_dev->name = "aw86927-haptics";
836 haptics->input_dev->close = aw86927_close;
837
838 input_set_drvdata(haptics->input_dev, haptics);
839 input_set_capability(haptics->input_dev, EV_FF, FF_RUMBLE);
840
841 err = input_ff_create_memless(haptics->input_dev, NULL, aw86927_haptics_play);
842 if (err)
843 return dev_err_probe(haptics->dev, err, "Failed to create FF dev\n");
844
845 /* Set up registers */
846 err = aw86927_play_mode(haptics, AW86927_STANDBY_MODE);
847 if (err)
848 return dev_err_probe(haptics->dev, err,
849 "Failed to enter standby for Haptic init\n");
850
851 err = aw86927_haptic_init(haptics);
852 if (err)
853 return dev_err_probe(haptics->dev, err, "Haptic init failed\n");
854
855 /* RAM init, upload the waveform for playback */
856 err = aw86927_ram_init(haptics);
857 if (err)
858 return dev_err_probe(haptics->dev, err, "Failed to init aw86927 sram\n");
859
860 err = input_register_device(haptics->input_dev);
861 if (err)
862 return dev_err_probe(haptics->dev, err, "Failed to register input device\n");
863
864 return 0;
865 }
866
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
© 2016 - 2026 Red Hat, Inc.