Re: [PATCH v3 1/2] huawei: detect SecondaryDevice which is used for events

Re: [PATCH v3 1/2] huawei: detect SecondaryDevice which is used for events

[Denis Kenzior]

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

Hi Kalle,

Please keep the commit header to less than 50 lines.

> +	device = ofono_modem_get_string(modem, "SecondaryDevice");
> +	if (!device)
> +		return -EINVAL;

Two tabs is enough here.

> +
> +	channel = g_at_tty_open(device, NULL);
> +	if (!channel)
> +		return -EIO;
> +
> +	syntax = g_at_syntax_new_gsm_permissive();
> +	data->event = g_at_chat_new(channel, syntax);
> +	g_at_syntax_unref(syntax);
> +	g_io_channel_unref(channel);
> +
> +	if (!data->event)
> +		return -EIO;
> +
> +	g_at_chat_add_terminator(data->event, "COMMAND NOT SUPPORT", -1,
> +				 FALSE);

Don't mix spaces and tabs for indentation

> +
> +	if (getenv("OFONO_AT_DEBUG"))
> +		g_at_chat_set_debug(data->event, huawei_event_debug, NULL);
> +
> +

Why the extra space?

Also, this entire sequence leaks the data->chat in case the 2nd port could not 
be opened.  See plugins/mbm.c for an example on how to do this better.

Regards,
-Denis

Re: [PATCH v3 2/2] huawei: add gprs context

[Denis Kenzior]

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

Hi Kalle,

> +	ret = at_util_parse_reg_unsolicited(result, "+CGREG:", &status,
> +					    NULL, NULL, NULL,
> +					    OFONO_VENDOR_HUAWEI);

No mixing of tabs & spaces for indentation

> +
> +	if (ret == FALSE)
> +		return;
> +
> +	ofono_gprs_status_notify(gprs, status);
> +}
> +
> +static inline void report_signal_strength(struct ofono_netreg *netreg,
> +					  int strength)

And here

> +{
> +	DBG("%d", strength);
> +
> +	if (strength == 99)
> +		strength = -1;
> +	else
> +		strength = (strength * 100) / 31;
> +
> +	ofono_netreg_strength_notify(netreg, strength);
> +}

You might really want to make this into an atutil function instead of copy-
pasting it around.

