mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [RFC PATCH 00/10] ratp: new generic RATP command support
@ 2018-02-02 11:14 Aleksander Morgado
  2018-02-02 11:14 ` [PATCH 01/10] ratp: define message type flags Aleksander Morgado
                   ` (10 more replies)
  0 siblings, 11 replies; 20+ messages in thread
From: Aleksander Morgado @ 2018-02-02 11:14 UTC (permalink / raw)
  To: barebox; +Cc: Aleksander Morgado

Until now, the barebox-specific RATP commands were all defined and
implemented in common/ratp.c. This series of patches allow ratp
commands to be defined in a similar way to console commands.

The first patches (1-5) break the current RATP API, by introducing
the concept of requests, responses and indications:
 * Requests sent to one endpoint are expected to be replied with
   a response by the peer endpoint.
 * Indications are messages sent from one endpoint to another which
   are not expected to be replied.

The current RATP operations are reformatted using this approach, by
specifying the message type in the until now unused 'flags' field of
the RATP barebox message, and making all messages of the same
operation share the same command id.

The next patches (6-8) add support to specifying RATP commands in
separate implementation files, defined with some new helper
BAREBOX_RATP_CMD_START/BAREBOX_RATP_CMD_END macros. The getenv and
ping commands are updated to use this new approach.

The last patches (9-10) implement three new commands via RATP: reset,
md and mw. Both md and mw operations are defined by a binary API, and
allow reading/writing memory without needing to do any kind of
parsing (as it was the case when e.g. running the md or mw console
commands).

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

What do you think of these changes? The initial RATP API break is bad
but not sure how many other RATP API users are around except for
bbremote (ported along with the changes) and the libratp-barebox I'm
writing.

Aleksander Morgado (10):
  ratp: define message type flags
  ratp: port command operation to req/rsp/ind format
  ratp: port ping operation to req/rsp format
  ratp: port getenv operation to req/rsp format
  ratp: port filesystem operation to req/rsp format
  ratp: implement generic command support
  ratp: implement ping as a standard ratp command
  ratp: implement getenv as a standard ratp command
  ratp: new reset command
  ratp: new md and mw commands

 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 +
 commands/Makefile                         |   2 +
 commands/md.c                             | 209 ++++++++++++++++++----
 commands/mw.c                             | 150 +++++++++++++++-
 commands/ratp-getenv.c                    |  50 ++++++
 commands/ratp-ping.c                      |  38 ++++
 commands/reset.c                          |  48 ++++-
 common/module.lds.S                       |   2 +
 common/ratp.c                             | 283 +++++++++++++++---------------
 include/asm-generic/barebox.lds.h         |   2 +
 include/ratp_bb.h                         |  49 ++++++
 scripts/remote/controller.py              |  71 ++++----
 scripts/remote/messages.py                |  90 ++++++----
 scripts/remote/ratpfs.py                  |   6 +-
 25 files changed, 800 insertions(+), 256 deletions(-)
 create mode 100644 commands/ratp-getenv.c
 create mode 100644 commands/ratp-ping.c

-- 
2.15.1


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

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

* [PATCH 01/10] ratp: define message type flags
  2018-02-02 11:14 [RFC PATCH 00/10] ratp: new generic RATP command support Aleksander Morgado
@ 2018-02-02 11:14 ` Aleksander Morgado
  2018-02-02 11:14 ` [PATCH 02/10] ratp: port command operation to req/rsp/ind format Aleksander Morgado
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Aleksander Morgado @ 2018-02-02 11:14 UTC (permalink / raw)
  To: barebox; +Cc: Aleksander Morgado

Split message types in 3 different groups:

  * Requests: messages generated by one RATP endpoint and sent to the
    the other endpoint to be processed.

  * Responses: messages generated by the RATP endpoint as a result of
    having received and processed a specific request message.

  * Indications: messages generated by one RATP endpoint for which
    there is no need to generate an explicit response message.

These message types are identified by new command flags.

Signed-off-by: Aleksander Morgado <aleksander@aleksander.es>
---
 common/ratp.c              | 4 ++++
 scripts/remote/messages.py | 5 +++++
 2 files changed, 9 insertions(+)

diff --git a/common/ratp.c b/common/ratp.c
index 80863f81f..a1fa6fd5f 100644
--- a/common/ratp.c
+++ b/common/ratp.c
@@ -41,6 +41,10 @@
 #define BB_RATP_TYPE_FS			8
 #define BB_RATP_TYPE_FS_RETURN		9
 
