Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1391083 > unrolled thread

[PATCH] gpio: tegra: Implement gpio_get_direction callback

Back to article view | Back to linux.kernel

Contents

  [PATCH] gpio: tegra: Implement gpio_get_direction callback Laxman Dewangan <ldewangan@nvidia.com> - 2016-04-29 14:50 +0200
    Re: [PATCH] gpio: tegra: Implement gpio_get_direction callback Jon Hunter <jonathanh@nvidia.com> - 2016-04-29 17:40 +0200
    Re: [PATCH] gpio: tegra: Implement gpio_get_direction callback Stephen Warren <swarren@wwwdotorg.org> - 2016-04-29 18:20 +0200

#1391083 — [PATCH] gpio: tegra: Implement gpio_get_direction callback

Implement gpio_get_direction() callback for Tegra GPIO.
The direction is only valid if the pin is configured as
GPIO. If pin is not configured in GPIO mode then this
function return error.

This makes debugfs and initial reading of the state of
the lines more accurate.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
This patch is based on discussion on series 
Re: [PATCH V5 0/4] gpio: tegra: Cleanups and support for debounce
From Linus W:
It would be nice if you also implement .get_direction() which makes
debugfs and initial reading of the state of the lines more accurate.

 drivers/gpio/gpio-tegra.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/gpio/gpio-tegra.c b/drivers/gpio/gpio-tegra.c
index b3ddd92..6f2292b 100644
--- a/drivers/gpio/gpio-tegra.c
+++ b/drivers/gpio/gpio-tegra.c
@@ -191,6 +191,24 @@ static int tegra_gpio_direction_output(struct gpio_chip *chip, unsigned offset,
 	return 0;
 }
 
+static int tegra_gpio_get_direction(struct gpio_chip *chip, unsigned offset)
+{
+	struct tegra_gpio_info *tgi = gpiochip_get_data(chip);
+	u32 pin_mask = BIT(GPIO_BIT(offset));
+	u32 cnf, oe;
+
+	cnf = tegra_gpio_readl(tgi, GPIO_CNF(tgi, offset));
+	oe = tegra_gpio_readl(tgi, GPIO_OE(tgi, offset));
+	if (cnf & pin_mask) {
+		if (oe & pin_mask)
+			return GPIOF_DIR_OUT;
+
+		return GPIOF_DIR_IN;
+	}
+
+	return -EINVAL;
+}
+
 static int tegra_gpio_set_debounce(struct gpio_chip *chip, unsigned int offset,
 				   unsigned int debounce)
 {
@@ -575,6 +593,7 @@ static int tegra_gpio_probe(struct platform_device *pdev)
 	tgi->gc.get			= tegra_gpio_get;
 	tgi->gc.direction_output	= tegra_gpio_direction_output;
 	tgi->gc.set			= tegra_gpio_set;
+	tgi->gc.get_direction		= tegra_gpio_get_direction;
 	tgi->gc.to_irq			= tegra_gpio_to_irq;
 	tgi->gc.base			= 0;
 	tgi->gc.ngpio			= tgi->bank_count * 32;
-- 
2.1.4

[toc] | [next] | [standalone]

#1391256

On 29/04/16 13:31, Laxman Dewangan wrote:
> Implement gpio_get_direction() callback for Tegra GPIO.
> The direction is only valid if the pin is configured as
> GPIO. If pin is not configured in GPIO mode then this
> function return error.
> 
> This makes debugfs and initial reading of the state of
> the lines more accurate.
> 
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
> ---
> This patch is based on discussion on series 
> Re: [PATCH V5 0/4] gpio: tegra: Cleanups and support for debounce
> From Linus W:
> It would be nice if you also implement .get_direction() which makes
> debugfs and initial reading of the state of the lines more accurate.
> 
>  drivers/gpio/gpio-tegra.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/drivers/gpio/gpio-tegra.c b/drivers/gpio/gpio-tegra.c
> index b3ddd92..6f2292b 100644
> --- a/drivers/gpio/gpio-tegra.c
> +++ b/drivers/gpio/gpio-tegra.c
> @@ -191,6 +191,24 @@ static int tegra_gpio_direction_output(struct gpio_chip *chip, unsigned offset,
>  	return 0;
>  }
>  
> +static int tegra_gpio_get_direction(struct gpio_chip *chip, unsigned offset)
> +{
> +	struct tegra_gpio_info *tgi = gpiochip_get_data(chip);
> +	u32 pin_mask = BIT(GPIO_BIT(offset));
> +	u32 cnf, oe;
> +
> +	cnf = tegra_gpio_readl(tgi, GPIO_CNF(tgi, offset));
> +	oe = tegra_gpio_readl(tgi, GPIO_OE(tgi, offset));
> +	if (cnf & pin_mask) {

Nit ... does not look like there is any point in reading GPIO_OE if cnf
& pin_mask == 0, so you could move the test before reading GPIO_OE and
then ...

> +		if (oe & pin_mask)
> +			return GPIOF_DIR_OUT;
> +
> +		return GPIOF_DIR_IN;

You could ...

	return (oe & mask) ? GPIOF_DIR_OUT : GPIOF_DIR_IN;

Cheers
Jon

[toc] | [prev] | [next] | [standalone]

#1391275

On 04/29/2016 06:31 AM, Laxman Dewangan wrote:
> Implement gpio_get_direction() callback for Tegra GPIO.
> The direction is only valid if the pin is configured as
> GPIO. If pin is not configured in GPIO mode then this
> function return error.
>
> This makes debugfs and initial reading of the state of
> the lines more accurate.

> +static int tegra_gpio_get_direction(struct gpio_chip *chip, unsigned offset)
> +{
> +	struct tegra_gpio_info *tgi = gpiochip_get_data(chip);
> +	u32 pin_mask = BIT(GPIO_BIT(offset));
> +	u32 cnf, oe;
> +
> +	cnf = tegra_gpio_readl(tgi, GPIO_CNF(tgi, offset));
> +	oe = tegra_gpio_readl(tgi, GPIO_OE(tgi, offset));
> +	if (cnf & pin_mask) {
> +		if (oe & pin_mask)
> +			return GPIOF_DIR_OUT;
> +
> +		return GPIOF_DIR_IN;
> +	}
> +
> +	return -EINVAL;
> +}

Conceptually looks fine, but I have similar comments to Jon; whenever 
there's an error condition, just fail immediately. That way you avoid 
the entire rest of the function being indented:

cnf = ...
if (!(cnf ...)
     return -EINVAL;
oe = ...
return ...

The only indented code there is the error handling. At least to me, this 
makes the code a lot more readable since there are far fewer 
combinations of conditionals.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web