All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 0/5] arm: socfpga: implement proper peripheral reset handling
@ 2019-01-25 20:30 Simon Goldschmidt
  2019-01-25 20:30 ` [U-Boot] [PATCH 1/5] arm: socfpga: gen5: add reset handles to devicetrees Simon Goldschmidt
                   ` (4 more replies)
  0 siblings, 5 replies; 35+ messages in thread
From: Simon Goldschmidt @ 2019-01-25 20:30 UTC (permalink / raw)
  To: u-boot

This series implements peripheral reset handling for socfpga gen5.

It moves from enabling all peripherals during SPL startup to using the
socfpga reset driver from all peripherals and enabling peripherals when
they are used.

As Linux cannot even handle this in 4.20, a Kconfig option is added to
keep the old behaviour of just enabling all peripherals from SPL. This
option is default 'y' for now but should be moved to default 'n' in the
near future once a Linux kernel supports reset handling for all drivers.


Simon Goldschmidt (5):
  arm: socfpga: gen5: add reset handles to devicetrees
  arm: socfpga: move SDR reset handling to driver
  mtd: rawnand: denali: add reset handling
  spi: cadence_qspi: add reset handling
  arm: socfpga: implement proper peripheral reset

 arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi |  6 ++
 arch/arm/dts/socfpga_cyclone5_dbm_soc1.dts    |  5 ++
 .../socfpga_cyclone5_de0_nano_soc-u-boot.dtsi |  6 ++
 arch/arm/dts/socfpga_cyclone5_de10_nano.dts   |  5 ++
 arch/arm/dts/socfpga_cyclone5_de1_soc.dts     |  5 ++
 arch/arm/dts/socfpga_cyclone5_is1.dts         |  5 ++
 .../dts/socfpga_cyclone5_socdk-u-boot.dtsi    |  6 ++
 .../dts/socfpga_cyclone5_sockit-u-boot.dtsi   |  6 ++
 .../dts/socfpga_cyclone5_socrates-u-boot.dtsi | 10 +++
 arch/arm/dts/socfpga_cyclone5_sr1500.dts      |  5 ++
 .../socfpga_cyclone5_vining_fpga-u-boot.dtsi  |  6 ++
 arch/arm/dts/socfpga_u-boot.dtsi              | 61 +++++++++++++++++++
 arch/arm/mach-socfpga/Kconfig                 | 10 +++
 arch/arm/mach-socfpga/misc_gen5.c             |  2 +
 arch/arm/mach-socfpga/spl_gen5.c              | 11 +++-
 drivers/ddr/altera/sdram_gen5.c               |  4 ++
 drivers/mtd/nand/raw/denali_dt.c              |  8 +++
 drivers/spi/cadence_qspi.c                    |  9 +++
 18 files changed, 169 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/dts/socfpga_u-boot.dtsi

-- 
2.17.1

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [U-Boot] [PATCH 1/5] arm: socfpga: gen5: add reset handles to devicetrees
  2019-01-25 20:30 [U-Boot] [PATCH 0/5] arm: socfpga: implement proper peripheral reset handling Simon Goldschmidt
@ 2019-01-25 20:30 ` Simon Goldschmidt
  2019-01-26  8:57   ` Marek Vasut
  2019-01-25 20:30 ` [U-Boot] [PATCH 2/5] arm: socfpga: move SDR reset handling to driver Simon Goldschmidt
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 35+ messages in thread
From: Simon Goldschmidt @ 2019-01-25 20:30 UTC (permalink / raw)
  To: u-boot

The SPL for socfpga gen5 currently takes all peripherals out of reset
unconditionally. To implement proper reset handling for peripherals,
 some device tree nodes are missing reset handles.

Since the socfpga gen5 devicetrees are currently in sync with Linux 4.20,
and these reset handles are not added to Linux, yet, let's add them to
U-Boot specific dtsi files to ease future merging.

This adds a new file socfpga_u-boot.dtsi (note the '_' - if using a '-'
this file gets pulled in for some a10/s10 devicetrees) that adds the
missing reset handles.

Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
---

 arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi |  6 ++
 arch/arm/dts/socfpga_cyclone5_dbm_soc1.dts    |  5 ++
 .../socfpga_cyclone5_de0_nano_soc-u-boot.dtsi |  6 ++
 arch/arm/dts/socfpga_cyclone5_de10_nano.dts   |  5 ++
 arch/arm/dts/socfpga_cyclone5_de1_soc.dts     |  5 ++
 arch/arm/dts/socfpga_cyclone5_is1.dts         |  5 ++
 .../dts/socfpga_cyclone5_socdk-u-boot.dtsi    |  6 ++
 .../dts/socfpga_cyclone5_sockit-u-boot.dtsi   |  6 ++
 .../dts/socfpga_cyclone5_socrates-u-boot.dtsi | 10 +++
 arch/arm/dts/socfpga_cyclone5_sr1500.dts      |  5 ++
 .../socfpga_cyclone5_vining_fpga-u-boot.dtsi  |  6 ++
 arch/arm/dts/socfpga_u-boot.dtsi              | 61 +++++++++++++++++++
 12 files changed, 126 insertions(+)
 create mode 100644 arch/arm/dts/socfpga_u-boot.dtsi

diff --git a/arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi b/arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi
index c44d1ee2fa..b4cd73d367 100644
--- a/arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi
+++ b/arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi
@@ -6,6 +6,8 @@
  * Copyright (c) 2018 Simon Goldschmidt
  */
 
+#include "socfpga_u-boot.dtsi"
+
 /{
 	aliases {
 		spi0 = "/soc/spi at ff705000";
@@ -17,6 +19,10 @@
 	};
 };
 
+&rst {
+	u-boot,dm-pre-reloc;
+};
+
 &watchdog0 {
 	status = "disabled";
 };
diff --git a/arch/arm/dts/socfpga_cyclone5_dbm_soc1.dts b/arch/arm/dts/socfpga_cyclone5_dbm_soc1.dts
index a387071674..f616740981 100644
--- a/arch/arm/dts/socfpga_cyclone5_dbm_soc1.dts
+++ b/arch/arm/dts/socfpga_cyclone5_dbm_soc1.dts
@@ -4,6 +4,7 @@
  */
 
 #include "socfpga_cyclone5.dtsi"
+#include "socfpga_u-boot.dtsi"
 
 / {
 	model = "Devboards.de DBM-SoC1";
@@ -30,6 +31,10 @@
 	};
 };
 
+&rst {
+	u-boot,dm-pre-reloc;
+};
+
 &gmac1 {
 	status = "okay";
 	phy-mode = "rgmii";
diff --git a/arch/arm/dts/socfpga_cyclone5_de0_nano_soc-u-boot.dtsi b/arch/arm/dts/socfpga_cyclone5_de0_nano_soc-u-boot.dtsi
index 08d81da169..63dd0de23b 100644
--- a/arch/arm/dts/socfpga_cyclone5_de0_nano_soc-u-boot.dtsi
+++ b/arch/arm/dts/socfpga_cyclone5_de0_nano_soc-u-boot.dtsi
@@ -6,6 +6,8 @@
  * Copyright (c) 2018 Simon Goldschmidt
  */
 
+#include "socfpga_u-boot.dtsi"
+
 /{
 	aliases {
 		udc0 = &usb1;
@@ -16,6 +18,10 @@
 	};
 };
 
+&rst {
+	u-boot,dm-pre-reloc;
+};
+
 &watchdog0 {
 	status = "disabled";
 };
diff --git a/arch/arm/dts/socfpga_cyclone5_de10_nano.dts b/arch/arm/dts/socfpga_cyclone5_de10_nano.dts
index e9105743ea..7106b0300a 100644
--- a/arch/arm/dts/socfpga_cyclone5_de10_nano.dts
+++ b/arch/arm/dts/socfpga_cyclone5_de10_nano.dts
@@ -6,6 +6,7 @@
  */
 
 #include "socfpga_cyclone5.dtsi"
+#include "socfpga_u-boot.dtsi"
 
 / {
 	model = "Terasic DE10-Nano";
@@ -32,6 +33,10 @@
 	};
 };
 
+&rst {
+	u-boot,dm-pre-reloc;
+};
+
 &gmac1 {
 	status = "okay";
 	phy-mode = "rgmii";
diff --git a/arch/arm/dts/socfpga_cyclone5_de1_soc.dts b/arch/arm/dts/socfpga_cyclone5_de1_soc.dts
index 4f076bce93..5d426dad68 100644
--- a/arch/arm/dts/socfpga_cyclone5_de1_soc.dts
+++ b/arch/arm/dts/socfpga_cyclone5_de1_soc.dts
@@ -4,6 +4,7 @@
  */
 
 #include "socfpga_cyclone5.dtsi"
+#include "socfpga_u-boot.dtsi"
 
 / {
 	model = "Terasic DE1-SoC";
@@ -30,6 +31,10 @@
 	};
 };
 
+&rst {
+	u-boot,dm-pre-reloc;
+};
+
 &gmac1 {
 	status = "okay";
 	phy-mode = "rgmii";
diff --git a/arch/arm/dts/socfpga_cyclone5_is1.dts b/arch/arm/dts/socfpga_cyclone5_is1.dts
index b7054bfd5a..3383754cb4 100644
--- a/arch/arm/dts/socfpga_cyclone5_is1.dts
+++ b/arch/arm/dts/socfpga_cyclone5_is1.dts
@@ -4,6 +4,7 @@
  */
 
 #include "socfpga_cyclone5.dtsi"
+#include "socfpga_u-boot.dtsi"
 
 / {
 	model = "SoCFPGA Cyclone V IS1";
@@ -37,6 +38,10 @@
 	};
 };
 
+&rst {
+	u-boot,dm-pre-reloc;
+};
+
 &gmac1 {
 	status = "okay";
 	phy-mode = "rgmii";
diff --git a/arch/arm/dts/socfpga_cyclone5_socdk-u-boot.dtsi b/arch/arm/dts/socfpga_cyclone5_socdk-u-boot.dtsi
index 9436e0fa8b..0c39017d55 100644
--- a/arch/arm/dts/socfpga_cyclone5_socdk-u-boot.dtsi
+++ b/arch/arm/dts/socfpga_cyclone5_socdk-u-boot.dtsi
@@ -6,6 +6,8 @@
  * Copyright (c) 2018 Simon Goldschmidt
  */
 
+#include "socfpga_u-boot.dtsi"
+
 /{
 	aliases {
 		spi0 = "/soc/spi at ff705000";
@@ -17,6 +19,10 @@
 	};
 };
 
+&rst {
+	u-boot,dm-pre-reloc;
+};
+
 &can0 {
 	status = "okay";
 };
diff --git a/arch/arm/dts/socfpga_cyclone5_sockit-u-boot.dtsi b/arch/arm/dts/socfpga_cyclone5_sockit-u-boot.dtsi
index 648f1bd01d..284e19a66f 100644
--- a/arch/arm/dts/socfpga_cyclone5_sockit-u-boot.dtsi
+++ b/arch/arm/dts/socfpga_cyclone5_sockit-u-boot.dtsi
@@ -6,6 +6,8 @@
  * Copyright (c) 2018 Simon Goldschmidt
  */
 
+#include "socfpga_u-boot.dtsi"
+
 /{
 	aliases {
 		spi0 = "/soc/spi at ff705000";
@@ -17,6 +19,10 @@
 	};
 };
 
+&rst {
+	u-boot,dm-pre-reloc;
+};
+
 &watchdog0 {
 	status = "disabled";
 };
diff --git a/arch/arm/dts/socfpga_cyclone5_socrates-u-boot.dtsi b/arch/arm/dts/socfpga_cyclone5_socrates-u-boot.dtsi
index 31bd1dba0f..4a0ca813cb 100644
--- a/arch/arm/dts/socfpga_cyclone5_socrates-u-boot.dtsi
+++ b/arch/arm/dts/socfpga_cyclone5_socrates-u-boot.dtsi
@@ -6,6 +6,8 @@
  * Copyright (c) 2018 Simon Goldschmidt
  */
 
+#include "socfpga_u-boot.dtsi"
+
 /{
 	aliases {
 		spi0 = "/soc/spi at ff705000";
@@ -17,10 +19,18 @@
 	};
 };
 
+&rst {
+	u-boot,dm-pre-reloc;
+};
+
 &watchdog0 {
 	status = "disabled";
 };
 
+&rst {
+	u-boot,dm-pre-reloc;
+};
+
 &mmc {
 	u-boot,dm-pre-reloc;
 };
diff --git a/arch/arm/dts/socfpga_cyclone5_sr1500.dts b/arch/arm/dts/socfpga_cyclone5_sr1500.dts
index 6a6c29be79..0025baf29e 100644
--- a/arch/arm/dts/socfpga_cyclone5_sr1500.dts
+++ b/arch/arm/dts/socfpga_cyclone5_sr1500.dts
@@ -4,6 +4,7 @@
  */
 
 #include "socfpga_cyclone5.dtsi"
+#include "socfpga_u-boot.dtsi"
 
 / {
 	model = "SoCFPGA Cyclone V SR1500";
@@ -33,6 +34,10 @@
 	};
 };
 
+&rst {
+	u-boot,dm-pre-reloc;
+};
+
 &gmac1 {
 	status = "okay";
 	phy-mode = "rgmii";
diff --git a/arch/arm/dts/socfpga_cyclone5_vining_fpga-u-boot.dtsi b/arch/arm/dts/socfpga_cyclone5_vining_fpga-u-boot.dtsi
index 360b946ba2..4a24fcdacd 100644
--- a/arch/arm/dts/socfpga_cyclone5_vining_fpga-u-boot.dtsi
+++ b/arch/arm/dts/socfpga_cyclone5_vining_fpga-u-boot.dtsi
@@ -6,6 +6,8 @@
  * Copyright (c) 2018 Simon Goldschmidt
  */
 
+#include "socfpga_u-boot.dtsi"
+
 /{
 	aliases {
 		spi0 = "/soc/spi at ff705000";
@@ -17,6 +19,10 @@
 	};
 };
 
+&rst {
+	u-boot,dm-pre-reloc;
+};
+
 &watchdog0 {
 	status = "disabled";
 };
diff --git a/arch/arm/dts/socfpga_u-boot.dtsi b/arch/arm/dts/socfpga_u-boot.dtsi
new file mode 100644
index 0000000000..1abd8ec41b
--- /dev/null
+++ b/arch/arm/dts/socfpga_u-boot.dtsi
@@ -0,0 +1,61 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * U-Boot additions
+ *
+ * Copyright (c) 2019 Simon Goldschmidt
+ *
+ * Note: this file deliberately is not named "-u-boot.dtsi" to prevent it
+ * getting auto-included for non-gen5 socfpga devicetrees.
+ */
+
+&pdma {
+	resets = <&rst DMA_RESET>;
+};
+
+&can0 {
+	resets = <&rst CAN0_RESET>;
+};
+
+&can1 {
+	resets = <&rst CAN1_RESET>;
+};
+
+&gpio0 {
+	resets = <&rst GPIO0_RESET>;
+};
+
+&gpio1 {
+	resets = <&rst GPIO1_RESET>;
+};
+
+&gpio2 {
+	resets = <&rst GPIO2_RESET>;
+};
+
+&mmc {
+	resets = <&rst SDMMC_RESET>;
+};
+
+&nand0 {
+	resets = <&rst NAND_RESET>;
+};
+
+&qspi {
+	resets = <&rst QSPI_RESET>;
+};
+
+&spi0 {
+	resets = <&rst SPIM0_RESET>;
+};
+
+&spi1 {
+	resets = <&rst SPIM1_RESET>;
+};
+
+&uart0 {
+	resets = <&rst UART0_RESET>;
+};
+
+&uart1 {
+	resets = <&rst UART1_RESET>;
+};
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [U-Boot] [PATCH 2/5] arm: socfpga: move SDR reset handling to driver
  2019-01-25 20:30 [U-Boot] [PATCH 0/5] arm: socfpga: implement proper peripheral reset handling Simon Goldschmidt
  2019-01-25 20:30 ` [U-Boot] [PATCH 1/5] arm: socfpga: gen5: add reset handles to devicetrees Simon Goldschmidt
@ 2019-01-25 20:30 ` Simon Goldschmidt
  2019-01-26  8:58   ` Marek Vasut
  2019-01-25 20:30 ` [U-Boot] [PATCH 3/5] mtd: rawnand: denali: add reset handling Simon Goldschmidt
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 35+ messages in thread
From: Simon Goldschmidt @ 2019-01-25 20:30 UTC (permalink / raw)
  To: u-boot

To clean up reset handling for socfpga gen5, let's move the code snippet
taking the DDR controller out of reset from SPL to the DDR driver.

Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
---

 arch/arm/mach-socfpga/spl_gen5.c | 1 -
 drivers/ddr/altera/sdram_gen5.c  | 4 ++++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-socfpga/spl_gen5.c b/arch/arm/mach-socfpga/spl_gen5.c
index ccdc661d05..f9bea892b1 100644
--- a/arch/arm/mach-socfpga/spl_gen5.c
+++ b/arch/arm/mach-socfpga/spl_gen5.c
@@ -98,7 +98,6 @@ void board_init_f(ulong dummy)
 		socfpga_bridges_reset(1);
 	}
 
-	socfpga_per_reset(SOCFPGA_RESET(SDR), 0);
 	socfpga_per_reset(SOCFPGA_RESET(UART0), 0);
 	socfpga_per_reset(SOCFPGA_RESET(OSC1TIMER0), 0);
 
diff --git a/drivers/ddr/altera/sdram_gen5.c b/drivers/ddr/altera/sdram_gen5.c
index 821060459c..bd54c420f8 100644
--- a/drivers/ddr/altera/sdram_gen5.c
+++ b/drivers/ddr/altera/sdram_gen5.c
@@ -7,6 +7,7 @@
 #include <div64.h>
 #include <watchdog.h>
 #include <asm/arch/fpga_manager.h>
+#include <asm/arch/reset_manager.h>
 #include <asm/arch/sdram.h>
 #include <asm/arch/system_manager.h>
 #include <asm/io.h>
@@ -434,6 +435,9 @@ int sdram_mmr_init_full(unsigned int sdr_phy_reg)
 			SDR_CTRLGRP_DRAMADDRW_ROWBITS_LSB;
 	int ret;
 
+	/* release DDR from reset */
+	socfpga_per_reset(SOCFPGA_RESET(SDR), 0);
+
 	writel(rows, &sysmgr_regs->iswgrp_handoff[4]);
 
 	sdr_load_regs(cfg);
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [U-Boot] [PATCH 3/5] mtd: rawnand: denali: add reset handling
  2019-01-25 20:30 [U-Boot] [PATCH 0/5] arm: socfpga: implement proper peripheral reset handling Simon Goldschmidt
  2019-01-25 20:30 ` [U-Boot] [PATCH 1/5] arm: socfpga: gen5: add reset handles to devicetrees Simon Goldschmidt
  2019-01-25 20:30 ` [U-Boot] [PATCH 2/5] arm: socfpga: move SDR reset handling to driver Simon Goldschmidt
@ 2019-01-25 20:30 ` Simon Goldschmidt
  2019-01-26  8:58   ` Marek Vasut
  2019-01-28  9:08   ` Miquel Raynal
  2019-01-25 20:30 ` [U-Boot] [PATCH 4/5] spi: cadence_qspi: " Simon Goldschmidt
  2019-01-25 20:30 ` [U-Boot] [PATCH 5/5] arm: socfpga: implement proper peripheral reset Simon Goldschmidt
  4 siblings, 2 replies; 35+ messages in thread
From: Simon Goldschmidt @ 2019-01-25 20:30 UTC (permalink / raw)
  To: u-boot

This adds reset handling to the devicetree-enabled denalid nand driver.

For backwards compatibility, only a warning is printed when failing to
get reset handles.

Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
---

 drivers/mtd/nand/raw/denali_dt.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/mtd/nand/raw/denali_dt.c b/drivers/mtd/nand/raw/denali_dt.c
index d384b974df..21c4540637 100644
--- a/drivers/mtd/nand/raw/denali_dt.c
+++ b/drivers/mtd/nand/raw/denali_dt.c
@@ -9,6 +9,7 @@
 #include <linux/io.h>
 #include <linux/ioport.h>
 #include <linux/printk.h>
+#include <reset.h>
 
 #include "denali.h"
 
@@ -64,6 +65,7 @@ static int denali_dt_probe(struct udevice *dev)
 	const struct denali_dt_data *data;
 	struct clk clk, clk_x, clk_ecc;
 	struct resource res;
+	struct reset_ctl_bulk reset_bulk;
 	int ret;
 
 	data = (void *)dev_get_driver_data(dev);
@@ -131,6 +133,12 @@ static int denali_dt_probe(struct udevice *dev)
 		denali->clk_x_rate = 200000000;
 	}
 
+	ret = reset_get_bulk(bus, &reset_bulk);
+	if (ret)
+		dev_warn(bus, "Cant' get reset: %d\n", ret);
+	else
+		reset_deassert_bulk(&reset_bulk);
+
 	return denali_init(denali);
 }
 
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [U-Boot] [PATCH 4/5] spi: cadence_qspi: add reset handling
  2019-01-25 20:30 [U-Boot] [PATCH 0/5] arm: socfpga: implement proper peripheral reset handling Simon Goldschmidt
                   ` (2 preceding siblings ...)
  2019-01-25 20:30 ` [U-Boot] [PATCH 3/5] mtd: rawnand: denali: add reset handling Simon Goldschmidt
@ 2019-01-25 20:30 ` Simon Goldschmidt
  2019-01-26  8:59   ` Marek Vasut
  2019-01-25 20:30 ` [U-Boot] [PATCH 5/5] arm: socfpga: implement proper peripheral reset Simon Goldschmidt
  4 siblings, 1 reply; 35+ messages in thread
From: Simon Goldschmidt @ 2019-01-25 20:30 UTC (permalink / raw)
  To: u-boot

This adds reset handling to the devicetree-enabled denalid nand driver.

For backwards compatibility, only a warning is printed when failing to
get reset handles.

Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
---

 drivers/spi/cadence_qspi.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c
index 11fce9c4fe..f68c827e6d 100644
--- a/drivers/spi/cadence_qspi.c
+++ b/drivers/spi/cadence_qspi.c
@@ -8,6 +8,7 @@
 #include <dm.h>
 #include <fdtdec.h>
 #include <malloc.h>
+#include <reset.h>
 #include <spi.h>
 #include <linux/errno.h>
 #include "cadence_qspi.h"
@@ -154,10 +155,18 @@ static int cadence_spi_probe(struct udevice *bus)
 {
 	struct cadence_spi_platdata *plat = bus->platdata;
 	struct cadence_spi_priv *priv = dev_get_priv(bus);
+	struct reset_ctl_bulk reset_bulk;
+	int ret;
 
 	priv->regbase = plat->regbase;
 	priv->ahbbase = plat->ahbbase;
 
+	ret = reset_get_bulk(bus, &reset_bulk);
+	if (ret)
+		dev_warn(bus, "Cant' get reset: %d\n", ret);
+	else
+		reset_deassert_bulk(&reset_bulk);
+
 	if (!priv->qspi_is_init) {
 		cadence_qspi_apb_controller_init(plat);
 		priv->qspi_is_init = 1;
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [U-Boot] [PATCH 5/5] arm: socfpga: implement proper peripheral reset
  2019-01-25 20:30 [U-Boot] [PATCH 0/5] arm: socfpga: implement proper peripheral reset handling Simon Goldschmidt
                   ` (3 preceding siblings ...)
  2019-01-25 20:30 ` [U-Boot] [PATCH 4/5] spi: cadence_qspi: " Simon Goldschmidt
@ 2019-01-25 20:30 ` Simon Goldschmidt
  2019-01-26  9:00   ` Marek Vasut
  4 siblings, 1 reply; 35+ messages in thread
From: Simon Goldschmidt @ 2019-01-25 20:30 UTC (permalink / raw)
  To: u-boot

This commit removes ad-hoc reset handling for peripheral resets from SPL
for socfpga gen5.

This is done because as U-Boot drivers support reset handling by now.

For kernels that don't support taking peripherals out of reset that are
not enabled by U-Boot, a new Kconfig option "OLD_SOCFPGA_KERNEL_COMPAT"
is added, which keeps the old behaviour of just enabling all peripherals.

This new option is enabled by default for now, as even Linux 4.20 does
not support reset handling on all peripherals.

Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
---

 arch/arm/mach-socfpga/Kconfig     | 10 ++++++++++
 arch/arm/mach-socfpga/misc_gen5.c |  2 ++
 arch/arm/mach-socfpga/spl_gen5.c  | 10 ++++++++++
 3 files changed, 22 insertions(+)

diff --git a/arch/arm/mach-socfpga/Kconfig b/arch/arm/mach-socfpga/Kconfig
index 5e87371f8c..89acced8d8 100644
--- a/arch/arm/mach-socfpga/Kconfig
+++ b/arch/arm/mach-socfpga/Kconfig
@@ -142,4 +142,14 @@ config SYS_CONFIG_NAME
 	default "socfpga_stratix10_socdk" if TARGET_SOCFPGA_STRATIX10_SOCDK
 	default "socfpga_vining_fpga" if TARGET_SOCFPGA_SAMTEC_VINING_FPGA
 
+config OLD_SOCFPGA_KERNEL_COMPAT
+	bool "Enable workarounds for booting old kernels"
+	depends on TARGET_SOCFPGA_GEN5
+	default y
+	help
+	  Set this to enable various workarounds for old kernels (e.g. take all
+	  peripherals out of reset because old kernels cannot handle reset).
+	  This results in sub-optimal settings for newer kernels, only enable
+	  if needed.
+
 endif
diff --git a/arch/arm/mach-socfpga/misc_gen5.c b/arch/arm/mach-socfpga/misc_gen5.c
index 04f237d100..168669923e 100644
--- a/arch/arm/mach-socfpga/misc_gen5.c
+++ b/arch/arm/mach-socfpga/misc_gen5.c
@@ -243,6 +243,7 @@ int arch_early_init_r(void)
 	/* Add device descriptor to FPGA device table */
 	socfpga_fpga_add(&altera_fpga[0]);
 
+#ifdef CONFIG_OLD_SOCFPGA_KERNEL_COMPAT
 #ifdef CONFIG_DESIGNWARE_SPI
 	/* Get Designware SPI controller out of reset */
 	socfpga_per_reset(SOCFPGA_RESET(SPIM0), 0);
@@ -252,6 +253,7 @@ int arch_early_init_r(void)
 #ifdef CONFIG_NAND_DENALI
 	socfpga_per_reset(SOCFPGA_RESET(NAND), 0);
 #endif
+#endif /* CONFIG_OLD_SOCFPGA_KERNEL_COMPAT */
 
 	return 0;
 }
diff --git a/arch/arm/mach-socfpga/spl_gen5.c b/arch/arm/mach-socfpga/spl_gen5.c
index f9bea892b1..19a211256d 100644
--- a/arch/arm/mach-socfpga/spl_gen5.c
+++ b/arch/arm/mach-socfpga/spl_gen5.c
@@ -35,16 +35,22 @@ u32 spl_boot_device(void)
 		return BOOT_DEVICE_RAM;
 	case 0x2:	/* NAND Flash (1.8V) */
 	case 0x3:	/* NAND Flash (3.0V) */
+#ifdef CONFIG_OLD_SOCFPGA_KERNEL_COMPAT
 		socfpga_per_reset(SOCFPGA_RESET(NAND), 0);
+#endif
 		return BOOT_DEVICE_NAND;
 	case 0x4:	/* SD/MMC External Transceiver (1.8V) */
 	case 0x5:	/* SD/MMC Internal Transceiver (3.0V) */
+#ifdef CONFIG_OLD_SOCFPGA_KERNEL_COMPAT
 		socfpga_per_reset(SOCFPGA_RESET(SDMMC), 0);
 		socfpga_per_reset(SOCFPGA_RESET(DMA), 0);
+#endif
 		return BOOT_DEVICE_MMC1;
 	case 0x6:	/* QSPI Flash (1.8V) */
 	case 0x7:	/* QSPI Flash (3.0V) */
+#ifdef CONFIG_OLD_SOCFPGA_KERNEL_COMPAT
 		socfpga_per_reset(SOCFPGA_RESET(QSPI), 0);
+#endif
 		return BOOT_DEVICE_SPI;
 	default:
 		printf("Invalid boot device (bsel=%08x)!\n", bsel);
@@ -98,7 +104,9 @@ void board_init_f(ulong dummy)
 		socfpga_bridges_reset(1);
 	}
 
+#ifdef CONFIG_OLD_SOCFPGA_KERNEL_COMPAT
 	socfpga_per_reset(SOCFPGA_RESET(UART0), 0);
+#endif
 	socfpga_per_reset(SOCFPGA_RESET(OSC1TIMER0), 0);
 
 	timer_init();
@@ -122,9 +130,11 @@ void board_init_f(ulong dummy)
 	sysmgr_pinmux_init();
 	sysmgr_config_warmrstcfgio(0);
 
+#ifdef CONFIG_OLD_SOCFPGA_KERNEL_COMPAT
 	/* De-assert reset for peripherals and bridges based on handoff */
 	reset_deassert_peripherals_handoff();
 	socfpga_bridges_reset(0);
+#endif
 
 	debug("Unfreezing/Thaw all I/O banks\n");
 	/* unfreeze / thaw all IO banks */
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [U-Boot] [PATCH 1/5] arm: socfpga: gen5: add reset handles to devicetrees
  2019-01-25 20:30 ` [U-Boot] [PATCH 1/5] arm: socfpga: gen5: add reset handles to devicetrees Simon Goldschmidt
@ 2019-01-26  8:57   ` Marek Vasut
  2019-01-27  8:45     ` Simon Goldschmidt
  0 siblings, 1 reply; 35+ messages in thread
From: Marek Vasut @ 2019-01-26  8:57 UTC (permalink / raw)
  To: u-boot

On 1/25/19 9:30 PM, Simon Goldschmidt wrote:
> The SPL for socfpga gen5 currently takes all peripherals out of reset
> unconditionally. To implement proper reset handling for peripherals,
>  some device tree nodes are missing reset handles.
> 
> Since the socfpga gen5 devicetrees are currently in sync with Linux 4.20,
> and these reset handles are not added to Linux, yet, let's add them to
> U-Boot specific dtsi files to ease future merging.
> 
> This adds a new file socfpga_u-boot.dtsi (note the '_' - if using a '-'
> this file gets pulled in for some a10/s10 devicetrees) that adds the
> missing reset handles.
> 
> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>

Shouldn't all this be in u-boot specific DTs, instead of the ones
imported from Linux ?

> ---
> 
>  arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi |  6 ++
>  arch/arm/dts/socfpga_cyclone5_dbm_soc1.dts    |  5 ++
>  .../socfpga_cyclone5_de0_nano_soc-u-boot.dtsi |  6 ++
>  arch/arm/dts/socfpga_cyclone5_de10_nano.dts   |  5 ++
[...]

-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [U-Boot] [PATCH 2/5] arm: socfpga: move SDR reset handling to driver
  2019-01-25 20:30 ` [U-Boot] [PATCH 2/5] arm: socfpga: move SDR reset handling to driver Simon Goldschmidt
@ 2019-01-26  8:58   ` Marek Vasut
  2019-01-27  8:47     ` Simon Goldschmidt
  0 siblings, 1 reply; 35+ messages in thread
From: Marek Vasut @ 2019-01-26  8:58 UTC (permalink / raw)
  To: u-boot

On 1/25/19 9:30 PM, Simon Goldschmidt wrote:
> To clean up reset handling for socfpga gen5, let's move the code snippet
> taking the DDR controller out of reset from SPL to the DDR driver.
> 
> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> ---
> 
>  arch/arm/mach-socfpga/spl_gen5.c | 1 -
>  drivers/ddr/altera/sdram_gen5.c  | 4 ++++
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-socfpga/spl_gen5.c b/arch/arm/mach-socfpga/spl_gen5.c
> index ccdc661d05..f9bea892b1 100644
> --- a/arch/arm/mach-socfpga/spl_gen5.c
> +++ b/arch/arm/mach-socfpga/spl_gen5.c
> @@ -98,7 +98,6 @@ void board_init_f(ulong dummy)
>  		socfpga_bridges_reset(1);
>  	}
>  
> -	socfpga_per_reset(SOCFPGA_RESET(SDR), 0);
>  	socfpga_per_reset(SOCFPGA_RESET(UART0), 0);
>  	socfpga_per_reset(SOCFPGA_RESET(OSC1TIMER0), 0);
>  
> diff --git a/drivers/ddr/altera/sdram_gen5.c b/drivers/ddr/altera/sdram_gen5.c
> index 821060459c..bd54c420f8 100644
> --- a/drivers/ddr/altera/sdram_gen5.c
> +++ b/drivers/ddr/altera/sdram_gen5.c
> @@ -7,6 +7,7 @@
>  #include <div64.h>
>  #include <watchdog.h>
>  #include <asm/arch/fpga_manager.h>
> +#include <asm/arch/reset_manager.h>
>  #include <asm/arch/sdram.h>
>  #include <asm/arch/system_manager.h>
>  #include <asm/io.h>
> @@ -434,6 +435,9 @@ int sdram_mmr_init_full(unsigned int sdr_phy_reg)
>  			SDR_CTRLGRP_DRAMADDRW_ROWBITS_LSB;
>  	int ret;
>  
> +	/* release DDR from reset */
> +	socfpga_per_reset(SOCFPGA_RESET(SDR), 0);
> +

Can the reset framework do this ?

>  	writel(rows, &sysmgr_regs->iswgrp_handoff[4]);
>  
>  	sdr_load_regs(cfg);
> 


-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [U-Boot] [PATCH 3/5] mtd: rawnand: denali: add reset handling
  2019-01-25 20:30 ` [U-Boot] [PATCH 3/5] mtd: rawnand: denali: add reset handling Simon Goldschmidt
