| Message ID | 20231012193737.7251-1-laurent.pinchart@ideasonboard.com |
|---|---|
| Headers | show |
| Series | media: i2c: Add driver for THine THP7312 ISP | expand |
Hi Krzysztof, On Thu, Oct 12, 2023 at 09:57:38PM +0200, Krzysztof Kozlowski wrote: > On 12/10/2023 21:37, Laurent Pinchart wrote: > > Thanks for the changes You're welcome. Sorry again for missing some of your review comments on v1. > > + > > + port: > > + $ref: /schemas/graph.yaml#/$defs/port-base > > + additionalProperties: false > > + > > + properties: > > + endpoint: > > + $ref: /schemas/media/video-interfaces.yaml# > > + unevaluatedProperties: false > > + > > + properties: > > + data-lanes: > > + description: > > + This property is for lane reordering between the THP7312 and the > > + SoC. The sensor supports either two-lane, or four-lane operation. > > + If this property is omitted four-lane operation is assumed. For > > + two-lane operation the property must be set to <1 2>. > > + minItems: 2 > > + maxItems: 4 > > + items: > > + maximum: 4 > > + > > + sensors: > > + type: object > > + description: List of connected sensors > > I don't understand why do you list sensors here. From the binding > description I understood these are external sensors, which usually sit > on I2C bus. Good question :-) The sensors connected to the THP7312 input are controlled over I2C by the THP7312 itself. The host operating system doesn't have access to that I2C bus. The sensors are listed here because their power supplies need to be controlled by the host operating system. > > + > > + properties: > > + "#address-cells": > > + const: 1 > > + > > + "#size-cells": > > + const: 0 > > + > > + patternProperties: > > + "^sensor@[01]": > > + type: object > > + description: > > + Sensors connected to the first and second input, with one node per > > + sensor. > > + > > + properties: > > + thine,model: > > + $ref: /schemas/types.yaml#/definitions/string > > + description: > > + Model of the connected sensors. Must be a valid compatible string. > > Then why this isn't compatible? We picked a vendor-specific property to avoid implying that the sensor nodes will result in devices being created by the host operating system. I don't mind using "compatible" instead, but as far as I understand, a compatible string implies that corresponding device DT bindings should exist, and that won't be the case here necessarily. > > + > > + reg: > > + maxItems: 1 > > + description: THP7312 input port number > > + > > + data-lanes: > > + $ref: /schemas/media/video-interfaces.yaml#/properties/data-lanes > > + items: > > + maxItems: 4 > > + description: > > + This property is for lane reordering between the THP7312 and the imaging > > + sensor that it is connected to. > > + > > + patternProperties: > > + ".*-supply": > > + description: Power supplies for the sensor > > + > > + required: > > + - reg > > + - data-lanes > > + > > + additionalProperties: false > > + > > + required: > > + - "#address-cells" > > + - "#size-cells" > > + > > + additionalProperties: false > > + > > +required: > > + - compatible > > + - reg > > + - reset-gpios > > + - clocks > > + - vddcore-supply > > + - vhtermrx-supply > > + - vddtx-supply > > + - vddhost-supply > > + - vddcmos-supply > > + - vddgpio-0-supply > > + - vddgpio-1-supply > > + - sensors > > + - port > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + #include <dt-bindings/gpio/gpio.h> > > + > > + i2c { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + camera@61 { > > + compatible = "thine,thp7312"; > > + reg = <0x61>; > > + > > + pinctrl-names = "default"; > > + pinctrl-0 = <&cam1_pins_default>; > > + > > + reset-gpios = <&pio 119 GPIO_ACTIVE_LOW>; > > + clocks = <&camera61_clk>; > > + > > + vddcore-supply = <&vsys_v4p2>; > > + vhtermrx-supply = <&vsys_v4p2>; > > + vddtx-supply = <&vsys_v4p2>; > > + vddhost-supply = <&vsys_v4p2>; > > + vddcmos-supply = <&vsys_v4p2>; > > + vddgpio-0-supply = <&vsys_v4p2>; > > + vddgpio-1-supply = <&vsys_v4p2>; > > + > > + orientation = <0>; > > + rotation = <0>; > > + > > + sensors { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + sensor@0 { > > + thine,model = "sony,imx258"; > > + reg = <0>; > > + > > + data-lanes = <4 1 3 2>; > > + > > + dovdd-supply = <&vsys_v4p2>; > > + avdd-supply = <&vsys_v4p2>; > > + dvdd-supply = <&vsys_v4p2>; > > + }; > > + }; > > + > > + port { > > + thp7312_2_endpoint: endpoint { > > + remote-endpoint = <&mipi_thp7312_2>; > > + data-lanes = <4 2 1 3>; > > + }; > > + }; > > + }; > > + };
Hello, This patch series adds a new driver for the THine THP7312 ISP. It has been tested on an OLogic Pumpkin i350, which has a Mediatek MT8365 SoC, with the THine THSCG101 camera module. Technically the driver itself (and its bindings) have no dependencies, but to run/test this on the Pumpkin i350 with the mainline kernel, a number of patches are needed to support the board and the MT8365 SoC. Some of those patches are on their way to mainline, and some, like the Pumpkin i350 board device tree, will require more work. For convenience and reference, the needed patches are available in [1]. [1] https://git.kernel.org/pub/scm/linux/kernel/git/pinchartl/linux.git/log/?h=mtk/v6.6/pumpkin/camera Example overlays for DT integration of the THP7312 are available in that branch, in arch/arm64/boot/dts/mediatek/mt8365-pumpkin-csi0-thp7312-imx258.dtso and arch/arm64/boot/dts/mediatek/mt8365-pumpkin-csi1-thp7312-imx258.dtso. Compared to v2, all comments received for the DT bindings in v1 have now been properly taken into account (as far as I can tell). The driver has also been improved slightly. Please see individual patches for detailed changelogs. I also managed to get the mainline kernel running properly on the Pumpkin board, so this series has been properly tested on v6.6-rc5. Below is the mandatory v4l2-compliance report. Careful readers may notice that my v4l2-utils version is three commits behind upstream, but that makes no practical difference as those commits are not related to v4l2-compliance. root@buildroot ~ # v4l2-compliance -u /dev/v4l-subdev2 v4l2-compliance 1.25.0-5097, 64 bits, 64-bit time_t v4l2-compliance SHA: b79e00a74fde 2023-09-13 07:19:23 Compliance test for device /dev/v4l-subdev2: Driver Info: Driver version : 6.6.0 Capabilities : 0x00000000 Required ioctls: test VIDIOC_SUDBEV_QUERYCAP: OK test invalid ioctls: OK Allow for multiple opens: test second /dev/v4l-subdev2 open: OK test VIDIOC_SUBDEV_QUERYCAP: OK test for unlimited opens: OK Debug ioctls: test VIDIOC_LOG_STATUS: OK Input ioctls: test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported) test VIDIOC_G/S_FREQUENCY: OK (Not Supported) test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported) test VIDIOC_ENUMAUDIO: OK (Not Supported) test VIDIOC_G/S/ENUMINPUT: OK (Not Supported) test VIDIOC_G/S_AUDIO: OK (Not Supported) Inputs: 0 Audio Inputs: 0 Tuners: 0 Output ioctls: test VIDIOC_G/S_MODULATOR: OK (Not Supported) test VIDIOC_G/S_FREQUENCY: OK (Not Supported) test VIDIOC_ENUMAUDOUT: OK (Not Supported) test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported) test VIDIOC_G/S_AUDOUT: OK (Not Supported) Outputs: 0 Audio Outputs: 0 Modulators: 0 Input/Output configuration ioctls: test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported) test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported) test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported) test VIDIOC_G/S_EDID: OK (Not Supported) Control ioctls: test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK test VIDIOC_QUERYCTRL: OK test VIDIOC_G/S_CTRL: OK test VIDIOC_G/S/TRY_EXT_CTRLS: OK test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK test VIDIOC_G/S_JPEGCOMP: OK (Not Supported) Standard Controls: 17 Private Controls: 4 Format ioctls: test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK (Not Supported) test VIDIOC_G/S_PARM: OK (Not Supported) test VIDIOC_G_FBUF: OK (Not Supported) test VIDIOC_G_FMT: OK (Not Supported) test VIDIOC_TRY_FMT: OK (Not Supported) test VIDIOC_S_FMT: OK (Not Supported) test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported) test Cropping: OK (Not Supported) test Composing: OK (Not Supported) test Scaling: OK (Not Supported) Codec ioctls: test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported) test VIDIOC_G_ENC_INDEX: OK (Not Supported) test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported) Buffer ioctls: test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK (Not Supported) test VIDIOC_EXPBUF: OK (Not Supported) test Requests: OK (Not Supported) Total for device /dev/v4l-subdev2: 43, Succeeded: 43, Failed: 0, Warnings: 0 Laurent Pinchart (1): media: uapi: Add controls for the THP7312 ISP Paul Elder (2): dt-bindings: media: Add bindings for THine THP7312 ISP media: i2c: Add driver for THine THP7312 .../bindings/media/i2c/thine,thp7312.yaml | 226 ++ .../userspace-api/media/drivers/index.rst | 1 + .../userspace-api/media/drivers/thp7312.rst | 32 + MAINTAINERS | 10 + drivers/media/i2c/Kconfig | 16 + drivers/media/i2c/Makefile | 1 + drivers/media/i2c/thp7312.c | 2330 +++++++++++++++++ include/uapi/linux/thp7312.h | 19 + include/uapi/linux/v4l2-controls.h | 6 + 9 files changed, 2641 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/i2c/thine,thp7312.yaml create mode 100644 Documentation/userspace-api/media/drivers/thp7312.rst create mode 100644 drivers/media/i2c/thp7312.c create mode 100644 include/uapi/linux/thp7312.h base-commit: a1766a4fd83befa0b34d932d532e7ebb7fab1fa7