Re: [PATCH 1/2] Documentation: devicetree: arm: cpus/cpu nodes bindings updates

Re: [PATCH 1/2] Documentation: devicetree: arm: cpus/cpu nodes bindings updates

[Grant Likely]

On Thu, 15 Aug 2013 10:42:22 +0100, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote:
> +=====================================
> +cpus and cpu node bindings definition
> +=====================================
> +
> +The ARM architecture, in accordance with the ePAPR, requires the cpus and cpu
> +nodes to be present and contain the properties described below.
> +
> +- cpus node
> +
> +	Description: Container of cpu nodes
> +
> +	The node name must be "cpus".
> +
> +	A cpus node must define the following properties:
> +
> +	- #address-cells
> +		Usage: required
> +		Value type: <u32>
> +
> +		Definition depends on ARM architecture version and
> +		configuration:
> +
> +			# On uniprocessor ARM architectures previous to v7
> +			  value must be 0, since they have no register
> +			  providing CPU identification.

Just clued into this point from the other thread; Don't do this.
Uniprocessor should follow the same rules as 32bit or 64bit for the
value of #address-cells, and cpu nodes should have a reg property, even
if it is merely set to '0'.

g.

Re: [PATCH 2/2] Documentation: DT: arm: define CPU topology bindings

[Lorenzo Pieralisi]

[replying to self]

Bindings defined in this patch have been hanging in the balance for
quite a while, I would ask DT maintainers (and everyone else) please to have
a look and provide an ACK/NACK as they deem fit, thank you very much.

Comments welcome.

Thanks,
Lorenzo

