On 05/05/2020 23:30:18+0200, Rasmus Villemoes wrote: > On 05/05/2020 22.13, Alexandre Belloni wrote: > > Add support for the RTC_VL_BACKUP_SWITCH flag to report battery switch over > > events. > > > > Signed-off-by: Alexandre Belloni <[email protected]> > > --- > > drivers/rtc/rtc-pcf2127.c | 16 ++++++++++++---- > > 1 file changed, 12 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c > > index 039078029bd4..967de68e1b03 100644 > > --- a/drivers/rtc/rtc-pcf2127.c > > +++ b/drivers/rtc/rtc-pcf2127.c > > @@ -188,18 +188,27 @@ static int pcf2127_rtc_ioctl(struct device *dev, > > unsigned int cmd, unsigned long arg) > > { > > struct pcf2127 *pcf2127 = dev_get_drvdata(dev); > > - int touser; > > + int val, touser = 0; > > int ret; > > > > switch (cmd) { > > case RTC_VL_READ: > > - ret = regmap_read(pcf2127->regmap, PCF2127_REG_CTRL3, &touser); > > + ret = regmap_read(pcf2127->regmap, PCF2127_REG_CTRL3, &val); > > if (ret) > > return ret; > > > > - touser = touser & PCF2127_BIT_CTRL3_BLF ? RTC_VL_BACKUP_LOW : 0; > > + if (val & PCF2127_BIT_CTRL3_BLF) > > + touser = RTC_VL_BACKUP_LOW; > > + > > + if (val & PCF2127_BIT_CTRL3_BF) > > + touser |= RTC_VL_BACKUP_SWITCH; > > I think it's a bit easier to read if you use |= in both cases. > > Re patch 3, one saves a little .text by eliding the ioctl function when, > as you say, it cannot be called anyway. No strong opinion either way, I > don't think anybody actually builds without CONFIG_RTC_INTF_DEV, but > those that do are probably the ones that care about having a tiny vmlinux. >
Honestly, I don't think it is worth doing that. On armv7, this only removes 248 bytes. Also, compiling without CONFIG_RTC_INTF_DEV simply makes the RTC unusable. There are no tools actually using the sysfs interface instead of the char device interface. I prefer keeping CONFIG_RTC_INTF_DEV private to the core. -- Alexandre Belloni, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
