linux-kernel - RE: [RESEND PATCH v7 1/2] dt-bindings: PCI: xilinx-cpm: Add `cpm_crx` and `cpm5nc_fw

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID:
 <DM4PR12MB61581A9D423C750921FA0116CDBB2@DM4PR12MB6158.namprd12.prod.outlook.com>
Date: Tue, 22 Apr 2025 06:45:18 +0000
From: "Musham, Sai Krishna" <sai.krishna.musham@....com>
To: Krzysztof Kozlowski <krzk@...nel.org>
CC: "bhelgaas@...gle.com" <bhelgaas@...gle.com>, "lpieralisi@...nel.org"
	<lpieralisi@...nel.org>, "kw@...ux.com" <kw@...ux.com>,
	"manivannan.sadhasivam@...aro.org" <manivannan.sadhasivam@...aro.org>,
	"robh@...nel.org" <robh@...nel.org>, "krzk+dt@...nel.org"
	<krzk+dt@...nel.org>, "conor+dt@...nel.org" <conor+dt@...nel.org>,
	"cassel@...nel.org" <cassel@...nel.org>, "linux-pci@...r.kernel.org"
	<linux-pci@...r.kernel.org>, "devicetree@...r.kernel.org"
	<devicetree@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "Simek, Michal" <michal.simek@....com>,
	"Gogada, Bharat Kumar" <bharat.kumar.gogada@....com>, "Havalige, Thippeswamy"
	<thippeswamy.havalige@....com>
Subject: RE: [RESEND PATCH v7 1/2] dt-bindings: PCI: xilinx-cpm: Add `cpm_crx`
 and `cpm5nc_fw_attr` properties

[AMD Official Use Only - AMD Internal Distribution Only]

Hi Krzysztof,

> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@...nel.org>
> Sent: Tuesday, April 15, 2025 11:04 AM
> To: Musham, Sai Krishna <sai.krishna.musham@....com>
> Cc: bhelgaas@...gle.com; lpieralisi@...nel.org; kw@...ux.com;
> manivannan.sadhasivam@...aro.org; robh@...nel.org; krzk+dt@...nel.org;
> conor+dt@...nel.org; cassel@...nel.org; linux-pci@...r.kernel.org;
> devicetree@...r.kernel.org; linux-kernel@...r.kernel.org; Simek, Michal
> <michal.simek@....com>; Gogada, Bharat Kumar
> <bharat.kumar.gogada@....com>; Havalige, Thippeswamy
> <thippeswamy.havalige@....com>
> Subject: Re: [RESEND PATCH v7 1/2] dt-bindings: PCI: xilinx-cpm: Add `cpm_crx`
> and `cpm5nc_fw_attr` properties
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> On 14/04/2025 14:23, Musham, Sai Krishna wrote:
> > [AMD Official Use Only - AMD Internal Distribution Only]
> >
> > Hi Krzysztof,
> >
> > Thanks for the review.
> >
> >> -----Original Message-----
> >> From: Krzysztof Kozlowski <krzk@...nel.org>
> >> Sent: Monday, April 14, 2025 12:32 PM
> >> To: Musham, Sai Krishna <sai.krishna.musham@....com>
> >> Cc: bhelgaas@...gle.com; lpieralisi@...nel.org; kw@...ux.com;
> >> manivannan.sadhasivam@...aro.org; robh@...nel.org;
> >> krzk+dt@...nel.org;
> >> conor+dt@...nel.org; cassel@...nel.org; linux-pci@...r.kernel.org;
> >> devicetree@...r.kernel.org; linux-kernel@...r.kernel.org; Simek,
> >> Michal <michal.simek@....com>; Gogada, Bharat Kumar
> >> <bharat.kumar.gogada@....com>; Havalige, Thippeswamy
> >> <thippeswamy.havalige@....com>
> >> Subject: Re: [RESEND PATCH v7 1/2] dt-bindings: PCI: xilinx-cpm: Add
> >> `cpm_crx` and `cpm5nc_fw_attr` properties
> >>
> >> Caution: This message originated from an External Source. Use proper
> >> caution when opening attachments, clicking links, or responding.
> >>
> >>
> >> On Mon, Apr 14, 2025 at 08:53:03AM GMT, Sai Krishna Musham wrote:
> >>> Add the `cpm_crx` property to manage the PCIe IP reset, and
> >>> `cpm5nc_fw_attr` property to clear firewall after link reset, while
> >>> maintaining backward compatibility with existing device trees.
> >>>
> >>> Also, incorporate `reset-gpios` in example for GPIO-based handling
> >>> of the PCIe Root Port (RP) PERST# signal for enabling assert and
> >>> deassert control.
> >>>
> >>> The `reset-gpios` and `cpm_crx` properties must be provided for CPM,
> >>> CPM5 and CPM5_HOST1. For CPM5NC, all three properties -
> >>> `reset-gpios`, `cpm_crx` and `cpm5nc_fw_attr` must be explicitly
> >>> defined to ensure
> >>
> >> This we see from the diff, but why they must be defined?
> >>
> >>> proper functionality.
> >>
> >> What functionality?
> >>
> >
> > For our controller, if cpm_crx is not provided lane errors will be observed.
> > Specifically for CPM5NC, if cpm5nc_fw_attr property is not provided,
> > the firewall is not cleared after reset and further PCIe transactions will not be
> allowed.
> > Therefore, these properties must be defined.
>
> This must be in the commit msg.
>

Sure, I will add this in commit message. Thanks.

> >
> >>>
> >>> Include an example DTS node and complete the binding documentation
> >>> for CPM5NC. Also, fix the bridge register address size in the
> >>> example for CPM5.
> >>>
> >>> Signed-off-by: Sai Krishna Musham <sai.krishna.musham@....com>
> >>> ---
> >>> Changes for v7:
> >>> - Update CPM5NC device tree binding.
> >>> - Add CPM5NC device tree example node.
> >>> - Update commit message.
> >>>
> >>> Changes for v6:
> >>> - Resolve ABI break.
> >>> - Update commit message.
> >>>
> >>
> >> ...
> >>
> >>> +  - if:
> >>> +      properties:
> >>> +        compatible:
> >>> +          contains:
> >>> +            enum:
> >>> +              - xlnx,versal-cpm5nc-host
> >>> +    then:
> >>> +      properties:
> >>> +        reg:
> >>> +          items:
> >>> +            - description: CPM system level control and status registers.
> >>> +            - description: Configuration space region and bridge registers.
> >>> +            - description: CPM clock and reset control registers.
> >>> +            - description: CPM5NC Firewall attribute register.
> >>> +          minItems: 2
> >>> +        reg-names:
> >>> +          items:
> >>> +            - const: cpm_slcr
> >>> +            - const: cfg
> >>> +            - const: cpm_crx
> >>> +            - const: cpm5nc_fw_attr
> >>> +          minItems: 2
> >>
> >> Why interrupts are not required for this variant? Why isn't this an
> >> interrupt controller?
> >>
> >
> > MSI and MSI-X interrupts are handled via GIC, so msi-map property is
> > required for interrupt handling.
> > Legacy interrupt support is not available, and Error interrupt support
> > will be added in future, once it is added corresponding DT changes will be added.
>
> I don't think commit msg explained this.
>

Sure, I will mention this also in commit message and send in next patch.

> >
>
>
>
> Best regards,
> Krzysztof

Thanks,
Sai Krishna