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
_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Reply via email to