mbox series

[v2,00/20] media: i2c: imx290: Miscellaneous improvements

Message ID 20221016061523.30127-1-laurent.pinchart@ideasonboard.com
Headers show
Series media: i2c: imx290: Miscellaneous improvements | expand

Message

Laurent Pinchart Oct. 16, 2022, 6:15 a.m. UTC
Hello,

This patch series gathers miscellaneous improvements for the imx290
driver. The most notable changes on the kernel side is patch 08/20 that
simplifies register access, and on the userspace API side patches 15/20,
16/20 and 19/20 that extend the driver with controls and selection
rectangles required by libcamera.

Laurent Pinchart (20):
  media: dt-bindings: Convert imx290.txt to YAML
  media: i2c: imx290: Use device lock for the control handler
  media: i2c: imx290: Print error code when I2C transfer fails
  media: i2c: imx290: Replace macro with explicit ARRAY_SIZE()
  media: i2c: imx290: Drop imx290_write_buffered_reg()
  media: i2c: imx290: Drop regmap cache
  media: i2c: imx290: Specify HMAX values in decimal
  media: i2c: imx290: Support variable-sized registers
  media: i2c: imx290: Correct register sizes
  media: i2c: imx290: Simplify error handling when writing registers
  media: i2c: imx290: Define more register macros
  media: i2c: imx290: Add exposure time control
  media: i2c: imx290: Fix max gain value
  media: i2c: imx290: Split control initialization to separate function
  media: i2c: imx290: Implement HBLANK and VBLANK controls
  media: i2c: imx290: Create controls for fwnode properties
  media: i2c: imx290: Move registers with fixed value to init array
  media: i2c: imx290: Factor out format retrieval to separate function
  media: i2c: imx290: Add crop selection targets support
  media: i2c: imx290: Replace GAIN control with ANALOGUE_GAIN

 .../devicetree/bindings/media/i2c/imx290.txt  |  57 --
 .../bindings/media/i2c/sony,imx290.yaml       | 129 +++
 MAINTAINERS                                   |   2 +-
 drivers/media/i2c/imx290.c                    | 784 ++++++++++--------
 4 files changed, 591 insertions(+), 381 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/media/i2c/imx290.txt
 create mode 100644 Documentation/devicetree/bindings/media/i2c/sony,imx290.yaml

Comments

Dave Stevenson Oct. 17, 2022, 1:57 p.m. UTC | #1
Hi Laurent

