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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On 6/15/23 11:54, Sakari Ailus wrote:
> Hi Hans,
> 
> On Thu, Jun 15, 2023 at 10:45:35AM +0200, Hans de Goede wrote:
>> Hi Sakari,
>>
>> On 6/14/23 22:39, Sakari Ailus wrote:
> 
> ...
>>>> diff --git a/drivers/media/v4l2-core/Kconfig b/drivers/media/v4l2-core/Kconfig
>>>> index 348559bc2468..523ba243261d 100644
>>>> --- a/drivers/media/v4l2-core/Kconfig
>>>> +++ b/drivers/media/v4l2-core/Kconfig
>>>> @@ -74,6 +74,11 @@ config V4L2_FWNODE
>>>>  config V4L2_ASYNC
>>>>  	tristate
>>>>  
>>>> +config V4L2_CCI
>>>> +	tristate
>>>> +	depends on I2C
>>>
>>> This won't do anything if the dependent driver will select V4L2_CCI, will
>>> it? I'd let the sensor driver depend on I2C instead. CCI is also supported
>>> on I3C, for instance.
>>
>> It will cause a Kconfig error if the dependent driver does not depend
>> on I2C. Kconfig items doing select MUST depend on all the depends on
>> of the items they are selecting; and (continued below)
> 
> Maybe this has changed? It used to be that these cases were silently
> ignored and it wasn't that long ago. I haven't been following this up.
> 
> Nevertheless, this shouldn't depend on I2C as such.
> 
>>
>>>
>>>> +	select REGMAP_I2C
>>>
>>> This is a good question.
>>>
>>> How about adding V4L2_CCI_I2C that would select REGMAP_I2C?
>>
>> v4l2-cci.ko uses the devm_regmap_init_i2c() symbol, so
>> REGMAP_I2C must be enabled when V4L2_CCI is enabled and
>> REGMAP_I2C is a symbol which should be selected rather
>> then depended on when necessary.
> 
> I agree.

If you agree that because of the symbol dependency that
the select REGMAP_I2C is necessary then the depends on I2C
is also necessary because any Kconfig symbol selecting
another symbol MUST depends on all of the dependencies
of the selected symbol and REGMAP_I2C has:

config REGMAP_I2C
        tristate
        depends on I2C

<snip>

> This is documented in
> Documentation/driver-api/media/maintainer-entry-profile.rst and media tree
> follows that.

Ah, I missed that. Ok, I'll run

./scripts/checkpatch.pl --strict --max-line-length=80

and fix the warnings, with maybe one or 2 exceptions
where longer lines really make the code more readable.

Regards,

Hans




[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux