diff mbox series

[v1,1/2] dt-bindings: media: starfive: Match driver and yaml property names

Message ID 20240119100639.84029-2-changhuang.liang@starfivetech.com (mailing list archive)
State Handled Elsewhere
Delegated to: Conor Dooley
Headers show
Series Add camera subsystem for StarFive | expand

Checks

Context Check Description
conchuod/vmtest-for-next-PR success PR summary
conchuod/patch-1-test-1 success .github/scripts/patches/tests/build_rv32_defconfig.sh
conchuod/patch-1-test-2 success .github/scripts/patches/tests/build_rv64_clang_allmodconfig.sh
conchuod/patch-1-test-3 success .github/scripts/patches/tests/build_rv64_gcc_allmodconfig.sh
conchuod/patch-1-test-4 success .github/scripts/patches/tests/build_rv64_nommu_k210_defconfig.sh
conchuod/patch-1-test-5 success .github/scripts/patches/tests/build_rv64_nommu_virt_defconfig.sh
conchuod/patch-1-test-6 success .github/scripts/patches/tests/checkpatch.sh
conchuod/patch-1-test-7 success .github/scripts/patches/tests/dtb_warn_rv64.sh
conchuod/patch-1-test-8 success .github/scripts/patches/tests/header_inline.sh
conchuod/patch-1-test-9 success .github/scripts/patches/tests/kdoc.sh
conchuod/patch-1-test-10 success .github/scripts/patches/tests/module_param.sh
conchuod/patch-1-test-11 success .github/scripts/patches/tests/verify_fixes.sh
conchuod/patch-1-test-12 success .github/scripts/patches/tests/verify_signedoff.sh

Commit Message

Changhuang Liang Jan. 19, 2024, 10:06 a.m. UTC 
Drop some unused properties for clocks, resets and interrupts for
StarFive JH7110 camera subsystem.

Reviewed-by: Hal Feng <hal.feng@starfivetech.com>
Reviewed-by: Jack Zhu <jack.zhu@starfivetech.com>
Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
---
 .../bindings/media/starfive,jh7110-camss.yaml | 31 +++++--------------
 1 file changed, 8 insertions(+), 23 deletions(-)

Comments

Conor Dooley Jan. 19, 2024, 10:16 a.m. UTC | #1 
On Fri, Jan 19, 2024 at 02:06:38AM -0800, Changhuang Liang wrote:
> Drop some unused properties for clocks, resets and interrupts for
> StarFive JH7110 camera subsystem.

What do you mean "unused"?

Do these clocks etc exist but are not used by the driver?

Or do they not exist at all?

The two are very different!

Thanks,
Conor.

