Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752575AbbDAWdm (ORCPT ); Wed, 1 Apr 2015 18:33:42 -0400 Received: from aserp1040.oracle.com ([141.146.126.69]:39328 "EHLO aserp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751566AbbDAWdl (ORCPT ); Wed, 1 Apr 2015 18:33:41 -0400 Date: Thu, 2 Apr 2015 01:34:03 +0300 From: Dan Carpenter To: Phong Tran Cc: gregkh@linuxfoundation.org, arve@android.com, riandrews@android.com, devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 3/3] staging: android: ion_test: unregister the platform device Message-ID: <20150401223403.GG10964@mwanda> References: <1427909900-22650-1-git-send-email-tranmanphong@gmail.com> <1427909900-22650-4-git-send-email-tranmanphong@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1427909900-22650-4-git-send-email-tranmanphong@gmail.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-Source-IP: userv0022.oracle.com [156.151.31.74] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2161 Lines: 60 On Thu, Apr 02, 2015 at 12:38:20AM +0700, Phong Tran wrote: > The driver has to unregister from platform device when it's unloaded > > Signed-off-by: Phong Tran > --- > drivers/staging/android/ion/ion_test.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/drivers/staging/android/ion/ion_test.c b/drivers/staging/android/ion/ion_test.c > index f36a35e..d2a236e 100644 > --- a/drivers/staging/android/ion/ion_test.c > +++ b/drivers/staging/android/ion/ion_test.c > @@ -278,6 +278,7 @@ static int ion_test_remove(struct platform_device *pdev) > return ret; > } > > +static struct platform_device *ion_test_platform_device; This name is too long. You will run into the 80 character limit. > static struct platform_driver ion_test_platform_driver = { > .remove = ion_test_remove, > .driver = { > @@ -287,13 +288,21 @@ static struct platform_driver ion_test_platform_driver = { > > static int __init ion_test_init(void) > { > - platform_device_register_simple("ion-test", -1, NULL, 0); > + ion_test_platform_device = platform_device_register_simple("ion-test", > + -1, NULL, 0); This indenting is off. It should be: ion_test_dev = platform_device_register_simple("ion-test", -1, NULL, 0); or something similar. > + > + if (!ion_test_platform_device) { > + pr_err("failed to register ion-test platform device\n"); People add error messages without thinking about it because they think, "Obviously, the more error messages the better." Almost all the bad things that can happen in platform_device_register_simple() generate their own error message. Also platform_driver_probe() doesn't have an error message so it's possible to leave them out if you want. Be bold! Leave out the message! Or if you want go ahead and leave it in. So long as you have thought about it is what matters. regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/