Re: [PATCH 3/3] dt-bindings: mfd: google,cros-ec: add missing properties

From: Rob Herring <robh@kernel.org>
To: "Ricardo Cañuelo" <ricardo.canuelo@collabora.com>
Cc: Collabora Kernel ML <kernel@collabora.com>,
	Enric Balletbo i Serra <enric.balletbo@collabora.com>,
	Benson Leung <bleung@chromium.org>,
	Guenter Roeck <groeck@chromium.org>,
	Simon Glass <sjg@chromium.org>,
	Doug Anderson <dianders@chromium.org>,
	devicetree@vger.kernel.org
Subject: Re: [PATCH 3/3] dt-bindings: mfd: google,cros-ec: add missing properties
Date: Mon, 5 Oct 2020 10:37:09 -0500	[thread overview]
Message-ID: <CAL_JsqJ5E6LSis1LzgEGPN6aEktkFamRn19v0s-x_OZ+8yMTiA@mail.gmail.com> (raw)
In-Reply-To: <20201005071403.17450-4-ricardo.canuelo@collabora.com>

On Mon, Oct 5, 2020 at 2:14 AM Ricardo Cañuelo
<ricardo.canuelo@collabora.com> wrote:
>
> Add missing properties that are currently used in the examples of
> subnode bindings and in many DTs.
> This fixes all current dt_binding_check and dtbs_check warnings related
> to this binding.
>
> Signed-off-by: Ricardo Cañuelo <ricardo.canuelo@collabora.com>
> ---
>  .../bindings/mfd/google,cros-ec.yaml          | 40 +++++++++++++++++++
>  1 file changed, 40 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> index f49c0d5d31ad..c2dc05cdef9f 100644
> --- a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> +++ b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> @@ -59,18 +59,58 @@ properties:
>        whether this nvram is present or not.
>      type: boolean
>
> +  mtk,rpmsg-name:

This should have been mediatek,rpmsg-name, but I guess we're stuck with it.

> +    description:
> +      Must be defined if the cros-ec is a rpmsg device for a Mediatek
> +      ARM Cortex M4 Co-processor. Contains the name pf the rpmsg
> +      device. Used to match the subnode to the rpmsg device announced by
> +      the SCP.
> +    $ref: /schemas/types.yaml#/definitions/string
> +
>    spi-max-frequency:
>      description: Maximum SPI frequency of the device in Hz.
>
>    reg:
>      maxItems: 1
>
> +  '#address-cells':
> +    enum: [1, 2]
> +
> +  '#size-cells':
> +    enum: [0, 1]

This doesn't really make sense. Either there's a size or there isn't.

[...]

> +  "^regulator@[a-f0-9]+$":
> +  "^ec-codec@[a-f0-9]+$":

What does the number space represent and is it the same for each of
these? If not, then this is kind of broken. There's only 1 number
space at a given level.

Rob