Discussion:
[PATCH 1/2] ste: Add support for multiple pdp contexts.
Marit Henriksen
2011-02-08 11:17:26 UTC
Permalink
From: Marit Henriksen <***@stericsson.com>

---
plugins/ste.c | 17 ++++++++++++-----
1 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/plugins/ste.c b/plugins/ste.c
index cf8aed8..749c4f3 100644
--- a/plugins/ste.c
+++ b/plugins/ste.c
@@ -66,6 +66,7 @@
#include <drivers/stemodem/if_caif.h>

#define NUM_CHAT 1
+#define MAX_PDP_CONTEXTS 4

static const char *cpin_prefix[] = { "+CPIN:", NULL };

@@ -363,6 +364,7 @@ static void ste_post_online(struct ofono_modem *modem)
struct ofono_message_waiting *mw;
struct ofono_gprs *gprs;
struct ofono_gprs_context *gc;
+ int i;

DBG("%p", modem);

@@ -378,13 +380,18 @@ static void ste_post_online(struct ofono_modem *modem)

gprs = ofono_gprs_create(modem, OFONO_VENDOR_MBM,
"atmodem", data->chat);
- gc = ofono_gprs_context_create(modem, 0, "stemodem", data->chat);
-
- if (gprs && gc)
- ofono_gprs_add_context(gprs, gc);
+ if (gprs) {
+ for (i = 0; i < MAX_PDP_CONTEXTS; i++) {
+ gc = ofono_gprs_context_create(
+ modem, 0, "stemodem", data->chat);
+ if (gc == NULL)
+ break;
+
+ ofono_gprs_add_context(gprs, gc);
+ }
+ }

mw = ofono_message_waiting_create(modem);
-
if (mw)
ofono_message_waiting_register(mw);
}
--
1.7.1
Marcel Holtmann
2011-02-08 15:42:01 UTC
Permalink
Hi Marit,
Post by Marit Henriksen
diff --git a/plugins/ste.c b/plugins/ste.c
index cf8aed8..749c4f3 100644
--- a/plugins/ste.c
+++ b/plugins/ste.c
@@ -66,6 +66,7 @@
#include <drivers/stemodem/if_caif.h>
#define NUM_CHAT 1
+#define MAX_PDP_CONTEXTS 4
static const char *cpin_prefix[] = { "+CPIN:", NULL };
@@ -363,6 +364,7 @@ static void ste_post_online(struct ofono_modem *modem)
struct ofono_message_waiting *mw;
struct ofono_gprs *gprs;
struct ofono_gprs_context *gc;
+ int i;
DBG("%p", modem);
@@ -378,13 +380,18 @@ static void ste_post_online(struct ofono_modem *modem)
gprs = ofono_gprs_create(modem, OFONO_VENDOR_MBM,
"atmodem", data->chat);
- gc = ofono_gprs_context_create(modem, 0, "stemodem", data->chat);
-
- if (gprs && gc)
- ofono_gprs_add_context(gprs, gc);
+ if (gprs) {
+ for (i = 0; i < MAX_PDP_CONTEXTS; i++) {
+ gc = ofono_gprs_context_create(
+ modem, 0, "stemodem", data->chat);
+ if (gc == NULL)
+ break;
+
+ ofono_gprs_add_context(gprs, gc);
+ }
+ }
you do not need to create the GPRS context atom multiple times. You can
just add the gc multiple times.

So I just wanna make sure that you guys wanna have multiple GPRS context
atom instances. For things like PPP and RawIP we have no other choice,
but for example ISI is a bit more flexible here. That is the reason why
we do allow it. Maybe CAIF is as flexible.

It is a bit question about resources that are used. I am fine either
way, but I need you to at least think about it ;)

Regards

