[[PATCH v2]] OMAP: omap4-panda: add WiLink shared transport power functions

[[PATCH v2]] OMAP: omap4-panda: add WiLink shared transport power functions

[Luciano Coelho]

The code to enable and disable the WiLink shared transport has been
removed from the TI-ST driver, so it must be implemented in the board
files instead.  Add the relevant operations to Panda's board file.

Additionally, add the UART2 muxing data, so it's properly configured.

Cc: stable <stable@vger.kernel.org> [3.7]
Signed-off-by: Luciano Coelho <coelho@ti.com>
---

In v2: use gpio_request_one() instead of gpio_request() and
gpio_direction_output(). (Thanks Fabio!)

 arch/arm/mach-omap2/board-omap4panda.c |   50 +++++++++++++++++++++++++++++---
 1 file changed, 46 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-omap2/board-omap4panda.c b/arch/arm/mach-omap2/board-omap4panda.c
index 5c8e9ce..f44fccf 100644
--- a/arch/arm/mach-omap2/board-omap4panda.c
+++ b/arch/arm/mach-omap2/board-omap4panda.c
@@ -51,18 +51,50 @@
 #define GPIO_HUB_NRESET		62
 #define GPIO_WIFI_PMENA		43
 #define GPIO_WIFI_IRQ		53
+#define GPIO_BT_EN		46
 
 /* wl127x BT, FM, GPS connectivity chip */
+static int plat_kim_chip_enable(struct kim_data_s *kim_data)
+{
+	gpio_set_value(GPIO_BT_EN, GPIO_LOW);
+	mdelay(5);
+	gpio_set_value(GPIO_BT_EN, GPIO_HIGH);
+	mdelay(100);
+
+	return 0;
+}
+
+static int plat_kim_chip_disable(struct kim_data_s *kim_data)
+{
+	gpio_set_value(GPIO_BT_EN, GPIO_LOW);
+	mdelay(1);
+	gpio_set_value(GPIO_BT_EN, GPIO_HIGH);
+	mdelay(1);
+	gpio_set_value(GPIO_BT_EN, GPIO_LOW);
+
+	return 0;
+}
+
 static struct ti_st_plat_data wilink_platform_data = {
-	.nshutdown_gpio	= 46,
 	.dev_name	= "/dev/ttyO1",
 	.flow_cntrl	= 1,
 	.baud_rate	= 3000000,
-	.chip_enable	= NULL,
-	.suspend	= NULL,
-	.resume		= NULL,
+	.chip_enable	= plat_kim_chip_enable,
+	.chip_disable	= plat_kim_chip_disable,
 };
 
+static int wilink_st_init(void)
+{
+	int status;
+
+	status = gpio_request_one(GPIO_BT_EN, GPIOF_OUT_INIT_LOW, "kim");
+	if (status)
+		pr_err("%s: failed to request gpio %d\n", __func__,
+		       GPIO_BT_EN);
+
+	return status;
+}
+
 static struct platform_device wl1271_device = {
 	.name	= "kim",
 	.id	= -1,
@@ -397,6 +429,12 @@ static struct omap_board_mux board_mux[] __initdata = {
 		  OMAP_PULL_ENA),
 	OMAP4_MUX(ABE_MCBSP1_FSX, OMAP_MUX_MODE0 | OMAP_PIN_INPUT),
 
+	/* UART2 - BT/FM/GPS shared transport */
+	OMAP4_MUX(UART2_CTS,	OMAP_PIN_INPUT	| OMAP_MUX_MODE0),
+	OMAP4_MUX(UART2_RTS,	OMAP_PIN_OUTPUT	| OMAP_MUX_MODE0),
+	OMAP4_MUX(UART2_RX,	OMAP_PIN_INPUT	| OMAP_MUX_MODE0),
+	OMAP4_MUX(UART2_TX,	OMAP_PIN_OUTPUT	| OMAP_MUX_MODE0),
+
 	{ .reg_offset = OMAP_MUX_TERMINATOR },
 };
 
@@ -433,6 +471,10 @@ static void __init omap4_panda_init(void)
 	if (ret)
 		pr_err("error setting wl12xx data: %d\n", ret);
 
+	ret = wilink_st_init();
+	if (ret)
+		pr_err("WiLink shared transport init failed: %d\n", ret);
+
 	omap4_panda_init_rev();
 	omap4_panda_i2c_init();
 	platform_add_devices(panda_devices, ARRAY_SIZE(panda_devices));
-- 
1.7.10.4

[[PATCH v2]] OMAP: omap4-panda: add WiLink shared transport power functions

[Peter Ujfalusi]

Hi Luca,

On 01/16/2013 10:45 PM, Luciano Coelho wrote:
> The code to enable and disable the WiLink shared transport has been
> removed from the TI-ST driver, so it must be implemented in the board
> files instead.  Add the relevant operations to Panda's board file.
> 
> Additionally, add the UART2 muxing data, so it's properly configured.
> 
> Cc: stable <stable@vger.kernel.org> [3.7]
> Signed-off-by: Luciano Coelho <coelho@ti.com>
> ---
> 
> In v2: use gpio_request_one() instead of gpio_request() and
> gpio_direction_output(). (Thanks Fabio!)
> 
>  arch/arm/mach-omap2/board-omap4panda.c |   50 +++++++++++++++++++++++++++++---
>  1 file changed, 46 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/board-omap4panda.c b/arch/arm/mach-omap2/board-omap4panda.c
> index 5c8e9ce..f44fccf 100644
> --- a/arch/arm/mach-omap2/board-omap4panda.c
> +++ b/arch/arm/mach-omap2/board-omap4panda.c
> @@ -51,18 +51,50 @@
>  #define GPIO_HUB_NRESET		62
>  #define GPIO_WIFI_PMENA		43
>  #define GPIO_WIFI_IRQ		53
> +#define GPIO_BT_EN		46
>  
>  /* wl127x BT, FM, GPS connectivity chip */
> +static int plat_kim_chip_enable(struct kim_data_s *kim_data)
> +{
> +	gpio_set_value(GPIO_BT_EN, GPIO_LOW);
> +	mdelay(5);
> +	gpio_set_value(GPIO_BT_EN, GPIO_HIGH);
> +	mdelay(100);
> +
> +	return 0;
> +}
> +
> +static int plat_kim_chip_disable(struct kim_data_s *kim_data)
> +{
> +	gpio_set_value(GPIO_BT_EN, GPIO_LOW);
> +	mdelay(1);
> +	gpio_set_value(GPIO_BT_EN, GPIO_HIGH);
> +	mdelay(1);
> +	gpio_set_value(GPIO_BT_EN, GPIO_LOW);
> +
> +	return 0;
> +}
> +
>  static struct ti_st_plat_data wilink_platform_data = {
> -	.nshutdown_gpio	= 46,
>  	.dev_name	= "/dev/ttyO1",
>  	.flow_cntrl	= 1,
>  	.baud_rate	= 3000000,
> -	.chip_enable	= NULL,
> -	.suspend	= NULL,
> -	.resume		= NULL,
> +	.chip_enable	= plat_kim_chip_enable,
> +	.chip_disable	= plat_kim_chip_disable,

I just wonder how this is going to work with DT... You are not going to have
the ability to use callback in this form.
I think the GPIO handling should be done in the driver itself rather than in
the board file.

>  };
>  
> +static int wilink_st_init(void)
> +{
> +	int status;
> +
> +	status = gpio_request_one(GPIO_BT_EN, GPIOF_OUT_INIT_LOW, "kim");
> +	if (status)
> +		pr_err("%s: failed to request gpio %d\n", __func__,
> +		       GPIO_BT_EN);
> +
> +	return status;
> +}
> +
>  static struct platform_device wl1271_device = {
>  	.name	= "kim",
>  	.id	= -1,
> @@ -397,6 +429,12 @@ static struct omap_board_mux board_mux[] __initdata = {
>  		  OMAP_PULL_ENA),
>  	OMAP4_MUX(ABE_MCBSP1_FSX, OMAP_MUX_MODE0 | OMAP_PIN_INPUT),
>  
> +	/* UART2 - BT/FM/GPS shared transport */
> +	OMAP4_MUX(UART2_CTS,	OMAP_PIN_INPUT	| OMAP_MUX_MODE0),
> +	OMAP4_MUX(UART2_RTS,	OMAP_PIN_OUTPUT	| OMAP_MUX_MODE0),
> +	OMAP4_MUX(UART2_RX,	OMAP_PIN_INPUT	| OMAP_MUX_MODE0),
> +	OMAP4_MUX(UART2_TX,	OMAP_PIN_OUTPUT	| OMAP_MUX_MODE0),
> +
>  	{ .reg_offset = OMAP_MUX_TERMINATOR },
>  };
>  
> @@ -433,6 +471,10 @@ static void __init omap4_panda_init(void)
>  	if (ret)
>  		pr_err("error setting wl12xx data: %d\n", ret);
>  
> +	ret = wilink_st_init();
> +	if (ret)
> +		pr_err("WiLink shared transport init failed: %d\n", ret);
> +
>  	omap4_panda_init_rev();
>  	omap4_panda_i2c_init();
>  	platform_add_devices(panda_devices, ARRAY_SIZE(panda_devices));
> 


-- 
P?ter

[[PATCH v2]] OMAP: omap4-panda: add WiLink shared transport power functions

[Felipe Balbi]

