On 10/23/2025 11:39 PM, Jijie Shao wrote: > > 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. > Right, since it only affects this one cause. Got it. >> >> 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. > Sure that seems reasonable. Reviewed-by: Jacob Keller