On Thu, Aug 15, 2013 at 10:42:23AM +0100, Lorenzo Pieralisi wrote:
> The advent of multi-cluster ARM systems requires a mechanism to describe
> how in hierarchical terms CPUs are connected in ARM SoCs so that the kernel
> can initialize and map resources like IRQs and memory space to specific
> group(s) of CPUs.
> 
> The CPU topology is made up of multiple hierarchy levels whose bottom
> layers (aka leaf nodes in device tree syntax) contain links to the HW
> CPUs in the system.
> 
> The topology bindings are generic for both 32-bit and 64-bit systems and
> lay the groundwork on top of which affinity schemes can be built.
> 
> This patch provides the documentation in the kernel required to define the
> device tree bindings describing the CPU topology for ARM 32-bit and 64-bit
> systems.
> 
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/arm/topology.txt | 492 +++++++++++++++++++++
>  1 file changed, 492 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/arm/topology.txt
> 
> diff --git a/Documentation/devicetree/bindings/arm/topology.txt b/Documentation/devicetree/bindings/arm/topology.txt
> new file mode 100644
> index 0000000..73fe7dd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/topology.txt
> @@ -0,0 +1,492 @@
> +===========================================
> +ARM topology binding description
> +===========================================
> +
> +===========================================
> +1 - Introduction
> +===========================================
> +
> +In an ARM system, the hierarchy of CPUs is defined through three entities that
> +are used to describe the layout of physical CPUs in the system:
> +
> +- cluster
> +- core
> +- thread
> +
> +The cpu nodes (bindings defined in [1]) represent the devices that
> +correspond to physical CPUs and are to be mapped to the hierarchy levels.
> +
> +The bottom hierarchy level sits at core or thread level depending on whether
> +symmetric multi-threading (SMT) is supported or not.
> +
> +For instance in a system where CPUs support SMT, "cpu" nodes represent all
> +threads existing in the system and map to the hierarchy level "thread" above.
> +In systems where SMT is not supported "cpu" nodes represent all cores present
> +in the system and map to the hierarchy level "core" above.
> +
> +ARM topology bindings allow one to associate cpu nodes with hierarchical groups
> +corresponding to the system hierarchy; syntactically they are defined as device
> +tree nodes.
> +
> +The remainder of this document provides the topology bindings for ARM, based
> +on the ePAPR standard, available from:
> +
> +http://www.power.org/documentation/epapr-version-1-1/
> +
> +If not stated otherwise, whenever a reference to a cpu node phandle is made its
> +value must point to a cpu node compliant with the cpu node bindings as
> +documented in [1].
> +A topology description containing phandles to cpu nodes that are not compliant
> +with bindings standardized in [1] is therefore considered invalid.
> +
> +===========================================
> +2 - cpu-map node
> +===========================================
> +
> +The ARM CPU topology is defined within the cpu-map node, which is a direct
> +child of the cpus node and provides a container where the actual topology
> +nodes are listed.
> +
> +- cpu-map node
> +
> +       Usage: Optional - Provide ARM CPU topology to the OS
> +
> +       Description: The cpu-map node is just a container node where its
> +                    subnodes describe the CPU topology
> +
> +       Node name must be "cpu-map".
> +
> +       The cpu-map node's parent node must be the cpus node.
> +
> +       The cpu-map node's child nodes can be:
> +
> +       - one or more cluster nodes
> +
> +       Any other configuration is considered invalid.
> +
> +The cpu-map node can only contain three types of child nodes:
> +
> +- cluster node
> +- core node
> +- thread node
> +
> +whose bindings are described in paragraph 3.
> +
> +The nodes describing the CPU topology (cluster/core/thread) can only be
> +defined within the cpu-map node.
> +Any other configuration is consider invalid and therefore must be ignored.
> +
> +===========================================
> +2.1 - cpu-map child nodes naming convention
> +===========================================
> +
> +cpu-map child nodes must follow a naming convention where the node name
> +must be "clusterN", "coreN", "threadN" depending on the node type (ie
> +cluster/core/thread) (where N = {0, 1, ...} is the node number; nodes which
> +are siblings within a single common parent node must be given a unique and
> +sequential N value, starting from 0).
> +cpu-map child nodes which do not share a common parent node can have the same
> +name (ie same number N as other cpu-map child nodes at different device tree
> +levels) since name uniqueness will be guaranteed by the device tree hierarchy.
> +
> +===========================================
> +3 - cluster/core/thread node bindings
> +===========================================
> +
> +Bindings for cluster/cpu/thread nodes are defined as follows:
> +
> +- cluster node
> +
> +        Description: must be declared within a cpu-map node, one node
> +                     per cluster. A system can contain several layers of
> +                     clustering and cluster nodes can be contained in parent
> +                     cluster nodes.
> +
> +       The cluster node name must be "clusterN" as described in 2.1 above.
> +       A cluster node can not be a leaf node.
> +
> +       A cluster node's child nodes must be:
> +
> +       - one or more cluster nodes; or
> +       - one or more core nodes
> +
> +       Any other configuration is considered invalid.
> +
> +- core node
> +
> +       Description: must be declared in a cluster node, one node per core in
> +                    the cluster. If the system does not support SMT, core
> +                    nodes are leaf nodes, otherwise they become containers of
> +                    thread nodes.
> +
> +       The core node name must be "coreN" as described in 2.1 above.
> +
> +       A core node must be a leaf node if SMT is not supported.
> +
> +       Properties for core nodes that are leaf nodes:
> +
> +       - cpu
> +               Usage: required
> +               Value type: <phandle>
> +               Definition: a phandle to the cpu node that corresponds to the
> +                           core node.
> +
> +       If a core node is not a leaf node (CPUs supporting SMT) a core node's
> +       child nodes can be:
> +
> +       - one or more thread nodes
> +
> +       Any other configuration is considered invalid.
> +
> +- thread node
> +
> +       Description: must be declared in a core node, one node per thread
> +                    in the core if the system supports SMT. Thread nodes are
> +                    always leaf nodes in the device tree.
> +
> +       The thread node name must be "threadN" as described in 2.1 above.
> +
> +       A thread node must be a leaf node.
> +
> +       A thread node must contain the following property:
> +
> +       - cpu
> +               Usage: required
> +               Value type: <phandle>
> +               Definition: a phandle to the cpu node that corresponds to
> +                           the thread node.
> +
> +===========================================
> +4 - Example dts
> +===========================================
> +
> +Example 1 (ARM 64-bit, 16-cpu system, two clusters of clusters):
> +
> +cpus {
> +       #size-cells = <0>;
> +       #address-cells = <2>;
> +
> +       cpu-map {
> +               cluster0 {
> +                       cluster0 {
> +                               core0 {
> +                                       thread0 {
> +                                               cpu = <&CPU0>;
> +                                       };
> +                                       thread1 {
> +                                               cpu = <&CPU1>;
> +                                       };
> +                               };
> +
> +                               core1 {
> +                                       thread0 {
> +                                               cpu = <&CPU2>;
> +                                       };
> +                                       thread1 {
> +                                               cpu = <&CPU3>;
> +                                       };
> +                               };
> +                       };
> +
> +                       cluster1 {
> +                               core0 {
> +                                       thread0 {
> +                                               cpu = <&CPU4>;
> +                                       };
> +                                       thread1 {
> +                                               cpu = <&CPU5>;
> +                                       };
> +                               };
> +
> +                               core1 {
> +                                       thread0 {
> +                                               cpu = <&CPU6>;
> +                                       };
> +                                       thread1 {
> +                                               cpu = <&CPU7>;
> +                                       };
> +                               };
> +                       };
> +               };
> +
> +               cluster1 {
> +                       cluster0 {
> +                               core0 {
> +                                       thread0 {
> +                                               cpu = <&CPU8>;
> +                                       };
> +                                       thread1 {
> +                                               cpu = <&CPU9>;
> +                                       };
> +                               };
> +                               core1 {
> +                                       thread0 {
> +                                               cpu = <&CPU10>;
> +                                       };
> +                                       thread1 {
> +                                               cpu = <&CPU11>;
> +                                       };
> +                               };
> +                       };
> +
> +                       cluster1 {
> +                               core0 {
> +                                       thread0 {
> +                                               cpu = <&CPU12>;
> +                                       };
> +                                       thread1 {
> +                                               cpu = <&CPU13>;
> +                                       };
> +                               };
> +                               core1 {
> +                                       thread0 {
> +                                               cpu = <&CPU14>;
> +                                       };
> +                                       thread1 {
> +                                               cpu = <&CPU15>;
> +                                       };
> +                               };
> +                       };
> +               };
> +       };
> +
> +       CPU0: cpu@0 {
> +               device_type = "cpu";
> +               compatible = "arm,cortex-a57";
> +               reg = <0x0 0x0>;
> +               enable-method = "spin-table";
> +               cpu-release-addr = <0 0x20000000>;
> +       };
> +
> +       CPU1: cpu@1 {
> +               device_type = "cpu";
> +               compatible = "arm,cortex-a57";
> +               reg = <0x0 0x1>;
> +               enable-method = "spin-table";
> +               cpu-release-addr = <0 0x20000000>;
> +       };
> +
> +       CPU2: cpu@100 {
> +               device_type = "cpu";
> +               compatible = "arm,cortex-a57";
> +               reg = <0x0 0x100>;
> +               enable-method = "spin-table";
> +               cpu-release-addr = <0 0x20000000>;
> +       };
> +
> +       CPU3: cpu@101 {
> +               device_type = "cpu";
> +               compatible = "arm,cortex-a57";
> +               reg = <0x0 0x101>;
> +               enable-method = "spin-table";
> +               cpu-release-addr = <0 0x20000000>;
> +       };
> +
> +       CPU4: cpu@10000 {
> +               device_type = "cpu";
> +               compatible = "arm,cortex-a57";
> +               reg = <0x0 0x10000>;
> +               enable-method = "spin-table";
> +               cpu-release-addr = <0 0x20000000>;
> +       };
> +
> +       CPU5: cpu@10001 {
> +               device_type = "cpu";
> +               compatible = "arm,cortex-a57";
> +               reg = <0x0 0x10001>;
> +               enable-method = "spin-table";
> +               cpu-release-addr = <0 0x20000000>;
> +       };
> +
> +       CPU6: cpu@10100 {
> +               device_type = "cpu";
> +               compatible = "arm,cortex-a57";
> +               reg = <0x0 0x10100>;
> +               enable-method = "spin-table";
> +               cpu-release-addr = <0 0x20000000>;
> +       };
> +
> +       CPU7: cpu@10101 {
> +               device_type = "cpu";
> +               compatible = "arm,cortex-a57";
> +               reg = <0x0 0x10101>;
> +               enable-method = "spin-table";
> +               cpu-release-addr = <0 0x20000000>;
> +       };
> +
> +       CPU8: cpu@100000000 {
> +               device_type = "cpu";
> +               compatible = "arm,cortex-a57";
> +               reg = <0x1 0x0>;
> +               enable-method = "spin-table";
> +               cpu-release-addr = <0 0x20000000>;
> +       };
> +
> +       CPU9: cpu@100000001 {
> +               device_type = "cpu";
> +               compatible = "arm,cortex-a57";
> +               reg = <0x1 0x1>;
> +               enable-method = "spin-table";
> +               cpu-release-addr = <0 0x20000000>;
> +       };
> +
> +       CPU10: cpu@100000100 {
> +               device_type = "cpu";
> +               compatible = "arm,cortex-a57";
> +               reg = <0x1 0x100>;
> +               enable-method = "spin-table";
> +               cpu-release-addr = <0 0x20000000>;
> +       };
> +
> +       CPU11: cpu@100000101 {
> +               device_type = "cpu";
> +               compatible = "arm,cortex-a57";
> +               reg = <0x1 0x101>;
> +               enable-method = "spin-table";
> +               cpu-release-addr = <0 0x20000000>;
> +       };
> +
> +       CPU12: cpu@100010000 {
> +               device_type = "cpu";
> +               compatible = "arm,cortex-a57";
> +               reg = <0x1 0x10000>;
> +               enable-method = "spin-table";
> +               cpu-release-addr = <0 0x20000000>;
> +       };
> +
> +       CPU13: cpu@100010001 {
> +               device_type = "cpu";
> +               compatible = "arm,cortex-a57";
> +               reg = <0x1 0x10001>;
> +               enable-method = "spin-table";
> +               cpu-release-addr = <0 0x20000000>;
> +       };
> +
> +       CPU14: cpu@100010100 {
> +               device_type = "cpu";
> +               compatible = "arm,cortex-a57";
> +               reg = <0x1 0x10100>;
> +               enable-method = "spin-table";
> +               cpu-release-addr = <0 0x20000000>;
> +       };
> +
> +       CPU15: cpu@100010101 {
> +               device_type = "cpu";
> +               compatible = "arm,cortex-a57";
> +               reg = <0x1 0x10101>;
> +               enable-method = "spin-table";
> +               cpu-release-addr = <0 0x20000000>;
> +       };
> +};
> +
> +Example 2 (ARM 32-bit, dual-cluster, 8-cpu system, no SMT):
> +
> +cpus {
> +       #size-cells = <0>;
> +       #address-cells = <1>;
> +
> +       cpu-map {
> +               cluster0 {
> +                       core0 {
> +                               cpu = <&CPU0>;
> +                       };
> +                       core1 {
> +                               cpu = <&CPU1>;
> +                       };
> +                       core2 {
> +                               cpu = <&CPU2>;
> +                       };
> +                       core3 {
> +                               cpu = <&CPU3>;
> +                       };
> +               };
> +
> +               cluster1 {
> +                       core0 {
> +                               cpu = <&CPU4>;
> +                       };
> +                       core1 {
> +                               cpu = <&CPU5>;
> +                       };
> +                       core2 {
> +                               cpu = <&CPU6>;
> +                       };
> +                       core3 {
> +                               cpu = <&CPU7>;
> +                       };
> +               };
> +       };
> +
> +       CPU0: cpu@0 {
> +               device_type = "cpu";
> +               compatible = "arm,cortex-a15";
> +               reg = <0x0>;
> +       };
> +
> +       CPU1: cpu@1 {
> +               device_type = "cpu";
> +               compatible = "arm,cortex-a15";
> +               reg = <0x1>;
> +       };
> +
> +       CPU2: cpu@2 {
> +               device_type = "cpu";
> +               compatible = "arm,cortex-a15";
> +               reg = <0x2>;
> +       };
> +
> +       CPU3: cpu@3 {
> +               device_type = "cpu";
> +               compatible = "arm,cortex-a15";
> +               reg = <0x3>;
> +       };
> +
> +       CPU4: cpu@100 {
> +               device_type = "cpu";
> +               compatible = "arm,cortex-a7";
> +               reg = <0x100>;
> +       };
> +
> +       CPU5: cpu@101 {
> +               device_type = "cpu";
> +               compatible = "arm,cortex-a7";
> +               reg = <0x101>;
> +       };
> +
> +       CPU6: cpu@102 {
> +               device_type = "cpu";
> +               compatible = "arm,cortex-a7";
> +               reg = <0x102>;
> +       };
> +
> +       CPU7: cpu@103 {
> +               device_type = "cpu";
> +               compatible = "arm,cortex-a7";
> +               reg = <0x103>;
> +       };
> +};
> +
> +Example 3 (ARM 32-bit, cortex-a8 single core):
> +
> +cpus {
> +       #size-cells = <0>;
> +       #address-cells = <1>;
> +
> +       cpu-map {
> +               cluster0 {
> +                       core0 {
> +                               cpu = <&CPU0>;
> +                       };
> +               };
> +       };
> +
> +       CPU0: cpu@0 {
> +               device_type = "cpu";
> +               compatible = "arm,cortex-a8";
> +               reg = <0x0>;
> +       };
> +};
> +
> +===============================================================================
> +[1] ARM Linux kernel documentation
> +    Documentation/devicetree/bindings/arm/cpus.txt
> --
> 1.8.2.2
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] Documentation: devicetree: arm: cpus/cpu nodes bindings updates

