[PATCH 1/4] misc: ti-st: use GPIO_LOW for initial value of nshutdown

[PATCH 1/4] misc: ti-st: use GPIO_LOW for initial value of nshutdown

[Eric Nelson]

All other uses of the nshutdown gpio use macros GPIO_LOW and
GPIO_HIGH to allow changing the pin polarity.

Do this in gpio_direction_output as well.

Signed-off-by: Eric Nelson <eric.nelson-Q5RJGjKts06CY9SHAMCTRUEOCMrvLtNR@public.gmane.org>
---
 drivers/misc/ti-st/st_kim.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/misc/ti-st/st_kim.c b/drivers/misc/ti-st/st_kim.c
index 18e7a03..aaa17b0 100644
--- a/drivers/misc/ti-st/st_kim.c
+++ b/drivers/misc/ti-st/st_kim.c
@@ -817,7 +817,7 @@ static int kim_probe(struct platform_device *pdev)
 	}
 
 	/* Configure nShutdown GPIO as output=0 */
-	err = gpio_direction_output(kim_gdata->nshutdown, 0);
+	err = gpio_direction_output(kim_gdata->nshutdown, GPIO_LOW);
 	if (unlikely(err)) {
 		pr_err(" unable to configure gpio %d", kim_gdata->nshutdown);
 		return err;
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/4] misc: ti-st: use of_get_named_gpio_flags for nshutdown

[Eric Nelson]

Signed-off-by: Eric Nelson <eric.nelson-Q5RJGjKts06CY9SHAMCTRUEOCMrvLtNR@public.gmane.org>
---
 drivers/misc/ti-st/st_kim.c  | 21 +++++++++++++--------
 include/linux/ti_wilink_st.h |  6 +++---
 2 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/drivers/misc/ti-st/st_kim.c b/drivers/misc/ti-st/st_kim.c
index aaa17b0..c5a0e15 100644
--- a/drivers/misc/ti-st/st_kim.c
+++ b/drivers/misc/ti-st/st_kim.c
@@ -32,6 +32,7 @@
 #include <linux/sched.h>
 #include <linux/sysfs.h>
 #include <linux/tty.h>
+#include <linux/of_gpio.h>
 
 #include <linux/skbuff.h>
 #include <linux/ti_wilink_st.h>
@@ -482,9 +483,9 @@ long st_kim_start(void *kim_data)
 			pdata->chip_enable(kim_gdata);
 
 		/* Configure BT nShutdown to HIGH state */
-		gpio_set_value(kim_gdata->nshutdown, GPIO_LOW);
+		gpio_set_value(kim_gdata->nshutdown, kim_gdata->shutdown_val);
 		mdelay(5);	/* FIXME: a proper toggle */
-		gpio_set_value(kim_gdata->nshutdown, GPIO_HIGH);
+		gpio_set_value(kim_gdata->nshutdown, !kim_gdata->shutdown_val);
 		mdelay(100);
 		/* re-initialize the completion */
 		reinit_completion(&kim_gdata->ldisc_installed);
@@ -566,11 +567,11 @@ long st_kim_stop(void *kim_data)
 	}
 
 	/* By default configure BT nShutdown to LOW state */
-	gpio_set_value(kim_gdata->nshutdown, GPIO_LOW);
+	gpio_set_value(kim_gdata->nshutdown, kim_gdata->shutdown_val);
 	mdelay(1);
-	gpio_set_value(kim_gdata->nshutdown, GPIO_HIGH);
+	gpio_set_value(kim_gdata->nshutdown, !kim_gdata->shutdown_val);
 	mdelay(1);
-	gpio_set_value(kim_gdata->nshutdown, GPIO_LOW);
+	gpio_set_value(kim_gdata->nshutdown, kim_gdata->shutdown_val);
 
 	/* platform specific disable */
 	if (pdata->chip_disable)