@ 2019-01-26  8:58   ` Marek Vasut
  2019-01-28  9:08   ` Miquel Raynal
  1 sibling, 0 replies; 35+ messages in thread
From: Marek Vasut @ 2019-01-26  8:58 UTC (permalink / raw)
  To: u-boot

On 1/25/19 9:30 PM, Simon Goldschmidt wrote:
> This adds reset handling to the devicetree-enabled denalid nand driver.
> 
> For backwards compatibility, only a warning is printed when failing to
> get reset handles.
> 
> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> ---
> 
>  drivers/mtd/nand/raw/denali_dt.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/mtd/nand/raw/denali_dt.c b/drivers/mtd/nand/raw/denali_dt.c
> index d384b974df..21c4540637 100644
> --- a/drivers/mtd/nand/raw/denali_dt.c
> +++ b/drivers/mtd/nand/raw/denali_dt.c
> @@ -9,6 +9,7 @@
>  #include <linux/io.h>
>  #include <linux/ioport.h>
>  #include <linux/printk.h>
> +#include <reset.h>
>  
>  #include "denali.h"
>  
> @@ -64,6 +65,7 @@ static int denali_dt_probe(struct udevice *dev)
>  	const struct denali_dt_data *data;
>  	struct clk clk, clk_x, clk_ecc;
>  	struct resource res;
> +	struct reset_ctl_bulk reset_bulk;
>  	int ret;
>  
>  	data = (void *)dev_get_driver_data(dev);
> @@ -131,6 +133,12 @@ static int denali_dt_probe(struct udevice *dev)
>  		denali->clk_x_rate = 200000000;
>  	}
>  
> +	ret = reset_get_bulk(bus, &reset_bulk);
> +	if (ret)
> +		dev_warn(bus, "Cant' get reset: %d\n", ret);

Can't , misplaced "'"

> +	else
> +		reset_deassert_bulk(&reset_bulk);
> +
>  	return denali_init(denali);
>  }
>  
> 


-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [U-Boot] [PATCH 4/5] spi: cadence_qspi: add reset handling
  2019-01-25 20:30 ` [U-Boot] [PATCH 4/5] spi: cadence_qspi: " Simon Goldschmidt
@ 2019-01-26  8:59   ` Marek Vasut
  2019-01-27  8:49     ` Simon Goldschmidt
  0 siblings, 1 reply; 35+ messages in thread
From: Marek Vasut @ 2019-01-26  8:59 UTC (permalink / raw)
  To: u-boot

On 1/25/19 9:30 PM, Simon Goldschmidt wrote:
> This adds reset handling to the devicetree-enabled denalid nand driver.
> 
> For backwards compatibility, only a warning is printed when failing to
> get reset handles.
> 
> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> ---
> 
>  drivers/spi/cadence_qspi.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c
> index 11fce9c4fe..f68c827e6d 100644
> --- a/drivers/spi/cadence_qspi.c
> +++ b/drivers/spi/cadence_qspi.c
> @@ -8,6 +8,7 @@
>  #include <dm.h>
>  #include <fdtdec.h>
>  #include <malloc.h>
> +#include <reset.h>
>  #include <spi.h>
>  #include <linux/errno.h>
>  #include "cadence_qspi.h"
> @@ -154,10 +155,18 @@ static int cadence_spi_probe(struct udevice *bus)
>  {
>  	struct cadence_spi_platdata *plat = bus->platdata;
>  	struct cadence_spi_priv *priv = dev_get_priv(bus);
> +	struct reset_ctl_bulk reset_bulk;
> +	int ret;
>  
>  	priv->regbase = plat->regbase;
>  	priv->ahbbase = plat->ahbbase;
>  
> +	ret = reset_get_bulk(bus, &reset_bulk);
> +	if (ret)
> +		dev_warn(bus, "Cant' get reset: %d\n", ret);

Can't

Also, shouldn't there be some reset_release or reset_free somewhere ?
What about .remove(), shouldn't that release the reset ?

> +	else
> +		reset_deassert_bulk(&reset_bulk);
> +
>  	if (!priv->qspi_is_init) {
>  		cadence_qspi_apb_controller_init(plat);
>  		priv->qspi_is_init = 1;
> 


-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [U-Boot] [PATCH 5/5] arm: socfpga: implement proper peripheral reset
  2019-01-25 20:30 ` [U-Boot] [PATCH 5/5] arm: socfpga: implement proper peripheral reset Simon Goldschmidt
@ 2019-01-26  9:00   ` Marek Vasut
  2019-01-27  8:56     ` Simon Goldschmidt
  0 siblings, 1 reply; 35+ messages in thread
From: Marek Vasut @ 2019-01-26  9:00 UTC (permalink / raw)
  To: u-boot

On 1/25/19 9:30 PM, Simon Goldschmidt wrote:
> This commit removes ad-hoc reset handling for peripheral resets from SPL
> for socfpga gen5.
> 
> This is done because as U-Boot drivers support reset handling by now.
> 
> For kernels that don't support taking peripherals out of reset that are
> not enabled by U-Boot, a new Kconfig option "OLD_SOCFPGA_KERNEL_COMPAT"
> is added, which keeps the old behaviour of just enabling all peripherals.
> 
> This new option is enabled by default for now, as even Linux 4.20 does
> not support reset handling on all peripherals.
> 
> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>

Well ew, don't we have some pre-linux-boot hook which could just flip
the reset register bits in one place ? And also, I'd prefer this to be
runtime-configurable, not compile-time configurable.

> ---
> 
>  arch/arm/mach-socfpga/Kconfig     | 10 ++++++++++
>  arch/arm/mach-socfpga/misc_gen5.c |  2 ++
>  arch/arm/mach-socfpga/spl_gen5.c  | 10 ++++++++++
>  3 files changed, 22 insertions(+)
> 
> diff --git a/arch/arm/mach-socfpga/Kconfig b/arch/arm/mach-socfpga/Kconfig
> index 5e87371f8c..89acced8d8 100644
> --- a/arch/arm/mach-socfpga/Kconfig
> +++ b/arch/arm/mach-socfpga/Kconfig
> @@ -142,4 +142,14 @@ config SYS_CONFIG_NAME
>  	default "socfpga_stratix10_socdk" if TARGET_SOCFPGA_STRATIX10_SOCDK
>  	default "socfpga_vining_fpga" if TARGET_SOCFPGA_SAMTEC_VINING_FPGA
>  
> +config OLD_SOCFPGA_KERNEL_COMPAT
> +	bool "Enable workarounds for booting old kernels"
> +	depends on TARGET_SOCFPGA_GEN5
> +	default y
> +	help
> +	  Set this to enable various workarounds for old kernels (e.g. take all
> +	  peripherals out of reset because old kernels cannot handle reset).
> +	  This results in sub-optimal settings for newer kernels, only enable
> +	  if needed.
> +
>  endif
> diff --git a/arch/arm/mach-socfpga/misc_gen5.c b/arch/arm/mach-socfpga/misc_gen5.c
> index 04f237d100..168669923e 100644
> --- a/arch/arm/mach-socfpga/misc_gen5.c
> +++ b/arch/arm/mach-socfpga/misc_gen5.c
> @@ -243,6 +243,7 @@ int arch_early_init_r(void)
>  	/* Add device descriptor to FPGA device table */
>  	socfpga_fpga_add(&altera_fpga[0]);
>  
> +#ifdef CONFIG_OLD_SOCFPGA_KERNEL_COMPAT
>  #ifdef CONFIG_DESIGNWARE_SPI
>  	/* Get Designware SPI controller out of reset */
>  	socfpga_per_reset(SOCFPGA_RESET(SPIM0), 0);
> @@ -252,6 +253,7 @@ int arch_early_init_r(void)
>  #ifdef CONFIG_NAND_DENALI
>  	socfpga_per_reset(SOCFPGA_RESET(NAND), 0);
>  #endif
> +#endif /* CONFIG_OLD_SOCFPGA_KERNEL_COMPAT */
>  
>  	return 0;
>  }
> diff --git a/arch/arm/mach-socfpga/spl_gen5.c b/arch/arm/mach-socfpga/spl_gen5.c
> index f9bea892b1..19a211256d 100644
> --- a/arch/arm/mach-socfpga/spl_gen5.c
> +++ b/arch/arm/mach-socfpga/spl_gen5.c
> @@ -35,16 +35,22 @@ u32 spl_boot_device(void)
>  		return BOOT_DEVICE_RAM;
>  	case 0x2:	/* NAND Flash (1.8V) */
>  	case 0x3:	/* NAND Flash (3.0V) */
> +#ifdef CONFIG_OLD_SOCFPGA_KERNEL_COMPAT
>  		socfpga_per_reset(SOCFPGA_RESET(NAND), 0);
> +#endif
>  		return BOOT_DEVICE_NAND;
>  	case 0x4:	/* SD/MMC External Transceiver (1.8V) */
>  	case 0x5:	/* SD/MMC Internal Transceiver (3.0V) */
> +#ifdef CONFIG_OLD_SOCFPGA_KERNEL_COMPAT
>  		socfpga_per_reset(SOCFPGA_RESET(SDMMC), 0);
>  		socfpga_per_reset(SOCFPGA_RESET(DMA), 0);
> +#endif
>  		return BOOT_DEVICE_MMC1;
>  	case 0x6:	/* QSPI Flash (1.8V) */
>  	case 0x7:	/* QSPI Flash (3.0V) */
> +#ifdef CONFIG_OLD_SOCFPGA_KERNEL_COMPAT
>  		socfpga_per_reset(SOCFPGA_RESET(QSPI), 0);
> +#endif
>  		return BOOT_DEVICE_SPI;
>  	default:
>  		printf("Invalid boot device (bsel=%08x)!\n", bsel);
> @@ -98,7 +104,9 @@ void board_init_f(ulong dummy)
>  		socfpga_bridges_reset(1);
>  	}
>  
> +#ifdef CONFIG_OLD_SOCFPGA_KERNEL_COMPAT
>  	socfpga_per_reset(SOCFPGA_RESET(UART0), 0);
> +#endif
>  	socfpga_per_reset(SOCFPGA_RESET(OSC1TIMER0), 0);
>  
>  	timer_init();
> @@ -122,9 +130,11 @@ void board_init_f(ulong dummy)
>  	sysmgr_pinmux_init();
>  	sysmgr_config_warmrstcfgio(0);
>  
> +#ifdef CONFIG_OLD_SOCFPGA_KERNEL_COMPAT
>  	/* De-assert reset for peripherals and bridges based on handoff */
>  	reset_deassert_peripherals_handoff();
>  	socfpga_bridges_reset(0);
> +#endif
>  
>  	debug("Unfreezing/Thaw all I/O banks\n");
>  	/* unfreeze / thaw all IO banks */
> 


-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [U-Boot] [PATCH 1/5] arm: socfpga: gen5: add reset handles to devicetrees
  2019-01-26  8:57   ` Marek Vasut
@ 2019-01-27  8:45     ` Simon Goldschmidt
  2019-01-28 10:54       ` Marek Vasut
  0 siblings, 1 reply; 35+ messages in thread
From: Simon Goldschmidt @ 2019-01-27  8:45 UTC (permalink / raw)
  To: u-boot

Am 26.01.2019 um 09:57 schrieb Marek Vasut:
> On 1/25/19 9:30 PM, Simon Goldschmidt wrote:
>> The SPL for socfpga gen5 currently takes all peripherals out of reset
>> unconditionally. To implement proper reset handling for peripherals,
>>   some device tree nodes are missing reset handles.
>>
>> Since the socfpga gen5 devicetrees are currently in sync with Linux 4.20,
>> and these reset handles are not added to Linux, yet, let's add them to
>> U-Boot specific dtsi files to ease future merging.
>>
>> This adds a new file socfpga_u-boot.dtsi (note the '_' - if using a '-'
>> this file gets pulled in for some a10/s10 devicetrees) that adds the
>> missing reset handles.
>>
>> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> 
> Shouldn't all this be in u-boot specific DTs, instead of the ones
> imported from Linux ?

The "u-boot,dm-pre-reloc" thing should of course stay in u-boot specific 
DTs. But I had to add many missing "resets = " references that are yet 
missing in upstream Linux DTs. Dinh is in the process of adding them and 
I'll make sure to sync that back to U-Boot once it's in Linux mainline 
git. I just did not want to wait until that happens...

Regards,
Simon

> 
>> ---
>>
>>   arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi |  6 ++
>>   arch/arm/dts/socfpga_cyclone5_dbm_soc1.dts    |  5 ++
>>   .../socfpga_cyclone5_de0_nano_soc-u-boot.dtsi |  6 ++
>>   arch/arm/dts/socfpga_cyclone5_de10_nano.dts   |  5 ++
> [...]
> 

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [U-Boot] [PATCH 2/5] arm: socfpga: move SDR reset handling to driver
  2019-01-26  8:58   ` Marek Vasut
@ 2019-01-27  8:47     ` Simon Goldschmidt
  2019-01-28 10:55       ` Marek Vasut
  0 siblings, 1 reply; 35+ messages in thread
From: Simon Goldschmidt @ 2019-01-27  8:47 UTC (permalink / raw)
  To: u-boot

Am 26.01.2019 um 09:58 schrieb Marek Vasut:
> On 1/25/19 9:30 PM, Simon Goldschmidt wrote:
>> To clean up reset handling for socfpga gen5, let's move the code snippet
>> taking the DDR controller out of reset from SPL to the DDR driver.
>>
>> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
>> ---
>>
>>   arch/arm/mach-socfpga/spl_gen5.c | 1 -
>>   drivers/ddr/altera/sdram_gen5.c  | 4 ++++
>>   2 files changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/mach-socfpga/spl_gen5.c b/arch/arm/mach-socfpga/spl_gen5.c
>> index ccdc661d05..f9bea892b1 100644
>> --- a/arch/arm/mach-socfpga/spl_gen5.c
>> +++ b/arch/arm/mach-socfpga/spl_gen5.c
>> @@ -98,7 +98,6 @@ void board_init_f(ulong dummy)
>>   		socfpga_bridges_reset(1);
>>   	}
>>   
>> -	socfpga_per_reset(SOCFPGA_RESET(SDR), 0);
>>   	socfpga_per_reset(SOCFPGA_RESET(UART0), 0);
>>   	socfpga_per_reset(SOCFPGA_RESET(OSC1TIMER0), 0);
>>   
>> diff --git a/drivers/ddr/altera/sdram_gen5.c b/drivers/ddr/altera/sdram_gen5.c
>> index 821060459c..bd54c420f8 100644
>> --- a/drivers/ddr/altera/sdram_gen5.c
>> +++ b/drivers/ddr/altera/sdram_gen5.c
>> @@ -7,6 +7,7 @@
>>   #include <div64.h>
>>   #include <watchdog.h>
>>   #include <asm/arch/fpga_manager.h>
>> +#include <asm/arch/reset_manager.h>
>>   #include <asm/arch/sdram.h>
>>   #include <asm/arch/system_manager.h>
>>   #include <asm/io.h>
>> @@ -434,6 +435,9 @@ int sdram_mmr_init_full(unsigned int sdr_phy_reg)
>>   			SDR_CTRLGRP_DRAMADDRW_ROWBITS_LSB;
>>   	int ret;
>>   
>> +	/* release DDR from reset */
>> +	socfpga_per_reset(SOCFPGA_RESET(SDR), 0);
>> +
> 
> Can the reset framework do this ?

Hmm, it probably could, but I see that as a diferent patch. The altera 
DDR driver currently does not work with devicetree, so using the reset 
framework here would be a bigger patch, I think.

Can we do that later and clean up this by just moving the code?

Regards,
Simon

