[U-Boot] [PATCH 02/11] imx: imx-common: introduce Resource Domain Controller support

Peng Fan
2016-01-07T07:34:05Z

Hi Stefan,

On Wed, Jan 06, 2016 at 10:52:26PM -0800, Stefan Agner wrote:
>Hi Peng,
>
>Some minor questions/remarks below.
>
>On 2016-01-04 21:56, Peng Fan wrote:
>> From: Peng Fan <peng.fan@nxp.com>
>>
>> Introduce Resource Domain Controller support for i.MX.
>> Now i.MX6SX and i.MX7D supports this feature to assign masters
>> and peripherals to different domains.
>>
>> Signed-off-by: Ye.Li <ye.li@nxp.com>
>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
>> ---
>> arch/arm/imx-common/Kconfig | 8 ++
>> arch/arm/imx-common/Makefile | 1 +
>> arch/arm/imx-common/rdc-sema.c | 184 +++++++++++++++++++++++++++++
>> arch/arm/include/asm/arch-mx6/imx-rdc.h | 12 ++
>> arch/arm/include/asm/imx-common/rdc-sema.h | 117 ++++++++++++++++++
>> 5 files changed, 322 insertions(+)
>> create mode 100644 arch/arm/imx-common/rdc-sema.c
>> create mode 100644 arch/arm/include/asm/arch-mx6/imx-rdc.h
>> create mode 100644 arch/arm/include/asm/imx-common/rdc-sema.h
>>
>> diff --git a/arch/arm/imx-common/Kconfig b/arch/arm/imx-common/Kconfig
>> index 2296239..c4f48bb 100644
>> --- a/arch/arm/imx-common/Kconfig
>> +++ b/arch/arm/imx-common/Kconfig
>> @@ -3,3 +3,11 @@ config IMX_CONFIG
>>
>> config ROM_UNIFIED_SECTIONS
>> bool
>> +
>> +config IMX_RDC
>> + bool "i.MX Resource domain controller driver"
>> + depends on ARCH_MX6 || ARCH_MX7
>> + help
>> + i.MX Resource domain controller is used to assign masters
>> + and peripherals to differet domains. This can be used to
>> + isolate resources.
>> diff --git a/arch/arm/imx-common/Makefile b/arch/arm/imx-common/Makefile
>> index e7190c3..568f41c 100644
>> --- a/arch/arm/imx-common/Makefile
>> +++ b/arch/arm/imx-common/Makefile
>> @@ -27,6 +27,7 @@ ifeq ($(SOC),$(filter $(SOC),mx6 mx7))
>> obj-y += cache.o init.o
>> obj-$(CONFIG_CMD_SATA) += sata.o
>> obj-$(CONFIG_IMX_VIDEO_SKIP) += video.o
>> +obj-$(CONFIG_IMX_RDC) += rdc-sema.o
>> obj-$(CONFIG_SECURE_BOOT) += hab.o
>> endif
>> ifeq ($(SOC),$(filter $(SOC),vf610))
>> diff --git a/arch/arm/imx-common/rdc-sema.c b/arch/arm/imx-common/rdc-sema.c
>> new file mode 100644
>> index 0000000..7db1ec5
>> --- /dev/null
>> +++ b/arch/arm/imx-common/rdc-sema.c
>> @@ -0,0 +1,184 @@
>> +/*
>> + * Copyright (C) 2016 Freescale Semiconductor, Inc.
>> + *
>> + * SPDX-License-Identifier: GPL-2.0+
>> + */
>> +#include <common.h>
>> +#include <asm/io.h>
>> +#include <asm/arch/imx-regs.h>
>> +#include <asm/imx-common/rdc-sema.h>
>> +#include <asm/arch/imx-rdc.h>
>> +#include <asm-generic/errno.h>
>> +
>> +/*
>> + * Check if the RDC Semaphore is required for this peripheral.
>> + */
>> +static inline int imx_rdc_check_sema_required(int per_id)
>> +{
>> + struct rdc_regs *imx_rdc = (struct rdc_regs *)RDC_BASE_ADDR;
>> + u32 reg;
>> +
>> + reg = readl(&imx_rdc->pdap[per_id]);
>> + /*
>> + * No semaphore:
>> + * Intial value or this peripheral is assigned to only one domain
>> + */
>> + if (!(reg & RDC_PDAP_SREQ_MASK))
>> + return -ENOENT;
>> +
>> + return 0;
>> +}
>> +
>> +/*
>> + * Check the peripheral read / write access permission on Domain 0.
>> + * (Always assume the main CPU is in Domain 0)
>
>Hence you assuming that U-Boot is also always running on main CPU right?

Yeah. To i.MX6SX and 7D, U-Boot is only run on the Cortex-Ax core.

>I guess this is ok for now.
>
>> + */
>> +int imx_rdc_check_permission(int per_id)
>> +{
>> + struct rdc_regs *imx_rdc = (struct rdc_regs *)RDC_BASE_ADDR;
>> + u32 reg;
>> +
>> + reg = readl(&imx_rdc->pdap[per_id]);
>> + if (!(reg & (RDC_PDAP_D0W_MASK | RDC_PDAP_D0R_MASK)))
>> + return -EACCES; /*No access*/
>> +
>> + return 0;
>> +}

[..........]

>> diff --git a/arch/arm/include/asm/arch-mx6/imx-rdc.h
>> b/arch/arm/include/asm/arch-mx6/imx-rdc.h
>> new file mode 100644
>> index 0000000..5754f04
>> --- /dev/null
>> +++ b/arch/arm/include/asm/arch-mx6/imx-rdc.h
>> @@ -0,0 +1,12 @@
>> +/*
>> + * Copyright (C) 2016 Freescale Semiconductor, Inc.
>> + *
>> + * SPDX-License-Identifier: GPL-2.0+
>> + */
>> +
>> +#ifndef __IMX_RDC_H__
>> +#define __IMX_RDC_H__
>> +
>> +#error "Please select cpu"
>
>I don't understand this, wouldn't lead this always to an error?

The CONFIG is not enabled, when add i.MX6SX and 7D support. This will
be ok.

>

[............]

>> +
>> +#define RDC_PDAP_D0W_SHIFT 0
>> +#define RDC_PDAP_D0W_MASK (0x1 << RDC_PDAP_D0W_SHIFT)
>> +#define RDC_PDAP_D0R_SHIFT 1
>> +#define RDC_PDAP_D0R_MASK (0x1 << RDC_PDAP_D0R_SHIFT)
>> +#define RDC_PDAP_D1W_SHIFT 2
>> +#define RDC_PDAP_D1W_MASK (0x1 << RDC_PDAP_D1W_SHIFT)
>> +#define RDC_PDAP_D1R_SHIFT 3
>> +#define RDC_PDAP_D1R_MASK (0x1 << RDC_PDAP_D1R_SHIFT)
>> +#define RDC_PDAP_D2W_SHIFT 4
>> +#define RDC_PDAP_D2W_MASK (0x1 << RDC_PDAP_D2W_SHIFT)
>> +#define RDC_PDAP_D2R_SHIFT 5
>> +#define RDC_PDAP_D2R_MASK (0x1 << RDC_PDAP_D2R_SHIFT)
>> +#define RDC_PDAP_D3W_SHIFT 6
>> +#define RDC_PDAP_D3W_MASK (0x1 << RDC_PDAP_D3W_SHIFT)
>> +#define RDC_PDAP_D3R_SHIFT 7
>> +#define RDC_PDAP_D3R_MASK (0x1 << RDC_PDAP_D3R_SHIFT)
>
>Nit: How about RDC_PDAP_DXW_SHIFT(domain) (2 * domain) style defines?

Will try this way.

[........]

Thanks,
Peng.
>