drivers/i2c/algos/i2c-algo-pcf.c | 50 +++++++++++++++++++++----------- 1 file changed, 33 insertions(+), 17 deletions(-)
Fixed all 18 errors revealed using checkpatch.pl on i2c-algo-pcf.c
file. Errors fixed were: macros starting with 'if' should be
enclosed by do - while loop to avoid possible if/else logic defects,
do not use assignment in if condition, spaces required around '=' ,
';', '<' and ','.
Motivation is to fix all errors and warnings i2c-algo-pcf kerenel
module.
Testing:
* built kernel with my changes and I2C_ALGOPCF=m enabled
and it built successfully.
* installed kernel and external modules generated by build
* rebooted and loaded using modprobe i2c-algo-pcf kernel module
with param i2c_debug=3 and no message was found related to
module in dmesg. But also no error was generated.
Errors on patch: on running checkpatch.pl on this patch 4 warnings
were raised. Will be fixed on following warnings fixes patch.
Signed-off-by: Cezar Chiru <chiru.cezar.89@gmail.com>
---
drivers/i2c/algos/i2c-algo-pcf.c | 50 +++++++++++++++++++++-----------
1 file changed, 33 insertions(+), 17 deletions(-)
diff --git a/drivers/i2c/algos/i2c-algo-pcf.c b/drivers/i2c/algos/i2c-algo-pcf.c
index fd563e845d4b..18ba21ff8992 100644
--- a/drivers/i2c/algos/i2c-algo-pcf.c
+++ b/drivers/i2c/algos/i2c-algo-pcf.c
@@ -23,9 +23,18 @@
#include "i2c-algo-pcf.h"
-#define DEB2(x) if (i2c_debug >= 2) x
-#define DEB3(x) if (i2c_debug >= 3) x /* print several statistical values */
-#define DEBPROTO(x) if (i2c_debug >= 9) x;
+#define DEB2(x) do { \
+ if (i2c_debug >= 2) \
+ x; \
+ } while (0)
+#define DEB3(x) do { \
+ if (i2c_debug >= 3) \
+ x; \ /* print several statistical values */
+ } while (0)
+#define DEBPROTO(x) do { \
+ if (i2c_debug >= 9) \
+ x; \
+ } while (0)
/* debug the protocol by showing transferred bits */
#define DEF_TIMEOUT 16
@@ -160,7 +169,8 @@ static int pcf_init_8584 (struct i2c_algo_pcf_data *adap)
* check to see S1 now used as R/W ctrl -
* PCF8584 does that when ESO is zero
*/
- if (((temp = get_pcf(adap, 1)) & 0x7f) != (0)) {
+ temp = get_pcf(adap, 1);
+ if ((temp & 0x7f) != (0)) {
DEB2(printk(KERN_ERR "i2c-algo-pcf.o: PCF detection failed -- can't select S0 (0x%02x).\n", temp));
return -ENXIO; /* definitely not PCF8584 */
}
@@ -168,7 +178,8 @@ static int pcf_init_8584 (struct i2c_algo_pcf_data *adap)
/* load own address in S0, effective address is (own << 1) */
i2c_outb(adap, get_own(adap));
/* check it's really written */
- if ((temp = i2c_inb(adap)) != get_own(adap)) {
+ temp = i2c_inb(adap);
+ if (temp != get_own(adap)) {
DEB2(printk(KERN_ERR "i2c-algo-pcf.o: PCF detection failed -- can't set S0 (0x%02x).\n", temp));
return -ENXIO;
}
@@ -176,7 +187,8 @@ static int pcf_init_8584 (struct i2c_algo_pcf_data *adap)
/* S1=0xA0, next byte in S2 */
set_pcf(adap, 1, I2C_PCF_PIN | I2C_PCF_ES1);
/* check to see S2 now selected */
- if (((temp = get_pcf(adap, 1)) & 0x7f) != I2C_PCF_ES1) {
+ temp = get_pcf(adap, 1);
+ if ((temp & 0x7f) != I2C_PCF_ES1) {
DEB2(printk(KERN_ERR "i2c-algo-pcf.o: PCF detection failed -- can't select S2 (0x%02x).\n", temp));
return -ENXIO;
}
@@ -184,7 +196,8 @@ static int pcf_init_8584 (struct i2c_algo_pcf_data *adap)
/* load clock register S2 */
i2c_outb(adap, get_clock(adap));
/* check it's really written, the only 5 lowest bits does matter */
- if (((temp = i2c_inb(adap)) & 0x1f) != get_clock(adap)) {
+ temp = i2c_inb(adap);
+ if ((temp & 0x1f) != get_clock(adap)) {
DEB2(printk(KERN_ERR "i2c-algo-pcf.o: PCF detection failed -- can't set S2 (0x%02x).\n", temp));
return -ENXIO;
}
@@ -193,7 +206,8 @@ static int pcf_init_8584 (struct i2c_algo_pcf_data *adap)
set_pcf(adap, 1, I2C_PCF_IDLE);
/* check to see PCF is really idled and we can access status register */
- if ((temp = get_pcf(adap, 1)) != (I2C_PCF_PIN | I2C_PCF_BB)) {
+ temp = get_pcf(adap, 1);
+ if (temp != (I2C_PCF_PIN | I2C_PCF_BB)) {
DEB2(printk(KERN_ERR "i2c-algo-pcf.o: PCF detection failed -- can't select S1` (0x%02x).\n", temp));
return -ENXIO;
}
@@ -209,7 +223,7 @@ static int pcf_sendbytes(struct i2c_adapter *i2c_adap, const char *buf,
struct i2c_algo_pcf_data *adap = i2c_adap->algo_data;
int wrcount, status, timeout;
- for (wrcount=0; wrcount<count; ++wrcount) {
+ for (wrcount = 0; wrcount < count; ++wrcount) {
DEB2(dev_dbg(&i2c_adap->dev, "i2c_write: writing %2.2X\n",
buf[wrcount] & 0xff));
i2c_outb(adap, buf[wrcount]);
@@ -246,7 +260,8 @@ static int pcf_readbytes(struct i2c_adapter *i2c_adap, char *buf,
/* increment number of bytes to read by one -- read dummy byte */
for (i = 0; i <= count; i++) {
- if ((wfp = wait_for_pin(adap, &status))) {
+ wfp = wait_for_pin(adap, &status);
+ if (wfp) {
if (wfp == -EINTR)
return -EINTR; /* arbitration lost */
@@ -299,7 +314,7 @@ static int pcf_xfer(struct i2c_adapter *i2c_adap,
struct i2c_algo_pcf_data *adap = i2c_adap->algo_data;
struct i2c_msg *pmsg;
int i;
- int ret=0, timeout, status;
+ int ret = 0, timeout, status;
if (adap->xfer_begin)
adap->xfer_begin(adap->data);
@@ -313,7 +328,7 @@ static int pcf_xfer(struct i2c_adapter *i2c_adap,
goto out;
}
- for (i = 0;ret >= 0 && i < num; i++) {
+ for (i = 0; ret >= 0 && i < num; i++) {
pmsg = &msgs[i];
DEB2(printk(KERN_DEBUG "i2c-algo-pcf.o: Doing %s %d bytes to 0x%02x - %d of %d messages\n",
@@ -358,9 +373,9 @@ static int pcf_xfer(struct i2c_adapter *i2c_adap,
if (ret != pmsg->len) {
DEB2(printk(KERN_DEBUG "i2c-algo-pcf.o: fail: "
- "only read %d bytes.\n",ret));
+ "only read %d bytes.\n", ret));
} else {
- DEB2(printk(KERN_DEBUG "i2c-algo-pcf.o: read %d bytes.\n",ret));
+ DEB2(printk(KERN_DEBUG "i2c-algo-pcf.o: read %d bytes.\n", ret));
}
} else {
ret = pcf_sendbytes(i2c_adap, pmsg->buf, pmsg->len,
@@ -368,9 +383,9 @@ static int pcf_xfer(struct i2c_adapter *i2c_adap,
if (ret != pmsg->len) {
DEB2(printk(KERN_DEBUG "i2c-algo-pcf.o: fail: "
- "only wrote %d bytes.\n",ret));
+ "only wrote %d bytes.\n", ret));
} else {
- DEB2(printk(KERN_DEBUG "i2c-algo-pcf.o: wrote %d bytes.\n",ret));
+ DEB2(printk(KERN_DEBUG "i2c-algo-pcf.o: wrote %d bytes.\n", ret));
}
}
}
@@ -406,7 +421,8 @@ int i2c_pcf_add_bus(struct i2c_adapter *adap)
/* register new adapter to i2c module... */
adap->algo = &pcf_algo;
- if ((rval = pcf_init_8584(pcf_adap)))
+ rval = pcf_init_8584(pcf_adap);
+ if (rval)
return rval;
rval = i2c_add_adapter(adap);
--
2.43.0Fixed all 18 errors revealed using checkpatch.pl on i2c-algo-pcf.c
file. Errors fixed were: macros starting with 'if' should be
enclosed by do - while loop to avoid possible if/else logic defects,
do not use assignment in if condition, spaces required around '=' ,
';', '<' and ','.
Motivation is to fix all errors and warnings i2c-algo-pcf kerenel
module.
Testing:
* built kernel with my changes and I2C_ALGOPCF=m enabled
and it built successfully.
* installed kernel and external modules generated by build
* rebooted and loaded using modprobe i2c-algo-pcf kernel module
with param i2c_debug=3 and no message was found related to
module in dmesg. But also no error was generated.
Checkpatch.pl warnings on patch: on running checkpatch.pl on this
patch 7 warnings were raised. Will be fixed on following warnings
fixes patch.
v2:
Fixed build errors generated by missing ; after do - while.
Missed to git add latest changes to patch. Build is ok.
Signed-off-by: Cezar Chiru <chiru.cezar.89@gmail.com>
---
drivers/i2c/algos/i2c-algo-pcf.c | 42 +++++++++++++++++++-------------
1 file changed, 25 insertions(+), 17 deletions(-)
diff --git a/drivers/i2c/algos/i2c-algo-pcf.c b/drivers/i2c/algos/i2c-algo-pcf.c
index fd563e845d4b..f5174f38d777 100644
--- a/drivers/i2c/algos/i2c-algo-pcf.c
+++ b/drivers/i2c/algos/i2c-algo-pcf.c
@@ -23,9 +23,10 @@
#include "i2c-algo-pcf.h"
-#define DEB2(x) if (i2c_debug >= 2) x
-#define DEB3(x) if (i2c_debug >= 3) x /* print several statistical values */
-#define DEBPROTO(x) if (i2c_debug >= 9) x;
+#define DEB2(x) do { if (i2c_debug >= 2) x; } while (0);
+#define DEB3(x) do { if (i2c_debug >= 3) x; } while (0);
+ /* print several statistical values */
+#define DEBPROTO(x) do { if (i2c_debug >= 9) x; } while (0);
/* debug the protocol by showing transferred bits */
#define DEF_TIMEOUT 16
@@ -160,7 +161,8 @@ static int pcf_init_8584 (struct i2c_algo_pcf_data *adap)
* check to see S1 now used as R/W ctrl -
* PCF8584 does that when ESO is zero
*/
- if (((temp = get_pcf(adap, 1)) & 0x7f) != (0)) {
+ temp = get_pcf(adap, 1);
+ if ((temp & 0x7f) != (0)) {
DEB2(printk(KERN_ERR "i2c-algo-pcf.o: PCF detection failed -- can't select S0 (0x%02x).\n", temp));
return -ENXIO; /* definitely not PCF8584 */
}
@@ -168,7 +170,8 @@ static int pcf_init_8584 (struct i2c_algo_pcf_data *adap)
/* load own address in S0, effective address is (own << 1) */
i2c_outb(adap, get_own(adap));
/* check it's really written */
- if ((temp = i2c_inb(adap)) != get_own(adap)) {
+ temp = i2c_inb(adap);
+ if (temp != get_own(adap)) {
DEB2(printk(KERN_ERR "i2c-algo-pcf.o: PCF detection failed -- can't set S0 (0x%02x).\n", temp));
return -ENXIO;
}
@@ -176,7 +179,8 @@ static int pcf_init_8584 (struct i2c_algo_pcf_data *adap)
/* S1=0xA0, next byte in S2 */
set_pcf(adap, 1, I2C_PCF_PIN | I2C_PCF_ES1);
/* check to see S2 now selected */
- if (((temp = get_pcf(adap, 1)) & 0x7f) != I2C_PCF_ES1) {
+ temp = get_pcf(adap, 1);
+ if ((temp & 0x7f) != I2C_PCF_ES1) {
DEB2(printk(KERN_ERR "i2c-algo-pcf.o: PCF detection failed -- can't select S2 (0x%02x).\n", temp));
return -ENXIO;
}
@@ -184,7 +188,8 @@ static int pcf_init_8584 (struct i2c_algo_pcf_data *adap)
/* load clock register S2 */
i2c_outb(adap, get_clock(adap));
/* check it's really written, the only 5 lowest bits does matter */
- if (((temp = i2c_inb(adap)) & 0x1f) != get_clock(adap)) {
+ temp = i2c_inb(adap);
+ if ((temp & 0x1f) != get_clock(adap)) {
DEB2(printk(KERN_ERR "i2c-algo-pcf.o: PCF detection failed -- can't set S2 (0x%02x).\n", temp));
return -ENXIO;
}
@@ -193,7 +198,8 @@ static int pcf_init_8584 (struct i2c_algo_pcf_data *adap)
set_pcf(adap, 1, I2C_PCF_IDLE);
/* check to see PCF is really idled and we can access status register */
- if ((temp = get_pcf(adap, 1)) != (I2C_PCF_PIN | I2C_PCF_BB)) {
+ temp = get_pcf(adap, 1);
+ if (temp != (I2C_PCF_PIN | I2C_PCF_BB)) {
DEB2(printk(KERN_ERR "i2c-algo-pcf.o: PCF detection failed -- can't select S1` (0x%02x).\n", temp));
return -ENXIO;
}
@@ -209,7 +215,7 @@ static int pcf_sendbytes(struct i2c_adapter *i2c_adap, const char *buf,
struct i2c_algo_pcf_data *adap = i2c_adap->algo_data;
int wrcount, status, timeout;
- for (wrcount=0; wrcount<count; ++wrcount) {
+ for (wrcount = 0; wrcount < count; ++wrcount) {
DEB2(dev_dbg(&i2c_adap->dev, "i2c_write: writing %2.2X\n",
buf[wrcount] & 0xff));
i2c_outb(adap, buf[wrcount]);
@@ -246,7 +252,8 @@ static int pcf_readbytes(struct i2c_adapter *i2c_adap, char *buf,
/* increment number of bytes to read by one -- read dummy byte */
for (i = 0; i <= count; i++) {
- if ((wfp = wait_for_pin(adap, &status))) {
+ wfp = wait_for_pin(adap, &status);
+ if (wfp) {
if (wfp == -EINTR)
return -EINTR; /* arbitration lost */
@@ -299,7 +306,7 @@ static int pcf_xfer(struct i2c_adapter *i2c_adap,
struct i2c_algo_pcf_data *adap = i2c_adap->algo_data;
struct i2c_msg *pmsg;
int i;
- int ret=0, timeout, status;
+ int ret = 0, timeout, status;
if (adap->xfer_begin)
adap->xfer_begin(adap->data);
@@ -313,7 +320,7 @@ static int pcf_xfer(struct i2c_adapter *i2c_adap,
goto out;
}
- for (i = 0;ret >= 0 && i < num; i++) {
+ for (i = 0; ret >= 0 && i < num; i++) {
pmsg = &msgs[i];
DEB2(printk(KERN_DEBUG "i2c-algo-pcf.o: Doing %s %d bytes to 0x%02x - %d of %d messages\n",
@@ -358,9 +365,9 @@ static int pcf_xfer(struct i2c_adapter *i2c_adap,
if (ret != pmsg->len) {
DEB2(printk(KERN_DEBUG "i2c-algo-pcf.o: fail: "
- "only read %d bytes.\n",ret));
+ "only read %d bytes.\n", ret));
} else {
- DEB2(printk(KERN_DEBUG "i2c-algo-pcf.o: read %d bytes.\n",ret));
+ DEB2(printk(KERN_DEBUG "i2c-algo-pcf.o: read %d bytes.\n", ret));
}
} else {
ret = pcf_sendbytes(i2c_adap, pmsg->buf, pmsg->len,
@@ -368,9 +375,9 @@ static int pcf_xfer(struct i2c_adapter *i2c_adap,
if (ret != pmsg->len) {
DEB2(printk(KERN_DEBUG "i2c-algo-pcf.o: fail: "
- "only wrote %d bytes.\n",ret));
+ "only wrote %d bytes.\n", ret));
} else {
- DEB2(printk(KERN_DEBUG "i2c-algo-pcf.o: wrote %d bytes.\n",ret));
+ DEB2(printk(KERN_DEBUG "i2c-algo-pcf.o: wrote %d bytes.\n", ret));
}
}
}
@@ -406,7 +413,8 @@ int i2c_pcf_add_bus(struct i2c_adapter *adap)
/* register new adapter to i2c module... */
adap->algo = &pcf_algo;
- if ((rval = pcf_init_8584(pcf_adap)))
+ rval = pcf_init_8584(pcf_adap);
+ if (rval)
return rval;
rval = i2c_add_adapter(adap);
--
2.43.0> Fixed all 18 errors revealed using checkpatch.pl on i2c-algo-pcf.c
…
See also:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.17-rc4#n81
…
> Motivation is to fix all errors and warnings i2c-algo-pcf kerenel
…
kernel?
…
> v2:
> Fixed build errors generated by missing ; after do - while.
> Missed to git add latest changes to patch. Build is ok.
>
> Signed-off-by: Cezar Chiru <chiru.cezar.89@gmail.com>
> ---
> drivers/i2c/algos/i2c-algo-pcf.c | 42 +++++++++++++++++++-------------
…
* Please move your patch version descriptions behind the marker line.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.17-rc4#n784
* Will enumerations become more helpful?
Regards,
Markus
On Sun, Sep 07, 2025 at 03:19:20PM +0200, Markus Elfring wrote: Hello Markus, > > Fixed all 18 errors revealed using checkpatch.pl on i2c-algo-pcf.c > … > > See also: > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.17-rc4#n81 Refactored the commit message to be more compact > … > > Motivation is to fix all errors and warnings i2c-algo-pcf kerenel > … > kernel? Yes, "kernel" was intended. Fixed. > > v2: > > Fixed build errors generated by missing ; after do - while. > > Missed to git add latest changes to patch. Build is ok. > > > > Signed-off-by: Cezar Chiru <chiru.cezar.89@gmail.com> > > --- > > drivers/i2c/algos/i2c-algo-pcf.c | 42 +++++++++++++++++++------------- > … I moved the version info and information from commit message that didn't needed to reach the commit message after the --- marker line of the patch. > > * Please move your patch version descriptions behind the marker line. > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.17-rc4#n784 > > * Will enumerations become more helpful? If you were talking about enumerating the type of errors fixed in the commit message then I done that. If something else, could you please explain it to me? Markus, Andi, new [PATCH v3] attached to this email. Thank you, Best regards, Cezar Chiru
>> See also: >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.17-rc4#n81 > Refactored the commit message to be more compact Would you get into the mood to contribute another improved patch series? https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.17-rc4#n168 > Markus, Andi, new [PATCH v3] attached to this email. Please take another look at the usual development message requirements. Regards, Markus
Testing:
*built kernel and modules with I2C_ALGOPCF=m and my 3 patches applied on top.
*installed kernel and external modules generated by build on my laptop
*rebooted and loaded i2c-algo-pcf.ko with param i2c_debug=2/3/9.
*No success message related to i2c_algo_pcf was seen in dmesg but also no errors.
*Module loading and unloading successfull.
*No PCF8584 Hardware was available.
Patches 1 and 3 report 4 and 6 warnings when running checkpatch on them. But the warnings will be fixed in a new patchset that addresses fixing warnings.
Cezar Chiru (3):
i2c : pcf8584 : Fix debug macros defines of if statements
i2c : PCF8584 : Fix do not use assignment in 'if' conditional
i2c : PCF8584 : Fixed space required after different operators
drivers/i2c/algos/i2c-algo-pcf.c | 62 ++++++++++++++++++++------------
1 file changed, 39 insertions(+), 23 deletions(-)
--
2.43.0
The issue was that macros starting with 'if' should be enclosed by
do - while loop. Revealed by checkpatch.pl.
The patch fixes this and is necessary because by enclosure possible
if/else logic defects are avoided . Also fixed inconsistent macro
usage ending ';', which caused build error with the macro defines
enclosure in some cases.
Signed-off-by: Cezar Chiru <chiru.cezar.89@gmail.com>
---
drivers/i2c/algos/i2c-algo-pcf.c | 27 ++++++++++++++++++---------
1 file changed, 18 insertions(+), 9 deletions(-)
diff --git a/drivers/i2c/algos/i2c-algo-pcf.c b/drivers/i2c/algos/i2c-algo-pcf.c
index fd563e845d4b..3fc4b5080a32 100644
--- a/drivers/i2c/algos/i2c-algo-pcf.c
+++ b/drivers/i2c/algos/i2c-algo-pcf.c
@@ -23,10 +23,19 @@
#include "i2c-algo-pcf.h"
-#define DEB2(x) if (i2c_debug >= 2) x
-#define DEB3(x) if (i2c_debug >= 3) x /* print several statistical values */
-#define DEBPROTO(x) if (i2c_debug >= 9) x;
- /* debug the protocol by showing transferred bits */
+#define DEB2(x) do { \
+ if (i2c_debug >= 2) \
+ x; \
+ } while (0)
+#define DEB3(x) do { \
+ if (i2c_debug >= 3) \
+ x; /* print several statistical values */ \
+ } while (0)
+#define DEBPROTO(x) do { \
+ if (i2c_debug >= 9) \
+ x; \
+ /* debug the protocol by showing transferred bits */ \
+ } while (0)
#define DEF_TIMEOUT 16
/*
@@ -308,7 +317,7 @@ static int pcf_xfer(struct i2c_adapter *i2c_adap,
timeout = wait_for_bb(adap);
if (timeout) {
DEB2(printk(KERN_ERR "i2c-algo-pcf.o: "
- "Timeout waiting for BB in pcf_xfer\n");)
+ "Timeout waiting for BB in pcf_xfer\n"));
i = -EIO;
goto out;
}
@@ -318,7 +327,7 @@ static int pcf_xfer(struct i2c_adapter *i2c_adap,
DEB2(printk(KERN_DEBUG "i2c-algo-pcf.o: Doing %s %d bytes to 0x%02x - %d of %d messages\n",
str_read_write(pmsg->flags & I2C_M_RD),
- pmsg->len, pmsg->addr, i + 1, num);)
+ pmsg->len, pmsg->addr, i + 1, num));
ret = pcf_doAddress(adap, pmsg);
@@ -336,7 +345,7 @@ static int pcf_xfer(struct i2c_adapter *i2c_adap,
}
i2c_stop(adap);
DEB2(printk(KERN_ERR "i2c-algo-pcf.o: Timeout waiting "
- "for PIN(1) in pcf_xfer\n");)
+ "for PIN(1) in pcf_xfer\n"));
i = -EREMOTEIO;
goto out;
}
@@ -344,13 +353,13 @@ static int pcf_xfer(struct i2c_adapter *i2c_adap,
/* Check LRB (last rcvd bit - slave ack) */
if (status & I2C_PCF_LRB) {
i2c_stop(adap);
- DEB2(printk(KERN_ERR "i2c-algo-pcf.o: No LRB(1) in pcf_xfer\n");)
+ DEB2(printk(KERN_ERR "i2c-algo-pcf.o: No LRB(1) in pcf_xfer\n"));
i = -EREMOTEIO;
goto out;
}
DEB3(printk(KERN_DEBUG "i2c-algo-pcf.o: Msg %d, addr=0x%x, flags=0x%x, len=%d\n",
- i, msgs[i].addr, msgs[i].flags, msgs[i].len);)
+ i, msgs[i].addr, msgs[i].flags, msgs[i].len));
if (pmsg->flags & I2C_M_RD) {
ret = pcf_readbytes(i2c_adap, pmsg->buf, pmsg->len,
--
2.43.0…> The patch fixes this and … Would you care for the usage of imperative mood in your change descriptions? https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.17-rc5#n94 How do you think about to omit space characters before colons in patch prefixes? https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.17-rc5#n649 Regards, Markus
--- Order of patches to be applied: 1st: * i2c: PCF8584: Fix debug macros defines of if statements 2nd: * i2c: PCF8584: Fix do not use assignment in 'if' conditional 3rd: * i2c: PCF8584: Fixed space(s) required after different operators Testing: *built kernel and modules with I2C_ALGOPCF=m and my 3 patches applied on top. *installed kernel and external modules generated by build on my laptop *rebooted and loaded i2c-algo-pcf.ko with param i2c_debug=2/3/9. *No success message related to i2c_algo_pcf was seen in dmesg but also no errors. *Module loading and unloading successfull. *No PCF8584 Hardware was available. Patches 1 and 3 report 4 and 6 warnings when running checkpatch on them. But the warnings will be fixed in a new patchset that addresses fixing warnings. Cezar Chiru (3): i2c: PCF8584: Fix debug macros defines of if statements i2c: PCF8584: Fix do not use assignment in 'if' conditional i2c: PCF8584: Fixed space(s) required after different operators drivers/i2c/algos/i2c-algo-pcf.c | 62 ++++++++++++++++++++------------ 1 file changed, 39 insertions(+), 23 deletions(-) -- 2.43.0
Macros starting with 'if' should be enclosed by do - while loop.
Revealed by checkpatch.pl.
Change is necessary because by enclosure possible if/else logic
defects are avoided . Also fixed inconsistent macro usage ending
';', which caused build error with the macro defines
enclosure in some cases.
Signed-off-by: Cezar Chiru <chiru.cezar.89@gmail.com>
---
drivers/i2c/algos/i2c-algo-pcf.c | 27 ++++++++++++++++++---------
1 file changed, 18 insertions(+), 9 deletions(-)
diff --git a/drivers/i2c/algos/i2c-algo-pcf.c b/drivers/i2c/algos/i2c-algo-pcf.c
index fd563e845d4b..3fc4b5080a32 100644
--- a/drivers/i2c/algos/i2c-algo-pcf.c
+++ b/drivers/i2c/algos/i2c-algo-pcf.c
@@ -23,10 +23,19 @@
#include "i2c-algo-pcf.h"
-#define DEB2(x) if (i2c_debug >= 2) x
-#define DEB3(x) if (i2c_debug >= 3) x /* print several statistical values */
-#define DEBPROTO(x) if (i2c_debug >= 9) x;
- /* debug the protocol by showing transferred bits */
+#define DEB2(x) do { \
+ if (i2c_debug >= 2) \
+ x; \
+ } while (0)
+#define DEB3(x) do { \
+ if (i2c_debug >= 3) \
+ x; /* print several statistical values */ \
+ } while (0)
+#define DEBPROTO(x) do { \
+ if (i2c_debug >= 9) \
+ x; \
+ /* debug the protocol by showing transferred bits */ \
+ } while (0)
#define DEF_TIMEOUT 16
/*
@@ -308,7 +317,7 @@ static int pcf_xfer(struct i2c_adapter *i2c_adap,
timeout = wait_for_bb(adap);
if (timeout) {
DEB2(printk(KERN_ERR "i2c-algo-pcf.o: "
- "Timeout waiting for BB in pcf_xfer\n");)
+ "Timeout waiting for BB in pcf_xfer\n"));
i = -EIO;
goto out;
}
@@ -318,7 +327,7 @@ static int pcf_xfer(struct i2c_adapter *i2c_adap,
DEB2(printk(KERN_DEBUG "i2c-algo-pcf.o: Doing %s %d bytes to 0x%02x - %d of %d messages\n",
str_read_write(pmsg->flags & I2C_M_RD),
- pmsg->len, pmsg->addr, i + 1, num);)
+ pmsg->len, pmsg->addr, i + 1, num));
ret = pcf_doAddress(adap, pmsg);
@@ -336,7 +345,7 @@ static int pcf_xfer(struct i2c_adapter *i2c_adap,
}
i2c_stop(adap);
DEB2(printk(KERN_ERR "i2c-algo-pcf.o: Timeout waiting "
- "for PIN(1) in pcf_xfer\n");)
+ "for PIN(1) in pcf_xfer\n"));
i = -EREMOTEIO;
goto out;
}
@@ -344,13 +353,13 @@ static int pcf_xfer(struct i2c_adapter *i2c_adap,
/* Check LRB (last rcvd bit - slave ack) */
if (status & I2C_PCF_LRB) {
i2c_stop(adap);
- DEB2(printk(KERN_ERR "i2c-algo-pcf.o: No LRB(1) in pcf_xfer\n");)
+ DEB2(printk(KERN_ERR "i2c-algo-pcf.o: No LRB(1) in pcf_xfer\n"));
i = -EREMOTEIO;
goto out;
}
DEB3(printk(KERN_DEBUG "i2c-algo-pcf.o: Msg %d, addr=0x%x, flags=0x%x, len=%d\n",
- i, msgs[i].addr, msgs[i].flags, msgs[i].len);)
+ i, msgs[i].addr, msgs[i].flags, msgs[i].len));
if (pmsg->flags & I2C_M_RD) {
ret = pcf_readbytes(i2c_adap, pmsg->buf, pmsg->len,
--
2.43.0…> Change is necessary because … Does anything hinder you to choose imperative mood for change descriptions? https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.17-rc5#n94 Would you occasionally care more also for word wrapping? https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.17-rc5#n658 …> --- > drivers/i2c/algos/i2c-algo-pcf.c | 27 ++++++++++++++++++--------- … Some contributors would appreciate patch version descriptions. https://lore.kernel.org/all/?q=%22This+looks+like+a+new+version+of+a+previously+submitted+patch%22 https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.17-rc5#n310 Regards, Markus
On Mon, Sep 08, 2025 at 04:46:47PM +0200, Markus Elfring wrote: Hello Markus, First I want to thank you for reviewing my patches and patchset and making comments for me to improve my patches. I am new to kernel hacking and this is the first patchset that I submit to linux kernel. I'm doing my best to improve my work so that it can be accepted into the kernel tree. > …> Change is necessary because … > > Does anything hinder you to choose imperative mood for change descriptions? > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.17-rc5#n94 Sorry, I didn't understood the first time. I will try my best. > > Would you occasionally care more also for word wrapping? > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.17-rc5#n658 > I will follow from now on the line wrapped at 75 columns. > > …> --- > > drivers/i2c/algos/i2c-algo-pcf.c | 27 ++++++++++++++++++--------- > … > > Some contributors would appreciate patch version descriptions. > https://lore.kernel.org/all/?q=%22This+looks+like+a+new+version+of+a+previously+submitted+patch%22 > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.17-rc5#n310 > After I sent V2 of patchset I realized I didn't included changes after --- marker line. I will send V3 trying to apply your suggestions. Best regards, Cezar Chiru
Fixed errors usage of assignment inside of 'if' conditional
statements. Revealed by checkpatch.pl.
Fixed by moving assignment from inside 'if' to 1 line before each
if conditional statement that caused errors.
Signed-off-by: Cezar Chiru <chiru.cezar.89@gmail.com>
---
drivers/i2c/algos/i2c-algo-pcf.c | 21 ++++++++++++++-------
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/drivers/i2c/algos/i2c-algo-pcf.c b/drivers/i2c/algos/i2c-algo-pcf.c
index 3fc4b5080a32..598bf000bf4a 100644
--- a/drivers/i2c/algos/i2c-algo-pcf.c
+++ b/drivers/i2c/algos/i2c-algo-pcf.c
@@ -169,7 +169,8 @@ static int pcf_init_8584 (struct i2c_algo_pcf_data *adap)
* check to see S1 now used as R/W ctrl -
* PCF8584 does that when ESO is zero
*/
- if (((temp = get_pcf(adap, 1)) & 0x7f) != (0)) {
+ temp = get_pcf(adap, 1);
+ if ((temp & 0x7f) != (0)) {
DEB2(printk(KERN_ERR "i2c-algo-pcf.o: PCF detection failed -- can't select S0 (0x%02x).\n", temp));
return -ENXIO; /* definitely not PCF8584 */
}
@@ -177,7 +178,8 @@ static int pcf_init_8584 (struct i2c_algo_pcf_data *adap)
/* load own address in S0, effective address is (own << 1) */
i2c_outb(adap, get_own(adap));
/* check it's really written */
- if ((temp = i2c_inb(adap)) != get_own(adap)) {
+ temp = i2c_inb(adap);
+ if (temp != get_own(adap)) {
DEB2(printk(KERN_ERR "i2c-algo-pcf.o: PCF detection failed -- can't set S0 (0x%02x).\n", temp));
return -ENXIO;
}
@@ -185,7 +187,8 @@ static int pcf_init_8584 (struct i2c_algo_pcf_data *adap)
/* S1=0xA0, next byte in S2 */
set_pcf(adap, 1, I2C_PCF_PIN | I2C_PCF_ES1);
/* check to see S2 now selected */
- if (((temp = get_pcf(adap, 1)) & 0x7f) != I2C_PCF_ES1) {
+ temp = get_pcf(adap, 1);
+ if ((temp & 0x7f) != I2C_PCF_ES1) {
DEB2(printk(KERN_ERR "i2c-algo-pcf.o: PCF detection failed -- can't select S2 (0x%02x).\n", temp));
return -ENXIO;
}
@@ -193,7 +196,8 @@ static int pcf_init_8584 (struct i2c_algo_pcf_data *adap)
/* load clock register S2 */
i2c_outb(adap, get_clock(adap));
/* check it's really written, the only 5 lowest bits does matter */
- if (((temp = i2c_inb(adap)) & 0x1f) != get_clock(adap)) {
+ temp = i2c_inb(adap);
+ if ((temp & 0x1f) != get_clock(adap)) {
DEB2(printk(KERN_ERR "i2c-algo-pcf.o: PCF detection failed -- can't set S2 (0x%02x).\n", temp));
return -ENXIO;
}
@@ -202,7 +206,8 @@ static int pcf_init_8584 (struct i2c_algo_pcf_data *adap)
set_pcf(adap, 1, I2C_PCF_IDLE);
/* check to see PCF is really idled and we can access status register */
- if ((temp = get_pcf(adap, 1)) != (I2C_PCF_PIN | I2C_PCF_BB)) {
+ temp = get_pcf(adap, 1);
+ if (temp != (I2C_PCF_PIN | I2C_PCF_BB)) {
DEB2(printk(KERN_ERR "i2c-algo-pcf.o: PCF detection failed -- can't select S1` (0x%02x).\n", temp));
return -ENXIO;
}
@@ -255,7 +260,8 @@ static int pcf_readbytes(struct i2c_adapter *i2c_adap, char *buf,
/* increment number of bytes to read by one -- read dummy byte */
for (i = 0; i <= count; i++) {
- if ((wfp = wait_for_pin(adap, &status))) {
+ wfp = wait_for_pin(adap, &status);
+ if (wfp) {
if (wfp == -EINTR)
return -EINTR; /* arbitration lost */
@@ -415,7 +421,8 @@ int i2c_pcf_add_bus(struct i2c_adapter *adap)
/* register new adapter to i2c module... */
adap->algo = &pcf_algo;
- if ((rval = pcf_init_8584(pcf_adap)))
+ rval = pcf_init_8584(pcf_adap);
+ if (rval)
return rval;
rval = i2c_add_adapter(adap);
--
2.43.0Error: spaces required around or before or after '=', ';', '<'
and ',' operators. Revealed by checkpatch.pl.
Added space(s) around or before or after different operators.
Signed-off-by: Cezar Chiru <chiru.cezar.89@gmail.com>
---
drivers/i2c/algos/i2c-algo-pcf.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/i2c/algos/i2c-algo-pcf.c b/drivers/i2c/algos/i2c-algo-pcf.c
index 598bf000bf4a..3439b7387a54 100644
--- a/drivers/i2c/algos/i2c-algo-pcf.c
+++ b/drivers/i2c/algos/i2c-algo-pcf.c
@@ -223,7 +223,7 @@ static int pcf_sendbytes(struct i2c_adapter *i2c_adap, const char *buf,
struct i2c_algo_pcf_data *adap = i2c_adap->algo_data;
int wrcount, status, timeout;
- for (wrcount=0; wrcount<count; ++wrcount) {
+ for (wrcount = 0; wrcount < count; ++wrcount) {
DEB2(dev_dbg(&i2c_adap->dev, "i2c_write: writing %2.2X\n",
buf[wrcount] & 0xff));
i2c_outb(adap, buf[wrcount]);
@@ -314,7 +314,7 @@ static int pcf_xfer(struct i2c_adapter *i2c_adap,
struct i2c_algo_pcf_data *adap = i2c_adap->algo_data;
struct i2c_msg *pmsg;
int i;
- int ret=0, timeout, status;
+ int ret = 0, timeout, status;
if (adap->xfer_begin)
adap->xfer_begin(adap->data);
@@ -328,7 +328,7 @@ static int pcf_xfer(struct i2c_adapter *i2c_adap,
goto out;
}
- for (i = 0;ret >= 0 && i < num; i++) {
+ for (i = 0; ret >= 0 && i < num; i++) {
pmsg = &msgs[i];
DEB2(printk(KERN_DEBUG "i2c-algo-pcf.o: Doing %s %d bytes to 0x%02x - %d of %d messages\n",
@@ -373,9 +373,9 @@ static int pcf_xfer(struct i2c_adapter *i2c_adap,
if (ret != pmsg->len) {
DEB2(printk(KERN_DEBUG "i2c-algo-pcf.o: fail: "
- "only read %d bytes.\n",ret));
+ "only read %d bytes.\n", ret));
} else {
- DEB2(printk(KERN_DEBUG "i2c-algo-pcf.o: read %d bytes.\n",ret));
+ DEB2(printk(KERN_DEBUG "i2c-algo-pcf.o: read %d bytes.\n", ret));
}
} else {
ret = pcf_sendbytes(i2c_adap, pmsg->buf, pmsg->len,
@@ -383,9 +383,9 @@ static int pcf_xfer(struct i2c_adapter *i2c_adap,
if (ret != pmsg->len) {
DEB2(printk(KERN_DEBUG "i2c-algo-pcf.o: fail: "
- "only wrote %d bytes.\n",ret));
+ "only wrote %d bytes.\n", ret));
} else {
- DEB2(printk(KERN_DEBUG "i2c-algo-pcf.o: wrote %d bytes.\n",ret));
+ DEB2(printk(KERN_DEBUG "i2c-algo-pcf.o: wrote %d bytes.\n", ret));
}
}
}
--
2.43.0Fixed errors usage of assignment inside of 'if' conditional
statements. Revealed by checkpatch.pl.
Fixed by moving assignment from inside 'if' to 1 line before each
if conditional statement that caused errors.
Signed-off-by: Cezar Chiru <chiru.cezar.89@gmail.com>
---
drivers/i2c/algos/i2c-algo-pcf.c | 21 ++++++++++++++-------
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/drivers/i2c/algos/i2c-algo-pcf.c b/drivers/i2c/algos/i2c-algo-pcf.c
index 3fc4b5080a32..598bf000bf4a 100644
--- a/drivers/i2c/algos/i2c-algo-pcf.c
+++ b/drivers/i2c/algos/i2c-algo-pcf.c
@@ -169,7 +169,8 @@ static int pcf_init_8584 (struct i2c_algo_pcf_data *adap)
* check to see S1 now used as R/W ctrl -
* PCF8584 does that when ESO is zero
*/
- if (((temp = get_pcf(adap, 1)) & 0x7f) != (0)) {
+ temp = get_pcf(adap, 1);
+ if ((temp & 0x7f) != (0)) {
DEB2(printk(KERN_ERR "i2c-algo-pcf.o: PCF detection failed -- can't select S0 (0x%02x).\n", temp));
return -ENXIO; /* definitely not PCF8584 */
}
@@ -177,7 +178,8 @@ static int pcf_init_8584 (struct i2c_algo_pcf_data *adap)
/* load own address in S0, effective address is (own << 1) */
i2c_outb(adap, get_own(adap));
/* check it's really written */
- if ((temp = i2c_inb(adap)) != get_own(adap)) {
+ temp = i2c_inb(adap);
+ if (temp != get_own(adap)) {
DEB2(printk(KERN_ERR "i2c-algo-pcf.o: PCF detection failed -- can't set S0 (0x%02x).\n", temp));
return -ENXIO;
}
@@ -185,7 +187,8 @@ static int pcf_init_8584 (struct i2c_algo_pcf_data *adap)
/* S1=0xA0, next byte in S2 */
set_pcf(adap, 1, I2C_PCF_PIN | I2C_PCF_ES1);
/* check to see S2 now selected */
- if (((temp = get_pcf(adap, 1)) & 0x7f) != I2C_PCF_ES1) {
+ temp = get_pcf(adap, 1);
+ if ((temp & 0x7f) != I2C_PCF_ES1) {
DEB2(printk(KERN_ERR "i2c-algo-pcf.o: PCF detection failed -- can't select S2 (0x%02x).\n", temp));
return -ENXIO;
}
@@ -193,7 +196,8 @@ static int pcf_init_8584 (struct i2c_algo_pcf_data *adap)
/* load clock register S2 */
i2c_outb(adap, get_clock(adap));
/* check it's really written, the only 5 lowest bits does matter */
- if (((temp = i2c_inb(adap)) & 0x1f) != get_clock(adap)) {
+ temp = i2c_inb(adap);
+ if ((temp & 0x1f) != get_clock(adap)) {
DEB2(printk(KERN_ERR "i2c-algo-pcf.o: PCF detection failed -- can't set S2 (0x%02x).\n", temp));
return -ENXIO;
}
@@ -202,7 +206,8 @@ static int pcf_init_8584 (struct i2c_algo_pcf_data *adap)
set_pcf(adap, 1, I2C_PCF_IDLE);
/* check to see PCF is really idled and we can access status register */
- if ((temp = get_pcf(adap, 1)) != (I2C_PCF_PIN | I2C_PCF_BB)) {
+ temp = get_pcf(adap, 1);
+ if (temp != (I2C_PCF_PIN | I2C_PCF_BB)) {
DEB2(printk(KERN_ERR "i2c-algo-pcf.o: PCF detection failed -- can't select S1` (0x%02x).\n", temp));
return -ENXIO;
}
@@ -255,7 +260,8 @@ static int pcf_readbytes(struct i2c_adapter *i2c_adap, char *buf,
/* increment number of bytes to read by one -- read dummy byte */
for (i = 0; i <= count; i++) {
- if ((wfp = wait_for_pin(adap, &status))) {
+ wfp = wait_for_pin(adap, &status);
+ if (wfp) {
if (wfp == -EINTR)
return -EINTR; /* arbitration lost */
@@ -415,7 +421,8 @@ int i2c_pcf_add_bus(struct i2c_adapter *adap)
/* register new adapter to i2c module... */
adap->algo = &pcf_algo;
- if ((rval = pcf_init_8584(pcf_adap)))
+ rval = pcf_init_8584(pcf_adap);
+ if (rval)
return rval;
rval = i2c_add_adapter(adap);
--
2.43.0The error was: spaces required around or before or after '=',
';', '<' and ',' operators.
Fixed by adding space(s) around or before or after different
operators.
Signed-off-by: Cezar Chiru <chiru.cezar.89@gmail.com>
---
drivers/i2c/algos/i2c-algo-pcf.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/i2c/algos/i2c-algo-pcf.c b/drivers/i2c/algos/i2c-algo-pcf.c
index 598bf000bf4a..3439b7387a54 100644
--- a/drivers/i2c/algos/i2c-algo-pcf.c
+++ b/drivers/i2c/algos/i2c-algo-pcf.c
@@ -223,7 +223,7 @@ static int pcf_sendbytes(struct i2c_adapter *i2c_adap, const char *buf,
struct i2c_algo_pcf_data *adap = i2c_adap->algo_data;
int wrcount, status, timeout;
- for (wrcount=0; wrcount<count; ++wrcount) {
+ for (wrcount = 0; wrcount < count; ++wrcount) {
DEB2(dev_dbg(&i2c_adap->dev, "i2c_write: writing %2.2X\n",
buf[wrcount] & 0xff));
i2c_outb(adap, buf[wrcount]);
@@ -314,7 +314,7 @@ static int pcf_xfer(struct i2c_adapter *i2c_adap,
struct i2c_algo_pcf_data *adap = i2c_adap->algo_data;
struct i2c_msg *pmsg;
int i;
- int ret=0, timeout, status;
+ int ret = 0, timeout, status;
if (adap->xfer_begin)
adap->xfer_begin(adap->data);
@@ -328,7 +328,7 @@ static int pcf_xfer(struct i2c_adapter *i2c_adap,
goto out;
}
- for (i = 0;ret >= 0 && i < num; i++) {
+ for (i = 0; ret >= 0 && i < num; i++) {
pmsg = &msgs[i];
DEB2(printk(KERN_DEBUG "i2c-algo-pcf.o: Doing %s %d bytes to 0x%02x - %d of %d messages\n",
@@ -373,9 +373,9 @@ static int pcf_xfer(struct i2c_adapter *i2c_adap,
if (ret != pmsg->len) {
DEB2(printk(KERN_DEBUG "i2c-algo-pcf.o: fail: "
- "only read %d bytes.\n",ret));
+ "only read %d bytes.\n", ret));
} else {
- DEB2(printk(KERN_DEBUG "i2c-algo-pcf.o: read %d bytes.\n",ret));
+ DEB2(printk(KERN_DEBUG "i2c-algo-pcf.o: read %d bytes.\n", ret));
}
} else {
ret = pcf_sendbytes(i2c_adap, pmsg->buf, pmsg->len,
@@ -383,9 +383,9 @@ static int pcf_xfer(struct i2c_adapter *i2c_adap,
if (ret != pmsg->len) {
DEB2(printk(KERN_DEBUG "i2c-algo-pcf.o: fail: "
- "only wrote %d bytes.\n",ret));
+ "only wrote %d bytes.\n", ret));
} else {
- DEB2(printk(KERN_DEBUG "i2c-algo-pcf.o: wrote %d bytes.\n",ret));
+ DEB2(printk(KERN_DEBUG "i2c-algo-pcf.o: wrote %d bytes.\n", ret));
}
}
}
--
2.43.0© 2016 - 2025 Red Hat, Inc.