mbox series

[v2,0/6] Add Starfive Camera Subsystem driver

Message ID 20230310120553.60586-1-jack.zhu@starfivetech.com
Headers show
Series Add Starfive Camera Subsystem driver | expand

Message

Jack Zhu March 10, 2023, 12:05 p.m. UTC
Hi,

This patch series adds support for the Starfive Camera Subsystem
found on Starfive JH7110 SoC.

The driver implements V4L2, Media controller and V4L2 subdev interfaces.
Camera sensor using V4L2 subdev interface in the kernel is supported.

The driver is tested on VisionFive V2 board with IMX219 camera sensor.
GStreamer 1.18.5 with v4l2src plugin is supported.

Changes since v1:
- Deleted starfive,jh7110-mipi-csi2.yaml.
- Converted cdns,csi2rx.txt to cdns,csi2rx.yaml and added ‘resets’
  properties.
- Added ‘cdns,csi2rx.yaml’ in ‘CADENCE MIPI-CSI2 BRIDGES’ entry.
- The following contents were modified in starfive,jh7110-camss.yaml:
  dropped quotes from ’id’ and ‘schema’; dropped ‘|’ for ‘description’;
  corrected the wrong or redundant words: ‘a ISP’, ‘PD ISP’;
  dropped ‘minItems’ for ‘reg’, ‘clocks’, ‘resets’ and ‘interrupts’;
  dropped the '_clk' and 'rst_' prefix about the 'clock-names' and
  'reset-names';
  changed ‘endpoint@1’ to ‘endpoint’; updated examples;
- Updated Subject for some patches.
- Merged patch 6, 7, 8, 9, 10, 11 into one patch.

Jack Zhu (6):
  media: dt-bindings: Add bindings for JH7110 Camera Subsystem
  media: dt-bindings: cadence-csi2rx: Convert to DT schema
  media: admin-guide: Add starfive_camss.rst for Starfive Camera
    Subsystem
  media: cadence: Add support for external dphy and JH7110 SoC
  MAINTAINERS: Add Starfive Camera Subsystem driver
  media: starfive: Add Starfive Camera Subsystem driver

 .../admin-guide/media/starfive_camss.rst      |   68 +
 .../media/starfive_camss_graph.dot            |   28 +
 .../admin-guide/media/v4l-drivers.rst         |    1 +
 .../devicetree/bindings/media/cdns,csi2rx.txt |  100 --
 .../bindings/media/cdns,csi2rx.yaml           |  163 ++
 .../bindings/media/starfive,jh7110-camss.yaml |  144 ++
 MAINTAINERS                                   |   10 +
 drivers/media/platform/Kconfig                |    1 +
 drivers/media/platform/Makefile               |    1 +
 drivers/media/platform/cadence/cdns-csi2rx.c  |  273 +++-
 drivers/media/platform/starfive/Kconfig       |   18 +
 drivers/media/platform/starfive/Makefile      |   14 +
 drivers/media/platform/starfive/stf_camss.c   |  728 +++++++++
 drivers/media/platform/starfive/stf_camss.h   |  104 ++
 drivers/media/platform/starfive/stf_common.h  |  149 ++
 drivers/media/platform/starfive/stf_isp.c     | 1079 ++++++++++++++
 drivers/media/platform/starfive/stf_isp.h     |  183 +++
 .../media/platform/starfive/stf_isp_hw_ops.c  | 1286 ++++++++++++++++
 drivers/media/platform/starfive/stf_video.c   | 1286 ++++++++++++++++
 drivers/media/platform/starfive/stf_video.h   |   89 ++
 drivers/media/platform/starfive/stf_vin.c     | 1314 +++++++++++++++++
 drivers/media/platform/starfive/stf_vin.h     |  194 +++
 .../media/platform/starfive/stf_vin_hw_ops.c  |  357 +++++
 include/uapi/linux/stf_isp_ioctl.h            |  127 ++
 24 files changed, 7607 insertions(+), 110 deletions(-)
 create mode 100644 Documentation/admin-guide/media/starfive_camss.rst
 create mode 100644 Documentation/admin-guide/media/starfive_camss_graph.dot
 delete mode 100644 Documentation/devicetree/bindings/media/cdns,csi2rx.txt
 create mode 100644 Documentation/devicetree/bindings/media/cdns,csi2rx.yaml
 create mode 100644 Documentation/devicetree/bindings/media/starfive,jh7110-camss.yaml
 create mode 100644 drivers/media/platform/starfive/Kconfig
 create mode 100644 drivers/media/platform/starfive/Makefile
 create mode 100644 drivers/media/platform/starfive/stf_camss.c
 create mode 100644 drivers/media/platform/starfive/stf_camss.h
 create mode 100644 drivers/media/platform/starfive/stf_common.h
 create mode 100644 drivers/media/platform/starfive/stf_isp.c
 create mode 100644 drivers/media/platform/starfive/stf_isp.h
 create mode 100644 drivers/media/platform/starfive/stf_isp_hw_ops.c
 create mode 100644 drivers/media/platform/starfive/stf_video.c
 create mode 100644 drivers/media/platform/starfive/stf_video.h
 create mode 100644 drivers/media/platform/starfive/stf_vin.c
 create mode 100644 drivers/media/platform/starfive/stf_vin.h
 create mode 100644 drivers/media/platform/starfive/stf_vin_hw_ops.c
 create mode 100644 include/uapi/linux/stf_isp_ioctl.h

