Hi Jerome, Mark Thank you for your feedback > > I understand you are trying to fold some code but I'm not sure about this > > snd_soc_daifmt_clock_provider_pickup(). > > > Instead of repeating the if clause around DAIFMT (which is a bit verbose > > but fairly easy to understand and maintain) there is now the calculation > > of a made up constant (which is rather opaque as it is), which later > > translate to the same type of test around DAIFMT. > > > I'd be in favor of dropping the snd_soc_daifmt_clock_provider_pickup() > > part for the sake or readability. This apply to the rest of the series, > > not just fsl. > > Yeah, I'm also just finding the _pickup() name unclear and can't really > immediately think how to make it clearer - I think the bit being > factored out needs to be at least as complex/unclear as the interface > being introduced to do so. Because there are many kind of use case, it was difficult to do this thing by 1 function. And indeed the naming was unclear. Main purpose of this patch-set was that I want to have clock_provider related functions. But indeed it was a little overkill to force it to use it to existing drivers. I will fixup it in v2 Thank you for your help !! Best regards --- Kuninori Morimoto