linux-kernel - Re: [PATCH v7 1/3] dmaengine: ptdma: Initial driver for the AMD PTDMA

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <a4108a6d-9122-eb22-46a5-cba98dcfd032@amd.com>
Date:   Mon, 22 Mar 2021 12:10:54 +0530
From:   Sanjay R Mehta <sanmehta@....com>
To:     Vinod Koul <vkoul@...nel.org>
Cc:     gregkh@...uxfoundation.org, dan.j.williams@...el.com,
        Thomas.Lendacky@....com, Shyam-sundar.S-k@....com,
        Nehal-bakulchandra.Shah@....com, robh@...nel.org,
        mchehab+samsung@...nel.org, davem@...emloft.net,
        linux-kernel@...r.kernel.org, dmaengine@...r.kernel.org
Subject: Re: [PATCH v7 1/3] dmaengine: ptdma: Initial driver for the AMD PTDMA



On 3/22/2021 11:34 AM, Vinod Koul wrote:
> [CAUTION: External Email]
> 
> On 18-03-21, 16:16, Sanjay R Mehta wrote:
>>>> +#include <linux/delay.h>
>>>> +#include <linux/interrupt.h>
>>>> +#include <linux/kernel.h>
>>>> +#include <linux/kthread.h>
>>>> +#include <linux/module.h>
>>>> +#include <linux/pci_ids.h>
>>>> +#include <linux/pci.h>
>>>> +#include <linux/spinlock.h>
>>>> +#include <linux/sched.h>
>>>
>>> why do you need sched.h here?
>>>
>>>> +
>>>> +#include "ptdma.h"
>>>> +
>>>> +/* Ever-increasing value to produce unique unit numbers */
>>>> +static atomic_t pt_ordinal;
>>>
>>> What is the need of that?
>>>
>>
> 
> [please wrap your emails within 80 chars]
> 
Sure Vinod.

>> The "pt_ordinal" is incremented for each DMA instances and its number
>> is used only to assign device name for each instances.  This same
>> device name is passed as a string parameter in many places in code
>> like while using request_irq(), dma_pool_create() and in debugfs.
> 
> Why do you need that, why not use device name which is unique..?
> 
Can we take this as part of bug fixes series in future?

>> Also, I have implemented all of the comments for this patch except
>> this. if this is fine, will send the next version for review.
> 
> Am not sure I remember all the comments I gave, it has been _quite_ a
> while since the feedback was provided. In order to have effective review
> it would be great to revert back on a reasonable timeline and discuss...
> 
Apologies from my side. I understand that I have taken more time. But I assure it doesn't happen again.
I have already sent out v8, can you please have a look at and provide your valuable feedback.


> Thanks
> --
> ~Vinod
>