Re: [Embedded-pv-devel] [PATCH v10] xen: add para-virtual sound interface header file

[Oleksandr Andrushchenko <andr2000@xxxxxxxxx>]

>> +struct xensnd_close_req {
>> +};
>
> I'm afraid structures without any members aren't permitted by C89.
>
There are 2 options I see:
1. Remove empty structures
2. Leave them in, but add a comment and add a dummy place holder
I would prefer option 1 as this way the interface seems to be clearer,
like all requests are defined etc.
>> +struct xensnd_request {
>> +     uint8_t raw[64];
>> +};
>> +
>> +struct xensnd_response {
>> +     uint8_t raw[64];
>> +};
>
> What are these two needed for now? If you want to enforce a
> minimum size, that would better go ...
>
That is because we want to be cache line aligned. What it means in practice
is not clear as we have so many HW around, so it is not possible to
fit all. So the options could be:
1. 32 or 64
2. 48 I think is not an option here
I would stick to 32
>> +union xensnd_req {
>> +     struct xensnd_request raw;
>> +     struct {
>> +             uint16_t id;
>> +             uint8_t operation;
>> +             uint8_t stream_idx;
>> +             uint32_t padding;
>> +             union {
>> +                     struct xensnd_open_req open;
>> +                     struct xensnd_close_req close;
>> +                     struct xensnd_write_req write;
>> +                     struct xensnd_read_req read;
>> +                     struct xensnd_get_vol_req get_vol;
>> +                     struct xensnd_set_vol_req set_vol;
>> +                     struct xensnd_mute_req mute;
>> +                     struct xensnd_unmute_req unmute;
>
> ... here.
>
> Also please observe ./CODING_STYLE (we don't use tabs for
> indentation).
>
My fault, was thinking Linux kernel codestyle as the patch is in the kernel,
but actully platform independent - will fix
> Jan
>
Oleksandr Andrushchenko

_______________________________________________
Embedded-pv-devel mailing list
Embedded-pv-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/cgi-bin/mailman/listinfo/embedded-pv-devel