Re: [PATCH v5 2/2] drm/tiny: add driver for Apple Touch Bars in x86 Macs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi

Am 25.02.25 um 15:54 schrieb Aditya Garg:
[...]
+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,
That's never going to work with DRM. There's so much code in a DRM probe function that uses the DRM error functions.

This specific instance here is wrong, as the drm pointer hasn't been initialized. But as soon as it is, it's much better to use drm_err() and friends. It will do the right thing and give consistent output across drivers.

Ok so this is actually an interesting case, since I am trying to fix it. To initialise the drm pointer, we need to initialise adev, and to initialise adev, we need to run usb_find_common_endpoints first. So IMO, we cannot use drm_err here, but rather dev_err_probe can be used.

Maybe start reading those compiler warnings. They are there for a reason. Your compiler tells you that you are dereferencing an uninitialized pointer, right here:

https://elixir.bootlin.com/linux/v6.13.4/source/include/drm/drm_print.h#L586

Clearing that pointer to NULL will fix the error and make drm_err() work.

Best regards
Thomas


+        return ret;
+    }
+
+    adev = devm_drm_dev_alloc(dev, &appletbdrm_drm_driver, struct appletbdrm_device, drm);
+    if (IS_ERR(adev))
+        return PTR_ERR(adev);
+
+    adev->in_ep = bulk_in->bEndpointAddress;
+    adev->out_ep = bulk_out->bEndpointAddress;
+    adev->dmadev = dev;
+
+    drm = &adev->drm;
+
+    usb_set_intfdata(intf, adev);
+
+    ret = appletbdrm_get_information(adev);
+    if (ret) {
+        drm_err(drm, "Failed to get display information\n");
+        return ret;
+    }
+
+    ret = appletbdrm_signal_readiness(adev);
+    if (ret) {
+        drm_err(drm, "Failed to signal readiness\n");
+        return ret;
+    }
+
+    ret = appletbdrm_setup_mode_config(adev);
+    if (ret) {
+        drm_err(drm, "Failed to setup mode config\n");
+        return ret;
+    }
+
+    ret = drm_dev_register(drm, 0);
+    if (ret) {
+        drm_err(drm, "Failed to register DRM device\n");
+        return ret;
+    }
+
+    ret = appletbdrm_clear_display(adev);
+    if (ret) {
+        drm_err(drm, "Failed to clear display\n");
+        return ret;
+    }
+
+    return 0;
+}
--
--
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)


--
--
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)




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux