All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCHv2] zynqmp: restore the jtag interface
@ 2021-10-13  8:25 Jorge Ramirez-Ortiz
  2021-10-13  8:37 ` Michal Simek
  0 siblings, 1 reply; 5+ messages in thread
From: Jorge Ramirez-Ortiz @ 2021-10-13  8:25 UTC (permalink / raw)
  To: jorge, michal.simek, ashok.reddy.soma, adrian.fiergolski,
	t.karthik.reddy
  Cc: sjg, mike.looijmans, u-boot, igor.opaniuk

When boot.bin is configured for secure boot the CSU will disable the
JTAG interface on all cases.

Some boards might rely on this interface for flashing to QSPI in which
case those systems might end up bricked during development.

This commit will restore the interface under CSU control

Signed-off-by: Jorge Ramirez-Ortiz <jorge@foundries.io>
---
 arch/arm/mach-zynqmp/Kconfig                 |  8 +++++
 arch/arm/mach-zynqmp/include/mach/hardware.h | 31 +++++++++++++++-----
 board/xilinx/zynqmp/zynqmp.c                 | 20 ++++++++++++-
 3 files changed, 51 insertions(+), 8 deletions(-)

diff --git a/arch/arm/mach-zynqmp/Kconfig b/arch/arm/mach-zynqmp/Kconfig
index f7b08db355..ee0895d9a2 100644
--- a/arch/arm/mach-zynqmp/Kconfig
+++ b/arch/arm/mach-zynqmp/Kconfig
@@ -149,6 +149,14 @@ config SPL_ZYNQMP_ALT_BOOTMODE_ENABLED
 	  Overwrite bootmode selected via boot mode pins to tell SPL what should
 	  be the next boot device.
 
+config SPL_ZYNQMP_RESTORE_JTAG
+	bool "Restore JTAG"
+	depends on SPL
+	help
+	 Booting SPL in secure mode causes the CSU to disable the JTAG interface
+	 even if no eFuses were burnt. This option restores the interface if
+	 possible.
+
 config ZYNQ_SDHCI_MAX_FREQ
 	default 200000000
 
diff --git a/arch/arm/mach-zynqmp/include/mach/hardware.h b/arch/arm/mach-zynqmp/include/mach/hardware.h
index eebf38551c..e6a3ee4a57 100644
--- a/arch/arm/mach-zynqmp/include/mach/hardware.h
+++ b/arch/arm/mach-zynqmp/include/mach/hardware.h
@@ -39,20 +39,26 @@
 #define RESET_REASON_INTERNAL	BIT(1)
 #define RESET_REASON_EXTERNAL	BIT(0)
 
+#define CRLAPB_DBG_LPD_CTRL_SETUP_CLK	0x01002002
+#define CRLAPB_RST_LPD_DBG_RESET	0
+
 struct crlapb_regs {
 	u32 reserved0[36];
 	u32 cpu_r5_ctrl; /* 0x90 */
-	u32 reserved1[37];
+	u32 reserved1[7];
+	u32 dbg_lpd_ctrl; /* 0xB0 */
+	u32 reserved2[29];
 	u32 timestamp_ref_ctrl; /* 0x128 */
-	u32 reserved2[53];
+	u32 reserved3[53];
 	u32 boot_mode; /* 0x200 */
-	u32 reserved3_0[7];
+	u32 reserved4_0[7];
 	u32 reset_reason; /* 0x220 */
-	u32 reserved3_1[6];
+	u32 reserved4_1[6];
 	u32 rst_lpd_top; /* 0x23C */
-	u32 reserved4[4];
+	u32 rst_lpd_dbg; /* 0x240 */
+	u32 reserved5[3];
 	u32 boot_pin_ctrl; /* 0x250 */
-	u32 reserved5[21];
+	u32 reserved6[21];
 };
 
 #define crlapb_base ((struct crlapb_regs *)ZYNQMP_CRL_APB_BASEADDR)
@@ -141,12 +147,23 @@ struct apu_regs {
 #define ZYNQMP_SILICON_VER_MASK		0xF
 #define ZYNQMP_SILICON_VER_SHIFT	0
 
+#define CSU_JTAG_SEC_GATE_DISABLE	GENMASK(7, 0)
+#define CSU_JTAG_DAP_ENABLE_DEBUG	GENMASK(7, 0)
+#define CSU_JTAG_CHAIN_WR_SETUP		GENMASK(1, 0)
+#define CSU_PCAP_PROG_RELEASE_PL	BIT(0)
+
 struct csu_regs {
 	u32 reserved0[4];
 	u32 multi_boot;
-	u32 reserved1[11];
+	u32 reserved1[7];
+	u32 jtag_chain_status_wr;
+	u32 jtag_chain_status;
+	u32 jtag_sec;
+	u32 jtag_dap_cfg;
 	u32 idcode;
 	u32 version;
+	u32 reserved2[3055];
+	u32 pcap_prog;
 };
 
 #define csu_base ((struct csu_regs *)ZYNQMP_CSU_BASEADDR)
diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
index 000a7cde8d..483f0b9ab2 100644
--- a/board/xilinx/zynqmp/zynqmp.c
+++ b/board/xilinx/zynqmp/zynqmp.c
@@ -358,6 +358,21 @@ static int multi_boot(void)
 	return multiboot;
 }
 
+#if defined(CONFIG_SPL_BUILD)
+static void restore_jtag(void)
+{
+	if (current_el() != 3)
+		return;
+
+	writel(CSU_JTAG_SEC_GATE_DISABLE, &csu_base->jtag_sec);
+	writel(CSU_JTAG_DAP_ENABLE_DEBUG, &csu_base->jtag_dap_cfg);
+	writel(CSU_JTAG_CHAIN_WR_SETUP, &csu_base->jtag_chain_status_wr);
+	writel(CRLAPB_DBG_LPD_CTRL_SETUP_CLK, &crlapb_base->dbg_lpd_ctrl);
+	writel(CRLAPB_RST_LPD_DBG_RESET, &crlapb_base->rst_lpd_dbg);
+	writel(CSU_PCAP_PROG_RELEASE_PL, &csu_base->pcap_prog);
+}
+#endif
+
 #define PS_SYSMON_ANALOG_BUS_VAL	0x3210
 #define PS_SYSMON_ANALOG_BUS_REG	0xFFA50914
 
@@ -377,11 +392,14 @@ int board_init(void)
 		zynqmp_pmufw_load_config_object(zynqmp_pm_cfg_obj,
 						zynqmp_pm_cfg_obj_size);
 	printf("Silicon version:\t%d\n", zynqmp_get_silicon_version());
+
+	/* the CSU disables the JTAG interface when secure boot is enabled */
+	if (CONFIG_IS_ENABLED(SPL_ZYNQMP_RESTORE_JTAG))
+		restore_jtag();
 #else
 	if (CONFIG_IS_ENABLED(DM_I2C) && CONFIG_IS_ENABLED(I2C_EEPROM))
 		xilinx_read_eeprom();
 #endif
-
 	printf("EL Level:\tEL%d\n", current_el());
 
 	/* Bug in ROM sets wrong value in this register */
-- 
2.31.1


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

* Re: [PATCHv2] zynqmp: restore the jtag interface
  2021-10-13  8:25 [PATCHv2] zynqmp: restore the jtag interface Jorge Ramirez-Ortiz
@ 2021-10-13  8:37 ` Michal Simek
  2021-10-13 11:40   ` Jorge Ramirez-Ortiz, Foundries
  0 siblings, 1 reply; 5+ messages in thread
From: Michal Simek @ 2021-10-13  8:37 UTC (permalink / raw)
  To: Jorge Ramirez-Ortiz, michal.simek, ashok.reddy.soma,
	adrian.fiergolski, t.karthik.reddy
  Cc: sjg, mike.looijmans, u-boot, igor.opaniuk



On 10/13/21 10:25, Jorge Ramirez-Ortiz wrote:
> When boot.bin is configured for secure boot the CSU will disable the
> JTAG interface on all cases.
> 
> Some boards might rely on this interface for flashing to QSPI in which
> case those systems might end up bricked during development.
> 
> This commit will restore the interface under CSU control
> 
> Signed-off-by: Jorge Ramirez-Ortiz <jorge@foundries.io>
> ---

I can't see changelog here. What has changed in 3 minutes between v1 and v2?

>   arch/arm/mach-zynqmp/Kconfig                 |  8 +++++
>   arch/arm/mach-zynqmp/include/mach/hardware.h | 31 +++++++++++++++-----
>   board/xilinx/zynqmp/zynqmp.c                 | 20 ++++++++++++-
>   3 files changed, 51 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/arm/mach-zynqmp/Kconfig b/arch/arm/mach-zynqmp/Kconfig
> index f7b08db355..ee0895d9a2 100644
> --- a/arch/arm/mach-zynqmp/Kconfig
> +++ b/arch/arm/mach-zynqmp/Kconfig
> @@ -149,6 +149,14 @@ config SPL_ZYNQMP_ALT_BOOTMODE_ENABLED
>   	  Overwrite bootmode selected via boot mode pins to tell SPL what should
>   	  be the next boot device.
>   
> +config SPL_ZYNQMP_RESTORE_JTAG
> +	bool "Restore JTAG"
> +	depends on SPL
> +	help
> +	 Booting SPL in secure mode causes the CSU to disable the JTAG interface
> +	 even if no eFuses were burnt. This option restores the interface if
> +	 possible.
> +
>   config ZYNQ_SDHCI_MAX_FREQ
>   	default 200000000
>   
> diff --git a/arch/arm/mach-zynqmp/include/mach/hardware.h b/arch/arm/mach-zynqmp/include/mach/hardware.h
> index eebf38551c..e6a3ee4a57 100644
> --- a/arch/arm/mach-zynqmp/include/mach/hardware.h
> +++ b/arch/arm/mach-zynqmp/include/mach/hardware.h
> @@ -39,20 +39,26 @@
>   #define RESET_REASON_INTERNAL	BIT(1)
>   #define RESET_REASON_EXTERNAL	BIT(0)
>   
> +#define CRLAPB_DBG_LPD_CTRL_SETUP_CLK	0x01002002
> +#define CRLAPB_RST_LPD_DBG_RESET	0
> +
>   struct crlapb_regs {
>   	u32 reserved0[36];
>   	u32 cpu_r5_ctrl; /* 0x90 */
> -	u32 reserved1[37];
> +	u32 reserved1[7];
> +	u32 dbg_lpd_ctrl; /* 0xB0 */
> +	u32 reserved2[29];
>   	u32 timestamp_ref_ctrl; /* 0x128 */
> -	u32 reserved2[53];
> +	u32 reserved3[53];
>   	u32 boot_mode; /* 0x200 */
> -	u32 reserved3_0[7];
> +	u32 reserved4_0[7];
>   	u32 reset_reason; /* 0x220 */
> -	u32 reserved3_1[6];
> +	u32 reserved4_1[6];
>   	u32 rst_lpd_top; /* 0x23C */
> -	u32 reserved4[4];
> +	u32 rst_lpd_dbg; /* 0x240 */
> +	u32 reserved5[3];
>   	u32 boot_pin_ctrl; /* 0x250 */
> -	u32 reserved5[21];
> +	u32 reserved6[21];
>   };
>   
>   #define crlapb_base ((struct crlapb_regs *)ZYNQMP_CRL_APB_BASEADDR)
> @@ -141,12 +147,23 @@ struct apu_regs {
>   #define ZYNQMP_SILICON_VER_MASK		0xF
>   #define ZYNQMP_SILICON_VER_SHIFT	0
>   
> +#define CSU_JTAG_SEC_GATE_DISABLE	GENMASK(7, 0)
> +#define CSU_JTAG_DAP_ENABLE_DEBUG	GENMASK(7, 0)
> +#define CSU_JTAG_CHAIN_WR_SETUP		GENMASK(1, 0)
> +#define CSU_PCAP_PROG_RELEASE_PL	BIT(0)
> +
>   struct csu_regs {
>   	u32 reserved0[4];
>   	u32 multi_boot;
> -	u32 reserved1[11];
> +	u32 reserved1[7];
> +	u32 jtag_chain_status_wr;
> +	u32 jtag_chain_status;
> +	u32 jtag_sec;
> +	u32 jtag_dap_cfg;
>   	u32 idcode;
>   	u32 version;
> +	u32 reserved2[3055];
> +	u32 pcap_prog;
>   };
>   
>   #define csu_base ((struct csu_regs *)ZYNQMP_CSU_BASEADDR)
> diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
> index 000a7cde8d..483f0b9ab2 100644
> --- a/board/xilinx/zynqmp/zynqmp.c
> +++ b/board/xilinx/zynqmp/zynqmp.c
> @@ -358,6 +358,21 @@ static int multi_boot(void)
>   	return multiboot;
>   }
>   
> +#if defined(CONFIG_SPL_BUILD)
> +static void restore_jtag(void)
> +{
> +	if (current_el() != 3)
> +		return;
> +
> +	writel(CSU_JTAG_SEC_GATE_DISABLE, &csu_base->jtag_sec);
> +	writel(CSU_JTAG_DAP_ENABLE_DEBUG, &csu_base->jtag_dap_cfg);
> +	writel(CSU_JTAG_CHAIN_WR_SETUP, &csu_base->jtag_chain_status_wr);
> +	writel(CRLAPB_DBG_LPD_CTRL_SETUP_CLK, &crlapb_base->dbg_lpd_ctrl);
> +	writel(CRLAPB_RST_LPD_DBG_RESET, &crlapb_base->rst_lpd_dbg);
> +	writel(CSU_PCAP_PROG_RELEASE_PL, &csu_base->pcap_prog);
> +}
> +#endif
> +
>   #define PS_SYSMON_ANALOG_BUS_VAL	0x3210
>   #define PS_SYSMON_ANALOG_BUS_REG	0xFFA50914
>   
> @@ -377,11 +392,14 @@ int board_init(void)
>   		zynqmp_pmufw_load_config_object(zynqmp_pm_cfg_obj,
>   						zynqmp_pm_cfg_obj_size);
>   	printf("Silicon version:\t%d\n", zynqmp_get_silicon_version());
> +
> +	/* the CSU disables the JTAG interface when secure boot is enabled */
> +	if (CONFIG_IS_ENABLED(SPL_ZYNQMP_RESTORE_JTAG))
> +		restore_jtag();
>   #else
>   	if (CONFIG_IS_ENABLED(DM_I2C) && CONFIG_IS_ENABLED(I2C_EEPROM))
>   		xilinx_read_eeprom();
>   #endif
> -

Unrelated.

>   	printf("EL Level:\tEL%d\n", current_el());
>   
>   	/* Bug in ROM sets wrong value in this register */
> 

M

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

* Re: [PATCHv2] zynqmp: restore the jtag interface
  2021-10-13  8:37 ` Michal Simek