On Thu, Jan 17, 2013 at 10:30:15AM +0100, Peter Ujfalusi wrote:
> Hi Luca,
> 
> On 01/16/2013 10:45 PM, Luciano Coelho wrote:
> > The code to enable and disable the WiLink shared transport has been
> > removed from the TI-ST driver, so it must be implemented in the board
> > files instead.  Add the relevant operations to Panda's board file.
> > 
> > Additionally, add the UART2 muxing data, so it's properly configured.
> > 
> > Cc: stable <stable@vger.kernel.org> [3.7]
> > Signed-off-by: Luciano Coelho <coelho@ti.com>
> > ---
> > 
> > In v2: use gpio_request_one() instead of gpio_request() and
> > gpio_direction_output(). (Thanks Fabio!)
> > 
> >  arch/arm/mach-omap2/board-omap4panda.c |   50 +++++++++++++++++++++++++++++---
> >  1 file changed, 46 insertions(+), 4 deletions(-)
> > 
> > diff --git a/arch/arm/mach-omap2/board-omap4panda.c b/arch/arm/mach-omap2/board-omap4panda.c
> > index 5c8e9ce..f44fccf 100644
> > --- a/arch/arm/mach-omap2/board-omap4panda.c
> > +++ b/arch/arm/mach-omap2/board-omap4panda.c
> > @@ -51,18 +51,50 @@
> >  #define GPIO_HUB_NRESET		62
> >  #define GPIO_WIFI_PMENA		43
> >  #define GPIO_WIFI_IRQ		53
> > +#define GPIO_BT_EN		46
> >  
> >  /* wl127x BT, FM, GPS connectivity chip */
> > +static int plat_kim_chip_enable(struct kim_data_s *kim_data)
> > +{
> > +	gpio_set_value(GPIO_BT_EN, GPIO_LOW);
> > +	mdelay(5);
> > +	gpio_set_value(GPIO_BT_EN, GPIO_HIGH);
> > +	mdelay(100);
> > +
> > +	return 0;
> > +}
> > +
> > +static int plat_kim_chip_disable(struct kim_data_s *kim_data)
> > +{
> > +	gpio_set_value(GPIO_BT_EN, GPIO_LOW);
> > +	mdelay(1);
> > +	gpio_set_value(GPIO_BT_EN, GPIO_HIGH);
> > +	mdelay(1);
> > +	gpio_set_value(GPIO_BT_EN, GPIO_LOW);
> > +
> > +	return 0;
> > +}
> > +
> >  static struct ti_st_plat_data wilink_platform_data = {
> > -	.nshutdown_gpio	= 46,
> >  	.dev_name	= "/dev/ttyO1",
> >  	.flow_cntrl	= 1,
> >  	.baud_rate	= 3000000,
> > -	.chip_enable	= NULL,
> > -	.suspend	= NULL,
> > -	.resume		= NULL,
> > +	.chip_enable	= plat_kim_chip_enable,
> > +	.chip_disable	= plat_kim_chip_disable,
> 
> I just wonder how this is going to work with DT... You are not going to have
> the ability to use callback in this form.
> I think the GPIO handling should be done in the driver itself rather than in
> the board file.

that can (should ?) be moved to ti-st eventually. In fact I don't know
why it was removed in the first place, we would need Pavan to help us
with that query.

Still, for -rc, the minimal patch had to be cooked, right ?

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130117/fcb572ec/attachment.sig>

[[PATCH v2]] OMAP: omap4-panda: add WiLink shared transport power functions

[Peter Ujfalusi]

On 01/17/2013 10:34 AM, Felipe Balbi wrote:
>> I just wonder how this is going to work with DT... You are not going to have
>> the ability to use callback in this form.
>> I think the GPIO handling should be done in the driver itself rather than in
>> the board file.
> 
> that can (should ?) be moved to ti-st eventually. In fact I don't know
> why it was removed in the first place, we would need Pavan to help us
> with that query.

Yes, this is a good question. I don't know what is the spacial thing platforms
need to do in the callback..

> Still, for -rc, the minimal patch had to be cooked, right ?

Sure it need to be fixed. I would try to revert the patch which caused the
issue (eccf2979 drivers/misc/ti-st: remove gpio handling).

Should fix the legacy boot, but it is going to be even bigger fun to move to
DT (and get rid of the callbacks).

I don't have anything against this patch as such. Just wanted to point out the
obvious that the comfort of callbacks are not going to be around in some cases.

-- 
P?ter

[[PATCH v2]] OMAP: omap4-panda: add WiLink shared transport power functions

[Felipe Balbi]

Hi,

On Thu, Jan 17, 2013 at 10:55:14AM +0100, Peter Ujfalusi wrote:
> On 01/17/2013 10:34 AM, Felipe Balbi wrote:
> >> I just wonder how this is going to work with DT... You are not going to have
> >> the ability to use callback in this form.
> >> I think the GPIO handling should be done in the driver itself rather than in
> >> the board file.
> > 
> > that can (should ?) be moved to ti-st eventually. In fact I don't know
> > why it was removed in the first place, we would need Pavan to help us
> > with that query.
> 
> Yes, this is a good question. I don't know what is the spacial thing platforms
> need to do in the callback..
> 
> > Still, for -rc, the minimal patch had to be cooked, right ?
> 
> Sure it need to be fixed. I would try to revert the patch which caused the
> issue (eccf2979 drivers/misc/ti-st: remove gpio handling).
> 
> Should fix the legacy boot, but it is going to be even bigger fun to move to
> DT (and get rid of the callbacks).
> 
> I don't have anything against this patch as such. Just wanted to point out the
> obvious that the comfort of callbacks are not going to be around in some cases.

I guess Luca's choice was to cause the minimum impact to current code,
also reverting original commit isn't free os hassles either.

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130117/ce117e87/attachment.sig>

[[PATCH v2]] OMAP: omap4-panda: add WiLink shared transport power functions

[Felipe Balbi]

On Thu, Jan 17, 2013 at 12:05:10PM +0200, Felipe Balbi wrote:
> Hi,
> 
> On Thu, Jan 17, 2013 at 10:55:14AM +0100, Peter Ujfalusi wrote:
> > On 01/17/2013 10:34 AM, Felipe Balbi wrote:
> > >> I just wonder how this is going to work with DT... You are not going to have
> > >> the ability to use callback in this form.
> > >> I think the GPIO handling should be done in the driver itself rather than in
> > >> the board file.
> > > 
> > > that can (should ?) be moved to ti-st eventually. In fact I don't know
> > > why it was removed in the first place, we would need Pavan to help us
> > > with that query.
> > 
> > Yes, this is a good question. I don't know what is the spacial thing platforms
> > need to do in the callback..

hah! looks like I found the reason:

http://git.omapzoom.org/?p=kernel/omap.git;a=blob;f=arch/arm/mach-omap2/board-44xx-54xx-connectivity.c;h=e4852b93e91b6daa8f85cca91a1e7fbcc778f45b;hb=594aedd9e7da0491523411f8999efd98297f4fe4#l177

IMHO:

a) removing gpio handling wasn't necessary, we could just check
	if gpio_is_valid(nshutdown_gpio)

b) that whole omap_serial_ext_uart_enable() looks really hacky. I'm sure
	we can come up with something better.

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130117/bb543361/attachment-0001.sig>

[[PATCH v2]] OMAP: omap4-panda: add WiLink shared transport power functions

[Luciano Coelho]

