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

From: Zubair Lutfullah Kakakhel
Date: Tue Nov 04 2014 - 09:25:48 EST


Hi Andy,


On 04/11/14 13:33, 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.
>
> Change-Id: I85e8d08754052b118423729a01c6d17bf485f383
> ---
> 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
>

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.

The maintainers might be able to guide on how to further split the patches if necessary.

Cheers,
ZubairLK
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/