mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [RFC PATCH v2 0/8] ratp: new generic RATP command support
@ 2018-02-08 13:22 Aleksander Morgado
  2018-02-08 13:22 ` [RFC PATCH v2 1/8] ratp: implement generic " Aleksander Morgado
                   ` (7 more replies)
  0 siblings, 8 replies; 12+ messages in thread
From: Aleksander Morgado @ 2018-02-08 13:22 UTC (permalink / raw)
  To: barebox; +Cc: Aleksander Morgado

This updated patchset doesn't break the RATP API. It keeps the separate IDs for requests and responses, and doesn't try to flag responses or indications in a different way.

The command definition logic is also updated so that the request and response IDs associated to each command are defined in the body of the command, instead of magically constructing the id fields with macro glues.

All the RATP specific commands are now implemented in files completely separated from the console commands, and they are all kept in common/ratp, along with the core ratp implementation itself.

As additional changes w.r.t. v1, this patchset also includes a change to make it possible building the RATP logic without full console support (e.g. making it possible to enable CONFIG_RATP with CONFIG_CONSOLE_RATP disabled).

The new commands were tested with the libratp-barebox library
(wip2/md-mw branch) in https://github.com/aleksander0m/libratp-barebox

Aleksander Morgado (8):
  ratp: implement generic command support
  ratp: moved logic to its own subdirectory
  ratp: allow building without full console support
  ratp: implement ping as a standard ratp command
  ratp: implement getenv as a standard ratp command
  ratp: use xstrndup() instead of a custom xmemdup_add_zero()
  ratp: new md and mw commands
  ratp: new reset command

 arch/arm/lib32/barebox.lds.S              |   4 +
 arch/arm/lib64/barebox.lds.S              |   4 +
 arch/blackfin/boards/ipe337/barebox.lds.S |   5 +-
 arch/mips/lib/barebox.lds.S               |   4 +
 arch/nios2/cpu/barebox.lds.S              |   5 +-
 arch/openrisc/cpu/barebox.lds.S           |   4 +
 arch/ppc/boards/pcm030/barebox.lds.S      |   4 +
 arch/ppc/mach-mpc85xx/barebox.lds.S       |   4 +
 arch/sandbox/board/barebox.lds.S          |   5 +
 arch/x86/lib/barebox.lds.S                |   7 ++
 arch/x86/mach-efi/elf_ia32_efi.lds.S      |   5 +
 arch/x86/mach-efi/elf_x86_64_efi.lds.S    |   5 +
 common/Kconfig                            |  13 +-
 common/Makefile                           |   3 +-
 common/module.lds.S                       |   2 +
 common/ratp/Kconfig                       |  14 +++
 common/ratp/Makefile                      |   6 +
 common/ratp/getenv.c                      |  51 ++++++++
 common/ratp/md.c                          | 202 ++++++++++++++++++++++++++++++
 common/ratp/mw.c                          | 173 +++++++++++++++++++++++++
 common/ratp/ping.c                        |  40 ++++++
 common/{ => ratp}/ratp.c                  | 156 +++++++++++------------
 common/ratp/reset.c                       |  55 ++++++++
 fs/Kconfig                                |   2 +-
 include/asm-generic/barebox.lds.h         |   2 +
 include/ratp_bb.h                         |  52 ++++++++
 lib/Kconfig                               |   2 +-
 lib/readline.c                            |   2 +-
 28 files changed, 728 insertions(+), 103 deletions(-)
 create mode 100644 common/ratp/Kconfig
 create mode 100644 common/ratp/Makefile
 create mode 100644 common/ratp/getenv.c
 create mode 100644 common/ratp/md.c
 create mode 100644 common/ratp/mw.c
 create mode 100644 common/ratp/ping.c
 rename common/{ => ratp}/ratp.c (81%)
 create mode 100644 common/ratp/reset.c

--
2.15.1

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [RFC PATCH v2 1/8] ratp: implement generic command support
  2018-02-08 13:22 [RFC PATCH v2 0/8] ratp: new generic RATP command support Aleksander Morgado
@ 2018-02-08 13:22 ` Aleksander Morgado
  2018-02-08 13:22 ` [RFC PATCH v2 2/8] ratp: moved logic to its own subdirectory Aleksander Morgado
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Aleksander Morgado @ 2018-02-08 13:22 UTC (permalink / raw)
  To: barebox; +Cc: Aleksander Morgado

The RATP implementation now allows executing generic commands with a
binary interface: binary requests are received and binary responses
are returned.

Each command can define its own RATP request contents (e.g. to specify
command-specific options) as well as its own RATP response contents
(if any data is to be returned).

Each command is associated with a pair of numeric unique request and
response IDs, and for easy reference these IDs are maintained in the
common ratp_bb header. Modules may override generic implemented
commands or include their own new ones (as long as the numeric IDs
introduced are unique).

Signed-off-by: Aleksander Morgado <aleksander@aleksander.es>
---
 arch/arm/lib32/barebox.lds.S              |  4 ++
 arch/arm/lib64/barebox.lds.S              |  4 ++
 arch/blackfin/boards/ipe337/barebox.lds.S |  5 +-
 arch/mips/lib/barebox.lds.S               |  4 ++
 arch/nios2/cpu/barebox.lds.S              |  5 +-
 arch/openrisc/cpu/barebox.lds.S           |  4 ++
 arch/ppc/boards/pcm030/barebox.lds.S      |  4 ++
 arch/ppc/mach-mpc85xx/barebox.lds.S       |  4 ++
 arch/sandbox/board/barebox.lds.S          |  5 ++
 arch/x86/lib/barebox.lds.S                |  7 +++
 arch/x86/mach-efi/elf_ia32_efi.lds.S      |  5 ++
 arch/x86/mach-efi/elf_x86_64_efi.lds.S    |  5 ++
 common/module.lds.S                       |  2 +
 common/ratp.c                             | 82 +++++++++++++++++++++++++------
 include/asm-generic/barebox.lds.h         |  2 +
 include/ratp_bb.h                         | 47 ++++++++++++++++++
 16 files changed, 171 insertions(+), 18 deletions(-)

diff --git a/arch/arm/lib32/barebox.lds.S b/arch/arm/lib32/barebox.lds.S
index e7b87b7cd..6fadc2a35 100644
--- a/arch/arm/lib32/barebox.lds.S
+++ b/arch/arm/lib32/barebox.lds.S
@@ -85,6 +85,10 @@ SECTIONS
 	.barebox_cmd : { BAREBOX_CMDS }
 	__barebox_cmd_end = .;

+	__barebox_ratp_cmd_start = .;
+	.barebox_ratp_cmd : { BAREBOX_RATP_CMDS }
+	__barebox_ratp_cmd_end = .;
+
 	__barebox_magicvar_start = .;
 	.barebox_magicvar : { BAREBOX_MAGICVARS }
 	__barebox_magicvar_end = .;
diff --git a/arch/arm/lib64/barebox.lds.S b/arch/arm/lib64/barebox.lds.S
index 240699f1a..a53b933bb 100644
--- a/arch/arm/lib64/barebox.lds.S
+++ b/arch/arm/lib64/barebox.lds.S
@@ -82,6 +82,10 @@ SECTIONS
 	.barebox_cmd : { BAREBOX_CMDS }
 	__barebox_cmd_end = .;

+	__barebox_ratp_cmd_start = .;
+	.barebox_ratp_cmd : { BAREBOX_RATP_CMDS }
+	__barebox_ratp_cmd_end = .;
+
 	__barebox_magicvar_start = .;
 	.barebox_magicvar : { BAREBOX_MAGICVARS }
 	__barebox_magicvar_end = .;
diff --git a/arch/blackfin/boards/ipe337/barebox.lds.S b/arch/blackfin/boards/ipe337/barebox.lds.S
index 51a586af2..7e82a1bd7 100644
--- a/arch/blackfin/boards/ipe337/barebox.lds.S
+++ b/arch/blackfin/boards/ipe337/barebox.lds.S
@@ -68,6 +68,10 @@ SECTIONS
 	.barebox_cmd : { BAREBOX_CMDS }
 	___barebox_cmd_end = .;

+	___barebox_ratp_cmd_start = .;
+	.barebox_ratp_cmd : { BAREBOX_RATP_CMDS }
+	___barebox_ratp_cmd_end = .;
+
 	___barebox_magicvar_start = .;
 	.barebox_magicvar : { BAREBOX_MAGICVARS }
 	___barebox_magicvar_end = .;
@@ -91,4 +95,3 @@ SECTIONS
 	___bss_stop = .;
 	_end = .;
 }
-
diff --git a/arch/mips/lib/barebox.lds.S b/arch/mips/lib/barebox.lds.S
index 899f62b96..660d4be85 100644
--- a/arch/mips/lib/barebox.lds.S
+++ b/arch/mips/lib/barebox.lds.S
@@ -55,6 +55,10 @@ SECTIONS
 	.barebox_cmd : { BAREBOX_CMDS }
 	__barebox_cmd_end = .;

+	__barebox_ratp_cmd_start = .;
+	.barebox_ratp_cmd : { BAREBOX_RATP_CMDS }
+	__barebox_ratp_cmd_end = .;
+
 	__barebox_magicvar_start = .;
 	.barebox_magicvar : { BAREBOX_MAGICVARS }
 	__barebox_magicvar_end = .;
