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

[Dan Carpenter]

On Wed, May 23, 2012 at 11:28:32AM -0500, 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.

That function wouldn't be so bad if we just removed all the consts.
There is no point because we drop all of them anyway when we return
the struct.

regards,
dan carpenter


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/