Hi Simon,

On Wed, Jun 17, 2020 at 8:42 AM Simon Glass sjg@chromium.org wrote:

On Wed, 10 Jun 2020 at 04:41, Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com wrote:

...

Add L3 memory flush support for NS3.

Signed-off-by: Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com

arch/arm/cpu/armv8/Makefile | 1 + arch/arm/cpu/armv8/bcmns3/Makefile | 5 ++ arch/arm/cpu/armv8/bcmns3/lowlevel.S | 90 ++++++++++++++++++++++++++++ 3 files changed, 96 insertions(+) create mode 100644 arch/arm/cpu/armv8/bcmns3/Makefile create mode 100644 arch/arm/cpu/armv8/bcmns3/lowlevel.S

diff --git a/arch/arm/cpu/armv8/Makefile b/arch/arm/cpu/armv8/Makefile index 2e48df0eb9..7e33a183d5 100644 --- a/arch/arm/cpu/armv8/Makefile +++ b/arch/arm/cpu/armv8/Makefile @@ -39,3 +39,4 @@ obj-$(CONFIG_S32V234) += s32v234/ obj-$(CONFIG_TARGET_HIKEY) += hisilicon/ obj-$(CONFIG_ARMV8_PSCI) += psci.o obj-$(CONFIG_ARCH_SUNXI) += lowlevel_init.o +obj-$(CONFIG_TARGET_BCMNS3) += bcmns3/ diff --git a/arch/arm/cpu/armv8/bcmns3/Makefile b/arch/arm/cpu/armv8/bcmns3/Makefile new file mode 100644 index 0000000000..a35e29d11a --- /dev/null +++ b/arch/arm/cpu/armv8/bcmns3/Makefile @@ -0,0 +1,5 @@ +# SPDX-License-Identifier: GPL-2.0+ +# +# Copyright 2020 Broadcom.

+obj-y += lowlevel.o diff --git a/arch/arm/cpu/armv8/bcmns3/lowlevel.S b/arch/arm/cpu/armv8/bcmns3/lowlevel.S new file mode 100644 index 0000000000..202286248e --- /dev/null +++ b/arch/arm/cpu/armv8/bcmns3/lowlevel.S @@ -0,0 +1,90 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/*

• • Copyright 2020 Broadcom
• • Extracted from fsl-layerscape/lowlevel.S

Should this file be common, then? Is the (c) correct?

Do you mean, file "arch/arm/cpu/armv8/bcmns3/lowlevel.S" should be common and for common file copyright tag should be "(C) Copyright 2020" instead of "Copyright 2020 Broadcom".

Please let me know.

Best regards, Rayagonda

Hi Simon,

On Fri, Jun 26, 2020 at 7:13 AM Simon Glass sjg@chromium.org wrote:

Hi Rayagonda,

On Fri, 19 Jun 2020 at 10:55, Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com wrote:

...

Hi Simon,

On Wed, Jun 17, 2020 at 8:42 AM Simon Glass sjg@chromium.org wrote:

...

On Wed, 10 Jun 2020 at 04:41, Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com wrote:

...

Add L3 memory flush support for NS3.

Signed-off-by: Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com

arch/arm/cpu/armv8/Makefile | 1 + arch/arm/cpu/armv8/bcmns3/Makefile | 5 ++ arch/arm/cpu/armv8/bcmns3/lowlevel.S | 90 ++++++++++++++++++++++++++++ 3 files changed, 96 insertions(+) create mode 100644 arch/arm/cpu/armv8/bcmns3/Makefile create mode 100644 arch/arm/cpu/armv8/bcmns3/lowlevel.S

diff --git a/arch/arm/cpu/armv8/Makefile b/arch/arm/cpu/armv8/Makefile index 2e48df0eb9..7e33a183d5 100644 --- a/arch/arm/cpu/armv8/Makefile +++ b/arch/arm/cpu/armv8/Makefile @@ -39,3 +39,4 @@ obj-$(CONFIG_S32V234) += s32v234/ obj-$(CONFIG_TARGET_HIKEY) += hisilicon/ obj-$(CONFIG_ARMV8_PSCI) += psci.o obj-$(CONFIG_ARCH_SUNXI) += lowlevel_init.o +obj-$(CONFIG_TARGET_BCMNS3) += bcmns3/ diff --git a/arch/arm/cpu/armv8/bcmns3/Makefile b/arch/arm/cpu/armv8/bcmns3/Makefile new file mode 100644 index 0000000000..a35e29d11a --- /dev/null +++ b/arch/arm/cpu/armv8/bcmns3/Makefile @@ -0,0 +1,5 @@ +# SPDX-License-Identifier: GPL-2.0+ +# +# Copyright 2020 Broadcom.

+obj-y += lowlevel.o diff --git a/arch/arm/cpu/armv8/bcmns3/lowlevel.S b/arch/arm/cpu/armv8/bcmns3/lowlevel.S new file mode 100644 index 0000000000..202286248e --- /dev/null +++ b/arch/arm/cpu/armv8/bcmns3/lowlevel.S @@ -0,0 +1,90 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/*

• • Copyright 2020 Broadcom
• • Extracted from fsl-layerscape/lowlevel.S

Should this file be common, then? Is the (c) correct?

Do you mean, file "arch/arm/cpu/armv8/bcmns3/lowlevel.S" should be common and for common file copyright tag should be "(C) Copyright 2020" instead of "Copyright 2020 Broadcom".

The comment suggests it was copied from another file, in which case you should keep the (c) from that file, perhaps adding your own.

But if the two files are the same, can you just have one file and move it to a common location?

The comment is misleading. Though it is copied from another file but both files are not same. Let me remove that comment.

Thank you, Rayagonda

...

Please let me know.

Consider yourself let.

Regards, Simon

On Fri, Jun 26, 2020 at 6:41 AM Simon Glass sjg@chromium.org wrote:

On Wed, 10 Jun 2020 at 04:41, Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com wrote:

...

From: Abhishek Shah abhishek.shah@broadcom.com

Add API to save boot parameters passed from BL31

Use assembly implementation of save_boot_params instead of c function. Because generally ATF does not set up SP_EL2 on exiting. Thus, usage of a C function immediately after exiting with no stack setup done by ATF explicitly, may cause SP_EL2 to be not sane, which in turn causes a crash if this boot was not lucky to get an SP_EL2 in valid range. Replace C implementation with assembly one which does not use stack this early, and let u-boot to set up its stack later.

Can this be fixed in ATF?

We are passing boot parameters for uboot using arm cpu x0, x1, x2 and x3 registers.

...

Signed-off-by: Abhishek Shah abhishek.shah@broadcom.com Signed-off-by: Rajesh Ravi rajesh.ravi@broadcom.com Signed-off-by: Vladimir Olovyannikov vladimir.olovyannikov@broadcom.com Signed-off-by: Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com

---

arch/arm/cpu/armv8/bcmns3/lowlevel.S | 9 +++++++ arch/arm/include/asm/arch-bcmns3/bl33_info.h | 26 ++++++++++++++++++++ board/broadcom/bcmns3/ns3.c | 10 ++++++++ 3 files changed, 45 insertions(+) create mode 100644 arch/arm/include/asm/arch-bcmns3/bl33_info.h

Reviewed-by: Simon Glass sjg@chromium.org

...

diff --git a/board/broadcom/bcmns3/ns3.c b/board/broadcom/bcmns3/ns3.c index e38156723c..5e644bd466 100644 --- a/board/broadcom/bcmns3/ns3.c +++ b/board/broadcom/bcmns3/ns3.c @@ -8,6 +8,7 @@ #include <asm/io.h> #include <asm/system.h> #include <asm/armv8/mmu.h> +#include <asm/arch-bcmns3/bl33_info.h>

static struct mm_region ns3_mem_map[] = { { @@ -33,8 +34,17 @@ struct mm_region *mem_map = ns3_mem_map;

DECLARE_GLOBAL_DATA_PTR;

+/*

• • Force the bl33_info to the data-section, as .bss will not be valid
• • when save_boot_params is invoked.
• */

