I had a few style comments on this patchset. Nothing that couldn't be fixed later. On Mon, Sep 10, 2012 at 10:51:41AM +0200, Samuel Iglesias Gonsálvez wrote: > From: Jens Taprogge <jens.taprogge@xxxxxxxxxxxx> > > Provide get_clockrate, set_clockrate, get_error, get_timeout and reset_timeout > callbacks. > > Signed-off-by: Jens Taprogge <jens.taprogge@xxxxxxxxxxxx> > Signed-off-by: Samuel Iglesias Gonsálvez <siglesias@xxxxxxxxxx> > --- > drivers/staging/ipack/bridges/tpci200.c | 107 +++++++++++++++++++++++++++++++ > 1 file changed, 107 insertions(+) > > diff --git a/drivers/staging/ipack/bridges/tpci200.c b/drivers/staging/ipack/bridges/tpci200.c > index 4383953..861b00d 100644 > --- a/drivers/staging/ipack/bridges/tpci200.c > +++ b/drivers/staging/ipack/bridges/tpci200.c > @@ -14,6 +14,20 @@ > #include <linux/module.h> > #include "tpci200.h" > > +static u16 tpci200_status_timeout[] = { > + TPCI200_A_TIMEOUT, > + TPCI200_B_TIMEOUT, > + TPCI200_C_TIMEOUT, > + TPCI200_D_TIMEOUT, > +}; > + > +static u16 tpci200_status_error[] = { > + TPCI200_A_ERROR, > + TPCI200_B_ERROR, > + TPCI200_C_ERROR, > + TPCI200_D_ERROR, > +}; > + > static struct ipack_bus_ops tpci200_bus_ops; > > static int tpci200_slot_unregister(struct ipack_device *dev); > @@ -507,6 +521,94 @@ out_unlock: > return res; > } > > +static int tpci200_get_clockrate(struct ipack_device *dev) > +{ > + struct tpci200_board *tpci200 = check_slot(dev); > + __le16 __iomem *addr; The point of the underscores in the __le16 is that you don't want to pollute user space headers in glibc with a bunch of kernel typedefs. It is not needed here. (Or if it is, then we would need to replace the u16 uses as well). > + > + if (!tpci200) > + return -ENODEV; > + > + addr = &tpci200->info->interface_regs->control[dev->slot]; > + return (ioread16(addr) & TPCI200_CLK32) ? 32 : 8; > +} > + > +static int tpci200_set_clockrate(struct ipack_device *dev, int mherz) > +{ > + struct tpci200_board *tpci200 = check_slot(dev); > + __le16 __iomem *addr; > + u16 reg; > + > + if (!tpci200) > + return -ENODEV; > + > + addr = &tpci200->info->interface_regs->control[dev->slot]; > + > + /* Ensure the control register is not changed by another task after we > + * have read it. */ > + mutex_lock(&tpci200->mutex); > + reg = ioread16(addr); > + switch (mherz) { > + case 8: > + reg &= ~(TPCI200_CLK32); break; > + case 32: > + reg |= TPCI200_CLK32; break; Put the breaks on the next line so that we can see them. At first I thought it fell through. > + default: > + mutex_unlock(&tpci200->mutex); > + return -EINVAL; > + } > + iowrite16(reg, addr); > + mutex_unlock(&tpci200->mutex); > + return 0; > +} > + > +static int tpci200_get_error(struct ipack_device *dev) > +{ > + struct tpci200_board *tpci200 = check_slot(dev); > + __le16 __iomem *addr; > + u16 mask; > + > + if (!tpci200) > + return -ENODEV; > + > + addr = &tpci200->info->interface_regs->status; > + mask = tpci200_status_error[dev->slot]; > + return (ioread16(addr) & mask) ? 1 : 0; > +} > + > +static int tpci200_get_timeout(struct ipack_device *dev) > +{ > + struct tpci200_board *tpci200 = check_slot(dev); > + __le16 __iomem *addr; > + u16 mask; > + > + if (!tpci200) > + return -ENODEV; > + > + addr = &tpci200->info->interface_regs->status; > + mask = tpci200_status_timeout[dev->slot]; > + > + return (ioread16(addr) & mask) ? 1 : 0; > +} > + > +static int tpci200_reset_timeout(struct ipack_device *dev) > +{ > + struct tpci200_board *tpci200 = check_slot(dev); > + __le16 __iomem *addr; > + u16 mask; > + > + if (!tpci200) > + return -ENODEV; > + > + addr = &tpci200->info->interface_regs->status; > + mask = tpci200_status_timeout[dev->slot]; > + > + iowrite16(mask, addr); > + return 0; > +} > + > + > + Only one blank line is here. > static void tpci200_uninstall(struct tpci200_board *tpci200) > { > int i; > @@ -524,6 +626,11 @@ static struct ipack_bus_ops tpci200_bus_ops = { > .request_irq = tpci200_request_irq, > .free_irq = tpci200_free_irq, > .remove_device = tpci200_slot_unregister, > + .get_clockrate = tpci200_get_clockrate, > + .set_clockrate = tpci200_set_clockrate, > + .get_error = tpci200_get_error, > + .get_timeout = tpci200_get_timeout, > + .reset_timeout = tpci200_reset_timeout, > }; > > static int tpci200_install(struct tpci200_board *tpci200) > -- > 1.7.10.4 > > _______________________________________________ > devel mailing list > devel@xxxxxxxxxxxxxxxxxxxxxx > http://driverdev.linuxdriverproject.org/mailman/listinfo/devel _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/devel