Message ID | 20230613125852.211636-3-xingyu.wu@starfivetech.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:994d:0:b0:3d9:f83d:47d9 with SMTP id k13csp553938vqr; Tue, 13 Jun 2023 06:42:39 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ5CBo7ND47wt6l8KnvmLc64xiGznAjNPcWf//BK4itDhBNS9ypxJGoD51IVdRMjUJydyzcl X-Received: by 2002:a05:6a20:1442:b0:10c:1501:3c22 with SMTP id a2-20020a056a20144200b0010c15013c22mr13527502pzi.29.1686663759127; Tue, 13 Jun 2023 06:42:39 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1686663759; cv=none; d=google.com; s=arc-20160816; b=pBmx1FDTaQWPTxUQGL2MNt/O4GcfSPcbaFXp/Oq92+s/rtm0hZP5HoPre2OBAdf+mg O8vZjPRaf3K0aD8GmwwHpBoVbdu0lEB3WUty4YPsywOTCGmIeNz2m4FeyXLoZbh+CfnP 8kG+I58I/gjsa18tiK/bkR7rYXVfmoUPJvhwan90Nd9RIphs/4h+aAhi0R5gciyuTmx0 ktdcXbXYSvx7hrAUmQNxFOrFOOrvlCZbnWUyXci2bH4miWX4heDzQY8Sc/mGxe83VotB zKS6H2ikOpb64Do0S0gpZgK+LV5fNJpslnNa3Nxu/lLz3zAgK/VsDl+Do+Rsq7IzLBeC 91ZQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from; bh=6nX/ihuxQtb8WnAZoYpRzKTu2urzsDiYtSUskq4OB50=; b=aJ5UuIoppJ8nFLeHhZUFc7pef4gtM1qf7F7RbfgkjNeJ9F/bHqshWnX0EtRvS1E6My qw/qldMyqiecXSJALqIWifcSJqIpjdsNm9jehFLifC3Va09VzQUzjuJub36bLolceNa4 EU0VCDaQgzKZOINiF3wcBykVUWYpqRqsSKIrOOFOVJZWm+Bb43cAFG3O/pp1qWASvK+Y jWDgYgZZaZ6piuy05CM/YaQfHphygvYyoU4paHCWMkCPOQc5XuAMQXgoTLaynajmMOv0 z9hvdY3BIU4ucdJQHu/qNNtdND1+ZVIBbvILP9RRFXQh3vTWd/Ax7/W6l3dutLA8PI7x Oklg== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id t22-20020a639556000000b005406e766072si9138988pgn.576.2023.06.13.06.42.26; Tue, 13 Jun 2023 06:42:39 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S242573AbjFMNga convert rfc822-to-8bit (ORCPT <rfc822;lekhanya01809@gmail.com> + 99 others); Tue, 13 Jun 2023 09:36:30 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42414 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S242575AbjFMNgX (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Tue, 13 Jun 2023 09:36:23 -0400 Received: from fd01.gateway.ufhost.com (fd01.gateway.ufhost.com [61.152.239.71]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id DACCB1715; Tue, 13 Jun 2023 06:36:20 -0700 (PDT) Received: from EXMBX166.cuchost.com (unknown [175.102.18.54]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "EXMBX166", Issuer "EXMBX166" (not verified)) by fd01.gateway.ufhost.com (Postfix) with ESMTP id D58CB24DCF1; Tue, 13 Jun 2023 21:36:11 +0800 (CST) Received: from EXMBX061.cuchost.com (172.16.6.61) by EXMBX166.cuchost.com (172.16.6.76) with Microsoft SMTP Server (TLS) id 15.0.1497.42; Tue, 13 Jun 2023 20:58:55 +0800 Received: from localhost.localdomain (113.72.145.34) by EXMBX061.cuchost.com (172.16.6.61) with Microsoft SMTP Server (TLS) id 15.0.1497.42; Tue, 13 Jun 2023 20:58:54 +0800 From: Xingyu Wu <xingyu.wu@starfivetech.com> To: <linux-riscv@lists.infradead.org>, <devicetree@vger.kernel.org>, "Michael Turquette" <mturquette@baylibre.com>, Stephen Boyd <sboyd@kernel.org>, "Rob Herring" <robh+dt@kernel.org>, Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>, Philipp Zabel <p.zabel@pengutronix.de>, Conor Dooley <conor+dt@kernel.org>, Emil Renner Berthing <emil.renner.berthing@canonical.com> CC: Paul Walmsley <paul.walmsley@sifive.com>, Palmer Dabbelt <palmer@dabbelt.com>, Albert Ou <aou@eecs.berkeley.edu>, Hal Feng <hal.feng@starfivetech.com>, Xingyu Wu <xingyu.wu@starfivetech.com>, "William Qiu" <william.qiu@starfivetech.com>, <linux-kernel@vger.kernel.org>, <linux-clk@vger.kernel.org> Subject: [PATCH v5 2/7] dt-bindings: soc: starfive: Add StarFive syscon module Date: Tue, 13 Jun 2023 20:58:47 +0800 Message-ID: <20230613125852.211636-3-xingyu.wu@starfivetech.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20230613125852.211636-1-xingyu.wu@starfivetech.com> References: <20230613125852.211636-1-xingyu.wu@starfivetech.com> MIME-Version: 1.0 Content-Type: text/plain X-Originating-IP: [113.72.145.34] X-ClientProxiedBy: EXCAS062.cuchost.com (172.16.6.22) To EXMBX061.cuchost.com (172.16.6.61) X-YovoleRuleAgent: yovoleflag Content-Transfer-Encoding: 8BIT X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,SPF_HELO_PASS, SPF_PASS,T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1768595137778619731?= X-GMAIL-MSGID: =?utf-8?q?1768595137778619731?= |
Series |
Add PLL clocks driver and syscon for StarFive JH7110 SoC
|
|
Commit Message
Xingyu Wu
June 13, 2023, 12:58 p.m. UTC
From: William Qiu <william.qiu@starfivetech.com> Add documentation to describe StarFive System Controller Registers. Co-developed-by: Xingyu Wu <xingyu.wu@starfivetech.com> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com> Signed-off-by: William Qiu <william.qiu@starfivetech.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
Comments
On 13/06/2023 14:58, Xingyu Wu wrote: > From: William Qiu <william.qiu@starfivetech.com> > > Add documentation to describe StarFive System Controller Registers. > > Co-developed-by: Xingyu Wu <xingyu.wu@starfivetech.com> > Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com> > Signed-off-by: William Qiu <william.qiu@starfivetech.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@starfivetech.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. Best regards, Krzysztof
On 13/06/2023 14:58, Xingyu Wu wrote: > From: William Qiu <william.qiu@starfivetech.com> > > Add documentation to describe StarFive System Controller Registers. > > Co-developed-by: Xingyu Wu <xingyu.wu@starfivetech.com> > Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com> > Signed-off-by: William Qiu <william.qiu@starfivetech.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@starfivetech.com> > + > +description: | Do not need '|' unless you need to preserve formatting. > + 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" > + > +additionalProperties: false > + > +examples: > + - | > + syscon@10240000 { > + compatible = "starfive,jh7110-stg-syscon", "syscon"; > + reg = <0x10240000 0x1000>; Extend example - add clock controller, add power-domains to STG (since it has them, as you claim in the binding). Make this one example complete. Best regards, Krzysztof
On 2023/6/14 2:31, Krzysztof Kozlowski wrote: > On 13/06/2023 14:58, Xingyu Wu wrote: >> From: William Qiu <william.qiu@starfivetech.com> >> >> Add documentation to describe StarFive System Controller Registers. >> >> Co-developed-by: Xingyu Wu <xingyu.wu@starfivetech.com> >> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com> >> Signed-off-by: William Qiu <william.qiu@starfivetech.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@starfivetech.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 + - if: properties: compatible: contains: const: starfive,jh7110-aon-syscon then: - required: - - "#power-domain-cells" + properties: + "#power-domain-cells": + const: 1 -additionalProperties: false +additionalProperties: true Would it be better to show that sys-syscon only has clock-controller and aon-syscon is power controller? Best regards, Xingyu Wu
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@starfivetech.com> > >> > >> Add documentation to describe StarFive System Controller Registers. > >> > >> Co-developed-by: Xingyu Wu <xingyu.wu@starfivetech.com> > >> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com> > >> Signed-off-by: William Qiu <william.qiu@starfivetech.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@starfivetech.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.
On 2023/6/29 1:34, Conor Dooley wrote: > 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@starfivetech.com> >> >> >> >> Add documentation to describe StarFive System Controller Registers. >> >> >> >> Co-developed-by: Xingyu Wu <xingyu.wu@starfivetech.com> >> >> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com> >> >> Signed-off-by: William Qiu <william.qiu@starfivetech.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@starfivetech.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. > Yeah, following your advice, I modified the codes and there are two options: --- a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml +++ b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml @@ -41,6 +41,16 @@ required: - reg allOf: + - if: + properties: + compatible: + contains: + const: starfive,jh7110-sys-syscon + then: + required: + - clock-controller + properties: + "#power-domain-cells": false - if: properties: compatible: contains: const: starfive,jh7110-aon-syscon then: required: - "#power-domain-cells" + properties: + clock-controller: false + - if: + properties: + compatible: + contains: + const: starfive,jh7110-stg-syscon + then: + properties: + clock-controller: false + "#power-domain-cells": false additionalProperties: false Or : --- a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml +++ b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml @@ -41,6 +41,17 @@ required: - reg allOf: + - if: + properties: + compatible: + contains: + const: starfive,jh7110-sys-syscon + then: + required: + - clock-controller + else: + properties: + clock-controller: false - if: properties: compatible: contains: const: starfive,jh7110-aon-syscon then: required: - "#power-domain-cells" + else: + properties: + "#power-domain-cells": false additionalProperties: false Which one is better? Thanks. Best regards, Xingyu Wu
On Thu, Jun 29, 2023 at 02:42:39PM +0800, Xingyu Wu wrote: > On 2023/6/29 1:34, Conor Dooley wrote: > > 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@starfivetech.com> > >> >> +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. > Yeah, following your advice, I modified the codes and there are two options: > > --- a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml > +++ b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml > @@ -41,6 +41,16 @@ required: > - reg > > allOf: > + - if: > + properties: > + compatible: > + contains: > + const: starfive,jh7110-sys-syscon > + then: > + required: > + - clock-controller > + properties: > + "#power-domain-cells": false > - if: > properties: > compatible: > contains: > const: starfive,jh7110-aon-syscon > then: > required: > - "#power-domain-cells" > + properties: > + clock-controller: false > + - if: > + properties: > + compatible: > + contains: > + const: starfive,jh7110-stg-syscon > + then: > + properties: > + clock-controller: false > + "#power-domain-cells": false > > additionalProperties: false > > Or : > > --- a/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml > +++ b/Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml > @@ -41,6 +41,17 @@ required: > - reg > > allOf: > + - if: > + properties: > + compatible: > + contains: > + const: starfive,jh7110-sys-syscon > + then: > + required: > + - clock-controller > + else: > + properties: > + clock-controller: false > - if: > properties: > compatible: > contains: > const: starfive,jh7110-aon-syscon > then: > required: > - "#power-domain-cells" > + else: > + properties: > + "#power-domain-cells": false > > additionalProperties: false > > Which one is better? Thanks. This second one looks better to me, as it achieves the same thing in a simpler way. Cheers, Conor.
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@starfivetech.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" + +additionalProperties: false + +examples: + - | + syscon@10240000 { + compatible = "starfive,jh7110-stg-syscon", "syscon"; + reg = <0x10240000 0x1000>; + }; + +... diff --git a/MAINTAINERS b/MAINTAINERS index f794002a192e..ae037ba7fc47 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -20122,6 +20122,12 @@ S: Supported F: Documentation/devicetree/bindings/mmc/starfive* F: drivers/mmc/host/dw_mmc-starfive.c +STARFIVE JH7110 SYSCON +M: William Qiu <william.qiu@starfivetech.com> +M: Xingyu Wu <xingyu.wu@starfivetech.com> +S: Supported +F: Documentation/devicetree/bindings/soc/starfive/starfive,jh7110-syscon.yaml + STARFIVE JH71X0 CLOCK DRIVERS M: Emil Renner Berthing <kernel@esmil.dk> M: Hal Feng <hal.feng@starfivetech.com> @@ -20159,6 +20165,7 @@ STARFIVE SOC DRIVERS M: Conor Dooley <conor@kernel.org> S: Maintained T: git https://git.kernel.org/pub/scm/linux/kernel/git/conor/linux.git/ +F: Documentation/devicetree/bindings/soc/starfive/ F: drivers/soc/starfive/ STARFIVE TRNG DRIVER