lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 28 Oct 2021 11:19:33 -0400 From: Sean Anderson <sean.anderson@...o.com> To: Rob Herring <robh@...nel.org> Cc: Philipp Zabel <p.zabel@...gutronix.de>, linux-kernel@...r.kernel.org, devicetree@...r.kernel.org Subject: Re: [PATCH 1/2] dt-bindings: reset: Add generic GPIO reset binding Hi Rob, On 10/26/21 10:27 PM, Rob Herring wrote: > On Mon, Oct 18, 2021 at 07:49:21PM -0400, Sean Anderson wrote: >> This adds a binding for a generic GPIO reset driver. This driver is >> designed to easily add a GPIO-based reset to a driver which expected a >> reset controller. It offers greater flexibility than a reset-gpios >> property, and allows for one code path to be shared for GPIO resets and >> MMIO-based resets. > > I would like to do this last part, but not requiring a binding change. > IOW, be able to register any 'reset-gpios' property as a reset provider > directly without this added level of indirection. That would be nice, but it seems like someone would have to go through every driver with a reset-gpios property and convert them. Since the reset GPIOs are >> >> Signed-off-by: Sean Anderson <sean.anderson@...o.com> >> --- >> >> .../devicetree/bindings/reset/gpio-reset.yaml | 93 +++++++++++++++++++ >> 1 file changed, 93 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/reset/gpio-reset.yaml >> >> diff --git a/Documentation/devicetree/bindings/reset/gpio-reset.yaml b/Documentation/devicetree/bindings/reset/gpio-reset.yaml >> new file mode 100644 >> index 000000000000..de2ab074cea3 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/reset/gpio-reset.yaml >> @@ -0,0 +1,93 @@ >> +# SPDX-License-Identifier: (GPL-2.0+ OR BSD-2-Clause) > > GPL-2.0-only not GPL-2.0+ GPL-2.0+ is a strict superset. And bindings are required to be BSD anyway. I don't see the issue. >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/reset/gpio-reset.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Generic GPIO reset driver >> + >> +maintainers: >> + - Sean Anderson <seanga2@...il.com> >> + >> +description: | >> + This is a generic GPIO reset driver which can provide a reset-controller >> + interface for GPIO-based reset lines. This driver always operates with >> + logical GPIO values; to invert the polarity, specify GPIO_ACTIVE_LOW in the >> + GPIO's flags. >> + >> +properties: >> + compatible: >> + const: gpio-reset >> + >> + '#reset-cells': >> + const: 1 >> + >> + reset-gpios: >> + description: | >> + GPIOs to assert when asserting a reset. There is a one-to-one mapping >> + between the reset specifier and the index of the GPIO in this list to >> + assert. >> + >> + done-gpios: >> + description: | >> + GPIOs which indicate that the device controlled by the GPIO has exited >> + reset. There must be one done GPIO for each reset GPIO, or no done GPIOs >> + at all. The driver will wait for up to done-timeout-us for the >> + corresponding done GPIO to assert before returning. > > This is odd. Do you have some examples of h/w needing this done signal? > It certainly doesn't seem like something we have a generic need for. Yes [1]. This device has a "reset done" signal, but no reset timings specified in the datasheet. I don't know if this is truly needed, because we can read the ID register, but it is nice when bringing up the device. I left it in because I was using it. [1] https://lore.kernel.org/netdev/20211004191527.1610759-16-sean.anderson@seco.com/ >> + >> + pre-assert-us: >> + default: 0 >> + description: | >> + Microseconds to delay between when the reset was requested to be >> + asserted, and asserting the reset GPIO >> + >> + post-assert-us: >> + default: 0 >> + description: | >> + Microseconds to delay after asserting the reset GPIO and before returning >> + to the caller. >> + >> + pre-deassert-us: >> + default: 0 >> + description: | >> + Microseconds to delay between when the reset was requested to be >> + deasserted, and asserting the reset GPIO >> + >> + post-deassert-us: >> + default: 0 >> + description: | >> + Microseconds to delay after deasserting the reset GPIO and before >> + returning to the caller. This delay is always present, even if the done >> + GPIO goes high earlier. >> + >> + done-timeout-us: >> + default: 1000 >> + description: >> + Microseconds to wait for the done GPIO to assert after deasserting the >> + reset GPIO. If post-deassert-us is present, this property defaults to 10 >> + times that delay. The timeout starts after waiting for the post deassert >> + delay. > > There's a reason we don't have all these timing values in DT. The timing > requirements are defined by each device (being reset) and implied by > their compatible strings. If we wanted a macro language for power > sequence timings of regulators, clocks, resets, enables, etc., then we > would have designed such a thing already. Well, there are already things like reset-assert-us and reset-deassert-us in [2, 3, 4] (with different names(!)). Part of what I want to address with this device is that there are several existing properties which specify various aspects of the above timings. I think it would be good to standardize on these. Maybe this should be a property which applies to the reset consumer? Analogously, we also have assigned-clocks so that not every driver has to know what the correct frequency/parent is (especially when they can vary among different hardware variations). --Sean [2] Documentation/devicetree/bindings/net/ethernet-phy.yaml [3] Documentation/devicetree/bindings/net/mdio.yaml [4] Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.yaml >> + >> +required: >> + - '#reset-cells' >> + - compatible >> + - reset-gpios >> + >> +additionalProperties: false >> + >> +examples: >> + - | >> + #include <dt-bindings/gpio/gpio.h> >> + pcs_reset: reset-pcs { >> + #reset-cells = <1>; >> + compatible = "gpio-reset"; >> + reset-gpios = <&gpio 0 GPIO_ACTIVE_LOW>, >> + <&gpio 1 GPIO_ACTIVE_LOW>, >> + <&gpio 2 GPIO_ACTIVE_LOW>, >> + <&gpio 3 GPIO_ACTIVE_LOW>; >> + done-gpios = <&gpio 4 GPIO_ACTIVE_HIGH>, >> + <&gpio 5 GPIO_ACTIVE_HIGH>, >> + <&gpio 6 GPIO_ACTIVE_HIGH>, >> + <&gpio 7 GPIO_ACTIVE_HIGH>; >> + post-deassert-us = <100>; >> + }; >> -- >> 2.25.1 >> >> >
Powered by blists - more mailing lists