linux-kernel - Re: [PATCH 3/4] iio: accel: mc3230: add mc3510c support

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: <20250112105233.283f3d49@jic23-huawei>
Date: Sun, 12 Jan 2025 10:52:33 +0000
From: Jonathan Cameron <jic23@...nel.org>
To: Markuss Broks <markuss.broks@...il.com>
Cc: nekodevelopper@...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,
 linux-kernel@...r.kernel.org, devicetree@...r.kernel.org
Subject: Re: [PATCH 3/4] iio: accel: mc3230: add mc3510c support

On Sun, 12 Jan 2025 01:04:34 +0200
Markuss Broks <markuss.broks@...il.com> wrote:

> On 1/11/25 10:11 PM, Vasiliy Doylov via B4 Relay wrote:
> > From: Vasiliy Doylov <nekodevelopper@...il.com>
> >
> > This commit integrates support for the mc3510c into the mc3230 driver.
> >
> > Tested on Huawei MediaPad T3 10 (huawei-agassi)
> >
> > Signed-off-by: Vasiliy Doylov <nekodevelopper@...il.com>
> > ---
> >   drivers/iio/accel/mc3230.c | 55 ++++++++++++++++++++++++++++++++++++----------
> >   1 file changed, 44 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/iio/accel/mc3230.c b/drivers/iio/accel/mc3230.c
> > index 3cad6f2d7a2a79df38f90e5656763f6ed019a920..ebbb96c658d87a83007c7c3c7212ce9ebf039963 100644
> > --- a/drivers/iio/accel/mc3230.c
> > +++ b/drivers/iio/accel/mc3230.c
> > @@ -22,20 +22,41 @@
> >   #define MC3230_MODE_OPCON_STANDBY	0x03
> >   
> >   #define MC3230_REG_CHIP_ID		0x18
> > -#define MC3230_CHIP_ID			0x01
> > -
> >   #define MC3230_REG_PRODUCT_CODE		0x3b
> > -#define MC3230_PRODUCT_CODE		0x19
> >   
> >   /*
> >    * The accelerometer has one measurement range:
> >    *
> >    * -1.5g - +1.5g (8-bit, signed)
> >    *
> > - * scale = (1.5 + 1.5) * 9.81 / (2^8 - 1)	= 0.115411765
> >    */
> >   
> > -static const int mc3230_nscale = 115411765;
> > +enum mc3xxx_chips {
> > +	MC3230,
> > +	MC3510C,
> > +};
> > +
> > +struct mc3xxx_chip_info {
> > +	const char *name;
> > +	const u8 chip_id;
> > +	const u8 product_code;
> > +	const int scale;
> > +};  
> The struct members are usually ordered alphabetically. Also, const 
> specifiers for u8s and int are redundant, you will only want it for the 
> pointer, usually.

No they are not usually ordered alphabetically (in kernel code anyway)
Much more important characteristics apply when choosing structure ordering.

1) Comprehensibility - keep related items next to each other.
2) Slight potential performance benefit from frequently accessed items as first entry.
3) Padding concerns.  pahole will help but generally it is easy to work out
   from first principles.

In that order.   Sure you can do alphabetical if none of the above apply, but
it is far from a critical factor.

Const specifiers here are harmless as anotation but not necessary as you say.

Jonathan