Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754861AbbKXULA (ORCPT ); Tue, 24 Nov 2015 15:11:00 -0500 Received: from mail-ig0-f177.google.com ([209.85.213.177]:33786 "EHLO mail-ig0-f177.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754516AbbKXUKz convert rfc822-to-8bit (ORCPT ); Tue, 24 Nov 2015 15:10:55 -0500 MIME-Version: 1.0 In-Reply-To: References: <1448014157-36526-1-git-send-email-raphael.poggi@avalun.com> <1448014157-36526-5-git-send-email-raphael.poggi@avalun.com> Date: Tue, 24 Nov 2015 21:10:54 +0100 Message-ID: Subject: Re: [alsa-devel] [PATCH 4/4] sound: codecs: pcm1792a: introduce channel side From: Michael Trimarchi To: =?UTF-8?Q?Rapha=C3=ABl_Poggi?= Cc: perex@perex.cz, tiwai@suse.com, "devicetree@vger.kernel.org" , "alsa-devel@alsa-project.org" , LKML Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 8086 Lines: 211 Hi On Tue, Nov 24, 2015 at 5:04 PM, Raphaël Poggi wrote: > Hi, > > 2015-11-24 9:27 GMT+01:00 Michael Trimarchi : >> Hi >> >> On Tue, Nov 24, 2015 at 9:21 AM, Raphaël Poggi wrote: >>> Hi, >>> >>> 2015-11-21 10:37 GMT+01:00 Michael Trimarchi : >>>> Hi >>>> >>>> On Fri, Nov 20, 2015 at 11:09 AM, Raphael Poggi wrote: >>>>> From: Raphael Poggi >>>>> >>>>> Add possibility to choose the channel side using the device tree, >>>>> and also modify it using alsa ctrl. >>>>> >>>>> Signed-off-by: Raphael Poggi >>>>> --- >>>>> sound/soc/codecs/pcm1792a.c | 46 +++++++++++++++++++++++++++++++++++++++++++++ >>>>> 1 file changed, 46 insertions(+) >>>>> >>>>> diff --git a/sound/soc/codecs/pcm1792a.c b/sound/soc/codecs/pcm1792a.c >>>>> index febaa48..005b679 100644 >>>>> --- a/sound/soc/codecs/pcm1792a.c >>>>> +++ b/sound/soc/codecs/pcm1792a.c >>>>> @@ -65,6 +65,12 @@ static const struct reg_default pcm1792a_reg_defaults[] = { >>>>> { PCM1792A_DEVICE_ID, 0x00 }, >>>>> }; >>>>> >>>>> +enum __pcm1792_side { >>>>> + STEREO, >>>>> + MONORAL_LEFT, >>>>> + MONORAL_RIGHT >>>>> +}; >>>>> + >>>>> static bool pcm1792a_accessible_reg(struct device *dev, unsigned int reg) >>>>> { >>>>> return reg >= 0x10 && reg <= 0x17; >>>>> @@ -83,6 +89,7 @@ struct pcm1792a_private { >>>>> struct regmap *regmap; >>>>> unsigned int format; >>>>> unsigned int rate; >>>>> + unsigned int side; >>>>> }; >>>>> >>>>> static int pcm1792a_set_dai_fmt(struct snd_soc_dai *codec_dai, >>>>> @@ -153,6 +160,13 @@ static int pcm1792a_hw_params(struct snd_pcm_substream *substream, >>>>> ret = regmap_update_bits(priv->regmap, PCM1792A_FMT_CONTROL, >>>>> PCM1792A_FMT_MASK | PCM1792A_ATLD_ENABLE, val); >>>>> >>>>> + if (priv->side == MONORAL_LEFT) >>>>> + val = PCM1792A_CHANNEL_MONO_LEFT; >>>>> + else if (priv->side == MONORAL_RIGHT) >>>>> + val = PCM1792A_CHANNEL_MONO_RIGHT; >>>>> + >>>>> + regmap_update_bits(priv->regmap, PCM1792A_CHANNEL, PCM1792A_CHANNEL_MONO_MASK, val); >>>>> + >>>>> return ret; >>>>> } >>>>> >>>>> @@ -222,10 +236,31 @@ static struct snd_soc_codec_driver soc_codec_dev_pcm1792a = { >>>>> .num_dapm_routes = ARRAY_SIZE(pcm1792a_dapm_routes), >>>>> }; >>>>> >>>>> +static int pcm1792a_of_init(struct spi_device *spi) >>>>> +{ >>>>> + int ret = 0; >>>>> + struct pcm1792a_private *pcm1792a = dev_get_drvdata(&spi->dev); >>>>> + const char *name; >>>>> + struct device_node *np = spi->dev.of_node; >>>>> + >>>>> + name = of_get_property(np, "ti,side", NULL); >>>>> + if (name) { >>>>> + if (!strcmp(name, "left")) >>>>> + pcm1792a->side = MONORAL_LEFT; >>>>> + else >>>>> + pcm1792a->side = MONORAL_RIGHT; >>>>> + } >>>>> + else >>>>> + pcm1792a->side = STEREO; >>>>> + >>>>> + return ret; >>>>> +} >>>>> + >>>>> static int pcm1792a_spi_probe(struct spi_device *spi) >>>>> { >>>>> struct pcm1792a_private *pcm1792a; >>>>> int ret; >>>>> + struct device_node *np = spi->dev.of_node; >>>>> >>>>> pcm1792a = devm_kzalloc(&spi->dev, sizeof(struct pcm1792a_private), >>>>> GFP_KERNEL); >>>>> @@ -241,6 +276,17 @@ static int pcm1792a_spi_probe(struct spi_device *spi) >>>>> return ret; >>>>> } >>>>> >>>>> + if (np) { >>>>> + ret = pcm1792a_of_init(spi); >>>>> + if (ret) >>>>> + return ret; >>>>> + } >>>>> + >>>>> + if (pcm1792a->side == MONORAL_LEFT) >>>>> + regmap_update_bits(pcm1792a->regmap, PCM1792A_CHANNEL, PCM1792A_CHANNEL_MONO_MASK, PCM1792A_CHANNEL_MONO_LEFT); >>>>> + else if (pcm1792a->side == MONORAL_RIGHT) >>>>> + regmap_update_bits(pcm1792a->regmap, PCM1792A_CHANNEL, PCM1792A_CHANNEL_MONO_MASK, PCM1792A_CHANNEL_MONO_RIGHT); >>>>> + >>>> >>>> Why this is a part of dts and not a part of mixer control? >>>> >>>> Michael >>>> >>> >>> The "channel side" bindings should be used when the pcm7192a is used >>> in monaural mode (which requires two DACs, both DACs operate in a >>> balanced mode for one channel of audio input data). >>> This is a hardware level design, it will never change at runtime. >>> I feel that a device tree binding is more appropriate than a control. >>> If you put this in mixer control then you can avoid to have it in dts because you not suppose to boot playing music. Basically you can get the same result restore some alsa config that match your topology. Even if you have two mono amplifier you can have the situation that you want to invert channel left and right and this can be done easily just reconfigure one as mono left and the other as mono right or write your own sound card that do in one control, change both together. I suggest to wait some other opinions. Michael >> >> Well I know exactly why is used for, but I don't really know if it's >> somenthing that should be >> in dts. Do I want to do channel inversion? >> >> Michael >> > > I am not sure to understand. You mean "What if I want to do channel > inversion", right ? > If so, we have two differents "problems", one at hardware level design > because monaural mode requires two DACs, and an another one at > software level > "channel inversion". > I have always thought that hardware description should be in dts, > that's why I implement monaural mode using dts. > > Honestly, I don't know what is the best solution between "this is a > hardware design, it should be in dts and shouldn't change" vs "this > should be an alsa controler because we want extra features independent > from hardware design" > > Raphaël > >>>>> return snd_soc_register_codec(&spi->dev, >>>>> &soc_codec_dev_pcm1792a, &pcm1792a_dai, 1); >>>>> } >>>>> -- >>>>> 2.1.0 >>>>> >>>>> _______________________________________________ >>>>> Alsa-devel mailing list >>>>> Alsa-devel@alsa-project.org >>>>> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel >>>> >>>> >>>> >>>> -- >>>> | Michael Nazzareno Trimarchi Amarula Solutions BV | >>>> | COO - Founder Cruquiuskade 47 | >>>> | +31(0)851119172 Amsterdam 1018 AM NL | >>>> | [`as] http://www.amarulasolutions.com | >>>> -- >>>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in >>>> the body of a message to majordomo@vger.kernel.org >>>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>>> Please read the FAQ at http://www.tux.org/lkml/ >> >> >> >> -- >> | Michael Nazzareno Trimarchi Amarula Solutions BV | >> | COO - Founder Cruquiuskade 47 | >> | +31(0)851119172 Amsterdam 1018 AM NL | >> | [`as] http://www.amarulasolutions.com | >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> Please read the FAQ at http://www.tux.org/lkml/ -- | Michael Nazzareno Trimarchi Amarula Solutions BV | | COO - Founder Cruquiuskade 47 | | +31(0)851119172 Amsterdam 1018 AM NL | | [`as] http://www.amarulasolutions.com | -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/