| Message ID | 20211101171949.1059566-1-kieran.bingham+renesas@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: i2c: max9286: Depend on VIDEO_V4L2 | expand |
Hi Kieran, Thanks for your work. On 2021-11-01 17:19:49 +0000, Kieran Bingham wrote: > The MAX9286 has not explicitly declared a dependency upon VIDEO_V4L2. > While this dependency has likely always been met by configurations > including it, the device does use V4L2 core, and should depend upon it. > > Add VIDEO_V4L2 as a dependency to match other drivers and prevent > failures when compile testing. > > Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > > --- > A bit of an RFC almost, as I haven't seen any failure on this, however > this does stand out as different to other drivers, and the recent > "max96712: Select VIDEO_V4L2" posting has shown that these deserialiser > drivers could find themselves being compile tested in a manner which > would other wise break. > --- > drivers/media/i2c/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig > index d6a5d4ca439a..9eac5e96c6aa 100644 > --- a/drivers/media/i2c/Kconfig > +++ b/drivers/media/i2c/Kconfig > @@ -468,7 +468,7 @@ config VIDEO_VPX3220 > > config VIDEO_MAX9286 > tristate "Maxim MAX9286 GMSL deserializer support" > - depends on I2C && I2C_MUX > + depends on VIDEO_V4L2 && I2C && I2C_MUX I think the new 'depends on' shall be on a separate line. Reading this is confusing as now the V4L2 is mixed with I2C while GPIO is still on a separate line. > depends on OF_GPIO > select V4L2_FWNODE > select VIDEO_V4L2_SUBDEV_API > -- > 2.30.2 >
Quoting Niklas Söderlund (2021-11-01 17:23:22) > Hi Kieran, > > Thanks for your work. > > On 2021-11-01 17:19:49 +0000, Kieran Bingham wrote: > > The MAX9286 has not explicitly declared a dependency upon VIDEO_V4L2. > > While this dependency has likely always been met by configurations > > including it, the device does use V4L2 core, and should depend upon it. > > > > Add VIDEO_V4L2 as a dependency to match other drivers and prevent > > failures when compile testing. > > > > Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > > > > --- > > A bit of an RFC almost, as I haven't seen any failure on this, however > > this does stand out as different to other drivers, and the recent > > "max96712: Select VIDEO_V4L2" posting has shown that these deserialiser > > drivers could find themselves being compile tested in a manner which > > would other wise break. > > --- > > drivers/media/i2c/Kconfig | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig > > index d6a5d4ca439a..9eac5e96c6aa 100644 > > --- a/drivers/media/i2c/Kconfig > > +++ b/drivers/media/i2c/Kconfig > > @@ -468,7 +468,7 @@ config VIDEO_VPX3220 > > > > config VIDEO_MAX9286 > > tristate "Maxim MAX9286 GMSL deserializer support" > > - depends on I2C && I2C_MUX > > + depends on VIDEO_V4L2 && I2C && I2C_MUX > > I think the new 'depends on' shall be on a separate line. Reading this > is confusing as now the V4L2 is mixed with I2C while GPIO is still on a > separate line. Indeed, I'm happy to put it on a new line too, but so very many of the other users of VIDEO_V4L2 and I2C here in media/i2c/Kconfig use depends on VIDEO_V4L2 && I2C So the difference is having the I2C_MUX ... There are only two other 'patterns' that have also added directly to the end of that: drivers/media/i2c/Kconfig: depends on VIDEO_V4L2 && I2C && I2C_MUX drivers/media/i2c/Kconfig: depends on VIDEO_V4L2 && I2C && GPIOLIB drivers/media/platform/Kconfig: depends on VIDEO_V4L2 && I2C && PM (Where the I2C_MUX is MAX9286) but it's not a very strong pattern, so splitting is still fine with me. -- Kieran > > depends on OF_GPIO > > select V4L2_FWNODE > > select VIDEO_V4L2_SUBDEV_API > > -- > > 2.30.2 > > > > -- > Regards, > Niklas Söderlund
Hi Kieran, On Mon, Nov 1, 2021 at 6:48 PM Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> wrote: > Quoting Niklas Söderlund (2021-11-01 17:23:22) > > On 2021-11-01 17:19:49 +0000, Kieran Bingham wrote: > > > The MAX9286 has not explicitly declared a dependency upon VIDEO_V4L2. > > > While this dependency has likely always been met by configurations > > > including it, the device does use V4L2 core, and should depend upon it. > > > > > > Add VIDEO_V4L2 as a dependency to match other drivers and prevent > > > failures when compile testing. > > > > > > Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > > > @@ -468,7 +468,7 @@ config VIDEO_VPX3220 > > > > > > config VIDEO_MAX9286 > > > tristate "Maxim MAX9286 GMSL deserializer support" > > > - depends on I2C && I2C_MUX > > > + depends on VIDEO_V4L2 && I2C && I2C_MUX > > > > I think the new 'depends on' shall be on a separate line. Reading this > > is confusing as now the V4L2 is mixed with I2C while GPIO is still on a > > separate line. > > Indeed, I'm happy to put it on a new line too, but so very many of the > other users of VIDEO_V4L2 and I2C here in media/i2c/Kconfig use > depends on VIDEO_V4L2 && I2C > > So the difference is having the I2C_MUX ... I2C_MUX already depends on I2C, so you can drop the latter dependency. > There are only two other 'patterns' that have also added directly to the > end of that: > > > drivers/media/i2c/Kconfig: depends on VIDEO_V4L2 && I2C && I2C_MUX > drivers/media/i2c/Kconfig: depends on VIDEO_V4L2 && I2C && GPIOLIB > drivers/media/platform/Kconfig: depends on VIDEO_V4L2 && I2C && PM > > (Where the I2C_MUX is MAX9286) but it's not a very strong pattern, so > splitting is still fine with me. I would put it on a single line. Unless you start adding COMPILE_TEST support, and the dependencies can be split in hard (needed to build) and soft (needed to run) dependencies. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Quoting Geert Uytterhoeven (2021-11-02 08:30:28) > Hi Kieran, > > On Mon, Nov 1, 2021 at 6:48 PM Kieran Bingham > <kieran.bingham+renesas@ideasonboard.com> wrote: > > Quoting Niklas Söderlund (2021-11-01 17:23:22) > > > On 2021-11-01 17:19:49 +0000, Kieran Bingham wrote: > > > > The MAX9286 has not explicitly declared a dependency upon VIDEO_V4L2. > > > > While this dependency has likely always been met by configurations > > > > including it, the device does use V4L2 core, and should depend upon it. > > > > > > > > Add VIDEO_V4L2 as a dependency to match other drivers and prevent > > > > failures when compile testing. > > > > > > > > Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > > > > > @@ -468,7 +468,7 @@ config VIDEO_VPX3220 > > > > > > > > config VIDEO_MAX9286 > > > > tristate "Maxim MAX9286 GMSL deserializer support" > > > > - depends on I2C && I2C_MUX > > > > + depends on VIDEO_V4L2 && I2C && I2C_MUX > > > > > > I think the new 'depends on' shall be on a separate line. Reading this > > > is confusing as now the V4L2 is mixed with I2C while GPIO is still on a > > > separate line. > > > > Indeed, I'm happy to put it on a new line too, but so very many of the > > other users of VIDEO_V4L2 and I2C here in media/i2c/Kconfig use > > depends on VIDEO_V4L2 && I2C > > > > So the difference is having the I2C_MUX ... > > I2C_MUX already depends on I2C, so you can drop the latter dependency. Good point. But I'll leave it, as I'm not going to change that line now ;-) > > > There are only two other 'patterns' that have also added directly to the > > end of that: > > > > > > drivers/media/i2c/Kconfig: depends on VIDEO_V4L2 && I2C && I2C_MUX > > drivers/media/i2c/Kconfig: depends on VIDEO_V4L2 && I2C && GPIOLIB > > drivers/media/platform/Kconfig: depends on VIDEO_V4L2 && I2C && PM > > > > (Where the I2C_MUX is MAX9286) but it's not a very strong pattern, so > > splitting is still fine with me. > > I would put it on a single line. > > Unless you start adding COMPILE_TEST support, and the dependencies > can be split in hard (needed to build) and soft (needed to run) > dependencies. > v2 sent with a single line. No point overthinking this at this stage ;-) Thanks Kieran > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig index d6a5d4ca439a..9eac5e96c6aa 100644 --- a/drivers/media/i2c/Kconfig +++ b/drivers/media/i2c/Kconfig @@ -468,7 +468,7 @@ config VIDEO_VPX3220 config VIDEO_MAX9286 tristate "Maxim MAX9286 GMSL deserializer support" - depends on I2C && I2C_MUX + depends on VIDEO_V4L2 && I2C && I2C_MUX depends on OF_GPIO select V4L2_FWNODE select VIDEO_V4L2_SUBDEV_API
The MAX9286 has not explicitly declared a dependency upon VIDEO_V4L2. While this dependency has likely always been met by configurations including it, the device does use V4L2 core, and should depend upon it. Add VIDEO_V4L2 as a dependency to match other drivers and prevent failures when compile testing. Signed-off-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> --- A bit of an RFC almost, as I haven't seen any failure on this, however this does stand out as different to other drivers, and the recent "max96712: Select VIDEO_V4L2" posting has shown that these deserialiser drivers could find themselves being compile tested in a manner which would other wise break. --- drivers/media/i2c/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)