diff mbox series

[v6,1/4] dt-bindings: media: imx258: add bindings for IMX258 sensor

Message ID 20201118202715.6692-1-krzk@kernel.org
State Superseded
Headers show
Series [v6,1/4] dt-bindings: media: imx258: add bindings for IMX258 sensor | expand

Commit Message

Krzysztof Kozlowski Nov. 18, 2020, 8:27 p.m. UTC 
Add bindings for the IMX258 camera sensor.  The bindings, just like the
driver, are quite limited, e.g. do not support regulator supplies.

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
Reviewed-by: Rob Herring <robh@kernel.org>

---

Changes since v4:
1. Add clock-lanes,
2. Add Rob's review,
3. Add one more example and extend existing one,
4. Add common clock properties (assigned-*).

Changes since v3:
1. Document also two lane setup.

Changes since v2:
1. Remove clock-frequency, add reset GPIOs, add supplies.
2. Use additionalProperties.

Changes since v1:
1. None
---
 .../devicetree/bindings/media/i2c/imx258.yaml | 140 ++++++++++++++++++
 MAINTAINERS                                   |   1 +
 2 files changed, 141 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/imx258.yaml

Comments

Sakari Ailus Jan. 22, 2021, 9:18 a.m. UTC | #1 
Hi Krysztof,

On Wed, Nov 18, 2020 at 09:27:12PM +0100, Krzysztof Kozlowski wrote:
> Add bindings for the IMX258 camera sensor.  The bindings, just like the

> driver, are quite limited, e.g. do not support regulator supplies.

> 

> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>

> Reviewed-by: Rob Herring <robh@kernel.org>

> 

> ---

> 

> Changes since v4:

> 1. Add clock-lanes,

> 2. Add Rob's review,

> 3. Add one more example and extend existing one,

> 4. Add common clock properties (assigned-*).

> 

> Changes since v3:

> 1. Document also two lane setup.

> 

> Changes since v2:

> 1. Remove clock-frequency, add reset GPIOs, add supplies.

> 2. Use additionalProperties.

> 

> Changes since v1:

> 1. None

> ---

>  .../devicetree/bindings/media/i2c/imx258.yaml | 140 ++++++++++++++++++

>  MAINTAINERS                                   |   1 +

>  2 files changed, 141 insertions(+)

>  create mode 100644 Documentation/devicetree/bindings/media/i2c/imx258.yaml

> 

> diff --git a/Documentation/devicetree/bindings/media/i2c/imx258.yaml b/Documentation/devicetree/bindings/media/i2c/imx258.yaml

> new file mode 100644

> index 000000000000..4a3471fb88a1

> --- /dev/null

> +++ b/Documentation/devicetree/bindings/media/i2c/imx258.yaml

> @@ -0,0 +1,140 @@

> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)

> +%YAML 1.2

> +---

> +$id: http://devicetree.org/schemas/media/i2c/imx258.yaml#

> +$schema: http://devicetree.org/meta-schemas/core.yaml#

> +

> +title: Sony IMX258 13 Mpixel CMOS Digital Image Sensor

> +

> +maintainers:

> +  - Krzysztof Kozlowski <krzk@kernel.org>

> +

> +description: |-

> +  IMX258 is a diagonal 5.867mm (Type 1/3.06) 13 Mega-pixel CMOS active pixel

> +  type stacked image sensor with a square pixel array of size 4208 x 3120. It

> +  is programmable through I2C interface.  Image data is sent through MIPI

> +  CSI-2.

> +

> +properties:

> +  compatible:

> +    const: sony,imx258

> +

> +  assigned-clocks: true

> +  assigned-clock-parents: true

> +  assigned-clock-rates: true

> +

> +  clocks:

> +    description:

> +      Clock frequency from 6 to 27 MHz.

> +    maxItems: 1

> +

> +  reg:

> +    maxItems: 1

> +

> +  reset-gpios:

> +    description: |-

> +      Reference to the GPIO connected to the XCLR pin, if any.

> +

> +  vana-supply:

> +    description:

