[PATCH v2 0/3] Add RTC for MStar SSD20xD SoCs

[PATCH v2 0/3] Add RTC for MStar SSD20xD SoCs

[Romain Perier]

This patches series adds a new driver for the RTC found in the Mstar
SSD202D SoCs. It adds a basic rtc driver, the corresponding devicetree
bindings.

The rtctest (from selftests) has been passed on this driver, with the
following output:

# rtctest 
TAP version 13
1..8
# Starting 8 tests from 1 test cases.
#  RUN           rtc.date_read ...
# rtctest.c:52:date_read:Current RTC date/time is 17/05/2023 15:58:12.
#            OK  rtc.date_read
ok 1 rtc.date_read
#  RUN           rtc.date_read_loop ...
# rtctest.c:95:date_read_loop:Continuously reading RTC time for 30s (with 11ms breaks after every read).
# rtctest.c:122:date_read_loop:Performed 888 RTC time reads.
#            OK  rtc.date_read_loop
ok 2 rtc.date_read_loop
#  RUN           rtc.uie_read ...
# rtctest.c:137:uie_read:skip update IRQs not supported.
#            OK  rtc.uie_read
ok 3 rtc.uie_read
#  RUN           rtc.uie_select ...
# rtctest.c:166:uie_select:skip update IRQs not supported.
#            OK  rtc.uie_select
ok 4 rtc.uie_select
#  RUN           rtc.alarm_alm_set ...
# rtctest.c:214:alarm_alm_set:skip alarms are not supported.
#            OK  rtc.alarm_alm_set
ok 5 rtc.alarm_alm_set
#  RUN           rtc.alarm_wkalm_set ...
# rtctest.c:274:alarm_wkalm_set:skip alarms are not supported.
#            OK  rtc.alarm_wkalm_set
ok 6 rtc.alarm_wkalm_set
#  RUN           rtc.alarm_alm_set_minute ...
# rtctest.c:324:alarm_alm_set_minute:skip alarms are not supported.
#            OK  rtc.alarm_alm_set_minute
ok 7 rtc.alarm_alm_set_minute
#  RUN           rtc.alarm_wkalm_set_minute ...
# rtctest.c:384:alarm_wkalm_set_minute:skip alarms are not supported.
#            OK  rtc.alarm_wkalm_set_minute
ok 8 rtc.alarm_wkalm_set_minute
# PASSED: 8 / 8 tests passed.
# Totals: pass:8 fail:0 xfail:0 xpass:0 skip:0 error:0

Changes since v1:
- Changed the compatible from mstar,ssd20xd-rtc to mstar,ssd20d-rtc.
  So the driver, its documentation and the commit messages have been
  reworked accordingly.
- Re-worked the dt-binding, I have also simplified the commit message
- Re-worked the commit message for the driver
- Remove redundant logging message for the user in the driver, as
  requested by the subsystem maintainer. As these messages are helpful
  for debugging purpose, I have switched these to dev_dbg().
- Updated the list of maintainers (sorry for that).

Romain Perier (3):
  rtc: Add support for the SSD202D RTC
  dt-bindings: rtc: Add Mstar SSD202D RTC
  ARM: dts: mstar: Enable rtc for SSD202D

 .../bindings/rtc/mstar,ssd202d-rtc.yaml       |  35 +++
 .../boot/dts/sigmastar/mstar-infinity2m.dtsi  |   5 +
 drivers/rtc/Kconfig                           |  11 +
 drivers/rtc/Makefile                          |   1 +
 drivers/rtc/rtc-ssd202d.c                     | 249 ++++++++++++++++++
 5 files changed, 301 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/rtc/mstar,ssd202d-rtc.yaml
 create mode 100644 drivers/rtc/rtc-ssd202d.c

-- 
2.39.2

[PATCH v2 1/3] rtc: Add support for the SSD202D RTC

[Romain Perier]

Newer SigmaStar SSD202D SoCs contain a Real Time Clock, capable of
running while the system is sleeping (battery powered), this is not the
case with the other RTC on older SoCs. This adds basic support for this
RTC block.

Signed-off-by: Romain Perier <romain.perier@gmail.com>
Co-developed-by: Daniel Palmer <daniel@0x0f.com>
Signed-off-by: Daniel Palmer <daniel@0x0f.com>
---
 drivers/rtc/Kconfig       |  11 ++
 drivers/rtc/Makefile      |   1 +
 drivers/rtc/rtc-ssd202d.c | 249 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 261 insertions(+)
 create mode 100644 drivers/rtc/rtc-ssd202d.c

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index d7502433c78a..f6c49348d7aa 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -1984,4 +1984,15 @@ config RTC_DRV_POLARFIRE_SOC
 	  This driver can also be built as a module, if so, the module
 	  will be called "rtc-mpfs".
 
