mbox series

[v3,0/2] Add IMX296 CMOS image sensor support

Message ID 20191025175908.14260-1-manivannan.sadhasivam@linaro.org
Headers show
Series Add IMX296 CMOS image sensor support | expand

Message

Manivannan Sadhasivam Oct. 25, 2019, 5:59 p.m. UTC
Hello,

This patchset adds support for IMX296 CMOS image sensor from Sony.
Sensor can be programmed through I2C and 4-wire interface but the
current driver only supports I2C interface. The sensor is
capable of outputting frames in CSI2 format (1 Lane). In the case
of sensor resolution, driver only supports 1440x1088 at 30 FPS.

The driver has been validated using Framos IMX296 module interfaced to
96Boards Dragonboard410c.

Thanks,
Mani

Changes in v3:

* Fixed the reference to video-interfaces.txt in binding.

Changes in v2:

* Switched to YAML binding

Manivannan Sadhasivam (2):
  dt-bindings: media: i2c: Add IMX296 CMOS sensor binding
  media: i2c: Add IMX296 CMOS image sensor driver

 .../devicetree/bindings/media/i2c/imx296.yaml |  98 +++
 MAINTAINERS                                   |   8 +
 drivers/media/i2c/Kconfig                     |  11 +
 drivers/media/i2c/Makefile                    |   1 +
 drivers/media/i2c/imx296.c                    | 733 ++++++++++++++++++
 5 files changed, 851 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/imx296.yaml
 create mode 100644 drivers/media/i2c/imx296.c

-- 
2.17.1

Comments

Sakari Ailus Oct. 28, 2019, 12:21 p.m. UTC | #1
Hi Manivannan,

Thanks for the update.

On Fri, Oct 25, 2019 at 11:29:07PM +0530, Manivannan Sadhasivam wrote:
> Add YAML devicetree binding for IMX296 CMOS image sensor.

> 

> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

> ---

>  .../devicetree/bindings/media/i2c/imx296.yaml | 98 +++++++++++++++++++

>  1 file changed, 98 insertions(+)

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

> 

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

> new file mode 100644

> index 000000000000..4e204fd7cf90

> --- /dev/null

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

> @@ -0,0 +1,98 @@

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

> +%YAML 1.2

> +---

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

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

> +

> +title: Sony IMX296 1/2.8-Inch CMOS Image Sensor

> +

> +maintainers:

> +  - Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

> +

> +description: |-

> +  The Sony IMX296 is a 1/2.9-Inch active pixel type CMOS Solid-state image

> +  sensor with square pixel array and 1.58 M effective pixels. This chip

> +  features a global shutter with variable charge-integration time. It is

> +  programmable through I2C and 4-wire interfaces. The sensor output is

> +  available via CSI-2 serial data output (1 Lane).

> +

> +properties:

> +  compatible:

> +    const: sony,imx296

> +

> +  reg:

> +    maxItems: 1

> +

> +  clocks:

> +    maxItems: 1

> +

> +  clock-names:

> +    description:

> +      Input clock for the sensor.

> +    items:

> +      - const: mclk

> +

> +  clock-frequency:

> +    description:

> +      Frequency of the mclk clock in Hertz.

> +    default: 37125000


I think you could omit the default.

> +

> +  vddo-supply:

> +    description:

> +      Definition of the regulator used as interface power supply.

> +    maxItems: 1

> +

> +  vdda-supply:

> +    description:

> +      Definition of the regulator used as analog power supply.

> +    maxItems: 1

> +

> +  vddd-supply:

> +    description:

> +      Definition of the regulator used as digital power supply.

> +    maxItems: 1

> +

> +  reset-gpios:

> +    description:

> +      The phandle and specifier for the GPIO that controls sensor reset.

> +    maxItems: 1

> +

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

> +

> +required:

> +  - compatible

> +  - reg

> +  - clocks

> +  - clock-names

> +  - clock-frequency

> +  - vddo-supply

> +  - vdda-supply

> +  - vddd-supply


I think the port and endpoint nodes should documented here as well.

> +

> +additionalProperties: false

> +

> +examples:

> +  - |

> +    #include <dt-bindings/gpio/gpio.h>

