[PATCH] Revert "drivers/misc/ti-st: remove gpio handling"

[PATCH] Revert "drivers/misc/ti-st: remove gpio handling"

[Luciano Coelho]

This reverts commit eccf2979b2c034b516e01b8a104c3739f7ef07d1.

The reason is that it broke TI WiLink shared transport on Panda.
Also, callback functions should not be added to board files anymore,
so revert to implementing the power functions in the driver itself.

Additionally, changed a variable name ('status' to 'err') so that this
revert compiles properly.

Cc: stable <stable@vger.kernel.org> [3.7]
Signed-off-by: Luciano Coelho <coelho@ti.com>
---
 drivers/misc/ti-st/st_kim.c |   37 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/drivers/misc/ti-st/st_kim.c b/drivers/misc/ti-st/st_kim.c
index 9ff942a..83269f1 100644
--- a/drivers/misc/ti-st/st_kim.c
+++ b/drivers/misc/ti-st/st_kim.c
@@ -468,6 +468,11 @@ long st_kim_start(void *kim_data)
 		if (pdata->chip_enable)
 			pdata->chip_enable(kim_gdata);
 
+		/* Configure BT nShutdown to HIGH state */
+		gpio_set_value(kim_gdata->nshutdown, GPIO_LOW);
+		mdelay(5);	/* FIXME: a proper toggle */
+		gpio_set_value(kim_gdata->nshutdown, GPIO_HIGH);
+		mdelay(100);
 		/* re-initialize the completion */
 		INIT_COMPLETION(kim_gdata->ldisc_installed);
 		/* send notification to UIM */
@@ -509,7 +514,8 @@ long st_kim_start(void *kim_data)
  *	(b) upon failure to either install ldisc or download firmware.
  *	The function is responsible to (a) notify UIM about un-installation,
  *	(b) flush UART if the ldisc was installed.
- *	(c) invoke platform's chip disabling routine.
+ *	(c) reset BT_EN - pull down nshutdown at the end.
+ *	(d) invoke platform's chip disabling routine.
  */
 long st_kim_stop(void *kim_data)
 {
@@ -541,6 +547,13 @@ long st_kim_stop(void *kim_data)
 		err = -ETIMEDOUT;
 	}
 
+	/* By default configure BT nShutdown to LOW state */
+	gpio_set_value(kim_gdata->nshutdown, GPIO_LOW);
+	mdelay(1);
+	gpio_set_value(kim_gdata->nshutdown, GPIO_HIGH);
+	mdelay(1);
+	gpio_set_value(kim_gdata->nshutdown, GPIO_LOW);
+
 	/* platform specific disable */
 	if (pdata->chip_disable)
 		pdata->chip_disable(kim_gdata);
@@ -733,6 +746,20 @@ static int kim_probe(struct platform_device *pdev)
 	/* refer to itself */
 	kim_gdata->core_data->kim_data = kim_gdata;
 
+	/* Claim the chip enable nShutdown gpio from the system */
+	kim_gdata->nshutdown = pdata->nshutdown_gpio;
+	err = gpio_request(kim_gdata->nshutdown, "kim");
+	if (unlikely(err)) {
+		pr_err(" gpio %ld request failed ", kim_gdata->nshutdown);
+		return err;
+	}
+
+	/* Configure nShutdown GPIO as output=0 */
+	err = gpio_direction_output(kim_gdata->nshutdown, 0);
+	if (unlikely(err)) {
+		pr_err(" unable to configure gpio %ld", kim_gdata->nshutdown);
+		return err;
+	}
 	/* get reference of pdev for request_firmware
 	 */
 	kim_gdata->kim_pdev = pdev;
