Message-ID: <344418a4-0365-40f0-a4d6-380fbfbf96dc@kernel.org>
Date: Tue, 27 Aug 2024 12:54:26 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Jan Kiszka <jan.kiszka@...mens.com>,
 Siddharth Vadapalli <s-vadapalli@...com>
Cc: Nishanth Menon <nm@...com>, Santosh Shilimkar <ssantosh@...nel.org>,
 Vignesh Raghavendra <vigneshr@...com>, Rob Herring <robh+dt@...nel.org>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
 Conor Dooley <conor+dt@...nel.org>, linux-arm-kernel@...ts.infradead.org,
 linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
 Bao Cheng Su <baocheng.su@...mens.com>, Hua Qian Li
 <huaqian.li@...mens.com>, Diogo Ivo <diogo.ivo@...mens.com>
Subject: Re: [PATCH v2 3/6] dt-bindings: PCI: ti,am65: Extend for use with PVU

On 27/08/2024 12:46, Jan Kiszka wrote:
> 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@...mens.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?

They need "minItems: 4".

Best regards,
Krzysztof

Powered by blists - more mailing lists