On 04-09-19, 08:31, Pierre-Louis Bossart wrote: > On 9/4/19 2:35 AM, Vinod Koul wrote: > > On 22-08-19, 08:53, Pierre-Louis Bossart wrote: > > > Thanks for the review Guennadi > > > > > > > > +static int sdw_config_stream(void *arg, void *s, void *dai, > > > > > + void *params, int link_id, int alh_stream_id) > > > > > > > > I realise, that these function prototypes aren't being introduced by these > > > > patches, but just wondering whether such overly generic prototype is really > > > > a good idea here, whether some of those "void *" pointers could be given > > > > real types. The first one could be "struct device *" etc. > > > > > > In this case the 'arg' parameter is actually a private 'struct snd_sof_dev', > > > as shown below [1]. We probably want to keep this relatively opaque, this is > > > a context that doesn't need to be exposed to the SoundWire code. > > > > This does look bit ugly. > > > > > The dai and params are indeed cases where we could use stronger types, they > > > are snd_soc_dai and hw_params respectively. I don't recall why the existing > > > code is this way, Vinod and Sanyog may have the history of this. > > > > Yes we wanted to decouple the sdw and audio bits that is the reason why > > none of the audio types are used here, but I think it should be revisited > > and perhaps made as: > > > > sdw_config_stream(struct device *sdw, struct sdw_callback_ctx *ctx) > > > > where the callback context contains all the other args. That would make > > it look lot neater too and of course use real structs if possible > > the suggested sdw_callbback_ctx is really intel-specific at the moment, e.g. > the notion of link_id and alh_stream_id are due to the hardware, it's not > generic at all. And in the latest code we also pass the dai->id. s/sdw_callback_ctx/intel_sdw_callback_ctx Yes this code is intel specific and this would be intel specific too -- ~Vinod _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
