| Message ID | 1420555701-24645-3-git-send-email-p.zabel@pengutronix.de |
|---|---|
| State | Changes Requested |
| Headers | show |
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 * prevent controller from generating another * clock cycle */ Regards, Andy -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RnJvbTogUGhpbGlwcCBaYWJlbCA8cC56YWJlbEBwZW5ndXRyb25peC5kZT4NClNlbnQ6IFdlZG5l c2RheSwgSmFudWFyeSAwNywgMjAxNSA3OjAyIFBNDQo+IFRvOiBEdWFuIEZ1Z2FuZy1CMzg2MTEN Cj4gQ2M6IFdvbGZyYW0gU2FuZzsgRXN0ZXZhbSBGYWJpby1SNDk0OTY7IGxpbnV4LWkyY0B2Z2Vy Lmtlcm5lbC5vcmc7DQo+IGtlcm5lbEBwZW5ndXRyb25peC5kZTsgWXVhbiBZYW8tQjQ2NjgzDQo+ IFN1YmplY3Q6IFJlOiBbUEFUQ0ggMy8zXSBpMmM6IGlteDogd2hpdGVzcGFjZSBhbmQgY2hlY2tw YXRjaCBjbGVhbnVwDQo+IA0KPiBIaSBBbmR5LA0KPiANCj4gQW0gTWl0dHdvY2gsIGRlbiAwNy4w MS4yMDE1LCAwMjozNiArMDAwMCBzY2hyaWViDQo+IGZ1Z2FuZy5kdWFuQGZyZWVzY2FsZS5jb206 DQo+ID4gRnJvbTogUGhpbGlwcCBaYWJlbCA8cC56YWJlbEBwZW5ndXRyb25peC5kZT4NCj4gPiBT ZW50OiBUdWVzZGF5LCBKYW51YXJ5IDA2LCAyMDE1IDEwOjQ4IFBNDQo+ID4gPiBUbzogV29sZnJh bSBTYW5nDQo+ID4gPiBDYzogbGludXgtaTJjQHZnZXIua2VybmVsLm9yZzsgRHVhbiBGdWdhbmct QjM4NjExOyBZdWFuIFlhby1CNDY2ODM7DQo+ID4gPiBFc3RldmFtIEZhYmlvLVI0OTQ5Njsga2Vy bmVsQHBlbmd1dHJvbml4LmRlOyBQaGlsaXBwIFphYmVsDQo+ID4gPiBTdWJqZWN0OiBbUEFUQ0gg My8zXSBpMmM6IGlteDogd2hpdGVzcGFjZSBhbmQgY2hlY2twYXRjaCBjbGVhbnVwDQo+ID4gPg0K PiA+ID4gVGhpcyBwYXRjaCBmaXhlcyB1cCBzb21lIHdoaXRlc3BhY2UgaXNzdWVzIGFuZCBhZGRy ZXNzZXMgYSBmZXcNCj4gPiA+IGNoZWNrcGF0Y2ggd2FybmluZ3MuDQo+ID4gPg0KPiA+ID4gU2ln bmVkLW9mZi1ieTogUGhpbGlwcCBaYWJlbCA8cC56YWJlbEBwZW5ndXRyb25peC5kZT4NCj4gPiA+ IC0tLQ0KPiA+ID4gIGRyaXZlcnMvaTJjL2J1c3Nlcy9pMmMtaW14LmMgfCA1Mw0KPiA+ID4gKysr KysrKysrKysrKysrKysrKysrKysrKystLS0tLS0tLS0tLS0NCj4gPiA+IC0tLS0tLQ0KPiA+ID4g IDEgZmlsZSBjaGFuZ2VkLCAzMiBpbnNlcnRpb25zKCspLCAyMSBkZWxldGlvbnMoLSkNCj4gPg0K PiA+IFtzbmlwXQ0KPiA+ID4gIAkJaWYgKGkgPT0gKG1zZ3MtPmxlbiAtIDEpKSB7DQo+ID4gPiAg CQkJaWYgKGlzX2xhc3Rtc2cpIHsNCj4gPiA+ICAJCQkJLyoNCj4gPiA+IC0JCQkJICogSXQgbXVz dCBnZW5lcmF0ZSBTVE9QIGJlZm9yZSByZWFkIEkyRFIgdG8NCj4gPiA+IHByZXZlbnQNCj4gPiA+ IC0JCQkJICogY29udHJvbGxlciBmcm9tIGdlbmVyYXRpbmcgYW5vdGhlciBjbG9jayBjeWNsZQ0K PiA+ID4gKwkJCQkgKiBJdCBtdXN0IGdlbmVyYXRlIFNUT1AgYmVmb3JlIHJlYWQgSTJEUiB0bw0K PiA+ID4gKwkJCQkgKiBwcmV2ZW50IGNvbnRyb2xsZXIgZnJvbSBnZW5lcmF0aW5nIGFub3RoZXIN Cj4gPiA+ICsJCQkJICogY2xvY2sgY3ljbGUNCj4gPiA+ICAJCQkJICovDQo+ID4NCj4gPiBJdCBp cyBiZXR0ZXIgZm9yIHRoZSBmb3JtYXQ6DQo+ID4gCS8qIEl0IG11c3QgZ2VuZXJhdGUgU1RPUCBi ZWZvcmUgcmVhZCBJMkRSIHRvDQo+IA0KPiBUaGFuayB5b3UgZm9yIHRoZSByZXZpZXcsIGJ1dCBh Y2NvcmRpbmcgdG8gRG9jdW1lbnRhdGlvbi9Db2RpbmdTdHlsZSB0aGlzDQo+IG11bHRpLWxpbmUg Y29tbWVudCBmb3JtYXQgaXMgb25seSBwcmVmZXJyZWQgZm9yIGNvZGUgaW4gbmV0LyBhbmQNCj4g ZHJpdmVycy9uZXQuDQo+IA0KPiByZWdhcmRzDQo+IFBoaWxpcHANCg0KSSBhZ3JlZS4gU28gSSBh bSBmaW5lIGZvciB0aGUgcGF0Y2guIFRoYW5rcyBmb3IgeW91ciBjb2RlIGNsZWFuLg0KDQpBY2tl ZC1ieTogRnVnYW5nIER1YW4gPEIzODYxMUBmcmVlc2NhbGUuY29tPg0KDQpSZWdhcmRzLA0KQW5k eQ0K -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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 -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 */
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(-)