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@xxxxxxxxxxx> >>>>>>> >>>>>>> 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
