Message-ID: <4f7bf109-bf18-42be-971c-5d5edd9595b5@kernel.org>
Date: Sat, 22 Feb 2025 10:59:09 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Alex Elder <elder@...cstar.com>, Haylen Chu <heylenay@....org>
Cc: Michael Turquette <mturquette@...libre.com>,
 Stephen Boyd <sboyd@...nel.org>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Haylen Chu <heylenay@...look.com>,
 Yixun Lan <dlan@...too.org>, linux-riscv@...ts.infradead.org,
 linux-clk@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, Inochi Amaoto <inochiama@...look.com>,
 Chen Wang <unicornxdotw@...mail.com>, Jisheng Zhang <jszhang@...nel.org>,
 Meng Zhang <zhangmeng.kevin@...ux.spacemit.com>,
 Guodong Xu <guodong@...cstar.com>
Subject: Re: [PATCH v4 2/4] dt-bindings: soc: spacemit: Add spacemit,k1-syscon

On 22/02/2025 00:40, Alex Elder wrote:
> I have a general proposal on how to represent this, but I'd
> like to know whether it makes sense.  It might be what Krzysztof
> is suggesting, but in any case, I hope this representation would
> work, because it could simplify the code, and compartmentalizes
> things.
> 
> Part of what motivates this is that I've been looking at the
> downstream reset code this week.  It contains a large number of
> register offset definitions identical to what's used for the
> clock driver.  The reset driver uses exactly the same registers
> as the clock driver does.  Downstream they are separate drivers,
> but the clock driver exports a shared spinlock for both drivers
> to use.
> 
> These really need to be incorporated into the same driver for
> upstream.

Why? First, it is not related to the topic here at all. You can design
drivers as you wish and still nothing to do with discussion about binding.
Second, different subsystems justify different drivers and Linux handles
this well already. No need for custom spinlock - regmap already does it.


> 
> The clock code defines four distinct "units" (a term I'll use
> from here on; there might be a better name):
>    MPMU  Main Power Management Unit
>    APMU  Application Power Management Unit
>    APBC  APB Clock
>    APBS  APB Spare
> 
> The reset code defines some of those, but doesn't use APBS.
> It also defines three more:
>    APBC2 Another APB Clock
>    RCPU  Real-time CPU?
>    RCPU2 Another Real-time CPU
> 
> Each of these "units" has a distinct I/O memory region that
> contains registers that manage the clocks and reset signals.

So there are children - mpmu, apmu, apbclock, apbspare, apbclock2, rcpu
1+2? But previous statements were saying these are intermixed?

" I'll make APMU/MPMU act as a whole device"

> 
> I suggest a single "k1-clocks" device be created, which has

For four devices? Or for one device?

No, it's again going to wrong direction. I already said:

"You need to define what is the device here. Don't create fake nodes ust
for your drivers. If registers are interleaved and manual says "this is
block APMU/MPMU" then you have one device, so one node with 'reg'."

So what is the device here? Can you people actually answer?



> access to all of the I/O address ranges.  And then within
> the DT node for that device there is a sub-node for the

Uh, confusing. You said there is one device for all the clocks, so if
there is one device so also one device node. No children.

Maybe you have more devices but none of you is explaining the hardware
that way. Mixing talk about drivers is really not helping.

> 


Best regards,
Krzysztof

Powered by blists - more mailing lists