On Fri, Aug 26, 2016 at 07:12:50PM +0800, Anson Huang wrote: > diff --git a/arch/arm/mach-imx/gpcv2.c b/arch/arm/mach-imx/gpcv2.c > new file mode 100644 > index 0000000..98577c4 > --- /dev/null > +++ b/arch/arm/mach-imx/gpcv2.c > @@ -0,0 +1,66 @@ > +/* > + * Copyright 2016 Freescale Semiconductor, Inc. > + * > + * The code contained herein is licensed under the GNU General Public > + * License. You may obtain a copy of the GNU General Public License > + * Version 2 or later at the following locations: > + * > + * http://www.opensource.org/licenses/gpl-license.html > + * http://www.gnu.org/copyleft/gpl.html > + */ > + > +#include <linux/clk.h> > +#include <linux/delay.h> > +#include <linux/io.h> > +#include <linux/of.h> > +#include <linux/of_address.h> > + > +#include "common.h" > + > +#define GPC_CPU_PGC_SW_PUP_REQ 0xf0 > +#define GPC_CPU_PGC_SW_PDN_REQ 0xfc > +#define GPC_PGC_C1 0x840 > + > +#define BM_CPU_PGC_SW_PDN_PUP_REQ_CORE1_A7 0x2 > +#define BM_GPC_PGC_PCG 0x1 > + > +static void __iomem *gpcv2_base; > + > +static void imx_gpcv2_set_m_core_pgc(bool enable, u32 offset) > +{ > + u32 val = readl_relaxed(gpcv2_base + offset) & (~BM_GPC_PGC_PCG); Unnecessary parens, and the code doesn't flow with the bit clearance here... > + > + if (enable) > + val |= BM_GPC_PGC_PCG; My first read of this lead me to think "okay, so what's the point of enable=false". It would be clearer with: u32 val = readl_relaxed(gpcv2_base + offset); if (enable) val |= BM_GPC_PGC_PCG; else val &= ~BM_GPC_PGC_PCG; here. > + > + writel_relaxed(val, gpcv2_base + offset); > +} > + > +void imx_gpcv2_set_core1_pdn_pup_by_software(bool pdn) > +{ > + u32 val = readl_relaxed(gpcv2_base + (pdn ? > + GPC_CPU_PGC_SW_PDN_REQ : GPC_CPU_PGC_SW_PUP_REQ)); > + > + imx_gpcv2_set_m_core_pgc(true, GPC_PGC_C1); > + val |= BM_CPU_PGC_SW_PDN_PUP_REQ_CORE1_A7; > + writel_relaxed(val, gpcv2_base + (pdn ? > + GPC_CPU_PGC_SW_PDN_REQ : GPC_CPU_PGC_SW_PUP_REQ)); > + > + while ((readl_relaxed(gpcv2_base + (pdn ? > + GPC_CPU_PGC_SW_PDN_REQ : GPC_CPU_PGC_SW_PUP_REQ)) & > + BM_CPU_PGC_SW_PDN_PUP_REQ_CORE1_A7) != 0) > + ; > + imx_gpcv2_set_m_core_pgc(false, GPC_PGC_C1); > +} > + > +void __init imx_gpcv2_check_dt(void) > +{ > + struct device_node *np; > + > + np = of_find_compatible_node(NULL, NULL, "fsl,imx7d-gpc"); > + if (WARN_ON(!np)) > + return; > + > + gpcv2_base = of_iomap(np, 0); > + WARN_ON(!gpcv2_base); What happens if gpcv2_base is NULL (apart from the obvious warning from the above line)? I guess a bit later in the boot, imx_gpcv2_set_core1_pdn_pup_by_software() oopses the kernel, probably before the console has been initialised. Probably not nice behaviour. > +} > -- > 1.9.1 > -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
