[v2] ARM: realview: basic device tree implementation

This implements basic device tree boot support for the RealView
platforms, with a basic device tree for ARM PB1176 as an example.

The implementation is done with a new DT-specific board file
using only pre-existing bindings for the basic IRQ, timer and
serial port drivers. A new compatible type is added to the GIC
for the ARM1176.

This implementation uses the MFD syscon handle from day one to
access the system controller registers, and register the devices
using the SoC bus.

Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Rob Herring <robh@kernel.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v1->v2:
- Adjust timer clock names to be the same as the example in the
  device tree binding.
- Remove all memory fixup code - this should be handled by the
  device tree specification of memory areas or by special MM hacks
  for the RealView PBX.
- Fix the documentation around syscon to specify that it can be in
  any node, need not be the root node.
- Switch device tree license to the BSD license, taken from
  arch/powerpc/boot/dts/p1024rdb_32b.dts
- Add a hunk for the new compatible string to
  Documentation/devicetree/bindings/arm/gic.txt
- Move the clocks out of the SoC node, certainly the xtal is not
  sitting on the SoC...
- Sort the selects in Kconfig alphabetically
- Use IS_ENABLED() for the l2x0 code snippet
- Instead of checking the board variant in the reset routine to
  figure out how to tweak the reset controller, have a compatible
  string for each syscon variant, map it to an enum that provides a
  unique type ID and that way figure out how to handle it in a maybe
  more elegant way.
- Open issue: what do to with the l2x0 stuff?
---
 Documentation/devicetree/bindings/arm/arm-boards |  57 ++++++
 Documentation/devicetree/bindings/arm/gic.txt    |   1 +
 arch/arm/boot/dts/Makefile                       |   1 +
 arch/arm/boot/dts/arm-realview-pb1176.dts        | 177 ++++++++++++++++++
 arch/arm/mach-realview/Kconfig                   |  10 +
 arch/arm/mach-realview/Makefile                  |   1 +
 arch/arm/mach-realview/realview-dt.c             | 225 +++++++++++++++++++++++
 arch/arm/mm/Kconfig                              |   2 +-
 drivers/irqchip/irq-gic.c                        |   1 +
 9 files changed, 474 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/boot/dts/arm-realview-pb1176.dts
 create mode 100644 arch/arm/mach-realview/realview-dt.c

On Friday 09 May 2014, Linus Walleij wrote:

> diff --git a/arch/arm/boot/dts/arm-realview-pb1176.dts b/arch/arm/boot/dts/arm-realview-pb1176.dts
> new file mode 100644
> index 000000000000..0eea0f4a7b39
> --- /dev/null
> +++ b/arch/arm/boot/dts/arm-realview-pb1176.dts

Can you split this up into an arm-realview.dtsi file for the common parts and
a pb1176 specific file for the rest?

> +	soc {
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		compatible = "simple-bus";
> +		ranges;
> +
> +		syscon: syscon@10000000 {
> +			compatible = "arm,realview-pb1176-syscon", "syscon";
> +			reg = <0x10000000 0x1000>;
> +		};

Hmm, in order to have a proper reset driver, we probably want a separate
node for the reset controller. I believe the x-gene people have just
posted a generic reset driver based on syscon. Let's see if we can share
that.

> +		pb1176_serial0: uart@1010c000 {
> +			compatible = "arm,pl011", "arm,primecell";
> +			reg = <0x1010c000 0x1000>;
> +			interrupt-parent = <&intc_dc1176>;
> +			interrupts = <0 18 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&uartclk>, <&pclk>;
> +			clock-names = "uartclk", "apb_pclk";
> +		};

I believe the "official" name for a uart is serial@1010c000, not uart@1010c000.
I know we are inconsistent with that.

> +
> +/* Pointer to the system controller */
> +struct regmap *syscon_regmap;
> +u8 syscon_type;
> +
> +static struct map_desc realview_dt_io_desc[] __initdata = {
> +	{
> +		/* FIXME: static map needed for LED driver */
> +		.virtual	= IO_ADDRESS(REALVIEW_SYS_BASE),
> +		.pfn		= __phys_to_pfn(REALVIEW_SYS_BASE),
> +		.length		= SZ_4K,
> +		.type		= MT_DEVICE,
> +	},
> +};

I've looked at the driver and I don't see why this is required.
The driver does a devm_ioremap_resource(), which should work with
or without a static mapping.

> +/*
> + * We detect the different syscon types from the compatible strings.
> + */
> +enum realview_syscon {
> +	REALVIEW_SYSCON_EB,
> +	REALVIEW_SYSCON_PB1176,
> +	REALVIEW_SYSCON_PB11MP,
> +	REALVIEW_SYSCON_PBA8,
> +	REALVIEW_SYSCON_PBX,
> +};
> +
> +static const struct of_device_id realview_syscon_match[] = {
> +	{
> +		.compatible = "arm,realview-eb-syscon",
> +		.data = (void *)REALVIEW_SYSCON_EB,
> +	},
> +	{
> +		.compatible = "arm,realview-pb1176-syscon",
> +		.data = (void *)REALVIEW_SYSCON_PB1176,
> +	},
> +	{
> +		.compatible = "arm,realview-pb11mp-syscon",
> +		.data = (void *)REALVIEW_SYSCON_PB11MP,
> +	},
> +	{
> +		.compatible = "arm,realview-pba8-syscon",
> +		.data = (void *)REALVIEW_SYSCON_PBA8,
> +	},
> +	{
> +		.compatible = "arm,realview-pbx-syscon",
> +		.data = (void *)REALVIEW_SYSCON_PBX,
> +	},
> +};

If not the generic syscon reset driver, it should at least live in
drivers/power/reset/ rather than here.

> +#if IS_ENABLED(CONFIG_CACHE_L2X0)
> +	if (of_machine_is_compatible("arm,realview-eb"))
> +		/*
> +		 * 1MB (128KB/way), 8-way associativity,
> +		 * evmon/parity/share enabled
> +		 * Bits:  .... ...0 0111 1001 0000 .... .... ....
> +		 */
> +		l2x0_of_init(0x00790000, 0xfe000fff);
> +	else if (of_machine_is_compatible("arm,realview-pb1176"))
> +		/*
> +		 * 128Kb (16Kb/way) 8-way associativity.
> +		 * evmon/parity/share enabled.
> +		 */
> +		l2x0_of_init(0x00730000, 0xfe000fff);
> +	else if (of_machine_is_compatible("arm,realview-pb11mp"))
> +		/*
> +		 * 1MB (128KB/way), 8-way associativity,
> +		 * evmon/parity/share enabled
> +		 * Bits:  .... ...0 0111 1001 0000 .... .... ....
> +		 */
> +		l2x0_of_init(0x00730000, 0xfe000fff);
> +	else if (of_machine_is_compatible("arm,realview-pbx"))
> +		/*
> +		 * 16KB way size, 8-way associativity, parity disabled
> +		 * Bits:  .. 0 0 0 0 1 00 1 0 1 001 0 000 0 .... .... ....
> +		 */
> +		l2x0_of_init(0x02520000, 0xc0000fff);
> +#endif

You wrote that you added the #if IS_ENABLED() here. Why?

l2x0_of_init is already a nop if CONFIG_CACHE_L2X0 is not set. If you want to
skip a bit of dead code here, you could make it an inline function and do

	if (IS_ENABLED(CONFIG_CACHE_L2X0))
		realview_setup_l2x0();

Other than that, I still think we should avoid doing anything platform
specific here at all, and move it all into the l2x0_of_init function,
based on Russell's l2x0 patch series.

> +	soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
> +	if (!soc_dev_attr)
> +		return;
> +	ret = of_property_read_string(root, "compatible",
> +				      &soc_dev_attr->soc_id);
> +	if (ret)
> +		return;
> +	ret = of_property_read_string(root, "model", &soc_dev_attr->machine);
> +        if (ret)
> +		return;
> +	soc_dev_attr->family = "RealView";
> +	soc_dev = soc_device_register(soc_dev_attr);
> +	if (IS_ERR(soc_dev)) {
> +		kfree(soc_dev_attr);
> +		return;
> +	}
> +	parent = soc_device_to_device(soc_dev);
> +	ret = of_platform_populate(root, of_default_bus_match_table,
> +				   NULL, parent);
> +	if (ret) {
> +		pr_crit("could not populate device tree\n");
> +		return;
> +	}

I wonder if there is a way to do this without having code in the platform.
I would hate it if we get to the point where each platform is completely
empty but still requires a copy of this code.

We just faced the same discussion on the exynos chip id patches.

I also noticed that you pass the DT root here, which isn't actually
the intention of the soc device interface: you should pass the DT node
that has the on-chip devices but not the board-level (top-level) nodes
such as your fixed clocks.

> diff --git a/arch/arm/mm/Kconfig b/arch/arm/mm/Kconfig
> index 5bf7c3c3b301..5461c4401ed6 100644
> --- a/arch/arm/mm/Kconfig
> +++ b/arch/arm/mm/Kconfig
> @@ -928,7 +928,7 @@ config ARM_L1_CACHE_SHIFT
>  config ARM_DMA_MEM_BUFFERABLE
>  	bool "Use non-cacheable memory for DMA" if (CPU_V6 || CPU_V6K) && !CPU_V7
>  	depends on !(MACH_REALVIEW_PB1176 || REALVIEW_EB_ARM11MP || \
> -		     MACH_REALVIEW_PB11MP)
> +		     MACH_REALVIEW_PB11MP || REALVIEW_DT)
>  	default y if CPU_V6 || CPU_V6K || CPU_V7
>  	help
>  	  Historically, the kernel has used strongly ordered mappings to

