lkml.org 
[lkml]   [2020]   [Jun]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH 1/7] drm/rockchip: prepare common code for cdns and rk dpi/dp driver
    HI Sandor Yu

    On Mon, 1 Jun 2020 at 07:29, <sandor.yu@nxp.com> wrote:
    >
    > From: Sandor Yu <Sandor.yu@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;
    }

    \
     
     \ /
      Last update: 2020-06-02 16:00    [W:2.865 / U:0.136 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site