[PATCH 0/7] IFX6X60 patchset: 01/05/2011

[PATCH 0/7] IFX6X60 patchset: 01/05/2011

[Russ Gorby]

Hello SPI/Serial Maintainers,

I am sending a patchset for the ifx6x60 driver to be considered for
inclusion in the Linux kernel.
Patches were made against the following GIT repo
http://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
A breif description of the patches follows:

[PATCH 1/7] serial: ifx6x60: fixed call to tty_port_init
       - fixed order of setting port ops
[PATCH 2/7] serial: ifx6x60: dma_alloc_coherent must use parent dev
       - fixed device for which the allocation is made
[PATCH 3/7] serial: ifx6x60: changed internal bpw from boolean to int
[PATCH 4/7] serial: ifx6x60: set SPI max_speed_hz based on platform type
       - set max speed to 25Mhz for platforms that support it
[PATCH 5/7] serial: ifx6x60: probe routine needs to call spi_setup
       - configured spi for the h/w device
[PATCH 6/7] serial: ifx6x60: fix modem names to ones recognized by firmware
[PATCH 7/7] serial: ifx6x60: cleanup

Re: [PATCH 0/7] IFX6X60 patchset: 01/05/2011

[Greg KH]

On Wed, Jan 05, 2011 at 04:05:06PM -0800, Russ Gorby wrote:
> Hello SPI/Serial Maintainers,

Um, you really got the cc: list wrong, please fix that next time.

> I am sending a patchset for the ifx6x60 driver to be considered for
> inclusion in the Linux kernel.
> Patches were made against the following GIT repo
> http://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
> A breif description of the patches follows:

It's too late now, sorry.  We are in the middle of the big merge window
for .38-rc1 (which means your patches needed to be in a maintainer's
tree a few weeks ago).  I'll consider these after that comes out.

thanks,

greg k-h

RE: [PATCH 0/7] IFX6X60 patchset: 01/05/2011

[Gorby, Russ]

>-----Original Message-----
>
>On Wed, Jan 05, 2011 at 04:05:06PM -0800, Russ Gorby wrote:
>> Hello SPI/Serial Maintainers,
>
>Um, you really got the cc: list wrong, please fix that next time.
>
[Gorby, Russ] sorry, get_maintainers.pl output was not properly consumed by my script and got some additional garbage addresses

>
>It's too late now, sorry.  We are in the middle of the big merge window
>for .38-rc1 (which means your patches needed to be in a maintainer's
>tree a few weeks ago).  I'll consider these after that comes out.
>

[Gorby, Russ] fair enough - do you want me to resubmit the patchset later on or will you cache it and take it up later?

Re: [PATCH 0/7] IFX6X60 patchset: 01/05/2011

[Greg KH]

On Wed, Jan 05, 2011 at 04:33:39PM -0800, Gorby, Russ wrote:
> 
> 
> >-----Original Message-----
> >
> >On Wed, Jan 05, 2011 at 04:05:06PM -0800, Russ Gorby wrote:
> >> Hello SPI/Serial Maintainers,
> >
> >Um, you really got the cc: list wrong, please fix that next time.
> >
> [Gorby, Russ] sorry, get_maintainers.pl output was not properly consumed by my script and got some additional garbage addresses

That's some wierd email quoting, please fix your email client (i.e. use
a sane one.)

> >It's too late now, sorry.  We are in the middle of the big merge window
> >for .38-rc1 (which means your patches needed to be in a maintainer's
> >tree a few weeks ago).  I'll consider these after that comes out.
> >
> 
> [Gorby, Russ] fair enough - do you want me to resubmit the patchset later on or will you cache it and take it up later?

Please send it again, as it isn't correct as pointed out by Alan :)

thanks,

greg k-h

[PATCH 1/7] serial: ifx6x60: fixed call to tty_port_init

[Russ Gorby]

The port ops must be set AFTER calling port init as that function
zeroes the structure

Signed-off-by: Russ Gorby <russ.gorby@intel.com>
---
 drivers/serial/ifx6x60.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/serial/ifx6x60.c b/drivers/serial/ifx6x60.c
index ab93763..1eb44f9 100644
--- a/drivers/serial/ifx6x60.c
+++ b/drivers/serial/ifx6x60.c
@@ -798,8 +798,8 @@ static int ifx_spi_create_port(struct ifx_spi_device *ifx_dev)
 		goto error_ret;
 	}
 
-	pport->ops = &ifx_tty_port_ops;
 	tty_port_init(pport);
+	pport->ops = &ifx_tty_port_ops;
 	ifx_dev->minor = IFX_SPI_TTY_ID;
 	ifx_dev->tty_dev = tty_register_device(tty_drv, ifx_dev->minor,
 					       &ifx_dev->spi_dev->dev);
-- 
1.7.2.3

Re: [PATCH 1/7] serial: ifx6x60: fixed call to tty_port_init

[Alan Cox]

On Wed,  5 Jan 2011 16:05:07 -0800
Russ Gorby <russ.gorby@intel.com> wrote:

> The port ops must be set AFTER calling port init as that function
> zeroes the structure
> 
> Signed-off-by: Russ Gorby <russ.gorby@intel.com>

Acked-by: Alan Cox <alan@linux.intel.com>

(Your mail headers appear corrupt btw ?)

[PATCH 2/7] serial: ifx6x60: dma_alloc_coherent must use parent dev

[Russ Gorby]

This driver is a SPI protocol driver and has no DMA ops
associated with the device so the call will fail. Furthermore,
the DMA allocation made here will be used by the SPI
controller driver (parent dev) so it makes sense to
pass that device instead.

Signed-off-by: Russ Gorby <russ.gorby@intel.com>
---
 drivers/serial/ifx6x60.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/serial/ifx6x60.c b/drivers/serial/ifx6x60.c
index 1eb44f9..e18ae38 100644
--- a/drivers/serial/ifx6x60.c
+++ b/drivers/serial/ifx6x60.c
@@ -990,7 +990,7 @@ static int ifx_spi_spi_probe(struct spi_device *spi)
 	ifx_dev->spi_slave_cts = 0;
 
 	/*initialize transfer and dma buffers */
-	ifx_dev->tx_buffer = dma_alloc_coherent(&ifx_dev->spi_dev->dev,
+	ifx_dev->tx_buffer = dma_alloc_coherent(ifx_dev->spi_dev->dev.parent,
 				IFX_SPI_TRANSFER_SIZE,
 				&ifx_dev->tx_bus,
 				GFP_KERNEL);
@@ -999,7 +999,7 @@ static int ifx_spi_spi_probe(struct spi_device *spi)
 		ret = -ENOMEM;
 		goto error_ret;
 	}
-	ifx_dev->rx_buffer = dma_alloc_coherent(&ifx_dev->spi_dev->dev,
+	ifx_dev->rx_buffer = dma_alloc_coherent(ifx_dev->spi_dev->dev.parent,
 				IFX_SPI_TRANSFER_SIZE,
 				&ifx_dev->rx_bus,
 				GFP_KERNEL);
-- 
1.7.2.3

[PATCH 3/7] serial: ifx6x60: changed internal bpw from boolean to int

[Russ Gorby]

driver should support 32bit SPI transfers. The boolean variable
only allowed 8/16.

Changed to support 8/16/32 for future enabling
of 32 bpw.

Signed-off-by: Russ Gorby <russ.gorby@intel.com>
---
 drivers/serial/ifx6x60.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/serial/ifx6x60.c b/drivers/serial/ifx6x60.c
index e18ae38..fd10103 100644
--- a/drivers/serial/ifx6x60.c
+++ b/drivers/serial/ifx6x60.c
@@ -76,7 +76,7 @@
 static void ifx_spi_handle_srdy(struct ifx_spi_device *ifx_dev);
 
 /* local variables */
-static int spi_b16 = 1;			/* 8 or 16 bit word length */
+static int spi_bpw = 16;		/* 8, 16 or 32 bit word length */
 static struct tty_driver *tty_drv;
 static struct ifx_spi_device *saved_ifx_dev;
 static struct lock_class_key ifx_spi_key;
@@ -724,7 +724,7 @@ static void ifx_spi_io(unsigned long data)
 		ifx_dev->spi_xfer.cs_change = 0;
 		ifx_dev->spi_xfer.speed_hz = 12500000;
 		/* ifx_dev->spi_xfer.speed_hz = 390625; */
-		ifx_dev->spi_xfer.bits_per_word = spi_b16 ? 16 : 8;
+		ifx_dev->spi_xfer.bits_per_word = spi_bpw;
 
 		ifx_dev->spi_xfer.tx_buf = ifx_dev->tx_buffer;
 		ifx_dev->spi_xfer.rx_buf = ifx_dev->rx_buffer;
-- 
1.7.2.3

Re: [PATCH 3/7] serial: ifx6x60: changed internal bpw from boolean to int

[Alan Cox]

On Wed,  5 Jan 2011 16:05:09 -0800
Russ Gorby <russ.gorby@intel.com> wrote:

> driver should support 32bit SPI transfers. The boolean variable
> only allowed 8/16.
> 
> Changed to support 8/16/32 for future enabling
> of 32 bpw.
> 
> Signed-off-by: Russ Gorby <russ.gorby@intel.com>


Acked-by: Alan Cox <alan@linux.intel.com>

[PATCH 4/7] serial: ifx6x60: set SPI max_speed_hz based on platform type

[Russ Gorby]

Platforms containing the 6260 can run up to 25Mhz.

For these platforms set max_speed_hz to 25Mhz.

Signed-off-by: Russ Gorby <russ.gorby@intel.com>
---
 drivers/serial/ifx6x60.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/drivers/serial/ifx6x60.c b/drivers/serial/ifx6x60.c
index fd10103..b54a990 100644
--- a/drivers/serial/ifx6x60.c
+++ b/drivers/serial/ifx6x60.c
@@ -722,7 +722,7 @@ static void ifx_spi_io(unsigned long data)
 		/* note len is BYTES, not transfers */
 		ifx_dev->spi_xfer.len = IFX_SPI_TRANSFER_SIZE;
 		ifx_dev->spi_xfer.cs_change = 0;
-		ifx_dev->spi_xfer.speed_hz = 12500000;
+		ifx_dev->spi_xfer.speed_hz = ifx_dev->spi_dev->max_speed_hz;
 		/* ifx_dev->spi_xfer.speed_hz = 390625; */
 		ifx_dev->spi_xfer.bits_per_word = spi_bpw;
 
@@ -984,6 +984,10 @@ static int ifx_spi_spi_probe(struct spi_device *spi)
 	ifx_dev->spi_timer.function = ifx_spi_timeout;
 	ifx_dev->spi_timer.data = (unsigned long)ifx_dev;
 	ifx_dev->is_6160 = pl_data->is_6160;
+	if (ifx_dev->is_6160)
+		spi->max_speed_hz = 12500000;
+	else
+		spi->max_speed_hz = 25000000;
 
 	/* ensure SPI protocol flags are initialized to enable transfer */
 	ifx_dev->spi_more = 0;
-- 
1.7.2.3

Re: [PATCH 4/7] serial: ifx6x60: set SPI max_speed_hz based on platform type

[Alan Cox]

> +	if (ifx_dev->is_6160)
> +		spi->max_speed_hz = 12500000;
> +	else
> +		spi->max_speed_hz = 25000000;

This should be coming directly from the platform data, ditto I suspect
wiring the DMA to is_6160 should probably be a platform "use_dma"
field ?

RE: [PATCH 4/7] serial: ifx6x60: set SPI max_speed_hz based on platform type

[Gorby, Russ]

>-----Original Message-----
>From: Alan Cox [mailto:alan@linux.intel.com]
>Sent: Wednesday, January 05, 2011 4:27 PM
>To: Gorby, Russ
>Cc: gregkh@suse.de; segoon@openwall.com; Gorby, Russ; linux-
>kernel@vger.kernel.org; Ahmed, Suhail
>Subject: Re: [PATCH 4/7] serial: ifx6x60: set SPI max_speed_hz based on
>platform type
>
>> +	if (ifx_dev->is_6160)
>> +		spi->max_speed_hz = 12500000;
>> +	else
>> +		spi->max_speed_hz = 25000000;
>
>This should be coming directly from the platform data, ditto I suspect
>wiring the DMA to is_6160 should probably be a platform "use_dma"
>field ?
[Gorby, Russ]
OK I agree for the max clock rate, but I'm not so sure about the "use_dma" case.

Use_dma is used so the protocol driver matches the expectations (capabilities) of the controller driver it attaches to. In the general case I would think it could support several different controller drivers that might have differing capabilities. Currently the driver does assume the value of use_dma is constant for a platform so putting this indication in the platform data would suffice (for now) but to me, this is not really a platform attribute.
Is there a better way to handle this is?

Re: [PATCH 4/7] serial: ifx6x60: set SPI max_speed_hz based on platform type

[Alan Cox]

> Use_dma is used so the protocol driver matches the expectations
> (capabilities) of the controller driver it attaches to. In the
> general case I would think it could support several different
> controller drivers that might have differing capabilities. Currently
> the driver does assume the value of use_dma is constant for a
> platform so putting this indication in the platform data would
> suffice (for now) but to me, this is not really a platform attribute.
> Is there a better way to handle this is?

The driver should know nothing about the plumbing itself. So the
platform data should deal with this and pass it in generic terms.

In the Intel MID case arch/x86/platform/mrst/ should know about whether
DMA can be used and what mode the SFI controller should be in. The bits
that are in ifx6x60.c should be as architecture neutral as possible.

Alan

[PATCH 5/7] serial: ifx6x60: probe routine needs to call spi_setup

[Russ Gorby]

The probe routine should call spi_setup() to configure
the SPI bus so it can properly communicate with the device.
E.g. the device operates in SPI mode 1.

Called spi_setup to configure SPI mode, max_speed_hz, and bpw

Signed-off-by: Russ Gorby <russ.gorby@intel.com>
---
 drivers/serial/ifx6x60.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/drivers/serial/ifx6x60.c b/drivers/serial/ifx6x60.c
index b54a990..6dfb47a 100644
--- a/drivers/serial/ifx6x60.c
+++ b/drivers/serial/ifx6x60.c
@@ -67,6 +67,7 @@
 #define IFX_SPI_MORE_MASK		0x10
 #define IFX_SPI_MORE_BIT		12	/* bit position in u16 */
 #define IFX_SPI_CTS_BIT			13	/* bit position in u16 */
+#define IFX_SPI_MODE			SPI_MODE_1
 #define IFX_SPI_TTY_ID			0
 #define IFX_SPI_TIMEOUT_SEC		2
 #define IFX_SPI_HEADER_0		(-1)
@@ -984,10 +985,18 @@ static int ifx_spi_spi_probe(struct spi_device *spi)
 	ifx_dev->spi_timer.function = ifx_spi_timeout;
 	ifx_dev->spi_timer.data = (unsigned long)ifx_dev;
 	ifx_dev->is_6160 = pl_data->is_6160;
+	/* initialize spi mode, etc */
 	if (ifx_dev->is_6160)
 		spi->max_speed_hz = 12500000;
 	else
 		spi->max_speed_hz = 25000000;
+	spi->mode = IFX_SPI_MODE | (SPI_LOOP & spi->mode);
+	spi->bits_per_word = spi_bpw;
+	ret = spi_setup(spi);
+	if (ret) {
+		dev_err(&spi->dev, "SPI setup wasn't successful %d", ret);
+		return -ENODEV;
+	}
 
 	/* ensure SPI protocol flags are initialized to enable transfer */
 	ifx_dev->spi_more = 0;
-- 
1.7.2.3

[PATCH 6/7] serial: ifx6x60: fix modem names to ones recognized by firmware

[Russ Gorby]

The current strings used in ID table are not correct and prevent
protocol driver connecting to controller driver.
Also, the indication of which modem h/w is present is not
currently provided by the platform data.

Changed ifx_id_table[] to use strings recognized by platform firmware

Signed-off-by: Russ Gorby <russ.gorby@intel.com>
---
 drivers/serial/ifx6x60.c |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/serial/ifx6x60.c b/drivers/serial/ifx6x60.c
index 6dfb47a..5a0321a 100644
--- a/drivers/serial/ifx6x60.c
+++ b/drivers/serial/ifx6x60.c
@@ -81,6 +81,10 @@ static int spi_bpw = 16;		/* 8, 16 or 32 bit word length */
 static struct tty_driver *tty_drv;
 static struct ifx_spi_device *saved_ifx_dev;
 static struct lock_class_key ifx_spi_key;
+enum ifx_mdm {
+	IFX_6160 = 1,
+	IFX_6260,
+};
 
 /* GPIO/GPE settings */
 
@@ -984,7 +988,10 @@ static int ifx_spi_spi_probe(struct spi_device *spi)
 	init_timer(&ifx_dev->spi_timer);
 	ifx_dev->spi_timer.function = ifx_spi_timeout;
 	ifx_dev->spi_timer.data = (unsigned long)ifx_dev;
+	/* FIXME: platform data does not currently supply this information
 	ifx_dev->is_6160 = pl_data->is_6160;
+	*/
+	ifx_dev->is_6160 = (spi_get_device_id(spi)->driver_data == IFX_6160);
 	/* initialize spi mode, etc */
 	if (ifx_dev->is_6160)
 		spi->max_speed_hz = 12500000;
@@ -1328,8 +1335,8 @@ static const struct dev_pm_ops ifx_spi_pm = {
 };
 
 static const struct spi_device_id ifx_id_table[] = {
-	{"ifx6160", 0},
-	{"ifx6260", 0},
+	{ "spi_6160_modem", IFX_6160 },
+	{ "spi_6260_modem", IFX_6260 },
 	{ }
 };
 MODULE_DEVICE_TABLE(spi, ifx_id_table);
-- 
1.7.2.3

Re: [PATCH 6/7] serial: ifx6x60: fix modem names to ones recognized by firmware

[Alan Cox]

On Wed,  5 Jan 2011 16:05:12 -0800
Russ Gorby <russ.gorby@intel.com> wrote:

> The current strings used in ID table are not correct and prevent
> protocol driver connecting to controller driver.
> Also, the indication of which modem h/w is present is not
> currently provided by the platform data.
> 
> Changed ifx_id_table[] to use strings recognized by platform firmware

No - the id table should be the actual I²C device names. The platform
code needs to do any name conversions from platform glue to standard
Linux naming.

Alan

[PATCH 7/7] serial: ifx6x60: minor cleanup

[Russ Gorby]

renamed spi_driver variable to not be h/w specific
set driver name to use DRVNAME define
removed commented-out define

Signed-off-by: Russ Gorby <russ.gorby@intel.com>
---
 drivers/serial/ifx6x60.c |    8 ++++----
 drivers/serial/ifx6x60.h |    2 --
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/serial/ifx6x60.c b/drivers/serial/ifx6x60.c
index 5a0321a..e296f97 100644
--- a/drivers/serial/ifx6x60.c
+++ b/drivers/serial/ifx6x60.c
@@ -1342,9 +1342,9 @@ static const struct spi_device_id ifx_id_table[] = {
 MODULE_DEVICE_TABLE(spi, ifx_id_table);
 
 /* spi operations */
-static const struct spi_driver ifx_spi_driver_6160 = {
+static const struct spi_driver ifx_spi_driver = {
 	.driver = {
-		.name = "ifx6160",
+		.name = DRVNAME,
 		.bus = &spi_bus_type,
 		.pm = &ifx_spi_pm,
 		.owner = THIS_MODULE},
@@ -1366,7 +1366,7 @@ static void __exit ifx_spi_exit(void)
 {
 	/* unregister */
 	tty_unregister_driver(tty_drv);
-	spi_unregister_driver((void *)&ifx_spi_driver_6160);
+	spi_unregister_driver((void *)&ifx_spi_driver);
 }
 
 /**
@@ -1408,7 +1408,7 @@ static int __init ifx_spi_init(void)
 		return result;
 	}
 
-	result = spi_register_driver((void *)&ifx_spi_driver_6160);
+	result = spi_register_driver((void *)&ifx_spi_driver);
 	if (result) {
 		pr_err("%s: spi_register_driver failed(%d)",
 			DRVNAME, result);
diff --git a/drivers/serial/ifx6x60.h b/drivers/serial/ifx6x60.h
index deb7b8d..5952c40 100644
--- a/drivers/serial/ifx6x60.h
+++ b/drivers/serial/ifx6x60.h
@@ -29,8 +29,6 @@
 #define DRVNAME				"ifx6x60"
 #define TTYNAME				"ttyIFX"
 
-/* #define IFX_THROTTLE_CODE */
-
 #define IFX_SPI_MAX_MINORS		1
 #define IFX_SPI_TRANSFER_SIZE		2048
 #define IFX_SPI_FIFO_SIZE		4096
-- 
1.7.2.3

Re: [PATCH 7/7] serial: ifx6x60: minor cleanup

[Alan Cox]

On Wed,  5 Jan 2011 16:05:13 -0800
Russ Gorby <russ.gorby@intel.com> wrote:

> renamed spi_driver variable to not be h/w specific
> set driver name to use DRVNAME define
> removed commented-out define
> 
> Signed-off-by: Russ Gorby <russ.gorby@intel.com>

Acked-by: Alan Cox <alan@linux.intel.com>