Do you have an explanation for why this is needed? I don't remember seeing
this before, but I guess it gets in the way of multiplatform support.
Is this just a performance optimization on realview, or is it actually
required?


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

On Fri, May 9, 2014 at 12:44 PM, Arnd Bergmann <arnd@arndb.de> wrote:

>> +++ b/arch/arm/boot/dts/arm-realview-pb1176.dts
>
> Can you split this up into an arm-realview.dtsi file for the common parts and
> a pb1176 specific file for the rest?

I wonder if that makes any kind of sense. The RealView platforms does not
really share much more than the name, sadly. For example: IP blocks
aren't even at the same address.

I could create a .dtsi file but it would be empty :-(

>> +     soc {
>> +             #address-cells = <1>;
>> +             #size-cells = <1>;
>> +             compatible = "simple-bus";
>> +             ranges;
>> +
>> +             syscon: syscon@10000000 {
>> +                     compatible = "arm,realview-pb1176-syscon", "syscon";
>> +                     reg = <0x10000000 0x1000>;
>> +             };
>
> Hmm, in order to have a proper reset driver, we probably want a separate
> node for the reset controller. I believe the x-gene people have just
> posted a generic reset driver based on syscon. Let's see if we can share
> that.

I have looked at it. Patch titled
"power: reset: Add generic SYSCON register mapped reset"

Sadly it does not work for our case. This is because it assumes that
reboot will be triggered by writing one value to one register. The RealViews
need you to *first* write to a special unlock register, then write a magic
value to a magic register.

So of course we could extend the syscon regmap reset driver by
starting to encode a jam table such as a sequence of register writes
to a sequence of registers, but we have refused such code in the past
because as I recall Grant said, it is the beginning of a byte code
interpreter and then we could as well bite the bullet and start
implementing open firmware.

This is exactly the type of thing that ACPI AML code usually does
by the way so what he says makes a *lot* of sense.

So for now I keep to our special driver.

>> +             pb1176_serial0: uart@1010c000 {
>> +                     compatible = "arm,pl011", "arm,primecell";
>> +                     reg = <0x1010c000 0x1000>;
>> +                     interrupt-parent = <&intc_dc1176>;
>> +                     interrupts = <0 18 IRQ_TYPE_LEVEL_HIGH>;
>> +                     clocks = <&uartclk>, <&pclk>;
>> +                     clock-names = "uartclk", "apb_pclk";
>> +             };
>
> I believe the "official" name for a uart is serial@1010c000, not uart@1010c000.
> I know we are inconsistent with that.

OK fixed.

>> +/* Pointer to the system controller */
>> +struct regmap *syscon_regmap;
>> +u8 syscon_type;
>> +
>> +static struct map_desc realview_dt_io_desc[] __initdata = {
>> +     {
>> +             /* FIXME: static map needed for LED driver */
>> +             .virtual        = IO_ADDRESS(REALVIEW_SYS_BASE),
>> +             .pfn            = __phys_to_pfn(REALVIEW_SYS_BASE),
>> +             .length         = SZ_4K,
>> +             .type           = MT_DEVICE,
>> +     },
>> +};
>
> I've looked at the driver and I don't see why this is required.
> The driver does a devm_ioremap_resource(), which should work with
> or without a static mapping.

Well yeah, the version of the driver that is in linux-next does this.
This was parallel work and I had no idea which would reach
mainline first :-)

Anyway, I just deleted because I realized it was better if I came
up with the concept of syscon-leds to handle this through the
syscon.

>> +/*
>> + * We detect the different syscon types from the compatible strings.
>> + */
>> +enum realview_syscon {
>> +     REALVIEW_SYSCON_EB,
>> +     REALVIEW_SYSCON_PB1176,
>> +     REALVIEW_SYSCON_PB11MP,
>> +     REALVIEW_SYSCON_PBA8,
>> +     REALVIEW_SYSCON_PBX,
>> +};
>> +
>> +static const struct of_device_id realview_syscon_match[] = {
>> +     {
>> +             .compatible = "arm,realview-eb-syscon",
>> +             .data = (void *)REALVIEW_SYSCON_EB,
>> +     },
>> +     {
>> +             .compatible = "arm,realview-pb1176-syscon",
>> +             .data = (void *)REALVIEW_SYSCON_PB1176,
>> +     },
>> +     {
>> +             .compatible = "arm,realview-pb11mp-syscon",
>> +             .data = (void *)REALVIEW_SYSCON_PB11MP,
>> +     },
>> +     {
>> +             .compatible = "arm,realview-pba8-syscon",
>> +             .data = (void *)REALVIEW_SYSCON_PBA8,
>> +     },
>> +     {
>> +             .compatible = "arm,realview-pbx-syscon",
>> +             .data = (void *)REALVIEW_SYSCON_PBX,
>> +     },
>> +};
>
> If not the generic syscon reset driver, it should at least live in
> drivers/power/reset/ rather than here.

This we can fix.

>> +#if IS_ENABLED(CONFIG_CACHE_L2X0)
>> +     if (of_machine_is_compatible("arm,realview-eb"))
>> +             /*
>> +              * 1MB (128KB/way), 8-way associativity,
>> +              * evmon/parity/share enabled
>> +              * Bits:  .... ...0 0111 1001 0000 .... .... ....
>> +              */
>> +             l2x0_of_init(0x00790000, 0xfe000fff);
>> +     else if (of_machine_is_compatible("arm,realview-pb1176"))
>> +             /*
>> +              * 128Kb (16Kb/way) 8-way associativity.
>> +              * evmon/parity/share enabled.
>> +              */
>> +             l2x0_of_init(0x00730000, 0xfe000fff);
>> +     else if (of_machine_is_compatible("arm,realview-pb11mp"))
>> +             /*
>> +              * 1MB (128KB/way), 8-way associativity,
>> +              * evmon/parity/share enabled
>> +              * Bits:  .... ...0 0111 1001 0000 .... .... ....
>> +              */
>> +             l2x0_of_init(0x00730000, 0xfe000fff);
>> +     else if (of_machine_is_compatible("arm,realview-pbx"))
>> +             /*
>> +              * 16KB way size, 8-way associativity, parity disabled
>> +              * Bits:  .. 0 0 0 0 1 00 1 0 1 001 0 000 0 .... .... ....
>> +              */
>> +             l2x0_of_init(0x02520000, 0xc0000fff);
>> +#endif
>
> You wrote that you added the #if IS_ENABLED() here. Why?

Mainly because I had this other review comment that I
should use IS_ENABLED() rather than #ifdef and it
seemed better but I don't know anymore...

> l2x0_of_init is already a nop if CONFIG_CACHE_L2X0 is not set. If you want to
> skip a bit of dead code here, you could make it an inline function and do
>
>         if (IS_ENABLED(CONFIG_CACHE_L2X0))
>                 realview_setup_l2x0();
>
> Other than that, I still think we should avoid doing anything platform
> specific here at all, and move it all into the l2x0_of_init function,
> based on Russell's l2x0 patch series.

I just need a base branch I guess, then I can build on
top of Russell's patches, it's just a matter of dependencies.

>> +     soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
>> +     if (!soc_dev_attr)
>> +             return;
>> +     ret = of_property_read_string(root, "compatible",
>> +                                   &soc_dev_attr->soc_id);
>> +     if (ret)
>> +             return;
>> +     ret = of_property_read_string(root, "model", &soc_dev_attr->machine);
>> +        if (ret)
>> +             return;
>> +     soc_dev_attr->family = "RealView";
>> +     soc_dev = soc_device_register(soc_dev_attr);
>> +     if (IS_ERR(soc_dev)) {
>> +             kfree(soc_dev_attr);
>> +             return;
>> +     }
>> +     parent = soc_device_to_device(soc_dev);
>> +     ret = of_platform_populate(root, of_default_bus_match_table,
>> +                                NULL, parent);
>> +     if (ret) {
>> +             pr_crit("could not populate device tree\n");
>> +             return;
>> +     }
>
> I wonder if there is a way to do this without having code in the platform.
> I would hate it if we get to the point where each platform is completely
> empty but still requires a copy of this code.

The question can be rephrased like this:
Should we create drivers/soc?

The structure of the SoC bus device is maybe a bit irritating but it
has been hammered into the device core so now we have to live with
it as it is.

> We just faced the same discussion on the exynos chip id patches.

Yes, same answer :-)

