Re: [PATCH 4/4] clk: siflower: Add SF19A2890 peripheral clock and reset manager driver

[Philipp Zabel <p.zabel@pengutronix.de>]

On Mi, 2026-11-04 at 02:21 -0500, Joseph Telaak via B4 Relay wrote:
> From: Chuanhong Guo <gch981213@gmail.com>
> 
> Add a platform driver for the peripheral clock and reset manager (CRM)
> block found on Siflower SF19A2890 and compatible SoCs.
> 
> Each CRM block manages a group of APB peripherals through three registers:
> a reset register, a clock gate register, and a shared Bus Output Enable
> (BOE) register that must be asserted for any peripheral in the block to
> respond to bus transactions.
> 
> The driver registers a clock gate controller and a reset controller from
> a single DT node. A shared spinlock protects all read-modify-write
> accesses across both. BOE is managed automatically: asserted when the
> first clock gate is enabled and deasserted when the last one is disabled.
> 
> Signed-off-by: Chuanhong Guo <gch981213@gmail.com>
> [jt: from Chuanhong's siflower-7.1 branch; one compatible per block with
>  its gate bits, critical gates and reset masks in tables here instead of
>  the siflower,valid-gates, siflower,critical-gates, siflower,reset-masks
>  and siflower,num-resets properties, gate N parented to input clock N by
>  index instead of by global name, the gate updates under the spinlock too,
>  no separator comments, Kconfig dependencies and help, subject]
> Signed-off-by: Joseph Telaak <joseph@telaak.dev>
> ---
>  drivers/clk/siflower/Kconfig                   |  11 +
>  drivers/clk/siflower/Makefile                  |   1 +
>  drivers/clk/siflower/clk-sf19a2890-periphcrm.c | 360 +++++++++++++++++++++++++
>  3 files changed, 372 insertions(+)
> 
[...]
> diff --git a/drivers/clk/siflower/clk-sf19a2890-periphcrm.c b/drivers/clk/siflower/clk-sf19a2890-periphcrm.c
> new file mode 100644
> index 000000000..95d4dd449
> --- /dev/null
> +++ b/drivers/clk/siflower/clk-sf19a2890-periphcrm.c
> @@ -0,0 +1,360 @@
[...]
> +#define CRM_DATA(_g, _r)						\
> +	{								\
> +		.gates = sf19a2890_##_g##_gates,			\
> +		.num_gates = ARRAY_SIZE(sf19a2890_##_g##_gates),	\
> +		.reset_masks = sf19a2890_##_r##_resets,			\
> +		.num_resets = ARRAY_SIZE(sf19a2890_##_r##_resets),	\
> +	}
> +
> +static const u32 sf19a2890_single_resets[] = { BIT(0) };
> +
[...]
> +
> +static const u32 sf19a2890_gmac_resets[] = { GENMASK(1, 0) };

Are these two bits separate resets? Are they individually controllable?
Why are they bundled into a single reset_control like this? The commit
message doesn't explain.

[...]
> +static const u32 sf19a2890_wlan_resets[] = { GENMASK(2, 0) };
[...]
> +static const u32 sf19a2890_usb_resets[] = { GENMASK(2, 0), BIT(3), BIT(4) };
[...]
> +static const u32 sf19a2890_spi_resets[] = { GENMASK(5, 4) };
[...]
> +static const u32 sf19a2890_uart_resets[] = {
> +	BIT(0) | BIT(4), BIT(1) | BIT(5), BIT(2) | BIT(6),
> +};

Same question for the other multi-bit masks.


regards
Philipp