[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAE-0n50i9rm6fcuyjCCPXjtxTyXwAiRVx91dXT4BDpbGA-tKjg@mail.gmail.com>
Date: Mon, 13 Sep 2021 12:49:24 -0700
From: Stephen Boyd <swboyd@...omium.org>
To: Srinivasa Rao Mandadapu <srivasam@...eaurora.org>,
agross@...nel.org, alsa-devel@...a-project.org,
bgoswami@...eaurora.org, bjorn.andersson@...aro.org,
broonie@...nel.org, devicetree@...r.kernel.org,
judyhsiao@...omium.org, lgirdwood@...il.com,
linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org,
perex@...ex.cz, plai@...eaurora.org, robh+dt@...nel.org,
rohitkr@...eaurora.org, srinivas.kandagatla@...aro.org,
tiwai@...e.com
Subject: Re: [PATCH v2 2/2] ASoC: qcom: SC7280: Add machine driver
Quoting Srinivasa Rao Mandadapu (2021-09-13 06:17:42)
> diff --git a/sound/soc/qcom/sc7280.c b/sound/soc/qcom/sc7280.c
> new file mode 100644
> index 0000000..906910c
> --- /dev/null
> +++ b/sound/soc/qcom/sc7280.c
> @@ -0,0 +1,343 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +//
> +// Copyright (c) 2020-2021, The Linux Foundation. All rights reserved.
> +//
> +// sc7280.c -- ALSA SoC Machine driver for sc7280
> +
> +#include <linux/gpio.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <sound/core.h>
> +#include <sound/jack.h>
> +#include <sound/pcm.h>
> +#include <sound/soc.h>
> +#include <uapi/linux/input-event-codes.h>
Looks like the include should be <linux/input.h> instead. I see that
practically no other code in the kernel is including the uapi header as
it's for userspace, not kernel. The uapi header is included in input.h
though so it's not actually all that different.
> +
> +#include <dt-bindings/sound/sc7180-lpass.h>
> +#include <dt-bindings/sound/qcom,q6afe.h>
> +
> +#include "../codecs/wcd938x.h"
> +#include "common.h"
> +#include "lpass.h"
> +
[...]
> +static int sc7280_snd_platform_probe(struct platform_device *pdev)
> +{
> + struct snd_soc_card *card;
> + struct sc7280_snd_data *data;
> + struct device *dev = &pdev->dev;
> + struct snd_soc_dai_link *link;
> + int ret, i;
> +
> + /* Allocate the private data */
This comment is worthless.
> + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + card = &data->card;
> + snd_soc_card_set_drvdata(card, data);
> +
> + card->owner = THIS_MODULE;
> + card->driver_name = "SC7280";
> + card->dev = dev;
> +
> + ret = qcom_snd_parse_of(card);
> + if (ret)
> + return ret;
> +
> + for_each_card_prelinks(card, i, link) {
> + link->init = sc7280_init;
> + link->ops = &sc7280_ops;
> + }
Nitpick: Newline here.
> + return devm_snd_soc_register_card(dev, card);
> +}
> +
Powered by blists - more mailing lists