diff mbox

AM335x board with disabled RTC crashes

Message ID 20131121053006.GA19129@kahuna (mailing list archive)
State New, archived
Headers show

Commit Message

Nishanth Menon Nov. 21, 2013, 5:30 a.m. UTC 
On 05:32-20131121, Stefan Roese wrote:
> On 11/21/2013 05:28 AM, Stefan Roese wrote:
> >>>> Disabling the RTC in the DT doesn't help (of course). So what's the
> >>>> recommended way to disable this hwmod access to the RTC registers?
> >>>
> >>>
> >>> http://www.spinics.net/lists/linux-omap/msg98207.html
> >>> might help you - we had thought it might get queued for 3.12, but it
> >>> was queued for 3.13 instead..
> > 
> > Unfortunately this patch doesn't fix the crash for me (yet). With this
> > patch applied and the RTC DT node completely removed from am33xx.dtsi it
> > works. But only disabling it (via status = "disabled" in the board dts
> > file) doesn't work.
> > 
> > I'll dig a bit deeper today...
> 
> This (hacky) patch works, but I'm not sure if this is acceptable upstream:
> 
> am335x-board_foo.dts:
> 
> ...
> 
> &rtc {
> 	reg = <0x0 0x0>;
> };
> 
You should be able to achieve the same effect as following (example from
BBB) - though I dont see this defined in
Documentation/devicetree/bindings/arm/omap/omap.txt

I will allow the wisdom of others to comment better here :)

Comments

Stefan Roese Nov. 21, 2013, 5:42 a.m. UTC | #1 
Thanks Nishanth!

On 11/21/2013 06:30 AM, Nishanth Menon wrote:
>> This (hacky) patch works, but I'm not sure if this is acceptable upstream:
>>
>> am335x-board_foo.dts:
>>
>> ...
>>
>> &rtc {
>> 	reg = <0x0 0x0>;
>> };
>>
> You should be able to achieve the same effect as following (example from
> BBB) - though I dont see this defined in
> Documentation/devicetree/bindings/arm/omap/omap.txt
> 
> I will allow the wisdom of others to comment better here :)
> diff --git a/arch/arm/boot/dts/am335x-boneblack.dts b/arch/arm/boot/dts/am335x-boneblack.dts
> index 6b71ad9..a734ef4 100644
> --- a/arch/arm/boot/dts/am335x-boneblack.dts
> +++ b/arch/arm/boot/dts/am335x-boneblack.dts
> @@ -66,6 +66,11 @@
>  	status = "okay";
>  };
>  
> +&am335xrtc {
> +	status = "disabled";
> +	ti,hwmods="disabled";
> +};

Yes, this works too. Thanks.

Which leaves only the quite ugly WARN() resulting from Suman's patch:

[    0.230270] omap_hwmod: rtc: Could not ioremap
[    0.234962] ------------[ cut here ]------------
[    0.239936] WARNING: CPU: 0 PID: 1 at arch/arm/mach-omap2/omap_hwmod.c:2434 _init+0x144/0x310()
[    0.249054] omap_hwmod: rtc: doesn't have mpu register target base
[    0.255526] Modules linked in:
[    0.258845] CPU: 0 PID: 1 Comm: swapper Not tainted 3.12.0-00004-gfcb6c2c-dirty #31
[    0.266938] [<c00197f4>] (unwind_backtrace+0x0/0xf0) from [<c0016ee8>] (show_stack+0x10/0x14)
[    0.275916] [<c0016ee8>] (show_stack+0x10/0x14) from [<c003be68>] (warn_slowpath_common+0x6c/0x8c)
[    0.285284] [<c003be68>] (warn_slowpath_common+0x6c/0x8c) from [<c003bf1c>] (warn_slowpath_fmt+0x30/0x40)
[    0.295323] [<c003bf1c>] (warn_slowpath_fmt+0x30/0x40) from [<c06fdac8>] (_init+0x144/0x310)
[    0.304200] [<c06fdac8>] (_init+0x144/0x310) from [<c0029e40>] (omap_hwmod_for_each+0x34/0x5c)
[    0.313250] [<c0029e40>] (omap_hwmod_for_each+0x34/0x5c) from [<c06fe1fc>] (__omap_hwmod_setup_all+0x24/0x40)
[    0.323643] [<c06fe1fc>] (__omap_hwmod_setup_all+0x24/0x40) from [<c00087e8>] (do_one_initcall+0x34/0x160)
[    0.333774] [<c00087e8>] (do_one_initcall+0x34/0x160) from [<c06f3af0>] (kernel_init_freeable+0xe8/0x1b4)
[    0.343824] [<c06f3af0>] (kernel_init_freeable+0xe8/0x1b4) from [<c04d0ff4>] (kernel_init+0x8/0xe4)
[    0.353338] [<c04d0ff4>] (kernel_init+0x8/0xe4) from [<c00139e8>] (ret_from_fork+0x14/0x2c)
[    0.362428] ---[ end trace 1b75b31a2719ed1c ]---

Do we really need this WARN() here? Or is the "Could not ioremap" line
enough?

