30 Apr
2011
30 Apr
'11
9:53 a.m.
Hi,
good news: mails are back
bad news: phone lost a battery pin :(
Here are two small patches to help the mediatek-branch compile without calypso
references. These were tested before the breakage and 'make all' still builds
fine for compal.
Regards,
Wolfram
Wolfram Sang (2):
comm: msgb: don't set backlight on error
lib: move delay.c from calypso to lib
src/target/firmware/calypso/Makefile | 2 +-
src/target/firmware/calypso/delay.c | 16 ----------------
src/target/firmware/comm/msgb.c | 1 -
src/target/firmware/lib/Makefile | 2 +-
src/target/firmware/lib/delay.c | 16 ++++++++++++++++
5 files changed, 18 insertions(+), 19 deletions(-)
delete mode 100644 src/target/firmware/calypso/delay.c
create mode 100644 src/target/firmware/lib/delay.c
--
1.7.2.5
30 Apr
30 Apr
9:53 a.m.
New subject: [PATCH 1/2] comm: msgb: don't set backlight on error
It seems just to be a debugging aid, but brings in an unwanted
calypso-dependency.
Signed-off-by: Wolfram Sang <wolfram(a)the-dreams.de>
Cc: Harald Welte <laforge(a)gnumonks.org>
---
src/target/firmware/comm/msgb.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/src/target/firmware/comm/msgb.c b/src/target/firmware/comm/msgb.c
index ff18e65..792abc6 100644
--- a/src/target/firmware/comm/msgb.c
+++ b/src/target/firmware/comm/msgb.c
@@ -60,7 +60,6 @@ void *_talloc_zero(void *ctx, unsigned int size, const char *name)
}
}
cons_puts("unable to allocate msgb\n");
- bl_level(++i % 50);
delay_ms(50);
}
panic:
--
1.7.2.5
10:12 a.m.
New subject: [PATCH 1/2] comm: msgb: don't set backlight on error
Hi,
On Sat, Apr 30, 2011 at 9:53 AM, Wolfram Sang <wolfram(a)the-dreams.de> wrote:
It seems just to be a debugging aid, but brings in an
unwanted
calypso-dependency.
Well it is :)
I suggest you just replace it by a 'board_panic' call or something and
leave it to the board specific code how to signal panic conditions.
Cheers,
Sylvain
1 May
1 May
10:18 a.m.
New subject: [PATCH 1/2] comm: msgb: don't set backlight on error
On 30/04/11 10:12, Sylvain Munaut wrote:
Hi,
On Sat, Apr 30, 2011 at 9:53 AM, Wolfram Sang<wolfram(a)the-dreams.de> wrote:
I had another look and still have the impression that backlight usage
should just go. Look at the implementation when the file was introduced:
+panic:
+ while (1) {
+ bl_level(++i % 50);
+ delay_ms(50);
+ }
+ return NULL;
This, I understand, it blinks. But it was changed three days later to
the current state (c917fd43797c30385a1ba860fef95be97c84d51d) which broke
it ('i' is constant now). So it is broken for over a year and nobody
noticed :) So, instead of introducing a board_panic which nobody seems
to need, I'd just suggest using plain osmo_panic as in other places.
Much cleaner, too, IMO.
Regards,
Wolfram
It seems just to be a debugging aid, but brings
in an unwanted
calypso-dependency.
Well it is :)
I suggest you just replace it by a 'board_panic' call or something and
leave it to the board specific code how to signal panic conditions.
10:36 a.m.
New subject: [PATCH 1/2] comm: msgb: don't set backlight on error
noticed :) So, instead of introducing a board_panic
which nobody seems
to need, I'd just suggest using plain osmo_panic as in other places.
Much cleaner, too, IMO.
Ehrm, considering the while(1)-loop in the current form, this isn't even
a panic? So just print the msg as is now? Confused...
2 May
2 May
9:15 a.m.
New subject: [PATCH 1/2] comm: msgb: don't set backlight on error
Hi Wolfgang,
On Sun, May 01, 2011 at 10:18:04AM +0200, Wolfram Sang wrote:
This, I understand, it blinks. But it was changed
three days later
to the current state (c917fd43797c30385a1ba860fef95be97c84d51d)
which broke it ('i' is constant now). So it is broken for over a
year and nobody noticed :)
Well, let's say we noticed that we get layer1 panics without any indication.
So, instead of introducing a board_panic which nobody
seems to need, I'd just
suggest using plain osmo_panic as in other places. Much cleaner, too, IMO.
I disagree. We desperately need board_panic, (with much more than just
backlight blinking in the future), to be able to solve the various mysterious
layer1/firmware crashes that we're seeing more or less all the time.
I had recently discussed this with Andreas and Sylvain at the Easterhegg,
and they both agreed that we need a way to debug crashes better. We need
to tap into the various exception handlers, we need to use the debug unit
to get backtraces, and we need ways like backlight blinking or buzzer sounds
that can be done from a few lines of stack-independent assembly code to
reliably indicate what has happened to the system.
Regards,
Harald
--
- Harald Welte <laforge(a)gnumonks.org> http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
(ETSI EN 300 175-7 Ch. A6)
2:02 p.m.
New subject: [PATCH 1/2] comm: msgb: don't set backlight on error
Hi,
I disagree. We desperately need board_panic, (with
much more than just
backlight blinking in the future), to be able to solve the various mysterious
layer1/firmware crashes that we're seeing more or less all the time.
Yes, it definitely needed.
And calling osmo_panic might be the right thing, and then have the
board register their osmo_panic handler in its init (the buzzer would
be a good choice :)
(libosmocore already has a way to register a panic handler IIRC which
is for the target a while(1) loop by default)
Cheers,
Sylvain
2:27 p.m.
New subject: [PATCH 1/2] comm: msgb: don't set backlight on error
On 02/05/11 14:02, Sylvain Munaut wrote:
Okay, understood that it is needed. Still, the discussion also showed that
I seem to not have the proper knowledge of what is desired instead by
those people hacking at layer 1 (what I don't do) :) Thus, may I suggest
that I just remove the (static value) backlight setting in msgb.c to
remove the calypso-dependency and leave a proper board_panic for someone
else with a seperate patchset? To me, Harald's post indicates that it is a
seperate topic?
Regards,
Wolfram
I disagree.
We desperately need board_panic, (with much more than just
backlight blinking in the future), to be able to solve the various mysterious
layer1/firmware crashes that we're seeing more or less all the time.
Yes, it definitely needed.
And calling osmo_panic might be the right thing, and then have the
board register their osmo_panic handler in its init (the buzzer would
be a good choice :)
10:57 p.m.
New subject: [PATCH 1/2] comm: msgb: don't set backlight on error
On Mon, May 02, 2011 at 02:27:03PM +0200, Wolfram Sang wrote:
Okay, understood that it is needed. Still, the
discussion also showed that
I seem to not have the proper knowledge of what is desired instead by
those people hacking at layer 1 (what I don't do) :) Thus, may I suggest
that I just remove the (static value) backlight setting in msgb.c to
remove the calypso-dependency and leave a proper board_panic for someone
else with a seperate patchset? To me, Harald's post indicates that it is a
seperate topic?
ACK.
--
- Harald Welte <laforge(a)gnumonks.org> http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
(ETSI EN 300 175-7 Ch. A6)
30 Apr
30 Apr
9:53 a.m.
New subject: [PATCH 2/2] lib: move delay.c from calypso to lib
Nothing calypso-related in there and needed for Mediatek, too.
Signed-off-by: Wolfram Sang <wolfram(a)the-dreams.de>
---
src/target/firmware/calypso/Makefile | 2 +-
src/target/firmware/calypso/delay.c | 16 ----------------
src/target/firmware/lib/Makefile | 2 +-
src/target/firmware/lib/delay.c | 16 ++++++++++++++++
4 files changed, 18 insertions(+), 18 deletions(-)
delete mode 100644 src/target/firmware/calypso/delay.c
create mode 100644 src/target/firmware/lib/delay.c
diff --git a/src/target/firmware/calypso/Makefile b/src/target/firmware/calypso/Makefile
index c0620ed..610a82c 100644
--- a/src/target/firmware/calypso/Makefile
+++ b/src/target/firmware/calypso/Makefile
@@ -1,4 +1,4 @@
LIBRARIES+=calypso
calypso_DIR=calypso
-calypso_SRCS=arm.c buzzer.c clock.c delay.c dma.c dsp.c du.c i2c.c irq.c rtc.c sim.c
spi.c tpu.c tsp.c keypad.c misc.c timer.c backlight.c uart.c uwire.c
+calypso_SRCS=arm.c buzzer.c clock.c dma.c dsp.c du.c i2c.c irq.c rtc.c sim.c spi.c tpu.c
tsp.c keypad.c misc.c timer.c backlight.c uart.c uwire.c
diff --git a/src/target/firmware/calypso/delay.c b/src/target/firmware/calypso/delay.c
deleted file mode 100644
index 443ca82..0000000
--- a/src/target/firmware/calypso/delay.c
+++ /dev/null
@@ -1,16 +0,0 @@
-#include <delay.h>
-
-/* FIXME: We need properly calibrated delay loops at some point! */
-void delay_us(unsigned int us)
-{
- volatile unsigned int i;
-
- for (i= 0; i < us*4; i++) { i; }
-}
-
-void delay_ms(unsigned int ms)
-{
- volatile unsigned int i;
-
- for (i= 0; i < ms*1300; i++) { i; }
-}
diff --git a/src/target/firmware/lib/Makefile b/src/target/firmware/lib/Makefile
index 987857c..83f9966 100644
--- a/src/target/firmware/lib/Makefile
+++ b/src/target/firmware/lib/Makefile
@@ -2,6 +2,6 @@
LIBRARIES+=mini
mini_DIR=lib
mini_SRCS=vsprintf.c string.c ctype.c printf.c console.c ctors.c \
- changebit.S clearbit.S div64.S lib1funcs.S memcpy.S memset.S setbit.S testchangebit.S
testclearbit.S testsetbit.S
+ changebit.S clearbit.S delay.c div64.S lib1funcs.S memcpy.S memset.S setbit.S
testchangebit.S testclearbit.S testsetbit.S
diff --git a/src/target/firmware/lib/delay.c b/src/target/firmware/lib/delay.c
new file mode 100644
index 0000000..443ca82
--- /dev/null
+++ b/src/target/firmware/lib/delay.c
@@ -0,0 +1,16 @@
+#include <delay.h>
+
+/* FIXME: We need properly calibrated delay loops at some point! */
+void delay_us(unsigned int us)
+{
+ volatile unsigned int i;
+
+ for (i= 0; i < us*4; i++) { i; }
+}
+
+void delay_ms(unsigned int ms)
+{
+ volatile unsigned int i;
+
+ for (i= 0; i < ms*1300; i++) { i; }
+}
--
1.7.2.5
4752
days inactive
4752
days old
baseband-devel@lists.osmocom.org
9 comments
3 participants
participants (3)
-
Harald Welte -
Sylvain Munaut -
Wolfram Sang