> 
>>   	writel(rows, &sysmgr_regs->iswgrp_handoff[4]);
>>   
>>   	sdr_load_regs(cfg);
>>
> 
> 

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [U-Boot] [PATCH 4/5] spi: cadence_qspi: add reset handling
  2019-01-26  8:59   ` Marek Vasut
@ 2019-01-27  8:49     ` Simon Goldschmidt
  2019-01-28 10:56       ` Marek Vasut
  0 siblings, 1 reply; 35+ messages in thread
From: Simon Goldschmidt @ 2019-01-27  8:49 UTC (permalink / raw)
  To: u-boot

Am 26.01.2019 um 09:59 schrieb Marek Vasut:
> On 1/25/19 9:30 PM, Simon Goldschmidt wrote:
>> This adds reset handling to the devicetree-enabled denalid nand driver.
>>
>> For backwards compatibility, only a warning is printed when failing to
>> get reset handles.
>>
>> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
>> ---
>>
>>   drivers/spi/cadence_qspi.c | 9 +++++++++
>>   1 file changed, 9 insertions(+)
>>
>> diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c
>> index 11fce9c4fe..f68c827e6d 100644
>> --- a/drivers/spi/cadence_qspi.c
>> +++ b/drivers/spi/cadence_qspi.c
>> @@ -8,6 +8,7 @@
>>   #include <dm.h>
>>   #include <fdtdec.h>
>>   #include <malloc.h>
>> +#include <reset.h>
>>   #include <spi.h>
>>   #include <linux/errno.h>
>>   #include "cadence_qspi.h"
>> @@ -154,10 +155,18 @@ static int cadence_spi_probe(struct udevice *bus)
>>   {
>>   	struct cadence_spi_platdata *plat = bus->platdata;
>>   	struct cadence_spi_priv *priv = dev_get_priv(bus);
>> +	struct reset_ctl_bulk reset_bulk;
>> +	int ret;
>>   
>>   	priv->regbase = plat->regbase;
>>   	priv->ahbbase = plat->ahbbase;
>>   
>> +	ret = reset_get_bulk(bus, &reset_bulk);
>> +	if (ret)
>> +		dev_warn(bus, "Cant' get reset: %d\n", ret);
> 
> Can't

Right. Copy & paste from 3/5...

> 
> Also, shouldn't there be some reset_release or reset_free somewhere ?
> What about .remove(), shouldn't that release the reset ?

Ehrm, well... this is my first driver using the reset framework. I 
haven't though of remove. I'll do a V2 for that soon.

Regards,
Simon

> 
>> +	else
>> +		reset_deassert_bulk(&reset_bulk);
>> +
>>   	if (!priv->qspi_is_init) {
>>   		cadence_qspi_apb_controller_init(plat);
>>   		priv->qspi_is_init = 1;
>>
> 
> 

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [U-Boot] [PATCH 5/5] arm: socfpga: implement proper peripheral reset
  2019-01-26  9:00   ` Marek Vasut
@ 2019-01-27  8:56     ` Simon Goldschmidt
  2019-01-28 10:58       ` Marek Vasut
  0 siblings, 1 reply; 35+ messages in thread
From: Simon Goldschmidt @ 2019-01-27  8:56 UTC (permalink / raw)
  To: u-boot

Am 26.01.2019 um 10:00 schrieb Marek Vasut:
> On 1/25/19 9:30 PM, Simon Goldschmidt wrote:
>> This commit removes ad-hoc reset handling for peripheral resets from SPL
>> for socfpga gen5.
>>
>> This is done because as U-Boot drivers support reset handling by now.
>>
>> For kernels that don't support taking peripherals out of reset that are
>> not enabled by U-Boot, a new Kconfig option "OLD_SOCFPGA_KERNEL_COMPAT"
>> is added, which keeps the old behaviour of just enabling all peripherals.
>>
>> This new option is enabled by default for now, as even Linux 4.20 does
>> not support reset handling on all peripherals.
>>
>> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> 
> Well ew, don't we have some pre-linux-boot hook which could just flip
> the reset register bits in one place ? And also, I'd prefer this to be
> runtime-configurable, not compile-time configurable.


And this will be the biggest change for v2. It's also why I needed so 
long to send this series. What I sent in this version is how Tom said 
the same thing is being handled for mach-sunxi (Kconfig 
"OLD_SUNXI_KERNEL_COMPAT").

So if I remember your last mails about this topic, you'd prefer an 
environment variable with a callback that leads to enabling all 
peripherals just before jumping to Linux (yes, I'll find a hook for that).

I see two problems here:
- enabling all peripherals will need to be the default for some time, 
until Linux supports reset handling in all drivers. How do you want to 
deal with that?
- is this scheme used somewhere else for compatibility? I think it would 
make sense for multiple architectures to use the same way to handle such 
compatibility issues...

Regards,
Simon

> 
>> ---
>>
>>   arch/arm/mach-socfpga/Kconfig     | 10 ++++++++++
>>   arch/arm/mach-socfpga/misc_gen5.c |  2 ++
>>   arch/arm/mach-socfpga/spl_gen5.c  | 10 ++++++++++
>>   3 files changed, 22 insertions(+)
>>
>> diff --git a/arch/arm/mach-socfpga/Kconfig b/arch/arm/mach-socfpga/Kconfig
>> index 5e87371f8c..89acced8d8 100644
>> --- a/arch/arm/mach-socfpga/Kconfig
>> +++ b/arch/arm/mach-socfpga/Kconfig
>> @@ -142,4 +142,14 @@ config SYS_CONFIG_NAME
>>   	default "socfpga_stratix10_socdk" if TARGET_SOCFPGA_STRATIX10_SOCDK
>>   	default "socfpga_vining_fpga" if TARGET_SOCFPGA_SAMTEC_VINING_FPGA
>>   
>> +config OLD_SOCFPGA_KERNEL_COMPAT
>> +	bool "Enable workarounds for booting old kernels"
>> +	depends on TARGET_SOCFPGA_GEN5
>> +	default y
>> +	help
>> +	  Set this to enable various workarounds for old kernels (e.g. take all
>> +	  peripherals out of reset because old kernels cannot handle reset).
>> +	  This results in sub-optimal settings for newer kernels, only enable
>> +	  if needed.
>> +
>>   endif
>> diff --git a/arch/arm/mach-socfpga/misc_gen5.c b/arch/arm/mach-socfpga/misc_gen5.c
>> index 04f237d100..168669923e 100644
>> --- a/arch/arm/mach-socfpga/misc_gen5.c
>> +++ b/arch/arm/mach-socfpga/misc_gen5.c
>> @@ -243,6 +243,7 @@ int arch_early_init_r(void)
>>   	/* Add device descriptor to FPGA device table */
>>   	socfpga_fpga_add(&altera_fpga[0]);
>>   
>> +#ifdef CONFIG_OLD_SOCFPGA_KERNEL_COMPAT
>>   #ifdef CONFIG_DESIGNWARE_SPI
>>   	/* Get Designware SPI controller out of reset */
>>   	socfpga_per_reset(SOCFPGA_RESET(SPIM0), 0);
>> @@ -252,6 +253,7 @@ int arch_early_init_r(void)
>>   #ifdef CONFIG_NAND_DENALI
>>   	socfpga_per_reset(SOCFPGA_RESET(NAND), 0);
>>   #endif
>> +#endif /* CONFIG_OLD_SOCFPGA_KERNEL_COMPAT */
>>   
>>   	return 0;
>>   }
>> diff --git a/arch/arm/mach-socfpga/spl_gen5.c b/arch/arm/mach-socfpga/spl_gen5.c
>> index f9bea892b1..19a211256d 100644
>> --- a/arch/arm/mach-socfpga/spl_gen5.c
>> +++ b/arch/arm/mach-socfpga/spl_gen5.c
>> @@ -35,16 +35,22 @@ u32 spl_boot_device(void)
>>   		return BOOT_DEVICE_RAM;
>>   	case 0x2:	/* NAND Flash (1.8V) */
>>   	case 0x3:	/* NAND Flash (3.0V) */
>> +#ifdef CONFIG_OLD_SOCFPGA_KERNEL_COMPAT
>>   		socfpga_per_reset(SOCFPGA_RESET(NAND), 0);
>> +#endif
>>   		return BOOT_DEVICE_NAND;
>>   	case 0x4:	/* SD/MMC External Transceiver (1.8V) */
>>   	case 0x5:	/* SD/MMC Internal Transceiver (3.0V) */
>> +#ifdef CONFIG_OLD_SOCFPGA_KERNEL_COMPAT
>>   		socfpga_per_reset(SOCFPGA_RESET(SDMMC), 0);
>>   		socfpga_per_reset(SOCFPGA_RESET(DMA), 0);
>> +#endif
>>   		return BOOT_DEVICE_MMC1;
>>   	case 0x6:	/* QSPI Flash (1.8V) */
>>   	case 0x7:	/* QSPI Flash (3.0V) */
>> +#ifdef CONFIG_OLD_SOCFPGA_KERNEL_COMPAT
>>   		socfpga_per_reset(SOCFPGA_RESET(QSPI), 0);
>> +#endif
>>   		return BOOT_DEVICE_SPI;
>>   	default:
>>   		printf("Invalid boot device (bsel=%08x)!\n", bsel);
>> @@ -98,7 +104,9 @@ void board_init_f(ulong dummy)
>>   		socfpga_bridges_reset(1);
>>   	}
>>   
>> +#ifdef CONFIG_OLD_SOCFPGA_KERNEL_COMPAT
>>   	socfpga_per_reset(SOCFPGA_RESET(UART0), 0);
>> +#endif
>>   	socfpga_per_reset(SOCFPGA_RESET(OSC1TIMER0), 0);
>>   
>>   	timer_init();
>> @@ -122,9 +130,11 @@ void board_init_f(ulong dummy)
>>   	sysmgr_pinmux_init();
>>   	sysmgr_config_warmrstcfgio(0);
>>   
>> +#ifdef CONFIG_OLD_SOCFPGA_KERNEL_COMPAT
>>   	/* De-assert reset for peripherals and bridges based on handoff */
>>   	reset_deassert_peripherals_handoff();
>>   	socfpga_bridges_reset(0);
>> +#endif
>>   
>>   	debug("Unfreezing/Thaw all I/O banks\n");
>>   	/* unfreeze / thaw all IO banks */
>>
> 
> 

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [U-Boot] [PATCH 3/5] mtd: rawnand: denali: add reset handling
  2019-01-25 20:30 ` [U-Boot] [PATCH 3/5] mtd: rawnand: denali: add reset handling Simon Goldschmidt
  2019-01-26  8:58   ` Marek Vasut
@ 2019-01-28  9:08   ` Miquel Raynal
  2019-01-28  9:16     ` Simon Goldschmidt
  1 sibling, 1 reply; 35+ messages in thread
From: Miquel Raynal @ 2019-01-28  9:08 UTC (permalink / raw)
  To: u-boot

Hi Simon,

Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> wrote on Fri, 25
Jan 2019 21:30:49 +0100:

> This adds reset handling to the devicetree-enabled denalid nand driver.

"Denali NAND driver"

> 
> For backwards compatibility, only a warning is printed when failing to
> get reset handles.
> 
> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> ---
> 
>  drivers/mtd/nand/raw/denali_dt.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/mtd/nand/raw/denali_dt.c b/drivers/mtd/nand/raw/denali_dt.c
> index d384b974df..21c4540637 100644
> --- a/drivers/mtd/nand/raw/denali_dt.c
> +++ b/drivers/mtd/nand/raw/denali_dt.c
> @@ -9,6 +9,7 @@
>  #include <linux/io.h>
>  #include <linux/ioport.h>
>  #include <linux/printk.h>
> +#include <reset.h>
>  
>  #include "denali.h"
>  
> @@ -64,6 +65,7 @@ static int denali_dt_probe(struct udevice *dev)
>  	const struct denali_dt_data *data;
>  	struct clk clk, clk_x, clk_ecc;
>  	struct resource res;
> +	struct reset_ctl_bulk reset_bulk;
>  	int ret;
>  
>  	data = (void *)dev_get_driver_data(dev);
> @@ -131,6 +133,12 @@ static int denali_dt_probe(struct udevice *dev)
>  		denali->clk_x_rate = 200000000;
>  	}
>  
> +	ret = reset_get_bulk(bus, &reset_bulk);
> +	if (ret)
> +		dev_warn(bus, "Cant' get reset: %d\n", ret);
> +	else
> +		reset_deassert_bulk(&reset_bulk);

Where are reset_get_bulk/reset_deassert_bulk() defined?

> +
>  	return denali_init(denali);
>  }
>  

Thanks,
Miquèl

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [U-Boot] [PATCH 3/5] mtd: rawnand: denali: add reset handling
  2019-01-28  9:08   ` Miquel Raynal
@ 2019-01-28  9:16     ` Simon Goldschmidt
  2019-01-28  9:22       ` Miquel Raynal
  0 siblings, 1 reply; 35+ messages in thread
From: Simon Goldschmidt @ 2019-01-28  9:16 UTC (permalink / raw)
  To: u-boot

Hi Miquel,

On Mon, Jan 28, 2019 at 10:08 AM Miquel Raynal
<miquel.raynal@bootlin.com> wrote:
>
> Hi Simon,
>
> Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> wrote on Fri, 25
> Jan 2019 21:30:49 +0100:
>
> > This adds reset handling to the devicetree-enabled denalid nand driver.
>
> "Denali NAND driver"

Right.

>
> >
> > For backwards compatibility, only a warning is printed when failing to
> > get reset handles.
> >
> > Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> > ---
> >
> >  drivers/mtd/nand/raw/denali_dt.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/mtd/nand/raw/denali_dt.c b/drivers/mtd/nand/raw/denali_dt.c
> > index d384b974df..21c4540637 100644
> > --- a/drivers/mtd/nand/raw/denali_dt.c
> > +++ b/drivers/mtd/nand/raw/denali_dt.c
> > @@ -9,6 +9,7 @@
> >  #include <linux/io.h>
> >  #include <linux/ioport.h>
> >  #include <linux/printk.h>
> > +#include <reset.h>
> >
> >  #include "denali.h"
> >
> > @@ -64,6 +65,7 @@ static int denali_dt_probe(struct udevice *dev)
> >       const struct denali_dt_data *data;
> >       struct clk clk, clk_x, clk_ecc;
> >       struct resource res;
> > +     struct reset_ctl_bulk reset_bulk;
> >       int ret;
> >
> >       data = (void *)dev_get_driver_data(dev);
> > @@ -131,6 +133,12 @@ static int denali_dt_probe(struct udevice *dev)
> >               denali->clk_x_rate = 200000000;
> >       }
> >
> > +     ret = reset_get_bulk(bus, &reset_bulk);
> > +     if (ret)
> > +             dev_warn(bus, "Cant' get reset: %d\n", ret);
> > +     else
> > +             reset_deassert_bulk(&reset_bulk);
>
> Where are reset_get_bulk/reset_deassert_bulk() defined?

In reset.h. That's why I added this include (see above)...

Regards,
Simon

>
> > +
> >       return denali_init(denali);
> >  }
> >
>
> Thanks,
> Miquèl

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [U-Boot] [PATCH 3/5] mtd: rawnand: denali: add reset handling
  2019-01-28  9:16     ` Simon Goldschmidt
@ 2019-01-28  9:22       ` Miquel Raynal
  2019-01-28  9:30         ` Simon Goldschmidt
  0 siblings, 1 reply; 35+ messages in thread
From: Miquel Raynal @ 2019-01-28  9:22 UTC (permalink / raw)
  To: u-boot

Hi Simon,

Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> wrote on Mon, 28
Jan 2019 10:16:28 +0100:

> Hi Miquel,
> 
> On Mon, Jan 28, 2019 at 10:08 AM Miquel Raynal
> <miquel.raynal@bootlin.com> wrote:
> >
> > Hi Simon,
> >
> > Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> wrote on Fri, 25
> > Jan 2019 21:30:49 +0100:
> >  
> > > This adds reset handling to the devicetree-enabled denalid nand driver.  
> >
> > "Denali NAND driver"  
> 
> Right.
> 
> >  
> > >
> > > For backwards compatibility, only a warning is printed when failing to
> > > get reset handles.
> > >
> > > Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> > > ---
> > >
> > >  drivers/mtd/nand/raw/denali_dt.c | 8 ++++++++
> > >  1 file changed, 8 insertions(+)
> > >
> > > diff --git a/drivers/mtd/nand/raw/denali_dt.c b/drivers/mtd/nand/raw/denali_dt.c
> > > index d384b974df..21c4540637 100644
> > > --- a/drivers/mtd/nand/raw/denali_dt.c
> > > +++ b/drivers/mtd/nand/raw/denali_dt.c
> > > @@ -9,6 +9,7 @@
> > >  #include <linux/io.h>
> > >  #include <linux/ioport.h>
> > >  #include <linux/printk.h>
> > > +#include <reset.h>
> > >
> > >  #include "denali.h"
> > >
> > > @@ -64,6 +65,7 @@ static int denali_dt_probe(struct udevice *dev)
> > >       const struct denali_dt_data *data;
> > >       struct clk clk, clk_x, clk_ecc;
> > >       struct resource res;
> > > +     struct reset_ctl_bulk reset_bulk;
> > >       int ret;
> > >
> > >       data = (void *)dev_get_driver_data(dev);
> > > @@ -131,6 +133,12 @@ static int denali_dt_probe(struct udevice *dev)
> > >               denali->clk_x_rate = 200000000;
> > >       }
> > >
> > > +     ret = reset_get_bulk(bus, &reset_bulk);
> > > +     if (ret)
> > > +             dev_warn(bus, "Cant' get reset: %d\n", ret);
> > > +     else
> > > +             reset_deassert_bulk(&reset_bulk);  
> >
> > Where are reset_get_bulk/reset_deassert_bulk() defined?  
> 
> In reset.h. That's why I added this include (see above)...

Yes but I guess it is something that will be merged in 5.1? Maybe I
will wait for the next release to take this one.


Thanks,
Miquèl

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [U-Boot] [PATCH 3/5] mtd: rawnand: denali: add reset handling
  2019-01-28  9:22       ` Miquel Raynal
@ 2019-01-28  9:30         ` Simon Goldschmidt
  2019-01-28  9:33           ` Miquel Raynal
  0 siblings, 1 reply; 35+ messages in thread
From: Simon Goldschmidt @ 2019-01-28  9:30 UTC (permalink / raw)
  To: u-boot

Hi Miquel,

On Mon, Jan 28, 2019 at 10:22 AM Miquel Raynal
<miquel.raynal@bootlin.com> wrote:
>
> Hi Simon,
>
> Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> wrote on Mon, 28
> Jan 2019 10:16:28 +0100:
>
> > Hi Miquel,
> >
> > On Mon, Jan 28, 2019 at 10:08 AM Miquel Raynal
> > <miquel.raynal@bootlin.com> wrote:
> > >
> > > Hi Simon,
> > >
> > > Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> wrote on Fri, 25
> > > Jan 2019 21:30:49 +0100:
> > >
> > > > This adds reset handling to the devicetree-enabled denalid nand driver.
> > >
> > > "Denali NAND driver"
> >
> > Right.
> >
> > >
> > > >
> > > > For backwards compatibility, only a warning is printed when failing to
> > > > get reset handles.
> > > >
> > > > Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> > > > ---
> > > >
> > > >  drivers/mtd/nand/raw/denali_dt.c | 8 ++++++++
> > > >  1 file changed, 8 insertions(+)
> > > >
> > > > diff --git a/drivers/mtd/nand/raw/denali_dt.c b/drivers/mtd/nand/raw/denali_dt.c
> > > > index d384b974df..21c4540637 100644
> > > > --- a/drivers/mtd/nand/raw/denali_dt.c
> > > > +++ b/drivers/mtd/nand/raw/denali_dt.c
> > > > @@ -9,6 +9,7 @@
> > > >  #include <linux/io.h>
> > > >  #include <linux/ioport.h>
> > > >  #include <linux/printk.h>
> > > > +#include <reset.h>
> > > >
> > > >  #include "denali.h"
> > > >
> > > > @@ -64,6 +65,7 @@ static int denali_dt_probe(struct udevice *dev)
> > > >       const struct denali_dt_data *data;
> > > >       struct clk clk, clk_x, clk_ecc;
> > > >       struct resource res;
> > > > +     struct reset_ctl_bulk reset_bulk;
> > > >       int ret;
> > > >
> > > >       data = (void *)dev_get_driver_data(dev);
> > > > @@ -131,6 +133,12 @@ static int denali_dt_probe(struct udevice *dev)
> > > >               denali->clk_x_rate = 200000000;
> > > >       }
> > > >
> > > > +     ret = reset_get_bulk(bus, &reset_bulk);
> > > > +     if (ret)
> > > > +             dev_warn(bus, "Cant' get reset: %d\n", ret);
> > > > +     else
> > > > +             reset_deassert_bulk(&reset_bulk);
> > >
> > > Where are reset_get_bulk/reset_deassert_bulk() defined?
> >
> > In reset.h. That's why I added this include (see above)...
>
> Yes but I guess it is something that will be merged in 5.1? Maybe I
> will wait for the next release to take this one.

You mean Linux 5.1, right? This patch is for U-Boot, not Linux.

Regards,
Simon

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [U-Boot] [PATCH 3/5] mtd: rawnand: denali: add reset handling
  2019-01-28  9:30         ` Simon Goldschmidt
@ 2019-01-28  9:33           ` Miquel Raynal
  0 siblings, 0 replies; 35+ messages in thread
From: Miquel Raynal @ 2019-01-28  9:33 UTC (permalink / raw)
  To: u-boot

Hi Simon,

Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> wrote on Mon, 28
Jan 2019 10:30:12 +0100:

> Hi Miquel,
> 
> On Mon, Jan 28, 2019 at 10:22 AM Miquel Raynal
> <miquel.raynal@bootlin.com> wrote:
> >
> > Hi Simon,
> >
> > Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> wrote on Mon, 28
> > Jan 2019 10:16:28 +0100:
> >  
> > > Hi Miquel,
> > >
> > > On Mon, Jan 28, 2019 at 10:08 AM Miquel Raynal
> > > <miquel.raynal@bootlin.com> wrote:  
> > > >
> > > > Hi Simon,
> > > >
> > > > Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com> wrote on Fri, 25
> > > > Jan 2019 21:30:49 +0100:
> > > >  
> > > > > This adds reset handling to the devicetree-enabled denalid nand driver.  
> > > >
> > > > "Denali NAND driver"  
> > >
> > > Right.
> > >  
> > > >  
> > > > >
> > > > > For backwards compatibility, only a warning is printed when failing to
> > > > > get reset handles.
> > > > >
> > > > > Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> > > > > ---
> > > > >
> > > > >  drivers/mtd/nand/raw/denali_dt.c | 8 ++++++++
> > > > >  1 file changed, 8 insertions(+)
> > > > >
> > > > > diff --git a/drivers/mtd/nand/raw/denali_dt.c b/drivers/mtd/nand/raw/denali_dt.c
> > > > > index d384b974df..21c4540637 100644
> > > > > --- a/drivers/mtd/nand/raw/denali_dt.c
> > > > > +++ b/drivers/mtd/nand/raw/denali_dt.c
> > > > > @@ -9,6 +9,7 @@
> > > > >  #include <linux/io.h>
> > > > >  #include <linux/ioport.h>
> > > > >  #include <linux/printk.h>
> > > > > +#include <reset.h>
> > > > >
> > > > >  #include "denali.h"
> > > > >
> > > > > @@ -64,6 +65,7 @@ static int denali_dt_probe(struct udevice *dev)
> > > > >       const struct denali_dt_data *data;
> > > > >       struct clk clk, clk_x, clk_ecc;
> > > > >       struct resource res;
> > > > > +     struct reset_ctl_bulk reset_bulk;
> > > > >       int ret;
> > > > >
> > > > >       data = (void *)dev_get_driver_data(dev);
> > > > > @@ -131,6 +133,12 @@ static int denali_dt_probe(struct udevice *dev)
> > > > >               denali->clk_x_rate = 200000000;
> > > > >       }
> > > > >
> > > > > +     ret = reset_get_bulk(bus, &reset_bulk);
> > > > > +     if (ret)
> > > > > +             dev_warn(bus, "Cant' get reset: %d\n", ret);
> > > > > +     else
> > > > > +             reset_deassert_bulk(&reset_bulk);  
> > > >
> > > > Where are reset_get_bulk/reset_deassert_bulk() defined?  
> > >
> > > In reset.h. That's why I added this include (see above)...  
> >
> > Yes but I guess it is something that will be merged in 5.1? Maybe I
> > will wait for the next release to take this one.  
> 
> You mean Linux 5.1, right? This patch is for U-Boot, not Linux.

My bad, I had SIEVE rules issues, the patch did not got flagged for
"U-Boot". Sorry for the misunderstanding.

Thanks,
Miquèl

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [U-Boot] [PATCH 1/5] arm: socfpga: gen5: add reset handles to devicetrees
  2019-01-27  8:45     ` Simon Goldschmidt
@ 2019-01-28 10:54       ` Marek Vasut
  2019-01-28 11:46         ` Simon Goldschmidt
  0 siblings, 1 reply; 35+ messages in thread
From: Marek Vasut @ 2019-01-28 10:54 UTC (permalink / raw)
  To: u-boot

On 1/27/19 9:45 AM, Simon Goldschmidt wrote:
> Am 26.01.2019 um 09:57 schrieb Marek Vasut:
>> On 1/25/19 9:30 PM, Simon Goldschmidt wrote:
>>> The SPL for socfpga gen5 currently takes all peripherals out of reset
>>> unconditionally. To implement proper reset handling for peripherals,
>>>   some device tree nodes are missing reset handles.
>>>
>>> Since the socfpga gen5 devicetrees are currently in sync with Linux
>>> 4.20,
>>> and these reset handles are not added to Linux, yet, let's add them to
>>> U-Boot specific dtsi files to ease future merging.
>>>
>>> This adds a new file socfpga_u-boot.dtsi (note the '_' - if using a '-'
>>> this file gets pulled in for some a10/s10 devicetrees) that adds the
>>> missing reset handles.
>>>
>>> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
>>
>> Shouldn't all this be in u-boot specific DTs, instead of the ones
>> imported from Linux ?
> 
> The "u-boot,dm-pre-reloc" thing should of course stay in u-boot specific
> DTs. But I had to add many missing "resets = " references that are yet
> missing in upstream Linux DTs. Dinh is in the process of adding them and
> I'll make sure to sync that back to U-Boot once it's in Linux mainline
> git. I just did not want to wait until that happens...

Maybe you can just submit this patch to Linux too ?

-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [U-Boot] [PATCH 2/5] arm: socfpga: move SDR reset handling to driver
  2019-01-27  8:47     ` Simon Goldschmidt
@ 2019-01-28 10:55       ` Marek Vasut
  2019-01-28 11:49         ` Simon Goldschmidt
  0 siblings, 1 reply; 35+ messages in thread
From: Marek Vasut @ 2019-01-28 10:55 UTC (permalink / raw)
  To: u-boot

On 1/27/19 9:47 AM, Simon Goldschmidt wrote:
> Am 26.01.2019 um 09:58 schrieb Marek Vasut:
>> On 1/25/19 9:30 PM, Simon Goldschmidt wrote:
>>> To clean up reset handling for socfpga gen5, let's move the code snippet
>>> taking the DDR controller out of reset from SPL to the DDR driver.
>>>
>>> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
>>> ---
>>>
>>>   arch/arm/mach-socfpga/spl_gen5.c | 1 -
>>>   drivers/ddr/altera/sdram_gen5.c  | 4 ++++
>>>   2 files changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm/mach-socfpga/spl_gen5.c
>>> b/arch/arm/mach-socfpga/spl_gen5.c
>>> index ccdc661d05..f9bea892b1 100644
>>> --- a/arch/arm/mach-socfpga/spl_gen5.c
>>> +++ b/arch/arm/mach-socfpga/spl_gen5.c
>>> @@ -98,7 +98,6 @@ void board_init_f(ulong dummy)
>>>           socfpga_bridges_reset(1);
>>>       }
>>>   -    socfpga_per_reset(SOCFPGA_RESET(SDR), 0);
>>>       socfpga_per_reset(SOCFPGA_RESET(UART0), 0);
>>>       socfpga_per_reset(SOCFPGA_RESET(OSC1TIMER0), 0);
>>>   diff --git a/drivers/ddr/altera/sdram_gen5.c
>>> b/drivers/ddr/altera/sdram_gen5.c
>>> index 821060459c..bd54c420f8 100644
>>> --- a/drivers/ddr/altera/sdram_gen5.c
>>> +++ b/drivers/ddr/altera/sdram_gen5.c
>>> @@ -7,6 +7,7 @@
>>>   #include <div64.h>
>>>   #include <watchdog.h>
>>>   #include <asm/arch/fpga_manager.h>
>>> +#include <asm/arch/reset_manager.h>
>>>   #include <asm/arch/sdram.h>
>>>   #include <asm/arch/system_manager.h>
>>>   #include <asm/io.h>
>>> @@ -434,6 +435,9 @@ int sdram_mmr_init_full(unsigned int sdr_phy_reg)
>>>               SDR_CTRLGRP_DRAMADDRW_ROWBITS_LSB;
>>>       int ret;
>>>   +    /* release DDR from reset */
>>> +    socfpga_per_reset(SOCFPGA_RESET(SDR), 0);
>>> +
>>
>> Can the reset framework do this ?
> 
> Hmm, it probably could, but I see that as a diferent patch. The altera
> DDR driver currently does not work with devicetree, so using the reset
> framework here would be a bigger patch, I think.
> 
> Can we do that later and clean up this by just moving the code?

How much effort is it to switch this driver over to DM ?

-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [U-Boot] [PATCH 4/5] spi: cadence_qspi: add reset handling
  2019-01-27  8:49     ` Simon Goldschmidt
@ 2019-01-28 10:56       ` Marek Vasut
  0 siblings, 0 replies; 35+ messages in thread
From: Marek Vasut @ 2019-01-28 10:56 UTC (permalink / raw)
  To: u-boot

On 1/27/19 9:49 AM, Simon Goldschmidt wrote:
> Am 26.01.2019 um 09:59 schrieb Marek Vasut:
>> On 1/25/19 9:30 PM, Simon Goldschmidt wrote:
>>> This adds reset handling to the devicetree-enabled denalid nand driver.
>>>
>>> For backwards compatibility, only a warning is printed when failing to
>>> get reset handles.
>>>
>>> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
>>> ---
>>>
>>>   drivers/spi/cadence_qspi.c | 9 +++++++++
>>>   1 file changed, 9 insertions(+)
>>>
>>> diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c
>>> index 11fce9c4fe..f68c827e6d 100644
>>> --- a/drivers/spi/cadence_qspi.c
>>> +++ b/drivers/spi/cadence_qspi.c
>>> @@ -8,6 +8,7 @@
>>>   #include <dm.h>
>>>   #include <fdtdec.h>
>>>   #include <malloc.h>
>>> +#include <reset.h>
>>>   #include <spi.h>
>>>   #include <linux/errno.h>
>>>   #include "cadence_qspi.h"
>>> @@ -154,10 +155,18 @@ static int cadence_spi_probe(struct udevice *bus)
>>>   {
>>>       struct cadence_spi_platdata *plat = bus->platdata;
>>>       struct cadence_spi_priv *priv = dev_get_priv(bus);
>>> +    struct reset_ctl_bulk reset_bulk;
>>> +    int ret;
>>>         priv->regbase = plat->regbase;
>>>       priv->ahbbase = plat->ahbbase;
>>>   +    ret = reset_get_bulk(bus, &reset_bulk);
>>> +    if (ret)
>>> +        dev_warn(bus, "Cant' get reset: %d\n", ret);
>>
>> Can't
> 
> Right. Copy & paste from 3/5...
> 
>>
>> Also, shouldn't there be some reset_release or reset_free somewhere ?
>> What about .remove(), shouldn't that release the reset ?
> 
> Ehrm, well... this is my first driver using the reset framework. I
> haven't though of remove. I'll do a V2 for that soon.

Thanks

-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [U-Boot] [PATCH 5/5] arm: socfpga: implement proper peripheral reset
  2019-01-27  8:56     ` Simon Goldschmidt
@ 2019-01-28 10:58       ` Marek Vasut
  2019-01-28 11:50         ` Simon Goldschmidt
  0 siblings, 1 reply; 35+ messages in thread
From: Marek Vasut @ 2019-01-28 10:58 UTC (permalink / raw)
  To: u-boot

