Re: [PATCH v3 1/9] media: dt-bindings: Add bindings for JH7110 Camera Subsystem

Jack Zhu
2023-04-04T02:44:15Z

On 2023/4/4 5:48, Rob Herring wrote:
> On Mon, Apr 03, 2023 at 05:14:25PM +0800, Jack Zhu wrote:
>>
>>
>> On 2023/4/1 4:09, Krzysztof Kozlowski wrote:
>> > On 31/03/2023 14:18, Jack Zhu wrote:
>> >> Add the bindings documentation for Starfive JH7110 Camera Subsystem
>> >> which is used for handing image sensor data.
>> >>
>> >> Signed-off-by: Jack Zhu <jack.zhu@starfivetech.com>
>> >> ---
>> >> .../bindings/media/starfive,jh7110-camss.yaml | 159 ++++++++++++++++++
>> >> MAINTAINERS | 7 +
>> >> 2 files changed, 166 insertions(+)
>> >> create mode 100644 Documentation/devicetree/bindings/media/starfive,jh7110-camss.yaml
>> >>
>> >> diff --git a/Documentation/devicetree/bindings/media/starfive,jh7110-camss.yaml b/Documentation/devicetree/bindings/media/starfive,jh7110-camss.yaml
>> >> new file mode 100644
>> >> index 000000000000..0235c70e7793
>> >> --- /dev/null
>> >> +++ b/Documentation/devicetree/bindings/media/starfive,jh7110-camss.yaml
>> >> @@ -0,0 +1,159 @@
>> >> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> >> +
>> >> +%YAML 1.2
>> >> +---
>> >> +$id: http://devicetree.org/schemas/media/starfive,jh7110-camss.yaml#
>> >> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> >> +
>> >> +title: Starfive SoC CAMSS ISP
>> >> +
>> >> +maintainers:
>> >> + - Jack Zhu <jack.zhu@starfivetech.com>
>> >> + - Changhuang Liang <changhuang.liang@starfivetech.com>
>> >> +
>> >> +description:
>> >> + The Starfive CAMSS ISP is a Camera interface for Starfive JH7110 SoC. It
>> >> + consists of a VIN controller (Video In Controller, a top-level control until)
>> >> + and an ISP.
>> >> +
>> >> +properties:
>> >> + compatible:
>> >> + const: starfive,jh7110-camss
>> >> +
>> >> + reg:
>> >> + maxItems: 2
>> >> +
>> >> + reg-names:
>> >> + items:
>> >> + - const: syscon
>> >> + - const: isp
>> >> +
>> >> + clocks:
>> >> + maxItems: 7
>> >> +
>> >> + clock-names:
>> >> + items:
>> >> + - const: apb_func
>> >> + - const: wrapper_clk_c
>> >> + - const: dvp_inv
>> >> + - const: axiwr
>> >> + - const: mipi_rx0_pxl
>> >> + - const: ispcore_2x
>> >> + - const: isp_axi
>> >> +
>> >> + resets:
>> >> + maxItems: 6
>> >> +
>> >> + reset-names:
>> >> + items:
>> >> + - const: wrapper_p
>> >> + - const: wrapper_c
>> >> + - const: axird
>> >> + - const: axiwr
>> >> + - const: isp_top_n
>> >> + - const: isp_top_axi
>> >> +
>> >> + power-domains:
>> >> + items:
>> >> + - description: JH7110 ISP Power Domain Switch Controller.
>> >> +
>> >> + interrupts:
>> >> + maxItems: 4
>> >> +
>> >> + ports:
>> >> + $ref: /schemas/graph.yaml#/properties/ports
>> >> +
>> >> + properties:
>> >> + port@0:
>> >> + $ref: /schemas/graph.yaml#/properties/port
>> >> + unevaluatedProperties: false
>> >> + description:
>> >> + Input port for receiving DVP data.
>> >> +
>> >> + properties:
>> >> + endpoint:
>> >> + $ref: video-interfaces.yaml#
>> >
>> > I don't think we use video-interfaces with port. Are you sure this works
>> > fine? Please extend the example with appropriate properties to check...
>> >
>> Thank you for your review.
>> the following change, is it OK?
>
> What Krzysztof meant is port@0 needs to $ref '$defs/port-base' rather
> then 'properties/port' to work with video-interfaces.yaml. If you made
> that change, then this is fine.
>
Thank you for your comment.
OK, I have made that change. Thank you.

port@0:
$ref: /schemas/graph.yaml#/$defs/port-base
unevaluatedProperties: false
description: Input port for receiving DVP data.

properties:
endpoint:
$ref: video-interfaces.yaml#
unevaluatedProperties: false

properties:
bus-width:
const: 8

data-shift:
const: 2

>>
>> endpoint:
>> $ref: video-interfaces.yaml#
>> unevaluatedProperties: false
>>
>> properties:
>> bus-width:
>> const: 8
>>
>> data-shift:
>> const: 2
>>
>> in example:
>> port@0 {
>> reg = <0>;
>> vin_from_sc2235: endpoint {
>> remote-endpoint = <&sc2235_to_vin>;
>> bus-width = <8>;
>> data-shift = <2>;
>> hsync-active = <1>;
>> vsync-active = <0>;
>> pclk-sample = <1>;
>> };
>> };
>>
>>
>> >> + unevaluatedProperties: false
>> >> +
>> >> + port@1:
>> >> + $ref: /schemas/graph.yaml#/properties/port
>> >> + unevaluatedProperties: false
>> >> + description:
>> >> + Input port for receiving CSI data.
>> >> +
>> >> + properties:
>> >> + endpoint:
>> >> + $ref: video-interfaces.yaml#
>> >> + unevaluatedProperties: false
>> >
>> > Same concerns here.
>> >
>> for port@1, We don't use any properties as the allwinner,sun6i-a31-isp.yaml file.
>> Use the following modification, I don't know if it is ok?
>
> Yes.
>
>> port@1:
>> $ref: /schemas/graph.yaml#/properties/port
>> description: Input port for receiving CSI data.
>
> Rob