> +

> +    imx296: camera-sensor@1a {

> +        compatible = "sony,imx296";

> +        reg = <0x1a>;

> +        reset-gpios = <&msmgpio 35 GPIO_ACTIVE_LOW>;

> +        pinctrl-names = "default";

> +        pinctrl-0 = <&camera_rear_default>;

> +        clocks = <&gcc 90>;

> +        clock-names = "mclk";

> +        clock-frequency = <37125000>;

> +        vddo-supply = <&camera_vddo_1v8>;

> +        vdda-supply = <&camera_vdda_3v3>;

> +        vddd-supply = <&camera_vddd_1v2>;

> +

> +        port {

> +            imx296_ep: endpoint {

> +                remote-endpoint = <&csiphy0_ep>;

> +            };

> +        };

> +    };

> +

> +...


-- 
Regards,

Sakari Ailus
Rob Herring Oct. 29, 2019, 10:09 p.m. UTC | #2
On Fri, Oct 25, 2019 at 11:29:07PM +0530, Manivannan Sadhasivam wrote:
> Add YAML devicetree binding for IMX296 CMOS image sensor.

> 

> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

> ---

>  .../devicetree/bindings/media/i2c/imx296.yaml | 98 +++++++++++++++++++

>  1 file changed, 98 insertions(+)

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

> 

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

> new file mode 100644

> index 000000000000..4e204fd7cf90

> --- /dev/null

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

> @@ -0,0 +1,98 @@

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

> +%YAML 1.2

> +---

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

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

> +

> +title: Sony IMX296 1/2.8-Inch CMOS Image Sensor

> +

> +maintainers:

> +  - Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

> +

> +description: |-

> +  The Sony IMX296 is a 1/2.9-Inch active pixel type CMOS Solid-state image

> +  sensor with square pixel array and 1.58 M effective pixels. This chip

> +  features a global shutter with variable charge-integration time. It is

> +  programmable through I2C and 4-wire interfaces. The sensor output is

> +  available via CSI-2 serial data output (1 Lane).

> +

> +properties:

> +  compatible:

> +    const: sony,imx296

> +

> +  reg:

> +    maxItems: 1

> +

> +  clocks:

> +    maxItems: 1

> +

> +  clock-names:

> +    description:

> +      Input clock for the sensor.

> +    items:

> +      - const: mclk

> +

> +  clock-frequency:

> +    description:

> +      Frequency of the mclk clock in Hertz.

> +    default: 37125000

> +

> +  vddo-supply:

> +    description:

> +      Definition of the regulator used as interface power supply.

> +    maxItems: 1


You don't need 'maxItems' on *-supply. It's not an array.

> +

> +  vdda-supply:

> +    description:

> +      Definition of the regulator used as analog power supply.

> +    maxItems: 1

> +

> +  vddd-supply:

> +    description:

> +      Definition of the regulator used as digital power supply.

> +    maxItems: 1

> +

> +  reset-gpios:

> +    description:

> +      The phandle and specifier for the GPIO that controls sensor reset.

> +    maxItems: 1

> +

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

> +

> +required:

> +  - compatible

> +  - reg

> +  - clocks

> +  - clock-names

> +  - clock-frequency

> +  - vddo-supply

> +  - vdda-supply

> +  - vddd-supply

> +

> +additionalProperties: false

> +

> +examples:

> +  - |

> +    #include <dt-bindings/gpio/gpio.h>

> +

> +    imx296: camera-sensor@1a {

> +        compatible = "sony,imx296";

> +        reg = <0x1a>;

> +        reset-gpios = <&msmgpio 35 GPIO_ACTIVE_LOW>;

> +        pinctrl-names = "default";

> +        pinctrl-0 = <&camera_rear_default>;

> +        clocks = <&gcc 90>;

> +        clock-names = "mclk";

> +        clock-frequency = <37125000>;

> +        vddo-supply = <&camera_vddo_1v8>;

> +        vdda-supply = <&camera_vdda_3v3>;

> +        vddd-supply = <&camera_vddd_1v2>;

> +

> +        port {

> +            imx296_ep: endpoint {

> +                remote-endpoint = <&csiphy0_ep>;

> +            };

> +        };

> +    };

