diff mbox series

[v2,03/15] dt-bindings: devfreq: rk3399_dmc: Fix Hz units

Message ID 20220127150615.v2.3.I9341269171c114d0e04e41d48037fd32816e2d8c@changeid (mailing list archive)
State Superseded
Delegated to: Chanwoo Choi
Headers show
Series rk3399: Clean up and enable DDR DVFS | expand

Commit Message

Brian Norris Jan. 27, 2022, 11:07 p.m. UTC
The driver and all downstream device trees [1] are using Hz units, but
the document claims MHz. DRAM frequency for these systems can't possibly
exceed 2^32-1 Hz, so the choice of unit doesn't really matter than much.

Rather than add unnecessary risk in getting the units wrong, let's just
go with the unofficial convention and make the docs match reality.

A sub-1MHz frequency is extremely unlikely, so include a minimum in the
schema, to help catch anybody who might have believed this was MHz.

[1] And notably, also those trying to upstream them:
https://lore.kernel.org/lkml/20210308233858.24741-3-daniel.lezcano@linaro.org/

Signed-off-by: Brian Norris <briannorris@chromium.org>
---

(no changes since v1)

 .../bindings/devfreq/rk3399_dmc.yaml          | 24 +++++++++----------
 1 file changed, 12 insertions(+), 12 deletions(-)

Comments

Chanwoo Choi Feb. 4, 2022, 1:59 a.m. UTC | #1
On 1/28/22 8:07 AM, Brian Norris wrote:
> The driver and all downstream device trees [1] are using Hz units, but
> the document claims MHz. DRAM frequency for these systems can't possibly
> exceed 2^32-1 Hz, so the choice of unit doesn't really matter than much.
> 
> Rather than add unnecessary risk in getting the units wrong, let's just
> go with the unofficial convention and make the docs match reality.
> 
> A sub-1MHz frequency is extremely unlikely, so include a minimum in the
> schema, to help catch anybody who might have believed this was MHz.
> 
> [1] And notably, also those trying to upstream them:
> https://protect2.fireeye.com/v1/url?k=0a7de78e-55e6dec8-0a7c6cc1-0cc47a3003e8-4f0969a9fa7b496e&q=1&e=6129c5df-8bd2-4072-86ef-79b79b36ec89&u=https%3A%2F%2Flore.kernel.org%2Flkml%2F20210308233858.24741-3-daniel.lezcano%40linaro.org%2F
> 
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> ---
> 
> (no changes since v1)
> 
>  .../bindings/devfreq/rk3399_dmc.yaml          | 24 +++++++++----------
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/devfreq/rk3399_dmc.yaml b/Documentation/devicetree/bindings/devfreq/rk3399_dmc.yaml
> index fd62a8cd62d5..8bb778df92ae 100644
> --- a/Documentation/devicetree/bindings/devfreq/rk3399_dmc.yaml
> +++ b/Documentation/devicetree/bindings/devfreq/rk3399_dmc.yaml
> @@ -116,11 +116,11 @@ properties:
>  
>    rockchip,ddr3_odt_dis_freq:
>      $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 1000000  # In case anyone thought this was MHz.
>      description:
>        When the DRAM type is DDR3, this parameter defines the ODT disable
> -      frequency in MHz (Mega Hz). When the DDR frequency is less then
> -      ddr3_odt_dis_freq, the ODT on the DRAM side and controller side are both
> -      disabled.
> +      frequency in Hz. When the DDR frequency is less then ddr3_odt_dis_freq,
> +      the ODT on the DRAM side and controller side are both disabled.
>  
>    rockchip,ddr3_drv:
>      deprecated: true
> @@ -160,11 +160,11 @@ properties:
>  
>    rockchip,lpddr3_odt_dis_freq:
>      $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 1000000  # In case anyone thought this was MHz.
>      description:
>        When the DRAM type is LPDDR3, this parameter defines then ODT disable
> -      frequency in MHz (Mega Hz). When DDR frequency is less then
> -      ddr3_odt_dis_freq, the ODT on the DRAM side and controller side are both
> -      disabled.
> +      frequency in Hz. When DDR frequency is less then ddr3_odt_dis_freq, the
> +      ODT on the DRAM side and controller side are both disabled.
>  
>    rockchip,lpddr3_drv:
>      deprecated: true
> @@ -204,11 +204,11 @@ properties:
>  
>    rockchip,lpddr4_odt_dis_freq:
>      $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 1000000  # In case anyone thought this was MHz.
>      description:
>        When the DRAM type is LPDDR4, this parameter defines the ODT disable
> -      frequency in MHz (Mega Hz). When the DDR frequency is less then
> -      ddr3_odt_dis_freq, the ODT on the DRAM side and controller side are both
> -      disabled.
> +      frequency in Hz. When the DDR frequency is less then ddr3_odt_dis_freq,
> +      the ODT on the DRAM side and controller side are both disabled.
>  
>    rockchip,lpddr4_drv:
>      deprecated: true
> @@ -287,7 +287,7 @@ examples:
>        rockchip,sr_mc_gate_idle = <0x3>;
>        rockchip,srpd_lite_idle = <0x4>;
>        rockchip,standby_idle = <0x2000>;
> -      rockchip,ddr3_odt_dis_freq = <333>;
> -      rockchip,lpddr3_odt_dis_freq = <333>;
> -      rockchip,lpddr4_odt_dis_freq = <333>;
> +      rockchip,ddr3_odt_dis_freq = <333000000>;
> +      rockchip,lpddr3_odt_dis_freq = <333000000>;
> +      rockchip,lpddr4_odt_dis_freq = <333000000>;
>      };
> 

