All of lore.kernel.org
 help / color / mirror / Atom feed
* [rtc-linux] [PATCH 00/11] rtc: clean up bin attribute read/write functions
@ 2015-07-26 21:48 Vladimir Zapolskiy
  2015-07-26 21:48 ` [rtc-linux] [PATCH 01/11] rtc: cmos: clean up cmos_nvram_read()/cmos_nvram_write() Vladimir Zapolskiy
                   ` (11 more replies)
  0 siblings, 12 replies; 19+ messages in thread
From: Vladimir Zapolskiy @ 2015-07-26 21:48 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni; +Cc: rtc-linux, Greg Kroah-Hartman

This change removes a number of redundant checks on bin attribute
client's side, the same checks are done by sysfs_kf_bin_read() or
sysfs_kf_bin_write() caller from fs/sysfs/file.c.

Note, drivers/rtc/rtc-ds1685.c may be updated in a similar way,
however due to #ifndef magic involved this task is not done.

No functional change intended.

Vladimir Zapolskiy (11):
  rtc: cmos: clean up cmos_nvram_read()/cmos_nvram_write()
  rtc: ds1305: clean up ds1305_nvram_read()/ds1305_nvram_write()
  rtc: ds1307: clean up ds1307_nvram_read()/ds1307_nvram_write()
  rtc: ds1343: clean up ds1343_nvram_read()/ds1343_nvram_write()
  rtc: ds1511: clean up ds1511_nvram_read()/ds1511_nvram_write()
  rtc: ds1553: clean up ds1553_nvram_read()/ds1553_nvram_write()
  rtc: ds1742: clean up ds1742_nvram_read()/ds1742_nvram_write()
  rtc: m48t59: clean up m48t59_nvram_read()/m48t59_nvram_write()
  rtc: rp5c01: clean up rp5c01_nvram_read()/rp5c01_nvram_write()
  rtc: stk17ta8: clean up stk17ta8_nvram_read()/stk17ta8_nvram_write()
  rtc: tx4939: clean up tx4939_rtc_nvram_read()/tx4939_rtc_nvram_write()

 drivers/rtc/rtc-cmos.c     | 13 -------------
 drivers/rtc/rtc-ds1305.c   | 18 ------------------
 drivers/rtc/rtc-ds1307.c   | 14 --------------
 drivers/rtc/rtc-ds1343.c   | 12 ------------
 drivers/rtc/rtc-ds1511.c   | 38 ++++++++------------------------------
 drivers/rtc/rtc-ds1553.c   |  4 ++--
 drivers/rtc/rtc-ds1742.c   |  4 ++--
 drivers/rtc/rtc-m48t59.c   | 16 ++++++++--------
 drivers/rtc/rtc-rp5c01.c   |  4 ++--
 drivers/rtc/rtc-stk17ta8.c |  4 ++--
 drivers/rtc/rtc-tx4939.c   |  6 ++----
 11 files changed, 26 insertions(+), 107 deletions(-)

-- 
2.1.4

-- 
-- 
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.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [rtc-linux] [PATCH 01/11] rtc: cmos: clean up cmos_nvram_read()/cmos_nvram_write()
  2015-07-26 21:48 [rtc-linux] [PATCH 00/11] rtc: clean up bin attribute read/write functions Vladimir Zapolskiy
@ 2015-07-26 21:48 ` Vladimir Zapolskiy
  2015-07-26 21:48 ` [rtc-linux] [PATCH 02/11] rtc: ds1305: clean up ds1305_nvram_read()/ds1305_nvram_write() Vladimir Zapolskiy
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 19+ messages in thread
From: Vladimir Zapolskiy @ 2015-07-26 21:48 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni; +Cc: rtc-linux, Greg Kroah-Hartman

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-cmos.c | 13 -------------
 1 file changed, 13 deletions(-)

diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c
index a82556a..1ecd374 100644
--- a/drivers/rtc/rtc-cmos.c
+++ b/drivers/rtc/rtc-cmos.c
@@ -512,13 +512,6 @@ cmos_nvram_read(struct file *filp, struct kobject *kobj,
 {
 	int	retval;
 
-	if (unlikely(off >= attr->size))
-		return 0;
-	if (unlikely(off < 0))
-		return -EINVAL;
-	if ((off + count) > attr->size)
-		count = attr->size - off;
-
 	off += NVRAM_OFFSET;
 	spin_lock_irq(&rtc_lock);
 	for (retval = 0; count; count--, off++, retval++) {
@@ -543,12 +536,6 @@ cmos_nvram_write(struct file *filp, struct kobject *kobj,
 	int		retval;
 
 	cmos = dev_get_drvdata(container_of(kobj, struct device, kobj));
-	if (unlikely(off >= attr->size))
-		return -EFBIG;
-	if (unlikely(off < 0))
-		return -EINVAL;
-	if ((off + count) > attr->size)
-		count = attr->size - off;
 
 	/* NOTE:  on at least PCs and Ataris, the boot firmware uses a
 	 * checksum on part of the NVRAM data.  That's currently ignored
-- 
2.1.4

-- 
-- 
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.

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [rtc-linux] [PATCH 02/11] rtc: ds1305: clean up ds1305_nvram_read()/ds1305_nvram_write()
  2015-07-26 21:48 [rtc-linux] [PATCH 00/11] rtc: clean up bin attribute read/write functions Vladimir Zapolskiy
  2015-07-26 21:48 ` [rtc-linux] [PATCH 01/11] rtc: cmos: clean up cmos_nvram_read()/cmos_nvram_write() Vladimir Zapolskiy
@ 2015-07-26 21:48 ` Vladimir Zapolskiy
  2015-07-26 21:48 ` [rtc-linux] [PATCH 03/11] rtc: ds1307: clean up ds1307_nvram_read()/ds1307_nvram_write() Vladimir Zapolskiy
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 19+ messages in thread
From: Vladimir Zapolskiy @ 2015-07-26 21:48 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni; +Cc: rtc-linux, Greg Kroah-Hartman

The change removes redundant sysfs binary file boundary checks, since
this task is already done on caller size in fs/sysfs/file.c

Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
---
 drivers/rtc/rtc-ds1305.c | 18 ------------------
 1 file changed, 18 deletions(-)

diff --git a/drivers/rtc/rtc-ds1305.c b/drivers/rtc/rtc-ds1305.c
index 12b0715..baa5d04 100644
--- a/drivers/rtc/rtc-ds1305.c
+++ b/drivers/rtc/rtc-ds1305.c
@@ -538,15 +538,6 @@ ds1305_nvram_read(struct file *filp, struct kobject *kobj,
 
 	spi = container_of(kobj, struct spi_device, dev.kobj);
 
-	if (unlikely(off >= DS1305_NVRAM_LEN))
-		return 0;
-	if (count >= DS1305_NVRAM_LEN)
-		count = DS1305_NVRAM_LEN;
-	if ((off + count) > DS1305_NVRAM_LEN)
-		count = DS1305_NVRAM_LEN - off;
-	if (unlikely(!count))
-		return count;
-
 	addr = DS1305_NVRAM + off;
 	msg_init(&m, x, &addr, count, NULL, buf);
 
@@ -569,15 +560,6 @@ ds1305_nvram_write(struct file *filp, struct kobject *kobj,
 
 	spi = container_of(kobj, struct spi_device, dev.kobj);
 
-	if (unlikely(off >= DS1305_NVRAM_LEN))
-		return -EFBIG;
-	if (count >= DS1305_NVRAM_LEN)
-		count = DS1305_NVRAM_LEN;
-	if ((off + count) > DS1305_NVRAM_LEN)
-		count = DS1305_NVRAM_LEN - off;
-	if (unlikely(!count))
-		return count;
-
 	addr = (DS1305_WRITE | DS1305_NVRAM) + off;
 	msg_init(&m, x, &addr, count, buf, NULL);
 
-- 
2.1.4

-- 
-- 
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.

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [rtc-linux] [PATCH 03/11] rtc: ds1307: clean up ds1307_nvram_read()/ds1307_nvram_write()
  2015-07-26 21:48 [rtc-linux] [PATCH 00/11] rtc: clean up bin attribute read/write functions Vladimir Zapolskiy
  2015-07-26 21:48 ` [rtc-linux] [PATCH 01/11] rtc: cmos: clean up cmos_nvram_read()/cmos_nvram_write() Vladimir Zapolskiy
  2015-07-26 21:48 ` [rtc-linux] [PATCH 02/11] rtc: ds1305: clean up ds1305_nvram_read()/ds1305_nvram_write() Vladimir Zapolskiy
@ 2015-07-26 21:48 ` Vladimir Zapolskiy
  2015-07-26 21:48 ` [rtc-linux] [PATCH 04/11] rtc: ds1343: clean up ds1343_nvram_read()/ds1343_nvram_write() Vladimir Zapolskiy
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 19+ messages in thread
From: Vladimir Zapolskiy @ 2015-07-26 21:48 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni; +Cc: rtc-linux, Greg Kroah-Hartman

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-ds1307.c | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/drivers/rtc/rtc-ds1307.c b/drivers/rtc/rtc-ds1307.c
index 6e76de1..f1ba3cc 100644
--- a/drivers/rtc/rtc-ds1307.c
+++ b/drivers/rtc/rtc-ds1307.c
@@ -798,13 +798,6 @@ ds1307_nvram_read(struct file *filp, struct kobject *kobj,
 	client = kobj_to_i2c_client(kobj);
 	ds1307 = i2c_get_clientdata(client);
 
-	if (unlikely(off >= ds1307->nvram->size))
-		return 0;
-	if ((off + count) > ds1307->nvram->size)
-		count = ds1307->nvram->size - off;
-	if (unlikely(!count))
-		return count;
-
 	result = ds1307->read_block_data(client, ds1307->nvram_offset + off,
 								count, buf);
 	if (result < 0)
@@ -824,13 +817,6 @@ ds1307_nvram_write(struct file *filp, struct kobject *kobj,
 	client = kobj_to_i2c_client(kobj);
 	ds1307 = i2c_get_clientdata(client);
 
-	if (unlikely(off >= ds1307->nvram->size))
-		return -EFBIG;
-	if ((off + count) > ds1307->nvram->size)
-		count = ds1307->nvram->size - off;
-	if (unlikely(!count))
-		return count;
-
 	result = ds1307->write_block_data(client, ds1307->nvram_offset + off,
 								count, buf);
 	if (result < 0) {
-- 
2.1.4

-- 
-- 
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.

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [rtc-linux] [PATCH 04/11] rtc: ds1343: clean up ds1343_nvram_read()/ds1343_nvram_write()
  2015-07-26 21:48 [rtc-linux] [PATCH 00/11] rtc: clean up bin attribute read/write functions Vladimir Zapolskiy
                   ` (2 preceding siblings ...)
  2015-07-26 21:48 ` [rtc-linux] [PATCH 03/11] rtc: ds1307: clean up ds1307_nvram_read()/ds1307_nvram_write() Vladimir Zapolskiy
@ 2015-07-26 21:48 ` Vladimir Zapolskiy
  2015-07-26 21:48 ` [rtc-linux] [PATCH 05/11] rtc: ds1511: clean up ds1511_nvram_read()/ds1511_nvram_write() Vladimir Zapolskiy
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 19+ messages in thread
From: Vladimir Zapolskiy @ 2015-07-26 21:48 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni; +Cc: rtc-linux, Greg Kroah-Hartman

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-ds1343.c | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/drivers/rtc/rtc-ds1343.c b/drivers/rtc/rtc-ds1343.c
index ae9f997..79a06dd 100644
--- a/drivers/rtc/rtc-ds1343.c
+++ b/drivers/rtc/rtc-ds1343.c
@@ -162,12 +162,6 @@ static ssize_t ds1343_nvram_write(struct file *filp, struct kobject *kobj,
 	struct device *dev = kobj_to_dev(kobj);
 	struct ds1343_priv *priv = dev_get_drvdata(dev);
 
-	if (unlikely(!count))
-		return count;
-
-	if ((count + off) > DS1343_NVRAM_LEN)
-		count = DS1343_NVRAM_LEN - off;
-
 	address = DS1343_NVRAM + off;
 
 	ret = regmap_bulk_write(priv->map, address, buf, count);
@@ -187,12 +181,6 @@ static ssize_t ds1343_nvram_read(struct file *filp, struct kobject *kobj,
 	struct device *dev = kobj_to_dev(kobj);
 	struct ds1343_priv *priv = dev_get_drvdata(dev);
 
-	if (unlikely(!count))
-		return count;
-
-	if ((count + off) > DS1343_NVRAM_LEN)
-		count = DS1343_NVRAM_LEN - off;
-
 	address = DS1343_NVRAM + off;
 
 	ret = regmap_bulk_read(priv->map, address, buf, count);
-- 
2.1.4

-- 
-- 
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.

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [rtc-linux] [PATCH 05/11] rtc: ds1511: clean up ds1511_nvram_read()/ds1511_nvram_write()
  2015-07-26 21:48 [rtc-linux] [PATCH 00/11] rtc: clean up bin attribute read/write functions Vladimir Zapolskiy
                   ` (3 preceding siblings ...)
  2015-07-26 21:48 ` [rtc-linux] [PATCH 04/11] rtc: ds1343: clean up ds1343_nvram_read()/ds1343_nvram_write() Vladimir Zapolskiy
@ 2015-07-26 21:48 ` Vladimir Zapolskiy
  2015-08-05  8:24   ` [rtc-linux] " Alexandre Belloni
  2015-08-05 18:12   ` [rtc-linux] [PATCH v2 " Vladimir Zapolskiy
  2015-07-26 21:48 ` [rtc-linux] [PATCH 06/11] rtc: ds1553: clean up ds1553_nvram_read()/ds1553_nvram_write() Vladimir Zapolskiy
                   ` (6 subsequent siblings)
  11 siblings, 2 replies; 19+ messages in thread
From: Vladimir Zapolskiy @ 2015-07-26 21:48 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni; +Cc: rtc-linux, Greg Kroah-Hartman

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);
 
 	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

-- 
-- 
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.

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [rtc-linux] [PATCH 06/11] rtc: ds1553: clean up ds1553_nvram_read()/ds1553_nvram_write()
  2015-07-26 21:48 [rtc-linux] [PATCH 00/11] rtc: clean up bin attribute read/write functions Vladimir Zapolskiy
                   ` (4 preceding siblings ...)
  2015-07-26 21:48 ` [rtc-linux] [PATCH 05/11] rtc: ds1511: clean up ds1511_nvram_read()/ds1511_nvram_write() Vladimir Zapolskiy
@ 2015-07-26 21:48 ` Vladimir Zapolskiy
  2015-07-26 21:48 ` [rtc-linux] [PATCH 07/11] rtc: ds1742: clean up ds1742_nvram_read()/ds1742_nvram_write() Vladimir Zapolskiy
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 19+ messages in thread
From: Vladimir Zapolskiy @ 2015-07-26 21:48 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni; +Cc: rtc-linux, Greg Kroah-Hartman

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-ds1553.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/rtc/rtc-ds1553.c b/drivers/rtc/rtc-ds1553.c
index a24e091..38422ab 100644
--- a/drivers/rtc/rtc-ds1553.c
+++ b/drivers/rtc/rtc-ds1553.c
@@ -245,7 +245,7 @@ static ssize_t ds1553_nvram_read(struct file *filp, struct kobject *kobj,
 	void __iomem *ioaddr = pdata->ioaddr;
 	ssize_t count;
 
-	for (count = 0; size > 0 && pos < RTC_OFFSET; count++, size--)
+	for (count = 0; count < size; count++)
 		*buf++ = readb(ioaddr + pos++);
 	return count;
 }
@@ -260,7 +260,7 @@ static ssize_t ds1553_nvram_write(struct file *filp, struct kobject *kobj,
 	void __iomem *ioaddr = pdata->ioaddr;
 	ssize_t count;
 
-	for (count = 0; size > 0 && pos < RTC_OFFSET; count++, size--)
+	for (count = 0; count < size; count++)
 		writeb(*buf++, ioaddr + pos++);
 	return count;
 }
-- 
2.1.4

-- 
-- 
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.

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [rtc-linux] [PATCH 07/11] rtc: ds1742: clean up ds1742_nvram_read()/ds1742_nvram_write()
  2015-07-26 21:48 [rtc-linux] [PATCH 00/11] rtc: clean up bin attribute read/write functions Vladimir Zapolskiy
                   ` (5 preceding siblings ...)
  2015-07-26 21:48 ` [rtc-linux] [PATCH 06/11] rtc: ds1553: clean up ds1553_nvram_read()/ds1553_nvram_write() Vladimir Zapolskiy
@ 2015-07-26 21:48 ` Vladimir Zapolskiy
  2015-07-26 21:48 ` [rtc-linux] [PATCH 08/11] rtc: m48t59: clean up m48t59_nvram_read()/m48t59_nvram_write() Vladimir Zapolskiy
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 19+ messages in thread
From: Vladimir Zapolskiy @ 2015-07-26 21:48 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni; +Cc: rtc-linux, Greg Kroah-Hartman

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-ds1742.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/rtc/rtc-ds1742.c b/drivers/rtc/rtc-ds1742.c
index 0f8d8ac..c5168b3 100644
--- a/drivers/rtc/rtc-ds1742.c
+++ b/drivers/rtc/rtc-ds1742.c
@@ -134,7 +134,7 @@ static ssize_t ds1742_nvram_read(struct file *filp, struct kobject *kobj,
 	void __iomem *ioaddr = pdata->ioaddr_nvram;
 	ssize_t count;
 
-	for (count = 0; size > 0 && pos < pdata->size_nvram; count++, size--)
+	for (count = 0; count < size; count++)
 		*buf++ = readb(ioaddr + pos++);
 	return count;
 }
@@ -149,7 +149,7 @@ static ssize_t ds1742_nvram_write(struct file *filp, struct kobject *kobj,
 	void __iomem *ioaddr = pdata->ioaddr_nvram;
 	ssize_t count;
 
-	for (count = 0; size > 0 && pos < pdata->size_nvram; count++, size--)
+	for (count = 0; count < size; count++)
 		writeb(*buf++, ioaddr + pos++);
 	return count;
 }
-- 
2.1.4

-- 
-- 
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.

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [rtc-linux] [PATCH 08/11] rtc: m48t59: clean up m48t59_nvram_read()/m48t59_nvram_write()
  2015-07-26 21:48 [rtc-linux] [PATCH 00/11] rtc: clean up bin attribute read/write functions Vladimir Zapolskiy
                   ` (6 preceding siblings ...)
  2015-07-26 21:48 ` [rtc-linux] [PATCH 07/11] rtc: ds1742: clean up ds1742_nvram_read()/ds1742_nvram_write() Vladimir Zapolskiy
@ 2015-07-26 21:48 ` Vladimir Zapolskiy
  2015-08-05  8:27   ` [rtc-linux] " Alexandre Belloni
  2015-08-05 18:13   ` [rtc-linux] [PATCH v2 " Vladimir Zapolskiy
  2015-07-26 21:48 ` [rtc-linux] [PATCH 09/11] rtc: rp5c01: clean up rp5c01_nvram_read()/rp5c01_nvram_write() Vladimir Zapolskiy
                   ` (3 subsequent siblings)
  11 siblings, 2 replies; 19+ messages in thread
From: Vladimir Zapolskiy @ 2015-07-26 21:48 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni; +Cc: rtc-linux, Greg Kroah-Hartman

The change removes redundant sysfs binary file boundary checks, since
this task is already done on caller side in fs/sysfs/file.c

Spinlock acquisition/release is moved out of the loop body to get
atomic states of NVRAM reading and writing operations.

Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
---
 drivers/rtc/rtc-m48t59.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/rtc/rtc-m48t59.c b/drivers/rtc/rtc-m48t59.c
index 90abb5b..512c44b 100644
--- a/drivers/rtc/rtc-m48t59.c
+++ b/drivers/rtc/rtc-m48t59.c
@@ -345,11 +345,12 @@ static ssize_t m48t59_nvram_read(struct file *filp, struct kobject *kobj,
 	ssize_t cnt = 0;
 	unsigned long flags;
 
-	for (; size > 0 && pos < pdata->offset; cnt++, size--) {
-		spin_lock_irqsave(&m48t59->lock, flags);
+	spin_lock_irqsave(&m48t59->lock, flags);
+
+	for (; cnt < size; cnt++)
 		*buf++ = M48T59_READ(cnt);
-		spin_unlock_irqrestore(&m48t59->lock, flags);
-	}
+
+	spin_unlock_irqrestore(&m48t59->lock, flags);
 
 	return cnt;
 }
@@ -365,11 +366,10 @@ static ssize_t m48t59_nvram_write(struct file *filp, struct kobject *kobj,
 	ssize_t cnt = 0;
 	unsigned long flags;
 
-	for (; size > 0 && pos < pdata->offset; cnt++, size--) {
-		spin_lock_irqsave(&m48t59->lock, flags);
+	spin_lock_irqsave(&m48t59->lock, flags);
+
+	for (; cnt < size; cnt++)
 		M48T59_WRITE(*buf++, cnt);
-		spin_unlock_irqrestore(&m48t59->lock, flags);
-	}
 
 	return cnt;
 }
-- 
2.1.4

-- 
-- 
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.

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [rtc-linux] [PATCH 09/11] rtc: rp5c01: clean up rp5c01_nvram_read()/rp5c01_nvram_write()
  2015-07-26 21:48 [rtc-linux] [PATCH 00/11] rtc: clean up bin attribute read/write functions Vladimir Zapolskiy
                   ` (7 preceding siblings ...)
  2015-07-26 21:48 ` [rtc-linux] [PATCH 08/11] rtc: m48t59: clean up m48t59_nvram_read()/m48t59_nvram_write() Vladimir Zapolskiy
@ 2015-07-26 21:48 ` Vladimir Zapolskiy
  2015-07-26 21:48 ` [rtc-linux] [PATCH 10/11] rtc: stk17ta8: clean up stk17ta8_nvram_read()/stk17ta8_nvram_write() Vladimir Zapolskiy
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 19+ messages in thread
From: Vladimir Zapolskiy @ 2015-07-26 21:48 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni; +Cc: rtc-linux, Greg Kroah-Hartman

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-rp5c01.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/rtc/rtc-rp5c01.c b/drivers/rtc/rtc-rp5c01.c
index b548551..0260353 100644
--- a/drivers/rtc/rtc-rp5c01.c
+++ b/drivers/rtc/rtc-rp5c01.c
@@ -170,7 +170,7 @@ static ssize_t rp5c01_nvram_read(struct file *filp, struct kobject *kobj,
 
 	spin_lock_irq(&priv->lock);
 
-	for (count = 0; size > 0 && pos < RP5C01_MODE; count++, size--) {
+	for (count = 0; count < size; count++) {
 		u8 data;
 
 		rp5c01_write(priv,
@@ -200,7 +200,7 @@ static ssize_t rp5c01_nvram_write(struct file *filp, struct kobject *kobj,
 
 	spin_lock_irq(&priv->lock);
 
-	for (count = 0; size > 0 && pos < RP5C01_MODE; count++, size--) {
+	for (count = 0; count < size; count++) {
 		u8 data = *buf++;
 
 		rp5c01_write(priv,
-- 
2.1.4

-- 
-- 
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.

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [rtc-linux] [PATCH 10/11] rtc: stk17ta8: clean up stk17ta8_nvram_read()/stk17ta8_nvram_write()
  2015-07-26 21:48 [rtc-linux] [PATCH 00/11] rtc: clean up bin attribute read/write functions Vladimir Zapolskiy
                   ` (8 preceding siblings ...)
  2015-07-26 21:48 ` [rtc-linux] [PATCH 09/11] rtc: rp5c01: clean up rp5c01_nvram_read()/rp5c01_nvram_write() Vladimir Zapolskiy
@ 2015-07-26 21:48 ` Vladimir Zapolskiy
  2015-07-26 21:48 ` [rtc-linux] [PATCH 11/11] rtc: tx4939: clean up tx4939_rtc_nvram_read()/tx4939_rtc_nvram_write() Vladimir Zapolskiy
  2015-08-14 22:21 ` [rtc-linux] [PATCH 00/11] rtc: clean up bin attribute read/write functions Alexandre Belloni
  11 siblings, 0 replies; 19+ messages in thread
From: Vladimir Zapolskiy @ 2015-07-26 21:48 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni; +Cc: rtc-linux, Greg Kroah-Hartman

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-stk17ta8.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/rtc/rtc-stk17ta8.c b/drivers/rtc/rtc-stk17ta8.c
index 0e93b71..ba6a83b 100644
--- a/drivers/rtc/rtc-stk17ta8.c
+++ b/drivers/rtc/rtc-stk17ta8.c
@@ -254,7 +254,7 @@ static ssize_t stk17ta8_nvram_read(struct file *filp, struct kobject *kobj,
 	void __iomem *ioaddr = pdata->ioaddr;
 	ssize_t count;
 
-	for (count = 0; size > 0 && pos < RTC_OFFSET; count++, size--)
+	for (count = 0; count < size; count++)
 		*buf++ = readb(ioaddr + pos++);
 	return count;
 }
@@ -269,7 +269,7 @@ static ssize_t stk17ta8_nvram_write(struct file *filp, struct kobject *kobj,
 	void __iomem *ioaddr = pdata->ioaddr;
 	ssize_t count;
 
-	for (count = 0; size > 0 && pos < RTC_OFFSET; count++, size--)
+	for (count = 0; count < size; count++)
 		writeb(*buf++, ioaddr + pos++);
 	return count;
 }
-- 
2.1.4

-- 
-- 
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.

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [rtc-linux] [PATCH 11/11] rtc: tx4939: clean up tx4939_rtc_nvram_read()/tx4939_rtc_nvram_write()
  2015-07-26 21:48 [rtc-linux] [PATCH 00/11] rtc: clean up bin attribute read/write functions Vladimir Zapolskiy
                   ` (9 preceding siblings ...)
  2015-07-26 21:48 ` [rtc-linux] [PATCH 10/11] rtc: stk17ta8: clean up stk17ta8_nvram_read()/stk17ta8_nvram_write() Vladimir Zapolskiy
@ 2015-07-26 21:48 ` Vladimir Zapolskiy
  2015-08-14 22:21 ` [rtc-linux] [PATCH 00/11] rtc: clean up bin attribute read/write functions Alexandre Belloni
  11 siblings, 0 replies; 19+ messages in thread
From: Vladimir Zapolskiy @ 2015-07-26 21:48 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni; +Cc: rtc-linux, Greg Kroah-Hartman

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-tx4939.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/rtc/rtc-tx4939.c b/drivers/rtc/rtc-tx4939.c
index cb7f94e..560d9a5 100644
--- a/drivers/rtc/rtc-tx4939.c
+++ b/drivers/rtc/rtc-tx4939.c
@@ -199,8 +199,7 @@ static ssize_t tx4939_rtc_nvram_read(struct file *filp, struct kobject *kobj,
 	ssize_t count;
 
 	spin_lock_irq(&pdata->lock);
-	for (count = 0; size > 0 && pos < TX4939_RTC_REG_RAMSIZE;
-	     count++, size--) {
+	for (count = 0; count < size; count++) {
 		__raw_writel(pos++, &rtcreg->adr);
 		*buf++ = __raw_readl(&rtcreg->dat);
 	}
@@ -218,8 +217,7 @@ static ssize_t tx4939_rtc_nvram_write(struct file *filp, struct kobject *kobj,
 	ssize_t count;
 
 	spin_lock_irq(&pdata->lock);
-	for (count = 0; size > 0 && pos < TX4939_RTC_REG_RAMSIZE;
-	     count++, size--) {
+	for (count = 0; count < size; count++) {
 		__raw_writel(pos++, &rtcreg->adr);
 		__raw_writel(*buf++, &rtcreg->dat);
 	}
-- 
2.1.4

-- 
-- 
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.

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [rtc-linux] Re: [PATCH 05/11] rtc: ds1511: clean up ds1511_nvram_read()/ds1511_nvram_write()
  2015-07-26 21:48 ` [rtc-linux] [PATCH 05/11] rtc: ds1511: clean up ds1511_nvram_read()/ds1511_nvram_write() Vladimir Zapolskiy
@ 2015-08-05  8:24   ` Alexandre Belloni
  2015-08-05 16:22     ` Vladimir Zapolskiy
  2015-08-05 18:12   ` [rtc-linux] [PATCH v2 " Vladimir Zapolskiy
  1 sibling, 1 reply; 19+ messages in thread
From: Alexandre Belloni @ 2015-08-05  8:24 UTC (permalink / raw)
  To: Vladimir Zapolskiy; +Cc: Alessandro Zummo, rtc-linux, Greg Kroah-Hartman

Hi,

On 27/07/2015 at 00:48:30 +0300, Vladimir Zapolskiy wrote :
> 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.

>  
>  	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.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [rtc-linux] Re: [PATCH 08/11] rtc: m48t59: clean up m48t59_nvram_read()/m48t59_nvram_write()
  2015-07-26 21:48 ` [rtc-linux] [PATCH 08/11] rtc: m48t59: clean up m48t59_nvram_read()/m48t59_nvram_write() Vladimir Zapolskiy
@ 2015-08-05  8:27   ` Alexandre Belloni
  2015-08-05 16:04     ` Vladimir Zapolskiy
  2015-08-05 18:13   ` [rtc-linux] [PATCH v2 " Vladimir Zapolskiy
  1 sibling, 1 reply; 19+ messages in thread
From: Alexandre Belloni @ 2015-08-05  8:27 UTC (permalink / raw)
  To: Vladimir Zapolskiy; +Cc: Alessandro Zummo, rtc-linux, Greg Kroah-Hartman

On 27/07/2015 at 00:48:33 +0300, Vladimir Zapolskiy wrote :
> @@ -365,11 +366,10 @@ static ssize_t m48t59_nvram_write(struct file *filp, struct kobject *kobj,
>  	ssize_t cnt = 0;
>  	unsigned long flags;
>  
> -	for (; size > 0 && pos < pdata->offset; cnt++, size--) {
> -		spin_lock_irqsave(&m48t59->lock, flags);
> +	spin_lock_irqsave(&m48t59->lock, flags);
> +
> +	for (; cnt < size; cnt++)
>  		M48T59_WRITE(*buf++, cnt);
> -		spin_unlock_irqrestore(&m48t59->lock, flags);
> -	}
>  

Isn't a spin_unlock_irqrestore() missing?

-- 
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.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [rtc-linux] Re: [PATCH 08/11] rtc: m48t59: clean up m48t59_nvram_read()/m48t59_nvram_write()
  2015-08-05  8:27   ` [rtc-linux] " Alexandre Belloni
@ 2015-08-05 16:04     ` Vladimir Zapolskiy
  0 siblings, 0 replies; 19+ messages in thread
From: Vladimir Zapolskiy @ 2015-08-05 16:04 UTC (permalink / raw)
  To: Alexandre Belloni; +Cc: rtc-linux, Alessandro Zummo, Greg Kroah-Hartman

Hi Alexandre,

On 05.08.2015 11:27, Alexandre Belloni wrote:
> On 27/07/2015 at 00:48:33 +0300, Vladimir Zapolskiy wrote :
>> @@ -365,11 +366,10 @@ static ssize_t m48t59_nvram_write(struct file *filp, struct kobject *kobj,
>>  	ssize_t cnt = 0;
>>  	unsigned long flags;
>>  
>> -	for (; size > 0 && pos < pdata->offset; cnt++, size--) {
>> -		spin_lock_irqsave(&m48t59->lock, flags);
>> +	spin_lock_irqsave(&m48t59->lock, flags);
>> +
>> +	for (; cnt < size; cnt++)
>>  		M48T59_WRITE(*buf++, cnt);
>> -		spin_unlock_irqrestore(&m48t59->lock, flags);
>> -	}
>>  
> 
> Isn't a spin_unlock_irqrestore() missing?
> 

oops, you are correct, will update this change.

With best wishes,
Vladimir

-- 
-- 
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.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [rtc-linux] Re: [PATCH 05/11] rtc: ds1511: clean up ds1511_nvram_read()/ds1511_nvram_write()
  2015-08-05  8:24   ` [rtc-linux] " Alexandre Belloni
@ 2015-08-05 16:22     ` Vladimir Zapolskiy
  0 siblings, 0 replies; 19+ messages in thread
From: Vladimir Zapolskiy @ 2015-08-05 16:22 UTC (permalink / raw)
  To: rtc-linux, Alexandre Belloni; +Cc: Alessandro Zummo, Greg Kroah-Hartman

Hi Alexandre,

On 05.08.2015 11:24, Alexandre Belloni wrote:
> Hi,
> 
> On 27/07/2015 at 00:48:30 +0300, Vladimir Zapolskiy wrote :
>> 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.
> 

I believe this is a correct fix, burst mode should be always enabled,
probably it might be better even to move its configuration to probe().

>From the spec:

  The burst-mode enable bit allows the extended user RAM address
  registers to automatically increment for consecutive reads and writes.

  When BME is set to 1, the automatic incrementing is enabled; when BME
  is set to 0, the automatic incrementing is disabled.

Prior to the change the sequence of

  read(fd, buf, 1);
  read(fd, buf, 1);
  read(fd, buf, 1);
  ...

won't sequentially read NVRAM byte by byte as it should be IMO, at least
it contradicts to common and expected usage of read().

If there is no objections, I'll keep this part of the proposed change
unmodified, but thanks for attracting my attention to the potential
problem, I've managed to notice that DS1511_RAM_MAX value is invalid, it
must be 256 --- the current value of 0xff *and* "no-burst" mode for
1-byte reading probably is done as a clumsy attempt to avoid overflow.

I'll resend the change with DS1511_RAM_MAX update and improved commit
message.

With best wishes,
Vladimir

>>  
>>  	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
>>
> 

-- 
-- 
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.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [rtc-linux] [PATCH v2 05/11] rtc: ds1511: clean up ds1511_nvram_read()/ds1511_nvram_write()
  2015-07-26 21:48 ` [rtc-linux] [PATCH 05/11] rtc: ds1511: clean up ds1511_nvram_read()/ds1511_nvram_write() Vladimir Zapolskiy
  2015-08-05  8:24   ` [rtc-linux] " Alexandre Belloni
@ 2015-08-05 18:12   ` Vladimir Zapolskiy
  1 sibling, 0 replies; 19+ messages in thread
From: Vladimir Zapolskiy @ 2015-08-05 18:12 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni; +Cc: rtc-linux, Greg Kroah-Hartman

The change removes redundant sysfs binary file boundary checks, since
this task is already done on caller side in fs/sysfs/file.c

The change enables burst mode of access to SRAM for any read()/write()
operations, it is worth to mention that this may influence on
userspace, for instance prior to the change

  read(fd, buf, 1);
  read(fd, buf + 1, 1);

and

  read(fd, buf, 2);

sequences of syscalls over DS1511's sysfs "nvram" fd led to different
DS1511 state changes and/or buf content, if some userspace applications
are written specifically for DS1511 and exploit this strange
"feature", they may be impacted.

Also the change corrects NVRAM size accessible to userspace from 255
bytes to 256 bytes.

Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
---
Changes from v1 to v2:
* configuration of burst mode access to NVRAM from probe() allows to
  remove another 10 lines of code,
* NVRAM size is set correctly to 256 bytes,
* extended commit message with a warning addressed to any potentially
  impacted userspace applications due to always enabled burst mode

 drivers/rtc/rtc-ds1511.c | 42 +++++-------------------------------------
 1 file changed, 5 insertions(+), 37 deletions(-)

diff --git a/drivers/rtc/rtc-ds1511.c b/drivers/rtc/rtc-ds1511.c
index 7415c2b..da3d04c 100644
--- a/drivers/rtc/rtc-ds1511.c
+++ b/drivers/rtc/rtc-ds1511.c
@@ -64,7 +64,7 @@ enum ds1511reg {
 #define DS1511_KIE	0x04
 #define DS1511_WDE	0x02
 #define DS1511_WDS	0x01
-#define DS1511_RAM_MAX	0xff
+#define DS1511_RAM_MAX	0x100
 
 #define RTC_CMD		DS1511_CONTROL_B
 #define RTC_CMD1	DS1511_CONTROL_A
@@ -159,7 +159,7 @@ ds1511_wdog_set(unsigned long deciseconds)
 	/*
 	 * set wdog enable and wdog 'steering' bit to issue a reset
 	 */
-	rtc_write(DS1511_WDE | DS1511_WDS, RTC_CMD);
+	rtc_write(rtc_read(RTC_CMD) | DS1511_WDE | DS1511_WDS, RTC_CMD);
 }
 
 void
@@ -407,26 +407,10 @@ 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;
-
 	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);
-
 	return count;
 }
 
@@ -437,26 +421,10 @@ 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;
-
 	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);
