From: Simon Horman <horms@verge.net.au>
To: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>,
Geert Uytterhoeven <geert+renesas@glider.be>,
Magnus Damm <magnus.damm@gmail.com>,
Yoshihiro Kaneko <ykaneko0929@gmail.com>,
devicetree@vger.kernel.org,
"open list:MEDIA DRIVERS FOR RENESAS - FCP"
<linux-renesas-soc@vger.kernel.org>
Subject: Re: [PATCH 1/2] dt-bindings: bus: simple-pm-bus: convert bindings to json-schema
Date: Thu, 19 Sep 2019 17:10:15 +0200 [thread overview]
Message-ID: <20190919151014.4azdfh2feg5ot6no@verge.net.au> (raw)
In-Reply-To: <CAL_JsqJHiAmH0eeUMLH1q9X6e+88EVZrmMtM33rVWCyBAszY8A@mail.gmail.com>
On Tue, Sep 17, 2019 at 07:12:16AM -0500, Rob Herring wrote:
> On Mon, Sep 16, 2019 at 10:35 AM Simon Horman
> <horms+renesas@verge.net.au> wrote:
> >
> > Convert Simple Power-Managed Bus bindings documentation to json-schema.
> >
> > As a side effect of this change only simple-pm-bus is used in example. A
> > follow-up patch will provide an example for the separately documented
> > Renesas Bus State Controller (BSC) that uses "renesas,bsc-sh73a0" and
> > "renesas,bsc" compat strings.
> >
> > Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> > ---
> > * Tested using:
> > # ARCH=arm64 make dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/bus/simple-pm-bus.yaml
> > # ARCH=arm make dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/bus/simple-pm-bus.yaml
> > ---
> > .../devicetree/bindings/bus/simple-pm-bus.txt | 44 --------------
> > .../devicetree/bindings/bus/simple-pm-bus.yaml | 68 ++++++++++++++++++++++
> > 2 files changed, 68 insertions(+), 44 deletions(-)
> > delete mode 100644 Documentation/devicetree/bindings/bus/simple-pm-bus.txt
> > create mode 100644 Documentation/devicetree/bindings/bus/simple-pm-bus.yaml
>
> > diff --git a/Documentation/devicetree/bindings/bus/simple-pm-bus.yaml b/Documentation/devicetree/bindings/bus/simple-pm-bus.yaml
> > new file mode 100644
> > index 000000000000..72a3644974e3
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/bus/simple-pm-bus.yaml
> > @@ -0,0 +1,68 @@
>
> SPDX tag?
>
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/bus/simple-pm-bus.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Simple Power-Managed Bus
> > +
> > +maintainers:
> > + - Geert Uytterhoeven <geert+renesas@glider.be>
> > +
> > +description: |
> > + A Simple Power-Managed Bus is a transparent bus that doesn't need a real
> > + driver, as it's typically initialized by the boot loader.
> > +
> > + However, its bus controller is part of a PM domain, or under the control
> > + of a functional clock. Hence, the bus controller's PM domain and/or
> > + clock must be enabled for child devices connected to the bus (either
> > + on-SoC or externally) to function.
> > +
> > + While "simple-pm-bus" follows the "simple-bus" set of properties, as
> > + specified in the Devicetree Specification, it is not an extension of
> > + "simple-bus".
> > +
> > +
> > +properties:
>
> Add $nodename in here.
For now I have gone with:
$nodename:
pattern: "^bus@[0-9a-f]+$"
But this implies updating both msm8996.dtsi and the proposed
example (see below) to use bus@ rather than agnoc@.
If this is the right way to to then perhaps it is best to use the
following until msm8996.dtsi is updated.
$nodename:
pattern: "^(bus|agnoc)@[0-9a-f]+$"
>
> > + compatible:
> > + items:
> > + - const: simple-pm-bus
>
> extra leading space.
>
> > +
> > + '#address-cells':
> > + const: 1
> > +
> > + '#size-cells':
> > + const: 1
>
> 1 or 2 should be valid...
>
> > +
> > + ranges:
> > + # Mapping between parent address and child address spaces.
> > + maxItems: 1
>
> empty or multiple ranges should be possible.
>
> > +
> > + clocks:
> > + # Functional clocks
> > + # Required if power-domains is absent, optional otherwise
> > + minItems: 1
>
> This will imply maxItems is 1 which I don't think you want.
>
> Though more than 1 starts to mean you need to know specifically what the h/w is.
I have changed this to:
clocks: true
# Functional clocks
# Required if power-domains is absent, optional otherwise
> > +
> > + power-domains:
> > + # Required if clocks is absent, optional otherwise
> > + minItems: 1
> > +
> > +required:
> > + - compatible
> > + - '#address-cells'
> > + - '#size-cells'
> > + - ranges
>
> This will capture what you commented above:
>
> oneOf:
> - required:
> - clocks
> - required:
> - power-domains
Thanks. Unfortunately dtbs_check does not seem happy
if both clocks and power-domains are present.
# cr make dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/bus/simple-pm-bus.yaml
...
/home/horms/projects/linux/renesas/arch/arm/boot/dts/r8a73a4-ape6evm.dt.yaml: bus@fec10000: {'compatible': ['renesas,bsc-r8a73a4', 'renesas,bsc', 'simple-pm-bus'], '#address-cells': [[1]], '#size-cells': [[1]], 'ranges': [[0, 0, 0, 536870912]], 'reg': [[0, 4274061312, 0, 1024]], 'clocks': [[27]], 'power-domains': [[15]], 'flash@0': {'compatible': ['cfi-flash', 'mtd-rom'], 'reg': [[0, 134217728]], 'bank-width': [[2]], 'partitions': {'compatible': ['fixed-partitions'], '#address-cells': [[1]], '#size-cells': [[1]], 'partition@0': {'label': ['uboot'], 'reg': [[0, 262144]], 'read-only': True}, 'partition@40000': {'label': ['uboot-env'], 'reg': [[262144, 262144]], 'read-only': True}, 'partition@80000': {'label': ['flash'], 'reg': [[524288, 133693440]]}}}, 'ethernet@8000000': {'compatible': ['smsc,lan9220', 'smsc,lan9115'], 'reg': [[134217728, 4096]], 'interrupt-parent': [[18]], 'interrupts': [[8, 4]], 'phy-mode': ['mii'], 'reg-io-width': [[4]], 'smsc,irq-active-high': True, 'smsc,irq-push
-pull': True, 'vdd33a-supply': [[23]], 'vddvario-supply': [[28]]}, '$nodename': ['bus@fec10000']} is valid under each of {'required': ['power-domains']}, {'required': ['clocks']}
{'$filename': '/home/horms/projects/linux/renesas/Documentation/devicetree/bindings/bus/simple-pm-bus.yaml',
'$id': 'http://devicetree.org/schemas/bus/simple-pm-bus.yaml#',
'$schema': 'http://devicetree.org/meta-schemas/core.yaml#',
'$select_validator': <jsonschema.validators.create.<locals>.Validator object at 0x7fa9f2596048>,
'oneOf': [{'required': ['clocks']}, {'required': ['power-domains']}],
'patternProperties': {'pinctrl-[0-9]+': True},
'properties': {'#address-cells': {'items': {'items': {'const': 1},
'type': 'array'},
'type': 'array'},
'#size-cells': {'items': {'items': {'enum': [1, 2]},
'type': 'array'},
'type': 'array'},
'$nodename': {'additionalItems': False,
'items': [{'pattern': '^(bus|agnoc)(@.*|-[0-9a-f])*$'}],
'maxItems': 1,
'minItems': 1,
'type': 'array'},
'clocks': True,
'compatible': {'contains': {'const': 'simple-pm-bus'}},
'phandle': True,
'pinctrl-names': True,
'power-domains': {'maxItems': 1, 'minItems': 1},
'ranges': True,
'status': True},
'required': ['compatible', '#address-cells', '#size-cells', 'ranges'],
'select': {'properties': {'compatible': {'contains': {'enum': ['simple-pm-bus']}}},
'required': ['compatible']},
'title': 'Simple Power-Managed Bus'}
> > +
> > +examples:
> > + - |
> > + #include <dt-bindings/interrupt-controller/irq.h>
> > +
> > + bsc: bus@fec10000 {
> > + compatible = "simple-pm-bus";
> > + #address-cells = <1>;
> > + #size-cells = <1>;
> > + ranges = <0 0 0x20000000>;
> > + reg = <0xfec10000 0x400>;
>
> If you have reg, then it shouldn't be "simple-pm-bus" unless you can
> function without accessing the regs.
>
> > + interrupts = <0 39 IRQ_TYPE_LEVEL_HIGH>;
>
> Not documented?
>
> > + clocks = <&zb_clk>;
> > + power-domains = <&pd_a4s>;
> > + };
As per discussion elsewhere in this thread, I have updated this to the
example in msm8996.dtsi.
agnoc@0 {
power-domains = <&gcc AGGRE0_NOC_GDSC>;
compatible = "simple-pm-bus";
#address-cells = <1>;
#size-cells = <1>;
ranges;
};
> > --
> > 2.11.0
> >
>
next prev parent reply other threads:[~2019-09-19 15:10 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-16 15:33 [PATCH 0/2] dt-bindings: bus: simple-pm-bus, renesas-bsc: convert bindings to json-schema Simon Horman
2019-09-16 15:33 ` [PATCH 1/2] dt-bindings: bus: simple-pm-bus: " Simon Horman
2019-09-17 10:43 ` Ulrich Hecht
2019-09-17 11:29 ` Simon Horman
2019-09-17 12:48 ` Rob Herring
2019-09-19 14:59 ` Simon Horman
2019-09-17 12:12 ` Rob Herring
2019-09-17 12:44 ` Geert Uytterhoeven
2019-09-19 15:10 ` Simon Horman [this message]
2019-09-19 19:33 ` Rob Herring
2019-09-23 11:53 ` Simon Horman
2019-09-23 12:40 ` Rob Herring
2019-09-24 9:22 ` Simon Horman
2019-09-16 15:33 ` [PATCH 2/2] dt-bindings: bus: renesas-bsc: " Simon Horman
2019-09-17 10:43 ` Ulrich Hecht
2019-09-17 11:08 ` Simon Horman
2019-09-17 12:41 ` Rob Herring
2019-09-17 11:09 ` Simon Horman
2019-09-17 11:12 ` Simon Horman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190919151014.4azdfh2feg5ot6no@verge.net.au \
--to=horms@verge.net.au \
--cc=devicetree@vger.kernel.org \
--cc=geert+renesas@glider.be \
--cc=linux-renesas-soc@vger.kernel.org \
--cc=magnus.damm@gmail.com \
--cc=mark.rutland@arm.com \
--cc=robh+dt@kernel.org \
--cc=ykaneko0929@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).