Hi, On 12/12/2018 13:39, Eugeniy Paltsev wrote: > Hi Gustavo, > > On Wed, 2018-12-12 at 12:13 +0100, Gustavo Pimentel wrote: >> Add support for the eDMA IP version 0 driver for both register maps (legacy >> and unroll). > [snip] >> diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c >> new file mode 100644 >> index 0000000..cc362b0 >> --- /dev/null >> +++ b/drivers/dma/dw-edma/dw-edma-v0-core.c >> @@ -0,0 +1,346 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +// Copyright (c) 2018 Synopsys, Inc. and/or its affiliates. >> +// Synopsys DesignWare eDMA v0 core >> + >> +#include "dw-edma-core.h" >> +#include "dw-edma-v0-core.h" >> +#include "dw-edma-v0-regs.h" >> +#include "dw-edma-v0-debugfs.h" >> + >> +#define QWORD_HI(value) ((value & 0xFFFFFFFF00000000llu) >> 32) > Use upper_32_bits() macros. > >> +#define QWORD_LO(value) (value & 0x00000000FFFFFFFFllu) > Use lower_32_bits() macros. Ok, makes sense. > >> +enum dw_edma_control { >> + DW_EDMA_CB = BIT(0), >> + DW_EDMA_TCB = BIT(1), >> + DW_EDMA_LLP = BIT(2), >> + DW_EDMA_LIE = BIT(3), >> + DW_EDMA_RIE = BIT(4), > > > [snip] >> + viewport_sel = (ch & 0x00000007ul); > [snip] >> +void dw_edma_v0_core_off(struct dw_edma *dw) >> +{ >> + SET_BOTH(dw, int_mask, 0x00FF00FFul); >> + SET_BOTH(dw, int_clear, 0x00FF00FFul); >> + SET_BOTH(dw, engine_en, 0x00000000ul); >> +} > [snip] >> + num_ch &= 0x0000000Ful; >> + } else { >> + num_ch &= 0x000F0000ul; > > I guess it's better to name this magic numbers via defines. > Ok, also makes sense. > > [snip] > >> + >> +bool dw_edma_v0_core_status_done_int(struct dw_edma_chan *chan) >> +{ >> + struct dw_edma *dw = chan->chip->dw; >> + u32 tmp; >> + >> + tmp = GET_RW(dw, chan->dir, int_status); >> + tmp &= BIT(chan->id); >> + >> + return tmp ? true : false; > > return !!tmp; Ok, I'll do it. > >> +} >> + >> +bool dw_edma_v0_core_status_abort_int(struct dw_edma_chan *chan) >> +{ >> + struct dw_edma *dw = chan->chip->dw; >> + u32 tmp; >> + >> + tmp = GET_RW(dw, chan->dir, int_status); >> + tmp &= BIT(chan->id + 16); >> + >> + return tmp ? true : false; > ditto. > Thanks!