Hi Adrian,
Some few comments/nits below,
Missatge de Adrian Ratiu <adrian.ratiu@xxxxxxxxxxxxx> del dia dt., 14
d’abr. 2020 a les 17:19:
>
> In order to support multiple versions of the Synopsis MIPI DSI host
> controller, which have different register layouts but almost identical
> HW protocols, we add a regmap infrastructure which can abstract away
> register accesses for platform drivers using the bridge.
>
> The controller HW revision is detected during bridge probe which will
> be used in future commits to load the relevant register layout which
> the bridge will use transparently to the platform drivers.
>
> Suggested-by: Ezequiel Garcia <ezequiel@xxxxxxxxxxxxx>
> Tested-by: Adrian Pop <pop.adrian61@xxxxxxxxx>
> Tested-by: Arnaud Ferraris <arnaud.ferraris@xxxxxxxxxxxxx>
> Signed-off-by: Adrian Ratiu <adrian.ratiu@xxxxxxxxxxxxx>
> ---
> New in v5.
> ---
> drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 208 ++++++++++--------
> 1 file changed, 117 insertions(+), 91 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> index 5ef0f154aa7b..6d9e2f21c9cc 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> @@ -15,6 +15,7 @@
> #include <linux/module.h>
> #include <linux/of_device.h>
> #include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
Should Kconfig select REGMAP for this driver?
> #include <linux/reset.h>
>
> #include <video/mipi_display.h>
> @@ -227,6 +228,7 @@ struct dw_mipi_dsi {
> struct drm_bridge *panel_bridge;
> struct device *dev;
> void __iomem *base;
> + struct regmap *regs;
>
> struct clk *pclk;
>
> @@ -235,6 +237,7 @@ struct dw_mipi_dsi {
> u32 lanes;
> u32 format;
> unsigned long mode_flags;
> + u32 hw_version;
>
> #ifdef CONFIG_DEBUG_FS
> struct dentry *debugfs;
> @@ -249,6 +252,13 @@ struct dw_mipi_dsi {
> const struct dw_mipi_dsi_plat_data *plat_data;
> };
>
> +static const struct regmap_config dw_mipi_dsi_regmap_cfg = {
> + .reg_bits = 32,
> + .val_bits = 32,
> + .reg_stride = 4,
> + .name = "dw-mipi-dsi",
> +};
> +
> /*
> * Check if either a link to a master or slave is present
> */
> @@ -280,16 +290,6 @@ static inline struct dw_mipi_dsi *bridge_to_dsi(struct drm_bridge *bridge)
> return container_of(bridge, struct dw_mipi_dsi, bridge);
> }
>
> -static inline void dsi_write(struct dw_mipi_dsi *dsi, u32 reg, u32 val)
> -{
> - writel(val, dsi->base + reg);
> -}
> -
> -static inline u32 dsi_read(struct dw_mipi_dsi *dsi, u32 reg)
> -{
> - return readl(dsi->base + reg);
> -}
> -
> static int dw_mipi_dsi_host_attach(struct mipi_dsi_host *host,
> struct mipi_dsi_device *device)
> {
> @@ -366,29 +366,29 @@ static void dw_mipi_message_config(struct dw_mipi_dsi *dsi,
> if (lpm)
> val |= CMD_MODE_ALL_LP;
>
> - dsi_write(dsi, DSI_LPCLK_CTRL, lpm ? 0 : PHY_TXREQUESTCLKHS);
> - dsi_write(dsi, DSI_CMD_MODE_CFG, val);
> + regmap_write(dsi->regs, DSI_LPCLK_CTRL, lpm ? 0 : PHY_TXREQUESTCLKHS);
> + regmap_write(dsi->regs, DSI_CMD_MODE_CFG, val);
> }
>
> static int dw_mipi_dsi_gen_pkt_hdr_write(struct dw_mipi_dsi *dsi, u32 hdr_val)
> {
> int ret;
> - u32 val, mask;
> + u32 val = 0, mask;
>
I think that this change is not needed, `val` is an input variable
that will be overwritten inside the regmap_read_poll_timeout.
Initialize here is not needed.
> - ret = readl_poll_timeout(dsi->base + DSI_CMD_PKT_STATUS,
> - val, !(val & GEN_CMD_FULL), 1000,
> - CMD_PKT_STATUS_TIMEOUT_US);
> + ret = regmap_read_poll_timeout(dsi->regs, DSI_CMD_PKT_STATUS,
> + val, !(val & GEN_CMD_FULL), 1000,
> + CMD_PKT_STATUS_TIMEOUT_US);
> if (ret) {
> dev_err(dsi->dev, "failed to get available command FIFO\n");
> return ret;
> }
>
> - dsi_write(dsi, DSI_GEN_HDR, hdr_val);
> + regmap_write(dsi->regs, DSI_GEN_HDR, hdr_val);
>
> mask = GEN_CMD_EMPTY | GEN_PLD_W_EMPTY;
> - ret = readl_poll_timeout(dsi->base + DSI_CMD_PKT_STATUS,
> - val, (val & mask) == mask,
> - 1000, CMD_PKT_STATUS_TIMEOUT_US);
> + ret = regmap_read_poll_timeout(dsi->regs, DSI_CMD_PKT_STATUS,
> + val, (val & mask) == mask,
> + 1000, CMD_PKT_STATUS_TIMEOUT_US);
> if (ret) {
> dev_err(dsi->dev, "failed to write command FIFO\n");
> return ret;
> @@ -403,24 +403,26 @@ static int dw_mipi_dsi_write(struct dw_mipi_dsi *dsi,
> const u8 *tx_buf = packet->payload;
> int len = packet->payload_length, pld_data_bytes = sizeof(u32), ret;
> __le32 word;
> - u32 val;
> + u32 val = 0;
>
The same here, 'val' is used for the regmap_read_poll_timeout and will
be overwritten, no need to initialize.
> while (len) {
> if (len < pld_data_bytes) {
> word = 0;
> memcpy(&word, tx_buf, len);
> - dsi_write(dsi, DSI_GEN_PLD_DATA, le32_to_cpu(word));
> + regmap_write(dsi->regs, DSI_GEN_PLD_DATA,
> + le32_to_cpu(word));
> len = 0;
> } else {
> memcpy(&word, tx_buf, pld_data_bytes);
> - dsi_write(dsi, DSI_GEN_PLD_DATA, le32_to_cpu(word));
> + regmap_write(dsi->regs, DSI_GEN_PLD_DATA,
> + le32_to_cpu(word));
> tx_buf += pld_data_bytes;
> len -= pld_data_bytes;
> }
>
> - ret = readl_poll_timeout(dsi->base + DSI_CMD_PKT_STATUS,
> - val, !(val & GEN_PLD_W_FULL), 1000,
> - CMD_PKT_STATUS_TIMEOUT_US);
> + ret = regmap_read_poll_timeout(dsi->regs, DSI_CMD_PKT_STATUS,
> + val, !(val & GEN_PLD_W_FULL),
> + 1000, CMD_PKT_STATUS_TIMEOUT_US);
> if (ret) {
> dev_err(dsi->dev,
> "failed to get available write payload FIFO\n");
> @@ -438,12 +440,12 @@ static int dw_mipi_dsi_read(struct dw_mipi_dsi *dsi,
> {
> int i, j, ret, len = msg->rx_len;
> u8 *buf = msg->rx_buf;
> - u32 val;
> + u32 val = 0;
>
ditto
> /* Wait end of the read operation */
> - ret = readl_poll_timeout(dsi->base + DSI_CMD_PKT_STATUS,
> - val, !(val & GEN_RD_CMD_BUSY),
> - 1000, CMD_PKT_STATUS_TIMEOUT_US);
> + ret = regmap_read_poll_timeout(dsi->regs, DSI_CMD_PKT_STATUS,
> + val, !(val & GEN_RD_CMD_BUSY),
> + 1000, CMD_PKT_STATUS_TIMEOUT_US);
> if (ret) {
> dev_err(dsi->dev, "Timeout during read operation\n");
> return ret;
> @@ -451,15 +453,15 @@ static int dw_mipi_dsi_read(struct dw_mipi_dsi *dsi,
>
> for (i = 0; i < len; i += 4) {
> /* Read fifo must not be empty before all bytes are read */
> - ret = readl_poll_timeout(dsi->base + DSI_CMD_PKT_STATUS,
> - val, !(val & GEN_PLD_R_EMPTY),
> - 1000, CMD_PKT_STATUS_TIMEOUT_US);
> + ret = regmap_read_poll_timeout(dsi->regs, DSI_CMD_PKT_STATUS,
> + val, !(val & GEN_PLD_R_EMPTY),
> + 1000, CMD_PKT_STATUS_TIMEOUT_US);
> if (ret) {
> dev_err(dsi->dev, "Read payload FIFO is empty\n");
> return ret;
> }
>
> - val = dsi_read(dsi, DSI_GEN_PLD_DATA);
> + regmap_read(dsi->regs, DSI_GEN_PLD_DATA, &val);
> for (j = 0; j < 4 && j + i < len; j++)
> buf[i + j] = val >> (8 * j);
> }
> @@ -536,29 +538,29 @@ static void dw_mipi_dsi_video_mode_config(struct dw_mipi_dsi *dsi)
> }
> #endif /* CONFIG_DEBUG_FS */
>
> - dsi_write(dsi, DSI_VID_MODE_CFG, val);
> + regmap_write(dsi->regs, DSI_VID_MODE_CFG, val);
> }
>
> static void dw_mipi_dsi_set_mode(struct dw_mipi_dsi *dsi,
> unsigned long mode_flags)
> {
> - dsi_write(dsi, DSI_PWR_UP, RESET);
> + regmap_write(dsi->regs, DSI_PWR_UP, RESET);
>
> if (mode_flags & MIPI_DSI_MODE_VIDEO) {
> - dsi_write(dsi, DSI_MODE_CFG, ENABLE_VIDEO_MODE);
> + regmap_write(dsi->regs, DSI_MODE_CFG, ENABLE_VIDEO_MODE);
> dw_mipi_dsi_video_mode_config(dsi);
> - dsi_write(dsi, DSI_LPCLK_CTRL, PHY_TXREQUESTCLKHS);
> + regmap_write(dsi->regs, DSI_LPCLK_CTRL, PHY_TXREQUESTCLKHS);
> } else {
> - dsi_write(dsi, DSI_MODE_CFG, ENABLE_CMD_MODE);
> + regmap_write(dsi->regs, DSI_MODE_CFG, ENABLE_CMD_MODE);
> }
>
> - dsi_write(dsi, DSI_PWR_UP, POWERUP);
> + regmap_write(dsi->regs, DSI_PWR_UP, POWERUP);
> }
>
> static void dw_mipi_dsi_disable(struct dw_mipi_dsi *dsi)
> {
> - dsi_write(dsi, DSI_PWR_UP, RESET);
> - dsi_write(dsi, DSI_PHY_RSTZ, PHY_RSTZ);
> + regmap_write(dsi->regs, DSI_PWR_UP, RESET);
> + regmap_write(dsi->regs, DSI_PHY_RSTZ, PHY_RSTZ);
> }
>
> static void dw_mipi_dsi_init(struct dw_mipi_dsi *dsi)
> @@ -573,14 +575,14 @@ static void dw_mipi_dsi_init(struct dw_mipi_dsi *dsi)
> */
> u32 esc_clk_division = (dsi->lane_mbps >> 3) / 20 + 1;
>
> - dsi_write(dsi, DSI_PWR_UP, RESET);
> + regmap_write(dsi->regs, DSI_PWR_UP, RESET);
>
> /*
> * TODO dw drv improvements
> * timeout clock division should be computed with the
> * high speed transmission counter timeout and byte lane...
> */
> - dsi_write(dsi, DSI_CLKMGR_CFG, TO_CLK_DIVISION(10) |
> + regmap_write(dsi->regs, DSI_CLKMGR_CFG, TO_CLK_DIVISION(10) |
> TX_ESC_CLK_DIVISION(esc_clk_division));
> }
>
> @@ -609,22 +611,22 @@ static void dw_mipi_dsi_dpi_config(struct dw_mipi_dsi *dsi,
> if (mode->flags & DRM_MODE_FLAG_NHSYNC)
> val |= HSYNC_ACTIVE_LOW;
>
> - dsi_write(dsi, DSI_DPI_VCID, DPI_VCID(dsi->channel));
> - dsi_write(dsi, DSI_DPI_COLOR_CODING, color);
> - dsi_write(dsi, DSI_DPI_CFG_POL, val);
> + regmap_write(dsi->regs, DSI_DPI_VCID, DPI_VCID(dsi->channel));
> + regmap_write(dsi->regs, DSI_DPI_COLOR_CODING, color);
> + regmap_write(dsi->regs, DSI_DPI_CFG_POL, val);
> /*
> * TODO dw drv improvements
> * largest packet sizes during hfp or during vsa/vpb/vfp
> * should be computed according to byte lane, lane number and only
> * if sending lp cmds in high speed is enable (PHY_TXREQUESTCLKHS)
> */
> - dsi_write(dsi, DSI_DPI_LP_CMD_TIM, OUTVACT_LPCMD_TIME(4)
> + regmap_write(dsi->regs, DSI_DPI_LP_CMD_TIM, OUTVACT_LPCMD_TIME(4)
> | INVACT_LPCMD_TIME(4));
> }
>
> static void dw_mipi_dsi_packet_handler_config(struct dw_mipi_dsi *dsi)
> {
> - dsi_write(dsi, DSI_PCKHDL_CFG, CRC_RX_EN | ECC_RX_EN | BTA_EN);
> + regmap_write(dsi->regs, DSI_PCKHDL_CFG, CRC_RX_EN | ECC_RX_EN | BTA_EN);
> }
>
> static void dw_mipi_dsi_video_packet_config(struct dw_mipi_dsi *dsi,
> @@ -638,7 +640,7 @@ static void dw_mipi_dsi_video_packet_config(struct dw_mipi_dsi *dsi,
> * non-burst video modes, see dw_mipi_dsi_video_mode_config()...
> */
>
> - dsi_write(dsi, DSI_VID_PKT_SIZE,
> + regmap_write(dsi->regs, DSI_VID_PKT_SIZE,
> dw_mipi_is_dual_mode(dsi) ?
> VID_PKT_SIZE(mode->hdisplay / 2) :
> VID_PKT_SIZE(mode->hdisplay));
> @@ -651,14 +653,15 @@ static void dw_mipi_dsi_command_mode_config(struct dw_mipi_dsi *dsi)
> * compute high speed transmission counter timeout according
> * to the timeout clock division (TO_CLK_DIVISION) and byte lane...
> */
> - dsi_write(dsi, DSI_TO_CNT_CFG, HSTX_TO_CNT(1000) | LPRX_TO_CNT(1000));
> + regmap_write(dsi->regs, DSI_TO_CNT_CFG,
> + HSTX_TO_CNT(1000) | LPRX_TO_CNT(1000));
> /*
> * TODO dw drv improvements
> * the Bus-Turn-Around Timeout Counter should be computed
> * according to byte lane...
> */
> - dsi_write(dsi, DSI_BTA_TO_CNT, 0xd00);
> - dsi_write(dsi, DSI_MODE_CFG, ENABLE_CMD_MODE);
> + regmap_write(dsi->regs, DSI_BTA_TO_CNT, 0xd00);
> + regmap_write(dsi->regs, DSI_MODE_CFG, ENABLE_CMD_MODE);
> }
>
> /* Get lane byte clock cycles. */
> @@ -692,13 +695,13 @@ static void dw_mipi_dsi_line_timer_config(struct dw_mipi_dsi *dsi,
> * computations below may be improved...
> */
> lbcc = dw_mipi_dsi_get_hcomponent_lbcc(dsi, mode, htotal);
> - dsi_write(dsi, DSI_VID_HLINE_TIME, lbcc);
> + regmap_write(dsi->regs, DSI_VID_HLINE_TIME, lbcc);
>
> lbcc = dw_mipi_dsi_get_hcomponent_lbcc(dsi, mode, hsa);
> - dsi_write(dsi, DSI_VID_HSA_TIME, lbcc);
> + regmap_write(dsi->regs, DSI_VID_HSA_TIME, lbcc);
>
> lbcc = dw_mipi_dsi_get_hcomponent_lbcc(dsi, mode, hbp);
> - dsi_write(dsi, DSI_VID_HBP_TIME, lbcc);
> + regmap_write(dsi->regs, DSI_VID_HBP_TIME, lbcc);
> }
>
> static void dw_mipi_dsi_vertical_timing_config(struct dw_mipi_dsi *dsi,
> @@ -711,10 +714,10 @@ static void dw_mipi_dsi_vertical_timing_config(struct dw_mipi_dsi *dsi,
> vfp = mode->vsync_start - mode->vdisplay;
> vbp = mode->vtotal - mode->vsync_end;
>
> - dsi_write(dsi, DSI_VID_VACTIVE_LINES, vactive);
> - dsi_write(dsi, DSI_VID_VSA_LINES, vsa);
> - dsi_write(dsi, DSI_VID_VFP_LINES, vfp);
> - dsi_write(dsi, DSI_VID_VBP_LINES, vbp);
> + regmap_write(dsi->regs, DSI_VID_VACTIVE_LINES, vactive);
> + regmap_write(dsi->regs, DSI_VID_VSA_LINES, vsa);
> + regmap_write(dsi->regs, DSI_VID_VFP_LINES, vfp);
> + regmap_write(dsi->regs, DSI_VID_VBP_LINES, vbp);
> }
>
> static void dw_mipi_dsi_dphy_timing_config(struct dw_mipi_dsi *dsi)
> @@ -737,23 +740,25 @@ static void dw_mipi_dsi_dphy_timing_config(struct dw_mipi_dsi *dsi)
> * DSI_CMD_MODE_CFG.MAX_RD_PKT_SIZE_LP (see CMD_MODE_ALL_LP)
> */
>
> - hw_version = dsi_read(dsi, DSI_VERSION) & VERSION;
> + regmap_read(dsi->regs, DSI_VERSION, &hw_version);
> + hw_version &= VERSION;
If I am not wrong, you already introduced a function to get the
hardware version and is stored in dsi->hw_version. Maybe you could you
use that variable and remove that code.
>
> if (hw_version >= HWVER_131) {
> - dsi_write(dsi, DSI_PHY_TMR_CFG,
> - PHY_HS2LP_TIME_V131(timing.data_hs2lp) |
> - PHY_LP2HS_TIME_V131(timing.data_lp2hs));
> - dsi_write(dsi, DSI_PHY_TMR_RD_CFG, MAX_RD_TIME_V131(10000));
> + regmap_write(dsi->regs, DSI_PHY_TMR_CFG,
> + PHY_HS2LP_TIME_V131(timing.data_hs2lp) |
> + PHY_LP2HS_TIME_V131(timing.data_lp2hs));
> + regmap_write(dsi->regs, DSI_PHY_TMR_RD_CFG,
> + MAX_RD_TIME_V131(10000));
> } else {
> - dsi_write(dsi, DSI_PHY_TMR_CFG,
> - PHY_HS2LP_TIME(timing.data_hs2lp) |
> - PHY_LP2HS_TIME(timing.data_lp2hs) |
> - MAX_RD_TIME(10000));
> + regmap_write(dsi->regs, DSI_PHY_TMR_CFG,
> + PHY_HS2LP_TIME(timing.data_hs2lp) |
> + PHY_LP2HS_TIME(timing.data_lp2hs) |
> + MAX_RD_TIME(10000));
> }
>
> - dsi_write(dsi, DSI_PHY_TMR_LPCLK_CFG,
> - PHY_CLKHS2LP_TIME(timing.clk_hs2lp) |
> - PHY_CLKLP2HS_TIME(timing.clk_lp2hs));
> + regmap_write(dsi->regs, DSI_PHY_TMR_LPCLK_CFG,
> + PHY_CLKHS2LP_TIME(timing.clk_hs2lp) |
> + PHY_CLKLP2HS_TIME(timing.clk_lp2hs));
> }
>
> static void dw_mipi_dsi_dphy_interface_config(struct dw_mipi_dsi *dsi)
> @@ -763,46 +768,49 @@ static void dw_mipi_dsi_dphy_interface_config(struct dw_mipi_dsi *dsi)
> * stop wait time should be the maximum between host dsi
> * and panel stop wait times
> */
> - dsi_write(dsi, DSI_PHY_IF_CFG, PHY_STOP_WAIT_TIME(0x20) |
> - N_LANES(dsi->lanes));
> + regmap_write(dsi->regs, DSI_PHY_IF_CFG,
> + PHY_STOP_WAIT_TIME(0x20) | N_LANES(dsi->lanes));
> }
>
> static void dw_mipi_dsi_dphy_init(struct dw_mipi_dsi *dsi)
> {
> /* Clear PHY state */
> - dsi_write(dsi, DSI_PHY_RSTZ, PHY_DISFORCEPLL | PHY_DISABLECLK
> - | PHY_RSTZ | PHY_SHUTDOWNZ);
> - dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_UNTESTCLR);
> - dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_TESTCLR);
> - dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_UNTESTCLR);
> + regmap_write(dsi->regs, DSI_PHY_RSTZ, PHY_DISFORCEPLL | PHY_DISABLECLK
> + | PHY_RSTZ | PHY_SHUTDOWNZ);
> + regmap_write(dsi->regs, DSI_PHY_TST_CTRL0, PHY_UNTESTCLR);
> + regmap_write(dsi->regs, DSI_PHY_TST_CTRL0, PHY_TESTCLR);
> + regmap_write(dsi->regs, DSI_PHY_TST_CTRL0, PHY_UNTESTCLR);
> }
>
> static void dw_mipi_dsi_dphy_enable(struct dw_mipi_dsi *dsi)
> {
> - u32 val;
> + u32 val = 0;
ditto
> int ret;
>
> - dsi_write(dsi, DSI_PHY_RSTZ, PHY_ENFORCEPLL | PHY_ENABLECLK |
> - PHY_UNRSTZ | PHY_UNSHUTDOWNZ);
> + regmap_write(dsi->regs, DSI_PHY_RSTZ, PHY_ENFORCEPLL | PHY_ENABLECLK |
> + PHY_UNRSTZ | PHY_UNSHUTDOWNZ);
>
> - ret = readl_poll_timeout(dsi->base + DSI_PHY_STATUS, val,
> - val & PHY_LOCK, 1000, PHY_STATUS_TIMEOUT_US);
> + ret = regmap_read_poll_timeout(dsi->regs, DSI_PHY_STATUS,
> + val, val & PHY_LOCK,
> + 1000, PHY_STATUS_TIMEOUT_US);
> if (ret)
> DRM_DEBUG_DRIVER("failed to wait phy lock state\n");
>
> - ret = readl_poll_timeout(dsi->base + DSI_PHY_STATUS,
> - val, val & PHY_STOP_STATE_CLK_LANE, 1000,
> - PHY_STATUS_TIMEOUT_US);
> + ret = regmap_read_poll_timeout(dsi->regs, DSI_PHY_STATUS,
> + val, val & PHY_STOP_STATE_CLK_LANE, 1000,
> + PHY_STATUS_TIMEOUT_US);
> if (ret)
> DRM_DEBUG_DRIVER("failed to wait phy clk lane stop state\n");
> }
>
> static void dw_mipi_dsi_clear_err(struct dw_mipi_dsi *dsi)
> {
> - dsi_read(dsi, DSI_INT_ST0);
> - dsi_read(dsi, DSI_INT_ST1);
> - dsi_write(dsi, DSI_INT_MSK0, 0);
> - dsi_write(dsi, DSI_INT_MSK1, 0);
> + u32 val;
> +
> + regmap_read(dsi->regs, DSI_INT_ST0, &val);
> + regmap_read(dsi->regs, DSI_INT_ST1, &val);
> + regmap_write(dsi->regs, DSI_INT_MSK0, 0);
> + regmap_write(dsi->regs, DSI_INT_MSK1, 0);
> }
>
> static void dw_mipi_dsi_bridge_post_disable(struct drm_bridge *bridge)
> @@ -989,6 +997,14 @@ static void dw_mipi_dsi_debugfs_remove(struct dw_mipi_dsi *dsi) { }
>
> #endif /* CONFIG_DEBUG_FS */
>
> +static void dw_mipi_dsi_get_hw_version(struct dw_mipi_dsi *dsi)
> +{
> + regmap_read(dsi->regs, DSI_VERSION, &dsi->hw_version);
> + dsi->hw_version &= VERSION;
> + if (!dsi->hw_version)
> + dev_err(dsi->dev, "Failed to read DSI hw version register\n");
Is this an error that should be ignored? If you can't get the HW
version, probably, there is something wrong with your hardware so,
don't you need to return an error?
> +}
> +
> static struct dw_mipi_dsi *
> __dw_mipi_dsi_probe(struct platform_device *pdev,
> const struct dw_mipi_dsi_plat_data *plat_data)
> @@ -1020,6 +1036,14 @@ __dw_mipi_dsi_probe(struct platform_device *pdev,
> dsi->base = plat_data->base;
> }
>
> + dsi->regs = devm_regmap_init_mmio(dev, dsi->base,
> + &dw_mipi_dsi_regmap_cfg);
> + if (IS_ERR(dsi->regs)) {
> + ret = PTR_ERR(dsi->regs);
> + DRM_ERROR("Failed to create DW MIPI DSI regmap: %d\n", ret);
> + return ERR_PTR(ret);
> + }
> +
> dsi->pclk = devm_clk_get(dev, "pclk");
> if (IS_ERR(dsi->pclk)) {
> ret = PTR_ERR(dsi->pclk);
> @@ -1055,6 +1079,8 @@ __dw_mipi_dsi_probe(struct platform_device *pdev,
> clk_disable_unprepare(dsi->pclk);
> }
>
> + dw_mipi_dsi_get_hw_version(dsi);
> +
> dw_mipi_dsi_debugfs_init(dsi);
> pm_runtime_enable(dev);
>
> --
> 2.26.0
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
