[1/6] ARM: cygnus: Initial support for Broadcom Cygnus SoC

Commit Message

Scott Branden Oct. 8, 2014, 5:27 a.m. UTC

From: Jonathan Richardson <[e-mail]>

Adds initial support for the Cygnus SoC based on Broadcom’s iProc series.

Reviewed-by: Ray Jui <[e-mail]>
Reviewed-by: Desmond Liu <[e-mail]>
Reviewed-by: JD (Jiandong) Zheng <[e-mail]>
Tested-by: Jonathan Richardson <[e-mail]>
Signed-off-by: Scott Branden <[e-mail]>
---
 arch/arm/mach-bcm/Kconfig      |   31 ++++++++
 arch/arm/mach-bcm/Makefile     |    3 +
 arch/arm/mach-bcm/bcm_cygnus.c |  166 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 200 insertions(+)
 create mode 100644 arch/arm/mach-bcm/bcm_cygnus.c

Comments

Arnd Bergmann Oct. 8, 2014, 7:54 a.m. UTC | #1

On Tuesday 07 October 2014 22:27:00 Scott Branden wrote:
> From: Jonathan Richardson <[e-mail]>
> 
> Adds initial support for the Cygnus SoC based on Broadcom’s iProc series.
> 
> Reviewed-by: Ray Jui <[e-mail]>
> Reviewed-by: Desmond Liu <[e-mail]>
> Reviewed-by: JD (Jiandong) Zheng <[e-mail]>
> Tested-by: Jonathan Richardson <[e-mail]>
> Signed-off-by: Scott Branden <[e-mail]>
> ---
>  arch/arm/mach-bcm/Kconfig      |   31 ++++++++
>  arch/arm/mach-bcm/Makefile     |    3 +
>  arch/arm/mach-bcm/bcm_cygnus.c |  166 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 200 insertions(+)
>  create mode 100644 arch/arm/mach-bcm/bcm_cygnus.c
> 
> diff --git a/arch/arm/mach-bcm/Kconfig b/arch/arm/mach-bcm/Kconfig
> index fc93800..2dd3f78 100644
> --- a/arch/arm/mach-bcm/Kconfig
> +++ b/arch/arm/mach-bcm/Kconfig
> @@ -5,6 +5,37 @@ menuconfig ARCH_BCM
>  
>  if ARCH_BCM
>  
> +config ARCH_BCM_IPROC
> +	bool "Broadcom ARMv7 iProc boards" if ARCH_MULTI_V7
> +	select ARM_GIC
> +	select CACHE_L2X0
> +	select HAVE_ARM_TWD if LOCAL_TIMERS
> +	select HAVE_CLK
> +	select CLKSRC_OF
> +	select CLKSRC_MMIO
> +	select GENERIC_CLOCKEVENTS
> +	select ARM_GLOBAL_TIMER
> +	select ARCH_REQUIRE_GPIOLIB
> +	select ARM_AMBA
> +	select PINCTRL
> +	select DEBUG_UART_8250

A lot of these are implied by ARCH_MULTI_V7, just drop them here.

Some others like DEBUG_UART_8250 should remain user-selectable, if
the platform works without them.

> +	help
> +	  This enables support for systems based on Broadcom IPROC architected SoCs.
> +	  The IPROC complex contains one or more ARM CPUs along with common
> +	  core periperals. Application specific SoCs are created by adding a
> +	  uArchitecture containing peripherals outside of the IPROC complex.
> +	  Currently supported SoCs are Cygnus.
> +
> +menu "iProc SoC based Machine types"
> +	depends on ARCH_BCM_IPROC
> +
> +	config ARCH_BCM_CYGNUS
> +		bool "Support Broadcom Cygnus board"
> +		select USB_ARCH_HAS_EHCI if USB_SUPPORT
> +		help
> +		  Support for Broadcom Cygnus SoC.
> +endmenu

I don't think you need per-board config options. The main option
above should be enough.

> +
> +#define CRMU_MAIL_BOX1      0x03024028
> +#define CRMU_SOFT_RESET_CMD 0xFFFFFFFF

Never hardcode physical register locations in source. This should come
from DT, and get moved into a regular 'reset' device driver.

You probably want to use drivers/power/reset/syscon-reboot.c

> +/* CRU_RESET register */
> +static void * __iomem crmu_mail_box1_reg;
> +
> +#ifdef CONFIG_NEON
> +
> +#define CRU_BASE                  0x1800e000
> +#define CRU_SIZE                  0x34
> +#define CRU_CONTROL_OFFSET        0x0
> +#define CRU_PWRDWN_EN_OFFSET      0x4
> +#define CRU_PWRDWN_STATUS_OFFSET  0x8
> +#define CRU_NEON0_HW_RESET  6
> +#define CRU_CLAMP_ON_NEON0  20
> +#define CRU_PWRONIN_NEON0   21
> +#define CRU_PWRONOUT_NEON0  21
> +#define CRU_PWROKIN_NEON0   22
> +#define CRU_PWROKOUT_NEON0  22
> +#define CRU_STATUS_DELAY_NS 500
> +#define CRU_MAX_RETRY_COUNT 10
> +#define CRU_RETRY_INTVL_US  1
> +
> +/* Power up the NEON/VFPv3 block. */
> +static void bcm_cygnus_powerup_neon(void)
> +{
> +	void * __iomem cru_base = ioremap(CRU_BASE, CRU_SIZE);
> +	u32 reg, i;

Same thing here: this should really use the device node for CRU.

Can you describe what the CRU is? Is this specific to NEON or is
it some general-purpose power management unit?

> +static void __init bcm_cygnus_init(void)
> +{
> +	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
> +
> +	l2x0_of_init(0, ~0UL);

The l2x0_of_init can be removed now, just move the arguments into the
respective fields of the machine descriptor.

> +	crmu_mail_box1_reg = ioremap(CRMU_MAIL_BOX1, SZ_4);
> +	WARN_ON(!crmu_mail_box1_reg);
> +
> +#ifdef CONFIG_NEON
> +	bcm_cygnus_powerup_neon();
> +#endif
> +}

In general, try to avoid #ifdef, use

	if (IS_ENABLED(CONFIG_NEON))
		bcm_cygnus_powerup_neon();

instead.

> +
> +static const char const *bcm_cygnus_dt_compat[] = {
> +	"brcm,cygnus",
> +	NULL,
> +};
> +
> +DT_MACHINE_START(BCM_CYGNUS_DT, "Broadcom Cygnus SoC")
> +	.init_machine = bcm_cygnus_init,
> +	.map_io = debug_ll_io_init,
> +	.dt_compat = bcm_cygnus_dt_compat,
> +	.restart   = bcm_cygnus_restart
> +MACHINE_END

The map_io pointer is unnecessary, and the restart pointer should get
set by the reset driver. I hope we can find a way to avoid the 
bcm_cygnus_init callback as well.

	Arnd

Russell King - ARM Linux Oct. 8, 2014, 8:11 a.m. UTC | #2

On Tue, Oct 07, 2014 at 10:27:00PM -0700, Scott Branden wrote:
> +static void __init bcm_cygnus_init(void)
> +{
> +	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
> +
> +	l2x0_of_init(0, ~0UL);

Please don't explicitly call l2x0 initialisation.  Instead, set the
appropriate l2c members here:

> +DT_MACHINE_START(BCM_CYGNUS_DT, "Broadcom Cygnus SoC")
> +	.init_machine = bcm_cygnus_init,
> +	.map_io = debug_ll_io_init,
> +	.dt_compat = bcm_cygnus_dt_compat,
> +	.restart   = bcm_cygnus_restart
> +MACHINE_END

and let the core code call it at the appropriate time.  Thanks.

Scott Branden Oct. 8, 2014, 11:17 a.m. UTC | #3

On 14-10-08 01:11 AM, Russell King - ARM Linux wrote:
> On Tue, Oct 07, 2014 at 10:27:00PM -0700, Scott Branden wrote:
>> +static void __init bcm_cygnus_init(void)
>> +{
>> +	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
>> +
>> +	l2x0_of_init(0, ~0UL);
>
> Please don't explicitly call l2x0 initialisation.  Instead, set the
> appropriate l2c members here:
>
>> +DT_MACHINE_START(BCM_CYGNUS_DT, "Broadcom Cygnus SoC")
>> +	.init_machine = bcm_cygnus_init,
>> +	.map_io = debug_ll_io_init,
>> +	.dt_compat = bcm_cygnus_dt_compat,
>> +	.restart   = bcm_cygnus_restart
>> +MACHINE_END
>
> and let the core code call it at the appropriate time.  Thanks.
>
Thanks - was unaware of this functionality.  Will add .l2c_aux_val and 
.l2c_aux mask to DT_MATCHINE_START.  BUT, what happens when we need to 
add trustzone support and make SMC call to secure monitor?

Russell King - ARM Linux Oct. 8, 2014, 11:41 a.m. UTC | #4

On Wed, Oct 08, 2014 at 04:17:29AM -0700, Scott Branden wrote:
> Thanks - was unaware of this functionality.  Will add .l2c_aux_val and  
> .l2c_aux mask to DT_MATCHINE_START.  BUT, what happens when we need to  
> add trustzone support and make SMC call to secure monitor?

You will then need to implement the .l2c_write_sec initialiser in the
same place.

Note that there's work to revise the trustzone support in this area
which will probably be merged for 3.19.

Scott Branden Oct. 8, 2014, 12:27 p.m. UTC | #5

Thanks for the review - comments inline.

On 14-10-08 12:54 AM, Arnd Bergmann wrote:
> On Tuesday 07 October 2014 22:27:00 Scott Branden wrote:
>> From: Jonathan Richardson <[e-mail]>
>>
>> Adds initial support for the Cygnus SoC based on Broadcom’s iProc series.
>>
>> Reviewed-by: Ray Jui <[e-mail]>
>> Reviewed-by: Desmond Liu <[e-mail]>
>> Reviewed-by: JD (Jiandong) Zheng <[e-mail]>
>> Tested-by: Jonathan Richardson <[e-mail]>
>> Signed-off-by: Scott Branden <[e-mail]>
>> ---
>>   arch/arm/mach-bcm/Kconfig      |   31 ++++++++
>>   arch/arm/mach-bcm/Makefile     |    3 +
>>   arch/arm/mach-bcm/bcm_cygnus.c |  166 ++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 200 insertions(+)
>>   create mode 100644 arch/arm/mach-bcm/bcm_cygnus.c
>>
>> diff --git a/arch/arm/mach-bcm/Kconfig b/arch/arm/mach-bcm/Kconfig
>> index fc93800..2dd3f78 100644
>> --- a/arch/arm/mach-bcm/Kconfig
>> +++ b/arch/arm/mach-bcm/Kconfig
>> @@ -5,6 +5,37 @@ menuconfig ARCH_BCM
>>
>>   if ARCH_BCM
>>
>> +config ARCH_BCM_IPROC
>> +	bool "Broadcom ARMv7 iProc boards" if ARCH_MULTI_V7
>> +	select ARM_GIC
>> +	select CACHE_L2X0
>> +	select HAVE_ARM_TWD if LOCAL_TIMERS
>> +	select HAVE_CLK
>> +	select CLKSRC_OF
>> +	select CLKSRC_MMIO
>> +	select GENERIC_CLOCKEVENTS
>> +	select ARM_GLOBAL_TIMER
>> +	select ARCH_REQUIRE_GPIOLIB
>> +	select ARM_AMBA
>> +	select PINCTRL
>> +	select DEBUG_UART_8250
>
> A lot of these are implied by ARCH_MULTI_V7, just drop them here.
>
> Some others like DEBUG_UART_8250 should remain user-selectable, if
> the platform works without them.
>
Will review.  It looks like DEBUG_UART_8250 actually has to move to 
Kconfig.debug as that is where everyone else selects it.
>> +	help
>> +	  This enables support for systems based on Broadcom IPROC architected SoCs.
>> +	  The IPROC complex contains one or more ARM CPUs along with common
>> +	  core periperals. Application specific SoCs are created by adding a
>> +	  uArchitecture containing peripherals outside of the IPROC complex.
>> +	  Currently supported SoCs are Cygnus.
>> +
>> +menu "iProc SoC based Machine types"
>> +	depends on ARCH_BCM_IPROC
>> +
>> +	config ARCH_BCM_CYGNUS
>> +		bool "Support Broadcom Cygnus board"
>> +		select USB_ARCH_HAS_EHCI if USB_SUPPORT
>> +		help
>> +		  Support for Broadcom Cygnus SoC.
>> +endmenu
>
> I don't think you need per-board config options. The main option
> above should be enough.
This is not a per-board config option.  This is actually a per-SoC 
uArchtecture selection.  More major uArchectures will be added to the 
IPROC.  Will Change comment to "Support Broadcom Cygnus SoC"
>
>> +
>> +#define CRMU_MAIL_BOX1      0x03024028
>> +#define CRMU_SOFT_RESET_CMD 0xFFFFFFFF
>
> Never hardcode physical register locations in source. This should come
> from DT, and get moved into a regular 'reset' device driver.
>
> You probably want to use drivers/power/reset/syscon-reboot.c
>
OK, thanks for advice on where to put reset device driver.  Will 
investigate.
>> +/* CRU_RESET register */
>> +static void * __iomem crmu_mail_box1_reg;
>> +
>> +#ifdef CONFIG_NEON
>> +
>> +#define CRU_BASE                  0x1800e000
>> +#define CRU_SIZE                  0x34
>> +#define CRU_CONTROL_OFFSET        0x0
>> +#define CRU_PWRDWN_EN_OFFSET      0x4
>> +#define CRU_PWRDWN_STATUS_OFFSET  0x8
>> +#define CRU_NEON0_HW_RESET  6
>> +#define CRU_CLAMP_ON_NEON0  20
>> +#define CRU_PWRONIN_NEON0   21
>> +#define CRU_PWRONOUT_NEON0  21
>> +#define CRU_PWROKIN_NEON0   22
>> +#define CRU_PWROKOUT_NEON0  22
>> +#define CRU_STATUS_DELAY_NS 500
>> +#define CRU_MAX_RETRY_COUNT 10
>> +#define CRU_RETRY_INTVL_US  1
>> +
>> +/* Power up the NEON/VFPv3 block. */
>> +static void bcm_cygnus_powerup_neon(void)
>> +{
>> +	void * __iomem cru_base = ioremap(CRU_BASE, CRU_SIZE);
>> +	u32 reg, i;
>
> Same thing here: this should really use the device node for CRU.
>
> Can you describe what the CRU is? Is this specific to NEON or is
> it some general-purpose power management unit?
>
It's a central resource unit with a lot of random registers to perform 
various operations.  To reduce confusion I'll probably move this out of 
the kernel init and into the bootloader.  This will simplify the kernel 
init.
>> +static void __init bcm_cygnus_init(void)
>> +{
>> +	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
>> +
>> +	l2x0_of_init(0, ~0UL);
>
> The l2x0_of_init can be removed now, just move the arguments into the
> respective fields of the machine descriptor.
>
Yes, thanks for pointing this out.
>> +	crmu_mail_box1_reg = ioremap(CRMU_MAIL_BOX1, SZ_4);
>> +	WARN_ON(!crmu_mail_box1_reg);
>> +
>> +#ifdef CONFIG_NEON
>> +	bcm_cygnus_powerup_neon();
>> +#endif
>> +}
>
> In general, try to avoid #ifdef, use
>
> 	if (IS_ENABLED(CONFIG_NEON))
> 		bcm_cygnus_powerup_neon();
>
> instead.
>
>> +
>> +static const char const *bcm_cygnus_dt_compat[] = {
>> +	"brcm,cygnus",
>> +	NULL,
>> +};
>> +
>> +DT_MACHINE_START(BCM_CYGNUS_DT, "Broadcom Cygnus SoC")
>> +	.init_machine = bcm_cygnus_init,
>> +	.map_io = debug_ll_io_init,
>> +	.dt_compat = bcm_cygnus_dt_compat,
>> +	.restart   = bcm_cygnus_restart
>> +MACHINE_END
>
> The map_io pointer is unnecessary, and the restart pointer should get
> set by the reset driver. I hope we can find a way to avoid the
> bcm_cygnus_init callback as well.
bcm_cygnus_init callback can be removed by moving initialization to 
bootloader.
>
> 	Arnd
>

Rob Herring Oct. 8, 2014, 1:10 p.m. UTC | #6

On Wed, Oct 8, 2014 at 12:27 AM, Scott Branden <[e-mail]> wrote:
> From: Jonathan Richardson <[e-mail]>
>
> Adds initial support for the Cygnus SoC based on Broadcom’s iProc series.
>
> Reviewed-by: Ray Jui <[e-mail]>
> Reviewed-by: Desmond Liu <[e-mail]>
> Reviewed-by: JD (Jiandong) Zheng <[e-mail]>
> Tested-by: Jonathan Richardson <[e-mail]>
> Signed-off-by: Scott Branden <[e-mail]>
> ---
>  arch/arm/mach-bcm/Kconfig      |   31 ++++++++
>  arch/arm/mach-bcm/Makefile     |    3 +
>  arch/arm/mach-bcm/bcm_cygnus.c |  166 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 200 insertions(+)
>  create mode 100644 arch/arm/mach-bcm/bcm_cygnus.c
>
> diff --git a/arch/arm/mach-bcm/Kconfig b/arch/arm/mach-bcm/Kconfig
> index fc93800..2dd3f78 100644
> --- a/arch/arm/mach-bcm/Kconfig
> +++ b/arch/arm/mach-bcm/Kconfig
> @@ -5,6 +5,37 @@ menuconfig ARCH_BCM
>
>  if ARCH_BCM
>
> +config ARCH_BCM_IPROC
> +       bool "Broadcom ARMv7 iProc boards" if ARCH_MULTI_V7
> +       select ARM_GIC
> +       select CACHE_L2X0
> +       select HAVE_ARM_TWD if LOCAL_TIMERS

LOCAL_TIMERS does not exist anymore.

> +       select HAVE_CLK

Selected already by multi-platform.

> +       select CLKSRC_OF
> +       select CLKSRC_MMIO

These should be selected by the timers that need these rather than the platform.

> +       select GENERIC_CLOCKEVENTS

Selected already by multi-platform.

> +       select ARM_GLOBAL_TIMER
> +       select ARCH_REQUIRE_GPIOLIB
> +       select ARM_AMBA
> +       select PINCTRL
> +       select DEBUG_UART_8250

This entry should not be a select. It will break multi-platform.

Sort the select entries alphabetically.

> +       help
> +         This enables support for systems based on Broadcom IPROC architected SoCs.
> +         The IPROC complex contains one or more ARM CPUs along with common
> +         core periperals. Application specific SoCs are created by adding a
> +         uArchitecture containing peripherals outside of the IPROC complex.
> +         Currently supported SoCs are Cygnus.
> +
> +menu "iProc SoC based Machine types"
> +       depends on ARCH_BCM_IPROC
> +
> +       config ARCH_BCM_CYGNUS
> +               bool "Support Broadcom Cygnus board"
> +               select USB_ARCH_HAS_EHCI if USB_SUPPORT
> +               help
> +                 Support for Broadcom Cygnus SoC.
> +endmenu
> +
>  config ARCH_BCM_MOBILE
>         bool "Broadcom Mobile SoC Support" if ARCH_MULTI_V7
>         select ARCH_REQUIRE_GPIOLIB
> diff --git a/arch/arm/mach-bcm/Makefile b/arch/arm/mach-bcm/Makefile
> index b19a396..46e092a 100644
> --- a/arch/arm/mach-bcm/Makefile
> +++ b/arch/arm/mach-bcm/Makefile
> @@ -10,6 +10,9 @@
>  # of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>  # GNU General Public License for more details.
>
> +# Cygnus
> +obj-$(CONFIG_ARCH_BCM_CYGNUS) +=  bcm_cygnus.o
> +
>  # BCM281XX
>  obj-$(CONFIG_ARCH_BCM_281XX)   += board_bcm281xx.o
>
> diff --git a/arch/arm/mach-bcm/bcm_cygnus.c b/arch/arm/mach-bcm/bcm_cygnus.c
> new file mode 100644
> index 0000000..8e430ed
> --- /dev/null
> +++ b/arch/arm/mach-bcm/bcm_cygnus.c
> @@ -0,0 +1,166 @@
> +/*
> + * Copyright 2014 Broadcom Corporation.  All rights reserved.
> + *
> + * Unless you and Broadcom execute a separate written software license
> + * agreement governing use of this software, this software is licensed to you
> + * under the terms of the GNU General Public License as
> + * published by the Free Software Foundation version 2.
> + *
> + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
> + * kind, whether express or implied; without even the implied warranty
> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/of_address.h>
> +#include <linux/of_platform.h>
> +#include <linux/clocksource.h>
> +#include <linux/clk-provider.h>
> +#include <linux/delay.h>
> +#include <asm/mach/arch.h>
> +#include <asm/mach/map.h>
> +#include <asm/proc-fns.h>
> +#include <asm/hardware/cache-l2x0.h>
> +
> +#define CRMU_MAIL_BOX1      0x03024028
> +#define CRMU_SOFT_RESET_CMD 0xFFFFFFFF
> +
> +/* CRU_RESET register */
> +static void * __iomem crmu_mail_box1_reg;
> +
> +#ifdef CONFIG_NEON
> +
> +#define CRU_BASE                  0x1800e000
> +#define CRU_SIZE                  0x34
> +#define CRU_CONTROL_OFFSET        0x0
> +#define CRU_PWRDWN_EN_OFFSET      0x4
> +#define CRU_PWRDWN_STATUS_OFFSET  0x8
> +#define CRU_NEON0_HW_RESET  6
> +#define CRU_CLAMP_ON_NEON0  20
> +#define CRU_PWRONIN_NEON0   21
> +#define CRU_PWRONOUT_NEON0  21
> +#define CRU_PWROKIN_NEON0   22
> +#define CRU_PWROKOUT_NEON0  22
> +#define CRU_STATUS_DELAY_NS 500
> +#define CRU_MAX_RETRY_COUNT 10
> +#define CRU_RETRY_INTVL_US  1
> +
> +/* Power up the NEON/VFPv3 block. */
> +static void bcm_cygnus_powerup_neon(void)
> +{
> +       void * __iomem cru_base = ioremap(CRU_BASE, CRU_SIZE);
> +       u32 reg, i;
> +
> +       if (WARN_ON(!cru_base))
> +               return;
> +
> +       /* De-assert the neon hardware block reset */
> +       reg = readl(cru_base + CRU_CONTROL_OFFSET);
> +       reg &= ~(1 << CRU_NEON0_HW_RESET);
> +       writel(reg, cru_base + CRU_CONTROL_OFFSET);
> +
> +       /* Assert the power ON register bit */
> +       reg = readl(cru_base + CRU_PWRDWN_EN_OFFSET);
> +       reg |= (1 << CRU_PWRONIN_NEON0);
> +       writel(reg, cru_base + CRU_PWRDWN_EN_OFFSET);
> +
> +       /*
> +        * Wait up to 10 usec in 1 usec increments for the
> +        * status register to acknowledge the power ON assert
> +        */
> +       for (i = 0; i < CRU_MAX_RETRY_COUNT; i++) {
> +               reg = readl(cru_base + CRU_PWRDWN_STATUS_OFFSET);
> +               if (reg & CRU_PWRONOUT_NEON0)
> +                       break;
> +
> +               udelay(CRU_RETRY_INTVL_US);
> +       }
> +
> +       if (WARN_ON(i == CRU_MAX_RETRY_COUNT))
> +               goto neon_unmap;
> +
> +       /* Wait 0.5 usec = 500 nsec */
> +       ndelay(CRU_STATUS_DELAY_NS);
> +
> +       /* Assert the power OK register bit */
> +       reg = readl(cru_base + CRU_PWRDWN_EN_OFFSET);
> +       reg |= (1 << CRU_PWROKIN_NEON0);
> +       writel(reg, cru_base + CRU_PWRDWN_EN_OFFSET);
> +
> +       /*
> +        * Wait up to 10 usec in 1 usec increments for the
> +        * status register to acknowledge the power OK assert
> +        */
> +       for (i = 0; i < CRU_MAX_RETRY_COUNT; i++) {
> +               reg = readl(cru_base + CRU_PWRDWN_STATUS_OFFSET);
> +               if (reg & CRU_PWROKOUT_NEON0)
> +                       break;
> +
> +               udelay(CRU_RETRY_INTVL_US);
> +       }
> +
> +       if (WARN_ON(i == CRU_MAX_RETRY_COUNT))
> +               goto neon_unmap;
> +
> +       /* Wait 0.5 usec = 500 nsec */
> +       ndelay(CRU_STATUS_DELAY_NS);
> +
> +       /* Set the logic clamp for the neon block */
> +       reg = readl(cru_base + CRU_PWRDWN_EN_OFFSET);
> +       reg &= ~(1 << CRU_CLAMP_ON_NEON0);
> +       writel(reg, cru_base + CRU_PWRDWN_EN_OFFSET);
> +
> +       /* Wait 0.5 usec = 500 nsec */
> +       ndelay(CRU_STATUS_DELAY_NS);
> +
> +       /* Reset the neon hardware block */
> +       reg = readl(cru_base + CRU_CONTROL_OFFSET);
> +       reg |= (1 << CRU_NEON0_HW_RESET);
> +       writel(reg, cru_base + CRU_CONTROL_OFFSET);
> +
> +neon_unmap:
> +       iounmap(cru_base);
> +}
> +#endif /* CONFIG_NEON */

Is this a single core chip? If not, it seems like all this would
change when you add SMP support.

Rob

Arnd Bergmann Oct. 8, 2014, 1:28 p.m. UTC | #7

On Wednesday 08 October 2014 05:27:24 Scott Branden wrote:
> >> diff --git a/arch/arm/mach-bcm/Kconfig b/arch/arm/mach-bcm/Kconfig
> >> index fc93800..2dd3f78 100644
> >> --- a/arch/arm/mach-bcm/Kconfig
> >> +++ b/arch/arm/mach-bcm/Kconfig
> >> @@ -5,6 +5,37 @@ menuconfig ARCH_BCM
> >>
> >>   if ARCH_BCM
> >>
> >> +config ARCH_BCM_IPROC
> >> +	bool "Broadcom ARMv7 iProc boards" if ARCH_MULTI_V7
> >> +	select ARM_GIC
> >> +	select CACHE_L2X0
> >> +	select HAVE_ARM_TWD if LOCAL_TIMERS
> >> +	select HAVE_CLK
> >> +	select CLKSRC_OF
> >> +	select CLKSRC_MMIO
> >> +	select GENERIC_CLOCKEVENTS
> >> +	select ARM_GLOBAL_TIMER
> >> +	select ARCH_REQUIRE_GPIOLIB
> >> +	select ARM_AMBA
> >> +	select PINCTRL
> >> +	select DEBUG_UART_8250
> >
> > A lot of these are implied by ARCH_MULTI_V7, just drop them here.
> >
> > Some others like DEBUG_UART_8250 should remain user-selectable, if
> > the platform works without them.
> >
> Will review.  It looks like DEBUG_UART_8250 actually has to move to 
> Kconfig.debug as that is where everyone else selects it.

Actually I think you also need to use DEBUG_LL_UART_8250 rather than
DEBUG_UART_8250.

> >> +	help
> >> +	  This enables support for systems based on Broadcom IPROC architected SoCs.
> >> +	  The IPROC complex contains one or more ARM CPUs along with common
> >> +	  core periperals. Application specific SoCs are created by adding a
> >> +	  uArchitecture containing peripherals outside of the IPROC complex.
> >> +	  Currently supported SoCs are Cygnus.
> >> +
> >> +menu "iProc SoC based Machine types"
> >> +	depends on ARCH_BCM_IPROC
> >> +
> >> +	config ARCH_BCM_CYGNUS
> >> +		bool "Support Broadcom Cygnus board"
> >> +		select USB_ARCH_HAS_EHCI if USB_SUPPORT
> >> +		help
> >> +		  Support for Broadcom Cygnus SoC.
> >> +endmenu
> >
> > I don't think you need per-board config options. The main option
> > above should be enough.
> This is not a per-board config option.  This is actually a per-SoC 
> uArchtecture selection.  More major uArchectures will be added to the 
> IPROC.  Will Change comment to "Support Broadcom Cygnus SoC"

Ok, sounds fine, but remove ARCH_BCM_IPROC then. There should be
no need for a three-level deep hierarchy (BCM -> IPROC -> CYGNUS)


> >> +/* CRU_RESET register */
> >> +static void * __iomem crmu_mail_box1_reg;
> >> +
> >> +#ifdef CONFIG_NEON
> >> +
> >> +#define CRU_BASE                  0x1800e000
> >> +#define CRU_SIZE                  0x34
> >> +#define CRU_CONTROL_OFFSET        0x0
> >> +#define CRU_PWRDWN_EN_OFFSET      0x4
> >> +#define CRU_PWRDWN_STATUS_OFFSET  0x8
> >> +#define CRU_NEON0_HW_RESET  6
> >> +#define CRU_CLAMP_ON_NEON0  20
> >> +#define CRU_PWRONIN_NEON0   21
> >> +#define CRU_PWRONOUT_NEON0  21
> >> +#define CRU_PWROKIN_NEON0   22
> >> +#define CRU_PWROKOUT_NEON0  22
> >> +#define CRU_STATUS_DELAY_NS 500
> >> +#define CRU_MAX_RETRY_COUNT 10
> >> +#define CRU_RETRY_INTVL_US  1
> >> +
> >> +/* Power up the NEON/VFPv3 block. */
> >> +static void bcm_cygnus_powerup_neon(void)
> >> +{
> >> +	void * __iomem cru_base = ioremap(CRU_BASE, CRU_SIZE);
> >> +	u32 reg, i;
> >
> > Same thing here: this should really use the device node for CRU.
> >
> > Can you describe what the CRU is? Is this specific to NEON or is
> > it some general-purpose power management unit?
> >
> It's a central resource unit with a lot of random registers to perform 
> various operations.  To reduce confusion I'll probably move this out of 
> the kernel init and into the bootloader.  This will simplify the kernel 
> init.

That would help too, yes.

> >> +
> >> +static const char const *bcm_cygnus_dt_compat[] = {
> >> +	"brcm,cygnus",
> >> +	NULL,
> >> +};
> >> +
> >> +DT_MACHINE_START(BCM_CYGNUS_DT, "Broadcom Cygnus SoC")
> >> +	.init_machine = bcm_cygnus_init,
> >> +	.map_io = debug_ll_io_init,
> >> +	.dt_compat = bcm_cygnus_dt_compat,
> >> +	.restart   = bcm_cygnus_restart
> >> +MACHINE_END
> >
> > The map_io pointer is unnecessary, and the restart pointer should get
> > set by the reset driver. I hope we can find a way to avoid the
> > bcm_cygnus_init callback as well.
> bcm_cygnus_init callback can be removed by moving initialization to 
> bootloader.

Ok, perfect!

	Arnd

Scott Branden Oct. 8, 2014, 4:27 p.m. UTC | #8

On 14-10-08 06:28 AM, Arnd Bergmann wrote:
> On Wednesday 08 October 2014 05:27:24 Scott Branden wrote:
>>>> diff --git a/arch/arm/mach-bcm/Kconfig b/arch/arm/mach-bcm/Kconfig
>>>> index fc93800..2dd3f78 100644
>>>> --- a/arch/arm/mach-bcm/Kconfig
>>>> +++ b/arch/arm/mach-bcm/Kconfig
>>>> @@ -5,6 +5,37 @@ menuconfig ARCH_BCM
>>>>
>>>>    if ARCH_BCM
>>>>
>>>> +config ARCH_BCM_IPROC
>>>> +	bool "Broadcom ARMv7 iProc boards" if ARCH_MULTI_V7
>>>> +	select ARM_GIC
>>>> +	select CACHE_L2X0
>>>> +	select HAVE_ARM_TWD if LOCAL_TIMERS
>>>> +	select HAVE_CLK
>>>> +	select CLKSRC_OF
>>>> +	select CLKSRC_MMIO
>>>> +	select GENERIC_CLOCKEVENTS
>>>> +	select ARM_GLOBAL_TIMER
>>>> +	select ARCH_REQUIRE_GPIOLIB
>>>> +	select ARM_AMBA
>>>> +	select PINCTRL
>>>> +	select DEBUG_UART_8250
>>>
>>> A lot of these are implied by ARCH_MULTI_V7, just drop them here.
>>>
>>> Some others like DEBUG_UART_8250 should remain user-selectable, if
>>> the platform works without them.
>>>
>> Will review.  It looks like DEBUG_UART_8250 actually has to move to
>> Kconfig.debug as that is where everyone else selects it.
>
> Actually I think you also need to use DEBUG_LL_UART_8250 rather than
> DEBUG_UART_8250.
>
>>>> +	help
>>>> +	  This enables support for systems based on Broadcom IPROC architected SoCs.
>>>> +	  The IPROC complex contains one or more ARM CPUs along with common
>>>> +	  core periperals. Application specific SoCs are created by adding a
>>>> +	  uArchitecture containing peripherals outside of the IPROC complex.
>>>> +	  Currently supported SoCs are Cygnus.
>>>> +
>>>> +menu "iProc SoC based Machine types"
>>>> +	depends on ARCH_BCM_IPROC
>>>> +
>>>> +	config ARCH_BCM_CYGNUS
>>>> +		bool "Support Broadcom Cygnus board"
>>>> +		select USB_ARCH_HAS_EHCI if USB_SUPPORT
>>>> +		help
>>>> +		  Support for Broadcom Cygnus SoC.
>>>> +endmenu
>>>
>>> I don't think you need per-board config options. The main option
>>> above should be enough.
>> This is not a per-board config option.  This is actually a per-SoC
>> uArchtecture selection.  More major uArchectures will be added to the
>> IPROC.  Will Change comment to "Support Broadcom Cygnus SoC"
>
> Ok, sounds fine, but remove ARCH_BCM_IPROC then. There should be
> no need for a three-level deep hierarchy (BCM -> IPROC -> CYGNUS)
>
I do not need a 3-deep hierarchy, I need a 2-deep hierarchy for IPROC 
and CYGNUS (and future SoCs that have IPROC Architecture in common).  I 
can move IPROC out of the mach-bcm directory if you like a create a new 
directory?  But it looks like the purpose of mach-bcm is to consolidate 
all Broadcom chipsets in it?
>
>>>> +/* CRU_RESET register */
>>>> +static void * __iomem crmu_mail_box1_reg;
>>>> +
>>>> +#ifdef CONFIG_NEON
>>>> +
>>>> +#define CRU_BASE                  0x1800e000
>>>> +#define CRU_SIZE                  0x34
>>>> +#define CRU_CONTROL_OFFSET        0x0
>>>> +#define CRU_PWRDWN_EN_OFFSET      0x4
>>>> +#define CRU_PWRDWN_STATUS_OFFSET  0x8
>>>> +#define CRU_NEON0_HW_RESET  6
>>>> +#define CRU_CLAMP_ON_NEON0  20
>>>> +#define CRU_PWRONIN_NEON0   21
>>>> +#define CRU_PWRONOUT_NEON0  21
>>>> +#define CRU_PWROKIN_NEON0   22
>>>> +#define CRU_PWROKOUT_NEON0  22
>>>> +#define CRU_STATUS_DELAY_NS 500
>>>> +#define CRU_MAX_RETRY_COUNT 10
>>>> +#define CRU_RETRY_INTVL_US  1
>>>> +
>>>> +/* Power up the NEON/VFPv3 block. */
>>>> +static void bcm_cygnus_powerup_neon(void)
>>>> +{
>>>> +	void * __iomem cru_base = ioremap(CRU_BASE, CRU_SIZE);
>>>> +	u32 reg, i;
>>>
>>> Same thing here: this should really use the device node for CRU.
>>>
>>> Can you describe what the CRU is? Is this specific to NEON or is
>>> it some general-purpose power management unit?
>>>
>> It's a central resource unit with a lot of random registers to perform
>> various operations.  To reduce confusion I'll probably move this out of
>> the kernel init and into the bootloader.  This will simplify the kernel
>> init.
>
> That would help too, yes.
>
>>>> +
>>>> +static const char const *bcm_cygnus_dt_compat[] = {
>>>> +	"brcm,cygnus",
>>>> +	NULL,
>>>> +};
>>>> +
>>>> +DT_MACHINE_START(BCM_CYGNUS_DT, "Broadcom Cygnus SoC")
>>>> +	.init_machine = bcm_cygnus_init,
>>>> +	.map_io = debug_ll_io_init,
>>>> +	.dt_compat = bcm_cygnus_dt_compat,
>>>> +	.restart   = bcm_cygnus_restart
>>>> +MACHINE_END
>>>
>>> The map_io pointer is unnecessary, and the restart pointer should get
>>> set by the reset driver. I hope we can find a way to avoid the
>>> bcm_cygnus_init callback as well.
>> bcm_cygnus_init callback can be removed by moving initialization to
>> bootloader.
>
> Ok, perfect!
>
> 	Arnd
>

Scott Branden Oct. 8, 2014, 4:34 p.m. UTC | #9

Thanks for comments - inline.

On 14-10-08 06:10 AM, Rob Herring wrote:
> On Wed, Oct 8, 2014 at 12:27 AM, Scott Branden <[e-mail]> wrote:
>> From: Jonathan Richardson <[e-mail]>
>>
>> Adds initial support for the Cygnus SoC based on Broadcom’s iProc series.
>>
>> Reviewed-by: Ray Jui <[e-mail]>
>> Reviewed-by: Desmond Liu <[e-mail]>
>> Reviewed-by: JD (Jiandong) Zheng <[e-mail]>
>> Tested-by: Jonathan Richardson <[e-mail]>
>> Signed-off-by: Scott Branden <[e-mail]>
>> ---
>>   arch/arm/mach-bcm/Kconfig      |   31 ++++++++
>>   arch/arm/mach-bcm/Makefile     |    3 +
>>   arch/arm/mach-bcm/bcm_cygnus.c |  166 ++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 200 insertions(+)
>>   create mode 100644 arch/arm/mach-bcm/bcm_cygnus.c
>>
>> diff --git a/arch/arm/mach-bcm/Kconfig b/arch/arm/mach-bcm/Kconfig
>> index fc93800..2dd3f78 100644
>> --- a/arch/arm/mach-bcm/Kconfig
>> +++ b/arch/arm/mach-bcm/Kconfig
>> @@ -5,6 +5,37 @@ menuconfig ARCH_BCM
>>
>>   if ARCH_BCM
>>
>> +config ARCH_BCM_IPROC
>> +       bool "Broadcom ARMv7 iProc boards" if ARCH_MULTI_V7
>> +       select ARM_GIC
>> +       select CACHE_L2X0
>> +       select HAVE_ARM_TWD if LOCAL_TIMERS
>
> LOCAL_TIMERS does not exist anymore.
Will change to if SMP so it will work for other iproc chips that have 
SMP going forward.
>
>> +       select HAVE_CLK
>
> Selected already by multi-platform.
>
>> +       select CLKSRC_OF
>> +       select CLKSRC_MMIO
>
> These should be selected by the timers that need these rather than the platform.
>
OK
>> +       select GENERIC_CLOCKEVENTS
>
> Selected already by multi-platform.
>
>> +       select ARM_GLOBAL_TIMER
>> +       select ARCH_REQUIRE_GPIOLIB
>> +       select ARM_AMBA
>> +       select PINCTRL
>> +       select DEBUG_UART_8250
>
> This entry should not be a select. It will break multi-platform.
Yes, this select will be removed from here in next version and moved to 
kconfig.debug as other platforms do.
>
> Sort the select entries alphabetically.
>
>> +       help
>> +         This enables support for systems based on Broadcom IPROC architected SoCs.
>> +         The IPROC complex contains one or more ARM CPUs along with common
>> +         core periperals. Application specific SoCs are created by adding a
>> +         uArchitecture containing peripherals outside of the IPROC complex.
>> +         Currently supported SoCs are Cygnus.
>> +
>> +menu "iProc SoC based Machine types"
>> +       depends on ARCH_BCM_IPROC
>> +
>> +       config ARCH_BCM_CYGNUS
>> +               bool "Support Broadcom Cygnus board"
>> +               select USB_ARCH_HAS_EHCI if USB_SUPPORT
>> +               help
>> +                 Support for Broadcom Cygnus SoC.
>> +endmenu
>> +
>>   config ARCH_BCM_MOBILE
>>          bool "Broadcom Mobile SoC Support" if ARCH_MULTI_V7
>>          select ARCH_REQUIRE_GPIOLIB
>> diff --git a/arch/arm/mach-bcm/Makefile b/arch/arm/mach-bcm/Makefile
>> index b19a396..46e092a 100644
>> --- a/arch/arm/mach-bcm/Makefile
>> +++ b/arch/arm/mach-bcm/Makefile
>> @@ -10,6 +10,9 @@
>>   # of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>   # GNU General Public License for more details.
>>
>> +# Cygnus
>> +obj-$(CONFIG_ARCH_BCM_CYGNUS) +=  bcm_cygnus.o
>> +
>>   # BCM281XX
>>   obj-$(CONFIG_ARCH_BCM_281XX)   += board_bcm281xx.o
>>
>> diff --git a/arch/arm/mach-bcm/bcm_cygnus.c b/arch/arm/mach-bcm/bcm_cygnus.c
>> new file mode 100644
>> index 0000000..8e430ed
>> --- /dev/null
>> +++ b/arch/arm/mach-bcm/bcm_cygnus.c
>> @@ -0,0 +1,166 @@
>> +/*
>> + * Copyright 2014 Broadcom Corporation.  All rights reserved.
>> + *
>> + * Unless you and Broadcom execute a separate written software license
>> + * agreement governing use of this software, this software is licensed to you
>> + * under the terms of the GNU General Public License as
>> + * published by the Free Software Foundation version 2.
>> + *
>> + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
>> + * kind, whether express or implied; without even the implied warranty
>> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#include <linux/of_address.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/clocksource.h>
>> +#include <linux/clk-provider.h>
>> +#include <linux/delay.h>
>> +#include <asm/mach/arch.h>
>> +#include <asm/mach/map.h>
>> +#include <asm/proc-fns.h>
>> +#include <asm/hardware/cache-l2x0.h>
>> +
>> +#define CRMU_MAIL_BOX1      0x03024028
>> +#define CRMU_SOFT_RESET_CMD 0xFFFFFFFF
>> +
>> +/* CRU_RESET register */
>> +static void * __iomem crmu_mail_box1_reg;
>> +
>> +#ifdef CONFIG_NEON
>> +
>> +#define CRU_BASE                  0x1800e000
>> +#define CRU_SIZE                  0x34
>> +#define CRU_CONTROL_OFFSET        0x0
>> +#define CRU_PWRDWN_EN_OFFSET      0x4
>> +#define CRU_PWRDWN_STATUS_OFFSET  0x8
>> +#define CRU_NEON0_HW_RESET  6
>> +#define CRU_CLAMP_ON_NEON0  20
>> +#define CRU_PWRONIN_NEON0   21
>> +#define CRU_PWRONOUT_NEON0  21
>> +#define CRU_PWROKIN_NEON0   22
>> +#define CRU_PWROKOUT_NEON0  22
>> +#define CRU_STATUS_DELAY_NS 500
>> +#define CRU_MAX_RETRY_COUNT 10
>> +#define CRU_RETRY_INTVL_US  1
>> +
>> +/* Power up the NEON/VFPv3 block. */
>> +static void bcm_cygnus_powerup_neon(void)
>> +{
>> +       void * __iomem cru_base = ioremap(CRU_BASE, CRU_SIZE);
>> +       u32 reg, i;
>> +
>> +       if (WARN_ON(!cru_base))
>> +               return;
>> +
>> +       /* De-assert the neon hardware block reset */
>> +       reg = readl(cru_base + CRU_CONTROL_OFFSET);
>> +       reg &= ~(1 << CRU_NEON0_HW_RESET);
>> +       writel(reg, cru_base + CRU_CONTROL_OFFSET);
>> +
>> +       /* Assert the power ON register bit */
>> +       reg = readl(cru_base + CRU_PWRDWN_EN_OFFSET);
>> +       reg |= (1 << CRU_PWRONIN_NEON0);
>> +       writel(reg, cru_base + CRU_PWRDWN_EN_OFFSET);
>> +
>> +       /*
>> +        * Wait up to 10 usec in 1 usec increments for the
>> +        * status register to acknowledge the power ON assert
>> +        */
>> +       for (i = 0; i < CRU_MAX_RETRY_COUNT; i++) {
>> +               reg = readl(cru_base + CRU_PWRDWN_STATUS_OFFSET);
>> +               if (reg & CRU_PWRONOUT_NEON0)
>> +                       break;
>> +
>> +               udelay(CRU_RETRY_INTVL_US);
>> +       }
>> +
>> +       if (WARN_ON(i == CRU_MAX_RETRY_COUNT))
>> +               goto neon_unmap;
>> +
>> +       /* Wait 0.5 usec = 500 nsec */
>> +       ndelay(CRU_STATUS_DELAY_NS);
>> +
>> +       /* Assert the power OK register bit */
>> +       reg = readl(cru_base + CRU_PWRDWN_EN_OFFSET);
>> +       reg |= (1 << CRU_PWROKIN_NEON0);
>> +       writel(reg, cru_base + CRU_PWRDWN_EN_OFFSET);
>> +
>> +       /*
>> +        * Wait up to 10 usec in 1 usec increments for the
>> +        * status register to acknowledge the power OK assert
>> +        */
>> +       for (i = 0; i < CRU_MAX_RETRY_COUNT; i++) {
>> +               reg = readl(cru_base + CRU_PWRDWN_STATUS_OFFSET);
>> +               if (reg & CRU_PWROKOUT_NEON0)
>> +                       break;
>> +
>> +               udelay(CRU_RETRY_INTVL_US);
>> +       }
>> +
>> +       if (WARN_ON(i == CRU_MAX_RETRY_COUNT))
>> +               goto neon_unmap;
>> +
>> +       /* Wait 0.5 usec = 500 nsec */
>> +       ndelay(CRU_STATUS_DELAY_NS);
>> +
>> +       /* Set the logic clamp for the neon block */
>> +       reg = readl(cru_base + CRU_PWRDWN_EN_OFFSET);
>> +       reg &= ~(1 << CRU_CLAMP_ON_NEON0);
>> +       writel(reg, cru_base + CRU_PWRDWN_EN_OFFSET);
>> +
>> +       /* Wait 0.5 usec = 500 nsec */
>> +       ndelay(CRU_STATUS_DELAY_NS);
>> +
>> +       /* Reset the neon hardware block */
>> +       reg = readl(cru_base + CRU_CONTROL_OFFSET);
>> +       reg |= (1 << CRU_NEON0_HW_RESET);
>> +       writel(reg, cru_base + CRU_CONTROL_OFFSET);
>> +
>> +neon_unmap:
>> +       iounmap(cru_base);
>> +}
>> +#endif /* CONFIG_NEON */
>
> Is this a single core chip? If not, it seems like all this would
> change when you add SMP support.
Cygnus is a single core chip.  There are other chips in the IPROC family 
that are multi-core.  But they are not part of the Cygnus 
sub-architecture and would have different ways of powering up.  These 
will be added in a different patch in the future.
>
> Rob
>

Arnd Bergmann Oct. 8, 2014, 6:12 p.m. UTC | #10

On Wednesday 08 October 2014 09:27:08 Scott Branden wrote:
> On 14-10-08 06:28 AM, Arnd Bergmann wrote:
> > On Wednesday 08 October 2014 05:27:24 Scott Branden wrote:
> >>>
> >>> I don't think you need per-board config options. The main option
> >>> above should be enough.
> >> This is not a per-board config option.  This is actually a per-SoC
> >> uArchtecture selection.  More major uArchectures will be added to the
> >> IPROC.  Will Change comment to "Support Broadcom Cygnus SoC"
> >
> > Ok, sounds fine, but remove ARCH_BCM_IPROC then. There should be
> > no need for a three-level deep hierarchy (BCM -> IPROC -> CYGNUS)
> >
> I do not need a 3-deep hierarchy, I need a 2-deep hierarchy for IPROC 
> and CYGNUS (and future SoCs that have IPROC Architecture in common).  I 
> can move IPROC out of the mach-bcm directory if you like a create a new 
> directory?  But it looks like the purpose of mach-bcm is to consolidate 
> all Broadcom chipsets in it?

Yes, better leave it all in mach-bcm. You really shouldn't need much
code at all that is soc specific, so adding new directories is not
encouraged. We have some platforms that need no code at all, and on
arm64 that is required.

Isn't Northstar also IPROC? That one didn't seem to need the symbol.

Could you make ARCH_BCM_IPROC a silent symbol that is just selected
by each SoC family specific symbol?

	Arnd

Scott Branden Oct. 8, 2014, 6:45 p.m. UTC | #11

On 14-10-08 11:12 AM, Arnd Bergmann wrote:
> On Wednesday 08 October 2014 09:27:08 Scott Branden wrote:
>> On 14-10-08 06:28 AM, Arnd Bergmann wrote:
>>> On Wednesday 08 October 2014 05:27:24 Scott Branden wrote:
>>>>>
>>>>> I don't think you need per-board config options. The main option
>>>>> above should be enough.
>>>> This is not a per-board config option.  This is actually a per-SoC
>>>> uArchtecture selection.  More major uArchectures will be added to the
>>>> IPROC.  Will Change comment to "Support Broadcom Cygnus SoC"
>>>
>>> Ok, sounds fine, but remove ARCH_BCM_IPROC then. There should be
>>> no need for a three-level deep hierarchy (BCM -> IPROC -> CYGNUS)
>>>
>> I do not need a 3-deep hierarchy, I need a 2-deep hierarchy for IPROC
>> and CYGNUS (and future SoCs that have IPROC Architecture in common).  I
>> can move IPROC out of the mach-bcm directory if you like a create a new
>> directory?  But it looks like the purpose of mach-bcm is to consolidate
>> all Broadcom chipsets in it?
>
> Yes, better leave it all in mach-bcm. You really shouldn't need much
> code at all that is soc specific, so adding new directories is not
> encouraged. We have some platforms that need no code at all, and on
> arm64 that is required.
>
> Isn't Northstar also IPROC? That one didn't seem to need the symbol.
Yes, Northstar is an older version of IPROC.  We may be able to 
consolidate it under the IPROC family as we upstream additional SoCs and 
drivers.  Have the commonality with ARCH_BCM_IPROC will allow this.
>
> Could you make ARCH_BCM_IPROC a silent symbol that is just selected
> by each SoC family specific symbol?
OK, I will make ARCH_BCM_IPROC a silent symbol.
>
> 	Arnd
>

Scott Branden Oct. 8, 2014, 10:16 p.m. UTC | #12

On 14-10-08 11:12 AM, Arnd Bergmann wrote:
> On Wednesday 08 October 2014 09:27:08 Scott Branden wrote:
>> On 14-10-08 06:28 AM, Arnd Bergmann wrote:
>>> On Wednesday 08 October 2014 05:27:24 Scott Branden wrote:
>>>>>
>>>>> I don't think you need per-board config options. The main option
>>>>> above should be enough.
>>>> This is not a per-board config option.  This is actually a per-SoC
>>>> uArchtecture selection.  More major uArchectures will be added to the
>>>> IPROC.  Will Change comment to "Support Broadcom Cygnus SoC"
>>>
>>> Ok, sounds fine, but remove ARCH_BCM_IPROC then. There should be
>>> no need for a three-level deep hierarchy (BCM -> IPROC -> CYGNUS)
>>>
>> I do not need a 3-deep hierarchy, I need a 2-deep hierarchy for IPROC
>> and CYGNUS (and future SoCs that have IPROC Architecture in common).  I
>> can move IPROC out of the mach-bcm directory if you like a create a new
>> directory?  But it looks like the purpose of mach-bcm is to consolidate
>> all Broadcom chipsets in it?
>
> Yes, better leave it all in mach-bcm. You really shouldn't need much
> code at all that is soc specific, so adding new directories is not
> encouraged. We have some platforms that need no code at all, and on
> arm64 that is required.
>
> Isn't Northstar also IPROC? That one didn't seem to need the symbol.
>
> Could you make ARCH_BCM_IPROC a silent symbol that is just selected
> by each SoC family specific symbol?
It looks like I will also be able to add Northstar under the 
ARCH_BCM_IPROC.  This will be good momentum to cleaning up and 
consolidate support going forward.
I will change the Kconfig such that ARCH_BCM_5301X selects ARCH_BCM_IPROC.

Thanks.
>
> 	Arnd
>

Arnd Bergmann Oct. 9, 2014, 7:32 a.m. UTC | #13

On Wednesday 08 October 2014 15:16:55 Scott Branden wrote:
> It looks like I will also be able to add Northstar under the 
> ARCH_BCM_IPROC.  This will be good momentum to cleaning up and 
> consolidate support going forward.
> I will change the Kconfig such that ARCH_BCM_5301X selects ARCH_BCM_IPROC.
> 
> 

Ok, sounds good.

	Arnd

Patch

diff --git a/arch/arm/mach-bcm/Kconfig b/arch/arm/mach-bcm/Kconfig
index fc93800..2dd3f78 100644
--- a/arch/arm/mach-bcm/Kconfig
+++ b/arch/arm/mach-bcm/Kconfig
@@ -5,6 +5,37 @@  menuconfig ARCH_BCM
 
 if ARCH_BCM
 
+config ARCH_BCM_IPROC
+	bool "Broadcom ARMv7 iProc boards" if ARCH_MULTI_V7
+	select ARM_GIC
+	select CACHE_L2X0
+	select HAVE_ARM_TWD if LOCAL_TIMERS
+	select HAVE_CLK
+	select CLKSRC_OF
+	select CLKSRC_MMIO
+	select GENERIC_CLOCKEVENTS
+	select ARM_GLOBAL_TIMER
+	select ARCH_REQUIRE_GPIOLIB
+	select ARM_AMBA
+	select PINCTRL
+	select DEBUG_UART_8250
+	help
+	  This enables support for systems based on Broadcom IPROC architected SoCs.
+	  The IPROC complex contains one or more ARM CPUs along with common
+	  core periperals. Application specific SoCs are created by adding a
+	  uArchitecture containing peripherals outside of the IPROC complex.
+	  Currently supported SoCs are Cygnus.
+
+menu "iProc SoC based Machine types"
+	depends on ARCH_BCM_IPROC
+
+	config ARCH_BCM_CYGNUS
+		bool "Support Broadcom Cygnus board"
+		select USB_ARCH_HAS_EHCI if USB_SUPPORT
+		help
+		  Support for Broadcom Cygnus SoC.
+endmenu
+
 config ARCH_BCM_MOBILE
 	bool "Broadcom Mobile SoC Support" if ARCH_MULTI_V7
 	select ARCH_REQUIRE_GPIOLIB
diff --git a/arch/arm/mach-bcm/Makefile b/arch/arm/mach-bcm/Makefile
index b19a396..46e092a 100644
--- a/arch/arm/mach-bcm/Makefile
+++ b/arch/arm/mach-bcm/Makefile
@@ -10,6 +10,9 @@ 
 # of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 # GNU General Public License for more details.
 
+# Cygnus
+obj-$(CONFIG_ARCH_BCM_CYGNUS) +=  bcm_cygnus.o
+
 # BCM281XX
 obj-$(CONFIG_ARCH_BCM_281XX)	+= board_bcm281xx.o
 
diff --git a/arch/arm/mach-bcm/bcm_cygnus.c b/arch/arm/mach-bcm/bcm_cygnus.c
new file mode 100644
index 0000000..8e430ed
--- /dev/null
+++ b/arch/arm/mach-bcm/bcm_cygnus.c
@@ -0,0 +1,166 @@ 
+/*
+ * Copyright 2014 Broadcom Corporation.  All rights reserved.
+ *
+ * Unless you and Broadcom execute a separate written software license
+ * agreement governing use of this software, this software is licensed to you
+ * under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/of_address.h>
+#include <linux/of_platform.h>
+#include <linux/clocksource.h>
+#include <linux/clk-provider.h>
+#include <linux/delay.h>
+#include <asm/mach/arch.h>
+#include <asm/mach/map.h>
+#include <asm/proc-fns.h>
+#include <asm/hardware/cache-l2x0.h>
+
+#define CRMU_MAIL_BOX1      0x03024028
+#define CRMU_SOFT_RESET_CMD 0xFFFFFFFF
+
+/* CRU_RESET register */
+static void * __iomem crmu_mail_box1_reg;
+
+#ifdef CONFIG_NEON
+
+#define CRU_BASE                  0x1800e000
+#define CRU_SIZE                  0x34
+#define CRU_CONTROL_OFFSET        0x0
+#define CRU_PWRDWN_EN_OFFSET      0x4
+#define CRU_PWRDWN_STATUS_OFFSET  0x8
+#define CRU_NEON0_HW_RESET  6
+#define CRU_CLAMP_ON_NEON0  20
+#define CRU_PWRONIN_NEON0   21
+#define CRU_PWRONOUT_NEON0  21
+#define CRU_PWROKIN_NEON0   22
+#define CRU_PWROKOUT_NEON0  22
+#define CRU_STATUS_DELAY_NS 500
+#define CRU_MAX_RETRY_COUNT 10
+#define CRU_RETRY_INTVL_US  1
+
+/* Power up the NEON/VFPv3 block. */
+static void bcm_cygnus_powerup_neon(void)
+{
+	void * __iomem cru_base = ioremap(CRU_BASE, CRU_SIZE);
+	u32 reg, i;
+
+	if (WARN_ON(!cru_base))
+		return;
+
+	/* De-assert the neon hardware block reset */
+	reg = readl(cru_base + CRU_CONTROL_OFFSET);
+	reg &= ~(1 << CRU_NEON0_HW_RESET);
+	writel(reg, cru_base + CRU_CONTROL_OFFSET);
+
+	/* Assert the power ON register bit */
+	reg = readl(cru_base + CRU_PWRDWN_EN_OFFSET);
+	reg |= (1 << CRU_PWRONIN_NEON0);
+	writel(reg, cru_base + CRU_PWRDWN_EN_OFFSET);
+
+	/*
+	 * Wait up to 10 usec in 1 usec increments for the
+	 * status register to acknowledge the power ON assert
+	 */
+	for (i = 0; i < CRU_MAX_RETRY_COUNT; i++) {
+		reg = readl(cru_base + CRU_PWRDWN_STATUS_OFFSET);
+		if (reg & CRU_PWRONOUT_NEON0)
+			break;
+
+		udelay(CRU_RETRY_INTVL_US);
+	}
+
+	if (WARN_ON(i == CRU_MAX_RETRY_COUNT))
+		goto neon_unmap;
+
+	/* Wait 0.5 usec = 500 nsec */
+	ndelay(CRU_STATUS_DELAY_NS);
+
+	/* Assert the power OK register bit */
+	reg = readl(cru_base + CRU_PWRDWN_EN_OFFSET);
+	reg |= (1 << CRU_PWROKIN_NEON0);
+	writel(reg, cru_base + CRU_PWRDWN_EN_OFFSET);
+
+	/*
+	 * Wait up to 10 usec in 1 usec increments for the
+	 * status register to acknowledge the power OK assert
+	 */
+	for (i = 0; i < CRU_MAX_RETRY_COUNT; i++) {
+		reg = readl(cru_base + CRU_PWRDWN_STATUS_OFFSET);
+		if (reg & CRU_PWROKOUT_NEON0)
+			break;
+
+		udelay(CRU_RETRY_INTVL_US);
+	}
+
+	if (WARN_ON(i == CRU_MAX_RETRY_COUNT))
+		goto neon_unmap;
+
+	/* Wait 0.5 usec = 500 nsec */
+	ndelay(CRU_STATUS_DELAY_NS);
+
+	/* Set the logic clamp for the neon block */
+	reg = readl(cru_base + CRU_PWRDWN_EN_OFFSET);
+	reg &= ~(1 << CRU_CLAMP_ON_NEON0);
+	writel(reg, cru_base + CRU_PWRDWN_EN_OFFSET);
+
+	/* Wait 0.5 usec = 500 nsec */
+	ndelay(CRU_STATUS_DELAY_NS);
+
+	/* Reset the neon hardware block */
+	reg = readl(cru_base + CRU_CONTROL_OFFSET);
+	reg |= (1 << CRU_NEON0_HW_RESET);
+	writel(reg, cru_base + CRU_CONTROL_OFFSET);
+
+neon_unmap:
+	iounmap(cru_base);
+}
+#endif /* CONFIG_NEON */
+
+static void __init bcm_cygnus_init(void)
+{
+	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
+
+	l2x0_of_init(0, ~0UL);
+
+	crmu_mail_box1_reg = ioremap(CRMU_MAIL_BOX1, SZ_4);
+	WARN_ON(!crmu_mail_box1_reg);
+
+#ifdef CONFIG_NEON
+	bcm_cygnus_powerup_neon();
+#endif
+}
+
+/*
+ * Reset the system
+ */
+void bcm_cygnus_restart(enum reboot_mode mode, const char *cmd)
+{
+	/* Send reset command to M0 via Mailbox. */
+	if (crmu_mail_box1_reg) {
+		writel(CRMU_SOFT_RESET_CMD, crmu_mail_box1_reg);
+		iounmap(crmu_mail_box1_reg);
+	}
+
+	/* Wait for M0 to reset the chip. */
+	while (1)
+		cpu_do_idle();
+}
+
+static const char const *bcm_cygnus_dt_compat[] = {
+	"brcm,cygnus",
+	NULL,
+};
+
+DT_MACHINE_START(BCM_CYGNUS_DT, "Broadcom Cygnus SoC")
+	.init_machine = bcm_cygnus_init,
+	.map_io = debug_ll_io_init,
+	.dt_compat = bcm_cygnus_dt_compat,
+	.restart   = bcm_cygnus_restart
+MACHINE_END