| Message ID | 20210528113346.37137-4-sebastian.reichel@collabora.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | GE Healthcare PPD firmware upgrade driver for ACHC | expand |
On Fri, 28 May 2021 13:33:45 +0200, Sebastian Reichel wrote: > Convert the binding to DT schema format. Also update the binding > to fix shortcomings > > * Add "nxp,kinetis-k20" fallback compatible > * add programming SPI interface and reset GPIO > * add main clock > * add voltage supplies > * drop spi-max-frequency from required properties, > driver will setup max. frequency > > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com> > --- > .../devicetree/bindings/misc/ge-achc.txt | 26 ------- > .../devicetree/bindings/misc/ge-achc.yaml | 67 +++++++++++++++++++ > 2 files changed, 67 insertions(+), 26 deletions(-) > delete mode 100644 Documentation/devicetree/bindings/misc/ge-achc.txt > create mode 100644 Documentation/devicetree/bindings/misc/ge-achc.yaml > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/misc/ge-achc.yaml: properties:reg:minItems: False schema does not allow 2 hint: "minItems/maxItems" equal to the "items" list length are not necessary from schema $id: http://devicetree.org/meta-schemas/items.yaml# /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/misc/ge-achc.yaml: properties:reg:maxItems: False schema does not allow 2 hint: "minItems/maxItems" equal to the "items" list length are not necessary from schema $id: http://devicetree.org/meta-schemas/items.yaml# /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/misc/ge-achc.yaml: ignoring, error in schema: properties: reg: minItems warning: no schema found in file: ./Documentation/devicetree/bindings/misc/ge-achc.yaml Documentation/devicetree/bindings/misc/ge-achc.example.dt.yaml:0:0: /example-0/spi/spi@1: failed to match any schema with compatible: ['ge,achc', 'nxp,kinetis-k20'] Documentation/devicetree/bindings/misc/ge-achc.example.dt.yaml:0:0: /example-0/spi/spi@1: failed to match any schema with compatible: ['ge,achc', 'nxp,kinetis-k20'] See https://patchwork.ozlabs.org/patch/1485219 This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit.
On Fri, May 28, 2021 at 01:33:45PM +0200, Sebastian Reichel wrote: > -Required SPI properties: > - > -- reg : Should be address of the device chip select within > - the controller. There is an existing binding... > + reg: > + minItems: 2 > + maxItems: 2 > + items: > + - description: Control interface > + - description: Firmware programming interface ...but this new one is incompatible with it.
Hi, On Wed, Jun 09, 2021 at 12:47:47PM +0100, Mark Brown wrote: > On Fri, May 28, 2021 at 01:33:45PM +0200, Sebastian Reichel wrote: > > > -Required SPI properties: > > - > > -- reg : Should be address of the device chip select within > > - the controller. > > There is an existing binding... > > > + reg: > > + minItems: 2 > > + maxItems: 2 > > + items: > > + - description: Control interface > > + - description: Firmware programming interface > > ...but this new one is incompatible with it. The current binding is broken, since it uses the same compatible value for two different SPI "devices" (main SPI interface and EzPort). It only "works" because compatible string is only used to export the device to userspace via spidev and otherwise ignored. But the main spidev is not used by current FW and FW update cannot be done from userspace because it requires reset pin handling during spi transactions. So basically upstream support for this binding is completley broken anyways (downstream has additional patches to work around the limitations). In PATCHv2 it was pointed out, that there should be one device for both chip selects, since the reset/clocks/... are shared. -- Sebastian
diff --git a/Documentation/devicetree/bindings/misc/ge-achc.txt b/Documentation/devicetree/bindings/misc/ge-achc.txt deleted file mode 100644 index 77df94d7a32f..000000000000 --- a/Documentation/devicetree/bindings/misc/ge-achc.txt +++ /dev/null @@ -1,26 +0,0 @@ -* GE Healthcare USB Management Controller - -A device which handles data aquisition from compatible USB based peripherals. -SPI is used for device management. - -Note: This device does not expose the peripherals as USB devices. - -Required properties: - -- compatible : Should be "ge,achc" - -Required SPI properties: - -- reg : Should be address of the device chip select within - the controller. - -- spi-max-frequency : Maximum SPI clocking speed of device in Hz, should be - 1MHz for the GE ACHC. - -Example: - -spidev0: spi@0 { - compatible = "ge,achc"; - reg = <0>; - spi-max-frequency = <1000000>; -}; diff --git a/Documentation/devicetree/bindings/misc/ge-achc.yaml b/Documentation/devicetree/bindings/misc/ge-achc.yaml new file mode 100644 index 000000000000..a9c36fa7cf20 --- /dev/null +++ b/Documentation/devicetree/bindings/misc/ge-achc.yaml @@ -0,0 +1,67 @@ +# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause +# Copyright (C) 2021 GE Inc. +# Copyright (C) 2021 Collabora Ltd. +%YAML 1.2 +--- +$id: "http://devicetree.org/schemas/misc/ge-achc.yaml#" +$schema: "http://devicetree.org/meta-schemas/core.yaml#" + +title: GE Healthcare USB Management Controller + +description: | + A device which handles data acquisition from compatible USB based peripherals. + SPI is used for device management. + + Note: This device does not expose the peripherals as USB devices. + +maintainers: + - Sebastian Reichel <sre@kernel.org> + +properties: + compatible: + items: + - const: ge,achc + - const: nxp,kinetis-k20 + + clocks: + maxItems: 1 + + vdd-supply: + description: Digital power supply regulator on VDD pin + + vdda-supply: + description: Analog power supply regulator on VDDA pin + + reg: + minItems: 2 + maxItems: 2 + items: + - description: Control interface + - description: Firmware programming interface + + reset-gpios: + description: GPIO used for hardware reset. + maxItems: 1 + +required: + - compatible + - clocks + - reg + - reset-gpios + +additionalProperties: false + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + spi { + #address-cells = <1>; + #size-cells = <0>; + + spi@1 { + compatible = "ge,achc", "nxp,kinetis-k20"; + reg = <1>, <0>; + clocks = <&achc_24M>; + reset-gpios = <&gpio3 6 GPIO_ACTIVE_LOW>; + }; + };
Convert the binding to DT schema format. Also update the binding to fix shortcomings * Add "nxp,kinetis-k20" fallback compatible * add programming SPI interface and reset GPIO * add main clock * add voltage supplies * drop spi-max-frequency from required properties, driver will setup max. frequency Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com> --- .../devicetree/bindings/misc/ge-achc.txt | 26 ------- .../devicetree/bindings/misc/ge-achc.yaml | 67 +++++++++++++++++++ 2 files changed, 67 insertions(+), 26 deletions(-) delete mode 100644 Documentation/devicetree/bindings/misc/ge-achc.txt create mode 100644 Documentation/devicetree/bindings/misc/ge-achc.yaml