All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] SIM900 GPRS fixes
@ 2013-07-17  7:14 Jesper Larsen
  2013-07-17  7:14 ` [PATCH 1/3] sim900: Enable serial receiver Jesper Larsen
  2013-07-18  7:49 ` [PATCH v2 1/3] atmodem: Add vendor entry for SIM900 module Jesper Larsen
  0 siblings, 2 replies; 11+ messages in thread
From: Jesper Larsen @ 2013-07-17  7:14 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 587 bytes --]

I need patches 1 and 2 in order to have a functional ppp connection to the SIM900 module.
Renat Zaripov has on the mailing list also reported the need to use the ATD*99# command.

The 3rd patch is just a fix for debug output.

Jesper Larsen (3):
  sim900: Enable serial receiver
  atmodem: Add gprs-context quirk for SIM900
  sim900: Fix order of dlc prefixes

 drivers/atmodem/gprs-context.c |    8 ++++++--
 drivers/atmodem/vendor.h       |    1 +
 plugins/sim900.c               |   13 +++++++------
 3 files changed, 14 insertions(+), 8 deletions(-)

-- 
1.7.10.4


^ permalink raw reply	[flat|nested] 11+ messages in thread
* [PATCH 1/3] sim900: Enable serial receiver
  2013-07-17  7:14 [PATCH 0/3] SIM900 GPRS fixes Jesper Larsen
@ 2013-07-17  7:14 ` Jesper Larsen
  2013-07-17  7:14   ` [PATCH 2/3] atmodem: Add gprs-context quirk for SIM900 Jesper Larsen
  2013-07-17 15:29   ` [PATCH 1/3] sim900: Enable serial receiver Denis Kenzior
  2013-07-18  7:49 ` [PATCH v2 1/3] atmodem: Add vendor entry for SIM900 module Jesper Larsen
  1 sibling, 2 replies; 11+ messages in thread
From: Jesper Larsen @ 2013-07-17  7:14 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 670 bytes --]

The read option must be set to 'on' in order for two-way
communication with SIM900 module to work.
---
 plugins/sim900.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/plugins/sim900.c b/plugins/sim900.c