> 
> Reviewed-by: Hal Feng <hal.feng@starfivetech.com>
> Reviewed-by: Jack Zhu <jack.zhu@starfivetech.com>
> Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
> ---
>  .../bindings/media/starfive,jh7110-camss.yaml | 31 +++++--------------
>  1 file changed, 8 insertions(+), 23 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/starfive,jh7110-camss.yaml b/Documentation/devicetree/bindings/media/starfive,jh7110-camss.yaml
> index c66586d90fa2..2504381058b3 100644
> --- a/Documentation/devicetree/bindings/media/starfive,jh7110-camss.yaml
> +++ b/Documentation/devicetree/bindings/media/starfive,jh7110-camss.yaml
> @@ -4,14 +4,14 @@
>  $id: http://devicetree.org/schemas/media/starfive,jh7110-camss.yaml#
>  $schema: http://devicetree.org/meta-schemas/core.yaml#
>  
> -title: Starfive SoC CAMSS ISP
> +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
> +  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 unit)
>    and an ISP.
>  
> @@ -28,26 +28,21 @@ properties:
>        - const: isp
>  
>    clocks:
> -    maxItems: 7
> +    maxItems: 3
>  
>    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
> +    maxItems: 5
>  
>    reset-names:
>      items:
>        - const: wrapper_p
>        - const: wrapper_c
> -      - const: axird
>        - const: axiwr
>        - const: isp_top_n
>        - const: isp_top_axi
> @@ -57,7 +52,7 @@ properties:
>        - description: JH7110 ISP Power Domain Switch Controller.
>  
>    interrupts:
> -    maxItems: 4
> +    maxItems: 3
>  
>    ports:
>      $ref: /schemas/graph.yaml#/properties/ports
> @@ -125,34 +120,24 @@ examples:
>          reg = <0x19840000 0x10000>,
>                <0x19870000 0x30000>;
>          reg-names = "syscon", "isp";
> -        clocks = <&ispcrg 0>,
> -                 <&ispcrg 13>,
> -                 <&ispcrg 2>,
> -                 <&ispcrg 12>,
> -                 <&ispcrg 1>,
> +        clocks = <&ispcrg 13>,
>                   <&syscrg 51>,
>                   <&syscrg 52>;
> -        clock-names = "apb_func",
> -                      "wrapper_clk_c",
> -                      "dvp_inv",
> -                      "axiwr",
> -                      "mipi_rx0_pxl",
> +        clock-names = "wrapper_clk_c",
>                        "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>;
> +        interrupts = <92>, <87>, <90>;
>  
>          ports {
>              #address-cells = <1>;
> -- 
> 2.25.1
> 
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
Changhuang Liang Jan. 19, 2024, 12:57 p.m. UTC | #2 
Hi , Conor

> 主题: Re: [PATCH v1 1/2] dt-bindings: media: starfive: Match driver and yaml
> property names
> 
> On Fri, Jan 19, 2024 at 02:06:38AM -0800, Changhuang Liang wrote:
> > Drop some unused properties for clocks, resets and interrupts for
> > StarFive JH7110 camera subsystem.
> 
> What do you mean "unused"?
> 
> Do these clocks etc exist but are not used by the driver?
> 
> Or do they not exist at all?
> 
> The two are very different!
> 
> Thanks,
> Conor.
> 

These clocks etc exist but are not used by the driver.
Conor Dooley Jan. 19, 2024, 1:58 p.m. UTC | #3 
On Fri, Jan 19, 2024 at 12:57:22PM +0000, Changhuang Liang wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Hi , Conor
> 
> > 主题: Re: [PATCH v1 1/2] dt-bindings: media: starfive: Match driver and yaml
> > property names
> >
> > On Fri, Jan 19, 2024 at 02:06:38AM -0800, Changhuang Liang wrote:
> > > Drop some unused properties for clocks, resets and interrupts for
> > > StarFive JH7110 camera subsystem.
> >
> > What do you mean "unused"?
> >
> > Do these clocks etc exist but are not used by the driver?
> >
> > Or do they not exist at all?
> >
> > The two are very different!

> These clocks etc exist but are not used by the driver.

That's not an acceptable reason for removing them from the binding. If
they exist, they should be documented, regardless of whether the driver
makes use of them. NAK.

Also, your mail didn't end up on LKML, so there's something wrong with
your setup.

Thanks,
Conor.
Changhuang Liang Jan. 22, 2024, 1:33 a.m. UTC | #4 
> On Fri, Jan 19, 2024 at 12:57:22PM +0000, Changhuang Liang wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know
> > the content is safe
> >
> > Hi , Conor
> >
> > > 主题: Re: [PATCH v1 1/2] dt-bindings: media: starfive: Match driver
> > > and yaml property names
> > >
> > > On Fri, Jan 19, 2024 at 02:06:38AM -0800, Changhuang Liang wrote:
> > > > Drop some unused properties for clocks, resets and interrupts for
> > > > StarFive JH7110 camera subsystem.
> > >
> > > What do you mean "unused"?
> > >
> > > Do these clocks etc exist but are not used by the driver?
> > >
> > > Or do they not exist at all?
> > >
> > > The two are very different!
> 
> > These clocks etc exist but are not used by the driver.
> 
> That's not an acceptable reason for removing them from the binding. If they
> exist, they should be documented, regardless of whether the driver makes use
> of them. NAK.

If so, how to avoid the warning of dtbs_check.

Thanks,
Changhuang
Conor Dooley Jan. 22, 2024, 8:24 a.m. UTC | #5 
On Mon, Jan 22, 2024 at 01:33:56AM +0000, Changhuang Liang wrote:
> 
> > On Fri, Jan 19, 2024 at 12:57:22PM +0000, Changhuang Liang wrote:
> > > EXTERNAL EMAIL: Do not click links or open attachments unless you know
> > > the content is safe
> > >
> > > Hi , Conor
> > >
> > > > 主题: Re: [PATCH v1 1/2] dt-bindings: media: starfive: Match driver
> > > > and yaml property names
> > > >
> > > > On Fri, Jan 19, 2024 at 02:06:38AM -0800, Changhuang Liang wrote:
> > > > > Drop some unused properties for clocks, resets and interrupts for
> > > > > StarFive JH7110 camera subsystem.
> > > >
> > > > What do you mean "unused"?
> > > >
> > > > Do these clocks etc exist but are not used by the driver?
> > > >
> > > > Or do they not exist at all?
> > > >
> > > > The two are very different!
> > 
> > > These clocks etc exist but are not used by the driver.
> > 
> > That's not an acceptable reason for removing them from the binding. If they
> > exist, they should be documented, regardless of whether the driver makes use
> > of them. NAK.
> 
> If so, how to avoid the warning of dtbs_check.

By also adding the clocks, resets and interrupts to the dts.
Conor Dooley March 27, 2024, 6:03 p.m. UTC | #6 
On Mon, Jan 22, 2024 at 08:24:51AM +0000, Conor Dooley wrote:
> On Mon, Jan 22, 2024 at 01:33:56AM +0000, Changhuang Liang wrote:
> > 
> > > On Fri, Jan 19, 2024 at 12:57:22PM +0000, Changhuang Liang wrote:
> > > > EXTERNAL EMAIL: Do not click links or open attachments unless you know
> > > > the content is safe
> > > >
> > > > Hi , Conor
> > > >
> > > > > 主题: Re: [PATCH v1 1/2] dt-bindings: media: starfive: Match driver
> > > > > and yaml property names
> > > > >
> > > > > On Fri, Jan 19, 2024 at 02:06:38AM -0800, Changhuang Liang wrote:
> > > > > > Drop some unused properties for clocks, resets and interrupts for
> > > > > > StarFive JH7110 camera subsystem.
> > > > >
> > > > > What do you mean "unused"?
> > > > >
> > > > > Do these clocks etc exist but are not used by the driver?
> > > > >
> > > > > Or do they not exist at all?
> > > > >
> > > > > The two are very different!
> > > 
> > > > These clocks etc exist but are not used by the driver.
> > > 
> > > That's not an acceptable reason for removing them from the binding. If they
> > > exist, they should be documented, regardless of whether the driver makes use
> > > of them. NAK.
> > 
> > If so, how to avoid the warning of dtbs_check.
> 
> By also adding the clocks, resets and interrupts to the dts.

Going through patchwork stuff now that the merge window is done. I'm
gonna mark the dts patch as changes requeted. The binding (and dts)
should describe all of the clocks the hardware has, whether or not you
choose to use them all in software does not matter. Can you please
resend the dts patch, with all of the clocks, resets and interrupts
present?

Thanks,
Conor.
Changhuang Liang March 28, 2024, 1:06 a.m. UTC | #7 
Hi, Conor,

> On Mon, Jan 22, 2024 at 08:24:51AM +0000, Conor Dooley wrote:
[...]
> > > > > > Do these clocks etc exist but are not used by the driver?
> > > > > >
> > > > > > Or do they not exist at all?
> > > > > >
> > > > > > The two are very different!
> > > >
> > > > > These clocks etc exist but are not used by the driver.
> > > >
> > > > That's not an acceptable reason for removing them from the
> > > > binding. If they exist, they should be documented, regardless of
> > > > whether the driver makes use of them. NAK.
> > >
> > > If so, how to avoid the warning of dtbs_check.
> >
> > By also adding the clocks, resets and interrupts to the dts.
> 
> Going through patchwork stuff now that the merge window is done. I'm gonna
> mark the dts patch as changes requeted. The binding (and dts) should
> describe all of the clocks the hardware has, whether or not you choose to use
> them all in software does not matter. Can you please resend the dts patch,
> with all of the clocks, resets and interrupts present?
> 

You have applied the dts patch.
https://lore.kernel.org/all/20240219032741.18387-1-changhuang.liang@starfivetech.com/

Regards,
Changhuang
Conor Dooley March 28, 2024, 6:28 a.m. UTC | #8 
On 28 March 2024 01:06:42 GMT, Changhuang Liang <changhuang.liang@starfivetech.com> wrote:
>Hi, Conor,
>
>> On Mon, Jan 22, 2024 at 08:24:51AM +0000, Conor Dooley wrote:
>[...]
>> > > > > > Do these clocks etc exist but are not used by the driver?
>> > > > > >
>> > > > > > Or do they not exist at all?
>> > > > > >
>> > > > > > The two are very different!
>> > > >
>> > > > > These clocks etc exist but are not used by the driver.
>> > > >
>> > > > That's not an acceptable reason for removing them from the
>> > > > binding. If they exist, they should be documented, regardless of
>> > > > whether the driver makes use of them. NAK.
>> > >
>> > > If so, how to avoid the warning of dtbs_check.
>> >
>> > By also adding the clocks, resets and interrupts to the dts.
>> 
>> Going through patchwork stuff now that the merge window is done. I'm gonna
>> mark the dts patch as changes requeted. The binding (and dts) should
>> describe all of the clocks the hardware has, whether or not you choose to use
>> them all in software does not matter. Can you please resend the dts patch,
>> with all of the clocks, resets and interrupts present?
>> 
>
>You have applied the dts patch.
>https://lore.kernel.org/all/20240219032741.18387-1-changhuang.liang@starfivetech.com/

Oh, sorry for the noise then. I guess I was
mislead by patchwork marking this as new :)
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/media/starfive,jh7110-camss.yaml b/Documentation/devicetree/bindings/media/starfive,jh7110-camss.yaml
index c66586d90fa2..2504381058b3 100644
--- a/Documentation/devicetree/bindings/media/starfive,jh7110-camss.yaml
+++ b/Documentation/devicetree/bindings/media/starfive,jh7110-camss.yaml
@@ -4,14 +4,14 @@ 
 $id: http://devicetree.org/schemas/media/starfive,jh7110-camss.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: Starfive SoC CAMSS ISP