[Lorenzo Pieralisi]

Hi Rob, all,

On Thu, Aug 15, 2013 at 04:22:38PM +0100, Lorenzo Pieralisi wrote:
> [adding Andrew, Gregory and Thomas to check the Marvell compatible names]
> 
> On Thu, Aug 15, 2013 at 03:32:05PM +0100, Rob Herring wrote:
> > On 08/15/2013 04:42 AM, Lorenzo Pieralisi wrote:
> > > In order to extend the current cpu nodes bindings to newer CPUs
> > > inclusive of AArch64 and to update support for older ARM CPUs this
> > > patch updates device tree documentation for the cpu nodes bindings.
> > > 
> > > Main changes:
> > >     - adds 64-bit bindings
> > >     - define usage of #address-cells
> > >     - defines behaviour on pre and post v7 uniprocessor systems
> > >     - adds ARM 11MPcore specific reg property definition
> > > 
> > > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>
> > > ---
> > >  Documentation/devicetree/bindings/arm/cpus.txt | 424 ++++++++++++++++++++++---
> > >  1 file changed, 377 insertions(+), 47 deletions(-)
> > > 
> > 
> > The binding looks mostly fine to me.

These bindings have still not received an ACK, and need review by
Marvell guys in copy for the new compatible strings below.

Most importantly, we need to make a decision on the pre v7 uniprocessor
systems, where MPIDR/CPUID are non-existent and the reg property is a
pure SW enumeration. Current bindings (ie this patch) define

