Hi Lorenzo,

On Tue, 4 Dec 2018 15:12:27 +0000 [off-list ref] wrote:

On Wed, Nov 28, 2018 at 01:04:26PM +0900, Kunihiko Hayashi wrote:

[...]

quoted

+static void uniphier_pcie_irq_ack(struct irq_data *d)
+{
+	struct pcie_port *pp = irq_data_get_irq_chip_data(d);
+	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+	struct uniphier_pcie_priv *priv = to_uniphier_pcie(pci);
+	u32 val;
+
+	val = readl(priv->base + PCL_RCV_INTX);
+	val &= ~PCL_RCV_INTX_ALL_STATUS;
+	val |= BIT(irqd_to_hwirq(d) + PCL_RCV_INTX_STATUS_SHIFT);
+	writel(val, priv->base + PCL_RCV_INTX);
+}
+
+static void uniphier_pcie_irq_mask(struct irq_data *d)
+{
+	struct pcie_port *pp = irq_data_get_irq_chip_data(d);
+	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+	struct uniphier_pcie_priv *priv = to_uniphier_pcie(pci);
+	u32 val;
+
+	val = readl(priv->base + PCL_RCV_INTX);
+	val &= ~PCL_RCV_INTX_ALL_STATUS;
+	val |= BIT(irqd_to_hwirq(d) + PCL_RCV_INTX_MASK_SHIFT);
+	writel(val, priv->base + PCL_RCV_INTX);
+}
+
+static void uniphier_pcie_irq_unmask(struct irq_data *d)
+{
+	struct pcie_port *pp = irq_data_get_irq_chip_data(d);
+	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+	struct uniphier_pcie_priv *priv = to_uniphier_pcie(pci);
+	u32 val;
+
+	val = readl(priv->base + PCL_RCV_INTX);
+	val &= ~PCL_RCV_INTX_ALL_STATUS;

I have noticed this operation is carried out on ACK/MASK/UNMASK,
what's its purpose ?

Thanks for pointing out. This is wrong.

The register PCL_RCV_INTX has 3 parts of bits.

#define PCL_RCV_INTX_ALL_ENABLE         GENMASK(19, 16)
#define PCL_RCV_INTX_ALL_MASK           GENMASK(11, 8)
#define PCL_RCV_INTX_ALL_STATUS         GENMASK(3, 0)

In the mask/unmask operation,
We should use PCL_RCV_INTX_ALL_MASK to represent 'mask bits',
not PCL_RCV_INTX_ALL_STATUS.
And the ack operation is effective by writing 1 to 'status bits'.

Thank you,

---
Best Regards,
Kunihiko Hayashi