linux-kernel - Re: [PATCH v2 2/2] ASoC: qcom: SC7280: Add machine driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

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);
> +}
> +