public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 1/2] ARM: socfpga: Actually put bridges into reset on Gen5 in bridge disable
@ 2019-11-20 21:34 Marek Vasut
  2019-11-20 21:34 ` [U-Boot] [PATCH 2/2] ARM: socfpga: Purge pending transactions upon enabling bridges on Gen5 Marek Vasut
  2019-11-21  9:56 ` [U-Boot] [PATCH 1/2] ARM: socfpga: Actually put bridges into reset on Gen5 in bridge disable Tan, Ley Foon
  0 siblings, 2 replies; 6+ messages in thread
From: Marek Vasut @ 2019-11-20 21:34 UTC (permalink / raw)
  To: u-boot

On Gen5, the 'bridge disable' command write 0x0 to brgmodrst register,
which releases all bridges from reset, instead of putting all bridges
into reset. Fix this by inverting the mask and actually putting the
bridges into reset.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Chin Liang See <chin.liang.see@intel.com>
Cc: Dalon Westergreen <dwesterg@gmail.com>
Cc: Dinh Nguyen <dinguyen@kernel.org>
Cc: Ley Foon Tan <ley.foon.tan@intel.com>
Cc: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
Cc: Tien Fong Chee <tien.fong.chee@intel.com>
---
 arch/arm/mach-socfpga/misc_gen5.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-socfpga/misc_gen5.c b/arch/arm/mach-socfpga/misc_gen5.c
index 31681b799d..36f00aee31 100644
--- a/arch/arm/mach-socfpga/misc_gen5.c
+++ b/arch/arm/mach-socfpga/misc_gen5.c
@@ -231,7 +231,7 @@ void do_bridge_reset(int enable, unsigned int mask)
 	} else {
 		writel(0, &sysmgr_regs->fpgaintfgrp_module);
 		writel(0, &sdr_ctrl->fpgaport_rst);
-		writel(0, &reset_manager_base->brg_mod_reset);
+		writel(0x7, &reset_manager_base->brg_mod_reset);
 		writel(1, &nic301_regs->remap);
 	}
 }
-- 
2.24.0.432.g9d3f5f5b63

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

* [U-Boot] [PATCH 2/2] ARM: socfpga: Purge pending transactions upon enabling bridges on Gen5
  2019-11-20 21:34 [U-Boot] [PATCH 1/2] ARM: socfpga: Actually put bridges into reset on Gen5 in bridge disable Marek Vasut
@ 2019-11-20 21:34 ` Marek Vasut
  2019-11-21  9:59   ` Tan, Ley Foon
  2019-11-21  9:56 ` [U-Boot] [PATCH 1/2] ARM: socfpga: Actually put bridges into reset on Gen5 in bridge disable Tan, Ley Foon
  1 sibling, 1 reply; 6+ messages in thread
From: Marek Vasut @ 2019-11-20 21:34 UTC (permalink / raw)
  To: u-boot

On Gen5, when the FPGA is loaded and there was some prior interaction
between the HPS and the FPGA via bridges (e.g. Linux was running and
using some of the IPs in the FPGA) followed by warm reset, it has been
observed that there might be outstanding unfinished transactions. This
leads to an obscure misbehavior of the bridge.

When the bridge is enabled again in U-Boot and there are outstanding
transactions, a read from within the bridge address range would return
a result of the previous read instead. Example:
=> bridge enable ; md 0xff200000 1
ff200000: 1234abcd
=> bridge enable ; md 0xff200010 1
ff200010: 5678dcba <------- this is in fact a value which is stored in
                            a memory@0xff200000
=> bridge enable ; md 0xff200000 1
ff200000: 90effe09 <------- this is in fact a value which is stored in
                            a memory at 0xff200010
and so it continues. Issuing a write does lock the system up completely.

This patch opens the FPGA bridges in 'bridge enable' command, the tears
them down again, and then opens them again. This allows these outstanding
transactions to complete and makes this misbehavior go away.

However, it is not entirely clear whether this is the correct solution.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Chin Liang See <chin.liang.see@intel.com>
Cc: Dalon Westergreen <dwesterg@gmail.com>
Cc: Dinh Nguyen <dinguyen@kernel.org>
Cc: Ley Foon Tan <ley.foon.tan@intel.com>
Cc: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
Cc: Tien Fong Chee <tien.fong.chee@intel.com>
---
 arch/arm/mach-socfpga/misc_gen5.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/mach-socfpga/misc_gen5.c b/arch/arm/mach-socfpga/misc_gen5.c
index 36f00aee31..65d3485bc5 100644
--- a/arch/arm/mach-socfpga/misc_gen5.c
+++ b/arch/arm/mach-socfpga/misc_gen5.c
@@ -228,6 +228,9 @@ void do_bridge_reset(int enable, unsigned int mask)
 		writel(iswgrp_handoff[3], &sdr_ctrl->fpgaport_rst);
 		writel(iswgrp_handoff[0], &reset_manager_base->brg_mod_reset);
 		writel(iswgrp_handoff[1], &nic301_regs->remap);
+
+		writel(0x7, &reset_manager_base->brg_mod_reset);
+		writel(iswgrp_handoff[0], &reset_manager_base->brg_mod_reset);
 	} else {
 		writel(0, &sysmgr_regs->fpgaintfgrp_module);
 		writel(0, &sdr_ctrl->fpgaport_rst);
-- 
2.24.0.432.g9d3f5f5b63

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

* [U-Boot] [PATCH 1/2] ARM: socfpga: Actually put bridges into reset on Gen5 in bridge disable
  2019-11-20 21:34 [U-Boot] [PATCH 1/2] ARM: socfpga: Actually put bridges into reset on Gen5 in bridge disable Marek Vasut
  2019-11-20 21:34 ` [U-Boot] [PATCH 2/2] ARM: socfpga: Purge pending transactions upon enabling bridges on Gen5 Marek Vasut
@ 2019-11-21  9:56 ` Tan, Ley Foon
  2019-11-21 10:26   ` Simon Goldschmidt
  1 sibling, 1 reply; 6+ messages in thread
From: Tan, Ley Foon @ 2019-11-21  9:56 UTC (permalink / raw)
  To: u-boot



> -----Original Message-----
> From: Marek Vasut <marex@denx.de>
> Sent: Thursday, November 21, 2019 5:35 AM
> To: u-boot at lists.denx.de
> Cc: Marek Vasut <marex@denx.de>; See, Chin Liang
> <chin.liang.see@intel.com>; Dalon Westergreen <dwesterg@gmail.com>;
> Dinh Nguyen <dinguyen@kernel.org>; Tan, Ley Foon
> <ley.foon.tan@intel.com>; Simon Goldschmidt
> <simon.k.r.goldschmidt@gmail.com>; Chee, Tien Fong
> <tien.fong.chee@intel.com>
> Subject: [PATCH 1/2] ARM: socfpga: Actually put bridges into reset on Gen5
> in bridge disable
> 
> On Gen5, the 'bridge disable' command write 0x0 to brgmodrst register,
> which releases all bridges from reset, instead of putting all bridges into reset.
> Fix this by inverting the mask and actually putting the bridges into reset.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Chin Liang See <chin.liang.see@intel.com>
> Cc: Dalon Westergreen <dwesterg@gmail.com>
> Cc: Dinh Nguyen <dinguyen@kernel.org>
> Cc: Ley Foon Tan <ley.foon.tan@intel.com>
> Cc: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> Cc: Tien Fong Chee <tien.fong.chee@intel.com>

Reviewed-by: Ley Foon Tan <ley.foon.tan@intel.com>

> ---
>  arch/arm/mach-socfpga/misc_gen5.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-socfpga/misc_gen5.c b/arch/arm/mach-
> socfpga/misc_gen5.c
> index 31681b799d..36f00aee31 100644
> --- a/arch/arm/mach-socfpga/misc_gen5.c
> +++ b/arch/arm/mach-socfpga/misc_gen5.c
> @@ -231,7 +231,7 @@ void do_bridge_reset(int enable, unsigned int mask)
>  	} else {
>  		writel(0, &sysmgr_regs->fpgaintfgrp_module);
>  		writel(0, &sdr_ctrl->fpgaport_rst);
> -		writel(0, &reset_manager_base->brg_mod_reset);
> +		writel(0x7, &reset_manager_base->brg_mod_reset);
>  		writel(1, &nic301_regs->remap);
>  	}
>  }
> --
> 2.24.0.432.g9d3f5f5b63

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

