On Thu, Aug 01, 2024 at 08:18:20PM -0700, jitendra.vegiraju@xxxxxxxxxxxx wrote: > +static int rd_dma_ch_ind(void __iomem *ioaddr, u8 mode, u32 channel) > +{ > + u32 reg_val = 0; > + u32 val = 0; val is unnecessary. > + > + reg_val |= mode << XGMAC4_MSEL_SHIFT & XGMAC4_MODE_SELECT; Consider using: reg_val |= FIELD_PREP(XGMAC4_MODE_SELECT, mode); and similarly everywhere else you use a shift and mask. With this, you can remove _all_ _SHIFT definitions in your header file. > + reg_val |= channel << XGMAC4_AOFF_SHIFT & XGMAC4_ADDR_OFFSET; > + reg_val |= XGMAC4_CMD_TYPE | XGMAC4_OB; > + writel(reg_val, ioaddr + XGMAC4_DMA_CH_IND_CONTROL); > + val = readl(ioaddr + XGMAC4_DMA_CH_IND_DATA); > + return val; return readl(ioaddr + XGMAC4_DMA_CH_IND_DATA); ... > +void dwxgmac4_dma_init(void __iomem *ioaddr, > + struct stmmac_dma_cfg *dma_cfg, int atds) > +{ > + u32 value; > + u32 i; > + > + value = readl(ioaddr + XGMAC_DMA_SYSBUS_MODE); > + > + if (dma_cfg->aal) > + value |= XGMAC_AAL; > + > + if (dma_cfg->eame) > + value |= XGMAC_EAME; What if dma_cfg doesn't have these bits set? Is it possible they will be set in the register? Thanks. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!