linux-kernel - Re: [PATCH v3 2/5] PCI: sg2042: Add Sophgo SG2042 PCIe driver

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] [thread-next>] [day] [month] [year] [list]

Message-ID:
 <MA0PR01MB56715414B26AA601CFFB54C8FEFB2@MA0PR01MB5671.INDPRD01.PROD.OUTLOOK.COM>
Date: Mon, 17 Feb 2025 16:22:08 +0800
From: Chen Wang <unicorn_wang@...look.com>
To: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>, maz@...nel.org
Cc: Chen Wang <unicornxw@...il.com>, kw@...ux.com,
 u.kleine-koenig@...libre.com, aou@...s.berkeley.edu, arnd@...db.de,
 bhelgaas@...gle.com, conor+dt@...nel.org, guoren@...nel.org,
 inochiama@...look.com, krzk+dt@...nel.org, lee@...nel.org,
 lpieralisi@...nel.org, palmer@...belt.com, paul.walmsley@...ive.com,
 pbrobinson@...il.com, robh@...nel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org,
 linux-riscv@...ts.infradead.org, chao.wei@...hgo.com,
 xiaoguang.xing@...hgo.com, fengchun.li@...hgo.com, helgaas@...nel.org
Subject: Re: [PATCH v3 2/5] PCI: sg2042: Add Sophgo SG2042 PCIe driver


On 2025/1/23 1:34, Manivannan Sadhasivam wrote:

[......]
>>>> +/*
>>>> + * SG2042 PCIe controller supports two ways to report MSI:
>>>> + *
>>>> + * - Method A, the PCIe controller implements an MSI interrupt controller
>>>> + *   inside, and connect to PLIC upward through one interrupt line.
>>>> + *   Provides memory-mapped MSI address, and by programming the upper 32
>>>> + *   bits of the address to zero, it can be compatible with old PCIe devices
>>>> + *   that only support 32-bit MSI address.
>>>> + *
>>>> + * - Method B, the PCIe controller connects to PLIC upward through an
>>>> + *   independent MSI controller "sophgo,sg2042-msi" on the SOC. The MSI
>>>> + *   controller provides multiple(up to 32) interrupt sources to PLIC.
>>>> + *   Compared with the first method, the advantage is that the interrupt
>>>> + *   source is expanded, but because for SG2042, the MSI address provided by
>>>> + *   the MSI controller is fixed and only supports 64-bit address(> 2^32),
>>>> + *   it is not compatible with old PCIe devices that only support 32-bit MSI
>>>> + *   address.
>>>> + *
>>>> + * Method A & B can be configured in DTS, default is Method B.
>>> How to configure them? I can only see "sophgo,sg2042-msi" in the binding.
>>
>> The value of the msi-parent attribute is used in dts to distinguish them,
>> for example:
>>
>> ```dts
>>
>> msi: msi-controller@...0010300 {
>>      ......
>> };
>>
>> pcie_rc0: pcie@...0000000 {
>>      msi-parent = <&msi>;
>> };
>>
>> pcie_rc1: pcie@...2000000 {
>>      ......
>>      msi-parent = <&msi_pcie>;
>>      msi_pcie: interrupt-controller {
>>          ......
>>      };
>> };
>>
>> ```
>>
>> Which means:
>>
>> pcie_rc0 uses Method B
>>
>> pcie_rc1 uses Method A.
>>
> Ok. you mentioned 'default method' which is not accurate since the choice
> obviously depends on DT. Maybe you should say, 'commonly used method'? But both
> the binding and dts patches make use of in-built MSI controller only (method A).

"commonly used method" looks ok to me.

Binding example only shows the case for Method A, due to I think the 
writing of case for Method A  covers the writing of case for Method B.

DTS patches use both Method A and B. You can see patch 4 of this 
patchset, pcie_rc1 uses Method A, pcie_rc0 & pcie_rc2 use Method B.

> In general, for MSI implementations inside the PCIe IP, we don't usually add a
> dedicated devicetree node since the IP is the same. But in your reply to the my
> question on the bindings patch, you said it is a separate IP. I'm confused now.

I learned the writing of DTS from "brcm,iproc-pcie", see 
arch/arm/boot/dts/broadcom/bcm-cygnus.dtsi for example. Wouldn't it be 
clearer to embed an msi controller in topo?

And regarding what you said, "we don't usually add a dedicated 
devicetree node", do you have any example I can refer to?

Thanks,

Chen

[......]