On 09.09.24 09:49, Krzysztof Kozlowski wrote: > On 09/09/2024 08:48, Jan Kiszka wrote: >> On 09.09.24 08:22, Krzysztof Kozlowski wrote: >>> On Sun, Sep 08, 2024 at 07:32:28PM +0200, Jan Kiszka wrote: >>>> From: Jan Kiszka <jan.kiszka@xxxxxxxxxxx> >>>> >>>> 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 optional unless the PVU shall be used for PCIe. >>>> >>>> Signed-off-by: Jan Kiszka <jan.kiszka@xxxxxxxxxxx> >>>> --- >>>> CC: Lorenzo Pieralisi <lpieralisi@xxxxxxxxxx> >>>> CC: "Krzysztof Wilczyński" <kw@xxxxxxxxx> >>>> CC: Bjorn Helgaas <bhelgaas@xxxxxxxxxx> >>>> CC: linux-pci@xxxxxxxxxxxxxxx >>>> --- >>>> .../bindings/pci/ti,am65-pci-host.yaml | 29 +++++++++++++++++-- >>>> 1 file changed, 26 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/Documentation/devicetree/bindings/pci/ti,am65-pci-host.yaml b/Documentation/devicetree/bindings/pci/ti,am65-pci-host.yaml >>>> index 0a9d10532cc8..0c297d12173c 100644 >>>> --- a/Documentation/devicetree/bindings/pci/ti,am65-pci-host.yaml >>>> +++ b/Documentation/devicetree/bindings/pci/ti,am65-pci-host.yaml >>>> @@ -20,14 +20,18 @@ properties: >>>> - ti,keystone-pcie >>>> >>>> reg: >>>> - maxItems: 4 >>>> + minItems: 4 >>>> + maxItems: 6 >>>> >>>> reg-names: >>>> + minItems: 4 >>>> items: >>>> - const: app >>>> - const: dbics >>>> - const: config >>>> - const: atu >>>> + - const: vmap_lp >>>> + - const: vmap_hp >>>> >>>> interrupts: >>>> maxItems: 1 >>>> @@ -83,13 +87,30 @@ if: >>>> compatible: >>>> enum: >>>> - ti,am654-pcie-rc >>>> + >>>> then: >>>> + properties: >>>> + memory-region: >>> >>> I think I said it two times already. You must define properties in >>> top-level. That's how we expect, that's how dtschema works (even if it >>> works fine otherwise, it's not always that case), that's how almost all >>> bindings are written. >> >> Look, if you have such rules, also enhance the checker, or people like >> me will continue to work intuitively. Add reasoning along that as well, > > That would be ideal, but I also asked to do this twice. It does not > matter if dtschema or me tells you this, if you do not implement it. > >> would help further to reduce your review effort. The current situation >> with rather fuzzy results from the checker and strange mechanisms inside >> (see my maxItems finding) is not very helpful IMHO. >> >> I this concrete case, I would add this item top-level, just to set >> maxItems to 0 for ti,keystone-pcie? Not a pattern I'm finding anywhere. >> Or do we have to allow memory-regions for all compatibles now? > > Is it really not suitable for all the compatibles? Maybe these are quite > different devices in such case? > > But if it is not really suitable, then you can disallow it for other > variants with :false. This is also explicitly documented in example-schema: > https://elixir.bootlin.com/linux/v5.19/source/Documentation/devicetree/bindings/example-schema.yaml#L212 > I will address this via documentation: The property is not hardware related but permits swiotlb. It can be applied to devices as well that have no hardware enforcement, unlike the am654. Jan -- Siemens AG, Technology Linux Expert Center