+config RTC_DRV_SSD202D
+	tristate "SigmaStar SSD202D RTC"
+	depends on ARCH_MSTARV7 || COMPILE_TEST
+	default ARCH_MSTARV7
+	help
+	  If you say yes here you get support for the SigmaStar SSD202D On-Chip
+	  Real Time Clock.
+
+	  This driver can also be built as a module, if so, the module
+	  will be called "rtc-ssd20xd".
+
 endif # RTC_CLASS
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index fd209883ee2e..7b03c3abfd78 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -103,6 +103,7 @@ obj-$(CONFIG_RTC_DRV_MESON)	+= rtc-meson.o
 obj-$(CONFIG_RTC_DRV_MOXART)	+= rtc-moxart.o
 obj-$(CONFIG_RTC_DRV_MPC5121)	+= rtc-mpc5121.o
 obj-$(CONFIG_RTC_DRV_MSC313)	+= rtc-msc313.o
+obj-$(CONFIG_RTC_DRV_SSD202D)	+= rtc-ssd202d.o
 obj-$(CONFIG_RTC_DRV_MSM6242)	+= rtc-msm6242.o
 obj-$(CONFIG_RTC_DRV_MT2712)	+= rtc-mt2712.o
 obj-$(CONFIG_RTC_DRV_MT6397)	+= rtc-mt6397.o
