[alsa-devel] [PATCH 08/13] ASoC: Add SoundWire stream programming interface

[Takashi Iwai]

On Mon, 02 Apr 2018 08:26:35 +0200,
Takashi Sakamoto wrote:
> 
> Hi Greg,
> 
> I'm sorry for delay but I had a short trip.
> 
> On Mar 30 2018 15:03, Greg KH wrote:
> > On Fri, Mar 30, 2018 at 12:05:00PM +0900, Takashi Sakamoto wrote:
> >> Hi,
> >>
> >> On Mar 28 2018 18:38, Vinod Koul wrote:
> >>> diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
> >>> index c0edac80df34..690e56a35237 100644
> >>> --- a/sound/soc/soc-core.c
> >>> +++ b/sound/soc/soc-core.c
> >>> @@ -2882,6 +2882,26 @@ int snd_soc_dai_set_tdm_slot(struct snd_soc_dai *dai,
> >>>    EXPORT_SYMBOL_GPL(snd_soc_dai_set_tdm_slot);
> >>>    /**
> >>> + * snd_soc_dai_set_sdw_stream() - Configures a DAI for SDW stream operation
> >>> + * @dai: DAI
> >>> + * @stream: STREAM
> >>> + * @direction: Stream direction(Playback/Capture)
> >>> + * SoundWire subsystem doesn't have a notion of direction and we reuse
> >>> + * the ASoC stream direction to configure sink/source ports.
> >>> + *
> >>> + * Returns 0 on success, a negative error code otherwise.
> >>> + */
> >>> +int snd_soc_dai_set_sdw_stream(struct snd_soc_dai *dai,
> >>> +				void *stream, int direction)
> >>> +{
> >>> +	if (dai->driver->ops->set_sdw_stream)
> >>> +		return dai->driver->ops->set_sdw_stream(dai, stream, direction);
> >>> +	else
> >>> +		return -ENOTSUPP;
> >>> +}
> >>> +EXPORT_SYMBOL_GPL(snd_soc_dai_set_sdw_stream);
> >>
> >> It's better for this kind of code to be incline function in any header. In
> >> general, new symbols increase maintenance cost of binary of kernel-related
> >> stuffs. It's preferable to avoid the addition if possible, IMO.
> >
> > I don't understand, functionally it's the same, there should not be any
> > increased maintenance either way.  Please explain how this makes things
> > "harder"?
> 
> Hm, if so it might be my misunderstanding to reasons for typical usage
> of inline functions in kernel source, sorry.
> 
> In my understanding, exported symbols are put into some sections of
> ELF binary. Addition of new symbols increases size of the
> section. Additionally, after linking vmlinux, kbuild scans built-in
> symbols and make a file with entries of them. The addition increases
> time of this step. Furthermore, at the end of building kernel, kmod is
> called to generate some map files for exported symbols in loadable
> module. In a view of distributors, these files are maintained by
> binary packages of any type carefully because some incompatibilities
> can be delivered such as version mismatch.
> 
> For these reasons, I think thing goes harder when people carelessly
> add new symbols for functions with a few codes; e.g. accessing to a
> member of structure, then simply check an return it. Actually I can
> see some examples in upstreamed headers.

The advantage of inline function isn't about the maintenance cost.
It's mostly for performance, as well as the binary size reduction.

Actually, when a kernel live-patching comes into play, an inline
function is worse from the maintenance POV.  Then we'd have to patch
every place that is expanded instead of a single place.

However, this doesn't discourage the use of inline function, either.
Overall, the performance is still the most important factor for major
cases.  So I agree with that this particular case can be well inlined,
supposing that no complex code is planned to be added in future.


thanks,

Takashi