Comments

Laurent Pinchart March 12, 2023, 11:12 a.m. UTC | #1
Hi Jack,

Thank you for the patch.

On Fri, Mar 10, 2023 at 08:05:48PM +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@starfivetech.com>
> ---
>  .../bindings/media/starfive,jh7110-camss.yaml | 144 ++++++++++++++++++
>  1 file changed, 144 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..3f348dd53441
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/starfive,jh7110-camss.yaml
> @@ -0,0 +1,144 @@
> +# 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

s/.It/. It/

> +  consists of a VIN controller(Video In Controller, a top-level control until)

s/(/ (/

> +  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@1:

Why port@1 if there is no port@0 ?

> +        $ref: /schemas/graph.yaml#/$defs/port-base

I think you can use

        $ref: /schemas/graph.yaml#/properties/port

here as you don't define any additional property for the port node.

> +        unevaluatedProperties: false
> +        description:
> +          Input port for receiving CSI data.
> +
> +        properties:
> +          endpoint:
> +            $ref: video-interfaces.yaml#
> +            unevaluatedProperties: false
> +
> +    required:
> +      - port@1
> +
> +required:
> +  - compatible
> +  - reg
> +  - reg-names
> +  - clocks
> +  - clock-names
> +  - resets
> +  - reset-names
> +  - power-domains
> +  - interrupts

The 'ports' node should be mandatory too.

> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    stfcamss: isp@19840000 {

You can drop the label as it's not used.

> +        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@1 {
> +                reg = <1>;
> +                #address-cells = <1>;
> +                #size-cells = <0>;
> +
> +                vin_from_csi2rx: endpoint {
> +                    remote-endpoint = <&csi2rx_to_vin>;
> +                };
> +            };
> +        };
> +    };
Laurent Pinchart March 12, 2023, 11:14 a.m. UTC | #2
Hi Jack,

Thank you for the patch.

On Fri, Mar 10, 2023 at 08:05:52PM +0800, Jack Zhu wrote:
> Add an entry for Starfive Camera Subsystem driver.
> 
> Signed-off-by: Jack Zhu <jack.zhu@starfivetech.com>
> ---
>  MAINTAINERS | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index b2e7ca5603c3..c67faea9f967 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -19907,6 +19907,15 @@ M:	Ion Badulescu <ionut@badula.org>
>  S:	Odd Fixes
>  F:	drivers/net/ethernet/adaptec/starfire*
>  
> +STARFIVE CAMERA SUBSYSTEM DRIVER
> +M:	Jack Zhu <jack.zhu@starfivetech.com>
> +M:	Changhuang Liang <changhuang.liang@starfivetech.com>
> +L:	linux-media@vger.kernel.org
> +S:	Maintained
> +F:	Documentation/admin-guide/media/starfive_camss.rst
> +F:	Documentation/devicetree/bindings/media/starfive,jh7110-camss.yaml
> +F:	drivers/media/platform/starfive/

You could also add the MAINTAINERS entry in patch 1/6, with just the
.yaml file to start with, and extend it in the patches that add more
files. Up to you, I don't mind much.

> +
>  STARFIVE DEVICETREES
>  M:	Emil Renner Berthing <kernel@esmil.dk>
>  S:	Maintained
Krzysztof Kozlowski March 12, 2023, 11:16 a.m. UTC | #3
On 10/03/2023 13:05, 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 | 144 ++++++++++++++++++
>  1 file changed, 144 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..3f348dd53441
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/starfive,jh7110-camss.yaml
> @@ -0,0 +1,144 @@
> +# 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:

You should have here port@0. The binding does not represent what is
supported by Linux. Binding represents hardware and you said you have
port@0 for the sensor.


> +      port@1:
> +        $ref: /schemas/graph.yaml#/$defs/port-base
> +        unevaluatedProperties: false
> +        description:
> +          Input port for receiving CSI data.
> +
> +        properties:
> +          endpoint:
> +            $ref: video-interfaces.yaml#
> +            unevaluatedProperties: false
> +
> +    required:
> +      - port@1
> +
> +required:
> +  - compatible
> +  - reg
> +  - reg-names
> +  - clocks
> +  - clock-names
> +  - resets
> +  - reset-names
> +  - power-domains
> +  - interrupts
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    stfcamss: 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>;
> +

Same here, DTS describes the hardware, so you need port@0.



Best regards,
Krzysztof
Jack Zhu March 20, 2023, 6:24 a.m. UTC | #4
On 2023/3/12 19:12, Laurent Pinchart wrote:
> Hi Jack,
> 
> Thank you for the patch.

Thank you for your comments.

> 
> On Fri, Mar 10, 2023 at 08:05:48PM +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@starfivetech.com>
>> ---
>>  .../bindings/media/starfive,jh7110-camss.yaml | 144 ++++++++++++++++++
>>  1 file changed, 144 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..3f348dd53441
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/starfive,jh7110-camss.yaml
>> @@ -0,0 +1,144 @@
>> +# 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
> 
> s/.It/. It/

OK, will fix

> 
>> +  consists of a VIN controller(Video In Controller, a top-level control until)
> 
> s/(/ (/

OK, will fix

> 
>> +  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@1:
> 
> Why port@1 if there is no port@0 ?

Will add port@0 node in the next version.
port@0 is reserved for DVP sensor which is not currently supported.

> 
>> +        $ref: /schemas/graph.yaml#/$defs/port-base
> 
> I think you can use
> 
>         $ref: /schemas/graph.yaml#/properties/port
> 
> here as you don't define any additional property for the port node.

OK, will alter it

> 
>> +        unevaluatedProperties: false
>> +        description:
>> +          Input port for receiving CSI data.
>> +
>> +        properties:
>> +          endpoint:
>> +            $ref: video-interfaces.yaml#
>> +            unevaluatedProperties: false
>> +
>> +    required:
>> +      - port@1
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - reg-names
>> +  - clocks
>> +  - clock-names
>> +  - resets
>> +  - reset-names
>> +  - power-domains
>> +  - interrupts
> 
> The 'ports' node should be mandatory too.

OK, will fix. Thanks.

> 
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    stfcamss: isp@19840000 {
> 
> You can drop the label as it's not used.

OK, will drop.

> 
>> +        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@1 {
>> +                reg = <1>;
>> +                #address-cells = <1>;
>> +                #size-cells = <0>;
>> +
>> +                vin_from_csi2rx: endpoint {
>> +                    remote-endpoint = <&csi2rx_to_vin>;
>> +                };
>> +            };
>> +        };
>> +    };
>
Jack Zhu March 20, 2023, 6:27 a.m. UTC | #5
On 2023/3/12 19:16, Krzysztof Kozlowski wrote:
> On 10/03/2023 13:05, 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 | 144 ++++++++++++++++++
>>  1 file changed, 144 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..3f348dd53441
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/starfive,jh7110-camss.yaml
>> @@ -0,0 +1,144 @@
>> +# 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:
> 
> You should have here port@0. The binding does not represent what is
> supported by Linux. Binding represents hardware and you said you have
> port@0 for the sensor.
> 

OK, will add port@0. Thanks.

> 
>> +      port@1:
>> +        $ref: /schemas/graph.yaml#/$defs/port-base
>> +        unevaluatedProperties: false
>> +        description:
>> +          Input port for receiving CSI data.
>> +
>> +        properties:
>> +          endpoint:
>> +            $ref: video-interfaces.yaml#
>> +            unevaluatedProperties: false
>> +
>> +    required:
>> +      - port@1
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - reg-names
>> +  - clocks
>> +  - clock-names
>> +  - resets
>> +  - reset-names
>> +  - power-domains
>> +  - interrupts
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    stfcamss: 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>;
>> +
> 
> Same here, DTS describes the hardware, so you need port@0.

OK, will add it.

> 
> 
> 
> Best regards,
> Krzysztof
>
Jack Zhu March 20, 2023, 12:03 p.m. UTC | #6
On 2023/3/12 19:14, Laurent Pinchart wrote:
> Hi Jack,
> 
> Thank you for the patch.
> 
> On Fri, Mar 10, 2023 at 08:05:52PM +0800, Jack Zhu wrote:
>> Add an entry for Starfive Camera Subsystem driver.
>> 
>> Signed-off-by: Jack Zhu <jack.zhu@starfivetech.com>
>> ---
>>  MAINTAINERS | 9 +++++++++
>>  1 file changed, 9 insertions(+)
>> 
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index b2e7ca5603c3..c67faea9f967 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -19907,6 +19907,15 @@ M:	Ion Badulescu <ionut@badula.org>
>>  S:	Odd Fixes
>>  F:	drivers/net/ethernet/adaptec/starfire*
>>  
>> +STARFIVE CAMERA SUBSYSTEM DRIVER
>> +M:	Jack Zhu <jack.zhu@starfivetech.com>
>> +M:	Changhuang Liang <changhuang.liang@starfivetech.com>
>> +L:	linux-media@vger.kernel.org
>> +S:	Maintained
>> +F:	Documentation/admin-guide/media/starfive_camss.rst
>> +F:	Documentation/devicetree/bindings/media/starfive,jh7110-camss.yaml
>> +F:	drivers/media/platform/starfive/
> 
> You could also add the MAINTAINERS entry in patch 1/6, with just the
> .yaml file to start with, and extend it in the patches that add more
> files. Up to you, I don't mind much.

Thank you for your suggestion, I will do that.

> 
>> +
>>  STARFIVE DEVICETREES
>>  M:	Emil Renner Berthing <kernel@esmil.dk>
>>  S:	Maintained
>