@@ -779,10 +806,18 @@ err_core_init:
 
 static int kim_remove(struct platform_device *pdev)
 {
+	/* free the GPIOs requested */
+	struct ti_st_plat_data	*pdata = pdev->dev.platform_data;
 	struct kim_data_s	*kim_gdata;
 
 	kim_gdata = dev_get_drvdata(&pdev->dev);
 
+	/* Free the Bluetooth/FM/GPIO
+	 * nShutdown gpio from the system
+	 */
+	gpio_free(pdata->nshutdown_gpio);
+	pr_info("nshutdown GPIO Freed");
+
 	debugfs_remove_recursive(kim_debugfs_dir);
 	sysfs_remove_group(&pdev->dev.kobj, &uim_attr_grp);
 	pr_info("sysfs entries removed");
-- 
1.7.10.4

Re: [PATCH] Revert "drivers/misc/ti-st: remove gpio handling"

[Tony Lindgren]

* Luciano Coelho <coelho@ti.com> [130121 03:16]:
> This reverts commit eccf2979b2c034b516e01b8a104c3739f7ef07d1.
> 
> The reason is that it broke TI WiLink shared transport on Panda.
> Also, callback functions should not be added to board files anymore,
> so revert to implementing the power functions in the driver itself.
> 
> Additionally, changed a variable name ('status' to 'err') so that this
> revert compiles properly.
> 
> Cc: stable <stable@vger.kernel.org> [3.7]
> Signed-off-by: Luciano Coelho <coelho@ti.com>

This also makes sense from converting to DT based boot as the GPIO
calls should happen in the driver. Those mdelays probably can be fixed,
but since this is a revert to a known working version:

Acked-by: Tony Lindgren <tony@atomide.com>

> ---
>  drivers/misc/ti-st/st_kim.c |   37 ++++++++++++++++++++++++++++++++++++-
>  1 file changed, 36 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/misc/ti-st/st_kim.c b/drivers/misc/ti-st/st_kim.c
> index 9ff942a..83269f1 100644
> --- a/drivers/misc/ti-st/st_kim.c
> +++ b/drivers/misc/ti-st/st_kim.c
> @@ -468,6 +468,11 @@ long st_kim_start(void *kim_data)
>  		if (pdata->chip_enable)
>  			pdata->chip_enable(kim_gdata);
>  
> +		/* Configure BT nShutdown to HIGH state */
> +		gpio_set_value(kim_gdata->nshutdown, GPIO_LOW);
> +		mdelay(5);	/* FIXME: a proper toggle */
> +		gpio_set_value(kim_gdata->nshutdown, GPIO_HIGH);
> +		mdelay(100);
>  		/* re-initialize the completion */
>  		INIT_COMPLETION(kim_gdata->ldisc_installed);
>  		/* send notification to UIM */
> @@ -509,7 +514,8 @@ long st_kim_start(void *kim_data)
>   *	(b) upon failure to either install ldisc or download firmware.
>   *	The function is responsible to (a) notify UIM about un-installation,
>   *	(b) flush UART if the ldisc was installed.
> - *	(c) invoke platform's chip disabling routine.
> + *	(c) reset BT_EN - pull down nshutdown at the end.
> + *	(d) invoke platform's chip disabling routine.
>   */
>  long st_kim_stop(void *kim_data)
>  {
> @@ -541,6 +547,13 @@ long st_kim_stop(void *kim_data)
>  		err = -ETIMEDOUT;
>  	}
>  
> +	/* By default configure BT nShutdown to LOW state */
> +	gpio_set_value(kim_gdata->nshutdown, GPIO_LOW);
> +	mdelay(1);
> +	gpio_set_value(kim_gdata->nshutdown, GPIO_HIGH);
> +	mdelay(1);
> +	gpio_set_value(kim_gdata->nshutdown, GPIO_LOW);
> +
>  	/* platform specific disable */
>  	if (pdata->chip_disable)
>  		pdata->chip_disable(kim_gdata);
> @@ -733,6 +746,20 @@ static int kim_probe(struct platform_device *pdev)
>  	/* refer to itself */
>  	kim_gdata->core_data->kim_data = kim_gdata;
>  
> +	/* Claim the chip enable nShutdown gpio from the system */
> +	kim_gdata->nshutdown = pdata->nshutdown_gpio;
> +	err = gpio_request(kim_gdata->nshutdown, "kim");
> +	if (unlikely(err)) {
> +		pr_err(" gpio %ld request failed ", kim_gdata->nshutdown);
> +		return err;
> +	}
> +
> +	/* Configure nShutdown GPIO as output=0 */
> +	err = gpio_direction_output(kim_gdata->nshutdown, 0);
> +	if (unlikely(err)) {
> +		pr_err(" unable to configure gpio %ld", kim_gdata->nshutdown);
> +		return err;
> +	}
>  	/* get reference of pdev for request_firmware
>  	 */
>  	kim_gdata->kim_pdev = pdev;
> @@ -779,10 +806,18 @@ err_core_init:
>  
>  static int kim_remove(struct platform_device *pdev)
>  {
> +	/* free the GPIOs requested */
> +	struct ti_st_plat_data	*pdata = pdev->dev.platform_data;
>  	struct kim_data_s	*kim_gdata;
>  
>  	kim_gdata = dev_get_drvdata(&pdev->dev);
>  
> +	/* Free the Bluetooth/FM/GPIO
> +	 * nShutdown gpio from the system
> +	 */
> +	gpio_free(pdata->nshutdown_gpio);
> +	pr_info("nshutdown GPIO Freed");
> +
>  	debugfs_remove_recursive(kim_debugfs_dir);
>  	sysfs_remove_group(&pdev->dev.kobj, &uim_attr_grp);
>  	pr_info("sysfs entries removed");
> -- 
> 1.7.10.4
> 

Re: [PATCH] Revert "drivers/misc/ti-st: remove gpio handling"

[Luciano Coelho]

On Tue, 2013-01-22 at 12:14 -0800, Tony Lindgren wrote:
> * Luciano Coelho <coelho@ti.com> [130121 03:16]:
> > This reverts commit eccf2979b2c034b516e01b8a104c3739f7ef07d1.
> > 
> > The reason is that it broke TI WiLink shared transport on Panda.
> > Also, callback functions should not be added to board files anymore,
> > so revert to implementing the power functions in the driver itself.
> > 
> > Additionally, changed a variable name ('status' to 'err') so that this
> > revert compiles properly.
> > 
> > Cc: stable <stable@vger.kernel.org> [3.7]
> > Signed-off-by: Luciano Coelho <coelho@ti.com>
> 
> This also makes sense from converting to DT based boot as the GPIO
> calls should happen in the driver. Those mdelays probably can be fixed,
> but since this is a revert to a known working version:

I thought about checking exactly what we need as delays and whether we
really need to do all that toggling.  At least in some boards, the
toggles are not needed, but I don't know for sure whether it works in
all different variations of the chip, so I kept them to be safe.  Also,
I tried to deviate as little as possible from a plain revert, since I'd
like to get this into stable as well.


> Acked-by: Tony Lindgren <tony@atomide.com>

Thanks!

--
Luca.

Re: [PATCH] Revert "drivers/misc/ti-st: remove gpio handling"

[Peter Ujfalusi]

On 01/22/2013 10:31 PM, Luciano Coelho wrote:
> I thought about checking exactly what we need as delays and whether we
> really need to do all that toggling.  At least in some boards, the
> toggles are not needed, but I don't know for sure whether it works in
> all different variations of the chip, so I kept them to be safe.  Also,
> I tried to deviate as little as possible from a plain revert, since I'd
> like to get this into stable as well.

I can not find the documentation for the WiLink, but I would guess that it
needs some delay between high->low(off)->(wait)->high(on)->(power-on-time)->up

-- 
Péter

Re: [PATCH] Revert "drivers/misc/ti-st: remove gpio handling"

[Peter Ujfalusi]

On 01/23/2013 10:04 AM, Peter Ujfalusi wrote:
> On 01/22/2013 10:31 PM, Luciano Coelho wrote:
>> I thought about checking exactly what we need as delays and whether we
>> really need to do all that toggling.  At least in some boards, the
>> toggles are not needed, but I don't know for sure whether it works in
>> all different variations of the chip, so I kept them to be safe.  Also,
>> I tried to deviate as little as possible from a plain revert, since I'd
>> like to get this into stable as well.
> 
> I can not find the documentation for the WiLink, but I would guess that it
> needs some delay between high->low(off)->(wait)->high(on)->(power-on-time)->up

But mdelay() looks a bit overkill IMHO.

-- 
Péter

Re: [PATCH] Revert "drivers/misc/ti-st: remove gpio handling"

[Luciano Coelho]

On Wed, 2013-01-23 at 10:05 +0100, Peter Ujfalusi wrote:
> On 01/23/2013 10:04 AM, Peter Ujfalusi wrote:
> > On 01/22/2013 10:31 PM, Luciano Coelho wrote:
> >> I thought about checking exactly what we need as delays and whether we
> >> really need to do all that toggling.  At least in some boards, the
> >> toggles are not needed, but I don't know for sure whether it works in
> >> all different variations of the chip, so I kept them to be safe.  Also,
> >> I tried to deviate as little as possible from a plain revert, since I'd
> >> like to get this into stable as well.
> > 
> > I can not find the documentation for the WiLink, but I would guess that it
> > needs some delay between high->low(off)->(wait)->high(on)->(power-on-time)->up

Yes, we probably need some kind of delay.  I'm not sure about the
low-high-low kind of thing though.  I'll try to figure it out.


> But mdelay() looks a bit overkill IMHO.

Yes, most likely we can use usleep or something.  I'll look into that as
part of my clean-up series. ;)

--
Luca.

Re: [PATCH] Revert "drivers/misc/ti-st: remove gpio handling"

[Peter Ujfalusi]

On 01/23/2013 10:14 AM, Luciano Coelho wrote:
> Yes, most likely we can use usleep or something.  I'll look into that as
> part of my clean-up series. ;)

I would suggest to use usleep_range() instead of usleep() if you can.

-- 
Péter

Re: [PATCH] Revert "drivers/misc/ti-st: remove gpio handling"

[Luciano Coelho]

On Wed, 2013-01-23 at 10:21 +0100, Peter Ujfalusi wrote:
> On 01/23/2013 10:14 AM, Luciano Coelho wrote:
> > Yes, most likely we can use usleep or something.  I'll look into that as
> > part of my clean-up series. ;)
> 
> I would suggest to use usleep_range() instead of usleep() if you can.

Yes, probably something like that.  This doesn't need to be very
accurate, but it must be just long enough.

--
Luca.