Re: [PATCH] drivers: most: add ALSA sound driver

[Greg KH <gregkh@linuxfoundation.org>]

On Mon, Nov 02, 2020 at 04:14:03PM +0100, Christian Gromm wrote:
> This patch moves the ALSA sound driver out of the staging area and adds it
> to the stable part of the MOST driver. Modifications to the Makefiles and
> Kconfigs are done accordingly to not break the build.
> 
> Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
> ---
>  drivers/most/Kconfig                |  10 +
>  drivers/most/Makefile               |   1 +
>  drivers/most/most_snd.c             | 753 ++++++++++++++++++++++++++++++++++++
>  drivers/staging/most/Kconfig        |   2 -
>  drivers/staging/most/Makefile       |   1 -
>  drivers/staging/most/sound/Kconfig  |  14 -
>  drivers/staging/most/sound/Makefile |   4 -
>  drivers/staging/most/sound/sound.c  | 753 ------------------------------------
>  8 files changed, 764 insertions(+), 774 deletions(-)
>  create mode 100644 drivers/most/most_snd.c
>  delete mode 100644 drivers/staging/most/sound/Kconfig
>  delete mode 100644 drivers/staging/most/sound/Makefile
>  delete mode 100644 drivers/staging/most/sound/sound.c
> 
> diff --git a/drivers/most/Kconfig b/drivers/most/Kconfig
> index ebfe84e..4b8145b 100644
> --- a/drivers/most/Kconfig
> +++ b/drivers/most/Kconfig
> @@ -32,4 +32,14 @@ config MOST_CDEV
>  
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called most_cdev.
> +
> +config MOST_SND
> +	tristate "Sound"
> +	depends on SND
> +	select SND_PCM
> +	help
> +	  Say Y here if you want to commumicate via ALSA/sound devices.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called most_sound.
>  endif
> diff --git a/drivers/most/Makefile b/drivers/most/Makefile
> index 8b53ca4..60db6cd 100644
> --- a/drivers/most/Makefile
> +++ b/drivers/most/Makefile
> @@ -5,3 +5,4 @@ most_core-y :=	core.o \
>  
>  obj-$(CONFIG_MOST_USB_HDM) += most_usb.o
>  obj-$(CONFIG_MOST_CDEV) += most_cdev.o
> +obj-$(CONFIG_MOST_SND) += most_snd.o
> diff --git a/drivers/most/most_snd.c b/drivers/most/most_snd.c
> new file mode 100644
> index 0000000..8a449ab
> --- /dev/null
> +++ b/drivers/most/most_snd.c
> @@ -0,0 +1,753 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * sound.c - Sound component for Mostcore
> + *
> + * Copyright (C) 2015 Microchip Technology Germany II GmbH & Co. KG
> + */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/module.h>
> +#include <linux/printk.h>
> +#include <linux/kernel.h>
> +#include <linux/slab.h>
> +#include <linux/init.h>
> +#include <sound/core.h>
> +#include <sound/pcm.h>
> +#include <sound/pcm_params.h>
> +#include <linux/sched.h>
> +#include <linux/kthread.h>
> +#include <linux/most.h>
> +
> +#define DRIVER_NAME "sound"
> +#define STRING_SIZE	80
> +
> +static struct most_component comp;
> +
> +/**
> + * struct channel - private structure to keep channel specific data
> + * @substream: stores the substream structure
> + * @iface: interface for which the channel belongs to
> + * @cfg: channel configuration
> + * @card: registered sound card
> + * @list: list for private use
> + * @id: channel index
> + * @period_pos: current period position (ring buffer)
> + * @buffer_pos: current buffer position (ring buffer)
> + * @is_stream_running: identifies whether a stream is running or not
> + * @opened: set when the stream is opened
> + * @playback_task: playback thread
> + * @playback_waitq: waitq used by playback thread
> + */
> +struct channel {
> +	struct snd_pcm_substream *substream;
> +	struct snd_pcm_hardware pcm_hardware;
> +	struct most_interface *iface;
> +	struct most_channel_config *cfg;
> +	struct snd_card *card;
> +	struct list_head list;
> +	int id;
> +	unsigned int period_pos;
> +	unsigned int buffer_pos;
> +	bool is_stream_running;
> +	struct task_struct *playback_task;
> +	wait_queue_head_t playback_waitq;
> +	void (*copy_fn)(void *alsa, void *most, unsigned int bytes);
> +};
> +
> +struct sound_adapter {
> +	struct list_head dev_list;
> +	struct most_interface *iface;
> +	struct snd_card *card;
> +	struct list_head list;
> +	bool registered;
> +	int pcm_dev_idx;
> +};
> +
> +static struct list_head adpt_list;
> +
> +#define MOST_PCM_INFO (SNDRV_PCM_INFO_MMAP | \
> +		       SNDRV_PCM_INFO_MMAP_VALID | \
> +		       SNDRV_PCM_INFO_BATCH | \
> +		       SNDRV_PCM_INFO_INTERLEAVED | \
> +		       SNDRV_PCM_INFO_BLOCK_TRANSFER)
> +
> +#define swap16(val) ( \
> +	(((u16)(val) << 8) & (u16)0xFF00) | \
> +	(((u16)(val) >> 8) & (u16)0x00FF))
> +
> +#define swap32(val) ( \
> +	(((u32)(val) << 24) & (u32)0xFF000000) | \
> +	(((u32)(val) <<  8) & (u32)0x00FF0000) | \
> +	(((u32)(val) >>  8) & (u32)0x0000FF00) | \
> +	(((u32)(val) >> 24) & (u32)0x000000FF))

Doesn't swab16() and swab32() work for this?  I don't think you need to
reimplement these.

> +
> +static void swap_copy16(u16 *dest, const u16 *source, unsigned int bytes)
> +{
> +	unsigned int i = 0;
> +
> +	while (i < (bytes / 2)) {
> +		dest[i] = swap16(source[i]);
> +		i++;
> +	}
> +}
> +
> +static void swap_copy24(u8 *dest, const u8 *source, unsigned int bytes)
> +{
> +	unsigned int i = 0;
> +
> +	while (i < bytes - 2) {
> +		dest[i] = source[i + 2];
> +		dest[i + 1] = source[i + 1];
> +		dest[i + 2] = source[i];
> +		i += 3;
> +	}
> +}
> +
> +static void swap_copy32(u32 *dest, const u32 *source, unsigned int bytes)
> +{
> +	unsigned int i = 0;
> +
> +	while (i < bytes / 4) {
> +		dest[i] = swap32(source[i]);
> +		i++;
> +	}
> +}

Same for the above, don't we have functions for this?

thanks,

greg k-h