[PATCH master 0/4] ARM: socfpga: add HANDOFF_FPGA2SDR handling

[PATCH master 0/4] ARM: socfpga: add HANDOFF_FPGA2SDR handling

[Ahmad Fatoum]

The Quartus headers for Cyclone V can contain a reset mask for the
fpga2sdram bridge, which was so far ignored. This series remedies that.

Ahmad Fatoum (4):
  ARM: socfpga: complete definitions of handoff registers
  ARM: socfpga: populate HANDOFF_FPGA2SDR with Quartus value
  fpga: socfpga-fpga2sdram-bridge: consult HANDOFF_FPGA2SDR register
  fpga: socfpga-fpga2sdram-bridge: always populate HANDOFF_FPGA2SDR
    register

 drivers/fpga/socfpga-fpga2sdram-bridge.c       | 13 ++++++++-----
 include/mach/socfpga/cyclone5-sdram-config.h   |  3 +++
 include/mach/socfpga/cyclone5-system-manager.h | 13 +++++++++----
 3 files changed, 20 insertions(+), 9 deletions(-)

-- 
2.39.2

[PATCH master 1/4] ARM: socfpga: complete definitions of handoff registers

[Ahmad Fatoum]

The four ISWGRP_HANDOFF_* macro definitions expand to undefined macros.
In preparation for using one of them, fix them so they expand to the
address of the respective handoff register.

No functional change as the macros are yet unused.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 include/mach/socfpga/cyclone5-system-manager.h | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/include/mach/socfpga/cyclone5-system-manager.h b/include/mach/socfpga/cyclone5-system-manager.h
index 7cec60937b84..341eaaac7e53 100644
--- a/include/mach/socfpga/cyclone5-system-manager.h
+++ b/include/mach/socfpga/cyclone5-system-manager.h
@@ -28,10 +28,15 @@ void socfpga_sysmgr_pinmux_init(unsigned long *sys_mgr_init_table, int num);
 /* EMAC interface selection */
 #define CONFIG_SYSMGR_EMAC_CTRL		(CYCLONE5_SYSMGR_ADDRESS + 0x60)
 
-#define ISWGRP_HANDOFF_AXIBRIDGE	SYSMGR_ISWGRP_HANDOFF0
-#define ISWGRP_HANDOFF_L3REMAP		SYSMGR_ISWGRP_HANDOFF1
-#define ISWGRP_HANDOFF_FPGAINTF		SYSMGR_ISWGRP_HANDOFF2
-#define ISWGRP_HANDOFF_FPGA2SDR		SYSMGR_ISWGRP_HANDOFF3
+#define SYSMGR_ISWGRP_HANDOFF		0x80
+
+#define SYSMGR_ISWGRP_HANDOFF_ADDR(i)	\
+	IOMEM(CYCLONE5_SYSMGR_ADDRESS + SYSMGR_ISWGRP_HANDOFF + ((i) * sizeof(u32)))
+
+#define ISWGRP_HANDOFF_AXIBRIDGE	SYSMGR_ISWGRP_HANDOFF_ADDR(0)
+#define ISWGRP_HANDOFF_L3REMAP		SYSMGR_ISWGRP_HANDOFF_ADDR(1)
+#define ISWGRP_HANDOFF_FPGAINTF		SYSMGR_ISWGRP_HANDOFF_ADDR(2)
+#define ISWGRP_HANDOFF_FPGA2SDR		SYSMGR_ISWGRP_HANDOFF_ADDR(3)
 
 /* pin mux */
 #define SYSMGR_PINMUXGRP		(CYCLONE5_SYSMGR_ADDRESS + 0x400)
-- 
2.39.2

[PATCH master 2/4] ARM: socfpga: populate HANDOFF_FPGA2SDR with Quartus value

[Ahmad Fatoum]

