[U-Boot] [PATCH v2 0/8] arm: socfpga: implement proper peripheral reset handling

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, the reset driver implements a compatibility mode where it takes all peripherals out of reset before jumpint to the OS if an environment variable "socfpga_permodrst_ungate=1" is found. This is enabled by default for socfpga gen5 boards, but should be moved to default off in the near future once a Linux kernel supports reset handling for all drivers.
Changes in v2: - cleanly merged Linux dts (moved change of SDR controller base address to a separate patch) - port DDR driver to DM UCLASS_RAM - don't change DDR calibration training driver (code got too big) - use reset.h code instead of socfpga_per_reset() - fix copy/paste issues - add .remove callback to release the resets - add .remove callback to release the resets - moved from Kernel option "OLD_SOCFPGA_KERNEL_COMPAT" to environment variable "socfpga_permodrst_ungate" - this patch is new in v2 - removed Kconfig option OLD_SOCFPGA_KERNEL_COMPAT since compatibility now uses an environment variable
Simon Goldschmidt (8): arm: socfpga: gen5: sync devicetrees to Linux arm: socfpga: gen5: add reset & sdr node to SPL devicetrees arm: socfpga: move gen5 SDR driver to DM mtd: rawnand: denali: add reset handling spi: cadence_qspi: add reset handling reset: socfpga: add reset handling for old kernels arm: socfpga: gen5: deassert peripheral reset by default arm: socfpga: implement proper peripheral reset
arch/arm/Kconfig | 2 + arch/arm/dts/socfpga.dtsi | 23 ++- arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi | 8 + arch/arm/dts/socfpga_cyclone5_dbm_soc1.dts | 8 + .../socfpga_cyclone5_de0_nano_soc-u-boot.dtsi | 8 + arch/arm/dts/socfpga_cyclone5_de10_nano.dts | 8 + arch/arm/dts/socfpga_cyclone5_de1_soc.dts | 8 + arch/arm/dts/socfpga_cyclone5_is1.dts | 8 + .../dts/socfpga_cyclone5_socdk-u-boot.dtsi | 8 + .../dts/socfpga_cyclone5_sockit-u-boot.dtsi | 8 + .../dts/socfpga_cyclone5_socrates-u-boot.dtsi | 8 + arch/arm/dts/socfpga_cyclone5_socrates.dts | 2 - arch/arm/dts/socfpga_cyclone5_sr1500.dts | 8 + .../socfpga_cyclone5_vining_fpga-u-boot.dtsi | 8 + .../mach-socfpga/include/mach/sdram_gen5.h | 4 - arch/arm/mach-socfpga/misc_gen5.c | 10 -- arch/arm/mach-socfpga/spl_gen5.c | 39 ++--- drivers/ddr/altera/Kconfig | 1 + drivers/ddr/altera/sdram_gen5.c | 143 ++++++++++++++++-- drivers/ddr/altera/sequencer.c | 9 +- drivers/ddr/altera/sequencer.h | 35 +++++ drivers/mtd/nand/raw/denali.h | 2 + drivers/mtd/nand/raw/denali_dt.c | 14 ++ drivers/reset/reset-socfpga.c | 34 +++++ drivers/spi/cadence_qspi.c | 16 ++ drivers/spi/cadence_qspi.h | 4 + include/configs/socfpga_common.h | 14 ++ 27 files changed, 374 insertions(+), 66 deletions(-)

This is again a sync to linux-next + pending patches in Dinh's tree at commit 1c909b2dfe6a ("ARM: dts: socfpga: update more missing reset properties")'
It adds missing peripheral reset properties to socfpga.dtsi and removes U-Boot specific leftovers from socfpga_cyclone5_socrates.dts.
Signed-off-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com ---
Changes in v2: - cleanly merged Linux dts (moved change of SDR controller base address to a separate patch)
arch/arm/dts/socfpga.dtsi | 19 +++++++++++++++++-- arch/arm/dts/socfpga_cyclone5_socrates.dts | 2 -- 2 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/arch/arm/dts/socfpga.dtsi b/arch/arm/dts/socfpga.dtsi index 2458d6707d..ec1966480f 100644 --- a/arch/arm/dts/socfpga.dtsi +++ b/arch/arm/dts/socfpga.dtsi @@ -84,6 +84,7 @@ #dma-requests = <32>; clocks = <&l4_main_clk>; clock-names = "apb_pclk"; + resets = <&rst DMA_RESET>; }; };
@@ -100,6 +101,7 @@ reg = <0xffc00000 0x1000>; interrupts = <0 131 4>, <0 132 4>, <0 133 4>, <0 134 4>; clocks = <&can0_clk>; + resets = <&rst CAN0_RESET>; status = "disabled"; };
@@ -108,6 +110,7 @@ reg = <0xffc01000 0x1000>; interrupts = <0 135 4>, <0 136 4>, <0 137 4>, <0 138 4>; clocks = <&can1_clk>; + resets = <&rst CAN1_RESET>; status = "disabled"; };
@@ -585,6 +588,7 @@ compatible = "snps,dw-apb-gpio"; reg = <0xff708000 0x1000>; clocks = <&l4_mp_clk>; + resets = <&rst GPIO0_RESET>; status = "disabled";
porta: gpio-controller@0 { @@ -605,6 +609,7 @@ compatible = "snps,dw-apb-gpio"; reg = <0xff709000 0x1000>; clocks = <&l4_mp_clk>; + resets = <&rst GPIO1_RESET>; status = "disabled";
portb: gpio-controller@0 { @@ -625,6 +630,7 @@ compatible = "snps,dw-apb-gpio"; reg = <0xff70a000 0x1000>; clocks = <&l4_mp_clk>; + resets = <&rst GPIO2_RESET>; status = "disabled";
portc: gpio-controller@0 { @@ -735,6 +741,7 @@ #size-cells = <0>; clocks = <&l4_mp_clk>, <&sdmmc_clk_divided>; clock-names = "biu", "ciu"; + resets = <&rst SDMMC_RESET>; status = "disabled"; };
@@ -746,9 +753,9 @@ <0xffb80000 0x10000>; reg-names = "nand_data", "denali_reg"; interrupts = <0x0 0x90 0x4>; - dma-mask = <0xffffffff>; clocks = <&nand_clk>, <&nand_x_clk>, <&nand_ecc_clk>; clock-names = "nand", "nand_x", "ecc"; + resets = <&rst NAND_RESET>; status = "disabled"; };
@@ -759,7 +766,7 @@
qspi: spi@ff705000 { compatible = "cdns,qspi-nor"; - #address-cells = <1>; + #address-cells = <1>; #size-cells = <0>; reg = <0xff705000 0x1000>, <0xffa00000 0x1000>; @@ -768,6 +775,7 @@ cdns,fifo-width = <4>; cdns,trigger-address = <0x00000000>; clocks = <&qspi_clk>; + resets = <&rst QSPI_RESET>; status = "disabled"; };
@@ -786,6 +794,7 @@ sdr: sdr@ffc25000 { compatible = "altr,sdr-ctl", "syscon"; reg = <0xffc25000 0x1000>; + resets = <&rst SDR_RESET>; };
sdramedac { @@ -802,6 +811,7 @@ interrupts = <0 154 4>; num-cs = <4>; clocks = <&spi_m_clk>; + resets = <&rst SPIM0_RESET>; status = "disabled"; };
@@ -813,6 +823,7 @@ interrupts = <0 155 4>; num-cs = <4>; clocks = <&spi_m_clk>; + resets = <&rst SPIM1_RESET>; status = "disabled"; };
@@ -879,6 +890,7 @@ dmas = <&pdma 28>, <&pdma 29>; dma-names = "tx", "rx"; + resets = <&rst UART0_RESET>; };
uart1: serial1@ffc03000 { @@ -891,6 +903,7 @@ dmas = <&pdma 30>, <&pdma 31>; dma-names = "tx", "rx"; + resets = <&rst UART1_RESET>; };
usbphy0: usbphy { @@ -930,6 +943,7 @@ reg = <0xffd02000 0x1000>; interrupts = <0 171 4>; clocks = <&osc1>; + resets = <&rst L4WD0_RESET>; status = "disabled"; };
@@ -938,6 +952,7 @@ reg = <0xffd03000 0x1000>; interrupts = <0 172 4>; clocks = <&osc1>; + resets = <&rst L4WD1_RESET>; status = "disabled"; }; }; diff --git a/arch/arm/dts/socfpga_cyclone5_socrates.dts b/arch/arm/dts/socfpga_cyclone5_socrates.dts index 93c3fa4a48..8d5d3996f6 100644 --- a/arch/arm/dts/socfpga_cyclone5_socrates.dts +++ b/arch/arm/dts/socfpga_cyclone5_socrates.dts @@ -76,7 +76,6 @@
&qspi { status = "okay"; - u-boot,dm-pre-reloc;
flash: flash@0 { #address-cells = <1>; @@ -91,6 +90,5 @@ cdns,tchsh-ns = <4>; cdns,tslch-ns = <4>; status = "okay"; - u-boot,dm-pre-reloc; }; };

On 2/21/19 10:43 PM, Simon Goldschmidt wrote:
This is again a sync to linux-next + pending patches in Dinh's tree at commit 1c909b2dfe6a ("ARM: dts: socfpga: update more missing reset properties")'
It adds missing peripheral reset properties to socfpga.dtsi and removes U-Boot specific leftovers from socfpga_cyclone5_socrates.dts.
Signed-off-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com
Changes in v2:
- cleanly merged Linux dts (moved change of SDR controller base address to a separate patch)
arch/arm/dts/socfpga.dtsi | 19 +++++++++++++++++-- arch/arm/dts/socfpga_cyclone5_socrates.dts | 2 -- 2 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/arch/arm/dts/socfpga.dtsi b/arch/arm/dts/socfpga.dtsi index 2458d6707d..ec1966480f 100644 --- a/arch/arm/dts/socfpga.dtsi +++ b/arch/arm/dts/socfpga.dtsi @@ -84,6 +84,7 @@ #dma-requests = <32>; clocks = <&l4_main_clk>; clock-names = "apb_pclk";
};resets = <&rst DMA_RESET>; };
@@ -100,6 +101,7 @@ reg = <0xffc00000 0x1000>; interrupts = <0 131 4>, <0 132 4>, <0 133 4>, <0 134 4>; clocks = <&can0_clk>;
};resets = <&rst CAN0_RESET>; status = "disabled";
@@ -108,6 +110,7 @@ reg = <0xffc01000 0x1000>; interrupts = <0 135 4>, <0 136 4>, <0 137 4>, <0 138 4>; clocks = <&can1_clk>;
};resets = <&rst CAN1_RESET>; status = "disabled";
@@ -585,6 +588,7 @@ compatible = "snps,dw-apb-gpio"; reg = <0xff708000 0x1000>; clocks = <&l4_mp_clk>;
resets = <&rst GPIO0_RESET>; status = "disabled"; porta: gpio-controller@0 {
@@ -605,6 +609,7 @@ compatible = "snps,dw-apb-gpio"; reg = <0xff709000 0x1000>; clocks = <&l4_mp_clk>;
resets = <&rst GPIO1_RESET>; status = "disabled"; portb: gpio-controller@0 {
@@ -625,6 +630,7 @@ compatible = "snps,dw-apb-gpio"; reg = <0xff70a000 0x1000>; clocks = <&l4_mp_clk>;
resets = <&rst GPIO2_RESET>; status = "disabled"; portc: gpio-controller@0 {
@@ -735,6 +741,7 @@ #size-cells = <0>; clocks = <&l4_mp_clk>, <&sdmmc_clk_divided>; clock-names = "biu", "ciu";
};resets = <&rst SDMMC_RESET>; status = "disabled";
@@ -746,9 +753,9 @@ <0xffb80000 0x10000>; reg-names = "nand_data", "denali_reg"; interrupts = <0x0 0x90 0x4>;
dma-mask = <0xffffffff>; clocks = <&nand_clk>, <&nand_x_clk>, <&nand_ecc_clk>; clock-names = "nand", "nand_x", "ecc";
};resets = <&rst NAND_RESET>; status = "disabled";
@@ -759,7 +766,7 @@
qspi: spi@ff705000 { compatible = "cdns,qspi-nor";
#address-cells = <1>;
#address-cells = <1>; #size-cells = <0>; reg = <0xff705000 0x1000>, <0xffa00000 0x1000>;
@@ -768,6 +775,7 @@ cdns,fifo-width = <4>; cdns,trigger-address = <0x00000000>; clocks = <&qspi_clk>;
};resets = <&rst QSPI_RESET>; status = "disabled";
@@ -786,6 +794,7 @@ sdr: sdr@ffc25000 { compatible = "altr,sdr-ctl", "syscon"; reg = <0xffc25000 0x1000>;
resets = <&rst SDR_RESET>;
};
sdramedac {
@@ -802,6 +811,7 @@ interrupts = <0 154 4>; num-cs = <4>; clocks = <&spi_m_clk>;
};resets = <&rst SPIM0_RESET>; status = "disabled";
@@ -813,6 +823,7 @@ interrupts = <0 155 4>; num-cs = <4>; clocks = <&spi_m_clk>;
};resets = <&rst SPIM1_RESET>; status = "disabled";
@@ -879,6 +890,7 @@ dmas = <&pdma 28>, <&pdma 29>; dma-names = "tx", "rx";
resets = <&rst UART0_RESET>;
};
uart1: serial1@ffc03000 {
@@ -891,6 +903,7 @@ dmas = <&pdma 30>, <&pdma 31>; dma-names = "tx", "rx";
resets = <&rst UART1_RESET>;
};
usbphy0: usbphy {
@@ -930,6 +943,7 @@ reg = <0xffd02000 0x1000>; interrupts = <0 171 4>; clocks = <&osc1>;
};resets = <&rst L4WD0_RESET>; status = "disabled";
@@ -938,6 +952,7 @@ reg = <0xffd03000 0x1000>; interrupts = <0 172 4>; clocks = <&osc1>;
}; };resets = <&rst L4WD1_RESET>; status = "disabled";
diff --git a/arch/arm/dts/socfpga_cyclone5_socrates.dts b/arch/arm/dts/socfpga_cyclone5_socrates.dts index 93c3fa4a48..8d5d3996f6 100644 --- a/arch/arm/dts/socfpga_cyclone5_socrates.dts +++ b/arch/arm/dts/socfpga_cyclone5_socrates.dts @@ -76,7 +76,6 @@
&qspi { status = "okay";
u-boot,dm-pre-reloc;
flash: flash@0 { #address-cells = <1>;
@@ -91,6 +90,5 @@ cdns,tchsh-ns = <4>; cdns,tslch-ns = <4>; status = "okay";
u-boot,dm-pre-reloc;
Is this intended ?
}; };

Am Do., 21. Feb. 2019, 22:56 hat Marek Vasut marex@denx.de geschrieben:
On 2/21/19 10:43 PM, Simon Goldschmidt wrote:
This is again a sync to linux-next + pending patches in Dinh's tree at commit 1c909b2dfe6a ("ARM: dts: socfpga: update more missing reset properties")'
It adds missing peripheral reset properties to socfpga.dtsi and removes U-Boot specific leftovers from socfpga_cyclone5_socrates.dts.
Signed-off-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com
Changes in v2:
- cleanly merged Linux dts (moved change of SDR controller base address to a separate patch)
arch/arm/dts/socfpga.dtsi | 19 +++++++++++++++++-- arch/arm/dts/socfpga_cyclone5_socrates.dts | 2 -- 2 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/arch/arm/dts/socfpga.dtsi b/arch/arm/dts/socfpga.dtsi index 2458d6707d..ec1966480f 100644 --- a/arch/arm/dts/socfpga.dtsi +++ b/arch/arm/dts/socfpga.dtsi @@ -84,6 +84,7 @@ #dma-requests = <32>; clocks = <&l4_main_clk>; clock-names = "apb_pclk";
resets = <&rst DMA_RESET>; }; };
@@ -100,6 +101,7 @@ reg = <0xffc00000 0x1000>; interrupts = <0 131 4>, <0 132 4>, <0 133 4>, <0
134 4>;
clocks = <&can0_clk>;
resets = <&rst CAN0_RESET>; status = "disabled"; };
@@ -108,6 +110,7 @@ reg = <0xffc01000 0x1000>; interrupts = <0 135 4>, <0 136 4>, <0 137 4>, <0
138 4>;
clocks = <&can1_clk>;
resets = <&rst CAN1_RESET>; status = "disabled"; };
@@ -585,6 +588,7 @@ compatible = "snps,dw-apb-gpio"; reg = <0xff708000 0x1000>; clocks = <&l4_mp_clk>;
resets = <&rst GPIO0_RESET>; status = "disabled"; porta: gpio-controller@0 {
@@ -605,6 +609,7 @@ compatible = "snps,dw-apb-gpio"; reg = <0xff709000 0x1000>; clocks = <&l4_mp_clk>;
resets = <&rst GPIO1_RESET>; status = "disabled"; portb: gpio-controller@0 {
@@ -625,6 +630,7 @@ compatible = "snps,dw-apb-gpio"; reg = <0xff70a000 0x1000>; clocks = <&l4_mp_clk>;
resets = <&rst GPIO2_RESET>; status = "disabled"; portc: gpio-controller@0 {
@@ -735,6 +741,7 @@ #size-cells = <0>; clocks = <&l4_mp_clk>, <&sdmmc_clk_divided>; clock-names = "biu", "ciu";
resets = <&rst SDMMC_RESET>; status = "disabled"; };
@@ -746,9 +753,9 @@ <0xffb80000 0x10000>; reg-names = "nand_data", "denali_reg"; interrupts = <0x0 0x90 0x4>;
dma-mask = <0xffffffff>; clocks = <&nand_clk>, <&nand_x_clk>,
<&nand_ecc_clk>;
clock-names = "nand", "nand_x", "ecc";
resets = <&rst NAND_RESET>; status = "disabled"; };
@@ -759,7 +766,7 @@
qspi: spi@ff705000 { compatible = "cdns,qspi-nor";
#address-cells = <1>;
#address-cells = <1>; #size-cells = <0>; reg = <0xff705000 0x1000>, <0xffa00000 0x1000>;
@@ -768,6 +775,7 @@ cdns,fifo-width = <4>; cdns,trigger-address = <0x00000000>; clocks = <&qspi_clk>;
resets = <&rst QSPI_RESET>; status = "disabled"; };
@@ -786,6 +794,7 @@ sdr: sdr@ffc25000 { compatible = "altr,sdr-ctl", "syscon"; reg = <0xffc25000 0x1000>;
resets = <&rst SDR_RESET>; }; sdramedac {
@@ -802,6 +811,7 @@ interrupts = <0 154 4>; num-cs = <4>; clocks = <&spi_m_clk>;
resets = <&rst SPIM0_RESET>; status = "disabled"; };
@@ -813,6 +823,7 @@ interrupts = <0 155 4>; num-cs = <4>; clocks = <&spi_m_clk>;
resets = <&rst SPIM1_RESET>; status = "disabled"; };
@@ -879,6 +890,7 @@ dmas = <&pdma 28>, <&pdma 29>; dma-names = "tx", "rx";
resets = <&rst UART0_RESET>; }; uart1: serial1@ffc03000 {
@@ -891,6 +903,7 @@ dmas = <&pdma 30>, <&pdma 31>; dma-names = "tx", "rx";
resets = <&rst UART1_RESET>; }; usbphy0: usbphy {
@@ -930,6 +943,7 @@ reg = <0xffd02000 0x1000>; interrupts = <0 171 4>; clocks = <&osc1>;
resets = <&rst L4WD0_RESET>; status = "disabled"; };
@@ -938,6 +952,7 @@ reg = <0xffd03000 0x1000>; interrupts = <0 172 4>; clocks = <&osc1>;
resets = <&rst L4WD1_RESET>; status = "disabled"; }; };
diff --git a/arch/arm/dts/socfpga_cyclone5_socrates.dts
b/arch/arm/dts/socfpga_cyclone5_socrates.dts
index 93c3fa4a48..8d5d3996f6 100644 --- a/arch/arm/dts/socfpga_cyclone5_socrates.dts +++ b/arch/arm/dts/socfpga_cyclone5_socrates.dts @@ -76,7 +76,6 @@
&qspi { status = "okay";
u-boot,dm-pre-reloc; flash: flash@0 { #address-cells = <1>;
@@ -91,6 +90,5 @@ cdns,tchsh-ns = <4>; cdns,tslch-ns = <4>; status = "okay";
u-boot,dm-pre-reloc;
Is this intended ?
Yes, see the commit message. We have it in a U-Boot specific overlay and this is a sync to Linux patch.
Regards, Simon
};
};

On 2/21/19 11:02 PM, Simon Goldschmidt wrote:
Am Do., 21. Feb. 2019, 22:56 hat Marek Vasut <marex@denx.de mailto:marex@denx.de> geschrieben:
On 2/21/19 10:43 PM, Simon Goldschmidt wrote: > This is again a sync to linux-next + pending patches in Dinh's tree at > commit 1c909b2dfe6a ("ARM: dts: socfpga: update more missing reset > properties")' > > It adds missing peripheral reset properties to socfpga.dtsi and removes > U-Boot specific leftovers from socfpga_cyclone5_socrates.dts. > > Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com <mailto:simon.k.r.goldschmidt@gmail.com>> > --- > > Changes in v2: > - cleanly merged Linux dts (moved change of SDR controller base address > to a separate patch) > > arch/arm/dts/socfpga.dtsi | 19 +++++++++++++++++-- > arch/arm/dts/socfpga_cyclone5_socrates.dts | 2 -- > 2 files changed, 17 insertions(+), 4 deletions(-) > > diff --git a/arch/arm/dts/socfpga.dtsi b/arch/arm/dts/socfpga.dtsi > index 2458d6707d..ec1966480f 100644 > --- a/arch/arm/dts/socfpga.dtsi > +++ b/arch/arm/dts/socfpga.dtsi > @@ -84,6 +84,7 @@ > #dma-requests = <32>; > clocks = <&l4_main_clk>; > clock-names = "apb_pclk"; > + resets = <&rst DMA_RESET>; > }; > }; > > @@ -100,6 +101,7 @@ > reg = <0xffc00000 0x1000>; > interrupts = <0 131 4>, <0 132 4>, <0 133 4>, <0 134 4>; > clocks = <&can0_clk>; > + resets = <&rst CAN0_RESET>; > status = "disabled"; > }; > > @@ -108,6 +110,7 @@ > reg = <0xffc01000 0x1000>; > interrupts = <0 135 4>, <0 136 4>, <0 137 4>, <0 138 4>; > clocks = <&can1_clk>; > + resets = <&rst CAN1_RESET>; > status = "disabled"; > }; > > @@ -585,6 +588,7 @@ > compatible = "snps,dw-apb-gpio"; > reg = <0xff708000 0x1000>; > clocks = <&l4_mp_clk>; > + resets = <&rst GPIO0_RESET>; > status = "disabled"; > > porta: gpio-controller@0 { > @@ -605,6 +609,7 @@ > compatible = "snps,dw-apb-gpio"; > reg = <0xff709000 0x1000>; > clocks = <&l4_mp_clk>; > + resets = <&rst GPIO1_RESET>; > status = "disabled"; > > portb: gpio-controller@0 { > @@ -625,6 +630,7 @@ > compatible = "snps,dw-apb-gpio"; > reg = <0xff70a000 0x1000>; > clocks = <&l4_mp_clk>; > + resets = <&rst GPIO2_RESET>; > status = "disabled"; > > portc: gpio-controller@0 { > @@ -735,6 +741,7 @@ > #size-cells = <0>; > clocks = <&l4_mp_clk>, <&sdmmc_clk_divided>; > clock-names = "biu", "ciu"; > + resets = <&rst SDMMC_RESET>; > status = "disabled"; > }; > > @@ -746,9 +753,9 @@ > <0xffb80000 0x10000>; > reg-names = "nand_data", "denali_reg"; > interrupts = <0x0 0x90 0x4>; > - dma-mask = <0xffffffff>; > clocks = <&nand_clk>, <&nand_x_clk>, <&nand_ecc_clk>; > clock-names = "nand", "nand_x", "ecc"; > + resets = <&rst NAND_RESET>; > status = "disabled"; > }; > > @@ -759,7 +766,7 @@ > > qspi: spi@ff705000 { > compatible = "cdns,qspi-nor"; > - #address-cells = <1>; > + #address-cells = <1>; > #size-cells = <0>; > reg = <0xff705000 0x1000>, > <0xffa00000 0x1000>; > @@ -768,6 +775,7 @@ > cdns,fifo-width = <4>; > cdns,trigger-address = <0x00000000>; > clocks = <&qspi_clk>; > + resets = <&rst QSPI_RESET>; > status = "disabled"; > }; > > @@ -786,6 +794,7 @@ > sdr: sdr@ffc25000 { > compatible = "altr,sdr-ctl", "syscon"; > reg = <0xffc25000 0x1000>; > + resets = <&rst SDR_RESET>; > }; > > sdramedac { > @@ -802,6 +811,7 @@ > interrupts = <0 154 4>; > num-cs = <4>; > clocks = <&spi_m_clk>; > + resets = <&rst SPIM0_RESET>; > status = "disabled"; > }; > > @@ -813,6 +823,7 @@ > interrupts = <0 155 4>; > num-cs = <4>; > clocks = <&spi_m_clk>; > + resets = <&rst SPIM1_RESET>; > status = "disabled"; > }; > > @@ -879,6 +890,7 @@ > dmas = <&pdma 28>, > <&pdma 29>; > dma-names = "tx", "rx"; > + resets = <&rst UART0_RESET>; > }; > > uart1: serial1@ffc03000 { > @@ -891,6 +903,7 @@ > dmas = <&pdma 30>, > <&pdma 31>; > dma-names = "tx", "rx"; > + resets = <&rst UART1_RESET>; > }; > > usbphy0: usbphy { > @@ -930,6 +943,7 @@ > reg = <0xffd02000 0x1000>; > interrupts = <0 171 4>; > clocks = <&osc1>; > + resets = <&rst L4WD0_RESET>; > status = "disabled"; > }; > > @@ -938,6 +952,7 @@ > reg = <0xffd03000 0x1000>; > interrupts = <0 172 4>; > clocks = <&osc1>; > + resets = <&rst L4WD1_RESET>; > status = "disabled"; > }; > }; > diff --git a/arch/arm/dts/socfpga_cyclone5_socrates.dts b/arch/arm/dts/socfpga_cyclone5_socrates.dts > index 93c3fa4a48..8d5d3996f6 100644 > --- a/arch/arm/dts/socfpga_cyclone5_socrates.dts > +++ b/arch/arm/dts/socfpga_cyclone5_socrates.dts > @@ -76,7 +76,6 @@ > > &qspi { > status = "okay"; > - u-boot,dm-pre-reloc; > > flash: flash@0 { > #address-cells = <1>; > @@ -91,6 +90,5 @@ > cdns,tchsh-ns = <4>; > cdns,tslch-ns = <4>; > status = "okay"; > - u-boot,dm-pre-reloc; Is this intended ?
Yes, see the commit message. We have it in a U-Boot specific overlay and this is a sync to Linux patch.
OK