-
 	return count;
 }
 
@@ -490,7 +458,7 @@ static int ds1511_rtc_probe(struct platform_device *pdev)
 	/*
 	 * turn on the clock and the crystal, etc.
 	 */
-	rtc_write(0, RTC_CMD);
+	rtc_write(DS1511_BME, RTC_CMD);
 	rtc_write(0, RTC_CMD1);
 	/*
 	 * clear the wdog counter
-- 
2.1.4

-- 
-- 
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.

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [rtc-linux] [PATCH v2 08/11] rtc: m48t59: clean up m48t59_nvram_read()/m48t59_nvram_write()
  2015-07-26 21:48 ` [rtc-linux] [PATCH 08/11] rtc: m48t59: clean up m48t59_nvram_read()/m48t59_nvram_write() Vladimir Zapolskiy
  2015-08-05  8:27   ` [rtc-linux] " Alexandre Belloni
@ 2015-08-05 18:13   ` Vladimir Zapolskiy
  1 sibling, 0 replies; 19+ messages in thread
From: Vladimir Zapolskiy @ 2015-08-05 18:13 UTC (permalink / raw)
  To: Alessandro Zummo, Alexandre Belloni; +Cc: rtc-linux, Greg Kroah-Hartman

The change removes redundant sysfs binary file boundary checks, since
this task is already done on caller side in fs/sysfs/file.c

Spinlock acquisition/release is moved out of the loop body to get
atomic states of NVRAM reading and writing operations.

Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
---
Changes from v1 to v2:
* added a lost spin_unlock_irqrestore(), thanks to Alexandre

 drivers/rtc/rtc-m48t59.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/rtc/rtc-m48t59.c b/drivers/rtc/rtc-m48t59.c
index 90abb5b..d99a705 100644
--- a/drivers/rtc/rtc-m48t59.c
+++ b/drivers/rtc/rtc-m48t59.c
@@ -345,11 +345,12 @@ static ssize_t m48t59_nvram_read(struct file *filp, struct kobject *kobj,
 	ssize_t cnt = 0;
 	unsigned long flags;
 
-	for (; size > 0 && pos < pdata->offset; cnt++, size--) {
-		spin_lock_irqsave(&m48t59->lock, flags);
+	spin_lock_irqsave(&m48t59->lock, flags);
+
+	for (; cnt < size; cnt++)
 		*buf++ = M48T59_READ(cnt);
-		spin_unlock_irqrestore(&m48t59->lock, flags);
-	}
+
+	spin_unlock_irqrestore(&m48t59->lock, flags);
 
 	return cnt;
 }
@@ -365,11 +366,12 @@ static ssize_t m48t59_nvram_write(struct file *filp, struct kobject *kobj,
 	ssize_t cnt = 0;
 	unsigned long flags;
 
-	for (; size > 0 && pos < pdata->offset; cnt++, size--) {
-		spin_lock_irqsave(&m48t59->lock, flags);
+	spin_lock_irqsave(&m48t59->lock, flags);
+
+	for (; cnt < size; cnt++)
 		M48T59_WRITE(*buf++, cnt);
-		spin_unlock_irqrestore(&m48t59->lock, flags);
-	}
+
+	spin_unlock_irqrestore(&m48t59->lock, flags);
 
 	return cnt;
 }
-- 
2.1.4

-- 
-- 
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.

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [rtc-linux] [PATCH 00/11] rtc: clean up bin attribute read/write functions
  2015-07-26 21:48 [rtc-linux] [PATCH 00/11] rtc: clean up bin attribute read/write functions Vladimir Zapolskiy
                   ` (10 preceding siblings ...)
  2015-07-26 21:48 ` [rtc-linux] [PATCH 11/11] rtc: tx4939: clean up tx4939_rtc_nvram_read()/tx4939_rtc_nvram_write() Vladimir Zapolskiy
@ 2015-08-14 22:21 ` Alexandre Belloni
  11 siblings, 0 replies; 19+ messages in thread
From: Alexandre Belloni @ 2015-08-14 22:21 UTC (permalink / raw)
  To: Vladimir Zapolskiy; +Cc: Alessandro Zummo, rtc-linux, Greg Kroah-Hartman

On 27/07/2015 at 00:48:25 +0300, Vladimir Zapolskiy wrote :
> This change removes a number of redundant checks on bin attribute
> client's side, the same checks are done by sysfs_kf_bin_read() or
> sysfs_kf_bin_write() caller from fs/sysfs/file.c.
> 
> Note, drivers/rtc/rtc-ds1685.c may be updated in a similar way,
> however due to #ifndef magic involved this task is not done.
> 
> No functional change intended.
> 
> Vladimir Zapolskiy (11):
>   rtc: cmos: clean up cmos_nvram_read()/cmos_nvram_write()
>   rtc: ds1305: clean up ds1305_nvram_read()/ds1305_nvram_write()
>   rtc: ds1307: clean up ds1307_nvram_read()/ds1307_nvram_write()
>   rtc: ds1343: clean up ds1343_nvram_read()/ds1343_nvram_write()
>   rtc: ds1511: clean up ds1511_nvram_read()/ds1511_nvram_write()
>   rtc: ds1553: clean up ds1553_nvram_read()/ds1553_nvram_write()
>   rtc: ds1742: clean up ds1742_nvram_read()/ds1742_nvram_write()
>   rtc: m48t59: clean up m48t59_nvram_read()/m48t59_nvram_write()
>   rtc: rp5c01: clean up rp5c01_nvram_read()/rp5c01_nvram_write()
>   rtc: stk17ta8: clean up stk17ta8_nvram_read()/stk17ta8_nvram_write()
>   rtc: tx4939: clean up tx4939_rtc_nvram_read()/tx4939_rtc_nvram_write()
> 
>  drivers/rtc/rtc-cmos.c     | 13 -------------
>  drivers/rtc/rtc-ds1305.c   | 18 ------------------
>  drivers/rtc/rtc-ds1307.c   | 14 --------------
>  drivers/rtc/rtc-ds1343.c   | 12 ------------
>  drivers/rtc/rtc-ds1511.c   | 38 ++++++++------------------------------
>  drivers/rtc/rtc-ds1553.c   |  4 ++--
>  drivers/rtc/rtc-ds1742.c   |  4 ++--
>  drivers/rtc/rtc-m48t59.c   | 16 ++++++++--------
>  drivers/rtc/rtc-rp5c01.c   |  4 ++--
>  drivers/rtc/rtc-stk17ta8.c |  4 ++--
>  drivers/rtc/rtc-tx4939.c   |  6 ++----
>  11 files changed, 26 insertions(+), 107 deletions(-)
> 

All applied (v2 when available), thanks


-- 
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.

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2015-08-14 22:21 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-26 21:48 [rtc-linux] [PATCH 00/11] rtc: clean up bin attribute read/write functions Vladimir Zapolskiy
2015-07-26 21:48 ` [rtc-linux] [PATCH 01/11] rtc: cmos: clean up cmos_nvram_read()/cmos_nvram_write() Vladimir Zapolskiy
2015-07-26 21:48 ` [rtc-linux] [PATCH 02/11] rtc: ds1305: clean up ds1305_nvram_read()/ds1305_nvram_write() Vladimir Zapolskiy
2015-07-26 21:48 ` [rtc-linux] [PATCH 03/11] rtc: ds1307: clean up ds1307_nvram_read()/ds1307_nvram_write() Vladimir Zapolskiy
2015-07-26 21:48 ` [rtc-linux] [PATCH 04/11] rtc: ds1343: clean up ds1343_nvram_read()/ds1343_nvram_write() Vladimir Zapolskiy
2015-07-26 21:48 ` [rtc-linux] [PATCH 05/11] rtc: ds1511: clean up ds1511_nvram_read()/ds1511_nvram_write() Vladimir Zapolskiy
2015-08-05  8:24   ` [rtc-linux] " Alexandre Belloni
2015-08-05 16:22     ` Vladimir Zapolskiy
2015-08-05 18:12   ` [rtc-linux] [PATCH v2 " Vladimir Zapolskiy
2015-07-26 21:48 ` [rtc-linux] [PATCH 06/11] rtc: ds1553: clean up ds1553_nvram_read()/ds1553_nvram_write() Vladimir Zapolskiy
2015-07-26 21:48 ` [rtc-linux] [PATCH 07/11] rtc: ds1742: clean up ds1742_nvram_read()/ds1742_nvram_write() Vladimir Zapolskiy
2015-07-26 21:48 ` [rtc-linux] [PATCH 08/11] rtc: m48t59: clean up m48t59_nvram_read()/m48t59_nvram_write() Vladimir Zapolskiy
2015-08-05  8:27   ` [rtc-linux] " Alexandre Belloni
2015-08-05 16:04     ` Vladimir Zapolskiy
2015-08-05 18:13   ` [rtc-linux] [PATCH v2 " Vladimir Zapolskiy
2015-07-26 21:48 ` [rtc-linux] [PATCH 09/11] rtc: rp5c01: clean up rp5c01_nvram_read()/rp5c01_nvram_write() Vladimir Zapolskiy
2015-07-26 21:48 ` [rtc-linux] [PATCH 10/11] rtc: stk17ta8: clean up stk17ta8_nvram_read()/stk17ta8_nvram_write() Vladimir Zapolskiy
2015-07-26 21:48 ` [rtc-linux] [PATCH 11/11] rtc: tx4939: clean up tx4939_rtc_nvram_read()/tx4939_rtc_nvram_write() Vladimir Zapolskiy
2015-08-14 22:21 ` [rtc-linux] [PATCH 00/11] rtc: clean up bin attribute read/write functions Alexandre Belloni

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.