Hi Quentin,
On 7/30/2026 5:19 PM, Quentin Schulz wrote:
> 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)
> ?
Possible, what we really should do is to import hw_bitfield.h from Linux
and start using FIELD_PREP_WM16() instead of using the rk_xregs and
RK_xBITS macros, should be easier now that bitfield.h have been updated.
The rk_xregs and RK_xBITS macros currently have what I consider a design
flaw/limitation in that they have no mask parameter and instead treat
(clr | set) as the mask, meaning you can very easily overflow the
intended field or be incorrectly used to clear one field and set another.
I have been working on a future series that converts RK35xx clock
drivers to use FIELD_GET/PREP, but should probably also change to use
FIELD_PREP_WM16 and drop use of rk_clrsetreg() before posting.
Will see if I can import hw_bitfield.h and change to use FIELD_PREP_WM16
for a v2.
Regards,
Jonas
>
> Cheers,
> Quentin