#address-cells = <0>;

for those processors (and there are a number of dts in the kernel with that
set-up); Grant and Benjamin had a strong feeling against this choice, I
have to make a decision on how to proceed, please let me know.

Comments welcome.

Lorenzo

> > 
> > [snip]
> > 
> > > +			    "faraday,fa526"
> > > +			    "intel,sa110"
> > > +			    "intel,sa1100"
> > > +			    "marvell,feroceon"
> > > +			    "marvell,mohawk"
> > > +			    "marvell,pj4"
> > > +			    "marvell,sheeva-v7"
> > > +			    "marvell,xsc3"
> > > +			    "marvell,xscale"
> > 
> > Better make sure the Marvell folks are happy with these. We don't need
> > another rename here. I'm too annoyed with all the renames to pay attention.
> 
> Ok, I will verify that, I copied maintainers in.
> 
> On a side note I have just noticed that last cycle some dts were merged in the
> kernel with cpu nodes that are not compliant (not picking on anyone,
> eg am4372.dtsi).
> Please prevent this from happening from now onwards, really please.
> 
> > > +
> > > +Example 4 (ARM Cortex-A57 64-bit system running OS in AArch64):
> > > +
> > 
> > Going back to my comments that the dtb can't be dependent on the OS,
> > these 2 examples don't make sense.
> 
> Gah, my bad sorry, I missed the examples while removing dependency on the OS
> from the bindings.
> 
> > > +
> > > +Example 5 (ARM Cortex-A57 64-bit system running OS in AArch32):
> > 
> > This example should be removed.
> 
> Yes, consider it done, see above.
> 
> Thanks,
> Lorenzo
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] Documentation: devicetree: arm: cpus/cpu nodes bindings updates