+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
+  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 unit)
   and an ISP.
 
@@ -28,26 +28,21 @@  properties:
       - const: isp
 
   clocks:
-    maxItems: 7
+    maxItems: 3
 
   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
+    maxItems: 5
 
   reset-names:
     items:
       - const: wrapper_p
       - const: wrapper_c
-      - const: axird
       - const: axiwr
       - const: isp_top_n
       - const: isp_top_axi
@@ -57,7 +52,7 @@  properties:
       - description: JH7110 ISP Power Domain Switch Controller.
 
   interrupts:
-    maxItems: 4
+    maxItems: 3
 
   ports:
     $ref: /schemas/graph.yaml#/properties/ports
@@ -125,34 +120,24 @@  examples:
         reg = <0x19840000 0x10000>,
               <0x19870000 0x30000>;
         reg-names = "syscon", "isp";
-        clocks = <&ispcrg 0>,
-                 <&ispcrg 13>,
-                 <&ispcrg 2>,
-                 <&ispcrg 12>,
-                 <&ispcrg 1>,
+        clocks = <&ispcrg 13>,
                  <&syscrg 51>,
                  <&syscrg 52>;
-        clock-names = "apb_func",
-                      "wrapper_clk_c",
-                      "dvp_inv",
-                      "axiwr",
-                      "mipi_rx0_pxl",
+        clock-names = "wrapper_clk_c",
                       "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>;
+        interrupts = <92>, <87>, <90>;
 
         ports {
             #address-cells = <1>;