| Message ID | 1420793987-7621-8-git-send-email-Peng.Fan@freescale.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Stefano Babic |
| Headers | show |
Hi Peng, On 09/01/2015 09:59, Peng Fan wrote: > The basic graph for voltage input is: > VDDARM_IN ---> LDO_DIG(ARM) ---> VDD_ARM_CAP > VDDSOC_IN ---> LDO_DIG(SOC) ---> VDD_SOC_CAP > > We can bypass the LDO to save power, if the board already has pmic. > > set_anatop_bypass is the function to do the bypass VDDARM and VDDSOC > work. > > Current only set VDDARM_IN@1.175V/VDDSOC_IN@1.175V before ldo > bypass switch. So until ldo bypass switch happened, these voltage > setting is set in ldo-enable mode. But in datasheet, we need > 1.15V + 125mV = 1.275V for VDDARM_IN. We need to downgrade cpufreq > to 400Mhz and restore after ldo bypass mode switch. So add > prep_anatop_bypass/finish_anatop_bypass/set_arm_freq_400M to do > this work. > > LDO bypass is dependent on the flatten device tree file. If speed > grading fuse is for 1.2GHz, enable LDO bypass and setup PMIC voltages. > So add check for 1.2GHz core speed. So add check_1_2G function. > > In ldo-bypass mode, we need trigger WDOG_B pin to reset pmic in > ldo-bypass mode. So add set_wdog_reset to do this work. > > Also add related function prototype in sys_proto.h > Ok - with this explanation, I would try to understand how the changes can be split. If the feature/change works for several boards, it makes sense to have it common and general. If it is only for one board, must flow into the board directory. It looks like that ldo-bypass is strictly dependent on the board. Firstly, it must have PMIC, and not all boards have it. Your last sentence: > In ldo-bypass mode, we need trigger WDOG_B pin to reset pmic in > ldo-bypass mode. So add set_wdog_reset to do this work. This looks to me as an item very bound to the board. Could it be possible to use another pin (I do not know the schematics, I remember that such as reset pin was fix on previous i.MX) ? If answer is yes, can these changes be used by other board or are they only for sabresd ? > Signed-off-by: Peng Fan <Peng.Fan@freescale.com> > Signed-off-by: Robin Gong <b38343@freescale.com> > Signed-off-by: Nitin Garg <nitin.garg@freescale.com> > --- > arch/arm/cpu/armv7/mx6/soc.c | 141 ++++++++++++++++++++++++++++++ > arch/arm/include/asm/arch-mx6/sys_proto.h | 9 ++ > 2 files changed, 150 insertions(+) > > diff --git a/arch/arm/cpu/armv7/mx6/soc.c b/arch/arm/cpu/armv7/mx6/soc.c > index 5f5f497..5d02755 100644 > --- a/arch/arm/cpu/armv7/mx6/soc.c > +++ b/arch/arm/cpu/armv7/mx6/soc.c > @@ -18,6 +18,7 @@ > #include <asm/arch/sys_proto.h> > #include <asm/imx-common/boot_mode.h> > #include <asm/imx-common/dma.h> > +#include <libfdt.h> > #include <stdbool.h> > #include <asm/arch/mxc_hdmi.h> > #include <asm/arch/crm_regs.h> > @@ -429,6 +430,146 @@ void s_init(void) > writel(mask528, &anatop->pfd_528_clr); > } > > +#ifdef CONFIG_LDO_BYPASS_CHECK > +DECLARE_GLOBAL_DATA_PTR; > +static int ldo_bypass; mmmhh....global to the module ? > + > +int check_ldo_bypass(void) > +{ > + const int *ldo_mode; > + int node; > + > + /* get the right fdt_blob from the global working_fdt */ > + gd->fdt_blob = working_fdt; > + /* Get the node from FDT for anatop ldo-bypass */ > + node = fdt_node_offset_by_compatible(gd->fdt_blob, -1, > + "fsl,imx6q-gpc"); > + if (node < 0) { > + printf("No gpc device node %d, force to ldo-enable.\n", node); > + return 0; > + } > + ldo_mode = fdt_getprop(gd->fdt_blob, node, "fsl,ldo-bypass", NULL); I am quite lost. I have searched in kernel (current TOT), and I have not found such property. Can you help me to understand ? > + /* > + * return 1 if "fsl,ldo-bypass = <1>", else return 0 if > + * "fsl,ldo-bypass = <0>" or no "fsl,ldo-bypass" property > + */ > + ldo_bypass = fdt32_to_cpu(*ldo_mode) == 1 ? 1 : 0; > + > + return ldo_bypass; > +} > + > +int check_1_2G(void) > +{ > + u32 reg; > + int result = 0; > + struct ocotp_regs *ocotp = (struct ocotp_regs *)OCOTP_BASE_ADDR; > + struct fuse_bank *bank = &ocotp->bank[0]; > + struct fuse_bank0_regs *fuse_bank0 = > + (struct fuse_bank0_regs *)bank->fuse_regs; > + > + reg = readl(&fuse_bank0->cfg3); > + if (((reg >> 16) & 0x3) == 0x3) { > + if (ldo_bypass) { > + printf("Wrong dtb file used! i.MX6Q@1.2Ghz only works with ldo-enable mode!\n"); > + /* > + * Currently, only imx6q-sabresd board might be here, > + * since only i.MX6Q support 1.2G and only Sabresd board > + * support ldo-bypass mode. So hardcode here. > + * You can also modify your board(i.MX6Q) dtb name if it > + * supports both ldo-bypass and ldo-enable mode. This enforce my doubts. > + */ > + printf("Please use imx6q-sabresd-ldo.dtb!\n"); In any case, do not use hard-coded filenames into u-boot. They can change. > + hang(); > + } > + result = 1; > + } > + > + return result; > +} > + > +static int arm_orig_podf; > +void set_arm_freq_400M(bool is_400M) > +{ > + struct mxc_ccm_reg *mxc_ccm = (struct mxc_ccm_reg *)CCM_BASE_ADDR; > + > + if (is_400M) > + writel(0x1, &mxc_ccm->cacrr); > + else > + writel(arm_orig_podf, &mxc_ccm->cacrr); > +} > + > +void prep_anatop_bypass(void) > +{ > + struct mxc_ccm_reg *mxc_ccm = (struct mxc_ccm_reg *)CCM_BASE_ADDR; > + > + arm_orig_podf = readl(&mxc_ccm->cacrr); > + /* > + * Downgrade ARM speed to 400Mhz as half of boot 800Mhz before ldo > + * bypassed, also downgrade internal vddarm ldo to 0.975V. > + * VDDARM_IN 0.975V + 125mV = 1.1V < Max(1.3V) > + * otherwise at 800Mhz(i.mx6dl): > + * VDDARM_IN 1.175V + 125mV = 1.3V = Max(1.3V) > + * We need provide enough gap in this case. > + * skip if boot from 400M. > + */ > + if (!arm_orig_podf) > + set_arm_freq_400M(true); > +#if !defined(CONFIG_MX6DL) && !defined(CONFIG_MX6SX) > + set_ldo_voltage(LDO_ARM, 975); > +#else > + set_ldo_voltage(LDO_ARM, 1150); > +#endif > +} > + > +void set_wdog_reset(struct wdog_regs *wdog) > +{ > + u32 reg = readw(&wdog->wcr); > + /* > + * use WDOG_B mode to reset external pmic because it's risky for the > + * following watchdog reboot in case of cpu freq at lowest 400Mhz with > + * ldo-bypass mode. Because boot frequency maybe higher 800Mhz i.e. So > + * in ldo-bypass mode watchdog reset will only triger POR reset, not > + * WDOG reset. But below code depends on hardware design, if HW didn't > + * connect WDOG_B pin to external pmic such as i.mx6slevk, we can skip > + * these code since it assumed boot from 400Mhz always. > + */ > + reg = readw(&wdog->wcr); > + reg |= 1 << 3; > + /* > + * WDZST bit is write-once only bit. Align this bit in kernel, > + * otherwise kernel code will have no chance to set this bit. > + */ > + reg |= 1 << 0; > + writew(reg, &wdog->wcr); > +} > + > +int set_anatop_bypass(int wdog_reset_pin) > +{ > + struct mxc_ccm_reg *ccm_regs = (struct mxc_ccm_reg *)CCM_BASE_ADDR; > + struct wdog_regs *wdog; > + u32 reg = readl(&ccm_regs->reg_core); > + > + /* bypass VDDARM/VDDSOC */ > + reg = reg | (0x1F << 18) | 0x1F; > + writel(reg, &ccm_regs->reg_core); > + > + if (wdog_reset_pin == 2) > + wdog = (struct wdog_regs *)WDOG2_BASE_ADDR; > + else if (wdog_reset_pin == 1) > + wdog = (struct wdog_regs *)WDOG1_BASE_ADDR; > + else > + return arm_orig_podf; > + set_wdog_reset(wdog); > + return arm_orig_podf; > +} > + > +void finish_anatop_bypass(void) > +{ > + if (!arm_orig_podf) > + set_arm_freq_400M(false); > +} > +#endif > + > #ifdef CONFIG_IMX_HDMI > void imx_enable_hdmi_phy(void) > { > diff --git a/arch/arm/include/asm/arch-mx6/sys_proto.h b/arch/arm/include/asm/arch-mx6/sys_proto.h > index 28ba844..e6f2112 100644 > --- a/arch/arm/include/asm/arch-mx6/sys_proto.h > +++ b/arch/arm/include/asm/arch-mx6/sys_proto.h > @@ -28,6 +28,15 @@ const char *get_imx_type(u32 imxtype); > unsigned imx_ddr_size(void); > void set_chipselect_size(int const); > > +void set_wdog_reset(struct wdog_regs *wdog); > +#ifdef CONFIG_LDO_BYPASS_CHECK Why do we need #ifdef ? > +int check_ldo_bypass(void); > +int check_1_2G(void); > +void ldo_mode_set(int ldo_bypass); > +int set_anatop_bypass(int wdog_reset_pin); > +void prep_anatop_bypass(void); > +void finish_anatop_bypass(void); > +#endif > /* > * Initializes on-chip ethernet controllers. > * to override, implement board_eth_init() > Best regards, Stefano Babic
On Fri, Jan 9, 2015 at 12:59 AM, Peng Fan <Peng.Fan@freescale.com> wrote: > The basic graph for voltage input is: > VDDARM_IN ---> LDO_DIG(ARM) ---> VDD_ARM_CAP > VDDSOC_IN ---> LDO_DIG(SOC) ---> VDD_SOC_CAP > Hi Peng, Glad to see someone else interested in IMX6 LDO bypass mode. I've made a couple of stabs at getting it supported in mainline but I haven't had the time to follow-through yet there. > We can bypass the LDO to save power, if the board already has pmic. > > set_anatop_bypass is the function to do the bypass VDDARM and VDDSOC > work. > > Current only set VDDARM_IN@1.175V/VDDSOC_IN@1.175V before ldo > bypass switch. So until ldo bypass switch happened, these voltage > setting is set in ldo-enable mode. But in datasheet, we need > 1.15V + 125mV = 1.275V for VDDARM_IN. We need to downgrade cpufreq > to 400Mhz and restore after ldo bypass mode switch. So add > prep_anatop_bypass/finish_anatop_bypass/set_arm_freq_400M to do > this work. > > LDO bypass is dependent on the flatten device tree file. If speed > grading fuse is for 1.2GHz, enable LDO bypass and setup PMIC voltages. > So add check for 1.2GHz core speed. So add check_1_2G function. This isn't quite how it works. If you are 'operating at 1.2GHz' (supposing you had a processor cabable of it) you must use the LDO (to avoid ripple sensitivity issues). > > In ldo-bypass mode, we need trigger WDOG_B pin to reset pmic in > ldo-bypass mode. So add set_wdog_reset to do this work. This is very board dependent. Here you are referring to a board that has a reset input to the PMIC's from the IMX6's watchdog output. In this case, this reset routing/pinmux would be needed regardless of using ldo-bypass mode or not and that should just be a pinmux of the pin your using for WDOG_B. > <snip> > diff --git a/arch/arm/cpu/armv7/mx6/soc.c b/arch/arm/cpu/armv7/mx6/soc.c > index 5f5f497..5d02755 100644 > --- a/arch/arm/cpu/armv7/mx6/soc.c > +++ b/arch/arm/cpu/armv7/mx6/soc.c > @@ -18,6 +18,7 @@ > #include <asm/arch/sys_proto.h> > #include <asm/imx-common/boot_mode.h> > #include <asm/imx-common/dma.h> > +#include <libfdt.h> > #include <stdbool.h> > #include <asm/arch/mxc_hdmi.h> > #include <asm/arch/crm_regs.h> > @@ -429,6 +430,146 @@ void s_init(void) > writel(mask528, &anatop->pfd_528_clr); > } > > +#ifdef CONFIG_LDO_BYPASS_CHECK > +DECLARE_GLOBAL_DATA_PTR; > +static int ldo_bypass; > + > +int check_ldo_bypass(void) > +{ > + const int *ldo_mode; > + int node; > + > + /* get the right fdt_blob from the global working_fdt */ > + gd->fdt_blob = working_fdt; > + /* Get the node from FDT for anatop ldo-bypass */ > + node = fdt_node_offset_by_compatible(gd->fdt_blob, -1, > + "fsl,imx6q-gpc"); > + if (node < 0) { > + printf("No gpc device node %d, force to ldo-enable.\n", node); > + return 0; > + } > + ldo_mode = fdt_getprop(gd->fdt_blob, node, "fsl,ldo-bypass", NULL); > + /* > + * return 1 if "fsl,ldo-bypass = <1>", else return 0 if > + * "fsl,ldo-bypass = <0>" or no "fsl,ldo-bypass" property > + */ > + ldo_bypass = fdt32_to_cpu(*ldo_mode) == 1 ? 1 : 0; > + > + return ldo_bypass; > +} What you are doing here is relying on a device-tree binding from the Freescale 'vendor' kernel, which will NEVER make it upstream and this is one of the issues I was running into getting ldo-bypass capability upstream in the kernel. The issue here is that LDO bypass is dependent on the following things: 1. your voltage rail requirements - which are dependent on the CPU frequency (there is a nice table in the IMX6 datasheets of voltage on each rail at each frequency operating point validated by Freescale). The exception of always using the LDO for 1.2GHz is specified here as well. 2. you have a PMIC in your design on VDD_ARM_IN and VDD_SOC_IN rails - this should be specified in the device-tree as well 3. you have valid PMIC drivers configured In the kernel, its not desired to have a single device-tree node called 'fsl,ldo-bypass' for an enable. Instead you need to make sure that you PMIC regulators that are 'not' the internal IMX6 anatop regulators. This property is not a mainline linux device-tree binding. > + > +int check_1_2G(void) > +{ > + u32 reg; > + int result = 0; > + struct ocotp_regs *ocotp = (struct ocotp_regs *)OCOTP_BASE_ADDR; > + struct fuse_bank *bank = &ocotp->bank[0]; > + struct fuse_bank0_regs *fuse_bank0 = > + (struct fuse_bank0_regs *)bank->fuse_regs; > + > + reg = readl(&fuse_bank0->cfg3); > + if (((reg >> 16) & 0x3) == 0x3) { > + if (ldo_bypass) { > + printf("Wrong dtb file used! i.MX6Q@1.2Ghz only works with ldo-enable mode!\n"); > + /* > + * Currently, only imx6q-sabresd board might be here, > + * since only i.MX6Q support 1.2G and only Sabresd board > + * support ldo-bypass mode. So hardcode here. > + * You can also modify your board(i.MX6Q) dtb name if it > + * supports both ldo-bypass and ldo-enable mode. > + */ > + printf("Please use imx6q-sabresd-ldo.dtb!\n"); > + hang(); > + } > + result = 1; > + } > + > + return result; > +} While it is correct that you must not use LDO bypass when operating at 1.2GHz, that does not mean that a CPU capable of 1.2GHz can't use LDO bypass at the lower operating points. > + > +static int arm_orig_podf; > +void set_arm_freq_400M(bool is_400M) > +{ > + struct mxc_ccm_reg *mxc_ccm = (struct mxc_ccm_reg *)CCM_BASE_ADDR; > + > + if (is_400M) > + writel(0x1, &mxc_ccm->cacrr); > + else > + writel(arm_orig_podf, &mxc_ccm->cacrr); > +} I have no idea what is going on here. Are we now running at different CPU frequencies in U-boot? The IMX6 comes up in either 800MHz or 400MHz per BOOT_CFG3 e-fuse and from the U-Boot I'm working with (still on 2010.04 but pretty sure its the same in 2014.10) the CPU frequency is never changed in the bootloader. This brings me to the question of why does LDO bypass have anything at all do to with the bootloader? It is true that the Freescale vendor kernel will keep the LDO in bypass mode if its registers are set that way from the bootloader, so that is probably what your after here. But again, that is a poor kernel implementation that likely won't make it upstream and a solution that doesn't handle the dynamic situation of a 1.2GHz cpu stepping down and not needing the LDO. It is also true that there are alternate device-tree's for the Freescale vendor kernel for some boards that have a PMIC such that one device-tree is setup to always use the LDO, and one is setup to never use the LDO. I think that is a result of taking the easy way out instead of giving the kernel the smarts to use the LDO only when needed on these boards. Tim
On Tue, Feb 10, 2015 at 3:23 AM, Stefano Babic <sbabic@denx.de> wrote: > > Ok - with this explanation, I would try to understand how the changes > can be split. If the feature/change works for several boards, it makes > sense to have it common and general. If it is only for one board, must > flow into the board directory. It should be common as there are several boards which use PMIC's and can use it. > > It looks like that ldo-bypass is strictly dependent on the board. > Firstly, it must have PMIC, and not all boards have it. Your last sentence: Any board that has a PMIC capable of regulating VDD_ARM_IN and VDD_SOC_IN to the setpoints from the IMX6 datasheet can operate in LDO bypass mode, unless operating at 1.2GHz in which case the datasheet states that the LDO must be used (not bypassed) to avoid ripple sensitivity issues. > >> In ldo-bypass mode, we need trigger WDOG_B pin to reset pmic in >> ldo-bypass mode. So add set_wdog_reset to do this work. > > This looks to me as an item very bound to the board. Could it be > possible to use another pin (I do not know the schematics, I remember > that such as reset pin was fix on previous i.MX) ? If answer is yes, can > these changes be used by other board or are they only for sabresd ? > agreed - this is a board-specific pinmux > >> Signed-off-by: Peng Fan <Peng.Fan@freescale.com> >> Signed-off-by: Robin Gong <b38343@freescale.com> >> Signed-off-by: Nitin Garg <nitin.garg@freescale.com> >> --- >> arch/arm/cpu/armv7/mx6/soc.c | 141 ++++++++++++++++++++++++++++++ >> arch/arm/include/asm/arch-mx6/sys_proto.h | 9 ++ >> 2 files changed, 150 insertions(+) >> >> diff --git a/arch/arm/cpu/armv7/mx6/soc.c b/arch/arm/cpu/armv7/mx6/soc.c >> index 5f5f497..5d02755 100644 >> --- a/arch/arm/cpu/armv7/mx6/soc.c >> +++ b/arch/arm/cpu/armv7/mx6/soc.c >> @@ -18,6 +18,7 @@ >> #include <asm/arch/sys_proto.h> >> #include <asm/imx-common/boot_mode.h> >> #include <asm/imx-common/dma.h> >> +#include <libfdt.h> >> #include <stdbool.h> >> #include <asm/arch/mxc_hdmi.h> >> #include <asm/arch/crm_regs.h> >> @@ -429,6 +430,146 @@ void s_init(void) >> writel(mask528, &anatop->pfd_528_clr); >> } >> >> +#ifdef CONFIG_LDO_BYPASS_CHECK >> +DECLARE_GLOBAL_DATA_PTR; >> +static int ldo_bypass; > > mmmhh....global to the module ? > >> + >> +int check_ldo_bypass(void) >> +{ >> + const int *ldo_mode; >> + int node; >> + >> + /* get the right fdt_blob from the global working_fdt */ >> + gd->fdt_blob = working_fdt; >> + /* Get the node from FDT for anatop ldo-bypass */ >> + node = fdt_node_offset_by_compatible(gd->fdt_blob, -1, >> + "fsl,imx6q-gpc"); >> + if (node < 0) { >> + printf("No gpc device node %d, force to ldo-enable.\n", node); >> + return 0; >> + } >> + ldo_mode = fdt_getprop(gd->fdt_blob, node, "fsl,ldo-bypass", NULL); > > I am quite lost. I have searched in kernel (current TOT), and I have not > found such property. Can you help me to understand ? Right - you won't find it because its a Freescale vendor kernel implementation only. A hack if you ask me to avoid having to doing ldo-byapss the right way. Here are the threads that I know of regarding ldo-bypass in the kernel, where it needs to be: https://lkml.org/lkml/2014/12/18/255 https://lkml.org/lkml/2014/10/31/3 Peng, I think what you are trying to do here is to put the anatop regulators in bypass mode so that the Freescale vendor kernel leaves them bypassed (which is what the 3.10.x based vendor kernels supporting device-tree at http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git do). This is what the Freescale vendor U-Boot does and so they have created a horrible dependence between kernel and bootloader. Instead you may be interested in what I did for our BSP's that use the Freescale vendor kernel. Instead of touching U-Boot, I look for the fsl,ldo-bypass node in the kernel and enable it just like their bootloader would have: https://github.com/Gateworks/linux-imx6/commit/a1af6ac6f00b4da7c8a5656e8ff093d4ab5cadee That said, I would love to see some help getting IMX6 ldo-bypass support upstream. All of our boards have an external PMIC and are capable of bypass mode. Bypassing the LDO on such boards really helps reduce overall board power consumption as well as move heat from the CPU to the PMIC. Tim
Hi Tim, On Tue, Feb 10, 2015 at 12:50 PM, Tim Harvey <tharvey@gateworks.com> wrote: > I think what you are trying to do here is to put the anatop regulators > in bypass mode so that the Freescale vendor kernel leaves them > bypassed (which is what the 3.10.x based vendor kernels supporting > device-tree at http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git > do). This is what the Freescale vendor U-Boot does and so they have > created a horrible dependence between kernel and bootloader. I agree. > Instead you may be interested in what I did for our BSP's that use the > Freescale vendor kernel. Instead of touching U-Boot, I look for the > fsl,ldo-bypass node in the kernel and enable it just like their > bootloader would have: > https://github.com/Gateworks/linux-imx6/commit/a1af6ac6f00b4da7c8a5656e8ff093d4ab5cadee > > That said, I would love to see some help getting IMX6 ldo-bypass > support upstream. All of our boards have an external PMIC and are I want to help you on upstreaming ldo-bypass support in the kernel, Tim. Can we do like your approach, but defining imx_anatop_ldobypass_enable() inside rivers/regulator/anatop-regulator.c instead? Regards, Fabio Estevam
On Tue, Feb 10, 2015 at 6:59 AM, Fabio Estevam <festevam@gmail.com> wrote: > Hi Tim, > > On Tue, Feb 10, 2015 at 12:50 PM, Tim Harvey <tharvey@gateworks.com> wrote: > >> I think what you are trying to do here is to put the anatop regulators >> in bypass mode so that the Freescale vendor kernel leaves them >> bypassed (which is what the 3.10.x based vendor kernels supporting >> device-tree at http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git >> do). This is what the Freescale vendor U-Boot does and so they have >> created a horrible dependence between kernel and bootloader. > > I agree. > >> Instead you may be interested in what I did for our BSP's that use the >> Freescale vendor kernel. Instead of touching U-Boot, I look for the >> fsl,ldo-bypass node in the kernel and enable it just like their >> bootloader would have: >> https://github.com/Gateworks/linux-imx6/commit/a1af6ac6f00b4da7c8a5656e8ff093d4ab5cadee >> >> That said, I would love to see some help getting IMX6 ldo-bypass >> support upstream. All of our boards have an external PMIC and are > > I want to help you on upstreaming ldo-bypass support in the kernel, Tim. Great! > > Can we do like your approach, but defining > imx_anatop_ldobypass_enable() inside > rivers/regulator/anatop-regulator.c instead? Yes, I think that makes sense. I hope to be able to get back to this in a couple of weeks after a round of U-Boot updates that are next on my list. Thanks, Tim
Hi Tim, On 10/02/2015 15:50, Tim Harvey wrote: > On Tue, Feb 10, 2015 at 3:23 AM, Stefano Babic <sbabic@denx.de> wrote: >> >> Ok - with this explanation, I would try to understand how the changes >> can be split. If the feature/change works for several boards, it makes >> sense to have it common and general. If it is only for one board, must >> flow into the board directory. > > It should be common as there are several boards which use PMIC's and can use it. > Fully agree. >> I am quite lost. I have searched in kernel (current TOT), and I have not >> found such property. Can you help me to understand ? > > Right - you won't find it because its a Freescale vendor kernel > implementation only. A hack if you ask me to avoid having to doing > ldo-byapss the right way. Exactly, this cannot flow into mainline. > > Here are the threads that I know of regarding ldo-bypass in the > kernel, where it needs to be: > > https://lkml.org/lkml/2014/12/18/255 > https://lkml.org/lkml/2014/10/31/3 Thanks for pointing out ! > > Peng, > > I think what you are trying to do here is to put the anatop regulators > in bypass mode so that the Freescale vendor kernel leaves them > bypassed (which is what the 3.10.x based vendor kernels supporting > device-tree at http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git > do). This is what the Freescale vendor U-Boot does and so they have > created a horrible dependence between kernel and bootloader. > You are perfectly right. > Instead you may be interested in what I did for our BSP's that use the > Freescale vendor kernel. Instead of touching U-Boot, I look for the > fsl,ldo-bypass node in the kernel and enable it just like their > bootloader would have: > https://github.com/Gateworks/linux-imx6/commit/a1af6ac6f00b4da7c8a5656e8ff093d4ab5cadee > Thanks - you are on the right direction ;-) Best regards, Stefano
On Tue, Feb 10, 2015 at 06:33:38AM -0800, Tim Harvey wrote: > On Fri, Jan 9, 2015 at 12:59 AM, Peng Fan <Peng.Fan@freescale.com> wrote: > > The basic graph for voltage input is: > > VDDARM_IN ---> LDO_DIG(ARM) ---> VDD_ARM_CAP > > VDDSOC_IN ---> LDO_DIG(SOC) ---> VDD_SOC_CAP > > > > Hi Peng, > > Glad to see someone else interested in IMX6 LDO bypass mode. I've made > a couple of stabs at getting it supported in mainline but I haven't > had the time to follow-through yet there. > > > We can bypass the LDO to save power, if the board already has pmic. > > > > set_anatop_bypass is the function to do the bypass VDDARM and VDDSOC > > work. > > > > Current only set VDDARM_IN@1.175V/VDDSOC_IN@1.175V before ldo > > bypass switch. So until ldo bypass switch happened, these voltage > > setting is set in ldo-enable mode. But in datasheet, we need > > 1.15V + 125mV = 1.275V for VDDARM_IN. We need to downgrade cpufreq > > to 400Mhz and restore after ldo bypass mode switch. So add > > prep_anatop_bypass/finish_anatop_bypass/set_arm_freq_400M to do > > this work. > > > > LDO bypass is dependent on the flatten device tree file. If speed > > grading fuse is for 1.2GHz, enable LDO bypass and setup PMIC voltages. > > So add check for 1.2GHz core speed. So add check_1_2G function. > > This isn't quite how it works. If you are 'operating at 1.2GHz' > (supposing you had a processor cabable of it) you must use the LDO (to > avoid ripple sensitivity issues). > Hi Peng, the limitation is "must use ldo-enable mode in 1.2Ghz", NOT ldo-bypass > > > > In ldo-bypass mode, we need trigger WDOG_B pin to reset pmic in > > ldo-bypass mode. So add set_wdog_reset to do this work. > > This is very board dependent. Here you are referring to a board that > has a reset input to the PMIC's from the IMX6's watchdog output. In > this case, this reset routing/pinmux would be needed regardless of > using ldo-bypass mode or not and that should just be a pinmux of the > pin your using for WDOG_B. > There are two types of reboot in our chip by watchdog : SRS/warm reset (Software Reset Signal) and WDOG_B(reset signal output to external pmic to trigger next power cycle). In fact, warm reset can work most cases except ldo-bypass mode and this is why we connect WDOG_B reset and ldo-bypass here: kernel may trigger warm reset at the lowest cpu freq setpoint, for example VDDARM 0.975v@396Mhz ldo-bypass mode. i.mx6q will reboot with ldo-enable mode @792Mhz and the VDDARM_IN 0.975v can't support system boot.Thus, we need use WDOG_B pin to reset external pmic if using ldo-byapss mode. > > > > <snip> > > > diff --git a/arch/arm/cpu/armv7/mx6/soc.c b/arch/arm/cpu/armv7/mx6/soc.c > > index 5f5f497..5d02755 100644 > > --- a/arch/arm/cpu/armv7/mx6/soc.c > > +++ b/arch/arm/cpu/armv7/mx6/soc.c > > @@ -18,6 +18,7 @@ > > #include <asm/arch/sys_proto.h> > > #include <asm/imx-common/boot_mode.h> > > #include <asm/imx-common/dma.h> > > +#include <libfdt.h> > > #include <stdbool.h> > > #include <asm/arch/mxc_hdmi.h> > > #include <asm/arch/crm_regs.h> > > @@ -429,6 +430,146 @@ void s_init(void) > > writel(mask528, &anatop->pfd_528_clr); > > } > > > > +#ifdef CONFIG_LDO_BYPASS_CHECK > > +DECLARE_GLOBAL_DATA_PTR; > > +static int ldo_bypass; > > + > > +int check_ldo_bypass(void) > > +{ > > + const int *ldo_mode; > > + int node; > > + > > + /* get the right fdt_blob from the global working_fdt */ > > + gd->fdt_blob = working_fdt; > > + /* Get the node from FDT for anatop ldo-bypass */ > > + node = fdt_node_offset_by_compatible(gd->fdt_blob, -1, > > + "fsl,imx6q-gpc"); > > + if (node < 0) { > > + printf("No gpc device node %d, force to ldo-enable.\n", node); > > + return 0; > > + } > > + ldo_mode = fdt_getprop(gd->fdt_blob, node, "fsl,ldo-bypass", NULL); > > + /* > > + * return 1 if "fsl,ldo-bypass = <1>", else return 0 if > > + * "fsl,ldo-bypass = <0>" or no "fsl,ldo-bypass" property > > + */ > > + ldo_bypass = fdt32_to_cpu(*ldo_mode) == 1 ? 1 : 0; > > + > > + return ldo_bypass; > > +} > > What you are doing here is relying on a device-tree binding from the > Freescale 'vendor' kernel, which will NEVER make it upstream and this > is one of the issues I was running into getting ldo-bypass capability > upstream in the kernel. > > The issue here is that LDO bypass is dependent on the following things: > 1. your voltage rail requirements - which are dependent on the CPU > frequency (there is a nice table in the IMX6 datasheets of voltage on > each rail at each frequency operating point validated by Freescale). > The exception of always using the LDO for 1.2GHz is specified here as > well. > 2. you have a PMIC in your design on VDD_ARM_IN and VDD_SOC_IN rails > - this should be specified in the device-tree as well > 3. you have valid PMIC drivers configured > > In the kernel, its not desired to have a single device-tree node > called 'fsl,ldo-bypass' for an enable. Instead you need to make sure > that you PMIC regulators that are 'not' the internal IMX6 anatop > regulators. This property is not a mainline linux device-tree binding. > Yes, understood. But we have no better solution, because we need touch both internal anatop regulator and external pmic regulator in ldo-bypass mode, that bring kernel cpufreq driver code too messy.... > > + > > +int check_1_2G(void) > > +{ > > + u32 reg; > > + int result = 0; > > + struct ocotp_regs *ocotp = (struct ocotp_regs *)OCOTP_BASE_ADDR; > > + struct fuse_bank *bank = &ocotp->bank[0]; > > + struct fuse_bank0_regs *fuse_bank0 = > > + (struct fuse_bank0_regs *)bank->fuse_regs; > > + > > + reg = readl(&fuse_bank0->cfg3); > > + if (((reg >> 16) & 0x3) == 0x3) { > > + if (ldo_bypass) { > > + printf("Wrong dtb file used! i.MX6Q@1.2Ghz only works with ldo-enable mode!\n"); > > + /* > > + * Currently, only imx6q-sabresd board might be here, > > + * since only i.MX6Q support 1.2G and only Sabresd board > > + * support ldo-bypass mode. So hardcode here. > > + * You can also modify your board(i.MX6Q) dtb name if it > > + * supports both ldo-bypass and ldo-enable mode. > > + */ > > + printf("Please use imx6q-sabresd-ldo.dtb!\n"); > > + hang(); > > + } > > + result = 1; > > + } > > + > > + return result; > > +} > > While it is correct that you must not use LDO bypass when operating at > 1.2GHz, that does not mean that a CPU capable of 1.2GHz can't use LDO > bypass at the lower operating points. > I see, but to simply things, we regard as 1.2GHz chip(fused) may running at 1.2GHz and force it work in ldo-enable mode although it has chance to running at 1Ghz. In other words, ldo-bypass mode only set once not dynamically. > > + > > +static int arm_orig_podf; > > +void set_arm_freq_400M(bool is_400M) > > +{ > > + struct mxc_ccm_reg *mxc_ccm = (struct mxc_ccm_reg *)CCM_BASE_ADDR; > > + > > + if (is_400M) > > + writel(0x1, &mxc_ccm->cacrr); > > + else > > + writel(arm_orig_podf, &mxc_ccm->cacrr); > > +} > > I have no idea what is going on here. Are we now running at different > CPU frequencies in U-boot? The IMX6 comes up in either 800MHz or > 400MHz per BOOT_CFG3 e-fuse and from the U-Boot I'm working with > (still on 2010.04 but pretty sure its the same in 2014.10) the CPU > frequency is never changed in the bootloader. > Let's try to explain why we downgrade cpufreq to 400Mhz before ldo-bypass mode switch(i.mx6q): cpufreq VDDSOC VDDARM 400Mhz 0.975v 1.175v 800Mhz 1.175v 1.175v If system boot from 800Mhz, considering VDDARM setting with 1.175v after ldo-bypass mode switch, and the voltage drop which bring by internal regulator 125mv, we have to set VDDARM 1.175v+125mv = 1.3v before ldo-bypass mode switch, but the 1.3v beyond our max voltage for VDDARM. So we have to downgrade the cpufreq to 400Mhz. > This brings me to the question of why does LDO bypass have anything at > all do to with the bootloader? > We only need do once ldo-bypass switch, so we hope it can be implemented in u-boot > It is true that the Freescale vendor kernel will keep the LDO in > bypass mode if its registers are set that way from the bootloader, so > that is probably what your after here. But again, that is a poor > kernel implementation that likely won't make it upstream and a > solution that doesn't handle the dynamic situation of a 1.2GHz cpu > stepping down and not needing the LDO. It is also true that there are > alternate device-tree's for the Freescale vendor kernel for some > boards that have a PMIC such that one device-tree is setup to always > use the LDO, and one is setup to never use the LDO. I think that is a > result of taking the easy way out instead of giving the kernel the > smarts to use the LDO only when needed on these boards. > > Tim
On Wed, Feb 11, 2015 at 2:49 AM, Robin Gong <b38343@freescale.com> wrote: <snip> >> >> This is very board dependent. Here you are referring to a board that >> has a reset input to the PMIC's from the IMX6's watchdog output. In >> this case, this reset routing/pinmux would be needed regardless of >> using ldo-bypass mode or not and that should just be a pinmux of the >> pin your using for WDOG_B. >> > There are two types of reboot in our chip by watchdog : SRS/warm reset > (Software Reset Signal) and WDOG_B(reset signal output to external pmic > to trigger next power cycle). In fact, warm reset can work most cases except > ldo-bypass mode and this is why we connect WDOG_B reset and ldo-bypass here: > kernel may trigger warm reset at the lowest cpu freq setpoint, for example > VDDARM 0.975v@396Mhz ldo-bypass mode. i.mx6q will reboot with ldo-enable mode > @792Mhz and the VDDARM_IN 0.975v can't support system boot.Thus, we need use > WDOG_B pin to reset external pmic if using ldo-byapss mode. Hi Robin, I understand that configuring WDOG_B external reset must be used when LDO bypass is used. This should be done in the kernel but you can also set this pinmux up in uboot in the board-specific init. If your kernel is providing the PMIC driver and cpufreq driver that alter the cpu core frequencies it must also be configuring pinmux so that WDOG_B gets routed to the pin that connects to the PMIC so any reset based on that wdog (SRS or timeout) issues a hard reset to the PMIC. Failure to do so is a kernel bug. I believe this is done properly in the Freescale kernel for the reference boards. If you are trying to take care of an issue caused by a watchdog reset (SRS or timeout) not properly resetting a PMIC (ie perhaps a PCB errata where signal doesn't go to the PMIC) then you should probably simply set the PMIC rails where they need to be for the 800MHz operation in the bootloader and not muck with the CPU frequency. Honestly, if your PMIC rails are too low for 800MHz on powerup your bootloader may not be stable anyway right? Note that the operating setpoints have the same SOC voltage for both 792MHz and 396MHz, its only the ARM voltage that is lower for 396 vs 792 and chances are your not going to have an issue just in the bootloader at that point. <snip> >> >> What you are doing here is relying on a device-tree binding from the >> Freescale 'vendor' kernel, which will NEVER make it upstream and this >> is one of the issues I was running into getting ldo-bypass capability >> upstream in the kernel. >> >> The issue here is that LDO bypass is dependent on the following things: >> 1. your voltage rail requirements - which are dependent on the CPU >> frequency (there is a nice table in the IMX6 datasheets of voltage on >> each rail at each frequency operating point validated by Freescale). >> The exception of always using the LDO for 1.2GHz is specified here as >> well. >> 2. you have a PMIC in your design on VDD_ARM_IN and VDD_SOC_IN rails >> - this should be specified in the device-tree as well >> 3. you have valid PMIC drivers configured >> >> In the kernel, its not desired to have a single device-tree node >> called 'fsl,ldo-bypass' for an enable. Instead you need to make sure >> that you PMIC regulators that are 'not' the internal IMX6 anatop >> regulators. This property is not a mainline linux device-tree binding. >> > Yes, understood. But we have no better solution, because we need touch both > internal anatop regulator and external pmic regulator in ldo-bypass mode, that > bring kernel cpufreq driver code too messy.... I just don't see the point in having the CPU frequency changed in the bootloader - leave this up to the OS kernel. I don't think any of this patch should go into mainline uboot. I think my kernel patch I referenced provides you with a simple kernel solution that de-couples ldo-bypass completely from the bootloader. <snip> >> >> While it is correct that you must not use LDO bypass when operating at >> 1.2GHz, that does not mean that a CPU capable of 1.2GHz can't use LDO >> bypass at the lower operating points. >> > I see, but to simply things, we regard as 1.2GHz chip(fused) may running at > 1.2GHz and force it work in ldo-enable mode although it has chance to running > at 1Ghz. In other words, ldo-bypass mode only set once not dynamically. Are you saying there is an IMX6 variant that powers up per eFUSE settings at 1.2GHz? The IMX6QDLRM efuse settings I'm looking at just have two power-up frequency options: 392MHz and 792MHz. In my opinion, your PMIC should be setting VDD_ARM and VDD_SOC at the necessary voltages for what the CPU is currently running at, in the bootloader. Its up to your OS to properly control this to its needs later. Again, if for some reason (hardware errata) you have a situation where the PMIC maybe didn't get reset and the board powers up into the bootloader at a frequency higher than the rails are set to, then simply set its rails to where they need to be for the freq your running at in the bootloader. >> > + >> > +static int arm_orig_podf; >> > +void set_arm_freq_400M(bool is_400M) >> > +{ >> > + struct mxc_ccm_reg *mxc_ccm = (struct mxc_ccm_reg *)CCM_BASE_ADDR; >> > + >> > + if (is_400M) >> > + writel(0x1, &mxc_ccm->cacrr); >> > + else >> > + writel(arm_orig_podf, &mxc_ccm->cacrr); >> > +} >> >> I have no idea what is going on here. Are we now running at different >> CPU frequencies in U-boot? The IMX6 comes up in either 800MHz or >> 400MHz per BOOT_CFG3 e-fuse and from the U-Boot I'm working with >> (still on 2010.04 but pretty sure its the same in 2014.10) the CPU >> frequency is never changed in the bootloader. >> > Let's try to explain why we downgrade cpufreq to 400Mhz before ldo-bypass mode > switch(i.mx6q): > cpufreq VDDSOC VDDARM > 400Mhz 0.975v 1.175v > 800Mhz 1.175v 1.175v > If system boot from 800Mhz, considering VDDARM setting with 1.175v after > ldo-bypass mode switch, and the voltage drop which bring by internal regulator > 125mv, we have to set VDDARM 1.175v+125mv = 1.3v before ldo-bypass mode switch, > but the 1.3v beyond our max voltage for VDDARM. So we have to downgrade the cpufreq > to 400Mhz. Too much complexity in my opinion for the power reduction benefits while in the bootloader. Leave ldo-bypass out of the bootloader and you won't have to bother with this. The IMX6 can handle a max of 1.3V on VDD_ARM_IN and VDD_SOC_IN (and 1.5V on the output side of the LDO). So you can simply set your PMIC in the bootloader to something between that and 1.175v and be done with it right? >> This brings me to the question of why does LDO bypass have anything at >> all do to with the bootloader? >> > We only need do once ldo-bypass switch, so we hope it can be implemented in > u-boot It doesn't belong in uboot. It was wrong for Freescale to create a dependence between the bootloader and the kernel when it is so easy to avoid. I don't think any of this particular patch should go into mainline uboot. Regards, Tim
On Wed, Feb 11, 2015 at 7:47 AM, Tim Harvey <tharvey@gateworks.com> wrote: > On Wed, Feb 11, 2015 at 2:49 AM, Robin Gong <b38343@freescale.com> wrote: > <snip> >>> >>> This is very board dependent. Here you are referring to a board that >>> has a reset input to the PMIC's from the IMX6's watchdog output. In >>> this case, this reset routing/pinmux would be needed regardless of >>> using ldo-bypass mode or not and that should just be a pinmux of the >>> pin your using for WDOG_B. >>> >> There are two types of reboot in our chip by watchdog : SRS/warm reset >> (Software Reset Signal) and WDOG_B(reset signal output to external pmic >> to trigger next power cycle). In fact, warm reset can work most cases except >> ldo-bypass mode and this is why we connect WDOG_B reset and ldo-bypass here: >> kernel may trigger warm reset at the lowest cpu freq setpoint, for example >> VDDARM 0.975v@396Mhz ldo-bypass mode. i.mx6q will reboot with ldo-enable mode >> @792Mhz and the VDDARM_IN 0.975v can't support system boot.Thus, we need use >> WDOG_B pin to reset external pmic if using ldo-byapss mode. > > Hi Robin, > > I understand that configuring WDOG_B external reset must be used when > LDO bypass is used. This should be done in the kernel but you can also > set this pinmux up in uboot in the board-specific init. > > If your kernel is providing the PMIC driver and cpufreq driver that > alter the cpu core frequencies it must also be configuring pinmux so > that WDOG_B gets routed to the pin that connects to the PMIC so any > reset based on that wdog (SRS or timeout) issues a hard reset to the > PMIC. Failure to do so is a kernel bug. I believe this is done > properly in the Freescale kernel for the reference boards. > > If you are trying to take care of an issue caused by a watchdog reset > (SRS or timeout) not properly resetting a PMIC (ie perhaps a PCB > errata where signal doesn't go to the PMIC) then you should probably > simply set the PMIC rails where they need to be for the 800MHz > operation in the bootloader and not muck with the CPU frequency. > Honestly, if your PMIC rails are too low for 800MHz on powerup your > bootloader may not be stable anyway right? Note that the operating > setpoints have the same SOC voltage for both 792MHz and 396MHz, its > only the ARM voltage that is lower for 396 vs 792 and chances are your > not going to have an issue just in the bootloader at that point. > Robin, The issue your describing was interesting to me and I ran some tests and found that on a board with no reset to the PMIC, an IMX6Q CPU, ldo-bypass enabled, and the CPU freq set to 400MHz (such that VDD_ARM rail set to 0.960V for 400MHz operation) the chip does not even come out of reset (ie when SRS is set in the watchdog controller). So I don't really see any ability to work around this in bootloader software since you won't get there in this case. Possible solutions I can think of for boards without a PMIC reset is to either blow the eFUSE so the chip comes up in 400MHz or in the kernel never allow the VDD_ARM or VDD_SOC rail to go below where they need to be for CPU startup (the only one that does is VDD_ARM) or before soft-reboot make sure the cpufreq is at 800MHz or above (which must be done at higher levels before single-cpu mode in machine_restart). This also does not deal with the case of a watchdog reset and/or a crash handler. Are you findings different? Tim
On Thu, Feb 12, 2015 at 04:08:51PM -0800, Tim Harvey wrote: > On Wed, Feb 11, 2015 at 7:47 AM, Tim Harvey <tharvey@gateworks.com> wrote: > > On Wed, Feb 11, 2015 at 2:49 AM, Robin Gong <b38343@freescale.com> wrote: > > <snip> > >>> > >>> This is very board dependent. Here you are referring to a board that > >>> has a reset input to the PMIC's from the IMX6's watchdog output. In > >>> this case, this reset routing/pinmux would be needed regardless of > >>> using ldo-bypass mode or not and that should just be a pinmux of the > >>> pin your using for WDOG_B. > >>> > >> There are two types of reboot in our chip by watchdog : SRS/warm reset > >> (Software Reset Signal) and WDOG_B(reset signal output to external pmic > >> to trigger next power cycle). In fact, warm reset can work most cases except > >> ldo-bypass mode and this is why we connect WDOG_B reset and ldo-bypass here: > >> kernel may trigger warm reset at the lowest cpu freq setpoint, for example > >> VDDARM 0.975v@396Mhz ldo-bypass mode. i.mx6q will reboot with ldo-enable mode > >> @792Mhz and the VDDARM_IN 0.975v can't support system boot.Thus, we need use > >> WDOG_B pin to reset external pmic if using ldo-byapss mode. > > > > Hi Robin, > > > > I understand that configuring WDOG_B external reset must be used when > > LDO bypass is used. This should be done in the kernel but you can also > > set this pinmux up in uboot in the board-specific init. > > > > If your kernel is providing the PMIC driver and cpufreq driver that > > alter the cpu core frequencies it must also be configuring pinmux so > > that WDOG_B gets routed to the pin that connects to the PMIC so any > > reset based on that wdog (SRS or timeout) issues a hard reset to the > > PMIC. Failure to do so is a kernel bug. I believe this is done > > properly in the Freescale kernel for the reference boards. > > > > If you are trying to take care of an issue caused by a watchdog reset > > (SRS or timeout) not properly resetting a PMIC (ie perhaps a PCB > > errata where signal doesn't go to the PMIC) then you should probably > > simply set the PMIC rails where they need to be for the 800MHz > > operation in the bootloader and not muck with the CPU frequency. > > Honestly, if your PMIC rails are too low for 800MHz on powerup your > > bootloader may not be stable anyway right? Note that the operating > > setpoints have the same SOC voltage for both 792MHz and 396MHz, its > > only the ARM voltage that is lower for 396 vs 792 and chances are your > > not going to have an issue just in the bootloader at that point. > > > > Robin, > > The issue your describing was interesting to me and I ran some tests > and found that on a board with no reset to the PMIC, an IMX6Q CPU, > ldo-bypass enabled, and the CPU freq set to 400MHz (such that VDD_ARM > rail set to 0.960V for 400MHz operation) the chip does not even come > out of reset (ie when SRS is set in the watchdog controller). So I > don't really see any ability to work around this in bootloader > software since you won't get there in this case. Yes, ldo-bypass mode depends on reset PMIC while reboot happen. But even on this feature supported boards, we'd better provide warm reset in ldo-enable mode,for example, warm reset can keep printed log in DDR while system crash in last time and trigger watchdog reset. Someone may use DCDC instead of PMIC, or some board didn't connect WDOG_B reset with PMIC, have to use ldo-enable mode. > > Possible solutions I can think of for boards without a PMIC reset is > to either blow the eFUSE so the chip comes up in 400MHz or in the > kernel never allow the VDD_ARM or VDD_SOC rail to go below where they > need to be for CPU startup (the only one that does is VDD_ARM) or > before soft-reboot make sure the cpufreq is at 800MHz or above (which > must be done at higher levels before single-cpu mode in > machine_restart). This also does not deal with the case of a watchdog > reset and/or a crash handler. > Yes, absolutely. But we suggest connecting PMIC reset pin with WDOG_B or just use ldo-enable mode if they are not very care of power number. > Are you findings different? > > Tim
On Wed, Feb 11, 2015 at 07:47:57AM -0800, Tim Harvey wrote: > On Wed, Feb 11, 2015 at 2:49 AM, Robin Gong <b38343@freescale.com> wrote: > <snip> > >> > >> This is very board dependent. Here you are referring to a board that > >> has a reset input to the PMIC's from the IMX6's watchdog output. In > >> this case, this reset routing/pinmux would be needed regardless of > >> using ldo-bypass mode or not and that should just be a pinmux of the > >> pin your using for WDOG_B. > >> > > There are two types of reboot in our chip by watchdog : SRS/warm reset > > (Software Reset Signal) and WDOG_B(reset signal output to external pmic > > to trigger next power cycle). In fact, warm reset can work most cases except > > ldo-bypass mode and this is why we connect WDOG_B reset and ldo-bypass here: > > kernel may trigger warm reset at the lowest cpu freq setpoint, for example > > VDDARM 0.975v@396Mhz ldo-bypass mode. i.mx6q will reboot with ldo-enable mode > > @792Mhz and the VDDARM_IN 0.975v can't support system boot.Thus, we need use > > WDOG_B pin to reset external pmic if using ldo-byapss mode. > > Hi Robin, > > I understand that configuring WDOG_B external reset must be used when > LDO bypass is used. This should be done in the kernel but you can also > set this pinmux up in uboot in the board-specific init. > > If your kernel is providing the PMIC driver and cpufreq driver that > alter the cpu core frequencies it must also be configuring pinmux so > that WDOG_B gets routed to the pin that connects to the PMIC so any > reset based on that wdog (SRS or timeout) issues a hard reset to the > PMIC. Failure to do so is a kernel bug. I believe this is done > properly in the Freescale kernel for the reference boards. > pinmux is not enough. WDT bit of WDOGx_WCR need to be taken care of if you want to enable WDOG_B pin reset function. But unfortunately the current wdog driver of kernel not touch this write-once bit. To limit the impact from kernel, set this bit in u-boot. > If you are trying to take care of an issue caused by a watchdog reset > (SRS or timeout) not properly resetting a PMIC (ie perhaps a PCB > errata where signal doesn't go to the PMIC) then you should probably > simply set the PMIC rails where they need to be for the 800MHz > operation in the bootloader and not muck with the CPU frequency. > Honestly, if your PMIC rails are too low for 800MHz on powerup your > bootloader may not be stable anyway right? Note that the operating > setpoints have the same SOC voltage for both 792MHz and 396MHz, its > only the ARM voltage that is lower for 396 vs 792 and chances are your > not going to have an issue just in the bootloader at that point. > As you saw in another mail, bootloader is too late. It'll hang in ROM code. > <snip> > >> > >> What you are doing here is relying on a device-tree binding from the > >> Freescale 'vendor' kernel, which will NEVER make it upstream and this > >> is one of the issues I was running into getting ldo-bypass capability > >> upstream in the kernel. > >> > >> The issue here is that LDO bypass is dependent on the following things: > >> 1. your voltage rail requirements - which are dependent on the CPU > >> frequency (there is a nice table in the IMX6 datasheets of voltage on > >> each rail at each frequency operating point validated by Freescale). > >> The exception of always using the LDO for 1.2GHz is specified here as > >> well. > >> 2. you have a PMIC in your design on VDD_ARM_IN and VDD_SOC_IN rails > >> - this should be specified in the device-tree as well > >> 3. you have valid PMIC drivers configured > >> > >> In the kernel, its not desired to have a single device-tree node > >> called 'fsl,ldo-bypass' for an enable. Instead you need to make sure > >> that you PMIC regulators that are 'not' the internal IMX6 anatop > >> regulators. This property is not a mainline linux device-tree binding. > >> > > Yes, understood. But we have no better solution, because we need touch both > > internal anatop regulator and external pmic regulator in ldo-bypass mode, that > > bring kernel cpufreq driver code too messy.... > > I just don't see the point in having the CPU frequency changed in the > bootloader - leave this up to the OS kernel. I don't think any of this > patch should go into mainline uboot. > > I think my kernel patch I referenced provides you with a simple kernel > solution that de-couples ldo-bypass completely from the bootloader. > Can you please share me the patch link? Thanks. > <snip> > >> > >> While it is correct that you must not use LDO bypass when operating at > >> 1.2GHz, that does not mean that a CPU capable of 1.2GHz can't use LDO > >> bypass at the lower operating points. > >> > > I see, but to simply things, we regard as 1.2GHz chip(fused) may running at > > 1.2GHz and force it work in ldo-enable mode although it has chance to running > > at 1Ghz. In other words, ldo-bypass mode only set once not dynamically. > > Are you saying there is an IMX6 variant that powers up per eFUSE > settings at 1.2GHz? The IMX6QDLRM efuse settings I'm looking at just > have two power-up frequency options: 392MHz and 792MHz. > Sorry, I mean max frequency(SPEED_GRADING[1:0]), not power-up frequency. > In my opinion, your PMIC should be setting VDD_ARM and VDD_SOC at the > necessary voltages for what the CPU is currently running at, in the > bootloader. Its up to your OS to properly control this to its needs > later. Again, if for some reason (hardware errata) you have a > situation where the PMIC maybe didn't get reset and the board powers > up into the bootloader at a frequency higher than the rails are set > to, then simply set its rails to where they need to be for the freq > your running at in the bootloader. > > >> > + > >> > +static int arm_orig_podf; > >> > +void set_arm_freq_400M(bool is_400M) > >> > +{ > >> > + struct mxc_ccm_reg *mxc_ccm = (struct mxc_ccm_reg *)CCM_BASE_ADDR; > >> > + > >> > + if (is_400M) > >> > + writel(0x1, &mxc_ccm->cacrr); > >> > + else > >> > + writel(arm_orig_podf, &mxc_ccm->cacrr); > >> > +} > >> > >> I have no idea what is going on here. Are we now running at different > >> CPU frequencies in U-boot? The IMX6 comes up in either 800MHz or > >> 400MHz per BOOT_CFG3 e-fuse and from the U-Boot I'm working with > >> (still on 2010.04 but pretty sure its the same in 2014.10) the CPU > >> frequency is never changed in the bootloader. > >> > > Let's try to explain why we downgrade cpufreq to 400Mhz before ldo-bypass mode > > switch(i.mx6q): > > cpufreq VDDSOC VDDARM > > 400Mhz 0.975v 1.175v > > 800Mhz 1.175v 1.175v > > If system boot from 800Mhz, considering VDDARM setting with 1.175v after > > ldo-bypass mode switch, and the voltage drop which bring by internal regulator > > 125mv, we have to set VDDARM 1.175v+125mv = 1.3v before ldo-bypass mode switch, > > but the 1.3v beyond our max voltage for VDDARM. So we have to downgrade the cpufreq > > to 400Mhz. > > Too much complexity in my opinion for the power reduction benefits > while in the bootloader. Leave ldo-bypass out of the bootloader and > you won't have to bother with this. The IMX6 can handle a max of 1.3V > on VDD_ARM_IN and VDD_SOC_IN (and 1.5V on the output side of the LDO). > So you can simply set your PMIC in the bootloader to something between > that and 1.175v and be done with it right? > Not for power reduction, just for match with the voltage setting which data sheet saying in any time or any narrow timing window. Have to set VDDRAM_IN 1.3v for 800Mhz(VDDARM 1.175v) before ldo-bypass mode switched. And set VDDARM_IN to 1.175v after ldo-bypass mode switched. But in the narrow window which VDDARM_IN from 1.3v to 1.175v with ldo-bypass mode enabled. The 1.3v plus power ripple may beyond the max volage. > >> This brings me to the question of why does LDO bypass have anything at > >> all do to with the bootloader? > >> > > We only need do once ldo-bypass switch, so we hope it can be implemented in > > u-boot > > It doesn't belong in uboot. It was wrong for Freescale to create a > dependence between the bootloader and the kernel when it is so easy to > avoid. > > I don't think any of this particular patch should go into mainline uboot. > Tim, thanks for your reviewing, understood your concerning. Looks it's better done in kernel. > Regards, > > Tim
On Fri, Feb 13, 2015 at 12:16 AM, Robin Gong <b38343@freescale.com> wrote: > On Wed, Feb 11, 2015 at 07:47:57AM -0800, Tim Harvey wrote: >> On Wed, Feb 11, 2015 at 2:49 AM, Robin Gong <b38343@freescale.com> wrote: >> <snip> >> >> >> >> This is very board dependent. Here you are referring to a board that >> >> has a reset input to the PMIC's from the IMX6's watchdog output. In >> >> this case, this reset routing/pinmux would be needed regardless of >> >> using ldo-bypass mode or not and that should just be a pinmux of the >> >> pin your using for WDOG_B. >> >> >> > There are two types of reboot in our chip by watchdog : SRS/warm reset >> > (Software Reset Signal) and WDOG_B(reset signal output to external pmic >> > to trigger next power cycle). In fact, warm reset can work most cases except >> > ldo-bypass mode and this is why we connect WDOG_B reset and ldo-bypass here: >> > kernel may trigger warm reset at the lowest cpu freq setpoint, for example >> > VDDARM 0.975v@396Mhz ldo-bypass mode. i.mx6q will reboot with ldo-enable mode >> > @792Mhz and the VDDARM_IN 0.975v can't support system boot.Thus, we need use >> > WDOG_B pin to reset external pmic if using ldo-byapss mode. >> >> Hi Robin, >> >> I understand that configuring WDOG_B external reset must be used when >> LDO bypass is used. This should be done in the kernel but you can also >> set this pinmux up in uboot in the board-specific init. >> >> If your kernel is providing the PMIC driver and cpufreq driver that >> alter the cpu core frequencies it must also be configuring pinmux so >> that WDOG_B gets routed to the pin that connects to the PMIC so any >> reset based on that wdog (SRS or timeout) issues a hard reset to the >> PMIC. Failure to do so is a kernel bug. I believe this is done >> properly in the Freescale kernel for the reference boards. >> > pinmux is not enough. WDT bit of WDOGx_WCR need to be taken care of if you want > to enable WDOG_B pin reset function. But unfortunately the current wdog driver > of kernel not touch this write-once bit. To limit the impact from kernel, set > this bit in u-boot. Hi Robin, 'To limit the impact from kernel, set this bit in u-boot' indicates that you are trying to create a u-boot and kernel dependence on each other which is a bad idea. The kernel should take care of everything it needs without making assumptions on what the bootloader did. You are correct in that the current imx2_wdt.c does not 'set' the write-once WCR bit to enable the external reset but it does seem funny to me that Freescale decided to patch system.c's mxc_restart (http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/commit/arch/arm/mach-imx/system.c?h=imx_3.10.53_1.1.0_ga&id=ee07b144e1adff13d262e7f353f5804e3c2d3e06) to try to deal with the machine_restart case but did not patch imx2_wdt.c to deal with the watchdog timeout case. Wouldn't these boards also hang when reset from a watchdog timeout when using ldo-enabled and CPU@397Mhz. It seems to me that you should also be patching imx2_wdt.c to set WCR for that case. >> If you are trying to take care of an issue caused by a watchdog reset >> (SRS or timeout) not properly resetting a PMIC (ie perhaps a PCB >> errata where signal doesn't go to the PMIC) then you should probably >> simply set the PMIC rails where they need to be for the 800MHz >> operation in the bootloader and not muck with the CPU frequency. >> Honestly, if your PMIC rails are too low for 800MHz on powerup your >> bootloader may not be stable anyway right? Note that the operating >> setpoints have the same SOC voltage for both 792MHz and 396MHz, its >> only the ARM voltage that is lower for 396 vs 792 and chances are your >> not going to have an issue just in the bootloader at that point. >> > As you saw in another mail, bootloader is too late. It'll hang in ROM code. >> <snip> >> > Can you please share me the patch link? Thanks. The link was further up in the thread in response to Peng: Instead of touching U-Boot, in my 'Freescale' kernel I look for the fsl,ldo-bypass node in the kernel init and enable it just like their bootloader would have: https://github.com/Gateworks/linux-imx6/commit/a1af6ac6f00b4da7c8a5656e8ff093d4ab5cadee It is still not good to rely on the fsl,ldo-bypass property to be set (which will never make it into mainline) because this does not garuntee that ldo-bypass is setup and functioning correctly at all (ie user could have PMIC driver disabled, or not configured in device-tree properly as source for arm/soc rails) but I haven't figured out the best solution upstream yet and haven't had time to get back to it. Its still a much lesser evil than requiring that ldo-bypass be configured in the bootloader. This patch removes the dependence upon the bootloader (I will never use Freescales hacked up U-Boot - I think its still based on a 2009 branch). I have given up on getting patches into Freescale's kernel - perhaps being an employee you will have better luck :) I'm still hoping that someday mainline linux will have all the IPU/VPU/GPU support for IMX6 that is missing (but getting very close!) which is what is forcing me to use Freescale's kernel for some of our BSP's. Tim
diff --git a/arch/arm/cpu/armv7/mx6/soc.c b/arch/arm/cpu/armv7/mx6/soc.c index 5f5f497..5d02755 100644 --- a/arch/arm/cpu/armv7/mx6/soc.c +++ b/arch/arm/cpu/armv7/mx6/soc.c @@ -18,6 +18,7 @@ #include <asm/arch/sys_proto.h> #include <asm/imx-common/boot_mode.h> #include <asm/imx-common/dma.h> +#include <libfdt.h> #include <stdbool.h> #include <asm/arch/mxc_hdmi.h> #include <asm/arch/crm_regs.h> @@ -429,6 +430,146 @@ void s_init(void) writel(mask528, &anatop->pfd_528_clr); } +#ifdef CONFIG_LDO_BYPASS_CHECK +DECLARE_GLOBAL_DATA_PTR; +static int ldo_bypass; + +int check_ldo_bypass(void) +{ + const int *ldo_mode; + int node; + + /* get the right fdt_blob from the global working_fdt */ + gd->fdt_blob = working_fdt; + /* Get the node from FDT for anatop ldo-bypass */ + node = fdt_node_offset_by_compatible(gd->fdt_blob, -1, + "fsl,imx6q-gpc"); + if (node < 0) { + printf("No gpc device node %d, force to ldo-enable.\n", node); + return 0; + } + ldo_mode = fdt_getprop(gd->fdt_blob, node, "fsl,ldo-bypass", NULL); + /* + * return 1 if "fsl,ldo-bypass = <1>", else return 0 if + * "fsl,ldo-bypass = <0>" or no "fsl,ldo-bypass" property + */ + ldo_bypass = fdt32_to_cpu(*ldo_mode) == 1 ? 1 : 0; + + return ldo_bypass; +} + +int check_1_2G(void) +{ + u32 reg; + int result = 0; + struct ocotp_regs *ocotp = (struct ocotp_regs *)OCOTP_BASE_ADDR; + struct fuse_bank *bank = &ocotp->bank[0]; + struct fuse_bank0_regs *fuse_bank0 = + (struct fuse_bank0_regs *)bank->fuse_regs; + + reg = readl(&fuse_bank0->cfg3); + if (((reg >> 16) & 0x3) == 0x3) { + if (ldo_bypass) { + printf("Wrong dtb file used! i.MX6Q@1.2Ghz only works with ldo-enable mode!\n"); + /* + * Currently, only imx6q-sabresd board might be here, + * since only i.MX6Q support 1.2G and only Sabresd board + * support ldo-bypass mode. So hardcode here. + * You can also modify your board(i.MX6Q) dtb name if it + * supports both ldo-bypass and ldo-enable mode. + */ + printf("Please use imx6q-sabresd-ldo.dtb!\n"); + hang(); + } + result = 1; + } + + return result; +} + +static int arm_orig_podf; +void set_arm_freq_400M(bool is_400M) +{ + struct mxc_ccm_reg *mxc_ccm = (struct mxc_ccm_reg *)CCM_BASE_ADDR; + + if (is_400M) + writel(0x1, &mxc_ccm->cacrr); + else + writel(arm_orig_podf, &mxc_ccm->cacrr); +} + +void prep_anatop_bypass(void) +{ + struct mxc_ccm_reg *mxc_ccm = (struct mxc_ccm_reg *)CCM_BASE_ADDR; + + arm_orig_podf = readl(&mxc_ccm->cacrr); + /* + * Downgrade ARM speed to 400Mhz as half of boot 800Mhz before ldo + * bypassed, also downgrade internal vddarm ldo to 0.975V. + * VDDARM_IN 0.975V + 125mV = 1.1V < Max(1.3V) + * otherwise at 800Mhz(i.mx6dl): + * VDDARM_IN 1.175V + 125mV = 1.3V = Max(1.3V) + * We need provide enough gap in this case. + * skip if boot from 400M. + */ + if (!arm_orig_podf) + set_arm_freq_400M(true); +#if !defined(CONFIG_MX6DL) && !defined(CONFIG_MX6SX) + set_ldo_voltage(LDO_ARM, 975); +#else + set_ldo_voltage(LDO_ARM, 1150); +#endif +} + +void set_wdog_reset(struct wdog_regs *wdog) +{ + u32 reg = readw(&wdog->wcr); + /* + * use WDOG_B mode to reset external pmic because it's risky for the + * following watchdog reboot in case of cpu freq at lowest 400Mhz with + * ldo-bypass mode. Because boot frequency maybe higher 800Mhz i.e. So + * in ldo-bypass mode watchdog reset will only triger POR reset, not + * WDOG reset. But below code depends on hardware design, if HW didn't + * connect WDOG_B pin to external pmic such as i.mx6slevk, we can skip + * these code since it assumed boot from 400Mhz always. + */ + reg = readw(&wdog->wcr); + reg |= 1 << 3; + /* + * WDZST bit is write-once only bit. Align this bit in kernel, + * otherwise kernel code will have no chance to set this bit. + */ + reg |= 1 << 0; + writew(reg, &wdog->wcr); +} + +int set_anatop_bypass(int wdog_reset_pin) +{ + struct mxc_ccm_reg *ccm_regs = (struct mxc_ccm_reg *)CCM_BASE_ADDR; + struct wdog_regs *wdog; + u32 reg = readl(&ccm_regs->reg_core); + + /* bypass VDDARM/VDDSOC */ + reg = reg | (0x1F << 18) | 0x1F; + writel(reg, &ccm_regs->reg_core); + + if (wdog_reset_pin == 2) + wdog = (struct wdog_regs *)WDOG2_BASE_ADDR; + else if (wdog_reset_pin == 1) + wdog = (struct wdog_regs *)WDOG1_BASE_ADDR; + else + return arm_orig_podf; + set_wdog_reset(wdog); + return arm_orig_podf; +} + +void finish_anatop_bypass(void) +{ + if (!arm_orig_podf) + set_arm_freq_400M(false); +} +#endif + #ifdef CONFIG_IMX_HDMI void imx_enable_hdmi_phy(void) { diff --git a/arch/arm/include/asm/arch-mx6/sys_proto.h b/arch/arm/include/asm/arch-mx6/sys_proto.h index 28ba844..e6f2112 100644 --- a/arch/arm/include/asm/arch-mx6/sys_proto.h +++ b/arch/arm/include/asm/arch-mx6/sys_proto.h @@ -28,6 +28,15 @@ const char *get_imx_type(u32 imxtype); unsigned imx_ddr_size(void); void set_chipselect_size(int const); +void set_wdog_reset(struct wdog_regs *wdog); +#ifdef CONFIG_LDO_BYPASS_CHECK +int check_ldo_bypass(void); +int check_1_2G(void); +void ldo_mode_set(int ldo_bypass); +int set_anatop_bypass(int wdog_reset_pin); +void prep_anatop_bypass(void); +void finish_anatop_bypass(void); +#endif /* * Initializes on-chip ethernet controllers. * to override, implement board_eth_init()