Re: [PATCH v3] video: udlfb: Switch from the pr_*() to the dev_*() logging functions

[Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>]

On Monday, January 15, 2018 04:40:27 PM Ladislav Michl wrote:
> Use dev_err() and dev_info() instead of pr_err() and pr_info().
> USB device is used as argument to dev_*() functions for probe
> and urb manipulation, FB device for framebuffer related info.
> 
> Also noisy device probe output was partly removed as idVendor,
> idProduct, name and serial are already printed by usb core,
> and partly turned into debug output.
> 
> Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
> ---
>  Changes:
>  - v2: Moved warnings removal into separate patch
>  - v3: Fixed warning about ignored return value, fixed space
>        before end of comment text, fixed few checkpatch warnings
> 
>  drivers/video/fbdev/udlfb.c | 232 ++++++++++++++++++++++----------------------
>  1 file changed, 116 insertions(+), 116 deletions(-)

[...]

> @@ -1455,7 +1452,7 @@ static const struct bin_attribute edid_attr = {
>  	.write = edid_store
>  };
>  
> -static struct device_attribute fb_device_attrs[] = {
> +static const struct device_attribute fb_device_attrs[] = {

This has been introduced in v3 and should be in a separate pre-patch.

>  	__ATTR_RO(metrics_bytes_rendered),
>  	__ATTR_RO(metrics_bytes_identical),
>  	__ATTR_RO(metrics_bytes_sent),

[...]

> @@ -1569,56 +1568,46 @@ static int dlfb_parse_vendor_descriptor(struct dlfb_data *dlfb,
>  
>  static void dlfb_init_framebuffer_work(struct work_struct *work);
>  
> -static int dlfb_usb_probe(struct usb_interface *interface,
> -			const struct usb_device_id *id)
> +static int dlfb_usb_probe(struct usb_interface *intf,
> +			  const struct usb_device_id *id)
>  {
> -	struct usb_device *usbdev;
>  	struct dlfb_data *dlfb;
>  	int retval = -ENOMEM;
> +	struct usb_device *usbdev = interface_to_usbdev(intf);
>  
>  	/* usb initialization */
> -
> -	usbdev = interface_to_usbdev(interface);
> -
>  	dlfb = kzalloc(sizeof(*dlfb), GFP_KERNEL);
> -	if (dlfb = NULL) {
> -		dev_err(&interface->dev, "dlfb_usb_probe: failed alloc of dev struct\n");

This dev_err() removal has been introduced in v3 and I believe that
it should be reversed (according to previous request of not deleting
allocation error messages).

> +	if (!dlfb)
>  		goto error;
> -	}

The rest looks fine.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics