Re: [PATCH RESEND 4/7] clk: spacemit: define existing syscon resets

Alex Elder
2025-03-23T13:23:41Z

On 3/22/25 11:29 AM, Yixun Lan wrote:
> On 10:18 Fri 21 Mar , Alex Elder wrote:
>> Define reset controls associated with the MPMU, APBC, and APMU
>> SpacemiT K1 CCUs. These already have clocks associated with them.
>>
>> Signed-off-by: Alex Elder <elder@riscstar.com>
>> ---
>> drivers/clk/spacemit/ccu-k1.c | 132 ++++++++++++++++++++++++++++++++++
>> 1 file changed, 132 insertions(+)
>>
>> diff --git a/drivers/clk/spacemit/ccu-k1.c b/drivers/clk/spacemit/ccu-k1.c
>> index 6d879411c6c05..be8abd27753cb 100644
>> --- a/drivers/clk/spacemit/ccu-k1.c
>> +++ b/drivers/clk/spacemit/ccu-k1.c
> ..
>> +static const struct ccu_reset_data apmu_reset_data[] = {
>> + [RST_CCIC_4X] = RST_DATA(APMU_CCIC_CLK_RES_CTRL, 0, BIT(1)),
>> + [RST_CCIC1_PHY] = RST_DATA(APMU_CCIC_CLK_RES_CTRL, 0, BIT(2)),
>> + [RST_SDH_AXI] = RST_DATA(APMU_SDH0_CLK_RES_CTRL, 0, BIT(0)),
>> + [RST_SDH0] = RST_DATA(APMU_SDH0_CLK_RES_CTRL, 0, BIT(1)),
>> + [RST_SDH1] = RST_DATA(APMU_SDH1_CLK_RES_CTRL, 0, BIT(1)),
>> + [RST_SDH2] = RST_DATA(APMU_SDH2_CLK_RES_CTRL, 0, BIT(1)),
>> + [RST_USBP1_AXI] = RST_DATA(APMU_USB_CLK_RES_CTRL, 0, BIT(4)),
>> + [RST_USB_AXI] = RST_DATA(APMU_USB_CLK_RES_CTRL, 0, BIT(0)),
> ..
>> + [RST_USB3_0] = RST_DATA(APMU_USB_CLK_RES_CTRL, 0,
>> + BIT(9)|BIT(10)|BIT(11)),
> 100 column if possible, also add one space between "BIT(9) | BIT(10) .."
> continuous bits can just use GENMASK for short?

You'll notice that every place that has multiple bits in the mask
also have a line break. I kind of liked that as a way to highlight
that fact (i.e., it goes beyond my preference for 80 columns).

I will definitely add spaces, that was a mistake not to.

I will not define this with GENMASK(). In this case each bit
represents a single reset, and so each one is significant on
its own. It is *not* a mask of contiguous bits, it's a set
of bits that happen to have consecutive positions.

-Alex

> but may result slightly unreadable, anyway, either way is fine by me
>
>> + [RST_QSPI] = RST_DATA(APMU_QSPI_CLK_RES_CTRL, 0, BIT(1)),
>> + [RST_QSPI_BUS] = RST_DATA(APMU_QSPI_CLK_RES_CTRL, 0, BIT(0)),
>> + [RST_DMA] = RST_DATA(APMU_DMA_CLK_RES_CTRL, 0, BIT(0)),
>> + [RST_AES] = RST_DATA(APMU_AES_CLK_RES_CTRL, 0, BIT(4)),
>> + [RST_VPU] = RST_DATA(APMU_VPU_CLK_RES_CTRL, 0, BIT(0)),
>> + [RST_GPU] = RST_DATA(APMU_GPU_CLK_RES_CTRL, 0, BIT(1)),
>> + [RST_EMMC] = RST_DATA(APMU_PMUA_EM_CLK_RES_CTRL, 0, BIT(1)),
>> + [RST_EMMC_X] = RST_DATA(APMU_PMUA_EM_CLK_RES_CTRL, 0, BIT(0)),
>> + [RST_AUDIO] = RST_DATA(APMU_AUDIO_CLK_RES_CTRL, 0,
>> + BIT(0) | BIT(2) | BIT(3)),
>> + [RST_HDMI] = RST_DATA(APMU_HDMI_CLK_RES_CTRL, 0, BIT(9)),
>> + [RST_PCIE0] = RST_DATA(APMU_PCIE_CLK_RES_CTRL_0, BIT(8),
>> + BIT(3) | BIT(4) | BIT(5)),
>> + [RST_PCIE1] = RST_DATA(APMU_PCIE_CLK_RES_CTRL_1, BIT(8),
>> + BIT(3) | BIT(4) | BIT(5)),
>> + [RST_PCIE2] = RST_DATA(APMU_PCIE_CLK_RES_CTRL_2, BIT(8),
>> + BIT(3) | BIT(4) | BIT(5)),
>> + [RST_EMAC0] = RST_DATA(APMU_EMAC0_CLK_RES_CTRL, 0, BIT(1)),
>> + [RST_EMAC1] = RST_DATA(APMU_EMAC1_CLK_RES_CTRL, 0, BIT(1)),
>> + [RST_JPG] = RST_DATA(APMU_JPG_CLK_RES_CTRL, 0, BIT(0)),
>> + [RST_CCIC2PHY] = RST_DATA(APMU_CSI_CCIC2_CLK_RES_CTRL, 0, BIT(2)),
>> + [RST_CCIC3PHY] = RST_DATA(APMU_CSI_CCIC2_CLK_RES_CTRL, 0, BIT(29)),
>> + [RST_CSI] = RST_DATA(APMU_CSI_CCIC2_CLK_RES_CTRL, 0, BIT(1)),
>> + [RST_ISP] = RST_DATA(APMU_ISP_CLK_RES_CTRL, 0, BIT(0)),
>> + [RST_ISP_CPP] = RST_DATA(APMU_ISP_CLK_RES_CTRL, 0, BIT(27)),
>> + [RST_ISP_BUS] = RST_DATA(APMU_ISP_CLK_RES_CTRL, 0, BIT(3)),
>> + [RST_ISP_CI] = RST_DATA(APMU_ISP_CLK_RES_CTRL, 0, BIT(16)),
>> + [RST_DPU_MCLK] = RST_DATA(APMU_LCD_CLK_RES_CTRL2, 0, BIT(9)),
>> + [RST_DPU_ESC] = RST_DATA(APMU_LCD_CLK_RES_CTRL1, 0, BIT(3)),
>> + [RST_DPU_HCLK] = RST_DATA(APMU_LCD_CLK_RES_CTRL1, 0, BIT(4)),
>> + [RST_DPU_SPIBUS] = RST_DATA(APMU_LCD_SPI_CLK_RES_CTRL, 0, BIT(4)),
>> + [RST_DPU_SPI_HBUS] = RST_DATA(APMU_LCD_SPI_CLK_RES_CTRL, 0, BIT(2)),
>> + [RST_V2D] = RST_DATA(APMU_LCD_CLK_RES_CTRL1, 0, BIT(27)),
>> + [RST_MIPI] = RST_DATA(APMU_LCD_CLK_RES_CTRL1, 0, BIT(15)),
>> + [RST_MC] = RST_DATA(APMU_PMUA_MC_CTRL, 0, BIT(0)),
>> +};
>> +
>> +static const struct ccu_reset_controller_data apmu_reset_controller_data = {
>> + .count = ARRAY_SIZE(apmu_reset_data),
>> + .data = apmu_reset_data,
>> +};
>> +
>> static const struct k1_ccu_data k1_ccu_apmu_data = {
>> .clk = k1_ccu_apmu_clks,
>> + .rst_data = &apmu_reset_controller_data,
>> };
>>
>> static struct ccu_reset_controller *
>> --
>> 2.43.0
>>
>