linux-kernel - Re: [PATCH V2 1/2] imx-drm: imx-hdmi: split imx soc specific code from imx-hdmi

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <545A2680.2@imgtec.com>
Date:	Wed, 5 Nov 2014 13:30:40 +0000
From:	Zubair Lutfullah Kakakhel <Zubair.Kakakhel@...tec.com>
To:	Andy Yan <andy.yan@...k-chips.com>, <airlied@...ux.ie>,
	<heiko@...ech.de>, <fabio.estevam@...escale.com>,
	<rmk+kernel@....linux.org.uk>
CC:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Grant Likely <grant.likely@...aro.org>,
	Rob Herring <robh+dt@...nel.org>,
	Philipp Zabel <p.zabel@...gutronix.de>,
	Shawn Guo <shawn.guo@...aro.org>,
	Josh Boyer <jwboyer@...hat.com>,
	Sean Paul <seanpaul@...omium.org>,
	Inki Dae <inki.dae@...sung.com>,
	Dave Airlie <airlied@...hat.com>,
	Arnd Bergmann <arnd@...db.de>,
	Lucas Stach <l.stach@...gutronix.de>, <djkurtz@...gle.com>,
	<ykk@...k-chips.com>, <linux-kernel@...r.kernel.org>,
	<dri-devel@...ts.freedesktop.org>, <devel@...verdev.osuosl.org>,
	<devicetree@...r.kernel.org>, <linux-rockchip@...ts.infradead.org>
Subject: Re: [PATCH V2 1/2] imx-drm: imx-hdmi: split imx soc specific code
 from imx-hdmi


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.

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

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 <andy.yan@...k-chips.com>
> ---
>  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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/