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 <jo...@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

Reply via email to