Re: [U-Boot] [PATCH v2 2/3] env: Tidy up some of the env code
On 27/11/2019 05:52, AKASHI Takahiro wrote: On Thu, Nov 21, 2019 at 02:32:47PM +, James Byrne wrote: This commit tidies up a few things in the env code to make it safer and easier to extend: - The hsearch_r() function took a 'struct env_entry' as its first parameter, but only used the 'key' and 'data' fields. Some callers would set the other fields, others would leave them undefined. Another disadvantage was that the struct 'data' member is a 'char *', but this function does not modify it, so it should be 'const char *'. To resolve these issues the function now takes explcit 'key' and 'data' parameters that are 'const char *', and all the callers have been modified. I don't have a strong opinion here, but we'd rather maintain the current interface. Yes, currently no users use any fields other than key/data, but in the future, this function may be extended to accept additional *search* parameters in a key, say flag?. At that time, we won't have to change the interface again. As I said in my commit log comment, there are two key arguments against this: - The fact that the 'data' member of 'struct env_entry' is a 'char *' is really inconvenient because this is a read-only function where most of the callers should be using 'const char *' pointers, and having to cast away the constness isn't good practice and makes the calling code less readable. - As you can see from the calling code I've had to tidy up, the callers were very inconsistent about whether they bothered to initialise any fields other than 'key' and 'value', so if you ever wanted to extend the interface to check other parameters you'd have to go around and fix them all up anyway to avoid unpredictable behaviour. Given that only 'key' and 'value' are used at the moment I think my change is preferable because it makes it explicit what is being used and avoids any nasty surprises you might get if you changed hsearch_r() without changing all the callers. If you anticipate wanting to match on other fields, it might be better to define an alternative query structure using 'const char *' pointers for key and value, then extend that, but I would argue that that's something you could do at the point you find it is needed rather than now. Regards, James ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v3] gpio: at91_gpio: Add bank names
Make the at91_gpio driver set sensible GPIO bank names in the platform
data. This makes the 'gpio status' command a lot more useful.
Signed-off-by: James Byrne
---
Changes in v3:
- Move at91_get_bank_name() into #ifdef CONFIG_DM_GPIO section.
Changes in v2:
- Use "undefined" for an unknown bank name.
drivers/gpio/at91_gpio.c | 29 ++---
1 file changed, 26 insertions(+), 3 deletions(-)
diff --git a/drivers/gpio/at91_gpio.c b/drivers/gpio/at91_gpio.c
index 965becf77a..dbfed72c61 100644
--- a/drivers/gpio/at91_gpio.c
+++ b/drivers/gpio/at91_gpio.c
@@ -556,6 +556,28 @@ static int at91_gpio_get_function(struct udevice *dev,
unsigned offset)
return GPIOF_INPUT;
}
+static const char *at91_get_bank_name(uint32_t base_addr)
+{
+ switch (base_addr) {
+ case ATMEL_BASE_PIOA:
+ return "PIOA";
+ case ATMEL_BASE_PIOB:
+ return "PIOB";
+ case ATMEL_BASE_PIOC:
+ return "PIOC";
+#if (ATMEL_PIO_PORTS > 3)
+ case ATMEL_BASE_PIOD:
+ return "PIOD";
+#if (ATMEL_PIO_PORTS > 4)
+ case ATMEL_BASE_PIOE:
+ return "PIOE";
+#endif
+#endif
+ }
+
+ return "undefined";
+}
+
static const struct dm_gpio_ops gpio_at91_ops = {
.direction_input= at91_gpio_direction_input,
.direction_output = at91_gpio_direction_output,
@@ -582,14 +604,15 @@ static int at91_gpio_probe(struct udevice *dev)
clk_free();
- uc_priv->bank_name = plat->bank_name;
- uc_priv->gpio_count = GPIO_PER_BANK;
-
#if CONFIG_IS_ENABLED(OF_CONTROL)
plat->base_addr = (uint32_t)devfdt_get_addr_ptr(dev);
#endif
+ plat->bank_name = at91_get_bank_name(plat->base_addr);
port->regs = (struct at91_port *)plat->base_addr;
+ uc_priv->bank_name = plat->bank_name;
+ uc_priv->gpio_count = GPIO_PER_BANK;
+
return 0;
}
--
2.24.0
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v2 3/3] env: Provide programmatic equivalent to 'setenv -f'
Add env_force_set() to provide an equivalent to 'setenv -f' that can be
used programmatically.
Signed-off-by: James Byrne
---
Changes in v2: None
cmd/nvedit.c | 17 ++---
include/env.h | 13 +
2 files changed, 27 insertions(+), 3 deletions(-)
diff --git a/cmd/nvedit.c b/cmd/nvedit.c
index b30669a45e..106c69147b 100644
--- a/cmd/nvedit.c
+++ b/cmd/nvedit.c
@@ -241,16 +241,27 @@ static int do_env_update(const char *name, const char
*value, int env_flag)
return 0;
}
-int env_set(const char *varname, const char *varvalue)
+static int do_programmatic_env_set(const char *varname, const char *varvalue,
+ int env_flag)
{
/* before import into hashtable */
if (!(gd->flags & GD_FLG_ENV_READY))
return 1;
if (!varvalue || varvalue[0] == '\0')
- return do_env_remove(varname, H_PROGRAMMATIC);
+ return do_env_remove(varname, H_PROGRAMMATIC | env_flag);
+
+ return do_env_update(varname, varvalue, H_PROGRAMMATIC | env_flag);
+}
+
+int env_set(const char *varname, const char *varvalue)
+{
+ return do_programmatic_env_set(varname, varvalue, 0);
+}
- return do_env_update(varname, varvalue, H_PROGRAMMATIC);
+int env_force_set(const char *varname, const char *varvalue)
+{
+ return do_programmatic_env_set(varname, varvalue, H_FORCE);
}
#ifndef CONFIG_SPL_BUILD
diff --git a/include/env.h b/include/env.h
index b72239f6a5..da54f51805 100644
--- a/include/env.h
+++ b/include/env.h
@@ -145,6 +145,19 @@ int env_get_yesno(const char *var);
*/
int env_set(const char *varname, const char *value);
+/**
+ * env_force_set() - forcibly set an environment variable
+ *
+ * This sets or deletes the value of an environment variable. It is the same
+ * as env_set(), except that the variable will be forcibly updated/deleted,
+ * even if it has access protection flags set.
+ *
+ * @varname: Variable to adjust
+ * @value: Value to set for the variable, or NULL or "" to delete the variable
+ * @return 0 if OK, 1 on error
+ */
+int env_force_set(const char *varname, const char *varvalue);
+
/**
* env_get_ulong() - Return an environment variable as an integer value
*
--
2.24.0
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v2 1/3] tools: checkpatch: Restore 'debug' and 'printf' to logFunctions list
The 'debug' and 'printf' functions were previously added to the list of
logFunctions in commit 0cab42110dbf ("checkpatch.pl: Add 'debug' to
the list of logFunctions") and commit 397bfd4642c1 ("checkpatch.pl:
Add 'printf' to logFunctions") but these additions were lost when newer
versions of checkpatch were pulled in from the upstream Linux
kernel version.
This restores them so that you don't end up in a situation where
checkpatch will give a warning for "quoted string split across lines"
which you cannot fix without getting a warning for "line over 80
characters" instead.
Signed-off-by: James Byrne
---
Changes in v2: None
scripts/checkpatch.pl | 2 ++
1 file changed, 2 insertions(+)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 6fcc66afb0..c2641bc995 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -464,6 +464,8 @@ our $logFunctions = qr{(?x:
TP_printk|
WARN(?:_RATELIMIT|_ONCE|)|
panic|
+ debug|
+ printf|
MODULE_[A-Z_]+|
seq_vprintf|seq_printf|seq_puts
)};
--
2.24.0
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v2 2/3] env: Tidy up some of the env code
This commit tidies up a few things in the env code to make it safer and
easier to extend:
- The hsearch_r() function took a 'struct env_entry' as its first
parameter, but only used the 'key' and 'data' fields. Some callers would
set the other fields, others would leave them undefined. Another
disadvantage was that the struct 'data' member is a 'char *', but this
function does not modify it, so it should be 'const char *'. To resolve
these issues the function now takes explcit 'key' and 'data' parameters
that are 'const char *', and all the callers have been modified.
- Break up _do_env_set() so that it only does the argument handling,
rename it to do_interactive_env_set() and use 'const char *' pointers
for argv. The actual variable modification has been split out to two
separate functions, do_env_remove() and do_env_update(), which can also
be called from the programmatic version env_set(), meaning it no longer
has to create fake command line parameters. The do_interactive_env_set()
function is not required in SPL builds.
- Fix some warnings identified by checkpatch.pl
Signed-off-by: James Byrne
---
Changes in v2:
- Fix checkpatch.pl so that this patchset can pass without warnings.
- Tidy up the underlying code before adding env_force_set()
- Rename new function from env_force() to env_force_set()
api/api.c | 5 +-
cmd/nvedit.c | 111 +++---
drivers/tee/sandbox.c | 17 +++
env/callback.c| 7 +--
env/flags.c | 7 +--
include/search.h | 2 +-
lib/hashtable.c | 83 ---
test/env/hashtable.c | 23 ++---
8 files changed, 119 insertions(+), 136 deletions(-)
diff --git a/api/api.c b/api/api.c
index 71fa03804e..b950d8cbb7 100644
--- a/api/api.c
+++ b/api/api.c
@@ -514,7 +514,7 @@ static int API_env_enum(va_list ap)
{
int i, buflen;
char *last, **next, *s;
- struct env_entry *match, search;
+ struct env_entry *match;
static char *var;
last = (char *)va_arg(ap, unsigned long);
@@ -530,8 +530,7 @@ static int API_env_enum(va_list ap)
s = strchr(var, '=');
if (s != NULL)
*s = 0;
- search.key = var;
- i = hsearch_r(search, ENV_FIND, , _htab, 0);
+ i = hsearch_r(var, NULL, ENV_FIND, , _htab, 0);
if (i == 0) {
i = API_EINVAL;
goto done;
diff --git a/cmd/nvedit.c b/cmd/nvedit.c
index 99a3bc57b1..b30669a45e 100644
--- a/cmd/nvedit.c
+++ b/cmd/nvedit.c
@@ -94,11 +94,9 @@ static int env_print(char *name, int flag)
ssize_t len;
if (name) { /* print a single name */
- struct env_entry e, *ep;
+ struct env_entry *ep;
- e.key = name;
- e.data = NULL;
- hsearch_r(e, ENV_FIND, , _htab, flag);
+ hsearch_r(name, NULL, ENV_FIND, , _htab, flag);
if (ep == NULL)
return 0;
len = printf("%s=%s\n", ep->key, ep->data);
@@ -217,15 +215,55 @@ DONE:
#endif
#endif /* CONFIG_SPL_BUILD */
+static int do_env_remove(const char *name, int env_flag)
+{
+ int rc;
+
+ env_id++;
+
+ rc = hdelete_r(name, _htab, env_flag);
+ return !rc;
+}
+
+static int do_env_update(const char *name, const char *value, int env_flag)
+{
+ struct env_entry *ep;
+
+ env_id++;
+
+ hsearch_r(name, value, ENV_ENTER, , _htab, env_flag);
+ if (!ep) {
+ printf("## Error inserting \"%s\" variable, errno=%d\n",
+ name, errno);
+ return 1;
+ }
+
+ return 0;
+}
+
+int env_set(const char *varname, const char *varvalue)
+{
+ /* before import into hashtable */
+ if (!(gd->flags & GD_FLG_ENV_READY))
+ return 1;
+
+ if (!varvalue || varvalue[0] == '\0')
+ return do_env_remove(varname, H_PROGRAMMATIC);
+
+ return do_env_update(varname, varvalue, H_PROGRAMMATIC);
+}
+
+#ifndef CONFIG_SPL_BUILD
/*
* Set a new environment variable,
* or replace or delete an existing one.
*/
-static int _do_env_set(int flag, int argc, char * const argv[], int env_flag)
+static int do_interactive_env_set(int flag, int argc, const char * const
argv[])
{
- int i, len;
- char *name, *value, *s;
- struct env_entry e, *ep;
+ int env_flag = H_INTERACTIVE;
+ int i, len, rc;
+ const char *name;
+ char *value, *s;
debug("Initial value for argc=%d\n", argc);
@@ -235,7 +273,7 @@ static int _do_env_set(int flag, int argc, char * const
argv[], int env_flag)
#endif
while (argc > 1 && **(argv + 1) == '-') {
- char *arg = *++argv;
+ const char *arg = *++argv;
--ar
Re: [U-Boot] [PATCH] env: Provide programmatic equivalent to 'setenv -f'
On 19/11/2019 21:01, Simon Goldschmidt wrote:
Heinrich Schuchardt mailto:xypron.g...@gmx.de>>
schrieb am Di., 19. Nov. 2019, 21:56:
On 11/19/19 9:30 PM, Simon Goldschmidt wrote:
> Am 19.11.2019 um 18:31 schrieb James Byrne:
>> Add env_force() to provide an equivalent to 'setenv -f' that can
be used
>> programmatically.
>>
>> Also tighten up the definition of argv in _do_env_set() so that
>> 'const char *' pointers are used.
>>
>> Signed-off-by: James Byrne mailto:james.by...@origamienergy.com>>
>
> OK, I'm on CC, so I'll give my two cent:
>
> I always thought this code to be messed up a bit: I think it's better
> programming style to have the "string argument parsing" code call
real C
> functions with typed arguments. The env code instead does it the
other
> way round (here, you add do_programmatic_env_set() that sets up an
> argv[] array to call another function).
>
> I'm not a maintainer for the ENV code, but maybe that should be
sorted
> out instead of adding yet more code that goes this way?
There is no maintainer for the ENV code. Simon makes a valid point here.
By creating a function for setting variables and separating it from
parsing arguments you get the function you need for forcing the value of
a variable for free.
Right. I thought someone had volunteered but a look at the maintainers
file proves me wrong.
In any way, I'd be more open to review a cleanup patch than a patch
continuing this messy code flow.
Having looked at it again, I agree. I have now redone it, but I have
ended up changing quite a lot more of the underlying code. I will
resubmit a revised patch (probably tomorrow) in two parts, one to apply
some tidying up to the env code, and one to add the new function. It
will be a much bigger patch set though!
James
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH] env: Provide programmatic equivalent to 'setenv -f'
Add env_force() to provide an equivalent to 'setenv -f' that can be used
programmatically.
Also tighten up the definition of argv in _do_env_set() so that
'const char *' pointers are used.
Signed-off-by: James Byrne
---
cmd/nvedit.c | 43 +--
include/env.h | 13 +
2 files changed, 42 insertions(+), 14 deletions(-)
diff --git a/cmd/nvedit.c b/cmd/nvedit.c
index 99a3bc57b1..1f363ba9f4 100644
--- a/cmd/nvedit.c
+++ b/cmd/nvedit.c
@@ -221,10 +221,12 @@ DONE:
* Set a new environment variable,
* or replace or delete an existing one.
*/
-static int _do_env_set(int flag, int argc, char * const argv[], int env_flag)
+static int _do_env_set(int flag, int argc, const char * const argv[],
+ int env_flag)
{
int i, len;
- char *name, *value, *s;
+ const char *name;
+ char *value, *s;
struct env_entry e, *ep;
debug("Initial value for argc=%d\n", argc);
@@ -235,7 +237,7 @@ static int _do_env_set(int flag, int argc, char * const
argv[], int env_flag)
#endif
while (argc > 1 && **(argv + 1) == '-') {
- char *arg = *++argv;
+ const char *arg = *++argv;
--argc;
while (*++arg) {
@@ -277,7 +279,7 @@ static int _do_env_set(int flag, int argc, char * const
argv[], int env_flag)
return 1;
}
for (i = 2, s = value; i < argc; ++i) {
- char *v = argv[i];
+ const char *v = argv[i];
while ((*s++ = *v++) != '\0')
;
@@ -299,18 +301,30 @@ static int _do_env_set(int flag, int argc, char * const
argv[], int env_flag)
return 0;
}
-int env_set(const char *varname, const char *varvalue)
+static int do_programmatic_env_set(int argc, const char * const argv[])
{
- const char * const argv[4] = { "setenv", varname, varvalue, NULL };
-
/* before import into hashtable */
if (!(gd->flags & GD_FLG_ENV_READY))
return 1;
- if (varvalue == NULL || varvalue[0] == '\0')
- return _do_env_set(0, 2, (char * const *)argv, H_PROGRAMMATIC);
- else
- return _do_env_set(0, 3, (char * const *)argv, H_PROGRAMMATIC);
+ if (!argv[argc - 1] || argv[argc - 1][0] == '\0')
+ argc--;
+
+ return _do_env_set(0, argc, argv, H_PROGRAMMATIC);
+}
+
+int env_set(const char *varname, const char *varvalue)
+{
+ const char * const argv[4] = {"setenv", varname, varvalue, NULL};
+
+ return do_programmatic_env_set(3, argv);
+}
+
+int env_force(const char *varname, const char *varvalue)
+{
+ const char * const argv[5] = {"setenv", "-f", varname, varvalue, NULL};
+
+ return do_programmatic_env_set(4, argv);
}
/**
@@ -382,7 +396,8 @@ static int do_env_set(cmd_tbl_t *cmdtp, int flag, int argc,
char * const argv[])
if (argc < 2)
return CMD_RET_USAGE;
- return _do_env_set(flag, argc, argv, H_INTERACTIVE);
+ return _do_env_set(flag, argc, (const char * const *)argv,
+ H_INTERACTIVE);
}
/*
@@ -643,12 +658,12 @@ static int do_env_edit(cmd_tbl_t *cmdtp, int flag, int
argc,
if (buffer[0] == '\0') {
const char * const _argv[3] = { "setenv", argv[1], NULL };
- return _do_env_set(0, 2, (char * const *)_argv, H_INTERACTIVE);
+ return _do_env_set(0, 2, _argv, H_INTERACTIVE);
} else {
const char * const _argv[4] = { "setenv", argv[1], buffer,
NULL };
- return _do_env_set(0, 3, (char * const *)_argv, H_INTERACTIVE);
+ return _do_env_set(0, 3, _argv, H_INTERACTIVE);
}
}
#endif /* CONFIG_CMD_EDITENV */
diff --git a/include/env.h b/include/env.h
index b72239f6a5..37bbf1117d 100644
--- a/include/env.h
+++ b/include/env.h
@@ -145,6 +145,19 @@ int env_get_yesno(const char *var);
*/
int env_set(const char *varname, const char *value);
+/**
+ * env_force() - forcibly set an environment variable
+ *
+ * This sets or deletes the value of an environment variable. It is the same
+ * as env_set(), except that the variable will be forcibly updated/deleted,
+ * even if it has access protection flags set.
+ *
+ * @varname: Variable to adjust
+ * @value: Value to set for the variable, or NULL or "" to delete the variable
+ * @return 0 if OK, 1 on error
+ */
+int env_force(const char *varname, const char *varvalue);
+
/**
* env_get_ulong() - Return an environment variable as an integer value
*
--
2.24.0
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH v2] gpio: at91_gpio: Add bank names
Make the at91_gpio driver set sensible GPIO bank names in the platform
data. This makes the 'gpio status' command a lot more useful.
Signed-off-by: James Byrne
---
Changes in v2:
- Use "undefined" for an unknown bank name.
drivers/gpio/at91_gpio.c | 29 ++---
1 file changed, 26 insertions(+), 3 deletions(-)
diff --git a/drivers/gpio/at91_gpio.c b/drivers/gpio/at91_gpio.c
index 965becf77a..2c5a2098fe 100644
--- a/drivers/gpio/at91_gpio.c
+++ b/drivers/gpio/at91_gpio.c
@@ -19,6 +19,28 @@
#define GPIO_PER_BANK 32
+static const char *at91_get_bank_name(uint32_t base_addr)
+{
+ switch (base_addr) {
+ case ATMEL_BASE_PIOA:
+ return "PIOA";
+ case ATMEL_BASE_PIOB:
+ return "PIOB";
+ case ATMEL_BASE_PIOC:
+ return "PIOC";
+#if (ATMEL_PIO_PORTS > 3)
+ case ATMEL_BASE_PIOD:
+ return "PIOD";
+#if (ATMEL_PIO_PORTS > 4)
+ case ATMEL_BASE_PIOE:
+ return "PIOE";
+#endif
+#endif
+ }
+
+ return "undefined";
+}
+
static struct at91_port *at91_pio_get_port(unsigned port)
{
switch (port) {
@@ -582,14 +604,15 @@ static int at91_gpio_probe(struct udevice *dev)
clk_free();
- uc_priv->bank_name = plat->bank_name;
- uc_priv->gpio_count = GPIO_PER_BANK;
-
#if CONFIG_IS_ENABLED(OF_CONTROL)
plat->base_addr = (uint32_t)devfdt_get_addr_ptr(dev);
#endif
+ plat->bank_name = at91_get_bank_name(plat->base_addr);
port->regs = (struct at91_port *)plat->base_addr;
+ uc_priv->bank_name = plat->bank_name;
+ uc_priv->gpio_count = GPIO_PER_BANK;
+
return 0;
}
--
2.24.0
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] gpio: at91_gpio: Add bank names
Hi Eugen, On 18/11/2019 08:59, eugen.hris...@microchip.com wrote: @@ -582,14 +604,15 @@ static int at91_gpio_probe(struct udevice *dev) clk_free(); - uc_priv->bank_name = plat->bank_name; - uc_priv->gpio_count = GPIO_PER_BANK; - #if CONFIG_IS_ENABLED(OF_CONTROL) plat->base_addr = (uint32_t)devfdt_get_addr_ptr(dev); #endif + plat->bank_name = at91_get_bank_name(plat->base_addr); Hello James, Here you are rewriting the plat->bank_name... What was the old name that comes from the platform struct ? It was unset (null). Regards, James ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH] gpio: at91_gpio: Add bank names
Make the at91_gpio driver set sensible GPIO bank names in the platform
data. This makes the 'gpio status' command a lot more useful.
Signed-off-by: James Byrne
---
drivers/gpio/at91_gpio.c | 29 ++---
1 file changed, 26 insertions(+), 3 deletions(-)
diff --git a/drivers/gpio/at91_gpio.c b/drivers/gpio/at91_gpio.c
index 965becf77a..94b2b63a04 100644
--- a/drivers/gpio/at91_gpio.c
+++ b/drivers/gpio/at91_gpio.c
@@ -19,6 +19,28 @@
#define GPIO_PER_BANK 32
+static const char *at91_get_bank_name(uint32_t base_addr)
+{
+ switch (base_addr) {
+ case ATMEL_BASE_PIOA:
+ return "PIOA";
+ case ATMEL_BASE_PIOB:
+ return "PIOB";
+ case ATMEL_BASE_PIOC:
+ return "PIOC";
+#if (ATMEL_PIO_PORTS > 3)
+ case ATMEL_BASE_PIOD:
+ return "PIOD";
+#if (ATMEL_PIO_PORTS > 4)
+ case ATMEL_BASE_PIOE:
+ return "PIOE";
+#endif
+#endif
+ }
+
+ return "";
+}
+
static struct at91_port *at91_pio_get_port(unsigned port)
{
switch (port) {
@@ -582,14 +604,15 @@ static int at91_gpio_probe(struct udevice *dev)
clk_free();
- uc_priv->bank_name = plat->bank_name;
- uc_priv->gpio_count = GPIO_PER_BANK;
-
#if CONFIG_IS_ENABLED(OF_CONTROL)
plat->base_addr = (uint32_t)devfdt_get_addr_ptr(dev);
#endif
+ plat->bank_name = at91_get_bank_name(plat->base_addr);
port->regs = (struct at91_port *)plat->base_addr;
+ uc_priv->bank_name = plat->bank_name;
+ uc_priv->gpio_count = GPIO_PER_BANK;
+
return 0;
}
--
2.24.0
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH] board: Remove unnecessary inclusion of micrel.h from boards
Several boards still unnecessarily included micrel.h but no longer require it since the switch to Device Tree configuration. Signed-off-by: James Byrne --- board/atmel/sama5d3xek/sama5d3xek.c | 2 -- board/kosagi/novena/novena.c | 2 -- board/seco/mx6quq7/mx6quq7.c | 1 - board/toradex/colibri_imx6/colibri_imx6.c | 1 - board/tqc/tqma6/tqma6_wru4.c | 1 - 5 files changed, 7 deletions(-) diff --git a/board/atmel/sama5d3xek/sama5d3xek.c b/board/atmel/sama5d3xek/sama5d3xek.c index acf61486d2..331d31cfc7 100644 --- a/board/atmel/sama5d3xek/sama5d3xek.c +++ b/board/atmel/sama5d3xek/sama5d3xek.c @@ -14,8 +14,6 @@ #include #include #include -#include -#include #include #include #include diff --git a/board/kosagi/novena/novena.c b/board/kosagi/novena/novena.c index b7b747d196..45c8e798b6 100644 --- a/board/kosagi/novena/novena.c +++ b/board/kosagi/novena/novena.c @@ -32,8 +32,6 @@ #include #include #include -#include -#include #include #include #include diff --git a/board/seco/mx6quq7/mx6quq7.c b/board/seco/mx6quq7/mx6quq7.c index c1e36b652e..c0a93175fb 100644 --- a/board/seco/mx6quq7/mx6quq7.c +++ b/board/seco/mx6quq7/mx6quq7.c @@ -26,7 +26,6 @@ #include #include #include -#include #include #include diff --git a/board/toradex/colibri_imx6/colibri_imx6.c b/board/toradex/colibri_imx6/colibri_imx6.c index ad40b589c1..39718b567d 100644 --- a/board/toradex/colibri_imx6/colibri_imx6.c +++ b/board/toradex/colibri_imx6/colibri_imx6.c @@ -27,7 +27,6 @@ #include #include #include -#include #include #include #include diff --git a/board/tqc/tqma6/tqma6_wru4.c b/board/tqc/tqma6/tqma6_wru4.c index 99196ad685..6095bf3926 100644 --- a/board/tqc/tqma6/tqma6_wru4.c +++ b/board/tqc/tqma6/tqma6_wru4.c @@ -25,7 +25,6 @@ #include #include #include -#include #include #include #include -- 2.24.0 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH] net: phy: micrel: Allow KSZ8xxx and KSZ90x1 to be used together
Commit d397f7c45b0b ("net: phy: micrel: Separate KSZ9000 drivers from
KSZ8000 drivers") separated the KSZ8xxx and KSZ90x1 drivers and warns
that you shouldn't select both of them due to a device ID clash between
the KSZ9021 and the KS8721, asserting that "it is highly unlikely for a
system to contain both a KSZ8000 and a KSZ9000 PHY". Unfortunately
boards like the SAMA5D3xEK do contain both types of PHY, but fortunately
the Linux Micrel PHY driver provides a solution by using different PHY
ID and mask values to distinguish these chips.
This commit contains the following changes:
- The PHY ID and mask values for the KSZ9021 and the KS8721 now match
those used by the Linux driver.
- The warnings about not enabling both drivers have been removed.
- The description for PHY_MICREL_KSZ8XXX has been corrected (these are
10/100 PHYs, not GbE PHYs).
- PHY_MICREL_KSZ9021 and PHY_MICREL_KSZ9031 no longer select PHY_GIGE
since this is selected by PHY_MICREL_KSZ90X1.
- All of the relevant defconfig files have been updated now that
PHY_MICREL_KSZ8XXX does not default to 'Y'.
Signed-off-by: James Byrne
---
configs/alt_defconfig| 1 +
configs/aristainetos_defconfig | 1 +
configs/bk4r1_defconfig | 1 +
configs/colibri_imx6_defconfig | 1 +
configs/colibri_imx6_nospl_defconfig | 1 +
configs/colibri_imx7_defconfig | 1 +
configs/colibri_imx7_emmc_defconfig | 1 +
configs/colibri_vf_defconfig | 1 +
configs/flea3_defconfig | 1 +
configs/gose_defconfig | 1 +
configs/imx6dl_mamoj_defconfig | 1 +
configs/imx6qdl_icore_rqs_defconfig | 1 +
configs/k2g_evm_defconfig| 1 +
configs/k2g_hs_evm_defconfig | 1 +
configs/koelsch_defconfig| 1 +
configs/lager_defconfig | 1 +
configs/m53menlo_defconfig | 1 +
configs/mx6ul_14x14_evk_defconfig| 1 +
configs/mx6ul_9x9_evk_defconfig | 1 +
configs/opos6uldev_defconfig | 1 +
configs/pcm052_defconfig | 1 +
configs/phycore_pcl063_defconfig | 1 +
configs/pico-hobbit-imx6ul_defconfig | 1 +
configs/pico-imx6ul_defconfig| 1 +
configs/pico-pi-imx6ul_defconfig | 1 +
configs/porter_defconfig | 1 +
configs/silk_defconfig | 1 +
configs/stout_defconfig | 1 +
configs/stv0991_defconfig| 1 +
configs/udoo_neo_defconfig | 1 +
configs/vf610twr_defconfig | 1 +
configs/vf610twr_nand_defconfig | 1 +
configs/woodburn_defconfig | 1 +
configs/woodburn_sd_defconfig| 1 +
drivers/net/phy/Kconfig | 19 +--
drivers/net/phy/micrel_ksz8xxx.c | 8 +---
drivers/net/phy/micrel_ksz90x1.c | 2 +-
37 files changed, 45 insertions(+), 18 deletions(-)
diff --git a/configs/alt_defconfig b/configs/alt_defconfig
index c4ece79507..44f25a4b94 100644
--- a/configs/alt_defconfig
+++ b/configs/alt_defconfig
@@ -67,6 +67,7 @@ CONFIG_SPI_FLASH=y
CONFIG_SPI_FLASH_SPANSION=y
CONFIG_SPI_FLASH_MTD=y
CONFIG_PHY_MICREL=y
+CONFIG_PHY_MICREL_KSZ8XXX=y
CONFIG_DM_ETH=y
CONFIG_SH_ETHER=y
CONFIG_PCI=y
diff --git a/configs/aristainetos_defconfig b/configs/aristainetos_defconfig
index 950f9f6baa..0f71f4621e 100644
--- a/configs/aristainetos_defconfig
+++ b/configs/aristainetos_defconfig
@@ -44,6 +44,7 @@ CONFIG_MTD_UBI_FASTMAP=y
CONFIG_MTD_UBI_FASTMAP_AUTOCONVERT=1
CONFIG_PHYLIB=y
CONFIG_PHY_MICREL=y
+CONFIG_PHY_MICREL_KSZ8XXX=y
CONFIG_MII=y
CONFIG_SPI=y
CONFIG_MXC_SPI=y
diff --git a/configs/bk4r1_defconfig b/configs/bk4r1_defconfig
index 9e31b4ac97..5608110c3a 100644
--- a/configs/bk4r1_defconfig
+++ b/configs/bk4r1_defconfig
@@ -39,6 +39,7 @@ CONFIG_SPI_FLASH_STMICRO=y
CONFIG_SPI_FLASH_MTD=y
CONFIG_PHYLIB=y
CONFIG_PHY_MICREL=y
+CONFIG_PHY_MICREL_KSZ8XXX=y
CONFIG_MII=y
CONFIG_RTC_M41T62=y
CONFIG_DM_SERIAL=y
diff --git a/configs/colibri_imx6_defconfig b/configs/colibri_imx6_defconfig
index 2072281354..4127a47115 100644
--- a/configs/colibri_imx6_defconfig
+++ b/configs/colibri_imx6_defconfig
@@ -57,6 +57,7 @@ CONFIG_DFU_MMC=y
CONFIG_FSL_ESDHC=y
CONFIG_PHYLIB=y
CONFIG_PHY_MICREL=y
+CONFIG_PHY_MICREL_KSZ8XXX=y
CONFIG_MII=y
CONFIG_USB=y
CONFIG_USB_STORAGE=y
diff --git a/configs/colibri_imx6_nospl_defconfig
b/configs/colibri_imx6_nospl_defconfig
index 5e9490bc42..b1609b8b7e 100644
--- a/configs/colibri_imx6_nospl_defconfig
+++ b/configs/colibri_imx6_nospl_defconfig
@@ -46,6 +46,7 @@ CONFIG_DFU_MMC=y
CONFIG_FSL_ESDHC=y
CONFIG_PHYLIB=y
CONFIG_PHY_MICREL=y
+CONFIG_PHY_MICREL_KSZ8XXX=y
CONFIG_MII=y
CONFIG_USB=y
CONFIG_USB_STORAGE=y
diff --git a/configs/colibri_imx7_defconfig b/configs/colibri_imx7_defconfig
index 7a52361a2a..f4c78dfa1d 100644
--- a/configs/colibri_imx7_defconfig
+++ b/configs/colibri_imx7_defconfig
@@ -57,6 +57,7 @@ CONFIG_NAND_MXS_DT=y
CONFIG_MTD_UBI_FASTMAP=y
CONFIG_PHYLIB=y
CONFIG_PHY_MICREL=y
+CONFIG_
[U-Boot] [PATCH 0/2] Apply correct skew values to KSZ9021 PHYs
I have been having problems making the Gigabit Ethernet interface work on the SAMA5D3xEK board with recent versions of U-Boot. After much debugging it eventually transpired that this was because the PHY's skew registers were not being programmed, and it has never worked since this board was converted to use device trees. After fixing the problem that caused the skew values to be missed, I found that the wrong values got programmed, so I had to fix that as well. The following patches resolve both issues. James Byrne (2): net: phy: micrel: Use correct skew values on KSZ9021 net: phy: micrel: Find Micrel PHY node correctly arch/arm/dts/sama5d3xcm.dtsi | 32 +-- arch/arm/dts/sama5d3xcm_cmp.dtsi | 32 +-- arch/arm/dts/socfpga_arria5_socdk.dts | 4 +-- arch/arm/dts/socfpga_cyclone5_is1.dts | 4 +-- arch/arm/dts/socfpga_cyclone5_socdk.dts | 4 +-- arch/arm/dts/socfpga_cyclone5_sockit.dts | 4 +-- arch/arm/dts/socfpga_cyclone5_vining_fpga.dts | 4 +-- .../net/micrel-ksz90x1.txt| 27 drivers/net/phy/micrel_ksz90x1.c | 24 +++--- 9 files changed, 88 insertions(+), 47 deletions(-) -- 2.17.1 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH 2/2] net: phy: micrel: Find Micrel PHY node correctly
In some of the device trees that specify skew values for KSZ90x1 PHYs
the values are stored (incorrectly) in the MAC node, whereas in others
it is in an 'ethernet-phy' subnode. Previously the code would fail to
find and program these skew values, so this commit changes it to look
for an "ethernet-phy" subnode first, and revert to looking in the MAC
node if there isn't one.
The device trees affected (where the skew values are in a subnode) are
imx6qdl-icore-rqs.dtsi, r8a77970-eagle.dts, r8a77990-ebisu.dts,
r8a77995-draak.dts, salvator-common.dtsi, sama5d3xcm.dtsi,
sama5d3xcm_cmp.dtsi, socfpga_cyclone5_vining_fpga.dts,
socfpga_stratix10_socdk.dts and ulcb.dtsi. Before this change the skew
values in these device trees would be ignored.
The device trees where the skew values are in the MAC node are
socfpga_arria10_socdk.dtsi, socfpga_arria5_socdk.dts,
socfpga_cyclone5_de0_nano_soc.dts, socfpga_cyclone5_de10_nano.dts,
socfpga_cyclone5_de1_soc.dts, socfpga_cyclone5_is1.dts,
socfpga_cyclone5_socdk.dts, socfpga_cyclone5_sockit.dts. These should be
unaffected by this change.
The changes were tested on a sama5d3xcm.
Signed-off-by: James Byrne
---
drivers/net/phy/micrel_ksz90x1.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/net/phy/micrel_ksz90x1.c b/drivers/net/phy/micrel_ksz90x1.c
index 1f8d86ab2e..8dec9f2367 100644
--- a/drivers/net/phy/micrel_ksz90x1.c
+++ b/drivers/net/phy/micrel_ksz90x1.c
@@ -114,12 +114,20 @@ static int ksz90x1_of_config_group(struct phy_device
*phydev,
int val[4];
int i, changed = 0, offset, max;
u16 regval = 0;
+ ofnode node;
if (!drv || !drv->writeext)
return -EOPNOTSUPP;
+ /* Look for a PHY node under the Ethernet node */
+ node = dev_read_subnode(dev, "ethernet-phy");
+ if (!ofnode_valid(node)) {
+ /* No node found, look in the Ethernet node */
+ node = dev_ofnode(dev);
+ }
+
for (i = 0; i < ofcfg->grpsz; i++) {
- val[i] = dev_read_u32_default(dev, ofcfg->grp[i].name, ~0);
+ val[i] = ofnode_read_u32_default(node, ofcfg->grp[i].name, ~0);
offset = ofcfg->grp[i].off;
if (val[i] == -1) {
/* Default register value for KSZ9021 */
--
2.17.1
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH 1/2] net: phy: micrel: Use correct skew values on KSZ9021
Commit ff7bd212cb8a ("net: phy: micrel: fix divisor value for KSZ9031
phy skew") fixed the skew value divisor for the KSZ9031, but left the
code using the same divisor for the KSZ9021, which is incorrect.
The preceding commit c16e69f702b1 ("net: phy: micrel: add documentation
for Micrel KSZ90x1 binding") added the DTS documentation for the
KSZ90x1, changing it from the equivalent file in the Linux kernel to
correctly state that for this part the skew value is set in 120ps steps,
whereas the Linux documentation and driver continue to this day to use
the incorrect value of 200 that came from the original KSZ9021 datasheet
before it was corrected in revision 1.2 (Feb 2014).
This commit sorts out the resulting confusion in a consistent way by
making the following changes:
- Update the documentation to be clear about what the skew values mean,
in the same was as for the KSZ9031.
- Update the Micrel PHY driver to select the appropriate divisor for
both parts.
- Adjust all the device trees that state skew values for KSZ9021 PHYs to
use values based on 120ps steps instead of 200ps steps. This will result
in the same values being programmed into the skew registers as the
equivalent device trees in the Linux kernel do, where it incorrectly
uses 200ps steps (since that's where all these device trees were copied
from).
Signed-off-by: James Byrne
---
arch/arm/dts/sama5d3xcm.dtsi | 32 +--
arch/arm/dts/sama5d3xcm_cmp.dtsi | 32 +--
arch/arm/dts/socfpga_arria5_socdk.dts | 4 +--
arch/arm/dts/socfpga_cyclone5_is1.dts | 4 +--
arch/arm/dts/socfpga_cyclone5_socdk.dts | 4 +--
arch/arm/dts/socfpga_cyclone5_sockit.dts | 4 +--
arch/arm/dts/socfpga_cyclone5_vining_fpga.dts | 4 +--
.../net/micrel-ksz90x1.txt| 27
drivers/net/phy/micrel_ksz90x1.c | 14 +---
9 files changed, 79 insertions(+), 46 deletions(-)
diff --git a/arch/arm/dts/sama5d3xcm.dtsi b/arch/arm/dts/sama5d3xcm.dtsi
index 2cf9c3611d..d123057f30 100644
--- a/arch/arm/dts/sama5d3xcm.dtsi
+++ b/arch/arm/dts/sama5d3xcm.dtsi
@@ -44,28 +44,28 @@
reg = <0x1>;
interrupt-parent = <>;
interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
- txen-skew-ps = <800>;
- txc-skew-ps = <3000>;
- rxdv-skew-ps = <400>;
- rxc-skew-ps = <3000>;
- rxd0-skew-ps = <400>;
- rxd1-skew-ps = <400>;
- rxd2-skew-ps = <400>;
- rxd3-skew-ps = <400>;
+ txen-skew-ps = <480>;
+ txc-skew-ps = <1800>;
+ rxdv-skew-ps = <240>;
+ rxc-skew-ps = <1800>;
+ rxd0-skew-ps = <240>;
+ rxd1-skew-ps = <240>;
+ rxd2-skew-ps = <240>;
+ rxd3-skew-ps = <240>;
};
ethernet-phy@7 {
reg = <0x7>;
interrupt-parent = <>;
interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
- txen-skew-ps = <800>;
- txc-skew-ps = <3000>;
- rxdv-skew-ps = <400>;
- rxc-skew-ps = <3000>;
- rxd0-skew-ps = <400>;
- rxd1-skew-ps = <400>;
- rxd2-skew-ps = <400>;
- rxd3-skew-ps = <400>;
+ txen-skew-ps = <480>;
+ txc-skew-ps = <1800>;
+ rxdv-skew-ps = <240>;
+ rxc-skew-ps = <1800>;
+ rxd0-skew-ps = <240>;
+ rxd1-skew-ps = <240>;
+ rxd2-skew-ps = <240>;
+ rxd3-skew-ps = <240>;
};
};
};
diff --git a/arch
[U-Boot] [PATCH v3] common: cli_readline: Improve command line editing
This improves the cread_line() function so that it will correctly
process the 'Home', 'End', 'Delete' and arrow key escape sequences
produced by various terminal emulators. This makes command line editing
a more pleasant experience.
The previous code only supported the cursor keys and the 'Home' key, and
only for certain terminal emulator configurations. This adds support for
the 'End and 'Delete' keys, and recognises a wider range of escape
sequences. For example, the left arrow key can be 'ESC O D' instead of
'ESC [ D', and the 'Home' key can be 'ESC [ H', 'ESC O H', 'ESC 1 ~' or
'ESC 7 ~', depending on what terminal emulator you use and how it is
configured.
Signed-off-by: James Byrne <james.by...@origamienergy.com>
Changes for v2
- Explicitly initialize variable to avoid spurious compiler warning.
Changes for v3
- Remove unnecessary setting of 'act' to ESC_REJECT (now its default
value).
---
common/cli_readline.c | 98 +++
1 file changed, 67 insertions(+), 31 deletions(-)
diff --git a/common/cli_readline.c b/common/cli_readline.c
index c1476e4..ecded11 100644
--- a/common/cli_readline.c
+++ b/common/cli_readline.c
@@ -283,46 +283,82 @@ static int cread_line(const char *const prompt, char
*buf, unsigned int *len,
* handle standard linux xterm esc sequences for arrow key, etc.
*/
if (esc_len != 0) {
+ enum { ESC_REJECT, ESC_SAVE, ESC_CONVERTED } act =
ESC_REJECT;
+
if (esc_len == 1) {
- if (ichar == '[') {
- esc_save[esc_len] = ichar;
- esc_len = 2;
- } else {
- cread_add_str(esc_save, esc_len,
- insert, , _num,
- buf, *len);
- esc_len = 0;
+ if (ichar == '[' || ichar == 'O')
+ act = ESC_SAVE;
+ } else if (esc_len == 2) {
+ switch (ichar) {
+ case 'D': /* <- key */
+ ichar = CTL_CH('b');
+ act = ESC_CONVERTED;
+ break; /* pass off to ^B handler */
+ case 'C': /* -> key */
+ ichar = CTL_CH('f');
+ act = ESC_CONVERTED;
+ break; /* pass off to ^F handler */
+ case 'H': /* Home key */
+ ichar = CTL_CH('a');
+ act = ESC_CONVERTED;
+ break; /* pass off to ^A handler */
+ case 'F': /* End key */
+ ichar = CTL_CH('e');
+ act = ESC_CONVERTED;
+ break; /* pass off to ^E handler */
+ case 'A': /* up arrow */
+ ichar = CTL_CH('p');
+ act = ESC_CONVERTED;
+ break; /* pass off to ^P handler */
+ case 'B': /* down arrow */
+ ichar = CTL_CH('n');
+ act = ESC_CONVERTED;
+ break; /* pass off to ^N handler */
+ case '1':
+ case '3':
+ case '4':
+ case '7':
+ case '8':
+ if (esc_save[1] == '[') {
+ /* see if next character is ~ */
+ act = ESC_SAVE;
+ }
+ break;
+ }
+ } else if (esc_len == 3) {
+ if (ichar == '~') {
+ switch (esc_save[2]) {
+ case '3': /* Delete key */
+ ichar = CTL_CH('d');
+ act = ESC_CONVERTED;
+ break; /* pass to ^D handler */
+ case '1': /* Home key */
+
[U-Boot] [PATCH v2] common: cli_readline: Improve command line editing
This improves the cread_line() function so that it will correctly
process the 'Home', 'End', 'Delete' and arrow key escape sequences
produced by various terminal emulators. This makes command line editing
a more pleasant experience.
The previous code only supported the cursor keys and the 'Home' key, and
only for certain terminal emulator configurations. This adds support for
the 'End and 'Delete' keys, and recognises a wider range of escape
sequences. For example, the left arrow key can be 'ESC O D' instead of
'ESC [ D', and the 'Home' key can be 'ESC [ H', 'ESC O H', 'ESC 1 ~' or
'ESC 7 ~', depending on what terminal emulator you use and how it is
configured.
Signed-off-by: James Byrne <james.by...@origamienergy.com>
---
Changes for v2
- Explicitly initialize variable to avoid spurious compiler warning.
common/cli_readline.c | 108 --
1 file changed, 78 insertions(+), 30 deletions(-)
diff --git a/common/cli_readline.c b/common/cli_readline.c
index c1476e4..6690c13 100644
--- a/common/cli_readline.c
+++ b/common/cli_readline.c
@@ -283,46 +283,94 @@ static int cread_line(const char *const prompt, char
*buf, unsigned int *len,
* handle standard linux xterm esc sequences for arrow key, etc.
*/
if (esc_len != 0) {
+ enum { ESC_REJECT, ESC_SAVE, ESC_CONVERTED } act =
ESC_REJECT;
+
if (esc_len == 1) {
- if (ichar == '[') {
- esc_save[esc_len] = ichar;
- esc_len = 2;
+ if (ichar == '[' || ichar == 'O')
+ act = ESC_SAVE;
+ else
+ act = ESC_REJECT;
+ } else if (esc_len == 2) {
+ switch (ichar) {
+ case 'D': /* <- key */
+ ichar = CTL_CH('b');
+ act = ESC_CONVERTED;
+ break; /* pass off to ^B handler */
+ case 'C': /* -> key */
+ ichar = CTL_CH('f');
+ act = ESC_CONVERTED;
+ break; /* pass off to ^F handler */
+ case 'H': /* Home key */
+ ichar = CTL_CH('a');
+ act = ESC_CONVERTED;
+ break; /* pass off to ^A handler */
+ case 'F': /* End key */
+ ichar = CTL_CH('e');
+ act = ESC_CONVERTED;
+ break; /* pass off to ^E handler */
+ case 'A': /* up arrow */
+ ichar = CTL_CH('p');
+ act = ESC_CONVERTED;
+ break; /* pass off to ^P handler */
+ case 'B': /* down arrow */
+ ichar = CTL_CH('n');
+ act = ESC_CONVERTED;
+ break; /* pass off to ^N handler */
+ case '1':
+ case '3':
+ case '4':
+ case '7':
+ case '8':
+ if (esc_save[1] == '[') {
+ /* see if next character is ~ */
+ act = ESC_SAVE;
+ } else {
+ act = ESC_REJECT;
+ }
+ break;
+ default:
+ act = ESC_REJECT;
+ break;
+ }
+ } else if (esc_len == 3) {
+ if (ichar == '~') {
+ switch (esc_save[2]) {
+ case '3': /* Delete key */
+ ichar = CTL_CH('d');
+ act = ESC_CONVERTED;
+ break; /* pass to ^D handler */
+ case '1': /* Home key */
+ case '7':
+ ic
[U-Boot] [PATCH] common: cli_readline: Improve command line editing
This improves the cread_line() function so that it will correctly
process the 'Home', 'End', 'Delete' and arrow key escape sequences
produced by various terminal emulators. This makes command line editing
a more pleasant experience.
The previous code only supported the cursor keys and the 'Home' key, and
only for certain terminal emulator configurations. This adds support for
the 'End and 'Delete' keys, and recognises a wider range of escape
sequences. For example, the left arrow key can be 'ESC O D' instead of
'ESC [ D', and the 'Home' key can be 'ESC [ H', 'ESC O H', 'ESC 1 ~' or
'ESC 7 ~', depending on what terminal emulator you use and how it is
configured.
Signed-off-by: James Byrne <james.by...@origamienergy.com>
---
common/cli_readline.c | 108 --
1 file changed, 78 insertions(+), 30 deletions(-)
diff --git a/common/cli_readline.c b/common/cli_readline.c
index c1476e4..0d4ad49 100644
--- a/common/cli_readline.c
+++ b/common/cli_readline.c
@@ -283,46 +283,94 @@ static int cread_line(const char *const prompt, char
*buf, unsigned int *len,
* handle standard linux xterm esc sequences for arrow key, etc.
*/
if (esc_len != 0) {
+ enum { ESC_REJECT, ESC_SAVE, ESC_CONVERTED } act;
+
if (esc_len == 1) {
- if (ichar == '[') {
- esc_save[esc_len] = ichar;
- esc_len = 2;
+ if (ichar == '[' || ichar == 'O')
+ act = ESC_SAVE;
+ else
+ act = ESC_REJECT;
+ } else if (esc_len == 2) {
+ switch (ichar) {
+ case 'D': /* <- key */
+ ichar = CTL_CH('b');
+ act = ESC_CONVERTED;
+ break; /* pass off to ^B handler */
+ case 'C': /* -> key */
+ ichar = CTL_CH('f');
+ act = ESC_CONVERTED;
+ break; /* pass off to ^F handler */
+ case 'H': /* Home key */
+ ichar = CTL_CH('a');
+ act = ESC_CONVERTED;
+ break; /* pass off to ^A handler */
+ case 'F': /* End key */
+ ichar = CTL_CH('e');
+ act = ESC_CONVERTED;
+ break; /* pass off to ^E handler */
+ case 'A': /* up arrow */
+ ichar = CTL_CH('p');
+ act = ESC_CONVERTED;
+ break; /* pass off to ^P handler */
+ case 'B': /* down arrow */
+ ichar = CTL_CH('n');
+ act = ESC_CONVERTED;
+ break; /* pass off to ^N handler */
+ case '1':
+ case '3':
+ case '4':
+ case '7':
+ case '8':
+ if (esc_save[1] == '[') {
+ /* see if next character is ~ */
+ act = ESC_SAVE;
+ } else {
+ act = ESC_REJECT;
+ }
+ break;
+ default:
+ act = ESC_REJECT;
+ break;
+ }
+ } else if (esc_len == 3) {
+ if (ichar == '~') {
+ switch (esc_save[2]) {
+ case '3': /* Delete key */
+ ichar = CTL_CH('d');
+ act = ESC_CONVERTED;
+ break; /* pass to ^D handler */
+ case '1': /* Home key */
+ case '7':
+ ichar = CTL_CH('a');
+ act = E
