[PATCH 0/3] drivers:staging:ti-st: patches

[PATCH 0/3] drivers:staging:ti-st: patches

[pavan_savoy]

From: Pavan Savoy <pavan_savoy@ti.com>

The following patches cleanup bit of a mess and also adds functionality to protocol drivers.
with the 3rd patch now providing context to even the protocol drivers, the single device limit
or support for multiple devices would be easier to implement.

These patches depend on the previously submitted
0001-drivers-staging-ti-st-make-use-of-linux-err-codes.patch
commit d39d49b393d94f4137cee4f64526a4695352f183

Pavan Savoy (3):
  drivers:staging:ti-st: smarten, reduce logs
  drivers:staging:ti-st: cleanup code comments
  drivers:staging:ti-st: give proto drivers context

 drivers/staging/ti-st/bt_drv.c  |   23 +++++---
 drivers/staging/ti-st/st.h      |   52 +++++++++--------
 drivers/staging/ti-st/st_core.c |  118 +++++++++++++++++++--------------------
 drivers/staging/ti-st/st_core.h |   74 +++++++++++++++++--------
 drivers/staging/ti-st/st_kim.c  |   73 ++++++++++++++----------
 drivers/staging/ti-st/st_kim.h  |   77 ++++++++++++++++---------
 drivers/staging/ti-st/st_ll.c   |    4 +-
 drivers/staging/ti-st/st_ll.h   |    9 +++-
 8 files changed, 255 insertions(+), 175 deletions(-)

[PATCH 1/3] drivers:staging:ti-st: smarten, reduce logs

[pavan_savoy]

From: Pavan Savoy <pavan_savoy@ti.com>

Replace looping on the data buffers and printk-ing by
print_hex_dump.
Also replace most of the pr_info by pr_debug to reduce logging at
default loglevels (7 in our case..)

Signed-off-by: Pavan Savoy <pavan_savoy@ti.com>
---
 drivers/staging/ti-st/st_core.c |   56 ++++++++++++++++----------------------
 drivers/staging/ti-st/st_kim.c  |   21 ++++++--------
 drivers/staging/ti-st/st_ll.c   |    4 +-
 3 files changed, 35 insertions(+), 46 deletions(-)

diff --git a/drivers/staging/ti-st/st_core.c b/drivers/staging/ti-st/st_core.c
index 0ad8cec..7806843 100644
--- a/drivers/staging/ti-st/st_core.c
+++ b/drivers/staging/ti-st/st_core.c
@@ -38,6 +38,7 @@
 #include "st_ll.h"
 #include "st.h"
 
+#define VERBOSE
 #ifdef DEBUG
 /* strings to be used for rfkill entries and by
  * ST Core to be used for sysfs debug entry
@@ -61,7 +62,7 @@ void (*st_recv) (void*, const unsigned char*, long);
 bool is_protocol_list_empty(void)
 {
 	unsigned char i = 0;
-	pr_info(" %s ", __func__);
+	pr_debug(" %s ", __func__);
 	for (i = 0; i < ST_MAX; i++) {
 		if (st_gdata->list[i] != NULL)
 			return ST_NOTEMPTY;
@@ -81,9 +82,6 @@ bool is_protocol_list_empty(void)
 int st_int_write(struct st_data_s *st_gdata,
 	const unsigned char *data, int count)
 {
-#ifdef VERBOSE			/* for debug */
-	int i;
-#endif
 	struct tty_struct *tty;
 	if (unlikely(st_gdata == NULL || st_gdata->tty == NULL)) {
 		pr_err("tty unavailable to perform write");
@@ -91,10 +89,8 @@ int st_int_write(struct st_data_s *st_gdata,
 	}
 	tty = st_gdata->tty;
 #ifdef VERBOSE
-	printk(KERN_ERR "start data..\n");
-	for (i = 0; i < count; i++)	/* no newlines for each datum */
-		printk(" %x", data[i]);
-	printk(KERN_ERR "\n ..end data\n");
+	print_hex_dump(KERN_DEBUG, "<out<", DUMP_PREFIX_NONE,
+		16, 1, data, count, 0);
 #endif
 	return tty->ops->write(tty, data, count);
 
@@ -132,7 +128,6 @@ void st_send_frame(enum proto_type protoid, struct st_data_s *st_gdata)
 		pr_err(" proto stack %d's ->recv null", protoid);
 		kfree_skb(st_gdata->rx_skb);
 	}
-	pr_info(" done %s", __func__);
 	return;
 }
 
@@ -156,7 +151,7 @@ static inline int st_check_data_len(struct st_data_s *st_gdata,
 {
 	register int room = skb_tailroom(st_gdata->rx_skb);
 
-	pr_info("len %d room %d", len, room);
+	pr_debug("len %d room %d", len, room);
 
 	if (!len) {
 		/* Received packet has only packet header and
@@ -259,7 +254,7 @@ void st_int_recv(void *disc_data,
 
 				/* Waiting for complete packet ? */
 			case ST_BT_W4_DATA:
-				pr_info("Complete pkt received");
+				pr_debug("Complete pkt received");
 
 				/* Ask ST CORE to forward
 				 * the packet to protocol driver */
@@ -275,7 +270,7 @@ void st_int_recv(void *disc_data,
 				eh = (struct hci_event_hdr *)st_gdata->rx_skb->
 				    data;
 
-				pr_info("Event header: evt 0x%2.2x"
+				pr_debug("Event header: evt 0x%2.2x"
 					   "plen %d", eh->evt, eh->plen);
 
 				st_check_data_len(st_gdata, protoid, eh->plen);
@@ -439,7 +434,7 @@ void st_int_recv(void *disc_data,
 			break;
 		}
 	}
-	pr_info("done %s", __func__);
+	pr_debug("done %s", __func__);
 	return;
 }
 
@@ -451,7 +446,7 @@ struct sk_buff *st_int_dequeue(struct st_data_s *st_gdata)
 {
 	struct sk_buff *returning_skb;
 
-	pr_info("%s", __func__);
+	pr_debug("%s", __func__);
 	/* if the previous skb wasn't written completely
 	 */
 	if (st_gdata->tx_skb != NULL) {
@@ -475,7 +470,7 @@ void st_int_enqueue(struct st_data_s *st_gdata, struct sk_buff *skb)
 {
 	unsigned long flags = 0;
 
-	pr_info("%s", __func__);
+	pr_debug("%s", __func__);
 	/* this function can be invoked in more then one context.
 	 * so have a lock */
 	spin_lock_irqsave(&st_gdata->lock, flags);
@@ -508,7 +503,7 @@ void st_int_enqueue(struct st_data_s *st_gdata, struct sk_buff *skb)
 		break;
 	}
 	spin_unlock_irqrestore(&st_gdata->lock, flags);
-	pr_info("done %s", __func__);
+	pr_debug("done %s", __func__);
 	return;
 }
 
@@ -522,7 +517,7 @@ void st_tx_wakeup(struct st_data_s *st_data)
 {
 	struct sk_buff *skb;
 	unsigned long flags;	/* for irq save flags */
-	pr_info("%s", __func__);
+	pr_debug("%s", __func__);
 	/* check for sending & set flag sending here */
 	if (test_and_set_bit(ST_TX_SENDING, &st_data->tx_state)) {
 		pr_info("ST already sending");
@@ -674,7 +669,7 @@ long st_register(struct st_proto_s *new_proto)
 		 */
 		if ((st_gdata->protos_registered != ST_EMPTY) &&
 		    (test_bit(ST_REG_PENDING, &st_gdata->st_state))) {
-			pr_info(" call reg complete callback ");
+			pr_debug(" call reg complete callback ");
 			st_reg_complete(st_gdata, 0);
 		}
 		clear_bit(ST_REG_PENDING, &st_gdata->st_state);
@@ -721,7 +716,7 @@ long st_register(struct st_proto_s *new_proto)
 		spin_unlock_irqrestore(&st_gdata->lock, flags);
 		return err;
 	}
-	pr_info("done %s(%d) ", __func__, new_proto->type);
+	pr_debug("done %s(%d) ", __func__, new_proto->type);
 }
 EXPORT_SYMBOL_GPL(st_register);
 
@@ -734,7 +729,7 @@ long st_unregister(enum proto_type type)
 	unsigned long flags = 0;
 	struct st_data_s	*st_gdata;
 
-	pr_info("%s: %d ", __func__, type);
+	pr_debug("%s: %d ", __func__, type);
 
 	st_kim_ref(&st_gdata);
 	if (type < ST_BT || type >= ST_MAX) {
@@ -816,7 +811,7 @@ long st_write(struct sk_buff *skb)
 		return -1;
 	}
 #endif
-	pr_info("%d to be written", skb->len);
+	pr_debug("%d to be written", skb->len);
 	len = skb->len;
 
 	/* st_ll to decide where to enqueue the skb */
@@ -859,7 +854,7 @@ static int st_tty_open(struct tty_struct *tty)
 	 * installation of N_TI_WL ldisc is complete
 	 */
 	st_kim_complete(st_gdata->kim_data);
-	pr_info("done %s", __func__);
+	pr_debug("done %s", __func__);
 	return err;
 }
 
@@ -903,7 +898,7 @@ static void st_tty_close(struct tty_struct *tty)
 	st_gdata->rx_skb = NULL;
 	spin_unlock_irqrestore(&st_gdata->lock, flags);
 
-	pr_info("%s: done ", __func__);
+	pr_debug("%s: done ", __func__);
 }
 
 static void st_tty_receive(struct tty_struct *tty, const unsigned char *data,
@@ -911,11 +906,8 @@ static void st_tty_receive(struct tty_struct *tty, const unsigned char *data,
 {
 
 #ifdef VERBOSE
-	long i;
-	printk(KERN_ERR "incoming data...\n");
-	for (i = 0; i < count; i++)
-		printk(" %x", data[i]);
-	printk(KERN_ERR "\n.. data end\n");
+	print_hex_dump(KERN_DEBUG, ">in>", DUMP_PREFIX_NONE,
+		16, 1, data, count, 0);
 #endif
 
 	/*
@@ -923,7 +915,7 @@ static void st_tty_receive(struct tty_struct *tty, const unsigned char *data,
 	 * to KIM for validation
 	 */
 	st_recv(tty->disc_data, data, count);
-	pr_info("done %s", __func__);
+	pr_debug("done %s", __func__);
 }
 
 /* wake-up function called in from the TTY layer
@@ -932,7 +924,7 @@ static void st_tty_receive(struct tty_struct *tty, const unsigned char *data,
 static void st_tty_wakeup(struct tty_struct *tty)
 {
 	struct	st_data_s *st_gdata = tty->disc_data;
-	pr_info("%s ", __func__);
+	pr_debug("%s ", __func__);
 	/* don't do an wakeup for now */
 	clear_bit(TTY_DO_WRITE_WAKEUP, &tty->flags);
 
@@ -943,7 +935,7 @@ static void st_tty_wakeup(struct tty_struct *tty)
 static void st_tty_flush_buffer(struct tty_struct *tty)
 {
 	struct	st_data_s *st_gdata = tty->disc_data;
-	pr_info("%s ", __func__);
+	pr_debug("%s ", __func__);
 
 	kfree_skb(st_gdata->tx_skb);
 	st_gdata->tx_skb = NULL;
@@ -982,7 +974,7 @@ int st_core_init(struct st_data_s **core_data)
 		kfree(st_ldisc_ops);
 		return err;
 	}
-	pr_info("registered n_shared line discipline");
+	pr_debug("registered n_shared line discipline");
 
 	st_gdata = kzalloc(sizeof(struct st_data_s), GFP_KERNEL);
 	if (!st_gdata) {
diff --git a/drivers/staging/ti-st/st_kim.c b/drivers/staging/ti-st/st_kim.c
index 33fc4d0..5ab69d1 100644
--- a/drivers/staging/ti-st/st_kim.c
+++ b/drivers/staging/ti-st/st_kim.c
@@ -129,7 +129,7 @@ static inline int kim_check_data_len(struct kim_data_s *kim_gdata, int len)
 {
 	register int room = skb_tailroom(kim_gdata->rx_skb);
 
-	pr_info("len %d room %d", len, room);
+	pr_debug("len %d room %d", len, room);
 
 	if (!len) {
 		validate_firmware_response(kim_gdata);
@@ -170,7 +170,7 @@ void kim_int_recv(struct kim_data_s *kim_gdata,
 	struct hci_event_hdr *eh;
 	register int len = 0, type = 0;
 
-	pr_info("%s", __func__);
+	pr_debug("%s", __func__);
 	/* Decode received bytes here */
 	ptr = (char *)data;
 	if (unlikely(ptr == NULL)) {
@@ -192,7 +192,7 @@ void kim_int_recv(struct kim_data_s *kim_gdata,
 			switch (kim_gdata->rx_state) {
 				/* Waiting for complete packet ? */
 			case ST_BT_W4_DATA:
-				pr_info("Complete pkt received");
+				pr_debug("Complete pkt received");
 				validate_firmware_response(kim_gdata);
 				kim_gdata->rx_state = ST_W4_PACKET_TYPE;
 				kim_gdata->rx_skb = NULL;
@@ -201,7 +201,7 @@ void kim_int_recv(struct kim_data_s *kim_gdata,
 			case ST_BT_W4_EVENT_HDR:
 				eh = (struct hci_event_hdr *)kim_gdata->
 				    rx_skb->data;
-				pr_info("Event header: evt 0x%2.2x"
+				pr_debug("Event header: evt 0x%2.2x"
 					   "plen %d", eh->evt, eh->plen);
 				kim_check_data_len(kim_gdata, eh->plen);
 				continue;
@@ -242,7 +242,7 @@ static long read_local_version(struct kim_data_s *kim_gdata, char *bts_scr_name)
 	unsigned short version = 0, chip = 0, min_ver = 0, maj_ver = 0;
 	char read_ver_cmd[] = { 0x01, 0x01, 0x10, 0x00 };
 
-	pr_info("%s", __func__);
+	pr_debug("%s", __func__);
 
 	INIT_COMPLETION(kim_gdata->kim_rcvd);
 	if (4 != st_int_write(kim_gdata->core_data, read_ver_cmd, 4)) {
@@ -289,8 +289,6 @@ static long download_firmware(struct kim_data_s *kim_gdata)
 	register unsigned char *action_ptr = NULL;
 	unsigned char bts_scr_name[30] = { 0 };	/* 30 char long bts scr name? */
 
-	pr_info("%s", __func__);
-
 	err = read_local_version(kim_gdata, bts_scr_name);
 	if (err != 0) {
 		pr_err("kim: failed to read local ver");
@@ -314,7 +312,7 @@ static long download_firmware(struct kim_data_s *kim_gdata)
 	len -= sizeof(struct bts_header);
 
 	while (len > 0 && ptr) {
-		pr_info(" action size %d, type %d ",
+		pr_debug(" action size %d, type %d ",
 			   ((struct bts_action *)ptr)->size,
 			   ((struct bts_action *)ptr)->type);
 
@@ -327,8 +325,8 @@ static long download_firmware(struct kim_data_s *kim_gdata)
 				/* ignore remote change
 				 * baud rate HCI VS command */
 				pr_err
-				    (" change remote baud\
-				    rate command in firmware");
+				    (" change remote baud"
+				    " rate command in firmware");
 				break;
 			}
 
@@ -558,7 +556,6 @@ static ssize_t store_pid(struct device *dev, struct device_attribute
 			 *devattr, char *buf, size_t count)
 {
 	struct kim_data_s	*kim_gdata = dev_get_drvdata(dev);
-	pr_info("%s: pid %s ", __func__, buf);
 	sscanf(buf, "%ld", &kim_gdata->uim_pid);
 	/* to be made use by kim_start to signal SIGUSR2
 	 */
@@ -590,7 +587,7 @@ static ssize_t show_list(struct device *dev, struct device_attribute
 static int kim_toggle_radio(void *data, bool blocked)
 {
 	enum proto_type type = *((enum proto_type *)data);
-	pr_info(" %s: %d ", __func__, type);
+	pr_debug(" %s: %d ", __func__, type);
 
 	switch (type) {
 	case ST_BT:
diff --git a/drivers/staging/ti-st/st_ll.c b/drivers/staging/ti-st/st_ll.c
index 6bc0759..7a1fb6d 100644
--- a/drivers/staging/ti-st/st_ll.c
+++ b/drivers/staging/ti-st/st_ll.c
@@ -34,7 +34,7 @@ static void send_ll_cmd(struct st_data_s *st_data,
 
 static void ll_device_want_to_sleep(struct st_data_s *st_data)
 {
-	pr_info("%s", __func__);
+	pr_debug("%s", __func__);
 	/* sanity check */
 	if (st_data->ll_state != ST_LL_AWAKE)
 		pr_err("ERR hcill: ST_LL_GO_TO_SLEEP_IND"
@@ -101,7 +101,7 @@ void st_ll_wakeup(struct st_data_s *ll)
 /* called when ST Core wants the state */
 unsigned long st_ll_getstate(struct st_data_s *ll)
 {
-	pr_info(" returning state %ld", ll->ll_state);
+	pr_debug(" returning state %ld", ll->ll_state);
 	return ll->ll_state;
 }
 
-- 
1.5.6.3

[PATCH 2/3] drivers:staging:ti-st: cleanup code comments

[pavan_savoy]

From: Pavan Savoy <pavan_savoy@ti.com>

cleanup the code commenting in the headers/structures,
also cleanup few inline commenting in the function

Signed-off-by: Pavan Savoy <pavan_savoy@ti.com>
---
 drivers/staging/ti-st/st.h      |   44 +++++++++++------------
 drivers/staging/ti-st/st_core.c |   54 ++++++++++++++-------------
 drivers/staging/ti-st/st_core.h |   74 ++++++++++++++++++++++++++------------
 drivers/staging/ti-st/st_kim.c  |   52 ++++++++++++++++++---------
 drivers/staging/ti-st/st_kim.h  |   77 +++++++++++++++++++++++++--------------
 drivers/staging/ti-st/st_ll.h   |    9 ++++-
 6 files changed, 193 insertions(+), 117 deletions(-)

diff --git a/drivers/staging/ti-st/st.h b/drivers/staging/ti-st/st.h
index c4288aa..c96c01e 100644
--- a/drivers/staging/ti-st/st.h
+++ b/drivers/staging/ti-st/st.h
@@ -24,24 +24,24 @@
 #define ST_H
 
 #include <linux/skbuff.h>
-/*
- * st.h
- */
 
 /* TODO:
  * Move the following to tty.h upon acceptance
  */
 #define N_TI_WL	20	/* Ldisc for TI's WL BT, FM, GPS combo chips */
 
-/* some gpios have active high, others like fm have
- * active low
+/**
+ * enum kim_gpio_state - Few protocols such as FM have ACTIVE LOW
+ *	gpio states for their chip/core enable gpios
  */
 enum kim_gpio_state {
 	KIM_GPIO_INACTIVE,
 	KIM_GPIO_ACTIVE,
 };
-/*
- * the list of protocols on chip
+
+/**
+ * enum proto-type - The protocol on WiLink chips which share a
+ *	common physical interface like UART.
  */
 enum proto_type {
 	ST_BT,
@@ -50,28 +50,26 @@ enum proto_type {
 	ST_MAX,
 };
 
-/* per protocol structure
- * for BT/FM and GPS
+/**
+ * struct st_proto_s - Per Protocol structure from BT/FM/GPS to ST
+ * @type: type of the protocol being registered among the
+ *	available proto_type(BT, FM, GPS the protocol which share TTY).
+ * @recv: the receiver callback pointing to a function in the
+ *	protocol drivers called by the ST driver upon receiving
+ *	relevant data.
+ * @match_packet: reserved for future use, to make ST more generic
+ * @reg_complete_cb: callback handler pointing to a function in protocol
+ *	handler called by ST when the pending registrations are complete.
+ *	The registrations are marked pending, in situations when fw
+ *	download is in progress.
+ * @write: pointer to function in ST provided to protocol drivers from ST,
+ *	to be made use when protocol drivers have data to send to TTY.
  */
 struct st_proto_s {
 	enum proto_type type;
-/*
- * to be called by ST when data arrives
- */
 	long (*recv) (struct sk_buff *);
-/*
- * for future use, logic now to be in ST
- */
 	unsigned char (*match_packet) (const unsigned char *data);
-/*
- * subsequent registration return PENDING,
- * signalled complete by this callback function
- */
 	void (*reg_complete_cb) (char data);
-/*
- * write function, sent in as NULL and to be returned to
- * protocol drivers
- */
 	long (*write) (struct sk_buff *skb);
 };
 
diff --git a/drivers/staging/ti-st/st_core.c b/drivers/staging/ti-st/st_core.c
index 7806843..b20ab73 100644
--- a/drivers/staging/ti-st/st_core.c
+++ b/drivers/staging/ti-st/st_core.c
@@ -72,6 +72,7 @@ bool is_protocol_list_empty(void)
 	return ST_EMPTY;
 }
 #endif
+
 /* can be called in from
  * -- KIM (during fw download)
  * -- ST Core (during st_write)
@@ -131,7 +132,8 @@ void st_send_frame(enum proto_type protoid, struct st_data_s *st_gdata)
 	return;
 }
 
-/*
+/**
+ * st_reg_complete -
  * to call registration complete callbacks
  * of all protocol stack drivers
  */
@@ -185,8 +187,9 @@ static inline int st_check_data_len(struct st_data_s *st_gdata,
 	return 0;
 }
 
-/* internal function for action when wake-up ack
- * received
+/**
+ * st_wakeup_ack - internal function for action when wake-up ack
+ *	received
  */
 static inline void st_wakeup_ack(struct st_data_s *st_gdata,
 	unsigned char cmd)
@@ -209,9 +212,13 @@ static inline void st_wakeup_ack(struct st_data_s *st_gdata,
 	st_tx_wakeup(st_gdata);
 }
 
-/* Decodes received RAW data and forwards to corresponding
- * client drivers (Bluetooth,FM,GPS..etc).
- *
+/**
+ * st_int_recv - ST's internal receive function.
+ *	Decodes received RAW data and forwards to corresponding
+ *	client drivers (Bluetooth,FM,GPS..etc).
+ *	This can receive various types of packets,
+ *	HCI-Events, ACL, SCO, 4 types of HCI-LL PM packets
+ *	CH-8 packets from FM, CH-9 packets from GPS cores.
  */
 void st_int_recv(void *disc_data,
 	const unsigned char *data, long count)
@@ -438,41 +445,39 @@ void st_int_recv(void *disc_data,
 	return;
 }
 
-/* internal de-Q function
- * -- return previous in-completely written skb
- *  or return the skb in the txQ
+/**
+ * st_int_dequeue - internal de-Q function.
+ *	If the previous data set was not written
+ *	completely, return that skb which has the pending data.
+ *	In normal cases, return top of txq.
  */
 struct sk_buff *st_int_dequeue(struct st_data_s *st_gdata)
 {
 	struct sk_buff *returning_skb;
 
 	pr_debug("%s", __func__);
-	/* if the previous skb wasn't written completely
-	 */
 	if (st_gdata->tx_skb != NULL) {
 		returning_skb = st_gdata->tx_skb;
 		st_gdata->tx_skb = NULL;
 		return returning_skb;
 	}
-
-	/* de-Q from the txQ always if previous write is complete */
 	return skb_dequeue(&st_gdata->txq);
 }
 
-/* internal Q-ing function
- * will either Q the skb to txq or the tx_waitq
- * depending on the ST LL state
- *
- * lock the whole func - since ll_getstate and Q-ing should happen
- * in one-shot
+/**
+ * st_int_enqueue - internal Q-ing function.
+ *	Will either Q the skb to txq or the tx_waitq
+ *	depending on the ST LL state.
+ *	If the chip is asleep, then Q it onto waitq and
+ *	wakeup the chip.
+ *	txq and waitq needs protection since the other contexts
+ *	may be sending data, waking up chip.
  */
 void st_int_enqueue(struct st_data_s *st_gdata, struct sk_buff *skb)
 {
 	unsigned long flags = 0;
 
 	pr_debug("%s", __func__);
-	/* this function can be invoked in more then one context.
-	 * so have a lock */
 	spin_lock_irqsave(&st_gdata->lock, flags);
 
 	switch (st_ll_getstate(st_gdata)) {
@@ -483,16 +488,12 @@ void st_int_enqueue(struct st_data_s *st_gdata, struct sk_buff *skb)
 	case ST_LL_ASLEEP_TO_AWAKE:
 		skb_queue_tail(&st_gdata->tx_waitq, skb);
 		break;
-	case ST_LL_AWAKE_TO_ASLEEP:	/* host cannot be in this state */
+	case ST_LL_AWAKE_TO_ASLEEP:
 		pr_err("ST LL is illegal state(%ld),"
 			   "purging received skb.", st_ll_getstate(st_gdata));
 		kfree_skb(skb);
 		break;
-
 	case ST_LL_ASLEEP:
-		/* call a function of ST LL to put data
-		 * in tx_waitQ and wake_ind in txQ
-		 */
 		skb_queue_tail(&st_gdata->tx_waitq, skb);
 		st_ll_wakeup(st_gdata);
 		break;
@@ -502,6 +503,7 @@ void st_int_enqueue(struct st_data_s *st_gdata, struct sk_buff *skb)
 		kfree_skb(skb);
 		break;
 	}
+
 	spin_unlock_irqrestore(&st_gdata->lock, flags);
 	pr_debug("done %s", __func__);
 	return;
diff --git a/drivers/staging/ti-st/st_core.h b/drivers/staging/ti-st/st_core.h
index 53369f2..e0c32d1 100644
--- a/drivers/staging/ti-st/st_core.h
+++ b/drivers/staging/ti-st/st_core.h
@@ -36,59 +36,87 @@
 #define ST_REG_PENDING		3
 #define ST_WAITING_FOR_RESP	4
 
-/*
- * local data required for ST/KIM/ST-HCI-LL
+/**
+ * struct st_data_s - ST core internal structure
+ * @st_state: different states of ST like initializing, registration
+ *	in progress, this is mainly used to return relevant err codes
+ *	when protocol drivers are registering. It is also used to track
+ *	the recv function, as in during fw download only HCI events
+ *	can occur , where as during other times other events CH8, CH9
+ *	can occur.
+ * @tty: tty provided by the TTY core for line disciplines.
+ * @ldisc_ops: the procedures that this line discipline registers with TTY.
+ * @tx_skb: If for some reason the tty's write returns lesser bytes written
+ *	then to maintain the rest of data to be written on next instance.
+ *	This needs to be protected, hence the lock inside wakeup func.
+ * @tx_state: if the data is being written onto the TTY and protocol driver
+ *	wants to send more, queue up data and mark that there is
+ *	more data to send.
+ * @list: the list of protocols registered, only MAX can exist, one protocol
+ *	can register only once.
+ * @rx_state: states to be maintained inside st's tty receive
+ * @rx_count: count to be maintained inside st's tty receieve
+ * @rx_skb: the skb where all data for a protocol gets accumulated,
+ *	since tty might not call receive when a complete event packet
+ *	is received, the states, count and the skb needs to be maintained.
+ * @txq: the list of skbs which needs to be sent onto the TTY.
+ * @tx_waitq: if the chip is not in AWAKE state, the skbs needs to be queued
+ *	up in here, PM(WAKEUP_IND) data needs to be sent and then the skbs
+ *	from waitq can be moved onto the txq.
+ *	Needs locking too.
+ * @lock: the lock to protect skbs, queues, and ST states.
+ * @protos_registered: count of the protocols registered, also when 0 the
+ *	chip enable gpio can be toggled, and when it changes to 1 the fw
+ *	needs to be downloaded to initialize chip side ST.
+ * @ll_state: the various PM states the chip can be, the states are notified
+ *	to us, when the chip sends relevant PM packets(SLEEP_IND, WAKE_IND).
+ * @kim_data: reference to the parent encapsulating structure.
+ *
  */
 struct st_data_s {
 	unsigned long st_state;
-/*
- * an instance of tty_struct & ldisc ops to move around
- */
 	struct tty_struct *tty;
 	struct tty_ldisc_ops *ldisc_ops;
-/*
- * the tx skb -
- * if the skb is already dequeued and the tty failed to write the same
- * maintain the skb to write in the next transaction
- */
 	struct sk_buff *tx_skb;
 #define ST_TX_SENDING	1
 #define ST_TX_WAKEUP	2
 	unsigned long tx_state;
-/*
- * list of protocol registered
- */
 	struct st_proto_s *list[ST_MAX];
-/*
- * lock
- */
 	unsigned long rx_state;
 	unsigned long rx_count;
 	struct sk_buff *rx_skb;
 	struct sk_buff_head txq, tx_waitq;
-	spinlock_t lock;	/* ST LL state lock  */
+	spinlock_t lock;
 	unsigned char	protos_registered;
-	unsigned long ll_state;	/* ST LL power state */
-/* device reference to kim data */
+	unsigned long ll_state;
 	void *kim_data;
 };
 
-/* point this to tty->driver->write or tty->ops->write
+/**
+ * st_int_write -
+ * point this to tty->driver->write or tty->ops->write
  * depending upon the kernel version
  */
 int st_int_write(struct st_data_s*, const unsigned char*, int);
-/* internal write function, passed onto protocol drivers
+
+/**
+ * st_write -
+ * internal write function, passed onto protocol drivers
  * via the write function ptr of protocol struct
  */
 long st_write(struct sk_buff *);
-/* function to be called from ST-LL
- */
+
+/* function to be called from ST-LL */
 void st_ll_send_frame(enum proto_type, struct sk_buff *);
+
 /* internal wake up function */
 void st_tx_wakeup(struct st_data_s *st_data);
 
+/* init, exit entry funcs called from KIM */
 int st_core_init(struct st_data_s **);
 void st_core_exit(struct st_data_s *);
+
+/* ask for reference from KIM */
 void st_kim_ref(struct st_data_s **);
 
 #define GPS_STUB_TEST
diff --git a/drivers/staging/ti-st/st_kim.c b/drivers/staging/ti-st/st_kim.c
index 5ab69d1..7e34e4d 100644
--- a/drivers/staging/ti-st/st_kim.c
+++ b/drivers/staging/ti-st/st_kim.c
@@ -104,10 +104,11 @@ const unsigned char *protocol_names[] = {
 /**********************************************************************/
 /* internal functions */
 
-/*
- * function to return whether the firmware response was proper
- * in case of error don't complete so that waiting for proper
- * response times out
+/**
+ * validate_firmware_response -
+ *	function to return whether the firmware response was proper
+ *	in case of error don't complete so that waiting for proper
+ *	response times out
  */
 void validate_firmware_response(struct kim_data_s *kim_gdata)
 {
@@ -158,10 +159,11 @@ static inline int kim_check_data_len(struct kim_data_s *kim_gdata, int len)
 	return 0;
 }
 
-/* receive function called during firmware download
- * - firmware download responses on different UART drivers
- *   have been observed to come in bursts of different
- *   tty_receive and hence the logic
+/**
+ * kim_int_recv - receive function called during firmware download
+ *	firmware download responses on different UART drivers
+ *	have been observed to come in bursts of different
+ *	tty_receive and hence the logic
  */
 void kim_int_recv(struct kim_data_s *kim_gdata,
 	const unsigned char *data, long count)
@@ -220,7 +222,7 @@ void kim_int_recv(struct kim_data_s *kim_gdata,
 			ptr++;
 			count--;
 			continue;
-		}		/* end of switch *ptr */
+		}
 		ptr++;
 		count--;
 		kim_gdata->rx_skb =
@@ -230,9 +232,9 @@ void kim_int_recv(struct kim_data_s *kim_gdata,
 			kim_gdata->rx_state = ST_W4_PACKET_TYPE;
 			kim_gdata->rx_count = 0;
 			return;
-		} /* not necessary in this case */
+		}
 		bt_cb(kim_gdata->rx_skb)->pkt_type = type;
-	}			/* end of while count */
+	}
 	pr_info("done %s", __func__);
 	return;
 }
@@ -278,8 +280,10 @@ static long read_local_version(struct kim_data_s *kim_gdata, char *bts_scr_name)
 	return 0;
 }
 
-/* internal function which parses through the .bts firmware script file
- * intreprets SEND, DELAY actions only as of now
+/**
+ * download_firmware -
+ *	internal function which parses through the .bts firmware
+ *	script file intreprets SEND, DELAY actions only as of now
  */
 static long download_firmware(struct kim_data_s *kim_gdata)
 {
@@ -445,8 +449,13 @@ void st_kim_complete(void *kim_data)
 	complete(&kim_gdata->ldisc_installed);
 }
 
-/* called from ST Core upon 1st registration
-*/
+/**
+ * st_kim_start - called from ST Core upon 1st registration
+ *	This involves toggling the chip enable gpio, reading
+ *	the firmware version from chip, forming the fw file name
+ *	based on the chip version, requesting the fw, parsing it
+ *	and perform download(send/recv).
+ */
 long st_kim_start(void *kim_data)
 {
 	long err = 0;
@@ -501,8 +510,10 @@ long st_kim_start(void *kim_data)
 	return err;
 }
 
-/* called from ST Core, on the last un-registration
-*/
+/**
+ * st_kim_stop - called from ST Core, on the last un-registration
+ *	toggle low the chip enable gpio
+ */
 long st_kim_stop(void *kim_data)
 {
 	long err = 0;
@@ -607,6 +618,13 @@ static int kim_toggle_radio(void *data, bool blocked)
 	return 0;
 }
 
+/**
+ * st_kim_ref - reference the core's data
+ *	This references the per-ST platform device in the arch/xx/
+ *	board-xx.c file.
+ *	This would enable multiple such platform devices to exist
+ *	on a given platform
+ */
 void st_kim_ref(struct st_data_s **core_data)
 {
 	struct platform_device	*pdev;
diff --git a/drivers/staging/ti-st/st_kim.h b/drivers/staging/ti-st/st_kim.h
index d5685f4..225cacd 100644
--- a/drivers/staging/ti-st/st_kim.h
+++ b/drivers/staging/ti-st/st_kim.h
@@ -43,13 +43,9 @@
  * since the self-test for chip takes a while
  */
 #define POR_RETRY_COUNT 5
-/*
- * legacy rfkill support where-in 3 rfkill
- * devices are created for the 3 gpios
- * that ST has requested
- */
 
-/* chip version storage
+/**
+ * struct chip_version - save the chip version
  */
 struct chip_version {
 	unsigned short full;
@@ -58,18 +54,40 @@ struct chip_version {
 	unsigned short maj_ver;
 };
 
-/*
- * header file for ST provided by KIM
+/**
+ * struct kim_data_s - the KIM internal data, embedded as the
+ *	platform's drv data. One for each ST device in the system.
+ * @uim_pid: KIM needs to communicate with UIM to request to install
+ *	the ldisc by opening UART when protocol drivers register.
+ * @kim_pdev: the platform device added in one of the board-XX.c file
+ *	in arch/XX/ directory, 1 for each ST device.
+ * @kim_rcvd: completion handler to notify when data was received,
+ *	mainly used during fw download, which involves multiple send/wait
+ *	for each of the HCI-VS commands.
+ * @ldisc_installed: completion handler to notify that the UIM accepted
+ *	the request to install ldisc, notify from tty_open which suggests
+ *	the ldisc was properly installed.
+ * @resp_buffer: data buffer for the .bts fw file name.
+ * @fw_entry: firmware class struct to request/release the fw.
+ * @gpios: the list of core/chip enable gpios for BT, FM and GPS cores.
+ * @rx_state: the rx state for kim's receive func during fw download.
+ * @rx_count: the rx count for the kim's receive func during fw download.
+ * @rx_skb: all of fw data might not come at once, and hence data storage for
+ *	whole of the fw response, only HCI_EVENTs and hence diff from ST's
+ *	response.
+ * @rfkill: rfkill data for each of the cores to be registered with rfkill.
+ * @rf_protos: proto types of the data registered with rfkill sub-system.
+ * @core_data: ST core's data, which mainly is the tty's disc_data
+ * @version: chip version available via a sysfs entry.
+ *
  */
 struct kim_data_s {
 	long uim_pid;
 	struct platform_device *kim_pdev;
 	struct completion kim_rcvd, ldisc_installed;
-	/* MAX len of the .bts firmware script name */
 	char resp_buffer[30];
 	const struct firmware *fw_entry;
 	long gpios[ST_MAX];
-/* used by kim_int_recv to validate fw response */
 	unsigned long rx_state;
 	unsigned long rx_count;
 	struct sk_buff *rx_skb;
@@ -79,20 +97,17 @@ struct kim_data_s {
 	struct chip_version version;
 };
 
+/**
+ * functions called when 1 of the protocol drivers gets
+ * registered, these need to communicate with UIM to request
+ * ldisc installed, read chip_version, download relevant fw
+ */
 long st_kim_start(void *);
 long st_kim_stop(void *);
-/*
- * called from st_tty_receive to authenticate fw_download
- */
-void st_kim_recv(void *, const unsigned char *, long count);
 
+void st_kim_recv(void *, const unsigned char *, long count);
 void st_kim_chip_toggle(enum proto_type, enum kim_gpio_state);
-
 void st_kim_complete(void *);
-
-/* function called from ST KIM to ST Core, to
- * list out the protocols registered
- */
 void kim_st_list_protocols(struct st_data_s *, char *);
 
 /*
@@ -105,9 +120,13 @@ void kim_st_list_protocols(struct st_data_s *, char *);
 #define ACTION_RUN_SCRIPT       5
 #define ACTION_REMARKS          6
 
-/*
- *  * BRF Firmware header
- *   */
+/**
+ * struct bts_header - the fw file is NOT binary which can
+ *	be sent onto TTY as is. The .bts is more a script
+ *	file which has different types of actions.
+ *	Each such action needs to be parsed by the KIM and
+ *	relevant procedure to be called.
+ */
 struct bts_header {
 	uint32_t magic;
 	uint32_t version;
@@ -115,9 +134,10 @@ struct bts_header {
 	uint8_t actions[0];
 } __attribute__ ((packed));
 
-/*
- *  * BRF Actions structure
- *   */
+/**
+ * struct bts_action - Each .bts action has its own type of
+ *	data.
+ */
 struct bts_action {
 	uint16_t type;
 	uint16_t size;
@@ -143,8 +163,11 @@ struct bts_action_serial {
 	uint32_t flow_control;
 } __attribute__ ((packed));
 
-/* for identifying the change speed HCI VS
- * command
+/**
+ * struct hci_command - the HCI-VS for intrepreting
+ *	the change baud rate of host-side UART, which
+ *	needs to be ignored, since UIM would do that
+ *	when it receives request from KIM for ldisc installation.
  */
 struct hci_command {
 	uint8_t prefix;
diff --git a/drivers/staging/ti-st/st_ll.h b/drivers/staging/ti-st/st_ll.h
index 77dfbf0..e4dfacd 100644
--- a/drivers/staging/ti-st/st_ll.h
+++ b/drivers/staging/ti-st/st_ll.h
@@ -41,6 +41,7 @@
 #define ST_LL_AWAKE_TO_ASLEEP      3
 #define ST_LL_INVALID		   4
 
+/* different PM notifications coming from chip */
 #define LL_SLEEP_IND	0x30
 #define LL_SLEEP_ACK	0x31
 #define LL_WAKE_UP_IND	0x32
@@ -50,13 +51,19 @@
 long st_ll_init(struct st_data_s *);
 long st_ll_deinit(struct st_data_s *);
 
-/* enable/disable ST LL along with KIM start/stop
+/**
+ * enable/disable ST LL along with KIM start/stop
  * called by ST Core
  */
 void st_ll_enable(struct st_data_s *);
 void st_ll_disable(struct st_data_s *);
 
+/**
+ * various funcs used by ST core to set/get the various PM states
+ * of the chip.
+ */
 unsigned long st_ll_getstate(struct st_data_s *);
 unsigned long st_ll_sleep_state(struct st_data_s *, unsigned char);
 void st_ll_wakeup(struct st_data_s *);
+
 #endif /* ST_LL_H */
-- 
1.5.6.3

[PATCH 3/3] drivers:staging:ti-st: give proto drivers context

[pavan_savoy]

From: Pavan Savoy <pavan_savoy@ti.com>

protocol drivers such as BT, FM and GPS when registering
to ST now provide their own private data which they expect
when their functions namely registration completed & receive
are called.
Also upon tty_close, set protos_registered count to 0, although
all protocols are marked un-registered.

Signed-off-by: Pavan Savoy <pavan_savoy@ti.com>
---
 drivers/staging/ti-st/bt_drv.c  |   23 +++++++++++++++--------
 drivers/staging/ti-st/st.h      |    8 ++++++--
 drivers/staging/ti-st/st_core.c |    8 ++++++--
 3 files changed, 27 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/ti-st/bt_drv.c b/drivers/staging/ti-st/bt_drv.c
index d70aea1..61ae988 100644
--- a/drivers/staging/ti-st/bt_drv.c
+++ b/drivers/staging/ti-st/bt_drv.c
@@ -80,31 +80,33 @@ static inline void hci_st_tx_complete(struct hci_st *hst, int pkt_type)
  * status.hci_st_open() function will wait for signal from this
  * API when st_register() function returns ST_PENDING.
  */
-static void hci_st_registration_completion_cb(char data)
+static void hci_st_registration_completion_cb(void *priv_data, char data)
 {
+	struct hci_st *lhst = (struct hci_st *)priv_data;
 	BTDRV_API_START();
 
 	/* hci_st_open() function needs value of 'data' to know
 	 * the registration status(success/fail),So have a back
 	 * up of it.
 	 */
-	hst->streg_cbdata = data;
+	lhst->streg_cbdata = data;
 
 	/* Got a feedback from ST for BT driver registration
 	 * request.Wackup hci_st_open() function to continue
 	 * it's open operation.
 	 */
-	complete(&hst->wait_for_btdrv_reg_completion);
+	complete(&lhst->wait_for_btdrv_reg_completion);
 
 	BTDRV_API_EXIT(0);
 }
 
 /* Called by Shared Transport layer when receive data is
  * available */
-static long hci_st_receive(struct sk_buff *skb)
+static long hci_st_receive(void *priv_data, struct sk_buff *skb)
 {
 	int err;
 	int len;
+	struct hci_st *lhst = (struct hci_st *)priv_data;
 
 	BTDRV_API_START();
 
@@ -116,13 +118,13 @@ static long hci_st_receive(struct sk_buff *skb)
 		BTDRV_API_EXIT(-EFAULT);
 		return -EFAULT;
 	}
-	if (!hst) {
+	if (!lhst) {
 		kfree_skb(skb);
 		BT_DRV_ERR("Invalid hci_st memory,freeing SKB");
 		BTDRV_API_EXIT(-EFAULT);
 		return -EFAULT;
 	}
-	if (!test_bit(BT_DRV_RUNNING, &hst->flags)) {
+	if (!test_bit(BT_DRV_RUNNING, &lhst->flags)) {
 		kfree_skb(skb);
 		BT_DRV_ERR("Device is not running,freeing SKB");
 		BTDRV_API_EXIT(-EINVAL);
@@ -130,7 +132,7 @@ static long hci_st_receive(struct sk_buff *skb)
 	}
 
 	len = skb->len;
-	skb->dev = (struct net_device *)hst->hdev;
+	skb->dev = (struct net_device *)lhst->hdev;
 
 	/* Forward skb to HCI CORE layer */
 	err = hci_recv_frame(skb);
@@ -141,7 +143,7 @@ static long hci_st_receive(struct sk_buff *skb)
 		BTDRV_API_EXIT(err);
 		return err;
 	}
-	hst->hdev->stat.byte_rx += len;
+	lhst->hdev->stat.byte_rx += len;
 
 	BTDRV_API_EXIT(0);
 	return 0;
@@ -189,6 +191,11 @@ static int hci_st_open(struct hci_dev *hdev)
 	 * make it as NULL */
 	hci_st_proto.write = NULL;
 
+	/* send in the hst to be received at registration complete callback
+	 * and during st's receive
+	 */
+	hci_st_proto.priv_data = hst;
+
 	/* Register with ST layer */
 	err = st_register(&hci_st_proto);
 	if (err == -EINPROGRESS) {
diff --git a/drivers/staging/ti-st/st.h b/drivers/staging/ti-st/st.h
index c96c01e..9952579 100644
--- a/drivers/staging/ti-st/st.h
+++ b/drivers/staging/ti-st/st.h
@@ -64,13 +64,17 @@ enum proto_type {
  *	download is in progress.
  * @write: pointer to function in ST provided to protocol drivers from ST,
  *	to be made use when protocol drivers have data to send to TTY.
+ * @priv_data: privdate data holder for the protocol drivers, sent
+ *	from the protocol drivers during registration, and sent back on
+ *	reg_complete_cb and recv.
  */
 struct st_proto_s {
 	enum proto_type type;
-	long (*recv) (struct sk_buff *);
+	long (*recv) (void *, struct sk_buff *);
 	unsigned char (*match_packet) (const unsigned char *data);
-	void (*reg_complete_cb) (char data);
+	void (*reg_complete_cb) (void *, char data);
 	long (*write) (struct sk_buff *skb);
+	void *priv_data;
 };
 
 extern long st_register(struct st_proto_s *);
diff --git a/drivers/staging/ti-st/st_core.c b/drivers/staging/ti-st/st_core.c
index b20ab73..fc6de63 100644
--- a/drivers/staging/ti-st/st_core.c
+++ b/drivers/staging/ti-st/st_core.c
@@ -119,7 +119,9 @@ void st_send_frame(enum proto_type protoid, struct st_data_s *st_gdata)
 	 *   protocol stack driver
 	 */
 	if (likely(st_gdata->list[protoid]->recv != NULL)) {
-		if (unlikely(st_gdata->list[protoid]->recv(st_gdata->rx_skb)
+		if (unlikely
+			(st_gdata->list[protoid]->recv
+			(st_gdata->list[protoid]->priv_data, st_gdata->rx_skb)
 			     != 0)) {
 			pr_err(" proto stack %d's ->recv failed", protoid);
 			kfree_skb(st_gdata->rx_skb);
@@ -144,7 +146,8 @@ void st_reg_complete(struct st_data_s *st_gdata, char err)
 	for (i = 0; i < ST_MAX; i++) {
 		if (likely(st_gdata != NULL && st_gdata->list[i] != NULL &&
 			   st_gdata->list[i]->reg_complete_cb != NULL))
-			st_gdata->list[i]->reg_complete_cb(err);
+			st_gdata->list[i]->reg_complete_cb
+				(st_gdata->list[i]->priv_data, err);
 	}
 }
 
@@ -878,6 +881,7 @@ static void st_tty_close(struct tty_struct *tty)
 			pr_err("%d not un-registered", i);
 		st_gdata->list[i] = NULL;
 	}
+	st_gdata->protos_registered = 0;
 	spin_unlock_irqrestore(&st_gdata->lock, flags);
 	/*
 	 * signal to UIM via KIM that -
-- 
1.5.6.3

Re: [PATCH 0/3] drivers:staging:ti-st: patches

[Randy Dunlap]

On Thu, 22 Jul 2010 05:32:04 -0500 pavan_savoy@ti.com wrote:

> From: Pavan Savoy <pavan_savoy@ti.com>
> 
> The following patches cleanup bit of a mess and also adds functionality to protocol drivers.
> with the 3rd patch now providing context to even the protocol drivers, the single device limit
> or support for multiple devices would be easier to implement.
> 
> These patches depend on the previously submitted
> 0001-drivers-staging-ti-st-make-use-of-linux-err-codes.patch
> commit d39d49b393d94f4137cee4f64526a4695352f183
> 
> Pavan Savoy (3):
>   drivers:staging:ti-st: smarten, reduce logs
>   drivers:staging:ti-st: cleanup code comments
>   drivers:staging:ti-st: give proto drivers context
> 
>  drivers/staging/ti-st/bt_drv.c  |   23 +++++---
>  drivers/staging/ti-st/st.h      |   52 +++++++++--------
>  drivers/staging/ti-st/st_core.c |  118 +++++++++++++++++++--------------------
>  drivers/staging/ti-st/st_core.h |   74 +++++++++++++++++--------
>  drivers/staging/ti-st/st_kim.c  |   73 ++++++++++++++----------
>  drivers/staging/ti-st/st_kim.h  |   77 ++++++++++++++++---------
>  drivers/staging/ti-st/st_ll.c   |    4 +-
>  drivers/staging/ti-st/st_ll.h   |    9 +++-
>  8 files changed, 255 insertions(+), 175 deletions(-)

Hi,

I have reported this error a few times.  Where is the patch for it??

ERROR: "st_get_plat_device" [drivers/staging/ti-st/st_drv.ko] undefined!


thanks,
---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Re: [PATCH 0/3] drivers:staging:ti-st: patches

[Pavan Savoy]

Randy,

On Fri, Jul 23, 2010 at 1:23 AM, Randy Dunlap <randy.dunlap@oracle.com> wrote:
> On Thu, 22 Jul 2010 05:32:04 -0500 pavan_savoy@ti.com wrote:
>
>> From: Pavan Savoy <pavan_savoy@ti.com>
>>
>> The following patches cleanup bit of a mess and also adds functionality to protocol drivers.
>> with the 3rd patch now providing context to even the protocol drivers, the single device limit
>> or support for multiple devices would be easier to implement.
>>
>> These patches depend on the previously submitted
>> 0001-drivers-staging-ti-st-make-use-of-linux-err-codes.patch
>> commit d39d49b393d94f4137cee4f64526a4695352f183
>>
>> Pavan Savoy (3):
>>   drivers:staging:ti-st: smarten, reduce logs
>>   drivers:staging:ti-st: cleanup code comments
>>   drivers:staging:ti-st: give proto drivers context
>>
>>  drivers/staging/ti-st/bt_drv.c  |   23 +++++---
>>  drivers/staging/ti-st/st.h      |   52 +++++++++--------
>>  drivers/staging/ti-st/st_core.c |  118 +++++++++++++++++++--------------------
>>  drivers/staging/ti-st/st_core.h |   74 +++++++++++++++++--------
>>  drivers/staging/ti-st/st_kim.c  |   73 ++++++++++++++----------
>>  drivers/staging/ti-st/st_kim.h  |   77 ++++++++++++++++---------
>>  drivers/staging/ti-st/st_ll.c   |    4 +-
>>  drivers/staging/ti-st/st_ll.h   |    9 +++-
>>  8 files changed, 255 insertions(+), 175 deletions(-)
>
> Hi,
>
> I have reported this error a few times.  Where is the patch for it??
>
> ERROR: "st_get_plat_device" [drivers/staging/ti-st/st_drv.ko] undefined!


Yes, on one of the earlier patch sets, I had mentioned that the ST
driver being a platform device, needs definition in any of the
arch/XX/mach-XX/board-XX.c or devices.c or somewhere...

and hence it is in that board-XX.c file that the symbol
st_get_plat_device needs to be exported, the reason for that being,

ST driver being both a TTY ldisc driver and platform driver, in TTY
contexts it would need to refer to platform driver's data. So it does
a st_get_plat_device which returns the platform device structure, and
then does a dev_getdrvdata from it.

here's a snippet of code ...
/*
 * ST related functions related functions
 */
#include <linux/platform_device.h>

long gpios[] = { 55, -1, -1 };
static struct platform_device ti_st_device = {
        .name           = "kim",
        .id             = -1,
        .dev.platform_data      = &gpios,
};

struct platform_device *st_get_plat_device(void)
{
        return &ti_st_device;
}
EXPORT_SYMBOL(st_get_plat_device);

static __init int add_ti_st_device(void)
{
        platform_device_register(&ti_st_device);
        dev_info(&ti_st_device.dev,"registered platform TI ST device\n");

        return 0;
}
device_initcall(add_ti_st_device);


We have that in our local trees in arch/arm/mach-omap2/board-sdp4430.c

>
> thanks,
> ---
> ~Randy
> *** Remember to use Documentation/SubmitChecklist when testing your code ***
>

Re: [PATCH 0/3] drivers:staging:ti-st: patches

[Randy Dunlap]

On 07/22/10 21:56, Pavan Savoy wrote:
> Randy,
> 
> On Fri, Jul 23, 2010 at 1:23 AM, Randy Dunlap <randy.dunlap@oracle.com> wrote:
>> On Thu, 22 Jul 2010 05:32:04 -0500 pavan_savoy@ti.com wrote:
>>
>>> From: Pavan Savoy <pavan_savoy@ti.com>
>>>
>>> The following patches cleanup bit of a mess and also adds functionality to protocol drivers.
>>> with the 3rd patch now providing context to even the protocol drivers, the single device limit
>>> or support for multiple devices would be easier to implement.
>>>
>>> These patches depend on the previously submitted
>>> 0001-drivers-staging-ti-st-make-use-of-linux-err-codes.patch
>>> commit d39d49b393d94f4137cee4f64526a4695352f183
>>>
>>> Pavan Savoy (3):
>>>   drivers:staging:ti-st: smarten, reduce logs
>>>   drivers:staging:ti-st: cleanup code comments
>>>   drivers:staging:ti-st: give proto drivers context
>>>
>>>  drivers/staging/ti-st/bt_drv.c  |   23 +++++---
>>>  drivers/staging/ti-st/st.h      |   52 +++++++++--------
>>>  drivers/staging/ti-st/st_core.c |  118 +++++++++++++++++++--------------------
>>>  drivers/staging/ti-st/st_core.h |   74 +++++++++++++++++--------
>>>  drivers/staging/ti-st/st_kim.c  |   73 ++++++++++++++----------
>>>  drivers/staging/ti-st/st_kim.h  |   77 ++++++++++++++++---------
>>>  drivers/staging/ti-st/st_ll.c   |    4 +-
>>>  drivers/staging/ti-st/st_ll.h   |    9 +++-
>>>  8 files changed, 255 insertions(+), 175 deletions(-)
>>
>> Hi,
>>
>> I have reported this error a few times.  Where is the patch for it??
>>
>> ERROR: "st_get_plat_device" [drivers/staging/ti-st/st_drv.ko] undefined!
> 
> 
> Yes, on one of the earlier patch sets, I had mentioned that the ST
> driver being a platform device, needs definition in any of the
> arch/XX/mach-XX/board-XX.c or devices.c or somewhere...
> 
> and hence it is in that board-XX.c file that the symbol
> st_get_plat_device needs to be exported, the reason for that being,
> 
> ST driver being both a TTY ldisc driver and platform driver, in TTY
> contexts it would need to refer to platform driver's data. So it does
> a st_get_plat_device which returns the platform device structure, and
> then does a dev_getdrvdata from it.
> 
> here's a snippet of code ...
> /*
>  * ST related functions related functions
>  */
> #include <linux/platform_device.h>
> 
> long gpios[] = { 55, -1, -1 };
> static struct platform_device ti_st_device = {
>         .name           = "kim",
>         .id             = -1,
>         .dev.platform_data      = &gpios,
> };
> 
> struct platform_device *st_get_plat_device(void)
> {
>         return &ti_st_device;
> }
> EXPORT_SYMBOL(st_get_plat_device);
> 
> static __init int add_ti_st_device(void)
> {
>         platform_device_register(&ti_st_device);
>         dev_info(&ti_st_device.dev,"registered platform TI ST device\n");
> 
>         return 0;
> }
> device_initcall(add_ti_st_device);
> 
> 
> We have that in our local trees in arch/arm/mach-omap2/board-sdp4430.c

Thanks for the explanation.

Is the driver platform-specific?
E.g., should it not even be built on x86?

-- 
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Re: [PATCH 0/3] drivers:staging:ti-st: patches

[Pavan Savoy]

On Fri, Jul 23, 2010 at 8:47 PM, Randy Dunlap <randy.dunlap@oracle.com> wrote:
> On 07/22/10 21:56, Pavan Savoy wrote:
>> Randy,
>>
>> On Fri, Jul 23, 2010 at 1:23 AM, Randy Dunlap <randy.dunlap@oracle.com> wrote:
>>> On Thu, 22 Jul 2010 05:32:04 -0500 pavan_savoy@ti.com wrote:
>>>
>>>> From: Pavan Savoy <pavan_savoy@ti.com>
>>>>
>>>> The following patches cleanup bit of a mess and also adds functionality to protocol drivers.
>>>> with the 3rd patch now providing context to even the protocol drivers, the single device limit
>>>> or support for multiple devices would be easier to implement.
>>>>
>>>> These patches depend on the previously submitted
>>>> 0001-drivers-staging-ti-st-make-use-of-linux-err-codes.patch
>>>> commit d39d49b393d94f4137cee4f64526a4695352f183
>>>>
>>>> Pavan Savoy (3):
>>>>   drivers:staging:ti-st: smarten, reduce logs
>>>>   drivers:staging:ti-st: cleanup code comments
>>>>   drivers:staging:ti-st: give proto drivers context
>>>>
>>>>  drivers/staging/ti-st/bt_drv.c  |   23 +++++---
>>>>  drivers/staging/ti-st/st.h      |   52 +++++++++--------
>>>>  drivers/staging/ti-st/st_core.c |  118 +++++++++++++++++++--------------------
>>>>  drivers/staging/ti-st/st_core.h |   74 +++++++++++++++++--------
>>>>  drivers/staging/ti-st/st_kim.c  |   73 ++++++++++++++----------
>>>>  drivers/staging/ti-st/st_kim.h  |   77 ++++++++++++++++---------
>>>>  drivers/staging/ti-st/st_ll.c   |    4 +-
>>>>  drivers/staging/ti-st/st_ll.h   |    9 +++-
>>>>  8 files changed, 255 insertions(+), 175 deletions(-)
>>>
>>> Hi,
>>>
>>> I have reported this error a few times.  Where is the patch for it??
>>>
>>> ERROR: "st_get_plat_device" [drivers/staging/ti-st/st_drv.ko] undefined!
>>
>>
>> Yes, on one of the earlier patch sets, I had mentioned that the ST
>> driver being a platform device, needs definition in any of the
>> arch/XX/mach-XX/board-XX.c or devices.c or somewhere...
>>
>> and hence it is in that board-XX.c file that the symbol
>> st_get_plat_device needs to be exported, the reason for that being,
>>
>> ST driver being both a TTY ldisc driver and platform driver, in TTY
>> contexts it would need to refer to platform driver's data. So it does
>> a st_get_plat_device which returns the platform device structure, and
>> then does a dev_getdrvdata from it.
>>
>> here's a snippet of code ...
>> /*
>>  * ST related functions related functions
>>  */
>> #include <linux/platform_device.h>
>>
>> long gpios[] = { 55, -1, -1 };
>> static struct platform_device ti_st_device = {
>>         .name           = "kim",
>>         .id             = -1,
>>         .dev.platform_data      = &gpios,
>> };
>>
>> struct platform_device *st_get_plat_device(void)
>> {
>>         return &ti_st_device;
>> }
>> EXPORT_SYMBOL(st_get_plat_device);
>>
>> static __init int add_ti_st_device(void)
>> {
>>         platform_device_register(&ti_st_device);
>>         dev_info(&ti_st_device.dev,"registered platform TI ST device\n");
>>
>>         return 0;
>> }
>> device_initcall(add_ti_st_device);
>>
>>
>> We have that in our local trees in arch/arm/mach-omap2/board-sdp4430.c
>
> Thanks for the explanation.
>
> Is the driver platform-specific?
> E.g., should it not even be built on x86?

Yes. Requirement of the hardware is very much a must.
However it is a separate peripheral (WiLink 7 - uart interfaced), may
be there is a x86 platform with this - but certainly not desktops.

on linux-next, I generally put in that st_dev.c file for x86 - verify
whether it builds as a module, inserts/rmmod, basic other
functionalities (which doesn't involve response from chip..)
But verify full functionality on board which constitutes that.

> --
> ~Randy
> *** Remember to use Documentation/SubmitChecklist when testing your code ***
>

Re: [PATCH 0/3] drivers:staging:ti-st: patches

[Randy Dunlap]

On Mon, 26 Jul 2010 10:37:02 +0530 Pavan Savoy wrote:

> On Fri, Jul 23, 2010 at 8:47 PM, Randy Dunlap <randy.dunlap@oracle.com> wrote:
> > On 07/22/10 21:56, Pavan Savoy wrote:
> >> Randy,
> >>
> >> On Fri, Jul 23, 2010 at 1:23 AM, Randy Dunlap <randy.dunlap@oracle.com> wrote:
> >>> On Thu, 22 Jul 2010 05:32:04 -0500 pavan_savoy@ti.com wrote:
> >>>
> >>>> From: Pavan Savoy <pavan_savoy@ti.com>
> >>>>
> >>>> The following patches cleanup bit of a mess and also adds functionality to protocol drivers.
> >>>> with the 3rd patch now providing context to even the protocol drivers, the single device limit
> >>>> or support for multiple devices would be easier to implement.
> >>>>
> >>>> These patches depend on the previously submitted
> >>>> 0001-drivers-staging-ti-st-make-use-of-linux-err-codes.patch
> >>>> commit d39d49b393d94f4137cee4f64526a4695352f183
> >>>>
> >>>> Pavan Savoy (3):
> >>>>   drivers:staging:ti-st: smarten, reduce logs
> >>>>   drivers:staging:ti-st: cleanup code comments
> >>>>   drivers:staging:ti-st: give proto drivers context
> >>>>
> >>>>  drivers/staging/ti-st/bt_drv.c  |   23 +++++---
> >>>>  drivers/staging/ti-st/st.h      |   52 +++++++++--------
> >>>>  drivers/staging/ti-st/st_core.c |  118 +++++++++++++++++++--------------------
> >>>>  drivers/staging/ti-st/st_core.h |   74 +++++++++++++++++--------
> >>>>  drivers/staging/ti-st/st_kim.c  |   73 ++++++++++++++----------
> >>>>  drivers/staging/ti-st/st_kim.h  |   77 ++++++++++++++++---------
> >>>>  drivers/staging/ti-st/st_ll.c   |    4 +-
> >>>>  drivers/staging/ti-st/st_ll.h   |    9 +++-
> >>>>  8 files changed, 255 insertions(+), 175 deletions(-)
> >>>
> >>> Hi,
> >>>
> >>> I have reported this error a few times.  Where is the patch for it??
> >>>
> >>> ERROR: "st_get_plat_device" [drivers/staging/ti-st/st_drv.ko] undefined!
> >>
> >>
> >> Yes, on one of the earlier patch sets, I had mentioned that the ST
> >> driver being a platform device, needs definition in any of the
> >> arch/XX/mach-XX/board-XX.c or devices.c or somewhere...
> >>
> >> and hence it is in that board-XX.c file that the symbol
> >> st_get_plat_device needs to be exported, the reason for that being,
> >>
> >> ST driver being both a TTY ldisc driver and platform driver, in TTY
> >> contexts it would need to refer to platform driver's data. So it does
> >> a st_get_plat_device which returns the platform device structure, and
> >> then does a dev_getdrvdata from it.
> >>
> >> here's a snippet of code ...
> >> /*
> >>  * ST related functions related functions
> >>  */
> >> #include <linux/platform_device.h>
> >>
> >> long gpios[] = { 55, -1, -1 };
> >> static struct platform_device ti_st_device = {
> >>         .name           = "kim",
> >>         .id             = -1,
> >>         .dev.platform_data      = &gpios,
> >> };
> >>
> >> struct platform_device *st_get_plat_device(void)
> >> {
> >>         return &ti_st_device;
> >> }
> >> EXPORT_SYMBOL(st_get_plat_device);
> >>
> >> static __init int add_ti_st_device(void)
> >> {
> >>         platform_device_register(&ti_st_device);
> >>         dev_info(&ti_st_device.dev,"registered platform TI ST device\n");
> >>
> >>         return 0;
> >> }
> >> device_initcall(add_ti_st_device);
> >>
> >>
> >> We have that in our local trees in arch/arm/mach-omap2/board-sdp4430.c
> >
> > Thanks for the explanation.
> >
> > Is the driver platform-specific?
> > E.g., should it not even be built on x86?
> 
> Yes. Requirement of the hardware is very much a must.
> However it is a separate peripheral (WiLink 7 - uart interfaced), may
> be there is a x86 platform with this - but certainly not desktops.
> 
> on linux-next, I generally put in that st_dev.c file for x86 - verify
> whether it builds as a module, inserts/rmmod, basic other
> functionalities (which doesn't involve response from chip..)
> But verify full functionality on board which constitutes that.

Hi,
Please make this driver build cleanly on X86 or prevent it from being built there.

thanks,
---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

RE: [PATCH 0/3] drivers:staging:ti-st: patches

[Savoy, Pavan]

Randy,


> -----Original Message-----
> From: Randy Dunlap [mailto:randy.dunlap@oracle.com]
> Sent: Wednesday, August 18, 2010 4:05 PM
> To: Savoy, Pavan
> Cc: devel@driverdev.osuosl.org; gregkh@suse.de; linux-kernel@vger.kernel.org;
> alan@lxorguk.ukuu.org.uk
> Subject: Re: [PATCH 0/3] drivers:staging:ti-st: patches
> 
> On Mon, 26 Jul 2010 10:37:02 +0530 Pavan Savoy wrote:
> 
> > On Fri, Jul 23, 2010 at 8:47 PM, Randy Dunlap <randy.dunlap@oracle.com>
> wrote:
> > > On 07/22/10 21:56, Pavan Savoy wrote:
> > >> Randy,
> > >>
> > >> On Fri, Jul 23, 2010 at 1:23 AM, Randy Dunlap <randy.dunlap@oracle.com>
> wrote:
> > >>> On Thu, 22 Jul 2010 05:32:04 -0500 pavan_savoy@ti.com wrote:
> > >>>
> > >>>> From: Pavan Savoy <pavan_savoy@ti.com>
> > >>>>
> > >>>> The following patches cleanup bit of a mess and also adds functionality
> to protocol drivers.
> > >>>> with the 3rd patch now providing context to even the protocol drivers,
> the single device limit
> > >>>> or support for multiple devices would be easier to implement.
> > >>>>
> > >>>> These patches depend on the previously submitted
> > >>>> 0001-drivers-staging-ti-st-make-use-of-linux-err-codes.patch
> > >>>> commit d39d49b393d94f4137cee4f64526a4695352f183
> > >>>>
> > >>>> Pavan Savoy (3):
> > >>>>   drivers:staging:ti-st: smarten, reduce logs
> > >>>>   drivers:staging:ti-st: cleanup code comments
> > >>>>   drivers:staging:ti-st: give proto drivers context
> > >>>>
> > >>>>  drivers/staging/ti-st/bt_drv.c  |   23 +++++---
> > >>>>  drivers/staging/ti-st/st.h      |   52 +++++++++--------
> > >>>>  drivers/staging/ti-st/st_core.c |  118 +++++++++++++++++++------------
> --------
> > >>>>  drivers/staging/ti-st/st_core.h |   74 +++++++++++++++++--------
> > >>>>  drivers/staging/ti-st/st_kim.c  |   73 ++++++++++++++----------
> > >>>>  drivers/staging/ti-st/st_kim.h  |   77 ++++++++++++++++---------
> > >>>>  drivers/staging/ti-st/st_ll.c   |    4 +-
> > >>>>  drivers/staging/ti-st/st_ll.h   |    9 +++-
> > >>>>  8 files changed, 255 insertions(+), 175 deletions(-)
> > >>>
> > >>> Hi,
> > >>>
> > >>> I have reported this error a few times.  Where is the patch for it??
> > >>>
> > >>> ERROR: "st_get_plat_device" [drivers/staging/ti-st/st_drv.ko] undefined!
> > >>
> > >>
> > >> Yes, on one of the earlier patch sets, I had mentioned that the ST
> > >> driver being a platform device, needs definition in any of the
> > >> arch/XX/mach-XX/board-XX.c or devices.c or somewhere...
> > >>
> > >> and hence it is in that board-XX.c file that the symbol
> > >> st_get_plat_device needs to be exported, the reason for that being,
> > >>
> > >> ST driver being both a TTY ldisc driver and platform driver, in TTY
> > >> contexts it would need to refer to platform driver's data. So it does
> > >> a st_get_plat_device which returns the platform device structure, and
> > >> then does a dev_getdrvdata from it.
> > >>
> > >> here's a snippet of code ...
> > >> /*
> > >>  * ST related functions related functions
> > >>  */
> > >> #include <linux/platform_device.h>
> > >>
> > >> long gpios[] = { 55, -1, -1 };
> > >> static struct platform_device ti_st_device = {
> > >>         .name           = "kim",
> > >>         .id             = -1,
> > >>         .dev.platform_data      = &gpios,
> > >> };
> > >>
> > >> struct platform_device *st_get_plat_device(void)
> > >> {
> > >>         return &ti_st_device;
> > >> }
> > >> EXPORT_SYMBOL(st_get_plat_device);
> > >>
> > >> static __init int add_ti_st_device(void)
> > >> {
> > >>         platform_device_register(&ti_st_device);
> > >>         dev_info(&ti_st_device.dev,"registered platform TI ST device\n");
> > >>
> > >>         return 0;
> > >> }
> > >> device_initcall(add_ti_st_device);
> > >>
> > >>
> > >> We have that in our local trees in arch/arm/mach-omap2/board-sdp4430.c
> > >
> > > Thanks for the explanation.
> > >
> > > Is the driver platform-specific?
> > > E.g., should it not even be built on x86?
> >
> > Yes. Requirement of the hardware is very much a must.
> > However it is a separate peripheral (WiLink 7 - uart interfaced), may
> > be there is a x86 platform with this - but certainly not desktops.
> >
> > on linux-next, I generally put in that st_dev.c file for x86 - verify
> > whether it builds as a module, inserts/rmmod, basic other
> > functionalities (which doesn't involve response from chip..)
> > But verify full functionality on board which constitutes that.
> 
> Hi,
> Please make this driver build cleanly on X86 or prevent it from being built
> there.

Do you do something like a make allyesconfig?
I am having a hard time figuring out why is it building for you in the first place?
make defconfig doesn't build it, neither does make i386_defconfig.

May be a patch which does 'default N' in drivers/staging/ti-st/Kconfig, would suffice?
Please suggest...

> thanks,
> ---
> ~Randy
> *** Remember to use Documentation/SubmitChecklist when testing your code ***

Re: [PATCH 0/3] drivers:staging:ti-st: patches

[Randy Dunlap]

On 08/18/10 15:46, Savoy, Pavan wrote:
> Randy,
> 
> 
>> -----Original Message-----
>> From: Randy Dunlap [mailto:randy.dunlap@oracle.com]
>> Sent: Wednesday, August 18, 2010 4:05 PM
>> To: Savoy, Pavan
>> Cc: devel@driverdev.osuosl.org; gregkh@suse.de; linux-kernel@vger.kernel.org;
>> alan@lxorguk.ukuu.org.uk
>> Subject: Re: [PATCH 0/3] drivers:staging:ti-st: patches
>>
>> On Mon, 26 Jul 2010 10:37:02 +0530 Pavan Savoy wrote:
>>
>>> On Fri, Jul 23, 2010 at 8:47 PM, Randy Dunlap <randy.dunlap@oracle.com>
>> wrote:
>>>> On 07/22/10 21:56, Pavan Savoy wrote:
>>>>> Randy,
>>>>>
>>>>> On Fri, Jul 23, 2010 at 1:23 AM, Randy Dunlap <randy.dunlap@oracle.com>
>> wrote:
>>>>>> On Thu, 22 Jul 2010 05:32:04 -0500 pavan_savoy@ti.com wrote:
>>>>>>
>>>>>>> From: Pavan Savoy <pavan_savoy@ti.com>
>>>>>>>
>>>>>>> The following patches cleanup bit of a mess and also adds functionality
>> to protocol drivers.
>>>>>>> with the 3rd patch now providing context to even the protocol drivers,
>> the single device limit
>>>>>>> or support for multiple devices would be easier to implement.
>>>>>>>
>>>>>>> These patches depend on the previously submitted
>>>>>>> 0001-drivers-staging-ti-st-make-use-of-linux-err-codes.patch
>>>>>>> commit d39d49b393d94f4137cee4f64526a4695352f183
>>>>>>>
>>>>>>> Pavan Savoy (3):
>>>>>>>   drivers:staging:ti-st: smarten, reduce logs
>>>>>>>   drivers:staging:ti-st: cleanup code comments
>>>>>>>   drivers:staging:ti-st: give proto drivers context
>>>>>>>
>>>>>>>  drivers/staging/ti-st/bt_drv.c  |   23 +++++---
>>>>>>>  drivers/staging/ti-st/st.h      |   52 +++++++++--------
>>>>>>>  drivers/staging/ti-st/st_core.c |  118 +++++++++++++++++++------------
>> --------
>>>>>>>  drivers/staging/ti-st/st_core.h |   74 +++++++++++++++++--------
>>>>>>>  drivers/staging/ti-st/st_kim.c  |   73 ++++++++++++++----------
>>>>>>>  drivers/staging/ti-st/st_kim.h  |   77 ++++++++++++++++---------
>>>>>>>  drivers/staging/ti-st/st_ll.c   |    4 +-
>>>>>>>  drivers/staging/ti-st/st_ll.h   |    9 +++-
>>>>>>>  8 files changed, 255 insertions(+), 175 deletions(-)
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> I have reported this error a few times.  Where is the patch for it??
>>>>>>
>>>>>> ERROR: "st_get_plat_device" [drivers/staging/ti-st/st_drv.ko] undefined!
>>>>>
>>>>>
>>>>> Yes, on one of the earlier patch sets, I had mentioned that the ST
>>>>> driver being a platform device, needs definition in any of the
>>>>> arch/XX/mach-XX/board-XX.c or devices.c or somewhere...
>>>>>
>>>>> and hence it is in that board-XX.c file that the symbol
>>>>> st_get_plat_device needs to be exported, the reason for that being,
>>>>>
>>>>> ST driver being both a TTY ldisc driver and platform driver, in TTY
>>>>> contexts it would need to refer to platform driver's data. So it does
>>>>> a st_get_plat_device which returns the platform device structure, and
>>>>> then does a dev_getdrvdata from it.
>>>>>
>>>>> here's a snippet of code ...
>>>>> /*
>>>>>  * ST related functions related functions
>>>>>  */
>>>>> #include <linux/platform_device.h>
>>>>>
>>>>> long gpios[] = { 55, -1, -1 };
>>>>> static struct platform_device ti_st_device = {
>>>>>         .name           = "kim",
>>>>>         .id             = -1,
>>>>>         .dev.platform_data      = &gpios,
>>>>> };
>>>>>
>>>>> struct platform_device *st_get_plat_device(void)
>>>>> {
>>>>>         return &ti_st_device;
>>>>> }
>>>>> EXPORT_SYMBOL(st_get_plat_device);
>>>>>
>>>>> static __init int add_ti_st_device(void)
>>>>> {
>>>>>         platform_device_register(&ti_st_device);
>>>>>         dev_info(&ti_st_device.dev,"registered platform TI ST device\n");
>>>>>
>>>>>         return 0;
>>>>> }
>>>>> device_initcall(add_ti_st_device);
>>>>>
>>>>>
>>>>> We have that in our local trees in arch/arm/mach-omap2/board-sdp4430.c
>>>>
>>>> Thanks for the explanation.
>>>>
>>>> Is the driver platform-specific?
>>>> E.g., should it not even be built on x86?
>>>
>>> Yes. Requirement of the hardware is very much a must.
>>> However it is a separate peripheral (WiLink 7 - uart interfaced), may
>>> be there is a x86 platform with this - but certainly not desktops.
>>>
>>> on linux-next, I generally put in that st_dev.c file for x86 - verify
>>> whether it builds as a module, inserts/rmmod, basic other
>>> functionalities (which doesn't involve response from chip..)
>>> But verify full functionality on board which constitutes that.
>>
>> Hi,
>> Please make this driver build cleanly on X86 or prevent it from being built
>> there.
> 
> Do you do something like a make allyesconfig?

I do 50 X86 randconfig builds per night (scripted, while I sleep).
As long as STAGING is enabled and RFKILL is enabled, then TI_ST can be enabled,
so usually it fails to build.

> I am having a hard time figuring out why is it building for you in the first place?
> make defconfig doesn't build it, neither does make i386_defconfig.

That is irrelevant.  It's now in mainline and it is causing build failures.

> May be a patch which does 'default N' in drivers/staging/ti-st/Kconfig, would suffice?

Nope.

> Please suggest...

>>>>>> ERROR: "st_get_plat_device" [drivers/staging/ti-st/st_drv.ko] undefined!

st_get_plat_device will have to be defined for any build platform,
or not referenced, or make it not possible to enable TI_ST unless
st_get_plat_device is defined.


-- 
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Re: [PATCH 0/3] drivers:staging:ti-st: patches

[Pavan Savoy]

Randy,

On Thu, Aug 19, 2010 at 4:26 AM, Randy Dunlap <randy.dunlap@oracle.com> wrote:
> On 08/18/10 15:46, Savoy, Pavan wrote:
>> Randy,
>>
>>
>>> -----Original Message-----
>>> From: Randy Dunlap [mailto:randy.dunlap@oracle.com]
>>> Sent: Wednesday, August 18, 2010 4:05 PM
>>> To: Savoy, Pavan
>>> Cc: devel@driverdev.osuosl.org; gregkh@suse.de; linux-kernel@vger.kernel.org;
>>> alan@lxorguk.ukuu.org.uk
>>> Subject: Re: [PATCH 0/3] drivers:staging:ti-st: patches
>>>
>>> On Mon, 26 Jul 2010 10:37:02 +0530 Pavan Savoy wrote:
>>>
>>>> On Fri, Jul 23, 2010 at 8:47 PM, Randy Dunlap <randy.dunlap@oracle.com>
>>> wrote:
>>>>> On 07/22/10 21:56, Pavan Savoy wrote:
>>>>>> Randy,
>>>>>>
>>>>>> On Fri, Jul 23, 2010 at 1:23 AM, Randy Dunlap <randy.dunlap@oracle.com>
>>> wrote:
>>>>>>> On Thu, 22 Jul 2010 05:32:04 -0500 pavan_savoy@ti.com wrote:
>>>>>>>
>>>>>>>> From: Pavan Savoy <pavan_savoy@ti.com>
>>>>>>>>
>>>>>>>> The following patches cleanup bit of a mess and also adds functionality
>>> to protocol drivers.
>>>>>>>> with the 3rd patch now providing context to even the protocol drivers,
>>> the single device limit
>>>>>>>> or support for multiple devices would be easier to implement.
>>>>>>>>
>>>>>>>> These patches depend on the previously submitted
>>>>>>>> 0001-drivers-staging-ti-st-make-use-of-linux-err-codes.patch
>>>>>>>> commit d39d49b393d94f4137cee4f64526a4695352f183
>>>>>>>>
>>>>>>>> Pavan Savoy (3):
>>>>>>>>   drivers:staging:ti-st: smarten, reduce logs
>>>>>>>>   drivers:staging:ti-st: cleanup code comments
>>>>>>>>   drivers:staging:ti-st: give proto drivers context
>>>>>>>>
>>>>>>>>  drivers/staging/ti-st/bt_drv.c  |   23 +++++---
>>>>>>>>  drivers/staging/ti-st/st.h      |   52 +++++++++--------
>>>>>>>>  drivers/staging/ti-st/st_core.c |  118 +++++++++++++++++++------------
>>> --------
>>>>>>>>  drivers/staging/ti-st/st_core.h |   74 +++++++++++++++++--------
>>>>>>>>  drivers/staging/ti-st/st_kim.c  |   73 ++++++++++++++----------
>>>>>>>>  drivers/staging/ti-st/st_kim.h  |   77 ++++++++++++++++---------
>>>>>>>>  drivers/staging/ti-st/st_ll.c   |    4 +-
>>>>>>>>  drivers/staging/ti-st/st_ll.h   |    9 +++-
>>>>>>>>  8 files changed, 255 insertions(+), 175 deletions(-)
>>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> I have reported this error a few times.  Where is the patch for it??
>>>>>>>
>>>>>>> ERROR: "st_get_plat_device" [drivers/staging/ti-st/st_drv.ko] undefined!
>>>>>>
>>>>>>
>>>>>> Yes, on one of the earlier patch sets, I had mentioned that the ST
>>>>>> driver being a platform device, needs definition in any of the
>>>>>> arch/XX/mach-XX/board-XX.c or devices.c or somewhere...
>>>>>>
>>>>>> and hence it is in that board-XX.c file that the symbol
>>>>>> st_get_plat_device needs to be exported, the reason for that being,
>>>>>>
>>>>>> ST driver being both a TTY ldisc driver and platform driver, in TTY
>>>>>> contexts it would need to refer to platform driver's data. So it does
>>>>>> a st_get_plat_device which returns the platform device structure, and
>>>>>> then does a dev_getdrvdata from it.
>>>>>>
>>>>>> here's a snippet of code ...
>>>>>> /*
>>>>>>  * ST related functions related functions
>>>>>>  */
>>>>>> #include <linux/platform_device.h>
>>>>>>
>>>>>> long gpios[] = { 55, -1, -1 };
>>>>>> static struct platform_device ti_st_device = {
>>>>>>         .name           = "kim",
>>>>>>         .id             = -1,
>>>>>>         .dev.platform_data      = &gpios,
>>>>>> };
>>>>>>
>>>>>> struct platform_device *st_get_plat_device(void)
>>>>>> {
>>>>>>         return &ti_st_device;
>>>>>> }
>>>>>> EXPORT_SYMBOL(st_get_plat_device);
>>>>>>
>>>>>> static __init int add_ti_st_device(void)
>>>>>> {
>>>>>>         platform_device_register(&ti_st_device);
>>>>>>         dev_info(&ti_st_device.dev,"registered platform TI ST device\n");
>>>>>>
>>>>>>         return 0;
>>>>>> }
>>>>>> device_initcall(add_ti_st_device);
>>>>>>
>>>>>>
>>>>>> We have that in our local trees in arch/arm/mach-omap2/board-sdp4430.c
>>>>>
>>>>> Thanks for the explanation.
>>>>>
>>>>> Is the driver platform-specific?
>>>>> E.g., should it not even be built on x86?
>>>>
>>>> Yes. Requirement of the hardware is very much a must.
>>>> However it is a separate peripheral (WiLink 7 - uart interfaced), may
>>>> be there is a x86 platform with this - but certainly not desktops.
>>>>
>>>> on linux-next, I generally put in that st_dev.c file for x86 - verify
>>>> whether it builds as a module, inserts/rmmod, basic other
>>>> functionalities (which doesn't involve response from chip..)
>>>> But verify full functionality on board which constitutes that.
>>>
>>> Hi,
>>> Please make this driver build cleanly on X86 or prevent it from being built
>>> there.
>>
>> Do you do something like a make allyesconfig?
>
> I do 50 X86 randconfig builds per night (scripted, while I sleep).
> As long as STAGING is enabled and RFKILL is enabled, then TI_ST can be enabled,
> so usually it fails to build.
>
>> I am having a hard time figuring out why is it building for you in the first place?
>> make defconfig doesn't build it, neither does make i386_defconfig.
>
> That is irrelevant.  It's now in mainline and it is causing build failures.
>
>> May be a patch which does 'default N' in drivers/staging/ti-st/Kconfig, would suffice?
>
> Nope.
>
>> Please suggest...
>
>>>>>>> ERROR: "st_get_plat_device" [drivers/staging/ti-st/st_drv.ko] undefined!
>
> st_get_plat_device will have to be defined for any build platform,
> or not referenced, or make it not possible to enable TI_ST unless
> st_get_plat_device is defined.

I was thinking as to why do I even have this platform device reference
via an exported symbol,
and thanks for pointing out, I don't even need this function
"st_get_plat_device()".
I already have the platform_device reference in my _probe function !!!..
I will send out a patch 2morrow.

This actually sorts of clarifies how I  support multiple devices too...
This is how my probe would then look like ...
static int kim_probe(struct platform_device *pdev)
{
....
...
platform_devices_i_have[id] = pdev;
.....
....
}

and st_get_plat_device(int id) would be defined as...
{
return platform_devices_i_have[id];
}

Does this sound ok ?


> --
> ~Randy
> *** Remember to use Documentation/SubmitChecklist when testing your code ***
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>
>
>

Re: [PATCH 0/3] drivers:staging:ti-st: patches

[Randy Dunlap]

On 08/18/10 18:29, Pavan Savoy wrote:
> Randy,
> 
> On Thu, Aug 19, 2010 at 4:26 AM, Randy Dunlap <randy.dunlap@oracle.com> wrote:
>> On 08/18/10 15:46, Savoy, Pavan wrote:
>>> Randy,
>>>
>>>
>>>> -----Original Message-----
>>>> From: Randy Dunlap [mailto:randy.dunlap@oracle.com]
>>>> Sent: Wednesday, August 18, 2010 4:05 PM
>>>> To: Savoy, Pavan
>>>> Cc: devel@driverdev.osuosl.org; gregkh@suse.de; linux-kernel@vger.kernel.org;
>>>> alan@lxorguk.ukuu.org.uk
>>>> Subject: Re: [PATCH 0/3] drivers:staging:ti-st: patches
>>>>
>>>> On Mon, 26 Jul 2010 10:37:02 +0530 Pavan Savoy wrote:
>>>>
>>>>> On Fri, Jul 23, 2010 at 8:47 PM, Randy Dunlap <randy.dunlap@oracle.com>
>>>> wrote:
>>>>>> On 07/22/10 21:56, Pavan Savoy wrote:
>>>>>>> Randy,
>>>>>>>
>>>>>>> On Fri, Jul 23, 2010 at 1:23 AM, Randy Dunlap <randy.dunlap@oracle.com>
>>>> wrote:
>>>>>>>> On Thu, 22 Jul 2010 05:32:04 -0500 pavan_savoy@ti.com wrote:
>>>>>>>>
>>>>>>>>> From: Pavan Savoy <pavan_savoy@ti.com>
>>>>>>>>>
>>>>>>>>> The following patches cleanup bit of a mess and also adds functionality
>>>> to protocol drivers.
>>>>>>>>> with the 3rd patch now providing context to even the protocol drivers,
>>>> the single device limit
>>>>>>>>> or support for multiple devices would be easier to implement.
>>>>>>>>>
>>>>>>>>> These patches depend on the previously submitted
>>>>>>>>> 0001-drivers-staging-ti-st-make-use-of-linux-err-codes.patch
>>>>>>>>> commit d39d49b393d94f4137cee4f64526a4695352f183
>>>>>>>>>
>>>>>>>>> Pavan Savoy (3):
>>>>>>>>>   drivers:staging:ti-st: smarten, reduce logs
>>>>>>>>>   drivers:staging:ti-st: cleanup code comments
>>>>>>>>>   drivers:staging:ti-st: give proto drivers context
>>>>>>>>>
>>>>>>>>>  drivers/staging/ti-st/bt_drv.c  |   23 +++++---
>>>>>>>>>  drivers/staging/ti-st/st.h      |   52 +++++++++--------
>>>>>>>>>  drivers/staging/ti-st/st_core.c |  118 +++++++++++++++++++------------
>>>> --------
>>>>>>>>>  drivers/staging/ti-st/st_core.h |   74 +++++++++++++++++--------
>>>>>>>>>  drivers/staging/ti-st/st_kim.c  |   73 ++++++++++++++----------
>>>>>>>>>  drivers/staging/ti-st/st_kim.h  |   77 ++++++++++++++++---------
>>>>>>>>>  drivers/staging/ti-st/st_ll.c   |    4 +-
>>>>>>>>>  drivers/staging/ti-st/st_ll.h   |    9 +++-
>>>>>>>>>  8 files changed, 255 insertions(+), 175 deletions(-)
>>>>>>>>
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> I have reported this error a few times.  Where is the patch for it??
>>>>>>>>
>>>>>>>> ERROR: "st_get_plat_device" [drivers/staging/ti-st/st_drv.ko] undefined!
>>>>>>>
>>>>>>>
>>>>>>> Yes, on one of the earlier patch sets, I had mentioned that the ST
>>>>>>> driver being a platform device, needs definition in any of the
>>>>>>> arch/XX/mach-XX/board-XX.c or devices.c or somewhere...
>>>>>>>
>>>>>>> and hence it is in that board-XX.c file that the symbol
>>>>>>> st_get_plat_device needs to be exported, the reason for that being,
>>>>>>>
>>>>>>> ST driver being both a TTY ldisc driver and platform driver, in TTY
>>>>>>> contexts it would need to refer to platform driver's data. So it does
>>>>>>> a st_get_plat_device which returns the platform device structure, and
>>>>>>> then does a dev_getdrvdata from it.
>>>>>>>
>>>>>>> here's a snippet of code ...
>>>>>>> /*
>>>>>>>  * ST related functions related functions
>>>>>>>  */
>>>>>>> #include <linux/platform_device.h>
>>>>>>>
>>>>>>> long gpios[] = { 55, -1, -1 };
>>>>>>> static struct platform_device ti_st_device = {
>>>>>>>         .name           = "kim",
>>>>>>>         .id             = -1,
>>>>>>>         .dev.platform_data      = &gpios,
>>>>>>> };
>>>>>>>
>>>>>>> struct platform_device *st_get_plat_device(void)
>>>>>>> {
>>>>>>>         return &ti_st_device;
>>>>>>> }
>>>>>>> EXPORT_SYMBOL(st_get_plat_device);
>>>>>>>
>>>>>>> static __init int add_ti_st_device(void)
>>>>>>> {
>>>>>>>         platform_device_register(&ti_st_device);
>>>>>>>         dev_info(&ti_st_device.dev,"registered platform TI ST device\n");
>>>>>>>
>>>>>>>         return 0;
>>>>>>> }
>>>>>>> device_initcall(add_ti_st_device);
>>>>>>>
>>>>>>>
>>>>>>> We have that in our local trees in arch/arm/mach-omap2/board-sdp4430.c
>>>>>>
>>>>>> Thanks for the explanation.
>>>>>>
>>>>>> Is the driver platform-specific?
>>>>>> E.g., should it not even be built on x86?
>>>>>
>>>>> Yes. Requirement of the hardware is very much a must.
>>>>> However it is a separate peripheral (WiLink 7 - uart interfaced), may
>>>>> be there is a x86 platform with this - but certainly not desktops.
>>>>>
>>>>> on linux-next, I generally put in that st_dev.c file for x86 - verify
>>>>> whether it builds as a module, inserts/rmmod, basic other
>>>>> functionalities (which doesn't involve response from chip..)
>>>>> But verify full functionality on board which constitutes that.
>>>>
>>>> Hi,
>>>> Please make this driver build cleanly on X86 or prevent it from being built
>>>> there.
>>>
>>> Do you do something like a make allyesconfig?
>>
>> I do 50 X86 randconfig builds per night (scripted, while I sleep).
>> As long as STAGING is enabled and RFKILL is enabled, then TI_ST can be enabled,
>> so usually it fails to build.
>>
>>> I am having a hard time figuring out why is it building for you in the first place?
>>> make defconfig doesn't build it, neither does make i386_defconfig.
>>
>> That is irrelevant.  It's now in mainline and it is causing build failures.
>>
>>> May be a patch which does 'default N' in drivers/staging/ti-st/Kconfig, would suffice?
>>
>> Nope.
>>
>>> Please suggest...
>>
>>>>>>>> ERROR: "st_get_plat_device" [drivers/staging/ti-st/st_drv.ko] undefined!
>>
>> st_get_plat_device will have to be defined for any build platform,
>> or not referenced, or make it not possible to enable TI_ST unless
>> st_get_plat_device is defined.
> 
> I was thinking as to why do I even have this platform device reference
> via an exported symbol,
> and thanks for pointing out, I don't even need this function
> "st_get_plat_device()".
> I already have the platform_device reference in my _probe function !!!..
> I will send out a patch 2morrow.

That's all good to hear.

> This actually sorts of clarifies how I  support multiple devices too...
> This is how my probe would then look like ...
> static int kim_probe(struct platform_device *pdev)
> {
> ....
> ...
> platform_devices_i_have[id] = pdev;
> .....
> ....
> }
> 
> and st_get_plat_device(int id) would be defined as...
> {
> return platform_devices_i_have[id];
> }
> 
> Does this sound ok ?

Sounds OK, but let's see the patch (tomorrow).

thanks,
-- 
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***