+#define BB_RATP_FLAG_NONE		0
+#define BB_RATP_FLAG_RESPONSE		(1 << 0) /* Packet is a response */
+#define BB_RATP_FLAG_INDICATION		(1 << 1) /* Packet is an indication */
+
 struct ratp_bb {
 	uint16_t type;
 	uint16_t flags;
diff --git a/scripts/remote/messages.py b/scripts/remote/messages.py
index 8e8495b12..7a597bc9d 100644
--- a/scripts/remote/messages.py
+++ b/scripts/remote/messages.py
@@ -5,6 +5,11 @@ from __future__ import absolute_import, division, print_function
 
 import struct
 
+class BBFlag(object):
+    none = 0
+    response = 1 << 0
+    indication = 1 << 1
+
 
 class BBType(object):
     command = 1
-- 
2.15.1


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

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

* [PATCH 02/10] ratp: port command operation to req/rsp/ind format
  2018-02-02 11:14 [RFC PATCH 00/10] ratp: new generic RATP command support Aleksander Morgado
  2018-02-02 11:14 ` [PATCH 01/10] ratp: define message type flags Aleksander Morgado
@ 2018-02-02 11:14 ` Aleksander Morgado
  2018-02-02 11:14 ` [PATCH 03/10] ratp: port ping operation to req/rsp format Aleksander Morgado
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Aleksander Morgado @ 2018-02-02 11:14 UTC (permalink / raw)
  To: barebox; +Cc: Aleksander Morgado

The commands executed by the client via RATP are processed in the
following way:

 * The client sends a 'command' packet to the barebox console.
 * The result (errno) of the command is returned to the client via a
   'command_return' message.
 * The standard output of the command executed is sent to the client
   via 'consolemsg' packets.

If the client doesn't need any explicit response to the command sent
(e.g. when simulating a console in the client side), it may also just
do the following:

 * The client sends 'consolemsg' packets to the barebox console.

We now consolidate this process using the request, response and
indication packet flags, and making them part of the same 'console'
packet type. In this specific message type, indications may be sent in
both directions.

Signed-off-by: Aleksander Morgado <aleksander@aleksander.es>
---
 common/ratp.c                | 28 +++++++++++++++-------------
 scripts/remote/controller.py | 32 ++++++++++++++++----------------
 scripts/remote/messages.py   | 28 +++++++++++++++-------------
 3 files changed, 46 insertions(+), 42 deletions(-)

diff --git a/common/ratp.c b/common/ratp.c
index a1fa6fd5f..7d7fb5fcd 100644
--- a/common/ratp.c
+++ b/common/ratp.c
@@ -31,9 +31,7 @@
 #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_CONSOLE		1
 #define BB_RATP_TYPE_PING		4
 #define BB_RATP_TYPE_PONG		5
 #define BB_RATP_TYPE_GETENV		6
@@ -120,7 +118,8 @@ static void ratp_queue_console_tx(struct ratp_ctx *ctx)
 	unsigned int now, maxlen = 255 - sizeof(*rbb);
 	int ret;
 
-	rbb->type = cpu_to_be16(BB_RATP_TYPE_CONSOLEMSG);
+	rbb->type = cpu_to_be16(BB_RATP_TYPE_CONSOLE);
+	rbb->flags = cpu_to_be16(BB_RATP_FLAG_INDICATION);
 
 	while (1) {
 		now = min(maxlen, kfifo_len(ctx->console_transmit_fifo));
@@ -149,7 +148,8 @@ static int ratp_bb_send_command_return(struct ratp_ctx *ctx, uint32_t errno)
 	rbb = buf;
 	rbb_ret = buf + sizeof(*rbb);
 
-	rbb->type = cpu_to_be16(BB_RATP_TYPE_COMMAND_RETURN);
+	rbb->type = cpu_to_be16(BB_RATP_TYPE_CONSOLE);
+	rbb->flags = cpu_to_be16(BB_RATP_FLAG_RESPONSE);
 	rbb_ret->errno = cpu_to_be32(errno);
 
 	ret = ratp_send(&ctx->ratp, buf, len);
@@ -211,27 +211,29 @@ 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 flags = be16_to_cpu(rbb->flags);
 
 	switch (be16_to_cpu(rbb->type)) {
-	case BB_RATP_TYPE_COMMAND:
+	case BB_RATP_TYPE_CONSOLE:
+		if (flags & BB_RATP_FLAG_RESPONSE)
+			break;
+
+		if (flags & BB_RATP_FLAG_INDICATION) {
+			kfifo_put(ctx->console_recv_fifo, rbb->data, dlen);
+			break;
+		}
+
 		if (ratp_command)
 			return 0;
 
 		ratp_command = xmemdup_add_zero(&rbb->data, dlen);
 		ratp_ctx = ctx;
 		pr_debug("got command: %s\n", ratp_command);
-
 		break;
 
-	case BB_RATP_TYPE_COMMAND_RETURN:
 	case BB_RATP_TYPE_PONG:
 		break;
 
-	case BB_RATP_TYPE_CONSOLEMSG:
-
-		kfifo_put(ctx->console_recv_fifo, rbb->data, dlen);
-		break;
-
 	case BB_RATP_TYPE_PING:
 		ret = ratp_bb_send_pong(ctx);
 		break;
diff --git a/scripts/remote/controller.py b/scripts/remote/controller.py
index a7257ecc9..c3f29334a 100644
--- a/scripts/remote/controller.py
+++ b/scripts/remote/controller.py
@@ -20,17 +20,17 @@ except:
 
 
 def unpack(data):
-    p_type, = struct.unpack("!H", data[:2])
-    logging.debug("unpack: %r data=%r", p_type, repr(data))
-    if p_type == BBType.command:
-        logging.debug("received: command")
-        return BBPacketCommand(raw=data)
-    elif p_type == BBType.command_return:
-        logging.debug("received: command_return")
-        return BBPacketCommandReturn(raw=data)
-    elif p_type == BBType.consolemsg:
-        logging.debug("received: consolemsg")
-        return BBPacketConsoleMsg(raw=data)
+    p_type, p_flag = struct.unpack("!HH", data[:4])
+    logging.debug("unpack: type %r flag %r data=%r", p_type, p_flag, repr(data))
+    if p_type == BBType.console:
+        if p_flag & BBFlag.response:
+            logging.debug("received: console response")
+            return BBPacketConsoleResponse(raw=data)
+        if p_flag & BBFlag.indication:
+            logging.debug("received: console indication")
+            return BBPacketConsoleIndication(raw=data)
+        logging.debug("received: console request")
+        return BBPacketConsoleRequest(raw=data)
     elif p_type == BBType.ping:
         logging.debug("received: ping")
         return BBPacketPing(raw=data)
@@ -67,7 +67,7 @@ class Controller(Thread):
         self.conn.send(bbpkt.pack())
 
     def _handle(self, bbpkt):
-        if isinstance(bbpkt, BBPacketConsoleMsg):
+        if isinstance(bbpkt, BBPacketConsoleIndication):
             os.write(sys.stdout.fileno(), bbpkt.text)
         elif isinstance(bbpkt, BBPacketPong):
             print("pong",)
@@ -102,8 +102,8 @@ class Controller(Thread):
             return 0
 
     def command(self, cmd):
-        self._send(BBPacketCommand(cmd=cmd))
-        r = self._expect(BBPacketCommandReturn, timeout=None)
+        self._send(BBPacketConsoleRequest(cmd=cmd))
+        r = self._expect(BBPacketConsoleResponse, timeout=None)
         logging.info("Command: %r", r)
         return r.exit_code
 
@@ -123,7 +123,7 @@ class Controller(Thread):
                 pkt = self.conn.recv()
                 if pkt:
                     bbpkt = unpack(pkt)
-                    if isinstance(bbpkt, BBPacketConsoleMsg):
+                    if isinstance(bbpkt, BBPacketConsoleIndication):
                         self.rxq.put((self, bbpkt.text))
                     else:
                         self._handle(bbpkt)
@@ -154,7 +154,7 @@ class Controller(Thread):
         self._txq.put(pkt)
 
     def send_async_console(self, text):
-        self._txq.put(BBPacketConsoleMsg(text=text))
+        self._txq.put(BBPacketConsoleIndication(text=text))
 
     def send_async_ping(self):
         self._txq.put(BBPacketPing())
diff --git a/scripts/remote/messages.py b/scripts/remote/messages.py
index 7a597bc9d..2f63f1831 100644
--- a/scripts/remote/messages.py
+++ b/scripts/remote/messages.py
@@ -12,9 +12,7 @@ class BBFlag(object):
 
 
 class BBType(object):
-    command = 1
-    command_return = 2
-    consolemsg = 3
+    console = 1
     ping = 4
     pong = 5
     getenv = 6
@@ -50,13 +48,14 @@ class BBPacket(object):
             self._pack_payload()
 
 
-class BBPacketCommand(BBPacket):
+class BBPacketConsoleRequest(BBPacket):
     def __init__(self, raw=None, cmd=None):
         self.cmd = cmd
-        super(BBPacketCommand, self).__init__(BBType.command, raw=raw)
+        super(BBPacketConsoleRequest, self).__init__(BBType.console,
+                                                     raw=raw)
 
     def __repr__(self):
-        return "BBPacketCommand(cmd=%r)" % self.cmd
+        return "BBPacketConsoleRequest(cmd=%r)" % self.cmd
 
     def _unpack_payload(self, payload):
         self.cmd = payload
@@ -65,14 +64,15 @@ class BBPacketCommand(BBPacket):
         return self.cmd
 
 
-class BBPacketCommandReturn(BBPacket):
+class BBPacketConsoleResponse(BBPacket):
     def __init__(self, raw=None, exit_code=None):
         self.exit_code = exit_code
-        super(BBPacketCommandReturn, self).__init__(BBType.command_return,
-                                                    raw=raw)
+        super(BBPacketConsoleResponse, self).__init__(BBType.console,
+                                                      BBFlag.response,
+                                                      raw=raw)
 
     def __repr__(self):
-        return "BBPacketCommandReturn(exit_code=%i)" % self.exit_code
+        return "BBPacketConsoleResponse(exit_code=%i)" % self.exit_code
 
     def _unpack_payload(self, data):
         self.exit_code, = struct.unpack("!L", data[:4])
@@ -81,13 +81,15 @@ class BBPacketCommandReturn(BBPacket):
         return struct.pack("!L", self.exit_code)
 
 
-class BBPacketConsoleMsg(BBPacket):
+class BBPacketConsoleIndication(BBPacket):
     def __init__(self, raw=None, text=None):
         self.text = text
-        super(BBPacketConsoleMsg, self).__init__(BBType.consolemsg, raw=raw)
+        super(BBPacketConsoleIndication, self).__init__(BBType.console,
+                                                        BBFlag.indication,
+                                                        raw=raw)
 
     def __repr__(self):
-        return "BBPacketConsoleMsg(text=%r)" % self.text
+        return "BBPacketConsoleIndication(text=%r)" % self.text
 
     def _unpack_payload(self, payload):
         self.text = payload
-- 
2.15.1


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

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

* [PATCH 03/10] ratp: port ping operation to req/rsp format
  2018-02-02 11:14 [RFC PATCH 00/10] ratp: new generic RATP command support Aleksander Morgado
  2018-02-02 11:14 ` [PATCH 01/10] ratp: define message type flags Aleksander Morgado
  2018-02-02 11:14 ` [PATCH 02/10] ratp: port command operation to req/rsp/ind format Aleksander Morgado
@ 2018-02-02 11:14 ` Aleksander Morgado
  2018-02-02 11:14 ` [PATCH 04/10] ratp: port getenv " Aleksander Morgado
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Aleksander Morgado @ 2018-02-02 11:14 UTC (permalink / raw)
  To: barebox; +Cc: Aleksander Morgado

The ping operation executed via RATP is processed in the following way:

 * The client sends a 'ping' packet to barebox.
 * Barebox replies with a 'pong' packet.

We now consolidate this process using the request and response
packet flags, and making them part of the same 'ping' packet type.

Signed-off-by: Aleksander Morgado <aleksander@aleksander.es>
---
 common/ratp.c                | 12 ++++++------
 scripts/remote/controller.py | 14 +++++++-------
 scripts/remote/messages.py   | 18 ++++++++++--------
 3 files changed, 23 insertions(+), 21 deletions(-)

diff --git a/common/ratp.c b/common/ratp.c
index 7d7fb5fcd..222bf624a 100644
--- a/common/ratp.c
+++ b/common/ratp.c
@@ -32,8 +32,7 @@
 #include <fs.h>
 
 #define BB_RATP_TYPE_CONSOLE		1
-#define BB_RATP_TYPE_PING		4
-#define BB_RATP_TYPE_PONG		5
+#define BB_RATP_TYPE_PING		2
 #define BB_RATP_TYPE_GETENV		6
 #define BB_RATP_TYPE_GETENV_RETURN	7
 #define BB_RATP_TYPE_FS			8
@@ -169,7 +168,8 @@ static int ratp_bb_send_pong(struct ratp_ctx *ctx)
 	buf = xzalloc(len);
 	rbb = buf;
 
-	rbb->type = cpu_to_be16(BB_RATP_TYPE_PONG);
+	rbb->type = cpu_to_be16(BB_RATP_TYPE_PING);
+	rbb->flags = cpu_to_be16(BB_RATP_FLAG_RESPONSE);
 
 	ret = ratp_send(&ctx->ratp, buf, len);
 
@@ -231,10 +231,10 @@ static int ratp_bb_dispatch(struct ratp_ctx *ctx, const void *buf, int len)
 		pr_debug("got command: %s\n", ratp_command);
 		break;
 
-	case BB_RATP_TYPE_PONG:
-		break;
-
 	case BB_RATP_TYPE_PING:
+		if (flags & BB_RATP_FLAG_RESPONSE)
+			break;
+
 		ret = ratp_bb_send_pong(ctx);
 		break;
 
diff --git a/scripts/remote/controller.py b/scripts/remote/controller.py
index c3f29334a..518973038 100644
--- a/scripts/remote/controller.py
+++ b/scripts/remote/controller.py
@@ -32,11 +32,11 @@ def unpack(data):
         logging.debug("received: console request")
         return BBPacketConsoleRequest(raw=data)
     elif p_type == BBType.ping:
+        if p_flag & BBFlag.response:
+            logging.debug("received: pong")
+            return BBPacketPingResponse(raw=data)
         logging.debug("received: ping")
-        return BBPacketPing(raw=data)
-    elif p_type == BBType.pong:
-        logging.debug("received: pong")
-        return BBPacketPong(raw=data)
+        return BBPacketPingRequest(raw=data)
     elif p_type == BBType.getenv_return:
         logging.debug("received: getenv_return")
         return BBPacketGetenvReturn(raw=data)
@@ -92,8 +92,8 @@ class Controller(Thread):
         self.fsserver = RatpFSServer(path)
 
     def ping(self):
-        self._send(BBPacketPing())
-        r = self._expect(BBPacketPong)
+        self._send(BBPacketPingRequest())
+        r = self._expect(BBPacketPingResponse)
         logging.info("Ping: %r", r)
         if not r:
             return 1
@@ -157,7 +157,7 @@ class Controller(Thread):
         self._txq.put(BBPacketConsoleIndication(text=text))
 
     def send_async_ping(self):
-        self._txq.put(BBPacketPing())
+        self._txq.put(BBPacketPingRequest())
 
 
 def main():
diff --git a/scripts/remote/messages.py b/scripts/remote/messages.py
index 2f63f1831..6c5601d78 100644
--- a/scripts/remote/messages.py
+++ b/scripts/remote/messages.py
@@ -13,8 +13,7 @@ class BBFlag(object):
 
 class BBType(object):
     console = 1
-    ping = 4
-    pong = 5
+    ping = 2
     getenv = 6
     getenv_return = 7
     fs = 8
@@ -98,20 +97,23 @@ class BBPacketConsoleIndication(BBPacket):
         return self.text
 
 
-class BBPacketPing(BBPacket):
+class BBPacketPingRequest(BBPacket):
     def __init__(self, raw=None):
-        super(BBPacketPing, self).__init__(BBType.ping, raw=raw)
+        super(BBPacketPingRequest, self).__init__(BBType.ping,
+                                                  raw=raw)
 
     def __repr__(self):
-        return "BBPacketPing()"
+        return "BBPacketPingRequest()"
 
 
-class BBPacketPong(BBPacket):
+class BBPacketPingResponse(BBPacket):
     def __init__(self, raw=None):
-        super(BBPacketPong, self).__init__(BBType.pong, raw=raw)
+        super(BBPacketPingResponse, self).__init__(BBType.ping,
+                                                   BBFlag.response,
+                                                   raw=raw)
 
     def __repr__(self):
-        return "BBPacketPong()"
+        return "BBPacketPingResponse()"
 
 
 class BBPacketGetenv(BBPacket):
-- 
2.15.1


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

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

* [PATCH 04/10] ratp: port getenv operation to req/rsp format
  2018-02-02 11:14 [RFC PATCH 00/10] ratp: new generic RATP command support Aleksander Morgado
                   ` (2 preceding siblings ...)
  2018-02-02 11:14 ` [PATCH 03/10] ratp: port ping operation to req/rsp format Aleksander Morgado
@ 2018-02-02 11:14 ` Aleksander Morgado
  2018-02-02 11:14 ` [PATCH 05/10] ratp: port filesystem " Aleksander Morgado
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Aleksander Morgado @ 2018-02-02 11:14 UTC (permalink / raw)
  To: barebox; +Cc: Aleksander Morgado

The getenv operation executed via RATP is processed in the following
way:

 * The client sends a 'getenv' packet to barebox specifying the name
   of the variable to read.
 * Barebox replies with a 'getenv_result' packet including the value
   of the variable read.

We now consolidate this process using the request and response
packet flags, and making them part of the same 'getenv' packet type.

Signed-off-by: Aleksander Morgado <aleksander@aleksander.es>
---
 common/ratp.c                | 10 ++++++----
 scripts/remote/controller.py | 13 ++++++++-----
 scripts/remote/messages.py   | 19 ++++++++++---------
 3 files changed, 24 insertions(+), 18 deletions(-)

diff --git a/common/ratp.c b/common/ratp.c
index 222bf624a..2423c0651 100644
--- a/common/ratp.c
+++ b/common/ratp.c
@@ -33,8 +33,7 @@
 
 #define BB_RATP_TYPE_CONSOLE		1
 #define BB_RATP_TYPE_PING		2
-#define BB_RATP_TYPE_GETENV		6
-#define BB_RATP_TYPE_GETENV_RETURN	7
+#define BB_RATP_TYPE_GETENV		3
 #define BB_RATP_TYPE_FS			8
 #define BB_RATP_TYPE_FS_RETURN		9
 
@@ -192,7 +191,8 @@ static int ratp_bb_send_getenv_return(struct ratp_ctx *ctx, const char *val)
 	rbb = buf;
 	strcpy(rbb->data, val);
 
-	rbb->type = cpu_to_be16(BB_RATP_TYPE_GETENV_RETURN);
+	rbb->type = cpu_to_be16(BB_RATP_TYPE_GETENV);
+	rbb->flags = cpu_to_be16(BB_RATP_FLAG_RESPONSE);
 
 	ret = ratp_send(&ctx->ratp, buf, len);
 
@@ -239,8 +239,10 @@ static int ratp_bb_dispatch(struct ratp_ctx *ctx, const void *buf, int len)
 		break;
 
 	case BB_RATP_TYPE_GETENV:
-		varname = xmemdup_add_zero(&rbb->data, dlen);
+		if (flags & BB_RATP_FLAG_RESPONSE)
+			break;
 
+		varname = xmemdup_add_zero(&rbb->data, dlen);
 		ret = ratp_bb_send_getenv_return(ctx, getenv(varname));
 		break;
 
diff --git a/scripts/remote/controller.py b/scripts/remote/controller.py
index 518973038..29aa42ad9 100644
--- a/scripts/remote/controller.py
+++ b/scripts/remote/controller.py
@@ -37,9 +37,12 @@ def unpack(data):
             return BBPacketPingResponse(raw=data)
         logging.debug("received: ping")
         return BBPacketPingRequest(raw=data)
-    elif p_type == BBType.getenv_return:
-        logging.debug("received: getenv_return")
-        return BBPacketGetenvReturn(raw=data)
+    elif p_type == BBType.getenv:
+        if p_flag & BBFlag.response:
+            logging.debug("received: getenv response")
+            return BBPacketGetenvResponse(raw=data)
+        logging.debug("received: getenv")
+        return BBPacketGetenvRequest(raw=data)
     elif p_type == BBType.fs:
         logging.debug("received: fs")
         return BBPacketFS(raw=data)
@@ -108,8 +111,8 @@ class Controller(Thread):
         return r.exit_code
 
     def getenv(self, varname):
-        self._send(BBPacketGetenv(varname=varname))
-        r = self._expect(BBPacketGetenvReturn)
+        self._send(BBPacketGetenvRequest(varname=varname))
+        r = self._expect(BBPacketGetenvResponse)
         return r.text
 
     def close(self):
diff --git a/scripts/remote/messages.py b/scripts/remote/messages.py
index 6c5601d78..88841f4f6 100644
--- a/scripts/remote/messages.py
+++ b/scripts/remote/messages.py
@@ -14,8 +14,7 @@ class BBFlag(object):
 class BBType(object):
     console = 1
     ping = 2
-    getenv = 6
-    getenv_return = 7
+    getenv = 3
     fs = 8
     fs_return = 9
 
@@ -116,13 +115,14 @@ class BBPacketPingResponse(BBPacket):
         return "BBPacketPingResponse()"
 
 
-class BBPacketGetenv(BBPacket):
+class BBPacketGetenvRequest(BBPacket):
     def __init__(self, raw=None, varname=None):
         self.varname = varname
-        super(BBPacketGetenv, self).__init__(BBType.getenv, raw=raw)
+        super(BBPacketGetenvRequest, self).__init__(BBType.getenv,
+                                                    raw=raw)
 
     def __repr__(self):
-        return "BBPacketGetenv(varname=%r)" % self.varname
+        return "BBPacketGetenvRequest(varname=%r)" % self.varname
 
     def _unpack_payload(self, payload):
         self.varname = payload
@@ -131,14 +131,15 @@ class BBPacketGetenv(BBPacket):
         return self.varname
 
 
-class BBPacketGetenvReturn(BBPacket):
+class BBPacketGetenvResponse(BBPacket):
     def __init__(self, raw=None, text=None):
         self.text = text
-        super(BBPacketGetenvReturn, self).__init__(BBType.getenv_return,
-                                                   raw=raw)
+        super(BBPacketGetenvResponse, self).__init__(BBType.getenv,
+                                                     BBFlag.response,
+                                                     raw=raw)
 
     def __repr__(self):
-        return "BBPacketGetenvReturn(varvalue=%s)" % self.text
+        return "BBPacketGetenvResponse(varvalue=%s)" % self.text
 
     def _unpack_payload(self, payload):
         self.text = payload
-- 
2.15.1


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

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

* [PATCH 05/10] ratp: port filesystem operation to req/rsp format
  2018-02-02 11:14 [RFC PATCH 00/10] ratp: new generic RATP command support Aleksander Morgado
                   ` (3 preceding siblings ...)
  2018-02-02 11:14 ` [PATCH 04/10] ratp: port getenv " Aleksander Morgado
@ 2018-02-02 11:14 ` Aleksander Morgado
  2018-02-02 11:14 ` [PATCH 06/10] ratp: implement generic command support Aleksander Morgado
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Aleksander Morgado @ 2018-02-02 11:14 UTC (permalink / raw)
  To: barebox; +Cc: Aleksander Morgado

The filesystem operation executed via RATP is processed in the
following way:

 * The barebox RATP endpoint sends 'fs' packets to the client, with
   the specific filesystem operations to execute.
 * The client replies with 'fs_result' packets to barebox, containing
   the result of the filesystem operation.

We now consolidate this process using the request and response
packet flags, and making them part of the same 'fs' packet type.

Signed-off-by: Aleksander Morgado <aleksander@aleksander.es>
---
 common/ratp.c                |  9 ++++++---
 scripts/remote/controller.py | 12 ++++++------
 scripts/remote/messages.py   | 20 ++++++++++++--------
 scripts/remote/ratpfs.py     |  6 +++---
 4 files changed, 27 insertions(+), 20 deletions(-)

diff --git a/common/ratp.c b/common/ratp.c
index 2423c0651..79b0a9906 100644
--- a/common/ratp.c
+++ b/common/ratp.c
@@ -34,8 +34,7 @@
 #define BB_RATP_TYPE_CONSOLE		1
 #define BB_RATP_TYPE_PING		2
 #define BB_RATP_TYPE_GETENV		3
-#define BB_RATP_TYPE_FS			8
-#define BB_RATP_TYPE_FS_RETURN		9
+#define BB_RATP_TYPE_FS			4
 
 #define BB_RATP_FLAG_NONE		0
 #define BB_RATP_FLAG_RESPONSE		(1 << 0) /* Packet is a response */
@@ -246,7 +245,11 @@ static int ratp_bb_dispatch(struct ratp_ctx *ctx, const void *buf, int len)
 		ret = ratp_bb_send_getenv_return(ctx, getenv(varname));
 		break;
 
-	case BB_RATP_TYPE_FS_RETURN:
+	case BB_RATP_TYPE_FS:
+		/* Only responses expected */
+		if (!(flags & BB_RATP_FLAG_RESPONSE))
+			break;
+
 		pkt = xzalloc(sizeof(*pkt) + dlen);
 		pkt->len = dlen;
 		memcpy(pkt->data, &rbb->data, dlen);
diff --git a/scripts/remote/controller.py b/scripts/remote/controller.py
index 29aa42ad9..db4af3120 100644
--- a/scripts/remote/controller.py
+++ b/scripts/remote/controller.py
@@ -44,11 +44,11 @@ def unpack(data):
         logging.debug("received: getenv")
         return BBPacketGetenvRequest(raw=data)
     elif p_type == BBType.fs:
-        logging.debug("received: fs")
-        return BBPacketFS(raw=data)
-    elif p_type == BBType.fs_return:
-        logging.debug("received: fs_return")
-        return BBPacketFSReturn(raw=data)
+        if p_flag & BBFlag.response:
+            logging.debug("received: fs response")
+            return BBPacketFsResponse(raw=data)
+        logging.debug("received: fs request")
+        return BBPacketFsRequest(raw=data)
     else:
         logging.debug("received: UNKNOWN")
         return BBPacket(raw=data)
@@ -74,7 +74,7 @@ class Controller(Thread):
             os.write(sys.stdout.fileno(), bbpkt.text)
         elif isinstance(bbpkt, BBPacketPong):
             print("pong",)
-        elif isinstance(bbpkt, BBPacketFS):
+        elif isinstance(bbpkt, BBPacketFsRequest):
             if self.fsserver != None:
                 self._send(self.fsserver.handle(bbpkt))
 
diff --git a/scripts/remote/messages.py b/scripts/remote/messages.py
index 88841f4f6..98dda8b79 100644
--- a/scripts/remote/messages.py
+++ b/scripts/remote/messages.py
@@ -15,8 +15,7 @@ class BBType(object):
     console = 1
     ping = 2
     getenv = 3
-    fs = 8
-    fs_return = 9
+    fs = 4
 
 
 class BBPacket(object):
@@ -148,17 +147,22 @@ class BBPacketGetenvResponse(BBPacket):
         return self.text
 
 
-class BBPacketFS(BBPacket):
+class BBPacketFsRequest(BBPacket):
     def __init__(self, raw=None, payload=None):
-        super(BBPacketFS, self).__init__(BBType.fs, payload=payload, raw=raw)
+        super(BBPacketFsRequest, self).__init__(BBType.fs,
+                                                payload=payload,
+                                                raw=raw)
 
     def __repr__(self):
-        return "BBPacketFS(payload=%r)" % self.payload
+        return "BBPacketFsRequest(payload=%r)" % self.payload
 
 
-class BBPacketFSReturn(BBPacket):
+class BBPacketFsResponse(BBPacket):
     def __init__(self, raw=None, payload=None):
-        super(BBPacketFSReturn, self).__init__(BBType.fs_return, payload=payload, raw=raw)
+        super(BBPacketFsResponse, self).__init__(BBType.fs,
+                                                 BBFlag.response,
+                                                 payload=payload,
+                                                 raw=raw)
 
     def __repr__(self):
-        return "BBPacketFSReturn(payload=%r)" % self.payload
+        return "BBPacketFsResponse(payload=%r)" % self.payload
diff --git a/scripts/remote/ratpfs.py b/scripts/remote/ratpfs.py
index 91ca04454..9e88b03a6 100644
--- a/scripts/remote/ratpfs.py
+++ b/scripts/remote/ratpfs.py
@@ -9,7 +9,7 @@ import stat
 import struct
 from enum import IntEnum
 
-from .messages import BBPacketFS, BBPacketFSReturn
+from .messages import BBPacketFsRequest, BBPacketFsResponse
 
 class RatpFSType(IntEnum):
     invalid = 0
@@ -138,7 +138,7 @@ class RatpFSServer(object):
         return ""
 
     def handle(self, bbcall):
-        assert isinstance(bbcall, BBPacketFS)
+        assert isinstance(bbcall, BBPacketFsRequest)
         logging.debug("bb-call: %s", bbcall)
         fscall = RatpFSPacket(raw=bbcall.payload)
         logging.info("fs-call: %s", fscall)
@@ -184,6 +184,6 @@ class RatpFSServer(object):
             raise RatpFSError()
 
         logging.info("fs-return: %s", fsreturn)
-        bbreturn = BBPacketFSReturn(payload=fsreturn.pack())
+        bbreturn = BBPacketFsResponse(payload=fsreturn.pack())
         logging.debug("bb-return: %s", bbreturn)
         return bbreturn
-- 
2.15.1


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

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

* [PATCH 06/10] ratp: implement generic command support
  2018-02-02 11:14 [RFC PATCH 00/10] ratp: new generic RATP command support Aleksander Morgado
                   ` (4 preceding siblings ...)
  2018-02-02 11:14 ` [PATCH 05/10] ratp: port filesystem " Aleksander Morgado
@ 2018-02-02 11:14 ` Aleksander Morgado
  2018-02-06  9:30   ` Sascha Hauer
  2018-02-02 11:14 ` [PATCH 07/10] ratp: implement ping as a standard ratp command Aleksander Morgado
                   ` (4 subsequent siblings)
  10 siblings, 1 reply; 20+ messages in thread
From: Aleksander Morgado @ 2018-02-02 11:14 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 numeric unique command ID, 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                             | 255 ++++++++++++++++++------------
 include/asm-generic/barebox.lds.h         |   2 +
 include/ratp_bb.h                         |  47 ++++++
 16 files changed, 258 insertions(+), 104 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 79b0a9906..2cdb1cd89 100644
--- a/common/ratp.c
+++ b/common/ratp.c
@@ -31,20 +31,10 @@
 #include <ratp_bb.h>
 #include <fs.h>
 
-#define BB_RATP_TYPE_CONSOLE		1
-#define BB_RATP_TYPE_PING		2
-#define BB_RATP_TYPE_GETENV		3
-#define BB_RATP_TYPE_FS			4
+LIST_HEAD(ratp_command_list);
+EXPORT_SYMBOL(ratp_command_list);
 
-#define BB_RATP_FLAG_NONE		0
-#define BB_RATP_FLAG_RESPONSE		(1 << 0) /* Packet is a response */
-#define BB_RATP_FLAG_INDICATION		(1 << 1) /* Packet is an indication */
-
-struct ratp_bb {
-	uint16_t type;
-	uint16_t flags;
-	uint8_t data[];
-};
+#define for_each_ratp_command(cmd) list_for_each_entry(cmd, &ratp_command_list, list)
 
 struct ratp_bb_command_return {
 	uint32_t errno;
@@ -203,66 +193,6 @@ static int ratp_bb_send_getenv_return(struct ratp_ctx *ctx, const char *val)
 static char *ratp_command;
 static struct ratp_ctx *ratp_ctx;
 
-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 flags = be16_to_cpu(rbb->flags);
-
-	switch (be16_to_cpu(rbb->type)) {
-	case BB_RATP_TYPE_CONSOLE:
-		if (flags & BB_RATP_FLAG_RESPONSE)
-			break;
-
-		if (flags & BB_RATP_FLAG_INDICATION) {
-			kfifo_put(ctx->console_recv_fifo, rbb->data, dlen);
-			break;
-		}
-
-		if (ratp_command)
-			return 0;
-
-		ratp_command = xmemdup_add_zero(&rbb->data, dlen);
-		ratp_ctx = ctx;
-		pr_debug("got command: %s\n", ratp_command);
-		break;
-
-	case BB_RATP_TYPE_PING:
-		if (flags & BB_RATP_FLAG_RESPONSE)
-			break;
-
-		ret = ratp_bb_send_pong(ctx);
-		break;
-
-	case BB_RATP_TYPE_GETENV:
-		if (flags & BB_RATP_FLAG_RESPONSE)
-			break;
-
-		varname = xmemdup_add_zero(&rbb->data, dlen);
-		ret = ratp_bb_send_getenv_return(ctx, getenv(varname));
-		break;
-
-	case BB_RATP_TYPE_FS:
-		/* Only responses expected */
-		if (!(flags & BB_RATP_FLAG_RESPONSE))
-			break;
-
-		pkt = xzalloc(sizeof(*pkt) + dlen);
-		pkt->len = dlen;
-		memcpy(pkt->data, &rbb->data, dlen);
-		ctx->fs_rx = pkt;
-		break;
-	default:
-		printf("%s: unhandled packet type 0x%04x\n", __func__, be16_to_cpu(rbb->type));
-		break;
-	}
-
-	return ret;
-}
-
 static int ratp_console_getc(struct console_device *cdev)
 {
 	struct ratp_ctx *ctx = container_of(cdev, struct ratp_ctx, ratp_console);
@@ -374,35 +304,6 @@ static void ratp_console_unregister(struct ratp_ctx *ctx)
 	}
 }
 
-static void ratp_poller(struct poller_struct *poller)
-{
-	struct ratp_ctx *ctx = container_of(poller, struct ratp_ctx, poller);
-	int ret;
-	size_t len;
-	void *buf;
-
-	ratp_queue_console_tx(ctx);
-
-	ret = ratp_poll(&ctx->ratp);
-	if (ret == -EINTR)
-		goto out;
-	if (ratp_closed(&ctx->ratp))
-		goto out;
-
-	ret = ratp_recv(&ctx->ratp, &buf, &len);
-	if (ret < 0)
-		return;
-
-	ratp_bb_dispatch(ctx, buf, len);
-
-	free(buf);
-
-	return;
-
-out:
-	ratp_console_unregister(ctx);
-}
-
 int barebox_ratp_fs_call(struct ratp_bb_pkt *tx, struct ratp_bb_pkt **rx)
 {
 	struct ratp_ctx *ctx = ratp_ctx;
@@ -442,6 +343,156 @@ int barebox_ratp_fs_call(struct ratp_bb_pkt *tx, struct ratp_bb_pkt **rx)
 	return 0;
 }
 
+static int compare_ratp_command(struct list_head *a, struct list_head *b)
+{
+	int id_a = list_entry(a, struct ratp_command, list)->id;
+	int id_b = list_entry(b, struct ratp_command, list)->id;
+
+	return (id_a - id_b);
+}
+
+int register_ratp_command(struct ratp_command *cmd)
+{
+	debug("register ratp command %hu\n", cmd->id);
+	list_add_sort(&cmd->list, &ratp_command_list, compare_ratp_command);
+	return 0;
+}
+EXPORT_SYMBOL(register_ratp_command);
+
+struct ratp_command *find_ratp_cmd(uint16_t cmd_id)
+{
+	struct ratp_command *cmdtp;
+
+	for_each_ratp_command(cmdtp)
+		if (cmd_id == cmdtp->id)
+			return cmdtp;
+
+	return NULL;	/* not found or ambiguous command */
+}
+
+static int dispatch_ratp_message(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 flags;
+	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_cmd(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;
+	}
+
+	flags = be16_to_cpu(rbb->flags);
+	switch (type) {
+	case BB_RATP_TYPE_CONSOLE:
+		if (flags & BB_RATP_FLAG_RESPONSE)
+			break;
+
+		if (flags & BB_RATP_FLAG_INDICATION) {
+			kfifo_put(ctx->console_recv_fifo, rbb->data, dlen);
+			break;
+		}
+
+		if (ratp_command)
+			return 0;
+
+		ratp_command = xmemdup_add_zero(&rbb->data, dlen);
+		ratp_ctx = ctx;
+		pr_debug("got command: %s\n", ratp_command);
+		break;
+
+	case BB_RATP_TYPE_PING:
+		if (flags & BB_RATP_FLAG_RESPONSE)
+			break;
+
+		ret = ratp_bb_send_pong(ctx);
+		break;
+
+	case BB_RATP_TYPE_GETENV:
+		if (flags & BB_RATP_FLAG_RESPONSE)
+			break;
+
+		varname = xmemdup_add_zero(&rbb->data, dlen);
+		ret = ratp_bb_send_getenv_return(ctx, getenv(varname));
+		break;
+
+	case BB_RATP_TYPE_FS:
+		/* Only responses expected */
+		if (!(flags & BB_RATP_FLAG_RESPONSE))
+			break;
+
+		pkt = xzalloc(sizeof(*pkt) + dlen);
+		pkt->len = dlen;
+		memcpy(pkt->data, &rbb->data, dlen);
+		ctx->fs_rx = pkt;
+		break;
+	default:
+		printf("%s: unhandled packet type 0x%04x\n", __func__, be16_to_cpu(rbb->type));
+		break;
+	}
+
+	return ret;
+}
+
+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 void ratp_poller(struct poller_struct *poller)
+{
+	struct ratp_ctx *ctx = container_of(poller, struct ratp_ctx, poller);
+	int ret;
+	size_t len;
+	void *buf;
+
+	ratp_queue_console_tx(ctx);
+
+	ret = ratp_poll(&ctx->ratp);
+	if (ret == -EINTR)
+		goto out;
+	if (ratp_closed(&ctx->ratp))
+		goto out;
+
+	ret = ratp_recv(&ctx->ratp, &buf, &len);
+	if (ret < 0)
+		return;
+
+	dispatch_ratp_message(ctx, buf, len);
+
+	free(buf);
+
+	return;
+
+out:
+	ratp_console_unregister(ctx);
+}
+
 int barebox_ratp(struct console_device *cdev)
 {
 	int ret;
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..1c2aa1fb7 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_CONSOLE		1
+#define BB_RATP_TYPE_PING		2
+#define BB_RATP_TYPE_GETENV		3
+#define BB_RATP_TYPE_FS			4
+#define BB_RATP_TYPE_RESET		5
+
+#define BB_RATP_FLAG_NONE		0
+#define BB_RATP_FLAG_RESPONSE		(1 << 0) /* Packet is a response */
+#define BB_RATP_FLAG_INDICATION		(1 << 1) /* Packet is an indication */
+
+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          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_cmd_##_name;					\
+const struct ratp_command __barebox_cmd_##_name						\
+	__attribute__ ((unused,section (".barebox_ratp_cmd_" __stringify(_name)))) = {	\
+	.id		= BB_RATP_TYPE_##_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] 20+ messages in thread

* [PATCH 07/10] ratp: implement ping as a standard ratp command
  2018-02-02 11:14 [RFC PATCH 00/10] ratp: new generic RATP command support Aleksander Morgado
                   ` (5 preceding siblings ...)
  2018-02-02 11:14 ` [PATCH 06/10] ratp: implement generic command support Aleksander Morgado
@ 2018-02-02 11:14 ` Aleksander Morgado
  2018-02-06  9:33   ` Sascha Hauer
  2018-02-02 11:14 ` [PATCH 08/10] ratp: implement getenv " Aleksander Morgado
                   ` (3 subsequent siblings)
  10 siblings, 1 reply; 20+ messages in thread
From: Aleksander Morgado @ 2018-02-02 11:14 UTC (permalink / raw)
  To: barebox; +Cc: Aleksander Morgado

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

diff --git a/commands/Makefile b/commands/Makefile
index 00a863919..012a3ad68 100644
--- a/commands/Makefile
+++ b/commands/Makefile
@@ -126,3 +126,4 @@ obj-$(CONFIG_CMD_SEED)		+= seed.o
 obj-$(CONFIG_CMD_ZODIAC_PIC)	+= pic.o zii-pic-esb.o \
 				zii-pic-mezz.o zii-pic-niu.o \
 				zii-pic-rdu.o zii-pic-rdu2.o
+obj-$(CONFIG_RATP)		+= ratp-ping.o
diff --git a/commands/ratp-ping.c b/commands/ratp-ping.c
new file mode 100644
index 000000000..837c56762
--- /dev/null
+++ b/commands/ratp-ping.c
@@ -0,0 +1,38 @@
+/*
+ * 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)
+{
+	*rsp_len = sizeof(struct ratp_bb);
+	*rsp = xzalloc(*rsp_len);
+	(*rsp)->type = cpu_to_be16(BB_RATP_TYPE_PING);
+	(*rsp)->flags = cpu_to_be16(BB_RATP_FLAG_RESPONSE);
+	return 0;
+}
+
+BAREBOX_RATP_CMD_START(PING)
+	.cmd = ratp_cmd_ping
+BAREBOX_RATP_CMD_END
diff --git a/common/ratp.c b/common/ratp.c
index 2cdb1cd89..a880e8e03 100644
--- a/common/ratp.c
+++ b/common/ratp.c
@@ -146,26 +146,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_PING);
-	rbb->flags = cpu_to_be16(BB_RATP_FLAG_RESPONSE);
-
-	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;
@@ -414,13 +394,6 @@ static int dispatch_ratp_message(struct ratp_ctx *ctx, const void *buf, int len)
 		pr_debug("got command: %s\n", ratp_command);
 		break;
 
-	case BB_RATP_TYPE_PING:
-		if (flags & BB_RATP_FLAG_RESPONSE)
-			break;
-
-		ret = ratp_bb_send_pong(ctx);
-		break;
-
 	case BB_RATP_TYPE_GETENV:
 		if (flags & BB_RATP_FLAG_RESPONSE)
 			break;
-- 
2.15.1


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

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

* [PATCH 08/10] ratp: implement getenv as a standard ratp command
  2018-02-02 11:14 [RFC PATCH 00/10] ratp: new generic RATP command support Aleksander Morgado
                   ` (6 preceding siblings ...)
  2018-02-02 11:14 ` [PATCH 07/10] ratp: implement ping as a standard ratp command Aleksander Morgado
@ 2018-02-02 11:14 ` Aleksander Morgado
  2018-02-02 11:14 ` [PATCH 09/10] ratp: new reset command Aleksander Morgado
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Aleksander Morgado @ 2018-02-02 11:14 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>
---
 commands/Makefile      |  1 +
 commands/ratp-getenv.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 common/ratp.c          | 34 ----------------------------------
 3 files changed, 51 insertions(+), 34 deletions(-)
 create mode 100644 commands/ratp-getenv.c

diff --git a/commands/Makefile b/commands/Makefile
index 012a3ad68..5e5464ec3 100644
--- a/commands/Makefile
+++ b/commands/Makefile
@@ -127,3 +127,4 @@ obj-$(CONFIG_CMD_ZODIAC_PIC)	+= pic.o zii-pic-esb.o \
 				zii-pic-mezz.o zii-pic-niu.o \
 				zii-pic-rdu.o zii-pic-rdu2.o
 obj-$(CONFIG_RATP)		+= ratp-ping.o
+obj-$(CONFIG_RATP)		+= ratp-getenv.o
diff --git a/commands/ratp-getenv.c b/commands/ratp-getenv.c
new file mode 100644
index 000000000..2daaa63d8
--- /dev/null
+++ b/commands/ratp-getenv.c
@@ -0,0 +1,50 @@
+/*
+ * 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);
+	(*rsp)->flags = cpu_to_be16(BB_RATP_FLAG_RESPONSE);
+	memcpy ((*rsp)->data, value, dlen);
+	return 0;
+}
+
+BAREBOX_RATP_CMD_START(GETENV)
+	.cmd = ratp_cmd_getenv
+BAREBOX_RATP_CMD_END
diff --git a/common/ratp.c b/common/ratp.c
index a880e8e03..e1ecb314c 100644
--- a/common/ratp.c
+++ b/common/ratp.c
@@ -24,7 +24,6 @@
 #include <ratp.h>
 #include <command.h>
 #include <byteorder.h>
-#include <environment.h>
 #include <kfifo.h>
 #include <poller.h>
 #include <linux/sizes.h>
@@ -146,30 +145,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);
-	rbb->flags = cpu_to_be16(BB_RATP_FLAG_RESPONSE);
-
-	ret = ratp_send(&ctx->ratp, buf, len);
-
-	free(buf);
-
-	return ret;
-}
-
 static char *ratp_command;
 static struct ratp_ctx *ratp_ctx;
 
@@ -355,7 +330,6 @@ static int dispatch_ratp_message(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 flags;
 	uint16_t type = be16_to_cpu(rbb->type);
@@ -394,14 +368,6 @@ static int dispatch_ratp_message(struct ratp_ctx *ctx, const void *buf, int len)
 		pr_debug("got command: %s\n", ratp_command);
 		break;
 
-	case BB_RATP_TYPE_GETENV:
-		if (flags & BB_RATP_FLAG_RESPONSE)
-			break;
-
-		varname = xmemdup_add_zero(&rbb->data, dlen);
-		ret = ratp_bb_send_getenv_return(ctx, getenv(varname));
-		break;
-
 	case BB_RATP_TYPE_FS:
 		/* Only responses expected */
 		if (!(flags & BB_RATP_FLAG_RESPONSE))
-- 
2.15.1


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

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

* [PATCH 09/10] ratp: new reset command
  2018-02-02 11:14 [RFC PATCH 00/10] ratp: new generic RATP command support Aleksander Morgado
                   ` (7 preceding siblings ...)
  2018-02-02 11:14 ` [PATCH 08/10] ratp: implement getenv " Aleksander Morgado
@ 2018-02-02 11:14 ` Aleksander Morgado
  2018-02-02 11:14 ` [PATCH 10/10] ratp: new md and mw commands Aleksander Morgado
  2018-02-06  9:24 ` [RFC PATCH 00/10] ratp: new generic RATP command support Sascha Hauer
  10 siblings, 0 replies; 20+ messages in thread
From: Aleksander Morgado @ 2018-02-02 11:14 UTC (permalink / raw)
  To: barebox; +Cc: Aleksander Morgado

Signed-off-by: Aleksander Morgado <aleksander@aleksander.es>
---
 commands/reset.c | 48 +++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 41 insertions(+), 7 deletions(-)

diff --git a/commands/reset.c b/commands/reset.c
index 6eac53262..7971b9f2f 100644
--- a/commands/reset.c
+++ b/commands/reset.c
@@ -19,10 +19,26 @@
 
 #include <common.h>
 #include <command.h>
+#include <ratp_bb.h>
 #include <complete.h>
 #include <getopt.h>
 #include <restart.h>
 
+static int common_reset (int shutdown_flag)
+{
+	debug("running reset %s\n", shutdown_flag ? "" : "(forced)");
+
+	if (shutdown_flag)
+		shutdown_barebox();
+
+	restart_machine();
+
+	/* Not reached */
+	return 1;
+}
+
+/* Console command */
+
 static int cmd_reset(int argc, char *argv[])
 {
 	int opt, shutdown_flag;
@@ -39,13 +55,7 @@ static int cmd_reset(int argc, char *argv[])
 		}
 	}
 
-	if (shutdown_flag)
-		shutdown_barebox();
-
-	restart_machine();
-
-	/* Not reached */
-	return 1;
+	return common_reset (shutdown_flag);
 }
 
 BAREBOX_CMD_HELP_START(reset)
@@ -61,3 +71,27 @@ BAREBOX_CMD_START(reset)
 	BAREBOX_CMD_HELP(cmd_reset_help)
 	BAREBOX_CMD_COMPLETE(empty_complete)
 BAREBOX_CMD_END
+
+/* RATP command */
+
+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;
+	}
+
+	return common_reset (reset_req->force);
+}
+
+BAREBOX_RATP_CMD_START(RESET)
+	.cmd = ratp_cmd_reset
+BAREBOX_RATP_CMD_END
-- 
2.15.1


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

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

* [PATCH 10/10] ratp: new md and mw commands
  2018-02-02 11:14 [RFC PATCH 00/10] ratp: new generic RATP command support Aleksander Morgado
                   ` (8 preceding siblings ...)
  2018-02-02 11:14 ` [PATCH 09/10] ratp: new reset command Aleksander Morgado
@ 2018-02-02 11:14 ` Aleksander Morgado
  2018-02-06  9:24 ` [RFC PATCH 00/10] ratp: new generic RATP command support Sascha Hauer
  10 siblings, 0 replies; 20+ messages in thread
