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

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

On 04/04/2024 21:11, 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.


...

> +
> +static int qcom_pcie_ecam_suspend_noirq(struct device *dev)
> +{
> +	return pm_runtime_put_sync(dev);
> +}
> +
> +static int qcom_pcie_ecam_resume_noirq(struct device *dev)
> +{
> +	return pm_runtime_get_sync(dev);
> +}
> +
> +static int qcom_pcie_ecam_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct qcom_msi *msi;
> +	int ret;
> +
> +	ret = devm_pm_runtime_enable(dev);
> +	if (ret)
> +		return ret;
> +
> +	ret = pm_runtime_resume_and_get(dev);
> +	if (ret < 0) {
> +		dev_err(dev, "fail to enable pcie controller: %d\n", ret);
> +		return ret;
> +	}
> +
> +	msi = qcom_msi_init(dev);
> +	if (IS_ERR(msi)) {
> +		pm_runtime_put_sync(dev);
> +		return PTR_ERR(msi);
> +	}
> +
> +	ret = pci_host_common_probe(pdev);
> +	if (ret) {
> +		dev_err(dev, "pci_host_common_probe() failed:%d\n", ret);

Don't print function name, but instead say something useful. Above error
message is so not useful that just drop it.

> +		qcom_msi_deinit(msi);
> +		pm_runtime_put_sync(dev);
> +	}
> +
> +	return ret;
> +}
> +
> +static const struct dev_pm_ops qcom_pcie_ecam_pm_ops = {
> +	NOIRQ_SYSTEM_SLEEP_PM_OPS(qcom_pcie_ecam_suspend_noirq,
> +				qcom_pcie_ecam_resume_noirq)
> +};
> +
> +static const struct pci_ecam_ops qcom_pcie_ecam_ops = {
> +	.pci_ops	= {
> +		.map_bus	= pci_ecam_map_bus,
> +		.read		= pci_generic_config_read,
> +		.write		= pci_generic_config_write,
> +	}
> +};
> +
> +static const struct of_device_id qcom_pcie_ecam_of_match[] = {
> +	{
> +		.compatible	= "qcom,pcie-ecam-rc",
> +		.data		= &qcom_pcie_ecam_ops,

Why do you have ops/match data for generic compatible?

Best regards,
Krzysztof