On Thu, 2013-01-17 at 12:09 +0200, Felipe Balbi wrote:
> On Thu, Jan 17, 2013 at 12:05:10PM +0200, Felipe Balbi wrote:
> > Hi,
> > 
> > On Thu, Jan 17, 2013 at 10:55:14AM +0100, Peter Ujfalusi wrote:
> > > On 01/17/2013 10:34 AM, Felipe Balbi wrote:
> > > >> I just wonder how this is going to work with DT... You are not going to have
> > > >> the ability to use callback in this form.
> > > >> I think the GPIO handling should be done in the driver itself rather than in
> > > >> the board file.
> > > > 
> > > > that can (should ?) be moved to ti-st eventually. In fact I don't know
> > > > why it was removed in the first place, we would need Pavan to help us
> > > > with that query.
> > > 
> > > Yes, this is a good question. I don't know what is the spacial thing platforms
> > > need to do in the callback..
> 
> hah! looks like I found the reason:
> 
> http://git.omapzoom.org/?p=kernel/omap.git;a=blob;f=arch/arm/mach-omap2/board-44xx-54xx-connectivity.c;h=e4852b93e91b6daa8f85cca91a1e7fbcc778f45b;hb=594aedd9e7da0491523411f8999efd98297f4fe4#l177
> 
> IMHO:
> 
> a) removing gpio handling wasn't necessary, we could just check
> 	if gpio_is_valid(nshutdown_gpio)
> 
> b) that whole omap_serial_ext_uart_enable() looks really hacky. I'm sure
> 	we can come up with something better.
> 

This out-of-tree code doesn't explain why we need to do the
enable/disable in the board file.  We just need to do things a bit
differently in the driver.  I'll start cleaning all this stuff up for
-next pretty soon.

For now, ie. 3.7 (stable) and 3.8, do we agree in taking this patch so
that TI-ST at least works on Panda? Simply reverting the gpio removal
patch doesn't help, because we also need to handle the UART2 muxing
(which my patch does as well).

--
Cheers,
Luca.

[[PATCH v2]] OMAP: omap4-panda: add WiLink shared transport power functions

[Peter Ujfalusi]

On 01/17/2013 11:35 AM, Luciano Coelho wrote:
> This out-of-tree code doesn't explain why we need to do the
> enable/disable in the board file.  We just need to do things a bit
> differently in the driver.  I'll start cleaning all this stuff up for
> -next pretty soon.
> 
> For now, ie. 3.7 (stable) and 3.8, do we agree in taking this patch so
> that TI-ST at least works on Panda? Simply reverting the gpio removal
> patch doesn't help, because we also need to handle the UART2 muxing
> (which my patch does as well).

I don't see better way to fix this either. In any case, I give you my:

Reviewed-by: Peter Ujfalusi <peter.ujfalusi@ti.com>

[[PATCH v2]] OMAP: omap4-panda: add WiLink shared transport power functions

[Tony Lindgren]

* Peter Ujfalusi <peter.ujfalusi@ti.com> [130117 02:44]:
> On 01/17/2013 11:35 AM, Luciano Coelho wrote:
> > This out-of-tree code doesn't explain why we need to do the
> > enable/disable in the board file.  We just need to do things a bit
> > differently in the driver.  I'll start cleaning all this stuff up for
> > -next pretty soon.
> > 
> > For now, ie. 3.7 (stable) and 3.8, do we agree in taking this patch so
> > that TI-ST at least works on Panda? Simply reverting the gpio removal
> > patch doesn't help, because we also need to handle the UART2 muxing
> > (which my patch does as well).
> 
> I don't see better way to fix this either. In any case, I give you my:
> 
> Reviewed-by: Peter Ujfalusi <peter.ujfalusi@ti.com>

So what is actually broken? The horrible bluetooth muxing over serial
port? If so, let's rather fix it properly than even attempt to fix
it as it seems that it's been broken for two merge windows now.

Also, let's just do absolutely minimal board-*.c file fixes now.
This thing should be just moved to use DT so we can flip over omap4
to be DT only and drop estimated 5k LOC from mach-omap2.

Regards,

Tony 

[[PATCH v2]] OMAP: omap4-panda: add WiLink shared transport power functions

[Luciano Coelho]

Hi Tony,

On Thu, 2013-01-17 at 09:31 -0800, Tony Lindgren wrote:
> * Peter Ujfalusi <peter.ujfalusi@ti.com> [130117 02:44]:
> > On 01/17/2013 11:35 AM, Luciano Coelho wrote:
> > > This out-of-tree code doesn't explain why we need to do the
> > > enable/disable in the board file.  We just need to do things a bit
> > > differently in the driver.  I'll start cleaning all this stuff up for
> > > -next pretty soon.
> > > 
> > > For now, ie. 3.7 (stable) and 3.8, do we agree in taking this patch so
> > > that TI-ST at least works on Panda? Simply reverting the gpio removal
> > > patch doesn't help, because we also need to handle the UART2 muxing
> > > (which my patch does as well).
> > 
> > I don't see better way to fix this either. In any case, I give you my:
> > 
> > Reviewed-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> 
> So what is actually broken? The horrible bluetooth muxing over serial
> port? If so, let's rather fix it properly than even attempt to fix
> it as it seems that it's been broken for two merge windows now.

Yes, it is the horrible Bluetooth muxing over serial that is broken.  It
has been broken for two merge windows, because nobody seemed to care
until I started looking into it and tried to figure out how to get it to
work. :)

The implementation is really weak and there are loads of things I want
to start fixing/cleaning up.  This patch was just my intention to start
with a relatively clean table (ie. horrible or not, at least working).


> Also, let's just do absolutely minimal board-*.c file fixes now.
> This thing should be just moved to use DT so we can flip over omap4
> to be DT only and drop estimated 5k LOC from mach-omap2.

I totally agree, I'll start looking into that next.

