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.0On 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,
>
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,
>>
>
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,
> >>
> >
>
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
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
>
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.
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
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
© 2016 - 2026 Red Hat, Inc.