Hi Iiro, 2011/8/3 <iiro.kaihlani...@nokia.com>: > diff --git a/drivers/isimodem/network-registration.c > b/drivers/isimodem/network-registration.c > index cc48579..ffcfab4 100644 > --- a/drivers/isimodem/network-registration.c > +++ b/drivers/isimodem/network-registration.c > @@ -946,6 +946,84 @@ error: > g_free(cbd); > } > > +static void cs_access_config_resp_cb(const GIsiMessage *msg, void *data) > +{ > + struct ofono_netreg *netreg = data; > + struct netreg_data *nd = ofono_netreg_get_data(netreg); > + GIsiSubBlockIter iter; > + > + DBG(""); > + if (g_isi_msg_id(msg) != NET_NW_ACCESS_CONF_RESP) > + return;
Indent with a single tab, no spaces allowed. This applies to the rest of the patch as well. > + for (g_isi_sb_iter_init(&iter, msg, 2); > + g_isi_sb_iter_is_valid(&iter); > + g_isi_sb_iter_next(&iter)) > + { > + uint8_t id = g_isi_sb_iter_get_id(&iter); > + uint8_t mode; > + > + DBG("SB=%02X", id); > + if ((id == 0x55) || (id == 0x59)) > + { > + g_isi_sb_iter_get_byte(&iter, &mode, 2); > + DBG("Reg %X", mode); > + } > + else if ((id == 0x56) || (id == 0x5A)) > + { > + g_isi_sb_iter_get_byte(&iter, &mode, 2); > + DBG("Roam %X", mode); > + } These subblock IDs need to be added in network.h. I would also recommend a switch statement here for more readability. > + else > + { > + DBG("Unknown subblock"); > + } > + } Regarding the entire for and if blocks above and elsewhere in the patch, the style in oFono is to attach curly brackets on the same line as the for/if/while statement. Please take a look at doc/coding-style.txt. To catch style errors early, I would recommend also using the checkpatch.pl script before submitting. > +} > + > +static void cs_state_resp_cb(const GIsiMessage *msg, void *data) > +{ > + struct ofono_netreg *netreg = data; > + struct netreg_data *nd = ofono_netreg_get_data(netreg); > + uint8_t code; > + > + DBG(""); > + if (g_isi_msg_id(msg) != NET_CS_STATE_RESP) > + return; > + > + if (!g_isi_msg_data_get_byte(msg, 0, &code)) > + return; > + > + if (code != NET_CAUSE_OK) > + { > + DBG("Failed with cause=%X", code); > + return; > + } > + > + if (!g_isi_msg_data_get_byte(msg, 1, &code)) > + return; > + DBG("CS STATE=%X", code); > + > + if (code == NET_CS_INACTIVE) > + { > + DBG("CS INACTIVE - DO POWER ON NOT IMPLEMENTED!!!!!!"); No shouting please ;) > + } > + else > + { > + /* Enable registration and roaming */ > + const uint8_t req[] = { > + NET_NW_ACCESS_CONF_REQ, 0, 2, > + /* Subblock 1 */ > + 0x59, 4, 1, 0, > + /* Subblock 2 */ > + 0x5A, 4, 1, 0, Should really add these subblock IDs in network.h. > + }; > + > + DBG("CS ACTIVE - Check access config"); > + g_isi_client_send(nd->client, req, sizeof(req), > cs_access_config_resp_cb, netreg, NULL); Please take a look at coding style rule O2, and rather than putting all this in an else statement, just return above if code == NET_CS_INACTIVE. By the way, is CS access config ever set if when probing the CS state is NET_CS_INACTIVE? Should we also subscribe to the equivalent indication to drive this code? > + } > +} > + > static void subscribe_indications(GIsiClient *cl, void *data) > { > g_isi_client_ind_subscribe(cl, NET_RSSI_IND, rssi_ind_cb, data); > @@ -994,6 +1072,11 @@ static void pn_modem_network_reachable_cb(const > GIsiMessage *msg, void *data) > struct ofono_netreg *netreg = data; > struct netreg_data *nd = ofono_netreg_get_data(netreg); > > + const uint8_t req[] = { > + NET_CS_STATE_REQ, > + }; > + > + Indent with tabs here. Cheers, Aki _______________________________________________ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono