All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/2] dt-bindings: media: i2c: Add IMX519 CMOS sensor binding
@ 2022-11-04  7:26 lee
  2022-11-04 12:36 ` Rob Herring
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: lee @ 2022-11-04  7:26 UTC (permalink / raw)
  To: linux-media
  Cc: robh+dt, krzysztof.kozlowski+dt, shawnguo, s.hauer, devicetree

Add YAML device tree binding for IMX519 CMOS image sensor, and
the relevant MAINTAINERS entries.

Signed-off-by: Lee Jackson <lee@arducam.com>
---
 .../bindings/media/i2c/sony,imx519.yaml       | 107 ++++++++++++++++++
 MAINTAINERS                                   |   9 ++
 2 files changed, 116 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/sony,imx519.yaml

diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx519.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx519.yaml
new file mode 100644
index 000000000000..9b6cda96f613
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/sony,imx519.yaml
@@ -0,0 +1,107 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/i2c/sony,imx519.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sony 1/2.5-Inch 16Mpixel CMOS Digital Image Sensor
+
+maintainers:
+  - Lee Jackson <lee@arducam.com>
+
+description: |-
+  The Sony IMX519 is a 1/2.5-inch CMOS active pixel digital image sensor
+  with an active array size of 4656H x 3496V. It is programmable through
+  I2C interface. The I2C address is fixed to 0x1A 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,imx519
+
+  reg:
+    description: I2C device address
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  VDIG-supply:
+    description:
+      Digital I/O voltage supply, 1.05 volts
+
+  VANA-supply:
+    description:
+      Analog voltage supply, 2.8 volts
+
+  VDDL-supply:
+    description:
+      Digital core voltage supply, 1.8 volts
+
+  reset-gpios:
+    description: |-
+      Reference to the GPIO connected to the xclr pin, if any.
+      Must be released (set high) after all supplies and INCK are applied.
+
+port:
+    $ref: /schemas/graph.yaml#/$defs/port-base
+    additionalProperties: false
+
+    properties:
+      endpoint:
+        $ref: /schemas/media/video-interfaces.yaml#
+        unevaluatedProperties: false
+
+        properties:
+          data-lanes:
+            description: |-
+              The driver only supports two-lane operation.
+            items:
+              - const: 1
+              - const: 2
+
+          clock-noncontinuous: true
+          link-frequencies: true
+
+        required:
+          - data-lanes
+          - link-frequencies
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - VANA-supply
+  - VDIG-supply
+  - VDDL-supply
+  - port
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c0 {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        imx519: sensor@1a {
+            compatible = "sony,imx519";
+            reg = <0x1a>;
+            clocks = <&imx519_clk>;
+            VANA-supply = <&imx519_vana>;   /* 2.8v */
+            VDIG-supply = <&imx519_vdig>;   /* 1.05v */
+            VDDL-supply = <&imx519_vddl>;   /* 1.8v */
+
+            port {
+                imx519_0: endpoint {
+                    remote-endpoint = <&csi1_ep>;
+                    data-lanes = <1 2>;
+                    clock-noncontinuous;
+                    link-frequencies = /bits/ 64 <493500000>;
+                };
+            };
+        };
+    };
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index e04d944005ba..5a617ab8c9b2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19236,6 +19236,15 @@ T:	git git://linuxtv.org/media_tree.git
 F:	Documentation/devicetree/bindings/media/i2c/sony,imx412.yaml
 F:	drivers/media/i2c/imx412.c
 
+SONY IMX519 SENSOR DRIVER
+M:	Arducam Kernel Maintenance <info@arducam.com>
+M:	Lee Jackson <lee@arducam.com>
+L:	linux-media@vger.kernel.org
+S:	Maintained
+T:	git git://linuxtv.org/media_tree.git
+F:	Documentation/devicetree/bindings/media/i2c/sony,imx519.yaml
+F:	drivers/media/i2c/imx519.c
+
 SONY MEMORYSTICK SUBSYSTEM
 M:	Maxim Levitsky <maximlevitsky@gmail.com>
 M:	Alex Dubov <oakad@yahoo.com>
-- 
2.25.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/2] dt-bindings: media: i2c: Add IMX519 CMOS sensor binding
  2022-11-04  7:26 [PATCH v2 1/2] dt-bindings: media: i2c: Add IMX519 CMOS sensor binding lee
@ 2022-11-04 12:36 ` Rob Herring
  2022-11-04 13:17 ` Krzysztof Kozlowski
  2022-11-04 13:18 ` Krzysztof Kozlowski
  2 siblings, 0 replies; 11+ messages in thread
