| Message ID | 20220614195144.2794796-2-swboyd@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | dt-bindings: cros-ec: Update for fingerprint devices | expand |
Hi, On Tue, Jun 14, 2022 at 12:51 PM Stephen Boyd <swboyd@chromium.org> wrote: > > Various properties in the cros-ec binding only apply to different > compatible strings. For example, the interrupts and reg property are > required for all cros-ec devices except for the rpmsg version. Add some > conditions to update the availability of properties so that they can't > be used with compatibles that don't support them. > > Cc: Rob Herring <robh@kernel.org> > Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org> > Cc: <devicetree@vger.kernel.org> > Cc: <chrome-platform@lists.linux.dev> > Cc: Guenter Roeck <groeck@chromium.org> > Cc: Douglas Anderson <dianders@chromium.org> > Cc: Craig Hesling <hesling@chromium.org> > Cc: Tom Hughes <tomhughes@chromium.org> > Cc: Alexandru M Stan <amstan@chromium.org> > Cc: Tzung-Bi Shih <tzungbi@kernel.org> > Cc: Matthias Kaehlcke <mka@chromium.org> > Cc: Benson Leung <bleung@chromium.org> > Cc: Lee Jones <lee.jones@linaro.org> > Signed-off-by: Stephen Boyd <swboyd@chromium.org> > --- > .../bindings/chrome/google,cros-ec-typec.yaml | 1 + > .../bindings/extcon/extcon-usbc-cros-ec.yaml | 1 + > .../i2c/google,cros-ec-i2c-tunnel.yaml | 1 + > .../bindings/mfd/google,cros-ec.yaml | 29 +++++++++++++------ > .../bindings/pwm/google,cros-ec-pwm.yaml | 1 + > .../regulator/google,cros-ec-regulator.yaml | 1 + > .../bindings/sound/google,cros-ec-codec.yaml | 1 + > 7 files changed, 26 insertions(+), 9 deletions(-) slight nit that from reading the subject of this patch I'd expect that it was a no-op. Just a reorganization / cleanup. In fact, it actually is more than a no-op. It enforces restrictions that should probably have always been enforced. I think it'd be better if the subject was something like "tighten property requirements" or something like that. > @@ -158,12 +154,27 @@ allOf: > - google,cros-ec-rpmsg > then: > properties: > + controller-data: false > google,cros-ec-spi-pre-delay: false > google,cros-ec-spi-msg-delay: false > spi-max-frequency: false > else: > $ref: /schemas/spi/spi-peripheral-props.yaml > > + - if: > + properties: > + compatible: > + not: > + contains: > + const: google,cros-ec-rpmsg > + then: > + properties: > + mediatek,rpmsg-name: false > + > + required: > + - reg > + - interrupts slight nit that think it would be easier to understand this bottom section if you made the "SPI" and "RPMSG" sections more symmetric to each other. I think it would be easy to just change the SPI one to say "not SPI" instead of explicitly listing "i2c" and "rpmsg". In any case, this overall looks pretty nice to me. My two requests are both pretty small nits, so either with or without fixing them: Reviewed-by: Douglas Anderson <dianders@chromium.org>
Quoting Doug Anderson (2022-06-14 15:41:38) > > slight nit that from reading the subject of this patch I'd expect that > it was a no-op. Just a reorganization / cleanup. In fact, it actually > is more than a no-op. It enforces restrictions that should probably > have always been enforced. I think it'd be better if the subject was > something like "tighten property requirements" or something like that. Sure. It sort of got out of control but I didn't update the commit text to explain that we're enforcing reg and interrupts for i2c/spi devices. > > slight nit that think it would be easier to understand this bottom > section if you made the "SPI" and "RPMSG" sections more symmetric to > each other. I think it would be easy to just change the SPI one to say > "not SPI" instead of explicitly listing "i2c" and "rpmsg". I had done that earlier but now it has an 'else' condition after commit f412fe11c1a9 ("mfd: dt-bindings: google,cros-ec: Reference Samsung SPI bindings"), so this kept the diff smaller. > > In any case, this overall looks pretty nice to me. My two requests are > both pretty small nits, so either with or without fixing them: > > Reviewed-by: Douglas Anderson <dianders@chromium.org> But if it gets a reviewed-by tag with more diff then I'll do it ;-)
On Tue, 14 Jun 2022 12:51:43 -0700, Stephen Boyd wrote: > Various properties in the cros-ec binding only apply to different > compatible strings. For example, the interrupts and reg property are > required for all cros-ec devices except for the rpmsg version. Add some > conditions to update the availability of properties so that they can't > be used with compatibles that don't support them. > > Cc: Rob Herring <robh@kernel.org> > Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org> > Cc: <devicetree@vger.kernel.org> > Cc: <chrome-platform@lists.linux.dev> > Cc: Guenter Roeck <groeck@chromium.org> > Cc: Douglas Anderson <dianders@chromium.org> > Cc: Craig Hesling <hesling@chromium.org> > Cc: Tom Hughes <tomhughes@chromium.org> > Cc: Alexandru M Stan <amstan@chromium.org> > Cc: Tzung-Bi Shih <tzungbi@kernel.org> > Cc: Matthias Kaehlcke <mka@chromium.org> > Cc: Benson Leung <bleung@chromium.org> > Cc: Lee Jones <lee.jones@linaro.org> > Signed-off-by: Stephen Boyd <swboyd@chromium.org> > --- > .../bindings/chrome/google,cros-ec-typec.yaml | 1 + > .../bindings/extcon/extcon-usbc-cros-ec.yaml | 1 + > .../i2c/google,cros-ec-i2c-tunnel.yaml | 1 + > .../bindings/mfd/google,cros-ec.yaml | 29 +++++++++++++------ > .../bindings/pwm/google,cros-ec-pwm.yaml | 1 + > .../regulator/google,cros-ec-regulator.yaml | 1 + > .../bindings/sound/google,cros-ec-codec.yaml | 1 + > 7 files changed, 26 insertions(+), 9 deletions(-) > Reviewed-by: Rob Herring <robh@kernel.org>
diff --git a/Documentation/devicetree/bindings/chrome/google,cros-ec-typec.yaml b/Documentation/devicetree/bindings/chrome/google,cros-ec-typec.yaml index 2d98f7c4d3bc..adde8c44372b 100644 --- a/Documentation/devicetree/bindings/chrome/google,cros-ec-typec.yaml +++ b/Documentation/devicetree/bindings/chrome/google,cros-ec-typec.yaml @@ -37,6 +37,7 @@ examples: cros_ec: ec@0 { compatible = "google,cros-ec-spi"; reg = <0>; + interrupts = <35 0>; typec { compatible = "google,cros-ec-typec"; diff --git a/Documentation/devicetree/bindings/extcon/extcon-usbc-cros-ec.yaml b/Documentation/devicetree/bindings/extcon/extcon-usbc-cros-ec.yaml index 2d82b44268db..2e5b39881449 100644 --- a/Documentation/devicetree/bindings/extcon/extcon-usbc-cros-ec.yaml +++ b/Documentation/devicetree/bindings/extcon/extcon-usbc-cros-ec.yaml @@ -40,6 +40,7 @@ examples: cros-ec@0 { compatible = "google,cros-ec-spi"; reg = <0>; + interrupts = <44 0>; usbc_extcon0: extcon0 { compatible = "google,extcon-usbc-cros-ec"; diff --git a/Documentation/devicetree/bindings/i2c/google,cros-ec-i2c-tunnel.yaml b/Documentation/devicetree/bindings/i2c/google,cros-ec-i2c-tunnel.yaml index 6e1c70e9275e..cf523615f5e3 100644 --- a/Documentation/devicetree/bindings/i2c/google,cros-ec-i2c-tunnel.yaml +++ b/Documentation/devicetree/bindings/i2c/google,cros-ec-i2c-tunnel.yaml @@ -47,6 +47,7 @@ examples: compatible = "google,cros-ec-spi"; reg = <0>; spi-max-frequency = <5000000>; + interrupts = <99 0>; i2c-tunnel { compatible = "google,cros-ec-i2c-tunnel"; diff --git a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml index e25caf8ef9f4..178e26ab115c 100644 --- a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml +++ b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml @@ -20,19 +20,16 @@ properties: compatible: oneOf: - description: - For implementations of the EC is connected through I2C. + For implementations of the EC connected through I2C. const: google,cros-ec-i2c - description: - For implementations of the EC is connected through SPI. + For implementations of the EC connected through SPI. const: google,cros-ec-spi - description: - For implementations of the EC is connected through RPMSG. + For implementations of the EC connected through RPMSG. const: google,cros-ec-rpmsg - controller-data: - description: - SPI controller data, see bindings/spi/samsung,spi-peripheral-props.yaml - type: object + controller-data: true google,cros-ec-spi-pre-delay: description: @@ -62,8 +59,7 @@ properties: the SCP. $ref: "/schemas/types.yaml#/definitions/string" - spi-max-frequency: - description: Maximum SPI frequency of the device in Hz. + spi-max-frequency: true reg: maxItems: 1 @@ -158,12 +154,27 @@ allOf: - google,cros-ec-rpmsg then: properties: + controller-data: false google,cros-ec-spi-pre-delay: false google,cros-ec-spi-msg-delay: false spi-max-frequency: false else: $ref: /schemas/spi/spi-peripheral-props.yaml + - if: + properties: + compatible: + not: + contains: + const: google,cros-ec-rpmsg + then: + properties: + mediatek,rpmsg-name: false + + required: + - reg + - interrupts + additionalProperties: false examples: diff --git a/Documentation/devicetree/bindings/pwm/google,cros-ec-pwm.yaml b/Documentation/devicetree/bindings/pwm/google,cros-ec-pwm.yaml index c8577bdf6c94..3afe1480df52 100644 --- a/Documentation/devicetree/bindings/pwm/google,cros-ec-pwm.yaml +++ b/Documentation/devicetree/bindings/pwm/google,cros-ec-pwm.yaml @@ -48,6 +48,7 @@ examples: cros-ec@0 { compatible = "google,cros-ec-spi"; reg = <0>; + interrupts = <101 0>; cros_ec_pwm: pwm { compatible = "google,cros-ec-pwm"; diff --git a/Documentation/devicetree/bindings/regulator/google,cros-ec-regulator.yaml b/Documentation/devicetree/bindings/regulator/google,cros-ec-regulator.yaml index 69e5402da761..0921f012c901 100644 --- a/Documentation/devicetree/bindings/regulator/google,cros-ec-regulator.yaml +++ b/Documentation/devicetree/bindings/regulator/google,cros-ec-regulator.yaml @@ -41,6 +41,7 @@ examples: reg = <0>; #address-cells = <1>; #size-cells = <0>; + interrupts = <99 0>; regulator@0 { compatible = "google,cros-ec-regulator"; diff --git a/Documentation/devicetree/bindings/sound/google,cros-ec-codec.yaml b/Documentation/devicetree/bindings/sound/google,cros-ec-codec.yaml index c3e9f3485449..67134e06765a 100644 --- a/Documentation/devicetree/bindings/sound/google,cros-ec-codec.yaml +++ b/Documentation/devicetree/bindings/sound/google,cros-ec-codec.yaml @@ -57,6 +57,7 @@ examples: cros-ec@0 { compatible = "google,cros-ec-spi"; reg = <0>; + interrupts = <93 0>; codecs { #address-cells = <2>;
Various properties in the cros-ec binding only apply to different compatible strings. For example, the interrupts and reg property are required for all cros-ec devices except for the rpmsg version. Add some conditions to update the availability of properties so that they can't be used with compatibles that don't support them. Cc: Rob Herring <robh@kernel.org> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org> Cc: <devicetree@vger.kernel.org> Cc: <chrome-platform@lists.linux.dev> Cc: Guenter Roeck <groeck@chromium.org> Cc: Douglas Anderson <dianders@chromium.org> Cc: Craig Hesling <hesling@chromium.org> Cc: Tom Hughes <tomhughes@chromium.org> Cc: Alexandru M Stan <amstan@chromium.org> Cc: Tzung-Bi Shih <tzungbi@kernel.org> Cc: Matthias Kaehlcke <mka@chromium.org> Cc: Benson Leung <bleung@chromium.org> Cc: Lee Jones <lee.jones@linaro.org> Signed-off-by: Stephen Boyd <swboyd@chromium.org> --- .../bindings/chrome/google,cros-ec-typec.yaml | 1 + .../bindings/extcon/extcon-usbc-cros-ec.yaml | 1 + .../i2c/google,cros-ec-i2c-tunnel.yaml | 1 + .../bindings/mfd/google,cros-ec.yaml | 29 +++++++++++++------ .../bindings/pwm/google,cros-ec-pwm.yaml | 1 + .../regulator/google,cros-ec-regulator.yaml | 1 + .../bindings/sound/google,cros-ec-codec.yaml | 1 + 7 files changed, 26 insertions(+), 9 deletions(-)