Re: [PATCH v13 04/11] PCI: stm32: Add PCIe host support for STM32MP25

[Christian Bruel <christian.bruel@foss.st.com>]

On 8/25/25 11:15, Philipp Zabel wrote:
> On Mi, 2025-08-20 at 09:54 +0200, Christian Bruel wrote:
>> Add driver for the STM32MP25 SoC PCIe Gen1 2.5 GT/s and Gen2 5GT/s
>> controller based on the DesignWare PCIe core.
>>
>> Supports MSI via GICv2m, Single Virtual Channel, Single Function
>>
>> Supports WAKE# GPIO.
>>
>> Signed-off-by: Christian Bruel <christian.bruel@foss.st.com>
>> ---
>>   drivers/pci/controller/dwc/Kconfig      |  12 +
>>   drivers/pci/controller/dwc/Makefile     |   1 +
>>   drivers/pci/controller/dwc/pcie-stm32.c | 360 ++++++++++++++++++++++++
>>   drivers/pci/controller/dwc/pcie-stm32.h |  15 +
>>   4 files changed, 388 insertions(+)
>>   create mode 100644 drivers/pci/controller/dwc/pcie-stm32.c
>>   create mode 100644 drivers/pci/controller/dwc/pcie-stm32.h
>>
>> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
>> index deafc512b079..a8174817fd5b 100644
>> --- a/drivers/pci/controller/dwc/Kconfig
>> +++ b/drivers/pci/controller/dwc/Kconfig
>> @@ -423,6 +423,18 @@ config PCIE_SPEAR13XX
>>   	help
>>   	  Say Y here if you want PCIe support on SPEAr13XX SoCs.
>>   
>> +config PCIE_STM32_HOST
>> +	tristate "STMicroelectronics STM32MP25 PCIe Controller (host mode)"
>> +	depends on ARCH_STM32 || COMPILE_TEST
>> +	depends on PCI_MSI
>> +	select PCIE_DW_HOST
>> +	help
>> +	  Enables Root Complex (RC) support for the DesignWare core based PCIe
>> +	  controller found in STM32MP25 SoC.
>> +
>> +	  This driver can also be built as a module. If so, the module
>> +	  will be called pcie-stm32.
>> +
>>   config PCI_DRA7XX
>>   	tristate
>>   
>> diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
>> index 6919d27798d1..1307a87b1cf0 100644
>> --- a/drivers/pci/controller/dwc/Makefile
>> +++ b/drivers/pci/controller/dwc/Makefile
>> @@ -31,6 +31,7 @@ obj-$(CONFIG_PCIE_UNIPHIER) += pcie-uniphier.o
>>   obj-$(CONFIG_PCIE_UNIPHIER_EP) += pcie-uniphier-ep.o
>>   obj-$(CONFIG_PCIE_VISCONTI_HOST) += pcie-visconti.o
>>   obj-$(CONFIG_PCIE_RCAR_GEN4) += pcie-rcar-gen4.o
>> +obj-$(CONFIG_PCIE_STM32_HOST) += pcie-stm32.o
>>   
>>   # The following drivers are for devices that use the generic ACPI
>>   # pci_root.c driver but don't support standard ECAM config access.
>> diff --git a/drivers/pci/controller/dwc/pcie-stm32.c b/drivers/pci/controller/dwc/pcie-stm32.c
>> new file mode 100644
>> index 000000000000..964fa6f674c8
>> --- /dev/null
>> +++ b/drivers/pci/controller/dwc/pcie-stm32.c
>> @@ -0,0 +1,360 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * STMicroelectronics STM32MP25 PCIe root complex driver.
>> + *
>> + * Copyright (C) 2025 STMicroelectronics
>> + * Author: Christian Bruel <christian.bruel@foss.st.com>
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/phy/phy.h>
>> +#include <linux/pinctrl/consumer.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/pm_runtime.h>
>> +#include <linux/pm_wakeirq.h>
>> +#include <linux/regmap.h>
>> +#include <linux/reset.h>
>> +#include "pcie-designware.h"
>> +#include "pcie-stm32.h"
>> +#include "../../pci.h"
>> +
>> +struct stm32_pcie {
>> +	struct dw_pcie pci;
>> +	struct regmap *regmap;
>> +	struct reset_control *rst;
> 
> This could be a local variable in stm32_pcie_probe().


Thank you for pointing that out.

Since we use the same common resources in stm32_pcie for both the host 
and endpoint drivers, aligning the same fields in the struct stm32_pcie 
seems more consistent.

Additionally, we could improve the code by moving regmap, clk, and rst 
out of probe into a new function, stm32_pcie_resource_get().

Which approach do you think is best? Moving rst to stm32_pcie_probe() 
offers slight optimization, while using a new stm32_pcie_resource_get() 
provides better modularity.

Shall I re-spin a v14 with either of these options?

thank you,

Christian

> 
> regards
> Philipp