Messages in this thread | | | Date | Tue, 19 May 2015 14:32:55 +0300 | From | Dan Carpenter <> | Subject | Re: [PATCH v5 WIP 5/5] paride: use new parport device model |
| |
On Wed, May 06, 2015 at 03:46:17PM +0530, Sudip Mukherjee wrote: > +void *pi_register_driver(char *name) > +{ > + struct parport_driver *parp_drv; > + int ret; > + > + parp_drv = kzalloc(sizeof(*parp_drv), GFP_KERNEL); > + if (!parp_drv) > + return NULL; > + > + parp_drv->name = name; > + parp_drv->probe = pi_probe; > + > + ret = parport_register_driver(parp_drv); > + > + if (ret) > + return NULL;
Remove the blank line between the call and the error handling. We should probably free parp_drv on error.
> + return (void *)parp_drv; > +} > +EXPORT_SYMBOL(pi_register_driver); > + > +void pi_unregister_driver(void *_drv) > +{ > + struct parport_driver *drv = _drv; > + > + parport_unregister_driver(drv); > + kfree(drv); > +} > +EXPORT_SYMBOL(pi_unregister_driver); > diff --git a/drivers/block/paride/paride.h b/drivers/block/paride/paride.h > index 2bddbf4..ddb9e58 100644 > --- a/drivers/block/paride/paride.h > +++ b/drivers/block/paride/paride.h > @@ -165,6 +165,8 @@ typedef struct pi_protocol PIP; > > extern int paride_register( PIP * ); > extern void paride_unregister ( PIP * ); > +void *pi_register_driver(char *); > +void pi_unregister_driver(void *); > > #endif /* __DRIVERS_PARIDE_H__ */ > /* end of paride.h */ > diff --git a/drivers/block/paride/pcd.c b/drivers/block/paride/pcd.c > index 3b7c9f1..9336236 100644 > --- a/drivers/block/paride/pcd.c > +++ b/drivers/block/paride/pcd.c > @@ -221,6 +221,7 @@ static int pcd_busy; /* request being processed ? */ > static int pcd_sector; /* address of next requested sector */ > static int pcd_count; /* number of blocks still to do */ > static char *pcd_buf; /* buffer for request in progress */ > +static void *par_drv; /* reference of parport driver */ > > /* kernel glue structures */ > > @@ -690,6 +691,12 @@ static int pcd_detect(void) > printk("%s: %s version %s, major %d, nice %d\n", > name, name, PCD_VERSION, major, nice); > > + par_drv = pi_register_driver(name); > + if (!par_drv) { > + pr_err("failed to register %s driver\n", name); > + return -1; > + } > + > k = 0; > if (pcd_drive_count == 0) { /* nothing spec'd - so autoprobe for 1 */ > cd = pcd; > @@ -723,6 +730,7 @@ static int pcd_detect(void) > printk("%s: No CD-ROM drive found\n", name); > for (unit = 0, cd = pcd; unit < PCD_UNITS; unit++, cd++) > put_disk(cd->disk); > + pi_unregister_driver(par_drv);
You didn't introduce this, but I think the error handling here is broken for the autoprobe case. The autoprobe case doesn't have a for loop.
> return -1; > } >
regards, dan carpenter
| |