Hello! On Tuesday 20 April 2021 14:17:21 Jianjun Wang wrote: > +static void mtk_pcie_enable_msi(struct mtk_pcie_port *port) > +{ > + int i; > + u32 val; > + > + for (i = 0; i < PCIE_MSI_SET_NUM; i++) { > + struct mtk_msi_set *msi_set = &port->msi_sets[i]; > + > + msi_set->base = port->base + PCIE_MSI_SET_BASE_REG + > + i * PCIE_MSI_SET_OFFSET; > + msi_set->msg_addr = port->reg_base + PCIE_MSI_SET_BASE_REG + > + i * PCIE_MSI_SET_OFFSET; > + > + /* Configure the MSI capture address */ > + writel_relaxed(lower_32_bits(msi_set->msg_addr), msi_set->base); > + writel_relaxed(upper_32_bits(msi_set->msg_addr), > + port->base + PCIE_MSI_SET_ADDR_HI_BASE + > + i * PCIE_MSI_SET_ADDR_HI_OFFSET); This looks like as setting MSI doorbell address to MSI doorbell address. > +static void mtk_compose_msi_msg(struct irq_data *data, struct msi_msg *msg) > +{ > + struct mtk_msi_set *msi_set = irq_data_get_irq_chip_data(data); > + struct mtk_pcie_port *port = data->domain->host_data; > + unsigned long hwirq; > + > + hwirq = data->hwirq % PCIE_MSI_IRQS_PER_SET; > + > + msg->address_hi = upper_32_bits(msi_set->msg_addr); > + msg->address_lo = lower_32_bits(msi_set->msg_addr); > + msg->data = hwirq; > + dev_dbg(port->dev, "msi#%#lx address_hi %#x address_lo %#x data %d\n", > + hwirq, msg->address_hi, msg->address_lo, msg->data); ... which is later used in compose_msi_msg(). Marc in some other patches for other pci controller drivers changed this address to just main "port" structure. It simplified implementations and also avoided need to declare additional member "msg_addr". Marc, would it be possible to simplify it also for this driver and just set msg_addr to virt_to_phys(port)?