Re: [RFC PATCH 1/8] Documentation: add DT binding for ARM System Control and Management Interface(SCMI) protocol

[Rob Herring <robh+dt@kernel.org>]

On Fri, Jun 9, 2017 at 1:12 PM, Matt Sealey <neko@bakuhatsu.net> wrote:
> Hullo all,
>
> This is a long one.. apologies for odd linefeeds and so on.
>
> On Wed, Jun 7, 2017 at 11:10 AM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>
>> +Clock/Performance bindings for the clocks/OPPs based on SCMI Message
>> Protocol
>> +------------------------------------------------------------
>> +
>> +This binding uses the common clock binding[1].
>> +
>> +Required properties:
>> +- compatible : shall be "arm,scmi-clocks" or "arm,scmi-perf-domains"
>
> After a little thought, there are a couple objections to be made here.
> Firstly,
> the SCMI protocol families are  discoverable - you only really need to know
> that
> it is usable (and where to use it, mboxes etc.) - whether it supports the
> clock
> management, performance domains, power domains et al. protocols is a
> function
> of querying the base protocol for a list.

Unfortunately, being discoverable doesn't really help. Presumably we can discover that clock mgmt is supported, but not clock connections (e.g. the clock for uart1 is ?). Even if discovering that connection was possible, I don't think we want yet another way to do that on top of platform_data, DT, and ACPI. Firmware could populate the DT or ACPI tables using the data from SCMI though.

> These protocols are identified by a value, several of which are
> standardized,
> some being vendor extension numbers. All protocols must be able to be
> queried
> for information.
>
> As such, defining compatible properties for each protocol is treading that
> fine
> line of tying device trees to particular driver subsystems and giving
> operating
> systems an ability to ignore any discovery procedure. While I can't make a
> case
> for clock management (which should obviously conform with a particular clock
> management definition in DT, as already defined), there is plenty of past
> evidence
> of bindings for particular devices being mis-used or used in non-intended
> ways
> (regulators as reset GPIOs is the one that immediately came to mind) in lieu
> of a
> more fleshed out way of defining a particular class of device for a binding.
> The
> same would be true of tying a 'performance domain' to the concept of clock
> management.
>
> From the point of view of being able to specify things against a particular
> binding
> (whatever that might be), one could imagine something that mapped protocols
> to
> those bindings without introducing compatible names. SCMI ids would be
> verbatim,
> and per-protocol. Things like clock-indices are therefore not relevant and
> defining which indices go with which protocol at the SCMI level isn't needed
> anymore. It is really up to the protocol how many cells it would need to
> define
> it's protocol behavior but for the purpose of some standardization, we could
> imagine a binding that defined protocols as such:
>
> scmi: arm_scmi {
>         compatible = "arm,scmi,1.0";
>         mboxes = <y>;
>         shmem = <x>;
>         protocols {
>               scmi_clocks: protocol@0x14 {
>                         #whatever-cells = 3;
>                };
>                foo_smic: protocol@0x89 {
>                         #foo-cells = 4;
>                         #bar-cells = 5;
>                 };
>         };
> };
>
> uart: myuart@80000000 {
>         compatible = "arm,pl011";
>         clocks = <&foo_smic 3>;
> };

Once you just have nodes with just cell sizes, this can be simplified to:

scmi: arm_scmi {
         compatible = "arm,scmi,1.0";
         #clock-cells = <1>;
         ...
};

uart: myuart@80000000 {
        compatible = "arm,pl011";
        clocks = <&smic 3>;
};

Usually, having child nodes and compatibles in cases like this is simply because people want to have platform devices created for them.