The SPL for socfpga gen5 currently takes all peripherals out of reset unconditionally. To implement proper reset handling for peripherals, the reset node has to be provided with the SPL dts.
In preparation to move the DDR driver to DM, the sdr node is required in SPL, too.
This patch adds "u-boot,dm-pre-reloc" to U-Boot specific dtsi addon files so that the reset manager and SDR driver correctly probe in SPL.
Signed-off-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com ---
Changes in v2: None
arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi | 8 ++++++++ arch/arm/dts/socfpga_cyclone5_dbm_soc1.dts | 8 ++++++++ arch/arm/dts/socfpga_cyclone5_de0_nano_soc-u-boot.dtsi | 8 ++++++++ arch/arm/dts/socfpga_cyclone5_de10_nano.dts | 8 ++++++++ arch/arm/dts/socfpga_cyclone5_de1_soc.dts | 8 ++++++++ arch/arm/dts/socfpga_cyclone5_is1.dts | 8 ++++++++ arch/arm/dts/socfpga_cyclone5_socdk-u-boot.dtsi | 8 ++++++++ arch/arm/dts/socfpga_cyclone5_sockit-u-boot.dtsi | 8 ++++++++ arch/arm/dts/socfpga_cyclone5_socrates-u-boot.dtsi | 8 ++++++++ arch/arm/dts/socfpga_cyclone5_sr1500.dts | 8 ++++++++ arch/arm/dts/socfpga_cyclone5_vining_fpga-u-boot.dtsi | 8 ++++++++ 11 files changed, 88 insertions(+)
diff --git a/arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi b/arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi index c44d1ee2fa..8aaec56285 100644 --- a/arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi +++ b/arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi @@ -17,6 +17,14 @@ }; };
+&rst { + u-boot,dm-pre-reloc; +}; + +&sdr { + 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..61907771e0 100644 --- a/arch/arm/dts/socfpga_cyclone5_dbm_soc1.dts +++ b/arch/arm/dts/socfpga_cyclone5_dbm_soc1.dts @@ -30,6 +30,14 @@ }; };
+&rst { + u-boot,dm-pre-reloc; +}; + +&sdr { + 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..00434185f6 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 @@ -16,6 +16,14 @@ }; };
+&rst { + u-boot,dm-pre-reloc; +}; + +&sdr { + 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..dc84f3de26 100644 --- a/arch/arm/dts/socfpga_cyclone5_de10_nano.dts +++ b/arch/arm/dts/socfpga_cyclone5_de10_nano.dts @@ -32,6 +32,14 @@ }; };
+&rst { + u-boot,dm-pre-reloc; +}; + +&sdr { + 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..585d914e30 100644 --- a/arch/arm/dts/socfpga_cyclone5_de1_soc.dts +++ b/arch/arm/dts/socfpga_cyclone5_de1_soc.dts @@ -30,6 +30,14 @@ }; };
+&rst { + u-boot,dm-pre-reloc; +}; + +&sdr { + 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..8947128be9 100644 --- a/arch/arm/dts/socfpga_cyclone5_is1.dts +++ b/arch/arm/dts/socfpga_cyclone5_is1.dts @@ -37,6 +37,14 @@ }; };
+&rst { + u-boot,dm-pre-reloc; +}; + +&sdr { + 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..13d44072f4 100644 --- a/arch/arm/dts/socfpga_cyclone5_socdk-u-boot.dtsi +++ b/arch/arm/dts/socfpga_cyclone5_socdk-u-boot.dtsi @@ -17,6 +17,14 @@ }; };
+&rst { + u-boot,dm-pre-reloc; +}; + +&sdr { + 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..07564a9f13 100644 --- a/arch/arm/dts/socfpga_cyclone5_sockit-u-boot.dtsi +++ b/arch/arm/dts/socfpga_cyclone5_sockit-u-boot.dtsi @@ -17,6 +17,14 @@ }; };
+&rst { + u-boot,dm-pre-reloc; +}; + +&sdr { + 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..cf0eb8bb8c 100644 --- a/arch/arm/dts/socfpga_cyclone5_socrates-u-boot.dtsi +++ b/arch/arm/dts/socfpga_cyclone5_socrates-u-boot.dtsi @@ -17,6 +17,14 @@ }; };
+&rst { + u-boot,dm-pre-reloc; +}; + +&sdr { + u-boot,dm-pre-reloc; +}; + &watchdog0 { status = "disabled"; }; diff --git a/arch/arm/dts/socfpga_cyclone5_sr1500.dts b/arch/arm/dts/socfpga_cyclone5_sr1500.dts index 6a6c29be79..8a1678ed72 100644 --- a/arch/arm/dts/socfpga_cyclone5_sr1500.dts +++ b/arch/arm/dts/socfpga_cyclone5_sr1500.dts @@ -33,6 +33,14 @@ }; };
+&rst { + u-boot,dm-pre-reloc; +}; + +&sdr { + 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..90d1fd8858 100644 --- a/arch/arm/dts/socfpga_cyclone5_vining_fpga-u-boot.dtsi +++ b/arch/arm/dts/socfpga_cyclone5_vining_fpga-u-boot.dtsi @@ -17,6 +17,14 @@ }; };
+&rst { + u-boot,dm-pre-reloc; +}; + +&sdr { + u-boot,dm-pre-reloc; +}; + &watchdog0 { status = "disabled"; };

On 2/21/19 10:43 PM, Simon Goldschmidt wrote:
The SPL for socfpga gen5 currently takes all peripherals out of reset unconditionally. To implement proper reset handling for peripherals, the reset node has to be provided with the SPL dts.
In preparation to move the DDR driver to DM, the sdr node is required in SPL, too.
This patch adds "u-boot,dm-pre-reloc" to U-Boot specific dtsi addon files so that the reset manager and SDR driver correctly probe in SPL.
Signed-off-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com
Changes in v2: None
arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi | 8 ++++++++ arch/arm/dts/socfpga_cyclone5_dbm_soc1.dts | 8 ++++++++ arch/arm/dts/socfpga_cyclone5_de0_nano_soc-u-boot.dtsi | 8 ++++++++ arch/arm/dts/socfpga_cyclone5_de10_nano.dts | 8 ++++++++ arch/arm/dts/socfpga_cyclone5_de1_soc.dts | 8 ++++++++ arch/arm/dts/socfpga_cyclone5_is1.dts | 8 ++++++++ arch/arm/dts/socfpga_cyclone5_socdk-u-boot.dtsi | 8 ++++++++ arch/arm/dts/socfpga_cyclone5_sockit-u-boot.dtsi | 8 ++++++++ arch/arm/dts/socfpga_cyclone5_socrates-u-boot.dtsi | 8 ++++++++ arch/arm/dts/socfpga_cyclone5_sr1500.dts | 8 ++++++++ arch/arm/dts/socfpga_cyclone5_vining_fpga-u-boot.dtsi | 8 ++++++++ 11 files changed, 88 insertions(+)
diff --git a/arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi b/arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi index c44d1ee2fa..8aaec56285 100644 --- a/arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi +++ b/arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi @@ -17,6 +17,14 @@ }; };
+&rst {
- u-boot,dm-pre-reloc;
+};
+&sdr {
- u-boot,dm-pre-reloc;
+};
What about some socfpga-common-u-boot.dtsi to avoid duplication ?

Am Do., 21. Feb. 2019, 22:56 hat Marek Vasut marex@denx.de geschrieben:
On 2/21/19 10:43 PM, Simon Goldschmidt wrote:
The SPL for socfpga gen5 currently takes all peripherals out of reset unconditionally. To implement proper reset handling for peripherals, the reset node has to be provided with the SPL dts.
In preparation to move the DDR driver to DM, the sdr node is required in SPL, too.
This patch adds "u-boot,dm-pre-reloc" to U-Boot specific dtsi addon files so that the reset manager and SDR driver correctly probe in SPL.
Signed-off-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com
Changes in v2: None
arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi | 8 ++++++++ arch/arm/dts/socfpga_cyclone5_dbm_soc1.dts | 8 ++++++++ arch/arm/dts/socfpga_cyclone5_de0_nano_soc-u-boot.dtsi | 8 ++++++++ arch/arm/dts/socfpga_cyclone5_de10_nano.dts | 8 ++++++++ arch/arm/dts/socfpga_cyclone5_de1_soc.dts | 8 ++++++++ arch/arm/dts/socfpga_cyclone5_is1.dts | 8 ++++++++ arch/arm/dts/socfpga_cyclone5_socdk-u-boot.dtsi | 8 ++++++++ arch/arm/dts/socfpga_cyclone5_sockit-u-boot.dtsi | 8 ++++++++ arch/arm/dts/socfpga_cyclone5_socrates-u-boot.dtsi | 8 ++++++++ arch/arm/dts/socfpga_cyclone5_sr1500.dts | 8 ++++++++ arch/arm/dts/socfpga_cyclone5_vining_fpga-u-boot.dtsi | 8 ++++++++ 11 files changed, 88 insertions(+)
diff --git a/arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi
b/arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi
index c44d1ee2fa..8aaec56285 100644 --- a/arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi +++ b/arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi @@ -17,6 +17,14 @@ }; };
+&rst {
u-boot,dm-pre-reloc;
+};
+&sdr {
u-boot,dm-pre-reloc;
+};
What about some socfpga-common-u-boot.dtsi to avoid duplication ?
Right. I tested with 'socfpga-u-boot.dtsi' but the hot included for gen10 as well. But your suggested name should work fine I guess.
Regards, Simon

On 2/21/19 11:03 PM, Simon Goldschmidt wrote:
Am Do., 21. Feb. 2019, 22:56 hat Marek Vasut <marex@denx.de mailto:marex@denx.de> geschrieben:
On 2/21/19 10:43 PM, Simon Goldschmidt wrote: > The SPL for socfpga gen5 currently takes all peripherals out of reset > unconditionally. To implement proper reset handling for peripherals, > the reset node has to be provided with the SPL dts. > > In preparation to move the DDR driver to DM, the sdr node is required > in SPL, too. > > This patch adds "u-boot,dm-pre-reloc" to U-Boot specific dtsi addon > files so that the reset manager and SDR driver correctly probe in SPL. > > Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com <mailto:simon.k.r.goldschmidt@gmail.com>> > --- > > Changes in v2: None > > arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi | 8 ++++++++ > arch/arm/dts/socfpga_cyclone5_dbm_soc1.dts | 8 ++++++++ > arch/arm/dts/socfpga_cyclone5_de0_nano_soc-u-boot.dtsi | 8 ++++++++ > arch/arm/dts/socfpga_cyclone5_de10_nano.dts | 8 ++++++++ > arch/arm/dts/socfpga_cyclone5_de1_soc.dts | 8 ++++++++ > arch/arm/dts/socfpga_cyclone5_is1.dts | 8 ++++++++ > arch/arm/dts/socfpga_cyclone5_socdk-u-boot.dtsi | 8 ++++++++ > arch/arm/dts/socfpga_cyclone5_sockit-u-boot.dtsi | 8 ++++++++ > arch/arm/dts/socfpga_cyclone5_socrates-u-boot.dtsi | 8 ++++++++ > arch/arm/dts/socfpga_cyclone5_sr1500.dts | 8 ++++++++ > arch/arm/dts/socfpga_cyclone5_vining_fpga-u-boot.dtsi | 8 ++++++++ > 11 files changed, 88 insertions(+) > > diff --git a/arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi b/arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi > index c44d1ee2fa..8aaec56285 100644 > --- a/arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi > +++ b/arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi > @@ -17,6 +17,14 @@ > }; > }; > > +&rst { > + u-boot,dm-pre-reloc; > +}; > + > +&sdr { > + u-boot,dm-pre-reloc; > +}; What about some socfpga-common-u-boot.dtsi to avoid duplication ?
Right. I tested with 'socfpga-u-boot.dtsi' but the hot included for gen10 as well.
hot included ? I don't think I understand.
But your suggested name should work fine I guess.
Regards, Simon

Am Do., 21. Feb. 2019, 23:05 hat Marek Vasut marex@denx.de geschrieben:
On 2/21/19 11:03 PM, Simon Goldschmidt wrote:
Am Do., 21. Feb. 2019, 22:56 hat Marek Vasut <marex@denx.de mailto:marex@denx.de> geschrieben:
On 2/21/19 10:43 PM, Simon Goldschmidt wrote: > The SPL for socfpga gen5 currently takes all peripherals out of
reset
> unconditionally. To implement proper reset handling for
peripherals,
> the reset node has to be provided with the SPL dts. > > In preparation to move the DDR driver to DM, the sdr node is
required
> in SPL, too. > > This patch adds "u-boot,dm-pre-reloc" to U-Boot specific dtsi addon > files so that the reset manager and SDR driver correctly probe in
SPL.
> > Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com <mailto:simon.k.r.goldschmidt@gmail.com>> > --- > > Changes in v2: None > > arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi | 8
++++++++
> arch/arm/dts/socfpga_cyclone5_dbm_soc1.dts | 8
++++++++
> arch/arm/dts/socfpga_cyclone5_de0_nano_soc-u-boot.dtsi | 8
++++++++
> arch/arm/dts/socfpga_cyclone5_de10_nano.dts | 8
++++++++
> arch/arm/dts/socfpga_cyclone5_de1_soc.dts | 8
++++++++
> arch/arm/dts/socfpga_cyclone5_is1.dts | 8
++++++++
> arch/arm/dts/socfpga_cyclone5_socdk-u-boot.dtsi | 8
++++++++
> arch/arm/dts/socfpga_cyclone5_sockit-u-boot.dtsi | 8
++++++++
> arch/arm/dts/socfpga_cyclone5_socrates-u-boot.dtsi | 8
++++++++
> arch/arm/dts/socfpga_cyclone5_sr1500.dts | 8
++++++++
> arch/arm/dts/socfpga_cyclone5_vining_fpga-u-boot.dtsi | 8
++++++++
> 11 files changed, 88 insertions(+) > > diff --git a/arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi b/arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi > index c44d1ee2fa..8aaec56285 100644 > --- a/arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi > +++ b/arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi > @@ -17,6 +17,14 @@ > }; > }; > > +&rst { > + u-boot,dm-pre-reloc; > +}; > + > +&sdr { > + u-boot,dm-pre-reloc; > +}; What about some socfpga-common-u-boot.dtsi to avoid duplication ?
Right. I tested with 'socfpga-u-boot.dtsi' but the hot included for gen10 as well.
hot included ? I don't think I understand.
Argh. I'm writing from my mobile...
What I meant was that file got included by the automatism so I couldn't use it.
Regards, Simon
But your suggested name should work fine I guess.
Regards, Simon

On 2/21/19 11:11 PM, Simon Goldschmidt wrote:
Am Do., 21. Feb. 2019, 23:05 hat Marek Vasut <marex@denx.de mailto:marex@denx.de> geschrieben:
On 2/21/19 11:03 PM, Simon Goldschmidt wrote: > > > Am Do., 21. Feb. 2019, 22:56 hat Marek Vasut <marex@denx.de <mailto:marex@denx.de> > <mailto:marex@denx.de <mailto:marex@denx.de>>> geschrieben: > > On 2/21/19 10:43 PM, Simon Goldschmidt wrote: > > The SPL for socfpga gen5 currently takes all peripherals out of reset > > unconditionally. To implement proper reset handling for peripherals, > > the reset node has to be provided with the SPL dts. > > > > In preparation to move the DDR driver to DM, the sdr node is required > > in SPL, too. > > > > This patch adds "u-boot,dm-pre-reloc" to U-Boot specific dtsi addon > > files so that the reset manager and SDR driver correctly probe in SPL. > > > > Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com <mailto:simon.k.r.goldschmidt@gmail.com> > <mailto:simon.k.r.goldschmidt@gmail.com <mailto:simon.k.r.goldschmidt@gmail.com>>> > > --- > > > > Changes in v2: None > > > > arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi | 8 ++++++++ > > arch/arm/dts/socfpga_cyclone5_dbm_soc1.dts | 8 ++++++++ > > arch/arm/dts/socfpga_cyclone5_de0_nano_soc-u-boot.dtsi | 8 ++++++++ > > arch/arm/dts/socfpga_cyclone5_de10_nano.dts | 8 ++++++++ > > arch/arm/dts/socfpga_cyclone5_de1_soc.dts | 8 ++++++++ > > arch/arm/dts/socfpga_cyclone5_is1.dts | 8 ++++++++ > > arch/arm/dts/socfpga_cyclone5_socdk-u-boot.dtsi | 8 ++++++++ > > arch/arm/dts/socfpga_cyclone5_sockit-u-boot.dtsi | 8 ++++++++ > > arch/arm/dts/socfpga_cyclone5_socrates-u-boot.dtsi | 8 ++++++++ > > arch/arm/dts/socfpga_cyclone5_sr1500.dts | 8 ++++++++ > > arch/arm/dts/socfpga_cyclone5_vining_fpga-u-boot.dtsi | 8 ++++++++ > > 11 files changed, 88 insertions(+) > > > > diff --git a/arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi > b/arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi > > index c44d1ee2fa..8aaec56285 100644 > > --- a/arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi > > +++ b/arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi > > @@ -17,6 +17,14 @@ > > }; > > }; > > > > +&rst { > > + u-boot,dm-pre-reloc; > > +}; > > + > > +&sdr { > > + u-boot,dm-pre-reloc; > > +}; > > What about some socfpga-common-u-boot.dtsi to avoid duplication ? > > > Right. I tested with 'socfpga-u-boot.dtsi' but the hot included for > gen10 as well. hot included ? I don't think I understand.
Argh. I'm writing from my mobile...
It can wait till tomorrow ...
What I meant was that file got included by the automatism so I couldn't use it.
Errr, which file ? What automatism ? Really, wait till tomorrow with a reply, please.
[...]

On Thu, Feb 21, 2019 at 11:13 PM Marek Vasut marex@denx.de wrote:
On 2/21/19 11:11 PM, Simon Goldschmidt wrote:
Am Do., 21. Feb. 2019, 23:05 hat Marek Vasut <marex@denx.de mailto:marex@denx.de> geschrieben:
On 2/21/19 11:03 PM, Simon Goldschmidt wrote: > > > Am Do., 21. Feb. 2019, 22:56 hat Marek Vasut <marex@denx.de <mailto:marex@denx.de> > <mailto:marex@denx.de <mailto:marex@denx.de>>> geschrieben: > > On 2/21/19 10:43 PM, Simon Goldschmidt wrote: > > The SPL for socfpga gen5 currently takes all peripherals out of reset > > unconditionally. To implement proper reset handling for peripherals, > > the reset node has to be provided with the SPL dts. > > > > In preparation to move the DDR driver to DM, the sdr node is required > > in SPL, too. > > > > This patch adds "u-boot,dm-pre-reloc" to U-Boot specific dtsi addon > > files so that the reset manager and SDR driver correctly probe in SPL. > > > > Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com <mailto:simon.k.r.goldschmidt@gmail.com> > <mailto:simon.k.r.goldschmidt@gmail.com <mailto:simon.k.r.goldschmidt@gmail.com>>> > > --- > > > > Changes in v2: None > > > > arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi | 8 ++++++++ > > arch/arm/dts/socfpga_cyclone5_dbm_soc1.dts | 8 ++++++++ > > arch/arm/dts/socfpga_cyclone5_de0_nano_soc-u-boot.dtsi | 8 ++++++++ > > arch/arm/dts/socfpga_cyclone5_de10_nano.dts | 8 ++++++++ > > arch/arm/dts/socfpga_cyclone5_de1_soc.dts | 8 ++++++++ > > arch/arm/dts/socfpga_cyclone5_is1.dts | 8 ++++++++ > > arch/arm/dts/socfpga_cyclone5_socdk-u-boot.dtsi | 8 ++++++++ > > arch/arm/dts/socfpga_cyclone5_sockit-u-boot.dtsi | 8 ++++++++ > > arch/arm/dts/socfpga_cyclone5_socrates-u-boot.dtsi | 8 ++++++++ > > arch/arm/dts/socfpga_cyclone5_sr1500.dts | 8 ++++++++ > > arch/arm/dts/socfpga_cyclone5_vining_fpga-u-boot.dtsi | 8 ++++++++ > > 11 files changed, 88 insertions(+) > > > > diff --git a/arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi > b/arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi > > index c44d1ee2fa..8aaec56285 100644 > > --- a/arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi > > +++ b/arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi > > @@ -17,6 +17,14 @@ > > }; > > }; > > > > +&rst { > > + u-boot,dm-pre-reloc; > > +}; > > + > > +&sdr { > > + u-boot,dm-pre-reloc; > > +}; > > What about some socfpga-common-u-boot.dtsi to avoid duplication ? > > > Right. I tested with 'socfpga-u-boot.dtsi' but the hot included for > gen10 as well. hot included ? I don't think I understand.
Argh. I'm writing from my mobile...
It can wait till tomorrow ...
What I meant was that file got included by the automatism so I couldn't use it.
Errr, which file ? What automatism ? Really, wait till tomorrow with a reply, please.
Let me try again: I started this patch with trying to centralize the "u-boot,dm-pre-reloc" lines in one file named "socfpga-u-boot.dtsi" since it is valid for all gen5 devices and including it automatically would be OK.
However, the automatism to search for a "-u-boot.dtsi" file caught that file also for a10 and even s10 due to the lack of a more specific match.
In the end I dropped that idea and added it to all board dtsi files.
However, by using a name that doesn't match the automatism rules, I can centralize these settings. I'll do that for V3.
Regards, Simon

