| Message ID | 20191227122114.23075-2-andrey.konovalov@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add IMX219 CMOS image sensor support | expand |
Hi Andrey, Thanks for the patchset. On Fri, Dec 27, 2019 at 03:21:13PM +0300, Andrey Konovalov wrote: > Add YAML device tree binding for IMX219 CMOS image sensor, and > the relevant MAINTAINERS entries. > > Signed-off-by: Andrey Konovalov <andrey.konovalov@linaro.org> > --- > .../devicetree/bindings/media/i2c/imx219.yaml | 134 ++++++++++++++++++ > MAINTAINERS | 8 ++ > 2 files changed, 142 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/i2c/imx219.yaml > > diff --git a/Documentation/devicetree/bindings/media/i2c/imx219.yaml b/Documentation/devicetree/bindings/media/i2c/imx219.yaml > new file mode 100644 > index 000000000000..b58aa49a7c03 > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/i2c/imx219.yaml > @@ -0,0 +1,134 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/media/i2c/imx219.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Sony 1/4.0-Inch 8Mpixel CMOS Digital Image Sensor > + > +maintainers: > + - Dave Stevenson <dave.stevenson@raspberrypi.com> > + > +description: |- > + The Sony imx219 is a 1/4.0-inch CMOS active pixel digital image sensor > + with an active array size of 3280H x 2464V. It is programmable through > + I2C interface. The I2C address is fixed to 0x10 as per sensor data sheet. > + Image data is sent through MIPI CSI-2, which is configured as either 2 or > + 4 data lanes. > + > +properties: > + compatible: > + const: sony,imx219 > + > + reg: > + description: I2C device address > + maxItems: 1 > + > + clocks: > + maxItems: 1 > + > + clock-names: > + items: > + - const: xclk There's a single clock. Does it need a name? I'd just omit it. > + > + VDIG-supply: > + description: > + Digital I/O voltage supply, 1.8 volts > + > + VANA-supply: > + description: > + Analog voltage supply, 2.8 volts > + > + VDDL-supply: > + description: > + Digital core voltage supply, 1.2 volts > + > + xclr-gpios: > + description: |- > + Reference to the GPIO connected to the xclr pin, if any. > + Must be released (set high) after all supplies are applied. A common name for this in camera sensors is xshutdown. I'd suggest to use that. > + > + camera-clk: > + type: object > + > + description: Clock source for imx219 > + > + properties: > + clock-frequency: true > + > + required: > + - clock-frequency Hmm. The driver doesn't seem to use this for anything. There are two approaches to this; either you can get and check the frequency, or specify it in DT bindings, set and then check it. See e.g. Documentation/devicetree/bindings/media/i2c/nokia,smia.txt (not in YAML though). > + > + # See ../video-interfaces.txt for more details > + port: > + type: object > + properties: > + endpoint: > + type: object > + properties: > + clock-lanes: > + const: 0 If the hardware does not support lane reordering, you can omit the clock-lanes property as it provides no information. > + > + data-lanes: > + description: |- > + Should be <1 2> for two-lane operation, or <1 2 3 4> for > + four-lane operation. > + oneOf: > + - const: [[ 1, 2 ]] > + - const: [[ 1, 2, 3, 4 ]] > + > + clock-noncontinuous: > + type: boolean > + description: |- > + Presence of this boolean property decides whether the MIPI CSI-2 > + clock is continuous or non-continuous. How about: MIPI CSI-2 clock will be non-continuous if this property is present, otherwise it's continuous. > + > + required: > + - clock-lanes > + - data-lanes > + > +required: > + - compatible > + - reg > + - clocks > + - clock-names > + - VANA-supply > + - VDIG-supply > + - VDDL-supply > + - port > + > +additionalProperties: false > + > +examples: > + - | > + i2c0 { > + #address-cells = <1>; > + #size-cells = <0>; > + > + imx219: sensor@10 { > + compatible = "sony,imx219"; > + reg = <0x10>; > + clocks = <&imx219_clk>; > + clock-names = "xclk"; > + VANA-supply = <&imx219_vana>; /* 2.8v */ > + VDIG-supply = <&imx219_vdig>; /* 1.8v */ > + VDDL-supply = <&imx219_vddl>; /* 1.2v */ > + > + imx219_clk: camera-clk { > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + clock-frequency = <24000000>; > + }; > + > + port { > + imx219_0: endpoint { > + remote-endpoint = <&csi1_ep>; > + clock-lanes = <0>; > + data-lanes = <1 2>; > + clock-noncontinuous; > + }; > + }; > + }; > + }; > + > +... > diff --git a/MAINTAINERS b/MAINTAINERS > index ffa3371bc750..f7b6c24ec081 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -15350,6 +15350,14 @@ S: Maintained > F: drivers/media/i2c/imx214.c > F: Documentation/devicetree/bindings/media/i2c/sony,imx214.txt > > +SONY IMX219 SENSOR DRIVER > +M: Dave Stevenson <dave.stevenson@raspberrypi.com> Is Dave aware of this? :-) > +L: linux-media@vger.kernel.org > +T: git git://linuxtv.org/media_tree.git > +S: Maintained > +F: drivers/media/i2c/imx219.c > +F: Documentation/devicetree/bindings/media/i2c/imx219.yaml > + > SONY IMX258 SENSOR DRIVER > M: Sakari Ailus <sakari.ailus@linux.intel.com> > L: linux-media@vger.kernel.org
Hi Sakari, Thank you for reviewing the patchset, and for the pointers on improving the driver (nokia,smia.txt etc)! I'll write a separate email later, or just fix what you suggested in v3 (I agree with the proposed changes I didn't comment on in this email). Just few quick answers below. Thanks, Andrey On 27.12.2019 17:17, Sakari Ailus wrote: > Hi Andrey, > > Thanks for the patchset. > > On Fri, Dec 27, 2019 at 03:21:13PM +0300, Andrey Konovalov wrote: >> Add YAML device tree binding for IMX219 CMOS image sensor, and >> the relevant MAINTAINERS entries. >> >> Signed-off-by: Andrey Konovalov <andrey.konovalov@linaro.org> >> --- >> .../devicetree/bindings/media/i2c/imx219.yaml | 134 ++++++++++++++++++ >> MAINTAINERS | 8 ++ >> 2 files changed, 142 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/media/i2c/imx219.yaml >> >> diff --git a/Documentation/devicetree/bindings/media/i2c/imx219.yaml b/Documentation/devicetree/bindings/media/i2c/imx219.yaml >> new file mode 100644 >> index 000000000000..b58aa49a7c03 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/media/i2c/imx219.yaml >> @@ -0,0 +1,134 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/media/i2c/imx219.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Sony 1/4.0-Inch 8Mpixel CMOS Digital Image Sensor >> + >> +maintainers: >> + - Dave Stevenson <dave.stevenson@raspberrypi.com> >> + >> +description: |- >> + The Sony imx219 is a 1/4.0-inch CMOS active pixel digital image sensor >> + with an active array size of 3280H x 2464V. It is programmable through >> + I2C interface. The I2C address is fixed to 0x10 as per sensor data sheet. >> + Image data is sent through MIPI CSI-2, which is configured as either 2 or >> + 4 data lanes. >> + >> +properties: >> + compatible: >> + const: sony,imx219 >> + >> + reg: >> + description: I2C device address >> + maxItems: 1 >> + >> + clocks: >> + maxItems: 1 >> + >> + clock-names: >> + items: >> + - const: xclk > > There's a single clock. Does it need a name? I'd just omit it. > >> + >> + VDIG-supply: >> + description: >> + Digital I/O voltage supply, 1.8 volts >> + >> + VANA-supply: >> + description: >> + Analog voltage supply, 2.8 volts >> + >> + VDDL-supply: >> + description: >> + Digital core voltage supply, 1.2 volts >> + >> + xclr-gpios: >> + description: |- >> + Reference to the GPIO connected to the xclr pin, if any. >> + Must be released (set high) after all supplies are applied. > > A common name for this in camera sensors is xshutdown. I'd suggest to use > that. Indeed, "xshutdown" is the pin name commonly used by OmniVision for their sensors. (In older sensors they used "pwdn" which is similar, but the polarity is reversed.) In their sensor datasheets Sony consistently use "xclr" for the pin and signal otherwise very similar to OmniVision's "xshutdown". Wouldn't using the signal name from the sensor by the different vendor just add more confusion instead? >> + >> + camera-clk: >> + type: object >> + >> + description: Clock source for imx219 >> + >> + properties: >> + clock-frequency: true >> + >> + required: >> + - clock-frequency > > Hmm. The driver doesn't seem to use this for anything. > > There are two approaches to this; either you can get and check the > frequency, or specify it in DT bindings, set and then check it. > > See e.g. Documentation/devicetree/bindings/media/i2c/nokia,smia.txt (not in > YAML though). > >> + >> + # See ../video-interfaces.txt for more details >> + port: >> + type: object >> + properties: >> + endpoint: >> + type: object >> + properties: >> + clock-lanes: >> + const: 0 > > If the hardware does not support lane reordering, you can omit the > clock-lanes property as it provides no information. > >> + >> + data-lanes: >> + description: |- >> + Should be <1 2> for two-lane operation, or <1 2 3 4> for >> + four-lane operation. >> + oneOf: >> + - const: [[ 1, 2 ]] >> + - const: [[ 1, 2, 3, 4 ]] >> + >> + clock-noncontinuous: >> + type: boolean >> + description: |- >> + Presence of this boolean property decides whether the MIPI CSI-2 >> + clock is continuous or non-continuous. > > How about: MIPI CSI-2 clock will be non-continuous if this property is > present, otherwise it's continuous. This statement is more clear than the original. Thanks! >> + >> + required: >> + - clock-lanes >> + - data-lanes >> + >> +required: >> + - compatible >> + - reg >> + - clocks >> + - clock-names >> + - VANA-supply >> + - VDIG-supply >> + - VDDL-supply >> + - port >> + >> +additionalProperties: false >> + >> +examples: >> + - | >> + i2c0 { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + imx219: sensor@10 { >> + compatible = "sony,imx219"; >> + reg = <0x10>; >> + clocks = <&imx219_clk>; >> + clock-names = "xclk"; >> + VANA-supply = <&imx219_vana>; /* 2.8v */ >> + VDIG-supply = <&imx219_vdig>; /* 1.8v */ >> + VDDL-supply = <&imx219_vddl>; /* 1.2v */ >> + >> + imx219_clk: camera-clk { >> + compatible = "fixed-clock"; >> + #clock-cells = <0>; >> + clock-frequency = <24000000>; >> + }; >> + >> + port { >> + imx219_0: endpoint { >> + remote-endpoint = <&csi1_ep>; >> + clock-lanes = <0>; >> + data-lanes = <1 2>; >> + clock-noncontinuous; >> + }; >> + }; >> + }; >> + }; >> + >> +... >> diff --git a/MAINTAINERS b/MAINTAINERS >> index ffa3371bc750..f7b6c24ec081 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -15350,6 +15350,14 @@ S: Maintained >> F: drivers/media/i2c/imx214.c >> F: Documentation/devicetree/bindings/media/i2c/sony,imx214.txt >> >> +SONY IMX219 SENSOR DRIVER >> +M: Dave Stevenson <dave.stevenson@raspberrypi.com> > > Is Dave aware of this? :-) Yes, he is :) But I forgot to add him to Cc this time. My bad.. Thanks, Andrey >> +L: linux-media@vger.kernel.org >> +T: git git://linuxtv.org/media_tree.git >> +S: Maintained >> +F: drivers/media/i2c/imx219.c >> +F: Documentation/devicetree/bindings/media/i2c/imx219.yaml >> + >> SONY IMX258 SENSOR DRIVER >> M: Sakari Ailus <sakari.ailus@linux.intel.com> >> L: linux-media@vger.kernel.org >
Hi Andrey, On Fri, Dec 27, 2019 at 09:26:25PM +0300, Andrey Konovalov wrote: > Hi Sakari, > > Thank you for reviewing the patchset, and for the pointers on improving the driver (nokia,smia.txt etc)! > I'll write a separate email later, or just fix what you suggested in v3 (I agree with the proposed changes > I didn't comment on in this email). Ack. Please wrap the lines around 72 chars or so. > > Just few quick answers below. > > Thanks, > Andrey > > On 27.12.2019 17:17, Sakari Ailus wrote: > > Hi Andrey, > > > > Thanks for the patchset. > > > > On Fri, Dec 27, 2019 at 03:21:13PM +0300, Andrey Konovalov wrote: > > > Add YAML device tree binding for IMX219 CMOS image sensor, and > > > the relevant MAINTAINERS entries. > > > > > > Signed-off-by: Andrey Konovalov <andrey.konovalov@linaro.org> > > > --- > > > .../devicetree/bindings/media/i2c/imx219.yaml | 134 ++++++++++++++++++ > > > MAINTAINERS | 8 ++ > > > 2 files changed, 142 insertions(+) > > > create mode 100644 Documentation/devicetree/bindings/media/i2c/imx219.yaml > > > > > > diff --git a/Documentation/devicetree/bindings/media/i2c/imx219.yaml b/Documentation/devicetree/bindings/media/i2c/imx219.yaml > > > new file mode 100644 > > > index 000000000000..b58aa49a7c03 > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/media/i2c/imx219.yaml > > > @@ -0,0 +1,134 @@ > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > +%YAML 1.2 > > > +--- > > > +$id: http://devicetree.org/schemas/media/i2c/imx219.yaml# > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > + > > > +title: Sony 1/4.0-Inch 8Mpixel CMOS Digital Image Sensor > > > + > > > +maintainers: > > > + - Dave Stevenson <dave.stevenson@raspberrypi.com> > > > + > > > +description: |- > > > + The Sony imx219 is a 1/4.0-inch CMOS active pixel digital image sensor > > > + with an active array size of 3280H x 2464V. It is programmable through > > > + I2C interface. The I2C address is fixed to 0x10 as per sensor data sheet. > > > + Image data is sent through MIPI CSI-2, which is configured as either 2 or > > > + 4 data lanes. > > > + > > > +properties: > > > + compatible: > > > + const: sony,imx219 > > > + > > > + reg: > > > + description: I2C device address > > > + maxItems: 1 > > > + > > > + clocks: > > > + maxItems: 1 > > > + > > > + clock-names: > > > + items: > > > + - const: xclk > > > > There's a single clock. Does it need a name? I'd just omit it. > > > > > + > > > + VDIG-supply: > > > + description: > > > + Digital I/O voltage supply, 1.8 volts > > > + > > > + VANA-supply: > > > + description: > > > + Analog voltage supply, 2.8 volts > > > + > > > + VDDL-supply: > > > + description: > > > + Digital core voltage supply, 1.2 volts > > > + > > > + xclr-gpios: > > > + description: |- > > > + Reference to the GPIO connected to the xclr pin, if any. > > > + Must be released (set high) after all supplies are applied. > > > > A common name for this in camera sensors is xshutdown. I'd suggest to use > > that. > > Indeed, "xshutdown" is the pin name commonly used by OmniVision for their sensors. Some Aptina (OnSemi) devices also use xshutdown, and so does the CCS standard. > (In older sensors they used "pwdn" which is similar, but the polarity is reversed.) Some devices have both xshutdown and pwdn. I don't see why; they're controlled together in practice anyway. > > In their sensor datasheets Sony consistently use "xclr" for the pin and signal otherwise > very similar to OmniVision's "xshutdown". > > Wouldn't using the signal name from the sensor by the different vendor just add more confusion > instead? What matters is the functionality really. I checked the existing bindings, and it seems devices using "xshutdown" document reset-gpios property under Documentation/devicetree/bindings/media/i2c . None refers to xclr. So "reset-gpios" is the right name. The vast majority are active low (i.e. lifting the sensor from reset requires high output) but there's at least one that is active high. > > > > + > > > + camera-clk: > > > + type: object > > > + > > > + description: Clock source for imx219 > > > + > > > + properties: > > > + clock-frequency: true > > > + > > > + required: > > > + - clock-frequency > > > > Hmm. The driver doesn't seem to use this for anything. > > > > There are two approaches to this; either you can get and check the > > frequency, or specify it in DT bindings, set and then check it. > > > > See e.g. Documentation/devicetree/bindings/media/i2c/nokia,smia.txt (not in > > YAML though). > > > > > + > > > + # See ../video-interfaces.txt for more details > > > + port: > > > + type: object > > > + properties: > > > + endpoint: > > > + type: object > > > + properties: > > > + clock-lanes: > > > + const: 0 > > > > If the hardware does not support lane reordering, you can omit the > > clock-lanes property as it provides no information. > > > > > + > > > + data-lanes: > > > + description: |- > > > + Should be <1 2> for two-lane operation, or <1 2 3 4> for > > > + four-lane operation. > > > + oneOf: > > > + - const: [[ 1, 2 ]] > > > + - const: [[ 1, 2, 3, 4 ]] > > > + > > > + clock-noncontinuous: > > > + type: boolean > > > + description: |- > > > + Presence of this boolean property decides whether the MIPI CSI-2 > > > + clock is continuous or non-continuous. > > > > How about: MIPI CSI-2 clock will be non-continuous if this property is > > present, otherwise it's continuous. > > This statement is more clear than the original. Thanks! Perhaps: s/will be/is/ In order to keep the same tense.
On Fri, Dec 27, 2019 at 03:21:13PM +0300, Andrey Konovalov wrote: > Add YAML device tree binding for IMX219 CMOS image sensor, and > the relevant MAINTAINERS entries. > > Signed-off-by: Andrey Konovalov <andrey.konovalov@linaro.org> > --- > .../devicetree/bindings/media/i2c/imx219.yaml | 134 ++++++++++++++++++ > MAINTAINERS | 8 ++ > 2 files changed, 142 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/i2c/imx219.yaml > > diff --git a/Documentation/devicetree/bindings/media/i2c/imx219.yaml b/Documentation/devicetree/bindings/media/i2c/imx219.yaml > new file mode 100644 > index 000000000000..b58aa49a7c03 > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/i2c/imx219.yaml > @@ -0,0 +1,134 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/media/i2c/imx219.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Sony 1/4.0-Inch 8Mpixel CMOS Digital Image Sensor > + > +maintainers: > + - Dave Stevenson <dave.stevenson@raspberrypi.com> > + > +description: |- > + The Sony imx219 is a 1/4.0-inch CMOS active pixel digital image sensor > + with an active array size of 3280H x 2464V. It is programmable through > + I2C interface. The I2C address is fixed to 0x10 as per sensor data sheet. > + Image data is sent through MIPI CSI-2, which is configured as either 2 or > + 4 data lanes. > + > +properties: > + compatible: > + const: sony,imx219 > + > + reg: > + description: I2C device address > + maxItems: 1 > + > + clocks: > + maxItems: 1 > + > + clock-names: > + items: > + - const: xclk > + > + VDIG-supply: > + description: > + Digital I/O voltage supply, 1.8 volts > + > + VANA-supply: > + description: > + Analog voltage supply, 2.8 volts > + > + VDDL-supply: > + description: > + Digital core voltage supply, 1.2 volts > + > + xclr-gpios: > + description: |- > + Reference to the GPIO connected to the xclr pin, if any. > + Must be released (set high) after all supplies are applied. > + > + camera-clk: > + type: object > + > + description: Clock source for imx219 This clock is external to the sensor, so a node for a fixed clock should be too. > + > + properties: > + clock-frequency: true > + > + required: > + - clock-frequency > + > + # See ../video-interfaces.txt for more details > + port: > + type: object > + properties: > + endpoint: > + type: object > + properties: > + clock-lanes: > + const: 0 > + > + data-lanes: > + description: |- > + Should be <1 2> for two-lane operation, or <1 2 3 4> for > + four-lane operation. > + oneOf: > + - const: [[ 1, 2 ]] > + - const: [[ 1, 2, 3, 4 ]] Not sure if this actually works. If it does, it exposes how we encode the DT yaml format which we don't want to do here. It should be: oneOf: - items: - const: 1 - const: 2 - items: - const: 1 - const: 2 - const: 3 - const: 4 Really, I think you shouldn't need the 2nd case as that should be the default. > + > + clock-noncontinuous: > + type: boolean > + description: |- > + Presence of this boolean property decides whether the MIPI CSI-2 > + clock is continuous or non-continuous. > + > + required: > + - clock-lanes > + - data-lanes > + > +required: > + - compatible > + - reg > + - clocks > + - clock-names > + - VANA-supply > + - VDIG-supply > + - VDDL-supply > + - port > + > +additionalProperties: false > + > +examples: > + - | > + i2c0 { > + #address-cells = <1>; > + #size-cells = <0>; > + > + imx219: sensor@10 { > + compatible = "sony,imx219"; > + reg = <0x10>; > + clocks = <&imx219_clk>; > + clock-names = "xclk"; > + VANA-supply = <&imx219_vana>; /* 2.8v */ > + VDIG-supply = <&imx219_vdig>; /* 1.8v */ > + VDDL-supply = <&imx219_vddl>; /* 1.2v */ > + > + imx219_clk: camera-clk { > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + clock-frequency = <24000000>; > + }; > + > + port { > + imx219_0: endpoint { > + remote-endpoint = <&csi1_ep>; > + clock-lanes = <0>; > + data-lanes = <1 2>; > + clock-noncontinuous; > + }; > + }; > + }; > + }; > + > +... > diff --git a/MAINTAINERS b/MAINTAINERS > index ffa3371bc750..f7b6c24ec081 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -15350,6 +15350,14 @@ S: Maintained > F: drivers/media/i2c/imx214.c > F: Documentation/devicetree/bindings/media/i2c/sony,imx214.txt > > +SONY IMX219 SENSOR DRIVER > +M: Dave Stevenson <dave.stevenson@raspberrypi.com> > +L: linux-media@vger.kernel.org > +T: git git://linuxtv.org/media_tree.git > +S: Maintained > +F: drivers/media/i2c/imx219.c > +F: Documentation/devicetree/bindings/media/i2c/imx219.yaml > + > SONY IMX258 SENSOR DRIVER > M: Sakari Ailus <sakari.ailus@linux.intel.com> > L: linux-media@vger.kernel.org > -- > 2.17.1 >
Hi Sakari On Fri, 27 Dec 2019 at 14:18, Sakari Ailus <sakari.ailus@iki.fi> wrote: > > Hi Andrey, > > Thanks for the patchset. > > On Fri, Dec 27, 2019 at 03:21:13PM +0300, Andrey Konovalov wrote: > > Add YAML device tree binding for IMX219 CMOS image sensor, and > > the relevant MAINTAINERS entries. > > > > Signed-off-by: Andrey Konovalov <andrey.konovalov@linaro.org> > > --- > > .../devicetree/bindings/media/i2c/imx219.yaml | 134 ++++++++++++++++++ > > MAINTAINERS | 8 ++ > > 2 files changed, 142 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/media/i2c/imx219.yaml > > > > diff --git a/Documentation/devicetree/bindings/media/i2c/imx219.yaml b/Documentation/devicetree/bindings/media/i2c/imx219.yaml > > new file mode 100644 > > index 000000000000..b58aa49a7c03 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/media/i2c/imx219.yaml > > @@ -0,0 +1,134 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/media/i2c/imx219.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Sony 1/4.0-Inch 8Mpixel CMOS Digital Image Sensor > > + > > +maintainers: > > + - Dave Stevenson <dave.stevenson@raspberrypi.com> > > + > > +description: |- > > + The Sony imx219 is a 1/4.0-inch CMOS active pixel digital image sensor > > + with an active array size of 3280H x 2464V. It is programmable through > > + I2C interface. The I2C address is fixed to 0x10 as per sensor data sheet. > > + Image data is sent through MIPI CSI-2, which is configured as either 2 or > > + 4 data lanes. > > + > > +properties: > > + compatible: > > + const: sony,imx219 > > + > > + reg: > > + description: I2C device address > > + maxItems: 1 > > + > > + clocks: > > + maxItems: 1 > > + > > + clock-names: > > + items: > > + - const: xclk > > There's a single clock. Does it need a name? I'd just omit it. > > > + > > + VDIG-supply: > > + description: > > + Digital I/O voltage supply, 1.8 volts > > + > > + VANA-supply: > > + description: > > + Analog voltage supply, 2.8 volts > > + > > + VDDL-supply: > > + description: > > + Digital core voltage supply, 1.2 volts > > + > > + xclr-gpios: > > + description: |- > > + Reference to the GPIO connected to the xclr pin, if any. > > + Must be released (set high) after all supplies are applied. > > A common name for this in camera sensors is xshutdown. I'd suggest to use > that. > > > + > > + camera-clk: > > + type: object > > + > > + description: Clock source for imx219 > > + > > + properties: > > + clock-frequency: true > > + > > + required: > > + - clock-frequency > > Hmm. The driver doesn't seem to use this for anything. > > There are two approaches to this; either you can get and check the > frequency, or specify it in DT bindings, set and then check it. > > See e.g. Documentation/devicetree/bindings/media/i2c/nokia,smia.txt (not in > YAML though). > > > + > > + # See ../video-interfaces.txt for more details > > + port: > > + type: object > > + properties: > > + endpoint: > > + type: object > > + properties: > > + clock-lanes: > > + const: 0 > > If the hardware does not support lane reordering, you can omit the > clock-lanes property as it provides no information. > > > + > > + data-lanes: > > + description: |- > > + Should be <1 2> for two-lane operation, or <1 2 3 4> for > > + four-lane operation. > > + oneOf: > > + - const: [[ 1, 2 ]] > > + - const: [[ 1, 2, 3, 4 ]] > > + > > + clock-noncontinuous: > > + type: boolean > > + description: |- > > + Presence of this boolean property decides whether the MIPI CSI-2 > > + clock is continuous or non-continuous. > > How about: MIPI CSI-2 clock will be non-continuous if this property is > present, otherwise it's continuous. > > > + > > + required: > > + - clock-lanes > > + - data-lanes > > + > > +required: > > + - compatible > > + - reg > > + - clocks > > + - clock-names > > + - VANA-supply > > + - VDIG-supply > > + - VDDL-supply > > + - port > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + i2c0 { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + imx219: sensor@10 { > > + compatible = "sony,imx219"; > > + reg = <0x10>; > > + clocks = <&imx219_clk>; > > + clock-names = "xclk"; > > + VANA-supply = <&imx219_vana>; /* 2.8v */ > > + VDIG-supply = <&imx219_vdig>; /* 1.8v */ > > + VDDL-supply = <&imx219_vddl>; /* 1.2v */ > > + > > + imx219_clk: camera-clk { > > + compatible = "fixed-clock"; > > + #clock-cells = <0>; > > + clock-frequency = <24000000>; > > + }; > > + > > + port { > > + imx219_0: endpoint { > > + remote-endpoint = <&csi1_ep>; > > + clock-lanes = <0>; > > + data-lanes = <1 2>; > > + clock-noncontinuous; > > + }; > > + }; > > + }; > > + }; > > + > > +... > > diff --git a/MAINTAINERS b/MAINTAINERS > > index ffa3371bc750..f7b6c24ec081 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -15350,6 +15350,14 @@ S: Maintained > > F: drivers/media/i2c/imx214.c > > F: Documentation/devicetree/bindings/media/i2c/sony,imx214.txt > > > > +SONY IMX219 SENSOR DRIVER > > +M: Dave Stevenson <dave.stevenson@raspberrypi.com> > > Is Dave aware of this? :-) Yes, I'm aware, and happy to be listed as maintainer. I'm very grateful to Andrey for his efforts in upstreaming this - it's a task that I simply haven't the time for at present. Dave > > +L: linux-media@vger.kernel.org > > +T: git git://linuxtv.org/media_tree.git > > +S: Maintained > > +F: drivers/media/i2c/imx219.c > > +F: Documentation/devicetree/bindings/media/i2c/imx219.yaml > > + > > SONY IMX258 SENSOR DRIVER > > M: Sakari Ailus <sakari.ailus@linux.intel.com> > > L: linux-media@vger.kernel.org > > -- > Regards, > > Sakari Ailus
Hi Rob, Thanks for the review! On 05.01.2020 00:53, Rob Herring wrote: > On Fri, Dec 27, 2019 at 03:21:13PM +0300, Andrey Konovalov wrote: >> Add YAML device tree binding for IMX219 CMOS image sensor, and >> the relevant MAINTAINERS entries. >> >> Signed-off-by: Andrey Konovalov <andrey.konovalov@linaro.org> >> --- >> .../devicetree/bindings/media/i2c/imx219.yaml | 134 ++++++++++++++++++ >> MAINTAINERS | 8 ++ >> 2 files changed, 142 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/media/i2c/imx219.yaml >> >> diff --git a/Documentation/devicetree/bindings/media/i2c/imx219.yaml b/Documentation/devicetree/bindings/media/i2c/imx219.yaml >> new file mode 100644 >> index 000000000000..b58aa49a7c03 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/media/i2c/imx219.yaml >> @@ -0,0 +1,134 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/media/i2c/imx219.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Sony 1/4.0-Inch 8Mpixel CMOS Digital Image Sensor >> + >> +maintainers: >> + - Dave Stevenson <dave.stevenson@raspberrypi.com> >> + >> +description: |- >> + The Sony imx219 is a 1/4.0-inch CMOS active pixel digital image sensor >> + with an active array size of 3280H x 2464V. It is programmable through >> + I2C interface. The I2C address is fixed to 0x10 as per sensor data sheet. >> + Image data is sent through MIPI CSI-2, which is configured as either 2 or >> + 4 data lanes. >> + >> +properties: >> + compatible: >> + const: sony,imx219 >> + >> + reg: >> + description: I2C device address >> + maxItems: 1 >> + >> + clocks: >> + maxItems: 1 >> + >> + clock-names: >> + items: >> + - const: xclk >> + >> + VDIG-supply: >> + description: >> + Digital I/O voltage supply, 1.8 volts >> + >> + VANA-supply: >> + description: >> + Analog voltage supply, 2.8 volts >> + >> + VDDL-supply: >> + description: >> + Digital core voltage supply, 1.2 volts >> + >> + xclr-gpios: >> + description: |- >> + Reference to the GPIO connected to the xclr pin, if any. >> + Must be released (set high) after all supplies are applied. >> + >> + camera-clk: >> + type: object >> + >> + description: Clock source for imx219 > > This clock is external to the sensor, so a node for a fixed clock should > be too. Done in v3. >> + >> + properties: >> + clock-frequency: true >> + >> + required: >> + - clock-frequency >> + >> + # See ../video-interfaces.txt for more details >> + port: >> + type: object >> + properties: >> + endpoint: >> + type: object >> + properties: >> + clock-lanes: >> + const: 0 >> + >> + data-lanes: >> + description: |- >> + Should be <1 2> for two-lane operation, or <1 2 3 4> for >> + four-lane operation. >> + oneOf: >> + - const: [[ 1, 2 ]] >> + - const: [[ 1, 2, 3, 4 ]] > > Not sure if this actually works. If it does, it exposes how we encode > the DT yaml format which we don't want to do here. It does work - I am still not quite comfortable with json schema, and got to the above by looking at the files produced by 'make dt_binding_check'. > It should be: > > oneOf: > - items: > - const: 1 > - const: 2 > - items: > - const: 1 > - const: 2 > - const: 3 > - const: 4 Thanks, got it. > Really, I think you shouldn't need the 2nd case as that should be the > default. I've tried to implement that in v3 of the patch set. Thanks, Andrey >> + >> + clock-noncontinuous: >> + type: boolean >> + description: |- >> + Presence of this boolean property decides whether the MIPI CSI-2 >> + clock is continuous or non-continuous. >> + >> + required: >> + - clock-lanes >> + - data-lanes >> + >> +required: >> + - compatible >> + - reg >> + - clocks >> + - clock-names >> + - VANA-supply >> + - VDIG-supply >> + - VDDL-supply >> + - port >> + >> +additionalProperties: false >> + >> +examples: >> + - | >> + i2c0 { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + imx219: sensor@10 { >> + compatible = "sony,imx219"; >> + reg = <0x10>; >> + clocks = <&imx219_clk>; >> + clock-names = "xclk"; >> + VANA-supply = <&imx219_vana>; /* 2.8v */ >> + VDIG-supply = <&imx219_vdig>; /* 1.8v */ >> + VDDL-supply = <&imx219_vddl>; /* 1.2v */ >> + >> + imx219_clk: camera-clk { >> + compatible = "fixed-clock"; >> + #clock-cells = <0>; >> + clock-frequency = <24000000>; >> + }; >> + >> + port { >> + imx219_0: endpoint { >> + remote-endpoint = <&csi1_ep>; >> + clock-lanes = <0>; >> + data-lanes = <1 2>; >> + clock-noncontinuous; >> + }; >> + }; >> + }; >> + }; >> + >> +... >> diff --git a/MAINTAINERS b/MAINTAINERS >> index ffa3371bc750..f7b6c24ec081 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -15350,6 +15350,14 @@ S: Maintained >> F: drivers/media/i2c/imx214.c >> F: Documentation/devicetree/bindings/media/i2c/sony,imx214.txt >> >> +SONY IMX219 SENSOR DRIVER >> +M: Dave Stevenson <dave.stevenson@raspberrypi.com> >> +L: linux-media@vger.kernel.org >> +T: git git://linuxtv.org/media_tree.git >> +S: Maintained >> +F: drivers/media/i2c/imx219.c >> +F: Documentation/devicetree/bindings/media/i2c/imx219.yaml >> + >> SONY IMX258 SENSOR DRIVER >> M: Sakari Ailus <sakari.ailus@linux.intel.com> >> L: linux-media@vger.kernel.org >> -- >> 2.17.1 >>
diff --git a/Documentation/devicetree/bindings/media/i2c/imx219.yaml b/Documentation/devicetree/bindings/media/i2c/imx219.yaml new file mode 100644 index 000000000000..b58aa49a7c03 --- /dev/null +++ b/Documentation/devicetree/bindings/media/i2c/imx219.yaml @@ -0,0 +1,134 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/media/i2c/imx219.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Sony 1/4.0-Inch 8Mpixel CMOS Digital Image Sensor + +maintainers: + - Dave Stevenson <dave.stevenson@raspberrypi.com> + +description: |- + The Sony imx219 is a 1/4.0-inch CMOS active pixel digital image sensor + with an active array size of 3280H x 2464V. It is programmable through + I2C interface. The I2C address is fixed to 0x10 as per sensor data sheet. + Image data is sent through MIPI CSI-2, which is configured as either 2 or + 4 data lanes. + +properties: + compatible: + const: sony,imx219 + + reg: + description: I2C device address + maxItems: 1 + + clocks: + maxItems: 1 + + clock-names: + items: + - const: xclk + + VDIG-supply: + description: + Digital I/O voltage supply, 1.8 volts + + VANA-supply: + description: + Analog voltage supply, 2.8 volts + + VDDL-supply: + description: + Digital core voltage supply, 1.2 volts + + xclr-gpios: + description: |- + Reference to the GPIO connected to the xclr pin, if any. + Must be released (set high) after all supplies are applied. + + camera-clk: + type: object + + description: Clock source for imx219 + + properties: + clock-frequency: true + + required: + - clock-frequency + + # See ../video-interfaces.txt for more details + port: + type: object + properties: + endpoint: + type: object + properties: + clock-lanes: + const: 0 + + data-lanes: + description: |- + Should be <1 2> for two-lane operation, or <1 2 3 4> for + four-lane operation. + oneOf: + - const: [[ 1, 2 ]] + - const: [[ 1, 2, 3, 4 ]] + + clock-noncontinuous: + type: boolean + description: |- + Presence of this boolean property decides whether the MIPI CSI-2 + clock is continuous or non-continuous. + + required: + - clock-lanes + - data-lanes + +required: + - compatible + - reg + - clocks + - clock-names + - VANA-supply + - VDIG-supply + - VDDL-supply + - port + +additionalProperties: false + +examples: + - | + i2c0 { + #address-cells = <1>; + #size-cells = <0>; + + imx219: sensor@10 { + compatible = "sony,imx219"; + reg = <0x10>; + clocks = <&imx219_clk>; + clock-names = "xclk"; + VANA-supply = <&imx219_vana>; /* 2.8v */ + VDIG-supply = <&imx219_vdig>; /* 1.8v */ + VDDL-supply = <&imx219_vddl>; /* 1.2v */ + + imx219_clk: camera-clk { + compatible = "fixed-clock"; + #clock-cells = <0>; + clock-frequency = <24000000>; + }; + + port { + imx219_0: endpoint { + remote-endpoint = <&csi1_ep>; + clock-lanes = <0>; + data-lanes = <1 2>; + clock-noncontinuous; + }; + }; + }; + }; + +... diff --git a/MAINTAINERS b/MAINTAINERS index ffa3371bc750..f7b6c24ec081 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -15350,6 +15350,14 @@ S: Maintained F: drivers/media/i2c/imx214.c F: Documentation/devicetree/bindings/media/i2c/sony,imx214.txt +SONY IMX219 SENSOR DRIVER +M: Dave Stevenson <dave.stevenson@raspberrypi.com> +L: linux-media@vger.kernel.org +T: git git://linuxtv.org/media_tree.git +S: Maintained +F: drivers/media/i2c/imx219.c +F: Documentation/devicetree/bindings/media/i2c/imx219.yaml + SONY IMX258 SENSOR DRIVER M: Sakari Ailus <sakari.ailus@linux.intel.com> L: linux-media@vger.kernel.org
Add YAML device tree binding for IMX219 CMOS image sensor, and the relevant MAINTAINERS entries. Signed-off-by: Andrey Konovalov <andrey.konovalov@linaro.org> --- .../devicetree/bindings/media/i2c/imx219.yaml | 134 ++++++++++++++++++ MAINTAINERS | 8 ++ 2 files changed, 142 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/i2c/imx219.yaml