On 9/10/19 1:35 AM, Jose Abreu wrote: > From: Thierry Reding <thierry.reding@xxxxxxxxx> > Date: Sep/09/2019, 20:13:29 (UTC+00:00) > >> On Mon, Sep 09, 2019 at 04:05:52PM +0000, Jose Abreu wrote: >>> From: Thierry Reding <thierry.reding@xxxxxxxxx> >>> Date: Sep/09/2019, 16:25:46 (UTC+00:00) >>> >>>> @@ -79,6 +79,10 @@ static void dwmac4_dma_init_rx_chan(void __iomem *ioaddr, >>>> value = value | (rxpbl << DMA_BUS_MODE_RPBL_SHIFT); >>>> writel(value, ioaddr + DMA_CHAN_RX_CONTROL(chan)); >>>> >>>> + if (dma_cfg->eame) >>> >>> There is no need for this check. If EAME is not enabled then upper 32 >>> bits will be zero. >> >> The idea here was to potentially guard against this register not being >> available on some revisions. Having the check here would avoid access to >> the register if the device doesn't support enhanced addressing. > > I see your point but I don't think there will be any problems unless you > have some strange system that doesn't handle the write accesses to > unimplemented features properly ... Is not it then just safer to not do the write to a register that you do not know how the implementation is going to respond to with one of a target abort, timeout, decoding error, just dead lock? Also, would it make sense to consider adding an #ifdef CONFIG_PHYS_ADDR_T_64BIT plus the conditional check so that you can be slightly more optimal in the hot-path here? -- Florian
