[PATCH v4 0/2] iio: light: Add support for Capella cm36686 and cm36672p sensors

[PATCH v4 0/2] iio: light: Add support for Capella cm36686 and cm36672p sensors

[Erikas Bitovtas]

This patch series adds support for Capella cm36686 and cm36672p ambient
light and proximity sensors.

Capella cm36686 is a combined ambient light and proximity sensor with
adjustable integration time, interrupt and hysteresis support. It has
the slave address of 0x60. cm36672p is fully compatible with cm36686,
except that it is a proximity-only sensor.

Unfortunately, datasheets for these sensors are not publicly
available. Initially, this patch series introduced a new driver, which
had code based on Android downstream kernels for devices which did use
these sensors and a previous submission for cm36672p to mailing lists:
https://github.com/LineageOS/android_kernel_xiaomi_msm8992/blob/cm-14.1/drivers/iio/light/cm36686.c
https://github.com/shakalaca/ASUS_ZenFone_ZD551KL/blob/android-6.0/kernel/drivers/input/misc/cm36283.c
https://lore.kernel.org/linux-iio/1465462845-1571-1-git-send-email-capellamicro@gmail.com/

However, a compatible driver has been found which is already upstream
and can be used instead. Hence, this patch series adds support for
Capella CM36686 and CM36672P to an existing driver for VCNL4040.

The following code has been tested on Asus ZenFone 2 Laser/Selfie, which
uses cm36686 as its ambient light and proximity sensor.

Changes since v3 (misversioned as v1):
- Move Capella enum IDs up so device IDs are sorted by string literal.
- Move device tree table entries up so they are sorted by string
  literal.
- Add a trailing comma to the cm36672p_channels proximity channel entry.
- Link to v3:
https://lore.kernel.org/linux-iio/20260210-cm36686-v1-0-aef68dd46ad4@gmail.com/

Changes since v2:
- Remove the previous unnecessary proposed driver and bindings.
- Add a fallback compatible for cm36686 of vcnl4040.
- Add a new compatible for cm36672p.
- Add channel info for cm36672p.
- Remove redundant information in the dt-bindings commit message.
- Link to v2:
https://lore.kernel.org/linux-iio/20260209-cm36686-v2-0-a48126d2b124@gmail.com/

Changes since v1:
- Add copyright information.
- Sort includes in alphabetical order.
- Add trailing commas.
- Remove blank spaces where unnecessary.
- Add a fallback for capella,cm36686 compatible.
- Make power supplies required.
- Add '-microamp' suffix for capella,proximity-led-current.
- Replace local caching and i2c_smbus calls with regmap API.
- Make interrupt optional.
- Add action or reset only after setup is done.
- Replace mutex_[un]lock calls with guard(mutex)
- Add comments on where mutex is used.
- Add comments on proximity register defaults.
- Remove default proximity sensor duty ratio and integration time. Those
  were taken from the testing device and had no real reason to be there.
- Replace dev_err_probe on device's part ID with a warning.
- Replace chip->supplies property with a single
  devm_regulator_bulk_get_enable call.
- Use individual structs instead of array-style device info
- Remove enums which are no longer used.
- Link to v1:
https://lore.kernel.org/linux-iio/20260201-cm36686-v1-0-4949a2a9ba63@gmail.com/

Signed-off-by: Erikas Bitovtas <xerikasxx@gmail.com>
---
Erikas Bitovtas (2):
      dt-bindings: iio: light: vcnl4000: add Capella CM36686 and CM36672P
      iio: light: vcnl4000: add support for Capella CM36686 and CM36672P

 .../bindings/iio/light/vishay,vcnl4000.yaml        | 17 +++++----
 drivers/iio/light/vcnl4000.c                       | 40 ++++++++++++++++++++++
 2 files changed, 51 insertions(+), 6 deletions(-)
---
base-commit: 9152bc8cebcb14dc16b03ec81f2377ee8ce12268
change-id: 20260201-cm36686-fc7a8385f1cd

Best regards,
-- 
Erikas Bitovtas <xerikasxx@gmail.com>

[PATCH v4 1/2] dt-bindings: iio: light: vcnl4000: add Capella CM36686 and CM36672P

[Erikas Bitovtas]

Capella CM36686 is an ambient light and proximity sensor developed by
Capella Microsystems, now a subsidiary of Vishay Intertechnology Inc. It
has an I2C address of 0x60 and is fully compatible with an existing
driver for VCNL4040. Capella CM36672P is a proximity-only sensor that
is fully compatible with CM36686, and therefore with VCNL4040. Add
compatibles for cm36672p and cm36686, with a fallback for cm36686 of
vcnl4040.

Signed-off-by: Erikas Bitovtas <xerikasxx@gmail.com>
---
 .../devicetree/bindings/iio/light/vishay,vcnl4000.yaml  | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml b/Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml
index 4d1a225e8868..2ba4d5de4ec4 100644
--- a/Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml
+++ b/Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml
@@ -18,12 +18,17 @@ allOf:
 
 properties:
   compatible:
-    enum:
-      - vishay,vcnl4000
-      - vishay,vcnl4010
-      - vishay,vcnl4020
-      - vishay,vcnl4040
-      - vishay,vcnl4200
+    oneOf:
+      - enum:
+          - capella,cm36672p
+          - vishay,vcnl4000
+          - vishay,vcnl4010
+          - vishay,vcnl4020
+          - vishay,vcnl4040
+          - vishay,vcnl4200
+      - items:
+          - const: capella,cm36686
+          - const: vishay,vcnl4040
 
   interrupts:
     maxItems: 1

-- 
2.53.0

[PATCH v4 2/2] iio: light: vcnl4000: add support for Capella CM36686 and CM36672P

[Erikas Bitovtas]

Add support for Capella's CM36686 and CM36672P sensors. Capella
CM36686 is an ambient light and proximity sensor that is fully
compatible with VCNL4040 and can be used as is. For CM36672P, which is
a proximity-only sensor, also remove the IIO_LIGHT channel.

Signed-off-by: Erikas Bitovtas <xerikasxx@gmail.com>
---
 drivers/iio/light/vcnl4000.c | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
index a36c23813679..1f8f4e4586f4 100644
--- a/drivers/iio/light/vcnl4000.c
+++ b/drivers/iio/light/vcnl4000.c
@@ -185,6 +185,7 @@ static const int vcnl4040_ps_oversampling_ratio[] = {1, 2, 4, 8};
 #define VCNL4000_SLEEP_DELAY_MS	2000 /* before we enter pm_runtime_suspend */
 
 enum vcnl4000_device_ids {
+	CM36672P,
 	VCNL4000,
 	VCNL4010,
 	VCNL4040,
@@ -235,6 +236,8 @@ struct vcnl4000_chip_spec {
 };
 
 static const struct i2c_device_id vcnl4000_id[] = {
+	{ "cm36672p", CM36672P },
+	{ "cm36686", VCNL4040 },
 	{ "vcnl4000", VCNL4000 },
 	{ "vcnl4010", VCNL4010 },
 	{ "vcnl4020", VCNL4010 },
@@ -1842,6 +1845,22 @@ static const struct iio_chan_spec vcnl4040_channels[] = {
 	}
 };
 
+static const struct iio_chan_spec cm36672p_channels[] = {
+	{
+		.type = IIO_PROXIMITY,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+			BIT(IIO_CHAN_INFO_INT_TIME) |
+			BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO) |
+			BIT(IIO_CHAN_INFO_CALIBBIAS),
+		.info_mask_separate_available = BIT(IIO_CHAN_INFO_INT_TIME) |
+			BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO) |
+			BIT(IIO_CHAN_INFO_CALIBBIAS),
+		.ext_info = vcnl4000_ext_info,
+		.event_spec = vcnl4040_event_spec,
+		.num_event_specs = ARRAY_SIZE(vcnl4040_event_spec),
+	},
+};
+
 static const struct iio_info vcnl4000_info = {
 	.read_raw = vcnl4000_read_raw,
 };
