Hi Tomasz, Dongchun, On Thu, May 07, 2020 at 03:50:40PM +0200, Tomasz Figa wrote: > Hi Sakari and Dongchun, > > On Thu, May 7, 2020 at 3:00 PM Dongchun Zhu <dongchun.zhu@xxxxxxxxxxxx> wrote: > > > > Hi Sakari, > > > > Thanks for the review. > > > > On Wed, 2020-05-06 at 14:21 +0300, Sakari Ailus wrote: > > > Hi Dongchun, > > > > > > On Tue, May 05, 2020 at 10:17:18PM +0800, Dongchun Zhu wrote: > > > > Hi Sakari, > > > > > > > > Thanks for the review. > > > > > > > > On Tue, 2020-05-05 at 10:04 +0300, Sakari Ailus wrote: > > > > > Hi Dongchun, > > > > > > > > > > On Thu, Apr 30, 2020 at 04:09:23PM +0800, Dongchun Zhu wrote: > > > > > > Add DT bindings documentation for Omnivision OV02A10 image sensor. > > > > > > > > > > > > Signed-off-by: Dongchun Zhu <dongchun.zhu@xxxxxxxxxxxx> > > > > > > --- > > > > > > .../bindings/media/i2c/ovti,ov02a10.yaml | 148 +++++++++++++++++++++ > > > > > > MAINTAINERS | 7 + > > > > > > 2 files changed, 155 insertions(+) > > > > > > create mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ov02a10.yaml > > > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov02a10.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov02a10.yaml > > > > > > new file mode 100644 > > > > > > index 0000000..2be4bd2 > > > > > > --- /dev/null > > > > > > +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov02a10.yaml > > > > > > @@ -0,0 +1,148 @@ > > > > > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > > > > > > +# Copyright (c) 2020 MediaTek Inc. > > > > > > +%YAML 1.2 > > > > > > +--- > > > > > > +$id: http://devicetree.org/schemas/media/i2c/ovti,ov02a10.yaml# > > > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > > > + > > > > > > +title: Omnivision OV02A10 CMOS Sensor Device Tree Bindings > > > > > > + > > > > > > +maintainers: > > > > > > + - Dongchun Zhu <dongchun.zhu@xxxxxxxxxxxx> > > > > > > + > > > > > > +description: |- > > > > > > + The Omnivision OV02A10 is a low-cost, high performance, 1/5-inch, 2 megapixel > > > > > > + image sensor, which is the latest production derived from Omnivision's CMOS > > > > > > + image sensor technology. Ihis chip supports high frame rate speeds up to 30fps > > > > > > + @ 1600x1200 (UXGA) resolution transferred over a 1-lane MIPI interface. The > > > > > > + sensor output is available via CSI-2 serial data output. > > > > > > + > > > > > > +properties: > > > > > > + compatible: > > > > > > + const: ovti,ov02a10 > > > > > > + > > > > > > + reg: > > > > > > + maxItems: 1 > > > > > > + > > > > > > + clocks: > > > > > > + items: > > > > > > + - description: top mux camtg clock > > > > > > + - description: devider clock > > > > > > + > > > > > > + clock-names: > > > > > > + items: > > > > > > + - const: eclk > > > > > > + - const: freq_mux > > > > > > + > > > > > > + clock-frequency: > > > > > > + description: > > > > > > + Frequency of the eclk clock in Hertz. > > > > > > + > > > > > > + dovdd-supply: > > > > > > + description: > > > > > > + Definition of the regulator used as interface power supply. > > > > > > + > > > > > > + avdd-supply: > > > > > > + description: > > > > > > + Definition of the regulator used as analog power supply. > > > > > > + > > > > > > + dvdd-supply: > > > > > > + description: > > > > > > + Definition of the regulator used as digital power supply. > > > > > > + > > > > > > + powerdown-gpios: > > > > > > + description: > > > > > > + The phandle and specifier for the GPIO that controls sensor powerdown. > > > > > > + > > > > > > + reset-gpios: > > > > > > + description: > > > > > > + The phandle and specifier for the GPIO that controls sensor reset. > > > > > > + > > > > > > + rotation: > > > > > > + description: > > > > > > + Definition of the sensor's placement, valid values are 0 and 180. > > > > > > + allOf: > > > > > > + - $ref: "/schemas/types.yaml#/definitions/uint32" > > > > > > + - enum: > > > > > > + - 0 # Sensor Mounted Upright > > > > > > + - 180 # Sensor Mounted Upside Down > > > > > > + > > > > > > + ovti,mipi-tx-speed: > > > > > > + description: > > > > > > + Indication of MIPI transmission speed select. > > > > > > > > > > What exactly does this signify? And how do you come up with the number? > > > > > > > > > > > > > Apologies for not addressing this number clear. > > > > > > > > From the datasheet, P1:0xA1 register represents TX_SPEED_AREA_SEL with > > > > the default val: 0x03. > > > > The description of this RW register is as below: > > > > Bit[2:0]: MIPI transmission speed select. > > > > > > > > Thus the enum should be definited as [ 0, 1, 2, 3, 4, 5, 6, 7 ]. > > > > This would be fixed in next release. > > > > > > > > In the meantime, as the default val of P1:0xA1 is 0x03, we hope to keep > > > > that value if there is no setting for this private property in DT. > > > > The caller in driver would be updated like this in next release. > > > > if (ov02a10->mipi_clock_tx_speed) > > > > ret = i2c_smbus_write_byte_data(...,...); > > > > > > How did you pick the value in the example? And why do you believe it is > > > specific to a platform, and not e.g. a sensor mode? > > > > > > > We look into P1:0XA1, one register that defines MIPI transmission speed > > select. > > From the datasheet, we can get the possible values that could be set to > > P1:0xA1. > > > > Actually this register is an independent of sensor mode, it is just > > included in sensor mode's register setting table. > > > > In addition, this private DT Property is created to fix the MIPI test > > failure. The register values are adjusted and verified from vendor to > > make sensor signal meet MIPI specification. > > > > In theory the value could depend on the mode, because different link > rate could impose different requirements for the physical interface. > In practice, we haven't seen any hardware that would require different > values for different modes. The mode (possibly in conjunction with other information available to the driver via V4L2 fwnode interface) precisely defines the parameters of the CSI-2 bus --- apart from the possible exception of the bus timing related parameters but this is not supported by the name of the parameter. Therefore I don't see how this parameter, which supposedly is used to determine the CSI-2 transmissions speed, could be board specific and thus belong to DT. -- Kind regards, Sakari Ailus
