From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755150AbdBPOAQ (ORCPT ); Thu, 16 Feb 2017 09:00:16 -0500 Received: from mail-pg0-f68.google.com ([74.125.83.68]:36487 "EHLO mail-pg0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754721AbdBPOAO (ORCPT ); Thu, 16 Feb 2017 09:00:14 -0500 Date: Thu, 16 Feb 2017 22:00:01 +0800 From: Cheah Kok Cheong To: Ian Abbott Cc: hsweeten@visionengravers.com, gregkh@linuxfoundation.org, devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2] Staging: comedi: drivers: comedi_test: Add auto-configuration capability Message-ID: <20170216140001.GA29532@linux-Precision-WorkStation-T5500> References: <1486809467-5434-1-git-send-email-thrust73@gmail.com> <20170215060548.GA10944@linux-Precision-WorkStation-T5500> <0ec646fc-d72c-a2a4-351b-cf7f6498e0f1@mev.co.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <0ec646fc-d72c-a2a4-351b-cf7f6498e0f1@mev.co.uk> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Feb 16, 2017 at 10:10:34AM +0000, Ian Abbott wrote: > On 15/02/17 06:05, Cheah Kok Cheong wrote: > >On Mon, Feb 13, 2017 at 11:14:14AM +0000, Ian Abbott wrote: > >>On 11/02/17 10:37, Cheah Kok Cheong wrote: > [snip] > >>>+static void __exit comedi_test_exit(void) > >>>+{ > >>>+ comedi_auto_unconfig(ctdev); > >>>+ > >>>+ device_destroy(ctcls, MKDEV(0, 0)); > >>>+ > >>>+ class_destroy(ctcls); > >> > >>If the driver init returned successfully, but failed to set-up the > >>auto-configured device, the device and class will not exist at this point, > >>so those three calls need to go in an 'if' statement. Perhaps you could use > >>'if (ctcls) {', and set 'ctcls = NULL;' after calling > >>'class_destroy(ctcls);' in the init function. > >> > >>Apart from that, it looks fine. > >> > > > >Thanks for pointing out those two pointers have to be "NULL" if > >auto-configuration fails. > > > >Please review below snippet. > >Sorry for the inconvenience caused. > > > >[ Snip ] > > > >static int __init comedi_test_init(void) > >{ > > int ret; > > > > ret = comedi_driver_register(&waveform_driver); > > if (ret) { > > pr_err("comedi_test: unable to register driver\n"); > > return ret; > > } > > > > if (!config_mode) { > > ctcls = class_create(THIS_MODULE, CLASS_NAME); > > if (IS_ERR(ctcls)) { > > pr_warn("comedi_test: unable to create class\n"); > > goto clean3; > > } > > > > ctdev = device_create(ctcls, NULL, MKDEV(0, 0), NULL, DEV_NAME); > > if (IS_ERR(ctdev)) { > > pr_warn("comedi_test: unable to create device\n"); > > goto clean2; > > } > > > > ret = comedi_auto_config(ctdev, &waveform_driver, 0); > > if (ret) { > > pr_warn("comedi_test: unable to auto-configure device\n"); > > goto clean; > > } > > } > > > > return 0; > > > >clean: > > device_destroy(ctcls, MKDEV(0, 0)); > >clean2: > > class_destroy(ctcls); > > ctdev = NULL; > >clean3: > > ctcls = NULL; > > > > return 0; > >} > >module_init(comedi_test_init); > > > >static void __exit comedi_test_exit(void) > >{ > > if (ctdev) > > comedi_auto_unconfig(ctdev); > > > > if (ctcls) { > > device_destroy(ctcls, MKDEV(0, 0)); > > class_destroy(ctcls); > > } > > > > comedi_driver_unregister(&waveform_driver); > >} > >module_exit(comedi_test_exit); > > > >[ Snip ] > > > >Thks. > >Brgds, > >CheahKC > > > > Yes, that looks fine. > Thanks. I'll send V3 shortly. Brgds, CheahKC