Hi Hans On 22.10.20 12:17, Hans de Goede wrote: > Hi, > > On 10/22/20 11:30 AM, Thomas Zimmermann wrote: >> Hi >> >> On 22.10.20 11:20, Hans de Goede wrote: >>> Hi, >>> >>> On 10/21/20 3:07 PM, Thomas Zimmermann wrote: >>>> The drivers gm12u320 and udl operate on USB devices. They leave the >>>> PCI device in struct drm_device empty and store the USB device in their >>>> own driver structure. >>>> >>>> Fix this special case and save a few bytes by putting the USB device >>>> into an anonymous union with the PCI data. It's expected that DRM >>>> core and helpers only touch the PCI-device field for actual PCI devices. >>>> >>>> Thomas Zimmermann (3): >>>> drm: Add reference to USB device to struct drm_device >>>> drm/tiny/gm12u320: Store USB device in struct drm_device.udev >>>> drm/udl: Store USB device in struct drm_device.udev >>> >>> This series looks good to me: >>> >>> Reviewed-by: Hans de Goede <hdegoede@xxxxxxxxxx> >> >> Thanks a lot. Following Daniel's request, I'll drop patch 1 and instead >> do an upcast from drm_device.dev to the USB device structure. The driver >> patches 2 and 3 will be slightly different. Unless you object, I''ll >> take the r-b into the new patches. > > I somehow missed Daniel's reply about this. > > With that said, hmm that is going to be an interesting up-cast, at least > for the gm12u320, that is going to look something like this: > > struct usb_device *udev = interface_to_usbdev(to_usb_interface(drm_dev->dev)); > > (I wrote drm_dev instead of dev to make it more clear what is going on) > > For the DRM_DEV_ERROR() macro you can just use gm12u320->dev.dev , > that will make the errors be printed with the in usb-interface device-name > as prefix instead of the usb-device device-name, but that is fine. > > I wonder of this is all worth it then though, just to save those few bytes ? > Daniel and I briefly discussed on IRC if we could make drm_device.pdev (the PCI dev ) a legacy field in the longer term. All Linux devices would be retrieved from drm_device.dev then. > The first version made some sense since it made how drm devices with > usb resp. pci parents are handled consistent. Now it seems to make the code > somewhat harder to understand just to save the storage for a single pointer... What's the implication of setting drm_device.dev to the value of struct usb_device.dev ? That's the device all the code interacts with. Best regards Thomas > > Regards, > > Hans > > _______________________________________________ > dri-devel mailing list > dri-devel@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/dri-devel > -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
