Hi Hans, On Thu, Jun 15, 2023 at 12:15:47PM +0200, Hans de Goede wrote: > 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 Yes. How about putting cci_regmap_init_i2c() behind an #ifdef? Then there wouldn't be a need for REGMAP_I2C unconditionally, but dependent on I2C. I guess right now I2C is more or less given in many systems but binding CCI to it still seems dubious. > > <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. Thank you. -- Kind regards, Sakari Ailus
