Re: [PATCH v2 2/2] media: dt-bindings: media: i2c: Add IMX300 CMOS sensor binding

[Sakari Ailus <sakari.ailus@iki.fi>]

Hi Rob, AngeloGioacchino,

On Wed, Nov 04, 2020 at 03:48:06PM -0600, Rob Herring wrote:
> On Thu, Oct 29, 2020 at 06:29:47PM +0100, kholk11@gmail.com wrote:
> > From: AngeloGioacchino Del Regno <kholk11@gmail.com>
> > 
> > Add YAML device tree binding for IMX300 CMOS image sensor, and
> > the relevant MAINTAINERS entries.
> > 
> > Signed-off-by: AngeloGioacchino Del Regno <kholk11@gmail.com>
> > ---
> >  .../bindings/media/i2c/sony,imx300.yaml       | 112 ++++++++++++++++++
> >  MAINTAINERS                                   |   7 ++
> >  2 files changed, 119 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/media/i2c/sony,imx300.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx300.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx300.yaml
> > new file mode 100644
> > index 000000000000..8f1d795f8072
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx300.yaml
> > @@ -0,0 +1,112 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/media/i2c/sony,imx300.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Sony 1/2.3-Inch 8Mpixel Stacked CMOS Digital Image Sensor
> > +
> > +maintainers:
> > +  - AngeloGioacchino Del Regno <kholk11@gmail.com>
> > +
> > +description: |-
> > +  The Sony IMX300 is a 1/2.3-inch Stacked CMOS (Exmor-RS) digital image
> > +  sensor with a pixel size of 1.08um and an active array size of
> > +  5948H x 4140V. It is programmable through I2C interface at address 0x10.
> > +  Image data is sent through MIPI CSI-2, which is configured as either 2 or
> > +  4 data lanes.
> > +
> > +properties:
> > +  compatible:
> > +    const: sony,imx300
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    maxItems: 1

The frequency needs to come from DT; it's not a property of the sensor.

> > +
> > +  vdig-supply:
> > +    description:
> > +      Digital I/O voltage supply, 1.15-1.20 volts
> > +
> > +  vana-supply:
> > +    description:
> > +      Analog voltage supply, 2.2 volts
> > +
> > +  vddl-supply:
> > +    description:
> > +      Digital core voltage supply, 1.8 volts
> > +
> > +  reset-gpios:
> 
> maxItems: 1
> 
> > +    description: |-
> > +      Reference to the GPIO connected to the xclr pin, if any.
> > +      Must be released (set high) after all supplies are applied.
> > +
> > +  # See ../video-interfaces.txt for more details
> > +  port:
> > +    type: object
> > +    properties:
> > +      endpoint:
> > +        type: object
> > +
> > +        properties:
> > +          data-lanes:
> > +            description: |-
> > +              The sensor supports either two-lane, or four-lane operation,
> > +              but the driver currently supports only four-lane.
> 
> What the driver is not relevant. Please define all possible 
> configurations. Or just omit? What's the behavior if omitted?

I think we've used to specify the number of lanes if it's configurable,
instead of having defaults. I'd do the same here, i.e. require data-lanes.

> 
> > +            items:
> > +              - const: 0
> > +              - const: 1
> > +              - const: 2
> > +              - const: 3

-- 
Kind regards,

Sakari Ailus