From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1751871AbdBOOGL (ORCPT <rfc822;w@1wt.eu>);
        Wed, 15 Feb 2017 09:06:11 -0500
Received: from mail-pg0-f68.google.com ([74.125.83.68]:34866 "EHLO
        mail-pg0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1751813AbdBOOGA (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Wed, 15 Feb 2017 09:06:00 -0500
Date: Wed, 15 Feb 2017 14:05:48 +0800
From: Cheah Kok Cheong <thrust73@gmail.com>
To: Ian Abbott <abbotti@mev.co.uk>
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: <20170215060548.GA10944@linux-Precision-WorkStation-T5500>
References: <1486809467-5434-1-git-send-email-thrust73@gmail.com>
 <a5a9a523-f025-f0da-1265-7e3e507f194e@mev.co.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <a5a9a523-f025-f0da-1265-7e3e507f194e@mev.co.uk>
User-Agent: Mutt/1.5.24 (2015-08-30)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Mon, Feb 13, 2017 at 11:14:14AM +0000, Ian Abbott wrote:
> On 11/02/17 10:37, Cheah Kok Cheong wrote:
> >+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");
> >+			return ret;
> >+		}
> >+
> >+		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);
> >+
> >+	return 0;
> >+}
> >+module_init(comedi_test_init);
> >+
> >+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