@@ -749,6 +750,7 @@ static struct ti_st_plat_data *get_platform_data(struct device *dev)
 {
 	struct device_node *np = dev->of_node;
 	const u32 *dt_property;
+	enum of_gpio_flags flags;
 	int len;
 
 	dt_pdata = kzalloc(sizeof(*dt_pdata), GFP_KERNEL);
@@ -759,8 +761,9 @@ static struct ti_st_plat_data *get_platform_data(struct device *dev)
 	dt_property = of_get_property(np, "dev_name", &len);
 	if (dt_property)
 		memcpy(&dt_pdata->dev_name, dt_property, len);
-	of_property_read_u32(np, "nshutdown_gpio",
-			     &dt_pdata->nshutdown_gpio);
+	dt_pdata->nshutdown_gpio = of_get_named_gpio_flags
+					(np, "nshutdown_gpio", 0, &flags);
+	dt_pdata->shutdown_val = !(flags & OF_GPIO_ACTIVE_LOW);
 	of_property_read_u32(np, "flow_cntrl", &dt_pdata->flow_cntrl);
 	of_property_read_u32(np, "baud_rate", &dt_pdata->baud_rate);
 
@@ -810,6 +813,7 @@ static int kim_probe(struct platform_device *pdev)
 
 	/* Claim the chip enable nShutdown gpio from the system */
 	kim_gdata->nshutdown = pdata->nshutdown_gpio;
+	kim_gdata->shutdown_val = pdata->shutdown_val;
 	err = gpio_request(kim_gdata->nshutdown, "kim");
 	if (unlikely(err)) {
 		pr_err(" gpio %d request failed ", kim_gdata->nshutdown);
@@ -817,7 +821,8 @@ static int kim_probe(struct platform_device *pdev)
 	}
 
 	/* Configure nShutdown GPIO as output=0 */
-	err = gpio_direction_output(kim_gdata->nshutdown, GPIO_LOW);
+	err = gpio_direction_output(kim_gdata->nshutdown,
+				    kim_gdata->shutdown_val);
 	if (unlikely(err)) {
 		pr_err(" unable to configure gpio %d", kim_gdata->nshutdown);
 		return err;
diff --git a/include/linux/ti_wilink_st.h b/include/linux/ti_wilink_st.h
index c78dcfe..7154194 100644
--- a/include/linux/ti_wilink_st.h
+++ b/include/linux/ti_wilink_st.h
@@ -212,9 +212,6 @@ void gps_chrdrv_stub_init(void);
 #define MAKEWORD(a, b)  ((unsigned short)(((unsigned char)(a)) \
 	| ((unsigned short)((unsigned char)(b))) << 8))
 
-#define GPIO_HIGH 1
-#define GPIO_LOW  0
-
 /* the Power-On-Reset logic, requires to attempt
  * to download firmware onto chip more than once
  * since the self-test for chip takes a while
@@ -263,6 +260,7 @@ struct kim_data_s {
 	char resp_buffer[30];
 	const struct firmware *fw_entry;
 	unsigned nshutdown;
+	unsigned shutdown_val;
 	unsigned long rx_state;
 	unsigned long rx_count;
 	struct sk_buff *rx_skb;
@@ -418,6 +416,7 @@ struct gps_event_hdr {
  * struct ti_st_plat_data - platform data shared between ST driver and
  *	platform specific board file which adds the ST device.
  * @nshutdown_gpio: Host's GPIO line to which chip's BT_EN is connected.
+ * @shutdown_val: shutdown state (0/1) of gpio
  * @dev_name: The UART/TTY name to which chip is interfaced. (eg: /dev/ttyS1)
  * @flow_cntrl: Should always be 1, since UART's CTS/RTS is used for PM
  *	purposes.
@@ -438,6 +437,7 @@ struct gps_event_hdr {
  */
 struct ti_st_plat_data {
 	u32 nshutdown_gpio;
+	int shutdown_val;
 	unsigned char dev_name[UART_DEV_NAME_LEN]; /* uart name */
 	u32 flow_cntrl; /* flow control flag */
 	u32 baud_rate;
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/4] misc: ti-st: use of_get_named_gpio_flags for nshutdown

[Dan Carpenter]

It's not obvious to me what the point of this patch is.  Why are we
making this change?  There is no changelog.

> @@ -482,9 +483,9 @@ long st_kim_start(void *kim_data)
>  			pdata->chip_enable(kim_gdata);
>  
>  		/* Configure BT nShutdown to HIGH state */

These comments are misleading now.

> -		gpio_set_value(kim_gdata->nshutdown, GPIO_LOW);
> +		gpio_set_value(kim_gdata->nshutdown, kim_gdata->shutdown_val);
>  		mdelay(5);	/* FIXME: a proper toggle */
> -		gpio_set_value(kim_gdata->nshutdown, GPIO_HIGH);
> +		gpio_set_value(kim_gdata->nshutdown, !kim_gdata->shutdown_val);
>  		mdelay(100);
>  		/* re-initialize the completion */
>  		reinit_completion(&kim_gdata->ldisc_installed);
> @@ -566,11 +567,11 @@ long st_kim_stop(void *kim_data)
>  	}
>  
>  	/* By default configure BT nShutdown to LOW state */

This as well.

> -	gpio_set_value(kim_gdata->nshutdown, GPIO_LOW);
> +	gpio_set_value(kim_gdata->nshutdown, kim_gdata->shutdown_val);
>  	mdelay(1);
> -	gpio_set_value(kim_gdata->nshutdown, GPIO_HIGH);
> +	gpio_set_value(kim_gdata->nshutdown, !kim_gdata->shutdown_val);
>  	mdelay(1);
> -	gpio_set_value(kim_gdata->nshutdown, GPIO_LOW);
> +	gpio_set_value(kim_gdata->nshutdown, kim_gdata->shutdown_val);
>  
>  	/* platform specific disable */
>  	if (pdata->chip_disable)
> @@ -749,6 +750,7 @@ static struct ti_st_plat_data *get_platform_data(struct device *dev)
> @@ -817,7 +821,8 @@ static int kim_probe(struct platform_device *pdev)
>  	}
>  
>  	/* Configure nShutdown GPIO as output=0 */

This comment.

> -	err = gpio_direction_output(kim_gdata->nshutdown, GPIO_LOW);
> +	err = gpio_direction_output(kim_gdata->nshutdown,
> +				    kim_gdata->shutdown_val);
>  	if (unlikely(err)) {
>  		pr_err(" unable to configure gpio %d", kim_gdata->nshutdown);
>  		return err;

regards,
dan carpenter

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/4] misc: ti-st: use of_get_named_gpio_flags for nshutdown

[Eric Nelson]

Thanks Dan,

On 04/27/2015 01:49 AM, Dan Carpenter wrote:
> 
> It's not obvious to me what the point of this patch is.  Why are we
> making this change?  There is no changelog.
> 
>> @@ -482,9 +483,9 @@ long st_kim_start(void *kim_data)
>>  			pdata->chip_enable(kim_gdata);
>>  
>>  		/* Configure BT nShutdown to HIGH state */
> 
> These comments are misleading now.
> 

Will fix in V2.

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/4] misc: ti-st: don't use NULL pdata

[Eric Nelson]

Signed-off-by: Eric Nelson <eric.nelson-Q5RJGjKts06CY9SHAMCTRUEOCMrvLtNR@public.gmane.org>
---
 drivers/misc/ti-st/st_kim.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/misc/ti-st/st_kim.c b/drivers/misc/ti-st/st_kim.c
index c5a0e15..4b44345 100644
--- a/drivers/misc/ti-st/st_kim.c
+++ b/drivers/misc/ti-st/st_kim.c
@@ -755,8 +755,10 @@ static struct ti_st_plat_data *get_platform_data(struct device *dev)
 
 	dt_pdata = kzalloc(sizeof(*dt_pdata), GFP_KERNEL);
 
-	if (!dt_pdata)
+	if (!dt_pdata) {
 		pr_err("Can't allocate device_tree platform data\n");
+		return 0;
+	}
 
 	dt_property = of_get_property(np, "dev_name", &len);
 	if (dt_property)
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/4] misc: ti-st: don't use NULL pdata

[Dan Carpenter]

On Sat, Apr 25, 2015 at 03:15:17PM -0700, Eric Nelson wrote:
> Signed-off-by: Eric Nelson <eric.nelson-Q5RJGjKts06CY9SHAMCTRUEOCMrvLtNR@public.gmane.org>
> ---
>  drivers/misc/ti-st/st_kim.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/misc/ti-st/st_kim.c b/drivers/misc/ti-st/st_kim.c
> index c5a0e15..4b44345 100644
> --- a/drivers/misc/ti-st/st_kim.c
> +++ b/drivers/misc/ti-st/st_kim.c
> @@ -755,8 +755,10 @@ static struct ti_st_plat_data *get_platform_data(struct device *dev)
>  
>  	dt_pdata = kzalloc(sizeof(*dt_pdata), GFP_KERNEL);
>  
> -	if (!dt_pdata)
> +	if (!dt_pdata) {
>  		pr_err("Can't allocate device_tree platform data\n");
> +		return 0;

		return NULL;

Zero means success.  You can delete the error message if you want.
That's just a waste of memory.  1) It's never going to be printed.
2)  If this particular allocation failed then we would be able to tell
exactly what failed from the stack dump and from the message in the
caller function.

> +	}

regards,
dan carpenter

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/4] misc: ti-st: don't use NULL pdata

[Eric Nelson]

Hi Dan,

On 04/27/2015 01:52 AM, Dan Carpenter wrote:
> On Sat, Apr 25, 2015 at 03:15:17PM -0700, Eric Nelson wrote:
>> Signed-off-by: Eric Nelson <eric.nelson-Q5RJGjKts06CY9SHAMCTRUEOCMrvLtNR@public.gmane.org>
>> ---
>>  drivers/misc/ti-st/st_kim.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/misc/ti-st/st_kim.c b/drivers/misc/ti-st/st_kim.c
>> index c5a0e15..4b44345 100644
>> --- a/drivers/misc/ti-st/st_kim.c
>> +++ b/drivers/misc/ti-st/st_kim.c
>> @@ -755,8 +755,10 @@ static struct ti_st_plat_data *get_platform_data(struct device *dev)
>>  
>>  	dt_pdata = kzalloc(sizeof(*dt_pdata), GFP_KERNEL);
>>  
>> -	if (!dt_pdata)
>> +	if (!dt_pdata) {
>>  		pr_err("Can't allocate device_tree platform data\n");
>> +		return 0;
> 
> 		return NULL;
> 
> Zero means success.  You can delete the error message if you want.
> That's just a waste of memory.  1) It's never going to be printed.
> 2)  If this particular allocation failed then we would be able to tell
> exactly what failed from the stack dump and from the message in the
> caller function.
> 