@ 2021-10-13 11:40   ` Jorge Ramirez-Ortiz, Foundries
  2021-10-13 13:03     ` Michal Simek
  0 siblings, 1 reply; 5+ messages in thread
From: Jorge Ramirez-Ortiz, Foundries @ 2021-10-13 11:40 UTC (permalink / raw)
  To: Michal Simek
  Cc: Jorge Ramirez-Ortiz, ashok.reddy.soma, adrian.fiergolski,
	t.karthik.reddy, sjg, mike.looijmans, u-boot, igor.opaniuk

On 13/10/21, Michal Simek wrote:
> 
> 
> On 10/13/21 10:25, Jorge Ramirez-Ortiz wrote:
> > When boot.bin is configured for secure boot the CSU will disable the
> > JTAG interface on all cases.
> > 
> > Some boards might rely on this interface for flashing to QSPI in which
> > case those systems might end up bricked during development.
> > 
> > This commit will restore the interface under CSU control
> > 
> > Signed-off-by: Jorge Ramirez-Ortiz <jorge@foundries.io>
> > ---
> 
> I can't see changelog here. What has changed in 3 minutes between v1 and v2?

Not in 3 minutes but in 3 months: the first version was submited on
2021-07-16.

The first patch sent this morning was missing the "v2" tag hence why I
resent three minutes later.

