On Mon, Jan 15, 2018 at 05:01:30PM +0100, Bartlomiej Zolnierkiewicz wrote:

On Monday, January 15, 2018 04:40:27 PM Ladislav Michl wrote:

quoted

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 <redacted>
---
 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(-)

[...]

quoted

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

Well, as I hadn't the same toolchain installed I didn't want to risk another
warning. Members of this structure are assigned to the const pointer later.

I'll take opportunity and constify some more pointers.

quoted

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

[...]

quoted

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

Reversed (it is the only allocation failure point in this function, so it 
should be clear where it failed, but let's leave it here)

quoted

+	if (!dlfb)
 		goto error;
-	}

The rest looks fine.

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