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,
 	},
 	.probe		= mc3230_probe,
 	.remove		= mc3230_remove,

-- 
2.47.1

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,
>

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,