| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Sat, 20 Jul 2013 22:23:01 +0200
From: Linus Walleij <linus.walleij@...aro.org>
To: "Kim, Milo" <Milo.Kim@...com>
Cc: "grant.likely@...aro.org" <grant.likely@...aro.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-pwm@...r.kernel.org" <linux-pwm@...r.kernel.org>,
"thierry.reding@...il.com" <thierry.reding@...il.com>,
"sameo@...ux.intel.com" <sameo@...ux.intel.com>,
"lee.jones@...aro.org" <lee.jones@...aro.org>
Subject: Re: [PATCH 2/3] gpio: add LP3943 I2C GPIO expander driver
On Tue, Jul 16, 2013 at 4:38 AM, Kim, Milo <Milo.Kim@...com> wrote:
> +++ b/drivers/gpio/gpio-lp3943.c
> @@ -0,0 +1,224 @@
> +/*
> + * TI/National Semiconductor LP3943 GPIO driver
> + *
> + * Copyright (C) 2013 Texas Instruments
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; version 2.
> + */
> +
> +#include <linux/err.h>
> +#include <linux/gpio.h>
> +#include <linux/i2c.h>
> +#include <linux/mfd/lp3943.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
#include <linus/bitops.h>
> +#define to_lp3943_gpio(_chip) container_of(_chip, struct lp3943_gpio, chip)
> +
> +enum lp3943_gpios {
> + LP3943_GPIO1,
> + LP3943_GPIO2,
> + LP3943_GPIO3,
> + LP3943_GPIO4,
> + LP3943_GPIO5,
> + LP3943_GPIO6,
> + LP3943_GPIO7,
> + LP3943_GPIO8,
> + LP3943_GPIO9,
> + LP3943_GPIO10,
> + LP3943_GPIO11,
> + LP3943_GPIO12,
> + LP3943_GPIO13,
> + LP3943_GPIO14,
> + LP3943_GPIO15,
> + LP3943_GPIO16,
> + LP3943_MAX_GPIO,
> +};
> +
> +struct lp3943_gpio {
> + struct gpio_chip chip;
> + struct lp3943 *l;
> + bool is_input[LP3943_MAX_GPIO];
Please do this instead:
u16 is_input;
> +static int lp3943_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
> +{
> + struct lp3943_gpio *lg = to_lp3943_gpio(chip);
> + const struct lp3943_reg_cfg *mux = lg->l->mux_cfg;
> + u8 addr;
> + u8 mask;
> + u8 shift;
> +
> + lg->is_input[offset] = true;
So it would be:
lg->is_input |= BIT(offset);
> + addr = mux[offset].reg;
> + mask = mux[offset].mask;
> + shift = mux[offset].shift;
> +
> + return lp3943_update_bits(lg->l, addr, mask, LP3943_GPIO_IN << shift);
> +}
> +
> +static int lp3943_get_gpio_in_status(struct lp3943_gpio *lg,
> + struct gpio_chip *chip, unsigned offset)
> +{
> + u8 addr;
> + u8 read;
> + int err;
> +
> + switch (offset) {
> + case LP3943_GPIO1 ... LP3943_GPIO8:
> + addr = LP3943_REG_GPIO_A;
> + break;
> + case LP3943_GPIO9 ... LP3943_GPIO16:
> + addr = LP3943_REG_GPIO_B;
> + offset = offset - 8;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + err = lp3943_read_byte(lg->l, addr, &read);
> + if (err)
> + return err;
> +
> + if (read & (1 << offset))
> + return 1;
> + else
> + return 0;
Just:
return !!(read & BIT(offset));
> +}
> +
> +static int lp3943_get_gpio_out_status(struct lp3943_gpio *lg,
> + struct gpio_chip *chip, unsigned offset)
> +{
> + const struct lp3943_reg_cfg *mux = lg->l->mux_cfg;
> + u8 addr = mux[offset].reg;
> + u8 mask = mux[offset].mask;
> + u8 shift = mux[offset].shift;
> + u8 read;
> + int err;
> +
> + err = lp3943_read_byte(lg->l, addr, &read);
> + if (err)
> + return err;
> +
> + read = (read & mask) >> shift;
> +
> + if (read == LP3943_GPIO_OUT_HIGH)
> + return 1;
> + else if (read == LP3943_GPIO_OUT_LOW)
> + return 0;
> + else
> + return -EINVAL;
> +}
> +
> +static int lp3943_gpio_get(struct gpio_chip *chip, unsigned offset)
> +{
> + struct lp3943_gpio *lg = to_lp3943_gpio(chip);
> +
> + /*
> + * Limitation:
> + * LP3943 doesn't have the GPIO direction register. It provides
> + * only input and output status registers.
> + * So, direction info is required to handle the 'get' operation.
> + * This variable is updated whenever the direction is changed and
> + * it is used here.
> + */
> +
> + if (lg->is_input[offset])
if (lg->is_input & BIT(offset))
> + return lp3943_get_gpio_in_status(lg, chip, offset);
> + else
> + return lp3943_get_gpio_out_status(lg, chip, offset);
> +}
> +
> +static void lp3943_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
> +{
> + struct lp3943_gpio *lg = to_lp3943_gpio(chip);
> + const struct lp3943_reg_cfg *mux = lg->l->mux_cfg;
> + u8 addr;
> + u8 mask;
> + u8 shift;
> + u8 data;
> +
> + addr = mux[offset].reg;
> + mask = mux[offset].mask;
> + shift = mux[offset].shift;
> +
> + if (value)
> + data = LP3943_GPIO_OUT_HIGH;
> + else
> + data = LP3943_GPIO_OUT_LOW;
> +
> + lp3943_update_bits(lg->l, addr, mask, data << shift);
> +}
> +
> +static int lp3943_gpio_direction_output(struct gpio_chip *chip, unsigned offset,
> + int value)
> +{
> + struct lp3943_gpio *lg = to_lp3943_gpio(chip);
> +
> + lp3943_gpio_set(chip, offset, value);
> + lg->is_input[offset] = false;
lg->is_input &= ~BIT(offset);
> +
> + return 0;
> +}
> +
> +static const struct gpio_chip lp3943_gpio_chip = {
> + .label = "lp3943",
> + .owner = THIS_MODULE,
> + .direction_input = lp3943_gpio_direction_input,
> + .get = lp3943_gpio_get,
> + .direction_output = lp3943_gpio_direction_output,
> + .set = lp3943_gpio_set,
> + .base = -1,
> + .ngpio = LP3943_MAX_GPIO,
> + .can_sleep = 1,
> +};
> +
> +static int lp3943_gpio_probe(struct platform_device *pdev)
> +{
> + struct lp3943 *l = dev_get_drvdata(pdev->dev.parent);
> + struct lp3943_gpio *lg;
> + int ret;
> +
> + lg = devm_kzalloc(&pdev->dev, sizeof(struct lp3943_gpio), GFP_KERNEL);
> + if (!lg)
> + return -ENOMEM;
> +
> + lg->l = l;
> + lg->chip = lp3943_gpio_chip;
> + lg->chip.dev = &pdev->dev;
> +
> + platform_set_drvdata(pdev, lg);
> +
> + ret = gpiochip_add(&lg->chip);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "failed to add GPIO chip, err:%d\n", ret);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int lp3943_gpio_remove(struct platform_device *pdev)
> +{
> + struct lp3943_gpio *lg = platform_get_drvdata(pdev);
> + return gpiochip_remove(&lg->chip);
> +}
> +
> +static struct platform_driver lp3943_gpio_driver = {
> + .probe = lp3943_gpio_probe,
> + .remove = lp3943_gpio_remove,
> + .driver = {
> + .name = "lp3943-gpio",
> + .owner = THIS_MODULE,
> + },
No device tree probing support? Why?
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists