On 21.9.2018 14:38, Nicolas Ferre wrote: > Michal, > > On 20/09/2018 at 08:23, Michal Simek wrote: >> On 19.9.2018 20:08, Edgar E. Iglesias wrote: >>> On Wed, Sep 19, 2018 at 06:08:18PM +0200, Michal Simek wrote: >>>> Clear ADDR64 dma bit in DMACFG register in case that HW_DMA_CAP_64B >>>> is not detected on 64bit system. >>>> The issue was observed when bootloader(u-boot) does not check macb >>>> feature at DCFG6 register (DAW64_OFFSET) and enabling 64bit dma support >>>> by default. Then macb driver is reading DMACFG register back and only >>>> adding 64bit dma configuration but not cleaning it out. >>>> >>>> This is also align with other features which are also cleared if >>>> they are not >>>> present. >>> >>> Hi Michal, >>> >>>> >>>> Signed-off-by: Michal Simek <michal.si...@xilinx.com> >>>> --- >>>> >>>> drivers/net/ethernet/cadence/macb_main.c | 2 ++ >>>> 1 file changed, 2 insertions(+) >>>> >>>> diff --git a/drivers/net/ethernet/cadence/macb_main.c >>>> b/drivers/net/ethernet/cadence/macb_main.c >>>> index 16e4ef7d7185..79707dff3f13 100644 >>>> --- a/drivers/net/ethernet/cadence/macb_main.c >>>> +++ b/drivers/net/ethernet/cadence/macb_main.c >>>> @@ -2163,6 +2163,8 @@ static void macb_configure_dma(struct macb *bp) >>>> #ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT >>>> if (bp->hw_dma_cap & HW_DMA_CAP_64B) >>>> dmacfg |= GEM_BIT(ADDR64); >>>> + else >>>> + dmacfg &= ~GEM_BIT(ADDR64); >>>> #endif >>> >>> I think you might want to do this clearing outside of the #ifdef. >>> If CONFIG_ARCH_DMA_ADDR_T_64BIT is not defined, we'd want to make >>> sure the ADDR64 is cleared. E.g something like: >>> >>> dmacfg &= ~GEM_BIT(ADDR64); >>> #ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT >>> if (bp->hw_dma_cap & HW_DMA_CAP_64B) >>> dmacfg |= GEM_BIT(ADDR64); >>> #endif >>> >>> >>> Same thing for the USE_HWSTAMP/PTP flags below. >> >> Origin patch, which introduce this read with mask, >> macfg = gem_readl(bp, DMACFG) & ~GEM_BF(RXBS, -1L); >> was done in 2011 and from that time this function was extended a little >> bit. I am even not quite sure if make sense to read this reg and apply >> setting on the top of it. >> >> Nicolas: Isn't it better simply compose that reg from scratch? > > I have several arguments against composing this register from scratch: > > 1/ the reset value of this register is non-null for both of our > platforms and it could be meaningful to keep some of these values. > > 2/ one bitfield could use different values between Zynq and AT91: RXBMS > (1kB to 8kB for Zynq and 512 to 4KB for AT91), with same encoding. > > 3/ and well, this is the type of register with multiple bits that are > marked as "reserved" and that experience tells that they might be > connected to something... > > So, I'm all for correcting the code like what Edgar suggests.
ok. I have sent v2. M _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
