[PATCH 0/2] Support byte access in the RTC7301 driver

[PATCH 0/2] Support byte access in the RTC7301 driver

[Linus Walleij]

This augments the Epson RTC7301 driver so that is supports
both 8bit byte and 32bit word access.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
Linus Walleij (2):
      rtc: rtc7301: Rewrite bindings in schema
      rtc: rtc7301: Support byte-addressed IO

 .../devicetree/bindings/rtc/epson,rtc7301.txt      | 16 -------
 .../devicetree/bindings/rtc/epson,rtc7301.yaml     | 49 ++++++++++++++++++++++
 drivers/rtc/rtc-r7301.c                            | 32 +++++++++++++-
 3 files changed, 79 insertions(+), 18 deletions(-)
---
base-commit: 0bb80ecc33a8fb5a682236443c1e740d5c917d1d
change-id: 20230921-rtc-7301-regwidth-18ba1e3d0edb

Best regards,
-- 
Linus Walleij <linus.walleij@linaro.org>

[PATCH 1/2] rtc: rtc7301: Rewrite bindings in schema

[Linus Walleij]

This rewrites the Epson RTC7301 bindings to use YAML schema,
and adds a property for "reg-io-width" as used in several
other bindings to account for different register strides.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 .../devicetree/bindings/rtc/epson,rtc7301.txt      | 16 -------
 .../devicetree/bindings/rtc/epson,rtc7301.yaml     | 49 ++++++++++++++++++++++
 2 files changed, 49 insertions(+), 16 deletions(-)

diff --git a/Documentation/devicetree/bindings/rtc/epson,rtc7301.txt b/Documentation/devicetree/bindings/rtc/epson,rtc7301.txt
deleted file mode 100644
index 5f9df3f1467c..000000000000
--- a/Documentation/devicetree/bindings/rtc/epson,rtc7301.txt
+++ /dev/null
@@ -1,16 +0,0 @@
-EPSON TOYOCOM RTC-7301SF/DG
-
-Required properties:
-
-- compatible: Should be "epson,rtc7301sf" or "epson,rtc7301dg"
-- reg: Specifies base physical address and size of the registers.
-- interrupts: A single interrupt specifier.
-
-Example:
-
-rtc: rtc@44a00000 {
-	compatible = "epson,rtc7301dg";
-	reg = <0x44a00000 0x10000>;
-	interrupt-parent = <&axi_intc_0>;
-	interrupts = <3 2>;
-};
diff --git a/Documentation/devicetree/bindings/rtc/epson,rtc7301.yaml b/Documentation/devicetree/bindings/rtc/epson,rtc7301.yaml
new file mode 100644
index 000000000000..4bcf1716cab6
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/epson,rtc7301.yaml
@@ -0,0 +1,49 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/rtc/epson,rtc7301.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Epson Toyocom RTC-7301SF/DG
+
+description: The only difference between the two variants is the
+  packaging. The DG variant is a DIL package, and the SF variant
+  is a flat package.
+
+maintainers:
+  - Akinobu Mita <akinobu.mita@gmail.com>
+
+properties:
+  compatible:
+    enum:
+      - epson,rtc7301dg
+      - epson,rtc7301sf
+
+  reg:
+    maxItems: 1
+
+  reg-io-width:
+    description:
+      The size (in bytes) of the IO accesses that should be performed
+      on the device.
+    enum: [1, 4]
+
+  interrupts:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    rtc: rtc@44a00000 {
+        compatible = "epson,rtc7301dg";
+        reg = <0x44a00000 0x10000>;
+        reg-io-width = <4>;
+        interrupt-parent = <&axi_intc_0>;
+        interrupts = <3 2>;
+    };

-- 
2.41.0

Re: [PATCH 1/2] rtc: rtc7301: Rewrite bindings in schema

[Conor Dooley]

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

On Thu, Sep 21, 2023 at 10:27:42PM +0200, Linus Walleij wrote:
> This rewrites the Epson RTC7301 bindings to use YAML schema,
> and adds a property for "reg-io-width" as used in several
> other bindings to account for different register strides.

It'd probably be good to mention that there is hardware that needs this,
rather that saying "other bindings use this".

> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  .../devicetree/bindings/rtc/epson,rtc7301.txt      | 16 -------
>  .../devicetree/bindings/rtc/epson,rtc7301.yaml     | 49 ++++++++++++++++++++++
>  2 files changed, 49 insertions(+), 16 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/rtc/epson,rtc7301.txt b/Documentation/devicetree/bindings/rtc/epson,rtc7301.txt
> deleted file mode 100644
> index 5f9df3f1467c..000000000000
> --- a/Documentation/devicetree/bindings/rtc/epson,rtc7301.txt
> +++ /dev/null
> @@ -1,16 +0,0 @@
> -EPSON TOYOCOM RTC-7301SF/DG
> -
> -Required properties:
> -
> -- compatible: Should be "epson,rtc7301sf" or "epson,rtc7301dg"
> -- reg: Specifies base physical address and size of the registers.
> -- interrupts: A single interrupt specifier.
> -
> -Example:
> -
> -rtc: rtc@44a00000 {
> -	compatible = "epson,rtc7301dg";
> -	reg = <0x44a00000 0x10000>;
> -	interrupt-parent = <&axi_intc_0>;
> -	interrupts = <3 2>;
> -};
> diff --git a/Documentation/devicetree/bindings/rtc/epson,rtc7301.yaml b/Documentation/devicetree/bindings/rtc/epson,rtc7301.yaml
> new file mode 100644
> index 000000000000..4bcf1716cab6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/rtc/epson,rtc7301.yaml
> @@ -0,0 +1,49 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/rtc/epson,rtc7301.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Epson Toyocom RTC-7301SF/DG
> +
> +description: The only difference between the two variants is the
> +  packaging. The DG variant is a DIL package, and the SF variant
> +  is a flat package.

The lack of a \n after "description:" looks odd, and my OCD would like
to have it.

Otherwise, SGTM!

Thanks,
Conor.

> +
> +maintainers:
> +  - Akinobu Mita <akinobu.mita@gmail.com>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - epson,rtc7301dg
> +      - epson,rtc7301sf
> +
> +  reg:
> +    maxItems: 1
> +
> +  reg-io-width:
> +    description:
> +      The size (in bytes) of the IO accesses that should be performed
> +      on the device.
> +    enum: [1, 4]
> +
> +  interrupts:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    rtc: rtc@44a00000 {
> +        compatible = "epson,rtc7301dg";
> +        reg = <0x44a00000 0x10000>;
> +        reg-io-width = <4>;
> +        interrupt-parent = <&axi_intc_0>;
> +        interrupts = <3 2>;
> +    };
> 
> -- 
> 2.41.0
> 

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

Re: [PATCH 1/2] rtc: rtc7301: Rewrite bindings in schema

[Linus Walleij]

On Fri, Sep 22, 2023 at 12:45 PM Conor Dooley <conor@kernel.org> wrote:
> On Thu, Sep 21, 2023 at 10:27:42PM +0200, Linus Walleij wrote:

> > This rewrites the Epson RTC7301 bindings to use YAML schema,
> > and adds a property for "reg-io-width" as used in several
> > other bindings to account for different register strides.
>
> It'd probably be good to mention that there is hardware that needs this,
> rather that saying "other bindings use this".

That is an explanation why I call it "reg-io-width", rather than,
say "reg-stride-bytes" as there are other bindings that use this

But I'll mention some hardware, no problem!

Yours,
Linus Walleij

Re: [PATCH 1/2] rtc: rtc7301: Rewrite bindings in schema

[Rob Herring]

On Thu, Sep 21, 2023 at 10:27:42PM +0200, Linus Walleij wrote:
> This rewrites the Epson RTC7301 bindings to use YAML schema,
> and adds a property for "reg-io-width" as used in several
> other bindings to account for different register strides.
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  .../devicetree/bindings/rtc/epson,rtc7301.txt      | 16 -------
>  .../devicetree/bindings/rtc/epson,rtc7301.yaml     | 49 ++++++++++++++++++++++
>  2 files changed, 49 insertions(+), 16 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/rtc/epson,rtc7301.txt b/Documentation/devicetree/bindings/rtc/epson,rtc7301.txt
> deleted file mode 100644
> index 5f9df3f1467c..000000000000
> --- a/Documentation/devicetree/bindings/rtc/epson,rtc7301.txt
> +++ /dev/null
> @@ -1,16 +0,0 @@
> -EPSON TOYOCOM RTC-7301SF/DG
> -
> -Required properties:
> -
> -- compatible: Should be "epson,rtc7301sf" or "epson,rtc7301dg"
> -- reg: Specifies base physical address and size of the registers.
> -- interrupts: A single interrupt specifier.
> -
> -Example:
> -
> -rtc: rtc@44a00000 {
> -	compatible = "epson,rtc7301dg";
> -	reg = <0x44a00000 0x10000>;
> -	interrupt-parent = <&axi_intc_0>;
> -	interrupts = <3 2>;
> -};
> diff --git a/Documentation/devicetree/bindings/rtc/epson,rtc7301.yaml b/Documentation/devicetree/bindings/rtc/epson,rtc7301.yaml
> new file mode 100644
> index 000000000000..4bcf1716cab6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/rtc/epson,rtc7301.yaml
> @@ -0,0 +1,49 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/rtc/epson,rtc7301.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Epson Toyocom RTC-7301SF/DG
> +
> +description: The only difference between the two variants is the
> +  packaging. The DG variant is a DIL package, and the SF variant
> +  is a flat package.
> +
> +maintainers:
> +  - Akinobu Mita <akinobu.mita@gmail.com>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - epson,rtc7301dg
> +      - epson,rtc7301sf
> +
> +  reg:
> +    maxItems: 1
> +
> +  reg-io-width:
> +    description:
> +      The size (in bytes) of the IO accesses that should be performed
> +      on the device.
> +    enum: [1, 4]

