On 22/02/2025 11:48, Haylen Chu wrote:
> On Sat, Feb 22, 2025 at 10:59:09AM +0100, Krzysztof Kozlowski wrote:
>> 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"
>
> My reply seems somehow misleading. The statement means I will merge the
> children with the syscon into one devicetree node, which applies for
> both APMU and MPMU. I wasn't going to say that APMU and MPMU are
> intermixed.
>
> As Alex said, all these units have their own distinct and separate MMIO
> regions.
>
>>>
>>> I suggest a single "k1-clocks" device be created, which has
>>
>> For four devices? Or for one device?
>
> By Alex's example, I think he means a device node taking all these
> distinct MMIO regions as resource.
You still do not answer about the hardware: how many devices is there?
>
> clock {
> compatible = "spacemit,k1-clocks";
>
> reg = <0x0 0xc0880000 0x0 0x2050>,
> <0x0 0xc0888000 0x0 0x30>,
> <0x0 0xd4015000 0x0 0x1000>,
> <0x0 0xd4050000 0x0 0x209c>,
> <0x0 0xd4090000 0x0 0x1000>,
> <0x0 0xd4282800 0x0 0x400>,
> <0x0 0xf0610000 0x0 0x20>;
> reg-names = "rcpu",
> "rcpu2",
> "apbc",
> "mpmu",
> "apbs",
> "apmu",
> "apbc2";
>
> /* ... */
> };
>
>> 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?
>>
>
> I'm not sure about the apbc2, rcpu and rcpu2 regions; they aren't
> related to the thread, either. For APBC, MPMU, APBS and APMU, I'm pretty
> sure they're standalone blocks with distinct and separate MMIO regions,
> this could be confirmed by the address mapping[1].
They were brought here to discuss for some reason. Long discussions,
long emails, unrelated topics like hardware or different devices - all
this is not making it easier for me to understand.
Best regards,
Krzysztof
