Hello Laurent, On 2023/4/24 20:02, Laurent Pinchart wrote: > Hi Jack, > > On Thu, Apr 20, 2023 at 04:51:39PM +0800, Jack Zhu wrote: >> On 2023/4/19 14:15, Laurent Pinchart wrote: >> > On Thu, Apr 13, 2023 at 11:55:39AM +0800, 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@xxxxxxxxxxxxxxxx> >> >> --- >> >> .../bindings/media/starfive,jh7110-camss.yaml | 164 ++++++++++++++++++ >> >> MAINTAINERS | 7 + >> >> 2 files changed, 171 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..4cd144f1b845 >> >> --- /dev/null >> >> +++ b/Documentation/devicetree/bindings/media/starfive,jh7110-camss.yaml >> >> @@ -0,0 +1,164 @@ >> >> +# 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@xxxxxxxxxxxxxxxx> >> >> + - Changhuang Liang <changhuang.liang@xxxxxxxxxxxxxxxx> >> >> + >> >> +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#/$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 >> > >> > As far as I can tell, those two properties are not handled by the >> > driver. I assume this is because the driver doesn't support the DVP >> > input yet. That's fine, but it makes it a bit hard to review the device >> > tree. Could you provide some information about the DVP hardware >> > interface ? Does it support both BT.656 and sync signals, or just sync >> > signals ? Are the polarities of the clock and h/v sync controllable ? >> > Is the parallel input bus 8-bit wide or are other options supported ? >> > And finally, what are you modelling with data-shift: 2 ? >> >> Hello Laurent, >> >> The DVP hardware supports BT.656 and sync signals, can control the >> polarities of h/v sync, supports 8/10/12 bit wide, and data-shift: 2 is >> line 9-2. > > Thank you for the information. Endpoints for port@0 should then require > bus-type. The hsync-active and vsync-active should be listed with > default values and be optional. bus-width should have 8, 10 and 12 as > supported values, and be required, or have a default (I think making it > required is best). data-shift should list supported values too, > including 0, and be optional with a default of 0. > Thank you for your suggestion. I will make the following modifications: properties: bus-type: enum: [5, 6] bus-width: enum: [8, 10, 12] data-shift: enum: [0, 2] default: 0 hsync-active: enum: [0, 1] default: 1 vsync-active: enum: [0, 1] default: 1 required: - bus-type - bus-width >> >> + >> >> + port@1: >> >> + $ref: /schemas/graph.yaml#/properties/port >> >> + description: Input port for receiving CSI data. >> >> + >> >> + required: >> >> + - port@0 >> >> + - port@1 >> >> + >> >> +required: >> >> + - compatible >> >> + - reg >> >> + - reg-names >> >> + - clocks >> >> + - clock-names >> >> + - resets >> >> + - reset-names >> >> + - power-domains >> >> + - interrupts >> >> + - ports >> >> + >> >> +additionalProperties: false >> >> + >> >> +examples: >> >> + - | >> >> + isp@19840000 { >> >> + compatible = "starfive,jh7110-camss"; >> >> + reg = <0x19840000 0x10000>, >> >> + <0x19870000 0x30000>; >> >> + reg-names = "syscon", "isp"; >> >> + clocks = <&ispcrg 0>, >> >> + <&ispcrg 13>, >> >> + <&ispcrg 2>, >> >> + <&ispcrg 12>, >> >> + <&ispcrg 1>, >> >> + <&syscrg 51>, >> >> + <&syscrg 52>; >> >> + clock-names = "apb_func", >> >> + "wrapper_clk_c", >> >> + "dvp_inv", >> >> + "axiwr", >> >> + "mipi_rx0_pxl", >> >> + "ispcore_2x", >> >> + "isp_axi"; >> >> + resets = <&ispcrg 0>, >> >> + <&ispcrg 1>, >> >> + <&ispcrg 10>, >> >> + <&ispcrg 11>, >> >> + <&syscrg 41>, >> >> + <&syscrg 42>; >> >> + reset-names = "wrapper_p", >> >> + "wrapper_c", >> >> + "axird", >> >> + "axiwr", >> >> + "isp_top_n", >> >> + "isp_top_axi"; >> >> + power-domains = <&pwrc 5>; >> >> + interrupts = <92>, <87>, <88>, <90>; >> >> + >> >> + ports { >> >> + #address-cells = <1>; >> >> + #size-cells = <0>; >> >> + 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>; >> >> + }; >> >> + }; >> >> + >> >> + port@1 { >> >> + reg = <1>; >> >> + vin_from_csi2rx: endpoint { >> >> + remote-endpoint = <&csi2rx_to_vin>; >> >> + }; >> >> + }; >> >> + }; >> >> + }; >> >> diff --git a/MAINTAINERS b/MAINTAINERS >> >> index bbb8b5c0187b..b8c76b0d7eb3 100644 >> >> --- a/MAINTAINERS >> >> +++ b/MAINTAINERS >> >> @@ -19909,6 +19909,13 @@ M: Ion Badulescu <ionut@xxxxxxxxxx> >> >> S: Odd Fixes >> >> F: drivers/net/ethernet/adaptec/starfire* >> >> >> >> +STARFIVE CAMERA SUBSYSTEM DRIVER >> >> +M: Jack Zhu <jack.zhu@xxxxxxxxxxxxxxxx> >> >> +M: Changhuang Liang <changhuang.liang@xxxxxxxxxxxxxxxx> >> >> +L: linux-media@xxxxxxxxxxxxxxx >> >> +S: Maintained >> >> +F: Documentation/devicetree/bindings/media/starfive,jh7110-camss.yaml >> >> + >> >> STARFIVE DEVICETREES >> >> M: Emil Renner Berthing <kernel@xxxxxxxx> >> >> S: Maintained >
