On Tue, Feb 21, 2023 at 05:10:00PM +0100, Robin van der Gracht wrote: > On 2023-02-21 14:40, Andy Shevchenko wrote: > > On Tue, Feb 21, 2023 at 03:33:06PM +0200, Andy Shevchenko wrote: > > > Switching to use device_get_match_data() helps getting of > > > i2c_of_match_device() API. ... > > > - id = i2c_of_match_device(dev->driver->of_match_table, client); > > > - if (id) > > > - priv->type = (uintptr_t)id->data; > > > + priv->type = (uintptr_t)device_get_match_data(dev); > > > > Looking closer the I²C ID table should provide DISP_MATRIX to keep > > default and this needs to be not dropped. > > > > So, the question is what to do with unknown type then, return -EINVAL > > from probe()? > > If you leave out your addition of the DISP_UNKNOWN type, the default type > will be DISP_MATRIX if no match is found, which is as it is now. > > In that case the following change should suffice: > > @@ -713,7 +715,6 @@ static int ht16k33_seg_probe(struct device *dev, struct > ht16k33_priv *priv, > static int ht16k33_probe(struct i2c_client *client) > { > struct device *dev = &client->dev; > - const struct of_device_id *id; > struct ht16k33_priv *priv; > uint32_t dft_brightness; > int err; > @@ -728,9 +729,8 @@ static int ht16k33_probe(struct i2c_client *client) > return -ENOMEM; > > priv->client = client; > - id = i2c_of_match_device(dev->driver->of_match_table, client); > - if (id) > - priv->type = (uintptr_t)id->data; > + priv->type = (uintptr_t)device_get_match_data(dev); > + > i2c_set_clientdata(client, priv); > > err = ht16k33_initialize(priv); > > Or do you think falling back to DISP_MATRIX if no match is found is wrong? First of all, the I²C ID table should actually use DISP_MATRIX. Second, there are two points: - It would be nice to check if the OF ID table doesn't provide a setting (shouldn't we try I²C ID table and then, if still nothing, bail out?) - The I²C ID table can be extended in the future with another entry which may want to have different default -- With Best Regards, Andy Shevchenko