* [PATCH 0/2] Staging: fbtft: Fix probing of gpio descriptor
@ 2019-07-11 8:31 Phil Reid
2019-07-11 8:31 ` [PATCH 1/2] " Phil Reid
` (3 more replies)
0 siblings, 4 replies; 7+ messages in thread
From: Phil Reid @ 2019-07-11 8:31 UTC (permalink / raw)
To: gregkh, bhanusreemahesh, leobras.c, nishadkamdar, preid,
dri-devel, linux-fbdev, devel, nsaenzjulienne
GPIO probing and reset polarity are broken.
Fix them.
Fixes: c440eee1a7a1 ("Staging: fbtft: Switch to the gpio descriptor interface")
Phil Reid (2):
Staging: fbtft: Fix probing of gpio descriptor
Staging: fbtft: Fix reset assertion when using gpio descriptor
drivers/staging/fbtft/fbtft-core.c | 43 ++++++++++++++++++--------------------
1 file changed, 20 insertions(+), 23 deletions(-)
--
1.8.3.1
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 1/2] Staging: fbtft: Fix probing of gpio descriptor 2019-07-11 8:31 [PATCH 0/2] Staging: fbtft: Fix probing of gpio descriptor Phil Reid @ 2019-07-11 8:31 ` Phil Reid 2019-07-11 8:31 ` [PATCH 2/2] Staging: fbtft: Fix reset assertion when using " Phil Reid ` (2 subsequent siblings) 3 siblings, 0 replies; 7+ messages in thread From: Phil Reid @ 2019-07-11 8:31 UTC (permalink / raw) To: gregkh, bhanusreemahesh, leobras.c, nishadkamdar, preid, dri-devel, linux-fbdev, devel, nsaenzjulienne Conversion to use gpio descriptors broke all gpio lookups as devm_gpiod_get_index was converted to use dev->driver->name for the gpio name lookup. Fix this by using the name param. In addition gpiod_get post-fixes the -gpios to the name so that shouldn't be included in the call. However this then breaks the of_find_property call to see if the gpio entry exists as all fbtft treats all gpios as optional. So use devm_gpiod_get_index_optional instead which achieves the same thing and is simpler. Nishad confirmed the changes where only ever compile tested. Fixes: c440eee1a7a1 ("Staging: fbtft: Switch to the gpio descriptor interface") Signed-off-by: Phil Reid <preid@electromag.com.au> --- drivers/staging/fbtft/fbtft-core.c | 39 ++++++++++++++++++-------------------- 1 file changed, 18 insertions(+), 21 deletions(-) diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c index 9b07bad..44b8074 100644 --- a/drivers/staging/fbtft/fbtft-core.c +++ b/drivers/staging/fbtft/fbtft-core.c @@ -76,21 +76,18 @@ static int fbtft_request_one_gpio(struct fbtft_par *par, struct gpio_desc **gpiop) { struct device *dev = par->info->device; - struct device_node *node = dev->of_node; int ret = 0; - if (of_find_property(node, name, NULL)) { - *gpiop = devm_gpiod_get_index(dev, dev->driver->name, index, - GPIOD_OUT_HIGH); - if (IS_ERR(*gpiop)) { - ret = PTR_ERR(*gpiop); - dev_err(dev, - "Failed to request %s GPIO:%d\n", name, ret); - return ret; - } - fbtft_par_dbg(DEBUG_REQUEST_GPIOS, par, "%s: '%s' GPIO\n", - __func__, name); + *gpiop = devm_gpiod_get_index_optional(dev, name, index, + GPIOD_OUT_HIGH); + if (IS_ERR(*gpiop)) { + ret = PTR_ERR(*gpiop); + dev_err(dev, + "Failed to request %s GPIO: %d\n", name, ret); + return ret; } + fbtft_par_dbg(DEBUG_REQUEST_GPIOS, par, "%s: '%s' GPIO\n", + __func__, name); return ret; } @@ -103,34 +100,34 @@ static int fbtft_request_gpios_dt(struct fbtft_par *par) if (!par->info->device->of_node) return -EINVAL; - ret = fbtft_request_one_gpio(par, "reset-gpios", 0, &par->gpio.reset); + ret = fbtft_request_one_gpio(par, "reset", 0, &par->gpio.reset); if (ret) return ret; - ret = fbtft_request_one_gpio(par, "dc-gpios", 0, &par->gpio.dc); + ret = fbtft_request_one_gpio(par, "dc", 0, &par->gpio.dc); if (ret) return ret; - ret = fbtft_request_one_gpio(par, "rd-gpios", 0, &par->gpio.rd); + ret = fbtft_request_one_gpio(par, "rd", 0, &par->gpio.rd); if (ret) return ret; - ret = fbtft_request_one_gpio(par, "wr-gpios", 0, &par->gpio.wr); + ret = fbtft_request_one_gpio(par, "wr", 0, &par->gpio.wr); if (ret) return ret; - ret = fbtft_request_one_gpio(par, "cs-gpios", 0, &par->gpio.cs); + ret = fbtft_request_one_gpio(par, "cs", 0, &par->gpio.cs); if (ret) return ret; - ret = fbtft_request_one_gpio(par, "latch-gpios", 0, &par->gpio.latch); + ret = fbtft_request_one_gpio(par, "latch", 0, &par->gpio.latch); if (ret) return ret; for (i = 0; i < 16; i++) { - ret = fbtft_request_one_gpio(par, "db-gpios", i, + ret = fbtft_request_one_gpio(par, "db", i, &par->gpio.db[i]); if (ret) return ret; - ret = fbtft_request_one_gpio(par, "led-gpios", i, + ret = fbtft_request_one_gpio(par, "led", i, &par->gpio.led[i]); if (ret) return ret; - ret = fbtft_request_one_gpio(par, "aux-gpios", i, + ret = fbtft_request_one_gpio(par, "aux", i, &par->gpio.aux[i]); if (ret) return ret; -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] Staging: fbtft: Fix reset assertion when using gpio descriptor 2019-07-11 8:31 [PATCH 0/2] Staging: fbtft: Fix probing of gpio descriptor Phil Reid 2019-07-11 8:31 ` [PATCH 1/2] " Phil Reid @ 2019-07-11 8:31 ` Phil Reid 2019-07-15 14:27 ` [PATCH 0/2] Staging: fbtft: Fix probing of " Nicolas Saenz Julienne 2019-07-16 0:24 ` Phil Reid 3 siblings, 0 replies; 7+ messages in thread From: Phil Reid @ 2019-07-11 8:31 UTC (permalink / raw) To: gregkh, bhanusreemahesh, leobras.c, nishadkamdar, preid, dri-devel, linux-fbdev, devel, nsaenzjulienne Typically gpiod_set_value calls would assert the reset line and then release it using the symantics of: gpiod_set_value(par->gpio.reset, 0); ... delay gpiod_set_value(par->gpio.reset, 1); And the gpio binding would specify the polarity. Prior to conversion to gpiod calls the polarity in the DT was ignored and assumed to be active low. Fix it so that DT polarity is respected. Signed-off-by: Phil Reid <preid@electromag.com.au> --- drivers/staging/fbtft/fbtft-core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c index 44b8074..bc75025 100644 --- a/drivers/staging/fbtft/fbtft-core.c +++ b/drivers/staging/fbtft/fbtft-core.c @@ -231,9 +231,9 @@ static void fbtft_reset(struct fbtft_par *par) if (!par->gpio.reset) return; fbtft_par_dbg(DEBUG_RESET, par, "%s()\n", __func__); - gpiod_set_value_cansleep(par->gpio.reset, 0); - usleep_range(20, 40); gpiod_set_value_cansleep(par->gpio.reset, 1); + usleep_range(20, 40); + gpiod_set_value_cansleep(par->gpio.reset, 0); msleep(120); } -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 0/2] Staging: fbtft: Fix probing of gpio descriptor 2019-07-11 8:31 [PATCH 0/2] Staging: fbtft: Fix probing of gpio descriptor Phil Reid 2019-07-11 8:31 ` [PATCH 1/2] " Phil Reid 2019-07-11 8:31 ` [PATCH 2/2] Staging: fbtft: Fix reset assertion when using " Phil Reid @ 2019-07-15 14:27 ` Nicolas Saenz Julienne 2019-07-16 0:24 ` Phil Reid 3 siblings, 0 replies; 7+ messages in thread From: Nicolas Saenz Julienne @ 2019-07-15 14:27 UTC (permalink / raw) To: Phil Reid, gregkh, bhanusreemahesh, leobras.c, nishadkamdar, dri-devel, linux-fbdev, devel [-- Attachment #1: Type: text/plain, Size: 827 bytes --] On Thu, 2019-07-11 at 16:31 +0800, Phil Reid wrote: > GPIO probing and reset polarity are broken. > Fix them. > > Fixes: c440eee1a7a1 ("Staging: fbtft: Switch to the gpio descriptor > interface") > > Phil Reid (2): > Staging: fbtft: Fix probing of gpio descriptor > Staging: fbtft: Fix reset assertion when using gpio descriptor > > drivers/staging/fbtft/fbtft-core.c | 43 ++++++++++++++++++------------------- > - > 1 file changed, 20 insertions(+), 23 deletions(-) > You can add my: Reviewed-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de> Tested-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de> The only issue I see is in the second patch, who should also have the same 'Fixes' tag. BTW, while testing I found another issue, I'll send a fix shortly. Kind regards, Nicolas [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 0/2] Staging: fbtft: Fix probing of gpio descriptor 2019-07-11 8:31 [PATCH 0/2] Staging: fbtft: Fix probing of gpio descriptor Phil Reid ` (2 preceding siblings ...) 2019-07-15 14:27 ` [PATCH 0/2] Staging: fbtft: Fix probing of " Nicolas Saenz Julienne @ 2019-07-16 0:24 ` Phil Reid 2019-07-16 0:24 ` [PATCH v2 1/2] " Phil Reid 2019-07-16 0:24 ` [PATCH v2 2/2] Staging: fbtft: Fix reset assertion when using " Phil Reid 3 siblings, 2 replies; 7+ messages in thread From: Phil Reid @ 2019-07-16 0:24 UTC (permalink / raw) To: gregkh, bhanusreemahesh, leobras.c, nsaenzjulienne, nishadkamdar, preid, dri-devel, linux-fbdev, devel GPIO probing and reset polarity are broken. Fix them. Fixes: c440eee1a7a1 ("Staging: fbtft: Switch to the gpio descriptor interface") Changes from v2: - Add fixes tag to "Fix reset assertion when using gpio descriptor" - Add tested-by / reviewed-by tags Phil Reid (2): Staging: fbtft: Fix probing of gpio descriptor Staging: fbtft: Fix reset assertion when using gpio descriptor drivers/staging/fbtft/fbtft-core.c | 43 ++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 23 deletions(-) -- 1.8.3.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 1/2] Staging: fbtft: Fix probing of gpio descriptor 2019-07-16 0:24 ` Phil Reid @ 2019-07-16 0:24 ` Phil Reid 2019-07-16 0:24 ` [PATCH v2 2/2] Staging: fbtft: Fix reset assertion when using " Phil Reid 1 sibling, 0 replies; 7+ messages in thread From: Phil Reid @ 2019-07-16 0:24 UTC (permalink / raw) To: gregkh, bhanusreemahesh, leobras.c, nsaenzjulienne, nishadkamdar, preid, dri-devel, linux-fbdev, devel Conversion to use gpio descriptors broke all gpio lookups as devm_gpiod_get_index was converted to use dev->driver->name for the gpio name lookup. Fix this by using the name param. In addition gpiod_get post-fixes the -gpios to the name so that shouldn't be included in the call. However this then breaks the of_find_property call to see if the gpio entry exists as all fbtft treats all gpios as optional. So use devm_gpiod_get_index_optional instead which achieves the same thing and is simpler. Nishad confirmed the changes where only ever compile tested. Fixes: c440eee1a7a1 ("Staging: fbtft: Switch to the gpio descriptor interface") Reviewed-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de> Tested-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de> Tested-by: Jan Sebastian Götte <linux@jaseg.net> Signed-off-by: Phil Reid <preid@electromag.com.au> --- drivers/staging/fbtft/fbtft-core.c | 39 ++++++++++++++++++-------------------- 1 file changed, 18 insertions(+), 21 deletions(-) diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c index 9b07bad..44b8074 100644 --- a/drivers/staging/fbtft/fbtft-core.c +++ b/drivers/staging/fbtft/fbtft-core.c @@ -76,21 +76,18 @@ static int fbtft_request_one_gpio(struct fbtft_par *par, struct gpio_desc **gpiop) { struct device *dev = par->info->device; - struct device_node *node = dev->of_node; int ret = 0; - if (of_find_property(node, name, NULL)) { - *gpiop = devm_gpiod_get_index(dev, dev->driver->name, index, - GPIOD_OUT_HIGH); - if (IS_ERR(*gpiop)) { - ret = PTR_ERR(*gpiop); - dev_err(dev, - "Failed to request %s GPIO:%d\n", name, ret); - return ret; - } - fbtft_par_dbg(DEBUG_REQUEST_GPIOS, par, "%s: '%s' GPIO\n", - __func__, name); + *gpiop = devm_gpiod_get_index_optional(dev, name, index, + GPIOD_OUT_HIGH); + if (IS_ERR(*gpiop)) { + ret = PTR_ERR(*gpiop); + dev_err(dev, + "Failed to request %s GPIO: %d\n", name, ret); + return ret; } + fbtft_par_dbg(DEBUG_REQUEST_GPIOS, par, "%s: '%s' GPIO\n", + __func__, name); return ret; } @@ -103,34 +100,34 @@ static int fbtft_request_gpios_dt(struct fbtft_par *par) if (!par->info->device->of_node) return -EINVAL; - ret = fbtft_request_one_gpio(par, "reset-gpios", 0, &par->gpio.reset); + ret = fbtft_request_one_gpio(par, "reset", 0, &par->gpio.reset); if (ret) return ret; - ret = fbtft_request_one_gpio(par, "dc-gpios", 0, &par->gpio.dc); + ret = fbtft_request_one_gpio(par, "dc", 0, &par->gpio.dc); if (ret) return ret; - ret = fbtft_request_one_gpio(par, "rd-gpios", 0, &par->gpio.rd); + ret = fbtft_request_one_gpio(par, "rd", 0, &par->gpio.rd); if (ret) return ret; - ret = fbtft_request_one_gpio(par, "wr-gpios", 0, &par->gpio.wr); + ret = fbtft_request_one_gpio(par, "wr", 0, &par->gpio.wr); if (ret) return ret; - ret = fbtft_request_one_gpio(par, "cs-gpios", 0, &par->gpio.cs); + ret = fbtft_request_one_gpio(par, "cs", 0, &par->gpio.cs); if (ret) return ret; - ret = fbtft_request_one_gpio(par, "latch-gpios", 0, &par->gpio.latch); + ret = fbtft_request_one_gpio(par, "latch", 0, &par->gpio.latch); if (ret) return ret; for (i = 0; i < 16; i++) { - ret = fbtft_request_one_gpio(par, "db-gpios", i, + ret = fbtft_request_one_gpio(par, "db", i, &par->gpio.db[i]); if (ret) return ret; - ret = fbtft_request_one_gpio(par, "led-gpios", i, + ret = fbtft_request_one_gpio(par, "led", i, &par->gpio.led[i]); if (ret) return ret; - ret = fbtft_request_one_gpio(par, "aux-gpios", i, + ret = fbtft_request_one_gpio(par, "aux", i, &par->gpio.aux[i]); if (ret) return ret; -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 2/2] Staging: fbtft: Fix reset assertion when using gpio descriptor 2019-07-16 0:24 ` Phil Reid 2019-07-16 0:24 ` [PATCH v2 1/2] " Phil Reid @ 2019-07-16 0:24 ` Phil Reid 1 sibling, 0 replies; 7+ messages in thread From: Phil Reid @ 2019-07-16 0:24 UTC (permalink / raw) To: gregkh, bhanusreemahesh, leobras.c, nsaenzjulienne, nishadkamdar, preid, dri-devel, linux-fbdev, devel Typically gpiod_set_value calls would assert the reset line and then release it using the symantics of: gpiod_set_value(par->gpio.reset, 0); ... delay gpiod_set_value(par->gpio.reset, 1); And the gpio binding would specify the polarity. Prior to conversion to gpiod calls the polarity in the DT was ignored and assumed to be active low. Fix it so that DT polarity is respected. Fixes: c440eee1a7a1 ("Staging: fbtft: Switch to the gpio descriptor interface") Reviewed-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de> Tested-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de> Tested-by: Jan Sebastian Götte <linux@jaseg.net> Signed-off-by: Phil Reid <preid@electromag.com.au> --- drivers/staging/fbtft/fbtft-core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c index 44b8074..bc75025 100644 --- a/drivers/staging/fbtft/fbtft-core.c +++ b/drivers/staging/fbtft/fbtft-core.c @@ -231,9 +231,9 @@ static void fbtft_reset(struct fbtft_par *par) if (!par->gpio.reset) return; fbtft_par_dbg(DEBUG_RESET, par, "%s()\n", __func__); - gpiod_set_value_cansleep(par->gpio.reset, 0); - usleep_range(20, 40); gpiod_set_value_cansleep(par->gpio.reset, 1); + usleep_range(20, 40); + gpiod_set_value_cansleep(par->gpio.reset, 0); msleep(120); } -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-07-16 0:24 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-07-11 8:31 [PATCH 0/2] Staging: fbtft: Fix probing of gpio descriptor Phil Reid 2019-07-11 8:31 ` [PATCH 1/2] " Phil Reid 2019-07-11 8:31 ` [PATCH 2/2] Staging: fbtft: Fix reset assertion when using " Phil Reid 2019-07-15 14:27 ` [PATCH 0/2] Staging: fbtft: Fix probing of " Nicolas Saenz Julienne 2019-07-16 0:24 ` Phil Reid 2019-07-16 0:24 ` [PATCH v2 1/2] " Phil Reid 2019-07-16 0:24 ` [PATCH v2 2/2] Staging: fbtft: Fix reset assertion when using " Phil Reid
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).