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

[Joseph Telaak]

On Wed, Nov 11, 2026 at 10:56:47AM +0100, Philipp Zabel wrote:
> > + spin_lock_irqsave(&crm->lock, flags);
>
> Could be simplified with guard(spinlock)(&crm->lock);

Will do in v3, as guard(spinlock_irqsave)(), since the clock gates take
the same lock from clk_enable(), which can run in interrupt context. I'll
convert the gate enable and disable paths as well.

> > +static const u32 sf19a2890_usb_resets[] = {
> > + [RST_USB] = GENMASK(2, 0),
>
> Do these three bits control separate reset lines?
> Does the hardware require them to be changed together?

Yes, they are separate lines, one per clock domain of the DWC2
controller. Bit 0 matches the controller's gate and bit 2 its AXI
master's (usb_axim). Bit 1 has no gate, so it is probably the PHY clock
domain, whose clock comes from the PHY rather than from this block.

There is no public documentation for this block. Chuanhong explained in
v1 that the lines of one IP's clock domains have to be asserted together
[1], the same as the GMAC's. The dwc2 binding also takes a single reset
for the controller.

> > +static const u32 sf19a2890_uart_resets[] = {
> > + [RST_UART0] = BIT(0) | BIT(4),
> > + [RST_UART1] = BIT(1) | BIT(5),
> > + [RST_UART2] = BIT(2) | BIT(6),
>
> Same question as above.

Yes, they are separate. Going by the gates at the same bit positions,
bits 0-2 are the UARTs' APB resets (PRESETn) and bits 4-6 their UART
clock domain resets (nUARTRST). The PL011 TRM (DDI 0183, section 2.4.1)
says the reset controller must use PRESETn to assert nUARTRST. This block
leaves both to software, so the driver asserts them together. The pl011
binding also allows a single reset.

The same goes for the other two groups. The SPI block's bits 4 and 5 are
the PL022's PRESETn and nSSPRST, which its TRM (DDI 0194) ties together in
the same way. The WLAN blocks' bits 0-2 match their AXI slave, AXI master
and platform gates.

v3 will explain each multi-bit group in the commit message, not just the
GMAC's.

[1] https://lore.kernel.org/all/CAJsYDV+PSoAU9qNyRoEKuFoc4oeBkw8b=vdcCap4Mx3dCA1kAw@xxxxxxxxxxxxxx/

Thanks,
Joseph