On Sun, 16 Oct 2022 at 07:15, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Convert the Sony IMX290 DT binding from text to YAML. Add Manivannan as
> a maintainer given that he is listed in MAINTAINERS for the file, as
> volunteering myself.
>
> The name of the input clock, "xclk", is wrong as the hardware manual
> names it INCK. As the device has a single clock, the name could be
> omitted, but that would require a corresponding change to the driver and
> is thus a candidate for further patches.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  .../devicetree/bindings/media/i2c/imx290.txt  |  57 --------
>  .../bindings/media/i2c/sony,imx290.yaml       | 129 ++++++++++++++++++
>  MAINTAINERS                                   |   2 +-
>  3 files changed, 130 insertions(+), 58 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/media/i2c/imx290.txt
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/sony,imx290.yaml
>
> diff --git a/Documentation/devicetree/bindings/media/i2c/imx290.txt b/Documentation/devicetree/bindings/media/i2c/imx290.txt
> deleted file mode 100644
> index a3cc21410f7c..000000000000
> --- a/Documentation/devicetree/bindings/media/i2c/imx290.txt
> +++ /dev/null
> @@ -1,57 +0,0 @@
> -* Sony IMX290 1/2.8-Inch CMOS Image Sensor
> -
> -The Sony IMX290 is a 1/2.8-Inch CMOS Solid-state image sensor with
> -Square Pixel for Color Cameras. It is programmable through I2C and 4-wire
> -interfaces. The sensor output is available via CMOS logic parallel SDR output,
> -Low voltage LVDS DDR output and CSI-2 serial data output. The CSI-2 bus is the
> -default. No bindings have been defined for the other busses.
> -
> -Required Properties:
> -- compatible: Should be "sony,imx290"
> -- reg: I2C bus address of the device
> -- clocks: Reference to the xclk clock.
> -- clock-names: Should be "xclk".
> -- clock-frequency: Frequency of the xclk clock in Hz.
> -- vdddo-supply: Sensor digital IO regulator.
> -- vdda-supply: Sensor analog regulator.
> -- vddd-supply: Sensor digital core regulator.
> -
> -Optional Properties:
> -- reset-gpios: Sensor reset GPIO
> -
> -The imx290 device node should contain one 'port' child node with
> -an 'endpoint' subnode. For further reading on port node refer to
> -Documentation/devicetree/bindings/media/video-interfaces.txt.
> -
> -Required Properties on endpoint:
> -- data-lanes: check ../video-interfaces.txt
> -- link-frequencies: check ../video-interfaces.txt
> -- remote-endpoint: check ../video-interfaces.txt
> -
> -Example:
> -       &i2c1 {
> -               ...
> -               imx290: camera-sensor@1a {
> -                       compatible = "sony,imx290";
> -                       reg = <0x1a>;
> -
> -                       reset-gpios = <&msmgpio 35 GPIO_ACTIVE_LOW>;
> -                       pinctrl-names = "default";
> -                       pinctrl-0 = <&camera_rear_default>;
> -
> -                       clocks = <&gcc GCC_CAMSS_MCLK0_CLK>;
> -                       clock-names = "xclk";
> -                       clock-frequency = <37125000>;
> -
> -                       vdddo-supply = <&camera_vdddo_1v8>;
> -                       vdda-supply = <&camera_vdda_2v8>;
> -                       vddd-supply = <&camera_vddd_1v5>;
> -
> -                       port {
> -                               imx290_ep: endpoint {
> -                                       data-lanes = <1 2 3 4>;
> -                                       link-frequencies = /bits/ 64 <445500000>;
> -                                       remote-endpoint = <&csiphy0_ep>;
> -                               };
> -                       };
> -               };
> diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx290.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx290.yaml
> new file mode 100644
> index 000000000000..21377daae026
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx290.yaml
> @@ -0,0 +1,129 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/i2c/sony,imx290.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Sony IMX290 1/2.8-Inch CMOS Image Sensor
> +
> +maintainers:
> +  - Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> +  - Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> +
> +description: |-
> +  The Sony IMX290 is a 1/2.8-Inch CMOS Solid-state image sensor with Square
> +  Pixel for Color Cameras. It is programmable through I2C and 4-wire
> +  interfaces. The sensor output is available via CMOS logic parallel SDR
> +  output, Low voltage LVDS DDR output and CSI-2 serial data output. The CSI-2
> +  bus is the default. No bindings have been defined for the other busses.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - sony,imx290
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  clock-names:
> +    description: Input clock (37.125 MHz or 74.25 MHz)
> +    items:
> +      - const: xclk
> +
> +  clock-frequency:
> +    description: Frequency of the xclk clock in Hz
> +
> +  vdda-supply:
> +    description: Analog power supply (2.9V)
> +
> +  vddd-supply:
> +    description: Digital core power supply (1.2V)
> +
> +  vdddo-supply:
> +    description: Digital I/O power supply (1.8V)
> +
> +  reset-gpios:
> +    description: Sensor reset (XCLR) GPIO
> +    maxItems: 1
> +
> +  port:
> +    $ref: /schemas/graph.yaml#/$defs/port-base
> +    description: |
> +      Video output port
> +
> +    properties:
> +      endpoint:
> +        $ref: /schemas/media/video-interfaces.yaml#
> +        unevaluatedProperties: false
> +
> +        properties:
> +          data-lanes:
> +            anyOf:
> +              - items:
> +                  - const: 1
> +                  - const: 2
> +              - items:
> +                  - const: 1
> +                  - const: 2
> +                  - const: 3
> +                  - const: 4
> +
> +          link-frequencies: true
> +
> +        required:
> +          - data-lanes
> +          - link-frequencies
> +
> +    additionalProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - clock-frequency
> +  - vdda-supply
> +  - vddd-supply
> +  - vdddo-supply
> +  - port
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        imx290: camera-sensor@1a {
> +            compatible = "sony,imx290";
> +            reg = <0x1a>;
> +
> +            pinctrl-names = "default";
> +            pinctrl-0 = <&camera_rear_default>;
> +
> +            clocks = <&gcc 90>;
> +            clock-names = "xclk";
> +            clock-frequency = <37125000>;
> +
> +            vdddo-supply = <&camera_vdddo_1v8>;
> +            vdda-supply = <&camera_vdda_2v8>;
> +            vddd-supply = <&camera_vddd_1v5>;
> +
> +            reset-gpios = <&msmgpio 35 GPIO_ACTIVE_LOW>;
> +
> +            port {
> +                imx290_ep: endpoint {
> +                    data-lanes = <1 2 3 4>;
> +                    link-frequencies = /bits/ 64 <445500000>;

Minor nit that this won't work with the current Linux driver due to a
driver restriction implementing the recommended settings from Sony.
OV8865 has the same restrictions and notes it in the binding[1]. I
don't know if this is the preferred approach or not.

  Dave

[1] https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/media/i2c/ov8856.yaml#L78

> +                    remote-endpoint = <&csiphy0_ep>;
> +                };
> +            };
> +        };
> +    };
> +...
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 72b9654f764c..c431fd20e89b 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -18982,7 +18982,7 @@ M:      Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
>  L:     linux-media@vger.kernel.org
>  S:     Maintained
>  T:     git git://linuxtv.org/media_tree.git
> -F:     Documentation/devicetree/bindings/media/i2c/imx290.txt
> +F:     Documentation/devicetree/bindings/media/i2c/sony,imx290.yaml
>  F:     drivers/media/i2c/imx290.c
>
>  SONY IMX319 SENSOR DRIVER
> --
> Regards,
>
> Laurent Pinchart
>
Sakari Ailus Oct. 25, 2022, 2:12 p.m. UTC | #2
Hi Dave,

On Mon, Oct 17, 2022 at 02:57:58PM +0100, Dave Stevenson wrote:
> > +                    link-frequencies = /bits/ 64 <445500000>;
> 
> Minor nit that this won't work with the current Linux driver due to a
> driver restriction implementing the recommended settings from Sony.
> OV8865 has the same restrictions and notes it in the binding[1]. I
> don't know if this is the preferred approach or not.

The DT describes hardware and is not bound by (current) driver limitations.
So I'd say that if the hardware supports multiple frequencies, the
supported ones need to be listed here.

I'd also like to merge the series, it's been out of tree for long already.