On Tue, Sep 26, 2023 at 02:52:36PM -0700, Nuno Das Neves wrote:

On 9/26/2023 1:03 AM, Greg KH wrote:

quoted

On Tue, Sep 26, 2023 at 07:00:51AM +0000, Wei Liu wrote:

quoted

On Tue, Sep 26, 2023 at 08:31:03AM +0200, Greg KH wrote:

quoted

On Tue, Sep 26, 2023 at 05:54:34AM +0000, Wei Liu wrote:

quoted

On Tue, Sep 26, 2023 at 06:52:46AM +0200, Greg KH wrote:

quoted

On Mon, Sep 25, 2023 at 05:07:24PM -0700, Nuno Das Neves wrote:

quoted

On 9/23/2023 12:58 AM, Greg KH wrote:

quoted

Also, drivers should never call pr_*() calls, always use the proper
dev_*() calls instead.

We only use struct device in one place in this driver, I think that is the
only place it makes sense to use dev_*() over pr_*() calls.

Then the driver needs to be fixed to use struct device properly so that
you do have access to it when you want to print messages.  That's a
valid reason to pass around your device structure when needed.

What is the tangible benefit of using dev_*() over pr_*()?

Unified reporting and handling of userspace of kernel log messages so
they can be classified properly as well as dealing correctly with the
dynamic debugging kernel infrastructure.

Why wouldn't you want to use it?

As I said,
our use of struct device is very limited compared to all the places we
may need to log errors.

Then please fix that.

pr_*() is used by many, many drivers; it seems to be the norm.

Not at all, it is not.

We can certainly add a pr_fmt to improve the logging.

Please do it correctly so you don't have to go and add support for it
later when your tools people ask you why they can't properly parse your
driver's kernel log messages.

quoted

quoted

If we're working with real devices like network cards or graphics cards
I would agree -- it is easy to imagine that we have several cards of the
same model in the system -- but in real world there won't be two
hypervisor instances running on the same hardware.

We can stash the struct device inside some private data fields, but that
doesn't change the fact that we're still having one instance of the
structure. Is this what you want? Or do you have something else in mind?

You have a real device, it's how userspace interacts with your
subsystem.  Please use that, it is dynamically created and handled and
is the correct representation here.

Are you referring to the struct device we get from calling
misc_register?

Yes.

How would you suggest we get a reference to that device via e.g. open()
or ioctl() without keeping a global reference to it?

You explicitly have it in your open() and ioctl() call, you never need a
global reference for it the kernel gives it to you!

thanks,

greg k-h

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel