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.tapro...@taprogge.org>
>
> Provide get_clockrate, set_clockrate, get_error, get_timeout and reset_timeout
> callbacks.
>
> Signed-off-by: Jens Taprogge <jens.tapro...@taprogge.org>
> Signed-off-by: Samuel Iglesias Gonsálvez <sigles...@igalia.com>
> ---
> 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
> de...@linuxdriverproject.org
> http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