From: Rob Herring @ 2022-11-04 12:36 UTC (permalink / raw)
  To: lee
  Cc: robh+dt, krzysztof.kozlowski+dt, devicetree, shawnguo, s.hauer,
	linux-media


On Fri, 04 Nov 2022 15:26:01 +0800, lee wrote:
> Add YAML device tree binding for IMX519 CMOS image sensor, and
> the relevant MAINTAINERS entries.
> 
> Signed-off-by: Lee Jackson <lee@arducam.com>
> ---
>  .../bindings/media/i2c/sony,imx519.yaml       | 107 ++++++++++++++++++
>  MAINTAINERS                                   |   9 ++
>  2 files changed, 116 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/sony,imx519.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:
./Documentation/devicetree/bindings/media/i2c/sony,imx519.yaml:48:5: [warning] wrong indentation: expected 2 but found 4 (indentation)

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/i2c/sony,imx519.yaml: 'port' is not one of ['$id', '$schema', 'title', 'description', 'examples', 'required', 'allOf', 'anyOf', 'oneOf', 'definitions', '$defs', 'additionalProperties', 'dependencies', 'dependentRequired', 'dependentSchemas', 'patternProperties', 'properties', 'if', 'then', 'else', 'unevaluatedProperties', 'deprecated', 'maintainers', 'select', '$ref']
	from schema $id: http://devicetree.org/meta-schemas/base.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/i2c/sony,imx519.example.dtb: sensor@1a: 'port' does not match any of the regexes: 'pinctrl-[0-9]+'
	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/i2c/sony,imx519.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/2] dt-bindings: media: i2c: Add IMX519 CMOS sensor binding
  2022-11-04  7:26 [PATCH v2 1/2] dt-bindings: media: i2c: Add IMX519 CMOS sensor binding lee
  2022-11-04 12:36 ` Rob Herring
@ 2022-11-04 13:17 ` Krzysztof Kozlowski
  2022-11-16  1:37   ` lee
  2022-11-04 13:18 ` Krzysztof Kozlowski
  2 siblings, 1 reply; 11+ messages in thread
From: Krzysztof Kozlowski @ 2022-11-04 13:17 UTC (permalink / raw)
  To: lee, linux-media
  Cc: robh+dt, krzysztof.kozlowski+dt, shawnguo, s.hauer, devicetree

On 04/11/2022 03:26, lee wrote:
> Add YAML device tree binding for IMX519 CMOS image sensor, and
> the relevant MAINTAINERS entries.
> 
> Signed-off-by: Lee Jackson <lee@arducam.com>
> ---
>  .../bindings/media/i2c/sony,imx519.yaml       | 107 ++++++++++++++++++
>  MAINTAINERS                                   |   9 ++
>  2 files changed, 116 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/sony,imx519.yaml
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx519.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx519.yaml
> new file mode 100644
> index 000000000000..9b6cda96f613
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx519.yaml
> @@ -0,0 +1,107 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/i2c/sony,imx519.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Sony 1/2.5-Inch 16Mpixel CMOS Digital Image Sensor
> +
> +maintainers:
> +  - Lee Jackson <lee@arducam.com>
> +
> +description: |-
> +  The Sony IMX519 is a 1/2.5-inch CMOS active pixel digital image sensor
> +  with an active array size of 4656H x 3496V. It is programmable through
> +  I2C interface. The I2C address is fixed to 0x1A 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,imx519
> +
> +  reg:
> +    description: I2C device address

This is a friendly reminder during the review process.

It seems my previous comments were not fully addressed. Maybe my
feedback got lost between the quotes, maybe you just forgot to apply it.
Please go back to the previous discussion and either implement all
requested changes or keep discussing them.

Thank you.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/2] dt-bindings: media: i2c: Add IMX519 CMOS sensor binding
  2022-11-04  7:26 [PATCH v2 1/2] dt-bindings: media: i2c: Add IMX519 CMOS sensor binding lee
  2022-11-04 12:36 ` Rob Herring
  2022-11-04 13:17 ` Krzysztof Kozlowski
