From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andy Yan Subject: Re: [PATCH V2 1/2] imx-drm: imx-hdmi: split imx soc specific code from imx-hdmi Date: Fri, 07 Nov 2014 10:56:49 +0800 Message-ID: <545C34F1.5020206@rock-chips.com> References: <1415192385-6572-1-git-send-email-andy.yan@rock-chips.com> <545A2680.2@imgtec.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; Format="flowed" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <545A2680.2@imgtec.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: driverdev-devel-bounces@linuxdriverproject.org Sender: "devel" To: Zubair Lutfullah Kakakhel , airlied@linux.ie, heiko@sntech.de, fabio.estevam@freescale.com, rmk+kernel@arm.linux.org.uk Cc: devel@driverdev.osuosl.org, devicetree@vger.kernel.org, Arnd Bergmann , Josh Boyer , Greg Kroah-Hartman , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, Inki Dae , linux-rockchip@lists.infradead.org, Rob Herring , Sean Paul , djkurtz@google.com, Philipp Zabel , ykk@rock-chips.com, Grant Likely , Dave Airlie , Shawn Guo , Lucas Stach List-Id: devicetree@vger.kernel.org Ck9uIDIwMTTlubQxMeaciDA15pelIDIxOjMwLCBadWJhaXIgTHV0ZnVsbGFoIEtha2FraGVsIHdy b3RlOgo+IFRoaXMgb25lIHBhdGNoIGRvZXMgdG9vIG11Y2ggdG8gYmUgcmV2aWV3ZWQgZWFzaWx5 Lgo+Cj4gT25lIHBhdGNoIGlzIHN1cHBvc2VkIHRvIG1vZGlmeS9hZGQgb25lIHRoaW5nIGF0IGEg dGltZSBpbiB0aGUga2VybmVsLgo+Cj4gU2VwYXJhdGluZyBwbGF0Zm9ybSBzcGVjaWZpYyBjb2Rl IGZyb20gaW14LWRybS9pbXgtaGRtaSBpcyBvbmUgdGhpbmcuCj4KPiBBZGRpbmcgc3VwcG9ydCBm b3IgbXVsdGktYnl0ZSByZWdpc3RlciBhY2Nlc3MgaXMgc29tZXRoaW5nIGRpZmZlcmVudC4KPgo+ IGkuZS4gU29tZXRoaW5nIGxpa2UuCj4gMS8zIHNwbGl0IHBsYXRmb3JtIHNwZWNpZmljIGNvZGUg b3V0Lgo+IDIvMyBtb3ZlL3JlbmFtZSBpbXgtaGRtaSBvdXRzaWRlIHRoZSBmb2xkZXIKPiAzLzMg YWRkIHN1cHBvcnQgZm9yIG11bHRpIGJ5dGUgcmVnaXN0ZXIgd2lkdGggYWNjZXNzLgo+Cj4gSWYg dGhlcmUgYXJlIG90aGVyIHRoaW5ncyB0aGF0IGFyZSBub3QgZGlyZWN0bHkgcmVsZXZhbnQgdG8g dGhlIHBhdGNoLAo+IGl0IGdvZXMgaW4gYSBkaWZmZXJlbnQgcGF0Y2guIEJ1ZyBmaXhlcyBhcmUg YWxzbyBzZXBhcmF0ZS4KPgo+IFRoaXMgc2hvdWxkIHJlc3VsdCBpbiByZWFkYWJsZSBwYXRjaGVz IHRoYXQgY2FuIGJlIHJldmlld2VkIGVhc2lseS4KSSBoYXZlIHNwbGl0IHRoZSBwYXRjaCBpbiB0 aHJlZSBwYXJ0cyBpbiBQQVRDSCBWMywgdGthbmtzIGZvciB5b3VyIApzdWdnZXN0aW9uCj4KPiBB bHNvLCB0aGUgYXBwcm9hY2ggd2l0aCA0IGJ5dGUgYWNjZXNzIGlzIG9rLiBCdXQgeW91IGNvdWxk IHVzZSByZWctc2hpZnRzIGFzIHdlbGwgcGVyaGFwcy4KPiBUaGVuIHlvdSB3b24ndCBoYXZlIHRv IGNoYW5nZSBzbyBtdWNoIG9mIHRoZSBjb2RlLgo+Cj4gc3RhdGljIGlubGluZSB2b2lkIGhkbWlf d3JpdGViKHN0cnVjdCBkd2NfaGRtaSAqaGRtaSwgdTggdmFsLCBpbnQgb2Zmc2V0KQo+ICt7Cj4g KyB3cml0ZWIodmFsLCBoZG1pLT5yZWdzICsgKG9mZnNldCA8PCBoZG1pLT5yZWdfc2hpZnQpKTsK PiArfQo+ICsKPiArc3RhdGljIGlubGluZSB1OCBoZG1pX3JlYWRiKHN0cnVjdCBkd2NfaGRtaSAq aGRtaSwgaW50IG9mZnNldCkKPiArewo+ICsgcmV0dXJuIHJlYWRiKGhkbWktPnJlZ3MgKyAob2Zm c2V0IDw8IGhkbWktPnJlZ19zaGlmdCkpOwo+ICt9Cj4KPiBBbmQgdGhlbiBpbiBwcm9iZQo+Cj4g K2hkbWktPnJlZ19zaGlmdCA9IDA7Cj4gKwo+ICsgaWYgKG9mX3Byb3BlcnR5X3JlYWRfdTMyKG5w LCAicmVnLXNoaWZ0IiwgJmhkbWktPnJlZ19zaGlmdCkpCj4gKyBkZXZfd2FybihoZG1pLT5kZXYs ICJObyByZWctc2hpZnRcbiIpOwo+Cj4gVGhpcyB3YXkgdGhlIHJlZy1zaGlmdCBwcm9wZXJ0eSBj YW4gYmUgZGVmaW5lZCB1c2luZyBEVAoKcmszMjg4IGNhbiBvbmx5IGFjY2VzcyB0aGUgcmVnaXN0 ZXIgYnkgMzJiaXRzKHJlYWRsL3dyaXRlbCksIGJ5dGUgYWNjZXNzIAp3aWxsIGNhdXNlcyBhbgpp bXByZWNpc2UgZXh0ZXJuYWwgYWJvcnQuCkkgaGF2ZSByZWZhY3RvciB0aGUgcmVnaXN0ZXIgYWNj ZXNzIGluIFBBVENIIFYzLCBpZiB5b3UgaGF2ZSBhbnkgZnV0aGVyIApzdWdnZXN0aW9uICxwbGVh c2UKdGVsbCBtZS4KCj4KPiBDaGVlcnMsCj4gWnViYWlyTEsKPgo+IE9uIDA1LzExLzE0IDEyOjU5 LCBBbmR5IFlhbiB3cm90ZToKPj4gaW14NiBhbmQgcm9ja2NoaXAgcmszMjg4IGFuZCBKWjQ3ODAg KEluZ2VuaWMgWGJ1cnN0L01JUFMpCj4+IHVzZSB0aGUgaW50ZXJmYWNlIGNvbXBhdGlibGUgRGVz aWdud2FyZSBIRE1JIElQLCBidXQgdGhleQo+PiBhbHNvIGhhdmUgc29tZSBsaWdodGx5IGRpZmZl cmVuY2UsIHN1Y2ggYXMgcGh5IHBsbCBjb25maWd1cmF0aW9uLAo+PiByZWdpc3RlciB3aWR0aChp bXggaGRtaSByZWdpc3RlciBpcyBvbmUgYnl0ZSwgYnV0IHJrMzI4OCBpcyA0Cj4+IGJ5dGVzIHdp ZHRoKSwgNEsgc3VwcG9ydChpbXg2IGRvZXNuJ3Qgc3VwcG9ydCA0aywgYnV0IHJrMzI4OCBkb2Vz KSwKPj4gY2xrIHVzZWFnZSxhbmQgdGhlIGNydGMgbXV4IGNvbmZpZ3VyYXRpb24gaXMgYWxzbyBw bGF0Zm9ybSBzcGVjaWZpYy4KPj4KPj4gVG8gcmV1c2UgdGhlIGlteCBoZG1pIGRyaXZlciwgc3Bs aXQgdGhlIHBsYXRmb3JtIHNwZWNpZmljIGNvZGUgb3V0Cj4+IHRvIGR3X2hkbWktaW14LmMuCj4+ Cj4+IFNpZ25lZC1vZmYtYnk6IEFuZHkgWWFuIDxhbmR5LnlhbkByb2NrLWNoaXBzLmNvbT4KPj4g LS0tCj4+ICAgZHJpdmVycy9zdGFnaW5nL2lteC1kcm0vTWFrZWZpbGUgICAgICB8ICAgMiArLQo+ PiAgIGRyaXZlcnMvc3RhZ2luZy9pbXgtZHJtL2R3X2hkbWktaW14LmMgfCAyMTQgKysrKysrKysr Kwo+PiAgIGRyaXZlcnMvc3RhZ2luZy9pbXgtZHJtL2lteC1oZG1pLmMgICAgfCA3MjYgKysrKysr KysrKysrKystLS0tLS0tLS0tLS0tLS0tLS0tLQo+PiAgIGluY2x1ZGUvZHJtL2JyaWRnZS9kd19o ZG1pLmggICAgICAgICAgfCAxMTQgKysrKysrCj4+ICAgNCBmaWxlcyBjaGFuZ2VkLCA2MzQgaW5z ZXJ0aW9ucygrKSwgNDIyIGRlbGV0aW9ucygtKQo+PiAgIGNyZWF0ZSBtb2RlIDEwMDY0NCBkcml2 ZXJzL3N0YWdpbmcvaW14LWRybS9kd19oZG1pLWlteC5jCj4+ICAgY3JlYXRlIG1vZGUgMTAwNjQ0 IGluY2x1ZGUvZHJtL2JyaWRnZS9kd19oZG1pLmgKPgo+CgoKX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX18KZGV2ZWwgbWFpbGluZyBsaXN0CmRldmVsQGxpbnV4 ZHJpdmVycHJvamVjdC5vcmcKaHR0cDovL2RyaXZlcmRldi5saW51eGRyaXZlcnByb2plY3Qub3Jn L21haWxtYW4vbGlzdGluZm8vZHJpdmVyZGV2LWRldmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751410AbaKGC5N (ORCPT ); Thu, 6 Nov 2014 21:57:13 -0500 Received: from regular1.263xmail.com ([211.150.99.131]:49838 "EHLO regular1.263xmail.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751069AbaKGC5L (ORCPT ); Thu, 6 Nov 2014 21:57:11 -0500 X-263anti-spam: KSV:0; X-MAIL-GRAY: 0 X-MAIL-DELIVERY: 1 X-KSVirus-check: 0 X-ABS-CHECKED: 4 X-RL-SENDER: andy.yan@rock-chips.com X-FST-TO: linux-rockchip@lists.infradead.org X-SENDER-IP: 121.15.173.1 X-LOGIN-NAME: andy.yan@rock-chips.com X-UNIQUE-TAG: <1c7656c0e0f06c4a10f02cc9d109840e> X-ATTACHMENT-NUM: 0 X-DNS-TYPE: 0 Message-ID: <545C34F1.5020206@rock-chips.com> Date: Fri, 07 Nov 2014 10:56:49 +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: Zubair Lutfullah Kakakhel , airlied@linux.ie, heiko@sntech.de, fabio.estevam@freescale.com, rmk+kernel@arm.linux.org.uk CC: Greg Kroah-Hartman , Grant Likely , Rob Herring , Philipp Zabel , Shawn Guo , Josh Boyer , Sean Paul , Inki Dae , Dave Airlie , Arnd Bergmann , Lucas Stach , 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 Subject: Re: [PATCH V2 1/2] imx-drm: imx-hdmi: split imx soc specific code from imx-hdmi References: <1415192385-6572-1-git-send-email-andy.yan@rock-chips.com> <545A2680.2@imgtec.com> In-Reply-To: <545A2680.2@imgtec.com> 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 On 2014年11月05日 21:30, Zubair Lutfullah Kakakhel wrote: > This one patch does too much to be reviewed easily. > > One patch is supposed to modify/add one thing at a time in the kernel. > > Separating platform specific code from imx-drm/imx-hdmi is one thing. > > Adding support for multi-byte register access is something different. > > i.e. Something like. > 1/3 split platform specific code out. > 2/3 move/rename imx-hdmi outside the folder > 3/3 add support for multi byte register width access. > > If there are other things that are not directly relevant to the patch, > it goes in a different patch. Bug fixes are also separate. > > This should result in readable patches that can be reviewed easily. I have split the patch in three parts in PATCH V3, tkanks for your suggestion > > Also, the approach with 4 byte access is ok. But you could use reg-shifts as well perhaps. > Then you won't have to change so much of the code. > > static inline void hdmi_writeb(struct dwc_hdmi *hdmi, u8 val, int offset) > +{ > + writeb(val, hdmi->regs + (offset << hdmi->reg_shift)); > +} > + > +static inline u8 hdmi_readb(struct dwc_hdmi *hdmi, int offset) > +{ > + return readb(hdmi->regs + (offset << hdmi->reg_shift)); > +} > > And then in probe > > +hdmi->reg_shift = 0; > + > + if (of_property_read_u32(np, "reg-shift", &hdmi->reg_shift)) > + dev_warn(hdmi->dev, "No reg-shift\n"); > > This way the reg-shift property can be defined using DT rk3288 can only access the register by 32bits(readl/writel), byte access will causes an imprecise external abort. I have refactor the register access in PATCH V3, if you have any futher suggestion ,please tell me. > > Cheers, > ZubairLK > > On 05/11/14 12:59, Andy Yan wrote: >> imx6 and rockchip rk3288 and JZ4780 (Ingenic Xburst/MIPS) >> use the interface compatible Designware HDMI IP, but they >> also have some lightly difference, such as phy pll configuration, >> register width(imx hdmi register is one byte, but rk3288 is 4 >> bytes width), 4K support(imx6 doesn't support 4k, but rk3288 does), >> clk useage,and the crtc mux configuration is also platform specific. >> >> To reuse the imx hdmi driver, split the platform specific code out >> to dw_hdmi-imx.c. >> >> Signed-off-by: Andy Yan >> --- >> drivers/staging/imx-drm/Makefile | 2 +- >> drivers/staging/imx-drm/dw_hdmi-imx.c | 214 ++++++++++ >> drivers/staging/imx-drm/imx-hdmi.c | 726 ++++++++++++++-------------------- >> include/drm/bridge/dw_hdmi.h | 114 ++++++ >> 4 files changed, 634 insertions(+), 422 deletions(-) >> create mode 100644 drivers/staging/imx-drm/dw_hdmi-imx.c >> create mode 100644 include/drm/bridge/dw_hdmi.h > >