Messages in this thread | | | Date | Wed, 15 Feb 2017 14:05:48 +0800 | From | Cheah Kok Cheong <> | Subject | Re: [PATCH v2] Staging: comedi: drivers: comedi_test: Add auto-configuration capability |
| |
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
| |