[PATCH] drm: Add Grain Media GM12U320 driver v2

[Hans de Goede]

Hi,

On 23-07-19 09:28, Sam Ravnborg wrote:
> Hi Hans.
> 
> Driver looks good. Nce to see so much functionality in a driver less
> than 1000 loc.
> 
> Following some bike-shedding only - that should have been sent for v1
> already. But I thought better late then never.

Thank you for the feedback I've prepared a small followup cleanup
patch addressing some of your remarks.

<snip>

>> +#define DRIVER_NAME		"gm12u320"
>> +#define DRIVER_DESC		"Grain Media GM12U320 USB projector display"
>> +#define DRIVER_DATE		"2019"
>> +#define DRIVER_MAJOR		1
>> +#define DRIVER_MINOR		0
>> +#define DRIVER_PATCHLEVEL	1
> DRIVER_PATCHLEVEL is not used

Good one, fixed.

<snip>

>> +	vaddr = drm_gem_shmem_vmap(fb->obj[0]);
>> +	if (IS_ERR(vaddr)) {
>> +		DRM_ERROR("failed to vmap fb: %ld\n", PTR_ERR(vaddr));
>> +		goto put_fb;
>> +	}
>> +
>> +	if (fb->obj[0]->import_attach) {
>> +		ret = dma_buf_begin_cpu_access(
>> +			fb->obj[0]->import_attach->dmabuf, DMA_FROM_DEVICE);
>> +		if (ret) {
>> +			DRM_ERROR("dma_buf_begin_cpu_access err: %d\n", ret);
>> +			goto vunmap;
>> +		}
>> +	}
> Here the code uses DRM_ERROR("...");
> 
> 
>> +	/* Do not log errors caused by module unload or device unplug */
>> +	if (ret != -ECONNRESET && ret != -ESHUTDOWN)
>> +		dev_err(&gm12u320->udev->dev, "Frame update error: %d\n", ret);
>> +}
> And here dev_err(dev, "...");
> It had been more consistent to use DRM_DEV_ERROR(dev, "..."); here.

Good one, I've added a second follow-up patch using DRM_DEV_ERROR
consistently everywhere.

>> +/* ------------------------------------------------------------------ */
>> +/* gm12u320 connector						      */
>> +
> 
>> +
>> +#ifdef CONFIG_PM
>> +static int gm12u320_suspend(struct usb_interface *interface,
>> +			    pm_message_t message)
>> +{
> You can use __maybe_unused to get rid of the #ifdef,

Right, Noralf said that too, but I forgot. I've fixed this in the cleanup
patch.

Regards,

Hans