| Message ID | 20250111-mainlining-mc3510c-v1-2-57be503addf8@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | iio: accel: mc3230: add mount matrix, of match and mc3510c support | expand |
On 1/11/25 10:11 PM, Vasiliy Doylov via B4 Relay wrote: > From: Vasiliy Doylov <nekodevelopper@gmail.com> > > This will make the driver probe-able via device-tree. > While the I2C match table may be sufficient, this should extend the cover > of this driver being probed by other methods. > > Signed-off-by: Vasiliy Doylov <nekodevelopper@gmail.com> > --- > drivers/iio/accel/mc3230.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/iio/accel/mc3230.c b/drivers/iio/accel/mc3230.c > index 48787c0494ae6f0ef1d4d22bc5a4608035cbe123..3cad6f2d7a2a79df38f90e5656763f6ed019a920 100644 > --- a/drivers/iio/accel/mc3230.c > +++ b/drivers/iio/accel/mc3230.c > @@ -205,10 +205,17 @@ static const struct i2c_device_id mc3230_i2c_id[] = { > }; > MODULE_DEVICE_TABLE(i2c, mc3230_i2c_id); > > +static const struct of_device_id mc3230_of_match[] = { > + { .compatible = "mcube,mc3230" }, > + { }, > +}; > +MODULE_DEVICE_TABLE(of, mc3230_of_match); > + > static struct i2c_driver mc3230_driver = { > .driver = { > .name = "mc3230", > .pm = pm_sleep_ptr(&mc3230_pm_ops), > + .of_match_table = mc3230_of_match, Should also be alphabetic over here. > }, > .probe = mc3230_probe, > .remove = mc3230_remove,
On Sun, 12 Jan 2025 01:07:44 +0200 Markuss Broks <markuss.broks@gmail.com> wrote: > On 1/11/25 10:11 PM, Vasiliy Doylov via B4 Relay wrote: > > From: Vasiliy Doylov <nekodevelopper@gmail.com> > > > > This will make the driver probe-able via device-tree. > > While the I2C match table may be sufficient, this should extend the cover > > of this driver being probed by other methods. > > > > Signed-off-by: Vasiliy Doylov <nekodevelopper@gmail.com> > > --- > > drivers/iio/accel/mc3230.c | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/drivers/iio/accel/mc3230.c b/drivers/iio/accel/mc3230.c > > index 48787c0494ae6f0ef1d4d22bc5a4608035cbe123..3cad6f2d7a2a79df38f90e5656763f6ed019a920 100644 > > --- a/drivers/iio/accel/mc3230.c > > +++ b/drivers/iio/accel/mc3230.c > > @@ -205,10 +205,17 @@ static const struct i2c_device_id mc3230_i2c_id[] = { > > }; > > MODULE_DEVICE_TABLE(i2c, mc3230_i2c_id); > > > > +static const struct of_device_id mc3230_of_match[] = { > > + { .compatible = "mcube,mc3230" }, > > + { }, > > +}; > > +MODULE_DEVICE_TABLE(of, mc3230_of_match); > > + > > static struct i2c_driver mc3230_driver = { > > .driver = { > > .name = "mc3230", > > .pm = pm_sleep_ptr(&mc3230_pm_ops), > > + .of_match_table = mc3230_of_match, > Should also be alphabetic over here. Why? I'm on board with reordering this to be closer to the definitions in struct device_driver, but alphabetic doesn't make much sense in general for filling structures. So I agree with reorder but not for that reason. Jonathan > > }, > > .probe = mc3230_probe, > > .remove = mc3230_remove,
On Sat, 11 Jan 2025 23:11:07 +0300 Vasiliy Doylov via B4 Relay <devnull+nekodevelopper.gmail.com@kernel.org> wrote: > From: Vasiliy Doylov <nekodevelopper@gmail.com> > > This will make the driver probe-able via device-tree. Why is it not today? I2C has fallbacks to match against the compatible with the vendor prefix stripped off. For module autoprobing it will fall back to the driver name. So are you actually seeing a problem probing this in a DT set up? I'm not against the change, which I think reflects best practice as there may be mc3230 devices from other vendors with different intended drivers. Jonathan > While the I2C match table may be sufficient, this should extend the cover > of this driver being probed by other methods. > > Signed-off-by: Vasiliy Doylov <nekodevelopper@gmail.com> > --- > drivers/iio/accel/mc3230.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/iio/accel/mc3230.c b/drivers/iio/accel/mc3230.c > index 48787c0494ae6f0ef1d4d22bc5a4608035cbe123..3cad6f2d7a2a79df38f90e5656763f6ed019a920 100644 > --- a/drivers/iio/accel/mc3230.c > +++ b/drivers/iio/accel/mc3230.c > @@ -205,10 +205,17 @@ static const struct i2c_device_id mc3230_i2c_id[] = { > }; > MODULE_DEVICE_TABLE(i2c, mc3230_i2c_id); > > +static const struct of_device_id mc3230_of_match[] = { > + { .compatible = "mcube,mc3230" }, > + { }, Better to have no comma after a terminating entry. We should not make it easy for people to add stuff after this (as it makes no sense if they do!) Jonathan > +}; > +MODULE_DEVICE_TABLE(of, mc3230_of_match); > + > static struct i2c_driver mc3230_driver = { > .driver = { > .name = "mc3230", > .pm = pm_sleep_ptr(&mc3230_pm_ops), > + .of_match_table = mc3230_of_match, > }, > .probe = mc3230_probe, > .remove = mc3230_remove, >
diff --git a/drivers/iio/accel/mc3230.c b/drivers/iio/accel/mc3230.c index 48787c0494ae6f0ef1d4d22bc5a4608035cbe123..3cad6f2d7a2a79df38f90e5656763f6ed019a920 100644 --- a/drivers/iio/accel/mc3230.c +++ b/drivers/iio/accel/mc3230.c @@ -205,10 +205,17 @@ static const struct i2c_device_id mc3230_i2c_id[] = { }; MODULE_DEVICE_TABLE(i2c, mc3230_i2c_id); +static const struct of_device_id mc3230_of_match[] = { + { .compatible = "mcube,mc3230" }, + { }, +}; +MODULE_DEVICE_TABLE(of, mc3230_of_match); + static struct i2c_driver mc3230_driver = { .driver = { .name = "mc3230", .pm = pm_sleep_ptr(&mc3230_pm_ops), + .of_match_table = mc3230_of_match, }, .probe = mc3230_probe, .remove = mc3230_remove,