Hi Lorenzo,
On Tue, 4 Dec 2018 15:12:27 +0000 <lorenzo.pieralisi@xxxxxxx> wrote:
> On Wed, Nov 28, 2018 at 01:04:26PM +0900, Kunihiko Hayashi wrote:
>
> [...]
>
> > +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