> +      Analog voltage (VANA) supply, 2.7 V

> +

> +  vdig-supply:

> +    description:

> +      Digital I/O voltage (VDIG) supply, 1.2 V

> +

> +  vif-supply:

> +    description:

> +      Interface voltage (VIF) supply, 1.8 V

> +

> +  # See ../video-interfaces.txt for more details

> +  port:

> +    type: object

> +    properties:

> +      endpoint:

> +        type: object

> +        properties:

> +          clock-lanes:

> +            const: 0


This is redundant. Please remove, same for the examples. Can be a separate
patch, too.

With this change the set seems good to me.

-- 
Sakari Ailus
Krzysztof Kozlowski Jan. 27, 2021, 12:23 p.m. UTC | #2 
On Fri, Jan 22, 2021 at 11:18:22AM +0200, Sakari Ailus wrote:
> Hi Krysztof,

> 

> On Wed, Nov 18, 2020 at 09:27:12PM +0100, Krzysztof Kozlowski wrote:

> > Add bindings for the IMX258 camera sensor.  The bindings, just like the

> > driver, are quite limited, e.g. do not support regulator supplies.

> > 

> > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>

> > Reviewed-by: Rob Herring <robh@kernel.org>

> > 

> > ---

> > 

> > Changes since v4:

> > 1. Add clock-lanes,

> > 2. Add Rob's review,

> > 3. Add one more example and extend existing one,

> > 4. Add common clock properties (assigned-*).

> > 

> > Changes since v3:

> > 1. Document also two lane setup.

> > 

> > Changes since v2:

> > 1. Remove clock-frequency, add reset GPIOs, add supplies.

> > 2. Use additionalProperties.

> > 

> > Changes since v1:

> > 1. None

> > ---

> >  .../devicetree/bindings/media/i2c/imx258.yaml | 140 ++++++++++++++++++

> >  MAINTAINERS                                   |   1 +

> >  2 files changed, 141 insertions(+)

> >  create mode 100644 Documentation/devicetree/bindings/media/i2c/imx258.yaml

> > 

> > diff --git a/Documentation/devicetree/bindings/media/i2c/imx258.yaml b/Documentation/devicetree/bindings/media/i2c/imx258.yaml

> > new file mode 100644

> > index 000000000000..4a3471fb88a1

> > --- /dev/null

> > +++ b/Documentation/devicetree/bindings/media/i2c/imx258.yaml

> > @@ -0,0 +1,140 @@

> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)

> > +%YAML 1.2

> > +---

> > +$id: http://devicetree.org/schemas/media/i2c/imx258.yaml#

> > +$schema: http://devicetree.org/meta-schemas/core.yaml#

> > +

> > +title: Sony IMX258 13 Mpixel CMOS Digital Image Sensor

> > +

> > +maintainers:

> > +  - Krzysztof Kozlowski <krzk@kernel.org>

> > +

> > +description: |-

> > +  IMX258 is a diagonal 5.867mm (Type 1/3.06) 13 Mega-pixel CMOS active pixel

> > +  type stacked image sensor with a square pixel array of size 4208 x 3120. It

> > +  is programmable through I2C interface.  Image data is sent through MIPI

> > +  CSI-2.

> > +

> > +properties:

> > +  compatible:

> > +    const: sony,imx258

> > +

> > +  assigned-clocks: true

> > +  assigned-clock-parents: true

> > +  assigned-clock-rates: true

> > +

> > +  clocks:

> > +    description:

> > +      Clock frequency from 6 to 27 MHz.

> > +    maxItems: 1

> > +

> > +  reg:

> > +    maxItems: 1

> > +

> > +  reset-gpios:

> > +    description: |-

> > +      Reference to the GPIO connected to the XCLR pin, if any.

> > +

> > +  vana-supply:

> > +    description:

> > +      Analog voltage (VANA) supply, 2.7 V

> > +

> > +  vdig-supply:

> > +    description:

> > +      Digital I/O voltage (VDIG) supply, 1.2 V

> > +

> > +  vif-supply:

