RE: [PATCH v2 2/3] dt-bindings: mfd: Add Realtek MISC system controller

["Yu-Chun Lin [林祐君]" <eleanor.lin@realtek.com>]

Hi Conor and Krzysztof,

> Documentation/devicetree/bindings/mfd/realtek,misc.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mfd/realtek,misc.yaml
> b/Documentation/devicetree/bindings/mfd/realtek,misc.yaml
> new file mode 100644
> index 000000000000..4f4a9ae250be
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/realtek,misc.yaml
> @@ -0,0 +1,72 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/realtek,misc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Realtek MISC System Controller
> +
> +description:
> +  The Realtek MISC System Controller is a register area that contains
> +  miscellaneous system registers for the SoC and serves as a parent
> +node
> +  for other functions.
> +
> +maintainers:
> +  - James Tai <james.tai@realtek.com>
> +  - Yu-Chun Lin <eleanor.lin@realtek.com>
> +
> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - realtek,misc

I apologize for the current compatible string, which was initially named
by referencing existing patterns like 'brcm,misc' and thus violates the
naming guidance against "wildcards" and general non-SoC specific names.

Let me explain the purpose of the device node (Realtek system controller).

This MISC area contains several peripheral sub-modules such as uart,
watchdog, rtc or i2c ..... These blocks share a unified register region
implemented as a single hardware module, which remains powered during
system suspend states (e.g., S3). These blocks share the same MMIO region
and appear as child nodes under the MISC syscon node. Currently, it
includes uart.

Regarding the current structure, the device node is defined in a kent.dtsi
and is included by each SoC's DTSI.

I've considered two ways to write compatible string naming.

Option 1: Use a single SoC-specific compatible string

Rename "realtek,misc" to "realtek,rtd1861-misc"

/* kent.dtsi */
misc: syscon@... {
    compatible = "realtek,rtd1861-misc", "syscon", "simple-mfd";
};

Pros: Only one compatible string is needed, simplifying maintenance across
the driver and DTS.

Cons: Violates the "SoC-specific compatible" rule for other SoCs
(RTD1501, RTD1920).

Option 2: SoC-specific + fallback (Compliant but Verbose)

Define the full list in the schema, and override the compatible string in each SoC DTSI.

/* schema binding */

compatible:
  items:
    - enum:
        - realtek,rtd1501-misc
        - realtek,rtd1861-misc
        - realtek,rtd1920-misc
        # ... add new SoCs here
    - const: realtek,kent-misc
    - const: syscon
    - const: simple-mfd


/* kent.dtsi */

misc: syscon@... {
    compatible = "realtek,kent-misc", "syscon", "simple-mfd";
};

SoC-specific override (e.g. rtd1920s-smallville.dtsi):

&misc {
    compatible = "realtek,rtd1920-misc", "realtek,kent-misc", "syscon",
                 "simple-mfd";
};

Pros: Fully compliant with DT rules

Cons: Requires override in every SoC file; slight duplication.

Is Option 2 the expected pattern?
Thanks for your guidance!

Best regards,
Yu-Chun

> +      - const: syscon
> +      - const: simple-mfd
> +
> +  reg:
> +    maxItems: 1
> +
> +  ranges:
> +    maxItems: 1
> +
> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 1
> +
> +patternProperties:
> +  "^serial@[0-9a-f]+$":
> +    type: object
> +    description: UART controllers inside MISC area
> +
> +required:
> +  - compatible
> +  - reg
> +  - ranges
> +  - '#address-cells'
> +  - '#size-cells'
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    syscon@7000 {
> +        compatible = "realtek,misc", "syscon", "simple-mfd";
> +        reg = <0x7000 0x1000>;
> +        ranges = <0x0 0x7000 0x1000>;
> +        #address-cells = <1>;
> +        #size-cells = <1>;
> +
> +        uart0: serial@800 {
> +            compatible = "snps,dw-apb-uart";
> +            reg = <0x800 0x100>;
> +            clock-frequency = <432000000>;
> +            interrupts = <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>;
> +            reg-io-width = <4>;
> +            reg-shift = <2>;
> +            status = "disabled";
> +        };
> +    };
> --
> 2.34.1