From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lars-Peter Clausen Subject: Re: [alsa-devel] [PATCH 2/5 v4] drm/i2c/adv7511: Add audio support Date: Mon, 11 Apr 2016 11:33:47 +0200 Message-ID: <570B6F7B.2020906@metafoo.de> References: <5707D249.5040707@metafoo.de> <5707D88A.3010400@synopsys.com> <57091999.4010906@metafoo.de> <570B6DF2.1080008@synopsys.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <570B6DF2.1080008@synopsys.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Jose Abreu , linux-snps-arc@lists.infradead.org, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, alsa-devel@alsa-project.org Cc: Vineet.Gupta1@synopsys.com, Alexey.Brodkin@synopsys.com, CARLOS.PALMINHA@synopsys.com, lgirdwood@gmail.com, broonie@kernel.org, tiwai@suse.com List-Id: alsa-devel@alsa-project.org T24gMDQvMTEvMjAxNiAxMToyNyBBTSwgSm9zZSBBYnJldSB3cm90ZToKPiBIaSBMYXJzLAo+IAo+ IAo+IE9uIDA5LTA0LTIwMTYgMTY6MDIsIExhcnMtUGV0ZXIgQ2xhdXNlbiB3cm90ZToKPj4gT24g MDQvMDgvMjAxNiAwNjoxMiBQTSwgSm9zZSBBYnJldSB3cm90ZToKPj4gWy4uLl0KPj4+PiBbLi4u XQo+Pj4+PiArLSBhZGksZW5hYmxlLWF1ZGlvOiBJZiBzZXQgdGhlIEFEVjc1MTEgZHJpdmVyIHdp bGwgcmVnaXN0ZXIgYSBjb2RlYyBpbnRlcmZhY2UKPj4+Pj4gKyAgaW50byBBTFNBIFNvQy4KPj4+ PiBUaGlzIGlzIG5vdCBhIGRlc2NyaXB0aW9uIG9mIHRoZSBoYXJkd2FyZS4KPj4+IElzIHRoaXMg b2theTogImFkaSxlbmFibGUtYXVkaW86IFNldCB0aGlzIGJvb2xlYW4gcGFyYW1ldGVyIGlmIEFE Vjc1MTEKPj4+IHRyYW5zbWl0dGVyIHJvdXRlcyBhdWRpbyBzaWduYWxzIiA/Cj4+IEkgZG9uJ3Qg dGhpbmsgd2UgbmVlZCB0aGlzIHByb3BlcnR5LiBUaGVyZSBpcyBubyBwcm9ibGVtIHdpdGggcmVn aXN0ZXJpbmcKPj4gdGhlIGF1ZGlvIHBhcnQgdW5jb25kaXRpb25hbGx5LiBBcyBsb25nIGFzIHRo ZXJlIGlzIG5vIGNvbm5lY3Rpb24gd2Ugd29udAo+PiBjcmVhdGUgYSBzb3VuZCBjYXJkIHRoYXQg aXMgZXhwb3NlZCB0byB1c2Vyc3BhY2UuCj4+Cj4gCj4gVGhpcyBjaGFuZ2Ugd2FzIHN1Z2dlc3Rl ZCBieSBMYXVyZW50IFBpbmNoYXJ0IGFuZCB3YXMgaW50cm9kdWNlZCBpbiB2My4gUXVvdGluZwo+ IExhdXJlbnQ6Cj4gIlRoZSBpZGVhIGlzIHRoYXQgZW5hYmxpbmcgc3VwcG9ydCBmb3IgQURWNzUx MSBhdWRpbyBpbiB0aGUga2VybmVsIGlzbid0IGNvdXBsZWQKPiB3aXRoIHdoZXRoZXIgdGhlIHN5 c3RlbSBpbmNsdWRlcyBhdWRpbyBzdXBwb3J0LiBJdCB3b3VsZCBiZSBjb25mdXNpbmcsIGFuZCB3 b3VsZAo+IGFsc28gd2FzdGUgcmVzb3VyY2VzLCB0byBjcmVhdGUgYSBMaW51eCBzb3VuZCBkZXZp Y2Ugd2hlbiBubyBzb3VuZCBjaGFubmVsIGlzCj4gcm91dGVkIG9uIHRoZSBib2FyZC4iCgpJIHdv dWxkbid0IGNhcmUgdG9vIG11Y2ggYWJvdXQgdGhpcyBhdCB0aGlzIHBvaW50LCB0aGUgZXh0cmEg YW1vdW50IG9mCnJlc291cmNlcyByZXF1aXJlZCBmb3IgcmVnaXN0ZXJpbmcgdGhlIENPREVDIChi dXQgbm90IHRoZSBzb3VuZCBjYXJkKSBpcwpqdXN0IGEgZmV3IGJ5dGVzIChzaXplb2Yoc3RydWN0 IHNuZF9zb2NfY29kZWMpKS4KCk5ldmVydGhlbGVzcyB3aGF0IHdlIHNob3VsZCBkbyBpcyBkZXNj cmliZSB0aGUgaGFyZHdhcmUgYW5kIGZyb20gdGhpcwppbmZvcm1hdGlvbiBpbmZlciB3aGV0aGVy IHRoZXJlIGlzIGEgYXVkaW8gY29ubmVjdGlvbiBvciBub3QgYW5kIGlmIHRoZXJlIGlzCm5vbmUg d2UgbWlnaHQgc2tpcCByZWdpc3RlcmluZyB0aGUgQ09ERUMuIEluIG15IG9waW5pb24gdGhpcyBo YXJkd2FyZQpkZXNjcmlwdGlvbiBzaG91bGQgYmUgbW9kZWxlZCB1c2luZyBvZi1ncmFwaCwgaGF2 aW5nIGEgY29ubmVjdGlvbiBiZXR3ZWVuCnRoZSBTb0Mgc2lkZSBhbmQgdGhlIGFkdjc1MTEgU1BE SUYgb3IgSTJTIHBvcnQuCgpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fXwpkcmktZGV2ZWwgbWFpbGluZyBsaXN0CmRyaS1kZXZlbEBsaXN0cy5mcmVlZGVza3Rv cC5vcmcKaHR0cHM6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmkt ZGV2ZWwK From mboxrd@z Thu Jan 1 00:00:00 1970 From: lars@metafoo.de (Lars-Peter Clausen) Date: Mon, 11 Apr 2016 11:33:47 +0200 Subject: [alsa-devel] [PATCH 2/5 v4] drm/i2c/adv7511: Add audio support In-Reply-To: <570B6DF2.1080008@synopsys.com> References: <5707D249.5040707@metafoo.de> <5707D88A.3010400@synopsys.com> <57091999.4010906@metafoo.de> <570B6DF2.1080008@synopsys.com> List-ID: Message-ID: <570B6F7B.2020906@metafoo.de> To: linux-snps-arc@lists.infradead.org On 04/11/2016 11:27 AM, Jose Abreu wrote: > Hi Lars, > > > On 09-04-2016 16:02, Lars-Peter Clausen wrote: >> On 04/08/2016 06:12 PM, Jose Abreu wrote: >> [...] >>>> [...] >>>>> +- adi,enable-audio: If set the ADV7511 driver will register a codec interface >>>>> + into ALSA SoC. >>>> This is not a description of the hardware. >>> Is this okay: "adi,enable-audio: Set this boolean parameter if ADV7511 >>> transmitter routes audio signals" ? >> I don't think we need this property. There is no problem with registering >> the audio part unconditionally. As long as there is no connection we wont >> create a sound card that is exposed to userspace. >> > > This change was suggested by Laurent Pinchart and was introduced in v3. Quoting > Laurent: > "The idea is that enabling support for ADV7511 audio in the kernel isn't coupled > with whether the system includes audio support. It would be confusing, and would > also waste resources, to create a Linux sound device when no sound channel is > routed on the board." I wouldn't care too much about this at this point, the extra amount of resources required for registering the CODEC (but not the sound card) is just a few bytes (sizeof(struct snd_soc_codec)). Nevertheless what we should do is describe the hardware and from this information infer whether there is a audio connection or not and if there is none we might skip registering the CODEC. In my opinion this hardware description should be modeled using of-graph, having a connection between the SoC side and the adv7511 SPDIF or I2S port. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754197AbcDKJdz (ORCPT ); Mon, 11 Apr 2016 05:33:55 -0400 Received: from www381.your-server.de ([78.46.137.84]:43524 "EHLO www381.your-server.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753912AbcDKJdx (ORCPT ); Mon, 11 Apr 2016 05:33:53 -0400 Subject: Re: [alsa-devel] [PATCH 2/5 v4] drm/i2c/adv7511: Add audio support To: Jose Abreu , linux-snps-arc@lists.infradead.org, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, alsa-devel@alsa-project.org References: <5707D249.5040707@metafoo.de> <5707D88A.3010400@synopsys.com> <57091999.4010906@metafoo.de> <570B6DF2.1080008@synopsys.com> Cc: architt@codeaurora.org, airlied@linux.ie, Vineet.Gupta1@synopsys.com, Alexey.Brodkin@synopsys.com, lgirdwood@gmail.com, CARLOS.PALMINHA@synopsys.com, broonie@kernel.org, tiwai@suse.com From: Lars-Peter Clausen Message-ID: <570B6F7B.2020906@metafoo.de> Date: Mon, 11 Apr 2016 11:33:47 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Icedove/38.6.0 MIME-Version: 1.0 In-Reply-To: <570B6DF2.1080008@synopsys.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-Authenticated-Sender: lars@metafoo.de Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 04/11/2016 11:27 AM, Jose Abreu wrote: > Hi Lars, > > > On 09-04-2016 16:02, Lars-Peter Clausen wrote: >> On 04/08/2016 06:12 PM, Jose Abreu wrote: >> [...] >>>> [...] >>>>> +- adi,enable-audio: If set the ADV7511 driver will register a codec interface >>>>> + into ALSA SoC. >>>> This is not a description of the hardware. >>> Is this okay: "adi,enable-audio: Set this boolean parameter if ADV7511 >>> transmitter routes audio signals" ? >> I don't think we need this property. There is no problem with registering >> the audio part unconditionally. As long as there is no connection we wont >> create a sound card that is exposed to userspace. >> > > This change was suggested by Laurent Pinchart and was introduced in v3. Quoting > Laurent: > "The idea is that enabling support for ADV7511 audio in the kernel isn't coupled > with whether the system includes audio support. It would be confusing, and would > also waste resources, to create a Linux sound device when no sound channel is > routed on the board." I wouldn't care too much about this at this point, the extra amount of resources required for registering the CODEC (but not the sound card) is just a few bytes (sizeof(struct snd_soc_codec)). Nevertheless what we should do is describe the hardware and from this information infer whether there is a audio connection or not and if there is none we might skip registering the CODEC. In my opinion this hardware description should be modeled using of-graph, having a connection between the SoC side and the adv7511 SPDIF or I2S port.