Re: [PATCH net-next 1/2] net: stmmac: add stmmac_mac_irq_modify()

["Russell King (Oracle)" <linux@armlinux.org.uk>]

On Fri, Oct 24, 2025 at 07:01:59PM -0700, Jakub Kicinski wrote:
> On Thu, 23 Oct 2025 10:46:20 +0100 Russell King (Oracle) wrote:
> > Add a function to allow interrupts to be enabled and disabled in a
> > core independent manner.
> 
> Sorry for a general question but I'm curious why stick to the callback
> format this driver insists on. Looks like we could get away with
> parameterizing the code with the register offset via the priv structure.

Not quite - some cores, it's a mask (bits need to be set to be disabled).
Other cores, it's an enable (bits need to be set to enable). So one
can't get away with just "this is where the register is", it would need
three pieces of information - register offset, type of regster (mask or
enable) and then a core specific bitmask.

> Mostly curious. Personally, I'm always annoyed having to dig thru the
> indirections in this driver.

Me too, especially when it's not obvious what is an indirection and
what is not. There's the fun that a lot of the PTP-related indirection
actually has no difference. For example, at the bottom of
stmmac_hwtstamp.c, the struct stmmac_hwtimestamp initialisers have
almost all of the methods pointing at the same implementation
with the exeption of .get_ptptime, .timestamp_interrupt and
.hwtstamp_correct_latency.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!