RE: [PATCH v7 03/14] ASoC: Intel: catpt: Add IPC message handlers

["Rojewski, Cezary" <cezary.rojewski@intel.com>]

On 2020-09-21 4:23 PM, Andy Shevchenko wrote:
> On Mon, Sep 21, 2020 at 05:20:12PM +0300, Andy Shevchenko wrote:
>> On Mon, Sep 21, 2020 at 03:58:33PM +0200, Amadeusz Sławiński wrote:
>>> On 9/21/2020 2:59 PM, Andy Shevchenko wrote:
>>>>> +struct catpt_set_volume_input {
>>>>> +	u32 channel;
>>>>> +	u32 target_volume;
>>>>> +	u64 curve_duration;
>>>>> +	enum catpt_audio_curve_type curve_type __aligned(4);
>>>>> +} __packed;
>>>> How this __packed changes anything? In general __packed doesn't make sense for
>>>> in-kernel data structures. Otherwise you have to use proper (POD) types for
>>>> data. Ditto for all similar cases.
>>>
>>> All of __packed use in code is done on structures used to communicate with
>>> FW, which is binary interface, so it is not kernel only structure, as it is
>>> also FW one. While we can expect compiler to do the right thing, I consider
>>> it is better to be explicit about what kind of data we are handling, so
>>> there aren't any surprises.
>>
>> Size of enum is compiler defined. It may not be used in the ABIs.
> 
> I have to elaborate that I'm talking in ABIs which implies different compilers
> and even may be run on different CPU architectures.
> 
>> __uXX vs. uXX I dunno.
> 
> And here I'm talking about FW <--> OS interface. It's not user visible ABI, but
> still some Ext <--> Int protocol. I saw uXX types in data structures of FW
> communication protocols.
> 

Will remove enum members from IPC structs in v8 as requested.

Thanks,
Czarek