Message-ID: <b662067b-2bb0-477b-a2bc-cbaa7d84126d@siemens.com>
Date: Fri, 6 Sep 2024 09:00:52 +0200
From: Jan Kiszka <jan.kiszka@...mens.com>
To: Krzysztof Kozlowski <krzk@...nel.org>
Cc: Nishanth Menon <nm@...com>, Santosh Shilimkar <ssantosh@...nel.org>,
 Vignesh Raghavendra <vigneshr@...com>, Tero Kristo <kristo@...nel.org>,
 Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 linux-pci@...r.kernel.org, Siddharth Vadapalli <s-vadapalli@...com>,
 Bao Cheng Su <baocheng.su@...mens.com>, Hua Qian Li
 <huaqian.li@...mens.com>, Diogo Ivo <diogo.ivo@...mens.com>,
 Lorenzo Pieralisi <lpieralisi@...nel.org>,
 Krzysztof Wilczyński <kw@...ux.com>,
 Bjorn Helgaas <bhelgaas@...gle.com>
Subject: Re: [PATCH v4 2/7] dt-bindings: PCI: ti,am65: Extend for use with PVU

On 05.09.24 09:56, Krzysztof Kozlowski wrote:
> On 05/09/2024 09:50, Krzysztof Kozlowski wrote:
>> On 05/09/2024 09:15, Jan Kiszka wrote:
>>> On 05.09.24 08:53, Krzysztof Kozlowski wrote:
>>>> On 05/09/2024 08:40, Jan Kiszka wrote:
>>>>> On 05.09.24 08:32, Krzysztof Kozlowski wrote:
>>>>>> On Wed, Sep 04, 2024 at 12:00:11PM +0200, Jan Kiszka wrote:
>>>>>>> From: Jan Kiszka <jan.kiszka@...mens.com>
>>>>>>>
>>>>>>> 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 unless the PVU shall used for PCIe.
>>>>>>>
>>>>>>> Signed-off-by: Jan Kiszka <jan.kiszka@...mens.com>
>>>>>>> ---
>>>>>>> CC: Lorenzo Pieralisi <lpieralisi@...nel.org>
>>>>>>> CC: "Krzysztof Wilczyński" <kw@...ux.com>
>>>>>>> CC: Bjorn Helgaas <bhelgaas@...gle.com>
>>>>>>> CC: linux-pci@...r.kernel.org
>>>>>>> ---
>>>>>>>  .../bindings/pci/ti,am65-pci-host.yaml        | 52 ++++++++++++++-----
>>>>>>>  1 file changed, 40 insertions(+), 12 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..d8182bad92de 100644
>>>>>>> --- a/Documentation/devicetree/bindings/pci/ti,am65-pci-host.yaml
>>>>>>> +++ b/Documentation/devicetree/bindings/pci/ti,am65-pci-host.yaml
>>>>>>> @@ -19,16 +19,6 @@ properties:
>>>>>>>        - ti,am654-pcie-rc
>>>>>>>        - ti,keystone-pcie
>>>>>>>  
>>>>>>> -  reg:
>>>>>>> -    maxItems: 4
>>>>>>> -
>>>>>>> -  reg-names:
>>>>>>> -    items:
>>>>>>> -      - const: app
>>>>>>> -      - const: dbics
>>>>>>> -      - const: config
>>>>>>> -      - const: atu
>>>>>>
>>>>>>
>>>>>> Nothing improved here.
>>>>>
>>>>> Yes, explained the background to you. Sorry, if you do not address my
>>>>> replies, I'm lost with your feedback.
>>>>
>>>> My magic ball could not figure out the problem, so did not provide the
>>>> answer.
>>>>
>>>> I gave you the exact code which illustrates how to do it. If you do it
>>>> that way: it works. If you do it other way: it might not work. However
>>>
>>> The link you provided was unfortunately not self-explanatory because if 
>>> I - apparently - do it like that example, I'm getting the errors below.
>>>
>>>> without seeing anything, magic ball was silent, so I am not
>>>> participating in game: would you be so kind to give more information so
>>>> I won't waste my day in asking what is wrong.
>>>
>>> With my patch:
>>>
>>> # make ... dtbs_check
>>>   DTC [C] arch/arm64/boot/dts/ti/k3-am6528-iot2050-basic.dtb
>>>   DTC [C] arch/arm64/boot/dts/ti/k3-am6528-iot2050-basic-pg2.dtb
>>>   DTC [C] arch/arm64/boot/dts/ti/k3-am6548-iot2050-advanced.dtb
>>>   DTC [C] arch/arm64/boot/dts/ti/k3-am6548-iot2050-advanced-m2.dtb
>>>   OVL [C] arch/arm64/boot/dts/ti/k3-am6548-iot2050-advanced-m2-bkey-ekey-pcie.dtb
>>>   OVL [C] arch/arm64/boot/dts/ti/k3-am6548-iot2050-advanced-m2-bkey-usb3.dtb
>>>   DTC [C] arch/arm64/boot/dts/ti/k3-am6548-iot2050-advanced-pg2.dtb
>>>   DTC [C] arch/arm64/boot/dts/ti/k3-am6548-iot2050-advanced-sm.dtb
>>>   DTC [C] arch/arm64/boot/dts/ti/k3-am654-base-board.dtb
>>>
>>> With this revert on top:
>>>
>>> diff --git a/Documentation/devicetree/bindings/pci/ti,am65-pci-host.yaml b/Documentation/devicetree/bindings/pci/ti,am65-pci-host.yaml
>>> index d8182bad92de..dd753dae24c6 100644
>>> --- a/Documentation/devicetree/bindings/pci/ti,am65-pci-host.yaml
>>> +++ b/Documentation/devicetree/bindings/pci/ti,am65-pci-host.yaml
>>> @@ -19,6 +19,16 @@ properties:
>>>        - ti,am654-pcie-rc
>>>        - ti,keystone-pcie
>>>  
>>> +  reg:
>>> +    maxItems: 4
>>> +
>>> +  reg-names:
>>> +    items:
>>> +      - const: app
>>> +      - const: dbics
>>> +      - const: config
>>> +      - const: atu
>>
>> There is nothing like that in that example.
>> https://elixir.bootlin.com/linux/v6.8/source/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml#L44
>>
>>> +
>>>    interrupts:
>>>      maxItems: 1
>>>  
>>> @@ -104,18 +114,6 @@ then:
>>>      - msi-map
>>>      - num-viewport
>>>  
>>> -else:
>>> -  properties:
>>> -    reg:
>>> -      maxItems: 4
>>> -
>>> -    reg-names:
>>> -      items:
>>> -        - const: app
>>> -        - const: dbics
>>> -        - const: config
>>> -        - const: atu
>>
>> Neither this.
>>
>> Each case MUST be covered, look:
>> https://elixir.bootlin.com/linux/v6.8/source/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml#L191
> 
> Actually your case fits better another example from UFS, so take that one:
> 
> https://elixir.bootlin.com/linux/v6.11-rc6/source/Documentation/devicetree/bindings/ufs/samsung,exynos-ufs.yaml#L39
> 

Via lots of trial-and-error, I think I now understood the magic behind
this: You may reduce maxItems in a condition, but you cannot increase
it. That's why all my intuition-based attempts failed before. Not sure
if this is a specified property of the meta schema or just an oddity of
the validator.

Jan

-- 
Siemens AG, Technology
Linux Expert Center

Powered by blists - more mailing lists