[Rob Herring]

On 09/13/2013 11:57 AM, Lorenzo Pieralisi wrote:
> Hi Rob, all,
> 
> On Thu, Aug 15, 2013 at 04:22:38PM +0100, Lorenzo Pieralisi wrote:
>> [adding Andrew, Gregory and Thomas to check the Marvell compatible names]
>>
>> On Thu, Aug 15, 2013 at 03:32:05PM +0100, Rob Herring wrote:
>>> On 08/15/2013 04:42 AM, Lorenzo Pieralisi wrote:
>>>> In order to extend the current cpu nodes bindings to newer CPUs
>>>> inclusive of AArch64 and to update support for older ARM CPUs this
>>>> patch updates device tree documentation for the cpu nodes bindings.
>>>>
>>>> Main changes:
>>>>     - adds 64-bit bindings
>>>>     - define usage of #address-cells
>>>>     - defines behaviour on pre and post v7 uniprocessor systems
>>>>     - adds ARM 11MPcore specific reg property definition
>>>>
>>>> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>
>>>> ---
>>>>  Documentation/devicetree/bindings/arm/cpus.txt | 424 ++++++++++++++++++++++---
>>>>  1 file changed, 377 insertions(+), 47 deletions(-)
>>>>
>>>
>>> The binding looks mostly fine to me.
> 
> These bindings have still not received an ACK, and need review by
> Marvell guys in copy for the new compatible strings below.

If you can't get comment, then leave them out if they are not used
already. If they are used, then tough shit when they want to change.

> 
> Most importantly, we need to make a decision on the pre v7 uniprocessor
> systems, where MPIDR/CPUID are non-existent and the reg property is a
> pure SW enumeration. Current bindings (ie this patch) define
> 
> #address-cells = <0>;
> 
> for those processors (and there are a number of dts in the kernel with that
> set-up); Grant and Benjamin had a strong feeling against this choice, I
> have to make a decision on how to proceed, please let me know.

