linux-kernel - Re: [PATCH 1/5] dt-bindings: spi: zynqmp-qspi: Add reset

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250127175752.GA511180-robh@kernel.org>
Date: Mon, 27 Jan 2025 11:57:52 -0600
From: Rob Herring <robh@...nel.org>
To: Sean Anderson <sean.anderson@...ux.dev>
Cc: Michal Simek <michal.simek@....com>, Mark Brown <broonie@...nel.org>,
	linux-spi@...r.kernel.org, linux-kernel@...r.kernel.org,
	Jinjie Ruan <ruanjinjie@...wei.com>,
	linux-arm-kernel@...ts.infradead.org,
	Amit Kumar Mahapatra <amit.kumar-mahapatra@....com>,
	Miquel Raynal <miquel.raynal@...tlin.com>,
	Conor Dooley <conor+dt@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	devicetree@...r.kernel.org
Subject: Re: [PATCH 1/5] dt-bindings: spi: zynqmp-qspi: Add reset

On Thu, Jan 23, 2025 at 05:57:41PM -0500, Sean Anderson wrote:
> On 1/23/25 17:45, Rob Herring wrote:
> > On Fri, Jan 17, 2025 at 11:12:15AM -0500, Sean Anderson wrote:
> >> On 1/17/25 02:14, Michal Simek wrote:
> >> > 
> >> > 
> >> > On 1/16/25 23:55, Sean Anderson wrote:
> >> >> Add a reset to help recover from cancelled operations.
> >> >>
> >> >> Signed-off-by: Sean Anderson <sean.anderson@...ux.dev>
> >> >> ---
> >> >>
> >> >>   Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.yaml | 6 ++++++
> >> >>   1 file changed, 6 insertions(+)
> >> >>
> >> >> diff --git a/Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.yaml b/Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.yaml
> >> >> index 04d4d3b4916d..901e15fcce2d 100644
> >> >> --- a/Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.yaml
> >> >> +++ b/Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.yaml
> >> >> @@ -36,12 +36,16 @@ properties:
> >> >>     power-domains:
> >> >>       maxItems: 1
> >> >>   +  resets:
> >> >> +    maxItems: 1
> >> >> +
> >> >>   required:
> >> >>     - compatible
> >> >>     - reg
> >> >>     - interrupts
> >> >>     - clock-names
> >> >>     - clocks
> >> >> +  - resets
> >> > 
> >> > In 2/5 you are calling devm_reset_control_get_optional_exclusive() that's why I expect reset is not really required property.
> >> 
> >> It's optional for the driver for backwards compatibility. But for the
> >> devicetree we make it mandatory since it should be included in all new
> >> devicetrees.
> > 
> > Generally, we discourage new required properties as that's an ABI 
> > change. The exception is really when optional was a mistake. That's 
> > arguably the case here if the h/w always has a reset.
> 
> This device has a reset on ZynqMP and Versal.
> 
> The driver still considers this property optional, so it's not an ABI break.
> But I made it required in the schema to help out the folks at AMD when they
> get around to upstreaming the Versal devicetree :)

Not 'the driver', but 'a driver'. You can't say what *all* drivers do. 
If I write a new driver and read the schema, then I can say 'resets is 
required so I'll make it required in my new driver'. But then my new 
driver doesn't work with an older DT that didn't have resets which was 
valid at the time.

Rob