Thanks,
Stefan

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Nishanth Menon Nov. 21, 2013, 5:52 a.m. UTC | #2 
On 11/20/2013 11:42 PM, Stefan Roese wrote:
> Thanks Nishanth!
> 
> On 11/21/2013 06:30 AM, Nishanth Menon wrote:
>>> This (hacky) patch works, but I'm not sure if this is acceptable upstream:
>>>
>>> am335x-board_foo.dts:
>>>
>>> ...
>>>
>>> &rtc {
>>> 	reg = <0x0 0x0>;
>>> };
>>>
>> You should be able to achieve the same effect as following (example from
>> BBB) - though I dont see this defined in
>> Documentation/devicetree/bindings/arm/omap/omap.txt
>>
>> I will allow the wisdom of others to comment better here :)
>> diff --git a/arch/arm/boot/dts/am335x-boneblack.dts b/arch/arm/boot/dts/am335x-boneblack.dts
>> index 6b71ad9..a734ef4 100644
>> --- a/arch/arm/boot/dts/am335x-boneblack.dts
>> +++ b/arch/arm/boot/dts/am335x-boneblack.dts
>> @@ -66,6 +66,11 @@
>>  	status = "okay";
>>  };
>>  
>> +&am335xrtc {
>> +	status = "disabled";
>> +	ti,hwmods="disabled";
>> +};
> 
> Yes, this works too. Thanks.
> 
> Which leaves only the quite ugly WARN() resulting from Suman's patch:
> 
> [    0.230270] omap_hwmod: rtc: Could not ioremap
> [    0.234962] ------------[ cut here ]------------
> [    0.239936] WARNING: CPU: 0 PID: 1 at arch/arm/mach-omap2/omap_hwmod.c:2434 _init+0x144/0x310()
> [    0.249054] omap_hwmod: rtc: doesn't have mpu register target base
> [    0.255526] Modules linked in:
> [    0.258845] CPU: 0 PID: 1 Comm: swapper Not tainted 3.12.0-00004-gfcb6c2c-dirty #31
> [    0.266938] [<c00197f4>] (unwind_backtrace+0x0/0xf0) from [<c0016ee8>] (show_stack+0x10/0x14)
> [    0.275916] [<c0016ee8>] (show_stack+0x10/0x14) from [<c003be68>] (warn_slowpath_common+0x6c/0x8c)
> [    0.285284] [<c003be68>] (warn_slowpath_common+0x6c/0x8c) from [<c003bf1c>] (warn_slowpath_fmt+0x30/0x40)
> [    0.295323] [<c003bf1c>] (warn_slowpath_fmt+0x30/0x40) from [<c06fdac8>] (_init+0x144/0x310)
> [    0.304200] [<c06fdac8>] (_init+0x144/0x310) from [<c0029e40>] (omap_hwmod_for_each+0x34/0x5c)
> [    0.313250] [<c0029e40>] (omap_hwmod_for_each+0x34/0x5c) from [<c06fe1fc>] (__omap_hwmod_setup_all+0x24/0x40)
> [    0.323643] [<c06fe1fc>] (__omap_hwmod_setup_all+0x24/0x40) from [<c00087e8>] (do_one_initcall+0x34/0x160)
> [    0.333774] [<c00087e8>] (do_one_initcall+0x34/0x160) from [<c06f3af0>] (kernel_init_freeable+0xe8/0x1b4)
> [    0.343824] [<c06f3af0>] (kernel_init_freeable+0xe8/0x1b4) from [<c04d0ff4>] (kernel_init+0x8/0xe4)
> [    0.353338] [<c04d0ff4>] (kernel_init+0x8/0xe4) from [<c00139e8>] (ret_from_fork+0x14/0x2c)
> [    0.362428] ---[ end trace 1b75b31a2719ed1c ]---
> 
> Do we really need this WARN() here? Or is the "Could not ioremap" line
> enough?

I had seen this too :) - There is a bit of history around this:
- we are in the process of moving functionality from hwmod into dts
since both are meant to describe the hardware and dt is the way to go.
	- at this point - hwmod is supposed to accurately represent SoC
modules and dts is supposed to match up.
- as part of this conversion, one of the first things to move is to
memory range description from hwmod to dts (reg description).
- the WARN() was introduced to catch such instances where hwmod
contain nodes and dts entries dont contain the description.

What we did miss is a valid condition where the generic definitions
for the presence of a SoC peripheral depends on a pin pull on the
board for the same SoC.

What should happen: if maintainers think that 'ti,hwmods="disabled";'
is an sufficient way of describing this current scenario, then we can
intelligently parse the same and avoid the warning for these cases.
diff mbox

Patch

diff --git a/arch/arm/boot/dts/am335x-boneblack.dts b/arch/arm/boot/dts/am335x-boneblack.dts
index 6b71ad9..a734ef4 100644
--- a/arch/arm/boot/dts/am335x-boneblack.dts
+++ b/arch/arm/boot/dts/am335x-boneblack.dts
@@ -66,6 +66,11 @@ 
 	status = "okay";
 };
 
+&am335xrtc {
+	status = "disabled";
+	ti,hwmods="disabled";
+};
+
 / {
 	hdmi {
 		compatible = "ti,tilcdc,slave";
diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
index f6d8ffe..71bf2ec 100644
--- a/arch/arm/boot/dts/am33xx.dtsi
+++ b/arch/arm/boot/dts/am33xx.dtsi
@@ -373,7 +373,7 @@ 
 			ti,timer-pwm;
 		};
 
-		rtc@44e3e000 {
+		am335xrtc: rtc@44e3e000 {
 			compatible = "ti,da830-rtc";
 			reg = <0x44e3e000 0x1000>;
 			interrupts = <75