> +
> +static void huawei_rssi_notify(GAtResult *result, gpointer user_data)
> +{
> +	struct ofono_netreg *netreg = user_data;
> +	GAtResultIter iter;
> +	int strength;
> +
> +	g_at_result_iter_init(&iter, result);
> +
> +	if (!g_at_result_iter_next(&iter, "^RSSI:"))
> +		return;
> +
> +	if (!g_at_result_iter_next_number(&iter, &strength))
> +		return;
> +
> +	report_signal_strength(netreg, strength);
> +}
> +
>  static void huawei_post_sim(struct ofono_modem *modem)
>  {
>  	struct huawei_data *data = ofono_modem_get_data(modem);
> +	struct ofono_gprs_context *gc;
> +	struct ofono_netreg *netreg;
> +	struct ofono_gprs *gprs;
> 
>  	DBG("%p", modem);
> 
> -	ofono_netreg_create(modem, OFONO_VENDOR_HUAWEI, "atmodem", data->chat);
> +	netreg = ofono_netreg_create(modem, OFONO_VENDOR_HUAWEI, "atmodem",
> +				     data->chat);

Again with mixed indentation

>  	ofono_sms_create(modem, OFONO_VENDOR_QUALCOMM_MSM, "atmodem",
>  data->chat); +
> +	gprs = ofono_gprs_create(modem, OFONO_VENDOR_HUAWEI, "atmodem",
> +				 data->chat);

And here

> +	gc = ofono_gprs_context_create(modem, 0, "atmodem", data->chat);
> +
> +	if (gprs && gc) {
> +		ofono_gprs_add_context(gprs, gc);
> +
> +		/* huawei has a separate channel for CGREG notifications */
> +		g_at_chat_register(data->event, "+CGREG:",
> +				   huawei_cgreg_notify, FALSE, gprs, NULL);

And here

> +
> +		/* huawei uses non-standard "^RSSI:18" strings */
> +		g_at_chat_register(data->event, "^RSSI:",
> +				   huawei_rssi_notify, FALSE, netreg, NULL);

And here

> +	}
>  }
> 
>  static struct ofono_modem_driver huawei_driver = {

Regards,
-Denis

[PATCH v3 0/2] Huawei GPRS support

[Kalle Valo]

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

Here's v3 of my Huawei GPRS patches. I followed Marcel's advice and
implemented all Huawei quirks in the huawei plugin. Now the patches
are a lot smaller.

Please comment.

---

Kalle Valo (2):
      huawei: add gprs context
      huawei: detect SecondaryDevice which is used for events


 drivers/atmodem/network-registration.c |    4 +
 plugins/huawei.c                       |  112 +++++++++++++++++++++++++++++++-
 plugins/udev.c                         |   61 +++++++++++++++--
 3 files changed, 167 insertions(+), 10 deletions(-)

[PATCH v3 1/2] huawei: detect SecondaryDevice which is used for events

[Kalle Valo]

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


---

 plugins/huawei.c |   39 ++++++++++++++++++++++++++++++++++-
 plugins/udev.c   |   61 +++++++++++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 91 insertions(+), 9 deletions(-)

diff --git a/plugins/huawei.c b/plugins/huawei.c
index df4d177..83a46e3 100644
--- a/plugins/huawei.c
+++ b/plugins/huawei.c
@@ -45,7 +45,7 @@
 #include <drivers/atmodem/vendor.h>
 
 struct huawei_data {
-	GAtChat *chat;
+	GAtChat *chat, *event;
 };
 
 static int huawei_probe(struct ofono_modem *modem)
@@ -72,6 +72,7 @@ static void huawei_remove(struct ofono_modem *modem)
 	ofono_modem_set_data(modem, NULL);
 
 	g_at_chat_unref(data->chat);
+	g_at_chat_unref(data->event);
 	g_free(data);
 }
 
@@ -80,6 +81,11 @@ static void huawei_debug(const char *str, void *user_data)
 	ofono_info("%s", str);
 }
 
+static void huawei_event_debug(const char *str, void *user_data)
+{
+	ofono_info("* %s", str);
+}
+
 static void cfun_enable(gboolean ok, GAtResult *result, gpointer user_data)
 {
 	struct ofono_modem *modem = user_data;
@@ -120,6 +126,29 @@ static int huawei_enable(struct ofono_modem *modem)
 	if (getenv("OFONO_AT_DEBUG"))
 		g_at_chat_set_debug(data->chat, huawei_debug, NULL);
 
+	device = ofono_modem_get_string(modem, "SecondaryDevice");
+	if (!device)
+		return -EINVAL;
+
+	channel = g_at_tty_open(device, NULL);
+	if (!channel)
+		return -EIO;
+
+	syntax = g_at_syntax_new_gsm_permissive();
+	data->event = g_at_chat_new(channel, syntax);
+	g_at_syntax_unref(syntax);
+	g_io_channel_unref(channel);
+
+	if (!data->event)
+		return -EIO;
+
+	g_at_chat_add_terminator(data->event, "COMMAND NOT SUPPORT", -1,
+				 FALSE);
+
+	if (getenv("OFONO_AT_DEBUG"))
+		g_at_chat_set_debug(data->event, huawei_event_debug, NULL);
+
+
 	g_at_chat_send(data->chat, "ATE0", NULL, NULL, NULL, NULL);
 
 	g_at_chat_send(data->chat, "AT+CFUN=1", NULL,
@@ -138,6 +167,9 @@ static void cfun_disable(gboolean ok, GAtResult *result, gpointer user_data)
 	g_at_chat_unref(data->chat);
 	data->chat = NULL;
 
+	g_at_chat_unref(data->event);
+	data->event = NULL;
+
 	if (ok)
 		ofono_modem_set_powered(modem, FALSE);
 }
@@ -148,6 +180,11 @@ static int huawei_disable(struct ofono_modem *modem)
 
 	DBG("%p", modem);
 
+	if (data->event) {
+		g_at_chat_cancel_all(data->event);
+		g_at_chat_unregister_all(data->event);
+	}
+
 	if (!data->chat)
 		return 0;
 
diff --git a/plugins/udev.c b/plugins/udev.c
index 3a6ea28..bdac4fd 100644
--- a/plugins/udev.c
+++ b/plugins/udev.c
@@ -89,6 +89,24 @@ static const char *get_serial(struct udev_device *udev_device)
 	return serial;
 }
 
