Introduce vendor prefix for whwave, Inc.
for SD3078 rtc device.
Signed-off-by: zoro <[email protected]>
---
.../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
The devicetree documentation for the SD3078 device tree
binding is added with a short example.
Signed-off-by: zoro <[email protected]>
---
.../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 <[email protected]>
L: [email protected]
S: Maintained
F: drivers/rtc/rtc-sd3078.c
+F: Documentation/devicetree/bindings/rtc/rtc-sd3078.txt
WIIMOTE HID DRIVER
M: David Herrmann <[email protected]>
--
1.7.9.5
The sd3078 is a combination RTC and SRAM device with I2C interface.
Signed-off-by: zoro <[email protected]>
---
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: [email protected]
S: Maintained
F: drivers/gpio/gpio-wcove.c
+WHWAVE RTC DRIVER
+M: Zoro Li <[email protected]>
+L: [email protected]
+S: Maintained
+F: drivers/rtc/rtc-sd3078.c
+
WIIMOTE HID DRIVER
M: David Herrmann <[email protected]>
L: [email protected]
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, ®map_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 <[email protected]>");
+MODULE_DESCRIPTION("SD3078 RTC driver");
+MODULE_LICENSE("GPL v2");
--
1.7.9.5
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, ®map_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
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 <[email protected]>
> ---
> .../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 <[email protected]>
> L: [email protected]
> 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 <[email protected]>
> --
> 1.7.9.5
>
>
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
On 13/11/2018 00:55:49+0800, [email protected] 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 <[email protected]>
> > L: [email protected]
> > 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