@ 2022-11-04 13:18 ` Krzysztof Kozlowski
  2 siblings, 0 replies; 11+ messages in thread
From: Krzysztof Kozlowski @ 2022-11-04 13:18 UTC (permalink / raw)
  To: lee, linux-media
  Cc: robh+dt, krzysztof.kozlowski+dt, shawnguo, s.hauer, devicetree

On 04/11/2022 03:26, lee wrote:
> Add YAML device tree binding for IMX519 CMOS image sensor, and
> the relevant MAINTAINERS entries.
> 
> Signed-off-by: Lee Jackson <lee@arducam.com>
> ---

For the record - seems none of the comments were applied (not just one
or two), so you just ignored all of them.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/2] dt-bindings: media: i2c: Add IMX519 CMOS sensor binding
  2022-11-04 13:17 ` Krzysztof Kozlowski
@ 2022-11-16  1:37   ` lee
  0 siblings, 0 replies; 11+ messages in thread
From: lee @ 2022-11-16  1:37 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: linux-media, robh+dt, krzysztof.kozlowski+dt, shawnguo, s.hauer,
	devicetree

On Fri, 4 Nov 2022 09:17:15 -0400
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:

> On 04/11/2022 03:26, lee wrote:
> > Add YAML device tree binding for IMX519 CMOS image sensor, and
> > the relevant MAINTAINERS entries.
> > 
> > Signed-off-by: Lee Jackson <lee@arducam.com>
> > ---
> >  .../bindings/media/i2c/sony,imx519.yaml       | 107
> > ++++++++++++++++++ MAINTAINERS                                   |
> >  9 ++ 2 files changed, 116 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/media/i2c/sony,imx519.yaml
> > 
> > diff --git
> > a/Documentation/devicetree/bindings/media/i2c/sony,imx519.yaml
> > b/Documentation/devicetree/bindings/media/i2c/sony,imx519.yaml new
> > file mode 100644 index 000000000000..9b6cda96f613 --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx519.yaml
> > @@ -0,0 +1,107 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/media/i2c/sony,imx519.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Sony 1/2.5-Inch 16Mpixel CMOS Digital Image Sensor
> > +
> > +maintainers:
> > +  - Lee Jackson <lee@arducam.com>
> > +
> > +description: |-
> > +  The Sony IMX519 is a 1/2.5-inch CMOS active pixel digital image
> > sensor
> > +  with an active array size of 4656H x 3496V. It is programmable
> > through
> > +  I2C interface. The I2C address is fixed to 0x1A 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,imx519
> > +
> > +  reg:
> > +    description: I2C device address  
> 
> This is a friendly reminder during the review process.
> 
> It seems my previous comments were not fully addressed. Maybe my
> feedback got lost between the quotes, maybe you just forgot to apply
> it. Please go back to the previous discussion and either implement all
> requested changes or keep discussing them.
> 
> Thank you.
> 
> Best regards,
> Krzysztof
> 

Hi,
Thanks for your friendly reminder. Sorry for my last error commit. I have modified the dts based on your comments and resubmitted new driver. 
I have checked it using make dt_binding_check command. 
You are welcome to submit new comments again.
Have a nice day.
Lee.
 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/2] dt-bindings: media: i2c: Add IMX519 CMOS sensor binding
  2022-11-07  8:24     ` Krzysztof Kozlowski
@ 2022-11-07 10:24       ` lee
  0 siblings, 0 replies; 11+ messages in thread
From: lee @ 2022-11-07 10:24 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: linux-media, robh+dt, krzysztof.kozlowski+dt, shawnguo, s.hauer,
	devicetree

On Mon, 7 Nov 2022 09:24:53 +0100
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:

> On 07/11/2022 03:42, lee wrote:
> > On Thu, 3 Nov 2022 09:05:05 -0400
> > Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
> > 
> > Hi,
> > Thanks a lot for detailed comments and suggestions for
> > revisions,there is one place where I have some doubts.  
> >> On 02/11/2022 23:04, lee wrote:  
> >>> Add YAML device tree binding for IMX519 CMOS image sensor, and
> >>> the relevant MAINTAINERS entries.
> >>>
> >>> Signed-off-by: Lee Jackson <lee@arducam.com>    
> >>
> >> Your From does not match fully SoB.
> >>  
> >>> ---
> >>>  .../bindings/media/i2c/sony,imx519.yaml       | 107
> >>> ++++++++++++++++++ MAINTAINERS                                   |
> >>>  9 ++ 2 files changed, 116 insertions(+)
> >>>  create mode 100644
> >>> Documentation/devicetree/bindings/media/i2c/sony,imx519.yaml
> >>>
> >>> diff --git
> >>> a/Documentation/devicetree/bindings/media/i2c/sony,imx519.yaml
> >>> b/Documentation/devicetree/bindings/media/i2c/sony,imx519.yaml new
> >>> file mode 100644 index 000000000000..9b6cda96f613 --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx519.yaml
> >>> @@ -0,0 +1,107 @@
> >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >>> +%YAML 1.2
> >>> +---
> >>> +$id: http://devicetree.org/schemas/media/i2c/sony,imx519.yaml#
> >>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >>> +
> >>> +title: Sony 1/2.5-Inch 16Mpixel CMOS Digital Image Sensor
> >>> +
> >>> +maintainers:
> >>> +  - Lee Jackson <lee@arducam.com>
> >>> +
> >>> +description: |-
> >>> +  The Sony IMX519 is a 1/2.5-inch CMOS active pixel digital image
> >>> sensor
> >>> +  with an active array size of 4656H x 3496V. It is programmable
> >>> through
> >>> +  I2C interface. The I2C address is fixed to 0x1A 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,imx519
> >>> +
> >>> +  reg:
> >>> +    description: I2C device address    
> >>
> >> Skip description.
> >>  
> >>> +    maxItems: 1
> >>> +
> >>> +  clocks:
> >>> +    maxItems: 1
> >>> +
> >>> +  VDIG-supply:    
> >>
> >> Use lower case names.  
> > 
> > Here we refer to other drivers (imx219), they are all uppercase, is
> > lowercase mandatory?  
> 
> Lowercase is the convention. Just because some other driver used
> uppercase is not really correct argument. If other driver has bug, you
> also copy it?
> 
> 
> Best regards,
> Krzysztof
> 

Hi,
Thanks for your quick reply. 
I just want to refer to the driver that has passed the review, as a template, is there anything I can refer to?
Do you have any good suggestions?

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/2] dt-bindings: media: i2c: Add IMX519 CMOS sensor binding
  2022-11-07  2:42   ` lee
@ 2022-11-07  8:24     ` Krzysztof Kozlowski
  2022-11-07 10:24       ` lee
  0 siblings, 1 reply; 11+ messages in thread
From: Krzysztof Kozlowski @ 2022-11-07  8:24 UTC (permalink / raw)
  To: lee
  Cc: linux-media, robh+dt, krzysztof.kozlowski+dt, shawnguo, s.hauer,
	devicetree

On 07/11/2022 03:42, lee wrote:
> On Thu, 3 Nov 2022 09:05:05 -0400
> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
> 
> Hi,
> Thanks a lot for detailed comments and suggestions for revisions,there is one place where I have some doubts.
>> On 02/11/2022 23:04, lee wrote:
>>> Add YAML device tree binding for IMX519 CMOS image sensor, and
>>> the relevant MAINTAINERS entries.
>>>
>>> Signed-off-by: Lee Jackson <lee@arducam.com>  
>>
>> Your From does not match fully SoB.
>>
>>> ---
>>>  .../bindings/media/i2c/sony,imx519.yaml       | 107
>>> ++++++++++++++++++ MAINTAINERS                                   |
>>>  9 ++ 2 files changed, 116 insertions(+)
>>>  create mode 100644
>>> Documentation/devicetree/bindings/media/i2c/sony,imx519.yaml
>>>
>>> diff --git
>>> a/Documentation/devicetree/bindings/media/i2c/sony,imx519.yaml
>>> b/Documentation/devicetree/bindings/media/i2c/sony,imx519.yaml new
>>> file mode 100644 index 000000000000..9b6cda96f613 --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx519.yaml
>>> @@ -0,0 +1,107 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/media/i2c/sony,imx519.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Sony 1/2.5-Inch 16Mpixel CMOS Digital Image Sensor
>>> +
>>> +maintainers:
>>> +  - Lee Jackson <lee@arducam.com>
>>> +
>>> +description: |-
>>> +  The Sony IMX519 is a 1/2.5-inch CMOS active pixel digital image
>>> sensor
>>> +  with an active array size of 4656H x 3496V. It is programmable
>>> through
>>> +  I2C interface. The I2C address is fixed to 0x1A 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,imx519
>>> +
>>> +  reg:
>>> +    description: I2C device address  
>>
>> Skip description.
>>
>>> +    maxItems: 1
>>> +
>>> +  clocks:
>>> +    maxItems: 1
>>> +
>>> +  VDIG-supply:  
>>
>> Use lower case names.
> 
> Here we refer to other drivers (imx219), they are all uppercase, is lowercase mandatory?

