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:
> > 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@xxxxxxxxxxxxxx>
> > ---
> > 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.
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.
> > __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).
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)
> > + if (!dlfb)
> > goto error;
> > - }
>
> The rest looks fine.
>
> Best regards,
> --
> Bartlomiej Zolnierkiewicz
> Samsung R&D Institute Poland
> Samsung Electronics
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