+struct bl33_info *bl33_info __section(".data");

int board_init(void) {

•   if (bl33_info->version != BL33_INFO_VERSION)
  

•           printf("*** warning: ATF BL31 and u-boot not in sync! ***\n");
  

Shouldn't this be a fatal error?

It's not a fatal error. Its warning message for the user indicates that BL31 and BL33 are out of sync. By knowing this warning message, users can upgrade the binarie from uboot prompt.

Best regards, Rayagonda

...

•   return 0;
  

}

-- 2.17.1

Hi Rayagonda,

On Wed, 10 Jun 2020 at 04:42, Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com wrote:

Default "reset" from u-boot to L3 reset.

U-Boot

"reset" command with argument will trigger L1 reset.

Signed-off-by: Rajesh Ravi rajesh.ravi@broadcom.com Signed-off-by: Bharat Kumar Reddy Gooty bharat.gooty@broadcom.com Signed-off-by: Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com

---

board/broadcom/bcmns3/ns3.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/board/broadcom/bcmns3/ns3.c b/board/broadcom/bcmns3/ns3.c index 5e644bd466..1221f26ddc 100644 --- a/board/broadcom/bcmns3/ns3.c +++ b/board/broadcom/bcmns3/ns3.c @@ -68,7 +68,23 @@ int dram_init_banksize(void) return 0; }

-void reset_cpu(ulong addr) +void reset_cpu(ulong level) {

This should be in a sysreset driver. It supports different reset types.

•   psci_system_reset();
  

+#define L3_RESET 30

•   u32 reset_level, strap_val;
  

•   /* Default reset type is L3 reset */
  

•   if (!level) {
  

•           /*
  

•            * Encoding: u-boot reset command expects decimal argument
  

•            * strap val = 1st decimal digit;reset level = 2nd decimal digit
  

•            */
  

•           strap_val = L3_RESET % 10;
  

•           level = L3_RESET / 10;
  

•           reset_level = level % 10;
  

•           psci_system_reset2(reset_level, strap_val);
  

•   } else {
  

•           /* U-boot cmd "reset" with any arg will trigger L1 reset */
  

•           psci_system_reset();
  

•   }
  

}

2.17.1

Regards, Simon

Hi Rayagonda,

On Mon, 29 Jun 2020 at 22:43, Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com wrote:

Hi Simon,

On Fri, Jun 26, 2020 at 6:42 AM Simon Glass sjg@chromium.org wrote:

...

Hi Rayagonda,

On Wed, 10 Jun 2020 at 04:42, Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com wrote:

...

Default "reset" from u-boot to L3 reset.

U-Boot

Thank you, will fix this.

...

...

"reset" command with argument will trigger L1 reset.

Signed-off-by: Rajesh Ravi rajesh.ravi@broadcom.com Signed-off-by: Bharat Kumar Reddy Gooty bharat.gooty@broadcom.com Signed-off-by: Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com

---

board/broadcom/bcmns3/ns3.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/board/broadcom/bcmns3/ns3.c b/board/broadcom/bcmns3/ns3.c index 5e644bd466..1221f26ddc 100644 --- a/board/broadcom/bcmns3/ns3.c +++ b/board/broadcom/bcmns3/ns3.c @@ -68,7 +68,23 @@ int dram_init_banksize(void) return 0; }

-void reset_cpu(ulong addr) +void reset_cpu(ulong level) {

This should be in a sysreset driver. It supports different reset types.

I checked the sysreset driver and found a generic/common psci driver - drivers/sysreset/sysreset_psci.c. We could use this common driver in our platform.

Right now this common driver uses the same command or function_id for both WARM and COLD reset. But in our case we should use different commands for WARM and COLD reset.

I am planning to add one kconfig option (USE_FN64_CMD) through which we can select different commands or the same command for COLD reset.

Something like this,

static int psci_sysreset_request(struct udevice *dev, enum sysreset_t type) { unsigned long function_id;

switch (type) {
case SYSRESET_WARM:
     function_id = PSCI_0_2_FN_SYSTEM_RESET;
     break;

 case SYSRESET_COLD:
     if (CONFIG_IS_ENABLED(USE_FN64_CMD))
          function_id = PSCI_0_2_FN64_SYSTEM_RESET;
     else
          function_id = PSCI_0_2_FN_SYSTEM_RESET;
     break;

 case SYSRESET_POWER_OFF:
     function_id = PSCI_0_2_FN_SYSTEM_OFF;
     break;

 default:
     return -ENOSYS;
}

invoke_psci_fn(function_id, 0, 0, 0);

return -EINPROGRESS;

}

This way any platform can define/select USE_FN64_CMD if they need different commands for WARM reset. Please let me know about this approach.

It seems OK, but better would be to use the device tree to determine the command (e.g. compatible string or a property).

Regards, Simon

Hi Rayagonda,

On Wed, 8 Jul 2020 at 01:01, Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com wrote:

Hi Simon,

On Fri, Jul 3, 2020 at 6:16 AM Simon Glass sjg@chromium.org wrote:

...

Hi Rayagonda,

On Mon, 29 Jun 2020 at 22:43, Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com wrote:

...

Hi Simon,

On Fri, Jun 26, 2020 at 6:42 AM Simon Glass sjg@chromium.org wrote:

...

Hi Rayagonda,

On Wed, 10 Jun 2020 at 04:42, Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com wrote:

...

Default "reset" from u-boot to L3 reset.

U-Boot

Thank you, will fix this.

...

...

"reset" command with argument will trigger L1 reset.

Signed-off-by: Rajesh Ravi rajesh.ravi@broadcom.com Signed-off-by: Bharat Kumar Reddy Gooty bharat.gooty@broadcom.com Signed-off-by: Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com

---

board/broadcom/bcmns3/ns3.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/board/broadcom/bcmns3/ns3.c b/board/broadcom/bcmns3/ns3.c index 5e644bd466..1221f26ddc 100644 --- a/board/broadcom/bcmns3/ns3.c +++ b/board/broadcom/bcmns3/ns3.c @@ -68,7 +68,23 @@ int dram_init_banksize(void) return 0; }

