Thread (25 messages) 25 messages, 6 authors, 2017-06-12

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

From: Rob Herring <robh+dt@kernel.org>
Date: 2017-06-12 21:13:49

On Fri, Jun 9, 2017 at 1:12 PM, Matt Sealey [off-list ref] 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 [off-list ref] wrote:
quoted
+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 [off-list ref]
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help