Thanks for the review. V2 forthcoming.

Regards,


Eric

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V2 1/4] misc: ti-st: use GPIO_LOW for initial value of nshutdown

[Eric Nelson]

All other uses of the nshutdown gpio use macros GPIO_LOW and
GPIO_HIGH to allow changing the pin polarity.

Do this in gpio_direction_output as well.

Signed-off-by: Eric Nelson <eric.nelson-Q5RJGjKts06CY9SHAMCTRUEOCMrvLtNR@public.gmane.org>
---
No changes in V2
 drivers/misc/ti-st/st_kim.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/misc/ti-st/st_kim.c b/drivers/misc/ti-st/st_kim.c
index 18e7a03..aaa17b0 100644
--- a/drivers/misc/ti-st/st_kim.c
+++ b/drivers/misc/ti-st/st_kim.c
@@ -817,7 +817,7 @@ static int kim_probe(struct platform_device *pdev)
 	}
 
 	/* Configure nShutdown GPIO as output=0 */
-	err = gpio_direction_output(kim_gdata->nshutdown, 0);
+	err = gpio_direction_output(kim_gdata->nshutdown, GPIO_LOW);
 	if (unlikely(err)) {
 		pr_err(" unable to configure gpio %d", kim_gdata->nshutdown);
 		return err;
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V2 2/4] misc: ti-st: use of_get_named_gpio_flags for nshutdown

[Eric Nelson]

Use of_get_named_gpio_flags to retrieve the "nshutdown" gpio connected
to the BT_EN pin of the device when retrieving platform data from device
tree.

This allows the polarity to be specified using GPIO_ACTIVE_HIGH/LOW 
in device tree.

Signed-off-by: Eric Nelson <eric.nelson-Q5RJGjKts06CY9SHAMCTRUEOCMrvLtNR@public.gmane.org>
---
V2 changes comments to use "assert" and "de-assert" to refer to
the state of the nshutdown gpio instead of low/high/0/1.

 drivers/misc/ti-st/st_kim.c  | 34 +++++++++++++++++++++-------------
 include/linux/ti_wilink_st.h |  6 +++---
 2 files changed, 24 insertions(+), 16 deletions(-)

diff --git a/drivers/misc/ti-st/st_kim.c b/drivers/misc/ti-st/st_kim.c
index aaa17b0..54d5f50 100644
--- a/drivers/misc/ti-st/st_kim.c
+++ b/drivers/misc/ti-st/st_kim.c
@@ -32,6 +32,7 @@
 #include <linux/sched.h>
 #include <linux/sysfs.h>
 #include <linux/tty.h>
+#include <linux/of_gpio.h>
 
 #include <linux/skbuff.h>
 #include <linux/ti_wilink_st.h>
@@ -481,10 +482,13 @@ 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);
+		/*
+		 * De-assert nShutdown (enable bluetooth),
+		 * but force a transition
+		 */
+		gpio_set_value(kim_gdata->nshutdown, kim_gdata->shutdown_val);
 		mdelay(5);	/* FIXME: a proper toggle */
-		gpio_set_value(kim_gdata->nshutdown, GPIO_HIGH);
+		gpio_set_value(kim_gdata->nshutdown, !kim_gdata->shutdown_val);
 		mdelay(100);
 		/* re-initialize the completion */
 		reinit_completion(&kim_gdata->ldisc_installed);
@@ -527,7 +531,7 @@ 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) reset BT_EN - pull down nshutdown at the end.
+ *	(c) reset BT_EN - assert nshutdown at the end.
  *	(d) invoke platform's chip disabling routine.
  */
 long st_kim_stop(void *kim_data)
@@ -565,12 +569,12 @@ 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);
+	/* Assert nShutdown (chip disabled), but force transitions */
+	gpio_set_value(kim_gdata->nshutdown, kim_gdata->shutdown_val);
 	mdelay(1);
-	gpio_set_value(kim_gdata->nshutdown, GPIO_HIGH);
+	gpio_set_value(kim_gdata->nshutdown, !kim_gdata->shutdown_val);
 	mdelay(1);
-	gpio_set_value(kim_gdata->nshutdown, GPIO_LOW);
+	gpio_set_value(kim_gdata->nshutdown, kim_gdata->shutdown_val);
 
 	/* platform specific disable */
 	if (pdata->chip_disable)
