[PATCH v5 0/2] ov9282: Add ov9281 support

[PATCH v5 0/2] ov9282: Add ov9281 support

[Alexander Stein]

Hello all,

This is v5 of the ov9281 support series. Thanks for the feedback I received
over the time.

Changes in v5:
* Removed unrelated patches from the series
* Squashed previous patch 3 & 6 (now patch 2)
* Removed error handling for device_get_match_data() call
* Added Kieran's r-b to patch 2

Best regards,
Alexander

Alexander Stein (2):
  media: dt-bindings: media: Add compatible for ov9281
  media: i2c: ov9282: Add ov9281 compatible

 Documentation/devicetree/bindings/media/i2c/ovti,ov9282.yaml | 5 ++++-
 drivers/media/i2c/ov9282.c                                   | 5 ++++-
 2 files changed, 8 insertions(+), 2 deletions(-)

-- 
2.34.1

[PATCH v5 1/2] media: dt-bindings: media: Add compatible for ov9281

[Alexander Stein]

This is a slightly different hardware with identical software interface.

Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 Documentation/devicetree/bindings/media/i2c/ovti,ov9282.yaml | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov9282.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov9282.yaml
index 652b4cfeded7..0c4654e70d46 100644
--- a/Documentation/devicetree/bindings/media/i2c/ovti,ov9282.yaml
+++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov9282.yaml
@@ -16,10 +16,13 @@ description:
   sensor with an active array size of 1296H x 816V. It is programmable through
   I2C interface. The I2C client address is fixed to 0x60/0x70 as per sensor data
   sheet. Image data is sent through MIPI CSI-2.
+  OV9281 has a different lens chief ray angle.
 
 properties:
   compatible:
-    const: ovti,ov9282
+    enum:
+      - ovti,ov9281
+      - ovti,ov9282
   reg:
     description: I2C address
     maxItems: 1
-- 
2.34.1

[PATCH v5 2/2] media: i2c: ov9282: Add ov9281 compatible

[Alexander Stein]

According to product brief they are identical from software point of view.
Differences are a different chief ray angle (CRA) and the package.
To distinguish ov9281 & ov9282 in userspace the name has to be explicitly
set. Provide a fixed string using platform data.

Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
Acked-by: Daniele Alessandrelli <daniele.alessandrelli@intel.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 drivers/media/i2c/ov9282.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c
index 072144668892..3d31d6f58dab 100644
--- a/drivers/media/i2c/ov9282.c
+++ b/drivers/media/i2c/ov9282.c
@@ -1400,6 +1400,8 @@ static int ov9282_probe(struct i2c_client *client)
 
 	/* Initialize subdev */
 	v4l2_i2c_subdev_init(&ov9282->sd, client, &ov9282_subdev_ops);
