Re: [PATCH v2 3/6] dt-bindings: PCI: ti,am65: Extend for use with PVU

[Jan Kiszka <jan.kiszka@siemens.com>]

On 27.08.24 12:06, Siddharth Vadapalli wrote:
> On Tue, Aug 27, 2024 at 11:32:02AM +0200, Jan Kiszka wrote:
>> On 27.08.24 11:29, Krzysztof Kozlowski wrote:
>>> On 27/08/2024 11:22, Jan Kiszka wrote:
>>>> On 27.08.24 08:37, Krzysztof Kozlowski wrote:
>>>>> On Mon, Aug 26, 2024 at 11:50:04PM +0200, Jan Kiszka wrote:
>>>>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>>>>
>>>>>> Describe also the VMAP registers which are needed in order to make use
>>>>>> of the PVU with this PCI host. Furthermore, permit to specify a
>>>>>> restricted DMA pool by phandle.
>>>>>
>>>>> That's an ABI break without explanation why it is necessary.
>>>>>
>>>>
>>>> It is needed in order to support the PVU, as written above.
>>>
>>> Above say only that you want a new feature and that's not really
>>> suitable explanation for ABI break, because answer to this is: add new
>>> feature without breaking existing users. But maybe there is a bug or
>>> something does not work or never work or there are no users, don't know.
>>>
>>>>
>>>> Previous versions of this binding likely didn't consider this use case
>>>> and therefore didn't describe all registers associated with the hardware.
>>>>
>>>> BTW, if you see a way to add the required registers without breaking
>>>> more than needed, I'm all ears. At least the kernel driver will continue
>>>> to work with older DTs when you disable PVU support or do not add a DMA
>>>> pool to the DT.
>>>
>>> If there is no ABI break, because driver still handles correctly old
>>> DTB, then mention it in the commit msg.
>>
>> Well, this is strictly spoken not a topic for this commit because this
>> one should have no clue about what drivers do with DTs according to this
>> binding. But I can put a hint and go into details in the driver patch.
> 
> Based on the Techincal Reference Manual for AM654 and the driver
> implementation in patch 5/6, I think that the following might be one way
> of hinting that ABI won't break:
> 
> The PVU on the AM65 SoC is capable of restricting DMA from PCIe devices to
> specific regions of host memory. Add the optional property "memory-regions"
> to point to such regions of memory when PVU is used. Since the PVU deals
> with system physical addresses, utilizing the PVU with PCIe devices also
> requires setting up the VMAP registers to map the Requester ID of the
> PCIe device to the CBA Virtual ID, which in turn is mapped to the system
> physical address. Hence, describe the VMAP registers which are optionally
> configured whenever PVU is used for PCIe.

Thanks, will reuse this!

Additionally, we should then likely do

  reg:
    minItems: 4
    maxItems: 6

to underline that the old form is still fine. Do I need to do anything
to reg-names then as well?

Jan

-- 
Siemens AG, Technology
Linux Expert Center