On Wed, Jul 21, 2021 at 10:21:46AM +0200, Jean Delvare wrote:
> Hi Andy,
>
> On Mon, 12 Jul 2021 17:11:19 +0300, Andy Shevchenko wrote:
> > Switch to use module_parport_driver() to reduce boilerplate code.
>
> This has the downside of moving the sanity check of the type parameter
> to run time, instead of driver load time. In particular this means that
> loading the i2c-parport driver without specifying the type will no
> longer fail.
And this is actually an advantage of the change if you think about it.
Now we can have a module that won't fail at boot time and give the user
a chance to amend parameter at run time if it was wrong at boot time.
> The driver will load successfully, but won't do anything.
>
> While I prefer user errors to be reported as soon as possible, I don't
> really mind here, as parallel port drivers are not something worth
> debating over at this point in time. As a matter of fact, I can't
> possibly test this change as I no longer have a parallel port on any of
> my systems.
>
> So if that's the direction we want to take then so be it.
>
> Reviewed-by: Jean Delvare <jdelvare@xxxxxxx>
Thanks!
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> > ---
> > v2: fixed compilation error (Jean, LKP)
> > drivers/i2c/busses/i2c-parport.c | 36 ++++++++++----------------------
> > 1 file changed, 11 insertions(+), 25 deletions(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-parport.c b/drivers/i2c/busses/i2c-parport.c
> > index a535889acca6..231145c48728 100644
> > --- a/drivers/i2c/busses/i2c-parport.c
> > +++ b/drivers/i2c/busses/i2c-parport.c
> > @@ -267,6 +267,16 @@ static void i2c_parport_attach(struct parport *port)
> > int i;
> > struct pardev_cb i2c_parport_cb;
> >
> > + if (type < 0) {
> > + pr_warn("adapter type unspecified\n");
> > + return;
> > + }
> > +
> > + if (type >= ARRAY_SIZE(adapter_parm)) {
> > + pr_warn("invalid type (%d)\n", type);
> > + return;
> > + }
> > +
> > for (i = 0; i < MAX_DEVICE; i++) {
> > if (parport[i] == -1)
> > continue;
> > @@ -392,32 +402,8 @@ static struct parport_driver i2c_parport_driver = {
> > .detach = i2c_parport_detach,
> > .devmodel = true,
> > };
> > -
> > -/* ----- Module loading, unloading and information ------------------------ */
> > -
> > -static int __init i2c_parport_init(void)
> > -{
> > - if (type < 0) {
> > - pr_warn("adapter type unspecified\n");
> > - return -ENODEV;
> > - }
> > -
> > - if (type >= ARRAY_SIZE(adapter_parm)) {
> > - pr_warn("invalid type (%d)\n", type);
> > - return -ENODEV;
> > - }
> > -
> > - return parport_register_driver(&i2c_parport_driver);
> > -}
> > -
> > -static void __exit i2c_parport_exit(void)
> > -{
> > - parport_unregister_driver(&i2c_parport_driver);
> > -}
> > +module_parport_driver(i2c_parport_driver);
> >
> > MODULE_AUTHOR("Jean Delvare <jdelvare@xxxxxxx>");
> > MODULE_DESCRIPTION("I2C bus over parallel port");
> > MODULE_LICENSE("GPL");
> > -
> > -module_init(i2c_parport_init);
> > -module_exit(i2c_parport_exit);
>
>
> --
> Jean Delvare
> SUSE L3 Support
--
With Best Regards,
Andy Shevchenko
