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

[Cheah Kok Cheong <thrust73@gmail.com>]

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