Re: [PATCH v2 02/11] drm/udl: Add reset_resume

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Sep 06, 2022 at 09:39:42AM +0200, Takashi Iwai wrote:
> From: Thomas Zimmermann <tzimmermann@suse.de>
> 
> Implement the reset_resume callback of struct usb_driver. Set the
> standard channel when called.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>  drivers/gpu/drm/udl/udl_drv.c  | 11 +++++++++++
>  drivers/gpu/drm/udl/udl_drv.h  |  1 +
>  drivers/gpu/drm/udl/udl_main.c |  2 +-
>  3 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c
> index 5703277c6f52..0ba88e5472a9 100644
> --- a/drivers/gpu/drm/udl/udl_drv.c
> +++ b/drivers/gpu/drm/udl/udl_drv.c
> @@ -32,6 +32,16 @@ static int udl_usb_resume(struct usb_interface *interface)
>  	return drm_mode_config_helper_resume(dev);
>  }
>  
> +static int udl_usb_reset_resume(struct usb_interface *interface)
> +{
> +	struct drm_device *dev = usb_get_intfdata(interface);
> +	struct udl_device *udl = to_udl(dev);
> +
> +	udl_select_std_channel(udl);
> +
> +	return drm_mode_config_helper_resume(dev);

Bit a bikeshed, but in the driver load case it would be neat to put the
call to udl_select_std_channel right above the call to
drm_mode_config_reset. But that is _really_ a bikeshed :-)

Also thanks for sending me on a bit a wild goose chase trying to figure
out what this reset_resume hook actually does and why.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> +}
> +
>  /*
>   * FIXME: Dma-buf sharing requires DMA support by the importing device.
>   *        This function is a workaround to make USB devices work as well.
> @@ -140,6 +150,7 @@ static struct usb_driver udl_driver = {
>  	.disconnect = udl_usb_disconnect,
>  	.suspend = udl_usb_suspend,
>  	.resume = udl_usb_resume,
> +	.reset_resume = udl_usb_reset_resume,
>  	.id_table = id_table,
>  };
>  module_usb_driver(udl_driver);
> diff --git a/drivers/gpu/drm/udl/udl_drv.h b/drivers/gpu/drm/udl/udl_drv.h
> index 28aaf75d71cf..37c14b0ff1fc 100644
> --- a/drivers/gpu/drm/udl/udl_drv.h
> +++ b/drivers/gpu/drm/udl/udl_drv.h
> @@ -95,6 +95,7 @@ int udl_render_hline(struct drm_device *dev, int log_bpp, struct urb **urb_ptr,
>  		     u32 byte_offset, u32 device_byte_offset, u32 byte_width);
>  
>  int udl_drop_usb(struct drm_device *dev);
> +int udl_select_std_channel(struct udl_device *udl);
>  
>  #define CMD_WRITE_RAW8   "\xAF\x60" /**< 8 bit raw write command. */
>  #define CMD_WRITE_RL8    "\xAF\x61" /**< 8 bit run length command. */
> diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c
> index fdafbf8f3c3c..7d1e6bbc165c 100644
> --- a/drivers/gpu/drm/udl/udl_main.c
> +++ b/drivers/gpu/drm/udl/udl_main.c
> @@ -92,7 +92,7 @@ static int udl_parse_vendor_descriptor(struct udl_device *udl)
>  /*
>   * Need to ensure a channel is selected before submitting URBs
>   */
> -static int udl_select_std_channel(struct udl_device *udl)
> +int udl_select_std_channel(struct udl_device *udl)
>  {
>  	static const u8 set_def_chn[] = {0x57, 0xCD, 0xDC, 0xA7,
>  					 0x1C, 0x88, 0x5E, 0x15,
> -- 
> 2.35.3
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch