Hi Francois,
> [...]
> > +int spl2sw_rx_poll(struct napi_struct *napi, int budget) {
> [...]
> > + wmb(); /* make sure settings are effective. */
> > + mask = readl(comm->l2sw_reg_base + L2SW_SW_INT_MASK_0);
> > + mask &= ~MAC_INT_RX;
> > + writel(mask, comm->l2sw_reg_base + L2SW_SW_INT_MASK_0);
> > +
> > + napi_complete(napi);
> > + return 0;
> > +}
> > +
> > +int spl2sw_tx_poll(struct napi_struct *napi, int budget) {
> [...]
> > + wmb(); /* make sure settings are effective. */
> > + mask = readl(comm->l2sw_reg_base + L2SW_SW_INT_MASK_0);
> > + mask &= ~MAC_INT_TX;
> > + writel(mask, comm->l2sw_reg_base + L2SW_SW_INT_MASK_0);
> > +
> > + napi_complete(napi);
> > + return 0;
> > +}
> > +
> > +irqreturn_t spl2sw_ethernet_interrupt(int irq, void *dev_id) {
> [...]
> > + if (status & MAC_INT_RX) {
> > + /* Disable RX interrupts. */
> > + mask = readl(comm->l2sw_reg_base + L2SW_SW_INT_MASK_0);
> > + mask |= MAC_INT_RX;
> > + writel(mask, comm->l2sw_reg_base + L2SW_SW_INT_MASK_0);
> [...]
> > + napi_schedule(&comm->rx_napi);
> > + }
> > +
> > + if (status & MAC_INT_TX) {
> > + /* Disable TX interrupts. */
> > + mask = readl(comm->l2sw_reg_base + L2SW_SW_INT_MASK_0);
> > + mask |= MAC_INT_TX;
> > + writel(mask, comm->l2sw_reg_base + L2SW_SW_INT_MASK_0);
> > +
> > + if (unlikely(status & MAC_INT_TX_DES_ERR)) {
> [...]
> > + } else {
> > + napi_schedule(&comm->tx_napi);
> > + }
> > + }
>
> The readl/writel sequence in rx_poll (or tx_poll) races with the irq handler performing
> MAC_INT_TX (or MAC_INT_RX) work. If the readl returns the same value to both callers,
> one of the writel will be overwritten.
>
> --
> Ueimor
I will add disable_irq() and enable_irq() for spl2sw_rx_poll() and spl2sw_tx_poll() as shown below:
spl2sw_rx_poll():
wmb(); /* make sure settings are effective. */
disable_irq(comm->irq);
mask = readl(comm->l2sw_reg_base + L2SW_SW_INT_MASK_0);
mask &= ~MAC_INT_RX;
writel(mask, comm->l2sw_reg_base + L2SW_SW_INT_MASK_0);
enable_irq(comm->irq);
spl2sw_tx_poll():
wmb(); /* make sure settings are effective. */
disable_irq(comm->irq);
mask = readl(comm->l2sw_reg_base + L2SW_SW_INT_MASK_0);
mask &= ~MAC_INT_TX;
writel(mask, comm->l2sw_reg_base + L2SW_SW_INT_MASK_0);
enable_irq(comm->irq);
Is the modification ok?
Thank you for your review.
Best regards,
Wells
