[PATCH 1/3] ti-st: st_kim: use ERR_PTR(-ENOMEM) instead of NULL

[PATCH 1/3] ti-st: st_kim: use ERR_PTR(-ENOMEM) instead of NULL

[Jürg Billeter]

This allows return of other error codes.

Signed-off-by: Jürg Billeter <j@bitron.ch>
---
 drivers/misc/ti-st/st_kim.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/misc/ti-st/st_kim.c b/drivers/misc/ti-st/st_kim.c
index 5027b8f..af71584 100644
--- a/drivers/misc/ti-st/st_kim.c
+++ b/drivers/misc/ti-st/st_kim.c
@@ -752,8 +752,11 @@ static struct ti_st_plat_data *get_platform_data(struct device *dev)
 	int len;
 
 	dt_pdata = kzalloc(sizeof(*dt_pdata), GFP_KERNEL);
-	if (!dt_pdata)
-		return NULL;
+
+	if (!dt_pdata) {
+		pr_err("Can't allocate device_tree platform data\n");
+		return ERR_PTR(-ENOMEM);
+	}
 
 	dt_property = of_get_property(np, "dev_name", &len);
 	if (dt_property)
@@ -773,10 +776,13 @@ static int kim_probe(struct platform_device *pdev)
 	struct ti_st_plat_data	*pdata;
 	int err;
 
-	if (pdev->dev.of_node)
+	if (pdev->dev.of_node) {
 		pdata = get_platform_data(&pdev->dev);
-	else
+		if (IS_ERR(pdata))
+			return PTR_ERR(pdata);
+	} else {
 		pdata = pdev->dev.platform_data;
+	}
 
 	if (pdata == NULL) {
 		dev_err(&pdev->dev, "Platform Data is missing\n");
-- 
2.4.3

[PATCH 2/3] ti-st: st_kim: fix nshutdown_gpio in get_platform_data

[Jürg Billeter]

Use of_get_named_gpio instead of of_property_read_u32.

Signed-off-by: Jürg Billeter <j@bitron.ch>
---
 drivers/misc/ti-st/st_kim.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/misc/ti-st/st_kim.c b/drivers/misc/ti-st/st_kim.c
index af71584..8df8faa 100644
--- a/drivers/misc/ti-st/st_kim.c
+++ b/drivers/misc/ti-st/st_kim.c
@@ -38,6 +38,7 @@
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
+#include <linux/of_gpio.h>
 
 #define MAX_ST_DEVICES	3	/* Imagine 1 on each UART for now */
 static struct platform_device *st_kim_devices[MAX_ST_DEVICES];
@@ -749,7 +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;
-	int len;
+	int len, gpio;
 
 	dt_pdata = kzalloc(sizeof(*dt_pdata), GFP_KERNEL);
 
@@ -761,8 +762,14 @@ 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);
+
+	gpio = of_get_named_gpio(np, "nshutdown_gpio", 0);
+	if (gpio < 0) {
+		kfree(dt_pdata);
+		return ERR_PTR(gpio);
+	}
+	dt_pdata->nshutdown_gpio = gpio;
+
 	of_property_read_u32(np, "flow_cntrl", &dt_pdata->flow_cntrl);
 	of_property_read_u32(np, "baud_rate", &dt_pdata->baud_rate);
 
-- 
2.4.3

[PATCH 3/3] ti-st: st_kim: use gpio_set_value_cansleep to fix warning

[Jürg Billeter]

GPIO accessor functions may sleep.

Signed-off-by: Jürg Billeter <j@bitron.ch>
---
 drivers/misc/ti-st/st_kim.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/misc/ti-st/st_kim.c b/drivers/misc/ti-st/st_kim.c
index 8df8faa..0ab81d7 100644
--- a/drivers/misc/ti-st/st_kim.c
+++ b/drivers/misc/ti-st/st_kim.c
@@ -483,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_cansleep(kim_gdata->nshutdown, GPIO_LOW);
 		mdelay(5);	/* FIXME: a proper toggle */
