* [PATCH] ARM: dts: am437x-gp-evm: Do not reset gpio5 @ 2014-03-21 5:20 ` Lokesh Vutla 0 siblings, 0 replies; 18+ messages in thread From: Lokesh Vutla @ 2014-03-21 5:20 UTC (permalink / raw) To: linux-omap Cc: linux-arm-kernel, bcousson, tony, devicetree, nsekhar, rnayak, nm, mark.rutland, lokeshvutla, Dave Gerlach From: Dave Gerlach <d-gerlach@ti.com> Do not reset GPIO5 at boot-up because GPIO5_7 is used on AM437x GP-EVM to control VTT regulators on DDR3. Without this some GP-EVM boards will fail to boot because of DDR3 corruption. Reported-by: Nishanth Menon <nm@ti.com> Tested-by: Nishanth Menon <nm@ti.com> Signed-off-by: Dave Gerlach <d-gerlach@ti.com> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com> --- This is applied on top of current linux-next. git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master arch/arm/boot/dts/am437x-gp-evm.dts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/arch/arm/boot/dts/am437x-gp-evm.dts b/arch/arm/boot/dts/am437x-gp-evm.dts index df8798e..a055f7f 100644 --- a/arch/arm/boot/dts/am437x-gp-evm.dts +++ b/arch/arm/boot/dts/am437x-gp-evm.dts @@ -117,6 +117,11 @@ status = "okay"; }; +&gpio5 { + status = "okay"; + ti,no-reset-on-init; +}; + &mmc1 { status = "okay"; vmmc-supply = <&vmmcsd_fixed>; -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH] ARM: dts: am437x-gp-evm: Do not reset gpio5 @ 2014-03-21 5:20 ` Lokesh Vutla 0 siblings, 0 replies; 18+ messages in thread From: Lokesh Vutla @ 2014-03-21 5:20 UTC (permalink / raw) To: linux-arm-kernel From: Dave Gerlach <d-gerlach@ti.com> Do not reset GPIO5 at boot-up because GPIO5_7 is used on AM437x GP-EVM to control VTT regulators on DDR3. Without this some GP-EVM boards will fail to boot because of DDR3 corruption. Reported-by: Nishanth Menon <nm@ti.com> Tested-by: Nishanth Menon <nm@ti.com> Signed-off-by: Dave Gerlach <d-gerlach@ti.com> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com> --- This is applied on top of current linux-next. git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master arch/arm/boot/dts/am437x-gp-evm.dts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/arch/arm/boot/dts/am437x-gp-evm.dts b/arch/arm/boot/dts/am437x-gp-evm.dts index df8798e..a055f7f 100644 --- a/arch/arm/boot/dts/am437x-gp-evm.dts +++ b/arch/arm/boot/dts/am437x-gp-evm.dts @@ -117,6 +117,11 @@ status = "okay"; }; +&gpio5 { + status = "okay"; + ti,no-reset-on-init; +}; + &mmc1 { status = "okay"; vmmc-supply = <&vmmcsd_fixed>; -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 18+ messages in thread
[parent not found: <1395379213-15451-1-git-send-email-lokeshvutla-l0cyMroinI0@public.gmane.org>]
* Re: [PATCH] ARM: dts: am437x-gp-evm: Do not reset gpio5 2014-03-21 5:20 ` Lokesh Vutla @ 2014-03-21 5:52 ` Felipe Balbi -1 siblings, 0 replies; 18+ messages in thread From: Felipe Balbi @ 2014-03-21 5:52 UTC (permalink / raw) To: Lokesh Vutla Cc: linux-omap-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, bcousson-rdvid1DuHRBWk0Htik3J/w, tony-4v6yS6AI5VpBDgjK7y7TUQ, devicetree-u79uwXL29TY76Z2rM5mHXA, nsekhar-l0cyMroinI0, rnayak-l0cyMroinI0, nm-l0cyMroinI0, mark.rutland-5wv7dgnIgG8, Dave Gerlach [-- Attachment #1: Type: text/plain, Size: 4448 bytes --] On Fri, Mar 21, 2014 at 10:50:13AM +0530, Lokesh Vutla wrote: > From: Dave Gerlach <d-gerlach-l0cyMroinI0@public.gmane.org> > > Do not reset GPIO5 at boot-up because GPIO5_7 is used > on AM437x GP-EVM to control VTT regulators on DDR3. > Without this some GP-EVM boards will fail to boot because > of DDR3 corruption. > > Reported-by: Nishanth Menon <nm-l0cyMroinI0@public.gmane.org> > Tested-by: Nishanth Menon <nm-l0cyMroinI0@public.gmane.org> > Signed-off-by: Dave Gerlach <d-gerlach-l0cyMroinI0@public.gmane.org> > Signed-off-by: Lokesh Vutla <lokeshvutla-l0cyMroinI0@public.gmane.org> every now and again we see a patch like this because yet another board is using a GPIO to toggle DDR regulators. Instead of constantly patching things like this, how about we try something like below (build-tested only): diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c index 1f33f5d..f5962ff 100644 --- a/arch/arm/mach-omap2/omap_hwmod.c +++ b/arch/arm/mach-omap2/omap_hwmod.c @@ -2610,6 +2610,10 @@ static int __init _setup_reset(struct omap_hwmod *oh) if (oh->flags & HWMOD_EXT_OPT_MAIN_CLK) return -EPERM; + /* NEVER reset GPIO blocks */ + if (strncmp(oh->name, "gpio", 4) == 0) + return 0; + if (oh->rst_lines_cnt == 0) { r = _enable(oh); if (r) { diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c index 4243190..ce8b53a 100644 --- a/drivers/gpio/gpio-omap.c +++ b/drivers/gpio/gpio-omap.c @@ -1130,6 +1130,29 @@ static void omap_gpio_chip_init(struct gpio_bank *bank) static const struct of_device_id omap_gpio_match[]; +static void omap_gpio_init_context(struct gpio_bank *p) +{ + struct omap_gpio_reg_offs *regs = p->regs; + void __iomem *base = p->base; + + p->context.ctrl = readl_relaxed(base + regs->ctrl); + p->context.oe = readl_relaxed(base + regs->direction); + p->context.wake_en = readl_relaxed(base + regs->wkup_en); + p->context.leveldetect0 = readl_relaxed(base + regs->leveldetect0); + p->context.leveldetect1 = readl_relaxed(base + regs->leveldetect1); + p->context.risingdetect = readl_relaxed(base + regs->risingdetect); + p->context.fallingdetect = readl_relaxed(base + regs->fallingdetect); + p->context.irqenable1 = readl_relaxed(base + regs->irqenable); + p->context.irqenable2 = readl_relaxed(base + regs->irqenable2); + + if (regs->set_dataout && p->regs->clr_dataout) + p->context.dataout = readl_relaxed(base + regs->set_dataout); + else + p->context.dataout = readl_relaxed(base + regs->dataout); + + p->context_valid = true; +} + static int omap_gpio_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; @@ -1246,6 +1269,7 @@ static int omap_gpio_probe(struct platform_device *pdev) omap_gpio_mod_init(bank); omap_gpio_chip_init(bank); omap_gpio_show_rev(bank); + omap_gpio_init_context(bank); pm_runtime_put(bank->dev); @@ -1325,8 +1349,6 @@ update_gpio_context_count: return 0; } -static void omap_gpio_init_context(struct gpio_bank *p); - static int omap_gpio_runtime_resume(struct device *dev) { struct platform_device *pdev = to_platform_device(dev); @@ -1466,29 +1488,6 @@ void omap2_gpio_resume_after_idle(void) } #if defined(CONFIG_PM_RUNTIME) -static void omap_gpio_init_context(struct gpio_bank *p) -{ - struct omap_gpio_reg_offs *regs = p->regs; - void __iomem *base = p->base; - - p->context.ctrl = readl_relaxed(base + regs->ctrl); - p->context.oe = readl_relaxed(base + regs->direction); - p->context.wake_en = readl_relaxed(base + regs->wkup_en); - p->context.leveldetect0 = readl_relaxed(base + regs->leveldetect0); - p->context.leveldetect1 = readl_relaxed(base + regs->leveldetect1); - p->context.risingdetect = readl_relaxed(base + regs->risingdetect); - p->context.fallingdetect = readl_relaxed(base + regs->fallingdetect); - p->context.irqenable1 = readl_relaxed(base + regs->irqenable); - p->context.irqenable2 = readl_relaxed(base + regs->irqenable2); - - if (regs->set_dataout && p->regs->clr_dataout) - p->context.dataout = readl_relaxed(base + regs->set_dataout); - else - p->context.dataout = readl_relaxed(base + regs->dataout); - - p->context_valid = true; -} - static void omap_gpio_restore_context(struct gpio_bank *bank) { writel_relaxed(bank->context.wake_en, Then, we can even remove ti,no-reset flag from all GPIO DT nodes. -- balbi [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH] ARM: dts: am437x-gp-evm: Do not reset gpio5 @ 2014-03-21 5:52 ` Felipe Balbi 0 siblings, 0 replies; 18+ messages in thread From: Felipe Balbi @ 2014-03-21 5:52 UTC (permalink / raw) To: linux-arm-kernel On Fri, Mar 21, 2014 at 10:50:13AM +0530, Lokesh Vutla wrote: > From: Dave Gerlach <d-gerlach@ti.com> > > Do not reset GPIO5 at boot-up because GPIO5_7 is used > on AM437x GP-EVM to control VTT regulators on DDR3. > Without this some GP-EVM boards will fail to boot because > of DDR3 corruption. > > Reported-by: Nishanth Menon <nm@ti.com> > Tested-by: Nishanth Menon <nm@ti.com> > Signed-off-by: Dave Gerlach <d-gerlach@ti.com> > Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com> every now and again we see a patch like this because yet another board is using a GPIO to toggle DDR regulators. Instead of constantly patching things like this, how about we try something like below (build-tested only): diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c index 1f33f5d..f5962ff 100644 --- a/arch/arm/mach-omap2/omap_hwmod.c +++ b/arch/arm/mach-omap2/omap_hwmod.c @@ -2610,6 +2610,10 @@ static int __init _setup_reset(struct omap_hwmod *oh) if (oh->flags & HWMOD_EXT_OPT_MAIN_CLK) return -EPERM; + /* NEVER reset GPIO blocks */ + if (strncmp(oh->name, "gpio", 4) == 0) + return 0; + if (oh->rst_lines_cnt == 0) { r = _enable(oh); if (r) { diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c index 4243190..ce8b53a 100644 --- a/drivers/gpio/gpio-omap.c +++ b/drivers/gpio/gpio-omap.c @@ -1130,6 +1130,29 @@ static void omap_gpio_chip_init(struct gpio_bank *bank) static const struct of_device_id omap_gpio_match[]; +static void omap_gpio_init_context(struct gpio_bank *p) +{ + struct omap_gpio_reg_offs *regs = p->regs; + void __iomem *base = p->base; + + p->context.ctrl = readl_relaxed(base + regs->ctrl); + p->context.oe = readl_relaxed(base + regs->direction); + p->context.wake_en = readl_relaxed(base + regs->wkup_en); + p->context.leveldetect0 = readl_relaxed(base + regs->leveldetect0); + p->context.leveldetect1 = readl_relaxed(base + regs->leveldetect1); + p->context.risingdetect = readl_relaxed(base + regs->risingdetect); + p->context.fallingdetect = readl_relaxed(base + regs->fallingdetect); + p->context.irqenable1 = readl_relaxed(base + regs->irqenable); + p->context.irqenable2 = readl_relaxed(base + regs->irqenable2); + + if (regs->set_dataout && p->regs->clr_dataout) + p->context.dataout = readl_relaxed(base + regs->set_dataout); + else + p->context.dataout = readl_relaxed(base + regs->dataout); + + p->context_valid = true; +} + static int omap_gpio_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; @@ -1246,6 +1269,7 @@ static int omap_gpio_probe(struct platform_device *pdev) omap_gpio_mod_init(bank); omap_gpio_chip_init(bank); omap_gpio_show_rev(bank); + omap_gpio_init_context(bank); pm_runtime_put(bank->dev); @@ -1325,8 +1349,6 @@ update_gpio_context_count: return 0; } -static void omap_gpio_init_context(struct gpio_bank *p); - static int omap_gpio_runtime_resume(struct device *dev) { struct platform_device *pdev = to_platform_device(dev); @@ -1466,29 +1488,6 @@ void omap2_gpio_resume_after_idle(void) } #if defined(CONFIG_PM_RUNTIME) -static void omap_gpio_init_context(struct gpio_bank *p) -{ - struct omap_gpio_reg_offs *regs = p->regs; - void __iomem *base = p->base; - - p->context.ctrl = readl_relaxed(base + regs->ctrl); - p->context.oe = readl_relaxed(base + regs->direction); - p->context.wake_en = readl_relaxed(base + regs->wkup_en); - p->context.leveldetect0 = readl_relaxed(base + regs->leveldetect0); - p->context.leveldetect1 = readl_relaxed(base + regs->leveldetect1); - p->context.risingdetect = readl_relaxed(base + regs->risingdetect); - p->context.fallingdetect = readl_relaxed(base + regs->fallingdetect); - p->context.irqenable1 = readl_relaxed(base + regs->irqenable); - p->context.irqenable2 = readl_relaxed(base + regs->irqenable2); - - if (regs->set_dataout && p->regs->clr_dataout) - p->context.dataout = readl_relaxed(base + regs->set_dataout); - else - p->context.dataout = readl_relaxed(base + regs->dataout); - - p->context_valid = true; -} - static void omap_gpio_restore_context(struct gpio_bank *bank) { writel_relaxed(bank->context.wake_en, Then, we can even remove ti,no-reset flag from all GPIO DT nodes. -- balbi -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140321/6740694b/attachment.sig> ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH] ARM: dts: am437x-gp-evm: Do not reset gpio5 2014-03-21 5:52 ` Felipe Balbi @ 2014-03-24 15:29 ` Dave Gerlach -1 siblings, 0 replies; 18+ messages in thread From: Dave Gerlach @ 2014-03-24 15:29 UTC (permalink / raw) To: balbi Cc: Lokesh Vutla, linux-omap, linux-arm-kernel, bcousson, tony, devicetree, nsekhar, rnayak, nm, mark.rutland On 03/21/2014 12:52 AM, Felipe Balbi wrote: > On Fri, Mar 21, 2014 at 10:50:13AM +0530, Lokesh Vutla wrote: >> From: Dave Gerlach <d-gerlach@ti.com> >> >> Do not reset GPIO5 at boot-up because GPIO5_7 is used >> on AM437x GP-EVM to control VTT regulators on DDR3. >> Without this some GP-EVM boards will fail to boot because >> of DDR3 corruption. >> >> Reported-by: Nishanth Menon <nm@ti.com> >> Tested-by: Nishanth Menon <nm@ti.com> >> Signed-off-by: Dave Gerlach <d-gerlach@ti.com> >> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com> > > every now and again we see a patch like this because yet another board > is using a GPIO to toggle DDR regulators. > > Instead of constantly patching things like this, how about we try > something like below (build-tested only): Why should we change all of them? Is it correct to leave every single GPIO at the mercy of the bootloader in every situation? The reason we see these patches only every now and again is because it's a special case that should be handled only for that situation. I also don't think it makes sense to make gpio's a unique case that never gets reset while every other IP does by default. Regards, Dave > > diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c > index 1f33f5d..f5962ff 100644 > --- a/arch/arm/mach-omap2/omap_hwmod.c > +++ b/arch/arm/mach-omap2/omap_hwmod.c > @@ -2610,6 +2610,10 @@ static int __init _setup_reset(struct omap_hwmod *oh) > if (oh->flags & HWMOD_EXT_OPT_MAIN_CLK) > return -EPERM; > > + /* NEVER reset GPIO blocks */ > + if (strncmp(oh->name, "gpio", 4) == 0) > + return 0; > + > if (oh->rst_lines_cnt == 0) { > r = _enable(oh); > if (r) { > diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c > index 4243190..ce8b53a 100644 > --- a/drivers/gpio/gpio-omap.c > +++ b/drivers/gpio/gpio-omap.c > @@ -1130,6 +1130,29 @@ static void omap_gpio_chip_init(struct gpio_bank *bank) > > static const struct of_device_id omap_gpio_match[]; > > +static void omap_gpio_init_context(struct gpio_bank *p) > +{ > + struct omap_gpio_reg_offs *regs = p->regs; > + void __iomem *base = p->base; > + > + p->context.ctrl = readl_relaxed(base + regs->ctrl); > + p->context.oe = readl_relaxed(base + regs->direction); > + p->context.wake_en = readl_relaxed(base + regs->wkup_en); > + p->context.leveldetect0 = readl_relaxed(base + regs->leveldetect0); > + p->context.leveldetect1 = readl_relaxed(base + regs->leveldetect1); > + p->context.risingdetect = readl_relaxed(base + regs->risingdetect); > + p->context.fallingdetect = readl_relaxed(base + regs->fallingdetect); > + p->context.irqenable1 = readl_relaxed(base + regs->irqenable); > + p->context.irqenable2 = readl_relaxed(base + regs->irqenable2); > + > + if (regs->set_dataout && p->regs->clr_dataout) > + p->context.dataout = readl_relaxed(base + regs->set_dataout); > + else > + p->context.dataout = readl_relaxed(base + regs->dataout); > + > + p->context_valid = true; > +} > + > static int omap_gpio_probe(struct platform_device *pdev) > { > struct device *dev = &pdev->dev; > @@ -1246,6 +1269,7 @@ static int omap_gpio_probe(struct platform_device *pdev) > omap_gpio_mod_init(bank); > omap_gpio_chip_init(bank); > omap_gpio_show_rev(bank); > + omap_gpio_init_context(bank); > > pm_runtime_put(bank->dev); > > @@ -1325,8 +1349,6 @@ update_gpio_context_count: > return 0; > } > > -static void omap_gpio_init_context(struct gpio_bank *p); > - > static int omap_gpio_runtime_resume(struct device *dev) > { > struct platform_device *pdev = to_platform_device(dev); > @@ -1466,29 +1488,6 @@ void omap2_gpio_resume_after_idle(void) > } > > #if defined(CONFIG_PM_RUNTIME) > -static void omap_gpio_init_context(struct gpio_bank *p) > -{ > - struct omap_gpio_reg_offs *regs = p->regs; > - void __iomem *base = p->base; > - > - p->context.ctrl = readl_relaxed(base + regs->ctrl); > - p->context.oe = readl_relaxed(base + regs->direction); > - p->context.wake_en = readl_relaxed(base + regs->wkup_en); > - p->context.leveldetect0 = readl_relaxed(base + regs->leveldetect0); > - p->context.leveldetect1 = readl_relaxed(base + regs->leveldetect1); > - p->context.risingdetect = readl_relaxed(base + regs->risingdetect); > - p->context.fallingdetect = readl_relaxed(base + regs->fallingdetect); > - p->context.irqenable1 = readl_relaxed(base + regs->irqenable); > - p->context.irqenable2 = readl_relaxed(base + regs->irqenable2); > - > - if (regs->set_dataout && p->regs->clr_dataout) > - p->context.dataout = readl_relaxed(base + regs->set_dataout); > - else > - p->context.dataout = readl_relaxed(base + regs->dataout); > - > - p->context_valid = true; > -} > - > static void omap_gpio_restore_context(struct gpio_bank *bank) > { > writel_relaxed(bank->context.wake_en, > > Then, we can even remove ti,no-reset flag from all GPIO DT nodes. > ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] ARM: dts: am437x-gp-evm: Do not reset gpio5 @ 2014-03-24 15:29 ` Dave Gerlach 0 siblings, 0 replies; 18+ messages in thread From: Dave Gerlach @ 2014-03-24 15:29 UTC (permalink / raw) To: linux-arm-kernel On 03/21/2014 12:52 AM, Felipe Balbi wrote: > On Fri, Mar 21, 2014 at 10:50:13AM +0530, Lokesh Vutla wrote: >> From: Dave Gerlach <d-gerlach@ti.com> >> >> Do not reset GPIO5 at boot-up because GPIO5_7 is used >> on AM437x GP-EVM to control VTT regulators on DDR3. >> Without this some GP-EVM boards will fail to boot because >> of DDR3 corruption. >> >> Reported-by: Nishanth Menon <nm@ti.com> >> Tested-by: Nishanth Menon <nm@ti.com> >> Signed-off-by: Dave Gerlach <d-gerlach@ti.com> >> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com> > > every now and again we see a patch like this because yet another board > is using a GPIO to toggle DDR regulators. > > Instead of constantly patching things like this, how about we try > something like below (build-tested only): Why should we change all of them? Is it correct to leave every single GPIO at the mercy of the bootloader in every situation? The reason we see these patches only every now and again is because it's a special case that should be handled only for that situation. I also don't think it makes sense to make gpio's a unique case that never gets reset while every other IP does by default. Regards, Dave > > diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c > index 1f33f5d..f5962ff 100644 > --- a/arch/arm/mach-omap2/omap_hwmod.c > +++ b/arch/arm/mach-omap2/omap_hwmod.c > @@ -2610,6 +2610,10 @@ static int __init _setup_reset(struct omap_hwmod *oh) > if (oh->flags & HWMOD_EXT_OPT_MAIN_CLK) > return -EPERM; > > + /* NEVER reset GPIO blocks */ > + if (strncmp(oh->name, "gpio", 4) == 0) > + return 0; > + > if (oh->rst_lines_cnt == 0) { > r = _enable(oh); > if (r) { > diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c > index 4243190..ce8b53a 100644 > --- a/drivers/gpio/gpio-omap.c > +++ b/drivers/gpio/gpio-omap.c > @@ -1130,6 +1130,29 @@ static void omap_gpio_chip_init(struct gpio_bank *bank) > > static const struct of_device_id omap_gpio_match[]; > > +static void omap_gpio_init_context(struct gpio_bank *p) > +{ > + struct omap_gpio_reg_offs *regs = p->regs; > + void __iomem *base = p->base; > + > + p->context.ctrl = readl_relaxed(base + regs->ctrl); > + p->context.oe = readl_relaxed(base + regs->direction); > + p->context.wake_en = readl_relaxed(base + regs->wkup_en); > + p->context.leveldetect0 = readl_relaxed(base + regs->leveldetect0); > + p->context.leveldetect1 = readl_relaxed(base + regs->leveldetect1); > + p->context.risingdetect = readl_relaxed(base + regs->risingdetect); > + p->context.fallingdetect = readl_relaxed(base + regs->fallingdetect); > + p->context.irqenable1 = readl_relaxed(base + regs->irqenable); > + p->context.irqenable2 = readl_relaxed(base + regs->irqenable2); > + > + if (regs->set_dataout && p->regs->clr_dataout) > + p->context.dataout = readl_relaxed(base + regs->set_dataout); > + else > + p->context.dataout = readl_relaxed(base + regs->dataout); > + > + p->context_valid = true; > +} > + > static int omap_gpio_probe(struct platform_device *pdev) > { > struct device *dev = &pdev->dev; > @@ -1246,6 +1269,7 @@ static int omap_gpio_probe(struct platform_device *pdev) > omap_gpio_mod_init(bank); > omap_gpio_chip_init(bank); > omap_gpio_show_rev(bank); > + omap_gpio_init_context(bank); > > pm_runtime_put(bank->dev); > > @@ -1325,8 +1349,6 @@ update_gpio_context_count: > return 0; > } > > -static void omap_gpio_init_context(struct gpio_bank *p); > - > static int omap_gpio_runtime_resume(struct device *dev) > { > struct platform_device *pdev = to_platform_device(dev); > @@ -1466,29 +1488,6 @@ void omap2_gpio_resume_after_idle(void) > } > > #if defined(CONFIG_PM_RUNTIME) > -static void omap_gpio_init_context(struct gpio_bank *p) > -{ > - struct omap_gpio_reg_offs *regs = p->regs; > - void __iomem *base = p->base; > - > - p->context.ctrl = readl_relaxed(base + regs->ctrl); > - p->context.oe = readl_relaxed(base + regs->direction); > - p->context.wake_en = readl_relaxed(base + regs->wkup_en); > - p->context.leveldetect0 = readl_relaxed(base + regs->leveldetect0); > - p->context.leveldetect1 = readl_relaxed(base + regs->leveldetect1); > - p->context.risingdetect = readl_relaxed(base + regs->risingdetect); > - p->context.fallingdetect = readl_relaxed(base + regs->fallingdetect); > - p->context.irqenable1 = readl_relaxed(base + regs->irqenable); > - p->context.irqenable2 = readl_relaxed(base + regs->irqenable2); > - > - if (regs->set_dataout && p->regs->clr_dataout) > - p->context.dataout = readl_relaxed(base + regs->set_dataout); > - else > - p->context.dataout = readl_relaxed(base + regs->dataout); > - > - p->context_valid = true; > -} > - > static void omap_gpio_restore_context(struct gpio_bank *bank) > { > writel_relaxed(bank->context.wake_en, > > Then, we can even remove ti,no-reset flag from all GPIO DT nodes. > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] ARM: dts: am437x-gp-evm: Do not reset gpio5 2014-03-24 15:29 ` Dave Gerlach @ 2014-03-24 18:50 ` Felipe Balbi -1 siblings, 0 replies; 18+ messages in thread From: Felipe Balbi @ 2014-03-24 18:50 UTC (permalink / raw) To: Dave Gerlach Cc: nm, devicetree, Lokesh Vutla, rnayak, nsekhar, balbi, tony, bcousson, mark.rutland, linux-omap, linux-arm-kernel [-- Attachment #1.1: Type: text/plain, Size: 2190 bytes --] Hi, On Mon, Mar 24, 2014 at 10:29:58AM -0500, Dave Gerlach wrote: > On 03/21/2014 12:52 AM, Felipe Balbi wrote: > >On Fri, Mar 21, 2014 at 10:50:13AM +0530, Lokesh Vutla wrote: > >>From: Dave Gerlach <d-gerlach@ti.com> > >> > >>Do not reset GPIO5 at boot-up because GPIO5_7 is used > >>on AM437x GP-EVM to control VTT regulators on DDR3. > >>Without this some GP-EVM boards will fail to boot because > >>of DDR3 corruption. > >> > >>Reported-by: Nishanth Menon <nm@ti.com> > >>Tested-by: Nishanth Menon <nm@ti.com> > >>Signed-off-by: Dave Gerlach <d-gerlach@ti.com> > >>Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com> > > > >every now and again we see a patch like this because yet another board > >is using a GPIO to toggle DDR regulators. > > > >Instead of constantly patching things like this, how about we try > >something like below (build-tested only): > > Why should we change all of them? Is it correct to leave every single GPIO > at the mercy of the bootloader in every situation? The reason we see these it's not leaving anything at the mercy of the bootloader. It's simply looking at the HW itself and asking "what's the current state of this GPIO ?" > patches only every now and again is because it's a special case that should I wouldn't call it "special". A GPIO block is pretty dumb, its registers only give you current pin state, there's virtually no state machine involved whatsoever. > be handled only for that situation. I also don't think it makes sense > to make gpio's a unique case that never gets reset while every other > IP does by default. Well, if it doesn't need to be reset, why would you spend that time resetting it ? In the GPIO case, you gain nothing by resetting the IP, nothing at all, other than "now I'm sure the IP is in no-standby/no-idle" but that can be easily read back from SYS[SC] registers anyway. The point is that we have two choices here: a) every time a new board comes around using GPIO as an enable signal for DDR, we spend a few days debugging why it's not booting. b) make sure no GPIO block is ever reset, so we never go through the debugging cycle again. -- balbi [-- Attachment #1.2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] [-- Attachment #2: Type: text/plain, Size: 176 bytes --] _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] ARM: dts: am437x-gp-evm: Do not reset gpio5 @ 2014-03-24 18:50 ` Felipe Balbi 0 siblings, 0 replies; 18+ messages in thread From: Felipe Balbi @ 2014-03-24 18:50 UTC (permalink / raw) To: linux-arm-kernel Hi, On Mon, Mar 24, 2014 at 10:29:58AM -0500, Dave Gerlach wrote: > On 03/21/2014 12:52 AM, Felipe Balbi wrote: > >On Fri, Mar 21, 2014 at 10:50:13AM +0530, Lokesh Vutla wrote: > >>From: Dave Gerlach <d-gerlach@ti.com> > >> > >>Do not reset GPIO5 at boot-up because GPIO5_7 is used > >>on AM437x GP-EVM to control VTT regulators on DDR3. > >>Without this some GP-EVM boards will fail to boot because > >>of DDR3 corruption. > >> > >>Reported-by: Nishanth Menon <nm@ti.com> > >>Tested-by: Nishanth Menon <nm@ti.com> > >>Signed-off-by: Dave Gerlach <d-gerlach@ti.com> > >>Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com> > > > >every now and again we see a patch like this because yet another board > >is using a GPIO to toggle DDR regulators. > > > >Instead of constantly patching things like this, how about we try > >something like below (build-tested only): > > Why should we change all of them? Is it correct to leave every single GPIO > at the mercy of the bootloader in every situation? The reason we see these it's not leaving anything at the mercy of the bootloader. It's simply looking at the HW itself and asking "what's the current state of this GPIO ?" > patches only every now and again is because it's a special case that should I wouldn't call it "special". A GPIO block is pretty dumb, its registers only give you current pin state, there's virtually no state machine involved whatsoever. > be handled only for that situation. I also don't think it makes sense > to make gpio's a unique case that never gets reset while every other > IP does by default. Well, if it doesn't need to be reset, why would you spend that time resetting it ? In the GPIO case, you gain nothing by resetting the IP, nothing at all, other than "now I'm sure the IP is in no-standby/no-idle" but that can be easily read back from SYS[SC] registers anyway. The point is that we have two choices here: a) every time a new board comes around using GPIO as an enable signal for DDR, we spend a few days debugging why it's not booting. b) make sure no GPIO block is ever reset, so we never go through the debugging cycle again. -- balbi -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140324/2ba8b747/attachment.sig> ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] ARM: dts: am437x-gp-evm: Do not reset gpio5 2014-03-24 18:50 ` Felipe Balbi @ 2014-03-24 19:01 ` Nishanth Menon -1 siblings, 0 replies; 18+ messages in thread From: Nishanth Menon @ 2014-03-24 19:01 UTC (permalink / raw) To: balbi, Dave Gerlach Cc: Lokesh Vutla, linux-omap, linux-arm-kernel, bcousson, tony, devicetree, nsekhar, rnayak, mark.rutland On 03/24/2014 01:50 PM, Felipe Balbi wrote: > Hi, > > On Mon, Mar 24, 2014 at 10:29:58AM -0500, Dave Gerlach wrote: >> On 03/21/2014 12:52 AM, Felipe Balbi wrote: >>> On Fri, Mar 21, 2014 at 10:50:13AM +0530, Lokesh Vutla wrote: >>>> From: Dave Gerlach <d-gerlach@ti.com> >>>> >>>> Do not reset GPIO5 at boot-up because GPIO5_7 is used >>>> on AM437x GP-EVM to control VTT regulators on DDR3. >>>> Without this some GP-EVM boards will fail to boot because >>>> of DDR3 corruption. >>>> >>>> Reported-by: Nishanth Menon <nm@ti.com> >>>> Tested-by: Nishanth Menon <nm@ti.com> >>>> Signed-off-by: Dave Gerlach <d-gerlach@ti.com> >>>> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com> >>> >>> every now and again we see a patch like this because yet another board >>> is using a GPIO to toggle DDR regulators. >>> >>> Instead of constantly patching things like this, how about we try >>> something like below (build-tested only): >> >> Why should we change all of them? Is it correct to leave every single GPIO >> at the mercy of the bootloader in every situation? The reason we see these > > it's not leaving anything at the mercy of the bootloader. It's simply > looking at the HW itself and asking "what's the current state of this > GPIO ?" And you assume here that every GPIO pin left in what ever state by bootloader is the correct state for kernel to function in? that is not exactly a good idea from even power consumption perspective - example GPIOs used by bootloader to detect board revision is not used in kernel, leaving such GPIOs active is not optimal, certain other GPIOs used by external peripherals might even be wrongly configured by bootloader issues -> the idea of ti,no-reset-on-init is to ensure that we *know* which instances "need special handling" and we depend on bootloader configuration. Taking it a step further, why is "not doing IP module kernel reset" just a constraint for GPIO then? Why not leave every IP module in what ever state bootloader left it at? >> patches only every now and again is because it's a special case that should > > I wouldn't call it "special". A GPIO block is pretty dumb, its registers > only give you current pin state, there's virtually no state machine > involved whatsoever. > >> be handled only for that situation. I also don't think it makes sense >> to make gpio's a unique case that never gets reset while every other >> IP does by default. > > Well, if it doesn't need to be reset, why would you spend that time > resetting it ? In the GPIO case, you gain nothing by resetting the IP, > nothing at all, other than "now I'm sure the IP is in > no-standby/no-idle" but that can be easily read back from SYS[SC] > registers anyway. because, you do not know how else the system might be used. instead of assuming the bootloader or host OS (in a virtualized environment) will always be doing the right thing, kernel takes responsibility of peripherals and exceptions are clearly marked (such as with ti,no-reset-on-init;). > > The point is that we have two choices here: > > a) every time a new board comes around using GPIO as an enable signal > for DDR, we spend a few days debugging why it's not booting. yeah - read the darned schematics - this is valid for anyone doing board/platform bringup - this is the right way to do it. > > b) make sure no GPIO block is ever reset, so we never go through the > debugging cycle again. > the holy grail of new bugs! Sigh! -- Regards, Nishanth Menon ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] ARM: dts: am437x-gp-evm: Do not reset gpio5 @ 2014-03-24 19:01 ` Nishanth Menon 0 siblings, 0 replies; 18+ messages in thread From: Nishanth Menon @ 2014-03-24 19:01 UTC (permalink / raw) To: linux-arm-kernel On 03/24/2014 01:50 PM, Felipe Balbi wrote: > Hi, > > On Mon, Mar 24, 2014 at 10:29:58AM -0500, Dave Gerlach wrote: >> On 03/21/2014 12:52 AM, Felipe Balbi wrote: >>> On Fri, Mar 21, 2014 at 10:50:13AM +0530, Lokesh Vutla wrote: >>>> From: Dave Gerlach <d-gerlach@ti.com> >>>> >>>> Do not reset GPIO5 at boot-up because GPIO5_7 is used >>>> on AM437x GP-EVM to control VTT regulators on DDR3. >>>> Without this some GP-EVM boards will fail to boot because >>>> of DDR3 corruption. >>>> >>>> Reported-by: Nishanth Menon <nm@ti.com> >>>> Tested-by: Nishanth Menon <nm@ti.com> >>>> Signed-off-by: Dave Gerlach <d-gerlach@ti.com> >>>> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com> >>> >>> every now and again we see a patch like this because yet another board >>> is using a GPIO to toggle DDR regulators. >>> >>> Instead of constantly patching things like this, how about we try >>> something like below (build-tested only): >> >> Why should we change all of them? Is it correct to leave every single GPIO >> at the mercy of the bootloader in every situation? The reason we see these > > it's not leaving anything at the mercy of the bootloader. It's simply > looking at the HW itself and asking "what's the current state of this > GPIO ?" And you assume here that every GPIO pin left in what ever state by bootloader is the correct state for kernel to function in? that is not exactly a good idea from even power consumption perspective - example GPIOs used by bootloader to detect board revision is not used in kernel, leaving such GPIOs active is not optimal, certain other GPIOs used by external peripherals might even be wrongly configured by bootloader issues -> the idea of ti,no-reset-on-init is to ensure that we *know* which instances "need special handling" and we depend on bootloader configuration. Taking it a step further, why is "not doing IP module kernel reset" just a constraint for GPIO then? Why not leave every IP module in what ever state bootloader left it at? >> patches only every now and again is because it's a special case that should > > I wouldn't call it "special". A GPIO block is pretty dumb, its registers > only give you current pin state, there's virtually no state machine > involved whatsoever. > >> be handled only for that situation. I also don't think it makes sense >> to make gpio's a unique case that never gets reset while every other >> IP does by default. > > Well, if it doesn't need to be reset, why would you spend that time > resetting it ? In the GPIO case, you gain nothing by resetting the IP, > nothing at all, other than "now I'm sure the IP is in > no-standby/no-idle" but that can be easily read back from SYS[SC] > registers anyway. because, you do not know how else the system might be used. instead of assuming the bootloader or host OS (in a virtualized environment) will always be doing the right thing, kernel takes responsibility of peripherals and exceptions are clearly marked (such as with ti,no-reset-on-init;). > > The point is that we have two choices here: > > a) every time a new board comes around using GPIO as an enable signal > for DDR, we spend a few days debugging why it's not booting. yeah - read the darned schematics - this is valid for anyone doing board/platform bringup - this is the right way to do it. > > b) make sure no GPIO block is ever reset, so we never go through the > debugging cycle again. > the holy grail of new bugs! Sigh! -- Regards, Nishanth Menon ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] ARM: dts: am437x-gp-evm: Do not reset gpio5 2014-03-24 19:01 ` Nishanth Menon @ 2014-03-24 19:22 ` Felipe Balbi -1 siblings, 0 replies; 18+ messages in thread From: Felipe Balbi @ 2014-03-24 19:22 UTC (permalink / raw) To: Nishanth Menon Cc: balbi, Dave Gerlach, Lokesh Vutla, linux-omap, linux-arm-kernel, bcousson, tony, devicetree, nsekhar, rnayak, mark.rutland [-- Attachment #1: Type: text/plain, Size: 4557 bytes --] Hi, On Mon, Mar 24, 2014 at 02:01:30PM -0500, Nishanth Menon wrote: > On 03/24/2014 01:50 PM, Felipe Balbi wrote: > > Hi, > > > > On Mon, Mar 24, 2014 at 10:29:58AM -0500, Dave Gerlach wrote: > >> On 03/21/2014 12:52 AM, Felipe Balbi wrote: > >>> On Fri, Mar 21, 2014 at 10:50:13AM +0530, Lokesh Vutla wrote: > >>>> From: Dave Gerlach <d-gerlach@ti.com> > >>>> > >>>> Do not reset GPIO5 at boot-up because GPIO5_7 is used > >>>> on AM437x GP-EVM to control VTT regulators on DDR3. > >>>> Without this some GP-EVM boards will fail to boot because > >>>> of DDR3 corruption. > >>>> > >>>> Reported-by: Nishanth Menon <nm@ti.com> > >>>> Tested-by: Nishanth Menon <nm@ti.com> > >>>> Signed-off-by: Dave Gerlach <d-gerlach@ti.com> > >>>> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com> > >>> > >>> every now and again we see a patch like this because yet another board > >>> is using a GPIO to toggle DDR regulators. > >>> > >>> Instead of constantly patching things like this, how about we try > >>> something like below (build-tested only): > >> > >> Why should we change all of them? Is it correct to leave every single GPIO > >> at the mercy of the bootloader in every situation? The reason we see these > > > > it's not leaving anything at the mercy of the bootloader. It's simply > > looking at the HW itself and asking "what's the current state of this > > GPIO ?" > > And you assume here that every GPIO pin left in what ever state by > bootloader is the correct state for kernel to function in? that is not > exactly a good idea from even power consumption perspective - example right, it's a much better idea to reset the IP which, well, enables the f-ing DDR. Congrats! > GPIOs used by bootloader to detect board revision is not used in > kernel, leaving such GPIOs active is not optimal, certain other GPIOs > used by external peripherals might even be wrongly configured by > bootloader issues -> the idea of ti,no-reset-on-init is to ensure that > we *know* which instances "need special handling" and we depend on > bootloader configuration. or you can just make your driver smarter and for GPIOs which are not used by the kernel, you ignore context save-restore. The problem is that our GPIO driver is so fucked up that nobody really cares about fixing it. If you really wanted to fix it, it wouldn't be really difficult to make use of pm_runtime's (and clock API's) usage counters to avoid keeping certain blocks up when not necessary. > Taking it a step further, why is "not doing IP module kernel reset" > just a constraint for GPIO then? Why not leave every IP module in what > ever state bootloader left it at? yeah, that's exactly what I suggested. Thanks for clarifying what *I* *SAID*. > >> patches only every now and again is because it's a special case that should > > > > I wouldn't call it "special". A GPIO block is pretty dumb, its registers > > only give you current pin state, there's virtually no state machine > > involved whatsoever. > > > >> be handled only for that situation. I also don't think it makes sense > >> to make gpio's a unique case that never gets reset while every other > >> IP does by default. > > > > Well, if it doesn't need to be reset, why would you spend that time > > resetting it ? In the GPIO case, you gain nothing by resetting the IP, > > nothing at all, other than "now I'm sure the IP is in > > no-standby/no-idle" but that can be easily read back from SYS[SC] > > registers anyway. > > because, you do not know how else the system might be used. instead of > assuming the bootloader or host OS (in a virtualized environment) will > always be doing the right thing, kernel takes responsibility of > peripherals and exceptions are clearly marked (such as with > ti,no-reset-on-init;). yada, yada, yada... > > The point is that we have two choices here: > > > > a) every time a new board comes around using GPIO as an enable signal > > for DDR, we spend a few days debugging why it's not booting. > > yeah - read the darned schematics - this is valid for anyone doing > board/platform bringup - this is the right way to do it. sure, I'll let you handle that next time, be my guest. > > b) make sure no GPIO block is ever reset, so we never go through the > > debugging cycle again. > > > the holy grail of new bugs! Sigh! right, our code so god-damned perfect, ain't it ? We better not touch it anymore. Fair enough, I withdraw my comments, do as you wish. -- balbi [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] ARM: dts: am437x-gp-evm: Do not reset gpio5 @ 2014-03-24 19:22 ` Felipe Balbi 0 siblings, 0 replies; 18+ messages in thread From: Felipe Balbi @ 2014-03-24 19:22 UTC (permalink / raw) To: linux-arm-kernel Hi, On Mon, Mar 24, 2014 at 02:01:30PM -0500, Nishanth Menon wrote: > On 03/24/2014 01:50 PM, Felipe Balbi wrote: > > Hi, > > > > On Mon, Mar 24, 2014 at 10:29:58AM -0500, Dave Gerlach wrote: > >> On 03/21/2014 12:52 AM, Felipe Balbi wrote: > >>> On Fri, Mar 21, 2014 at 10:50:13AM +0530, Lokesh Vutla wrote: > >>>> From: Dave Gerlach <d-gerlach@ti.com> > >>>> > >>>> Do not reset GPIO5 at boot-up because GPIO5_7 is used > >>>> on AM437x GP-EVM to control VTT regulators on DDR3. > >>>> Without this some GP-EVM boards will fail to boot because > >>>> of DDR3 corruption. > >>>> > >>>> Reported-by: Nishanth Menon <nm@ti.com> > >>>> Tested-by: Nishanth Menon <nm@ti.com> > >>>> Signed-off-by: Dave Gerlach <d-gerlach@ti.com> > >>>> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com> > >>> > >>> every now and again we see a patch like this because yet another board > >>> is using a GPIO to toggle DDR regulators. > >>> > >>> Instead of constantly patching things like this, how about we try > >>> something like below (build-tested only): > >> > >> Why should we change all of them? Is it correct to leave every single GPIO > >> at the mercy of the bootloader in every situation? The reason we see these > > > > it's not leaving anything at the mercy of the bootloader. It's simply > > looking at the HW itself and asking "what's the current state of this > > GPIO ?" > > And you assume here that every GPIO pin left in what ever state by > bootloader is the correct state for kernel to function in? that is not > exactly a good idea from even power consumption perspective - example right, it's a much better idea to reset the IP which, well, enables the f-ing DDR. Congrats! > GPIOs used by bootloader to detect board revision is not used in > kernel, leaving such GPIOs active is not optimal, certain other GPIOs > used by external peripherals might even be wrongly configured by > bootloader issues -> the idea of ti,no-reset-on-init is to ensure that > we *know* which instances "need special handling" and we depend on > bootloader configuration. or you can just make your driver smarter and for GPIOs which are not used by the kernel, you ignore context save-restore. The problem is that our GPIO driver is so fucked up that nobody really cares about fixing it. If you really wanted to fix it, it wouldn't be really difficult to make use of pm_runtime's (and clock API's) usage counters to avoid keeping certain blocks up when not necessary. > Taking it a step further, why is "not doing IP module kernel reset" > just a constraint for GPIO then? Why not leave every IP module in what > ever state bootloader left it at? yeah, that's exactly what I suggested. Thanks for clarifying what *I* *SAID*. > >> patches only every now and again is because it's a special case that should > > > > I wouldn't call it "special". A GPIO block is pretty dumb, its registers > > only give you current pin state, there's virtually no state machine > > involved whatsoever. > > > >> be handled only for that situation. I also don't think it makes sense > >> to make gpio's a unique case that never gets reset while every other > >> IP does by default. > > > > Well, if it doesn't need to be reset, why would you spend that time > > resetting it ? In the GPIO case, you gain nothing by resetting the IP, > > nothing at all, other than "now I'm sure the IP is in > > no-standby/no-idle" but that can be easily read back from SYS[SC] > > registers anyway. > > because, you do not know how else the system might be used. instead of > assuming the bootloader or host OS (in a virtualized environment) will > always be doing the right thing, kernel takes responsibility of > peripherals and exceptions are clearly marked (such as with > ti,no-reset-on-init;). yada, yada, yada... > > The point is that we have two choices here: > > > > a) every time a new board comes around using GPIO as an enable signal > > for DDR, we spend a few days debugging why it's not booting. > > yeah - read the darned schematics - this is valid for anyone doing > board/platform bringup - this is the right way to do it. sure, I'll let you handle that next time, be my guest. > > b) make sure no GPIO block is ever reset, so we never go through the > > debugging cycle again. > > > the holy grail of new bugs! Sigh! right, our code so god-damned perfect, ain't it ? We better not touch it anymore. Fair enough, I withdraw my comments, do as you wish. -- balbi -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140324/898c41e8/attachment.sig> ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] ARM: dts: am437x-gp-evm: Do not reset gpio5 2014-03-21 5:20 ` Lokesh Vutla @ 2014-03-25 15:02 ` Nishanth Menon -1 siblings, 0 replies; 18+ messages in thread From: Nishanth Menon @ 2014-03-25 15:02 UTC (permalink / raw) To: Lokesh Vutla, linux-omap, bcousson Cc: linux-arm-kernel, tony, devicetree, nsekhar, rnayak, mark.rutland, Dave Gerlach On 03/21/2014 12:20 AM, Lokesh Vutla wrote: > From: Dave Gerlach <d-gerlach@ti.com> > > Do not reset GPIO5 at boot-up because GPIO5_7 is used > on AM437x GP-EVM to control VTT regulators on DDR3. > Without this some GP-EVM boards will fail to boot because > of DDR3 corruption. > > Reported-by: Nishanth Menon <nm@ti.com> > Tested-by: Nishanth Menon <nm@ti.com> > Signed-off-by: Dave Gerlach <d-gerlach@ti.com> > Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com> > --- > This is applied on top of current linux-next. > git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master > > arch/arm/boot/dts/am437x-gp-evm.dts | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/arch/arm/boot/dts/am437x-gp-evm.dts b/arch/arm/boot/dts/am437x-gp-evm.dts > index df8798e..a055f7f 100644 > --- a/arch/arm/boot/dts/am437x-gp-evm.dts > +++ b/arch/arm/boot/dts/am437x-gp-evm.dts > @@ -117,6 +117,11 @@ > status = "okay"; > }; > > +&gpio5 { > + status = "okay"; > + ti,no-reset-on-init; > +}; > + > &mmc1 { > status = "okay"; > vmmc-supply = <&vmmcsd_fixed>; > Tony, any chance of having this in 3.15 cycle? as of today's linux-next tag, the only platform I have that does not boot is due to the lack of this patch. next-20140325-omap2plus_defconfig 1: am335x-evm: Boot PASS: http://slexy.org/raw/s20o6CfGL8 2: am335x-sk: Boot PASS: http://slexy.org/raw/s2Ogu1zPtD 3: am3517-evm: Boot PASS: http://slexy.org/raw/s2m2WhHYO8 4: am37x-evm: Boot PASS: http://slexy.org/raw/s2jOybaVno 5: am43xx-epos: Boot PASS: http://slexy.org/raw/s21tt52N4b 6: am43xx-gpevm: Boot FAIL: http://slexy.org/raw/s21eDQmFWJ 7: beagleboard: Boot PASS: http://slexy.org/raw/s2q7vzrZHb 8: beaglebone-black: Boot PASS: http://slexy.org/raw/s2HCAJVz0H 9: beaglebone: Boot PASS: http://slexy.org/raw/s2NioACYsM 10: craneboard: Boot PASS: http://slexy.org/raw/s2s7jyo52c 11: dra7: Boot PASS: http://slexy.org/raw/s2OQ1BMerx 12: ldp: Boot PASS: http://slexy.org/raw/s2cWgQrdjb 13: pandaboard-es: Boot PASS: http://slexy.org/raw/s2lOaznVLO 14: sdp2430: Boot PASS: http://slexy.org/raw/s21RCJlUDU 15: sdp3430: Boot PASS: http://slexy.org/raw/s2181J9eWO 16: sdp4430: Boot PASS: http://slexy.org/raw/s20UumAiXK 17: OMAP5432uEVM: Boot PASS: http://slexy.org/raw/s21ypXvTNc TOTAL = 17 boards, Booted Boards = 16, No Boot boards = 1 -- Regards, Nishanth Menon ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] ARM: dts: am437x-gp-evm: Do not reset gpio5 @ 2014-03-25 15:02 ` Nishanth Menon 0 siblings, 0 replies; 18+ messages in thread From: Nishanth Menon @ 2014-03-25 15:02 UTC (permalink / raw) To: linux-arm-kernel On 03/21/2014 12:20 AM, Lokesh Vutla wrote: > From: Dave Gerlach <d-gerlach@ti.com> > > Do not reset GPIO5 at boot-up because GPIO5_7 is used > on AM437x GP-EVM to control VTT regulators on DDR3. > Without this some GP-EVM boards will fail to boot because > of DDR3 corruption. > > Reported-by: Nishanth Menon <nm@ti.com> > Tested-by: Nishanth Menon <nm@ti.com> > Signed-off-by: Dave Gerlach <d-gerlach@ti.com> > Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com> > --- > This is applied on top of current linux-next. > git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master > > arch/arm/boot/dts/am437x-gp-evm.dts | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/arch/arm/boot/dts/am437x-gp-evm.dts b/arch/arm/boot/dts/am437x-gp-evm.dts > index df8798e..a055f7f 100644 > --- a/arch/arm/boot/dts/am437x-gp-evm.dts > +++ b/arch/arm/boot/dts/am437x-gp-evm.dts > @@ -117,6 +117,11 @@ > status = "okay"; > }; > > +&gpio5 { > + status = "okay"; > + ti,no-reset-on-init; > +}; > + > &mmc1 { > status = "okay"; > vmmc-supply = <&vmmcsd_fixed>; > Tony, any chance of having this in 3.15 cycle? as of today's linux-next tag, the only platform I have that does not boot is due to the lack of this patch. next-20140325-omap2plus_defconfig 1: am335x-evm: Boot PASS: http://slexy.org/raw/s20o6CfGL8 2: am335x-sk: Boot PASS: http://slexy.org/raw/s2Ogu1zPtD 3: am3517-evm: Boot PASS: http://slexy.org/raw/s2m2WhHYO8 4: am37x-evm: Boot PASS: http://slexy.org/raw/s2jOybaVno 5: am43xx-epos: Boot PASS: http://slexy.org/raw/s21tt52N4b 6: am43xx-gpevm: Boot FAIL: http://slexy.org/raw/s21eDQmFWJ 7: beagleboard: Boot PASS: http://slexy.org/raw/s2q7vzrZHb 8: beaglebone-black: Boot PASS: http://slexy.org/raw/s2HCAJVz0H 9: beaglebone: Boot PASS: http://slexy.org/raw/s2NioACYsM 10: craneboard: Boot PASS: http://slexy.org/raw/s2s7jyo52c 11: dra7: Boot PASS: http://slexy.org/raw/s2OQ1BMerx 12: ldp: Boot PASS: http://slexy.org/raw/s2cWgQrdjb 13: pandaboard-es: Boot PASS: http://slexy.org/raw/s2lOaznVLO 14: sdp2430: Boot PASS: http://slexy.org/raw/s21RCJlUDU 15: sdp3430: Boot PASS: http://slexy.org/raw/s2181J9eWO 16: sdp4430: Boot PASS: http://slexy.org/raw/s20UumAiXK 17: OMAP5432uEVM: Boot PASS: http://slexy.org/raw/s21ypXvTNc TOTAL = 17 boards, Booted Boards = 16, No Boot boards = 1 -- Regards, Nishanth Menon ^ permalink raw reply [flat|nested] 18+ messages in thread
[parent not found: <53319A7D.1060904-l0cyMroinI0@public.gmane.org>]
* Re: [PATCH] ARM: dts: am437x-gp-evm: Do not reset gpio5 2014-03-25 15:02 ` Nishanth Menon @ 2014-04-17 16:34 ` Tony Lindgren -1 siblings, 0 replies; 18+ messages in thread From: Tony Lindgren @ 2014-04-17 16:34 UTC (permalink / raw) To: Nishanth Menon Cc: Lokesh Vutla, linux-omap-u79uwXL29TY76Z2rM5mHXA, bcousson-rdvid1DuHRBWk0Htik3J/w, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree-u79uwXL29TY76Z2rM5mHXA, nsekhar-l0cyMroinI0, rnayak-l0cyMroinI0, mark.rutland-5wv7dgnIgG8, Dave Gerlach * Nishanth Menon <nm-l0cyMroinI0@public.gmane.org> [140325 08:05]: > On 03/21/2014 12:20 AM, Lokesh Vutla wrote: > > From: Dave Gerlach <d-gerlach-l0cyMroinI0@public.gmane.org> > > > > Do not reset GPIO5 at boot-up because GPIO5_7 is used > > on AM437x GP-EVM to control VTT regulators on DDR3. > > Without this some GP-EVM boards will fail to boot because > > of DDR3 corruption. How funny :) Ideally we would be able to specify which GPIO pins should maintain their state during the boot. AFAIK, this patch currently means that the kernel has no idea what state the whole GPIO bank is in. At minimum we should parse the GPIO bank state so the kernel knows it and then it should be safe to set the no reset flag. So for the workaround, can you guys please try to see test if the old mux trick in the bootloader works to mux the pin into something PIN_INPUT_PULLUP | MUX_MODE7? Or a PULLDOWN depending on the direction naturally. That would allow leaving out the GPIO completely from this. Regards, Tony > > Reported-by: Nishanth Menon <nm-l0cyMroinI0@public.gmane.org> > > Tested-by: Nishanth Menon <nm-l0cyMroinI0@public.gmane.org> > > Signed-off-by: Dave Gerlach <d-gerlach-l0cyMroinI0@public.gmane.org> > > Signed-off-by: Lokesh Vutla <lokeshvutla-l0cyMroinI0@public.gmane.org> > > --- > > This is applied on top of current linux-next. > > git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master > > > > arch/arm/boot/dts/am437x-gp-evm.dts | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/arch/arm/boot/dts/am437x-gp-evm.dts b/arch/arm/boot/dts/am437x-gp-evm.dts > > index df8798e..a055f7f 100644 > > --- a/arch/arm/boot/dts/am437x-gp-evm.dts > > +++ b/arch/arm/boot/dts/am437x-gp-evm.dts > > @@ -117,6 +117,11 @@ > > status = "okay"; > > }; > > > > +&gpio5 { > > + status = "okay"; > > + ti,no-reset-on-init; > > +}; > > + > > &mmc1 { > > status = "okay"; > > vmmc-supply = <&vmmcsd_fixed>; > > > > > Tony, > any chance of having this in 3.15 cycle? > > as of today's linux-next tag, the only platform I have that does not > boot is due to the lack of this patch. > > > next-20140325-omap2plus_defconfig > 1: am335x-evm: Boot PASS: http://slexy.org/raw/s20o6CfGL8 > 2: am335x-sk: Boot PASS: http://slexy.org/raw/s2Ogu1zPtD > 3: am3517-evm: Boot PASS: http://slexy.org/raw/s2m2WhHYO8 > 4: am37x-evm: Boot PASS: http://slexy.org/raw/s2jOybaVno > 5: am43xx-epos: Boot PASS: http://slexy.org/raw/s21tt52N4b > 6: am43xx-gpevm: Boot FAIL: http://slexy.org/raw/s21eDQmFWJ > 7: beagleboard: Boot PASS: http://slexy.org/raw/s2q7vzrZHb > 8: beaglebone-black: Boot PASS: http://slexy.org/raw/s2HCAJVz0H > 9: beaglebone: Boot PASS: http://slexy.org/raw/s2NioACYsM > 10: craneboard: Boot PASS: http://slexy.org/raw/s2s7jyo52c > 11: dra7: Boot PASS: http://slexy.org/raw/s2OQ1BMerx > 12: ldp: Boot PASS: http://slexy.org/raw/s2cWgQrdjb > 13: pandaboard-es: Boot PASS: http://slexy.org/raw/s2lOaznVLO > 14: sdp2430: Boot PASS: http://slexy.org/raw/s21RCJlUDU > 15: sdp3430: Boot PASS: http://slexy.org/raw/s2181J9eWO > 16: sdp4430: Boot PASS: http://slexy.org/raw/s20UumAiXK > 17: OMAP5432uEVM: Boot PASS: http://slexy.org/raw/s21ypXvTNc > TOTAL = 17 boards, Booted Boards = 16, No Boot boards = 1 > > -- > Regards, > Nishanth Menon -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] ARM: dts: am437x-gp-evm: Do not reset gpio5 @ 2014-04-17 16:34 ` Tony Lindgren 0 siblings, 0 replies; 18+ messages in thread From: Tony Lindgren @ 2014-04-17 16:34 UTC (permalink / raw) To: linux-arm-kernel * Nishanth Menon <nm@ti.com> [140325 08:05]: > On 03/21/2014 12:20 AM, Lokesh Vutla wrote: > > From: Dave Gerlach <d-gerlach@ti.com> > > > > Do not reset GPIO5 at boot-up because GPIO5_7 is used > > on AM437x GP-EVM to control VTT regulators on DDR3. > > Without this some GP-EVM boards will fail to boot because > > of DDR3 corruption. How funny :) Ideally we would be able to specify which GPIO pins should maintain their state during the boot. AFAIK, this patch currently means that the kernel has no idea what state the whole GPIO bank is in. At minimum we should parse the GPIO bank state so the kernel knows it and then it should be safe to set the no reset flag. So for the workaround, can you guys please try to see test if the old mux trick in the bootloader works to mux the pin into something PIN_INPUT_PULLUP | MUX_MODE7? Or a PULLDOWN depending on the direction naturally. That would allow leaving out the GPIO completely from this. Regards, Tony > > Reported-by: Nishanth Menon <nm@ti.com> > > Tested-by: Nishanth Menon <nm@ti.com> > > Signed-off-by: Dave Gerlach <d-gerlach@ti.com> > > Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com> > > --- > > This is applied on top of current linux-next. > > git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master > > > > arch/arm/boot/dts/am437x-gp-evm.dts | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/arch/arm/boot/dts/am437x-gp-evm.dts b/arch/arm/boot/dts/am437x-gp-evm.dts > > index df8798e..a055f7f 100644 > > --- a/arch/arm/boot/dts/am437x-gp-evm.dts > > +++ b/arch/arm/boot/dts/am437x-gp-evm.dts > > @@ -117,6 +117,11 @@ > > status = "okay"; > > }; > > > > +&gpio5 { > > + status = "okay"; > > + ti,no-reset-on-init; > > +}; > > + > > &mmc1 { > > status = "okay"; > > vmmc-supply = <&vmmcsd_fixed>; > > > > > Tony, > any chance of having this in 3.15 cycle? > > as of today's linux-next tag, the only platform I have that does not > boot is due to the lack of this patch. > > > next-20140325-omap2plus_defconfig > 1: am335x-evm: Boot PASS: http://slexy.org/raw/s20o6CfGL8 > 2: am335x-sk: Boot PASS: http://slexy.org/raw/s2Ogu1zPtD > 3: am3517-evm: Boot PASS: http://slexy.org/raw/s2m2WhHYO8 > 4: am37x-evm: Boot PASS: http://slexy.org/raw/s2jOybaVno > 5: am43xx-epos: Boot PASS: http://slexy.org/raw/s21tt52N4b > 6: am43xx-gpevm: Boot FAIL: http://slexy.org/raw/s21eDQmFWJ > 7: beagleboard: Boot PASS: http://slexy.org/raw/s2q7vzrZHb > 8: beaglebone-black: Boot PASS: http://slexy.org/raw/s2HCAJVz0H > 9: beaglebone: Boot PASS: http://slexy.org/raw/s2NioACYsM > 10: craneboard: Boot PASS: http://slexy.org/raw/s2s7jyo52c > 11: dra7: Boot PASS: http://slexy.org/raw/s2OQ1BMerx > 12: ldp: Boot PASS: http://slexy.org/raw/s2cWgQrdjb > 13: pandaboard-es: Boot PASS: http://slexy.org/raw/s2lOaznVLO > 14: sdp2430: Boot PASS: http://slexy.org/raw/s21RCJlUDU > 15: sdp3430: Boot PASS: http://slexy.org/raw/s2181J9eWO > 16: sdp4430: Boot PASS: http://slexy.org/raw/s20UumAiXK > 17: OMAP5432uEVM: Boot PASS: http://slexy.org/raw/s21ypXvTNc > TOTAL = 17 boards, Booted Boards = 16, No Boot boards = 1 > > -- > Regards, > Nishanth Menon ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] ARM: dts: am437x-gp-evm: Do not reset gpio5 2014-04-17 16:34 ` Tony Lindgren @ 2014-04-25 16:46 ` Tony Lindgren -1 siblings, 0 replies; 18+ messages in thread From: Tony Lindgren @ 2014-04-25 16:46 UTC (permalink / raw) To: Nishanth Menon Cc: Lokesh Vutla, linux-omap, bcousson, linux-arm-kernel, devicetree, nsekhar, rnayak, mark.rutland, Dave Gerlach * Tony Lindgren <tony@atomide.com> [140417 09:34]: > * Nishanth Menon <nm@ti.com> [140325 08:05]: > > On 03/21/2014 12:20 AM, Lokesh Vutla wrote: > > > From: Dave Gerlach <d-gerlach@ti.com> > > > > > > Do not reset GPIO5 at boot-up because GPIO5_7 is used > > > on AM437x GP-EVM to control VTT regulators on DDR3. > > > Without this some GP-EVM boards will fail to boot because > > > of DDR3 corruption. > > How funny :) > > Ideally we would be able to specify which GPIO pins should > maintain their state during the boot. > > AFAIK, this patch currently means that the kernel has no idea > what state the whole GPIO bank is in. At minimum we should > parse the GPIO bank state so the kernel knows it and then it > should be safe to set the no reset flag. > > So for the workaround, can you guys please try to see test > if the old mux trick in the bootloader works to mux the pin > into something PIN_INPUT_PULLUP | MUX_MODE7? Or a PULLDOWN > depending on the direction naturally. That would allow > leaving out the GPIO completely from this. OK so no safe mode as MUX_MODE7 on am335x. So based on the tests done by Dave on various GPIO banks without the reset, this seems OK to do. So applying into omap-for-v3.15/fixes-v2 to get the board booting properly. Naturally this is not a reason to stop further work on making sure the GPIO driver actually knows what state the bank is as suggested by Felipe. Tony ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] ARM: dts: am437x-gp-evm: Do not reset gpio5 @ 2014-04-25 16:46 ` Tony Lindgren 0 siblings, 0 replies; 18+ messages in thread From: Tony Lindgren @ 2014-04-25 16:46 UTC (permalink / raw) To: linux-arm-kernel * Tony Lindgren <tony@atomide.com> [140417 09:34]: > * Nishanth Menon <nm@ti.com> [140325 08:05]: > > On 03/21/2014 12:20 AM, Lokesh Vutla wrote: > > > From: Dave Gerlach <d-gerlach@ti.com> > > > > > > Do not reset GPIO5 at boot-up because GPIO5_7 is used > > > on AM437x GP-EVM to control VTT regulators on DDR3. > > > Without this some GP-EVM boards will fail to boot because > > > of DDR3 corruption. > > How funny :) > > Ideally we would be able to specify which GPIO pins should > maintain their state during the boot. > > AFAIK, this patch currently means that the kernel has no idea > what state the whole GPIO bank is in. At minimum we should > parse the GPIO bank state so the kernel knows it and then it > should be safe to set the no reset flag. > > So for the workaround, can you guys please try to see test > if the old mux trick in the bootloader works to mux the pin > into something PIN_INPUT_PULLUP | MUX_MODE7? Or a PULLDOWN > depending on the direction naturally. That would allow > leaving out the GPIO completely from this. OK so no safe mode as MUX_MODE7 on am335x. So based on the tests done by Dave on various GPIO banks without the reset, this seems OK to do. So applying into omap-for-v3.15/fixes-v2 to get the board booting properly. Naturally this is not a reason to stop further work on making sure the GPIO driver actually knows what state the bank is as suggested by Felipe. Tony ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2014-04-25 16:46 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-03-21 5:20 [PATCH] ARM: dts: am437x-gp-evm: Do not reset gpio5 Lokesh Vutla
2014-03-21 5:20 ` Lokesh Vutla
[not found] ` <1395379213-15451-1-git-send-email-lokeshvutla-l0cyMroinI0@public.gmane.org>
2014-03-21 5:52 ` Felipe Balbi
2014-03-21 5:52 ` Felipe Balbi
2014-03-24 15:29 ` Dave Gerlach
2014-03-24 15:29 ` Dave Gerlach
2014-03-24 18:50 ` Felipe Balbi
2014-03-24 18:50 ` Felipe Balbi
2014-03-24 19:01 ` Nishanth Menon
2014-03-24 19:01 ` Nishanth Menon
2014-03-24 19:22 ` Felipe Balbi
2014-03-24 19:22 ` Felipe Balbi
2014-03-25 15:02 ` Nishanth Menon
2014-03-25 15:02 ` Nishanth Menon
[not found] ` <53319A7D.1060904-l0cyMroinI0@public.gmane.org>
2014-04-17 16:34 ` Tony Lindgren
2014-04-17 16:34 ` Tony Lindgren
2014-04-25 16:46 ` Tony Lindgren
2014-04-25 16:46 ` Tony Lindgren
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.