But this patch is pretty small and simple, so why not include it to at
least fix the breakage in 3.7 and 3.8? Whether you take it or not now
won't make any difference in the 5k LOC in these kernel versions.


--
Cheers,
Luca.

[[PATCH v2]] OMAP: omap4-panda: add WiLink shared transport power functions

[Tony Lindgren]

* Luciano Coelho <coelho@ti.com> [130117 10:04]:
> Hi Tony,
> 
> On Thu, 2013-01-17 at 09:31 -0800, Tony Lindgren wrote:
> > * Peter Ujfalusi <peter.ujfalusi@ti.com> [130117 02:44]:
> > > On 01/17/2013 11:35 AM, Luciano Coelho wrote:
> > > > This out-of-tree code doesn't explain why we need to do the
> > > > enable/disable in the board file.  We just need to do things a bit
> > > > differently in the driver.  I'll start cleaning all this stuff up for
> > > > -next pretty soon.
> > > > 
> > > > For now, ie. 3.7 (stable) and 3.8, do we agree in taking this patch so
> > > > that TI-ST at least works on Panda? Simply reverting the gpio removal
> > > > patch doesn't help, because we also need to handle the UART2 muxing
> > > > (which my patch does as well).
> > > 
> > > I don't see better way to fix this either. In any case, I give you my:
> > > 
> > > Reviewed-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
> > 
> > So what is actually broken? The horrible bluetooth muxing over serial
> > port? If so, let's rather fix it properly than even attempt to fix
> > it as it seems that it's been broken for two merge windows now.
> 
> Yes, it is the horrible Bluetooth muxing over serial that is broken.  It
> has been broken for two merge windows, because nobody seemed to care
> until I started looking into it and tried to figure out how to get it to
> work. :)

Heh I see.
 
> The implementation is really weak and there are loads of things I want
> to start fixing/cleaning up.  This patch was just my intention to start
> with a relatively clean table (ie. horrible or not, at least working).
> 
> 
> > Also, let's just do absolutely minimal board-*.c file fixes now.
> > This thing should be just moved to use DT so we can flip over omap4
> > to be DT only and drop estimated 5k LOC from mach-omap2.
> 
> I totally agree, I'll start looking into that next.
> 
> But this patch is pretty small and simple, so why not include it to at
> least fix the breakage in 3.7 and 3.8? Whether you take it or not now
> won't make any difference in the 5k LOC in these kernel versions.

Well we are planning to drop the non-DT support for omap4 as soon as it's
usable with DT. For omap4 we are only carrying SDP and panda support to
make this transition easier. The only bindings missing AFAIK are wl12xx and
USB.

If we add this, then it implies we're somehow supporting it, which is not
the way to go IMHO as we need to get rid of these platform callbacks instead.

What's your estimate of having minimal wl12xx WLAN DT binding available?

Regards,

Tony

[[PATCH v2]] OMAP: omap4-panda: add WiLink shared transport power functions

[Luciano Coelho]

On Thu, 2013-01-17 at 15:16 -0800, Tony Lindgren wrote:
> * Luciano Coelho <coelho@ti.com> [130117 10:04]:
> > But this patch is pretty small and simple, so why not include it to at
> > least fix the breakage in 3.7 and 3.8? Whether you take it or not now
> > won't make any difference in the 5k LOC in these kernel versions.
> 
> Well we are planning to drop the non-DT support for omap4 as soon as it's
> usable with DT. For omap4 we are only carrying SDP and panda support to
> make this transition easier. The only bindings missing AFAIK are wl12xx and
> USB.

In my view this is a regression and it should be fixed with as simple a
patch as possible.  The alternative to my solution is to revert the
patch that removed the enable/disable from the ti-st driver *and* fix
u-boot, because if it doesn't mux the UART2 pins properly (and it
doesn't) the shared transport still won't work.


> If we add this, then it implies we're somehow supporting it, which is not
> the way to go IMHO as we need to get rid of these platform callbacks instead.

It's a regression fix, not a new feature.  I also think these callbacks
are silly, but it's the quickest solution I found for 3.7 and 3.8.


> What's your estimate of having minimal wl12xx WLAN DT binding available?

To tell you the truth, I haven't even started looking into DT for wl12xx
myself.  So I have no idea when it will be ready.  Benoit has been
looking into it, but I don't know how far he is.

--
Luca.

[[PATCH v2]] OMAP: omap4-panda: add WiLink shared transport power functions

[Tony Lindgren]

* Luciano Coelho <coelho@ti.com> [130118 01:03]:
> On Thu, 2013-01-17 at 15:16 -0800, Tony Lindgren wrote:
> > * Luciano Coelho <coelho@ti.com> [130117 10:04]:
> > > But this patch is pretty small and simple, so why not include it to at
> > > least fix the breakage in 3.7 and 3.8? Whether you take it or not now
> > > won't make any difference in the 5k LOC in these kernel versions.
> > 
> > Well we are planning to drop the non-DT support for omap4 as soon as it's
> > usable with DT. For omap4 we are only carrying SDP and panda support to
> > make this transition easier. The only bindings missing AFAIK are wl12xx and
> > USB.
> 
> In my view this is a regression and it should be fixed with as simple a
> patch as possible.  The alternative to my solution is to revert the
> patch that removed the enable/disable from the ti-st driver *and* fix
> u-boot, because if it doesn't mux the UART2 pins properly (and it
> doesn't) the shared transport still won't work.

Fixing the muxing here makes sense naturally as we cannot do that in the driver
until we've flipped things over to use DT.

But I don't think we should fix the driver regression by adding more platform
callbacks as we are getting rid of them anyways.

