Re: [PATCH RFC v5 01/10] dt-bindings: devfreq: Add bindings for generic imx buses

From: Rob Herring <robh@kernel.org>
To: Leonard Crestez <leonard.crestez@nxp.com>
Cc: "Georgi Djakov" <georgi.djakov@linaro.org>,
	"Chanwoo Choi" <cw00.choi@samsung.com>,
	"Artur Świgoń" <a.swigon@partner.samsung.com>,
	"Alexandre Bailon" <abailon@baylibre.com>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	"Jacky Bai" <ping.bai@nxp.com>,
	"Anson Huang" <Anson.Huang@nxp.com>,
	"Abel Vesa" <abel.vesa@nxp.com>,
	"Krzysztof Kozlowski" <krzk@kernel.org>,
	"MyungJoo Ham" <myungjoo.ham@samsung.com>,
	"Kyungmin Park" <kyungmin.park@samsung.com>,
	"Saravana Kannan" <saravanak@google.com>,
	"Mark Rutland" <mark.rutland@arm.com>,
	"Viresh Kumar" <viresh.kumar@linaro.org>,
	"Shawn Guo" <shawnguo@kernel.org>,
	"Dong Aisheng" <aisheng.dong@nxp.com>,
	"Fabio Estevam" <fabio.estevam@nxp.com>,
	"Stephen Boyd" <sboyd@kernel.org>,
	"Michael Turquette" <mturquette@baylibre.com>,
	"Matthias Kaehlcke" <mka@chromium.org>,
	"Angus Ainslie" <angus@akkea.ca>,
	"Martin Kepplinger" <martink@posteo.de>,
	linux-pm@vger.kernel.org, kernel@pengutronix.de,
	linux-imx@nxp.com, devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH RFC v5 01/10] dt-bindings: devfreq: Add bindings for generic imx buses
Date: Mon, 4 Nov 2019 16:49:46 -0600	[thread overview]
Message-ID: <20191104224946.GB17515@bogus> (raw)
In-Reply-To: <0e4118456f8eb67e1ba8a7c23127fc3def58547b.1572562150.git.leonard.crestez@nxp.com>

On Fri, Nov 01, 2019 at 12:52:00AM +0200, Leonard Crestez wrote:
> Add initial dt bindings for the interconnects inside i.MX chips.
> Multiple external IPs are involved but SOC integration means the
> software controllable interfaces are very similar.
> 
> Single node also acts as interconnect provider if #interconnect-cells is
> present.
> 
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> Acked-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> ---
>  .../devicetree/bindings/devfreq/imx.yaml      | 83 +++++++++++++++++++

bindings/interconnect/

>  1 file changed, 83 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/devfreq/imx.yaml
> 
> diff --git a/Documentation/devicetree/bindings/devfreq/imx.yaml b/Documentation/devicetree/bindings/devfreq/imx.yaml
> new file mode 100644
> index 000000000000..bfc825407764
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/devfreq/imx.yaml
> @@ -0,0 +1,83 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/devfreq/imx.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Generic i.MX bus frequency device

i.MX8 specific?

> +
> +maintainers:
> +  - Leonard Crestez <leonard.crestez@nxp.com>
> +
> +description: |
> +  The i.MX SoC family has multiple buses for which clock frequency (and
> +  sometimes voltage) can be adjusted.
> +
> +  Some of those buses expose register areas mentioned in the memory maps as GPV
> +  ("Global Programmers View") but not all. Access to this area might be denied
> +  for normal (non-secure) world.
> +
> +  The buses are based on externally licensed IPs such as ARM NIC-301 and
> +  Arteris FlexNOC but DT bindings are specific to the integration of these bus
> +  interconnect IPs into imx SOCs.
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - items:
> +        - enum:
> +          - fsl,imx8mn-nic
> +          - fsl,imx8mm-nic
> +          - fsl,imx8mq-nic
> +        - const: fsl,imx8m-nic
> +      - items:
> +        - enum:
> +          - fsl,imx8mn-noc
> +          - fsl,imx8mm-noc
> +          - fsl,imx8mq-noc
> +        - const: fsl,imx8m-noc
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  operating-points-v2: true
> +
> +  devfreq:
> +    description: |
> +      Phandle to another devfreq device to match OPPs with by using the
> +      passive governor.
> +    $ref: "/schemas/types.yaml#/definitions/phandle"
> +
> +  '#interconnect-cells':
> +    description: |
> +      If specified then also act as an interconnect provider. Should only be
> +      set once per soc on main noc.
> +    const: 1
> +
> +  interconnect-node-id:

Looks like common property, but it's not...

Generally, we don't do indexes or instance ids. So it needs a better 
explanation or drop this. The driver side looks like an odd marriage 
between interconnect and devfreq drivers that needs better integration, 
but I'm not all that familar with either.

> +    description: |
> +      i.MX chips have multiple scalable buses based on the same IP, this is
> +      used to distinguish between. Uses same identifier namespace as consumer

It's not names, so number space? Just guessing because there's no type 
nor example.

> +      "interconnects" property, for example one of the values in
> +      "include/dt-bindings/interconnect/imx8mm.h"
> +
> +    const: 1
> +
> +required:
> +  - compatible
> +  - clocks
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/imx8mm-clock.h>
> +    noc: noc@32700000 {
> +            compatible = "fsl,imx8mm-noc", "fsl,imx8m-noc";
> +            reg = <0x32700000 0x100000>;
> +            clocks = <&clk IMX8MM_CLK_NOC>;
> +            operating-points-v2 = <&noc_opp_table>;
> +    };
> -- 
> 2.17.1
> 