From: Aleksander Morgado @ 2018-02-02 11:14 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>
---
 commands/md.c     | 209 ++++++++++++++++++++++++++++++++++++++++++++++--------
 commands/mw.c     | 150 ++++++++++++++++++++++++++++++++++++++-
 include/ratp_bb.h |   2 +
 3 files changed, 330 insertions(+), 31 deletions(-)

diff --git a/commands/md.c b/commands/md.c
index 3e83c723a..6cd3b9bcf 100644
--- a/commands/md.c
+++ b/commands/md.c
@@ -1,5 +1,6 @@
 /*
- * Copyright (c) 2011 Sascha Hauer <s.hauer@pengutronix.de>, Pengutronix
+ * 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.
@@ -23,6 +24,7 @@
 
 #include <common.h>
 #include <command.h>
+#include <ratp_bb.h>
 #include <init.h>
 #include <driver.h>
 #include <malloc.h>
@@ -36,66 +38,84 @@
 
 extern char *mem_rw_buf;
 
-static int do_mem_md(int argc, char *argv[])
+static int common_mem_md(const char *filename,
+			 loff_t start,
+			 loff_t size,
+			 int swab,
+			 int mode,
+			 uint8_t *output)
 {
-	loff_t	start = 0, size = 0x100;
-	int	r, now;
-	int	ret = 0;
+	int r, now, t;
+	int ret = 0;
 	int fd;
-	char *filename = "/dev/mem";
-	int mode = O_RWSIZE_4;
-	int swab = 0;
 	void *map;
 
-	if (argc < 2)
-		return COMMAND_ERROR_USAGE;
-
-	if (mem_parse_options(argc, argv, "bwlqs:x", &mode, &filename, NULL,
-			&swab) < 0)
-		return 1;
-
-	if (optind < argc) {
-		if (parse_area_spec(argv[optind], &start, &size)) {
-			printf("could not parse: %s\n", argv[optind]);
-			return 1;
-		}
-		if (size == ~0)
-			size = 0x100;
-	}
-
 	fd = open_and_lseek(filename, mode | O_RDONLY, start);
 	if (fd < 0)
 		return 1;
 
 	map = memmap(fd, PROT_READ);
 	if (map != (void *)-1) {
-		ret = memory_display(map + start, start, size,
-				mode >> O_RWSIZE_SHIFT, swab);
+		if (output)
+			memcpy(output, (uint8_t *)(map + start), size);
+		 else
+			ret = memory_display(map + start, start, size,
+					     mode >> O_RWSIZE_SHIFT, swab);
 		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;
 
-		if ((ret = memory_display(mem_rw_buf, start, r,
-				mode >> O_RWSIZE_SHIFT, swab)))
+		if (output)
+			memcpy(output + t, (uint8_t *)(mem_rw_buf), r);
+		else if ((ret = memory_display(mem_rw_buf, start + t, r,
+					       mode >> O_RWSIZE_SHIFT, swab)))
 			goto out;
 
-		start += r;
 		size  -= r;
+		t     += r;
 	} while (size);
 
 out:
 	close(fd);
 
-	return ret ? 1 : 0;
+	return ret;
+}
+
+static int do_mem_md(int argc, char *argv[])
+{
+	loff_t	start = 0, size = 0x100;
+	char *filename = "/dev/mem";
+	int mode = O_RWSIZE_4;
+	int swab = 0;
+
+	if (argc < 2)
+		return COMMAND_ERROR_USAGE;
+
+	if (mem_parse_options(argc, argv, "bwlqs:x", &mode, &filename, NULL,
+			&swab) < 0)
+		return 1;
+
+	if (optind < argc) {
+		if (parse_area_spec(argv[optind], &start, &size)) {
+			printf("could not parse: %s\n", argv[optind]);
+			return 1;
+		}
+		if (size == ~0)
+			size = 0x100;
+	}
+
+	return common_mem_md(filename, start, size, swab, mode, NULL);
 }
 
 
@@ -124,3 +144,132 @@ BAREBOX_CMD_START(md)
 	BAREBOX_CMD_GROUP(CMD_GRP_MEM)
 	BAREBOX_CMD_HELP(cmd_md_help)
 BAREBOX_CMD_END
+
+/* RATP command */
+
+/* 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));
+
+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);
+	md_rsp->header.flags = cpu_to_be16(BB_RATP_FLAG_RESPONSE);
+	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 = common_mem_md(path, addr, size, 0, O_RWSIZE_1, 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)
+	.cmd = ratp_cmd_md
+BAREBOX_RATP_CMD_END
diff --git a/commands/mw.c b/commands/mw.c
index bb6a16ef3..85560d532 100644
--- a/commands/mw.c
+++ b/commands/mw.c
@@ -1,5 +1,6 @@
 /*
- * Copyright (c) 2011 Sascha Hauer <s.hauer@pengutronix.de>, Pengutronix
+ * 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.
@@ -23,6 +24,7 @@
 
 #include <common.h>
 #include <command.h>
+#include <ratp_bb.h>
 #include <init.h>
 #include <driver.h>
 #include <malloc.h>
@@ -116,3 +118,149 @@ BAREBOX_CMD_START(mw)
 	BAREBOX_CMD_GROUP(CMD_GRP_MEM)
 	BAREBOX_CMD_HELP(cmd_mw_help)
 BAREBOX_CMD_END
+
+/* RATP command */
+
+/* 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);
+	mw_rsp->header.flags = cpu_to_be16(BB_RATP_FLAG_RESPONSE);
+	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)
+	.cmd = ratp_cmd_mw
+BAREBOX_RATP_CMD_END
diff --git a/include/ratp_bb.h b/include/ratp_bb.h
index 1c2aa1fb7..5c546822b 100644
--- a/include/ratp_bb.h
+++ b/include/ratp_bb.h
@@ -8,6 +8,8 @@
 #define BB_RATP_TYPE_GETENV		3
 #define BB_RATP_TYPE_FS			4
 #define BB_RATP_TYPE_RESET		5
+#define BB_RATP_TYPE_MD			6
+#define BB_RATP_TYPE_MW			7
 
 #define BB_RATP_FLAG_NONE		0
 #define BB_RATP_FLAG_RESPONSE		(1 << 0) /* Packet is a response */