On 1/27/19 9:56 AM, Simon Goldschmidt wrote:
> Am 26.01.2019 um 10:00 schrieb Marek Vasut:
>> On 1/25/19 9:30 PM, Simon Goldschmidt wrote:
>>> This commit removes ad-hoc reset handling for peripheral resets from SPL
>>> for socfpga gen5.
>>>
>>> This is done because as U-Boot drivers support reset handling by now.
>>>
>>> For kernels that don't support taking peripherals out of reset that are
>>> not enabled by U-Boot, a new Kconfig option "OLD_SOCFPGA_KERNEL_COMPAT"
>>> is added, which keeps the old behaviour of just enabling all
>>> peripherals.
>>>
>>> This new option is enabled by default for now, as even Linux 4.20 does
>>> not support reset handling on all peripherals.
>>>
>>> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
>>
>> Well ew, don't we have some pre-linux-boot hook which could just flip
>> the reset register bits in one place ? And also, I'd prefer this to be
>> runtime-configurable, not compile-time configurable.
> 
> 
> And this will be the biggest change for v2. It's also why I needed so
> long to send this series. What I sent in this version is how Tom said
> the same thing is being handled for mach-sunxi (Kconfig
> "OLD_SUNXI_KERNEL_COMPAT").
> 
> So if I remember your last mails about this topic, you'd prefer an
> environment variable with a callback that leads to enabling all
> peripherals just before jumping to Linux (yes, I'll find a hook for that).

Rather, I'd prefer some .remove() code in the SoCFPGA reset driver which
would check an environment variable to determine whether it should
ungate the resets before booting Linux or not.

> I see two problems here:
> - enabling all peripherals will need to be the default for some time,
> until Linux supports reset handling in all drivers. How do you want to
> deal with that?

Set the env variable to default=1 for all socfpga boards ?

> - is this scheme used somewhere else for compatibility? I think it would
> make sense for multiple architectures to use the same way to handle such
> compatibility issues...

I don't think so, however it's better than compile-time setting.

[...]

-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [U-Boot] [PATCH 1/5] arm: socfpga: gen5: add reset handles to devicetrees
  2019-01-28 10:54       ` Marek Vasut
@ 2019-01-28 11:46         ` Simon Goldschmidt
  2019-01-28 11:48           ` Marek Vasut
  0 siblings, 1 reply; 35+ messages in thread
From: Simon Goldschmidt @ 2019-01-28 11:46 UTC (permalink / raw)
  To: u-boot

On Mon, Jan 28, 2019 at 12:30 PM Marek Vasut <marex@denx.de> wrote:
>
> On 1/27/19 9:45 AM, Simon Goldschmidt wrote:
> > Am 26.01.2019 um 09:57 schrieb Marek Vasut:
> >> On 1/25/19 9:30 PM, Simon Goldschmidt wrote:
> >>> The SPL for socfpga gen5 currently takes all peripherals out of reset
> >>> unconditionally. To implement proper reset handling for peripherals,
> >>>   some device tree nodes are missing reset handles.
> >>>
> >>> Since the socfpga gen5 devicetrees are currently in sync with Linux
> >>> 4.20,
> >>> and these reset handles are not added to Linux, yet, let's add them to
> >>> U-Boot specific dtsi files to ease future merging.
> >>>
> >>> This adds a new file socfpga_u-boot.dtsi (note the '_' - if using a '-'
> >>> this file gets pulled in for some a10/s10 devicetrees) that adds the
> >>> missing reset handles.
> >>>
> >>> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> >>
> >> Shouldn't all this be in u-boot specific DTs, instead of the ones
> >> imported from Linux ?
> >
> > The "u-boot,dm-pre-reloc" thing should of course stay in u-boot specific
> > DTs. But I had to add many missing "resets = " references that are yet
> > missing in upstream Linux DTs. Dinh is in the process of adding them and
> > I'll make sure to sync that back to U-Boot once it's in Linux mainline
> > git. I just did not want to wait until that happens...
>
> Maybe you can just submit this patch to Linux too ?

Dinh has already done that, that's why I have him in the loop here.
But his patch was missing some reset pins, so this would need another update.

Regards,
Simon

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [U-Boot] [PATCH 1/5] arm: socfpga: gen5: add reset handles to devicetrees
  2019-01-28 11:46         ` Simon Goldschmidt
@ 2019-01-28 11:48           ` Marek Vasut
  0 siblings, 0 replies; 35+ messages in thread
From: Marek Vasut @ 2019-01-28 11:48 UTC (permalink / raw)
  To: u-boot

On 1/28/19 12:46 PM, Simon Goldschmidt wrote:
> On Mon, Jan 28, 2019 at 12:30 PM Marek Vasut <marex@denx.de> wrote:
>>
>> On 1/27/19 9:45 AM, Simon Goldschmidt wrote:
>>> Am 26.01.2019 um 09:57 schrieb Marek Vasut:
>>>> On 1/25/19 9:30 PM, Simon Goldschmidt wrote:
>>>>> The SPL for socfpga gen5 currently takes all peripherals out of reset
>>>>> unconditionally. To implement proper reset handling for peripherals,
>>>>>   some device tree nodes are missing reset handles.
>>>>>
>>>>> Since the socfpga gen5 devicetrees are currently in sync with Linux
>>>>> 4.20,
>>>>> and these reset handles are not added to Linux, yet, let's add them to
>>>>> U-Boot specific dtsi files to ease future merging.
>>>>>
>>>>> This adds a new file socfpga_u-boot.dtsi (note the '_' - if using a '-'
>>>>> this file gets pulled in for some a10/s10 devicetrees) that adds the
>>>>> missing reset handles.
>>>>>
>>>>> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
>>>>
>>>> Shouldn't all this be in u-boot specific DTs, instead of the ones
>>>> imported from Linux ?
>>>
>>> The "u-boot,dm-pre-reloc" thing should of course stay in u-boot specific
>>> DTs. But I had to add many missing "resets = " references that are yet
>>> missing in upstream Linux DTs. Dinh is in the process of adding them and
>>> I'll make sure to sync that back to U-Boot once it's in Linux mainline
>>> git. I just did not want to wait until that happens...
>>
>> Maybe you can just submit this patch to Linux too ?
> 
> Dinh has already done that, that's why I have him in the loop here.
> But his patch was missing some reset pins, so this would need another update.

OK, just make sure the DTs stay in sync.

-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [U-Boot] [PATCH 2/5] arm: socfpga: move SDR reset handling to driver
  2019-01-28 10:55       ` Marek Vasut
@ 2019-01-28 11:49         ` Simon Goldschmidt
  2019-01-28 11:58           ` Marek Vasut
  0 siblings, 1 reply; 35+ messages in thread
From: Simon Goldschmidt @ 2019-01-28 11:49 UTC (permalink / raw)
  To: u-boot

On Mon, Jan 28, 2019 at 12:30 PM Marek Vasut <marex@denx.de> wrote:
>
> On 1/27/19 9:47 AM, Simon Goldschmidt wrote:
> > Am 26.01.2019 um 09:58 schrieb Marek Vasut:
> >> On 1/25/19 9:30 PM, Simon Goldschmidt wrote:
> >>> To clean up reset handling for socfpga gen5, let's move the code snippet
> >>> taking the DDR controller out of reset from SPL to the DDR driver.
> >>>
> >>> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> >>> ---
> >>>
> >>>   arch/arm/mach-socfpga/spl_gen5.c | 1 -
> >>>   drivers/ddr/altera/sdram_gen5.c  | 4 ++++
> >>>   2 files changed, 4 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/arch/arm/mach-socfpga/spl_gen5.c
> >>> b/arch/arm/mach-socfpga/spl_gen5.c
> >>> index ccdc661d05..f9bea892b1 100644
> >>> --- a/arch/arm/mach-socfpga/spl_gen5.c
> >>> +++ b/arch/arm/mach-socfpga/spl_gen5.c
> >>> @@ -98,7 +98,6 @@ void board_init_f(ulong dummy)
> >>>           socfpga_bridges_reset(1);
> >>>       }
> >>>   -    socfpga_per_reset(SOCFPGA_RESET(SDR), 0);
> >>>       socfpga_per_reset(SOCFPGA_RESET(UART0), 0);
> >>>       socfpga_per_reset(SOCFPGA_RESET(OSC1TIMER0), 0);
> >>>   diff --git a/drivers/ddr/altera/sdram_gen5.c
> >>> b/drivers/ddr/altera/sdram_gen5.c
> >>> index 821060459c..bd54c420f8 100644
> >>> --- a/drivers/ddr/altera/sdram_gen5.c
> >>> +++ b/drivers/ddr/altera/sdram_gen5.c
> >>> @@ -7,6 +7,7 @@
> >>>   #include <div64.h>
> >>>   #include <watchdog.h>
> >>>   #include <asm/arch/fpga_manager.h>
> >>> +#include <asm/arch/reset_manager.h>
> >>>   #include <asm/arch/sdram.h>
> >>>   #include <asm/arch/system_manager.h>
> >>>   #include <asm/io.h>
> >>> @@ -434,6 +435,9 @@ int sdram_mmr_init_full(unsigned int sdr_phy_reg)
> >>>               SDR_CTRLGRP_DRAMADDRW_ROWBITS_LSB;
> >>>       int ret;
> >>>   +    /* release DDR from reset */
> >>> +    socfpga_per_reset(SOCFPGA_RESET(SDR), 0);
> >>> +
> >>
> >> Can the reset framework do this ?
> >
> > Hmm, it probably could, but I see that as a diferent patch. The altera
> > DDR driver currently does not work with devicetree, so using the reset
> > framework here would be a bigger patch, I think.
> >
> > Can we do that later and clean up this by just moving the code?
>
> How much effort is it to switch this driver over to DM ?

I really don't know. Searching through drivers/ddr does not seem to give me
an example of a DTS-enabled ddr driver. Given that, I'd rather just push this
patch now. While it's true that it doesn't clean up everything, it's
not as if it
would make things worse.

Regards,
Simon

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [U-Boot] [PATCH 5/5] arm: socfpga: implement proper peripheral reset
  2019-01-28 10:58       ` Marek Vasut
@ 2019-01-28 11:50         ` Simon Goldschmidt
  0 siblings, 0 replies; 35+ messages in thread
From: Simon Goldschmidt @ 2019-01-28 11:50 UTC (permalink / raw)
  To: u-boot

On Mon, Jan 28, 2019 at 12:30 PM Marek Vasut <marex@denx.de> wrote:
>
> On 1/27/19 9:56 AM, Simon Goldschmidt wrote:
> > Am 26.01.2019 um 10:00 schrieb Marek Vasut:
> >> On 1/25/19 9:30 PM, Simon Goldschmidt wrote:
> >>> This commit removes ad-hoc reset handling for peripheral resets from SPL
> >>> for socfpga gen5.
> >>>
> >>> This is done because as U-Boot drivers support reset handling by now.
> >>>
> >>> For kernels that don't support taking peripherals out of reset that are
> >>> not enabled by U-Boot, a new Kconfig option "OLD_SOCFPGA_KERNEL_COMPAT"
> >>> is added, which keeps the old behaviour of just enabling all
> >>> peripherals.
> >>>
> >>> This new option is enabled by default for now, as even Linux 4.20 does
> >>> not support reset handling on all peripherals.
> >>>
> >>> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> >>
> >> Well ew, don't we have some pre-linux-boot hook which could just flip
> >> the reset register bits in one place ? And also, I'd prefer this to be
> >> runtime-configurable, not compile-time configurable.
> >
> >
> > And this will be the biggest change for v2. It's also why I needed so
> > long to send this series. What I sent in this version is how Tom said
> > the same thing is being handled for mach-sunxi (Kconfig
> > "OLD_SUNXI_KERNEL_COMPAT").
> >
> > So if I remember your last mails about this topic, you'd prefer an
> > environment variable with a callback that leads to enabling all
> > peripherals just before jumping to Linux (yes, I'll find a hook for that).
>
> Rather, I'd prefer some .remove() code in the SoCFPGA reset driver which
> would check an environment variable to determine whether it should
> ungate the resets before booting Linux or not.

OK, that should work. I'll work on that for v2.

Regards,
Simon

>
> > I see two problems here:
> > - enabling all peripherals will need to be the default for some time,
> > until Linux supports reset handling in all drivers. How do you want to
> > deal with that?
>
> Set the env variable to default=1 for all socfpga boards ?
>
> > - is this scheme used somewhere else for compatibility? I think it would
> > make sense for multiple architectures to use the same way to handle such
> > compatibility issues...
>
> I don't think so, however it's better than compile-time setting.
>
> [...]
>
> --
> Best regards,
> Marek Vasut

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [U-Boot] [PATCH 2/5] arm: socfpga: move SDR reset handling to driver
  2019-01-28 11:49         ` Simon Goldschmidt
@ 2019-01-28 11:58           ` Marek Vasut
  2019-01-28 12:38             ` Simon Goldschmidt
  0 siblings, 1 reply; 35+ messages in thread
From: Marek Vasut @ 2019-01-28 11:58 UTC (permalink / raw)
  To: u-boot

On 1/28/19 12:49 PM, Simon Goldschmidt wrote:
> On Mon, Jan 28, 2019 at 12:30 PM Marek Vasut <marex@denx.de> wrote:
>>
>> On 1/27/19 9:47 AM, Simon Goldschmidt wrote:
>>> Am 26.01.2019 um 09:58 schrieb Marek Vasut:
>>>> On 1/25/19 9:30 PM, Simon Goldschmidt wrote:
>>>>> To clean up reset handling for socfpga gen5, let's move the code snippet
>>>>> taking the DDR controller out of reset from SPL to the DDR driver.
>>>>>
>>>>> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
>>>>> ---
>>>>>
>>>>>   arch/arm/mach-socfpga/spl_gen5.c | 1 -
>>>>>   drivers/ddr/altera/sdram_gen5.c  | 4 ++++
>>>>>   2 files changed, 4 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/arch/arm/mach-socfpga/spl_gen5.c
>>>>> b/arch/arm/mach-socfpga/spl_gen5.c
>>>>> index ccdc661d05..f9bea892b1 100644
>>>>> --- a/arch/arm/mach-socfpga/spl_gen5.c
>>>>> +++ b/arch/arm/mach-socfpga/spl_gen5.c
>>>>> @@ -98,7 +98,6 @@ void board_init_f(ulong dummy)
>>>>>           socfpga_bridges_reset(1);
>>>>>       }
>>>>>   -    socfpga_per_reset(SOCFPGA_RESET(SDR), 0);
>>>>>       socfpga_per_reset(SOCFPGA_RESET(UART0), 0);
>>>>>       socfpga_per_reset(SOCFPGA_RESET(OSC1TIMER0), 0);
>>>>>   diff --git a/drivers/ddr/altera/sdram_gen5.c
>>>>> b/drivers/ddr/altera/sdram_gen5.c
>>>>> index 821060459c..bd54c420f8 100644
>>>>> --- a/drivers/ddr/altera/sdram_gen5.c
>>>>> +++ b/drivers/ddr/altera/sdram_gen5.c
>>>>> @@ -7,6 +7,7 @@
>>>>>   #include <div64.h>
>>>>>   #include <watchdog.h>
>>>>>   #include <asm/arch/fpga_manager.h>
>>>>> +#include <asm/arch/reset_manager.h>
>>>>>   #include <asm/arch/sdram.h>
>>>>>   #include <asm/arch/system_manager.h>
>>>>>   #include <asm/io.h>
>>>>> @@ -434,6 +435,9 @@ int sdram_mmr_init_full(unsigned int sdr_phy_reg)
>>>>>               SDR_CTRLGRP_DRAMADDRW_ROWBITS_LSB;
>>>>>       int ret;
>>>>>   +    /* release DDR from reset */
>>>>> +    socfpga_per_reset(SOCFPGA_RESET(SDR), 0);
>>>>> +
>>>>
>>>> Can the reset framework do this ?
>>>
>>> Hmm, it probably could, but I see that as a diferent patch. The altera
>>> DDR driver currently does not work with devicetree, so using the reset
>>> framework here would be a bigger patch, I think.
>>>
>>> Can we do that later and clean up this by just moving the code?
>>
>> How much effort is it to switch this driver over to DM ?
> 
> I really don't know. Searching through drivers/ddr does not seem to give me
> an example of a DTS-enabled ddr driver. Given that, I'd rather just push this
> patch now. While it's true that it doesn't clean up everything, it's
> not as if it
> would make things worse.

That's a valid point.

I guess you can add DRAM uclass and just probe the driver, which should
be all that's needed.

-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [U-Boot] [PATCH 2/5] arm: socfpga: move SDR reset handling to driver
  2019-01-28 11:58           ` Marek Vasut
@ 2019-01-28 12:38             ` Simon Goldschmidt
  2019-01-28 19:02               ` Marek Vasut
  0 siblings, 1 reply; 35+ messages in thread
From: Simon Goldschmidt @ 2019-01-28 12:38 UTC (permalink / raw)
  To: u-boot

On Mon, Jan 28, 2019 at 12:59 PM Marek Vasut <marex@denx.de> wrote:
>
> On 1/28/19 12:49 PM, Simon Goldschmidt wrote:
> > On Mon, Jan 28, 2019 at 12:30 PM Marek Vasut <marex@denx.de> wrote:
> >>
> >> On 1/27/19 9:47 AM, Simon Goldschmidt wrote:
> >>> Am 26.01.2019 um 09:58 schrieb Marek Vasut:
> >>>> On 1/25/19 9:30 PM, Simon Goldschmidt wrote:
> >>>>> To clean up reset handling for socfpga gen5, let's move the code snippet
> >>>>> taking the DDR controller out of reset from SPL to the DDR driver.
> >>>>>
> >>>>> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> >>>>> ---
> >>>>>
> >>>>>   arch/arm/mach-socfpga/spl_gen5.c | 1 -
> >>>>>   drivers/ddr/altera/sdram_gen5.c  | 4 ++++
> >>>>>   2 files changed, 4 insertions(+), 1 deletion(-)
> >>>>>
> >>>>> diff --git a/arch/arm/mach-socfpga/spl_gen5.c
> >>>>> b/arch/arm/mach-socfpga/spl_gen5.c
> >>>>> index ccdc661d05..f9bea892b1 100644
> >>>>> --- a/arch/arm/mach-socfpga/spl_gen5.c
> >>>>> +++ b/arch/arm/mach-socfpga/spl_gen5.c
> >>>>> @@ -98,7 +98,6 @@ void board_init_f(ulong dummy)
> >>>>>           socfpga_bridges_reset(1);
> >>>>>       }
> >>>>>   -    socfpga_per_reset(SOCFPGA_RESET(SDR), 0);
> >>>>>       socfpga_per_reset(SOCFPGA_RESET(UART0), 0);
> >>>>>       socfpga_per_reset(SOCFPGA_RESET(OSC1TIMER0), 0);
> >>>>>   diff --git a/drivers/ddr/altera/sdram_gen5.c
> >>>>> b/drivers/ddr/altera/sdram_gen5.c
> >>>>> index 821060459c..bd54c420f8 100644
> >>>>> --- a/drivers/ddr/altera/sdram_gen5.c
> >>>>> +++ b/drivers/ddr/altera/sdram_gen5.c
> >>>>> @@ -7,6 +7,7 @@
> >>>>>   #include <div64.h>
> >>>>>   #include <watchdog.h>
> >>>>>   #include <asm/arch/fpga_manager.h>
> >>>>> +#include <asm/arch/reset_manager.h>
> >>>>>   #include <asm/arch/sdram.h>
> >>>>>   #include <asm/arch/system_manager.h>
> >>>>>   #include <asm/io.h>
> >>>>> @@ -434,6 +435,9 @@ int sdram_mmr_init_full(unsigned int sdr_phy_reg)
> >>>>>               SDR_CTRLGRP_DRAMADDRW_ROWBITS_LSB;
> >>>>>       int ret;
> >>>>>   +    /* release DDR from reset */
> >>>>> +    socfpga_per_reset(SOCFPGA_RESET(SDR), 0);
> >>>>> +
> >>>>
> >>>> Can the reset framework do this ?
> >>>
> >>> Hmm, it probably could, but I see that as a diferent patch. The altera
> >>> DDR driver currently does not work with devicetree, so using the reset
> >>> framework here would be a bigger patch, I think.
> >>>
> >>> Can we do that later and clean up this by just moving the code?
> >>
> >> How much effort is it to switch this driver over to DM ?
> >
> > I really don't know. Searching through drivers/ddr does not seem to give me
> > an example of a DTS-enabled ddr driver. Given that, I'd rather just push this
> > patch now. While it's true that it doesn't clean up everything, it's
> > not as if it
> > would make things worse.
>
> That's a valid point.
>
> I guess you can add DRAM uclass and just probe the driver, which should
> be all that's needed.

Hmm, there *is* a UCLASS_RAM, but its drivers are in 'drivers/ram'. Is there
any reason those are separated from 'drivers/ddr'?

Regards,
Simon

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [U-Boot] [PATCH 2/5] arm: socfpga: move SDR reset handling to driver
  2019-01-28 12:38             ` Simon Goldschmidt
@ 2019-01-28 19:02               ` Marek Vasut
  2019-01-28 19:17                 ` Simon Goldschmidt
  0 siblings, 1 reply; 35+ messages in thread
From: Marek Vasut @ 2019-01-28 19:02 UTC (permalink / raw)
  To: u-boot

On 1/28/19 1:38 PM, Simon Goldschmidt wrote:
> On Mon, Jan 28, 2019 at 12:59 PM Marek Vasut <marex@denx.de> wrote:
>>
>> On 1/28/19 12:49 PM, Simon Goldschmidt wrote:
>>> On Mon, Jan 28, 2019 at 12:30 PM Marek Vasut <marex@denx.de> wrote:
>>>>
>>>> On 1/27/19 9:47 AM, Simon Goldschmidt wrote:
>>>>> Am 26.01.2019 um 09:58 schrieb Marek Vasut:
>>>>>> On 1/25/19 9:30 PM, Simon Goldschmidt wrote:
>>>>>>> To clean up reset handling for socfpga gen5, let's move the code snippet
>>>>>>> taking the DDR controller out of reset from SPL to the DDR driver.
>>>>>>>
>>>>>>> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
>>>>>>> ---
>>>>>>>
>>>>>>>   arch/arm/mach-socfpga/spl_gen5.c | 1 -
>>>>>>>   drivers/ddr/altera/sdram_gen5.c  | 4 ++++
>>>>>>>   2 files changed, 4 insertions(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/arch/arm/mach-socfpga/spl_gen5.c
>>>>>>> b/arch/arm/mach-socfpga/spl_gen5.c
>>>>>>> index ccdc661d05..f9bea892b1 100644
>>>>>>> --- a/arch/arm/mach-socfpga/spl_gen5.c
>>>>>>> +++ b/arch/arm/mach-socfpga/spl_gen5.c
>>>>>>> @@ -98,7 +98,6 @@ void board_init_f(ulong dummy)
>>>>>>>           socfpga_bridges_reset(1);
>>>>>>>       }
>>>>>>>   -    socfpga_per_reset(SOCFPGA_RESET(SDR), 0);
>>>>>>>       socfpga_per_reset(SOCFPGA_RESET(UART0), 0);
>>>>>>>       socfpga_per_reset(SOCFPGA_RESET(OSC1TIMER0), 0);
>>>>>>>   diff --git a/drivers/ddr/altera/sdram_gen5.c
>>>>>>> b/drivers/ddr/altera/sdram_gen5.c
>>>>>>> index 821060459c..bd54c420f8 100644
>>>>>>> --- a/drivers/ddr/altera/sdram_gen5.c
>>>>>>> +++ b/drivers/ddr/altera/sdram_gen5.c
>>>>>>> @@ -7,6 +7,7 @@
>>>>>>>   #include <div64.h>
>>>>>>>   #include <watchdog.h>
>>>>>>>   #include <asm/arch/fpga_manager.h>
>>>>>>> +#include <asm/arch/reset_manager.h>
>>>>>>>   #include <asm/arch/sdram.h>
>>>>>>>   #include <asm/arch/system_manager.h>
>>>>>>>   #include <asm/io.h>
>>>>>>> @@ -434,6 +435,9 @@ int sdram_mmr_init_full(unsigned int sdr_phy_reg)
>>>>>>>               SDR_CTRLGRP_DRAMADDRW_ROWBITS_LSB;
>>>>>>>       int ret;
>>>>>>>   +    /* release DDR from reset */
>>>>>>> +    socfpga_per_reset(SOCFPGA_RESET(SDR), 0);
>>>>>>> +
>>>>>>
>>>>>> Can the reset framework do this ?
>>>>>
>>>>> Hmm, it probably could, but I see that as a diferent patch. The altera
>>>>> DDR driver currently does not work with devicetree, so using the reset
>>>>> framework here would be a bigger patch, I think.
>>>>>
>>>>> Can we do that later and clean up this by just moving the code?
>>>>
>>>> How much effort is it to switch this driver over to DM ?
>>>
>>> I really don't know. Searching through drivers/ddr does not seem to give me
>>> an example of a DTS-enabled ddr driver. Given that, I'd rather just push this
>>> patch now. While it's true that it doesn't clean up everything, it's
>>> not as if it
>>> would make things worse.
>>
>> That's a valid point.
>>
>> I guess you can add DRAM uclass and just probe the driver, which should
>> be all that's needed.
> 
> Hmm, there *is* a UCLASS_RAM, but its drivers are in 'drivers/ram'. Is there
> any reason those are separated from 'drivers/ddr'?

I don't think so, seems like these two directories should be merged.

-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [U-Boot] [PATCH 2/5] arm: socfpga: move SDR reset handling to driver
  2019-01-28 19:02               ` Marek Vasut
@ 2019-01-28 19:17                 ` Simon Goldschmidt
  2019-01-28 19:24                   ` Marek Vasut
  0 siblings, 1 reply; 35+ messages in thread
From: Simon Goldschmidt @ 2019-01-28 19:17 UTC (permalink / raw)
  To: u-boot

Am 28.01.2019 um 20:02 schrieb Marek Vasut:
> On 1/28/19 1:38 PM, Simon Goldschmidt wrote:
>> On Mon, Jan 28, 2019 at 12:59 PM Marek Vasut <marex@denx.de> wrote:
>>>
>>> On 1/28/19 12:49 PM, Simon Goldschmidt wrote:
>>>> On Mon, Jan 28, 2019 at 12:30 PM Marek Vasut <marex@denx.de> wrote:
>>>>>
>>>>> On 1/27/19 9:47 AM, Simon Goldschmidt wrote:
>>>>>> Am 26.01.2019 um 09:58 schrieb Marek Vasut:
>>>>>>> On 1/25/19 9:30 PM, Simon Goldschmidt wrote:
>>>>>>>> To clean up reset handling for socfpga gen5, let's move the code snippet
>>>>>>>> taking the DDR controller out of reset from SPL to the DDR driver.
>>>>>>>>
>>>>>>>> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
>>>>>>>> ---
>>>>>>>>
>>>>>>>>    arch/arm/mach-socfpga/spl_gen5.c | 1 -
>>>>>>>>    drivers/ddr/altera/sdram_gen5.c  | 4 ++++
>>>>>>>>    2 files changed, 4 insertions(+), 1 deletion(-)
>>>>>>>>
>>>>>>>> diff --git a/arch/arm/mach-socfpga/spl_gen5.c
>>>>>>>> b/arch/arm/mach-socfpga/spl_gen5.c
>>>>>>>> index ccdc661d05..f9bea892b1 100644
>>>>>>>> --- a/arch/arm/mach-socfpga/spl_gen5.c
>>>>>>>> +++ b/arch/arm/mach-socfpga/spl_gen5.c
>>>>>>>> @@ -98,7 +98,6 @@ void board_init_f(ulong dummy)
>>>>>>>>            socfpga_bridges_reset(1);
>>>>>>>>        }
>>>>>>>>    -    socfpga_per_reset(SOCFPGA_RESET(SDR), 0);
>>>>>>>>        socfpga_per_reset(SOCFPGA_RESET(UART0), 0);
>>>>>>>>        socfpga_per_reset(SOCFPGA_RESET(OSC1TIMER0), 0);
>>>>>>>>    diff --git a/drivers/ddr/altera/sdram_gen5.c
>>>>>>>> b/drivers/ddr/altera/sdram_gen5.c
>>>>>>>> index 821060459c..bd54c420f8 100644
>>>>>>>> --- a/drivers/ddr/altera/sdram_gen5.c
>>>>>>>> +++ b/drivers/ddr/altera/sdram_gen5.c
>>>>>>>> @@ -7,6 +7,7 @@
>>>>>>>>    #include <div64.h>
>>>>>>>>    #include <watchdog.h>
>>>>>>>>    #include <asm/arch/fpga_manager.h>
>>>>>>>> +#include <asm/arch/reset_manager.h>
>>>>>>>>    #include <asm/arch/sdram.h>
>>>>>>>>    #include <asm/arch/system_manager.h>
>>>>>>>>    #include <asm/io.h>
>>>>>>>> @@ -434,6 +435,9 @@ int sdram_mmr_init_full(unsigned int sdr_phy_reg)
>>>>>>>>                SDR_CTRLGRP_DRAMADDRW_ROWBITS_LSB;
>>>>>>>>        int ret;
>>>>>>>>    +    /* release DDR from reset */
>>>>>>>> +    socfpga_per_reset(SOCFPGA_RESET(SDR), 0);
>>>>>>>> +
>>>>>>>
>>>>>>> Can the reset framework do this ?
>>>>>>
>>>>>> Hmm, it probably could, but I see that as a diferent patch. The altera
>>>>>> DDR driver currently does not work with devicetree, so using the reset
>>>>>> framework here would be a bigger patch, I think.
>>>>>>
>>>>>> Can we do that later and clean up this by just moving the code?
>>>>>
>>>>> How much effort is it to switch this driver over to DM ?
>>>>
>>>> I really don't know. Searching through drivers/ddr does not seem to give me
>>>> an example of a DTS-enabled ddr driver. Given that, I'd rather just push this
>>>> patch now. While it's true that it doesn't clean up everything, it's
>>>> not as if it
>>>> would make things worse.
>>>
>>> That's a valid point.
>>>
>>> I guess you can add DRAM uclass and just probe the driver, which should
>>> be all that's needed.
>>
>> Hmm, there *is* a UCLASS_RAM, but its drivers are in 'drivers/ram'. Is there
>> any reason those are separated from 'drivers/ddr'?
> 
> I don't think so, seems like these two directories should be merged.

Yes, I think so too by now.

I'll see if I can change this patch to use UCLASS_RAM. A patch/series to 
merge drivers/ddr wih drivers/ram should be separate.

Regads,
Simon

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [U-Boot] [PATCH 2/5] arm: socfpga: move SDR reset handling to driver
  2019-01-28 19:17                 ` Simon Goldschmidt
@ 2019-01-28 19:24                   ` Marek Vasut
  2019-01-29  8:23                     ` Simon Goldschmidt
  0 siblings, 1 reply; 35+ messages in thread
From: Marek Vasut @ 2019-01-28 19:24 UTC (permalink / raw)
  To: u-boot

On 1/28/19 8:17 PM, Simon Goldschmidt wrote:
> Am 28.01.2019 um 20:02 schrieb Marek Vasut:
>> On 1/28/19 1:38 PM, Simon Goldschmidt wrote:
>>> On Mon, Jan 28, 2019 at 12:59 PM Marek Vasut <marex@denx.de> wrote:
>>>>
>>>> On 1/28/19 12:49 PM, Simon Goldschmidt wrote:
>>>>> On Mon, Jan 28, 2019 at 12:30 PM Marek Vasut <marex@denx.de> wrote:
>>>>>>
>>>>>> On 1/27/19 9:47 AM, Simon Goldschmidt wrote:
>>>>>>> Am 26.01.2019 um 09:58 schrieb Marek Vasut:
>>>>>>>> On 1/25/19 9:30 PM, Simon Goldschmidt wrote:
>>>>>>>>> To clean up reset handling for socfpga gen5, let's move the
>>>>>>>>> code snippet
>>>>>>>>> taking the DDR controller out of reset from SPL to the DDR driver.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
>>>>>>>>> ---
>>>>>>>>>
>>>>>>>>>    arch/arm/mach-socfpga/spl_gen5.c | 1 -
>>>>>>>>>    drivers/ddr/altera/sdram_gen5.c  | 4 ++++
>>>>>>>>>    2 files changed, 4 insertions(+), 1 deletion(-)
>>>>>>>>>
>>>>>>>>> diff --git a/arch/arm/mach-socfpga/spl_gen5.c
>>>>>>>>> b/arch/arm/mach-socfpga/spl_gen5.c
>>>>>>>>> index ccdc661d05..f9bea892b1 100644
>>>>>>>>> --- a/arch/arm/mach-socfpga/spl_gen5.c
>>>>>>>>> +++ b/arch/arm/mach-socfpga/spl_gen5.c
>>>>>>>>> @@ -98,7 +98,6 @@ void board_init_f(ulong dummy)
>>>>>>>>>            socfpga_bridges_reset(1);
>>>>>>>>>        }
>>>>>>>>>    -    socfpga_per_reset(SOCFPGA_RESET(SDR), 0);
>>>>>>>>>        socfpga_per_reset(SOCFPGA_RESET(UART0), 0);
>>>>>>>>>        socfpga_per_reset(SOCFPGA_RESET(OSC1TIMER0), 0);
>>>>>>>>>    diff --git a/drivers/ddr/altera/sdram_gen5.c
>>>>>>>>> b/drivers/ddr/altera/sdram_gen5.c
>>>>>>>>> index 821060459c..bd54c420f8 100644
>>>>>>>>> --- a/drivers/ddr/altera/sdram_gen5.c
>>>>>>>>> +++ b/drivers/ddr/altera/sdram_gen5.c
>>>>>>>>> @@ -7,6 +7,7 @@
>>>>>>>>>    #include <div64.h>
>>>>>>>>>    #include <watchdog.h>
>>>>>>>>>    #include <asm/arch/fpga_manager.h>
>>>>>>>>> +#include <asm/arch/reset_manager.h>
>>>>>>>>>    #include <asm/arch/sdram.h>
>>>>>>>>>    #include <asm/arch/system_manager.h>
>>>>>>>>>    #include <asm/io.h>
>>>>>>>>> @@ -434,6 +435,9 @@ int sdram_mmr_init_full(unsigned int
>>>>>>>>> sdr_phy_reg)
>>>>>>>>>                SDR_CTRLGRP_DRAMADDRW_ROWBITS_LSB;
>>>>>>>>>        int ret;
>>>>>>>>>    +    /* release DDR from reset */
>>>>>>>>> +    socfpga_per_reset(SOCFPGA_RESET(SDR), 0);
>>>>>>>>> +
>>>>>>>>
>>>>>>>> Can the reset framework do this ?
>>>>>>>
>>>>>>> Hmm, it probably could, but I see that as a diferent patch. The
>>>>>>> altera
>>>>>>> DDR driver currently does not work with devicetree, so using the
>>>>>>> reset
>>>>>>> framework here would be a bigger patch, I think.
>>>>>>>
>>>>>>> Can we do that later and clean up this by just moving the code?
>>>>>>
>>>>>> How much effort is it to switch this driver over to DM ?
>>>>>
>>>>> I really don't know. Searching through drivers/ddr does not seem to
>>>>> give me
>>>>> an example of a DTS-enabled ddr driver. Given that, I'd rather just
>>>>> push this
>>>>> patch now. While it's true that it doesn't clean up everything, it's
>>>>> not as if it
>>>>> would make things worse.
>>>>
>>>> That's a valid point.
>>>>
>>>> I guess you can add DRAM uclass and just probe the driver, which should
>>>> be all that's needed.
>>>
>>> Hmm, there *is* a UCLASS_RAM, but its drivers are in 'drivers/ram'.
>>> Is there
>>> any reason those are separated from 'drivers/ddr'?
>>
>> I don't think so, seems like these two directories should be merged.
> 
> Yes, I think so too by now.
> 
> I'll see if I can change this patch to use UCLASS_RAM. A patch/series to
> merge drivers/ddr wih drivers/ram should be separate.

Sounds good.

-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [U-Boot] [PATCH 2/5] arm: socfpga: move SDR reset handling to driver
  2019-01-28 19:24                   ` Marek Vasut