I agree that we should define #address-cells to 1 and reg will be simply
0,1,2,etc. in this case.

This change and fixing the example as I pointed out are what I was
waiting to see.

Rob

> 
> Comments welcome.
> 
> Lorenzo
> 
>>>
>>> [snip]
>>>
>>>> +			    "faraday,fa526"
>>>> +			    "intel,sa110"
>>>> +			    "intel,sa1100"
>>>> +			    "marvell,feroceon"
>>>> +			    "marvell,mohawk"
>>>> +			    "marvell,pj4"
>>>> +			    "marvell,sheeva-v7"
>>>> +			    "marvell,xsc3"
>>>> +			    "marvell,xscale"
>>>
>>> Better make sure the Marvell folks are happy with these. We don't need
>>> another rename here. I'm too annoyed with all the renames to pay attention.
>>
>> Ok, I will verify that, I copied maintainers in.
>>
>> On a side note I have just noticed that last cycle some dts were merged in the
>> kernel with cpu nodes that are not compliant (not picking on anyone,
>> eg am4372.dtsi).
>> Please prevent this from happening from now onwards, really please.
>>
>>>> +
>>>> +Example 4 (ARM Cortex-A57 64-bit system running OS in AArch64):
>>>> +
>>>
>>> Going back to my comments that the dtb can't be dependent on the OS,
>>> these 2 examples don't make sense.
>>
>> Gah, my bad sorry, I missed the examples while removing dependency on the OS
>> from the bindings.
>>
>>>> +
>>>> +Example 5 (ARM Cortex-A57 64-bit system running OS in AArch32):
>>>
>>> This example should be removed.
>>
>> Yes, consider it done, see above.
>>
>> Thanks,
>> Lorenzo
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe devicetree" in
>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] Documentation: DT: arm: define CPU topology bindings

[Rob Herring]

On 08/15/2013 04:42 AM, Lorenzo Pieralisi wrote:
> The advent of multi-cluster ARM systems requires a mechanism to describe
> how in hierarchical terms CPUs are connected in ARM SoCs so that the kernel
> can initialize and map resources like IRQs and memory space to specific
> group(s) of CPUs.
> 
> The CPU topology is made up of multiple hierarchy levels whose bottom
> layers (aka leaf nodes in device tree syntax) contain links to the HW
> CPUs in the system.
> 
> The topology bindings are generic for both 32-bit and 64-bit systems and
> lay the groundwork on top of which affinity schemes can be built.

By affinity schemes, you mean further bindings? Do we need this binding
until that point?

As is, I don't have much comment.

[snip]

> +Example 3 (ARM 32-bit, cortex-a8 single core):
> +
> +cpus {
> +	#size-cells = <0>;
> +	#address-cells = <1>;
> +
> +	cpu-map {
> +		cluster0 {
> +			core0 {
> +				cpu = <&CPU0>;
> +			};
> +		};
> +	};

This example seems utterly pointless. I think we should be specific that
single core does not contain a cpu-map. I suppose we could have a
threaded, single core case, but let's address that if we ever do.

Rob

> +
> +	CPU0: cpu@0 {
> +		device_type = "cpu";
> +		compatible = "arm,cortex-a8";
> +		reg = <0x0>;
> +	};
> +};
> +
> +===============================================================================
> +[1] ARM Linux kernel documentation
> +    Documentation/devicetree/bindings/arm/cpus.txt
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] Documentation: devicetree: arm: cpus/cpu nodes bindings updates

[Andrew Lunn]

On Fri, Sep 13, 2013 at 03:51:56PM -0500, Rob Herring wrote:
> On 09/13/2013 11:57 AM, Lorenzo Pieralisi wrote:
> > Hi Rob, all,
> > 
> > On Thu, Aug 15, 2013 at 04:22:38PM +0100, Lorenzo Pieralisi wrote:
> >> [adding Andrew, Gregory and Thomas to check the Marvell compatible names]
> >>
> >> On Thu, Aug 15, 2013 at 03:32:05PM +0100, Rob Herring wrote:
> >>> On 08/15/2013 04:42 AM, Lorenzo Pieralisi wrote:
> >>>> In order to extend the current cpu nodes bindings to newer CPUs
> >>>> inclusive of AArch64 and to update support for older ARM CPUs this
> >>>> patch updates device tree documentation for the cpu nodes bindings.
> >>>>
> >>>> Main changes:
> >>>>     - adds 64-bit bindings
> >>>>     - define usage of #address-cells
> >>>>     - defines behaviour on pre and post v7 uniprocessor systems
> >>>>     - adds ARM 11MPcore specific reg property definition
> >>>>
> >>>> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>
> >>>> ---
> >>>>  Documentation/devicetree/bindings/arm/cpus.txt | 424 ++++++++++++++++++++++---
> >>>>  1 file changed, 377 insertions(+), 47 deletions(-)
> >>>>
> >>>
> >>> The binding looks mostly fine to me.
> > 
> > These bindings have still not received an ACK, and need review by
> > Marvell guys in copy for the new compatible strings below.
> 
> If you can't get comment, then leave them out if they are not used
> already. If they are used, then tough shit when they want to change.