-void reset_cpu(ulong addr) +void reset_cpu(ulong level) {

This should be in a sysreset driver. It supports different reset types.

I checked the sysreset driver and found a generic/common psci driver - drivers/sysreset/sysreset_psci.c. We could use this common driver in our platform.

Right now this common driver uses the same command or function_id for both WARM and COLD reset. But in our case we should use different commands for WARM and COLD reset.

I am planning to add one kconfig option (USE_FN64_CMD) through which we can select different commands or the same command for COLD reset.

Something like this,

static int psci_sysreset_request(struct udevice *dev, enum sysreset_t type) { unsigned long function_id;

switch (type) {
case SYSRESET_WARM:
     function_id = PSCI_0_2_FN_SYSTEM_RESET;
     break;

 case SYSRESET_COLD:
     if (CONFIG_IS_ENABLED(USE_FN64_CMD))
          function_id = PSCI_0_2_FN64_SYSTEM_RESET;
     else
          function_id = PSCI_0_2_FN_SYSTEM_RESET;
     break;

 case SYSRESET_POWER_OFF:
     function_id = PSCI_0_2_FN_SYSTEM_OFF;
     break;

 default:
     return -ENOSYS;
}

invoke_psci_fn(function_id, 0, 0, 0);

return -EINPROGRESS;

}

This way any platform can define/select USE_FN64_CMD if they need different commands for WARM reset. Please let me know about this approach.

It seems OK, but better would be to use the device tree to determine the command (e.g. compatible string or a property).

I was checking linux psci drivers (drivers/firmware/psci/psci.c), they are handling SYSTEM_RESET and SYSTEM_RESET2 without dt ie they read psci capability, if it supports SYSTEM_RESET2 then they use it else they use SYSTEM_RESET only. I think, in U-Boot also we should handle in the same way. This is a little bit more work, hence I am planning to handle this as a separate patch set only. So request you to allow this patch as it is.

Please let me know.

That's fine with me. So for now can we drop the Kconfig and just support SYSTEM_RESET?

Regards, Simon

Hi Simon,

On Fri, Jun 26, 2020 at 6:42 AM Simon Glass sjg@chromium.org wrote:

On Wed, 10 Jun 2020 at 04:42, Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com wrote:

...

Update NS3 GIC LPI address.

Signed-off-by: Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com

include/dt-bindings/memory/bcm-ns3-mc.h | 3 +++ 1 file changed, 3 insertions(+)

Reviewed-by: Simon Glass sjg@chromium.org

Lower case hex

Can these be in the device tree and use a driver, like syscon maybe?

I tried using a device tree and syscon driver class (UCLASS_SYSCON) but I am facing an issue ie

When I call syscon_get_regmap() to get regmap, I am getting zero for regmap->ranges[0].start and regmap->ranges[0].size. I am passing the parent node udevice pointer to syscon_get_regmap().

I could see syscon_pre_probe() is not getting called during uboot booting, hence regmap is not initialized.

Following are my code changes:

//dt changes: + gic_lpi: syscon@0x8ad70000 { + compatible = "syscon"; + reg = <0x0 0x8ad70000 0x0 0x90000>; + status = "okay"; + }; + + scr { + compatible = "simple-bus"; + #address-cells = <1>; + #size-cells = <1>; + ranges = <0x0 0x0 0x61000000 0x05000000>; + + gic: interrupt-controller@2c00000 { + compatible = "arm,gic-v3"; + #interrupt-cells = <3>; + #address-cells = <1>; + #size-cells = <1>; + ranges; + interrupt-controller; + reg = <0x02c00000 0x010000>, /* GICD */ + <0x02e00000 0x600000>; /* GICR */ + regmap = <&gic_lpi>; + status = "okay"; + }; + }; +

//driver changes +static int gic_v3_its_get_gic_addr(struct gic_v3_its_priv *priv) +{ + struct udevice *dev; + struct regmap *regmap; + fdt_addr_t addr; + int ret; + + ret = uclass_first_device_err(UCLASS_IRQ, &dev); + if (ret && ret != -ENODEV) { + pr_err("%s: Canont find irq device\n", __func__); + return ret; + } + + addr = dev_read_addr_index(dev, 0); + if (addr == FDT_ADDR_T_NONE) { + pr_err("%s: Failed to get GICD address\n", __func__); + return -EINVAL; + } + priv->gicd_base = addr; + + regmap = syscon_get_regmap(dev); + if (!regmap) { + pr_err("%s: ** unable to find gic-lpi device\n", __func__); + return -ENODEV; + } + + priv->lpi_base = regmap->ranges[0].start; + + return 0; +}

//defconfig changes CONFIG_DM=y +CONFIG_REGMAP=y +CONFIG_SYSCON=y CONFIG_DM_GPIO=y

If I call uclass_first_device_err(UCLASS_SYSCON, &dev) before syscon_get_regmap(dev) then it works. Do I need to explicitly call uclass_first_device_err(UCLASS_SYSCON, &dev).

Please let me know.

Best regards, Rayagonda

...

diff --git a/include/dt-bindings/memory/bcm-ns3-mc.h b/include/dt-bindings/memory/bcm-ns3-mc.h index b4f78584a5..d6e7717ba2 100644 --- a/include/dt-bindings/memory/bcm-ns3-mc.h +++ b/include/dt-bindings/memory/bcm-ns3-mc.h @@ -31,4 +31,7 @@ #define BCM_NS3_MEM_CRMU_PT_START 0x880000000 #define BCM_NS3_MEM_CRMU_PT_LEN 0x200000

+#define BCM_NS3_GIC_LPI_BASE 0x8AD70000 +#define BCM_NS3_GIC_LPI_SIZE 0x90000

#endif

2.17.1

Hi Simon,

On Sun, Jul 5, 2020 at 12:20 AM Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com wrote:

Hi Simon,

On Sat, Jul 4, 2020 at 1:20 PM Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com wrote:

...

Hi Simon,

On Fri, Jun 26, 2020 at 6:42 AM Simon Glass sjg@chromium.org wrote:

...

On Wed, 10 Jun 2020 at 04:42, Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com wrote:

...

Update NS3 GIC LPI address.

Signed-off-by: Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com

include/dt-bindings/memory/bcm-ns3-mc.h | 3 +++ 1 file changed, 3 insertions(+)

Reviewed-by: Simon Glass sjg@chromium.org

Lower case hex

Can these be in the device tree and use a driver, like syscon maybe?

I will use dt and syscon to get the LPI base address. This will require change in arch/arm/lib/gic-v3-its.c driver.

Hope this is okay. Please let me know.

Best regards, Rayagonda

...

I tried using a device tree and syscon driver class (UCLASS_SYSCON) but I am facing an issue ie

When I call syscon_get_regmap() to get regmap, I am getting zero for regmap->ranges[0].start and regmap->ranges[0].size. I am passing the parent node udevice pointer to syscon_get_regmap().

I could see syscon_pre_probe() is not getting called during uboot booting, hence regmap is not initialized.

Following are my code changes:

//dt changes:

•   gic_lpi: syscon@0x8ad70000 {
  

•           compatible = "syscon";
  

•           reg = <0x0 0x8ad70000 0x0 0x90000>;
  

•           status = "okay";
  

•   };
  

•   scr {
  

•           compatible = "simple-bus";
  

•           #address-cells = <1>;
  

•           #size-cells = <1>;
  

•           ranges = <0x0 0x0 0x61000000 0x05000000>;
  

•           gic: interrupt-controller@2c00000 {
  

•                   compatible = "arm,gic-v3";
  

•                   #interrupt-cells = <3>;
  

•                   #address-cells = <1>;
  

•                   #size-cells = <1>;
  

•                   ranges;
  

•                   interrupt-controller;
  

•                   reg = <0x02c00000 0x010000>, /* GICD */
  

•                         <0x02e00000 0x600000>; /* GICR */
  

•                   regmap = <&gic_lpi>;
  

•                   status = "okay";
  

•           };
  

•   };
  

//driver changes +static int gic_v3_its_get_gic_addr(struct gic_v3_its_priv *priv) +{

•   struct udevice *dev;
  

•   struct regmap *regmap;
  

•   fdt_addr_t addr;
  

•   int ret;
  

•   ret = uclass_first_device_err(UCLASS_IRQ, &dev);
  

•   if (ret && ret != -ENODEV) {
  

•           pr_err("%s: Canont find irq device\n", __func__);
  

•           return ret;
  

•   }
  

•   addr = dev_read_addr_index(dev, 0);
  

•   if (addr == FDT_ADDR_T_NONE) {
  

•           pr_err("%s: Failed to get GICD address\n", __func__);
  

•           return -EINVAL;
  

•   }
  

•   priv->gicd_base = addr;
  

•   regmap = syscon_get_regmap(dev);
  

•   if (!regmap) {
  

•           pr_err("%s: ** unable to find gic-lpi device\n", __func__);
  

•           return -ENODEV;
  

•   }
  

•   priv->lpi_base = regmap->ranges[0].start;
  

•   return 0;
  

+}

//defconfig changes CONFIG_DM=y +CONFIG_REGMAP=y +CONFIG_SYSCON=y CONFIG_DM_GPIO=y

If I call uclass_first_device_err(UCLASS_SYSCON, &dev) before syscon_get_regmap(dev) then it works. Do I need to explicitly call uclass_first_device_err(UCLASS_SYSCON, &dev).

Please let me know.

I used uclass_get_device_by_driver() api and it's working. Please ignore the above question.

Thank you, Rayagonda

...

Best regards, Rayagonda

...

...

diff --git a/include/dt-bindings/memory/bcm-ns3-mc.h b/include/dt-bindings/memory/bcm-ns3-mc.h index b4f78584a5..d6e7717ba2 100644 --- a/include/dt-bindings/memory/bcm-ns3-mc.h +++ b/include/dt-bindings/memory/bcm-ns3-mc.h @@ -31,4 +31,7 @@ #define BCM_NS3_MEM_CRMU_PT_START 0x880000000 #define BCM_NS3_MEM_CRMU_PT_LEN 0x200000

+#define BCM_NS3_GIC_LPI_BASE 0x8AD70000 +#define BCM_NS3_GIC_LPI_SIZE 0x90000

#endif

2.17.1

Hi Simon,

On Fri, Jun 26, 2020 at 6:42 AM Simon Glass sjg@chromium.org wrote:

Hi Rayagonda,

On Wed, 10 Jun 2020 at 04:42, Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com wrote:

...

U-boot programs the GIC LPI configuration tables and enables the LPI table.

Signed-off-by: Bharat Kumar Reddy Gooty bharat.gooty@broadcom.com Signed-off-by: Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com

---

board/broadcom/bcmns3/ns3.c | 12 ++++++++++++ include/configs/bcm_ns3.h | 6 ++++++ 2 files changed, 18 insertions(+)

diff --git a/board/broadcom/bcmns3/ns3.c b/board/broadcom/bcmns3/ns3.c index 1221f26ddc..d4f374a7c6 100644 --- a/board/broadcom/bcmns3/ns3.c +++ b/board/broadcom/bcmns3/ns3.c @@ -6,9 +6,12 @@