@ 2019-01-29  8:23                     ` Simon Goldschmidt
  2019-01-29  9:52                       ` Marek Vasut
  0 siblings, 1 reply; 35+ messages in thread
From: Simon Goldschmidt @ 2019-01-29  8:23 UTC (permalink / raw)
  To: u-boot

On Mon, Jan 28, 2019 at 8:25 PM Marek Vasut <marex@denx.de> wrote:
>
> On 1/28/19 8:17 PM, Simon Goldschmidt wrote:
> > Am 28.01.2019 um 20:02 schrieb Marek Vasut:
> >> On 1/28/19 1:38 PM, Simon Goldschmidt wrote:
> >>> On Mon, Jan 28, 2019 at 12:59 PM Marek Vasut <marex@denx.de> wrote:
> >>>>
> >>>> On 1/28/19 12:49 PM, Simon Goldschmidt wrote:
> >>>>> On Mon, Jan 28, 2019 at 12:30 PM Marek Vasut <marex@denx.de> wrote:
> >>>>>>
> >>>>>> On 1/27/19 9:47 AM, Simon Goldschmidt wrote:
> >>>>>>> Am 26.01.2019 um 09:58 schrieb Marek Vasut:
> >>>>>>>> On 1/25/19 9:30 PM, Simon Goldschmidt wrote:
> >>>>>>>>> To clean up reset handling for socfpga gen5, let's move the
> >>>>>>>>> code snippet
> >>>>>>>>> taking the DDR controller out of reset from SPL to the DDR driver.
> >>>>>>>>>
> >>>>>>>>> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> >>>>>>>>> ---
> >>>>>>>>>
> >>>>>>>>>    arch/arm/mach-socfpga/spl_gen5.c | 1 -
> >>>>>>>>>    drivers/ddr/altera/sdram_gen5.c  | 4 ++++
> >>>>>>>>>    2 files changed, 4 insertions(+), 1 deletion(-)
> >>>>>>>>>
> >>>>>>>>> diff --git a/arch/arm/mach-socfpga/spl_gen5.c
> >>>>>>>>> b/arch/arm/mach-socfpga/spl_gen5.c
> >>>>>>>>> index ccdc661d05..f9bea892b1 100644
> >>>>>>>>> --- a/arch/arm/mach-socfpga/spl_gen5.c
> >>>>>>>>> +++ b/arch/arm/mach-socfpga/spl_gen5.c
> >>>>>>>>> @@ -98,7 +98,6 @@ void board_init_f(ulong dummy)
> >>>>>>>>>            socfpga_bridges_reset(1);
> >>>>>>>>>        }
> >>>>>>>>>    -    socfpga_per_reset(SOCFPGA_RESET(SDR), 0);
> >>>>>>>>>        socfpga_per_reset(SOCFPGA_RESET(UART0), 0);
> >>>>>>>>>        socfpga_per_reset(SOCFPGA_RESET(OSC1TIMER0), 0);
> >>>>>>>>>    diff --git a/drivers/ddr/altera/sdram_gen5.c
> >>>>>>>>> b/drivers/ddr/altera/sdram_gen5.c
> >>>>>>>>> index 821060459c..bd54c420f8 100644
> >>>>>>>>> --- a/drivers/ddr/altera/sdram_gen5.c
> >>>>>>>>> +++ b/drivers/ddr/altera/sdram_gen5.c
> >>>>>>>>> @@ -7,6 +7,7 @@
> >>>>>>>>>    #include <div64.h>
> >>>>>>>>>    #include <watchdog.h>
> >>>>>>>>>    #include <asm/arch/fpga_manager.h>
> >>>>>>>>> +#include <asm/arch/reset_manager.h>
> >>>>>>>>>    #include <asm/arch/sdram.h>
> >>>>>>>>>    #include <asm/arch/system_manager.h>
> >>>>>>>>>    #include <asm/io.h>
> >>>>>>>>> @@ -434,6 +435,9 @@ int sdram_mmr_init_full(unsigned int
> >>>>>>>>> sdr_phy_reg)
> >>>>>>>>>                SDR_CTRLGRP_DRAMADDRW_ROWBITS_LSB;
> >>>>>>>>>        int ret;
> >>>>>>>>>    +    /* release DDR from reset */
> >>>>>>>>> +    socfpga_per_reset(SOCFPGA_RESET(SDR), 0);
> >>>>>>>>> +
> >>>>>>>>
> >>>>>>>> Can the reset framework do this ?
> >>>>>>>
> >>>>>>> Hmm, it probably could, but I see that as a diferent patch. The
> >>>>>>> altera
> >>>>>>> DDR driver currently does not work with devicetree, so using the
> >>>>>>> reset
> >>>>>>> framework here would be a bigger patch, I think.
> >>>>>>>
> >>>>>>> Can we do that later and clean up this by just moving the code?
> >>>>>>
> >>>>>> How much effort is it to switch this driver over to DM ?
> >>>>>
> >>>>> I really don't know. Searching through drivers/ddr does not seem to
> >>>>> give me
> >>>>> an example of a DTS-enabled ddr driver. Given that, I'd rather just
> >>>>> push this
> >>>>> patch now. While it's true that it doesn't clean up everything, it's
> >>>>> not as if it
> >>>>> would make things worse.
> >>>>
> >>>> That's a valid point.
> >>>>
> >>>> I guess you can add DRAM uclass and just probe the driver, which should
> >>>> be all that's needed.
> >>>
> >>> Hmm, there *is* a UCLASS_RAM, but its drivers are in 'drivers/ram'.
> >>> Is there
> >>> any reason those are separated from 'drivers/ddr'?
> >>
> >> I don't think so, seems like these two directories should be merged.
> >
> > Yes, I think so too by now.
> >
> > I'll see if I can change this patch to use UCLASS_RAM. A patch/series to
> > merge drivers/ddr wih drivers/ram should be separate.
>
> Sounds good.

It kind of works with UCLASS_RAM, but there's a problem with the devicetree:
it describes the SDR controller starting at 0xffc25000 where the official memory
map from Intel says it starts at 0xffc20000, but describes its
registers starting
from offset 0x5000. However, in U-Boot, the file
'drivers/ddr/altera/sequencer.c'
uses those lower addresses.

So now I can either change the dts to let SDR registers start at 0xffc20000
instead of 0xffc25000 (and in consequence adapt Linux, too) or add a new driver
for the sequencer that occupies this lower range (and make sdram_gen5.c use
it somehow).

Before I spin another round, what would be the preferred way to take?

Anyway, I failed to find public documentation for this sequencer thing. Do you
happen to know why it's done like this?

Regards,
Simon

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [U-Boot] [PATCH 2/5] arm: socfpga: move SDR reset handling to driver
  2019-01-29  8:23                     ` Simon Goldschmidt
@ 2019-01-29  9:52                       ` Marek Vasut
  0 siblings, 0 replies; 35+ messages in thread
From: Marek Vasut @ 2019-01-29  9:52 UTC (permalink / raw)
  To: u-boot

On 1/29/19 9:23 AM, Simon Goldschmidt wrote:
> On Mon, Jan 28, 2019 at 8:25 PM Marek Vasut <marex@denx.de> wrote:
>>
>> On 1/28/19 8:17 PM, Simon Goldschmidt wrote:
>>> Am 28.01.2019 um 20:02 schrieb Marek Vasut:
>>>> On 1/28/19 1:38 PM, Simon Goldschmidt wrote:
>>>>> On Mon, Jan 28, 2019 at 12:59 PM Marek Vasut <marex@denx.de> wrote:
>>>>>>
>>>>>> On 1/28/19 12:49 PM, Simon Goldschmidt wrote:
>>>>>>> On Mon, Jan 28, 2019 at 12:30 PM Marek Vasut <marex@denx.de> wrote:
>>>>>>>>
>>>>>>>> On 1/27/19 9:47 AM, Simon Goldschmidt wrote:
>>>>>>>>> Am 26.01.2019 um 09:58 schrieb Marek Vasut:
>>>>>>>>>> On 1/25/19 9:30 PM, Simon Goldschmidt wrote:
>>>>>>>>>>> To clean up reset handling for socfpga gen5, let's move the
>>>>>>>>>>> code snippet
>>>>>>>>>>> taking the DDR controller out of reset from SPL to the DDR driver.
>>>>>>>>>>>
>>>>>>>>>>> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
>>>>>>>>>>> ---
>>>>>>>>>>>
>>>>>>>>>>>    arch/arm/mach-socfpga/spl_gen5.c | 1 -
>>>>>>>>>>>    drivers/ddr/altera/sdram_gen5.c  | 4 ++++
>>>>>>>>>>>    2 files changed, 4 insertions(+), 1 deletion(-)
>>>>>>>>>>>
>>>>>>>>>>> diff --git a/arch/arm/mach-socfpga/spl_gen5.c
>>>>>>>>>>> b/arch/arm/mach-socfpga/spl_gen5.c
>>>>>>>>>>> index ccdc661d05..f9bea892b1 100644
>>>>>>>>>>> --- a/arch/arm/mach-socfpga/spl_gen5.c
>>>>>>>>>>> +++ b/arch/arm/mach-socfpga/spl_gen5.c
>>>>>>>>>>> @@ -98,7 +98,6 @@ void board_init_f(ulong dummy)
>>>>>>>>>>>            socfpga_bridges_reset(1);
>>>>>>>>>>>        }
>>>>>>>>>>>    -    socfpga_per_reset(SOCFPGA_RESET(SDR), 0);
>>>>>>>>>>>        socfpga_per_reset(SOCFPGA_RESET(UART0), 0);
>>>>>>>>>>>        socfpga_per_reset(SOCFPGA_RESET(OSC1TIMER0), 0);
>>>>>>>>>>>    diff --git a/drivers/ddr/altera/sdram_gen5.c
>>>>>>>>>>> b/drivers/ddr/altera/sdram_gen5.c
>>>>>>>>>>> index 821060459c..bd54c420f8 100644
>>>>>>>>>>> --- a/drivers/ddr/altera/sdram_gen5.c
>>>>>>>>>>> +++ b/drivers/ddr/altera/sdram_gen5.c
>>>>>>>>>>> @@ -7,6 +7,7 @@
>>>>>>>>>>>    #include <div64.h>
>>>>>>>>>>>    #include <watchdog.h>
>>>>>>>>>>>    #include <asm/arch/fpga_manager.h>
>>>>>>>>>>> +#include <asm/arch/reset_manager.h>
>>>>>>>>>>>    #include <asm/arch/sdram.h>
>>>>>>>>>>>    #include <asm/arch/system_manager.h>
>>>>>>>>>>>    #include <asm/io.h>
>>>>>>>>>>> @@ -434,6 +435,9 @@ int sdram_mmr_init_full(unsigned int
>>>>>>>>>>> sdr_phy_reg)
>>>>>>>>>>>                SDR_CTRLGRP_DRAMADDRW_ROWBITS_LSB;
>>>>>>>>>>>        int ret;
>>>>>>>>>>>    +    /* release DDR from reset */
>>>>>>>>>>> +    socfpga_per_reset(SOCFPGA_RESET(SDR), 0);
>>>>>>>>>>> +
>>>>>>>>>>
>>>>>>>>>> Can the reset framework do this ?
>>>>>>>>>
>>>>>>>>> Hmm, it probably could, but I see that as a diferent patch. The
>>>>>>>>> altera
>>>>>>>>> DDR driver currently does not work with devicetree, so using the
>>>>>>>>> reset
>>>>>>>>> framework here would be a bigger patch, I think.
>>>>>>>>>
>>>>>>>>> Can we do that later and clean up this by just moving the code?
>>>>>>>>
>>>>>>>> How much effort is it to switch this driver over to DM ?
>>>>>>>
>>>>>>> I really don't know. Searching through drivers/ddr does not seem to
>>>>>>> give me
>>>>>>> an example of a DTS-enabled ddr driver. Given that, I'd rather just
>>>>>>> push this
>>>>>>> patch now. While it's true that it doesn't clean up everything, it's
>>>>>>> not as if it
>>>>>>> would make things worse.
>>>>>>
>>>>>> That's a valid point.
>>>>>>
>>>>>> I guess you can add DRAM uclass and just probe the driver, which should
>>>>>> be all that's needed.
>>>>>
>>>>> Hmm, there *is* a UCLASS_RAM, but its drivers are in 'drivers/ram'.
>>>>> Is there
>>>>> any reason those are separated from 'drivers/ddr'?
>>>>
>>>> I don't think so, seems like these two directories should be merged.
>>>
>>> Yes, I think so too by now.
>>>
>>> I'll see if I can change this patch to use UCLASS_RAM. A patch/series to
>>> merge drivers/ddr wih drivers/ram should be separate.
>>
>> Sounds good.
> 
> It kind of works with UCLASS_RAM, but there's a problem with the devicetree:
> it describes the SDR controller starting at 0xffc25000 where the official memory
> map from Intel says it starts at 0xffc20000, but describes its
> registers starting
> from offset 0x5000. However, in U-Boot, the file
> 'drivers/ddr/altera/sequencer.c'
> uses those lower addresses.
> 
> So now I can either change the dts to let SDR registers start at 0xffc20000
> instead of 0xffc25000 (and in consequence adapt Linux, too) or add a new driver
> for the sequencer that occupies this lower range (and make sdram_gen5.c use
> it somehow).

Fix the DT, I think the DT is buggy. The DRAM controller probably starts
at 0xffc2_0000 and the various 4k chunks above 0xffc2_0000 are various
subcomponents of the DDR controller.

> Before I spin another round, what would be the preferred way to take?
> 
> Anyway, I failed to find public documentation for this sequencer thing. Do you
> happen to know why it's done like this?

I don't have one, but I _think_ it's a HW instance of the altera DDR3
controller which you can also put into the FPGA, and that one is in quartus.

-- 
Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 35+ messages in thread

end of thread, other threads:[~2019-01-29  9:52 UTC | newest]

Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-25 20:30 [U-Boot] [PATCH 0/5] arm: socfpga: implement proper peripheral reset handling Simon Goldschmidt
2019-01-25 20:30 ` [U-Boot] [PATCH 1/5] arm: socfpga: gen5: add reset handles to devicetrees Simon Goldschmidt
2019-01-26  8:57   ` Marek Vasut
2019-01-27  8:45     ` Simon Goldschmidt
2019-01-28 10:54       ` Marek Vasut
2019-01-28 11:46         ` Simon Goldschmidt
2019-01-28 11:48           ` Marek Vasut
2019-01-25 20:30 ` [U-Boot] [PATCH 2/5] arm: socfpga: move SDR reset handling to driver Simon Goldschmidt
2019-01-26  8:58   ` Marek Vasut
2019-01-27  8:47     ` Simon Goldschmidt
2019-01-28 10:55       ` Marek Vasut
2019-01-28 11:49         ` Simon Goldschmidt
2019-01-28 11:58           ` Marek Vasut
2019-01-28 12:38             ` Simon Goldschmidt
2019-01-28 19:02               ` Marek Vasut
2019-01-28 19:17                 ` Simon Goldschmidt
2019-01-28 19:24                   ` Marek Vasut
2019-01-29  8:23                     ` Simon Goldschmidt
2019-01-29  9:52                       ` Marek Vasut
2019-01-25 20:30 ` [U-Boot] [PATCH 3/5] mtd: rawnand: denali: add reset handling Simon Goldschmidt
2019-01-26  8:58   ` Marek Vasut
2019-01-28  9:08   ` Miquel Raynal
2019-01-28  9:16     ` Simon Goldschmidt
2019-01-28  9:22       ` Miquel Raynal
2019-01-28  9:30         ` Simon Goldschmidt
2019-01-28  9:33           ` Miquel Raynal
2019-01-25 20:30 ` [U-Boot] [PATCH 4/5] spi: cadence_qspi: " Simon Goldschmidt
2019-01-26  8:59   ` Marek Vasut
2019-01-27  8:49     ` Simon Goldschmidt
2019-01-28 10:56       ` Marek Vasut
2019-01-25 20:30 ` [U-Boot] [PATCH 5/5] arm: socfpga: implement proper peripheral reset Simon Goldschmidt
2019-01-26  9:00   ` Marek Vasut
2019-01-27  8:56     ` Simon Goldschmidt
2019-01-28 10:58       ` Marek Vasut
2019-01-28 11:50         ` Simon Goldschmidt

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.