[PATCHv2] zynqmp: restore the jtag interface

[Michal Simek]

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 at 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