From mboxrd@z Thu Jan 1 00:00:00 1970 From: Laurent Pinchart Subject: Re: [PATCH 1/3 v2] drm/i2c/adv7511: Add audio support Date: Tue, 05 Apr 2016 00:41:27 +0300 Message-ID: <2037371.nWufVmEmFd@avalon> References: <5276355.2bQYPXCpz6@avalon> <57022E63.2010900@synopsys.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <57022E63.2010900@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 Cc: tixy@linaro.org, mark.rutland@arm.com, alsa-devel@alsa-project.org, Alexey.Brodkin@synopsys.com, dri-devel@lists.freedesktop.org, lgirdwood@gmail.com, yitian.bu@tangramtek.com, wsa+renesas@sang-engineering.com, laurent.pinchart+renesas@ideasonboard.com, tiwai@suse.com, nariman@opensource.wolfsonmicro.com, linux-snps-arc@lists.infradead.org, devicetree@vger.kernel.org, Maruthi.Bayyavarapu@amd.com, pawel.moll@arm.com, ijc+devicetree@hellion.org.uk, Vineet.Gupta1@synopsys.com, buyitian@gmail.com, robh+dt@kernel.org, perex@perex.cz, linux-kernel@vger.kernel.org, broonie@kernel.org, galak@codeaurora.org, alexander.deucher@amd.com, CARLOS.PALMINHA@synopsys.com List-Id: alsa-devel@alsa-project.org SGkgSm9zZSwKCk9uIE1vbmRheSAwNCBBcHIgMjAxNiAxMDowNTozOSBKb3NlIEFicmV1IHdyb3Rl Ogo+IE9uIDAxLTA0LTIwMTYgMTg6MTAsIExhdXJlbnQgUGluY2hhcnQgd3JvdGU6Cj4gPiBPbiBN b25kYXkgMjggTWFyIDIwMTYgMTU6MzY6MDkgSm9zZSBBYnJldSB3cm90ZToKPiA+PiBUaGlzIHBh dGNoIGFkZHMgYXVkaW8gc3VwcG9ydCBmb3IgdGhlIEFEVjc1MTEgSERNSSB0cmFuc21pdHRlcgo+ ID4+IHVzaW5nIEFMU0EgU29DLgo+ID4+IAo+ID4+IFRoZSBjb2RlIHdhcyBwb3J0ZWQgZnJvbSBB bmFsb2cgRGV2aWNlcyBsaW51eCB0cmVlIGZyb20KPiA+PiBjb21taXQgMTc3MGM0YTFlMzJiICgi TWVyZ2UgcmVtb3RlLXRyYWNraW5nIGJyYW5jaAo+ID4+IAo+ID4+ICd4aWxpbngvbWFzdGVyJyBp bnRvIHhjb21tX3p5bnEiKSwgd2hpY2ggaXMgYXZhaWxhYmxlIGF0Ogo+ID4+IAktIGh0dHBzOi8v Z2l0aHViLmNvbS9hbmFsb2dkZXZpY2VzaW5jL2xpbnV4Lwo+ID4+IAo+ID4+IFRoZSBtYWluIGNv cmUgZmlsZSB3YXMgcmVuYW1lZCBmcm9tIGFkdjc1MTEuYyB0byBhZHY3NTExX2NvcmUuYwo+ID4+ IHNvIHRoYXQgYXVkaW8gYW5kIHZpZGVvIGNvbXBpbGUgaW50byBhIHNpbmdsZSBhZHY3NTExLmtv IG1vZHVsZQo+ID4+IGFuZCB0byBrZWVwIHVwIHdpdGggQW5hbG9nIERldmljZXMga2VybmVsIHRy ZWUuCj4gPj4gCj4gPj4gVGhlIGF1ZGlvIGNhbiBiZSBkaXNhYmxlZCB1c2luZyBtZW51LWNvbmZp ZyBzbyBpdCBpcyBwb3NzaWJsZQo+ID4+IHRvIHVzZSBvbmx5IHZpZGVvIG1vZGUuCj4gPj4gCj4g Pj4gVGhlIEhETUkgbW9kZSBpcyBhdXRvbWF0aWNhbGx5IHN0YXJ0ZWQgYXQgYm9vdCBhbmQgdGhl IGF1ZGlvCj4gPj4gKHdoZW4gZW5hYmxlZCkgcmVnaXN0ZXJzIGFzIGEgY29kZWMgaW50byBBTFNB Lgo+ID4+IAo+ID4+IFNQRElGIERBSSBmb3JtYXQgd2FzIGFsc28gYWRkZWQgdG8gQVNvQyBhcyBp dCBpcyByZXF1aXJlZAo+ID4+IGJ5IGFkdjc1MTEgYXVkaW8uCj4gPj4gCj4gPj4gU2lnbmVkLW9m Zi1ieTogSm9zZSBBYnJldSA8am9hYnJldUBzeW5vcHN5cy5jb20+Cj4gPj4gLS0tCj4gPj4gCj4g Pj4gTm8gY2hhbmdlcyB2MSAtPiB2Mi4KPiA+PiAKPiA+PiAgZHJpdmVycy9ncHUvZHJtL2kyYy9L Y29uZmlnICAgICAgICAgfCAgIDExICsKPiA+PiAgZHJpdmVycy9ncHUvZHJtL2kyYy9NYWtlZmls ZSAgICAgICAgfCAgICAyICsKPiA+PiAgZHJpdmVycy9ncHUvZHJtL2kyYy9hZHY3NTExLmMgICAg ICAgfCAxMDI0IC0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0KPiA+PiAgZHJpdmVycy9ncHUv ZHJtL2kyYy9hZHY3NTExLmggICAgICAgfCAgIDQxICsrCj4gPj4gIGRyaXZlcnMvZ3B1L2RybS9p MmMvYWR2NzUxMV9hdWRpby5jIHwgIDMxMCArKysrKysrKysrKwo+ID4+ICBkcml2ZXJzL2dwdS9k cm0vaTJjL2Fkdjc1MTFfY29yZS5jICB8IDEwMDUgKysrKysrKysrKysrKysrKysrKysrKysrKysr Kwo+ID4gCj4gPiBQbGVhc2UgdXNlIGdpdC1mb3JtYXQtcGF0Y2ggLU0gdG8gZGV0ZWN0IHJlbmFt ZXMgaWYgeW91IHNlbmQgYSBuZXcgdmVyc2lvbgo+ID4gb2YgdGhpcyBzZXJpZXMsIGl0IHdpbGwg aGVscCB3aXRoIHJldmlldy4KPiAKPiBPaywgd2lsbCBkbyB0aGF0IGluIG5leHQgdmVyc2lvbi4K PiAKPiA+PiAgaW5jbHVkZS9zb3VuZC9zb2MtZGFpLmggICAgICAgICAgICAgfCAgICAxICsKPiA+ PiAgNyBmaWxlcyBjaGFuZ2VkLCAxMzcwIGluc2VydGlvbnMoKyksIDEwMjQgZGVsZXRpb25zKC0p Cj4gPj4gIGRlbGV0ZSBtb2RlIDEwMDY0NCBkcml2ZXJzL2dwdS9kcm0vaTJjL2Fkdjc1MTEuYwo+ ID4+ICBjcmVhdGUgbW9kZSAxMDA2NDQgZHJpdmVycy9ncHUvZHJtL2kyYy9hZHY3NTExX2F1ZGlv LmMKPiA+PiAgY3JlYXRlIG1vZGUgMTAwNjQ0IGRyaXZlcnMvZ3B1L2RybS9pMmMvYWR2NzUxMV9j b3JlLmMKPiA+IAo+ID4gW3NuaXBdCj4gPiAKPiA+PiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9ncHUv ZHJtL2kyYy9hZHY3NTExX2NvcmUuYwo+ID4+IGIvZHJpdmVycy9ncHUvZHJtL2kyYy9hZHY3NTEx X2NvcmUuYyBuZXcgZmlsZSBtb2RlIDEwMDY0NAo+ID4+IGluZGV4IDAwMDAwMDAuLmQ1NDI1NmEK PiA+PiAtLS0gL2Rldi9udWxsCj4gPj4gKysrIGIvZHJpdmVycy9ncHUvZHJtL2kyYy9hZHY3NTEx X2NvcmUuYwo+ID4gCj4gPiBbc25pcF0KPiA+IAo+ID4+ICtzdGF0aWMgaW50IGFkdjc1MTFfcHJv YmUoc3RydWN0IGkyY19jbGllbnQgKmkyYywgY29uc3Qgc3RydWN0Cj4gPj4gaTJjX2RldmljZV9p ZCAqaWQpICt7Cj4gPj4gKwlzdHJ1Y3QgYWR2NzUxMV9saW5rX2NvbmZpZyBsaW5rX2NvbmZpZzsK PiA+PiArCXN0cnVjdCBhZHY3NTExICphZHY3NTExOwo+ID4+ICsJc3RydWN0IGRldmljZSAqZGV2 ID0gJmkyYy0+ZGV2Owo+ID4+ICsJdW5zaWduZWQgaW50IHZhbDsKPiA+PiArCWludCByZXQ7Cj4g Pj4gKwo+ID4+ICsJaWYgKCFkZXYtPm9mX25vZGUpCj4gPj4gKwkJcmV0dXJuIC1FSU5WQUw7Cj4g Pj4gKwo+ID4+ICsJYWR2NzUxMSA9IGRldm1fa3phbGxvYyhkZXYsIHNpemVvZigqYWR2NzUxMSks IEdGUF9LRVJORUwpOwo+ID4+ICsJaWYgKCFhZHY3NTExKQo+ID4+ICsJCXJldHVybiAtRU5PTUVN Owo+ID4+ICsKPiA+PiArCWFkdjc1MTEtPnBvd2VyZWQgPSBmYWxzZTsKPiA+PiArCWFkdjc1MTEt PnN0YXR1cyA9IGNvbm5lY3Rvcl9zdGF0dXNfZGlzY29ubmVjdGVkOwo+ID4+ICsKPiA+PiArCXJl dCA9IGFkdjc1MTFfcGFyc2VfZHQoZGV2LT5vZl9ub2RlLCAmbGlua19jb25maWcpOwo+ID4+ICsJ aWYgKHJldCkKPiA+PiArCQlyZXR1cm4gcmV0Owo+ID4+ICsKPiA+PiArCS8qCj4gPj4gKwkgKiBU aGUgcG93ZXIgZG93biBHUElPIGlzIG9wdGlvbmFsLiBJZiBwcmVzZW50LCB0b2dnbGUgaXQgZnJv bSBhY3RpdmUKPiA+PiB0bwo+ID4+ICsJICogaW5hY3RpdmUgdG8gd2FrZSB1cCB0aGUgZW5jb2Rl ci4KPiA+PiArCSAqLwo+ID4+ICsJYWR2NzUxMS0+Z3Bpb19wZCA9IGRldm1fZ3Bpb2RfZ2V0X29w dGlvbmFsKGRldiwgInBkIiwgCkdQSU9EX09VVF9ISUdIKTsKPiA+PiArCWlmIChJU19FUlIoYWR2 NzUxMS0+Z3Bpb19wZCkpCj4gPj4gKwkJcmV0dXJuIFBUUl9FUlIoYWR2NzUxMS0+Z3Bpb19wZCk7 Cj4gPj4gKwo+ID4+ICsJaWYgKGFkdjc1MTEtPmdwaW9fcGQpIHsKPiA+PiArCQltZGVsYXkoNSk7 Cj4gPj4gKwkJZ3Bpb2Rfc2V0X3ZhbHVlX2NhbnNsZWVwKGFkdjc1MTEtPmdwaW9fcGQsIDApOwo+ ID4+ICsJfQo+ID4+ICsKPiA+PiArCWFkdjc1MTEtPnJlZ21hcCA9IGRldm1fcmVnbWFwX2luaXRf aTJjKGkyYywgJmFkdjc1MTFfcmVnbWFwX2NvbmZpZyk7Cj4gPj4gKwlpZiAoSVNfRVJSKGFkdjc1 MTEtPnJlZ21hcCkpCj4gPj4gKwkJcmV0dXJuIFBUUl9FUlIoYWR2NzUxMS0+cmVnbWFwKTsKPiA+ PiArCj4gPj4gKwlyZXQgPSByZWdtYXBfcmVhZChhZHY3NTExLT5yZWdtYXAsIEFEVjc1MTFfUkVH X0NISVBfUkVWSVNJT04sICZ2YWwpOwo+ID4+ICsJaWYgKHJldCkKPiA+PiArCQlyZXR1cm4gcmV0 Owo+ID4+ICsJZGV2X2RiZyhkZXYsICJSZXYuICVkXG4iLCB2YWwpOwo+ID4+ICsKPiA+PiArCXJl dCA9IHJlZ21hcF9yZWdpc3Rlcl9wYXRjaChhZHY3NTExLT5yZWdtYXAsIGFkdjc1MTFfZml4ZWRf cmVnaXN0ZXJzLAo+ID4+ICsJCQkJICAgIEFSUkFZX1NJWkUoYWR2NzUxMV9maXhlZF9yZWdpc3Rl cnMpKTsKPiA+PiArCWlmIChyZXQpCj4gPj4gKwkJcmV0dXJuIHJldDsKPiA+PiArCj4gPj4gKwly ZWdtYXBfd3JpdGUoYWR2NzUxMS0+cmVnbWFwLCBBRFY3NTExX1JFR19FRElEX0kyQ19BRERSLAo+ ID4+IGVkaWRfaTJjX2FkZHIpOwo+ID4+ICsJcmVnbWFwX3dyaXRlKGFkdjc1MTEtPnJlZ21hcCwg QURWNzUxMV9SRUdfUEFDS0VUX0kyQ19BRERSLAo+ID4+ICsJCSAgICAgcGFja2V0X2kyY19hZGRy KTsKPiA+PiArCXJlZ21hcF93cml0ZShhZHY3NTExLT5yZWdtYXAsIEFEVjc1MTFfUkVHX0NFQ19J MkNfQUREUiwgCmNlY19pMmNfYWRkcik7Cj4gPj4gKwlhZHY3NTExX3BhY2tldF9kaXNhYmxlKGFk djc1MTEsIDB4ZmZmZik7Cj4gPj4gKwo+ID4+ICsJYWR2NzUxMS0+aTJjX21haW4gPSBpMmM7Cj4g Pj4gKwlhZHY3NTExLT5pMmNfZWRpZCA9IGkyY19uZXdfZHVtbXkoaTJjLT5hZGFwdGVyLCBlZGlk X2kyY19hZGRyID4+IDEpOwo+ID4+ICsJaWYgKCFhZHY3NTExLT5pMmNfZWRpZCkKPiA+PiArCQly ZXR1cm4gLUVOT01FTTsKPiA+PiArCj4gPj4gKwlpZiAoaTJjLT5pcnEpIHsKPiA+PiArCQlpbml0 X3dhaXRxdWV1ZV9oZWFkKCZhZHY3NTExLT53cSk7Cj4gPj4gKwo+ID4+ICsJCXJldCA9IGRldm1f cmVxdWVzdF90aHJlYWRlZF9pcnEoZGV2LCBpMmMtPmlycSwgTlVMTCwKPiA+PiArCQkJCQkJYWR2 NzUxMV9pcnFfaGFuZGxlciwKPiA+PiArCQkJCQkJSVJRRl9PTkVTSE9ULCBkZXZfbmFtZShkZXYp LAo+ID4+ICsJCQkJCQlhZHY3NTExKTsKPiA+PiArCQlpZiAocmV0KQo+ID4+ICsJCQlnb3RvIGVy cl9pMmNfdW5yZWdpc3Rlcl9kZXZpY2U7Cj4gPj4gKwl9Cj4gPj4gKwo+ID4+ICsJLyogQ0VDIGlz IHVudXNlZCBmb3Igbm93ICovCj4gPj4gKwlyZWdtYXBfd3JpdGUoYWR2NzUxMS0+cmVnbWFwLCBB RFY3NTExX1JFR19DRUNfQ1RSTCwKPiA+PiArCQkgICAgIEFEVjc1MTFfQ0VDX0NUUkxfUE9XRVJf RE9XTik7Cj4gPj4gKwo+ID4+ICsJYWR2NzUxMV9wb3dlcl9vZmYoYWR2NzUxMSk7Cj4gPj4gKwo+ ID4+ICsJaTJjX3NldF9jbGllbnRkYXRhKGkyYywgYWR2NzUxMSk7Cj4gPj4gKwo+ID4+ICsjaWZk ZWYgQ09ORklHX0RSTV9JMkNfQURWNzUxMV9BVURJTwo+ID4+ICsJYWR2NzUxMV9hdWRpb19pbml0 KCZpMmMtPmRldik7Cj4gPj4gKyNlbmRpZgo+ID4gCj4gPiBTaG91bGRuJ3Qgd2UgY29uZGl0aW9u IHRoaXMgdG8gdGhlIGF1ZGlvIGNoYW5uZWwgYmVpbmcgc29tZWhvdyBkZXNjcmliZWQKPiA+IGlu IERUID8gSWYgYSBib2FyZCBkb2Vzbid0IHJvdXRlIGF1ZGlvIHNpZ25hbHMgdG8gdGhlIEFEVjc1 MTEgYXVkaW8KPiA+IGlucHV0LCB0aGVyZSdzIG5vIG5lZWQgdG8gcmVnaXN0ZXIgYW4gYXVkaW8g Y29kZWMuCj4gCj4gSSBjYW4gZG8gdGhpcyBidXQgdGhlIGF1ZGlvIGlzIGFscmVhZHkgY29uZGl0 aW9uYWxseSBjb21waWxlZCB1c2luZwo+IG1lbnVjb25maWcuIElzIGl0IHJlYWxseSBuZWNlc3Nh cnkgdG8gYWRkIHRoaXMgZXh0cmEgbGF5ZXIgb2YgY29uZGl0aW9uPwoKVGhlIGlkZWEgaXMgdGhh dCBlbmFibGluZyBzdXBwb3J0IGZvciBBRFY3NTExIGF1ZGlvIGluIHRoZSBrZXJuZWwgaXNuJ3Qg CmNvdXBsZWQgd2l0aCB3aGV0aGVyIHRoZSBzeXN0ZW0gaW5jbHVkZXMgYXVkaW8gc3VwcG9ydC4g SXQgd291bGQgYmUgY29uZnVzaW5nLCAKYW5kIHdvdWxkIGFsc28gd2FzdGUgcmVzb3VyY2VzLCB0 byBjcmVhdGUgYSBMaW51eCBzb3VuZCBkZXZpY2Ugd2hlbiBubyBzb3VuZCAKY2hhbm5lbCBpcyBy b3V0ZWQgb24gdGhlIGJvYXJkLgoKPiA+PiArCj4gPj4gKwlhZHY3NTExX3NldF9saW5rX2NvbmZp ZyhhZHY3NTExLCAmbGlua19jb25maWcpOwo+ID4+ICsKPiA+PiArCS8qIEVuYWJsZSBIRE1JIG1v ZGUgKi8KPiA+PiArCXJlZ21hcF91cGRhdGVfYml0cyhhZHY3NTExLT5yZWdtYXAsIEFEVjc1MTFf UkVHX0hEQ1BfSERNSV9DRkcsCj4gPj4gKwkJCUFEVjc1MTFfSERNSV9DRkdfTU9ERV9NQVNLLAo+ ID4+ICsJCQlBRFY3NTExX0hETUlfQ0ZHX01PREVfSERNSSk7Cj4gPj4gKwo+ID4+ICsJcmV0dXJu IDA7Cj4gPj4gKwo+ID4+ICtlcnJfaTJjX3VucmVnaXN0ZXJfZGV2aWNlOgo+ID4+ICsJaTJjX3Vu cmVnaXN0ZXJfZGV2aWNlKGFkdjc1MTEtPmkyY19lZGlkKTsKPiA+PiArCj4gPj4gKwlyZXR1cm4g cmV0Owo+ID4+ICt9CgotLSAKUmVnYXJkcywKCkxhdXJlbnQgUGluY2hhcnQKCl9fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmRyaS1kZXZlbCBtYWlsaW5nIGxp c3QKZHJpLWRldmVsQGxpc3RzLmZyZWVkZXNrdG9wLm9yZwpodHRwczovL2xpc3RzLmZyZWVkZXNr dG9wLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2RyaS1kZXZlbAo= From mboxrd@z Thu Jan 1 00:00:00 1970 From: laurent.pinchart@ideasonboard.com (Laurent Pinchart) Date: Tue, 05 Apr 2016 00:41:27 +0300 Subject: [PATCH 1/3 v2] drm/i2c/adv7511: Add audio support In-Reply-To: <57022E63.2010900@synopsys.com> References: <5276355.2bQYPXCpz6@avalon> <57022E63.2010900@synopsys.com> List-ID: Message-ID: <2037371.nWufVmEmFd@avalon> To: linux-snps-arc@lists.infradead.org Hi Jose, On Monday 04 Apr 2016 10:05:39 Jose Abreu wrote: > On 01-04-2016 18:10, Laurent Pinchart wrote: > > On Monday 28 Mar 2016 15:36:09 Jose Abreu wrote: > >> This patch adds audio support for the ADV7511 HDMI transmitter > >> using ALSA SoC. > >> > >> The code was ported from Analog Devices linux tree from > >> commit 1770c4a1e32b ("Merge remote-tracking branch > >> > >> 'xilinx/master' into xcomm_zynq"), which is available at: > >> - https://github.com/analogdevicesinc/linux/ > >> > >> The main core file was renamed from adv7511.c to adv7511_core.c > >> so that audio and video compile into a single adv7511.ko module > >> and to keep up with Analog Devices kernel tree. > >> > >> The audio can be disabled using menu-config so it is possible > >> to use only video mode. > >> > >> The HDMI mode is automatically started at boot and the audio > >> (when enabled) registers as a codec into ALSA. > >> > >> SPDIF DAI format was also added to ASoC as it is required > >> by adv7511 audio. > >> > >> Signed-off-by: Jose Abreu > >> --- > >> > >> No changes v1 -> v2. > >> > >> drivers/gpu/drm/i2c/Kconfig | 11 + > >> drivers/gpu/drm/i2c/Makefile | 2 + > >> drivers/gpu/drm/i2c/adv7511.c | 1024 ---------------------------- > >> drivers/gpu/drm/i2c/adv7511.h | 41 ++ > >> drivers/gpu/drm/i2c/adv7511_audio.c | 310 +++++++++++ > >> drivers/gpu/drm/i2c/adv7511_core.c | 1005 ++++++++++++++++++++++++++++ > > > > Please use git-format-patch -M to detect renames if you send a new version > > of this series, it will help with review. > > Ok, will do that in next version. > > >> include/sound/soc-dai.h | 1 + > >> 7 files changed, 1370 insertions(+), 1024 deletions(-) > >> delete mode 100644 drivers/gpu/drm/i2c/adv7511.c > >> create mode 100644 drivers/gpu/drm/i2c/adv7511_audio.c > >> create mode 100644 drivers/gpu/drm/i2c/adv7511_core.c > > > > [snip] > > > >> diff --git a/drivers/gpu/drm/i2c/adv7511_core.c > >> b/drivers/gpu/drm/i2c/adv7511_core.c new file mode 100644 > >> index 0000000..d54256a > >> --- /dev/null > >> +++ b/drivers/gpu/drm/i2c/adv7511_core.c > > > > [snip] > > > >> +static int adv7511_probe(struct i2c_client *i2c, const struct > >> i2c_device_id *id) +{ > >> + struct adv7511_link_config link_config; > >> + struct adv7511 *adv7511; > >> + struct device *dev = &i2c->dev; > >> + unsigned int val; > >> + int ret; > >> + > >> + if (!dev->of_node) > >> + return -EINVAL; > >> + > >> + adv7511 = devm_kzalloc(dev, sizeof(*adv7511), GFP_KERNEL); > >> + if (!adv7511) > >> + return -ENOMEM; > >> + > >> + adv7511->powered = false; > >> + adv7511->status = connector_status_disconnected; > >> + > >> + ret = adv7511_parse_dt(dev->of_node, &link_config); > >> + if (ret) > >> + return ret; > >> + > >> + /* > >> + * The power down GPIO is optional. If present, toggle it from active > >> to > >> + * inactive to wake up the encoder. > >> + */ > >> + adv7511->gpio_pd = devm_gpiod_get_optional(dev, "pd", GPIOD_OUT_HIGH); > >> + if (IS_ERR(adv7511->gpio_pd)) > >> + return PTR_ERR(adv7511->gpio_pd); > >> + > >> + if (adv7511->gpio_pd) { > >> + mdelay(5); > >> + gpiod_set_value_cansleep(adv7511->gpio_pd, 0); > >> + } > >> + > >> + adv7511->regmap = devm_regmap_init_i2c(i2c, &adv7511_regmap_config); > >> + if (IS_ERR(adv7511->regmap)) > >> + return PTR_ERR(adv7511->regmap); > >> + > >> + ret = regmap_read(adv7511->regmap, ADV7511_REG_CHIP_REVISION, &val); > >> + if (ret) > >> + return ret; > >> + dev_dbg(dev, "Rev. %d\n", val); > >> + > >> + ret = regmap_register_patch(adv7511->regmap, adv7511_fixed_registers, > >> + ARRAY_SIZE(adv7511_fixed_registers)); > >> + if (ret) > >> + return ret; > >> + > >> + regmap_write(adv7511->regmap, ADV7511_REG_EDID_I2C_ADDR, > >> edid_i2c_addr); > >> + regmap_write(adv7511->regmap, ADV7511_REG_PACKET_I2C_ADDR, > >> + packet_i2c_addr); > >> + regmap_write(adv7511->regmap, ADV7511_REG_CEC_I2C_ADDR, cec_i2c_addr); > >> + adv7511_packet_disable(adv7511, 0xffff); > >> + > >> + adv7511->i2c_main = i2c; > >> + adv7511->i2c_edid = i2c_new_dummy(i2c->adapter, edid_i2c_addr >> 1); > >> + if (!adv7511->i2c_edid) > >> + return -ENOMEM; > >> + > >> + if (i2c->irq) { > >> + init_waitqueue_head(&adv7511->wq); > >> + > >> + ret = devm_request_threaded_irq(dev, i2c->irq, NULL, > >> + adv7511_irq_handler, > >> + IRQF_ONESHOT, dev_name(dev), > >> + adv7511); > >> + if (ret) > >> + goto err_i2c_unregister_device; > >> + } > >> + > >> + /* CEC is unused for now */ > >> + regmap_write(adv7511->regmap, ADV7511_REG_CEC_CTRL, > >> + ADV7511_CEC_CTRL_POWER_DOWN); > >> + > >> + adv7511_power_off(adv7511); > >> + > >> + i2c_set_clientdata(i2c, adv7511); > >> + > >> +#ifdef CONFIG_DRM_I2C_ADV7511_AUDIO > >> + adv7511_audio_init(&i2c->dev); > >> +#endif > > > > Shouldn't we condition this to the audio channel being somehow described > > in DT ? If a board doesn't route audio signals to the ADV7511 audio > > input, there's no need to register an audio codec. > > I can do this but the audio is already conditionally compiled using > menuconfig. Is it really necessary to add this extra layer of condition? 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. > >> + > >> + adv7511_set_link_config(adv7511, &link_config); > >> + > >> + /* Enable HDMI mode */ > >> + regmap_update_bits(adv7511->regmap, ADV7511_REG_HDCP_HDMI_CFG, > >> + ADV7511_HDMI_CFG_MODE_MASK, > >> + ADV7511_HDMI_CFG_MODE_HDMI); > >> + > >> + return 0; > >> + > >> +err_i2c_unregister_device: > >> + i2c_unregister_device(adv7511->i2c_edid); > >> + > >> + return ret; > >> +} -- Regards, Laurent Pinchart From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756565AbcDDVl2 (ORCPT ); Mon, 4 Apr 2016 17:41:28 -0400 Received: from galahad.ideasonboard.com ([185.26.127.97]:48484 "EHLO galahad.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754350AbcDDVl0 (ORCPT ); Mon, 4 Apr 2016 17:41:26 -0400 From: Laurent Pinchart To: Jose Abreu Cc: dri-devel@lists.freedesktop.org, linux-snps-arc@lists.infradead.org, linux-kernel@vger.kernel.org, alsa-devel@alsa-project.org, devicetree@vger.kernel.org, tixy@linaro.org, mark.rutland@arm.com, broonie@kernel.org, Alexey.Brodkin@synopsys.com, lgirdwood@gmail.com, yitian.bu@tangramtek.com, wsa+renesas@sang-engineering.com, laurent.pinchart+renesas@ideasonboard.com, nariman@opensource.wolfsonmicro.com, Maruthi.Bayyavarapu@amd.com, pawel.moll@arm.com, ijc+devicetree@hellion.org.uk, Vineet.Gupta1@synopsys.com, buyitian@gmail.com, perex@perex.cz, tiwai@suse.com, robh+dt@kernel.org, galak@codeaurora.org, alexander.deucher@amd.com, CARLOS.PALMINHA@synopsys.com Subject: Re: [PATCH 1/3 v2] drm/i2c/adv7511: Add audio support Date: Tue, 05 Apr 2016 00:41:27 +0300 Message-ID: <2037371.nWufVmEmFd@avalon> User-Agent: KMail/4.14.10 (Linux/4.1.15-gentoo-r1; KDE/4.14.16; x86_64; ; ) In-Reply-To: <57022E63.2010900@synopsys.com> References: <5276355.2bQYPXCpz6@avalon> <57022E63.2010900@synopsys.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Jose, On Monday 04 Apr 2016 10:05:39 Jose Abreu wrote: > On 01-04-2016 18:10, Laurent Pinchart wrote: > > On Monday 28 Mar 2016 15:36:09 Jose Abreu wrote: > >> This patch adds audio support for the ADV7511 HDMI transmitter > >> using ALSA SoC. > >> > >> The code was ported from Analog Devices linux tree from > >> commit 1770c4a1e32b ("Merge remote-tracking branch > >> > >> 'xilinx/master' into xcomm_zynq"), which is available at: > >> - https://github.com/analogdevicesinc/linux/ > >> > >> The main core file was renamed from adv7511.c to adv7511_core.c > >> so that audio and video compile into a single adv7511.ko module > >> and to keep up with Analog Devices kernel tree. > >> > >> The audio can be disabled using menu-config so it is possible > >> to use only video mode. > >> > >> The HDMI mode is automatically started at boot and the audio > >> (when enabled) registers as a codec into ALSA. > >> > >> SPDIF DAI format was also added to ASoC as it is required > >> by adv7511 audio. > >> > >> Signed-off-by: Jose Abreu > >> --- > >> > >> No changes v1 -> v2. > >> > >> drivers/gpu/drm/i2c/Kconfig | 11 + > >> drivers/gpu/drm/i2c/Makefile | 2 + > >> drivers/gpu/drm/i2c/adv7511.c | 1024 ---------------------------- > >> drivers/gpu/drm/i2c/adv7511.h | 41 ++ > >> drivers/gpu/drm/i2c/adv7511_audio.c | 310 +++++++++++ > >> drivers/gpu/drm/i2c/adv7511_core.c | 1005 ++++++++++++++++++++++++++++ > > > > Please use git-format-patch -M to detect renames if you send a new version > > of this series, it will help with review. > > Ok, will do that in next version. > > >> include/sound/soc-dai.h | 1 + > >> 7 files changed, 1370 insertions(+), 1024 deletions(-) > >> delete mode 100644 drivers/gpu/drm/i2c/adv7511.c > >> create mode 100644 drivers/gpu/drm/i2c/adv7511_audio.c > >> create mode 100644 drivers/gpu/drm/i2c/adv7511_core.c > > > > [snip] > > > >> diff --git a/drivers/gpu/drm/i2c/adv7511_core.c > >> b/drivers/gpu/drm/i2c/adv7511_core.c new file mode 100644 > >> index 0000000..d54256a > >> --- /dev/null > >> +++ b/drivers/gpu/drm/i2c/adv7511_core.c > > > > [snip] > > > >> +static int adv7511_probe(struct i2c_client *i2c, const struct > >> i2c_device_id *id) +{ > >> + struct adv7511_link_config link_config; > >> + struct adv7511 *adv7511; > >> + struct device *dev = &i2c->dev; > >> + unsigned int val; > >> + int ret; > >> + > >> + if (!dev->of_node) > >> + return -EINVAL; > >> + > >> + adv7511 = devm_kzalloc(dev, sizeof(*adv7511), GFP_KERNEL); > >> + if (!adv7511) > >> + return -ENOMEM; > >> + > >> + adv7511->powered = false; > >> + adv7511->status = connector_status_disconnected; > >> + > >> + ret = adv7511_parse_dt(dev->of_node, &link_config); > >> + if (ret) > >> + return ret; > >> + > >> + /* > >> + * The power down GPIO is optional. If present, toggle it from active > >> to > >> + * inactive to wake up the encoder. > >> + */ > >> + adv7511->gpio_pd = devm_gpiod_get_optional(dev, "pd", GPIOD_OUT_HIGH); > >> + if (IS_ERR(adv7511->gpio_pd)) > >> + return PTR_ERR(adv7511->gpio_pd); > >> + > >> + if (adv7511->gpio_pd) { > >> + mdelay(5); > >> + gpiod_set_value_cansleep(adv7511->gpio_pd, 0); > >> + } > >> + > >> + adv7511->regmap = devm_regmap_init_i2c(i2c, &adv7511_regmap_config); > >> + if (IS_ERR(adv7511->regmap)) > >> + return PTR_ERR(adv7511->regmap); > >> + > >> + ret = regmap_read(adv7511->regmap, ADV7511_REG_CHIP_REVISION, &val); > >> + if (ret) > >> + return ret; > >> + dev_dbg(dev, "Rev. %d\n", val); > >> + > >> + ret = regmap_register_patch(adv7511->regmap, adv7511_fixed_registers, > >> + ARRAY_SIZE(adv7511_fixed_registers)); > >> + if (ret) > >> + return ret; > >> + > >> + regmap_write(adv7511->regmap, ADV7511_REG_EDID_I2C_ADDR, > >> edid_i2c_addr); > >> + regmap_write(adv7511->regmap, ADV7511_REG_PACKET_I2C_ADDR, > >> + packet_i2c_addr); > >> + regmap_write(adv7511->regmap, ADV7511_REG_CEC_I2C_ADDR, cec_i2c_addr); > >> + adv7511_packet_disable(adv7511, 0xffff); > >> + > >> + adv7511->i2c_main = i2c; > >> + adv7511->i2c_edid = i2c_new_dummy(i2c->adapter, edid_i2c_addr >> 1); > >> + if (!adv7511->i2c_edid) > >> + return -ENOMEM; > >> + > >> + if (i2c->irq) { > >> + init_waitqueue_head(&adv7511->wq); > >> + > >> + ret = devm_request_threaded_irq(dev, i2c->irq, NULL, > >> + adv7511_irq_handler, > >> + IRQF_ONESHOT, dev_name(dev), > >> + adv7511); > >> + if (ret) > >> + goto err_i2c_unregister_device; > >> + } > >> + > >> + /* CEC is unused for now */ > >> + regmap_write(adv7511->regmap, ADV7511_REG_CEC_CTRL, > >> + ADV7511_CEC_CTRL_POWER_DOWN); > >> + > >> + adv7511_power_off(adv7511); > >> + > >> + i2c_set_clientdata(i2c, adv7511); > >> + > >> +#ifdef CONFIG_DRM_I2C_ADV7511_AUDIO > >> + adv7511_audio_init(&i2c->dev); > >> +#endif > > > > Shouldn't we condition this to the audio channel being somehow described > > in DT ? If a board doesn't route audio signals to the ADV7511 audio > > input, there's no need to register an audio codec. > > I can do this but the audio is already conditionally compiled using > menuconfig. Is it really necessary to add this extra layer of condition? 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. > >> + > >> + adv7511_set_link_config(adv7511, &link_config); > >> + > >> + /* Enable HDMI mode */ > >> + regmap_update_bits(adv7511->regmap, ADV7511_REG_HDCP_HDMI_CFG, > >> + ADV7511_HDMI_CFG_MODE_MASK, > >> + ADV7511_HDMI_CFG_MODE_HDMI); > >> + > >> + return 0; > >> + > >> +err_i2c_unregister_device: > >> + i2c_unregister_device(adv7511->i2c_edid); > >> + > >> + return ret; > >> +} -- Regards, Laurent Pinchart