Re: [PATCH v2 3/3] udev: add Telit UC864-G and update udev rules

[Marcel Holtmann <marcel@holtmann.org>]

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

Hi Bernard,

> *for now only check for Modem, add GPS later
> 
> Co-authored-by: Christopher Vogl <Christopher.Vogl@hale.at>
> ---
>  plugins/ofono.rules |    8 ++++++++
>  plugins/udev.c      |   31 +++++++++++++++++++++++++++++++
>  2 files changed, 39 insertions(+), 0 deletions(-)
> 
> diff --git a/plugins/ofono.rules b/plugins/ofono.rules
> index 5a36380..87ef065 100644
> --- a/plugins/ofono.rules
> +++ b/plugins/ofono.rules
> @@ -344,6 +344,14 @@ ATTRS{idVendor}=="12d1", ATTRS{idProduct}=="1485", ENV{OFONO_IFACE_NUM}=="02", E
>  ATTRS{idVendor}=="12d1", ATTRS{idProduct}=="1486", ENV{OFONO_IFACE_NUM}=="00", ENV{OFONO_HUAWEI_TYPE}="Modem"
>  ATTRS{idVendor}=="12d1", ATTRS{idProduct}=="1486", ENV{OFONO_IFACE_NUM}=="02", ENV{OFONO_HUAWEI_TYPE}="Pcui"
>  
> +#Telit UC864-G
> +ATTRS{idVendor}=="1bc7", ATTRS{idProduct}=="1004", ENV{OFONO_IFACE_NUM}=="00", ENV{OFONO_TELIT_TYPE}="Modem"
> +ATTRS{idVendor}=="1bc7", ATTRS{idProduct}=="1004", ENV{OFONO_IFACE_NUM}=="01", ENV{OFONO_TELIT_TYPE}="Aux"
> +ATTRS{idVendor}=="1bc7", ATTRS{idProduct}=="1004", ENV{OFONO_IFACE_NUM}=="02", ENV{OFONO_TELIT_TYPE}="GPS"
> +ATTRS{idVendor}=="1bc7", ATTRS{idProduct}=="1004", ENV{OFONO_IFACE_NUM}=="03", ENV{OFONO_TELIT_TYPE}="UART"
> +

I can actually look this up, but which ones are you planning to use. I
can see the Modem, Aux and GPS, but what is the UART for?

> +ATTRS{idVendor}=="1bc7", ENV{OFONO_DRIVER}="telit"
> +
>  LABEL="ofono_tty_end"
>  
>  # ISI/Phonet drivers
> diff --git a/plugins/udev.c b/plugins/udev.c
> index cbb596d..28fff72 100644
> --- a/plugins/udev.c
> +++ b/plugins/udev.c
> @@ -588,6 +588,35 @@ static void add_linktop(struct ofono_modem *modem,
>  	}
>  }
>  
> +static void add_telit(struct ofono_modem *modem,
> +					struct udev_device *udev_device)
> +{
> +	struct udev_list_entry *entry;
> +	const char *devnode;
> +	gboolean found = FALSE;
> +
> +	DBG("modem %p", modem);
> +
> +	entry = udev_device_get_properties_list_entry(udev_device);
> +	while (entry) {
> +		const char *name = udev_list_entry_get_name(entry);
> +		const char *value = udev_list_entry_get_value(entry);
> +
> +		if (g_str_equal(name, "OFONO_TELIT_TYPE") == TRUE &&
> +					g_str_equal(value, "Modem") == TRUE) {
> +			found = TRUE;
> +		}
> +		entry = udev_list_entry_get_next(entry);
> +	}
> +
> +	if (found == FALSE)
> +		return;
> +
> +	devnode = udev_device_get_devnode(udev_device);
> +	ofono_modem_set_string(modem, "Modem", devnode);
> +	ofono_modem_register(modem);
> +}
> +

You can not really do it like this. You have to wait to register the
modem until all mandatory TTYs are present. Especially in case you
require two or more TTYs, which it seems you will.

Check the other ones like Novatel for examples.

>  static void add_modem(struct udev_device *udev_device)
>  {
>  	struct ofono_modem *modem;
> @@ -682,6 +711,8 @@ done:
>  		add_calypso(modem, udev_device);
>  	else if (g_strcmp0(driver, "tc65") == 0)
>  		add_tc65(modem, udev_device);
> +	else if (g_strcmp0(driver, "telit") == 0)
> +		add_telit(modem, udev_device);
>  	else if (g_strcmp0(driver, "nokiacdma") == 0)
>  		add_nokiacdma(modem, udev_device);
>          else if (g_strcmp0(driver, "linktop") == 0)

Regards

Marcel