15/10/2021 11:59, fengchengwen:
> 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:
No, if previous answer was enough, I would not add a new comment.
Please read again:
"
no need to dma_release from rte_dma_pmd_release,
because it is already called from rte_dma_close
"
> 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).
You are justifying the existence of the functions, OK,
but I am just discussing one call of the function which is useless.
Anyway, now I am in the process of merging v26,
so I will send a fix.
