On Sat, Dec 28, 2024 at 11:37:59AM +0530, Nihar Chaithanya wrote:
> The function ines_pcmcia_init_module() can be replaced by calling
> pcmcia_register_driver() directly. The error value from this
> function is not returned and the previous registering functions
> are not unregistered.
>
> The function gpib_register_driver() can fail and similar to
> pcmcia_register_driver() function failing, the previous registering
> functions are not unregistered.
>
> Replace cb_pcmcia_init_module() with pcmcia_register_driver().
> Unregister the gpib and pci register functions if the subsequent
> gpib or pcmcia register functions fail and return the error value.
>
> Signed-off-by: Nihar Chaithanya <niharchaithanya@xxxxxxxxx>
> ---
> drivers/staging/gpib/ines/ines_gpib.c | 81 ++++++++++++++++++++-------
> 1 file changed, 60 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/staging/gpib/ines/ines_gpib.c b/drivers/staging/gpib/ines/ines_gpib.c
> index 9d8387c3bf01..73df4cd16926 100644
> --- a/drivers/staging/gpib/ines/ines_gpib.c
> +++ b/drivers/staging/gpib/ines/ines_gpib.c
> @@ -1227,12 +1227,6 @@ static struct pcmcia_driver ines_gpib_cs_driver = {
> .resume = ines_gpib_resume,
> };
>
> -int ines_pcmcia_init_module(void)
> -{
> - pcmcia_register_driver(&ines_gpib_cs_driver);
> - return 0;
> -}
> -
> void ines_pcmcia_cleanup_module(void)
> {
> DEBUG(0, "ines_cs: unloading\n");
> @@ -1420,28 +1414,73 @@ void ines_pcmcia_detach(gpib_board_t *board)
>
> static int __init ines_init_module(void)
> {
> - int err = 0;
> + int ret;
>
> - err = pci_register_driver(&ines_pci_driver);
> - if (err) {
> - pr_err("ines_gpib: pci_driver_register failed!\n");
> - return err;
> + ret = pci_register_driver(&ines_pci_driver);
> + if (ret) {
> + pr_err("ines_gpib: pci_driver_register failed: error = %d\n", ret);
> + return ret;
> }
>
> - gpib_register_driver(&ines_pci_interface, THIS_MODULE);
> - gpib_register_driver(&ines_pci_unaccel_interface, THIS_MODULE);
> - gpib_register_driver(&ines_pci_accel_interface, THIS_MODULE);
> - gpib_register_driver(&ines_isa_interface, THIS_MODULE);
> + ret = gpib_register_driver(&ines_pci_interface, THIS_MODULE);
> + if (ret)
> + goto err_pci;
> +
> + ret = gpib_register_driver(&ines_pci_unaccel_interface, THIS_MODULE);
> + if (ret)
> + goto err_pci_unaccel;
> +
> + ret = gpib_register_driver(&ines_pci_accel_interface, THIS_MODULE);
> + if (ret)
> + goto err_pci_accel;
> +
> + ret = gpib_register_driver(&ines_isa_interface, THIS_MODULE);
> + if (ret)
> + goto err_isa;
> +
> #ifdef GPIB_PCMCIA
> - gpib_register_driver(&ines_pcmcia_interface, THIS_MODULE);
> - gpib_register_driver(&ines_pcmcia_unaccel_interface, THIS_MODULE);
> - gpib_register_driver(&ines_pcmcia_accel_interface, THIS_MODULE);
> - err += ines_pcmcia_init_module();
> + ret = gpib_register_driver(&ines_pcmcia_interface, THIS_MODULE);
> + if (ret)
> + goto err_pcmcia;
> +
> + ret = gpib_register_driver(&ines_pcmcia_unaccel_interface, THIS_MODULE);
> + if (ret)
> + goto err_pcmcia_unaccel;
> +
> + ret = gpib_register_driver(&ines_pcmcia_accel_interface, THIS_MODULE);
> + if (ret)
> + goto err_pcmcia_accel;
> +
> + ret = pcmcia_register_driver(&ines_gpib_cs_driver);
> + if (ret)
> + goto err_pcmcia_driver;
> #endif
> - if (err)
> - return -1;
>
> + pr_info("ines_gpib: module init is complete\n");
Why did you add this debugging line here that was not here at all?
> return 0;
> +
> +#ifdef GPIB_PCMCIA
> +err_pcmcia_driver:
> + pr_err("ines_gpib: pcmcia_register_driver failed: error = %d\n", ret);
Why just this error message for only this one failure?
> + gpib_unregister_driver(&ines_pcmcia_accel_interface);
> +err_pcmcia_accel:
> + gpib_unregister_driver(&ines_pcmcia_unaccel_interface);
> +err_pcmcia_unaccel:
> + gpib_unregister_driver(&ines_pcmcia_interface);
> +err_pcmcia:
> +#endif
> + gpib_unregister_driver(&ines_isa_interface);
> +err_isa:
> + gpib_unregister_driver(&ines_pci_accel_interface);
> +err_pci_accel:
> + gpib_unregister_driver(&ines_pci_unaccel_interface);
> +err_pci_unaccel:
> + gpib_unregister_driver(&ines_pci_interface);
> +err_pci:
> + pci_unregister_driver(&ines_pci_driver);
> +
> + pr_err("ines_gpib: gpib_register_driver failed!\n");
Why not provide the error number here too?
thanks,
greg k-h
