Hi
Am 25.02.25 um 14:27 schrieb andriy.shevchenko@xxxxxxxxxxxxxxx:
On Tue, Feb 25, 2025 at 12:59:43PM +0100, Thomas Zimmermann wrote:
Am 25.02.25 um 12:01 schrieb andriy.shevchenko@xxxxxxxxxxxxxxx:
On Tue, Feb 25, 2025 at 10:48:53AM +0000, Aditya Garg wrote:
On 25 Feb 2025, at 4:17 PM, andriy.shevchenko@xxxxxxxxxxxxxxx wrote:
On Tue, Feb 25, 2025 at 10:36:03AM +0000, Aditya Garg wrote:
On 25 Feb 2025, at 4:03 PM, andriy.shevchenko@xxxxxxxxxxxxxxx wrote:
On Tue, Feb 25, 2025 at 10:09:42AM +0000, Aditya Garg wrote:
...
+static int appletbdrm_probe(struct usb_interface *intf,
+ const struct usb_device_id *id)
+{
+ struct usb_endpoint_descriptor *bulk_in, *bulk_out;
+ struct device *dev = &intf->dev;
+ struct appletbdrm_device *adev;
+ struct drm_device *drm;
+ int ret;
+
+ ret = usb_find_common_endpoints(intf->cur_altsetting, &bulk_in, &bulk_out, NULL, NULL);
+ if (ret) {
+ drm_err(drm, "Failed to find bulk endpoints\n");
This is simply wrong (and in this case even lead to crash in some circumstances).
drm_err() may not be used here. That's my point in previous discussions.
Independently on the subsystem the ->probe() for the sake of consistency and
being informative should only rely on struct device *dev,
I'm not sure how drm_err works,
It's a macro.
but struct drm_device does have a struct device *dev as well.
Yes, but only when it's initialized.
Anyways, this is something I'll leave for Thomas to reply.
The code above is wrong independently on his reply :-)
I'm kinda stuck between contrasting views of 2 kernel maintainers lol,
so I said let Thomas reply.
Sure. I also want him to clarify my question about potential drm_err_probe().
These threads get a little lengthy. What is the question?
How drm_err_probe() can be (consistently) implemented as there are and will be
cases when we want to return an error code with the message and having DRM devce
not being available, please?
The DRM logging works with a DRM device pointer of NULL. It'll simply
leave out device infos.
Also, drm_err() has a downside of not checking for deferred probe and
potentially leads to the noisy log floods.
I think it should be possible to export __dev_probe_failed() [1] from
the core and write drm_err_probe() and drm_warn_probe() around this. The
output then looks like a DRM logging, but behaves like dev-based
logging. Note that DRM logging already is an elaborate wrapper around
the dev-based logging, so it will be more of the same.
Best regards
Thomas
[1]
https://elixir.bootlin.com/linux/v6.13.4/source/drivers/base/core.c#L5008
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