The Terasic DE10-Nano is the only upstream board that has a non-zero
value for CONFIG_HPS_SDR_CTRLCFG_FPGAPORTRST in the headers generated by
Quartus. Yet, barebox ignored this value, with the effect that either:

  - The Linux bridge driver reads a zero value from the handoff3 register
    and keeps all fpga2sdram ports in reset

  - The barebox bridge driver uses its hardcoded mask of 0x3fff, when
    the bridge is enabled, which is different than the Quartus value
    of 0x1FF.

Fix the first point by populating the handoff3 register, so an enabled
Linux driver can enable the correct ports. A fix for barebox configurations
with CONFIG_SOCFPGA_FPGA_BRIDGE=y follows in the follow-up commit.

Cc: Gwenhael Goavec-Merou <gwenhael.goavec-merou@trabucayre.com>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 include/mach/socfpga/cyclone5-sdram-config.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/mach/socfpga/cyclone5-sdram-config.h b/include/mach/socfpga/cyclone5-sdram-config.h
index 06f06ef5d78f..2abef7f31121 100644
--- a/include/mach/socfpga/cyclone5-sdram-config.h
+++ b/include/mach/socfpga/cyclone5-sdram-config.h
@@ -155,6 +155,9 @@ static inline void socfpga_sdram_mmr_init(void)
 		CONFIG_HPS_SDR_CTRLCFG_DRAMODT_WRITE << SDR_CTRLGRP_DRAMODT_WRITE_LSB;
 	sdram_write(SDR_CTRLGRP_DRAMODT_ADDRESS, val);
 
+	val = CONFIG_HPS_SDR_CTRLCFG_FPGAPORTRST;
+	writel(val, ISWGRP_HANDOFF_FPGA2SDR);
+
 	val = readl(CYCLONE5_SDR_ADDRESS + SDR_CTRLGRP_STATICCFG_ADDRESS);
 	val &= ~(SDR_CTRLGRP_STATICCFG_APPLYCFG_MASK);
 	val |= 1 << SDR_CTRLGRP_STATICCFG_APPLYCFG_LSB;
-- 
2.39.2

[PATCH master 3/4] fpga: socfpga-fpga2sdram-bridge: consult HANDOFF_FPGA2SDR register

[Ahmad Fatoum]

Previous commit started populating the handoff register, which the
Linux SoCFPA FPGA2SDRAM bridge driver would use to bring the correct
SDRAM ports on the bridge out of reset.

An alternative is skipping the Linux driver and doing setup in barebox.
So far, we just enabled all ports, but this can lead to discrepancy
between barebox and Linux if both drivers are enabled.

This is the case for the upstream Terasic DE10-Nano, which specifies a
mask of 0x1ff in its generated Quartus headers, which is narrower than
the maximum possible bitmask of 0x3fff.

Therefore, let's start using the handoff register in barebox as well.
At probe time, the register is currently only populated on Cyclone V SoCs,
so arria10 will just read a zero and fallback to the old behavior of writing
0x3fff.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/fpga/socfpga-fpga2sdram-bridge.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/fpga/socfpga-fpga2sdram-bridge.c b/drivers/fpga/socfpga-fpga2sdram-bridge.c
index 202dfaa82b5e..961a083ae512 100644
--- a/drivers/fpga/socfpga-fpga2sdram-bridge.c
+++ b/drivers/fpga/socfpga-fpga2sdram-bridge.c
@@ -106,8 +106,11 @@ static int alt_fpga_bridge_probe(struct device *dev)
 	if (!priv)
 		return -ENOMEM;
 
-	/* enable all ports for now */
-	priv->mask = ALT_SDR_CTL_FPGAPORTRST_PORTRSTN_MSK;
+	priv->mask = readl(SOCFPGA_SYSMGR_ADDR + SYSMGR_ISWGRP_HANDOFF3);
+	if (!priv->mask) {
+		/* enable all ports if we don't know better */
+		priv->mask = ALT_SDR_CTL_FPGAPORTRST_PORTRSTN_MSK;
+	}
 
 	priv->dev = dev;
 
-- 
2.39.2

[PATCH master 4/4] fpga: socfpga-fpga2sdram-bridge: always populate HANDOFF_FPGA2SDR register

