[PATCH 1/4] serial: imx: checkpatch cleanup

[PATCH 1/4] serial: imx: checkpatch cleanup

[Aurelien BOUIN]

This patch solve various issue detected by checkpatch.pl

Signed-off-by: Aurelien BOUIN <a.bouin@gmail.com>

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index a086eef..63eb457 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -275,9 +275,12 @@ static struct platform_device_id imx_uart_devtype[] = {
 MODULE_DEVICE_TABLE(platform, imx_uart_devtype);
 
 static struct of_device_id imx_uart_dt_ids[] = {
-	{ .compatible = "fsl,imx6q-uart", .data = &imx_uart_devdata[IMX6Q_UART], },
-	{ .compatible = "fsl,imx1-uart", .data = &imx_uart_devdata[IMX1_UART], },
-	{ .compatible = "fsl,imx21-uart", .data = &imx_uart_devdata[IMX21_UART], },
+	{ .compatible = "fsl,imx6q-uart",
+			.data = &imx_uart_devdata[IMX6Q_UART], },
+	{ .compatible = "fsl,imx1-uart",
+			.data = &imx_uart_devdata[IMX1_UART], },
+	{ .compatible = "fsl,imx21-uart",
+			.data = &imx_uart_devdata[IMX21_UART], },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, imx_uart_dt_ids);
@@ -430,6 +433,7 @@ static void imx_stop_tx(struct uart_port *port)
 	if (USE_IRDA(sport)) {
 		/* half duplex - wait for end of transmission */
 		int n = 256;
+
 		while ((--n > 0) &&
 		      !(readl(sport->port.membase + USR2) & USR2_TXDC)) {
 			udelay(5);
@@ -621,7 +625,6 @@ static void imx_dma_tx(struct imx_port *sport)
 	sport->dma_is_txing = 1;
 	dmaengine_submit(desc);
 	dma_async_issue_pending(chan);
-	return;
 }
 
 /*
@@ -870,7 +873,8 @@ static unsigned int imx_tx_empty(struct uart_port *port)
 	struct imx_port *sport = (struct imx_port *)port;
 	unsigned int ret;
 
-	ret = (readl(sport->port.membase + USR2) & USR2_TXDC) ?  TIOCSER_TEMT : 0;
+	ret = (readl(sport->port.membase + USR2) & USR2_TXDC) ?
+			TIOCSER_TEMT : 0;
 
 	/* If the TX DMA is working, return 0. */
 	if (sport->dma_is_enabled && sport->dma_is_txing)
@@ -1256,6 +1260,7 @@ static int imx_startup(struct uart_port *port)
 
 	if (USE_IRDA(sport)) {
 		struct imxuart_platform_data *pdata;
+
 		pdata = dev_get_platdata(sport->port.dev);
 		sport->irda_inv_rx = pdata->irda_inv_rx;
 		sport->irda_inv_tx = pdata->irda_inv_tx;
@@ -1299,6 +1304,7 @@ static void imx_shutdown(struct uart_port *port)
 
 	if (USE_IRDA(sport)) {
 		struct imxuart_platform_data *pdata;
+
 		pdata = dev_get_platdata(sport->port.dev);
 		if (pdata->irda_enable)
 			pdata->irda_enable(0);
@@ -1754,7 +1760,8 @@ imx_console_get_options(struct imx_port *sport, int *baud,
 		ubir = readl(sport->port.membase + UBIR) & 0xffff;
 		ubmr = readl(sport->port.membase + UBMR) & 0xffff;
 
-		ucfr_rfdiv = (readl(sport->port.membase + UFCR) & UFCR_RFDIV) >> 7;
+		ucfr_rfdiv = (readl(sport->port.membase + UFCR) & UFCR_RFDIV)
+				>> 7;
 		if (ucfr_rfdiv == 6)
 			ucfr_rfdiv = 7;
 		else
@@ -1763,12 +1770,13 @@ imx_console_get_options(struct imx_port *sport, int *baud,
 		uartclk = clk_get_rate(sport->clk_per);
 		uartclk /= ucfr_rfdiv;
 
-		{	/*
-			 * The next code provides exact computation of
-			 *   baud_raw = round(((uartclk/16) * (ubir + 1)) / (ubmr + 1))
-			 * without need of float support or long long division,
-			 * which would be required to prevent 32bit arithmetic overflow
-			 */
+		{
+		/*
+		 * The next code provides exact computation of
+		 *   baud_raw = round(((uartclk/16) * (ubir + 1)) / (ubmr + 1))
+		 * without need of float support or long long division,
+		 * which would be required to prevent 32bit arithmetic overflow
+		 */
 			unsigned int mul = ubir + 1;
 			unsigned int div = 16 * (ubmr + 1);
 			unsigned int rem = uartclk % div;
@@ -1844,7 +1852,7 @@ static struct console imx_console = {
 	.data		= &imx_reg,
 };
 
-#define IMX_CONSOLE	&imx_console
+#define IMX_CONSOLE	(&imx_console)
 #else
 #define IMX_CONSOLE	NULL
 #endif
-- 
1.9.1

[PATCH 2/4] spi: imx: checkpatch cleanup

[Aurelien BOUIN]

This patch solve various issue detected by checkpatch.pl

Signed-off-by: Aurelien BOUIN <a.bouin@gmail.com>

diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
index fe1b769..f8400b2 100644
--- a/drivers/spi/spi-imx.c
+++ b/drivers/spi/spi-imx.c
@@ -282,7 +282,8 @@ static unsigned int mx51_ecspi_clkdiv(unsigned int fin, unsigned int fspi,
 		(post << MX51_ECSPI_CTRL_POSTDIV_OFFSET);
 }
 
-static void __maybe_unused mx51_ecspi_intctrl(struct spi_imx_data *spi_imx, int enable)
+static void __maybe_unused mx51_ecspi_intctrl(struct spi_imx_data *spi_imx,
+		int enable)
 {
 	unsigned val = 0;
 
@@ -426,7 +427,8 @@ static void __maybe_unused mx51_ecspi_reset(struct spi_imx_data *spi_imx)
  * the i.MX35 has a slightly different register layout for bits
  * we do not use here.
  */
-static void __maybe_unused mx31_intctrl(struct spi_imx_data *spi_imx, int enable)
+static void __maybe_unused mx31_intctrl(struct spi_imx_data *spi_imx,
+		int enable)
 {
 	unsigned int val = 0;
 
@@ -504,7 +506,8 @@ static void __maybe_unused mx31_reset(struct spi_imx_data *spi_imx)
 #define MX21_CSPICTRL_DR_SHIFT	14
 #define MX21_CSPICTRL_CS_SHIFT	19
 
-static void __maybe_unused mx21_intctrl(struct spi_imx_data *spi_imx, int enable)
+static void __maybe_unused mx21_intctrl(struct spi_imx_data *spi_imx,
+		int enable)
 {
 	unsigned int val = 0;
 
@@ -1111,6 +1114,7 @@ static int spi_imx_probe(struct platform_device *pdev)
 
 	for (i = 0; i < master->num_chipselect; i++) {
 		int cs_gpio = of_get_named_gpio(np, "cs-gpios", i);
+
 		if (!gpio_is_valid(cs_gpio) && mxc_platform_info)
 			cs_gpio = mxc_platform_info->chipselect[i];
 
-- 
1.9.1

[PATCH 3/4] spi: imx: Chose DMA bits per word transfered

[Aurelien BOUIN]

This patch enable to give a parameter to device tree to set the number of bits per word the DMA uses in transfer
In device tree you can specify fsl,spi-dma-bits-per-word = <16>; in the ecspi section

Signed-off-by: Aurelien BOUIN <a.bouin@gmail.com>

diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
index f8400b2..9690d57 100644
--- a/drivers/spi/spi-imx.c
+++ b/drivers/spi/spi-imx.c
@@ -110,6 +110,7 @@ struct spi_imx_data {
 	u32 rxt_wml;
 	struct completion dma_rx_completion;
 	struct completion dma_tx_completion;
+	unsigned int dma_bits_per_word;
 
 	const struct spi_imx_devtype_data *devtype_data;
 	int chipselect[0];
@@ -825,6 +826,7 @@ static int spi_imx_sdma_init(struct device *dev, struct spi_imx_data *spi_imx,
 {
 	struct dma_slave_config slave_config = {};
 	int ret;
+	enum dma_slave_buswidth dsb_default = DMA_SLAVE_BUSWIDTH_1_BYTE;
 
 	/* use pio mode for i.mx6dl chip TKT238285 */
 	if (of_machine_is_compatible("fsl,imx6dl"))
@@ -837,10 +839,20 @@ static int spi_imx_sdma_init(struct device *dev, struct spi_imx_data *spi_imx,
 		ret = -EINVAL;
 		goto err;
 	}
-
+	switch (spi_imx->dma_bits_per_word) {
+	case 32:
+		dsb_default = DMA_SLAVE_BUSWIDTH_4_BYTES;
+		break;
+	case 16:
+		dsb_default = DMA_SLAVE_BUSWIDTH_2_BYTES;
+		break;
+	default:
+		dsb_default = DMA_SLAVE_BUSWIDTH_1_BYTE;
+		break;
+	}
 	slave_config.direction = DMA_MEM_TO_DEV;
 	slave_config.dst_addr = res->start + MXC_CSPITXDATA;
-	slave_config.dst_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
+	slave_config.dst_addr_width = dsb_default;
 	slave_config.dst_maxburst = spi_imx_get_fifosize(spi_imx) / 2;
 	ret = dmaengine_slave_config(master->dma_tx, &slave_config);
 	if (ret) {
@@ -858,7 +870,7 @@ static int spi_imx_sdma_init(struct device *dev, struct spi_imx_data *spi_imx,
 
 	slave_config.direction = DMA_DEV_TO_MEM;
 	slave_config.src_addr = res->start + MXC_CSPIRXDATA;
-	slave_config.src_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
+	slave_config.src_addr_width = dsb_default;
 	slave_config.src_maxburst = spi_imx_get_fifosize(spi_imx) / 2;
 	ret = dmaengine_slave_config(master->dma_rx, &slave_config);
 	if (ret) {
@@ -1083,7 +1095,7 @@ static int spi_imx_probe(struct platform_device *pdev)
 	struct spi_master *master;
 	struct spi_imx_data *spi_imx;
 	struct resource *res;
-	int i, ret, num_cs;
+	int i, ret, num_cs, dma_bits_per_word = 8;
 
 	if (!np && !mxc_platform_info) {
 		dev_err(&pdev->dev, "can't get the platform data\n");
@@ -1138,6 +1150,11 @@ static int spi_imx_probe(struct platform_device *pdev)
 	spi_imx->bitbang.master->prepare_message = spi_imx_prepare_message;
 	spi_imx->bitbang.master->unprepare_message = spi_imx_unprepare_message;
 	spi_imx->bitbang.master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH;
+	if (of_property_read_u32(np, "fsl,spi-dma-bits-per-word",
+			&dma_bits_per_word))
+		spi_imx->dma_bits_per_word = dma_bits_per_word;
+	else
+		spi_imx->dma_bits_per_word = 8;
 
 	init_completion(&spi_imx->xfer_done);
 
-- 
1.9.1

[PATCH 4/4] dt: fsl-imx-cspi.txt: Add fsl,spi-dma-bits-per-word

[Aurelien BOUIN]

This patch add the possibility to set the number of bits per word transfered by DMA

Signed-off-by: Aurelien BOUIN <a.bouin@gmail.com>

diff --git a/Documentation/devicetree/bindings/spi/fsl-imx-cspi.txt b/Documentation/devicetree/bindings/spi/fsl-imx-cspi.txt
index aad527b..76f82de 100644
--- a/Documentation/devicetree/bindings/spi/fsl-imx-cspi.txt
+++ b/Documentation/devicetree/bindings/spi/fsl-imx-cspi.txt
@@ -6,6 +6,7 @@ Required properties:
 - reg : Offset and length of the register set for the device
 - interrupts : Should contain CSPI/eCSPI interrupt
 - fsl,spi-num-chipselects : Contains the number of the chipselect
+- fsl,spi-dma-bits-per-word : Contains the number of bits per word transfered by DMA
 - cs-gpios : Specifies the gpio pins to be used for chipselects.
 - dmas: DMA specifiers for tx and rx dma. See the DMA client binding,
 		Documentation/devicetree/bindings/dma/dma.txt
@@ -20,6 +21,7 @@ ecspi@70010000 {
 	reg = <0x70010000 0x4000>;
 	interrupts = <36>;
 	fsl,spi-num-chipselects = <2>;
+	fsl,spi-dma-bits-per-word = <8>; /* 8 bits per word is the default value, can be 16 or 32 */
 	cs-gpios = <&gpio3 24 0>, /* GPIO3_24 */
 		   <&gpio3 25 0>; /* GPIO3_25 */
 	dmas = <&sdma 3 7 1>, <&sdma 4 7 2>;
-- 
1.9.1

Re: [PATCH 2/4] spi: imx: checkpatch cleanup

[Joe Perches]

On Thu, 2015-02-12 at 13:07 +0100, Aurelien BOUIN wrote:
> This patch solve various issue detected by checkpatch.pl

trivia:

> diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
[]
> @@ -282,7 +282,8 @@ static unsigned int mx51_ecspi_clkdiv(unsigned int fin, unsigned int fspi,
>  		(post << MX51_ECSPI_CTRL_POSTDIV_OFFSET);
>  }
>  
> -static void __maybe_unused mx51_ecspi_intctrl(struct spi_imx_data *spi_imx, int enable)
> +static void __maybe_unused mx51_ecspi_intctrl(struct spi_imx_data *spi_imx,
> +		int enable)

Perhaps it'd be better to use this style:

__maybe_unused
static void foo(...)

ie:

__maybe_unused
static void mx51_ecspi_intctrl(struct spi_imx_data *spi_imx, int enable)

etc..

Re: [PATCH 2/4] spi: imx: checkpatch cleanup

[aurélien bouin]

Hello,
It does not change readability ... Often breaks are in functions parameters ...
Up to you

2015-02-12 13:42 GMT+01:00 Joe Perches <joe@perches.com>:
>
> On Thu, 2015-02-12 at 13:07 +0100, Aurelien BOUIN wrote:
> > This patch solve various issue detected by checkpatch.pl
>
> trivia:
>
> > diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
> []
> > @@ -282,7 +282,8 @@ static unsigned int mx51_ecspi_clkdiv(unsigned int fin, unsigned int fspi,
> >               (post << MX51_ECSPI_CTRL_POSTDIV_OFFSET);
> >  }
> >
> > -static void __maybe_unused mx51_ecspi_intctrl(struct spi_imx_data *spi_imx, int enable)
> > +static void __maybe_unused mx51_ecspi_intctrl(struct spi_imx_data *spi_imx,
> > +             int enable)
>
> Perhaps it'd be better to use this style:
>
> __maybe_unused
> static void foo(...)
>
> ie:
>
> __maybe_unused
> static void mx51_ecspi_intctrl(struct spi_imx_data *spi_imx, int enable)
>
> etc..
>

Re: [PATCH 2/4] spi: imx: checkpatch cleanup

[Joe Perches]

On Thu, 2015-02-12 at 14:03 +0100, aurélien bouin wrote:
> Hello,

Rehi

> It does not change readability ... Often breaks are in functions parameters ...

[please don't top post, more below]

> > > diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
> > []
> > > @@ -282,7 +282,8 @@ static unsigned int mx51_ecspi_clkdiv(unsigned int fin, unsigned int fspi,
[]
> > > -static void __maybe_unused mx51_ecspi_intctrl(struct spi_imx_data *spi_imx, int enable)
> > > +static void __maybe_unused mx51_ecspi_intctrl(struct spi_imx_data *spi_imx,
> > > +             int enable)
> > Perhaps it'd be better to use this style:
> > __maybe_unused
> > static void foo(...)
> > ie:
> > __maybe_unused
> > static void mx51_ecspi_intctrl(struct spi_imx_data *spi_imx, int enable)

It's not the break in the function parameters that
matters much, it's the more difficult mechanism to
find the function itself.

typical is:

[static] [const] <type> <function>(params...)

inserting <attribute> between <type> and <function>
breaks that pattern and can make it harder for
less comprehensive code parsers to identify the
function names.

Re: [PATCH 2/4] spi: imx: checkpatch cleanup

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 314 bytes --]

On Thu, Feb 12, 2015 at 01:07:26PM +0100, Aurelien BOUIN wrote:
> This patch solve various issue detected by checkpatch.pl

I need a proper changelog for this, sorry - what issues and how are they
being fixed?  It's hard to review a random collection of changes with no
description of what they're supposed to do.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH 3/4] spi: imx: Chose DMA bits per word transfered

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 397 bytes --]

On Thu, Feb 12, 2015 at 01:07:27PM +0100, Aurelien BOUIN wrote:
> This patch enable to give a parameter to device tree to set the number of bits per word the DMA uses in transfer
> In device tree you can specify fsl,spi-dma-bits-per-word = <16>; in the ecspi section

Why would this be a DT property?  It seems like something the driver
ought to be able to figure out automatically if it matters.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH 3/4] spi: imx: Chose DMA bits per word transfered

[aurélien bouin]

At the moment there is no possibility to change the dma bits per word use ...
It is an easy way to be able to do so.
Feel free to propose something better

2015-02-21 15:35 GMT+01:00 Mark Brown <broonie@kernel.org>:
> On Thu, Feb 12, 2015 at 01:07:27PM +0100, Aurelien BOUIN wrote:
>> This patch enable to give a parameter to device tree to set the number of bits per word the DMA uses in transfer
>> In device tree you can specify fsl,spi-dma-bits-per-word = <16>; in the ecspi section
>
> Why would this be a DT property?  It seems like something the driver
> ought to be able to figure out automatically if it matters.

Re: [PATCH 2/4] spi: imx: checkpatch cleanup

[aurélien bouin]

I supposed that it was enough clear ...
Just use scripts/checkpatch.pl and you will figure out by yourself
what does the patch ...


2015-02-21 15:34 GMT+01:00 Mark Brown <broonie@kernel.org>:
> On Thu, Feb 12, 2015 at 01:07:26PM +0100, Aurelien BOUIN wrote:
>> This patch solve various issue detected by checkpatch.pl
>
> I need a proper changelog for this, sorry - what issues and how are they
> being fixed?  It's hard to review a random collection of changes with no
> description of what they're supposed to do.

Re: [PATCH 2/4] spi: imx: checkpatch cleanup

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 455 bytes --]

On Mon, Feb 23, 2015 at 09:45:50AM +0100, aurélien bouin wrote:
> I supposed that it was enough clear ...
> Just use scripts/checkpatch.pl and you will figure out by yourself
> what does the patch ...

Don't top post and sorry but this is just not good enough, running
checkpatch myself and figuring out how it ties into your changes is not
really a good tradeoff - it's getting to the point where it's quicker
and simpler to just redo the work.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH 3/4] spi: imx: Chose DMA bits per word transfered

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 461 bytes --]

On Mon, Feb 23, 2015 at 09:44:50AM +0100, aurélien bouin wrote:
> At the moment there is no possibility to change the dma bits per word use ...
> It is an easy way to be able to do so.
> Feel free to propose something better

Once more don't top post.  To repeat what I said:

| Why would this be a DT property?  It seems like something the driver
| ought to be able to figure out automatically if it matters.

The above doesn't address my question.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH 3/4] spi: imx: Chose DMA bits per word transfered

[aurélien bouin]

2015-02-23 15:46 GMT+01:00 Mark Brown <broonie@kernel.org>:
> On Mon, Feb 23, 2015 at 09:44:50AM +0100, aurélien bouin wrote:
>> At the moment there is no possibility to change the dma bits per word use ...
>> It is an easy way to be able to do so.
>> Feel free to propose something better
>
> Once more don't top post.  To repeat what I said:
>
> | Why would this be a DT property?  It seems like something the driver
> | ought to be able to figure out automatically if it matters.
>
> The above doesn't address my question.
Currently the driver does not figure it out automatically, it is a fix
8bits per word DMA use.

Re: [PATCH 3/4] spi: imx: Chose DMA bits per word transfered

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 724 bytes --]

On Mon, Feb 23, 2015 at 04:25:51PM +0100, aurélien bouin wrote:
> 2015-02-23 15:46 GMT+01:00 Mark Brown <broonie@kernel.org>:
> > On Mon, Feb 23, 2015 at 09:44:50AM +0100, aurélien bouin wrote:

> > Once more don't top post.  To repeat what I said:

> > | Why would this be a DT property?  It seems like something the driver
> > | ought to be able to figure out automatically if it matters.

> > The above doesn't address my question.

> Currently the driver does not figure it out automatically, it is a fix
> 8bits per word DMA use.

You're still not explaining why it's not a better fix for the driver to
figure this out for itself, or why it's not possible for it to do that
even if it would be better.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]