| Message ID | 1432308599-28643-6-git-send-email-shawn.guo@linaro.org |
|---|---|
| State | New |
| Headers | show |
Hi Shawn, On Fri, May 22, 2015 at 8:29 AM, Shawn Guo <shawn.guo@linaro.org> wrote: > It creates a gpt device speicific data structure and adds function hook > gpt_setup_tctl in there to set up gpt TCTL register. > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org> Starting with next-20150529, imx6dl-wandboard (both dual and solo) started having boot failures[1]. Today I bisected it down to this commit. It didn't revert cleanly, so I didn't verify that this patch alone is the root cause, but reproducing the boot failure should be pretty easy on wandboard with recent linux-next. Kevin [1] http://kernelci.org/boot/all/job/next/kernel/next-20150529/
Hi Kevin, On Mon, Jun 01, 2015 at 04:53:28PM -0700, Kevin Hilman wrote: > Hi Shawn, > > On Fri, May 22, 2015 at 8:29 AM, Shawn Guo <shawn.guo@linaro.org> wrote: > > It creates a gpt device speicific data structure and adds function hook > > gpt_setup_tctl in there to set up gpt TCTL register. > > > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org> > > Starting with next-20150529, imx6dl-wandboard (both dual and solo) > started having boot failures[1]. Today I bisected it down to this > commit. It didn't revert cleanly, so I didn't verify that this patch > alone is the root cause, but reproducing the boot failure should be > pretty easy on wandboard with recent linux-next. The imx clocksource driver was marked as BROKEN on linux-next, because it causes a build error on powerpc allyesconfig build [1]. I'm working on a fix for it. Can you please try to remove the BROKEN mark in your test to see if it fixes your problem? Sorry for the breakage. Shawn [1] http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg901156.html
On Mon, Jun 1, 2015 at 6:11 PM, Shawn Guo <shawn.guo@linaro.org> wrote: > Hi Kevin, > > On Mon, Jun 01, 2015 at 04:53:28PM -0700, Kevin Hilman wrote: >> Hi Shawn, >> >> On Fri, May 22, 2015 at 8:29 AM, Shawn Guo <shawn.guo@linaro.org> wrote: >> > It creates a gpt device speicific data structure and adds function hook >> > gpt_setup_tctl in there to set up gpt TCTL register. >> > >> > Signed-off-by: Shawn Guo <shawn.guo@linaro.org> >> >> Starting with next-20150529, imx6dl-wandboard (both dual and solo) >> started having boot failures[1]. Today I bisected it down to this >> commit. It didn't revert cleanly, so I didn't verify that this patch >> alone is the root cause, but reproducing the boot failure should be >> pretty easy on wandboard with recent linux-next. > > The imx clocksource driver was marked as BROKEN on linux-next, because > it causes a build error on powerpc allyesconfig build [1]. I'm working > on a fix for it. > > Can you please try to remove the BROKEN mark in your test to see if it > fixes your problem? I'm just using multi_v7_defconfig, and BROKEN is not set, but that driver is still built because it's explicitly selected by ARCH_MXC: $ git describe next-20150601 $ rm build/.config $ make ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- O=build multi_v7_defconfig make[1]: Entering directory `/work/kernel/next/build' GEN ./Makefile arch/arm/configs/multi_v7_defconfig:603:warning: override: reassigning to symbol COMMON_CLK_QCOM warning: (ARCH_MXC) selects CLKSRC_IMX_GPT which has unmet direct dependencies (OF && BROKEN) warning: (ARCH_MXC) selects CLKSRC_IMX_GPT which has unmet direct dependencies (OF && BROKEN) # # configuration written to .config # make[1]: Leaving directory `/work/kernel/next/build' $ grep CLKSRC_IMX_GPT build/.config CONFIG_CLKSRC_IMX_GPT=y $ Kevin
Shawn Guo <shawnguo@kernel.org> writes: > On Tue, Jun 02, 2015 at 08:56:02AM -0700, Kevin Hilman wrote: >> I'm just using multi_v7_defconfig, and BROKEN is not set, but that >> driver is still built because it's explicitly selected by ARCH_MXC: >> >> $ git describe >> next-20150601 >> $ rm build/.config >> $ make ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- O=build multi_v7_defconfig >> make[1]: Entering directory `/work/kernel/next/build' >> GEN ./Makefile >> arch/arm/configs/multi_v7_defconfig:603:warning: override: reassigning >> to symbol COMMON_CLK_QCOM >> warning: (ARCH_MXC) selects CLKSRC_IMX_GPT which has unmet direct >> dependencies (OF && BROKEN) >> warning: (ARCH_MXC) selects CLKSRC_IMX_GPT which has unmet direct >> dependencies (OF && BROKEN) >> # >> # configuration written to .config >> # >> make[1]: Leaving directory `/work/kernel/next/build' >> $ grep CLKSRC_IMX_GPT build/.config >> CONFIG_CLKSRC_IMX_GPT=y >> $ > > Yes, you're right. It's broken on i.MX6DL/S indeed. The cause is that > i.MX6DL/S DTBs are using the same GPT compatible string as i.MX6Q. It > effectively makes kernel select an incorrect GPT device type on > i.MX6DL/S. We will need to update i.MX6DL/S device tree to use > a correct compatible string. But to keep the existing DTBs continue > working, we need the following change. As I'm asked by Arnd to rebase > my branch, I would like fold the changes into the original patch to > save the git bisect issue. I tried your patch on top of next-20150529 (where I originally found the issue) and it gets my wandboard dual and solo booting again. Tested-by: Kevin Hilman <khilman@linaro.org> Kevin
On Wed, Jun 03, 2015 at 10:41:29AM -0700, Kevin Hilman wrote: > I tried your patch on top of next-20150529 (where I originally found the > issue) and it gets my wandboard dual and solo booting again. > > Tested-by: Kevin Hilman <khilman@linaro.org> Thanks for confirming, Kevin. Shawn
diff --git a/arch/arm/mach-imx/time.c b/arch/arm/mach-imx/time.c index cf88355654dd..2fbc3022ce3f 100644 --- a/arch/arm/mach-imx/time.c +++ b/arch/arm/mach-imx/time.c @@ -92,6 +92,11 @@ struct imx_timer { int irq; struct clk *clk_per; struct clk *clk_ipg; + const struct imx_gpt_data *gpt; +}; + +struct imx_gpt_data { + void (*gpt_setup_tctl)(struct imx_timer *imxtm); }; static void __iomem *timer_base; @@ -307,13 +312,83 @@ static int __init mxc_clockevent_init(struct imx_timer *imxtm) return 0; } -static void __init _mxc_timer_init(struct imx_timer *imxtm) +static void imx1_gpt_setup_tctl(struct imx_timer *imxtm) +{ + u32 tctl_val; + + tctl_val = MX1_2_TCTL_FRR | MX1_2_TCTL_CLK_PCLK1 | MXC_TCTL_TEN; + writel_relaxed(tctl_val, imxtm->base + MXC_TCTL); +} +#define imx21_gpt_setup_tctl imx1_gpt_setup_tctl + +static void imx31_gpt_setup_tctl(struct imx_timer *imxtm) +{ + u32 tctl_val; + + tctl_val = V2_TCTL_FRR | V2_TCTL_WAITEN | MXC_TCTL_TEN; + if (clk_get_rate(imxtm->clk_per) == V2_TIMER_RATE_OSC_DIV8) + tctl_val |= V2_TCTL_CLK_OSC_DIV8; + else + tctl_val |= V2_TCTL_CLK_PER; + + writel_relaxed(tctl_val, imxtm->base + MXC_TCTL); +} + +static void imx6dl_gpt_setup_tctl(struct imx_timer *imxtm) { - uint32_t tctl_val; + u32 tctl_val; + + tctl_val = V2_TCTL_FRR | V2_TCTL_WAITEN | MXC_TCTL_TEN; + if (clk_get_rate(imxtm->clk_per) == V2_TIMER_RATE_OSC_DIV8) { + tctl_val |= V2_TCTL_CLK_OSC_DIV8; + /* 24 / 8 = 3 MHz */ + writel_relaxed(7 << V2_TPRER_PRE24M, imxtm->base + MXC_TPRER); + tctl_val |= V2_TCTL_24MEN; + } else { + tctl_val |= V2_TCTL_CLK_PER; + } + + writel_relaxed(tctl_val, imxtm->base + MXC_TCTL); +} +static const struct imx_gpt_data imx1_gpt_data = { + .gpt_setup_tctl = imx1_gpt_setup_tctl, +}; + +static const struct imx_gpt_data imx21_gpt_data = { + .gpt_setup_tctl = imx21_gpt_setup_tctl, +}; + +static const struct imx_gpt_data imx31_gpt_data = { + .gpt_setup_tctl = imx31_gpt_setup_tctl, +}; + +static const struct imx_gpt_data imx6dl_gpt_data = { + .gpt_setup_tctl = imx6dl_gpt_setup_tctl, +}; + +static void __init _mxc_timer_init(struct imx_timer *imxtm) +{ /* Temporary */ timer_base = imxtm->base; + switch (imxtm->type) { + case GPT_TYPE_IMX1: + imxtm->gpt = &imx1_gpt_data; + break; + case GPT_TYPE_IMX21: + imxtm->gpt = &imx21_gpt_data; + break; + case GPT_TYPE_IMX31: + imxtm->gpt = &imx31_gpt_data; + break; + case GPT_TYPE_IMX6DL: + imxtm->gpt = &imx6dl_gpt_data; + break; + default: + BUG(); + } + if (IS_ERR(imxtm->clk_per)) { pr_err("i.MX timer: unable to get clk\n"); return; @@ -331,24 +406,7 @@ static void __init _mxc_timer_init(struct imx_timer *imxtm) writel_relaxed(0, imxtm->base + MXC_TCTL); writel_relaxed(0, imxtm->base + MXC_TPRER); /* see datasheet note */ - if (timer_is_v2()) { - tctl_val = V2_TCTL_FRR | V2_TCTL_WAITEN | MXC_TCTL_TEN; - if (clk_get_rate(imxtm->clk_per) == V2_TIMER_RATE_OSC_DIV8) { - tctl_val |= V2_TCTL_CLK_OSC_DIV8; - if (cpu_is_imx6dl() || cpu_is_imx6sx()) { - /* 24 / 8 = 3 MHz */ - writel_relaxed(7 << V2_TPRER_PRE24M, - imxtm->base + MXC_TPRER); - tctl_val |= V2_TCTL_24MEN; - } - } else { - tctl_val |= V2_TCTL_CLK_PER; - } - } else { - tctl_val = MX1_2_TCTL_FRR | MX1_2_TCTL_CLK_PCLK1 | MXC_TCTL_TEN; - } - - writel_relaxed(tctl_val, imxtm->base + MXC_TCTL); + imxtm->gpt->gpt_setup_tctl(imxtm); /* init and register the timer to the framework */ mxc_clocksource_init(imxtm);
It creates a gpt device speicific data structure and adds function hook gpt_setup_tctl in there to set up gpt TCTL register. Signed-off-by: Shawn Guo <shawn.guo@linaro.org> --- arch/arm/mach-imx/time.c | 98 ++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 78 insertions(+), 20 deletions(-)