[PATCH v4] sound: rawmidi: Add framing mode

[Jaroslav Kysela]

Dne 12. 04. 21 v 12:04 Takashi Iwai napsal(a):
> On Mon, 12 Apr 2021 11:43:02 +0200,
> Jaroslav Kysela wrote:
>>
>> Dne 12. 04. 21 v 11:31 Takashi Iwai napsal(a):
>>> On Sun, 11 Apr 2021 19:52:21 +0200,
>>> Jaroslav Kysela wrote:
>>>>>>>   struct snd_rawmidi_params {
>>>>>>>   	int stream;
>>>>>>>   	size_t buffer_size;		/* queue size in bytes */
>>>>>>>   	size_t avail_min;		/* minimum avail bytes for wakeup */
>>>>>>>   	unsigned int no_active_sensing: 1; /* do not send active sensing byte in close() */
>>>>>>> -	unsigned char reserved[16];	/* reserved for future use */
>>>>>>> +	unsigned char framing;		/* For input data only, frame incoming data */
>>>>>> We can move this flag to above 32-bit word (no_active_sensing). I'm not sure,
>>>>>> if we need 8 bits for this. It's first change after 20 years. Another flag may
>>>>>> obsolete this one.
>>>>>
>>>>> Not sure what you mean by this, could you show the code? Framing is an 
>>>>> enum rather than a flag, in case we find other framing formats with 
>>>>> other sizes that would obsolete this one.
>>>>
>>>> unsigned int no_active_sensing: 1;
>>>> unsigned int framing32: 1;
>>>> unsigned int framing64: 1; /* future extension, framing32 == 0 in this case */
>>>>
>>>> or:
>>>>
>>>> unsigned int framing_mode: 3;	/* three bits for future types */
>>>>
>>>> perhaps also:
>>>>
>>>> unsigned int clock_type: 3;	/* three bits for the clock type selection */
>>>
>>> The usage of bit fields in an ioctl struct is a bad idea from the
>>> compat layer POV.  Let's avoid it.
>>
>> What exactly do you mean?
> 
> The compat layer has the hard time to deal with the conversion of bit
> fields to a different struct.  And, it's a nightmare for the emulator
> like QEMU.  If it has to convert the ioctl struct, it has to consider
> about the byte order as well (and there is no strict definition how to
> put the bit fields in C language).

The endianness of the 32-bit word can be changed quite easily - and the
situation is similar to bit flags stored in the 32-bit word. Nobody prevents
QEMU to work with the whole 32-bit word instead bit fields in this case.

The linux compat layer may copy the whole 32-bit word, too.

I think that your argument is not so strong here.

> That said, a bit field can be useful for the internal object
> definition (if there is no racy access), but not for an object for the
> communication that may be interpreted among different architectures.

In this case, we have 31 free bits and this information can be stored there. I
would prefer to keep the reserved bytes for some large fields.

					Jaroslav

-- 
Jaroslav Kysela <perex at perex.cz>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.