Re: [PATCH v4 2/4] dt-bindings: soc: spacemit: Add spacemit,k1-syscon

[Haylen Chu <heylenay@4d2.org>]

On Sat, Feb 22, 2025 at 10:52:02AM +0100, Krzysztof Kozlowski wrote:
> On 15/02/2025 09:41, Haylen Chu wrote:
> > 
> >>> 	};
> >>>
> >>> For the other two clock controllers (APBS and APBC), syscons are really
> >>> unnecessary and it's simple to fold them.
> >>
> >>
> >> I don't follow. Do we talk about children or syscon compatible?
> > 
> > APBS region contains only clock (PLL) bits and APBC region contains only
> > reset and clock bits, so I was thinking about dropping the syscon nodes
> > and changing their compatible to spacemit,k1-plls and
> > spacemit,k1-cru-apbc.
> > 
> > In summary, my plan is,
> > 
> > - For MPMU, APMU and APBC region, keep the binding in soc/spacemit.
> >   They'll be reset, clock and power controllers, with compatible
> >   "spacemit,k1-syscon-*".
> > - For APBS region, write a new binding clock/spacemit,k1-plls, as it
> >   contains only PLL-related bits. It acts as clock controller.
> > - All split children will be eliminated, there'll be only four device
> >   nodes, one for each region, matching the datasheet.
> > - Put all clock-related binding definition of SpacemiT K1 in
> >   dt-bindings/clock/spacemit,k1-ccu.h
> > 
> > Is it fine for you?
> > 
> 
> That did not explain hardware to me.

Sorry if my replies haven't made things clear. I'm goint to make a
(hopefully) more clear conclusion,

> You assume that some way, maybe
> through magical crystal ball, I know your hardware and will tell you
> what to do.
>
> No.
> 
> I have dozens of other patches in my inbox. It's you who should explain
> the hardware in simple, concise way so we can judge whether DT
> description is correct.
> 
> Again: define what is the actual device, what is its address space, what
> are its possible *separate* and *distinctive* children.

The series covers four seperate blocks,

- Application Power Manage Unit, APMU
- Main Power Manage Unit, MPMU
- APB Bus Clock Unit, APBC
- APB Spare, APBS

they're clearly separate blocks and have their own distinct, separate
address spaces, confirmed by the Address Mapping section in the TRM[1].

These four blocks provide hardware bits for three purposes: power
management, reset signals and clocks. Not every block is capable of all
the three functionalities,

- APMU, MPMU: power, reset, clock
- APBC: clock, reset
- APBS: clock

Reset and clock bits, if present, always stay in the same register.
Power management bits stay in others. These two types of registers
interleave if present in the same block (APMU and MPMU case).

These blocks have no child: power, clock and reset definitions differ
from block to block, no reusable nodes could be split from them.

Hope this conclusion will help the reviewing. Please tell if something
is unclear.

> 
> Best regards,
> Krzysztof

Thanks,
Haylen Chu

[1]: https://developer.spacemit.com/documentation?token=LzJyw97BCipK1dkUygrcbT0NnMg