* [U-Boot] [PATCH 2/2] ARM: socfpga: Purge pending transactions upon enabling bridges on Gen5
  2019-11-20 21:34 ` [U-Boot] [PATCH 2/2] ARM: socfpga: Purge pending transactions upon enabling bridges on Gen5 Marek Vasut
@ 2019-11-21  9:59   ` Tan, Ley Foon
  2019-11-21 10:17     ` Marek Vasut
  0 siblings, 1 reply; 6+ messages in thread
From: Tan, Ley Foon @ 2019-11-21  9:59 UTC (permalink / raw)
  To: u-boot



> -----Original Message-----
> From: Marek Vasut <marex@denx.de>
> Sent: Thursday, November 21, 2019 5:35 AM
> To: u-boot at lists.denx.de
> Cc: Marek Vasut <marex@denx.de>; See, Chin Liang
> <chin.liang.see@intel.com>; Dalon Westergreen <dwesterg@gmail.com>;
> Dinh Nguyen <dinguyen@kernel.org>; Tan, Ley Foon
> <ley.foon.tan@intel.com>; Simon Goldschmidt
> <simon.k.r.goldschmidt@gmail.com>; Chee, Tien Fong
> <tien.fong.chee@intel.com>
> Subject: [PATCH 2/2] ARM: socfpga: Purge pending transactions upon
> enabling bridges on Gen5
> 
> On Gen5, when the FPGA is loaded and there was some prior interaction
> between the HPS and the FPGA via bridges (e.g. Linux was running and using
> some of the IPs in the FPGA) followed by warm reset, it has been observed
> that there might be outstanding unfinished transactions. This leads to an
> obscure misbehavior of the bridge.
> 
> When the bridge is enabled again in U-Boot and there are outstanding
> transactions, a read from within the bridge address range would return a
> result of the previous read instead. Example:
> => bridge enable ; md 0xff200000 1
> ff200000: 1234abcd
> => bridge enable ; md 0xff200010 1
> ff200010: 5678dcba <------- this is in fact a value which is stored in
>                             a memory at 0xff200000 => bridge enable ; md 0xff200000 1
> ff200000: 90effe09 <------- this is in fact a value which is stored in
>                             a memory at 0xff200010 and so it continues. Issuing a write
> does lock the system up completely.
> 
> This patch opens the FPGA bridges in 'bridge enable' command, the tears
> them down again, and then opens them again. This allows these outstanding
> transactions to complete and makes this misbehavior go away.
> 
> However, it is not entirely clear whether this is the correct solution.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Chin Liang See <chin.liang.see@intel.com>
> Cc: Dalon Westergreen <dwesterg@gmail.com>
> Cc: Dinh Nguyen <dinguyen@kernel.org>
> Cc: Ley Foon Tan <ley.foon.tan@intel.com>
> Cc: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> Cc: Tien Fong Chee <tien.fong.chee@intel.com>

Reviewed-by: Ley Foon Tan <ley.foon.tan@intel.com>

Regards
Ley Foon

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

* [U-Boot] [PATCH 2/2] ARM: socfpga: Purge pending transactions upon enabling bridges on Gen5
  2019-11-21  9:59   ` Tan, Ley Foon
@ 2019-11-21 10:17     ` Marek Vasut
  0 siblings, 0 replies; 6+ messages in thread
From: Marek Vasut @ 2019-11-21 10:17 UTC (permalink / raw)
  To: u-boot

On 11/21/19 10:59 AM, Tan, Ley Foon wrote:
[...]

Hi,

>> Subject: [PATCH 2/2] ARM: socfpga: Purge pending transactions upon
>> enabling bridges on Gen5
>>
>> On Gen5, when the FPGA is loaded and there was some prior interaction
>> between the HPS and the FPGA via bridges (e.g. Linux was running and using
>> some of the IPs in the FPGA) followed by warm reset, it has been observed
>> that there might be outstanding unfinished transactions. This leads to an
>> obscure misbehavior of the bridge.
>>
>> When the bridge is enabled again in U-Boot and there are outstanding
>> transactions, a read from within the bridge address range would return a
>> result of the previous read instead. Example:
>> => bridge enable ; md 0xff200000 1
>> ff200000: 1234abcd
>> => bridge enable ; md 0xff200010 1
>> ff200010: 5678dcba <------- this is in fact a value which is stored in
>>                             a memory at 0xff200000 => bridge enable ; md 0xff200000 1
>> ff200000: 90effe09 <------- this is in fact a value which is stored in
>>                             a memory at 0xff200010 and so it continues. Issuing a write
>> does lock the system up completely.
>>
>> This patch opens the FPGA bridges in 'bridge enable' command, the tears
>> them down again, and then opens them again. This allows these outstanding
>> transactions to complete and makes this misbehavior go away.
>>
>> However, it is not entirely clear whether this is the correct solution.
>>
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> Cc: Chin Liang See <chin.liang.see@intel.com>
>> Cc: Dalon Westergreen <dwesterg@gmail.com>
>> Cc: Dinh Nguyen <dinguyen@kernel.org>
>> Cc: Ley Foon Tan <ley.foon.tan@intel.com>
>> Cc: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
>> Cc: Tien Fong Chee <tien.fong.chee@intel.com>
> 
> Reviewed-by: Ley Foon Tan <ley.foon.tan@intel.com>

Do you think there is anyone still at Altera/intel who might be able to
tell us what this problem really is about ? And whether there is some
way to purge those stuck transactions without turning the bridges on and
off and then on again ? Or whether there is a way to even verify there
are stuck transactions ?

-- 
Best regards,
Marek Vasut

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

* [U-Boot] [PATCH 1/2] ARM: socfpga: Actually put bridges into reset on Gen5 in bridge disable
  2019-11-21  9:56 ` [U-Boot] [PATCH 1/2] ARM: socfpga: Actually put bridges into reset on Gen5 in bridge disable Tan, Ley Foon