diff --git a/arch/nios2/cpu/barebox.lds.S b/arch/nios2/cpu/barebox.lds.S
index a2d7fa8cd..fbcd1cd3f 100644
--- a/arch/nios2/cpu/barebox.lds.S
+++ b/arch/nios2/cpu/barebox.lds.S
@@ -55,6 +55,10 @@ SECTIONS
 	.barebox_cmd : { BAREBOX_CMDS }
 	__barebox_cmd_end = .;

+	__barebox_ratp_cmd_start = .;
+	.barebox_ratp_cmd : { BAREBOX_RATP_CMDS }
+	__barebox_ratp_cmd_end = .;
+
 	__barebox_magicvar_start = .;
 	.barebox_magicvar : { BAREBOX_MAGICVARS }
 	__barebox_magicvar_end = .;
@@ -129,4 +133,3 @@ SECTIONS
 	_end = .;
 	PROVIDE (end = .);
 }
-
diff --git a/arch/openrisc/cpu/barebox.lds.S b/arch/openrisc/cpu/barebox.lds.S
index b819ca099..c6807aec3 100644
--- a/arch/openrisc/cpu/barebox.lds.S
+++ b/arch/openrisc/cpu/barebox.lds.S
@@ -57,6 +57,10 @@ SECTIONS
 	.barebox_cmd : { BAREBOX_CMDS } > ram
 	__barebox_cmd_end = .;

+	__barebox_ratp_cmd_start = .;
+	.barebox_ratp_cmd : { BAREBOX_RATP_CMDS } > ram
+	__barebox_ratp_cmd_end = .;
+
 	__barebox_magicvar_start = .;
 	.barebox_magicvar : { BAREBOX_MAGICVARS } > ram
 	__barebox_magicvar_end = .;
diff --git a/arch/ppc/boards/pcm030/barebox.lds.S b/arch/ppc/boards/pcm030/barebox.lds.S
index 0e34f0a41..3b8bf3c0d 100644
--- a/arch/ppc/boards/pcm030/barebox.lds.S
+++ b/arch/ppc/boards/pcm030/barebox.lds.S
@@ -104,6 +104,10 @@ SECTIONS
   .barebox_cmd : { BAREBOX_CMDS }
   __barebox_cmd_end = .;

+  __barebox_ratp_cmd_start = .;
+  .barebox_ratp_cmd : { BAREBOX_RATP_CMDS }
+  __barebox_ratp_cmd_end = .;
+
   __barebox_magicvar_start = .;
   .barebox_magicvar : { BAREBOX_MAGICVARS }
   __barebox_magicvar_end = .;
diff --git a/arch/ppc/mach-mpc85xx/barebox.lds.S b/arch/ppc/mach-mpc85xx/barebox.lds.S
index beebab39d..000197283 100644
--- a/arch/ppc/mach-mpc85xx/barebox.lds.S
+++ b/arch/ppc/mach-mpc85xx/barebox.lds.S
@@ -105,6 +105,10 @@ SECTIONS
   .barebox_cmd : { BAREBOX_CMDS }
   __barebox_cmd_end = .;

+  __barebox_ratp_cmd_start = .;
+  .barebox_ratp_cmd : { BAREBOX_RATP_CMDS }
+  __barebox_ratp_cmd_end = .;
+
   __barebox_initcalls_start = .;
   .barebox_initcalls : { INITCALLS }
   __barebox_initcalls_end = .;
diff --git a/arch/sandbox/board/barebox.lds.S b/arch/sandbox/board/barebox.lds.S
index 0d67ab660..80e27fe87 100644
--- a/arch/sandbox/board/barebox.lds.S
+++ b/arch/sandbox/board/barebox.lds.S
@@ -21,6 +21,11 @@ SECTIONS
 	__barebox_cmd_start = .;
 	__barebox_cmd : { BAREBOX_CMDS }
 	__barebox_cmd_end = .;
+
+	. = ALIGN(64);
+	__barebox_ratp_cmd_start = .;
+	__barebox_ratp_cmd : { BAREBOX_RATP_CMDS }
+	__barebox_ratp_cmd_end = .;
 }

 INSERT BEFORE .rodata;
diff --git a/arch/x86/lib/barebox.lds.S b/arch/x86/lib/barebox.lds.S
index 23d754653..6ee9342f4 100644
--- a/arch/x86/lib/barebox.lds.S
+++ b/arch/x86/lib/barebox.lds.S
@@ -171,6 +171,13 @@ SECTIONS
 		. = ALIGN(4);
 	} > barebox

+	.barebox_ratp_cmd : AT ( LOADADDR(.got) + SIZEOF (.got) ) {
+		__barebox_ratp_cmd_start = .;
+		BAREBOX_RATP_CMDS
+		__barebox_ratp_cmd_end = .;
+		. = ALIGN(4);
+	} > barebox
+
 	.barebox_magicvars : AT ( LOADADDR(.barebox_cmd) + SIZEOF (.barebox_cmd) ) {
 		__barebox_magicvar_start = .;
 		BAREBOX_MAGICVARS
diff --git a/arch/x86/mach-efi/elf_ia32_efi.lds.S b/arch/x86/mach-efi/elf_ia32_efi.lds.S
index 69f43f554..9477aa7d7 100644
--- a/arch/x86/mach-efi/elf_ia32_efi.lds.S
+++ b/arch/x86/mach-efi/elf_ia32_efi.lds.S
@@ -70,6 +70,11 @@ SECTIONS
 	__barebox_cmd : { BAREBOX_CMDS }
 	__barebox_cmd_end = .;

+	. = ALIGN(64);
+	__barebox_ratp_cmd_start = .;
+	__barebox_ratp_cmd : { BAREBOX_RATP_CMDS }
+	__barebox_ratp_cmd_end = .;
+
 	. = ALIGN(4096);
 	.dynamic : { *(.dynamic) }
 	. = ALIGN(4096);
diff --git a/arch/x86/mach-efi/elf_x86_64_efi.lds.S b/arch/x86/mach-efi/elf_x86_64_efi.lds.S
index 93d34d17a..90b6b9f3f 100644
--- a/arch/x86/mach-efi/elf_x86_64_efi.lds.S
+++ b/arch/x86/mach-efi/elf_x86_64_efi.lds.S
@@ -72,6 +72,11 @@ SECTIONS
 	__barebox_cmd : { BAREBOX_CMDS }
 	__barebox_cmd_end = .;

+	. = ALIGN(64);
+	__barebox_ratp_cmd_start = .;
+	__barebox_ratp_cmd : { BAREBOX_RATP_CMDS }
+	__barebox_ratp_cmd_end = .;
+
 	. = ALIGN(4096);
 	.dynamic : { *(.dynamic) }
 	. = ALIGN(4096);
diff --git a/common/module.lds.S b/common/module.lds.S
index a03d04f40..f3dbb12f4 100644
--- a/common/module.lds.S
+++ b/common/module.lds.S
@@ -35,6 +35,8 @@ SECTIONS
 	.got : { *(.got) }

 	.barebox_cmd : { BAREBOX_CMDS }
+	.barebox_ratp_cmd : { BAREBOX_RATP_CMDS }
+
 	. = ALIGN(4);
 	.bss : { *(.bss) }
 }
diff --git a/common/ratp.c b/common/ratp.c
index 80863f81f..b051fdee4 100644
--- a/common/ratp.c
+++ b/common/ratp.c
@@ -31,21 +31,10 @@
 #include <ratp_bb.h>
 #include <fs.h>

-#define BB_RATP_TYPE_COMMAND		1
-#define BB_RATP_TYPE_COMMAND_RETURN	2
-#define BB_RATP_TYPE_CONSOLEMSG		3
-#define BB_RATP_TYPE_PING		4
-#define BB_RATP_TYPE_PONG		5
-#define BB_RATP_TYPE_GETENV		6
-#define BB_RATP_TYPE_GETENV_RETURN	7
-#define BB_RATP_TYPE_FS			8
-#define BB_RATP_TYPE_FS_RETURN		9
-
-struct ratp_bb {
-	uint16_t type;
-	uint16_t flags;
-	uint8_t data[];
-};
+LIST_HEAD(ratp_command_list);
+EXPORT_SYMBOL(ratp_command_list);
+
+#define for_each_ratp_command(cmd) list_for_each_entry(cmd, &ratp_command_list, list)

 struct ratp_bb_command_return {
 	uint32_t errno;
@@ -66,6 +55,51 @@ struct ratp_ctx {
 	struct poller_struct poller;
 };

+static int compare_ratp_command(struct list_head *a, struct list_head *b)
+{
+	int id_a = list_entry(a, struct ratp_command, list)->request_id;
+	int id_b = list_entry(b, struct ratp_command, list)->request_id;
+
+	return (id_a - id_b);
+}
+
+int register_ratp_command(struct ratp_command *cmd)
+{
+	debug("register ratp command: request %hu, response %hu\n",
+	      cmd->request_id, cmd->response_id);
+	list_add_sort(&cmd->list, &ratp_command_list, compare_ratp_command);
+	return 0;
+}
+EXPORT_SYMBOL(register_ratp_command);
+
+struct ratp_command *find_ratp_request(uint16_t request_id)
+{
+	struct ratp_command *cmdtp;
+
+	for_each_ratp_command(cmdtp)
+		if (request_id == cmdtp->request_id)
+			return cmdtp;
+
+	return NULL;	/* not found */
+}
+
+extern struct ratp_command __barebox_ratp_cmd_start;
+extern struct ratp_command __barebox_ratp_cmd_end;
+
+static int init_ratp_command_list(void)
+{
+	struct ratp_command *cmdtp;
+
+	for (cmdtp = &__barebox_ratp_cmd_start;
+			cmdtp != &__barebox_ratp_cmd_end;
+			cmdtp++)
+		register_ratp_command(cmdtp);
+
+	return 0;
+}
+
+late_initcall(init_ratp_command_list);
+
 static int console_recv(struct ratp *r, uint8_t *data)
 {
 	struct ratp_ctx *ctx = container_of(r, struct ratp_ctx, ratp);
@@ -207,8 +241,24 @@ static int ratp_bb_dispatch(struct ratp_ctx *ctx, const void *buf, int len)
 	int dlen = len - sizeof(struct ratp_bb);
 	char *varname;
 	int ret = 0;
+	uint16_t type = be16_to_cpu(rbb->type);
+	struct ratp_command *cmd;
+
+	/* See if there's a command registered to this type */
+	cmd = find_ratp_request(type);
+	if (cmd) {
+		struct ratp_bb *rsp = NULL;
+		int rsp_len = 0;
+
+		ret = cmd->cmd(rbb, len, &rsp, &rsp_len);
+		if (!ret)
+			ret = ratp_send(&ctx->ratp, rsp, rsp_len);
+
+		free(rsp);
+		return ret;
+	}

-	switch (be16_to_cpu(rbb->type)) {
+	switch (type) {
 	case BB_RATP_TYPE_COMMAND:
 		if (ratp_command)
 			return 0;
diff --git a/include/asm-generic/barebox.lds.h b/include/asm-generic/barebox.lds.h
index c8a919b92..74d3ca4a9 100644
--- a/include/asm-generic/barebox.lds.h
+++ b/include/asm-generic/barebox.lds.h
@@ -44,6 +44,8 @@

 #define BAREBOX_CMDS	KEEP(*(SORT_BY_NAME(.barebox_cmd*)))

+#define BAREBOX_RATP_CMDS	KEEP(*(SORT_BY_NAME(.barebox_ratp_cmd*)))
+
 #define BAREBOX_SYMS	KEEP(*(__usymtab))

 #define BAREBOX_MAGICVARS	KEEP(*(SORT_BY_NAME(.barebox_magicvar*)))
diff --git a/include/ratp_bb.h b/include/ratp_bb.h
index f485f7d8a..75aabed55 100644
--- a/include/ratp_bb.h
+++ b/include/ratp_bb.h
@@ -1,6 +1,24 @@
 #ifndef __RATP_BB_H
 #define __RATP_BB_H

+#include <linux/stringify.h>
+
+#define BB_RATP_TYPE_COMMAND		1
+#define BB_RATP_TYPE_COMMAND_RETURN	2
+#define BB_RATP_TYPE_CONSOLEMSG		3
+#define BB_RATP_TYPE_PING		4
+#define BB_RATP_TYPE_PONG		5
+#define BB_RATP_TYPE_GETENV		6
+#define BB_RATP_TYPE_GETENV_RETURN	7
+#define BB_RATP_TYPE_FS			8
+#define BB_RATP_TYPE_FS_RETURN		9
+
+struct ratp_bb {
+	uint16_t type;
+	uint16_t flags;
+	uint8_t data[];
+};
+
 struct ratp_bb_pkt {
 	unsigned int len;
 	uint8_t data[];
@@ -11,4 +29,33 @@ void barebox_ratp_command_run(void);
 int  barebox_ratp_fs_call(struct ratp_bb_pkt *tx, struct ratp_bb_pkt **rx);
 int  barebox_ratp_fs_mount(const char *path);

+/*
+ * RATP commands definition
+ */
+
+struct ratp_command {
+	struct list_head  list;
+	uint16_t          request_id;
+	uint16_t          response_id;
+	int		(*cmd)(const struct ratp_bb *req,
+			       int req_len,
+			       struct ratp_bb **rsp,
+			       int *rsp_len);
+}
+#ifdef __x86_64__
+/* This is required because the linker will put symbols on a 64 bit alignment */
+__attribute__((aligned(64)))
+#endif
+;
+
+#define BAREBOX_RATP_CMD_START(_name)							\
+extern const struct ratp_command __barebox_ratp_cmd_##_name;				\
+const struct ratp_command __barebox_ratp_cmd_##_name					\
+	__attribute__ ((unused,section (".barebox_ratp_cmd_" __stringify(_name)))) = {
+
+#define BAREBOX_RATP_CMD_END								\
+};
+
+int register_ratp_command(struct ratp_command *cmd);
+
 #endif /* __RATP_BB_H */
--
2.15.1

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [RFC PATCH v2 2/8] ratp: moved logic to its own subdirectory
  2018-02-08 13:22 [RFC PATCH v2 0/8] ratp: new generic RATP command support Aleksander Morgado
  2018-02-08 13:22 ` [RFC PATCH v2 1/8] ratp: implement generic " Aleksander Morgado
@ 2018-02-08 13:22 ` Aleksander Morgado
  2018-02-08 13:22 ` [RFC PATCH v2 3/8] ratp: allow building without full console support Aleksander Morgado
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Aleksander Morgado @ 2018-02-08 13:22 UTC (permalink / raw)
  To: barebox; +Cc: Aleksander Morgado

We are going to add new RATP command implementations in separate files
within this subdirectory.

Signed-off-by: Aleksander Morgado <aleksander@aleksander.es>
---
 common/Kconfig           | 13 +------------
 common/Makefile          |  4 ++--
 common/ratp/Kconfig      | 14 ++++++++++++++
 common/ratp/Makefile     |  1 +
 common/{ => ratp}/ratp.c |  0
 5 files changed, 18 insertions(+), 14 deletions(-)
 create mode 100644 common/ratp/Kconfig
 create mode 100644 common/ratp/Makefile
 rename common/{ => ratp}/ratp.c (100%)

diff --git a/common/Kconfig b/common/Kconfig
index 57418cadc..a96c6ef20 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -755,18 +755,7 @@ config PBL_CONSOLE
 	  must be running at the address it's linked at and bss must
 	  be cleared. On ARM that would be after setup_c().

-config CONSOLE_RATP
-	bool
-	select RATP
-	select CRC16
-	select POLLER
-	depends on CONSOLE_FULL
-	prompt "RATP console support"
-	help
-	  This option adds support for remote controlling barebox via serial
-	  port. The regular console is designed for human interaction whereas
-	  this option adds a machine readable interface for controlling barebox.
-	  Say yes here if you want to control barebox from a remote host.
+source common/ratp/Kconfig

 config PARTITION
 	bool
diff --git a/common/Makefile b/common/Makefile
index 8cd0ab300..90d5f19ec 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -46,7 +46,8 @@ obj-$(CONFIG_RESET_SOURCE)	+= reset_source.o
 obj-$(CONFIG_SHELL_HUSH)	+= hush.o
 obj-$(CONFIG_SHELL_SIMPLE)	+= parser.o
 obj-$(CONFIG_STATE)		+= state/
-obj-$(CONFIG_RATP)		+= ratp.o
+obj-$(CONFIG_RATP)		+= ratp/
+obj-$(CONFIG_CONSOLE_RATP)	+= ratp/
 obj-$(CONFIG_BOOTCHOOSER)	+= bootchooser.o
 obj-$(CONFIG_UIMAGE)		+= image.o uimage.o
 obj-$(CONFIG_FITIMAGE)		+= image-fit.o
@@ -60,7 +61,6 @@ obj-$(CONFIG_FILE_LIST)		+= file-list.o
 obj-$(CONFIG_FIRMWARE)		+= firmware.o
 obj-$(CONFIG_UBIFORMAT)		+= ubiformat.o
 obj-$(CONFIG_BAREBOX_UPDATE_IMX_NAND_FCB) += imx-bbu-nand-fcb.o
-obj-$(CONFIG_CONSOLE_RATP)	+= ratp.o
 obj-$(CONFIG_BOOT)		+= boot.o

 quiet_cmd_pwd_h = PWDH    $@
diff --git a/common/ratp/Kconfig b/common/ratp/Kconfig
new file mode 100644
index 000000000..93ff75d64
--- /dev/null
+++ b/common/ratp/Kconfig
@@ -0,0 +1,14 @@
+
+config CONSOLE_RATP
+	bool
+	select RATP
+	select CRC16
+	select POLLER
+	depends on CONSOLE_FULL
+	prompt "RATP console support"
+	help
+	  This option adds support for remote controlling barebox via serial
+	  port. The regular console is designed for human interaction whereas
+	  this option adds a machine readable interface for controlling barebox.
+	  Say yes here if you want to control barebox from a remote host.
+
diff --git a/common/ratp/Makefile b/common/ratp/Makefile
new file mode 100644
index 000000000..cab14c6fb
--- /dev/null
+++ b/common/ratp/Makefile
@@ -0,0 +1 @@
+obj-y += ratp.o
diff --git a/common/ratp.c b/common/ratp/ratp.c
similarity index 100%
rename from common/ratp.c
rename to common/ratp/ratp.c
--
2.15.1

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [RFC PATCH v2 3/8] ratp: allow building without full console support
  2018-02-08 13:22 [RFC PATCH v2 0/8] ratp: new generic RATP command support Aleksander Morgado
  2018-02-08 13:22 ` [RFC PATCH v2 1/8] ratp: implement generic " Aleksander Morgado
  2018-02-08 13:22 ` [RFC PATCH v2 2/8] ratp: moved logic to its own subdirectory Aleksander Morgado
@ 2018-02-08 13:22 ` Aleksander Morgado
  2018-02-08 13:22 ` [RFC PATCH v2 4/8] ratp: implement ping as a standard ratp command Aleksander Morgado
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Aleksander Morgado @ 2018-02-08 13:22 UTC (permalink / raw)
  To: barebox; +Cc: Aleksander Morgado

Make CONFIG_RATP a selectable config option, so that the user can
enable RATP support without explicitly needing to enable the full
console support over RATP (e.g. only for RATP FS or built-in command
support).

The full console can still be explicitly enabled with
CONFIG_CONSOLE_RATP.

Signed-off-by: Aleksander Morgado <aleksander@aleksander.es>
---
 common/Makefile    | 1 -
 common/ratp/ratp.c | 7 +++++--
 fs/Kconfig         | 2 +-
 lib/Kconfig        | 2 +-
 lib/readline.c     | 2 +-
 5 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/common/Makefile b/common/Makefile
index 90d5f19ec..8a0ce84e1 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -47,7 +47,6 @@ obj-$(CONFIG_SHELL_HUSH)	+= hush.o
 obj-$(CONFIG_SHELL_SIMPLE)	+= parser.o
 obj-$(CONFIG_STATE)		+= state/
 obj-$(CONFIG_RATP)		+= ratp/
-obj-$(CONFIG_CONSOLE_RATP)	+= ratp/
 obj-$(CONFIG_BOOTCHOOSER)	+= bootchooser.o
 obj-$(CONFIG_UIMAGE)		+= image.o uimage.o
 obj-$(CONFIG_FITIMAGE)		+= image-fit.o
diff --git a/common/ratp/ratp.c b/common/ratp/ratp.c
index b051fdee4..7c8a2f6f5 100644
--- a/common/ratp/ratp.c
+++ b/common/ratp/ratp.c
@@ -260,7 +260,7 @@ static int ratp_bb_dispatch(struct ratp_ctx *ctx, const void *buf, int len)

 	switch (type) {
 	case BB_RATP_TYPE_COMMAND:
-		if (ratp_command)
+		if (!IS_ENABLED(CONFIG_CONSOLE_RATP) || ratp_command)
 			return 0;

 		ratp_command = xmemdup_add_zero(&rbb->data, dlen);
@@ -274,6 +274,8 @@ static int ratp_bb_dispatch(struct ratp_ctx *ctx, const void *buf, int len)
 		break;

 	case BB_RATP_TYPE_CONSOLEMSG:
+		if (!IS_ENABLED(CONFIG_CONSOLE_RATP))
+			return 0;

 		kfifo_put(ctx->console_recv_fifo, rbb->data, dlen);
 		break;
@@ -420,7 +422,8 @@ static void ratp_poller(struct poller_struct *poller)
 	size_t len;
 	void *buf;

-	ratp_queue_console_tx(ctx);
+	if (IS_ENABLED(CONFIG_CONSOLE_RATP))
+		ratp_queue_console_tx(ctx);

 	ret = ratp_poll(&ctx->ratp);
 	if (ret == -EINTR)
diff --git a/fs/Kconfig b/fs/Kconfig
index 57f2676f4..351200055 100644
--- a/fs/Kconfig
+++ b/fs/Kconfig
@@ -94,7 +94,7 @@ source fs/squashfs/Kconfig

 config FS_RATP
 	bool
-	depends on CONSOLE_RATP
+	depends on RATP
 	prompt "RATP filesystem support"
 	help
 	  This enables support for transferring files over RATP. A host can
diff --git a/lib/Kconfig b/lib/Kconfig
index 9562b1b8c..572985860 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -84,7 +84,7 @@ config STMP_DEVICE

 config RATP
 	select CRC16
-	bool
+	bool "RATP protocol support"
 	help
 	  Reliable Asynchronous Transfer Protocol (RATP) is a protocol for reliably
 	  transferring packets over serial links described in RFC916. This implementation
diff --git a/lib/readline.c b/lib/readline.c
index 1e380abec..904a77639 100644
--- a/lib/readline.c
+++ b/lib/readline.c
@@ -202,7 +202,7 @@ int readline(const char *prompt, char *buf, int len)
 	while (1) {
 		while (!tstc()) {
 			poller_call();
-			if (IS_ENABLED(CONFIG_RATP))
+			if (IS_ENABLED(CONFIG_CONSOLE_RATP))
 				barebox_ratp_command_run();
 		}

--
2.15.1

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [RFC PATCH v2 4/8] ratp: implement ping as a standard ratp command
  2018-02-08 13:22 [RFC PATCH v2 0/8] ratp: new generic RATP command support Aleksander Morgado
                   ` (2 preceding siblings ...)
  2018-02-08 13:22 ` [RFC PATCH v2 3/8] ratp: allow building without full console support Aleksander Morgado
@ 2018-02-08 13:22 ` Aleksander Morgado
  2018-02-08 13:22 ` [RFC PATCH v2 5/8] ratp: implement getenv " Aleksander Morgado
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Aleksander Morgado @ 2018-02-08 13:22 UTC (permalink / raw)
  To: barebox; +Cc: Aleksander Morgado

Signed-off-by: Aleksander Morgado <aleksander@aleksander.es>
---
 common/ratp/Makefile |  1 +
 common/ratp/ping.c   | 40 ++++++++++++++++++++++++++++++++++++++++
 common/ratp/ratp.c   | 24 ------------------------
 3 files changed, 41 insertions(+), 24 deletions(-)
 create mode 100644 common/ratp/ping.c

diff --git a/common/ratp/Makefile b/common/ratp/Makefile
index cab14c6fb..acad61ee5 100644
--- a/common/ratp/Makefile
+++ b/common/ratp/Makefile
@@ -1 +1,2 @@
 obj-y += ratp.o
+obj-y += ping.o
diff --git a/common/ratp/ping.c b/common/ratp/ping.c
new file mode 100644
index 000000000..cc29ea36c
--- /dev/null
+++ b/common/ratp/ping.c
@@ -0,0 +1,40 @@
+/*
+ * Copyright (c) 2018 Sascha Hauer <s.hauer@pengutronix.de>, Pengutronix
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * 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 in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+/*
+ * RATP ping
+ */
+
+#include <common.h>
+#include <ratp_bb.h>
+#include <malloc.h>
+
+static int ratp_cmd_ping(const struct ratp_bb *req, int req_len,
+			 struct ratp_bb **rsp, int *rsp_len)
+{
+	/* Just build response */
+	*rsp_len = sizeof(struct ratp_bb);
+	*rsp = xzalloc(*rsp_len);
+	(*rsp)->type = cpu_to_be16(BB_RATP_TYPE_PONG);
+	return 0;
+}
+
+BAREBOX_RATP_CMD_START(PING)
+	.request_id = BB_RATP_TYPE_PING,
+	.response_id = BB_RATP_TYPE_PONG,
+	.cmd = ratp_cmd_ping
+BAREBOX_RATP_CMD_END
diff --git a/common/ratp/ratp.c b/common/ratp/ratp.c
index 7c8a2f6f5..965c67a0a 100644
--- a/common/ratp/ratp.c
+++ b/common/ratp/ratp.c
@@ -189,25 +189,6 @@ static int ratp_bb_send_command_return(struct ratp_ctx *ctx, uint32_t errno)
 	return ret;
 }

-static int ratp_bb_send_pong(struct ratp_ctx *ctx)
-{
-	void *buf;
-	struct ratp_bb *rbb;
-	int len = sizeof(*rbb);
-	int ret;
-
-	buf = xzalloc(len);
-	rbb = buf;
-
-	rbb->type = cpu_to_be16(BB_RATP_TYPE_PONG);
-
-	ret = ratp_send(&ctx->ratp, buf, len);
-
-	free(buf);
-
-	return ret;
-}
-
 static int ratp_bb_send_getenv_return(struct ratp_ctx *ctx, const char *val)
 {
 	void *buf;
@@ -270,7 +251,6 @@ static int ratp_bb_dispatch(struct ratp_ctx *ctx, const void *buf, int len)
 		break;

 	case BB_RATP_TYPE_COMMAND_RETURN:
-	case BB_RATP_TYPE_PONG:
 		break;

 	case BB_RATP_TYPE_CONSOLEMSG:
@@ -280,10 +260,6 @@ static int ratp_bb_dispatch(struct ratp_ctx *ctx, const void *buf, int len)
 		kfifo_put(ctx->console_recv_fifo, rbb->data, dlen);
 		break;

-	case BB_RATP_TYPE_PING:
-		ret = ratp_bb_send_pong(ctx);
-		break;
-
 	case BB_RATP_TYPE_GETENV:
 		varname = xmemdup_add_zero(&rbb->data, dlen);

--
2.15.1

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [RFC PATCH v2 5/8] ratp: implement getenv as a standard ratp command
  2018-02-08 13:22 [RFC PATCH v2 0/8] ratp: new generic RATP command support Aleksander Morgado
                   ` (3 preceding siblings ...)
  2018-02-08 13:22 ` [RFC PATCH v2 4/8] ratp: implement ping as a standard ratp command Aleksander Morgado
@ 2018-02-08 13:22 ` Aleksander Morgado
  2018-02-08 13:22 ` [RFC PATCH v2 6/8] ratp: use xstrndup() instead of a custom xmemdup_add_zero() Aleksander Morgado
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Aleksander Morgado @ 2018-02-08 13:22 UTC (permalink / raw)
  To: barebox; +Cc: Aleksander Morgado

Also, use xstrndup() instead of the custom xmemdup_add_zero() as we're
working with strings anyway.

Signed-off-by: Aleksander Morgado <aleksander@aleksander.es>
---
 common/ratp/Makefile |  1 +
 common/ratp/getenv.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
 common/ratp/ratp.c   | 30 ------------------------------
 3 files changed, 52 insertions(+), 30 deletions(-)
 create mode 100644 common/ratp/getenv.c

diff --git a/common/ratp/Makefile b/common/ratp/Makefile
index acad61ee5..2fa9d63c0 100644
--- a/common/ratp/Makefile
+++ b/common/ratp/Makefile
@@ -1,2 +1,3 @@
 obj-y += ratp.o
 obj-y += ping.o
+obj-y += getenv.o
diff --git a/common/ratp/getenv.c b/common/ratp/getenv.c
new file mode 100644
index 000000000..b40963488
--- /dev/null
+++ b/common/ratp/getenv.c
@@ -0,0 +1,51 @@
+/*
+ * Copyright (c) 2018 Sascha Hauer <s.hauer@pengutronix.de>, Pengutronix
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * 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 in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+/*
+ * RATP getenv
+ */
+
+#include <common.h>
+#include <ratp_bb.h>
+#include <malloc.h>
+#include <environment.h>
+
+static int ratp_cmd_getenv(const struct ratp_bb *req, int req_len,
+			   struct ratp_bb **rsp, int *rsp_len)
+{
+	int dlen = req_len - sizeof (struct ratp_bb);
+	char *varname;
+	const char *value;
+
+	varname = xstrndup ((const char *)req->data, dlen);
+	value = getenv (varname);
+	free (varname);
+
+	dlen = strlen (value);
+
+	*rsp_len = sizeof(struct ratp_bb) + dlen;
+	*rsp = xzalloc(*rsp_len);
+	(*rsp)->type = cpu_to_be16(BB_RATP_TYPE_GETENV_RETURN);
+	memcpy ((*rsp)->data, value, dlen);
+	return 0;
+}
+
+BAREBOX_RATP_CMD_START(GETENV)
+	.request_id = BB_RATP_TYPE_GETENV,
+	.response_id = BB_RATP_TYPE_GETENV_RETURN,
+	.cmd = ratp_cmd_getenv
+BAREBOX_RATP_CMD_END
diff --git a/common/ratp/ratp.c b/common/ratp/ratp.c
index 965c67a0a..1e9c65179 100644
--- a/common/ratp/ratp.c
+++ b/common/ratp/ratp.c
@@ -189,29 +189,6 @@ static int ratp_bb_send_command_return(struct ratp_ctx *ctx, uint32_t errno)
 	return ret;
 }

-static int ratp_bb_send_getenv_return(struct ratp_ctx *ctx, const char *val)
-{
-	void *buf;
-	struct ratp_bb *rbb;
-	int len, ret;
-
-	if (!val)
-	    val = "";
-
-	len = sizeof(*rbb) + strlen(val);
-	buf = xzalloc(len);
-	rbb = buf;
-	strcpy(rbb->data, val);
-
-	rbb->type = cpu_to_be16(BB_RATP_TYPE_GETENV_RETURN);
-
-	ret = ratp_send(&ctx->ratp, buf, len);
-
-	free(buf);
-
-	return ret;
-}
-
 static char *ratp_command;
 static struct ratp_ctx *ratp_ctx;

@@ -220,7 +197,6 @@ static int ratp_bb_dispatch(struct ratp_ctx *ctx, const void *buf, int len)
 	const struct ratp_bb *rbb = buf;
 	struct ratp_bb_pkt *pkt;
 	int dlen = len - sizeof(struct ratp_bb);
-	char *varname;
 	int ret = 0;
 	uint16_t type = be16_to_cpu(rbb->type);
 	struct ratp_command *cmd;
@@ -260,12 +236,6 @@ static int ratp_bb_dispatch(struct ratp_ctx *ctx, const void *buf, int len)
 		kfifo_put(ctx->console_recv_fifo, rbb->data, dlen);
 		break;

-	case BB_RATP_TYPE_GETENV:
-		varname = xmemdup_add_zero(&rbb->data, dlen);
-
-		ret = ratp_bb_send_getenv_return(ctx, getenv(varname));
-		break;
-
 	case BB_RATP_TYPE_FS_RETURN:
 		pkt = xzalloc(sizeof(*pkt) + dlen);
 		pkt->len = dlen;
--
2.15.1

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [RFC PATCH v2 6/8] ratp: use xstrndup() instead of a custom xmemdup_add_zero()
  2018-02-08 13:22 [RFC PATCH v2 0/8] ratp: new generic RATP command support Aleksander Morgado
                   ` (4 preceding siblings ...)
  2018-02-08 13:22 ` [RFC PATCH v2 5/8] ratp: implement getenv " Aleksander Morgado
@ 2018-02-08 13:22 ` Aleksander Morgado
  2018-02-08 13:23 ` [RFC PATCH v2 7/8] ratp: new md and mw commands Aleksander Morgado
  2018-02-08 13:23 ` [RFC PATCH v2 8/8] ratp: new reset command Aleksander Morgado
  7 siblings, 0 replies; 12+ messages in thread
From: Aleksander Morgado @ 2018-02-08 13:22 UTC (permalink / raw)
  To: barebox; +Cc: Aleksander Morgado

The console operations done via RATP expect strings, so just use
xstrndup() instead of defining a custom method.

Signed-off-by: Aleksander Morgado <aleksander@aleksander.es>
---
 common/ratp/ratp.c | 13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/common/ratp/ratp.c b/common/ratp/ratp.c
index 1e9c65179..fae9cec5b 100644
--- a/common/ratp/ratp.c
+++ b/common/ratp/ratp.c
@@ -132,17 +132,6 @@ static int console_send(struct ratp *r, void *pkt, int len)
 	return 0;
 }

-static void *xmemdup_add_zero(const void *buf, int len)
-{
-	void *ret;
-
-	ret = xzalloc(len + 1);
-	*(uint8_t *)(ret + len) = 0;
-	memcpy(ret, buf, len);
-
-	return ret;
-}
-
 static void ratp_queue_console_tx(struct ratp_ctx *ctx)
 {
 	u8 buf[255];
@@ -220,7 +209,7 @@ static int ratp_bb_dispatch(struct ratp_ctx *ctx, const void *buf, int len)
 		if (!IS_ENABLED(CONFIG_CONSOLE_RATP) || ratp_command)
 			return 0;

-		ratp_command = xmemdup_add_zero(&rbb->data, dlen);
+		ratp_command = xstrndup((const char *)rbb->data, dlen);
 		ratp_ctx = ctx;
 		pr_debug("got command: %s\n", ratp_command);

--
2.15.1

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [RFC PATCH v2 7/8] ratp: new md and mw commands
  2018-02-08 13:22 [RFC PATCH v2 0/8] ratp: new generic RATP command support Aleksander Morgado
                   ` (5 preceding siblings ...)
  2018-02-08 13:22 ` [RFC PATCH v2 6/8] ratp: use xstrndup() instead of a custom xmemdup_add_zero() Aleksander Morgado
@ 2018-02-08 13:23 ` Aleksander Morgado
  2018-02-13  7:55   ` Sascha Hauer
  2018-02-08 13:23 ` [RFC PATCH v2 8/8] ratp: new reset command Aleksander Morgado
  7 siblings, 1 reply; 12+ messages in thread
From: Aleksander Morgado @ 2018-02-08 13:23 UTC (permalink / raw)
  To: barebox; +Cc: Aleksander Morgado

This commit introduces support for running the md and mw commands
using the binary interface provided by RAPT. This allows clients to
read and write memory files without needing to do custom string
parsing on the data returned by the console 'md' and 'mw' operations.

The request and response messages used for these new operations are
structured in the same way:

 * An initial fixed-sized section includes the fixed-sized
   variables (e.g. integers), as well as the size and offset of the
   variable-length variables.

 * After the initial fixed-sized section, the buffer is given, which
   contains the variable-length variables in the offsets previously
   defined and with the size previously defined.

The message also defines separately the offset of the buffer
w.r.t. the start of the message. The endpoint reading the message will
use this information to decide where the buffer starts. This allows to
extend the message format in the future without needing to break the
message API, as new fields can be appended to the fixed-sized section
as long as the buffer offset is also updated to report the new
position of the buffer.

E.g. testing with ratp-barebox-cli:

  $ ratp-barebox-cli -t /dev/ttyUSB2 --md "/dev/pic_eeprom_rdu,0x107,5" --timeout 1000
  Sending md request: read '/dev/pic_eeprom_rdu': 0x0107 (+5 bytes)
  00:00:00:00:00

  $ ratp-barebox-cli -t /dev/ttyUSB2 --mw "/dev/pic_eeprom_rdu,0x107,01:02:03:04:05" --timeout 1000
  Sending mw request: write '/dev/pic_eeprom_rdu': 0x0107 (+5 bytes)
  5/5 bytes written

  $ ratp-barebox-cli -t /dev/ttyUSB2 --md "/dev/pic_eeprom_rdu,0x107,5" --timeout 1000
  Sending md request: read '/dev/pic_eeprom_rdu': 0x0107 (+5 bytes)
  01:02:03:04:05

  $ ratp-barebox-cli -t /dev/ttyUSB2 --mw "/dev/pic_eeprom_rdu,0x107,00:00:00:00:00" --timeout 1000
  Sending mw request: write '/dev/pic_eeprom_rdu': 0x0107 (+5 bytes)
  5/5 bytes written

  $ ratp-barebox-cli -t /dev/ttyUSB2 --md "/dev/pic_eeprom_rdu,0x107,5" --timeout 1000
  Sending md request: read '/dev/pic_eeprom_rdu': 0x0107 (+5 bytes)
  00:00:00:00:00

Signed-off-by: Aleksander Morgado <aleksander@aleksander.es>
---
 common/ratp/Makefile |   2 +
 common/ratp/md.c     | 202 +++++++++++++++++++++++++++++++++++++++++++++++++++
 common/ratp/mw.c     | 173 +++++++++++++++++++++++++++++++++++++++++++
 include/ratp_bb.h    |   4 +
 4 files changed, 381 insertions(+)
 create mode 100644 common/ratp/md.c
 create mode 100644 common/ratp/mw.c

diff --git a/common/ratp/Makefile b/common/ratp/Makefile
index 2fa9d63c0..d4cfdf95f 100644
--- a/common/ratp/Makefile
+++ b/common/ratp/Makefile
@@ -1,3 +1,5 @@
 obj-y += ratp.o
 obj-y += ping.o
 obj-y += getenv.o
+obj-y += md.o
+obj-y += mw.o
diff --git a/common/ratp/md.c b/common/ratp/md.c
new file mode 100644
index 000000000..bbb7a50f0
--- /dev/null
+++ b/common/ratp/md.c
@@ -0,0 +1,202 @@
+/*
+ * Copyright (c) 2011-2018 Sascha Hauer <s.hauer@pengutronix.de>, Pengutronix
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * 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 in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <common.h>
+#include <ratp_bb.h>
+#include <init.h>
+#include <driver.h>
+#include <malloc.h>
+#include <errno.h>
+#include <fs.h>
+#include <libfile.h>
+#include <fcntl.h>
+#include <linux/stat.h>
+#include <xfuncs.h>
+
+/* NOTE:
+ *  - Fixed-size fields (e.g. integers) are given just after the header.
+ *  - Variable-length fields are stored inside the buffer[] and their position
+ *    within the buffer[] and their size are given as fixed-sized fields after
+ *    the header.
+ *  The message may be extended at any time keeping backwards compatibility,
+ *  as the position of the buffer[] is given by the buffer_offset field. i.e.
+ *  increasing the buffer_offset field we can extend the fixed-sized section
+ *  to add more fields.
+ */
+
+struct ratp_bb_md_request {
+	struct ratp_bb header;
+	uint16_t buffer_offset;
+	uint16_t addr;
+	uint16_t size;
+	uint16_t path_size;
+	uint16_t path_offset;
+	uint8_t  buffer[];
+} __attribute__((packed));
+
+struct ratp_bb_md_response {
+	struct ratp_bb header;
+	uint16_t buffer_offset;
+	uint32_t errno;
+	uint16_t data_size;
+	uint16_t data_offset;
+	uint8_t  buffer[];
+} __attribute__((packed));
+
+extern char *mem_rw_buf;
+
+static int do_ratp_mem_md(const char *filename,
+			  loff_t start,
+			  loff_t size,
+			  uint8_t *output)
+{
+	int r, now, t;
+	int ret = 0;
+	int fd;
+	void *map;
+
+	fd = open_and_lseek(filename, O_RWSIZE_1 | O_RDONLY, start);
+	if (fd < 0)
+		return 1;
+
+	map = memmap(fd, PROT_READ);
+	if (map != (void *)-1) {
+		memcpy(output, (uint8_t *)(map + start), size);
+		goto out;
+	}
+
+	t = 0;
+	do {
+		now = min(size, (loff_t)RW_BUF_SIZE);
+		r = read(fd, mem_rw_buf, now);
+		if (r < 0) {
+			ret = -errno;
+			perror("read");
+			goto out;
+		}
+		if (!r)
+			goto out;
+
+		memcpy(output + t, (uint8_t *)(mem_rw_buf), r);
+
+		size  -= r;
+		t     += r;
+	} while (size);
+
+out:
+	close(fd);
+
+	return ret;
+}
+
+static int ratp_cmd_md(const struct ratp_bb *req, int req_len,
+		       struct ratp_bb **rsp, int *rsp_len)
+{
+	struct ratp_bb_md_request *md_req = (struct ratp_bb_md_request *)req;
+	struct ratp_bb_md_response *md_rsp;
+	uint8_t *buffer;
+	uint16_t buffer_offset;
+	uint16_t buffer_size;
+	int md_rsp_len;
+	uint16_t addr;
+	uint16_t size;
+	uint16_t path_size;
+	uint16_t path_offset;
+	char *path = NULL;
+	int ret = 0;
+
+	/* At least message header should be valid */
+	if (req_len < sizeof(*md_req)) {
+		printf("ratp md ignored: size mismatch (%d < %zu)\n",
+		       req_len, sizeof (*md_req));
+		ret = -EINVAL;
+		goto out;
+	}
+
+	/* Validate buffer position and size */
+	buffer_offset = be16_to_cpu(md_req->buffer_offset);
+	if (req_len < buffer_offset) {
+		printf("ratp md ignored: invalid buffer offset (%d < %hu)\n",
+		       req_len, buffer_offset);
+		ret = -EINVAL;
+		goto out;
+	}
+	buffer_size = req_len - buffer_offset;
+	buffer = ((uint8_t *)md_req) + buffer_offset;
+
+	/* Validate path position and size */
+	path_offset = be16_to_cpu(md_req->path_offset);
+	if (path_offset != 0) {
+		printf("ratp md ignored: invalid path offset\n");
+		ret = -EINVAL;
+		goto out;
+	}
+	path_size = be16_to_cpu(md_req->path_size);
+	if (!path_size) {
+		printf("ratp md ignored: no filepath given\n");
+		ret = -EINVAL;
+		goto out;
+	}
+
+	/* Validate buffer size */
+	if (buffer_size < path_size) {
+		printf("ratp mw ignored: size mismatch (%d < %zu): path may not be fully given\n",
+		       req_len, path_size);
+		ret = -EINVAL;
+		goto out;
+	}
+
+	addr = be16_to_cpu (md_req->addr);
+	size = be16_to_cpu (md_req->size);
+	path = xstrndup((const char *)&buffer[path_offset], path_size);
+
+out:
+	/* Avoid reading anything on error */
+	if (ret != 0)
+		size = 0;
+
+	md_rsp_len = sizeof(*md_rsp) + size;
+	md_rsp = xzalloc(md_rsp_len);
+	md_rsp->header.type = cpu_to_be16(BB_RATP_TYPE_MD_RETURN);
+	md_rsp->buffer_offset = cpu_to_be16(sizeof(*md_rsp));
+	md_rsp->data_offset = 0;
+
+	/* Don't read anything on error or if 0 bytes were requested */
+	if (size > 0)
+		ret = do_ratp_mem_md(path, addr, size, md_rsp->buffer);
+
+	if (ret != 0) {
+		md_rsp->data_size = 0;
+		md_rsp->errno = cpu_to_be32(ret);
+		md_rsp_len = sizeof(*md_rsp);
+	} else {
+		md_rsp->data_size = cpu_to_be16(size);
+		md_rsp->errno = 0;
+	}
+
+	*rsp = (struct ratp_bb *)md_rsp;
+	*rsp_len = md_rsp_len;
+
+	free (path);
+	return ret;
+}
+
+BAREBOX_RATP_CMD_START(MD)
+	.request_id = BB_RATP_TYPE_MD,
+	.response_id = BB_RATP_TYPE_MD_RETURN,
+	.cmd = ratp_cmd_md
+BAREBOX_RATP_CMD_END
diff --git a/common/ratp/mw.c b/common/ratp/mw.c
new file mode 100644
index 000000000..7fbcde75a
--- /dev/null
+++ b/common/ratp/mw.c
@@ -0,0 +1,173 @@
+/*
+ * Copyright (c) 2011-2018 Sascha Hauer <s.hauer@pengutronix.de>, Pengutronix
+ * Copyright (c) 2018 Zodiac Inflight Innovations
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * 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 in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <common.h>
+#include <ratp_bb.h>
+#include <init.h>
+#include <driver.h>
+#include <malloc.h>
+#include <errno.h>
+#include <fs.h>
+#include <libfile.h>
+#include <fcntl.h>
+#include <xfuncs.h>
+
+/* NOTE:
+ *  - Fixed-size fields (e.g. integers) are given just after the header.
+ *  - Variable-length fields are stored inside the buffer[] and their position
+ *    within the buffer[] and their size are given as fixed-sized fields after
+ *    the header.
+ *  The message may be extended at any time keeping backwards compatibility,
+ *  as the position of the buffer[] is given by the buffer_offset field. i.e.
+ *  increasing the buffer_offset field we can extend the fixed-sized section
+ *  to add more fields.
+ */
+
+struct ratp_bb_mw_request {
+	struct ratp_bb header;
+	uint16_t buffer_offset;
+	uint16_t addr;
+	uint16_t path_size;
+	uint16_t path_offset;
+	uint16_t data_size;
+	uint16_t data_offset;
+	uint8_t  buffer[];
+} __attribute__((packed));
+
+struct ratp_bb_mw_response {
+	struct ratp_bb header;
+	uint16_t buffer_offset;
+	uint32_t errno;
+	uint32_t written;
+	uint8_t  buffer[];
+} __attribute__((packed));
+
+static int ratp_cmd_mw(const struct ratp_bb *req, int req_len,
+		       struct ratp_bb **rsp, int *rsp_len)
+{
+	struct ratp_bb_mw_request *mw_req = (struct ratp_bb_mw_request *)req;
+	struct ratp_bb_mw_response *mw_rsp;
+	uint8_t *buffer;
+	uint16_t buffer_offset;
+	uint16_t buffer_size;
+	uint16_t addr;
+	uint16_t path_size;
+	uint16_t path_offset;
+	uint16_t data_size;
+	uint16_t data_offset;
+	ssize_t written = 0;
+	char *path = NULL;
+	int fd;
+	int ret = 0;
+
+	/* At least message header should be valid */
+	if (req_len < sizeof(*mw_req)) {
+		printf("ratp mw ignored: size mismatch (%d < %zu)\n",
+		       req_len, sizeof (*mw_req));
+		ret = -EINVAL;
+		goto out;
+	}
+
+	/* Validate buffer position and size */
+	buffer_offset = be16_to_cpu(mw_req->buffer_offset);
+	if (req_len < buffer_offset) {
+		printf("ratp mw ignored: invalid buffer offset (%d < %hu)\n",
+		       req_len, buffer_offset);
+		ret = -EINVAL;
+		goto out;
+	}
+	buffer_size = req_len - buffer_offset;
+	buffer = ((uint8_t *)mw_req) + buffer_offset;
+
+	/* Validate path position and size */
+	path_offset = be16_to_cpu(mw_req->path_offset);
+	if (path_offset != 0) {
+		printf("ratp mw ignored: invalid path offset\n");
+		ret = -EINVAL;
+		goto out;
+	}
+	path_size = be16_to_cpu(mw_req->path_size);
+	if (!path_size) {
+		printf("ratp mw ignored: no filepath given\n");
+		ret = -EINVAL;
+		goto out;
+	}
+
+	/* Validate data position and size */
+	data_offset = be16_to_cpu(mw_req->data_offset);
+	if (data_offset != (path_offset + path_size)) {
+		printf("ratp mw ignored: invalid path offset\n");
+		ret = -EINVAL;
+		goto out;
+	}
+	data_size = be16_to_cpu(mw_req->data_size);
+	if (!data_size) {
+		/* Success */
+		goto out;
+	}
+
+	/* Validate buffer size */
+	if (buffer_size < (path_size + data_size)) {
+		printf("ratp mw ignored: size mismatch (%d < %zu): path or data not be fully given\n",
+		       req_len, path_size + data_size);
+		ret = -EINVAL;
+		goto out;
+	}
+
+	addr = be16_to_cpu (mw_req->addr);
+	path = xstrndup((const char *)&buffer[path_offset], path_size);
+
+	fd = open_and_lseek(path, O_RWSIZE_1 | O_WRONLY, addr);
+	if (fd < 0) {
+		ret = -errno;
+		goto out;
+	}
+
+	written = write(fd, &buffer[data_offset], data_size);
+	if (written < 0) {
+		ret = -errno;
+		perror("write");
+	}
+
+	close(fd);
+
+out:
+	mw_rsp = xzalloc(sizeof(*mw_rsp));
+	mw_rsp->header.type = cpu_to_be16(BB_RATP_TYPE_MW_RETURN);
+	mw_rsp->buffer_offset = cpu_to_be16(sizeof(*mw_rsp)); /* n/a */
+
+	if (ret != 0) {
+		mw_rsp->written = 0;
+		mw_rsp->errno = cpu_to_be32(ret);
+	} else {
+		mw_rsp->written = cpu_to_be16((uint16_t)written);
+		mw_rsp->errno = 0;
+	}
+
+	*rsp = (struct ratp_bb *)mw_rsp;
+	*rsp_len = sizeof(*mw_rsp);
+
+	free (path);
+	return ret;
+}
+
+BAREBOX_RATP_CMD_START(MW)
+	.request_id = BB_RATP_TYPE_MW,
+	.response_id = BB_RATP_TYPE_MW_RETURN,
+	.cmd = ratp_cmd_mw
+BAREBOX_RATP_CMD_END
diff --git a/include/ratp_bb.h b/include/ratp_bb.h
index 75aabed55..00b165f77 100644
--- a/include/ratp_bb.h
+++ b/include/ratp_bb.h
@@ -12,6 +12,10 @@
 #define BB_RATP_TYPE_GETENV_RETURN	7
 #define BB_RATP_TYPE_FS			8
 #define BB_RATP_TYPE_FS_RETURN		9
+#define BB_RATP_TYPE_MD			10
+#define BB_RATP_TYPE_MD_RETURN		11
+#define BB_RATP_TYPE_MW			12
+#define BB_RATP_TYPE_MW_RETURN		13

 struct ratp_bb {
 	uint16_t type;
--
2.15.1

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [RFC PATCH v2 8/8] ratp: new reset command
  2018-02-08 13:22 [RFC PATCH v2 0/8] ratp: new generic RATP command support Aleksander Morgado
                   ` (6 preceding siblings ...)
  2018-02-08 13:23 ` [RFC PATCH v2 7/8] ratp: new md and mw commands Aleksander Morgado
@ 2018-02-08 13:23 ` Aleksander Morgado
  7 siblings, 0 replies; 12+ messages in thread
From: Aleksander Morgado @ 2018-02-08 13:23 UTC (permalink / raw)
  To: barebox; +Cc: Aleksander Morgado

Signed-off-by: Aleksander Morgado <aleksander@aleksander.es>
---
 common/ratp/Makefile |  1 +
 common/ratp/reset.c  | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 include/ratp_bb.h    |  1 +
 3 files changed, 57 insertions(+)
 create mode 100644 common/ratp/reset.c

diff --git a/common/ratp/Makefile b/common/ratp/Makefile
index d4cfdf95f..2c6d674f6 100644
--- a/common/ratp/Makefile
+++ b/common/ratp/Makefile
@@ -3,3 +3,4 @@ obj-y += ping.o
 obj-y += getenv.o
 obj-y += md.o
 obj-y += mw.o
+obj-y += reset.o
diff --git a/common/ratp/reset.c b/common/ratp/reset.c
new file mode 100644
index 000000000..ca8be4e62
--- /dev/null
+++ b/common/ratp/reset.c
@@ -0,0 +1,55 @@
+/*
+ * reset.c - reset the cpu
+ *
+ * Copyright (c) 2007 Sascha Hauer <s.hauer@pengutronix.de>, Pengutronix
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * 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 in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include <common.h>
+#include <command.h>
+#include <ratp_bb.h>
+#include <complete.h>
+#include <getopt.h>
+#include <restart.h>
+
+struct ratp_bb_reset {
+	struct ratp_bb header;
+	uint8_t        force;
+} __attribute__((packed));
+
+static int ratp_cmd_reset(const struct ratp_bb *req, int req_len,
+			  struct ratp_bb **rsp, int *rsp_len)
+{
+	struct ratp_bb_reset *reset_req = (struct ratp_bb_reset *)req;
+
+	if (req_len < sizeof (*reset_req)) {
+		printf ("ratp reset ignored: size mismatch (%d < %zu)\n", req_len, sizeof (*reset_req));
+		return 2;
+	}
+
+	debug("running reset %s\n", reset_req->force ? "(forced)" : "");
+
+	if (!reset_req->force)
+		shutdown_barebox();
+
+	restart_machine();
+	/* Not reached */
+	return 1;
+}
+
+BAREBOX_RATP_CMD_START(RESET)
+	.request_id = BB_RATP_TYPE_RESET,
+	.cmd = ratp_cmd_reset
+BAREBOX_RATP_CMD_END
diff --git a/include/ratp_bb.h b/include/ratp_bb.h
index 00b165f77..3a80cf6ae 100644
--- a/include/ratp_bb.h
+++ b/include/ratp_bb.h
@@ -16,6 +16,7 @@
 #define BB_RATP_TYPE_MD_RETURN		11
 #define BB_RATP_TYPE_MW			12
 #define BB_RATP_TYPE_MW_RETURN		13
+#define BB_RATP_TYPE_RESET		14

 struct ratp_bb {
 	uint16_t type;
--
2.15.1

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC PATCH v2 7/8] ratp: new md and mw commands
  2018-02-08 13:23 ` [RFC PATCH v2 7/8] ratp: new md and mw commands Aleksander Morgado
@ 2018-02-13  7:55   ` Sascha Hauer
  2018-02-21 13:10     ` Aleksander Morgado
  0 siblings, 1 reply; 12+ messages in thread
From: Sascha Hauer @ 2018-02-13  7:55 UTC (permalink / raw)
  To: Aleksander Morgado; +Cc: barebox

On Thu, Feb 08, 2018 at 02:23:00PM +0100, Aleksander Morgado wrote:
> This commit introduces support for running the md and mw commands
> using the binary interface provided by RAPT. This allows clients to

s/RAPT/RATP/

> read and write memory files without needing to do custom string
> parsing on the data returned by the console 'md' and 'mw' operations.
> 
> The request and response messages used for these new operations are
> structured in the same way:
> 
>  * An initial fixed-sized section includes the fixed-sized
>    variables (e.g. integers), as well as the size and offset of the
>    variable-length variables.
> 
>  * After the initial fixed-sized section, the buffer is given, which
>    contains the variable-length variables in the offsets previously
>    defined and with the size previously defined.
> 
> The message also defines separately the offset of the buffer
> w.r.t. the start of the message. The endpoint reading the message will
> use this information to decide where the buffer starts. This allows to
> extend the message format in the future without needing to break the
> message API, as new fields can be appended to the fixed-sized section
> as long as the buffer offset is also updated to report the new
> position of the buffer.
> 
> E.g. testing with ratp-barebox-cli:
> 
>   $ ratp-barebox-cli -t /dev/ttyUSB2 --md "/dev/pic_eeprom_rdu,0x107,5" --timeout 1000
>   Sending md request: read '/dev/pic_eeprom_rdu': 0x0107 (+5 bytes)
>   00:00:00:00:00

It would be good to have to pointer to libratp and ratp-barebox-cli in
Documentation/user/remote-control.rst.

What's your plan for the bbremote tool? It's a bit unfortunate to have
the new features only available in an external tool.

> +static int do_ratp_mem_md(const char *filename,
> +			  loff_t start,
> +			  loff_t size,
> +			  uint8_t *output)
> +{
> +	int r, now, t;
> +	int ret = 0;
> +	int fd;
> +	void *map;
> +
> +	fd = open_and_lseek(filename, O_RWSIZE_1 | O_RDONLY, start);

It would be nice to support different read/write widths. Not every
memory is readable in byte size chunks. But this could be added later
aswell, I just saw that the way you defined the messages allows it to
add additional fields later.

> +	if (fd < 0)
> +		return 1;

Is '1' the right return value here? It goes into a variable named
'errno' which looks wrong.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC PATCH v2 7/8] ratp: new md and mw commands
  2018-02-13  7:55   ` Sascha Hauer
@ 2018-02-21 13:10     ` Aleksander Morgado
  2018-02-22  7:59       ` Sascha Hauer
  0 siblings, 1 reply; 12+ messages in thread
From: Aleksander Morgado @ 2018-02-21 13:10 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox

>> read and write memory files without needing to do custom string
>> parsing on the data returned by the console 'md' and 'mw' operations.
>>
>> The request and response messages used for these new operations are
>> structured in the same way:
>>
>>  * An initial fixed-sized section includes the fixed-sized
>>    variables (e.g. integers), as well as the size and offset of the
>>    variable-length variables.
>>
>>  * After the initial fixed-sized section, the buffer is given, which
>>    contains the variable-length variables in the offsets previously
>>    defined and with the size previously defined.
>>
>> The message also defines separately the offset of the buffer
>> w.r.t. the start of the message. The endpoint reading the message will
>> use this information to decide where the buffer starts. This allows to
>> extend the message format in the future without needing to break the
>> message API, as new fields can be appended to the fixed-sized section
>> as long as the buffer offset is also updated to report the new
>> position of the buffer.
>>
>> E.g. testing with ratp-barebox-cli:
>>
>>   $ ratp-barebox-cli -t /dev/ttyUSB2 --md "/dev/pic_eeprom_rdu,0x107,5" --timeout 1000
>>   Sending md request: read '/dev/pic_eeprom_rdu': 0x0107 (+5 bytes)
>>   00:00:00:00:00
>
> It would be good to have to pointer to libratp and ratp-barebox-cli in
> Documentation/user/remote-control.rst.
>

I'll add the info, ok.

> What's your plan for the bbremote tool? It's a bit unfortunate to have
> the new features only available in an external tool.
>

Yeah, I knew you were going to say that :) So, don't know. Didn't want
to spend much time on it because the new commands (md, mw, reset)
could directly be run with bbremote as "bbremote run ..." and you
would get the same output just with a different format. The benefit of
the binary API is clear in libratp-barebox, i.e. to integrate it into
applications that would make use of those operations without requiring
formatting output for the human eye. The ratp-barebox-cli and bbremote
support of the commands with the binary API would just be a
convenience. I actually only developed the support for the new
commands in the cli to make sure the library worked, as a way of
testing it. That said, if you want I can try to implement them in
bbremote as well and provide the same kind of output that you'd see in
ratp-barebox-cli; it would at least be a way of testing the API
directly within barebox without requiring any external tool.

>> +static int do_ratp_mem_md(const char *filename,
>> +                       loff_t start,
>> +                       loff_t size,
>> +                       uint8_t *output)
>> +{
>> +     int r, now, t;
>> +     int ret = 0;
>> +     int fd;
>> +     void *map;
>> +
>> +     fd = open_and_lseek(filename, O_RWSIZE_1 | O_RDONLY, start);
>
> It would be nice to support different read/write widths. Not every
> memory is readable in byte size chunks. But this could be added later
> aswell, I just saw that the way you defined the messages allows it to
> add additional fields later.

Yes, the binary message API should allow extending the format with
additional fields. But now that you said that, I may actually include
the width in the format, will check that.

>> +     if (fd < 0)
>> +             return 1;
>
> Is '1' the right return value here? It goes into a variable named
> 'errno' which looks wrong.
>

Oh, probably not. I'll review that.

-- 
Aleksander
https://aleksander.es

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC PATCH v2 7/8] ratp: new md and mw commands
  2018-02-21 13:10     ` Aleksander Morgado
@ 2018-02-22  7:59       ` Sascha Hauer
  0 siblings, 0 replies; 12+ messages in thread
From: Sascha Hauer @ 2018-02-22  7:59 UTC (permalink / raw)
  To: Aleksander Morgado; +Cc: barebox

On Wed, Feb 21, 2018 at 02:10:09PM +0100, Aleksander Morgado wrote:
> >> read and write memory files without needing to do custom string
> >> parsing on the data returned by the console 'md' and 'mw' operations.
> >>
> >> The request and response messages used for these new operations are
> >> structured in the same way:
> >>
> >>  * An initial fixed-sized section includes the fixed-sized
> >>    variables (e.g. integers), as well as the size and offset of the
> >>    variable-length variables.
> >>
> >>  * After the initial fixed-sized section, the buffer is given, which
> >>    contains the variable-length variables in the offsets previously
> >>    defined and with the size previously defined.
> >>
> >> The message also defines separately the offset of the buffer
> >> w.r.t. the start of the message. The endpoint reading the message will
> >> use this information to decide where the buffer starts. This allows to
> >> extend the message format in the future without needing to break the
> >> message API, as new fields can be appended to the fixed-sized section
> >> as long as the buffer offset is also updated to report the new
> >> position of the buffer.
> >>
> >> E.g. testing with ratp-barebox-cli:
> >>
> >>   $ ratp-barebox-cli -t /dev/ttyUSB2 --md "/dev/pic_eeprom_rdu,0x107,5" --timeout 1000
> >>   Sending md request: read '/dev/pic_eeprom_rdu': 0x0107 (+5 bytes)
> >>   00:00:00:00:00
> >
> > It would be good to have to pointer to libratp and ratp-barebox-cli in
> > Documentation/user/remote-control.rst.
> >
> 
> I'll add the info, ok.
> 
> > What's your plan for the bbremote tool? It's a bit unfortunate to have
> > the new features only available in an external tool.
> >
> 
> Yeah, I knew you were going to say that :) So, don't know. Didn't want
> to spend much time on it because the new commands (md, mw, reset)
> could directly be run with bbremote as "bbremote run ..." and you
> would get the same output just with a different format. The benefit of
> the binary API is clear in libratp-barebox, i.e. to integrate it into
> applications that would make use of those operations without requiring
> formatting output for the human eye. The ratp-barebox-cli and bbremote
> support of the commands with the binary API would just be a
> convenience. I actually only developed the support for the new
> commands in the cli to make sure the library worked, as a way of
> testing it. That said, if you want I can try to implement them in
> bbremote as well and provide the same kind of output that you'd see in
> ratp-barebox-cli; it would at least be a way of testing the API
> directly within barebox without requiring any external tool.

If that isn't too much work then this would be great.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2018-02-22  7:59 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-08 13:22 [RFC PATCH v2 0/8] ratp: new generic RATP command support Aleksander Morgado
2018-02-08 13:22 ` [RFC PATCH v2 1/8] ratp: implement generic " Aleksander Morgado
2018-02-08 13:22 ` [RFC PATCH v2 2/8] ratp: moved logic to its own subdirectory Aleksander Morgado
2018-02-08 13:22 ` [RFC PATCH v2 3/8] ratp: allow building without full console support Aleksander Morgado
2018-02-08 13:22 ` [RFC PATCH v2 4/8] ratp: implement ping as a standard ratp command Aleksander Morgado
2018-02-08 13:22 ` [RFC PATCH v2 5/8] ratp: implement getenv " Aleksander Morgado
2018-02-08 13:22 ` [RFC PATCH v2 6/8] ratp: use xstrndup() instead of a custom xmemdup_add_zero() Aleksander Morgado
2018-02-08 13:23 ` [RFC PATCH v2 7/8] ratp: new md and mw commands Aleksander Morgado
2018-02-13  7:55   ` Sascha Hauer
2018-02-21 13:10     ` Aleksander Morgado
2018-02-22  7:59       ` Sascha Hauer
2018-02-08 13:23 ` [RFC PATCH v2 8/8] ratp: new reset command Aleksander Morgado

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox