* [PATCH 3/3] sim800: add udev detection
2018-10-03 13:28 [PATCH 1/3] sim800: add support for sim800 modem Clement Viel
@ 2018-10-03 13:28 ` Clement Viel
0 siblings, 0 replies; 12+ messages in thread
From: Clement Viel @ 2018-10-03 13:28 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 501 bytes --]
---
plugins/udevng.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/plugins/udevng.c b/plugins/udevng.c
index 3c39e68..202519e 100644
--- a/plugins/udevng.c
+++ b/plugins/udevng.c
@@ -1300,6 +1300,7 @@ static struct {
{ "calypso", setup_serial_modem },
{ "cinterion", setup_serial_modem },
{ "nokiacdma", setup_serial_modem },
+ { "sim800", setup_serial_modem },
{ "sim900", setup_serial_modem },
{ "wavecom", setup_wavecom },
{ "tc65", setup_tc65 },
--
2.7.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 3/3] sim800: add udev detection
2018-10-15 17:27 [PATCH 1/3] sim800: add support for sim800 modem Clement Viel
@ 2018-10-15 17:27 ` Clement Viel
0 siblings, 0 replies; 12+ messages in thread
From: Clement Viel @ 2018-10-15 17:27 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 501 bytes --]
---
plugins/udevng.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/plugins/udevng.c b/plugins/udevng.c
index 3c39e68..202519e 100644
--- a/plugins/udevng.c
+++ b/plugins/udevng.c
@@ -1300,6 +1300,7 @@ static struct {
{ "calypso", setup_serial_modem },
{ "cinterion", setup_serial_modem },
{ "nokiacdma", setup_serial_modem },
+ { "sim800", setup_serial_modem },
{ "sim900", setup_serial_modem },
{ "wavecom", setup_wavecom },
{ "tc65", setup_tc65 },
--
2.7.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 3/3] sim800: add udev detection.
2018-11-07 10:14 [PATCH 1/3] sim800: add support for sim800 modem Clement Viel
@ 2018-11-07 10:14 ` Clement Viel
2018-11-07 13:30 ` Jonas Bonn
0 siblings, 1 reply; 12+ messages in thread
From: Clement Viel @ 2018-11-07 10:14 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 5614 bytes --]
---
plugins/udevng.c | 92 +++++++++++++++++++++++++++++++++++++++++---------------
1 file changed, 68 insertions(+), 24 deletions(-)
diff --git a/plugins/udevng.c b/plugins/udevng.c
index ff6e1fc..ee6f9e8 100644
--- a/plugins/udevng.c
+++ b/plugins/udevng.c
@@ -711,8 +711,56 @@ static gboolean setup_telitqmi(struct modem_info *modem)
return TRUE;
}
-/* TODO: Not used as we have no simcom driver */
-static gboolean setup_simcom(struct modem_info *modem)
+
+static gboolean setup_sim800(struct modem_info *modem)
+{
+ const char *mdm = NULL, *aux = NULL, *gps = NULL, *diag = NULL;
+ GSList *list;
+
+ DBG("%s", modem->syspath);
+
+ for (list = modem->devices; list; list = list->next) {
+ struct device_info *info = list->data;
+
+ DBG("%s %s %s %s", info->devnode, info->interface,
+ info->number, info->label);
+
+ if (g_strcmp0(info->label, "aux") == 0) {
+ DBG("setting aux as info->devnode");
+ aux = info->devnode;
+ if (mdm != NULL)
+ break;
+ } else if (g_strcmp0(info->label, "modem") == 0) {
+ mdm = info->devnode;
+ if (aux != NULL)
+ break;
+ } else if (g_strcmp0(info->interface, "255/0/0") == 0) {
+ if (g_strcmp0(info->number, "00") == 0)
+ mdm = info->devnode;
+ else if (g_strcmp0(info->number, "01") == 0)
+ gps = info->devnode;
+ else if (g_strcmp0(info->number, "02") == 0)
+ aux = info->devnode;
+ else if (g_strcmp0(info->number, "03") == 0)
+ mdm = info->devnode;
+ }
+ }
+ DBG("modem=%s aux=%s gps=%s diag=%s", mdm, aux, gps, diag);
+
+ if (mdm == NULL) {
+ DBG("modem did not set up");
+ return FALSE;
+ }
+
+ ofono_modem_set_string(modem->modem, "Device", mdm);
+ ofono_modem_set_string(modem->modem, "Modem", mdm);
+
+ return TRUE;
+}
+
+
+
+static gboolean setup_sim900(struct modem_info *modem)
{
const char *mdm = NULL, *aux = NULL, *gps = NULL, *diag = NULL;
GSList *list;
@@ -962,6 +1010,8 @@ static gboolean setup_mbim(struct modem_info *modem)
ofono_modem_set_string(modem->modem, "Device", ctl);
ofono_modem_set_string(modem->modem, "NetworkInterface", net);
ofono_modem_set_string(modem->modem, "DescriptorFile", descriptors);
+ ofono_modem_set_string(modem->modem, "Vendor", modem->vendor);
+ ofono_modem_set_string(modem->modem, "Model", modem->model);
return TRUE;
}
@@ -1193,22 +1243,12 @@ static gboolean setup_xmm7xxx(struct modem_info *modem)
info->interface, info->number, info->label,
info->sysattr, info->subsystem);
- if (g_strcmp0(modem->model,"095a") == 0) {
- if (g_strcmp0(info->subsystem, "tty") == 0) {
- if (g_strcmp0(info->number, "00") == 0)
- mdm = info->devnode;
- } else if (g_strcmp0(info->subsystem, "net") == 0) {
- if (g_strcmp0(info->number, "06") == 0)
- net = info->devnode;
- }
- } else {
- if (g_strcmp0(info->subsystem, "tty") == 0) {
- if (g_strcmp0(info->number, "02") == 0)
- mdm = info->devnode;
- } else if (g_strcmp0(info->subsystem, "net") == 0) {
- if (g_strcmp0(info->number, "00") == 0)
- net = info->devnode;
- }
+ if (g_strcmp0(info->subsystem, "tty") == 0) {
+ if (g_strcmp0(info->number, "02") == 0)
+ mdm = info->devnode;
+ } else if (g_strcmp0(info->subsystem, "net") == 0) {
+ if (g_strcmp0(info->number, "00") == 0)
+ net = info->devnode;
}
}
@@ -1290,7 +1330,8 @@ static struct {
{ "nokia", setup_nokia },
{ "telit", setup_telit, "device/interface" },
{ "telitqmi", setup_telitqmi },
- { "simcom", setup_simcom },
+ { "sim800", setup_sim800 },
+ { "sim900", setup_sim900 },
{ "sim7100", setup_sim7100 },
{ "zte", setup_zte },
{ "icera", setup_icera },
@@ -1308,7 +1349,9 @@ static struct {
{ "calypso", setup_serial_modem },
{ "cinterion", setup_serial_modem },
{ "nokiacdma", setup_serial_modem },
+ { "sim800", setup_serial_modem },
{ "sim900", setup_serial_modem },
+ { "sim800", setup_serial_modem },
{ "wavecom", setup_wavecom },
{ "tc65", setup_tc65 },
{ "ehs6", setup_ehs6 },
@@ -1471,7 +1514,7 @@ static void add_serial_device(struct udev_device *dev)
const char *subsystem;
struct udev_device* mdev;
const char* driver;
-
+ DBG("adding %s interface", udev_device_get_devpath(dev));
mdev = get_serial_modem_device(dev);
if (!mdev) {
DBG("Device is missing required OFONO_DRIVER property");
@@ -1663,7 +1706,8 @@ static struct {
{ "alcatel", "option", "1bbb", "0017" },
{ "novatel", "option", "1410" },
{ "zte", "option", "19d2" },
- { "simcom", "option", "05c6", "9000" },
+ { "sim800", "option", "05c6", "9000" },
+ { "sim900", "option", "05c6", "9000" },
{ "sim7100", "option", "1e0e", "9001" },
{ "telit", "usbserial", "1bc7" },
{ "telit", "option", "1bc7" },
@@ -1693,7 +1737,6 @@ static struct {
{ "xmm7xxx", "cdc_ncm", "8087" },
{ }
};
-
static void check_usb_device(struct udev_device *device)
{
struct udev_device *usb_device;
@@ -1722,9 +1765,10 @@ static void check_usb_device(struct udev_device *device)
udev_device_get_parent_with_subsystem_devtype(
device, "usb", "usb_interface");
- if (usb_interface)
+ if (usb_interface) {
driver = udev_device_get_property_value(
usb_interface, "OFONO_DRIVER");
+ }
}
if (driver == NULL) {
@@ -1801,7 +1845,7 @@ static gboolean create_modem(gpointer key, gpointer value, gpointer user_data)
struct modem_info *modem = value;
const char *syspath = key;
unsigned int i;
-
+ DBG("");
if (modem->modem != NULL)
return FALSE;
--
2.7.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] sim800: add udev detection.
2018-11-07 10:14 ` [PATCH 3/3] sim800: add udev detection Clement Viel
@ 2018-11-07 13:30 ` Jonas Bonn
2018-11-07 13:58 ` Clement Viel
0 siblings, 1 reply; 12+ messages in thread
From: Jonas Bonn @ 2018-11-07 13:30 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 6638 bytes --]
Hi Clement,
On 07/11/2018 11:14, Clement Viel wrote:
> ---
> plugins/udevng.c | 92 +++++++++++++++++++++++++++++++++++++++++---------------
> 1 file changed, 68 insertions(+), 24 deletions(-)
>
> diff --git a/plugins/udevng.c b/plugins/udevng.c
> index ff6e1fc..ee6f9e8 100644
> --- a/plugins/udevng.c
> +++ b/plugins/udevng.c
> @@ -711,8 +711,56 @@ static gboolean setup_telitqmi(struct modem_info *modem)
> return TRUE;
> }
>
> -/* TODO: Not used as we have no simcom driver */
> -static gboolean setup_simcom(struct modem_info *modem)
> +
> +static gboolean setup_sim800(struct modem_info *modem)
> +{
> + const char *mdm = NULL, *aux = NULL, *gps = NULL, *diag = NULL;
> + GSList *list;
> +
> + DBG("%s", modem->syspath);
> +
> + for (list = modem->devices; list; list = list->next) {
> + struct device_info *info = list->data;
> +
> + DBG("%s %s %s %s", info->devnode, info->interface,
> + info->number, info->label);
> +
> + if (g_strcmp0(info->label, "aux") == 0) {
> + DBG("setting aux as info->devnode");
> + aux = info->devnode;
> + if (mdm != NULL)
> + break;
> + } else if (g_strcmp0(info->label, "modem") == 0) {
> + mdm = info->devnode;
> + if (aux != NULL)
> + break;
> + } else if (g_strcmp0(info->interface, "255/0/0") == 0) {
> + if (g_strcmp0(info->number, "00") == 0)
> + mdm = info->devnode;
> + else if (g_strcmp0(info->number, "01") == 0)
> + gps = info->devnode;
> + else if (g_strcmp0(info->number, "02") == 0)
> + aux = info->devnode;
> + else if (g_strcmp0(info->number, "03") == 0)
> + mdm = info->devnode;
> + }
> + }
> + DBG("modem=%s aux=%s gps=%s diag=%s", mdm, aux, gps, diag);
> +
gps is unused.
> + if (mdm == NULL) {
> + DBG("modem did not set up");
> + return FALSE;
> + }
> +
> + ofono_modem_set_string(modem->modem, "Device", mdm);
You probably mean 'aux' here.
> + ofono_modem_set_string(modem->modem, "Modem", mdm);
> +
> + return TRUE;
> +}
> +
> +
> +
> +static gboolean setup_sim900(struct modem_info *modem)
> {
> const char *mdm = NULL, *aux = NULL, *gps = NULL, *diag = NULL;
> GSList *list;
> @@ -962,6 +1010,8 @@ static gboolean setup_mbim(struct modem_info *modem)
> ofono_modem_set_string(modem->modem, "Device", ctl);
> ofono_modem_set_string(modem->modem, "NetworkInterface", net);
> ofono_modem_set_string(modem->modem, "DescriptorFile", descriptors);
> + ofono_modem_set_string(modem->modem, "Vendor", modem->vendor);
> + ofono_modem_set_string(modem->modem, "Model", modem->model);
I don't see that these strings are being used anywhere in the modem driver.
>
> return TRUE;
> }
> @@ -1193,22 +1243,12 @@ static gboolean setup_xmm7xxx(struct modem_info *modem)
> info->interface, info->number, info->label,
> info->sysattr, info->subsystem);
>
> - if (g_strcmp0(modem->model,"095a") == 0) {
> - if (g_strcmp0(info->subsystem, "tty") == 0) {
> - if (g_strcmp0(info->number, "00") == 0)
> - mdm = info->devnode;
> - } else if (g_strcmp0(info->subsystem, "net") == 0) {
> - if (g_strcmp0(info->number, "06") == 0)
> - net = info->devnode;
> - }
> - } else {
> - if (g_strcmp0(info->subsystem, "tty") == 0) {
> - if (g_strcmp0(info->number, "02") == 0)
> - mdm = info->devnode;
> - } else if (g_strcmp0(info->subsystem, "net") == 0) {
> - if (g_strcmp0(info->number, "00") == 0)
> - net = info->devnode;
> - }
> + if (g_strcmp0(info->subsystem, "tty") == 0) {
> + if (g_strcmp0(info->number, "02") == 0)
> + mdm = info->devnode;
> + } else if (g_strcmp0(info->subsystem, "net") == 0) {
> + if (g_strcmp0(info->number, "00") == 0)
> + net = info->devnode;
> }
> }
>
This change doesn't look like it belongs to this patch.
> @@ -1290,7 +1330,8 @@ static struct {
> { "nokia", setup_nokia },
> { "telit", setup_telit, "device/interface" },
> { "telitqmi", setup_telitqmi },
> - { "simcom", setup_simcom },
> + { "sim800", setup_sim800 },
> + { "sim900", setup_sim900 },
Is the sim900 not a serial modem? These setup routines are really only
for USB devices.
> { "sim7100", setup_sim7100 },
> { "zte", setup_zte },
> { "icera", setup_icera },
> @@ -1308,7 +1349,9 @@ static struct {
> { "calypso", setup_serial_modem },
> { "cinterion", setup_serial_modem },
> { "nokiacdma", setup_serial_modem },
> + { "sim800", setup_serial_modem },
> { "sim900", setup_serial_modem },
> + { "sim800", setup_serial_modem },
You've added the same line twice. Furthermore, the sim800 is a USB
modem (???) so it should not be set up in this way.
> { "wavecom", setup_wavecom },
> { "tc65", setup_tc65 },
> { "ehs6", setup_ehs6 },
> @@ -1471,7 +1514,7 @@ static void add_serial_device(struct udev_device *dev)
> const char *subsystem;
> struct udev_device* mdev;
> const char* driver;
> -
> + DBG("adding %s interface", udev_device_get_devpath(dev));
> mdev = get_serial_modem_device(dev);
> if (!mdev) {
> DBG("Device is missing required OFONO_DRIVER property");
> @@ -1663,7 +1706,8 @@ static struct {
> { "alcatel", "option", "1bbb", "0017" },
> { "novatel", "option", "1410" },
> { "zte", "option", "19d2" },
> - { "simcom", "option", "05c6", "9000" },
> + { "sim800", "option", "05c6", "9000" }
> + { "sim900", "option", "05c6", "9000" },
The sim900 is a serial modem... there is no vendor id nor product id.
> { "sim7100", "option", "1e0e", "9001" },
> { "telit", "usbserial", "1bc7" },
> { "telit", "option", "1bc7" },
> @@ -1693,7 +1737,6 @@ static struct {
> { "xmm7xxx", "cdc_ncm", "8087" },
> { }
> };
> -
> static void check_usb_device(struct udev_device *device)
> {
> struct udev_device *usb_device;
> @@ -1722,9 +1765,10 @@ static void check_usb_device(struct udev_device *device)
> udev_device_get_parent_with_subsystem_devtype(
> device, "usb", "usb_interface");
>
> - if (usb_interface)
> + if (usb_interface) {
> driver = udev_device_get_property_value(
> usb_interface, "OFONO_DRIVER");
> + }
> }
No.
>
> if (driver == NULL) {
> @@ -1801,7 +1845,7 @@ static gboolean create_modem(gpointer key, gpointer value, gpointer user_data)
> struct modem_info *modem = value;
> const char *syspath = key;
> unsigned int i;
> -
> + DBG("");
> if (modem->modem != NULL)
> return FALSE;
>
Doesn't belong to this patch, if at all.
>
/Jonas
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] sim800: add udev detection.
2018-11-07 13:30 ` Jonas Bonn
@ 2018-11-07 13:58 ` Clement Viel
0 siblings, 0 replies; 12+ messages in thread
From: Clement Viel @ 2018-11-07 13:58 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 7023 bytes --]
On Wed, Nov 07, 2018 at 02:30:43PM +0100, Jonas Bonn wrote:
> Hi Clement,
>
> On 07/11/2018 11:14, Clement Viel wrote:
> >---
> > plugins/udevng.c | 92 +++++++++++++++++++++++++++++++++++++++++---------------
> > 1 file changed, 68 insertions(+), 24 deletions(-)
> >
> >diff --git a/plugins/udevng.c b/plugins/udevng.c
> >index ff6e1fc..ee6f9e8 100644
> >--- a/plugins/udevng.c
> >+++ b/plugins/udevng.c
> >@@ -711,8 +711,56 @@ static gboolean setup_telitqmi(struct modem_info *modem)
> > return TRUE;
> > }
> >-/* TODO: Not used as we have no simcom driver */
> >-static gboolean setup_simcom(struct modem_info *modem)
> >+
> >+static gboolean setup_sim800(struct modem_info *modem)
> >+{
> >+ const char *mdm = NULL, *aux = NULL, *gps = NULL, *diag = NULL;
> >+ GSList *list;
> >+
> >+ DBG("%s", modem->syspath);
> >+
> >+ for (list = modem->devices; list; list = list->next) {
> >+ struct device_info *info = list->data;
> >+
> >+ DBG("%s %s %s %s", info->devnode, info->interface,
> >+ info->number, info->label);
> >+
> >+ if (g_strcmp0(info->label, "aux") == 0) {
> >+ DBG("setting aux as info->devnode");
> >+ aux = info->devnode;
> >+ if (mdm != NULL)
> >+ break;
> >+ } else if (g_strcmp0(info->label, "modem") == 0) {
> >+ mdm = info->devnode;
> >+ if (aux != NULL)
> >+ break;
> >+ } else if (g_strcmp0(info->interface, "255/0/0") == 0) {
> >+ if (g_strcmp0(info->number, "00") == 0)
> >+ mdm = info->devnode;
> >+ else if (g_strcmp0(info->number, "01") == 0)
> >+ gps = info->devnode;
> >+ else if (g_strcmp0(info->number, "02") == 0)
> >+ aux = info->devnode;
> >+ else if (g_strcmp0(info->number, "03") == 0)
> >+ mdm = info->devnode;
> >+ }
> >+ }
> >+ DBG("modem=%s aux=%s gps=%s diag=%s", mdm, aux, gps, diag);
> >+
>
> gps is unused.
>
>
> >+ if (mdm == NULL) {
> >+ DBG("modem did not set up");
> >+ return FALSE;
> >+ }
> >+
> >+ ofono_modem_set_string(modem->modem, "Device", mdm);
>
> You probably mean 'aux' here.
>
> >+ ofono_modem_set_string(modem->modem, "Modem", mdm);
> >+
> >+ return TRUE;
> >+}
> >+
> >+
> >+
> >+static gboolean setup_sim900(struct modem_info *modem)
> > {
> > const char *mdm = NULL, *aux = NULL, *gps = NULL, *diag = NULL;
> > GSList *list;
> >@@ -962,6 +1010,8 @@ static gboolean setup_mbim(struct modem_info *modem)
> > ofono_modem_set_string(modem->modem, "Device", ctl);
> > ofono_modem_set_string(modem->modem, "NetworkInterface", net);
> > ofono_modem_set_string(modem->modem, "DescriptorFile", descriptors);
> >+ ofono_modem_set_string(modem->modem, "Vendor", modem->vendor);
> >+ ofono_modem_set_string(modem->modem, "Model", modem->model);
>
> I don't see that these strings are being used anywhere in the modem driver.
>
> > return TRUE;
> > }
> >@@ -1193,22 +1243,12 @@ static gboolean setup_xmm7xxx(struct modem_info *modem)
> > info->interface, info->number, info->label,
> > info->sysattr, info->subsystem);
> >- if (g_strcmp0(modem->model,"095a") == 0) {
> >- if (g_strcmp0(info->subsystem, "tty") == 0) {
> >- if (g_strcmp0(info->number, "00") == 0)
> >- mdm = info->devnode;
> >- } else if (g_strcmp0(info->subsystem, "net") == 0) {
> >- if (g_strcmp0(info->number, "06") == 0)
> >- net = info->devnode;
> >- }
> >- } else {
> >- if (g_strcmp0(info->subsystem, "tty") == 0) {
> >- if (g_strcmp0(info->number, "02") == 0)
> >- mdm = info->devnode;
> >- } else if (g_strcmp0(info->subsystem, "net") == 0) {
> >- if (g_strcmp0(info->number, "00") == 0)
> >- net = info->devnode;
> >- }
> >+ if (g_strcmp0(info->subsystem, "tty") == 0) {
> >+ if (g_strcmp0(info->number, "02") == 0)
> >+ mdm = info->devnode;
> >+ } else if (g_strcmp0(info->subsystem, "net") == 0) {
> >+ if (g_strcmp0(info->number, "00") == 0)
> >+ net = info->devnode;
> > }
> > }
>
> This change doesn't look like it belongs to this patch.
>
>
> >@@ -1290,7 +1330,8 @@ static struct {
> > { "nokia", setup_nokia },
> > { "telit", setup_telit, "device/interface" },
> > { "telitqmi", setup_telitqmi },
> >- { "simcom", setup_simcom },
> >+ { "sim800", setup_sim800 },
> >+ { "sim900", setup_sim900 },
>
> Is the sim900 not a serial modem? These setup routines are really only for
> USB devices.
>
> > { "sim7100", setup_sim7100 },
> > { "zte", setup_zte },
> > { "icera", setup_icera },
> >@@ -1308,7 +1349,9 @@ static struct {
> > { "calypso", setup_serial_modem },
> > { "cinterion", setup_serial_modem },
> > { "nokiacdma", setup_serial_modem },
> >+ { "sim800", setup_serial_modem },
> > { "sim900", setup_serial_modem },
> >+ { "sim800", setup_serial_modem },
>
> You've added the same line twice. Furthermore, the sim800 is a USB modem
> (???) so it should not be set up in this way.
>
> > { "wavecom", setup_wavecom },
> > { "tc65", setup_tc65 },
> > { "ehs6", setup_ehs6 },
> >@@ -1471,7 +1514,7 @@ static void add_serial_device(struct udev_device *dev)
> > const char *subsystem;
> > struct udev_device* mdev;
> > const char* driver;
> >-
> >+ DBG("adding %s interface", udev_device_get_devpath(dev));
> > mdev = get_serial_modem_device(dev);
> > if (!mdev) {
> > DBG("Device is missing required OFONO_DRIVER property");
> >@@ -1663,7 +1706,8 @@ static struct {
> > { "alcatel", "option", "1bbb", "0017" },
> > { "novatel", "option", "1410" },
> > { "zte", "option", "19d2" },
> >- { "simcom", "option", "05c6", "9000" },
> >+ { "sim800", "option", "05c6", "9000" }
> >+ { "sim900", "option", "05c6", "9000" },
>
>
> The sim900 is a serial modem... there is no vendor id nor product id.
>
>
> > { "sim7100", "option", "1e0e", "9001" },
> > { "telit", "usbserial", "1bc7" },
> > { "telit", "option", "1bc7" },
> >@@ -1693,7 +1737,6 @@ static struct {
> > { "xmm7xxx", "cdc_ncm", "8087" },
> > { }
> > };
> >-
> > static void check_usb_device(struct udev_device *device)
> > {
> > struct udev_device *usb_device;
> >@@ -1722,9 +1765,10 @@ static void check_usb_device(struct udev_device *device)
> > udev_device_get_parent_with_subsystem_devtype(
> > device, "usb", "usb_interface");
> >- if (usb_interface)
> >+ if (usb_interface) {
> > driver = udev_device_get_property_value(
> > usb_interface, "OFONO_DRIVER");
> >+ }
> > }
>
> No.
>
> > if (driver == NULL) {
> >@@ -1801,7 +1845,7 @@ static gboolean create_modem(gpointer key, gpointer value, gpointer user_data)
> > struct modem_info *modem = value;
> > const char *syspath = key;
> > unsigned int i;
> >-
> >+ DBG("");
> > if (modem->modem != NULL)
> > return FALSE;
>
> Doesn't belong to this patch, if at all.
>
> >
>
> /Jonas
Hi Jonas,
Thanks for the review, this patch is all wrong because it's not the good one...litlle bit tired, i'm sorry.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 3/3] sim800: add udev detection.
@ 2018-11-07 14:00 Clement Viel
2018-11-07 14:02 ` Jonas Bonn
0 siblings, 1 reply; 12+ messages in thread
From: Clement Viel @ 2018-11-07 14:00 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 501 bytes --]
---
plugins/udevng.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/plugins/udevng.c b/plugins/udevng.c
index ff6e1fc..d0a849d 100644
--- a/plugins/udevng.c
+++ b/plugins/udevng.c
@@ -1308,6 +1308,7 @@ static struct {
{ "calypso", setup_serial_modem },
{ "cinterion", setup_serial_modem },
{ "nokiacdma", setup_serial_modem },
+ { "sim800", setup_serial_modem },
{ "sim900", setup_serial_modem },
{ "wavecom", setup_wavecom },
{ "tc65", setup_tc65 },
--
2.7.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] sim800: add udev detection.
2018-11-07 14:00 [PATCH 3/3] sim800: add udev detection Clement Viel
@ 2018-11-07 14:02 ` Jonas Bonn
2018-11-07 14:08 ` Clement Viel
0 siblings, 1 reply; 12+ messages in thread
From: Jonas Bonn @ 2018-11-07 14:02 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 744 bytes --]
On 07/11/2018 15:00, Clement Viel wrote:
> ---
> plugins/udevng.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/plugins/udevng.c b/plugins/udevng.c
> index ff6e1fc..d0a849d 100644
> --- a/plugins/udevng.c
> +++ b/plugins/udevng.c
> @@ -1308,6 +1308,7 @@ static struct {
> { "calypso", setup_serial_modem },
> { "cinterion", setup_serial_modem },
> { "nokiacdma", setup_serial_modem },
> + { "sim800", setup_serial_modem },
Right... that's much simpler. And given that you are extending the
sim900 driver to handle the sim800 model, you don't need the above line
either.
/Jonas
> { "sim900", setup_serial_modem },
> { "wavecom", setup_wavecom },
> { "tc65", setup_tc65 },
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] sim800: add udev detection.
2018-11-07 14:02 ` Jonas Bonn
@ 2018-11-07 14:08 ` Clement Viel
2018-11-08 16:59 ` Denis Kenzior
0 siblings, 1 reply; 12+ messages in thread
From: Clement Viel @ 2018-11-07 14:08 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 1001 bytes --]
On Wed, Nov 07, 2018 at 03:02:54PM +0100, Jonas Bonn wrote:
>
>
> On 07/11/2018 15:00, Clement Viel wrote:
> >---
> > plugins/udevng.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> >diff --git a/plugins/udevng.c b/plugins/udevng.c
> >index ff6e1fc..d0a849d 100644
> >--- a/plugins/udevng.c
> >+++ b/plugins/udevng.c
> >@@ -1308,6 +1308,7 @@ static struct {
> > { "calypso", setup_serial_modem },
> > { "cinterion", setup_serial_modem },
> > { "nokiacdma", setup_serial_modem },
> >+ { "sim800", setup_serial_modem },
>
> Right... that's much simpler. And given that you are extending the sim900
> driver to handle the sim800 model, you don't need the above line either.
>
> /Jonas
>
> > { "sim900", setup_serial_modem },
> > { "wavecom", setup_wavecom },
> > { "tc65", setup_tc65 },
> >
I think I must keep this line because the udev rule added by the user will specify that modem is "OFONO_DRIVER=sim800" so udevng.c must know what to do with that.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] sim800: add udev detection.
2018-11-07 14:08 ` Clement Viel
@ 2018-11-08 16:59 ` Denis Kenzior
2018-11-12 14:16 ` Clement Viel
2018-11-12 14:21 ` Clement Viel
0 siblings, 2 replies; 12+ messages in thread
From: Denis Kenzior @ 2018-11-08 16:59 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 596 bytes --]
Hi Clement,
>>> + { "sim800", setup_serial_modem }, > I think I must keep this line because the udev rule added by the user
will specify that modem is "OFONO_DRIVER=sim800" so udevng.c must know
what to do with that.
This is kind of weird that we have a driver line for a modem driver that
doesn't exist. We never do things this way. Why don't you just modify
the driver instructions to have OFONO_DRIVER=sim900 instead?
Also, we could simply rename the sim900 driver into sim_x00 or
simcom_serial or something too. That could make things a bit more clear.
Regards,
-Denis
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] sim800: add udev detection.
2018-11-08 16:59 ` Denis Kenzior
@ 2018-11-12 14:16 ` Clement Viel
2018-11-12 16:34 ` Denis Kenzior
2018-11-12 14:21 ` Clement Viel
1 sibling, 1 reply; 12+ messages in thread
From: Clement Viel @ 2018-11-12 14:16 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 1141 bytes --]
On Thu, Nov 08, 2018 at 10:59:30AM -0600, Denis Kenzior wrote:
Hi Denis,
> Hi Clement,
> >>>+ { "sim800", setup_serial_modem }, > I think I must keep this line
> >>>because the udev rule added by the user
> will specify that modem is "OFONO_DRIVER=sim800" so udevng.c must know what
> to do with that.
> This is kind of weird that we have a driver line for a modem driver that
> doesn't exist. We never do things this way. Why don't you just modify the
> driver instructions to have OFONO_DRIVER=sim900 instead?
>
I agree with you, I was thinking the sim800 must be mentioned to avoid confusing
doc/sim900.txt to mention that sim800 is supported by this driver. Then, keep in
the users.
> Also, we could simply rename the sim900 driver into sim_x00 or simcom_serial
> or something too. That could make things a bit more clear.
I think we can keep that in mind for further development.
>
> Regards,
> -Denis
Just tell me whether I should add mention of sim800 in sim900.txt or rename sim900.txt
in simcom.txt to mention the two drivers. Then I'll upload the patchset with doc in it.
Regards
Clement
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] sim800: add udev detection.
2018-11-08 16:59 ` Denis Kenzior
2018-11-12 14:16 ` Clement Viel
@ 2018-11-12 14:21 ` Clement Viel
1 sibling, 0 replies; 12+ messages in thread
From: Clement Viel @ 2018-11-12 14:21 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 786 bytes --]
On Thu, Nov 08, 2018 at 10:59:30AM -0600, Denis Kenzior wrote:
> Hi Clement,
> >>>+ { "sim800", setup_serial_modem }, > I think I must keep this line
> >>>because the udev rule added by the user
> will specify that modem is "OFONO_DRIVER=sim800" so udevng.c must know what
> to do with that.
> This is kind of weird that we have a driver line for a modem driver that
> doesn't exist. We never do things this way. Why don't you just modify the
> driver instructions to have OFONO_DRIVER=sim900 instead?
>
> Also, we could simply rename the sim900 driver into sim_x00 or simcom_serial
> or something too. That could make things a bit more clear.
>
> Regards,
> -Denis
Moreover if we go for the OFONO_DRIVER=sim900 solution, plugins/udevng.c will stay
the same.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 3/3] sim800: add udev detection.
2018-11-12 14:16 ` Clement Viel
@ 2018-11-12 16:34 ` Denis Kenzior
0 siblings, 0 replies; 12+ messages in thread
From: Denis Kenzior @ 2018-11-12 16:34 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 370 bytes --]
Hi Clement,
> Just tell me whether I should add mention of sim800 in sim900.txt or rename sim900.txt
> in simcom.txt to mention the two drivers. Then I'll upload the patchset with doc in it.
I don't feel strongly either way, but if I had to pick, I say lets
rename sim900.txt into simcom.txt and maintain all driver instructions
there.
Regards,
-Denis
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2018-11-12 16:34 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-11-07 14:00 [PATCH 3/3] sim800: add udev detection Clement Viel
2018-11-07 14:02 ` Jonas Bonn
2018-11-07 14:08 ` Clement Viel
2018-11-08 16:59 ` Denis Kenzior
2018-11-12 14:16 ` Clement Viel
2018-11-12 16:34 ` Denis Kenzior
2018-11-12 14:21 ` Clement Viel
-- strict thread matches above, loose matches on Subject: below --
2018-11-07 10:14 [PATCH 1/3] sim800: add support for sim800 modem Clement Viel
2018-11-07 10:14 ` [PATCH 3/3] sim800: add udev detection Clement Viel
2018-11-07 13:30 ` Jonas Bonn
2018-11-07 13:58 ` Clement Viel
2018-10-15 17:27 [PATCH 1/3] sim800: add support for sim800 modem Clement Viel
2018-10-15 17:27 ` [PATCH 3/3] sim800: add udev detection Clement Viel
2018-10-03 13:28 [PATCH 1/3] sim800: add support for sim800 modem Clement Viel
2018-10-03 13:28 ` [PATCH 3/3] sim800: add udev detection Clement Viel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox