* [PATCH v1 0/4] rtc: pcf2127: a fix, a cleanup and two questionable features
@ 2015-10-02 9:17 ` Uwe Kleine-König
0 siblings, 0 replies; 16+ messages in thread
From: Uwe Kleine-König @ 2015-10-02 9:17 UTC (permalink / raw)
To: Alexandre Belloni
Cc: Alessandro Zummo, rtc-linux, kernel, Greg Kroah-Hartman, linux-kernel
Hello,
the fix and the cleanup should be fine. Patches 3 and 4 are probably
wrong (but still state of the art for other in-tree drivers) because
sysfs_create_file is called too late. See
http://kroah.com/log/blog/2013/06/26/how-to-create-a-sysfs-file-correctly/
for some details.
I failed to find how to do it correctly though:
- the attributes are device specific, so device_driver::groups is wrong
- for using the i2c's device group it's too late in .probe
- the rtc's device is only malloc'd in rtc_device_register and when
this returns it's already too late again.
- these are neither class nor bus attributes, so these are ruled out, too.
Uwe Kleine-König (4):
rtc: pcf2127: fix reading uninitialized value on RTC_READ_VL ioctl
rtc: pcf2127: remove useless driver version
rtc: pcf2127: implement reading battery status bits
rtc: pcf2127: implement access to nvram
drivers/rtc/rtc-pcf2127.c | 285 ++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 261 insertions(+), 24 deletions(-)
--
2.6.0
^ permalink raw reply [flat|nested] 16+ messages in thread
* [rtc-linux] [PATCH v1 0/4] rtc: pcf2127: a fix, a cleanup and two questionable features
@ 2015-10-02 9:17 ` Uwe Kleine-König
0 siblings, 0 replies; 16+ messages in thread
From: Uwe Kleine-König @ 2015-10-02 9:17 UTC (permalink / raw)
To: Alexandre Belloni
Cc: Alessandro Zummo, rtc-linux, kernel, Greg Kroah-Hartman, linux-kernel
Hello,
the fix and the cleanup should be fine. Patches 3 and 4 are probably
wrong (but still state of the art for other in-tree drivers) because
sysfs_create_file is called too late. See
http://kroah.com/log/blog/2013/06/26/how-to-create-a-sysfs-file-correctly/
for some details.
I failed to find how to do it correctly though:
- the attributes are device specific, so device_driver::groups is wrong
- for using the i2c's device group it's too late in .probe
- the rtc's device is only malloc'd in rtc_device_register and when
this returns it's already too late again.
- these are neither class nor bus attributes, so these are ruled out, too.
Uwe Kleine-K=C3=B6nig (4):
rtc: pcf2127: fix reading uninitialized value on RTC_READ_VL ioctl
rtc: pcf2127: remove useless driver version
rtc: pcf2127: implement reading battery status bits
rtc: pcf2127: implement access to nvram
drivers/rtc/rtc-pcf2127.c | 285 ++++++++++++++++++++++++++++++++++++++++++=
----
1 file changed, 261 insertions(+), 24 deletions(-)
--=20
2.6.0
--=20
--=20
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.
---=20
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 e=
mail to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v1 1/4] rtc: pcf2127: fix reading uninitialized value on RTC_READ_VL ioctl
2015-10-02 9:17 ` [rtc-linux] " Uwe Kleine-König
@ 2015-10-02 9:17 ` Uwe Kleine-König
-1 siblings, 0 replies; 16+ messages in thread
From: Uwe Kleine-König @ 2015-10-02 9:17 UTC (permalink / raw)
To: Alexandre Belloni
Cc: Alessandro Zummo, rtc-linux, kernel, Greg Kroah-Hartman, linux-kernel
The flag reported on the RTC_READ_VL ioctl is only initialized when the
date is read out. So the voltage low value doesn't represent reality but
the status at the time the date was read (or 0 if the date was not read
yet).
Moreover when userspace requests a value via an ioctl there is no added
benefit to also make a prosa representation of this (and other) values
appear in the kernel log so remove the calls to dev_info and the driver
data members to track their state.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
Notes:
While this is a fix I don't think it is critical enough to backport it to
stable (or apply it during the rc phase).
Looking at
http://codesearch.debian.net/perpackage-results/RTC_VL_READ
this ioctl isn't used much.
drivers/rtc/rtc-pcf2127.c | 36 ++++++++++++++++++------------------
1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c
index 4c7738f706c1..4f66c4216b5a 100644
--- a/drivers/rtc/rtc-pcf2127.c
+++ b/drivers/rtc/rtc-pcf2127.c
@@ -39,8 +39,6 @@ static struct i2c_driver pcf2127_driver;
struct pcf2127 {
struct rtc_device *rtc;
- int voltage_low; /* indicates if a low_voltage was detected */
- int oscillator_failed; /* OSF was detected and date is unreliable */
};
/*
@@ -49,7 +47,6 @@ struct pcf2127 {
*/
static int pcf2127_get_datetime(struct i2c_client *client, struct rtc_time *tm)
{
- struct pcf2127 *pcf2127 = i2c_get_clientdata(client);
unsigned char buf[10] = { PCF2127_REG_CTRL1 };
/* read registers */
@@ -59,18 +56,15 @@ static int pcf2127_get_datetime(struct i2c_client *client, struct rtc_time *tm)
return -EIO;
}
- if (buf[PCF2127_REG_CTRL3] & 0x04) {
- pcf2127->voltage_low = 1;
+ if (buf[PCF2127_REG_CTRL3] & 0x04)
dev_info(&client->dev,
"low voltage detected, check/replace RTC battery.\n");
- }
if (buf[PCF2127_REG_SC] & PCF2127_OSF) {
/*
* no need clear the flag here,
* it will be cleared once the new date is saved
*/
- pcf2127->oscillator_failed = 1;
dev_warn(&client->dev,
"oscillator stop detected, date/time is not reliable\n");
return -EINVAL;
@@ -107,7 +101,6 @@ static int pcf2127_get_datetime(struct i2c_client *client, struct rtc_time *tm)
static int pcf2127_set_datetime(struct i2c_client *client, struct rtc_time *tm)
{
- struct pcf2127 *pcf2127 = i2c_get_clientdata(client);
unsigned char buf[8];
int i = 0, err;
@@ -141,9 +134,6 @@ static int pcf2127_set_datetime(struct i2c_client *client, struct rtc_time *tm)
return -EIO;
}
- /* clear OSF flag in client data */
- pcf2127->oscillator_failed = 0;
-
return 0;
}
@@ -151,17 +141,27 @@ static int pcf2127_set_datetime(struct i2c_client *client, struct rtc_time *tm)
static int pcf2127_rtc_ioctl(struct device *dev,
unsigned int cmd, unsigned long arg)
{
- struct pcf2127 *pcf2127 = i2c_get_clientdata(to_i2c_client(dev));
+ struct i2c_client *client = to_i2c_client(dev);
+ unsigned char buf = PCF2127_REG_CTRL3;
+ int ret;
switch (cmd) {
case RTC_VL_READ:
- if (pcf2127->voltage_low)
- dev_info(dev, "low voltage detected, check/replace battery\n");
- if (pcf2127->oscillator_failed)
- dev_info(dev, "oscillator stop detected, date/time is not reliable\n");
+ ret = i2c_master_send(client, &buf, 1);
+ if (!ret)
+ ret = -EIO;
+ if (ret < 0)
+ return ret;
+
+ ret = i2c_master_recv(client, &buf, 1);
+ if (!ret)
+ ret = -EIO;
+ if (ret < 0)
+ return ret;
+
+ buf = buf & PCF2127_REG_CTRL3_BLF ? 1 : 0;
- if (copy_to_user((void __user *)arg, &pcf2127->voltage_low,
- sizeof(int)))
+ if (copy_to_user((void __user *)arg, &buf, sizeof(int)))
return -EFAULT;
return 0;
default:
--
2.6.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [rtc-linux] [PATCH v1 1/4] rtc: pcf2127: fix reading uninitialized value on RTC_READ_VL ioctl
@ 2015-10-02 9:17 ` Uwe Kleine-König
0 siblings, 0 replies; 16+ messages in thread
From: Uwe Kleine-König @ 2015-10-02 9:17 UTC (permalink / raw)
To: Alexandre Belloni
Cc: Alessandro Zummo, rtc-linux, kernel, Greg Kroah-Hartman, linux-kernel
The flag reported on the RTC_READ_VL ioctl is only initialized when the
date is read out. So the voltage low value doesn't represent reality but
the status at the time the date was read (or 0 if the date was not read
yet).
Moreover when userspace requests a value via an ioctl there is no added
benefit to also make a prosa representation of this (and other) values
appear in the kernel log so remove the calls to dev_info and the driver
data members to track their state.
Signed-off-by: Uwe Kleine-K=C3=B6nig <u.kleine-koenig@pengutronix.de>
---
Notes:
While this is a fix I don't think it is critical enough to backport it =
to
stable (or apply it during the rc phase).
=20
Looking at
=20
http://codesearch.debian.net/perpackage-results/RTC_VL_READ
=20
this ioctl isn't used much.
drivers/rtc/rtc-pcf2127.c | 36 ++++++++++++++++++------------------
1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c
index 4c7738f706c1..4f66c4216b5a 100644
--- a/drivers/rtc/rtc-pcf2127.c
+++ b/drivers/rtc/rtc-pcf2127.c
@@ -39,8 +39,6 @@ static struct i2c_driver pcf2127_driver;
=20
struct pcf2127 {
struct rtc_device *rtc;
- int voltage_low; /* indicates if a low_voltage was detected */
- int oscillator_failed; /* OSF was detected and date is unreliable */
};
=20
/*
@@ -49,7 +47,6 @@ struct pcf2127 {
*/
static int pcf2127_get_datetime(struct i2c_client *client, struct rtc_time=
*tm)
{
- struct pcf2127 *pcf2127 =3D i2c_get_clientdata(client);
unsigned char buf[10] =3D { PCF2127_REG_CTRL1 };
=20
/* read registers */
@@ -59,18 +56,15 @@ static int pcf2127_get_datetime(struct i2c_client *clie=
nt, struct rtc_time *tm)
return -EIO;
}
=20
- if (buf[PCF2127_REG_CTRL3] & 0x04) {
- pcf2127->voltage_low =3D 1;
+ if (buf[PCF2127_REG_CTRL3] & 0x04)
dev_info(&client->dev,
"low voltage detected, check/replace RTC battery.\n");
- }
=20
if (buf[PCF2127_REG_SC] & PCF2127_OSF) {
/*
* no need clear the flag here,
* it will be cleared once the new date is saved
*/
- pcf2127->oscillator_failed =3D 1;
dev_warn(&client->dev,
"oscillator stop detected, date/time is not reliable\n");
return -EINVAL;
@@ -107,7 +101,6 @@ static int pcf2127_get_datetime(struct i2c_client *clie=
nt, struct rtc_time *tm)
=20
static int pcf2127_set_datetime(struct i2c_client *client, struct rtc_time=
*tm)
{
- struct pcf2127 *pcf2127 =3D i2c_get_clientdata(client);
unsigned char buf[8];
int i =3D 0, err;
=20
@@ -141,9 +134,6 @@ static int pcf2127_set_datetime(struct i2c_client *clie=
nt, struct rtc_time *tm)
return -EIO;
}
=20
- /* clear OSF flag in client data */
- pcf2127->oscillator_failed =3D 0;
-
return 0;
}
=20
@@ -151,17 +141,27 @@ static int pcf2127_set_datetime(struct i2c_client *cl=
ient, struct rtc_time *tm)
static int pcf2127_rtc_ioctl(struct device *dev,
unsigned int cmd, unsigned long arg)
{
- struct pcf2127 *pcf2127 =3D i2c_get_clientdata(to_i2c_client(dev));
+ struct i2c_client *client =3D to_i2c_client(dev);
+ unsigned char buf =3D PCF2127_REG_CTRL3;
+ int ret;
=20
switch (cmd) {
case RTC_VL_READ:
- if (pcf2127->voltage_low)
- dev_info(dev, "low voltage detected, check/replace battery\n");
- if (pcf2127->oscillator_failed)
- dev_info(dev, "oscillator stop detected, date/time is not reliable\n");
+ ret =3D i2c_master_send(client, &buf, 1);
+ if (!ret)
+ ret =3D -EIO;
+ if (ret < 0)
+ return ret;
+
+ ret =3D i2c_master_recv(client, &buf, 1);
+ if (!ret)
+ ret =3D -EIO;
+ if (ret < 0)
+ return ret;
+
+ buf =3D buf & PCF2127_REG_CTRL3_BLF ? 1 : 0;
=20
- if (copy_to_user((void __user *)arg, &pcf2127->voltage_low,
- sizeof(int)))
+ if (copy_to_user((void __user *)arg, &buf, sizeof(int)))
return -EFAULT;
return 0;
default:
--=20
2.6.0
--=20
--=20
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.
---=20
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 e=
mail to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v1 2/4] rtc: pcf2127: remove useless driver version
2015-10-02 9:17 ` [rtc-linux] " Uwe Kleine-König
@ 2015-10-02 9:17 ` Uwe Kleine-König
-1 siblings, 0 replies; 16+ messages in thread
From: Uwe Kleine-König @ 2015-10-02 9:17 UTC (permalink / raw)
To: Alexandre Belloni
Cc: Alessandro Zummo, rtc-linux, kernel, Greg Kroah-Hartman, linux-kernel
A driver version is only really sensible for oot drivers. Also the
dev_info about having found a chip only signals that allocating the
driver data succeeded and so isn't worth much.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
drivers/rtc/rtc-pcf2127.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c
index 4f66c4216b5a..7eb6ff26185e 100644
--- a/drivers/rtc/rtc-pcf2127.c
+++ b/drivers/rtc/rtc-pcf2127.c
@@ -20,8 +20,6 @@
#include <linux/module.h>
#include <linux/of.h>
-#define DRV_VERSION "0.0.1"
-
#define PCF2127_REG_CTRL1 (0x00) /* Control Register 1 */
#define PCF2127_REG_CTRL2 (0x01) /* Control Register 2 */
#define PCF2127_REG_CTRL3 (0x02) /* Control Register 3 */
@@ -203,8 +201,6 @@ static int pcf2127_probe(struct i2c_client *client,
if (!pcf2127)
return -ENOMEM;
- dev_info(&client->dev, "chip found, driver version " DRV_VERSION "\n");
-
i2c_set_clientdata(client, pcf2127);
pcf2127->rtc = devm_rtc_device_register(&client->dev,
@@ -243,4 +239,3 @@ module_i2c_driver(pcf2127_driver);
MODULE_AUTHOR("Renaud Cerrato <r.cerrato@til-technologies.fr>");
MODULE_DESCRIPTION("NXP PCF2127 RTC driver");
MODULE_LICENSE("GPL v2");
-MODULE_VERSION(DRV_VERSION);
--
2.6.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [rtc-linux] [PATCH v1 2/4] rtc: pcf2127: remove useless driver version
@ 2015-10-02 9:17 ` Uwe Kleine-König
0 siblings, 0 replies; 16+ messages in thread
From: Uwe Kleine-König @ 2015-10-02 9:17 UTC (permalink / raw)
To: Alexandre Belloni
Cc: Alessandro Zummo, rtc-linux, kernel, Greg Kroah-Hartman, linux-kernel
A driver version is only really sensible for oot drivers. Also the
dev_info about having found a chip only signals that allocating the
driver data succeeded and so isn't worth much.
Signed-off-by: Uwe Kleine-K=C3=B6nig <u.kleine-koenig@pengutronix.de>
---
drivers/rtc/rtc-pcf2127.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c
index 4f66c4216b5a..7eb6ff26185e 100644
--- a/drivers/rtc/rtc-pcf2127.c
+++ b/drivers/rtc/rtc-pcf2127.c
@@ -20,8 +20,6 @@
#include <linux/module.h>
#include <linux/of.h>
=20
-#define DRV_VERSION "0.0.1"
-
#define PCF2127_REG_CTRL1 (0x00) /* Control Register 1 */
#define PCF2127_REG_CTRL2 (0x01) /* Control Register 2 */
#define PCF2127_REG_CTRL3 (0x02) /* Control Register 3 */
@@ -203,8 +201,6 @@ static int pcf2127_probe(struct i2c_client *client,
if (!pcf2127)
return -ENOMEM;
=20
- dev_info(&client->dev, "chip found, driver version " DRV_VERSION "\n");
-
i2c_set_clientdata(client, pcf2127);
=20
pcf2127->rtc =3D devm_rtc_device_register(&client->dev,
@@ -243,4 +239,3 @@ module_i2c_driver(pcf2127_driver);
MODULE_AUTHOR("Renaud Cerrato <r.cerrato@til-technologies.fr>");
MODULE_DESCRIPTION("NXP PCF2127 RTC driver");
MODULE_LICENSE("GPL v2");
-MODULE_VERSION(DRV_VERSION);
--=20
2.6.0
--=20
--=20
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.
---=20
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 e=
mail to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v1 3/4 RFC] rtc: pcf2127: implement reading battery status bits
2015-10-02 9:17 ` [rtc-linux] " Uwe Kleine-König
@ 2015-10-02 9:17 ` Uwe Kleine-König
-1 siblings, 0 replies; 16+ messages in thread
From: Uwe Kleine-König @ 2015-10-02 9:17 UTC (permalink / raw)
To: Alexandre Belloni
Cc: Alessandro Zummo, rtc-linux, kernel, Greg Kroah-Hartman, linux-kernel
The rtc features a battery switch-over function. Export battery status
and if a switch-over occured via sysfs.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
Notes:
Note this patch is wrong, don't merge!
Calling sysfs_create_files in the driver's probe is too late. I didn't
find a nice alternative though. But in case someone feels like pointing
me in the right direction or using this even with this being wrong, here
comes the patch anyhow.
drivers/rtc/rtc-pcf2127.c | 132 +++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 130 insertions(+), 2 deletions(-)
diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c
index 7eb6ff26185e..db057db88031 100644
--- a/drivers/rtc/rtc-pcf2127.c
+++ b/drivers/rtc/rtc-pcf2127.c
@@ -22,7 +22,11 @@
#define PCF2127_REG_CTRL1 (0x00) /* Control Register 1 */
#define PCF2127_REG_CTRL2 (0x01) /* Control Register 2 */
+
#define PCF2127_REG_CTRL3 (0x02) /* Control Register 3 */
+#define PCF2127_REG_CTRL3_BF 0x08
+#define PCF2127_REG_CTRL3_BLF 0x04
+
#define PCF2127_REG_SC (0x03) /* datetime */
#define PCF2127_REG_MN (0x04)
#define PCF2127_REG_HR (0x05)
@@ -54,7 +58,7 @@ static int pcf2127_get_datetime(struct i2c_client *client, struct rtc_time *tm)
return -EIO;
}
- if (buf[PCF2127_REG_CTRL3] & 0x04)
+ if (buf[PCF2127_REG_CTRL3] & PCF2127_REG_CTRL3_BLF)
dev_info(&client->dev,
"low voltage detected, check/replace RTC battery.\n");
@@ -186,10 +190,116 @@ static const struct rtc_class_ops pcf2127_rtc_ops = {
.set_time = pcf2127_rtc_set_time,
};
+struct pcf2127_bitattr {
+ struct device_attribute attr;
+ unsigned bitmask;
+};
+
+static ssize_t pcf2127_bitattr_clear(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ unsigned char i2cbuf[2] = { PCF2127_REG_CTRL3, };
+ struct pcf2127_bitattr *bitattr =
+ container_of(attr, struct pcf2127_bitattr, attr);
+ int ret;
+ long val;
+
+ if (!count)
+ return 0;
+
+ ret = kstrtol(buf, 0, &val);
+ if (ret)
+ return ret;
+
+ /* we only accept writing 0 */
+ if (val != 0)
+ return -EINVAL;
+
+ ret = i2c_master_send(client, i2cbuf, 1);
+ if (!ret)
+ ret = -EIO;
+ if (ret < 0)
+ return ret;
+
+ ret = i2c_master_recv(client, i2cbuf + 1, 1);
+ if (!ret)
+ ret = -EIO;
+ if (ret < 0)
+ return ret;
+
+ i2cbuf[1] &= ~bitattr->bitmask;
+
+ ret = i2c_master_send(client, i2cbuf, 2);
+ if (!ret)
+ ret = -EIO;
+ if (ret < 0)
+ return ret;
+
+ return count;
+}
+
+static ssize_t pcf2127_bitattr_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+ unsigned char reg = PCF2127_REG_CTRL3;
+ struct pcf2127_bitattr *bitattr =
+ container_of(attr, struct pcf2127_bitattr, attr);
+ int ret;
+
+ ret = i2c_master_send(client, ®, 1);
+ if (!ret)
+ ret = -EIO;
+ if (ret < 0)
+ return ret;
+
+ ret = i2c_master_recv(client, ®, 1);
+ if (!ret)
+ ret = -EIO;
+ if (ret < 0)
+ return ret;
+
+ return scnprintf(buf, PAGE_SIZE,
+ "%d\n", !!(reg & bitattr->bitmask));
+}
+
+static struct pcf2127_bitattr pcf2127_battery_low = {
+ .attr = {
+ .attr = {
+ .name = "battery_low",
+ .mode = S_IRUGO,
+ },
+ .show = pcf2127_bitattr_show,
+ },
+ .bitmask = PCF2127_REG_CTRL3_BLF,
+};
+
+static struct pcf2127_bitattr pcf2127_battery_switch_over = {
+ .attr = {
+ .attr = {
+ .name = "battery_switch_over",
+ .mode = S_IWUSR | S_IRUGO,
+ },
+ .show = pcf2127_bitattr_show,
+ .store = pcf2127_bitattr_clear,
+ },
+ .bitmask = PCF2127_REG_CTRL3_BF,
+};
+
+static const struct attribute *pcf2127_attributes[] = {
+ &pcf2127_battery_low.attr.attr,
+ &pcf2127_battery_switch_over.attr.attr,
+ NULL
+};
+
static int pcf2127_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
struct pcf2127 *pcf2127;
+ int ret;
dev_dbg(&client->dev, "%s\n", __func__);
@@ -203,11 +313,28 @@ static int pcf2127_probe(struct i2c_client *client,
i2c_set_clientdata(client, pcf2127);
+ ret = sysfs_create_files(&client->dev.kobj, pcf2127_attributes);
+ if (ret < 0) {
+ dev_err(&client->dev, "failed to register sysfs attributes\n");
+ return ret;
+ }
+
pcf2127->rtc = devm_rtc_device_register(&client->dev,
pcf2127_driver.driver.name,
&pcf2127_rtc_ops, THIS_MODULE);
- return PTR_ERR_OR_ZERO(pcf2127->rtc);
+ if (IS_ERR(pcf2127->rtc)) {
+ ret = PTR_ERR(pcf2127->rtc);
+ sysfs_remove_files(&client->dev.kobj, pcf2127_attributes);
+ }
+
+ return ret;
+}
+
+static int pcf2127_remove(struct i2c_client *client)
+{
+ sysfs_remove_files(&client->dev.kobj, pcf2127_attributes);
+ return 0;
}
static const struct i2c_device_id pcf2127_id[] = {
@@ -231,6 +358,7 @@ static struct i2c_driver pcf2127_driver = {
.of_match_table = of_match_ptr(pcf2127_of_match),
},
.probe = pcf2127_probe,
+ .remove = pcf2127_remove,
.id_table = pcf2127_id,
};
--
2.6.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [rtc-linux] [PATCH v1 3/4 RFC] rtc: pcf2127: implement reading battery status bits
@ 2015-10-02 9:17 ` Uwe Kleine-König
0 siblings, 0 replies; 16+ messages in thread
From: Uwe Kleine-König @ 2015-10-02 9:17 UTC (permalink / raw)
To: Alexandre Belloni
Cc: Alessandro Zummo, rtc-linux, kernel, Greg Kroah-Hartman, linux-kernel
The rtc features a battery switch-over function. Export battery status
and if a switch-over occured via sysfs.
Signed-off-by: Uwe Kleine-K=C3=B6nig <u.kleine-koenig@pengutronix.de>
---
Notes:
Note this patch is wrong, don't merge!
=20
Calling sysfs_create_files in the driver's probe is too late. I didn't
find a nice alternative though. But in case someone feels like pointing
me in the right direction or using this even with this being wrong, her=
e
comes the patch anyhow.
drivers/rtc/rtc-pcf2127.c | 132 ++++++++++++++++++++++++++++++++++++++++++=
+++-
1 file changed, 130 insertions(+), 2 deletions(-)
diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c
index 7eb6ff26185e..db057db88031 100644
--- a/drivers/rtc/rtc-pcf2127.c
+++ b/drivers/rtc/rtc-pcf2127.c
@@ -22,7 +22,11 @@
=20
#define PCF2127_REG_CTRL1 (0x00) /* Control Register 1 */
#define PCF2127_REG_CTRL2 (0x01) /* Control Register 2 */
+
#define PCF2127_REG_CTRL3 (0x02) /* Control Register 3 */
+#define PCF2127_REG_CTRL3_BF 0x08
+#define PCF2127_REG_CTRL3_BLF 0x04
+
#define PCF2127_REG_SC (0x03) /* datetime */
#define PCF2127_REG_MN (0x04)
#define PCF2127_REG_HR (0x05)
@@ -54,7 +58,7 @@ static int pcf2127_get_datetime(struct i2c_client *client=
, struct rtc_time *tm)
return -EIO;
}
=20
- if (buf[PCF2127_REG_CTRL3] & 0x04)
+ if (buf[PCF2127_REG_CTRL3] & PCF2127_REG_CTRL3_BLF)
dev_info(&client->dev,
"low voltage detected, check/replace RTC battery.\n");
=20
@@ -186,10 +190,116 @@ static const struct rtc_class_ops pcf2127_rtc_ops =
=3D {
.set_time =3D pcf2127_rtc_set_time,
};
=20
+struct pcf2127_bitattr {
+ struct device_attribute attr;
+ unsigned bitmask;
+};
+
+static ssize_t pcf2127_bitattr_clear(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct i2c_client *client =3D to_i2c_client(dev);
+ unsigned char i2cbuf[2] =3D { PCF2127_REG_CTRL3, };
+ struct pcf2127_bitattr *bitattr =3D
+ container_of(attr, struct pcf2127_bitattr, attr);
+ int ret;
+ long val;
+
+ if (!count)
+ return 0;
+
+ ret =3D kstrtol(buf, 0, &val);
+ if (ret)
+ return ret;
+
+ /* we only accept writing 0 */
+ if (val !=3D 0)
+ return -EINVAL;
+
+ ret =3D i2c_master_send(client, i2cbuf, 1);
+ if (!ret)
+ ret =3D -EIO;
+ if (ret < 0)
+ return ret;
+
+ ret =3D i2c_master_recv(client, i2cbuf + 1, 1);
+ if (!ret)
+ ret =3D -EIO;
+ if (ret < 0)
+ return ret;
+
+ i2cbuf[1] &=3D ~bitattr->bitmask;
+
+ ret =3D i2c_master_send(client, i2cbuf, 2);
+ if (!ret)
+ ret =3D -EIO;
+ if (ret < 0)
+ return ret;
+
+ return count;
+}
+
+static ssize_t pcf2127_bitattr_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct i2c_client *client =3D to_i2c_client(dev);
+ unsigned char reg =3D PCF2127_REG_CTRL3;
+ struct pcf2127_bitattr *bitattr =3D
+ container_of(attr, struct pcf2127_bitattr, attr);
+ int ret;
+
+ ret =3D i2c_master_send(client, ®, 1);
+ if (!ret)
+ ret =3D -EIO;
+ if (ret < 0)
+ return ret;
+
+ ret =3D i2c_master_recv(client, ®, 1);
+ if (!ret)
+ ret =3D -EIO;
+ if (ret < 0)
+ return ret;
+
+ return scnprintf(buf, PAGE_SIZE,
+ "%d\n", !!(reg & bitattr->bitmask));
+}
+
+static struct pcf2127_bitattr pcf2127_battery_low =3D {
+ .attr =3D {
+ .attr =3D {
+ .name =3D "battery_low",
+ .mode =3D S_IRUGO,
+ },
+ .show =3D pcf2127_bitattr_show,
+ },
+ .bitmask =3D PCF2127_REG_CTRL3_BLF,
+};
+
+static struct pcf2127_bitattr pcf2127_battery_switch_over =3D {
+ .attr =3D {
+ .attr =3D {
+ .name =3D "battery_switch_over",
+ .mode =3D S_IWUSR | S_IRUGO,
+ },
+ .show =3D pcf2127_bitattr_show,
+ .store =3D pcf2127_bitattr_clear,
+ },
+ .bitmask =3D PCF2127_REG_CTRL3_BF,
+};
+
+static const struct attribute *pcf2127_attributes[] =3D {
+ &pcf2127_battery_low.attr.attr,
+ &pcf2127_battery_switch_over.attr.attr,
+ NULL
+};
+
static int pcf2127_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
struct pcf2127 *pcf2127;
+ int ret;
=20
dev_dbg(&client->dev, "%s\n", __func__);
=20
@@ -203,11 +313,28 @@ static int pcf2127_probe(struct i2c_client *client,
=20
i2c_set_clientdata(client, pcf2127);
=20
+ ret =3D sysfs_create_files(&client->dev.kobj, pcf2127_attributes);
+ if (ret < 0) {
+ dev_err(&client->dev, "failed to register sysfs attributes\n");
+ return ret;
+ }
+
pcf2127->rtc =3D devm_rtc_device_register(&client->dev,
pcf2127_driver.driver.name,
&pcf2127_rtc_ops, THIS_MODULE);
=20
- return PTR_ERR_OR_ZERO(pcf2127->rtc);
+ if (IS_ERR(pcf2127->rtc)) {
+ ret =3D PTR_ERR(pcf2127->rtc);
+ sysfs_remove_files(&client->dev.kobj, pcf2127_attributes);
+ }
+
+ return ret;
+}
+
+static int pcf2127_remove(struct i2c_client *client)
+{
+ sysfs_remove_files(&client->dev.kobj, pcf2127_attributes);
+ return 0;
}
=20
static const struct i2c_device_id pcf2127_id[] =3D {
@@ -231,6 +358,7 @@ static struct i2c_driver pcf2127_driver =3D {
.of_match_table =3D of_match_ptr(pcf2127_of_match),
},
.probe =3D pcf2127_probe,
+ .remove =3D pcf2127_remove,
.id_table =3D pcf2127_id,
};
=20
--=20
2.6.0
--=20
--=20
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.
---=20
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 e=
mail to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v1 4/4 RFC] rtc: pcf2127: implement access to nvram
2015-10-02 9:17 ` [rtc-linux] " Uwe Kleine-König
@ 2015-10-02 9:17 ` Uwe Kleine-König
-1 siblings, 0 replies; 16+ messages in thread
From: Uwe Kleine-König @ 2015-10-02 9:17 UTC (permalink / raw)
To: Alexandre Belloni
Cc: Alessandro Zummo, rtc-linux, kernel, Greg Kroah-Hartman, linux-kernel
The rtc features a 512 byte battery backed RAM. Make it available via
sysfs.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
Notes:
This patch is broken in the same way as the previous one. Don't merge!
drivers/rtc/rtc-pcf2127.c | 114 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 114 insertions(+)
diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c
index db057db88031..e21dc5dfc9fb 100644
--- a/drivers/rtc/rtc-pcf2127.c
+++ b/drivers/rtc/rtc-pcf2127.c
@@ -35,6 +35,10 @@
#define PCF2127_REG_MO (0x08)
#define PCF2127_REG_YR (0x09)
+#define PCF2127_REG_RAM_addr_MSB 0x1a
+#define PCF2127_REG_RAM_wrt_cmd 0x1c
+#define PCF2127_REG_RAM_rd_cmd 0x1d
+
#define PCF2127_OSF BIT(7) /* Oscillator Fail flag */
static struct i2c_driver pcf2127_driver;
@@ -295,6 +299,106 @@ static const struct attribute *pcf2127_attributes[] = {
NULL
};
+#define PCF2127_NVRAM_SIZE 512
+
+static ssize_t pcf2127_nvram_read(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *attr, char *buf,
+ loff_t off, size_t count)
+{
+ struct i2c_client *client;
+ int ret;
+ unsigned char i2cbuf[3] = { PCF2127_REG_RAM_addr_MSB, off >> 8, off };
+
+ if (off >= PCF2127_NVRAM_SIZE)
+ return 0;
+
+ if (count > PCF2127_NVRAM_SIZE - off)
+ count = PCF2127_NVRAM_SIZE - off;
+
+ if (!count)
+ return 0;
+
+ client = kobj_to_i2c_client(kobj);
+
+ /* set offset */
+ ret = i2c_master_send(client, i2cbuf, 3);
+ if (!ret)
+ ret = -EIO;
+ if (ret < 0) {
+ dev_err(&client->dev, "failed to set RAM address\n");
+ return ret;
+ }
+
+ i2cbuf[0] = PCF2127_REG_RAM_rd_cmd;
+ ret = i2c_master_send(client, i2cbuf, 1);
+ if (!ret)
+ ret = -EIO;
+ if (ret < 0) {
+ dev_err(&client->dev, "failed to issue read command\n");
+ return ret;
+ }
+
+ ret = i2c_master_recv(client, buf, count);
+ if (!ret)
+ ret = -EIO;
+ if (ret < 0)
+ dev_err(&client->dev, "failed to read data\n");
+
+ return ret;
+}
+
+static ssize_t pcf2127a_nvram_write(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *attr, char *buf,
+ loff_t off, size_t count)
+{
+ struct i2c_client *client;
+ int ret;
+ unsigned char i2cbuf[PCF2127_NVRAM_SIZE + 1] = {
+ PCF2127_REG_RAM_addr_MSB, off >> 8, off };
+
+ if (off >= PCF2127_NVRAM_SIZE)
+ return 0;
+
+ if (count > PCF2127_NVRAM_SIZE - off)
+ count = PCF2127_NVRAM_SIZE - off;
+
+ if (!count)
+ return 0;
+
+ client = kobj_to_i2c_client(kobj);
+
+ /* set offset */
+ ret = i2c_master_send(client, i2cbuf, 3);
+ if (!ret)
+ ret = -EIO;
+ if (ret < 0) {
+ dev_err(&client->dev, "failed to set RAM address\n");
+ return ret;
+ }
+
+ i2cbuf[0] = PCF2127_REG_RAM_wrt_cmd;
+ memcpy(i2cbuf + 1, buf, count);
+ ret = i2c_master_send(client, i2cbuf, count + 1);
+ if (!ret)
+ ret = -EIO;
+ if (ret < 0) {
+ dev_err(&client->dev, "failed to issue write command\n");
+ return ret;
+ }
+
+ return ret - 1;
+}
+
+static const struct bin_attribute pcf2127_nvram = {
+ .attr = {
+ .name = "nvram",
+ .mode = S_IWUSR | S_IRUGO,
+ },
+ .read = pcf2127_nvram_read,
+ .write = pcf2127a_nvram_write,
+ .size = PCF2127_NVRAM_SIZE,
+};
+
static int pcf2127_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
@@ -319,12 +423,21 @@ static int pcf2127_probe(struct i2c_client *client,
return ret;
}
+ ret = sysfs_create_bin_file(&client->dev.kobj, &pcf2127_nvram);
+ if (ret < 0) {
+ dev_err(&client->dev,
+ "failed to register sysfs nvram attribute\n");
+ goto err_create_bin_file;
+ }
+
pcf2127->rtc = devm_rtc_device_register(&client->dev,
pcf2127_driver.driver.name,
&pcf2127_rtc_ops, THIS_MODULE);
if (IS_ERR(pcf2127->rtc)) {
ret = PTR_ERR(pcf2127->rtc);
+ sysfs_remove_bin_file(&client->dev.kobj, &pcf2127_nvram);
+err_create_bin_file:
sysfs_remove_files(&client->dev.kobj, pcf2127_attributes);
}
@@ -333,6 +446,7 @@ static int pcf2127_probe(struct i2c_client *client,
static int pcf2127_remove(struct i2c_client *client)
{
+ sysfs_remove_bin_file(&client->dev.kobj, &pcf2127_nvram);
sysfs_remove_files(&client->dev.kobj, pcf2127_attributes);
return 0;
}
--
2.6.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [rtc-linux] [PATCH v1 4/4 RFC] rtc: pcf2127: implement access to nvram
@ 2015-10-02 9:17 ` Uwe Kleine-König
0 siblings, 0 replies; 16+ messages in thread
From: Uwe Kleine-König @ 2015-10-02 9:17 UTC (permalink / raw)
To: Alexandre Belloni
Cc: Alessandro Zummo, rtc-linux, kernel, Greg Kroah-Hartman, linux-kernel
The rtc features a 512 byte battery backed RAM. Make it available via
sysfs.
Signed-off-by: Uwe Kleine-K=C3=B6nig <u.kleine-koenig@pengutronix.de>
---
Notes:
This patch is broken in the same way as the previous one. Don't merge!
drivers/rtc/rtc-pcf2127.c | 114 ++++++++++++++++++++++++++++++++++++++++++=
++++
1 file changed, 114 insertions(+)
diff --git a/drivers/rtc/rtc-pcf2127.c b/drivers/rtc/rtc-pcf2127.c
index db057db88031..e21dc5dfc9fb 100644
--- a/drivers/rtc/rtc-pcf2127.c
+++ b/drivers/rtc/rtc-pcf2127.c
@@ -35,6 +35,10 @@
#define PCF2127_REG_MO (0x08)
#define PCF2127_REG_YR (0x09)
=20
+#define PCF2127_REG_RAM_addr_MSB 0x1a
+#define PCF2127_REG_RAM_wrt_cmd 0x1c
+#define PCF2127_REG_RAM_rd_cmd 0x1d
+
#define PCF2127_OSF BIT(7) /* Oscillator Fail flag */
=20
static struct i2c_driver pcf2127_driver;
@@ -295,6 +299,106 @@ static const struct attribute *pcf2127_attributes[] =
=3D {
NULL
};
=20
+#define PCF2127_NVRAM_SIZE 512
+
+static ssize_t pcf2127_nvram_read(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *attr, char *buf,
+ loff_t off, size_t count)
+{
+ struct i2c_client *client;
+ int ret;
+ unsigned char i2cbuf[3] =3D { PCF2127_REG_RAM_addr_MSB, off >> 8, off };
+
+ if (off >=3D PCF2127_NVRAM_SIZE)
+ return 0;
+
+ if (count > PCF2127_NVRAM_SIZE - off)
+ count =3D PCF2127_NVRAM_SIZE - off;
+
+ if (!count)
+ return 0;
+
+ client =3D kobj_to_i2c_client(kobj);
+
+ /* set offset */
+ ret =3D i2c_master_send(client, i2cbuf, 3);
+ if (!ret)
+ ret =3D -EIO;
+ if (ret < 0) {
+ dev_err(&client->dev, "failed to set RAM address\n");
+ return ret;
+ }
+
+ i2cbuf[0] =3D PCF2127_REG_RAM_rd_cmd;
+ ret =3D i2c_master_send(client, i2cbuf, 1);
+ if (!ret)
+ ret =3D -EIO;
+ if (ret < 0) {
+ dev_err(&client->dev, "failed to issue read command\n");
+ return ret;
+ }
+
+ ret =3D i2c_master_recv(client, buf, count);
+ if (!ret)
+ ret =3D -EIO;
+ if (ret < 0)
+ dev_err(&client->dev, "failed to read data\n");
+
+ return ret;
+}
+
+static ssize_t pcf2127a_nvram_write(struct file *filp, struct kobject *kob=
j,
+ struct bin_attribute *attr, char *buf,
+ loff_t off, size_t count)
+{
+ struct i2c_client *client;
+ int ret;
+ unsigned char i2cbuf[PCF2127_NVRAM_SIZE + 1] =3D {
+ PCF2127_REG_RAM_addr_MSB, off >> 8, off };
+
+ if (off >=3D PCF2127_NVRAM_SIZE)
+ return 0;
+
+ if (count > PCF2127_NVRAM_SIZE - off)
+ count =3D PCF2127_NVRAM_SIZE - off;
+
+ if (!count)
+ return 0;
+
+ client =3D kobj_to_i2c_client(kobj);
+
+ /* set offset */
+ ret =3D i2c_master_send(client, i2cbuf, 3);
+ if (!ret)
+ ret =3D -EIO;
+ if (ret < 0) {
+ dev_err(&client->dev, "failed to set RAM address\n");
+ return ret;
+ }
+
+ i2cbuf[0] =3D PCF2127_REG_RAM_wrt_cmd;
+ memcpy(i2cbuf + 1, buf, count);
+ ret =3D i2c_master_send(client, i2cbuf, count + 1);
+ if (!ret)
+ ret =3D -EIO;
+ if (ret < 0) {
+ dev_err(&client->dev, "failed to issue write command\n");
+ return ret;
+ }
+
+ return ret - 1;
+}
+
+static const struct bin_attribute pcf2127_nvram =3D {
+ .attr =3D {
+ .name =3D "nvram",
+ .mode =3D S_IWUSR | S_IRUGO,
+ },
+ .read =3D pcf2127_nvram_read,
+ .write =3D pcf2127a_nvram_write,
+ .size =3D PCF2127_NVRAM_SIZE,
+};
+
static int pcf2127_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
@@ -319,12 +423,21 @@ static int pcf2127_probe(struct i2c_client *client,
return ret;
}
=20
+ ret =3D sysfs_create_bin_file(&client->dev.kobj, &pcf2127_nvram);
+ if (ret < 0) {
+ dev_err(&client->dev,
+ "failed to register sysfs nvram attribute\n");
+ goto err_create_bin_file;
+ }
+
pcf2127->rtc =3D devm_rtc_device_register(&client->dev,
pcf2127_driver.driver.name,
&pcf2127_rtc_ops, THIS_MODULE);
=20
if (IS_ERR(pcf2127->rtc)) {
ret =3D PTR_ERR(pcf2127->rtc);
+ sysfs_remove_bin_file(&client->dev.kobj, &pcf2127_nvram);
+err_create_bin_file:
sysfs_remove_files(&client->dev.kobj, pcf2127_attributes);
}
=20
@@ -333,6 +446,7 @@ static int pcf2127_probe(struct i2c_client *client,
=20
static int pcf2127_remove(struct i2c_client *client)
{
+ sysfs_remove_bin_file(&client->dev.kobj, &pcf2127_nvram);
sysfs_remove_files(&client->dev.kobj, pcf2127_attributes);
return 0;
}
--=20
2.6.0
--=20
--=20
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.
---=20
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 e=
mail to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v1 0/4] rtc: pcf2127: a fix, a cleanup and two questionable features
2015-10-02 9:17 ` [rtc-linux] " Uwe Kleine-König
@ 2015-10-14 23:50 ` Alexandre Belloni
-1 siblings, 0 replies; 16+ messages in thread
From: Alexandre Belloni @ 2015-10-14 23:50 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Alessandro Zummo, rtc-linux, kernel, Greg Kroah-Hartman, linux-kernel
On 02/10/2015 at 11:17:18 +0200, Uwe Kleine-König wrote :
> Hello,
>
> the fix and the cleanup should be fine. Patches 3 and 4 are probably
> wrong (but still state of the art for other in-tree drivers) because
> sysfs_create_file is called too late. See
> http://kroah.com/log/blog/2013/06/26/how-to-create-a-sysfs-file-correctly/
> for some details.
>
> I failed to find how to do it correctly though:
> - the attributes are device specific, so device_driver::groups is wrong
> - for using the i2c's device group it's too late in .probe
> - the rtc's device is only malloc'd in rtc_device_register and when
> this returns it's already too late again.
> - these are neither class nor bus attributes, so these are ruled out, too.
>
Most of the drivers currently don't care about that race condition so I
could merge them as-is.
I currently have a PoC that creates the nvram file with the rest of the
sysfs files which solves that. I think this could also be extended to
support battery status.
> Uwe Kleine-König (4):
> rtc: pcf2127: fix reading uninitialized value on RTC_READ_VL ioctl
> rtc: pcf2127: remove useless driver version
> rtc: pcf2127: implement reading battery status bits
> rtc: pcf2127: implement access to nvram
>
> drivers/rtc/rtc-pcf2127.c | 285 ++++++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 261 insertions(+), 24 deletions(-)
>
> --
> 2.6.0
>
--
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 16+ messages in thread
* [rtc-linux] Re: [PATCH v1 0/4] rtc: pcf2127: a fix, a cleanup and two questionable features
@ 2015-10-14 23:50 ` Alexandre Belloni
0 siblings, 0 replies; 16+ messages in thread
From: Alexandre Belloni @ 2015-10-14 23:50 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Alessandro Zummo, rtc-linux, kernel, Greg Kroah-Hartman, linux-kernel
On 02/10/2015 at 11:17:18 +0200, Uwe Kleine-K=C3=B6nig wrote :
> Hello,
>=20
> the fix and the cleanup should be fine. Patches 3 and 4 are probably
> wrong (but still state of the art for other in-tree drivers) because
> sysfs_create_file is called too late. See
> http://kroah.com/log/blog/2013/06/26/how-to-create-a-sysfs-file-correctly=
/
> for some details.
>=20
> I failed to find how to do it correctly though:
> - the attributes are device specific, so device_driver::groups is wrong
> - for using the i2c's device group it's too late in .probe
> - the rtc's device is only malloc'd in rtc_device_register and when
> this returns it's already too late again.
> - these are neither class nor bus attributes, so these are ruled out, to=
o.
>=20
Most of the drivers currently don't care about that race condition so I
could merge them as-is.
I currently have a PoC that creates the nvram file with the rest of the
sysfs files which solves that. I think this could also be extended to
support battery status.
> Uwe Kleine-K=C3=B6nig (4):
> rtc: pcf2127: fix reading uninitialized value on RTC_READ_VL ioctl
> rtc: pcf2127: remove useless driver version
> rtc: pcf2127: implement reading battery status bits
> rtc: pcf2127: implement access to nvram
>=20
> drivers/rtc/rtc-pcf2127.c | 285 ++++++++++++++++++++++++++++++++++++++++=
++----
> 1 file changed, 261 insertions(+), 24 deletions(-)
>=20
> --=20
> 2.6.0
>=20
--=20
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--=20
--=20
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.
---=20
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 e=
mail to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v1 1/4] rtc: pcf2127: fix reading uninitialized value on RTC_READ_VL ioctl
2015-10-02 9:17 ` [rtc-linux] " Uwe Kleine-König
@ 2015-10-18 0:14 ` Alexandre Belloni
-1 siblings, 0 replies; 16+ messages in thread
From: Alexandre Belloni @ 2015-10-18 0:14 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Alessandro Zummo, rtc-linux, kernel, Greg Kroah-Hartman, linux-kernel
On 02/10/2015 at 11:17:19 +0200, Uwe Kleine-König wrote :
> The flag reported on the RTC_READ_VL ioctl is only initialized when the
> date is read out. So the voltage low value doesn't represent reality but
> the status at the time the date was read (or 0 if the date was not read
> yet).
>
> Moreover when userspace requests a value via an ioctl there is no added
> benefit to also make a prosa representation of this (and other) values
> appear in the kernel log so remove the calls to dev_info and the driver
> data members to track their state.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Applied, thanks.
--
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 16+ messages in thread
* [rtc-linux] Re: [PATCH v1 1/4] rtc: pcf2127: fix reading uninitialized value on RTC_READ_VL ioctl
@ 2015-10-18 0:14 ` Alexandre Belloni
0 siblings, 0 replies; 16+ messages in thread
From: Alexandre Belloni @ 2015-10-18 0:14 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Alessandro Zummo, rtc-linux, kernel, Greg Kroah-Hartman, linux-kernel
On 02/10/2015 at 11:17:19 +0200, Uwe Kleine-K=C3=B6nig wrote :
> The flag reported on the RTC_READ_VL ioctl is only initialized when the
> date is read out. So the voltage low value doesn't represent reality but
> the status at the time the date was read (or 0 if the date was not read
> yet).
>=20
> Moreover when userspace requests a value via an ioctl there is no added
> benefit to also make a prosa representation of this (and other) values
> appear in the kernel log so remove the calls to dev_info and the driver
> data members to track their state.
>=20
> Signed-off-by: Uwe Kleine-K=C3=B6nig <u.kleine-koenig@pengutronix.de>
Applied, thanks.
--=20
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--=20
--=20
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.
---=20
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 e=
mail to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v1 2/4] rtc: pcf2127: remove useless driver version
2015-10-02 9:17 ` [rtc-linux] " Uwe Kleine-König
@ 2015-10-18 0:14 ` Alexandre Belloni
-1 siblings, 0 replies; 16+ messages in thread
From: Alexandre Belloni @ 2015-10-18 0:14 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Alessandro Zummo, rtc-linux, kernel, Greg Kroah-Hartman, linux-kernel
On 02/10/2015 at 11:17:20 +0200, Uwe Kleine-König wrote :
> A driver version is only really sensible for oot drivers. Also the
> dev_info about having found a chip only signals that allocating the
> driver data succeeded and so isn't worth much.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> drivers/rtc/rtc-pcf2127.c | 5 -----
> 1 file changed, 5 deletions(-)
>
Applied, thanks.
--
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 16+ messages in thread
* [rtc-linux] Re: [PATCH v1 2/4] rtc: pcf2127: remove useless driver version
@ 2015-10-18 0:14 ` Alexandre Belloni
0 siblings, 0 replies; 16+ messages in thread
From: Alexandre Belloni @ 2015-10-18 0:14 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Alessandro Zummo, rtc-linux, kernel, Greg Kroah-Hartman, linux-kernel
On 02/10/2015 at 11:17:20 +0200, Uwe Kleine-K=C3=B6nig wrote :
> A driver version is only really sensible for oot drivers. Also the
> dev_info about having found a chip only signals that allocating the
> driver data succeeded and so isn't worth much.
>=20
> Signed-off-by: Uwe Kleine-K=C3=B6nig <u.kleine-koenig@pengutronix.de>
> ---
> drivers/rtc/rtc-pcf2127.c | 5 -----
> 1 file changed, 5 deletions(-)
>=20
Applied, thanks.
--=20
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--=20
--=20
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.
---=20
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 e=
mail to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2015-10-18 0:14 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-02 9:17 [PATCH v1 0/4] rtc: pcf2127: a fix, a cleanup and two questionable features Uwe Kleine-König
2015-10-02 9:17 ` [rtc-linux] " Uwe Kleine-König
2015-10-02 9:17 ` [PATCH v1 1/4] rtc: pcf2127: fix reading uninitialized value on RTC_READ_VL ioctl Uwe Kleine-König
2015-10-02 9:17 ` [rtc-linux] " Uwe Kleine-König
2015-10-18 0:14 ` Alexandre Belloni
2015-10-18 0:14 ` [rtc-linux] " Alexandre Belloni
2015-10-02 9:17 ` [PATCH v1 2/4] rtc: pcf2127: remove useless driver version Uwe Kleine-König
2015-10-02 9:17 ` [rtc-linux] " Uwe Kleine-König
2015-10-18 0:14 ` Alexandre Belloni
2015-10-18 0:14 ` [rtc-linux] " Alexandre Belloni
2015-10-02 9:17 ` [PATCH v1 3/4 RFC] rtc: pcf2127: implement reading battery status bits Uwe Kleine-König
2015-10-02 9:17 ` [rtc-linux] " Uwe Kleine-König
2015-10-02 9:17 ` [PATCH v1 4/4 RFC] rtc: pcf2127: implement access to nvram Uwe Kleine-König
2015-10-02 9:17 ` [rtc-linux] " Uwe Kleine-König
2015-10-14 23:50 ` [PATCH v1 0/4] rtc: pcf2127: a fix, a cleanup and two questionable features Alexandre Belloni
2015-10-14 23:50 ` [rtc-linux] " 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.