[PATCH net-next v7 0/3] ptp: Add driver for R-Car Gen4

[PATCH net-next v7 0/3] ptp: Add driver for R-Car Gen4

[Niklas Söderlund]

Hello,

This series is the first part cleaning up how PTP timer support is
implemented on R-Car Gen4. Currently there is partial support for it in
some of the Ethernet devices that can use it, but not all.

The partial support have been implemented by hacking the gPTP module
directly into the first Ethernet device driver that used it, RTSN for
V4H and RSWITCH for S4. This is understandable as earlier R-Car
generations had a dedicated gPTP timer for each Ethernet device, but on
Gen4 there is a single system-wide PTP timer shared by all.

The current implementation makes it impossible for other Ethernet
devices on the platform to use the PTP timer without messing around with
other Ethernet device drivers.

The effort to clean this up starts with this series which adds the
system-wide gPTP timer as its own driver and device tree node.

This series will be followed by work to the RTSN and RSWITCH drivers
will be be switched from its current partial support by mapping the gPTP
address space directly to instead use this driver.

Having both this and RTSN/RSWITCH described and enabled (!) in device
tree will not work as they will try to use the same memory region. For
this reason this new solution will only be enabled on platforms
after all user's of the gPTP clock have moved to only use the new
centralized timer. But in the interim both devices will be described
(but not enabled) in the platforms base dtsi file.

For some platforms this is straight forward, such as V4H Sparrow Hawk,
which only have the RAVB Ethernet interface. This platform currently
have no users of the PTP timer, but still advertise it supports it. This
and the soon to be posted RAVB patches solves that.

The gPTP driver itself is heavily influence by the existing partial
support for gPTP in the RTSN and RSWITCH drivers and the Renesas BSP.

Niklas Söderlund (3):
  dt-bindings: ptp: renesas,rcar-gen4-gptp: Add R-Car Gen4
  ptp: Add driver for R-Car Gen4
  arm64: dts: renesas: r8a779g0: Add gPTP node

 .../bindings/ptp/renesas,rcar-gen4-gptp.yaml  |  64 +++++
 MAINTAINERS                                   |   7 +
 arch/arm64/boot/dts/renesas/r8a779g0.dtsi     |   9 +
 drivers/ptp/Kconfig                           |  12 +
 drivers/ptp/Makefile                          |   1 +
 drivers/ptp/ptp_rcar_gen4.c                   | 225 ++++++++++++++++++
 6 files changed, 318 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/ptp/renesas,rcar-gen4-gptp.yaml
 create mode 100644 drivers/ptp/ptp_rcar_gen4.c

-- 
2.55.0

[PATCH net-next v7 1/3] dt-bindings: ptp: renesas,rcar-gen4-gptp: Add R-Car Gen4

[Niklas Söderlund]

Add bindings for the R-Car Gen4 gPTP timer. The timer enables accurate
synchronization of the clock in the control system. The timer is
system-wide and used by different Ethernet devices on each Gen4 platform.

  - On R-Car S4 it is shared between RSWITCH and RAVB.

  - On R-Car V4H it is shared between RTSN and RAVB.

  - On R-Car V4M it is only used by RAVB.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
---
* Changes since v1
- Drop 'binding for' for patch subject.
- Drop comment for renesas,rcar-gen4-gptp compatible to match other
  Renesas bindings.
- Drop unused label in example.
- Rename node ptp in example.
---
 .../bindings/ptp/renesas,rcar-gen4-gptp.yaml  | 64 +++++++++++++++++++
 MAINTAINERS                                   |  6 ++
 2 files changed, 70 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/ptp/renesas,rcar-gen4-gptp.yaml

diff --git a/Documentation/devicetree/bindings/ptp/renesas,rcar-gen4-gptp.yaml b/Documentation/devicetree/bindings/ptp/renesas,rcar-gen4-gptp.yaml
new file mode 100644
index 000000000000..3edd64d40038
--- /dev/null
+++ b/Documentation/devicetree/bindings/ptp/renesas,rcar-gen4-gptp.yaml
@@ -0,0 +1,64 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+# Copyright (C) 2026 Renesas Electronics Corp.
+# Copyright (C) 2026 Niklas Söderlund <niklas.soderlund@ragnatech.se>
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/ptp/renesas,rcar-gen4-gptp.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas R-Car Gen4 gPTP timer
+
+maintainers:
+  - Niklas Söderlund <niklas.soderlund@ragnatech.se>
+
+description:
+  The R-Car Gen4 gPTP timer enables accurate synchronization of the clock in
+  the control system. The timer is system-wide and used by different Ethernet
+  devices on each Gen4 platform.
+
+    - On R-Car S4 it is shared between RSWITCH and RAVB.
+    - On R-Car V4H it is shared between RTSN and RAVB.
+    - On R-Car V4M it is only used by RAVB.
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - renesas,r8a779f0-gptp # S4-8
+          - renesas,r8a779g0-gptp # V4H
+          - renesas,r8a779h0-gptp # V4M
+      - const: renesas,rcar-gen4-gptp
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  power-domains:
+    maxItems: 1
+
+  resets:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - power-domains
+  - resets
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/r8a779g0-cpg-mssr.h>
+    #include <dt-bindings/power/r8a779g0-sysc.h>
+
+    ptp@e6449000 {
+            compatible = "renesas,r8a779g0-gptp", "renesas,rcar-gen4-gptp";
+            reg = <0xe6449000 0x500>;
+            clocks = <&cpg CPG_MOD 2723>;
+            power-domains = <&sysc R8A779G0_PD_ALWAYS_ON>;
+            resets = <&cpg 2723>;
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 72ca3aab2106..9c1d592cd788 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -23199,6 +23199,12 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/mtd/renesas-nandc.yaml
 F:	drivers/mtd/nand/raw/renesas-nand-controller.c
 
+RENESAS R-CAR GEN4 GPTP DRIVER
+M:	Niklas Söderlund <niklas.soderlund@ragnatech.se>
+L:	linux-renesas-soc@vger.kernel.org
+S:	Supported
+F:	Documentation/devicetree/bindings/ptp/renesas,rcar-gen4-gptp.yaml
+
 RENESAS R-CAR GYROADC DRIVER
 M:	Marek Vasut <marek.vasut+renesas@mailbox.org>
 L:	linux-iio@vger.kernel.org
-- 
2.55.0

Re: [PATCH net-next v7 1/3] dt-bindings: ptp: renesas,rcar-gen4-gptp: Add R-Car Gen4

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] ptp: devm_ioremap_resource() conflict with rtsn Ethernet driver
--

commit 2999d29f8c3c04d8ff6cdf49a3fa0d43c2a42ee4
Author: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

dt-bindings: ptp: renesas,rcar-gen4-gptp: Add R-Car Gen4

This commit adds device tree bindings for the R-Car Gen4 gPTP timer. The
timer provides clock synchronization and is shared among different Ethernet
devices across Gen4 platforms.

> diff --git a/Documentation/devicetree/bindings/ptp/renesas,rcar-gen4-gptp.yaml b/Documentation/devicetree/bindings/ptp/renesas,rcar-gen4-gptp.yaml
> new file mode 100644
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/ptp/renesas,rcar-gen4-gptp.yaml

[ ... ]

> +examples:
> +  - |
> +    #include <dt-bindings/clock/r8a779g0-cpg-mssr.h>
> +    #include <dt-bindings/power/r8a779g0-sysc.h>
> +
> +    ptp@e6449000 {
> +            compatible = "renesas,r8a779g0-gptp", "renesas,rcar-gen4-gptp";
> +            reg = <0xe6449000 0x500>;

[Severity: High]
Does this new node's memory region conflict with the existing Ethernet TSN
driver?

The existing renesas,ethertsn binding also claims this exact same memory
region for the generalized Precision Time Protocol target.

During probe in drivers/net/ethernet/renesas/rtsn.c:rtsn_probe(), the
existing Ethernet driver requests this region:

        res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "gptp");
        ...
        priv->gptp = devm_ioremap_resource(&pdev->dev, res);

