From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-9.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id F0D56C433E2 for ; Fri, 11 Sep 2020 17:21:54 +0000 (UTC) Received: from alsa0.perex.cz (alsa0.perex.cz [77.48.224.243]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 017FF221E7 for ; Fri, 11 Sep 2020 17:21:53 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=alsa-project.org header.i=@alsa-project.org header.b="BaKgbC2p"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="nrtqiqrz" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 017FF221E7 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=chromium.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=alsa-devel-bounces@alsa-project.org Received: from alsa1.perex.cz (alsa1.perex.cz [207.180.221.201]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by alsa0.perex.cz (Postfix) with ESMTPS id 4B2111668; Fri, 11 Sep 2020 19:21:02 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz 4B2111668 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1599844912; bh=27obaAe0J9xkRrJy6AGRuW0PV6bkacveLtK4TJczKQI=; h=References:In-Reply-To:From:Date:Subject:To:Cc:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=BaKgbC2pHOdXUkPnN5zFJ6mb1i5ylVLMtlg6OA8sVL7ktK+Yeq7Fl80d+wiYGIwOX j/89MI8xUxW+fY0EPiAo/vM6GvUr/E39lv/Yy6CJEU+W7YRyQK9fqsWxFilis/zqk7 6KZe+MB76jFzu9b8d6aj85XUKc6d1NY+tR96Lolw= Received: from alsa1.perex.cz (localhost.localdomain [127.0.0.1]) by alsa1.perex.cz (Postfix) with ESMTP id BD5FFF8021C; Fri, 11 Sep 2020 19:21:01 +0200 (CEST) Received: by alsa1.perex.cz (Postfix, from userid 50401) id 2C344F8021C; Fri, 11 Sep 2020 19:21:00 +0200 (CEST) Received: from mail-wm1-x344.google.com (mail-wm1-x344.google.com [IPv6:2a00:1450:4864:20::344]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by alsa1.perex.cz (Postfix) with ESMTPS id DD121F80100 for ; Fri, 11 Sep 2020 19:20:50 +0200 (CEST) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz DD121F80100 Authentication-Results: alsa1.perex.cz; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="nrtqiqrz" Received: by mail-wm1-x344.google.com with SMTP id b79so5488879wmb.4 for ; Fri, 11 Sep 2020 10:20:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=y0IusjOAMzlu0LEffu8/teWA22xcDWq2yUsAVTXhPgg=; b=nrtqiqrzQFIINClpSv3fM9Bge5ZuVLwMEJ0o5IkBjDr289chYtyE1GrCw6/3gVtVK6 Djn9lLs7l/lwQgTkizVLyTBOOj3jttfWOp+DY8X9hJs5bZRs74/APQ1g18cg1iGNND0E z9pHWqbslF6YHfnP8lWH30Hr7rGkduixM3k3g= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=y0IusjOAMzlu0LEffu8/teWA22xcDWq2yUsAVTXhPgg=; b=IdlAEFWud1Zs4ISmwU/+Q4s+jVtjnI1X+2jY10rdp/SAKT4QHRoNEGshQWarHO8Jl4 udza60XxMSmVEez4XCMtuDVRjG0pHv8j7zymUBc40PZZN7/pGXEy4eEoAt4YaUE/zkZW 3mQNr+DI5s9juOYWQ5+n53YUw79Aw+J9wMLmLsXR3H3yLnFJIbXn4pl4QFZOVQ1WRNYI 3Bj9VOL9qSzeGVEVRqr24ps1BQIHWb7JIuzz4AnpbUJCDqbSXxma4uRr3K8qh50B1STG hRRjEYOsRaDQsMc1x3y0pt0fcdQBCWPevbBDpkBpe3duXFn1uP12Bg793GtT/6lrrTei jLtg== X-Gm-Message-State: AOAM531x8C1il3u3RX8SewLtIucIQz2MXTAHlH+tTy5/wTDYHYwPch98 3mOTbApW9zldYtEXY2isYmFm9KvqPuM1KRaVgHssPg== X-Google-Smtp-Source: ABdhPJwdBoOxRZoIzMjovIzxIonxRQ3iSgblHos1YDPe0j4W4/ySaBtzWvU7O7+Nsx5h1p5/wZOlP1cZ1kBU4K/v1Ns= X-Received: by 2002:a1c:1f42:: with SMTP id f63mr3104803wmf.1.1599844848452; Fri, 11 Sep 2020 10:20:48 -0700 (PDT) MIME-Version: 1.0 References: <20200911102259.3667381-1-cychiang@chromium.org> <20200911102259.3667381-4-cychiang@chromium.org> <20200911150044.GA2352@gerhold.net> In-Reply-To: <20200911150044.GA2352@gerhold.net> From: Cheng-yi Chiang Date: Sat, 12 Sep 2020 01:20:17 +0800 Message-ID: Subject: Re: [PATCH v9 3/3] ASoC: qcom: sc7180: Add machine driver for sound card registration To: Stephan Gerhold Content-Type: text/plain; charset="UTF-8" Cc: Taniya Das , "moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM..." , Banajit Goswami , Heiko Stuebner , Liam Girdwood , Rohit kumar , Patrick Lai , Takashi Iwai , Ajit Pandey , "open list:ARM/Rockchip SoC..." , Andy Gross , Dylan Reid , "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" , Tzung-Bi Shih , Srinivasa Rao , linux-arm-msm , Rob Herring , "moderated list:ARM/Mediatek SoC support" , Matthias Brugger , Bjorn Andersson , Linux ARM , Doug Anderson , linux-kernel , Mark Brown , Srini Kandagatla X-BeenThere: alsa-devel@alsa-project.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: "Alsa-devel mailing list for ALSA developers - http://www.alsa-project.org" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: "Alsa-devel" On Fri, Sep 11, 2020 at 11:01 PM Stephan Gerhold wrote: > > Hi, > > Thanks for removing the weird use of auxilliary devices :) > Hi Stephan, Thanks for the suggestion! > On Fri, Sep 11, 2020 at 06:22:59PM +0800, Cheng-Yi Chiang wrote: > > From: Ajit Pandey > > > > Add new driver to register sound card on sc7180 trogdor board and > > do the required configuration for lpass cpu dai and external codecs > > connected over MI2S interfaces. > > > > Signed-off-by: Ajit Pandey > > Signed-off-by: Cheng-Yi Chiang > > --- > > sound/soc/qcom/Kconfig | 12 ++ > > sound/soc/qcom/Makefile | 2 + > > sound/soc/qcom/sc7180.c | 267 ++++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 281 insertions(+) > > create mode 100644 sound/soc/qcom/sc7180.c > > > > diff --git a/sound/soc/qcom/Kconfig b/sound/soc/qcom/Kconfig > > index a607ace8b089..0459185ee243 100644 > > --- a/sound/soc/qcom/Kconfig > > +++ b/sound/soc/qcom/Kconfig > > @@ -116,4 +116,16 @@ config SND_SOC_SDM845 > > SDM845 SoC-based systems. > > Say Y if you want to use audio device on this SoCs. > > > > +config SND_SOC_SC7180 > > + tristate "SoC Machine driver for SC7180 boards" > > + depends on I2C > > + select SND_SOC_QCOM_COMMON > > + select SND_SOC_LPASS_SC7180 > > + select SND_SOC_MAX98357A > > + select SND_SOC_RT5682_I2C > > + help > > + To add support for audio on Qualcomm Technologies Inc. > > + SC7180 SoC-based systems. > > + Say Y if you want to use audio device on this SoCs. > > + > > endif #SND_SOC_QCOM > > diff --git a/sound/soc/qcom/Makefile b/sound/soc/qcom/Makefile > > index 7972c9479ab0..0cdcbf367ef1 100644 > > --- a/sound/soc/qcom/Makefile > > +++ b/sound/soc/qcom/Makefile > > @@ -17,12 +17,14 @@ snd-soc-storm-objs := storm.o > > snd-soc-apq8016-sbc-objs := apq8016_sbc.o > > snd-soc-apq8096-objs := apq8096.o > > snd-soc-sdm845-objs := sdm845.o > > +snd-soc-sc7180-objs := sc7180.o > > snd-soc-qcom-common-objs := common.o > > > > obj-$(CONFIG_SND_SOC_STORM) += snd-soc-storm.o > > obj-$(CONFIG_SND_SOC_APQ8016_SBC) += snd-soc-apq8016-sbc.o > > obj-$(CONFIG_SND_SOC_MSM8996) += snd-soc-apq8096.o > > obj-$(CONFIG_SND_SOC_SDM845) += snd-soc-sdm845.o > > +obj-$(CONFIG_SND_SOC_SC7180) += snd-soc-sc7180.o > > obj-$(CONFIG_SND_SOC_QCOM_COMMON) += snd-soc-qcom-common.o > > > > #DSP lib > > diff --git a/sound/soc/qcom/sc7180.c b/sound/soc/qcom/sc7180.c > > new file mode 100644 > > index 000000000000..40bc4fc98842 > > --- /dev/null > > +++ b/sound/soc/qcom/sc7180.c > > @@ -0,0 +1,267 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +// > > +// Copyright (c) 2020, The Linux Foundation. All rights reserved. > > +// > > +// sc7180.c -- ALSA SoC Machine driver for SC7180 > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +#include "../codecs/rt5682.h" > > +#include "common.h" > > +#include "lpass.h" > > + > > +#define DEFAULT_SAMPLE_RATE_48K 48000 > > +#define DEFAULT_MCLK_RATE 19200000 > > +#define RT5682_PLL1_FREQ (48000 * 512) > > + > > +struct sc7180_snd_data { > > + u32 pri_mi2s_clk_count; > > + struct snd_soc_jack hs_jack; > > + struct snd_soc_jack hdmi_jack; > > +}; > > [...] > > > + > > +static const struct snd_soc_ops sc7180_ops = { > > + .startup = sc7180_snd_startup, > > + .shutdown = sc7180_snd_shutdown, > > +}; > > + > > +static const struct snd_soc_dapm_widget sc7180_snd_widgets[] = { > > + SND_SOC_DAPM_HP("Headphone Jack", NULL), > > + SND_SOC_DAPM_MIC("Headset Mic", NULL), > > +}; > > + > > +static struct snd_soc_card sc7180_card = { > > + .owner = THIS_MODULE, > > + .dapm_widgets = sc7180_snd_widgets, > > + .num_dapm_widgets = ARRAY_SIZE(sc7180_snd_widgets), > > +}; > > Given that you modify this struct and already allocate some memory > dynamically (sc7810_snd_data), it might be a bit cleaner to avoid > modifying global memory and instead allocate snd_soc_card dynamically as > well. Could just add it to sc7180_snd_data for example (see e.g. apq8016_sbc) > Good idea. I will modify it in v10. > > + > > +static void sc7180_add_ops(struct snd_soc_card *card) > > +{ > > + struct snd_soc_dai_link *link; > > + int i; > > + > > + for_each_card_prelinks(card, i, link) { > > + link->ops = &sc7180_ops; > > + link->init = sc7180_init; > > + } > > +} > > + > > +static int sc7180_snd_platform_probe(struct platform_device *pdev) > > +{ > > + struct snd_soc_card *card = &sc7180_card; > > + struct sc7180_snd_data *data; > > + struct device *dev = &pdev->dev; > > + int ret; > > + > > + /* Allocate the private data */ > > + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); > > + if (!data) > > + return -ENOMEM; > > + > > + card->dev = dev; > > + snd_soc_card_set_drvdata(card, data); > > + > > + ret = qcom_snd_parse_of(card); > > + if (ret) { > > + dev_err(dev, "Error parsing OF data\n"); > > This will just add noise in case of probe deferral. qcom_snd_parse_of() > already logs a message for most errors so you can just remove this one. > I will remove it in v10. > > + return ret; > > + } > > + > > + sc7180_add_ops(card); > > + > > + return devm_snd_soc_register_card(dev, card); > > +} > > + > > +static const struct of_device_id sc7180_snd_device_id[] = { > > + { .compatible = "qcom,sc7180-sndcard" }, > > Will all SC7180 use the configuration in this driver? (With RT5682, > HDMI, the jack configuration etc). Otherwise a more specific compatible > string might be better, so other device-specific ones can be added later. > I thought we can do that in the next patch when we want to introduce different board variations. But maybe doing it earlier is better. There will be three variations on trogdor: rt5682 (1mic), max98357, dp rt5682 (2mic), max98357, dp adau7002, max98357, dp I would like to name them qcom,sc7180-sndcard-rt5682-m98357-1mic qcom,sc7180-sndcard-rt5682-m98357-2mic qcom,sc7180-sndcard-ad7002-m98357 What do you think ? Thanks for the review and suggestions! > Thanks! > Stephan