From: Philippe Nunes <philippe.nunes@linux.intel.com>
To: ofono@ofono.org
Subject: Re: [PATCH 3/5] udevng.c: tty assignment according OFONO_LABEL should take precedence
Date: Mon, 19 Dec 2011 17:32:02 +0100 [thread overview]
Message-ID: <4EEF6702.5090509@linux.intel.com> (raw)
In-Reply-To: <1324273728.1965.99.camel@aeonflux>
[-- Attachment #1: Type: text/plain, Size: 6891 bytes --]
Hi Marcel,
On 12/19/2011 06:48 AM, Marcel Holtmann wrote:
> Hi Philippe,
>
>> plugins/udevng.c | 61 +++++++++++++++++++++++++++++++++--------------------
>> 1 files changed, 38 insertions(+), 23 deletions(-)
>>
>> diff --git a/plugins/udevng.c b/plugins/udevng.c
>> index be87320..6a3c970 100644
>> --- a/plugins/udevng.c
>> +++ b/plugins/udevng.c
>> @@ -235,6 +235,7 @@ static gboolean setup_sierra(struct modem_info *modem)
>> static gboolean setup_huawei(struct modem_info *modem)
>> {
>> const char *mdm = NULL, *pcui = NULL, *diag = NULL;
>> + const char *default_pcui = NULL;
>> GSList *list;
>>
>> DBG("%s", modem->syspath);
>> @@ -264,19 +265,24 @@ static gboolean setup_huawei(struct modem_info *modem)
>> if (mdm != NULL&& pcui != NULL)
>> break;
>> } else if (g_strcmp0(info->interface, "255/255/255") == 0) {
>> - if (g_strcmp0(info->number, "00") == 0)
>> + if (mdm == NULL&& g_strcmp0(info->number, "00") == 0)
>> mdm = info->devnode;
>> - else if (g_strcmp0(info->number, "01") == 0)
>> - pcui = info->devnode;
>> - else if (g_strcmp0(info->number, "02") == 0)
>> - pcui = info->devnode;
>> - else if (g_strcmp0(info->number, "03") == 0)
>> - pcui = info->devnode;
>> - else if (g_strcmp0(info->number, "04") == 0)
>> - pcui = info->devnode;
>> + else if (pcui == NULL) {
>> + if (g_strcmp0(info->number, "01") == 0)
>> + default_pcui = info->devnode;
>> + else if (g_strcmp0(info->number, "02") == 0)
>> + default_pcui = info->devnode;
>> + else if (g_strcmp0(info->number, "03") == 0)
>> + default_pcui = info->devnode;
>> + else if (g_strcmp0(info->number, "04") == 0)
>> + default_pcui = info->devnode;
>> + }
>> }
>> }
>>
>> + if (pcui == NULL&& default_pcui != NULL)
>> + pcui = default_pcui;
>> +
>
> what is this default_pcui doing here. I am failing to understand its
> usage.
We need to prevent the assignment done through the label, to be
overridden by the 'default' assignment (the one done by the implicit rule).
That's why I introduced new conditions:
if (mdm == NULL && g_strcmp0(info->number, "00") == 0)
and
else if (pcui == NULL) {
In practice, if the pcui is already assigned through the label, the
'default' assignment' algorithm is not considered.
On the contrary, the 'default' assignment is done using a dedicated
variable 'default_pcui'.
If we were still using the variable 'pcui', we couldn't update anymore
this variable through the 'default' assignment due to the new condition
'if (pcui == NULL)'.
>
>> if (mdm == NULL || pcui == NULL)
>> return FALSE;
>>
>> @@ -291,7 +297,7 @@ static gboolean setup_huawei(struct modem_info *modem)
>>
>> static gboolean setup_speedup(struct modem_info *modem)
>> {
>> - const char *aux = NULL, *mdm = NULL;
>> + const char *aux = NULL, *mdm = NULL, *default_mdm = NULL;
>> GSList *list;
>>
>> DBG("%s", modem->syspath);
>> @@ -311,15 +317,20 @@ static gboolean setup_speedup(struct modem_info *modem)
>> if (aux != NULL)
>> break;
>> } else if (g_strcmp0(info->interface, "255/255/255") == 0) {
>> - if (g_strcmp0(info->number, "01") == 0)
>> + if (aux == NULL&& g_strcmp0(info->number, "01") == 0)
>> aux = info->devnode;
>> - else if (g_strcmp0(info->number, "02") == 0)
>> - mdm = info->devnode;
>> - else if (g_strcmp0(info->number, "03") == 0)
>> - mdm = info->devnode;
>> + else if (mdm == NULL) {
>> + if (g_strcmp0(info->number, "02") == 0)
>> + default_mdm = info->devnode;
>> + else if (g_strcmp0(info->number, "03") == 0)
>> + default_mdm = info->devnode;
>> + }
>> }
>> }
>>
>> + if (mdm == NULL&& default_mdm != NULL)
>> + mdm = default_mdm;
>> +
>
> Same here. This makes the code pretty much unreadable.
This logic becomes however necessary as I introduced dedicated rules for
Speedup and Huawei based on the label.
Without this patch, the 'label' assignment doesn't work necessarily (in
fact, it depends of the order of the interfaces).
>
>> if (aux == NULL || mdm == NULL)
>> return FALSE;
>>
>> @@ -385,9 +396,10 @@ static gboolean setup_alcatel(struct modem_info *modem)
>> if (aux != NULL)
>> break;
>> } else if (g_strcmp0(info->interface, "255/255/255") == 0) {
>> - if (g_strcmp0(info->number, "03") == 0)
>> + if (aux == NULL&& g_strcmp0(info->number, "03") == 0)
>> aux = info->devnode;
>> - else if (g_strcmp0(info->number, "05") == 0)
>> + else if (mdm == NULL&&
>> + g_strcmp0(info->number, "05") == 0)
>> mdm = info->devnode;
>> }
>> }
>> @@ -425,9 +437,10 @@ static gboolean setup_novatel(struct modem_info *modem)
>> if (aux != NULL)
>> break;
>> } else if (g_strcmp0(info->interface, "255/255/255") == 0) {
>> - if (g_strcmp0(info->number, "00") == 0)
>> + if (aux == NULL&& g_strcmp0(info->number, "00") == 0)
>> aux = info->devnode;
>> - else if (g_strcmp0(info->number, "01") == 0)
>> + else if (mdm == NULL&&
>> + g_strcmp0(info->number, "01") == 0)
>> mdm = info->devnode;
>> }
>> }
>> @@ -465,9 +478,10 @@ static gboolean setup_nokia(struct modem_info *modem)
>> if (aux != NULL)
>> break;
>> } else if (g_strcmp0(info->interface, "10/0/0") == 0) {
>> - if (g_strcmp0(info->number, "02") == 0)
>> + if (mdm == NULL&& g_strcmp0(info->number, "02") == 0)
>> mdm = info->devnode;
>> - else if (g_strcmp0(info->number, "04") == 0)
>> + else if (aux == NULL&&
>> + g_strcmp0(info->number, "04") == 0)
>> aux = info->devnode;
>> }
>> }
>> @@ -505,13 +519,14 @@ static gboolean setup_telit(struct modem_info *modem)
>> if (aux != NULL)
>> break;
>> } else if (g_strcmp0(info->interface, "255/255/255") == 0) {
>> - if (g_strcmp0(info->number, "00") == 0)
>> + if (mdm == NULL&& g_strcmp0(info->number, "00") == 0)
>> mdm = info->devnode;
>> else if (g_strcmp0(info->number, "01") == 0)
>> diag = info->devnode;
>> else if (g_strcmp0(info->number, "02") == 0)
>> gps = info->devnode;
>> - else if (g_strcmp0(info->number, "03") == 0)
>> + else if (aux == NULL
>> + && g_strcmp0(info->number, "03") == 0)
>> aux = info->devnode;
>> }
>> }
>
> This becomes unreadable. What are we trying to achieve here.
For Alcatel, Novatel, Nokia, Telit, I don't have a bug stating that we
could have an overriding issue.
I just propagated the logic explained above for more safety.
Regards,
Philippe.
>
> Regards
>
> Marcel
>
>
> _______________________________________________
> ofono mailing list
> ofono(a)ofono.org
> http://lists.ofono.org/listinfo/ofono
>
next prev parent reply other threads:[~2011-12-19 16:32 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-11-29 12:21 [PATCH 0/5] udev rules update Philippe Nunes
2011-11-29 12:20 ` Marcel Holtmann
2011-11-29 14:51 ` Philippe Nunes
2011-11-29 12:21 ` [PATCH 1/5] udev: Add rules to support ZTE MF668 dongle Philippe Nunes
2011-12-19 5:49 ` Marcel Holtmann
2011-12-19 17:34 ` Philippe Nunes
2011-11-29 12:21 ` [PATCH 2/5] udev: Add rules to support ZTE MF190 dongle Philippe Nunes
2011-12-19 5:50 ` Marcel Holtmann
2011-12-19 17:45 ` Philippe Nunes
2011-11-29 12:21 ` [PATCH 3/5] udevng.c: tty assignment according OFONO_LABEL should take precedence Philippe Nunes
2011-12-19 5:48 ` Marcel Holtmann
2011-12-19 16:32 ` Philippe Nunes [this message]
2011-11-29 12:21 ` [PATCH 4/5] udev: Add rules to support Speedup 7300 dongle Philippe Nunes
2011-12-19 5:51 ` Marcel Holtmann
2011-11-29 12:21 ` [PATCH 5/5] udev: Add rules to support SpeedUp 9800 dongle Philippe Nunes
2011-12-19 5:52 ` Marcel Holtmann
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4EEF6702.5090509@linux.intel.com \
--to=philippe.nunes@linux.intel.com \
--cc=ofono@ofono.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.