Re: [PATCH v6 2/4] dmaengine: dw-axi-dmac: Add support for StarFive JH7110 DMA

Walker Chen
2023-04-03T12:07:38Z

On 2023/3/31 20:04, Vinod Koul wrote:
> On 22-03-23, 17:48, Walker Chen wrote:
>> Add DMA reset operation in device probe and use different configuration
>> on CH_CFG registers according to match data. Update all uses of
>> of_device_is_compatible with of_device_get_match_data.
>>
>> Signed-off-by: Walker Chen <walker.chen@starfivetech.com>
>> ---
>> .../dma/dw-axi-dmac/dw-axi-dmac-platform.c | 38 ++++++++++++++++---
>> drivers/dma/dw-axi-dmac/dw-axi-dmac.h | 1 +
>> 2 files changed, 34 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
>> index 4169e1d7d5ca..6cfcb541d8c3 100644
>> --- a/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
>> +++ b/drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c
>> @@ -21,10 +21,12 @@
>> #include <linux/kernel.h>
>> #include <linux/module.h>
>> #include <linux/of.h>
>> +#include <linux/of_device.h>
>> #include <linux/of_dma.h>
>> #include <linux/platform_device.h>
>> #include <linux/pm_runtime.h>
>> #include <linux/property.h>
>> +#include <linux/reset.h>
>> #include <linux/slab.h>
>> #include <linux/types.h>
>>
>> @@ -46,6 +48,10 @@
>> DMA_SLAVE_BUSWIDTH_32_BYTES | \
>> DMA_SLAVE_BUSWIDTH_64_BYTES)
>>
>> +#define AXI_DMA_FLAG_HAS_APB_REGS BIT(0)
>> +#define AXI_DMA_FLAG_HAS_RESETS BIT(1)
>> +#define AXI_DMA_FLAG_USE_CFG2 BIT(2)
>> +
>> static inline void
>> axi_dma_iowrite32(struct axi_dma_chip *chip, u32 reg, u32 val)
>> {
>> @@ -86,7 +92,8 @@ static inline void axi_chan_config_write(struct axi_dma_chan *chan,
>>
>> cfg_lo = (config->dst_multblk_type << CH_CFG_L_DST_MULTBLK_TYPE_POS |
>> config->src_multblk_type << CH_CFG_L_SRC_MULTBLK_TYPE_POS);
>> - if (chan->chip->dw->hdata->reg_map_8_channels) {
>> + if (chan->chip->dw->hdata->reg_map_8_channels &&
>> + !chan->chip->dw->hdata->use_cfg2) {
>
> I think this will break existing users..
>
> This is set for reg_map_8_channels && use_cfg2, latter being set only
> for new controller, so what about existing users of these bits?

Firstly thank you for your comments!
There is a statement 'use_cfg2 = !!(flags & AXI_DMA_FLAG_USE_CFG2);' to be added in dw_probe function.
Assuming older/existing platform run this code block, e.g. when compatible is "snps,axi-dma-1.01a",
the value of variable 'use_cfg2' is still false, the original logic will not be broken. So other existing
users are not affected by this.
Looking forward to your more comments. Thanks!

Best regards,
Walker