From: Tony Lindgren <tony@atomide.com>
To: Philipp Zabel <p.zabel@pengutronix.de>
Cc: Arnd Bergmann <arnd@arndb.de>, Tero Kristo <t-kristo@ti.com>,
Philipp Zabel <philipp.zabel@gmail.com>,
linux-omap@vger.kernel.org, Lee Jones <lee.jones@linaro.org>,
linux-arm-kernel <linux-arm-kernel@lists.infradead.org>
Subject: Re: GPIO regression in Linux next caused by syscon change
Date: Mon, 15 Feb 2016 10:06:36 -0800 [thread overview]
Message-ID: <20160215180635.GW19432@atomide.com> (raw)
In-Reply-To: <20160215164757.GV19432@atomide.com>
* Tony Lindgren <tony@atomide.com> [160215 08:49]:
> * Philipp Zabel <p.zabel@pengutronix.de> [160215 08:17]:
> > Am Montag, den 15.02.2016, 08:01 -0800 schrieb Tony Lindgren:
> > > * Philipp Zabel <philipp.zabel@gmail.com> [160214 11:24]:
> > > > I've just replaced the of_iomap() call with an open coded version,
> > > > calling of_address_to_resource() and ioremap() directly. That was
> > > > needed so I can use the struct resource returned by
> > > > of_address_to_resource() to set the syscon_config.max_register. I
> > > > don't see where this could cause resource overlap. Does just setting
> > > > syscon_config.max_register to zero again make the problem disappear?
> > >
> > > Yes commenting out the syscon_config.max_register line in your patch
> > > makes things work again.
> > >
> > > So what does that tell us about the problem?
> >
> > Maybe some out of bounds writes that previously worked are now catched
> > by the max_register check in regmap_writable and regmap_write returns
> > -EIO instead of the write being executed.
>
> Hmm weird that something like that would not produce errors?
>
> > Is there any omap_ctrl_write?() call with an offset > 0x32c into
> > scm_conf?
>
> Indeed, that's where things go wrong. Adding Tero to Cc, something
> is wrong there.
Something like this might fix it? Needs to be tested to see if it
happens on other omaps.
Tero, got any better ideas?
Regards,
Tony
8< -------------------
--- a/arch/arm/mach-omap2/control.c
+++ b/arch/arm/mach-omap2/control.c
@@ -167,15 +167,23 @@ u16 omap_ctrl_readw(u16 offset)
u32 omap_ctrl_readl(u16 offset)
{
u32 val;
+ int err;
offset &= 0xfffc;
- if (!omap2_ctrl_syscon)
- val = readl_relaxed(omap2_ctrl_base + offset);
- else
- regmap_read(omap2_ctrl_syscon, omap2_ctrl_offset + offset,
- &val);
+ if (omap2_ctrl_syscon) {
+ err = regmap_read(omap2_ctrl_syscon,
+ omap2_ctrl_offset + offset, &val);
+ if (!err)
+ return val;
+ }
+
+ if (WARN(!omap2_ctrl_base,
+ "syscon out of range for offset 0x%x, no omap2_ctrl_base?\n",
+ offset)) {
+ return 0;
+ }
- return val;
+ return readl_relaxed(omap2_ctrl_base + offset);
}
void omap_ctrl_writeb(u8 val, u16 offset)
@@ -206,12 +214,23 @@ void omap_ctrl_writew(u16 val, u16 offset)
void omap_ctrl_writel(u32 val, u16 offset)
{
+ int err;
+
offset &= 0xfffc;
- if (!omap2_ctrl_syscon)
- writel_relaxed(val, omap2_ctrl_base + offset);
- else
- regmap_write(omap2_ctrl_syscon, omap2_ctrl_offset + offset,
- val);
+ if (omap2_ctrl_syscon) {
+ err = regmap_write(omap2_ctrl_syscon,
+ omap2_ctrl_offset + offset, val);
+ if (!err)
+ return;
+ }
+
+ if (WARN(!omap2_ctrl_base,
+ "syscon out of range for offset 0x%x, no omap2_ctrl_base?\n",
+ offset)) {
+ return;
+ }
+
+ writel_relaxed(val, omap2_ctrl_base + offset);
}
#ifdef CONFIG_ARCH_OMAP3
@@ -724,9 +743,6 @@ int __init omap_control_init(void)
if (ret)
return ret;
}
-
- iounmap(omap2_ctrl_base);
- omap2_ctrl_base = NULL;
} else {
/* No scm_conf found, direct access */
ret = omap2_clk_provider_init(np, data->index, NULL,
WARNING: multiple messages have this Message-ID (diff)
From: tony@atomide.com (Tony Lindgren)
To: linux-arm-kernel@lists.infradead.org
Subject: GPIO regression in Linux next caused by syscon change
Date: Mon, 15 Feb 2016 10:06:36 -0800 [thread overview]
Message-ID: <20160215180635.GW19432@atomide.com> (raw)
In-Reply-To: <20160215164757.GV19432@atomide.com>
* Tony Lindgren <tony@atomide.com> [160215 08:49]:
> * Philipp Zabel <p.zabel@pengutronix.de> [160215 08:17]:
> > Am Montag, den 15.02.2016, 08:01 -0800 schrieb Tony Lindgren:
> > > * Philipp Zabel <philipp.zabel@gmail.com> [160214 11:24]:
> > > > I've just replaced the of_iomap() call with an open coded version,
> > > > calling of_address_to_resource() and ioremap() directly. That was
> > > > needed so I can use the struct resource returned by
> > > > of_address_to_resource() to set the syscon_config.max_register. I
> > > > don't see where this could cause resource overlap. Does just setting
> > > > syscon_config.max_register to zero again make the problem disappear?
> > >
> > > Yes commenting out the syscon_config.max_register line in your patch
> > > makes things work again.
> > >
> > > So what does that tell us about the problem?
> >
> > Maybe some out of bounds writes that previously worked are now catched
> > by the max_register check in regmap_writable and regmap_write returns
> > -EIO instead of the write being executed.
>
> Hmm weird that something like that would not produce errors?
>
> > Is there any omap_ctrl_write?() call with an offset > 0x32c into
> > scm_conf?
>
> Indeed, that's where things go wrong. Adding Tero to Cc, something
> is wrong there.
Something like this might fix it? Needs to be tested to see if it
happens on other omaps.
Tero, got any better ideas?
Regards,
Tony
8< -------------------
--- a/arch/arm/mach-omap2/control.c
+++ b/arch/arm/mach-omap2/control.c
@@ -167,15 +167,23 @@ u16 omap_ctrl_readw(u16 offset)
u32 omap_ctrl_readl(u16 offset)
{
u32 val;
+ int err;
offset &= 0xfffc;
- if (!omap2_ctrl_syscon)
- val = readl_relaxed(omap2_ctrl_base + offset);
- else
- regmap_read(omap2_ctrl_syscon, omap2_ctrl_offset + offset,
- &val);
+ if (omap2_ctrl_syscon) {
+ err = regmap_read(omap2_ctrl_syscon,
+ omap2_ctrl_offset + offset, &val);
+ if (!err)
+ return val;
+ }
+
+ if (WARN(!omap2_ctrl_base,
+ "syscon out of range for offset 0x%x, no omap2_ctrl_base?\n",
+ offset)) {
+ return 0;
+ }
- return val;
+ return readl_relaxed(omap2_ctrl_base + offset);
}
void omap_ctrl_writeb(u8 val, u16 offset)
@@ -206,12 +214,23 @@ void omap_ctrl_writew(u16 val, u16 offset)
void omap_ctrl_writel(u32 val, u16 offset)
{
+ int err;
+
offset &= 0xfffc;
- if (!omap2_ctrl_syscon)
- writel_relaxed(val, omap2_ctrl_base + offset);
- else
- regmap_write(omap2_ctrl_syscon, omap2_ctrl_offset + offset,
- val);
+ if (omap2_ctrl_syscon) {
+ err = regmap_write(omap2_ctrl_syscon,
+ omap2_ctrl_offset + offset, val);
+ if (!err)
+ return;
+ }
+
+ if (WARN(!omap2_ctrl_base,
+ "syscon out of range for offset 0x%x, no omap2_ctrl_base?\n",
+ offset)) {
+ return;
+ }
+
+ writel_relaxed(val, omap2_ctrl_base + offset);
}
#ifdef CONFIG_ARCH_OMAP3
@@ -724,9 +743,6 @@ int __init omap_control_init(void)
if (ret)
return ret;
}
-
- iounmap(omap2_ctrl_base);
- omap2_ctrl_base = NULL;
} else {
/* No scm_conf found, direct access */
ret = omap2_clk_provider_init(np, data->index, NULL,
next prev parent reply other threads:[~2016-02-15 18:06 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-02-13 0:26 GPIO regression in Linux next caused by syscon change Tony Lindgren
2016-02-13 0:26 ` Tony Lindgren
2016-02-13 0:49 ` Tony Lindgren
2016-02-13 0:49 ` Tony Lindgren
2016-02-14 19:22 ` Philipp Zabel
2016-02-14 19:22 ` Philipp Zabel
2016-02-15 16:01 ` Tony Lindgren
2016-02-15 16:01 ` Tony Lindgren
2016-02-15 16:16 ` Philipp Zabel
2016-02-15 16:16 ` Philipp Zabel
2016-02-15 16:47 ` Tony Lindgren
2016-02-15 16:47 ` Tony Lindgren
2016-02-15 18:06 ` Tony Lindgren [this message]
2016-02-15 18:06 ` Tony Lindgren
2016-02-20 10:59 ` Tero Kristo
2016-02-20 10:59 ` Tero Kristo
2016-02-22 16:35 ` Tony Lindgren
2016-02-22 16:35 ` Tony Lindgren
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160215180635.GW19432@atomide.com \
--to=tony@atomide.com \
--cc=arnd@arndb.de \
--cc=lee.jones@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-omap@vger.kernel.org \
--cc=p.zabel@pengutronix.de \
--cc=philipp.zabel@gmail.com \
--cc=t-kristo@ti.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.