Hi Pekka,
On 08/13/2010 01:31 PM, pekka.pe...@nokia.com wrote:
> From: Pekka Pessi <pekka.pe...@nokia.com>
>
> The N900 modem is based on isimodem. It works natively on N900 with Maemo or
> Meego kernel.
>
> The configuration option --disable-n900modem can be used to disable it (if,
> for instance, using ofono with Maemo 5 userspace).
Just had a quick look and this caught my eye:
> +static int n900modem_init(void)
> +{
> + isi_devinfo_init();
> + isi_phonebook_init();
> + isi_netreg_init();
> + isi_voicecall_init();
> + isi_sms_init();
> + isi_cbs_init();
> + isi_sim_init();
> + isi_ssn_init();
> + isi_ussd_init();
> + isi_call_forwarding_init();
> + isi_call_settings_init();
> + isi_call_barring_init();
> + isi_call_meter_init();
> + isi_radio_settings_init();
> +
> + ofono_modem_driver_register(&driver);
> +
> + return 0;
> +}
> +
> +static void n900modem_exit(void)
> +{
> + ofono_modem_driver_unregister(&driver);
> +
> + isi_devinfo_exit();
> + isi_phonebook_exit();
> + isi_netreg_exit();
> + isi_voicecall_exit();
> + isi_sms_exit();
> + isi_cbs_exit();
> + isi_sim_exit();
> + isi_ssn_exit();
> + isi_ussd_exit();
> + isi_call_forwarding_exit();
> + isi_call_settings_exit();
> + isi_call_barring_exit();
> + isi_call_meter_exit();
> + isi_radio_settings_exit();
> +}
> +
Why are we doing this inside n900modem? Can't we simply:
1. Make isimodem/isimodem.c register these (just like atmodem, stemodem,
mbmmodem, etc does)
2. Rip out the actual modem driver out of isimodem/isimodem.c
3. Have plugins/n900.c contain the modem driver?
> +OFONO_PLUGIN_DEFINE(n900modem, "Nokia N900 modem driver", VERSION,
> + OFONO_PLUGIN_PRIORITY_HIGH, n900modem_init, n900modem_exit)
> diff --git a/plugins/usbpnmodem.c b/plugins/usbpnmodem.c
> index 68beb6f..9fcf0c8 100644
> --- a/plugins/usbpnmodem.c
> +++ b/plugins/usbpnmodem.c
> @@ -39,9 +39,22 @@
>
> static GPhonetNetlink *link = NULL;
>
> +static int match_ifname(char const *name, char const *ifname)
> +{
> + size_t namelen = strlen(name);
> +
> + if (strncmp(name, ifname, namelen) != 0)
> + return FALSE;
> +
> + if (ifname[namelen + strspn(ifname + namelen, "0123456789")] != '\0')
> + return FALSE;
> +
> + return TRUE;
> +}
> +
> /*
> * Add or remove isimodems
> - * when usbpn* phonet interfaces are added/removed
> + * when usbpn* or phonet* phonet interfaces are added/removed
> */
> static void usbpn_status_cb(GIsiModem *idx,
> GPhonetLinkState st,
> @@ -50,15 +63,25 @@ static void usbpn_status_cb(GIsiModem *idx,
> {
> struct ofono_modem *modem;
> int error;
> + char const *driver;
> + uint8_t address = 0;
>
> DBG("Phonet link %s (%u) is %s",
> ifname, g_isi_modem_index(idx),
> st == PN_LINK_REMOVED ? "removed" :
> st == PN_LINK_DOWN ? "down" : "up");
>
> - /* Expect phonet interface name usbpn<idx> */
> - if (strncmp(ifname, "usbpn", 5) ||
> - ifname[5 + strspn(ifname + 5, "0123456789")])
> + if (match_ifname("usbpn", ifname)) {
> + driver = "isimodem";
> + address = PN_DEV_PC;
> + } else if (strcmp("phonet0", ifname) == 0) {
> +#if HAVE_N900MODEM
> + driver = "n900modem";
> +#else
> + driver = "isimodem";
> +#endif
And this really needs to go...
> + address = PN_DEV_SOS;
> + } else
> return;
Regards,
-Denis
_______________________________________________
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
