On 10/10/24 19:49, Manivannan Sadhasivam wrote: >> +static int rockchip_pcie_ep_setup_irq(struct pci_epc *epc) >> +{ >> + struct rockchip_pcie_ep *ep = epc_get_drvdata(epc); >> + struct rockchip_pcie *rockchip = &ep->rockchip; >> + struct device *dev = rockchip->dev; >> + int ret; >> + >> + if (!rockchip->ep_gpio) >> + return 0; >> + >> + /* PCIe reset interrupt */ >> + ep->perst_irq = gpiod_to_irq(rockchip->ep_gpio); >> + if (ep->perst_irq < 0) { >> + dev_err(dev, "No corresponding IRQ for PERST GPIO\n"); >> + return ep->perst_irq; >> + } >> + >> + ep->perst_asserted = true; > > How come? Yeah, a bit confusing. This is because the gpio active low / inactive high, so as soon as we enable the IRQ, we are going to get one IRQ even though perst gpio signal has not changed yet. So to be consistent with this, perst_asserted is initialized to true so that on the first shot of rockchip_pcie_ep_perst_irq_thread() we end up calling rockchip_pcie_ep_perst_assert() as if we got an assert from the host (we have not yet). I will add a comment clarifying that. -- Damien Le Moal Western Digital Research