Re: [PATCH 0/3] drm: Store USB device in struct drm_device

[Hans de Goede <hdegoede@redhat.com>]

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@redhat.com>
> 
> 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 ?

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

Regards,

Hans

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel