On Mon, Jul 27, 2020 at 1:13 AM Kuninori Morimoto kuninori.morimoto.gx@renesas.com wrote:

Hi Daniel

Thank you for asking And sorry for late response, Japan was holiday.

...

Looking at snd_soc_component_driver I see there are some operations like: open, close, hw_params, hw_free. (1)

Now, snd_soc_component_driver has snd_compress_ops.

Do you think it is worth it to group operations from (1) in a similar structure maybe snd_<xyz>_ops.

It seems snd_soc_component_driver is using many functions and flags. Keeping these in the some structure is better, IMO.

I think separating "component" and "compress" is better cleaning ?

    struct snd_compress_ops {
            ...
    };

    struct snd_soc_component_driver {
            ...

•           const struct snd_compress_ops *compress_ops;
            ...
    };
  
    struct snd_soc_component {
            ...
            struct snd_soc_component_driver *driver;
  

•           const struct snd_compress_ops *compress_ops;
            ...
    };
  
  

Hi Morimoto-san,

Thanks for your answer.

Although I still have many months ahead to understand ASoC framework, I don't think moving compress_ops out of snd_soc_component_driver it is a good idea.

For me it looks like snd_soc_component_driver abstracts the functionality of a component so operations should still stay under snd_soc_component_driver.

Indeed snd_soc_component_driver has a lot of operations and flags, I think we can group the operations as follows:

* operations on PCM substreams (e.g: open(component,substream)) * operations on DT nodes (e.g: of_xlate_dai_id(component, device_node) * component operations: (e.g: set_pll(component) * constructor / destructor.

I think a first step would be to create an equivalent of snd_compress_ops for PCM substreams. The name ideally would be snd_pcm_ops and all the functions will have a component and a PCM substream.

Anyhow, snd_pcm_ops already exists and acts only on a component.

So, I think those operations should be called: * for PCM: snd_substream_pcm_ops * for Compress: rename snd_compr_ops with snd_stream_compr_ops.

Another thing I don't understand is why PCM uses 'substream' term while compress uses 'stream' term.

I must admit that my head hurts every time I try to understand ASoC related stuff. Thanks a lot for taking your time to clean it up and make the code easier to understand.