All of lore.kernel.org
 help / color / mirror / Atom feed
From: Quentin Schulz <quentin.schulz@cherry.de>
To: Jonas Karlman <jonas@kwiboo.se>
Cc: Kever Yang <kever.yang@rock-chips.com>,
	Tom Rini <trini@konsulko.com>,
	Ilias Apalodimas <ilias.apalodimas@linaro.org>,
	Simon Glass <sjg@chromium.org>,
	u-boot@lists.u-boot-project.org
Subject: Re: [PATCH] rockchip: rk3576: Disable force_jtag by default
Date: Thu, 30 Jul 2026 17:19:01 +0200	[thread overview]
Message-ID: <efa0739a-eeee-4e30-97cb-a54c034e7744@cherry.de> (raw)
In-Reply-To: <62593683-5bb0-4e14-80eb-08239867ee88@kwiboo.se>

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 <jonas@kwiboo.se>
>>> ---
>>>    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

  reply	other threads:[~2026-07-30 15:19 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-30 12:30 [PATCH] rockchip: rk3576: Disable force_jtag by default Jonas Karlman
2026-07-30 14:17 ` Quentin Schulz
2026-07-30 14:35   ` Jonas Karlman
2026-07-30 15:19     ` Quentin Schulz [this message]
2026-07-30 19:29       ` Jonas Karlman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=efa0739a-eeee-4e30-97cb-a54c034e7744@cherry.de \
    --to=quentin.schulz@cherry.de \
    --cc=ilias.apalodimas@linaro.org \
    --cc=jonas@kwiboo.se \
    --cc=kever.yang@rock-chips.com \
    --cc=sjg@chromium.org \
    --cc=trini@konsulko.com \
    --cc=u-boot@lists.u-boot-project.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.