| Message ID | 20220309211204.26050-8-Frank.Li@nxp.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Enable designware PCI EP EDMA locally. | expand |
Hi Frank,
I love your patch! Yet something to improve:
[auto build test ERROR on vkoul-dmaengine/next]
[also build test ERROR on helgaas-pci/next linus/master v5.17-rc7 next-20220309]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Frank-Li/Enable-designware-PCI-EP-EDMA-locally/20220310-051510
base: https://git.kernel.org/pub/scm/linux/kernel/git/vkoul/dmaengine.git next
config: csky-buildonly-randconfig-r004-20220309 (https://download.01.org/0day-ci/archive/20220310/202203100843.qzRV56ko-lkp@intel.com/config)
compiler: csky-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/f7947d784b0fe089bdaef2fee8e57f84e390d3f2
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Frank-Li/Enable-designware-PCI-EP-EDMA-locally/20220310-051510
git checkout f7947d784b0fe089bdaef2fee8e57f84e390d3f2
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=csky SHELL=/bin/bash drivers/dma/dw-edma/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
drivers/dma/dw-edma/dw-edma-v0-core.c: In function 'dw_edma_v0_core_start':
>> drivers/dma/dw-edma/dw-edma-v0-core.c:427:25: error: implicit declaration of function 'SET_CH_64'; did you mean 'SET_CH_32'? [-Werror=implicit-function-declaration]
427 | SET_CH_64(dw, chan->dir, chan->id, llp.reg,
| ^~~~~~~~~
| SET_CH_32
>> drivers/dma/dw-edma/dw-edma-v0-core.c:427:60: error: 'llp' undeclared (first use in this function)
427 | SET_CH_64(dw, chan->dir, chan->id, llp.reg,
| ^~~
drivers/dma/dw-edma/dw-edma-v0-core.c:427:60: note: each undeclared identifier is reported only once for each function it appears in
cc1: some warnings being treated as errors
vim +427 drivers/dma/dw-edma/dw-edma-v0-core.c
359
360 void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
361 {
362 struct dw_edma_chan *chan = chunk->chan;
363 struct dw_edma *dw = chan->dw;
364 u32 tmp;
365
366 dw_edma_v0_core_write_chunk(chunk);
367
368 if (first) {
369 /* Enable engine */
370 SET_RW_32(dw, chan->dir, engine_en, BIT(0));
371 if (dw->chip->mf == EDMA_MF_HDMA_COMPAT) {
372 switch (chan->id) {
373 case 0:
374 SET_RW_COMPAT(dw, chan->dir, ch0_pwr_en,
375 BIT(0));
376 break;
377 case 1:
378 SET_RW_COMPAT(dw, chan->dir, ch1_pwr_en,
379 BIT(0));
380 break;
381 case 2:
382 SET_RW_COMPAT(dw, chan->dir, ch2_pwr_en,
383 BIT(0));
384 break;
385 case 3:
386 SET_RW_COMPAT(dw, chan->dir, ch3_pwr_en,
387 BIT(0));
388 break;
389 case 4:
390 SET_RW_COMPAT(dw, chan->dir, ch4_pwr_en,
391 BIT(0));
392 break;
393 case 5:
394 SET_RW_COMPAT(dw, chan->dir, ch5_pwr_en,
395 BIT(0));
396 break;
397 case 6:
398 SET_RW_COMPAT(dw, chan->dir, ch6_pwr_en,
399 BIT(0));
400 break;
401 case 7:
402 SET_RW_COMPAT(dw, chan->dir, ch7_pwr_en,
403 BIT(0));
404 break;
405 }
406 }
407 /* Interrupt unmask - done, abort */
408 tmp = GET_RW_32(dw, chan->dir, int_mask);
409 tmp &= ~FIELD_PREP(EDMA_V0_DONE_INT_MASK, BIT(chan->id));
410 tmp &= ~FIELD_PREP(EDMA_V0_ABORT_INT_MASK, BIT(chan->id));
411 SET_RW_32(dw, chan->dir, int_mask, tmp);
412 /* Linked list error */
413 tmp = GET_RW_32(dw, chan->dir, linked_list_err_en);
414 tmp |= FIELD_PREP(EDMA_V0_LINKED_LIST_ERR_MASK, BIT(chan->id));
415 SET_RW_32(dw, chan->dir, linked_list_err_en, tmp);
416 /* Channel control */
417 SET_CH_32(dw, chan->dir, chan->id, ch_control1,
418 (DW_EDMA_V0_CCS | DW_EDMA_V0_LLE));
419 /* Linked list */
420 if ((chan->dw->chip->flags & DW_EDMA_CHIP_32BIT_DBI) ||
421 !IS_ENABLED(CONFIG_64BIT)) {
422 SET_CH_32(dw, chan->dir, chan->id, llp.lsb,
423 lower_32_bits(chunk->ll_region.paddr));
424 SET_CH_32(dw, chan->dir, chan->id, llp.msb,
425 upper_32_bits(chunk->ll_region.paddr));
426 } else {
> 427 SET_CH_64(dw, chan->dir, chan->id, llp.reg,
428 chunk->ll_region.paddr);
429 }
430 }
431 /* Doorbell */
432 SET_RW_32(dw, chan->dir, doorbell,
433 FIELD_PREP(EDMA_V0_DOORBELL_CH_MASK, chan->id));
434 }
435
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Hi Frank,
I love your patch! Yet something to improve:
[auto build test ERROR on vkoul-dmaengine/next]
[also build test ERROR on helgaas-pci/next linus/master v5.17-rc7 next-20220309]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Frank-Li/Enable-designware-PCI-EP-EDMA-locally/20220310-051510
base: https://git.kernel.org/pub/scm/linux/kernel/git/vkoul/dmaengine.git next
config: i386-randconfig-a015 (https://download.01.org/0day-ci/archive/20220310/202203100909.fpkA804r-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 276ca87382b8f16a65bddac700202924228982f6)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/f7947d784b0fe089bdaef2fee8e57f84e390d3f2
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Frank-Li/Enable-designware-PCI-EP-EDMA-locally/20220310-051510
git checkout f7947d784b0fe089bdaef2fee8e57f84e390d3f2
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/dma/dw-edma/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> drivers/dma/dw-edma/dw-edma-v0-core.c:427:4: error: implicit declaration of function 'SET_CH_64' [-Werror,-Wimplicit-function-declaration]
SET_CH_64(dw, chan->dir, chan->id, llp.reg,
^
>> drivers/dma/dw-edma/dw-edma-v0-core.c:427:39: error: use of undeclared identifier 'llp'
SET_CH_64(dw, chan->dir, chan->id, llp.reg,
^
2 errors generated.
vim +/SET_CH_64 +427 drivers/dma/dw-edma/dw-edma-v0-core.c
359
360 void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first)
361 {
362 struct dw_edma_chan *chan = chunk->chan;
363 struct dw_edma *dw = chan->dw;
364 u32 tmp;
365
366 dw_edma_v0_core_write_chunk(chunk);
367
368 if (first) {
369 /* Enable engine */
370 SET_RW_32(dw, chan->dir, engine_en, BIT(0));
371 if (dw->chip->mf == EDMA_MF_HDMA_COMPAT) {
372 switch (chan->id) {
373 case 0:
374 SET_RW_COMPAT(dw, chan->dir, ch0_pwr_en,
375 BIT(0));
376 break;
377 case 1:
378 SET_RW_COMPAT(dw, chan->dir, ch1_pwr_en,
379 BIT(0));
380 break;
381 case 2:
382 SET_RW_COMPAT(dw, chan->dir, ch2_pwr_en,
383 BIT(0));
384 break;
385 case 3:
386 SET_RW_COMPAT(dw, chan->dir, ch3_pwr_en,
387 BIT(0));
388 break;
389 case 4:
390 SET_RW_COMPAT(dw, chan->dir, ch4_pwr_en,
391 BIT(0));
392 break;
393 case 5:
394 SET_RW_COMPAT(dw, chan->dir, ch5_pwr_en,
395 BIT(0));
396 break;
397 case 6:
398 SET_RW_COMPAT(dw, chan->dir, ch6_pwr_en,
399 BIT(0));
400 break;
401 case 7:
402 SET_RW_COMPAT(dw, chan->dir, ch7_pwr_en,
403 BIT(0));
404 break;
405 }
406 }
407 /* Interrupt unmask - done, abort */
408 tmp = GET_RW_32(dw, chan->dir, int_mask);
409 tmp &= ~FIELD_PREP(EDMA_V0_DONE_INT_MASK, BIT(chan->id));
410 tmp &= ~FIELD_PREP(EDMA_V0_ABORT_INT_MASK, BIT(chan->id));
411 SET_RW_32(dw, chan->dir, int_mask, tmp);
412 /* Linked list error */
413 tmp = GET_RW_32(dw, chan->dir, linked_list_err_en);
414 tmp |= FIELD_PREP(EDMA_V0_LINKED_LIST_ERR_MASK, BIT(chan->id));
415 SET_RW_32(dw, chan->dir, linked_list_err_en, tmp);
416 /* Channel control */
417 SET_CH_32(dw, chan->dir, chan->id, ch_control1,
418 (DW_EDMA_V0_CCS | DW_EDMA_V0_LLE));
419 /* Linked list */
420 if ((chan->dw->chip->flags & DW_EDMA_CHIP_32BIT_DBI) ||
421 !IS_ENABLED(CONFIG_64BIT)) {
422 SET_CH_32(dw, chan->dir, chan->id, llp.lsb,
423 lower_32_bits(chunk->ll_region.paddr));
424 SET_CH_32(dw, chan->dir, chan->id, llp.msb,
425 upper_32_bits(chunk->ll_region.paddr));
426 } else {
> 427 SET_CH_64(dw, chan->dir, chan->id, llp.reg,
428 chunk->ll_region.paddr);
429 }
430 }
431 /* Doorbell */
432 SET_RW_32(dw, chan->dir, doorbell,
433 FIELD_PREP(EDMA_V0_DOORBELL_CH_MASK, chan->id));
434 }
435
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
On Wed, Mar 09, 2022 at 03:12:03PM -0600, Frank Li wrote: > Allow PCI EP probe DMA locally and prevent use of remote MSI > to remote PCI host. > > Add option to force 32bit DBI register access even on > 64-bit systems. i.MX8 hardware only allowed 32bit register > access. Could you please split this patch up into two? These flags are unrelated thus adding them is two unrelated changes. That can be implicitly inferred from your commit log and the patch title. -Sergey > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > --- > Change from v3 to v4 > - None > Change from v2 to v3 > - rework commit message > - Change to DW_EDMA_CHIP_32BIT_DBI > - using DW_EDMA_CHIP_LOCAL control msi > - Apply Bjorn's comments, > if (!j) { > control |= DW_EDMA_V0_LIE; > if (!(chan->chip->flags & DW_EDMA_CHIP_LOCAL)) > control |= DW_EDMA_V0_RIE; > } > > if ((chan->chip->flags & DW_EDMA_CHIP_REG32BIT) || > !IS_ENABLED(CONFIG_64BIT)) { > SET_CH_32(...); > SET_CH_32(...); > } else { > SET_CH_64(...); > } > > > Change from v1 to v2 > - none > drivers/dma/dw-edma/dw-edma-v0-core.c | 20 ++++++++++++-------- > include/linux/dma/edma.h | 9 +++++++++ > 2 files changed, 21 insertions(+), 8 deletions(-) > > diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c > index 35f2adac93e46..00a00d68d44e7 100644 > --- a/drivers/dma/dw-edma/dw-edma-v0-core.c > +++ b/drivers/dma/dw-edma/dw-edma-v0-core.c > @@ -301,6 +301,7 @@ u32 dw_edma_v0_core_status_abort_int(struct dw_edma *dw, enum dw_edma_dir dir) > static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk) > { > struct dw_edma_burst *child; > + struct dw_edma_chan *chan = chunk->chan; > struct dw_edma_v0_lli __iomem *lli; > struct dw_edma_v0_llp __iomem *llp; > u32 control = 0, i = 0; > @@ -314,9 +315,11 @@ static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk) > j = chunk->bursts_alloc; > list_for_each_entry(child, &chunk->burst->list, list) { > j--; > - if (!j) > - control |= (DW_EDMA_V0_LIE | DW_EDMA_V0_RIE); > - > + if (!j) { > + control |= DW_EDMA_V0_LIE; > + if (!(chan->dw->chip->flags & DW_EDMA_CHIP_LOCAL)) > + control |= DW_EDMA_V0_RIE; > + } > /* Channel control */ > SET_LL_32(&lli[i].control, control); > /* Transfer size */ > @@ -414,15 +417,16 @@ void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first) > SET_CH_32(dw, chan->dir, chan->id, ch_control1, > (DW_EDMA_V0_CCS | DW_EDMA_V0_LLE)); > /* Linked list */ > - #ifdef CONFIG_64BIT > - SET_CH_64(dw, chan->dir, chan->id, llp.reg, > - chunk->ll_region.paddr); > - #else /* CONFIG_64BIT */ > + if ((chan->dw->chip->flags & DW_EDMA_CHIP_32BIT_DBI) || > + !IS_ENABLED(CONFIG_64BIT)) { > SET_CH_32(dw, chan->dir, chan->id, llp.lsb, > lower_32_bits(chunk->ll_region.paddr)); > SET_CH_32(dw, chan->dir, chan->id, llp.msb, > upper_32_bits(chunk->ll_region.paddr)); > - #endif /* CONFIG_64BIT */ > + } else { > + SET_CH_64(dw, chan->dir, chan->id, llp.reg, > + chunk->ll_region.paddr); > + } > } > /* Doorbell */ > SET_RW_32(dw, chan->dir, doorbell, > diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h > index c2039246fc08c..eea11b1d9e688 100644 > --- a/include/linux/dma/edma.h > +++ b/include/linux/dma/edma.h > @@ -33,6 +33,12 @@ enum dw_edma_map_format { > EDMA_MF_HDMA_COMPAT = 0x5 > }; > > +/* Probe EDMA engine locally and prevent generate MSI to host side*/ > +#define DW_EDMA_CHIP_LOCAL BIT(0) > + > +/* Only support 32bit DBI register access */ > +#define DW_EDMA_CHIP_32BIT_DBI BIT(1) > + > /** > * struct dw_edma_chip - representation of DesignWare eDMA controller hardware > * @dev: struct device of the eDMA controller > @@ -40,6 +46,8 @@ enum dw_edma_map_format { > * @nr_irqs: total dma irq number > * @ops DMA channel to IRQ number mapping > * @reg_base DMA register base address > + * @flags - DW_EDMA_CHIP_LOCAL > + * - DW_EDMA_CHIP_32BIT_DBI > * @ll_wr_cnt DMA write link list number > * @ll_rd_cnt DMA read link list number > * @rg_region DMA register region > @@ -53,6 +61,7 @@ struct dw_edma_chip { > int id; > int nr_irqs; > const struct dw_edma_core_ops *ops; > + u32 flags; > > void __iomem *reg_base; > > -- > 2.24.0.rc1 >
On Thu, Mar 10, 2022 at 11:47 AM Serge Semin <fancer.lancer@gmail.com> wrote: > > On Wed, Mar 09, 2022 at 03:12:03PM -0600, Frank Li wrote: > > Allow PCI EP probe DMA locally and prevent use of remote MSI > > to remote PCI host. > > > > Add option to force 32bit DBI register access even on > > 64-bit systems. i.MX8 hardware only allowed 32bit register > > access. > > Could you please split this patch up into two? These flags are > unrelated thus adding them is two unrelated changes. That can be > implicitly inferred from your commit log and the patch title. I don't think it needs to be separated. It also show why need 32bit mask to control features and reserved futured extension capability . The two flags were descriptions for EDMA chip features. > > -Sergey > > > > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > > --- > > Change from v3 to v4 > > - None > > Change from v2 to v3 > > - rework commit message > > - Change to DW_EDMA_CHIP_32BIT_DBI > > - using DW_EDMA_CHIP_LOCAL control msi > > - Apply Bjorn's comments, > > if (!j) { > > control |= DW_EDMA_V0_LIE; > > if (!(chan->chip->flags & DW_EDMA_CHIP_LOCAL)) > > control |= DW_EDMA_V0_RIE; > > } > > > > if ((chan->chip->flags & DW_EDMA_CHIP_REG32BIT) || > > !IS_ENABLED(CONFIG_64BIT)) { > > SET_CH_32(...); > > SET_CH_32(...); > > } else { > > SET_CH_64(...); > > } > > > > > > Change from v1 to v2 > > - none > > drivers/dma/dw-edma/dw-edma-v0-core.c | 20 ++++++++++++-------- > > include/linux/dma/edma.h | 9 +++++++++ > > 2 files changed, 21 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c > > index 35f2adac93e46..00a00d68d44e7 100644 > > --- a/drivers/dma/dw-edma/dw-edma-v0-core.c > > +++ b/drivers/dma/dw-edma/dw-edma-v0-core.c > > @@ -301,6 +301,7 @@ u32 dw_edma_v0_core_status_abort_int(struct dw_edma *dw, enum dw_edma_dir dir) > > static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk) > > { > > struct dw_edma_burst *child; > > + struct dw_edma_chan *chan = chunk->chan; > > struct dw_edma_v0_lli __iomem *lli; > > struct dw_edma_v0_llp __iomem *llp; > > u32 control = 0, i = 0; > > @@ -314,9 +315,11 @@ static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk) > > j = chunk->bursts_alloc; > > list_for_each_entry(child, &chunk->burst->list, list) { > > j--; > > - if (!j) > > - control |= (DW_EDMA_V0_LIE | DW_EDMA_V0_RIE); > > - > > + if (!j) { > > + control |= DW_EDMA_V0_LIE; > > + if (!(chan->dw->chip->flags & DW_EDMA_CHIP_LOCAL)) > > + control |= DW_EDMA_V0_RIE; > > + } > > /* Channel control */ > > SET_LL_32(&lli[i].control, control); > > /* Transfer size */ > > @@ -414,15 +417,16 @@ void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first) > > SET_CH_32(dw, chan->dir, chan->id, ch_control1, > > (DW_EDMA_V0_CCS | DW_EDMA_V0_LLE)); > > /* Linked list */ > > - #ifdef CONFIG_64BIT > > - SET_CH_64(dw, chan->dir, chan->id, llp.reg, > > - chunk->ll_region.paddr); > > - #else /* CONFIG_64BIT */ > > + if ((chan->dw->chip->flags & DW_EDMA_CHIP_32BIT_DBI) || > > + !IS_ENABLED(CONFIG_64BIT)) { > > SET_CH_32(dw, chan->dir, chan->id, llp.lsb, > > lower_32_bits(chunk->ll_region.paddr)); > > SET_CH_32(dw, chan->dir, chan->id, llp.msb, > > upper_32_bits(chunk->ll_region.paddr)); > > - #endif /* CONFIG_64BIT */ > > + } else { > > + SET_CH_64(dw, chan->dir, chan->id, llp.reg, > > + chunk->ll_region.paddr); > > + } > > } > > /* Doorbell */ > > SET_RW_32(dw, chan->dir, doorbell, > > diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h > > index c2039246fc08c..eea11b1d9e688 100644 > > --- a/include/linux/dma/edma.h > > +++ b/include/linux/dma/edma.h > > @@ -33,6 +33,12 @@ enum dw_edma_map_format { > > EDMA_MF_HDMA_COMPAT = 0x5 > > }; > > > > +/* Probe EDMA engine locally and prevent generate MSI to host side*/ > > +#define DW_EDMA_CHIP_LOCAL BIT(0) > > + > > +/* Only support 32bit DBI register access */ > > +#define DW_EDMA_CHIP_32BIT_DBI BIT(1) > > + > > /** > > * struct dw_edma_chip - representation of DesignWare eDMA controller hardware > > * @dev: struct device of the eDMA controller > > @@ -40,6 +46,8 @@ enum dw_edma_map_format { > > * @nr_irqs: total dma irq number > > * @ops DMA channel to IRQ number mapping > > * @reg_base DMA register base address > > + * @flags - DW_EDMA_CHIP_LOCAL > > + * - DW_EDMA_CHIP_32BIT_DBI > > * @ll_wr_cnt DMA write link list number > > * @ll_rd_cnt DMA read link list number > > * @rg_region DMA register region > > @@ -53,6 +61,7 @@ struct dw_edma_chip { > > int id; > > int nr_irqs; > > const struct dw_edma_core_ops *ops; > > + u32 flags; > > > > void __iomem *reg_base; > > > > -- > > 2.24.0.rc1 > >
On Thu, Mar 10, 2022 at 11:54:30AM -0600, Zhi Li wrote: > On Thu, Mar 10, 2022 at 11:47 AM Serge Semin <fancer.lancer@gmail.com> wrote: > > > > On Wed, Mar 09, 2022 at 03:12:03PM -0600, Frank Li wrote: > > > Allow PCI EP probe DMA locally and prevent use of remote MSI > > > to remote PCI host. > > > > > > Add option to force 32bit DBI register access even on > > > 64-bit systems. i.MX8 hardware only allowed 32bit register > > > access. > > > > Could you please split this patch up into two? These flags are > > unrelated thus adding them is two unrelated changes. That can be > > implicitly inferred from your commit log and the patch title. > > I don't think it needs to be separated. It also show why need 32bit mask to > control features and reserved futured extension capability . > > The two flags were descriptions for EDMA chip features. Both of these flags define separate platform features. Each of which can be distinctively specific for a particular SoC, not only for the i.MX8. Even though your log messages does mention i.MX8 hardware the corresponding flag is defined as generic in the patch. As I said both flags are unrelated to each other and can be independently specific to one or another platform. So one SoC can be restricted to use 32bits DBI IOs and do have the Synopsys PCIe End-point IP prototype kit detected on the PCIe bus. Another SoC can have the eDMA core embedded into the DW PCIe EP/Host controller, but with no 32-bits DBI IOs requirement. Secondly since both of these flags are unrelated then having a monolithic patch as you suggest will harden the bisection procedure in case of a bug is caused by one of these modifications. Bisecting will lead straight to the bogus change if the patch is split up into two. Thirdly as I said before your commit log states two distinctive changes, which means the log can be split up into two together with the logical changes. In addition referring to a particular change in further commits will be more informative. Finally please see [1] regarding the patches splitting up, logical changes, bisection, etc. [1] Documentation/process/submitting-patches.rst: "Separate your changes" -Sergey > > > > > -Sergey > > > > > > > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > > > --- > > > Change from v3 to v4 > > > - None > > > Change from v2 to v3 > > > - rework commit message > > > - Change to DW_EDMA_CHIP_32BIT_DBI > > > - using DW_EDMA_CHIP_LOCAL control msi > > > - Apply Bjorn's comments, > > > if (!j) { > > > control |= DW_EDMA_V0_LIE; > > > if (!(chan->chip->flags & DW_EDMA_CHIP_LOCAL)) > > > control |= DW_EDMA_V0_RIE; > > > } > > > > > > if ((chan->chip->flags & DW_EDMA_CHIP_REG32BIT) || > > > !IS_ENABLED(CONFIG_64BIT)) { > > > SET_CH_32(...); > > > SET_CH_32(...); > > > } else { > > > SET_CH_64(...); > > > } > > > > > > > > > Change from v1 to v2 > > > - none > > > drivers/dma/dw-edma/dw-edma-v0-core.c | 20 ++++++++++++-------- > > > include/linux/dma/edma.h | 9 +++++++++ > > > 2 files changed, 21 insertions(+), 8 deletions(-) > > > > > > diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c > > > index 35f2adac93e46..00a00d68d44e7 100644 > > > --- a/drivers/dma/dw-edma/dw-edma-v0-core.c > > > +++ b/drivers/dma/dw-edma/dw-edma-v0-core.c > > > @@ -301,6 +301,7 @@ u32 dw_edma_v0_core_status_abort_int(struct dw_edma *dw, enum dw_edma_dir dir) > > > static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk) > > > { > > > struct dw_edma_burst *child; > > > + struct dw_edma_chan *chan = chunk->chan; > > > struct dw_edma_v0_lli __iomem *lli; > > > struct dw_edma_v0_llp __iomem *llp; > > > u32 control = 0, i = 0; > > > @@ -314,9 +315,11 @@ static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk) > > > j = chunk->bursts_alloc; > > > list_for_each_entry(child, &chunk->burst->list, list) { > > > j--; > > > - if (!j) > > > - control |= (DW_EDMA_V0_LIE | DW_EDMA_V0_RIE); > > > - > > > + if (!j) { > > > + control |= DW_EDMA_V0_LIE; > > > + if (!(chan->dw->chip->flags & DW_EDMA_CHIP_LOCAL)) > > > + control |= DW_EDMA_V0_RIE; > > > + } > > > /* Channel control */ > > > SET_LL_32(&lli[i].control, control); > > > /* Transfer size */ > > > @@ -414,15 +417,16 @@ void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first) > > > SET_CH_32(dw, chan->dir, chan->id, ch_control1, > > > (DW_EDMA_V0_CCS | DW_EDMA_V0_LLE)); > > > /* Linked list */ > > > - #ifdef CONFIG_64BIT > > > - SET_CH_64(dw, chan->dir, chan->id, llp.reg, > > > - chunk->ll_region.paddr); > > > - #else /* CONFIG_64BIT */ > > > + if ((chan->dw->chip->flags & DW_EDMA_CHIP_32BIT_DBI) || > > > + !IS_ENABLED(CONFIG_64BIT)) { > > > SET_CH_32(dw, chan->dir, chan->id, llp.lsb, > > > lower_32_bits(chunk->ll_region.paddr)); > > > SET_CH_32(dw, chan->dir, chan->id, llp.msb, > > > upper_32_bits(chunk->ll_region.paddr)); > > > - #endif /* CONFIG_64BIT */ > > > + } else { > > > + SET_CH_64(dw, chan->dir, chan->id, llp.reg, > > > + chunk->ll_region.paddr); > > > + } > > > } > > > /* Doorbell */ > > > SET_RW_32(dw, chan->dir, doorbell, > > > diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h > > > index c2039246fc08c..eea11b1d9e688 100644 > > > --- a/include/linux/dma/edma.h > > > +++ b/include/linux/dma/edma.h > > > @@ -33,6 +33,12 @@ enum dw_edma_map_format { > > > EDMA_MF_HDMA_COMPAT = 0x5 > > > }; > > > > > > +/* Probe EDMA engine locally and prevent generate MSI to host side*/ > > > +#define DW_EDMA_CHIP_LOCAL BIT(0) > > > + > > > +/* Only support 32bit DBI register access */ > > > +#define DW_EDMA_CHIP_32BIT_DBI BIT(1) > > > + > > > /** > > > * struct dw_edma_chip - representation of DesignWare eDMA controller hardware > > > * @dev: struct device of the eDMA controller > > > @@ -40,6 +46,8 @@ enum dw_edma_map_format { > > > * @nr_irqs: total dma irq number > > > * @ops DMA channel to IRQ number mapping > > > * @reg_base DMA register base address > > > + * @flags - DW_EDMA_CHIP_LOCAL > > > + * - DW_EDMA_CHIP_32BIT_DBI > > > * @ll_wr_cnt DMA write link list number > > > * @ll_rd_cnt DMA read link list number > > > * @rg_region DMA register region > > > @@ -53,6 +61,7 @@ struct dw_edma_chip { > > > int id; > > > int nr_irqs; > > > const struct dw_edma_core_ops *ops; > > > + u32 flags; > > > > > > void __iomem *reg_base; > > > > > > -- > > > 2.24.0.rc1 > > >
diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c index 35f2adac93e46..00a00d68d44e7 100644 --- a/drivers/dma/dw-edma/dw-edma-v0-core.c +++ b/drivers/dma/dw-edma/dw-edma-v0-core.c @@ -301,6 +301,7 @@ u32 dw_edma_v0_core_status_abort_int(struct dw_edma *dw, enum dw_edma_dir dir) static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk) { struct dw_edma_burst *child; + struct dw_edma_chan *chan = chunk->chan; struct dw_edma_v0_lli __iomem *lli; struct dw_edma_v0_llp __iomem *llp; u32 control = 0, i = 0; @@ -314,9 +315,11 @@ static void dw_edma_v0_core_write_chunk(struct dw_edma_chunk *chunk) j = chunk->bursts_alloc; list_for_each_entry(child, &chunk->burst->list, list) { j--; - if (!j) - control |= (DW_EDMA_V0_LIE | DW_EDMA_V0_RIE); - + if (!j) { + control |= DW_EDMA_V0_LIE; + if (!(chan->dw->chip->flags & DW_EDMA_CHIP_LOCAL)) + control |= DW_EDMA_V0_RIE; + } /* Channel control */ SET_LL_32(&lli[i].control, control); /* Transfer size */ @@ -414,15 +417,16 @@ void dw_edma_v0_core_start(struct dw_edma_chunk *chunk, bool first) SET_CH_32(dw, chan->dir, chan->id, ch_control1, (DW_EDMA_V0_CCS | DW_EDMA_V0_LLE)); /* Linked list */ - #ifdef CONFIG_64BIT - SET_CH_64(dw, chan->dir, chan->id, llp.reg, - chunk->ll_region.paddr); - #else /* CONFIG_64BIT */ + if ((chan->dw->chip->flags & DW_EDMA_CHIP_32BIT_DBI) || + !IS_ENABLED(CONFIG_64BIT)) { SET_CH_32(dw, chan->dir, chan->id, llp.lsb, lower_32_bits(chunk->ll_region.paddr)); SET_CH_32(dw, chan->dir, chan->id, llp.msb, upper_32_bits(chunk->ll_region.paddr)); - #endif /* CONFIG_64BIT */ + } else { + SET_CH_64(dw, chan->dir, chan->id, llp.reg, + chunk->ll_region.paddr); + } } /* Doorbell */ SET_RW_32(dw, chan->dir, doorbell, diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h index c2039246fc08c..eea11b1d9e688 100644 --- a/include/linux/dma/edma.h +++ b/include/linux/dma/edma.h @@ -33,6 +33,12 @@ enum dw_edma_map_format { EDMA_MF_HDMA_COMPAT = 0x5 }; +/* Probe EDMA engine locally and prevent generate MSI to host side*/ +#define DW_EDMA_CHIP_LOCAL BIT(0) + +/* Only support 32bit DBI register access */ +#define DW_EDMA_CHIP_32BIT_DBI BIT(1) + /** * struct dw_edma_chip - representation of DesignWare eDMA controller hardware * @dev: struct device of the eDMA controller @@ -40,6 +46,8 @@ enum dw_edma_map_format { * @nr_irqs: total dma irq number * @ops DMA channel to IRQ number mapping * @reg_base DMA register base address + * @flags - DW_EDMA_CHIP_LOCAL + * - DW_EDMA_CHIP_32BIT_DBI * @ll_wr_cnt DMA write link list number * @ll_rd_cnt DMA read link list number * @rg_region DMA register region @@ -53,6 +61,7 @@ struct dw_edma_chip { int id; int nr_irqs; const struct dw_edma_core_ops *ops; + u32 flags; void __iomem *reg_base;
Allow PCI EP probe DMA locally and prevent use of remote MSI to remote PCI host. Add option to force 32bit DBI register access even on 64-bit systems. i.MX8 hardware only allowed 32bit register access. Signed-off-by: Frank Li <Frank.Li@nxp.com> --- Change from v3 to v4 - None Change from v2 to v3 - rework commit message - Change to DW_EDMA_CHIP_32BIT_DBI - using DW_EDMA_CHIP_LOCAL control msi - Apply Bjorn's comments, if (!j) { control |= DW_EDMA_V0_LIE; if (!(chan->chip->flags & DW_EDMA_CHIP_LOCAL)) control |= DW_EDMA_V0_RIE; } if ((chan->chip->flags & DW_EDMA_CHIP_REG32BIT) || !IS_ENABLED(CONFIG_64BIT)) { SET_CH_32(...); SET_CH_32(...); } else { SET_CH_64(...); } Change from v1 to v2 - none drivers/dma/dw-edma/dw-edma-v0-core.c | 20 ++++++++++++-------- include/linux/dma/edma.h | 9 +++++++++ 2 files changed, 21 insertions(+), 8 deletions(-)