Hi Rob

We are working on it. Hope to be able to comment soon.

   Andrew
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] Documentation: devicetree: arm: cpus/cpu nodes bindings updates

[Andrew Lunn]

> > > > +			    "faraday,fa526"
> > > > +			    "intel,sa110"
> > > > +			    "intel,sa1100"
> > > > +			    "marvell,feroceon"
> > > > +			    "marvell,mohawk"
> > > > +			    "marvell,pj4"
> > > > +			    "marvell,sheeva-v7"
> > > > +			    "marvell,xsc3"
> > > > +			    "marvell,xscale"
> > > 
> > > Better make sure the Marvell folks are happy with these. We don't need
> > > another rename here. I'm too annoyed with all the renames to pay attention.
> > 
> > Ok, I will verify that, I copied maintainers in.

Hi Lorenzo

Here are the compatibility strings we would like, and a comment about
which SoCs will/do use them.

"marvell,feroceon"  - Orion5x
"marvell,sheeva-v5" - Mv78xx0, Kirkwood
"marvell,pj4a"      - Dove
"marvell,pj4b"      - Armada XP, Armada 370, Armada 1500
"marvell,mohawk"    - Armada 100

"marvell,xscale" & "marvell,xsc3" should be removed.

The compatibility strings are not correct, or present in all the
current <soc>.dtsi files. 

orion5x cpus needs adding.
mv78xx0 needs adding.
kirkwood.dtsi needs changing.
armada-370-xp.dtsi need changing.
armada-xp-mv*.dtsi needs changing.

We will deal with these in the next cycle.

Thanks
        Andrew

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] Documentation: devicetree: arm: cpus/cpu nodes bindings updates

[Lorenzo Pieralisi]

On Fri, Sep 13, 2013 at 09:51:56PM +0100, Rob Herring wrote:
> On 09/13/2013 11:57 AM, Lorenzo Pieralisi wrote:
> > Hi Rob, all,
> > 
> > On Thu, Aug 15, 2013 at 04:22:38PM +0100, Lorenzo Pieralisi wrote:
> >> [adding Andrew, Gregory and Thomas to check the Marvell compatible names]
> >>
> >> On Thu, Aug 15, 2013 at 03:32:05PM +0100, Rob Herring wrote:
> >>> On 08/15/2013 04:42 AM, Lorenzo Pieralisi wrote:
> >>>> In order to extend the current cpu nodes bindings to newer CPUs
> >>>> inclusive of AArch64 and to update support for older ARM CPUs this
> >>>> patch updates device tree documentation for the cpu nodes bindings.
> >>>>
> >>>> Main changes:
> >>>>     - adds 64-bit bindings
> >>>>     - define usage of #address-cells
> >>>>     - defines behaviour on pre and post v7 uniprocessor systems
> >>>>     - adds ARM 11MPcore specific reg property definition
> >>>>
> >>>> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>
> >>>> ---
> >>>>  Documentation/devicetree/bindings/arm/cpus.txt | 424 ++++++++++++++++++++++---
> >>>>  1 file changed, 377 insertions(+), 47 deletions(-)
> >>>>
> >>>
> >>> The binding looks mostly fine to me.
> > 
> > These bindings have still not received an ACK, and need review by
> > Marvell guys in copy for the new compatible strings below.
> 
> If you can't get comment, then leave them out if they are not used
> already. If they are used, then tough shit when they want to change.
> 
> > 
> > Most importantly, we need to make a decision on the pre v7 uniprocessor
> > systems, where MPIDR/CPUID are non-existent and the reg property is a
> > pure SW enumeration. Current bindings (ie this patch) define
> > 
> > #address-cells = <0>;
> > 
> > for those processors (and there are a number of dts in the kernel with that
> > set-up); Grant and Benjamin had a strong feeling against this choice, I
> > have to make a decision on how to proceed, please let me know.
> 
> I agree that we should define #address-cells to 1 and reg will be simply
> 0,1,2,etc. in this case.

