Re: [PATCH v4 2/3] arm64: dts: ti: k3-am62l: add initial infrastructure

[Krzysztof Kozlowski <krzk@kernel.org>]

On 16/04/2025 16:42, Bryan Brattlof wrote:
> On April 12, 2025 thus sayeth Krzysztof Kozlowski:
>> On 11/04/2025 20:26, Bryan Brattlof wrote:
>>>>> +
>>>>> +		usb0_phy_ctrl: syscon@45000 {
>>>>> +			compatible = "ti,am62-usb-phy-ctrl", "syscon";
>>>>> +			reg = <0x45000 0x4>;
>>>>> +			bootph-all;
>>>>> +		};
>>>>> +
>>>>> +		usb1_phy_ctrl: syscon@45004 {
>>>>> +			compatible = "ti,am62-usb-phy-ctrl", "syscon";
>>>>> +			reg = <0x45004 0x4>;
>>>>
>>>> No, you do not get syscon per register. The entire point of syscon is to
>>>> collect ALL registers. Your device is the syscon, not a register.
>>>>
>>>
>>> My understanding from [0] was that we would need to break this up into 
>>> smaller syscon nodes because the alternative would be to mark the entire 
>>> region as a syscon and every other node using it would need to use it's 
>>> base + offset which was kinda undesirable especially for the small 
>>> number of drivers that need data from this region.
>>>
>>>     a-device {
>>>         clocks = <&epwm_tbclk 0>;
>>
>>
>> Hm? That's how you use the syscon, so how it can be undesirable?
>>
>> Anyway, one register is not a device, so no device node per register.
>>
>> In the link you provided I was repeating the same, so you got same
>> review in multiple places.
>>
> 
> Interesting. The way I read that thread was the opposite and it's why we 
> did this for the 62, 62A, and 62P devices. I mainly say it's unfortunate 

Really? What was unclear here:

https://lore.kernel.org/lkml/20250124-able-beagle-of-prowess-f5eb7a@krzk-bin/

Un-acked, I missed the point that you really speak in commit msg about
register and you really treat one register is a device. I assumed you
only need that register from this device, but no. That obviously is not
what this device is. Device is not a single register among 10000 others.
IOW, You do not have 10000 devices there.

NAK

> because if we have a block of miscellaneous registers there's no clear 
> guidance on how big or small that range can or should be and we still 
> need to encode the offset to that exact register.
> 
> By labeling each register we at least have the opportunity to describe 
> each register and if they are even used.
Repeated many times: no device nodes per clock (also TI invention), no
device nodes per register. This is not an opportunity. This is just not
desired.

Best regards,
Krzysztof