staging: comedi: remove duplicate pointer assignments in attach functions

Some board pointer are assigned twice via comedi_board() in the comedi low level driver attach functions. Remove the duplicate assignment from the variable definition where the pointer is not used anyway until assigned later in the function when dev->board_ptr, that comedi_board() relies on, is setup correctly.

---
Compile tested and applies against v3.15-rc2 as well as branch staging-next of tree git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git

---
 drivers/staging/comedi/drivers/das1800.c     | 2 +-
 drivers/staging/comedi/drivers/das800.c      | 2 +-
 drivers/staging/comedi/drivers/dt2801.c      | 2 +-
 drivers/staging/comedi/drivers/ni_at_a2150.c | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

index d581029..0335a70 100644
@@ -1479,7 +1479,7 @@ static int das1800_probe(struct comedi_device *dev)
 static int das1800_attach(struct comedi_device *dev,
 			  struct comedi_devconfig *it)
 {
-	const struct das1800_board *thisboard = comedi_board(dev);
+	const struct das1800_board *thisboard;
 	struct das1800_private *devpriv;
 	struct comedi_subdevice *s;
 	unsigned int irq = it->options[1];

index b23a12c..e2dc43d 100644
@@ -683,7 +683,7 @@ static int das800_probe(struct comedi_device *dev)
 
 static int das800_attach(struct comedi_device *dev, struct comedi_devconfig *it)
 {
-	const struct das800_board *thisboard = comedi_board(dev);
+	const struct das800_board *thisboard;
 	struct das800_private *devpriv;
 	struct comedi_subdevice *s;
 	unsigned int irq = it->options[1];

index d4d4e4b..4263014 100644
@@ -545,7 +545,7 @@ static int dt2801_dio_insn_config(struct comedi_device *dev,
 */
 static int dt2801_attach(struct comedi_device *dev, struct comedi_devconfig *it)
 {
-	const struct dt2801_board *board = comedi_board(dev);
+	const struct dt2801_board *board;
 	struct dt2801_private *devpriv;
 	struct comedi_subdevice *s;
 	int board_code, type;

index afbf251..fefd97e 100644
@@ -695,7 +695,7 @@ static int a2150_probe(struct comedi_device *dev)
 
 static int a2150_attach(struct comedi_device *dev, struct comedi_devconfig *it)
 {
-	const struct a2150_board *thisboard = comedi_board(dev);
+	const struct a2150_board *thisboard;
 	struct a2150_private *devpriv;
 	struct comedi_subdevice *s;
 	unsigned int irq = it->options[1];

1.9.1

Looks good!

-=( Ian Abbott @ MEV Ltd.    E-mail: <[email protected]>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-

Technically, these drivers are fine as-is.

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.

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.

Regards,
Hartley

They are. The proposed change falls under minor code maintenance only.

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);

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