Re: [PATCH 07/10] ratp: implement ping as a standard ratp command

2018-02-07 Thread Sascha Hauer
On Tue, Feb 06, 2018 at 05:51:59PM +0100, Aleksander Morgado wrote:
> On Tue, Feb 6, 2018 at 10:33 AM, Sascha Hauer  wrote:
> > On Fri, Feb 02, 2018 at 12:14:39PM +0100, Aleksander Morgado wrote:
> >> Signed-off-by: Aleksander Morgado 
> >> ---
> >>  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- |

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


Re: [PATCH 07/10] ratp: implement ping as a standard ratp command

2018-02-06 Thread Aleksander Morgado
On Tue, Feb 6, 2018 at 10:33 AM, Sascha Hauer  wrote:
> On Fri, Feb 02, 2018 at 12:14:39PM +0100, Aleksander Morgado wrote:
>> Signed-off-by: Aleksander Morgado 
>> ---
>>  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


Re: [PATCH 07/10] ratp: implement ping as a standard ratp command

2018-02-06 Thread Sascha Hauer
On Fri, Feb 02, 2018 at 12:14:39PM +0100, Aleksander Morgado wrote:
> Signed-off-by: Aleksander Morgado 
> ---
>  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- |

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


[PATCH 07/10] ratp: implement ping as a standard ratp command

2018-02-02 Thread Aleksander Morgado
Signed-off-by: Aleksander Morgado 
---
 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 0..837c56762
--- /dev/null
+++ b/commands/ratp-ping.c
@@ -0,0 +1,38 @@
+/*
+ * Copyright (c) 2018 Sascha Hauer , 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 
+#include 
+#include 
+
+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