On 16-06-21, 15:16, Sanjay R Mehta wrote:

On 6/16/2021 1:29 PM, Greg KH wrote:

quoted

[CAUTION: External Email]

On Wed, Jun 16, 2021 at 01:22:54PM +0530, Vinod Koul wrote:

quoted

On 16-06-21, 12:27, Sanjay R Mehta wrote:

quoted

On 6/16/2021 11:46 AM, Greg KH wrote:

quoted

[CAUTION: External Email]

On Wed, Jun 16, 2021 at 10:24:52AM +0530, Sanjay R Mehta wrote:

quoted

On 6/16/2021 9:45 AM, Vinod Koul wrote:

quoted

[CAUTION: External Email]

On 15-06-21, 16:50, Sanjay R Mehta wrote:

quoted

quoted

quoted

+static struct pt_device *pt_alloc_struct(struct device *dev)

In looking at this, why are you dealing with a "raw" struct device?
Shouldn't this be a parent pointer?  Why not pass in the real type that
this can be made a child of?

quoted

quoted

quoted

quoted

quoted

quoted

quoted

quoted

+{
+     struct pt_device *pt;
+
+     pt = devm_kzalloc(dev, sizeof(*pt), GFP_KERNEL);
+
+     if (!pt)
+             return NULL;
+     pt->dev = dev;
+     pt->ord = atomic_inc_return(&pt_ordinal);

What is the use of this number?

There are eight similar instances of this DMA engine on AMD SOC.
It is to differentiate each of these instances.

Are they individual device objects?

Yes, they are individual device objects.

Then what is "ord" for?  Why are you using an atomic variable for this?
What does this field do?  Why doesn't the normal way of naming a device
come into play here instead?

Hi Greg,

The value of "ord" is incremented for each device instance and then it
is used to store different name for each device as shown in below snippet.

    pt->ord = atomic_inc_return(&pt_ordinal);
    snprintf(pt->name, MAX_PT_NAME_LEN, "pt-%u", pt->ord);

Okay why not use device->name ?

Ah, I missed this.  Yes, do not have 2 names for the same structure,
that is wasteful and confusing.

Thanks, Greg & Vinod. I just verified with "dev_name(dev)" and this is
serving the purpose :).

I will send this change in the next version.

Great, but there are few more questions I had, like who creates the
device etc, can you please respond to those questions as well, so that
we understand properly how this device works

Thanks
-- 
~Vinod