diff mbox series

[1/2] PCI: mediatek: Allocate MSI address with dmam_alloc_coherent

Message ID 20231206083753.18186-2-jianjun.wang@mediatek.com
State New
Headers
Series PCI: mediatek: Allocate MSI address with dmam_alloc_coherent |

Commit Message

Jianjun Wang (王建军) Dec. 6, 2023, 8:37 a.m. UTC 
  Use 'dmam_alloc_coherent' to allocate the MSI address, instead of using
'virt_to_phys'.

Signed-off-by: Jianjun Wang <jianjun.wang@mediatek.com>
---
 drivers/pci/controller/pcie-mediatek.c | 29 ++++++++++++++++++--------
 1 file changed, 20 insertions(+), 9 deletions(-)

Comments

Bjorn Helgaas Dec. 6, 2023, 5:07 p.m. UTC | #1 
On Wed, Dec 06, 2023 at 04:37:52PM +0800, Jianjun Wang wrote:
> Use 'dmam_alloc_coherent' to allocate the MSI address, instead of using
> 'virt_to_phys'.

s/'dmam_alloc_coherent'/dmam_alloc_coherent()/
s/'virt_to_phys'/virt_to_phys()/

In subject also.

> @@ -732,8 +740,11 @@ static int mtk_pcie_startup_port_v2(struct mtk_pcie_port *port)
>  	val &= ~INTX_MASK;
>  	writel(val, port->base + PCIE_INT_MASK);
>  
> -	if (IS_ENABLED(CONFIG_PCI_MSI))
> -		mtk_pcie_enable_msi(port);
> +	if (IS_ENABLED(CONFIG_PCI_MSI)) {
> +		err = mtk_pcie_enable_msi(port);
> +		if (err)
> +			return err;

Is failure to enable MSI a fatal issue?  It looks like this will make
the host controller completely unusable if we can't set up MSI, even
if downstream PCI devices could use INTx and get along without MSI.

Bjorn
Jianjun Wang (王建军) Dec. 7, 2023, 8:18 a.m. UTC | #2 
Hi Bjorn,

On Wed, 2023-12-06 at 11:07 -0600, Bjorn Helgaas wrote:
>  On Wed, Dec 06, 2023 at 04:37:52PM +0800, Jianjun Wang wrote:
> > Use 'dmam_alloc_coherent' to allocate the MSI address, instead of
> using
> > 'virt_to_phys'.
> 
> s/'dmam_alloc_coherent'/dmam_alloc_coherent()/
> s/'virt_to_phys'/virt_to_phys()/
> 
> In subject also.
> 
> > @@ -732,8 +740,11 @@ static int mtk_pcie_startup_port_v2(struct
> mtk_pcie_port *port)
> >  val &= ~INTX_MASK;
> >  writel(val, port->base + PCIE_INT_MASK);
> >  
> > -if (IS_ENABLED(CONFIG_PCI_MSI))
> > -mtk_pcie_enable_msi(port);
> > +if (IS_ENABLED(CONFIG_PCI_MSI)) {
> > +err = mtk_pcie_enable_msi(port);
> > +if (err)
> > +return err;
> 
> Is failure to enable MSI a fatal issue?  It looks like this will make
> the host controller completely unusable if we can't set up MSI, even
> if downstream PCI devices could use INTx and get along without MSI.

This shouldn't be a fatal issue, we can still use INTx, I'll fix them
in the next version, thanks for your review.

Thanks.
> 
> Bjorn
diff mbox series

Patch

diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
index 66a8f73296fc..b080f7ca6da0 100644
--- a/drivers/pci/controller/pcie-mediatek.c
+++ b/drivers/pci/controller/pcie-mediatek.c
@@ -178,6 +178,7 @@  struct mtk_pcie_soc {
  * @phy: pointer to PHY control block
  * @slot: port slot
  * @irq: GIC irq
+ * @msg_addr: MSI message address
  * @irq_domain: legacy INTx IRQ domain
  * @inner_domain: inner IRQ domain
  * @msi_domain: MSI IRQ domain
@@ -198,6 +199,7 @@  struct mtk_pcie_port {
 	struct phy *phy;
 	u32 slot;
 	int irq;
+	dma_addr_t msg_addr;
 	struct irq_domain *irq_domain;
 	struct irq_domain *inner_domain;
 	struct irq_domain *msi_domain;
@@ -394,12 +396,10 @@  static struct pci_ops mtk_pcie_ops_v2 = {
 static void mtk_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
 {
 	struct mtk_pcie_port *port = irq_data_get_irq_chip_data(data);
-	phys_addr_t addr;
 
 	/* MT2712/MT7622 only support 32-bit MSI addresses */
-	addr = virt_to_phys(port->base + PCIE_MSI_VECTOR);
 	msg->address_hi = 0;
-	msg->address_lo = lower_32_bits(addr);
+	msg->address_lo = lower_32_bits(port->msg_addr);
 
 	msg->data = data->hwirq;
 
@@ -515,18 +515,26 @@  static int mtk_pcie_allocate_msi_domains(struct mtk_pcie_port *port)
 	return 0;
 }
 
-static void mtk_pcie_enable_msi(struct mtk_pcie_port *port)
+static int mtk_pcie_enable_msi(struct mtk_pcie_port *port)
 {
 	u32 val;
-	phys_addr_t msg_addr;
+	void *msi_vaddr;
 
-	msg_addr = virt_to_phys(port->base + PCIE_MSI_VECTOR);
-	val = lower_32_bits(msg_addr);
+	msi_vaddr = dmam_alloc_coherent(port->pcie->dev, sizeof(dma_addr_t), &port->msg_addr,
+					GFP_KERNEL);
+	if (!msi_vaddr) {
+		dev_err(port->pcie->dev, "failed to alloc and map MSI data\n");
+		return -ENOMEM;
+	}
+
+	val = lower_32_bits(port->msg_addr);
 	writel(val, port->base + PCIE_IMSI_ADDR);
 
 	val = readl(port->base + PCIE_INT_MASK);
 	val &= ~MSI_MASK;
 	writel(val, port->base + PCIE_INT_MASK);
+
+	return 0;
 }
 
 static void mtk_pcie_irq_teardown(struct mtk_pcie *pcie)
@@ -732,8 +740,11 @@  static int mtk_pcie_startup_port_v2(struct mtk_pcie_port *port)
 	val &= ~INTX_MASK;
 	writel(val, port->base + PCIE_INT_MASK);
 
-	if (IS_ENABLED(CONFIG_PCI_MSI))
-		mtk_pcie_enable_msi(port);
+	if (IS_ENABLED(CONFIG_PCI_MSI)) {
+		err = mtk_pcie_enable_msi(port);
+		if (err)
+			return err;
+	}
 
 	/* Set AHB to PCIe translation windows */
 	val = lower_32_bits(mem->start) |