On Wed, 2014-03-05 at 13:02 -0800, Stephen Boyd wrote:
> On 03/05/14 11:29, Josh Cartwright wrote:
> > Before performing additional cleanups to this driver, do the easy
> > cleanups first.
> >
> > Signed-off-by: Josh Cartwright <[email protected]>
>
> Reviewed-by: Stephen Boyd <[email protected]>
>
> > @@ -253,7 +253,7 @@ static int pm8xxx_rtc_set_alarm(struct device *dev,
> > struct rtc_wkalrm *alarm)
> >
> > ctrl_reg = rtc_dd->ctrl_reg;
> > ctrl_reg = alarm->enabled ? (ctrl_reg | PM8xxx_RTC_ALARM_ENABLE) :
> > - (ctrl_reg & ~PM8xxx_RTC_ALARM_ENABLE);
> > + (ctrl_reg & ~PM8xxx_RTC_ALARM_ENABLE);
> >
> > rc = pm8xxx_write_wrapper(rtc_dd, &ctrl_reg, rtc_dd->rtc_base, 1);
> > if (rc < 0) {
>
> This could be better style with more lines:
>
> if (alarm->enabled)
> ctrl_reg |= PM8xxx_RTC_ALARM_ENABLE;
> else
> ctrl_reg &= ~PM8xxx_RTC_ALARM_ENABLE;
>
I agree.
Maybe something like a set_or_clear_bits macro similar to clamp
could be useful.
Maybe:
#define set_or_clear_bits(value, test, bits) \
({ \
typeof value _value; \
if (test) \
_value = (value) | (bits); \
else \
_value = (value) & (~(bits)); \
_value; \
})
so the code above could be something like:
ctrl_reg = set_or_clear_bits(ctrl_reg, alarm->enabled,
PM8xxx_RTC_ALARM_ENABLE);
(or some other better macro name, or maybe not type
the value name twice and have it set by the macro)
There's at least a few dozen like alarm->enabled use
above that could be converted in drivers/.
( a few false positives here too )
$ grep-2.5.4 -rP --include=*.[ch]
"([\w\.\>\[\]\-]+)\s*=[^;\?]+\?\s*\(?\s*\1\s*[\&\|][^;]+;" drivers/
drivers/input/misc/twl4030-vibra.c: reg = (dir) ? (reg |
TWL4030_VIBRA_DIR) :
(reg & ~TWL4030_VIBRA_DIR);
drivers/ata/libata-scsi.c: tf->device = dev->devno ?
tf->device | ATA_DEV1 : tf->device & ~ATA_DEV1;
drivers/ata/pata_samsung_cf.c: reg = mode ? (reg & ~S3C_ATA_CFG_SWAP) : (reg |
S3C_ATA_CFG_SWAP);
drivers/ata/pata_samsung_cf.c: temp = state ? (temp | 1) : (temp & ~1);
drivers/staging/iio/accel/lis3l02dq_core.c: control = val & 0x3f ?
(control | LIS3L02DQ_REG_CTRL_2_ENABLE_INTERRUPT) :
(control & ~LIS3L02DQ_REG_CTRL_2_ENABLE_INTERRUPT);
drivers/staging/media/sn9c102/sn9c102_core.c: rect->left = (s->_rect.left &
1L) ? rect->left | 1L : rect->left & ~1L;
drivers/staging/media/sn9c102/sn9c102_core.c: rect->top = (s->_rect.top & 1L)
? rect->top | 1L : rect->top & ~1L;
drivers/message/fusion/mptsas.c: ioc->device_missing_delay =
(device_missing_delay & MPI_SAS_IOUNIT1_REPORT_MISSING_UNIT_16) ?
(device_missing_delay &
MPI_SAS_IOUNIT1_REPORT_MISSING_TIMEOUT_MASK) * 16 :
device_missing_delay & MPI_SAS_IOUNIT1_REPORT_MISSING_TIMEOUT_MASK;
drivers/gpio/gpio-max732x.c: reg_out = (val) ? reg_out | mask : reg_out &
~mask;
drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.c: mac_filters->ucast_drop_all =
drop_all_ucast ?
mac_filters->ucast_drop_all | mask :
mac_filters->ucast_drop_all & ~mask;
drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.c: mac_filters->mcast_drop_all =
drop_all_mcast ?
mac_filters->mcast_drop_all | mask :
mac_filters->mcast_drop_all & ~mask;
drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.c: mac_filters->ucast_accept_all =
accp_all_ucast ?
mac_filters->ucast_accept_all | mask :
mac_filters->ucast_accept_all & ~mask;
drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.c: mac_filters->mcast_accept_all =
accp_all_mcast ?
mac_filters->mcast_accept_all | mask :
mac_filters->mcast_accept_all & ~mask;
drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.c: mac_filters->bcast_accept_all =
accp_all_bcast ?
mac_filters->bcast_accept_all | mask :
mac_filters->bcast_accept_all & ~mask;
drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.c: mac_filters->unmatched_unicast
= unmatched_unicast ?
mac_filters->unmatched_unicast | mask :
mac_filters->unmatched_unicast & ~mask;
drivers/net/ethernet/broadcom/bnx2x/bnx2x_init.h:
reg_bit_map = new_cos ?
(reg_bit_map | q_bit_map) :
(reg_bit_map & (~q_bit_map));
drivers/ide/pdc202xx_old.c: word_count = (rq_data_dir(rq) == READ) ?
word_count | 0x05000000 :
word_count | 0x06000000;
drivers/spi/spi-fsl-spi.c: slvsel = on ? (slvsel | (1 << cs)) :
(slvsel & ~(1 << cs));
drivers/rtc/rtc-pm8xxx.c: ctrl_reg = alarm->enabled ? (ctrl_reg |
PM8xxx_RTC_ALARM_ENABLE) :
(ctrl_reg & ~PM8xxx_RTC_ALARM_ENABLE);
drivers/rtc/rtc-pm8xxx.c: ctrl_reg = (enable) ? (ctrl_reg |
PM8xxx_RTC_ALARM_ENABLE) :
(ctrl_reg & ~PM8xxx_RTC_ALARM_ENABLE);
drivers/platform/x86/acer-wmi.c: set_params.devices = (value) ? (devices
| device) : (devices & ~device);
drivers/media/i2c/ov9650.c: reg = awb ? reg | REG_COM8 : reg &
~REG_COM8;
drivers/media/i2c/ov9650.c: com14 = value ? com14 | COM14_EDGE_EN : com14 &
~COM14_EDGE_EN;
drivers/media/i2c/ov9650.c: reg = value ? reg | COM23_TEST_MODE : reg &
~COM23_TEST_MODE;
drivers/media/i2c/s5k6aa.c: reg = awb ? reg | AALG_WB_EN_MASK : reg
& ~AALG_WB_EN_MASK;
drivers/media/usb/dvb-usb-v2/lmedm04.c: cold = (cold > 0) ? (cold & 1) : 0;
drivers/media/usb/gspca/m5602/m5602_s5k83a.c: data[0] = (vflip) ? data[0] |
0x40 : data[0];
drivers/media/usb/gspca/m5602/m5602_s5k83a.c: data[0] = (hflip) ? data[0] |
0x80 : data[0];
drivers/media/dvb-frontends/dib3000mb.c: pid = (onoff ? pid |
DIB3000_ACTIVATE_PID_FILTERING : 0);
drivers/video/s1d13xxxfb.c: regno=((regno & 1) ? (regno & ~1L) : (regno +
1));
drivers/video/s1d13xxxfb.c: regno=((regno & 1) ? (regno & ~1L) : (regno +
1));
drivers/video/i810/i810_main.c: val = (mode == OFF) ? val | SCR_OFF :
val & ~SCR_OFF;
drivers/video/i810/i810_main.c: reg = (mode == OFF) ? reg & ~0x80 :
reg | 0x80;
drivers/video/i810/i810_main.c: temp = (mode == ON) ? temp | CURSOR_ENABLE_MASK
:
temp & ~CURSOR_ENABLE_MASK;
drivers/atm/eni.c: tonga = (address >> j) & 1 ? tonga |
SEPROM_DATA :
tonga & ~SEPROM_DATA;
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