Lowercase is the convention. Just because some other driver used
uppercase is not really correct argument. If other driver has bug, you
also copy it?


Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/2] dt-bindings: media: i2c: Add IMX519 CMOS sensor binding
  2022-11-03 13:05 ` Krzysztof Kozlowski
@ 2022-11-07  2:42   ` lee
  2022-11-07  8:24     ` Krzysztof Kozlowski
  0 siblings, 1 reply; 11+ messages in thread
From: lee @ 2022-11-07  2:42 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: linux-media, robh+dt, krzysztof.kozlowski+dt, shawnguo, s.hauer,
	devicetree

On Thu, 3 Nov 2022 09:05:05 -0400
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:

Hi,
Thanks a lot for detailed comments and suggestions for revisions,there is one place where I have some doubts.
> On 02/11/2022 23:04, lee wrote:
> > Add YAML device tree binding for IMX519 CMOS image sensor, and
> > the relevant MAINTAINERS entries.
> > 
> > Signed-off-by: Lee Jackson <lee@arducam.com>  
> 
> Your From does not match fully SoB.
> 
> > ---
> >  .../bindings/media/i2c/sony,imx519.yaml       | 107
> > ++++++++++++++++++ MAINTAINERS                                   |
> >  9 ++ 2 files changed, 116 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/media/i2c/sony,imx519.yaml
> > 
> > diff --git
> > a/Documentation/devicetree/bindings/media/i2c/sony,imx519.yaml
> > b/Documentation/devicetree/bindings/media/i2c/sony,imx519.yaml new
> > file mode 100644 index 000000000000..9b6cda96f613 --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx519.yaml
> > @@ -0,0 +1,107 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/media/i2c/sony,imx519.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Sony 1/2.5-Inch 16Mpixel CMOS Digital Image Sensor
> > +
> > +maintainers:
> > +  - Lee Jackson <lee@arducam.com>
> > +
> > +description: |-
> > +  The Sony IMX519 is a 1/2.5-inch CMOS active pixel digital image
> > sensor
> > +  with an active array size of 4656H x 3496V. It is programmable
> > through
> > +  I2C interface. The I2C address is fixed to 0x1A 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,imx519
> > +
> > +  reg:
> > +    description: I2C device address  
> 
> Skip description.
> 
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    maxItems: 1
> > +
> > +  VDIG-supply:  
> 
> Use lower case names.

Here we refer to other drivers (imx219), they are all uppercase, is lowercase mandatory?
> 
> > +    description:
> > +      Digital I/O voltage supply, 1.05 volts
> > +
> > +  VANA-supply:
> > +    description:
> > +      Analog voltage supply, 2.8 volts
> > +
> > +  VDDL-supply:
> > +    description:
> > +      Digital core voltage supply, 1.8 volts
> > +
> > +  reset-gpios:
> > +    description: |-
> > +      Reference to the GPIO connected to the xclr pin, if any.
> > +      Must be released (set high) after all supplies and INCK are
> > applied. +
> > +port:
> > +    $ref: /schemas/graph.yaml#/$defs/port-base
> > +    additionalProperties: false
> > +
> > +    properties:
> > +      endpoint:
> > +        $ref: /schemas/media/video-interfaces.yaml#
> > +        unevaluatedProperties: false
> > +
> > +        properties:
> > +          data-lanes:
> > +            description: |-
> > +              The driver only supports two-lane operation.  
> 
> Please describe the hardware, not driver.
> 
> > +            items:
> > +              - const: 1
> > +              - const: 2
> > +
> > +          clock-noncontinuous: true
> > +          link-frequencies: true  
> 
> I think these are coming from video-interfaces, so no need for them.
> 
> > +
> > +        required:
> > +          - data-lanes
> > +          - link-frequencies
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - clocks
> > +  - VANA-supply
> > +  - VDIG-supply
> > +  - VDDL-supply
> > +  - port
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    i2c0 {  
> 
> i2c
> 
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +  
> 
> > +        imx519: sensor@1a {
> > +            compatible = "sony,imx519";
> > +            reg = <0x1a>;
> > +            clocks = <&imx519_clk>;
> > +            VANA-supply = <&imx519_vana>;   /* 2.8v */
> > +            VDIG-supply = <&imx519_vdig>;   /* 1.05v */
> > +            VDDL-supply = <&imx519_vddl>;   /* 1.8v */
> > +
> > +            port {
> > +                imx519_0: endpoint {
> > +                    remote-endpoint = <&csi1_ep>;
> > +                    data-lanes = <1 2>;
> > +                    clock-noncontinuous;
> > +                    link-frequencies = /bits/ 64 <493500000>;
> > +                };
> > +            };
> > +        };
> > +    };
> > +
> > +...
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index e04d944005ba..5a617ab8c9b2 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -19236,6 +19236,15 @@ T:	git
> > git://linuxtv.org/media_tree.git F:
> > Documentation/devicetree/bindings/media/i2c/sony,imx412.yaml
> > F:	drivers/media/i2c/imx412.c 
> > +SONY IMX519 SENSOR DRIVER
> > +M:	Arducam Kernel Maintenance <info@arducam.com>
> > +M:	Lee Jackson <lee@arducam.com>
> > +L:	linux-media@vger.kernel.org
> > +S:	Maintained
> > +T:	git git://linuxtv.org/media_tree.git
> > +F:
> > Documentation/devicetree/bindings/media/i2c/sony,imx519.yaml
> > +F:	drivers/media/i2c/imx519.c  
> 
> The file does not exist, so path is not correct (yet). Add it in 2/2.
> 
> 
> Best regards,
> Krzysztof
> 

Best regards,
lee

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/2] dt-bindings: media: i2c: Add IMX519 CMOS sensor binding
  2022-11-03  3:04 lee
  2022-11-03 12:35 ` Rob Herring
