On Fri, Feb 12, 2010 at 03:50:53PM -0800, Ira W. Snyder wrote: > > The Janz ICAN3 is a MODULbus daughterboard which fits on the Janz CMOD-IO > PCI carrier board. It is an intelligent CAN controller, with a > microcontroller and associated firmware. > > Signed-off-by: Ira W. Snyder <[email protected]> > --- > [...] > +/* Maximum number of buffers on a CMOD-IO carrier board */ > +#define JANZ_MAX_MODULES 4 > + > +struct janz_device { > + struct device *dev; > + struct pci_dev *pdev; is dev == &pdev->dev? > + > + void __iomem *modulbus_regs; > + void __iomem *onboard_regs; > + > + /* hex switch position */ > + u8 hex; > + > + /* list of available modules */ > + struct list_head devices; dev (see above) already contains a list of devices You could avoid locking problems by just dropping this list? see include/linux/device.h for macros walking trough the device list of a device. > +}; > + > +/*----------------------------------------------------------------------------*/ > +/* Subdevice Support > */ > +/*----------------------------------------------------------------------------*/ > + > +struct janz_subdevice { > + struct list_head entry; > + struct platform_device pdev; > +}; and removing this structure in total (when the janz_device.devices is dropped). > + [...] > + > + pdata = pdev->dev.platform_data; > + pdata->modno = modno; > + pdev->id = modno; why twice. I'd rather avoid confusion and assign modno only once.
pdev->id should be independant of the board/module number. Right now, a second carrier board is not possible in the system because of this assignement to pdev->id. > + dev_dbg(priv->dev, "%s: PDATA %p name %s modno %d\n", __func__, pdata, > name, pdata->modno); > + > + /* MODULbus registers */ I think you're getting real close to a proper driver seperation. Good job. Kurt _______________________________________________ Socketcan-core mailing list [email protected] https://lists.berlios.de/mailman/listinfo/socketcan-core