#include <common.h> #include <asm/io.h> +#include <asm/gic.h> +#include <asm/gic-v3.h> #include <asm/system.h> #include <asm/armv8/mmu.h> #include <asm/arch-bcmns3/bl33_info.h> +#include <dt-bindings/memory/bcm-ns3-mc.h>

static struct mm_region ns3_mem_map[] = { { @@ -88,3 +91,12 @@ void reset_cpu(ulong level) psci_system_reset(); } }

+#ifdef CONFIG_OF_BOARD_SETUP +int ft_board_setup(void *fdt, bd_t *bd) +{

•   gic_lpi_tables_init(BCM_NS3_GIC_LPI_BASE, MAX_GIC_REDISTRIBUTORS);
  

•   return 0;
  

+} +#endif /* CONFIG_OF_BOARD_SETUP */ diff --git a/include/configs/bcm_ns3.h b/include/configs/bcm_ns3.h index 02a736456a..a17b9d51c0 100644 --- a/include/configs/bcm_ns3.h +++ b/include/configs/bcm_ns3.h @@ -37,4 +37,10 @@ #define CONFIG_SYS_MAXARGS 64 #define CONFIG_SYS_BARGSIZE CONFIG_SYS_CBSIZE

+/* Generic Interrupt Controller Definitions */ +#define GICD_BASE 0x63C00000 +#define GICR_BASE 0x63E00000

Please use device tree and a UCLASS_IRQ driver

Thank you. I will use UCLASS_IRQ.

This will require changes in arch/arm/lib/gic-v3-its.c driver ie I will use UCLASS_IRQ in arch/arm/lib/gic-v3-its.c driver to get GICD and GICR base address.

Hope this is okay. Please let me know.

Best regards, Rayagonda

...

+/* Number of GIC re-distributors */ +#define MAX_GIC_REDISTRIBUTORS 8

#endif /* __BCM_NS3_H */

2.17.1

Regards, Simon

On Wed, 10 Jun 2020 at 04:42, Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com wrote:

Enables the GIC_V3 ITS LPI configuration table and LPI table.

Please write out these in full somewhere. As it is they are meaningless to people reading the commit.

Signed-off-by: Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com Signed-off-by: Bharat Kumar Reddy Gooty bharat.gooty@broadcom.com

---

configs/bcm_ns3_defconfig | 1 + 1 file changed, 1 insertion(+)

Reviewed-by: Simon Glass sjg@chromium.org

diff --git a/configs/bcm_ns3_defconfig b/configs/bcm_ns3_defconfig index 7e51a926f7..040e753f9f 100644 --- a/configs/bcm_ns3_defconfig +++ b/configs/bcm_ns3_defconfig @@ -1,4 +1,5 @@ CONFIG_ARM=y +CONFIG_GIC_V3_ITS=y CONFIG_TARGET_BCMNS3=y CONFIG_SYS_TEXT_BASE=0xFF000000 CONFIG_ENV_SIZE=0x80000 -- 2.17.1

On Wed, 10 Jun 2020 at 04:42, Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com wrote:

Add ddr memory definitions.

Signed-off-by: Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com

include/dt-bindings/memory/bcm-ns3-mc.h | 30 ++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-)

Reviewed-by: Simon Glass sjg@chromium.org

Please use lower-case hex throughout.

diff --git a/include/dt-bindings/memory/bcm-ns3-mc.h b/include/dt-bindings/memory/bcm-ns3-mc.h index d6e7717ba2..d04b842d19 100644 --- a/include/dt-bindings/memory/bcm-ns3-mc.h +++ b/include/dt-bindings/memory/bcm-ns3-mc.h @@ -7,7 +7,8 @@ #define DT_BINDINGS_BCM_NS3_MC_H

/*

• • Reserved Memory Map : SHMEM & TZDRAM.

• • +--------+----------+ 0x8B000000
• • | NITRO CRASH DUMP | 32MB
  • +--------+----------+ 0x8D000000
  • | SHMEM (NS) | 16 MB
  • +-------------------+ 0x8E000000

@@ -20,6 +21,10 @@

• +-------------------+ 0x8F100000

*/

+#define BCM_NS3_MEM_NITRO_CRASH_START 0x8AE00000 +#define BCM_NS3_MEM_NITRO_CRASH_LEN 0x21fffff +#define BCM_NS3_MEM_NITRO_CRASH_SIZE 0x2200000

#define BCM_NS3_MEM_SHARE_START 0x8D000000 #define BCM_NS3_MEM_SHARE_LEN 0x020FFFFF

@@ -31,7 +36,30 @@ #define BCM_NS3_MEM_CRMU_PT_START 0x880000000 #define BCM_NS3_MEM_CRMU_PT_LEN 0x200000

+/* default memory starting address and length */ +#define BCM_NS3_MEM_START 0x80000000UL +#define BCM_NS3_MEM_LEN 0x80000000UL +#define BCM_NS3_MEM_END (BCM_NS3_MEM_START + BCM_NS3_MEM_LEN)

+/* memory starting address and length for BANK_1 */ +#define BCM_NS3_BANK_1_MEM_START 0x880000000UL +#define BCM_NS3_BANK_1_MEM_LEN 0x180000000UL

+/* memory layout information */ +#define BCM_NS3_DDR_INFO_BASE 0x8F220000 +#define BCM_NS3_DDR_INFO_RSVD_LEN 0x1000 +#define BCM_NS3_DDR_INFO_LEN 73 +#define BCM_NS3_DDR_INFO_SIG 0x42434d44 +#define BCM_NS3_MAX_NR_BANKS 4

#define BCM_NS3_GIC_LPI_BASE 0x8AD70000 #define BCM_NS3_GIC_LPI_SIZE 0x90000

+#define BCM_NS3_MEM_RSVE_START BCM_NS3_GIC_LPI_BASE +#define BCM_NS3_MEM_RSVE_END ((BCM_NS3_MEM_ELOG_START + \

•                              BCM_NS3_MEM_ELOG_LEN) - \
  

•                              BCM_NS3_MEM_RSVE_START)
  

+#define BCM_NS3_CRMU_PGT_START 0x880000000UL +#define BCM_NS3_CRMU_PGT_SIZE 0x100000

#endif

2.17.1

Hi Rayagonda,

On Wed, 10 Jun 2020 at 04:42, Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com wrote:

Add both DRAM banks memory information and the corresponding MMU page table mappings.

Signed-off-by: Bharat Kumar Reddy Gooty bharat.gooty@broadcom.com Signed-off-by: Rayagonda Kokatanur rayagonda.kokatanur@broadcom.com

---

arch/arm/dts/ns3-board.dts | 23 ++++++++++ board/broadcom/bcmns3/ns3.c | 83 +++++++++++++++++++++++++++++++++++-- configs/bcm_ns3_defconfig | 2 + 3 files changed, 105 insertions(+), 3 deletions(-)

Reviewed-by: Simon Glass sjg@chromium.org

nits below

diff --git a/arch/arm/dts/ns3-board.dts b/arch/arm/dts/ns3-board.dts index 54e56879a5..c660e74dc8 100644 --- a/arch/arm/dts/ns3-board.dts +++ b/arch/arm/dts/ns3-board.dts @@ -5,6 +5,29 @@

/dts-v1/;

+#include <dt-bindings/memory/bcm-ns3-mc.h>

+/*

• • Single mem reserve region which includes the following:
• • Components name Start Addr Size

• • ---

• • GIC LPI tables 0x8AD7_0000 0x0009_0000
• • Nitro FW 0x8AE0_0000 0x0020_0000
• • Nitro Crash dump 0x8B00_0000 0x0200_0000
• • OPTEE OS 0x8D00_0000 0x0200_0000
• • BL31 services 0x8F00_0000 0x0010_0000
• • Tmon 0x8F10_0000 0x0000_1000
• • LPM/reserved 0x8F10_1000 0x0000_1000
• • ATF to Bl33 info 0x8F10_2000 0x0000_1000
• • ATF error logs 0x8F10_3000 0x0001_0000
• • Error log parser 0x8F11_3000 0x0010_0000
• */

+/memreserve/ BCM_NS3_MEM_RSVE_START BCM_NS3_MEM_RSVE_END;

+/* CRMU page tables */ +/memreserve/ BCM_NS3_CRMU_PGT_START BCM_NS3_CRMU_PGT_SIZE;

#include "ns3.dtsi"

