| 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
| ||
|
Message-Id: <20121219170357.0BA403E0C4A@localhost>
Date: Wed, 19 Dec 2012 17:03:56 +0000
From: Grant Likely <grant.likely@...retlab.ca>
To: Peter Ujfalusi <peter.ujfalusi@...com>,
Linus Walleij <linus.walleij@...aro.org>
Cc: linux-kernel@...r.kernel.org, linux-omap@...r.kernel.org
Subject: Re: [PATCH v2 2/3] gpio: twl4030: Cache the direction and output states in private data
On Thu, 6 Dec 2012 11:52:06 +0100, Peter Ujfalusi <peter.ujfalusi@...com> wrote:
> Use more coherent locking in the driver. Use bitfield to store the GPIO
> direction and if the pin is configured as output store the status also in a
> bitfiled.
> In this way we can just look at these bitfields when we need information
> about the pin status and only reach out to the chip when it is needed.
>
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@...com>
Applied, thanks
g.
> ---
> drivers/gpio/gpio-twl4030.c | 99 ++++++++++++++++++++++++++++++---------------
> 1 file changed, 66 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/gpio/gpio-twl4030.c b/drivers/gpio/gpio-twl4030.c
> index c092739..a38e6e9c 100644
> --- a/drivers/gpio/gpio-twl4030.c
> +++ b/drivers/gpio/gpio-twl4030.c
> @@ -64,14 +64,15 @@
> /* Mask for GPIO registers when aggregated into a 32-bit integer */
> #define GPIO_32_MASK 0x0003ffff
>
> -/* Data structures */
> -static DEFINE_MUTEX(gpio_lock);
> -
> struct gpio_twl4030_priv {
> struct gpio_chip gpio_chip;
> + struct mutex mutex;
> int irq_base;
>
> + /* Bitfields for state caching */
> unsigned int usage_count;
> + unsigned int direction;
> + unsigned int out_state;
> };
>
> /*----------------------------------------------------------------------*/
> @@ -130,7 +131,7 @@ static inline int gpio_twl4030_read(u8 address)
>
> /*----------------------------------------------------------------------*/
>
> -static u8 cached_leden; /* protected by gpio_lock */
> +static u8 cached_leden;
>
> /* The LED lines are open drain outputs ... a FET pulls to GND, so an
> * external pullup is needed. We could also expose the integrated PWM
> @@ -144,14 +145,12 @@ static void twl4030_led_set_value(int led, int value)
> if (led)
> mask <<= 1;
>
> - mutex_lock(&gpio_lock);
> if (value)
> cached_leden &= ~mask;
> else
> cached_leden |= mask;
> status = twl_i2c_write_u8(TWL4030_MODULE_LED, cached_leden,
> TWL4030_LED_LEDEN_REG);
> - mutex_unlock(&gpio_lock);
> }
>
> static int twl4030_set_gpio_direction(int gpio, int is_input)
> @@ -162,7 +161,6 @@ static int twl4030_set_gpio_direction(int gpio, int is_input)
> u8 base = REG_GPIODATADIR1 + d_bnk;
> int ret = 0;
>
> - mutex_lock(&gpio_lock);
> ret = gpio_twl4030_read(base);
> if (ret >= 0) {
> if (is_input)
> @@ -172,7 +170,6 @@ static int twl4030_set_gpio_direction(int gpio, int is_input)
>
> ret = gpio_twl4030_write(base, reg);
> }
> - mutex_unlock(&gpio_lock);
> return ret;
> }
>
> @@ -212,7 +209,7 @@ static int twl_request(struct gpio_chip *chip, unsigned offset)
> struct gpio_twl4030_priv *priv = to_gpio_twl4030(chip);
> int status = 0;
>
> - mutex_lock(&gpio_lock);
> + mutex_lock(&priv->mutex);
>
> /* Support the two LED outputs as output-only GPIOs. */
> if (offset >= TWL4030_GPIO_MAX) {
> @@ -271,7 +268,7 @@ done:
> if (!status)
> priv->usage_count |= BIT(offset);
>
> - mutex_unlock(&gpio_lock);
> + mutex_unlock(&priv->mutex);
> return status;
> }
>
> @@ -279,64 +276,98 @@ static void twl_free(struct gpio_chip *chip, unsigned offset)
> {
> struct gpio_twl4030_priv *priv = to_gpio_twl4030(chip);
>
> + mutex_lock(&priv->mutex);
> if (offset >= TWL4030_GPIO_MAX) {
> twl4030_led_set_value(offset - TWL4030_GPIO_MAX, 1);
> return;
> }
>
> - mutex_lock(&gpio_lock);
> -
> priv->usage_count &= ~BIT(offset);
>
> /* on last use, switch off GPIO module */
> if (!priv->usage_count)
> gpio_twl4030_write(REG_GPIO_CTRL, 0x0);
>
> - mutex_unlock(&gpio_lock);
> + mutex_unlock(&priv->mutex);
> }
>
> static int twl_direction_in(struct gpio_chip *chip, unsigned offset)
> {
> - return (offset < TWL4030_GPIO_MAX)
> - ? twl4030_set_gpio_direction(offset, 1)
> - : -EINVAL;
> + struct gpio_twl4030_priv *priv = to_gpio_twl4030(chip);
> + int ret;
> +
> + mutex_lock(&priv->mutex);
> + if (offset < TWL4030_GPIO_MAX)
> + ret = twl4030_set_gpio_direction(offset, 1);
> + else
> + ret = -EINVAL;
> +
> + if (!ret)
> + priv->direction &= ~BIT(offset);
> +
> + mutex_unlock(&priv->mutex);
> +
> + return ret;
> }
>
> static int twl_get(struct gpio_chip *chip, unsigned offset)
> {
> struct gpio_twl4030_priv *priv = to_gpio_twl4030(chip);
> + int ret;
> int status = 0;
>
> - if (!(priv->usage_count & BIT(offset)))
> - return -EPERM;
> + mutex_lock(&priv->mutex);
> + if (!(priv->usage_count & BIT(offset))) {
> + ret = -EPERM;
> + goto out;
> + }
>
> - if (offset < TWL4030_GPIO_MAX)
> - status = twl4030_get_gpio_datain(offset);
> - else if (offset == TWL4030_GPIO_MAX)
> - status = cached_leden & LEDEN_LEDAON;
> + if (priv->direction & BIT(offset))
> + status = priv->out_state & BIT(offset);
> else
> - status = cached_leden & LEDEN_LEDBON;
> + status = twl4030_get_gpio_datain(offset);
>
> - return (status < 0) ? 0 : status;
> + ret = (status <= 0) ? 0 : 1;
> +out:
> + mutex_unlock(&priv->mutex);
> + dev_err(chip->dev, "%s: offset %d value %d\n", __func__, offset, ret);
> + return ret;
> }
>
> -static int twl_direction_out(struct gpio_chip *chip, unsigned offset, int value)
> +static void twl_set(struct gpio_chip *chip, unsigned offset, int value)
> {
> - if (offset < TWL4030_GPIO_MAX) {
> + struct gpio_twl4030_priv *priv = to_gpio_twl4030(chip);
> +
> + dev_err(chip->dev, "%s: offset %d value %d\n", __func__, offset, value);
> + mutex_lock(&priv->mutex);
> + if (offset < TWL4030_GPIO_MAX)
> twl4030_set_gpio_dataout(offset, value);
> - return twl4030_set_gpio_direction(offset, 0);
> - } else {
> + else
> twl4030_led_set_value(offset - TWL4030_GPIO_MAX, value);
> - return 0;
> - }
> +
> + if (value)
> + priv->out_state |= BIT(offset);
> + else
> + priv->out_state &= ~BIT(offset);
> +
> + mutex_unlock(&priv->mutex);
> }
>
> -static void twl_set(struct gpio_chip *chip, unsigned offset, int value)
> +static int twl_direction_out(struct gpio_chip *chip, unsigned offset, int value)
> {
> + struct gpio_twl4030_priv *priv = to_gpio_twl4030(chip);
> +
> + dev_err(chip->dev, "%s: offset %d value %d\n", __func__, offset, value);
> + mutex_lock(&priv->mutex);
> if (offset < TWL4030_GPIO_MAX)
> twl4030_set_gpio_dataout(offset, value);
> - else
> - twl4030_led_set_value(offset - TWL4030_GPIO_MAX, value);
> +
> + priv->direction |= BIT(offset);
> + mutex_unlock(&priv->mutex);
> +
> + twl_set(chip, offset, value);
> +
> + return 0;
> }
>
> static int twl_to_irq(struct gpio_chip *chip, unsigned offset)
> @@ -469,6 +500,8 @@ no_irqs:
> priv->gpio_chip.ngpio = TWL4030_GPIO_MAX;
> priv->gpio_chip.dev = &pdev->dev;
>
> + mutex_init(&priv->mutex);
> +
> if (node)
> pdata = of_gpio_twl4030(&pdev->dev);
>
> --
> 1.8.0
>
--
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.
--
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