Because both drivers use devm_ioremap_resource(), which attempts to acquire
exclusive access to the underlying physical memory via
devm_request_mem_region(), whichever driver probes second will fail with
-EBUSY. This will cause a driver probe failure and make the device unusable.

How should this shared memory resource be managed so both drivers can probe
successfully?

> +            clocks = <&cpg CPG_MOD 2723>;
> +            power-domains = <&sysc R8A779G0_PD_ALWAYS_ON>;
> +            resets = <&cpg 2723>;
> +    };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261007185920.4042783-1-niklas.soderlund+renesas@ragnatech.se?part=1

Re: [PATCH net-next v7 1/3] dt-bindings: ptp: renesas,rcar-gen4-gptp: Add R-Car Gen4

[Niklas Söderlund]

On 2026-10-08 19:00:01 +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] ptp: devm_ioremap_resource() conflict with rtsn Ethernet driver
> --
> 
> commit 2999d29f8c3c04d8ff6cdf49a3fa0d43c2a42ee4
> Author: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> 
> dt-bindings: ptp: renesas,rcar-gen4-gptp: Add R-Car Gen4
> 
> This commit adds device tree bindings for the R-Car Gen4 gPTP timer. The
> timer provides clock synchronization and is shared among different Ethernet
> devices across Gen4 platforms.
> 
> > diff --git a/Documentation/devicetree/bindings/ptp/renesas,rcar-gen4-gptp.yaml b/Documentation/devicetree/bindings/ptp/renesas,rcar-gen4-gptp.yaml
> > new file mode 100644
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/ptp/renesas,rcar-gen4-gptp.yaml
> 
> [ ... ]
> 
> > +examples:
> > +  - |
> > +    #include <dt-bindings/clock/r8a779g0-cpg-mssr.h>
> > +    #include <dt-bindings/power/r8a779g0-sysc.h>
> > +
> > +    ptp@e6449000 {
> > +            compatible = "renesas,r8a779g0-gptp", "renesas,rcar-gen4-gptp";
> > +            reg = <0xe6449000 0x500>;
> 
> [Severity: High]
> Does this new node's memory region conflict with the existing Ethernet TSN
> driver?

Yes it does, please read the cover letter to see why.

> 
> The existing renesas,ethertsn binding also claims this exact same memory
> region for the generalized Precision Time Protocol target.
> 
> During probe in drivers/net/ethernet/renesas/rtsn.c:rtsn_probe(), the
> existing Ethernet driver requests this region:
> 
>         res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "gptp");
>         ...
>         priv->gptp = devm_ioremap_resource(&pdev->dev, res);
> 
> Because both drivers use devm_ioremap_resource(), which attempts to acquire
> exclusive access to the underlying physical memory via
> devm_request_mem_region(), whichever driver probes second will fail with
> -EBUSY. This will cause a driver probe failure and make the device unusable.
> 
> How should this shared memory resource be managed so both drivers can probe
> successfully?
> 
> > +            clocks = <&cpg CPG_MOD 2723>;
> > +            power-domains = <&sysc R8A779G0_PD_ALWAYS_ON>;
> > +            resets = <&cpg 2723>;
> > +    };
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20261007185920.4042783-1-niklas.soderlund+renesas@ragnatech.se?part=1

-- 
Kind Regards,
Niklas Söderlund

Re: [PATCH net-next v7 1/3] dt-bindings: ptp: renesas,rcar-gen4-gptp: Add R-Car Gen4

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 0 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] ptp: devm_ioremap_resource() conflict with rtsn Ethernet driver

--

> diff --git a/Documentation/devicetree/bindings/ptp/renesas,rcar-gen4-gptp.yaml b/Documentation/devicetree/bindings/ptp/renesas,rcar-gen4-gptp.yaml
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/ptp/renesas,rcar-gen4-gptp.yaml
> @@ -0,0 +1,64 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +# Copyright (C) 2026 Renesas Electronics Corp.
> +# Copyright (C) 2026 Niklas Söderlund <niklas.soderlund@ragnatech.se>
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/ptp/renesas,rcar-gen4-gptp.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Renesas R-Car Gen4 gPTP timer
> +
> +maintainers:
> +  - Niklas Söderlund <niklas.soderlund@ragnatech.se>
> +
> +description:
> +  The R-Car Gen4 gPTP timer enables accurate synchronization of the clock in
> +  the control system. The timer is system-wide and used by different Ethernet
> +  devices on each Gen4 platform.
> +
> +    - On R-Car S4 it is shared between RSWITCH and RAVB.
> +    - On R-Car V4H it is shared between RTSN and RAVB.
> +    - On R-Car V4M it is only used by RAVB.
> +
> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - renesas,r8a779f0-gptp # S4-8
> +          - renesas,r8a779g0-gptp # V4H
> +          - renesas,r8a779h0-gptp # V4M
> +      - const: renesas,rcar-gen4-gptp
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  power-domains:
> +    maxItems: 1
> +
> +  resets:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - power-domains
> +  - resets
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/r8a779g0-cpg-mssr.h>
> +    #include <dt-bindings/power/r8a779g0-sysc.h>
> +
> +    ptp@e6449000 {
> +            compatible = "renesas,r8a779g0-gptp", "renesas,rcar-gen4-gptp";
> +            reg = <0xe6449000 0x500>;
> +            clocks = <&cpg CPG_MOD 2723>;
> +            power-domains = <&sysc R8A779G0_PD_ALWAYS_ON>;
> +            resets = <&cpg 2723>;
> +    };

[Severity: High]
This example places the timer at 0xe6449000. That is the same region
renesas,ethertsn.yaml already describes as the second reg entry
("gptp") of the RTSN node on V4H. rtsn_probe() looks that range up with
platform_get_resource_byname(pdev, IORESOURCE_MEM, "gptp") and maps it
with devm_ioremap_resource(), which requests the region exclusively.

Suppose a V4H DT contains both the ethertsn node and this new ptp node,
and a driver for this binding also maps its reg with
devm_ioremap_resource(). Then whichever of the two probes second will
fail with -EBUSY, and either Ethernet or the gPTP timer is lost at boot.

How is the transition meant to work here? One option is to update
renesas,ethertsn.yaml to drop the gptp reg entry, reference this node
with a phandle instead, and change rtsn.c to match. Or is the plan for
the gPTP driver to avoid claiming the region exclusively? It would be
good to have that spelled out in this series. As it stands, the
bindings describe the same registers in two places.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261007185920.4042783-1-niklas.soderlund%2Brenesas%40ragnatech.se

[PATCH net-next v7 2/3] ptp: Add driver for R-Car Gen4

[Niklas Söderlund]

Add driver for the gPTP timer found on R-Car Gen4 devices. The timer is
system-wide and shared by different Ethernet devices on each Gen4
platform. The operation of the timer is however not completely in
depended of the systems Ethernet devices.

  - On R-Car S4 is gated by the RSWITCH Ethernet module clock.

  - On R-Car V4H is gated by the RTSN Ethernet module clock.

  - On R-Car V4M is gated by its own module clock, the system have
    neither RTSN or RSWITCH device. But the module clock is the same as
    RTSN on V4H and the documentation referees to it as tsn (EtherTSN).

The gPTP device do have its own register space on all three platforms.
But on S4 and V4H it will share its clock and reset property with
RSWITCH or RTSN, respectively.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
---
* Changes since v5
- Use pm_runtime_resume_and_get() instead of pm_runtime_get_sync().

* Changes since v4
- Drop mod_devicetable.h include.
- Remove clap of register value as .max_adj prevents any overflow.

