At 2022-07-20 14:39:56, "Péter Ujfalusi" <peter.ujfalusi@xxxxxxxxx> wrote:
>Hi,
>
>On 16/07/2022 11:46, Liang He wrote:
>> We should call of_node_put() for the reference returned by
>> of_parse_phandle() in fail path or when it is not used anymore.
>> Here we only need to move the of_node_put() before the check.
>
>Thank you for the analysis, yes, this is a bug.
>
>If you add the missing blank line, you can attach my:
>Acked-by: Peter Ujfalusi <peter.ujfalusi@xxxxxxxxx>
>
Thanks very much for your effort to review my patch.
I will do that in new version soon.
>> Fixes: d70241913413 ("dmaengine: ti: k3-udma: Add glue layer for non DMAengine users")
>> Signed-off-by: Liang He <windhl@xxxxxxx>
>> ---
>>
>> I cannot find the 'k3-udma-private.c' comiple item in drivers/dma/ti/Makefile,
>> I wonder if the author has forgotten the compile item and the
>> k3-udma-private.c has not been compiled before.
>>
>> I have tried to add k3-udma-private.o in the Makefile, but there are lots of
>> compile errors.
>> Please check it carefully and if possbile please teach me how to compile it, thanks.
>
>the k3-udma-private.c is included in to k3-udma.c (see end of file).
>When the UDMA stack was introduced we needed to have a glue layer to
>service the networking users due to the lack of infrastructure via
>DMAengine.
>The glue layer needs to tap in to the DMAengine driver, but I did not
>wanted to expose low level interfaces, structs, ops in order to be able
>to get rid of the glue layer when we have all the needed features in
>DMAengine.
>
>The k3-udma-private.c is part of the k3-udma.c but kept as separate file
>to make it explicit that it is suppose to go away and to not mix it with
>the DMAengine driver.
>
Thanks for you explaination, this is a great lesson!
>> drivers/dma/ti/k3-udma-private.c | 5 ++---
>> 1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/dma/ti/k3-udma-private.c b/drivers/dma/ti/k3-udma-private.c
>> index d4f1e4e9603a..ec274ef7d5ea 100644
>> --- a/drivers/dma/ti/k3-udma-private.c
>> +++ b/drivers/dma/ti/k3-udma-private.c
>> @@ -31,14 +31,13 @@ struct udma_dev *of_xudma_dev_get(struct device_node *np, const char *property)
>> }
>>
>> pdev = of_find_device_by_node(udma_node);
>> + if (np != udma_node)
>> + of_node_put(udma_node);
>
>Can you add a blank line here for readability?
>
Thanks, I will do it.
>> if (!pdev) {
>> pr_debug("UDMA device not found\n");
>> return ERR_PTR(-EPROBE_DEFER);
>> }
>>
>> - if (np != udma_node)
>> - of_node_put(udma_node);
>> -
>> ud = platform_get_drvdata(pdev);
>> if (!ud) {
>> pr_debug("UDMA has not been probed\n");
>
>--
>Péter
