Hi,
On Wed, Aug 23, 2023 at 9:53 AM Andy Shevchenko
<andy.shevchenko@xxxxxxxxx> wrote:
>
> On Wed, Aug 23, 2023 at 5:36 PM Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote:
> > > On Sun, Aug 13, 2023 at 1:51 AM Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > > wrote:
>
> ...
>
> > > It seems like this is a sign that nobody is actually using the i2c match
> > > table.
>
> You can't know. The I2C ID table allows to instantiate a device from
> user space by supplying it's address and a name, that has to be
> matched with the one in ID table.
In general, right, you can't know. ...and in general, I wouldn't have
suggested removing the table. However, in this specific case I think
we have a very good idea that nobody is using it. Specifically, if you
take a look at Biju's patch you can see that if anyone had been trying
to use the I2C table then they would have been getting a NULL pointer
dereference at probe time for the last ~5 years.
Specifically, I think that as of commit 025910db8057 ("drm/bridge:
analogix-anx78xx: add support for 7808 addresses") that if anyone were
using the I2C ID table:
1. In anx78xx_i2c_probe(), device_get_match_data() would have returned NULL
2. We would have tried to dereference that NULL in the loop.
> > > It was probably added because the original author just copy/pasted
> > > from something else, but obviously it hasn't been kept up to date and isn't
> > > working.
>
> How can you be so sure?
Unless I misunderstood the code, they'd be crashing.
> > > While your patch would make it work for "anx7814", it wouldn't
> > > make it work for any of the other similar parts. ...and yes, you could add
> > > support for those parts in your patch too, but IMO it makes more sense to
> > > just delete the i2c table and when someone has an actual need then they can
> > > re-add it.
> > >
> > > Sound OK?
>
> No. Please, do not remove the I2C ID table. It had already been
> discussed a few years ago.
If you really want the table kept then it's no skin off my teeth. I
just happened to see that nobody was responding to the patch and I was
trying to be helpful. My analysis above showed that the I2C table must
not be used, but if you feel strongly that we need to add code then
feel free to provide a Reviewed-by tag to Biju's patch! :-)
-Doug
