Messages in this thread | | | Date | Wed, 20 May 2015 10:07:48 +0200 | From | Jean Delvare <> | Subject | Re: [PATCH v5 WIP 5/5] paride: use new parport device model |
| |
Hi Sudip,
On Wed, 6 May 2015 15:46:17 +0530, Sudip Mukherjee wrote: > modify paride driver to use the new parallel port device model.
Leading capital please ;-)
> > Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org> > --- > > in v4 of i2c-parport patch Jean mentioned to use the full name while > in probe. That has been done in other drivers except this one. > The higher layer drivers (pcd , pd etc.) are appending the unit number > to the name before the name is being sent to the lower level (paride).
Mmmpf. If not all drivers can stick to the convention, this kind of voids my original point.
Anyway... Again not a complete review, just two things which caught my eye:
> -static int pi_register_parport(PIA * pi, int verbose) > +static int pi_register_parport(PIA *pi, int verbose, int unit) > { > struct parport *port; > + struct pardev_cb *par_cb; > > port = parport_find_base(pi->port); > if (!port) > return 0; > - > - pi->pardev = parport_register_device(port, > - pi->device, NULL, > - pi_wake_up, NULL, 0, (void *) pi); > + par_cb = kzalloc(sizeof(*par_cb), GFP_KERNEL); > + if (!par_cb) { > + parport_put_port(port); > + return 0; > + } > + par_cb->flags = 0; > + par_cb->wakeup = pi_wake_up; > + par_cb->private = (void *)pi; > + pi->pardev = parport_register_dev_model(port, pi->device, par_cb, > + unit);
If pi->device already includes the device number, and you are passing it again as unit, won't you end up with odd names like pcd0.0, pcd1.1, pcd2.2 etc?
> parport_put_port(port); > + kfree(par_cb);
If you don't need par_cb anywhere else then you shouldn't be allocating it dynamically. It's small enough to fit on the stack really.
-- Jean Delvare SUSE L3 Support
| |