Re: [PATCH 1/3] media: Add MIPI CCI register access helper functions

[Hans de Goede <hdegoede@redhat.com>]

Hi Andy,

On 6/7/23 22:07, Andy Shevchenko wrote:
> On Wed, Jun 07, 2023 at 09:01:40PM +0200, Hans de Goede wrote:
>> On 6/7/23 20:18, Laurent Pinchart wrote:
>>> On Tue, Jun 06, 2023 at 06:58:06PM +0200, Hans de Goede wrote:
> 
> ...
> 
>>>> +		if (regs[i].delay_us)
>>>> +			fsleep(regs[i].delay_us);
>>>
>>> Do you have an immediate need for this ? If not, I'd drop support for
>>> the delay, and add it later when and if needed. It will be easier to
>>> discuss the API and use cases with a real user.
>>
>> This is a 1:1 mirror of regmap_multi_reg_write() note this uses
>> the existing struct reg_sequence delay_us field and the:
>>
>> 		if (regs[i].delay_us)
>> 			fsleep(regs[i].delay_us);
>>
>> is copied from the implementation of regmap_multi_reg_write()
> 
> Reading this I'm wondering if we can actually implement a regmap-cci inside
> drivers/base/regmap and use it. It might be that this is impossible, but can
> save us from repeating existing code I think.

Someone (you I believe?) already suggested this when discussing replacing
the ov_16bit_addr_reg_helpers.h . This is not possible because regmap
assumes a fixed register width for the device and this assumption is
all over the place in regmap.

The whole purpose of the CCI helpers is to provide a thin layer on top
to deal with different register widths while delegating everything else
to regmap.

Regards,

Hans