[alsa-devel] [PATCH v2 1/7] ASoC: omap: Introduce the generic_dmaengine_pcm based sdma-pcm

[Sebastian Reichel]

Hi,

On Mon, Apr 30, 2018 at 02:05:06PM +0300, Peter Ujfalusi wrote:
> On 2018-04-30 13:55, Sebastian Reichel wrote:
> > On Mon, Apr 30, 2018 at 09:57:42AM +0300, Peter Ujfalusi wrote:
> >> [...]
> >> diff --git a/sound/soc/omap/sdma-pcm.h b/sound/soc/omap/sdma-pcm.h
> >> new file mode 100644
> >> index 000000000000..ce13edfc52d8
> >> --- /dev/null
> >> +++ b/sound/soc/omap/sdma-pcm.h
> >> @@ -0,0 +1,21 @@
> >> +/* SPDX-License-Identifier: GPL-2.0 */
> >> +/*
> >> + *  Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com
> >> + *  Author: Peter Ujfalusi <peter.ujfalusi at ti.com>
> >> + */
> >> +
> >> +#ifndef __SDMA_PCM_H__
> >> +#define __SDMA_PCM_H__
> >> +
> >> +#if IS_ENABLED(CONFIG_SND_SDMA_SOC)
> >> +int sdma_pcm_platform_register(struct device *dev,
> >> +			       char *txdmachan, char *rxdmachan);
> >> +#else
> >> +static inline int sdma_pcm_platform_register(struct device *dev,
> >> +					     char *txdmachan, char *rxdmachan)
> >> +{
> >> +	return 0;
> > 
> > I would expect some error code instead?
> 
> Yeah, it could return -ENODEV.

I think it should. Returning success without providing the intended
functionality is bad API design.

> It is there so the McASP can be compiled for daVinci/am335x/am43xx where
> we do not have sDMA, only EDMA.

Are you sure, that you need the stub at all? Looking at the next
patch the call is guarded with #if IS_BUILTIN(CONFIG_SND_SDMA_SOC)
|| IS_MODULE(CONFIG_SND_SDMA_SOC). I don't think it is called with
CONFIG_SND_SDMA_SOC being disabled.

-- Sebastian
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://mailman.alsa-project.org/pipermail/alsa-devel/attachments/20180430/66000c25/attachment.sig>