@@ -749,6 +753,7 @@ static struct ti_st_plat_data *get_platform_data(struct device *dev)
 {
 	struct device_node *np = dev->of_node;
 	const u32 *dt_property;
+	enum of_gpio_flags flags;
 	int len;
 
 	dt_pdata = kzalloc(sizeof(*dt_pdata), GFP_KERNEL);
@@ -759,8 +764,9 @@ static struct ti_st_plat_data *get_platform_data(struct device *dev)
 	dt_property = of_get_property(np, "dev_name", &len);
 	if (dt_property)
 		memcpy(&dt_pdata->dev_name, dt_property, len);
-	of_property_read_u32(np, "nshutdown_gpio",
-			     &dt_pdata->nshutdown_gpio);
+	dt_pdata->nshutdown_gpio = of_get_named_gpio_flags
+					(np, "nshutdown_gpio", 0, &flags);
+	dt_pdata->shutdown_val = !(flags & OF_GPIO_ACTIVE_LOW);
 	of_property_read_u32(np, "flow_cntrl", &dt_pdata->flow_cntrl);
 	of_property_read_u32(np, "baud_rate", &dt_pdata->baud_rate);
 
@@ -808,16 +814,18 @@ 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 */
+	/* Claim the nShutdown GPIO */
 	kim_gdata->nshutdown = pdata->nshutdown_gpio;
+	kim_gdata->shutdown_val = pdata->shutdown_val;
 	err = gpio_request(kim_gdata->nshutdown, "kim");
 	if (unlikely(err)) {
 		pr_err(" gpio %d request failed ", kim_gdata->nshutdown);
 		return err;
 	}
 
-	/* Configure nShutdown GPIO as output=0 */
-	err = gpio_direction_output(kim_gdata->nshutdown, GPIO_LOW);
+	/* Configure nShutdown GPIO as output and assert */
+	err = gpio_direction_output(kim_gdata->nshutdown,
+				    kim_gdata->shutdown_val);
 	if (unlikely(err)) {
 		pr_err(" unable to configure gpio %d", kim_gdata->nshutdown);
 		return err;
diff --git a/include/linux/ti_wilink_st.h b/include/linux/ti_wilink_st.h
index c78dcfe..7154194 100644
--- a/include/linux/ti_wilink_st.h
+++ b/include/linux/ti_wilink_st.h
@@ -212,9 +212,6 @@ void gps_chrdrv_stub_init(void);
 #define MAKEWORD(a, b)  ((unsigned short)(((unsigned char)(a)) \
 	| ((unsigned short)((unsigned char)(b))) << 8))
 
-#define GPIO_HIGH 1
-#define GPIO_LOW  0
-
 /* the Power-On-Reset logic, requires to attempt
  * to download firmware onto chip more than once
  * since the self-test for chip takes a while
@@ -263,6 +260,7 @@ struct kim_data_s {
 	char resp_buffer[30];
 	const struct firmware *fw_entry;
 	unsigned nshutdown;
+	unsigned shutdown_val;
 	unsigned long rx_state;
 	unsigned long rx_count;
 	struct sk_buff *rx_skb;
@@ -418,6 +416,7 @@ struct gps_event_hdr {
  * struct ti_st_plat_data - platform data shared between ST driver and
  *	platform specific board file which adds the ST device.
  * @nshutdown_gpio: Host's GPIO line to which chip's BT_EN is connected.
+ * @shutdown_val: shutdown state (0/1) of gpio
  * @dev_name: The UART/TTY name to which chip is interfaced. (eg: /dev/ttyS1)
  * @flow_cntrl: Should always be 1, since UART's CTS/RTS is used for PM
  *	purposes.
@@ -438,6 +437,7 @@ struct gps_event_hdr {
  */
 struct ti_st_plat_data {
 	u32 nshutdown_gpio;
+	int shutdown_val;
 	unsigned char dev_name[UART_DEV_NAME_LEN]; /* uart name */
 	u32 flow_cntrl; /* flow control flag */
 	u32 baud_rate;
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 2/4] misc: ti-st: use of_get_named_gpio_flags for nshutdown

[Peter Hurley]

On 04/27/2015 03:27 PM, Eric Nelson wrote:
> Use of_get_named_gpio_flags to retrieve the "nshutdown" gpio connected
> to the BT_EN pin of the device when retrieving platform data from device
> tree.

This breaks all existing DTs wrt the 'nshutdown_gpio' key.
I suggest using a different, optional key in the absence of 'nshutdown_gpio'.

Regards,
Peter Hurley

> This allows the polarity to be specified using GPIO_ACTIVE_HIGH/LOW 
> in device tree.
>
> Signed-off-by: Eric Nelson <eric.nelson-Q5RJGjKts06CY9SHAMCTRUEOCMrvLtNR@public.gmane.org>
> ---
> V2 changes comments to use "assert" and "de-assert" to refer to
> the state of the nshutdown gpio instead of low/high/0/1.
> 
>  drivers/misc/ti-st/st_kim.c  | 34 +++++++++++++++++++++-------------
>  include/linux/ti_wilink_st.h |  6 +++---
>  2 files changed, 24 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/misc/ti-st/st_kim.c b/drivers/misc/ti-st/st_kim.c
> index aaa17b0..54d5f50 100644
> --- a/drivers/misc/ti-st/st_kim.c
> +++ b/drivers/misc/ti-st/st_kim.c
> @@ -32,6 +32,7 @@
>  #include <linux/sched.h>
>  #include <linux/sysfs.h>
>  #include <linux/tty.h>
> +#include <linux/of_gpio.h>
>  
>  #include <linux/skbuff.h>
>  #include <linux/ti_wilink_st.h>
> @@ -481,10 +482,13 @@ 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);
> +		/*
> +		 * De-assert nShutdown (enable bluetooth),
> +		 * but force a transition
> +		 */
> +		gpio_set_value(kim_gdata->nshutdown, kim_gdata->shutdown_val);
>  		mdelay(5);	/* FIXME: a proper toggle */
> -		gpio_set_value(kim_gdata->nshutdown, GPIO_HIGH);
> +		gpio_set_value(kim_gdata->nshutdown, !kim_gdata->shutdown_val);
>  		mdelay(100);
>  		/* re-initialize the completion */
>  		reinit_completion(&kim_gdata->ldisc_installed);
> @@ -527,7 +531,7 @@ 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) reset BT_EN - pull down nshutdown at the end.
> + *	(c) reset BT_EN - assert nshutdown at the end.
>   *	(d) invoke platform's chip disabling routine.
>   */
>  long st_kim_stop(void *kim_data)
> @@ -565,12 +569,12 @@ 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);
> +	/* Assert nShutdown (chip disabled), but force transitions */
> +	gpio_set_value(kim_gdata->nshutdown, kim_gdata->shutdown_val);
>  	mdelay(1);
> -	gpio_set_value(kim_gdata->nshutdown, GPIO_HIGH);
> +	gpio_set_value(kim_gdata->nshutdown, !kim_gdata->shutdown_val);
>  	mdelay(1);
> -	gpio_set_value(kim_gdata->nshutdown, GPIO_LOW);
> +	gpio_set_value(kim_gdata->nshutdown, kim_gdata->shutdown_val);
>  
>  	/* platform specific disable */
>  	if (pdata->chip_disable)
> @@ -749,6 +753,7 @@ static struct ti_st_plat_data *get_platform_data(struct device *dev)
>  {
>  	struct device_node *np = dev->of_node;
>  	const u32 *dt_property;
> +	enum of_gpio_flags flags;
>  	int len;
>  
>  	dt_pdata = kzalloc(sizeof(*dt_pdata), GFP_KERNEL);
> @@ -759,8 +764,9 @@ static struct ti_st_plat_data *get_platform_data(struct device *dev)
>  	dt_property = of_get_property(np, "dev_name", &len);
>  	if (dt_property)
>  		memcpy(&dt_pdata->dev_name, dt_property, len);
> -	of_property_read_u32(np, "nshutdown_gpio",
> -			     &dt_pdata->nshutdown_gpio);
> +	dt_pdata->nshutdown_gpio = of_get_named_gpio_flags
> +					(np, "nshutdown_gpio", 0, &flags);
> +	dt_pdata->shutdown_val = !(flags & OF_GPIO_ACTIVE_LOW);
>  	of_property_read_u32(np, "flow_cntrl", &dt_pdata->flow_cntrl);
>  	of_property_read_u32(np, "baud_rate", &dt_pdata->baud_rate);
>  
> @@ -808,16 +814,18 @@ 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 */
> +	/* Claim the nShutdown GPIO */
>  	kim_gdata->nshutdown = pdata->nshutdown_gpio;
> +	kim_gdata->shutdown_val = pdata->shutdown_val;
>  	err = gpio_request(kim_gdata->nshutdown, "kim");
>  	if (unlikely(err)) {
>  		pr_err(" gpio %d request failed ", kim_gdata->nshutdown);
>  		return err;
>  	}
>  
> -	/* Configure nShutdown GPIO as output=0 */
> -	err = gpio_direction_output(kim_gdata->nshutdown, GPIO_LOW);
> +	/* Configure nShutdown GPIO as output and assert */
> +	err = gpio_direction_output(kim_gdata->nshutdown,
> +				    kim_gdata->shutdown_val);
>  	if (unlikely(err)) {
>  		pr_err(" unable to configure gpio %d", kim_gdata->nshutdown);
>  		return err;
> diff --git a/include/linux/ti_wilink_st.h b/include/linux/ti_wilink_st.h
> index c78dcfe..7154194 100644
> --- a/include/linux/ti_wilink_st.h
> +++ b/include/linux/ti_wilink_st.h
> @@ -212,9 +212,6 @@ void gps_chrdrv_stub_init(void);
>  #define MAKEWORD(a, b)  ((unsigned short)(((unsigned char)(a)) \
>  	| ((unsigned short)((unsigned char)(b))) << 8))
>  
> -#define GPIO_HIGH 1
> -#define GPIO_LOW  0
> -
>  /* the Power-On-Reset logic, requires to attempt
>   * to download firmware onto chip more than once
>   * since the self-test for chip takes a while
> @@ -263,6 +260,7 @@ struct kim_data_s {
>  	char resp_buffer[30];
>  	const struct firmware *fw_entry;
>  	unsigned nshutdown;
> +	unsigned shutdown_val;
>  	unsigned long rx_state;
>  	unsigned long rx_count;
>  	struct sk_buff *rx_skb;
> @@ -418,6 +416,7 @@ struct gps_event_hdr {
>   * struct ti_st_plat_data - platform data shared between ST driver and
>   *	platform specific board file which adds the ST device.
>   * @nshutdown_gpio: Host's GPIO line to which chip's BT_EN is connected.
> + * @shutdown_val: shutdown state (0/1) of gpio
>   * @dev_name: The UART/TTY name to which chip is interfaced. (eg: /dev/ttyS1)
>   * @flow_cntrl: Should always be 1, since UART's CTS/RTS is used for PM
>   *	purposes.
> @@ -438,6 +437,7 @@ struct gps_event_hdr {
>   */
>  struct ti_st_plat_data {
>  	u32 nshutdown_gpio;
> +	int shutdown_val;
>  	unsigned char dev_name[UART_DEV_NAME_LEN]; /* uart name */
>  	u32 flow_cntrl; /* flow control flag */
>  	u32 baud_rate;
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 2/4] misc: ti-st: use of_get_named_gpio_flags for nshutdown