> I also noticed that you pass the DT root here, which isn't actually
> the intention of the soc device interface: you should pass the DT node
> that has the on-chip devices but not the board-level (top-level) nodes
> such as your fixed clocks.

Hm that's true. I introduced yet another set of compatible tuples
like this:
compatible = "simple-bus", "arm-realview-pb1176-soc"

It will, just like the reset driver generate a new batch of identical
compatible-strings and documenation sets, but it does have the
upside of clean separation.

>> diff --git a/arch/arm/mm/Kconfig b/arch/arm/mm/Kconfig
>> index 5bf7c3c3b301..5461c4401ed6 100644
>> --- a/arch/arm/mm/Kconfig
>> +++ b/arch/arm/mm/Kconfig
>> @@ -928,7 +928,7 @@ config ARM_L1_CACHE_SHIFT
>>  config ARM_DMA_MEM_BUFFERABLE
>>       bool "Use non-cacheable memory for DMA" if (CPU_V6 || CPU_V6K) && !CPU_V7
>>       depends on !(MACH_REALVIEW_PB1176 || REALVIEW_EB_ARM11MP || \
>> -                  MACH_REALVIEW_PB11MP)
>> +                  MACH_REALVIEW_PB11MP || REALVIEW_DT)
>>       default y if CPU_V6 || CPU_V6K || CPU_V7
>>       help
>>         Historically, the kernel has used strongly ordered mappings to
>
> Do you have an explanation for why this is needed? I don't remember seeing
> this before, but I guess it gets in the way of multiplatform support.
> Is this just a performance optimization on realview, or is it actually
> required?

I don't know that actually. And as DMA is likely only used
on PCI I have no way of testing it either without such hardware.
But I guess I can just leave it out for the time being.

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

On Thu, May 22, 2014 at 10:31 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Thursday 22 May 2014 09:41:39 Linus Walleij wrote:

>> The question can be rephrased like this:
>> Should we create drivers/soc?
>
> I think we just did, not sure if the patches have made it into linux-next
> already, but there were a couple of other platforms already that want this
> directory.

Awesomeness. Can you give me the title of a patch or branch
from ARM SoC I should pull in to build on top of this? Then I can
split off all the SoC bus business and have the machine file real
small, nice and clean.

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

