linux-kernel - Re: [PATCH v3 1/3] dt-bindings: reset: add sky1 reset controller

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] [day] [month] [year] [list]

Message-ID: <20251124-selector-blemish-ec6e9a356bc6@spud>
Date: Mon, 24 Nov 2025 19:54:07 +0000
From: Conor Dooley <conor@...nel.org>
To: Gary Yang <gary.yang@...tech.com>
Cc: p.zabel@...gutronix.de, robh@...nel.org, krzk+dt@...nel.org,
	conor+dt@...nel.org, devicetree@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	cix-kernel-upstream@...tech.com
Subject: Re: [PATCH v3 1/3] dt-bindings: reset: add sky1 reset controller

On Mon, Nov 24, 2025 at 02:32:33PM +0800, Gary Yang wrote:
> There are two reset controllers on Cix sky1 Soc.
> One is located in S0 domain, and the other is located
> in S5 domain.
> 
> Signed-off-by: Gary Yang <gary.yang@...tech.com>
> ---
>  .../bindings/reset/cix,sky1-rst.yaml          |  50 ++++++
>  include/dt-bindings/reset/cix,sky1-rst-fch.h  |  42 +++++
>  include/dt-bindings/reset/cix,sky1-rst.h      | 164 ++++++++++++++++++
>  3 files changed, 256 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/reset/cix,sky1-rst.yaml
>  create mode 100644 include/dt-bindings/reset/cix,sky1-rst-fch.h
>  create mode 100644 include/dt-bindings/reset/cix,sky1-rst.h
> 
> diff --git a/Documentation/devicetree/bindings/reset/cix,sky1-rst.yaml b/Documentation/devicetree/bindings/reset/cix,sky1-rst.yaml
> new file mode 100644
> index 000000000000..a28f938a283d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reset/cix,sky1-rst.yaml
> @@ -0,0 +1,50 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/reset/cix,sky1-rst.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: CIX Sky1 Reset Controller
> +
> +maintainers:
> +  - Gary Yang <gary.yang@...tech.com>
> +
> +description: |
> +  CIX Sky1 reset controller can be used to reset various set of peripherals.
> +  There are two reset controllers, one is located in S0 domain, the other
> +  is located in S5 domain.
> +
> +  See also:
> +  - include/dt-bindings/reset/cix,sky1-rst.h
> +
> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - cix,sky1-rst
> +          - cix,sky1-rst-fch

You've not addressed my v2 commentary:
https://lore.kernel.org/all/20251114-problem-overbook-383f8e45cd0b@spud/
I asked what else the device does, but you didn't answer me. Dropping the
syscon doesn't make sense if the device genuinely has other functions.

> +
> +  reg:
> +    minItems: 1
> +    maxItems: 3
> +
> +  '#reset-cells':
> +    const: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - '#reset-cells'
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/reset/cix,sky1-rst.h>
> +    reset-controller@...00304 {
> +      compatible = "cix,sky1-rst";

> +      reg = <0x16000304 0xc>,
> +            <0x16000400 0x10>,
> +            <0x16000800 0x8>;

This is also highly suspect, and I believe what you had before was
probably much more realistic.
Do things properly and fully *now*, rather than pay the price of
unravelling it all later. I just did this for one of my own platforms,
and putting in the effort to completely describe stuff up front is
actually worth it rather than having to refactor years down the line.

Cheers,
Conor.

pw-bot: changes-requested

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)