Re: [PATCH RFC 8/8] clk: sunxi-ng: a733: Add reset lines

[Andre Przywara <andre.przywara@arm.com>]

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?

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

> +	[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.

> +	[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.

> +	[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[] = {
>