linux-kernel - Re: [PATCH v2 2/6] iio: imu: bmi270: Provide chip info as configuration structure

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <875xpnbyyn.fsf@justinweiss.com>
Date: Sat, 19 Oct 2024 13:49:04 -0700
From: Justin Weiss <justin@...tinweiss.com>
To: Jonathan Cameron <jic23@...nel.org>
Cc: Alex Lanzano <lanzano.alex@...il.com>,  Lars-Peter Clausen
 <lars@...afoo.de>,  Rob Herring <robh@...nel.org>,  Krzysztof Kozlowski
 <krzk+dt@...nel.org>,  Conor Dooley <conor+dt@...nel.org>,
  linux-iio@...r.kernel.org,  devicetree@...r.kernel.org,
  linux-kernel@...r.kernel.org,  "Derek J . Clark"
 <derekjohn.clark@...il.com>,  Philip Müller
 <philm@...jaro.org>
Subject: Re: [PATCH v2 2/6] iio: imu: bmi270: Provide chip info as
 configuration structure

Jonathan Cameron <jic23@...nel.org> writes:

> On Fri, 18 Oct 2024 16:36:08 -0700
> Justin Weiss <justin@...tinweiss.com> wrote:
>
>> Prepare the bmi270 driver to support similar devices like the bmi260.
>> 
>> Signed-off-by: Justin Weiss <justin@...tinweiss.com>
> 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.
>
>>  	{ }
>>  };
>>