RE: [PATCH 1/6] dt-bindings: pci: cadence: Add property "hpa" for PCIe controllers

[Manikandan Karunakaran Pillai]

>-----Original Message-----
>From: Krzysztof Kozlowski <krzk@xxxxxxxxxx>
>Sent: Monday, March 24, 2025 9:29 PM
>To: Manikandan Karunakaran Pillai <mpillai@xxxxxxxxxxx>;
>lpieralisi@xxxxxxxxxx; manivannan.sadhasivam@xxxxxxxxxx;
>bhelgaas@xxxxxxxxxx; kw@xxxxxxxxx; robh@xxxxxxxxxx;
>devicetree@xxxxxxxxxxxxxxx
>Cc: linux-kernel@xxxxxxxxxxxxxxx; linux-pci@xxxxxxxxxxxxxxx
>Subject: Re: [PATCH 1/6] dt-bindings: pci: cadence: Add property "hpa" for PCIe
>controllers
>
>EXTERNAL MAIL
>
>
>On 24/03/2025 10:08, Manikandan Karunakaran Pillai wrote:
>> Document the newly added property "hpa" for Cadence PCIe controllers
>> in Documentation/devicetree/bindings/pci/ for Root Port and Endpoint
>> configurations
>
>Drop the path, pointless.
Ok
>
>>
>
>Please run scripts/checkpatch.pl on the patches and fix reported warnings.
>After that, run also 'scripts/checkpatch.pl --strict' on the patches and
>(probably) fix more warnings. Some warnings can be ignored, especially from -
>-strict run, but the code here looks like it needs a fix. Feel free to get in touch if
>the warning is not clear.
>

The scripts/checkpatch.pl has been run  with and without --strict. With the --strict option
4 checks are generated on 1 patch(patch 0002 of the series), which can be ignored. There are 
no code fixes required for these checks. The rest of the 'scripts/checkpatch.pl' 
is clean.

><form letter>
>Please use scripts/get_maintainers.pl to get a list of necessary people and lists
>to CC (and consider --no-git-fallback argument, so you will not CC people just
>because they made one commit years ago). It might happen, that command
>when run on an older kernel, gives you outdated entries. Therefore please be
>sure you base your patches on recent Linux kernel.

Ok

>
>Tools like b4 or scripts/get_maintainer.pl provide you proper list of people, so
>fix your workflow. Tools might also fail if you work on some ancient tree (don't,
>instead use mainline) or work on fork of kernel (don't, instead use mainline).
>Just use b4 and everything should be fine, although remember about `b4 prep
>--auto-to-cc` if you added new patches to the patchset.
></form letter>
>
I used an earlier list generated. Will take care to generate the list on latest tree.

>> Signed-off-by: Manikandan K Pillai <mpillai@xxxxxxxxxxx>
>> ---
>>  .../devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml         | 4 ++++
>>  .../devicetree/bindings/pci/cdns,cdns-pcie-host.yaml       | 7 +++++++
>>  2 files changed, 11 insertions(+)
>>
>> diff --git
>> a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml
>> b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml
>> index 98651ab22103..4e839fa90b23 100644
>> --- a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml
>> +++ b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-ep.yaml
>> @@ -24,6 +24,10 @@ properties:
>>        - const: reg
>>        - const: mem
>>
>> +  hpa:
>> +    description: If present PCI controller is high performance
>> + architecture
>
>compatible defines this.

Ok, Will delete this property and use compatible instead. 

>
>> +    type: boolean
>> +
>>  required:
>>    - reg
>>    - reg-names
>> diff --git
>> a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml
>> b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml
>> index a8190d9b100f..c219fe15c879 100644
>> --- a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml
>> +++ b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml
>> @@ -9,8 +9,11 @@ title: Cadence PCIe host controller
>>  maintainers:
>>    - Tom Joseph <tjoseph@xxxxxxxxxxx>
>>
>> +select: false

>
>No clue what you wanted to achieve here, no explanation in commit msg and
>this makes not much sense.
>
Was done to remove compilation warnings. This is one of existing yaml files and without any changes it is giving errors/warnings. I had to change the scripts 'dt-extract-example' to clear some of the errors(as shown below)
	(changed to raw mode)          root_node = re.search(r'^/\s*{', ex)
	---
	(existing code)                          root_node = re.search('^/\s*{', ex)
The first time running a - 		
make ARCH=arm64 dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml fails, 
a subsequent run passes. 

>> +
>>  allOf:
>>    - $ref: cdns-pcie-host.yaml#
>> +  - $ref: cdns-pcie.yaml#
>
>Why?

The addition of these were to remove dt_binding_check errors.

>
>>
>>  properties:
>>    compatible:
>> @@ -24,6 +27,10 @@ properties:
>>        - const: reg
>>        - const: cfg
>>
>> +  hpa:
>
>Again, compatible defines this.
>
>Anyway, follow standard DT binding rules (see writing bindings, numerous talks
>or guides).
>
>Best regards,
>Krzysztof