+static const char *get_usb_num(struct udev_device *udev_device)
+{
+	struct udev_list_entry *entry;
+	const char *num = NULL;
+
+	entry = udev_device_get_properties_list_entry(udev_device);
+	while (entry) {
+		const char *name = udev_list_entry_get_name(entry);
+
+		if (g_strcmp0(name, "ID_USB_INTERFACE_NUM") == 0)
+			num = udev_list_entry_get_value(entry);
+
+		entry = udev_list_entry_get_next(entry);
+	}
+
+	return num;
+}
+
 #define MODEM_DEVICE		"ModemDevice"
 #define DATA_DEVICE		"DataDevice"
 #define GPS_DEVICE		"GPSDevice"
@@ -201,18 +219,45 @@ static void add_hso(struct ofono_modem *modem,
 static void add_huawei(struct ofono_modem *modem,
 					struct udev_device *udev_device)
 {
-	const char *devnode;
-	int registered;
+	const char *devnode, *num;
+	int primary, secondary;
 
-	registered = ofono_modem_get_integer(modem, "Registered");
-	if (registered != 0)
+	primary = ofono_modem_get_integer(modem, "PrimaryRegistered");
+	secondary = ofono_modem_get_integer(modem, "SecondaryRegistered");
+
+	if (primary && secondary)
 		return;
 
-	devnode = udev_device_get_devnode(udev_device);
-	ofono_modem_set_string(modem, "Device", devnode);
+	num = get_usb_num(udev_device);
 
-	ofono_modem_set_integer(modem, "Registered", 1);
-	ofono_modem_register(modem);
+	/*
+	 * Here is is assumed that that usb port number 0 is the control
+	 * port and port 2 is the event port. This assumption will surely
+	 * be false with some devices and better heuristics is needed.
+	 */
+	if (g_strcmp0(num, "00") == 0) {
+		if (primary != 0)
+			return;
+
+		devnode = udev_device_get_devnode(udev_device);
+		ofono_modem_set_string(modem, "Device", devnode);
+
+		primary = 1;
+		ofono_modem_set_integer(modem, "PrimaryRegistered", primary);
+	} else if (g_strcmp0(num, "02") == 0) {
+		if (secondary != 0)
+			return;
+
+		devnode = udev_device_get_devnode(udev_device);
+		ofono_modem_set_string(modem, "SecondaryDevice", devnode);
+
+		secondary = 1;
+		ofono_modem_set_integer(modem, "SecondaryRegistered",
+					secondary);
+	}
+
+	if (primary && secondary)
+		ofono_modem_register(modem);
 }
 
 static void add_em770(struct ofono_modem *modem,

[PATCH v3 2/2] huawei: add gprs context

[Kalle Valo]

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

Tested with Huawei E1552 HSDPA USB stick using a finnish Saunalahti prepaid
SIM.
---

 drivers/atmodem/network-registration.c |    4 ++
 plugins/huawei.c                       |   73 ++++++++++++++++++++++++++++++++
 2 files changed, 76 insertions(+), 1 deletions(-)

diff --git a/drivers/atmodem/network-registration.c b/drivers/atmodem/network-registration.c
index f7aafbe..f528339 100644
--- a/drivers/atmodem/network-registration.c
+++ b/drivers/atmodem/network-registration.c
@@ -916,6 +916,10 @@ static void at_creg_set_cb(gboolean ok, GAtResult *result, gpointer user_data)
 				cind_support_cb, netreg, NULL);
 
 		break;
+	case OFONO_VENDOR_HUAWEI:
+		/* huawei doesn't support CIND */
+		ofono_netreg_register(netreg);
+		break;
 	default:
 		g_at_chat_send(nd->chat, "AT+CIND=?", cind_prefix,
 				cind_support_cb, netreg, NULL);
diff --git a/plugins/huawei.c b/plugins/huawei.c
index 83a46e3..0d3373f 100644
--- a/plugins/huawei.c
+++ b/plugins/huawei.c
@@ -41,7 +41,10 @@
 #include <ofono/gprs.h>
 #include <ofono/voicecall.h>
 #include <ofono/log.h>
+#include <ofono/gprs.h>
+#include <ofono/gprs-context.h>
 
+#include <drivers/atmodem/atutil.h>
 #include <drivers/atmodem/vendor.h>
 
 struct huawei_data {
@@ -211,14 +214,82 @@ static void huawei_pre_sim(struct ofono_modem *modem)
 		ofono_sim_inserted_notify(sim, TRUE);
 }
 
+static void huawei_cgreg_notify(GAtResult *result, gpointer user_data)
+{
+	struct ofono_gprs *gprs = user_data;
+	gboolean ret;
+	int status;
+
+	DBG("");
+
+	ret = at_util_parse_reg_unsolicited(result, "+CGREG:", &status,
+					    NULL, NULL, NULL,
+					    OFONO_VENDOR_HUAWEI);
+
+	if (ret == FALSE)
+		return;
+
+	ofono_gprs_status_notify(gprs, status);
+}
+
+static inline void report_signal_strength(struct ofono_netreg *netreg,
+					  int strength)
+{
+	DBG("%d", strength);
+
+	if (strength == 99)
+		strength = -1;
+	else
+		strength = (strength * 100) / 31;
+
+	ofono_netreg_strength_notify(netreg, strength);
+}
+
+static void huawei_rssi_notify(GAtResult *result, gpointer user_data)
+{
+	struct ofono_netreg *netreg = user_data;
+	GAtResultIter iter;
+	int strength;
+
+	g_at_result_iter_init(&iter, result);
+
+	if (!g_at_result_iter_next(&iter, "^RSSI:"))
+		return;
+
+	if (!g_at_result_iter_next_number(&iter, &strength))
+		return;
+
+	report_signal_strength(netreg, strength);
+}
+
 static void huawei_post_sim(struct ofono_modem *modem)
 {
 	struct huawei_data *data = ofono_modem_get_data(modem);
+	struct ofono_gprs_context *gc;
+	struct ofono_netreg *netreg;
+	struct ofono_gprs *gprs;
 
 	DBG("%p", modem);
 
-	ofono_netreg_create(modem, OFONO_VENDOR_HUAWEI, "atmodem", data->chat);
+	netreg = ofono_netreg_create(modem, OFONO_VENDOR_HUAWEI, "atmodem",
+				     data->chat);
 	ofono_sms_create(modem, OFONO_VENDOR_QUALCOMM_MSM, "atmodem", data->chat);
+
+	gprs = ofono_gprs_create(modem, OFONO_VENDOR_HUAWEI, "atmodem",
+				 data->chat);
+	gc = ofono_gprs_context_create(modem, 0, "atmodem", data->chat);
+
+	if (gprs && gc) {
+		ofono_gprs_add_context(gprs, gc);
+
+		/* huawei has a separate channel for CGREG notifications */
+		g_at_chat_register(data->event, "+CGREG:",
+				   huawei_cgreg_notify, FALSE, gprs, NULL);
+
+		/* huawei uses non-standard "^RSSI:18" strings */
+		g_at_chat_register(data->event, "^RSSI:",
+				   huawei_rssi_notify, FALSE, netreg, NULL);
+	}
 }
 
 static struct ofono_modem_driver huawei_driver = {

Re: [PATCH v3 1/2] huawei: detect SecondaryDevice which is used for events

[Kalle Valo]

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

Denis Kenzior <denkenz@gmail.com> writes:

> Hi Kalle,

Hi Denis,

> Please keep the commit header to less than 50 lines.

Fixed.

>> +	device = ofono_modem_get_string(modem, "SecondaryDevice");
>> +	if (!device)
>> +		return -EINVAL;
>
> Two tabs is enough here.

Sorry, but I see only two tabs here. What am I missing? Ah, now I
understand. I think you are commenting the line earlier in
huawei_enable() which is indented wrong. I'll fix that.

>> +	channel = g_at_tty_open(device, NULL);
>> +	if (!channel)
>> +		return -EIO;
>> +
>> +	syntax = g_at_syntax_new_gsm_permissive();
>> +	data->event = g_at_chat_new(channel, syntax);
>> +	g_at_syntax_unref(syntax);
>> +	g_io_channel_unref(channel);
>> +
>> +	if (!data->event)
>> +		return -EIO;
>> +
>> +	g_at_chat_add_terminator(data->event, "COMMAND NOT SUPPORT", -1,
>> +				 FALSE);
>
> Don't mix spaces and tabs for indentation

Fixed.

>> +
>> +	if (getenv("OFONO_AT_DEBUG"))
>> +		g_at_chat_set_debug(data->event, huawei_event_debug, NULL);
>> +
>> +
>
> Why the extra space?

You mean the newline, right? That's by accident, I removed it now.

> Also, this entire sequence leaks the data->chat in case the 2nd port
> could not be opened. See plugins/mbm.c for an example on how to do
> this better.

Damn, so it does. I changed it now to follow the style from mbm.c.

Thanks a lot for the comments.

-- 
Kalle Valo

Re: [PATCH v3 2/2] huawei: add gprs context

[Kalle Valo]

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

Denis Kenzior <denkenz@gmail.com> writes:

> Hi Kalle,

Hi again Denis,

>> +	ret = at_util_parse_reg_unsolicited(result, "+CGREG:", &status,
>> +					    NULL, NULL, NULL,
>> +					    OFONO_VENDOR_HUAWEI);
>
> No mixing of tabs & spaces for indentation

Fixed.

Oh man, I have to fix my emacs settings :/

>> +
>> +	if (ret == FALSE)
>> +		return;
>> +
>> +	ofono_gprs_status_notify(gprs, status);
>> +}
>> +
>> +static inline void report_signal_strength(struct ofono_netreg *netreg,
>> +					  int strength)
>
> And here

This function is gone now.

>> +	DBG("%d", strength);
>> +
>> +	if (strength == 99)
>> +		strength = -1;
>> +	else
>> +		strength = (strength * 100) / 31;
>> +
>> +	ofono_netreg_strength_notify(netreg, strength);
>> +}
>
> You might really want to make this into an atutil function instead of copy-
> pasting it around.

I created this:

static inline int at_util_convert_signal_strength(int strength)
{
	int result;

	if (strength == 99)
		result = -1;
	else
		result = (strength * 100) / 31;

	return result;
}

>> -	ofono_netreg_create(modem, OFONO_VENDOR_HUAWEI, "atmodem", data->chat);
>> +	netreg = ofono_netreg_create(modem, OFONO_VENDOR_HUAWEI, "atmodem",
>> +				     data->chat);
>
> Again with mixed indentation

Fixed.

>>  	ofono_sms_create(modem, OFONO_VENDOR_QUALCOMM_MSM, "atmodem",
>>  data->chat); +
>> +	gprs = ofono_gprs_create(modem, OFONO_VENDOR_HUAWEI, "atmodem",
>> +				 data->chat);
>
> And here

Fixed.

>> +	gc = ofono_gprs_context_create(modem, 0, "atmodem", data->chat);
>> +
>> +	if (gprs && gc) {
>> +		ofono_gprs_add_context(gprs, gc);
>> +
>> +		/* huawei has a separate channel for CGREG notifications */
>> +		g_at_chat_register(data->event, "+CGREG:",
>> +				   huawei_cgreg_notify, FALSE, gprs, NULL);
>
> And here

Fixed.

>> +
>> +		/* huawei uses non-standard "^RSSI:18" strings */
>> +		g_at_chat_register(data->event, "^RSSI:",
>> +				   huawei_rssi_notify, FALSE, netreg, NULL);
>
> And here

Fixed.

Thank you for the comments. I'll run some tests first and send v4 soon.

-- 
Kalle Valo

Re: [PATCH v3 2/2] huawei: add gprs context

[Inaky Perez-Gonzalez]

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

Hi Kale

On Thu, 2010-05-20 at 10:35 +0300, Kalle Valo wrote: 
> Denis Kenzior <denkenz@gmail.com> writes:
> 
> > Hi Kalle,
> 
> Hi again Denis,
> 
> >> +	ret = at_util_parse_reg_unsolicited(result, "+CGREG:", &status,
> >> +					    NULL, NULL, NULL,
> >> +					    OFONO_VENDOR_HUAWEI);
> >
> > No mixing of tabs & spaces for indentation
> 
> Fixed.
> 
> Oh man, I have to fix my emacs settings :/

Did you find a set of emacs offset settings to get to oFono's liking?
I've been fiddling with it for a while, but keep alking in circles
around it. It's been too long since I've done emacs config :(

thanks!

Re: [PATCH v3 2/2] huawei: add gprs context

[Kalle Valo]

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

Inaky Perez-Gonzalez <inaky@linux.intel.com> writes:

> Hi Kale

Hi Inaky,

> On Thu, 2010-05-20 at 10:35 +0300, Kalle Valo wrote: 
>> Denis Kenzior <denkenz@gmail.com> writes:
>> 
>> > No mixing of tabs & spaces for indentation
>> 
>> Fixed.
>> 
>> Oh man, I have to fix my emacs settings :/
>
> Did you find a set of emacs offset settings to get to oFono's liking?
> I've been fiddling with it for a while, but keep alking in circles
> around it. It's been too long since I've done emacs config :(

This is my setup:

; from linux/Documentation/CodingStyle
(defun c-lineup-arglist-tabs-only (ignored)
  "Line up argument lists by tabs, not spaces"
  (let* ((anchor (c-langelem-pos c-syntactic-element))
	 (column (c-langelem-2nd-pos c-syntactic-element))
	 (offset (- (1+ column) anchor))
	 (steps (floor offset c-basic-offset)))
    (* (max steps 1)
       c-basic-offset)))

(add-hook 'c-mode-common-hook
          (lambda ()
            ;; Add kernel style
            (c-add-style
             "linux-tabs-only"
             '("linux" (c-offsets-alist
                        (arglist-cont-nonempty
                         c-lineup-gcc-asm-reg
                         c-lineup-arglist-tabs-only))))))

And whenever I start editing ofono or connman, I run command "C-c .
linux-tabs-only" choose the style. But it's not perfect yet, for example
I need to manually add tabs when having multi-line function calls.

-- 
Kalle Valo

Re: [PATCH v3 2/2] huawei: add gprs context

[Inaky Perez-Gonzalez]

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

Hi Kale

On Thu, 2010-08-05 at 08:54 +0300, Kalle Valo wrote: 
> .... 
> >> Oh man, I have to fix my emacs settings :/
> >
> > Did you find a set of emacs offset settings to get to oFono's liking?
> > I've been fiddling with it for a while, but keep alking in circles
> > around it. It's been too long since I've done emacs config :(
> 
> This is my setup:
> 
> ; from linux/Documentation/CodingStyle
> (defun c-lineup-arglist-tabs-only (ignored)
>   "Line up argument lists by tabs, not spaces"
>   (let* ((anchor (c-langelem-pos c-syntactic-element))
> 	 (column (c-langelem-2nd-pos c-syntactic-element))
> 	 (offset (- (1+ column) anchor))
> 	 (steps (floor offset c-basic-offset)))
>     (* (max steps 1)
>        c-basic-offset)))

Thanks! Phew ... this is a handful of lisp! I don't understand anything
that is not basic anymore :) -- I gave it a whiz and one thing I saw was
that

struct tx_queue_entry *__ofono_sms_txq_submit(struct ofono_sms *sms,
						GSList *list,
						unsigned int flags, unsigned msg_id,
						ofono_sms_msg_stch_cb_t stch_cb,
						void *data, ofono_destroy_func destroy)

is being formatted as:

struct tx_queue_entry *__ofono_sms_txq_submit(struct ofono_sms *sms,
					GSList *list,
					unsigned int flags, unsigned msg_id,
					ofono_sms_msg_stch_cb_t stch_cb,
					void *data, ofono_destroy_func destroy)

ie: one c-basic-offset less than it should -- I really can't figure out where to poke.

> (add-hook 'c-mode-common-hook
>           (lambda ()
>             ;; Add kernel style
>             (c-add-style
>              "linux-tabs-only"
>              '("linux" (c-offsets-alist
>                         (arglist-cont-nonempty
>                          c-lineup-gcc-asm-reg
>                          c-lineup-arglist-tabs-only))))))
> 
> And whenever I start editing ofono or connman, I run command "C-c .
> linux-tabs-only" choose the style. But it's not perfect yet, for example
> I need to manually add tabs when having multi-line function calls.
> 

I have a code snippet for automate the mode setting:

;; Initialization hook
(add-hook 'c-mode-common-hook 'c-select-style)

;; this looks at the path and based on the name, sets a style -- it 
;; is setup to my work habits -- I keep all my ofono work in
;; ~/something/ofono/ofono.git, kernel work in ~/something/linux/*..

(defun c-select-style ()
  (let
      ((style c-default-style))
    (if (string-match 
         "linux[^/]*"
         buffer-file-truename)
        (progn
          (c-set-style "linux")
          (setq indent-tabs-mode t)
          (setq c-basic-offset 8)
          (setq style "linux")
          )
      (if (string-match "ofono[^/]*/" buffer-file-truename)
          (progn
;; I renamed your c-style to 'ofono' so not to get confused :)
            (c-set-style "ofono")
            (setq indent-tabs-mode t)
            (setq c-basic-offset 8)
            (setq style "ofono")
            )
        ;; Default
        (progn
          (c-set-style "linux")
          (setq indent-tabs-mode t)
          (setq c-basic-offset 8)
          (setq style "ofono")
          )
        )
      )
    (setq c-indent-comments-syntactically-p t)
    (define-key c-mode-map "\C-m" 'c-context-line-break)
    (c-toggle-auto-hungry-state 1)
    (message (concat "C style set to " style))
    )
  )

Re: [PATCH v3 2/2] huawei: add gprs context

[Kalle Valo]

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

Inaky Perez-Gonzalez <inaky@linux.intel.com> writes:

> Hi Kale

Hola,

> On Thu, 2010-08-05 at 08:54 +0300, Kalle Valo wrote: 
>>
>> This is my setup:
>> 
>> ; from linux/Documentation/CodingStyle
>> (defun c-lineup-arglist-tabs-only (ignored)
>>   "Line up argument lists by tabs, not spaces"
>>   (let* ((anchor (c-langelem-pos c-syntactic-element))
>> 	 (column (c-langelem-2nd-pos c-syntactic-element))
>> 	 (offset (- (1+ column) anchor))
>> 	 (steps (floor offset c-basic-offset)))
>>     (* (max steps 1)
>>        c-basic-offset)))
>
> Thanks! Phew ... this is a handful of lisp! I don't understand anything
> that is not basic anymore :)

It sure is. I just copied it from Linux CodingStyle document and it
seems to work somehow.

> -- I gave it a whiz and one thing I saw was that
>
> struct tx_queue_entry *__ofono_sms_txq_submit(struct ofono_sms *sms,
> 						GSList *list,
> 						unsigned int flags, unsigned msg_id,
> 						ofono_sms_msg_stch_cb_t stch_cb,
> 						void *data, ofono_destroy_func destroy)
>
> is being formatted as:
>
> struct tx_queue_entry *__ofono_sms_txq_submit(struct ofono_sms *sms,
> 					GSList *list,
> 					unsigned int flags, unsigned msg_id,
> 					ofono_sms_msg_stch_cb_t stch_cb,
> 					void *data, ofono_destroy_func destroy)
>
> ie: one c-basic-offset less than it should -- I really can't figure out where to poke.

That's my problem as well. Currently I just manually add one more tab to
each line with M-i. If you find a way to fix this, please let me know.

-- 
Kalle Valo

Re: [PATCH v3 2/2] huawei: add gprs context

[Inaky Perez-Gonzalez]

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

On Fri, 2010-08-06 at 10:26 +0300, Kalle Valo wrote: 
> Inaky Perez-Gonzalez <inaky@linux.intel.com> writes:
> 
> > Hi Kale
> 
> Hola,
> 
> > On Thu, 2010-08-05 at 08:54 +0300, Kalle Valo wrote: 
> >>
> >> This is my setup:
> >> 
> >> ; from linux/Documentation/CodingStyle
> >> (defun c-lineup-arglist-tabs-only (ignored)
> >>   "Line up argument lists by tabs, not spaces"
> >>   (let* ((anchor (c-langelem-pos c-syntactic-element))
> >> 	 (column (c-langelem-2nd-pos c-syntactic-element))
> >> 	 (offset (- (1+ column) anchor))
> >> 	 (steps (floor offset c-basic-offset)))
> >>     (* (max steps 1)
> >>        c-basic-offset)))
> >
> > Thanks! Phew ... this is a handful of lisp! I don't understand anything
> > that is not basic anymore :)
> 
> It sure is. I just copied it from Linux CodingStyle document and it
> seems to work somehow.
> 
> > -- I gave it a whiz and one thing I saw was that
> >
> > struct tx_queue_entry *__ofono_sms_txq_submit(struct ofono_sms *sms,
> > 						GSList *list,
> > 						unsigned int flags, unsigned msg_id,
> > 						ofono_sms_msg_stch_cb_t stch_cb,
> > 						void *data, ofono_destroy_func destroy)
> >
> > is being formatted as:
> >
> > struct tx_queue_entry *__ofono_sms_txq_submit(struct ofono_sms *sms,
> > 					GSList *list,
> > 					unsigned int flags, unsigned msg_id,
> > 					ofono_sms_msg_stch_cb_t stch_cb,
> > 					void *data, ofono_destroy_func destroy)
> >
> > ie: one c-basic-offset less than it should -- I really can't figure out where to poke.
> 
> That's my problem as well. Currently I just manually add one more tab to
> each line with M-i. If you find a way to fix this, please let me know.

I think I found it, seems to cut it; replace:

(steps (floor offset c-basic-offset)))

with

         (steps (floor (+ offset (- c-basic-offset 1)) c-basic-offset)))

Lisp nuts me out, but it is doing the:

steps = floor(offset/c-basic-offset);

with

steps = floor((offset + c-basic-offset - 1) / c-basic-offset);

thus making sure there is always one more tab except when the offset is
right on a tab boundary that we keep the same.

So far, so good -- probably it fails in some case, because this is
totally trial and error as unscientific as it can :)