Re: [PATCH 2/3] udev: Handle Linktop data card

[Denis Kenzior <denkenz@gmail.com>]

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

Hi Amit,

On 02/09/2011 11:15 PM, Amit Mendapara wrote:
> Signed-off-by: Amit Mendapara <mendapara.amit@gmail.com>
> ---
>  plugins/udev.c |   32 ++++++++++++++++++++++++++++++++
>  1 files changed, 32 insertions(+), 0 deletions(-)
> 

Patch has been applied, thanks.

Some comments on further improvements:

> +static void add_linktop(struct ofono_modem *modem,
> +					struct udev_device *udev_device)
> +{
> +	const char *devnode, *intfnum;
> +	struct udev_device *parent;
> +	int registered;
> +
> +	DBG("modem %p", modem);
> +
> +	registered = ofono_modem_get_integer(modem, "Registered");
> +	if (registered != 0)
> +		return;
> +
> +	parent = udev_device_get_parent(udev_device);
> +	intfnum = udev_device_get_sysattr_value(parent, "bInterfaceNumber");
> +
> +	DBG("intfnum %s", intfnum);
> +
> +	if (g_strcmp0(intfnum, "01") == 0) {
> +		devnode = udev_device_get_devnode(udev_device);
> +		ofono_modem_set_string(modem, "Modem", devnode);
> +	} else if (g_strcmp0(intfnum, "03") == 0) {
> +		devnode = udev_device_get_devnode(udev_device);
> +		ofono_modem_set_string(modem, "Control", devnode);
> +
> +		ofono_modem_set_integer(modem, "Registered", 1);
> +		ofono_modem_register(modem);
> +	}

Instead of hardcoding this info, you might want to use an approach
similar to how the Huawei udev rules are structured.  However, this
really depends on how many variations of this Linktop card there are.
Since there's only 1 right now, this is fine.

Regards,
-Denis