[rtc-linux] [PATCH v1 0/4] rtc: pcf2127: a fix, a cleanup and two questionable features

[rtc-linux] [PATCH v1 0/4] rtc: pcf2127: a fix, a cleanup and two questionable features

[Uwe Kleine-König]

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.

[rtc-linux] [PATCH v1 1/4] rtc: pcf2127: fix reading uninitialized value on RTC_READ_VL ioctl

[Uwe Kleine-König]

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.

[rtc-linux] Re: [PATCH v1 1/4] rtc: pcf2127: fix reading uninitialized value on RTC_READ_VL ioctl

[Alexandre Belloni]

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.

[rtc-linux] [PATCH v1 2/4] rtc: pcf2127: remove useless driver version

[Uwe Kleine-König]

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.

[rtc-linux] Re: [PATCH v1 2/4] rtc: pcf2127: remove useless driver version

[Alexandre Belloni]

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.

[rtc-linux] [PATCH v1 3/4 RFC] rtc: pcf2127: implement reading battery status bits

[Uwe Kleine-König]

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, &reg, 1);
+	if (!ret)
+		ret =3D -EIO;
+	if (ret < 0)
+		return ret;
+
+	ret =3D i2c_master_recv(client, &reg, 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.

[rtc-linux] [PATCH v1 4/4 RFC] rtc: pcf2127: implement access to nvram

[Uwe Kleine-König]

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.

[rtc-linux] Re: [PATCH v1 0/4] rtc: pcf2127: a fix, a cleanup and two questionable features

[Alexandre Belloni]

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.