-- 
2.15.1


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

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

* Re: [RFC PATCH 00/10] ratp: new generic RATP command support
  2018-02-02 11:14 [RFC PATCH 00/10] ratp: new generic RATP command support Aleksander Morgado
                   ` (9 preceding siblings ...)
  2018-02-02 11:14 ` [PATCH 10/10] ratp: new md and mw commands Aleksander Morgado
@ 2018-02-06  9:24 ` Sascha Hauer
  2018-02-06 16:43   ` Aleksander Morgado
  10 siblings, 1 reply; 20+ messages in thread
From: Sascha Hauer @ 2018-02-06  9:24 UTC (permalink / raw)
  To: Aleksander Morgado; +Cc: barebox

Hi Aleksander,

On Fri, Feb 02, 2018 at 12:14:32PM +0100, Aleksander Morgado wrote:
> Until now, the barebox-specific RATP commands were all defined and
> implemented in common/ratp.c. This series of patches allow ratp
> commands to be defined in a similar way to console commands.

I like the idea.

> 
> The first patches (1-5) break the current RATP API, by introducing
> the concept of requests, responses and indications:
>  * Requests sent to one endpoint are expected to be replied with
>    a response by the peer endpoint.
>  * Indications are messages sent from one endpoint to another which
>    are not expected to be replied.

I do not see why we have to break the RATP API. I mean currently we
have BB_RATP_TYPE_COMMAND and BB_RATP_TYPE_COMMAND_RETURN which you
convert to .type = BB_RATP_TYPE_COMMAND, .flags = 0 | RESPONSE.

I see that using flags looks somewhat nicer, but besides of that,
what is your selling point to break the API?

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] 20+ messages in thread

* Re: [PATCH 06/10] ratp: implement generic command support
  2018-02-02 11:14 ` [PATCH 06/10] ratp: implement generic command support Aleksander Morgado
@ 2018-02-06  9:30   ` Sascha Hauer
  2018-02-06 16:49     ` Aleksander Morgado
  0 siblings, 1 reply; 20+ messages in thread
From: Sascha Hauer @ 2018-02-06  9:30 UTC (permalink / raw)
  To: Aleksander Morgado; +Cc: barebox

On Fri, Feb 02, 2018 at 12:14:38PM +0100, Aleksander Morgado wrote:
> 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 numeric unique command ID, 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>
> ---
> @@ -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          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_cmd_##_name;					\

You shouldn't use the same name as the existing barebox commands,
otherwise there might be name clashes. Add some _ratp_ to it.

> +const struct ratp_command __barebox_cmd_##_name						\
> +	__attribute__ ((unused,section (".barebox_ratp_cmd_" __stringify(_name)))) = {	\
> +	.id		= BB_RATP_TYPE_##_name,

I am not sure if I like the magic construction of the id field. Being
able to grep for BB_RATP_TYPE_PING and finding the user has advantages.
How about manually setting the field in the body of the command instead
of constructing it?

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] 20+ messages in thread

* Re: [PATCH 07/10] ratp: implement ping as a standard ratp command
  2018-02-02 11:14 ` [PATCH 07/10] ratp: implement ping as a standard ratp command Aleksander Morgado
@ 2018-02-06  9:33   ` Sascha Hauer
  2018-02-06 16:51     ` Aleksander Morgado
  0 siblings, 1 reply; 20+ messages in thread
From: Sascha Hauer @ 2018-02-06  9:33 UTC (permalink / raw)
  To: Aleksander Morgado; +Cc: barebox

On Fri, Feb 02, 2018 at 12:14:39PM +0100, Aleksander Morgado wrote:
> Signed-off-by: Aleksander Morgado <aleksander@aleksander.es>
> ---
>  commands/Makefile    |  1 +
>  commands/ratp-ping.c | 38 ++++++++++++++++++++++++++++++++++++++
>  common/ratp.c        | 27 ---------------------------

Can we put the ratp commands to a separate directory like
common/ratp/commands/ or similar?
There doesn't seem to be very much code sharing between the regular
commands and their ratp correspondents. Most good commands are written
in the way that the functionality is in common code outside the command
file itself anyway.

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] 20+ messages in thread

* Re: [RFC PATCH 00/10] ratp: new generic RATP command support
  2018-02-06  9:24 ` [RFC PATCH 00/10] ratp: new generic RATP command support Sascha Hauer
@ 2018-02-06 16:43   ` Aleksander Morgado
  2018-02-07  8:33     ` Sascha Hauer
  0 siblings, 1 reply; 20+ messages in thread
From: Aleksander Morgado @ 2018-02-06 16:43 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox

>>
>> The first patches (1-5) break the current RATP API, by introducing
>> the concept of requests, responses and indications:
>>  * Requests sent to one endpoint are expected to be replied with
>>    a response by the peer endpoint.
>>  * Indications are messages sent from one endpoint to another which
>>    are not expected to be replied.
>
> I do not see why we have to break the RATP API. I mean currently we
> have BB_RATP_TYPE_COMMAND and BB_RATP_TYPE_COMMAND_RETURN which you
> convert to .type = BB_RATP_TYPE_COMMAND, .flags = 0 | RESPONSE.
>
> I see that using flags looks somewhat nicer, but besides of that,
> what is your selling point to break the API?
>

Well, it was just easier to say "if I send a request of type X, I
expect back a response of the same type X". It avoids the need of
having to define which command id is expected as response for which
command id request. I don't think I have any other selling point :)
Being so early in the amount of commands we have in RATP, thought this
could be a good improvement. Maybe I'm biased because I'm used to
talking to mobile broadband modems, and that is how all protocols I
know are defined. That said, changing this to keep the API would still
be very easy, it's no big deal.

-- 
Aleksander
https://aleksander.es

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

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

* Re: [PATCH 06/10] ratp: implement generic command support
  2018-02-06  9:30   ` Sascha Hauer
