| Message ID | 20220314232214.4183078-2-swboyd@chromium.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/2] dt-bindings: mfd: Add ChromeOS fingerprint binding | expand |
On 15/03/2022 00:22, Stephen Boyd wrote: > Add a binding to describe the fingerprint processor found on Chromeboks > with a fingerprint sensor. > > Cc: Rob Herring <robh+dt@kernel.org> > Cc: <devicetree@vger.kernel.org> > 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> > Signed-off-by: Stephen Boyd <swboyd@chromium.org> > --- > .../bindings/mfd/google,cros-ec-fp.yaml | 89 +++++++++++++++++++ > 1 file changed, 89 insertions(+) > create mode 100644 Documentation/devicetree/bindings/mfd/google,cros-ec-fp.yaml > > diff --git a/Documentation/devicetree/bindings/mfd/google,cros-ec-fp.yaml b/Documentation/devicetree/bindings/mfd/google,cros-ec-fp.yaml > new file mode 100644 > index 000000000000..05d2b2b9b713 > --- /dev/null > +++ b/Documentation/devicetree/bindings/mfd/google,cros-ec-fp.yaml > @@ -0,0 +1,89 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/mfd/google,cros-ec-fp.yaml# Why is this in the MFD directory? Is it really a Multi Function Device? Description is rather opposite. You also did not CC MFD maintainer. Best regards, Krzysztof
On Tue, 15 Mar 2022, Krzysztof Kozlowski wrote: > On 15/03/2022 00:22, Stephen Boyd wrote: > > Add a binding to describe the fingerprint processor found on Chromeboks > > with a fingerprint sensor. > > > > Cc: Rob Herring <robh+dt@kernel.org> > > Cc: <devicetree@vger.kernel.org> > > 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> > > Signed-off-by: Stephen Boyd <swboyd@chromium.org> > > --- > > .../bindings/mfd/google,cros-ec-fp.yaml | 89 +++++++++++++++++++ > > 1 file changed, 89 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/mfd/google,cros-ec-fp.yaml > > > > diff --git a/Documentation/devicetree/bindings/mfd/google,cros-ec-fp.yaml b/Documentation/devicetree/bindings/mfd/google,cros-ec-fp.yaml > > new file mode 100644 > > index 000000000000..05d2b2b9b713 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/mfd/google,cros-ec-fp.yaml > > @@ -0,0 +1,89 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/mfd/google,cros-ec-fp.yaml# > > Why is this in the MFD directory? Is it really a Multi Function Device? > Description is rather opposite. You also did not CC MFD maintainer. A lot of the ChromeOS Embedded Controller support used to be located in MFD. There are still remnants, but most was moved to drivers/platform IIRC. Please see: drivers/mfd/cros_ec_dev.c However, yes, I should have been on the recipients list.
Quoting Lee Jones (2022-03-15 04:30:49) > On Tue, 15 Mar 2022, Krzysztof Kozlowski wrote: > > > On 15/03/2022 12:10, Lee Jones wrote: > > > On Tue, 15 Mar 2022, Krzysztof Kozlowski wrote: > > > > > >> On 15/03/2022 00:22, Stephen Boyd wrote: > > >>> Add a binding to describe the fingerprint processor found on Chromeboks > > >>> with a fingerprint sensor. > > >>> > > >>> Cc: Rob Herring <robh+dt@kernel.org> > > >>> Cc: <devicetree@vger.kernel.org> > > >>> 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> > > >>> Signed-off-by: Stephen Boyd <swboyd@chromium.org> > > >>> --- > > >>> .../bindings/mfd/google,cros-ec-fp.yaml | 89 +++++++++++++++++++ > > >>> 1 file changed, 89 insertions(+) > > >>> create mode 100644 Documentation/devicetree/bindings/mfd/google,cros-ec-fp.yaml > > >>> > > >>> diff --git a/Documentation/devicetree/bindings/mfd/google,cros-ec-fp.yaml b/Documentation/devicetree/bindings/mfd/google,cros-ec-fp.yaml > > >>> new file mode 100644 > > >>> index 000000000000..05d2b2b9b713 > > >>> --- /dev/null > > >>> +++ b/Documentation/devicetree/bindings/mfd/google,cros-ec-fp.yaml > > >>> @@ -0,0 +1,89 @@ > > >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > >>> +%YAML 1.2 > > >>> +--- > > >>> +$id: http://devicetree.org/schemas/mfd/google,cros-ec-fp.yaml# > > >> > > >> Why is this in the MFD directory? Is it really a Multi Function Device? > > >> Description is rather opposite. You also did not CC MFD maintainer. > > > > > > A lot of the ChromeOS Embedded Controller support used to be located > > > in MFD. There are still remnants, but most was moved to > > > drivers/platform IIRC. > > > > > > Please see: drivers/mfd/cros_ec_dev.c > > > > Yes, I know, that part is a MFD. But why the fingerprint controller part > > is MFD? To me it is closer to input device. > > It's tough to say from what I was sent above. > > But yes, sounds like it. > > We do not want any device 'functionality' in MFD ideally. > I put it next to the existing cros-ec binding. The existing binding is there because of historical reasons as far as I know. Otherwise it didn't seem MFD related so I didn't Cc mfd maintainer/list. New file additions don't usually conflict with anything and this is in the bindings directory so the driver side maintainer would be picking up the binding.
Quoting Alexandru M Stan (2022-03-14 17:23:38) > On Mon, Mar 14, 2022 at 4:22 PM Stephen Boyd <swboyd@chromium.org> wrote: > > > > + compatible = "google,cros-ec-fp"; > > + reg = <0>; > > + interrupt-parent = <&gpio_controller>; > > + interrupts = <4 IRQ_TYPE_LEVEL_LOW>; > > + spi-max-frequency = <3000000>; > > + google,cros-ec-spi-msg-delay = <37>; > > + google,cros-ec-spi-pre-delay = <5>; > > + reset-gpios = <&gpio_controller 5 GPIO_ACTIVE_LOW>; > > + boot0-gpios = <&gpio_controller 10 GPIO_ACTIVE_LOW>; > This should say GPIO_ACTIVE_HIGH, since there's no inverting going on > either with a real inverter, or the convention (of 'N' being in the > pin name). > > It might be easier to reason about if there's no invesion going for this signal. > > Consider it like an enum instead of a verb (unlike active_low > reset-gpios which can be considered: in reset if it's set): > > enum boot0 { > normal = 0, > bootloader = 1, > }; Ok got it! I have in my notes that physically high line means normal boot mode and physically low is bootloader mode. I confused myself. I'll fix this.
On Tue, 15 Mar 2022, Stephen Boyd wrote: > Quoting Lee Jones (2022-03-15 08:48:08) > > On Tue, 15 Mar 2022, Stephen Boyd wrote: > > > > > Quoting Lee Jones (2022-03-15 04:30:49) > > > > It's tough to say from what I was sent above. > > > > > > > > But yes, sounds like it. > > > > > > > > We do not want any device 'functionality' in MFD ideally. > > > > > > > > > > I put it next to the existing cros-ec binding. The existing binding is > > > there because of historical reasons as far as I know. Otherwise it > > > didn't seem MFD related so I didn't Cc mfd maintainer/list. New file > > > additions don't usually conflict with anything and this is in the > > > bindings directory so the driver side maintainer would be picking up the > > > binding. > > > > That's not how it works unfortunately. > > > > This file is located in the MFD bindings directory, so I would be > > picking it up (if it ends up staying here). > > The way it works is arbitrary Correct. > and up to maintainer's choice. Which *should* be reflected in MAINTAINERS and by extension get_maintainer.pl. As is the case here. :) > I'll move it out of the mfd directory :) That does sound like a good solution, thanks Stephen.
diff --git a/Documentation/devicetree/bindings/mfd/google,cros-ec-fp.yaml b/Documentation/devicetree/bindings/mfd/google,cros-ec-fp.yaml new file mode 100644 index 000000000000..05d2b2b9b713 --- /dev/null +++ b/Documentation/devicetree/bindings/mfd/google,cros-ec-fp.yaml @@ -0,0 +1,89 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/mfd/google,cros-ec-fp.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: ChromeOS Embedded Fingerprint Controller + +description: + Google's ChromeOS embedded fingerprint controller is a device which + implements fingerprint functionality such as unlocking a Chromebook + without typing a password. + +maintainers: + - Tom Hughes <tomhughes@chromium.org> + +properties: + compatible: + const: google,cros-ec-fp + + reg: + maxItems: 1 + + spi-max-frequency: + maximum: 3000000 + + interrupts: + maxItems: 1 + + reset-gpios: + maxItems: 1 + description: reset signal (active low). + + boot0-gpios: + maxItems: 1 + description: boot signal (low for normal boot; high for bootloader). + + vdd-supply: + description: Power supply for the fingerprint controller. + + google,cros-ec-spi-pre-delay: + description: + This property specifies the delay in usecs between the + assertion of the CS and the first clock pulse. + allOf: + - $ref: /schemas/types.yaml#/definitions/uint32 + - default: 0 + - minimum: 0 + + google,cros-ec-spi-msg-delay: + description: + This property specifies the delay in usecs between messages. + allOf: + - $ref: /schemas/types.yaml#/definitions/uint32 + - default: 0 + - minimum: 0 + +required: + - compatible + - reg + - interrupts + - reset-gpios + - boot0-gpios + - vdd-supply + - spi-max-frequency + +additionalProperties: false + +examples: + - | + #include <dt-bindings/interrupt-controller/irq.h> + #include <dt-bindings/gpio/gpio.h> + spi { + #address-cells = <0x1>; + #size-cells = <0x0>; + ec@0 { + compatible = "google,cros-ec-fp"; + reg = <0>; + interrupt-parent = <&gpio_controller>; + interrupts = <4 IRQ_TYPE_LEVEL_LOW>; + spi-max-frequency = <3000000>; + google,cros-ec-spi-msg-delay = <37>; + google,cros-ec-spi-pre-delay = <5>; + reset-gpios = <&gpio_controller 5 GPIO_ACTIVE_LOW>; + boot0-gpios = <&gpio_controller 10 GPIO_ACTIVE_LOW>; + vdd-supply = <&pp3300_fp_mcu>; + }; + }; +...
Add a binding to describe the fingerprint processor found on Chromeboks with a fingerprint sensor. Cc: Rob Herring <robh+dt@kernel.org> Cc: <devicetree@vger.kernel.org> 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> Signed-off-by: Stephen Boyd <swboyd@chromium.org> --- .../bindings/mfd/google,cros-ec-fp.yaml | 89 +++++++++++++++++++ 1 file changed, 89 insertions(+) create mode 100644 Documentation/devicetree/bindings/mfd/google,cros-ec-fp.yaml