* Changes since v3
- Clamp increment calculated to register limitations.
- Check return value of clk_get_rate().
- Disable PM if ptp_clock_register() fails.
---
 MAINTAINERS                 |   1 +
 drivers/ptp/Kconfig         |  12 ++
 drivers/ptp/Makefile        |   1 +
 drivers/ptp/ptp_rcar_gen4.c | 225 ++++++++++++++++++++++++++++++++++++
 4 files changed, 239 insertions(+)
 create mode 100644 drivers/ptp/ptp_rcar_gen4.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 9c1d592cd788..2a3b0c417d65 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -23204,6 +23204,7 @@ M:	Niklas Söderlund <niklas.soderlund@ragnatech.se>
 L:	linux-renesas-soc@vger.kernel.org
 S:	Supported
 F:	Documentation/devicetree/bindings/ptp/renesas,rcar-gen4-gptp.yaml
+F:	drivers/ptp/ptp_rcar_gen4.c
 
 RENESAS R-CAR GYROADC DRIVER
 M:	Marek Vasut <marek.vasut+renesas@mailbox.org>
diff --git a/drivers/ptp/Kconfig b/drivers/ptp/Kconfig
index feb50f8cc406..573334a66557 100644
--- a/drivers/ptp/Kconfig
+++ b/drivers/ptp/Kconfig
@@ -264,4 +264,16 @@ config PTP_NETC_V4_TIMER
 	  synchronization. It also supports periodic output signal (e.g. PPS)
 	  and external trigger timestamping.
 
+config PTP_RCAR_GEN4
+	tristate "Renesas R-Car Gen4 PTP Driver"
+	depends on ARCH_RENESAS || COMPILE_TEST
+	depends on PTP_1588_CLOCK
+	help
+	  This driver adds support for using the Renesas R-Car Gen4 gPTP timer
+	  as a PTP clock, the clock can then be used by Gen4 Ethernet drivers
+	  for PTP time synchronization.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called ptp_rcar_gen4.
+
 endmenu
diff --git a/drivers/ptp/Makefile b/drivers/ptp/Makefile
index bdc47e284f14..0464a586bed2 100644
--- a/drivers/ptp/Makefile
+++ b/drivers/ptp/Makefile
@@ -22,3 +22,4 @@ obj-$(CONFIG_PTP_1588_CLOCK_OCP)	+= ptp_ocp.o
 obj-$(CONFIG_PTP_DFL_TOD)		+= ptp_dfl_tod.o
 obj-$(CONFIG_PTP_S390)			+= ptp_s390.o
 obj-$(CONFIG_PTP_NETC_V4_TIMER)		+= ptp_netc.o
+obj-$(CONFIG_PTP_RCAR_GEN4)		+= ptp_rcar_gen4.o
diff --git a/drivers/ptp/ptp_rcar_gen4.c b/drivers/ptp/ptp_rcar_gen4.c
new file mode 100644
index 000000000000..e82b6c5abc30
--- /dev/null
+++ b/drivers/ptp/ptp_rcar_gen4.c
@@ -0,0 +1,225 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Renesas R-Car Gen4 gPTP device driver
+ *
+ * Copyright (C) 2026 Renesas Electronics Corporation
+ * Copyright (C) 2026 Niklas Söderlund <niklas.soderlund@ragnatech.se>
+ */
+
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/ptp_clock_kernel.h>
+#include <linux/types.h>
+
+#define PTPTMEC_REG		0x0010
+#define PTPTMDC_REG		0x0014
+#define PTPTIVC0_REG		0x0020
+#define PTPTOVC00_REG		0x0030
+#define PTPTOVC10_REG		0x0034
+#define PTPTOVC20_REG		0x0038
+#define PTPGPTPTM00_REG		0x0050
+#define PTPGPTPTM10_REG		0x0054
+#define PTPGPTPTM20_REG		0x0058
+
+struct ptp_rcar_gen4_priv {
+	void __iomem *base;
+	struct clk *clk;
+
+	struct ptp_clock *clock;
+	struct ptp_clock_info info;
+
+	spinlock_t lock;	/* Registers access. */
+	s64 default_addend;
+};
+
+#define ptp_to_priv(ptp) container_of(ptp, struct ptp_rcar_gen4_priv, info)
+
+static int ptp_rcar_gen4_adjfine(struct ptp_clock_info *ptp, long scaled_ppm)
+{
+	struct ptp_rcar_gen4_priv *priv = ptp_to_priv(ptp);
+	s64 addend = priv->default_addend;
+	bool neg_adj = scaled_ppm < 0;
+	unsigned long flags;
+	s64 diff;
+
+	if (neg_adj)
+		scaled_ppm = -scaled_ppm;
+	diff = div_s64(addend * scaled_ppm_to_ppb(scaled_ppm), NSEC_PER_SEC);
+	addend = neg_adj ? addend - diff : addend + diff;
+
+	spin_lock_irqsave(&priv->lock, flags);
+	iowrite32(addend, priv->base + PTPTIVC0_REG);
+	spin_unlock_irqrestore(&priv->lock, flags);
+
+	return 0;
+}
+
+static void _ptp_rcar_gen4_gettime(struct ptp_clock_info *ptp,
+				   struct timespec64 *ts)
+{
+	struct ptp_rcar_gen4_priv *priv = ptp_to_priv(ptp);
+
+	lockdep_assert_held(&priv->lock);
+
+	ts->tv_nsec = ioread32(priv->base + PTPGPTPTM00_REG);
+	ts->tv_sec = ioread32(priv->base + PTPGPTPTM10_REG) |
+		((s64)ioread32(priv->base + PTPGPTPTM20_REG) << 32);
+}
+
+static int ptp_rcar_gen4_gettime(struct ptp_clock_info *ptp,
+				 struct timespec64 *ts)
+{
+	struct ptp_rcar_gen4_priv *priv = ptp_to_priv(ptp);
+	unsigned long flags;
+
+	spin_lock_irqsave(&priv->lock, flags);
+	_ptp_rcar_gen4_gettime(ptp, ts);
+	spin_unlock_irqrestore(&priv->lock, flags);
+
+	return 0;
+}
+
+static void _ptp_rcar_gen4_settime(struct ptp_clock_info *ptp,
+				   const struct timespec64 *ts)
+{
+	struct ptp_rcar_gen4_priv *priv = ptp_to_priv(ptp);
+
+	lockdep_assert_held(&priv->lock);
+
+	iowrite32(1, priv->base + PTPTMDC_REG);
+	iowrite32(0, priv->base + PTPTOVC20_REG);
+	iowrite32(0, priv->base + PTPTOVC10_REG);
+	iowrite32(0, priv->base + PTPTOVC00_REG);
+	iowrite32(1, priv->base + PTPTMEC_REG);
+	iowrite32(ts->tv_sec >> 32, priv->base + PTPTOVC20_REG);
+	iowrite32(ts->tv_sec, priv->base + PTPTOVC10_REG);
+	iowrite32(ts->tv_nsec, priv->base + PTPTOVC00_REG);
+}
+
+static int ptp_rcar_gen4_settime(struct ptp_clock_info *ptp,
+				 const struct timespec64 *ts)
+{
+	struct ptp_rcar_gen4_priv *priv = ptp_to_priv(ptp);
+	unsigned long flags;
+
+	spin_lock_irqsave(&priv->lock, flags);
+	_ptp_rcar_gen4_settime(ptp, ts);
+	spin_unlock_irqrestore(&priv->lock, flags);
+
+	return 0;
+}
+
+static int ptp_rcar_gen4_adjtime(struct ptp_clock_info *ptp, s64 delta)
+{
+	struct ptp_rcar_gen4_priv *priv = ptp_to_priv(ptp);
+	struct timespec64 ts;
+	unsigned long flags;
+	s64 now;
+
+	spin_lock_irqsave(&priv->lock, flags);
+	_ptp_rcar_gen4_gettime(ptp, &ts);
+	now = ktime_to_ns(timespec64_to_ktime(ts));
+	ts = ns_to_timespec64(now + delta);
+	_ptp_rcar_gen4_settime(ptp, &ts);
+	spin_unlock_irqrestore(&priv->lock, flags);
+
+	return 0;
+}
+
+static struct ptp_clock_info ptp_rcar_gen4_info = {
+	.owner = THIS_MODULE,
+	.name = "R-Car Gen4 gPTP",
+	.max_adj = 50000000,
+	.adjfine = ptp_rcar_gen4_adjfine,
+	.adjtime = ptp_rcar_gen4_adjtime,
+	.gettime64 = ptp_rcar_gen4_gettime,
+	.settime64 = ptp_rcar_gen4_settime,
+};
+
+static int ptp_rcar_gen4_probe(struct platform_device *pdev)
+{
+	struct ptp_rcar_gen4_priv *priv;
+	struct device *dev = &pdev->dev;
+	unsigned long rate;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, priv);
+
+	priv->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(priv->base))
+		return PTR_ERR(priv->base);
+
+	priv->clk = devm_clk_get(dev, NULL);
+	if (IS_ERR(priv->clk))
+		return PTR_ERR(priv->clk);
+
+	rate = clk_get_rate(priv->clk);
+	if (!rate)
+		return -ENODEV;
+
+	spin_lock_init(&priv->lock);
+
+	priv->info = ptp_rcar_gen4_info;
+
+	/* Default timer increment in ns.
+	 * bit[31:27] - integer
+	 * bit[26:0]  - decimal
+	 * increment[ns] = perid[ns] * 2^27 => (1ns * 2^27) / rate[hz]
+	 */
+
+	priv->default_addend = div_s64(1000000000LL << 27, rate);
+
+	pm_runtime_enable(dev);
+	pm_runtime_resume_and_get(dev);
+
+	iowrite32(priv->default_addend, priv->base + PTPTIVC0_REG);
+	iowrite32(1, priv->base + PTPTMEC_REG);
+
+	priv->clock = ptp_clock_register(&priv->info, dev);
+	if (IS_ERR(priv->clock)) {
+		pm_runtime_put_sync(dev);
+		pm_runtime_disable(dev);
+		return PTR_ERR(priv->clock);
+	}
+
+	return 0;
+}
+
+static void ptp_rcar_gen4_remove(struct platform_device *pdev)
+{
+	struct ptp_rcar_gen4_priv *priv = platform_get_drvdata(pdev);
+	struct device *dev = &pdev->dev;
+
+	ptp_clock_unregister(priv->clock);
+
+	iowrite32(1, priv->base + PTPTMDC_REG);
+
+	pm_runtime_put_sync(dev);
+	pm_runtime_disable(dev);
+}
+
+static const struct of_device_id ptp_rcar_gen4_of_match[] = {
+	{ .compatible = "renesas,rcar-gen4-gptp", },
+	{ /* Sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, ptp_rcar_gen4_of_match);
+
+static struct platform_driver ptp_rcar_gen4_driver = {
+	.driver = {
+		.name = "ptp-rcar-gen4",
+		.of_match_table = ptp_rcar_gen4_of_match,
+	},
+	.probe    = ptp_rcar_gen4_probe,
+	.remove   = ptp_rcar_gen4_remove,
+};
+module_platform_driver(ptp_rcar_gen4_driver);
+
+MODULE_AUTHOR("Niklas Söderlund");
+MODULE_DESCRIPTION("Renesas R-Car Gen4 gPTP driver");
+MODULE_LICENSE("GPL");
-- 
2.55.0

Re: [PATCH net-next v7 2/3] ptp: Add driver for R-Car Gen4

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] pm: unchecked pm_runtime_resume_and_get() return in ptp_rcar_gen4_probe()
- [High] pm: missing return value check for pm_runtime_resume_and_get()
- [Medium] ptp: missing hardware timer disable on ptp_clock_register() error path
- [Medium] ptp: potential time tearing in gettime due to unlatched sequential reads
- [Medium] ptp: torn hardware state exposed during _ptp_rcar_gen4_settime()
--

commit 3db5fb281fb9f619fc7eb84cff8bc9d6876c1eab
Author: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

ptp: Add driver for R-Car Gen4

Add driver for the gPTP timer found on R-Car Gen4 devices. The timer is
system-wide and shared by different Ethernet devices on each Gen4
platform.

> diff --git a/drivers/ptp/ptp_rcar_gen4.c b/drivers/ptp/ptp_rcar_gen4.c
> new file mode 100644
> index 0000000000000..e82b6c5abc306
> --- /dev/null
> +++ b/drivers/ptp/ptp_rcar_gen4.c

[ ... ]

> +static void _ptp_rcar_gen4_gettime(struct ptp_clock_info *ptp,
> +				   struct timespec64 *ts)
> +{
> +	struct ptp_rcar_gen4_priv *priv = ptp_to_priv(ptp);
> +
> +	lockdep_assert_held(&priv->lock);
> +
> +	ts->tv_nsec = ioread32(priv->base + PTPGPTPTM00_REG);
> +	ts->tv_sec = ioread32(priv->base + PTPGPTPTM10_REG) |
> +		((s64)ioread32(priv->base + PTPGPTPTM20_REG) << 32);
> +}

[Severity: Medium]
Can these sequential reads result in a torn time read?

If the time advances across a second boundary between the read of
PTPGPTPTM00_REG and the subsequent reads of PTPGPTPTM10_REG and
PTPGPTPTM20_REG, the returned timestamp could be incorrect by a full second
unless there is a hardware latch or a software retry loop.

Since this is also used in ptp_rcar_gen4_adjtime(), a torn read there would
corrupt the hardware clock state by writing the torn value back.

[ ... ]

> +static void _ptp_rcar_gen4_settime(struct ptp_clock_info *ptp,
> +				   const struct timespec64 *ts)
> +{
> +	struct ptp_rcar_gen4_priv *priv = ptp_to_priv(ptp);
> +
> +	lockdep_assert_held(&priv->lock);
> +
> +	iowrite32(1, priv->base + PTPTMDC_REG);
> +	iowrite32(0, priv->base + PTPTOVC20_REG);
> +	iowrite32(0, priv->base + PTPTOVC10_REG);
> +	iowrite32(0, priv->base + PTPTOVC00_REG);
> +	iowrite32(1, priv->base + PTPTMEC_REG);
> +	iowrite32(ts->tv_sec >> 32, priv->base + PTPTOVC20_REG);
> +	iowrite32(ts->tv_sec, priv->base + PTPTOVC10_REG);
> +	iowrite32(ts->tv_nsec, priv->base + PTPTOVC00_REG);
> +}

[Severity: Medium]
Does re-enabling the timer before the new time offsets are fully written
create a race window?

Between the write to PTPTMEC_REG (which enables the timer) and the final write
to PTPTOVC00_REG, the timer is actively running with a partially set offset.
Could concurrent hardware timestamping events observe an intermediate torn
state during this window?

[ ... ]

> +static int ptp_rcar_gen4_probe(struct platform_device *pdev)
> +{
> +	struct ptp_rcar_gen4_priv *priv;
> +	struct device *dev = &pdev->dev;
> +	unsigned long rate;
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	platform_set_drvdata(pdev, priv);
> +
> +	priv->base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(priv->base))
> +		return PTR_ERR(priv->base);
> +
> +	priv->clk = devm_clk_get(dev, NULL);
> +	if (IS_ERR(priv->clk))
> +		return PTR_ERR(priv->clk);
> +
> +	rate = clk_get_rate(priv->clk);
> +	if (!rate)
> +		return -ENODEV;
> +
> +	spin_lock_init(&priv->lock);
> +
> +	priv->info = ptp_rcar_gen4_info;
> +
> +	/* Default timer increment in ns.
> +	 * bit[31:27] - integer
> +	 * bit[26:0]  - decimal
> +	 * increment[ns] = perid[ns] * 2^27 => (1ns * 2^27) / rate[hz]
> +	 */
> +
> +	priv->default_addend = div_s64(1000000000LL << 27, rate);
> +
> +	pm_runtime_enable(dev);
> +	pm_runtime_resume_and_get(dev);
> +
> +	iowrite32(priv->default_addend, priv->base + PTPTIVC0_REG);
> +	iowrite32(1, priv->base + PTPTMEC_REG);

[Severity: High]
If pm_runtime_resume_and_get() fails (e.g., due to power domain or clock
initialization failure), the device could remain suspended.
Could the subsequent iowrite32() calls then access unclocked or powered-off
hardware, resulting in a synchronous external abort or silent register
corruption?

> +
> +	priv->clock = ptp_clock_register(&priv->info, dev);
> +	if (IS_ERR(priv->clock)) {
> +		pm_runtime_put_sync(dev);

[Severity: High]
If the previous pm_runtime_resume_and_get() had failed, wouldn't this
unconditional pm_runtime_put_sync() cause the PM usage counter to underflow?

[Severity: Medium]
If ptp_clock_register() fails, the error path cuts power, but doesn't it
miss disabling the hardware timer logically?

In ptp_rcar_gen4_remove(), the hardware timer is explicitly disabled by
writing to PTPTMDC_REG before cutting power:

ptp_rcar_gen4_remove() {
	...
	iowrite32(1, priv->base + PTPTMDC_REG);
	pm_runtime_put_sync(dev);
	...
}

Could suspending the PM power domain while the IP block's internal timer is
still logically enabled lead to hardware state corruption or prevent a clean
restart upon module reload?

> +		pm_runtime_disable(dev);
> +		return PTR_ERR(priv->clock);
> +	}
> +
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261007185920.4042783-1-niklas.soderlund+renesas@ragnatech.se?part=2

Re: [PATCH net-next v7 2/3] ptp: Add driver for R-Car Gen4

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 8 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 4 · Low: 4

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The new standalone driver drives the same gPTP register block
  that drivers/net/ethernet/renesas/rcar_gen4_ptp.c already drives on…
- [Medium] ptp_rcar_gen4_probe() ignores the return value of
  pm_runtime_resume_and_get(dev) at line 179.
- [Medium] ptp: torn hardware state exposed during
  _ptp_rcar_gen4_settime()
- [Medium] ptp: potential time tearing in gettime due to unlatched
  sequential reads
- [Low] The binding (renesas,rcar-gen4-gptp.yaml) lists `resets` as
  required, and the V4H DTS node sets resets = <&cpg 2723>.
- [Low] The commit message for "ptp: Add driver for R-Car Gen4" has
  several typos and grammar errors in the only explanation of the…
- [Low] The PTPTIVC0 comment in ptp_rcar_gen4_probe() (lines 170-174) has
  'perid' for 'period' and calls bits[26:0] 'decimal' when they are a…
- [Low] In ptp_rcar_gen4_probe(), the timer is enabled (iowrite32(1,
  PTPTMEC_REG), line 182) before the fallible ptp_clock_register() call.

--

ptp: Add driver for R-Car Gen4

This commit adds a standalone platform driver for the gPTP timer block
found on R-Car Gen4 SoCs (S4, V4H and V4M). The driver maps the timer
registers, programs the default increment from the module clock rate, and
registers a PTP clock with adjfine, adjtime, gettime64 and settime64 ops.

> Add driver for the gPTP timer found on R-Car Gen4 devices. The timer is
> system-wide and shared by different Ethernet devices on each Gen4
> platform. The operation of the timer is however not completely in
> depended of the systems Ethernet devices.
>
>   - On R-Car S4 is gated by the RSWITCH Ethernet module clock.
>
>   - On R-Car V4H is gated by the RTSN Ethernet module clock.
>
>   - On R-Car V4M is gated by its own module clock, the system have
>     neither RTSN or RSWITCH device. But the module clock is the same as
>     RTSN on V4H and the documentation referees to it as tsn (EtherTSN).
>
> The gPTP device do have its own register space on all three platforms.

[Severity: Low]
This isn't a bug, but there are a few typos and grammar errors in the
commit message:

  "not completely in depended of" -> "not completely independent of"
  "On R-Car S4 is gated" -> "On R-Car S4 it is gated"
  "On R-Car V4H is gated" -> "On R-Car V4H it is gated"
  "the system have" -> "the system has"
  "the documentation referees to it" -> "the documentation refers to it"
  "The gPTP device do have" -> "The gPTP device does have"

> diff --git a/drivers/ptp/ptp_rcar_gen4.c b/drivers/ptp/ptp_rcar_gen4.c
> new file mode 100644
> index 0000000000000..e82b6c5abc306
> --- /dev/null
> +++ b/drivers/ptp/ptp_rcar_gen4.c
> @@ -0,0 +1,225 @@

[ ... ]

> +static int ptp_rcar_gen4_probe(struct platform_device *pdev)
> +{
> +	struct ptp_rcar_gen4_priv *priv;
> +	struct device *dev = &pdev->dev;
> +	unsigned long rate;
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	platform_set_drvdata(pdev, priv);
> +
> +	priv->base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(priv->base))
> +		return PTR_ERR(priv->base);

[Severity: Medium]
Is this the same register block that
drivers/net/ethernet/renesas/rcar_gen4_ptp.c already drives for RTSN (V4H)
and RSWITCH (S4)? The register offsets, the adjfine, gettime, settime and
adjtime bodies, the addend formula and max_adj all look like a copy of that
code. The commit message doesn't mention the existing implementation or
explain how the two are meant to coexist.

On V4H, the gptp node added in this series and the existing tsn0 node in
r8a779g0.dtsi describe the same range:

    gptp: ptp@e6449000 { ... reg = <0 0xe6449000 0 0x500>; ... }
    tsn0: ethernet@e6460000 {
        reg = <0 0xe6460000 0 0x7000>, <0 0xe6449000 0 0x500>;
        reg-names = "tsnes", "gptp";
        ...
    }

rtsn_probe() requests the "gptp" region with devm_ioremap_resource() and
registers its own PHC through rcar_gen4_ptp_register(). Here,
ptp_rcar_gen4_probe() requests the same region again through
devm_platform_ioremap_resource().

If a board enables both &gptp (so RAVB can use it through "ptp-timer") and
&tsn0, does whichever device probes second fail with -EBUSY? That would
lose either the RTSN network interface or the standalone PHC.

The DTS commit in this series says the module "is shared between the RAVB
and RTSN Ethernet devices", but rtsn and rswitch are left unchanged. Is
there a plan to move them over to this driver?

> +
> +	priv->clk = devm_clk_get(dev, NULL);
> +	if (IS_ERR(priv->clk))
> +		return PTR_ERR(priv->clk);

[Severity: Low]
The renesas,rcar-gen4-gptp.yaml binding lists resets as required, and the
V4H node uses resets = <&cpg 2723>. Should probe also get and deassert
the reset? ptp_rcar_gen4_probe() gets only the register range and the
clock, and ptp_rcar_gen4_remove() never asserts a reset.

Suppose the module reset is still asserted at boot, for example on V4M
where the gPTP block has its own reset. Would the PTPTIVC0 and PTPTMEC
writes later in probe have any effect? A PHC would still be registered.

The commit message also says the reset is shared with RTSN on V4H, and
rtsn_reset() pulses it with reset_control_reset(). Without a reset handle,
how would this driver coordinate with that, or reprogram the timer after
the pulse?

[ ... ]

> +	/* Default timer increment in ns.
> +	 * bit[31:27] - integer
> +	 * bit[26:0]  - decimal
> +	 * increment[ns] = perid[ns] * 2^27 => (1ns * 2^27) / rate[hz]
> +	 */
> +
> +	priv->default_addend = div_s64(1000000000LL << 27, rate);

[Severity: Low]
This isn't a bug, but the comment has a typo (perid -> period). Also,
bits [26:0] are a binary fraction rather than decimal.

The formula in the comment doesn't match the code either. The code
computes period[ns] * 2^27 = (1e9 * 2^27) / rate[Hz], not
(1ns * 2^27) / rate[hz].

The comment looks like it was copied from
rcar_gen4_ptp_rate_to_increment() in
drivers/net/ethernet/renesas/rcar_gen4_ptp.c.

> +
> +	pm_runtime_enable(dev);
> +	pm_runtime_resume_and_get(dev);

[Severity: Medium]
Should the return value of pm_runtime_resume_and_get() be checked here?
The v6 changelog says pm_runtime_get_sync() was replaced so that failures
could be handled, but the result is still ignored.

When pm_runtime_resume_and_get() fails, it drops the usage count itself
and returns the error. The device is not resumed and no reference is held.
Probe then goes on to do this:

    iowrite32(priv->default_addend, priv->base + PTPTIVC0_REG);
    iowrite32(1, priv->base + PTPTMEC_REG);

The driver never calls clk_prepare_enable(). The module clock is only
turned on by the runtime PM resume, so these writes may go to an unclocked
block.

ptp_clock_register() can still succeed after that. Userspace would then
get a PHC whose gettime, settime and adjtime access unclocked MMIO.

The later puts would also be unbalanced: the pm_runtime_put_sync() in the
ptp_clock_register() error path below, and the one in
ptp_rcar_gen4_remove():

    iowrite32(1, priv->base + PTPTMDC_REG);

    pm_runtime_put_sync(dev);
    pm_runtime_disable(dev);

Wouldn't rpm_drop_usage_count() then warn "Runtime PM usage count
underflow!"?

> +
> +	iowrite32(priv->default_addend, priv->base + PTPTIVC0_REG);
> +	iowrite32(1, priv->base + PTPTMEC_REG);
> +
> +	priv->clock = ptp_clock_register(&priv->info, dev);
> +	if (IS_ERR(priv->clock)) {
> +		pm_runtime_put_sync(dev);
> +		pm_runtime_disable(dev);
> +		return PTR_ERR(priv->clock);
> +	}

[Severity: Medium]
_ptp_rcar_gen4_settime() first stops the timer and clears the three
PTPTOVC offset registers. It then writes PTPTMEC to start the timer
again, and only after that programs the new offset. Between the
PTPTMEC write and the final PTPTOVC00 write, the counter runs with a
zero or partially written offset.

This timer is the time base shared by the Ethernet MACs on these SoCs
(RSWITCH, RTSN, and RAVB through ptp-timer). Could a packet that is
timestamped inside that window get a time near zero? Could it get the
new upper seconds word combined with a stale lower word?

The same window opens on every ptp_rcar_gen4_adjtime() call, since it
goes through this helper. So this is not limited to an initial
settime.

Is there a reason the timer has to be enabled before the offset is
loaded? If the hardware allows it, writing PTPTOVC20, PTPTOVC10 and
PTPTOVC00 while the timer is still stopped, and only then writing
PTPTMEC, would close the window.

If the hardware instead applies the offset atomically on the PTPTOVC00
write, could a comment say so? In that case, is the intermediate
zeroing of the offset registers needed at all?

[Severity: Low]
The PTPTMEC write above starts the timer before ptp_clock_register(),
which can fail. Should this error path also stop the timer, the way
ptp_rcar_gen4_remove() does before it drops runtime PM?

    iowrite32(1, priv->base + PTPTMDC_REG);

On S4 and V4H the gating clock belongs to RSWITCH or RTSN and may stay
on. Could the counter then keep running with no driver bound?

> +
> +	return 0;
> +}

[ ... ]

> @@ -0,0 +1,225 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Renesas R-Car Gen4 gPTP device driver
> + *
> + * Copyright (C) 2026 Renesas Electronics Corporation
> + * Copyright (C) 2026 Niklas Söderlund <niklas.soderlund@ragnatech.se>
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/ptp_clock_kernel.h>
> +#include <linux/types.h>
> +
> +#define PTPTMEC_REG		0x0010
> +#define PTPTMDC_REG		0x0014
> +#define PTPTIVC0_REG		0x0020
> +#define PTPTOVC00_REG		0x0030
> +#define PTPTOVC10_REG		0x0034
> +#define PTPTOVC20_REG		0x0038
> +#define PTPGPTPTM00_REG		0x0050
> +#define PTPGPTPTM10_REG		0x0054
> +#define PTPGPTPTM20_REG		0x0058
> +
> +struct ptp_rcar_gen4_priv {
> +	void __iomem *base;
> +	struct clk *clk;
> +
> +	struct ptp_clock *clock;
> +	struct ptp_clock_info info;
> +
> +	spinlock_t lock;	/* Registers access. */
> +	s64 default_addend;
> +};
> +
> +#define ptp_to_priv(ptp) container_of(ptp, struct ptp_rcar_gen4_priv, info)
> +
> +static int ptp_rcar_gen4_adjfine(struct ptp_clock_info *ptp, long scaled_ppm)
> +{
> +	struct ptp_rcar_gen4_priv *priv = ptp_to_priv(ptp);
> +	s64 addend = priv->default_addend;
> +	bool neg_adj = scaled_ppm < 0;
> +	unsigned long flags;
> +	s64 diff;
> +
> +	if (neg_adj)
> +		scaled_ppm = -scaled_ppm;
> +	diff = div_s64(addend * scaled_ppm_to_ppb(scaled_ppm), NSEC_PER_SEC);
> +	addend = neg_adj ? addend - diff : addend + diff;
> +
> +	spin_lock_irqsave(&priv->lock, flags);
> +	iowrite32(addend, priv->base + PTPTIVC0_REG);
> +	spin_unlock_irqrestore(&priv->lock, flags);
> +
> +	return 0;
> +}
> +
> +static void _ptp_rcar_gen4_gettime(struct ptp_clock_info *ptp,
> +				   struct timespec64 *ts)
> +{
> +	struct ptp_rcar_gen4_priv *priv = ptp_to_priv(ptp);
> +
> +	lockdep_assert_held(&priv->lock);
> +
> +	ts->tv_nsec = ioread32(priv->base + PTPGPTPTM00_REG);
> +	ts->tv_sec = ioread32(priv->base + PTPGPTPTM10_REG) |
> +		((s64)ioread32(priv->base + PTPGPTPTM20_REG) << 32);
> +}
> +
> +static int ptp_rcar_gen4_gettime(struct ptp_clock_info *ptp,
> +				 struct timespec64 *ts)
> +{
> +	struct ptp_rcar_gen4_priv *priv = ptp_to_priv(ptp);
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&priv->lock, flags);
> +	_ptp_rcar_gen4_gettime(ptp, ts);
> +	spin_unlock_irqrestore(&priv->lock, flags);
> +
> +	return 0;
> +}
> +
> +static void _ptp_rcar_gen4_settime(struct ptp_clock_info *ptp,
> +				   const struct timespec64 *ts)
> +{
> +	struct ptp_rcar_gen4_priv *priv = ptp_to_priv(ptp);
> +
> +	lockdep_assert_held(&priv->lock);
> +
> +	iowrite32(1, priv->base + PTPTMDC_REG);
> +	iowrite32(0, priv->base + PTPTOVC20_REG);
> +	iowrite32(0, priv->base + PTPTOVC10_REG);
> +	iowrite32(0, priv->base + PTPTOVC00_REG);
> +	iowrite32(1, priv->base + PTPTMEC_REG);
> +	iowrite32(ts->tv_sec >> 32, priv->base + PTPTOVC20_REG);
> +	iowrite32(ts->tv_sec, priv->base + PTPTOVC10_REG);
> +	iowrite32(ts->tv_nsec, priv->base + PTPTOVC00_REG);
> +}
> +
> +static int ptp_rcar_gen4_settime(struct ptp_clock_info *ptp,
> +				 const struct timespec64 *ts)
> +{
> +	struct ptp_rcar_gen4_priv *priv = ptp_to_priv(ptp);
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&priv->lock, flags);
> +	_ptp_rcar_gen4_settime(ptp, ts);
> +	spin_unlock_irqrestore(&priv->lock, flags);
> +
> +	return 0;
> +}
> +
> +static int ptp_rcar_gen4_adjtime(struct ptp_clock_info *ptp, s64 delta)
> +{
> +	struct ptp_rcar_gen4_priv *priv = ptp_to_priv(ptp);
> +	struct timespec64 ts;
> +	unsigned long flags;
> +	s64 now;
> +
> +	spin_lock_irqsave(&priv->lock, flags);
> +	_ptp_rcar_gen4_gettime(ptp, &ts);
> +	now = ktime_to_ns(timespec64_to_ktime(ts));
> +	ts = ns_to_timespec64(now + delta);
> +	_ptp_rcar_gen4_settime(ptp, &ts);
> +	spin_unlock_irqrestore(&priv->lock, flags);
> +
> +	return 0;
> +}
> +
> +static struct ptp_clock_info ptp_rcar_gen4_info = {
> +	.owner = THIS_MODULE,
> +	.name = "R-Car Gen4 gPTP",
> +	.max_adj = 50000000,
> +	.adjfine = ptp_rcar_gen4_adjfine,
> +	.adjtime = ptp_rcar_gen4_adjtime,
> +	.gettime64 = ptp_rcar_gen4_gettime,
> +	.settime64 = ptp_rcar_gen4_settime,
> +};
> +
> +static int ptp_rcar_gen4_probe(struct platform_device *pdev)
> +{
> +	struct ptp_rcar_gen4_priv *priv;
> +	struct device *dev = &pdev->dev;
> +	unsigned long rate;
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	platform_set_drvdata(pdev, priv);
> +
> +	priv->base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(priv->base))
> +		return PTR_ERR(priv->base);
> +
> +	priv->clk = devm_clk_get(dev, NULL);
> +	if (IS_ERR(priv->clk))
> +		return PTR_ERR(priv->clk);
> +
> +	rate = clk_get_rate(priv->clk);
> +	if (!rate)
> +		return -ENODEV;
> +
> +	spin_lock_init(&priv->lock);
> +
> +	priv->info = ptp_rcar_gen4_info;
> +
> +	/* Default timer increment in ns.
> +	 * bit[31:27] - integer
> +	 * bit[26:0]  - decimal
> +	 * increment[ns] = perid[ns] * 2^27 => (1ns * 2^27) / rate[hz]
> +	 */
> +
> +	priv->default_addend = div_s64(1000000000LL << 27, rate);
> +
> +	pm_runtime_enable(dev);
> +	pm_runtime_resume_and_get(dev);
> +
> +	iowrite32(priv->default_addend, priv->base + PTPTIVC0_REG);
> +	iowrite32(1, priv->base + PTPTMEC_REG);
> +
> +	priv->clock = ptp_clock_register(&priv->info, dev);
> +	if (IS_ERR(priv->clock)) {
> +		pm_runtime_put_sync(dev);
> +		pm_runtime_disable(dev);
> +		return PTR_ERR(priv->clock);
> +	}
> +
> +	return 0;
> +}
> +
> +static void ptp_rcar_gen4_remove(struct platform_device *pdev)
> +{
> +	struct ptp_rcar_gen4_priv *priv = platform_get_drvdata(pdev);
> +	struct device *dev = &pdev->dev;
> +
> +	ptp_clock_unregister(priv->clock);
> +
> +	iowrite32(1, priv->base + PTPTMDC_REG);
> +
> +	pm_runtime_put_sync(dev);
> +	pm_runtime_disable(dev);
> +}
> +
> +static const struct of_device_id ptp_rcar_gen4_of_match[] = {
> +	{ .compatible = "renesas,rcar-gen4-gptp", },
> +	{ /* Sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, ptp_rcar_gen4_of_match);
> +
> +static struct platform_driver ptp_rcar_gen4_driver = {
> +	.driver = {
> +		.name = "ptp-rcar-gen4",
> +		.of_match_table = ptp_rcar_gen4_of_match,
> +	},
> +	.probe    = ptp_rcar_gen4_probe,
> +	.remove   = ptp_rcar_gen4_remove,
> +};
> +module_platform_driver(ptp_rcar_gen4_driver);
> +
> +MODULE_AUTHOR("Niklas Söderlund");
> +MODULE_DESCRIPTION("Renesas R-Car Gen4 gPTP driver");
> +MODULE_LICENSE("GPL");

[Severity: Medium]
_ptp_rcar_gen4_gettime() reads PTPGPTPTM00 (nanoseconds) and then
PTPGPTPTM10 and PTPGPTPTM20 (seconds). These are three separate MMIO
reads with no capture command and no retry. Does the Gen4 gPTP block
latch the seconds registers when PTPGPTPTM00 is read?

If it does not, a second rollover between the first and second read
pairs a nanosecond value from just before the boundary with the
incremented seconds. The returned time then jumps almost a full
second ahead. The Gen3 ravb code issues GCCR_TCR_CAPTURE before
reading the time registers for this reason.

This matters more in ptp_rcar_gen4_adjtime(). It takes the result of
_ptp_rcar_gen4_gettime(), adds delta, and writes it back with
_ptp_rcar_gen4_settime(). A torn read there would be committed to the
hardware clock, not just returned to one caller.

If the hardware does latch on the PTPGPTPTM00 read, could a comment
document that? If not, would it be enough to read the seconds before
and after the nanosecond read and retry when they differ?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261007185920.4042783-1-niklas.soderlund%2Brenesas%40ragnatech.se

[PATCH net-next v7 3/3] arm64: dts: renesas: r8a779g0: Add gPTP node

[Niklas Söderlund]

The gPTP module is shared between the RAVB and RTSN Ethernet devices on
the SoC.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
* Changes since v2
- Preserve sort order by unit-address.

* Changes since v1
- Rename node ptp.
---
 arch/arm64/boot/dts/renesas/r8a779g0.dtsi | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r8a779g0.dtsi b/arch/arm64/boot/dts/renesas/r8a779g0.dtsi
index 8a291447b904..7430b663cf92 100644
--- a/arch/arm64/boot/dts/renesas/r8a779g0.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a779g0.dtsi
@@ -606,6 +606,15 @@ mfis: system-controller@e6260000 {
 			status = "disabled";
 		};
 
+		gptp: ptp@e6449000 {
+			compatible = "renesas,r8a779g0-gptp", "renesas,rcar-gen4-gptp";
+			reg = <0 0xe6449000 0 0x500>;
+			clocks = <&cpg CPG_MOD 2723>;
+			power-domains = <&sysc R8A779G0_PD_ALWAYS_ON>;
+			resets = <&cpg 2723>;
+			status = "disabled";
+		};
+
 		tsn0: ethernet@e6460000 {
 			compatible = "renesas,r8a779g0-ethertsn", "renesas,rcar-gen4-ethertsn";
 			reg = <0 0xe6460000 0 0x7000>,
-- 
2.55.0

Re: [PATCH net-next v7 3/3] arm64: dts: renesas: r8a779g0: Add gPTP node

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] dts: overlapping memory regions for gptp and tsn0 cause probe failure
- [High] dts: shared reset line between gptp and tsn0 causes probe failure and state corruption
--

commit bea10c72449c59315d90e1e324663f9ca4760bf4
Author: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

arm64: dts: renesas: r8a779g0: Add gPTP node

This commit adds a new gPTP node to the r8a779g0 device tree. The gPTP module
is intended to be shared between the RAVB and RTSN Ethernet devices on the SoC.

> diff --git a/arch/arm64/boot/dts/renesas/r8a779g0.dtsi b/arch/arm64/boot/dts/renesas/r8a779g0.dtsi
> index 8a291447b9045..7430b663cf92e 100644
> --- a/arch/arm64/boot/dts/renesas/r8a779g0.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a779g0.dtsi
> @@ -606,6 +606,15 @@ mfis: system-controller@e6260000 {
>  			status = "disabled";
>  		};
>  
> +		gptp: ptp@e6449000 {
> +			compatible = "renesas,r8a779g0-gptp", "renesas,rcar-gen4-gptp";
> +			reg = <0 0xe6449000 0 0x500>;

[Severity: High]
Does this memory region overlap with the existing tsn0 node?

The tsn0 node independently claims an overlapping memory region starting at
0xe6449000. When both devices are enabled in a board device tree, the driver
probing second will fail with -EBUSY because the kernel resource framework
enforces exclusive access to physical memory regions. This could permanently
disable either the PTP timer or the Ethernet TSN interface.

> +			clocks = <&cpg CPG_MOD 2723>;
> +			power-domains = <&sysc R8A779G0_PD_ALWAYS_ON>;
> +			resets = <&cpg 2723>;

[Severity: High]
Will sharing this reset line cause probe failures or hardware state corruption?

Both the newly added gptp node and the existing tsn0 node specify the same
hardware reset line. The rtsn driver exclusively acquires this reset. The
second driver to initialize will fail to acquire it, leading to a probe failure.

Furthermore, even if the drivers are updated to request a shared reset, the
rtsn driver unconditionally pulses this shared reset line during hardware
initialization:

drivers/net/ethernet/renesas/rtsn.c:rtsn_reset() {
    ...
    reset_control_reset(priv->reset);
    ...
}

Since the gPTP block is shared across multiple Ethernet devices, pulsing this
reset asynchronously wipes out the gPTP timer registers, which could
permanently break PTP synchronization for the system.

> +			status = "disabled";
> +		};
> +
>  		tsn0: ethernet@e6460000 {
>  			compatible = "renesas,r8a779g0-ethertsn", "renesas,rcar-gen4-ethertsn";
>  			reg = <0 0xe6460000 0 0x7000>,

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261007185920.4042783-1-niklas.soderlund+renesas@ragnatech.se?part=3

Re: [PATCH net-next v7 3/3] arm64: dts: renesas: r8a779g0: Add gPTP node

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 1 · Low: 0 (1)

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] dts: shared reset line between gptp and tsn0 causes probe
  failure and state corruption
- [Medium] The new `gptp: ptp@e6449000` node in r8a779g0.dtsi claims `reg
  = <0 0xe6449000 0 0x500>`, plus CPG clock 2723 and CPG reset 2723.

Pre-existing issues:
- [Low] Documentation/devicetree/bindings/ptp/renesas,rcar-gen4-gptp.yaml
  lists `resets` as required, and the new r8a779g0 node supplies `resets…

--

arm64: dts: renesas: r8a779g0: Add gPTP node

This adds a disabled gPTP node at 0xe6449000 to the R-Car V4H (r8a779g0)
SoC dtsi. The node uses CPG module clock and reset 2723 and the always-on
power domain. The commit message says the gPTP module is shared between
the RAVB and RTSN Ethernet devices.

> diff --git a/arch/arm64/boot/dts/renesas/r8a779g0.dtsi b/arch/arm64/boot/dts/renesas/r8a779g0.dtsi
> index 8a291447b9045..7430b663cf92e 100644
> --- a/arch/arm64/boot/dts/renesas/r8a779g0.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a779g0.dtsi
> @@ -606,6 +606,15 @@ mfis: system-controller@e6260000 {
>  			status = "disabled";
>  		};
>  
> +		gptp: ptp@e6449000 {
> +			compatible = "renesas,r8a779g0-gptp", "renesas,rcar-gen4-gptp";
> +			reg = <0 0xe6449000 0 0x500>;

[Severity: Medium]

Does this window overlap with the existing tsn0 node? The tsn0 node just
below is not changed by this patch. It still claims the same range as its
second reg entry and uses the same clock and reset:

	tsn0: ethernet@e6460000 {
		...
		reg = <0 0xe6460000 0 0x7000>,
		      <0 0xe6449000 0 0x500>;
		reg-names = "tsnes", "gptp";
		...
		clocks = <&cpg CPG_MOD 2723>;
		...
		resets = <&cpg 2723>;

Both drivers request this region exclusively. rtsn_probe() in
drivers/net/ethernet/renesas/rtsn.c does:

	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "gptp");
	...
	ptpaddr = devm_ioremap_resource(&pdev->dev, res);

It then registers its own PHC through rcar_gen4_ptp_register().

ptp_rcar_gen4_probe() in drivers/ptp/ptp_rcar_gen4.c does:

	priv->base = devm_platform_ioremap_resource(pdev, 0);

Both paths go through devm_request_mem_region(), so whichever driver
probes second fails with -EBUSY.

white-hawk-single.dtsi already enables tsn0. Suppose a V4H board like that
also enables &gptp so RAVB can reference it through ptp-timer in
ravb_gen4_ptp_probe(). Would one of the two devices then fail to probe?
The gptp node comes first in DT order, so RTSN would usually be the one
that fails, and the TSN Ethernet port would be lost.

The node is disabled and no board enables it yet, so nothing breaks at
this commit.

The commit message says:

  The gPTP module is shared between the RAVB and RTSN Ethernet devices on
  the SoC.

RTSN does not seem to be converted to use the shared node, though. It
still maps the window privately. Should tsn0 and rtsn.c be updated to
reference this node? If not, should the commit message say that enabling
&gptp together with &tsn0 is not supported?

> +			clocks = <&cpg CPG_MOD 2723>;
> +			power-domains = <&sysc R8A779G0_PD_ALWAYS_ON>;
> +			resets = <&cpg 2723>;

[Severity: High]
The existing tsn0 node also uses reset 2723, and rtsn.c treats
that reset as its own. rtsn_probe() takes it with
devm_reset_control_get(&pdev->dev, NULL), which is an exclusive get.
rtsn_reset() then pulses it with reset_control_reset() every time
rtsn_hw_init() runs, which happens whenever the TSN port is brought
up.

This node makes the gPTP block a resource that RAVB can reference
through ptp-timer. Suppose RTSN brings its port up after
ptp_rcar_gen4 has programmed the timer. The reset pulse clears the
gPTP registers (PTPTMEC_REG and the rest) under ptp_rcar_gen4 and
under every RAVB user of that clock. Nothing reprograms them
afterwards, so PTP synchronisation is lost for the whole system.

This also blocks handing the reset to the gPTP driver later.
ptp_rcar_gen4_probe() does not request a reset today, but the
binding requires the property. If the driver ever requests it, the
second exclusive get of the same line fails with -EBUSY, and one of
the two devices will not probe.

The node is disabled, so nothing breaks at this commit. But this
description is what lets both consumers be enabled together. How is
reset 2723 meant to be owned once gptp and tsn0 are both enabled?
Should rtsn.c stop pulsing the shared block when an external gPTP
node is in use, for example by switching to a shared reset that is
only deasserted? Or should tsn0 drop its reset and its gptp reg
window and reference this node instead?

[Severity: Low]

This isn't a bug introduced by this patch, but the
renesas,rcar-gen4-gptp.yaml binding lists resets as required, while
ptp_rcar_gen4_probe() never gets or deasserts a reset control:

	priv->base = devm_platform_ioremap_resource(pdev, 0);
	...
	priv->clk = devm_clk_get(dev, NULL);
	...
	pm_runtime_enable(dev);
	pm_runtime_resume_and_get(dev);

Does the driver depend on the gPTP module already being out of reset when
it probes? Reset 2723 given here is also used by tsn0.

The binding and the driver both come from earlier patches in this series
("dt-bindings: ptp: renesas,rcar-gen4-gptp: Add R-Car Gen4" and "ptp: Add
driver for R-Car Gen4"). Listing the reset in the binding is correct for
describing the hardware.

> +			status = "disabled";
> +		};
> +

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261007185920.4042783-1-niklas.soderlund%2Brenesas%40ragnatech.se