[UrJTAG-dev] [PATCH] cable_gpio: bugfix - gpio tab fails for pin number 4

[Jan Willeke]

cable_gpio: bugfix - gpio cabel fails for pin number 4

GPIO_REQUIRED is 4, thus no jtag pin can be number 4 because 4 is invalid.

Signed-off-by: Jan Willeke <***@smartmote.de>

---
urjtag/src/tap/cable/gpio.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/urjtag/src/tap/cable/gpio.c b/urjtag/src/tap/cable/gpio.c
index 41117f9..0878e24 100644
--- a/urjtag/src/tap/cable/gpio.c
+++ b/urjtag/src/tap/cable/gpio.c
@@ -42,7 +42,7 @@
#define GPIO_PATH "/sys/class/gpio/"
#define GPIO_EXPORT_PATH GPIO_PATH "export"
#define GPIO_UNEXPORT_PATH GPIO_PATH "unexport"
-
+#define GPIO_UNSET -1
/* pin mapping */
enum {
GPIO_TDI = 0,
@@ -216,10 +216,10 @@ gpio_connect (urj_cable_t *cable, const urj_param_t *params[])
return URJ_STATUS_FAIL;
}

- cable_params->jtag_gpios[GPIO_TDI] = GPIO_REQUIRED;
- cable_params->jtag_gpios[GPIO_TDO] = GPIO_REQUIRED;
- cable_params->jtag_gpios[GPIO_TMS] = GPIO_REQUIRED;
- cable_params->jtag_gpios[GPIO_TCK] = GPIO_REQUIRED;
+ cable_params->jtag_gpios[GPIO_TDI] = GPIO_UNSET;
+ cable_params->jtag_gpios[GPIO_TDO] = GPIO_UNSET;
+ cable_params->jtag_gpios[GPIO_TMS] = GPIO_UNSET;
+ cable_params->jtag_gpios[GPIO_TCK] = GPIO_UNSET;
if (params != NULL)
/* parse arguments beyond the cable name */
for (i = 0; params[i] != NULL; i++)
@@ -253,7 +253,7 @@ gpio_connect (urj_cable_t *cable, const urj_param_t *params[])
*/

for (i = GPIO_TDI; i <= GPIO_TDO; i++)
- if (cable_params->jtag_gpios[i] == GPIO_REQUIRED)
+ if (cable_params->jtag_gpios[i] == GPIO_UNSET)
{
urj_error_set (URJ_ERROR_SYNTAX, _("missing required gpios\n"));
gpio_help (URJ_ERROR_SYNTAX, "gpio");

[Geert Stappers]

Post by Jan Willeke
cable_gpio: bugfix - gpio cabel fails for pin number 4
GPIO_REQUIRED is 4, thus no jtag pin can be number 4 because 4 is invalid.
---
urjtag/src/tap/cable/gpio.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/urjtag/src/tap/cable/gpio.c b/urjtag/src/tap/cable/gpio.c
index 41117f9..0878e24 100644
--- a/urjtag/src/tap/cable/gpio.c
+++ b/urjtag/src/tap/cable/gpio.c
@@ -42,7 +42,7 @@
#define GPIO_PATH "/sys/class/gpio/"
#define GPIO_EXPORT_PATH GPIO_PATH "export"
#define GPIO_UNEXPORT_PATH GPIO_PATH "unexport"
-
+#define GPIO_UNSET -1
/* pin mapping */
enum {
GPIO_TDI = 0,
@@ -216,10 +216,10 @@ gpio_connect (urj_cable_t *cable, const urj_param_t *params[])
return URJ_STATUS_FAIL;
}
- cable_params->jtag_gpios[GPIO_TDI] = GPIO_REQUIRED;
- cable_params->jtag_gpios[GPIO_TDO] = GPIO_REQUIRED;
- cable_params->jtag_gpios[GPIO_TMS] = GPIO_REQUIRED;
- cable_params->jtag_gpios[GPIO_TCK] = GPIO_REQUIRED;
+ cable_params->jtag_gpios[GPIO_TDI] = GPIO_UNSET;
+ cable_params->jtag_gpios[GPIO_TDO] = GPIO_UNSET;
+ cable_params->jtag_gpios[GPIO_TMS] = GPIO_UNSET;
+ cable_params->jtag_gpios[GPIO_TCK] = GPIO_UNSET;
if (params != NULL)
/* parse arguments beyond the cable name */
for (i = 0; params[i] != NULL; i++)
@@ -253,7 +253,7 @@ gpio_connect (urj_cable_t *cable, const urj_param_t *params[])
*/
for (i = GPIO_TDI; i <= GPIO_TDO; i++)
- if (cable_params->jtag_gpios[i] == GPIO_REQUIRED)
+ if (cable_params->jtag_gpios[i] == GPIO_UNSET)
{
urj_error_set (URJ_ERROR_SYNTAX, _("missing required gpios\n"));
gpio_help (URJ_ERROR_SYNTAX, "gpio");
--
2.1.4

I think that I understand what the change in the code does.

It is the compaion text on the code change that makes
me hesiate to apply the patch.

To be continued.


Groeten
Geert Stappers

[Geert Stappers]

Post by Geert Stappers

Post by Jan Willeke
cable_gpio: bugfix - gpio cabel fails for pin number 4
--- a/urjtag/src/tap/cable/gpio.c
+++ b/urjtag/src/tap/cable/gpio.c
@@ -216,10 +216,10 @@ gpio_connect (urj_cable_t *cable, const urj_param_t *params[])
return URJ_STATUS_FAIL;
}
- cable_params->jtag_gpios[GPIO_TDI] = GPIO_REQUIRED;
- cable_params->jtag_gpios[GPIO_TDO] = GPIO_REQUIRED;
- cable_params->jtag_gpios[GPIO_TMS] = GPIO_REQUIRED;
- cable_params->jtag_gpios[GPIO_TCK] = GPIO_REQUIRED;
+ cable_params->jtag_gpios[GPIO_TDI] = GPIO_UNSET;
+ cable_params->jtag_gpios[GPIO_TDO] = GPIO_UNSET;
+ cable_params->jtag_gpios[GPIO_TMS] = GPIO_UNSET;
+ cable_params->jtag_gpios[GPIO_TCK] = GPIO_UNSET;
if (params != NULL)
/* parse arguments beyond the cable name */
for (i = 0; params[i] != NULL; i++)
@@ -253,7 +253,7 @@ gpio_connect (urj_cable_t *cable, const urj_param_t *params[])
*/
for (i = GPIO_TDI; i <= GPIO_TDO; i++)
- if (cable_params->jtag_gpios[i] == GPIO_REQUIRED)
+ if (cable_params->jtag_gpios[i] == GPIO_UNSET)
{
urj_error_set (URJ_ERROR_SYNTAX, _("missing required gpios\n"));
gpio_help (URJ_ERROR_SYNTAX, "gpio");
--
2.1.4

I think that I understand what the change in the code does.
It is the compaion text on the code change that makes
me hesiate to apply the patch.
To be continued.

Rewrite proposal of companion text:


enumerated GPIO_REQUIRED blocks using pin 4

GPIO_REQUIRED was used for two things: "last" and "unset"
Last pin can be a valid pin.
"unset" should be a separate value.


sign of by Jan Willeke so he gets the credits

Post by Geert Stappers

Post by Jan Willeke
@@ -42,7 +42,7 @@
#define GPIO_PATH "/sys/class/gpio/"
#define GPIO_EXPORT_PATH GPIO_PATH "export"
#define GPIO_UNEXPORT_PATH GPIO_PATH "unexport"
-
+#define GPIO_UNSET -1
/* pin mapping */
enum {
GPIO_TDI = 0,

@@ -45,6 +45,7 @@

/* pin mapping */
enum {
+ GPIO_UNSET = -1,
GPIO_TDI = 0,
GPIO_TCK,
GPIO_TMS,



Groeten
Geert Stappers

[Jan Willeke]

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

[Geert Stappers]

Post by Geert Stappers

Post by Geert Stappers

[ ... patch from Jan Willeke ... ]

I think that I understand what the change in the code does.
It is the compaion text on the code change that makes
me hesiate to apply the patch.
To be continued.

enumerated GPIO_REQUIRED blocks using pin 4
GPIO_REQUIRED was used for two things: "last" and "unset"
Last pin can be a valid pin.
"unset" should be a separate value.
sign of by Jan Willeke so he gets the credits
@@ -45,6 +45,7 @@
/* pin mapping */
enum {
* GPIO_UNSET = -1,
GPIO_TDI = 0,
GPIO_TCK,
GPIO_TMS,

Hi Geert,
Thanks for rewriting the companion text and putting the -1 into the enum.
I fully agree with your changes.

I did read that as: Mix it and apply it to our git repo

So, I did :-)


Groeten
Geert Stappers