Re: [PATCH] rockchip: rk3576: Disable force_jtag by default

Hi Jonas,

On 7/30/26 4:35 PM, Jonas Karlman wrote:
> Hi Quentin,
> 
> On 7/30/2026 4:17 PM, Quentin Schulz wrote:
>> Hi Jonas,
>>
>> On 7/30/26 2:30 PM, Jonas Karlman wrote:
>>> Rockchip SoCs can automatically switch between jtag and sdmmc based on
>>> the following rules:
>>> - all the SDMMC pins including SDMMC_DET set as SDMMC function in GRF,
>>> - force_jtag bit in GRF is 1,
>>> - SDMMC_DET is low (no card detected),
>>>
>>> Note that the BootROM may mux all SDMMC pins in their SDMMC function or
>>> not, depending on the boot medium that were tried.
>>>
>>> Because SDMMC_DET pin is not guaranteed to be used as an SD card card
>>> detect pin, it could be low at boot or even switch at runtime, which
>>> would enable the jtag function and render the SD card unusable.
>>>
>>> Or boards using cd-gpios may switch the SDMMC_DET pin to GPIO function,
>>> which would enable the jtag function and render the SD card unusable.
>>>
>>> With commit d0a838bdc629 ("Subtree merge tag 'v7.1-dts' of dts repo [1]
>>> into dts/upstream") there are now RK3576 boards that have changed to use
>>> cd-gpios for the SDMMC_DET pin, e.g. NanoPi R76S, that may have issues
>>> detecting SD card unless force_jtag is disabled.
>>>
>>> Signed-off-by: Jonas Karlman <[email protected]>
>>> ---
>>>    arch/arm/mach-rockchip/rk3576/rk3576.c | 7 +++++++
>>>    1 file changed, 7 insertions(+)
>>>
>>> diff --git a/arch/arm/mach-rockchip/rk3576/rk3576.c b/arch/arm/mach-rockchip/rk3576/rk3576.c
>>> index e3e93f663959..0f41f210a9a5 100644
>>> --- a/arch/arm/mach-rockchip/rk3576/rk3576.c
>>> +++ b/arch/arm/mach-rockchip/rk3576/rk3576.c
>>> @@ -26,6 +26,9 @@
>>>    #define SYS_SGRF_SOC_CON15	0x005C
>>>    #define SYS_SGRF_SOC_CON20	0x0070
>>>    
>>> +#define TOP_IOC_BASE		0x26044000
>>> +#define IOC_MISC_CON		0x00F0
>>> +
>>>    #define FW_PMU1SGRF_BASE	0x26003000
>>>    #define PMU1SGRF_SLV_LOOKUP0	0x80
>>>    
>>> @@ -190,6 +193,10 @@ int arch_cpu_init(void)
>>>    	 */
>>>    	writel(0xffffff00, SYS_SGRF_BASE + SYS_SGRF_SOC_CON20);
>>>    
>>> +	/* Disable JTAG exposed on SDMMC pins (GPIO2A2 and GPIO2A3) */
>>> +	if (IS_ENABLED(CONFIG_ROCKCHIP_DISABLE_FORCE_JTAG))
>>> +		writel(0x00020000, TOP_IOC_BASE + IOC_MISC_CON);
>>> +
>>
>> Please:
>> - use a constant (e.g. #define TOP_IOC_FORCE_JTAG BIT(1))
>> - use rk_clrreg(TOP_IOC_BASE + IOC_MISC_CON, TOP_IOC_FORCE_JTAG)
>>
>> It'd be nice to be consistent here and do the same for other writel all
>> over arch/arm/mach-rockchip/ but that's a different kind of task :)
> 
> I know we are inconsistent across multiple SoCs, however in rk3576 we
> are exclusivity using writel() so I decided to continue to use writel()
> for this change for consistency with surrounding code, and therefore
> disagree with your suggested changes :-)
> 
> In my opinion mixed used of both writel() and rk_reg() funcs are worse
> than a consistent use of writel() within same file and function.
> 

I understand but this here is quite misleading as it could be understood 
as "you need to write bit 16 to disable JTAG" which is technically 
correct, but only because bit 1 is 0 and that is the one that actually 
matters.

Can we maybe compromise on using
writel(RK_CLRBITS(TOP_IOC_FORCE_JTAG), TOP_IOC_BASE + IOC_MISC_CON)
?

Cheers,
Quentin