On Fri, Aug 02, 2024 at 11:47:22AM GMT, Russell King (Oracle) wrote: > Pass the stmmac_pcs into dwmac_pcs_isr() so that we have the base > address of the PCS block available. nitpicky, but I think it would be nice say something like "stmmac_pcs already contains the base address of the PCS registers. Pass that in instead of recalculating the base address again" if I'm following the motivation correctly. > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.h b/drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.h > index 083128e0013c..c73a08dab7b2 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.h > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pcs.h > @@ -61,18 +61,18 @@ > > /** > * dwmac_pcs_isr - TBI, RTBI, or SGMII PHY ISR > - * @ioaddr: IO registers pointer > + * @spcs: pointer to &struct stmmac_pcs > * @reg: Base address of the AN Control Register. > * @intr_status: GMAC core interrupt status > * @x: pointer to log these events as stats > * Description: it is the ISR for PCS events: Auto-Negotiation Completed and > * Link status. > */ > -static inline void dwmac_pcs_isr(void __iomem *ioaddr, u32 reg, > +static inline void dwmac_pcs_isr(struct stmmac_pcs *spcs, > unsigned int intr_status, > struct stmmac_extra_stats *x) Please drop the reg variable from the kerneldoc, you've annihilated it! Thanks, Andrew