> > +    description:

> > +      Interface voltage (VIF) supply, 1.8 V

> > +

> > +  # See ../video-interfaces.txt for more details

> > +  port:

> > +    type: object

> > +    properties:

> > +      endpoint:

> > +        type: object

> > +        properties:

> > +          clock-lanes:

> > +            const: 0

> 

> This is redundant. Please remove, same for the examples. Can be a separate

> patch, too.

> 

> With this change the set seems good to me.


OK, I'll remove it and send a v7.

Best regards,
Krzysztof
Krzysztof Kozlowski Jan. 27, 2021, 12:26 p.m. UTC | #3 
On Fri, Jan 22, 2021 at 11:18:22AM +0200, Sakari Ailus wrote:
> Hi Krysztof,
> 
> On Wed, Nov 18, 2020 at 09:27:12PM +0100, Krzysztof Kozlowski wrote:
> > Add bindings for the IMX258 camera sensor.  The bindings, just like the
> > driver, are quite limited, e.g. do not support regulator supplies.
> > 
> > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> > Reviewed-by: Rob Herring <robh@kernel.org>
> > 
> > ---
> > 
> > Changes since v4:
> > 1. Add clock-lanes,
> > 2. Add Rob's review,
> > 3. Add one more example and extend existing one,
> > 4. Add common clock properties (assigned-*).
> > 
> > Changes since v3:
> > 1. Document also two lane setup.
> > 
> > Changes since v2:
> > 1. Remove clock-frequency, add reset GPIOs, add supplies.
> > 2. Use additionalProperties.
> > 
> > Changes since v1:
> > 1. None
> > ---
> >  .../devicetree/bindings/media/i2c/imx258.yaml | 140 ++++++++++++++++++
> >  MAINTAINERS                                   |   1 +
> >  2 files changed, 141 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/media/i2c/imx258.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/media/i2c/imx258.yaml b/Documentation/devicetree/bindings/media/i2c/imx258.yaml
> > new file mode 100644
> > index 000000000000..4a3471fb88a1
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/i2c/imx258.yaml
> > @@ -0,0 +1,140 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/media/i2c/imx258.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Sony IMX258 13 Mpixel CMOS Digital Image Sensor
> > +
> > +maintainers:
> > +  - Krzysztof Kozlowski <krzk@kernel.org>
> > +
> > +description: |-
> > +  IMX258 is a diagonal 5.867mm (Type 1/3.06) 13 Mega-pixel CMOS active pixel
> > +  type stacked image sensor with a square pixel array of size 4208 x 3120. It
> > +  is programmable through I2C interface.  Image data is sent through MIPI
> > +  CSI-2.
> > +
> > +properties:
> > +  compatible:
> > +    const: sony,imx258
> > +
> > +  assigned-clocks: true
> > +  assigned-clock-parents: true
> > +  assigned-clock-rates: true
> > +
> > +  clocks:
> > +    description:
> > +      Clock frequency from 6 to 27 MHz.
> > +    maxItems: 1
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  reset-gpios:
> > +    description: |-
> > +      Reference to the GPIO connected to the XCLR pin, if any.
> > +
> > +  vana-supply:
> > +    description:
> > +      Analog voltage (VANA) supply, 2.7 V
> > +
> > +  vdig-supply:
> > +    description:
> > +      Digital I/O voltage (VDIG) supply, 1.2 V
> > +
> > +  vif-supply:
> > +    description:
> > +      Interface voltage (VIF) supply, 1.8 V
> > +
> > +  # See ../video-interfaces.txt for more details
> > +  port:
> > +    type: object
> > +    properties:
> > +      endpoint:
> > +        type: object
> > +        properties:
> > +          clock-lanes:
> > +            const: 0
> 
> This is redundant. Please remove, same for the examples. Can be a separate
> patch, too.

Before I remove too much - you mean "clock-lanes" is redundant or entire
set of properties in "port" node?

