On Mon, 16 Feb 2026 10:21:23 +0200 Erikas Bitovtas <[email protected]> wrote:
> On 2/15/26 11:55 PM, Jonathan Cameron wrote: > > On Sun, 15 Feb 2026 22:06:28 +0200 > > Erikas Bitovtas <[email protected]> wrote: > > > >> On 2/15/26 9:31 PM, Jonathan Cameron wrote: > >>> 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 > >>> > >> > >> 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