> > If we add this, then it implies we're somehow supporting it, which is not
> > the way to go IMHO as we need to get rid of these platform callbacks instead.
> 
> It's a regression fix, not a new feature.  I also think these callbacks
> are silly, but it's the quickest solution I found for 3.7 and 3.8.

Right, so how about let's fix the regression in the driver, and add the
muxing to platform init code? 
 
> > What's your estimate of having minimal wl12xx WLAN DT binding available?
> 
> To tell you the truth, I haven't even started looking into DT for wl12xx
> myself.  So I have no idea when it will be ready.  Benoit has been
> looking into it, but I don't know how far he is.

If it's going to take long we should just init the platform data for
it temporarily even in the DT boot case until the binding is available.

Regards,

Tony

[[PATCH v2]] OMAP: omap4-panda: add WiLink shared transport power functions

[Felipe Balbi]

Hi,

On Fri, Jan 18, 2013 at 09:36:35AM -0800, Tony Lindgren wrote:
> * Luciano Coelho <coelho@ti.com> [130118 01:03]:
> > On Thu, 2013-01-17 at 15:16 -0800, Tony Lindgren wrote:
> > > * Luciano Coelho <coelho@ti.com> [130117 10:04]:
> > > > But this patch is pretty small and simple, so why not include it to at
> > > > least fix the breakage in 3.7 and 3.8? Whether you take it or not now
> > > > won't make any difference in the 5k LOC in these kernel versions.
> > > 
> > > Well we are planning to drop the non-DT support for omap4 as soon as it's
> > > usable with DT. For omap4 we are only carrying SDP and panda support to
> > > make this transition easier. The only bindings missing AFAIK are wl12xx and
> > > USB.
> > 
> > In my view this is a regression and it should be fixed with as simple a
> > patch as possible.  The alternative to my solution is to revert the
> > patch that removed the enable/disable from the ti-st driver *and* fix
> > u-boot, because if it doesn't mux the UART2 pins properly (and it
> > doesn't) the shared transport still won't work.
> 
> Fixing the muxing here makes sense naturally as we cannot do that in the driver
> until we've flipped things over to use DT.
> 
> But I don't think we should fix the driver regression by adding more platform
> callbacks as we are getting rid of them anyways.

it's not adding more callbacks, solely implementing them as it should
have been done on Pavan's original patch.

> > > What's your estimate of having minimal wl12xx WLAN DT binding available?
> > 
> > To tell you the truth, I haven't even started looking into DT for wl12xx
> > myself.  So I have no idea when it will be ready.  Benoit has been
> > looking into it, but I don't know how far he is.
> 
> If it's going to take long we should just init the platform data for
> it temporarily even in the DT boot case until the binding is available.

which will boil down to having those platform_data callbacks to have it
at least functional. How will Luca test DT conversion if ti-st isn't
even functional ?

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130118/5dbd12a2/attachment-0001.sig>

[[PATCH v2]] OMAP: omap4-panda: add WiLink shared transport power functions

[Tony Lindgren]

* Felipe Balbi <balbi@ti.com> [130118 09:57]:
> Hi,
> 
> On Fri, Jan 18, 2013 at 09:36:35AM -0800, Tony Lindgren wrote:
> > * Luciano Coelho <coelho@ti.com> [130118 01:03]:
> > > On Thu, 2013-01-17 at 15:16 -0800, Tony Lindgren wrote:
> > > > * Luciano Coelho <coelho@ti.com> [130117 10:04]:
> > > > > But this patch is pretty small and simple, so why not include it to at
> > > > > least fix the breakage in 3.7 and 3.8? Whether you take it or not now
> > > > > won't make any difference in the 5k LOC in these kernel versions.
> > > > 
> > > > Well we are planning to drop the non-DT support for omap4 as soon as it's
> > > > usable with DT. For omap4 we are only carrying SDP and panda support to
> > > > make this transition easier. The only bindings missing AFAIK are wl12xx and
> > > > USB.
> > > 
> > > In my view this is a regression and it should be fixed with as simple a
> > > patch as possible.  The alternative to my solution is to revert the
> > > patch that removed the enable/disable from the ti-st driver *and* fix
> > > u-boot, because if it doesn't mux the UART2 pins properly (and it
> > > doesn't) the shared transport still won't work.
> > 
> > Fixing the muxing here makes sense naturally as we cannot do that in the driver
> > until we've flipped things over to use DT.
> > 
> > But I don't think we should fix the driver regression by adding more platform
> > callbacks as we are getting rid of them anyways.
> 
> it's not adding more callbacks, solely implementing them as it should
> have been done on Pavan's original patch.

It certainly is adding new callback functions to board-*.c files looking
at the diffstat :)

IMHO the right fix is to revert eccf2979 that caused the regression and then
adding the muxing to the board-*.c file(s).

It's OK for the driver to call the standard GPIO functions, and those will
be needed in the driver for the DT case anyways.

Regards,

Tony

[[PATCH v2]] OMAP: omap4-panda: add WiLink shared transport power functions

[Luciano Coelho]

On Fri, 2013-01-18 at 09:36 -0800, Tony Lindgren wrote:
> * Luciano Coelho <coelho@ti.com> [130118 01:03]:
> > On Thu, 2013-01-17 at 15:16 -0800, Tony Lindgren wrote:
> > > * Luciano Coelho <coelho@ti.com> [130117 10:04]:
> > > > But this patch is pretty small and simple, so why not include it to at
> > > > least fix the breakage in 3.7 and 3.8? Whether you take it or not now
> > > > won't make any difference in the 5k LOC in these kernel versions.
> > > 
> > > Well we are planning to drop the non-DT support for omap4 as soon as it's
> > > usable with DT. For omap4 we are only carrying SDP and panda support to
> > > make this transition easier. The only bindings missing AFAIK are wl12xx and
> > > USB.
> > 
> > In my view this is a regression and it should be fixed with as simple a
> > patch as possible.  The alternative to my solution is to revert the
> > patch that removed the enable/disable from the ti-st driver *and* fix
> > u-boot, because if it doesn't mux the UART2 pins properly (and it
> > doesn't) the shared transport still won't work.
> 
> Fixing the muxing here makes sense naturally as we cannot do that in the driver
> until we've flipped things over to use DT.
> 
> But I don't think we should fix the driver regression by adding more platform
> callbacks as we are getting rid of them anyways.

Okay, you're right.  We need at least to pass the GPIO number from the
board file to the driver so that it knows what to switch.  This is also
something that it will need from DT in the future.


> > > If we add this, then it implies we're somehow supporting it, which is not
> > > the way to go IMHO as we need to get rid of these platform callbacks instead.
> > 
> > It's a regression fix, not a new feature.  I also think these callbacks
> > are silly, but it's the quickest solution I found for 3.7 and 3.8.
> 
> Right, so how about let's fix the regression in the driver, and add the
> muxing to platform init code? 

Agreed.  I'll cook up a patch to revert the changes in eccf2979 (it
doesn't revert cleanly) and change the panda board file to do the muxing
and set the gpio number in the pdata for the driver to use.

 
> > > What's your estimate of having minimal wl12xx WLAN DT binding available?
> > 
> > To tell you the truth, I haven't even started looking into DT for wl12xx
> > myself.  So I have no idea when it will be ready.  Benoit has been
> > looking into it, but I don't know how far he is.
> 
> If it's going to take long we should just init the platform data for
> it temporarily even in the DT boot case until the binding is available.

I'll start looking into this now.  It's not going to be that simple, I'm
sure, but I need to look more into DT in general to be able to say
something more accurate.  I'll let you know as soon as I know more. :)

--
Cheers,
Luca.

[[PATCH v2]] OMAP: omap4-panda: add WiLink shared transport power functions

[Tony Lindgren]

* Luciano Coelho <coelho@ti.com> [130118 11:12]:
> On Fri, 2013-01-18 at 09:36 -0800, Tony Lindgren wrote:
> > * Luciano Coelho <coelho@ti.com> [130118 01:03]:
> > > On Thu, 2013-01-17 at 15:16 -0800, Tony Lindgren wrote:
> > > > * Luciano Coelho <coelho@ti.com> [130117 10:04]:
> > > > > But this patch is pretty small and simple, so why not include it to at
> > > > > least fix the breakage in 3.7 and 3.8? Whether you take it or not now
> > > > > won't make any difference in the 5k LOC in these kernel versions.
> > > > 
> > > > Well we are planning to drop the non-DT support for omap4 as soon as it's
> > > > usable with DT. For omap4 we are only carrying SDP and panda support to
> > > > make this transition easier. The only bindings missing AFAIK are wl12xx and
> > > > USB.
> > > 
> > > In my view this is a regression and it should be fixed with as simple a
> > > patch as possible.  The alternative to my solution is to revert the
> > > patch that removed the enable/disable from the ti-st driver *and* fix
> > > u-boot, because if it doesn't mux the UART2 pins properly (and it
> > > doesn't) the shared transport still won't work.
> > 
> > Fixing the muxing here makes sense naturally as we cannot do that in the driver
> > until we've flipped things over to use DT.
> > 
> > But I don't think we should fix the driver regression by adding more platform
> > callbacks as we are getting rid of them anyways.
> 
> Okay, you're right.  We need at least to pass the GPIO number from the
> board file to the driver so that it knows what to switch.  This is also
> something that it will need from DT in the future.

Yes I agree, the GPIO number needs to be passed in the fix. 
 
> > > > If we add this, then it implies we're somehow supporting it, which is not
> > > > the way to go IMHO as we need to get rid of these platform callbacks instead.
> > > 
> > > It's a regression fix, not a new feature.  I also think these callbacks
> > > are silly, but it's the quickest solution I found for 3.7 and 3.8.
> > 
> > Right, so how about let's fix the regression in the driver, and add the
> > muxing to platform init code? 
> 
> Agreed.  I'll cook up a patch to revert the changes in eccf2979 (it
> doesn't revert cleanly) and change the panda board file to do the muxing
> and set the gpio number in the pdata for the driver to use.

OK thanks. 
  
> > > > What's your estimate of having minimal wl12xx WLAN DT binding available?
> > > 
> > > To tell you the truth, I haven't even started looking into DT for wl12xx
> > > myself.  So I have no idea when it will be ready.  Benoit has been
> > > looking into it, but I don't know how far he is.
> > 
> > If it's going to take long we should just init the platform data for
> > it temporarily even in the DT boot case until the binding is available.
> 
> I'll start looking into this now.  It's not going to be that simple, I'm
> sure, but I need to look more into DT in general to be able to say
> something more accurate.  I'll let you know as soon as I know more. :)

OK thanks!

Tony

[[PATCH v2]] OMAP: omap4-panda: add WiLink shared transport power functions

[Peter Ujfalusi]

Hi Tony,

On 01/18/2013 12:16 AM, Tony Lindgren wrote:
> Well we are planning to drop the non-DT support for omap4 as soon as it's
> usable with DT.

Exactly, but right now we do have legacy and legacy should work in order to
help for example Luca to make the final push which allows us to move to DT
only for OMAP4.

> For omap4 we are only carrying SDP and panda support to
> make this transition easier. The only bindings missing AFAIK are wl12xx and
> USB.
>
> If we add this, then it implies we're somehow supporting it, which is not
> the way to go IMHO as we need to get rid of these platform callbacks instead.

IMHO we should keep these boards working as long as we have the legacy board
support for them.

So what is the policy now regarding to 'old' legacy but still used OMAP4 board
files? I think we should still maintain them, but the effort to get rid of
them should be the highest priority.

-- 
P?ter

[[PATCH v2]] OMAP: omap4-panda: add WiLink shared transport power functions

[Tony Lindgren]

* Peter Ujfalusi <peter.ujfalusi@ti.com> [130118 02:15]:
> Hi Tony,
> 
> On 01/18/2013 12:16 AM, Tony Lindgren wrote:
> > Well we are planning to drop the non-DT support for omap4 as soon as it's
> > usable with DT.
> 
> Exactly, but right now we do have legacy and legacy should work in order to
> help for example Luca to make the final push which allows us to move to DT
> only for OMAP4.

Yeah I'm suggesting we should do the muxing in the platform code and fix the
regression in the driver. But not add the new platform callbacks.
 
> > For omap4 we are only carrying SDP and panda support to
> > make this transition easier. The only bindings missing AFAIK are wl12xx and
> > USB.
> >
> > If we add this, then it implies we're somehow supporting it, which is not
> > the way to go IMHO as we need to get rid of these platform callbacks instead.
> 
> IMHO we should keep these boards working as long as we have the legacy board
> support for them.

Naturally. We cannot flip over to DT only boot until we have usable system :)
And to me "usable system" means being able to use my pandaboard es with a lapdock
for work things.

Currently it seems that we need the following working with DT to have panda
and blaze usable:

1. MMC (done)
2. USB (patches pending for EHCI and MUSB)
3. Ethernet (done for blaze, needs USB for panda)
4. DSS (various drivers pending, probably still needs some platform init code)
5. WLAN (no binding, needs platform init until done)
6. Audio (done?)

The DMA binding is still pending, but these devices should also work without it
until we have the binding. And various miscellaneous drivers we can init with
the platform code until the bindings are done.

> So what is the policy now regarding to 'old' legacy but still used OMAP4 board
> files? I think we should still maintain them, but the effort to get rid of
> them should be the highest priority.

Yes only minimal fixes. We should not add any new features to them as that will
just complicate the conversion to DT. Once we have a usable system booting with DT,
we can ust drop the old omap4 board-*.c files.

Regards,

Tony

[[PATCH v2]] OMAP: omap4-panda: add WiLink shared transport power functions

[Peter Ujfalusi]

Hi Tony,

On 01/18/2013 06:49 PM, Tony Lindgren wrote:
> Currently it seems that we need the following working with DT to have panda
> and blaze usable:
> 
> 1. MMC (done)
> 2. USB (patches pending for EHCI and MUSB)
> 3. Ethernet (done for blaze, needs USB for panda)
> 4. DSS (various drivers pending, probably still needs some platform init code)
> 5. WLAN (no binding, needs platform init until done)
> 6. Audio (done?)

Yes, audio is ready. We are covering OMAP2/3/4/5 (adding OMAP1 should not be a
big issue either) with SoC IPs in DT support. We have dmaengine for all OMAP
audio stuff. Codec drivers I maintain in upstream are also works with DT. We
have ASoC machine drivers to cover most of the upstream boards (OMAP3/4/5). So
audio is pretty much done in DT context, we are waiting for the DMA bindings
only AFAIK.

-- 
P?ter

[[PATCH v2]] OMAP: omap4-panda: add WiLink shared transport power functions

[Luciano Coelho]

On Thu, 2013-01-17 at 10:30 +0100, Peter Ujfalusi wrote:
> Hi Luca,

Hi P?ter!

> On 01/16/2013 10:45 PM, Luciano Coelho wrote:
> >  static struct ti_st_plat_data wilink_platform_data = {
> > -	.nshutdown_gpio	= 46,
> >  	.dev_name	= "/dev/ttyO1",
> >  	.flow_cntrl	= 1,
> >  	.baud_rate	= 3000000,
> > -	.chip_enable	= NULL,
> > -	.suspend	= NULL,
> > -	.resume		= NULL,
> > +	.chip_enable	= plat_kim_chip_enable,
> > +	.chip_disable	= plat_kim_chip_disable,
> 
> I just wonder how this is going to work with DT... You are not going to have
> the ability to use callback in this form.
> I think the GPIO handling should be done in the driver itself rather than in
> the board file.

I agree.  The problem is that it used to be in the ti-st driver itself,
but it has been removed in a patch that says "different platforms have
begun to have their own ways to power-up/down the chip." (eccf2979
drivers/misc/ti-st: remove gpio handling).

This needs to be clarified first.  I think we could use this for now and
later fix this properly (hopefully move it back to the ti-st driver).

--
Cheers,
Luca.

[[PATCH v2]] OMAP: omap4-panda: add WiLink shared transport power functions

[Peter Ujfalusi]

Hi Luca,

On 01/17/2013 10:35 AM, Luciano Coelho wrote:
>> I just wonder how this is going to work with DT... You are not going to have
>> the ability to use callback in this form.
>> I think the GPIO handling should be done in the driver itself rather than in
>> the board file.
> 
> I agree.  The problem is that it used to be in the ti-st driver itself,
> but it has been removed in a patch that says "different platforms have
> begun to have their own ways to power-up/down the chip." (eccf2979
> drivers/misc/ti-st: remove gpio handling).

Hrm, this is a strange patch for sure. Coming at times when we all suppose to
move to DT and get rid of such callbacks for drivers.

> This needs to be clarified first.  I think we could use this for now and
> later fix this properly (hopefully move it back to the ti-st driver).

Can the offending patch be reverted?
As I said to Felipe, I don't have any objections against this patch. It fits
the purpose. But with the DT support (and removing callbacks to platform code)
you are going to have fair amount work.

-- 
P?ter