[Ahmad Fatoum]

The driver will take care to write the handoff register, when the bridge
is first enabled or disabled. In the case that the driver merely probes
the device and no explicit action is taken, the handoff value is never
written. This is evident with a look at the console:

barebox output:

  altera-fpga2sdram-bridge ffc25080.fpga-bridge@ffc25080.of:
    driver initialized with handoff 00003fff
                                        ^^^^
Linux output:

  altera_fpga2sdram_bridge ffc25080.fpga-bridge:
    driver initialized with handoff 00000000
                                        ^^^^

The result of this that the Linux driver must be disabled, otherwise, it
will just reset the same bridges that barebox had previously moved out
of reset. Let's get rid of this discrepancy by moving the write to the
handoff value into the probe function.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/fpga/socfpga-fpga2sdram-bridge.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/fpga/socfpga-fpga2sdram-bridge.c b/drivers/fpga/socfpga-fpga2sdram-bridge.c
index 961a083ae512..7f72bd8a6536 100644
--- a/drivers/fpga/socfpga-fpga2sdram-bridge.c
+++ b/drivers/fpga/socfpga-fpga2sdram-bridge.c
@@ -68,9 +68,6 @@ static inline int _alt_fpga2sdram_enable_set(struct alt_fpga2sdram_data *priv,
 	else
 		val = 0;
 
-	/* The kernel driver expects this value in this register :-( */
-	writel(priv->mask, SOCFPGA_SYSMGR_ADDR + SYSMGR_ISWGRP_HANDOFF3);
-
 	dev_dbg(priv->dev, "setting fpgaportrst to 0x%08x\n", val);
 
 	return writel(val, SOCFPGA_SDRCTL_ADDR + ALT_SDR_CTL_FPGAPORTRST_OFST);
@@ -110,6 +107,9 @@ static int alt_fpga_bridge_probe(struct device *dev)
 	if (!priv->mask) {
 		/* enable all ports if we don't know better */
 		priv->mask = ALT_SDR_CTL_FPGAPORTRST_PORTRSTN_MSK;
+		/* The kernel driver expects this value in this register :-( */
+		writel(priv->mask, SOCFPGA_SYSMGR_ADDR + SYSMGR_ISWGRP_HANDOFF3);
+
 	}
 
 	priv->dev = dev;
-- 
2.39.2

Re: [PATCH master 0/4] ARM: socfpga: add HANDOFF_FPGA2SDR handling

[Sascha Hauer]

On Wed, 31 Jan 2024 23:59:34 +0100, Ahmad Fatoum wrote:
> The Quartus headers for Cyclone V can contain a reset mask for the
> fpga2sdram bridge, which was so far ignored. This series remedies that.
> 
> Ahmad Fatoum (4):
>   ARM: socfpga: complete definitions of handoff registers
>   ARM: socfpga: populate HANDOFF_FPGA2SDR with Quartus value
>   fpga: socfpga-fpga2sdram-bridge: consult HANDOFF_FPGA2SDR register
>   fpga: socfpga-fpga2sdram-bridge: always populate HANDOFF_FPGA2SDR
>     register
> 
> [...]

Applied, thanks!

[1/4] ARM: socfpga: complete definitions of handoff registers
      https://git.pengutronix.de/cgit/barebox/commit/?id=c23d507559f4 (link may not be stable)
[2/4] ARM: socfpga: populate HANDOFF_FPGA2SDR with Quartus value
      https://git.pengutronix.de/cgit/barebox/commit/?id=27f68143f533 (link may not be stable)
[3/4] fpga: socfpga-fpga2sdram-bridge: consult HANDOFF_FPGA2SDR register
      https://git.pengutronix.de/cgit/barebox/commit/?id=981214989220 (link may not be stable)
[4/4] fpga: socfpga-fpga2sdram-bridge: always populate HANDOFF_FPGA2SDR register
      https://git.pengutronix.de/cgit/barebox/commit/?id=ec3fa0f48cfc (link may not be stable)

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>