Re: [PATCH v4] NTB: Add IDT 89HPESxNTx PCIe-switches support

[Logan Gunthorpe <lsgunthorpe@gmail.com>]

On 06/03/17 06:57 PM, Serge Semin wrote:
> Agreed. Replaced with traditional dev_*() methods.

You should also be printing using the ntb's device and not the parent
device. ie use:

ndev->ntb.dev

instead of

ndev->ntb.pdev->dev


> It's obvious, to prevent a race condition of access to the Mapping table, which
> is implemented by Address and Data registers set.
> For the same reason the driver have spin lock to protect an access to the
> Global Switch registers using GASA-ADDR and GASA-DATA.

Ah, I see. Perhaps these should be in their own function in the same way
as the gasa functions? It would probably be a bit clearer.

> Kind of funny. You've sent some comments about devres methods usage, but
> suspected a memory leak at the moment where it's actually used. So take a look
> at the place of "ndev" structure allocation, and you'll find an answer.

Ah sorry, I missed that. Usually I expect to see the structure free'd by
the release function on the device. But NTB is a little odd in that regard.

Also, per section 8 of the Linux style guide[1], IMO this code is pretty
over commented.

Logan


[1] https://static.lwn.net/kerneldoc/process/coding-style.html#commenting