@@ -1867,6 +1886,19 @@ static const struct iio_info vcnl4040_info = {
 };
 
 static const struct vcnl4000_chip_spec vcnl4000_chip_spec_cfg[] = {
+	[CM36672P] = {
+		.prod = "CM36672P",
+		.init = vcnl4200_init,
+		.measure_proximity = vcnl4200_measure_proximity,
+		.set_power_state = vcnl4200_set_power_state,
+		.channels = cm36672p_channels,
+		.num_channels = ARRAY_SIZE(cm36672p_channels),
+		.info = &vcnl4040_info,
+		.irq_thread = vcnl4040_irq_thread,
+		.int_reg = VCNL4040_INT_FLAGS,
+		.ps_it_times = &vcnl4040_ps_it_times,
+		.num_ps_it_times = ARRAY_SIZE(vcnl4040_ps_it_times),
+	},
 	[VCNL4000] = {
 		.prod = "VCNL4000",
 		.init = vcnl4000_init,
@@ -2033,6 +2065,14 @@ static int vcnl4000_probe(struct i2c_client *client)
 }
 
 static const struct of_device_id vcnl_4000_of_match[] = {
+	{
+		.compatible = "capella,cm36672p",
+		.data = (void *)CM36672P,
+	},
+	{
+		.compatible = "capella,cm36686",
+		.data = (void *)VCNL4040,
+	},
 	{
 		.compatible = "vishay,vcnl4000",
 		.data = (void *)VCNL4000,

-- 
2.53.0

Re: [PATCH v4 2/2] iio: light: vcnl4000: add support for Capella CM36686 and CM36672P

[Andy Shevchenko]

On Thu, Feb 12, 2026 at 04:42:48PM +0200, Erikas Bitovtas wrote:
> Add support for Capella's CM36686 and CM36672P sensors. Capella
> CM36686 is an ambient light and proximity sensor that is fully
> compatible with VCNL4040 and can be used as is. For CM36672P, which is
> a proximity-only sensor, also remove the IIO_LIGHT channel.

Perfect!
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>

...

> +	{
> +		.compatible = "capella,cm36672p",
> +		.data = (void *)CM36672P,

Side note: Consider at some point to switch to chip_info, id est use
pointers directly instead of integer indices as pointers here and in
other ID tables. It's for a future development, and not required for
this series.

> +	},
> +	{
> +		.compatible = "capella,cm36686",
> +		.data = (void *)VCNL4040,
> +	},
>  	{
>  		.compatible = "vishay,vcnl4000",
>  		.data = (void *)VCNL4000,

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 1/2] dt-bindings: iio: light: vcnl4000: add Capella CM36686 and CM36672P

[Krzysztof Kozlowski]

On Thu, Feb 12, 2026 at 04:42:47PM +0200, Erikas Bitovtas wrote:
> Capella CM36686 is an ambient light and proximity sensor developed by
> Capella Microsystems, now a subsidiary of Vishay Intertechnology Inc. It
> has an I2C address of 0x60 and is fully compatible with an existing
> driver for VCNL4040.

I wonder why and how...

> Capella CM36672P is a proximity-only sensor that
> is fully compatible with CM36686, and therefore with VCNL4040. Add
> compatibles for cm36672p and cm36686, with a fallback for cm36686 of
> vcnl4040.
> 
> Signed-off-by: Erikas Bitovtas <xerikasxx@gmail.com>
> ---
>  .../devicetree/bindings/iio/light/vishay,vcnl4000.yaml  | 17 +++++++++++------
>  1 file changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml b/Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml
> index 4d1a225e8868..2ba4d5de4ec4 100644
> --- a/Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml
> +++ b/Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml
> @@ -18,12 +18,17 @@ allOf:
>  
>  properties:
>    compatible:
> -    enum:
> -      - vishay,vcnl4000
> -      - vishay,vcnl4010
> -      - vishay,vcnl4020
> -      - vishay,vcnl4040
> -      - vishay,vcnl4200
> +    oneOf:
> +      - enum:
> +          - capella,cm36672p

CM36672P is compatible with CM36686, but this is not expressed.
Confusing commit msg and code.

> +          - vishay,vcnl4000
> +          - vishay,vcnl4010
> +          - vishay,vcnl4020
> +          - vishay,vcnl4040

Best regards,
Krzysztof

Re: [PATCH v4 1/2] dt-bindings: iio: light: vcnl4000: add Capella CM36686 and CM36672P

[Erikas Bitovtas]

On 2/13/26 9:54 AM, Krzysztof Kozlowski wrote:
> On Thu, Feb 12, 2026 at 04:42:47PM +0200, Erikas Bitovtas wrote:
>> Capella CM36686 is an ambient light and proximity sensor developed by
>> Capella Microsystems, now a subsidiary of Vishay Intertechnology Inc. It
>> has an I2C address of 0x60 and is fully compatible with an existing
>> driver for VCNL4040.
> 
> I wonder why and how...

VCNL4040 shares the same digital interface as CM36686. All the registers
and their fields are the same. It is most likely Vishay just reused the
CM36686 design for VCNL4040.
> 
>> Capella CM36672P is a proximity-only sensor that
>> is fully compatible with CM36686, and therefore with VCNL4040. Add
>> compatibles for cm36672p and cm36686, with a fallback for cm36686 of
>> vcnl4040.
>>
>> Signed-off-by: Erikas Bitovtas <xerikasxx@gmail.com>
>> ---
>>  .../devicetree/bindings/iio/light/vishay,vcnl4000.yaml  | 17 +++++++++++------
>>  1 file changed, 11 insertions(+), 6 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml b/Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml
>> index 4d1a225e8868..2ba4d5de4ec4 100644
>> --- a/Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml
>> +++ b/Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml
>> @@ -18,12 +18,17 @@ allOf:
>>  
>>  properties:
>>    compatible:
>> -    enum:
>> -      - vishay,vcnl4000
>> -      - vishay,vcnl4010
>> -      - vishay,vcnl4020
>> -      - vishay,vcnl4040
>> -      - vishay,vcnl4200
>> +    oneOf:
>> +      - enum:
>> +          - capella,cm36672p
> 
> CM36672P is compatible with CM36686, but this is not expressed.
> Confusing commit msg and code. 

For CM36672P we create a dedicated compatible because it is a
proximity-only sensor which has the same proximity sensor configuration,
but ambient light sensor registers are missing (reserved).
>> +          - vishay,vcnl4000
>> +          - vishay,vcnl4010
>> +          - vishay,vcnl4020
>> +          - vishay,vcnl4040
> 
> Best regards,
> Krzysztof
> 

Re: [PATCH v4 1/2] dt-bindings: iio: light: vcnl4000: add Capella CM36686 and CM36672P

[Krzysztof Kozlowski]

On 13/02/2026 09:29, Erikas Bitovtas wrote:
>>> Signed-off-by: Erikas Bitovtas <xerikasxx@gmail.com>
>>> ---
>>>  .../devicetree/bindings/iio/light/vishay,vcnl4000.yaml  | 17 +++++++++++------
>>>  1 file changed, 11 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml b/Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml
>>> index 4d1a225e8868..2ba4d5de4ec4 100644
>>> --- a/Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml
>>> +++ b/Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml
>>> @@ -18,12 +18,17 @@ allOf:
>>>  
>>>  properties:
>>>    compatible:
>>> -    enum:
>>> -      - vishay,vcnl4000
>>> -      - vishay,vcnl4010
>>> -      - vishay,vcnl4020
>>> -      - vishay,vcnl4040
>>> -      - vishay,vcnl4200
>>> +    oneOf:
>>> +      - enum:
>>> +          - capella,cm36672p
>>
>> CM36672P is compatible with CM36686, but this is not expressed.
>> Confusing commit msg and code. 
> 
> For CM36672P we create a dedicated compatible because it is a
> proximity-only sensor which has the same proximity sensor configuration,
> but ambient light sensor registers are missing (reserved).

I don't understand this. You just wrote "fully compatible with CM36686"
and now you imply that not.

Decide.

Best regards,
Krzysztof

Re: [PATCH v4 1/2] dt-bindings: iio: light: vcnl4000: add Capella CM36686 and CM36672P

[Erikas Bitovtas]

On 2/13/26 10:51 AM, Krzysztof Kozlowski wrote:
> On 13/02/2026 09:29, Erikas Bitovtas wrote:
>>>> Signed-off-by: Erikas Bitovtas <xerikasxx@gmail.com>
>>>> ---
>>>>  .../devicetree/bindings/iio/light/vishay,vcnl4000.yaml  | 17 +++++++++++------
>>>>  1 file changed, 11 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml b/Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml
>>>> index 4d1a225e8868..2ba4d5de4ec4 100644
>>>> --- a/Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml
>>>> +++ b/Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml
>>>> @@ -18,12 +18,17 @@ allOf:
>>>>  
>>>>  properties:
>>>>    compatible:
>>>> -    enum:
>>>> -      - vishay,vcnl4000
>>>> -      - vishay,vcnl4010
>>>> -      - vishay,vcnl4020
>>>> -      - vishay,vcnl4040
>>>> -      - vishay,vcnl4200
>>>> +    oneOf:
>>>> +      - enum:
>>>> +          - capella,cm36672p
>>>
>>> CM36672P is compatible with CM36686, but this is not expressed.
>>> Confusing commit msg and code. 
>>
>> For CM36672P we create a dedicated compatible because it is a
>> proximity-only sensor which has the same proximity sensor configuration,
>> but ambient light sensor registers are missing (reserved).
> 
> I don't understand this. You just wrote "fully compatible with CM36686"
> and now you imply that not.
> 
> Decide.
> 
It is not. CM36672P supports only a subset of CM36686 features, in
particular the proximity sensor. That is what I meant initially.
I am sorry if the previous phrasing caused any confusion.
> Best regards,
> Krzysztof

Re: [PATCH v4 1/2] dt-bindings: iio: light: vcnl4000: add Capella CM36686 and CM36672P

[David Lechner]

On 2/13/26 2:56 AM, Erikas Bitovtas wrote:
> 
> 
> On 2/13/26 10:51 AM, Krzysztof Kozlowski wrote:
>> On 13/02/2026 09:29, Erikas Bitovtas wrote:
>>>>> Signed-off-by: Erikas Bitovtas <xerikasxx@gmail.com>
>>>>> ---
>>>>>  .../devicetree/bindings/iio/light/vishay,vcnl4000.yaml  | 17 +++++++++++------
>>>>>  1 file changed, 11 insertions(+), 6 deletions(-)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml b/Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml
>>>>> index 4d1a225e8868..2ba4d5de4ec4 100644
>>>>> --- a/Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml
>>>>> +++ b/Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml
>>>>> @@ -18,12 +18,17 @@ allOf:
>>>>>  
>>>>>  properties:
>>>>>    compatible:
>>>>> -    enum:
>>>>> -      - vishay,vcnl4000
>>>>> -      - vishay,vcnl4010
>>>>> -      - vishay,vcnl4020
>>>>> -      - vishay,vcnl4040
>>>>> -      - vishay,vcnl4200
>>>>> +    oneOf:
>>>>> +      - enum:
>>>>> +          - capella,cm36672p
>>>>
>>>> CM36672P is compatible with CM36686, but this is not expressed.
>>>> Confusing commit msg and code. 
>>>
>>> For CM36672P we create a dedicated compatible because it is a
>>> proximity-only sensor which has the same proximity sensor configuration,
>>> but ambient light sensor registers are missing (reserved).
>>
>> I don't understand this. You just wrote "fully compatible with CM36686"
>> and now you imply that not.
>>
>> Decide.
>>
> It is not. CM36672P supports only a subset of CM36686 features, in
> particular the proximity sensor. That is what I meant initially.
> I am sorry if the previous phrasing caused any confusion.

But CM36686 is fully compatible with CM36672P, right?

So this would make sense?

      - items:
          - const: capella,cm36686
          - const: vishay,vcnl4040
          - const: capella,cm36686p

Re: [PATCH v4 2/2] iio: light: vcnl4000: add support for Capella CM36686 and CM36672P

[Jonathan Cameron]

On Thu, 12 Feb 2026 16:42:48 +0200
Erikas Bitovtas <xerikasxx@gmail.com> wrote:

> Add support for Capella's CM36686 and CM36672P sensors. Capella
> CM36686 is an ambient light and proximity sensor that is fully
> compatible with VCNL4040 and can be used as is. For CM36672P, which is
> a proximity-only sensor, also remove the IIO_LIGHT channel.
> 
> Signed-off-by: Erikas Bitovtas <xerikasxx@gmail.com>
Hi Erikas,

The following isn't necessarily suggesting a change, more something that might cause
people to have a bit of a surprise.

Whilst the binding having a fallback compatible is the right thing to do
here, I wonder if you care that that when you use the drive via the fallback
that the reported device name ends up not matching with the part you have (cm36686)?

Sometimes we add specific chip_info for a part just to resolve that naming
issue even if it's otherwise fully compatible.

That means that an older kernel will support the device (via the fallback)
but a newer kernel will also give it the name people might expect.

It is an interesting corner case on whether that is preferable given it
is an ABI change as they upgrade their kernel, but the name then ends up being
what they want.  There isn't really a right answer to this.

One note below.

> ---
>  drivers/iio/light/vcnl4000.c | 40 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
> 
> diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
> index a36c23813679..1f8f4e4586f4 100644
> --- a/drivers/iio/light/vcnl4000.c
> +++ b/drivers/iio/light/vcnl4000.c
> @@ -185,6 +185,7 @@ static const int vcnl4040_ps_oversampling_ratio[] = {1, 2, 4, 8};
>  #define VCNL4000_SLEEP_DELAY_MS	2000 /* before we enter pm_runtime_suspend */
>  
>  enum vcnl4000_device_ids {
> +	CM36672P,
>  	VCNL4000,
>  	VCNL4010,
>  	VCNL4040,
> @@ -235,6 +236,8 @@ struct vcnl4000_chip_spec {
>  };
>  
>  static const struct i2c_device_id vcnl4000_id[] = {
> +	{ "cm36672p", CM36672P },
> +	{ "cm36686", VCNL4040 },
>  	{ "vcnl4000", VCNL4000 },
>  	{ "vcnl4010", VCNL4010 },
>  	{ "vcnl4020", VCNL4010 },
> @@ -1842,6 +1845,22 @@ static const struct iio_chan_spec vcnl4040_channels[] = {
>  	}
>  };

...

>  	[VCNL4000] = {
>  		.prod = "VCNL4000",
>  		.init = vcnl4000_init,
> @@ -2033,6 +2065,14 @@ static int vcnl4000_probe(struct i2c_client *client)
>  }
>  
>  static const struct of_device_id vcnl_4000_of_match[] = {
> +	{
> +		.compatible = "capella,cm36672p",
> +		.data = (void *)CM36672P,
> +	},
> +	{
> +		.compatible = "capella,cm36686",
> +		.data = (void *)VCNL4040,

Is this necessary? I 'think' if you drop it we'll match instead
on the vcnl4040 fallback and then the access to the data will be
through the stripped name only bit of the compatible (first entry, not
the fallback so cm36686 in this case). So you do need the cm36686
entry in the i2c_device_id table above. Probably better to keep
this here to avoid having to reason this out - but perhaps a
comment to that affect would be useful (assuming you verify my
reasoning).

As Andy suggested moving away from enum values an towards
direct pointers to the chip_info structures + drop the
i2c_client_get_device_id() in favour of i2c_get_match_data() which
uses the right firmware entry to get the data in all cases is the
right long term solution and avoids an association being necessary
between the two tables.

Jonathan




> +	},
>  	{
>  		.compatible = "vishay,vcnl4000",
>  		.data = (void *)VCNL4000,
> 

Re: [PATCH v4 1/2] dt-bindings: iio: light: vcnl4000: add Capella CM36686 and CM36672P

[Erikas Bitovtas]

On 2/14/26 6:44 PM, David Lechner wrote:
> On 2/13/26 2:56 AM, Erikas Bitovtas wrote:
>>
>>
>> On 2/13/26 10:51 AM, Krzysztof Kozlowski wrote:
>>> On 13/02/2026 09:29, Erikas Bitovtas wrote:
>>>>>> Signed-off-by: Erikas Bitovtas <xerikasxx@gmail.com>
>>>>>> ---
>>>>>>  .../devicetree/bindings/iio/light/vishay,vcnl4000.yaml  | 17 +++++++++++------
>>>>>>  1 file changed, 11 insertions(+), 6 deletions(-)
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml b/Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml
>>>>>> index 4d1a225e8868..2ba4d5de4ec4 100644
>>>>>> --- a/Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml
>>>>>> +++ b/Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml
>>>>>> @@ -18,12 +18,17 @@ allOf:
>>>>>>  
>>>>>>  properties:
>>>>>>    compatible:
>>>>>> -    enum:
>>>>>> -      - vishay,vcnl4000
>>>>>> -      - vishay,vcnl4010
>>>>>> -      - vishay,vcnl4020
>>>>>> -      - vishay,vcnl4040
>>>>>> -      - vishay,vcnl4200
>>>>>> +    oneOf:
>>>>>> +      - enum:
>>>>>> +          - capella,cm36672p
>>>>>
>>>>> CM36672P is compatible with CM36686, but this is not expressed.
>>>>> Confusing commit msg and code. 
>>>>
>>>> For CM36672P we create a dedicated compatible because it is a
>>>> proximity-only sensor which has the same proximity sensor configuration,
>>>> but ambient light sensor registers are missing (reserved).
>>>
>>> I don't understand this. You just wrote "fully compatible with CM36686"
>>> and now you imply that not.
>>>
>>> Decide.
>>>
>> It is not. CM36672P supports only a subset of CM36686 features, in
>> particular the proximity sensor. That is what I meant initially.
>> I am sorry if the previous phrasing caused any confusion.
> 
> But CM36686 is fully compatible with CM36672P, right?
> 
> So this would make sense?
> 
>       - items:
>           - const: capella,cm36686
>           - const: vishay,vcnl4040
>           - const: capella,cm36686p
> 
> 
If you try to use CM36686 compatible for CM36672P, proximity channels
will work, but in_illuminance_raw will return 0 and changing illuminance
parameters will have no effect. That is because CM36672P is a proximity
sensor only and the register fields for ambient light are reserved.
And if you try to use CM36672P compatible with CM36686, it will work,
but only proximity channel will be available, even though CM36686 also
can sense light.

Re: [PATCH v4 2/2] iio: light: vcnl4000: add support for Capella CM36686 and CM36672P

[Erikas Bitovtas]

On 2/14/26 8:09 PM, Jonathan Cameron wrote:
>> ---
>>  drivers/iio/light/vcnl4000.c | 40 ++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 40 insertions(+)
>>
>> diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
>> index a36c23813679..1f8f4e4586f4 100644
>> --- a/drivers/iio/light/vcnl4000.c
>> +++ b/drivers/iio/light/vcnl4000.c
>> @@ -185,6 +185,7 @@ static const int vcnl4040_ps_oversampling_ratio[] = {1, 2, 4, 8};
>>  #define VCNL4000_SLEEP_DELAY_MS	2000 /* before we enter pm_runtime_suspend */
>>  
>>  enum vcnl4000_device_ids {
>> +	CM36672P,
>>  	VCNL4000,
>>  	VCNL4010,
>>  	VCNL4040,
>> @@ -235,6 +236,8 @@ struct vcnl4000_chip_spec {
>>  };
>>  
>>  static const struct i2c_device_id vcnl4000_id[] = {
>> +	{ "cm36672p", CM36672P },
>> +	{ "cm36686", VCNL4040 },
>>  	{ "vcnl4000", VCNL4000 },
>>  	{ "vcnl4010", VCNL4010 },
>>  	{ "vcnl4020", VCNL4010 },
>> @@ -1842,6 +1845,22 @@ static const struct iio_chan_spec vcnl4040_channels[] = {
>>  	}
>>  };
> 
> ...
> 
>>  	[VCNL4000] = {
>>  		.prod = "VCNL4000",
>>  		.init = vcnl4000_init,
>> @@ -2033,6 +2065,14 @@ static int vcnl4000_probe(struct i2c_client *client)
>>  }
>>  
>>  static const struct of_device_id vcnl_4000_of_match[] = {
>> +	{
>> +		.compatible = "capella,cm36672p",
>> +		.data = (void *)CM36672P,
>> +	},
>> +	{
>> +		.compatible = "capella,cm36686",
>> +		.data = (void *)VCNL4040,
> 
> Is this necessary? I 'think' if you drop it we'll match instead
> on the vcnl4040 fallback and then the access to the data will be
> through the stripped name only bit of the compatible (first entry, not
> the fallback so cm36686 in this case). So you do need the cm36686
> entry in the i2c_device_id table above. Probably better to keep
> this here to avoid having to reason this out - but perhaps a
> comment to that affect would be useful (assuming you verify my
> reasoning).
>
After I removed the entry for "capella,cm36686", I received the "Unable
to handle kernel NULL pointer dereference" error in dmesg. And at least
stk3310 driver includes a compatible entry both for the device (stk3013)
and for the fallback (stk3310). So my assumption is that this entry is
needed.
I could include a comment explaining that cm36686 is fully compatible
with vcnl4040, however, if that is necessary.
> As Andy suggested moving away from enum values an towards
> direct pointers to the chip_info structures + drop the
> i2c_client_get_device_id() in favour of i2c_get_match_data() which
> uses the right firmware entry to get the data in all cases is the
> right long term solution and avoids an association being necessary
> between the two tables.
> 
> Jonathan
> 
> 
> 
> 
>> +	},
>>  	{
>>  		.compatible = "vishay,vcnl4000",
>>  		.data = (void *)VCNL4000,
>>
> 

Re: [PATCH v4 1/2] dt-bindings: iio: light: vcnl4000: add Capella CM36686 and CM36672P

[Jonathan Cameron]

On Sat, 14 Feb 2026 10:44:23 -0600
David Lechner <dlechner@baylibre.com> wrote:

> On 2/13/26 2:56 AM, Erikas Bitovtas wrote:
> > 
> > 
> > On 2/13/26 10:51 AM, Krzysztof Kozlowski wrote:  
> >> On 13/02/2026 09:29, Erikas Bitovtas wrote:  
> >>>>> Signed-off-by: Erikas Bitovtas <xerikasxx@gmail.com>
> >>>>> ---
> >>>>>  .../devicetree/bindings/iio/light/vishay,vcnl4000.yaml  | 17 +++++++++++------
> >>>>>  1 file changed, 11 insertions(+), 6 deletions(-)
> >>>>>
> >>>>> diff --git a/Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml b/Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml
> >>>>> index 4d1a225e8868..2ba4d5de4ec4 100644
> >>>>> --- a/Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml
> >>>>> +++ b/Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml
> >>>>> @@ -18,12 +18,17 @@ allOf:
> >>>>>  
> >>>>>  properties:
> >>>>>    compatible:
> >>>>> -    enum:
> >>>>> -      - vishay,vcnl4000
> >>>>> -      - vishay,vcnl4010
> >>>>> -      - vishay,vcnl4020
> >>>>> -      - vishay,vcnl4040
> >>>>> -      - vishay,vcnl4200
> >>>>> +    oneOf:
> >>>>> +      - enum:
> >>>>> +          - capella,cm36672p  
> >>>>
> >>>> CM36672P is compatible with CM36686, but this is not expressed.
> >>>> Confusing commit msg and code.   
> >>>
> >>> For CM36672P we create a dedicated compatible because it is a
> >>> proximity-only sensor which has the same proximity sensor configuration,
> >>> but ambient light sensor registers are missing (reserved).  
> >>
> >> I don't understand this. You just wrote "fully compatible with CM36686"
> >> and now you imply that not.
> >>
> >> Decide.
> >>  
> > It is not. CM36672P supports only a subset of CM36686 features, in
> > particular the proximity sensor. That is what I meant initially.
> > I am sorry if the previous phrasing caused any confusion.  
> 
> But CM36686 is fully compatible with CM36672P, right?

I'd be clear in this discussion that the P version is a subset.
So it's very much one way compatibility (your ordering below reflects
that right)

> 
> So this would make sense?
> 
>       - items:
>           - const: capella,cm36686
>           - const: vishay,vcnl4040
>           - const: capella,cm36686p

I'm not sure we can do that now given we'd also need the option
of vcnl4040 falling back to cm36686p for it to feel logical and
retrofitting fallbacks is a bit odd.

Jonathan



> 
> 

Re: [PATCH v4 1/2] dt-bindings: iio: light: vcnl4000: add Capella CM36686 and CM36672P

[Erikas Bitovtas]

On 2/15/26 7:49 PM, Jonathan Cameron wrote:
> On Sat, 14 Feb 2026 10:44:23 -0600
> David Lechner <dlechner@baylibre.com> wrote:
> 
>> On 2/13/26 2:56 AM, Erikas Bitovtas wrote:
>>>
>>>
>>> On 2/13/26 10:51 AM, Krzysztof Kozlowski wrote:  
>>>> On 13/02/2026 09:29, Erikas Bitovtas wrote:  
>>>>>>> Signed-off-by: Erikas Bitovtas <xerikasxx@gmail.com>
>>>>>>> ---
>>>>>>>  .../devicetree/bindings/iio/light/vishay,vcnl4000.yaml  | 17 +++++++++++------
>>>>>>>  1 file changed, 11 insertions(+), 6 deletions(-)
>>>>>>>
>>>>>>> diff --git a/Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml b/Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml
>>>>>>> index 4d1a225e8868..2ba4d5de4ec4 100644
>>>>>>> --- a/Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml
>>>>>>> +++ b/Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml
>>>>>>> @@ -18,12 +18,17 @@ allOf:
>>>>>>>  
>>>>>>>  properties:
>>>>>>>    compatible:
>>>>>>> -    enum:
>>>>>>> -      - vishay,vcnl4000
>>>>>>> -      - vishay,vcnl4010
>>>>>>> -      - vishay,vcnl4020
>>>>>>> -      - vishay,vcnl4040
>>>>>>> -      - vishay,vcnl4200
>>>>>>> +    oneOf:
>>>>>>> +      - enum:
>>>>>>> +          - capella,cm36672p  
>>>>>>
>>>>>> CM36672P is compatible with CM36686, but this is not expressed.
>>>>>> Confusing commit msg and code.   
>>>>>
>>>>> For CM36672P we create a dedicated compatible because it is a
>>>>> proximity-only sensor which has the same proximity sensor configuration,
>>>>> but ambient light sensor registers are missing (reserved).  
>>>>
>>>> I don't understand this. You just wrote "fully compatible with CM36686"
>>>> and now you imply that not.
>>>>
>>>> Decide.
>>>>  
>>> It is not. CM36672P supports only a subset of CM36686 features, in
>>> particular the proximity sensor. That is what I meant initially.
>>> I am sorry if the previous phrasing caused any confusion.  
>>
>> But CM36686 is fully compatible with CM36672P, right?
> 
> I'd be clear in this discussion that the P version is a subset.
> So it's very much one way compatibility (your ordering below reflects
> that right)
> 
As I said, only proximity register fields are compatible between
CM36672P and CM36686. CM36672P lacks ambient light sensing capabilities.
I am not sure if CM36672P should fall back to VCNL4040, or the other way
around.
>>
>> So this would make sense?
>>
>>       - items:
>>           - const: capella,cm36686
>>           - const: vishay,vcnl4040
>>           - const: capella,cm36686p
> 
> I'm not sure we can do that now given we'd also need the option
> of vcnl4040 falling back to cm36686p for it to feel logical and
> retrofitting fallbacks is a bit odd.
> 
> Jonathan
> 
To clarify, there is no such device as CM36686P. I suppose this is
supposed to be CM36672P here?

Re: [PATCH v4 2/2] iio: light: vcnl4000: add support for Capella CM36686 and CM36672P

[Jonathan Cameron]

On Sun, 15 Feb 2026 19:28:56 +0200
Erikas Bitovtas <xerikasxx@gmail.com> wrote:

> On 2/14/26 8:09 PM, Jonathan Cameron wrote:
> >> ---
> >>  drivers/iio/light/vcnl4000.c | 40 ++++++++++++++++++++++++++++++++++++++++
> >>  1 file changed, 40 insertions(+)
> >>
> >> diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
> >> index a36c23813679..1f8f4e4586f4 100644
> >> --- a/drivers/iio/light/vcnl4000.c
> >> +++ b/drivers/iio/light/vcnl4000.c
> >> @@ -185,6 +185,7 @@ static const int vcnl4040_ps_oversampling_ratio[] = {1, 2, 4, 8};
> >>  #define VCNL4000_SLEEP_DELAY_MS	2000 /* before we enter pm_runtime_suspend */
> >>  
> >>  enum vcnl4000_device_ids {
> >> +	CM36672P,
> >>  	VCNL4000,
> >>  	VCNL4010,
> >>  	VCNL4040,
> >> @@ -235,6 +236,8 @@ struct vcnl4000_chip_spec {
> >>  };
> >>  
> >>  static const struct i2c_device_id vcnl4000_id[] = {
> >> +	{ "cm36672p", CM36672P },
> >> +	{ "cm36686", VCNL4040 },
> >>  	{ "vcnl4000", VCNL4000 },
> >>  	{ "vcnl4010", VCNL4010 },
> >>  	{ "vcnl4020", VCNL4010 },
> >> @@ -1842,6 +1845,22 @@ static const struct iio_chan_spec vcnl4040_channels[] = {
> >>  	}
> >>  };  
> > 
> > ...
> >   
> >>  	[VCNL4000] = {
> >>  		.prod = "VCNL4000",
> >>  		.init = vcnl4000_init,
> >> @@ -2033,6 +2065,14 @@ static int vcnl4000_probe(struct i2c_client *client)
> >>  }
> >>  
> >>  static const struct of_device_id vcnl_4000_of_match[] = {
> >> +	{
> >> +		.compatible = "capella,cm36672p",
> >> +		.data = (void *)CM36672P,
> >> +	},
> >> +	{
> >> +		.compatible = "capella,cm36686",
> >> +		.data = (void *)VCNL4040,  
> > 
> > Is this necessary? I 'think' if you drop it we'll match instead
> > on the vcnl4040 fallback and then the access to the data will be
> > through the stripped name only bit of the compatible (first entry, not
> > the fallback so cm36686 in this case). So you do need the cm36686
> > entry in the i2c_device_id table above. Probably better to keep
> > this here to avoid having to reason this out - but perhaps a
> > comment to that affect would be useful (assuming you verify my
> > reasoning).
> >  
> After I removed the entry for "capella,cm36686", I received the "Unable
> to handle kernel NULL pointer dereference" error in dmesg. And at least
> stk3310 driver includes a compatible entry both for the device (stk3013)
> and for the fallback (stk3310). So my assumption is that this entry is
> needed.
> I could include a comment explaining that cm36686 is fully compatible
> with vcnl4040, however, if that is necessary.

Thanks for checking.

What did you get as the backtrace?  I'm hoping it'll explain what I'm
misunderstanding!  The hacks around using the wrong table for compatible
matches have tripped me up before.

Jonathan

> > As Andy suggested moving away from enum values an towards
> > direct pointers to the chip_info structures + drop the
> > i2c_client_get_device_id() in favour of i2c_get_match_data() which
> > uses the right firmware entry to get the data in all cases is the
> > right long term solution and avoids an association being necessary
> > between the two tables.
> > 
> > Jonathan
> > 
> > 
> > 
> >   
> >> +	},
> >>  	{
> >>  		.compatible = "vishay,vcnl4000",
> >>  		.data = (void *)VCNL4000,
> >>  
> >   
> 

Re: [PATCH v4 1/2] dt-bindings: iio: light: vcnl4000: add Capella CM36686 and CM36672P

[Jonathan Cameron]

On Sun, 15 Feb 2026 18:16:45 +0200
Erikas Bitovtas <xerikasxx@gmail.com> wrote:

> On 2/14/26 6:44 PM, David Lechner wrote:
> > On 2/13/26 2:56 AM, Erikas Bitovtas wrote:  
> >>
> >>
> >> On 2/13/26 10:51 AM, Krzysztof Kozlowski wrote:  
> >>> On 13/02/2026 09:29, Erikas Bitovtas wrote:  
> >>>>>> Signed-off-by: Erikas Bitovtas <xerikasxx@gmail.com>
> >>>>>> ---
> >>>>>>  .../devicetree/bindings/iio/light/vishay,vcnl4000.yaml  | 17 +++++++++++------
> >>>>>>  1 file changed, 11 insertions(+), 6 deletions(-)
> >>>>>>
> >>>>>> diff --git a/Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml b/Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml
> >>>>>> index 4d1a225e8868..2ba4d5de4ec4 100644
> >>>>>> --- a/Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml
> >>>>>> +++ b/Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml
> >>>>>> @@ -18,12 +18,17 @@ allOf:
> >>>>>>  
> >>>>>>  properties:
> >>>>>>    compatible:
> >>>>>> -    enum:
> >>>>>> -      - vishay,vcnl4000
> >>>>>> -      - vishay,vcnl4010
> >>>>>> -      - vishay,vcnl4020
> >>>>>> -      - vishay,vcnl4040
> >>>>>> -      - vishay,vcnl4200
> >>>>>> +    oneOf:
> >>>>>> +      - enum:
> >>>>>> +          - capella,cm36672p  
> >>>>>
> >>>>> CM36672P is compatible with CM36686, but this is not expressed.
> >>>>> Confusing commit msg and code.   
> >>>>
> >>>> For CM36672P we create a dedicated compatible because it is a
> >>>> proximity-only sensor which has the same proximity sensor configuration,
> >>>> but ambient light sensor registers are missing (reserved).  
> >>>
> >>> I don't understand this. You just wrote "fully compatible with CM36686"
> >>> and now you imply that not.
> >>>
> >>> Decide.
> >>>  
> >> It is not. CM36672P supports only a subset of CM36686 features, in
> >> particular the proximity sensor. That is what I meant initially.
> >> I am sorry if the previous phrasing caused any confusion.  
> > 
> > But CM36686 is fully compatible with CM36672P, right?
> > 
> > So this would make sense?
> > 
> >       - items:
> >           - const: capella,cm36686
> >           - const: vishay,vcnl4040
> >           - const: capella,cm36686p
> > 
> >   
> If you try to use CM36686 compatible for CM36672P, proximity channels
> will work, but in_illuminance_raw will return 0 and changing illuminance
> parameters will have no effect. 

Look at the ordering above.  The key I think is it's not saying the cm36672p can
fallback to the cm36686, but the other way around.

If that fallback was used (because we'd actually had the driver evolve in
a different order and older versions only supported the cm36672p) then we'd
see a proximity only device presented with the ambient light parts hidden
away. 

> That is because CM36672P is a proximity
> sensor only and the register fields for ambient light are reserved.
> And if you try to use CM36672P compatible with CM36686, it will work,
> but only proximity channel will be available, even though CM36686 also
> can sense light.
Understood.  So in one direction it is correctly considered backwards compatible
but not the other way round.  The ambient light can be thought of as 'value add'
new features on a 'newer' device (obviously that's not actually the case but
it's an easier way to think about how fallback compatibles are often used).

Jonathan

> 

Re: [PATCH v4 1/2] dt-bindings: iio: light: vcnl4000: add Capella CM36686 and CM36672P

[Jonathan Cameron]

On Sun, 15 Feb 2026 20:00:52 +0200
Erikas Bitovtas <xerikasxx@gmail.com> wrote:

> On 2/15/26 7:49 PM, Jonathan Cameron wrote:
> > On Sat, 14 Feb 2026 10:44:23 -0600
> > David Lechner <dlechner@baylibre.com> wrote:
> >   
> >> On 2/13/26 2:56 AM, Erikas Bitovtas wrote:  
> >>>
> >>>
> >>> On 2/13/26 10:51 AM, Krzysztof Kozlowski wrote:    
> >>>> On 13/02/2026 09:29, Erikas Bitovtas wrote:    
> >>>>>>> Signed-off-by: Erikas Bitovtas <xerikasxx@gmail.com>
> >>>>>>> ---
> >>>>>>>  .../devicetree/bindings/iio/light/vishay,vcnl4000.yaml  | 17 +++++++++++------
> >>>>>>>  1 file changed, 11 insertions(+), 6 deletions(-)
> >>>>>>>
> >>>>>>> diff --git a/Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml b/Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml
> >>>>>>> index 4d1a225e8868..2ba4d5de4ec4 100644
> >>>>>>> --- a/Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml
> >>>>>>> +++ b/Documentation/devicetree/bindings/iio/light/vishay,vcnl4000.yaml
> >>>>>>> @@ -18,12 +18,17 @@ allOf:
> >>>>>>>  
> >>>>>>>  properties:
> >>>>>>>    compatible:
> >>>>>>> -    enum:
> >>>>>>> -      - vishay,vcnl4000
> >>>>>>> -      - vishay,vcnl4010
> >>>>>>> -      - vishay,vcnl4020
> >>>>>>> -      - vishay,vcnl4040
> >>>>>>> -      - vishay,vcnl4200
> >>>>>>> +    oneOf:
> >>>>>>> +      - enum:
> >>>>>>> +          - capella,cm36672p    
> >>>>>>
> >>>>>> CM36672P is compatible with CM36686, but this is not expressed.
> >>>>>> Confusing commit msg and code.     
> >>>>>
> >>>>> For CM36672P we create a dedicated compatible because it is a
> >>>>> proximity-only sensor which has the same proximity sensor configuration,
> >>>>> but ambient light sensor registers are missing (reserved).    
> >>>>
> >>>> I don't understand this. You just wrote "fully compatible with CM36686"
> >>>> and now you imply that not.
> >>>>
> >>>> Decide.
> >>>>    
> >>> It is not. CM36672P supports only a subset of CM36686 features, in
> >>> particular the proximity sensor. That is what I meant initially.
> >>> I am sorry if the previous phrasing caused any confusion.    
> >>
> >> But CM36686 is fully compatible with CM36672P, right?  
> > 
> > I'd be clear in this discussion that the P version is a subset.
> > So it's very much one way compatibility (your ordering below reflects
> > that right)
> >   
> As I said, only proximity register fields are compatible between
> CM36672P and CM36686. CM36672P lacks ambient light sensing capabilities.
> I am not sure if CM36672P should fall back to VCNL4040, or the other way
> around.

Absolutely could have the vcnl4040 fall back the cm36672p as it would
make a full functioning proximity sensor. Other way around is definitely
not possible as you have noted as the ambient light parts would simply
not work, which is not something we can consider compatible.

I just don't think it makes sense now given the evolution of the binding.

> >>
> >> So this would make sense?
> >>
> >>       - items:
> >>           - const: capella,cm36686
> >>           - const: vishay,vcnl4040
> >>           - const: capella,cm36686p  
> > 
> > I'm not sure we can do that now given we'd also need the option
> > of vcnl4040 falling back to cm36686p for it to feel logical and
> > retrofitting fallbacks is a bit odd.
> > 
> > Jonathan
> >   
> To clarify, there is no such device as CM36686P. I suppose this is
> supposed to be CM36672P here?
Yes. I just didn't check David's list. We only really care about the P
bit meaning proximity only for this discussion.

Thanks,

Jonathan

Re: [PATCH v4 2/2] iio: light: vcnl4000: add support for Capella CM36686 and CM36672P

[Erikas Bitovtas]

On 2/15/26 9:31 PM, Jonathan Cameron wrote:
> On Sun, 15 Feb 2026 19:28:56 +0200
> Erikas Bitovtas <xerikasxx@gmail.com> wrote:
> 
>> On 2/14/26 8:09 PM, Jonathan Cameron wrote:
>>>> ---
>>>>  drivers/iio/light/vcnl4000.c | 40 ++++++++++++++++++++++++++++++++++++++++
>>>>  1 file changed, 40 insertions(+)
>>>>
>>>> diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
>>>> index a36c23813679..1f8f4e4586f4 100644
>>>> --- a/drivers/iio/light/vcnl4000.c
>>>> +++ b/drivers/iio/light/vcnl4000.c
>>>> @@ -185,6 +185,7 @@ static const int vcnl4040_ps_oversampling_ratio[] = {1, 2, 4, 8};
>>>>  #define VCNL4000_SLEEP_DELAY_MS	2000 /* before we enter pm_runtime_suspend */
>>>>  
>>>>  enum vcnl4000_device_ids {
>>>> +	CM36672P,
>>>>  	VCNL4000,
>>>>  	VCNL4010,
>>>>  	VCNL4040,
>>>> @@ -235,6 +236,8 @@ struct vcnl4000_chip_spec {
>>>>  };
>>>>  
>>>>  static const struct i2c_device_id vcnl4000_id[] = {
>>>> +	{ "cm36672p", CM36672P },
>>>> +	{ "cm36686", VCNL4040 },
>>>>  	{ "vcnl4000", VCNL4000 },
>>>>  	{ "vcnl4010", VCNL4010 },
>>>>  	{ "vcnl4020", VCNL4010 },
>>>> @@ -1842,6 +1845,22 @@ static const struct iio_chan_spec vcnl4040_channels[] = {
>>>>  	}
>>>>  };  
>>>
>>> ...
>>>   
>>>>  	[VCNL4000] = {
>>>>  		.prod = "VCNL4000",
>>>>  		.init = vcnl4000_init,
>>>> @@ -2033,6 +2065,14 @@ static int vcnl4000_probe(struct i2c_client *client)
>>>>  }
>>>>  
>>>>  static const struct of_device_id vcnl_4000_of_match[] = {
>>>> +	{
>>>> +		.compatible = "capella,cm36672p",
>>>> +		.data = (void *)CM36672P,
>>>> +	},
>>>> +	{
>>>> +		.compatible = "capella,cm36686",
>>>> +		.data = (void *)VCNL4040,  
>>>
>>> Is this necessary? I 'think' if you drop it we'll match instead
>>> on the vcnl4040 fallback and then the access to the data will be
>>> through the stripped name only bit of the compatible (first entry, not
>>> the fallback so cm36686 in this case). So you do need the cm36686
>>> entry in the i2c_device_id table above. Probably better to keep
>>> this here to avoid having to reason this out - but perhaps a
>>> comment to that affect would be useful (assuming you verify my
>>> reasoning).
>>>  
>> After I removed the entry for "capella,cm36686", I received the "Unable
>> to handle kernel NULL pointer dereference" error in dmesg. And at least
>> stk3310 driver includes a compatible entry both for the device (stk3013)
>> and for the fallback (stk3310). So my assumption is that this entry is
>> needed.
>> I could include a comment explaining that cm36686 is fully compatible
>> with vcnl4040, however, if that is necessary.
> 
> Thanks for checking.
> 
> What did you get as the backtrace?  I'm hoping it'll explain what I'm
> misunderstanding!  The hacks around using the wrong table for compatible
> matches have tripped me up before.
> 
> Jonathan
> 

I am attaching a link to the dmesg. There were quite a lot of lines in
the stack trace and I am not sure what is the right way to post logs in
the mailing list.

https://pastebin.com/QgeTdNEP

Re: [PATCH v4 2/2] iio: light: vcnl4000: add support for Capella CM36686 and CM36672P

[Jonathan Cameron]

On Sun, 15 Feb 2026 22:06:28 +0200
Erikas Bitovtas <xerikasxx@gmail.com> wrote:

> On 2/15/26 9:31 PM, Jonathan Cameron wrote:
> > On Sun, 15 Feb 2026 19:28:56 +0200
> > Erikas Bitovtas <xerikasxx@gmail.com> wrote:
> >   
> >> On 2/14/26 8:09 PM, Jonathan Cameron wrote:  
> >>>> ---
> >>>>  drivers/iio/light/vcnl4000.c | 40 ++++++++++++++++++++++++++++++++++++++++
> >>>>  1 file changed, 40 insertions(+)
> >>>>
> >>>> diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
> >>>> index a36c23813679..1f8f4e4586f4 100644
> >>>> --- a/drivers/iio/light/vcnl4000.c
> >>>> +++ b/drivers/iio/light/vcnl4000.c
> >>>> @@ -185,6 +185,7 @@ static const int vcnl4040_ps_oversampling_ratio[] = {1, 2, 4, 8};
> >>>>  #define VCNL4000_SLEEP_DELAY_MS	2000 /* before we enter pm_runtime_suspend */
> >>>>  
> >>>>  enum vcnl4000_device_ids {
> >>>> +	CM36672P,
> >>>>  	VCNL4000,
> >>>>  	VCNL4010,
> >>>>  	VCNL4040,
> >>>> @@ -235,6 +236,8 @@ struct vcnl4000_chip_spec {
> >>>>  };
> >>>>  
> >>>>  static const struct i2c_device_id vcnl4000_id[] = {
> >>>> +	{ "cm36672p", CM36672P },
> >>>> +	{ "cm36686", VCNL4040 },
> >>>>  	{ "vcnl4000", VCNL4000 },
> >>>>  	{ "vcnl4010", VCNL4010 },
> >>>>  	{ "vcnl4020", VCNL4010 },
> >>>> @@ -1842,6 +1845,22 @@ static const struct iio_chan_spec vcnl4040_channels[] = {
> >>>>  	}
> >>>>  };    
> >>>
> >>> ...
> >>>     
> >>>>  	[VCNL4000] = {
> >>>>  		.prod = "VCNL4000",
> >>>>  		.init = vcnl4000_init,
> >>>> @@ -2033,6 +2065,14 @@ static int vcnl4000_probe(struct i2c_client *client)
> >>>>  }
> >>>>  
> >>>>  static const struct of_device_id vcnl_4000_of_match[] = {
> >>>> +	{
> >>>> +		.compatible = "capella,cm36672p",
> >>>> +		.data = (void *)CM36672P,
> >>>> +	},
> >>>> +	{
> >>>> +		.compatible = "capella,cm36686",
> >>>> +		.data = (void *)VCNL4040,    
> >>>
> >>> Is this necessary? I 'think' if you drop it we'll match instead
> >>> on the vcnl4040 fallback and then the access to the data will be
> >>> through the stripped name only bit of the compatible (first entry, not
> >>> the fallback so cm36686 in this case). So you do need the cm36686
> >>> entry in the i2c_device_id table above. Probably better to keep
> >>> this here to avoid having to reason this out - but perhaps a
> >>> comment to that affect would be useful (assuming you verify my
> >>> reasoning).
> >>>    
> >> After I removed the entry for "capella,cm36686", I received the "Unable
> >> to handle kernel NULL pointer dereference" error in dmesg. And at least
> >> stk3310 driver includes a compatible entry both for the device (stk3013)
> >> and for the fallback (stk3310). So my assumption is that this entry is
> >> needed.
> >> I could include a comment explaining that cm36686 is fully compatible
> >> with vcnl4040, however, if that is necessary.  
> > 
> > Thanks for checking.
> > 
> > What did you get as the backtrace?  I'm hoping it'll explain what I'm
> > misunderstanding!  The hacks around using the wrong table for compatible
> > matches have tripped me up before.
> > 
> > Jonathan
> >   
> 
> I am attaching a link to the dmesg. There were quite a lot of lines in
> the stack trace and I am not sure what is the right way to post logs in
> the mailing list.
> 
> https://pastebin.com/QgeTdNEP

Thanks. only relevant bit is probably:

[   15.566076]  vcnl4000_probe+0x54/0x288 [vcnl4000] (P)
[   15.566102]  i2c_device_probe+0x2b0/0x358
[   15.566121]  really_probe+0x154/0x448

My guess is my understanding of i2c_client_get_device_id() is wrong and that
is returning NULL.  That can only happen if client->name is not a match for
anything the i2_device_id table.  If you have a chance, can you dump
what client->name is in this case? I thought it ended up as
cm36686 (stripped first entry in compatible) but seems I'm probably wrong on
that :(

The path I thought worked was via info->type (which gets copied to client->name)
set via of_alias_from_compatible() here.
https://elixir.bootlin.com/linux/v6.19-rc4/source/drivers/i2c/i2c-core-of.c#L30
Which should just return the first compatible without that vendor prefix.

Meh, this doesn't really matter anyway as once we refactor to actually use
the data in the of_device_id table, we will need the entry and in the meantime
it's sort of documentation.

J

> 

Re: [PATCH v4 1/2] dt-bindings: iio: light: vcnl4000: add Capella CM36686 and CM36672P

[Krzysztof Kozlowski]

On 15/02/2026 17:16, Erikas Bitovtas wrote:
>> But CM36686 is fully compatible with CM36672P, right?
>>
>> So this would make sense?
>>
>>       - items:
>>           - const: capella,cm36686
>>           - const: vishay,vcnl4040
>>           - const: capella,cm36686p
>>
>>
> If you try to use CM36686 compatible for CM36672P, proximity channels
> will work, but in_illuminance_raw will return 0 and changing illuminance
> parameters will have no effect. That is because CM36672P is a proximity
> sensor only and the register fields for ambient light are reserved.
> And if you try to use CM36672P compatible with CM36686, it will work,
> but only proximity channel will be available, even though CM36686 also
> can sense light.

So clearly CM36672P is the superset and should be used with CM36686
fallback.

Lack of the fallback how the patch is written now is a mistake.

Best regards,
Krzysztof

Re: [PATCH v4 2/2] iio: light: vcnl4000: add support for Capella CM36686 and CM36672P

[Erikas Bitovtas]

On 2/15/26 11:55 PM, Jonathan Cameron wrote:
> On Sun, 15 Feb 2026 22:06:28 +0200
> Erikas Bitovtas <xerikasxx@gmail.com> wrote:
> 
>> On 2/15/26 9:31 PM, Jonathan Cameron wrote:
>>> On Sun, 15 Feb 2026 19:28:56 +0200
>>> Erikas Bitovtas <xerikasxx@gmail.com> wrote:
>>>   
>>>> On 2/14/26 8:09 PM, Jonathan Cameron wrote:  
>>>>>> ---
>>>>>>  drivers/iio/light/vcnl4000.c | 40 ++++++++++++++++++++++++++++++++++++++++
>>>>>>  1 file changed, 40 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
>>>>>> index a36c23813679..1f8f4e4586f4 100644
>>>>>> --- a/drivers/iio/light/vcnl4000.c
>>>>>> +++ b/drivers/iio/light/vcnl4000.c
>>>>>> @@ -185,6 +185,7 @@ static const int vcnl4040_ps_oversampling_ratio[] = {1, 2, 4, 8};
>>>>>>  #define VCNL4000_SLEEP_DELAY_MS	2000 /* before we enter pm_runtime_suspend */
>>>>>>  
>>>>>>  enum vcnl4000_device_ids {
>>>>>> +	CM36672P,
>>>>>>  	VCNL4000,
>>>>>>  	VCNL4010,
>>>>>>  	VCNL4040,
>>>>>> @@ -235,6 +236,8 @@ struct vcnl4000_chip_spec {
>>>>>>  };
>>>>>>  
>>>>>>  static const struct i2c_device_id vcnl4000_id[] = {
>>>>>> +	{ "cm36672p", CM36672P },
>>>>>> +	{ "cm36686", VCNL4040 },
>>>>>>  	{ "vcnl4000", VCNL4000 },
>>>>>>  	{ "vcnl4010", VCNL4010 },
>>>>>>  	{ "vcnl4020", VCNL4010 },
>>>>>> @@ -1842,6 +1845,22 @@ static const struct iio_chan_spec vcnl4040_channels[] = {
>>>>>>  	}
>>>>>>  };    
>>>>>
>>>>> ...
>>>>>     
>>>>>>  	[VCNL4000] = {
>>>>>>  		.prod = "VCNL4000",
>>>>>>  		.init = vcnl4000_init,
>>>>>> @@ -2033,6 +2065,14 @@ static int vcnl4000_probe(struct i2c_client *client)
>>>>>>  }
>>>>>>  
>>>>>>  static const struct of_device_id vcnl_4000_of_match[] = {
>>>>>> +	{
>>>>>> +		.compatible = "capella,cm36672p",
>>>>>> +		.data = (void *)CM36672P,
>>>>>> +	},
>>>>>> +	{
>>>>>> +		.compatible = "capella,cm36686",
>>>>>> +		.data = (void *)VCNL4040,    
>>>>>
>>>>> Is this necessary? I 'think' if you drop it we'll match instead
>>>>> on the vcnl4040 fallback and then the access to the data will be
>>>>> through the stripped name only bit of the compatible (first entry, not
>>>>> the fallback so cm36686 in this case). So you do need the cm36686
>>>>> entry in the i2c_device_id table above. Probably better to keep
>>>>> this here to avoid having to reason this out - but perhaps a
>>>>> comment to that affect would be useful (assuming you verify my
>>>>> reasoning).
>>>>>    
>>>> After I removed the entry for "capella,cm36686", I received the "Unable
>>>> to handle kernel NULL pointer dereference" error in dmesg. And at least
>>>> stk3310 driver includes a compatible entry both for the device (stk3013)
>>>> and for the fallback (stk3310). So my assumption is that this entry is
>>>> needed.
>>>> I could include a comment explaining that cm36686 is fully compatible
>>>> with vcnl4040, however, if that is necessary.  
>>>
>>> Thanks for checking.
>>>
>>> What did you get as the backtrace?  I'm hoping it'll explain what I'm
>>> misunderstanding!  The hacks around using the wrong table for compatible
>>> matches have tripped me up before.
>>>
>>> Jonathan
>>>   
>>
>> I am attaching a link to the dmesg. There were quite a lot of lines in
>> the stack trace and I am not sure what is the right way to post logs in
>> the mailing list.
>>
>> https://pastebin.com/QgeTdNEP
> 
> Thanks. only relevant bit is probably:
> 
> [   15.566076]  vcnl4000_probe+0x54/0x288 [vcnl4000] (P)
> [   15.566102]  i2c_device_probe+0x2b0/0x358
> [   15.566121]  really_probe+0x154/0x448
> 
> My guess is my understanding of i2c_client_get_device_id() is wrong and that
> is returning NULL.  That can only happen if client->name is not a match for
> anything the i2_device_id table.  If you have a chance, can you dump
> what client->name is in this case? I thought it ended up as
> cm36686 (stripped first entry in compatible) but seems I'm probably wrong on
> that :(
> 
> The path I thought worked was via info->type (which gets copied to client->name)
> set via of_alias_from_compatible() here.
> https://elixir.bootlin.com/linux/v6.19-rc4/source/drivers/i2c/i2c-core-of.c#L30
> Which should just return the first compatible without that vendor prefix.
> 
> Meh, this doesn't really matter anyway as once we refactor to actually use
> the data in the of_device_id table, we will need the entry and in the meantime
> it's sort of documentation.
> 
> J
> 

Apparently I just had commented out the i2c_device_id entry for cm36686
as well, when I had to comment out only of_device_id entry. After adding
i2c_device_id entry back, it works, just as you said.
I will submit a v5 with of_device_id entry removed if that is necessary.

Re: [PATCH v4 1/2] dt-bindings: iio: light: vcnl4000: add Capella CM36686 and CM36672P

[Erikas Bitovtas]

On 2/16/26 9:27 AM, Krzysztof Kozlowski wrote:
> On 15/02/2026 17:16, Erikas Bitovtas wrote:
>>> But CM36686 is fully compatible with CM36672P, right?
>>>
>>> So this would make sense?
>>>
>>>       - items:
>>>           - const: capella,cm36686
>>>           - const: vishay,vcnl4040
>>>           - const: capella,cm36686p
>>>
>>>
>> If you try to use CM36686 compatible for CM36672P, proximity channels
>> will work, but in_illuminance_raw will return 0 and changing illuminance
>> parameters will have no effect. That is because CM36672P is a proximity
>> sensor only and the register fields for ambient light are reserved.
>> And if you try to use CM36672P compatible with CM36686, it will work,
>> but only proximity channel will be available, even though CM36686 also
>> can sense light.
> 
> So clearly CM36672P is the superset and should be used with CM36686
> fallback.
> 
> Lack of the fallback how the patch is written now is a mistake.
> 

Is it not the other way around? CM36686 compatible fully supports
CM36672P, but CM36672P does not fully support CM36686. This would make
CM36672P a subset of CM36686, because CM36672P is the proximity sensor,
and CM36686 is proximity and ambient light sensor, and therefore, a
superset of CM36672P.

Re: [PATCH v4 1/2] dt-bindings: iio: light: vcnl4000: add Capella CM36686 and CM36672P

[Krzysztof Kozlowski]

On 16/02/2026 09:49, Erikas Bitovtas wrote:
> 
> 
> On 2/16/26 9:27 AM, Krzysztof Kozlowski wrote:
>> On 15/02/2026 17:16, Erikas Bitovtas wrote:
>>>> But CM36686 is fully compatible with CM36672P, right?
>>>>
>>>> So this would make sense?
>>>>
>>>>       - items:
>>>>           - const: capella,cm36686
>>>>           - const: vishay,vcnl4040
>>>>           - const: capella,cm36686p
>>>>
>>>>
>>> If you try to use CM36686 compatible for CM36672P, proximity channels
>>> will work, but in_illuminance_raw will return 0 and changing illuminance
>>> parameters will have no effect. That is because CM36672P is a proximity
>>> sensor only and the register fields for ambient light are reserved.
>>> And if you try to use CM36672P compatible with CM36686, it will work,
>>> but only proximity channel will be available, even though CM36686 also
>>> can sense light.
>>
>> So clearly CM36672P is the superset and should be used with CM36686
>> fallback.
>>
>> Lack of the fallback how the patch is written now is a mistake.
>>
> 
> Is it not the other way around? CM36686 compatible fully supports
> CM36672P, but CM36672P does not fully support CM36686. This would make
> CM36672P a subset of CM36686, because CM36672P is the proximity sensor,
> and CM36686 is proximity and ambient light sensor, and therefore, a
> superset of CM36672P.


Yes, you are right. The sentence "CM36672P compatible with CM36686" was
a bit confusing what is the device what is the compatible. Anyway the
commit msg needs changes to clarify reason you chosen vcnl4040 as
fallback, even though there is compatibility between CM devices.


Best regards,
Krzysztof

Re: [PATCH v4 2/2] iio: light: vcnl4000: add support for Capella CM36686 and CM36672P

[Jonathan Cameron]

On Mon, 16 Feb 2026 10:21:23 +0200
Erikas Bitovtas <xerikasxx@gmail.com> wrote:

> On 2/15/26 11:55 PM, Jonathan Cameron wrote:
> > On Sun, 15 Feb 2026 22:06:28 +0200
> > Erikas Bitovtas <xerikasxx@gmail.com> wrote:
> >   
> >> On 2/15/26 9:31 PM, Jonathan Cameron wrote:  
> >>> On Sun, 15 Feb 2026 19:28:56 +0200
> >>> Erikas Bitovtas <xerikasxx@gmail.com> wrote:
> >>>     
> >>>> On 2/14/26 8:09 PM, Jonathan Cameron wrote:    
> >>>>>> ---
> >>>>>>  drivers/iio/light/vcnl4000.c | 40 ++++++++++++++++++++++++++++++++++++++++
> >>>>>>  1 file changed, 40 insertions(+)
> >>>>>>
> >>>>>> diff --git a/drivers/iio/light/vcnl4000.c b/drivers/iio/light/vcnl4000.c
> >>>>>> index a36c23813679..1f8f4e4586f4 100644
> >>>>>> --- a/drivers/iio/light/vcnl4000.c
> >>>>>> +++ b/drivers/iio/light/vcnl4000.c
> >>>>>> @@ -185,6 +185,7 @@ static const int vcnl4040_ps_oversampling_ratio[] = {1, 2, 4, 8};
> >>>>>>  #define VCNL4000_SLEEP_DELAY_MS	2000 /* before we enter pm_runtime_suspend */
> >>>>>>  
> >>>>>>  enum vcnl4000_device_ids {
> >>>>>> +	CM36672P,
> >>>>>>  	VCNL4000,
> >>>>>>  	VCNL4010,
> >>>>>>  	VCNL4040,
> >>>>>> @@ -235,6 +236,8 @@ struct vcnl4000_chip_spec {
> >>>>>>  };
> >>>>>>  
> >>>>>>  static const struct i2c_device_id vcnl4000_id[] = {
> >>>>>> +	{ "cm36672p", CM36672P },
> >>>>>> +	{ "cm36686", VCNL4040 },
> >>>>>>  	{ "vcnl4000", VCNL4000 },
> >>>>>>  	{ "vcnl4010", VCNL4010 },
> >>>>>>  	{ "vcnl4020", VCNL4010 },
> >>>>>> @@ -1842,6 +1845,22 @@ static const struct iio_chan_spec vcnl4040_channels[] = {
> >>>>>>  	}
> >>>>>>  };      
> >>>>>
> >>>>> ...
> >>>>>       
> >>>>>>  	[VCNL4000] = {
> >>>>>>  		.prod = "VCNL4000",
> >>>>>>  		.init = vcnl4000_init,
> >>>>>> @@ -2033,6 +2065,14 @@ static int vcnl4000_probe(struct i2c_client *client)
> >>>>>>  }
> >>>>>>  
> >>>>>>  static const struct of_device_id vcnl_4000_of_match[] = {
> >>>>>> +	{
> >>>>>> +		.compatible = "capella,cm36672p",
> >>>>>> +		.data = (void *)CM36672P,
> >>>>>> +	},
> >>>>>> +	{
> >>>>>> +		.compatible = "capella,cm36686",
> >>>>>> +		.data = (void *)VCNL4040,      
> >>>>>
> >>>>> Is this necessary? I 'think' if you drop it we'll match instead
> >>>>> on the vcnl4040 fallback and then the access to the data will be
> >>>>> through the stripped name only bit of the compatible (first entry, not
> >>>>> the fallback so cm36686 in this case). So you do need the cm36686
> >>>>> entry in the i2c_device_id table above. Probably better to keep
> >>>>> this here to avoid having to reason this out - but perhaps a
> >>>>> comment to that affect would be useful (assuming you verify my
> >>>>> reasoning).
> >>>>>      
> >>>> After I removed the entry for "capella,cm36686", I received the "Unable
> >>>> to handle kernel NULL pointer dereference" error in dmesg. And at least
> >>>> stk3310 driver includes a compatible entry both for the device (stk3013)
> >>>> and for the fallback (stk3310). So my assumption is that this entry is
> >>>> needed.
> >>>> I could include a comment explaining that cm36686 is fully compatible
> >>>> with vcnl4040, however, if that is necessary.    
> >>>
> >>> Thanks for checking.
> >>>
> >>> What did you get as the backtrace?  I'm hoping it'll explain what I'm
> >>> misunderstanding!  The hacks around using the wrong table for compatible
> >>> matches have tripped me up before.
> >>>
> >>> Jonathan
> >>>     
> >>
> >> I am attaching a link to the dmesg. There were quite a lot of lines in
> >> the stack trace and I am not sure what is the right way to post logs in
> >> the mailing list.
> >>
> >> https://pastebin.com/QgeTdNEP  
> > 
> > Thanks. only relevant bit is probably:
> > 
> > [   15.566076]  vcnl4000_probe+0x54/0x288 [vcnl4000] (P)
> > [   15.566102]  i2c_device_probe+0x2b0/0x358
> > [   15.566121]  really_probe+0x154/0x448
> > 
> > My guess is my understanding of i2c_client_get_device_id() is wrong and that
> > is returning NULL.  That can only happen if client->name is not a match for
> > anything the i2_device_id table.  If you have a chance, can you dump
> > what client->name is in this case? I thought it ended up as
> > cm36686 (stripped first entry in compatible) but seems I'm probably wrong on
> > that :(
> > 
> > The path I thought worked was via info->type (which gets copied to client->name)
> > set via of_alias_from_compatible() here.
> > https://elixir.bootlin.com/linux/v6.19-rc4/source/drivers/i2c/i2c-core-of.c#L30
> > Which should just return the first compatible without that vendor prefix.
> > 
> > Meh, this doesn't really matter anyway as once we refactor to actually use
> > the data in the of_device_id table, we will need the entry and in the meantime
> > it's sort of documentation.
> > 
> > J
> >   
> 
> Apparently I just had commented out the i2c_device_id entry for cm36686
> as well, when I had to comment out only of_device_id entry. After adding
> i2c_device_id entry back, it works, just as you said.
> I will submit a v5 with of_device_id entry removed if that is necessary.

It's not hugely important but I would expect to see it go away if a follow up
set moves to actually using the data form the of_device_id table.  At that point
the two types of table are largely independent.

Thanks,

Jonathan