Messages in this thread | | | Date | Tue, 29 Apr 2014 20:59:04 +0200 | From | Christian Engelmayer <> | Subject | Re: [PATCH] staging: comedi: remove duplicate pointer assignments in attach functions |
| |
On Mon, 28 Apr 2014 22:36:13 +0000, Hartley Sweeten <HartleyS@visionengravers.com> wrote: > Technically, these drivers are fine as-is.
They are. The proposed change falls under minor code maintenance only.
> They are all legacy comedi drivers and use the manual attach mechanism. The > dev->board pointer is setup by the comedi core before calling the drivers > (*attach) so the foo = comedi_board(dev) is getting the board pointer that > was found by the core.
> Unlike most comedi legacy drivers, these drivers then do an additional "probe" > to try and identify the board. This could result in the dev->board_ptr getting > changed which requires updating the local variable for the board pointer.
The point is that while updating dev->board_ptr is necessary in case of the manual attach use case, deriving the local pointer before dev->board_ptr is decided is not. Furthermore it might be a bit risky to already have a local pointer to a valid, but potentially wrong comedi struct preselected by the core, although it cannot be used safely anyway until overwritten after the manual probe is done.
Having had a short look over the comedi code I was under the impression that the change would make the 4 affected functions consistent to the other parts that seemingly follow the skeleton.
static int skel_attach(struct comedi_device *dev, struct comedi_devconfig *it) { const struct skel_board *thisboard; struct skel_private *devpriv;
/* * If you can probe the device to determine what device in a series * it is, this is the place to do it. Otherwise, dev->board_ptr * should already be initialized. */ /* dev->board_ptr = skel_probe(dev, it); */
thisboard = comedi_board(dev);
> These probe functions need to be looked at to see if they are actually needed. > For now I would prefer that the existing code stay as-is.
That added about the intention of the patch, I'm fine if You want to question the necessity of the probes as a whole and keep the legacy code meanwhile untouched.
Regards, Christian [unhandled content-type:application/pgp-signature] | |