2017-08-01 13:27:34

by Philippe Cornu

[permalink] [raw]
Subject: [PATCH v3 0/3] drm/bridge/synopsys: dsi: Various cleanups

Version 3:
- Add devm_reset_control_get_optional_exclusive (Philipp Zabel).

Version 2:
- Put back Synopsys mipi dsi unused registers.
- Add devm_reset_control_get_exclusive to follow explicit reset API.
- Add a missing commit message & reviewed-by.

Version 1:
- Initial commit

The purpose of this set of patches is to clean up the mipi dsi dw
Synopsys drm bridge.

Philippe CORNU (3):
drm/bridge/synopsys: dsi: Constify funcs structures
drm/bridge/synopsys: dsi: Register list clean up
drm/bridge/synopsys: dsi: explicitly request exclusive reset control

drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 104 +++++++++++++++-----------
1 file changed, 60 insertions(+), 44 deletions(-)

--
1.9.1


2017-08-01 13:24:08

by Philippe Cornu

[permalink] [raw]
Subject: [PATCH v3 3/3] drm/bridge/synopsys: dsi: explicitly request exclusive reset control

Based on patch "Convert drivers to explicit reset API" from Philipp Zabel

Commit a53e35db70d1 ("reset: Ensure drivers are explicit when requesting
reset lines") started to transition the reset control request API calls
to explicitly state whether the driver needs exclusive or shared reset
control behavior. Convert all drivers requesting exclusive resets to the
explicit API call so the temporary transition helpers can be removed.

No functional changes.

Cc: Philipp Zabel <[email protected]>
Signed-off-by: Philippe CORNU <[email protected]>
Acked-by: Philipp Zabel <[email protected]>
---
drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
index 781340d..4e1f91e 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
@@ -885,15 +885,11 @@ static int dw_mipi_dsi_bridge_attach(struct drm_bridge *bridge)
* Note that the reset was not defined in the initial device tree, so
* we have to be prepared for it not being found.
*/
- apb_rst = devm_reset_control_get(dev, "apb");
+ apb_rst = devm_reset_control_get_optional_exclusive(dev, "apb");
if (IS_ERR(apb_rst)) {
ret = PTR_ERR(apb_rst);
- if (ret == -ENOENT) {
- apb_rst = NULL;
- } else {
- dev_err(dev, "Unable to get reset control: %d\n", ret);
- return ERR_PTR(ret);
- }
+ dev_err(dev, "Unable to get reset control: %d\n", ret);
+ return ERR_PTR(ret);
}

if (apb_rst) {
--
1.9.1

2017-08-01 13:24:07

by Philippe Cornu

[permalink] [raw]
Subject: [PATCH v3 2/3] drm/bridge/synopsys: dsi: Register list clean up

This patch cleans up the Synopsys mipi dsi register list:
- rename registers according to the Synopsys documentation
(1.30 & 1.31)
- fix typos
- re-order registers for a better coherency

Signed-off-by: Philippe CORNU <[email protected]>
Reviewed-by: Laurent Pinchart <[email protected]>
---
drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 92 ++++++++++++++++-----------
1 file changed, 56 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
index 63c7a01..781340d 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
@@ -30,19 +30,20 @@
#include <video/mipi_display.h>

#define DSI_VERSION 0x00
+
#define DSI_PWR_UP 0x04
#define RESET 0
#define POWERUP BIT(0)

#define DSI_CLKMGR_CFG 0x08
-#define TO_CLK_DIVIDSION(div) (((div) & 0xff) << 8)
-#define TX_ESC_CLK_DIVIDSION(div) (((div) & 0xff) << 0)
+#define TO_CLK_DIVISION(div) (((div) & 0xff) << 8)
+#define TX_ESC_CLK_DIVISION(div) ((div) & 0xff)

#define DSI_DPI_VCID 0x0c
-#define DPI_VID(vid) (((vid) & 0x3) << 0)
+#define DPI_VCID(vcid) ((vcid) & 0x3)

#define DSI_DPI_COLOR_CODING 0x10
-#define EN18_LOOSELY BIT(8)
+#define LOOSELY18_EN BIT(8)
#define DPI_COLOR_CODING_16BIT_1 0x0
#define DPI_COLOR_CODING_16BIT_2 0x1
#define DPI_COLOR_CODING_16BIT_3 0x2
@@ -61,22 +62,25 @@
#define OUTVACT_LPCMD_TIME(p) (((p) & 0xff) << 16)
#define INVACT_LPCMD_TIME(p) ((p) & 0xff)

+#define DSI_DBI_VCID 0x1c
#define DSI_DBI_CFG 0x20
+#define DSI_DBI_PARTITIONING_EN 0x24
#define DSI_DBI_CMDSIZE 0x28

#define DSI_PCKHDL_CFG 0x2c
-#define EN_CRC_RX BIT(4)
-#define EN_ECC_RX BIT(3)
-#define EN_BTA BIT(2)
-#define EN_EOTP_RX BIT(1)
-#define EN_EOTP_TX BIT(0)
+#define CRC_RX_EN BIT(4)
+#define ECC_RX_EN BIT(3)
+#define BTA_EN BIT(2)
+#define EOTP_RX_EN BIT(1)
+#define EOTP_TX_EN BIT(0)
+
+#define DSI_GEN_VCID 0x30

#define DSI_MODE_CFG 0x34
#define ENABLE_VIDEO_MODE 0
#define ENABLE_CMD_MODE BIT(0)

#define DSI_VID_MODE_CFG 0x38
-#define FRAME_BTA_ACK BIT(14)
#define ENABLE_LOW_POWER (0x3f << 8)
#define ENABLE_LOW_POWER_MASK (0x3f << 8)
#define VID_MODE_TYPE_NON_BURST_SYNC_PULSES 0x0
@@ -85,8 +89,13 @@
#define VID_MODE_TYPE_MASK 0x3

#define DSI_VID_PKT_SIZE 0x3c
-#define VID_PKT_SIZE(p) (((p) & 0x3fff) << 0)
-#define VID_PKT_MAX_SIZE 0x3fff
+#define VID_PKT_SIZE(p) ((p) & 0x3fff)
+
+#define DSI_VID_NUM_CHUNKS 0x40
+#define VID_NUM_CHUNKS(c) ((c) & 0x1fff)
+
+#define DSI_VID_NULL_SIZE 0x44
+#define VID_NULL_SIZE(b) ((b) & 0x1fff)

#define DSI_VID_HSA_TIME 0x48
#define DSI_VID_HBP_TIME 0x4c
@@ -95,6 +104,8 @@
#define DSI_VID_VBP_LINES 0x58
#define DSI_VID_VFP_LINES 0x5c
#define DSI_VID_VACTIVE_LINES 0x60
+#define DSI_EDPI_CMD_SIZE 0x64
+
#define DSI_CMD_MODE_CFG 0x68
#define MAX_RD_PKT_SIZE_LP BIT(24)
#define DCS_LW_TX_LP BIT(19)
@@ -108,8 +119,8 @@
#define GEN_SW_2P_TX_LP BIT(10)
#define GEN_SW_1P_TX_LP BIT(9)
#define GEN_SW_0P_TX_LP BIT(8)
-#define EN_ACK_RQST BIT(1)
-#define EN_TEAR_FX BIT(0)
+#define ACK_RQST_EN BIT(1)
+#define TEAR_FX_EN BIT(0)

#define CMD_MODE_ALL_LP (MAX_RD_PKT_SIZE_LP | \
DCS_LW_TX_LP | \
@@ -125,27 +136,31 @@
GEN_SW_0P_TX_LP)

#define DSI_GEN_HDR 0x6c
+/* TODO These 2 defines will be reworked thanks to mipi_dsi_create_packet() */
#define GEN_HDATA(data) (((data) & 0xffff) << 8)
-#define GEN_HDATA_MASK (0xffff << 8)
#define GEN_HTYPE(type) (((type) & 0xff) << 0)
-#define GEN_HTYPE_MASK 0xff

#define DSI_GEN_PLD_DATA 0x70

#define DSI_CMD_PKT_STATUS 0x74
-#define GEN_CMD_EMPTY BIT(0)
-#define GEN_CMD_FULL BIT(1)
-#define GEN_PLD_W_EMPTY BIT(2)
-#define GEN_PLD_W_FULL BIT(3)
-#define GEN_PLD_R_EMPTY BIT(4)
-#define GEN_PLD_R_FULL BIT(5)
#define GEN_RD_CMD_BUSY BIT(6)
+#define GEN_PLD_R_FULL BIT(5)
+#define GEN_PLD_R_EMPTY BIT(4)
+#define GEN_PLD_W_FULL BIT(3)
+#define GEN_PLD_W_EMPTY BIT(2)
+#define GEN_CMD_FULL BIT(1)
+#define GEN_CMD_EMPTY BIT(0)

#define DSI_TO_CNT_CFG 0x78
#define HSTX_TO_CNT(p) (((p) & 0xffff) << 16)
#define LPRX_TO_CNT(p) ((p) & 0xffff)

+#define DSI_HS_RD_TO_CNT 0x7c
+#define DSI_LP_RD_TO_CNT 0x80
+#define DSI_HS_WR_TO_CNT 0x84
+#define DSI_LP_WR_TO_CNT 0x88
#define DSI_BTA_TO_CNT 0x8c
+
#define DSI_LPCLK_CTRL 0x94
#define AUTO_CLKLANE_CTRL BIT(1)
#define PHY_TXREQUESTCLKHS BIT(0)
@@ -154,6 +169,7 @@
#define PHY_CLKHS2LP_TIME(lbcc) (((lbcc) & 0x3ff) << 16)
#define PHY_CLKLP2HS_TIME(lbcc) ((lbcc) & 0x3ff)

+/* TODO Next register is slightly different between 1.30 & 1.31 IP version */
#define DSI_PHY_TMR_CFG 0x9c
#define PHY_HS2LP_TIME(lbcc) (((lbcc) & 0xff) << 24)
#define PHY_LP2HS_TIME(lbcc) (((lbcc) & 0xff) << 16)
@@ -170,12 +186,15 @@
#define PHY_UNSHUTDOWNZ BIT(0)

#define DSI_PHY_IF_CFG 0xa4
-#define N_LANES(n) ((((n) - 1) & 0x3) << 0)
#define PHY_STOP_WAIT_TIME(cycle) (((cycle) & 0xff) << 8)
+#define N_LANES(n) (((n) - 1) & 0x3)
+
+#define DSI_PHY_ULPS_CTRL 0xa8
+#define DSI_PHY_TX_TRIGGERS 0xac

#define DSI_PHY_STATUS 0xb0
-#define LOCK BIT(0)
-#define STOP_STATE_CLK_LANE BIT(2)
+#define PHY_STOP_STATE_CLK_LANE BIT(2)
+#define PHY_LOCK BIT(0)

#define DSI_PHY_TST_CTRL0 0xb4
#define PHY_TESTCLK BIT(1)
@@ -187,12 +206,13 @@
#define PHY_TESTEN BIT(16)
#define PHY_UNTESTEN 0
#define PHY_TESTDOUT(n) (((n) & 0xff) << 8)
-#define PHY_TESTDIN(n) (((n) & 0xff) << 0)
+#define PHY_TESTDIN(n) ((n) & 0xff)

#define DSI_INT_ST0 0xbc
#define DSI_INT_ST1 0xc0
#define DSI_INT_MSK0 0xc4
#define DSI_INT_MSK1 0xc8
+#define DSI_PHY_TMR_RD_CFG 0xf4

#define PHY_STATUS_TIMEOUT_US 10000
#define CMD_PKT_STATUS_TIMEOUT_US 20000
@@ -307,7 +327,7 @@ static void dw_mipi_message_config(struct dw_mipi_dsi *dsi,
u32 val = 0;

if (msg->flags & MIPI_DSI_MSG_REQ_ACK)
- val |= EN_ACK_RQST;
+ val |= ACK_RQST_EN;
if (lpm)
val |= CMD_MODE_ALL_LP;

@@ -506,8 +526,8 @@ static void dw_mipi_dsi_init(struct dw_mipi_dsi *dsi)
* timeout clock division should be computed with the
* high speed transmission counter timeout and byte lane...
*/
- dsi_write(dsi, DSI_CLKMGR_CFG, TO_CLK_DIVIDSION(10) |
- TX_ESC_CLK_DIVIDSION(esc_clk_division));
+ dsi_write(dsi, DSI_CLKMGR_CFG, TO_CLK_DIVISION(10) |
+ TX_ESC_CLK_DIVISION(esc_clk_division));
}

static void dw_mipi_dsi_dpi_config(struct dw_mipi_dsi *dsi,
@@ -520,7 +540,7 @@ static void dw_mipi_dsi_dpi_config(struct dw_mipi_dsi *dsi,
color = DPI_COLOR_CODING_24BIT;
break;
case MIPI_DSI_FMT_RGB666:
- color = DPI_COLOR_CODING_18BIT_2 | EN18_LOOSELY;
+ color = DPI_COLOR_CODING_18BIT_2 | LOOSELY18_EN;
break;
case MIPI_DSI_FMT_RGB666_PACKED:
color = DPI_COLOR_CODING_18BIT_1;
@@ -535,7 +555,7 @@ 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_VID(dsi->channel));
+ 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);
/*
@@ -550,7 +570,7 @@ static void dw_mipi_dsi_dpi_config(struct dw_mipi_dsi *dsi,

static void dw_mipi_dsi_packet_handler_config(struct dw_mipi_dsi *dsi)
{
- dsi_write(dsi, DSI_PCKHDL_CFG, EN_CRC_RX | EN_ECC_RX | EN_BTA);
+ dsi_write(dsi, DSI_PCKHDL_CFG, CRC_RX_EN | ECC_RX_EN | BTA_EN);
}

static void dw_mipi_dsi_video_packet_config(struct dw_mipi_dsi *dsi,
@@ -571,7 +591,7 @@ static void dw_mipi_dsi_command_mode_config(struct dw_mipi_dsi *dsi)
/*
* TODO dw drv improvements
* compute high speed transmission counter timeout according
- * to the timeout clock division (TO_CLK_DIVIDSION) and byte lane...
+ * 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));
/*
@@ -684,13 +704,13 @@ static void dw_mipi_dsi_dphy_enable(struct dw_mipi_dsi *dsi)
dsi_write(dsi, DSI_PHY_RSTZ, PHY_ENFORCEPLL | PHY_ENABLECLK |
PHY_UNRSTZ | PHY_UNSHUTDOWNZ);

- ret = readl_poll_timeout(dsi->base + DSI_PHY_STATUS,
- val, val & LOCK, 1000, PHY_STATUS_TIMEOUT_US);
+ ret = readl_poll_timeout(dsi->base + DSI_PHY_STATUS, val,
+ val & PHY_LOCK, 1000, PHY_STATUS_TIMEOUT_US);
if (ret < 0)
DRM_DEBUG_DRIVER("failed to wait phy lock state\n");

ret = readl_poll_timeout(dsi->base + DSI_PHY_STATUS,
- val, val & STOP_STATE_CLK_LANE, 1000,
+ val, val & PHY_STOP_STATE_CLK_LANE, 1000,
PHY_STATUS_TIMEOUT_US);
if (ret < 0)
DRM_DEBUG_DRIVER("failed to wait phy clk lane stop state\n");
--
1.9.1

2017-08-01 13:26:56

by Philippe Cornu

[permalink] [raw]
Subject: [PATCH v3 1/3] drm/bridge/synopsys: dsi: Constify funcs structures

Constify dw_mipi_dsi_bridge_funcs as these functions are not supposed
to change at runtime.

Signed-off-by: Philippe CORNU <[email protected]>
Reviewed-by: Laurent Pinchart <[email protected]>
---
drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
index 36f5ccb..63c7a01 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
@@ -811,7 +811,7 @@ static int dw_mipi_dsi_bridge_attach(struct drm_bridge *bridge)
return drm_bridge_attach(bridge->encoder, dsi->panel_bridge, bridge);
}

-static struct drm_bridge_funcs dw_mipi_dsi_bridge_funcs = {
+static const struct drm_bridge_funcs dw_mipi_dsi_bridge_funcs = {
.mode_set = dw_mipi_dsi_bridge_mode_set,
.enable = dw_mipi_dsi_bridge_enable,
.post_disable = dw_mipi_dsi_bridge_post_disable,
--
1.9.1

2017-08-01 13:32:12

by Philippe Cornu

[permalink] [raw]
Subject: Re: [PATCH v3 0/3] drm/bridge/synopsys: dsi: Various cleanups

Hi Archit,

The 2 first patches have been reviewed by Laurent. The 3rd one has been
"acked" by Philipp.

Do not hesitate to send me any comments so I can take them into account
before my holidays (end of next week)...

Many thanks for your support,

Philippe :-)


On 08/01/2017 03:23 PM, Philippe CORNU wrote:
> Version 3:
> - Add devm_reset_control_get_optional_exclusive (Philipp Zabel).
>
> Version 2:
> - Put back Synopsys mipi dsi unused registers.
> - Add devm_reset_control_get_exclusive to follow explicit reset API.
> - Add a missing commit message & reviewed-by.
>
> Version 1:
> - Initial commit
>
> The purpose of this set of patches is to clean up the mipi dsi dw
> Synopsys drm bridge.
>
> Philippe CORNU (3):
> drm/bridge/synopsys: dsi: Constify funcs structures
> drm/bridge/synopsys: dsi: Register list clean up
> drm/bridge/synopsys: dsi: explicitly request exclusive reset control
>
> drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 104 +++++++++++++++-----------
> 1 file changed, 60 insertions(+), 44 deletions(-)
>

2017-09-01 12:41:12

by Philippe Cornu

[permalink] [raw]
Subject: Re: [PATCH v3 0/3] drm/bridge/synopsys: dsi: Various cleanups

Hi Archit,
Gentle reminder :-)

Do not hesitate to send me any comments so I can update these patches.

Note that the "Constify funcs structures" patch is now obsolete as a
similar ones from Bhumika Goyal has been posted then integrated recently.

Many thanks for your support,
Philippe :-)


On 08/01/2017 03:30 PM, Philippe CORNU wrote:
> Hi Archit,
>
> The 2 first patches have been reviewed by Laurent. The 3rd one has been
> "acked" by Philipp.
>
> Do not hesitate to send me any comments so I can take them into account
> before my holidays (end of next week)...
>
> Many thanks for your support,
>
> Philippe :-)
>
>
> On 08/01/2017 03:23 PM, Philippe CORNU wrote:
>> Version 3:
>> - Add devm_reset_control_get_optional_exclusive (Philipp Zabel).
>>
>> Version 2:
>> - Put back Synopsys mipi dsi unused registers.
>> - Add devm_reset_control_get_exclusive to follow explicit reset API.
>> - Add a missing commit message & reviewed-by.
>>
>> Version 1:
>> - Initial commit
>>
>> The purpose of this set of patches is to clean up the mipi dsi dw
>> Synopsys drm bridge.
>>
>> Philippe CORNU (3):
>> drm/bridge/synopsys: dsi: Constify funcs structures
>> drm/bridge/synopsys: dsi: Register list clean up
>> drm/bridge/synopsys: dsi: explicitly request exclusive reset control
>>
>> drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 104
>> +++++++++++++++-----------
>> 1 file changed, 60 insertions(+), 44 deletions(-)
>>

2017-09-01 13:17:50

by Andrzej Hajda

[permalink] [raw]
Subject: Re: [PATCH v3 1/3] drm/bridge/synopsys: dsi: Constify funcs structures

On 01.08.2017 15:23, Philippe CORNU wrote:
> Constify dw_mipi_dsi_bridge_funcs as these functions are not supposed
> to change at runtime.
>
> Signed-off-by: Philippe CORNU <[email protected]>
> Reviewed-by: Laurent Pinchart <[email protected]>
> ---
> drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> index 36f5ccb..63c7a01 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> @@ -811,7 +811,7 @@ static int dw_mipi_dsi_bridge_attach(struct drm_bridge *bridge)
> return drm_bridge_attach(bridge->encoder, dsi->panel_bridge, bridge);
> }
>
> -static struct drm_bridge_funcs dw_mipi_dsi_bridge_funcs = {
> +static const struct drm_bridge_funcs dw_mipi_dsi_bridge_funcs = {
> .mode_set = dw_mipi_dsi_bridge_mode_set,
> .enable = dw_mipi_dsi_bridge_enable,
> .post_disable = dw_mipi_dsi_bridge_post_disable,

Reviewed-by: Andrzej Hajda <[email protected]>

--
Regards
Andrzej

2017-09-01 13:30:33

by Andrzej Hajda

[permalink] [raw]
Subject: Re: [PATCH v3 2/3] drm/bridge/synopsys: dsi: Register list clean up

On 01.08.2017 15:23, Philippe CORNU wrote:
> This patch cleans up the Synopsys mipi dsi register list:
> - rename registers according to the Synopsys documentation
> (1.30 & 1.31)
> - fix typos
> - re-order registers for a better coherency
>
> Signed-off-by: Philippe CORNU <[email protected]>
> Reviewed-by: Laurent Pinchart <[email protected]>
Reviewed-by: Andrzej Hajda <[email protected]>

--
Regards
Andrzej

2017-09-01 13:45:16

by Andrzej Hajda

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] drm/bridge/synopsys: dsi: explicitly request exclusive reset control

On 01.08.2017 15:23, Philippe CORNU wrote:
> Based on patch "Convert drivers to explicit reset API" from Philipp Zabel
>
> Commit a53e35db70d1 ("reset: Ensure drivers are explicit when requesting
> reset lines") started to transition the reset control request API calls
> to explicitly state whether the driver needs exclusive or shared reset
> control behavior. Convert all drivers requesting exclusive resets to the
> explicit API call so the temporary transition helpers can be removed.
>
> No functional changes.
>
> Cc: Philipp Zabel <[email protected]>
> Signed-off-by: Philippe CORNU <[email protected]>
> Acked-by: Philipp Zabel <[email protected]>
> ---
> drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 10 +++-------
> 1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> index 781340d..4e1f91e 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
> @@ -885,15 +885,11 @@ static int dw_mipi_dsi_bridge_attach(struct drm_bridge *bridge)
> * Note that the reset was not defined in the initial device tree, so
> * we have to be prepared for it not being found.
> */
> - apb_rst = devm_reset_control_get(dev, "apb");
> + apb_rst = devm_reset_control_get_optional_exclusive(dev, "apb");
> if (IS_ERR(apb_rst)) {
> ret = PTR_ERR(apb_rst);
> - if (ret == -ENOENT) {
> - apb_rst = NULL;
> - } else {
> - dev_err(dev, "Unable to get reset control: %d\n", ret);
> - return ERR_PTR(ret);
> - }
> + dev_err(dev, "Unable to get reset control: %d\n", ret);

I think in case of deferred probing it shouldn't be dev_err, but this is
rather suggestion.

> + return ERR_PTR(ret);

return apb_rst;

With this fixed:
Reviewed-by: Andrzej Hajda <[email protected]>

--
Regards
Andrzej

> }
>
> if (apb_rst) {


2017-09-04 11:40:04

by Archit Taneja

[permalink] [raw]
Subject: Re: [PATCH v3 2/3] drm/bridge/synopsys: dsi: Register list clean up



On 09/01/2017 06:59 PM, Andrzej Hajda wrote:
> On 01.08.2017 15:23, Philippe CORNU wrote:
>> This patch cleans up the Synopsys mipi dsi register list:
>> - rename registers according to the Synopsys documentation
>> (1.30 & 1.31)
>> - fix typos
>> - re-order registers for a better coherency
>>
>> Signed-off-by: Philippe CORNU <[email protected]>
>> Reviewed-by: Laurent Pinchart <[email protected]>
> Reviewed-by: Andrzej Hajda <[email protected]>
>

queued to drm-misc-next.

> --
> Regards
> Andrzej
>

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

2017-09-04 11:40:53

by Archit Taneja

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] drm/bridge/synopsys: dsi: explicitly request exclusive reset control



On 09/01/2017 07:15 PM, Andrzej Hajda wrote:
> On 01.08.2017 15:23, Philippe CORNU wrote:
>> Based on patch "Convert drivers to explicit reset API" from Philipp Zabel
>>
>> Commit a53e35db70d1 ("reset: Ensure drivers are explicit when requesting
>> reset lines") started to transition the reset control request API calls
>> to explicitly state whether the driver needs exclusive or shared reset
>> control behavior. Convert all drivers requesting exclusive resets to the
>> explicit API call so the temporary transition helpers can be removed.
>>
>> No functional changes.
>>
>> Cc: Philipp Zabel <[email protected]>
>> Signed-off-by: Philippe CORNU <[email protected]>
>> Acked-by: Philipp Zabel <[email protected]>
>> ---
>> drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 10 +++-------
>> 1 file changed, 3 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
>> index 781340d..4e1f91e 100644
>> --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
>> @@ -885,15 +885,11 @@ static int dw_mipi_dsi_bridge_attach(struct drm_bridge *bridge)
>> * Note that the reset was not defined in the initial device tree, so
>> * we have to be prepared for it not being found.
>> */
>> - apb_rst = devm_reset_control_get(dev, "apb");
>> + apb_rst = devm_reset_control_get_optional_exclusive(dev, "apb");
>> if (IS_ERR(apb_rst)) {
>> ret = PTR_ERR(apb_rst);
>> - if (ret == -ENOENT) {
>> - apb_rst = NULL;
>> - } else {
>> - dev_err(dev, "Unable to get reset control: %d\n", ret);
>> - return ERR_PTR(ret);
>> - }
>> + dev_err(dev, "Unable to get reset control: %d\n", ret);
>
> I think in case of deferred probing it shouldn't be dev_err, but this is
> rather suggestion.
>
>> + return ERR_PTR(ret);
>
> return apb_rst;
>
> With this fixed:
> Reviewed-by: Andrzej Hajda <[email protected]>

queued to drm-misc-next after incorporating Andrzej's comment.

>
> --
> Regards
> Andrzej
>
>> }
>>
>> if (apb_rst) {
>
>

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

2017-09-04 13:56:26

by Philippe Cornu

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] drm/bridge/synopsys: dsi: explicitly request exclusive reset control



On 09/04/2017 01:40 PM, Archit Taneja wrote:
>
>
> On 09/01/2017 07:15 PM, Andrzej Hajda wrote:
>> On 01.08.2017 15:23, Philippe CORNU wrote:
>>> Based on patch "Convert drivers to explicit reset API" from Philipp
>>> Zabel
>>>
>>> Commit a53e35db70d1 ("reset: Ensure drivers are explicit when requesting
>>> reset lines") started to transition the reset control request API calls
>>> to explicitly state whether the driver needs exclusive or shared reset
>>> control behavior. Convert all drivers requesting exclusive resets to the
>>> explicit API call so the temporary transition helpers can be removed.
>>>
>>> No functional changes.
>>>
>>> Cc: Philipp Zabel <[email protected]>
>>> Signed-off-by: Philippe CORNU <[email protected]>
>>> Acked-by: Philipp Zabel <[email protected]>
>>> ---
>>> drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 10 +++-------
>>> 1 file changed, 3 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
>>> b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
>>> index 781340d..4e1f91e 100644
>>> --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
>>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c
>>> @@ -885,15 +885,11 @@ static int dw_mipi_dsi_bridge_attach(struct
>>> drm_bridge *bridge)
>>> * Note that the reset was not defined in the initial device
>>> tree, so
>>> * we have to be prepared for it not being found.
>>> */
>>> - apb_rst = devm_reset_control_get(dev, "apb");
>>> + apb_rst = devm_reset_control_get_optional_exclusive(dev, "apb");
>>> if (IS_ERR(apb_rst)) {
>>> ret = PTR_ERR(apb_rst);
>>> - if (ret == -ENOENT) {
>>> - apb_rst = NULL;
>>> - } else {
>>> - dev_err(dev, "Unable to get reset control: %d\n", ret);
>>> - return ERR_PTR(ret);
>>> - }
>>> + dev_err(dev, "Unable to get reset control: %d\n", ret);
>>
>> I think in case of deferred probing it shouldn't be dev_err, but this is
>> rather suggestion.
>>
>>> + return ERR_PTR(ret);
>> return apb_rst;
>>
>> With this fixed:
>> Reviewed-by: Andrzej Hajda <[email protected]>
>
> queued to drm-misc-next after incorporating Andrzej's comment.
>

Dears Archit & Andrzej,
A big *THANK YOU* for review/rework/integration,
Philippe :-)

>>
>> --
>> Regards
>> Andrzej
>>
>>> }
>>> if (apb_rst) {
>>
>>
>