From mboxrd@z Thu Jan 1 00:00:00 1970 From: Heiko Stuebner Subject: Re: [PATCH v2 2/3] soc: rockchip: add driver handling grf setup Date: Thu, 17 Nov 2016 10:12:01 +0100 Message-ID: <3512052.5NJHIQHqMY@phil> References: <20161115223900.30728-1-heiko@sntech.de> <5037506.GEHFh4fruS@phil> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "Linux-rockchip" Errors-To: linux-rockchip-bounces+glpar-linux-rockchip=m.gmane.org-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org To: Shawn Lin Cc: olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org, linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, arnd-r2nGTMty4D4@public.gmane.org, dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org List-Id: linux-rockchip.vger.kernel.org QW0gRG9ubmVyc3RhZywgMTcuIE5vdmVtYmVyIDIwMTYsIDA5OjM4OjExIENFVCBzY2hyaWViIFNo YXduIExpbjoKPiBIaSBIZWlrbywKPiAKPiDlnKggMjAxNi8xMS8xNiAxNzo1OCwgSGVpa28gU3R1 ZWJuZXIg5YaZ6YGTOgo+ID4gSGkgU2hhd24sCj4gPiAKPiA+IEFtIE1pdHR3b2NoLCAxNi4gTm92 ZW1iZXIgMjAxNiwgMTc6MzM6MjEgQ0VUIHNjaHJpZWIgU2hhd24gTGluOgo+ID4+IOWcqCAyMDE2 LzExLzE2IDY6MzgsIEhlaWtvIFN0dWVibmVyIOWGmemBkzoKPiA+Pj4gVGhlIEdlbmVyYWwgUmVn aXN0ZXIgRmlsZXMgYXJlIGFuIGFyZWEgb2YgcmVnaXN0ZXJzIGNvbnRhaW5pbmcgYSBsb3QKPiA+ Pj4gb2Ygc2luZ2xlLWJpdCBzZXR0aW5ncyBmb3IgbnVtZXJvdXMgY29tcG9uZW50cyBhcyB3ZWxs IGZ1bGwgY29tcG9uZW50cwo+ID4+PiBsaWtlIHVzYnBoeSBjb250cm9sLiBUaGVyZWZvcmUgYWxs IHVzZWQgY29tcG9uZW50cyBhcmUgYWNjZXNzZWQKPiA+Pj4gdmlhIHRoZSBzeXNjb24gcHJvdmlk ZWQgYnkgdGhlIGdyZiBub2RlcyBvciBmcm9tIHRoZSBzdWItZGV2aWNlcwo+ID4+PiBjcmVhdGVk IHRocm91Z2ggdGhlIHNpbXBsZS1tZmQgY3JlYXRlZCBmcm9tIHRoZSBncmYgbm9kZS4KPiAKPiAt LS0tODwtLS0tLS0tLS0tLS0tLS0tCj4gWy4uLl0KPiAKPiA+Pj4gKwlmb3IgKGkgPSAwOyBpIDwg Z3JmX2luZm8tPm51bV92YWx1ZXM7IGkrKykgewo+ID4+PiArCQljb25zdCBzdHJ1Y3Qgcm9ja2No aXBfZ3JmX3ZhbHVlICp2YWwgPSAmZ3JmX2luZm8tPnZhbHVlc1tpXTsKPiA+Pj4gKwo+ID4+PiAr CQlwcl9kZWJ1ZygiJXM6IGFkanVzdGluZyAlcyBpbiAlIzZ4IHRvICUjMTB4XG4iLCBfX2Z1bmNf XywKPiA+Pj4gKwkJCXZhbC0+ZGVzYywgdmFsLT5yZWcsIHZhbC0+dmFsKTsKPiA+Pj4gKwkJcmV0 ID0gcmVnbWFwX3dyaXRlKGdyZiwgdmFsLT5yZWcsIHZhbC0+dmFsKTsKPiA+Pj4gKwkJaWYgKHJl dCA8IDApCj4gPj4+ICsJCQlwcl9lcnIoIiVzOiB3cml0ZSB0byAlIzZ4IGZhaWxlZCB3aXRoICVk XG4iLAo+ID4+PiArCQkJICAgICAgIF9fZnVuY19fLCB2YWwtPnJlZywgcmV0KTsKPiA+PiAKPiA+ PiBTbywgd2hlbiBmYWlsaW5nIHRvIGRvIG9uZSBvZiB0aGUgc2V0dGluZ3MsIHNob3VsZCB3ZSBz dGlsbCBsZXQgaXQgZ29lcz8KPiA+PiBTb21ldGltZXMgdGhlIGxvZyBvZiBwb3N0Y29yZV9pbml0 Y2FsbCBpcyBlYXN5IHRvIGJlIG5lZ2xlY3RlZCB3aGVuCj4gPj4gcGVvcGxlIGZpbmFsbHkgZmlu ZCBwcm9ibGVtcyBsYXRlciBidXQgdGhlIHZlcnkgZWFybGllciBsb2cgd2FzIG1pc3NpbmcKPiA+ PiBkdWUgdG8gd2hhdGV2ZXIgcmVhc29uIGxpa2UgYnVmZmVyIGxpbWl0YXRpb24sIGV0Yy4KPiA+ IAo+ID4gSSBleHBlY3QgZXJyb3JzIGhlcmUgdG8gYmUgdmVyeSByYXJlLiBJLmUuIERvdWcgdGhv dWdodCB0aGF0IHJlZ21hcCBtaWdodAo+ID4gcmV0dXJuIGVycm9ycyBpZiB3ZSB3cml0ZSBvdXRz aWRlIHRoZSBtYXBwZWQgcmVnaW9uLCB3aGljaCB3b3VsZCBtZWFuCj4gPiBzb21lb25lIHJlYWxs eSBtZXNzZWQgdXAgaW4gdGhpcyBjb3JlIGNvbXBvbmVudCBvciBhIGNvcmUtZWxlbWVudCBvZiB0 aGUKPiA+IGR0cy4gQnV0IGluIGdlbmVyYWwgdGhlIEdSRiBpcyBhbHdheXMgYSBtbWlvLXJlZ21h cCwgc28gdGhlcmUgd29uJ3QgYmUKPiA+IGFueSBpMmMvc3BpLyB3aGF0ZXZlciBlcnJvcnMgcG9z c2libGUuCj4gCj4gSSB3YXMganVzdCB0aGlua2luZyBhYm91dCB0aGUgc2NhbGFiaWxpdHkgdGhh dCBpbiB0aGUgZnV0dXJlIHNvbWUgb2YgdGhlCj4gR1JGIHNldHRpbmdzIG1heSBkZXBlbmQgb24g Z2VucGQgb3IgY2xvY2sgZXZlbiBpZiB0aGV5IGFyZSBvbmx5IGEKPiBtbWlvLXJlZ21hcCBidXQg dGhleSBkb24ndCBiZWxvbmcgdG8gdGhlIGdlbmVyYWwgcGQvY2xvY2sgZm9yIEdSRiwgZm9yCj4g aW5zdGFuY2UsIHNvbWUgb2YgdGhlIFBIWXMnIG9yIGNvbnRyb2xsZXIncyBjYXAobGlrZSBlbW1j KSBzZXR0aW5ncy4KPiAKPiBCdXQsIElJUkMsIHlvdSBzdWdnZXN0ZWQgdGhpcyBkcml2ZXIgc2hv dWxkbid0IGJlIGEgY2F0Y2hhbGwsIGFuZCB3ZQo+IG5vdyBhc2sgdGhlIGRyaXZlcnMgb2YgY29u dHJvbGxlcnMgYW5kIHBoeXMgdG8gdGFrZSBvdmVyIHRoaXMsIHNvIEkKPiBndWVzcyB3ZSB3b24n dCBwdXQgdGhvc2Ugc2V0dGluZ3MgaGVyZSBldmVyLiA6KQo+IAo+IFRoYW5rcyBmb3IgZXhwbGFp bmluZyB0aGlzLgoKY29ycmVjdCwgaXQgc2hvdWxkIG5ldmVyIGJlY29tZSBhIGNhdGNoYWxsIGFz IHRoaW5ncyBsaWtlIHRob3NlIHBoeS1zdWJkZXZpY2VzIAp0aGF0IG5lZWQgcnVudGltZSBoYW5k bGluZyBzaG91bGQgZGVmaW5pdGx5IGJlIGhhbmRsZWQgaW4gdGhlaXIgcmVzcGVjdGl2ZSAKZHJp dmVycy4KClNvIGZhciB3ZSdyZSBhbHNvIGFsd2F5cyBzdGFydGluZyB3aXRoIGFsbCBjbG9ja3Mg YW5kIHBvd2VyLWRvbWFpbnMgYmVpbmcgb24sIApzbyBzZXR0aW5ncyBzaG91bGQgYWx3YXlzIHN1 Y2NlZWQgYXMgd2UncmUgb25seSBhY3RpdmUgZHVyaW5nIGVhcmx5IGJvb3QgaGVyZS4KV2UnbGwg c2ltcGx5IGhhdmUgdG8gcmVldmFsdWF0ZSBpZiBzb21lIG5ldyBzb2MgYmVnaW5zIHRvIGJvb3Qg d2l0aCBzb21lIApuZWVkZWQgY2xvY2tzL3Bvd2VyLWRvbWFpbnMgYmVpbmcgb2ZmLgoKCkhlaWtv CgpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpMaW51eC1y b2NrY2hpcCBtYWlsaW5nIGxpc3QKTGludXgtcm9ja2NoaXBAbGlzdHMuaW5mcmFkZWFkLm9yZwpo dHRwOi8vbGlzdHMuaW5mcmFkZWFkLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2xpbnV4LXJvY2tjaGlw Cg== From mboxrd@z Thu Jan 1 00:00:00 1970 From: heiko@sntech.de (Heiko Stuebner) Date: Thu, 17 Nov 2016 10:12:01 +0100 Subject: [PATCH v2 2/3] soc: rockchip: add driver handling grf setup In-Reply-To: References: <20161115223900.30728-1-heiko@sntech.de> <5037506.GEHFh4fruS@phil> Message-ID: <3512052.5NJHIQHqMY@phil> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Am Donnerstag, 17. November 2016, 09:38:11 CET schrieb Shawn Lin: > Hi Heiko, > > ? 2016/11/16 17:58, Heiko Stuebner ??: > > Hi Shawn, > > > > Am Mittwoch, 16. November 2016, 17:33:21 CET schrieb Shawn Lin: > >> ? 2016/11/16 6:38, Heiko Stuebner ??: > >>> The General Register Files are an area of registers containing a lot > >>> of single-bit settings for numerous components as well full components > >>> like usbphy control. Therefore all used components are accessed > >>> via the syscon provided by the grf nodes or from the sub-devices > >>> created through the simple-mfd created from the grf node. > > ----8<---------------- > [...] > > >>> + for (i = 0; i < grf_info->num_values; i++) { > >>> + const struct rockchip_grf_value *val = &grf_info->values[i]; > >>> + > >>> + pr_debug("%s: adjusting %s in %#6x to %#10x\n", __func__, > >>> + val->desc, val->reg, val->val); > >>> + ret = regmap_write(grf, val->reg, val->val); > >>> + if (ret < 0) > >>> + pr_err("%s: write to %#6x failed with %d\n", > >>> + __func__, val->reg, ret); > >> > >> So, when failing to do one of the settings, should we still let it goes? > >> Sometimes the log of postcore_initcall is easy to be neglected when > >> people finally find problems later but the very earlier log was missing > >> due to whatever reason like buffer limitation, etc. > > > > I expect errors here to be very rare. I.e. Doug thought that regmap might > > return errors if we write outside the mapped region, which would mean > > someone really messed up in this core component or a core-element of the > > dts. But in general the GRF is always a mmio-regmap, so there won't be > > any i2c/spi/ whatever errors possible. > > I was just thinking about the scalability that in the future some of the > GRF settings may depend on genpd or clock even if they are only a > mmio-regmap but they don't belong to the general pd/clock for GRF, for > instance, some of the PHYs' or controller's cap(like emmc) settings. > > But, IIRC, you suggested this driver shouldn't be a catchall, and we > now ask the drivers of controllers and phys to take over this, so I > guess we won't put those settings here ever. :) > > Thanks for explaining this. correct, it should never become a catchall as things like those phy-subdevices that need runtime handling should definitly be handled in their respective drivers. So far we're also always starting with all clocks and power-domains being on, so settings should always succeed as we're only active during early boot here. We'll simply have to reevaluate if some new soc begins to boot with some needed clocks/power-domains being off. Heiko