From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andy Yan Subject: Re: [PATCH v13 07/12] drm: bridge/dw_hdmi: add support for multi-byte register width access Date: Fri, 28 Nov 2014 17:43:21 +0800 Message-ID: <547843B9.6000806@rock-chips.com> References: <1417008157-31861-1-git-send-email-andy.yan@rock-chips.com> <1417008759-32257-1-git-send-email-andy.yan@rock-chips.com> <1417019657.3177.10.camel@pengutronix.de> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; Format="flowed" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <1417019657.3177.10.camel@pengutronix.de> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: driverdev-devel-bounces@linuxdriverproject.org Sender: "devel" To: Philipp Zabel Cc: Mark Rutland , heiko@sntech.de, airlied@linux.ie, dri-devel@lists.freedesktop.org, ykk@rock-chips.com, devel@driverdev.osuosl.org, Pawel Moll , linux-rockchip@lists.infradead.org, Grant Likely , Dave Airlie , jay.xu@rock-chips.com, devicetree@vger.kernel.org, Zubair.Kakakhel@imgtec.com, Arnd Bergmann , Ian Campbell , Inki Dae , Rob Herring , Sean Paul , rmk+kernel@arm.linux.org.uk, mark.yao@rock-chips.com, fabio.estevam@freescale.com, Josh Boyer , Greg Kroah-Hartman , linux-kernel@vger.kernel.org, djkurtz@google.com, Kumar Gala , Shawn Guo , Lucas Stach List-Id: devicetree@vger.kernel.org SGkgWmFiZWw6Ck9uIDIwMTTlubQxMeaciDI35pelIDAwOjM0LCBQaGlsaXBwIFphYmVsIHdyb3Rl Ogo+IEFtIE1pdHR3b2NoLCBkZW4gMjYuMTEuMjAxNCwgMjE6MzIgKzA4MDAgc2NocmllYiBBbmR5 IFlhbjoKPj4gT24gcm9ja2NoaXAgcmszMjg4LCBvbmx5IHdvcmQoMzItYml0KSBhY2Nlc3NlcyBh cmUKPj4gcGVybWl0dGVkIGZvciBoZG1pIHJlZ2lzdGVycy4gIEJ5dGUgd2lkdGggYWNjZXNzZXMg KHdyaXRlYiwKPj4gcmVhZGIpIGdlbmVyYXRlIGFuIGltcHJlY2lzZSBleHRlcm5hbCBhYm9ydC4K Pj4KPj4gU2lnbmVkLW9mZi1ieTogQW5keSBZYW4gPGFuZHkueWFuQHJvY2stY2hpcHMuY29tPgo+ Pgo+PiAtLS0KPj4KPj4gQ2hhbmdlcyBpbiB2MTM6IE5vbmUKPj4gQ2hhbmdlcyBpbiB2MTI6IE5v bmUKPj4gQ2hhbmdlcyBpbiB2MTE6IE5vbmUKPj4gQ2hhbmdlcyBpbiB2MTA6IE5vbmUKPj4gQ2hh bmdlcyBpbiB2OTogTm9uZQo+PiBDaGFuZ2VzIGluIHY4OiBOb25lCj4+IENoYW5nZXMgaW4gdjc6 IE5vbmUKPj4gQ2hhbmdlcyBpbiB2NjoKPj4gLSByZWZhY3RvciByZWdpc3RlciBhY2Nlc3Mgd2l0 aG91dCByZWdfc2hpZnQKPj4KPj4gQ2hhbmdlcyBpbiB2NToKPj4gLSByZWZhY3RvciByZWctaW8t d2lkdGgKPj4KPj4gQ2hhbmdlcyBpbiB2NDogTm9uZQo+PiBDaGFuZ2VzIGluIHYzOgo+PiAtIHNw bGl0IG11bHRpLXJlZ2lzdGVyIGFjY2VzcyB0byBvbmUgaW5kZXBlbnQgcGF0Y2gKPj4KPj4gICBk cml2ZXJzL2dwdS9kcm0vYnJpZGdlL2R3X2hkbWkuYyB8IDU3ICsrKysrKysrKysrKysrKysrKysr KysrKysrKysrKysrKysrLS0tLS0KPj4gICAxIGZpbGUgY2hhbmdlZCwgNTEgaW5zZXJ0aW9ucygr KSwgNiBkZWxldGlvbnMoLSkKPj4KPj4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvZ3B1L2RybS9icmlk Z2UvZHdfaGRtaS5jIGIvZHJpdmVycy9ncHUvZHJtL2JyaWRnZS9kd19oZG1pLmMKPj4gaW5kZXgg YTUzYmY2My4uNWU4OGM4ZCAxMDA2NDQKPj4gLS0tIGEvZHJpdmVycy9ncHUvZHJtL2JyaWRnZS9k d19oZG1pLmMKPj4gKysrIGIvZHJpdmVycy9ncHUvZHJtL2JyaWRnZS9kd19oZG1pLmMKPj4gQEAg LTEwMCw2ICsxMDAsMTEgQEAgc3RydWN0IGhkbWlfZGF0YV9pbmZvIHsKPj4gICAJc3RydWN0IGhk bWlfdm1vZGUgdmlkZW9fbW9kZTsKPj4gICB9Owo+PiAgIAo+PiArdW5pb24gZHdfcmVnX3B0ciB7 Cj4+ICsJdTMyIF9faW9tZW0gKnAzMjsKPj4gKwl1OCBfX2lvbWVtICpwODsKPj4gK307Cj4gSSBz ZWUgbm8gbmVlZCB0byBpbnRyb2R1Y2UgdGhpcy4gSnVzdCBleHBsaWNpdGx5IG11bHRpcGx5IHRo ZSBvZmZzZXQgaW4KPiBkd19oZG1pX3dyaXRlbC4KPgogICAgIElzIHRoZXJlIGFueSBkaXNhZHZh bnRhZ2UgdG8gZG8gbGlrZSB0aGlzPwogICAgIFRoZSBjb21waWxlciBjYW4gaGVscCB1cyBkbyB0 aGUgZXhwbGljaXRseSBtdWx0aXBseSBieSB0aGlzIHdheS4KPj4gICBzdHJ1Y3QgZHdfaGRtaSB7 Cj4+ICAgCXN0cnVjdCBkcm1fY29ubmVjdG9yIGNvbm5lY3RvcjsKPj4gICAJc3RydWN0IGRybV9l bmNvZGVyICplbmNvZGVyOwo+PiBAQCAtMTIxLDIwICsxMjYsNDMgQEAgc3RydWN0IGR3X2hkbWkg ewo+PiAgIAo+PiAgIAlzdHJ1Y3QgcmVnbWFwICpyZWdtYXA7Cj4+ICAgCXN0cnVjdCBpMmNfYWRh cHRlciAqZGRjOwo+PiAtCXZvaWQgX19pb21lbSAqcmVnczsKPj4gKwl1bmlvbiBkd19yZWdfcHRy IHJlZ3M7Cj4gS2VlcCB0aGlzIGFzIHZvaWQgX19pb21lbSAqCj4KPj4gICAJdW5zaWduZWQgaW50 IHNhbXBsZV9yYXRlOwo+PiAgIAlpbnQgcmF0aW87Cj4+ICsKPj4gKwl2b2lkICgqd3JpdGUpKHN0 cnVjdCBkd19oZG1pICpoZG1pLCB1OCB2YWwsIGludCBvZmZzZXQpOwo+PiArCXU4ICgqcmVhZCko c3RydWN0IGR3X2hkbWkgKmhkbWksIGludCBvZmZzZXQpOwo+PiAgIH07Cj4+ICAgCj4+ICtzdGF0 aWMgdm9pZCBkd19oZG1pX3dyaXRlbChzdHJ1Y3QgZHdfaGRtaSAqaGRtaSwgdTggdmFsLCBpbnQg b2Zmc2V0KQo+PiArewo+PiArCXdyaXRlbCh2YWwsIGhkbWktPnJlZ3MucDMyICsgb2Zmc2V0KTsK PiBoZG1pLT5yZWdzICsgNCAqIG9mZnNldAo+Cj4+ICt9Cj4+ICsKPj4gK3N0YXRpYyB1OCBkd19o ZG1pX3JlYWRsKHN0cnVjdCBkd19oZG1pICpoZG1pLCBpbnQgb2Zmc2V0KQo+PiArewo+PiArCXJl dHVybiByZWFkbChoZG1pLT5yZWdzLnAzMiArIG9mZnNldCk7Cj4gc2FtZSBoZXJlCj4KPj4gK30K Pj4gKwo+PiArc3RhdGljIHZvaWQgZHdfaGRtaV93cml0ZWIoc3RydWN0IGR3X2hkbWkgKmhkbWks IHU4IHZhbCwgaW50IG9mZnNldCkKPj4gK3sKPj4gKwl3cml0ZWIodmFsLCBoZG1pLT5yZWdzLnA4 ICsgb2Zmc2V0KTsKPj4gK30KPj4gKwo+PiArc3RhdGljIHU4IGR3X2hkbWlfcmVhZGIoc3RydWN0 IGR3X2hkbWkgKmhkbWksIGludCBvZmZzZXQpCj4+ICt7Cj4+ICsJcmV0dXJuIHJlYWRiKGhkbWkt PnJlZ3MucDggKyBvZmZzZXQpOwo+PiArfQo+PiArCj4+ICAgc3RhdGljIGlubGluZSB2b2lkIGhk bWlfd3JpdGViKHN0cnVjdCBkd19oZG1pICpoZG1pLCB1OCB2YWwsIGludCBvZmZzZXQpCj4+ICAg ewo+PiAtCXdyaXRlYih2YWwsIGhkbWktPnJlZ3MgKyBvZmZzZXQpOwo+PiArCWhkbWktPndyaXRl KGhkbWksIHZhbCwgb2Zmc2V0KTsKPj4gICB9Cj4+ICAgCj4+ICAgc3RhdGljIGlubGluZSB1OCBo ZG1pX3JlYWRiKHN0cnVjdCBkd19oZG1pICpoZG1pLCBpbnQgb2Zmc2V0KQo+PiAgIHsKPj4gLQly ZXR1cm4gcmVhZGIoaGRtaS0+cmVncyArIG9mZnNldCk7Cj4+ICsJcmV0dXJuIGhkbWktPnJlYWQo aGRtaSwgb2Zmc2V0KTsKPj4gICB9Cj4+Cj4+ICAgc3RhdGljIHZvaWQgaGRtaV9tb2RiKHN0cnVj dCBkd19oZG1pICpoZG1pLCB1OCBkYXRhLCB1OCBtYXNrLCB1bnNpZ25lZCByZWcpCj4+IEBAIC0x NTA4LDYgKzE1MzYsNyBAQCBpbnQgZHdfaGRtaV9iaW5kKHN0cnVjdCBkZXZpY2UgKmRldiwgc3Ry dWN0IGRldmljZSAqbWFzdGVyLAo+PiAgIAlzdHJ1Y3QgZHdfaGRtaSAqaGRtaTsKPj4gICAJc3Ry dWN0IHJlc291cmNlICppb3JlczsKPj4gICAJaW50IHJldCwgaXJxOwo+PiArCXUzMiB2YWwgPSAx Owo+PiAgIAo+PiAgIAloZG1pID0gZGV2bV9remFsbG9jKCZwZGV2LT5kZXYsIHNpemVvZigqaGRt aSksIEdGUF9LRVJORUwpOwo+PiAgIAlpZiAoIWhkbWkpCj4+IEBAIC0xNTIwLDYgKzE1NDksMjIg QEAgaW50IGR3X2hkbWlfYmluZChzdHJ1Y3QgZGV2aWNlICpkZXYsIHN0cnVjdCBkZXZpY2UgKm1h c3RlciwKPj4gICAJaGRtaS0+cmF0aW8gPSAxMDA7Cj4+ICAgCWhkbWktPmVuY29kZXIgPSBlbmNv ZGVyOwo+PiAgIAo+PiArCW9mX3Byb3BlcnR5X3JlYWRfdTMyKG5wLCAicmVnLWlvLXdpZHRoIiwg JnZhbCk7Cj4+ICsKPj4gKwlzd2l0Y2ggKHZhbCkgewo+PiArCWNhc2UgNDoKPj4gKwkJaGRtaS0+ d3JpdGUgPSBkd19oZG1pX3dyaXRlbDsKPj4gKwkJaGRtaS0+cmVhZCA9IGR3X2hkbWlfcmVhZGw7 Cj4+ICsJCWJyZWFrOwo+PiArCWNhc2UgMToKPj4gKwkJaGRtaS0+d3JpdGUgPSBkd19oZG1pX3dy aXRlYjsKPj4gKwkJaGRtaS0+cmVhZCA9IGR3X2hkbWlfcmVhZGI7Cj4+ICsJCWJyZWFrOwo+PiAr CWRlZmF1bHQ6Cj4+ICsJCWRldl9lcnIoZGV2LCAicmVnLWlvLXdpZHRoIG11c3QgYmUgMSBvciA0 XG4iKTsKPj4gKwkJcmV0dXJuIC1FSU5WQUw7Cj4+ICsJfQo+PiArCj4+ICAgCWRkY19ub2RlID0g b2ZfcGFyc2VfcGhhbmRsZShucCwgImRkYy1pMmMtYnVzIiwgMCk7Cj4+ICAgCWlmIChkZGNfbm9k ZSkgewo+PiAgIAkJaGRtaS0+ZGRjID0gb2ZfZmluZF9pMmNfYWRhcHRlcl9ieV9ub2RlKGRkY19u b2RlKTsKPj4gQEAgLTE1NDQsOSArMTU4OSw5IEBAIGludCBkd19oZG1pX2JpbmQoc3RydWN0IGRl dmljZSAqZGV2LCBzdHJ1Y3QgZGV2aWNlICptYXN0ZXIsCj4+ICAgCQlyZXR1cm4gcmV0Owo+PiAg IAo+PiAgIAlpb3JlcyA9IHBsYXRmb3JtX2dldF9yZXNvdXJjZShwZGV2LCBJT1JFU09VUkNFX01F TSwgMCk7Cj4+IC0JaGRtaS0+cmVncyA9IGRldm1faW9yZW1hcF9yZXNvdXJjZShkZXYsIGlvcmVz KTsKPj4gLQlpZiAoSVNfRVJSKGhkbWktPnJlZ3MpKQo+PiAtCQlyZXR1cm4gUFRSX0VSUihoZG1p LT5yZWdzKTsKPj4gKwloZG1pLT5yZWdzLnAzMiA9IGRldm1faW9yZW1hcF9yZXNvdXJjZShkZXYs IGlvcmVzKTsKPj4gKwlpZiAoSVNfRVJSKGhkbWktPnJlZ3MucDMyKSkKPj4gKwkJcmV0dXJuIFBU Ul9FUlIoaGRtaS0+cmVncy5wMzIpOwo+PiAgIAo+PiAgIAkvKiBQcm9kdWN0IGFuZCByZXZpc2lv biBJRHMgKi8KPj4gICAJZGV2X2luZm8oZGV2LAo+IHJlZ2FyZHMKPiBQaGlsaXBwCj4KPgo+Cj4K Cl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmRldmVsIG1h aWxpbmcgbGlzdApkZXZlbEBsaW51eGRyaXZlcnByb2plY3Qub3JnCmh0dHA6Ly9kcml2ZXJkZXYu bGludXhkcml2ZXJwcm9qZWN0Lm9yZy9tYWlsbWFuL2xpc3RpbmZvL2RyaXZlcmRldi1kZXZlbAo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751502AbaK1Jnu (ORCPT ); Fri, 28 Nov 2014 04:43:50 -0500 Received: from va-smtp01.263.net ([54.88.144.211]:36575 "EHLO va-smtp01.263.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751041AbaK1Jns (ORCPT ); Fri, 28 Nov 2014 04:43:48 -0500 X-RL-SENDER: andy.yan@rock-chips.com X-FST-TO: galak@codeaurora.org X-SENDER-IP: 121.15.173.1 X-LOGIN-NAME: andy.yan@rock-chips.com X-UNIQUE-TAG: <78039072c1c0e2c457b22e0bcfc51de2> X-ATTACHMENT-NUM: 0 X-DNS-TYPE: 0 Message-ID: <547843B9.6000806@rock-chips.com> Date: Fri, 28 Nov 2014 17:43:21 +0800 From: Andy Yan User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.0 MIME-Version: 1.0 To: Philipp Zabel CC: airlied@linux.ie, heiko@sntech.de, fabio.estevam@freescale.com, rmk+kernel@arm.linux.org.uk, Greg Kroah-Hartman , Grant Likely , Rob Herring , Shawn Guo , Josh Boyer , Sean Paul , Inki Dae , Dave Airlie , Arnd Bergmann , Lucas Stach , Zubair.Kakakhel@imgtec.com, djkurtz@google.com, ykk@rock-chips.com, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, devel@driverdev.osuosl.org, devicetree@vger.kernel.org, linux-rockchip@lists.infradead.org, jay.xu@rock-chips.com, Pawel Moll , mark.yao@rock-chips.com, Mark Rutland , Ian Campbell , Kumar Gala Subject: Re: [PATCH v13 07/12] drm: bridge/dw_hdmi: add support for multi-byte register width access References: <1417008157-31861-1-git-send-email-andy.yan@rock-chips.com> <1417008759-32257-1-git-send-email-andy.yan@rock-chips.com> <1417019657.3177.10.camel@pengutronix.de> In-Reply-To: <1417019657.3177.10.camel@pengutronix.de> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Zabel: On 2014年11月27日 00:34, Philipp Zabel wrote: > Am Mittwoch, den 26.11.2014, 21:32 +0800 schrieb Andy Yan: >> On rockchip rk3288, only word(32-bit) accesses are >> permitted for hdmi registers. Byte width accesses (writeb, >> readb) generate an imprecise external abort. >> >> Signed-off-by: Andy Yan >> >> --- >> >> Changes in v13: None >> Changes in v12: None >> Changes in v11: None >> Changes in v10: None >> Changes in v9: None >> Changes in v8: None >> Changes in v7: None >> Changes in v6: >> - refactor register access without reg_shift >> >> Changes in v5: >> - refactor reg-io-width >> >> Changes in v4: None >> Changes in v3: >> - split multi-register access to one indepent patch >> >> drivers/gpu/drm/bridge/dw_hdmi.c | 57 +++++++++++++++++++++++++++++++++++----- >> 1 file changed, 51 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/gpu/drm/bridge/dw_hdmi.c b/drivers/gpu/drm/bridge/dw_hdmi.c >> index a53bf63..5e88c8d 100644 >> --- a/drivers/gpu/drm/bridge/dw_hdmi.c >> +++ b/drivers/gpu/drm/bridge/dw_hdmi.c >> @@ -100,6 +100,11 @@ struct hdmi_data_info { >> struct hdmi_vmode video_mode; >> }; >> >> +union dw_reg_ptr { >> + u32 __iomem *p32; >> + u8 __iomem *p8; >> +}; > I see no need to introduce this. Just explicitly multiply the offset in > dw_hdmi_writel. > Is there any disadvantage to do like this? The compiler can help us do the explicitly multiply by this way. >> struct dw_hdmi { >> struct drm_connector connector; >> struct drm_encoder *encoder; >> @@ -121,20 +126,43 @@ struct dw_hdmi { >> >> struct regmap *regmap; >> struct i2c_adapter *ddc; >> - void __iomem *regs; >> + union dw_reg_ptr regs; > Keep this as void __iomem * > >> unsigned int sample_rate; >> int ratio; >> + >> + void (*write)(struct dw_hdmi *hdmi, u8 val, int offset); >> + u8 (*read)(struct dw_hdmi *hdmi, int offset); >> }; >> >> +static void dw_hdmi_writel(struct dw_hdmi *hdmi, u8 val, int offset) >> +{ >> + writel(val, hdmi->regs.p32 + offset); > hdmi->regs + 4 * offset > >> +} >> + >> +static u8 dw_hdmi_readl(struct dw_hdmi *hdmi, int offset) >> +{ >> + return readl(hdmi->regs.p32 + offset); > same here > >> +} >> + >> +static void dw_hdmi_writeb(struct dw_hdmi *hdmi, u8 val, int offset) >> +{ >> + writeb(val, hdmi->regs.p8 + offset); >> +} >> + >> +static u8 dw_hdmi_readb(struct dw_hdmi *hdmi, int offset) >> +{ >> + return readb(hdmi->regs.p8 + offset); >> +} >> + >> static inline void hdmi_writeb(struct dw_hdmi *hdmi, u8 val, int offset) >> { >> - writeb(val, hdmi->regs + offset); >> + hdmi->write(hdmi, val, offset); >> } >> >> static inline u8 hdmi_readb(struct dw_hdmi *hdmi, int offset) >> { >> - return readb(hdmi->regs + offset); >> + return hdmi->read(hdmi, offset); >> } >> >> static void hdmi_modb(struct dw_hdmi *hdmi, u8 data, u8 mask, unsigned reg) >> @@ -1508,6 +1536,7 @@ int dw_hdmi_bind(struct device *dev, struct device *master, >> struct dw_hdmi *hdmi; >> struct resource *iores; >> int ret, irq; >> + u32 val = 1; >> >> hdmi = devm_kzalloc(&pdev->dev, sizeof(*hdmi), GFP_KERNEL); >> if (!hdmi) >> @@ -1520,6 +1549,22 @@ int dw_hdmi_bind(struct device *dev, struct device *master, >> hdmi->ratio = 100; >> hdmi->encoder = encoder; >> >> + of_property_read_u32(np, "reg-io-width", &val); >> + >> + switch (val) { >> + case 4: >> + hdmi->write = dw_hdmi_writel; >> + hdmi->read = dw_hdmi_readl; >> + break; >> + case 1: >> + hdmi->write = dw_hdmi_writeb; >> + hdmi->read = dw_hdmi_readb; >> + break; >> + default: >> + dev_err(dev, "reg-io-width must be 1 or 4\n"); >> + return -EINVAL; >> + } >> + >> ddc_node = of_parse_phandle(np, "ddc-i2c-bus", 0); >> if (ddc_node) { >> hdmi->ddc = of_find_i2c_adapter_by_node(ddc_node); >> @@ -1544,9 +1589,9 @@ int dw_hdmi_bind(struct device *dev, struct device *master, >> return ret; >> >> iores = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> - hdmi->regs = devm_ioremap_resource(dev, iores); >> - if (IS_ERR(hdmi->regs)) >> - return PTR_ERR(hdmi->regs); >> + hdmi->regs.p32 = devm_ioremap_resource(dev, iores); >> + if (IS_ERR(hdmi->regs.p32)) >> + return PTR_ERR(hdmi->regs.p32); >> >> /* Product and revision IDs */ >> dev_info(dev, > regards > Philipp > > > >