v2 addresses the issues you mentioned with the original patch from
July.

	v2
	--
	    Macros added to hardware.h
	    Used CONFIG_IS_ENABLED where possible
	    Fixed Kconfig identation
	    Removed unnecessary Kconfig default

> 
> >   arch/arm/mach-zynqmp/Kconfig                 |  8 +++++
> >   arch/arm/mach-zynqmp/include/mach/hardware.h | 31 +++++++++++++++-----
> >   board/xilinx/zynqmp/zynqmp.c                 | 20 ++++++++++++-
> >   3 files changed, 51 insertions(+), 8 deletions(-)
> > 
> > diff --git a/arch/arm/mach-zynqmp/Kconfig b/arch/arm/mach-zynqmp/Kconfig
> > index f7b08db355..ee0895d9a2 100644
> > --- a/arch/arm/mach-zynqmp/Kconfig
> > +++ b/arch/arm/mach-zynqmp/Kconfig
> > @@ -149,6 +149,14 @@ config SPL_ZYNQMP_ALT_BOOTMODE_ENABLED
> >   	  Overwrite bootmode selected via boot mode pins to tell SPL what should
> >   	  be the next boot device.
> > +config SPL_ZYNQMP_RESTORE_JTAG
> > +	bool "Restore JTAG"
> > +	depends on SPL
> > +	help
> > +	 Booting SPL in secure mode causes the CSU to disable the JTAG interface
> > +	 even if no eFuses were burnt. This option restores the interface if
> > +	 possible.
> > +
> >   config ZYNQ_SDHCI_MAX_FREQ
> >   	default 200000000
> > diff --git a/arch/arm/mach-zynqmp/include/mach/hardware.h b/arch/arm/mach-zynqmp/include/mach/hardware.h
> > index eebf38551c..e6a3ee4a57 100644
> > --- a/arch/arm/mach-zynqmp/include/mach/hardware.h
> > +++ b/arch/arm/mach-zynqmp/include/mach/hardware.h
> > @@ -39,20 +39,26 @@
> >   #define RESET_REASON_INTERNAL	BIT(1)
> >   #define RESET_REASON_EXTERNAL	BIT(0)
> > +#define CRLAPB_DBG_LPD_CTRL_SETUP_CLK	0x01002002
> > +#define CRLAPB_RST_LPD_DBG_RESET	0
> > +
> >   struct crlapb_regs {
> >   	u32 reserved0[36];
> >   	u32 cpu_r5_ctrl; /* 0x90 */
> > -	u32 reserved1[37];
> > +	u32 reserved1[7];
> > +	u32 dbg_lpd_ctrl; /* 0xB0 */
> > +	u32 reserved2[29];
> >   	u32 timestamp_ref_ctrl; /* 0x128 */
> > -	u32 reserved2[53];
> > +	u32 reserved3[53];
> >   	u32 boot_mode; /* 0x200 */
> > -	u32 reserved3_0[7];
> > +	u32 reserved4_0[7];
> >   	u32 reset_reason; /* 0x220 */
> > -	u32 reserved3_1[6];
> > +	u32 reserved4_1[6];
> >   	u32 rst_lpd_top; /* 0x23C */
> > -	u32 reserved4[4];
> > +	u32 rst_lpd_dbg; /* 0x240 */
> > +	u32 reserved5[3];
> >   	u32 boot_pin_ctrl; /* 0x250 */
> > -	u32 reserved5[21];
> > +	u32 reserved6[21];
> >   };
> >   #define crlapb_base ((struct crlapb_regs *)ZYNQMP_CRL_APB_BASEADDR)
> > @@ -141,12 +147,23 @@ struct apu_regs {
> >   #define ZYNQMP_SILICON_VER_MASK		0xF
> >   #define ZYNQMP_SILICON_VER_SHIFT	0
> > +#define CSU_JTAG_SEC_GATE_DISABLE	GENMASK(7, 0)
> > +#define CSU_JTAG_DAP_ENABLE_DEBUG	GENMASK(7, 0)
> > +#define CSU_JTAG_CHAIN_WR_SETUP		GENMASK(1, 0)
> > +#define CSU_PCAP_PROG_RELEASE_PL	BIT(0)
> > +
> >   struct csu_regs {
> >   	u32 reserved0[4];
> >   	u32 multi_boot;
> > -	u32 reserved1[11];
> > +	u32 reserved1[7];
> > +	u32 jtag_chain_status_wr;
> > +	u32 jtag_chain_status;
> > +	u32 jtag_sec;
> > +	u32 jtag_dap_cfg;
> >   	u32 idcode;
> >   	u32 version;
> > +	u32 reserved2[3055];
> > +	u32 pcap_prog;
> >   };
> >   #define csu_base ((struct csu_regs *)ZYNQMP_CSU_BASEADDR)
> > diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
> > index 000a7cde8d..483f0b9ab2 100644
> > --- a/board/xilinx/zynqmp/zynqmp.c
> > +++ b/board/xilinx/zynqmp/zynqmp.c
> > @@ -358,6 +358,21 @@ static int multi_boot(void)
> >   	return multiboot;
> >   }
> > +#if defined(CONFIG_SPL_BUILD)
> > +static void restore_jtag(void)
> > +{
> > +	if (current_el() != 3)
> > +		return;
> > +
> > +	writel(CSU_JTAG_SEC_GATE_DISABLE, &csu_base->jtag_sec);
> > +	writel(CSU_JTAG_DAP_ENABLE_DEBUG, &csu_base->jtag_dap_cfg);
> > +	writel(CSU_JTAG_CHAIN_WR_SETUP, &csu_base->jtag_chain_status_wr);
> > +	writel(CRLAPB_DBG_LPD_CTRL_SETUP_CLK, &crlapb_base->dbg_lpd_ctrl);
> > +	writel(CRLAPB_RST_LPD_DBG_RESET, &crlapb_base->rst_lpd_dbg);
> > +	writel(CSU_PCAP_PROG_RELEASE_PL, &csu_base->pcap_prog);
> > +}
> > +#endif
> > +
> >   #define PS_SYSMON_ANALOG_BUS_VAL	0x3210
> >   #define PS_SYSMON_ANALOG_BUS_REG	0xFFA50914
> > @@ -377,11 +392,14 @@ int board_init(void)
> >   		zynqmp_pmufw_load_config_object(zynqmp_pm_cfg_obj,
> >   						zynqmp_pm_cfg_obj_size);
> >   	printf("Silicon version:\t%d\n", zynqmp_get_silicon_version());
> > +
> > +	/* the CSU disables the JTAG interface when secure boot is enabled */
> > +	if (CONFIG_IS_ENABLED(SPL_ZYNQMP_RESTORE_JTAG))
> > +		restore_jtag();
> >   #else
> >   	if (CONFIG_IS_ENABLED(DM_I2C) && CONFIG_IS_ENABLED(I2C_EEPROM))
> >   		xilinx_read_eeprom();
> >   #endif
> > -
> 
> Unrelated.