index 6cfed65..643de51 100644
--- a/plugins/sim900.c
+++ b/plugins/sim900.c
@@ -127,6 +127,7 @@ static GAtChat *open_device(struct ofono_modem *modem,
 	g_hash_table_insert(options, "XonXoff", "off");
 	g_hash_table_insert(options, "Local", "off");
 	g_hash_table_insert(options, "RtsCts", "off");
+	g_hash_table_insert(options, "Read", "on");
 
 	channel = g_at_tty_open(device, options);
 	g_hash_table_destroy(options);
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 11+ messages in thread
* [PATCH 2/3] atmodem: Add gprs-context quirk for SIM900
  2013-07-17  7:14 ` [PATCH 1/3] sim900: Enable serial receiver Jesper Larsen
@ 2013-07-17  7:14   ` Jesper Larsen
  2013-07-17  7:14     ` [PATCH 3/3] sim900: Fix order of dlc prefixes Jesper Larsen
  2013-07-17 15:33     ` [PATCH 2/3] atmodem: Add gprs-context quirk for SIM900 Denis Kenzior
  2013-07-17 15:29   ` [PATCH 1/3] sim900: Enable serial receiver Denis Kenzior
  1 sibling, 2 replies; 11+ messages in thread
From: Jesper Larsen @ 2013-07-17  7:14 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 2032 bytes --]

The SIM900 module from SIMCOM does have a AT+CGDATA command.
However, it is not possible to make a ppp connection when CGDATA
has been used to bring up the gprs context.

This patch ads a quirk that uses the alternative ATD*99***<cid>#
command instead.
---
 drivers/atmodem/gprs-context.c |    8 ++++++--
 drivers/atmodem/vendor.h       |    1 +
 plugins/sim900.c               |    2 +-
 3 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/atmodem/gprs-context.c b/drivers/atmodem/gprs-context.c
index 3694c27..f46d881 100644
--- a/drivers/atmodem/gprs-context.c
+++ b/drivers/atmodem/gprs-context.c
@@ -208,8 +208,12 @@ static void at_cgdcont_cb(gboolean ok, GAtResult *result, gpointer user_data)
 		return;
 	}
 
-	sprintf(buf, "AT+CGDATA=\"PPP\",%u", gcd->active_context);
-	if (g_at_chat_send(gcd->chat, buf, none_prefix,
+        if (gcd->vendor == OFONO_VENDOR_SIMCOM_SIM900)
+		sprintf(buf, "ATD*99***%u#", gcd->active_context);
+	else
+		sprintf(buf, "AT+CGDATA=\"PPP\",%u", gcd->active_context);
+
+        if (g_at_chat_send(gcd->chat, buf, none_prefix,
 				at_cgdata_cb, gc, NULL) > 0)
 		return;
 
diff --git a/drivers/atmodem/vendor.h b/drivers/atmodem/vendor.h
index 0532d10..2b1a567 100644
--- a/drivers/atmodem/vendor.h
+++ b/drivers/atmodem/vendor.h
@@ -39,6 +39,7 @@ enum ofono_vendor {
 	OFONO_VENDOR_SPEEDUP,
 	OFONO_VENDOR_SAMSUNG,
 	OFONO_VENDOR_SIMCOM,
+        OFONO_VENDOR_SIMCOM_SIM900,
 	OFONO_VENDOR_ICERA,
 	OFONO_VENDOR_WAVECOM_Q2XXX,
 	OFONO_VENDOR_ALCATEL
diff --git a/plugins/sim900.c b/plugins/sim900.c
index 643de51..3b4fd84 100644
--- a/plugins/sim900.c
+++ b/plugins/sim900.c
@@ -361,7 +361,7 @@ static void sim900_post_sim(struct ofono_modem *modem)
 	if (gprs == NULL)
 		return;
 
-	gc = ofono_gprs_context_create(modem, OFONO_VENDOR_SIMCOM,
+	gc = ofono_gprs_context_create(modem, OFONO_VENDOR_SIMCOM_SIM900,
 					"atmodem", data->dlcs[GPRS_DLC]);
 	if (gc)
 		ofono_gprs_add_context(gprs, gc);
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 11+ messages in thread
* [PATCH 3/3] sim900: Fix order of dlc prefixes
  2013-07-17  7:14   ` [PATCH 2/3] atmodem: Add gprs-context quirk for SIM900 Jesper Larsen
@ 2013-07-17  7:14     ` Jesper Larsen
  2013-07-17 15:29       ` Denis Kenzior
  2013-07-17 15:33     ` [PATCH 2/3] atmodem: Add gprs-context quirk for SIM900 Denis Kenzior
  1 sibling, 1 reply; 11+ messages in thread
From: Jesper Larsen @ 2013-07-17  7:14 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 749 bytes --]

The order of the defines for the dlc prefixes does not match
the order of the array containing the strings to print.
---
 plugins/sim900.c |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/plugins/sim900.c b/plugins/sim900.c
index 3b4fd84..a7728cd 100644
--- a/plugins/sim900.c
+++ b/plugins/sim900.c
@@ -49,11 +49,11 @@
 
 #define NUM_DLC 5
 
-#define SETUP_DLC   0
-#define VOICE_DLC   1
-#define NETREG_DLC  2
-#define SMS_DLC     3
-#define GPRS_DLC    4
+#define VOICE_DLC   0
+#define NETREG_DLC  1
+#define SMS_DLC     2
+#define GPRS_DLC    3
+#define SETUP_DLC   4
 
 static char *dlc_prefixes[NUM_DLC] = { "Voice: ", "Net: ", "SMS: ",
 					"GPRS: " , "Setup: "};
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] sim900: Enable serial receiver
  2013-07-17  7:14 ` [PATCH 1/3] sim900: Enable serial receiver Jesper Larsen
  2013-07-17  7:14   ` [PATCH 2/3] atmodem: Add gprs-context quirk for SIM900 Jesper Larsen
@ 2013-07-17 15:29   ` Denis Kenzior
  1 sibling, 0 replies; 11+ messages in thread
From: Denis Kenzior @ 2013-07-17 15:29 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 299 bytes --]

