Re: [PATCH net 1/3] net: hibmcge: fix rx buf avl irq is not re-enabled in irq_handle issue

[Jijie Shao]

on 2025/10/24 9:15, Jacob Keller wrote:
> On 10/21/2025 7:00 AM, Jijie Shao wrote:
> > irq initialized with the macro HBG_ERR_IRQ_I will automatically
> > be re-enabled, whereas those initialized with the macro HBG_IRQ_I
> > will not be re-enabled.
> >
> > Since the rx buf avl irq is initialized using the macro HBG_IRQ_I,
> > it needs to be actively re-enabled.
> This seems like it would be quite a severe issue. Do you have
> reproduction or example of what the failure state looks like?

priv->stats.rx_fifo_less_empty_thrsld_cnt can only be increased to 1
and cannot be increased further.

It is not a very serious issue, it affects the accuracy of a statistical item.

>  From the fixed commit, the RX_BUF_AVL used to be HBG_ERR_IRQ_I but now
> it uses HBG_IRQ_I so that it can have its own custom handler.. but
> HBG_IRQ_I doesn't set re_enable to true...
>
> It seems like a better fix would be having an HBG_ERR_IRQ_I variant that
> lets you pass your own function instead of making the handler have to do
> the hbg_hw_irq_enable call in its handler?

Currently, only the RX_BUF_AVL interrupt needs to be enabled separately.
Personally, I think it is acceptable to temporarily not use the an HBG_ERR_IRQ_I variant.

> > Fixes: fd394a334b1c ("net: hibmcge: Add support for abnormal irq handling feature")
> > Signed-off-by: Jijie Shao <shaojijie@xxxxxxxxxx>
> > ---
> >   drivers/net/ethernet/hisilicon/hibmcge/hbg_irq.c | 1 +
> >   1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_irq.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_irq.c
> > index 8af0bc4cca21..ae4cb35186d8 100644
> > --- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_irq.c
> > +++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_irq.c
> > @@ -32,6 +32,7 @@ static void hbg_irq_handle_rx_buf_val(struct hbg_priv *priv,
> >   				      const struct hbg_irq_info *irq_info)
> >   {
> >   	priv->stats.rx_fifo_less_empty_thrsld_cnt++;
> > +	hbg_hw_irq_enable(priv, irq_info->mask, true);
> >   }
> >   
> >   #define HBG_IRQ_I(name, handle) \