Re: [PATCH v2] Staging: comedi: drivers: comedi_test: Add auto-configuration capability

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

--
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@xxxxxxxxx> )=-
-=(                          Web: http://www.mev.co.uk/  )=-
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel



[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux