Re:Re: [PATCH v4 2/4] dt-bindings: mfd: sprd: Add bindings for ums512 global registers

[ChunyanZhang]

Hi Stephen,

Thanks for the review.

At 2021-10-14 06:04:32, "Stephen Boyd" <sboyd@xxxxxxxxxx> wrote:
>Quoting Chunyan Zhang (2021-09-22 23:41:35)
>> diff --git a/Documentation/devicetree/bindings/mfd/sprd,ums512-glbreg.yaml b/Documentation/devicetree/bindings/mfd/sprd,ums512-glbreg.yaml
>> new file mode 100644
>> index 000000000000..3522f3d2d8de
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mfd/sprd,ums512-glbreg.yaml
>> @@ -0,0 +1,68 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/mfd/sprd,ums512-glbreg.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Unisoc System Global Register Device Tree Bindings
>> +
>> +maintainers:
>> +  - Orson Zhai <orsonzhai@xxxxxxxxx>
>> +  - Baolin Wang <baolin.wang7@xxxxxxxxx>
>> +  - Chunyan Zhang <zhang.lyra@xxxxxxxxx>
>> +
>> +description:
>> +  Unisoc system global registers provide register map
>> +  for clocks and some multimedia modules of the SoC.
>> +
>> +properties:
>> +  "#address-cells": true
>> +  "#size-cells": true
>> +
>> +  compatible:
>> +    items:
>> +      - const: sprd,ums512-glbregs
>> +      - const: syscon
>> +      - const: simple-mfd
>> +
>> +  ranges:
>> +    maxItems: 1
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +patternProperties:
>> +  "^.*@[0-9a-f]+$":
>> +    # Child node
>> +    type: object
>> +    $ref: "../clock/sprd,ums512-clk.yaml"
>> +    description:
>> +      Clock controller for the SoC clocks.
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    ap_apb_regs: syscon@71000000 {
>> +      compatible = "sprd,ums512-glbregs", "syscon", "simple-mfd";
>> +      reg = <0x71000000 0x3000>;
>> +      #address-cells = <1>;
>> +      #size-cells = <1>;
>> +      ranges = <0 0x71000000 0x3000>;
>> +
>> +      clock-controller@0 {
>> +        compatible = "sprd,ums512-apahb-gate";
>
>Why is this a subnode of a syscon and simple-mfd? Why not put the>clock-controller@71000000 directly onto the bus? Does making it a child
>node help somehow?

These clocks are at the same register range with global registers. I originally put them directly onto the bus indeed when submitting the patches for SC9863A clocks last year, and it had a private property named 'sprd,syscon' which could provide regmap for these clocks.

Rob suggested [1] us to make them a child of the syscon, and would not need the private property 'sprd, syscon' then.

Thanks,
Chunyan

[1] https://lkml.org/lkml/2019/12/26/212

>
>> +        reg = <0x0 0x2000>;
>> +        #clock-cells = <1>;
>> +      };
>> +    };