Re: [PATCH v4 3/7] media: i2c: ov9282: Add ov9281 compatible

[Alexander Stein <alexander.stein@ew.tq-group.com>]

Hello Kieran,

Am Samstag, 12. November 2022, 00:48:24 CET schrieb Kieran Bingham:
> Hi All,
> 
> Quoting Alexander Stein (2022-07-28 14:02:33)
> 
> > According to product brief they are identical from software point of view.
> > Differences are a different chief ray angle (CRA) and the package.
> > 
> > Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> > Acked-by: Daniele Alessandrelli <daniele.alessandrelli@intel.com>
> 
> Throwing my hat in the ring on this thread as I see it has been hanging
> around for a while and my attention was sent here from [0]
> 
> [0]
> https://lists.libcamera.org/pipermail/libcamera-devel/2022-November/035495.
> html

I postponed working on this change for a while, because there are (at least) 
two series from Dave pending which conflict a bit with this series.

> > ---
> > 
> >  drivers/media/i2c/ov9282.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c
> > index 8a252bf3b59f..c8d83a29f9bb 100644
> > --- a/drivers/media/i2c/ov9282.c
> > +++ b/drivers/media/i2c/ov9282.c
> > @@ -1113,6 +1113,7 @@ static const struct dev_pm_ops ov9282_pm_ops = {
> > 
> >  };
> >  
> >  static const struct of_device_id ov9282_of_match[] = {
> > 
> > +       { .compatible = "ovti,ov9281" },
> 
> I believe from my existing understanding of how we would support
> existing sensors even with very similar parts is that a direct
> compatible lets the DT express this.
> 
> If there were a common name that we could apply, we could have a generic
> name here too, but I don't see anything specifically generic, and I
> haven't yet seen a clear pattern in the namings schemes from omnivision
> so ov928x wouldn't be appropriate as I couldn't be sure that an
> unrelated ov9289 wouldn't exist with very different properties ... so ..
> 
> >         { .compatible = "ovti,ov9282" },
> 
> Either squashed with the later 6/7 that adds the name or not: (I think
> it's fine either separated or squashed)
> 
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> It does in turn bring questions into how we handle both the ov9281 and
> ov9282 together in libcamera, but I don't think that's an issue to solve
> here. Expressing the two separately to userspace also allows libcamera
> to make a distiction between the CRA should it need to.

Krzysztof is in favor of squashing so I'll respin this accordingly, removing 
the other changes from the ov9281 support series.

Best regards,
Alexander