diff mbox

[U-Boot] rockchip: arm: halt when enable bucks of PMIC rk808

Message ID 1491461881-31878-1-git-send-email-eric.gao@rock-chips.com
State Changes Requested
Delegated to: Simon Glass
Headers show

Commit Message

eric.gao@rock-chips.com April 6, 2017, 6:58 a.m. UTC
From: "Eric Gao" <eric.gao@rock-chips.com>

when enable PMIC rk808,the system will halt at very
 early stage,log is shown as bellow.

INFO:    plat_rockchip_pmu_init(1211): pd status 3e
INFO:    BL31: Initializing runtime services
INFO:    BL31: Preparing for EL3 exit to normal world
INFO:    Entry point address = 0x200000
INFO:    SPSR = 0x3c9
time 44561b, 0 (<<----Just stop here)

It's caused by the absence of "{ }" in syscon_rk3399.c
,which will lead to memory overflow like below.According
 to Sysmap file ,we can find the function buck_get_value
of rk808 is just follow the compatible struct,the pointer
"of_match" point to "buck_get_value",but it is not a
struct and don't have member of compatible, In this case,
system crash. So,on the face, it looks like that rk808 is
guilty.but he is really innocent.

while (of_match->compatible) { <<----------
    if (!strcmp(of_match->compatible, compat)) {
    *of_idp = of_match;
    return 0;
    }
    of_match++;
}

Signed-off-by: eric.gao <eric.gao@rock-chips.com>
Signed-off-by: Eric Gao <eric.gao@rock-chips.com>
---

 arch/arm/mach-rockchip/rk3399/syscon_rk3399.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Simon Glass April 9, 2017, 7:28 p.m. UTC | #1
Hi Eric,

On 6 April 2017 at 00:58, Eric Gao <eric.gao@rock-chips.com> wrote:
> From: "Eric Gao" <eric.gao@rock-chips.com>
>
> when enable PMIC rk808,the system will halt at very
>  early stage,log is shown as bellow.
>
> INFO:    plat_rockchip_pmu_init(1211): pd status 3e
> INFO:    BL31: Initializing runtime services
> INFO:    BL31: Preparing for EL3 exit to normal world
> INFO:    Entry point address = 0x200000
> INFO:    SPSR = 0x3c9
> time 44561b, 0 (<<----Just stop here)
>
> It's caused by the absence of "{ }" in syscon_rk3399.c
> ,which will lead to memory overflow like below.According
>  to Sysmap file ,we can find the function buck_get_value
> of rk808 is just follow the compatible struct,the pointer
> "of_match" point to "buck_get_value",but it is not a
> struct and don't have member of compatible, In this case,
> system crash. So,on the face, it looks like that rk808 is
> guilty.but he is really innocent.
>
> while (of_match->compatible) { <<----------
>     if (!strcmp(of_match->compatible, compat)) {
>     *of_idp = of_match;
>     return 0;
>     }
>     of_match++;
> }

Great description, but for your commit title, could you say something
like 'rockchip: rk3399: Add missing sentinel in syscon'?

Your current message suggests that you are trying to halt the board.

>
> Signed-off-by: eric.gao <eric.gao@rock-chips.com>
> Signed-off-by: Eric Gao <eric.gao@rock-chips.com>

Some inconsistency here. Can you use the same capital letters?

> ---
>
>  arch/arm/mach-rockchip/rk3399/syscon_rk3399.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm/mach-rockchip/rk3399/syscon_rk3399.c b/arch/arm/mach-rockchip/rk3399/syscon_rk3399.c
> index d32985b..74d4552 100644
> --- a/arch/arm/mach-rockchip/rk3399/syscon_rk3399.c
> +++ b/arch/arm/mach-rockchip/rk3399/syscon_rk3399.c
> @@ -14,6 +14,7 @@ static const struct udevice_id rk3399_syscon_ids[] = {
>         { .compatible = "rockchip,rk3399-pmugrf", .data = ROCKCHIP_SYSCON_PMUGRF },
>         { .compatible = "rockchip,rk3399-pmusgrf", .data = ROCKCHIP_SYSCON_PMUSGRF },
>         { .compatible = "rockchip,rk3399-cic", .data = ROCKCHIP_SYSCON_CIC },
> +       { }
>  };
>
>  U_BOOT_DRIVER(syscon_rk3399) = {
> --
> 1.9.1
>
>

Regards,
Simon
diff mbox

Patch

diff --git a/arch/arm/mach-rockchip/rk3399/syscon_rk3399.c b/arch/arm/mach-rockchip/rk3399/syscon_rk3399.c
index d32985b..74d4552 100644
--- a/arch/arm/mach-rockchip/rk3399/syscon_rk3399.c
+++ b/arch/arm/mach-rockchip/rk3399/syscon_rk3399.c
@@ -14,6 +14,7 @@  static const struct udevice_id rk3399_syscon_ids[] = {
 	{ .compatible = "rockchip,rk3399-pmugrf", .data = ROCKCHIP_SYSCON_PMUGRF },
 	{ .compatible = "rockchip,rk3399-pmusgrf", .data = ROCKCHIP_SYSCON_PMUSGRF },
 	{ .compatible = "rockchip,rk3399-cic", .data = ROCKCHIP_SYSCON_CIC },
+	{ }
 };
 
 U_BOOT_DRIVER(syscon_rk3399) = {