+#include <linux/err.h> +#include <linux/init.h> +#include <linux/module.h> +#include <linux/device.h>
alphabetical order?
+#include <linux/platform_device.h> +#include <linux/slab.h> +#include <linux/iommu.h> +#include <linux/dma-mapping.h> +#include <linux/dma-map-ops.h>
+#include <sound/pcm.h> +#include <sound/soc.h> +#include <sound/soc-usb.h> +#include <sound/pcm_params.h> +#include <sound/asound.h> +#include <sound/q6usboffload.h>
+#include "q6dsp-lpass-ports.h" +#include "q6afe.h"
+#define SID_MASK 0xF
Prefix? e.g. Q6_USB_SID_MASK?
+struct q6usb_port_data {
- struct q6afe_usb_cfg usb_cfg;
- struct snd_soc_usb *usb;
- struct q6usb_offload priv;
- int active_idx;
index of what?
+};
+static const struct snd_soc_dapm_widget q6usb_dai_widgets[] = {
- SND_SOC_DAPM_HP("USB_RX_BE", NULL),
+};
+static const struct snd_soc_dapm_route q6usb_dapm_routes[] = {
- {"USB Playback", NULL, "USB_RX_BE"},
+};
+static int q6usb_hw_params(struct snd_pcm_substream *substream,
struct snd_pcm_hw_params *params,struct snd_soc_dai *dai)+{
- return 0;
+}
+static const struct snd_soc_dai_ops q6usb_ops = {
- .hw_params = q6usb_hw_params,
+};
+static struct snd_soc_dai_driver q6usb_be_dais[] = {
- {
.playback = {.stream_name = "USB BE RX",.rates = SNDRV_PCM_RATE_8000 | SNDRV_PCM_RATE_11025 |SNDRV_PCM_RATE_16000 | SNDRV_PCM_RATE_22050 |SNDRV_PCM_RATE_32000 | SNDRV_PCM_RATE_44100 |SNDRV_PCM_RATE_48000 | SNDRV_PCM_RATE_96000 |SNDRV_PCM_RATE_192000,.formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S16_BE |SNDRV_PCM_FMTBIT_U16_LE | SNDRV_PCM_FMTBIT_U16_BE |SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S24_BE |SNDRV_PCM_FMTBIT_U24_LE | SNDRV_PCM_FMTBIT_U24_BE,.channels_min = 1,.channels_max = 2,.rate_max = 192000,.rate_min = 8000,},.id = USB_RX,.name = "USB_RX_BE",.ops = &q6usb_ops,- },
+};
This is a bit confusing.
In patch 9/34 you have a
static struct snd_soc_dai_driver q6dsp_audio_fe_dais[] = {
but this was not described anywhere in your cover letter.
or maybe this referred to the 'Q6AFE "cpu"', in which case you have inconsistent naming between q6dsp and q6afe?
+static int q6usb_audio_ports_of_xlate_dai_name(struct snd_soc_component *component,
const struct of_phandle_args *args,const char **dai_name)+{
- int id = args->args[0];
- int ret = -EINVAL;
- int i;
- for (i = 0; i < ARRAY_SIZE(q6usb_be_dais); i++) {
if (q6usb_be_dais[i].id == id) {*dai_name = q6usb_be_dais[i].name;ret = 0;break;}- }
- return ret;
+}
+static int q6usb_alsa_connection_cb(struct snd_soc_usb *usb,
struct snd_soc_usb_device *sdev, bool connected)+{
- struct q6usb_port_data *data;
- if (!usb->component)
return -ENODEV;- data = dev_get_drvdata(usb->component->dev);
- if (connected) {
/* We only track the latest USB headset plugged in */data->active_idx = sdev->card_idx;- }
- return 0;
+}
+static int q6usb_component_probe(struct snd_soc_component *component) +{
- struct q6usb_port_data *data = dev_get_drvdata(component->dev);
- data->usb = snd_soc_usb_add_port(component->dev, &data->priv, q6usb_alsa_connection_cb);
There is a conceptual problem here wrt. resource allocation.
snd_soc_usb_add_port() uses devm_kzalloc().
This cannot be used in a component probe, this has to be used in a real driver probe.
- if (IS_ERR(data->usb)) {
dev_err(component->dev, "failed to add usb port\n");return -ENODEV;- }
- data->usb->component = component;
- return 0;
+}
+static void q6usb_component_remove(struct snd_soc_component *component) +{
- snd_soc_usb_remove_port(component->dev);
and here as well, this should free the "struct snd_soc_usb *usb" allocated in the probe. relying on devm_ is not quite right. Or if you use devm_, you have to call it from the platform driver .probe.
+static struct platform_driver q6usb_dai_platform_driver = {
- .driver = {
.name = "q6usb-dai",.of_match_table = of_match_ptr(q6usb_dai_device_id),- },
- .probe = q6usb_dai_dev_probe,
- /*
* Remove not required as resources are cleaned up as part of* component removal. Others are device managed resources.*/+}; +module_platform_driver(q6usb_dai_platform_driver);
+MODULE_DESCRIPTION("Q6 USB backend dai driver"); +MODULE_LICENSE("GPL");