On 2/22/19 7:20 AM, Simon Goldschmidt wrote:
On Thu, Feb 21, 2019 at 11:13 PM Marek Vasut marex@denx.de wrote:
On 2/21/19 11:11 PM, Simon Goldschmidt wrote:
Am Do., 21. Feb. 2019, 23:05 hat Marek Vasut <marex@denx.de mailto:marex@denx.de> geschrieben:
On 2/21/19 11:03 PM, Simon Goldschmidt wrote: > > > Am Do., 21. Feb. 2019, 22:56 hat Marek Vasut <marex@denx.de <mailto:marex@denx.de> > <mailto:marex@denx.de <mailto:marex@denx.de>>> geschrieben: > > On 2/21/19 10:43 PM, Simon Goldschmidt wrote: > > The SPL for socfpga gen5 currently takes all peripherals out of reset > > unconditionally. To implement proper reset handling for peripherals, > > the reset node has to be provided with the SPL dts. > > > > In preparation to move the DDR driver to DM, the sdr node is required > > in SPL, too. > > > > This patch adds "u-boot,dm-pre-reloc" to U-Boot specific dtsi addon > > files so that the reset manager and SDR driver correctly probe in SPL. > > > > Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com <mailto:simon.k.r.goldschmidt@gmail.com> > <mailto:simon.k.r.goldschmidt@gmail.com <mailto:simon.k.r.goldschmidt@gmail.com>>> > > --- > > > > Changes in v2: None > > > > arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi | 8 ++++++++ > > arch/arm/dts/socfpga_cyclone5_dbm_soc1.dts | 8 ++++++++ > > arch/arm/dts/socfpga_cyclone5_de0_nano_soc-u-boot.dtsi | 8 ++++++++ > > arch/arm/dts/socfpga_cyclone5_de10_nano.dts | 8 ++++++++ > > arch/arm/dts/socfpga_cyclone5_de1_soc.dts | 8 ++++++++ > > arch/arm/dts/socfpga_cyclone5_is1.dts | 8 ++++++++ > > arch/arm/dts/socfpga_cyclone5_socdk-u-boot.dtsi | 8 ++++++++ > > arch/arm/dts/socfpga_cyclone5_sockit-u-boot.dtsi | 8 ++++++++ > > arch/arm/dts/socfpga_cyclone5_socrates-u-boot.dtsi | 8 ++++++++ > > arch/arm/dts/socfpga_cyclone5_sr1500.dts | 8 ++++++++ > > arch/arm/dts/socfpga_cyclone5_vining_fpga-u-boot.dtsi | 8 ++++++++ > > 11 files changed, 88 insertions(+) > > > > diff --git a/arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi > b/arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi > > index c44d1ee2fa..8aaec56285 100644 > > --- a/arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi > > +++ b/arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi > > @@ -17,6 +17,14 @@ > > }; > > }; > > > > +&rst { > > + u-boot,dm-pre-reloc; > > +}; > > + > > +&sdr { > > + u-boot,dm-pre-reloc; > > +}; > > What about some socfpga-common-u-boot.dtsi to avoid duplication ? > > > Right. I tested with 'socfpga-u-boot.dtsi' but the hot included for > gen10 as well. hot included ? I don't think I understand.
Argh. I'm writing from my mobile...
It can wait till tomorrow ...
What I meant was that file got included by the automatism so I couldn't use it.
Errr, which file ? What automatism ? Really, wait till tomorrow with a reply, please.
Let me try again: I started this patch with trying to centralize the "u-boot,dm-pre-reloc" lines in one file named "socfpga-u-boot.dtsi" since it is valid for all gen5 devices and including it automatically would be OK.
However, the automatism to search for a "-u-boot.dtsi" file caught that file also for a10 and even s10 due to the lack of a more specific match.
In the end I dropped that idea and added it to all board dtsi files.
However, by using a name that doesn't match the automatism rules, I can centralize these settings. I'll do that for V3.
I think applying it on A10 would be fine too, dunno about S10.

Am Fr., 22. Feb. 2019, 18:34 hat Marek Vasut marex@denx.de geschrieben:
On 2/22/19 7:20 AM, Simon Goldschmidt wrote:
On Thu, Feb 21, 2019 at 11:13 PM Marek Vasut marex@denx.de wrote:
On 2/21/19 11:11 PM, Simon Goldschmidt wrote:
Am Do., 21. Feb. 2019, 23:05 hat Marek Vasut <marex@denx.de mailto:marex@denx.de> geschrieben:
On 2/21/19 11:03 PM, Simon Goldschmidt wrote: > > > Am Do., 21. Feb. 2019, 22:56 hat Marek Vasut <marex@denx.de <mailto:marex@denx.de> > <mailto:marex@denx.de <mailto:marex@denx.de>>> geschrieben: > > On 2/21/19 10:43 PM, Simon Goldschmidt wrote: > > The SPL for socfpga gen5 currently takes all peripherals
out
of reset > > unconditionally. To implement proper reset handling for peripherals, > > the reset node has to be provided with the SPL dts. > > > > In preparation to move the DDR driver to DM, the sdr node
is
required > > in SPL, too. > > > > This patch adds "u-boot,dm-pre-reloc" to U-Boot specific dtsi addon > > files so that the reset manager and SDR driver correctly probe in SPL. > > > > Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com <mailto:simon.k.r.goldschmidt@gmail.com> > <mailto:simon.k.r.goldschmidt@gmail.com <mailto:simon.k.r.goldschmidt@gmail.com>>> > > --- > > > > Changes in v2: None > > > > arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi | 8 ++++++++ > > arch/arm/dts/socfpga_cyclone5_dbm_soc1.dts | 8 ++++++++ > > arch/arm/dts/socfpga_cyclone5_de0_nano_soc-u-boot.dtsi | 8 ++++++++ > > arch/arm/dts/socfpga_cyclone5_de10_nano.dts | 8 ++++++++ > > arch/arm/dts/socfpga_cyclone5_de1_soc.dts | 8 ++++++++ > > arch/arm/dts/socfpga_cyclone5_is1.dts | 8 ++++++++ > > arch/arm/dts/socfpga_cyclone5_socdk-u-boot.dtsi | 8 ++++++++ > > arch/arm/dts/socfpga_cyclone5_sockit-u-boot.dtsi | 8 ++++++++ > > arch/arm/dts/socfpga_cyclone5_socrates-u-boot.dtsi | 8 ++++++++ > > arch/arm/dts/socfpga_cyclone5_sr1500.dts | 8 ++++++++ > > arch/arm/dts/socfpga_cyclone5_vining_fpga-u-boot.dtsi | 8 ++++++++ > > 11 files changed, 88 insertions(+) > > > > diff --git a/arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi > b/arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi > > index c44d1ee2fa..8aaec56285 100644 > > --- a/arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi > > +++ b/arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi > > @@ -17,6 +17,14 @@ > > }; > > }; > > > > +&rst { > > + u-boot,dm-pre-reloc; > > +}; > > + > > +&sdr { > > + u-boot,dm-pre-reloc; > > +}; > > What about some socfpga-common-u-boot.dtsi to avoid
duplication ?
> > > Right. I tested with 'socfpga-u-boot.dtsi' but the hot included
for
> gen10 as well. hot included ? I don't think I understand.
Argh. I'm writing from my mobile...
It can wait till tomorrow ...
What I meant was that file got included by the automatism so I couldn't use it.
Errr, which file ? What automatism ? Really, wait till tomorrow with a reply, please.
Let me try again: I started this patch with trying to centralize the "u-boot,dm-pre-reloc" lines in one file named "socfpga-u-boot.dtsi" since it is valid for all gen5 devices and including it automatically would be OK.
However, the automatism to search for a "-u-boot.dtsi" file caught that file also for a10 and even s10 due to the lack of a more specific match.
In the end I dropped that idea and added it to all board dtsi files.
However, by using a name that doesn't match the automatism rules, I can centralize these settings. I'll do that for V3.
I think applying it on A10 would be fine too, dunno about S10.
S10 is where it failed compiling for me. A10 compiled OK but i could not test it. I guess going the safe way and applying for Gen5 only would be better.
Regards, Simon

On 2/22/19 8:12 PM, Simon Goldschmidt wrote:
Am Fr., 22. Feb. 2019, 18:34 hat Marek Vasut <marex@denx.de mailto:marex@denx.de> geschrieben:
On 2/22/19 7:20 AM, Simon Goldschmidt wrote: > On Thu, Feb 21, 2019 at 11:13 PM Marek Vasut <marex@denx.de <mailto:marex@denx.de>> wrote: >> >> On 2/21/19 11:11 PM, Simon Goldschmidt wrote: >>> >>> >>> Am Do., 21. Feb. 2019, 23:05 hat Marek Vasut <marex@denx.de <mailto:marex@denx.de> >>> <mailto:marex@denx.de <mailto:marex@denx.de>>> geschrieben: >>> >>> On 2/21/19 11:03 PM, Simon Goldschmidt wrote: >>> > >>> > >>> > Am Do., 21. Feb. 2019, 22:56 hat Marek Vasut <marex@denx.de <mailto:marex@denx.de> >>> <mailto:marex@denx.de <mailto:marex@denx.de>> >>> > <mailto:marex@denx.de <mailto:marex@denx.de> <mailto:marex@denx.de <mailto:marex@denx.de>>>> geschrieben: >>> > >>> > On 2/21/19 10:43 PM, Simon Goldschmidt wrote: >>> > > The SPL for socfpga gen5 currently takes all peripherals out >>> of reset >>> > > unconditionally. To implement proper reset handling for >>> peripherals, >>> > > the reset node has to be provided with the SPL dts. >>> > > >>> > > In preparation to move the DDR driver to DM, the sdr node is >>> required >>> > > in SPL, too. >>> > > >>> > > This patch adds "u-boot,dm-pre-reloc" to U-Boot specific >>> dtsi addon >>> > > files so that the reset manager and SDR driver correctly >>> probe in SPL. >>> > > >>> > > Signed-off-by: Simon Goldschmidt >>> <simon.k.r.goldschmidt@gmail.com <mailto:simon.k.r.goldschmidt@gmail.com> >>> <mailto:simon.k.r.goldschmidt@gmail.com <mailto:simon.k.r.goldschmidt@gmail.com>> >>> > <mailto:simon.k.r.goldschmidt@gmail.com <mailto:simon.k.r.goldschmidt@gmail.com> >>> <mailto:simon.k.r.goldschmidt@gmail.com <mailto:simon.k.r.goldschmidt@gmail.com>>>> >>> > > --- >>> > > >>> > > Changes in v2: None >>> > > >>> > > arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi | 8 >>> ++++++++ >>> > > arch/arm/dts/socfpga_cyclone5_dbm_soc1.dts | 8 >>> ++++++++ >>> > > arch/arm/dts/socfpga_cyclone5_de0_nano_soc-u-boot.dtsi | 8 >>> ++++++++ >>> > > arch/arm/dts/socfpga_cyclone5_de10_nano.dts | 8 >>> ++++++++ >>> > > arch/arm/dts/socfpga_cyclone5_de1_soc.dts | 8 >>> ++++++++ >>> > > arch/arm/dts/socfpga_cyclone5_is1.dts | 8 >>> ++++++++ >>> > > arch/arm/dts/socfpga_cyclone5_socdk-u-boot.dtsi | 8 >>> ++++++++ >>> > > arch/arm/dts/socfpga_cyclone5_sockit-u-boot.dtsi | 8 >>> ++++++++ >>> > > arch/arm/dts/socfpga_cyclone5_socrates-u-boot.dtsi | 8 >>> ++++++++ >>> > > arch/arm/dts/socfpga_cyclone5_sr1500.dts | 8 >>> ++++++++ >>> > > arch/arm/dts/socfpga_cyclone5_vining_fpga-u-boot.dtsi | 8 >>> ++++++++ >>> > > 11 files changed, 88 insertions(+) >>> > > >>> > > diff --git a/arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi >>> > b/arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi >>> > > index c44d1ee2fa..8aaec56285 100644 >>> > > --- a/arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi >>> > > +++ b/arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi >>> > > @@ -17,6 +17,14 @@ >>> > > }; >>> > > }; >>> > > >>> > > +&rst { >>> > > + u-boot,dm-pre-reloc; >>> > > +}; >>> > > + >>> > > +&sdr { >>> > > + u-boot,dm-pre-reloc; >>> > > +}; >>> > >>> > What about some socfpga-common-u-boot.dtsi to avoid duplication ? >>> > >>> > >>> > Right. I tested with 'socfpga-u-boot.dtsi' but the hot included for >>> > gen10 as well. >>> >>> hot included ? I don't think I understand. >>> >>> >>> Argh. I'm writing from my mobile... >> >> It can wait till tomorrow ... >> >>> What I meant was that file got included by the automatism so I couldn't >>> use it. >> Errr, which file ? What automatism ? Really, wait till tomorrow with a >> reply, please. > > Let me try again: I started this patch with trying to centralize the > "u-boot,dm-pre-reloc" > lines in one file named "socfpga-u-boot.dtsi" since it is valid for > all gen5 devices > and including it automatically would be OK. > > However, the automatism to search for a "-u-boot.dtsi" file caught > that file also for > a10 and even s10 due to the lack of a more specific match. > > In the end I dropped that idea and added it to all board dtsi files. > > However, by using a name that doesn't match the automatism rules, I > can centralize > these settings. I'll do that for V3. I think applying it on A10 would be fine too, dunno about S10.
S10 is where it failed compiling for me. A10 compiled OK but i could not test it. I guess going the safe way and applying for Gen5 only would be better.
Chee can test A10 for you.
What's the problem with S10 ?

Am Fr., 22. Feb. 2019, 20:36 hat Marek Vasut marex@denx.de geschrieben:
On 2/22/19 8:12 PM, Simon Goldschmidt wrote:
Am Fr., 22. Feb. 2019, 18:34 hat Marek Vasut <marex@denx.de mailto:marex@denx.de> geschrieben:
On 2/22/19 7:20 AM, Simon Goldschmidt wrote: > On Thu, Feb 21, 2019 at 11:13 PM Marek Vasut <marex@denx.de <mailto:marex@denx.de>> wrote: >> >> On 2/21/19 11:11 PM, Simon Goldschmidt wrote: >>> >>> >>> Am Do., 21. Feb. 2019, 23:05 hat Marek Vasut <marex@denx.de <mailto:marex@denx.de> >>> <mailto:marex@denx.de <mailto:marex@denx.de>>> geschrieben: >>> >>> On 2/21/19 11:03 PM, Simon Goldschmidt wrote: >>> > >>> > >>> > Am Do., 21. Feb. 2019, 22:56 hat Marek Vasut <marex@denx.de <mailto:marex@denx.de> >>> <mailto:marex@denx.de <mailto:marex@denx.de>> >>> > <mailto:marex@denx.de <mailto:marex@denx.de> <mailto:marex@denx.de <mailto:marex@denx.de>>>> geschrieben: >>> > >>> > On 2/21/19 10:43 PM, Simon Goldschmidt wrote: >>> > > The SPL for socfpga gen5 currently takes all peripherals out >>> of reset >>> > > unconditionally. To implement proper reset handling
for
>>> peripherals, >>> > > the reset node has to be provided with the SPL dts. >>> > > >>> > > In preparation to move the DDR driver to DM, the sdr node is >>> required >>> > > in SPL, too. >>> > > >>> > > This patch adds "u-boot,dm-pre-reloc" to U-Boot
specific
>>> dtsi addon >>> > > files so that the reset manager and SDR driver
correctly
>>> probe in SPL. >>> > > >>> > > Signed-off-by: Simon Goldschmidt >>> <simon.k.r.goldschmidt@gmail.com <mailto:simon.k.r.goldschmidt@gmail.com> >>> <mailto:simon.k.r.goldschmidt@gmail.com <mailto:simon.k.r.goldschmidt@gmail.com>> >>> > <mailto:simon.k.r.goldschmidt@gmail.com <mailto:simon.k.r.goldschmidt@gmail.com> >>> <mailto:simon.k.r.goldschmidt@gmail.com <mailto:simon.k.r.goldschmidt@gmail.com>>>> >>> > > --- >>> > > >>> > > Changes in v2: None >>> > > >>> > > arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi | 8 >>> ++++++++ >>> > > arch/arm/dts/socfpga_cyclone5_dbm_soc1.dts | 8 >>> ++++++++ >>> > > arch/arm/dts/socfpga_cyclone5_de0_nano_soc-u-boot.dtsi | 8 >>> ++++++++ >>> > > arch/arm/dts/socfpga_cyclone5_de10_nano.dts | 8 >>> ++++++++ >>> > > arch/arm/dts/socfpga_cyclone5_de1_soc.dts | 8 >>> ++++++++ >>> > > arch/arm/dts/socfpga_cyclone5_is1.dts | 8 >>> ++++++++ >>> > > arch/arm/dts/socfpga_cyclone5_socdk-u-boot.dtsi | 8 >>> ++++++++ >>> > > arch/arm/dts/socfpga_cyclone5_sockit-u-boot.dtsi | 8 >>> ++++++++ >>> > > arch/arm/dts/socfpga_cyclone5_socrates-u-boot.dtsi | 8 >>> ++++++++ >>> > > arch/arm/dts/socfpga_cyclone5_sr1500.dts | 8 >>> ++++++++ >>> > > arch/arm/dts/socfpga_cyclone5_vining_fpga-u-boot.dtsi | 8 >>> ++++++++ >>> > > 11 files changed, 88 insertions(+) >>> > > >>> > > diff --git a/arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi >>> > b/arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi >>> > > index c44d1ee2fa..8aaec56285 100644 >>> > > --- a/arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi >>> > > +++ b/arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi >>> > > @@ -17,6 +17,14 @@ >>> > > }; >>> > > }; >>> > > >>> > > +&rst { >>> > > + u-boot,dm-pre-reloc; >>> > > +}; >>> > > + >>> > > +&sdr { >>> > > + u-boot,dm-pre-reloc; >>> > > +}; >>> > >>> > What about some socfpga-common-u-boot.dtsi to avoid duplication ? >>> > >>> > >>> > Right. I tested with 'socfpga-u-boot.dtsi' but the hot included for >>> > gen10 as well. >>> >>> hot included ? I don't think I understand. >>> >>> >>> Argh. I'm writing from my mobile... >> >> It can wait till tomorrow ... >> >>> What I meant was that file got included by the automatism so I couldn't >>> use it. >> Errr, which file ? What automatism ? Really, wait till tomorrow with a >> reply, please. > > Let me try again: I started this patch with trying to centralize
the
> "u-boot,dm-pre-reloc" > lines in one file named "socfpga-u-boot.dtsi" since it is valid for > all gen5 devices > and including it automatically would be OK. > > However, the automatism to search for a "-u-boot.dtsi" file caught > that file also for > a10 and even s10 due to the lack of a more specific match. > > In the end I dropped that idea and added it to all board dtsi
files.
> > However, by using a name that doesn't match the automatism rules, I > can centralize > these settings. I'll do that for V3. I think applying it on A10 would be fine too, dunno about S10.
S10 is where it failed compiling for me. A10 compiled OK but i could not test it. I guess going the safe way and applying for Gen5 only would be better.
Chee can test A10 for you.
What's the problem with S10 ?
I really can't remember. However, That would mean converting all socfpga drivers to UCLASS_RAM and moving them to drivers/ram. That would be a good thing, but I don't know when I would find the time for that...
Regards, Simon

On 2/22/19 8:39 PM, Simon Goldschmidt wrote:
Am Fr., 22. Feb. 2019, 20:36 hat Marek Vasut <marex@denx.de mailto:marex@denx.de> geschrieben:
On 2/22/19 8:12 PM, Simon Goldschmidt wrote: > > > Am Fr., 22. Feb. 2019, 18:34 hat Marek Vasut <marex@denx.de <mailto:marex@denx.de> > <mailto:marex@denx.de <mailto:marex@denx.de>>> geschrieben: > > On 2/22/19 7:20 AM, Simon Goldschmidt wrote: > > On Thu, Feb 21, 2019 at 11:13 PM Marek Vasut <marex@denx.de <mailto:marex@denx.de> > <mailto:marex@denx.de <mailto:marex@denx.de>>> wrote: > >> > >> On 2/21/19 11:11 PM, Simon Goldschmidt wrote: > >>> > >>> > >>> Am Do., 21. Feb. 2019, 23:05 hat Marek Vasut <marex@denx.de <mailto:marex@denx.de> > <mailto:marex@denx.de <mailto:marex@denx.de>> > >>> <mailto:marex@denx.de <mailto:marex@denx.de> <mailto:marex@denx.de <mailto:marex@denx.de>>>> geschrieben: > >>> > >>> On 2/21/19 11:03 PM, Simon Goldschmidt wrote: > >>> > > >>> > > >>> > Am Do., 21. Feb. 2019, 22:56 hat Marek Vasut > <marex@denx.de <mailto:marex@denx.de> <mailto:marex@denx.de <mailto:marex@denx.de>> > >>> <mailto:marex@denx.de <mailto:marex@denx.de> <mailto:marex@denx.de <mailto:marex@denx.de>>> > >>> > <mailto:marex@denx.de <mailto:marex@denx.de> <mailto:marex@denx.de <mailto:marex@denx.de>> > <mailto:marex@denx.de <mailto:marex@denx.de> <mailto:marex@denx.de <mailto:marex@denx.de>>>>> geschrieben: > >>> > > >>> > On 2/21/19 10:43 PM, Simon Goldschmidt wrote: > >>> > > The SPL for socfpga gen5 currently takes all > peripherals out > >>> of reset > >>> > > unconditionally. To implement proper reset handling for > >>> peripherals, > >>> > > the reset node has to be provided with the SPL dts. > >>> > > > >>> > > In preparation to move the DDR driver to DM, the sdr > node is > >>> required > >>> > > in SPL, too. > >>> > > > >>> > > This patch adds "u-boot,dm-pre-reloc" to U-Boot specific > >>> dtsi addon > >>> > > files so that the reset manager and SDR driver correctly > >>> probe in SPL. > >>> > > > >>> > > Signed-off-by: Simon Goldschmidt > >>> <simon.k.r.goldschmidt@gmail.com <mailto:simon.k.r.goldschmidt@gmail.com> > <mailto:simon.k.r.goldschmidt@gmail.com <mailto:simon.k.r.goldschmidt@gmail.com>> > >>> <mailto:simon.k.r.goldschmidt@gmail.com <mailto:simon.k.r.goldschmidt@gmail.com> > <mailto:simon.k.r.goldschmidt@gmail.com <mailto:simon.k.r.goldschmidt@gmail.com>>> > >>> > <mailto:simon.k.r.goldschmidt@gmail.com <mailto:simon.k.r.goldschmidt@gmail.com> > <mailto:simon.k.r.goldschmidt@gmail.com <mailto:simon.k.r.goldschmidt@gmail.com>> > >>> <mailto:simon.k.r.goldschmidt@gmail.com <mailto:simon.k.r.goldschmidt@gmail.com> > <mailto:simon.k.r.goldschmidt@gmail.com <mailto:simon.k.r.goldschmidt@gmail.com>>>>> > >>> > > --- > >>> > > > >>> > > Changes in v2: None > >>> > > > >>> > > arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi > | 8 > >>> ++++++++ > >>> > > arch/arm/dts/socfpga_cyclone5_dbm_soc1.dts > | 8 > >>> ++++++++ > >>> > > > arch/arm/dts/socfpga_cyclone5_de0_nano_soc-u-boot.dtsi | 8 > >>> ++++++++ > >>> > > arch/arm/dts/socfpga_cyclone5_de10_nano.dts > | 8 > >>> ++++++++ > >>> > > arch/arm/dts/socfpga_cyclone5_de1_soc.dts > | 8 > >>> ++++++++ > >>> > > arch/arm/dts/socfpga_cyclone5_is1.dts > | 8 > >>> ++++++++ > >>> > > arch/arm/dts/socfpga_cyclone5_socdk-u-boot.dtsi > | 8 > >>> ++++++++ > >>> > > arch/arm/dts/socfpga_cyclone5_sockit-u-boot.dtsi > | 8 > >>> ++++++++ > >>> > > arch/arm/dts/socfpga_cyclone5_socrates-u-boot.dtsi > | 8 > >>> ++++++++ > >>> > > arch/arm/dts/socfpga_cyclone5_sr1500.dts > | 8 > >>> ++++++++ > >>> > > > arch/arm/dts/socfpga_cyclone5_vining_fpga-u-boot.dtsi | 8 > >>> ++++++++ > >>> > > 11 files changed, 88 insertions(+) > >>> > > > >>> > > diff --git > a/arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi > >>> > b/arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi > >>> > > index c44d1ee2fa..8aaec56285 100644 > >>> > > --- a/arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi > >>> > > +++ b/arch/arm/dts/socfpga_arria5_socdk-u-boot.dtsi > >>> > > @@ -17,6 +17,14 @@ > >>> > > }; > >>> > > }; > >>> > > > >>> > > +&rst { > >>> > > + u-boot,dm-pre-reloc; > >>> > > +}; > >>> > > + > >>> > > +&sdr { > >>> > > + u-boot,dm-pre-reloc; > >>> > > +}; > >>> > > >>> > What about some socfpga-common-u-boot.dtsi to avoid > duplication ? > >>> > > >>> > > >>> > Right. I tested with 'socfpga-u-boot.dtsi' but the hot > included for > >>> > gen10 as well. > >>> > >>> hot included ? I don't think I understand. > >>> > >>> > >>> Argh. I'm writing from my mobile... > >> > >> It can wait till tomorrow ... > >> > >>> What I meant was that file got included by the automatism so I > couldn't > >>> use it. > >> Errr, which file ? What automatism ? Really, wait till tomorrow > with a > >> reply, please. > > > > Let me try again: I started this patch with trying to centralize the > > "u-boot,dm-pre-reloc" > > lines in one file named "socfpga-u-boot.dtsi" since it is valid for > > all gen5 devices > > and including it automatically would be OK. > > > > However, the automatism to search for a "-u-boot.dtsi" file caught > > that file also for > > a10 and even s10 due to the lack of a more specific match. > > > > In the end I dropped that idea and added it to all board dtsi files. > > > > However, by using a name that doesn't match the automatism rules, I > > can centralize > > these settings. I'll do that for V3. > > I think applying it on A10 would be fine too, dunno about S10. > > > S10 is where it failed compiling for me. A10 compiled OK but i could not > test it. I guess going the safe way and applying for Gen5 only would be > better. Chee can test A10 for you. What's the problem with S10 ?
I really can't remember. However, That would mean converting all socfpga drivers to UCLASS_RAM and moving them to drivers/ram. That would be a good thing, but I don't know when I would find the time for that...
Ah ... OK, then let's skip it for now. I'd like this series to land after 2019.04.

