Re: [PATCH v4 6/7] clk: spacemit: define new syscons with only resets

Alex Elder
2025-04-15T12:32:16Z

On 4/15/25 3:22 AM, Philipp Zabel wrote:
> On Mo, 2025-04-14 at 14:17 -0500, Alex Elder wrote:
>> Enable support for three additional syscon CCUs which support reset
>> controls but no clocks: ARCPU, RCPU2, and APBC2.
>>
>> Signed-off-by: Alex Elder <elder@riscstar.com>

You are correct that those reset-only CCUs could have
their spacemit_ccu_data be const.

I'm going to give this series a little more time for
review but I'll gladly submit v5 with the added const
qualifier in those places.

Thanks.

-Alex

>> ---
>> drivers/clk/spacemit/ccu-k1.c | 93 +++++++++++++++++++++++++++++++++++
>> 1 file changed, 93 insertions(+)
>>
>> diff --git a/drivers/clk/spacemit/ccu-k1.c b/drivers/clk/spacemit/ccu-k1.c
>> index 7494c97c3c7ec..cd1a0668bd203 100644
>> --- a/drivers/clk/spacemit/ccu-k1.c
>> +++ b/drivers/clk/spacemit/ccu-k1.c
>> @@ -130,6 +130,36 @@
>> #define APMU_EMAC0_CLK_RES_CTRL 0x3e4
>> #define APMU_EMAC1_CLK_RES_CTRL 0x3ec
>>
>> +/* RCPU register offsets */
>> +#define RCPU_SSP0_CLK_RST 0x0028
>> +#define RCPU_I2C0_CLK_RST 0x0030
>> +#define RCPU_UART1_CLK_RST 0x003c
>> +#define RCPU_CAN_CLK_RST 0x0048
>> +#define RCPU_IR_CLK_RST 0x004c
>> +#define RCPU_UART0_CLK_RST 0x00d8
>> +#define AUDIO_HDMI_CLK_CTRL 0x2044
>> +
>> +/* RCPU2 register offsets */
>> +#define RCPU2_PWM0_CLK_RST 0x0000
>> +#define RCPU2_PWM1_CLK_RST 0x0004
>> +#define RCPU2_PWM2_CLK_RST 0x0008
>> +#define RCPU2_PWM3_CLK_RST 0x000c
>> +#define RCPU2_PWM4_CLK_RST 0x0010
>> +#define RCPU2_PWM5_CLK_RST 0x0014
>> +#define RCPU2_PWM6_CLK_RST 0x0018
>> +#define RCPU2_PWM7_CLK_RST 0x001c
>> +#define RCPU2_PWM8_CLK_RST 0x0020
>> +#define RCPU2_PWM9_CLK_RST 0x0024
>> +
>> +/* APBC2 register offsets */
>> +#define APBC2_UART1_CLK_RST 0x0000
>> +#define APBC2_SSP2_CLK_RST 0x0004
>> +#define APBC2_TWSI3_CLK_RST 0x0008
>> +#define APBC2_RTC_CLK_RST 0x000c
>> +#define APBC2_TIMERS0_CLK_RST 0x0010
>> +#define APBC2_KPC_CLK_RST 0x0014
>> +#define APBC2_GPIO_CLK_RST 0x001c
>> +
>> struct ccu_reset_data {
>> u32 offset;
>> u32 assert_mask;
>> @@ -1177,6 +1207,57 @@ static const struct spacemit_ccu_data k1_ccu_apmu_data = {
>> .reset_num = ARRAY_SIZE(apmu_reset_data),
>> };
>>
>> +static const struct ccu_reset_data rcpu_reset_data[] = {
>> + [RESET_RCPU_SSP0] = RESET_DATA(RCPU_SSP0_CLK_RST, 0, BIT(0)),
>> + [RESET_RCPU_I2C0] = RESET_DATA(RCPU_I2C0_CLK_RST, 0, BIT(0)),
>> + [RESET_RCPU_UART1] = RESET_DATA(RCPU_UART1_CLK_RST, 0, BIT(0)),
>> + [RESET_RCPU_IR] = RESET_DATA(RCPU_CAN_CLK_RST, 0, BIT(0)),
>> + [RESET_RCPU_CAN] = RESET_DATA(RCPU_IR_CLK_RST, 0, BIT(0)),
>> + [RESET_RCPU_UART0] = RESET_DATA(RCPU_UART0_CLK_RST, 0, BIT(0)),
>> + [RESET_RCPU_HDMI_AUDIO] = RESET_DATA(AUDIO_HDMI_CLK_CTRL, 0, BIT(0)),
>> +};
>> +
>> +static struct spacemit_ccu_data k1_ccu_rcpu_data = {
>
> Could be const.
>
>> + /* No clocks in the RCPU CCU */
>> + .reset_data = rcpu_reset_data,
>> + .reset_num = ARRAY_SIZE(rcpu_reset_data),
>> +};
>> +
>> +static const struct ccu_reset_data rcpu2_reset_data[] = {
>> + [RESET_RCPU2_PWM0] = RESET_DATA(RCPU2_PWM9_CLK_RST, BIT(2), BIT(0)),
>> + [RESET_RCPU2_PWM1] = RESET_DATA(RCPU2_PWM9_CLK_RST, BIT(2), BIT(0)),
>> + [RESET_RCPU2_PWM2] = RESET_DATA(RCPU2_PWM9_CLK_RST, BIT(2), BIT(0)),
>> + [RESET_RCPU2_PWM3] = RESET_DATA(RCPU2_PWM9_CLK_RST, BIT(2), BIT(0)),
>> + [RESET_RCPU2_PWM4] = RESET_DATA(RCPU2_PWM9_CLK_RST, BIT(2), BIT(0)),
>> + [RESET_RCPU2_PWM5] = RESET_DATA(RCPU2_PWM9_CLK_RST, BIT(2), BIT(0)),
>> + [RESET_RCPU2_PWM6] = RESET_DATA(RCPU2_PWM9_CLK_RST, BIT(2), BIT(0)),
>> + [RESET_RCPU2_PWM7] = RESET_DATA(RCPU2_PWM9_CLK_RST, BIT(2), BIT(0)),
>> + [RESET_RCPU2_PWM8] = RESET_DATA(RCPU2_PWM9_CLK_RST, BIT(2), BIT(0)),
>> + [RESET_RCPU2_PWM9] = RESET_DATA(RCPU2_PWM9_CLK_RST, BIT(2), BIT(0)),
>> +};
>> +
>> +static struct spacemit_ccu_data k1_ccu_rcpu2_data = {
>
> const
>
>> + /* No clocks in the RCPU2 CCU */
>> + .reset_data = rcpu2_reset_data,
>> + .reset_num = ARRAY_SIZE(rcpu2_reset_data),
>> +};
>> +
>> +static const struct ccu_reset_data apbc2_reset_data[] = {
>> + [RESET_APBC2_UART1] = RESET_DATA(APBC2_UART1_CLK_RST, BIT(2), (0)),
>> + [RESET_APBC2_SSP2] = RESET_DATA(APBC2_SSP2_CLK_RST, BIT(2), (0)),
>> + [RESET_APBC2_TWSI3] = RESET_DATA(APBC2_TWSI3_CLK_RST, BIT(2), (0)),
>> + [RESET_APBC2_RTC] = RESET_DATA(APBC2_RTC_CLK_RST, BIT(2), (0)),
>> + [RESET_APBC2_TIMERS0] = RESET_DATA(APBC2_TIMERS0_CLK_RST, BIT(2), (0)),
>> + [RESET_APBC2_KPC] = RESET_DATA(APBC2_KPC_CLK_RST, BIT(2), (0)),
>> + [RESET_APBC2_GPIO] = RESET_DATA(APBC2_GPIO_CLK_RST, BIT(2), (0)),
>
> Superfluous parentheses.
>
>> +};
>> +
>> +static struct spacemit_ccu_data k1_ccu_apbc2_data = {
>
> const
>
>> + /* No clocks in the APBC2 CCU */
>> + .reset_data = apbc2_reset_data,
>> + .reset_num = ARRAY_SIZE(apbc2_reset_data),
>> +};
>> +
>> static int spacemit_reset_update(struct reset_controller_dev *rcdev,
>> unsigned long id, bool assert)
>> {
>> @@ -1351,6 +1432,18 @@ static const struct of_device_id of_k1_ccu_match[] = {
>> .compatible = "spacemit,k1-syscon-apmu",
>> .data = &k1_ccu_apmu_data,
>> },
>> + {
>> + .compatible = "spacemit,k1-syscon-rcpu",
>> + .data = &k1_ccu_rcpu_data,
>> + },
>> + {
>> + .compatible = "spacemit,k1-syscon-rcpu2",
>> + .data = &k1_ccu_rcpu2_data,
>> + },
>> + {
>> + .compatible = "spacemit,k1-syscon-apbc2",
>> + .data = &k1_ccu_apbc2_data,
>> + },
>> { }
>> };
>> MODULE_DEVICE_TABLE(of, of_k1_ccu_match);
>
> regards
> Philipp