diff mbox series

[v2,1/3] media: dt-bindings: media: Rename imx412 to imx577

Message ID 20220718014215.1240114-2-bryan.odonoghue@linaro.org (mailing list archive)
State Superseded
Headers
Series Add imx577 compatible to imx412 |

Commit Message

Bryan O'Donoghue July 18, 2022, 1:42 a.m. UTC 
  The yaml and driver we have right now misidentifies the imx577 as the
imx412.

Looking at similar IMX chips which give their chip identifier via register
0x0016 we can see:

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

Right now imx412.c does:

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

As a first step to fixing this problem rename the supporting yaml file and
containing text from imx412 to imx577.

Fixes: 333b3125d130 ("media: dt-bindings: media: Add bindings for imx412")
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 .../i2c/{sony,imx412.yaml => sony,imx577.yaml} | 18 +++++++++---------
 MAINTAINERS                                    |  6 +++---
 2 files changed, 12 insertions(+), 12 deletions(-)
 rename Documentation/devicetree/bindings/media/i2c/{sony,imx412.yaml => sony,imx577.yaml} (83%)

Comments

Rob Herring July 20, 2022, 10:31 p.m. UTC | #1 
On Mon, 18 Jul 2022 02:42:13 +0100, Bryan O'Donoghue wrote:
> The yaml and driver we have right now misidentifies the imx577 as the
> imx412.
> 
> Looking at similar IMX chips which give their chip identifier via register
> 0x0016 we can see:
> 
> 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
> 
> Right now imx412.c does:
> 
> drivers/media/i2c/imx412.c:#define IMX412_REG_ID         0x0016
> drivers/media/i2c/imx412.c:#define IMX412_ID             0x577
> 
> As a first step to fixing this problem rename the supporting yaml file and
> containing text from imx412 to imx577.
> 
> Fixes: 333b3125d130 ("media: dt-bindings: media: Add bindings for imx412")
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
>  .../i2c/{sony,imx412.yaml => sony,imx577.yaml} | 18 +++++++++---------
>  MAINTAINERS                                    |  6 +++---
>  2 files changed, 12 insertions(+), 12 deletions(-)
>  rename Documentation/devicetree/bindings/media/i2c/{sony,imx412.yaml => sony,imx577.yaml} (83%)
> 

Acked-by: Rob Herring <robh@kernel.org>
Sakari Ailus July 22, 2022, 4:03 p.m. UTC | #2 
Hi Bryan,

On Mon, Jul 18, 2022 at 02:42:13AM +0100, Bryan O'Donoghue wrote:
> The yaml and driver we have right now misidentifies the imx577 as the
> imx412.
> 
> Looking at similar IMX chips which give their chip identifier via register
> 0x0016 we can see:
> 
> 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
> 
> Right now imx412.c does:
> 
> drivers/media/i2c/imx412.c:#define IMX412_REG_ID         0x0016
> drivers/media/i2c/imx412.c:#define IMX412_ID             0x577

This is not a proof the device in question is no IMX412. It's entirely
possible Sony has two sensors that both happen to have the same value in
the ID register. Quite possibly the sensors, for a reason or another, share
the same chip, so also the ID register will be the same. I wouldn't
necessarily expect them to make a variant just for that.

At least I'd like to see more convincing evidence IMX577 was just confused
with IMX412 before merging this.

But I'm fine with adding another compatible string for IMX577.
Bryan O'Donoghue July 22, 2022, 4:47 p.m. UTC | #3 
On 22/07/2022 17:03, Sakari Ailus wrote:
> Quite possibly the sensors, for a reason or another, share
> the same chip, so also the ID register will be the same. I wouldn't
> necessarily expect them to make a variant just for that.

Yeah it could be a silicon yield thing I suppose imx412 might be to 
imx577 as Celeron was to Pentium.

Assuming there is no other identifiable difference at the register 
level, wouldn't you think it would be more consistent at the naming 
level to have CHIP_ID 0x577 live in imx577.c and then to have 2 compat 
strings.

---
bod
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx412.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx577.yaml
similarity index 83%
rename from Documentation/devicetree/bindings/media/i2c/sony,imx412.yaml
rename to Documentation/devicetree/bindings/media/i2c/sony,imx577.yaml
index 26d1807d0bb6..e201048490e9 100644
--- a/Documentation/devicetree/bindings/media/i2c/sony,imx412.yaml
+++ b/Documentation/devicetree/bindings/media/i2c/sony,imx577.yaml
@@ -2,24 +2,24 @@ 
 # Copyright (C) 2021 Intel Corporation
 %YAML 1.2
 ---
-$id: http://devicetree.org/schemas/media/i2c/sony,imx412.yaml#
+$id: http://devicetree.org/schemas/media/i2c/sony,imx577.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: Sony IMX412 Sensor
+title: Sony IMX577 Sensor
 
 maintainers:
   - Paul J. Murphy <paul.j.murphy@intel.com>
   - Daniele Alessandrelli <daniele.alessandrelli@intel.com>
 
 description:
-  IMX412 sensor is a Sony CMOS active pixel digital image sensor with an active
+  IMX577 sensor is a Sony CMOS active pixel digital image sensor with an active
   array size of 4072H x 3176V. It is programmable through I2C interface. The
   I2C client address is fixed to 0x1a as per sensor data sheet. Image data is
   sent through MIPI CSI-2.
 
 properties:
   compatible:
-    const: sony,imx412
+    const: sony,imx577
   reg:
     description: I2C address
     maxItems: 1
@@ -80,16 +80,16 @@  examples:
         #size-cells = <0>;
 
         camera@1a {
-            compatible = "sony,imx412";
+            compatible = "sony,imx577";
             reg = <0x1a>;
-            clocks = <&imx412_clk>;
+            clocks = <&imx577_clk>;
 
-            assigned-clocks = <&imx412_clk>;
-            assigned-clock-parents = <&imx412_clk_parent>;
+            assigned-clocks = <&imx577_clk>;
+            assigned-clock-parents = <&imx577_clk_parent>;
             assigned-clock-rates = <24000000>;
 
             port {
-                imx412: endpoint {
+                imx577: endpoint {
                     remote-endpoint = <&cam>;
                     data-lanes = <1 2 3 4>;
                     link-frequencies = /bits/ 64 <600000000>;
diff --git a/MAINTAINERS b/MAINTAINERS
index 08b9ef368709..6a257af8178f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -18725,14 +18725,14 @@  S:	Maintained
 T:	git git://linuxtv.org/media_tree.git
 F:	drivers/media/i2c/imx355.c
 
-SONY IMX412 SENSOR DRIVER
+SONY IMX577 SENSOR DRIVER
 M:	Paul J. Murphy <paul.j.murphy@intel.com>
 M:	Daniele Alessandrelli <daniele.alessandrelli@intel.com>
 L:	linux-media@vger.kernel.org
 S:	Maintained
 T:	git git://linuxtv.org/media_tree.git
-F:	Documentation/devicetree/bindings/media/i2c/sony,imx412.yaml
-F:	drivers/media/i2c/imx412.c
+F:	Documentation/devicetree/bindings/media/i2c/sony,imx577.yaml
+F:	drivers/media/i2c/imx577.c
 
 SONY MEMORYSTICK SUBSYSTEM
 M:	Maxim Levitsky <maximlevitsky@gmail.com>