Hi Bryan On Thu, 15 Sept 2022 at 01:36, Bryan O'Donoghue <bryan.odonoghue@xxxxxxxxxx> wrote: > > The Sony imx477 uses the same silicon enabling reference code from Sony in > the available examples provided. Have you had any conversations with Sony on whether the same register settings are genuinely valid for all of these sensors? IMX477 is the sensor in the Raspberry Pi HQ camera, but the register set that we have from Sony has many undocumented (to us at least) registers for image quality tuning that aren't in the imx412 driver - we're up at 417 registers vs imx412's 231 [1]. Raspberry Pi has previously had discussions with Sony regarding IMX378 vs IMX477, which is also in the same family. Whilst nearly identical in register setup, they gave us 3 additional registers that have to be set for IMX378 to avoid image quality issues. I wouldn't be surprised if something similar weren't true for IMX412 and IMX577. I'm not saying it's impossible to remove a load of duplicated register settings by combining these sensors into one driver, but this current patch looks a little simplistic, and probably needs to be looking to have a struct for sensor specific registers, not just changing the compatible string and advertised name. Or perhaps we extend that later once the compatible string has been added? Merging them does bring issues with regard to testing as it is unlikely to be practical to test any new features across all variants. Dave [1] https://github.com/raspberrypi/linux/blob/rpi-5.15.y/drivers/media/i2c/imx477.c > Add an imx477 compatible string and re-use the existing imx412 code. > > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@xxxxxxxxxx> > --- > drivers/media/i2c/imx412.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/media/i2c/imx412.c b/drivers/media/i2c/imx412.c > index bc7fdb24235f..1013a5afc85f 100644 > --- a/drivers/media/i2c/imx412.c > +++ b/drivers/media/i2c/imx412.c > @@ -1289,6 +1289,7 @@ static const struct dev_pm_ops imx412_pm_ops = { > > static const struct of_device_id imx412_of_match[] = { > { .compatible = "sony,imx412", .data = "imx412" }, > + { .compatible = "sony,imx477", .data = "imx477" }, > { } > }; > > -- > 2.34.1 >