Hi Jesper,

On 07/17/2013 02:14 AM, Jesper Larsen wrote:
> The read option must be set to 'on' in order for two-way
> communication with SIM900 module to work.
> ---
>   plugins/sim900.c |    1 +
>   1 file changed, 1 insertion(+)
>

Patch has been applied, thanks.

Regards,
-Denis


^ permalink raw reply	[flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] sim900: Fix order of dlc prefixes
  2013-07-17  7:14     ` [PATCH 3/3] sim900: Fix order of dlc prefixes Jesper Larsen
@ 2013-07-17 15:29       ` Denis Kenzior
  0 siblings, 0 replies; 11+ messages in thread
From: Denis Kenzior @ 2013-07-17 15:29 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 343 bytes --]

Hi Jesper,

On 07/17/2013 02:14 AM, Jesper Larsen wrote:
> The order of the defines for the dlc prefixes does not match
> the order of the array containing the strings to print.
> ---
>   plugins/sim900.c |   10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)
>

Patch has been applied, thanks.

Regards,
-Denis


^ permalink raw reply	[flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] atmodem: Add gprs-context quirk for SIM900
  2013-07-17  7:14   ` [PATCH 2/3] atmodem: Add gprs-context quirk for SIM900 Jesper Larsen
  2013-07-17  7:14     ` [PATCH 3/3] sim900: Fix order of dlc prefixes Jesper Larsen
@ 2013-07-17 15:33     ` Denis Kenzior
  1 sibling, 0 replies; 11+ messages in thread
From: Denis Kenzior @ 2013-07-17 15:33 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 2707 bytes --]

Hi Jesper,

On 07/17/2013 02:14 AM, Jesper Larsen wrote:
> The SIM900 module from SIMCOM does have a AT+CGDATA command.
> However, it is not possible to make a ppp connection when CGDATA
> has been used to bring up the gprs context.
>
> This patch ads a quirk that uses the alternative ATD*99***<cid>#
> command instead.
> ---
>   drivers/atmodem/gprs-context.c |    8 ++++++--
>   drivers/atmodem/vendor.h       |    1 +

I'd like the vendor.h change in a separate commit

>   plugins/sim900.c               |    2 +-

The changes to files in different directories should be separated 
whenever possible.  See 'HACKING' document in the top level directory of 
the source tree, particularly the 'Submitting Patches' section.

So your commits should be:
vendor.h change
gprs-context.c change
sim900.c change

>   3 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/atmodem/gprs-context.c b/drivers/atmodem/gprs-context.c
> index 3694c27..f46d881 100644
> --- a/drivers/atmodem/gprs-context.c
> +++ b/drivers/atmodem/gprs-context.c
> @@ -208,8 +208,12 @@ static void at_cgdcont_cb(gboolean ok, GAtResult *result, gpointer user_data)
>   		return;
>   	}
>
> -	sprintf(buf, "AT+CGDATA=\"PPP\",%u", gcd->active_context);
> -	if (g_at_chat_send(gcd->chat, buf, none_prefix,
> +        if (gcd->vendor == OFONO_VENDOR_SIMCOM_SIM900)
> +		sprintf(buf, "ATD*99***%u#", gcd->active_context);
> +	else
> +		sprintf(buf, "AT+CGDATA=\"PPP\",%u", gcd->active_context);
> +
> +        if (g_at_chat_send(gcd->chat, buf, none_prefix,

You're mixing tabs and spaces for indentation all over here.  We only 
use tabs.

>   				at_cgdata_cb, gc, NULL) > 0)
>   		return;
>
> diff --git a/drivers/atmodem/vendor.h b/drivers/atmodem/vendor.h
> index 0532d10..2b1a567 100644
> --- a/drivers/atmodem/vendor.h
> +++ b/drivers/atmodem/vendor.h
> @@ -39,6 +39,7 @@ enum ofono_vendor {
>   	OFONO_VENDOR_SPEEDUP,
>   	OFONO_VENDOR_SAMSUNG,
>   	OFONO_VENDOR_SIMCOM,
> +        OFONO_VENDOR_SIMCOM_SIM900,

Again, tabs for indentation only please

>   	OFONO_VENDOR_ICERA,
>   	OFONO_VENDOR_WAVECOM_Q2XXX,
>   	OFONO_VENDOR_ALCATEL
> diff --git a/plugins/sim900.c b/plugins/sim900.c
> index 643de51..3b4fd84 100644
> --- a/plugins/sim900.c
> +++ b/plugins/sim900.c
> @@ -361,7 +361,7 @@ static void sim900_post_sim(struct ofono_modem *modem)
>   	if (gprs == NULL)
>   		return;
>
> -	gc = ofono_gprs_context_create(modem, OFONO_VENDOR_SIMCOM,
> +	gc = ofono_gprs_context_create(modem, OFONO_VENDOR_SIMCOM_SIM900,
>   					"atmodem", data->dlcs[GPRS_DLC]);
>   	if (gc)
>   		ofono_gprs_add_context(gprs, gc);
>

Regards,
-Denis

^ permalink raw reply	[flat|nested] 11+ messages in thread
* [PATCH v2 1/3] atmodem: Add vendor entry for SIM900 module
  2013-07-17  7:14 [PATCH 0/3] SIM900 GPRS fixes Jesper Larsen
  2013-07-17  7:14 ` [PATCH 1/3] sim900: Enable serial receiver Jesper Larsen
@ 2013-07-18  7:49 ` Jesper Larsen
  2013-07-18  7:49   ` [PATCH v2 2/3] atmodem: Add gprs-context quirk for SIM900 Jesper Larsen
  2013-07-18 19:13   ` [PATCH v2 1/3] atmodem: Add vendor entry for SIM900 module Denis Kenzior
  1 sibling, 2 replies; 11+ messages in thread
From: Jesper Larsen @ 2013-07-18  7:49 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 476 bytes --]

---
 drivers/atmodem/vendor.h |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/atmodem/vendor.h b/drivers/atmodem/vendor.h
index 0532d10..bf2b38a 100644
--- a/drivers/atmodem/vendor.h
+++ b/drivers/atmodem/vendor.h
@@ -39,6 +39,7 @@ enum ofono_vendor {
 	OFONO_VENDOR_SPEEDUP,
 	OFONO_VENDOR_SAMSUNG,
 	OFONO_VENDOR_SIMCOM,
+	OFONO_VENDOR_SIMCOM_SIM900,
 	OFONO_VENDOR_ICERA,
 	OFONO_VENDOR_WAVECOM_Q2XXX,
 	OFONO_VENDOR_ALCATEL
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 11+ messages in thread
* [PATCH v2 2/3] atmodem: Add gprs-context quirk for SIM900
  2013-07-18  7:49 ` [PATCH v2 1/3] atmodem: Add vendor entry for SIM900 module Jesper Larsen
@ 2013-07-18  7:49   ` Jesper Larsen
  2013-07-18  7:49     ` [PATCH v2 3/3] sim900: Use SIM900 quirk for gprs context Jesper Larsen
  2013-07-18 19:13   ` [PATCH v2 1/3] atmodem: Add vendor entry for SIM900 module Denis Kenzior
  1 sibling, 1 reply; 11+ messages in thread
From: Jesper Larsen @ 2013-07-18  7:49 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 1024 bytes --]

The SIM900 module from SIMCOM does have a AT+CGDATA command.
However, it is not possible to make a ppp connection when CGDATA
has been used to bring up the gprs context.

This patch adds a quirk that uses the alternative ATD*99***<cid>#
command instead.
---
 drivers/atmodem/gprs-context.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/atmodem/gprs-context.c b/drivers/atmodem/gprs-context.c
index 3694c27..2217097 100644
--- a/drivers/atmodem/gprs-context.c
+++ b/drivers/atmodem/gprs-context.c
@@ -208,7 +208,11 @@ static void at_cgdcont_cb(gboolean ok, GAtResult *result, gpointer user_data)
 		return;
 	}
 
-	sprintf(buf, "AT+CGDATA=\"PPP\",%u", gcd->active_context);
+	if (gcd->vendor == OFONO_VENDOR_SIMCOM_SIM900)
+		sprintf(buf, "ATD*99***%u#", gcd->active_context);
+	else
+		sprintf(buf, "AT+CGDATA=\"PPP\",%u", gcd->active_context);
+
 	if (g_at_chat_send(gcd->chat, buf, none_prefix,
 				at_cgdata_cb, gc, NULL) > 0)
 		return;
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 11+ messages in thread
* [PATCH v2 3/3] sim900: Use SIM900 quirk for gprs context
  2013-07-18  7:49   ` [PATCH v2 2/3] atmodem: Add gprs-context quirk for SIM900 Jesper Larsen