Best regards,
Krzysztof
Sakari Ailus Jan. 27, 2021, 12:45 p.m. UTC | #4 
On Wed, Jan 27, 2021 at 01:26:12PM +0100, Krzysztof Kozlowski wrote:
> On Fri, Jan 22, 2021 at 11:18:22AM +0200, Sakari Ailus wrote:

> > Hi Krysztof,

> > 

> > On Wed, Nov 18, 2020 at 09:27:12PM +0100, Krzysztof Kozlowski wrote:

> > > Add bindings for the IMX258 camera sensor.  The bindings, just like the

> > > driver, are quite limited, e.g. do not support regulator supplies.

> > > 

> > > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>

> > > Reviewed-by: Rob Herring <robh@kernel.org>

> > > 

> > > ---

> > > 

> > > Changes since v4:

> > > 1. Add clock-lanes,

> > > 2. Add Rob's review,

> > > 3. Add one more example and extend existing one,

> > > 4. Add common clock properties (assigned-*).

> > > 

> > > Changes since v3:

> > > 1. Document also two lane setup.

> > > 

> > > Changes since v2:

> > > 1. Remove clock-frequency, add reset GPIOs, add supplies.

> > > 2. Use additionalProperties.

> > > 

> > > Changes since v1:

> > > 1. None

> > > ---

> > >  .../devicetree/bindings/media/i2c/imx258.yaml | 140 ++++++++++++++++++

> > >  MAINTAINERS                                   |   1 +

> > >  2 files changed, 141 insertions(+)

> > >  create mode 100644 Documentation/devicetree/bindings/media/i2c/imx258.yaml

> > > 

> > > diff --git a/Documentation/devicetree/bindings/media/i2c/imx258.yaml b/Documentation/devicetree/bindings/media/i2c/imx258.yaml

> > > new file mode 100644

> > > index 000000000000..4a3471fb88a1

> > > --- /dev/null

> > > +++ b/Documentation/devicetree/bindings/media/i2c/imx258.yaml

> > > @@ -0,0 +1,140 @@

> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)

> > > +%YAML 1.2

> > > +---

> > > +$id: http://devicetree.org/schemas/media/i2c/imx258.yaml#

> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#

> > > +

> > > +title: Sony IMX258 13 Mpixel CMOS Digital Image Sensor

> > > +

> > > +maintainers:

> > > +  - Krzysztof Kozlowski <krzk@kernel.org>

> > > +

> > > +description: |-

> > > +  IMX258 is a diagonal 5.867mm (Type 1/3.06) 13 Mega-pixel CMOS active pixel

> > > +  type stacked image sensor with a square pixel array of size 4208 x 3120. It

> > > +  is programmable through I2C interface.  Image data is sent through MIPI

> > > +  CSI-2.

> > > +

> > > +properties:

> > > +  compatible:

> > > +    const: sony,imx258

> > > +

> > > +  assigned-clocks: true

> > > +  assigned-clock-parents: true

> > > +  assigned-clock-rates: true

> > > +

> > > +  clocks:

> > > +    description:

> > > +      Clock frequency from 6 to 27 MHz.

> > > +    maxItems: 1

> > > +

> > > +  reg:

> > > +    maxItems: 1

> > > +

> > > +  reset-gpios:

> > > +    description: |-

> > > +      Reference to the GPIO connected to the XCLR pin, if any.

> > > +

> > > +  vana-supply:

> > > +    description:

> > > +      Analog voltage (VANA) supply, 2.7 V

> > > +

> > > +  vdig-supply:

> > > +    description:

> > > +      Digital I/O voltage (VDIG) supply, 1.2 V

> > > +

> > > +  vif-supply:

> > > +    description:

> > > +      Interface voltage (VIF) supply, 1.8 V

> > > +

> > > +  # See ../video-interfaces.txt for more details

> > > +  port:

> > > +    type: object

> > > +    properties:

> > > +      endpoint:

> > > +        type: object

> > > +        properties:

> > > +          clock-lanes:

> > > +            const: 0

> > 

> > This is redundant. Please remove, same for the examples. Can be a separate

> > patch, too.

> 

> Before I remove too much - you mean "clock-lanes" is redundant or entire

> set of properties in "port" node?


Just clock-lanes. The rest are just as they should be IMO.

-- 
Sakari Ailus
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/media/i2c/imx258.yaml b/Documentation/devicetree/bindings/media/i2c/imx258.yaml
new file mode 100644
index 000000000000..4a3471fb88a1
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/imx258.yaml
@@ -0,0 +1,140 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/i2c/imx258.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sony IMX258 13 Mpixel CMOS Digital Image Sensor
+
+maintainers:
+  - Krzysztof Kozlowski <krzk@kernel.org>
+
+description: |-
+  IMX258 is a diagonal 5.867mm (Type 1/3.06) 13 Mega-pixel CMOS active pixel
+  type stacked image sensor with a square pixel array of size 4208 x 3120. It
+  is programmable through I2C interface.  Image data is sent through MIPI
+  CSI-2.
+
+properties:
+  compatible:
+    const: sony,imx258
+
+  assigned-clocks: true
+  assigned-clock-parents: true
+  assigned-clock-rates: true
+
+  clocks:
+    description:
+      Clock frequency from 6 to 27 MHz.
+    maxItems: 1
+
+  reg:
+    maxItems: 1
+
+  reset-gpios:
+    description: |-
+      Reference to the GPIO connected to the XCLR pin, if any.
+
+  vana-supply:
+    description:
+      Analog voltage (VANA) supply, 2.7 V
+
+  vdig-supply:
+    description:
+      Digital I/O voltage (VDIG) supply, 1.2 V
+
+  vif-supply:
+    description:
+      Interface voltage (VIF) supply, 1.8 V
+
+  # See ../video-interfaces.txt for more details
+  port:
+    type: object
+    properties:
+      endpoint:
+        type: object
+        properties:
+          clock-lanes:
+            const: 0
+
+          data-lanes:
+            oneOf:
+              - items:
+                  - const: 1
+                  - const: 2
+                  - const: 3
+                  - const: 4
+              - items:
+                  - const: 1
+                  - const: 2
+
+          link-frequencies:
+            allOf:
+              - $ref: /schemas/types.yaml#/definitions/uint64-array
+            description:
+              Allowed data bus frequencies.
+
+        required:
+          - clock-lanes
+          - data-lanes
+          - link-frequencies
+
+required:
+  - compatible
+  - reg
+  - port
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c0 {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        sensor@6c {
+            compatible = "sony,imx258";
+            reg = <0x6c>;
+            clocks = <&imx258_clk>;
+
+            port {
+                endpoint {
+                    remote-endpoint = <&csi1_ep>;
+                    clock-lanes = <0>;
+                    data-lanes = <1 2 3 4>;
+                    link-frequencies = /bits/ 64 <320000000>;
+                };
+            };
+        };
+    };
+
+    /* Oscillator on the camera board */
+    imx258_clk: clk {
+        compatible = "fixed-clock";
+        #clock-cells = <0>;
+        clock-frequency = <19200000>;
+    };
+
+  - |
+    i2c0 {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        sensor@6c {
+            compatible = "sony,imx258";
+            reg = <0x6c>;
+            clocks = <&imx258_clk>;
+
+            assigned-clocks = <&imx258_clk>;
+            assigned-clock-rates = <19200000>;
+
+            port {
+                endpoint {
+                    remote-endpoint = <&csi1_ep>;
+                    clock-lanes = <0>;
+                    data-lanes = <1 2 3 4>;
+                    link-frequencies = /bits/ 64 <633600000>;
+                };
+            };
+        };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 82ea6867ac2e..96ca9d2857db 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16395,6 +16395,7 @@  M:	Sakari Ailus <sakari.ailus@linux.intel.com>
 L:	linux-media@vger.kernel.org
 S:	Maintained
 T:	git git://linuxtv.org/media_tree.git
+F:	Documentation/devicetree/bindings/media/i2c/imx258.yaml
 F:	drivers/media/i2c/imx258.c
 
 SONY IMX274 SENSOR DRIVER