@ 2022-11-03 13:05 ` Krzysztof Kozlowski
  2022-11-07  2:42   ` lee
  1 sibling, 1 reply; 11+ messages in thread
From: Krzysztof Kozlowski @ 2022-11-03 13:05 UTC (permalink / raw)
  To: lee, linux-media
  Cc: robh+dt, krzysztof.kozlowski+dt, shawnguo, s.hauer, devicetree

On 02/11/2022 23:04, lee wrote:
> Add YAML device tree binding for IMX519 CMOS image sensor, and
> the relevant MAINTAINERS entries.
> 
> Signed-off-by: Lee Jackson <lee@arducam.com>

Your From does not match fully SoB.

> ---
>  .../bindings/media/i2c/sony,imx519.yaml       | 107 ++++++++++++++++++
>  MAINTAINERS                                   |   9 ++
>  2 files changed, 116 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/sony,imx519.yaml
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx519.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx519.yaml
> new file mode 100644
> index 000000000000..9b6cda96f613
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx519.yaml
> @@ -0,0 +1,107 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/i2c/sony,imx519.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Sony 1/2.5-Inch 16Mpixel CMOS Digital Image Sensor
> +
> +maintainers:
> +  - Lee Jackson <lee@arducam.com>
> +
> +description: |-
> +  The Sony IMX519 is a 1/2.5-inch CMOS active pixel digital image sensor
> +  with an active array size of 4656H x 3496V. It is programmable through
> +  I2C interface. The I2C address is fixed to 0x1A 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,imx519
> +
> +  reg:
> +    description: I2C device address

Skip description.

> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  VDIG-supply:

Use lower case names.

> +    description:
> +      Digital I/O voltage supply, 1.05 volts
> +
> +  VANA-supply:
> +    description:
> +      Analog voltage supply, 2.8 volts
> +
> +  VDDL-supply:
> +    description:
> +      Digital core voltage supply, 1.8 volts
> +
> +  reset-gpios:
> +    description: |-
> +      Reference to the GPIO connected to the xclr pin, if any.
> +      Must be released (set high) after all supplies and INCK are applied.
> +
> +port:
> +    $ref: /schemas/graph.yaml#/$defs/port-base
> +    additionalProperties: false
> +
> +    properties:
> +      endpoint:
> +        $ref: /schemas/media/video-interfaces.yaml#
> +        unevaluatedProperties: false
> +
> +        properties:
> +          data-lanes:
> +            description: |-
> +              The driver only supports two-lane operation.

Please describe the hardware, not driver.

> +            items:
> +              - const: 1
> +              - const: 2
> +
> +          clock-noncontinuous: true
> +          link-frequencies: true