/ { diff --git a/board/broadcom/bcmns3/ns3.c b/board/broadcom/bcmns3/ns3.c index d4f374a7c6..85d097887e 100644 --- a/board/broadcom/bcmns3/ns3.c +++ b/board/broadcom/bcmns3/ns3.c @@ -5,6 +5,7 @@ */

#include <common.h> +#include <fdt_support.h> #include <asm/io.h> #include <asm/gic.h> #include <asm/gic-v3.h> @@ -13,6 +14,8 @@ #include <asm/arch-bcmns3/bl33_info.h> #include <dt-bindings/memory/bcm-ns3-mc.h>

+#define BANK_OFFSET(bank) ((u64)BCM_NS3_DDR_INFO_BASE + 8 + ((bank) * 16))

static struct mm_region ns3_mem_map[] = { { .virt = 0x0UL, @@ -22,9 +25,15 @@ static struct mm_region ns3_mem_map[] = { PTE_BLOCK_NON_SHARE | PTE_BLOCK_PXN | PTE_BLOCK_UXN }, {

•           .virt = 0x80000000UL,
  

•           .phys = 0x80000000UL,
  

•           .size = 0x80000000UL,
  

•           .virt = BCM_NS3_MEM_START,
  

•           .phys = BCM_NS3_MEM_START,
  

•           .size = BCM_NS3_MEM_LEN,
  

•           .attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) |
  

•                    PTE_BLOCK_INNER_SHARE
  

•   }, {
  

•           .virt = BCM_NS3_BANK_1_MEM_START,
  

•           .phys = BCM_NS3_BANK_1_MEM_START,
  

•           .size = BCM_NS3_BANK_1_MEM_LEN,
            .attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) |
                     PTE_BLOCK_INNER_SHARE
    }, {
  

@@ -43,6 +52,72 @@ DECLARE_GLOBAL_DATA_PTR; */ struct bl33_info *bl33_info __section(".data");

+/*

• • Run modulo 256 checksum calculation and return the calculated checksum
• */

+static u8 checksum_calc(u8 *p, unsigned int len) +{

•   unsigned int i;
  

•   u8 chksum = 0;
  

•   for (i = 0; i < len; i++)
  

•           chksum += p[i];
  

•   return chksum;
  

+}

+/*

• • This function parses the memory layout informaion from a reserved area in

information

• • DDR, and then fix up the FDT before passing it to Linux.
• • In the case of error, do nothing and the default memory layout in DT will
• • be used
• */

+static void mem_info_parse_fixup(void *fdt) +{

•   u32 *p32, sig, i, nr_banks;
  

•   u64 *p64;
  

•   u64 start[BCM_NS3_MAX_NR_BANKS];
  

•   u64 len[BCM_NS3_MAX_NR_BANKS];
  

•   /* validate signature */
  

•   p32 = (u32 *)BCM_NS3_DDR_INFO_BASE;
  

Use a struct instead of ad-hoc pointer reading. Here I think you want something like:

struct xxx_dram_hdr { u32 sig; struct xxx_dram_bank bank[MAX_BANKS]; };

•   sig = *p32;
  

•   if (sig != BCM_NS3_DDR_INFO_SIG) {
  

•           printf("DDR info signature 0x%x invalid\n", sig);
  

•           return;
  

Return error code

•   }
  

•   /* run checksum test to validate data  */
  

•   if (checksum_calc((u8 *)p32, BCM_NS3_DDR_INFO_LEN) != 0) {
  

•           printf("Checksum on DDR info failed\n");
  

•           return;
  

•   }
  

•   /* parse information for each bank */
  

•   nr_banks = 0;
  

•   for (i = 0; i < BCM_NS3_MAX_NR_BANKS; i++) {
  

•           /* skip banks with a length of zero */
  

•           p64 = (u64 *)BANK_OFFSET(i);
  

•           if (*(p64 + 1) == 0)
  

•                   continue;
  

•           start[i] = *p64;
  

•           len[i] = *(p64 + 1);
  

•           printf("mem[%u] 0x%llx - 0x%llx\n", i, start[i],
  

•                  start[i] + len[i] - 1);
  

•           nr_banks++;
  

•   }
  

•   if (!nr_banks) {
  

•           printf("No DDR banks detected\n");
  

•           return;
  

•   }
  

•   fdt_fixup_memory_banks(fdt, start, len, nr_banks);
  

Check error - this can fail.

+}

int board_init(void) { if (bl33_info->version != BL33_INFO_VERSION) @@ -97,6 +172,8 @@ int ft_board_setup(void *fdt, bd_t *bd) { gic_lpi_tables_init(BCM_NS3_GIC_LPI_BASE, MAX_GIC_REDISTRIBUTORS);

•   mem_info_parse_fixup(fdt);
  

Check error.

•   return 0;
  

} #endif /* CONFIG_OF_BOARD_SETUP */ diff --git a/configs/bcm_ns3_defconfig b/configs/bcm_ns3_defconfig index 040e753f9f..9adb44cb51 100644 --- a/configs/bcm_ns3_defconfig +++ b/configs/bcm_ns3_defconfig @@ -4,6 +4,7 @@ CONFIG_TARGET_BCMNS3=y CONFIG_SYS_TEXT_BASE=0xFF000000 CONFIG_ENV_SIZE=0x80000 CONFIG_NR_DRAM_BANKS=2 +CONFIG_OF_BOARD_SETUP=y CONFIG_LOGLEVEL=7 CONFIG_SILENT_CONSOLE=y CONFIG_SILENT_U_BOOT_ONLY=y @@ -21,3 +22,4 @@ CONFIG_CLK=y CONFIG_CLK_CCF=y CONFIG_DM_SERIAL=y CONFIG_SYS_NS16550=y

+CONFIG_SPL_OF_LIBFDT=y

2.17.1

Regards, Simon

Hi Peter,

On Fri, Jun 26, 2020 at 7:57 PM Peter Tyser ptyser@xes-inc.com wrote:

<snip>

...

diff --git a/MAINTAINERS b/MAINTAINERS index 1fd975c72f..0c72deaa44 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -11,7 +11,7 @@ Descriptions of section entries: Type is one of: git, hg, quilt, stgit, topgit S: Status, one of the following: Supported: Someone is actually paid to look after this.

•    Maintained:  Someone actually looks after it.
  

•    Mne actually looks after it.
     Orphan:      No current maintainer [but maybe you could take the
                  role as you write your new code].
  F: Files and directories with wildcard patterns.
  

This looks to be an accidental change that should be fixed?

Thank you so much, I will fix it.

Regards, Rayagonda

Regards, Peter