[PATCH 0/3] net: ethernet: renesas: rcar_gen4_ptp: Simplify register layout

[PATCH 0/3] net: ethernet: renesas: rcar_gen4_ptp: Simplify register layout

[Niklas Söderlund]

Hello,

The daughter driver rcar_gen4_ptp used by both rswitch and rtsn where 
upstreamed with support for possible different memory layouts on 
different users. With all Gen4 boards upstream no such setup is 
documented.

There are other issues related to how the rcar_gen4_ptp driver is shared 
between multiple useres that needs to be cleaned up. But that will be a 
larger work. So before that get some simple fixes done.

Patch 1/3 and 2/3 removes the support to allow different register 
layouts on different SoCs by looking up offsets at runtime with a much 
simpler interface. The new interface computes the offsets at compile 
time.

While patch 3/3 is a drive-by patch taking a spurs comment and making a 
lockdep check of it.

There is no intentional functional change in this series just cleaning 
up in preparation of larger works to follow.

Niklas Söderlund (3):
  net: ethernet: renesas: rcar_gen4_ptp: Remove different memory layout
  net: ethernet: renesas: rcar_gen4_ptp: Hide register layout
  net: ethernet: renesas: rcar_gen4_ptp: Use lockdep to verify internal
    usage

 drivers/net/ethernet/renesas/rcar_gen4_ptp.c | 76 ++++++++------------
 drivers/net/ethernet/renesas/rcar_gen4_ptp.h | 33 +--------
 drivers/net/ethernet/renesas/rswitch_main.c  |  3 +-
 drivers/net/ethernet/renesas/rtsn.c          |  3 +-
 4 files changed, 32 insertions(+), 83 deletions(-)

-- 
2.51.0

[PATCH 1/3] net: ethernet: renesas: rcar_gen4_ptp: Remove different memory layout

[Niklas Söderlund]

When upstreaming the Gen4 PTP support for R-Car S4 the possibility for
different memory layouts on other Gen4 SoCs was build in. It turns out
this is not needed and instead needlessly makes the driver harder to
read, remove the support code that would have allowed different memory
layouts.

This change only deals with the public functions used by other drivers,
follow up work will clean up the rcar_gen4_ptp internals.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/net/ethernet/renesas/rcar_gen4_ptp.c | 11 +++--------
 drivers/net/ethernet/renesas/rcar_gen4_ptp.h |  7 +------
 drivers/net/ethernet/renesas/rswitch_main.c  |  3 +--
 drivers/net/ethernet/renesas/rtsn.c          |  3 +--
 4 files changed, 6 insertions(+), 18 deletions(-)

diff --git a/drivers/net/ethernet/renesas/rcar_gen4_ptp.c b/drivers/net/ethernet/renesas/rcar_gen4_ptp.c
index 4c3e8cc5046f..05d0ce193e97 100644
--- a/drivers/net/ethernet/renesas/rcar_gen4_ptp.c
+++ b/drivers/net/ethernet/renesas/rcar_gen4_ptp.c
@@ -130,12 +130,8 @@ static struct ptp_clock_info rcar_gen4_ptp_info = {
 	.enable = rcar_gen4_ptp_enable,
 };
 
-static int rcar_gen4_ptp_set_offs(struct rcar_gen4_ptp_private *ptp_priv,
-				  enum rcar_gen4_ptp_reg_layout layout)
+static int rcar_gen4_ptp_set_offs(struct rcar_gen4_ptp_private *ptp_priv)
 {
-	if (layout != RCAR_GEN4_PTP_REG_LAYOUT)
-		return -EINVAL;
-
 	ptp_priv->offs = &gen4_offs;
 
 	return 0;
@@ -151,8 +147,7 @@ static s64 rcar_gen4_ptp_rate_to_increment(u32 rate)
 	return div_s64(1000000000LL << 27, rate);
 }
 