Marcel
Marit Sofie Henriksen
2011-02-09 08:37:30 UTC
Permalink
Post by Marcel Holtmann
Hi Marit,
Post by Marit Henriksen
diff --git a/plugins/ste.c b/plugins/ste.c
index cf8aed8..749c4f3 100644
--- a/plugins/ste.c
+++ b/plugins/ste.c
@@ -66,6 +66,7 @@
#include <drivers/stemodem/if_caif.h>
#define NUM_CHAT 1
+#define MAX_PDP_CONTEXTS 4
static const char *cpin_prefix[] = { "+CPIN:", NULL };
@@ -363,6 +364,7 @@ static void ste_post_online(struct ofono_modem
*modem)
Post by Marit Henriksen
struct ofono_message_waiting *mw;
struct ofono_gprs *gprs;
struct ofono_gprs_context *gc;
+ int i;
DBG("%p", modem);
@@ -378,13 +380,18 @@ static void ste_post_online(struct ofono_modem
*modem)
Post by Marit Henriksen
gprs = ofono_gprs_create(modem, OFONO_VENDOR_MBM,
"atmodem", data->chat);
- gc = ofono_gprs_context_create(modem, 0, "stemodem", data->chat);
-
- if (gprs && gc)
- ofono_gprs_add_context(gprs, gc);
+ if (gprs) {
+ for (i = 0; i < MAX_PDP_CONTEXTS; i++) {
+ gc = ofono_gprs_context_create(
+ modem, 0, "stemodem", data->chat);
+ if (gc == NULL)
+ break;
+
+ ofono_gprs_add_context(gprs, gc);
+ }
+ }
you do not need to create the GPRS context atom multiple times. You can
just add the gc multiple times.
So I just wanna make sure that you guys wanna have multiple GPRS context
atom instances. For things like PPP and RawIP we have no other choice,
but for example ISI is a bit more flexible here. That is the reason why
we do allow it. Maybe CAIF is as flexible.
It is a bit question about resources that are used. I am fine either
way, but I need you to at least think about it ;)
Regards
Marcel
Hi Marcel,
Thanks for your input. We have given this some more thought and discussed
amongst us, and we would like to go for the solution as it is, adding
multiple GPRS context atom instances. I hope this is OK with you.

In the driver our internal gprs_context_data is associated with the
ofono_gprs_context. If we were to have only one gc, we would have to handle
all the activations with the same gprs_context_data, and that would bring us
back to where we were, with a significantly more complex implementation.

Br Marit
Post by Marcel Holtmann
_______________________________________________
ofono mailing list
http://lists.ofono.org/listinfo/ofono
Marit Sofie Henriksen
2011-02-18 08:29:13 UTC
Permalink
Post by Marcel Holtmann
Hi Marit,
Post by Marit Henriksen
Post by Marit Henriksen
diff --git a/plugins/ste.c b/plugins/ste.c
index cf8aed8..749c4f3 100644
--- a/plugins/ste.c
+++ b/plugins/ste.c
@@ -66,6 +66,7 @@
#include <drivers/stemodem/if_caif.h>
#define NUM_CHAT 1
+#define MAX_PDP_CONTEXTS 4
static const char *cpin_prefix[] = { "+CPIN:", NULL };
@@ -363,6 +364,7 @@ static void ste_post_online(struct ofono_modem
*modem)
Post by Marit Henriksen
struct ofono_message_waiting *mw;
struct ofono_gprs *gprs;
struct ofono_gprs_context *gc;
+ int i;
DBG("%p", modem);
@@ -378,13 +380,18 @@ static void ste_post_online(struct ofono_modem
*modem)
Post by Marit Henriksen
gprs = ofono_gprs_create(modem, OFONO_VENDOR_MBM,
"atmodem", data->chat);
- gc = ofono_gprs_context_create(modem, 0, "stemodem", data->chat);
-
- if (gprs && gc)
- ofono_gprs_add_context(gprs, gc);
+ if (gprs) {
+ for (i = 0; i < MAX_PDP_CONTEXTS; i++) {
+ gc = ofono_gprs_context_create(
+ modem, 0, "stemodem", data->chat);
+ if (gc == NULL)
+ break;
+
+ ofono_gprs_add_context(gprs, gc);
+ }
+ }
you do not need to create the GPRS context atom multiple times. You can
just add the gc multiple times.
So I just wanna make sure that you guys wanna have multiple GPRS context
atom instances. For things like PPP and RawIP we have no other choice,
but for example ISI is a bit more flexible here. That is the reason why
we do allow it. Maybe CAIF is as flexible.
It is a bit question about resources that are used. I am fine either
way, but I need you to at least think about it ;)
Regards
Marcel
Hi Marcel,
Thanks for your input. We have given this some more thought and discussed
amongst us, and we would like to go for the solution as it is, adding
multiple GPRS context atom instances. I hope this is OK with you.
In the driver our internal gprs_context_data is associated with the
ofono_gprs_context. If we were to have only one gc, we would have to handle
all the activations with the same gprs_context_data, and that would bring us
back to where we were, with a significantly more complex implementation.
Br Marit
Hi.
Is this approach ok, and has anyone had the chance to look at these pathces?
(this one plus [PATCH 2/2] stemodem: Add support for multiple pdp contexts)

br Marit
Post by Marcel Holtmann
Post by Marit Henriksen
_______________________________________________
ofono mailing list
http://lists.ofono.org/listinfo/ofono
Marcel Holtmann
2011-02-18 18:01:16 UTC
Permalink
Hi Marit,
Post by Marit Sofie Henriksen
Is this approach ok, and has anyone had the chance to look at these
pathces? (this one plus [PATCH 2/2] stemodem: Add support for multiple
pdp contexts)
it is fine with me. Just re-send them with the warning fixed.

Regards

Marcel

Loading...