From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Shevchenko, Andriy" Subject: Re: [PATCHv3] ASoC: Add support for BCM2835 Date: Mon, 18 Nov 2013 12:08:22 +0000 Message-ID: <1384776479.14845.208.camel@smile> References: <5289F9DF.8060406@koalo.de> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <5289F9DF.8060406-oZ8rN/sblLk@public.gmane.org> Content-Language: en-US Content-ID: <6DB828C271FD3848B21FBB4ED8069004-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> Sender: devicetree-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Florian Meier Cc: Liam Girdwood , Mark Brown , Stephen Warren , Lars-Peter Clausen , "alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw@public.gmane.org" , linux-rpi-kernel , devicetree , dmaengine , "linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , "linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org" List-Id: alsa-devel@alsa-project.org T24gTW9uLCAyMDEzLTExLTE4IGF0IDEyOjI4ICswMTAwLCBGbG9yaWFuIE1laWVyIHdyb3RlOg0K PiBUaGlzIGRyaXZlciBhZGRzIHN1cHBvcnQgZm9yIGRpZ2l0YWwgYXVkaW8gKEkyUykNCj4gZm9y IHRoZSBCQ00yODM1IFNvQyB0aGF0IGlzIHVzZWQgYnkgdGhlDQo+IFJhc3BiZXJyeSBQaS4gRXh0 ZXJuYWwgYXVkaW8gY29kZWNzIGNhbiBiZQ0KPiBjb25uZWN0ZWQgdG8gdGhlIFJhc3BiZXJyeSBQ aSB2aWEgUDUgaGVhZGVyLg0KPiANCj4gSXQgcmVsaWVzIG9uIGN5Y2xpYyBETUEgZW5naW5lIHN1 cHBvcnQgZm9yIEJDTTI4MzUuDQoNCkZldyBjb21tZW50cyBiZWxvdy4NCg0KW10NCg0KPiArc3Rh dGljIHZvaWQgYmNtMjgzNV9pMnNfc3RvcF9jbG9jayhzdHJ1Y3QgYmNtMjgzNV9pMnNfZGV2ICpk ZXYpDQo+ICt7DQo+ICsJdWludDMyX3QgY2xrcmVnOw0KPiArCWludCB0aW1lb3V0ID0gMTAwMDsN Cj4gKw0KPiArCS8qIFN0b3AgY2xvY2sgKi8NCj4gKwlyZWdtYXBfdXBkYXRlX2JpdHMoZGV2LT5j bGtfcmVnbWFwLCBCQ00yODM1X0NMS19QQ01DVExfUkVHLA0KPiArCQkJQkNNMjgzNV9DTEtfUEFT U1dEX01BU0sgfCBCQ00yODM1X0NMS19FTkFCLA0KPiArCQkJQkNNMjgzNV9DTEtfUEFTU1dEKTsN Cj4gKw0KPiArCS8qIFdhaXQgZm9yIHRoZSBCVVNZIGZsYWcgZ29pbmcgZG93biAqLw0KPiArCXdo aWxlICh0aW1lb3V0ID4gMCkgew0KPiArCQl0aW1lb3V0LS07DQoNCndoaWxlICgtLXRpbWVvdXQp IHsNCg0KPiArDQo+ICsJCXJlZ21hcF9yZWFkKGRldi0+Y2xrX3JlZ21hcCwgQkNNMjgzNV9DTEtf UENNQ1RMX1JFRywgJmNsa3JlZyk7DQo+ICsJCWlmICghKGNsa3JlZyAmIEJDTTI4MzVfQ0xLX0JV U1kpKQ0KPiArCQkJYnJlYWs7DQo+ICsJfQ0KPiArDQo+ICsJaWYgKHRpbWVvdXQgPD0gMCkgew0K DQppZiAoIXRpbWVvdXQpIHsNCg0KW10NCg0KPiArc3RhdGljIHZvaWQgYmNtMjgzNV9pMnNfY2xl YXJfZmlmb3Moc3RydWN0IGJjbTI4MzVfaTJzX2RldiAqZGV2LA0KPiArCQkJCSAgICBib29sIHR4 LCBib29sIHJ4KQ0KPiArew0KDQpbXQ0KDQo+ICsJLyogV2FpdCBmb3IgdGhlIFNZTkMgZmxhZyBj aGFuZ2luZyBpdCdzIHN0YXRlICovDQo+ICsJd2hpbGUgKHRpbWVvdXQgPiAwKSB7DQo+ICsJCXRp bWVvdXQtLTsNCg0Kd2hpbGUgKC0tdGltZW91dCkgew0KDQo+ICsJCXJlZ21hcF9yZWFkKGRldi0+ aTJzX3JlZ21hcCwgQkNNMjgzNV9JMlNfQ1NfQV9SRUcsICZjc3JlZyk7DQo+ICsJCWlmICgoY3Ny ZWcgJiBCQ00yODM1X0kyU19TWU5DKSAhPSBzeW5jdmFsKQ0KPiArCQkJYnJlYWs7DQo+ICsJfQ0K PiArDQo+ICsJaWYgKHRpbWVvdXQgPD0gMCkNCg0KaWYgKCF0aW1lb3V0KQ0KDQpbXQ0KDQo+ICtz dGF0aWMgaW50IGJjbTI4MzVfaTJzX2h3X3BhcmFtcyhzdHJ1Y3Qgc25kX3BjbV9zdWJzdHJlYW0g KnN1YnN0cmVhbSwNCj4gKwkJCQkgc3RydWN0IHNuZF9wY21faHdfcGFyYW1zICpwYXJhbXMsDQo+ ICsJCQkJIHN0cnVjdCBzbmRfc29jX2RhaSAqZGFpKQ0KPiArew0KPiArCXN0cnVjdCBiY20yODM1 X2kyc19kZXYgKmRldiA9IHNuZF9zb2NfZGFpX2dldF9kcnZkYXRhKGRhaSk7DQo+ICsNCj4gKwl1 bnNpZ25lZCBpbnQgc2FtcGxpbmdfcmF0ZSA9IHBhcmFtc19yYXRlKHBhcmFtcyk7DQo+ICsJdW5z aWduZWQgaW50IGRhdGFfbGVuZ3RoLCBkYXRhX2RlbGF5LCBiY2xrX3JhdGlvOw0KPiArCXVuc2ln bmVkIGludCBjaDFwb3MsIGNoMnBvcywgbW9kZSwgZm9ybWF0Ow0KPiArCXVuc2lnbmVkIGludCBt YXNoID0gQkNNMjgzNV9DTEtfTUFTSF8xOw0KPiArCXVuc2lnbmVkIGludCBkaXZpLCBkaXZmLCB0 YXJnZXRfZnJlcXVlbmN5Ow0KPiArCWludCBjbGtfc3JjID0gLTE7DQo+ICsJdW5zaWduZWQgaW50 IG1hc3RlciA9IGRldi0+Zm10ICYgU05EX1NPQ19EQUlGTVRfTUFTVEVSX01BU0s7DQo+ICsJdWlu dDMyX3QgYml0X21hc3RlciA9CShtYXN0ZXIgPT0gU05EX1NPQ19EQUlGTVRfQ0JTX0NGUw0KPiAr CQkJCQl8fCBtYXN0ZXIgPT0gU05EX1NPQ19EQUlGTVRfQ0JTX0NGTSk7DQoNCmJvb2wgPw0KDQo+ ICsNCj4gKwl1aW50MzJfdCBmcmFtZV9tYXN0ZXIgPQkobWFzdGVyID09IFNORF9TT0NfREFJRk1U X0NCU19DRlMNCg0KU28sIGlmIG1hc3RlciA9PSBTTkRfU09DX0RBSUZNVF9DQlNfQ0ZTIGJvdGgg Yml0X21hc3RlciBhbmQgZnJhbWVfbWFzdGVyDQp3aWxsIGJlIHRydWUuIElzIGl0IGNvcnJlY3Q/ DQoNCj4gKwkJCQkJfHwgbWFzdGVyID09IFNORF9TT0NfREFJRk1UX0NCTV9DRlMpOw0KDQpEaXR0 by4NCg0KPiArCXVpbnQzMl90IGNzcmVnOw0KPiArDQo+ICsJLyoNCj4gKwkgKiBJZiBhIHN0cmVh bSBpcyBhbHJlYWR5IGVuYWJsZWQsDQo+ICsJICogdGhlIHJlZ2lzdGVycyBhcmUgYWxyZWFkeSBz ZXQgcHJvcGVybHkuDQo+ICsJICovDQo+ICsJcmVnbWFwX3JlYWQoZGV2LT5pMnNfcmVnbWFwLCBC Q00yODM1X0kyU19DU19BX1JFRywgJmNzcmVnKTsNCj4gKw0KPiArCWlmIChjc3JlZyAmIChCQ00y ODM1X0kyU19UWE9OIHwgQkNNMjgzNV9JMlNfUlhPTikpDQo+ICsJCXJldHVybiAwOw0KPiArDQoN Cj4gKwlpZiAoYmNtMjgzNV9jbGtfZnJlcVtjbGtfc3JjXSAlIHRhcmdldF9mcmVxdWVuY3kgPT0g MA0KPiArCQkJJiYgYml0X21hc3RlciAmJiBmcmFtZV9tYXN0ZXIpIHsNCj4gKwkJZGl2aSA9IGJj bTI4MzVfY2xrX2ZyZXFbY2xrX3NyY10vdGFyZ2V0X2ZyZXF1ZW5jeTsNCj4gKwkJZGl2ZiA9IDA7 DQo+ICsJfSBlbHNlIHsNCj4gKwkJdWludDY0X3QgZGl2aWRlbmQ7DQo+ICsNCj4gKwkJaWYgKCFk ZXYtPmJjbGtfcmF0aW8pIHsNCj4gKwkJCS8qDQo+ICsJCQkgKiBPdmVyd3JpdGUgYmNsa19yYXRp bywgYmVjYXVzZSB0aGUNCj4gKwkJCSAqIGFib3ZlIHRyaWNrIGlzIG5vdCBuZWVkZWQgb3IgY2Fu DQo+ICsJCQkgKiBub3QgYmUgdXNlZC4NCj4gKwkJCSAqLw0KPiArCQkJYmNsa19yYXRpbyA9IDIq ZGF0YV9sZW5ndGg7DQoNCj0gMiAqIGRhdGFfbGVuZ3RoOw0KDQo+ICsJCX0NCj4gKw0KPiArCQl0 YXJnZXRfZnJlcXVlbmN5ID0gc2FtcGxpbmdfcmF0ZSpiY2xrX3JhdGlvOw0KPiArDQo+ICsJCWNs a19zcmMgPSBCQ00yODM1X0NMS19TUkNfUExMRDsNCj4gKwkJbWFzaCA9IEJDTTI4MzVfQ0xLX01B U0hfMTsNCj4gKw0KPiArCQlkaXZpZGVuZCA9IGJjbTI4MzVfY2xrX2ZyZXFbY2xrX3NyY107DQo+ ICsJCWRpdmlkZW5kICo9IDEwMjQ7DQo+ICsJCWRvX2RpdihkaXZpZGVuZCwgdGFyZ2V0X2ZyZXF1 ZW5jeSk7DQo+ICsJCWRpdmkgPSBkaXZpZGVuZCAvIDEwMjQ7DQo+ICsJCWRpdmYgPSBkaXZpZGVu ZCAlIDEwMjQ7DQoNCjEwMjQgaXMgYSBtYWdpYyBudW1iZXI/DQpDb3VsZCB5b3UgdXNlIHNoaWZ0 cyBhbmQgYml0d2lzZSBBTkRzIGhlcmU/DQoNCj4gKwljaDFwb3MgPSBkYXRhX2RlbGF5Ow0KPiAr CWNoMnBvcyA9IGJjbGtfcmF0aW8vMitkYXRhX2RlbGF5Ow0KDQonIC8gMiArICcNCg0KW10NCg0K PiArCW1vZGUgfD0gQkNNMjgzNV9JMlNfRkxFTihiY2xrX3JhdGlvLTEpOw0KPiArCW1vZGUgfD0g QkNNMjgzNV9JMlNfRlNMRU4oYmNsa19yYXRpby8yKTsNCg0KRGl0dG8uIFNwYWNlcyBhcmUgbWlz c2VkLg0KDQpbXQ0KDQo+ICsJY2FzZSBTTkRfU09DX0RBSUZNVF9JQl9JRjoNCj4gKwkJLyogQm90 aCAtIHRoZXJlZm9yZSwgbm9uZSBmb3IgQkNNKi8NCg0KJ00gKi8nDQoNCj4gKwkJYnJlYWs7DQo+ ICsJY2FzZSBTTkRfU09DX0RBSUZNVF9OQl9JRjoNCj4gKwkJLyoNCj4gKwkJICogSW52ZXJ0IG9u bHkgZnJhbWUgc3luYyAtIHRoZXJlZm9yZSwNCj4gKwkJICogSW52ZXJ0IG9ubHkgYml0IGNsb2Nr IGZvciBCQ00NCg0KU29tZXRoaW5nIHdyb25nIHdpdGggY2FzaW5nIGluIHRoZSBjb21tZW50YXJ5 Lg0KDQo+ICsJCSAqLw0KPiArCQltb2RlIHw9IEJDTTI4MzVfSTJTX0NMS0k7DQo+ICsJCWJyZWFr Ow0KPiArCWNhc2UgU05EX1NPQ19EQUlGTVRfSUJfTkY6DQo+ICsJCS8qDQo+ICsJCSAqIEludmVy dCBvbmx5IGJpdCBjbG9jayAtIHRoZXJlZm9yZSwNCj4gKwkJICogSW52ZXJ0IG9ubHkgZnJhbWUg c3luYyBmb3IgQkNNDQoNCkRpdHRvLg0KDQo+ICsJCSAqLw0KPiArCQltb2RlIHw9IEJDTTI4MzVf STJTX0ZTSTsNCj4gKwkJYnJlYWs7DQo+ICsJZGVmYXVsdDoNCj4gKwkJcmV0dXJuIC1FSU5WQUw7 DQo+ICsJfQ0KPiArDQo+ICsJcmVnbWFwX3dyaXRlKGRldi0+aTJzX3JlZ21hcCwgQkNNMjgzNV9J MlNfTU9ERV9BX1JFRywgbW9kZSk7DQo+ICsNCj4gKw0KDQpSZWR1bmRhbnQgZW1wdHkgbGluZS4N Cg0KW10NCg0KPiArDQo+ICsJcmV0dXJuIDA7DQo+ICt9DQo+ICsNCj4gKw0KDQpEaXR0by4NCg0K W10NCg0KPiArc3RhdGljIGludCBiY20yODM1X2kyc19zdGFydHVwKHN0cnVjdCBzbmRfcGNtX3N1 YnN0cmVhbSAqc3Vic3RyZWFtLA0KPiArCQkJICAgICAgIHN0cnVjdCBzbmRfc29jX2RhaSAqZGFp KQ0KPiArew0KPiArCXN0cnVjdCBiY20yODM1X2kyc19kZXYgKmRldiA9IHNuZF9zb2NfZGFpX2dl dF9kcnZkYXRhKGRhaSk7DQo+ICsNCj4gKwlpZiAoIWRhaS0+YWN0aXZlKSB7DQo+ICsJCS8qIFNo b3VsZCB0aGlzIHN0aWxsIGJlIHJ1bm5pbmcgc3RvcCBpdCAqLw0KPiArCQliY20yODM1X2kyc19z dG9wX2Nsb2NrKGRldik7DQo+ICsNCj4gKwkJLyogRW5hYmxlIFBDTSBibG9jayAqLw0KPiArCQly ZWdtYXBfdXBkYXRlX2JpdHMoZGV2LT5pMnNfcmVnbWFwLCBCQ00yODM1X0kyU19DU19BX1JFRywN Cj4gKwkJCQlCQ00yODM1X0kyU19FTiwgQkNNMjgzNV9JMlNfRU4pOw0KPiArDQo+ICsJCS8qDQo+ ICsJCSAqIERpc2FibGUgU1RCWQ0KDQpEb3QgaXMgbWlzc2luZz8NCg0KPiArCQkgKiBSZXF1aXJl cyBhdCBsZWFzdCA0IFBDTSBjbG9jayBjeWNsZXMgdG8gdGFrZSBlZmZlY3QNCj4gKwkJICovDQo+ ICsJCXJlZ21hcF91cGRhdGVfYml0cyhkZXYtPmkyc19yZWdtYXAsIEJDTTI4MzVfSTJTX0NTX0Ff UkVHLA0KPiArCQkJCUJDTTI4MzVfSTJTX1NUQlksIEJDTTI4MzVfSTJTX1NUQlkpOw0KPiArCX0N Cj4gKw0KPiArCXJldHVybiAwOw0KPiArfQ0KDQoNCg0KPiArc3RhdGljIHZvaWQgYmNtMjgzNV9p MnNfc2h1dGRvd24oc3RydWN0IHNuZF9wY21fc3Vic3RyZWFtICpzdWJzdHJlYW0sDQo+ICsJCXN0 cnVjdCBzbmRfc29jX2RhaSAqZGFpKQ0KPiArew0KPiArCXN0cnVjdCBiY20yODM1X2kyc19kZXYg KmRldiA9IHNuZF9zb2NfZGFpX2dldF9kcnZkYXRhKGRhaSk7DQo+ICsNCj4gKwliY20yODM1X2ky c19zdG9wKGRldiwgc3Vic3RyZWFtLCBkYWkpOw0KPiArDQo+ICsJLyogSWYgYm90aCBzdHJlYW1z IGFyZSBzdG9wcGVkLCBkaXNhYmxlIG1vZHVsZSBhbmQgY2xvY2sgKi8NCj4gKwlpZiAoIWRhaS0+ YWN0aXZlKSB7DQoNCmlmIChkYWktPmFjdGl2ZSkNCiByZXR1cm47DQoNCj4gKwkJLyogRGlzYWJs ZSB0aGUgbW9kdWxlICovDQo+ICsJCXJlZ21hcF91cGRhdGVfYml0cyhkZXYtPmkyc19yZWdtYXAs IEJDTTI4MzVfSTJTX0NTX0FfUkVHLA0KPiArCQkJCUJDTTI4MzVfSTJTX0VOLCAwKTsNCj4gKw0K PiArCQkvKg0KPiArCQkgKiBTdG9wcGluZyBjbG9jayBpcyBuZWNlc3NhcnksIGJlY2F1c2Ugc3Rv cCBkb2VzDQo+ICsJCSAqIG5vdCBzdG9wIHRoZSBjbG9jayB3aGVuIFNORF9TT0NfREFJRk1UX0NP TlQNCj4gKwkJICovDQo+ICsJCWJjbTI4MzVfaTJzX3N0b3BfY2xvY2soZGV2KTsNCj4gKwl9DQo+ ICt9DQo+ICsNCg0KW10NCg0KPiArc3RhdGljIGludCBiY20yODM1X2kyc19wcm9iZShzdHJ1Y3Qg cGxhdGZvcm1fZGV2aWNlICpwZGV2KQ0KPiArew0KPiArCXN0cnVjdCBiY20yODM1X2kyc19kZXYg KmRldjsNCj4gKwlpbnQgaTsNCj4gKwlpbnQgcmV0Ow0KPiArCXN0cnVjdCByZWdtYXAgKnJlZ21h cFsyXTsNCj4gKwlzdHJ1Y3QgcmVzb3VyY2UgKm1lbVsyXTsNCj4gKw0KPiArCS8qIFJlcXVlc3Qg Ym90aCBpb2FyZWFzICovDQo+ICsJZm9yIChpID0gMDsgaSA8PSAxOyBpKyspIHsNCj4gKwkJdm9p ZCBfX2lvbWVtICpiYXNlOw0KPiArDQo+ICsJCW1lbVtpXSA9IHBsYXRmb3JtX2dldF9yZXNvdXJj ZShwZGV2LCBJT1JFU09VUkNFX01FTSwgaSk7DQoNCj4gKwkJaWYgKCFtZW1baV0pIHsNCj4gKwkJ CWRldl9lcnIoJnBkZXYtPmRldiwgIkkyUyBwcm9iZTogTWVtb3J5IHJlc291cmNlIGNvdWxkIG5v dCBiZSBmb3VuZC5cbiIpOw0KPiArCQkJcmV0dXJuIC1FTk9ERVY7DQo+ICsJCX0NCg0KVW5uZWVk ZWQgbWVzc2FnaW5nIGFuZCBjaGVjay4NCg0KPiArDQo+ICsJCWJhc2UgPSBkZXZtX2lvcmVtYXBf cmVzb3VyY2UoJnBkZXYtPmRldiwgbWVtW2ldKTsNCj4gKwkJaWYgKCFiYXNlKSB7DQoNCkl0IHJl dHVybnMgRVJSX1BUUi4NClRodXMsDQoNCmlmIChJU19FUlIoYmFzZSkNCnJldHVybiBQVFJfRVJS KGJhc2UpDQoNCg0KPiArCQkJZGV2X2VycigmcGRldi0+ZGV2LCAiSTJTIHByb2JlOiBpb3JlbWFw IGZhaWxlZC5cbiIpOw0KDQpVbm5lZWRlZCBtZXNzYWdpbmcuDQoNCj4gKwkJCXJldHVybiAtRU5P TUVNOw0KDQpXcm9uZyBlcnJvciBjb2RlLiBTZWUgYWJvdmUuDQoNCg0KLS0gDQpBbmR5IFNoZXZj aGVua28gPGFuZHJpeS5zaGV2Y2hlbmtvQGludGVsLmNvbT4NCkludGVsIEZpbmxhbmQgT3kNCi0t LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t LS0tLS0tLS0tLQpJbnRlbCBGaW5sYW5kIE95ClJlZ2lzdGVyZWQgQWRkcmVzczogUEwgMjgxLCAw MDE4MSBIZWxzaW5raSAKQnVzaW5lc3MgSWRlbnRpdHkgQ29kZTogMDM1NzYwNiAtIDQgCkRvbWlj aWxlZCBpbiBIZWxzaW5raSAKClRoaXMgZS1tYWlsIGFuZCBhbnkgYXR0YWNobWVudHMgbWF5IGNv bnRhaW4gY29uZmlkZW50aWFsIG1hdGVyaWFsIGZvcgp0aGUgc29sZSB1c2Ugb2YgdGhlIGludGVu ZGVkIHJlY2lwaWVudChzKS4gQW55IHJldmlldyBvciBkaXN0cmlidXRpb24KYnkgb3RoZXJzIGlz IHN0cmljdGx5IHByb2hpYml0ZWQuIElmIHlvdSBhcmUgbm90IHRoZSBpbnRlbmRlZApyZWNpcGll bnQsIHBsZWFzZSBjb250YWN0IHRoZSBzZW5kZXIgYW5kIGRlbGV0ZSBhbGwgY29waWVzLgo= -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html From mboxrd@z Thu Jan 1 00:00:00 1970 From: andriy.shevchenko@intel.com (Shevchenko, Andriy) Date: Mon, 18 Nov 2013 12:08:22 +0000 Subject: [PATCHv3] ASoC: Add support for BCM2835 In-Reply-To: <5289F9DF.8060406@koalo.de> References: <5289F9DF.8060406@koalo.de> Message-ID: <1384776479.14845.208.camel@smile> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Mon, 2013-11-18 at 12:28 +0100, Florian Meier wrote: > This driver adds support for digital audio (I2S) > for the BCM2835 SoC that is used by the > Raspberry Pi. External audio codecs can be > connected to the Raspberry Pi via P5 header. > > It relies on cyclic DMA engine support for BCM2835. Few comments below. [] > +static void bcm2835_i2s_stop_clock(struct bcm2835_i2s_dev *dev) > +{ > + uint32_t clkreg; > + int timeout = 1000; > + > + /* Stop clock */ > + regmap_update_bits(dev->clk_regmap, BCM2835_CLK_PCMCTL_REG, > + BCM2835_CLK_PASSWD_MASK | BCM2835_CLK_ENAB, > + BCM2835_CLK_PASSWD); > + > + /* Wait for the BUSY flag going down */ > + while (timeout > 0) { > + timeout--; while (--timeout) { > + > + regmap_read(dev->clk_regmap, BCM2835_CLK_PCMCTL_REG, &clkreg); > + if (!(clkreg & BCM2835_CLK_BUSY)) > + break; > + } > + > + if (timeout <= 0) { if (!timeout) { [] > +static void bcm2835_i2s_clear_fifos(struct bcm2835_i2s_dev *dev, > + bool tx, bool rx) > +{ [] > + /* Wait for the SYNC flag changing it's state */ > + while (timeout > 0) { > + timeout--; while (--timeout) { > + regmap_read(dev->i2s_regmap, BCM2835_I2S_CS_A_REG, &csreg); > + if ((csreg & BCM2835_I2S_SYNC) != syncval) > + break; > + } > + > + if (timeout <= 0) if (!timeout) [] > +static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream, > + struct snd_pcm_hw_params *params, > + struct snd_soc_dai *dai) > +{ > + struct bcm2835_i2s_dev *dev = snd_soc_dai_get_drvdata(dai); > + > + unsigned int sampling_rate = params_rate(params); > + unsigned int data_length, data_delay, bclk_ratio; > + unsigned int ch1pos, ch2pos, mode, format; > + unsigned int mash = BCM2835_CLK_MASH_1; > + unsigned int divi, divf, target_frequency; > + int clk_src = -1; > + unsigned int master = dev->fmt & SND_SOC_DAIFMT_MASTER_MASK; > + uint32_t bit_master = (master == SND_SOC_DAIFMT_CBS_CFS > + || master == SND_SOC_DAIFMT_CBS_CFM); bool ? > + > + uint32_t frame_master = (master == SND_SOC_DAIFMT_CBS_CFS So, if master == SND_SOC_DAIFMT_CBS_CFS both bit_master and frame_master will be true. Is it correct? > + || master == SND_SOC_DAIFMT_CBM_CFS); Ditto. > + uint32_t csreg; > + > + /* > + * If a stream is already enabled, > + * the registers are already set properly. > + */ > + regmap_read(dev->i2s_regmap, BCM2835_I2S_CS_A_REG, &csreg); > + > + if (csreg & (BCM2835_I2S_TXON | BCM2835_I2S_RXON)) > + return 0; > + > + if (bcm2835_clk_freq[clk_src] % target_frequency == 0 > + && bit_master && frame_master) { > + divi = bcm2835_clk_freq[clk_src]/target_frequency; > + divf = 0; > + } else { > + uint64_t dividend; > + > + if (!dev->bclk_ratio) { > + /* > + * Overwrite bclk_ratio, because the > + * above trick is not needed or can > + * not be used. > + */ > + bclk_ratio = 2*data_length; = 2 * data_length; > + } > + > + target_frequency = sampling_rate*bclk_ratio; > + > + clk_src = BCM2835_CLK_SRC_PLLD; > + mash = BCM2835_CLK_MASH_1; > + > + dividend = bcm2835_clk_freq[clk_src]; > + dividend *= 1024; > + do_div(dividend, target_frequency); > + divi = dividend / 1024; > + divf = dividend % 1024; 1024 is a magic number? Could you use shifts and bitwise ANDs here? > + ch1pos = data_delay; > + ch2pos = bclk_ratio/2+data_delay; ' / 2 + ' [] > + mode |= BCM2835_I2S_FLEN(bclk_ratio-1); > + mode |= BCM2835_I2S_FSLEN(bclk_ratio/2); Ditto. Spaces are missed. [] > + case SND_SOC_DAIFMT_IB_IF: > + /* Both - therefore, none for BCM*/ 'M */' > + break; > + case SND_SOC_DAIFMT_NB_IF: > + /* > + * Invert only frame sync - therefore, > + * Invert only bit clock for BCM Something wrong with casing in the commentary. > + */ > + mode |= BCM2835_I2S_CLKI; > + break; > + case SND_SOC_DAIFMT_IB_NF: > + /* > + * Invert only bit clock - therefore, > + * Invert only frame sync for BCM Ditto. > + */ > + mode |= BCM2835_I2S_FSI; > + break; > + default: > + return -EINVAL; > + } > + > + regmap_write(dev->i2s_regmap, BCM2835_I2S_MODE_A_REG, mode); > + > + Redundant empty line. [] > + > + return 0; > +} > + > + Ditto. [] > +static int bcm2835_i2s_startup(struct snd_pcm_substream *substream, > + struct snd_soc_dai *dai) > +{ > + struct bcm2835_i2s_dev *dev = snd_soc_dai_get_drvdata(dai); > + > + if (!dai->active) { > + /* Should this still be running stop it */ > + bcm2835_i2s_stop_clock(dev); > + > + /* Enable PCM block */ > + regmap_update_bits(dev->i2s_regmap, BCM2835_I2S_CS_A_REG, > + BCM2835_I2S_EN, BCM2835_I2S_EN); > + > + /* > + * Disable STBY Dot is missing? > + * Requires at least 4 PCM clock cycles to take effect > + */ > + regmap_update_bits(dev->i2s_regmap, BCM2835_I2S_CS_A_REG, > + BCM2835_I2S_STBY, BCM2835_I2S_STBY); > + } > + > + return 0; > +} > +static void bcm2835_i2s_shutdown(struct snd_pcm_substream *substream, > + struct snd_soc_dai *dai) > +{ > + struct bcm2835_i2s_dev *dev = snd_soc_dai_get_drvdata(dai); > + > + bcm2835_i2s_stop(dev, substream, dai); > + > + /* If both streams are stopped, disable module and clock */ > + if (!dai->active) { if (dai->active) return; > + /* Disable the module */ > + regmap_update_bits(dev->i2s_regmap, BCM2835_I2S_CS_A_REG, > + BCM2835_I2S_EN, 0); > + > + /* > + * Stopping clock is necessary, because stop does > + * not stop the clock when SND_SOC_DAIFMT_CONT > + */ > + bcm2835_i2s_stop_clock(dev); > + } > +} > + [] > +static int bcm2835_i2s_probe(struct platform_device *pdev) > +{ > + struct bcm2835_i2s_dev *dev; > + int i; > + int ret; > + struct regmap *regmap[2]; > + struct resource *mem[2]; > + > + /* Request both ioareas */ > + for (i = 0; i <= 1; i++) { > + void __iomem *base; > + > + mem[i] = platform_get_resource(pdev, IORESOURCE_MEM, i); > + if (!mem[i]) { > + dev_err(&pdev->dev, "I2S probe: Memory resource could not be found.\n"); > + return -ENODEV; > + } Unneeded messaging and check. > + > + base = devm_ioremap_resource(&pdev->dev, mem[i]); > + if (!base) { It returns ERR_PTR. Thus, if (IS_ERR(base) return PTR_ERR(base) > + dev_err(&pdev->dev, "I2S probe: ioremap failed.\n"); Unneeded messaging. > + return -ENOMEM; Wrong error code. See above. -- Andy Shevchenko Intel Finland Oy --------------------------------------------------------------------- Intel Finland Oy Registered Address: PL 281, 00181 Helsinki Business Identity Code: 0357606 - 4 Domiciled in Helsinki This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751562Ab3KRMI4 (ORCPT ); Mon, 18 Nov 2013 07:08:56 -0500 Received: from mga03.intel.com ([143.182.124.21]:21327 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751057Ab3KRMIs (ORCPT ); Mon, 18 Nov 2013 07:08:48 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.93,723,1378882800"; d="scan'208";a="429557974" From: "Shevchenko, Andriy" To: Florian Meier CC: Liam Girdwood , Mark Brown , Stephen Warren , Lars-Peter Clausen , "alsa-devel@alsa-project.org" , linux-rpi-kernel , devicetree , dmaengine , "linux-kernel@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" Subject: Re: [PATCHv3] ASoC: Add support for BCM2835 Thread-Topic: [PATCHv3] ASoC: Add support for BCM2835 Thread-Index: AQHO5FbgOGake2IIQEGiv7ZL6GqSeQ== Date: Mon, 18 Nov 2013 12:08:22 +0000 Message-ID: <1384776479.14845.208.camel@smile> References: <5289F9DF.8060406@koalo.de> In-Reply-To: <5289F9DF.8060406@koalo.de> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.237.72.173] Content-Type: text/plain; charset="utf-8" Content-ID: <6DB828C271FD3848B21FBB4ED8069004@intel.com> MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from base64 to 8bit by mail.home.local id rAIC96Yv026932 On Mon, 2013-11-18 at 12:28 +0100, Florian Meier wrote: > This driver adds support for digital audio (I2S) > for the BCM2835 SoC that is used by the > Raspberry Pi. External audio codecs can be > connected to the Raspberry Pi via P5 header. > > It relies on cyclic DMA engine support for BCM2835. Few comments below. [] > +static void bcm2835_i2s_stop_clock(struct bcm2835_i2s_dev *dev) > +{ > + uint32_t clkreg; > + int timeout = 1000; > + > + /* Stop clock */ > + regmap_update_bits(dev->clk_regmap, BCM2835_CLK_PCMCTL_REG, > + BCM2835_CLK_PASSWD_MASK | BCM2835_CLK_ENAB, > + BCM2835_CLK_PASSWD); > + > + /* Wait for the BUSY flag going down */ > + while (timeout > 0) { > + timeout--; while (--timeout) { > + > + regmap_read(dev->clk_regmap, BCM2835_CLK_PCMCTL_REG, &clkreg); > + if (!(clkreg & BCM2835_CLK_BUSY)) > + break; > + } > + > + if (timeout <= 0) { if (!timeout) { [] > +static void bcm2835_i2s_clear_fifos(struct bcm2835_i2s_dev *dev, > + bool tx, bool rx) > +{ [] > + /* Wait for the SYNC flag changing it's state */ > + while (timeout > 0) { > + timeout--; while (--timeout) { > + regmap_read(dev->i2s_regmap, BCM2835_I2S_CS_A_REG, &csreg); > + if ((csreg & BCM2835_I2S_SYNC) != syncval) > + break; > + } > + > + if (timeout <= 0) if (!timeout) [] > +static int bcm2835_i2s_hw_params(struct snd_pcm_substream *substream, > + struct snd_pcm_hw_params *params, > + struct snd_soc_dai *dai) > +{ > + struct bcm2835_i2s_dev *dev = snd_soc_dai_get_drvdata(dai); > + > + unsigned int sampling_rate = params_rate(params); > + unsigned int data_length, data_delay, bclk_ratio; > + unsigned int ch1pos, ch2pos, mode, format; > + unsigned int mash = BCM2835_CLK_MASH_1; > + unsigned int divi, divf, target_frequency; > + int clk_src = -1; > + unsigned int master = dev->fmt & SND_SOC_DAIFMT_MASTER_MASK; > + uint32_t bit_master = (master == SND_SOC_DAIFMT_CBS_CFS > + || master == SND_SOC_DAIFMT_CBS_CFM); bool ? > + > + uint32_t frame_master = (master == SND_SOC_DAIFMT_CBS_CFS So, if master == SND_SOC_DAIFMT_CBS_CFS both bit_master and frame_master will be true. Is it correct? > + || master == SND_SOC_DAIFMT_CBM_CFS); Ditto. > + uint32_t csreg; > + > + /* > + * If a stream is already enabled, > + * the registers are already set properly. > + */ > + regmap_read(dev->i2s_regmap, BCM2835_I2S_CS_A_REG, &csreg); > + > + if (csreg & (BCM2835_I2S_TXON | BCM2835_I2S_RXON)) > + return 0; > + > + if (bcm2835_clk_freq[clk_src] % target_frequency == 0 > + && bit_master && frame_master) { > + divi = bcm2835_clk_freq[clk_src]/target_frequency; > + divf = 0; > + } else { > + uint64_t dividend; > + > + if (!dev->bclk_ratio) { > + /* > + * Overwrite bclk_ratio, because the > + * above trick is not needed or can > + * not be used. > + */ > + bclk_ratio = 2*data_length; = 2 * data_length; > + } > + > + target_frequency = sampling_rate*bclk_ratio; > + > + clk_src = BCM2835_CLK_SRC_PLLD; > + mash = BCM2835_CLK_MASH_1; > + > + dividend = bcm2835_clk_freq[clk_src]; > + dividend *= 1024; > + do_div(dividend, target_frequency); > + divi = dividend / 1024; > + divf = dividend % 1024; 1024 is a magic number? Could you use shifts and bitwise ANDs here? > + ch1pos = data_delay; > + ch2pos = bclk_ratio/2+data_delay; ' / 2 + ' [] > + mode |= BCM2835_I2S_FLEN(bclk_ratio-1); > + mode |= BCM2835_I2S_FSLEN(bclk_ratio/2); Ditto. Spaces are missed. [] > + case SND_SOC_DAIFMT_IB_IF: > + /* Both - therefore, none for BCM*/ 'M */' > + break; > + case SND_SOC_DAIFMT_NB_IF: > + /* > + * Invert only frame sync - therefore, > + * Invert only bit clock for BCM Something wrong with casing in the commentary. > + */ > + mode |= BCM2835_I2S_CLKI; > + break; > + case SND_SOC_DAIFMT_IB_NF: > + /* > + * Invert only bit clock - therefore, > + * Invert only frame sync for BCM Ditto. > + */ > + mode |= BCM2835_I2S_FSI; > + break; > + default: > + return -EINVAL; > + } > + > + regmap_write(dev->i2s_regmap, BCM2835_I2S_MODE_A_REG, mode); > + > + Redundant empty line. [] > + > + return 0; > +} > + > + Ditto. [] > +static int bcm2835_i2s_startup(struct snd_pcm_substream *substream, > + struct snd_soc_dai *dai) > +{ > + struct bcm2835_i2s_dev *dev = snd_soc_dai_get_drvdata(dai); > + > + if (!dai->active) { > + /* Should this still be running stop it */ > + bcm2835_i2s_stop_clock(dev); > + > + /* Enable PCM block */ > + regmap_update_bits(dev->i2s_regmap, BCM2835_I2S_CS_A_REG, > + BCM2835_I2S_EN, BCM2835_I2S_EN); > + > + /* > + * Disable STBY Dot is missing? > + * Requires at least 4 PCM clock cycles to take effect > + */ > + regmap_update_bits(dev->i2s_regmap, BCM2835_I2S_CS_A_REG, > + BCM2835_I2S_STBY, BCM2835_I2S_STBY); > + } > + > + return 0; > +} > +static void bcm2835_i2s_shutdown(struct snd_pcm_substream *substream, > + struct snd_soc_dai *dai) > +{ > + struct bcm2835_i2s_dev *dev = snd_soc_dai_get_drvdata(dai); > + > + bcm2835_i2s_stop(dev, substream, dai); > + > + /* If both streams are stopped, disable module and clock */ > + if (!dai->active) { if (dai->active) return; > + /* Disable the module */ > + regmap_update_bits(dev->i2s_regmap, BCM2835_I2S_CS_A_REG, > + BCM2835_I2S_EN, 0); > + > + /* > + * Stopping clock is necessary, because stop does > + * not stop the clock when SND_SOC_DAIFMT_CONT > + */ > + bcm2835_i2s_stop_clock(dev); > + } > +} > + [] > +static int bcm2835_i2s_probe(struct platform_device *pdev) > +{ > + struct bcm2835_i2s_dev *dev; > + int i; > + int ret; > + struct regmap *regmap[2]; > + struct resource *mem[2]; > + > + /* Request both ioareas */ > + for (i = 0; i <= 1; i++) { > + void __iomem *base; > + > + mem[i] = platform_get_resource(pdev, IORESOURCE_MEM, i); > + if (!mem[i]) { > + dev_err(&pdev->dev, "I2S probe: Memory resource could not be found.\n"); > + return -ENODEV; > + } Unneeded messaging and check. > + > + base = devm_ioremap_resource(&pdev->dev, mem[i]); > + if (!base) { It returns ERR_PTR. Thus, if (IS_ERR(base) return PTR_ERR(base) > + dev_err(&pdev->dev, "I2S probe: ioremap failed.\n"); Unneeded messaging. > + return -ENOMEM; Wrong error code. See above. -- Andy Shevchenko Intel Finland Oy --------------------------------------------------------------------- Intel Finland Oy Registered Address: PL 281, 00181 Helsinki Business Identity Code: 0357606 - 4 Domiciled in Helsinki This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. {.n++%ݶw{.n+{G{ayʇڙ,jfhz_(階ݢj"mG?&~iOzv^m ?I