[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <09f9acc1-d1ad-4971-8488-f0268cf08799@rock-chips.com>
Date: Thu, 22 Jan 2026 16:15:52 +0800
From: Shawn Lin <shawn.lin@...k-chips.com>
To: Qiang Yu <qiang.yu@....qualcomm.com>, Jingoo Han <jingoohan1@...il.com>,
Manivannan Sadhasivam <mani@...nel.org>,
Lorenzo Pieralisi <lpieralisi@...nel.org>,
Krzysztof Wilczyński <kwilczynski@...nel.org>,
Rob Herring <robh@...nel.org>, Bjorn Helgaas <bhelgaas@...gle.com>
Cc: shawn.lin@...k-chips.com, linux-pci@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org
Subject: Re: [PATCH 2/2] PCI: dwc: Rename has_msi_ctrl to uses_imsi_rx for
clarity
Hi Qiang,
在 2026/01/22 星期四 15:45, Qiang Yu 写道:
> The current "has_msi_ctrl" flag name is misleading because it suggests the
> presence of any MSI controller, while it is specifically set for platforms
> that lack .msi_init() callback and don't have "msi-parent" or "msi-map"
> device tree properties, indicating they rely on the iMSI-RX module for MSI
> functionality.
>
> Rename it to "uses_imsi_rx" to make the intent clear:
> - When true: Platform uses the iMSI-RX module for MSI handling
> - When false: Platform has other MSI controller support (ITS/MBI, custom
> handlers)
>
> No functional changes, only improves code readability and eliminates
> naming confusion.
>
I love this patch, it's indeed confusing. But I noticed the naming
seems inconsistency, for instance, there are already use_atu_msg,
use_parent_dt_ranges, etc. Should we use use_imsi_rx instead?
But in any case,
Reviewed-by: Shawn Lin <shawn.lin@...k-chips.com>
> Signed-off-by: Qiang Yu <qiang.yu@....qualcomm.com>
> ---
> drivers/pci/controller/dwc/pcie-designware-host.c | 16 ++++++++--------
> drivers/pci/controller/dwc/pcie-designware.h | 2 +-
> drivers/pci/controller/dwc/pcie-qcom.c | 4 ++--
> 3 files changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index a17833dd6f9d4b4aa06dc1fe09ffed5e2c28a82f..e7dbdf3670b426783e778be5064def10d2b209b1 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -255,7 +255,7 @@ void dw_pcie_msi_init(struct dw_pcie_rp *pp)
> u64 msi_target = (u64)pp->msi_data;
> u32 ctrl, num_ctrls;
>
> - if (!pci_msi_enabled() || !pp->has_msi_ctrl)
> + if (!pci_msi_enabled() || !pp->uses_imsi_rx)
> return;
>
> num_ctrls = pp->num_vectors / MAX_MSI_IRQS_PER_CTRL;
> @@ -603,15 +603,15 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
> }
>
> if (pci_msi_enabled()) {
> - pp->has_msi_ctrl = !(pp->ops->msi_init ||
> + pp->uses_imsi_rx = !(pp->ops->msi_init ||
> of_property_present(np, "msi-parent") ||
> of_property_present(np, "msi-map"));
>
> /*
> - * For the has_msi_ctrl case the default assignment is handled
> + * For the uses_imsi_rx case the default assignment is handled
> * in the dw_pcie_msi_host_init().
> */
> - if (!pp->has_msi_ctrl && !pp->num_vectors) {
> + if (!pp->uses_imsi_rx && !pp->num_vectors) {
> pp->num_vectors = MSI_DEF_NUM_VECTORS;
> } else if (pp->num_vectors > MAX_MSI_IRQS) {
> dev_err(dev, "Invalid number of vectors\n");
> @@ -623,7 +623,7 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
> ret = pp->ops->msi_init(pp);
> if (ret < 0)
> goto err_deinit_host;
> - } else if (pp->has_msi_ctrl) {
> + } else if (pp->uses_imsi_rx) {
> ret = dw_pcie_msi_host_init(pp);
> if (ret < 0)
> goto err_deinit_host;
> @@ -701,7 +701,7 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
> dw_pcie_edma_remove(pci);
>
> err_free_msi:
> - if (pp->has_msi_ctrl)
> + if (pp->uses_imsi_rx)
> dw_pcie_free_msi(pp);
>
> err_deinit_host:
> @@ -729,7 +729,7 @@ void dw_pcie_host_deinit(struct dw_pcie_rp *pp)
>
> dw_pcie_edma_remove(pci);
>
> - if (pp->has_msi_ctrl)
> + if (pp->uses_imsi_rx)
> dw_pcie_free_msi(pp);
>
> if (pp->ops->deinit)
> @@ -1130,7 +1130,7 @@ int dw_pcie_setup_rc(struct dw_pcie_rp *pp)
> * the MSI and MSI-X capabilities of the Root Port to allow the drivers
> * to fall back to INTx instead.
> */
> - if (pp->has_msi_ctrl) {
> + if (pp->uses_imsi_rx) {
> dw_pcie_remove_capability(pci, PCI_CAP_ID_MSI);
> dw_pcie_remove_capability(pci, PCI_CAP_ID_MSIX);
> }
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 53b65428fadb3d905b02bdcc06667065574f4f9d..33154bc89dd3b7d7fbe0ea749ca22bdc8292489c 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -416,7 +416,7 @@ struct dw_pcie_host_ops {
> };
>
> struct dw_pcie_rp {
> - bool has_msi_ctrl:1;
> + bool uses_imsi_rx:1;
> bool cfg0_io_shared:1;
> u64 cfg0_base;
> void __iomem *va_cfg0_base;
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 13e6c334e10d21b9ebfe5f82de0aff3bce6191e3..999f5e083cef4e78b85a0111d2a90c3de65c83b5 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -1633,7 +1633,7 @@ static void qcom_pci_free_msi(void *ptr)
> {
> struct dw_pcie_rp *pp = (struct dw_pcie_rp *)ptr;
>
> - if (pp && pp->has_msi_ctrl)
> + if (pp && pp->uses_imsi_rx)
> dw_pcie_free_msi(pp);
> }
>
> @@ -1657,7 +1657,7 @@ static int qcom_pcie_ecam_host_init(struct pci_config_window *cfg)
> if (ret)
> return ret;
>
> - pp->has_msi_ctrl = true;
> + pp->uses_imsi_rx = true;
> dw_pcie_msi_init(pp);
>
> return devm_add_action_or_reset(dev, qcom_pci_free_msi, pp);
>
Powered by blists - more mailing lists