+	v4l2_i2c_subdev_set_name(&ov9282->sd, client,
+				 device_get_match_data(ov9282->dev), NULL);
 
 	ret = ov9282_parse_hw_config(ov9282);
 	if (ret) {
@@ -1499,7 +1501,8 @@ static const struct dev_pm_ops ov9282_pm_ops = {
 };
 
 static const struct of_device_id ov9282_of_match[] = {
-	{ .compatible = "ovti,ov9282" },
+	{ .compatible = "ovti,ov9281", .data = "ov9281" },
+	{ .compatible = "ovti,ov9282", .data = "ov9282" },
 	{ }
 };
 
-- 
2.34.1

Re: [PATCH v5 0/2] ov9282: Add ov9281 support

[Hans Verkuil]

Hi Alexander,

On 11/24/22 11:29, Alexander Stein wrote:
> Hello all,
> 
> This is v5 of the ov9281 support series. Thanks for the feedback I received
> over the time.
> 
> Changes in v5:
> * Removed unrelated patches from the series
> * Squashed previous patch 3 & 6 (now patch 2)
> * Removed error handling for device_get_match_data() call
> * Added Kieran's r-b to patch 2
> 
> Best regards,
> Alexander
> 
> Alexander Stein (2):
>   media: dt-bindings: media: Add compatible for ov9281
>   media: i2c: ov9282: Add ov9281 compatible
> 
>  Documentation/devicetree/bindings/media/i2c/ovti,ov9282.yaml | 5 ++++-
>  drivers/media/i2c/ov9282.c                                   | 5 ++++-
>  2 files changed, 8 insertions(+), 2 deletions(-)
> 

With the latest staging tree I am now getting these smatch and sparse warnings:

sparse:

drivers/media/i2c/ov9282.c:267:24: warning: symbol 'common_regs_list' was not declared. Should it be static?
drivers/media/i2c/ov9282.c:145: warning: Function parameter or member 'crop' not described in 'ov9282_mode'
drivers/media/i2c/ov9282.c:190: warning: Function parameter or member 'supplies' not described in 'ov9282'
drivers/media/i2c/ov9282.c:190: warning: Function parameter or member 'pixel_rate' not described in 'ov9282'
drivers/media/i2c/ov9282.c:190: warning: Function parameter or member 'noncontinuous_clock' not described in 'ov9282'

smatch also has this:

drivers/media/i2c/ov9282.c:1262 ov9282_power_on() warn: 'ov9282->inclk' from clk_prepare_enable() not released on lines: 1252.

Can you take a look and post a patch on top fixing this?

Thanks!

	Hans

Re: [PATCH v5 0/2] ov9282: Add ov9281 support

[Sakari Ailus]

Hi Hans,

On Mon, Nov 28, 2022 at 02:52:28PM +0100, Hans Verkuil wrote:
> Hi Alexander,
> 
> On 11/24/22 11:29, Alexander Stein wrote:
> > Hello all,
> > 
> > This is v5 of the ov9281 support series. Thanks for the feedback I received
> > over the time.
> > 
> > Changes in v5:
> > * Removed unrelated patches from the series
> > * Squashed previous patch 3 & 6 (now patch 2)
> > * Removed error handling for device_get_match_data() call
> > * Added Kieran's r-b to patch 2
> > 
> > Best regards,
> > Alexander
> > 
> > Alexander Stein (2):
> >   media: dt-bindings: media: Add compatible for ov9281
> >   media: i2c: ov9282: Add ov9281 compatible
> > 
> >  Documentation/devicetree/bindings/media/i2c/ovti,ov9282.yaml | 5 ++++-
> >  drivers/media/i2c/ov9282.c                                   | 5 ++++-
> >  2 files changed, 8 insertions(+), 2 deletions(-)
> > 
> 
> With the latest staging tree I am now getting these smatch and sparse warnings:
> 
> sparse:
> 
> drivers/media/i2c/ov9282.c:267:24: warning: symbol 'common_regs_list' was not declared. Should it be static?
> drivers/media/i2c/ov9282.c:145: warning: Function parameter or member 'crop' not described in 'ov9282_mode'
> drivers/media/i2c/ov9282.c:190: warning: Function parameter or member 'supplies' not described in 'ov9282'
> drivers/media/i2c/ov9282.c:190: warning: Function parameter or member 'pixel_rate' not described in 'ov9282'
> drivers/media/i2c/ov9282.c:190: warning: Function parameter or member 'noncontinuous_clock' not described in 'ov9282'
> 
> smatch also has this:
> 
> drivers/media/i2c/ov9282.c:1262 ov9282_power_on() warn: 'ov9282->inclk' from clk_prepare_enable() not released on lines: 1252.
> 
> Can you take a look and post a patch on top fixing this?

The two patches aren't the culprit. I've seen a report on this, too. Let's
continue on the other thread once found.

-- 
Kind regards,

Sakari Ailus

Re: [PATCH v5 0/2] ov9282: Add ov9281 support

[Dave Stevenson]

Hi Hans & Sakari

On Mon, 28 Nov 2022 at 13:58, Sakari Ailus <sakari.ailus@iki.fi> wrote:
>
> Hi Hans,
>
> On Mon, Nov 28, 2022 at 02:52:28PM +0100, Hans Verkuil wrote:
> > Hi Alexander,
> >
> > On 11/24/22 11:29, Alexander Stein wrote:
> > > Hello all,
> > >
> > > This is v5 of the ov9281 support series. Thanks for the feedback I received
> > > over the time.
> > >
> > > Changes in v5:
> > > * Removed unrelated patches from the series
> > > * Squashed previous patch 3 & 6 (now patch 2)
> > > * Removed error handling for device_get_match_data() call
> > > * Added Kieran's r-b to patch 2
> > >
> > > Best regards,
> > > Alexander
> > >
> > > Alexander Stein (2):
> > >   media: dt-bindings: media: Add compatible for ov9281
> > >   media: i2c: ov9282: Add ov9281 compatible
> > >
> > >  Documentation/devicetree/bindings/media/i2c/ovti,ov9282.yaml | 5 ++++-
> > >  drivers/media/i2c/ov9282.c                                   | 5 ++++-
> > >  2 files changed, 8 insertions(+), 2 deletions(-)
> > >
> >
> > With the latest staging tree I am now getting these smatch and sparse warnings:
> >
> > sparse:
> >
> > drivers/media/i2c/ov9282.c:267:24: warning: symbol 'common_regs_list' was not declared. Should it be static?
> > drivers/media/i2c/ov9282.c:145: warning: Function parameter or member 'crop' not described in 'ov9282_mode'
> > drivers/media/i2c/ov9282.c:190: warning: Function parameter or member 'supplies' not described in 'ov9282'
> > drivers/media/i2c/ov9282.c:190: warning: Function parameter or member 'pixel_rate' not described in 'ov9282'
> > drivers/media/i2c/ov9282.c:190: warning: Function parameter or member 'noncontinuous_clock' not described in 'ov9282'
> >
> > smatch also has this:
> >
> > drivers/media/i2c/ov9282.c:1262 ov9282_power_on() warn: 'ov9282->inclk' from clk_prepare_enable() not released on lines: 1252.
> >
> > Can you take a look and post a patch on top fixing this?
>
> The two patches aren't the culprit. I've seen a report on this, too. Let's
> continue on the other thread once found.

These look to come from my series [1]. I'll send patches in a moment.

  Dave

[1] https://patchwork.linuxtv.org/project/linux-media/patch/20221028160902.2696973-2-dave.stevenson@raspberrypi.com/

> --
> Kind regards,
>
> Sakari Ailus