HI Sandor Yu On Mon, 1 Jun 2020 at 07:29, <sandor...@nxp.com> wrote: > > From: Sandor Yu <sandor...@nxp.com> > > - Extracted common fields from cdn_dp_device to a new cdns_mhdp_device > structure which will be used by two separate drivers later on. > - Moved some datatypes (audio_format, audio_info, vic_pxl_encoding_format, > video_info) from cdn-dp-core.c to cdn-dp-reg.h. > - Changed prefixes from cdn_dp to cdns_mhdp > cdn -> cdns to match the other Cadence's drivers > dp -> mhdp to distinguish it from a "just a DP" as the IP underneath > this registers map can be a HDMI (which is internally different, > but the interface for commands, events is pretty much the same). > - Modified cdn-dp-core.c to use the new driver structure and new function > names. > - writel and readl are replaced by cdns_mhdp_bus_write and > cdns_mhdp_bus_read. > The high-level idea is great - split, refactor and reuse the existing drivers.
Although looking at the patches themselves - they seems to be doing multiple things at once. As indicated by the extensive list in the commit log. I would suggest splitting those up a bit, roughly in line of the itemisation as per the commit message. Here is one hand wavy way to chunk this patch: 1) use put_unalligned* 2) 'use local variable dev' style of changes (as seem in cdn_dp_clk_enable) 3) add writel/readl wrappers 4) hookup struct cdns_mhdp_device, keep dp->mhdp detail internal. The cdn-dp-reg.h function names/signatures will stay the same. 5) finalize the helpers - use mhdp directly, rename HTH Emil Examples: 4) static int cdn_dp_mailbox_read(struct cdn_dp_device *dp) { +" struct cdns_mhdp_device *mhdp = dp->mhdp; int val, ret; - ret = readx_poll_timeout(readl, dp->regs + MAILBOX_EMPTY_ADDR, + ret = readx_poll_timeout(readl, mhdp->regs_base + MAILBOX_EMPTY_ADDR, ... return fancy_readl(dp, MAILBOX0_RD_DATA) & 0xff; } 5) -static int cdn_dp_mailbox_read(struct cdn_dp_device *dp) +static int mhdp_mailbox_read(struct cdns_mhdp_device *mhdp) { - struct cdns_mhdp_device *mhdp = dp->mhdp; int val, ret; ... - return fancy_readl(dp, MAILBOX0_RD_DATA) & 0xff; + return cdns_mhdp_bus_read(mhdp, MAILBOX0_RD_DATA) & 0xff; }