Ok, so this means that when these bindings are finally acked and queued
for merging, there will be a slew of dts in the kernel that will need
patching (again) and this is likely to ruffle feathers. Honestly I would
leave this task to platform maintainers, if we all agree.

> This change and fixing the example as I pointed out are what I was
> waiting to see.

Perfect, given that I got feedback from Andrew, I will be posting a
final version very soon.

Lorenzo

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] Documentation: devicetree: arm: cpus/cpu nodes bindings updates

[Lorenzo Pieralisi]

On Sun, Sep 15, 2013 at 09:36:58AM +0100, Andrew Lunn wrote:
> > > > > +			    "faraday,fa526"
> > > > > +			    "intel,sa110"
> > > > > +			    "intel,sa1100"
> > > > > +			    "marvell,feroceon"
> > > > > +			    "marvell,mohawk"
> > > > > +			    "marvell,pj4"
> > > > > +			    "marvell,sheeva-v7"
> > > > > +			    "marvell,xsc3"
> > > > > +			    "marvell,xscale"
> > > > 
> > > > Better make sure the Marvell folks are happy with these. We don't need
> > > > another rename here. I'm too annoyed with all the renames to pay attention.
> > > 
> > > Ok, I will verify that, I copied maintainers in.
> 
> Hi Lorenzo
> 
> Here are the compatibility strings we would like, and a comment about
> which SoCs will/do use them.
> 
> "marvell,feroceon"  - Orion5x
> "marvell,sheeva-v5" - Mv78xx0, Kirkwood
> "marvell,pj4a"      - Dove
> "marvell,pj4b"      - Armada XP, Armada 370, Armada 1500
> "marvell,mohawk"    - Armada 100
> 
> "marvell,xscale" & "marvell,xsc3" should be removed.
> 
> The compatibility strings are not correct, or present in all the
> current <soc>.dtsi files. 
> 
> orion5x cpus needs adding.
> mv78xx0 needs adding.
> kirkwood.dtsi needs changing.
> armada-370-xp.dtsi need changing.
> armada-xp-mv*.dtsi needs changing.
> 
> We will deal with these in the next cycle.

Thank you very much Andrew, appreciated. I will update the document
accordingly and should be able to post a final version very soon.

Lorenzo

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] Documentation: DT: arm: define CPU topology bindings

[Lorenzo Pieralisi]

On Fri, Sep 13, 2013 at 10:07:11PM +0100, Rob Herring wrote:
> On 08/15/2013 04:42 AM, Lorenzo Pieralisi wrote:
> > The advent of multi-cluster ARM systems requires a mechanism to describe
> > how in hierarchical terms CPUs are connected in ARM SoCs so that the kernel
> > can initialize and map resources like IRQs and memory space to specific
> > group(s) of CPUs.
> > 
> > The CPU topology is made up of multiple hierarchy levels whose bottom
> > layers (aka leaf nodes in device tree syntax) contain links to the HW
> > CPUs in the system.
> > 
> > The topology bindings are generic for both 32-bit and 64-bit systems and
> > lay the groundwork on top of which affinity schemes can be built.
> 
> By affinity schemes, you mean further bindings? Do we need this binding
> until that point?

Well, further bindings (for HW like perf counters) will rely on these
topology bindings to build IRQ affinity masks for instance.

I think we should go on and publish these bindings asap so that we can
rely on them to properly describe topologies for ARM from the beginning.

Current reliance on MPIDR is a leap of faith.

> As is, I don't have much comment.

I take this as positive feedback, or put it another way, that you think
bindings are sound.

> 
> [snip]
> 
> > +Example 3 (ARM 32-bit, cortex-a8 single core):
> > +
> > +cpus {
> > +	#size-cells = <0>;
> > +	#address-cells = <1>;
> > +
> > +	cpu-map {
> > +		cluster0 {
> > +			core0 {
> > +				cpu = <&CPU0>;
> > +			};
> > +		};
> > +	};
> 
> This example seems utterly pointless. I think we should be specific that
> single core does not contain a cpu-map. I suppose we could have a
> threaded, single core case, but let's address that if we ever do.

Ok right. We can have a multi cluster system where one cluster is made
up of a single CPU, then a binding like the one above makes sense.
I will rule cpu-map out for uniprocessor systems.

Lorenzo

> 
> Rob
> 
> > +
> > +	CPU0: cpu@0 {
> > +		device_type = "cpu";
> > +		compatible = "arm,cortex-a8";
> > +		reg = <0x0>;
> > +	};
> > +};
> > +
> > +===============================================================================
> > +[1] ARM Linux kernel documentation
> > +    Documentation/devicetree/bindings/arm/cpus.txt
> > 
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html