On Thu, May 8, 2014 at 5:08 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> This implements basic device tree boot support for the RealView
> platforms, with a basic device tree for ARM PB1176 as an example.
>
> The implementation is done with a new DT-specific board file
> using only pre-existing bindings for the basic IRQ, timer and
> serial port drivers. A new compatible type is added to the GIC
> for the ARM1176.
>
> This implementation uses the MFD syscon handle from day one to
> access the system controller registers, and register the devices
> using the SoC bus.
>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Rob Herring <robh@kernel.org>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v1->v2:
> - Adjust timer clock names to be the same as the example in the
>   device tree binding.
> - Remove all memory fixup code - this should be handled by the
>   device tree specification of memory areas or by special MM hacks
>   for the RealView PBX.
> - Fix the documentation around syscon to specify that it can be in
>   any node, need not be the root node.
> - Switch device tree license to the BSD license, taken from
>   arch/powerpc/boot/dts/p1024rdb_32b.dts
> - Add a hunk for the new compatible string to
>   Documentation/devicetree/bindings/arm/gic.txt
> - Move the clocks out of the SoC node, certainly the xtal is not
>   sitting on the SoC...
> - Sort the selects in Kconfig alphabetically
> - Use IS_ENABLED() for the l2x0 code snippet
> - Instead of checking the board variant in the reset routine to
>   figure out how to tweak the reset controller, have a compatible
>   string for each syscon variant, map it to an enum that provides a
>   unique type ID and that way figure out how to handle it in a maybe
>   more elegant way.
> - Open issue: what do to with the l2x0 stuff?
> ---
>  Documentation/devicetree/bindings/arm/arm-boards |  57 ++++++
>  Documentation/devicetree/bindings/arm/gic.txt    |   1 +
>  arch/arm/boot/dts/Makefile                       |   1 +
>  arch/arm/boot/dts/arm-realview-pb1176.dts        | 177 ++++++++++++++++++
>  arch/arm/mach-realview/Kconfig                   |  10 +
>  arch/arm/mach-realview/Makefile                  |   1 +
>  arch/arm/mach-realview/realview-dt.c             | 225 +++++++++++++++++++++++
>  arch/arm/mm/Kconfig                              |   2 +-
>  drivers/irqchip/irq-gic.c                        |   1 +
>  9 files changed, 474 insertions(+), 1 deletion(-)
>  create mode 100644 arch/arm/boot/dts/arm-realview-pb1176.dts
>  create mode 100644 arch/arm/mach-realview/realview-dt.c
>
> diff --git a/Documentation/devicetree/bindings/arm/arm-boards b/Documentation/devicetree/bindings/arm/arm-boards
> index 3509707f9320..86b36f399d1c 100644
> --- a/Documentation/devicetree/bindings/arm/arm-boards
> +++ b/Documentation/devicetree/bindings/arm/arm-boards
> @@ -86,3 +86,60 @@ Interrupt controllers:
>         compatible = "arm,versatile-sic";
>         interrupt-controller;
>         #interrupt-cells = <1>;
> +
> +
> +ARM RealView Boards
> +-------------------
> +The RealView boards cover tailored evaluation boards that are used to explore
> +the ARM11 and Cortex A-8 and Cortex A-9 processors.
> +
> +Required properties (in root node):
> +       /* RealView Emulation Baseboard */
> +       compatible = "arm,realview-eb";
> +        /* RealView Platform Baseboard for ARM1176JZF-S */
> +       compatible = "arm,realview-pb1176";
> +       /* RealView Platform Baseboard for ARM11 MPCore */
> +       compatible = "arm,realview-pb11mp";
> +       /* RealView Platform Baseboard for Cortex A-8 */
> +       compatible = "arm,realview-pba8";
> +       /* RealView Platform Baseboard Explore for Cortex A-9 */
> +       compatible = "arm,realview-pbx";
> +
> +Required nodes:
> +
> +- syscon: some node of the RealView platforms must have a
> +  system controller node pointing to the control registers,
> +  with the compatible string set to one of these tuples:
> +   "arm,realview-eb-syscon", "syscon"
> +   "arm,realview-pb1176-syscon", "syscon"
> +   "arm,realview-pb11mp-syscon", "syscon"
> +   "arm,realview-pba8-syscon", "syscon"
> +   "arm,realview-pbx-syscon", "syscon"
> +
> +  Required properties for the system controller:
> +  - regs: the location and size of the system controller registers,
> +    one range of 0x1000 bytes.
> +
> +Example:
> +
> +/dts-v1/;
> +#include <dt-bindings/interrupt-controller/irq.h>
> +#include "skeleton.dtsi"
> +
> +/ {
> +       model = "ARM RealView PB1176 with device tree";
> +       compatible = "arm,realview-pb1176";
> +
> +       soc {
> +               #address-cells = <1>;
> +               #size-cells = <1>;
> +               compatible = "simple-bus";
> +               ranges;
> +
> +               syscon: syscon@10000000 {
> +                       compatible = "arm,realview-syscon", "syscon";
> +                       reg = <0x10000000 0x1000>;
> +               };
> +
> +       };
> +};
> diff --git a/Documentation/devicetree/bindings/arm/gic.txt b/Documentation/devicetree/bindings/arm/gic.txt
> index 5573c08d3180..539420194efd 100644
> --- a/Documentation/devicetree/bindings/arm/gic.txt
> +++ b/Documentation/devicetree/bindings/arm/gic.txt
> @@ -16,6 +16,7 @@ Main node required properties:
>         "arm,cortex-a9-gic"
>         "arm,cortex-a7-gic"
>         "arm,arm11mp-gic"
> +       "arm,arm1176jzf-gic"
>  - interrupt-controller : Identifies the node as an interrupt controller
>  - #interrupt-cells : Specifies the number of cells needed to encode an
>    interrupt source.  The type shall be a <u32> and the value shall be 3.
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index 377b7c364033..1419a70026da 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -294,6 +294,7 @@ dtb-$(CONFIG_ARCH_PRIMA2) += prima2-evb.dtb
>  dtb-$(CONFIG_ARCH_QCOM) += qcom-msm8660-surf.dtb \
>         qcom-msm8960-cdp.dtb \
>         qcom-apq8074-dragonboard.dtb
> +dtb-$(CONFIG_ARCH_REALVIEW) += arm-realview-pb1176.dtb
>  dtb-$(CONFIG_ARCH_S3C24XX) += s3c2416-smdk2416.dtb
>  dtb-$(CONFIG_ARCH_S3C64XX) += s3c6410-mini6410.dtb \
>         s3c6410-smdk6410.dtb
> diff --git a/arch/arm/boot/dts/arm-realview-pb1176.dts b/arch/arm/boot/dts/arm-realview-pb1176.dts
> new file mode 100644
> index 000000000000..0eea0f4a7b39
> --- /dev/null
> +++ b/arch/arm/boot/dts/arm-realview-pb1176.dts
> @@ -0,0 +1,177 @@
> +/*
> + * Copyright 2014 Linaro Ltd
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a copy
> + * of this software and associated documentation files (the "Software"), to deal
> + * in the Software without restriction, including without limitation the rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +/dts-v1/;
> +#include <dt-bindings/interrupt-controller/irq.h>
> +#include "skeleton.dtsi"
> +
> +/ {
> +       model = "ARM RealView PB1176 with device tree";

Isn't "with device tree" redundant?

> +       compatible = "arm,realview-pb1176";
> +
> +       chosen { };
> +
> +       aliases {
> +               serial0 = &pb1176_serial0;
> +               serial1 = &pb1176_serial1;
> +               serial2 = &pb1176_serial2;
> +               serial3 = &pb1176_serial3;
> +       };
> +
> +       memory {
> +               /* 128 MiB memory @ 0x0 */
> +               reg = <0x00000000 0x08000000>;
> +       };
> +
> +       xtal24mhz: xtal24mhz@24M {
> +               #clock-cells = <0>;
> +               compatible = "fixed-clock";
> +               clock-frequency = <24000000>;
> +       };
> +
> +       timclk: timclk@1M {
> +               #clock-cells = <0>;
> +               compatible = "fixed-factor-clock";
> +               clock-div = <24>;
> +               clock-mult = <1>;
> +               clocks = <&xtal24mhz>;
> +       };
> +
> +       uartclk: uartclk@24M {
> +               #clock-cells = <0>;
> +               compatible = "fixed-factor-clock";
> +               clock-div = <1>;
> +               clock-mult = <1>;
> +               clocks = <&xtal24mhz>;
> +       };
> +
> +       /* FIXME: this actually hangs off the PLL clocks */
> +       pclk: pclk@0 {
> +               #clock-cells = <0>;
> +               compatible = "fixed-clock";
> +               clock-frequency = <0>;
> +       };
> +
> +       soc {
> +               #address-cells = <1>;
> +               #size-cells = <1>;
> +               compatible = "simple-bus";
> +               ranges;
> +
> +               syscon: syscon@10000000 {
> +                       compatible = "arm,realview-pb1176-syscon", "syscon";
> +                       reg = <0x10000000 0x1000>;
> +               };
> +
> +               /* Primary DevChip GIC synthesized with the CPU */
> +               intc_dc1176: interrupt-controller@10120000 {
> +                       compatible = "arm,arm1176jzf-gic";
> +                       #interrupt-cells = <3>;
> +                       #address-cells = <1>;
> +                       interrupt-controller;
> +                       reg = <0x10121000 0x1000>,
> +                             <0x10120000 0x100>;
> +               };
> +
> +               /* This GIC on the board is cascaded off the DevChip GIC */
> +               intc_pb1176: interrupt-controller@10040000 {
> +                       compatible = "arm,arm1176jzf-gic";
> +                       #interrupt-cells = <3>;
> +                       #address-cells = <1>;
> +                       interrupt-controller;
> +                       reg = <0x10041000 0x1000>,
> +                             <0x10040000 0x100>;
> +                       interrupt-parent = <&intc_dc1176>;
> +                       interrupts = <0 31 IRQ_TYPE_LEVEL_HIGH>;
> +               };
> +
> +               L2: l2-cache {
> +                       compatible = "arm,l220-cache";
> +                       reg = <0x10110000 0x1000>;
> +                       interrupt-parent = <&intc_dc1176>;
> +                       interrupts = <0 13 IRQ_TYPE_LEVEL_HIGH>;
> +                       cache-unified;
> +                       cache-level = <2>;
> +               };
> +
> +               pmu {
> +                       compatible = "arm,arm1176-pmu";
> +                       interrupt-parent = <&intc_dc1176>;
> +                       interrupts = <0 7 IRQ_TYPE_LEVEL_HIGH>;
> +               };
> +
> +               timer01: timer@10104000 {
> +                       compatible = "arm,sp804", "arm,primecell";
> +                       reg = <0x10104000 0x1000>;
> +                       interrupt-parent = <&intc_dc1176>;
> +                       interrupts = <0 8 IRQ_TYPE_LEVEL_HIGH>, <0 9 IRQ_TYPE_LEVEL_HIGH>;
> +                       clocks = <&timclk>, <&timclk>, <&pclk>;
> +                       clock-names = "timer1", "timer2", "apb_pclk";
> +               };
> +
> +               timer23: timer@10105000 {
> +                       compatible = "arm,sp804", "arm,primecell";
> +                       reg = <0x10105000 0x1000>;
> +                       interrupt-parent = <&intc_dc1176>;
> +                       interrupts = <0 10 IRQ_TYPE_LEVEL_HIGH>;
> +                       arm,sp804-has-irq = <1>;
> +                       clocks = <&timclk>, <&timclk>, <&pclk>;
> +                       clock-names = "timer1", "timer2", "apb_pclk";
> +               };
> +
> +               pb1176_serial0: uart@1010c000 {
> +                       compatible = "arm,pl011", "arm,primecell";
> +                       reg = <0x1010c000 0x1000>;
> +                       interrupt-parent = <&intc_dc1176>;
> +                       interrupts = <0 18 IRQ_TYPE_LEVEL_HIGH>;
> +                       clocks = <&uartclk>, <&pclk>;
> +                       clock-names = "uartclk", "apb_pclk";
> +               };
> +
> +               pb1176_serial1: uart@1010d000 {
> +                       compatible = "arm,pl011", "arm,primecell";
> +                       reg = <0x1010d000 0x1000>;
> +                       interrupt-parent = <&intc_dc1176>;
> +                       interrupts = <0 19 IRQ_TYPE_LEVEL_HIGH>;
> +                       clocks = <&uartclk>, <&pclk>;
> +                       clock-names = "uartclk", "apb_pclk";
> +               };
> +
> +               pb1176_serial2: uart@1010e000 {
> +                       compatible = "arm,pl011", "arm,primecell";
> +                       reg = <0x1010e000 0x1000>;
> +                       interrupt-parent = <&intc_dc1176>;
> +                       interrupts = <0 20 IRQ_TYPE_LEVEL_HIGH>;
> +                       clocks = <&uartclk>, <&pclk>;
> +                       clock-names = "uartclk", "apb_pclk";
> +               };
> +
> +               pb1176_serial3: uart@1010f000 {
> +                       compatible = "arm,pl011", "arm,primecell";
> +                       reg = <0x1010f000 0x1000>;
> +                       interrupt-parent = <&intc_dc1176>;
> +                       interrupts = <0 21 IRQ_TYPE_LEVEL_HIGH>;
> +                       clocks = <&uartclk>, <&pclk>;
> +                       clock-names = "uartclk", "apb_pclk";
> +               };
> +       };
> +};
> diff --git a/arch/arm/mach-realview/Kconfig b/arch/arm/mach-realview/Kconfig
> index 9db2029aa632..65a07f5b5225 100644
> --- a/arch/arm/mach-realview/Kconfig
> +++ b/arch/arm/mach-realview/Kconfig
> @@ -1,6 +1,16 @@
>  menu "RealView platform type"
>         depends on ARCH_REALVIEW
>
> +config REALVIEW_DT
> +       bool "Support RealView(R) Device Tree based boot"
> +       select ARM_GIC
> +       select MFD_SYSCON
> +       select SOC_BUS
> +       select USE_OF
> +       help
> +         Include support for booting the ARM(R) RealView(R) evaluation
> +         boards using a device tree machine description.
> +
>  config MACH_REALVIEW_EB
>         bool "Support RealView(R) Emulation Baseboard"
>         select ARM_GIC
> diff --git a/arch/arm/mach-realview/Makefile b/arch/arm/mach-realview/Makefile
> index 541fa4c109ef..e07fdf7ae8a7 100644
> --- a/arch/arm/mach-realview/Makefile
> +++ b/arch/arm/mach-realview/Makefile
> @@ -3,6 +3,7 @@
>  #
>
>  obj-y                                  := core.o
> +obj-$(CONFIG_REALVIEW_DT)              += realview-dt.o
>  obj-$(CONFIG_MACH_REALVIEW_EB)         += realview_eb.o
>  obj-$(CONFIG_MACH_REALVIEW_PB11MP)     += realview_pb11mp.o
>  obj-$(CONFIG_MACH_REALVIEW_PB1176)     += realview_pb1176.o
> diff --git a/arch/arm/mach-realview/realview-dt.c b/arch/arm/mach-realview/realview-dt.c
> new file mode 100644
> index 000000000000..4ba7c5303d09
> --- /dev/null
> +++ b/arch/arm/mach-realview/realview-dt.c
> @@ -0,0 +1,225 @@
> +/*
> + * Copyright (C) 2014 Linaro Ltd.
> + *
> + * Author: Linus Walleij <linus.walleij@linaro.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2, as
> + * published by the Free Software Foundation.
> + *
> + */
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/irqchip.h>
> +#include <linux/irqchip/arm-gic.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/regmap.h>
> +#include <linux/sys_soc.h>
> +#include <linux/slab.h>
> +#include <asm/mach/map.h>
> +#include <asm/mach/arch.h>
> +#include <asm/hardware/cache-l2x0.h>
> +#include <mach/hardware.h>
> +#include <mach/platform.h>
> +#include <mach/board-eb.h>
> +#include <mach/board-pb1176.h>
> +#include <mach/board-pb11mp.h>
> +#include <mach/board-pbx.h>
> +#include "core.h"
> +
> +/* Pointer to the system controller */
> +struct regmap *syscon_regmap;
> +u8 syscon_type;
> +
> +static struct map_desc realview_dt_io_desc[] __initdata = {
> +       {
> +               /* FIXME: static map needed for LED driver */

Don't you have patches to fix this? I need them for Versatile
conversion as well.

> +               .virtual        = IO_ADDRESS(REALVIEW_SYS_BASE),
> +               .pfn            = __phys_to_pfn(REALVIEW_SYS_BASE),
> +               .length         = SZ_4K,
> +               .type           = MT_DEVICE,
> +       },
> +};
> +
> +static void __init realview_dt_map_io(void)
> +{
> +       iotable_init(realview_dt_io_desc, ARRAY_SIZE(realview_dt_io_desc));
> +}
> +
> +/*
> + * We detect the different syscon types from the compatible strings.
> + */
> +enum realview_syscon {
> +       REALVIEW_SYSCON_EB,
> +       REALVIEW_SYSCON_PB1176,
> +       REALVIEW_SYSCON_PB11MP,
> +       REALVIEW_SYSCON_PBA8,
> +       REALVIEW_SYSCON_PBX,
> +};
> +
> +static const struct of_device_id realview_syscon_match[] = {
> +       {
> +               .compatible = "arm,realview-eb-syscon",
> +               .data = (void *)REALVIEW_SYSCON_EB,
> +       },
> +       {
> +               .compatible = "arm,realview-pb1176-syscon",
> +               .data = (void *)REALVIEW_SYSCON_PB1176,
> +       },
> +       {
> +               .compatible = "arm,realview-pb11mp-syscon",
> +               .data = (void *)REALVIEW_SYSCON_PB11MP,
> +       },
> +       {
> +               .compatible = "arm,realview-pba8-syscon",
> +               .data = (void *)REALVIEW_SYSCON_PBA8,
> +       },
> +       {
> +               .compatible = "arm,realview-pbx-syscon",
> +               .data = (void *)REALVIEW_SYSCON_PBX,
> +       },
> +};
> +
> +static void __init realview_dt_init_machine(void)
> +{
> +       struct device_node *root;
> +       struct device_node *syscon;
> +       const struct of_device_id *syscon_id;
> +       struct soc_device *soc_dev;
> +       struct soc_device_attribute *soc_dev_attr;
> +       struct device *parent;
> +       u32 coreid;
> +       int ret;
> +
> +#if IS_ENABLED(CONFIG_CACHE_L2X0)
> +       if (of_machine_is_compatible("arm,realview-eb"))
> +               /*
> +                * 1MB (128KB/way), 8-way associativity,

These should go into DT properties. Cache attributes are already
defined in ePAPR.

> +                * evmon/parity/share enabled

These can probably be unconditionally enabled except for parity which
could be a DT property.

> +                * Bits:  .... ...0 0111 1001 0000 .... .... ....
> +                */
> +               l2x0_of_init(0x00790000, 0xfe000fff);
> +       else if (of_machine_is_compatible("arm,realview-pb1176"))
> +               /*
> +                * 128Kb (16Kb/way) 8-way associativity.
> +                * evmon/parity/share enabled.
> +                */
> +               l2x0_of_init(0x00730000, 0xfe000fff);
> +       else if (of_machine_is_compatible("arm,realview-pb11mp"))
> +               /*
> +                * 1MB (128KB/way), 8-way associativity,
> +                * evmon/parity/share enabled
> +                * Bits:  .... ...0 0111 1001 0000 .... .... ....
> +                */
> +               l2x0_of_init(0x00730000, 0xfe000fff);
> +       else if (of_machine_is_compatible("arm,realview-pbx"))
> +               /*
> +                * 16KB way size, 8-way associativity, parity disabled
> +                * Bits:  .. 0 0 0 0 1 00 1 0 1 001 0 000 0 .... .... ....
> +                */
> +               l2x0_of_init(0x02520000, 0xc0000fff);
> +#endif
> +
> +        /* Here we create an SoC device for the root node */
> +       root = of_find_node_by_path("/");
> +       if (!root)
> +               return;
> +       syscon = of_find_matching_node_and_match(root, realview_syscon_match,
> +                                                &syscon_id);
> +       if (!syscon)
> +               return;
> +       syscon_type = (enum realview_syscon)syscon_id->data;
> +
> +       soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
> +       if (!soc_dev_attr)
> +               return;
> +       ret = of_property_read_string(root, "compatible",
> +                                     &soc_dev_attr->soc_id);
> +       if (ret)
> +               return;
> +       ret = of_property_read_string(root, "model", &soc_dev_attr->machine);
> +        if (ret)
> +               return;
> +       soc_dev_attr->family = "RealView";
> +       soc_dev = soc_device_register(soc_dev_attr);
> +       if (IS_ERR(soc_dev)) {
> +               kfree(soc_dev_attr);
> +               return;
> +       }
> +       parent = soc_device_to_device(soc_dev);

We really need to standardize this soc device stuff so that platforms
always have a parent soc device or not.

> +       ret = of_platform_populate(root, of_default_bus_match_table,
> +                                  NULL, parent);
> +       if (ret) {
> +               pr_crit("could not populate device tree\n");
> +               return;
> +       }
> +
> +       syscon_regmap = syscon_node_to_regmap(syscon);
> +       if (IS_ERR(syscon_regmap)) {
> +               pr_crit("could not locate syscon regmap\n");
> +               return;
> +       }
> +       ret = regmap_read(syscon_regmap, REALVIEW_SYS_ID_OFFSET, &coreid);
> +       if (ret)
> +               return;
> +       pr_info("RealView Syscon type %02x, Core ID: 0x%08x\n",
> +               syscon_type, coreid);
> +       /* FIXME: add attributes for SoC to sysfs */
> +}
> +
> +static void realview_dt_restart(enum reboot_mode mode, const char *cmd)
> +{
> +       if (IS_ERR(syscon_regmap))
> +               return;
> +
> +       /* Unlock the reset register */
> +       regmap_write(syscon_regmap, REALVIEW_SYS_LOCK_OFFSET,
> +                    REALVIEW_SYS_LOCK_VAL);
> +       /* Then hit reset on the different machines */
> +       switch (syscon_type) {
> +       case REALVIEW_SYSCON_EB:
> +               regmap_write(syscon_regmap,
> +                            REALVIEW_SYS_RESETCTL_OFFSET, 0x0008);
> +               break;
> +       case REALVIEW_SYSCON_PB1176:
> +               regmap_write(syscon_regmap,
> +                            REALVIEW_SYS_RESETCTL_OFFSET, 0x0100);
> +               break;
> +       case REALVIEW_SYSCON_PB11MP:
> +       case REALVIEW_SYSCON_PBA8:
> +               regmap_write(syscon_regmap, REALVIEW_SYS_RESETCTL_OFFSET,
> +                            0x0000);
> +               regmap_write(syscon_regmap, REALVIEW_SYS_RESETCTL_OFFSET,
> +                            0x0004);
> +               break;
> +       case REALVIEW_SYSCON_PBX:
> +               regmap_write(syscon_regmap, REALVIEW_SYS_RESETCTL_OFFSET,
> +                            0x00f0);
> +               regmap_write(syscon_regmap, REALVIEW_SYS_RESETCTL_OFFSET,
> +                            0x00f4);
> +               break;
> +       }
> +       dsb();

Barriers need comments as to why they are needed.

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

On Thu, May 22, 2014 at 3:17 PM, Rob Herring <robh@kernel.org> wrote:
> On Thu, May 8, 2014 at 5:08 PM, Linus Walleij <linus.walleij@linaro.org> wrote:

>> +       model = "ARM RealView PB1176 with device tree";
>
> Isn't "with device tree" redundant?

Nuked it.

>> +static struct map_desc realview_dt_io_desc[] __initdata = {
>> +       {
>> +               /* FIXME: static map needed for LED driver */
>
> Don't you have patches to fix this? I need them for Versatile
> conversion as well.

Done away with. The legacy driver was moved to drivers/leds/leds-versatile.c
and I then contemplated adding DT support to it but it was a *bad*
idea.

Instead I have created drivers/leds/leds-syscon.c that can handle
any LEDs in a syscon. Will post in next iteration as soon as
I get the drivers/soc dependency in place.

>> +#if IS_ENABLED(CONFIG_CACHE_L2X0)
>> +       if (of_machine_is_compatible("arm,realview-eb"))
>> +               /*
>> +                * 1MB (128KB/way), 8-way associativity,
>
> These should go into DT properties. Cache attributes are already
> defined in ePAPR.

Yes, the dependencies to do this with DT is floating around somewhere
in the ARM tree or Russells tree I think, maybe I cannot do this until
the next kernel cycle.

>> +                * evmon/parity/share enabled
>
> These can probably be unconditionally enabled except for parity which
> could be a DT property.

I'll wait and see what I need to add really.

>> +       parent = soc_device_to_device(soc_dev);
>
> We really need to standardize this soc device stuff so that platforms
> always have a parent soc device or not.

Arnds position is that the entire platform/board should not be
a "soc device" and that seems correct since the board is no SoC.

So I will spin this off into a SoC driver and push under
drivers/soc and bind to the SoC node in the device tree.

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

On Thursday 22 May 2014 15:30:39 Linus Walleij wrote:
> >> +       parent = soc_device_to_device(soc_dev);
> >
> > We really need to standardize this soc device stuff so that platforms
> > always have a parent soc device or not.
> 
> Arnds position is that the entire platform/board should not be
> a "soc device" and that seems correct since the board is no SoC.
> 
> So I will spin this off into a SoC driver and push under
> drivers/soc and bind to the SoC node in the device tree.

To be clear, this has nothing to do with whether this is strictly a SoC
or not. I would still want to see all the internal components of the
main chip under the soc node, and probably anything that is attached
to one of its interfaces under those nodes. The parts that should not
be under /soc are the ones that are not in the device hierarchy, such
as external clock generators.

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

On Thursday 22 May 2014 15:04:07 Linus Walleij wrote:
> On Thu, May 22, 2014 at 10:31 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Thursday 22 May 2014 09:41:39 Linus Walleij wrote:
> 
> >> The question can be rephrased like this:
> >> Should we create drivers/soc?
> >
> > I think we just did, not sure if the patches have made it into linux-next
> > already, but there were a couple of other platforms already that want this
> > directory.
> 
> Awesomeness. Can you give me the title of a patch or branch
> from ARM SoC I should pull in to build on top of this? Then I can
> split off all the SoC bus business and have the machine file real
> small, nice and clean.

I'd have to check again, it seems that as of today, there is no drivers/soc
on arm-soc yet. I do remember that there were at least two platforms besides
yours that want it though, so we have to coordinate it a bit.

Then again, as I expect all of them to go through arm-soc, we can probably
resolve the conflicts in our tree.

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

On Fri, May 09, 2014 at 12:08:16AM +0200, Linus Walleij wrote:
> This implements basic device tree boot support for the RealView
> platforms, with a basic device tree for ARM PB1176 as an example.
> 
> The implementation is done with a new DT-specific board file
> using only pre-existing bindings for the basic IRQ, timer and
> serial port drivers. A new compatible type is added to the GIC
> for the ARM1176.
> 
> This implementation uses the MFD syscon handle from day one to
> access the system controller registers, and register the devices
> using the SoC bus.
> 
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Rob Herring <robh@kernel.org>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v1->v2:
> - Adjust timer clock names to be the same as the example in the
>   device tree binding.
> - Remove all memory fixup code - this should be handled by the
>   device tree specification of memory areas or by special MM hacks
>   for the RealView PBX.
> - Fix the documentation around syscon to specify that it can be in
>   any node, need not be the root node.
> - Switch device tree license to the BSD license, taken from
>   arch/powerpc/boot/dts/p1024rdb_32b.dts
> - Add a hunk for the new compatible string to
>   Documentation/devicetree/bindings/arm/gic.txt
> - Move the clocks out of the SoC node, certainly the xtal is not
>   sitting on the SoC...
> - Sort the selects in Kconfig alphabetically
> - Use IS_ENABLED() for the l2x0 code snippet
> - Instead of checking the board variant in the reset routine to
>   figure out how to tweak the reset controller, have a compatible
>   string for each syscon variant, map it to an enum that provides a
>   unique type ID and that way figure out how to handle it in a maybe
>   more elegant way.
> - Open issue: what do to with the l2x0 stuff?
> ---
>  Documentation/devicetree/bindings/arm/arm-boards |  57 ++++++
>  Documentation/devicetree/bindings/arm/gic.txt    |   1 +
>  arch/arm/boot/dts/Makefile                       |   1 +
>  arch/arm/boot/dts/arm-realview-pb1176.dts        | 177 ++++++++++++++++++
>  arch/arm/mach-realview/Kconfig                   |  10 +
>  arch/arm/mach-realview/Makefile                  |   1 +
>  arch/arm/mach-realview/realview-dt.c             | 225 +++++++++++++++++++++++
>  arch/arm/mm/Kconfig                              |   2 +-
>  drivers/irqchip/irq-gic.c                        |   1 +
>  9 files changed, 474 insertions(+), 1 deletion(-)
>  create mode 100644 arch/arm/boot/dts/arm-realview-pb1176.dts
>  create mode 100644 arch/arm/mach-realview/realview-dt.c
> 
...
> diff --git a/Documentation/devicetree/bindings/arm/gic.txt b/Documentation/devicetree/bindings/arm/gic.txt
> index 5573c08d3180..539420194efd 100644
> --- a/Documentation/devicetree/bindings/arm/gic.txt
> +++ b/Documentation/devicetree/bindings/arm/gic.txt
> @@ -16,6 +16,7 @@ Main node required properties:
>  	"arm,cortex-a9-gic"
>  	"arm,cortex-a7-gic"
>  	"arm,arm11mp-gic"
> +	"arm,arm1176jzf-gic"
>  - interrupt-controller : Identifies the node as an interrupt controller
>  - #interrupt-cells : Specifies the number of cells needed to encode an
>    interrupt source.  The type shall be a <u32> and the value shall be 3.
...
> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
> index 57d165e026f4..70281b38c73b 100644
> --- a/drivers/irqchip/irq-gic.c
> +++ b/drivers/irqchip/irq-gic.c
> @@ -1073,6 +1073,7 @@ gic_of_init(struct device_node *node, struct device_node *parent)
>  }
>  IRQCHIP_DECLARE(cortex_a15_gic, "arm,cortex-a15-gic", gic_of_init);
>  IRQCHIP_DECLARE(cortex_a9_gic, "arm,cortex-a9-gic", gic_of_init);
> +IRQCHIP_DECLARE(arm1176jzf_gic, "arm,arm1176jzf-gic", gic_of_init);
>  IRQCHIP_DECLARE(msm_8660_qgic, "qcom,msm-8660-qgic", gic_of_init);
>  IRQCHIP_DECLARE(msm_qgic2, "qcom,msm-qgic2", gic_of_init);

Please place in alphabetical order.  We're starting to collect
compatible strings for this driver...

Acked-by: Jason Cooper <jason@lakedaemon.net>

thx,

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

On Thu, May 22, 2014 at 09:31:37AM +0100, Arnd Bergmann wrote:
> commit 42c4dafe803dcad82980fd8b0831a89032156f93
> Author: Catalin Marinas <catalin.marinas@arm.com>
> Date:   Thu Jul 1 13:22:48 2010 +0100
> 
>     ARM: 6202/1: Do not ARM_DMA_MEM_BUFFERABLE on RealView boards with L210/L220
>     
>     RealView boards with certain revisions of the L210/L220 cache controller
>     may have issues (hardware deadlock) with the mandatory barriers (DSB
>     followed by an L2 cache sync) when ARM_DMA_MEM_BUFFERABLE is enabled.
>     The patch disables ARM_DMA_MEM_BUFFERABLE for these boards.
>     
>     Tested-by: Linus Walleij <linus.walleij@stericsson.com>
>     Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
>     Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>

Looking through the L220 errata document, I think this relates to 425331
which says that a cache sync can deadlock the system if the write buffer
is not empty. By making the DMA memory Strongly Ordered, we avoid the
L220 write buffer. In addition to the above commit, we also had
2503a5ecd86c (ARM: 6201/1: RealView: Do not use outer_sync() on
ARM11MPCore boards with L220).

The above only solves the issue for coherent DMA allocations. Streaming
DMA requires additional changes to cache-l2x0.c but they are all weird
and depend on how the L2x0 is wired (for example, using SWP to access
the L2x0 registers works on one RealView board but not on another).
Anyway, I don't think we have any streaming DMA used on these RealView
boards.

If we only use the DMA coherent buffers for CLCD on these boards, we
don't really need the outer_sync() either so we could go back to
bufferable DMA memory (strangely enough, this L2 cache workaround looks
similar to Marvell's PCI+PL310 problem where we want to avoid
outer_sync()).

On Thursday 22 May 2014 15:45:03 Catalin Marinas wrote:
> On Thu, May 22, 2014 at 09:31:37AM +0100, Arnd Bergmann wrote:
> > commit 42c4dafe803dcad82980fd8b0831a89032156f93
> > Author: Catalin Marinas <catalin.marinas@arm.com>
> > Date:   Thu Jul 1 13:22:48 2010 +0100
> > 
> >     ARM: 6202/1: Do not ARM_DMA_MEM_BUFFERABLE on RealView boards with L210/L220
> >     
> >     RealView boards with certain revisions of the L210/L220 cache controller
> >     may have issues (hardware deadlock) with the mandatory barriers (DSB
> >     followed by an L2 cache sync) when ARM_DMA_MEM_BUFFERABLE is enabled.
> >     The patch disables ARM_DMA_MEM_BUFFERABLE for these boards.
> >     
> >     Tested-by: Linus Walleij <linus.walleij@stericsson.com>
> >     Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> >     Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> 
> Looking through the L220 errata document, I think this relates to 425331
> which says that a cache sync can deadlock the system if the write buffer
> is not empty.

Hmm, but isn't the write buffer empty here because we have just issued
a dsb()?

> By making the DMA memory Strongly Ordered, we avoid the
> L220 write buffer. In addition to the above commit, we also had
> 2503a5ecd86c (ARM: 6201/1: RealView: Do not use outer_sync() on
> ARM11MPCore boards with L220).

Ah, so this also stands in the way of multiplatform realview :(

> The above only solves the issue for coherent DMA allocations. Streaming
> DMA requires additional changes to cache-l2x0.c but they are all weird
> and depend on how the L2x0 is wired (for example, using SWP to access
> the L2x0 registers works on one RealView board but not on another).
> Anyway, I don't think we have any streaming DMA used on these RealView
> boards.

Yes, I think that is correct. MMCI, SSP and serial could all use
DMA but are not configured to do it.

> If we only use the DMA coherent buffers for CLCD on these boards, we
> don't really need the outer_sync() either so we could go back to
> bufferable DMA memory (strangely enough, this L2 cache workaround looks
> similar to Marvell's PCI+PL310 problem where we want to avoid
> outer_sync()).

Ah, so you think we can add another hook to NULL out outer_cache.sync
per platform?

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

On Thu, May 22, 2014 at 04:49:16PM +0100, Arnd Bergmann wrote:
> On Thursday 22 May 2014 15:45:03 Catalin Marinas wrote:
> > On Thu, May 22, 2014 at 09:31:37AM +0100, Arnd Bergmann wrote:
> > > commit 42c4dafe803dcad82980fd8b0831a89032156f93
> > > Author: Catalin Marinas <catalin.marinas@arm.com>
> > > Date:   Thu Jul 1 13:22:48 2010 +0100
> > > 
> > >     ARM: 6202/1: Do not ARM_DMA_MEM_BUFFERABLE on RealView boards with L210/L220
> > >     
> > >     RealView boards with certain revisions of the L210/L220 cache controller
> > >     may have issues (hardware deadlock) with the mandatory barriers (DSB
> > >     followed by an L2 cache sync) when ARM_DMA_MEM_BUFFERABLE is enabled.
> > >     The patch disables ARM_DMA_MEM_BUFFERABLE for these boards.
> > >     
> > >     Tested-by: Linus Walleij <linus.walleij@stericsson.com>
> > >     Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> > >     Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> > 
> > Looking through the L220 errata document, I think this relates to 425331
> > which says that a cache sync can deadlock the system if the write buffer
> > is not empty.
> 
> Hmm, but isn't the write buffer empty here because we have just issued
> a dsb()?

The problem with these outer caches is that DSB doesn't propagate to
them (and I hope to never see such caches again ;)). So an mb() on such
systems needs to be dsb()+outer_sync(). The dsb() pushes data from the
CPU write buffer into the L2x0 write buffer and outer_sync() does the
flushing out of the outer cache write buffer (which strangely enough
buffers even if the memory access is Normal NonCacheable).

> > If we only use the DMA coherent buffers for CLCD on these boards, we
> > don't really need the outer_sync() either so we could go back to
> > bufferable DMA memory (strangely enough, this L2 cache workaround looks
> > similar to Marvell's PCI+PL310 problem where we want to avoid
> > outer_sync()).
> 
> Ah, so you think we can add another hook to NULL out outer_cache.sync
> per platform?

Yes, with the note that mb() no longer guarantees the write being
flushed to RAM but just to the L2 write buffer. If CLCD framebuffer is
the only DMA device, we don't need to worry about such barriers since we
map it as writecombine anyway (hence bufferable).

Unless Linus or Pawel think there are other DMA capable devices in use
here, I'm happy to enable ARM_DMA_MEM_BUFFERABLE for these boards.

On Thu, May 22, 2014 at 5:49 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Thursday 22 May 2014 15:45:03 Catalin Marinas wrote:

>> Anyway, I don't think we have any streaming DMA used on these RealView
>> boards.
>
> Yes, I think that is correct. MMCI, SSP and serial could all use
> DMA but are not configured to do it.

PB1176 has no working DMA (borked DMA hardware), the PB11MPCore
actually has working DMA but only on UART TX and maybe AACI
(haven't tested) no clue about the Cortex boards PBA8 and PBX though.

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

On Thursday 22 May 2014 20:52:38 Linus Walleij wrote:
> On Thu, May 22, 2014 at 5:49 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Thursday 22 May 2014 15:45:03 Catalin Marinas wrote:
> 
> >> Anyway, I don't think we have any streaming DMA used on these RealView
> >> boards.
> >
> > Yes, I think that is correct. MMCI, SSP and serial could all use
> > DMA but are not configured to do it.
> 
> PB1176 has no working DMA (borked DMA hardware), the PB11MPCore
> actually has working DMA but only on UART TX and maybe AACI
> (haven't tested) no clue about the Cortex boards PBA8 and PBX though.

I don't see any DMA data passed to the pl011 driver for PB11MPCore
or any other realview board, so even if it works, we don't use it.

The AACI driver doesn't seem to support DMA.

PBA8 and PBX don't need the hack because they don't have a broken
l2x0.

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

On Thu, May 22, 2014 at 10:35 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Thursday 22 May 2014 20:52:38 Linus Walleij wrote:
>> On Thu, May 22, 2014 at 5:49 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>> > On Thursday 22 May 2014 15:45:03 Catalin Marinas wrote:
>>
>> >> Anyway, I don't think we have any streaming DMA used on these RealView
>> >> boards.
>> >
>> > Yes, I think that is correct. MMCI, SSP and serial could all use
>> > DMA but are not configured to do it.
>>
>> PB1176 has no working DMA (borked DMA hardware), the PB11MPCore
>> actually has working DMA but only on UART TX and maybe AACI
>> (haven't tested) no clue about the Cortex boards PBA8 and PBX though.
>
> I don't see any DMA data passed to the pl011 driver for PB11MPCore
> or any other realview board, so even if it works, we don't use it.

No I was more saying it works in theory... and we actually have
patches to enable it for e.g. memcpy on Versatile and PB11MP
that I floated at times, Russell also experimented around with it.

> The AACI driver doesn't seem to support DMA.

Nope, but the hardware does.

> PBA8 and PBX don't need the hack because they don't have a broken
> l2x0.

True!

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