Messages in this thread | | | Date | Thu, 11 Jun 2020 16:59:31 +0100 | From | Marc Zyngier <> | Subject | Re: [PATCH v8 2/2] PCI: xilinx-cpm: Add Versal CPM Root Port driver |
| |
On 2020-06-11 16:51, Bharat Kumar Gogada wrote:
[...]
>> > +/** >> > + * xilinx_cpm_pcie_init_port - Initialize hardware >> > + * @port: PCIe port information >> > + */ >> > +static void xilinx_cpm_pcie_init_port(struct xilinx_cpm_pcie_port >> > *port) >> > +{ >> > + if (cpm_pcie_link_up(port)) >> > + dev_info(port->dev, "PCIe Link is UP\n"); >> > + else >> > + dev_info(port->dev, "PCIe Link is DOWN\n"); >> > + >> > + /* Disable all interrupts */ >> > + pcie_write(port, ~XILINX_CPM_PCIE_IDR_ALL_MASK, >> > + XILINX_CPM_PCIE_REG_IMR); >> > + >> > + /* Clear pending interrupts */ >> > + pcie_write(port, pcie_read(port, XILINX_CPM_PCIE_REG_IDR) & >> > + XILINX_CPM_PCIE_IMR_ALL_MASK, >> > + XILINX_CPM_PCIE_REG_IDR); >> > + >> > + /* Enable all interrupts */ >> > + pcie_write(port, XILINX_CPM_PCIE_IMR_ALL_MASK, >> > + XILINX_CPM_PCIE_REG_IMR); >> > + pcie_write(port, XILINX_CPM_PCIE_IDRN_MASK, >> > + XILINX_CPM_PCIE_REG_IDRN_MASK); >> >> No. I've explained in the previous review why this was a terrible >> thing to do, >> and my patch got rid of it for a good reason. >> >> If the mask/unmask calls do not work, please explain what is wrong, >> and let's >> fix them. But we DO NOT enable interrupts outside of an irqchip >> callback, full >> stop. > The issue here is, we do not have any other register to enable > interrupts for above > events, in order to see an interrupt assert for these events, the > respective mask bits > shall be set to 1.
I still disagree, because you're not explaining anything.
We enable the interrupts as they are requested already (that's why we write to the these register in the respective mask/unmask callbacks). Why do you need to enable them ahead of the request?
M. -- Jazz is not dead. It just smells funny...
| |