From: "Padmanabhan Rajanbabu" <p.rajanbabu@samsung.com> To: "'Mark Brown'" <broonie@kernel.org> 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 Date: Fri, 21 Oct 2022 13:39:04 +0530 [thread overview] Message-ID: <04a801d8e524$65416ec0$2fc44c40$@samsung.com> (raw) In-Reply-To: <Y0lUuK0V8qVnr+LW@sirena.org.uk> > -----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
WARNING: multiple messages have this Message-ID (diff)
From: "Padmanabhan Rajanbabu" <p.rajanbabu@samsung.com> To: "'Mark Brown'" <broonie@kernel.org> Cc: devicetree@vger.kernel.org, alsa-devel@alsa-project.org, linux-samsung-soc@vger.kernel.org, krzysztof.kozlowski+dt@linaro.org, rcsekar@samsung.com, aswani.reddy@samsung.com, pankaj.dubey@samsung.com, tiwai@suse.com, lgirdwood@gmail.com, robh+dt@kernel.org, alim.akhtar@samsung.com, linux-kernel@vger.kernel.org Subject: RE: [PATCH 4/6] ASoC: samsung: fsd: Add FSD soundcard driver Date: Fri, 21 Oct 2022 13:39:04 +0530 [thread overview] Message-ID: <04a801d8e524$65416ec0$2fc44c40$@samsung.com> (raw) In-Reply-To: <Y0lUuK0V8qVnr+LW@sirena.org.uk> > -----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
next prev parent reply other threads:[~2022-10-21 8:20 UTC|newest] Thread overview: 66+ messages / expand[flat|nested] mbox.gz Atom feed top [not found] <CGME20221014104843epcas5p47f6daaad2e67e0c9eedd68c2256c025b@epcas5p4.samsung.com> 2022-10-14 10:21 ` [PATCH 0/6] ASoC: samsung: fsd: audio support for FSD SoC Padmanabhan Rajanbabu 2022-10-14 10:21 ` Padmanabhan Rajanbabu [not found] ` <CGME20221014104850epcas5p1a707b9d407a0947c3519077cf7fca5ff@epcas5p1.samsung.com> 2022-10-14 10:21 ` [PATCH 1/6] ASoC: samsung: i2s: TDM Support for CPU DAI driver Padmanabhan Rajanbabu 2022-10-14 10:21 ` Padmanabhan Rajanbabu [not found] ` <CGME20221014104857epcas5p2a275a1d606ca066227228d13bcf5b120@epcas5p2.samsung.com> 2022-10-14 10:21 ` [PATCH 2/6] ASoC: samsung: i2s: configure PSR from sound card Padmanabhan Rajanbabu 2022-10-14 10:21 ` Padmanabhan Rajanbabu 2022-10-14 12:02 ` Mark Brown 2022-10-14 12:02 ` Mark Brown 2022-10-21 8:00 ` Padmanabhan Rajanbabu 2022-10-21 8:00 ` Padmanabhan Rajanbabu 2022-10-21 11:53 ` Mark Brown 2022-10-21 11:53 ` Mark Brown 2022-11-08 5:23 ` Padmanabhan Rajanbabu 2022-11-08 5:23 ` Padmanabhan Rajanbabu 2022-11-09 17:38 ` Mark Brown 2022-11-09 17:38 ` Mark Brown 2023-01-03 4:55 ` Padmanabhan Rajanbabu 2023-01-03 4:55 ` Padmanabhan Rajanbabu 2022-10-23 13:12 ` Krzysztof Kozlowski 2022-10-23 13:12 ` Krzysztof Kozlowski [not found] ` <CGME20221014104901epcas5p1a61ea81c3b1640bd8a064633c0b1e40d@epcas5p1.samsung.com> 2022-10-14 10:21 ` [PATCH 3/6] dt-bindings: sound: Add sound card bindings for Tesla FSD Padmanabhan Rajanbabu 2022-10-14 10:21 ` Padmanabhan Rajanbabu 2022-10-14 15:13 ` Rob Herring 2022-10-14 15:13 ` Rob Herring 2022-10-21 8:44 ` Padmanabhan Rajanbabu 2022-10-21 8:44 ` Padmanabhan Rajanbabu 2022-10-22 16:48 ` Krzysztof Kozlowski 2022-10-22 16:48 ` Krzysztof Kozlowski 2022-11-08 5:33 ` Padmanabhan Rajanbabu 2022-11-08 5:33 ` Padmanabhan Rajanbabu 2022-11-08 10:36 ` Krzysztof Kozlowski 2022-11-08 10:36 ` Krzysztof Kozlowski [not found] ` <CGME20221014104904epcas5p4f458182cc9ac9c223d9a25566f3dd300@epcas5p4.samsung.com> 2022-10-14 10:21 ` [PATCH 4/6] ASoC: samsung: fsd: Add FSD soundcard driver Padmanabhan Rajanbabu 2022-10-14 10:21 ` Padmanabhan Rajanbabu 2022-10-14 12:23 ` Mark Brown 2022-10-14 12:23 ` Mark Brown 2022-10-21 8:09 ` Padmanabhan Rajanbabu [this message] 2022-10-21 8:09 ` Padmanabhan Rajanbabu 2022-10-16 15:18 ` Krzysztof Kozlowski 2022-10-16 15:18 ` Krzysztof Kozlowski 2022-10-21 9:04 ` Padmanabhan Rajanbabu 2022-10-21 9:04 ` Padmanabhan Rajanbabu [not found] ` <CGME20221014104911epcas5p394100ff6ed53be32c4d64c7e23e48833@epcas5p3.samsung.com> 2022-10-14 10:21 ` [PATCH 5/6] arm64: dts: fsd: Add I2S DAI node for Tesla FSD Padmanabhan Rajanbabu 2022-10-14 10:21 ` Padmanabhan Rajanbabu 2022-10-14 13:24 ` Alim Akhtar 2022-10-14 13:24 ` Alim Akhtar 2022-10-21 8:09 ` Padmanabhan Rajanbabu 2022-10-21 8:09 ` Padmanabhan Rajanbabu 2022-10-16 15:14 ` Krzysztof Kozlowski 2022-10-16 15:14 ` Krzysztof Kozlowski 2022-10-21 8:49 ` Padmanabhan Rajanbabu 2022-10-21 8:49 ` Padmanabhan Rajanbabu 2022-10-21 13:01 ` Krzysztof Kozlowski 2022-10-21 13:01 ` Krzysztof Kozlowski 2022-11-08 5:26 ` Padmanabhan Rajanbabu 2022-11-08 5:26 ` Padmanabhan Rajanbabu [not found] ` <CGME20221014104915epcas5p12414b87ea127b2d5bf521556bf841b00@epcas5p1.samsung.com> 2022-10-14 10:21 ` [PATCH 6/6] arm64: dts: fsd: Add sound card " Padmanabhan Rajanbabu 2022-10-14 10:21 ` Padmanabhan Rajanbabu 2022-10-14 13:29 ` Alim Akhtar 2022-10-14 13:29 ` Alim Akhtar 2022-10-21 8:12 ` Padmanabhan Rajanbabu 2022-10-21 8:12 ` Padmanabhan Rajanbabu 2022-10-21 12:53 ` Krzysztof Kozlowski 2022-10-21 12:53 ` Krzysztof Kozlowski 2022-11-08 5:25 ` Padmanabhan Rajanbabu 2022-11-08 5:25 ` Padmanabhan Rajanbabu
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='04a801d8e524$65416ec0$2fc44c40$@samsung.com' \ --to=p.rajanbabu@samsung.com \ --cc=alim.akhtar@samsung.com \ --cc=alsa-devel@alsa-project.org \ --cc=aswani.reddy@samsung.com \ --cc=broonie@kernel.org \ --cc=devicetree@vger.kernel.org \ --cc=krzysztof.kozlowski+dt@linaro.org \ --cc=lgirdwood@gmail.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-samsung-soc@vger.kernel.org \ --cc=pankaj.dubey@samsung.com \ --cc=perex@perex.cz \ --cc=rcsekar@samsung.com \ --cc=robh+dt@kernel.org \ --cc=s.nawrocki@samsung.com \ --cc=tiwai@suse.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.