Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <42774a4f-ae1e-7d25-6b01-67f5af8400a4@linux.intel.com>
Date:   Mon, 22 May 2023 11:39:49 -0500
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Firefox/102.0 Thunderbird/102.11.0
Subject: Re: [PATCH V2 4/9] ASoC: amd: ps: add SoundWire dma driver dma ops
Content-Language: en-US
To:     Vijendar Mukunda <Vijendar.Mukunda@amd.com>, broonie@kernel.org
Cc:     alsa-devel@alsa-project.org, Basavaraj.Hiregoudar@amd.com,
        Sunil-kumar.Dommati@amd.com, Mastan.Katragadda@amd.com,
        Arungopal.kondaveeti@amd.com, mario.limonciello@amd.com,
        Liam Girdwood <lgirdwood@gmail.com>,
        Jaroslav Kysela <perex@perex.cz>,
        Takashi Iwai <tiwai@suse.com>,
        Syed Saba Kareem <Syed.SabaKareem@amd.com>,
        open list <linux-kernel@vger.kernel.org>
References: <20230522133122.166841-1-Vijendar.Mukunda@amd.com>
 <20230522133122.166841-5-Vijendar.Mukunda@amd.com>
From:   Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
In-Reply-To: <20230522133122.166841-5-Vijendar.Mukunda@amd.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk


> +union acp_sdw_dma_count {
> +	struct {
> +	u32 low;
> +	u32 high;
> +	} bcount;

indentation seems off?

> +	u64 bytescount;
> +};
> +
> +struct sdw_dma_ring_buf_reg {
> +	u32 reg_dma_size;
> +	u32 reg_fifo_addr;
> +	u32 reg_fifo_size;
> +	u32 reg_ring_buf_size;
> +	u32 reg_ring_buf_addr;
> +	u32 water_mark_size_reg;
> +	u32 pos_low_reg;
> +	u32 pos_high_reg;
>  };
>\
> +static void acp63_config_dma(struct acp_sdw_dma_stream *stream, void __iomem *acp_base,
> +			     u32 stream_id)
> +{
> +	u16 page_idx;
> +	u32 low, high, val;
> +	u32 sdw_dma_pte_offset;
> +	dma_addr_t addr;
> +
> +	addr = stream->dma_addr;
> +	sdw_dma_pte_offset = SDW_PTE_OFFSET(stream->instance);
> +	val = sdw_dma_pte_offset + (stream_id * 256);

what is this 256 magic value? use a defined or << 8 ?
> +
> +	/* Group Enable */
> +	writel(ACP_SDW_SRAM_PTE_OFFSET | BIT(31), acp_base + ACPAXI2AXI_ATU_BASE_ADDR_GRP_2);
> +	writel(PAGE_SIZE_4K_ENABLE, acp_base + ACPAXI2AXI_ATU_PAGE_SIZE_GRP_2);
> +	for (page_idx = 0; page_idx < stream->num_pages; page_idx++) {
> +		/* Load the low address of page int ACP SRAM through SRBM */
> +		low = lower_32_bits(addr);
> +		high = upper_32_bits(addr);
> +
> +		writel(low, acp_base + ACP_SCRATCH_REG_0 + val);
> +		high |= BIT(31);
> +		writel(high, acp_base + ACP_SCRATCH_REG_0 + val + 4);
> +		val += 8;
> +		addr += PAGE_SIZE;
> +	}
> +	writel(0x1, acp_base + ACPAXI2AXI_ATU_CTRL);
> +}