?
checkpatch will fail if we leave two carriage returns

> 
> >   	printf("EL Level:\tEL%d\n", current_el());
> >   	/* Bug in ROM sets wrong value in this register */
> > 
> 
> M

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

* Re: [PATCHv2] zynqmp: restore the jtag interface
  2021-10-13 11:40   ` Jorge Ramirez-Ortiz, Foundries
@ 2021-10-13 13:03     ` Michal Simek
  2021-10-13 13:49       ` Jorge Ramirez-Ortiz, Foundries
  0 siblings, 1 reply; 5+ messages in thread
From: Michal Simek @ 2021-10-13 13:03 UTC (permalink / raw)
  To: Jorge Ramirez-Ortiz, Foundries, Michal Simek
  Cc: ashok.reddy.soma, adrian.fiergolski, t.karthik.reddy, sjg,
	mike.looijmans, u-boot, igor.opaniuk



On 10/13/21 13:40, Jorge Ramirez-Ortiz, Foundries wrote:
> On 13/10/21, Michal Simek wrote:
>>
>>
>> On 10/13/21 10:25, Jorge Ramirez-Ortiz wrote:
>>> When boot.bin is configured for secure boot the CSU will disable the
>>> JTAG interface on all cases.
>>>
>>> Some boards might rely on this interface for flashing to QSPI in which
>>> case those systems might end up bricked during development.
>>>
>>> This commit will restore the interface under CSU control
>>>
>>> Signed-off-by: Jorge Ramirez-Ortiz <jorge@foundries.io>
>>> ---
>>
>> I can't see changelog here. What has changed in 3 minutes between v1 and v2?
> 
> Not in 3 minutes but in 3 months: the first version was submited on
> 2021-07-16.
> 
> The first patch sent this morning was missing the "v2" tag hence why I
> resent three minutes later.

I didn't compare them. Just got two patches. One which looked as v1 and 
second as v2.

https://lists.denx.de/pipermail/u-boot/2021-October/thread.html


> 
> v2 addresses the issues you mentioned with the original patch from
> July.
> 
> 	v2
> 	--
> 	    Macros added to hardware.h
> 	    Used CONFIG_IS_ENABLED where possible
> 	    Fixed Kconfig identation
> 	    Removed unnecessary Kconfig default
> 
>>
>>>    arch/arm/mach-zynqmp/Kconfig                 |  8 +++++
>>>    arch/arm/mach-zynqmp/include/mach/hardware.h | 31 +++++++++++++++-----
>>>    board/xilinx/zynqmp/zynqmp.c                 | 20 ++++++++++++-
>>>    3 files changed, 51 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-zynqmp/Kconfig b/arch/arm/mach-zynqmp/Kconfig
>>> index f7b08db355..ee0895d9a2 100644
>>> --- a/arch/arm/mach-zynqmp/Kconfig
>>> +++ b/arch/arm/mach-zynqmp/Kconfig
>>> @@ -149,6 +149,14 @@ config SPL_ZYNQMP_ALT_BOOTMODE_ENABLED
>>>    	  Overwrite bootmode selected via boot mode pins to tell SPL what should
>>>    	  be the next boot device.
>>> +config SPL_ZYNQMP_RESTORE_JTAG
>>> +	bool "Restore JTAG"
>>> +	depends on SPL
>>> +	help
>>> +	 Booting SPL in secure mode causes the CSU to disable the JTAG interface
>>> +	 even if no eFuses were burnt. This option restores the interface if
>>> +	 possible.
>>> +
>>>    config ZYNQ_SDHCI_MAX_FREQ
>>>    	default 200000000
>>> diff --git a/arch/arm/mach-zynqmp/include/mach/hardware.h b/arch/arm/mach-zynqmp/include/mach/hardware.h
>>> index eebf38551c..e6a3ee4a57 100644
>>> --- a/arch/arm/mach-zynqmp/include/mach/hardware.h
>>> +++ b/arch/arm/mach-zynqmp/include/mach/hardware.h
>>> @@ -39,20 +39,26 @@
>>>    #define RESET_REASON_INTERNAL	BIT(1)
>>>    #define RESET_REASON_EXTERNAL	BIT(0)
>>> +#define CRLAPB_DBG_LPD_CTRL_SETUP_CLK	0x01002002
>>> +#define CRLAPB_RST_LPD_DBG_RESET	0
>>> +
>>>    struct crlapb_regs {
>>>    	u32 reserved0[36];
>>>    	u32 cpu_r5_ctrl; /* 0x90 */
>>> -	u32 reserved1[37];
>>> +	u32 reserved1[7];
>>> +	u32 dbg_lpd_ctrl; /* 0xB0 */
>>> +	u32 reserved2[29];
>>>    	u32 timestamp_ref_ctrl; /* 0x128 */
>>> -	u32 reserved2[53];
>>> +	u32 reserved3[53];
>>>    	u32 boot_mode; /* 0x200 */
>>> -	u32 reserved3_0[7];
>>> +	u32 reserved4_0[7];
>>>    	u32 reset_reason; /* 0x220 */
>>> -	u32 reserved3_1[6];
>>> +	u32 reserved4_1[6];
>>>    	u32 rst_lpd_top; /* 0x23C */
>>> -	u32 reserved4[4];
>>> +	u32 rst_lpd_dbg; /* 0x240 */
>>> +	u32 reserved5[3];
>>>    	u32 boot_pin_ctrl; /* 0x250 */
>>> -	u32 reserved5[21];
>>> +	u32 reserved6[21];
>>>    };
>>>    #define crlapb_base ((struct crlapb_regs *)ZYNQMP_CRL_APB_BASEADDR)
>>> @@ -141,12 +147,23 @@ struct apu_regs {
>>>    #define ZYNQMP_SILICON_VER_MASK		0xF
>>>    #define ZYNQMP_SILICON_VER_SHIFT	0
>>> +#define CSU_JTAG_SEC_GATE_DISABLE	GENMASK(7, 0)
>>> +#define CSU_JTAG_DAP_ENABLE_DEBUG	GENMASK(7, 0)
>>> +#define CSU_JTAG_CHAIN_WR_SETUP		GENMASK(1, 0)
>>> +#define CSU_PCAP_PROG_RELEASE_PL	BIT(0)
>>> +
>>>    struct csu_regs {
>>>    	u32 reserved0[4];
>>>    	u32 multi_boot;
>>> -	u32 reserved1[11];
>>> +	u32 reserved1[7];
>>> +	u32 jtag_chain_status_wr;
>>> +	u32 jtag_chain_status;
>>> +	u32 jtag_sec;
>>> +	u32 jtag_dap_cfg;
>>>    	u32 idcode;
>>>    	u32 version;
>>> +	u32 reserved2[3055];
>>> +	u32 pcap_prog;
>>>    };
>>>    #define csu_base ((struct csu_regs *)ZYNQMP_CSU_BASEADDR)
>>> diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
>>> index 000a7cde8d..483f0b9ab2 100644
>>> --- a/board/xilinx/zynqmp/zynqmp.c
>>> +++ b/board/xilinx/zynqmp/zynqmp.c
>>> @@ -358,6 +358,21 @@ static int multi_boot(void)
>>>    	return multiboot;
>>>    }
>>> +#if defined(CONFIG_SPL_BUILD)
>>> +static void restore_jtag(void)
>>> +{
>>> +	if (current_el() != 3)
>>> +		return;
>>> +
>>> +	writel(CSU_JTAG_SEC_GATE_DISABLE, &csu_base->jtag_sec);
>>> +	writel(CSU_JTAG_DAP_ENABLE_DEBUG, &csu_base->jtag_dap_cfg);
>>> +	writel(CSU_JTAG_CHAIN_WR_SETUP, &csu_base->jtag_chain_status_wr);
>>> +	writel(CRLAPB_DBG_LPD_CTRL_SETUP_CLK, &crlapb_base->dbg_lpd_ctrl);
>>> +	writel(CRLAPB_RST_LPD_DBG_RESET, &crlapb_base->rst_lpd_dbg);
>>> +	writel(CSU_PCAP_PROG_RELEASE_PL, &csu_base->pcap_prog);
>>> +}
>>> +#endif
>>> +
>>>    #define PS_SYSMON_ANALOG_BUS_VAL	0x3210
>>>    #define PS_SYSMON_ANALOG_BUS_REG	0xFFA50914
>>> @@ -377,11 +392,14 @@ int board_init(void)
>>>    		zynqmp_pmufw_load_config_object(zynqmp_pm_cfg_obj,
>>>    						zynqmp_pm_cfg_obj_size);
>>>    	printf("Silicon version:\t%d\n", zynqmp_get_silicon_version());
>>> +
>>> +	/* the CSU disables the JTAG interface when secure boot is enabled */
>>> +	if (CONFIG_IS_ENABLED(SPL_ZYNQMP_RESTORE_JTAG))
>>> +		restore_jtag();
>>>    #else
>>>    	if (CONFIG_IS_ENABLED(DM_I2C) && CONFIG_IS_ENABLED(I2C_EEPROM))
>>>    		xilinx_read_eeprom();
>>>    #endif
>>> -
>>
>> Unrelated.
> 
> ?
> checkpatch will fail if we leave two carriage returns

What is the exact issue are you getting?

M

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

* Re: [PATCHv2] zynqmp: restore the jtag interface
  2021-10-13 13:03     ` Michal Simek
@ 2021-10-13 13:49       ` Jorge Ramirez-Ortiz, Foundries
  0 siblings, 0 replies; 5+ messages in thread
From: Jorge Ramirez-Ortiz, Foundries @ 2021-10-13 13:49 UTC (permalink / raw)
  To: Michal Simek
  Cc: Jorge Ramirez-Ortiz, Foundries, ashok.reddy.soma,
	adrian.fiergolski, t.karthik.reddy, sjg, mike.looijmans, u-boot,
	igor.opaniuk

On 13/10/21, Michal Simek wrote:
> 
> 
> On 10/13/21 13:40, Jorge Ramirez-Ortiz, Foundries wrote:
> > On 13/10/21, Michal Simek wrote:
> > > 
> > > 
> > > On 10/13/21 10:25, Jorge Ramirez-Ortiz wrote:
> > > > When boot.bin is configured for secure boot the CSU will disable the
> > > > JTAG interface on all cases.
> > > > 
> > > > Some boards might rely on this interface for flashing to QSPI in which
> > > > case those systems might end up bricked during development.
> > > > 
> > > > This commit will restore the interface under CSU control
> > > > 
> > > > Signed-off-by: Jorge Ramirez-Ortiz <jorge@foundries.io>
> > > > ---
> > > 
> > > I can't see changelog here. What has changed in 3 minutes between v1 and v2?
> > 
> > Not in 3 minutes but in 3 months: the first version was submited on
> > 2021-07-16.
> > 
> > The first patch sent this morning was missing the "v2" tag hence why I
> > resent three minutes later.
> 
> I didn't compare them. Just got two patches. One which looked as v1 and
> second as v2.
> 
> https://lists.denx.de/pipermail/u-boot/2021-October/thread.html
> 
> 
> > 
> > v2 addresses the issues you mentioned with the original patch from
> > July.
> > 
> > 	v2
> > 	--
> > 	    Macros added to hardware.h
> > 	    Used CONFIG_IS_ENABLED where possible
> > 	    Fixed Kconfig identation
> > 	    Removed unnecessary Kconfig default
> > 
> > > 
> > > >    arch/arm/mach-zynqmp/Kconfig                 |  8 +++++
> > > >    arch/arm/mach-zynqmp/include/mach/hardware.h | 31 +++++++++++++++-----
> > > >    board/xilinx/zynqmp/zynqmp.c                 | 20 ++++++++++++-
> > > >    3 files changed, 51 insertions(+), 8 deletions(-)
> > > > 
> > > > diff --git a/arch/arm/mach-zynqmp/Kconfig b/arch/arm/mach-zynqmp/Kconfig
> > > > index f7b08db355..ee0895d9a2 100644
> > > > --- a/arch/arm/mach-zynqmp/Kconfig
> > > > +++ b/arch/arm/mach-zynqmp/Kconfig
> > > > @@ -149,6 +149,14 @@ config SPL_ZYNQMP_ALT_BOOTMODE_ENABLED
> > > >    	  Overwrite bootmode selected via boot mode pins to tell SPL what should
> > > >    	  be the next boot device.
> > > > +config SPL_ZYNQMP_RESTORE_JTAG
> > > > +	bool "Restore JTAG"
> > > > +	depends on SPL
> > > > +	help
> > > > +	 Booting SPL in secure mode causes the CSU to disable the JTAG interface
> > > > +	 even if no eFuses were burnt. This option restores the interface if
> > > > +	 possible.
> > > > +
> > > >    config ZYNQ_SDHCI_MAX_FREQ
> > > >    	default 200000000
> > > > diff --git a/arch/arm/mach-zynqmp/include/mach/hardware.h b/arch/arm/mach-zynqmp/include/mach/hardware.h
> > > > index eebf38551c..e6a3ee4a57 100644
> > > > --- a/arch/arm/mach-zynqmp/include/mach/hardware.h
> > > > +++ b/arch/arm/mach-zynqmp/include/mach/hardware.h
> > > > @@ -39,20 +39,26 @@
> > > >    #define RESET_REASON_INTERNAL	BIT(1)
> > > >    #define RESET_REASON_EXTERNAL	BIT(0)
> > > > +#define CRLAPB_DBG_LPD_CTRL_SETUP_CLK	0x01002002
> > > > +#define CRLAPB_RST_LPD_DBG_RESET	0
> > > > +
> > > >    struct crlapb_regs {
> > > >    	u32 reserved0[36];
> > > >    	u32 cpu_r5_ctrl; /* 0x90 */
> > > > -	u32 reserved1[37];
> > > > +	u32 reserved1[7];
> > > > +	u32 dbg_lpd_ctrl; /* 0xB0 */
> > > > +	u32 reserved2[29];
> > > >    	u32 timestamp_ref_ctrl; /* 0x128 */
> > > > -	u32 reserved2[53];
> > > > +	u32 reserved3[53];
> > > >    	u32 boot_mode; /* 0x200 */
> > > > -	u32 reserved3_0[7];
> > > > +	u32 reserved4_0[7];
> > > >    	u32 reset_reason; /* 0x220 */
> > > > -	u32 reserved3_1[6];
> > > > +	u32 reserved4_1[6];
> > > >    	u32 rst_lpd_top; /* 0x23C */
> > > > -	u32 reserved4[4];
> > > > +	u32 rst_lpd_dbg; /* 0x240 */
> > > > +	u32 reserved5[3];
> > > >    	u32 boot_pin_ctrl; /* 0x250 */
> > > > -	u32 reserved5[21];
> > > > +	u32 reserved6[21];
> > > >    };
> > > >    #define crlapb_base ((struct crlapb_regs *)ZYNQMP_CRL_APB_BASEADDR)
> > > > @@ -141,12 +147,23 @@ struct apu_regs {
> > > >    #define ZYNQMP_SILICON_VER_MASK		0xF
> > > >    #define ZYNQMP_SILICON_VER_SHIFT	0
> > > > +#define CSU_JTAG_SEC_GATE_DISABLE	GENMASK(7, 0)
> > > > +#define CSU_JTAG_DAP_ENABLE_DEBUG	GENMASK(7, 0)
> > > > +#define CSU_JTAG_CHAIN_WR_SETUP		GENMASK(1, 0)
> > > > +#define CSU_PCAP_PROG_RELEASE_PL	BIT(0)
> > > > +
> > > >    struct csu_regs {
> > > >    	u32 reserved0[4];
> > > >    	u32 multi_boot;
> > > > -	u32 reserved1[11];
> > > > +	u32 reserved1[7];
> > > > +	u32 jtag_chain_status_wr;
> > > > +	u32 jtag_chain_status;
> > > > +	u32 jtag_sec;
> > > > +	u32 jtag_dap_cfg;
> > > >    	u32 idcode;
> > > >    	u32 version;
> > > > +	u32 reserved2[3055];
> > > > +	u32 pcap_prog;
> > > >    };
> > > >    #define csu_base ((struct csu_regs *)ZYNQMP_CSU_BASEADDR)
> > > > diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
> > > > index 000a7cde8d..483f0b9ab2 100644
> > > > --- a/board/xilinx/zynqmp/zynqmp.c
> > > > +++ b/board/xilinx/zynqmp/zynqmp.c
> > > > @@ -358,6 +358,21 @@ static int multi_boot(void)
> > > >    	return multiboot;
> > > >    }
> > > > +#if defined(CONFIG_SPL_BUILD)
> > > > +static void restore_jtag(void)
> > > > +{
> > > > +	if (current_el() != 3)
> > > > +		return;
> > > > +
> > > > +	writel(CSU_JTAG_SEC_GATE_DISABLE, &csu_base->jtag_sec);
> > > > +	writel(CSU_JTAG_DAP_ENABLE_DEBUG, &csu_base->jtag_dap_cfg);
> > > > +	writel(CSU_JTAG_CHAIN_WR_SETUP, &csu_base->jtag_chain_status_wr);
> > > > +	writel(CRLAPB_DBG_LPD_CTRL_SETUP_CLK, &crlapb_base->dbg_lpd_ctrl);
> > > > +	writel(CRLAPB_RST_LPD_DBG_RESET, &crlapb_base->rst_lpd_dbg);
> > > > +	writel(CSU_PCAP_PROG_RELEASE_PL, &csu_base->pcap_prog);
> > > > +}
> > > > +#endif
> > > > +
> > > >    #define PS_SYSMON_ANALOG_BUS_VAL	0x3210
> > > >    #define PS_SYSMON_ANALOG_BUS_REG	0xFFA50914
> > > > @@ -377,11 +392,14 @@ int board_init(void)
> > > >    		zynqmp_pmufw_load_config_object(zynqmp_pm_cfg_obj,
> > > >    						zynqmp_pm_cfg_obj_size);
> > > >    	printf("Silicon version:\t%d\n", zynqmp_get_silicon_version());
> > > > +
> > > > +	/* the CSU disables the JTAG interface when secure boot is enabled */
> > > > +	if (CONFIG_IS_ENABLED(SPL_ZYNQMP_RESTORE_JTAG))
> > > > +		restore_jtag();
> > > >    #else
> > > >    	if (CONFIG_IS_ENABLED(DM_I2C) && CONFIG_IS_ENABLED(I2C_EEPROM))
> > > >    		xilinx_read_eeprom();
> > > >    #endif
> > > > -
> > > 
> > > Unrelated.
> > 
> > ?
> > checkpatch will fail if we leave two carriage returns
> 
> What is the exact issue are you getting?

um, must have been an error on my side.
sent v3 deleting the removal of the unrelated empty line
thanks

> 
> M

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

end of thread, other threads:[~2021-10-13 13:49 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-13  8:25 [PATCHv2] zynqmp: restore the jtag interface Jorge Ramirez-Ortiz
2021-10-13  8:37 ` Michal Simek
2021-10-13 11:40   ` Jorge Ramirez-Ortiz, Foundries
2021-10-13 13:03     ` Michal Simek
2021-10-13 13:49       ` Jorge Ramirez-Ortiz, Foundries

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