[Eric Nelson]

Hi Peter,

On 04/27/2015 01:11 PM, Peter Hurley wrote:
> On 04/27/2015 03:27 PM, Eric Nelson wrote:
>> Use of_get_named_gpio_flags to retrieve the "nshutdown" gpio connected
>> to the BT_EN pin of the device when retrieving platform data from device
>> tree.
> 
> This breaks all existing DTs wrt the 'nshutdown_gpio' key.
> I suggest using a different, optional key in the absence of 'nshutdown_gpio'.
> 

You mean all zero of them up-stream ;)?

I did forward a patch set to the maintainers of the ti-linux-kernel on
git.ti.com:

	https://git.ti.com/ti-linux-kernel/ti-linux-kernel

Regards,


Eric
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 2/4] misc: ti-st: use of_get_named_gpio_flags for nshutdown

[Peter Hurley]

Hi Eric,

On 04/27/2015 04:18 PM, Eric Nelson wrote:
> Hi Peter,
> 
> On 04/27/2015 01:11 PM, Peter Hurley wrote:
>> On 04/27/2015 03:27 PM, Eric Nelson wrote:
>>> Use of_get_named_gpio_flags to retrieve the "nshutdown" gpio connected
>>> to the BT_EN pin of the device when retrieving platform data from device
>>> tree.
>>
>> This breaks all existing DTs wrt the 'nshutdown_gpio' key.
>> I suggest using a different, optional key in the absence of 'nshutdown_gpio'.
>>
> 
> You mean all zero of them up-stream ;)?

"nshutdown_gpio" DT key became ABI as of 4.0


> I did forward a patch set to the maintainers of the ti-linux-kernel on
> git.ti.com:
> 
> 	https://git.ti.com/ti-linux-kernel/ti-linux-kernel
> 
> Regards,
> 
> 
> Eric
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V2 3/4] misc: ti-st: don't use NULL pdata

[Eric Nelson]

Signed-off-by: Eric Nelson <eric.nelson-Q5RJGjKts06CY9SHAMCTRUEOCMrvLtNR@public.gmane.org>
---
V2 returns NULL instead of 0 to be more explicit.

 drivers/misc/ti-st/st_kim.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/misc/ti-st/st_kim.c b/drivers/misc/ti-st/st_kim.c
index 54d5f50..97c94f7 100644
--- a/drivers/misc/ti-st/st_kim.c
+++ b/drivers/misc/ti-st/st_kim.c
@@ -759,7 +759,7 @@ static struct ti_st_plat_data *get_platform_data(struct device *dev)
 	dt_pdata = kzalloc(sizeof(*dt_pdata), GFP_KERNEL);
 
 	if (!dt_pdata)
-		pr_err("Can't allocate device_tree platform data\n");
+		return NULL;
 
 	dt_property = of_get_property(np, "dev_name", &len);
 	if (dt_property)
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH V2 4/4] Documentation: DT: misc: Add ti-st bindings

[Eric Nelson]

Signed-off-by: Eric Nelson <eric.nelson-Q5RJGjKts06CY9SHAMCTRUEOCMrvLtNR@public.gmane.org>
---
No changes in V2

 Documentation/devicetree/bindings/misc/ti-st.txt | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/misc/ti-st.txt

diff --git a/Documentation/devicetree/bindings/misc/ti-st.txt b/Documentation/devicetree/bindings/misc/ti-st.txt
new file mode 100644
index 0000000..a1948d9
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/ti-st.txt
@@ -0,0 +1,17 @@
+TI Shared-transport driver (a.k.a. kim)
+
+Required properties:
+- compatible : kim
+- nshutdown_gpio : gpio name and polarity
+- dev_name : uart used for communication
+- flow_cntrl : 1 if RTS/CTS hardware flow control is available
+- baud_rate : baud rate for the UART
+
+Example (from am355x-evm):
+	kim {
+		compatible = "kim";
+		nshutdown_gpio = <&gpio3 21 GPIO_ACTIVE_LOW>;
+		dev_name = "/dev/ttyS1";
+		flow_cntrl = <1>;
+		baud_rate = <3000000>;
+	};
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/4] Documentation: DT: misc: Add ti-st bindings

