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...
