[PATCH 0/3] shared transport robustness fixes

[PATCH 0/3] shared transport robustness fixes

[pavan_savoy]

From: Pavan Savoy <pavan_savoy@ti.com>

Texas Instruments' WiLink connectivity chipset's Bluetooth, FM and GPS
wireless technologies are interfaced over single UART, which is enabled
by the TI-ST line discipline driver.

The following patches are updates and few fixes found during robustness testing
of the driver.
1. The patch handles the delayed tty receive_buf function called by the TTY layer.
2. Removes the dependency on rfkill for build from Kconfig
3. Fix for the skipping the change remote baud rate command.


Pavan Savoy (2):
  drivers:misc:ti-st: handle delayed tty receive
  drivers:misc:ti-st: remove rfkill dependency

Victor Goldenshtein (1):
  drivers:misc:ti-st: fix skip remote baud logic

 drivers/misc/ti-st/Kconfig   |    1 -
 drivers/misc/ti-st/st_core.c |   23 +++++++++++++----------
 drivers/misc/ti-st/st_kim.c  |    6 ++++--
 include/linux/ti_wilink_st.h |    3 ++-
 4 files changed, 19 insertions(+), 14 deletions(-)

[PATCH 1/3] drivers:misc:ti-st: handle delayed tty receive

[pavan_savoy]

From: Pavan Savoy <pavan_savoy@ti.com>

When certain technologies shutdown their interface without waiting for
the acknowledgement from the chip. The receive_buf from the TTY would be
invoked a while after the relevant technology is unregistered.

This patch introduces a new flag "is_registered" which maintains the
state of protocols BT, FM or GPS and thereby removes the need to clear
the protocol data from ST when protocols gets unregistered.

This fixes corner cases when HCI RESET is sent down from bluetooth stack
and the receive_buf is called from tty after 250ms before which
bluetooth would have unregistered from the system.
OR - when FM application decides to close down the device without
sending a power-off FM command resulting in some RDS data or interrupt
data coming in after the driver is unregistered.

Signed-off-by: Pavan Savoy <pavan_savoy@ti.com>
---
 drivers/misc/ti-st/st_core.c |   23 +++++++++++++----------
 include/linux/ti_wilink_st.h |    3 ++-
 2 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/drivers/misc/ti-st/st_core.c b/drivers/misc/ti-st/st_core.c
index 486117f..f91f82e 100644
--- a/drivers/misc/ti-st/st_core.c
+++ b/drivers/misc/ti-st/st_core.c
@@ -43,13 +43,15 @@ static void add_channel_to_table(struct st_data_s *st_gdata,
 	pr_info("%s: id %d\n", __func__, new_proto->chnl_id);
 	/* list now has the channel id as index itself */
 	st_gdata->list[new_proto->chnl_id] = new_proto;
+	st_gdata->is_registered[new_proto->chnl_id] = true;
 }
 
 static void remove_channel_from_table(struct st_data_s *st_gdata,
 		struct st_proto_s *proto)
 {
 	pr_info("%s: id %d\n", __func__, proto->chnl_id);
-	st_gdata->list[proto->chnl_id] = NULL;
+/*	st_gdata->list[proto->chnl_id] = NULL; */
+	st_gdata->is_registered[proto->chnl_id] = false;
 }
 
 /*
@@ -104,7 +106,7 @@ void st_send_frame(unsigned char chnl_id, struct st_data_s *st_gdata)
 
 	if (unlikely
 	    (st_gdata == NULL || st_gdata->rx_skb == NULL
-	     || st_gdata->list[chnl_id] == NULL)) {
+	     || st_gdata->is_registered[chnl_id] == false)) {
 		pr_err("chnl_id %d not registered, no data to send?",
 			   chnl_id);
 		kfree_skb(st_gdata->rx_skb);
@@ -141,14 +143,15 @@ void st_reg_complete(struct st_data_s *st_gdata, char err)
 	unsigned char i = 0;
 	pr_info(" %s ", __func__);
 	for (i = 0; i < ST_MAX_CHANNELS; i++) {
-		if (likely(st_gdata != NULL && st_gdata->list[i] != NULL &&
-			   st_gdata->list[i]->reg_complete_cb != NULL)) {
+		if (likely(st_gdata != NULL &&
+			st_gdata->is_registered[i] == true &&
+				st_gdata->list[i]->reg_complete_cb != NULL)) {
 			st_gdata->list[i]->reg_complete_cb
 				(st_gdata->list[i]->priv_data, err);
 			pr_info("protocol %d's cb sent %d\n", i, err);
 			if (err) { /* cleanup registered protocol */
 				st_gdata->protos_registered--;
-				st_gdata->list[i] = NULL;
+				st_gdata->is_registered[i] = false;
 			}
 		}
 	}
