* [PATCH 0/2] Update cros-ec-spi for fingerprint devices @ 2022-03-14 23:22 Stephen Boyd 2022-03-14 23:22 ` [PATCH 1/2] dt-bindings: mfd: Add ChromeOS fingerprint binding Stephen Boyd 2022-03-14 23:22 ` [PATCH 2/2] platform/chrome: cros_ec_spi: Boot fingerprint processor during probe Stephen Boyd 0 siblings, 2 replies; 17+ messages in thread From: Stephen Boyd @ 2022-03-14 23:22 UTC (permalink / raw) To: Benson Leung Cc: linux-kernel, chrome-platform, Rob Herring, devicetree, Guenter Roeck, Douglas Anderson, Craig Hesling, Tom Hughes, Alexandru M Stan This patch series introduces a DT binding for chromeos fingerprint devices and then implements support to boot those processors during driver probe if the BIOS hasn't done it already. Stephen Boyd (2): dt-bindings: mfd: Add ChromeOS fingerprint binding platform/chrome: cros_ec_spi: Boot fingerprint processor during probe .../bindings/mfd/google,cros-ec-fp.yaml | 89 +++++++++++++++++++ drivers/platform/chrome/cros_ec_spi.c | 38 +++++++- 2 files changed, 124 insertions(+), 3 deletions(-) create mode 100644 Documentation/devicetree/bindings/mfd/google,cros-ec-fp.yaml 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> base-commit: ffb217a13a2eaf6d5bd974fc83036a53ca69f1e2 -- https://chromeos.dev ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/2] dt-bindings: mfd: Add ChromeOS fingerprint binding 2022-03-14 23:22 [PATCH 0/2] Update cros-ec-spi for fingerprint devices Stephen Boyd @ 2022-03-14 23:22 ` Stephen Boyd 2022-03-15 0:23 ` Alexandru M Stan ` (2 more replies) 2022-03-14 23:22 ` [PATCH 2/2] platform/chrome: cros_ec_spi: Boot fingerprint processor during probe Stephen Boyd 1 sibling, 3 replies; 17+ messages in thread From: Stephen Boyd @ 2022-03-14 23:22 UTC (permalink / raw) To: Benson Leung Cc: linux-kernel, chrome-platform, Rob Herring, devicetree, Guenter Roeck, Douglas Anderson, Craig Hesling, Tom Hughes, Alexandru M Stan 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# +$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>; + }; + }; +... -- https://chromeos.dev ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] dt-bindings: mfd: Add ChromeOS fingerprint binding 2022-03-14 23:22 ` [PATCH 1/2] dt-bindings: mfd: Add ChromeOS fingerprint binding Stephen Boyd @ 2022-03-15 0:23 ` Alexandru M Stan 2022-03-15 15:50 ` Stephen Boyd 2022-03-15 3:08 ` Tzung-Bi Shih 2022-03-15 10:41 ` Krzysztof Kozlowski 2 siblings, 1 reply; 17+ messages in thread From: Alexandru M Stan @ 2022-03-15 0:23 UTC (permalink / raw) To: Stephen Boyd Cc: Benson Leung, linux-kernel, chrome-platform, Rob Herring, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Guenter Roeck, Douglas Anderson, Craig Hesling, Tom Hughes On Mon, Mar 14, 2022 at 4:22 PM Stephen Boyd <swboyd@chromium.org> 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# > +$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). Maybe add "active high, same polarity as the fpmcu sees physically". > + 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>; 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, }; > + vdd-supply = <&pp3300_fp_mcu>; > + }; > + }; > +... > -- > https://chromeos.dev > -- Alexandru Stan (amstan) ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] dt-bindings: mfd: Add ChromeOS fingerprint binding 2022-03-15 0:23 ` Alexandru M Stan @ 2022-03-15 15:50 ` Stephen Boyd 0 siblings, 0 replies; 17+ messages in thread From: Stephen Boyd @ 2022-03-15 15:50 UTC (permalink / raw) To: Alexandru M Stan Cc: Benson Leung, linux-kernel, chrome-platform, Rob Herring, devicetree, Guenter Roeck, Douglas Anderson, Craig Hesling, Tom Hughes 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. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] dt-bindings: mfd: Add ChromeOS fingerprint binding 2022-03-14 23:22 ` [PATCH 1/2] dt-bindings: mfd: Add ChromeOS fingerprint binding Stephen Boyd 2022-03-15 0:23 ` Alexandru M Stan @ 2022-03-15 3:08 ` Tzung-Bi Shih 2022-03-15 15:38 ` Stephen Boyd 2022-03-15 10:41 ` Krzysztof Kozlowski 2 siblings, 1 reply; 17+ messages in thread From: Tzung-Bi Shih @ 2022-03-15 3:08 UTC (permalink / raw) To: Stephen Boyd Cc: Benson Leung, linux-kernel, chrome-platform, Rob Herring, devicetree, Guenter Roeck, Douglas Anderson, Craig Hesling, Tom Hughes, Alexandru M Stan On Mon, Mar 14, 2022 at 04:22:13PM -0700, Stephen Boyd wrote: > Add a binding to describe the fingerprint processor found on Chromeboks > with a fingerprint sensor. Nit: s/Chromeboks/Chromebooks/. > +properties: > + compatible: > + const: google,cros-ec-fp Not sure if it could make sense for FPS with other interfaces: would cros-ec-fp-spi or cros-ec-spi-fp be a better name? I am wondering because there are cros-ec-spi specific properties "google,cros-ec-spi-pre-delay" and "google,cros-ec-spi-msg-delay" in the binding. > + 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. To be consistent: s/Power/power/. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] dt-bindings: mfd: Add ChromeOS fingerprint binding 2022-03-15 3:08 ` Tzung-Bi Shih @ 2022-03-15 15:38 ` Stephen Boyd 0 siblings, 0 replies; 17+ messages in thread From: Stephen Boyd @ 2022-03-15 15:38 UTC (permalink / raw) To: Tzung-Bi Shih Cc: Benson Leung, linux-kernel, chrome-platform, Rob Herring, devicetree, Guenter Roeck, Douglas Anderson, Craig Hesling, Tom Hughes, Alexandru M Stan Quoting Tzung-Bi Shih (2022-03-14 20:08:47) > On Mon, Mar 14, 2022 at 04:22:13PM -0700, Stephen Boyd wrote: > > Add a binding to describe the fingerprint processor found on Chromeboks > > with a fingerprint sensor. > > Nit: s/Chromeboks/Chromebooks/. > > > +properties: > > + compatible: > > + const: google,cros-ec-fp > > Not sure if it could make sense for FPS with other interfaces: would > cros-ec-fp-spi or cros-ec-spi-fp be a better name? I am wondering because > there are cros-ec-spi specific properties "google,cros-ec-spi-pre-delay" and > "google,cros-ec-spi-msg-delay" in the binding. The delays are optional properties so I don't see much value in encoding the bus type into the compatible string. It would only help us find properties that are unused on a bus that isn't SPI. Let's not overcomplicate things. > > > + 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. > > To be consistent: s/Power/power/. I fixed the other two, thanks! ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] dt-bindings: mfd: Add ChromeOS fingerprint binding 2022-03-14 23:22 ` [PATCH 1/2] dt-bindings: mfd: Add ChromeOS fingerprint binding Stephen Boyd 2022-03-15 0:23 ` Alexandru M Stan 2022-03-15 3:08 ` Tzung-Bi Shih @ 2022-03-15 10:41 ` Krzysztof Kozlowski 2022-03-15 11:10 ` Lee Jones 2 siblings, 1 reply; 17+ messages in thread From: Krzysztof Kozlowski @ 2022-03-15 10:41 UTC (permalink / raw) To: Stephen Boyd, Benson Leung Cc: linux-kernel, chrome-platform, Rob Herring, devicetree, Guenter Roeck, Douglas Anderson, Craig Hesling, Tom Hughes, Alexandru M Stan, Lee Jones 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] dt-bindings: mfd: Add ChromeOS fingerprint binding 2022-03-15 10:41 ` Krzysztof Kozlowski @ 2022-03-15 11:10 ` Lee Jones 2022-03-15 11:22 ` Krzysztof Kozlowski 0 siblings, 1 reply; 17+ messages in thread From: Lee Jones @ 2022-03-15 11:10 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Stephen Boyd, Benson Leung, linux-kernel, chrome-platform, Rob Herring, devicetree, Guenter Roeck, Douglas Anderson, Craig Hesling, Tom Hughes, Alexandru M Stan 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. -- Lee Jones [李琼斯] Principal Technical Lead - Developer Services Linaro.org │ Open source software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] dt-bindings: mfd: Add ChromeOS fingerprint binding 2022-03-15 11:10 ` Lee Jones @ 2022-03-15 11:22 ` Krzysztof Kozlowski 2022-03-15 11:30 ` Lee Jones 0 siblings, 1 reply; 17+ messages in thread From: Krzysztof Kozlowski @ 2022-03-15 11:22 UTC (permalink / raw) To: Lee Jones Cc: Stephen Boyd, Benson Leung, linux-kernel, chrome-platform, Rob Herring, devicetree, Guenter Roeck, Douglas Anderson, Craig Hesling, Tom Hughes, Alexandru M Stan 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. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] dt-bindings: mfd: Add ChromeOS fingerprint binding 2022-03-15 11:22 ` Krzysztof Kozlowski @ 2022-03-15 11:30 ` Lee Jones 2022-03-15 15:41 ` Stephen Boyd 0 siblings, 1 reply; 17+ messages in thread From: Lee Jones @ 2022-03-15 11:30 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Stephen Boyd, Benson Leung, linux-kernel, chrome-platform, Rob Herring, devicetree, Guenter Roeck, Douglas Anderson, Craig Hesling, Tom Hughes, Alexandru M Stan 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. -- Lee Jones [李琼斯] Principal Technical Lead - Developer Services Linaro.org │ Open source software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] dt-bindings: mfd: Add ChromeOS fingerprint binding 2022-03-15 11:30 ` Lee Jones @ 2022-03-15 15:41 ` Stephen Boyd 2022-03-15 15:48 ` Lee Jones 0 siblings, 1 reply; 17+ messages in thread From: Stephen Boyd @ 2022-03-15 15:41 UTC (permalink / raw) To: Krzysztof Kozlowski, Lee Jones Cc: Benson Leung, linux-kernel, chrome-platform, Rob Herring, devicetree, Guenter Roeck, Douglas Anderson, Craig Hesling, Tom Hughes, Alexandru M Stan 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. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] dt-bindings: mfd: Add ChromeOS fingerprint binding 2022-03-15 15:41 ` Stephen Boyd @ 2022-03-15 15:48 ` Lee Jones 2022-03-15 16:19 ` Stephen Boyd 0 siblings, 1 reply; 17+ messages in thread From: Lee Jones @ 2022-03-15 15:48 UTC (permalink / raw) To: Stephen Boyd Cc: Krzysztof Kozlowski, Benson Leung, linux-kernel, chrome-platform, Rob Herring, devicetree, Guenter Roeck, Douglas Anderson, Craig Hesling, Tom Hughes, Alexandru M Stan On Tue, 15 Mar 2022, Stephen Boyd wrote: > 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. 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). Best to rely on `get_maintainer.pl` for this: $ ./scripts/get_maintainer.pl -f Documentation/devicetree/bindings/mfd/ Lee Jones <lee.jones@linaro.org> (supporter:MULTIFUNCTION DEVICES (MFD)) Rob Herring <robh+dt@kernel.org> (maintainer:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS) devicetree@vger.kernel.org (open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS) linux-kernel@vger.kernel.org (open list) -- Lee Jones [李琼斯] Principal Technical Lead - Developer Services Linaro.org │ Open source software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] dt-bindings: mfd: Add ChromeOS fingerprint binding 2022-03-15 15:48 ` Lee Jones @ 2022-03-15 16:19 ` Stephen Boyd 2022-03-16 7:25 ` Lee Jones 0 siblings, 1 reply; 17+ messages in thread From: Stephen Boyd @ 2022-03-15 16:19 UTC (permalink / raw) To: Lee Jones Cc: Krzysztof Kozlowski, Benson Leung, linux-kernel, chrome-platform, Rob Herring, devicetree, Guenter Roeck, Douglas Anderson, Craig Hesling, Tom Hughes, Alexandru M Stan 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 and up to maintainer's choice. I'll move it out of the mfd directory :) ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/2] dt-bindings: mfd: Add ChromeOS fingerprint binding 2022-03-15 16:19 ` Stephen Boyd @ 2022-03-16 7:25 ` Lee Jones 0 siblings, 0 replies; 17+ messages in thread From: Lee Jones @ 2022-03-16 7:25 UTC (permalink / raw) To: Stephen Boyd Cc: Krzysztof Kozlowski, Benson Leung, linux-kernel, chrome-platform, Rob Herring, devicetree, Guenter Roeck, Douglas Anderson, Craig Hesling, Tom Hughes, Alexandru M Stan 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. -- Lee Jones [李琼斯] Principal Technical Lead - Developer Services Linaro.org │ Open source software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 2/2] platform/chrome: cros_ec_spi: Boot fingerprint processor during probe 2022-03-14 23:22 [PATCH 0/2] Update cros-ec-spi for fingerprint devices Stephen Boyd 2022-03-14 23:22 ` [PATCH 1/2] dt-bindings: mfd: Add ChromeOS fingerprint binding Stephen Boyd @ 2022-03-14 23:22 ` Stephen Boyd 2022-03-15 0:36 ` Alexandru M Stan 1 sibling, 1 reply; 17+ messages in thread From: Stephen Boyd @ 2022-03-14 23:22 UTC (permalink / raw) To: Benson Leung Cc: linux-kernel, chrome-platform, Guenter Roeck, Douglas Anderson, Craig Hesling, Tom Hughes, Alexandru M Stan Add gpio control to this driver so that the fingerprint device can be booted if the BIOS isn't doing it already. This eases bringup of new hardware as we don't have to wait for the BIOS to be ready, supports kexec where the GPIOs may not be configured by the previous boot stage, and is all around good hygiene because we control GPIOs for this device from the device driver. 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> --- drivers/platform/chrome/cros_ec_spi.c | 38 ++++++++++++++++++++++++--- 1 file changed, 35 insertions(+), 3 deletions(-) diff --git a/drivers/platform/chrome/cros_ec_spi.c b/drivers/platform/chrome/cros_ec_spi.c index 14c4046fa04d..77577650afce 100644 --- a/drivers/platform/chrome/cros_ec_spi.c +++ b/drivers/platform/chrome/cros_ec_spi.c @@ -4,6 +4,7 @@ // Copyright (C) 2012 Google, Inc #include <linux/delay.h> +#include <linux/gpio/consumer.h> #include <linux/kernel.h> #include <linux/module.h> #include <linux/of.h> @@ -77,6 +78,8 @@ struct cros_ec_spi { unsigned int start_of_msg_delay; unsigned int end_of_msg_delay; struct kthread_worker *high_pri_worker; + struct gpio_desc *boot0; + struct gpio_desc *reset; }; typedef int (*cros_ec_xfer_fn_t) (struct cros_ec_device *ec_dev, @@ -690,7 +693,7 @@ static int cros_ec_cmd_xfer_spi(struct cros_ec_device *ec_dev, return cros_ec_xfer_high_pri(ec_dev, ec_msg, do_cros_ec_cmd_xfer_spi); } -static void cros_ec_spi_dt_probe(struct cros_ec_spi *ec_spi, struct device *dev) +static int cros_ec_spi_dt_probe(struct cros_ec_spi *ec_spi, struct device *dev) { struct device_node *np = dev->of_node; u32 val; @@ -703,6 +706,31 @@ static void cros_ec_spi_dt_probe(struct cros_ec_spi *ec_spi, struct device *dev) ret = of_property_read_u32(np, "google,cros-ec-spi-msg-delay", &val); if (!ret) ec_spi->end_of_msg_delay = val; + + if (!of_device_is_compatible(np, "google,cros-ec-fp")) + return 0; + + ec_spi->boot0 = devm_gpiod_get(dev, "boot0", 0); + if (IS_ERR(ec_spi->boot0)) + return PTR_ERR(ec_spi->boot0); + + ec_spi->reset = devm_gpiod_get(dev, "reset", 0); + if (IS_ERR(ec_spi->reset)) + return PTR_ERR(ec_spi->reset); + + /* + * Take the FPMCU out of reset and wait for it to boot if it's in + * bootloader mode or held in reset. Otherwise the BIOS has already + * powered on the device earlier in boot in which case there's nothing + * to do. + */ + if (!gpiod_get_value(ec_spi->boot0) || gpiod_get_value(ec_spi->reset)) { + gpiod_set_value(ec_spi->boot0, 1); + gpiod_set_value(ec_spi->reset, 0); + usleep_range(1000, 2000); + } + + return 0; } static void cros_ec_spi_high_pri_release(void *worker) @@ -754,8 +782,10 @@ static int cros_ec_spi_probe(struct spi_device *spi) if (!ec_dev) return -ENOMEM; - /* Check for any DT properties */ - cros_ec_spi_dt_probe(ec_spi, dev); + /* Check for any DT properties and boot fpmcu if applicable */ + err = cros_ec_spi_dt_probe(ec_spi, dev); + if (err) + return err; spi_set_drvdata(spi, ec_dev); ec_dev->dev = dev; @@ -813,12 +843,14 @@ static SIMPLE_DEV_PM_OPS(cros_ec_spi_pm_ops, cros_ec_spi_suspend, cros_ec_spi_resume); static const struct of_device_id cros_ec_spi_of_match[] = { + { .compatible = "google,cros-ec-fp", }, { .compatible = "google,cros-ec-spi", }, { /* sentinel */ }, }; MODULE_DEVICE_TABLE(of, cros_ec_spi_of_match); static const struct spi_device_id cros_ec_spi_id[] = { + { "cros-ec-fp", 0 }, { "cros-ec-spi", 0 }, { } }; -- https://chromeos.dev ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] platform/chrome: cros_ec_spi: Boot fingerprint processor during probe 2022-03-14 23:22 ` [PATCH 2/2] platform/chrome: cros_ec_spi: Boot fingerprint processor during probe Stephen Boyd @ 2022-03-15 0:36 ` Alexandru M Stan 2022-03-15 16:16 ` Stephen Boyd 0 siblings, 1 reply; 17+ messages in thread From: Alexandru M Stan @ 2022-03-15 0:36 UTC (permalink / raw) To: Stephen Boyd Cc: Benson Leung, linux-kernel, chrome-platform, Guenter Roeck, Douglas Anderson, Craig Hesling, Tom Hughes On Mon, Mar 14, 2022 at 4:22 PM Stephen Boyd <swboyd@chromium.org> wrote: > > Add gpio control to this driver so that the fingerprint device can be > booted if the BIOS isn't doing it already. This eases bringup of new > hardware as we don't have to wait for the BIOS to be ready, supports > kexec where the GPIOs may not be configured by the previous boot stage, > and is all around good hygiene because we control GPIOs for this device > from the device driver. > > 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> > --- > drivers/platform/chrome/cros_ec_spi.c | 38 ++++++++++++++++++++++++--- > 1 file changed, 35 insertions(+), 3 deletions(-) > > diff --git a/drivers/platform/chrome/cros_ec_spi.c b/drivers/platform/chrome/cros_ec_spi.c > index 14c4046fa04d..77577650afce 100644 > --- a/drivers/platform/chrome/cros_ec_spi.c > +++ b/drivers/platform/chrome/cros_ec_spi.c > @@ -4,6 +4,7 @@ > // Copyright (C) 2012 Google, Inc > > #include <linux/delay.h> > +#include <linux/gpio/consumer.h> > #include <linux/kernel.h> > #include <linux/module.h> > #include <linux/of.h> > @@ -77,6 +78,8 @@ struct cros_ec_spi { > unsigned int start_of_msg_delay; > unsigned int end_of_msg_delay; > struct kthread_worker *high_pri_worker; > + struct gpio_desc *boot0; > + struct gpio_desc *reset; > }; > > typedef int (*cros_ec_xfer_fn_t) (struct cros_ec_device *ec_dev, > @@ -690,7 +693,7 @@ static int cros_ec_cmd_xfer_spi(struct cros_ec_device *ec_dev, > return cros_ec_xfer_high_pri(ec_dev, ec_msg, do_cros_ec_cmd_xfer_spi); > } > > -static void cros_ec_spi_dt_probe(struct cros_ec_spi *ec_spi, struct device *dev) > +static int cros_ec_spi_dt_probe(struct cros_ec_spi *ec_spi, struct device *dev) > { > struct device_node *np = dev->of_node; > u32 val; > @@ -703,6 +706,31 @@ static void cros_ec_spi_dt_probe(struct cros_ec_spi *ec_spi, struct device *dev) > ret = of_property_read_u32(np, "google,cros-ec-spi-msg-delay", &val); > if (!ret) > ec_spi->end_of_msg_delay = val; > + > + if (!of_device_is_compatible(np, "google,cros-ec-fp")) > + return 0; > + > + ec_spi->boot0 = devm_gpiod_get(dev, "boot0", 0); > + if (IS_ERR(ec_spi->boot0)) > + return PTR_ERR(ec_spi->boot0); > + > + ec_spi->reset = devm_gpiod_get(dev, "reset", 0); > + if (IS_ERR(ec_spi->reset)) > + return PTR_ERR(ec_spi->reset); > + > + /* > + * Take the FPMCU out of reset and wait for it to boot if it's in > + * bootloader mode or held in reset. Otherwise the BIOS has already > + * powered on the device earlier in boot in which case there's nothing > + * to do. > + */ > + if (!gpiod_get_value(ec_spi->boot0) || gpiod_get_value(ec_spi->reset)) { > + gpiod_set_value(ec_spi->boot0, 1); If you follow my suggestions on the other patch to keep boot0 active_high, this needs to change back to a 0. Since normal=0. > + gpiod_set_value(ec_spi->reset, 0); > + usleep_range(1000, 2000); You have to be careful here. I assume by the end of this if you expect the fpmcu to be in normal mode. There is a corner case (which I guess is unlikely) that the fpmcu, just before the if, was out of reset but in bootloader mode (boot0=1, nrst=1, physically). At that point this code will enter the if body and set boot0=0, but the reset signal would not be changed (we're already out of reset!), so we'll still be in bootloader mode since there was no resetting (so no sampling of boot0). I suggest you change this if body to look like: + gpiod_set_value(ec_spi->boot0, 0); // normal mode + gpiod_set_value(ec_spi->reset, 1); // enter reset (these reset comments might be optional) + usleep_range(1000, 2000); + gpiod_set_value(ec_spi->reset, 0); // release reset + usleep_range(1000, 2000); // maybe increase? In the normal case (when we do have firmware support). All this code will probably get skipped and there will be plenty of time to boot the fpmcu before the driver enumerates. Speaking of, have you tried this path? I think booting (fpmcu all the way to RW) usually takes forever (seconds or so?). I remember Craig mentioning this is why we want to do it so early (in ap firmware) since by the time the kernel tries to enumerate it'll be done. If that's the case you might want to change that last usleep_range to actually wait that long. > + } > + > + return 0; > } > > static void cros_ec_spi_high_pri_release(void *worker) > @@ -754,8 +782,10 @@ static int cros_ec_spi_probe(struct spi_device *spi) > if (!ec_dev) > return -ENOMEM; > > - /* Check for any DT properties */ > - cros_ec_spi_dt_probe(ec_spi, dev); > + /* Check for any DT properties and boot fpmcu if applicable */ > + err = cros_ec_spi_dt_probe(ec_spi, dev); > + if (err) > + return err; > > spi_set_drvdata(spi, ec_dev); > ec_dev->dev = dev; > @@ -813,12 +843,14 @@ static SIMPLE_DEV_PM_OPS(cros_ec_spi_pm_ops, cros_ec_spi_suspend, > cros_ec_spi_resume); > > static const struct of_device_id cros_ec_spi_of_match[] = { > + { .compatible = "google,cros-ec-fp", }, > { .compatible = "google,cros-ec-spi", }, > { /* sentinel */ }, > }; > MODULE_DEVICE_TABLE(of, cros_ec_spi_of_match); > > static const struct spi_device_id cros_ec_spi_id[] = { > + { "cros-ec-fp", 0 }, > { "cros-ec-spi", 0 }, > { } > }; > -- > https://chromeos.dev > Alexandru Stan (amstan) ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/2] platform/chrome: cros_ec_spi: Boot fingerprint processor during probe 2022-03-15 0:36 ` Alexandru M Stan @ 2022-03-15 16:16 ` Stephen Boyd 0 siblings, 0 replies; 17+ messages in thread From: Stephen Boyd @ 2022-03-15 16:16 UTC (permalink / raw) To: Alexandru M Stan Cc: Benson Leung, linux-kernel, chrome-platform, Guenter Roeck, Douglas Anderson, Craig Hesling, Tom Hughes Quoting Alexandru M Stan (2022-03-14 17:36:47) > On Mon, Mar 14, 2022 at 4:22 PM Stephen Boyd <swboyd@chromium.org> wrote: > > > > Add gpio control to this driver so that the fingerprint device can be > > booted if the BIOS isn't doing it already. This eases bringup of new > > hardware as we don't have to wait for the BIOS to be ready, supports > > kexec where the GPIOs may not be configured by the previous boot stage, > > and is all around good hygiene because we control GPIOs for this device > > from the device driver. > > > > 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> > > --- > > drivers/platform/chrome/cros_ec_spi.c | 38 ++++++++++++++++++++++++--- > > 1 file changed, 35 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/platform/chrome/cros_ec_spi.c b/drivers/platform/chrome/cros_ec_spi.c > > index 14c4046fa04d..77577650afce 100644 > > --- a/drivers/platform/chrome/cros_ec_spi.c > > +++ b/drivers/platform/chrome/cros_ec_spi.c > > @@ -4,6 +4,7 @@ > > // Copyright (C) 2012 Google, Inc > > > > #include <linux/delay.h> > > +#include <linux/gpio/consumer.h> > > #include <linux/kernel.h> > > #include <linux/module.h> > > #include <linux/of.h> > > @@ -77,6 +78,8 @@ struct cros_ec_spi { > > unsigned int start_of_msg_delay; > > unsigned int end_of_msg_delay; > > struct kthread_worker *high_pri_worker; > > + struct gpio_desc *boot0; > > + struct gpio_desc *reset; > > }; > > > > typedef int (*cros_ec_xfer_fn_t) (struct cros_ec_device *ec_dev, > > @@ -690,7 +693,7 @@ static int cros_ec_cmd_xfer_spi(struct cros_ec_device *ec_dev, > > return cros_ec_xfer_high_pri(ec_dev, ec_msg, do_cros_ec_cmd_xfer_spi); > > } > > > > -static void cros_ec_spi_dt_probe(struct cros_ec_spi *ec_spi, struct device *dev) > > +static int cros_ec_spi_dt_probe(struct cros_ec_spi *ec_spi, struct device *dev) > > { > > struct device_node *np = dev->of_node; > > u32 val; > > @@ -703,6 +706,31 @@ static void cros_ec_spi_dt_probe(struct cros_ec_spi *ec_spi, struct device *dev) > > ret = of_property_read_u32(np, "google,cros-ec-spi-msg-delay", &val); > > if (!ret) > > ec_spi->end_of_msg_delay = val; > > + > > + if (!of_device_is_compatible(np, "google,cros-ec-fp")) > > + return 0; > > + > > + ec_spi->boot0 = devm_gpiod_get(dev, "boot0", 0); > > + if (IS_ERR(ec_spi->boot0)) > > + return PTR_ERR(ec_spi->boot0); > > + > > + ec_spi->reset = devm_gpiod_get(dev, "reset", 0); > > + if (IS_ERR(ec_spi->reset)) > > + return PTR_ERR(ec_spi->reset); > > + > > + /* > > + * Take the FPMCU out of reset and wait for it to boot if it's in > > + * bootloader mode or held in reset. Otherwise the BIOS has already > > + * powered on the device earlier in boot in which case there's nothing > > + * to do. > > + */ > > + if (!gpiod_get_value(ec_spi->boot0) || gpiod_get_value(ec_spi->reset)) { > > + gpiod_set_value(ec_spi->boot0, 1); > If you follow my suggestions on the other patch to keep boot0 > active_high, this needs to change back to a 0. Since normal=0. > > > + gpiod_set_value(ec_spi->reset, 0); > > + usleep_range(1000, 2000); > You have to be careful here. I assume by the end of this if you expect > the fpmcu to be in normal mode. > > There is a corner case (which I guess is unlikely) that the fpmcu, > just before the if, was out of reset but in bootloader mode (boot0=1, > nrst=1, physically). At that point this code will enter the if body > and set boot0=0, but the reset signal would not be changed (we're > already out of reset!), so we'll still be in bootloader mode since > there was no resetting (so no sampling of boot0). > > I suggest you change this if body to look like: > > + gpiod_set_value(ec_spi->boot0, 0); // normal mode > + gpiod_set_value(ec_spi->reset, 1); // enter reset > (these reset comments might be optional) > + usleep_range(1000, 2000); > + gpiod_set_value(ec_spi->reset, 0); // release reset > + usleep_range(1000, 2000); // maybe increase? Ok got it. I'll make the change and test it out. > > In the normal case (when we do have firmware support). All this code > will probably get skipped and there will be plenty of time to boot the > fpmcu before the driver enumerates. > > Speaking of, have you tried this path? I think booting (fpmcu all the > way to RW) usually takes forever (seconds or so?). I remember Craig > mentioning this is why we want to do it so early (in ap firmware) > since by the time the kernel tries to enumerate it'll be done. > > If that's the case you might want to change that last usleep_range to > actually wait that long. I tested this by running the flashing code and then seeing if the cros ec driver probed. I'll have to manually put it into bootloader mode and then bind this driver to see how long it takes. Is there any way I can poll an irq line or something to wait for boot? That would be much better vs. sticking some delay into here. ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2022-03-16 7:25 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-03-14 23:22 [PATCH 0/2] Update cros-ec-spi for fingerprint devices Stephen Boyd 2022-03-14 23:22 ` [PATCH 1/2] dt-bindings: mfd: Add ChromeOS fingerprint binding Stephen Boyd 2022-03-15 0:23 ` Alexandru M Stan 2022-03-15 15:50 ` Stephen Boyd 2022-03-15 3:08 ` Tzung-Bi Shih 2022-03-15 15:38 ` Stephen Boyd 2022-03-15 10:41 ` Krzysztof Kozlowski 2022-03-15 11:10 ` Lee Jones 2022-03-15 11:22 ` Krzysztof Kozlowski 2022-03-15 11:30 ` Lee Jones 2022-03-15 15:41 ` Stephen Boyd 2022-03-15 15:48 ` Lee Jones 2022-03-15 16:19 ` Stephen Boyd 2022-03-16 7:25 ` Lee Jones 2022-03-14 23:22 ` [PATCH 2/2] platform/chrome: cros_ec_spi: Boot fingerprint processor during probe Stephen Boyd 2022-03-15 0:36 ` Alexandru M Stan 2022-03-15 16:16 ` Stephen Boyd
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).