Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <93b19237-3afd-461f-bdb3-f2cd09cadd9a@gmail.com>
Date:   Fri, 21 Apr 2023 09:14:10 +0300
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.10.0
Content-Language: en-US, en-GB
To:     Mehdi Djait <mehdi.djait.k@gmail.com>, jic23@kernel.org
Cc:     krzysztof.kozlowski+dt@linaro.org,
        andriy.shevchenko@linux.intel.com, linux-iio@vger.kernel.org,
        linux-kernel@vger.kernel.org, devicetree@vger.kernel.org
References: <cover.1682019544.git.mehdi.djait.k@gmail.com>
 <2c5d71e37fc7e000091189b3c5d66ede1a6015d2.1682019544.git.mehdi.djait.k@gmail.com>
From:   Matti Vaittinen <mazziesaccount@gmail.com>
Subject: Re: [PATCH v2 4/5] iio: accel: kionix-kx022a: Add a function to
 retrieve number of bytes in buffer
In-Reply-To: <2c5d71e37fc7e000091189b3c5d66ede1a6015d2.1682019544.git.mehdi.djait.k@gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

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@gmail.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! ~~