Re: [PATCH 19/42] PCI: aardvark: Fix setting MSI address

[Marc Zyngier]

On Thu, 06 May 2021 16:31:30 +0100,
Pali Rohár <pali@xxxxxxxxxx> wrote:
> 
> MSI address for receiving MSI interrupts needs to be correctly set before
> enabling processing of MSI interrupts.
> 
> Move code for setting PCIE_MSI_ADDR_LOW_REG and PCIE_MSI_ADDR_HIGH_REG
> registers with MSI address from advk_pcie_init_msi_irq_domain() function to
> advk_pcie_setup_hw() function before enabling PCIE_CORE_CTRL2_MSI_ENABLE.
> 
> As part of this change, also remove unused variable msi_msg, which was used
> only for MSI doorbell address. MSI address can be any address which does
> not conflict with PCI space.

Not quite. It can be any address that cannot be used to *DMA* to.

> So change it to the address of the main struct advk_pcie.
>
> Signed-off-by: Pali Rohár <pali@xxxxxxxxxx>
> Reviewed-by: Marek Behún <kabel@xxxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx # f21a8b1b6837 ("PCI: aardvark: Move to MSI handling using generic MSI support")
> ---
>  drivers/pci/controller/pci-aardvark.c | 21 +++++++++------------
>  1 file changed, 9 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
> index 5e0243b2c473..199015215779 100644
> --- a/drivers/pci/controller/pci-aardvark.c
> +++ b/drivers/pci/controller/pci-aardvark.c
> @@ -195,7 +195,6 @@ struct advk_pcie {
>  	struct msi_domain_info msi_domain_info;
>  	DECLARE_BITMAP(msi_used, MSI_IRQ_NUM);
>  	struct mutex msi_used_lock;
> -	u16 msi_msg;
>  	int link_gen;
>  	struct pci_bridge_emul bridge;
>  	struct gpio_desc *reset_gpio;
> @@ -325,6 +324,7 @@ static void advk_pcie_train_link(struct advk_pcie *pcie)
>  
>  static void advk_pcie_setup_hw(struct advk_pcie *pcie)
>  {
> +	phys_addr_t msi_addr;
>  	u32 reg;
>  
>  	/* Enable TX */
> @@ -381,6 +381,11 @@ static void advk_pcie_setup_hw(struct advk_pcie *pcie)
>  	reg |= LANE_COUNT_1;
>  	advk_writel(pcie, reg, PCIE_CORE_CTRL0_REG);
>  
> +	/* Set MSI address */
> +	msi_addr = virt_to_phys(pcie);
> +	advk_writel(pcie, lower_32_bits(msi_addr), PCIE_MSI_ADDR_LOW_REG);
> +	advk_writel(pcie, upper_32_bits(msi_addr), PCIE_MSI_ADDR_HIGH_REG);
> +
>  	/* Enable MSI */
>  	reg = advk_readl(pcie, PCIE_CORE_CTRL2_REG);
>  	reg |= PCIE_CORE_CTRL2_MSI_ENABLE;
> @@ -862,10 +867,10 @@ static void advk_msi_irq_compose_msi_msg(struct irq_data *data,
>  					 struct msi_msg *msg)
>  {
>  	struct advk_pcie *pcie = irq_data_get_irq_chip_data(data);
> -	phys_addr_t msi_msg = virt_to_phys(&pcie->msi_msg);
> +	phys_addr_t msi_addr = virt_to_phys(pcie);
>  
> -	msg->address_lo = lower_32_bits(msi_msg);
> -	msg->address_hi = upper_32_bits(msi_msg);
> +	msg->address_lo = lower_32_bits(msi_addr);
> +	msg->address_hi = upper_32_bits(msi_addr);
>  	msg->data = data->hwirq;
>  }
>  
> @@ -960,7 +965,6 @@ static int advk_pcie_init_msi_irq_domain(struct advk_pcie *pcie)
>  	struct device_node *node = dev->of_node;
>  	struct irq_chip *bottom_ic, *msi_ic;
>  	struct msi_domain_info *msi_di;
> -	phys_addr_t msi_msg_phys;
>  
>  	mutex_init(&pcie->msi_used_lock);
>  
> @@ -978,13 +982,6 @@ static int advk_pcie_init_msi_irq_domain(struct advk_pcie *pcie)
>  		MSI_FLAG_MULTI_PCI_MSI;
>  	msi_di->chip = msi_ic;
>  
> -	msi_msg_phys = virt_to_phys(&pcie->msi_msg);
> -
> -	advk_writel(pcie, lower_32_bits(msi_msg_phys),
> -		    PCIE_MSI_ADDR_LOW_REG);
> -	advk_writel(pcie, upper_32_bits(msi_msg_phys),
> -		    PCIE_MSI_ADDR_HIGH_REG);
> -
>  	pcie->msi_inner_domain =
>  		irq_domain_add_linear(NULL, MSI_IRQ_NUM,
>  				      &advk_msi_domain_ops, pcie);

Otherwise,

Acked-by: Marc Zyngier <maz@xxxxxxxxxx>

	M.

-- 
Without deviation from the norm, progress is not possible.