[PATCH 1/4] dt-bindings: define vendor prefix for whwave, Inc.

[PATCH 1/4] dt-bindings: define vendor prefix for whwave, Inc.

[zoro]

Introduce vendor prefix for whwave, Inc.
for SD3078 rtc device.

Signed-off-by: zoro <long17.cool@163.com>
---
 .../devicetree/bindings/vendor-prefixes.txt        |    1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index 4b1a2a8..7d2e9b01 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -423,6 +423,7 @@ vot	Vision Optical Technology Co., Ltd.
 wd	Western Digital Corp.
 wetek	WeTek Electronics, limited.
 wexler	Wexler
+whwave	Shenzhen whwave Electronics crop
 wi2wi	Wi2Wi, Inc.
 winbond Winbond Electronics corp.
 winstar	Winstar Display Corp.
-- 
1.7.9.5

[PATCH 2/4] rtc: sd3078: new driver.

[zoro]

The sd3078 is a combination RTC and SRAM device with I2C interface.

Signed-off-by: zoro <long17.cool@163.com>
---
 MAINTAINERS              |    6 ++
 drivers/rtc/Kconfig      |    9 ++
 drivers/rtc/Makefile     |    1 +
 drivers/rtc/rtc-sd3078.c |  235 ++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 251 insertions(+)
 create mode 100644 drivers/rtc/rtc-sd3078.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 2f3eba4..002fcd7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16125,6 +16125,12 @@ L:	linux-gpio@vger.kernel.org
 S:	Maintained
 F:	drivers/gpio/gpio-wcove.c
 
+WHWAVE RTC DRIVER
+M:	Zoro Li <long17.cool@163.com>
+L:	linux-rtc@vger.kernel.org
+S:	Maintained
+F:	drivers/rtc/rtc-sd3078.c
+
 WIIMOTE HID DRIVER
 M:	David Herrmann <dh.herrmann@googlemail.com>
 L:	linux-input@vger.kernel.org
diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index a819ef0..5b4b1b4 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -646,6 +646,15 @@ config RTC_DRV_S5M
 	  This driver can also be built as a module. If so, the module
 	  will be called rtc-s5m.
 
+config RTC_DRV_SD3078
+    tristate "ZXW Crystal SD3078"
+    help
+      If you say yes here you get support for the ZXW Crystal
+      SD3078 RTC chips.
+
+      This driver can also be built as a module. If so, the module
+      will be called rtc-sd3078.
+
 endif # I2C
 
 comment "SPI RTC drivers"
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index 290c173..f973bd0 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -148,6 +148,7 @@ obj-$(CONFIG_RTC_DRV_S3C)	+= rtc-s3c.o
 obj-$(CONFIG_RTC_DRV_S5M)	+= rtc-s5m.o
 obj-$(CONFIG_RTC_DRV_SA1100)	+= rtc-sa1100.o
 obj-$(CONFIG_RTC_DRV_SC27XX)	+= rtc-sc27xx.o
+obj-$(CONFIG_RTC_DRV_SD3078)	+= rtc-sd3078.o
 obj-$(CONFIG_RTC_DRV_SH)	+= rtc-sh.o
 obj-$(CONFIG_RTC_DRV_SIRFSOC)	+= rtc-sirfsoc.o
 obj-$(CONFIG_RTC_DRV_SNVS)	+= rtc-snvs.o