@ 2018-02-06 16:49     ` Aleksander Morgado
  2018-02-07  8:34       ` Sascha Hauer
  0 siblings, 1 reply; 20+ messages in thread
From: Aleksander Morgado @ 2018-02-06 16:49 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox

>> +
>> +#define BAREBOX_RATP_CMD_START(_name)                                                        \
>> +extern const struct ratp_command __barebox_cmd_##_name;                                      \
>
> You shouldn't use the same name as the existing barebox commands,
> otherwise there might be name clashes. Add some _ratp_ to it.
>

Ah, sure.

>> +const struct ratp_command __barebox_cmd_##_name                                              \
>> +     __attribute__ ((unused,section (".barebox_ratp_cmd_" __stringify(_name)))) = {  \
>> +     .id             = BB_RATP_TYPE_##_name,
>
> I am not sure if I like the magic construction of the id field. Being
> able to grep for BB_RATP_TYPE_PING and finding the user has advantages.
> How about manually setting the field in the body of the command instead
> of constructing it?
>

You mean doing this instead?

BAREBOX_RATP_CMD_START(PING)
    .cmd = ratp_cmd_ping,
    .id = BB_RATP_TYPE_PING
BAREBOX_RATP_CMD_END


-- 
Aleksander
https://aleksander.es

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

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

* Re: [PATCH 07/10] ratp: implement ping as a standard ratp command
  2018-02-06  9:33   ` Sascha Hauer
@ 2018-02-06 16:51     ` Aleksander Morgado
  2018-02-07  8:26       ` Sascha Hauer
  0 siblings, 1 reply; 20+ messages in thread
From: Aleksander Morgado @ 2018-02-06 16:51 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox

On Tue, Feb 6, 2018 at 10:33 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> On Fri, Feb 02, 2018 at 12:14:39PM +0100, Aleksander Morgado wrote:
>> Signed-off-by: Aleksander Morgado <aleksander@aleksander.es>
>> ---
>>  commands/Makefile    |  1 +
>>  commands/ratp-ping.c | 38 ++++++++++++++++++++++++++++++++++++++
>>  common/ratp.c        | 27 ---------------------------
>
> Can we put the ratp commands to a separate directory like
> common/ratp/commands/ or similar?
> There doesn't seem to be very much code sharing between the regular
> commands and their ratp correspondents. Most good commands are written
> in the way that the functionality is in common code outside the command
> file itself anyway.
>

I actually did that originally, but then implemented "md" for RATP and
decided I could reuse part of the code doing the memory dump. But then
no other command I implemented shared anything with the console
counterpart... so yes, totally agree changing that would be much
better. I did think of moving them to commands/ratp/* instead of
common/ratp/commands/* though. Why do you suggest to have them in
common/? Shouldn't they be closer to the console commands in the
hierarchy?

-- 
Aleksander
https://aleksander.es

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

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

* Re: [PATCH 07/10] ratp: implement ping as a standard ratp command
  2018-02-06 16:51     ` Aleksander Morgado
@ 2018-02-07  8:26       ` Sascha Hauer
  0 siblings, 0 replies; 20+ messages in thread
From: Sascha Hauer @ 2018-02-07  8:26 UTC (permalink / raw)
  To: Aleksander Morgado; +Cc: barebox

On Tue, Feb 06, 2018 at 05:51:59PM +0100, Aleksander Morgado wrote:
> On Tue, Feb 6, 2018 at 10:33 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > On Fri, Feb 02, 2018 at 12:14:39PM +0100, Aleksander Morgado wrote:
> >> Signed-off-by: Aleksander Morgado <aleksander@aleksander.es>
> >> ---
> >>  commands/Makefile    |  1 +
> >>  commands/ratp-ping.c | 38 ++++++++++++++++++++++++++++++++++++++
> >>  common/ratp.c        | 27 ---------------------------
> >
> > Can we put the ratp commands to a separate directory like
> > common/ratp/commands/ or similar?
> > There doesn't seem to be very much code sharing between the regular
> > commands and their ratp correspondents. Most good commands are written
> > in the way that the functionality is in common code outside the command
> > file itself anyway.
> >
> 
> I actually did that originally, but then implemented "md" for RATP and
> decided I could reuse part of the code doing the memory dump. But then
> no other command I implemented shared anything with the console
> counterpart... so yes, totally agree changing that would be much
> better. I did think of moving them to commands/ratp/* instead of
> common/ratp/commands/* though. Why do you suggest to have them in
> common/? Shouldn't they be closer to the console commands in the
> hierarchy?

My idea probably was to have the ratp commands close to the ratp code,
thus moving ratp.c to common/ratp/ also.

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] 20+ messages in thread

* Re: [RFC PATCH 00/10] ratp: new generic RATP command support
  2018-02-06 16:43   ` Aleksander Morgado
@ 2018-02-07  8:33     ` Sascha Hauer
  0 siblings, 0 replies; 20+ messages in thread
From: Sascha Hauer @ 2018-02-07  8:33 UTC (permalink / raw)
  To: Aleksander Morgado; +Cc: barebox

On Tue, Feb 06, 2018 at 05:43:40PM +0100, Aleksander Morgado wrote:
> >>
> >> The first patches (1-5) break the current RATP API, by introducing
> >> the concept of requests, responses and indications:
> >>  * Requests sent to one endpoint are expected to be replied with
> >>    a response by the peer endpoint.
> >>  * Indications are messages sent from one endpoint to another which
> >>    are not expected to be replied.
> >
> > I do not see why we have to break the RATP API. I mean currently we
> > have BB_RATP_TYPE_COMMAND and BB_RATP_TYPE_COMMAND_RETURN which you
> > convert to .type = BB_RATP_TYPE_COMMAND, .flags = 0 | RESPONSE.
> >
> > I see that using flags looks somewhat nicer, but besides of that,
> > what is your selling point to break the API?
> >
> 
> Well, it was just easier to say "if I send a request of type X, I
> expect back a response of the same type X". It avoids the need of
> having to define which command id is expected as response for which
> command id request.

Well right now the lowest bit the type field can serve this purpose,
i.e. "if I send a request of type X, I expect back a response type X + 1"

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] 20+ messages in thread

* Re: [PATCH 06/10] ratp: implement generic command support
  2018-02-06 16:49     ` Aleksander Morgado
@ 2018-02-07  8:34       ` Sascha Hauer
  0 siblings, 0 replies; 20+ messages in thread
From: Sascha Hauer @ 2018-02-07  8:34 UTC (permalink / raw)
  To: Aleksander Morgado; +Cc: barebox

On Tue, Feb 06, 2018 at 05:49:10PM +0100, Aleksander Morgado wrote:
> >> +
> >> +#define BAREBOX_RATP_CMD_START(_name)                                                        \
> >> +extern const struct ratp_command __barebox_cmd_##_name;                                      \
> >
> > You shouldn't use the same name as the existing barebox commands,
> > otherwise there might be name clashes. Add some _ratp_ to it.
> >
> 
> Ah, sure.
> 
> >> +const struct ratp_command __barebox_cmd_##_name                                              \
> >> +     __attribute__ ((unused,section (".barebox_ratp_cmd_" __stringify(_name)))) = {  \
> >> +     .id             = BB_RATP_TYPE_##_name,
> >
> > I am not sure if I like the magic construction of the id field. Being
> > able to grep for BB_RATP_TYPE_PING and finding the user has advantages.
> > How about manually setting the field in the body of the command instead
> > of constructing it?
> >
> 
> You mean doing this instead?
> 
> BAREBOX_RATP_CMD_START(PING)
>     .cmd = ratp_cmd_ping,
>     .id = BB_RATP_TYPE_PING
> BAREBOX_RATP_CMD_END

Yes, that's what I meant.

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] 20+ messages in thread

end of thread, other threads:[~2018-02-07  8:34 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-02 11:14 [RFC PATCH 00/10] ratp: new generic RATP command support Aleksander Morgado
2018-02-02 11:14 ` [PATCH 01/10] ratp: define message type flags Aleksander Morgado
2018-02-02 11:14 ` [PATCH 02/10] ratp: port command operation to req/rsp/ind format Aleksander Morgado
2018-02-02 11:14 ` [PATCH 03/10] ratp: port ping operation to req/rsp format Aleksander Morgado
2018-02-02 11:14 ` [PATCH 04/10] ratp: port getenv " Aleksander Morgado
2018-02-02 11:14 ` [PATCH 05/10] ratp: port filesystem " Aleksander Morgado
2018-02-02 11:14 ` [PATCH 06/10] ratp: implement generic command support Aleksander Morgado
2018-02-06  9:30   ` Sascha Hauer
2018-02-06 16:49     ` Aleksander Morgado
2018-02-07  8:34       ` Sascha Hauer
2018-02-02 11:14 ` [PATCH 07/10] ratp: implement ping as a standard ratp command Aleksander Morgado
2018-02-06  9:33   ` Sascha Hauer
2018-02-06 16:51     ` Aleksander Morgado
2018-02-07  8:26       ` Sascha Hauer
2018-02-02 11:14 ` [PATCH 08/10] ratp: implement getenv " Aleksander Morgado
2018-02-02 11:14 ` [PATCH 09/10] ratp: new reset command Aleksander Morgado
2018-02-02 11:14 ` [PATCH 10/10] ratp: new md and mw commands Aleksander Morgado
2018-02-06  9:24 ` [RFC PATCH 00/10] ratp: new generic RATP command support Sascha Hauer
2018-02-06 16:43   ` Aleksander Morgado
2018-02-07  8:33     ` Sascha Hauer

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