[PATCH 1/3] i2c: imx: silence dma_request_slave_channel errors if dma not set up in device tree

[PATCH 1/3] i2c: imx: silence dma_request_slave_channel errors if dma not set up in device tree

[Philipp Zabel]

If the i2c device tree nodes don't contain the dmas and dma-names property, an
error is displayed for each i2c device:

    of_dma_request_slave_channel: dma-names property of node '/soc/aips-bus@02100000/i2c@021a0000' missing or empty

This patch avoids calling dma_request_slave_channel if the dma-names property
does not exist in the device tree in the first place.

Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 drivers/i2c/busses/i2c-imx.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index 7f3a9fe..f0d9904 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -288,6 +288,9 @@ static void i2c_imx_dma_request(struct imx_i2c_struct *i2c_imx,
 	struct device *dev = &i2c_imx->adapter.dev;
 	int ret;
 
+	if (!of_get_property(dev->of_node, "dma-names", NULL))
+		return;
+
 	dma = devm_kzalloc(dev, sizeof(*dma), GFP_KERNEL);
 	if (!dma)
 		return;
-- 
2.1.4

[PATCH 2/3] i2c: imx: remove unused return value assignments

[Philipp Zabel]

The ret variable is set and never used in the error path of i2c_imx_dma_request.

Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 drivers/i2c/busses/i2c-imx.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index f0d9904..b5c9ce4 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -298,7 +298,6 @@ static void i2c_imx_dma_request(struct imx_i2c_struct *i2c_imx,
 	dma->chan_tx = dma_request_slave_channel(dev, "tx");
 	if (!dma->chan_tx) {
 		dev_dbg(dev, "can't request DMA tx channel\n");
-		ret = -ENODEV;
 		goto fail_al;
 	}
 
@@ -316,7 +315,6 @@ static void i2c_imx_dma_request(struct imx_i2c_struct *i2c_imx,
 	dma->chan_rx = dma_request_slave_channel(dev, "rx");
 	if (!dma->chan_rx) {
 		dev_dbg(dev, "can't request DMA rx channel\n");
-		ret = -ENODEV;
 		goto fail_tx;
 	}
 
-- 
2.1.4

[PATCH 3/3] i2c: imx: whitespace and checkpatch cleanup

[Philipp Zabel]

This patch fixes up some whitespace issues and addresses a few
checkpatch warnings.

Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 drivers/i2c/busses/i2c-imx.c | 53 ++++++++++++++++++++++++++------------------
 1 file changed, 32 insertions(+), 21 deletions(-)

diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index b5c9ce4..97bb104 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -201,7 +201,7 @@ struct imx_i2c_struct {
 	void __iomem		*base;
 	wait_queue_head_t	queue;
 	unsigned long		i2csr;
-	unsigned int 		disable_delay;
+	unsigned int		disable_delay;
 	int			stopped;
 	unsigned int		ifdr; /* IMX_I2C_IFDR */
 	unsigned int		cur_clk;
@@ -482,8 +482,8 @@ static void i2c_imx_set_clk(struct imx_i2c_struct *i2c_imx)
 	i2c_clk_rate = clk_get_rate(i2c_imx->clk);
 	if (i2c_imx->cur_clk == i2c_clk_rate)
 		return;
-	else
-		i2c_imx->cur_clk = i2c_clk_rate;
+
+	i2c_imx->cur_clk = i2c_clk_rate;
 
 	div = (i2c_clk_rate + i2c_imx->bitrate - 1) / i2c_imx->bitrate;
 	if (div < i2c_clk_div[0].div)
@@ -491,7 +491,8 @@ static void i2c_imx_set_clk(struct imx_i2c_struct *i2c_imx)
 	else if (div > i2c_clk_div[i2c_imx->hwdata->ndivs - 1].div)
 		i = i2c_imx->hwdata->ndivs - 1;
 	else
-		for (i = 0; i2c_clk_div[i].div < div; i++);
+		for (i = 0; i2c_clk_div[i].div < div; i++)
+			;
 
 	/* Store divider value */
 	i2c_imx->ifdr = i2c_clk_div[i].val;
@@ -527,8 +528,10 @@ static int i2c_imx_start(struct imx_i2c_struct *i2c_imx)
 		return result;
 	imx_i2c_write_reg(i2c_imx->ifdr, i2c_imx, IMX_I2C_IFDR);
 	/* Enable I2C controller */
-	imx_i2c_write_reg(i2c_imx->hwdata->i2sr_clr_opcode, i2c_imx, IMX_I2C_I2SR);
-	imx_i2c_write_reg(i2c_imx->hwdata->i2cr_ien_opcode, i2c_imx, IMX_I2C_I2CR);
+	imx_i2c_write_reg(i2c_imx->hwdata->i2sr_clr_opcode, i2c_imx,
+			  IMX_I2C_I2SR);
+	imx_i2c_write_reg(i2c_imx->hwdata->i2cr_ien_opcode, i2c_imx,
+			  IMX_I2C_I2CR);
 
 	/* Wait controller to be stable */
 	udelay(50);
@@ -781,7 +784,8 @@ static int i2c_imx_write(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs)
 	return 0;
 }
 
-static int i2c_imx_read(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs, bool is_lastmsg)
+static int i2c_imx_read(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs,
+			bool is_lastmsg)
 {
 	int i, result;
 	unsigned int temp;
@@ -823,6 +827,7 @@ static int i2c_imx_read(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs, bo
 	/* read data */
 	for (i = 0; i < msgs->len; i++) {
 		u8 len = 0;
+
 		result = i2c_imx_trx_complete(i2c_imx);
 		if (result)
 			return result;
@@ -843,8 +848,9 @@ static int i2c_imx_read(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs, bo
 		if (i == (msgs->len - 1)) {
 			if (is_lastmsg) {
 				/*
-				 * It must generate STOP before read I2DR to prevent
-				 * controller from generating another clock cycle
+				 * It must generate STOP before read I2DR to
+				 * prevent controller from generating another
+				 * clock cycle
 				 */
 				dev_dbg(&i2c_imx->adapter.dev,
 					"<%s> clear MSTA\n", __func__);
@@ -855,11 +861,13 @@ static int i2c_imx_read(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs, bo
 				i2c_imx->stopped = 1;
 			} else {
 				/*
-				 * For i2c master receiver repeat restart operation like:
+				 * For i2c master receiver repeat restart
+				 * operation like:
 				 * read -> repeat MSTA -> read/write
-				 * The controller must set MTX before read the last byte in
-				 * the first read operation, otherwise the first read cost
-				 * one extra clock cycle.
+				 * The controller must set MTX before read the
+				 * last byte in the first read operation,
+				 * otherwise the first read cost one extra clock
+				 * cycle.
 				 */
 				temp = readb(i2c_imx->base + IMX_I2C_I2CR);
 				temp |= I2CR_MTX;
@@ -918,15 +926,16 @@ static int i2c_imx_xfer(struct i2c_adapter *adapter,
 		/* write/read data */
 #ifdef CONFIG_I2C_DEBUG_BUS
 		temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
-		dev_dbg(&i2c_imx->adapter.dev, "<%s> CONTROL: IEN=%d, IIEN=%d, "
-			"MSTA=%d, MTX=%d, TXAK=%d, RSTA=%d\n", __func__,
+		dev_dbg(&i2c_imx->adapter.dev,
+			"<%s> CONTROL: IEN=%d, IIEN=%d, MSTA=%d, MTX=%d, TXAK=%d, RSTA=%d\n",
+			__func__,
 			(temp & I2CR_IEN ? 1 : 0), (temp & I2CR_IIEN ? 1 : 0),
 			(temp & I2CR_MSTA ? 1 : 0), (temp & I2CR_MTX ? 1 : 0),
 			(temp & I2CR_TXAK ? 1 : 0), (temp & I2CR_RSTA ? 1 : 0));
 		temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2SR);
 		dev_dbg(&i2c_imx->adapter.dev,
-			"<%s> STATUS: ICF=%d, IAAS=%d, IBB=%d, "
-			"IAL=%d, SRW=%d, IIF=%d, RXAK=%d\n", __func__,
+			"<%s> STATUS: ICF=%d, IAAS=%d, IBB=%d, IAL=%d, SRW=%d, IIF=%d, RXAK=%d\n",
+			__func__,
 			(temp & I2SR_ICF ? 1 : 0), (temp & I2SR_IAAS ? 1 : 0),
 			(temp & I2SR_IBB ? 1 : 0), (temp & I2SR_IAL ? 1 : 0),
 			(temp & I2SR_SRW ? 1 : 0), (temp & I2SR_IIF ? 1 : 0),
@@ -1001,11 +1010,12 @@ static int i2c_imx_probe(struct platform_device *pdev)
 				platform_get_device_id(pdev)->driver_data;
 
 	/* Setup i2c_imx driver structure */
-	strlcpy(i2c_imx->adapter.name, pdev->name, sizeof(i2c_imx->adapter.name));
+	strlcpy(i2c_imx->adapter.name, pdev->name,
+		sizeof(i2c_imx->adapter.name));
 	i2c_imx->adapter.owner		= THIS_MODULE;
 	i2c_imx->adapter.algo		= &i2c_imx_algo;
 	i2c_imx->adapter.dev.parent	= &pdev->dev;
-	i2c_imx->adapter.nr 		= pdev->id;
+	i2c_imx->adapter.nr		= pdev->id;
 	i2c_imx->adapter.dev.of_node	= pdev->dev.of_node;
 	i2c_imx->base			= base;
 
@@ -1045,7 +1055,8 @@ static int i2c_imx_probe(struct platform_device *pdev)
 	/* Set up chip registers to defaults */
 	imx_i2c_write_reg(i2c_imx->hwdata->i2cr_ien_opcode ^ I2CR_IEN,
 			i2c_imx, IMX_I2C_I2CR);
-	imx_i2c_write_reg(i2c_imx->hwdata->i2sr_clr_opcode, i2c_imx, IMX_I2C_I2SR);
+	imx_i2c_write_reg(i2c_imx->hwdata->i2sr_clr_opcode, i2c_imx,
+			  IMX_I2C_I2SR);
 
 	/* Add I2C adapter */
 	ret = i2c_add_numbered_adapter(&i2c_imx->adapter);
@@ -1064,7 +1075,7 @@ static int i2c_imx_probe(struct platform_device *pdev)
 		i2c_imx->adapter.name);
 	dev_info(&i2c_imx->adapter.dev, "IMX I2C adapter registered\n");
 
-	/* Init DMA config if support*/
+	/* Init DMA config if supported */
 	i2c_imx_dma_request(i2c_imx, phy_addr);
 
 	return 0;   /* Return OK */
-- 
2.1.4

RE: [PATCH 1/3] i2c: imx: silence dma_request_slave_channel errors if dma not set up in device tree

[fugang.duan-KZfg59tc24xl57MIdRCFDg]

From: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Sent: Tuesday, January 06, 2015 10:48 PM
> To: Wolfram Sang
> Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Duan Fugang-B38611; Yuan Yao-B46683;
> Estevam Fabio-R49496; kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org; Philipp Zabel
> Subject: [PATCH 1/3] i2c: imx: silence dma_request_slave_channel errors
> if dma not set up in device tree
> 
> If the i2c device tree nodes don't contain the dmas and dma-names
> property, an error is displayed for each i2c device:
> 
>     of_dma_request_slave_channel: dma-names property of node '/soc/aips-
> bus@02100000/i2c@021a0000' missing or empty
> 
> This patch avoids calling dma_request_slave_channel if the dma-names
> property does not exist in the device tree in the first place.
> 
> Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> ---
>  drivers/i2c/busses/i2c-imx.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
> index 7f3a9fe..f0d9904 100644
> --- a/drivers/i2c/busses/i2c-imx.c
> +++ b/drivers/i2c/busses/i2c-imx.c
> @@ -288,6 +288,9 @@ static void i2c_imx_dma_request(struct imx_i2c_struct
> *i2c_imx,
>  	struct device *dev = &i2c_imx->adapter.dev;
>  	int ret;
> 
> +	if (!of_get_property(dev->of_node, "dma-names", NULL))
> +		return;
> +
>  	dma = devm_kzalloc(dev, sizeof(*dma), GFP_KERNEL);
>  	if (!dma)
>  		return;
> --

Acked-by: Fugang Duan <B38611-KZfg59tc24xl57MIdRCFDg@public.gmane.org>

RE: [PATCH 3/3] i2c: imx: whitespace and checkpatch cleanup

[fugang.duan-KZfg59tc24xl57MIdRCFDg]

From: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Sent: Tuesday, January 06, 2015 10:48 PM
> To: Wolfram Sang
> Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Duan Fugang-B38611; Yuan Yao-B46683;
> Estevam Fabio-R49496; kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org; Philipp Zabel
> Subject: [PATCH 3/3] i2c: imx: whitespace and checkpatch cleanup
> 
> This patch fixes up some whitespace issues and addresses a few checkpatch
> warnings.
> 
> Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> ---
>  drivers/i2c/busses/i2c-imx.c | 53 ++++++++++++++++++++++++++------------
> ------
>  1 file changed, 32 insertions(+), 21 deletions(-)

[snip]
>  		if (i == (msgs->len - 1)) {
>  			if (is_lastmsg) {
>  				/*
> -				 * It must generate STOP before read I2DR to
> prevent
> -				 * controller from generating another clock cycle
> +				 * It must generate STOP before read I2DR to
> +				 * prevent controller from generating another
> +				 * clock cycle
>  				 */

It is better for the format:
	/* It must generate STOP before read I2DR to
	 * prevent controller from generating another
	 * clock cycle
	 */

Regards,
Andy

RE: [PATCH 2/3] i2c: imx: remove unused return value assignments

[fugang.duan-KZfg59tc24xl57MIdRCFDg]

From: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Sent: Tuesday, January 06, 2015 10:48 PM
> To: Wolfram Sang
> Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Duan Fugang-B38611; Yuan Yao-B46683;
> Estevam Fabio-R49496; kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org; Philipp Zabel
> Subject: [PATCH 2/3] i2c: imx: remove unused return value assignments
> 
> The ret variable is set and never used in the error path of
> i2c_imx_dma_request.
> 
> Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> ---
>  drivers/i2c/busses/i2c-imx.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
> index f0d9904..b5c9ce4 100644
> --- a/drivers/i2c/busses/i2c-imx.c
> +++ b/drivers/i2c/busses/i2c-imx.c
> @@ -298,7 +298,6 @@ static void i2c_imx_dma_request(struct imx_i2c_struct
> *i2c_imx,
>  	dma->chan_tx = dma_request_slave_channel(dev, "tx");
>  	if (!dma->chan_tx) {
>  		dev_dbg(dev, "can't request DMA tx channel\n");
> -		ret = -ENODEV;
>  		goto fail_al;
>  	}
> 
> @@ -316,7 +315,6 @@ static void i2c_imx_dma_request(struct imx_i2c_struct
> *i2c_imx,
>  	dma->chan_rx = dma_request_slave_channel(dev, "rx");
>  	if (!dma->chan_rx) {
>  		dev_dbg(dev, "can't request DMA rx channel\n");
> -		ret = -ENODEV;
>  		goto fail_tx;
>  	}
> 

Acked-by: Fugang Duan <B38611-KZfg59tc24xl57MIdRCFDg@public.gmane.org>

Regards,
Andy

Re: [PATCH 3/3] i2c: imx: whitespace and checkpatch cleanup

[Philipp Zabel]

Hi Andy,

Am Mittwoch, den 07.01.2015, 02:36 +0000 schrieb
fugang.duan-KZfg59tc24xl57MIdRCFDg@public.gmane.org:
> From: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> Sent: Tuesday, January 06, 2015 10:48 PM
> > To: Wolfram Sang
> > Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Duan Fugang-B38611; Yuan Yao-B46683;
> > Estevam Fabio-R49496; kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org; Philipp Zabel
> > Subject: [PATCH 3/3] i2c: imx: whitespace and checkpatch cleanup
> > 
> > This patch fixes up some whitespace issues and addresses a few checkpatch
> > warnings.
> > 
> > Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > ---
> >  drivers/i2c/busses/i2c-imx.c | 53 ++++++++++++++++++++++++++------------
> > ------
> >  1 file changed, 32 insertions(+), 21 deletions(-)
> 
> [snip]
> >  		if (i == (msgs->len - 1)) {
> >  			if (is_lastmsg) {
> >  				/*
> > -				 * It must generate STOP before read I2DR to
> > prevent
> > -				 * controller from generating another clock cycle
> > +				 * It must generate STOP before read I2DR to
> > +				 * prevent controller from generating another
> > +				 * clock cycle
> >  				 */
> 
> It is better for the format:
> 	/* It must generate STOP before read I2DR to

Thank you for the review, but according to Documentation/CodingStyle
this multi-line comment format is only preferred for code in net/ and
drivers/net.

regards
Philipp

RE: [PATCH 3/3] i2c: imx: whitespace and checkpatch cleanup

[fugang.duan-KZfg59tc24xl57MIdRCFDg]

From: Philipp Zabel <p.zabel@pengutronix.de>
Sent: Wednesday, January 07, 2015 7:02 PM
> To: Duan Fugang-B38611
> Cc: Wolfram Sang; Estevam Fabio-R49496; linux-i2c@vger.kernel.org;
> kernel@pengutronix.de; Yuan Yao-B46683
> Subject: Re: [PATCH 3/3] i2c: imx: whitespace and checkpatch cleanup
> 
> Hi Andy,
> 
> Am Mittwoch, den 07.01.2015, 02:36 +0000 schrieb
> fugang.duan@freescale.com:
> > From: Philipp Zabel <p.zabel@pengutronix.de>
> > Sent: Tuesday, January 06, 2015 10:48 PM
> > > To: Wolfram Sang
> > > Cc: linux-i2c@vger.kernel.org; Duan Fugang-B38611; Yuan Yao-B46683;
> > > Estevam Fabio-R49496; kernel@pengutronix.de; Philipp Zabel
> > > Subject: [PATCH 3/3] i2c: imx: whitespace and checkpatch cleanup
> > >
> > > This patch fixes up some whitespace issues and addresses a few
> > > checkpatch warnings.
> > >
> > > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> > > ---
> > >  drivers/i2c/busses/i2c-imx.c | 53
> > > ++++++++++++++++++++++++++------------
> > > ------
> > >  1 file changed, 32 insertions(+), 21 deletions(-)
> >
> > [snip]
> > >  		if (i == (msgs->len - 1)) {
> > >  			if (is_lastmsg) {
> > >  				/*
> > > -				 * It must generate STOP before read I2DR to
> > > prevent
> > > -				 * controller from generating another clock cycle
> > > +				 * It must generate STOP before read I2DR to
> > > +				 * prevent controller from generating another
> > > +				 * clock cycle
> > >  				 */
> >
> > It is better for the format:
> > 	/* It must generate STOP before read I2DR to
> 
> Thank you for the review, but according to Documentation/CodingStyle this
> multi-line comment format is only preferred for code in net/ and
> drivers/net.
> 
> regards
> Philipp

I agree. So I am fine for the patch. Thanks for your code clean.

Acked-by: Fugang Duan <B38611@freescale.com>

Regards,
Andy

Re: [PATCH 1/3] i2c: imx: silence dma_request_slave_channel errors if dma not set up in device tree

[Wolfram Sang]

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

On Tue, Jan 06, 2015 at 03:48:19PM +0100, Philipp Zabel wrote:
> If the i2c device tree nodes don't contain the dmas and dma-names property, an
> error is displayed for each i2c device:
> 
>     of_dma_request_slave_channel: dma-names property of node '/soc/aips-bus@02100000/i2c@021a0000' missing or empty
> 
> This patch avoids calling dma_request_slave_channel if the dma-names property
> does not exist in the device tree in the first place.
> 
> Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

Aren't you curing the symptoms instead of the cause? Sending a
counterpatch in a second...

> ---
>  drivers/i2c/busses/i2c-imx.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
> index 7f3a9fe..f0d9904 100644
> --- a/drivers/i2c/busses/i2c-imx.c
> +++ b/drivers/i2c/busses/i2c-imx.c
> @@ -288,6 +288,9 @@ static void i2c_imx_dma_request(struct imx_i2c_struct *i2c_imx,
>  	struct device *dev = &i2c_imx->adapter.dev;
>  	int ret;
>  
> +	if (!of_get_property(dev->of_node, "dma-names", NULL))
> +		return;
> +
>  	dma = devm_kzalloc(dev, sizeof(*dma), GFP_KERNEL);
>  	if (!dma)
>  		return;
> -- 
> 2.1.4
> 

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

Re: [PATCH 2/3] i2c: imx: remove unused return value assignments

[Wolfram Sang]

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

On Tue, Jan 06, 2015 at 03:48:20PM +0100, Philipp Zabel wrote:
> The ret variable is set and never used in the error path of i2c_imx_dma_request.
> 
> Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

Applied to for-next, thanks!


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

Re: [PATCH 3/3] i2c: imx: whitespace and checkpatch cleanup

[Wolfram Sang]

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

On Tue, Jan 06, 2015 at 03:48:21PM +0100, Philipp Zabel wrote:
> This patch fixes up some whitespace issues and addresses a few
> checkpatch warnings.

Well, since you asked for it... I am not so strict with the 80 char
limit:

> @@ -527,8 +528,10 @@ static int i2c_imx_start(struct imx_i2c_struct *i2c_imx)
>  		return result;
>  	imx_i2c_write_reg(i2c_imx->ifdr, i2c_imx, IMX_I2C_IFDR);
>  	/* Enable I2C controller */
> -	imx_i2c_write_reg(i2c_imx->hwdata->i2sr_clr_opcode, i2c_imx, IMX_I2C_I2SR);
> -	imx_i2c_write_reg(i2c_imx->hwdata->i2cr_ien_opcode, i2c_imx, IMX_I2C_I2CR);
> +	imx_i2c_write_reg(i2c_imx->hwdata->i2sr_clr_opcode, i2c_imx,
> +			  IMX_I2C_I2SR);
> +	imx_i2c_write_reg(i2c_imx->hwdata->i2cr_ien_opcode, i2c_imx,
> +			  IMX_I2C_I2CR);

Please keep the old way, this is not more readable.

> @@ -781,7 +784,8 @@ static int i2c_imx_write(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs)
>  	return 0;
>  }
>  
> -static int i2c_imx_read(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs, bool is_lastmsg)
> +static int i2c_imx_read(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs,
> +			bool is_lastmsg)

ditto

> @@ -843,8 +848,9 @@ static int i2c_imx_read(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs, bo
>  		if (i == (msgs->len - 1)) {
>  			if (is_lastmsg) {
>  				/*
> -				 * It must generate STOP before read I2DR to prevent
> -				 * controller from generating another clock cycle
> +				 * It must generate STOP before read I2DR to
> +				 * prevent controller from generating another
> +				 * clock cycle

ditto

> @@ -855,11 +861,13 @@ static int i2c_imx_read(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs, bo
>  				i2c_imx->stopped = 1;
>  			} else {
>  				/*
> -				 * For i2c master receiver repeat restart operation like:
> +				 * For i2c master receiver repeat restart
> +				 * operation like:
>  				 * read -> repeat MSTA -> read/write
> -				 * The controller must set MTX before read the last byte in
> -				 * the first read operation, otherwise the first read cost
> -				 * one extra clock cycle.
> +				 * The controller must set MTX before read the
> +				 * last byte in the first read operation,
> +				 * otherwise the first read cost one extra clock
> +				 * cycle.

ditto

> @@ -918,15 +926,16 @@ static int i2c_imx_xfer(struct i2c_adapter *adapter,
>  		/* write/read data */
>  #ifdef CONFIG_I2C_DEBUG_BUS
>  		temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
> -		dev_dbg(&i2c_imx->adapter.dev, "<%s> CONTROL: IEN=%d, IIEN=%d, "
> -			"MSTA=%d, MTX=%d, TXAK=%d, RSTA=%d\n", __func__,
> +		dev_dbg(&i2c_imx->adapter.dev,
> +			"<%s> CONTROL: IEN=%d, IIEN=%d, MSTA=%d, MTX=%d, TXAK=%d, RSTA=%d\n",
> +			__func__,

This is better than before, good.

>  			(temp & I2CR_IEN ? 1 : 0), (temp & I2CR_IIEN ? 1 : 0),
>  			(temp & I2CR_MSTA ? 1 : 0), (temp & I2CR_MTX ? 1 : 0),
>  			(temp & I2CR_TXAK ? 1 : 0), (temp & I2CR_RSTA ? 1 : 0));
>  		temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2SR);
>  		dev_dbg(&i2c_imx->adapter.dev,
> -			"<%s> STATUS: ICF=%d, IAAS=%d, IBB=%d, "
> -			"IAL=%d, SRW=%d, IIF=%d, RXAK=%d\n", __func__,
> +			"<%s> STATUS: ICF=%d, IAAS=%d, IBB=%d, IAL=%d, SRW=%d, IIF=%d, RXAK=%d\n",
> +			__func__,

ditto

> @@ -1001,11 +1010,12 @@ static int i2c_imx_probe(struct platform_device *pdev)
>  				platform_get_device_id(pdev)->driver_data;
>  
>  	/* Setup i2c_imx driver structure */
> -	strlcpy(i2c_imx->adapter.name, pdev->name, sizeof(i2c_imx->adapter.name));
> +	strlcpy(i2c_imx->adapter.name, pdev->name,
> +		sizeof(i2c_imx->adapter.name));

This not IMO.

> @@ -1045,7 +1055,8 @@ static int i2c_imx_probe(struct platform_device *pdev)
>  	/* Set up chip registers to defaults */
>  	imx_i2c_write_reg(i2c_imx->hwdata->i2cr_ien_opcode ^ I2CR_IEN,
>  			i2c_imx, IMX_I2C_I2CR);
> -	imx_i2c_write_reg(i2c_imx->hwdata->i2sr_clr_opcode, i2c_imx, IMX_I2C_I2SR);
> +	imx_i2c_write_reg(i2c_imx->hwdata->i2sr_clr_opcode, i2c_imx,
> +			  IMX_I2C_I2SR);

ditto

Rest is appreciated, thanks.


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

Re: [PATCH 3/3] i2c: imx: whitespace and checkpatch cleanup

[Philipp Zabel]

Am Mittwoch, den 14.01.2015, 15:25 +0100 schrieb Wolfram Sang:
> On Tue, Jan 06, 2015 at 03:48:21PM +0100, Philipp Zabel wrote:
> > This patch fixes up some whitespace issues and addresses a few
> > checkpatch warnings.
> 
> Well, since you asked for it... I am not so strict with the 80 char
> limit:

Noted. I'll drop the line width changes you pointed out and resend.

thanks
Philipp

Re: [PATCH 1/3] i2c: imx: silence dma_request_slave_channel errors if dma not set up in device tree

[Wolfram Sang]

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

On Wed, Jan 14, 2015 at 03:14:07PM +0100, Wolfram Sang wrote:
> On Tue, Jan 06, 2015 at 03:48:19PM +0100, Philipp Zabel wrote:
> > If the i2c device tree nodes don't contain the dmas and dma-names property, an
> > error is displayed for each i2c device:
> > 
> >     of_dma_request_slave_channel: dma-names property of node '/soc/aips-bus@02100000/i2c@021a0000' missing or empty
> > 
> > This patch avoids calling dma_request_slave_channel if the dma-names property
> > does not exist in the device tree in the first place.
> > 
> > Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> 
> Aren't you curing the symptoms instead of the cause? Sending a
> counterpatch in a second...

Setting to superseded because of my counterpatch fixing this in the
dmaengine core. Maybe you want to ack that? :)


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

Re: [PATCH 1/3] i2c: imx: silence dma_request_slave_channel errors if dma not set up in device tree

[Philipp Zabel]

Am Donnerstag, den 22.01.2015, 15:33 +0100 schrieb Wolfram Sang:
> On Wed, Jan 14, 2015 at 03:14:07PM +0100, Wolfram Sang wrote:
> > On Tue, Jan 06, 2015 at 03:48:19PM +0100, Philipp Zabel wrote:
> > > If the i2c device tree nodes don't contain the dmas and dma-names property, an
> > > error is displayed for each i2c device:
> > > 
> > >     of_dma_request_slave_channel: dma-names property of node '/soc/aips-bus@02100000/i2c@021a0000' missing or empty
> > > 
> > > This patch avoids calling dma_request_slave_channel if the dma-names property
> > > does not exist in the device tree in the first place.
> > > 
> > > Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > 
> > Aren't you curing the symptoms instead of the cause? Sending a
> > counterpatch in a second...
> 
> Setting to superseded because of my counterpatch fixing this in the
> dmaengine core. Maybe you want to ack that? :)

Absolutely!