-		gpio_set_value(kim_gdata->nshutdown, GPIO_HIGH);
+		gpio_set_value_cansleep(kim_gdata->nshutdown, GPIO_HIGH);
 		mdelay(100);
 		/* re-initialize the completion */
 		reinit_completion(&kim_gdata->ldisc_installed);
@@ -567,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_cansleep(kim_gdata->nshutdown, GPIO_LOW);
 	mdelay(1);
-	gpio_set_value(kim_gdata->nshutdown, GPIO_HIGH);
+	gpio_set_value_cansleep(kim_gdata->nshutdown, GPIO_HIGH);
 	mdelay(1);
-	gpio_set_value(kim_gdata->nshutdown, GPIO_LOW);
+	gpio_set_value_cansleep(kim_gdata->nshutdown, GPIO_LOW);
 
 	/* platform specific disable */
 	if (pdata->chip_disable)
-- 
2.4.3

Re: [PATCH 2/3] ti-st: st_kim: fix nshutdown_gpio in get_platform_data

[Peter Hurley]

On 06/24/2015 06:24 AM, Jürg Billeter wrote:
> Use of_get_named_gpio instead of of_property_read_u32.
> 
> Signed-off-by: Jürg Billeter <j@bitron.ch>
> ---
>  drivers/misc/ti-st/st_kim.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/misc/ti-st/st_kim.c b/drivers/misc/ti-st/st_kim.c
> index af71584..8df8faa 100644
> --- a/drivers/misc/ti-st/st_kim.c
> +++ b/drivers/misc/ti-st/st_kim.c
> @@ -38,6 +38,7 @@
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/of_device.h>
> +#include <linux/of_gpio.h>
>  
>  #define MAX_ST_DEVICES	3	/* Imagine 1 on each UART for now */
>  static struct platform_device *st_kim_devices[MAX_ST_DEVICES];
> @@ -749,7 +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;
> -	int len;
> +	int len, gpio;
>  
>  	dt_pdata = kzalloc(sizeof(*dt_pdata), GFP_KERNEL);
>  
> @@ -761,8 +762,14 @@ 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);
> +
> +	gpio = of_get_named_gpio(np, "nshutdown_gpio", 0);

NAK. This breaks existing dtbs, since the format is not the same.

Regards,
Peter Hurley


> +	if (gpio < 0) {
> +		kfree(dt_pdata);
> +		return ERR_PTR(gpio);
> +	}
> +	dt_pdata->nshutdown_gpio = gpio;
> +
>  	of_property_read_u32(np, "flow_cntrl", &dt_pdata->flow_cntrl);
>  	of_property_read_u32(np, "baud_rate", &dt_pdata->baud_rate);
>  
> 

Re: [PATCH 2/3] ti-st: st_kim: fix nshutdown_gpio in get_platform_data

[Jürg Billeter]

On Wed, 2015-06-24 at 11:31 -0400, Peter Hurley wrote:
> On 06/24/2015 06:24 AM, Jürg Billeter wrote:
> > @@ -761,8 +762,14 @@ 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);
> > +
> > +	gpio = of_get_named_gpio(np, "nshutdown_gpio", 0);
> 
> NAK. This breaks existing dtbs, since the format is not the same.

Isn't the existing code completely broken as there is no predictable
GPIO numbering in general? There is also no documentation or use of
that device tree property in the kernel tree, as far as I can tell.

Do you have a suggestion how to fix this without breaking existing
dtbs? Do we need to introduce a second property and support both in the
driver?

Regards,
Jürg

Re: [PATCH 2/3] ti-st: st_kim: fix nshutdown_gpio in get_platform_data

[Peter Hurley]

On 06/24/2015 11:39 AM, Jürg Billeter wrote:
> Do you have a suggestion how to fix this without breaking existing
> dtbs? Do we need to introduce a second property and support both in the
> driver?

Exactly.

Regards,
Peter Hurley