Re: [PATCH 1/7] dt-bindings: pinctrl: Add support for Broadcom STB pin controller

[Krzysztof Kozlowski <krzk@kernel.org>]

On 31/07/2024 08:28, Ivan T. Ivanov wrote:
> It looks like they are few revisions of this chip which varies
> by number of pins. Perhaps not all of them are available on the
> market. Perhaps some of them where early engineering samples,
> I don't know. I decided to keep all of them just in case.
> 
> Cc: Andrea della Porta <andrea.porta@suse.com>
> Signed-off-by: Ivan T. Ivanov <iivanov@suse.de>
> ---
>  .../pinctrl/brcm,brcmstb-pinctrl.yaml         | 73 +++++++++++++++++++
>  1 file changed, 73 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pinctrl/brcm,brcmstb-pinctrl.yaml
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/brcm,brcmstb-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/brcm,brcmstb-pinctrl.yaml
> new file mode 100644
> index 000000000000..c5afdb49d784
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/brcm,brcmstb-pinctrl.yaml

bcm2712 is Rpi, so not really STB. The name is confusing. Please use
compatible as filename, so:
brcm,bcm2712-pinctrl.yaml

> @@ -0,0 +1,73 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pinctrl/brcm,brcmstb-pinctrl.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Broadcom STB family pin controller
> +
> +maintainers:
> +  - Ivan T. Ivanov <iivanov@suse.de>
> +
> +description:
> +  Broadcom's STB family memory-mapped pin controller.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - brcm,bcm2712-pinctrl
> +      - brcm,bcm2712-aon-pinctrl
> +      - brcm,bcm2712c0-pinctrl
> +      - brcm,bcm2712c0-aon-pinctrl
> +      - brcm,bcm2712d0-pinctrl
> +      - brcm,bcm2712d0-aon-pinctrl
> +
> +  reg:
> +    maxItems: 1
> +
> +allOf:
> +  - $ref: pinctrl.yaml#

"allOf:" block goes after "required:".

> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties:
> +  anyOf:
> +    - type: object
> +      allOf:
> +        - $ref: pincfg-node.yaml#
> +        - $ref: pinmux-node.yaml#
> +
> +      properties:
> +        function:
> +          enum:
> +            [

Unnecessary new line

> +              gpio, alt1, alt2, alt3, alt4, alt5, alt6, alt7, alt8,
> +              aon_cpu_standbyb, aon_fp_4sec_resetb, aon_gpclk, aon_pwm,
> +              arm_jtag, aud_fs_clk0, avs_pmu_bsc, bsc_m0, bsc_m1, bsc_m2,
> +              bsc_m3, clk_observe, ctl_hdmi_5v, enet0, enet0_mii, enet0_rgmii,
> +              ext_sc_clk, fl0, fl1, gpclk0, gpclk1, gpclk2, hdmi_tx0_auto_i2c,
> +              hdmi_tx0_bsc, hdmi_tx1_auto_i2c, hdmi_tx1_bsc, i2s_in, i2s_out,
> +              ir_in, mtsif, mtsif_alt, mtsif_alt1, pdm, pkt, pm_led_out, sc0,
> +              sd0, sd2, sd_card_a, sd_card_b, sd_card_c, sd_card_d, sd_card_e,
> +              sd_card_f, sd_card_g, spdif_out, spi_m, spi_s, sr_edm_sense, te0,
> +              te1, tsio, uart0, uart1, uart2, usb_pwr, usb_vbus, uui, vc_i2c0,
> +              vc_i2c3, vc_i2c4, vc_i2c5, vc_i2csl, vc_pcm, vc_pwm0, vc_pwm1,
> +              vc_spi0, vc_spi3, vc_spi4, vc_spi5, vc_uart0, vc_uart2, vc_uart3,
> +              vc_uart4,
> +            ]
> +
> +        pins:
> +          items:
> +            pattern: "^((aon_)?s?gpio[0-6]?[0-9])|(emmc_(clk|cmd|dat[0-7]|ds))$"
> +
> +        bias-disable: true
> +        bias-pull-down: true
> +        bias-pull-up: true
> +      additionalProperties: false
> +
> +    - type: object
> +      additionalProperties:
> +        $ref: "#/additionalProperties/anyOf/0"

I suggest going with patternProperties, fixed suffix for node names and
$defs. See for example:
Documentation/devicetree/bindings/pinctrl/qcom,x1e80100-tlmm.yaml

Missing example. I don't see this being part of other complete device,
so example is a requirement.

Best regards,
Krzysztof