Messages in this thread | | | Date | Wed, 20 May 2015 13:46:52 +0530 | From | Sudip Mukherjee <> | Subject | Re: [PATCH v5 WIP 4/5] i2c-parport: use new parport device model |
| |
On Wed, May 20, 2015 at 09:57:24AM +0200, Jean Delvare wrote: > Hi Sudip, > > On Wed, 6 May 2015 15:46:16 +0530, Sudip Mukherjee wrote: <snip> > > +static struct pardev_cb i2c_parport_cb = { > > + .flags = PARPORT_FLAG_EXCL, > > + .irq_func = i2c_parport_irq, > > +}; > > There's no reason for this variable to be global. It is only needed > temporarily at attach time if I understand correctly, so it should be > local to function i2c_parport_attach(). yes, this is only like a placeholder of the function pointers. will make it local in the patch. (I guess WIP is over) > > > + <snip> > > +static int i2c_parport_probe(struct pardevice *par_dev) > > +{ > > + if (strcmp(par_dev->name, "i2c-parport")) > > + return -ENODEV; > > + return 0; > > +} > > I'm wondering, is there any reason why this can't be automated by the > driver core part of the code? Most drivers will simply compare > drv->name with par_dev->name, which could be done in function > parport_probe() when no custom probe function is provided. yes, it can be done. I will add that in the patch. > > Now I see that you use the existence of the probe callback to decide > that the driver implements the device driver model. I suppose you could > use match_port instead, except that for some reason the paride driver > doesn't implement one. Maybe it should, or maybe you can check of the > presence of either to decide that this is a device model driver. as Dan suggested I am checking on devmodel now, instead of the existence of probe, so what you are suggestiong now can definitely be done with very small change. > > > + > > static struct parport_driver i2c_parport_driver = { > > .name = "i2c-parport", > > - .attach = i2c_parport_attach, > > + .match_port = i2c_parport_attach, > > .detach = i2c_parport_detach, > > + .probe = i2c_parport_probe, > > }; > > > > /* ----- Module loading, unloading and information ------------------------ */ > > Tested OK on my ADM1032 evaluation board. > > Tested-by: Jean Delvare <jdelvare@suse.de> Thanks again.
regards sudip > > -- > Jean Delvare > SUSE L3 Support
| |