* [PATCH v3 0/2] gpio: ts4900: Do not set DAT and OE together @ 2022-03-10 1:16 Kris Bahnsen 2022-03-10 1:16 ` [PATCH v3 1/2] " Kris Bahnsen 2022-03-10 1:16 ` [PATCH v3 2/2] gpio: ts4900: Use SPDX header Kris Bahnsen 0 siblings, 2 replies; 10+ messages in thread From: Kris Bahnsen @ 2022-03-10 1:16 UTC (permalink / raw) To: Linus Walleij, Bartosz Golaszewski, linux-gpio, linux-kernel Cc: Mark Featherston, Kris Bahnsen Work around hardware race condition when setting DAT and OE in same transaction. Also clean up license boilerplate and use SPDX. I was not sure if it was preferred to make it a series or individual patches. I went with series because "gpio: ts4900: Use SPDX header" cannot cleanly apply by itself due to copyright year changes in the fix commit. If this is not preferred, please let me know. V3: - Move addition of SPDX identifier to separate commit - Remove license boilerplate in file V2: - Add Fixes tag Kris Bahnsen (1): gpio: ts4900: Use SPDX header Mark Featherston (1): gpio: ts4900: Do not set DAT and OE together drivers/gpio/gpio-ts4900.c | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-) -- 2.11.0 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 1/2] gpio: ts4900: Do not set DAT and OE together 2022-03-10 1:16 [PATCH v3 0/2] gpio: ts4900: Do not set DAT and OE together Kris Bahnsen @ 2022-03-10 1:16 ` Kris Bahnsen 2022-03-10 9:06 ` Bartosz Golaszewski 2022-03-10 14:48 ` Andy Shevchenko 2022-03-10 1:16 ` [PATCH v3 2/2] gpio: ts4900: Use SPDX header Kris Bahnsen 1 sibling, 2 replies; 10+ messages in thread From: Kris Bahnsen @ 2022-03-10 1:16 UTC (permalink / raw) To: Linus Walleij, Bartosz Golaszewski, linux-gpio, linux-kernel Cc: Mark Featherston, Kris Bahnsen From: Mark Featherston <mark@embeddedTS.com> This works around an issue with the hardware where both OE and DAT are exposed in the same register. If both are updated simultaneously, the harware makes no guarantees that OE or DAT will actually change in any given order and may result in a glitch of a few ns on a GPIO pin when changing direction and value in a single write. Setting direction to input now only affects OE bit. Setting direction to output updates DAT first, then OE. Fixes: 9c6686322d74 ("gpio: add Technologic I2C-FPGA gpio support") Signed-off-by: Mark Featherston <mark@embeddedTS.com> Signed-off-by: Kris Bahnsen <kris@embeddedTS.com> --- drivers/gpio/gpio-ts4900.c | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/drivers/gpio/gpio-ts4900.c b/drivers/gpio/gpio-ts4900.c index d885032cf814..d918d2df4de2 100644 --- a/drivers/gpio/gpio-ts4900.c +++ b/drivers/gpio/gpio-ts4900.c @@ -1,7 +1,7 @@ /* * Digital I/O driver for Technologic Systems I2C FPGA Core * - * Copyright (C) 2015 Technologic Systems + * Copyright (C) 2015, 2018 Technologic Systems * Copyright (C) 2016 Savoir-Faire Linux * * This program is free software; you can redistribute it and/or @@ -55,19 +55,33 @@ static int ts4900_gpio_direction_input(struct gpio_chip *chip, { struct ts4900_gpio_priv *priv = gpiochip_get_data(chip); - /* - * This will clear the output enable bit, the other bits are - * dontcare when this is cleared + /* Only clear the OE bit here, requires a RMW. Prevents potential issue + * with OE and data getting to the physical pin at different times. */ - return regmap_write(priv->regmap, offset, 0); + return regmap_update_bits(priv->regmap, offset, TS4900_GPIO_OE, 0); } static int ts4900_gpio_direction_output(struct gpio_chip *chip, unsigned int offset, int value) { struct ts4900_gpio_priv *priv = gpiochip_get_data(chip); + unsigned int reg; int ret; + /* If changing from an input to an output, we need to first set the + * proper data bit to what is requested and then set OE bit. This + * prevents a glitch that can occur on the IO line + */ + regmap_read(priv->regmap, offset, ®); + if (!(reg & TS4900_GPIO_OE)) { + if (value) + reg = TS4900_GPIO_OUT; + else + reg &= ~TS4900_GPIO_OUT; + + regmap_write(priv->regmap, offset, reg); + } + if (value) ret = regmap_write(priv->regmap, offset, TS4900_GPIO_OE | TS4900_GPIO_OUT); -- 2.11.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/2] gpio: ts4900: Do not set DAT and OE together 2022-03-10 1:16 ` [PATCH v3 1/2] " Kris Bahnsen @ 2022-03-10 9:06 ` Bartosz Golaszewski 2022-03-10 14:48 ` Andy Shevchenko 1 sibling, 0 replies; 10+ messages in thread From: Bartosz Golaszewski @ 2022-03-10 9:06 UTC (permalink / raw) To: Kris Bahnsen Cc: Linus Walleij, open list:GPIO SUBSYSTEM, Linux Kernel Mailing List, Mark Featherston On Thu, Mar 10, 2022 at 2:16 AM Kris Bahnsen <kris@embeddedts.com> wrote: > > From: Mark Featherston <mark@embeddedTS.com> > > This works around an issue with the hardware where both OE and > DAT are exposed in the same register. If both are updated > simultaneously, the harware makes no guarantees that OE or DAT > will actually change in any given order and may result in a > glitch of a few ns on a GPIO pin when changing direction and value > in a single write. > > Setting direction to input now only affects OE bit. Setting > direction to output updates DAT first, then OE. > > Fixes: 9c6686322d74 ("gpio: add Technologic I2C-FPGA gpio support") > > Signed-off-by: Mark Featherston <mark@embeddedTS.com> > Signed-off-by: Kris Bahnsen <kris@embeddedTS.com> > --- > drivers/gpio/gpio-ts4900.c | 24 +++++++++++++++++++----- > 1 file changed, 19 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpio/gpio-ts4900.c b/drivers/gpio/gpio-ts4900.c > index d885032cf814..d918d2df4de2 100644 > --- a/drivers/gpio/gpio-ts4900.c > +++ b/drivers/gpio/gpio-ts4900.c > @@ -1,7 +1,7 @@ > /* > * Digital I/O driver for Technologic Systems I2C FPGA Core > * > - * Copyright (C) 2015 Technologic Systems > + * Copyright (C) 2015, 2018 Technologic Systems > * Copyright (C) 2016 Savoir-Faire Linux > * > * This program is free software; you can redistribute it and/or > @@ -55,19 +55,33 @@ static int ts4900_gpio_direction_input(struct gpio_chip *chip, > { > struct ts4900_gpio_priv *priv = gpiochip_get_data(chip); > > - /* > - * This will clear the output enable bit, the other bits are > - * dontcare when this is cleared > + /* Only clear the OE bit here, requires a RMW. Prevents potential issue > + * with OE and data getting to the physical pin at different times. > */ > - return regmap_write(priv->regmap, offset, 0); > + return regmap_update_bits(priv->regmap, offset, TS4900_GPIO_OE, 0); > } > > static int ts4900_gpio_direction_output(struct gpio_chip *chip, > unsigned int offset, int value) > { > struct ts4900_gpio_priv *priv = gpiochip_get_data(chip); > + unsigned int reg; > int ret; > > + /* If changing from an input to an output, we need to first set the > + * proper data bit to what is requested and then set OE bit. This > + * prevents a glitch that can occur on the IO line > + */ > + regmap_read(priv->regmap, offset, ®); > + if (!(reg & TS4900_GPIO_OE)) { > + if (value) > + reg = TS4900_GPIO_OUT; > + else > + reg &= ~TS4900_GPIO_OUT; > + > + regmap_write(priv->regmap, offset, reg); > + } > + > if (value) > ret = regmap_write(priv->regmap, offset, TS4900_GPIO_OE | > TS4900_GPIO_OUT); > -- > 2.11.0 > Applied for fixes, thanks! Bart ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/2] gpio: ts4900: Do not set DAT and OE together 2022-03-10 1:16 ` [PATCH v3 1/2] " Kris Bahnsen 2022-03-10 9:06 ` Bartosz Golaszewski @ 2022-03-10 14:48 ` Andy Shevchenko 2022-03-10 17:36 ` Kris Bahnsen 1 sibling, 1 reply; 10+ messages in thread From: Andy Shevchenko @ 2022-03-10 14:48 UTC (permalink / raw) To: Kris Bahnsen Cc: Linus Walleij, Bartosz Golaszewski, open list:GPIO SUBSYSTEM, Linux Kernel Mailing List, Mark Featherston On Thu, Mar 10, 2022 at 2:22 PM Kris Bahnsen <kris@embeddedts.com> wrote: > > From: Mark Featherston <mark@embeddedTS.com> Same comments as per v2. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/2] gpio: ts4900: Do not set DAT and OE together 2022-03-10 14:48 ` Andy Shevchenko @ 2022-03-10 17:36 ` Kris Bahnsen 2022-03-10 19:30 ` Bartosz Golaszewski 0 siblings, 1 reply; 10+ messages in thread From: Kris Bahnsen @ 2022-03-10 17:36 UTC (permalink / raw) To: Andy Shevchenko Cc: Linus Walleij, Bartosz Golaszewski, open list:GPIO SUBSYSTEM, Linux Kernel Mailing List, Mark Featherston On Thu, 2022-03-10 at 16:48 +0200, Andy Shevchenko wrote: > On Thu, Mar 10, 2022 at 2:22 PM Kris Bahnsen <kris@embeddedts.com> wrote: > > > > From: Mark Featherston <mark@embeddedTS.com> > > Same comments as per v2. > Thanks, I'll get a v4 put together shortly to clean that up. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/2] gpio: ts4900: Do not set DAT and OE together 2022-03-10 17:36 ` Kris Bahnsen @ 2022-03-10 19:30 ` Bartosz Golaszewski 2022-03-10 19:47 ` Kris Bahnsen 0 siblings, 1 reply; 10+ messages in thread From: Bartosz Golaszewski @ 2022-03-10 19:30 UTC (permalink / raw) To: Kris Bahnsen Cc: Andy Shevchenko, Linus Walleij, open list:GPIO SUBSYSTEM, Linux Kernel Mailing List, Mark Featherston On Thu, Mar 10, 2022 at 6:36 PM Kris Bahnsen <kris@embeddedts.com> wrote: > > On Thu, 2022-03-10 at 16:48 +0200, Andy Shevchenko wrote: > > On Thu, Mar 10, 2022 at 2:22 PM Kris Bahnsen <kris@embeddedts.com> wrote: > > > > > > From: Mark Featherston <mark@embeddedTS.com> > > > > Same comments as per v2. > > > > Thanks, I'll get a v4 put together shortly to clean that up. Hey Kris, I already sent it out to Linus, please create a follow-up patch for that. Bart ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/2] gpio: ts4900: Do not set DAT and OE together 2022-03-10 19:30 ` Bartosz Golaszewski @ 2022-03-10 19:47 ` Kris Bahnsen 2022-03-11 11:25 ` Andy Shevchenko 0 siblings, 1 reply; 10+ messages in thread From: Kris Bahnsen @ 2022-03-10 19:47 UTC (permalink / raw) To: Bartosz Golaszewski Cc: Andy Shevchenko, Linus Walleij, open list:GPIO SUBSYSTEM, Linux Kernel Mailing List, Mark Featherston On Thu, 2022-03-10 at 20:30 +0100, Bartosz Golaszewski wrote: > On Thu, Mar 10, 2022 at 6:36 PM Kris Bahnsen <kris@embeddedts.com> wrote: > > > > On Thu, 2022-03-10 at 16:48 +0200, Andy Shevchenko wrote: > > > On Thu, Mar 10, 2022 at 2:22 PM Kris Bahnsen <kris@embeddedts.com> wrote: > > > > > > > > From: Mark Featherston <mark@embeddedTS.com> > > > > > > Same comments as per v2. > > > > > > > Thanks, I'll get a v4 put together shortly to clean that up. > > Hey Kris, > > I already sent it out to Linus, please create a follow-up patch for that. > > Bart Can you please clarify what that entails since Andy had requested changes to the commit message. Should I just create a new patch on top of the commit already on master to address the comment changes? How would I address commit message changes, or is that not my responsibility at this point? Thanks for all the help on learning this process thus far! Kris ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/2] gpio: ts4900: Do not set DAT and OE together 2022-03-10 19:47 ` Kris Bahnsen @ 2022-03-11 11:25 ` Andy Shevchenko 0 siblings, 0 replies; 10+ messages in thread From: Andy Shevchenko @ 2022-03-11 11:25 UTC (permalink / raw) To: Kris Bahnsen Cc: Bartosz Golaszewski, Linus Walleij, open list:GPIO SUBSYSTEM, Linux Kernel Mailing List, Mark Featherston On Thu, Mar 10, 2022 at 9:47 PM Kris Bahnsen <kris@embeddedts.com> wrote: > On Thu, 2022-03-10 at 20:30 +0100, Bartosz Golaszewski wrote: > > On Thu, Mar 10, 2022 at 6:36 PM Kris Bahnsen <kris@embeddedts.com> wrote: ... > > Hey Kris, > > > > I already sent it out to Linus, please create a follow-up patch for that. > Can you please clarify what that entails since Andy had requested changes to the > commit message. Should I just create a new patch on top of the commit already on > master to address the comment changes? Address the comments. > How would I address commit message changes, > or is that not my responsibility at this point? It's impossible. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 2/2] gpio: ts4900: Use SPDX header 2022-03-10 1:16 [PATCH v3 0/2] gpio: ts4900: Do not set DAT and OE together Kris Bahnsen 2022-03-10 1:16 ` [PATCH v3 1/2] " Kris Bahnsen @ 2022-03-10 1:16 ` Kris Bahnsen 2022-03-10 9:06 ` Bartosz Golaszewski 1 sibling, 1 reply; 10+ messages in thread From: Kris Bahnsen @ 2022-03-10 1:16 UTC (permalink / raw) To: Linus Walleij, Bartosz Golaszewski, linux-gpio, linux-kernel Cc: Mark Featherston, Kris Bahnsen Remove boilerplate, use the SPDX license identifier. Signed-off-by: Kris Bahnsen <kris@embeddedTS.com> --- drivers/gpio/gpio-ts4900.c | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/drivers/gpio/gpio-ts4900.c b/drivers/gpio/gpio-ts4900.c index d918d2df4de2..69854fd2382a 100644 --- a/drivers/gpio/gpio-ts4900.c +++ b/drivers/gpio/gpio-ts4900.c @@ -1,17 +1,9 @@ +// SPDX-License-Identifier: GPL-2.0 /* * Digital I/O driver for Technologic Systems I2C FPGA Core * * Copyright (C) 2015, 2018 Technologic Systems * Copyright (C) 2016 Savoir-Faire Linux - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU General Public License version 2 as - * published by the Free Software Foundation. - * - * This program is distributed "as is" WITHOUT ANY WARRANTY of any - * kind, whether expressed or implied; without even the implied warranty - * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License version 2 for more details. */ #include <linux/gpio/driver.h> -- 2.11.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/2] gpio: ts4900: Use SPDX header 2022-03-10 1:16 ` [PATCH v3 2/2] gpio: ts4900: Use SPDX header Kris Bahnsen @ 2022-03-10 9:06 ` Bartosz Golaszewski 0 siblings, 0 replies; 10+ messages in thread From: Bartosz Golaszewski @ 2022-03-10 9:06 UTC (permalink / raw) To: Kris Bahnsen Cc: Linus Walleij, open list:GPIO SUBSYSTEM, Linux Kernel Mailing List, Mark Featherston On Thu, Mar 10, 2022 at 2:16 AM Kris Bahnsen <kris@embeddedts.com> wrote: > > Remove boilerplate, use the SPDX license identifier. > > Signed-off-by: Kris Bahnsen <kris@embeddedTS.com> > --- > drivers/gpio/gpio-ts4900.c | 10 +--------- > 1 file changed, 1 insertion(+), 9 deletions(-) > > diff --git a/drivers/gpio/gpio-ts4900.c b/drivers/gpio/gpio-ts4900.c > index d918d2df4de2..69854fd2382a 100644 > --- a/drivers/gpio/gpio-ts4900.c > +++ b/drivers/gpio/gpio-ts4900.c > @@ -1,17 +1,9 @@ > +// SPDX-License-Identifier: GPL-2.0 > /* > * Digital I/O driver for Technologic Systems I2C FPGA Core > * > * Copyright (C) 2015, 2018 Technologic Systems > * Copyright (C) 2016 Savoir-Faire Linux > - * > - * This program is free software; you can redistribute it and/or > - * modify it under the terms of the GNU General Public License version 2 as > - * published by the Free Software Foundation. > - * > - * This program is distributed "as is" WITHOUT ANY WARRANTY of any > - * kind, whether expressed or implied; without even the implied warranty > - * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > - * GNU General Public License version 2 for more details. > */ > > #include <linux/gpio/driver.h> > -- > 2.11.0 > Applied, thanks! Bart ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-03-11 11:26 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-03-10 1:16 [PATCH v3 0/2] gpio: ts4900: Do not set DAT and OE together Kris Bahnsen 2022-03-10 1:16 ` [PATCH v3 1/2] " Kris Bahnsen 2022-03-10 9:06 ` Bartosz Golaszewski 2022-03-10 14:48 ` Andy Shevchenko 2022-03-10 17:36 ` Kris Bahnsen 2022-03-10 19:30 ` Bartosz Golaszewski 2022-03-10 19:47 ` Kris Bahnsen 2022-03-11 11:25 ` Andy Shevchenko 2022-03-10 1:16 ` [PATCH v3 2/2] gpio: ts4900: Use SPDX header Kris Bahnsen 2022-03-10 9:06 ` Bartosz Golaszewski
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.