[PATCH v2 0/3] Add imx577 compatible to imx412

* [PATCH v2 0/3] Add imx577 compatible to imx412
@ 2022-07-18  1:42 Bryan O'Donoghue
  2022-07-18  1:42 ` [PATCH v2 1/3] media: dt-bindings: media: Rename imx412 to imx577 Bryan O'Donoghue
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Bryan O'Donoghue @ 2022-07-18  1:42 UTC (permalink / raw)
  To: sakari.ailus, jacopo, paul.j.murphy, daniele.alessandrelli,
	mchehab, linux-media, devicetree
  Cc: dmitry.baryshkov, konrad.dybcio, andrey.konovalov, bryan.odonoghue

V2:
Sakari wasn't especially satisfied with the answer imx412 and imx577 have
the same init sequence but, suggested setting the string for imx577 as is
done in the ccs driver.

https://lore.kernel.org/all/20220607134057.2427663-3-bryan.odonoghue@linaro.org/t/

I went to look at that and asked myself "how would I tell the difference
between the two silicon parts". The obvious answer is a chip identifier.

Luckily this class of IMX sensor has a chip identifier at offset 0x0016.

That looks like this for imx258, imx319 and imx355

drivers/media/i2c/imx258.c:#define IMX258_REG_CHIP_ID    0x0016
drivers/media/i2c/imx258.c:#define IMX258_CHIP_ID        0x0258

drivers/media/i2c/imx319.c:#define IMX319_REG_CHIP_ID    0x0016
drivers/media/i2c/imx319.c:#define IMX319_CHIP_ID        0x0319

drivers/media/i2c/imx355.c:#define IMX355_REG_CHIP_ID    0x0016
drivers/media/i2c/imx355.c:#define IMX355_CHIP_ID        0x0355

but then looks like this for imx412.

drivers/media/i2c/imx412.c:#define IMX412_REG_ID         0x0016
drivers/media/i2c/imx412.c:#define IMX412_ID             0x577

This made no sense at all to me, why is the imx412 driver not named imx577 ?

I went and dug into the Qualcomm camx/chi-cdk sources to find that a file
called cmk_imx577_sensor.xml has a property called sensorId which is
constrained to 0x0577.

In the Qualcomm stack this pairing of filename and identifier is
maintained for imx258, imx376, imx476, imx576, imx519, imx362, imx481,
imx318 imx334 and imx386.

Every single example I can find of a Sony IMX sensor which returns a chip
identifier at offset 0x0016 matches the driver name to the returned sensor
id both here upstream in Linux and in Qualcomm's camx stack.

The conclusion I draw from this is that imx412.c is inappropriately named.

I think the right thing to do is to rename imx412 to imx577. It is
confusing and I think wrong to pair imx412.c with a chip which identifies
as 0x0577.

V1:
Right now the imx412 and imx577 are code and pin compatible however, they
are distinct pieces of silicon.

Document imx577 as a compatible enum and add the compat string to imx412.c.
This allows us to differentiate these chips in DTS and potentially to apply
any future imx412 or imx577 specific changes appropriately.

Bryan O'Donoghue (3):
  media: dt-bindings: media: Rename imx412 to imx577
  media: i2c: imx577: Rename imx412.c to imx577.c
  media: i2c: imx577: Fix chip identifier define name

 .../{sony,imx412.yaml => sony,imx577.yaml}    |  18 +-
 MAINTAINERS                                   |   6 +-
 drivers/media/i2c/Kconfig                     |   8 +-
 drivers/media/i2c/Makefile                    |   2 +-
 drivers/media/i2c/{imx412.c => imx577.c}      | 622 +++++++++---------
 5 files changed, 328 insertions(+), 328 deletions(-)
 rename Documentation/devicetree/bindings/media/i2c/{sony,imx412.yaml => sony,imx577.yaml} (83%)
 rename drivers/media/i2c/{imx412.c => imx577.c} (55%)

-- 
2.34.1

^ permalink raw reply	[flat|nested] 13+ messages in thread