From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753605AbbCINgL (ORCPT ); Mon, 9 Mar 2015 09:36:11 -0400 Received: from mail-pd0-f179.google.com ([209.85.192.179]:40376 "EHLO mail-pd0-f179.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753334AbbCINgG (ORCPT ); Mon, 9 Mar 2015 09:36:06 -0400 Date: Mon, 9 Mar 2015 19:05:59 +0530 From: Sudip Mukherjee To: Willy Tarreau Cc: Greg Kroah-Hartman , devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 2/3] staging: panel: return register value Message-ID: <20150309133559.GB13002@sudip-PC> References: <1425836246-15355-1-git-send-email-sudipm.mukherjee@gmail.com> <1425836246-15355-2-git-send-email-sudipm.mukherjee@gmail.com> <20150309063007.GC6038@1wt.eu> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150309063007.GC6038@1wt.eu> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Mar 09, 2015 at 07:30:07AM +0100, Willy Tarreau wrote: > On Sun, Mar 08, 2015 at 11:07:25PM +0530, Sudip Mukherjee wrote: > > > > - if (parport_register_driver(&panel_driver)) { > > - pr_err("could not register with parport. Aborting.\n"); > > - return -EIO; > > - } > > - > > if (pprt) > > pr_info("driver version " PANEL_VERSION > > " registered on parport%d (io=0x%lx).\n", parport, > > @@ -2375,7 +2370,7 @@ static int __init panel_init_module(void) > > /* tells various subsystems about the fact that initialization > > is finished */ > > init_in_progress = 0; > > - return 0; > > + return parport_register_driver(&panel_driver); > > Here I'd rather keep the error message, as it's quite annoying when a > driver does not load and you don't know why, especially with something > like parport where there are many possible causes. Something like this > would be better in my opinion : > > - return 0; > + err = parport_register_driver(&panel_driver); > + if (err) > + pr_err("could not register with parport. Aborting.\n"); > + return err; > > Well, I just found that currently parport_register_driver() always > succeeds, but I still think it's better to verbosely handle errors. ok, i will send a v2, but is the error message really required? considering the fact that parport_register_driver() always succeeds .. regards sudip > > Thanks, > Willy >