@ 2019-11-21 10:26   ` Simon Goldschmidt
  0 siblings, 0 replies; 6+ messages in thread
From: Simon Goldschmidt @ 2019-11-21 10:26 UTC (permalink / raw)
  To: u-boot

On Thu, Nov 21, 2019 at 10:56 AM Tan, Ley Foon <ley.foon.tan@intel.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Marek Vasut <marex@denx.de>
> > Sent: Thursday, November 21, 2019 5:35 AM
> > To: u-boot at lists.denx.de
> > Cc: Marek Vasut <marex@denx.de>; See, Chin Liang
> > <chin.liang.see@intel.com>; Dalon Westergreen <dwesterg@gmail.com>;
> > Dinh Nguyen <dinguyen@kernel.org>; Tan, Ley Foon
> > <ley.foon.tan@intel.com>; Simon Goldschmidt
> > <simon.k.r.goldschmidt@gmail.com>; Chee, Tien Fong
> > <tien.fong.chee@intel.com>
> > Subject: [PATCH 1/2] ARM: socfpga: Actually put bridges into reset on Gen5
> > in bridge disable
> >
> > On Gen5, the 'bridge disable' command write 0x0 to brgmodrst register,
> > which releases all bridges from reset, instead of putting all bridges into reset.
> > Fix this by inverting the mask and actually putting the bridges into reset.

Haha, I didn't notice that as I only used 'bridge enable' so far :-)

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

> >
> > Signed-off-by: Marek Vasut <marex@denx.de>
> > Cc: Chin Liang See <chin.liang.see@intel.com>
> > Cc: Dalon Westergreen <dwesterg@gmail.com>
> > Cc: Dinh Nguyen <dinguyen@kernel.org>
> > Cc: Ley Foon Tan <ley.foon.tan@intel.com>
> > Cc: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> > Cc: Tien Fong Chee <tien.fong.chee@intel.com>
>
> Reviewed-by: Ley Foon Tan <ley.foon.tan@intel.com>
>
> > ---
> >  arch/arm/mach-socfpga/misc_gen5.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/arm/mach-socfpga/misc_gen5.c b/arch/arm/mach-
> > socfpga/misc_gen5.c
> > index 31681b799d..36f00aee31 100644
> > --- a/arch/arm/mach-socfpga/misc_gen5.c
> > +++ b/arch/arm/mach-socfpga/misc_gen5.c
> > @@ -231,7 +231,7 @@ void do_bridge_reset(int enable, unsigned int mask)
> >       } else {
> >               writel(0, &sysmgr_regs->fpgaintfgrp_module);
> >               writel(0, &sdr_ctrl->fpgaport_rst);
> > -             writel(0, &reset_manager_base->brg_mod_reset);
> > +             writel(0x7, &reset_manager_base->brg_mod_reset);
> >               writel(1, &nic301_regs->remap);
> >       }
> >  }
> > --
> > 2.24.0.432.g9d3f5f5b63
>

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

end of thread, other threads:[~2019-11-21 10:26 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-11-20 21:34 [U-Boot] [PATCH 1/2] ARM: socfpga: Actually put bridges into reset on Gen5 in bridge disable Marek Vasut
2019-11-20 21:34 ` [U-Boot] [PATCH 2/2] ARM: socfpga: Purge pending transactions upon enabling bridges on Gen5 Marek Vasut
2019-11-21  9:59   ` Tan, Ley Foon
2019-11-21 10:17     ` Marek Vasut
2019-11-21  9:56 ` [U-Boot] [PATCH 1/2] ARM: socfpga: Actually put bridges into reset on Gen5 in bridge disable Tan, Ley Foon
2019-11-21 10:26   ` Simon Goldschmidt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox