Re: [RFC PATCH 3/7] dt-bindings: bus: add STM32MP15 ETZPC firewall bus bindings

[Krzysztof Kozlowski]

On 09/01/2023 12:54, Gatien CHEVALLIER wrote:
>>>> Then why do you define them in bindings? Use raw numbers. Do you see
>>>> anywhere in arm/arm64 bindings for GIC_SPI interrupt numbers?
>>>>
>>>
>>> What would you think of simply removing the comments that state that IDs
>>> are reserved, mimicking the way it is for qcom bindings? Fundamentally,
>>> they are indeed only IDs and could be raw numbers.
>>
>> If these are IDs then there are no reserved numbers and they are
>> continuous from 0 to X. Without gaps.
>>
>>> IMO, this makes reading the device tree harder. Because you'd have to
>>> look what the raw number corresponds to.
>>
>> Sure, but that's not the reason to put numbers to the bindings... You
>> mix defines with bindings.
>>
>>> To take an example, it has already been done for SCMI clocks and I find
>>> it eases comprehension.
>>
>> You need to be a bit more specific...
> 
> Please see include/dt-bindings/clock/stm32mp1-clks.h, where there are 
> various clock IDs defined, some of them not contiguous.

These are pretty often added to accommodate space for exposing these
clocks in the future. IOW, these might be IDs just not all are shared
via header. There are such platforms and it is OK.

> 
> Errata: for SCMI clocks they are indeed contiguous but not clock IDs.
> 
>>
>> Anyway, IDs should be placed in bindings. Some mapping of
>> internal/hardware ports, registers, offsets, values - usually not.
>>
>> I don't know where exactly your case fits, but when some IDs are
>> reserved it is a clear sign that these are not IDs (again - IDs start
>> from 0 and go incrementally by one, without gaps).
>>
> 
> I do agree with your statement that IDs should not be reserved.
> 
> I think I've missed something to better highlight my point of view: It 
> would be perfectly fine using numbers that are not described in this 
> bindings file. It would just not correspond to an ID of a peripheral 
> described in the SoC reference manual, thus making no sense to use them. 
> Stating that they are reserved was incorrect, it's just that peripherals 
> get a firewall ID, depending on the platform.

Why peripheral ID should be put into the bindings? Why bindings is a
place for it? Interrupt numbers, GPIO indices/numbers, register offsets,
IOMMU ports - none of these are suitable for bindings.

> 
> I think it should be okay not describing IDs that are not relevant, what 
> do you think? I found that in include/dt-bindings/arm/qcom,ids.h, IDs 
> are not continuous. Not mentioning an ID could be used for deprecation.

These are not IDs of clocks. These are unique identifiers assigned by
vendor and used by different pieces: firmware/bootloaders, DTS and Linux
driver. We have no control of them but they exist. They also do not
represent any hardware number.

You bring some examples as an argument, but these examples are not
always related to your case. To be clear - we talk here about bindings,
so they bind different interfaces of software components (e.g. Linux
kernel with DTS). Now, what is the different interface here in your
case? If you say your peripheral hardware ID, then answer is no - this
is not software interface.

Best regards,
Krzysztof