[rtc-linux] Re: [PATCH 05/11] rtc: ds1511: clean up ds1511_nvram_read()/ds1511_nvram_write()
From: Alexandre Belloni <hidden>
Date: 2015-08-05 08:24:03
Hi, On 27/07/2015 at 00:48:30 +0300, Vladimir Zapolskiy wrote :
quoted hunk ↗ jump to hunk
The change removes redundant sysfs binary file boundary checks, since this task is already done on caller side in fs/sysfs/file.c Signed-off-by: Vladimir Zapolskiy <vz@mleia.com> --- drivers/rtc/rtc-ds1511.c | 38 ++++++++------------------------------ 1 file changed, 8 insertions(+), 30 deletions(-)diff --git a/drivers/rtc/rtc-ds1511.c b/drivers/rtc/rtc-ds1511.c index 7415c2b..2213fab 100644 --- a/drivers/rtc/rtc-ds1511.c +++ b/drivers/rtc/rtc-ds1511.c@@ -407,25 +407,14 @@ ds1511_nvram_read(struct file *filp, struct kobject *kobj, { ssize_t count; - /* - * if count is more than one, turn on "burst" mode - * turn it off when you're done - */ - if (size > 1) - rtc_write((rtc_read(RTC_CMD) | DS1511_BME), RTC_CMD); - - if (pos > DS1511_RAM_MAX) - pos = DS1511_RAM_MAX; - - if (size + pos > DS1511_RAM_MAX + 1) - size = DS1511_RAM_MAX - pos + 1; + /* turn on "burst" mode, turn it off when you're done */ + rtc_write((rtc_read(RTC_CMD) | DS1511_BME), RTC_CMD);
That one feels wrong, you are now unconditionally using burst mode, this was not the case before. If this has been tested and works, I'd ay that this at least require a mention in the commit message.
quoted hunk ↗ jump to hunk
rtc_write(pos, DS1511_RAMADDR_LSB); - for (count = 0; size > 0; count++, size--) + for (count = 0; count < size; count++) *buf++ = rtc_read(DS1511_RAMDATA); - if (count > 1) - rtc_write((rtc_read(RTC_CMD) & ~DS1511_BME), RTC_CMD); + rtc_write((rtc_read(RTC_CMD) & ~DS1511_BME), RTC_CMD); return count; }@@ -437,25 +426,14 @@ ds1511_nvram_write(struct file *filp, struct kobject *kobj, { ssize_t count; - /* - * if count is more than one, turn on "burst" mode - * turn it off when you're done - */ - if (size > 1) - rtc_write((rtc_read(RTC_CMD) | DS1511_BME), RTC_CMD); - - if (pos > DS1511_RAM_MAX) - pos = DS1511_RAM_MAX; - - if (size + pos > DS1511_RAM_MAX + 1) - size = DS1511_RAM_MAX - pos + 1; + /* turn on "burst" mode, turn it off when you're done */ + rtc_write((rtc_read(RTC_CMD) | DS1511_BME), RTC_CMD); rtc_write(pos, DS1511_RAMADDR_LSB); - for (count = 0; size > 0; count++, size--) + for (count = 0; count < size; count++) rtc_write(*buf++, DS1511_RAMDATA); - if (count > 1) - rtc_write((rtc_read(RTC_CMD) & ~DS1511_BME), RTC_CMD); + rtc_write((rtc_read(RTC_CMD) & ~DS1511_BME), RTC_CMD); return count; }-- 2.1.4
-- Alexandre Belloni, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -- -- You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. --- You received this message because you are subscribed to the Google Groups "rtc-linux" group. To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout.