| Message ID | 20190219204843.20352-1-linus.walleij@linaro.org |
|---|---|
| State | Accepted |
| Commit | 11da905412833d9b369a6a09a401f87149d674dc |
| Headers | show |
| Series | gpio: tegra: Fix offset of pinctrl calls | expand |
19.02.2019 23:48, Linus Walleij пишет: > This patch hunk is a lightly modified version of a diff found > in a Tegra code dump from a product tree. It makes a lot of > sense because this is what most drivers do. > > Cc: Thierry Reding <treding@nvidia.com> > Cc: Stefan Agner <stefan@agner.ch> > Cc: Dmitry Osipenko <digetx@gmail.com> > Cc: Jon Hunter <jonathanh@nvidia.com> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > --- > drivers/gpio/gpio-tegra.c | 25 +++++++++++++++++++++---- > 1 file changed, 21 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpio/gpio-tegra.c b/drivers/gpio/gpio-tegra.c > index 02f6db925fd5..1ececf2c3282 100644 > --- a/drivers/gpio/gpio-tegra.c > +++ b/drivers/gpio/gpio-tegra.c > @@ -2,6 +2,7 @@ > * arch/arm/mach-tegra/gpio.c > * > * Copyright (c) 2010 Google, Inc > + * Copyright (c) 2011-2016, NVIDIA CORPORATION. All rights reserved. > * > * Author: > * Erik Gilling <konkers@google.com> > @@ -141,14 +142,14 @@ static void tegra_gpio_disable(struct tegra_gpio_info *tgi, unsigned int gpio) > > static int tegra_gpio_request(struct gpio_chip *chip, unsigned int offset) > { > - return pinctrl_gpio_request(offset); > + return pinctrl_gpio_request(chip->base + offset); > } > > static void tegra_gpio_free(struct gpio_chip *chip, unsigned int offset) > { > struct tegra_gpio_info *tgi = gpiochip_get_data(chip); > > - pinctrl_gpio_free(offset); > + pinctrl_gpio_free(chip->base + offset); > tegra_gpio_disable(tgi, offset); > } > > @@ -176,10 +177,18 @@ static int tegra_gpio_direction_input(struct gpio_chip *chip, > unsigned int offset) > { > struct tegra_gpio_info *tgi = gpiochip_get_data(chip); > + int ret; > > tegra_gpio_mask_write(tgi, GPIO_MSK_OE(tgi, offset), offset, 0); > tegra_gpio_enable(tgi, offset); > - return 0; > + > + ret = pinctrl_gpio_direction_input(chip->base + offset); > + if (ret < 0) > + dev_err(tgi->dev, > + "Failed to set pinctrl input direction of GPIO %d: %d", > + chip->base + offset, ret); > + > + return ret; > } > > static int tegra_gpio_direction_output(struct gpio_chip *chip, > @@ -187,11 +196,19 @@ static int tegra_gpio_direction_output(struct gpio_chip *chip, > int value) > { > struct tegra_gpio_info *tgi = gpiochip_get_data(chip); > + int ret; > > tegra_gpio_set(chip, offset, value); > tegra_gpio_mask_write(tgi, GPIO_MSK_OE(tgi, offset), offset, 1); > tegra_gpio_enable(tgi, offset); > - return 0; > + > + ret = pinctrl_gpio_direction_output(chip->base + offset); > + if (ret < 0) > + dev_err(tgi->dev, > + "Failed to set pinctrl output direction of GPIO %d: %d", > + chip->base + offset, ret); > + > + return ret; > } > > static int tegra_gpio_get_direction(struct gpio_chip *chip, > This is formally a correct patch, but none of Tegra's upstream pinctrl drivers implement gpio_set_direction() and chip->base=0. Hence I'm not sure whether this all is really necessary.. or maybe you're going to implement the gpio_set_direction()?
On Tue, Feb 19, 2019 at 10:24 PM Dmitry Osipenko <digetx@gmail.com> wrote: > This is formally a correct patch, but none of Tegra's upstream pinctrl drivers implement > gpio_set_direction() and chip->base=0. Hence I'm not sure whether this all is really > necessary.. The API wants a valid global GPIO number, the current code is confusing since it sends random numbers (offsets) instead. > or maybe you're going to implement the gpio_set_direction()? No but since nVidia has fixed it in their outoftree codebase and it is formally correct I don't see why we shouldn't just fix it. It may confuse someone. Yours, Linus Walleij
21.02.2019 0:50, Linus Walleij пишет: > On Tue, Feb 19, 2019 at 10:24 PM Dmitry Osipenko <digetx@gmail.com> wrote: > >> This is formally a correct patch, but none of Tegra's upstream pinctrl drivers implement >> gpio_set_direction() and chip->base=0. Hence I'm not sure whether this all is really >> necessary.. > > The API wants a valid global GPIO number, the current code is confusing since it > sends random numbers (offsets) instead. > >> or maybe you're going to implement the gpio_set_direction()? > > No but since nVidia has fixed it in their outoftree codebase and it is formally > correct I don't see why we shouldn't just fix it. It may confuse someone. Well, downstream probably has a real use-case for a non-static PINCTRL-GPIO configuration. I don't really mind much if this makes easier for you to follow the code, maybe it will become really handy later on for upstream too. Reviewed-by: Dmitry Osipenko <digetx@gmail.com
diff --git a/drivers/gpio/gpio-tegra.c b/drivers/gpio/gpio-tegra.c index 02f6db925fd5..1ececf2c3282 100644 --- a/drivers/gpio/gpio-tegra.c +++ b/drivers/gpio/gpio-tegra.c @@ -2,6 +2,7 @@ * arch/arm/mach-tegra/gpio.c * * Copyright (c) 2010 Google, Inc + * Copyright (c) 2011-2016, NVIDIA CORPORATION. All rights reserved. * * Author: * Erik Gilling <konkers@google.com> @@ -141,14 +142,14 @@ static void tegra_gpio_disable(struct tegra_gpio_info *tgi, unsigned int gpio) static int tegra_gpio_request(struct gpio_chip *chip, unsigned int offset) { - return pinctrl_gpio_request(offset); + return pinctrl_gpio_request(chip->base + offset); } static void tegra_gpio_free(struct gpio_chip *chip, unsigned int offset) { struct tegra_gpio_info *tgi = gpiochip_get_data(chip); - pinctrl_gpio_free(offset); + pinctrl_gpio_free(chip->base + offset); tegra_gpio_disable(tgi, offset); } @@ -176,10 +177,18 @@ static int tegra_gpio_direction_input(struct gpio_chip *chip, unsigned int offset) { struct tegra_gpio_info *tgi = gpiochip_get_data(chip); + int ret; tegra_gpio_mask_write(tgi, GPIO_MSK_OE(tgi, offset), offset, 0); tegra_gpio_enable(tgi, offset); - return 0; + + ret = pinctrl_gpio_direction_input(chip->base + offset); + if (ret < 0) + dev_err(tgi->dev, + "Failed to set pinctrl input direction of GPIO %d: %d", + chip->base + offset, ret); + + return ret; } static int tegra_gpio_direction_output(struct gpio_chip *chip, @@ -187,11 +196,19 @@ static int tegra_gpio_direction_output(struct gpio_chip *chip, int value) { struct tegra_gpio_info *tgi = gpiochip_get_data(chip); + int ret; tegra_gpio_set(chip, offset, value); tegra_gpio_mask_write(tgi, GPIO_MSK_OE(tgi, offset), offset, 1); tegra_gpio_enable(tgi, offset); - return 0; + + ret = pinctrl_gpio_direction_output(chip->base + offset); + if (ret < 0) + dev_err(tgi->dev, + "Failed to set pinctrl output direction of GPIO %d: %d", + chip->base + offset, ret); + + return ret; } static int tegra_gpio_get_direction(struct gpio_chip *chip,
This patch hunk is a lightly modified version of a diff found in a Tegra code dump from a product tree. It makes a lot of sense because this is what most drivers do. Cc: Thierry Reding <treding@nvidia.com> Cc: Stefan Agner <stefan@agner.ch> Cc: Dmitry Osipenko <digetx@gmail.com> Cc: Jon Hunter <jonathanh@nvidia.com> Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- drivers/gpio/gpio-tegra.c | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) -- 2.20.1