-----Original Message----- From: Mark Brown [mailto:broonie@kernel.org] Sent: 14 October 2022 05:53 PM To: Padmanabhan Rajanbabu p.rajanbabu@samsung.com Cc: lgirdwood@gmail.com; robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org; s.nawrocki@samsung.com; perex@perex.cz; tiwai@suse.com; pankaj.dubey@samsung.com; alim.akhtar@samsung.com; rcsekar@samsung.com; aswani.reddy@samsung.com; alsa-devel@alsa-project.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-samsung- soc@vger.kernel.org Subject: Re: [PATCH 4/6] ASoC: samsung: fsd: Add FSD soundcard driver
On Fri, Oct 14, 2022 at 03:51:49PM +0530, Padmanabhan Rajanbabu wrote:
+++ b/sound/soc/samsung/fsd-card.c @@ -0,0 +1,349 @@ +// SPDX-License-Identifier: GPL-2.0 +/*
- ALSA SoC Audio Layer - FSD Soundcard driver
- Copyright (c) 2022 Samsung Electronics Co. Ltd.
- Padmanabhan Rajanbabu p.rajanbabu@samsung.com
Please make the entire comment a C++ one so things look more intentional.
okay
- if (link->dai_fmt & SND_SOC_DAIFMT_CBC_CFC) {
cdclk_dir = SND_SOC_CLOCK_OUT;- } else if (link->dai_fmt & SND_SOC_DAIFMT_CBP_CFP) {
cdclk_dir = SND_SOC_CLOCK_IN;- } else {
dev_err(card->dev, "Missing Clock Master information\n");goto err;- }
We're trying to modernise the langauge around clock providers, please use that term rather than the outdated terminology here.
Okay, I'll make the necessary change for the clock terminologies in the next patch set
- if (priv->tdm_slots) {
ret = snd_soc_dai_set_tdm_slot(cpu_dai, false, false,priv->tdm_slots, priv->tdm_slot_width);if (ret < 0) {dev_err(card->dev,"Failed to configure in TDM mode:%d\n",ret);
goto err;}- }
Just set things once on probe if they don't depend on the configuration,
it's
neater and marginally faster if nothing else.
Okay
- if (of_property_read_bool(dev->of_node, "widgets")) {
ret = snd_soc_of_parse_audio_simple_widgets(card,"widgets");
if (ret)return ERR_PTR(ret);- }
- /* Add DAPM routes to the card */
- if (of_property_read_bool(node, "audio-routing")) {
ret = snd_soc_of_parse_audio_routing(card, "audio-routing");
if (ret)return ERR_PTR(ret);- }
Just fix the library functions to handle missing properties gracefully,
every
card is going to need the same code here.
Okay
Thank you for reviewing the patch