[Eric Nelson]

Signed-off-by: Eric Nelson <eric.nelson-Q5RJGjKts06CY9SHAMCTRUEOCMrvLtNR@public.gmane.org>
---
 Documentation/devicetree/bindings/misc/ti-st.txt | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/misc/ti-st.txt

diff --git a/Documentation/devicetree/bindings/misc/ti-st.txt b/Documentation/devicetree/bindings/misc/ti-st.txt
new file mode 100644
index 0000000..a1948d9
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/ti-st.txt
@@ -0,0 +1,17 @@
+TI Shared-transport driver (a.k.a. kim)
+
+Required properties:
+- compatible : kim
+- nshutdown_gpio : gpio name and polarity
+- dev_name : uart used for communication
+- flow_cntrl : 1 if RTS/CTS hardware flow control is available
+- baud_rate : baud rate for the UART
+
+Example (from am355x-evm):
+	kim {
+		compatible = "kim";
+		nshutdown_gpio = <&gpio3 21 GPIO_ACTIVE_LOW>;
+		dev_name = "/dev/ttyS1";
+		flow_cntrl = <1>;
+		baud_rate = <3000000>;
+	};
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/4] misc: ti-st: use GPIO_LOW for initial value of nshutdown

[Lee Jones]

On Sat, 25 Apr 2015, Eric Nelson wrote:

> All other uses of the nshutdown gpio use macros GPIO_LOW and
> GPIO_HIGH to allow changing the pin polarity.
> 
> Do this in gpio_direction_output as well.
> 
> Signed-off-by: Eric Nelson <eric.nelson-Q5RJGjKts06CY9SHAMCTRUEOCMrvLtNR@public.gmane.org>
> ---
>  drivers/misc/ti-st/st_kim.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Where your cover-letter?

Why did you even sent me this set?

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/4] misc: ti-st: use GPIO_LOW for initial value of nshutdown

[Eric Nelson]

Hi Lee,

On 04/28/2015 04:17 AM, Lee Jones wrote:
> On Sat, 25 Apr 2015, Eric Nelson wrote:
> 
>> All other uses of the nshutdown gpio use macros GPIO_LOW and
>> GPIO_HIGH to allow changing the pin polarity.
>>
>> Do this in gpio_direction_output as well.
>>
>> Signed-off-by: Eric Nelson <eric.nelson-Q5RJGjKts06CY9SHAMCTRUEOCMrvLtNR@public.gmane.org>
>> ---
>>  drivers/misc/ti-st/st_kim.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Where your cover-letter?
> 

/dev/null (I didn't create one).

> Why did you even sent me this set?
> 
get_maintainer.pl told me to...

	Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
(commit_signer:2/5=40%,authored:2/5=40%,added_lines:2/12=17%,removed_lines:2/11=18%)

Regards,


Eric
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/4] misc: ti-st: use GPIO_LOW for initial value of nshutdown

[Lee Jones]

On Tue, 28 Apr 2015, Eric Nelson wrote:
> On 04/28/2015 04:17 AM, Lee Jones wrote:
> > On Sat, 25 Apr 2015, Eric Nelson wrote:
> > 
> >> All other uses of the nshutdown gpio use macros GPIO_LOW and
> >> GPIO_HIGH to allow changing the pin polarity.
> >>
> >> Do this in gpio_direction_output as well.
> >>
> >> Signed-off-by: Eric Nelson <eric.nelson-Q5RJGjKts06CY9SHAMCTRUEOCMrvLtNR@public.gmane.org>
> >> ---
> >>  drivers/misc/ti-st/st_kim.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > Where your cover-letter?
> > 
> 
> /dev/null (I didn't create one).

It's good practice to send cover letters attached to patch-sets.  It
gives a basic overview of what the set is trying to achieve, a nice
succinct centralised change-log and a diff-stat for the entire set.
All very handy information for a Maintainer/reviewer and is likely to
speed-up acceptance times.

> > Why did you even sent me this set?
> > 
> get_maintainer.pl told me to...
> 
> 	Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> (commit_signer:2/5=40%,authored:2/5=40%,added_lines:2/12=17%,removed_lines:2/11=18%)

I'd Cc "commit signers" with extreme caution to be honest.  As a rule
I never add them, unless they are the author of, or have made vast
changes to the file you're modifying.

I guess get_maintainer.pl is relating to change e4ebe5f where I was
fixing some Coverity reports.  Bottom line is, I shouldn't be in
receipt of this patch-set.  It's just clogging up my already very
clogged-up inbox.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html