[alsa-devel] [PATCH] [RFC 4/13] Intel SST driver common private headers

[Harsha, Priya]

>> +/*
>> +SST shim registers to structure mapping
>> +*/
>> +union config_status_reg {
>> +	struct {
>> +		u32 rsvd0:1;
>> +		u32 sst_reset:1;
>> +		u32 hw_rsvd:3;
>> +		u32 sst_clk:2;
>> +		u32 bypass:3;
>> +		u32 run_stall:1;
>> +		u32 rsvd:21;
>> +	} part;
>> +	u32 full;
>
>A union with bit fields is very bad for portability.
>If it's used as a communication parameter, don't use it.
>Just pass u32 and get/set via normal bit and/or operations.

We communicate with firmare using a 32 bit register only. The bits of 32 bit register mean different things. So this structure is just a way for use to read/write it using .full and manuplate using .part.field. 
I am not sure I understand where it fails in portability. Can you please help elaborate a little here?


>> +struct snd_sst_user_cap_list {
>> +	unsigned int iov_index; /*index of iov*/
>> +	unsigned long iov_offset; /*offset in iov*/
>> +	unsigned long offset; /*offset in kmem*/
>> +	unsigned long size; /*size copied*/
>
>Sure to use long here?
No. we will correct it to use u64


>
>Well, all these shouldn't be inline if you have no concrete reason
>to inline them.  It's no C++ template.  Better to put as normal functions.
Sure. Will change them accordingly.

Thanks,
Harsha