Re: [dpdk-dev] [PATCH v25 1/6] dmadev: introduce DMA device library

[fengchengwen <fengchengwen@huawei.com>]

On 2021/10/15 16:29, Thomas Monjalon wrote:
> 13/10/2021 09:41, Thomas Monjalon:
>> 13/10/2021 02:21, fengchengwen:
>>> On 2021/10/13 3:09, Thomas Monjalon wrote:
>>>> 11/10/2021 09:33, Chengwen Feng:
>>>>> +static void
>>>>> +dma_release(struct rte_dma_dev *dev)
>>>>> +{
>>>>> +	rte_free(dev->dev_private);
>>>>> +	memset(dev, 0, sizeof(struct rte_dma_dev));
>>>>> +}
>> [...]
>>>>> +int
>>>>> +rte_dma_pmd_release(const char *name)
>>>>> +{
>>>>> +	struct rte_dma_dev *dev;
>>>>> +
>>>>> +	if (dma_check_name(name) != 0)
>>>>> +		return -EINVAL;
>>>>> +
>>>>> +	dev = dma_find_by_name(name);
>>>>> +	if (dev == NULL)
>>>>> +		return -EINVAL;
>>>>> +
>>>>> +	dma_release(dev);
>>>>> +	return 0;
>>>>> +}
>>>>
>>>> Trying to understand the logic of creation/destroy.
>>>> skeldma_probe
>>>> \-> skeldma_create
>>>>     \-> rte_dma_pmd_allocate
>>>>         \-> dma_allocate
>>>>             \-> dma_data_prepare
>>>>                 \-> dma_dev_data_prepare
>>>> skeldma_remove
>>>> \-> skeldma_destroy
>>>>     \-> rte_dma_pmd_release
>>>>         \-> dma_release
>>>
>>> This patch only provide device allocate function, the 2st patch provide extra logic:
>>>
>>> 	diff --git a/lib/dmadev/rte_dmadev.c b/lib/dmadev/rte_dmadev.c
>>> 	index 42a4693bd9..a6a5680d2b 100644
>>> 	--- a/lib/dmadev/rte_dmadev.c
>>> 	+++ b/lib/dmadev/rte_dmadev.c
>>> 	@@ -201,6 +201,9 @@ rte_dma_pmd_release(const char *name)
>>> 	        if (dev == NULL)
>>>         	        return -EINVAL;
>>>
>>> 	+       if (dev->state == RTE_DMA_DEV_READY)
>>> 	+               return rte_dma_close(dev->dev_id);
>>> 	+
>>>         	dma_release(dev);
>>> 	        return 0;
>>> 	 }
>>>
>>> So the skeldma remove will be:
>>>
>>>  skeldma_remove
>>>  \-> skeldma_destroy
>>>      \-> rte_dma_pmd_release
>>>          \-> rte_dma_close
>>>              \-> dma_release
>>
>> OK, in this case, no need to dma_release from rte_dma_pmd_release,
>> because it is already called from rte_dma_close.
> 
> Ping for reply please.

Sorry, I think the previous reply was enough, Let me explain:

The PMD use following logic create dmadev:
  skeldma_probe
    \-> skeldma_create
      \-> rte_dma_pmd_allocate
        \-> dma_allocate
      \-> mark dmadev state to READY.

The PMD remove will be:
 skeldma_remove
  \-> skeldma_destroy
      \-> rte_dma_pmd_release
          \-> rte_dma_close
              \-> dma_release

The application close dmadev:
  rte_dma_close
   \-> dma_release

in the above case, the PMD remove and application close both call rte_dma_close,
I think that's what you expect.


skeldma is simple, please let me give you a complicated example:
  hisi_dma_probe
    \-> hisi_dma_create
      \-> rte_dma_pmd_allocate
        \-> dma_allocate
      \-> hisi_hw_init
        \-> if init fail, call rte_dma_pmd_release.
            \-> dma_release
        \-> if init OK, mark dmadev state to READY.

as you can see, if hisi_hw_init fail, it call rte_dma_pmd_release to release dmadev,
it will direct call dma_release.
if hisi_hw_init success, it mean the hardware also OK, then mark dmadev state to
READY. if PMD remove the dmadev it will call rte_dma_close because the dmadev's state
is READY, and the application could also call rte_dma_close to destroy dmadev.


The rte_dma_pmd_release take two function:
1. if the dmadev's hardware part init fail, the PMD could use this function release the
dmadev.
2. if the dmadev's hardware part init success, the PMD could use this function destroy
the dmadev.


If we don't have the rte_dma_pmd_release function, we should export dma_release function
which invoked when the hardware init fail.

And if we keep rte_dma_pmd_release, it correspond the rte_dma_pmd_allocate, the PMD just
invoke rte_dma_pmd_release to handle above both cases (hardware part init fail when probe
and remove phase).

Thanks.

> 
> 
> 
> 
> .
>