Acked-by: Chanwoo Choi <cw00.choi@samsung.com>
Rob Herring (Arm) Feb. 9, 2022, 8:19 p.m. UTC | #2
On Thu, Jan 27, 2022 at 03:07:14PM -0800, Brian Norris wrote:
> The driver and all downstream device trees [1] are using Hz units, but
> the document claims MHz. DRAM frequency for these systems can't possibly
> exceed 2^32-1 Hz, so the choice of unit doesn't really matter than much.
> 
> Rather than add unnecessary risk in getting the units wrong, let's just
> go with the unofficial convention and make the docs match reality.
> 
> A sub-1MHz frequency is extremely unlikely, so include a minimum in the
> schema, to help catch anybody who might have believed this was MHz.
> 
> [1] And notably, also those trying to upstream them:
> https://lore.kernel.org/lkml/20210308233858.24741-3-daniel.lezcano@linaro.org/
> 
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> ---
> 
> (no changes since v1)
> 
>  .../bindings/devfreq/rk3399_dmc.yaml          | 24 +++++++++----------
>  1 file changed, 12 insertions(+), 12 deletions(-)

Reviewed-by: Rob Herring <robh@kernel.org>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/devfreq/rk3399_dmc.yaml b/Documentation/devicetree/bindings/devfreq/rk3399_dmc.yaml
index fd62a8cd62d5..8bb778df92ae 100644
--- a/Documentation/devicetree/bindings/devfreq/rk3399_dmc.yaml
+++ b/Documentation/devicetree/bindings/devfreq/rk3399_dmc.yaml
@@ -116,11 +116,11 @@  properties:
 
   rockchip,ddr3_odt_dis_freq:
     $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 1000000  # In case anyone thought this was MHz.
     description:
       When the DRAM type is DDR3, this parameter defines the ODT disable
-      frequency in MHz (Mega Hz). When the DDR frequency is less then
-      ddr3_odt_dis_freq, the ODT on the DRAM side and controller side are both
-      disabled.
+      frequency in Hz. When the DDR frequency is less then ddr3_odt_dis_freq,
+      the ODT on the DRAM side and controller side are both disabled.
 
   rockchip,ddr3_drv:
     deprecated: true
@@ -160,11 +160,11 @@  properties:
 
   rockchip,lpddr3_odt_dis_freq:
     $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 1000000  # In case anyone thought this was MHz.
     description:
       When the DRAM type is LPDDR3, this parameter defines then ODT disable
-      frequency in MHz (Mega Hz). When DDR frequency is less then
-      ddr3_odt_dis_freq, the ODT on the DRAM side and controller side are both
-      disabled.
+      frequency in Hz. When DDR frequency is less then ddr3_odt_dis_freq, the
+      ODT on the DRAM side and controller side are both disabled.
 
   rockchip,lpddr3_drv:
     deprecated: true
@@ -204,11 +204,11 @@  properties:
 
   rockchip,lpddr4_odt_dis_freq:
     $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 1000000  # In case anyone thought this was MHz.
     description:
       When the DRAM type is LPDDR4, this parameter defines the ODT disable
-      frequency in MHz (Mega Hz). When the DDR frequency is less then
-      ddr3_odt_dis_freq, the ODT on the DRAM side and controller side are both
-      disabled.
+      frequency in Hz. When the DDR frequency is less then ddr3_odt_dis_freq,
+      the ODT on the DRAM side and controller side are both disabled.
 
   rockchip,lpddr4_drv:
     deprecated: true
@@ -287,7 +287,7 @@  examples:
       rockchip,sr_mc_gate_idle = <0x3>;
       rockchip,srpd_lite_idle = <0x4>;
       rockchip,standby_idle = <0x2000>;
-      rockchip,ddr3_odt_dis_freq = <333>;
-      rockchip,lpddr3_odt_dis_freq = <333>;
-      rockchip,lpddr4_odt_dis_freq = <333>;
+      rockchip,ddr3_odt_dis_freq = <333000000>;
+      rockchip,lpddr3_odt_dis_freq = <333000000>;
+      rockchip,lpddr4_odt_dis_freq = <333000000>;
     };