What's the default?

> +
> +  interrupts:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    rtc: rtc@44a00000 {
> +        compatible = "epson,rtc7301dg";
> +        reg = <0x44a00000 0x10000>;
> +        reg-io-width = <4>;
> +        interrupt-parent = <&axi_intc_0>;
> +        interrupts = <3 2>;
> +    };
> 
> -- 
> 2.41.0
> 

[PATCH 2/2] rtc: rtc7301: Support byte-addressed IO

[Linus Walleij]

The old RTC7301 driver in OpenWrt used byte access, but the
current mainline Linux driver uses 32bit word access.

Make this configurable using device properties using the
standard property "reg-io-width" in e.g. device tree.

This is needed for the USRobotics USR8200 which has the
chip connected using byte accesses.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/rtc/rtc-r7301.c | 32 ++++++++++++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/drivers/rtc/rtc-r7301.c b/drivers/rtc/rtc-r7301.c
index 5dbaeb7af648..843e16966b65 100644
--- a/drivers/rtc/rtc-r7301.c
+++ b/drivers/rtc/rtc-r7301.c
@@ -14,6 +14,7 @@
 #include <linux/module.h>
 #include <linux/mod_devicetable.h>
 #include <linux/delay.h>
+#include <linux/property.h>
 #include <linux/regmap.h>
 #include <linux/platform_device.h>
 #include <linux/rtc.h>
@@ -55,12 +56,23 @@ struct rtc7301_priv {
 	u8 bank;
 };
 
-static const struct regmap_config rtc7301_regmap_config = {
+/*
+ * When the device is memory-mapped, some platforms pack the registers into
+ * 32-bit access using the lower 8 bits at each 4-byte stride, while others
+ * expose them as simply consequitive bytes.
+ */
+static const struct regmap_config rtc7301_regmap_32_config = {
 	.reg_bits = 32,
 	.val_bits = 8,
 	.reg_stride = 4,
 };
 
+static const struct regmap_config rtc7301_regmap_8_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.reg_stride = 1,
+};
+
 static u8 rtc7301_read(struct rtc7301_priv *priv, unsigned int reg)
 {
 	int reg_stride = regmap_get_reg_stride(priv->regmap);
@@ -356,7 +368,9 @@ static int __init rtc7301_rtc_probe(struct platform_device *dev)
 	void __iomem *regs;
 	struct rtc7301_priv *priv;
 	struct rtc_device *rtc;
+	static const struct regmap_config *mapconf;
 	int ret;
+	u32 val;
 
 	priv = devm_kzalloc(&dev->dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
@@ -366,8 +380,22 @@ static int __init rtc7301_rtc_probe(struct platform_device *dev)
 	if (IS_ERR(regs))
 		return PTR_ERR(regs);
 
+	ret = device_property_read_u32(&dev->dev, "reg-io-width", &val);
+	if (!ret)
+		/* Default to 32bit accesses */
+		val = 4;
+
+	switch (val) {
+	case 1:
+		mapconf = &rtc7301_regmap_8_config;
+		break;
+	case 4:
+	default:
+		mapconf = &rtc7301_regmap_32_config;
+	}
+
 	priv->regmap = devm_regmap_init_mmio(&dev->dev, regs,
-					     &rtc7301_regmap_config);
+					     mapconf);
 	if (IS_ERR(priv->regmap))
 		return PTR_ERR(priv->regmap);
 

-- 
2.41.0

Re: [PATCH 2/2] rtc: rtc7301: Support byte-addressed IO

[Alexandre Belloni]

Hi,

On 21/09/2023 22:27:43+0200, Linus Walleij wrote:
> diff --git a/drivers/rtc/rtc-r7301.c b/drivers/rtc/rtc-r7301.c
> index 5dbaeb7af648..843e16966b65 100644
> --- a/drivers/rtc/rtc-r7301.c
> +++ b/drivers/rtc/rtc-r7301.c
> @@ -14,6 +14,7 @@
>  #include <linux/module.h>
>  #include <linux/mod_devicetable.h>
>  #include <linux/delay.h>
> +#include <linux/property.h>
>  #include <linux/regmap.h>
>  #include <linux/platform_device.h>
>  #include <linux/rtc.h>
> @@ -55,12 +56,23 @@ struct rtc7301_priv {
>  	u8 bank;
>  };
>  
> -static const struct regmap_config rtc7301_regmap_config = {
> +/*
> + * When the device is memory-mapped, some platforms pack the registers into
> + * 32-bit access using the lower 8 bits at each 4-byte stride, while others
> + * expose them as simply consequitive bytes.

Shouldn't that be consecutive?


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