diff --git a/wifimngr.c b/wifimngr.c
index 88fe915d9a2c65b352e48154d83d9975d0473cf2..d4b1c7ab242474ab5e328087df2462e9b2bb5b13 100644
--- a/wifimngr.c
+++ b/wifimngr.c
@@ -2425,7 +2425,6 @@ int ubus_add_interface_object(struct wifimngr *w,
 #endif	// if 0
 
 #define MAX_WIFI_METHODS	8
-
 int add_wifi_methods(struct ubus_object *wifi_obj)
 {
 	int n_methods = 0;
@@ -2435,16 +2434,9 @@ int add_wifi_methods(struct ubus_object *wifi_obj)
 	if (!wifi_methods)
 		return -ENOMEM;
 
-#define UBUS_METHOD_ADD(_tab, iter, __m)				\
-do {									\
-	struct ubus_method ___m = __m;					\
-	memcpy(&_tab[iter++], &___m, sizeof(struct ubus_method));	\
-} while(0)
-
 	UBUS_METHOD_ADD(wifi_methods, n_methods,
 		UBUS_METHOD_NOARG("status", wl_status));
 
-#undef UBUS_METHOD_ADD
 	wifi_obj->methods = wifi_methods;
 	wifi_obj->n_methods = n_methods;
 }
@@ -2499,13 +2491,6 @@ static int add_radio_methods(struct ubus_object *radio_obj,
 	if (!radio_methods)
 		return -ENOMEM;
 
-#define UBUS_METHOD_ADD(_tab, iter, __m)				\
-do {									\
-	struct ubus_method ___m = __m;					\
-	memcpy(&_tab[iter++], &___m, sizeof(struct ubus_method));	\
-} while(0)
-
-
 	UBUS_METHOD_ADD(radio_methods, n_methods,
 		UBUS_METHOD_NOARG("status", wl_radio_status));
 
@@ -2529,7 +2514,6 @@ do {									\
 	UBUS_METHOD_ADD(radio_methods, n_methods,
 		UBUS_METHOD("autochannel", wl_autochannel, wl_acs_policy));
 
-#undef UBUS_METHOD_ADD
 	radio_obj->methods = radio_methods;
 	radio_obj->n_methods = n_methods;
 }
@@ -2565,7 +2549,6 @@ int wifimngr_add_radio_object(struct wifimngr *w,
 }
 
 #define MAX_INTERFACE_METHODS	16
-
 static int add_interface_methods(struct ubus_object *interface_obj,
 				const char *ifname)
 {
@@ -2577,12 +2560,6 @@ static int add_interface_methods(struct ubus_object *interface_obj,
 	if (!interface_methods)
 		return -ENOMEM;
 
-#define UBUS_METHOD_ADD(_tab, iter, __m)				\
-do {									\
-	struct ubus_method ___m = __m;					\
-	memcpy(&_tab[iter++], &___m, sizeof(struct ubus_method));	\
-} while(0)
-
 	UBUS_METHOD_ADD(interface_methods, n_methods,
 			UBUS_METHOD_NOARG("status", wl_interface_status));
 
@@ -2638,7 +2615,6 @@ do {									\
 			UBUS_METHOD("vendor_ie_del", vsie_del, vsie_del_policy));
 #endif
 
-#undef UBUS_METHOD_ADD
 	interface_obj->methods = interface_methods;
 	interface_obj->n_methods = n_methods;
 }
@@ -2655,19 +2631,12 @@ static int add_interface_mgmt_methods(struct ubus_object *iface_mgmt_obj,
 	if (!iface_mgmt_methods)
 		return -ENOMEM;
 
-#define UBUS_METHOD_ADD(_tab, iter, __m)				\
-do {									\
-	struct ubus_method ___m = __m;					\
-	memcpy(&_tab[iter++], &___m, sizeof(struct ubus_method));	\
-} while(0)
-
 	UBUS_METHOD_ADD(iface_mgmt_methods, n_methods,
 			UBUS_METHOD_NOARG("link", wifi_iface_link));
 
 	UBUS_METHOD_ADD(iface_mgmt_methods, n_methods,
 			UBUS_METHOD_NOARG("qos", wifi_iface_qos));
 
-#undef UBUS_METHOD_ADD
 	iface_mgmt_obj->methods = iface_mgmt_methods;
 	iface_mgmt_obj->n_methods = n_methods;
 }
@@ -2684,16 +2653,9 @@ static int add_interface_diag_methods(struct ubus_object *iface_diag_obj,
 	if (!iface_diag_methods)
 		return -ENOMEM;
 
-#define UBUS_METHOD_ADD(_tab, iter, __m)				\
-do {									\
-	struct ubus_method ___m = __m;					\
-	memcpy(&_tab[iter++], &___m, sizeof(struct ubus_method));	\
-} while(0)
-
 	UBUS_METHOD_ADD(iface_diag_methods, n_methods,
 			UBUS_METHOD_NOARG("counters", wifi_iface_diag_counters));
 
-#undef UBUS_METHOD_ADD
 	iface_diag_obj->methods = iface_diag_methods;
 	iface_diag_obj->n_methods = n_methods;
 }
diff --git a/wifimngr.h b/wifimngr.h
index 1926b818c4050d54ece3c3cef89544fa440d9780..d710009fc3ebbc2c9d758b7875014365e7de4749 100644
--- a/wifimngr.h
+++ b/wifimngr.h
@@ -39,6 +39,13 @@ struct wifimngr {
 
 //#define wifimngr_add_object(w, u, n, type)  ubus_add_## type ##_object(w, u, n)
 
+#define UBUS_METHOD_ADD(_tab, iter, __m)				\
+do {									\
+	struct ubus_method ___m = __m;					\
+	memcpy(&_tab[iter++], &___m, sizeof(struct ubus_method));	\
+} while(0)
+
+
 extern int add_wifi_methods(struct ubus_object *wifi_obj);
 extern int add_wps_methods(struct ubus_object *wifi_obj);
 
diff --git a/wps.c b/wps.c
index 48cd7b987baf9a042ffe28d129998db335aad3a6..914895504e40b820799609e236d20daea01b87a6 100644
--- a/wps.c
+++ b/wps.c
@@ -158,6 +158,21 @@ static const struct blobmsg_policy pin_set_policy[__PIN_SET_MAX] = {
 	[PIN_SET_PIN] = { .name = "pin", .type = BLOBMSG_TYPE_STRING },
 };
 
+enum {
+	WPS_START_ATTR_IFNAME,
+	WPS_START_ATTR_MODE,
+	WPS_START_ATTR_ROLE,
+	WPS_START_ATTR_STA_PIN,
+	__WPS_START_ATTR_MAX,
+};
+
+static const struct blobmsg_policy wps_start_policy[__WPS_START_ATTR_MAX] = {
+	[WPS_START_ATTR_IFNAME] = { .name = "vif", .type = BLOBMSG_TYPE_STRING },
+	[WPS_START_ATTR_MODE] = { .name = "mode", .type = BLOBMSG_TYPE_STRING },
+	[WPS_START_ATTR_ROLE] = { .name = "role", .type = BLOBMSG_TYPE_STRING },
+	[WPS_START_ATTR_STA_PIN] = { .name = "pin", .type = BLOBMSG_TYPE_STRING },
+};
+
 char *wifi_get_main_interface(int freq_band, char *ifmain)
 {
 	char wdevs[16][16];
@@ -235,46 +250,60 @@ static int wps_status(struct ubus_context *ctx, struct ubus_object *obj,
 	return 0;
 }
 
-static int wps_start_pbc(struct ubus_context *ctx, struct ubus_object *obj,
+static int wps_start(struct ubus_context *ctx, struct ubus_object *obj,
 		  struct ubus_request_data *req, const char *method,
 		  struct blob_attr *msg)
 {
-	struct blob_attr *tb[__WL_MAX];
-	char ifname[16] = {0};
+	struct blob_attr *tb[__WPS_START_ATTR_MAX];
 	struct wps_param wps;
+	char ifname[16] = {0};
+	char role[16] = {0};
+	char mode[8] = {0};
+	unsigned long pin;
 
-	blobmsg_parse(vif_policy, __WL_MAX, tb, blob_data(msg), blob_len(msg));
-	if (!(tb[VIF_NAME])) {
+	blobmsg_parse(wps_start_policy, __WPS_START_ATTR_MAX,
+					tb, blob_data(msg), blob_len(msg));
+
+	if (!(tb[WPS_START_ATTR_IFNAME])) {
+		/* if interface not provided, assume 5Ghz main interface */
 		if (wifi_get_main_interface(5, ifname) == NULL)
 			return UBUS_STATUS_UNKNOWN_ERROR;
 	} else {
-		strncpy(ifname, blobmsg_data(tb[VIF_NAME]), 15);
+		strncpy(ifname, blobmsg_data(tb[WPS_START_ATTR_IFNAME]), 15);
 	}
 
-	wps.role = WPS_REGISTRAR;
-	wps.mode = WPS_MODE_PBC;
-
-	return wifi_start_wps(ifname, wps);
-}
-
-static int wps_start_pbc_client(struct ubus_context *ctx, struct ubus_object *obj,
-		  struct ubus_request_data *req, const char *method,
-		  struct blob_attr *msg)
-{
-	struct blob_attr *tb[__WL_MAX];
-	char ifname[16] = {0};
-	struct wps_param wps;
+	if (!(tb[WPS_START_ATTR_MODE])) {
+		wps.mode = WPS_MODE_PBC;
+	} else {
+		strncpy(mode, blobmsg_data(tb[WPS_START_ATTR_MODE]), 7);
+		if (!strcasecmp(mode, "pin"))
+			wps.mode = WPS_MODE_PIN;
+		else if (!strcasecmp(mode, "pbc"))
+			wps.mode = WPS_MODE_PBC;
+		else
+			return UBUS_STATUS_INVALID_ARGUMENT;
+	}
 
-	blobmsg_parse(vif_policy, __WL_MAX, tb, blob_data(msg), blob_len(msg));
-	if (!(tb[VIF_NAME])) {
-		if (wifi_get_main_interface(5, ifname) == NULL)
-			return UBUS_STATUS_UNKNOWN_ERROR;
+	if (!(tb[WPS_START_ATTR_ROLE])) {
+		wps.role = WPS_MODE_PBC;
 	} else {
-		strncpy(ifname, blobmsg_data(tb[VIF_NAME]), 15);
+		strncpy(role, blobmsg_data(tb[WPS_START_ATTR_ROLE]), 10);
+		if (!strcasecmp(role, "registrar"))
+			wps.role = WPS_REGISTRAR;
+		else if (!strcasecmp(mode, "enrollee"))
+			wps.role = WPS_ENROLLEE;
+		else
+			return UBUS_STATUS_INVALID_ARGUMENT;
 	}
 
-	wps.role = WPS_ENROLLEE;
-	wps.mode = WPS_MODE_PBC;
+	if (wps.mode == WPS_MODE_PIN && wps.role == WPS_REGISTRAR) {
+		/* expect enrollee's pin from user */
+		pin = strtoul(blobmsg_data(tb[WPS_START_ATTR_STA_PIN]), NULL, 10);
+		if (!wifi_is_wps_pin_valid(pin))
+			return UBUS_STATUS_INVALID_ARGUMENT;
+
+		wps.pin = pin;
+	}
 
 	return wifi_start_wps(ifname, wps);
 }
@@ -327,38 +356,7 @@ static int wps_checkpin(struct ubus_context *ctx, struct ubus_object *obj,
 	return UBUS_STATUS_OK;
 }
 
-static int wps_start_pin(struct ubus_context *ctx, struct ubus_object *obj,
-		  struct ubus_request_data *req, const char *method,
-		  struct blob_attr *msg)
-{
-	struct blob_attr *tb[__PIN_SET_MAX];
-	unsigned long pin;
-	char ifname[16] = {0};
-	struct wps_param wps;
-
-	blobmsg_parse(pin_set_policy, __PIN_SET_MAX, tb, blob_data(msg), blob_len(msg));
-	if (!(tb[PIN_SET_IFNAME])) {
-		if (wifi_get_main_interface(5, ifname) == NULL)
-			return UBUS_STATUS_UNKNOWN_ERROR;
-	} else {
-		strncpy(ifname, blobmsg_data(tb[PIN_SET_IFNAME]), 15);
-	}
-
-	if (!(tb[PIN_SET_PIN]))
-		return UBUS_STATUS_INVALID_ARGUMENT;
-
-	pin = strtoul(blobmsg_data(tb[PIN_SET_PIN]), NULL, 10);
-	if (!wifi_is_wps_pin_valid(pin))
-		return UBUS_STATUS_INVALID_ARGUMENT;
-
-	wps.role = WPS_REGISTRAR;
-	wps.mode = WPS_MODE_PIN;
-	wps.pin = pin;
-
-	return wifi_start_wps(ifname, wps);
-}
-
-static int wps_setpin(struct ubus_context *ctx, struct ubus_object *obj,
+static int wps_set_ap_pin(struct ubus_context *ctx, struct ubus_object *obj,
 		  struct ubus_request_data *req, const char *method,
 		  struct blob_attr *msg)
 {
@@ -387,7 +385,7 @@ static int wps_setpin(struct ubus_context *ctx, struct ubus_object *obj,
 	return UBUS_STATUS_OK;
 }
 
-static int wps_showpin(struct ubus_context *ctx, struct ubus_object *obj,
+static int wps_show_ap_pin(struct ubus_context *ctx, struct ubus_object *obj,
 		  struct ubus_request_data *req, const char *method,
 		  struct blob_attr *msg)
 {
@@ -439,32 +437,7 @@ static int wps_stop(struct ubus_context *ctx, struct ubus_object *obj,
 	return 0;
 }
 
-#if 0
-struct ubus_method wps_object_methods[] = {
-	UBUS_METHOD("status", wps_status, vif_policy),
-	UBUS_METHOD("pbc", wps_start_pbc, vif_policy),
-	UBUS_METHOD("pbc_client", wps_start_pbc_client, vif_policy),
-	UBUS_METHOD_NOARG("genpin", wps_genpin),
-	UBUS_METHOD("checkpin", wps_checkpin, pin_policy),
-	UBUS_METHOD("stapin", wps_start_pin, pin_set_policy),
-	UBUS_METHOD("setpin", wps_setpin, pin_set_policy),
-	UBUS_METHOD("showpin", wps_showpin, vif_policy),
-	UBUS_METHOD_NOARG("stop", wps_stop),
-};
-
-struct ubus_object_type wps_object_type =
-	UBUS_OBJECT_TYPE("wps", wps_object_methods);
-
-struct ubus_object wps_object = {
-	.name = "router.wps",
-	.type = &wps_object_type,
-	.methods = wps_object_methods,
-	.n_methods = ARRAY_SIZE(wps_object_methods),
-};
-#endif
-
 #define MAX_WPS_METHODS		10
-
 int add_wps_methods(struct ubus_object *wifix)
 {
 	int n_methods = 0;
@@ -475,77 +448,35 @@ int add_wps_methods(struct ubus_object *wifix)
 	if (!wps_methods)
 		return -ENOMEM;
 
-#define UBUS_METHOD_ADD(_tab, iter, __m)				\
-do {									\
-	struct ubus_method ___m = __m;					\
-	memcpy(&_tab[iter++], &___m, sizeof(struct ubus_method));	\
-} while(0)
-
-
-	UBUS_METHOD_ADD(wps_methods, n_methods,
-			UBUS_METHOD("status", wps_status, vif_policy));
-
-	UBUS_METHOD_ADD(wps_methods, n_methods,
-		UBUS_METHOD("pbc", wps_start_pbc, vif_policy));
-
-	/* TODO: rename method to 'pin'.
-	 * Better to have a single wps_start method as -
-	 *    start {"mode":"pbc|pin", ["role":"enrollee|registrar"]}
+	/* Usage -
+	 * start {"mode":"pbc|pin", "role":"enrollee|registrar", "pin":"pin"}
 	 * where default mode = pbc and default role = registrar.
+	 * If role = registrar, and mode = pin, then enrollee's pin
+	 * should be provided through the 'pin' attribute.
 	 */
 	UBUS_METHOD_ADD(wps_methods, n_methods,
-		UBUS_METHOD("stapin", wps_start_pin, pin_set_policy));
+		UBUS_METHOD("start", wps_start, wps_start_policy));
 
 	UBUS_METHOD_ADD(wps_methods, n_methods,
-		UBUS_METHOD("pbc_client", wps_start_pbc_client, vif_policy));
+		UBUS_METHOD_NOARG("stop", wps_stop));
+
 
 	UBUS_METHOD_ADD(wps_methods, n_methods,
-			UBUS_METHOD_NOARG("genpin", wps_genpin));
+			UBUS_METHOD("status", wps_status, vif_policy));
 
 	UBUS_METHOD_ADD(wps_methods, n_methods,
-			UBUS_METHOD("checkpin", wps_checkpin, pin_policy));
+			UBUS_METHOD_NOARG("generate_pin", wps_genpin));
 
 	UBUS_METHOD_ADD(wps_methods, n_methods,
-		UBUS_METHOD("setpin", wps_setpin, pin_set_policy));
+			UBUS_METHOD("validate_pin", wps_checkpin, pin_policy));
 
 	UBUS_METHOD_ADD(wps_methods, n_methods,
-		UBUS_METHOD("showpin", wps_showpin, vif_policy));
+		UBUS_METHOD("showpin", wps_show_ap_pin, vif_policy));
 
 	UBUS_METHOD_ADD(wps_methods, n_methods,
-		UBUS_METHOD_NOARG("stop", wps_stop));
+		UBUS_METHOD("setpin", wps_set_ap_pin, pin_set_policy));
+
 
 	wifix->methods = wps_methods;
 	wifix->n_methods = n_methods;
 }
-
-#if 0
-int wifimngr_add_wps_object(struct wifimngr *w,
-				struct ubus_context *ctx,
-				const char *ifname)
-{
-	struct wifi_ubus_object *wobj;
-	int ret;
-	char objname[32] = {0};
-
-	wobj = calloc(1, sizeof(struct wifi_ubus_object));
-	if (!wobj)
-		return -ENOMEM;
-
-	snprintf(objname, 25, "wifi.%s", ifname);
-	wobj->obj.name = strdup(objname);
-	add_interface_methods(&wobj->obj, ifname);
-
-	wobj->obj_type.name = wobj->obj.name;
-	wobj->obj_type.n_methods = wobj->obj.n_methods;
-	wobj->obj_type.methods = wobj->obj.methods;
-	wobj->obj.type = &wobj->obj_type;
-
-	ret = ubus_add_object(ctx, &wobj->obj);
-	if (!ret) {
-		/* fprintf(stderr, "Added '%s' radio obj\n", wobj->obj.name); */
-		list_add_tail(&wobj->list, &w->iflist);
-	}
-
-	return ret;
-}
-#endif