-int rcar_gen4_ptp_register(struct rcar_gen4_ptp_private *ptp_priv,
-			   enum rcar_gen4_ptp_reg_layout layout, u32 rate)
+int rcar_gen4_ptp_register(struct rcar_gen4_ptp_private *ptp_priv, u32 rate)
 {
 	int ret;
 
@@ -161,7 +156,7 @@ int rcar_gen4_ptp_register(struct rcar_gen4_ptp_private *ptp_priv,
 
 	spin_lock_init(&ptp_priv->lock);
 
-	ret = rcar_gen4_ptp_set_offs(ptp_priv, layout);
+	ret = rcar_gen4_ptp_set_offs(ptp_priv);
 	if (ret)
 		return ret;
 
diff --git a/drivers/net/ethernet/renesas/rcar_gen4_ptp.h b/drivers/net/ethernet/renesas/rcar_gen4_ptp.h
index e22da5acd53d..3343216526fe 100644
--- a/drivers/net/ethernet/renesas/rcar_gen4_ptp.h
+++ b/drivers/net/ethernet/renesas/rcar_gen4_ptp.h
@@ -11,10 +11,6 @@
 
 #define RCAR_GEN4_GPTP_OFFSET_S4	0x00018000
 
-enum rcar_gen4_ptp_reg_layout {
-	RCAR_GEN4_PTP_REG_LAYOUT
-};
-
 /* driver's definitions */
 #define RCAR_GEN4_RXTSTAMP_ENABLED		BIT(0)
 #define RCAR_GEN4_RXTSTAMP_TYPE_V2_L2_EVENT	BIT(1)
@@ -61,8 +57,7 @@ struct rcar_gen4_ptp_private {
 	bool initialized;
 };
 
-int rcar_gen4_ptp_register(struct rcar_gen4_ptp_private *ptp_priv,
-			   enum rcar_gen4_ptp_reg_layout layout, u32 rate);
+int rcar_gen4_ptp_register(struct rcar_gen4_ptp_private *ptp_priv, u32 rate);
 int rcar_gen4_ptp_unregister(struct rcar_gen4_ptp_private *ptp_priv);
 struct rcar_gen4_ptp_private *rcar_gen4_ptp_alloc(struct platform_device *pdev);
 
diff --git a/drivers/net/ethernet/renesas/rswitch_main.c b/drivers/net/ethernet/renesas/rswitch_main.c
index 59dceb81607c..ff5f966c98a9 100644
--- a/drivers/net/ethernet/renesas/rswitch_main.c
+++ b/drivers/net/ethernet/renesas/rswitch_main.c
@@ -2090,8 +2090,7 @@ static int rswitch_init(struct rswitch_private *priv)
 	if (err < 0)
 		goto err_fwd_init;
 
-	err = rcar_gen4_ptp_register(priv->ptp_priv, RCAR_GEN4_PTP_REG_LAYOUT,
-				     clk_get_rate(priv->clk));
+	err = rcar_gen4_ptp_register(priv->ptp_priv, clk_get_rate(priv->clk));
 	if (err < 0)
 		goto err_ptp_register;
 
diff --git a/drivers/net/ethernet/renesas/rtsn.c b/drivers/net/ethernet/renesas/rtsn.c
index 05c4b6c8c9c3..15a043e85431 100644
--- a/drivers/net/ethernet/renesas/rtsn.c
+++ b/drivers/net/ethernet/renesas/rtsn.c
@@ -1330,8 +1330,7 @@ static int rtsn_probe(struct platform_device *pdev)
 
 	device_set_wakeup_capable(&pdev->dev, 1);
 
-	ret = rcar_gen4_ptp_register(priv->ptp_priv, RCAR_GEN4_PTP_REG_LAYOUT,
-				     clk_get_rate(priv->clk));
+	ret = rcar_gen4_ptp_register(priv->ptp_priv, clk_get_rate(priv->clk));
 	if (ret)
 		goto error_pm;
 
-- 
2.51.0

[PATCH 2/3] net: ethernet: renesas: rcar_gen4_ptp: Hide register layout

[Niklas Söderlund]

With the support for multiple register layout removed all support
structures can be removed from the header file. Covert to a simpler
structure using defines for the register offsets.

There is no functional change, only switching from looking up offsets at
runtime to compile time.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/net/ethernet/renesas/rcar_gen4_ptp.c | 66 ++++++++------------
 drivers/net/ethernet/renesas/rcar_gen4_ptp.h | 26 --------
 2 files changed, 26 insertions(+), 66 deletions(-)

diff --git a/drivers/net/ethernet/renesas/rcar_gen4_ptp.c b/drivers/net/ethernet/renesas/rcar_gen4_ptp.c
index 05d0ce193e97..cf13eba9b65e 100644
--- a/drivers/net/ethernet/renesas/rcar_gen4_ptp.c
+++ b/drivers/net/ethernet/renesas/rcar_gen4_ptp.c
@@ -12,20 +12,19 @@
 #include <linux/slab.h>
 
 #include "rcar_gen4_ptp.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
+
 #define ptp_to_priv(ptp)	container_of(ptp, struct rcar_gen4_ptp_private, info)
 
-static const struct rcar_gen4_ptp_reg_offset gen4_offs = {
-	.enable = PTPTMEC,
-	.disable = PTPTMDC,
-	.increment = PTPTIVC0,
-	.config_t0 = PTPTOVC00,
-	.config_t1 = PTPTOVC10,
-	.config_t2 = PTPTOVC20,
-	.monitor_t0 = PTPGPTPTM00,
-	.monitor_t1 = PTPGPTPTM10,
-	.monitor_t2 = PTPGPTPTM20,
-};
-
 static int rcar_gen4_ptp_adjfine(struct ptp_clock_info *ptp, long scaled_ppm)
 {
 	struct rcar_gen4_ptp_private *ptp_priv = ptp_to_priv(ptp);
@@ -38,7 +37,7 @@ static int rcar_gen4_ptp_adjfine(struct ptp_clock_info *ptp, long scaled_ppm)
 	diff = div_s64(addend * scaled_ppm_to_ppb(scaled_ppm), NSEC_PER_SEC);
 	addend = neg_adj ? addend - diff : addend + diff;
 
-	iowrite32(addend, ptp_priv->addr + ptp_priv->offs->increment);
+	iowrite32(addend, ptp_priv->addr + PTPTIVC0_REG);
 
 	return 0;
 }
@@ -49,9 +48,9 @@ static void _rcar_gen4_ptp_gettime(struct ptp_clock_info *ptp,
 {
 	struct rcar_gen4_ptp_private *ptp_priv = ptp_to_priv(ptp);
 
-	ts->tv_nsec = ioread32(ptp_priv->addr + ptp_priv->offs->monitor_t0);
-	ts->tv_sec = ioread32(ptp_priv->addr + ptp_priv->offs->monitor_t1) |
-		     ((s64)ioread32(ptp_priv->addr + ptp_priv->offs->monitor_t2) << 32);
+	ts->tv_nsec = ioread32(ptp_priv->addr + PTPGPTPTM00_REG);
+	ts->tv_sec = ioread32(ptp_priv->addr + PTPGPTPTM10_REG) |
+		     ((s64)ioread32(ptp_priv->addr + PTPGPTPTM20_REG) << 32);
 }
 
 static int rcar_gen4_ptp_gettime(struct ptp_clock_info *ptp,
@@ -73,14 +72,14 @@ static void _rcar_gen4_ptp_settime(struct ptp_clock_info *ptp,
 {
 	struct rcar_gen4_ptp_private *ptp_priv = ptp_to_priv(ptp);
 
-	iowrite32(1, ptp_priv->addr + ptp_priv->offs->disable);
-	iowrite32(0, ptp_priv->addr + ptp_priv->offs->config_t2);
-	iowrite32(0, ptp_priv->addr + ptp_priv->offs->config_t1);
-	iowrite32(0, ptp_priv->addr + ptp_priv->offs->config_t0);
-	iowrite32(1, ptp_priv->addr + ptp_priv->offs->enable);
-	iowrite32(ts->tv_sec >> 32, ptp_priv->addr + ptp_priv->offs->config_t2);
-	iowrite32(ts->tv_sec, ptp_priv->addr + ptp_priv->offs->config_t1);
-	iowrite32(ts->tv_nsec, ptp_priv->addr + ptp_priv->offs->config_t0);
+	iowrite32(1, ptp_priv->addr + PTPTMDC_REG);
+	iowrite32(0, ptp_priv->addr + PTPTOVC20_REG);
+	iowrite32(0, ptp_priv->addr + PTPTOVC10_REG);
+	iowrite32(0, ptp_priv->addr + PTPTOVC00_REG);
+	iowrite32(1, ptp_priv->addr + PTPTMEC_REG);
+	iowrite32(ts->tv_sec >> 32, ptp_priv->addr + PTPTOVC20_REG);
+	iowrite32(ts->tv_sec, ptp_priv->addr + PTPTOVC10_REG);
+	iowrite32(ts->tv_nsec, ptp_priv->addr + PTPTOVC00_REG);
 }
 
 static int rcar_gen4_ptp_settime(struct ptp_clock_info *ptp,
@@ -130,13 +129,6 @@ static struct ptp_clock_info rcar_gen4_ptp_info = {
 	.enable = rcar_gen4_ptp_enable,
 };
 
-static int rcar_gen4_ptp_set_offs(struct rcar_gen4_ptp_private *ptp_priv)
-{
-	ptp_priv->offs = &gen4_offs;
-
-	return 0;
-}
-
 static s64 rcar_gen4_ptp_rate_to_increment(u32 rate)
 {
 	/* Timer increment in ns.
@@ -149,24 +141,18 @@ static s64 rcar_gen4_ptp_rate_to_increment(u32 rate)
 
 int rcar_gen4_ptp_register(struct rcar_gen4_ptp_private *ptp_priv, u32 rate)
 {
-	int ret;
-
 	if (ptp_priv->initialized)
 		return 0;
 
 	spin_lock_init(&ptp_priv->lock);
 
-	ret = rcar_gen4_ptp_set_offs(ptp_priv);
-	if (ret)
-		return ret;
-
 	ptp_priv->default_addend = rcar_gen4_ptp_rate_to_increment(rate);
-	iowrite32(ptp_priv->default_addend, ptp_priv->addr + ptp_priv->offs->increment);
+	iowrite32(ptp_priv->default_addend, ptp_priv->addr + PTPTIVC0_REG);
 	ptp_priv->clock = ptp_clock_register(&ptp_priv->info, NULL);
 	if (IS_ERR(ptp_priv->clock))
 		return PTR_ERR(ptp_priv->clock);
 
-	iowrite32(0x01, ptp_priv->addr + ptp_priv->offs->enable);
+	iowrite32(0x01, ptp_priv->addr + PTPTMEC_REG);
 	ptp_priv->initialized = true;
 
 	return 0;
@@ -175,7 +161,7 @@ EXPORT_SYMBOL_GPL(rcar_gen4_ptp_register);
 
 int rcar_gen4_ptp_unregister(struct rcar_gen4_ptp_private *ptp_priv)
 {
-	iowrite32(1, ptp_priv->addr + ptp_priv->offs->disable);
+	iowrite32(1, ptp_priv->addr + PTPTMDC_REG);
 
 	return ptp_clock_unregister(ptp_priv->clock);
 }
diff --git a/drivers/net/ethernet/renesas/rcar_gen4_ptp.h b/drivers/net/ethernet/renesas/rcar_gen4_ptp.h
index 3343216526fe..f77e79e47357 100644
--- a/drivers/net/ethernet/renesas/rcar_gen4_ptp.h
+++ b/drivers/net/ethernet/renesas/rcar_gen4_ptp.h
@@ -19,37 +19,11 @@
 
 #define RCAR_GEN4_TXTSTAMP_ENABLED		BIT(0)
 
-#define PTPRO				0
-
-enum rcar_gen4_ptp_reg {
-	PTPTMEC		= PTPRO + 0x0010,
-	PTPTMDC		= PTPRO + 0x0014,
-	PTPTIVC0	= PTPRO + 0x0020,
-	PTPTOVC00	= PTPRO + 0x0030,
-	PTPTOVC10	= PTPRO + 0x0034,
-	PTPTOVC20	= PTPRO + 0x0038,
-	PTPGPTPTM00	= PTPRO + 0x0050,
-	PTPGPTPTM10	= PTPRO + 0x0054,
-	PTPGPTPTM20	= PTPRO + 0x0058,
-};
-
-struct rcar_gen4_ptp_reg_offset {
-	u16 enable;
-	u16 disable;
-	u16 increment;
-	u16 config_t0;
-	u16 config_t1;
-	u16 config_t2;
-	u16 monitor_t0;
-	u16 monitor_t1;
-	u16 monitor_t2;
-};
 
 struct rcar_gen4_ptp_private {
 	void __iomem *addr;
 	struct ptp_clock *clock;
 	struct ptp_clock_info info;
-	const struct rcar_gen4_ptp_reg_offset *offs;
 	spinlock_t lock;	/* For multiple registers access */
 	u32 tstamp_tx_ctrl;
 	u32 tstamp_rx_ctrl;
-- 
2.51.0

[PATCH 3/3] net: ethernet: renesas: rcar_gen4_ptp: Use lockdep to verify internal usage

[Niklas Söderlund]

Instead of a having a comment that the lock must be held when calling
the internal helper add a lockdep check to enforce it.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/net/ethernet/renesas/rcar_gen4_ptp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/renesas/rcar_gen4_ptp.c b/drivers/net/ethernet/renesas/rcar_gen4_ptp.c
index cf13eba9b65e..d0979abd36de 100644
--- a/drivers/net/ethernet/renesas/rcar_gen4_ptp.c
+++ b/drivers/net/ethernet/renesas/rcar_gen4_ptp.c
@@ -42,12 +42,13 @@ static int rcar_gen4_ptp_adjfine(struct ptp_clock_info *ptp, long scaled_ppm)
 	return 0;
 }
 
-/* Caller must hold the lock */
 static void _rcar_gen4_ptp_gettime(struct ptp_clock_info *ptp,
 				   struct timespec64 *ts)
 {
 	struct rcar_gen4_ptp_private *ptp_priv = ptp_to_priv(ptp);
 
+	lockdep_assert_held(&ptp_priv->lock);
+
 	ts->tv_nsec = ioread32(ptp_priv->addr + PTPGPTPTM00_REG);
 	ts->tv_sec = ioread32(ptp_priv->addr + PTPGPTPTM10_REG) |
 		     ((s64)ioread32(ptp_priv->addr + PTPGPTPTM20_REG) << 32);
-- 
2.51.0

Re: [PATCH 1/3] net: ethernet: renesas: rcar_gen4_ptp: Remove different memory layout

[Simon Horman]

On Mon, Sep 08, 2025 at 05:44:24PM +0200, Niklas Söderlund wrote:
> When upstreaming the Gen4 PTP support for R-Car S4 the possibility for
> different memory layouts on other Gen4 SoCs was build in. It turns out
> this is not needed and instead needlessly makes the driver harder to
> read, remove the support code that would have allowed different memory
> layouts.
> 
> This change only deals with the public functions used by other drivers,
> follow up work will clean up the rcar_gen4_ptp internals.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH 2/3] net: ethernet: renesas: rcar_gen4_ptp: Hide register layout

[Simon Horman]

On Mon, Sep 08, 2025 at 05:44:25PM +0200, Niklas Söderlund wrote:
> With the support for multiple register layout removed all support
> structures can be removed from the header file. Covert to a simpler
> structure using defines for the register offsets.
> 
> There is no functional change, only switching from looking up offsets at
> runtime to compile time.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH 3/3] net: ethernet: renesas: rcar_gen4_ptp: Use lockdep to verify internal usage

[Simon Horman]

On Mon, Sep 08, 2025 at 05:44:26PM +0200, Niklas Söderlund wrote:
> Instead of a having a comment that the lock must be held when calling
> the internal helper add a lockdep check to enforce it.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH 0/3] net: ethernet: renesas: rcar_gen4_ptp: Simplify register layout

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Mon,  8 Sep 2025 17:44:23 +0200 you wrote:
> Hello,
> 
> The daughter driver rcar_gen4_ptp used by both rswitch and rtsn where
> upstreamed with support for possible different memory layouts on
> different users. With all Gen4 boards upstream no such setup is
> documented.
> 
> [...]

Here is the summary with links:
  - [1/3] net: ethernet: renesas: rcar_gen4_ptp: Remove different memory layout
    https://git.kernel.org/netdev/net-next/c/4da47931a924
  - [2/3] net: ethernet: renesas: rcar_gen4_ptp: Hide register layout
    https://git.kernel.org/netdev/net-next/c/492d816b1793
  - [3/3] net: ethernet: renesas: rcar_gen4_ptp: Use lockdep to verify internal usage
    https://git.kernel.org/netdev/net-next/c/fd2b2429fbc8

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html