Re: [PATCH] rtc: isl12026: Add driver.

2018-02-13 Thread David Daney 

On 02/13/2018 07:54 AM, Andy Shevchenko wrote:
[...]

diff --git a/drivers/rtc/rtc-isl12026.c b/drivers/rtc/rtc-isl12026.c
new file mode 100644
index ..a5f04e0faceb
--- /dev/null
+++ b/drivers/rtc/rtc-isl12026.c
@@ -0,0 +1,550 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * An I2C driver for the Intersil ISL 12026
+ *
+ * Copyright (c) 2018 Cavium, Inc.
+ */



+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 


Perhaps in alphabetical order.


Done.




+/* register offsets */
+#define ISL12026_REG_SC0x30
+
+#define ISL12026_REG_SR0x3f
+# define ISL12026_REG_SR_RTCF  BIT(0)
+# define ISL12026_REG_SR_WEL   BIT(1)
+# define ISL12026_REG_SR_RWEL  BIT(2)
+# define ISL12026_REG_SR_MBZ   BIT(3)
+# define ISL12026_REG_SR_OSCF  BIT(4)
+
+/* ISL register bits */
+#define ISL12026_HR_MILBIT(7)  /* military or 24 hour time */
+



+#define ISL12026_REG_PWR   0x14


Perhaps keep it ordered? (0x14 < 0x30 obviously)


Done.




+# define ISL12026_REG_PWR_BSW  BIT(6)
+# define ISL12026_REG_PWR_SBIB BIT(7)



[...]