To clean up reset handling for socfpga gen5, port the DDR driver to DM using UCLASS_RAM and implement proper reset handling.
This gets us rid of one ad-hoc call to socfpga_per_reset().
The gen5 driver is implemented in 2 distinct files. One of it (containing the calibration training) is not touched much and is kept at using hard coded addresses since the code grows even more otherwise.
SPL is changed from calling hard into the DDR driver code to just probing UCLASS_RESET and UCLASS_RAM. It is happy after finding a RAM driver after that.
Signed-off-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com ---
Changes in v2: - port DDR driver to DM UCLASS_RAM - don't change DDR calibration training driver (code got too big) - use reset.h code instead of socfpga_per_reset()
arch/arm/Kconfig | 2 + arch/arm/dts/socfpga.dtsi | 4 +- .../mach-socfpga/include/mach/sdram_gen5.h | 4 - arch/arm/mach-socfpga/spl_gen5.c | 29 ++-- drivers/ddr/altera/Kconfig | 1 + drivers/ddr/altera/sdram_gen5.c | 143 ++++++++++++++++-- drivers/ddr/altera/sequencer.c | 9 +- drivers/ddr/altera/sequencer.h | 35 +++++ 8 files changed, 185 insertions(+), 42 deletions(-)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 455f06cfee..8ff4529095 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -821,12 +821,14 @@ config ARCH_SOCFPGA select DM_SERIAL select ENABLE_ARM_SOC_BOOT0_HOOK if TARGET_SOCFPGA_GEN5 || TARGET_SOCFPGA_ARRIA10 select OF_CONTROL + select RAM if TARGET_SOCFPGA_GEN5 select SPL_DM_RESET if DM_RESET select SPL_DM_SERIAL select SPL_LIBCOMMON_SUPPORT select SPL_LIBGENERIC_SUPPORT select SPL_NAND_SUPPORT if SPL_NAND_DENALI select SPL_OF_CONTROL + select SPL_RAM if TARGET_SOCFPGA_GEN5 select SPL_SEPARATE_BSS if TARGET_SOCFPGA_STRATIX10 select SPL_SERIAL_SUPPORT select SPL_WATCHDOG_SUPPORT diff --git a/arch/arm/dts/socfpga.dtsi b/arch/arm/dts/socfpga.dtsi index ec1966480f..51a6a51b53 100644 --- a/arch/arm/dts/socfpga.dtsi +++ b/arch/arm/dts/socfpga.dtsi @@ -791,9 +791,9 @@ reg = <0xfffec000 0x100>; };
- sdr: sdr@ffc25000 { + sdr: sdr@ffc20000 { compatible = "altr,sdr-ctl", "syscon"; - reg = <0xffc25000 0x1000>; + reg = <0xffc20000 0x6000>; resets = <&rst SDR_RESET>; };
diff --git a/arch/arm/mach-socfpga/include/mach/sdram_gen5.h b/arch/arm/mach-socfpga/include/mach/sdram_gen5.h index a238d5d17f..c41208591a 100644 --- a/arch/arm/mach-socfpga/include/mach/sdram_gen5.h +++ b/arch/arm/mach-socfpga/include/mach/sdram_gen5.h @@ -7,10 +7,6 @@
#ifndef __ASSEMBLY__
-unsigned long sdram_calculate_size(void); -int sdram_mmr_init_full(unsigned int sdr_phy_reg); -int sdram_calibration_full(void); - const struct socfpga_sdram_config *socfpga_get_sdram_config(void);
void socfpga_get_seq_ac_init(const u32 **init, unsigned int *nelem); diff --git a/arch/arm/mach-socfpga/spl_gen5.c b/arch/arm/mach-socfpga/spl_gen5.c index 4c9f7997be..1bff8cbfcf 100644 --- a/arch/arm/mach-socfpga/spl_gen5.c +++ b/arch/arm/mach-socfpga/spl_gen5.c @@ -20,6 +20,7 @@ #include <debug_uart.h> #include <fdtdec.h> #include <watchdog.h> +#include <dm/uclass.h>
DECLARE_GLOBAL_DATA_PTR;
@@ -66,9 +67,9 @@ u32 spl_boot_mode(const u32 boot_device) void board_init_f(ulong dummy) { const struct cm_config *cm_default_cfg = cm_get_default_config(); - unsigned long sdram_size; unsigned long reg; int ret; + struct udevice *dev;
/* * First C code to run. Clear fake OCRAM ECC first as SBE @@ -98,7 +99,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);
@@ -142,27 +142,16 @@ void board_init_f(ulong dummy) hang(); }
+ ret = uclass_get_device(UCLASS_RESET, 0, &dev); + if (ret) + debug("Reset init failed: %d\n", ret); + /* enable console uart printing */ preloader_console_init();
- if (sdram_mmr_init_full(0xffffffff) != 0) { - puts("SDRAM init failed.\n"); - hang(); - } - - debug("SDRAM: Calibrating PHY\n"); - /* SDRAM calibration */ - if (sdram_calibration_full() == 0) { - puts("SDRAM calibration failed.\n"); - hang(); - } - - sdram_size = sdram_calculate_size(); - debug("SDRAM: %ld MiB\n", sdram_size >> 20); - - /* Sanity check ensure correct SDRAM size specified */ - if (get_ram_size(0, sdram_size) != sdram_size) { - puts("SDRAM size check failed!\n"); + ret = uclass_get_device(UCLASS_RAM, 0, &dev); + if (ret) { + debug("DRAM init failed: %d\n", ret); hang(); }
diff --git a/drivers/ddr/altera/Kconfig b/drivers/ddr/altera/Kconfig index 2b28a97f6e..7370d4133a 100644 --- a/drivers/ddr/altera/Kconfig +++ b/drivers/ddr/altera/Kconfig @@ -1,5 +1,6 @@ config ALTERA_SDRAM bool "SoCFPGA DDR SDRAM driver" + depends on RAM depends on TARGET_SOCFPGA_GEN5 || TARGET_SOCFPGA_ARRIA10 help Enable DDR SDRAM controller for the SoCFPGA devices. diff --git a/drivers/ddr/altera/sdram_gen5.c b/drivers/ddr/altera/sdram_gen5.c index 821060459c..fcd89b619d 100644 --- a/drivers/ddr/altera/sdram_gen5.c +++ b/drivers/ddr/altera/sdram_gen5.c @@ -3,14 +3,30 @@ * Copyright Altera Corporation (C) 2014-2015 */ #include <common.h> +#include <dm.h> #include <errno.h> #include <div64.h> +#include <ram.h> +#include <reset.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>
+#include "sequencer.h" + +#ifdef CONFIG_SPL_BUILD + +struct altera_gen5_sdram_priv { + struct ram_info info; +}; + +struct altera_gen5_sdram_platdata { + struct socfpga_sdr *sdr; +}; + struct sdram_prot_rule { u32 sdram_start; /* SDRAM start address */ u32 sdram_end; /* SDRAM end address */ @@ -26,8 +42,8 @@ struct sdram_prot_rule {
static struct socfpga_system_manager *sysmgr_regs = (struct socfpga_system_manager *)SOCFPGA_SYSMGR_ADDRESS; -static struct socfpga_sdr_ctrl *sdr_ctrl = - (struct socfpga_sdr_ctrl *)SDR_CTRLGRP_ADDRESS; + +static unsigned long sdram_calculate_size(struct socfpga_sdr_ctrl *sdr_ctrl);
/** * get_errata_rows() - Up the number of DRAM rows to cover entire address space @@ -104,7 +120,8 @@ static int get_errata_rows(const struct socfpga_sdram_config *cfg) }
/* SDRAM protection rules vary from 0-19, a total of 20 rules. */ -static void sdram_set_rule(struct sdram_prot_rule *prule) +static void sdram_set_rule(struct socfpga_sdr_ctrl *sdr_ctrl, + struct sdram_prot_rule *prule) { u32 lo_addr_bits; u32 hi_addr_bits; @@ -141,7 +158,8 @@ static void sdram_set_rule(struct sdram_prot_rule *prule) writel(0, &sdr_ctrl->prot_rule_rdwr); }
-static void sdram_get_rule(struct sdram_prot_rule *prule) +static void sdram_get_rule(struct socfpga_sdr_ctrl *sdr_ctrl, + struct sdram_prot_rule *prule) { u32 addr; u32 id; @@ -172,7 +190,8 @@ static void sdram_get_rule(struct sdram_prot_rule *prule) }
static void -sdram_set_protection_config(const u32 sdram_start, const u32 sdram_end) +sdram_set_protection_config(struct socfpga_sdr_ctrl *sdr_ctrl, + const u32 sdram_start, const u32 sdram_end) { struct sdram_prot_rule rule; int rules; @@ -185,7 +204,7 @@ sdram_set_protection_config(const u32 sdram_start, const u32 sdram_end)
for (rules = 0; rules < 20; rules++) { rule.rule = rules; - sdram_set_rule(&rule); + sdram_set_rule(sdr_ctrl, &rule); }
/* new rule: accept SDRAM */ @@ -200,13 +219,13 @@ sdram_set_protection_config(const u32 sdram_start, const u32 sdram_end) rule.rule = 0;
/* set new rule */ - sdram_set_rule(&rule); + sdram_set_rule(sdr_ctrl, &rule);
/* default rule: reject everything */ writel(0x3ff, &sdr_ctrl->protport_default); }
-static void sdram_dump_protection_config(void) +static void sdram_dump_protection_config(struct socfpga_sdr_ctrl *sdr_ctrl) { struct sdram_prot_rule rule; int rules; @@ -216,7 +235,7 @@ static void sdram_dump_protection_config(void)
for (rules = 0; rules < 20; rules++) { rule.rule = rules; - sdram_get_rule(&rule); + sdram_get_rule(sdr_ctrl, &rule); debug("Rule %d, rules ...\n", rules); debug(" sdram start %x\n", rule.sdram_start); debug(" sdram end %x\n", rule.sdram_end); @@ -322,7 +341,8 @@ static u32 sdr_get_addr_rw(const struct socfpga_sdram_config *cfg) * * This function loads the register values into the SDRAM controller block. */ -static void sdr_load_regs(const struct socfpga_sdram_config *cfg) +static void sdr_load_regs(struct socfpga_sdr_ctrl *sdr_ctrl, + const struct socfpga_sdram_config *cfg) { const u32 ctrl_cfg = sdr_get_ctrlcfg(cfg); const u32 dram_addrw = sdr_get_addr_rw(cfg); @@ -426,7 +446,8 @@ static void sdr_load_regs(const struct socfpga_sdram_config *cfg) * * Initialize the SDRAM MMR. */ -int sdram_mmr_init_full(unsigned int sdr_phy_reg) +int sdram_mmr_init_full(struct socfpga_sdr_ctrl *sdr_ctrl, + unsigned int sdr_phy_reg) { const struct socfpga_sdram_config *cfg = socfpga_get_sdram_config(); const unsigned int rows = @@ -436,7 +457,7 @@ int sdram_mmr_init_full(unsigned int sdr_phy_reg)
writel(rows, &sysmgr_regs->iswgrp_handoff[4]);
- sdr_load_regs(cfg); + sdr_load_regs(sdr_ctrl, cfg);
/* saving this value to SYSMGR.ISWGRP.HANDOFF.FPGA2SDR */ writel(cfg->fpgaport_rst, &sysmgr_regs->iswgrp_handoff[3]); @@ -459,9 +480,10 @@ int sdram_mmr_init_full(unsigned int sdr_phy_reg) SDR_CTRLGRP_STATICCFG_APPLYCFG_MASK, 1 << SDR_CTRLGRP_STATICCFG_APPLYCFG_LSB);
- sdram_set_protection_config(0, sdram_calculate_size() - 1); + sdram_set_protection_config(sdr_ctrl, 0, + sdram_calculate_size(sdr_ctrl) - 1);
- sdram_dump_protection_config(); + sdram_dump_protection_config(sdr_ctrl);
return 0; } @@ -472,7 +494,7 @@ int sdram_mmr_init_full(unsigned int sdr_phy_reg) * Calculate SDRAM device size based on SDRAM controller parameters. * Size is specified in bytes. */ -unsigned long sdram_calculate_size(void) +static unsigned long sdram_calculate_size(struct socfpga_sdr_ctrl *sdr_ctrl) { unsigned long temp; unsigned long row, bank, col, cs, width; @@ -534,3 +556,94 @@ unsigned long sdram_calculate_size(void)
return temp; } + +static int altera_gen5_sdram_ofdata_to_platdata(struct udevice *dev) +{ + struct altera_gen5_sdram_platdata *plat = dev->platdata; + + plat->sdr = (struct socfpga_sdr *)devfdt_get_addr_index(dev, 0); + if (!plat->sdr) + return -ENODEV; + + return 0; +} + +static int altera_gen5_sdram_probe(struct udevice *dev) +{ + int ret; + unsigned long sdram_size; + struct altera_gen5_sdram_platdata *plat = dev->platdata; + struct altera_gen5_sdram_priv *priv = dev_get_priv(dev); + struct socfpga_sdr_ctrl *sdr_ctrl = &plat->sdr->sdr_ctrl; + struct reset_ctl_bulk resets; + + ret = reset_get_bulk(dev, &resets); + if (ret) { + dev_err(dev, "Can't get reset: %d\n", ret); + return -ENODEV; + } + reset_deassert_bulk(&resets); + + if (sdram_mmr_init_full(sdr_ctrl, 0xffffffff) != 0) { + puts("SDRAM init failed.\n"); + goto failed; + } + + debug("SDRAM: Calibrating PHY\n"); + /* SDRAM calibration */ + if (sdram_calibration_full(plat->sdr) == 0) { + puts("SDRAM calibration failed.\n"); + goto failed; + } + + sdram_size = sdram_calculate_size(sdr_ctrl); + debug("SDRAM: %ld MiB\n", sdram_size >> 20); + + /* Sanity check ensure correct SDRAM size specified */ + if (get_ram_size(0, sdram_size) != sdram_size) { + puts("SDRAM size check failed!\n"); + goto failed; + } + + priv->info.base = 0; + priv->info.size = sdram_size; + + return 0; + +failed: + reset_release_bulk(&resets); + return -ENODEV; +} + +static int altera_gen5_sdram_get_info(struct udevice *dev, + struct ram_info *info) +{ + struct altera_gen5_sdram_priv *priv = dev_get_priv(dev); + + info->base = priv->info.base; + info->size = priv->info.size; + + return 0; +} + +static struct ram_ops altera_gen5_sdram_ops = { + .get_info = altera_gen5_sdram_get_info, +}; + +static const struct udevice_id altera_gen5_sdram_ids[] = { + { .compatible = "altr,sdr-ctl" }, + { /* sentinel */ } +}; + +U_BOOT_DRIVER(altera_gen5_sdram) = { + .name = "altr_sdr_ctl", + .id = UCLASS_RAM, + .of_match = altera_gen5_sdram_ids, + .ops = &altera_gen5_sdram_ops, + .ofdata_to_platdata = altera_gen5_sdram_ofdata_to_platdata, + .platdata_auto_alloc_size = sizeof(struct altera_gen5_sdram_platdata), + .probe = altera_gen5_sdram_probe, + .priv_auto_alloc_size = sizeof(struct altera_gen5_sdram_priv), +}; + +#endif /* CONFIG_SPL_BUILD */ diff --git a/drivers/ddr/altera/sequencer.c b/drivers/ddr/altera/sequencer.c index 5e7a943b68..0e4526288e 100644 --- a/drivers/ddr/altera/sequencer.c +++ b/drivers/ddr/altera/sequencer.c @@ -3705,12 +3705,19 @@ static void initialize_tracking(void) &sdr_reg_file->trk_rfsh); }
-int sdram_calibration_full(void) +int sdram_calibration_full(struct socfpga_sdr *sdr) { struct param_type my_param; struct gbl_type my_gbl; u32 pass;
+ /* + * For size reasons, this file uses hard coded addresses. + * Check if we are called with the correct address. + */ + if (sdr != (struct socfpga_sdr *)SOCFPGA_SDR_ADDRESS) + return -ENODEV; + memset(&my_param, 0, sizeof(my_param)); memset(&my_gbl, 0, sizeof(my_gbl));
diff --git a/drivers/ddr/altera/sequencer.h b/drivers/ddr/altera/sequencer.h index a5760b03a5..d7f6935201 100644 --- a/drivers/ddr/altera/sequencer.h +++ b/drivers/ddr/altera/sequencer.h @@ -223,4 +223,39 @@ struct socfpga_data_mgr { u32 mem_t_add; u32 t_rl_add; }; + +/* This struct describes the controller @ SOCFPGA_SDR_ADDRESS */ +struct socfpga_sdr { + /* SDR_PHYGRP_SCCGRP_ADDRESS */ + u8 _align1[0xe00]; + /* SDR_PHYGRP_SCCGRP_ADDRESS | 0xe00 */ + struct socfpga_sdr_scc_mgr sdr_scc_mgr; + u8 _align2[0x1bc]; + /* SDR_PHYGRP_PHYMGRGRP_ADDRESS */ + struct socfpga_phy_mgr_cmd phy_mgr_cmd; + u8 _align3[0x2c]; + /* SDR_PHYGRP_PHYMGRGRP_ADDRESS | 0x40 */ + struct socfpga_phy_mgr_cfg phy_mgr_cfg; + u8 _align4[0xfa0]; + /* SDR_PHYGRP_RWMGRGRP_ADDRESS */ + u8 rwmgr_grp[0x800]; + /* SDR_PHYGRP_RWMGRGRP_ADDRESS | 0x800 */ + struct socfpga_sdr_rw_load_manager sdr_rw_load_mgr_regs; + u8 _align5[0x3f0]; + /* SDR_PHYGRP_RWMGRGRP_ADDRESS | 0xC00 */ + struct socfpga_sdr_rw_load_jump_manager sdr_rw_load_jump_mgr_regs; + u8 _align6[0x13f0]; + /* SDR_PHYGRP_DATAMGRGRP_ADDRESS */ + struct socfpga_data_mgr data_mgr; + u8 _align7[0x7f0]; + /* SDR_PHYGRP_REGFILEGRP_ADDRESS */ + struct socfpga_sdr_reg_file sdr_reg_file; + u8 _align8[0x7c8]; + /* SDR_CTRLGRP_ADDRESS */ + struct socfpga_sdr_ctrl sdr_ctrl; + u8 _align9[0xea4]; +}; + +int sdram_calibration_full(struct socfpga_sdr *sdr); + #endif /* _SEQUENCER_H_ */

This adds reset handling to the devicetree-enabled 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 ---
Changes in v2: - fix copy/paste issues - add .remove callback to release the resets
drivers/mtd/nand/raw/denali.h | 2 ++ drivers/mtd/nand/raw/denali_dt.c | 14 ++++++++++++++ 2 files changed, 16 insertions(+)
diff --git a/drivers/mtd/nand/raw/denali.h b/drivers/mtd/nand/raw/denali.h index 019deda094..63ae828768 100644 --- a/drivers/mtd/nand/raw/denali.h +++ b/drivers/mtd/nand/raw/denali.h @@ -10,6 +10,7 @@ #include <linux/bitops.h> #include <linux/mtd/rawnand.h> #include <linux/types.h> +#include <reset.h>
#define DEVICE_RESET 0x0 #define DEVICE_RESET__BANK(bank) BIT(bank) @@ -315,6 +316,7 @@ struct denali_nand_info { void (*host_write)(struct denali_nand_info *denali, u32 addr, u32 data); void (*setup_dma)(struct denali_nand_info *denali, dma_addr_t dma_addr, int page, int write); + struct reset_ctl_bulk resets; };
#define DENALI_CAP_HW_ECC_FIXUP BIT(0) diff --git a/drivers/mtd/nand/raw/denali_dt.c b/drivers/mtd/nand/raw/denali_dt.c index d384b974df..a92abca022 100644 --- a/drivers/mtd/nand/raw/denali_dt.c +++ b/drivers/mtd/nand/raw/denali_dt.c @@ -131,15 +131,29 @@ static int denali_dt_probe(struct udevice *dev) denali->clk_x_rate = 200000000; }
+ ret = reset_get_bulk(dev, &denali->resets); + if (ret) + dev_warn(dev, "Can't get reset: %d\n", ret); + else + reset_deassert_bulk(&denali->resets); + return denali_init(denali); }
+static int denali_dt_remove(struct udevice *dev) +{ + struct denali_nand_info *denali = dev_get_priv(dev); + + return reset_release_bulk(&denali->resets); +} + U_BOOT_DRIVER(denali_nand_dt) = { .name = "denali-nand-dt", .id = UCLASS_MISC, .of_match = denali_nand_dt_ids, .probe = denali_dt_probe, .priv_auto_alloc_size = sizeof(struct denali_nand_info), + .remove = denali_dt_remove, };
void board_nand_init(void)

This adds reset handling to the cadence qspi 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 ---
Changes in v2: - add .remove callback to release the resets
drivers/spi/cadence_qspi.c | 16 ++++++++++++++++ drivers/spi/cadence_qspi.h | 4 ++++ 2 files changed, 20 insertions(+)
diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c index 11fce9c4fe..3bfa0201c4 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,17 @@ static int cadence_spi_probe(struct udevice *bus) { struct cadence_spi_platdata *plat = bus->platdata; struct cadence_spi_priv *priv = dev_get_priv(bus); + int ret;
priv->regbase = plat->regbase; priv->ahbbase = plat->ahbbase;
+ ret = reset_get_bulk(bus, &priv->resets); + if (ret) + dev_warn(bus, "Can't get reset: %d\n", ret); + else + reset_deassert_bulk(&priv->resets); + if (!priv->qspi_is_init) { cadence_qspi_apb_controller_init(plat); priv->qspi_is_init = 1; @@ -166,6 +174,13 @@ static int cadence_spi_probe(struct udevice *bus) return 0; }
+static int cadence_spi_remove(struct udevice *dev) +{ + struct cadence_spi_priv *priv = dev_get_priv(dev); + + return reset_release_bulk(&priv->resets); +} + static int cadence_spi_set_mode(struct udevice *bus, uint mode) { struct cadence_spi_priv *priv = dev_get_priv(bus); @@ -342,4 +357,5 @@ U_BOOT_DRIVER(cadence_spi) = { .platdata_auto_alloc_size = sizeof(struct cadence_spi_platdata), .priv_auto_alloc_size = sizeof(struct cadence_spi_priv), .probe = cadence_spi_probe, + .remove = cadence_spi_remove, }; diff --git a/drivers/spi/cadence_qspi.h b/drivers/spi/cadence_qspi.h index 055900def0..d4ede6e15e 100644 --- a/drivers/spi/cadence_qspi.h +++ b/drivers/spi/cadence_qspi.h @@ -7,6 +7,8 @@ #ifndef __CADENCE_QSPI_H__ #define __CADENCE_QSPI_H__
+#include <reset.h> + #define CQSPI_IS_ADDR(cmd_len) (cmd_len > 1 ? 1 : 0)
#define CQSPI_NO_DECODER_MAX_CS 4 @@ -42,6 +44,8 @@ struct cadence_spi_priv { unsigned int qspi_calibrated_hz; unsigned int qspi_calibrated_cs; unsigned int previous_hz; + + struct reset_ctl_bulk resets; };
/* Functions call declaration */

On 2/21/19 10:43 PM, Simon Goldschmidt wrote:
This adds reset handling to the cadence qspi 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
Changes in v2:
- add .remove callback to release the resets
drivers/spi/cadence_qspi.c | 16 ++++++++++++++++ drivers/spi/cadence_qspi.h | 4 ++++ 2 files changed, 20 insertions(+)
diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c index 11fce9c4fe..3bfa0201c4 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,17 @@ static int cadence_spi_probe(struct udevice *bus) { struct cadence_spi_platdata *plat = bus->platdata; struct cadence_spi_priv *priv = dev_get_priv(bus);
int ret;
priv->regbase = plat->regbase; priv->ahbbase = plat->ahbbase;
ret = reset_get_bulk(bus, &priv->resets);
if (ret)
dev_warn(bus, "Can't get reset: %d\n", ret);
else
reset_deassert_bulk(&priv->resets);
if (!priv->qspi_is_init) { cadence_qspi_apb_controller_init(plat); priv->qspi_is_init = 1;
@@ -166,6 +174,13 @@ static int cadence_spi_probe(struct udevice *bus) return 0; }
+static int cadence_spi_remove(struct udevice *dev) +{
- struct cadence_spi_priv *priv = dev_get_priv(dev);
- return reset_release_bulk(&priv->resets);
+}
static int cadence_spi_set_mode(struct udevice *bus, uint mode) { struct cadence_spi_priv *priv = dev_get_priv(bus); @@ -342,4 +357,5 @@ U_BOOT_DRIVER(cadence_spi) = { .platdata_auto_alloc_size = sizeof(struct cadence_spi_platdata), .priv_auto_alloc_size = sizeof(struct cadence_spi_priv), .probe = cadence_spi_probe,
- .remove = cadence_spi_remove,
.remove() only ever gets executed for drivers setting DM_FLAG_OS_PREPARE flag. Fix this in the other drivers too.

Am Do., 21. Feb. 2019, 22:56 hat Marek Vasut marex@denx.de geschrieben:
On 2/21/19 10:43 PM, Simon Goldschmidt wrote:
This adds reset handling to the cadence qspi 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
Changes in v2:
- add .remove callback to release the resets
drivers/spi/cadence_qspi.c | 16 ++++++++++++++++ drivers/spi/cadence_qspi.h | 4 ++++ 2 files changed, 20 insertions(+)
diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c index 11fce9c4fe..3bfa0201c4 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,17 @@ static int cadence_spi_probe(struct udevice *bus) { struct cadence_spi_platdata *plat = bus->platdata; struct cadence_spi_priv *priv = dev_get_priv(bus);
int ret; priv->regbase = plat->regbase; priv->ahbbase = plat->ahbbase;
ret = reset_get_bulk(bus, &priv->resets);
if (ret)
dev_warn(bus, "Can't get reset: %d\n", ret);
else
reset_deassert_bulk(&priv->resets);
if (!priv->qspi_is_init) { cadence_qspi_apb_controller_init(plat); priv->qspi_is_init = 1;
@@ -166,6 +174,13 @@ static int cadence_spi_probe(struct udevice *bus) return 0; }
+static int cadence_spi_remove(struct udevice *dev) +{
struct cadence_spi_priv *priv = dev_get_priv(dev);
return reset_release_bulk(&priv->resets);
+}
static int cadence_spi_set_mode(struct udevice *bus, uint mode) { struct cadence_spi_priv *priv = dev_get_priv(bus); @@ -342,4 +357,5 @@ U_BOOT_DRIVER(cadence_spi) = { .platdata_auto_alloc_size = sizeof(struct cadence_spi_platdata), .priv_auto_alloc_size = sizeof(struct cadence_spi_priv), .probe = cadence_spi_probe,
.remove = cadence_spi_remove,
.remove() only ever gets executed for drivers setting DM_FLAG_OS_PREPARE flag. Fix this in the other drivers too.
Ehrm, I haven't checked, but is this common practice? Why doesn't it always get called?
Regards, Simon

On 2/21/19 11:04 PM, Simon Goldschmidt wrote:
Am Do., 21. Feb. 2019, 22:56 hat Marek Vasut <marex@denx.de mailto:marex@denx.de> geschrieben:
On 2/21/19 10:43 PM, Simon Goldschmidt wrote: > This adds reset handling to the cadence qspi 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 <mailto:simon.k.r.goldschmidt@gmail.com>> > --- > > Changes in v2: > - add .remove callback to release the resets > > drivers/spi/cadence_qspi.c | 16 ++++++++++++++++ > drivers/spi/cadence_qspi.h | 4 ++++ > 2 files changed, 20 insertions(+) > > diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c > index 11fce9c4fe..3bfa0201c4 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,17 @@ static int cadence_spi_probe(struct udevice *bus) > { > struct cadence_spi_platdata *plat = bus->platdata; > struct cadence_spi_priv *priv = dev_get_priv(bus); > + int ret; > > priv->regbase = plat->regbase; > priv->ahbbase = plat->ahbbase; > > + ret = reset_get_bulk(bus, &priv->resets); > + if (ret) > + dev_warn(bus, "Can't get reset: %d\n", ret); > + else > + reset_deassert_bulk(&priv->resets); > + > if (!priv->qspi_is_init) { > cadence_qspi_apb_controller_init(plat); > priv->qspi_is_init = 1; > @@ -166,6 +174,13 @@ static int cadence_spi_probe(struct udevice *bus) > return 0; > } > > +static int cadence_spi_remove(struct udevice *dev) > +{ > + struct cadence_spi_priv *priv = dev_get_priv(dev); > + > + return reset_release_bulk(&priv->resets); > +} > + > static int cadence_spi_set_mode(struct udevice *bus, uint mode) > { > struct cadence_spi_priv *priv = dev_get_priv(bus); > @@ -342,4 +357,5 @@ U_BOOT_DRIVER(cadence_spi) = { > .platdata_auto_alloc_size = sizeof(struct cadence_spi_platdata), > .priv_auto_alloc_size = sizeof(struct cadence_spi_priv), > .probe = cadence_spi_probe, > + .remove = cadence_spi_remove, .remove() only ever gets executed for drivers setting DM_FLAG_OS_PREPARE flag. Fix this in the other drivers too.
Ehrm, I haven't checked, but is this common practice? Why doesn't it always get called?
That's how the code behaves. Probably to speed up booting the kernel on devices which don't need to be torn down.

On Thu, Feb 21, 2019 at 11:14 PM Marek Vasut marex@denx.de wrote:
On 2/21/19 11:04 PM, Simon Goldschmidt wrote:
Am Do., 21. Feb. 2019, 22:56 hat Marek Vasut <marex@denx.de mailto:marex@denx.de> geschrieben:
On 2/21/19 10:43 PM, Simon Goldschmidt wrote: > This adds reset handling to the cadence qspi 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 <mailto:simon.k.r.goldschmidt@gmail.com>> > --- > > Changes in v2: > - add .remove callback to release the resets > > drivers/spi/cadence_qspi.c | 16 ++++++++++++++++ > drivers/spi/cadence_qspi.h | 4 ++++ > 2 files changed, 20 insertions(+) > > diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c > index 11fce9c4fe..3bfa0201c4 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,17 @@ static int cadence_spi_probe(struct udevice *bus) > { > struct cadence_spi_platdata *plat = bus->platdata; > struct cadence_spi_priv *priv = dev_get_priv(bus); > + int ret; > > priv->regbase = plat->regbase; > priv->ahbbase = plat->ahbbase; > > + ret = reset_get_bulk(bus, &priv->resets); > + if (ret) > + dev_warn(bus, "Can't get reset: %d\n", ret); > + else > + reset_deassert_bulk(&priv->resets); > + > if (!priv->qspi_is_init) { > cadence_qspi_apb_controller_init(plat); > priv->qspi_is_init = 1; > @@ -166,6 +174,13 @@ static int cadence_spi_probe(struct udevice *bus) > return 0; > } > > +static int cadence_spi_remove(struct udevice *dev) > +{ > + struct cadence_spi_priv *priv = dev_get_priv(dev); > + > + return reset_release_bulk(&priv->resets); > +} > + > static int cadence_spi_set_mode(struct udevice *bus, uint mode) > { > struct cadence_spi_priv *priv = dev_get_priv(bus); > @@ -342,4 +357,5 @@ U_BOOT_DRIVER(cadence_spi) = { > .platdata_auto_alloc_size = sizeof(struct cadence_spi_platdata), > .priv_auto_alloc_size = sizeof(struct cadence_spi_priv), > .probe = cadence_spi_probe, > + .remove = cadence_spi_remove, .remove() only ever gets executed for drivers setting DM_FLAG_OS_PREPARE flag. Fix this in the other drivers too.
Ehrm, I haven't checked, but is this common practice? Why doesn't it always get called?
That's how the code behaves. Probably to speed up booting the kernel on devices which don't need to be torn down.
What surprises me is that the OS_PREPARE flag is used only for one spi driver and for 'mmc_blk' (but this is really new). Is it still the right thing to do? How could this be one of the first drivers releasing its reset before boot? :-)
Regards, Simon

On 2/22/19 7:06 AM, Simon Goldschmidt wrote:
On Thu, Feb 21, 2019 at 11:14 PM Marek Vasut marex@denx.de wrote:
On 2/21/19 11:04 PM, Simon Goldschmidt wrote:
Am Do., 21. Feb. 2019, 22:56 hat Marek Vasut <marex@denx.de mailto:marex@denx.de> geschrieben:
On 2/21/19 10:43 PM, Simon Goldschmidt wrote: > This adds reset handling to the cadence qspi 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 <mailto:simon.k.r.goldschmidt@gmail.com>> > --- > > Changes in v2: > - add .remove callback to release the resets > > drivers/spi/cadence_qspi.c | 16 ++++++++++++++++ > drivers/spi/cadence_qspi.h | 4 ++++ > 2 files changed, 20 insertions(+) > > diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c > index 11fce9c4fe..3bfa0201c4 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,17 @@ static int cadence_spi_probe(struct udevice *bus) > { > struct cadence_spi_platdata *plat = bus->platdata; > struct cadence_spi_priv *priv = dev_get_priv(bus); > + int ret; > > priv->regbase = plat->regbase; > priv->ahbbase = plat->ahbbase; > > + ret = reset_get_bulk(bus, &priv->resets); > + if (ret) > + dev_warn(bus, "Can't get reset: %d\n", ret); > + else > + reset_deassert_bulk(&priv->resets); > + > if (!priv->qspi_is_init) { > cadence_qspi_apb_controller_init(plat); > priv->qspi_is_init = 1; > @@ -166,6 +174,13 @@ static int cadence_spi_probe(struct udevice *bus) > return 0; > } > > +static int cadence_spi_remove(struct udevice *dev) > +{ > + struct cadence_spi_priv *priv = dev_get_priv(dev); > + > + return reset_release_bulk(&priv->resets); > +} > + > static int cadence_spi_set_mode(struct udevice *bus, uint mode) > { > struct cadence_spi_priv *priv = dev_get_priv(bus); > @@ -342,4 +357,5 @@ U_BOOT_DRIVER(cadence_spi) = { > .platdata_auto_alloc_size = sizeof(struct cadence_spi_platdata), > .priv_auto_alloc_size = sizeof(struct cadence_spi_priv), > .probe = cadence_spi_probe, > + .remove = cadence_spi_remove, .remove() only ever gets executed for drivers setting DM_FLAG_OS_PREPARE flag. Fix this in the other drivers too.
Ehrm, I haven't checked, but is this common practice? Why doesn't it always get called?
That's how the code behaves. Probably to speed up booting the kernel on devices which don't need to be torn down.
What surprises me is that the OS_PREPARE flag is used only for one spi driver and for 'mmc_blk' (but this is really new). Is it still the right thing to do? How could this be one of the first drivers releasing its reset before boot? :-)
I suspect this might be an area for improvement :)

This adds code to take peripherals out of reset based on an environment variable. This is in preparation for removing the code that does this from SPL.
However, some drivers even in current Linux cannot handle peripheral reset, so until this works, we need a compatibility workaround.
This workaround is implemented in the 'remove' callback of this reset driver, which is called on OS_PREPARE: it checks if the environment variable "socfpga_permodrst_ungate" - if it is set to 1, it deasserts all peripheral resets, which is what the gen5 SPL did up to now.
This is in preparation to clean up the SPL and implementing proper reset handling for U-Boot.
Signed-off-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com ---
Changes in v2: - moved from Kernel option "OLD_SOCFPGA_KERNEL_COMPAT" to environment variable "socfpga_permodrst_ungate"
drivers/reset/reset-socfpga.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+)
diff --git a/drivers/reset/reset-socfpga.c b/drivers/reset/reset-socfpga.c index b2acfcd2ec..749f671e05 100644 --- a/drivers/reset/reset-socfpga.c +++ b/drivers/reset/reset-socfpga.c @@ -89,6 +89,36 @@ static int socfpga_reset_probe(struct udevice *dev) return 0; }
+#ifndef CONFIG_SPL_BUILD +/* + * This remove function is called before starting OS to deassert peripheral + * resets for Kernels that don't support this. + * + * The reset driver checks the environment variable "socfpga_permodrst_ungate" + * when it is removed. If this variable is '1', all peripherals in 'permodrst' + * are taken out of reset before booting into the OS. + * This should be required for gen5 systems only that are running Linux kernels + * without proper peripheral reset support for all drivers used. + */ +static int socfpga_reset_remove(struct udevice *dev) +{ + struct socfpga_reset_data *data = dev_get_priv(dev); + const char *env_str; + long val; + + env_str = env_get("socfpga_permodrst_ungate"); + if (env_str) { + val = simple_strtol(env_str, NULL, 0); + if (val == 1) { + puts("Deasserting all peripheral resets\n"); + writel(0, data->membase + 4); + } + } + + return 0; +} +#endif + static const struct udevice_id socfpga_reset_match[] = { { .compatible = "altr,rst-mgr" }, { /* sentinel */ }, @@ -101,4 +131,8 @@ U_BOOT_DRIVER(socfpga_reset) = { .probe = socfpga_reset_probe, .priv_auto_alloc_size = sizeof(struct socfpga_reset_data), .ops = &socfpga_reset_ops, +#ifndef CONFIG_SPL_BUILD + .remove = socfpga_reset_remove, + .flags = DM_FLAG_OS_PREPARE, +#endif };

On 2/21/19 10:43 PM, Simon Goldschmidt wrote:
This adds code to take peripherals out of reset based on an environment variable. This is in preparation for removing the code that does this from SPL.
However, some drivers even in current Linux cannot handle peripheral reset, so until this works, we need a compatibility workaround.
This workaround is implemented in the 'remove' callback of this reset driver, which is called on OS_PREPARE: it checks if the environment variable "socfpga_permodrst_ungate" - if it is set to 1, it deasserts all peripheral resets, which is what the gen5 SPL did up to now.
This is in preparation to clean up the SPL and implementing proper reset handling for U-Boot.
Signed-off-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com
Changes in v2:
- moved from Kernel option "OLD_SOCFPGA_KERNEL_COMPAT" to environment variable "socfpga_permodrst_ungate"
drivers/reset/reset-socfpga.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+)
diff --git a/drivers/reset/reset-socfpga.c b/drivers/reset/reset-socfpga.c index b2acfcd2ec..749f671e05 100644 --- a/drivers/reset/reset-socfpga.c +++ b/drivers/reset/reset-socfpga.c @@ -89,6 +89,36 @@ static int socfpga_reset_probe(struct udevice *dev) return 0; }
+#ifndef CONFIG_SPL_BUILD +/*
- This remove function is called before starting OS to deassert peripheral
- resets for Kernels that don't support this.
- The reset driver checks the environment variable "socfpga_permodrst_ungate"
- when it is removed. If this variable is '1', all peripherals in 'permodrst'
- are taken out of reset before booting into the OS.
- This should be required for gen5 systems only that are running Linux kernels
- without proper peripheral reset support for all drivers used.
- */
+static int socfpga_reset_remove(struct udevice *dev) +{
- struct socfpga_reset_data *data = dev_get_priv(dev);
- const char *env_str;
- long val;
- env_str = env_get("socfpga_permodrst_ungate");
- if (env_str) {
val = simple_strtol(env_str, NULL, 0);
if (val == 1) {
puts("Deasserting all peripheral resets\n");
writel(0, data->membase + 4);
}
- }
- return 0;
+} +#endif
static const struct udevice_id socfpga_reset_match[] = { { .compatible = "altr,rst-mgr" }, { /* sentinel */ }, @@ -101,4 +131,8 @@ U_BOOT_DRIVER(socfpga_reset) = { .probe = socfpga_reset_probe, .priv_auto_alloc_size = sizeof(struct socfpga_reset_data), .ops = &socfpga_reset_ops, +#ifndef CONFIG_SPL_BUILD
- .remove = socfpga_reset_remove,
- .flags = DM_FLAG_OS_PREPARE,
+#endif };
Does this work if you use Falcon mode ? :)

Am Do., 21. Feb. 2019, 22:56 hat Marek Vasut marex@denx.de geschrieben:
On 2/21/19 10:43 PM, Simon Goldschmidt wrote:
This adds code to take peripherals out of reset based on an environment variable. This is in preparation for removing the code that does this
from
SPL.
However, some drivers even in current Linux cannot handle peripheral
reset,
so until this works, we need a compatibility workaround.
This workaround is implemented in the 'remove' callback of this reset driver, which is called on OS_PREPARE: it checks if the environment variable "socfpga_permodrst_ungate" - if it is set to 1, it deasserts all peripheral resets, which is what the gen5 SPL did up to now.
This is in preparation to clean up the SPL and implementing proper reset handling for U-Boot.
Signed-off-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com
Changes in v2:
- moved from Kernel option "OLD_SOCFPGA_KERNEL_COMPAT" to environment variable "socfpga_permodrst_ungate"
drivers/reset/reset-socfpga.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+)
diff --git a/drivers/reset/reset-socfpga.c
b/drivers/reset/reset-socfpga.c
index b2acfcd2ec..749f671e05 100644 --- a/drivers/reset/reset-socfpga.c +++ b/drivers/reset/reset-socfpga.c @@ -89,6 +89,36 @@ static int socfpga_reset_probe(struct udevice *dev) return 0; }
+#ifndef CONFIG_SPL_BUILD +/*
- This remove function is called before starting OS to deassert
peripheral
- resets for Kernels that don't support this.
- The reset driver checks the environment variable
"socfpga_permodrst_ungate"
- when it is removed. If this variable is '1', all peripherals in
'permodrst'
- are taken out of reset before booting into the OS.
- This should be required for gen5 systems only that are running Linux
kernels
- without proper peripheral reset support for all drivers used.
- */
+static int socfpga_reset_remove(struct udevice *dev) +{
struct socfpga_reset_data *data = dev_get_priv(dev);
const char *env_str;
long val;
env_str = env_get("socfpga_permodrst_ungate");
if (env_str) {
val = simple_strtol(env_str, NULL, 0);
if (val == 1) {
puts("Deasserting all peripheral resets\n");
writel(0, data->membase + 4);
}
}
return 0;
+} +#endif
static const struct udevice_id socfpga_reset_match[] = { { .compatible = "altr,rst-mgr" }, { /* sentinel */ }, @@ -101,4 +131,8 @@ U_BOOT_DRIVER(socfpga_reset) = { .probe = socfpga_reset_probe, .priv_auto_alloc_size = sizeof(struct socfpga_reset_data), .ops = &socfpga_reset_ops, +#ifndef CONFIG_SPL_BUILD
.remove = socfpga_reset_remove,
.flags = DM_FLAG_OS_PREPARE,
+#endif };
Does this work if you use Falcon mode ? :)
Probably not, but would anyone be able to use falcon mode in socfpga gen5 given the current SPL size? I think it didn't fit for,a last time I checked.
And if it fits in the future, we probably don't need this workaraound any more...
I did it like that since env_get() broke compilation of SPL. But if you insist of making this work even for SPL, I'll check again.
Regards, Simon

On 2/21/19 11:07 PM, Simon Goldschmidt wrote:
Am Do., 21. Feb. 2019, 22:56 hat Marek Vasut <marex@denx.de mailto:marex@denx.de> geschrieben:
On 2/21/19 10:43 PM, Simon Goldschmidt wrote: > This adds code to take peripherals out of reset based on an environment > variable. This is in preparation for removing the code that does this from > SPL. > > However, some drivers even in current Linux cannot handle peripheral reset, > so until this works, we need a compatibility workaround. > > This workaround is implemented in the 'remove' callback of this reset > driver, which is called on OS_PREPARE: it checks if the environment > variable "socfpga_permodrst_ungate" - if it is set to 1, it deasserts all > peripheral resets, which is what the gen5 SPL did up to now. > > This is in preparation to clean up the SPL and implementing proper reset > handling for U-Boot. > > Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com <mailto:simon.k.r.goldschmidt@gmail.com>> > --- > > Changes in v2: > - moved from Kernel option "OLD_SOCFPGA_KERNEL_COMPAT" to environment > variable "socfpga_permodrst_ungate" > > drivers/reset/reset-socfpga.c | 34 ++++++++++++++++++++++++++++++++++ > 1 file changed, 34 insertions(+) > > diff --git a/drivers/reset/reset-socfpga.c b/drivers/reset/reset-socfpga.c > index b2acfcd2ec..749f671e05 100644 > --- a/drivers/reset/reset-socfpga.c > +++ b/drivers/reset/reset-socfpga.c > @@ -89,6 +89,36 @@ static int socfpga_reset_probe(struct udevice *dev) > return 0; > } > > +#ifndef CONFIG_SPL_BUILD > +/* > + * This remove function is called before starting OS to deassert peripheral > + * resets for Kernels that don't support this. > + * > + * The reset driver checks the environment variable "socfpga_permodrst_ungate" > + * when it is removed. If this variable is '1', all peripherals in 'permodrst' > + * are taken out of reset before booting into the OS. > + * This should be required for gen5 systems only that are running Linux kernels > + * without proper peripheral reset support for all drivers used. > + */ > +static int socfpga_reset_remove(struct udevice *dev) > +{ > + struct socfpga_reset_data *data = dev_get_priv(dev); > + const char *env_str; > + long val; > + > + env_str = env_get("socfpga_permodrst_ungate"); > + if (env_str) { > + val = simple_strtol(env_str, NULL, 0); > + if (val == 1) { > + puts("Deasserting all peripheral resets\n"); > + writel(0, data->membase + 4); > + } > + } > + > + return 0; > +} > +#endif > + > static const struct udevice_id socfpga_reset_match[] = { > { .compatible = "altr,rst-mgr" }, > { /* sentinel */ }, > @@ -101,4 +131,8 @@ U_BOOT_DRIVER(socfpga_reset) = { > .probe = socfpga_reset_probe, > .priv_auto_alloc_size = sizeof(struct socfpga_reset_data), > .ops = &socfpga_reset_ops, > +#ifndef CONFIG_SPL_BUILD > + .remove = socfpga_reset_remove, > + .flags = DM_FLAG_OS_PREPARE, > +#endif > }; Does this work if you use Falcon mode ? :)
Probably not, but would anyone be able to use falcon mode in socfpga gen5 given the current SPL size? I think it didn't fit for,a last time I checked.
If you cut it down, why not ?
And if it fits in the future, we probably don't need this workaraound any more...
I did it like that since env_get() broke compilation of SPL. But if you insist of making this work even for SPL, I'll check again.
SPL can also check env, so some more conditionals need to be added. Should be a matter of some CONFIG_IS_ENABLED()

On Thu, Feb 21, 2019 at 11:15 PM Marek Vasut marex@denx.de wrote:
On 2/21/19 11:07 PM, Simon Goldschmidt wrote:
Am Do., 21. Feb. 2019, 22:56 hat Marek Vasut <marex@denx.de mailto:marex@denx.de> geschrieben:
On 2/21/19 10:43 PM, Simon Goldschmidt wrote: > This adds code to take peripherals out of reset based on an environment > variable. This is in preparation for removing the code that does this from > SPL. > > However, some drivers even in current Linux cannot handle peripheral reset, > so until this works, we need a compatibility workaround. > > This workaround is implemented in the 'remove' callback of this reset > driver, which is called on OS_PREPARE: it checks if the environment > variable "socfpga_permodrst_ungate" - if it is set to 1, it deasserts all > peripheral resets, which is what the gen5 SPL did up to now. > > This is in preparation to clean up the SPL and implementing proper reset > handling for U-Boot. > > Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com <mailto:simon.k.r.goldschmidt@gmail.com>> > --- > > Changes in v2: > - moved from Kernel option "OLD_SOCFPGA_KERNEL_COMPAT" to environment > variable "socfpga_permodrst_ungate" > > drivers/reset/reset-socfpga.c | 34 ++++++++++++++++++++++++++++++++++ > 1 file changed, 34 insertions(+) > > diff --git a/drivers/reset/reset-socfpga.c b/drivers/reset/reset-socfpga.c > index b2acfcd2ec..749f671e05 100644 > --- a/drivers/reset/reset-socfpga.c > +++ b/drivers/reset/reset-socfpga.c > @@ -89,6 +89,36 @@ static int socfpga_reset_probe(struct udevice *dev) > return 0; > } > > +#ifndef CONFIG_SPL_BUILD > +/* > + * This remove function is called before starting OS to deassert peripheral > + * resets for Kernels that don't support this. > + * > + * The reset driver checks the environment variable "socfpga_permodrst_ungate" > + * when it is removed. If this variable is '1', all peripherals in 'permodrst' > + * are taken out of reset before booting into the OS. > + * This should be required for gen5 systems only that are running Linux kernels > + * without proper peripheral reset support for all drivers used. > + */ > +static int socfpga_reset_remove(struct udevice *dev) > +{ > + struct socfpga_reset_data *data = dev_get_priv(dev); > + const char *env_str; > + long val; > + > + env_str = env_get("socfpga_permodrst_ungate"); > + if (env_str) { > + val = simple_strtol(env_str, NULL, 0); > + if (val == 1) { > + puts("Deasserting all peripheral resets\n"); > + writel(0, data->membase + 4); > + } > + } > + > + return 0; > +} > +#endif > + > static const struct udevice_id socfpga_reset_match[] = { > { .compatible = "altr,rst-mgr" }, > { /* sentinel */ }, > @@ -101,4 +131,8 @@ U_BOOT_DRIVER(socfpga_reset) = { > .probe = socfpga_reset_probe, > .priv_auto_alloc_size = sizeof(struct socfpga_reset_data), > .ops = &socfpga_reset_ops, > +#ifndef CONFIG_SPL_BUILD > + .remove = socfpga_reset_remove, > + .flags = DM_FLAG_OS_PREPARE, > +#endif > }; Does this work if you use Falcon mode ? :)
Probably not, but would anyone be able to use falcon mode in socfpga gen5 given the current SPL size? I think it didn't fit for,a last time I checked.
If you cut it down, why not ?
And if it fits in the future, we probably don't need this workaraound any more...
I did it like that since env_get() broke compilation of SPL. But if you insist of making this work even for SPL, I'll check again.
SPL can also check env, so some more conditionals need to be added. Should be a matter of some CONFIG_IS_ENABLED()
OK, I'll try that for V3.
Regards, Simon

To keep the current behaviour of taking all peripherals out of reset before booting the OS before removing that code from socfpga gen5 SPL, this enables the new behaviour by default for all gen5 boards by adding the environment variable "socfpga_permodrst_ungate=1" to the default environment.
This can be overridden in board config files or by saving an environment without this variable enabled.
Signed-off-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com ---
Changes in v2: - this patch is new in v2
include/configs/socfpga_common.h | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/include/configs/socfpga_common.h b/include/configs/socfpga_common.h index c9cbf8f5e3..2510c6fd7b 100644 --- a/include/configs/socfpga_common.h +++ b/include/configs/socfpga_common.h @@ -321,6 +321,19 @@ unsigned int cm_get_qspi_controller_clk_hz(void);
#include <config_distro_bootcmd.h>
+#ifdef CONFIG_TARGET_SOCFPGA_GEN5 +/* + * Handle compatibility for peripheral reset for Linux kernels that haven't + * implemented peripheral reset for all drivers. + * Define this to "" disable this compatibility. + */ +#ifndef SOCFPGA_PERMODRST_UNGATE +#define SOCFPGA_PERMODRST_UNGATE "socfpga_permodrst_ungate=1\0" +#endif +#else +#define SOCFPGA_PERMODRST_UNGATE "" +#endif + #ifndef CONFIG_EXTRA_ENV_SETTINGS #define CONFIG_EXTRA_ENV_SETTINGS \ "fdtfile=" CONFIG_DEFAULT_FDT_FILE "\0" \ @@ -330,6 +343,7 @@ unsigned int cm_get_qspi_controller_clk_hz(void); "scriptaddr=0x02100000\0" \ "pxefile_addr_r=0x02200000\0" \ "ramdisk_addr_r=0x02300000\0" \ + SOCFPGA_PERMODRST_UNGATE \ BOOTENV
#endif

On 2/21/19 10:43 PM, Simon Goldschmidt wrote:
To keep the current behaviour of taking all peripherals out of reset before booting the OS before removing that code from socfpga gen5 SPL, this enables the new behaviour by default for all gen5 boards by adding the environment variable "socfpga_permodrst_ungate=1" to the default environment.
This can be overridden in board config files or by saving an environment without this variable enabled.
Signed-off-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com
Changes in v2:
- this patch is new in v2
include/configs/socfpga_common.h | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/include/configs/socfpga_common.h b/include/configs/socfpga_common.h index c9cbf8f5e3..2510c6fd7b 100644 --- a/include/configs/socfpga_common.h +++ b/include/configs/socfpga_common.h @@ -321,6 +321,19 @@ unsigned int cm_get_qspi_controller_clk_hz(void);
#include <config_distro_bootcmd.h>
+#ifdef CONFIG_TARGET_SOCFPGA_GEN5 +/*
- Handle compatibility for peripheral reset for Linux kernels that haven't
- implemented peripheral reset for all drivers.
- Define this to "" disable this compatibility.
- */
+#ifndef SOCFPGA_PERMODRST_UNGATE +#define SOCFPGA_PERMODRST_UNGATE "socfpga_permodrst_ungate=1\0" +#endif +#else +#define SOCFPGA_PERMODRST_UNGATE "" +#endif
Just add this socfpga_permodrst_ungate to the default end and drop all those macros/Kconfig options.
I think it'd be better to call it socfpga_legacy_reset_compat or something.
#ifndef CONFIG_EXTRA_ENV_SETTINGS #define CONFIG_EXTRA_ENV_SETTINGS \ "fdtfile=" CONFIG_DEFAULT_FDT_FILE "\0" \ @@ -330,6 +343,7 @@ unsigned int cm_get_qspi_controller_clk_hz(void); "scriptaddr=0x02100000\0" \ "pxefile_addr_r=0x02200000\0" \ "ramdisk_addr_r=0x02300000\0" \
- SOCFPGA_PERMODRST_UNGATE \ BOOTENV
#endif

Am Do., 21. Feb. 2019, 22:56 hat Marek Vasut marex@denx.de geschrieben:
On 2/21/19 10:43 PM, Simon Goldschmidt wrote:
To keep the current behaviour of taking all peripherals out of reset before booting the OS before removing that code from socfpga gen5 SPL, this enables the new behaviour by default for all gen5 boards by adding the environment variable "socfpga_permodrst_ungate=1" to the default environment.
This can be overridden in board config files or by saving an environment without this variable enabled.
Signed-off-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com
Changes in v2:
- this patch is new in v2
include/configs/socfpga_common.h | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/include/configs/socfpga_common.h
b/include/configs/socfpga_common.h
index c9cbf8f5e3..2510c6fd7b 100644 --- a/include/configs/socfpga_common.h +++ b/include/configs/socfpga_common.h @@ -321,6 +321,19 @@ unsigned int cm_get_qspi_controller_clk_hz(void);
#include <config_distro_bootcmd.h>
+#ifdef CONFIG_TARGET_SOCFPGA_GEN5 +/*
- Handle compatibility for peripheral reset for Linux kernels that
haven't
- implemented peripheral reset for all drivers.
- Define this to "" disable this compatibility.
- */
+#ifndef SOCFPGA_PERMODRST_UNGATE +#define SOCFPGA_PERMODRST_UNGATE "socfpga_permodrst_ungate=1\0" +#endif +#else +#define SOCFPGA_PERMODRST_UNGATE "" +#endif
Just add this socfpga_permodrst_ungate to the default end and drop all those macros/Kconfig options.
But how would it then be overridden?
I think it'd be better to call it socfpga_legacy_reset_compat or something.
Ok.
Regards, Simon
#ifndef CONFIG_EXTRA_ENV_SETTINGS #define CONFIG_EXTRA_ENV_SETTINGS \ "fdtfile=" CONFIG_DEFAULT_FDT_FILE "\0" \ @@ -330,6 +343,7 @@ unsigned int cm_get_qspi_controller_clk_hz(void); "scriptaddr=0x02100000\0" \ "pxefile_addr_r=0x02200000\0" \ "ramdisk_addr_r=0x02300000\0" \
SOCFPGA_PERMODRST_UNGATE \ BOOTENV
#endif

On 2/21/19 11:09 PM, Simon Goldschmidt wrote:
Am Do., 21. Feb. 2019, 22:56 hat Marek Vasut <marex@denx.de mailto:marex@denx.de> geschrieben:
On 2/21/19 10:43 PM, Simon Goldschmidt wrote: > To keep the current behaviour of taking all peripherals out of reset > before booting the OS before removing that code from socfpga gen5 SPL, > this enables the new behaviour by default for all gen5 boards by adding > the environment variable "socfpga_permodrst_ungate=1" to the default > environment. > > This can be overridden in board config files or by saving an environment > without this variable enabled. > > Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com <mailto:simon.k.r.goldschmidt@gmail.com>> > --- > > Changes in v2: > - this patch is new in v2 > > include/configs/socfpga_common.h | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/include/configs/socfpga_common.h b/include/configs/socfpga_common.h > index c9cbf8f5e3..2510c6fd7b 100644 > --- a/include/configs/socfpga_common.h > +++ b/include/configs/socfpga_common.h > @@ -321,6 +321,19 @@ unsigned int cm_get_qspi_controller_clk_hz(void); > > #include <config_distro_bootcmd.h> > > +#ifdef CONFIG_TARGET_SOCFPGA_GEN5 > +/* > + * Handle compatibility for peripheral reset for Linux kernels that haven't > + * implemented peripheral reset for all drivers. > + * Define this to "" disable this compatibility. > + */ > +#ifndef SOCFPGA_PERMODRST_UNGATE > +#define SOCFPGA_PERMODRST_UNGATE "socfpga_permodrst_ungate=1\0" > +#endif > +#else > +#define SOCFPGA_PERMODRST_UNGATE "" > +#endif Just add this socfpga_permodrst_ungate to the default end and drop all those macros/Kconfig options.
But how would it then be overridden?
User would setenv it to "" and saveenv ? I might be missing something obvious.

On Thu, Feb 21, 2019 at 11:17 PM Marek Vasut marex@denx.de wrote:
On 2/21/19 11:09 PM, Simon Goldschmidt wrote:
Am Do., 21. Feb. 2019, 22:56 hat Marek Vasut <marex@denx.de mailto:marex@denx.de> geschrieben:
On 2/21/19 10:43 PM, Simon Goldschmidt wrote: > To keep the current behaviour of taking all peripherals out of reset > before booting the OS before removing that code from socfpga gen5 SPL, > this enables the new behaviour by default for all gen5 boards by adding > the environment variable "socfpga_permodrst_ungate=1" to the default > environment. > > This can be overridden in board config files or by saving an environment > without this variable enabled. > > Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com <mailto:simon.k.r.goldschmidt@gmail.com>> > --- > > Changes in v2: > - this patch is new in v2 > > include/configs/socfpga_common.h | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/include/configs/socfpga_common.h b/include/configs/socfpga_common.h > index c9cbf8f5e3..2510c6fd7b 100644 > --- a/include/configs/socfpga_common.h > +++ b/include/configs/socfpga_common.h > @@ -321,6 +321,19 @@ unsigned int cm_get_qspi_controller_clk_hz(void); > > #include <config_distro_bootcmd.h> > > +#ifdef CONFIG_TARGET_SOCFPGA_GEN5 > +/* > + * Handle compatibility for peripheral reset for Linux kernels that haven't > + * implemented peripheral reset for all drivers. > + * Define this to "" disable this compatibility. > + */ > +#ifndef SOCFPGA_PERMODRST_UNGATE > +#define SOCFPGA_PERMODRST_UNGATE "socfpga_permodrst_ungate=1\0" > +#endif > +#else > +#define SOCFPGA_PERMODRST_UNGATE "" > +#endif Just add this socfpga_permodrst_ungate to the default end and drop all those macros/Kconfig options.
But how would it then be overridden?
User would setenv it to "" and saveenv ? I might be missing something obvious.
Of course. I meant how to override it for the default env. E.g. to test on some boards.
But it's probably enough that CONFIG_EXTRA_ENV_SETTINGS is overridable.
Regards, Simon

On 2/22/19 6:55 AM, Simon Goldschmidt wrote:
On Thu, Feb 21, 2019 at 11:17 PM Marek Vasut marex@denx.de wrote:
On 2/21/19 11:09 PM, Simon Goldschmidt wrote:
Am Do., 21. Feb. 2019, 22:56 hat Marek Vasut <marex@denx.de mailto:marex@denx.de> geschrieben:
On 2/21/19 10:43 PM, Simon Goldschmidt wrote: > To keep the current behaviour of taking all peripherals out of reset > before booting the OS before removing that code from socfpga gen5 SPL, > this enables the new behaviour by default for all gen5 boards by adding > the environment variable "socfpga_permodrst_ungate=1" to the default > environment. > > This can be overridden in board config files or by saving an environment > without this variable enabled. > > Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com <mailto:simon.k.r.goldschmidt@gmail.com>> > --- > > Changes in v2: > - this patch is new in v2 > > include/configs/socfpga_common.h | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/include/configs/socfpga_common.h b/include/configs/socfpga_common.h > index c9cbf8f5e3..2510c6fd7b 100644 > --- a/include/configs/socfpga_common.h > +++ b/include/configs/socfpga_common.h > @@ -321,6 +321,19 @@ unsigned int cm_get_qspi_controller_clk_hz(void); > > #include <config_distro_bootcmd.h> > > +#ifdef CONFIG_TARGET_SOCFPGA_GEN5 > +/* > + * Handle compatibility for peripheral reset for Linux kernels that haven't > + * implemented peripheral reset for all drivers. > + * Define this to "" disable this compatibility. > + */ > +#ifndef SOCFPGA_PERMODRST_UNGATE > +#define SOCFPGA_PERMODRST_UNGATE "socfpga_permodrst_ungate=1\0" > +#endif > +#else > +#define SOCFPGA_PERMODRST_UNGATE "" > +#endif Just add this socfpga_permodrst_ungate to the default end and drop all those macros/Kconfig options.
But how would it then be overridden?
User would setenv it to "" and saveenv ? I might be missing something obvious.
Of course. I meant how to override it for the default env. E.g. to test on some boards.
But it's probably enough that CONFIG_EXTRA_ENV_SETTINGS is overridable.
I think so ... or ?

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.
Signed-off-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com ---
Changes in v2: - removed Kconfig option OLD_SOCFPGA_KERNEL_COMPAT since compatibility now uses an environment variable
arch/arm/mach-socfpga/misc_gen5.c | 10 ---------- arch/arm/mach-socfpga/spl_gen5.c | 10 ---------- 2 files changed, 20 deletions(-)
diff --git a/arch/arm/mach-socfpga/misc_gen5.c b/arch/arm/mach-socfpga/misc_gen5.c index 6e11ba6cb2..9865f5b5b1 100644 --- a/arch/arm/mach-socfpga/misc_gen5.c +++ b/arch/arm/mach-socfpga/misc_gen5.c @@ -201,16 +201,6 @@ int arch_early_init_r(void) /* Add device descriptor to FPGA device table */ socfpga_fpga_add(&altera_fpga[0]);
-#ifdef CONFIG_DESIGNWARE_SPI - /* Get Designware SPI controller out of reset */ - socfpga_per_reset(SOCFPGA_RESET(SPIM0), 0); - socfpga_per_reset(SOCFPGA_RESET(SPIM1), 0); -#endif - -#ifdef CONFIG_NAND_DENALI - socfpga_per_reset(SOCFPGA_RESET(NAND), 0); -#endif - return 0; }
diff --git a/arch/arm/mach-socfpga/spl_gen5.c b/arch/arm/mach-socfpga/spl_gen5.c index 1bff8cbfcf..e1e65261eb 100644 --- a/arch/arm/mach-socfpga/spl_gen5.c +++ b/arch/arm/mach-socfpga/spl_gen5.c @@ -36,16 +36,12 @@ u32 spl_boot_device(void) return BOOT_DEVICE_RAM; case 0x2: /* NAND Flash (1.8V) */ case 0x3: /* NAND Flash (3.0V) */ - socfpga_per_reset(SOCFPGA_RESET(NAND), 0); return BOOT_DEVICE_NAND; case 0x4: /* SD/MMC External Transceiver (1.8V) */ case 0x5: /* SD/MMC Internal Transceiver (3.0V) */ - socfpga_per_reset(SOCFPGA_RESET(SDMMC), 0); - socfpga_per_reset(SOCFPGA_RESET(DMA), 0); return BOOT_DEVICE_MMC1; case 0x6: /* QSPI Flash (1.8V) */ case 0x7: /* QSPI Flash (3.0V) */ - socfpga_per_reset(SOCFPGA_RESET(QSPI), 0); return BOOT_DEVICE_SPI; default: printf("Invalid boot device (bsel=%08x)!\n", bsel); @@ -99,9 +95,7 @@ void board_init_f(ulong dummy) socfpga_bridges_reset(1); }
- socfpga_per_reset(SOCFPGA_RESET(UART0), 0); socfpga_per_reset(SOCFPGA_RESET(OSC1TIMER0), 0); - timer_init();
debug("Reconfigure Clock Manager\n"); @@ -123,10 +117,6 @@ void board_init_f(ulong dummy) sysmgr_pinmux_init(); sysmgr_config_warmrstcfgio(0);
- /* De-assert reset for peripherals and bridges based on handoff */ - reset_deassert_peripherals_handoff(); - socfpga_bridges_reset(0); - debug("Unfreezing/Thaw all I/O banks\n"); /* unfreeze / thaw all IO banks */ sys_mgr_frzctrl_thaw_req();

On 2/21/19 10:43 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.
Signed-off-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com
Changes in v2:
- removed Kconfig option OLD_SOCFPGA_KERNEL_COMPAT since compatibility now uses an environment variable
arch/arm/mach-socfpga/misc_gen5.c | 10 ---------- arch/arm/mach-socfpga/spl_gen5.c | 10 ---------- 2 files changed, 20 deletions(-)
diff --git a/arch/arm/mach-socfpga/misc_gen5.c b/arch/arm/mach-socfpga/misc_gen5.c index 6e11ba6cb2..9865f5b5b1 100644 --- a/arch/arm/mach-socfpga/misc_gen5.c +++ b/arch/arm/mach-socfpga/misc_gen5.c @@ -201,16 +201,6 @@ int arch_early_init_r(void) /* Add device descriptor to FPGA device table */ socfpga_fpga_add(&altera_fpga[0]);
-#ifdef CONFIG_DESIGNWARE_SPI
- /* Get Designware SPI controller out of reset */
- socfpga_per_reset(SOCFPGA_RESET(SPIM0), 0);
- socfpga_per_reset(SOCFPGA_RESET(SPIM1), 0);
-#endif
-#ifdef CONFIG_NAND_DENALI
- socfpga_per_reset(SOCFPGA_RESET(NAND), 0);
-#endif
- return 0;
}
diff --git a/arch/arm/mach-socfpga/spl_gen5.c b/arch/arm/mach-socfpga/spl_gen5.c index 1bff8cbfcf..e1e65261eb 100644 --- a/arch/arm/mach-socfpga/spl_gen5.c +++ b/arch/arm/mach-socfpga/spl_gen5.c @@ -36,16 +36,12 @@ u32 spl_boot_device(void) return BOOT_DEVICE_RAM; case 0x2: /* NAND Flash (1.8V) */ case 0x3: /* NAND Flash (3.0V) */
return BOOT_DEVICE_NAND; case 0x4: /* SD/MMC External Transceiver (1.8V) */ case 0x5: /* SD/MMC Internal Transceiver (3.0V) */socfpga_per_reset(SOCFPGA_RESET(NAND), 0);
socfpga_per_reset(SOCFPGA_RESET(SDMMC), 0);
return BOOT_DEVICE_MMC1; case 0x6: /* QSPI Flash (1.8V) */ case 0x7: /* QSPI Flash (3.0V) */socfpga_per_reset(SOCFPGA_RESET(DMA), 0);
return BOOT_DEVICE_SPI; default: printf("Invalid boot device (bsel=%08x)!\n", bsel);socfpga_per_reset(SOCFPGA_RESET(QSPI), 0);
@@ -99,9 +95,7 @@ void board_init_f(ulong dummy) socfpga_bridges_reset(1); }
socfpga_per_reset(SOCFPGA_RESET(UART0), 0); socfpga_per_reset(SOCFPGA_RESET(OSC1TIMER0), 0);
timer_init();
debug("Reconfigure Clock Manager\n");
@@ -123,10 +117,6 @@ void board_init_f(ulong dummy) sysmgr_pinmux_init(); sysmgr_config_warmrstcfgio(0);
- /* De-assert reset for peripherals and bridges based on handoff */
- reset_deassert_peripherals_handoff();
- socfpga_bridges_reset(0);
- debug("Unfreezing/Thaw all I/O banks\n"); /* unfreeze / thaw all IO banks */ sys_mgr_frzctrl_thaw_req();
Nice!

On Thu, Feb 21, 2019 at 10:56 PM Marek Vasut marex@denx.de wrote:
On 2/21/19 10:43 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.
Signed-off-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com
Changes in v2:
- removed Kconfig option OLD_SOCFPGA_KERNEL_COMPAT since compatibility now uses an environment variable
arch/arm/mach-socfpga/misc_gen5.c | 10 ---------- arch/arm/mach-socfpga/spl_gen5.c | 10 ---------- 2 files changed, 20 deletions(-)
diff --git a/arch/arm/mach-socfpga/misc_gen5.c b/arch/arm/mach-socfpga/misc_gen5.c index 6e11ba6cb2..9865f5b5b1 100644 --- a/arch/arm/mach-socfpga/misc_gen5.c +++ b/arch/arm/mach-socfpga/misc_gen5.c @@ -201,16 +201,6 @@ int arch_early_init_r(void) /* Add device descriptor to FPGA device table */ socfpga_fpga_add(&altera_fpga[0]);
-#ifdef CONFIG_DESIGNWARE_SPI
/* Get Designware SPI controller out of reset */
socfpga_per_reset(SOCFPGA_RESET(SPIM0), 0);
socfpga_per_reset(SOCFPGA_RESET(SPIM1), 0);
-#endif
-#ifdef CONFIG_NAND_DENALI
socfpga_per_reset(SOCFPGA_RESET(NAND), 0);
-#endif
return 0;
}
diff --git a/arch/arm/mach-socfpga/spl_gen5.c b/arch/arm/mach-socfpga/spl_gen5.c index 1bff8cbfcf..e1e65261eb 100644 --- a/arch/arm/mach-socfpga/spl_gen5.c +++ b/arch/arm/mach-socfpga/spl_gen5.c @@ -36,16 +36,12 @@ u32 spl_boot_device(void) return BOOT_DEVICE_RAM; case 0x2: /* NAND Flash (1.8V) */ case 0x3: /* NAND Flash (3.0V) */
socfpga_per_reset(SOCFPGA_RESET(NAND), 0); return BOOT_DEVICE_NAND; case 0x4: /* SD/MMC External Transceiver (1.8V) */ case 0x5: /* SD/MMC Internal Transceiver (3.0V) */
socfpga_per_reset(SOCFPGA_RESET(SDMMC), 0);
socfpga_per_reset(SOCFPGA_RESET(DMA), 0); return BOOT_DEVICE_MMC1; case 0x6: /* QSPI Flash (1.8V) */ case 0x7: /* QSPI Flash (3.0V) */
socfpga_per_reset(SOCFPGA_RESET(QSPI), 0); return BOOT_DEVICE_SPI; default: printf("Invalid boot device (bsel=%08x)!\n", bsel);
@@ -99,9 +95,7 @@ void board_init_f(ulong dummy) socfpga_bridges_reset(1); }
socfpga_per_reset(SOCFPGA_RESET(UART0), 0); socfpga_per_reset(SOCFPGA_RESET(OSC1TIMER0), 0);
timer_init(); debug("Reconfigure Clock Manager\n");
@@ -123,10 +117,6 @@ void board_init_f(ulong dummy) sysmgr_pinmux_init(); sysmgr_config_warmrstcfgio(0);
/* De-assert reset for peripherals and bridges based on handoff */
reset_deassert_peripherals_handoff();
socfpga_bridges_reset(0);
I just saw that this line might have unwanted side effects in that it does not write the enabled bridges bitmask to the 'iswgrp_handoff' registers. So the 'bridge enable' command might not work.
This whole handoff thing looks somewhat broken to me anyway: the original Altera U-Boot did it because it obeyed Quartus output, while we now just always write a constant bitmask here (see socfpga_bridges_reset()).
Regards, Simon
debug("Unfreezing/Thaw all I/O banks\n"); /* unfreeze / thaw all IO banks */ sys_mgr_frzctrl_thaw_req();
Nice!
-- Best regards, Marek Vasut

On 2/22/19 7:35 AM, Simon Goldschmidt wrote:
On Thu, Feb 21, 2019 at 10:56 PM Marek Vasut marex@denx.de wrote:
On 2/21/19 10:43 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.
Signed-off-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com
Changes in v2:
- removed Kconfig option OLD_SOCFPGA_KERNEL_COMPAT since compatibility now uses an environment variable
arch/arm/mach-socfpga/misc_gen5.c | 10 ---------- arch/arm/mach-socfpga/spl_gen5.c | 10 ---------- 2 files changed, 20 deletions(-)
diff --git a/arch/arm/mach-socfpga/misc_gen5.c b/arch/arm/mach-socfpga/misc_gen5.c index 6e11ba6cb2..9865f5b5b1 100644 --- a/arch/arm/mach-socfpga/misc_gen5.c +++ b/arch/arm/mach-socfpga/misc_gen5.c @@ -201,16 +201,6 @@ int arch_early_init_r(void) /* Add device descriptor to FPGA device table */ socfpga_fpga_add(&altera_fpga[0]);
-#ifdef CONFIG_DESIGNWARE_SPI
/* Get Designware SPI controller out of reset */
socfpga_per_reset(SOCFPGA_RESET(SPIM0), 0);
socfpga_per_reset(SOCFPGA_RESET(SPIM1), 0);
-#endif
-#ifdef CONFIG_NAND_DENALI
socfpga_per_reset(SOCFPGA_RESET(NAND), 0);
-#endif
return 0;
}
diff --git a/arch/arm/mach-socfpga/spl_gen5.c b/arch/arm/mach-socfpga/spl_gen5.c index 1bff8cbfcf..e1e65261eb 100644 --- a/arch/arm/mach-socfpga/spl_gen5.c +++ b/arch/arm/mach-socfpga/spl_gen5.c @@ -36,16 +36,12 @@ u32 spl_boot_device(void) return BOOT_DEVICE_RAM; case 0x2: /* NAND Flash (1.8V) */ case 0x3: /* NAND Flash (3.0V) */
socfpga_per_reset(SOCFPGA_RESET(NAND), 0); return BOOT_DEVICE_NAND; case 0x4: /* SD/MMC External Transceiver (1.8V) */ case 0x5: /* SD/MMC Internal Transceiver (3.0V) */
socfpga_per_reset(SOCFPGA_RESET(SDMMC), 0);
socfpga_per_reset(SOCFPGA_RESET(DMA), 0); return BOOT_DEVICE_MMC1; case 0x6: /* QSPI Flash (1.8V) */ case 0x7: /* QSPI Flash (3.0V) */
socfpga_per_reset(SOCFPGA_RESET(QSPI), 0); return BOOT_DEVICE_SPI; default: printf("Invalid boot device (bsel=%08x)!\n", bsel);
@@ -99,9 +95,7 @@ void board_init_f(ulong dummy) socfpga_bridges_reset(1); }
socfpga_per_reset(SOCFPGA_RESET(UART0), 0); socfpga_per_reset(SOCFPGA_RESET(OSC1TIMER0), 0);
timer_init(); debug("Reconfigure Clock Manager\n");
@@ -123,10 +117,6 @@ void board_init_f(ulong dummy) sysmgr_pinmux_init(); sysmgr_config_warmrstcfgio(0);
/* De-assert reset for peripherals and bridges based on handoff */
reset_deassert_peripherals_handoff();
socfpga_bridges_reset(0);
I just saw that this line might have unwanted side effects in that it does not write the enabled bridges bitmask to the 'iswgrp_handoff' registers. So the 'bridge enable' command might not work.
This whole handoff thing looks somewhat broken to me anyway: the original Altera U-Boot did it because it obeyed Quartus output, while we now just always write a constant bitmask here (see socfpga_bridges_reset()).
Regards, Simon
Might make sense to finally clean it up ?

Am Fr., 22. Feb. 2019, 18:34 hat Marek Vasut marex@denx.de geschrieben:
On 2/22/19 7:35 AM, Simon Goldschmidt wrote:
On Thu, Feb 21, 2019 at 10:56 PM Marek Vasut marex@denx.de wrote:
On 2/21/19 10:43 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.
Signed-off-by: Simon Goldschmidt simon.k.r.goldschmidt@gmail.com
Changes in v2:
- removed Kconfig option OLD_SOCFPGA_KERNEL_COMPAT since compatibility now uses an environment variable
arch/arm/mach-socfpga/misc_gen5.c | 10 ---------- arch/arm/mach-socfpga/spl_gen5.c | 10 ---------- 2 files changed, 20 deletions(-)
diff --git a/arch/arm/mach-socfpga/misc_gen5.c
b/arch/arm/mach-socfpga/misc_gen5.c
index 6e11ba6cb2..9865f5b5b1 100644 --- a/arch/arm/mach-socfpga/misc_gen5.c +++ b/arch/arm/mach-socfpga/misc_gen5.c @@ -201,16 +201,6 @@ int arch_early_init_r(void) /* Add device descriptor to FPGA device table */ socfpga_fpga_add(&altera_fpga[0]);
-#ifdef CONFIG_DESIGNWARE_SPI
/* Get Designware SPI controller out of reset */
socfpga_per_reset(SOCFPGA_RESET(SPIM0), 0);
socfpga_per_reset(SOCFPGA_RESET(SPIM1), 0);
-#endif
-#ifdef CONFIG_NAND_DENALI
socfpga_per_reset(SOCFPGA_RESET(NAND), 0);
-#endif
return 0;
}
diff --git a/arch/arm/mach-socfpga/spl_gen5.c
b/arch/arm/mach-socfpga/spl_gen5.c
index 1bff8cbfcf..e1e65261eb 100644 --- a/arch/arm/mach-socfpga/spl_gen5.c +++ b/arch/arm/mach-socfpga/spl_gen5.c @@ -36,16 +36,12 @@ u32 spl_boot_device(void) return BOOT_DEVICE_RAM; case 0x2: /* NAND Flash (1.8V) */ case 0x3: /* NAND Flash (3.0V) */
socfpga_per_reset(SOCFPGA_RESET(NAND), 0); return BOOT_DEVICE_NAND; case 0x4: /* SD/MMC External Transceiver (1.8V) */ case 0x5: /* SD/MMC Internal Transceiver (3.0V) */
socfpga_per_reset(SOCFPGA_RESET(SDMMC), 0);
socfpga_per_reset(SOCFPGA_RESET(DMA), 0); return BOOT_DEVICE_MMC1; case 0x6: /* QSPI Flash (1.8V) */ case 0x7: /* QSPI Flash (3.0V) */
socfpga_per_reset(SOCFPGA_RESET(QSPI), 0); return BOOT_DEVICE_SPI; default: printf("Invalid boot device (bsel=%08x)!\n", bsel);
@@ -99,9 +95,7 @@ void board_init_f(ulong dummy) socfpga_bridges_reset(1); }
socfpga_per_reset(SOCFPGA_RESET(UART0), 0); socfpga_per_reset(SOCFPGA_RESET(OSC1TIMER0), 0);
timer_init(); debug("Reconfigure Clock Manager\n");
@@ -123,10 +117,6 @@ void board_init_f(ulong dummy) sysmgr_pinmux_init(); sysmgr_config_warmrstcfgio(0);
/* De-assert reset for peripherals and bridges based on handoff
*/
reset_deassert_peripherals_handoff();
socfpga_bridges_reset(0);
I just saw that this line might have unwanted side effects in that it
does
not write the enabled bridges bitmask to the 'iswgrp_handoff' registers. So the 'bridge enable' command might not work.
This whole handoff thing looks somewhat broken to me anyway: the original Altera U-Boot did it because it obeyed Quartus output, while we now just always write a constant bitmask here (see
socfpga_bridges_reset()).
Regards, Simon
Might make sense to finally clean it up ?
Yeah, well the problem is that for some things, handoff from Quartus is required while for other things, devicetree information like it is now is enough.
I'll set that in my list *g*
Regards, Simon

On 2/22/19 8:10 PM, Simon Goldschmidt wrote:
Am Fr., 22. Feb. 2019, 18:34 hat Marek Vasut <marex@denx.de mailto:marex@denx.de> geschrieben:
On 2/22/19 7:35 AM, Simon Goldschmidt wrote: > On Thu, Feb 21, 2019 at 10:56 PM Marek Vasut <marex@denx.de <mailto:marex@denx.de>> wrote: >> >> On 2/21/19 10:43 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. >>> >>> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com <mailto:simon.k.r.goldschmidt@gmail.com>> >>> --- >>> >>> Changes in v2: >>> - removed Kconfig option OLD_SOCFPGA_KERNEL_COMPAT since compatibility >>> now uses an environment variable >>> >>> arch/arm/mach-socfpga/misc_gen5.c | 10 ---------- >>> arch/arm/mach-socfpga/spl_gen5.c | 10 ---------- >>> 2 files changed, 20 deletions(-) >>> >>> diff --git a/arch/arm/mach-socfpga/misc_gen5.c b/arch/arm/mach-socfpga/misc_gen5.c >>> index 6e11ba6cb2..9865f5b5b1 100644 >>> --- a/arch/arm/mach-socfpga/misc_gen5.c >>> +++ b/arch/arm/mach-socfpga/misc_gen5.c >>> @@ -201,16 +201,6 @@ int arch_early_init_r(void) >>> /* Add device descriptor to FPGA device table */ >>> socfpga_fpga_add(&altera_fpga[0]); >>> >>> -#ifdef CONFIG_DESIGNWARE_SPI >>> - /* Get Designware SPI controller out of reset */ >>> - socfpga_per_reset(SOCFPGA_RESET(SPIM0), 0); >>> - socfpga_per_reset(SOCFPGA_RESET(SPIM1), 0); >>> -#endif >>> - >>> -#ifdef CONFIG_NAND_DENALI >>> - socfpga_per_reset(SOCFPGA_RESET(NAND), 0); >>> -#endif >>> - >>> return 0; >>> } >>> >>> diff --git a/arch/arm/mach-socfpga/spl_gen5.c b/arch/arm/mach-socfpga/spl_gen5.c >>> index 1bff8cbfcf..e1e65261eb 100644 >>> --- a/arch/arm/mach-socfpga/spl_gen5.c >>> +++ b/arch/arm/mach-socfpga/spl_gen5.c >>> @@ -36,16 +36,12 @@ u32 spl_boot_device(void) >>> return BOOT_DEVICE_RAM; >>> case 0x2: /* NAND Flash (1.8V) */ >>> case 0x3: /* NAND Flash (3.0V) */ >>> - socfpga_per_reset(SOCFPGA_RESET(NAND), 0); >>> return BOOT_DEVICE_NAND; >>> case 0x4: /* SD/MMC External Transceiver (1.8V) */ >>> case 0x5: /* SD/MMC Internal Transceiver (3.0V) */ >>> - socfpga_per_reset(SOCFPGA_RESET(SDMMC), 0); >>> - socfpga_per_reset(SOCFPGA_RESET(DMA), 0); >>> return BOOT_DEVICE_MMC1; >>> case 0x6: /* QSPI Flash (1.8V) */ >>> case 0x7: /* QSPI Flash (3.0V) */ >>> - socfpga_per_reset(SOCFPGA_RESET(QSPI), 0); >>> return BOOT_DEVICE_SPI; >>> default: >>> printf("Invalid boot device (bsel=%08x)!\n", bsel); >>> @@ -99,9 +95,7 @@ void board_init_f(ulong dummy) >>> socfpga_bridges_reset(1); >>> } >>> >>> - socfpga_per_reset(SOCFPGA_RESET(UART0), 0); >>> socfpga_per_reset(SOCFPGA_RESET(OSC1TIMER0), 0); >>> - >>> timer_init(); >>> >>> debug("Reconfigure Clock Manager\n"); >>> @@ -123,10 +117,6 @@ void board_init_f(ulong dummy) >>> sysmgr_pinmux_init(); >>> sysmgr_config_warmrstcfgio(0); >>> >>> - /* De-assert reset for peripherals and bridges based on handoff */ >>> - reset_deassert_peripherals_handoff(); >>> - socfpga_bridges_reset(0); > > I just saw that this line might have unwanted side effects in that it does > not write the enabled bridges bitmask to the 'iswgrp_handoff' registers. > So the 'bridge enable' command might not work. > > This whole handoff thing looks somewhat broken to me anyway: the > original Altera U-Boot did it because it obeyed Quartus output, while we > now just always write a constant bitmask here (see socfpga_bridges_reset()). > > Regards, > Simon Might make sense to finally clean it up ?
Yeah, well the problem is that for some things, handoff from Quartus is required while for other things, devicetree information like it is now is enough.
Might be just about the right time to convert quartus handoff files to DT ?

Am Fr., 22. Feb. 2019, 20:37 hat Marek Vasut marex@denx.de geschrieben:
On 2/22/19 8:10 PM, Simon Goldschmidt wrote:
Am Fr., 22. Feb. 2019, 18:34 hat Marek Vasut <marex@denx.de mailto:marex@denx.de> geschrieben:
On 2/22/19 7:35 AM, Simon Goldschmidt wrote: > On Thu, Feb 21, 2019 at 10:56 PM Marek Vasut <marex@denx.de <mailto:marex@denx.de>> wrote: >> >> On 2/21/19 10:43 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. >>> >>> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com <mailto:simon.k.r.goldschmidt@gmail.com>> >>> --- >>> >>> Changes in v2: >>> - removed Kconfig option OLD_SOCFPGA_KERNEL_COMPAT since compatibility >>> now uses an environment variable >>> >>> arch/arm/mach-socfpga/misc_gen5.c | 10 ---------- >>> arch/arm/mach-socfpga/spl_gen5.c | 10 ---------- >>> 2 files changed, 20 deletions(-) >>> >>> diff --git a/arch/arm/mach-socfpga/misc_gen5.c b/arch/arm/mach-socfpga/misc_gen5.c >>> index 6e11ba6cb2..9865f5b5b1 100644 >>> --- a/arch/arm/mach-socfpga/misc_gen5.c >>> +++ b/arch/arm/mach-socfpga/misc_gen5.c >>> @@ -201,16 +201,6 @@ int arch_early_init_r(void) >>> /* Add device descriptor to FPGA device table */ >>> socfpga_fpga_add(&altera_fpga[0]); >>> >>> -#ifdef CONFIG_DESIGNWARE_SPI >>> - /* Get Designware SPI controller out of reset */ >>> - socfpga_per_reset(SOCFPGA_RESET(SPIM0), 0); >>> - socfpga_per_reset(SOCFPGA_RESET(SPIM1), 0); >>> -#endif >>> - >>> -#ifdef CONFIG_NAND_DENALI >>> - socfpga_per_reset(SOCFPGA_RESET(NAND), 0); >>> -#endif >>> - >>> return 0; >>> } >>> >>> diff --git a/arch/arm/mach-socfpga/spl_gen5.c b/arch/arm/mach-socfpga/spl_gen5.c >>> index 1bff8cbfcf..e1e65261eb 100644 >>> --- a/arch/arm/mach-socfpga/spl_gen5.c >>> +++ b/arch/arm/mach-socfpga/spl_gen5.c >>> @@ -36,16 +36,12 @@ u32 spl_boot_device(void) >>> return BOOT_DEVICE_RAM; >>> case 0x2: /* NAND Flash (1.8V) */ >>> case 0x3: /* NAND Flash (3.0V) */ >>> - socfpga_per_reset(SOCFPGA_RESET(NAND), 0); >>> return BOOT_DEVICE_NAND; >>> case 0x4: /* SD/MMC External Transceiver (1.8V) */ >>> case 0x5: /* SD/MMC Internal Transceiver (3.0V) */ >>> - socfpga_per_reset(SOCFPGA_RESET(SDMMC), 0); >>> - socfpga_per_reset(SOCFPGA_RESET(DMA), 0); >>> return BOOT_DEVICE_MMC1; >>> case 0x6: /* QSPI Flash (1.8V) */ >>> case 0x7: /* QSPI Flash (3.0V) */ >>> - socfpga_per_reset(SOCFPGA_RESET(QSPI), 0); >>> return BOOT_DEVICE_SPI; >>> default: >>> printf("Invalid boot device (bsel=%08x)!\n", bsel); >>> @@ -99,9 +95,7 @@ void board_init_f(ulong dummy) >>> socfpga_bridges_reset(1); >>> } >>> >>> - socfpga_per_reset(SOCFPGA_RESET(UART0), 0); >>> socfpga_per_reset(SOCFPGA_RESET(OSC1TIMER0), 0); >>> - >>> timer_init(); >>> >>> debug("Reconfigure Clock Manager\n"); >>> @@ -123,10 +117,6 @@ void board_init_f(ulong dummy) >>> sysmgr_pinmux_init(); >>> sysmgr_config_warmrstcfgio(0); >>> >>> - /* De-assert reset for peripherals and bridges based on handoff */ >>> - reset_deassert_peripherals_handoff(); >>> - socfpga_bridges_reset(0); > > I just saw that this line might have unwanted side effects in that it does > not write the enabled bridges bitmask to the 'iswgrp_handoff' registers. > So the 'bridge enable' command might not work. > > This whole handoff thing looks somewhat broken to me anyway: the > original Altera U-Boot did it because it obeyed Quartus output, while we > now just always write a constant bitmask here (see socfpga_bridges_reset()). > > Regards, > Simon Might make sense to finally clean it up ?
Yeah, well the problem is that for some things, handoff from Quartus is required while for other things, devicetree information like it is now is enough.
Might be just about the right time to convert quartus handoff files to DT ?
That would be great: I do have a use case where the pin description should be changeable after SPL.
But that again would require more time than I currently have... :-(
Regards, Simon

On 2/22/19 8:42 PM, Simon Goldschmidt wrote:
Am Fr., 22. Feb. 2019, 20:37 hat Marek Vasut <marex@denx.de mailto:marex@denx.de> geschrieben:
On 2/22/19 8:10 PM, Simon Goldschmidt wrote: > > > Am Fr., 22. Feb. 2019, 18:34 hat Marek Vasut <marex@denx.de <mailto:marex@denx.de> > <mailto:marex@denx.de <mailto:marex@denx.de>>> geschrieben: > > On 2/22/19 7:35 AM, Simon Goldschmidt wrote: > > On Thu, Feb 21, 2019 at 10:56 PM Marek Vasut <marex@denx.de <mailto:marex@denx.de> > <mailto:marex@denx.de <mailto:marex@denx.de>>> wrote: > >> > >> On 2/21/19 10:43 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. > >>> > >>> Signed-off-by: Simon Goldschmidt > <simon.k.r.goldschmidt@gmail.com <mailto:simon.k.r.goldschmidt@gmail.com> > <mailto:simon.k.r.goldschmidt@gmail.com <mailto:simon.k.r.goldschmidt@gmail.com>>> > >>> --- > >>> > >>> Changes in v2: > >>> - removed Kconfig option OLD_SOCFPGA_KERNEL_COMPAT since > compatibility > >>> now uses an environment variable > >>> > >>> arch/arm/mach-socfpga/misc_gen5.c | 10 ---------- > >>> arch/arm/mach-socfpga/spl_gen5.c | 10 ---------- > >>> 2 files changed, 20 deletions(-) > >>> > >>> diff --git a/arch/arm/mach-socfpga/misc_gen5.c > b/arch/arm/mach-socfpga/misc_gen5.c > >>> index 6e11ba6cb2..9865f5b5b1 100644 > >>> --- a/arch/arm/mach-socfpga/misc_gen5.c > >>> +++ b/arch/arm/mach-socfpga/misc_gen5.c > >>> @@ -201,16 +201,6 @@ int arch_early_init_r(void) > >>> /* Add device descriptor to FPGA device table */ > >>> socfpga_fpga_add(&altera_fpga[0]); > >>> > >>> -#ifdef CONFIG_DESIGNWARE_SPI > >>> - /* Get Designware SPI controller out of reset */ > >>> - socfpga_per_reset(SOCFPGA_RESET(SPIM0), 0); > >>> - socfpga_per_reset(SOCFPGA_RESET(SPIM1), 0); > >>> -#endif > >>> - > >>> -#ifdef CONFIG_NAND_DENALI > >>> - socfpga_per_reset(SOCFPGA_RESET(NAND), 0); > >>> -#endif > >>> - > >>> return 0; > >>> } > >>> > >>> diff --git a/arch/arm/mach-socfpga/spl_gen5.c > b/arch/arm/mach-socfpga/spl_gen5.c > >>> index 1bff8cbfcf..e1e65261eb 100644 > >>> --- a/arch/arm/mach-socfpga/spl_gen5.c > >>> +++ b/arch/arm/mach-socfpga/spl_gen5.c > >>> @@ -36,16 +36,12 @@ u32 spl_boot_device(void) > >>> return BOOT_DEVICE_RAM; > >>> case 0x2: /* NAND Flash (1.8V) */ > >>> case 0x3: /* NAND Flash (3.0V) */ > >>> - socfpga_per_reset(SOCFPGA_RESET(NAND), 0); > >>> return BOOT_DEVICE_NAND; > >>> case 0x4: /* SD/MMC External Transceiver (1.8V) */ > >>> case 0x5: /* SD/MMC Internal Transceiver (3.0V) */ > >>> - socfpga_per_reset(SOCFPGA_RESET(SDMMC), 0); > >>> - socfpga_per_reset(SOCFPGA_RESET(DMA), 0); > >>> return BOOT_DEVICE_MMC1; > >>> case 0x6: /* QSPI Flash (1.8V) */ > >>> case 0x7: /* QSPI Flash (3.0V) */ > >>> - socfpga_per_reset(SOCFPGA_RESET(QSPI), 0); > >>> return BOOT_DEVICE_SPI; > >>> default: > >>> printf("Invalid boot device (bsel=%08x)!\n", bsel); > >>> @@ -99,9 +95,7 @@ void board_init_f(ulong dummy) > >>> socfpga_bridges_reset(1); > >>> } > >>> > >>> - socfpga_per_reset(SOCFPGA_RESET(UART0), 0); > >>> socfpga_per_reset(SOCFPGA_RESET(OSC1TIMER0), 0); > >>> - > >>> timer_init(); > >>> > >>> debug("Reconfigure Clock Manager\n"); > >>> @@ -123,10 +117,6 @@ void board_init_f(ulong dummy) > >>> sysmgr_pinmux_init(); > >>> sysmgr_config_warmrstcfgio(0); > >>> > >>> - /* De-assert reset for peripherals and bridges based on > handoff */ > >>> - reset_deassert_peripherals_handoff(); > >>> - socfpga_bridges_reset(0); > > > > I just saw that this line might have unwanted side effects in that > it does > > not write the enabled bridges bitmask to the 'iswgrp_handoff' > registers. > > So the 'bridge enable' command might not work. > > > > This whole handoff thing looks somewhat broken to me anyway: the > > original Altera U-Boot did it because it obeyed Quartus output, > while we > > now just always write a constant bitmask here (see > socfpga_bridges_reset()). > > > > Regards, > > Simon > > Might make sense to finally clean it up ? > > > Yeah, well the problem is that for some things, handoff from Quartus is > required while for other things, devicetree information like it is now > is enough. Might be just about the right time to convert quartus handoff files to DT ?
That would be great: I do have a use case where the pin description should be changeable after SPL.
Well, we don't have the pin control documentation, so unless we "observe" what quartus generates real hard ...
But that again would require more time than I currently have... :-(
Right, I'm not asking you to do it to get this series in.

Am Fr., 22. Feb. 2019, 20:47 hat Marek Vasut marex@denx.de geschrieben:
On 2/22/19 8:42 PM, Simon Goldschmidt wrote:
Am Fr., 22. Feb. 2019, 20:37 hat Marek Vasut <marex@denx.de mailto:marex@denx.de> geschrieben:
On 2/22/19 8:10 PM, Simon Goldschmidt wrote: > > > Am Fr., 22. Feb. 2019, 18:34 hat Marek Vasut <marex@denx.de <mailto:marex@denx.de> > <mailto:marex@denx.de <mailto:marex@denx.de>>> geschrieben: > > On 2/22/19 7:35 AM, Simon Goldschmidt wrote: > > On Thu, Feb 21, 2019 at 10:56 PM Marek Vasut <marex@denx.de <mailto:marex@denx.de> > <mailto:marex@denx.de <mailto:marex@denx.de>>> wrote: > >> > >> On 2/21/19 10:43 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. > >>> > >>> Signed-off-by: Simon Goldschmidt > <simon.k.r.goldschmidt@gmail.com <mailto:simon.k.r.goldschmidt@gmail.com> > <mailto:simon.k.r.goldschmidt@gmail.com <mailto:simon.k.r.goldschmidt@gmail.com>>> > >>> --- > >>> > >>> Changes in v2: > >>> - removed Kconfig option OLD_SOCFPGA_KERNEL_COMPAT since > compatibility > >>> now uses an environment variable > >>> > >>> arch/arm/mach-socfpga/misc_gen5.c | 10 ---------- > >>> arch/arm/mach-socfpga/spl_gen5.c | 10 ---------- > >>> 2 files changed, 20 deletions(-) > >>> > >>> diff --git a/arch/arm/mach-socfpga/misc_gen5.c > b/arch/arm/mach-socfpga/misc_gen5.c > >>> index 6e11ba6cb2..9865f5b5b1 100644 > >>> --- a/arch/arm/mach-socfpga/misc_gen5.c > >>> +++ b/arch/arm/mach-socfpga/misc_gen5.c > >>> @@ -201,16 +201,6 @@ int arch_early_init_r(void) > >>> /* Add device descriptor to FPGA device table */ > >>> socfpga_fpga_add(&altera_fpga[0]); > >>> > >>> -#ifdef CONFIG_DESIGNWARE_SPI > >>> - /* Get Designware SPI controller out of reset */ > >>> - socfpga_per_reset(SOCFPGA_RESET(SPIM0), 0); > >>> - socfpga_per_reset(SOCFPGA_RESET(SPIM1), 0); > >>> -#endif > >>> - > >>> -#ifdef CONFIG_NAND_DENALI > >>> - socfpga_per_reset(SOCFPGA_RESET(NAND), 0); > >>> -#endif > >>> - > >>> return 0; > >>> } > >>> > >>> diff --git a/arch/arm/mach-socfpga/spl_gen5.c > b/arch/arm/mach-socfpga/spl_gen5.c > >>> index 1bff8cbfcf..e1e65261eb 100644 > >>> --- a/arch/arm/mach-socfpga/spl_gen5.c > >>> +++ b/arch/arm/mach-socfpga/spl_gen5.c > >>> @@ -36,16 +36,12 @@ u32 spl_boot_device(void) > >>> return BOOT_DEVICE_RAM; > >>> case 0x2: /* NAND Flash (1.8V) */ > >>> case 0x3: /* NAND Flash (3.0V) */ > >>> - socfpga_per_reset(SOCFPGA_RESET(NAND), 0); > >>> return BOOT_DEVICE_NAND; > >>> case 0x4: /* SD/MMC External Transceiver
(1.8V) */
> >>> case 0x5: /* SD/MMC Internal Transceiver
(3.0V) */
> >>> - socfpga_per_reset(SOCFPGA_RESET(SDMMC), 0); > >>> - socfpga_per_reset(SOCFPGA_RESET(DMA), 0); > >>> return BOOT_DEVICE_MMC1; > >>> case 0x6: /* QSPI Flash (1.8V) */ > >>> case 0x7: /* QSPI Flash (3.0V) */ > >>> - socfpga_per_reset(SOCFPGA_RESET(QSPI), 0); > >>> return BOOT_DEVICE_SPI; > >>> default: > >>> printf("Invalid boot device (bsel=%08x)!\n", bsel); > >>> @@ -99,9 +95,7 @@ void board_init_f(ulong dummy) > >>> socfpga_bridges_reset(1); > >>> } > >>> > >>> - socfpga_per_reset(SOCFPGA_RESET(UART0), 0); > >>> socfpga_per_reset(SOCFPGA_RESET(OSC1TIMER0), 0); > >>> - > >>> timer_init(); > >>> > >>> debug("Reconfigure Clock Manager\n"); > >>> @@ -123,10 +117,6 @@ void board_init_f(ulong dummy) > >>> sysmgr_pinmux_init(); > >>> sysmgr_config_warmrstcfgio(0); > >>> > >>> - /* De-assert reset for peripherals and bridges based
on
> handoff */ > >>> - reset_deassert_peripherals_handoff(); > >>> - socfpga_bridges_reset(0); > > > > I just saw that this line might have unwanted side effects in that > it does > > not write the enabled bridges bitmask to the 'iswgrp_handoff' > registers. > > So the 'bridge enable' command might not work. > > > > This whole handoff thing looks somewhat broken to me anyway:
the
> > original Altera U-Boot did it because it obeyed Quartus
output,
> while we > > now just always write a constant bitmask here (see > socfpga_bridges_reset()). > > > > Regards, > > Simon > > Might make sense to finally clean it up ? > > > Yeah, well the problem is that for some things, handoff from Quartus is > required while for other things, devicetree information like it is
now
> is enough. Might be just about the right time to convert quartus handoff files to DT ?
That would be great: I do have a use case where the pin description should be changeable after SPL.
Well, we don't have the pin control documentation, so unless we "observe" what quartus generates real hard ...
Sadly, yes. However, I would think it would still be better to embed those u32 arrays we have now into the device tree. That would give us the possibility to apply them from SPL, U-Boot or even Linux.
My use case is that the pin settings change depending on the FPGA image (e.g. hps pins routed to FPGA etc.). With the current code, I have to update SPL to match the FPGA...
But that again would require more time than I currently have... :-(
Right, I'm not asking you to do it to get this series in.
I didn't mean to say that. But I'd like to have that better sooner than later. Only I can't find the time to do it ...
Regards, Simon

On 2/22/19 8:55 PM, Simon Goldschmidt wrote:
Am Fr., 22. Feb. 2019, 20:47 hat Marek Vasut <marex@denx.de mailto:marex@denx.de> geschrieben:
On 2/22/19 8:42 PM, Simon Goldschmidt wrote: > > > Am Fr., 22. Feb. 2019, 20:37 hat Marek Vasut <marex@denx.de <mailto:marex@denx.de> > <mailto:marex@denx.de <mailto:marex@denx.de>>> geschrieben: > > On 2/22/19 8:10 PM, Simon Goldschmidt wrote: > > > > > > Am Fr., 22. Feb. 2019, 18:34 hat Marek Vasut <marex@denx.de <mailto:marex@denx.de> > <mailto:marex@denx.de <mailto:marex@denx.de>> > > <mailto:marex@denx.de <mailto:marex@denx.de> <mailto:marex@denx.de <mailto:marex@denx.de>>>> geschrieben: > > > > On 2/22/19 7:35 AM, Simon Goldschmidt wrote: > > > On Thu, Feb 21, 2019 at 10:56 PM Marek Vasut <marex@denx.de <mailto:marex@denx.de> > <mailto:marex@denx.de <mailto:marex@denx.de>> > > <mailto:marex@denx.de <mailto:marex@denx.de> <mailto:marex@denx.de <mailto:marex@denx.de>>>> wrote: > > >> > > >> On 2/21/19 10:43 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. > > >>> > > >>> Signed-off-by: Simon Goldschmidt > > <simon.k.r.goldschmidt@gmail.com <mailto:simon.k.r.goldschmidt@gmail.com> > <mailto:simon.k.r.goldschmidt@gmail.com <mailto:simon.k.r.goldschmidt@gmail.com>> > > <mailto:simon.k.r.goldschmidt@gmail.com <mailto:simon.k.r.goldschmidt@gmail.com> > <mailto:simon.k.r.goldschmidt@gmail.com <mailto:simon.k.r.goldschmidt@gmail.com>>>> > > >>> --- > > >>> > > >>> Changes in v2: > > >>> - removed Kconfig option OLD_SOCFPGA_KERNEL_COMPAT since > > compatibility > > >>> now uses an environment variable > > >>> > > >>> arch/arm/mach-socfpga/misc_gen5.c | 10 ---------- > > >>> arch/arm/mach-socfpga/spl_gen5.c | 10 ---------- > > >>> 2 files changed, 20 deletions(-) > > >>> > > >>> diff --git a/arch/arm/mach-socfpga/misc_gen5.c > > b/arch/arm/mach-socfpga/misc_gen5.c > > >>> index 6e11ba6cb2..9865f5b5b1 100644 > > >>> --- a/arch/arm/mach-socfpga/misc_gen5.c > > >>> +++ b/arch/arm/mach-socfpga/misc_gen5.c > > >>> @@ -201,16 +201,6 @@ int arch_early_init_r(void) > > >>> /* Add device descriptor to FPGA device table */ > > >>> socfpga_fpga_add(&altera_fpga[0]); > > >>> > > >>> -#ifdef CONFIG_DESIGNWARE_SPI > > >>> - /* Get Designware SPI controller out of reset */ > > >>> - socfpga_per_reset(SOCFPGA_RESET(SPIM0), 0); > > >>> - socfpga_per_reset(SOCFPGA_RESET(SPIM1), 0); > > >>> -#endif > > >>> - > > >>> -#ifdef CONFIG_NAND_DENALI > > >>> - socfpga_per_reset(SOCFPGA_RESET(NAND), 0); > > >>> -#endif > > >>> - > > >>> return 0; > > >>> } > > >>> > > >>> diff --git a/arch/arm/mach-socfpga/spl_gen5.c > > b/arch/arm/mach-socfpga/spl_gen5.c > > >>> index 1bff8cbfcf..e1e65261eb 100644 > > >>> --- a/arch/arm/mach-socfpga/spl_gen5.c > > >>> +++ b/arch/arm/mach-socfpga/spl_gen5.c > > >>> @@ -36,16 +36,12 @@ u32 spl_boot_device(void) > > >>> return BOOT_DEVICE_RAM; > > >>> case 0x2: /* NAND Flash (1.8V) */ > > >>> case 0x3: /* NAND Flash (3.0V) */ > > >>> - socfpga_per_reset(SOCFPGA_RESET(NAND), 0); > > >>> return BOOT_DEVICE_NAND; > > >>> case 0x4: /* SD/MMC External Transceiver (1.8V) */ > > >>> case 0x5: /* SD/MMC Internal Transceiver (3.0V) */ > > >>> - socfpga_per_reset(SOCFPGA_RESET(SDMMC), 0); > > >>> - socfpga_per_reset(SOCFPGA_RESET(DMA), 0); > > >>> return BOOT_DEVICE_MMC1; > > >>> case 0x6: /* QSPI Flash (1.8V) */ > > >>> case 0x7: /* QSPI Flash (3.0V) */ > > >>> - socfpga_per_reset(SOCFPGA_RESET(QSPI), 0); > > >>> return BOOT_DEVICE_SPI; > > >>> default: > > >>> printf("Invalid boot device (bsel=%08x)!\n", > bsel); > > >>> @@ -99,9 +95,7 @@ void board_init_f(ulong dummy) > > >>> socfpga_bridges_reset(1); > > >>> } > > >>> > > >>> - socfpga_per_reset(SOCFPGA_RESET(UART0), 0); > > >>> socfpga_per_reset(SOCFPGA_RESET(OSC1TIMER0), 0); > > >>> - > > >>> timer_init(); > > >>> > > >>> debug("Reconfigure Clock Manager\n"); > > >>> @@ -123,10 +117,6 @@ void board_init_f(ulong dummy) > > >>> sysmgr_pinmux_init(); > > >>> sysmgr_config_warmrstcfgio(0); > > >>> > > >>> - /* De-assert reset for peripherals and bridges based on > > handoff */ > > >>> - reset_deassert_peripherals_handoff(); > > >>> - socfpga_bridges_reset(0); > > > > > > I just saw that this line might have unwanted side effects > in that > > it does > > > not write the enabled bridges bitmask to the 'iswgrp_handoff' > > registers. > > > So the 'bridge enable' command might not work. > > > > > > This whole handoff thing looks somewhat broken to me anyway: the > > > original Altera U-Boot did it because it obeyed Quartus output, > > while we > > > now just always write a constant bitmask here (see > > socfpga_bridges_reset()). > > > > > > Regards, > > > Simon > > > > Might make sense to finally clean it up ? > > > > > > Yeah, well the problem is that for some things, handoff from > Quartus is > > required while for other things, devicetree information like it is now > > is enough. > > Might be just about the right time to convert quartus handoff files > to DT ? > > > That would be great: I do have a use case where the pin description > should be changeable after SPL. Well, we don't have the pin control documentation, so unless we "observe" what quartus generates real hard ...
Sadly, yes. However, I would think it would still be better to embed those u32 arrays we have now into the device tree. That would give us the possibility to apply them from SPL, U-Boot or even Linux.
Well, it's SPL only, it's highly un-recommended to fiddle with the pin configuration at runtime. Although, I suspect that might be just altera being cautious.
My use case is that the pin settings change depending on the FPGA image (e.g. hps pins routed to FPGA etc.). With the current code, I have to update SPL to match the FPGA...
Right, having it in DT, we could have one SPL for multiple systems. Would be nice.
> But that again would require more time than I currently have... :-( Right, I'm not asking you to do it to get this series in.
I didn't mean to say that. But I'd like to have that better sooner than later. Only I can't find the time to do it ...
Right, that's what I meant.
btw you gonna be at EW next week ?

On 22.02.2019 22:14, Marek Vasut wrote:
On 2/22/19 8:55 PM, Simon Goldschmidt wrote:
Am Fr., 22. Feb. 2019, 20:47 hat Marek Vasut <marex@denx.de mailto:marex@denx.de> geschrieben:
On 2/22/19 8:42 PM, Simon Goldschmidt wrote: > > > Am Fr., 22. Feb. 2019, 20:37 hat Marek Vasut <marex@denx.de <mailto:marex@denx.de> > <mailto:marex@denx.de <mailto:marex@denx.de>>> geschrieben: > > On 2/22/19 8:10 PM, Simon Goldschmidt wrote: > > > > > > Am Fr., 22. Feb. 2019, 18:34 hat Marek Vasut <marex@denx.de <mailto:marex@denx.de> > <mailto:marex@denx.de <mailto:marex@denx.de>> > > <mailto:marex@denx.de <mailto:marex@denx.de> <mailto:marex@denx.de <mailto:marex@denx.de>>>> geschrieben: > > > > On 2/22/19 7:35 AM, Simon Goldschmidt wrote: > > > On Thu, Feb 21, 2019 at 10:56 PM Marek Vasut <marex@denx.de <mailto:marex@denx.de> > <mailto:marex@denx.de <mailto:marex@denx.de>> > > <mailto:marex@denx.de <mailto:marex@denx.de> <mailto:marex@denx.de <mailto:marex@denx.de>>>> wrote: > > >> > > >> On 2/21/19 10:43 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. > > >>> > > >>> Signed-off-by: Simon Goldschmidt > > <simon.k.r.goldschmidt@gmail.com <mailto:simon.k.r.goldschmidt@gmail.com> > <mailto:simon.k.r.goldschmidt@gmail.com <mailto:simon.k.r.goldschmidt@gmail.com>> > > <mailto:simon.k.r.goldschmidt@gmail.com <mailto:simon.k.r.goldschmidt@gmail.com> > <mailto:simon.k.r.goldschmidt@gmail.com <mailto:simon.k.r.goldschmidt@gmail.com>>>> > > >>> --- > > >>> > > >>> Changes in v2: > > >>> - removed Kconfig option OLD_SOCFPGA_KERNEL_COMPAT since > > compatibility > > >>> now uses an environment variable > > >>> > > >>> arch/arm/mach-socfpga/misc_gen5.c | 10 ---------- > > >>> arch/arm/mach-socfpga/spl_gen5.c | 10 ---------- > > >>> 2 files changed, 20 deletions(-) > > >>> > > >>> diff --git a/arch/arm/mach-socfpga/misc_gen5.c > > b/arch/arm/mach-socfpga/misc_gen5.c > > >>> index 6e11ba6cb2..9865f5b5b1 100644 > > >>> --- a/arch/arm/mach-socfpga/misc_gen5.c > > >>> +++ b/arch/arm/mach-socfpga/misc_gen5.c > > >>> @@ -201,16 +201,6 @@ int arch_early_init_r(void) > > >>> /* Add device descriptor to FPGA device table */ > > >>> socfpga_fpga_add(&altera_fpga[0]); > > >>> > > >>> -#ifdef CONFIG_DESIGNWARE_SPI > > >>> - /* Get Designware SPI controller out of reset */ > > >>> - socfpga_per_reset(SOCFPGA_RESET(SPIM0), 0); > > >>> - socfpga_per_reset(SOCFPGA_RESET(SPIM1), 0); > > >>> -#endif > > >>> - > > >>> -#ifdef CONFIG_NAND_DENALI > > >>> - socfpga_per_reset(SOCFPGA_RESET(NAND), 0); > > >>> -#endif > > >>> - > > >>> return 0; > > >>> } > > >>> > > >>> diff --git a/arch/arm/mach-socfpga/spl_gen5.c > > b/arch/arm/mach-socfpga/spl_gen5.c > > >>> index 1bff8cbfcf..e1e65261eb 100644 > > >>> --- a/arch/arm/mach-socfpga/spl_gen5.c > > >>> +++ b/arch/arm/mach-socfpga/spl_gen5.c > > >>> @@ -36,16 +36,12 @@ u32 spl_boot_device(void) > > >>> return BOOT_DEVICE_RAM; > > >>> case 0x2: /* NAND Flash (1.8V) */ > > >>> case 0x3: /* NAND Flash (3.0V) */ > > >>> - socfpga_per_reset(SOCFPGA_RESET(NAND), 0); > > >>> return BOOT_DEVICE_NAND; > > >>> case 0x4: /* SD/MMC External Transceiver (1.8V) */ > > >>> case 0x5: /* SD/MMC Internal Transceiver (3.0V) */ > > >>> - socfpga_per_reset(SOCFPGA_RESET(SDMMC), 0); > > >>> - socfpga_per_reset(SOCFPGA_RESET(DMA), 0); > > >>> return BOOT_DEVICE_MMC1; > > >>> case 0x6: /* QSPI Flash (1.8V) */ > > >>> case 0x7: /* QSPI Flash (3.0V) */ > > >>> - socfpga_per_reset(SOCFPGA_RESET(QSPI), 0); > > >>> return BOOT_DEVICE_SPI; > > >>> default: > > >>> printf("Invalid boot device (bsel=%08x)!\n", > bsel); > > >>> @@ -99,9 +95,7 @@ void board_init_f(ulong dummy) > > >>> socfpga_bridges_reset(1); > > >>> } > > >>> > > >>> - socfpga_per_reset(SOCFPGA_RESET(UART0), 0); > > >>> socfpga_per_reset(SOCFPGA_RESET(OSC1TIMER0), 0); > > >>> - > > >>> timer_init(); > > >>> > > >>> debug("Reconfigure Clock Manager\n"); > > >>> @@ -123,10 +117,6 @@ void board_init_f(ulong dummy) > > >>> sysmgr_pinmux_init(); > > >>> sysmgr_config_warmrstcfgio(0); > > >>> > > >>> - /* De-assert reset for peripherals and bridges based on > > handoff */ > > >>> - reset_deassert_peripherals_handoff(); > > >>> - socfpga_bridges_reset(0); > > > > > > I just saw that this line might have unwanted side effects > in that > > it does > > > not write the enabled bridges bitmask to the 'iswgrp_handoff' > > registers. > > > So the 'bridge enable' command might not work. > > > > > > This whole handoff thing looks somewhat broken to me anyway: the > > > original Altera U-Boot did it because it obeyed Quartus output, > > while we > > > now just always write a constant bitmask here (see > > socfpga_bridges_reset()). > > > > > > Regards, > > > Simon > > > > Might make sense to finally clean it up ? > > > > > > Yeah, well the problem is that for some things, handoff from > Quartus is > > required while for other things, devicetree information like it is now > > is enough. > > Might be just about the right time to convert quartus handoff files > to DT ? > > > That would be great: I do have a use case where the pin description > should be changeable after SPL. Well, we don't have the pin control documentation, so unless we "observe" what quartus generates real hard ...
Sadly, yes. However, I would think it would still be better to embed those u32 arrays we have now into the device tree. That would give us the possibility to apply them from SPL, U-Boot or even Linux.
Well, it's SPL only, it's highly un-recommended to fiddle with the pin configuration at runtime. Although, I suspect that might be just altera being cautious.
Yeah, I would have guesses as long as we change the pin settings without accessing the pins in between, we should probably be good...
My use case is that the pin settings change depending on the FPGA image (e.g. hps pins routed to FPGA etc.). With the current code, I have to update SPL to match the FPGA...
Right, having it in DT, we could have one SPL for multiple systems. Would be nice.
I'll try if I can test that. That would probably result in a pinctrl driver that only supports one preset pin set...
> But that again would require more time than I currently have... :-( Right, I'm not asking you to do it to get this series in.
I didn't mean to say that. But I'd like to have that better sooner than later. Only I can't find the time to do it ...
Right, that's what I meant.
btw you gonna be at EW next week ?
No, unfortunately not.
Regards,
Simon
participants (2)
-
Marek Vasut
-
Simon Goldschmidt