On Sun, 15 Feb 2026 19:28:56 +0200
Erikas Bitovtas <[email protected]> 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,
> >>
> >
>