> +

> +...

> -- 

> 2.17.1

>
Rob Herring Oct. 29, 2019, 10:12 p.m. UTC | #3
On Mon, Oct 28, 2019 at 02:21:15PM +0200, Sakari Ailus wrote:
> Hi Manivannan,

> 

> Thanks for the update.

> 

> On Fri, Oct 25, 2019 at 11:29:07PM +0530, Manivannan Sadhasivam wrote:

> > Add YAML devicetree binding for IMX296 CMOS image sensor.

> > 

> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

> > ---

> >  .../devicetree/bindings/media/i2c/imx296.yaml | 98 +++++++++++++++++++

> >  1 file changed, 98 insertions(+)

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

> > 

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

> > new file mode 100644

> > index 000000000000..4e204fd7cf90

> > --- /dev/null

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

> > @@ -0,0 +1,98 @@

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

> > +%YAML 1.2

> > +---

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

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

> > +

> > +title: Sony IMX296 1/2.8-Inch CMOS Image Sensor

> > +

> > +maintainers:

> > +  - Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

> > +

> > +description: |-

> > +  The Sony IMX296 is a 1/2.9-Inch active pixel type CMOS Solid-state image

> > +  sensor with square pixel array and 1.58 M effective pixels. This chip

> > +  features a global shutter with variable charge-integration time. It is

> > +  programmable through I2C and 4-wire interfaces. The sensor output is

> > +  available via CSI-2 serial data output (1 Lane).

> > +

> > +properties:

> > +  compatible:

> > +    const: sony,imx296

> > +

> > +  reg:

> > +    maxItems: 1

> > +

> > +  clocks:

> > +    maxItems: 1

> > +

> > +  clock-names:

> > +    description:

> > +      Input clock for the sensor.

> > +    items:

> > +      - const: mclk

> > +

> > +  clock-frequency:

> > +    description:

> > +      Frequency of the mclk clock in Hertz.

> > +    default: 37125000

> 

> I think you could omit the default.


Yes, there's no default if it is required.
 
> > +

> > +  vddo-supply:

> > +    description:

> > +      Definition of the regulator used as interface power supply.

> > +    maxItems: 1

> > +

> > +  vdda-supply:

> > +    description:

> > +      Definition of the regulator used as analog power supply.

> > +    maxItems: 1

> > +

> > +  vddd-supply:

> > +    description:

> > +      Definition of the regulator used as digital power supply.

> > +    maxItems: 1

> > +

> > +  reset-gpios:

> > +    description:

> > +      The phandle and specifier for the GPIO that controls sensor reset.

> > +    maxItems: 1

> > +

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


details on what?

> > +

> > +required:

> > +  - compatible

> > +  - reg

> > +  - clocks

> > +  - clock-names

> > +  - clock-frequency

> > +  - vddo-supply

> > +  - vdda-supply

> > +  - vddd-supply

> 

> I think the port and endpoint nodes should documented here as well.


port yes, but endpoint no. Unless you have endpoint properties other 
than remote-endpoint or reg.

> 

> > +

> > +additionalProperties: false

> > +

> > +examples:

> > +  - |

> > +    #include <dt-bindings/gpio/gpio.h>

> > +

> > +    imx296: camera-sensor@1a {

> > +        compatible = "sony,imx296";

> > +        reg = <0x1a>;

> > +        reset-gpios = <&msmgpio 35 GPIO_ACTIVE_LOW>;

> > +        pinctrl-names = "default";

> > +        pinctrl-0 = <&camera_rear_default>;

> > +        clocks = <&gcc 90>;

> > +        clock-names = "mclk";

> > +        clock-frequency = <37125000>;

> > +        vddo-supply = <&camera_vddo_1v8>;

> > +        vdda-supply = <&camera_vdda_3v3>;

> > +        vddd-supply = <&camera_vddd_1v2>;

> > +

> > +        port {

> > +            imx296_ep: endpoint {

> > +                remote-endpoint = <&csiphy0_ep>;

> > +            };

> > +        };

> > +    };

> > +

> > +...

> 

> -- 

> Regards,

> 

> Sakari Ailus