+
+static int isl12026_rtc_set_time(struct device *dev, struct rtc_time *tm)
+{
+   struct i2c_client *client = to_i2c_client(dev);
+   struct isl12026 *priv = i2c_get_clientdata(client);



+   int rv = 0;



+   u8 op[10];
+
+   struct i2c_msg msg = {
+   .addr   = client->addr,
+   .flags  = 0,
+   .len= 1,
+   .buf= op
+   };



+


Redundant.


+   int ret;


rv, ret, ... ???


I changed everything to use 'ret' to contain values returned from 
function calls, and 'rv' for the value the current function will return.





+
+   mutex_lock(>lock);
+
+   /* Set SR.WEL */
+   op[0] = 0;
+   op[1] = ISL12026_REG_SR;
+   op[2] = ISL12026_REG_SR_WEL;
+   msg.len = 3;



+   ret = i2c_transfer(client->adapter, , 1);
+   if (ret != 1) {


Can it return < 0? If so, why do you shadow the error code?


I think I fixed this driver wide.




[...]

+   ret = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
+   if (ret != ARRAY_SIZE(msgs)) {
+   dev_err(>dev, "read error, ret=%d\n", ret);
+   rv = -EIO;
+   goto out;
+   }
+



+   if (sr & ISL12026_REG_SR_RTCF)
+   dev_warn(>dev, "Real-Time Clock Failure on read\n");
+   if (sr & ISL12026_REG_SR_OSCF)
+   dev_warn(>dev, "Oscillator Failure on read\n");


Can you get them together?
if not, perhaps consider 'else' keyword.


They are independent error indicators, I think it is best to test for 
them unconditionally like this.





+
+   /* Second, CCR regs */
+   addr[0] = 0;
+   addr[1] = ISL12026_REG_SC;
+   msgs[1].len = sizeof(ccr);
+   msgs[1].buf = ccr;
+
+   ret = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
+   if (ret != ARRAY_SIZE(msgs)) {
+   dev_err(>dev, "read error, ret=%d\n", ret);
+   rv = -EIO;
+   goto out;
+   }
+



+   dev_dbg(>dev,
+   "raw data is sec=%02x, min=%02x, hr=%02x, mday=%02x, mon=%02x, 
year=%02x, wday=%02x, y2k=%02x\n",
+   ccr[0], ccr[1], ccr[2], ccr[3],
+   ccr[4], ccr[5], ccr[6], ccr[7]);


Ouch.



Not essential, I removed it.



+
+   tm->tm_sec = bcd2bin(ccr[0] & 0x7F);
+   tm->tm_min = bcd2bin(ccr[1] & 0x7F);
+   if (ccr[2] & ISL12026_HR_MIL)
+   tm->tm_hour = bcd2bin(ccr[2] & 0x3F);
+   else
+   tm->tm_hour = bcd2bin(ccr[2] & 0x1F) +
+   ((ccr[2] & 0x20) ? 12 : 0);
+   tm->tm_mday = bcd2bin(ccr[3] & 0x3F);
+   tm->tm_mon = bcd2bin(ccr[4] & 0x1F) - 1;
+   tm->tm_year = bcd2bin(ccr[5]);
+   if (bcd2bin(ccr[7]) == 20)
+   tm->tm_year += 100;
+   tm->tm_wday = ccr[6] & 0x07;
+
+   dev_dbg(>dev, "secs=%d, mins=%d, hours=%d, mday=%d, mon=%d, year=%d, 
wday=%d\n",
+   tm->tm_sec, tm->tm_min, tm->tm_hour,
+   tm->tm_mday, tm->tm_mon, tm->tm_year, tm->tm_wday);
+   rv = rtc_valid_tm(tm);



+out:
+   mutex_unlock(>lock);


Here one pattern, below another...

Hey, you have to learn being consistent.


I made them all like this one.  Single function exit point, with the 
unlock immediately before the return.






+   return rv;
+}
+
+static const struct rtc_class_ops isl12026_rtc_ops = {
+   .read_time  = isl12026_rtc_read_time,
+   .set_time   = isl12026_rtc_set_time,
+};
+
+static int isl12026_nvm_read(void *p, unsigned int offset,
+void *val, size_t bytes)
+{
+   struct isl12026 *priv = p;
+   int r;
+   u8 addr[2];
+   struct i2c_msg msgs[] = {
+   {
+   .addr   = priv->nvm_client->addr,
+   .flags  = 0,
+   .len=

Re: [PATCH] rtc: isl12026: Add driver.

2018-02-13 Thread David Daney 

On 02/13/2018 07:54 AM, Andy Shevchenko wrote:
[...]

diff --git a/drivers/rtc/rtc-isl12026.c b/drivers/rtc/rtc-isl12026.c
new file mode 100644
index ..a5f04e0faceb
--- /dev/null
+++ b/drivers/rtc/rtc-isl12026.c
@@ -0,0 +1,550 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * An I2C driver for the Intersil ISL 12026
+ *
+ * Copyright (c) 2018 Cavium, Inc.
+ */



+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 


Perhaps in alphabetical order.


Done.




+/* register offsets */
+#define ISL12026_REG_SC0x30
+
+#define ISL12026_REG_SR0x3f
+# define ISL12026_REG_SR_RTCF  BIT(0)
+# define ISL12026_REG_SR_WEL   BIT(1)
+# define ISL12026_REG_SR_RWEL  BIT(2)
+# define ISL12026_REG_SR_MBZ   BIT(3)
+# define ISL12026_REG_SR_OSCF  BIT(4)
+
+/* ISL register bits */
+#define ISL12026_HR_MILBIT(7)  /* military or 24 hour time */
+



+#define ISL12026_REG_PWR   0x14


Perhaps keep it ordered? (0x14 < 0x30 obviously)


Done.




+# define ISL12026_REG_PWR_BSW  BIT(6)
+# define ISL12026_REG_PWR_SBIB BIT(7)



[...]

+
+static int isl12026_rtc_set_time(struct device *dev, struct rtc_time *tm)
+{
+   struct i2c_client *client = to_i2c_client(dev);
+   struct isl12026 *priv = i2c_get_clientdata(client);



+   int rv = 0;



+   u8 op[10];
+
+   struct i2c_msg msg = {
+   .addr   = client->addr,
+   .flags  = 0,
+   .len= 1,
+   .buf= op
+   };



+


Redundant.


+   int ret;


rv, ret, ... ???


I changed everything to use 'ret' to contain values returned from 
function calls, and 'rv' for the value the current function will return.





+
+   mutex_lock(>lock);
+
+   /* Set SR.WEL */
+   op[0] = 0;
+   op[1] = ISL12026_REG_SR;
+   op[2] = ISL12026_REG_SR_WEL;
+   msg.len = 3;



+   ret = i2c_transfer(client->adapter, , 1);
+   if (ret != 1) {


Can it return < 0? If so, why do you shadow the error code?


I think I fixed this driver wide.




[...]

+   ret = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
+   if (ret != ARRAY_SIZE(msgs)) {
+   dev_err(>dev, "read error, ret=%d\n", ret);
+   rv = -EIO;
+   goto out;
+   }
+



+   if (sr & ISL12026_REG_SR_RTCF)
+   dev_warn(>dev, "Real-Time Clock Failure on read\n");
+   if (sr & ISL12026_REG_SR_OSCF)
+   dev_warn(>dev, "Oscillator Failure on read\n");


Can you get them together?
if not, perhaps consider 'else' keyword.


They are independent error indicators, I think it is best to test for 
them unconditionally like this.





+
+   /* Second, CCR regs */
+   addr[0] = 0;
+   addr[1] = ISL12026_REG_SC;
+   msgs[1].len = sizeof(ccr);
+   msgs[1].buf = ccr;
+
+   ret = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
+   if (ret != ARRAY_SIZE(msgs)) {
+   dev_err(>dev, "read error, ret=%d\n", ret);
+   rv = -EIO;
+   goto out;
+   }
+



+   dev_dbg(>dev,
+   "raw data is sec=%02x, min=%02x, hr=%02x, mday=%02x, mon=%02x, 
year=%02x, wday=%02x, y2k=%02x\n",
+   ccr[0], ccr[1], ccr[2], ccr[3],
+   ccr[4], ccr[5], ccr[6], ccr[7]);


Ouch.



Not essential, I removed it.



+
+   tm->tm_sec = bcd2bin(ccr[0] & 0x7F);
+   tm->tm_min = bcd2bin(ccr[1] & 0x7F);
+   if (ccr[2] & ISL12026_HR_MIL)
+   tm->tm_hour = bcd2bin(ccr[2] & 0x3F);
+   else
+   tm->tm_hour = bcd2bin(ccr[2] & 0x1F) +
+   ((ccr[2] & 0x20) ? 12 : 0);
+   tm->tm_mday = bcd2bin(ccr[3] & 0x3F);
+   tm->tm_mon = bcd2bin(ccr[4] & 0x1F) - 1;
+   tm->tm_year = bcd2bin(ccr[5]);
+   if (bcd2bin(ccr[7]) == 20)
+   tm->tm_year += 100;
+   tm->tm_wday = ccr[6] & 0x07;
+
+   dev_dbg(>dev, "secs=%d, mins=%d, hours=%d, mday=%d, mon=%d, year=%d, 
wday=%d\n",
+   tm->tm_sec, tm->tm_min, tm->tm_hour,
+   tm->tm_mday, tm->tm_mon, tm->tm_year, tm->tm_wday);
+   rv = rtc_valid_tm(tm);



+out:
+   mutex_unlock(>lock);


Here one pattern, below another...

Hey, you have to learn being consistent.


I made them all like this one.  Single function exit point, with the 
unlock immediately before the return.






+   return rv;
+}
+
+static const struct rtc_class_ops isl12026_rtc_ops = {
+   .read_time  = isl12026_rtc_read_time,
+   .set_time   = isl12026_rtc_set_time,
+};
+
+static int isl12026_nvm_read(void *p, unsigned int offset,
+void *val, size_t bytes)
+{
+   struct isl12026 *priv = p;
+   int r;
+   u8 addr[2];
+   struct i2c_msg msgs[] = {
+   {
+   .addr   = priv->nvm_client->addr,
+   .flags  = 0,
+   .len=

Re: [PATCH] rtc: isl12026: Add driver.

2018-02-13 Thread Andy Shevchenko 
On Mon, Feb 12, 2018 at 8:59 PM, David Daney  wrote:
> The ISL12026 is a combination RTC and EEPROM device with I2C
> interface.  The standard RTC driver interface is provided.  The EEPROM
> is accessed via the NVMEM interface via the "eeprom0" directory in the
> sysfs entry for the device.
>
> Signed-off-by: David Daney 
> ---
>  .../devicetree/bindings/rtc/isil,isl12026.txt  |  27 +
>  drivers/rtc/Kconfig|   9 +
>  drivers/rtc/Makefile   |   1 +
>  drivers/rtc/rtc-isl12026.c | 550 
> +
>  4 files changed, 587 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/rtc/isil,isl12026.txt
>  create mode 100644 drivers/rtc/rtc-isl12026.c
>
> diff --git a/Documentation/devicetree/bindings/rtc/isil,isl12026.txt 
> b/Documentation/devicetree/bindings/rtc/isil,isl12026.txt
> new file mode 100644
> index ..4b6c7177a95a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/rtc/isil,isl12026.txt
> @@ -0,0 +1,27 @@
> +ISL12026 I2C RTC/EEPROM
> +
> +ISL12026 is an I2C RTC/EEPROM combination device.  The RTC and control
> +registers respond at bus address 0x6f, and the EEPROM array responds
> +at bus address 0x57.  The canonical "reg" value will be for the RTC portion.
> +
> +Required properties supported by the device:
> +
> + - "compatible": must be "isil,isl12026"
> + - "reg": I2C bus address of the device (always 0x6f)
> +
> +Optional properties:
> +
> + - "isil,pwr-bsw": If present PWR.BSW bit must be set to the specified
> +   value for proper operation.
> +
> + - "isil,pwr-sbib": If present PWR.SBIB bit must be set to the specified
> +value for proper operation.
> +
> +
> +Example:
> +
> +   rtc@6f {
> +   reg = <0x6f>;
> +   isil,pwr-bsw = <0>;
> +   isil,pwr-sbib = <1>;
> +   }
> diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
> index 8ab5f0a5d323..85171e9e3ada 100644
> --- a/drivers/rtc/Kconfig
> +++ b/drivers/rtc/Kconfig
> @@ -407,6 +407,15 @@ config RTC_DRV_ISL12022
>   This driver can also be built as a module. If so, the module
>   will be called rtc-isl12022.
>
> +config RTC_DRV_ISL12026
> +   tristate "Intersil ISL12026"
> +   help
> + If you say yes here you get support for the
> + Intersil ISL12026 RTC chip.
> +
> + This driver can also be built as a module. If so, the module
> + will be called rtc-isl12026.
> +
>  config RTC_DRV_X1205
> tristate "Xicor/Intersil X1205"
> help
> diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
> index 4fbf87e45a7c..f481661a6eae 100644
> --- a/drivers/rtc/Makefile
> +++ b/drivers/rtc/Makefile
> @@ -76,6 +76,7 @@ obj-$(CONFIG_RTC_DRV_HID_SENSOR_TIME) += 
> rtc-hid-sensor-time.o
>  obj-$(CONFIG_RTC_DRV_HYM8563)  += rtc-hym8563.o
>  obj-$(CONFIG_RTC_DRV_IMXDI)+= rtc-imxdi.o
>  obj-$(CONFIG_RTC_DRV_ISL12022) += rtc-isl12022.o
> +obj-$(CONFIG_RTC_DRV_ISL12026) += rtc-isl12026.o
>  obj-$(CONFIG_RTC_DRV_ISL1208)  += rtc-isl1208.o
>  obj-$(CONFIG_RTC_DRV_JZ4740)   += rtc-jz4740.o
>  obj-$(CONFIG_RTC_DRV_LP8788)   += rtc-lp8788.o
> diff --git a/drivers/rtc/rtc-isl12026.c b/drivers/rtc/rtc-isl12026.c
> new file mode 100644
> index ..a5f04e0faceb
> --- /dev/null
> +++ b/drivers/rtc/rtc-isl12026.c
> @@ -0,0 +1,550 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * An I2C driver for the Intersil ISL 12026
> + *
> + * Copyright (c) 2018 Cavium, Inc.
> + */

> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 

Perhaps in alphabetical order.

> +/* register offsets */
> +#define ISL12026_REG_SC0x30
> +
> +#define ISL12026_REG_SR0x3f
> +# define ISL12026_REG_SR_RTCF  BIT(0)
> +# define ISL12026_REG_SR_WEL   BIT(1)
> +# define ISL12026_REG_SR_RWEL  BIT(2)
> +# define ISL12026_REG_SR_MBZ   BIT(3)
> +# define ISL12026_REG_SR_OSCF  BIT(4)
> +
> +/* ISL register bits */
> +#define ISL12026_HR_MILBIT(7)  /* military or 24 hour time */
> +

> +#define ISL12026_REG_PWR   0x14

Perhaps keep it ordered? (0x14 < 0x30 obviously)

> +# define ISL12026_REG_PWR_BSW  BIT(6)
> +# define ISL12026_REG_PWR_SBIB BIT(7)

> +#define ISL12026_PAGESIZE 16
> +#define ISL12026_NVMEM_WRITE_TIME 20
> +
> +struct isl12026 {
> +   struct rtc_device *rtc;
> +   struct i2c_client *nvm_client;
> +   struct nvmem_device *nvm_dev;
> +   struct nvmem_config nvm_cfg;
> +   /*
> +* RTC write operations require that multiple messages be
> +* transmitted, we hold the lock for all accesses to the
> +* device so that these sequences cannot be disrupted.  Also,
> +* the write cycle to the nvmem takes many mS during which the
> +* device does not respond to

Re: [PATCH] rtc: isl12026: Add driver.

2018-02-13 Thread Andy Shevchenko 
On Mon, Feb 12, 2018 at 8:59 PM, David Daney  wrote:
> The ISL12026 is a combination RTC and EEPROM device with I2C
> interface.  The standard RTC driver interface is provided.  The EEPROM
> is accessed via the NVMEM interface via the "eeprom0" directory in the
> sysfs entry for the device.
>
> Signed-off-by: David Daney 
> ---
>  .../devicetree/bindings/rtc/isil,isl12026.txt  |  27 +
>  drivers/rtc/Kconfig|   9 +
>  drivers/rtc/Makefile   |   1 +
>  drivers/rtc/rtc-isl12026.c | 550 
> +
>  4 files changed, 587 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/rtc/isil,isl12026.txt
>  create mode 100644 drivers/rtc/rtc-isl12026.c
>
> diff --git a/Documentation/devicetree/bindings/rtc/isil,isl12026.txt 
> b/Documentation/devicetree/bindings/rtc/isil,isl12026.txt
> new file mode 100644
> index ..4b6c7177a95a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/rtc/isil,isl12026.txt
> @@ -0,0 +1,27 @@
> +ISL12026 I2C RTC/EEPROM
> +
> +ISL12026 is an I2C RTC/EEPROM combination device.  The RTC and control
> +registers respond at bus address 0x6f, and the EEPROM array responds
> +at bus address 0x57.  The canonical "reg" value will be for the RTC portion.
> +
> +Required properties supported by the device:
> +
> + - "compatible": must be "isil,isl12026"
> + - "reg": I2C bus address of the device (always 0x6f)
> +
> +Optional properties:
> +
> + - "isil,pwr-bsw": If present PWR.BSW bit must be set to the specified
> +   value for proper operation.
> +
> + - "isil,pwr-sbib": If present PWR.SBIB bit must be set to the specified
> +value for proper operation.
> +
> +
> +Example:
> +
> +   rtc@6f {
> +   reg = <0x6f>;
> +   isil,pwr-bsw = <0>;
> +   isil,pwr-sbib = <1>;
> +   }
> diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
> index 8ab5f0a5d323..85171e9e3ada 100644
> --- a/drivers/rtc/Kconfig
> +++ b/drivers/rtc/Kconfig
> @@ -407,6 +407,15 @@ config RTC_DRV_ISL12022
>   This driver can also be built as a module. If so, the module
>   will be called rtc-isl12022.
>
> +config RTC_DRV_ISL12026
> +   tristate "Intersil ISL12026"
> +   help
> + If you say yes here you get support for the
> + Intersil ISL12026 RTC chip.
> +
> + This driver can also be built as a module. If so, the module
> + will be called rtc-isl12026.
> +
>  config RTC_DRV_X1205
> tristate "Xicor/Intersil X1205"
> help
> diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
> index 4fbf87e45a7c..f481661a6eae 100644
> --- a/drivers/rtc/Makefile
> +++ b/drivers/rtc/Makefile
> @@ -76,6 +76,7 @@ obj-$(CONFIG_RTC_DRV_HID_SENSOR_TIME) += 
> rtc-hid-sensor-time.o
>  obj-$(CONFIG_RTC_DRV_HYM8563)  += rtc-hym8563.o
>  obj-$(CONFIG_RTC_DRV_IMXDI)+= rtc-imxdi.o
>  obj-$(CONFIG_RTC_DRV_ISL12022) += rtc-isl12022.o
> +obj-$(CONFIG_RTC_DRV_ISL12026) += rtc-isl12026.o
>  obj-$(CONFIG_RTC_DRV_ISL1208)  += rtc-isl1208.o
>  obj-$(CONFIG_RTC_DRV_JZ4740)   += rtc-jz4740.o
>  obj-$(CONFIG_RTC_DRV_LP8788)   += rtc-lp8788.o
> diff --git a/drivers/rtc/rtc-isl12026.c b/drivers/rtc/rtc-isl12026.c
> new file mode 100644
> index ..a5f04e0faceb
> --- /dev/null
> +++ b/drivers/rtc/rtc-isl12026.c
> @@ -0,0 +1,550 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * An I2C driver for the Intersil ISL 12026
> + *
> + * Copyright (c) 2018 Cavium, Inc.
> + */

> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 

Perhaps in alphabetical order.

> +/* register offsets */
> +#define ISL12026_REG_SC0x30
> +
> +#define ISL12026_REG_SR0x3f
> +# define ISL12026_REG_SR_RTCF  BIT(0)
> +# define ISL12026_REG_SR_WEL   BIT(1)
> +# define ISL12026_REG_SR_RWEL  BIT(2)
> +# define ISL12026_REG_SR_MBZ   BIT(3)
> +# define ISL12026_REG_SR_OSCF  BIT(4)
> +
> +/* ISL register bits */
> +#define ISL12026_HR_MILBIT(7)  /* military or 24 hour time */
> +

> +#define ISL12026_REG_PWR   0x14

Perhaps keep it ordered? (0x14 < 0x30 obviously)

> +# define ISL12026_REG_PWR_BSW  BIT(6)
> +# define ISL12026_REG_PWR_SBIB BIT(7)

> +#define ISL12026_PAGESIZE 16
> +#define ISL12026_NVMEM_WRITE_TIME 20
> +
> +struct isl12026 {
> +   struct rtc_device *rtc;
> +   struct i2c_client *nvm_client;
> +   struct nvmem_device *nvm_dev;
> +   struct nvmem_config nvm_cfg;
> +   /*
> +* RTC write operations require that multiple messages be
> +* transmitted, we hold the lock for all accesses to the
> +* device so that these sequences cannot be disrupted.  Also,
> +* the write cycle to the nvmem takes many mS during which the
> +* device does not respond to commands, so holding the lock
> +* also

Re: [PATCH] rtc: isl12026: Add driver.

2018-02-12 Thread David Daney 

On 02/12/2018 10:59 AM, David Daney wrote:

The ISL12026 is a combination RTC and EEPROM device with I2C
interface.  The standard RTC driver interface is provided.  The EEPROM
is accessed via the NVMEM interface via the "eeprom0" directory in the
sysfs entry for the device.

Signed-off-by: David Daney 
---
  .../devicetree/bindings/rtc/isil,isl12026.txt  |  27 +
  drivers/rtc/Kconfig|   9 +
  drivers/rtc/Makefile   |   1 +
  drivers/rtc/rtc-isl12026.c | 550 +
  4 files changed, 587 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/rtc/isil,isl12026.txt
  create mode 100644 drivers/rtc/rtc-isl12026.c

diff --git a/Documentation/devicetree/bindings/rtc/isil,isl12026.txt 
b/Documentation/devicetree/bindings/rtc/isil,isl12026.txt
new file mode 100644
index ..4b6c7177a95a
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/isil,isl12026.txt
@@ -0,0 +1,27 @@
+ISL12026 I2C RTC/EEPROM
+
+ISL12026 is an I2C RTC/EEPROM combination device.  The RTC and control
+registers respond at bus address 0x6f, and the EEPROM array responds
+at bus address 0x57.  The canonical "reg" value will be for the RTC portion.
+
+Required properties supported by the device:
+
+ - "compatible": must be "isil,isl12026"
+ - "reg": I2C bus address of the device (always 0x6f)
+
+Optional properties:
+
+ - "isil,pwr-bsw": If present PWR.BSW bit must be set to the specified
+   value for proper operation.
+
+ - "isil,pwr-sbib": If present PWR.SBIB bit must be set to the specified
+value for proper operation.
+
+
+Example:
+
+   rtc@6f {


Two seconds after sending, I see that the compatible went missing.

I will wait several days for feedback and resubmit with the proper 
compatible


Sorry for the snafu,
David Daney



+   reg = <0x6f>;
+   isil,pwr-bsw = <0>;
+   isil,pwr-sbib = <1>;
+   }

Re: [PATCH] rtc: isl12026: Add driver.

2018-02-12 Thread David Daney 

On 02/12/2018 10:59 AM, David Daney wrote:

The ISL12026 is a combination RTC and EEPROM device with I2C
interface.  The standard RTC driver interface is provided.  The EEPROM
is accessed via the NVMEM interface via the "eeprom0" directory in the
sysfs entry for the device.

Signed-off-by: David Daney 
---
  .../devicetree/bindings/rtc/isil,isl12026.txt  |  27 +
  drivers/rtc/Kconfig|   9 +
  drivers/rtc/Makefile   |   1 +
  drivers/rtc/rtc-isl12026.c | 550 +
  4 files changed, 587 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/rtc/isil,isl12026.txt
  create mode 100644 drivers/rtc/rtc-isl12026.c

diff --git a/Documentation/devicetree/bindings/rtc/isil,isl12026.txt 
b/Documentation/devicetree/bindings/rtc/isil,isl12026.txt
new file mode 100644
index ..4b6c7177a95a
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/isil,isl12026.txt
@@ -0,0 +1,27 @@
+ISL12026 I2C RTC/EEPROM
+
+ISL12026 is an I2C RTC/EEPROM combination device.  The RTC and control
+registers respond at bus address 0x6f, and the EEPROM array responds
+at bus address 0x57.  The canonical "reg" value will be for the RTC portion.
+
+Required properties supported by the device:
+
+ - "compatible": must be "isil,isl12026"
+ - "reg": I2C bus address of the device (always 0x6f)
+
+Optional properties:
+
+ - "isil,pwr-bsw": If present PWR.BSW bit must be set to the specified
+   value for proper operation.
+
+ - "isil,pwr-sbib": If present PWR.SBIB bit must be set to the specified
+value for proper operation.
+
+
+Example:
+
+   rtc@6f {


Two seconds after sending, I see that the compatible went missing.

I will wait several days for feedback and resubmit with the proper 
compatible


Sorry for the snafu,
David Daney



+   reg = <0x6f>;
+   isil,pwr-bsw = <0>;
+   isil,pwr-sbib = <1>;
+   }