Re: [PATCH 06/10] ratp: implement generic command support
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- |
___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
Re: [PATCH 06/10] ratp: implement generic command support
>> +
>> +#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
Re: [PATCH 06/10] ratp: implement generic command support
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
> ---
> @@ -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- |
___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