diff --git a/drivers/rtc/rtc-ssd202d.c b/drivers/rtc/rtc-ssd202d.c
new file mode 100644
index 000000000000..a69e062ad1b4
--- /dev/null
+++ b/drivers/rtc/rtc-ssd202d.c
@@ -0,0 +1,249 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Real time clocks driver for MStar/SigmaStar SSD202D SoCs.
+ *
+ * (C) 2021 Daniel Palmer
+ * (C) 2023 Romain Perier
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/platform_device.h>
+#include <linux/rtc.h>
+#include <linux/regmap.h>
+#include <linux/pm.h>
+
+#define REG_CTRL	0x0
+#define REG_CTRL1	0x4
+#define REG_ISO_CTRL	0xc
+#define REG_WRDATA_L	0x10
+#define REG_WRDATA_H	0x14
+#define REG_ISOACK	0x20
+#define REG_RDDATA_L	0x24
+#define REG_RDDATA_H	0x28
+#define REG_RDCNT_L	0x30
+#define REG_RDCNT_H	0x34
+#define REG_CNT_TRIG	0x38
+#define REG_PWRCTRL	0x3c
+#define REG_RTC_TEST	0x54
+
+#define CNT_RD_TRIG_BIT BIT(0)
+#define CNT_RD_BIT BIT(0)
+#define BASE_WR_BIT BIT(1)
+#define BASE_RD_BIT BIT(2)
+#define CNT_RST_BIT BIT(3)
+#define ISO_CTRL_ACK_MASK BIT(3)
+#define ISO_CTRL_ACK_SHIFT 3
+#define SW0_WR_BIT BIT(5)
+#define SW1_WR_BIT BIT(6)
+#define SW0_RD_BIT BIT(7)
+#define SW1_RD_BIT BIT(8)
+
+#define ISO_CTRL_MASK GENMASK(2, 0)
+
+struct ssd202d_rtc {
+	struct rtc_device *rtc_dev;
+	void __iomem *base;
+};
+
+static u8 read_iso_en(void __iomem *base)
+{
+	return readb(base + REG_RTC_TEST) & 0x1;
+}
+
+static u8 read_iso_ctrl_ack(void __iomem *base)
+{
+	return (readb(base + REG_ISOACK) & ISO_CTRL_ACK_MASK) >> ISO_CTRL_ACK_SHIFT;
+}
+
+static int ssd202d_rtc_isoctrl(struct ssd202d_rtc *priv)
+{
+	static const unsigned int sequence[] = { 0x0, 0x1, 0x3, 0x7, 0x5, 0x1, 0x0 };
+	unsigned int val;
+	struct device *dev = &priv->rtc_dev->dev;
+	int i, ret;
+
+	/*
+	 * This gates iso_en by writing a special sequence of bytes to iso_ctrl
+	 * and ensuring that it has been correctly applied by reading iso_ctrl_ack
+	 */
+	for (i = 0; i < ARRAY_SIZE(sequence); i++) {
+		writeb(sequence[i] & ISO_CTRL_MASK, priv->base +  REG_ISO_CTRL);
+
+		ret = read_poll_timeout(read_iso_ctrl_ack, val, val == (i % 2), 100,
+					20 * 100, true, priv->base);
+		if (ret) {
+			dev_dbg(dev, "Timeout waiting for ack byte %i (%x) of sequence\n", i,
+				sequence[i]);
+			return ret;
+		}
+	}
+
+	/*
+	 * At this point iso_en should be raised for 1ms
+	 */
+	ret = read_poll_timeout(read_iso_en, val, val, 100, 22 * 100, true, priv->base);
+	if (ret)
+		dev_dbg(dev, "Timeout waiting for iso_en\n");
+	mdelay(2);
+	return 0;
+}
+
+static void ssd202d_rtc_read_reg(struct ssd202d_rtc *priv, unsigned int reg,
+				 unsigned int field, unsigned int *base)
+{
+	unsigned int l, h;
+	u16 val;
+
+	/* Ask for the content of an RTC value into RDDATA by gating iso_en,
+	 * then iso_en is gated and the content of RDDATA can be read
+	 */
+	val = readw(priv->base + reg);
+	writew(val | field, priv->base + reg);
+	ssd202d_rtc_isoctrl(priv);
+	writew(val & ~field, priv->base + reg);
+
+	l = readw(priv->base + REG_RDDATA_L);
+	h = readw(priv->base + REG_RDDATA_H);
+
+	*base = (h << 16) | l;
+}
+
+static void ssd202d_rtc_write_reg(struct ssd202d_rtc *priv, unsigned int reg,
+				  unsigned int field, u32 base)
+{
+	u16 val;
+
+	/* Set the content of an RTC value from WRDATA by gating iso_en */
+	val = readw(priv->base + reg);
+	writew(val | field, priv->base + reg);
+	writew(base, priv->base + REG_WRDATA_L);
+	writew(base >> 16, priv->base + REG_WRDATA_H);
+	ssd202d_rtc_isoctrl(priv);
+	writew(val & ~field, priv->base + reg);
+}
+
+static int ssd202d_rtc_read_counter(struct ssd202d_rtc *priv, unsigned int *counter)
+{
+	unsigned int l, h;
+	u16 val;
+
+	val = readw(priv->base + REG_CTRL1);
+	writew(val | CNT_RD_BIT, priv->base + REG_CTRL1);
+	ssd202d_rtc_isoctrl(priv);
+	writew(val & ~CNT_RD_BIT, priv->base + REG_CTRL1);
+
+	val = readw(priv->base + REG_CTRL1);
+	writew(val | CNT_RD_TRIG_BIT, priv->base + REG_CNT_TRIG);
+	writew(val & ~CNT_RD_TRIG_BIT, priv->base + REG_CNT_TRIG);
+
+	l = readw(priv->base + REG_RDCNT_L);
+	h = readw(priv->base + REG_RDCNT_H);
+
+	*counter = (h << 16) | l;
+
+	return 0;
+}
+
+static int ssd202d_rtc_read_time(struct device *dev, struct rtc_time *tm)
+{
+	struct ssd202d_rtc *priv = dev_get_drvdata(dev);
+	unsigned int sw0, base, counter;
+	u32 seconds;
+	int ret;
+
+	/* Check that RTC is enabled by SW */
+	ssd202d_rtc_read_reg(priv, REG_CTRL, SW0_RD_BIT, &sw0);
+	if (sw0 != 1)
+		return -EINVAL;
+
+	/* Get RTC base value from RDDATA */
+	ssd202d_rtc_read_reg(priv, REG_CTRL, BASE_RD_BIT, &base);
+	/* Get RTC counter value from RDDATA */
+	ret = ssd202d_rtc_read_counter(priv, &counter);
+	if (ret)
+		return ret;
+
+	seconds = base + counter;
+
+	rtc_time64_to_tm(seconds, tm);
+
+	return 0;
+}
+
+static int ssd202d_rtc_reset_counter(struct ssd202d_rtc *priv)
+{
+	u16 val;
+
+	val = readw(priv->base + REG_CTRL);
+	writew(val | CNT_RST_BIT, priv->base + REG_CTRL);
+	ssd202d_rtc_isoctrl(priv);
+	writew(val & ~CNT_RST_BIT, priv->base + REG_CTRL);
+	ssd202d_rtc_isoctrl(priv);
+
+	return 0;
+}
+
+static int ssd202d_rtc_set_time(struct device *dev, struct rtc_time *tm)
+{
+	struct ssd202d_rtc *priv = dev_get_drvdata(dev);
+	unsigned long seconds = rtc_tm_to_time64(tm);
+
+	ssd202d_rtc_write_reg(priv, REG_CTRL, BASE_WR_BIT, seconds);
+	ssd202d_rtc_reset_counter(priv);
+	ssd202d_rtc_write_reg(priv, REG_CTRL, SW0_WR_BIT, 1);
+
+	return 0;
+}
+
+static const struct rtc_class_ops ssd202d_rtc_ops = {
+	.read_time = ssd202d_rtc_read_time,
+	.set_time = ssd202d_rtc_set_time,
+};
+
+static int ssd202d_rtc_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct ssd202d_rtc *priv;
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(struct ssd202d_rtc), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(priv->base))
+		return PTR_ERR(priv->base);
+
+	priv->rtc_dev = devm_rtc_allocate_device(dev);
+	if (IS_ERR(priv->rtc_dev))
+		return PTR_ERR(priv->rtc_dev);
+
+	priv->rtc_dev->ops = &ssd202d_rtc_ops;
+	priv->rtc_dev->range_max = U32_MAX;
+
+	platform_set_drvdata(pdev, priv);
+
+	return devm_rtc_register_device(priv->rtc_dev);
+}
+
+static const struct of_device_id ssd202d_rtc_of_match_table[] = {
+	{ .compatible = "mstar,ssd202d-rtc" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ssd202d_rtc_of_match_table);
+
+static struct platform_driver ssd202d_rtc_driver = {
+	.probe = ssd202d_rtc_probe,
+	.driver = {
+		.name = "ssd202d-rtc",
+		.of_match_table = ssd202d_rtc_of_match_table,
+	},
+};
+module_platform_driver(ssd202d_rtc_driver);
+
+MODULE_AUTHOR("Daniel Palmer <daniel@thingy.jp>");
+MODULE_AUTHOR("Romain Perier <romain.perier@gmail.com>");
+MODULE_DESCRIPTION("MStar SSD202D RTC Driver");
+MODULE_LICENSE("GPL v2");
-- 
2.39.2

[PATCH v2 2/3] dt-bindings: rtc: Add Mstar SSD202D RTC

[Romain Perier]

Add YAML bindings for Mstar SSD202D RTC.

Signed-off-by: Romain Perier <romain.perier@gmail.com>
---
 .../bindings/rtc/mstar,ssd202d-rtc.yaml       | 35 +++++++++++++++++++
 1 file changed, 35 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/rtc/mstar,ssd202d-rtc.yaml

diff --git a/Documentation/devicetree/bindings/rtc/mstar,ssd202d-rtc.yaml b/Documentation/devicetree/bindings/rtc/mstar,ssd202d-rtc.yaml
new file mode 100644
index 000000000000..4c1f22ef5a2c
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/mstar,ssd202d-rtc.yaml
@@ -0,0 +1,35 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/rtc/mstar,ssd202d-rtc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Mstar SSD202D Real Time Clock
+
+maintainers:
+  - Daniel Palmer <daniel@0x0f.com>
+  - Romain Perier <romain.perier@gmail.com>
+
+allOf:
+  - $ref: rtc.yaml#
+
+properties:
+  compatible:
+    enum:
+      - mstar,ssd202d-rtc
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    rtc@6800 {
+        compatible = "mstar,ssd202d-rtc";
+        reg = <0x6800 0x200>;
+    };
+...
-- 
2.39.2

[PATCH v2 3/3] ARM: dts: mstar: Enable rtc for SSD202D

[Romain Perier]

This adds the definition of the rtc device node. It enables RTC block
for SSD202D SoCs and newer.

Signed-off-by: Romain Perier <romain.perier@gmail.com>
---
 arch/arm/boot/dts/sigmastar/mstar-infinity2m.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/sigmastar/mstar-infinity2m.dtsi b/arch/arm/boot/dts/sigmastar/mstar-infinity2m.dtsi
index 1b485efd7156..b4ac535a73c8 100644
--- a/arch/arm/boot/dts/sigmastar/mstar-infinity2m.dtsi
+++ b/arch/arm/boot/dts/sigmastar/mstar-infinity2m.dtsi
@@ -32,6 +32,11 @@ cpu1: cpu@1 {
 };
 
 &riu {
+	rtc@6800 {
+		compatible = "mstar,ssd202d-rtc";
+		reg = <0x6800 0x200>;
+	};
+
 	smpctrl: smpctrl@204000 {
 		reg = <0x204000 0x200>;
 		status = "disabled";
-- 
2.39.2

Re: [PATCH v2 2/3] dt-bindings: rtc: Add Mstar SSD202D RTC

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1705 bytes --]

On Wed, Sep 13, 2023 at 05:16:05PM +0200, Romain Perier wrote:
> Add YAML bindings for Mstar SSD202D RTC.
> 
> Signed-off-by: Romain Perier <romain.perier@gmail.com>
> ---
>  .../bindings/rtc/mstar,ssd202d-rtc.yaml       | 35 +++++++++++++++++++
>  1 file changed, 35 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/rtc/mstar,ssd202d-rtc.yaml
> 
> diff --git a/Documentation/devicetree/bindings/rtc/mstar,ssd202d-rtc.yaml b/Documentation/devicetree/bindings/rtc/mstar,ssd202d-rtc.yaml
> new file mode 100644
> index 000000000000..4c1f22ef5a2c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/rtc/mstar,ssd202d-rtc.yaml
> @@ -0,0 +1,35 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/rtc/mstar,ssd202d-rtc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Mstar SSD202D Real Time Clock
> +
> +maintainers:
> +  - Daniel Palmer <daniel@0x0f.com>
> +  - Romain Perier <romain.perier@gmail.com>
> +
> +allOf:
> +  - $ref: rtc.yaml#
> +
> +properties:
> +  compatible:
> +    enum:
> +      - mstar,ssd202d-rtc
> +  reg:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg

So, this seems fine to me in isolation, but isn't this now the sort of
thing that can be documented in trivial-rtc.yaml?
Its only got compatible & reg, which seems to fit the bill for that.

Apologies if I am missing something,
Conor.

> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    rtc@6800 {
> +        compatible = "mstar,ssd202d-rtc";
> +        reg = <0x6800 0x200>;
> +    };
> +...
> -- 
> 2.39.2
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 2/3] dt-bindings: rtc: Add Mstar SSD202D RTC

[Romain Perier]

Le mer. 13 sept. 2023 à 20:30, Conor Dooley <conor@kernel.org> a écrit :
>
> On Wed, Sep 13, 2023 at 05:16:05PM +0200, Romain Perier wrote:
> > Add YAML bindings for Mstar SSD202D RTC.
> >
> > Signed-off-by: Romain Perier <romain.perier@gmail.com>
> > ---
> >  .../bindings/rtc/mstar,ssd202d-rtc.yaml       | 35 +++++++++++++++++++
> >  1 file changed, 35 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/rtc/mstar,ssd202d-rtc.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/rtc/mstar,ssd202d-rtc.yaml b/Documentation/devicetree/bindings/rtc/mstar,ssd202d-rtc.yaml
> > new file mode 100644
> > index 000000000000..4c1f22ef5a2c
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/rtc/mstar,ssd202d-rtc.yaml
> > @@ -0,0 +1,35 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/rtc/mstar,ssd202d-rtc.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Mstar SSD202D Real Time Clock
> > +
> > +maintainers:
> > +  - Daniel Palmer <daniel@0x0f.com>
> > +  - Romain Perier <romain.perier@gmail.com>
> > +
> > +allOf:
> > +  - $ref: rtc.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - mstar,ssd202d-rtc
> > +  reg:
> > +    maxItems: 1
> > +
> > +required:
> > +  - compatible
> > +  - reg
>

Hi,

> So, this seems fine to me in isolation, but isn't this now the sort of
> thing that can be documented in trivial-rtc.yaml?
> Its only got compatible & reg, which seems to fit the bill for that.
>

With the current state, it might make sense. However, currently, the
RTC hw block is mostly
reverse-engineered, the driver is not complete yet, things like
external irq or wakeup irq might arrive later (once we know how it is
wired up and used).
So the content of the dt-bindings might change and differ from a
simple "compatible & reg" requirement.

What do you think ?

> Apologies if I am missing something,

No problem, I am open to suggestions :)

Thanks,
Regards,
Romain



> Conor.
>
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > +  - |
> > +    rtc@6800 {
> > +        compatible = "mstar,ssd202d-rtc";
> > +        reg = <0x6800 0x200>;
> > +    };
> > +...
> > --
> > 2.39.2
> >

Re: [PATCH v2 2/3] dt-bindings: rtc: Add Mstar SSD202D RTC

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2379 bytes --]

On Thu, Sep 14, 2023 at 09:08:52AM +0200, Romain Perier wrote:
> Le mer. 13 sept. 2023 à 20:30, Conor Dooley <conor@kernel.org> a écrit :
> >
> > On Wed, Sep 13, 2023 at 05:16:05PM +0200, Romain Perier wrote:
> > > Add YAML bindings for Mstar SSD202D RTC.
> > >
> > > Signed-off-by: Romain Perier <romain.perier@gmail.com>
> > > ---
> > >  .../bindings/rtc/mstar,ssd202d-rtc.yaml       | 35 +++++++++++++++++++
> > >  1 file changed, 35 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/rtc/mstar,ssd202d-rtc.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/rtc/mstar,ssd202d-rtc.yaml b/Documentation/devicetree/bindings/rtc/mstar,ssd202d-rtc.yaml
> > > new file mode 100644
> > > index 000000000000..4c1f22ef5a2c
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/rtc/mstar,ssd202d-rtc.yaml
> > > @@ -0,0 +1,35 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/rtc/mstar,ssd202d-rtc.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Mstar SSD202D Real Time Clock
> > > +
> > > +maintainers:
> > > +  - Daniel Palmer <daniel@0x0f.com>
> > > +  - Romain Perier <romain.perier@gmail.com>
> > > +
> > > +allOf:
> > > +  - $ref: rtc.yaml#
> > > +
> > > +properties:
> > > +  compatible:
> > > +    enum:
> > > +      - mstar,ssd202d-rtc
> > > +  reg:
> > > +    maxItems: 1
> > > +
> > > +required:
> > > +  - compatible
> > > +  - reg
> >
> 
> Hi,
> 
> > So, this seems fine to me in isolation, but isn't this now the sort of
> > thing that can be documented in trivial-rtc.yaml?
> > Its only got compatible & reg, which seems to fit the bill for that.
> >
> 
> With the current state, it might make sense. However, currently, the
> RTC hw block is mostly
> reverse-engineered, the driver is not complete yet, things like
> external irq or wakeup irq might arrive later (once we know how it is
> wired up and used).
> So the content of the dt-bindings might change and differ from a
> simple "compatible & reg" requirement.

