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 <andy.yan@xxxxxxxxxxxxxx>
---
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 devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
