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;
Received-SPF: Pass (protection.outlook.com: domain of amd.com designates
 165.204.84.17 as permitted sender) receiver=protection.outlook.com;
 client-ip=165.204.84.17; helo=SATLEXMB03.amd.com; pr=C
Message-ID: <aa375470-6297-0a8f-c520-7c3481520990@amd.com>
Date:   Tue, 30 May 2023 11:29:45 -0700
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.11.0
Subject: Re: [PATCH V1 QDMA 1/1] dmaengine: amd: qdma: Add AMD QDMA driver
Content-Language: en-US
To:     Christophe JAILLET <christophe.jaillet@wanadoo.fr>,
        <vkoul@kernel.org>, <dmaengine@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>
CC:     Nishad Saraf <nishads@amd.com>, <max.zhen@amd.com>,
        <sonal.santan@amd.com>, <nishad.saraf@amd.com>
References: <1685119795-11729-1-git-send-email-lizhi.hou@amd.com>
 <1685119795-11729-2-git-send-email-lizhi.hou@amd.com>
 <41f58a00-1ab5-3eae-0e32-0f6e05282cf1@wanadoo.fr>
From:   Lizhi Hou <lizhi.hou@amd.com>
In-Reply-To: <41f58a00-1ab5-3eae-0e32-0f6e05282cf1@wanadoo.fr>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 May 2023 18:29:47.2450
 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: ed43ebfe-f927-4390-7977-08db613bd8f3
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=3dd8961f-e488-4e60-8e11-a82d994e183d;Ip=[165.204.84.17];Helo=[SATLEXMB03.amd.com]
X-MS-Exchange-CrossTenant-AuthSource: CO1PEPF000042AB.namprd03.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Anonymous
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA1PR12MB7517
X-Spam-Status: No, score=-1.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,NICE_REPLY_A,
        RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_NONE,
        T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=no autolearn_force=no
        version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 5/27/23 06:33, Christophe JAILLET wrote:
> Le 26/05/2023 à 18:49, Lizhi Hou a écrit :
>> From: Nishad Saraf <nishads@amd.com>
>>
>> Adds driver to enable PCIe board which uses AMD QDMA (the Queue-based
>> Direct Memory Access) subsystem. For example, Xilinx Alveo V70 AI
>> Accelerator devices.
>>      https://www.xilinx.com/applications/data-center/v70.html
>>
>> The primary mechanism to transfer data using the QDMA is for the QDMA
>> engine to operate on instructions (descriptors) provided by the host
>> operating system. Using the descriptors, the QDMA can move data in both
>> the Host to Card (H2C) direction, or the Card to Host (C2H) direction.
>> The QDMA provides a per-queue basis option whether DMA traffic goes
>> to an AXI4 memory map (MM) interface or to an AXI4-Stream interface.
>>
>> The hardware detail is provided by
>>      https://docs.xilinx.com/r/en-US/pg302-qdma
>>
>> Implements dmaengine APIs to support MM DMA transfers.
>> - probe the available DMA channels
>> - use dma_slave_map for channel lookup
>> - use virtual channel to manage dmaengine tx descriptors
>> - implement device_prep_slave_sg callback to handle host scatter gather
>>    list
>> - implement descriptor metadata operations to set device address for DMA
>>    transfer
>>
>> Signed-off-by: Nishad Saraf <nishads@amd.com>
>> Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
>> ---
>
> [...]
>
>> +/**
>> + * qdma_alloc_queue_resources() - Allocate queue resources
>> + * @chan: DMA channel
>> + */
>> +static int qdma_alloc_queue_resources(struct dma_chan *chan)
>> +{
>> +    struct qdma_queue *queue = to_qdma_queue(chan);
>> +    struct qdma_device *qdev = queue->qdev;
>> +    struct qdma_ctxt_sw_desc desc;
>> +    size_t size;
>> +    int ret;
>> +
>> +    ret = qdma_clear_queue_context(queue);
>> +    if (ret)
>> +        return ret;
>> +
>> +    size = queue->ring_size * QDMA_MM_DESC_SIZE;
>> +    queue->desc_base = dma_alloc_coherent(qdev->dma_dev.dev, size,
>> +                          &queue->dma_desc_base,
>> +                          GFP_KERNEL | __GFP_ZERO);
>
> Nit: Useless (but harmless).
> AFAIK, dma_alloc_coherent() always returned some zeroed memory.
> (should you remove the __GFP_ZERO, there is another usage below)
Sure. I will remove __GFP_ZERO.
>
>> +    if (!queue->desc_base) {
>> +        qdma_err(qdev, "Failed to allocate descriptor ring");
>> +        return -ENOMEM;
>> +    }
>> +
>
> [...]
>
>> +/**
>> + * struct qdma_platdata - Platform specific data for QDMA engine
>> + * @max_mm_channels: Maximum number of MM DMA channels in each 
>> direction
>> + * @device_map: DMA slave map
>> + * @irq_index: The index of first IRQ
>> + */
>> +struct qdma_platdata {
>> +    u32            max_mm_channels;
>> +    struct dma_slave_map    *device_map;
>> +    u32            irq_index;
>> +};
>
> Noob question: this struct is only retrieved from dev_get_platdata(), 
> but there is no dev_set_platdata().
> How the link is done? How this structure is filled?

The platdata is generated with platform device. For example, a PCI 
driver may do

     struct qdma_platdata data = { .... }

     platform_device_register_resndata(.., &data, ...)


>
>
> Should it mater, keeping the 2 u32 one after the other, would avoid a 
> hole.

Sure. I will fix this.


Thanks,

Lizhi

>
> CJ
>
>> +
>> +#endif /* _PLATDATA_AMD_QDMA_H */
>