Re: [PATCH 1/2] ste: Add support for multiple pdp contexts.

2011-02-18 Thread Marit Sofie Henriksen 
2011/2/9 Marit Sofie Henriksen maritsofie.henriks...@gmail.com



 2011/2/8 Marcel Holtmann mar...@holtmann.org

 Hi Marit,

  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


 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







 ___
 ofono mailing list
 ofono@ofono.org
 http://lists.ofono.org/listinfo/ofono



___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono

Re: [PATCH 1/2] ste: Add support for multiple pdp contexts.

2011-02-18 Thread Marcel Holtmann 
Hi Marit,

 
 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



___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono

Re: [PATCH 1/2] ste: Add support for multiple pdp contexts.

2011-02-09 Thread Marit Sofie Henriksen 
2011/2/8 Marcel Holtmann mar...@holtmann.org

 Hi Marit,

  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


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



 ___
 ofono mailing list
 ofono@ofono.org
 http://lists.ofono.org/listinfo/ofono

___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono

[PATCH 1/2] ste: Add support for multiple pdp contexts.

2011-02-08 Thread Marit Henriksen 
From: Marit Henriksen marit.henrik...@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

___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono

Re: [PATCH 1/2] ste: Add support for multiple pdp contexts.

2011-02-08 Thread Marcel Holtmann 
Hi Marit,

 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


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono