[PATCH net-next 1/2] net: stmmac: EST interrupts handling and error reporting

[Jakub Kicinski]

On Tue, 16 Mar 2021 06:14:08 +0800 mohammad.athari.ismail at intel.com
wrote:
> From: Voon Weifeng <weifeng.voon at intel.com>
> 
> Enabled EST related interrupts as below:
> 1) Constant Gate Control Error (CGCE)
> 2) Head-of-Line Blocking due to Scheduling (HLBS)
> 3) Head-of-Line Blocking due to Frame Size (HLBF).
> 4) Base Time Register error (BTRE)
> 5) Switch to S/W owned list Complete (SWLC)
> 
> For HLBS, the user will get the info of all the queues that shows this
> error. For HLBF, the user will get the info of all the queue with the
> latest frame size which causes the error. Frame size 0 indicates no
> error.
> 
> The ISR handling takes place when EST feature is enabled by user.
> 
> Signed-off-by: Voon Weifeng <weifeng.voon at intel.com>
> Signed-off-by: Ong Boon Leong <boon.leong.ong at intel.com>
> Co-developed-by: Mohammad Athari Bin Ismail <mohammad.athari.ismail at intel.com>
> Signed-off-by: Mohammad Athari Bin Ismail <mohammad.athari.ismail at intel.com>

> +	if (status & HLBS) {
> +		value = readl(ioaddr + MTL_EST_SCH_ERR);
> +		value &= txqcnt_mask;
> +
> +		/* Clear Interrupt */
> +		writel(value, ioaddr + MTL_EST_SCH_ERR);
> +
> +		/* Collecting info to shows all the queues that has HLBS
> +		 * issue. The only way to clear this is to clear the
> +		 * statistic
> +		 */
> +		if (net_ratelimit())
> +			netdev_err(dev, "EST: HLB(sched) Queue %u\n", value);

This is a mask so probably better display it as hex?

> +	}
> +
> +	if (status & HLBF) {
> +		value = readl(ioaddr + MTL_EST_FRM_SZ_ERR);
> +		feqn = value & txqcnt_mask;
> +
> +		value = readl(ioaddr + MTL_EST_FRM_SZ_CAP);
> +		hbfq = (value & SZ_CAP_HBFQ_MASK(txqcnt)) >> SZ_CAP_HBFQ_SHIFT;
> +		hbfs = value & SZ_CAP_HBFS_MASK;
> +
> +		/* Clear Interrupt */
> +		writel(feqn, ioaddr + MTL_EST_FRM_SZ_ERR);
> +
> +		if (net_ratelimit())
> +			netdev_err(dev, "EST: HLB(size) Queue %u Size %u\n",
> +				   hbfq, hbfs);
> +	}
> +
> +	if (status & BTRE) {
> +		btrl = (status & BTRL) >> BTRL_SHIFT;
> +
> +		if (net_ratelimit())
> +			netdev_info(dev, "EST: BTR Error Loop Count %u\n",
> +				    btrl);
> +
> +		writel(BTRE, ioaddr + MTL_EST_STATUS);
> +	}
> +
> +	if (status & SWLC) {
> +		writel(SWLC, ioaddr + MTL_EST_STATUS);
> +		netdev_info(dev, "EST: SWOL has been switched\n");
> +	}
> +
> +	return status;

Caller never checks the return value, it probably should if this driver
supports shared irqs? Otherwise you can make this function void.

> +}