From: Florian Fainelli <f.faine...@gmail.com>
Date: Sep/10/2019, 20:01:01 (UTC+00:00)
> On 9/10/19 1:35 AM, Jose Abreu wrote:
> > From: Thierry Reding <thierry.red...@gmail.com>
> > 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.red...@gmail.com>
> >>> 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?
I don't think any of these will ever happen. Notice that this is already
been done for a long time in some registers that may not exist in some
random HW config and there is also the point that this is a write
operation so Slave Error would only get triggered if we did a read.
> 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?
Well, this is not hot-path. It's only done in HW open sequence. The
hot-path would be set_{rx/tx}_tail_ptr() but that's 32 bits only.
---
Thanks,
Jose Miguel Abreu