I think these are coming from video-interfaces, so no need for them.

> +
> +        required:
> +          - data-lanes
> +          - link-frequencies
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - VANA-supply
> +  - VDIG-supply
> +  - VDDL-supply
> +  - port
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    i2c0 {

i2c

> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +

> +        imx519: sensor@1a {
> +            compatible = "sony,imx519";
> +            reg = <0x1a>;
> +            clocks = <&imx519_clk>;
> +            VANA-supply = <&imx519_vana>;   /* 2.8v */
> +            VDIG-supply = <&imx519_vdig>;   /* 1.05v */
> +            VDDL-supply = <&imx519_vddl>;   /* 1.8v */
> +
> +            port {
> +                imx519_0: endpoint {
> +                    remote-endpoint = <&csi1_ep>;
> +                    data-lanes = <1 2>;
> +                    clock-noncontinuous;
> +                    link-frequencies = /bits/ 64 <493500000>;
> +                };
> +            };
> +        };
> +    };
> +
> +...
> diff --git a/MAINTAINERS b/MAINTAINERS
> index e04d944005ba..5a617ab8c9b2 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -19236,6 +19236,15 @@ T:	git git://linuxtv.org/media_tree.git
>  F:	Documentation/devicetree/bindings/media/i2c/sony,imx412.yaml
>  F:	drivers/media/i2c/imx412.c
>  
> +SONY IMX519 SENSOR DRIVER
> +M:	Arducam Kernel Maintenance <info@arducam.com>
> +M:	Lee Jackson <lee@arducam.com>
> +L:	linux-media@vger.kernel.org
> +S:	Maintained
> +T:	git git://linuxtv.org/media_tree.git
> +F:	Documentation/devicetree/bindings/media/i2c/sony,imx519.yaml
> +F:	drivers/media/i2c/imx519.c

The file does not exist, so path is not correct (yet). Add it in 2/2.


Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/2] dt-bindings: media: i2c: Add IMX519 CMOS sensor binding
  2022-11-03  3:04 lee
@ 2022-11-03 12:35 ` Rob Herring
  2022-11-03 13:05 ` Krzysztof Kozlowski
  1 sibling, 0 replies; 11+ messages in thread
From: Rob Herring @ 2022-11-03 12:35 UTC (permalink / raw)
  To: lee
  Cc: robh+dt, shawnguo, linux-media, s.hauer, devicetree,
	krzysztof.kozlowski+dt


On Thu, 03 Nov 2022 11:04:24 +0800, lee wrote:
> Add YAML device tree binding for IMX519 CMOS image sensor, and
> the relevant MAINTAINERS entries.
> 
> Signed-off-by: Lee Jackson <lee@arducam.com>
> ---
>  .../bindings/media/i2c/sony,imx519.yaml       | 107 ++++++++++++++++++
>  MAINTAINERS                                   |   9 ++
>  2 files changed, 116 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/sony,imx519.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:
./Documentation/devicetree/bindings/media/i2c/sony,imx519.yaml:48:5: [warning] wrong indentation: expected 2 but found 4 (indentation)

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/i2c/sony,imx519.yaml: 'port' is not one of ['$id', '$schema', 'title', 'description', 'examples', 'required', 'allOf', 'anyOf', 'oneOf', 'definitions', '$defs', 'additionalProperties', 'dependencies', 'dependentRequired', 'dependentSchemas', 'patternProperties', 'properties', 'if', 'then', 'else', 'unevaluatedProperties', 'deprecated', 'maintainers', 'select', '$ref']
	from schema $id: http://devicetree.org/meta-schemas/base.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/i2c/sony,imx519.example.dtb: sensor@1a: 'port' does not match any of the regexes: 'pinctrl-[0-9]+'
	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/media/i2c/sony,imx519.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 1/2] dt-bindings: media: i2c: Add IMX519 CMOS sensor binding
@ 2022-11-03  3:04 lee
  2022-11-03 12:35 ` Rob Herring
  2022-11-03 13:05 ` Krzysztof Kozlowski
  0 siblings, 2 replies; 11+ messages in thread
From: lee @ 2022-11-03  3:04 UTC (permalink / raw)
  To: linux-media
  Cc: robh+dt, krzysztof.kozlowski+dt, shawnguo, s.hauer, devicetree

Add YAML device tree binding for IMX519 CMOS image sensor, and
the relevant MAINTAINERS entries.