>
> If you manage to get a device tree that specifies a clock but there is no
> protocol 0x89 then you're just as hosed as if you specified an
> arm,scmi-clocks
> node when the protocol was not supported by SCMI itself, so we don't gain
> any
> new dangers, but we do gain the ability to instantiate SCMI, discover
> protocols,
> and then load drivers against those protocols, without duplicating the
> discovery
> process with a hardcoded tree. Device trees, from my point of view, are a
> contract
> between the SoC & board designer and the OS (helped along by firmware,
> hopefully).
> They shouldn't be dictating the driver behavior to be applied at this kind
> of level.
>
> Device trees need to be rock solid - agile development is fine but as soon
> as you ship,
> changing the device tree means cutting off support for existing software, or
> only
> working with augmented features on new software and severely reduced
> functionality
> on old software. That can be as simple as not being able to go to Turbo
> mode, or as
> bad as an inability to apply thermal limits and burning someone's board.  If
> we define
> a specific binding of a specific protocol to a specific way of interacting
> with that
> device which is a purely software construct (like treating performance
> domain as an
> bstract clock interface with a particular number of cells or clock-like
> behavior),
> then you lock down protocols to *existing* OS subsystems. That means
> maintaining that
> ubsystem and device tree specification to retain behavior, which is a lot of
> maintaining.
>
> It shouldn't be necessary to actually define which protocols exist and what
> number of
> cells they use in the device tree binding, because this is actually
> documented
> elsewhere. Just as we do not create compatible properties for new devices
> which work
> the same as old ones, or redefine features which would otherwise be
> ascertained
> by some kind of discovery (PCI devices, USB devices, but even as simple just
> reading out an ID register from a device to determine if it has a particular
> feature), we shouldn't do this to lock down a further discovery model for
> activity types or programmers' models under those protocols.
>
> Here's where I fall down: with a variable number of cells per protocol
> required
> (potentially) and no method to be able to assign a particular protocol's
> device
> or functionality to something else, discovery of what maps where has to be
> done
> as well. There's little of that in SCMI, that is to say it wouldn't be
> possible
> to infer that clock_id 20 in protocol 0x14 is the clock that goes with
> cpu@0. It is
> also, per the SCMI spec, a firmware table responsibility to define any
> dependencies
> on other parts of the protocol (for instance, clock trees). The best I can
> think
> of right now is that this would just have to be done on a global SCMI level:
>
> scmi: arm_scmi {
>         compatible = "arm,scmi,1.0";
>         mboxes = <y>;
>         shmem = <x>;
>         disable-protocols = <0x87, 0x88>; // these are buggy, don't load
> drivers even if they're implemented
>
>         #whatever-cells = 3;
>         required-whatever-prop;
>
>         #foo-cells = 4;
>
>         #bar-cells = 5;
>         optional-bar-prop = <5, 10, 15>;
>
>         #clock-cells = 10;
> };
>
> #define SCMI_PROTO_CLOCKMGMT 0x14
> #define SCMI_PROTO_VENDOR_CLOCKS 0x89
> uart: myuart {
>         compatible = "arm,pl011";
>         clocks = <&scmi SCMI_PROTO_CLOCKMGMT 3 0 0 0 0 0 0 0 0>, <&scmi
> SCMIU_PROTO_VENDOR_CLOCKS 3 4 7 99 0 0 0x9000 3>;
> };
>
> .. it is up to the driver to figure out what exactly this does in real
> life, without having to lock it to the clock et al. binding, but at least
> all
> drivers using protocols must be able to parse the number of cells defined.
> If
> a protocol only needs 3 cells, but another needs 10 cells for the same
> thing,
> then both protocols will by definition be defined as 10-cell groups. It
> implies
> hat any device that can be controlled or affected by SCMI can list the
> devices,
> and the protocol driver will be required to parse the remaining cells and
> ignore
> them. As long as the device tree can cover all cases in it's #foo-cells or
> other
> binding properties, it would be most flexible here.
>
> I don't like the lockdown of having to cover every binding that gets used
> whether it's truly for that device type or not, but it would be the most
> flexible
> within the current framework, without defeating discovery.
>
> We would do well to come up with a way of defining abstract interfaces to
> firmware or other processors that don't rely on there being a fixed binding
> that dictates what kind of device it is, where there isn't a way of defining
> that device type in a generic manner. There's no way of doing this right
> now and letting the driver in the OS know what's necessary - well, there is,
> things like RTAS support on CHRP got away with this in the old days, and
> PSCI
> does something very similar (which covers quite a lot of CPU management and
> also system domain activity), so it's not like we've come up against the
> issue
> before. But it's not been resolved when a device node would need to refer
> back
> to those abstraction interface nodes where there's not a reasonable way of
> binding the definition of the reference.
>
> RTAS had a property in every device node that depended on it and would be
> affected by it, "used-by-rtas". Perhaps we could augment devices with an
> "managed-by" property likewise to point to the protocol node.
> #protocol-cells
> might be a nice way of defining cell sizes generically (where protocol would
> be the name of the protocol - #scmi-cells perhaps - and the format of
> #scmi-cells would need to include the protocol number). Wrapping that up in
> a generic binding means each protocol then gets control of it's own world,
> and each device that is affected by that protocol would be able to realize
> this.
>
> If anyone comes up with a particular PSCI-like protocol for anything here
> that kind of adaptable binding for device/platform abstraction frameworks
> with non-discrete methods of working (i.e. it might not be able to be
> defined
> as "just a clock" or "just a power domain" or "just a thermal framework" or
> any combination without reducing functionality that would otherwise come
> from
> some built-in discovery system) would come in very handy.
>
> Just thoughts right now, though. I definitely don't have the whole story
> down,
> but it is definitely something to think about.
>
> Ta,
> Matt Sealey <neko@bakuhatsu.net>