linux-kernel - Re: [PATCH v2 4/5] iio: accel: kionix-kx022a: Add a function to retrieve number of bytes in buffer

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: <93b19237-3afd-461f-bdb3-f2cd09cadd9a@gmail.com>
Date:   Fri, 21 Apr 2023 09:14:10 +0300
From:   Matti Vaittinen <mazziesaccount@...il.com>
To:     Mehdi Djait <mehdi.djait.k@...il.com>, jic23@...nel.org
Cc:     krzysztof.kozlowski+dt@...aro.org,
        andriy.shevchenko@...ux.intel.com, linux-iio@...r.kernel.org,
        linux-kernel@...r.kernel.org, devicetree@...r.kernel.org
Subject: Re: [PATCH v2 4/5] iio: accel: kionix-kx022a: Add a function to
 retrieve number of bytes in buffer

Hi Mehdi,

Thanks for the v2!

On 4/20/23 23:22, Mehdi Djait wrote:
> Since Kionix accelerometers use various numbers of bits to report data, a
> device-specific function is required.

I think this is the right approach. Thanks for adding this 
device-specific function.

> Move the driver's private data to the header file to support the new function.

Hmm. Why this move is necessary? I didn't immediately spot this struct 
being used outside this C-file. I'd rather saw the struct in C-file if 
possible.

> Make the allocation of the "buffer" array in the fifo_flush function dynamic
> and more generic.
> 
> Signed-off-by: Mehdi Djait <mehdi.djait.k@...il.com>
> ---
> v2:
> - separated this change from the chip_info introduction and made it a patch in v2

I am unsure if this separation was needed. I'd only separate the "naming 
changes" which bring no changes to code flow, and then patches which are 
fixes and need to be backported (to minimize backporting effort and 
impact to stable branches). Well, I am fine with this separation though, 
seems like I am just making a noise here :).

> - changed the function from generic implementation for to device-specific one
> - removed blank lines pointed out by checkpatch
> - changed the allocation of the "buffer" array in __kx022a_fifo_flush
> 
>   drivers/iio/accel/kionix-kx022a.c | 72 +++++++++++++------------------
>   drivers/iio/accel/kionix-kx022a.h | 37 ++++++++++++++++
>   2 files changed, 66 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/iio/accel/kionix-kx022a.c b/drivers/iio/accel/kionix-kx022a.c
> index 7f9a2c29790b..1c81ea1657b9 100644
> --- a/drivers/iio/accel/kionix-kx022a.c
> +++ b/drivers/iio/accel/kionix-kx022a.c
> @@ -150,36 +150,6 @@ static const struct regmap_config kx022a_regmap_config = {
>   	.cache_type = REGCACHE_RBTREE,
>   };

Yours,
	-- Matti

-- 
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~