| 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: Wed, 28 Jun 2023 18:34:03 +0100
From: Conor Dooley <conor@...nel.org>
To: Xingyu Wu <xingyu.wu@...rfivetech.com>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
linux-riscv@...ts.infradead.org, devicetree@...r.kernel.org,
Michael Turquette <mturquette@...libre.com>,
Stephen Boyd <sboyd@...nel.org>,
Rob Herring <robh+dt@...nel.org>,
Philipp Zabel <p.zabel@...gutronix.de>,
Conor Dooley <conor+dt@...nel.org>,
Emil Renner Berthing <emil.renner.berthing@...onical.com>,
Paul Walmsley <paul.walmsley@...ive.com>,
Palmer Dabbelt <palmer@...belt.com>,
Albert Ou <aou@...s.berkeley.edu>,
Hal Feng <hal.feng@...rfivetech.com>,
William Qiu <william.qiu@...rfivetech.com>,
linux-kernel@...r.kernel.org, linux-clk@...r.kernel.org
Subject: Re: [PATCH v5 2/7] dt-bindings: soc: starfive: Add StarFive syscon
module
On Wed, Jun 28, 2023 at 02:44:10PM +0800, Xingyu Wu wrote:
> On 2023/6/14 2:31, Krzysztof Kozlowski wrote:
> > On 13/06/2023 14:58, Xingyu Wu wrote:
> >> From: William Qiu <william.qiu@...rfivetech.com>
> >>
> >> Add documentation to describe StarFive System Controller Registers.
> >>
> >> Co-developed-by: Xingyu Wu <xingyu.wu@...rfivetech.com>
> >> Signed-off-by: Xingyu Wu <xingyu.wu@...rfivetech.com>
> >> Signed-off-by: William Qiu <william.qiu@...rfivetech.com>
> >> ---
> >> .../soc/starfive/starfive,jh7110-syscon.yaml | 62 +++++++++++++++++++
> >> MAINTAINERS | 7 +++
> >> 2 files changed, 69 insertions(+)
> >> create mode 100644 Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
> >>
> >> diff --git a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
> >> new file mode 100644
> >> index 000000000000..a81190f8a54d
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
> >> @@ -0,0 +1,62 @@
> >> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> >> +%YAML 1.2
> >> +---
> >> +$id: http://devicetree.org/schemas/soc/starfive/starfive,jh7110-syscon.yaml#
> >> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >> +
> >> +title: StarFive JH7110 SoC system controller
> >> +
> >> +maintainers:
> >> + - William Qiu <william.qiu@...rfivetech.com>
> >> +
> >> +description: |
> >> + The StarFive JH7110 SoC system controller provides register information such
> >> + as offset, mask and shift to configure related modules such as MMC and PCIe.
> >> +
> >> +properties:
> >> + compatible:
> >> + oneOf:
> >> + - items:
> >> + - const: starfive,jh7110-sys-syscon
> >> + - const: syscon
> >> + - const: simple-mfd
> >> + - items:
> >> + - enum:
> >> + - starfive,jh7110-aon-syscon
> >> + - starfive,jh7110-stg-syscon
> >> + - const: syscon
> >> +
> >> + reg:
> >> + maxItems: 1
> >> +
> >> + clock-controller:
> >> + $ref: /schemas/clock/starfive,jh7110-pll.yaml#
> >> + type: object
> >> +
> >> + "#power-domain-cells":
> >> + const: 1
> >> +
> >> +required:
> >> + - compatible
> >> + - reg
> >> +
> >> +allOf:
> >> + - if:
> >> + properties:
> >> + compatible:
> >> + contains:
> >> + const: starfive,jh7110-aon-syscon
> >> + then:
> >> + required:
> >> + - "#power-domain-cells"
> >
> > Where did you implement the results of the discussion that only some
> > devices can have power and clock controller?
> >
> > According to your code all of above - sys, aon and stg - have clock and
> > power controllers. If not, then the code is not correct, so please do
> > not respond with what is where (like you did last time) but actually
> > implement what you say.
> >
>
> Hi Krzysztof, I need to modify the code to implement it.
> If I drop the 'clock-controller' and '"#power-domain-cells"' in properites, and change to this:
>
> --- a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
> +++ b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml
> @@ -29,28 +29,33 @@ properties:
> reg:
> maxItems: 1
>
> - clock-controller:
> - $ref: /schemas/clock/starfive,jh7110-pll.yaml#
> - type: object
> -
> - "#power-domain-cells":
> - const: 1
> -
> required:
> - compatible
> - reg
>
> allOf:
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: starfive,jh7110-sys-syscon
> + then:
> + properties:
> + clock-controller:
> + $ref: /schemas/clock/starfive,jh7110-pll.yaml#
> + type: object
Why do this?
Why not define the property has you have been doing, but only allow it
on the syscons that support it?
See the section starting at L205 of example-schema.yaml.
> +
> - if:
> properties:
> compatible:
> contains:
> const: starfive,jh7110-aon-syscon
> then:
> - required:
> - - "#power-domain-cells"
> + properties:
> + "#power-domain-cells":
> + const: 1
>
> -additionalProperties: false
> +additionalProperties: true
Why do you need this?
Allowing "additionalProperties: true" sounds like you've got some prblem
that you are trying to hide...
> Would it be better to show that sys-syscon only has clock-controller and aon-syscon is power controller?
You should only permit the properties where they are valid, yes.
Cheers,
Conor.
Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)
Powered by blists - more mailing lists