Signed-off-by: Lee Jackson <lee@arducam.com>
---
 .../bindings/media/i2c/sony,imx519.yaml       | 107 ++++++++++++++++++
 MAINTAINERS                                   |   9 ++
 2 files changed, 116 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/sony,imx519.yaml

diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx519.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx519.yaml
new file mode 100644
index 000000000000..9b6cda96f613
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/sony,imx519.yaml
@@ -0,0 +1,107 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/i2c/sony,imx519.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sony 1/2.5-Inch 16Mpixel CMOS Digital Image Sensor
+
+maintainers:
+  - Lee Jackson <lee@arducam.com>
+
+description: |-
+  The Sony IMX519 is a 1/2.5-inch CMOS active pixel digital image sensor
+  with an active array size of 4656H x 3496V. It is programmable through
+  I2C interface. The I2C address is fixed to 0x1A 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,imx519
+
+  reg:
+    description: I2C device address
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  VDIG-supply:
+    description:
+      Digital I/O voltage supply, 1.05 volts
+
+  VANA-supply:
+    description:
+      Analog voltage supply, 2.8 volts
+
+  VDDL-supply:
+    description:
+      Digital core voltage supply, 1.8 volts
+
+  reset-gpios:
+    description: |-
+      Reference to the GPIO connected to the xclr pin, if any.
+      Must be released (set high) after all supplies and INCK are applied.
+
+port:
+    $ref: /schemas/graph.yaml#/$defs/port-base
+    additionalProperties: false
+
+    properties:
+      endpoint:
+        $ref: /schemas/media/video-interfaces.yaml#
+        unevaluatedProperties: false
+
+        properties:
+          data-lanes:
+            description: |-
+              The driver only supports two-lane operation.
+            items:
+              - const: 1
+              - const: 2
+
+          clock-noncontinuous: true
+          link-frequencies: true
+
+        required:
+          - data-lanes
+          - link-frequencies
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - VANA-supply
+  - VDIG-supply
+  - VDDL-supply
+  - port
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c0 {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        imx519: sensor@1a {
+            compatible = "sony,imx519";
+            reg = <0x1a>;
+            clocks = <&imx519_clk>;
+            VANA-supply = <&imx519_vana>;   /* 2.8v */
+            VDIG-supply = <&imx519_vdig>;   /* 1.05v */
+            VDDL-supply = <&imx519_vddl>;   /* 1.8v */
+
+            port {
+                imx519_0: endpoint {
+                    remote-endpoint = <&csi1_ep>;
+                    data-lanes = <1 2>;
+                    clock-noncontinuous;
+                    link-frequencies = /bits/ 64 <493500000>;
+                };
+            };
+        };
+    };
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index e04d944005ba..5a617ab8c9b2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19236,6 +19236,15 @@ T:	git git://linuxtv.org/media_tree.git
 F:	Documentation/devicetree/bindings/media/i2c/sony,imx412.yaml
 F:	drivers/media/i2c/imx412.c
 
+SONY IMX519 SENSOR DRIVER
+M:	Arducam Kernel Maintenance <info@arducam.com>
+M:	Lee Jackson <lee@arducam.com>
+L:	linux-media@vger.kernel.org
+S:	Maintained
+T:	git git://linuxtv.org/media_tree.git
+F:	Documentation/devicetree/bindings/media/i2c/sony,imx519.yaml
+F:	drivers/media/i2c/imx519.c
+
 SONY MEMORYSTICK SUBSYSTEM
 M:	Maxim Levitsky <maximlevitsky@gmail.com>
 M:	Alex Dubov <oakad@yahoo.com>
-- 
2.25.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2022-11-16  1:37 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-04  7:26 [PATCH v2 1/2] dt-bindings: media: i2c: Add IMX519 CMOS sensor binding lee
2022-11-04 12:36 ` Rob Herring
2022-11-04 13:17 ` Krzysztof Kozlowski
2022-11-16  1:37   ` lee
2022-11-04 13:18 ` Krzysztof Kozlowski
  -- strict thread matches above, loose matches on Subject: below --
2022-11-03  3:04 lee
2022-11-03 12:35 ` Rob Herring
2022-11-03 13:05 ` Krzysztof Kozlowski
2022-11-07  2:42   ` lee
2022-11-07  8:24     ` Krzysztof Kozlowski
2022-11-07 10:24       ` lee

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.