It's always possible to move from that to a fully fledged binding at a
later date. re: interrupts, trivial-rtc permits a single interrupt, so
it'd still be suitable if the device only has one.

Thanks,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 2/3] dt-bindings: rtc: Add Mstar SSD202D RTC

[Romain Perier]

Le jeu. 14 sept. 2023 à 10:28, Conor Dooley
<conor.dooley@microchip.com> a écrit :
>
> On Thu, Sep 14, 2023 at 09:08:52AM +0200, Romain Perier wrote:
> > Le mer. 13 sept. 2023 à 20:30, Conor Dooley <conor@kernel.org> a écrit :
> > >
> > > On Wed, Sep 13, 2023 at 05:16:05PM +0200, Romain Perier wrote:
> > > > Add YAML bindings for Mstar SSD202D RTC.
> > > >
> > > > Signed-off-by: Romain Perier <romain.perier@gmail.com>
> > > > ---
> > > >  .../bindings/rtc/mstar,ssd202d-rtc.yaml       | 35 +++++++++++++++++++
> > > >  1 file changed, 35 insertions(+)
> > > >  create mode 100644 Documentation/devicetree/bindings/rtc/mstar,ssd202d-rtc.yaml
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/rtc/mstar,ssd202d-rtc.yaml b/Documentation/devicetree/bindings/rtc/mstar,ssd202d-rtc.yaml
> > > > new file mode 100644
> > > > index 000000000000..4c1f22ef5a2c
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/rtc/mstar,ssd202d-rtc.yaml
> > > > @@ -0,0 +1,35 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/rtc/mstar,ssd202d-rtc.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Mstar SSD202D Real Time Clock
> > > > +
> > > > +maintainers:
> > > > +  - Daniel Palmer <daniel@0x0f.com>
> > > > +  - Romain Perier <romain.perier@gmail.com>
> > > > +
> > > > +allOf:
> > > > +  - $ref: rtc.yaml#
> > > > +
> > > > +properties:
> > > > +  compatible:
> > > > +    enum:
> > > > +      - mstar,ssd202d-rtc
> > > > +  reg:
> > > > +    maxItems: 1
> > > > +
> > > > +required:
> > > > +  - compatible
> > > > +  - reg
> > >
> >
> > Hi,
> >
> > > So, this seems fine to me in isolation, but isn't this now the sort of
> > > thing that can be documented in trivial-rtc.yaml?
> > > Its only got compatible & reg, which seems to fit the bill for that.
> > >
> >
> > With the current state, it might make sense. However, currently, the
> > RTC hw block is mostly
> > reverse-engineered, the driver is not complete yet, things like
> > external irq or wakeup irq might arrive later (once we know how it is
> > wired up and used).
> > So the content of the dt-bindings might change and differ from a
> > simple "compatible & reg" requirement.
>
> It's always possible to move from that to a fully fledged binding at a
> later date. re: interrupts, trivial-rtc permits a single interrupt, so
> it'd still be suitable if the device only has one.

I agree, good point. Yes, we can make the change later, sure. I will
fix it in the next patch series.

Thanks,
Romain

Re: [PATCH v2 1/3] rtc: Add support for the SSD202D RTC

[Alexandre Belloni]

Hello Romain,

Sorry for the very long delay,

On 13/09/2023 17:16:04+0200, Romain Perier wrote:
> +MODULE_AUTHOR("Daniel Palmer <daniel@thingy.jp>");
> +MODULE_AUTHOR("Romain Perier <romain.perier@gmail.com>");
> +MODULE_DESCRIPTION("MStar SSD202D RTC Driver");
> +MODULE_LICENSE("GPL v2");

checkpatch complains rightfully about GPL v2 here. If you agree, I'll
change this to simply GPL.

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

Re: [PATCH v2 1/3] rtc: Add support for the SSD202D RTC

[Romain Perier]

Le dim. 15 oct. 2023 à 23:48, Alexandre Belloni
<alexandre.belloni@bootlin.com> a écrit :
>
> Hello Romain,
>
> Sorry for the very long delay,

Hi,

np,

>
> On 13/09/2023 17:16:04+0200, Romain Perier wrote:
> > +MODULE_AUTHOR("Daniel Palmer <daniel@thingy.jp>");
> > +MODULE_AUTHOR("Romain Perier <romain.perier@gmail.com>");
> > +MODULE_DESCRIPTION("MStar SSD202D RTC Driver");
> > +MODULE_LICENSE("GPL v2");
>
> checkpatch complains rightfully about GPL v2 here. If you agree, I'll
> change this to simply GPL.

Aah, good catch , yes I agree, it will be simpler.

Thanks,
Regards,
Romain

Re: [PATCH v2 0/3] Add RTC for MStar SSD20xD SoCs

[Alexandre Belloni]

On Wed, 13 Sep 2023 17:16:03 +0200, Romain Perier wrote:
> This patches series adds a new driver for the RTC found in the Mstar
> SSD202D SoCs. It adds a basic rtc driver, the corresponding devicetree
> bindings.
> 
> The rtctest (from selftests) has been passed on this driver, with the
> following output:
> 
> [...]

Applied, thanks!

[1/3] rtc: Add support for the SSD202D RTC
      commit: ebf6255868e6141c737cacb8d62b0b347f344877
[2/3] dt-bindings: rtc: Add Mstar SSD202D RTC
      commit: cfb67623ce281e045ec11e3eddb1b68b879b53a1

Best regards,

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

Re: [PATCH v2 0/3] Add RTC for MStar SSD20xD SoCs

[Romain Perier]

Le lun. 16 oct. 2023 à 16:55, Alexandre Belloni
<alexandre.belloni@bootlin.com> a écrit :
>
>
> On Wed, 13 Sep 2023 17:16:03 +0200, Romain Perier wrote:
> > This patches series adds a new driver for the RTC found in the Mstar
> > SSD202D SoCs. It adds a basic rtc driver, the corresponding devicetree
> > bindings.
> >
> > The rtctest (from selftests) has been passed on this driver, with the
> > following output:
> >
> > [...]
>
> Applied, thanks!

Hi,

Thanks!

>
> [1/3] rtc: Add support for the SSD202D RTC
>       commit: ebf6255868e6141c737cacb8d62b0b347f344877
> [2/3] dt-bindings: rtc: Add Mstar SSD202D RTC
>       commit: cfb67623ce281e045ec11e3eddb1b68b879b53a1
>
> Best regards,

Ah , you also merged dt-bindings, Conor (from dt maintainers) prefers
trivial-rtc.yaml, it makes sense with the current driver.
I planned to make the change for trivial-rtc in v3, so I can adapt the
commit and rename  mstar,ssd202d-rtc.yaml to trivial-rtc.yaml, what do
you think ?

Regards,
Romain



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

Re: [PATCH v2 0/3] Add RTC for MStar SSD20xD SoCs

[Alexandre Belloni]

On 17/10/2023 08:09:10+0200, Romain Perier wrote:
> Le lun. 16 oct. 2023 à 16:55, Alexandre Belloni
> <alexandre.belloni@bootlin.com> a écrit :
> >
> >
> > On Wed, 13 Sep 2023 17:16:03 +0200, Romain Perier wrote:
> > > This patches series adds a new driver for the RTC found in the Mstar
> > > SSD202D SoCs. It adds a basic rtc driver, the corresponding devicetree
> > > bindings.
> > >
> > > The rtctest (from selftests) has been passed on this driver, with the
> > > following output:
> > >
> > > [...]
> >
> > Applied, thanks!
> 
> Hi,
> 
> Thanks!
> 
> >
> > [1/3] rtc: Add support for the SSD202D RTC
> >       commit: ebf6255868e6141c737cacb8d62b0b347f344877
> > [2/3] dt-bindings: rtc: Add Mstar SSD202D RTC
> >       commit: cfb67623ce281e045ec11e3eddb1b68b879b53a1
> >
> > Best regards,
> 
> Ah , you also merged dt-bindings, Conor (from dt maintainers) prefers
> trivial-rtc.yaml, it makes sense with the current driver.
> I planned to make the change for trivial-rtc in v3, so I can adapt the
> commit and rename  mstar,ssd202d-rtc.yaml to trivial-rtc.yaml, what do
> you think ?
> 

This is fine as-is.

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

Re: [PATCH v2 2/3] dt-bindings: rtc: Add Mstar SSD202D RTC

[Romain Perier]

Hi,

Alexandre seems to be okay with the current (merged) dt-binding, what
do I do ? I can send a v3 for trivial-rtc.yaml that's not a problem
for me but both of you seem to
be disagree :)

Regards,
Romain


Le jeu. 14 sept. 2023 à 10:28, Conor Dooley
<conor.dooley@microchip.com> a écrit :
>
> On Thu, Sep 14, 2023 at 09:08:52AM +0200, Romain Perier wrote:
> > Le mer. 13 sept. 2023 à 20:30, Conor Dooley <conor@kernel.org> a écrit :
> > >
> > > On Wed, Sep 13, 2023 at 05:16:05PM +0200, Romain Perier wrote:
> > > > Add YAML bindings for Mstar SSD202D RTC.
> > > >
> > > > Signed-off-by: Romain Perier <romain.perier@gmail.com>
> > > > ---
> > > >  .../bindings/rtc/mstar,ssd202d-rtc.yaml       | 35 +++++++++++++++++++
> > > >  1 file changed, 35 insertions(+)
> > > >  create mode 100644 Documentation/devicetree/bindings/rtc/mstar,ssd202d-rtc.yaml
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/rtc/mstar,ssd202d-rtc.yaml b/Documentation/devicetree/bindings/rtc/mstar,ssd202d-rtc.yaml
> > > > new file mode 100644
> > > > index 000000000000..4c1f22ef5a2c
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/rtc/mstar,ssd202d-rtc.yaml
> > > > @@ -0,0 +1,35 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/rtc/mstar,ssd202d-rtc.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Mstar SSD202D Real Time Clock
> > > > +
> > > > +maintainers:
> > > > +  - Daniel Palmer <daniel@0x0f.com>
> > > > +  - Romain Perier <romain.perier@gmail.com>
> > > > +
> > > > +allOf:
> > > > +  - $ref: rtc.yaml#
> > > > +
> > > > +properties:
> > > > +  compatible:
> > > > +    enum:
> > > > +      - mstar,ssd202d-rtc
> > > > +  reg:
> > > > +    maxItems: 1
> > > > +
> > > > +required:
> > > > +  - compatible
> > > > +  - reg
> > >
> >
> > Hi,
> >
> > > So, this seems fine to me in isolation, but isn't this now the sort of
> > > thing that can be documented in trivial-rtc.yaml?
> > > Its only got compatible & reg, which seems to fit the bill for that.
> > >
> >
> > With the current state, it might make sense. However, currently, the
> > RTC hw block is mostly
> > reverse-engineered, the driver is not complete yet, things like
> > external irq or wakeup irq might arrive later (once we know how it is
> > wired up and used).
> > So the content of the dt-bindings might change and differ from a
> > simple "compatible & reg" requirement.
>
> It's always possible to move from that to a fully fledged binding at a
> later date. re: interrupts, trivial-rtc permits a single interrupt, so
> it'd still be suitable if the device only has one.
>
> Thanks,
> Conor.

Re: [PATCH v2 2/3] dt-bindings: rtc: Add Mstar SSD202D RTC

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 311 bytes --]

On Wed, Nov 08, 2023 at 02:27:37PM +0100, Romain Perier wrote:
> Hi,
> 
> Alexandre seems to be okay with the current (merged) dt-binding, what
> do I do ? I can send a v3 for trivial-rtc.yaml that's not a problem
> for me but both of you seem to disagree :)

Meh, I wouldn't waste your time moving it.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 2/3] dt-bindings: rtc: Add Mstar SSD202D RTC

[Romain Perier]

Le mer. 8 nov. 2023 à 16:44, Conor Dooley <conor@kernel.org> a écrit :
>
> On Wed, Nov 08, 2023 at 02:27:37PM +0100, Romain Perier wrote:
> > Hi,
> >
> > Alexandre seems to be okay with the current (merged) dt-binding, what
> > do I do ? I can send a v3 for trivial-rtc.yaml that's not a problem
> > for me but both of you seem to disagree :)
>
> Meh, I wouldn't waste your time moving it.

Ok, so I will just send a v3 with an update of the MAINTAINERS file
for the driver (I have completely forgot, my bad)

Regards,
Romain

Re: [PATCH v2 2/3] dt-bindings: rtc: Add Mstar SSD202D RTC

[Alexandre Belloni]

On 08/11/2023 17:55:06+0100, Romain Perier wrote:
> Le mer. 8 nov. 2023 à 16:44, Conor Dooley <conor@kernel.org> a écrit :
> >
> > On Wed, Nov 08, 2023 at 02:27:37PM +0100, Romain Perier wrote:
> > > Hi,
> > >
> > > Alexandre seems to be okay with the current (merged) dt-binding, what
> > > do I do ? I can send a v3 for trivial-rtc.yaml that's not a problem
> > > for me but both of you seem to disagree :)
> >
> > Meh, I wouldn't waste your time moving it.
> 
> Ok, so I will just send a v3 with an update of the MAINTAINERS file
> for the driver (I have completely forgot, my bad)
> 

v2 is already upstream:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/rtc/rtc-ssd202d.c?id=ebf6255868e6141c737cacb8d62b0b347f344877

> Regards,
> Romain

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