| Message ID | 20240930-cocci-opportunity-v1-0-81e137456ce0@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Ricardo, On Mon, Sep 30, 2024 at 12:03:55PM +0000, Ricardo Ribalda wrote: > include/linux/string_choices.h contains a set of helpers that can be > used instead of hard coding some strings. > > Cocci has located some places where the helpers can be used. This > patchset uses the diff generated by cocci, plus these changes: Personally I think most of those helpers just hinder readability for not much added gain. String de-duplication is done by the linker already. The only value I see in the helpers is ensuring that the strings are consistently written, and I think we can do so through other means. > diff --git a/drivers/media/dvb-frontends/ascot2e.c b/drivers/media/dvb-frontends/ascot2e.c > index 8c3eb5d69dda..ec7a718428fc 100644 > --- a/drivers/media/dvb-frontends/ascot2e.c > +++ b/drivers/media/dvb-frontends/ascot2e.c > @@ -104,7 +104,7 @@ static void ascot2e_i2c_debug(struct ascot2e_priv *priv, > u8 reg, u8 write, const u8 *data, u32 len) > { > dev_dbg(&priv->i2c->dev, "ascot2e: I2C %s reg 0x%02x size %d\n", > - str_read_write(write == 0), reg, len); > + str_write_read(write), reg, len); > print_hex_dump_bytes("ascot2e: I2C data: ", > DUMP_PREFIX_OFFSET, data, len); > } > diff --git a/drivers/media/dvb-frontends/cxd2841er.c b/drivers/media/dvb-frontends/cxd2841er.c > index db684f314b47..d1b84cd9c510 100644 > --- a/drivers/media/dvb-frontends/cxd2841er.c > +++ b/drivers/media/dvb-frontends/cxd2841er.c > @@ -206,7 +206,7 @@ static void cxd2841er_i2c_debug(struct cxd2841er_priv *priv, > { > dev_dbg(&priv->i2c->dev, > "cxd2841er: I2C %s addr %02x reg 0x%02x size %d data %*ph\n", > - str_read_write(write == 0), addr, reg, len, len, data); > + str_write_read(write), addr, reg, len, len, data); > } > > static int cxd2841er_write_regs(struct cxd2841er_priv *priv, > diff --git a/drivers/media/dvb-frontends/helene.c b/drivers/media/dvb-frontends/helene.c > index 52198cb49dba..b4527c141d9c 100644 > --- a/drivers/media/dvb-frontends/helene.c > +++ b/drivers/media/dvb-frontends/helene.c > @@ -279,7 +279,7 @@ static void helene_i2c_debug(struct helene_priv *priv, > u8 reg, u8 write, const u8 *data, u32 len) > { > dev_dbg(&priv->i2c->dev, "helene: I2C %s reg 0x%02x size %d\n", > - str_read_write(write == 0), reg, len); > + str_write_read(write), reg, len); > print_hex_dump_bytes("helene: I2C data: ", > DUMP_PREFIX_OFFSET, data, len); > } > diff --git a/drivers/media/dvb-frontends/horus3a.c b/drivers/media/dvb-frontends/horus3a.c > index 84385079918c..10300ebf3ca0 100644 > --- a/drivers/media/dvb-frontends/horus3a.c > +++ b/drivers/media/dvb-frontends/horus3a.c > @@ -38,7 +38,7 @@ static void horus3a_i2c_debug(struct horus3a_priv *priv, > u8 reg, u8 write, const u8 *data, u32 len) > { > dev_dbg(&priv->i2c->dev, "horus3a: I2C %s reg 0x%02x size %d\n", > - str_read_write(write == 0), reg, len); > + str_write_read(write), reg, len); > print_hex_dump_bytes("horus3a: I2C data: ", > DUMP_PREFIX_OFFSET, data, len); > } > diff --git a/drivers/media/i2c/adv7842.c b/drivers/media/i2c/adv7842.c > index ba174aa45afa..a43479c3ff03 100644 > --- a/drivers/media/i2c/adv7842.c > +++ b/drivers/media/i2c/adv7842.c > @@ -2763,7 +2763,7 @@ static int adv7842_cp_log_status(struct v4l2_subdev *sd) > str_true_false(io_read(sd, 0x6a) & 0x10)); > } > v4l2_info(sd, "CP free run: %s\n", > - str_on_off(!!(cp_read(sd, 0xff) & 0x10))); > + str_on_off(cp_read(sd, 0xff) & 0x10)); > v4l2_info(sd, "Prim-mode = 0x%x, video std = 0x%x, v_freq = 0x%x\n", > io_read(sd, 0x01) & 0x0f, io_read(sd, 0x00) & 0x3f, > (io_read(sd, 0x01) & 0x70) >> 4); > diff --git a/drivers/media/pci/saa7134/saa7134-cards.c b/drivers/media/pci/saa7134/saa7134-cards.c > index 301b89e799d8..79cd61fb0205 100644 > --- a/drivers/media/pci/saa7134/saa7134-cards.c > +++ b/drivers/media/pci/saa7134/saa7134-cards.c > @@ -7981,7 +7981,7 @@ int saa7134_board_init2(struct saa7134_dev *dev) > rc = i2c_transfer(&dev->i2c_adap, &msg, 1); > pr_info("%s: probe IR chip @ i2c 0x%02x: %s\n", > dev->name, msg.addr, > - str_yes_no(1 == rc)); > + str_yes_no(rc == 1)); > if (rc == 1) > dev->has_remote = SAA7134_REMOTE_I2C; > } > diff --git a/drivers/media/pci/saa7134/saa7134-input.c b/drivers/media/pci/saa7134/saa7134-input.c > index 90837ec6e70f..239f0b9d080a 100644 > --- a/drivers/media/pci/saa7134/saa7134-input.c > +++ b/drivers/media/pci/saa7134/saa7134-input.c > @@ -895,7 +895,7 @@ void saa7134_probe_i2c_ir(struct saa7134_dev *dev) > rc = i2c_transfer(&dev->i2c_adap, &msg_msi, 1); > input_dbg("probe 0x%02x @ %s: %s\n", > msg_msi.addr, dev->i2c_adap.name, > - str_yes_no(1 == rc)); > + str_yes_no(rc == 1)); > break; > case SAA7134_BOARD_SNAZIO_TVPVR_PRO: > dev->init_data.name = "SnaZio* TVPVR PRO"; > @@ -931,7 +931,7 @@ void saa7134_probe_i2c_ir(struct saa7134_dev *dev) > rc = i2c_transfer(&dev->i2c_adap, &msg_msi, 1); > input_dbg("probe 0x%02x @ %s: %s\n", > msg_msi.addr, dev->i2c_adap.name, > - str_yes_no(1 == rc)); > + str_yes_no(rc == 1)); > break; > case SAA7134_BOARD_HAUPPAUGE_HVR1110: > dev->init_data.name = saa7134_boards[dev->board].name; > diff --git a/drivers/media/usb/pvrusb2/pvrusb2-ctrl.c b/drivers/media/usb/pvrusb2/pvrusb2-ctrl.c > index 448c40caf363..b6c9bda214c8 100644 > --- a/drivers/media/usb/pvrusb2/pvrusb2-ctrl.c > +++ b/drivers/media/usb/pvrusb2/pvrusb2-ctrl.c > @@ -521,7 +521,7 @@ int pvr2_ctrl_value_to_sym_internal(struct pvr2_ctrl *cptr, > *len = scnprintf(buf,maxlen,"%d",val); > ret = 0; > } else if (cptr->info->type == pvr2_ctl_bool) { > - *len = scnprintf(buf,maxlen,"%s",str_true_false(val)); > + *len = scnprintf(buf, maxlen, "%s", str_true_false(val)); > ret = 0; > } else if (cptr->info->type == pvr2_ctl_enum) { > const char * const *names; > diff --git a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c > index 96d3a0045fac..761d718478ca 100644 > --- a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c > +++ b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c > @@ -338,7 +338,7 @@ static void trace_stbit(const char *name,int val) > { > pvr2_trace(PVR2_TRACE_STBITS, > "State bit %s <-- %s", > - name,str_true_false(val)); > + name, str_true_false(val)); > } > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> > --- > Ricardo Ribalda (45): > media: staging: ipu3: Use string_choices helpers > media: staging: atomisp: Use string_choices helpers > media: core: Use string_choices helpers > media: pwc-ctl: Use string_choices helpers > media: pvrusb2:Use string_choices helpers > media: em28xx: Use string_choices helpers > media: dvb-usb: Use string_choices helpers > media: dvb-usb-v2: Use string_choices helpers > media: cx231xx: Use string_choices helpers > media: tuners: Use string_choices helpers > media: rc: Use string_choices helpers > media: dvb-frontends: Use string_choices helpers > media: pci: cx23885: Use string_choices helpers > media: saa7134: Use string_choices helpers > media: amphion: Use string_choices helpers > media: pci: ivtv: Use string_choices helpers > media: bttv: Use string_choices helpers > media: xilinx: Use string_choices helpers > media: platform: ti: Use string_choices helpers > media: st: Use string_choices helpers > media: coda: Use string_choices helpers > media: aspeed: Use string_choices helpers > media: ipu6: Use string_choices helpers > media: cx18: Use string_choices helpers > media: cobalt: Use string_choices helpers > media: videobuf2: Use string_choices helpers > media: cec: Use string_choices helpers > media: b2c2: Use string_choices helpers > media: siano: Use string_choices helpers > media: i2c: cx25840: Use string_choices helpers > media: i2c: vpx3220: Use string_choices helpers > media: i2c: tvp7002: Use string_choices helpers > media: i2c: ths8200: Use string_choices helpers > media: i2c: tda1997x: Use string_choices helpers > media: i2c: tc358743: Use string_choices helpers > media: i2c: st-mipid02: Use string_choices helpers > media: i2c: msp3400: Use string_choices helpers > media: i2c: max9286: Use string_choices helpers > media: i2c: saa717x: Use string_choices helpers > media: i2c: saa7127: Use string_choices helpers > media: i2c: saa7115: Use string_choices helpers > media: i2c: saa7110: Use string_choices helpers > media: i2c: adv7842: Use string_choices helpers > media: i2c: adv76xx: Use string_choices helpers > media: i2c: adv7511: Use string_choices helpers > > drivers/media/cec/platform/cec-gpio/cec-gpio.c | 4 +- > drivers/media/cec/usb/pulse8/pulse8-cec.c | 4 +- > drivers/media/common/b2c2/flexcop-hw-filter.c | 4 +- > drivers/media/common/siano/sms-cards.c | 3 +- > drivers/media/common/videobuf2/videobuf2-core.c | 5 ++- > drivers/media/dvb-frontends/ascot2e.c | 2 +- > drivers/media/dvb-frontends/cx24120.c | 4 +- > drivers/media/dvb-frontends/cxd2841er.c | 2 +- > drivers/media/dvb-frontends/drxk_hard.c | 4 +- > drivers/media/dvb-frontends/helene.c | 2 +- > drivers/media/dvb-frontends/horus3a.c | 2 +- > drivers/media/dvb-frontends/sp2.c | 2 +- > drivers/media/i2c/adv7511-v4l2.c | 11 +++--- > drivers/media/i2c/adv7604.c | 25 ++++++------ > drivers/media/i2c/adv7842.c | 40 ++++++++++---------- > drivers/media/i2c/cx25840/cx25840-core.c | 4 +- > drivers/media/i2c/cx25840/cx25840-ir.c | 34 ++++++++--------- > drivers/media/i2c/max9286.c | 2 +- > drivers/media/i2c/msp3400-driver.c | 4 +- > drivers/media/i2c/saa7110.c | 2 +- > drivers/media/i2c/saa7115.c | 2 +- > drivers/media/i2c/saa7127.c | 15 +++++--- > drivers/media/i2c/saa717x.c | 2 +- > drivers/media/i2c/st-mipid02.c | 2 +- > drivers/media/i2c/tc358743.c | 44 ++++++++++------------ > drivers/media/i2c/tda1997x.c | 6 +-- > drivers/media/i2c/ths8200.c | 4 +- > drivers/media/i2c/tvp7002.c | 2 +- > drivers/media/i2c/vpx3220.c | 2 +- > drivers/media/pci/bt8xx/bttv-cards.c | 16 ++++---- > drivers/media/pci/bt8xx/bttv-driver.c | 6 +-- > drivers/media/pci/cobalt/cobalt-driver.c | 2 +- > drivers/media/pci/cx18/cx18-av-core.c | 4 +- > drivers/media/pci/cx23885/altera-ci.c | 2 +- > drivers/media/pci/cx23885/cimax2.c | 2 +- > drivers/media/pci/cx23885/cx23888-ir.c | 36 +++++++++--------- > drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c | 2 +- > drivers/media/pci/ivtv/ivtvfb.c | 4 +- > drivers/media/pci/saa7134/saa7134-cards.c | 2 +- > drivers/media/pci/saa7134/saa7134-dvb.c | 2 +- > drivers/media/pci/saa7134/saa7134-input.c | 6 +-- > drivers/media/pci/saa7134/saa7134-video.c | 2 +- > drivers/media/platform/amphion/venc.c | 2 +- > drivers/media/platform/amphion/vpu_dbg.c | 2 +- > drivers/media/platform/aspeed/aspeed-video.c | 4 +- > drivers/media/platform/chips-media/coda/imx-vdoa.c | 3 +- > drivers/media/platform/st/sti/hva/hva-debugfs.c | 6 +-- > drivers/media/platform/ti/omap3isp/ispstat.c | 2 +- > drivers/media/platform/xilinx/xilinx-csi2rxss.c | 18 ++++----- > drivers/media/rc/ene_ir.c | 3 +- > drivers/media/rc/mceusb.c | 3 +- > drivers/media/rc/serial_ir.c | 5 ++- > drivers/media/tuners/tda18250.c | 2 +- > drivers/media/tuners/tda9887.c | 10 ++--- > drivers/media/usb/cx231xx/cx231xx-i2c.c | 4 +- > drivers/media/usb/cx231xx/cx231xx-video.c | 2 +- > drivers/media/usb/dvb-usb-v2/az6007.c | 4 +- > drivers/media/usb/dvb-usb-v2/dvb_usb_core.c | 4 +- > drivers/media/usb/dvb-usb/af9005-fe.c | 4 +- > drivers/media/usb/dvb-usb/dvb-usb-dvb.c | 6 +-- > drivers/media/usb/dvb-usb/opera1.c | 8 ++-- > drivers/media/usb/em28xx/em28xx-i2c.c | 4 +- > drivers/media/usb/em28xx/em28xx-video.c | 2 +- > drivers/media/usb/pvrusb2/pvrusb2-ctrl.c | 2 +- > drivers/media/usb/pvrusb2/pvrusb2-debugifc.c | 3 +- > drivers/media/usb/pvrusb2/pvrusb2-encoder.c | 5 +-- > drivers/media/usb/pvrusb2/pvrusb2-hdw.c | 6 +-- > drivers/media/usb/pvrusb2/pvrusb2-i2c-core.c | 3 +- > drivers/media/usb/pwc/pwc-ctrl.c | 2 +- > drivers/media/v4l2-core/v4l2-ctrls-core.c | 3 +- > drivers/media/v4l2-core/v4l2-fwnode.c | 12 +++--- > .../media/atomisp/pci/atomisp_compat_css20.c | 2 +- > .../media/atomisp/pci/atomisp_csi2_bridge.c | 2 +- > .../media/atomisp/pci/atomisp_gmin_platform.c | 4 +- > drivers/staging/media/atomisp/pci/atomisp_v4l2.c | 4 +- > .../media/atomisp/pci/runtime/binary/src/binary.c | 2 +- > drivers/staging/media/atomisp/pci/sh_css.c | 2 +- > drivers/staging/media/ipu3/ipu3-v4l2.c | 4 +- > 78 files changed, 240 insertions(+), 239 deletions(-) > --- > base-commit: 9852d85ec9d492ebef56dc5f229416c925758edc > change-id: 20240930-cocci-opportunity-40bca6a17c42
On 30/09/2024 14:21, Laurent Pinchart wrote: > Hi Ricardo, > > On Mon, Sep 30, 2024 at 12:03:55PM +0000, Ricardo Ribalda wrote: >> include/linux/string_choices.h contains a set of helpers that can be >> used instead of hard coding some strings. >> >> Cocci has located some places where the helpers can be used. This >> patchset uses the diff generated by cocci, plus these changes: > > Personally I think most of those helpers just hinder readability for not > much added gain. String de-duplication is done by the linker already. > The only value I see in the helpers is ensuring that the strings are > consistently written, and I think we can do so through other means. Just my opinion: I'm OK with these new helpers, but I am not too keen to apply all this to existing source code. I.e., for new code it is fine, but if we have to update all drivers every time a new cocci test is added, then that will not scale. Note that I never ran cocci in my build scripts, so this is a new check that we haven't set rules for or have much experience with. checkpatch just checks the patches, it doesn't force you to fix existing code. Some of the cocci tests are clearly checking for incorrect code, but others are for code improvements (i.e. the code was correct, it can just be done slightly better). It's the second category were I think that should only apply to new code, and not existing code. Regards, Hans > >> diff --git a/drivers/media/dvb-frontends/ascot2e.c b/drivers/media/dvb-frontends/ascot2e.c >> index 8c3eb5d69dda..ec7a718428fc 100644 >> --- a/drivers/media/dvb-frontends/ascot2e.c >> +++ b/drivers/media/dvb-frontends/ascot2e.c >> @@ -104,7 +104,7 @@ static void ascot2e_i2c_debug(struct ascot2e_priv *priv, >> u8 reg, u8 write, const u8 *data, u32 len) >> { >> dev_dbg(&priv->i2c->dev, "ascot2e: I2C %s reg 0x%02x size %d\n", >> - str_read_write(write == 0), reg, len); >> + str_write_read(write), reg, len); >> print_hex_dump_bytes("ascot2e: I2C data: ", >> DUMP_PREFIX_OFFSET, data, len); >> } >> diff --git a/drivers/media/dvb-frontends/cxd2841er.c b/drivers/media/dvb-frontends/cxd2841er.c >> index db684f314b47..d1b84cd9c510 100644 >> --- a/drivers/media/dvb-frontends/cxd2841er.c >> +++ b/drivers/media/dvb-frontends/cxd2841er.c >> @@ -206,7 +206,7 @@ static void cxd2841er_i2c_debug(struct cxd2841er_priv *priv, >> { >> dev_dbg(&priv->i2c->dev, >> "cxd2841er: I2C %s addr %02x reg 0x%02x size %d data %*ph\n", >> - str_read_write(write == 0), addr, reg, len, len, data); >> + str_write_read(write), addr, reg, len, len, data); >> } >> >> static int cxd2841er_write_regs(struct cxd2841er_priv *priv, >> diff --git a/drivers/media/dvb-frontends/helene.c b/drivers/media/dvb-frontends/helene.c >> index 52198cb49dba..b4527c141d9c 100644 >> --- a/drivers/media/dvb-frontends/helene.c >> +++ b/drivers/media/dvb-frontends/helene.c >> @@ -279,7 +279,7 @@ static void helene_i2c_debug(struct helene_priv *priv, >> u8 reg, u8 write, const u8 *data, u32 len) >> { >> dev_dbg(&priv->i2c->dev, "helene: I2C %s reg 0x%02x size %d\n", >> - str_read_write(write == 0), reg, len); >> + str_write_read(write), reg, len); >> print_hex_dump_bytes("helene: I2C data: ", >> DUMP_PREFIX_OFFSET, data, len); >> } >> diff --git a/drivers/media/dvb-frontends/horus3a.c b/drivers/media/dvb-frontends/horus3a.c >> index 84385079918c..10300ebf3ca0 100644 >> --- a/drivers/media/dvb-frontends/horus3a.c >> +++ b/drivers/media/dvb-frontends/horus3a.c >> @@ -38,7 +38,7 @@ static void horus3a_i2c_debug(struct horus3a_priv *priv, >> u8 reg, u8 write, const u8 *data, u32 len) >> { >> dev_dbg(&priv->i2c->dev, "horus3a: I2C %s reg 0x%02x size %d\n", >> - str_read_write(write == 0), reg, len); >> + str_write_read(write), reg, len); >> print_hex_dump_bytes("horus3a: I2C data: ", >> DUMP_PREFIX_OFFSET, data, len); >> } >> diff --git a/drivers/media/i2c/adv7842.c b/drivers/media/i2c/adv7842.c >> index ba174aa45afa..a43479c3ff03 100644 >> --- a/drivers/media/i2c/adv7842.c >> +++ b/drivers/media/i2c/adv7842.c >> @@ -2763,7 +2763,7 @@ static int adv7842_cp_log_status(struct v4l2_subdev *sd) >> str_true_false(io_read(sd, 0x6a) & 0x10)); >> } >> v4l2_info(sd, "CP free run: %s\n", >> - str_on_off(!!(cp_read(sd, 0xff) & 0x10))); >> + str_on_off(cp_read(sd, 0xff) & 0x10)); >> v4l2_info(sd, "Prim-mode = 0x%x, video std = 0x%x, v_freq = 0x%x\n", >> io_read(sd, 0x01) & 0x0f, io_read(sd, 0x00) & 0x3f, >> (io_read(sd, 0x01) & 0x70) >> 4); >> diff --git a/drivers/media/pci/saa7134/saa7134-cards.c b/drivers/media/pci/saa7134/saa7134-cards.c >> index 301b89e799d8..79cd61fb0205 100644 >> --- a/drivers/media/pci/saa7134/saa7134-cards.c >> +++ b/drivers/media/pci/saa7134/saa7134-cards.c >> @@ -7981,7 +7981,7 @@ int saa7134_board_init2(struct saa7134_dev *dev) >> rc = i2c_transfer(&dev->i2c_adap, &msg, 1); >> pr_info("%s: probe IR chip @ i2c 0x%02x: %s\n", >> dev->name, msg.addr, >> - str_yes_no(1 == rc)); >> + str_yes_no(rc == 1)); >> if (rc == 1) >> dev->has_remote = SAA7134_REMOTE_I2C; >> } >> diff --git a/drivers/media/pci/saa7134/saa7134-input.c b/drivers/media/pci/saa7134/saa7134-input.c >> index 90837ec6e70f..239f0b9d080a 100644 >> --- a/drivers/media/pci/saa7134/saa7134-input.c >> +++ b/drivers/media/pci/saa7134/saa7134-input.c >> @@ -895,7 +895,7 @@ void saa7134_probe_i2c_ir(struct saa7134_dev *dev) >> rc = i2c_transfer(&dev->i2c_adap, &msg_msi, 1); >> input_dbg("probe 0x%02x @ %s: %s\n", >> msg_msi.addr, dev->i2c_adap.name, >> - str_yes_no(1 == rc)); >> + str_yes_no(rc == 1)); >> break; >> case SAA7134_BOARD_SNAZIO_TVPVR_PRO: >> dev->init_data.name = "SnaZio* TVPVR PRO"; >> @@ -931,7 +931,7 @@ void saa7134_probe_i2c_ir(struct saa7134_dev *dev) >> rc = i2c_transfer(&dev->i2c_adap, &msg_msi, 1); >> input_dbg("probe 0x%02x @ %s: %s\n", >> msg_msi.addr, dev->i2c_adap.name, >> - str_yes_no(1 == rc)); >> + str_yes_no(rc == 1)); >> break; >> case SAA7134_BOARD_HAUPPAUGE_HVR1110: >> dev->init_data.name = saa7134_boards[dev->board].name; >> diff --git a/drivers/media/usb/pvrusb2/pvrusb2-ctrl.c b/drivers/media/usb/pvrusb2/pvrusb2-ctrl.c >> index 448c40caf363..b6c9bda214c8 100644 >> --- a/drivers/media/usb/pvrusb2/pvrusb2-ctrl.c >> +++ b/drivers/media/usb/pvrusb2/pvrusb2-ctrl.c >> @@ -521,7 +521,7 @@ int pvr2_ctrl_value_to_sym_internal(struct pvr2_ctrl *cptr, >> *len = scnprintf(buf,maxlen,"%d",val); >> ret = 0; >> } else if (cptr->info->type == pvr2_ctl_bool) { >> - *len = scnprintf(buf,maxlen,"%s",str_true_false(val)); >> + *len = scnprintf(buf, maxlen, "%s", str_true_false(val)); >> ret = 0; >> } else if (cptr->info->type == pvr2_ctl_enum) { >> const char * const *names; >> diff --git a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c >> index 96d3a0045fac..761d718478ca 100644 >> --- a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c >> +++ b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c >> @@ -338,7 +338,7 @@ static void trace_stbit(const char *name,int val) >> { >> pvr2_trace(PVR2_TRACE_STBITS, >> "State bit %s <-- %s", >> - name,str_true_false(val)); >> + name, str_true_false(val)); >> } >> >> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> >> --- >> Ricardo Ribalda (45): >> media: staging: ipu3: Use string_choices helpers >> media: staging: atomisp: Use string_choices helpers >> media: core: Use string_choices helpers >> media: pwc-ctl: Use string_choices helpers >> media: pvrusb2:Use string_choices helpers >> media: em28xx: Use string_choices helpers >> media: dvb-usb: Use string_choices helpers >> media: dvb-usb-v2: Use string_choices helpers >> media: cx231xx: Use string_choices helpers >> media: tuners: Use string_choices helpers >> media: rc: Use string_choices helpers >> media: dvb-frontends: Use string_choices helpers >> media: pci: cx23885: Use string_choices helpers >> media: saa7134: Use string_choices helpers >> media: amphion: Use string_choices helpers >> media: pci: ivtv: Use string_choices helpers >> media: bttv: Use string_choices helpers >> media: xilinx: Use string_choices helpers >> media: platform: ti: Use string_choices helpers >> media: st: Use string_choices helpers >> media: coda: Use string_choices helpers >> media: aspeed: Use string_choices helpers >> media: ipu6: Use string_choices helpers >> media: cx18: Use string_choices helpers >> media: cobalt: Use string_choices helpers >> media: videobuf2: Use string_choices helpers >> media: cec: Use string_choices helpers >> media: b2c2: Use string_choices helpers >> media: siano: Use string_choices helpers >> media: i2c: cx25840: Use string_choices helpers >> media: i2c: vpx3220: Use string_choices helpers >> media: i2c: tvp7002: Use string_choices helpers >> media: i2c: ths8200: Use string_choices helpers >> media: i2c: tda1997x: Use string_choices helpers >> media: i2c: tc358743: Use string_choices helpers >> media: i2c: st-mipid02: Use string_choices helpers >> media: i2c: msp3400: Use string_choices helpers >> media: i2c: max9286: Use string_choices helpers >> media: i2c: saa717x: Use string_choices helpers >> media: i2c: saa7127: Use string_choices helpers >> media: i2c: saa7115: Use string_choices helpers >> media: i2c: saa7110: Use string_choices helpers >> media: i2c: adv7842: Use string_choices helpers >> media: i2c: adv76xx: Use string_choices helpers >> media: i2c: adv7511: Use string_choices helpers >> >> drivers/media/cec/platform/cec-gpio/cec-gpio.c | 4 +- >> drivers/media/cec/usb/pulse8/pulse8-cec.c | 4 +- >> drivers/media/common/b2c2/flexcop-hw-filter.c | 4 +- >> drivers/media/common/siano/sms-cards.c | 3 +- >> drivers/media/common/videobuf2/videobuf2-core.c | 5 ++- >> drivers/media/dvb-frontends/ascot2e.c | 2 +- >> drivers/media/dvb-frontends/cx24120.c | 4 +- >> drivers/media/dvb-frontends/cxd2841er.c | 2 +- >> drivers/media/dvb-frontends/drxk_hard.c | 4 +- >> drivers/media/dvb-frontends/helene.c | 2 +- >> drivers/media/dvb-frontends/horus3a.c | 2 +- >> drivers/media/dvb-frontends/sp2.c | 2 +- >> drivers/media/i2c/adv7511-v4l2.c | 11 +++--- >> drivers/media/i2c/adv7604.c | 25 ++++++------ >> drivers/media/i2c/adv7842.c | 40 ++++++++++---------- >> drivers/media/i2c/cx25840/cx25840-core.c | 4 +- >> drivers/media/i2c/cx25840/cx25840-ir.c | 34 ++++++++--------- >> drivers/media/i2c/max9286.c | 2 +- >> drivers/media/i2c/msp3400-driver.c | 4 +- >> drivers/media/i2c/saa7110.c | 2 +- >> drivers/media/i2c/saa7115.c | 2 +- >> drivers/media/i2c/saa7127.c | 15 +++++--- >> drivers/media/i2c/saa717x.c | 2 +- >> drivers/media/i2c/st-mipid02.c | 2 +- >> drivers/media/i2c/tc358743.c | 44 ++++++++++------------ >> drivers/media/i2c/tda1997x.c | 6 +-- >> drivers/media/i2c/ths8200.c | 4 +- >> drivers/media/i2c/tvp7002.c | 2 +- >> drivers/media/i2c/vpx3220.c | 2 +- >> drivers/media/pci/bt8xx/bttv-cards.c | 16 ++++---- >> drivers/media/pci/bt8xx/bttv-driver.c | 6 +-- >> drivers/media/pci/cobalt/cobalt-driver.c | 2 +- >> drivers/media/pci/cx18/cx18-av-core.c | 4 +- >> drivers/media/pci/cx23885/altera-ci.c | 2 +- >> drivers/media/pci/cx23885/cimax2.c | 2 +- >> drivers/media/pci/cx23885/cx23888-ir.c | 36 +++++++++--------- >> drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c | 2 +- >> drivers/media/pci/ivtv/ivtvfb.c | 4 +- >> drivers/media/pci/saa7134/saa7134-cards.c | 2 +- >> drivers/media/pci/saa7134/saa7134-dvb.c | 2 +- >> drivers/media/pci/saa7134/saa7134-input.c | 6 +-- >> drivers/media/pci/saa7134/saa7134-video.c | 2 +- >> drivers/media/platform/amphion/venc.c | 2 +- >> drivers/media/platform/amphion/vpu_dbg.c | 2 +- >> drivers/media/platform/aspeed/aspeed-video.c | 4 +- >> drivers/media/platform/chips-media/coda/imx-vdoa.c | 3 +- >> drivers/media/platform/st/sti/hva/hva-debugfs.c | 6 +-- >> drivers/media/platform/ti/omap3isp/ispstat.c | 2 +- >> drivers/media/platform/xilinx/xilinx-csi2rxss.c | 18 ++++----- >> drivers/media/rc/ene_ir.c | 3 +- >> drivers/media/rc/mceusb.c | 3 +- >> drivers/media/rc/serial_ir.c | 5 ++- >> drivers/media/tuners/tda18250.c | 2 +- >> drivers/media/tuners/tda9887.c | 10 ++--- >> drivers/media/usb/cx231xx/cx231xx-i2c.c | 4 +- >> drivers/media/usb/cx231xx/cx231xx-video.c | 2 +- >> drivers/media/usb/dvb-usb-v2/az6007.c | 4 +- >> drivers/media/usb/dvb-usb-v2/dvb_usb_core.c | 4 +- >> drivers/media/usb/dvb-usb/af9005-fe.c | 4 +- >> drivers/media/usb/dvb-usb/dvb-usb-dvb.c | 6 +-- >> drivers/media/usb/dvb-usb/opera1.c | 8 ++-- >> drivers/media/usb/em28xx/em28xx-i2c.c | 4 +- >> drivers/media/usb/em28xx/em28xx-video.c | 2 +- >> drivers/media/usb/pvrusb2/pvrusb2-ctrl.c | 2 +- >> drivers/media/usb/pvrusb2/pvrusb2-debugifc.c | 3 +- >> drivers/media/usb/pvrusb2/pvrusb2-encoder.c | 5 +-- >> drivers/media/usb/pvrusb2/pvrusb2-hdw.c | 6 +-- >> drivers/media/usb/pvrusb2/pvrusb2-i2c-core.c | 3 +- >> drivers/media/usb/pwc/pwc-ctrl.c | 2 +- >> drivers/media/v4l2-core/v4l2-ctrls-core.c | 3 +- >> drivers/media/v4l2-core/v4l2-fwnode.c | 12 +++--- >> .../media/atomisp/pci/atomisp_compat_css20.c | 2 +- >> .../media/atomisp/pci/atomisp_csi2_bridge.c | 2 +- >> .../media/atomisp/pci/atomisp_gmin_platform.c | 4 +- >> drivers/staging/media/atomisp/pci/atomisp_v4l2.c | 4 +- >> .../media/atomisp/pci/runtime/binary/src/binary.c | 2 +- >> drivers/staging/media/atomisp/pci/sh_css.c | 2 +- >> drivers/staging/media/ipu3/ipu3-v4l2.c | 4 +- >> 78 files changed, 240 insertions(+), 239 deletions(-) >> --- >> base-commit: 9852d85ec9d492ebef56dc5f229416c925758edc >> change-id: 20240930-cocci-opportunity-40bca6a17c42 >
On Mon, Sep 30, 2024 at 02:38:22PM +0200, Hans Verkuil wrote: > On 30/09/2024 14:21, Laurent Pinchart wrote: > > Hi Ricardo, > > > > On Mon, Sep 30, 2024 at 12:03:55PM +0000, Ricardo Ribalda wrote: > >> include/linux/string_choices.h contains a set of helpers that can be > >> used instead of hard coding some strings. > >> > >> Cocci has located some places where the helpers can be used. This > >> patchset uses the diff generated by cocci, plus these changes: > > > > Personally I think most of those helpers just hinder readability for not > > much added gain. String de-duplication is done by the linker already. > > The only value I see in the helpers is ensuring that the strings are > > consistently written, and I think we can do so through other means. > > Just my opinion: I'm OK with these new helpers, Coding style opinions are personal preferences of course :-) My opinion is that this hinders readability for no benefit. > but I am not too keen to apply > all this to existing source code. I.e., for new code it is fine, but if we have > to update all drivers every time a new cocci test is added, then that will not > scale. > > Note that I never ran cocci in my build scripts, so this is a new check that > we haven't set rules for or have much experience with. > > checkpatch just checks the patches, it doesn't force you to fix existing code. > > Some of the cocci tests are clearly checking for incorrect code, but others are > for code improvements (i.e. the code was correct, it can just be done slightly > better). It's the second category were I think that should only apply to new code, > and not existing code. > > >> diff --git a/drivers/media/dvb-frontends/ascot2e.c b/drivers/media/dvb-frontends/ascot2e.c > >> index 8c3eb5d69dda..ec7a718428fc 100644 > >> --- a/drivers/media/dvb-frontends/ascot2e.c > >> +++ b/drivers/media/dvb-frontends/ascot2e.c > >> @@ -104,7 +104,7 @@ static void ascot2e_i2c_debug(struct ascot2e_priv *priv, > >> u8 reg, u8 write, const u8 *data, u32 len) > >> { > >> dev_dbg(&priv->i2c->dev, "ascot2e: I2C %s reg 0x%02x size %d\n", > >> - str_read_write(write == 0), reg, len); > >> + str_write_read(write), reg, len); > >> print_hex_dump_bytes("ascot2e: I2C data: ", > >> DUMP_PREFIX_OFFSET, data, len); > >> } > >> diff --git a/drivers/media/dvb-frontends/cxd2841er.c b/drivers/media/dvb-frontends/cxd2841er.c > >> index db684f314b47..d1b84cd9c510 100644 > >> --- a/drivers/media/dvb-frontends/cxd2841er.c > >> +++ b/drivers/media/dvb-frontends/cxd2841er.c > >> @@ -206,7 +206,7 @@ static void cxd2841er_i2c_debug(struct cxd2841er_priv *priv, > >> { > >> dev_dbg(&priv->i2c->dev, > >> "cxd2841er: I2C %s addr %02x reg 0x%02x size %d data %*ph\n", > >> - str_read_write(write == 0), addr, reg, len, len, data); > >> + str_write_read(write), addr, reg, len, len, data); > >> } > >> > >> static int cxd2841er_write_regs(struct cxd2841er_priv *priv, > >> diff --git a/drivers/media/dvb-frontends/helene.c b/drivers/media/dvb-frontends/helene.c > >> index 52198cb49dba..b4527c141d9c 100644 > >> --- a/drivers/media/dvb-frontends/helene.c > >> +++ b/drivers/media/dvb-frontends/helene.c > >> @@ -279,7 +279,7 @@ static void helene_i2c_debug(struct helene_priv *priv, > >> u8 reg, u8 write, const u8 *data, u32 len) > >> { > >> dev_dbg(&priv->i2c->dev, "helene: I2C %s reg 0x%02x size %d\n", > >> - str_read_write(write == 0), reg, len); > >> + str_write_read(write), reg, len); > >> print_hex_dump_bytes("helene: I2C data: ", > >> DUMP_PREFIX_OFFSET, data, len); > >> } > >> diff --git a/drivers/media/dvb-frontends/horus3a.c b/drivers/media/dvb-frontends/horus3a.c > >> index 84385079918c..10300ebf3ca0 100644 > >> --- a/drivers/media/dvb-frontends/horus3a.c > >> +++ b/drivers/media/dvb-frontends/horus3a.c > >> @@ -38,7 +38,7 @@ static void horus3a_i2c_debug(struct horus3a_priv *priv, > >> u8 reg, u8 write, const u8 *data, u32 len) > >> { > >> dev_dbg(&priv->i2c->dev, "horus3a: I2C %s reg 0x%02x size %d\n", > >> - str_read_write(write == 0), reg, len); > >> + str_write_read(write), reg, len); > >> print_hex_dump_bytes("horus3a: I2C data: ", > >> DUMP_PREFIX_OFFSET, data, len); > >> } > >> diff --git a/drivers/media/i2c/adv7842.c b/drivers/media/i2c/adv7842.c > >> index ba174aa45afa..a43479c3ff03 100644 > >> --- a/drivers/media/i2c/adv7842.c > >> +++ b/drivers/media/i2c/adv7842.c > >> @@ -2763,7 +2763,7 @@ static int adv7842_cp_log_status(struct v4l2_subdev *sd) > >> str_true_false(io_read(sd, 0x6a) & 0x10)); > >> } > >> v4l2_info(sd, "CP free run: %s\n", > >> - str_on_off(!!(cp_read(sd, 0xff) & 0x10))); > >> + str_on_off(cp_read(sd, 0xff) & 0x10)); > >> v4l2_info(sd, "Prim-mode = 0x%x, video std = 0x%x, v_freq = 0x%x\n", > >> io_read(sd, 0x01) & 0x0f, io_read(sd, 0x00) & 0x3f, > >> (io_read(sd, 0x01) & 0x70) >> 4); > >> diff --git a/drivers/media/pci/saa7134/saa7134-cards.c b/drivers/media/pci/saa7134/saa7134-cards.c > >> index 301b89e799d8..79cd61fb0205 100644 > >> --- a/drivers/media/pci/saa7134/saa7134-cards.c > >> +++ b/drivers/media/pci/saa7134/saa7134-cards.c > >> @@ -7981,7 +7981,7 @@ int saa7134_board_init2(struct saa7134_dev *dev) > >> rc = i2c_transfer(&dev->i2c_adap, &msg, 1); > >> pr_info("%s: probe IR chip @ i2c 0x%02x: %s\n", > >> dev->name, msg.addr, > >> - str_yes_no(1 == rc)); > >> + str_yes_no(rc == 1)); > >> if (rc == 1) > >> dev->has_remote = SAA7134_REMOTE_I2C; > >> } > >> diff --git a/drivers/media/pci/saa7134/saa7134-input.c b/drivers/media/pci/saa7134/saa7134-input.c > >> index 90837ec6e70f..239f0b9d080a 100644 > >> --- a/drivers/media/pci/saa7134/saa7134-input.c > >> +++ b/drivers/media/pci/saa7134/saa7134-input.c > >> @@ -895,7 +895,7 @@ void saa7134_probe_i2c_ir(struct saa7134_dev *dev) > >> rc = i2c_transfer(&dev->i2c_adap, &msg_msi, 1); > >> input_dbg("probe 0x%02x @ %s: %s\n", > >> msg_msi.addr, dev->i2c_adap.name, > >> - str_yes_no(1 == rc)); > >> + str_yes_no(rc == 1)); > >> break; > >> case SAA7134_BOARD_SNAZIO_TVPVR_PRO: > >> dev->init_data.name = "SnaZio* TVPVR PRO"; > >> @@ -931,7 +931,7 @@ void saa7134_probe_i2c_ir(struct saa7134_dev *dev) > >> rc = i2c_transfer(&dev->i2c_adap, &msg_msi, 1); > >> input_dbg("probe 0x%02x @ %s: %s\n", > >> msg_msi.addr, dev->i2c_adap.name, > >> - str_yes_no(1 == rc)); > >> + str_yes_no(rc == 1)); > >> break; > >> case SAA7134_BOARD_HAUPPAUGE_HVR1110: > >> dev->init_data.name = saa7134_boards[dev->board].name; > >> diff --git a/drivers/media/usb/pvrusb2/pvrusb2-ctrl.c b/drivers/media/usb/pvrusb2/pvrusb2-ctrl.c > >> index 448c40caf363..b6c9bda214c8 100644 > >> --- a/drivers/media/usb/pvrusb2/pvrusb2-ctrl.c > >> +++ b/drivers/media/usb/pvrusb2/pvrusb2-ctrl.c > >> @@ -521,7 +521,7 @@ int pvr2_ctrl_value_to_sym_internal(struct pvr2_ctrl *cptr, > >> *len = scnprintf(buf,maxlen,"%d",val); > >> ret = 0; > >> } else if (cptr->info->type == pvr2_ctl_bool) { > >> - *len = scnprintf(buf,maxlen,"%s",str_true_false(val)); > >> + *len = scnprintf(buf, maxlen, "%s", str_true_false(val)); > >> ret = 0; > >> } else if (cptr->info->type == pvr2_ctl_enum) { > >> const char * const *names; > >> diff --git a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c > >> index 96d3a0045fac..761d718478ca 100644 > >> --- a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c > >> +++ b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c > >> @@ -338,7 +338,7 @@ static void trace_stbit(const char *name,int val) > >> { > >> pvr2_trace(PVR2_TRACE_STBITS, > >> "State bit %s <-- %s", > >> - name,str_true_false(val)); > >> + name, str_true_false(val)); > >> } > >> > >> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> > >> --- > >> Ricardo Ribalda (45): > >> media: staging: ipu3: Use string_choices helpers > >> media: staging: atomisp: Use string_choices helpers > >> media: core: Use string_choices helpers > >> media: pwc-ctl: Use string_choices helpers > >> media: pvrusb2:Use string_choices helpers > >> media: em28xx: Use string_choices helpers > >> media: dvb-usb: Use string_choices helpers > >> media: dvb-usb-v2: Use string_choices helpers > >> media: cx231xx: Use string_choices helpers > >> media: tuners: Use string_choices helpers > >> media: rc: Use string_choices helpers > >> media: dvb-frontends: Use string_choices helpers > >> media: pci: cx23885: Use string_choices helpers > >> media: saa7134: Use string_choices helpers > >> media: amphion: Use string_choices helpers > >> media: pci: ivtv: Use string_choices helpers > >> media: bttv: Use string_choices helpers > >> media: xilinx: Use string_choices helpers > >> media: platform: ti: Use string_choices helpers > >> media: st: Use string_choices helpers > >> media: coda: Use string_choices helpers > >> media: aspeed: Use string_choices helpers > >> media: ipu6: Use string_choices helpers > >> media: cx18: Use string_choices helpers > >> media: cobalt: Use string_choices helpers > >> media: videobuf2: Use string_choices helpers > >> media: cec: Use string_choices helpers > >> media: b2c2: Use string_choices helpers > >> media: siano: Use string_choices helpers > >> media: i2c: cx25840: Use string_choices helpers > >> media: i2c: vpx3220: Use string_choices helpers > >> media: i2c: tvp7002: Use string_choices helpers > >> media: i2c: ths8200: Use string_choices helpers > >> media: i2c: tda1997x: Use string_choices helpers > >> media: i2c: tc358743: Use string_choices helpers > >> media: i2c: st-mipid02: Use string_choices helpers > >> media: i2c: msp3400: Use string_choices helpers > >> media: i2c: max9286: Use string_choices helpers > >> media: i2c: saa717x: Use string_choices helpers > >> media: i2c: saa7127: Use string_choices helpers > >> media: i2c: saa7115: Use string_choices helpers > >> media: i2c: saa7110: Use string_choices helpers > >> media: i2c: adv7842: Use string_choices helpers > >> media: i2c: adv76xx: Use string_choices helpers > >> media: i2c: adv7511: Use string_choices helpers > >> > >> drivers/media/cec/platform/cec-gpio/cec-gpio.c | 4 +- > >> drivers/media/cec/usb/pulse8/pulse8-cec.c | 4 +- > >> drivers/media/common/b2c2/flexcop-hw-filter.c | 4 +- > >> drivers/media/common/siano/sms-cards.c | 3 +- > >> drivers/media/common/videobuf2/videobuf2-core.c | 5 ++- > >> drivers/media/dvb-frontends/ascot2e.c | 2 +- > >> drivers/media/dvb-frontends/cx24120.c | 4 +- > >> drivers/media/dvb-frontends/cxd2841er.c | 2 +- > >> drivers/media/dvb-frontends/drxk_hard.c | 4 +- > >> drivers/media/dvb-frontends/helene.c | 2 +- > >> drivers/media/dvb-frontends/horus3a.c | 2 +- > >> drivers/media/dvb-frontends/sp2.c | 2 +- > >> drivers/media/i2c/adv7511-v4l2.c | 11 +++--- > >> drivers/media/i2c/adv7604.c | 25 ++++++------ > >> drivers/media/i2c/adv7842.c | 40 ++++++++++---------- > >> drivers/media/i2c/cx25840/cx25840-core.c | 4 +- > >> drivers/media/i2c/cx25840/cx25840-ir.c | 34 ++++++++--------- > >> drivers/media/i2c/max9286.c | 2 +- > >> drivers/media/i2c/msp3400-driver.c | 4 +- > >> drivers/media/i2c/saa7110.c | 2 +- > >> drivers/media/i2c/saa7115.c | 2 +- > >> drivers/media/i2c/saa7127.c | 15 +++++--- > >> drivers/media/i2c/saa717x.c | 2 +- > >> drivers/media/i2c/st-mipid02.c | 2 +- > >> drivers/media/i2c/tc358743.c | 44 ++++++++++------------ > >> drivers/media/i2c/tda1997x.c | 6 +-- > >> drivers/media/i2c/ths8200.c | 4 +- > >> drivers/media/i2c/tvp7002.c | 2 +- > >> drivers/media/i2c/vpx3220.c | 2 +- > >> drivers/media/pci/bt8xx/bttv-cards.c | 16 ++++---- > >> drivers/media/pci/bt8xx/bttv-driver.c | 6 +-- > >> drivers/media/pci/cobalt/cobalt-driver.c | 2 +- > >> drivers/media/pci/cx18/cx18-av-core.c | 4 +- > >> drivers/media/pci/cx23885/altera-ci.c | 2 +- > >> drivers/media/pci/cx23885/cimax2.c | 2 +- > >> drivers/media/pci/cx23885/cx23888-ir.c | 36 +++++++++--------- > >> drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c | 2 +- > >> drivers/media/pci/ivtv/ivtvfb.c | 4 +- > >> drivers/media/pci/saa7134/saa7134-cards.c | 2 +- > >> drivers/media/pci/saa7134/saa7134-dvb.c | 2 +- > >> drivers/media/pci/saa7134/saa7134-input.c | 6 +-- > >> drivers/media/pci/saa7134/saa7134-video.c | 2 +- > >> drivers/media/platform/amphion/venc.c | 2 +- > >> drivers/media/platform/amphion/vpu_dbg.c | 2 +- > >> drivers/media/platform/aspeed/aspeed-video.c | 4 +- > >> drivers/media/platform/chips-media/coda/imx-vdoa.c | 3 +- > >> drivers/media/platform/st/sti/hva/hva-debugfs.c | 6 +-- > >> drivers/media/platform/ti/omap3isp/ispstat.c | 2 +- > >> drivers/media/platform/xilinx/xilinx-csi2rxss.c | 18 ++++----- > >> drivers/media/rc/ene_ir.c | 3 +- > >> drivers/media/rc/mceusb.c | 3 +- > >> drivers/media/rc/serial_ir.c | 5 ++- > >> drivers/media/tuners/tda18250.c | 2 +- > >> drivers/media/tuners/tda9887.c | 10 ++--- > >> drivers/media/usb/cx231xx/cx231xx-i2c.c | 4 +- > >> drivers/media/usb/cx231xx/cx231xx-video.c | 2 +- > >> drivers/media/usb/dvb-usb-v2/az6007.c | 4 +- > >> drivers/media/usb/dvb-usb-v2/dvb_usb_core.c | 4 +- > >> drivers/media/usb/dvb-usb/af9005-fe.c | 4 +- > >> drivers/media/usb/dvb-usb/dvb-usb-dvb.c | 6 +-- > >> drivers/media/usb/dvb-usb/opera1.c | 8 ++-- > >> drivers/media/usb/em28xx/em28xx-i2c.c | 4 +- > >> drivers/media/usb/em28xx/em28xx-video.c | 2 +- > >> drivers/media/usb/pvrusb2/pvrusb2-ctrl.c | 2 +- > >> drivers/media/usb/pvrusb2/pvrusb2-debugifc.c | 3 +- > >> drivers/media/usb/pvrusb2/pvrusb2-encoder.c | 5 +-- > >> drivers/media/usb/pvrusb2/pvrusb2-hdw.c | 6 +-- > >> drivers/media/usb/pvrusb2/pvrusb2-i2c-core.c | 3 +- > >> drivers/media/usb/pwc/pwc-ctrl.c | 2 +- > >> drivers/media/v4l2-core/v4l2-ctrls-core.c | 3 +- > >> drivers/media/v4l2-core/v4l2-fwnode.c | 12 +++--- > >> .../media/atomisp/pci/atomisp_compat_css20.c | 2 +- > >> .../media/atomisp/pci/atomisp_csi2_bridge.c | 2 +- > >> .../media/atomisp/pci/atomisp_gmin_platform.c | 4 +- > >> drivers/staging/media/atomisp/pci/atomisp_v4l2.c | 4 +- > >> .../media/atomisp/pci/runtime/binary/src/binary.c | 2 +- > >> drivers/staging/media/atomisp/pci/sh_css.c | 2 +- > >> drivers/staging/media/ipu3/ipu3-v4l2.c | 4 +- > >> 78 files changed, 240 insertions(+), 239 deletions(-) > >> --- > >> base-commit: 9852d85ec9d492ebef56dc5f229416c925758edc > >> change-id: 20240930-cocci-opportunity-40bca6a17c42
On Mon, 30 Sept 2024 at 14:38, Hans Verkuil <hverkuil@xs4all.nl> wrote: > > On 30/09/2024 14:21, Laurent Pinchart wrote: > > Hi Ricardo, > > > > On Mon, Sep 30, 2024 at 12:03:55PM +0000, Ricardo Ribalda wrote: > >> include/linux/string_choices.h contains a set of helpers that can be > >> used instead of hard coding some strings. > >> > >> Cocci has located some places where the helpers can be used. This > >> patchset uses the diff generated by cocci, plus these changes: > > > > Personally I think most of those helpers just hinder readability for not > > much added gain. String de-duplication is done by the linker already. > > The only value I see in the helpers is ensuring that the strings are > > consistently written, and I think we can do so through other means. > > Just my opinion: I'm OK with these new helpers, but I am not too keen to apply > all this to existing source code. I.e., for new code it is fine, but if we have > to update all drivers every time a new cocci test is added, then that will not > scale. > > Note that I never ran cocci in my build scripts, so this is a new check that > we haven't set rules for or have much experience with. > > checkpatch just checks the patches, it doesn't force you to fix existing code. > > Some of the cocci tests are clearly checking for incorrect code, but others are > for code improvements (i.e. the code was correct, it can just be done slightly > better). It's the second category were I think that should only apply to new code, > and not existing code. > > Regards, Julia, correct me if I am wrong, but I believe that cocci does not have the ability to check only new code. It runs against files not diffs. I agree with you, a "improvement cocci" shall not force us to drop the pen and work full time to fix it :) When a new "improvement" cocci check is landed, we should update our "allowlist" immediately to ignore those warnings until we slowly improve our codebase to meet the standard. That way the CI is not affected. Regarding this patchset.... It is pretty big, but also pretty simple. I added my "extra changes" on the cover letter to ease the review. If you or someone else have time to review it... then it will be great if we land it. But if none has time for it, then it can be ignored. Regards! > > Hans > > > > >> diff --git a/drivers/media/dvb-frontends/ascot2e.c b/drivers/media/dvb-frontends/ascot2e.c > >> index 8c3eb5d69dda..ec7a718428fc 100644 > >> --- a/drivers/media/dvb-frontends/ascot2e.c > >> +++ b/drivers/media/dvb-frontends/ascot2e.c > >> @@ -104,7 +104,7 @@ static void ascot2e_i2c_debug(struct ascot2e_priv *priv, > >> u8 reg, u8 write, const u8 *data, u32 len) > >> { > >> dev_dbg(&priv->i2c->dev, "ascot2e: I2C %s reg 0x%02x size %d\n", > >> - str_read_write(write == 0), reg, len); > >> + str_write_read(write), reg, len); > >> print_hex_dump_bytes("ascot2e: I2C data: ", > >> DUMP_PREFIX_OFFSET, data, len); > >> } > >> diff --git a/drivers/media/dvb-frontends/cxd2841er.c b/drivers/media/dvb-frontends/cxd2841er.c > >> index db684f314b47..d1b84cd9c510 100644 > >> --- a/drivers/media/dvb-frontends/cxd2841er.c > >> +++ b/drivers/media/dvb-frontends/cxd2841er.c > >> @@ -206,7 +206,7 @@ static void cxd2841er_i2c_debug(struct cxd2841er_priv *priv, > >> { > >> dev_dbg(&priv->i2c->dev, > >> "cxd2841er: I2C %s addr %02x reg 0x%02x size %d data %*ph\n", > >> - str_read_write(write == 0), addr, reg, len, len, data); > >> + str_write_read(write), addr, reg, len, len, data); > >> } > >> > >> static int cxd2841er_write_regs(struct cxd2841er_priv *priv, > >> diff --git a/drivers/media/dvb-frontends/helene.c b/drivers/media/dvb-frontends/helene.c > >> index 52198cb49dba..b4527c141d9c 100644 > >> --- a/drivers/media/dvb-frontends/helene.c > >> +++ b/drivers/media/dvb-frontends/helene.c > >> @@ -279,7 +279,7 @@ static void helene_i2c_debug(struct helene_priv *priv, > >> u8 reg, u8 write, const u8 *data, u32 len) > >> { > >> dev_dbg(&priv->i2c->dev, "helene: I2C %s reg 0x%02x size %d\n", > >> - str_read_write(write == 0), reg, len); > >> + str_write_read(write), reg, len); > >> print_hex_dump_bytes("helene: I2C data: ", > >> DUMP_PREFIX_OFFSET, data, len); > >> } > >> diff --git a/drivers/media/dvb-frontends/horus3a.c b/drivers/media/dvb-frontends/horus3a.c > >> index 84385079918c..10300ebf3ca0 100644 > >> --- a/drivers/media/dvb-frontends/horus3a.c > >> +++ b/drivers/media/dvb-frontends/horus3a.c > >> @@ -38,7 +38,7 @@ static void horus3a_i2c_debug(struct horus3a_priv *priv, > >> u8 reg, u8 write, const u8 *data, u32 len) > >> { > >> dev_dbg(&priv->i2c->dev, "horus3a: I2C %s reg 0x%02x size %d\n", > >> - str_read_write(write == 0), reg, len); > >> + str_write_read(write), reg, len); > >> print_hex_dump_bytes("horus3a: I2C data: ", > >> DUMP_PREFIX_OFFSET, data, len); > >> } > >> diff --git a/drivers/media/i2c/adv7842.c b/drivers/media/i2c/adv7842.c > >> index ba174aa45afa..a43479c3ff03 100644 > >> --- a/drivers/media/i2c/adv7842.c > >> +++ b/drivers/media/i2c/adv7842.c > >> @@ -2763,7 +2763,7 @@ static int adv7842_cp_log_status(struct v4l2_subdev *sd) > >> str_true_false(io_read(sd, 0x6a) & 0x10)); > >> } > >> v4l2_info(sd, "CP free run: %s\n", > >> - str_on_off(!!(cp_read(sd, 0xff) & 0x10))); > >> + str_on_off(cp_read(sd, 0xff) & 0x10)); > >> v4l2_info(sd, "Prim-mode = 0x%x, video std = 0x%x, v_freq = 0x%x\n", > >> io_read(sd, 0x01) & 0x0f, io_read(sd, 0x00) & 0x3f, > >> (io_read(sd, 0x01) & 0x70) >> 4); > >> diff --git a/drivers/media/pci/saa7134/saa7134-cards.c b/drivers/media/pci/saa7134/saa7134-cards.c > >> index 301b89e799d8..79cd61fb0205 100644 > >> --- a/drivers/media/pci/saa7134/saa7134-cards.c > >> +++ b/drivers/media/pci/saa7134/saa7134-cards.c > >> @@ -7981,7 +7981,7 @@ int saa7134_board_init2(struct saa7134_dev *dev) > >> rc = i2c_transfer(&dev->i2c_adap, &msg, 1); > >> pr_info("%s: probe IR chip @ i2c 0x%02x: %s\n", > >> dev->name, msg.addr, > >> - str_yes_no(1 == rc)); > >> + str_yes_no(rc == 1)); > >> if (rc == 1) > >> dev->has_remote = SAA7134_REMOTE_I2C; > >> } > >> diff --git a/drivers/media/pci/saa7134/saa7134-input.c b/drivers/media/pci/saa7134/saa7134-input.c > >> index 90837ec6e70f..239f0b9d080a 100644 > >> --- a/drivers/media/pci/saa7134/saa7134-input.c > >> +++ b/drivers/media/pci/saa7134/saa7134-input.c > >> @@ -895,7 +895,7 @@ void saa7134_probe_i2c_ir(struct saa7134_dev *dev) > >> rc = i2c_transfer(&dev->i2c_adap, &msg_msi, 1); > >> input_dbg("probe 0x%02x @ %s: %s\n", > >> msg_msi.addr, dev->i2c_adap.name, > >> - str_yes_no(1 == rc)); > >> + str_yes_no(rc == 1)); > >> break; > >> case SAA7134_BOARD_SNAZIO_TVPVR_PRO: > >> dev->init_data.name = "SnaZio* TVPVR PRO"; > >> @@ -931,7 +931,7 @@ void saa7134_probe_i2c_ir(struct saa7134_dev *dev) > >> rc = i2c_transfer(&dev->i2c_adap, &msg_msi, 1); > >> input_dbg("probe 0x%02x @ %s: %s\n", > >> msg_msi.addr, dev->i2c_adap.name, > >> - str_yes_no(1 == rc)); > >> + str_yes_no(rc == 1)); > >> break; > >> case SAA7134_BOARD_HAUPPAUGE_HVR1110: > >> dev->init_data.name = saa7134_boards[dev->board].name; > >> diff --git a/drivers/media/usb/pvrusb2/pvrusb2-ctrl.c b/drivers/media/usb/pvrusb2/pvrusb2-ctrl.c > >> index 448c40caf363..b6c9bda214c8 100644 > >> --- a/drivers/media/usb/pvrusb2/pvrusb2-ctrl.c > >> +++ b/drivers/media/usb/pvrusb2/pvrusb2-ctrl.c > >> @@ -521,7 +521,7 @@ int pvr2_ctrl_value_to_sym_internal(struct pvr2_ctrl *cptr, > >> *len = scnprintf(buf,maxlen,"%d",val); > >> ret = 0; > >> } else if (cptr->info->type == pvr2_ctl_bool) { > >> - *len = scnprintf(buf,maxlen,"%s",str_true_false(val)); > >> + *len = scnprintf(buf, maxlen, "%s", str_true_false(val)); > >> ret = 0; > >> } else if (cptr->info->type == pvr2_ctl_enum) { > >> const char * const *names; > >> diff --git a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c > >> index 96d3a0045fac..761d718478ca 100644 > >> --- a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c > >> +++ b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c > >> @@ -338,7 +338,7 @@ static void trace_stbit(const char *name,int val) > >> { > >> pvr2_trace(PVR2_TRACE_STBITS, > >> "State bit %s <-- %s", > >> - name,str_true_false(val)); > >> + name, str_true_false(val)); > >> } > >> > >> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> > >> --- > >> Ricardo Ribalda (45): > >> media: staging: ipu3: Use string_choices helpers > >> media: staging: atomisp: Use string_choices helpers > >> media: core: Use string_choices helpers > >> media: pwc-ctl: Use string_choices helpers > >> media: pvrusb2:Use string_choices helpers > >> media: em28xx: Use string_choices helpers > >> media: dvb-usb: Use string_choices helpers > >> media: dvb-usb-v2: Use string_choices helpers > >> media: cx231xx: Use string_choices helpers > >> media: tuners: Use string_choices helpers > >> media: rc: Use string_choices helpers > >> media: dvb-frontends: Use string_choices helpers > >> media: pci: cx23885: Use string_choices helpers > >> media: saa7134: Use string_choices helpers > >> media: amphion: Use string_choices helpers > >> media: pci: ivtv: Use string_choices helpers > >> media: bttv: Use string_choices helpers > >> media: xilinx: Use string_choices helpers > >> media: platform: ti: Use string_choices helpers > >> media: st: Use string_choices helpers > >> media: coda: Use string_choices helpers > >> media: aspeed: Use string_choices helpers > >> media: ipu6: Use string_choices helpers > >> media: cx18: Use string_choices helpers > >> media: cobalt: Use string_choices helpers > >> media: videobuf2: Use string_choices helpers > >> media: cec: Use string_choices helpers > >> media: b2c2: Use string_choices helpers > >> media: siano: Use string_choices helpers > >> media: i2c: cx25840: Use string_choices helpers > >> media: i2c: vpx3220: Use string_choices helpers > >> media: i2c: tvp7002: Use string_choices helpers > >> media: i2c: ths8200: Use string_choices helpers > >> media: i2c: tda1997x: Use string_choices helpers > >> media: i2c: tc358743: Use string_choices helpers > >> media: i2c: st-mipid02: Use string_choices helpers > >> media: i2c: msp3400: Use string_choices helpers > >> media: i2c: max9286: Use string_choices helpers > >> media: i2c: saa717x: Use string_choices helpers > >> media: i2c: saa7127: Use string_choices helpers > >> media: i2c: saa7115: Use string_choices helpers > >> media: i2c: saa7110: Use string_choices helpers > >> media: i2c: adv7842: Use string_choices helpers > >> media: i2c: adv76xx: Use string_choices helpers > >> media: i2c: adv7511: Use string_choices helpers > >> > >> drivers/media/cec/platform/cec-gpio/cec-gpio.c | 4 +- > >> drivers/media/cec/usb/pulse8/pulse8-cec.c | 4 +- > >> drivers/media/common/b2c2/flexcop-hw-filter.c | 4 +- > >> drivers/media/common/siano/sms-cards.c | 3 +- > >> drivers/media/common/videobuf2/videobuf2-core.c | 5 ++- > >> drivers/media/dvb-frontends/ascot2e.c | 2 +- > >> drivers/media/dvb-frontends/cx24120.c | 4 +- > >> drivers/media/dvb-frontends/cxd2841er.c | 2 +- > >> drivers/media/dvb-frontends/drxk_hard.c | 4 +- > >> drivers/media/dvb-frontends/helene.c | 2 +- > >> drivers/media/dvb-frontends/horus3a.c | 2 +- > >> drivers/media/dvb-frontends/sp2.c | 2 +- > >> drivers/media/i2c/adv7511-v4l2.c | 11 +++--- > >> drivers/media/i2c/adv7604.c | 25 ++++++------ > >> drivers/media/i2c/adv7842.c | 40 ++++++++++---------- > >> drivers/media/i2c/cx25840/cx25840-core.c | 4 +- > >> drivers/media/i2c/cx25840/cx25840-ir.c | 34 ++++++++--------- > >> drivers/media/i2c/max9286.c | 2 +- > >> drivers/media/i2c/msp3400-driver.c | 4 +- > >> drivers/media/i2c/saa7110.c | 2 +- > >> drivers/media/i2c/saa7115.c | 2 +- > >> drivers/media/i2c/saa7127.c | 15 +++++--- > >> drivers/media/i2c/saa717x.c | 2 +- > >> drivers/media/i2c/st-mipid02.c | 2 +- > >> drivers/media/i2c/tc358743.c | 44 ++++++++++------------ > >> drivers/media/i2c/tda1997x.c | 6 +-- > >> drivers/media/i2c/ths8200.c | 4 +- > >> drivers/media/i2c/tvp7002.c | 2 +- > >> drivers/media/i2c/vpx3220.c | 2 +- > >> drivers/media/pci/bt8xx/bttv-cards.c | 16 ++++---- > >> drivers/media/pci/bt8xx/bttv-driver.c | 6 +-- > >> drivers/media/pci/cobalt/cobalt-driver.c | 2 +- > >> drivers/media/pci/cx18/cx18-av-core.c | 4 +- > >> drivers/media/pci/cx23885/altera-ci.c | 2 +- > >> drivers/media/pci/cx23885/cimax2.c | 2 +- > >> drivers/media/pci/cx23885/cx23888-ir.c | 36 +++++++++--------- > >> drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c | 2 +- > >> drivers/media/pci/ivtv/ivtvfb.c | 4 +- > >> drivers/media/pci/saa7134/saa7134-cards.c | 2 +- > >> drivers/media/pci/saa7134/saa7134-dvb.c | 2 +- > >> drivers/media/pci/saa7134/saa7134-input.c | 6 +-- > >> drivers/media/pci/saa7134/saa7134-video.c | 2 +- > >> drivers/media/platform/amphion/venc.c | 2 +- > >> drivers/media/platform/amphion/vpu_dbg.c | 2 +- > >> drivers/media/platform/aspeed/aspeed-video.c | 4 +- > >> drivers/media/platform/chips-media/coda/imx-vdoa.c | 3 +- > >> drivers/media/platform/st/sti/hva/hva-debugfs.c | 6 +-- > >> drivers/media/platform/ti/omap3isp/ispstat.c | 2 +- > >> drivers/media/platform/xilinx/xilinx-csi2rxss.c | 18 ++++----- > >> drivers/media/rc/ene_ir.c | 3 +- > >> drivers/media/rc/mceusb.c | 3 +- > >> drivers/media/rc/serial_ir.c | 5 ++- > >> drivers/media/tuners/tda18250.c | 2 +- > >> drivers/media/tuners/tda9887.c | 10 ++--- > >> drivers/media/usb/cx231xx/cx231xx-i2c.c | 4 +- > >> drivers/media/usb/cx231xx/cx231xx-video.c | 2 +- > >> drivers/media/usb/dvb-usb-v2/az6007.c | 4 +- > >> drivers/media/usb/dvb-usb-v2/dvb_usb_core.c | 4 +- > >> drivers/media/usb/dvb-usb/af9005-fe.c | 4 +- > >> drivers/media/usb/dvb-usb/dvb-usb-dvb.c | 6 +-- > >> drivers/media/usb/dvb-usb/opera1.c | 8 ++-- > >> drivers/media/usb/em28xx/em28xx-i2c.c | 4 +- > >> drivers/media/usb/em28xx/em28xx-video.c | 2 +- > >> drivers/media/usb/pvrusb2/pvrusb2-ctrl.c | 2 +- > >> drivers/media/usb/pvrusb2/pvrusb2-debugifc.c | 3 +- > >> drivers/media/usb/pvrusb2/pvrusb2-encoder.c | 5 +-- > >> drivers/media/usb/pvrusb2/pvrusb2-hdw.c | 6 +-- > >> drivers/media/usb/pvrusb2/pvrusb2-i2c-core.c | 3 +- > >> drivers/media/usb/pwc/pwc-ctrl.c | 2 +- > >> drivers/media/v4l2-core/v4l2-ctrls-core.c | 3 +- > >> drivers/media/v4l2-core/v4l2-fwnode.c | 12 +++--- > >> .../media/atomisp/pci/atomisp_compat_css20.c | 2 +- > >> .../media/atomisp/pci/atomisp_csi2_bridge.c | 2 +- > >> .../media/atomisp/pci/atomisp_gmin_platform.c | 4 +- > >> drivers/staging/media/atomisp/pci/atomisp_v4l2.c | 4 +- > >> .../media/atomisp/pci/runtime/binary/src/binary.c | 2 +- > >> drivers/staging/media/atomisp/pci/sh_css.c | 2 +- > >> drivers/staging/media/ipu3/ipu3-v4l2.c | 4 +- > >> 78 files changed, 240 insertions(+), 239 deletions(-) > >> --- > >> base-commit: 9852d85ec9d492ebef56dc5f229416c925758edc > >> change-id: 20240930-cocci-opportunity-40bca6a17c42 > > >
Em Mon, 30 Sep 2024 15:46:19 +0300 Laurent Pinchart <laurent.pinchart@ideasonboard.com> escreveu: > > >> Cocci has located some places where the helpers can be used. This > > >> patchset uses the diff generated by cocci, plus these changes: > > > > > > Personally I think most of those helpers just hinder readability for not > > > much added gain. String de-duplication is done by the linker already. > > > The only value I see in the helpers is ensuring that the strings are > > > consistently written, and I think we can do so through other means. > > > > Just my opinion: I'm OK with these new helpers, > > Coding style opinions are personal preferences of course :-) My opinion > is that this hinders readability for no benefit. Agreed. New code somewhat obfuscates what it does with no benefit except maybe saving a few bytes on each drive. Thanks, Mauro
On September 30, 2024 8:38:22 AM EDT, Hans Verkuil <hverkuil@xs4all.nl> wrote: >On 30/09/2024 14:21, Laurent Pinchart wrote: >> Hi Ricardo, >> >> On Mon, Sep 30, 2024 at 12:03:55PM +0000, Ricardo Ribalda wrote: >>> include/linux/string_choices.h contains a set of helpers that can be >>> used instead of hard coding some strings. >>> >>> Cocci has located some places where the helpers can be used. This >>> patchset uses the diff generated by cocci, plus these changes: >> >> Personally I think most of those helpers just hinder readability for not >> much added gain. String de-duplication is done by the linker already. >> The only value I see in the helpers is ensuring that the strings are >> consistently written, and I think we can do so through other means. > >Just my opinion: I'm OK with these new helpers, but I am not too keen to apply >all this to existing source code. I.e., for new code it is fine, but if we have >to update all drivers every time a new cocci test is added, then that will not >scale. > >Note that I never ran cocci in my build scripts, so this is a new check that >we haven't set rules for or have much experience with. > >checkpatch just checks the patches, it doesn't force you to fix existing code. > >Some of the cocci tests are clearly checking for incorrect code, but others are >for code improvements (i.e. the code was correct, it can just be done slightly >better). It's the second category were I think that should only apply to new code, >and not existing code. > >Regards, > > Hans > >> >>> diff --git a/drivers/media/dvb-frontends/ascot2e.c b/drivers/media/dvb-frontends/ascot2e.c >>> index 8c3eb5d69dda..ec7a718428fc 100644 >>> --- a/drivers/media/dvb-frontends/ascot2e.c >>> +++ b/drivers/media/dvb-frontends/ascot2e.c >>> @@ -104,7 +104,7 @@ static void ascot2e_i2c_debug(struct ascot2e_priv *priv, >>> u8 reg, u8 write, const u8 *data, u32 len) >>> { >>> dev_dbg(&priv->i2c->dev, "ascot2e: I2C %s reg 0x%02x size %d\n", >>> - str_read_write(write == 0), reg, len); >>> + str_write_read(write), reg, len); >>> print_hex_dump_bytes("ascot2e: I2C data: ", >>> DUMP_PREFIX_OFFSET, data, len); >>> } >>> diff --git a/drivers/media/dvb-frontends/cxd2841er.c b/drivers/media/dvb-frontends/cxd2841er.c >>> index db684f314b47..d1b84cd9c510 100644 >>> --- a/drivers/media/dvb-frontends/cxd2841er.c >>> +++ b/drivers/media/dvb-frontends/cxd2841er.c >>> @@ -206,7 +206,7 @@ static void cxd2841er_i2c_debug(struct cxd2841er_priv *priv, >>> { >>> dev_dbg(&priv->i2c->dev, >>> "cxd2841er: I2C %s addr %02x reg 0x%02x size %d data %*ph\n", >>> - str_read_write(write == 0), addr, reg, len, len, data); >>> + str_write_read(write), addr, reg, len, len, data); >>> } >>> >>> static int cxd2841er_write_regs(struct cxd2841er_priv *priv, >>> diff --git a/drivers/media/dvb-frontends/helene.c b/drivers/media/dvb-frontends/helene.c >>> index 52198cb49dba..b4527c141d9c 100644 >>> --- a/drivers/media/dvb-frontends/helene.c >>> +++ b/drivers/media/dvb-frontends/helene.c >>> @@ -279,7 +279,7 @@ static void helene_i2c_debug(struct helene_priv *priv, >>> u8 reg, u8 write, const u8 *data, u32 len) >>> { >>> dev_dbg(&priv->i2c->dev, "helene: I2C %s reg 0x%02x size %d\n", >>> - str_read_write(write == 0), reg, len); >>> + str_write_read(write), reg, len); >>> print_hex_dump_bytes("helene: I2C data: ", >>> DUMP_PREFIX_OFFSET, data, len); >>> } >>> diff --git a/drivers/media/dvb-frontends/horus3a.c b/drivers/media/dvb-frontends/horus3a.c >>> index 84385079918c..10300ebf3ca0 100644 >>> --- a/drivers/media/dvb-frontends/horus3a.c >>> +++ b/drivers/media/dvb-frontends/horus3a.c >>> @@ -38,7 +38,7 @@ static void horus3a_i2c_debug(struct horus3a_priv *priv, >>> u8 reg, u8 write, const u8 *data, u32 len) >>> { >>> dev_dbg(&priv->i2c->dev, "horus3a: I2C %s reg 0x%02x size %d\n", >>> - str_read_write(write == 0), reg, len); >>> + str_write_read(write), reg, len); >>> print_hex_dump_bytes("horus3a: I2C data: ", >>> DUMP_PREFIX_OFFSET, data, len); >>> } >>> diff --git a/drivers/media/i2c/adv7842.c b/drivers/media/i2c/adv7842.c >>> index ba174aa45afa..a43479c3ff03 100644 >>> --- a/drivers/media/i2c/adv7842.c >>> +++ b/drivers/media/i2c/adv7842.c >>> @@ -2763,7 +2763,7 @@ static int adv7842_cp_log_status(struct v4l2_subdev *sd) >>> str_true_false(io_read(sd, 0x6a) & 0x10)); >>> } >>> v4l2_info(sd, "CP free run: %s\n", >>> - str_on_off(!!(cp_read(sd, 0xff) & 0x10))); >>> + str_on_off(cp_read(sd, 0xff) & 0x10)); >>> v4l2_info(sd, "Prim-mode = 0x%x, video std = 0x%x, v_freq = 0x%x\n", >>> io_read(sd, 0x01) & 0x0f, io_read(sd, 0x00) & 0x3f, >>> (io_read(sd, 0x01) & 0x70) >> 4); >>> diff --git a/drivers/media/pci/saa7134/saa7134-cards.c b/drivers/media/pci/saa7134/saa7134-cards.c >>> index 301b89e799d8..79cd61fb0205 100644 >>> --- a/drivers/media/pci/saa7134/saa7134-cards.c >>> +++ b/drivers/media/pci/saa7134/saa7134-cards.c >>> @@ -7981,7 +7981,7 @@ int saa7134_board_init2(struct saa7134_dev *dev) >>> rc = i2c_transfer(&dev->i2c_adap, &msg, 1); >>> pr_info("%s: probe IR chip @ i2c 0x%02x: %s\n", >>> dev->name, msg.addr, >>> - str_yes_no(1 == rc)); >>> + str_yes_no(rc == 1)); >>> if (rc == 1) >>> dev->has_remote = SAA7134_REMOTE_I2C; >>> } >>> diff --git a/drivers/media/pci/saa7134/saa7134-input.c b/drivers/media/pci/saa7134/saa7134-input.c >>> index 90837ec6e70f..239f0b9d080a 100644 >>> --- a/drivers/media/pci/saa7134/saa7134-input.c >>> +++ b/drivers/media/pci/saa7134/saa7134-input.c >>> @@ -895,7 +895,7 @@ void saa7134_probe_i2c_ir(struct saa7134_dev *dev) >>> rc = i2c_transfer(&dev->i2c_adap, &msg_msi, 1); >>> input_dbg("probe 0x%02x @ %s: %s\n", >>> msg_msi.addr, dev->i2c_adap.name, >>> - str_yes_no(1 == rc)); >>> + str_yes_no(rc == 1)); >>> break; >>> case SAA7134_BOARD_SNAZIO_TVPVR_PRO: >>> dev->init_data.name = "SnaZio* TVPVR PRO"; >>> @@ -931,7 +931,7 @@ void saa7134_probe_i2c_ir(struct saa7134_dev *dev) >>> rc = i2c_transfer(&dev->i2c_adap, &msg_msi, 1); >>> input_dbg("probe 0x%02x @ %s: %s\n", >>> msg_msi.addr, dev->i2c_adap.name, >>> - str_yes_no(1 == rc)); >>> + str_yes_no(rc == 1)); >>> break; >>> case SAA7134_BOARD_HAUPPAUGE_HVR1110: >>> dev->init_data.name = saa7134_boards[dev->board].name; >>> diff --git a/drivers/media/usb/pvrusb2/pvrusb2-ctrl.c b/drivers/media/usb/pvrusb2/pvrusb2-ctrl.c >>> index 448c40caf363..b6c9bda214c8 100644 >>> --- a/drivers/media/usb/pvrusb2/pvrusb2-ctrl.c >>> +++ b/drivers/media/usb/pvrusb2/pvrusb2-ctrl.c >>> @@ -521,7 +521,7 @@ int pvr2_ctrl_value_to_sym_internal(struct pvr2_ctrl *cptr, >>> *len = scnprintf(buf,maxlen,"%d",val); >>> ret = 0; >>> } else if (cptr->info->type == pvr2_ctl_bool) { >>> - *len = scnprintf(buf,maxlen,"%s",str_true_false(val)); >>> + *len = scnprintf(buf, maxlen, "%s", str_true_false(val)); >>> ret = 0; >>> } else if (cptr->info->type == pvr2_ctl_enum) { >>> const char * const *names; >>> diff --git a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c >>> index 96d3a0045fac..761d718478ca 100644 >>> --- a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c >>> +++ b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c >>> @@ -338,7 +338,7 @@ static void trace_stbit(const char *name,int val) >>> { >>> pvr2_trace(PVR2_TRACE_STBITS, >>> "State bit %s <-- %s", >>> - name,str_true_false(val)); >>> + name, str_true_false(val)); >>> } >>> >>> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> >>> --- >>> Ricardo Ribalda (45): >>> media: staging: ipu3: Use string_choices helpers >>> media: staging: atomisp: Use string_choices helpers >>> media: core: Use string_choices helpers >>> media: pwc-ctl: Use string_choices helpers >>> media: pvrusb2:Use string_choices helpers >>> media: em28xx: Use string_choices helpers >>> media: dvb-usb: Use string_choices helpers >>> media: dvb-usb-v2: Use string_choices helpers >>> media: cx231xx: Use string_choices helpers >>> media: tuners: Use string_choices helpers >>> media: rc: Use string_choices helpers >>> media: dvb-frontends: Use string_choices helpers >>> media: pci: cx23885: Use string_choices helpers >>> media: saa7134: Use string_choices helpers >>> media: amphion: Use string_choices helpers >>> media: pci: ivtv: Use string_choices helpers >>> media: bttv: Use string_choices helpers >>> media: xilinx: Use string_choices helpers >>> media: platform: ti: Use string_choices helpers >>> media: st: Use string_choices helpers >>> media: coda: Use string_choices helpers >>> media: aspeed: Use string_choices helpers >>> media: ipu6: Use string_choices helpers >>> media: cx18: Use string_choices helpers >>> media: cobalt: Use string_choices helpers >>> media: videobuf2: Use string_choices helpers >>> media: cec: Use string_choices helpers >>> media: b2c2: Use string_choices helpers >>> media: siano: Use string_choices helpers >>> media: i2c: cx25840: Use string_choices helpers >>> media: i2c: vpx3220: Use string_choices helpers >>> media: i2c: tvp7002: Use string_choices helpers >>> media: i2c: ths8200: Use string_choices helpers >>> media: i2c: tda1997x: Use string_choices helpers >>> media: i2c: tc358743: Use string_choices helpers >>> media: i2c: st-mipid02: Use string_choices helpers >>> media: i2c: msp3400: Use string_choices helpers >>> media: i2c: max9286: Use string_choices helpers >>> media: i2c: saa717x: Use string_choices helpers >>> media: i2c: saa7127: Use string_choices helpers >>> media: i2c: saa7115: Use string_choices helpers >>> media: i2c: saa7110: Use string_choices helpers >>> media: i2c: adv7842: Use string_choices helpers >>> media: i2c: adv76xx: Use string_choices helpers >>> media: i2c: adv7511: Use string_choices helpers >>> >>> drivers/media/cec/platform/cec-gpio/cec-gpio.c | 4 +- >>> drivers/media/cec/usb/pulse8/pulse8-cec.c | 4 +- >>> drivers/media/common/b2c2/flexcop-hw-filter.c | 4 +- >>> drivers/media/common/siano/sms-cards.c | 3 +- >>> drivers/media/common/videobuf2/videobuf2-core.c | 5 ++- >>> drivers/media/dvb-frontends/ascot2e.c | 2 +- >>> drivers/media/dvb-frontends/cx24120.c | 4 +- >>> drivers/media/dvb-frontends/cxd2841er.c | 2 +- >>> drivers/media/dvb-frontends/drxk_hard.c | 4 +- >>> drivers/media/dvb-frontends/helene.c | 2 +- >>> drivers/media/dvb-frontends/horus3a.c | 2 +- >>> drivers/media/dvb-frontends/sp2.c | 2 +- >>> drivers/media/i2c/adv7511-v4l2.c | 11 +++--- >>> drivers/media/i2c/adv7604.c | 25 ++++++------ >>> drivers/media/i2c/adv7842.c | 40 ++++++++++---------- >>> drivers/media/i2c/cx25840/cx25840-core.c | 4 +- >>> drivers/media/i2c/cx25840/cx25840-ir.c | 34 ++++++++--------- >>> drivers/media/i2c/max9286.c | 2 +- >>> drivers/media/i2c/msp3400-driver.c | 4 +- >>> drivers/media/i2c/saa7110.c | 2 +- >>> drivers/media/i2c/saa7115.c | 2 +- >>> drivers/media/i2c/saa7127.c | 15 +++++--- >>> drivers/media/i2c/saa717x.c | 2 +- >>> drivers/media/i2c/st-mipid02.c | 2 +- >>> drivers/media/i2c/tc358743.c | 44 ++++++++++------------ >>> drivers/media/i2c/tda1997x.c | 6 +-- >>> drivers/media/i2c/ths8200.c | 4 +- >>> drivers/media/i2c/tvp7002.c | 2 +- >>> drivers/media/i2c/vpx3220.c | 2 +- >>> drivers/media/pci/bt8xx/bttv-cards.c | 16 ++++---- >>> drivers/media/pci/bt8xx/bttv-driver.c | 6 +-- >>> drivers/media/pci/cobalt/cobalt-driver.c | 2 +- >>> drivers/media/pci/cx18/cx18-av-core.c | 4 +- >>> drivers/media/pci/cx23885/altera-ci.c | 2 +- >>> drivers/media/pci/cx23885/cimax2.c | 2 +- >>> drivers/media/pci/cx23885/cx23888-ir.c | 36 +++++++++--------- >>> drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c | 2 +- >>> drivers/media/pci/ivtv/ivtvfb.c | 4 +- >>> drivers/media/pci/saa7134/saa7134-cards.c | 2 +- >>> drivers/media/pci/saa7134/saa7134-dvb.c | 2 +- >>> drivers/media/pci/saa7134/saa7134-input.c | 6 +-- >>> drivers/media/pci/saa7134/saa7134-video.c | 2 +- >>> drivers/media/platform/amphion/venc.c | 2 +- >>> drivers/media/platform/amphion/vpu_dbg.c | 2 +- >>> drivers/media/platform/aspeed/aspeed-video.c | 4 +- >>> drivers/media/platform/chips-media/coda/imx-vdoa.c | 3 +- >>> drivers/media/platform/st/sti/hva/hva-debugfs.c | 6 +-- >>> drivers/media/platform/ti/omap3isp/ispstat.c | 2 +- >>> drivers/media/platform/xilinx/xilinx-csi2rxss.c | 18 ++++----- >>> drivers/media/rc/ene_ir.c | 3 +- >>> drivers/media/rc/mceusb.c | 3 +- >>> drivers/media/rc/serial_ir.c | 5 ++- >>> drivers/media/tuners/tda18250.c | 2 +- >>> drivers/media/tuners/tda9887.c | 10 ++--- >>> drivers/media/usb/cx231xx/cx231xx-i2c.c | 4 +- >>> drivers/media/usb/cx231xx/cx231xx-video.c | 2 +- >>> drivers/media/usb/dvb-usb-v2/az6007.c | 4 +- >>> drivers/media/usb/dvb-usb-v2/dvb_usb_core.c | 4 +- >>> drivers/media/usb/dvb-usb/af9005-fe.c | 4 +- >>> drivers/media/usb/dvb-usb/dvb-usb-dvb.c | 6 +-- >>> drivers/media/usb/dvb-usb/opera1.c | 8 ++-- >>> drivers/media/usb/em28xx/em28xx-i2c.c | 4 +- >>> drivers/media/usb/em28xx/em28xx-video.c | 2 +- >>> drivers/media/usb/pvrusb2/pvrusb2-ctrl.c | 2 +- >>> drivers/media/usb/pvrusb2/pvrusb2-debugifc.c | 3 +- >>> drivers/media/usb/pvrusb2/pvrusb2-encoder.c | 5 +-- >>> drivers/media/usb/pvrusb2/pvrusb2-hdw.c | 6 +-- >>> drivers/media/usb/pvrusb2/pvrusb2-i2c-core.c | 3 +- >>> drivers/media/usb/pwc/pwc-ctrl.c | 2 +- >>> drivers/media/v4l2-core/v4l2-ctrls-core.c | 3 +- >>> drivers/media/v4l2-core/v4l2-fwnode.c | 12 +++--- >>> .../media/atomisp/pci/atomisp_compat_css20.c | 2 +- >>> .../media/atomisp/pci/atomisp_csi2_bridge.c | 2 +- >>> .../media/atomisp/pci/atomisp_gmin_platform.c | 4 +- >>> drivers/staging/media/atomisp/pci/atomisp_v4l2.c | 4 +- >>> .../media/atomisp/pci/runtime/binary/src/binary.c | 2 +- >>> drivers/staging/media/atomisp/pci/sh_css.c | 2 +- >>> drivers/staging/media/ipu3/ipu3-v4l2.c | 4 +- >>> 78 files changed, 240 insertions(+), 239 deletions(-) >>> --- >>> base-commit: 9852d85ec9d492ebef56dc5f229416c925758edc >>> change-id: 20240930-cocci-opportunity-40bca6a17c42 >> > Hi Ricardo, I'll go a little further than Hans and Laurent: I staunchly oppose what I call editorial changes: 'sed s/happy/glad/g' in code that has been working for a long time for which many do not have time or hardware to perform regression tests against. Please explain the return on investment for why this change should be applied? Help me to understand how that return outweighs the risk of introducing a bug? (No human being really code reviews sweeping editorial change like these thoroughly.) -Andy
On Mon, Sep 30, 2024 at 02:53:51PM +0200, Ricardo Ribalda wrote: > On Mon, 30 Sept 2024 at 14:38, Hans Verkuil wrote: > > On 30/09/2024 14:21, Laurent Pinchart wrote: > > > Hi Ricardo, > > > > > > On Mon, Sep 30, 2024 at 12:03:55PM +0000, Ricardo Ribalda wrote: > > >> include/linux/string_choices.h contains a set of helpers that can be > > >> used instead of hard coding some strings. > > >> > > >> Cocci has located some places where the helpers can be used. This > > >> patchset uses the diff generated by cocci, plus these changes: > > > > > > Personally I think most of those helpers just hinder readability for not > > > much added gain. String de-duplication is done by the linker already. > > > The only value I see in the helpers is ensuring that the strings are > > > consistently written, and I think we can do so through other means. > > > > Just my opinion: I'm OK with these new helpers, but I am not too keen to apply > > all this to existing source code. I.e., for new code it is fine, but if we have > > to update all drivers every time a new cocci test is added, then that will not > > scale. > > > > Note that I never ran cocci in my build scripts, so this is a new check that > > we haven't set rules for or have much experience with. > > > > checkpatch just checks the patches, it doesn't force you to fix existing code. > > > > Some of the cocci tests are clearly checking for incorrect code, but others are > > for code improvements (i.e. the code was correct, it can just be done slightly > > better). It's the second category were I think that should only apply to new code, > > and not existing code. > > > > Regards, > > Julia, correct me if I am wrong, but I believe that cocci does not > have the ability to check only new code. It runs against files not > diffs. > > I agree with you, a "improvement cocci" shall not force us to drop the > pen and work full time to fix it :) > When a new "improvement" cocci check is landed, we should update our > "allowlist" immediately to ignore those warnings until we slowly > improve our codebase to meet the standard. > That way the CI is not affected. > > Regarding this patchset.... It is pretty big, but also pretty simple. > I added my "extra changes" on the cover letter to ease the review. > > If you or someone else have time to review it... then it will be great > if we land it. But if none has time for it, then it can be ignored. More than just a matter of review time, I would prefer avoiding these changes in the drivers I maintain. > > >> diff --git a/drivers/media/dvb-frontends/ascot2e.c b/drivers/media/dvb-frontends/ascot2e.c > > >> index 8c3eb5d69dda..ec7a718428fc 100644 > > >> --- a/drivers/media/dvb-frontends/ascot2e.c > > >> +++ b/drivers/media/dvb-frontends/ascot2e.c > > >> @@ -104,7 +104,7 @@ static void ascot2e_i2c_debug(struct ascot2e_priv *priv, > > >> u8 reg, u8 write, const u8 *data, u32 len) > > >> { > > >> dev_dbg(&priv->i2c->dev, "ascot2e: I2C %s reg 0x%02x size %d\n", > > >> - str_read_write(write == 0), reg, len); > > >> + str_write_read(write), reg, len); > > >> print_hex_dump_bytes("ascot2e: I2C data: ", > > >> DUMP_PREFIX_OFFSET, data, len); > > >> } > > >> diff --git a/drivers/media/dvb-frontends/cxd2841er.c b/drivers/media/dvb-frontends/cxd2841er.c > > >> index db684f314b47..d1b84cd9c510 100644 > > >> --- a/drivers/media/dvb-frontends/cxd2841er.c > > >> +++ b/drivers/media/dvb-frontends/cxd2841er.c > > >> @@ -206,7 +206,7 @@ static void cxd2841er_i2c_debug(struct cxd2841er_priv *priv, > > >> { > > >> dev_dbg(&priv->i2c->dev, > > >> "cxd2841er: I2C %s addr %02x reg 0x%02x size %d data %*ph\n", > > >> - str_read_write(write == 0), addr, reg, len, len, data); > > >> + str_write_read(write), addr, reg, len, len, data); > > >> } > > >> > > >> static int cxd2841er_write_regs(struct cxd2841er_priv *priv, > > >> diff --git a/drivers/media/dvb-frontends/helene.c b/drivers/media/dvb-frontends/helene.c > > >> index 52198cb49dba..b4527c141d9c 100644 > > >> --- a/drivers/media/dvb-frontends/helene.c > > >> +++ b/drivers/media/dvb-frontends/helene.c > > >> @@ -279,7 +279,7 @@ static void helene_i2c_debug(struct helene_priv *priv, > > >> u8 reg, u8 write, const u8 *data, u32 len) > > >> { > > >> dev_dbg(&priv->i2c->dev, "helene: I2C %s reg 0x%02x size %d\n", > > >> - str_read_write(write == 0), reg, len); > > >> + str_write_read(write), reg, len); > > >> print_hex_dump_bytes("helene: I2C data: ", > > >> DUMP_PREFIX_OFFSET, data, len); > > >> } > > >> diff --git a/drivers/media/dvb-frontends/horus3a.c b/drivers/media/dvb-frontends/horus3a.c > > >> index 84385079918c..10300ebf3ca0 100644 > > >> --- a/drivers/media/dvb-frontends/horus3a.c > > >> +++ b/drivers/media/dvb-frontends/horus3a.c > > >> @@ -38,7 +38,7 @@ static void horus3a_i2c_debug(struct horus3a_priv *priv, > > >> u8 reg, u8 write, const u8 *data, u32 len) > > >> { > > >> dev_dbg(&priv->i2c->dev, "horus3a: I2C %s reg 0x%02x size %d\n", > > >> - str_read_write(write == 0), reg, len); > > >> + str_write_read(write), reg, len); > > >> print_hex_dump_bytes("horus3a: I2C data: ", > > >> DUMP_PREFIX_OFFSET, data, len); > > >> } > > >> diff --git a/drivers/media/i2c/adv7842.c b/drivers/media/i2c/adv7842.c > > >> index ba174aa45afa..a43479c3ff03 100644 > > >> --- a/drivers/media/i2c/adv7842.c > > >> +++ b/drivers/media/i2c/adv7842.c > > >> @@ -2763,7 +2763,7 @@ static int adv7842_cp_log_status(struct v4l2_subdev *sd) > > >> str_true_false(io_read(sd, 0x6a) & 0x10)); > > >> } > > >> v4l2_info(sd, "CP free run: %s\n", > > >> - str_on_off(!!(cp_read(sd, 0xff) & 0x10))); > > >> + str_on_off(cp_read(sd, 0xff) & 0x10)); > > >> v4l2_info(sd, "Prim-mode = 0x%x, video std = 0x%x, v_freq = 0x%x\n", > > >> io_read(sd, 0x01) & 0x0f, io_read(sd, 0x00) & 0x3f, > > >> (io_read(sd, 0x01) & 0x70) >> 4); > > >> diff --git a/drivers/media/pci/saa7134/saa7134-cards.c b/drivers/media/pci/saa7134/saa7134-cards.c > > >> index 301b89e799d8..79cd61fb0205 100644 > > >> --- a/drivers/media/pci/saa7134/saa7134-cards.c > > >> +++ b/drivers/media/pci/saa7134/saa7134-cards.c > > >> @@ -7981,7 +7981,7 @@ int saa7134_board_init2(struct saa7134_dev *dev) > > >> rc = i2c_transfer(&dev->i2c_adap, &msg, 1); > > >> pr_info("%s: probe IR chip @ i2c 0x%02x: %s\n", > > >> dev->name, msg.addr, > > >> - str_yes_no(1 == rc)); > > >> + str_yes_no(rc == 1)); > > >> if (rc == 1) > > >> dev->has_remote = SAA7134_REMOTE_I2C; > > >> } > > >> diff --git a/drivers/media/pci/saa7134/saa7134-input.c b/drivers/media/pci/saa7134/saa7134-input.c > > >> index 90837ec6e70f..239f0b9d080a 100644 > > >> --- a/drivers/media/pci/saa7134/saa7134-input.c > > >> +++ b/drivers/media/pci/saa7134/saa7134-input.c > > >> @@ -895,7 +895,7 @@ void saa7134_probe_i2c_ir(struct saa7134_dev *dev) > > >> rc = i2c_transfer(&dev->i2c_adap, &msg_msi, 1); > > >> input_dbg("probe 0x%02x @ %s: %s\n", > > >> msg_msi.addr, dev->i2c_adap.name, > > >> - str_yes_no(1 == rc)); > > >> + str_yes_no(rc == 1)); > > >> break; > > >> case SAA7134_BOARD_SNAZIO_TVPVR_PRO: > > >> dev->init_data.name = "SnaZio* TVPVR PRO"; > > >> @@ -931,7 +931,7 @@ void saa7134_probe_i2c_ir(struct saa7134_dev *dev) > > >> rc = i2c_transfer(&dev->i2c_adap, &msg_msi, 1); > > >> input_dbg("probe 0x%02x @ %s: %s\n", > > >> msg_msi.addr, dev->i2c_adap.name, > > >> - str_yes_no(1 == rc)); > > >> + str_yes_no(rc == 1)); > > >> break; > > >> case SAA7134_BOARD_HAUPPAUGE_HVR1110: > > >> dev->init_data.name = saa7134_boards[dev->board].name; > > >> diff --git a/drivers/media/usb/pvrusb2/pvrusb2-ctrl.c b/drivers/media/usb/pvrusb2/pvrusb2-ctrl.c > > >> index 448c40caf363..b6c9bda214c8 100644 > > >> --- a/drivers/media/usb/pvrusb2/pvrusb2-ctrl.c > > >> +++ b/drivers/media/usb/pvrusb2/pvrusb2-ctrl.c > > >> @@ -521,7 +521,7 @@ int pvr2_ctrl_value_to_sym_internal(struct pvr2_ctrl *cptr, > > >> *len = scnprintf(buf,maxlen,"%d",val); > > >> ret = 0; > > >> } else if (cptr->info->type == pvr2_ctl_bool) { > > >> - *len = scnprintf(buf,maxlen,"%s",str_true_false(val)); > > >> + *len = scnprintf(buf, maxlen, "%s", str_true_false(val)); > > >> ret = 0; > > >> } else if (cptr->info->type == pvr2_ctl_enum) { > > >> const char * const *names; > > >> diff --git a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c > > >> index 96d3a0045fac..761d718478ca 100644 > > >> --- a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c > > >> +++ b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c > > >> @@ -338,7 +338,7 @@ static void trace_stbit(const char *name,int val) > > >> { > > >> pvr2_trace(PVR2_TRACE_STBITS, > > >> "State bit %s <-- %s", > > >> - name,str_true_false(val)); > > >> + name, str_true_false(val)); > > >> } > > >> > > >> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> > > >> --- > > >> Ricardo Ribalda (45): > > >> media: staging: ipu3: Use string_choices helpers > > >> media: staging: atomisp: Use string_choices helpers > > >> media: core: Use string_choices helpers > > >> media: pwc-ctl: Use string_choices helpers > > >> media: pvrusb2:Use string_choices helpers > > >> media: em28xx: Use string_choices helpers > > >> media: dvb-usb: Use string_choices helpers > > >> media: dvb-usb-v2: Use string_choices helpers > > >> media: cx231xx: Use string_choices helpers > > >> media: tuners: Use string_choices helpers > > >> media: rc: Use string_choices helpers > > >> media: dvb-frontends: Use string_choices helpers > > >> media: pci: cx23885: Use string_choices helpers > > >> media: saa7134: Use string_choices helpers > > >> media: amphion: Use string_choices helpers > > >> media: pci: ivtv: Use string_choices helpers > > >> media: bttv: Use string_choices helpers > > >> media: xilinx: Use string_choices helpers > > >> media: platform: ti: Use string_choices helpers > > >> media: st: Use string_choices helpers > > >> media: coda: Use string_choices helpers > > >> media: aspeed: Use string_choices helpers > > >> media: ipu6: Use string_choices helpers > > >> media: cx18: Use string_choices helpers > > >> media: cobalt: Use string_choices helpers > > >> media: videobuf2: Use string_choices helpers > > >> media: cec: Use string_choices helpers > > >> media: b2c2: Use string_choices helpers > > >> media: siano: Use string_choices helpers > > >> media: i2c: cx25840: Use string_choices helpers > > >> media: i2c: vpx3220: Use string_choices helpers > > >> media: i2c: tvp7002: Use string_choices helpers > > >> media: i2c: ths8200: Use string_choices helpers > > >> media: i2c: tda1997x: Use string_choices helpers > > >> media: i2c: tc358743: Use string_choices helpers > > >> media: i2c: st-mipid02: Use string_choices helpers > > >> media: i2c: msp3400: Use string_choices helpers > > >> media: i2c: max9286: Use string_choices helpers > > >> media: i2c: saa717x: Use string_choices helpers > > >> media: i2c: saa7127: Use string_choices helpers > > >> media: i2c: saa7115: Use string_choices helpers > > >> media: i2c: saa7110: Use string_choices helpers > > >> media: i2c: adv7842: Use string_choices helpers > > >> media: i2c: adv76xx: Use string_choices helpers > > >> media: i2c: adv7511: Use string_choices helpers > > >> > > >> drivers/media/cec/platform/cec-gpio/cec-gpio.c | 4 +- > > >> drivers/media/cec/usb/pulse8/pulse8-cec.c | 4 +- > > >> drivers/media/common/b2c2/flexcop-hw-filter.c | 4 +- > > >> drivers/media/common/siano/sms-cards.c | 3 +- > > >> drivers/media/common/videobuf2/videobuf2-core.c | 5 ++- > > >> drivers/media/dvb-frontends/ascot2e.c | 2 +- > > >> drivers/media/dvb-frontends/cx24120.c | 4 +- > > >> drivers/media/dvb-frontends/cxd2841er.c | 2 +- > > >> drivers/media/dvb-frontends/drxk_hard.c | 4 +- > > >> drivers/media/dvb-frontends/helene.c | 2 +- > > >> drivers/media/dvb-frontends/horus3a.c | 2 +- > > >> drivers/media/dvb-frontends/sp2.c | 2 +- > > >> drivers/media/i2c/adv7511-v4l2.c | 11 +++--- > > >> drivers/media/i2c/adv7604.c | 25 ++++++------ > > >> drivers/media/i2c/adv7842.c | 40 ++++++++++---------- > > >> drivers/media/i2c/cx25840/cx25840-core.c | 4 +- > > >> drivers/media/i2c/cx25840/cx25840-ir.c | 34 ++++++++--------- > > >> drivers/media/i2c/max9286.c | 2 +- > > >> drivers/media/i2c/msp3400-driver.c | 4 +- > > >> drivers/media/i2c/saa7110.c | 2 +- > > >> drivers/media/i2c/saa7115.c | 2 +- > > >> drivers/media/i2c/saa7127.c | 15 +++++--- > > >> drivers/media/i2c/saa717x.c | 2 +- > > >> drivers/media/i2c/st-mipid02.c | 2 +- > > >> drivers/media/i2c/tc358743.c | 44 ++++++++++------------ > > >> drivers/media/i2c/tda1997x.c | 6 +-- > > >> drivers/media/i2c/ths8200.c | 4 +- > > >> drivers/media/i2c/tvp7002.c | 2 +- > > >> drivers/media/i2c/vpx3220.c | 2 +- > > >> drivers/media/pci/bt8xx/bttv-cards.c | 16 ++++---- > > >> drivers/media/pci/bt8xx/bttv-driver.c | 6 +-- > > >> drivers/media/pci/cobalt/cobalt-driver.c | 2 +- > > >> drivers/media/pci/cx18/cx18-av-core.c | 4 +- > > >> drivers/media/pci/cx23885/altera-ci.c | 2 +- > > >> drivers/media/pci/cx23885/cimax2.c | 2 +- > > >> drivers/media/pci/cx23885/cx23888-ir.c | 36 +++++++++--------- > > >> drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c | 2 +- > > >> drivers/media/pci/ivtv/ivtvfb.c | 4 +- > > >> drivers/media/pci/saa7134/saa7134-cards.c | 2 +- > > >> drivers/media/pci/saa7134/saa7134-dvb.c | 2 +- > > >> drivers/media/pci/saa7134/saa7134-input.c | 6 +-- > > >> drivers/media/pci/saa7134/saa7134-video.c | 2 +- > > >> drivers/media/platform/amphion/venc.c | 2 +- > > >> drivers/media/platform/amphion/vpu_dbg.c | 2 +- > > >> drivers/media/platform/aspeed/aspeed-video.c | 4 +- > > >> drivers/media/platform/chips-media/coda/imx-vdoa.c | 3 +- > > >> drivers/media/platform/st/sti/hva/hva-debugfs.c | 6 +-- > > >> drivers/media/platform/ti/omap3isp/ispstat.c | 2 +- > > >> drivers/media/platform/xilinx/xilinx-csi2rxss.c | 18 ++++----- > > >> drivers/media/rc/ene_ir.c | 3 +- > > >> drivers/media/rc/mceusb.c | 3 +- > > >> drivers/media/rc/serial_ir.c | 5 ++- > > >> drivers/media/tuners/tda18250.c | 2 +- > > >> drivers/media/tuners/tda9887.c | 10 ++--- > > >> drivers/media/usb/cx231xx/cx231xx-i2c.c | 4 +- > > >> drivers/media/usb/cx231xx/cx231xx-video.c | 2 +- > > >> drivers/media/usb/dvb-usb-v2/az6007.c | 4 +- > > >> drivers/media/usb/dvb-usb-v2/dvb_usb_core.c | 4 +- > > >> drivers/media/usb/dvb-usb/af9005-fe.c | 4 +- > > >> drivers/media/usb/dvb-usb/dvb-usb-dvb.c | 6 +-- > > >> drivers/media/usb/dvb-usb/opera1.c | 8 ++-- > > >> drivers/media/usb/em28xx/em28xx-i2c.c | 4 +- > > >> drivers/media/usb/em28xx/em28xx-video.c | 2 +- > > >> drivers/media/usb/pvrusb2/pvrusb2-ctrl.c | 2 +- > > >> drivers/media/usb/pvrusb2/pvrusb2-debugifc.c | 3 +- > > >> drivers/media/usb/pvrusb2/pvrusb2-encoder.c | 5 +-- > > >> drivers/media/usb/pvrusb2/pvrusb2-hdw.c | 6 +-- > > >> drivers/media/usb/pvrusb2/pvrusb2-i2c-core.c | 3 +- > > >> drivers/media/usb/pwc/pwc-ctrl.c | 2 +- > > >> drivers/media/v4l2-core/v4l2-ctrls-core.c | 3 +- > > >> drivers/media/v4l2-core/v4l2-fwnode.c | 12 +++--- > > >> .../media/atomisp/pci/atomisp_compat_css20.c | 2 +- > > >> .../media/atomisp/pci/atomisp_csi2_bridge.c | 2 +- > > >> .../media/atomisp/pci/atomisp_gmin_platform.c | 4 +- > > >> drivers/staging/media/atomisp/pci/atomisp_v4l2.c | 4 +- > > >> .../media/atomisp/pci/runtime/binary/src/binary.c | 2 +- > > >> drivers/staging/media/atomisp/pci/sh_css.c | 2 +- > > >> drivers/staging/media/ipu3/ipu3-v4l2.c | 4 +- > > >> 78 files changed, 240 insertions(+), 239 deletions(-) > > >> --- > > >> base-commit: 9852d85ec9d492ebef56dc5f229416c925758edc > > >> change-id: 20240930-cocci-opportunity-40bca6a17c42
On Mon, Sep 30, 2024 at 08:57:44AM -0400, Andy Walls wrote:
> No human being really code reviews sweeping editorial change like these thoroughly.
I wonder if there is some way to verify that people are actually running the
Coccinelle script that they say they are without doing anything extra on the
side.
I use a script to filter out mechanical changes.
https://github.com/error27/rename_rev
It's kind of a pain in the butt to review something like this. The command
would be something like:
rename_rev.pl -e 's/(.*?,|^\W+)(.*) \? "enabled" : "disabled"/$1str_enabled_disabled($2)/' -e 's/(.*?,|^\W+)(.*) \? "enable" : "disable"/$1str_enable_disable($2)/' -e 's/(.*,|^\W+)(.*) \? "low" : "high"/$1str_low_high($2)/' -e 's/(.*,|^\W+)(.*) \? "on" : "off"/$1str_on_off($2)/' -e 's/(.*,|^\W+)(.*) \? "true" : "false"/$1str_true_false($2)/' -e 's/(.*,|^\W+)(.*) \? "high" : "low"/$1str_high_low($2)/' -e 's/(.*,|^\W+)(.*) \? "read" : "write"/$1str_read_write($2)/'
For every email in the series there was another new str_foo_bar() function so
the command line kept getting longer and longer. It doesn't work perfectly, but
it's often good enough so I can spot the interesting bits.
regards,
dan carpenter
On Mon, 30 Sep 2024, Ricardo Ribalda wrote: > On Mon, 30 Sept 2024 at 14:38, Hans Verkuil <hverkuil@xs4all.nl> wrote: > > > > On 30/09/2024 14:21, Laurent Pinchart wrote: > > > Hi Ricardo, > > > > > > On Mon, Sep 30, 2024 at 12:03:55PM +0000, Ricardo Ribalda wrote: > > >> include/linux/string_choices.h contains a set of helpers that can be > > >> used instead of hard coding some strings. > > >> > > >> Cocci has located some places where the helpers can be used. This > > >> patchset uses the diff generated by cocci, plus these changes: > > > > > > Personally I think most of those helpers just hinder readability for not > > > much added gain. String de-duplication is done by the linker already. > > > The only value I see in the helpers is ensuring that the strings are > > > consistently written, and I think we can do so through other means. > > > > Just my opinion: I'm OK with these new helpers, but I am not too keen to apply > > all this to existing source code. I.e., for new code it is fine, but if we have > > to update all drivers every time a new cocci test is added, then that will not > > scale. > > > > Note that I never ran cocci in my build scripts, so this is a new check that > > we haven't set rules for or have much experience with. > > > > checkpatch just checks the patches, it doesn't force you to fix existing code. > > > > Some of the cocci tests are clearly checking for incorrect code, but others are > > for code improvements (i.e. the code was correct, it can just be done slightly > > better). It's the second category were I think that should only apply to new code, > > and not existing code. > > > > Regards, > > Julia, correct me if I am wrong, but I believe that cocci does not > have the ability to check only new code. It runs against files not > diffs. Coccinelle runs on files normally. There is an option --use-patch-diff where you can give a commit id, and it should only work on the files mentioned in that commit, but it doesn't seem to focus on only the new code in that commit. julia > > I agree with you, a "improvement cocci" shall not force us to drop the > pen and work full time to fix it :) > When a new "improvement" cocci check is landed, we should update our > "allowlist" immediately to ignore those warnings until we slowly > improve our codebase to meet the standard. > That way the CI is not affected. > > Regarding this patchset.... It is pretty big, but also pretty simple. > I added my "extra changes" on the cover letter to ease the review. > > If you or someone else have time to review it... then it will be great > if we land it. But if none has time for it, then it can be ignored. > > > Regards! > > > > > Hans > > > > > > > >> diff --git a/drivers/media/dvb-frontends/ascot2e.c b/drivers/media/dvb-frontends/ascot2e.c > > >> index 8c3eb5d69dda..ec7a718428fc 100644 > > >> --- a/drivers/media/dvb-frontends/ascot2e.c > > >> +++ b/drivers/media/dvb-frontends/ascot2e.c > > >> @@ -104,7 +104,7 @@ static void ascot2e_i2c_debug(struct ascot2e_priv *priv, > > >> u8 reg, u8 write, const u8 *data, u32 len) > > >> { > > >> dev_dbg(&priv->i2c->dev, "ascot2e: I2C %s reg 0x%02x size %d\n", > > >> - str_read_write(write == 0), reg, len); > > >> + str_write_read(write), reg, len); > > >> print_hex_dump_bytes("ascot2e: I2C data: ", > > >> DUMP_PREFIX_OFFSET, data, len); > > >> } > > >> diff --git a/drivers/media/dvb-frontends/cxd2841er.c b/drivers/media/dvb-frontends/cxd2841er.c > > >> index db684f314b47..d1b84cd9c510 100644 > > >> --- a/drivers/media/dvb-frontends/cxd2841er.c > > >> +++ b/drivers/media/dvb-frontends/cxd2841er.c > > >> @@ -206,7 +206,7 @@ static void cxd2841er_i2c_debug(struct cxd2841er_priv *priv, > > >> { > > >> dev_dbg(&priv->i2c->dev, > > >> "cxd2841er: I2C %s addr %02x reg 0x%02x size %d data %*ph\n", > > >> - str_read_write(write == 0), addr, reg, len, len, data); > > >> + str_write_read(write), addr, reg, len, len, data); > > >> } > > >> > > >> static int cxd2841er_write_regs(struct cxd2841er_priv *priv, > > >> diff --git a/drivers/media/dvb-frontends/helene.c b/drivers/media/dvb-frontends/helene.c > > >> index 52198cb49dba..b4527c141d9c 100644 > > >> --- a/drivers/media/dvb-frontends/helene.c > > >> +++ b/drivers/media/dvb-frontends/helene.c > > >> @@ -279,7 +279,7 @@ static void helene_i2c_debug(struct helene_priv *priv, > > >> u8 reg, u8 write, const u8 *data, u32 len) > > >> { > > >> dev_dbg(&priv->i2c->dev, "helene: I2C %s reg 0x%02x size %d\n", > > >> - str_read_write(write == 0), reg, len); > > >> + str_write_read(write), reg, len); > > >> print_hex_dump_bytes("helene: I2C data: ", > > >> DUMP_PREFIX_OFFSET, data, len); > > >> } > > >> diff --git a/drivers/media/dvb-frontends/horus3a.c b/drivers/media/dvb-frontends/horus3a.c > > >> index 84385079918c..10300ebf3ca0 100644 > > >> --- a/drivers/media/dvb-frontends/horus3a.c > > >> +++ b/drivers/media/dvb-frontends/horus3a.c > > >> @@ -38,7 +38,7 @@ static void horus3a_i2c_debug(struct horus3a_priv *priv, > > >> u8 reg, u8 write, const u8 *data, u32 len) > > >> { > > >> dev_dbg(&priv->i2c->dev, "horus3a: I2C %s reg 0x%02x size %d\n", > > >> - str_read_write(write == 0), reg, len); > > >> + str_write_read(write), reg, len); > > >> print_hex_dump_bytes("horus3a: I2C data: ", > > >> DUMP_PREFIX_OFFSET, data, len); > > >> } > > >> diff --git a/drivers/media/i2c/adv7842.c b/drivers/media/i2c/adv7842.c > > >> index ba174aa45afa..a43479c3ff03 100644 > > >> --- a/drivers/media/i2c/adv7842.c > > >> +++ b/drivers/media/i2c/adv7842.c > > >> @@ -2763,7 +2763,7 @@ static int adv7842_cp_log_status(struct v4l2_subdev *sd) > > >> str_true_false(io_read(sd, 0x6a) & 0x10)); > > >> } > > >> v4l2_info(sd, "CP free run: %s\n", > > >> - str_on_off(!!(cp_read(sd, 0xff) & 0x10))); > > >> + str_on_off(cp_read(sd, 0xff) & 0x10)); > > >> v4l2_info(sd, "Prim-mode = 0x%x, video std = 0x%x, v_freq = 0x%x\n", > > >> io_read(sd, 0x01) & 0x0f, io_read(sd, 0x00) & 0x3f, > > >> (io_read(sd, 0x01) & 0x70) >> 4); > > >> diff --git a/drivers/media/pci/saa7134/saa7134-cards.c b/drivers/media/pci/saa7134/saa7134-cards.c > > >> index 301b89e799d8..79cd61fb0205 100644 > > >> --- a/drivers/media/pci/saa7134/saa7134-cards.c > > >> +++ b/drivers/media/pci/saa7134/saa7134-cards.c > > >> @@ -7981,7 +7981,7 @@ int saa7134_board_init2(struct saa7134_dev *dev) > > >> rc = i2c_transfer(&dev->i2c_adap, &msg, 1); > > >> pr_info("%s: probe IR chip @ i2c 0x%02x: %s\n", > > >> dev->name, msg.addr, > > >> - str_yes_no(1 == rc)); > > >> + str_yes_no(rc == 1)); > > >> if (rc == 1) > > >> dev->has_remote = SAA7134_REMOTE_I2C; > > >> } > > >> diff --git a/drivers/media/pci/saa7134/saa7134-input.c b/drivers/media/pci/saa7134/saa7134-input.c > > >> index 90837ec6e70f..239f0b9d080a 100644 > > >> --- a/drivers/media/pci/saa7134/saa7134-input.c > > >> +++ b/drivers/media/pci/saa7134/saa7134-input.c > > >> @@ -895,7 +895,7 @@ void saa7134_probe_i2c_ir(struct saa7134_dev *dev) > > >> rc = i2c_transfer(&dev->i2c_adap, &msg_msi, 1); > > >> input_dbg("probe 0x%02x @ %s: %s\n", > > >> msg_msi.addr, dev->i2c_adap.name, > > >> - str_yes_no(1 == rc)); > > >> + str_yes_no(rc == 1)); > > >> break; > > >> case SAA7134_BOARD_SNAZIO_TVPVR_PRO: > > >> dev->init_data.name = "SnaZio* TVPVR PRO"; > > >> @@ -931,7 +931,7 @@ void saa7134_probe_i2c_ir(struct saa7134_dev *dev) > > >> rc = i2c_transfer(&dev->i2c_adap, &msg_msi, 1); > > >> input_dbg("probe 0x%02x @ %s: %s\n", > > >> msg_msi.addr, dev->i2c_adap.name, > > >> - str_yes_no(1 == rc)); > > >> + str_yes_no(rc == 1)); > > >> break; > > >> case SAA7134_BOARD_HAUPPAUGE_HVR1110: > > >> dev->init_data.name = saa7134_boards[dev->board].name; > > >> diff --git a/drivers/media/usb/pvrusb2/pvrusb2-ctrl.c b/drivers/media/usb/pvrusb2/pvrusb2-ctrl.c > > >> index 448c40caf363..b6c9bda214c8 100644 > > >> --- a/drivers/media/usb/pvrusb2/pvrusb2-ctrl.c > > >> +++ b/drivers/media/usb/pvrusb2/pvrusb2-ctrl.c > > >> @@ -521,7 +521,7 @@ int pvr2_ctrl_value_to_sym_internal(struct pvr2_ctrl *cptr, > > >> *len = scnprintf(buf,maxlen,"%d",val); > > >> ret = 0; > > >> } else if (cptr->info->type == pvr2_ctl_bool) { > > >> - *len = scnprintf(buf,maxlen,"%s",str_true_false(val)); > > >> + *len = scnprintf(buf, maxlen, "%s", str_true_false(val)); > > >> ret = 0; > > >> } else if (cptr->info->type == pvr2_ctl_enum) { > > >> const char * const *names; > > >> diff --git a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c > > >> index 96d3a0045fac..761d718478ca 100644 > > >> --- a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c > > >> +++ b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c > > >> @@ -338,7 +338,7 @@ static void trace_stbit(const char *name,int val) > > >> { > > >> pvr2_trace(PVR2_TRACE_STBITS, > > >> "State bit %s <-- %s", > > >> - name,str_true_false(val)); > > >> + name, str_true_false(val)); > > >> } > > >> > > >> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> > > >> --- > > >> Ricardo Ribalda (45): > > >> media: staging: ipu3: Use string_choices helpers > > >> media: staging: atomisp: Use string_choices helpers > > >> media: core: Use string_choices helpers > > >> media: pwc-ctl: Use string_choices helpers > > >> media: pvrusb2:Use string_choices helpers > > >> media: em28xx: Use string_choices helpers > > >> media: dvb-usb: Use string_choices helpers > > >> media: dvb-usb-v2: Use string_choices helpers > > >> media: cx231xx: Use string_choices helpers > > >> media: tuners: Use string_choices helpers > > >> media: rc: Use string_choices helpers > > >> media: dvb-frontends: Use string_choices helpers > > >> media: pci: cx23885: Use string_choices helpers > > >> media: saa7134: Use string_choices helpers > > >> media: amphion: Use string_choices helpers > > >> media: pci: ivtv: Use string_choices helpers > > >> media: bttv: Use string_choices helpers > > >> media: xilinx: Use string_choices helpers > > >> media: platform: ti: Use string_choices helpers > > >> media: st: Use string_choices helpers > > >> media: coda: Use string_choices helpers > > >> media: aspeed: Use string_choices helpers > > >> media: ipu6: Use string_choices helpers > > >> media: cx18: Use string_choices helpers > > >> media: cobalt: Use string_choices helpers > > >> media: videobuf2: Use string_choices helpers > > >> media: cec: Use string_choices helpers > > >> media: b2c2: Use string_choices helpers > > >> media: siano: Use string_choices helpers > > >> media: i2c: cx25840: Use string_choices helpers > > >> media: i2c: vpx3220: Use string_choices helpers > > >> media: i2c: tvp7002: Use string_choices helpers > > >> media: i2c: ths8200: Use string_choices helpers > > >> media: i2c: tda1997x: Use string_choices helpers > > >> media: i2c: tc358743: Use string_choices helpers > > >> media: i2c: st-mipid02: Use string_choices helpers > > >> media: i2c: msp3400: Use string_choices helpers > > >> media: i2c: max9286: Use string_choices helpers > > >> media: i2c: saa717x: Use string_choices helpers > > >> media: i2c: saa7127: Use string_choices helpers > > >> media: i2c: saa7115: Use string_choices helpers > > >> media: i2c: saa7110: Use string_choices helpers > > >> media: i2c: adv7842: Use string_choices helpers > > >> media: i2c: adv76xx: Use string_choices helpers > > >> media: i2c: adv7511: Use string_choices helpers > > >> > > >> drivers/media/cec/platform/cec-gpio/cec-gpio.c | 4 +- > > >> drivers/media/cec/usb/pulse8/pulse8-cec.c | 4 +- > > >> drivers/media/common/b2c2/flexcop-hw-filter.c | 4 +- > > >> drivers/media/common/siano/sms-cards.c | 3 +- > > >> drivers/media/common/videobuf2/videobuf2-core.c | 5 ++- > > >> drivers/media/dvb-frontends/ascot2e.c | 2 +- > > >> drivers/media/dvb-frontends/cx24120.c | 4 +- > > >> drivers/media/dvb-frontends/cxd2841er.c | 2 +- > > >> drivers/media/dvb-frontends/drxk_hard.c | 4 +- > > >> drivers/media/dvb-frontends/helene.c | 2 +- > > >> drivers/media/dvb-frontends/horus3a.c | 2 +- > > >> drivers/media/dvb-frontends/sp2.c | 2 +- > > >> drivers/media/i2c/adv7511-v4l2.c | 11 +++--- > > >> drivers/media/i2c/adv7604.c | 25 ++++++------ > > >> drivers/media/i2c/adv7842.c | 40 ++++++++++---------- > > >> drivers/media/i2c/cx25840/cx25840-core.c | 4 +- > > >> drivers/media/i2c/cx25840/cx25840-ir.c | 34 ++++++++--------- > > >> drivers/media/i2c/max9286.c | 2 +- > > >> drivers/media/i2c/msp3400-driver.c | 4 +- > > >> drivers/media/i2c/saa7110.c | 2 +- > > >> drivers/media/i2c/saa7115.c | 2 +- > > >> drivers/media/i2c/saa7127.c | 15 +++++--- > > >> drivers/media/i2c/saa717x.c | 2 +- > > >> drivers/media/i2c/st-mipid02.c | 2 +- > > >> drivers/media/i2c/tc358743.c | 44 ++++++++++------------ > > >> drivers/media/i2c/tda1997x.c | 6 +-- > > >> drivers/media/i2c/ths8200.c | 4 +- > > >> drivers/media/i2c/tvp7002.c | 2 +- > > >> drivers/media/i2c/vpx3220.c | 2 +- > > >> drivers/media/pci/bt8xx/bttv-cards.c | 16 ++++---- > > >> drivers/media/pci/bt8xx/bttv-driver.c | 6 +-- > > >> drivers/media/pci/cobalt/cobalt-driver.c | 2 +- > > >> drivers/media/pci/cx18/cx18-av-core.c | 4 +- > > >> drivers/media/pci/cx23885/altera-ci.c | 2 +- > > >> drivers/media/pci/cx23885/cimax2.c | 2 +- > > >> drivers/media/pci/cx23885/cx23888-ir.c | 36 +++++++++--------- > > >> drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c | 2 +- > > >> drivers/media/pci/ivtv/ivtvfb.c | 4 +- > > >> drivers/media/pci/saa7134/saa7134-cards.c | 2 +- > > >> drivers/media/pci/saa7134/saa7134-dvb.c | 2 +- > > >> drivers/media/pci/saa7134/saa7134-input.c | 6 +-- > > >> drivers/media/pci/saa7134/saa7134-video.c | 2 +- > > >> drivers/media/platform/amphion/venc.c | 2 +- > > >> drivers/media/platform/amphion/vpu_dbg.c | 2 +- > > >> drivers/media/platform/aspeed/aspeed-video.c | 4 +- > > >> drivers/media/platform/chips-media/coda/imx-vdoa.c | 3 +- > > >> drivers/media/platform/st/sti/hva/hva-debugfs.c | 6 +-- > > >> drivers/media/platform/ti/omap3isp/ispstat.c | 2 +- > > >> drivers/media/platform/xilinx/xilinx-csi2rxss.c | 18 ++++----- > > >> drivers/media/rc/ene_ir.c | 3 +- > > >> drivers/media/rc/mceusb.c | 3 +- > > >> drivers/media/rc/serial_ir.c | 5 ++- > > >> drivers/media/tuners/tda18250.c | 2 +- > > >> drivers/media/tuners/tda9887.c | 10 ++--- > > >> drivers/media/usb/cx231xx/cx231xx-i2c.c | 4 +- > > >> drivers/media/usb/cx231xx/cx231xx-video.c | 2 +- > > >> drivers/media/usb/dvb-usb-v2/az6007.c | 4 +- > > >> drivers/media/usb/dvb-usb-v2/dvb_usb_core.c | 4 +- > > >> drivers/media/usb/dvb-usb/af9005-fe.c | 4 +- > > >> drivers/media/usb/dvb-usb/dvb-usb-dvb.c | 6 +-- > > >> drivers/media/usb/dvb-usb/opera1.c | 8 ++-- > > >> drivers/media/usb/em28xx/em28xx-i2c.c | 4 +- > > >> drivers/media/usb/em28xx/em28xx-video.c | 2 +- > > >> drivers/media/usb/pvrusb2/pvrusb2-ctrl.c | 2 +- > > >> drivers/media/usb/pvrusb2/pvrusb2-debugifc.c | 3 +- > > >> drivers/media/usb/pvrusb2/pvrusb2-encoder.c | 5 +-- > > >> drivers/media/usb/pvrusb2/pvrusb2-hdw.c | 6 +-- > > >> drivers/media/usb/pvrusb2/pvrusb2-i2c-core.c | 3 +- > > >> drivers/media/usb/pwc/pwc-ctrl.c | 2 +- > > >> drivers/media/v4l2-core/v4l2-ctrls-core.c | 3 +- > > >> drivers/media/v4l2-core/v4l2-fwnode.c | 12 +++--- > > >> .../media/atomisp/pci/atomisp_compat_css20.c | 2 +- > > >> .../media/atomisp/pci/atomisp_csi2_bridge.c | 2 +- > > >> .../media/atomisp/pci/atomisp_gmin_platform.c | 4 +- > > >> drivers/staging/media/atomisp/pci/atomisp_v4l2.c | 4 +- > > >> .../media/atomisp/pci/runtime/binary/src/binary.c | 2 +- > > >> drivers/staging/media/atomisp/pci/sh_css.c | 2 +- > > >> drivers/staging/media/ipu3/ipu3-v4l2.c | 4 +- > > >> 78 files changed, 240 insertions(+), 239 deletions(-) > > >> --- > > >> base-commit: 9852d85ec9d492ebef56dc5f229416c925758edc > > >> change-id: 20240930-cocci-opportunity-40bca6a17c42 > > > > > > > > -- > Ricardo Ribalda >
On Mon, Sep 30, 2024 at 9:38 PM Hans Verkuil <hverkuil@xs4all.nl> wrote: > > On 30/09/2024 14:21, Laurent Pinchart wrote: > > Hi Ricardo, > > > > On Mon, Sep 30, 2024 at 12:03:55PM +0000, Ricardo Ribalda wrote: > >> include/linux/string_choices.h contains a set of helpers that can be > >> used instead of hard coding some strings. > >> > >> Cocci has located some places where the helpers can be used. This > >> patchset uses the diff generated by cocci, plus these changes: > > > > Personally I think most of those helpers just hinder readability for not > > much added gain. String de-duplication is done by the linker already. > > The only value I see in the helpers is ensuring that the strings are > > consistently written, and I think we can do so through other means. > > Just my opinion: I'm OK with these new helpers, but I am not too keen to apply > all this to existing source code. I.e., for new code it is fine, but if we have > to update all drivers every time a new cocci test is added, then that will not > scale. +1 for avoiding applying this to existing code. It will just make it more difficult to backport changes to stable kernels because of meaningless conflicts. Best, Tomasz > > Note that I never ran cocci in my build scripts, so this is a new check that > we haven't set rules for or have much experience with. > > checkpatch just checks the patches, it doesn't force you to fix existing code. > > Some of the cocci tests are clearly checking for incorrect code, but others are > for code improvements (i.e. the code was correct, it can just be done slightly > better). It's the second category were I think that should only apply to new code, > and not existing code. > > Regards, > > Hans > > > > >> diff --git a/drivers/media/dvb-frontends/ascot2e.c b/drivers/media/dvb-frontends/ascot2e.c > >> index 8c3eb5d69dda..ec7a718428fc 100644 > >> --- a/drivers/media/dvb-frontends/ascot2e.c > >> +++ b/drivers/media/dvb-frontends/ascot2e.c > >> @@ -104,7 +104,7 @@ static void ascot2e_i2c_debug(struct ascot2e_priv *priv, > >> u8 reg, u8 write, const u8 *data, u32 len) > >> { > >> dev_dbg(&priv->i2c->dev, "ascot2e: I2C %s reg 0x%02x size %d\n", > >> - str_read_write(write == 0), reg, len); > >> + str_write_read(write), reg, len); > >> print_hex_dump_bytes("ascot2e: I2C data: ", > >> DUMP_PREFIX_OFFSET, data, len); > >> } > >> diff --git a/drivers/media/dvb-frontends/cxd2841er.c b/drivers/media/dvb-frontends/cxd2841er.c > >> index db684f314b47..d1b84cd9c510 100644 > >> --- a/drivers/media/dvb-frontends/cxd2841er.c > >> +++ b/drivers/media/dvb-frontends/cxd2841er.c > >> @@ -206,7 +206,7 @@ static void cxd2841er_i2c_debug(struct cxd2841er_priv *priv, > >> { > >> dev_dbg(&priv->i2c->dev, > >> "cxd2841er: I2C %s addr %02x reg 0x%02x size %d data %*ph\n", > >> - str_read_write(write == 0), addr, reg, len, len, data); > >> + str_write_read(write), addr, reg, len, len, data); > >> } > >> > >> static int cxd2841er_write_regs(struct cxd2841er_priv *priv, > >> diff --git a/drivers/media/dvb-frontends/helene.c b/drivers/media/dvb-frontends/helene.c > >> index 52198cb49dba..b4527c141d9c 100644 > >> --- a/drivers/media/dvb-frontends/helene.c > >> +++ b/drivers/media/dvb-frontends/helene.c > >> @@ -279,7 +279,7 @@ static void helene_i2c_debug(struct helene_priv *priv, > >> u8 reg, u8 write, const u8 *data, u32 len) > >> { > >> dev_dbg(&priv->i2c->dev, "helene: I2C %s reg 0x%02x size %d\n", > >> - str_read_write(write == 0), reg, len); > >> + str_write_read(write), reg, len); > >> print_hex_dump_bytes("helene: I2C data: ", > >> DUMP_PREFIX_OFFSET, data, len); > >> } > >> diff --git a/drivers/media/dvb-frontends/horus3a.c b/drivers/media/dvb-frontends/horus3a.c > >> index 84385079918c..10300ebf3ca0 100644 > >> --- a/drivers/media/dvb-frontends/horus3a.c > >> +++ b/drivers/media/dvb-frontends/horus3a.c > >> @@ -38,7 +38,7 @@ static void horus3a_i2c_debug(struct horus3a_priv *priv, > >> u8 reg, u8 write, const u8 *data, u32 len) > >> { > >> dev_dbg(&priv->i2c->dev, "horus3a: I2C %s reg 0x%02x size %d\n", > >> - str_read_write(write == 0), reg, len); > >> + str_write_read(write), reg, len); > >> print_hex_dump_bytes("horus3a: I2C data: ", > >> DUMP_PREFIX_OFFSET, data, len); > >> } > >> diff --git a/drivers/media/i2c/adv7842.c b/drivers/media/i2c/adv7842.c > >> index ba174aa45afa..a43479c3ff03 100644 > >> --- a/drivers/media/i2c/adv7842.c > >> +++ b/drivers/media/i2c/adv7842.c > >> @@ -2763,7 +2763,7 @@ static int adv7842_cp_log_status(struct v4l2_subdev *sd) > >> str_true_false(io_read(sd, 0x6a) & 0x10)); > >> } > >> v4l2_info(sd, "CP free run: %s\n", > >> - str_on_off(!!(cp_read(sd, 0xff) & 0x10))); > >> + str_on_off(cp_read(sd, 0xff) & 0x10)); > >> v4l2_info(sd, "Prim-mode = 0x%x, video std = 0x%x, v_freq = 0x%x\n", > >> io_read(sd, 0x01) & 0x0f, io_read(sd, 0x00) & 0x3f, > >> (io_read(sd, 0x01) & 0x70) >> 4); > >> diff --git a/drivers/media/pci/saa7134/saa7134-cards.c b/drivers/media/pci/saa7134/saa7134-cards.c > >> index 301b89e799d8..79cd61fb0205 100644 > >> --- a/drivers/media/pci/saa7134/saa7134-cards.c > >> +++ b/drivers/media/pci/saa7134/saa7134-cards.c > >> @@ -7981,7 +7981,7 @@ int saa7134_board_init2(struct saa7134_dev *dev) > >> rc = i2c_transfer(&dev->i2c_adap, &msg, 1); > >> pr_info("%s: probe IR chip @ i2c 0x%02x: %s\n", > >> dev->name, msg.addr, > >> - str_yes_no(1 == rc)); > >> + str_yes_no(rc == 1)); > >> if (rc == 1) > >> dev->has_remote = SAA7134_REMOTE_I2C; > >> } > >> diff --git a/drivers/media/pci/saa7134/saa7134-input.c b/drivers/media/pci/saa7134/saa7134-input.c > >> index 90837ec6e70f..239f0b9d080a 100644 > >> --- a/drivers/media/pci/saa7134/saa7134-input.c > >> +++ b/drivers/media/pci/saa7134/saa7134-input.c > >> @@ -895,7 +895,7 @@ void saa7134_probe_i2c_ir(struct saa7134_dev *dev) > >> rc = i2c_transfer(&dev->i2c_adap, &msg_msi, 1); > >> input_dbg("probe 0x%02x @ %s: %s\n", > >> msg_msi.addr, dev->i2c_adap.name, > >> - str_yes_no(1 == rc)); > >> + str_yes_no(rc == 1)); > >> break; > >> case SAA7134_BOARD_SNAZIO_TVPVR_PRO: > >> dev->init_data.name = "SnaZio* TVPVR PRO"; > >> @@ -931,7 +931,7 @@ void saa7134_probe_i2c_ir(struct saa7134_dev *dev) > >> rc = i2c_transfer(&dev->i2c_adap, &msg_msi, 1); > >> input_dbg("probe 0x%02x @ %s: %s\n", > >> msg_msi.addr, dev->i2c_adap.name, > >> - str_yes_no(1 == rc)); > >> + str_yes_no(rc == 1)); > >> break; > >> case SAA7134_BOARD_HAUPPAUGE_HVR1110: > >> dev->init_data.name = saa7134_boards[dev->board].name; > >> diff --git a/drivers/media/usb/pvrusb2/pvrusb2-ctrl.c b/drivers/media/usb/pvrusb2/pvrusb2-ctrl.c > >> index 448c40caf363..b6c9bda214c8 100644 > >> --- a/drivers/media/usb/pvrusb2/pvrusb2-ctrl.c > >> +++ b/drivers/media/usb/pvrusb2/pvrusb2-ctrl.c > >> @@ -521,7 +521,7 @@ int pvr2_ctrl_value_to_sym_internal(struct pvr2_ctrl *cptr, > >> *len = scnprintf(buf,maxlen,"%d",val); > >> ret = 0; > >> } else if (cptr->info->type == pvr2_ctl_bool) { > >> - *len = scnprintf(buf,maxlen,"%s",str_true_false(val)); > >> + *len = scnprintf(buf, maxlen, "%s", str_true_false(val)); > >> ret = 0; > >> } else if (cptr->info->type == pvr2_ctl_enum) { > >> const char * const *names; > >> diff --git a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c > >> index 96d3a0045fac..761d718478ca 100644 > >> --- a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c > >> +++ b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c > >> @@ -338,7 +338,7 @@ static void trace_stbit(const char *name,int val) > >> { > >> pvr2_trace(PVR2_TRACE_STBITS, > >> "State bit %s <-- %s", > >> - name,str_true_false(val)); > >> + name, str_true_false(val)); > >> } > >> > >> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> > >> --- > >> Ricardo Ribalda (45): > >> media: staging: ipu3: Use string_choices helpers > >> media: staging: atomisp: Use string_choices helpers > >> media: core: Use string_choices helpers > >> media: pwc-ctl: Use string_choices helpers > >> media: pvrusb2:Use string_choices helpers > >> media: em28xx: Use string_choices helpers > >> media: dvb-usb: Use string_choices helpers > >> media: dvb-usb-v2: Use string_choices helpers > >> media: cx231xx: Use string_choices helpers > >> media: tuners: Use string_choices helpers > >> media: rc: Use string_choices helpers > >> media: dvb-frontends: Use string_choices helpers > >> media: pci: cx23885: Use string_choices helpers > >> media: saa7134: Use string_choices helpers > >> media: amphion: Use string_choices helpers > >> media: pci: ivtv: Use string_choices helpers > >> media: bttv: Use string_choices helpers > >> media: xilinx: Use string_choices helpers > >> media: platform: ti: Use string_choices helpers > >> media: st: Use string_choices helpers > >> media: coda: Use string_choices helpers > >> media: aspeed: Use string_choices helpers > >> media: ipu6: Use string_choices helpers > >> media: cx18: Use string_choices helpers > >> media: cobalt: Use string_choices helpers > >> media: videobuf2: Use string_choices helpers > >> media: cec: Use string_choices helpers > >> media: b2c2: Use string_choices helpers > >> media: siano: Use string_choices helpers > >> media: i2c: cx25840: Use string_choices helpers > >> media: i2c: vpx3220: Use string_choices helpers > >> media: i2c: tvp7002: Use string_choices helpers > >> media: i2c: ths8200: Use string_choices helpers > >> media: i2c: tda1997x: Use string_choices helpers > >> media: i2c: tc358743: Use string_choices helpers > >> media: i2c: st-mipid02: Use string_choices helpers > >> media: i2c: msp3400: Use string_choices helpers > >> media: i2c: max9286: Use string_choices helpers > >> media: i2c: saa717x: Use string_choices helpers > >> media: i2c: saa7127: Use string_choices helpers > >> media: i2c: saa7115: Use string_choices helpers > >> media: i2c: saa7110: Use string_choices helpers > >> media: i2c: adv7842: Use string_choices helpers > >> media: i2c: adv76xx: Use string_choices helpers > >> media: i2c: adv7511: Use string_choices helpers > >> > >> drivers/media/cec/platform/cec-gpio/cec-gpio.c | 4 +- > >> drivers/media/cec/usb/pulse8/pulse8-cec.c | 4 +- > >> drivers/media/common/b2c2/flexcop-hw-filter.c | 4 +- > >> drivers/media/common/siano/sms-cards.c | 3 +- > >> drivers/media/common/videobuf2/videobuf2-core.c | 5 ++- > >> drivers/media/dvb-frontends/ascot2e.c | 2 +- > >> drivers/media/dvb-frontends/cx24120.c | 4 +- > >> drivers/media/dvb-frontends/cxd2841er.c | 2 +- > >> drivers/media/dvb-frontends/drxk_hard.c | 4 +- > >> drivers/media/dvb-frontends/helene.c | 2 +- > >> drivers/media/dvb-frontends/horus3a.c | 2 +- > >> drivers/media/dvb-frontends/sp2.c | 2 +- > >> drivers/media/i2c/adv7511-v4l2.c | 11 +++--- > >> drivers/media/i2c/adv7604.c | 25 ++++++------ > >> drivers/media/i2c/adv7842.c | 40 ++++++++++---------- > >> drivers/media/i2c/cx25840/cx25840-core.c | 4 +- > >> drivers/media/i2c/cx25840/cx25840-ir.c | 34 ++++++++--------- > >> drivers/media/i2c/max9286.c | 2 +- > >> drivers/media/i2c/msp3400-driver.c | 4 +- > >> drivers/media/i2c/saa7110.c | 2 +- > >> drivers/media/i2c/saa7115.c | 2 +- > >> drivers/media/i2c/saa7127.c | 15 +++++--- > >> drivers/media/i2c/saa717x.c | 2 +- > >> drivers/media/i2c/st-mipid02.c | 2 +- > >> drivers/media/i2c/tc358743.c | 44 ++++++++++------------ > >> drivers/media/i2c/tda1997x.c | 6 +-- > >> drivers/media/i2c/ths8200.c | 4 +- > >> drivers/media/i2c/tvp7002.c | 2 +- > >> drivers/media/i2c/vpx3220.c | 2 +- > >> drivers/media/pci/bt8xx/bttv-cards.c | 16 ++++---- > >> drivers/media/pci/bt8xx/bttv-driver.c | 6 +-- > >> drivers/media/pci/cobalt/cobalt-driver.c | 2 +- > >> drivers/media/pci/cx18/cx18-av-core.c | 4 +- > >> drivers/media/pci/cx23885/altera-ci.c | 2 +- > >> drivers/media/pci/cx23885/cimax2.c | 2 +- > >> drivers/media/pci/cx23885/cx23888-ir.c | 36 +++++++++--------- > >> drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c | 2 +- > >> drivers/media/pci/ivtv/ivtvfb.c | 4 +- > >> drivers/media/pci/saa7134/saa7134-cards.c | 2 +- > >> drivers/media/pci/saa7134/saa7134-dvb.c | 2 +- > >> drivers/media/pci/saa7134/saa7134-input.c | 6 +-- > >> drivers/media/pci/saa7134/saa7134-video.c | 2 +- > >> drivers/media/platform/amphion/venc.c | 2 +- > >> drivers/media/platform/amphion/vpu_dbg.c | 2 +- > >> drivers/media/platform/aspeed/aspeed-video.c | 4 +- > >> drivers/media/platform/chips-media/coda/imx-vdoa.c | 3 +- > >> drivers/media/platform/st/sti/hva/hva-debugfs.c | 6 +-- > >> drivers/media/platform/ti/omap3isp/ispstat.c | 2 +- > >> drivers/media/platform/xilinx/xilinx-csi2rxss.c | 18 ++++----- > >> drivers/media/rc/ene_ir.c | 3 +- > >> drivers/media/rc/mceusb.c | 3 +- > >> drivers/media/rc/serial_ir.c | 5 ++- > >> drivers/media/tuners/tda18250.c | 2 +- > >> drivers/media/tuners/tda9887.c | 10 ++--- > >> drivers/media/usb/cx231xx/cx231xx-i2c.c | 4 +- > >> drivers/media/usb/cx231xx/cx231xx-video.c | 2 +- > >> drivers/media/usb/dvb-usb-v2/az6007.c | 4 +- > >> drivers/media/usb/dvb-usb-v2/dvb_usb_core.c | 4 +- > >> drivers/media/usb/dvb-usb/af9005-fe.c | 4 +- > >> drivers/media/usb/dvb-usb/dvb-usb-dvb.c | 6 +-- > >> drivers/media/usb/dvb-usb/opera1.c | 8 ++-- > >> drivers/media/usb/em28xx/em28xx-i2c.c | 4 +- > >> drivers/media/usb/em28xx/em28xx-video.c | 2 +- > >> drivers/media/usb/pvrusb2/pvrusb2-ctrl.c | 2 +- > >> drivers/media/usb/pvrusb2/pvrusb2-debugifc.c | 3 +- > >> drivers/media/usb/pvrusb2/pvrusb2-encoder.c | 5 +-- > >> drivers/media/usb/pvrusb2/pvrusb2-hdw.c | 6 +-- > >> drivers/media/usb/pvrusb2/pvrusb2-i2c-core.c | 3 +- > >> drivers/media/usb/pwc/pwc-ctrl.c | 2 +- > >> drivers/media/v4l2-core/v4l2-ctrls-core.c | 3 +- > >> drivers/media/v4l2-core/v4l2-fwnode.c | 12 +++--- > >> .../media/atomisp/pci/atomisp_compat_css20.c | 2 +- > >> .../media/atomisp/pci/atomisp_csi2_bridge.c | 2 +- > >> .../media/atomisp/pci/atomisp_gmin_platform.c | 4 +- > >> drivers/staging/media/atomisp/pci/atomisp_v4l2.c | 4 +- > >> .../media/atomisp/pci/runtime/binary/src/binary.c | 2 +- > >> drivers/staging/media/atomisp/pci/sh_css.c | 2 +- > >> drivers/staging/media/ipu3/ipu3-v4l2.c | 4 +- > >> 78 files changed, 240 insertions(+), 239 deletions(-) > >> --- > >> base-commit: 9852d85ec9d492ebef56dc5f229416c925758edc > >> change-id: 20240930-cocci-opportunity-40bca6a17c42 > > >
Hi Julia, On 30/09/2024 16:36, Julia Lawall wrote: > > > On Mon, 30 Sep 2024, Ricardo Ribalda wrote: > >> On Mon, 30 Sept 2024 at 14:38, Hans Verkuil <hverkuil@xs4all.nl> wrote: >>> >>> On 30/09/2024 14:21, Laurent Pinchart wrote: >>>> Hi Ricardo, >>>> >>>> On Mon, Sep 30, 2024 at 12:03:55PM +0000, Ricardo Ribalda wrote: >>>>> include/linux/string_choices.h contains a set of helpers that can be >>>>> used instead of hard coding some strings. >>>>> >>>>> Cocci has located some places where the helpers can be used. This >>>>> patchset uses the diff generated by cocci, plus these changes: >>>> >>>> Personally I think most of those helpers just hinder readability for not >>>> much added gain. String de-duplication is done by the linker already. >>>> The only value I see in the helpers is ensuring that the strings are >>>> consistently written, and I think we can do so through other means. >>> >>> Just my opinion: I'm OK with these new helpers, but I am not too keen to apply >>> all this to existing source code. I.e., for new code it is fine, but if we have >>> to update all drivers every time a new cocci test is added, then that will not >>> scale. >>> >>> Note that I never ran cocci in my build scripts, so this is a new check that >>> we haven't set rules for or have much experience with. >>> >>> checkpatch just checks the patches, it doesn't force you to fix existing code. >>> >>> Some of the cocci tests are clearly checking for incorrect code, but others are >>> for code improvements (i.e. the code was correct, it can just be done slightly >>> better). It's the second category were I think that should only apply to new code, >>> and not existing code. >>> >>> Regards, >> >> Julia, correct me if I am wrong, but I believe that cocci does not >> have the ability to check only new code. It runs against files not >> diffs. > > Coccinelle runs on files normally. There is an option --use-patch-diff > where you can give a commit id, and it should only work on the files > mentioned in that commit, but it doesn't seem to focus on only the new > code in that commit. Is it possible to add support for that? And a related question: is it possible to skip certain tests? Basically, I don't want to see reports of code improvements, unless it is for a new source file. Reports of dangerous/wrong constructs are always welcome, but code improvements like these string_choice helpers really only make sense for patches adding new source files. Regards, Hans > > julia > >> >> I agree with you, a "improvement cocci" shall not force us to drop the >> pen and work full time to fix it :) >> When a new "improvement" cocci check is landed, we should update our >> "allowlist" immediately to ignore those warnings until we slowly >> improve our codebase to meet the standard. >> That way the CI is not affected. >> >> Regarding this patchset.... It is pretty big, but also pretty simple. >> I added my "extra changes" on the cover letter to ease the review. >> >> If you or someone else have time to review it... then it will be great >> if we land it. But if none has time for it, then it can be ignored. >> >> >> Regards! >> >>> >>> Hans >>> >>>> >>>>> diff --git a/drivers/media/dvb-frontends/ascot2e.c b/drivers/media/dvb-frontends/ascot2e.c >>>>> index 8c3eb5d69dda..ec7a718428fc 100644 >>>>> --- a/drivers/media/dvb-frontends/ascot2e.c >>>>> +++ b/drivers/media/dvb-frontends/ascot2e.c >>>>> @@ -104,7 +104,7 @@ static void ascot2e_i2c_debug(struct ascot2e_priv *priv, >>>>> u8 reg, u8 write, const u8 *data, u32 len) >>>>> { >>>>> dev_dbg(&priv->i2c->dev, "ascot2e: I2C %s reg 0x%02x size %d\n", >>>>> - str_read_write(write == 0), reg, len); >>>>> + str_write_read(write), reg, len); >>>>> print_hex_dump_bytes("ascot2e: I2C data: ", >>>>> DUMP_PREFIX_OFFSET, data, len); >>>>> } >>>>> diff --git a/drivers/media/dvb-frontends/cxd2841er.c b/drivers/media/dvb-frontends/cxd2841er.c >>>>> index db684f314b47..d1b84cd9c510 100644 >>>>> --- a/drivers/media/dvb-frontends/cxd2841er.c >>>>> +++ b/drivers/media/dvb-frontends/cxd2841er.c >>>>> @@ -206,7 +206,7 @@ static void cxd2841er_i2c_debug(struct cxd2841er_priv *priv, >>>>> { >>>>> dev_dbg(&priv->i2c->dev, >>>>> "cxd2841er: I2C %s addr %02x reg 0x%02x size %d data %*ph\n", >>>>> - str_read_write(write == 0), addr, reg, len, len, data); >>>>> + str_write_read(write), addr, reg, len, len, data); >>>>> } >>>>> >>>>> static int cxd2841er_write_regs(struct cxd2841er_priv *priv, >>>>> diff --git a/drivers/media/dvb-frontends/helene.c b/drivers/media/dvb-frontends/helene.c >>>>> index 52198cb49dba..b4527c141d9c 100644 >>>>> --- a/drivers/media/dvb-frontends/helene.c >>>>> +++ b/drivers/media/dvb-frontends/helene.c >>>>> @@ -279,7 +279,7 @@ static void helene_i2c_debug(struct helene_priv *priv, >>>>> u8 reg, u8 write, const u8 *data, u32 len) >>>>> { >>>>> dev_dbg(&priv->i2c->dev, "helene: I2C %s reg 0x%02x size %d\n", >>>>> - str_read_write(write == 0), reg, len); >>>>> + str_write_read(write), reg, len); >>>>> print_hex_dump_bytes("helene: I2C data: ", >>>>> DUMP_PREFIX_OFFSET, data, len); >>>>> } >>>>> diff --git a/drivers/media/dvb-frontends/horus3a.c b/drivers/media/dvb-frontends/horus3a.c >>>>> index 84385079918c..10300ebf3ca0 100644 >>>>> --- a/drivers/media/dvb-frontends/horus3a.c >>>>> +++ b/drivers/media/dvb-frontends/horus3a.c >>>>> @@ -38,7 +38,7 @@ static void horus3a_i2c_debug(struct horus3a_priv *priv, >>>>> u8 reg, u8 write, const u8 *data, u32 len) >>>>> { >>>>> dev_dbg(&priv->i2c->dev, "horus3a: I2C %s reg 0x%02x size %d\n", >>>>> - str_read_write(write == 0), reg, len); >>>>> + str_write_read(write), reg, len); >>>>> print_hex_dump_bytes("horus3a: I2C data: ", >>>>> DUMP_PREFIX_OFFSET, data, len); >>>>> } >>>>> diff --git a/drivers/media/i2c/adv7842.c b/drivers/media/i2c/adv7842.c >>>>> index ba174aa45afa..a43479c3ff03 100644 >>>>> --- a/drivers/media/i2c/adv7842.c >>>>> +++ b/drivers/media/i2c/adv7842.c >>>>> @@ -2763,7 +2763,7 @@ static int adv7842_cp_log_status(struct v4l2_subdev *sd) >>>>> str_true_false(io_read(sd, 0x6a) & 0x10)); >>>>> } >>>>> v4l2_info(sd, "CP free run: %s\n", >>>>> - str_on_off(!!(cp_read(sd, 0xff) & 0x10))); >>>>> + str_on_off(cp_read(sd, 0xff) & 0x10)); >>>>> v4l2_info(sd, "Prim-mode = 0x%x, video std = 0x%x, v_freq = 0x%x\n", >>>>> io_read(sd, 0x01) & 0x0f, io_read(sd, 0x00) & 0x3f, >>>>> (io_read(sd, 0x01) & 0x70) >> 4); >>>>> diff --git a/drivers/media/pci/saa7134/saa7134-cards.c b/drivers/media/pci/saa7134/saa7134-cards.c >>>>> index 301b89e799d8..79cd61fb0205 100644 >>>>> --- a/drivers/media/pci/saa7134/saa7134-cards.c >>>>> +++ b/drivers/media/pci/saa7134/saa7134-cards.c >>>>> @@ -7981,7 +7981,7 @@ int saa7134_board_init2(struct saa7134_dev *dev) >>>>> rc = i2c_transfer(&dev->i2c_adap, &msg, 1); >>>>> pr_info("%s: probe IR chip @ i2c 0x%02x: %s\n", >>>>> dev->name, msg.addr, >>>>> - str_yes_no(1 == rc)); >>>>> + str_yes_no(rc == 1)); >>>>> if (rc == 1) >>>>> dev->has_remote = SAA7134_REMOTE_I2C; >>>>> } >>>>> diff --git a/drivers/media/pci/saa7134/saa7134-input.c b/drivers/media/pci/saa7134/saa7134-input.c >>>>> index 90837ec6e70f..239f0b9d080a 100644 >>>>> --- a/drivers/media/pci/saa7134/saa7134-input.c >>>>> +++ b/drivers/media/pci/saa7134/saa7134-input.c >>>>> @@ -895,7 +895,7 @@ void saa7134_probe_i2c_ir(struct saa7134_dev *dev) >>>>> rc = i2c_transfer(&dev->i2c_adap, &msg_msi, 1); >>>>> input_dbg("probe 0x%02x @ %s: %s\n", >>>>> msg_msi.addr, dev->i2c_adap.name, >>>>> - str_yes_no(1 == rc)); >>>>> + str_yes_no(rc == 1)); >>>>> break; >>>>> case SAA7134_BOARD_SNAZIO_TVPVR_PRO: >>>>> dev->init_data.name = "SnaZio* TVPVR PRO"; >>>>> @@ -931,7 +931,7 @@ void saa7134_probe_i2c_ir(struct saa7134_dev *dev) >>>>> rc = i2c_transfer(&dev->i2c_adap, &msg_msi, 1); >>>>> input_dbg("probe 0x%02x @ %s: %s\n", >>>>> msg_msi.addr, dev->i2c_adap.name, >>>>> - str_yes_no(1 == rc)); >>>>> + str_yes_no(rc == 1)); >>>>> break; >>>>> case SAA7134_BOARD_HAUPPAUGE_HVR1110: >>>>> dev->init_data.name = saa7134_boards[dev->board].name; >>>>> diff --git a/drivers/media/usb/pvrusb2/pvrusb2-ctrl.c b/drivers/media/usb/pvrusb2/pvrusb2-ctrl.c >>>>> index 448c40caf363..b6c9bda214c8 100644 >>>>> --- a/drivers/media/usb/pvrusb2/pvrusb2-ctrl.c >>>>> +++ b/drivers/media/usb/pvrusb2/pvrusb2-ctrl.c >>>>> @@ -521,7 +521,7 @@ int pvr2_ctrl_value_to_sym_internal(struct pvr2_ctrl *cptr, >>>>> *len = scnprintf(buf,maxlen,"%d",val); >>>>> ret = 0; >>>>> } else if (cptr->info->type == pvr2_ctl_bool) { >>>>> - *len = scnprintf(buf,maxlen,"%s",str_true_false(val)); >>>>> + *len = scnprintf(buf, maxlen, "%s", str_true_false(val)); >>>>> ret = 0; >>>>> } else if (cptr->info->type == pvr2_ctl_enum) { >>>>> const char * const *names; >>>>> diff --git a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c >>>>> index 96d3a0045fac..761d718478ca 100644 >>>>> --- a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c >>>>> +++ b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c >>>>> @@ -338,7 +338,7 @@ static void trace_stbit(const char *name,int val) >>>>> { >>>>> pvr2_trace(PVR2_TRACE_STBITS, >>>>> "State bit %s <-- %s", >>>>> - name,str_true_false(val)); >>>>> + name, str_true_false(val)); >>>>> } >>>>> >>>>> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> >>>>> --- >>>>> Ricardo Ribalda (45): >>>>> media: staging: ipu3: Use string_choices helpers >>>>> media: staging: atomisp: Use string_choices helpers >>>>> media: core: Use string_choices helpers >>>>> media: pwc-ctl: Use string_choices helpers >>>>> media: pvrusb2:Use string_choices helpers >>>>> media: em28xx: Use string_choices helpers >>>>> media: dvb-usb: Use string_choices helpers >>>>> media: dvb-usb-v2: Use string_choices helpers >>>>> media: cx231xx: Use string_choices helpers >>>>> media: tuners: Use string_choices helpers >>>>> media: rc: Use string_choices helpers >>>>> media: dvb-frontends: Use string_choices helpers >>>>> media: pci: cx23885: Use string_choices helpers >>>>> media: saa7134: Use string_choices helpers >>>>> media: amphion: Use string_choices helpers >>>>> media: pci: ivtv: Use string_choices helpers >>>>> media: bttv: Use string_choices helpers >>>>> media: xilinx: Use string_choices helpers >>>>> media: platform: ti: Use string_choices helpers >>>>> media: st: Use string_choices helpers >>>>> media: coda: Use string_choices helpers >>>>> media: aspeed: Use string_choices helpers >>>>> media: ipu6: Use string_choices helpers >>>>> media: cx18: Use string_choices helpers >>>>> media: cobalt: Use string_choices helpers >>>>> media: videobuf2: Use string_choices helpers >>>>> media: cec: Use string_choices helpers >>>>> media: b2c2: Use string_choices helpers >>>>> media: siano: Use string_choices helpers >>>>> media: i2c: cx25840: Use string_choices helpers >>>>> media: i2c: vpx3220: Use string_choices helpers >>>>> media: i2c: tvp7002: Use string_choices helpers >>>>> media: i2c: ths8200: Use string_choices helpers >>>>> media: i2c: tda1997x: Use string_choices helpers >>>>> media: i2c: tc358743: Use string_choices helpers >>>>> media: i2c: st-mipid02: Use string_choices helpers >>>>> media: i2c: msp3400: Use string_choices helpers >>>>> media: i2c: max9286: Use string_choices helpers >>>>> media: i2c: saa717x: Use string_choices helpers >>>>> media: i2c: saa7127: Use string_choices helpers >>>>> media: i2c: saa7115: Use string_choices helpers >>>>> media: i2c: saa7110: Use string_choices helpers >>>>> media: i2c: adv7842: Use string_choices helpers >>>>> media: i2c: adv76xx: Use string_choices helpers >>>>> media: i2c: adv7511: Use string_choices helpers >>>>> >>>>> drivers/media/cec/platform/cec-gpio/cec-gpio.c | 4 +- >>>>> drivers/media/cec/usb/pulse8/pulse8-cec.c | 4 +- >>>>> drivers/media/common/b2c2/flexcop-hw-filter.c | 4 +- >>>>> drivers/media/common/siano/sms-cards.c | 3 +- >>>>> drivers/media/common/videobuf2/videobuf2-core.c | 5 ++- >>>>> drivers/media/dvb-frontends/ascot2e.c | 2 +- >>>>> drivers/media/dvb-frontends/cx24120.c | 4 +- >>>>> drivers/media/dvb-frontends/cxd2841er.c | 2 +- >>>>> drivers/media/dvb-frontends/drxk_hard.c | 4 +- >>>>> drivers/media/dvb-frontends/helene.c | 2 +- >>>>> drivers/media/dvb-frontends/horus3a.c | 2 +- >>>>> drivers/media/dvb-frontends/sp2.c | 2 +- >>>>> drivers/media/i2c/adv7511-v4l2.c | 11 +++--- >>>>> drivers/media/i2c/adv7604.c | 25 ++++++------ >>>>> drivers/media/i2c/adv7842.c | 40 ++++++++++---------- >>>>> drivers/media/i2c/cx25840/cx25840-core.c | 4 +- >>>>> drivers/media/i2c/cx25840/cx25840-ir.c | 34 ++++++++--------- >>>>> drivers/media/i2c/max9286.c | 2 +- >>>>> drivers/media/i2c/msp3400-driver.c | 4 +- >>>>> drivers/media/i2c/saa7110.c | 2 +- >>>>> drivers/media/i2c/saa7115.c | 2 +- >>>>> drivers/media/i2c/saa7127.c | 15 +++++--- >>>>> drivers/media/i2c/saa717x.c | 2 +- >>>>> drivers/media/i2c/st-mipid02.c | 2 +- >>>>> drivers/media/i2c/tc358743.c | 44 ++++++++++------------ >>>>> drivers/media/i2c/tda1997x.c | 6 +-- >>>>> drivers/media/i2c/ths8200.c | 4 +- >>>>> drivers/media/i2c/tvp7002.c | 2 +- >>>>> drivers/media/i2c/vpx3220.c | 2 +- >>>>> drivers/media/pci/bt8xx/bttv-cards.c | 16 ++++---- >>>>> drivers/media/pci/bt8xx/bttv-driver.c | 6 +-- >>>>> drivers/media/pci/cobalt/cobalt-driver.c | 2 +- >>>>> drivers/media/pci/cx18/cx18-av-core.c | 4 +- >>>>> drivers/media/pci/cx23885/altera-ci.c | 2 +- >>>>> drivers/media/pci/cx23885/cimax2.c | 2 +- >>>>> drivers/media/pci/cx23885/cx23888-ir.c | 36 +++++++++--------- >>>>> drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c | 2 +- >>>>> drivers/media/pci/ivtv/ivtvfb.c | 4 +- >>>>> drivers/media/pci/saa7134/saa7134-cards.c | 2 +- >>>>> drivers/media/pci/saa7134/saa7134-dvb.c | 2 +- >>>>> drivers/media/pci/saa7134/saa7134-input.c | 6 +-- >>>>> drivers/media/pci/saa7134/saa7134-video.c | 2 +- >>>>> drivers/media/platform/amphion/venc.c | 2 +- >>>>> drivers/media/platform/amphion/vpu_dbg.c | 2 +- >>>>> drivers/media/platform/aspeed/aspeed-video.c | 4 +- >>>>> drivers/media/platform/chips-media/coda/imx-vdoa.c | 3 +- >>>>> drivers/media/platform/st/sti/hva/hva-debugfs.c | 6 +-- >>>>> drivers/media/platform/ti/omap3isp/ispstat.c | 2 +- >>>>> drivers/media/platform/xilinx/xilinx-csi2rxss.c | 18 ++++----- >>>>> drivers/media/rc/ene_ir.c | 3 +- >>>>> drivers/media/rc/mceusb.c | 3 +- >>>>> drivers/media/rc/serial_ir.c | 5 ++- >>>>> drivers/media/tuners/tda18250.c | 2 +- >>>>> drivers/media/tuners/tda9887.c | 10 ++--- >>>>> drivers/media/usb/cx231xx/cx231xx-i2c.c | 4 +- >>>>> drivers/media/usb/cx231xx/cx231xx-video.c | 2 +- >>>>> drivers/media/usb/dvb-usb-v2/az6007.c | 4 +- >>>>> drivers/media/usb/dvb-usb-v2/dvb_usb_core.c | 4 +- >>>>> drivers/media/usb/dvb-usb/af9005-fe.c | 4 +- >>>>> drivers/media/usb/dvb-usb/dvb-usb-dvb.c | 6 +-- >>>>> drivers/media/usb/dvb-usb/opera1.c | 8 ++-- >>>>> drivers/media/usb/em28xx/em28xx-i2c.c | 4 +- >>>>> drivers/media/usb/em28xx/em28xx-video.c | 2 +- >>>>> drivers/media/usb/pvrusb2/pvrusb2-ctrl.c | 2 +- >>>>> drivers/media/usb/pvrusb2/pvrusb2-debugifc.c | 3 +- >>>>> drivers/media/usb/pvrusb2/pvrusb2-encoder.c | 5 +-- >>>>> drivers/media/usb/pvrusb2/pvrusb2-hdw.c | 6 +-- >>>>> drivers/media/usb/pvrusb2/pvrusb2-i2c-core.c | 3 +- >>>>> drivers/media/usb/pwc/pwc-ctrl.c | 2 +- >>>>> drivers/media/v4l2-core/v4l2-ctrls-core.c | 3 +- >>>>> drivers/media/v4l2-core/v4l2-fwnode.c | 12 +++--- >>>>> .../media/atomisp/pci/atomisp_compat_css20.c | 2 +- >>>>> .../media/atomisp/pci/atomisp_csi2_bridge.c | 2 +- >>>>> .../media/atomisp/pci/atomisp_gmin_platform.c | 4 +- >>>>> drivers/staging/media/atomisp/pci/atomisp_v4l2.c | 4 +- >>>>> .../media/atomisp/pci/runtime/binary/src/binary.c | 2 +- >>>>> drivers/staging/media/atomisp/pci/sh_css.c | 2 +- >>>>> drivers/staging/media/ipu3/ipu3-v4l2.c | 4 +- >>>>> 78 files changed, 240 insertions(+), 239 deletions(-) >>>>> --- >>>>> base-commit: 9852d85ec9d492ebef56dc5f229416c925758edc >>>>> change-id: 20240930-cocci-opportunity-40bca6a17c42 >>>> >>> >> >> >> -- >> Ricardo Ribalda >> >
> >> Julia, correct me if I am wrong, but I believe that cocci does not > >> have the ability to check only new code. It runs against files not > >> diffs. > > > > Coccinelle runs on files normally. There is an option --use-patch-diff > > where you can give a commit id, and it should only work on the files > > mentioned in that commit, but it doesn't seem to focus on only the new > > code in that commit. > > Is it possible to add support for that? It should be possible. The tricky part is that Coccinelle can match eg foo(); ... -bar() +xxx() So should the code be patched only if bar() is new? Or also if foo() is new. > And a related question: is it possible to skip certain tests? You can specify the test you want to have run, but I don't think there is a way to do the opposite: make coccicheck COCCI=<my_SP.cocci> MODE=patch Maybe if there is a way to focus on specific lines, the problem would go away? julia > > Basically, I don't want to see reports of code improvements, unless it is > for a new source file. Reports of dangerous/wrong constructs are always > welcome, but code improvements like these string_choice helpers really only > make sense for patches adding new source files. > > Regards, > > Hans > > > > > julia > > > >> > >> I agree with you, a "improvement cocci" shall not force us to drop the > >> pen and work full time to fix it :) > >> When a new "improvement" cocci check is landed, we should update our > >> "allowlist" immediately to ignore those warnings until we slowly > >> improve our codebase to meet the standard. > >> That way the CI is not affected. > >> > >> Regarding this patchset.... It is pretty big, but also pretty simple. > >> I added my "extra changes" on the cover letter to ease the review. > >> > >> If you or someone else have time to review it... then it will be great > >> if we land it. But if none has time for it, then it can be ignored. > >> > >> > >> Regards! > >> > >>> > >>> Hans > >>> > >>>> > >>>>> diff --git a/drivers/media/dvb-frontends/ascot2e.c b/drivers/media/dvb-frontends/ascot2e.c > >>>>> index 8c3eb5d69dda..ec7a718428fc 100644 > >>>>> --- a/drivers/media/dvb-frontends/ascot2e.c > >>>>> +++ b/drivers/media/dvb-frontends/ascot2e.c > >>>>> @@ -104,7 +104,7 @@ static void ascot2e_i2c_debug(struct ascot2e_priv *priv, > >>>>> u8 reg, u8 write, const u8 *data, u32 len) > >>>>> { > >>>>> dev_dbg(&priv->i2c->dev, "ascot2e: I2C %s reg 0x%02x size %d\n", > >>>>> - str_read_write(write == 0), reg, len); > >>>>> + str_write_read(write), reg, len); > >>>>> print_hex_dump_bytes("ascot2e: I2C data: ", > >>>>> DUMP_PREFIX_OFFSET, data, len); > >>>>> } > >>>>> diff --git a/drivers/media/dvb-frontends/cxd2841er.c b/drivers/media/dvb-frontends/cxd2841er.c > >>>>> index db684f314b47..d1b84cd9c510 100644 > >>>>> --- a/drivers/media/dvb-frontends/cxd2841er.c > >>>>> +++ b/drivers/media/dvb-frontends/cxd2841er.c > >>>>> @@ -206,7 +206,7 @@ static void cxd2841er_i2c_debug(struct cxd2841er_priv *priv, > >>>>> { > >>>>> dev_dbg(&priv->i2c->dev, > >>>>> "cxd2841er: I2C %s addr %02x reg 0x%02x size %d data %*ph\n", > >>>>> - str_read_write(write == 0), addr, reg, len, len, data); > >>>>> + str_write_read(write), addr, reg, len, len, data); > >>>>> } > >>>>> > >>>>> static int cxd2841er_write_regs(struct cxd2841er_priv *priv, > >>>>> diff --git a/drivers/media/dvb-frontends/helene.c b/drivers/media/dvb-frontends/helene.c > >>>>> index 52198cb49dba..b4527c141d9c 100644 > >>>>> --- a/drivers/media/dvb-frontends/helene.c > >>>>> +++ b/drivers/media/dvb-frontends/helene.c > >>>>> @@ -279,7 +279,7 @@ static void helene_i2c_debug(struct helene_priv *priv, > >>>>> u8 reg, u8 write, const u8 *data, u32 len) > >>>>> { > >>>>> dev_dbg(&priv->i2c->dev, "helene: I2C %s reg 0x%02x size %d\n", > >>>>> - str_read_write(write == 0), reg, len); > >>>>> + str_write_read(write), reg, len); > >>>>> print_hex_dump_bytes("helene: I2C data: ", > >>>>> DUMP_PREFIX_OFFSET, data, len); > >>>>> } > >>>>> diff --git a/drivers/media/dvb-frontends/horus3a.c b/drivers/media/dvb-frontends/horus3a.c > >>>>> index 84385079918c..10300ebf3ca0 100644 > >>>>> --- a/drivers/media/dvb-frontends/horus3a.c > >>>>> +++ b/drivers/media/dvb-frontends/horus3a.c > >>>>> @@ -38,7 +38,7 @@ static void horus3a_i2c_debug(struct horus3a_priv *priv, > >>>>> u8 reg, u8 write, const u8 *data, u32 len) > >>>>> { > >>>>> dev_dbg(&priv->i2c->dev, "horus3a: I2C %s reg 0x%02x size %d\n", > >>>>> - str_read_write(write == 0), reg, len); > >>>>> + str_write_read(write), reg, len); > >>>>> print_hex_dump_bytes("horus3a: I2C data: ", > >>>>> DUMP_PREFIX_OFFSET, data, len); > >>>>> } > >>>>> diff --git a/drivers/media/i2c/adv7842.c b/drivers/media/i2c/adv7842.c > >>>>> index ba174aa45afa..a43479c3ff03 100644 > >>>>> --- a/drivers/media/i2c/adv7842.c > >>>>> +++ b/drivers/media/i2c/adv7842.c > >>>>> @@ -2763,7 +2763,7 @@ static int adv7842_cp_log_status(struct v4l2_subdev *sd) > >>>>> str_true_false(io_read(sd, 0x6a) & 0x10)); > >>>>> } > >>>>> v4l2_info(sd, "CP free run: %s\n", > >>>>> - str_on_off(!!(cp_read(sd, 0xff) & 0x10))); > >>>>> + str_on_off(cp_read(sd, 0xff) & 0x10)); > >>>>> v4l2_info(sd, "Prim-mode = 0x%x, video std = 0x%x, v_freq = 0x%x\n", > >>>>> io_read(sd, 0x01) & 0x0f, io_read(sd, 0x00) & 0x3f, > >>>>> (io_read(sd, 0x01) & 0x70) >> 4); > >>>>> diff --git a/drivers/media/pci/saa7134/saa7134-cards.c b/drivers/media/pci/saa7134/saa7134-cards.c > >>>>> index 301b89e799d8..79cd61fb0205 100644 > >>>>> --- a/drivers/media/pci/saa7134/saa7134-cards.c > >>>>> +++ b/drivers/media/pci/saa7134/saa7134-cards.c > >>>>> @@ -7981,7 +7981,7 @@ int saa7134_board_init2(struct saa7134_dev *dev) > >>>>> rc = i2c_transfer(&dev->i2c_adap, &msg, 1); > >>>>> pr_info("%s: probe IR chip @ i2c 0x%02x: %s\n", > >>>>> dev->name, msg.addr, > >>>>> - str_yes_no(1 == rc)); > >>>>> + str_yes_no(rc == 1)); > >>>>> if (rc == 1) > >>>>> dev->has_remote = SAA7134_REMOTE_I2C; > >>>>> } > >>>>> diff --git a/drivers/media/pci/saa7134/saa7134-input.c b/drivers/media/pci/saa7134/saa7134-input.c > >>>>> index 90837ec6e70f..239f0b9d080a 100644 > >>>>> --- a/drivers/media/pci/saa7134/saa7134-input.c > >>>>> +++ b/drivers/media/pci/saa7134/saa7134-input.c > >>>>> @@ -895,7 +895,7 @@ void saa7134_probe_i2c_ir(struct saa7134_dev *dev) > >>>>> rc = i2c_transfer(&dev->i2c_adap, &msg_msi, 1); > >>>>> input_dbg("probe 0x%02x @ %s: %s\n", > >>>>> msg_msi.addr, dev->i2c_adap.name, > >>>>> - str_yes_no(1 == rc)); > >>>>> + str_yes_no(rc == 1)); > >>>>> break; > >>>>> case SAA7134_BOARD_SNAZIO_TVPVR_PRO: > >>>>> dev->init_data.name = "SnaZio* TVPVR PRO"; > >>>>> @@ -931,7 +931,7 @@ void saa7134_probe_i2c_ir(struct saa7134_dev *dev) > >>>>> rc = i2c_transfer(&dev->i2c_adap, &msg_msi, 1); > >>>>> input_dbg("probe 0x%02x @ %s: %s\n", > >>>>> msg_msi.addr, dev->i2c_adap.name, > >>>>> - str_yes_no(1 == rc)); > >>>>> + str_yes_no(rc == 1)); > >>>>> break; > >>>>> case SAA7134_BOARD_HAUPPAUGE_HVR1110: > >>>>> dev->init_data.name = saa7134_boards[dev->board].name; > >>>>> diff --git a/drivers/media/usb/pvrusb2/pvrusb2-ctrl.c b/drivers/media/usb/pvrusb2/pvrusb2-ctrl.c > >>>>> index 448c40caf363..b6c9bda214c8 100644 > >>>>> --- a/drivers/media/usb/pvrusb2/pvrusb2-ctrl.c > >>>>> +++ b/drivers/media/usb/pvrusb2/pvrusb2-ctrl.c > >>>>> @@ -521,7 +521,7 @@ int pvr2_ctrl_value_to_sym_internal(struct pvr2_ctrl *cptr, > >>>>> *len = scnprintf(buf,maxlen,"%d",val); > >>>>> ret = 0; > >>>>> } else if (cptr->info->type == pvr2_ctl_bool) { > >>>>> - *len = scnprintf(buf,maxlen,"%s",str_true_false(val)); > >>>>> + *len = scnprintf(buf, maxlen, "%s", str_true_false(val)); > >>>>> ret = 0; > >>>>> } else if (cptr->info->type == pvr2_ctl_enum) { > >>>>> const char * const *names; > >>>>> diff --git a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c > >>>>> index 96d3a0045fac..761d718478ca 100644 > >>>>> --- a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c > >>>>> +++ b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c > >>>>> @@ -338,7 +338,7 @@ static void trace_stbit(const char *name,int val) > >>>>> { > >>>>> pvr2_trace(PVR2_TRACE_STBITS, > >>>>> "State bit %s <-- %s", > >>>>> - name,str_true_false(val)); > >>>>> + name, str_true_false(val)); > >>>>> } > >>>>> > >>>>> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> > >>>>> --- > >>>>> Ricardo Ribalda (45): > >>>>> media: staging: ipu3: Use string_choices helpers > >>>>> media: staging: atomisp: Use string_choices helpers > >>>>> media: core: Use string_choices helpers > >>>>> media: pwc-ctl: Use string_choices helpers > >>>>> media: pvrusb2:Use string_choices helpers > >>>>> media: em28xx: Use string_choices helpers > >>>>> media: dvb-usb: Use string_choices helpers > >>>>> media: dvb-usb-v2: Use string_choices helpers > >>>>> media: cx231xx: Use string_choices helpers > >>>>> media: tuners: Use string_choices helpers > >>>>> media: rc: Use string_choices helpers > >>>>> media: dvb-frontends: Use string_choices helpers > >>>>> media: pci: cx23885: Use string_choices helpers > >>>>> media: saa7134: Use string_choices helpers > >>>>> media: amphion: Use string_choices helpers > >>>>> media: pci: ivtv: Use string_choices helpers > >>>>> media: bttv: Use string_choices helpers > >>>>> media: xilinx: Use string_choices helpers > >>>>> media: platform: ti: Use string_choices helpers > >>>>> media: st: Use string_choices helpers > >>>>> media: coda: Use string_choices helpers > >>>>> media: aspeed: Use string_choices helpers > >>>>> media: ipu6: Use string_choices helpers > >>>>> media: cx18: Use string_choices helpers > >>>>> media: cobalt: Use string_choices helpers > >>>>> media: videobuf2: Use string_choices helpers > >>>>> media: cec: Use string_choices helpers > >>>>> media: b2c2: Use string_choices helpers > >>>>> media: siano: Use string_choices helpers > >>>>> media: i2c: cx25840: Use string_choices helpers > >>>>> media: i2c: vpx3220: Use string_choices helpers > >>>>> media: i2c: tvp7002: Use string_choices helpers > >>>>> media: i2c: ths8200: Use string_choices helpers > >>>>> media: i2c: tda1997x: Use string_choices helpers > >>>>> media: i2c: tc358743: Use string_choices helpers > >>>>> media: i2c: st-mipid02: Use string_choices helpers > >>>>> media: i2c: msp3400: Use string_choices helpers > >>>>> media: i2c: max9286: Use string_choices helpers > >>>>> media: i2c: saa717x: Use string_choices helpers > >>>>> media: i2c: saa7127: Use string_choices helpers > >>>>> media: i2c: saa7115: Use string_choices helpers > >>>>> media: i2c: saa7110: Use string_choices helpers > >>>>> media: i2c: adv7842: Use string_choices helpers > >>>>> media: i2c: adv76xx: Use string_choices helpers > >>>>> media: i2c: adv7511: Use string_choices helpers > >>>>> > >>>>> drivers/media/cec/platform/cec-gpio/cec-gpio.c | 4 +- > >>>>> drivers/media/cec/usb/pulse8/pulse8-cec.c | 4 +- > >>>>> drivers/media/common/b2c2/flexcop-hw-filter.c | 4 +- > >>>>> drivers/media/common/siano/sms-cards.c | 3 +- > >>>>> drivers/media/common/videobuf2/videobuf2-core.c | 5 ++- > >>>>> drivers/media/dvb-frontends/ascot2e.c | 2 +- > >>>>> drivers/media/dvb-frontends/cx24120.c | 4 +- > >>>>> drivers/media/dvb-frontends/cxd2841er.c | 2 +- > >>>>> drivers/media/dvb-frontends/drxk_hard.c | 4 +- > >>>>> drivers/media/dvb-frontends/helene.c | 2 +- > >>>>> drivers/media/dvb-frontends/horus3a.c | 2 +- > >>>>> drivers/media/dvb-frontends/sp2.c | 2 +- > >>>>> drivers/media/i2c/adv7511-v4l2.c | 11 +++--- > >>>>> drivers/media/i2c/adv7604.c | 25 ++++++------ > >>>>> drivers/media/i2c/adv7842.c | 40 ++++++++++---------- > >>>>> drivers/media/i2c/cx25840/cx25840-core.c | 4 +- > >>>>> drivers/media/i2c/cx25840/cx25840-ir.c | 34 ++++++++--------- > >>>>> drivers/media/i2c/max9286.c | 2 +- > >>>>> drivers/media/i2c/msp3400-driver.c | 4 +- > >>>>> drivers/media/i2c/saa7110.c | 2 +- > >>>>> drivers/media/i2c/saa7115.c | 2 +- > >>>>> drivers/media/i2c/saa7127.c | 15 +++++--- > >>>>> drivers/media/i2c/saa717x.c | 2 +- > >>>>> drivers/media/i2c/st-mipid02.c | 2 +- > >>>>> drivers/media/i2c/tc358743.c | 44 ++++++++++------------ > >>>>> drivers/media/i2c/tda1997x.c | 6 +-- > >>>>> drivers/media/i2c/ths8200.c | 4 +- > >>>>> drivers/media/i2c/tvp7002.c | 2 +- > >>>>> drivers/media/i2c/vpx3220.c | 2 +- > >>>>> drivers/media/pci/bt8xx/bttv-cards.c | 16 ++++---- > >>>>> drivers/media/pci/bt8xx/bttv-driver.c | 6 +-- > >>>>> drivers/media/pci/cobalt/cobalt-driver.c | 2 +- > >>>>> drivers/media/pci/cx18/cx18-av-core.c | 4 +- > >>>>> drivers/media/pci/cx23885/altera-ci.c | 2 +- > >>>>> drivers/media/pci/cx23885/cimax2.c | 2 +- > >>>>> drivers/media/pci/cx23885/cx23888-ir.c | 36 +++++++++--------- > >>>>> drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c | 2 +- > >>>>> drivers/media/pci/ivtv/ivtvfb.c | 4 +- > >>>>> drivers/media/pci/saa7134/saa7134-cards.c | 2 +- > >>>>> drivers/media/pci/saa7134/saa7134-dvb.c | 2 +- > >>>>> drivers/media/pci/saa7134/saa7134-input.c | 6 +-- > >>>>> drivers/media/pci/saa7134/saa7134-video.c | 2 +- > >>>>> drivers/media/platform/amphion/venc.c | 2 +- > >>>>> drivers/media/platform/amphion/vpu_dbg.c | 2 +- > >>>>> drivers/media/platform/aspeed/aspeed-video.c | 4 +- > >>>>> drivers/media/platform/chips-media/coda/imx-vdoa.c | 3 +- > >>>>> drivers/media/platform/st/sti/hva/hva-debugfs.c | 6 +-- > >>>>> drivers/media/platform/ti/omap3isp/ispstat.c | 2 +- > >>>>> drivers/media/platform/xilinx/xilinx-csi2rxss.c | 18 ++++----- > >>>>> drivers/media/rc/ene_ir.c | 3 +- > >>>>> drivers/media/rc/mceusb.c | 3 +- > >>>>> drivers/media/rc/serial_ir.c | 5 ++- > >>>>> drivers/media/tuners/tda18250.c | 2 +- > >>>>> drivers/media/tuners/tda9887.c | 10 ++--- > >>>>> drivers/media/usb/cx231xx/cx231xx-i2c.c | 4 +- > >>>>> drivers/media/usb/cx231xx/cx231xx-video.c | 2 +- > >>>>> drivers/media/usb/dvb-usb-v2/az6007.c | 4 +- > >>>>> drivers/media/usb/dvb-usb-v2/dvb_usb_core.c | 4 +- > >>>>> drivers/media/usb/dvb-usb/af9005-fe.c | 4 +- > >>>>> drivers/media/usb/dvb-usb/dvb-usb-dvb.c | 6 +-- > >>>>> drivers/media/usb/dvb-usb/opera1.c | 8 ++-- > >>>>> drivers/media/usb/em28xx/em28xx-i2c.c | 4 +- > >>>>> drivers/media/usb/em28xx/em28xx-video.c | 2 +- > >>>>> drivers/media/usb/pvrusb2/pvrusb2-ctrl.c | 2 +- > >>>>> drivers/media/usb/pvrusb2/pvrusb2-debugifc.c | 3 +- > >>>>> drivers/media/usb/pvrusb2/pvrusb2-encoder.c | 5 +-- > >>>>> drivers/media/usb/pvrusb2/pvrusb2-hdw.c | 6 +-- > >>>>> drivers/media/usb/pvrusb2/pvrusb2-i2c-core.c | 3 +- > >>>>> drivers/media/usb/pwc/pwc-ctrl.c | 2 +- > >>>>> drivers/media/v4l2-core/v4l2-ctrls-core.c | 3 +- > >>>>> drivers/media/v4l2-core/v4l2-fwnode.c | 12 +++--- > >>>>> .../media/atomisp/pci/atomisp_compat_css20.c | 2 +- > >>>>> .../media/atomisp/pci/atomisp_csi2_bridge.c | 2 +- > >>>>> .../media/atomisp/pci/atomisp_gmin_platform.c | 4 +- > >>>>> drivers/staging/media/atomisp/pci/atomisp_v4l2.c | 4 +- > >>>>> .../media/atomisp/pci/runtime/binary/src/binary.c | 2 +- > >>>>> drivers/staging/media/atomisp/pci/sh_css.c | 2 +- > >>>>> drivers/staging/media/ipu3/ipu3-v4l2.c | 4 +- > >>>>> 78 files changed, 240 insertions(+), 239 deletions(-) > >>>>> --- > >>>>> base-commit: 9852d85ec9d492ebef56dc5f229416c925758edc > >>>>> change-id: 20240930-cocci-opportunity-40bca6a17c42 > >>>> > >>> > >> > >> > >> -- > >> Ricardo Ribalda > >> > > > >
diff --git a/drivers/media/dvb-frontends/ascot2e.c b/drivers/media/dvb-frontends/ascot2e.c index 8c3eb5d69dda..ec7a718428fc 100644 --- a/drivers/media/dvb-frontends/ascot2e.c +++ b/drivers/media/dvb-frontends/ascot2e.c @@ -104,7 +104,7 @@ static void ascot2e_i2c_debug(struct ascot2e_priv *priv, u8 reg, u8 write, const u8 *data, u32 len) { dev_dbg(&priv->i2c->dev, "ascot2e: I2C %s reg 0x%02x size %d\n", - str_read_write(write == 0), reg, len); + str_write_read(write), reg, len); print_hex_dump_bytes("ascot2e: I2C data: ", DUMP_PREFIX_OFFSET, data, len); } diff --git a/drivers/media/dvb-frontends/cxd2841er.c b/drivers/media/dvb-frontends/cxd2841er.c index db684f314b47..d1b84cd9c510 100644 --- a/drivers/media/dvb-frontends/cxd2841er.c +++ b/drivers/media/dvb-frontends/cxd2841er.c @@ -206,7 +206,7 @@ static void cxd2841er_i2c_debug(struct cxd2841er_priv *priv, { dev_dbg(&priv->i2c->dev, "cxd2841er: I2C %s addr %02x reg 0x%02x size %d data %*ph\n", - str_read_write(write == 0), addr, reg, len, len, data); + str_write_read(write), addr, reg, len, len, data); } static int cxd2841er_write_regs(struct cxd2841er_priv *priv, diff --git a/drivers/media/dvb-frontends/helene.c b/drivers/media/dvb-frontends/helene.c index 52198cb49dba..b4527c141d9c 100644 --- a/drivers/media/dvb-frontends/helene.c +++ b/drivers/media/dvb-frontends/helene.c @@ -279,7 +279,7 @@ static void helene_i2c_debug(struct helene_priv *priv, u8 reg, u8 write, const u8 *data, u32 len) { dev_dbg(&priv->i2c->dev, "helene: I2C %s reg 0x%02x size %d\n", - str_read_write(write == 0), reg, len); + str_write_read(write), reg, len); print_hex_dump_bytes("helene: I2C data: ", DUMP_PREFIX_OFFSET, data, len); } diff --git a/drivers/media/dvb-frontends/horus3a.c b/drivers/media/dvb-frontends/horus3a.c index 84385079918c..10300ebf3ca0 100644 --- a/drivers/media/dvb-frontends/horus3a.c +++ b/drivers/media/dvb-frontends/horus3a.c @@ -38,7 +38,7 @@ static void horus3a_i2c_debug(struct horus3a_priv *priv, u8 reg, u8 write, const u8 *data, u32 len) { dev_dbg(&priv->i2c->dev, "horus3a: I2C %s reg 0x%02x size %d\n", - str_read_write(write == 0), reg, len); + str_write_read(write), reg, len); print_hex_dump_bytes("horus3a: I2C data: ", DUMP_PREFIX_OFFSET, data, len); } diff --git a/drivers/media/i2c/adv7842.c b/drivers/media/i2c/adv7842.c index ba174aa45afa..a43479c3ff03 100644 --- a/drivers/media/i2c/adv7842.c +++ b/drivers/media/i2c/adv7842.c @@ -2763,7 +2763,7 @@ static int adv7842_cp_log_status(struct v4l2_subdev *sd) str_true_false(io_read(sd, 0x6a) & 0x10)); } v4l2_info(sd, "CP free run: %s\n", - str_on_off(!!(cp_read(sd, 0xff) & 0x10))); + str_on_off(cp_read(sd, 0xff) & 0x10)); v4l2_info(sd, "Prim-mode = 0x%x, video std = 0x%x, v_freq = 0x%x\n", io_read(sd, 0x01) & 0x0f, io_read(sd, 0x00) & 0x3f, (io_read(sd, 0x01) & 0x70) >> 4); diff --git a/drivers/media/pci/saa7134/saa7134-cards.c b/drivers/media/pci/saa7134/saa7134-cards.c index 301b89e799d8..79cd61fb0205 100644 --- a/drivers/media/pci/saa7134/saa7134-cards.c +++ b/drivers/media/pci/saa7134/saa7134-cards.c @@ -7981,7 +7981,7 @@ int saa7134_board_init2(struct saa7134_dev *dev) rc = i2c_transfer(&dev->i2c_adap, &msg, 1); pr_info("%s: probe IR chip @ i2c 0x%02x: %s\n", dev->name, msg.addr, - str_yes_no(1 == rc)); + str_yes_no(rc == 1)); if (rc == 1) dev->has_remote = SAA7134_REMOTE_I2C; } diff --git a/drivers/media/pci/saa7134/saa7134-input.c b/drivers/media/pci/saa7134/saa7134-input.c index 90837ec6e70f..239f0b9d080a 100644 --- a/drivers/media/pci/saa7134/saa7134-input.c +++ b/drivers/media/pci/saa7134/saa7134-input.c @@ -895,7 +895,7 @@ void saa7134_probe_i2c_ir(struct saa7134_dev *dev) rc = i2c_transfer(&dev->i2c_adap, &msg_msi, 1); input_dbg("probe 0x%02x @ %s: %s\n", msg_msi.addr, dev->i2c_adap.name, - str_yes_no(1 == rc)); + str_yes_no(rc == 1)); break; case SAA7134_BOARD_SNAZIO_TVPVR_PRO: dev->init_data.name = "SnaZio* TVPVR PRO"; @@ -931,7 +931,7 @@ void saa7134_probe_i2c_ir(struct saa7134_dev *dev) rc = i2c_transfer(&dev->i2c_adap, &msg_msi, 1); input_dbg("probe 0x%02x @ %s: %s\n", msg_msi.addr, dev->i2c_adap.name, - str_yes_no(1 == rc)); + str_yes_no(rc == 1)); break; case SAA7134_BOARD_HAUPPAUGE_HVR1110: dev->init_data.name = saa7134_boards[dev->board].name; diff --git a/drivers/media/usb/pvrusb2/pvrusb2-ctrl.c b/drivers/media/usb/pvrusb2/pvrusb2-ctrl.c index 448c40caf363..b6c9bda214c8 100644 --- a/drivers/media/usb/pvrusb2/pvrusb2-ctrl.c +++ b/drivers/media/usb/pvrusb2/pvrusb2-ctrl.c @@ -521,7 +521,7 @@ int pvr2_ctrl_value_to_sym_internal(struct pvr2_ctrl *cptr, *len = scnprintf(buf,maxlen,"%d",val); ret = 0; } else if (cptr->info->type == pvr2_ctl_bool) { - *len = scnprintf(buf,maxlen,"%s",str_true_false(val)); + *len = scnprintf(buf, maxlen, "%s", str_true_false(val)); ret = 0; } else if (cptr->info->type == pvr2_ctl_enum) { const char * const *names; diff --git a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c index 96d3a0045fac..761d718478ca 100644 --- a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c +++ b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c @@ -338,7 +338,7 @@ static void trace_stbit(const char *name,int val) { pvr2_trace(PVR2_TRACE_STBITS, "State bit %s <-- %s", - name,str_true_false(val)); + name, str_true_false(val)); } Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
include/linux/string_choices.h contains a set of helpers that can be used instead of hard coding some strings. Cocci has located some places where the helpers can be used. This patchset uses the diff generated by cocci, plus these changes: --- Ricardo Ribalda (45): media: staging: ipu3: Use string_choices helpers media: staging: atomisp: Use string_choices helpers media: core: Use string_choices helpers media: pwc-ctl: Use string_choices helpers media: pvrusb2:Use string_choices helpers media: em28xx: Use string_choices helpers media: dvb-usb: Use string_choices helpers media: dvb-usb-v2: Use string_choices helpers media: cx231xx: Use string_choices helpers media: tuners: Use string_choices helpers media: rc: Use string_choices helpers media: dvb-frontends: Use string_choices helpers media: pci: cx23885: Use string_choices helpers media: saa7134: Use string_choices helpers media: amphion: Use string_choices helpers media: pci: ivtv: Use string_choices helpers media: bttv: Use string_choices helpers media: xilinx: Use string_choices helpers media: platform: ti: Use string_choices helpers media: st: Use string_choices helpers media: coda: Use string_choices helpers media: aspeed: Use string_choices helpers media: ipu6: Use string_choices helpers media: cx18: Use string_choices helpers media: cobalt: Use string_choices helpers media: videobuf2: Use string_choices helpers media: cec: Use string_choices helpers media: b2c2: Use string_choices helpers media: siano: Use string_choices helpers media: i2c: cx25840: Use string_choices helpers media: i2c: vpx3220: Use string_choices helpers media: i2c: tvp7002: Use string_choices helpers media: i2c: ths8200: Use string_choices helpers media: i2c: tda1997x: Use string_choices helpers media: i2c: tc358743: Use string_choices helpers media: i2c: st-mipid02: Use string_choices helpers media: i2c: msp3400: Use string_choices helpers media: i2c: max9286: Use string_choices helpers media: i2c: saa717x: Use string_choices helpers media: i2c: saa7127: Use string_choices helpers media: i2c: saa7115: Use string_choices helpers media: i2c: saa7110: Use string_choices helpers media: i2c: adv7842: Use string_choices helpers media: i2c: adv76xx: Use string_choices helpers media: i2c: adv7511: Use string_choices helpers drivers/media/cec/platform/cec-gpio/cec-gpio.c | 4 +- drivers/media/cec/usb/pulse8/pulse8-cec.c | 4 +- drivers/media/common/b2c2/flexcop-hw-filter.c | 4 +- drivers/media/common/siano/sms-cards.c | 3 +- drivers/media/common/videobuf2/videobuf2-core.c | 5 ++- drivers/media/dvb-frontends/ascot2e.c | 2 +- drivers/media/dvb-frontends/cx24120.c | 4 +- drivers/media/dvb-frontends/cxd2841er.c | 2 +- drivers/media/dvb-frontends/drxk_hard.c | 4 +- drivers/media/dvb-frontends/helene.c | 2 +- drivers/media/dvb-frontends/horus3a.c | 2 +- drivers/media/dvb-frontends/sp2.c | 2 +- drivers/media/i2c/adv7511-v4l2.c | 11 +++--- drivers/media/i2c/adv7604.c | 25 ++++++------ drivers/media/i2c/adv7842.c | 40 ++++++++++---------- drivers/media/i2c/cx25840/cx25840-core.c | 4 +- drivers/media/i2c/cx25840/cx25840-ir.c | 34 ++++++++--------- drivers/media/i2c/max9286.c | 2 +- drivers/media/i2c/msp3400-driver.c | 4 +- drivers/media/i2c/saa7110.c | 2 +- drivers/media/i2c/saa7115.c | 2 +- drivers/media/i2c/saa7127.c | 15 +++++--- drivers/media/i2c/saa717x.c | 2 +- drivers/media/i2c/st-mipid02.c | 2 +- drivers/media/i2c/tc358743.c | 44 ++++++++++------------ drivers/media/i2c/tda1997x.c | 6 +-- drivers/media/i2c/ths8200.c | 4 +- drivers/media/i2c/tvp7002.c | 2 +- drivers/media/i2c/vpx3220.c | 2 +- drivers/media/pci/bt8xx/bttv-cards.c | 16 ++++---- drivers/media/pci/bt8xx/bttv-driver.c | 6 +-- drivers/media/pci/cobalt/cobalt-driver.c | 2 +- drivers/media/pci/cx18/cx18-av-core.c | 4 +- drivers/media/pci/cx23885/altera-ci.c | 2 +- drivers/media/pci/cx23885/cimax2.c | 2 +- drivers/media/pci/cx23885/cx23888-ir.c | 36 +++++++++--------- drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c | 2 +- drivers/media/pci/ivtv/ivtvfb.c | 4 +- drivers/media/pci/saa7134/saa7134-cards.c | 2 +- drivers/media/pci/saa7134/saa7134-dvb.c | 2 +- drivers/media/pci/saa7134/saa7134-input.c | 6 +-- drivers/media/pci/saa7134/saa7134-video.c | 2 +- drivers/media/platform/amphion/venc.c | 2 +- drivers/media/platform/amphion/vpu_dbg.c | 2 +- drivers/media/platform/aspeed/aspeed-video.c | 4 +- drivers/media/platform/chips-media/coda/imx-vdoa.c | 3 +- drivers/media/platform/st/sti/hva/hva-debugfs.c | 6 +-- drivers/media/platform/ti/omap3isp/ispstat.c | 2 +- drivers/media/platform/xilinx/xilinx-csi2rxss.c | 18 ++++----- drivers/media/rc/ene_ir.c | 3 +- drivers/media/rc/mceusb.c | 3 +- drivers/media/rc/serial_ir.c | 5 ++- drivers/media/tuners/tda18250.c | 2 +- drivers/media/tuners/tda9887.c | 10 ++--- drivers/media/usb/cx231xx/cx231xx-i2c.c | 4 +- drivers/media/usb/cx231xx/cx231xx-video.c | 2 +- drivers/media/usb/dvb-usb-v2/az6007.c | 4 +- drivers/media/usb/dvb-usb-v2/dvb_usb_core.c | 4 +- drivers/media/usb/dvb-usb/af9005-fe.c | 4 +- drivers/media/usb/dvb-usb/dvb-usb-dvb.c | 6 +-- drivers/media/usb/dvb-usb/opera1.c | 8 ++-- drivers/media/usb/em28xx/em28xx-i2c.c | 4 +- drivers/media/usb/em28xx/em28xx-video.c | 2 +- drivers/media/usb/pvrusb2/pvrusb2-ctrl.c | 2 +- drivers/media/usb/pvrusb2/pvrusb2-debugifc.c | 3 +- drivers/media/usb/pvrusb2/pvrusb2-encoder.c | 5 +-- drivers/media/usb/pvrusb2/pvrusb2-hdw.c | 6 +-- drivers/media/usb/pvrusb2/pvrusb2-i2c-core.c | 3 +- drivers/media/usb/pwc/pwc-ctrl.c | 2 +- drivers/media/v4l2-core/v4l2-ctrls-core.c | 3 +- drivers/media/v4l2-core/v4l2-fwnode.c | 12 +++--- .../media/atomisp/pci/atomisp_compat_css20.c | 2 +- .../media/atomisp/pci/atomisp_csi2_bridge.c | 2 +- .../media/atomisp/pci/atomisp_gmin_platform.c | 4 +- drivers/staging/media/atomisp/pci/atomisp_v4l2.c | 4 +- .../media/atomisp/pci/runtime/binary/src/binary.c | 2 +- drivers/staging/media/atomisp/pci/sh_css.c | 2 +- drivers/staging/media/ipu3/ipu3-v4l2.c | 4 +- 78 files changed, 240 insertions(+), 239 deletions(-) --- base-commit: 9852d85ec9d492ebef56dc5f229416c925758edc change-id: 20240930-cocci-opportunity-40bca6a17c42 Best regards,