* Re: [PATCH RFC 8/8] clk: sunxi-ng: a733: Add reset lines
2026-05-13 23:22 ` [PATCH RFC 8/8] clk: sunxi-ng: a733: Add reset lines Andre Przywara
@ 2026-05-14 2:41 ` Junhui Liu
0 siblings, 0 replies; 2+ messages in thread
From: Junhui Liu @ 2026-05-14 2:41 UTC (permalink / raw)
To: Andre Przywara, Junhui Liu
Cc: Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
Philipp Zabel, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Alexandre Ghiti, Richard Cochran, linux-clk, devicetree,
linux-arm-kernel, linux-sunxi, linux-kernel, linux-riscv, netdev
Hi Andre,
Thanks for your review.
On Thu May 14, 2026 at 7:22 AM CST, Andre Przywara wrote:
> On Tue, 10 Mar 2026 16:34:01 +0800
> Junhui Liu <junhui.liu@pigmoral.tech> wrote:
>
> Hi,
>
> compare the list below against my version of the manual. You list more
> than shown there, can you say where those extra reset bits come from?
They are from the vendor BSP, I will add comments to explain them in the
next version.
>
>> Add the reset lines for the Allwinner A733 SoC. These reset control bits
>> are integrated into the Bus Gate Reset (BGR) registers, typically
>> sharing the same register address with their corresponding bus clock
>> gates. Integrate them into the main CCU driver using the existing
>> sunxi-ng ccu_reset framework, allowing the CCU to also function as a
>> reset controller for the SoC.
>>
>> Signed-off-by: Junhui Liu <junhui.liu@pigmoral.tech>
>> ---
>> drivers/clk/sunxi-ng/ccu-sun60i-a733.c | 128 +++++++++++++++++++++++++++++++++
>> 1 file changed, 128 insertions(+)
>>
>> diff --git a/drivers/clk/sunxi-ng/ccu-sun60i-a733.c b/drivers/clk/sunxi-ng/ccu-sun60i-a733.c
>> index c0b09f9197d1..7d1ee9235436 100644
>> --- a/drivers/clk/sunxi-ng/ccu-sun60i-a733.c
>> +++ b/drivers/clk/sunxi-ng/ccu-sun60i-a733.c
>> @@ -17,6 +17,7 @@
>> #include "../clk.h"
>>
>> #include "ccu_common.h"
>> +#include "ccu_reset.h"
>>
>> #include "ccu_div.h"
>> #include "ccu_gate.h"
>> @@ -2169,11 +2170,138 @@ static struct clk_hw_onecell_data sun60i_a733_hw_clks = {
>> .num = CLK_FANOUT3 + 1,
>> };
>>
>> +static struct ccu_reset_map sun60i_a733_ccu_resets[] = {
>> + [RST_BUS_ITS_PCIE] = { 0x574, BIT(16) },
>> + [RST_BUS_NSI] = { 0x580, BIT(30) },
>
> What is this NSI device? Some interconnect? Do we really want to handle
> this reset and clock? Is there some device referencing this? Otherwise
> the kernel will turn at least the clock off, which is probably fatal.
> Also the manual says this is a secure register, so I feel like the
> kernel must not mess with this.
Yes, NSI appears to be a high-speed interconnect. The vendor kernel
provides a sunxi-nsi driver for it accross sun8i, sun55i, and sun60i.
Since the mainline kernel doesn't manage this interconnect for the
already mainlined sun8i and sun55i platforms, I will drop the NSI clocks
and resets in the next version.
>
>> + [RST_BUS_NSI_CFG] = { 0x584, BIT(16) },
>
> Similar here, I guess, though this might not be secure only.
>
>> + [RST_BUS_IOMMU0_SYS] = { 0x58c, BIT(16) },
>> + [RST_BUS_MSI_LITE0_AHB] = { 0x594, BIT(16) },
>> + [RST_BUS_MSI_LITE0_MBUS] = { 0x594, BIT(17) },
>> + [RST_BUS_MSI_LITE1_AHB] = { 0x59c, BIT(16) },
>> + [RST_BUS_MSI_LITE1_MBUS] = { 0x59c, BIT(17) },
>> + [RST_BUS_MSI_LITE2_AHB] = { 0x5a4, BIT(16) },
>> + [RST_BUS_MSI_LITE2_MBUS] = { 0x5a4, BIT(17) },
>> + [RST_BUS_IOMMU1_SYS] = { 0x5b4, BIT(16) },
>> + [RST_BUS_DMA0] = { 0x704, BIT(16) },
>> + [RST_BUS_DMA1] = { 0x70c, BIT(16) },
>> + [RST_BUS_SPINLOCK] = { 0x724, BIT(16) },
>> + [RST_BUS_MSGBOX] = { 0x744, BIT(16) },
>> + [RST_BUS_PWM0] = { 0x784, BIT(16) },
>> + [RST_BUS_PWM1] = { 0x78c, BIT(16) },
>> + [RST_BUS_DBG] = { 0x7a4, BIT(16) },
>> + [RST_BUS_SYSDAP] = { 0x7ac, BIT(16) },
>> + [RST_BUS_TIMER0] = { 0x850, BIT(16) },
>> + [RST_BUS_DE] = { 0xa04, BIT(16) },
>> + [RST_BUS_DI] = { 0xa24, BIT(16) },
>> + [RST_BUS_G2D] = { 0xa44, BIT(16) },
>> + [RST_BUS_EINK] = { 0xa6c, BIT(16) },
>> + [RST_BUS_DE_SYS] = { 0xa74, BIT(16) },
>> + [RST_BUS_VE_ENC] = { 0xa8c, BIT(16) },
>
> The manual calls this ENC0, and since bit 17 is not documented, I
> wonder if there is an ENC1 used with some other packaging, maybe? So
> maybe calling it ENC0 would be safer here.
Okay, I will rename it to ENC0.
>
>> + [RST_BUS_VE_DEC] = { 0xa8c, BIT(18) },
>> + [RST_BUS_CE] = { 0xac4, BIT(16) },
>> + [RST_BUS_CE_SYS] = { 0xac4, BIT(17) },
>> + [RST_BUS_NPU_CORE] = { 0xb04, BIT(16) },
>> + [RST_BUS_NPU_AXI] = { 0xb04, BIT(17) },
>> + [RST_BUS_NPU_AHB] = { 0xb04, BIT(18) },
>> + [RST_BUS_NPU_SRAM] = { 0xb04, BIT(19) },
>> + [RST_BUS_GPU] = { 0xb24, BIT(16) },
>> + [RST_BUS_DRAM] = { 0xc0c, BIT(16) },
>> + [RST_BUS_NAND] = { 0xc8c, BIT(16) },
>> + [RST_BUS_MMC0] = { 0xd0c, BIT(16) },
>> + [RST_BUS_MMC1] = { 0xd1c, BIT(16) },
>> + [RST_BUS_MMC2] = { 0xd2c, BIT(16) },
>> + [RST_BUS_MMC3] = { 0xd3c, BIT(16) },
>> + [RST_BUS_UFS_AHB] = { 0xd8c, BIT(16) },
>> + [RST_BUS_UFS_AXI] = { 0xd8c, BIT(17) },
>> + [RST_BUS_UFS_PHY] = { 0xd8c, BIT(18) },
>> + [RST_BUS_UFS_CORE] = { 0xd8c, BIT(19) },
>> + [RST_BUS_UART0] = { 0xe00, BIT(16) },
>> + [RST_BUS_UART1] = { 0xe04, BIT(16) },
>> + [RST_BUS_UART2] = { 0xe08, BIT(16) },
>> + [RST_BUS_UART3] = { 0xe0c, BIT(16) },
>> + [RST_BUS_UART4] = { 0xe10, BIT(16) },
>> + [RST_BUS_UART5] = { 0xe14, BIT(16) },
>> + [RST_BUS_UART6] = { 0xe18, BIT(16) },
>> + [RST_BUS_I2C0] = { 0xe80, BIT(16) },
>> + [RST_BUS_I2C1] = { 0xe84, BIT(16) },
>> + [RST_BUS_I2C2] = { 0xe88, BIT(16) },
>> + [RST_BUS_I2C3] = { 0xe8c, BIT(16) },
>> + [RST_BUS_I2C4] = { 0xe90, BIT(16) },
>> + [RST_BUS_I2C5] = { 0xe94, BIT(16) },
>> + [RST_BUS_I2C6] = { 0xe98, BIT(16) },
>> + [RST_BUS_I2C7] = { 0xe9c, BIT(16) },
>> + [RST_BUS_I2C8] = { 0xea0, BIT(16) },
>> + [RST_BUS_I2C9] = { 0xea4, BIT(16) },
>> + [RST_BUS_I2C10] = { 0xea8, BIT(16) },
>> + [RST_BUS_I2C11] = { 0xeac, BIT(16) },
>> + [RST_BUS_I2C12] = { 0xeb0, BIT(16) },
>> + [RST_BUS_SPI0] = { 0xf04, BIT(16) },
>> + [RST_BUS_SPI1] = { 0xf0c, BIT(16) },
>> + [RST_BUS_SPI2] = { 0xf14, BIT(16) },
>> + [RST_BUS_SPIF] = { 0xf1c, BIT(16) },
>> + [RST_BUS_SPI3] = { 0xf24, BIT(16) },
>> + [RST_BUS_SPI4] = { 0xf2c, BIT(16) },
>
> SPI4 isn't mentioned in my version of the manual.
>
>> + [RST_BUS_GPADC] = { 0xfc4, BIT(16) },
>> + [RST_BUS_THS] = { 0xfe4, BIT(16) },
>> + [RST_BUS_IRRX] = { 0x1004, BIT(16) },
>> + [RST_BUS_IRTX] = { 0x100c, BIT(16) },
>> + [RST_BUS_LRADC] = { 0x1024, BIT(16) },
>> + [RST_BUS_SGPIO] = { 0x1064, BIT(16) },
>> + [RST_BUS_LPC] = { 0x1084, BIT(16) },
>
> Where do those two come from? There are not in my version of the manual.
>
>> + [RST_BUS_I2SPCM0] = { 0x120c, BIT(16) },
>> + [RST_BUS_I2SPCM1] = { 0x121c, BIT(16) },
>> + [RST_BUS_I2SPCM2] = { 0x122c, BIT(16) },
>> + [RST_BUS_I2SPCM3] = { 0x123c, BIT(16) },
>> + [RST_BUS_I2SPCM4] = { 0x124c, BIT(16) },
>> + [RST_BUS_OWA] = { 0x128c, BIT(16) },
>> + [RST_BUS_DMIC] = { 0x12cc, BIT(16) },
>> + [RST_USB_PHY0] = { 0x1300, BIT(30) },
>> + [RST_BUS_OHCI0] = { 0x1304, BIT(16) },
>> + [RST_BUS_EHCI0] = { 0x1304, BIT(20) },
>> + [RST_BUS_OTG] = { 0x1304, BIT(24) },
>> + [RST_USB_PHY1] = { 0x1308, BIT(30) },
>> + [RST_BUS_OHCI1] = { 0x130c, BIT(16) },
>> + [RST_BUS_EHCI1] = { 0x130c, BIT(20) },
>> + [RST_BUS_USB2] = { 0x135c, BIT(16) },
>> + [RST_BUS_PCIE] = { 0x138c, BIT(17) },
>> + [RST_BUS_PCIE_PWRUP] = { 0x138c, BIT(16) },
>
> Just a nit, but those two are ordered wrongly.
Will do.
>
>> + [RST_BUS_SERDES] = { 0x13c4, BIT(16) },
>> + [RST_BUS_GMAC0] = { 0x141c, BIT(16) },
>> + [RST_BUS_GMAC0_AXI] = { 0x141c, BIT(17) },
>> + [RST_BUS_GMAC1] = { 0x142c, BIT(16) },
>> + [RST_BUS_GMAC1_AXI] = { 0x142c, BIT(17) },
>
> GMAC1 is not listed in my manual, where does this come from?
>
>> + [RST_BUS_TCON_LCD0] = { 0x1504, BIT(16) },
>> + [RST_BUS_TCON_LCD1] = { 0x150c, BIT(16) },
>> + [RST_BUS_TCON_LCD2] = { 0x1514, BIT(16) },
>
> No LCD2 in my manual.
>
> The rest looks alright.
>
> Cheers,
> Andre
>
>> + [RST_BUS_LVDS0] = { 0x1544, BIT(16) },
>> + [RST_BUS_LVDS1] = { 0x154c, BIT(16) },
>> + [RST_BUS_DSI0] = { 0x1584, BIT(16) },
>> + [RST_BUS_DSI1] = { 0x158c, BIT(16) },
>> + [RST_BUS_TCON_TV0] = { 0x1604, BIT(16) },
>> + [RST_BUS_TCON_TV1] = { 0x160c, BIT(16) },
>> + [RST_BUS_EDP] = { 0x164c, BIT(16) },
>> + [RST_BUS_HDMI_MAIN] = { 0x168c, BIT(16) },
>> + [RST_BUS_HDMI_SUB] = { 0x168c, BIT(17) },
>> + [RST_BUS_HDMI_HDCP] = { 0x168c, BIT(18) },
>> + [RST_BUS_DPSS_TOP0] = { 0x16c4, BIT(16) },
>> + [RST_BUS_DPSS_TOP1] = { 0x16cc, BIT(16) },
>> + [RST_BUS_VIDEO_OUT0] = { 0x16e4, BIT(16) },
>> + [RST_BUS_VIDEO_OUT1] = { 0x16ec, BIT(16) },
>> + [RST_BUS_LEDC] = { 0x1704, BIT(16) },
>> + [RST_BUS_DSC] = { 0x1744, BIT(16) },
>> + [RST_BUS_CSI] = { 0x1844, BIT(16) },
>> + [RST_BUS_VIDEO_IN] = { 0x1884, BIT(16) },
>> + [RST_BUS_APB2JTAG] = { 0x1c04, BIT(16) },
>> +};
>> +
>> static const struct sunxi_ccu_desc sun60i_a733_ccu_desc = {
>> .ccu_clks = sun60i_a733_ccu_clks,
>> .num_ccu_clks = ARRAY_SIZE(sun60i_a733_ccu_clks),
>>
>> .hw_clks = &sun60i_a733_hw_clks,
>> +
>> + .resets = sun60i_a733_ccu_resets,
>> + .num_resets = ARRAY_SIZE(sun60i_a733_ccu_resets),
>> };
>>
>> static const u32 pll_regs[] = {
>>
--
Best regards,
Junhui Liu
^ permalink raw reply [flat|nested] 2+ messages in thread