Re: [RFC PATCH 2/2] PCI: Add Qualcomm PCIe ECAM root complex driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 6/5/2024 7:39 PM, Manivannan Sadhasivam wrote:
On Fri, May 31, 2024 at 03:47:24PM -0700, Mayank Rana wrote:
Hi Rob / Mani

On 4/15/2024 4:30 PM, Mayank Rana wrote:
Hi Rob

Excuse me for late response on this (was OOO).
On 4/10/2024 9:58 AM, Rob Herring wrote:
On Mon, Apr 08, 2024 at 11:57:58AM -0700, Mayank Rana wrote:
Hi Mani

On 4/5/2024 9:17 PM, Manivannan Sadhasivam wrote:
On Fri, Apr 05, 2024 at 10:41:15AM -0700, Mayank Rana wrote:
Hi Mani

On 4/4/2024 10:30 PM, Manivannan Sadhasivam wrote:
On Thu, Apr 04, 2024 at 12:11:24PM -0700, Mayank Rana wrote:
On some of Qualcomm platform, firmware
configures PCIe controller into
ECAM mode allowing static memory allocation for
configuration space of
supported bus range. Firmware also takes care of
bringing up PCIe PHY
and performing required operation to bring PCIe
link into D0. Firmware
also manages system resources (e.g.
clocks/regulators/resets/ bus voting).
Hence add Qualcomm PCIe ECAM root complex driver
which enumerates PCIe
root complex and connected PCIe devices.
Firmware won't be enumerating
or powering up PCIe root complex until this
driver invokes power domain
based notification to bring PCIe link into D0/D3cold mode.


Is this an in-house PCIe IP of Qualcomm or the same
DWC IP that is used in other
SoCs?

- Mani
Driver is validated on SA8775p-ride platform using PCIe DWC IP for
now.Although this driver doesn't need to know used PCIe
controller and PHY
IP as well programming sequence as that would be taken
care by firmware.


Ok, so it is the same IP but firmware is controlling the
resources now. This
information should be present in the commit message.

Btw, there is an existing generic ECAM host controller driver:
drivers/pci/controller/pci-host-generic.c

This driver is already being used by several vendors as
well. So we should try
to extend it for Qcom usecase also.

I would take it a bit further and say if you need your own driver, then
just use the default QCom driver. Perhaps extend it to support ECAM.
Better yet, copy your firmware setup and always configure the QCom h/w
to use ECAM.
Good suggestion. Although here we are having 2 set of requirements:
1. ECAM configuration
2. Managing PCIe controller and PHY resources and programming from
firmware as well
Hence it is not feasible to use default QCOM driver.
If you want to extend the generic driver, that's fine, but we don't need
a 3rd.
I did consider this part before coming up with new driver. Although I
felt that
below mentioned functionality may not look more generic to be part of
pci-host-generic.c driver.
I did review pci-host-generic.c driver for usage. although there
are more
functionalityneeded for use case purpose as below:
1. MSI functionality

Pretty sure the generic driver already supports that.
I don't find any MSI support with pci-host-generic.c driver.
2. Suspend/Resume

Others might want that to work as well.
Others firmware won't have way to handle D3cold and D0 functionality
handling as
needed here for supporting suspend/resume as I don't find any interface
for pci-host-generic.c driver to notify firmware. here we are having way
to talk to firmware using GenPD based power domain usage to communicate
with firmware.

3. Wakeup Functionality (not part of current change, but would be added
later)

Others might want that to work as well.
possible if suspend/resume support is available or used.
4. Here this driver provides way to virtualized PCIe controller.
So VMs only
talk to a generic ECAM whereas HW is only directed accessed by
service VM.

That's the existing driver. If if doesn't work for a VM, fix the VM.
Correct.
5. Adding more Auto based safety use cases related implementation

Now that's just hand waving.
Here I am trying to provide new set of changes plan to be added as part
of required functionality.

Hence keeping pci-host-generic.c as generic driver where above
functionality
may not be needed.

Duplicating things to avoid touching existing drivers is not how kernel
development works.
I shall try your suggestion and see how it looks in terms of code
changes. Perhaps then we can have more clarity in terms of adding more
functionality into generic or having separate driver.
I just learnt that previously dwc related PCIe ECAM driver and MSI
controller driver tried out as:

https://lore.kernel.org/linux-pci/20170821192907.8695-1-ard.biesheuvel@xxxxxxxxxx/

Although there were few concerns at that time. Due to that having dwc
specific MSI functionality based driver was dropped, and pci-host-generic.c
driver is being updated using with dwc/snps specific ECAM operation.

In current discussion, it seems that we are discussing to have identical
approach here.

Atleast on Qualcomm SA8775p platform, I don't have any other way to support
MSI functionality i.e. extended SPI or ITS/LPI based MSI or using GICv2m
functionality are not supported.

I don't see any other approach other than MSI based implementation within
pci-host-generic.c driver for dwc/snps based MSI controller.

Do you have any suggestion on this ?


Since this ECAM driver is going to be used in newer Qcom SoCs, why can't you use
GICv3 for MSI handling?
Yes, that is plan further as look like we have limitation on just SA8775.
So I see two options here:
1. Update pcie-host-generic.c without MSI based functionality, and leave with MSI functionality differently on SA8775 2. Also possible to make pcie-host-designware.c based MSI functionality as separate driver, and try to use with pcie-host-generic.c driver. That way we would still use existing MSI related code base, and able to use with ECAM driver.

Do you see using above option 2 as good way to allow SNPS/DWC based MSI controller functionality with ECAM and Non-ECAM driver ?

Regards,
Mayank




[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux