Jonathan Cameron <jic23@xxxxxxxxxx> writes: > On Fri, 18 Oct 2024 16:36:08 -0700 > Justin Weiss <justin@xxxxxxxxxxxxxxx> wrote: > >> Prepare the bmi270 driver to support similar devices like the bmi260. >> >> Signed-off-by: Justin Weiss <justin@xxxxxxxxxxxxxxx> > One thing in here. The enum ID thing tends to end up costing more than > the benefit it brings, so for newer drivers preferred option is separate > structure instances rather than an array. That makes sense to me, even considering your comments on patch #4. I'll switch to separate structures here and keep the if / else in that later patch. Justin >> --- >> drivers/iio/imu/bmi270/bmi270.h | 15 ++++++++++++++- >> drivers/iio/imu/bmi270/bmi270_core.c | 18 +++++++++++++++--- >> drivers/iio/imu/bmi270/bmi270_i2c.c | 11 ++++++++--- >> drivers/iio/imu/bmi270/bmi270_spi.c | 11 ++++++++--- >> 4 files changed, 45 insertions(+), 10 deletions(-) >> >> diff --git a/drivers/iio/imu/bmi270/bmi270.h b/drivers/iio/imu/bmi270/bmi270.h >> index 8ac20ad7ee94..2e8d85a4e419 100644 >> --- a/drivers/iio/imu/bmi270/bmi270.h >> +++ b/drivers/iio/imu/bmi270/bmi270.h >> @@ -10,10 +10,23 @@ struct device; >> struct bmi270_data { >> struct device *dev; >> struct regmap *regmap; >> + const struct bmi270_chip_info *chip_info; >> +}; >> + >> +enum bmi270_device_type { >> + BMI270, > > Whilst quite a few drivers do it this way, over time we've found that it's > much easier to just skip the array of structures and have independent ones. > Increase the extern lines to one per supported device, but removes > need for an enum here and generally gives slightly more readable code. > > >> +}; > >> }; >> >> static const struct of_device_id bmi270_of_match[] = { >> - { .compatible = "bosch,bmi270" }, >> + { .compatible = "bosch,bmi270", .data = &bmi270_chip_info[BMI270] }, > > After dropping the enum this just becomes &bmi270_chip_info > and later you'll add bmi260_chip_info etc. > >> { } >> }; >>