diff --git a/drivers/rtc/rtc-sd3078.c b/drivers/rtc/rtc-sd3078.c
new file mode 100644
index 0000000..97e8f4b
--- /dev/null
+++ b/drivers/rtc/rtc-sd3078.c
@@ -0,0 +1,235 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Real Time Clock (RTC) Driver for sd3078
+ * Copyright (C) 2018 Zoro Li
+ */
+
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/bcd.h>
+#include <linux/rtc.h>
+#include <linux/slab.h>
+#include <linux/regmap.h>
+
+#define SD3078_REG_SC			0x00
+#define SD3078_REG_MN			0x01
+#define SD3078_REG_HR			0x02
+#define SD3078_REG_DW			0x03
+#define SD3078_REG_DM			0x04
+#define SD3078_REG_MO			0x05
+#define SD3078_REG_YR			0x06
+
+#define SD3078_REG_CTRL1		0x0f
+#define SD3078_REG_CTRL2		0x10
+#define SD3078_REG_CTRL3		0x11
+
+#define KEY_WRITE1		0x80
+#define KEY_WRITE2		0x04
+#define KEY_WRITE3		0x80
+
+#define NUM_TIME_REGS   (SD3078_REG_YR - SD3078_REG_SC + 1)
+
+/*
+ * The sd3078 has write protection
+ * and we can choose whether or not to use it.
+ * Write protection is turned off by default.
+ */
+#define WRITE_PROTECT_EN	0
+
+struct sd3078 {
+	struct rtc_device	*rtc;
+	struct regmap		*regmap;
+};
+
+/*
+ * In order to prevent arbitrary modification of the time register,
+ * when modification of the register,
+ * the "write" bit needs to be written in a certain order.
+ * 1. set WRITE1 bit
+ * 2. set WRITE2 bit
+ * 3. set WRITE3 bit
+ */
+static void sd3078_enable_reg_write(struct sd3078 *sd3078)
+{
+	regmap_update_bits(sd3078->regmap, SD3078_REG_CTRL2,
+		KEY_WRITE1, KEY_WRITE1);
+	regmap_update_bits(sd3078->regmap, SD3078_REG_CTRL1,
+		KEY_WRITE2, KEY_WRITE2);
+	regmap_update_bits(sd3078->regmap, SD3078_REG_CTRL1,
+		KEY_WRITE3, KEY_WRITE3);
+}
+
+#if WRITE_PROTECT_EN
+/*
+ * In order to prevent arbitrary modification of the time register,
+ * we should disable the write function.
+ * when disable write,
+ * the "write" bit needs to be clear in a certain order.
+ * 1. clear WRITE2 bit
+ * 2. clear WRITE3 bit
+ * 3. clear WRITE1 bit
+ */
+static void sd3078_disable_reg_write(struct sd3078 *sd3078)
+{
+	regmap_update_bits(sd3078->regmap, SD3078_REG_CTRL1,
+		KEY_WRITE2, 0);
+	regmap_update_bits(sd3078->regmap, SD3078_REG_CTRL1,
+		KEY_WRITE3, 0);
+	regmap_update_bits(sd3078->regmap, SD3078_REG_CTRL2,
+		KEY_WRITE1, 0);
+}
+#endif
+
+static int sd3078_rtc_read_time(struct device *dev, struct rtc_time *tm)
+{
+	unsigned char hour;
+	unsigned char rtc_data[NUM_TIME_REGS] = {0};
+	struct i2c_client *client = to_i2c_client(dev);
+	struct sd3078 *sd3078 = i2c_get_clientdata(client);
+	int ret;
+
+	ret = regmap_bulk_read(sd3078->regmap, SD3078_REG_SC, rtc_data,
+			NUM_TIME_REGS);
+	if (ret < 0) {
+		dev_err(dev, "reading from RTC failed with err:%d\n", ret);
+		return ret;
+	}
+
+	tm->tm_sec	= bcd2bin(rtc_data[SD3078_REG_SC] & 0x7F);
+	tm->tm_min	= bcd2bin(rtc_data[SD3078_REG_MN] & 0x7F);
+
+	/*
+	 * The sd3078 supports 12/24 hour mode.
+	 * When getting time,
+	 * we need to convert the 12 hour mode to the 24 hour mode.
+	 */
+	hour = rtc_data[SD3078_REG_HR];
+	if (hour & 0x80) /* 24H MODE */
+		tm->tm_hour = bcd2bin(rtc_data[SD3078_REG_HR] & 0x3F);
+	else if (hour & 0x20) /* 12H MODE PM */
+		tm->tm_hour = bcd2bin(rtc_data[SD3078_REG_HR] & 0x1F) + 12;
+	else /* 12H MODE AM */
+		tm->tm_hour = bcd2bin(rtc_data[SD3078_REG_HR] & 0x1F);
+
+	tm->tm_mday = bcd2bin(rtc_data[SD3078_REG_DM] & 0x3F);
+	tm->tm_wday = rtc_data[SD3078_REG_DW] & 0x07;
+	tm->tm_mon	= bcd2bin(rtc_data[SD3078_REG_MO] & 0x1F) - 1;
+	tm->tm_year = bcd2bin(rtc_data[SD3078_REG_YR])+100;
+
+	return 0;
+}
+
+static int sd3078_rtc_set_time(struct device *dev, struct rtc_time *tm)
+{
+	unsigned char rtc_data[NUM_TIME_REGS];
+	struct i2c_client *client = to_i2c_client(dev);
+	struct sd3078 *sd3078 = i2c_get_clientdata(client);
+	int ret;
+
+	rtc_data[SD3078_REG_SC] = bin2bcd(tm->tm_sec);
+	rtc_data[SD3078_REG_MN] = bin2bcd(tm->tm_min);
+	rtc_data[SD3078_REG_HR] = bin2bcd(tm->tm_hour) | 0x80;
+	rtc_data[SD3078_REG_DM] = bin2bcd(tm->tm_mday);
+	rtc_data[SD3078_REG_DW] = tm->tm_wday & 0x07;
+	rtc_data[SD3078_REG_MO] = bin2bcd(tm->tm_mon) + 1;
+	rtc_data[SD3078_REG_YR] = bin2bcd(tm->tm_year - 100);
+
+#if WRITE_PROTECT_EN
+	sd3078_enable_reg_write(sd3078);
+#endif
+
+	ret = regmap_bulk_write(sd3078->regmap, SD3078_REG_SC, rtc_data,
+			NUM_TIME_REGS);
+	if (ret < 0) {
+		dev_err(dev, "writing to RTC failed with err:%d\n", ret);
+		return ret;
+	}
+
+#if WRITE_PROTECT_EN
+	sd3078_disable_reg_write(sd3078);
+#endif
+
+	return 0;
+}
+
+static const struct rtc_class_ops sd3078_rtc_ops = {
+	.read_time	= sd3078_rtc_read_time,
+	.set_time	= sd3078_rtc_set_time,
+};
+
+static const struct regmap_config regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = 0x11,
+};
+
+static int sd3078_probe(struct i2c_client *client,
+		const struct i2c_device_id *id)
+{
+	int ret;
+	struct sd3078 *sd3078;
+
+
+	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
+		return -ENODEV;
+
+	sd3078 = devm_kzalloc(&client->dev, sizeof(*sd3078), GFP_KERNEL);
+	if (!sd3078)
+		return -ENOMEM;
+
+	sd3078->regmap = devm_regmap_init_i2c(client, &regmap_config);
+	if (IS_ERR(sd3078->regmap)) {
+		dev_err(&client->dev, "regmap allocation failed\n");
+		return PTR_ERR(sd3078->regmap);
+	}
+
+	i2c_set_clientdata(client, sd3078);
+
+	sd3078->rtc = devm_rtc_allocate_device(&client->dev);
+	if (IS_ERR(sd3078->rtc))
+		return PTR_ERR(sd3078->rtc);
+
+	sd3078->rtc->ops = &sd3078_rtc_ops;
+	sd3078->rtc->range_min = RTC_TIMESTAMP_BEGIN_2000;
+	sd3078->rtc->range_max = RTC_TIMESTAMP_END_2099;
+	sd3078->rtc->start_secs = RTC_TIMESTAMP_BEGIN_2000;
+	sd3078->rtc->set_start_time = true;
+
+	ret = rtc_register_device(sd3078->rtc);
+	if (ret) {
+		dev_err(&client->dev, "failed to register rtc device\n");
+		return ret;
+	}
+
+	sd3078_enable_reg_write(sd3078);
+
+	return 0;
+}
+
+static const struct i2c_device_id sd3078_id[] = {
+	{"sd3078", 0},
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, sd3078_id);
+
+static const struct of_device_id rtc_dt_match[] = {
+	{ .compatible = "whwave,sd3078" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, rtc_dt_match);
+
+struct i2c_driver sd3078_driver = {
+	.driver     = {
+		.name   = "sd3078",
+		.owner  = THIS_MODULE,
+		.of_match_table = of_match_ptr(rtc_dt_match),
+	},
+	.probe      = sd3078_probe,
+	.id_table   = sd3078_id,
+};
+
+module_i2c_driver(sd3078_driver);
+
+MODULE_AUTHOR("Zoro Li <long17.cool@163.com>");
+MODULE_DESCRIPTION("SD3078 RTC driver");
+MODULE_LICENSE("GPL v2");
-- 
1.7.9.5

Re: [PATCH 2/4] rtc: sd3078: new driver.

[Alexandre Belloni]

Hello,

A few things I missed in the previous review but this look mostly good.

On 12/11/2018 13:45:13+0800, zoro wrote:
> diff --git a/drivers/rtc/rtc-sd3078.c b/drivers/rtc/rtc-sd3078.c
> new file mode 100644
> index 0000000..97e8f4b
> --- /dev/null
> +++ b/drivers/rtc/rtc-sd3078.c
> @@ -0,0 +1,235 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Real Time Clock (RTC) Driver for sd3078
> + * Copyright (C) 2018 Zoro Li
> + */
> +
> +#include <linux/module.h>
> +#include <linux/i2c.h>
> +#include <linux/bcd.h>
> +#include <linux/rtc.h>
> +#include <linux/slab.h>
> +#include <linux/regmap.h>

The includes have to be sorted alphabetically.

> +
> +#define SD3078_REG_SC			0x00
> +#define SD3078_REG_MN			0x01
> +#define SD3078_REG_HR			0x02
> +#define SD3078_REG_DW			0x03
> +#define SD3078_REG_DM			0x04
> +#define SD3078_REG_MO			0x05
> +#define SD3078_REG_YR			0x06
> +
> +#define SD3078_REG_CTRL1		0x0f
> +#define SD3078_REG_CTRL2		0x10
> +#define SD3078_REG_CTRL3		0x11
> +
> +#define KEY_WRITE1		0x80
> +#define KEY_WRITE2		0x04
> +#define KEY_WRITE3		0x80
> +
> +#define NUM_TIME_REGS   (SD3078_REG_YR - SD3078_REG_SC + 1)
> +
> +/*
> + * The sd3078 has write protection
> + * and we can choose whether or not to use it.
> + * Write protection is turned off by default.
> + */
> +#define WRITE_PROTECT_EN	0
> +
> +struct sd3078 {
> +	struct rtc_device	*rtc;
> +	struct regmap		*regmap;
> +};
> +
> +/*
> + * In order to prevent arbitrary modification of the time register,
> + * when modification of the register,
> + * the "write" bit needs to be written in a certain order.
> + * 1. set WRITE1 bit
> + * 2. set WRITE2 bit
> + * 3. set WRITE3 bit
> + */
> +static void sd3078_enable_reg_write(struct sd3078 *sd3078)
> +{
> +	regmap_update_bits(sd3078->regmap, SD3078_REG_CTRL2,
> +		KEY_WRITE1, KEY_WRITE1);

This is not properly aligned. Please run checkpatch.pl --strict, you
have a bunch of lines that are not correctly aligned and a few
whitespace issues.

> +	regmap_update_bits(sd3078->regmap, SD3078_REG_CTRL1,
> +		KEY_WRITE2, KEY_WRITE2);
> +	regmap_update_bits(sd3078->regmap, SD3078_REG_CTRL1,
> +		KEY_WRITE3, KEY_WRITE3);
> +}

[..]

> +static int sd3078_probe(struct i2c_client *client,
> +		const struct i2c_device_id *id)
> +{
> +	int ret;
> +	struct sd3078 *sd3078;
> +
> +
> +	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
> +		return -ENODEV;
> +
> +	sd3078 = devm_kzalloc(&client->dev, sizeof(*sd3078), GFP_KERNEL);
> +	if (!sd3078)
> +		return -ENOMEM;
> +
> +	sd3078->regmap = devm_regmap_init_i2c(client, &regmap_config);
> +	if (IS_ERR(sd3078->regmap)) {
> +		dev_err(&client->dev, "regmap allocation failed\n");
> +		return PTR_ERR(sd3078->regmap);
> +	}
> +
> +	i2c_set_clientdata(client, sd3078);
> +
> +	sd3078->rtc = devm_rtc_allocate_device(&client->dev);
> +	if (IS_ERR(sd3078->rtc))
> +		return PTR_ERR(sd3078->rtc);
> +
> +	sd3078->rtc->ops = &sd3078_rtc_ops;
> +	sd3078->rtc->range_min = RTC_TIMESTAMP_BEGIN_2000;
> +	sd3078->rtc->range_max = RTC_TIMESTAMP_END_2099;
> +	sd3078->rtc->start_secs = RTC_TIMESTAMP_BEGIN_2000;
> +	sd3078->rtc->set_start_time = true;
> +

You mustn't set start_secs and set_start_time here as this match what
your hardware is doing. setting it from the driver is reserved for
legacy drivers.


-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[PATCH 3/4] dt-bindings: rtc: sd3078: add device tree documentation

[zoro]

The devicetree documentation for the SD3078 device tree
binding is added with a short example.

Signed-off-by: zoro <long17.cool@163.com>
---
 .../devicetree/bindings/rtc/rtc-sd3078.txt         |   15 +++++++++++++++
 MAINTAINERS                                        |    1 +
 2 files changed, 16 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/rtc/rtc-sd3078.txt

diff --git a/Documentation/devicetree/bindings/rtc/rtc-sd3078.txt b/Documentation/devicetree/bindings/rtc/rtc-sd3078.txt
new file mode 100644
index 0000000..9b45c8e
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/rtc-sd3078.txt
@@ -0,0 +1,15 @@
+SD3078 Real Time Clock
+============================
+
+Required properties:
+- compatible: Should contain "whwave,sd3078".
+- reg: I2C address for chip.
+
+Example:
+
+sd3078: sd3078@32 {
+       compatible = "whwave,sd3078";
+       reg = <0x32>;
+};
+
+
diff --git a/MAINTAINERS b/MAINTAINERS
index 002fcd7..50c038d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16130,6 +16130,7 @@ M:	Zoro Li <long17.cool@163.com>
 L:	linux-rtc@vger.kernel.org
 S:	Maintained
 F:	drivers/rtc/rtc-sd3078.c
+F:	Documentation/devicetree/bindings/rtc/rtc-sd3078.txt
 
 WIIMOTE HID DRIVER
 M:	David Herrmann <dh.herrmann@googlemail.com>
-- 
1.7.9.5

Re: [PATCH 3/4] dt-bindings: rtc: sd3078: add device tree documentation

[Alexandre Belloni]

This patch should come as 2/4 (BTW, I never got 4/4)

On 12/11/2018 13:45:14+0800, zoro wrote:
> The devicetree documentation for the SD3078 device tree
> binding is added with a short example.
> 
> Signed-off-by: zoro <long17.cool@163.com>
> ---
>  .../devicetree/bindings/rtc/rtc-sd3078.txt         |   15 +++++++++++++++
>  MAINTAINERS                                        |    1 +
>  2 files changed, 16 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/rtc/rtc-sd3078.txt
> 
> diff --git a/Documentation/devicetree/bindings/rtc/rtc-sd3078.txt b/Documentation/devicetree/bindings/rtc/rtc-sd3078.txt
> new file mode 100644
> index 0000000..9b45c8e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/rtc/rtc-sd3078.txt
> @@ -0,0 +1,15 @@
> +SD3078 Real Time Clock
> +============================
> +
> +Required properties:
> +- compatible: Should contain "whwave,sd3078".
> +- reg: I2C address for chip.
> +
> +Example:
> +
> +sd3078: sd3078@32 {
> +       compatible = "whwave,sd3078";
> +       reg = <0x32>;
> +};
> +
> +
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 002fcd7..50c038d 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -16130,6 +16130,7 @@ M:	Zoro Li <long17.cool@163.com>
>  L:	linux-rtc@vger.kernel.org
>  S:	Maintained
>  F:	drivers/rtc/rtc-sd3078.c
> +F:	Documentation/devicetree/bindings/rtc/rtc-sd3078.txt
>  

There is no need to list this file here if it is only done to silence
checkpatch.

>  WIIMOTE HID DRIVER
>  M:	David Herrmann <dh.herrmann@googlemail.com>
> -- 
> 1.7.9.5
> 
> 

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: Re: [PATCH 3/4] dt-bindings: rtc: sd3078: add device tree documentation

[Alexandre Belloni]

On 13/11/2018 00:55:49+0800, long17.cool@163.com wrote:
> hello：
> The patch of 4/4 is not used for this modules,  this is my problem.
> 
> I have a question : 
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -16130,6 +16130,7 @@ M: Zoro Li <long17.cool@163.com>
> >  L: linux-rtc@vger.kernel.org
> >  S: Maintained
> >  F: drivers/rtc/rtc-sd3078.c
> > +F: Documentation/devicetree/bindings/rtc/rtc-sd3078.txt
> > 
> 
> There is no need to list this file here if it is only done to silence checkpatch.
> What does it mean?
> 
> I found that if I didn't add that statement, when I checkpatch,
>  I would get the following warning.
> WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
> #16: 
> new file mode 100644
> Can I ignore this warning?
> 

Yes, you can.


-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com