@ 2013-07-18  7:49     ` Jesper Larsen
  0 siblings, 0 replies; 11+ messages in thread
From: Jesper Larsen @ 2013-07-18  7:49 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 657 bytes --]

Use the module specific quirk for gprs by using the vendor entry
OFONO_VENDOR_SIMCOM_SIM900
---
 plugins/sim900.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/plugins/sim900.c b/plugins/sim900.c
index 5d3cd92..a7728cd 100644
--- a/plugins/sim900.c
+++ b/plugins/sim900.c
@@ -361,7 +361,7 @@ static void sim900_post_sim(struct ofono_modem *modem)
 	if (gprs == NULL)
 		return;
 
-	gc = ofono_gprs_context_create(modem, OFONO_VENDOR_SIMCOM,
+	gc = ofono_gprs_context_create(modem, OFONO_VENDOR_SIMCOM_SIM900,
 					"atmodem", data->dlcs[GPRS_DLC]);
 	if (gc)
 		ofono_gprs_add_context(gprs, gc);
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/3] atmodem: Add vendor entry for SIM900 module
  2013-07-18  7:49 ` [PATCH v2 1/3] atmodem: Add vendor entry for SIM900 module Jesper Larsen
  2013-07-18  7:49   ` [PATCH v2 2/3] atmodem: Add gprs-context quirk for SIM900 Jesper Larsen
@ 2013-07-18 19:13   ` Denis Kenzior
  1 sibling, 0 replies; 11+ messages in thread
From: Denis Kenzior @ 2013-07-18 19:13 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 207 bytes --]

Hi Jesper,

On 07/18/2013 02:49 AM, Jesper Larsen wrote:
> ---
>   drivers/atmodem/vendor.h |    1 +
>   1 file changed, 1 insertion(+)
>

All three patches have been applied.

Regards,
-Denis


^ permalink raw reply	[flat|nested] 11+ messages in thread
end of thread, other threads:[~2013-07-18 19:13 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-07-17  7:14 [PATCH 0/3] SIM900 GPRS fixes Jesper Larsen
2013-07-17  7:14 ` [PATCH 1/3] sim900: Enable serial receiver Jesper Larsen
2013-07-17  7:14   ` [PATCH 2/3] atmodem: Add gprs-context quirk for SIM900 Jesper Larsen
2013-07-17  7:14     ` [PATCH 3/3] sim900: Fix order of dlc prefixes Jesper Larsen
2013-07-17 15:29       ` Denis Kenzior
2013-07-17 15:33     ` [PATCH 2/3] atmodem: Add gprs-context quirk for SIM900 Denis Kenzior
2013-07-17 15:29   ` [PATCH 1/3] sim900: Enable serial receiver Denis Kenzior
2013-07-18  7:49 ` [PATCH v2 1/3] atmodem: Add vendor entry for SIM900 module Jesper Larsen
2013-07-18  7:49   ` [PATCH v2 2/3] atmodem: Add gprs-context quirk for SIM900 Jesper Larsen
2013-07-18  7:49     ` [PATCH v2 3/3] sim900: Use SIM900 quirk for gprs context Jesper Larsen
2013-07-18 19:13   ` [PATCH v2 1/3] atmodem: Add vendor entry for SIM900 module Denis Kenzior

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.