@@ -475,9 +478,9 @@ void kim_st_list_protocols(struct st_data_s *st_gdata, void *buf)
 {
 	seq_printf(buf, "[%d]\nBT=%c\nFM=%c\nGPS=%c\n",
 			st_gdata->protos_registered,
-			st_gdata->list[0x04] != NULL ? 'R' : 'U',
-			st_gdata->list[0x08] != NULL ? 'R' : 'U',
-			st_gdata->list[0x09] != NULL ? 'R' : 'U');
+			st_gdata->is_registered[0x04] == true ? 'R' : 'U',
+			st_gdata->is_registered[0x08] == true ? 'R' : 'U',
+			st_gdata->is_registered[0x09] == true ? 'R' : 'U');
 }
 
 /********************************************************************/
@@ -504,7 +507,7 @@ long st_register(struct st_proto_s *new_proto)
 		return -EPROTONOSUPPORT;
 	}
 
-	if (st_gdata->list[new_proto->chnl_id] != NULL) {
+	if (st_gdata->is_registered[new_proto->chnl_id] == true) {
 		pr_err("chnl_id %d already registered", new_proto->chnl_id);
 		return -EALREADY;
 	}
@@ -563,7 +566,7 @@ long st_register(struct st_proto_s *new_proto)
 		/* check for already registered once more,
 		 * since the above check is old
 		 */
-		if (st_gdata->list[new_proto->chnl_id] != NULL) {
+		if (st_gdata->is_registered[new_proto->chnl_id] == true) {
 			pr_err(" proto %d already registered ",
 				   new_proto->chnl_id);
 			return -EALREADY;
diff --git a/include/linux/ti_wilink_st.h b/include/linux/ti_wilink_st.h
index 7071ec5..b004e55 100644
--- a/include/linux/ti_wilink_st.h
+++ b/include/linux/ti_wilink_st.h
@@ -140,12 +140,12 @@ extern long st_unregister(struct st_proto_s *);
  */
 struct st_data_s {
 	unsigned long st_state;
-	struct tty_struct *tty;
 	struct sk_buff *tx_skb;
 #define ST_TX_SENDING	1
 #define ST_TX_WAKEUP	2
 	unsigned long tx_state;
 	struct st_proto_s *list[ST_MAX_CHANNELS];
+	bool is_registered[ST_MAX_CHANNELS];
 	unsigned long rx_state;
 	unsigned long rx_count;
 	struct sk_buff *rx_skb;
@@ -155,6 +155,7 @@ struct st_data_s {
 	unsigned char	protos_registered;
 	unsigned long ll_state;
 	void *kim_data;
+	struct tty_struct *tty;
 };
 
 /*
-- 
1.7.0.4

[PATCH 2/3] drivers:misc:ti-st: remove rfkill dependency

[pavan_savoy]

From: Pavan Savoy <pavan_savoy@ti.com>

rfkill is no longer used by Texas Instruments shared transport driver to
communicate with user-space.
This patch removes the dependency of rfkill to be enabled to build
shared transport driver in the Kconfig.

Signed-off-by: Pavan Savoy <pavan_savoy@ti.com>
---
 drivers/misc/ti-st/Kconfig |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/drivers/misc/ti-st/Kconfig b/drivers/misc/ti-st/Kconfig
index 2c8c3f3..7c3e106 100644
--- a/drivers/misc/ti-st/Kconfig
+++ b/drivers/misc/ti-st/Kconfig
@@ -5,7 +5,6 @@
 menu "Texas Instruments shared transport line discipline"
 config TI_ST
 	tristate "Shared transport core driver"
-	depends on RFKILL
 	select FW_LOADER
 	help
 	  This enables the shared transport core driver for TI
-- 
1.7.0.4

[PATCH 3/3] drivers:misc:ti-st: fix skip remote baud logic

[pavan_savoy]

From: Victor Goldenshtein <victorg@ti.com>

Texas Instruments WiLink connectivity combo chipsets support custom baud
rates over its UART interface.
This can be achieved by sending the change remote baud rate command.
Since the baud rate can be properly set by the user-space on shared
transport the remote chip-side and local host-side baud rate is change
by the User Space Init Manager.

This patch when encounters the same command inside the firmware,
attempts to skip such command since it is handled by the user-space.
The logic to skip the command when it is un-commented is fixed by this
patch.

Signed-off-by: Pavan Savoy <pavan_savoy@ti.com>
Author:    Victor Goldenshtein <victorg@ti.com>
---
 drivers/misc/ti-st/st_kim.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/misc/ti-st/st_kim.c b/drivers/misc/ti-st/st_kim.c
index 9ee4c78..b638a7d 100644
--- a/drivers/misc/ti-st/st_kim.c
+++ b/drivers/misc/ti-st/st_kim.c
@@ -244,9 +244,9 @@ void skip_change_remote_baud(unsigned char **ptr, long *len)
 		pr_err("invalid action after change remote baud command");
 	} else {
 		*ptr = *ptr + sizeof(struct bts_action) +
-			((struct bts_action *)nxt_action)->size;
+			((struct bts_action *)cur_action)->size;
 		*len = *len - (sizeof(struct bts_action) +
-				((struct bts_action *)nxt_action)->size);
+				((struct bts_action *)cur_action)->size);
 		/* warn user on not commenting these in firmware */
 		pr_warn("skipping the wait event of change remote baud");
 	}
@@ -385,6 +385,8 @@ static long download_firmware(struct kim_data_s *kim_gdata)
 	}
 	/* fw download complete */
 	release_firmware(kim_gdata->fw_entry);
+	if (len != 0)
+		pr_err("%s:failed, script not parsed completely", __func__);
 	return 0;
 }
 
-- 
1.7.0.4

Re: [PATCH 2/3] drivers:misc:ti-st: remove rfkill dependency

[Valdis.Kletnieks]

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

On Fri, 08 Apr 2011 04:57:43 CDT, pavan_savoy@ti.com said:
> From: Pavan Savoy <pavan_savoy@ti.com>
> 
> rfkill is no longer used by Texas Instruments shared transport driver to
> communicate with user-space.

Color me confoozled.  What's it using instead to provide the rfkill functionality?

(I'm OK on the actual patch, just the changelog is a tad ambiguous for those
of us tuning in late...)

[-- Attachment #2: Type: application/pgp-signature, Size: 227 bytes --]

Re: [PATCH 2/3] drivers:misc:ti-st: remove rfkill dependency

[Pavan Savoy]

On Sun, Apr 10, 2011 at 1:00 AM,  <Valdis.Kletnieks@vt.edu> wrote:
> On Fri, 08 Apr 2011 04:57:43 CDT, pavan_savoy@ti.com said:
>> From: Pavan Savoy <pavan_savoy@ti.com>
>>
>> rfkill is no longer used by Texas Instruments shared transport driver to
>> communicate with user-space.
>
> Color me confoozled.  What's it using instead to provide the rfkill functionality?
>
> (I'm OK on the actual patch, just the changelog is a tad ambiguous for those
> of us tuning in late...)
>

You can have a look at, Documentation/ABI/testing/sysfs-platform-kim

Re: [PATCH 0/3] shared transport robustness fixes

[Pavan Savoy]

On Fri, Apr 8, 2011 at 3:27 PM,  <pavan_savoy@ti.com> wrote:
> From: Pavan Savoy <pavan_savoy@ti.com>
>
> Texas Instruments' WiLink connectivity chipset's Bluetooth, FM and GPS
> wireless technologies are interfaced over single UART, which is enabled
> by the TI-ST line discipline driver.
>
> The following patches are updates and few fixes found during robustness testing
> of the driver.
> 1. The patch handles the delayed tty receive_buf function called by the TTY layer.
> 2. Removes the dependency on rfkill for build from Kconfig
> 3. Fix for the skipping the change remote baud rate command.

Greg,

Please review and queue up for merge, if its alright..

Thanks,
Pavan


> Pavan Savoy (2):
>  drivers:misc:ti-st: handle delayed tty receive
>  drivers:misc:ti-st: remove rfkill dependency
>
> Victor Goldenshtein (1):
>  drivers:misc:ti-st: fix skip remote baud logic
>
>  drivers/misc/ti-st/Kconfig   |    1 -
>  drivers/misc/ti-st/st_core.c |   23 +++++++++++++----------
>  drivers/misc/ti-st/st_kim.c  |    6 ++++--
>  include/linux/ti_wilink_st.h |    3 ++-
>  4 files changed, 19 insertions(+), 14 deletions(-)
>
>

Re: [PATCH 0/3] shared transport robustness fixes

[Greg KH]

On Thu, Apr 14, 2011 at 05:54:40PM +0530, Pavan Savoy wrote:
> On Fri, Apr 8, 2011 at 3:27 PM,  <pavan_savoy@ti.com> wrote:
> > From: Pavan Savoy <pavan_savoy@ti.com>
> >
> > Texas Instruments' WiLink connectivity chipset's Bluetooth, FM and GPS
> > wireless technologies are interfaced over single UART, which is enabled
> > by the TI-ST line discipline driver.
> >
> > The following patches are updates and few fixes found during robustness testing
> > of the driver.
> > 1. The patch handles the delayed tty receive_buf function called by the TTY layer.
> > 2. Removes the dependency on rfkill for build from Kconfig
> > 3. Fix for the skipping the change remote baud rate command.
> 
> Greg,
> 
> Please review and queue up for merge, if its alright..

It's in my "to-apply" queue, still fighting through it all...

thanks,

greg k-h

Re: [PATCH 3/3] drivers:misc:ti-st: fix skip remote baud logic

[Greg KH]

On Fri, Apr 08, 2011 at 04:57:44AM -0500, pavan_savoy@ti.com wrote:
> From: Victor Goldenshtein <victorg@ti.com>
> 
> Texas Instruments WiLink connectivity combo chipsets support custom baud
> rates over its UART interface.
> This can be achieved by sending the change remote baud rate command.
> Since the baud rate can be properly set by the user-space on shared
> transport the remote chip-side and local host-side baud rate is change
> by the User Space Init Manager.
> 
> This patch when encounters the same command inside the firmware,
> attempts to skip such command since it is handled by the user-space.
> The logic to skip the command when it is un-commented is fixed by this
> patch.
> 
> Signed-off-by: Pavan Savoy <pavan_savoy@ti.com>
> Author:    Victor Goldenshtein <victorg@ti.com>

Um, we need a signed-off-by from the original author here.  Victor, can
you please send me that?

thanks,

greg k-h