Re: [PATCH] staging: comedi: remove this_board macro in the s526 driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2012-05-23 17:28, H Hartley Sweeten wrote:
On Wednesday, May 23, 2012 2:18 AM, Ian Abbott wrote:
On 2012-05-23 06:48, Dan Carpenter wrote:
On Tue, May 22, 2012 at 06:20:10PM -0700, H Hartley Sweeten wrote:
-/*
- * Initialize dev->board_name.  Note that we can use the "thisboard"
- * macro now, since we just initialized it in the last line.
- */
-	dev->board_ptr =&s526_boards[0];

Was this intended?  Most of the boards have auto probing so the
->board_ptr gets set automatically.  We already called
comedi_board() so I wonder if the autoprobed board is the same as
the&s526_boards[0];?  NULL pointer perhaps?  I don't know.

If .num_names in struct comedi_driver is non-zero, the core comedi
module will set dev->board_ptr to something non-NULL and meaningful to
the driver before it calls the driver's ->attach() method.  (This is
done by the comedi_recognize() function.)  The driver can change
dev->board_ptr to something else if it wants.  In this case,
dev->board_ptr will already be set to&s526_boards[0] so there is no
point setting it again.

Ian,

Side-note on the boardinfo stuff.

I wonder if it makes sense to create a common boardinfo struct
and modify the comedi_driver struct a bit. Something like:

struct comedi_boardinfo {
	const char *name;
	unsigned short	vendor;
	unsigned short device;
	void *private;
};

struct comedi_driver {
	struct comedi_driver *next;

	const char *driver_name;
	struct module *module;
	int (*attach) (struct comedi_device *, struct comedi_devconfig *);
	void (*detach) (struct comedi_device *);
	int (*attach_pci) (struct comedi_device *, struct pci_dev *);
	int (*attach_usb) (struct comedi_device *, struct usb_interface *);

	u32 num_boards;
	struct comedi_boardinfo *board;
};

This would allow comedi_recognize() to walk the boardinfo
to find the match without all the ugly casts. It should also
allow creating a common comedi_find_pcidev() for all the
comedi pci drivers to use when probing for the board_ptr.

This would create a bit of churn but I think it would be a
lot cleaner in the end.

What do you think?

Please don't do this yet. We might want to implement auto configuration for device types that have no concept of vendor ID or product ID, such as PCMCIA, platform devices, SPI devices etc., either by adding extra attach_xxx hooks to struct comedi_driver for the more common bus types, or a more generic attach_generic hook for the less common bus types.

Also, it would waste space in the ISA-only drivers.

--
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@xxxxxxxxx>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel


[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux